[SRLT-157] SSE 재연결 알림 복구 추가#97
Merged
Hidden character warning
The head ref may contain hidden characters: "SRLT-157-sse-\uc7ac\uc5f0\uacb0-\ubcf5\uad6c-\ucd94\uac00"
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Note
|
| Layer / File(s) | Summary |
|---|---|
알림 쿼리 및 퍼시스턴스 계층 src/main/java/starlight/adapter/notification/persistence/NotificationRepository.java, src/main/java/starlight/adapter/notification/persistence/NotificationJpa.java, src/main/java/starlight/application/notification/required/NotificationQueryPort.java |
findAllByMemberIdAndIdGreaterThanOrderByIdAsc(memberId, notificationId) JPA 메서드 및 포트 인터페이스를 추가하여 특정 memberId에 대해 주어진 notificationId보다 큰 알림들을 id 오름차순으로 조회하는 쿼리 계약을 정립합니다. |
포트 인터페이스 계약 확장 src/main/java/starlight/application/notification/required/NotificationRealtimePort.java, src/main/java/starlight/application/notification/provided/NotificationUseCase.java |
NotificationRealtimePort.subscribe()는 memberId와 함께 List<NotificationPublishMessage> missedMessages를 수용하고, NotificationUseCase.subscribe()는 lastEventId 파라미터를 추가하여 재연결 시 누락 메시지 컨텍스트를 전달하도록 계약이 변경됩니다. |
애플리케이션 서비스 로직 src/main/java/starlight/application/notification/NotificationService.java |
subscribe(memberId, lastEventId) 메서드가 확장되어 lastEventId가 null이 아니면 NotificationQueryPort로 누락 알림을 조회하고 NotificationPublishMessage로 변환한 뒤 NotificationRealtimePort.subscribe()에 전달합니다. |
REST API 어댑터 src/main/java/starlight/adapter/notification/webapi/NotificationController.java |
/subscribe 엔드포인트가 @RequestHeader("Last-Event-ID") 헤더와 @RequestParam("lastEventId") 쿼리 파라미터를 선택적으로 수신하며, 값이 있는 것을 resolvedLastEventId로 결정하여 NotificationUseCase.subscribe(memberId, resolvedLastEventId)에 전달합니다. |
SSE 실시간 전송 계층 src/main/java/starlight/adapter/notification/sse/NotificationSseRegistry.java |
subscribe() 메서드가 missedMessages 파라미터를 받아 초기 "connected" 이벤트 후 누락 메시지를 전송합니다. createNotificationEvent() 헬퍼로 SSE 이벤트 구성을 통일하고, sendMissedMessages() 헬퍼가 memberId 필터링 및 이벤트 전송을 담당합니다. |
API 문서화 src/main/java/starlight/adapter/notification/webapi/swagger/NotificationApiDoc.java |
Swagger @Parameter 어노테이션을 통해 Last-Event-ID 헤더와 lastEventId 쿼리 파라미터를 OpenAPI 계약에 문서화합니다. |
단위 테스트 src/test/java/starlight/application/notification/NotificationServiceUnitTest.java |
subscribe(memberId, null) 케이스에서 빈 missedMessages 전달을 검증하고, lastEventId 존재 시 정확한 누락 알림 조회와 NotificationPublishMessage 변환을 argThat 매처로 검증하는 테스트를 추가/업데이트합니다. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Controller as NotificationController
participant UseCase as NotificationUseCase
participant Service as NotificationService
participant QueryPort as NotificationQueryPort
participant RealtimePort as NotificationSseRegistry
participant Emitter as SseEmitter
Client->>Controller: GET /subscribe<br/>(Last-Event-ID or lastEventId)
Controller->>Controller: resolvedLastEventId = header or param
Controller->>UseCase: subscribe(memberId, resolvedLastEventId)
UseCase->>Service: subscribe(memberId, lastEventId)
alt lastEventId != null
Service->>QueryPort: findAllByMemberIdAndIdGreaterThanOrderByIdAsc(memberId, lastEventId)
QueryPort-->>Service: List<Notification>
Service->>Service: convert to List<NotificationPublishMessage>
else lastEventId == null
Service->>Service: missedMessages = List.of()
end
Service->>RealtimePort: subscribe(memberId, missedMessages)
RealtimePort->>Emitter: send('connected')
RealtimePort->>RealtimePort: sendMissedMessages(memberId, missedMessages)
loop 각 missedMessage
RealtimePort->>RealtimePort: createNotificationEvent(message)
RealtimePort->>Emitter: send(SSE event)
end
Emitter-->>Client: SSE stream (connected + missed + future)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- StartUpLight/STARLIGHT_BE#92: 이전 PR에서 SSE 알림 구독 기본 아키텍처를 정립했으며, 본 PR이 해당 기능을 확장하여 재연결 시 누락 메시지 복구 기능을 추가합니다.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 SSE 재연결 알림 복구 기능 추가라는 변경의 핵심을 명확하게 나타내며, 모든 파일 변경사항이 이 목표에 부합합니다. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
SRLT-157-sse-재연결-복구-추가
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 help to get the list of available commands and usage tips.
SeongHo5356
changed the title
Test Results268 tests 268 ✅ 11s ⏱️ Results for commit cd786c9. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/starlight/adapter/notification/sse/NotificationSseRegistry.java (1)
100-106: ⚡ Quick win
missedMessagesnull/empty 가드를 추가해 주세요.Line 105에서
missedMessages가 null이면 NPE로 구독이 실패할 수 있습니다. 포트 계약 변경이나 향후 호출 경로 확장에 대비해 방어 처리하는 편이 안전합니다.제안 수정안
private void sendMissedMessages( SseEmitter emitter, Long memberId, List<NotificationPublishMessage> missedMessages ) throws IOException { + if (missedMessages == null || missedMessages.isEmpty()) { + return; + } + for (NotificationPublishMessage missedMessage : missedMessages) { if (!memberId.equals(missedMessage.memberId())) { continue; }🤖 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 `@src/main/java/starlight/adapter/notification/sse/NotificationSseRegistry.java` around lines 100 - 106, sendMissedMessages 메서드에서 missedMessages가 null이거나 비어 있는 경우 NPE를 방지하도록 수비적 코드를 추가하세요: sendMissedMessages(SseEmitter emitter, Long memberId, List<NotificationPublishMessage> missedMessages) 시작부에 missedMessages == null || missedMessages.isEmpty() 검사 후 즉시 반환하도록 하고(또는 필요 시 로깅), 기존 루프(NotificationPublishMessage missedMessage 처리)로 진입하기 전에 빈 컬렉션에 대한 안전한 처리를 보장하세요.
🤖 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
`@src/main/java/starlight/adapter/notification/webapi/NotificationController.java`:
- Around line 45-46: The current logic sets resolvedLastEventId using the query
param first (Long resolvedLastEventId = lastEventId != null ? lastEventId :
lastEventIdHeader), which can ignore a browser-sent Last-Event-ID header; change
the precedence to prefer the header by computing resolvedLastEventId as
lastEventIdHeader != null ? lastEventIdHeader : lastEventId and then call
notificationUseCase.subscribe(authenticatedMember.getMemberId(),
resolvedLastEventId) so the SSE recovery baseline uses the header when present.
In `@src/main/java/starlight/application/notification/NotificationService.java`:
- Around line 76-83: findMissedMessages currently only checks for null and will
re-query all past notifications when lastEventId is 0 or negative; update the
validation in findMissedMessages to require lastEventId != null && lastEventId >
0 and return List.of() otherwise, before calling
notificationQueryPort.findAllByMemberIdAndIdGreaterThanOrderByIdAsc(memberId,
lastEventId) so the query only happens for a positive lastEventId.
---
Nitpick comments:
In
`@src/main/java/starlight/adapter/notification/sse/NotificationSseRegistry.java`:
- Around line 100-106: sendMissedMessages 메서드에서 missedMessages가 null이거나 비어 있는 경우
NPE를 방지하도록 수비적 코드를 추가하세요: sendMissedMessages(SseEmitter emitter, Long memberId,
List<NotificationPublishMessage> missedMessages) 시작부에 missedMessages == null ||
missedMessages.isEmpty() 검사 후 즉시 반환하도록 하고(또는 필요 시 로깅), 기존
루프(NotificationPublishMessage missedMessage 처리)로 진입하기 전에 빈 컬렉션에 대한 안전한 처리를
보장하세요.
🪄 Autofix (Beta)
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
Run ID: a7ffec3f-9d97-4c51-b4de-60c98035324b
📒 Files selected for processing (10)
src/main/java/starlight/adapter/notification/persistence/NotificationJpa.javasrc/main/java/starlight/adapter/notification/persistence/NotificationRepository.javasrc/main/java/starlight/adapter/notification/sse/NotificationSseRegistry.javasrc/main/java/starlight/adapter/notification/webapi/NotificationController.javasrc/main/java/starlight/adapter/notification/webapi/swagger/NotificationApiDoc.javasrc/main/java/starlight/application/notification/NotificationService.javasrc/main/java/starlight/application/notification/provided/NotificationUseCase.javasrc/main/java/starlight/application/notification/required/NotificationQueryPort.javasrc/main/java/starlight/application/notification/required/NotificationRealtimePort.javasrc/test/java/starlight/application/notification/NotificationServiceUnitTest.java
2ghrms
approved these changes
May 27, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🚀 Why - 해결하려는 문제가 무엇인가요?
/subscribe흐름에서는 재연결 시 마지막 수신 이벤트 이후의 누락 알림을 자동으로 복구해주지 않았습니다.✅ What - 무엇이 변경됐나요?
Last-Event-ID헤더와lastEventId쿼리 파라미터를 받을 수 있도록 변경했습니다.memberId + lastEventId기준으로 이후 알림을 DB에서 조회하는 포트/레포지토리 메서드를 추가했습니다.🛠️ How - 어떻게 해결했나요?
Last-Event-ID헤더 또는lastEventId쿼리 파라미터로 전달하면, 서버가 해당 값 이후의 알림을 DB에서 조회합니다.notification이벤트로 전송하며,notificationId를 SSE event id로 사용합니다.Notification데이터로 분리했습니다.Last-Event-ID헤더를 사용하기 어려운 클라이언트도 고려해lastEventId쿼리 파라미터를 함께 지원했습니다.Summary by CodeRabbit
릴리스 노트