[Feat] SSE 기반 알림 및 읽음 처리 기능 추가 (#120)#122
Merged
Merged
Conversation
SSE 기반 인앱 알림 기능을 추가했습니다. 알림 엔티티와 저장소를 도입하고, 목록 조회·미읽음 개수 조회·개별 읽음 처리·전체 읽음 처리 API를 구현했으며, 실시간 알림 스트림을 통해 프론트가 즉시 알림을 표시할 수 있도록 구성했습니다. 또한 async 작업 완료/실패 시 taskId, mockApplyId, jobPostingId 등 이동용 payload를 포함한 알림이 자동 생성되도록 연결했습니다.
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughIntroduces an in-app notification system: a new ChangesNotification feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant NotificationController
participant NotificationService
participant NotificationRepository
participant NotificationSseService
Client->>NotificationController: GET /stream
NotificationController->>NotificationService: subscribe(user)
NotificationService->>NotificationSseService: subscribe(userId, bootstrapSupplier)
NotificationSseService-->>Client: SseEmitter (bootstrap event)
Note over NotificationService: Later, async task completes
NotificationService->>NotificationRepository: save(Notification)
NotificationService->>NotificationService: publishAfterCommit(event)
NotificationService->>NotificationSseService: publish(userId, event)
NotificationSseService-->>Client: SSE event (CREATED)
sequenceDiagram
participant AsyncTaskService
participant NotificationService
participant NotificationRepository
participant NotificationSseService
AsyncTaskService->>AsyncTaskService: markSuccess(task) / markFailed(task)
AsyncTaskService->>NotificationService: createNotification(payload, type, target)
NotificationService->>NotificationRepository: save(Notification.create(...))
NotificationService->>NotificationSseService: publish(userId, streamEvent)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.java (1)
67-73: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winNotification errors can roll back task completion.
NotificationService.createNotification(...)is@Transactionalwith defaultREQUIRED, somarkSuccess/markFailedjoin the same transaction. If user lookup, payload serialization, ornotificationRepository.save(...)throws, the task’sSUCCEEDED/FAILEDtransition rolls back too, and the after-commit SSE event never fires. Isolate notification creation (REQUIRES_NEW) or catch/log notification failures.🤖 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/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.java` around lines 67 - 73, Notification creation in AnalysisAsyncTaskService.markSuccess (and the corresponding failure path) is joining the same transaction as the task status update, so any exception from NotificationService.createNotification can roll back the task transition and prevent publishAfterCommit from firing. Update the notification flow so it is isolated from the task transaction, either by moving notification creation to a REQUIRES_NEW transaction in NotificationService/createNotification or by wrapping createSuccessNotification/createFailureNotification in try-catch and logging failures without rethrowing, while keeping getTask and markSuccess/markFailed behavior unchanged.src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java (1)
71-86: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep notification failures off the completion path.
createSuccessNotification/createFailureNotificationrun inside the same@Transactionalmethod, so any exception rolls backmarkSuccess/markFailedand makes the worker request fail before the completedJobPostingIngestResponseis returned. Wrap them in a best-effort catch/log path or move them after commit.🤖 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/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java` around lines 71 - 86, `markSuccess` in `JobPostingAsyncTaskService` currently calls `createSuccessNotification` inside the transactional completion path, so a notification exception can roll back the task update and fail the worker request. Update `markSuccess` and the related `markFailed` flow to make notifications best-effort only: either move `createSuccessNotification` / `createFailureNotification` to an after-commit callback or wrap them in a try-catch that logs and suppresses notification errors, while still returning the completed `JobPostingIngestResponse`.
🧹 Nitpick comments (5)
ops/db/migrations/20260707_notifications.sql (1)
19-20: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a partial index for unread lookups.
countByUserIdAndReadAtIsNullandmarkAllAsReadonly ever query rows whereread_at IS NULL. A partial index limited to that condition is smaller and faster than indexing every row.Proposed partial index
-CREATE INDEX IF NOT EXISTS idx_notifications_user_read_at - ON notifications (user_id, read_at); +CREATE INDEX IF NOT EXISTS idx_notifications_user_unread + ON notifications (user_id) WHERE read_at IS NULL;🤖 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 `@ops/db/migrations/20260707_notifications.sql` around lines 19 - 20, The notifications index currently covers all rows, but `countByUserIdAndReadAtIsNull` and `markAllAsRead` only target unread notifications. Update the index definition in the migration to use a partial index on `notifications` constrained to rows where `read_at IS NULL`, keeping the existing `idx_notifications_user_read_at` name or equivalent so the query paths can use a smaller, faster index.src/main/java/com/jobdri/jobdri_api/domain/notification/controller/NotificationController.java (1)
53-59: 🩺 Stability & Availability | 🔵 TrivialConsider anti-buffering headers for the SSE stream.
If this endpoint sits behind a reverse proxy (nginx, etc.), responses can be buffered unless headers like
Cache-Control: no-cacheandX-Accel-Buffering: noare set, which would delay "real-time" delivery — the core goal of this feature per the PR objectives. Worth confirming the deployment topology and adding these headers (or proxy-level config) if applicable.🤖 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/com/jobdri/jobdri_api/domain/notification/controller/NotificationController.java` around lines 53 - 59, The SSE stream in NotificationController.streamNotifications should disable buffering so real-time events are not delayed by proxies. Update the response setup for this endpoint to include anti-buffering headers such as Cache-Control: no-cache and X-Accel-Buffering: no, either directly in the controller response or via the surrounding proxy configuration if that is where response headers are managed. Make sure the change is applied where streamNotifications returns the SseEmitter so the stream remains truly real-time behind nginx or similar reverse proxies.src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskServiceTest.java (2)
36-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an import instead of the fully-qualified type.
Minor style nit; add an
import com.jobdri.jobdri_api.domain.notification.service.NotificationService;instead of the inline fully-qualified reference.🤖 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/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskServiceTest.java` around lines 36 - 37, The test class uses a fully-qualified type for the NotificationService mock instead of an import, which is a style issue. Update JobPostingAsyncTaskServiceTest to import NotificationService and change the `@Mock` field to use the simple class name so the test matches the surrounding import style and is easier to read.
41-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for the new notification hooks.
The mock is wired in, but no test verifies
notificationService.createNotification(...)is invoked with correct arguments onmarkSuccess/markFailed, nor that it's not called on retry-scheduled paths. Given the transaction-coupling risk flagged inJobPostingAsyncTaskService, a test exercisingmarkSuccess/markFailedand verifying interactions withnotificationServicewould also help catch regressions here.🤖 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/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskServiceTest.java` around lines 41 - 49, Add test coverage in JobPostingAsyncTaskServiceTest for the new notification hooks: verify that JobPostingAsyncTaskService.markSuccess and markFailed invoke notificationService.createNotification with the expected arguments, and add assertions that retry-scheduled paths do not call notificationService. Use the existing setUp wiring, and target the markSuccess, markFailed, and any retry-related methods in JobPostingAsyncTaskService so the interaction with notificationService is explicitly covered.src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java (1)
267-272: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate condition computed three times.
result.getSaved() != null && result.getSaved().getJobPostingId() != nullis repeated for the target type and target id. Extract to a local variable for clarity.♻️ Suggested refactor
+ boolean hasJobPostingId = result.getSaved() != null && result.getSaved().getJobPostingId() != null; notificationService.createNotification( task.getUserId(), NotificationType.JOB_POSTING_ASYNC_SUCCEEDED, "채용 공고 작업이 완료되었습니다.", result.isSavedToDatabase() ? "채용 공고 분석과 저장이 완료되었습니다." : "채용 공고 분석이 완료되었습니다.", - result.getSaved() != null && result.getSaved().getJobPostingId() != null - ? NotificationTargetType.JOB_POSTING_RESULT - : NotificationTargetType.JOB_POSTING_TASK, - result.getSaved() != null && result.getSaved().getJobPostingId() != null - ? String.valueOf(result.getSaved().getJobPostingId()) - : task.getTaskId(), + hasJobPostingId ? NotificationTargetType.JOB_POSTING_RESULT : NotificationTargetType.JOB_POSTING_TASK, + hasJobPostingId ? String.valueOf(result.getSaved().getJobPostingId()) : task.getTaskId(), payload );🤖 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/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java` around lines 267 - 272, The condition in JobPostingAsyncTaskService is repeated multiple times when choosing the notification target type and target id, making the expression harder to read and maintain. Extract the repeated result.getSaved() != null && result.getSaved().getJobPostingId() != null check into a local boolean in the method that builds the notification payload, then reuse that variable for both the NotificationTargetType and target id selection.
🤖 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/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.java`:
- Line 200: The failure notification body in AnalysisAsyncTaskService is
exposing worker-provided task.getError() directly to users. Update the
notification-building logic around the task.getError() fallback to always send a
generic user-facing message, and keep the detailed error text only in internal
logs or admin-only fields so the public notification body is sanitized.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java`:
- Line 287: The notification in JobPostingAsyncTaskService currently exposes
worker-provided task.getError() directly to the user, so replace it with a
generic failure message and keep the detailed error only in server-side logging
or internal handling. Update the error selection logic in the notification path
that uses task.getError() so the user-facing text is always the fallback
message, while preserving the original error for logs or diagnostics elsewhere
in the service.
In
`@src/main/java/com/jobdri/jobdri_api/domain/notification/service/NotificationService.java`:
- Around line 115-149: `NotificationService.createNotification(...)` is still
participating in the caller’s transaction, so a failure in
`userService.getUser(...)` or `serializePayload(...)` can roll back
`AnalysisAsyncTaskService.markSuccess/markFailed` and
`JobPostingAsyncTaskService.markSuccess/markFailed`. Isolate this notification
side effect by moving the notification write to its own transaction boundary
(for example, a separate transactional method/service invoked from the async
task flow) so task status updates in those methods commit independently even if
notification creation fails.
- Around line 57-69: In NotificationService, guard the SSE bootstrap work in
subscribe() so repository failures don’t leave the emitter registered; move the
count/findTop50 bootstrap calls behind the notificationSseService registration
or wrap them so cleanup/error handling in subscribe() can still run if bootstrap
fails. Also isolate createNotification() from the async task-status transaction
by moving the user lookup/serialization/notification insert into a separate
best-effort transaction or helper so failures there do not roll back the task
transition.
---
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.java`:
- Around line 67-73: Notification creation in
AnalysisAsyncTaskService.markSuccess (and the corresponding failure path) is
joining the same transaction as the task status update, so any exception from
NotificationService.createNotification can roll back the task transition and
prevent publishAfterCommit from firing. Update the notification flow so it is
isolated from the task transaction, either by moving notification creation to a
REQUIRES_NEW transaction in NotificationService/createNotification or by
wrapping createSuccessNotification/createFailureNotification in try-catch and
logging failures without rethrowing, while keeping getTask and
markSuccess/markFailed behavior unchanged.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java`:
- Around line 71-86: `markSuccess` in `JobPostingAsyncTaskService` currently
calls `createSuccessNotification` inside the transactional completion path, so a
notification exception can roll back the task update and fail the worker
request. Update `markSuccess` and the related `markFailed` flow to make
notifications best-effort only: either move `createSuccessNotification` /
`createFailureNotification` to an after-commit callback or wrap them in a
try-catch that logs and suppresses notification errors, while still returning
the completed `JobPostingIngestResponse`.
---
Nitpick comments:
In `@ops/db/migrations/20260707_notifications.sql`:
- Around line 19-20: The notifications index currently covers all rows, but
`countByUserIdAndReadAtIsNull` and `markAllAsRead` only target unread
notifications. Update the index definition in the migration to use a partial
index on `notifications` constrained to rows where `read_at IS NULL`, keeping
the existing `idx_notifications_user_read_at` name or equivalent so the query
paths can use a smaller, faster index.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.java`:
- Around line 267-272: The condition in JobPostingAsyncTaskService is repeated
multiple times when choosing the notification target type and target id, making
the expression harder to read and maintain. Extract the repeated
result.getSaved() != null && result.getSaved().getJobPostingId() != null check
into a local boolean in the method that builds the notification payload, then
reuse that variable for both the NotificationTargetType and target id selection.
In
`@src/main/java/com/jobdri/jobdri_api/domain/notification/controller/NotificationController.java`:
- Around line 53-59: The SSE stream in
NotificationController.streamNotifications should disable buffering so real-time
events are not delayed by proxies. Update the response setup for this endpoint
to include anti-buffering headers such as Cache-Control: no-cache and
X-Accel-Buffering: no, either directly in the controller response or via the
surrounding proxy configuration if that is where response headers are managed.
Make sure the change is applied where streamNotifications returns the SseEmitter
so the stream remains truly real-time behind nginx or similar reverse proxies.
In
`@src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskServiceTest.java`:
- Around line 36-37: The test class uses a fully-qualified type for the
NotificationService mock instead of an import, which is a style issue. Update
JobPostingAsyncTaskServiceTest to import NotificationService and change the
`@Mock` field to use the simple class name so the test matches the surrounding
import style and is easier to read.
- Around line 41-49: Add test coverage in JobPostingAsyncTaskServiceTest for the
new notification hooks: verify that JobPostingAsyncTaskService.markSuccess and
markFailed invoke notificationService.createNotification with the expected
arguments, and add assertions that retry-scheduled paths do not call
notificationService. Use the existing setUp wiring, and target the markSuccess,
markFailed, and any retry-related methods in JobPostingAsyncTaskService so the
interaction with notificationService is explicitly covered.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fcc2c702-8c70-4116-ab81-a9184676c366
📒 Files selected for processing (17)
.gitignoreops/db/migrations/20260707_notifications.sqlsrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskService.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/controller/NotificationController.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/dto/response/NotificationReadAllResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/dto/response/NotificationResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/dto/response/NotificationStreamBootstrapResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/dto/response/NotificationStreamEventResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/dto/response/NotificationUnreadCountResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/entity/Notification.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/entity/NotificationTargetType.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/entity/NotificationType.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/repository/NotificationRepository.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/service/NotificationService.javasrc/main/java/com/jobdri/jobdri_api/domain/notification/service/NotificationSseService.javasrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncTaskServiceTest.java
- async task 완료/실패 시 알림 생성이 상태 전이 트랜잭션을 롤백하지 않도록 알림 저장을 별도 트랜잭션으로 분리하고 호출부를 best-effort 처리로 변경 - 분석/채용공고 실패 알림에서 worker 에러 원문 노출을 제거하고 사용자용 메시지로 통일 - 알림 SSE 응답에 Cache-Control: no-cache, X-Accel-Buffering: no 헤더 추가 - unread 조회 경로에 맞게 notifications unread index를 partial index로 조정 - job posting async notification 호출 테스트와 import/style 정리 추가
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
1 participant
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.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
SSE 기반 인앱 알림 기능을 추가했습니다. 알림 엔티티와 저장소를 도입하고, 목록 조회·미읽음 개수 조회·개별 읽음 처리·전체 읽음 처리 API를 구현했으며, 실시간 알림 스트림을 통해 프론트가 즉시 알림을 표시할 수 있도록 구성했습니다. 또한 async 작업 완료/실패 시 taskId, mockApplyId, jobPostingId 등 이동용 payload를 포함한 알림이 자동 생성되도록 연결했습니다.
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#121 ]
closes #121
Summary by CodeRabbit
New Features
Bug Fixes
Chores