Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | 변경 요약 |
|---|---|
SSE 이벤트 데이터 모델 backend/src/main/java/moadong/club/payload/dto/ApplicantStatusEvent.java |
지원자 상태 변경을 전달하는 불변 Java record 타입 추가 (applicantId, status, memo, timestamp, clubId, applicationFormId). |
글로벌 비동기 설정 backend/src/main/java/moadong/global/config/WebConfig.java |
SSE 처리를 위한 configureAsyncSupport(AsyncSupportConfigurer) 오버라이드 추가 (기본 타임아웃 30000ms). |
컨트롤러: SSE 엔드포인트 backend/src/main/java/moadong/club/controller/ClubApplyAdminController.java |
GET /api/club/applicant/{applicationFormId}/events 엔드포인트 추가(Produces text/event-stream, 반환형 SseEmitter), 인증·문서 주석 포함. |
서비스: SSE 연결관리 및 이벤트 브로드캐스트 backend/src/main/java/moadong/club/service/ClubApplyAdminService.java |
ClubRepository 주입·SSE 연결 저장소(ConcurrentHashMap) 및 타임아웃 상수 추가, createSseConnection(...), validateClubOwner(...), sendStatusChangeEvent(...) 등 SSE 관련 메서드 추가. editApplicantDetail에서 트랜잭션 커밋 후 ApplicantStatusEvent 생성·방출 로직 등록 및 연결 정리(완료/타임아웃/오류 처리) 추가. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as ClubApplyAdminController
participant Service as ClubApplyAdminService
participant SSEStore as SSE 저장소
participant TxnSync as TransactionSynchronization
Client->>Controller: GET /api/club/applicant/{formId}/events
Controller->>Service: createSseConnection(formId, user)
Service->>Service: validateClubOwner(clubId, user)
Service->>SSEStore: SseEmitter 생성·등록
Service->>Client: 초기 "connected" 이벤트 전송
Controller-->>Client: SseEmitter 반환 (스트림 유지)
Admin->>Controller: 지원자 상태 편집 요청
Controller->>Service: editApplicantDetail(...)
Service->>Service: validateClubOwner(clubId, user)
Service->>Service: DB 업데이트 (상태/메모)
Service->>TxnSync: afterCommit 콜백 등록
TxnSync-->>Service: afterCommit 실행
Service->>Service: ApplicantStatusEvent 생성
Service->>SSEStore: 관련 SseEmitter들로 "applicant-status-changed" 이벤트 전송
SSEStore-->>Client: 클라이언트가 이벤트 수신
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
검토 시 주의할 점:
- SseEmitter 생성·초기화와 완료/타임아웃/오류 콜백의 정확성 및 자원 정리
- ConcurrentHashMap 기반 연결 관리의 동시성 이슈 및 메모리 누수 가능성
- validateClubOwner의 권한 검사 시점과 예외 처리 일관성
- TransactionSynchronization 사용 위치와 이벤트 발송 시점 (트랜잭션 커밋 후 보장)
Possibly related issues
- MOA-289 (지원자의 지원서 상태를 실시간으로 공유한다): 본 PR은 SSE 엔드포인트, ApplicantStatusEvent 및 브로드캐스트 로직을 추가해 해당 이슈 목표를 구현합니다.
- [feature] MOA-289 지원자의 지원서 상태를 실시간으로 공유한다 #785: 동일한 SSE 기반 실시간 지원자 상태 공유 기능을 다루는 이슈로 직접 관련됩니다.
Possibly related PRs
- PR
#621: editApplicantDetail 경로와 DTO/엔티티 변경을 다루며, 본 PR이 해당 경로에서 ApplicantStatusEvent를 생성·발송하도록 연계합니다. - PR
#620: memo 지원 및 editApplicantDetail 구현 관련 변경과 겹치며, 본 PR은 그 편집 후 SSE 발송을 추가합니다. - PR
#622: 신청서 편집 흐름 변경과 연관되며, 상태 변경 시점 처리와 연계됩니다.
Suggested labels
📬 API
Suggested reviewers
- Zepelown
- lepitaaar
- seongwon030
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ 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 | PR의 모든 코드 변경사항(SSE 엔드포인트, 이벤트 구조, 비동기 처리 설정)이 MOA-289의 실시간 지원서 상태 공유 목표를 충족합니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 SSE 기반 실시간 상태 공유 기능 구현과 직접 관련되어 있으며, 범위를 벗어난 변경은 없습니다. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#785-sse-applicant-status-MOA-289
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.
![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 (5)
backend/src/main/java/moadong/global/config/WebConfig.java (1)
42-46: 매직 넘버를 상수로 추출하는 것을 고려하세요.타임아웃 값
30000L이ClubApplyService의SseEmitter생성 시에도 동일하게 사용됩니다. 이 값을 공통 상수로 추출하면 유지보수성이 향상됩니다.예시:
public class SseConstants { public static final long DEFAULT_TIMEOUT = 30000L; }그 후 양쪽에서 이 상수를 참조하도록 수정할 수 있습니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java (4)
480-494: 연결 성공 시 초기 이벤트를 전송하는 것을 권장합니다.SSE 연결이 성공적으로 수립되었을 때 클라이언트에게 초기 이벤트(예: "connected" 메시지)를 전송하면 연결 상태를 즉시 확인할 수 있습니다. 이는 SSE의 모범 사례입니다.
예시:
sseConnections.put(connectionKey, emitter); + try { + emitter.send(SseEmitter.event() + .name("connected") + .data("SSE connection established")); + } catch (Exception e) { + log.warn("초기 SSE 이벤트 전송 실패", e); + sseConnections.remove(connectionKey); + throw new RestApiException(ErrorCode.SSE_CONNECTION_ERROR); + } + emitter.onCompletion(() -> sseConnections.remove(connectionKey));
485-485: 타임아웃 값을 상수로 추출하는 것을 고려하세요.
30000L값이WebConfig에도 사용되고 있습니다. 공통 상수로 추출하여 일관성을 유지하세요.
497-510: SSE 이벤트에 이름을 지정하는 것을 권장합니다.라인 504에서
send(event)를 사용하고 있는데, 이벤트명을 지정하지 않으면 클라이언트에서 이벤트 타입을 구분하기 어렵습니다.SseEmitter.event()빌더를 사용하여 이벤트명을 명시하는 것이 좋습니다.다음과 같이 수정할 수 있습니다:
try { - entry.getValue().send(event); + entry.getValue().send(SseEmitter.event() + .name("status-change") + .data(event)); } catch (Exception e) {이렇게 하면 클라이언트에서
addEventListener('status-change', ...)로 특정 이벤트를 구독할 수 있습니다.
480-510: SSE 관리 로직을 별도 클래스로 분리하는 것을 고려하세요.SSE 연결 관리 로직이
ClubApplyService에 추가되면서 서비스 클래스가 비대해지고 있습니다. 단일 책임 원칙(SRP)을 준수하기 위해SseConnectionManager와 같은 별도 클래스로 SSE 관련 로직을 추출하는 것을 권장합니다.예시 구조:
@Component public class ApplicantStatusSseManager { private final Map<String, SseEmitter> connections = new ConcurrentHashMap<>(); public SseEmitter createConnection(String clubId, String applicationFormId, String userId) { ... } public void broadcast(String clubId, String applicationFormId, ApplicantStatusEvent event) { ... } private void removeConnection(String key) { ... } }그 후
ClubApplyService에서 이 매니저를 주입받아 사용하면 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
backend/src/main/java/moadong/club/controller/ClubApplyController.java(2 hunks)backend/src/main/java/moadong/club/payload/dto/ApplicantStatusEvent.java(1 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(4 hunks)backend/src/main/java/moadong/global/config/WebConfig.java(2 hunks)
🔇 Additional comments (4)
backend/src/main/java/moadong/club/payload/dto/ApplicantStatusEvent.java (1)
6-13: LGTM!불변 레코드를 사용한 이벤트 DTO 설계가 적절합니다. 필드 구성도 SSE 이벤트 페이로드로 충분합니다.
backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
139-148: LGTM!SSE 엔드포인트 구현이 올바릅니다. 인증 및 권한 검증도 적절하게 적용되어 있습니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
52-53: LGTM!
ConcurrentHashMap을 사용하여 SSE 연결을 관리하는 것은 스레드 안전성을 보장하므로 적절한 선택입니다.
322-332: LGTM!지원자 상태 변경 후 SSE 이벤트를 생성하고 전송하는 로직이 잘 구현되어 있습니다.
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
수고하셨습니다! WebSocket,SSE, Http Polling 이 두개 차이점과 왜 SSE를 사용했는지 생각해보면 좋을꺼같습니다!
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
497-511: SSE 이벤트 포맷을 명시하고 로깅을 보강하세요클라이언트 측 구독의 일관성을 위해 이벤트명을 명시하는 것을 권장합니다. 실패 로그에 key 컨텍스트를 포함하면 운영에 유용합니다.
- entry.getValue().send(event); + entry.getValue().send( + SseEmitter.event() + .name("applicant-status-changed") + .data(event) + ); - } catch (Exception e) { - log.warn("SSE 이벤트 발송 실패: {}", e.getMessage()); + } catch (Exception e) { + log.warn("SSE 이벤트 발송 실패 clubId={}, formId={}, key={}: {}", + clubId, applicationFormId, entry.getKey(), e.getMessage()); sseConnections.remove(entry.getKey()); }
52-53: 문자열 기반 키와 prefix 매칭은 취약 — 타입 안전한 키 구조 권장
clubId_applicationFormId_userId문자열 결합 +startsWith는 구분자 충돌·가독성·오타 검출 측면에서 취약합니다.record ConnectionKey(String clubId, String applicationFormId, String userId)같은 타입 안전 키 또는 중첩 맵(Map<clubId, Map<formId, Map<userId, List>>>) 구조로의 전환을 권장합니다. 최소한 고유 구분자 사용 또는String.join(delimiter, ...)로 일관성을 확보하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java(4 hunks)
🔇 Additional comments (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
484-486: ID 접근자 일관성 유지된 점 좋습니다
user.getId()사용으로 과거 불일치(getUserId) 이슈가 해소되었습니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
323-334: 트랜잭션 커밋 전 이벤트 발송 — 롤백 시 잘못된 알림 위험. AFTER_COMMIT으로 이동하세요.현재 @transactional 범위 내에서 즉시 비동기 발송됩니다. 커밋 이후에만 발송하도록 afterCommit 훅을 사용하세요. 타임스탬프도 KST로 고정 권장.
적용 diff 예시:
- ApplicantStatusEvent event = new ApplicantStatusEvent( + ApplicantStatusEvent event = new ApplicantStatusEvent( app.getId(), editRequest.status(), editRequest.memo(), - LocalDateTime.now(), + ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime(), clubId, applicationFormId ); - CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)); + // 커밋 이후에만 SSE 발송 + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)); + } + });추가 import:
import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.transaction.support.TransactionSynchronization;
481-495: 동일 key 재구독 처리 및 초기 플러시 이벤트, 타임아웃 값 개선.
- 동일 key에서 기존 emitter 덮어쓰며 기존 연결을 방치합니다 → prev.complete()로 정리.
- 초기 “connected” 이벤트를 한 번 보내 프록시 버퍼링을 깨주세요.
- 30초 타임아웃은 짧습니다(불필요한 재연결 유발) → 5분 등으로 상향 또는 설정값화.
적용 diff 예시:
- String connectionKey = clubId + "_" + applicationFormId + "_" + user.getId(); - SseEmitter emitter = new SseEmitter(30000L); // 30초 타임아웃 - - sseConnections.put(connectionKey, emitter); + String connectionKey = clubId + "_" + applicationFormId + "_" + user.getId(); + SseEmitter emitter = new SseEmitter(300_000L); // 5분 타임아웃(필요 시 설정값으로) + + SseEmitter prev = sseConnections.put(connectionKey, emitter); + if (prev != null) { + try { prev.complete(); } catch (Exception ignore) {} + } emitter.onCompletion(() -> sseConnections.remove(connectionKey)); emitter.onTimeout(() -> sseConnections.remove(connectionKey)); emitter.onError((ex) -> sseConnections.remove(connectionKey)); - - return emitter; + // 초기 핸드셰이크 이벤트로 프록시 버퍼 방지 + try { + emitter.send(SseEmitter.event().name("connected").data("ok")); + } catch (Exception e) { + sseConnections.remove(connectionKey); + emitter.completeWithError(e); + } + return emitter;
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
52-54: SSE 연결 레지스트리 구조 제안(중복 탭/브라우저 대비).단일 SseEmitter만 보관하면 동일 key 재연결 시 덮어쓰기/누수 우려가 있습니다. 선택지:
- 다중 연결 지원: Map<String, Set>로 확장.
- 단일 연결 유지: 신규 등록 시 이전 emitter를 complete()로 명시 종료. 아래 createSseConnection 코멘트의 diff 참고.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java(4 hunks)
🔇 Additional comments (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
39-41: LGTM: 필요한 import가 적절히 추가되었습니다.이전 컴파일 이슈(CompletableFuture, SseEmitter) 해소 확인했습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
482-495: 동일 키로 재연결 시 기존 Emitter가 정리되지 않아 리소스 누수가 발생합니다.같은 사용자가 동일한
connectionKey로 재구독하면 기존 emitter가 Map에서 덮어씌워지면서 참조만 사라지고 실제 연결은 정리되지 않습니다. 이로 인해 리소스 누수가 발생할 수 있습니다.다음과 같이 수정하세요:
String connectionKey = clubId + "_" + applicationFormId + "_" + user.getId(); SseEmitter emitter = new SseEmitter(300000L); // 5분 타임아웃 - sseConnections.put(connectionKey, emitter); + SseEmitter prev = sseConnections.put(connectionKey, emitter); + if (prev != null) { + try { + prev.complete(); + } catch (Exception ignored) {} + } emitter.onCompletion(() -> sseConnections.remove(connectionKey)); emitter.onTimeout(() -> sseConnections.remove(connectionKey)); emitter.onError((ex) -> sseConnections.remove(connectionKey)); + // 초기 연결 확인 이벤트 전송 (프록시 버퍼링 방지) + try { + emitter.send(SseEmitter.event().name("connected").data("ok")); + } catch (Exception e) { + sseConnections.remove(connectionKey); + emitter.completeWithError(e); + } + return emitter;
323-334: 트랜잭션 커밋 전 이벤트 전송으로 데이터 불일치 가능성이 있습니다.현재 코드는
@Transactional범위 내에서 즉시 비동기로 이벤트를 전송하므로, 트랜잭션이 롤백될 경우에도 클라이언트에게 잘못된 이벤트가 전달될 수 있습니다. 트랜잭션이 성공적으로 커밋된 이후에만 이벤트를 발송하도록 수정해야 합니다.다음과 같이 수정하세요:
+import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.support.TransactionSynchronization;app.updateMemo(editRequest.memo()); app.updateStatus(editRequest.status()); // SSE 이벤트 발송 ApplicantStatusEvent event = new ApplicantStatusEvent( app.getId(), editRequest.status(), editRequest.memo(), - LocalDateTime.now(), + ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime(), clubId, applicationFormId ); - CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)); + + // 트랜잭션 커밋 후에만 이벤트 전송 + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)); + } + }); });
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
498-517: 이벤트 발송 로직이 잘 구현되었습니다.접두어 매칭, 이벤트 메타데이터, 에러 처리가 모두 적절하게 구현되어 있습니다.
성능 개선을 위해
CompletableFuture.runAsync()의 기본ForkJoinPool.commonPool()대신 전용Executor를 주입하여 사용하는 것을 고려해보세요. 이렇게 하면 SSE 이벤트 전송이 다른 작업과 격리되어 더 안정적으로 동작할 수 있습니다.@Async("sseTaskExecutor") public void sendStatusChangeEventAsync(String clubId, String applicationFormId, ApplicantStatusEvent event) { sendStatusChangeEvent(clubId, applicationFormId, event); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/src/main/java/moadong/club/controller/ClubApplyController.java(2 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyService.javabackend/src/main/java/moadong/club/controller/ClubApplyController.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyService.javabackend/src/main/java/moadong/club/controller/ClubApplyController.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyService.java
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyService.java
🔇 Additional comments (4)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
39-41: Imports가 올바르게 추가되었습니다.SSE 기능 구현에 필요한 모든 import가 정확하게 추가되었습니다.
53-54: SSE 연결 저장소가 적절하게 선언되었습니다.
ConcurrentHashMap을 사용하여 동시성 환경에서 안전하게 SSE 연결을 관리할 수 있습니다.backend/src/main/java/moadong/club/controller/ClubApplyController.java (2)
31-31: SseEmitter import가 올바르게 추가되었습니다.SSE 엔드포인트 구현에 필요한 import가 정확하게 추가되었습니다.
139-148: SSE 엔드포인트가 올바르게 구현되었습니다.
produces="text/event-stream"속성이 명시되어 있고, 인증 및 권한 검증이 적절하게 설정되어 있으며, 서비스 계층으로의 위임도 정확합니다.다만, lepitaaar님의 이전 코멘트에서 언급하신 것처럼 SSE 연결 수가 많아질 경우 이벤트 전송 시간이 증가할 수 있습니다. 실제 운영 환경에서의 동시 연결 수를 고려하여 부하 테스트를 수행해보시는 것을 권장합니다.
Based on learnings
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/moadong/club/service/ClubApplyService.java
Outdated
Show resolved
Hide resolved
Test Results78 tests 75 ✅ 16s ⏱️ Results for commit 32f0d5a. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/src/main/java/moadong/club/controller/ClubApplyAdminController.java(2 hunks)backend/src/main/java/moadong/club/service/ClubApplyAdminService.java(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/club/controller/ClubApplyAdminController.javabackend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/controller/ClubApplyAdminController.javabackend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
🔇 Additional comments (1)
backend/src/main/java/moadong/club/controller/ClubApplyAdminController.java (1)
110-118: SSE 엔드포인트 추가 잘 반영되었습니다.
인증 검증과 서비스 위임이 간결하게 정리되어 있어 클라이언트 연동이 명확합니다.
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
237-276:CompletableFuture.runAsync()예외 처리를 추가하세요.현재 코드의
CompletableFuture.runAsync()(라인 270)는 명시적인 예외 처리가 없어서,sendStatusChangeEvent메서드 내에서 예외가 발생하거나 비동기 작업 자체에서 문제가 발생하면 조용히 실패할 수 있습니다. 메서드 내부의 try-catch만으로는 충분하지 않습니다.- CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)); + CompletableFuture.runAsync(() -> sendStatusChangeEvent(clubId, applicationFormId, event)) + .exceptionally(ex -> { + log.error("SSE 이벤트 발송 중 예외 발생: applicantId={}, error={}", app.getId(), ex.getMessage(), ex); + return null; + });또한 forEach 루프에서 각 지원자마다
TransactionSynchronization을 개별 등록하는 것이 의도된 동작인지 재검토하세요. 모든 이벤트를 단일 동기화로 묶어서 처리하는 것이 더 효율적일 수 있습니다.
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
365-371: 코드 중복: validateClubOwner 메서드를 공통 유틸리티로 추출하는 것을 권장합니다.
ClubApplyServiceV1에도 동일한validateClubOwner메서드가 존재합니다. 두 서비스 클래스에서 동일한 검증 로직을 사용하고 있으므로, 공통 유틸리티 클래스나 별도의 validator 컴포넌트로 추출하는 것을 고려해 보세요.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
🧬 Code graph analysis (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
backend/src/main/java/moadong/club/service/ClubApplyServiceV1.java (1)
Service(31-202)
🔇 Additional comments (3)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (3)
14-14: SSE 인프라가 적절히 추가되었습니다.필요한 의존성과 필드들이 올바르게 추가되었으며,
ConcurrentHashMap을 사용하여 동시성 안전성을 확보한 점이 좋습니다.Also applies to: 35-39, 45-45, 52-55
373-404: SSE 연결 관리가 올바르게 구현되었습니다.과거 리뷰에서 지적된 race condition 문제가 적절히 수정되었습니다. 기존 연결을 먼저 제거한 후 새 연결을 등록하고,
remove(key, emitter)메서드를 사용하여 동일 인스턴스일 때만 제거하도록 구현한 점이 좋습니다.
407-433: 이벤트 발송 메서드가 안전하게 구현되었습니다.스냅샷을 만들어 순회하여 동시성 문제를 방지하고, 실패 시 적절한 로그와 함께 연결을 정리하는 로직이 잘 구현되었습니다.
alsdddk
left a comment
alsdddk
left a comment
There was a problem hiding this comment.
수고하셨어요!! 비동기로 알림 구현에 대해서 저도 많이 배웠습니다
| @Transactional | ||
| public void editApplicantDetail(String applicationFormId, List<ClubApplicantEditRequest> request, CustomUserDetails user) { | ||
| String clubId = user.getClubId(); | ||
| validateClubOwner(clubId, user); |
There was a problem hiding this comment.
이미 userDetails로 받은 유저 정보에서 clubId를 가져왔기에 validateClubOwner()은 필요없긴 해용
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
44-44: 사용되지 않는 필드를 제거하는 것을 고려해보세요.
ClubRepository필드가 코드 내에서 사용되지 않는 것으로 보입니다. 이전 리뷰 코멘트에 따르면validateClubOwner가 제거되었는데, 해당 필드가 남아있는 것 같습니다.사용하지 않는다면 다음과 같이 제거할 수 있습니다:
- private final ClubRepository clubRepository; private final ClubApplicationFormsRepository clubApplicationFormsRepository;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
🧬 Code graph analysis (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
backend/src/main/java/moadong/club/service/ClubApplyServiceV1.java (1)
Service(31-202)
🔇 Additional comments (4)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (4)
34-38: SSE 구현을 위한 적절한 import입니다.SSE 연결 관리, 동시성 처리, 트랜잭션 커밋 후 이벤트 발송을 위한 import가 모두 적절하게 추가되었습니다.
50-54: SSE 연결 관리를 위한 적절한 필드 선언입니다.
ConcurrentHashMap을 사용한 thread-safe한 연결 관리와 5분 타임아웃 설정이 적절합니다.
236-274: 트랜잭션 커밋 후 SSE 이벤트 발송 로직이 잘 구현되었습니다.
TransactionSynchronization을 사용하여 DB 커밋이 성공한 후에만 SSE 이벤트를 발송하도록 구현한 것이 적절합니다. 각 지원자별로 개별 이벤트가 생성되어 세밀한 상태 업데이트가 가능합니다.
396-423: 이벤트 브로드캐스팅 로직이 안전하게 구현되었습니다.스냅샷을 사용하여 동시성 문제를 방지하고, 실패한 emitter를 적절히 정리하며, atomic removal로 race condition을 방지하고 있습니다. 에러 핸들링도 포괄적으로 구현되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
236-274: 치명적인 권한 검증 누락 - applicationFormId 소유권 미확인
editApplicantDetail메서드가applicationFormId가 사용자의 클럽에 속하는지 검증하지 않습니다. 244번 줄의findAllByIdInAndFormId는formId만 확인하고clubId는 확인하지 않습니다.악의적인 사용자가 다른 클럽의
applicationFormId를 알고 있다면, 해당 클럽의 지원자 상태와 메모를 임의로 수정할 수 있는 권한 상승 취약점이 존재합니다.동일 서비스의
getClubApplyInfo(192번 줄)처럼findByClubIdAndId를 사용하여 소유권을 검증해야 합니다.다음과 같이 메서드 시작 부분에 검증 로직을 추가하세요:
public void editApplicantDetail(String applicationFormId, List<ClubApplicantEditRequest> request, CustomUserDetails user) { String clubId = user.getClubId(); + + // applicationFormId가 user의 clubId에 속하는지 검증 + clubApplicationFormsRepository.findByClubIdAndId(clubId, applicationFormId) + .orElseThrow(() -> new RestApiException(ErrorCode.APPLICATION_NOT_FOUND)); Map<String, ClubApplicantEditRequest> requestMap = request.stream()
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
265-270: CompletableFuture의 기본 스레드 풀 사용 개선 고려268번 줄에서
CompletableFuture.runAsync()가 기본ForkJoinPool.commonPool()을 사용합니다. 이 풀은 CPU 집약적 작업에 최적화되어 있어, I/O 작업인 SSE 이벤트 발송에는 최적이 아닐 수 있습니다.이벤트 발송 빈도가 높아질 경우 전용
Executor를 주입받아 사용하는 것을 고려해보세요.예시:
@Service @AllArgsConstructor public class ClubApplyAdminService { private final Executor sseEventExecutor; // 전용 스레드 풀 주입 // ... CompletableFuture.runAsync( () -> sendStatusChangeEvent(clubId, applicationFormId, event), sseEventExecutor ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-03-19T05:18:07.818Z
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java
🧬 Code graph analysis (1)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (1)
backend/src/main/java/moadong/club/service/ClubApplyServiceV1.java (1)
Service(31-202)
🔇 Additional comments (2)
backend/src/main/java/moadong/club/service/ClubApplyAdminService.java (2)
364-398: SSE 연결 생성 로직이 안전하게 구현되었습니다이전 리뷰에서 지적된 race condition 문제가 해결되었습니다:
- 376-381번 줄: 기존 emitter를 맵에서 먼저 제거한 후 정리
- 385-387번 줄: atomic removal로 동시성 문제 방지
- 390-395번 줄: 초기 핸드셰이크 이벤트로 프록시 버퍼링 문제 해결
권한 검증(368-369번 줄)도 적절히 추가되어 보안 취약점이 해결되었습니다.
400-427: 이벤트 발송 로직이 견고하게 구현되었습니다동시성 안전성:
- 406-408번 줄: 스냅샷 생성으로
ConcurrentModificationException방지- 421번 줄: atomic removal로 race condition 방지
에러 처리:
- 419번 줄: 발송 실패 시 로깅
- 421-424번 줄: 실패한 emitter 정리 및 완료 처리
전체적으로 프로덕션 환경에 적합한 견고한 구현입니다.
4 participants
#️⃣연관된 이슈
📝작업 내용
실시간으로 지원서 상태를 확인하기 위해 SSE기능을 추가했습니다.
동기보다는 비동기가 실시간으로 데이터를 전송하는 것과
동시 연결 처리에 더 적합할 것 같아서 비동기로 했습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit