feat : 알림 이벤트 메시지큐 및 캐싱 적용 #122
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 캐싱(Redis)에 대해서는 어떻게 처리할 것인지 생각해봐야 함
- 알림 객체 생성 및 발행 중 에러가 발생하더라도 핵심 비즈니스 로직에는 영향이 가지 않도록 트랜잭션을 분리함
- @CacheEvict는 패턴에 맞는 캐시들 또는 여러 개의 캐시를 한 번에 무효화할 수 있는 방법이 없음 - 때문에 redisTemplate 사용해서 직접 캐시 조회 및 삭제함 - Page 객체를 그대로 캐시에 저장하려고 하면 에러 발생 - Jackson 라이브러리가 JSON 문자열을 다시 Page 객체(정확히는 PageImpl 클래스)로 만드는 방법(역직렬화)을 모르기 때문 - 보통 Mixin이나 wrapper class 등을 많이 사용한다고 하는데, 설정 이슈인지 계속해서 에러가 발생함 - 그래서 커스텀 페이지 응답 DTO를 만들어서 사용하기로 함
|
""" Walkthrough이 변경사항은 알림(Notification) 기능의 아키텍처를 이벤트 기반에서 JMS 및 MongoDB 기반으로 대대적으로 리팩토링합니다. 기존 Redis 및 이벤트 리스너/퍼블리셔 구조를 제거하고, JMS 큐, MongoDB, Redis 캐시, 그리고 새로운 서비스 및 DTO 구조를 도입합니다. 관련 서비스 및 예외 코드, 도메인 모델, 컨피그, 레포지토리, 퍼블리셔, 리스너 등이 전면적으로 교체 및 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Service as DiscussionVoteService/ReplyService/ReplyVoteService
participant Executor as NotificationExecutor
participant Publisher as NotificationEventPublisher (JMS)
participant Listener as NotificationEventListener (JMS)
participant Mongo as NotificationMongoRepository
participant Redis as RedisCache
participant WebSocket as SimpMessagingTemplate
Service->>Executor: execute(() -> List<NotificationCreateEvent>)
Executor->>Publisher: notify(NotificationCreateEvent)
Publisher->>Listener: JMS 메시지 발행
Listener->>Mongo: NotificationDocument 저장
Listener->>Redis: 캐시 삭제/갱신
Listener->>WebSocket: 사용자에게 알림 메시지 전송
Possibly related PRs
Suggested reviewers
Poem
✨ 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: 11
🧹 Nitpick comments (8)
src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationQueueConstants.java (1)
3-11: JMS 큐 상수 정의가 잘 구현되었습니다.유틸리티 클래스 패턴을 올바르게 적용했고, 큐 이름도 일관된 네이밍 컨벤션을 따릅니다. 그러나 하드코딩된 큐 이름 대신 설정 파일에서 관리하는 것을 고려해보세요.
큐 이름을 설정 파일로 외부화하는 것을 고려해보세요:
+@Value("${notification.queue.create:notification.queue.create}") +public static final String NOTIFICATION_QUEUE_CREATE;또는
application.yml에서 관리:notification: queue: create: notification.queue.create list: notification.queue.list mark-read: notification.queue.readsrc/main/java/org/ezcode/codetest/infrastructure/notification/repository/NotificationMongoRepository.java (1)
10-16: MongoDB 인덱스 설정 및 보안 고려사항을 검토하세요.저장소 인터페이스는 올바르게 구현되었지만 다음 사항들을 고려해보세요:
principalName필드에 대한 인덱스 설정이 필요할 수 있습니다 (쿼리 성능 향상을 위해)principalName파라미터에 대한 입력 검증이나 제약사항을 고려해보세요src/main/java/org/ezcode/codetest/infrastructure/notification/dto/NotificationPageResponse.java (1)
13-23: Serializable 구현 시 serialVersionUID 추가를 권장합니다.클래스가
Serializable을 구현하고 있지만serialVersionUID가 정의되지 않았습니다. 직렬화 호환성을 보장하기 위해 추가하는 것을 권장합니다.@Getter @AllArgsConstructor @NoArgsConstructor public class NotificationPageResponse<T> implements Serializable { + + private static final long serialVersionUID = 1L; private List<T> content;src/main/java/org/ezcode/codetest/application/community/service/ReplyService.java (1)
49-59: 알림 실행 로직 구현이 올바르게 적용되었습니다NotificationExecutor를 사용한 비동기 알림 처리 로직이 잘 구현되어 있습니다. 빈 목록 체크와 스트림 변환 로직도 적절합니다.
다만 다음 사항을 고려해보세요:
- NotificationExecutor.execute() 내부에서 예외가 발생하면 로깅만 하고 조용히 실패하는 구조입니다. 알림 실패가 비즈니스 로직에 영향을 주지 않는 것이 의도라면 적절하지만, 모니터링을 위한 메트릭 수집을 고려해볼 수 있습니다.
src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java (1)
28-33: null 안전성과 성능 고려사항null 체크와 빈 리스트 처리가 적절하게 구현되어 있습니다. 다만 대량의 알림 이벤트 처리 시 성능을 고려해볼 필요가 있습니다.
대량 처리 시 배치 처리나 비동기 처리를 고려해볼 수 있습니다:
if (events != null && !events.isEmpty()) { + // 대량 이벤트의 경우 배치 처리 고려 for (NotificationCreateEvent event : events) { notificationEventService.notify(event); }src/main/java/org/ezcode/codetest/infrastructure/notification/publisher/NotificationEventPublisher.java (2)
30-30: 주석 처리된 코드 정리 필요이전 이벤트 퍼블리셔 코드가 주석으로 남아있습니다. 코드 정리를 위해 제거하는 것이 좋겠습니다.
- // publisher.publishEvent(dto);Also applies to: 37-37, 44-44
47-58: JMS 메시지 전송 로직이 적절하게 구현되었습니다JSON 직렬화와 JMS 전송 로직이 올바르게 구현되어 있고, 예외 처리도 적절합니다. ObjectMapper를 통한 직렬화와 사용자 정의 예외 처리가 잘 되어 있습니다.
다만 다음 사항을 고려해보세요:
- 메시지 크기가 큰 경우를 위한 제한 검증
- JMS 전송 실패 시 재시도 메커니즘
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java (1)
66-76: 중복 데이터베이스 접근 최적화현재 알림을 조회한 후 다시 저장하는 방식은 비효율적입니다. MongoDB의 원자적 업데이트 연산을 활용하는 것이 좋습니다.
다음과 같이 원자적 업데이트로 개선할 수 있습니다:
public void markAsRead(NotificationMarkReadEvent event) { - NotificationDocument notificationDocument = mongoRepository - .findByIdAndPrincipalName(event.notificationId(), event.principalName()) - .orElseThrow(() -> new NotificationException(NotificationExceptionCode.NOTIFICATION_NOT_FOUND)); - - notificationDocument.markAsRead(); - mongoRepository.save(notificationDocument); + // MongoRepository에 다음 메서드 추가 필요: + // Optional<NotificationDocument> markAsReadByIdAndPrincipalName(String id, String principalName); + + Optional<NotificationDocument> updated = mongoRepository + .markAsReadByIdAndPrincipalName(event.notificationId(), event.principalName()); + + if (updated.isEmpty()) { + throw new NotificationException(NotificationExceptionCode.NOTIFICATION_NOT_FOUND); + } evictNotificationListCache(event.principalName()); }또는 MongoTemplate을 사용한 원자적 업데이트:
@Autowired private MongoTemplate mongoTemplate; public void markAsRead(NotificationMarkReadEvent event) { Query query = Query.query( Criteria.where("id").is(event.notificationId()) .and("principalName").is(event.principalName()) ); Update update = Update.update("isRead", true); UpdateResult result = mongoTemplate.updateFirst(query, update, NotificationDocument.class); if (result.getMatchedCount() == 0) { throw new NotificationException(NotificationExceptionCode.NOTIFICATION_NOT_FOUND); } evictNotificationListCache(event.principalName()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.gitignore(1 hunks)src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java(3 hunks)src/main/java/org/ezcode/codetest/application/community/service/ReplyService.java(3 hunks)src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java(3 hunks)src/main/java/org/ezcode/codetest/application/notification/event/NotificationMarkReadEvent.java(1 hunks)src/main/java/org/ezcode/codetest/application/notification/exception/NotificationExceptionCode.java(1 hunks)src/main/java/org/ezcode/codetest/application/notification/port/NotificationEventService.java(1 hunks)src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java(1 hunks)src/main/java/org/ezcode/codetest/application/notification/service/NotificationUseCase.java(2 hunks)src/main/java/org/ezcode/codetest/infrastructure/cache/config/CaffeineCacheConfig.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/cache/config/RedisCacheConfig.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/dto/NotificationRecord.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/dto/NotificationResponse.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/listener/NotificationEventListener.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/NotificationEventPublisher.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/event/publisher/StompMessageService.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/dto/NotificationPageResponse.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/dto/NotificationResponse.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationDocument.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationQueueConstants.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/publisher/NotificationEventPublisher.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/repository/NotificationMongoRepository.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/notification/NotificationRedisRepository.java(0 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/notification/NotificationRepository.java(0 hunks)
💤 Files with no reviewable changes (8)
- src/main/java/org/ezcode/codetest/infrastructure/cache/config/RedisCacheConfig.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/NotificationEventPublisher.java
- src/main/java/org/ezcode/codetest/infrastructure/event/publisher/StompMessageService.java
- src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/notification/NotificationRepository.java
- src/main/java/org/ezcode/codetest/infrastructure/event/dto/NotificationResponse.java
- src/main/java/org/ezcode/codetest/infrastructure/event/dto/NotificationRecord.java
- src/main/java/org/ezcode/codetest/infrastructure/event/listener/NotificationEventListener.java
- src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/notification/NotificationRedisRepository.java
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: chat26666
PR: ezcode-my/backend#37
File: src/main/java/org/ezcode/codetest/infrastructure/elasticsearch/repository/ProblemElasticsearchRepositoryDsl.java:10-10
Timestamp: 2025-06-06T19:56:27.759Z
Learning: When user chat26666 mentions that code was implemented in a previous PR but explanation was missing, they may submit a follow-up PR just to add documentation/explanation without the actual implementation code.
src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java (1)
Learnt from: NCookies
PR: ezcode-my/backend#65
File: src/main/java/org/ezcode/codetest/application/community/service/BaseVoteService.java:28-39
Timestamp: 2025-06-16T06:11:45.234Z
Learning: BaseVoteService 클래스에서는 기본 클래스의 toggleVote 메서드에 @Transactional을 추가하지 않고, 대신 서브클래스에서 이 메서드를 호출할 때 트랜잭션을 사용하는 방식을 선호한다.
src/main/java/org/ezcode/codetest/application/notification/exception/NotificationExceptionCode.java (1)
Learnt from: thezz9
PR: ezcode-my/backend#98
File: src/main/java/org/ezcode/codetest/application/submission/dto/event/SubmissionErrorEvent.java:17-22
Timestamp: 2025-06-25T01:24:30.754Z
Learning: JudgeClient에서 모든 예외를 SubmissionException으로 래핑해서 넘길 계획이므로, SubmissionErrorEvent.resolveCode() 메서드의 캐스팅은 안전할 예정임.
src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java (1)
Learnt from: NCookies
PR: ezcode-my/backend#65
File: src/main/java/org/ezcode/codetest/application/community/service/BaseVoteService.java:28-39
Timestamp: 2025-06-16T06:11:45.234Z
Learning: BaseVoteService 클래스에서는 기본 클래스의 toggleVote 메서드에 @Transactional을 추가하지 않고, 대신 서브클래스에서 이 메서드를 호출할 때 트랜잭션을 사용하는 방식을 선호한다.
🧬 Code Graph Analysis (3)
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java (4)
src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationQueueConstants.java (1)
NotificationQueueConstants(3-11)src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java (1)
Service(23-88)src/main/java/org/ezcode/codetest/application/notification/service/NotificationUseCase.java (1)
Service(11-30)src/main/java/org/ezcode/codetest/infrastructure/notification/publisher/NotificationEventPublisher.java (1)
Slf4j(18-59)
src/main/java/org/ezcode/codetest/infrastructure/notification/publisher/NotificationEventPublisher.java (2)
src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationQueueConstants.java (1)
NotificationQueueConstants(3-11)src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java (1)
Slf4j(23-80)
src/main/java/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java (1)
src/main/java/org/ezcode/codetest/infrastructure/cache/config/RedisCacheConfig.java (1)
Configuration(14-40)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (16)
.gitignore (1)
44-45: 로컬 데이터·DB 디렉터리 무시 추가는 적절하지만 서비스별 경로 중복 여부를 확인해 주세요
/data/,/db/디렉터리를 전체 레포지터리 루트에서 일괄 무시하도록 추가한 것은 로컬 MongoDB·ActiveMQ 등 인프라용 볼륨이 Git 이력에 섞이는 일을 방지한다는 점에서 타당합니다.
다만 다른 모듈(예: 테스트 픽스처, 마이그레이션 스크립트)이 동일한 경로명을 사용할 가능성이 있는지 한 번 더 점검해 주세요. 충돌 가능성이 있다면 보다 구체적인 서브경로(예:/data/mongodb/)로 제한하거나!.gitkeep전략을 고려할 수 있습니다.
별다른 충돌 요소가 없다면 그대로 유지하셔도 무방합니다.src/main/java/org/ezcode/codetest/application/notification/event/NotificationMarkReadEvent.java (1)
3-9: 이벤트 클래스명 변경이 적절합니다.
NotificationReadEvent에서NotificationMarkReadEvent로 변경하여 더 명확한 의미를 전달합니다. "MarkRead"가 "Read"보다 구체적인 액션을 나타내므로 코드 가독성이 향상되었습니다.src/main/java/org/ezcode/codetest/application/notification/service/NotificationUseCase.java (2)
4-4: Import 변경이 이벤트 클래스 리네이밍과 일치합니다.
NotificationMarkReadEventimport로 정확히 변경되어 이벤트 클래스 리네이밍과 일관성을 유지합니다.
27-27: 이벤트 인스턴스 생성이 올바르게 업데이트되었습니다.새로운
NotificationMarkReadEvent클래스를 사용하여 이벤트 객체를 생성하고 있으며, 파라미터 전달도 정확합니다.src/main/java/org/ezcode/codetest/infrastructure/cache/config/CaffeineCacheConfig.java (1)
12-12: @primary 어노테이션 추가가 적절합니다.여러 CacheManager 빈이 존재할 때 autowiring 충돌을 방지하기 위해 @primary 어노테이션을 추가한 것은 올바른 접근입니다. 알림 전용 Redis 캐시 매니저와 함께 Caffeine을 기본 캐시 매니저로 지정하는 설계가 합리적입니다.
Also applies to: 20-20
src/main/java/org/ezcode/codetest/application/notification/exception/NotificationExceptionCode.java (1)
14-15: 새로운 예외 코드가 JMS 아키텍처를 잘 지원합니다.
NOTIFICATION_CONVERT_MESSAGE_ERROR와NOTIFICATION_NOT_FOUND예외 코드가 적절히 정의되었습니다. 메시지 변환 오류와 데이터 누락 시나리오에 대한 명확한 에러 처리가 가능합니다. HTTP 상태 코드와 한국어 메시지도 적절합니다.src/main/java/org/ezcode/codetest/infrastructure/notification/dto/NotificationResponse.java (1)
9-33: 깔끔한 DTO 설계입니다.Java 레코드를 사용한 불변 DTO와 정적 팩토리 메서드를 통한 변환 로직이 잘 구현되었습니다. 불변성을 보장하고 간결한 코드로 작성되었습니다.
src/main/java/org/ezcode/codetest/application/notification/port/NotificationEventService.java (1)
9-13: 인터페이스 메서드명과 파라미터 타입 개선이 좋습니다.
saveAndNotify→notify로의 메서드명 변경과NotificationReadEvent→NotificationMarkReadEvent로의 타입 변경이 더 명확하고 의도를 잘 드러냅니다.src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java (1)
24-36: NotificationExecutor 도입으로 아키텍처가 개선되었습니다.직접적인
NotificationEventService사용에서NotificationExecutor로 변경하여 알림 로직의 트랜잭션 경계를 분리한 것은 좋은 아키텍처 개선입니다.src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationDocument.java (2)
38-48: 팩토리 메서드 구현이 적절합니다NotificationCreateEvent를 통한 객체 생성 로직이 명확하고 올바르게 구현되어 있습니다. ID를 null로 설정하여 MongoDB가 자동 생성하도록 하는 것도 적절합니다.
50-52: 불변성 위반 가능성
markAsRead()메서드가 필드를 직접 변경하고 있습니다. 불변 객체 패턴을 유지하거나, 변경 후 저장 로직이 필요할 수 있습니다.다음과 같이 새 인스턴스를 반환하거나 저장 로직을 추가하는 것을 고려해보세요:
public void markAsRead() { this.isRead = true; + // MongoDB 저장 로직 호출 또는 새 인스턴스 반환 패턴 적용 }Likely an incorrect or invalid review comment.
src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java (1)
22-22: 트랜잭션 전파 설정이 적절합니다
REQUIRES_NEW전파 설정을 통해 알림 처리를 별도 트랜잭션에서 실행하는 것은 주 비즈니스 로직과 분리하는 좋은 설계입니다. 알림 실패가 주 트랜잭션에 영향을 주지 않습니다.src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java (2)
48-55: 알림 실행 로직이 일관되게 적용되었습니다ReplyService와 동일한 패턴으로 NotificationExecutor가 적용되어 일관성이 유지되고 있습니다. Optional 처리와 빈 리스트 반환 로직도 적절합니다.
학습된 내용에 따르면 BaseVoteService에서는 서브클래스에서 트랜잭션을 관리하는 방식을 선호하므로, 현재 구조가 적합합니다.
26-35: 생성자 의존성 주입 변경이 적절합니다NotificationEventService에서 NotificationExecutor로의 의존성 변경이 올바르게 적용되었습니다. 생성자 기반 의존성 주입 패턴을 유지하면서 새로운 아키텍처에 맞게 적절히 수정되었습니다.
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java (1)
34-45: WebSocket 구독 경로 일관성 확인 필요현재 NotificationEventListener에서 알림 생성·조회 이벤트에 서로 다른 경로를 사용하고 있습니다.
의도된 차이가 없다면 단일 경로로 통일하거나, 의도된 분리가 맞다면 클라이언트 구독 코드에도 동일하게 반영되었는지 검토해주세요.
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java
• handleNotificationCreateEvent → "/queue/notification"
• handleNotificationListEvent → "/queue/notifications"
• handleNotificationMarkReadEvent 경로 확인 필요Front-end(클라이언트) 구독 코드에서도 위 경로 중 하나로 일관되게 subscribe 하고 있는지 확인 부탁드립니다.
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java (1)
38-45: 트랜잭션 처리 누락알림 생성과 캐시 무효화가 별도의 트랜잭션으로 처리되어 데이터 일관성 문제가 발생할 수 있습니다. MongoDB 저장 실패 시에도 캐시가 무효화될 수 있습니다.
다음과 같이 트랜잭션을 적용하거나 순서를 조정하세요:
+@Transactional public NotificationDocument createNewNotification(NotificationCreateEvent event) { NotificationDocument document = mongoRepository.save(NotificationDocument.from(event)); - evictNotificationListCache(event.principalName()); + // MongoDB 저장 성공 후에만 캐시 무효화 + try { + evictNotificationListCache(event.principalName()); + } catch (Exception e) { + log.warn("캐시 무효화 실패 (알림 저장은 성공): {}", e.getMessage()); + // 캐시 무효화 실패는 저장 작업에 영향을 주지 않도록 처리 + } return document; }Likely an incorrect or invalid review comment.
src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java
Show resolved
Hide resolved
src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationDocument.java
Outdated
Show resolved
Hide resolved
src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java
Show resolved
Hide resolved
...va/org/ezcode/codetest/infrastructure/notification/publisher/NotificationEventPublisher.java
Show resolved
Hide resolved
.../java/org/ezcode/codetest/infrastructure/notification/service/NotificationEventListener.java
Show resolved
Hide resolved
...ava/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java
Show resolved
Hide resolved
...ava/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java
Show resolved
Hide resolved
...ava/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java
Show resolved
Hide resolved
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java
Show resolved
Hide resolved
src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java
Show resolved
Hide resolved
thezz9
approved these changes
Jun 30, 2025
Kimminu7
approved these changes
Jul 1, 2025
- 알림 생성 트랜잭션 execute 수행 시 예외처리 추가 - PolymorphicTypeValidator allowIfSubType 구체화 - redis에서 성능 저하가 우려되는 keys 대신 scan 사용
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java(3 hunks)src/main/java/org/ezcode/codetest/application/community/service/ReplyService.java(4 hunks)src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java(3 hunks)src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java(1 hunks)src/main/java/org/ezcode/codetest/domain/community/exception/CommunityExceptionCode.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationDocument.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/ezcode/codetest/domain/community/exception/CommunityExceptionCode.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/java/org/ezcode/codetest/application/community/service/ReplyVoteService.java
- src/main/java/org/ezcode/codetest/application/community/service/ReplyService.java
- src/main/java/org/ezcode/codetest/infrastructure/notification/model/NotificationDocument.java
- src/main/java/org/ezcode/codetest/application/notification/service/NotificationExecutor.java
- src/main/java/org/ezcode/codetest/infrastructure/notification/config/NotificationRedisCacheConfig.java
- src/main/java/org/ezcode/codetest/infrastructure/notification/service/NotificationService.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: chat26666
PR: ezcode-my/backend#37
File: src/main/java/org/ezcode/codetest/infrastructure/elasticsearch/repository/ProblemElasticsearchRepositoryDsl.java:10-10
Timestamp: 2025-06-06T19:56:27.759Z
Learning: When user chat26666 mentions that code was implemented in a previous PR but explanation was missing, they may submit a follow-up PR just to add documentation/explanation without the actual implementation code.
src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java (1)
Learnt from: NCookies
PR: ezcode-my/backend#65
File: src/main/java/org/ezcode/codetest/application/community/service/BaseVoteService.java:28-39
Timestamp: 2025-06-16T06:11:45.234Z
Learning: BaseVoteService 클래스에서는 기본 클래스의 toggleVote 메서드에 @Transactional을 추가하지 않고, 대신 서브클래스에서 이 메서드를 호출할 때 트랜잭션을 사용하는 방식을 선호한다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java (4)
3-4: 임포트 추가가 적절합니다.새로운 람다 기반 알림 처리 로직에 필요한
Collections와List임포트가 적절히 추가되었습니다.
10-10: 알림 시스템 아키텍처 변경이 올바르게 반영되었습니다.
NotificationEventService에서NotificationExecutor로의 변경이 PR 목표인 메시지큐 기반 비동기 알림 처리 아키텍처와 일치합니다.
20-22: 로깅 기능 추가가 적절합니다.
@Slf4j어노테이션 추가로 에러 로깅이 가능해져 디버깅과 모니터링에 도움이 됩니다.
27-27: 의존성 주입 변경이 일관되게 처리되었습니다.필드 선언, 생성자 파라미터, 필드 할당이 모두 일관되게
NotificationExecutor로 변경되었습니다.Also applies to: 33-33, 37-37
Comment on lines
+51
to
+63
| notificationExecutor.execute(() -> { | ||
|
|
||
| try { | ||
| Discussion discussion = discussionDomainService.getDiscussionById(targetId); | ||
|
|
||
| Optional<NotificationCreateEvent> notificationEvent = voteDomainService.createDiscussionVoteNotification(voter, discussion); | ||
| Optional<NotificationCreateEvent> notificationEvent = voteDomainService.createDiscussionVoteNotification(voter, discussion); | ||
|
|
||
| notificationEvent.ifPresent(notificationEventService::saveAndNotify); | ||
| return notificationEvent.map(List::of).orElse(Collections.emptyList()); | ||
| } catch (Exception ex) { | ||
| log.error("토론글 추천 알림 생성 중 에러 발생 : {}", ex.getMessage()); | ||
| return Collections.emptyList(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
알림 처리 로직의 트랜잭션 경계를 확인해주세요.
새로운 NotificationExecutor 기반 람다 패턴이 도입되었는데, 다음 사항들을 검토해주세요:
- 트랜잭션 분리: 람다 내부에서
discussionDomainService.getDiscussionById(targetId)를 호출하는데, 이것이 메인 투표 트랜잭션과 별도의 트랜잭션에서 실행되는지 확인이 필요합니다. - 데이터 일관성: 알림 처리가 별도의 트랜잭션에서 실행될 경우, 투표 완료 후 토론글 데이터 조회 시점에서 일관성 문제가 발생할 수 있습니다.
다음 스크립트로 NotificationExecutor.execute() 메서드의 트랜잭션 처리 방식을 확인해주세요:
🏁 Script executed:
#!/bin/bash
# Description: NotificationExecutor의 트랜잭션 처리 방식 확인
# NotificationExecutor 클래스의 execute 메서드 구현 확인
ast-grep --pattern $'class NotificationExecutor {
$$$
execute($$$) {
$$$
}
$$$
}'
# @Transactional 어노테이션 사용 여부 확인
rg -A 10 "class NotificationExecutor"Length of output: 1545
NotificationExecutor.execute의 트랜잭션 전파가 REQUIRES_NEW임을 확인했습니다.
람다 내부 로직이 메인 투표 트랜잭션과 별도 트랜잭션에서 실행되므로, 아직 커밋되지 않은 투표 반영이 discussionDomainService.getDiscussionById(targetId) 호출 시점에 조회되지 않을 수 있습니다.
• src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java:51-63 –
notificationExecutor.execute 내부의 discussionDomainService.getDiscussionById(targetId)
해결 방안 예시:
- 메인 트랜잭션 내에서 알림용 토론글을 미리 조회해 람다에 전달
- 또는 투표 처리 커밋 후 별도 스케줄러/이벤트 리스너로 알림 실행 구조로 재구성
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/community/service/DiscussionVoteService.java
around lines 51 to 63, the notificationExecutor.execute lambda runs in a
REQUIRES_NEW transaction, causing
discussionDomainService.getDiscussionById(targetId) to potentially read stale
data not yet committed by the main vote transaction. To fix this, fetch the
Discussion entity within the main transaction before calling
notificationExecutor.execute, then pass the fetched Discussion object into the
lambda instead of querying inside it. This ensures the notification logic uses
up-to-date data without transaction isolation issues.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
작업 내용
트러블 슈팅
해결해야 할 문제
참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
신규 기능
버그 수정
리팩터
환경 설정