Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 18 seconds. ⌛ 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRedis 기반 다중 사용자 존재 조회 추가, JPA 리포지토리에 다중 조회/저장 메서드 추가, Expo 푸시를 배치(BATCH_SIZE=100)로 전송하도록 도입 및 NotificationService/NotificationInboxService의 그룹 알림 흐름이 배치 중심으로 리팩토링되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NotificationService
participant ChatPresenceService
participant MuteRepo as NotificationMuteSettingRepository
participant TokenRepo as NotificationDeviceTokenRepository
participant InboxService as NotificationInboxService
participant ExpoPushClient
participant ExpoAPI
Client->>NotificationService: sendGroupChatNotification(roomId, senderId, userIds, ...)
NotificationService->>ChatPresenceService: findUsersInChatRoom(roomId, userIds)
ChatPresenceService-->>NotificationService: activeUserIds
NotificationService->>MuteRepo: findMutedUserIdsByTargetTypeAndTargetIdAndUserIds(...)
MuteRepo-->>NotificationService: mutedUserIds
NotificationService->>NotificationService: compute targetRecipients (filteredRecipients - active - muted)
NotificationService->>InboxService: saveAll(targetRecipients, type, title, body, path)
InboxService-->>NotificationService: savedInboxes
NotificationService->>InboxService: sendSseBatch(targetRecipients, savedInboxes)
NotificationService->>TokenRepo: findTokensByUserIds(targetRecipients)
TokenRepo-->>NotificationService: tokens
NotificationService->>ExpoPushClient: sendBatchNotifications(messages)
ExpoPushClient->>ExpoAPI: POST /send (batches of up to 100)
ExpoAPI-->>ExpoPushClient: response
ExpoPushClient-->>NotificationService: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
동아리 단체 채팅 알림 발송에서 수신자 N명에 비례해 발생하던 Redis/DB/외부 API 호출을 일괄 처리(batch) 로 전환하여 알림 발송 지연과 DB 부하를 줄이기 위한 PR입니다.
Changes:
- 채팅방 접속 여부(Redis) / 뮤트 여부(DB) / 디바이스 토큰(DB)을 수신자 단위가 아닌 목록 단위로 조회하도록 변경
- 알림함(NotificationInbox) 저장을 saveAll로 일괄 처리하고, SSE도 저장 결과 기반으로 배치 전송
- Expo 푸시를 100개 단위 배치 전송(Expo 제한)하도록 클라이언트 기능 추가
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java | 그룹 채팅 알림 플로우를 배치 처리로 리팩터링(접속/뮤트/토큰/푸시 일괄) |
| src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java | 알림함 저장(save/saveAll)과 SSE 전송(sendSse/Batch)을 분리 |
| src/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.java | Expo 푸시 배치 전송(sendBatchNotifications) 및 partition 유틸 추가 |
| src/main/java/gg/agit/konect/domain/notification/repository/NotificationMuteSettingRepository.java | 뮤트 유저 ID를 IN 쿼리로 일괄 조회하는 메서드 추가 |
| src/main/java/gg/agit/konect/domain/notification/repository/NotificationInboxRepository.java | 알림함 saveAll 지원 추가 |
| src/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.java | 유저 ID 목록으로 토큰을 일괄 조회하는 쿼리 추가 |
| src/main/java/gg/agit/konect/domain/chat/service/ChatPresenceService.java | Redis multiGet으로 채팅방 접속 유저를 일괄 조회하는 메서드 추가 |
...in/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java (1)
39-44: 🧹 Nitpick | 🔵 Trivial[LEVEL: low] 사용되지 않는 상수와 레코드가 남아있습니다.
문제:
EXPO_PUSH_URL(line 39)과ExpoPushTicket(lines 298-299)은ExpoPushClient로 이전되어 더 이상 사용되지 않습니다.
영향: 코드 가독성 저하 및 혼란 유발 가능성이 있습니다.
제안: 미사용 코드를 제거하세요.♻️ 미사용 코드 제거
- private static final String EXPO_PUSH_URL = "https://exp.host/--/api/v2/push/send";- private record ExpoPushTicket(String status, String message, Map<String, Object> details) { - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java` around lines 39 - 44, Remove the unused EXPO_PUSH_URL constant from NotificationService and delete the ExpoPushTicket record (ExpoPushTicket) from this file since both were moved to ExpoPushClient; search for any remaining references to EXPO_PUSH_URL and ExpoPushTicket in the repository and update/remove them if found, then rebuild/tests to confirm nothing else depends on those symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.java`:
- Around line 41-46: Replace the raw List<Object[]> return with a type-safe
projection: create a record or DTO (e.g., UserTokenProjection with Integer
userId, String token) in gg.agit.konect.domain.notification.dto and change the
repository method findUserIdAndTokenByUserIds to return
List<UserTokenProjection>, updating the `@Query` to use the constructor expression
(new gg.agit.konect.domain.notification.dto.UserTokenProjection(ndt.user.id,
ndt.token)) so callers no longer need index-based casts and get compile-time
type safety.
In
`@src/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.java`:
- Around line 89-139: Add a matching `@Recover` method for the `@Retryable`
sendBatchNotifications(List<ExpoPushMessage>) so failures after retries are
handled consistently; implement a method named recoverSendBatchNotifications (or
similar) annotated with `@Recover` that takes the thrown Exception (or specific
Exception type) as the first parameter and a List<ExpoPushMessage> as the second
parameter, log the failure with contextual details (message count, exception)
and perform any necessary fallback/cleanup (e.g., metrics, DLQ enqueue) so the
exception is not leaked unexpectedly from sendBatchNotifications.
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java`:
- Around line 68-72: sendSseBatch currently calls inbox.getUser().getId() on
detached NotificationInbox instances which can trigger
LazyInitializationException; change the saveAll flow to return the userId
alongside the saved entity (e.g., create a record
SavedInboxWithUserId(SavedInboxWithUserId inbox, Integer userId) or add a userId
field on NotificationInbox and populate it inside the REQUIRES_NEW transaction),
then update sendSseBatch to iterate over the returned items and call
sendSse(item.userId(), NotificationInboxResponse.from(item.inbox())) (or
sendSse(inbox.getUserId(), NotificationInboxResponse.from(inbox)) if you added
the field) so no LAZY load occurs outside the transaction.
---
Outside diff comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java`:
- Around line 39-44: Remove the unused EXPO_PUSH_URL constant from
NotificationService and delete the ExpoPushTicket record (ExpoPushTicket) from
this file since both were moved to ExpoPushClient; search for any remaining
references to EXPO_PUSH_URL and ExpoPushTicket in the repository and
update/remove them if found, then rebuild/tests to confirm nothing else depends
on those symbols.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: ASSERTIVE
Plan: Pro
Run ID: c035fa9d-81d7-4a27-af4c-72c75889d87d
📒 Files selected for processing (7)
src/main/java/gg/agit/konect/domain/chat/service/ChatPresenceService.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationInboxRepository.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationMuteSettingRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/notification/repository/NotificationInboxRepository.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/chat/service/ChatPresenceService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationMuteSettingRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/notification/repository/NotificationInboxRepository.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/chat/service/ChatPresenceService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/repository/NotificationMuteSettingRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
🔇 Additional comments (8)
src/main/java/gg/agit/konect/domain/notification/repository/NotificationInboxRepository.java (1)
22-22: LGTM!Spring Data JPA의 표준
saveAll메서드 시그니처로, 배치 저장 기능이 올바르게 선언되었습니다.src/main/java/gg/agit/konect/domain/chat/service/ChatPresenceService.java (1)
59-80: LGTM!Redis
multiGet을 활용한 배치 프레즌스 조회가 올바르게 구현되었습니다. null/empty 입력에 대한 방어 로직과multiGet결과 처리가 적절합니다.src/main/java/gg/agit/konect/domain/notification/repository/NotificationMuteSettingRepository.java (1)
56-68: LGTM!배치 조회를 위한 쿼리가 올바르게 구현되었습니다.
Set<Integer>반환 타입이 중복 제거와contains()성능에 적합합니다.src/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.java (1)
141-150: LGTM!
partition헬퍼와ExpoPushMessage레코드가 적절하게 구현되었습니다.subList사용은 현재 컨텍스트에서 안전합니다.src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java (1)
34-38: LGTM!
save()메서드가 저장된 엔티티를 반환하도록 수정되어 호출부에서 SSE 전송에 활용할 수 있습니다.REQUIRES_NEW전파는 비동기 컨텍스트에서 적절합니다.src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java (3)
145-167: LGTM!배치 프레즌스 조회와 뮤트 사용자 필터링이 효율적으로 구현되었습니다. 기존 N개의 개별 호출이 각각 1개의 Redis/DB 호출로 최적화되었습니다.
173-204: LGTM!인박스 배치 저장, SSE 배치 전송, 토큰 배치 조회 및 Expo 푸시 배치 전송이 올바르게 구현되었습니다. PR 목표인 성능 개선(~27s → ~0.6s)에 부합하는 변경입니다.
220-254: LGTM!클럽 알림 메서드들이
save()반환값을 활용하여 SSE를 명시적으로 전송하도록 수정되었습니다. 저장과 SSE 전송의 관심사 분리가 명확해졌습니다.
...in/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
Show resolved
Hide resolved
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java (1)
45-65:⚠️ Potential issue | 🔴 Critical[LEVEL: high] 누락된 사용자로 인해 SSE 수신자가 밀릴 수 있습니다
문제:saveAll()은findAllByIdIn()에서 빠진 사용자(예: soft-delete)를 건너뛰지만sendSseBatch()는 여전히 원본userIds를 인덱스로 사용합니다.
영향: 중간에 누락된 사용자가 1명만 있어도 뒤쪽NotificationInbox가 다른 사용자 ID로 밀려 전송되어 잘못된 SSE와 알림 내용 노출이 발생합니다.
제안:userId와 저장된NotificationInbox를 함께 반환하는 구조로 바꾸고sendSseBatch()는 그userId로 전송하세요.Also applies to: 76-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java` around lines 45 - 65, saveAll currently drops missing users and returns only NotificationInbox objects, causing sendSseBatch to misalign SSE recipients when userIds contain gaps; change saveAll (in NotificationInboxService) to return a list of pairs/DTOs that preserve the original userId alongside the saved NotificationInbox (e.g., a small class or Map entry like (userId, notificationInbox)), build those by iterating the original userIds, looking up User via userMap, skipping nulls, creating and saving NotificationInbox with NotificationInbox.of(...), and return the list of (userId, notification) entries; then update sendSseBatch to iterate this list and use the preserved userId for sending SSEs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.java`:
- Around line 89-139: sendBatchNotifications currently has `@Retryable` on the
whole method which causes already-sent batches to be resent; refactor by
extracting the per-batch HTTP logic into a new method
sendSingleBatch(List<ExpoPushMessage> batch) and annotate that method with
`@Retryable`(maxAttempts = 2) so only individual failing batches are retried. Keep
partition(...) and the batching loop in sendBatchNotifications, remove
`@Retryable` from sendBatchNotifications, and inside the loop call
sendSingleBatch(batch) which should perform the headers creation, HttpEntity
creation, expoRestTemplate.exchange(EXPO_PUSH_URL, HttpMethod.POST, entity,
ExpoPushResponse.class), status/body checks, and ticket logging as currently
implemented.
---
Duplicate comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java`:
- Around line 45-65: saveAll currently drops missing users and returns only
NotificationInbox objects, causing sendSseBatch to misalign SSE recipients when
userIds contain gaps; change saveAll (in NotificationInboxService) to return a
list of pairs/DTOs that preserve the original userId alongside the saved
NotificationInbox (e.g., a small class or Map entry like (userId,
notificationInbox)), build those by iterating the original userIds, looking up
User via userMap, skipping nulls, creating and saving NotificationInbox with
NotificationInbox.of(...), and return the list of (userId, notification)
entries; then update sendSseBatch to iterate this list and use the preserved
userId for sending SSEs.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 56e48b85-852b-494a-b6bc-363c0a2728b1
📒 Files selected for processing (4)
src/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/notification/repository/NotificationDeviceTokenRepository.javasrc/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
src/main/java/gg/agit/konect/domain/notification/service/ExpoPushClient.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java`:
- Around line 206-208: The catch block around sendBatchNotifications currently
only logs the error (log.error(..., e)) so saved inbox entries (saveAll) never
get retried; modify the catch to enqueue the failed notification payload(s) for
retry (e.g., push to a retry queue/topic or a dead-letter table) and/or trigger
a retry mechanism with backoff. Locate the catch in NotificationService (around
sendBatchNotifications and the surrounding saveAll flow), capture the failing
context (roomId, senderId, message IDs) and publish those details to your retry
queue or persist them to a retry table instead of just logging; ensure the
enqueue operation is resilient (non-blocking or wrapped with its own error
handling) so failures there do not lose the original data.
- Around line 173-181: The current code maps original targetRecipients by index
to savedInboxes which can be smaller after saveAll filtered out non-existent
users; change sendSseBatch to use the actual saved recipients derived from
savedInboxes (e.g., map each NotificationInbox -> inbox.getUser().getId() or
extract stored userIds from savedInboxes) so the SSE payload aligns with
savedInboxes; update the call in NotificationService (replace
notificationInboxService.sendSseBatch(targetRecipients, savedInboxes) with a
call that passes the saved user IDs list and savedInboxes) and ensure
sendSseBatch's parameters/signature are used accordingly to avoid index
mismatches or IndexOutOfBoundsException.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5f6a8139-30a5-4f47-a8a3-271f484686a3
📒 Files selected for processing (1)
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
🔇 Additional comments (2)
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java (2)
100-119: LGTM - 1:1 채팅 알림 흐름이 적절하게 리팩터링됨저장 후 SSE 발송, 이후 푸시 알림 전송 순서가 올바르며, SSE 실패가 푸시 알림을 차단하지 않습니다.
221-244: LGTM - 동아리 신청 알림 메서드들이 일관된 패턴으로 리팩터링됨저장 → SSE 발송 → 푸시 알림 순서가
sendChatNotification과 동일하게 적용되었습니다.
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
Outdated
Show resolved
Hide resolved
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
Show resolved
Hide resolved
This was referenced Mar 27, 2026
Merged
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
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.
🔍 개요
🚀 주요 변경 내용
💬 참고 사항
✅ Checklist (완료 조건)