Conversation
-webhook알림 추가 -slackbot을 사용한 사용자 DM추가 -재처리 로직추가
Walkthrough이번 변경 사항에서는 notification-service에 Slack 알림 기능이 추가되었습니다. Slack 메시지 송신을 위한 클라이언트, DTO, 서비스 메서드가 도입되었으며, Kafka 이벤트 소비 시 Slack 알림을 전송하도록 로직이 확장되었습니다. 관련 도메인 enum 및 이벤트 클래스는 외부 공용 패키지로 이동되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Kafka as Kafka
participant Consumer as NotificationConsumer
participant Service as NotificationService
participant SlackClient as SlackClient
participant SlackAPI as Slack
Kafka->>Consumer: NotificationEvent 수신
Consumer->>Service: sendMessage(SlackWebHookMessage, email)
Service->>SlackClient: sendMessage(SlackWebHookMessage)
SlackClient->>SlackAPI: Webhook 메시지 전송
Service->>SlackClient: getUserIdByEmail(email)
SlackClient->>SlackAPI: /users.lookupByEmail 호출
SlackAPI-->>SlackClient: userId 반환
Service->>SlackClient: sendMessage(SlackBotMessage)
SlackClient->>SlackAPI: Bot 메시지 전송
Assessment against linked issues
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🔭 Outside diff range comments (3)
notification-service/src/main/java/com/timebank/notification_service/domain/entity/Notification.java (2)
58-65: 🧹 Nitpick (assertive)생성자 중복 코드 확인
생성자 내에서 isRead 필드를 false로 명시적 초기화하는데, 이미 클래스 레벨에서 초기화되어 있습니다(56줄).
클래스 레벨 초기화가 있으므로 생성자에서 중복 초기화는 제거할 수 있습니다:
public Notification(Long recipientId, Long senderId, NotificationType notificationType, String message) { this.recipientId = recipientId; this.senderId = senderId; this.notificationType = notificationType; this.message = message; - this.isRead = false; }
67-76: 🧹 Nitpick (assertive)생성자 코드 중복 개선
두 생성자간 중복 코드가 존재합니다. 이는 유지보수성을 저하시킬 수 있습니다.
this() 키워드를 사용해 생성자 체이닝을 적용하세요:
public Notification(Long recipientId, Long senderId, NotificationType notificationType, NotificationEventType eventType, String message) { - this.recipientId = recipientId; - this.senderId = senderId; - this.notificationType = notificationType; + this(recipientId, senderId, notificationType, message); this.eventType = eventType; - this.message = message; - this.isRead = false; }notification-service/src/main/java/com/timebank/notification_service/infrastructure/kafka/NotificationConsumer.java (1)
57-67: 🛠️ Refactor suggestion예외 처리 개선 필요
saveNotification 메서드에 예외 처리 로직이 없습니다. DB 저장 실패 시 슬랙 메시지 전송에도 영향을 줄 수 있습니다.
try-catch 블록을 추가하여 예외 처리 로직을 강화하세요:
private void saveNotification(NotificationEvent event) { + try { Notification notification = Notification.builder() .recipientId(event.getRecipientId()) .notificationType(event.getType()) .eventType(event.getEventType()) .message(event.getMessage()) .isRead(false) .build(); notificationRepository.save(notification); log.info("Notification saved: {}", notification); + } catch (Exception e) { + log.error("알림 저장 실패: {}", e.getMessage(), e); + // 필요에 따라 예외를 다시 던지거나 다른 대응 방법 적용 + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
common/src/main/java/com/timebank/common/infrastructure/external/notification/dto/NotificationEvent.java(1 hunks)notification-service/build.gradle(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/client/SlackClient.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/dto/NotificationDto.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackBotMessage.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackUserResponse.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackWebHookMessage.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/application/event/NotificationEvent.java(0 hunks)notification-service/src/main/java/com/timebank/notification_service/application/service/NotificationService.java(4 hunks)notification-service/src/main/java/com/timebank/notification_service/domain/entity/Notification.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/domain/entity/NotificationEventType.java(0 hunks)notification-service/src/main/java/com/timebank/notification_service/domain/entity/NotificationType.java(0 hunks)notification-service/src/main/java/com/timebank/notification_service/infrastructure/kafka/NotificationConsumer.java(3 hunks)notification-service/src/main/java/com/timebank/notification_service/infrastructure/slack/SlackBotClient.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/infrastructure/slack/SlackFeignClientImpl.java(1 hunks)notification-service/src/main/java/com/timebank/notification_service/infrastructure/slack/SlackWebHookClient.java(1 hunks)
💤 Files with no reviewable changes (3)
- notification-service/src/main/java/com/timebank/notification_service/application/event/NotificationEvent.java
- notification-service/src/main/java/com/timebank/notification_service/domain/entity/NotificationType.java
- notification-service/src/main/java/com/timebank/notification_service/domain/entity/NotificationEventType.java
🔇 Additional comments (9)
notification-service/build.gradle (1)
41-41: Feign 클라이언트 의존성 추가
Spring Cloud OpenFeign 의존성이 BOM을 통해 잘 관리되고 있습니다. Slack 통신을 위한 클라이언트 구현에 필요한 의존성 추가가 적절합니다.notification-service/src/main/java/com/timebank/notification_service/application/dto/NotificationDto.java (1)
3-3: 외부 공통 DTO 사용 일관성 확인
com.timebank.common.infrastructure.external.notification.dto.NotificationType로 변경되어 모듈 간 일관성이 높아졌습니다. 관련 엔티티(Notification)의 타입 필드도 동일한 enum을 참조하는지 확인했습니다.common/src/main/java/com/timebank/common/infrastructure/external/notification/dto/NotificationEvent.java (1)
26-26: Slack 사용자 이메일 필드 추가
slackUserEmail필드가 새롭게 추가되어 Slack 알림 전송 시 사용자 식별에 필요한 이메일 데이터를 보관합니다. Builder 패턴을 통해 설정 가능하며, 소비자에서 해당 값이 정상으로 세팅되는지 확인이 필요합니다.notification-service/src/main/java/com/timebank/notification_service/infrastructure/slack/SlackWebHookClient.java (1)
9-14: Feign 클라이언트 구현이 잘 되었습니다.Slack webhook을 사용해 메시지를 보내는 Feign 클라이언트가 잘 정의되어 있습니다. 구성 속성을 사용하여 URL과 경로를 설정한 것은 좋은 접근 방식입니다.
notification-service/src/main/java/com/timebank/notification_service/application/service/NotificationService.java (2)
14-19: 필요한 Slack 관련 클래스 가져오기가 잘 되었습니다.Slack 알림 기능을 위한 필수 클래스들의 import가 적절히 추가되었습니다.
34-34: 의존성 주입 확인SlackClient 의존성이 올바르게 주입되었습니다. 이는 서비스 계층에서 Slack 메시지 전송 기능을 사용하기 위한 필수 구성 요소입니다.
notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackWebHookMessage.java (1)
7-18: DTO 구현이 적절합니다.Slack webhook 메시지를 위한 DTO 클래스가 잘 구현되어 있으며, 롬복 어노테이션을 활용하여 보일러플레이트 코드를 줄였습니다. 팩토리 메소드
from을 제공하여 객체 생성을 용이하게 한 것도 좋은 접근입니다.notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackBotMessage.java (1)
7-20: Slack 봇 메시지 DTO 구현이 잘 되었습니다.SlackBotMessage DTO 클래스가 잘 설계되어 있으며, 필요한 필드와 생성 메소드가 적절히 구현되어 있습니다. 롬복 어노테이션을 활용하여 코드를 간결하게 유지한 것도 좋은 방법입니다.
notification-service/src/main/java/com/timebank/notification_service/domain/entity/Notification.java (1)
4-5: 공통 패키지 의존성 활용이 잘 되었습니다기존 내부 열거형 대신 공통 패키지의 NotificationType과 NotificationEventType을 활용하는 방식으로 잘 리팩토링하였습니다.
| implementation 'io.github.resilience4j:resilience4j-spring-boot3' | ||
| implementation 'io.github.resilience4j:resilience4j-retry' |
There was a problem hiding this comment.
Resilience4j 모듈 버전 명시 필요
resilience4j-spring-boot3 및 resilience4j-retry 의존성에 버전이 지정되어 있지 않습니다. 이는 빌드 시 불확실한 버전이 적용될 수 있어 안정성을 해칩니다. 명시적 버전 또는 BOM 관리를 통해 원하는 버전을 지정해주세요.
| package com.timebank.notification_service.application.dto; | ||
|
|
||
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class SlackUserResponse { | ||
| private boolean ok; | ||
| private SlackUser user; | ||
|
|
||
| @Getter | ||
| public static class SlackUser { | ||
| private String id; | ||
| private String teamId; | ||
| private String name; | ||
| private String realName; | ||
| private SlackProfile profile; | ||
|
|
||
| @Getter | ||
| public static class SlackProfile { | ||
| private String email; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Slack API 응답 매핑 검증
Lombok @Getter만 사용하여 DTO를 정의했으나, Slack API의 필드명이 snake_case(team_id, real_name)를 사용합니다. Jackson 설정에서 PropertyNamingStrategy.SNAKE_CASE를 활성화하지 않았다면, @JsonProperty("team_id"), @JsonProperty("real_name") 등을 추가해 필드 매핑을 보장해야 합니다.
| package com.timebank.notification_service.application.client; | ||
|
|
||
| import com.timebank.notification_service.application.dto.SlackBotMessage; | ||
| import com.timebank.notification_service.application.dto.SlackWebHookMessage; | ||
|
|
||
| public interface SlackClient { | ||
| void sendMessage(SlackWebHookMessage message); | ||
|
|
||
| void sendMessage(SlackBotMessage request); | ||
|
|
||
| String getUserIdByEmail(String email); | ||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
SlackClient 인터페이스 확인
Slack WebHook 전송과 Bot 메시지 전송을 위한 오버로드된 sendMessage 메서드가 정의되어 있습니다. 타입 별로 메서드명을 구분(sendWebHookMessage, sendBotMessage)하면 가독성이 더 높아질 수 있습니다.
| @PostMapping("${slack.webhook.path}") | ||
| void sendMessage(@RequestBody SlackWebHookMessage message); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
에러 처리 메커니즘 추가 고려
현재 구현에는 Slack API 호출 실패 시의 에러 처리가 포함되어 있지 않습니다. Resilience4j를 사용한 재시도 메커니즘이나 폴백 전략을 고려해 보세요.
@PostMapping("${slack.webhook.path}")
+@CircuitBreaker(name = "slackWebhook")
+@Retry(name = "slackWebhook")
void sendMessage(@RequestBody SlackWebHookMessage message);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @PostMapping("${slack.webhook.path}") | |
| void sendMessage(@RequestBody SlackWebHookMessage message); | |
| @PostMapping("${slack.webhook.path}") | |
| @CircuitBreaker(name = "slackWebhook") | |
| @Retry(name = "slackWebhook") | |
| void sendMessage(@RequestBody SlackWebHookMessage message); |
| // // Kafka 이벤트 발행 (DELETED) | ||
| // NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED); | ||
| // kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
주석 처리된 코드 제거 필요
Kafka 이벤트 발행 관련 주석 처리된 코드는 제거하는 것이 좋습니다. 주석 처리된 코드는 유지보수를 어렵게 만들고 코드 이해도를 떨어뜨립니다.
- // // Kafka 이벤트 발행 (DELETED)
- // NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED);
- // kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // // Kafka 이벤트 발행 (DELETED) | |
| // NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED); | |
| // kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event); | |
| } | |
| } |
|
|
||
| @Override | ||
| @Retry(name = "slackMessageRetry", fallbackMethod = "lookupByEmailFallback") | ||
| public String getUserIdByEmail(String email) { | ||
| SlackUserResponse response = slackBotClient.lookupByEmail(bearerPrefix + slackBotToken, email); | ||
| return response.getUser().getId(); | ||
| } |
There was a problem hiding this comment.
Null 처리 로직 개선 필요
getUserIdByEmail 메서드에서 response의 null 확인이 없어 NPE가 발생할 수 있습니다.
response 또는 response.getUser()가 null일 경우 안전하게 처리하는 로직을 추가하세요:
@Override
@Retry(name = "slackMessageRetry", fallbackMethod = "lookupByEmailFallback")
public String getUserIdByEmail(String email) {
SlackUserResponse response = slackBotClient.lookupByEmail(bearerPrefix + slackBotToken, email);
- return response.getUser().getId();
+ if (response == null || response.getUser() == null) {
+ log.warn("사용자 정보를 찾을 수 없습니다: {}", email);
+ return null;
+ }
+ return response.getUser().getId();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| @Retry(name = "slackMessageRetry", fallbackMethod = "lookupByEmailFallback") | |
| public String getUserIdByEmail(String email) { | |
| SlackUserResponse response = slackBotClient.lookupByEmail(bearerPrefix + slackBotToken, email); | |
| return response.getUser().getId(); | |
| } | |
| @Override | |
| @Retry(name = "slackMessageRetry", fallbackMethod = "lookupByEmailFallback") | |
| public String getUserIdByEmail(String email) { | |
| SlackUserResponse response = slackBotClient.lookupByEmail(bearerPrefix + slackBotToken, email); | |
| if (response == null || response.getUser() == null) { | |
| log.warn("사용자 정보를 찾을 수 없습니다: {}", email); | |
| return null; | |
| } | |
| return response.getUser().getId(); | |
| } |
| @Value("${slack.bot.token}") | ||
| private String slackBotToken; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
토큰 주입 방식 개선 필요
Slack 토큰이 @value 어노테이션을 통해 직접 주입되고 있습니다. 이는 테스트와 보안 측면에서 문제가 될 수 있습니다.
별도의 configuration 클래스를 통해 관리하거나 Vault 같은 보안 시스템 활용을 고려하세요:
-@Value("${slack.bot.token}")
-private String slackBotToken;
+private final String slackBotToken;
+public SlackFeignClientImpl(SlackWebHookClient slackWebHookClient,
+ SlackBotClient slackBotClient,
+ @Value("${slack.bot.token}") String slackBotToken) {
+ this.slackWebHookClient = slackWebHookClient;
+ this.slackBotClient = slackBotClient;
+ this.slackBotToken = slackBotToken;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Value("${slack.bot.token}") | |
| private String slackBotToken; | |
| private final String slackBotToken; | |
| public SlackFeignClientImpl(SlackWebHookClient slackWebHookClient, | |
| SlackBotClient slackBotClient, | |
| @Value("${slack.bot.token}") String slackBotToken) { | |
| this.slackWebHookClient = slackWebHookClient; | |
| this.slackBotClient = slackBotClient; | |
| this.slackBotToken = slackBotToken; | |
| } |
|
|
||
| notificationService.sendMessage( | ||
| SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 일관성 개선 필요
consumeCreated 메서드에서는 저장 후 메시지를 보내는 반면, consumeUpdated에서는 메시지를 보낸 후 저장합니다.
일관된 동작을 위해 패턴을 통일하세요:
public void consumeCreated(NotificationEvent event) {
log.info("Consumed CREATED event: {}", event);
saveNotification(event);
notificationService.sendMessage(
SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
}혹은 모든 메서드가 같은 순서를 따르도록 하세요. 트랜잭션 경계와 실패 시나리오를 고려하여 순서를 결정하는 것이 중요합니다.
Committable suggestion skipped: line range outside the PR's diff.
| notificationService.sendMessage( | ||
| SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail()); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
슬랙 메시지 생성 로직 검토 필요
SlackWebHookMessage.from 메서드가 event.getMessage()만 사용하고 있어 정보가 부족할 수 있습니다.
이벤트의 다른 속성(타입, 이벤트 유형 등)을 활용하여 더 풍부한 슬랙 메시지를 생성하는 것이 좋습니다:
-notificationService.sendMessage(
- SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
+notificationService.sendMessage(
+ SlackWebHookMessage.builder()
+ .text(String.format("[%s] %s", event.getType(), event.getMessage()))
+ .eventType(event.getEventType().name())
+ .build(),
+ event.getSlackUserEmail());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| notificationService.sendMessage( | |
| SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail()); | |
| notificationService.sendMessage( | |
| SlackWebHookMessage.builder() | |
| .text(String.format("[%s] %s", event.getType(), event.getMessage())) | |
| .eventType(event.getEventType().name()) | |
| .build(), | |
| event.getSlackUserEmail()); |
| notificationService.sendMessage( | ||
| SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail()); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
DELETE 이벤트 처리 개선 필요
DELETED 이벤트도 데이터베이스에 저장된 후 메시지를 보내고 있습니다. 삭제된 항목이 저장되는 로직이 있습니다.
DELETED 이벤트의 경우 로직 검토가 필요합니다. 정말 저장이 필요한지, 아니면 해당 알림을 DB에서 실제로 삭제해야 하는지 확인하세요.
🏁 Script executed:
#!/bin/bash
# saveNotification 메서드가 어떻게 호출되는지 확인
rg -n "saveNotification" --type java notification-service/Length of output: 673
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' notification-service/src/main/java/com/timebank/notification_service/infrastructure/kafka/NotificationConsumer.javaLength of output: 2510
DELETED 이벤트의 DB 저장 로직 검토 필요
현재 NotificationConsumer.consumeDeleted()에서 삭제 이벤트 수신 시에도 saveNotification(event)를 호출해 알림을 저장하고 있습니다. 삭제된 알림을 별도로 기록(audit)할 의도가 아니라면, 다음을 검토해주세요:
- NotificationConsumer.java
consumeDeleted()(약 49–57행)에서saveNotification(event)호출 여부
- 삭제 이벤트의 처리 방식
- DB에 그대로 저장이 필요한지
- 기존 알림을 삭제(
notificationRepository.deleteById(...))하거나, 저장 호출을 제거할지
의도에 맞춰 로직을 수정해주세요.
2 participants
💡 이슈
🤩 개요
slack 알림 추가 및 재시도 추가
🧑💻 작업 사항
-webhook알림 추가
-slackbot을 사용한 사용자 DM추가
-재처리 로직추가
📖 참고 사항
UserEmail을 통해 사용자 SlackId를 가져오는 과정에서 UserId를 헤더에 포함시켜서 알림이 있는 메소드 마다 UserEmail을 넣어줘야 할 것같아요 일단 하드코딩으로 테스트 해봤을때 메시지 수신은 확인했습니다. gateway에서 feign으로 email을 받아서 넣어줄지, 아니면 token에 email을 넣어서 헤더에 넣어줄지는 잘모르겠어서 나뒀습니다.
Summary by CodeRabbit
신규 기능
버그 수정
리팩터
라이브러리/의존성