[feature] 액세스 토큰 및 리프레시 토큰의 만료 시간을 수정하고, 로그인, 관리자 계정 관련의 동시성 문제를 해결한다#713
[feature] 액세스 토큰 및 리프레시 토큰의 만료 시간을 수정하고, 로그인, 관리자 계정 관련의 동시성 문제를 해결한다#713Zepelown merged 4 commits intodevelop/befrom
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Entity: MongoDB 전환 및 낙관적 락backend/src/main/java/moadong/club/entity/Club.java, backend/src/main/java/moadong/club/entity/ClubQuestion.java |
JPA 애노테이션 제거 → Spring Data(org.springframework.data.annotation.Id)로 전환. Persistable<String> 구현, @Version Long version 추가, isNew() 구현. 일부 JPA 제약(@NotNull/@Column/@Enumerated) 제거. |
Service: 트랜잭션 경계 및 단건 편집 메서드backend/src/main/java/moadong/club/service/ClubApplyService.java, backend/src/main/java/moadong/club/service/ClubProfileService.java |
메서드 단위 @Transactional 추가. editClubApplicationQuestion(...) 메서드 신설: 질문 조회·업데이트·저장 및 QUESTION_NOT_FOUND 예외 처리. |
Scheduler: 조건부 빈 등록backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java |
@ConditionalOnProperty(name="scheduling.enabled", havingValue="true", matchIfMissing=true) 추가로 스케줄러 온/오프 가능하도록 변경. |
Config: Mongo 트랜잭션 매니저 추가backend/src/main/java/moadong/global/config/MongoConfig.java |
MongoTransactionManager 빈 등록으로 MongoDB 트랜잭션 지원 추가. |
Global: 에러코드 추가backend/src/main/java/moadong/global/exception/ErrorCode.java |
CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1", "...") 상수 추가(HTTP 409). |
Tests & Fixtures: 동시성 테스트 및 픽스처backend/src/test/java/moadong/club/service/ClubApplyServiceTest.java, backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java, backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java, backend/src/test/java/moadong/fixture/UserFixture.java |
멀티스레드 동시 수정 통합 테스트 추가(낙관적 락 및 WriteConflict 검증). 테스트 픽스처 추가 및 collectUserId 값 변경, User 생성 헬퍼 추가. |
Build: 테스트 라이브러리 추가backend/build.gradle |
io.github.artsok:rerunner-jupiter:2.1.6 테스트 의존성 추가. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant ClubApplyService
participant ClubQuestionRepo
participant MongoDB
rect rgb(245,250,255)
note right of ClubApplyService: @Transactional\nisNew() + @Version 기반 저장
Client->>ClubApplyService: editClubApplicationQuestion(questionId, user, req)
ClubApplyService->>ClubQuestionRepo: findById(questionId)
alt not found
ClubApplyService-->>Client: throw QUESTION_NOT_FOUND
else found
ClubApplyService->>ClubApplyService: updateQuestions(), setEditedAt()
ClubApplyService->>ClubQuestionRepo: save(ClubQuestion)
ClubQuestionRepo->>MongoDB: update with @Version
alt version match
MongoDB-->>ClubQuestionRepo: OK
ClubApplyService-->>Client: success
else version mismatch
MongoDB-->>ClubQuestionRepo: WriteConflict / OptimisticLock
ClubApplyService-->>Client: OptimisticLockingFailureException
end
end
end
sequenceDiagram
autonumber
participant Client
participant ClubProfileService
participant ClubRepo
participant MongoDB
rect rgb(245,250,255)
note right of ClubProfileService: @Transactional\nisNew() + @Version 기반 저장
Client->>ClubProfileService: updateClubInfo(user, req)
ClubProfileService->>ClubRepo: findByUserId(user.id)
alt not found
ClubProfileService-->>Client: throw CLUB_NOT_FOUND
else found
ClubProfileService->>ClubProfileService: apply updates
ClubProfileService->>ClubRepo: save(Club)
ClubRepo->>MongoDB: update with @Version
alt success
MongoDB-->>ClubRepo: OK
ClubProfileService-->>Client: success
else conflict
MongoDB-->>ClubRepo: WriteConflict / OptimisticLock
ClubProfileService-->>Client: OptimisticLockingFailureException
end
end
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 토큰 만료 기간 수정 (MOA-204) | ❌ | 관련 코드(토큰 만료 설정/로그인 로직)가 PR에 없음. |
| 로그인 관련 동시성 문제 해결 (MOA-204) | ❌ | 인증/토큰 발급 흐름이나 관리자 동시성 제어 변경이 없음. |
| 관리자 계정 사용 시 동시성 문제 해결 (MOA-204) | ❌ | 관리자 계정 관련 코드 변경이 없음. |
| 도메인 수준 동시성 제어 도입(클럽 엔티티) (MOA-204) | ✅ | Club/ClubQuestion에 @Version·트랜잭션·테스트가 추가되어 도메인 낙관적 락이 도입됨. |
Assessment against linked issues: Out-of-scope changes
| Code Change | Explanation |
|---|---|
MongoTransactionManager 추가 (backend/src/main/java/moadong/global/config/MongoConfig.java) |
MOA-204의 토큰 만료·로그인·관리자 동시성 목표와 직접 연관된 변경이 아님. |
Club/ClubQuestion → MongoDB 전환 및 @Version 도입 (backend/src/main/java/moadong/club/entity/Club.java, .../ClubQuestion.java) |
도메인 영속성 및 락 방식 변경으로, 이슈의 토큰/인증 목표와 일치하지 않음. |
단건 질문 편집 메서드 추가 (backend/src/main/java/moadong/club/service/ClubApplyService.java) |
로그인/토큰 만료 관련 요구사항과 무관한 기능 추가. |
Possibly related PRs
- [fix] 스케줄러는 db를 참조하여 모집현황을 갱신한다 #401 — RecruitmentStateChecker 관련 변경(스케줄러 도입/수정)이 겹칠 가능성 있음.
- [feature] cover image 업로드 및 삭제 기능 구현 #537 — Club 엔티티 변경(필드/검증)과 충돌 가능성이 높음.
- [refactor] club api 리팩토링 #492 — Club 엔티티의 영속성/버전 관련 변경과 직접적 연관성 있음.
Suggested labels
✨ Feature, ✅ Test
Suggested reviewers
- lepitaaar
- PororoAndFriends
- seongwon030
- oesnuj
✨ Finishing Touches
- 📝 Generate Docstrings
🧪 Generate unit tests
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#708-token-MOA-204
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.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. - PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
CodeRabbit Commands (Invoked using PR/Issue comments)
Type @coderabbitai help to get the list of available commands.
Other keywords and placeholders
- Add
@coderabbitai ignoreor@coderabbit ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
Status, Documentation and Community
- Visit our Status Page to check the current availability of CodeRabbit.
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Test Results80 tests 77 ✅ 7s ⏱️ Results for commit e4b6597. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (1)
25-36: NPE 가능성과 불필요 저장을 방지하는 가드 추가 제안
club.getClubRecruitmentInformation()가 null일 경우 NPE 위험.ALWAYS인 경우 날짜 계산이 불필요하게 수행됨.- 상태 변경이 없을 때도 매번
save호출됨(쓰기 부하 및 Version 증가 가능성).아래와 같이 가드 및 변경 시에만 저장하도록 개선을 제안합니다.
- for (Club club : clubs) { - ClubRecruitmentInformation recruitInfo = club.getClubRecruitmentInformation(); - ZonedDateTime recruitmentStartDate = recruitInfo.getRecruitmentStart(); - ZonedDateTime recruitmentEndDate = recruitInfo.getRecruitmentEnd(); - if (recruitInfo.getClubRecruitmentStatus() == ClubRecruitmentStatus.ALWAYS) { - continue; - } - RecruitmentStateCalculator.calculate(club, recruitmentStartDate, recruitmentEndDate); - clubRepository.save(club); - } + for (Club club : clubs) { + ClubRecruitmentInformation recruitInfo = club.getClubRecruitmentInformation(); + if (recruitInfo == null) { + continue; // 필요 시 로그 추가 권장 + } + if (recruitInfo.getClubRecruitmentStatus() == ClubRecruitmentStatus.ALWAYS) { + continue; + } + + ZonedDateTime recruitmentStartDate = recruitInfo.getRecruitmentStart(); + ZonedDateTime recruitmentEndDate = recruitInfo.getRecruitmentEnd(); + + ClubRecruitmentStatus before = recruitInfo.getClubRecruitmentStatus(); + RecruitmentStateCalculator.calculate(club, recruitmentStartDate, recruitmentEndDate); + if (before != recruitInfo.getClubRecruitmentStatus()) { + clubRepository.save(club); + } + }참고: 계산 결과가 상태 외 필드도 변경한다면,
calculate가 변경 여부(boolean)를 반환하도록 리팩터링해 그 값으로 저장 여부를 결정하는 방식을 권장합니다.backend/src/main/java/moadong/club/entity/ClubQuestion.java (1)
3-3: MongoDB용 @id 어노테이션 잘못 사용 — 매핑 실패 가능성 높음
jakarta.persistence.Id는 JPA용입니다. Spring Data MongoDB에선org.springframework.data.annotation.Id를 써야_id매핑이 작동합니다. 현재 상태에선 ID가 제대로 매핑되지 않아 저장/조회 시 예기치 않은 필드로 저장될 수 있습니다.-import jakarta.persistence.Id; +import org.springframework.data.annotation.Id;backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
3-3: 트랜잭션 어노테이션 통일: Spring @transactional 사용 권장MongoTransactionManager와의 일관성을 위해 jakarta.transaction.Transactional 대신 org.springframework.transaction.annotation.Transactional 사용을 권장합니다.
-import jakarta.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;
♻️ Duplicate comments (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
270-303: create/update 질문 구성 로직 중복 — 공용 메서드로 추출 제안이전 리뷰 맥락(중복은 향후 리팩터 예정)을 알고 있으나, 이제 낙관적 락 도입으로 변경 지점이 늘어난 만큼, 변환 로직을 하나로 합치면 유지보수 리스크가 줄어듭니다.
원하시면 공용 빌더/매퍼 메서드 초안까지 제공하겠습니다.
🧹 Nitpick comments (13)
backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (3)
18-18: 스케줄 토글 도입은 적절합니다. 프로퍼티 키를 더 구체화하는 것을 권장합니다.환경 전역 충돌/오용을 줄이려면
scheduling.enabled대신 도메인에 특화된 키(예:moadong.scheduling.recruitment-state.enabled)를 권장합니다. 기본값 ON(matchIfMissing = true) 의도가 운영/테스트 모두에서 맞는지도 확인 부탁드립니다.적용 예시:
-@ConditionalOnProperty(name = "scheduling.enabled", havingValue = "true", matchIfMissing = true) +@ConditionalOnProperty(name = "moadong.scheduling.recruitment-state.enabled", havingValue = "true", matchIfMissing = true)
25-36: 대량 데이터 고려 시 findAll() 대신 페이지네이션/스트리밍 권장스케줄러가 전체 컬렉션을 매 시간 스캔하면 메모리/DB 부하가 커질 수 있습니다.
findAll(Pageable)로 배치 처리하거나 커서 기반 스트리밍을 검토해주세요.예시:
Pageable pageable = PageRequest.of(0, 500); Page<Club> page; do { page = clubRepository.findAll(pageable); for (Club club : page.getContent()) { // 동일 로직 } pageable = page.nextPageable(); } while (page.hasNext());
16-19: 멀티 인스턴스(멀티 파드) 환경에서 단일 실행 보장여러 인스턴스에서 동시에 스케줄이 돌 수 있습니다. 분산 락(ShedLock 등)으로 단일 실행을 보장하는 것을 권장합니다.
예시(개념):
//@SchedulerLock(name = "RecruitmentStateChecker.performTask", lockAtLeastFor = "PT1M", lockAtMostFor = "PT10M")backend/src/main/java/moadong/global/config/MongoConfig.java (1)
1-14: Mongo 트랜잭션 전제 조건 알림(운영 설정)Mongo 트랜잭션은 replica set/샤드에서만 동작합니다. 개발/운영 모두 해당 구성인지 확인하고, 단일 인스턴스라면 트랜잭션이 무시되니 주의하세요.
backend/src/main/java/moadong/club/entity/ClubQuestion.java (1)
9-11: 불필요/미사용 임포트 정리
@Transient는 사용되지 않습니다. 정리해 주세요. 또한@NotBlank임포트가 존재한다면 사용하지 않으면 제거 권장.backend/src/main/java/moadong/club/entity/Club.java (1)
13-14: 미사용 임포트 정리
org.springframework.data.annotation.Transient는 사용되지 않습니다.backend/src/test/java/moadong/fixture/UserFixture.java (1)
23-31: createUserDetails 내 ID 불일치 가능성
createUserDetails(String userId)에서User.id는 파라미터를 쓰지만,UserInformation은collectUserId를 사용합니다. 인증/인가 테스트에서 사용자 식별자 혼선이 생길 수 있으니 동일 값을 쓰도록 정렬을 권장합니다.- .userInformation(new UserInformation(collectUserId, collectPhoneNumber)) + .userInformation(new UserInformation(userId, collectPhoneNumber))backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java (2)
3-7: 불필요한 import 제거로 빌드/경고 정리ClubQuestionOption, ClubApplicationQuestionType, ClubApplyQuestion를 사용하지 않습니다. 제거해주세요.
-import moadong.club.entity.ClubQuestionOption; -import moadong.club.enums.ClubApplicationQuestionType; -import moadong.club.payload.request.ClubApplyQuestion;
8-9: List 유틸 사용을 위해 java.util.List import 추가 권장아래 제안(diff)처럼 List.of를 쓰려면 List import가 필요합니다.
-import java.util.ArrayList; +import java.util.ArrayList; +import java.util.List;backend/src/main/java/moadong/club/service/ClubProfileService.java (1)
41-52: updateClubRecruitmentInfo도 트랜잭션 경계 일관화 권장동일 문서 업데이트(save) 흐름이므로 @transactional 부여로 일관성을 맞추는 것을 권장합니다.
- public void updateClubRecruitmentInfo(ClubRecruitmentInfoUpdateRequest request, + @Transactional + public void updateClubRecruitmentInfo(ClubRecruitmentInfoUpdateRequest request, CustomUserDetails user) {backend/src/test/java/moadong/club/service/ClubApplyServiceTest.java (1)
65-88: 시작 동기화 추가로 경합 유도 강화 제안실제 경합을 보장하려면 startLatch 또는 CyclicBarrier로 동일 시점 시작을 유도하는 것이 좋습니다.
- for (int i = 0; i < numberOfThreads; i++) { + CountDownLatch startLatch = new CountDownLatch(1); + for (int i = 0; i < numberOfThreads; i++) { executorService.submit(() -> { try { + startLatch.await(); // 모든 작업 동일 시점 시작 ClubApplicationEditRequest request = ClubApplicationEditFixture.createClubApplicationEditRequest(); // 3. 조회해 둔 clubQuestion의 ID를 명확히 전달 clubApplyService.editClubApplicationQuestion(this.clubQuestion.getId(), userDetails, request); successCount.incrementAndGet(); } catch (OptimisticLockingFailureException e) { conflictCount.incrementAndGet(); } catch (DataAccessException e) { if (e.getMessage() != null && e.getMessage().contains("WriteConflict")) { conflictCount.incrementAndGet(); } else { e.printStackTrace(); } } catch (Exception e) { e.printStackTrace(); } finally { latch.countDown(); } }); } + startLatch.countDown();backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (2)
17-18: 미사용 import 제거PasswordEncoder를 사용하지 않습니다. 정리해주세요.
-import org.springframework.security.crypto.password.PasswordEncoder;
52-74: 경합 유도를 위한 시작 동기화 고려동시 시작을 보장하면 충돌 재현률이 높아집니다.
- for (int i = 0; i < numberOfThreads; i++) { + CountDownLatch startLatch = new CountDownLatch(1); + for (int i = 0; i < numberOfThreads; i++) { executorService.submit(() -> { try { + startLatch.await(); ClubInfoRequest request = ClubRequestFixture.createValidClubInfoRequest(); clubProfileService.updateClubInfo(request, userDetails); successCount.incrementAndGet(); } catch (OptimisticLockingFailureException e) { conflictCount.incrementAndGet(); } catch (DataAccessException e) { // WriteConflict 후 재시도 끝에 OptimisticLock으로 변환되지 못한 경우 // 이 경우도 충돌로 간주할 수 있음 - if (e.getMessage().contains("WriteConflict")) { + String msg = e.getMessage(); + if (msg != null && msg.contains("WriteConflict")) { conflictCount.incrementAndGet(); } else { e.printStackTrace(); } } catch (Exception e) { e.printStackTrace(); } finally { latch.countDown(); } }); } + startLatch.countDown();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
backend/src/main/java/moadong/club/entity/Club.java(4 hunks)backend/src/main/java/moadong/club/entity/ClubQuestion.java(4 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(1 hunks)backend/src/main/java/moadong/club/service/ClubProfileService.java(2 hunks)backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java(2 hunks)backend/src/main/java/moadong/global/config/MongoConfig.java(1 hunks)backend/src/main/java/moadong/global/exception/ErrorCode.java(1 hunks)backend/src/test/java/moadong/club/service/ClubApplyServiceTest.java(1 hunks)backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java(1 hunks)backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java(1 hunks)backend/src/test/java/moadong/fixture/UserFixture.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
PR: Moadong/moadong#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/test/java/moadong/fixture/ClubApplicationEditFixture.javabackend/src/test/java/moadong/club/service/ClubApplyServiceTest.javabackend/src/main/java/moadong/club/service/ClubApplyService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
PR: Moadong/moadong#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.javabackend/src/main/java/moadong/club/entity/ClubQuestion.java
🧬 Code graph analysis (4)
backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java (1)
backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
ClubRequestFixture(10-51)
backend/src/test/java/moadong/club/service/ClubApplyServiceTest.java (3)
backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java (1)
ClubApplicationEditFixture(10-26)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(9-33)backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (1)
IntegrationTest(26-82)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (2)
backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
ClubRequestFixture(10-51)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(9-33)
backend/src/main/java/moadong/club/service/ClubProfileService.java (1)
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
Service(27-317)
🪛 GitHub Actions: PR Test
backend/src/main/java/moadong/club/service/ClubProfileService.java
[error] 19-19: package org.springframework.retry.annotation does not exist (import org.springframework.retry.annotation.Backoff).
[error] 20-20: package org.springframework.retry.annotation does not exist (import org.springframework.retry.annotation.Retryable).
🔇 Additional comments (5)
backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (1)
23-24: 주석과 코드의 실행 주기 불일치 확인 필요
현재@Scheduled(fixedRate = 60 * 60 * 1000)은 1시간(3600000ms)마다 실행되고 있지만, 주석에는 “5분마다 실행”으로 되어 있습니다.
의도한 실행 주기(5분 vs. 1시간)를 결정한 뒤 코드와 주석을 일치시키고, 운영·개발 환경별 유연성을 위해 아래 중 한 가지를 적용하세요.
- 옵션 A: 5분이 의도라면
- @Scheduled(fixedRate = 60 * 60 * 1000) // 5분마다 실행 + @Scheduled(fixedRate = 5 * 60 * 1000) // 5분마다 실행- 옵션 B: 외부 프로퍼티화(기본 1시간)
- @Scheduled(fixedRate = 60 * 60 * 1000) // 5분마다 실행 + @Scheduled(fixedRateString = "${moadong.scheduling.recruitment-state.fixed-rate-ms:3600000}") // 기본 1시간backend/src/main/java/moadong/global/config/MongoConfig.java (1)
10-13: 프로젝트에서 DataSourceAutoConfiguration을 제외하여 JPA/DataSource 트랜잭션 매니저가 없으므로, MongoTransactionManager 빈 이름 충돌 우려가 없습니다. 네이밍 변경은 불필요합니다.Likely an incorrect or invalid review comment.
backend/src/test/java/moadong/fixture/UserFixture.java (2)
19-21: 오버로드된 createUser 편의 메서드 추가 👍패스워드 인코더 의존만으로 기본 유저를 만들 수 있어 테스트 가독성이 좋아집니다.
10-10: 하드코딩된 “test12345” 사용 사례 없음
코드베이스 전반에"test12345"가 직접 참조된 곳이 없어 변경에 따른 테스트 영향이 없습니다.backend/src/test/java/moadong/fixture/ClubApplicationEditFixture.java (1)
20-24: 빈 questions 전달 시 질문 전체 삭제 위험
ClubApplyService.updateQuestions는 요청 questions로 기존 데이터를 대체합니다. 빈 리스트 전달 시 모든 질문이 삭제될 수 있으므로 최소 1개 더미 질문을 포함하거나 이 픽스처가 통합 테스트에서 재사용되지 않는지 확인하세요.
| @Version | ||
| private Long version; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Persistable.isNew 구현 개선 — 버전 기반으로 신규 여부 판정
낙관적 락(@Version)을 도입했으므로 신규 여부를 id == null이 아닌 version == null로 판정하면, 사전 할당 ID(insert with assigned id) 시에도 정상적으로 insert 흐름을 탈 수 있습니다.
- @Override
- public boolean isNew() {
- return this.id == null;
- }
+ @Override
+ public boolean isNew() {
+ return this.version == null;
+ }Also applies to: 68-71
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/entity/ClubQuestion.java around lines
48-50 (and also apply same change to lines 68-71), the current Persistable.isNew
implementation uses id == null to decide newness; change it to use version ==
null instead so entities with pre-assigned IDs but no persisted version are
treated as new. Update the isNew() method to return true when version is null
and false otherwise, ensuring optimistic locking via @Version drives insert vs
update behavior.
| @Transactional | ||
| public void editClubApplicationQuestion(String questionId, CustomUserDetails user, ClubApplicationEditRequest request) { | ||
| ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND)); | ||
|
|
||
| updateQuestions(clubQuestion, request); | ||
| clubQuestion.updateEditedAt(); | ||
|
|
||
| clubQuestionRepository.save(clubQuestion); | ||
| } |
There was a problem hiding this comment.
권한 체크 부재: questionId만으로 수정 가능 — 보안 취약점
editClubApplicationQuestion는 소유자 검증 없이 questionId만 알면 수정 가능합니다. clubId 기반으로 소유자(user.id == club.userId) 검증을 추가하세요.
@Transactional
public void editClubApplicationQuestion(String questionId, CustomUserDetails user, ClubApplicationEditRequest request) {
- ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId)
- .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND));
+ ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId)
+ .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND));
+ // 소유자 검증 추가
+ Club club = clubRepository.findById(clubQuestion.getClubId())
+ .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND));
+ if (!user.getId().equals(club.getUserId())) {
+ throw new RestApiException(ErrorCode.USER_UNAUTHORIZED);
+ }
- updateQuestions(clubQuestion, request);
+ updateQuestions(clubQuestion, request);
clubQuestion.updateEditedAt();
clubQuestionRepository.save(clubQuestion);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Transactional | |
| public void editClubApplicationQuestion(String questionId, CustomUserDetails user, ClubApplicationEditRequest request) { | |
| ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND)); | |
| updateQuestions(clubQuestion, request); | |
| clubQuestion.updateEditedAt(); | |
| clubQuestionRepository.save(clubQuestion); | |
| } | |
| @Transactional | |
| public void editClubApplicationQuestion(String questionId, CustomUserDetails user, ClubApplicationEditRequest request) { | |
| ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND)); | |
| // 소유자 검증 추가 | |
| Club club = clubRepository.findById(clubQuestion.getClubId()) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (!user.getId().equals(club.getUserId())) { | |
| throw new RestApiException(ErrorCode.USER_UNAUTHORIZED); | |
| } | |
| updateQuestions(clubQuestion, request); | |
| clubQuestion.updateEditedAt(); | |
| clubQuestionRepository.save(clubQuestion); | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/service/ClubApplyService.java around lines
48 to 57, the editClubApplicationQuestion method currently allows updates with
only questionId and lacks ownership checks; update it to verify that the
authenticated user is the owner of the club before modifying the question. After
loading ClubQuestion, fetch the associated Club (by clubQuestion.getClubId() or
relevant relation) and compare club.getUserId() (or club.getOwnerId()) to
user.getId(); if they do not match, throw a RestApiException with an appropriate
ErrorCode (e.g., UNAUTHORIZED or FORBIDDEN). Only proceed to updateQuestions,
updateEditedAt, and save the question when the ownership check passes.
There was a problem hiding this comment.
@lepitaaar 안녕하세요!
이 코드는 테스트 코드가 아니라 실제 프로덕션 서비스 코드입니다. 현재 보고 계신 파일은 backend/src/main/java/moadong/club/service/ClubApplyService.java로, 클럽 지원서 관련 비즈니스 로직을 담당하는 서비스 클래스입니다.
새로 추가된 editClubApplicationQuestion 메서드는 questionId를 통해 직접 클럽 지원서 질문을 수정하는 기능인데, CodeRabbit이 지적한 보안 취약점(권한 체크 부재)이 실제로 존재합니다. 현재 코드는 questionId만 알면 누구나 해당 질문을 수정할 수 있어서, 클럽 소유자 검증 로직이 추가되어야 합니다.
테스트 코드는 PR에서 ClubApplyServiceTest.java와 ClubProfileServiceTest.java 파일들이 별도로 추가되었습니다.
🧠 Learnings used
Learnt from: lepitaaar
PR: Moadong/moadong#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.
Learnt from: lepitaaar
PR: Moadong/moadong#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.
Learnt from: seongwon030
PR: Moadong/moadong#195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
backend/src/main/java/moadong/club/service/ClubProfileService.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Getter | ||
| public enum ErrorCode { | ||
| CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1","다른 사용자가 먼저 수정했습니다. 페이지를 새로고침 후 다시 이용해주세요"), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
409 충돌 코드 추가는 적절합니다만, 메시지는 UI 지시보다는 서버-중립적 문구를 권장
API 오류 메시지에 “페이지를 새로고침”은 클라이언트 UI 전제라 모바일/서버 사용처에서 어색할 수 있습니다. 중립적으로 바꾸는 걸 제안드립니다.
- CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1","다른 사용자가 먼저 수정했습니다. 페이지를 새로고침 후 다시 이용해주세요"),
+ CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1","다른 사용자가 먼저 수정했습니다. 최신 상태를 불러온 뒤 다시 시도해주세요"),또한 전역 예외 처리기에서 OptimisticLockingFailureException, Mongo WriteConflict(112) 등을 이 코드로 매핑되는지 확인 부탁드립니다.
다음 스크립트로 매핑 존재 여부를 점검할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# 전역 예외 처리기에서 낙관적 락/WriteConflict 매핑 확인
rg -nP -C2 '@ExceptionHandler\((OptimisticLockingFailureException|DataAccessException|MongoException)' --glob '!**/build/**'
rg -nP -C2 'ErrorCode\.CONCURRENCY_CONFLICT|HttpStatus\.CONFLICT' --glob '!**/build/**'Length of output: 826
CONCURRENCY_CONFLICT 메시지 중립화 및 예외 매핑 추가 필요
- backend/src/main/java/moadong/global/exception/ErrorCode.java: 메시지를 UI 지시 제거한 서버-중립 문구로 변경 권장
- CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1","다른 사용자가 먼저 수정했습니다. 페이지를 새로고침 후 다시 이용해주세요"),
+ CONCURRENCY_CONFLICT(HttpStatus.CONFLICT, "100-1","다른 사용자가 먼저 수정했습니다. 최신 상태를 불러온 뒤 다시 시도해주세요"),- 전역 예외 처리기(GlobalExceptionHandler 등)에 OptimisticLockingFailureException 및 Mongo WriteConflict 예외를 CONCURRENCY_CONFLICT로 매핑하는
@ExceptionHandler핸들러 추가 필요
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/global/exception/ErrorCode.java around line 8,
update the CONCURRENCY_CONFLICT message to a server-neutral phrase (e.g.,
"Concurrent modification detected. Please retry the operation.") instead of
UI-directed text; then in the global exception handler (e.g.,
GlobalExceptionHandler) add an @ExceptionHandler method that catches
OptimisticLockingFailureException and the MongoDB write-conflict exception class
you use (e.g., MongoWriteConcernException or MongoWriteConflictException) and
converts them into a response using ErrorCode.CONCURRENCY_CONFLICT (set HTTP
status CONFLICT and the standardized error body), ensuring both exceptions map
to the same ErrorCode for consistent client handling.
| int numberOfThreads = 1; | ||
| ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads); | ||
| CountDownLatch latch = new CountDownLatch(numberOfThreads); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시성 테스트가 실질적 동시성을 검증하지 않음 (스레드 수 1)
테스트명/서술과 불일치합니다. 최소 2 스레드 이상으로 수정하세요.
- int numberOfThreads = 1;
+ int numberOfThreads = 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int numberOfThreads = 1; | |
| ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads); | |
| CountDownLatch latch = new CountDownLatch(numberOfThreads); | |
| int numberOfThreads = 2; | |
| ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads); | |
| CountDownLatch latch = new CountDownLatch(numberOfThreads); |
🤖 Prompt for AI Agents
In backend/src/test/java/moadong/club/service/ClubApplyServiceTest.java around
lines 58 to 61, the concurrency test sets numberOfThreads to 1 which does not
exercise real concurrency and contradicts the test description; change
numberOfThreads to at least 2 (or a higher realistic value) and initialize the
ExecutorService and CountDownLatch based on that value (e.g.,
newFixedThreadPool(numberOfThreads) and new CountDownLatch(numberOfThreads)) so
the test actually runs multiple threads concurrently.
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java
Outdated
Show resolved
Hide resolved
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
낙관적 락 구현 수고하셨습니다. 몽고디비에 대해 좀더 공부해봐야겠네요
| @Field("recruitmentInformation") | ||
| private ClubRecruitmentInformation clubRecruitmentInformation; | ||
|
|
||
| @org.springframework.data.annotation.Version |
There was a problem hiding this comment.
어노테이션 패키지 경로대신 임포트하셔야할꺼같습니다
There was a problem hiding this comment.
또한 버전 어노테이션이 어떤역할을 하는지 궁금해요!
There was a problem hiding this comment.
버전 어노테이션은 이제 엔티티를 변경할 때, 기존에 가져왔던 값이 어떤 버전인지를 나타내는 용도로 사용합니다.
만약 내가 수정한 엔티티가 db의 버전과 다르다면 이건 옛날 버전임을 뜻하기에 변경하지 않습니다.
원래라면 이제 sql문에 조건을 걸어야 하지만 이를 어노테이션으로 대체하는 거죠.
| @AllArgsConstructor | ||
| @Getter | ||
| public class Club { | ||
| public class Club implements Persistable<String> { |
| @Transactional | ||
| public void editClubApplicationQuestion(String questionId, CustomUserDetails user, ClubApplicationEditRequest request) { | ||
| ClubQuestion clubQuestion = clubQuestionRepository.findById(questionId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.QUESTION_NOT_FOUND)); | ||
|
|
||
| updateQuestions(clubQuestion, request); | ||
| clubQuestion.updateEditedAt(); | ||
|
|
||
| clubQuestionRepository.save(clubQuestion); | ||
| } |
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| @ConditionalOnProperty(name = "scheduling.enabled", havingValue = "true", matchIfMissing = true) |
There was a problem hiding this comment.
ConditionalOnProperty을 단게 성능과 관련이 있나요?
There was a problem hiding this comment.
이건 이제 테스트를 돌릴 때, 스케줄러가 작동해서 버전이 무지성으로 올라가는 문제가 있었습니다.
이를 위해서 application.properties에 scheduling.enabled: false 같이 입력하면 스케줄러가 작동하지 않도록 할 수 있습니다.
| @Configuration | ||
| public class MongoConfig { | ||
| @Bean | ||
| public MongoTransactionManager transactionManager(MongoDatabaseFactory dbFactory) { |
There was a problem hiding this comment.
몽고 db에서 트랜잭션을 사용하기위해 매니저를 등록해야하군요
There was a problem hiding this comment.
Transactional 이 실제로 작동하기 위해선 몽고디비 샤딩 과정을 거쳐야하는데 이는 altas를 제대로 확인해봐야합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/main/java/moadong/club/entity/Club.java (1)
123-126: isNew를 version 기반으로 판정 — 이전 코멘트 반영 완료
return this.version == null;로 수정된 점 확인했습니다. 운영 컬렉션의 기존 문서에version(예: 0) 선마이그레이션이 없다면 첫 저장 시 insert 경로로 빠져 충돌할 수 있으므로, 배포 전 마이그레이션이 완료되었는지 확인해 주세요.
🧹 Nitpick comments (1)
backend/src/main/java/moadong/club/entity/Club.java (1)
44-46: 낙관적 락 버전 필드 추가 👍 — API 응답 노출 방지를 위해 @JsonIgnore 권장엔티티를 직접 직렬화해 반환한다면
version은 내부 동시성 메타데이터이므로 숨기는 편이 안전합니다.적용 예시:
- @Version + @JsonIgnore + @Version private Long version;(임포트는 아래와 같이 추가)
+import com.fasterxml.jackson.annotation.JsonIgnore;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
backend/src/main/java/moadong/club/entity/Club.java(4 hunks)backend/src/main/java/moadong/club/entity/ClubQuestion.java(4 hunks)backend/src/main/java/moadong/club/service/ClubProfileService.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/main/java/moadong/club/service/ClubProfileService.java
- backend/src/main/java/moadong/club/entity/ClubQuestion.java
🔇 Additional comments (1)
backend/src/main/java/moadong/club/entity/Club.java (1)
24-24: 빌드/생성자 사용 신규 Club 인스턴스에서 version 보존 여부 확인 필요
UserCommandService.createClub의new Club(userId)와 기타Club.builder()경유 생성 지점에서 엔티티 매핑 시version필드가 누락되어 의도치 않은 insert 경로로 빠지지 않는지 검토해 주세요.
| import org.springframework.data.annotation.Id; | ||
| import org.springframework.data.annotation.Version; | ||
| import org.springframework.data.domain.Persistable; | ||
| import org.springframework.data.mongodb.core.mapping.Document; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Spring Data Mongo 전환 OK — JPA 어노테이션 잔존 여부만 점검해 주세요
org.springframework.data.* 임포트로의 전환이 적절합니다. 혹시 남아 있을 수 있는 jakarta/javax.persistence 및 @Entity 계열 어노테이션 혼용을 한 번 스캔해 주세요.
다음 스크립트로 리포지토리 전역의 혼용 여부를 빠르게 확인할 수 있습니다.
또한 버전 필드를 API 응답에 노출하지 않으려면 @JsonIgnore 임포트를 추가해 둘 것을 권장합니다:
+import com.fasterxml.jackson.annotation.JsonIgnore;🏁 Script executed:
#!/bin/bash
set -euo pipefail
# JPA 흔적 탐지
rg -nP -C2 '(jakarta|javax)\.persistence|@Entity\b|@Table\b|@Enumerated\b' \
-g '!**/target/**' -g '!**/build/**' || true
# Spring Data(Mongo) 사용 흔적
rg -nP -C2 'org\.springframework\.data\.(annotation|mongodb)' \
-g '!**/target/**' -g '!**/build/**' || trueLength of output: 15134
JPA 어노테이션 전역 제거 및 Club.version @JsonIgnore 적용
- 다수 엔티티 클래스(예: User, UserInformation, ClubQuestion, ClubRecruitmentInformation, ClubMetric, ClubApplicationQuestion, ClubApplication 등)에
jakarta.persistence.*import 및@Entity,@Enumerated등이 아직 남아 있습니다. 레거시 JPA 어노테이션을 모두 제거하세요. backend/src/main/java/moadong/club/entity/Club.java의version필드가 API 응답에 노출되지 않도록@JsonIgnore를 추가하세요. (import com.fasterxml.jackson.annotation.JsonIgnore;포함)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (1)
66-68: NPE 가드 반영 OK (과거 코멘트 해소).getMessage() null 체크가 추가되어 NPE 위험이 제거되었습니다.
🧹 Nitpick comments (6)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (6)
66-71: 예외 식별을 메시지 문자열에 의존하지 말고 원인 예외로 판별하세요.“WriteConflict” 문자열 매칭은 취약합니다. getMostSpecificCause()를 함께 확인하고, 예상 외 DataAccessException은 테스트를 실패시키는 편이 원인 분석에 유리합니다.
아래처럼 보완을 제안합니다:
- } catch (DataAccessException e) { - if (e.getMessage() != null && e.getMessage().contains("WriteConflict")) { - conflictCount.incrementAndGet(); - } else { - e.printStackTrace(); - } - } catch (InterruptedException | BrokenBarrierException e) { + } catch (DataAccessException e) { + Throwable cause = e.getMostSpecificCause(); + String msg = e.getMessage(); + if ((msg != null && msg.contains("WriteConflict")) + || (cause != null && cause.getMessage() != null && cause.getMessage().contains("WriteConflict"))) { + conflictCount.incrementAndGet(); + } else { + fail("예상치 못한 DataAccessException: " + e); + } + } catch (InterruptedException | BrokenBarrierException e) { Thread.currentThread().interrupt(); - e.printStackTrace(); - } catch (Exception e) { - e.printStackTrace(); + fail("동기화 중단/장애: " + e.getMessage()); + } catch (Exception e) { + fail("예상치 못한 예외: " + e); }
30-34: Optional.get() 직접 호출 지양 — 실패 원인 메시지 명확화.존재하지 않을 경우를 대비해 orElseThrow로 명확한 실패 메시지를 주세요.
- User user = userRepository.findUserByUserId(UserFixture.collectUserId).get(); + User user = userRepository.findUserByUserId(UserFixture.collectUserId) + .orElseThrow(() -> new IllegalStateException("테스트 사용자(" + UserFixture.collectUserId + ")가 존재하지 않습니다."));
82-84: 타임아웃과 종료 대기 추가로 행(hang) 방지.latch 무기한 대기는 CI 불안정 원인입니다. 타임아웃과 awaitTermination을 추가하세요.
- latch.await(); // 모든 스레드가 작업을 마칠 때까지 대기 - executorService.shutdown(); + boolean completed = latch.await(10, TimeUnit.SECONDS); // 최대 10초 대기 + if (!completed) { + executorService.shutdownNow(); + fail("작업이 10초 내에 종료되지 않았습니다."); + } + executorService.shutdown(); + executorService.awaitTermination(5, TimeUnit.SECONDS);
85-88: 모든 요청이 집계되었는지 보조 단언 추가.성공+충돌 카운트가 총 스레드 수와 일치하는지 검증하면 진단이 쉬워집니다.
assertEquals(1, successCount.get(), "성공한 요청은 1개여야 합니다."); assertEquals(numberOfThreads - 1, conflictCount.get(), "실패(충돌)한 요청은 " + (numberOfThreads - 1) + "개여야 합니다."); + assertEquals(numberOfThreads, successCount.get() + conflictCount.get(), "모든 요청의 결과가 집계되어야 합니다.");
20-20: fail 등을 사용하려면 정적 임포트 범위를 확장하세요.assertEquals 단독 임포트 대신 와일드카드를 사용하면 fail, assertTrue 등 호출이 간단해집니다.
-import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*;
49-55: Future 수집으로 스레드 내 예외를 테스트 스레드로 전파하세요.submit 반환 Future를 수집해 get()으로 예외를 재throw하면 숨겨진 실패를 줄일 수 있습니다. (또는 invokeAll 사용)
원하시면 invokeAll 기반으로 간결한 리팩터링 패치를 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (2)
backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
ClubRequestFixture(10-51)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(9-33)
🪛 GitHub Actions: PR Test
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java
[error] 86-86: Assertion failed in optimistic_lock_multi_thread_test: expected 1 but was 2. (성공한 요청은 1개여야 합니다. ==> expected: <1> but was: <2>)
[error] 1-1: Gradle test task failed. 1 test failed. See the test report: file:///home/runner/work/moadong/moadong/backend/build/reports/tests/test/index.html
🔇 Additional comments (1)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (1)
85-88: 테스트는 낙관적 락(@Version)으로 정확히 1개의 성공만 보장합니다.
Club 엔티티에@Version이 적용되어 있고, barrier 이후 모든 스레드가 동일한 버전을 읽어save()시 버전 불일치로 1회만 성공 후 나머지는OptimisticLockingFailureException이 발생하므로, 비결정성 문제 없이 “정확히 1개 성공”이 보장됩니다. 서비스나 테스트를 변경할 필요 없습니다.Likely an incorrect or invalid review comment.
| for (int i = 0; i < numberOfThreads; i++) { | ||
| executorService.submit(() -> { | ||
| try { | ||
| ClubInfoRequest request = ClubRequestFixture.createValidClubInfoRequest(); | ||
|
|
||
| // --- 핵심 변경점 --- | ||
| // 모든 스레드가 이 지점에서 대기. | ||
| // 마지막 스레드가 barrier.await()을 호출하면 모든 스레드가 동시에 다음 코드를 실행. | ||
| barrier.await(); | ||
|
|
||
| // 모든 스레드가 거의 동시에 이 메서드를 호출하게 되어 충돌 가능성이 극대화됨 | ||
| clubProfileService.updateClubInfo(request, userDetails); | ||
| successCount.incrementAndGet(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
동시 시작 배리어 위치만으로는 ‘동일 버전 경쟁’이 보장되지 않습니다.
메서드 내부의 최초 조회 타이밍이 선행 커밋 이후가 될 수 있어 충돌이 나지 않습니다. 테스트 안정화를 위해:
- 동일 스냅샷 강제: 사전에 Club를 한 번 조회해 version을 고정하고, expectedVersion을 모두에게 전달하여 조건부 업데이트를 유발(서비스 시그니처 확장 또는 별도 테스트 케이스).
- 또는 저장 직전 지점에 동기화 훅(테스트 전용) 삽입.
현재 구조만으론 플래키합니다.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the service implementation
service_file=$(find . -type f | rg 'ClubProfileService\.java')
echo "Service file: $service_file"
# Show class context
rg -nP --type=java '\bclass\s+ClubProfileService\b' "$service_file" -C3
# Show updateClubInfo method implementation
rg -nP --type=java '\bupdateClubInfo\s*\(' "$service_file" -C20Length of output: 2469
🏁 Script executed:
#!/bin/bash
set -e
# Locate Club entity class
entity_file=$(find backend/src -type f | rg 'Club\.java$')
echo "Entity file: $entity_file"
# Display version field annotation if present
rg -n '@Version' -C2 "$entity_file"Length of output: 457
동일 버전 경쟁 보장을 위한 테스트 동기화 강화
barrier.await()는 스레드 진입만 동기화하므로, 한 스레드의 조회–커밋이 다른 스레드의 조회 이전에 완료되면 충돌이 발생하지 않아 플래키합니다.
- Club 엔티티를 테스트 시작 전에 한 번 조회해 고정된 버전을 모든 스레드에 전달(expectedVersion 필드 추가)
- 또는 save 직전 지점에 테스트 전용 동기화 훅(barrierㆍCountDownLatch 등) 삽입해 모든 스레드가 동일 버전을 기준으로 업데이트하도록 강제
🤖 Prompt for AI Agents
In backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java around
lines 49 to 61, the current barrier.await() only synchronizes thread entry and
allows races where one thread's read/commit completes before others read,
causing flaky tests; fix by ensuring all threads operate on the same entity
version: either fetch the Club entity once before spawning threads and add an
expectedVersion field to the ClubInfoRequest that each thread uses for
updateClubInfo, or add a test-only synchronization hook immediately before the
save/commit point (e.g., a second Barrier/CountDownLatch inside the service test
path) so all threads reach the save call with the same read version; implement
one approach, pass the common version into each request or wire the
synchronization hook into the service test path, and assert concurrency conflict
behavior deterministically.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (2)
66-68: NPE 가드 반영 완료이전 리뷰에서 제안된 메시지 null 가드가 반영되었습니다. 👍
49-61: barrier 1회만으로는 ‘동일 버전 경쟁’이 보장되지 않아 플래키합니다한 스레드가 먼저 읽고 커밋한 뒤, 다른 스레드가 최신 버전을 읽으면 충돌이 3개가 아닌 2개/1개로 줄 수 있습니다. 이전 코멘트와 동일 관점입니다.
옵션 A(권장, 테스트 전용 훅): 서비스 save 직전 지점에 테스트 훅을 주입해 모두 동일 스냅샷에서 커밋하도록 강제.
// 예: ClubProfileService (테스트 프로파일에서만 빈 주입) + @Profile("test") + @Component + public class ConcurrencyTestGate { + private final CyclicBarrier beforeSave = new CyclicBarrier(4); + public void awaitBeforeSave() { try { beforeSave.await(); } catch (Exception ignored) {} } + } - repository.save(entity); + testGate.awaitBeforeSave(); + repository.save(entity);테스트에서 스레드 수만큼 게이트를 구성하면 성공 1/충돌 3을 결정론적으로 재현할 수 있습니다.
옵션 B(요청에 예상 버전 포함): 요청/서비스에 expectedVersion을 추가해 find-by-id-and-version 또는 버전 필드 세팅 후 save하도록 변경.
추가로, 집계 일관성 보장을 위해 합계 검증을 넣어 주세요.
- assertEquals(1, successCount.get(), "성공한 요청은 1개여야 합니다."); - assertEquals(numberOfThreads - 1, conflictCount.get(), "실패(충돌)한 요청은 " + (numberOfThreads - 1) + "개여야 합니다."); + assertEquals(1, successCount.get(), "성공한 요청은 1개여야 합니다."); + assertEquals(numberOfThreads - 1, conflictCount.get(), "실패(충돌)한 요청은 " + (numberOfThreads - 1) + "개여야 합니다."); + assertEquals(numberOfThreads, successCount.get() + conflictCount.get(), "집계 누락 없이 총합이 맞아야 합니다.");Also applies to: 85-88
🧹 Nitpick comments (5)
backend/build.gradle (1)
63-63: spring-test는 testImplementation으로 내려주세요프로덕션 클래스패스에 테스트 유틸이 포함됩니다. 스코프를 테스트 전용으로 이동하세요.
- implementation 'org.springframework:spring-test' + testImplementation 'org.springframework:spring-test'backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (4)
82-84: 잠재적 행 대기 방지: 대기 타임아웃 추가 권장비정상 상황에서 테스트가 장시간 블로킹될 수 있습니다. 타임아웃과 종료 확인을 추가하세요.
- latch.await(); // 모든 스레드가 작업을 마칠 때까지 대기 - executorService.shutdown(); + boolean completed = latch.await(10, TimeUnit.SECONDS); + executorService.shutdown(); + // 필요시 종료 보장 + executorService.awaitTermination(5, TimeUnit.SECONDS); + assertEquals(true, completed, "스레드 작업이 타임아웃 내 완료되지 않았습니다.");
32-34: Optional.get() 즉시 호출 대신 명시적 예외로 전환테스트 실패 시 원인 파악이 쉽도록 메시지 포함해 주세요.
- User user = userRepository.findUserByUserId(UserFixture.collectUserId).get(); + User user = userRepository.findUserByUserId(UserFixture.collectUserId) + .orElseThrow(() -> new IllegalStateException("사전 준비된 테스트 유저가 존재하지 않습니다: " + UserFixture.collectUserId));
52-53: 고정 ID 사용은 테스트 간 간섭 위험동일 "club_123"를 여러 테스트가 공유하면 간섭/순서 의존이 발생할 수 있습니다. 픽스처에 ID 파라미터를 추가하거나 UUID로 유니크 ID를 생성하세요.
- public static ClubInfoRequest createValidClubInfoRequest() { + public static ClubInfoRequest createValidClubInfoRequest() { + String id = "club_" + java.util.UUID.randomUUID(); return new ClubInfoRequest( - "club_123", + id, ... ); }또는
createValidClubInfoRequest(String id)오버로드를 추가하고 테스트에서 명시적으로 전달하세요.
63-71: 예외 분기 정밀도 개선(선택)메시지 문자열 매칭은 취약합니다. MongoDB WriteConflict(code 112)로 식별하면 더 견고합니다.
- } catch (DataAccessException e) { - if (e.getMessage() != null && e.getMessage().contains("WriteConflict")) { + } catch (DataAccessException e) { + Throwable root = org.springframework.core.NestedExceptionUtils.getMostSpecificCause(e); + Integer code = (root instanceof com.mongodb.MongoException me) ? me.getCode() : null; + if (Integer.valueOf(112).equals(code)) { conflictCount.incrementAndGet(); } else { e.printStackTrace(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/build.gradle(1 hunks)backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (2)
backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
ClubRequestFixture(10-51)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(9-33)
🔇 Additional comments (2)
backend/build.gradle (1)
44-44: rerunner-jupiter 호환성 검증 필요
CI 환경에서 JAVA_HOME 설정 오류(“/usr/lib/jvm/java-17-openjdk-amd64” 경로 미존재)로 자동 검증이 불가능했습니다. 로컬에서 유효한 JDK 경로를JAVA_HOME으로 설정한 뒤 아래 명령으로 재시도 횟수 및 리포팅 결과를 확인해주세요.export JAVA_HOME=/path/to/your/jdk cd backend ./gradlew -q dependencyInsight --configuration testRuntimeClasspath --dependency rerunner-jupiter ./gradlew test --tests '*ClubProfileServiceTest*'backend/src/test/java/moadong/club/service/ClubProfileServiceTest.java (1)
36-38: 플래키 완화용 재시도 적용👍Assertion 실패에만 재시도하도록 한 선택 적절합니다.
2 participants
#️⃣연관된 이슈
#708
📝작업 내용
중요
개발 서버에서 테스트를 거치고 메인에 머지가 되어야 함
그리고 메인 서버 컬렉션에 미리 동아리 정보, 동아리 지원서 version 필드를 추가시켜야 함
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
New Features
Refactor
Tests