feat: deliver notification SSE across instances via Redis pub/sub - #60
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRedis Pub/Sub으로 인스턴스 간 알림을 중계한다. ChangesRedis SSE 알림 중계
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant NotificationSseServiceImpl
participant Redis
participant RedisMessageListenerContainer
participant NotificationRedisSubscriber
participant LocalSseEmitter
NotificationSseServiceImpl->>Redis: NotificationSseMessage JSON 발행
Redis->>RedisMessageListenerContainer: notification-sse-channel 메시지 전달
RedisMessageListenerContainer->>NotificationRedisSubscriber: handleMessage 호출
NotificationRedisSubscriber->>LocalSseEmitter: notifications 이벤트 전송
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java (1)
56-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift인스턴스 간 전달을 검증하는 통합 테스트를 추가하세요.
이 구성은 채널, listener container, executor를 함께 연결합니다. 로컬 수동 테스트만으로는 직렬화, 구독, 인스턴스별 emitter 선택의 회귀를 막기 어렵습니다.
Testcontainers Redis 기반 테스트를 추가하세요. 서로 다른
NotificationEmitterRegistry를 가진 두 subscriber를 구독시키고, 대상 사용자가 연결된 registry만notifications이벤트를 받는지 검증하세요. 연결이 없는 인스턴스의 무시 동작과 잘못된 JSON 처리도 포함하세요.장점은 핵심 분산 전달 계약을 자동으로 검증하는 점입니다. 단점은 테스트 실행 시간이 늘어나는 점입니다.
As per path instructions, Java 21 기준에서 실무 활용 가능성과 서비스 설계를 검토했습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java` around lines 56 - 72, 이 구성의 분산 Redis Pub/Sub 계약을 검증하는 Testcontainers Redis 통합 테스트를 추가하세요. 서로 다른 NotificationEmitterRegistry를 사용하는 두 subscriber의 RedisMessageListenerContainer를 구성하고, 한 registry에 연결된 사용자만 notifications 이벤트를 수신하며 다른 인스턴스는 무시하는지 검증하세요. 구독 및 직렬화를 포함하고, 잘못된 JSON 메시지가 안전하게 처리되는 경우도 테스트하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisSubscriber.java`:
- Around line 25-35: Validate the deserialized result immediately after
readValue in handleMessage, checking message, message.userId(),
message.notification(), and message.notification().id() for null or missing
values. When validation fails, log a warning and return before calling
emitterRegistry.findAllByUserId or using the notification ID, while preserving
normal processing for valid messages.
---
Nitpick comments:
In
`@momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java`:
- Around line 56-72: 이 구성의 분산 Redis Pub/Sub 계약을 검증하는 Testcontainers Redis 통합
테스트를 추가하세요. 서로 다른 NotificationEmitterRegistry를 사용하는 두 subscriber의
RedisMessageListenerContainer를 구성하고, 한 registry에 연결된 사용자만 notifications 이벤트를
수신하며 다른 인스턴스는 무시하는지 검증하세요. 구독 및 직렬화를 포함하고, 잘못된 JSON 메시지가 안전하게 처리되는 경우도 테스트하세요.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 783cdee2-fb92-423d-8bf2-8bace8c25af3
📒 Files selected for processing (4)
momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.javamomogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisSubscriber.javamomogo-api/src/main/java/com/momogo/api/notification/redis/NotificationSseMessage.javamomogo-api/src/main/java/com/momogo/api/notification/service/NotificationSseServiceImpl.java
SungHuii
left a comment
SungHuii
left a comment
There was a problem hiding this comment.
수고하셨습니다 ! 👍 코멘트 확인 부탁드립니다~
There was a problem hiding this comment.
🧹 Nitpick comments (1)
momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java (1)
32-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPub/Sub serializer에 JavaTimeModule을 적용하세요.
NotificationResponse.createdAt는OffsetDateTime이고,Jackson2JsonRedisSerializer<>(NotificationSseMessage.class)는 기본적으로JavaTimeModule이 없습니다. 따라서 구독 측 역직렬화가 실패합니다. 발행 측과 동일한 Jackson 설정을 serializer에 직접 전달하는 컨테이너 Bean을 생성해 구독 메시지 계약의 시간 타입을 처리하세요. 컨테이너errorHandler는 실패 결과를 로그/통계로 남기도록 보강하는 것도 적합합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java` around lines 32 - 35, Update notificationListenerAdapter to use a Jackson ObjectMapper configured with JavaTimeModule when constructing the NotificationSseMessage serializer, matching the publisher’s Jackson settings so OffsetDateTime deserializes correctly. Register the configured serializer through the appropriate Redis listener container Bean, and enhance its errorHandler to record failed subscription processing in the existing logging or metrics mechanism.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.java`:
- Around line 32-35: Update notificationListenerAdapter to use a Jackson
ObjectMapper configured with JavaTimeModule when constructing the
NotificationSseMessage serializer, matching the publisher’s Jackson settings so
OffsetDateTime deserializes correctly. Register the configured serializer
through the appropriate Redis listener container Bean, and enhance its
errorHandler to record failed subscription processing in the existing logging or
metrics mechanism.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f5d712dd-2981-4cc2-b4e6-51c70fb8647b
📒 Files selected for processing (5)
momogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisExecutorConfig.javamomogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisPubSubConfig.javamomogo-api/src/main/java/com/momogo/api/notification/redis/NotificationRedisSubscriber.javamomogo-api/src/main/java/com/momogo/api/notification/service/NotificationSseServiceImpl.javamomogo-core/src/main/java/com/momogo/core/common/exception/GlobalErrorCode.java
3 participants
작업 내용
변경 사항
체크리스트
참고 사항
로컬에서 테스트 완료
관련 이슈
Summary by CodeRabbit
새 기능
개선