Merged
Conversation
dh2906
changed the title
📝 WalkthroughWalkthroughNotificationInboxService의 save/saveAll 메서드 트랜잭션 전파를 REQUIRES_NEW에서 기본 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
시
🚥 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 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
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)
230-248:⚠️ Potential issue | 🔴 Critical[LEVEL: high]
@Transactional누락으로 알림 저장 실패 발생
sendClubApplicationApprovedNotification과sendClubApplicationRejectedNotification에@Transactional이 없어 클래스 레벨의readOnly=true가 적용됩니다.NotificationInboxService.save()에서REQUIRES_NEW가 제거되었으므로, 읽기 전용 트랜잭션 내에서 저장 시도 시 데이터가 영속되지 않습니다. 다른 비동기 메서드와 동일하게@Transactional을 추가해야 합니다.🐛 누락된 `@Transactional` 추가 제안
`@Async` +@Transactional public void sendClubApplicationApprovedNotification(Integer receiverId, Integer clubId, String clubName) { String body = "동아리 지원이 승인되었어요."; String path = "clubs/" + clubId; NotificationInbox saved = notificationInboxService.save( receiverId, NotificationInboxType.CLUB_APPLICATION_APPROVED, clubName, body, path); notificationInboxService.sendSse(receiverId, NotificationInboxResponse.from(saved)); sendNotification(receiverId, clubName, body, path); } `@Async` +@Transactional public void sendClubApplicationRejectedNotification(Integer receiverId, Integer clubId, String clubName) { String body = "동아리 지원이 거절되었어요."; String path = "clubs/" + clubId; NotificationInbox saved = notificationInboxService.save( receiverId, NotificationInboxType.CLUB_APPLICATION_REJECTED, clubName, body, path); notificationInboxService.sendSse(receiverId, NotificationInboxResponse.from(saved)); sendNotification(receiverId, clubName, body, path); }🤖 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 230 - 248, The two async methods sendClubApplicationApprovedNotification and sendClubApplicationRejectedNotification are missing `@Transactional` so class-level readOnly=true prevents persisting via notificationInboxService.save() (REQUIRES_NEW was removed); add a method-level `@Transactional` (without readOnly) to both methods so the save executes in a writable transaction, keeping `@Async` and preserving existing logic including calls to notificationInboxService.save, notificationInboxService.sendSse, and sendNotification.
🤖 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/NotificationInboxService.java`:
- Around line 33-37: The class-level `@Transactional`(readOnly = true) prevents
write operations, so add `@Transactional` to the two methods
sendClubApplicationApprovedNotification and
sendClubApplicationRejectedNotification in NotificationInboxService to allow
notificationInboxService.save(...) to persist; annotate both methods with
`@Transactional` (and import if missing) so they run with a writable transaction
while leaving the class-level readOnly default for others.
---
Outside diff comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java`:
- Around line 230-248: The two async methods
sendClubApplicationApprovedNotification and
sendClubApplicationRejectedNotification are missing `@Transactional` so
class-level readOnly=true prevents persisting via
notificationInboxService.save() (REQUIRES_NEW was removed); add a method-level
`@Transactional` (without readOnly) to both methods so the save executes in a
writable transaction, keeping `@Async` and preserving existing logic including
calls to notificationInboxService.save, notificationInboxService.sendSse, and
sendNotification.
🪄 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: 5fd6069e-4529-43ca-9122-fed88c901cff
📒 Files selected for processing (2)
src/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/service/NotificationInboxService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.javasrc/main/java/gg/agit/konect/domain/notification/service/NotificationService.java
🔇 Additional comments (3)
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java (3)
76-77: [LEVEL: low]@Async와@Transactional조합 적용 확인됨비동기 메서드에
@Transactional이 추가되어notificationInboxService.save()가 쓰기 가능한 트랜잭션 내에서 실행됩니다. PR 목적에 부합하는 변경입니다.
126-127: [LEVEL: low] 그룹 채팅 알림 메서드 트랜잭션 설정 적용됨
sendGroupChatNotification에@Transactional이 추가되어saveAll()호출 시 쓰기 트랜잭션이 보장됩니다.
213-214: [LEVEL: low] 클럽 가입 신청 알림 메서드 트랜잭션 설정 적용됨
sendClubApplicationSubmittedNotification에@Transactional이 추가되어 알림 저장이 정상 동작합니다.
src/main/java/gg/agit/konect/domain/notification/service/NotificationInboxService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java (3)
213-228:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 예외 처리 누락으로 인한 알림 손실 가능성
sendChatNotification,sendGroupChatNotification과 달리 try-catch 블록이 없어서expoPushClient.sendNotification()호출 시 예외 발생하면 트랜잭션이 롤백되어 인박스 저장이 취소됩니다. 다른 알림 메서드들과 동일하게 try-catch로 감싸서 푸시 실패와 무관하게 인박스 저장이 보장되도록 수정하세요.🔧 제안 수정
`@Async` `@Transactional` public void sendClubApplicationSubmittedNotification( Integer receiverId, Integer applicationId, Integer clubId, String clubName, String applicantName ) { + try { String body = applicantName + "님이 동아리 가입을 신청했어요."; String path = "mypage/manager/" + clubId + "/applications/" + applicationId; NotificationInbox saved = notificationInboxService.save( receiverId, NotificationInboxType.CLUB_APPLICATION_SUBMITTED, clubName, body, path); notificationInboxService.sendSse(receiverId, NotificationInboxResponse.from(saved)); sendNotification(receiverId, clubName, body, path); + } catch (Exception e) { + log.error("Failed to send club application submitted notification: receiverId={}, clubId={}", receiverId, clubId, e); + } }🤖 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 213 - 228, The sendClubApplicationSubmittedNotification method lacks exception handling around the external push call, so failures in expoPushClient (triggered by sendNotification) can roll back the transaction and undo the saved NotificationInbox; wrap the sendNotification(receiverId, clubName, body, path) call in a try-catch that catches Exception (or a specific client exception), logs the error, and does not rethrow it so the saved NotificationInbox (notificationInboxService.save(...) stored in variable saved) and the SSE send (notificationInboxService.sendSse(...)) remain committed; alternatively move only the push dispatch outside the `@Transactional` boundary, but the simplest fix is to keep the save and sendSse as-is and surround sendNotification(...) with a try-catch that logs the failure.
230-239:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 예외 처리 누락으로 인한 알림 손실 가능성
위
sendClubApplicationSubmittedNotification과 동일한 문제입니다.sendNotification()실패 시 트랜잭션 롤백으로 인박스 저장이 취소됩니다. try-catch 블록을 추가하세요.🔧 제안 수정
`@Async` `@Transactional` public void sendClubApplicationApprovedNotification(Integer receiverId, Integer clubId, String clubName) { + try { String body = "동아리 지원이 승인되었어요."; String path = "clubs/" + clubId; NotificationInbox saved = notificationInboxService.save( receiverId, NotificationInboxType.CLUB_APPLICATION_APPROVED, clubName, body, path); notificationInboxService.sendSse(receiverId, NotificationInboxResponse.from(saved)); sendNotification(receiverId, clubName, body, path); + } catch (Exception e) { + log.error("Failed to send club application approved notification: receiverId={}, clubId={}", receiverId, clubId, e); + } }🤖 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 230 - 239, The sendClubApplicationApprovedNotification method currently calls sendNotification after saving the inbox entry inside the transactional method, so if sendNotification throws the transaction will roll back and the saved NotificationInbox (created via notificationInboxService.save) will be lost; wrap the call to sendNotification(receiverId, clubName, body, path) in a try-catch that catches Exception, logs the failure, and does not rethrow (keeping the method transactional for the save/sendSse steps), or alternatively move sendNotification outside the `@Transactional` scope so notificationInboxService.save and notificationInboxService.sendSse (and NotificationInboxResponse.from(saved)) always commit; update sendClubApplicationApprovedNotification accordingly to ensure failures in sendNotification do not roll back the saved inbox.
241-250:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 예외 처리 누락으로 인한 알림 손실 가능성
위 메서드들과 동일한 문제입니다.
sendNotification()실패 시 트랜잭션 롤백으로 인박스 저장이 취소됩니다. try-catch 블록을 추가하세요.🔧 제안 수정
`@Async` `@Transactional` public void sendClubApplicationRejectedNotification(Integer receiverId, Integer clubId, String clubName) { + try { String body = "동아리 지원이 거절되었어요."; String path = "clubs/" + clubId; NotificationInbox saved = notificationInboxService.save( receiverId, NotificationInboxType.CLUB_APPLICATION_REJECTED, clubName, body, path); notificationInboxService.sendSse(receiverId, NotificationInboxResponse.from(saved)); sendNotification(receiverId, clubName, body, path); + } catch (Exception e) { + log.error("Failed to send club application rejected notification: receiverId={}, clubId={}", receiverId, clubId, e); + } }🤖 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 241 - 250, The method sendClubApplicationRejectedNotification currently saves the inbox (notificationInboxService.save) and sends SSE (notificationInboxService.sendSse) but calls sendNotification without exception handling so a failure there can trigger a transaction rollback and lose the saved inbox; wrap the call to sendNotification in a try-catch (catch Exception) inside sendClubApplicationRejectedNotification, log the error, and ensure the saved NotificationInbox (saved) and SSE sending are not rolled back or retried; keep the current behavior of persisting and sending SSE, but swallow or record sendNotification failures instead of letting them propagate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/main/java/gg/agit/konect/domain/notification/service/NotificationService.java`:
- Around line 213-228: The sendClubApplicationSubmittedNotification method lacks
exception handling around the external push call, so failures in expoPushClient
(triggered by sendNotification) can roll back the transaction and undo the saved
NotificationInbox; wrap the sendNotification(receiverId, clubName, body, path)
call in a try-catch that catches Exception (or a specific client exception),
logs the error, and does not rethrow it so the saved NotificationInbox
(notificationInboxService.save(...) stored in variable saved) and the SSE send
(notificationInboxService.sendSse(...)) remain committed; alternatively move
only the push dispatch outside the `@Transactional` boundary, but the simplest fix
is to keep the save and sendSse as-is and surround sendNotification(...) with a
try-catch that logs the failure.
- Around line 230-239: The sendClubApplicationApprovedNotification method
currently calls sendNotification after saving the inbox entry inside the
transactional method, so if sendNotification throws the transaction will roll
back and the saved NotificationInbox (created via notificationInboxService.save)
will be lost; wrap the call to sendNotification(receiverId, clubName, body,
path) in a try-catch that catches Exception, logs the failure, and does not
rethrow (keeping the method transactional for the save/sendSse steps), or
alternatively move sendNotification outside the `@Transactional` scope so
notificationInboxService.save and notificationInboxService.sendSse (and
NotificationInboxResponse.from(saved)) always commit; update
sendClubApplicationApprovedNotification accordingly to ensure failures in
sendNotification do not roll back the saved inbox.
- Around line 241-250: The method sendClubApplicationRejectedNotification
currently saves the inbox (notificationInboxService.save) and sends SSE
(notificationInboxService.sendSse) but calls sendNotification without exception
handling so a failure there can trigger a transaction rollback and lose the
saved inbox; wrap the call to sendNotification in a try-catch (catch Exception)
inside sendClubApplicationRejectedNotification, log the error, and ensure the
saved NotificationInbox (saved) and SSE sending are not rolled back or retried;
keep the current behavior of persisting and sending SSE, but swallow or record
sendNotification failures instead of letting them propagate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4a7f9df-9545-46d9-b18f-156db95dee9f
📒 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)
76-124: LGTM - 트랜잭션 및 예외 처리 패턴이 적절합니다.
@Async메서드에@Transactional을 추가하여 읽기 전용 트랜잭션 문제를 해결했고, try-catch 블록으로 푸시 실패 시에도 인박스 저장이 롤백되지 않도록 처리되어 있습니다.
126-211: LGTM - 배치 알림 처리 로직이 적절합니다.
sendChatNotification과 동일한 패턴으로 트랜잭션과 예외 처리가 구성되어 있어 일관성이 유지됩니다.
This was referenced Apr 1, 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
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.
🔍 개요
인앱 알림 저장 로직의 트랜잭션 전파 옵션이
REQUIRES_NEW로 설정됨이로 인해, 새로운 트랜잭션이 생성되어 추가로 커넥션을 차지하게 되어 DB 커넥션 고갈하는 문제가 발생
🚀 주요 변경 내용
인앱 알림의
save(),saveAll()메소드 트랜잭션 전파 옵션을 제거했습니다.NotificationService.sendChatNotification()와 같은 비동기 메소드에서save()메소드를 호출하는데, 이는 읽기 전용 트랜잭션으로 되어있어 DB 저장에 실패하여 비동기 메소드에서도 트랜잭션을 명시했습니다.💬 참고 사항
✅ Checklist (완료 조건)