[FEAT] 챌린지 라운드 연장 과 챌린지 시작, 종료를 진행하는 API #201
Conversation
📝 WalkthroughWalkthrough라운드 연장/하차 결정 기능과 자동 라운드 진행/하차 배치가 추가되었습니다. 컨트롤러·서비스·레포지토리·스케줄러·유닛테스트가 확장되어 사용자 의도 수집, 결정 검증, 멱등성 처리, 다음 라운드 생성 및 참가자 상태 전이가 구현되었습니다. (요약 50단어 이내) Changes
Sequence Diagram(s)라운드 연장/하차 결정 흐름sequenceDiagram
participant User
participant Controller as RoundController
participant Service as RoundDecisionServiceImpl
participant ChallengeRepo
participant UserChallengeRepo
participant RoundRepo
participant RoundRecordRepo
User->>Controller: POST /api/v1/challenges/{id}/rounds/decision (intent)
Controller->>Service: decideNextRound(userId, challengeId, request)
Service->>ChallengeRepo: findById(challengeId)
ChallengeRepo-->>Service: Challenge
Service->>UserChallengeRepo: findByUserAndChallenge(userId, challenge)
UserChallengeRepo-->>Service: UserChallenge
Service->>RoundRepo: getCurrentRound(challenge)
RoundRepo-->>Service: currentRound
Note over Service: 검증(활성/기간/intent)
Service->>RoundRecordRepo: findByUserChallengeAndRound(...)
RoundRecordRepo-->>Service: RoundRecord
Service->>RoundRecordRepo: update(nextRoundIntent)
RoundRecordRepo-->>Service: OK
Service-->>Controller: void
Controller-->>User: 200 OK
라운드 종료 및 진행 흐름sequenceDiagram
participant Scheduler as RoundLifecycleScheduler
participant Service as RoundLifecycleServiceImpl
participant RoundRepo
participant RoundRecordRepo
participant ChallengeRepo
participant Converter as RoundConverter
Scheduler->>Service: processRoundsEndedAt(yesterday)
Service->>RoundRepo: findAllByEndDate(yesterday)
RoundRepo-->>Service: [endedRounds]
loop for each endedRound
Service->>RoundRepo: findByIdWithChallengeAndCurrentRound(endedRound.id)
RoundRepo-->>Service: endedRound(with challenge)
Note over Service: 멱등성 확인(currentRound 일치)
Service->>RoundRecordRepo: findJoinedByRound(endedRound)
RoundRecordRepo-->>Service: [records]
alt continuers empty
Service->>ChallengeRepo: updateStatus(FINISHED)
else
Service->>RoundRepo: findNextRound(...)
alt nextRound not found
Service->>Converter: toNextRoundEntity(challenge, endedRound)
Converter-->>Service: newRound
Service->>RoundRepo: save(newRound)
end
loop each continuer
Service->>RoundRecordRepo: existsByUserChallengeAndRound(...)
alt not exists
Service->>RoundRecordRepo: save(new RoundRecord(...))
end
end
Service->>ChallengeRepo: setCurrentRound(nextRound)
end
end
하차 사용자 처리 흐름sequenceDiagram
participant Scheduler as RoundDropScheduler
participant Service as RoundDropServiceImpl
participant RoundRepo
participant RoundRecordRepo
participant UserChallengeRepo
participant ChallengeRepo
Scheduler->>Service: dropNonContinuersAt(today)
Service->>RoundRepo: findAllByEndDate(today)
RoundRepo-->>Service: [rounds]
loop for each round
Service->>RoundRecordRepo: findJoinedByRound(round)
RoundRecordRepo-->>Service: [records]
loop each record
alt record.nextRoundIntent != CONTINUE && userChallenge == JOINED
Service->>UserChallengeRepo: updateStatus(DROPPED)
Service->>ChallengeRepo: decrementCurrentParticipants(challenge)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
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: 5
Fix all issues with AI Agents 🤖
In
@src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java:
- Around line 67-68: The inline comment before the if check in
RoundDecisionServiceImpl is at column 0; re-indent the comment so it aligns with
the surrounding code block (match the indentation of the following if statement
that checks today.isBefore(decisionOpenDate) ||
!today.isBefore(decisionCloseDate)), preserving the comment text and formatting
to conform to Java indentation conventions.
- Line 50: The code uses LocalDate.now() in RoundDecisionServiceImpl which
relies on the system default TZ and can diverge from RoundDropScheduler; define
a class-level ZoneId constant (e.g., ASIA_SEOUL = ZoneId.of("Asia/Seoul")) and
replace LocalDate.now() with LocalDate.now(ASIA_SEOUL) (and use the same
constant anywhere else in this class) to ensure consistent date calculations
with the scheduler.
In @src/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.java:
- Around line 20-26: The scheduler method RoundDropScheduler.dropNonContinuers
uses @Scheduled(zone = "Asia/Seoul") but calls LocalDate.now() which uses the
system default zone; replace that call with a zone-aware date retrieval such as
LocalDate.now(ZoneId.of("Asia/Seoul")) (or derive the ZoneId from the scheduler
zone) so the date used by roundDropService.dropNonContinuersAt(today) matches
the scheduled timezone and avoids off-by-one-day issues when system timezone
differs.
In @src/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java:
- Around line 21-23: The scheduled task closeEndedRounds currently relies on
system default timezone for both the @Scheduled cron and LocalDate.now(); update
the @Scheduled annotation to include zone = "Asia/Seoul" and replace
LocalDate.now() with LocalDate.now(ZoneId.of("Asia/Seoul")) inside
closeEndedRounds (and add the java.time.ZoneId import) so the cron and date
calculations consistently use the Asia/Seoul timezone.
🧹 Nitpick comments (10)
src/main/java/com/hrr/backend/domain/challenge/entity/Challenge.java (1)
119-122: 상태 전환 검증 로직 추가를 고려해보세요.현재
updateStatus메서드는 어떤 상태로든 자유롭게 변경할 수 있어, 잘못된 상태 전환(예:COMPLETED→UPCOMING)을 방지할 수 없습니다. 비즈니스 규칙에 따라 허용되는 상태 전환만 가능하도록 검증 로직을 추가하는 것을 권장합니다.🔎 상태 전환 검증 로직 예시
//상태 변경 메서드 추가 public void updateStatus(ChallengeStatus status) { + if (!this.status.canTransitionTo(status)) { + throw new IllegalStateException( + String.format("Cannot transition from %s to %s", this.status, status) + ); + } this.status = status; }그리고
ChallengeStatusenum에 전환 규칙을 정의:public enum ChallengeStatus { UPCOMING, IN_PROGRESS, COMPLETED, CANCELLED; public boolean canTransitionTo(ChallengeStatus newStatus) { // 전환 규칙 정의 return switch (this) { case UPCOMING -> newStatus == IN_PROGRESS || newStatus == CANCELLED; case IN_PROGRESS -> newStatus == COMPLETED || newStatus == CANCELLED; case COMPLETED, CANCELLED -> false; // 최종 상태 }; } }참고: Domain-Driven Design에서 엔티티의 상태 전환은 명확한 규칙을 따라야 하며, 상태 머신 패턴 적용을 고려해볼 수 있습니다.
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleService.java (1)
1-7: 인터페이스에 JavaDoc을 추가해주세요.Public API에 해당하는 인터페이스는 사용 방법과 계약을 명확히 문서화하는 것이 중요합니다. 특히 다음 사항들을 문서화해주세요:
endDate파라미터의 정확한 의미 (해당 날짜에 종료된 라운드를 처리하는지)- 메서드의 부작용 (DB 업데이트, 챌린지 상태 변경 등)
- 멱등성 보장 여부 (스케줄러 재실행 시 안전한지)
- 예외 발생 조건
🔎 JavaDoc 추가 예시
+/** + * 라운드 생명주기를 관리하는 서비스 인터페이스. + * 스케줄러에 의해 호출되어 종료된 라운드를 처리하고 다음 라운드를 생성합니다. + */ public interface RoundLifecycleService { + /** + * 지정된 날짜에 종료된 모든 라운드를 처리합니다. + * + * @param endDate 처리할 라운드의 종료 날짜 + * @throws IllegalStateException 라운드 처리 중 비즈니스 규칙 위반 시 + * + * <p>이 메서드는 멱등성을 보장하며, 동일한 endDate로 여러 번 호출되어도 + * 중복 처리가 발생하지 않습니다.</p> + */ void processRoundsEndedAt(LocalDate endDate); }참고: Effective Java Item 56 - "모든 공개 API 요소에 문서화 주석을 작성하라"
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionService.java (1)
1-7: 인터페이스 문서화가 필요합니다.Public API 인터페이스는 사용자(다른 개발자)가 올바르게 사용할 수 있도록 명확한 계약을 문서화해야 합니다. PR 요약에 따르면 이 메서드는 라운드 연장 여부를 결정하는 중요한 비즈니스 로직이므로, 다음 내용들을 JavaDoc으로 추가해주세요:
- 메서드의 목적 (다음 라운드 연장 여부 결정)
- 각 파라미터의 의미와 제약사항
- 결정 가능 기간 (종료 3일 전부터 하루 동안)
- 권한 요구사항 (방장과 일반 참가자 모두 가능한지)
- 발생 가능한 예외 (권한 없음, 이미 결정함, 기간 아님 등)
- 부작용 (RoundRecord 업데이트)
🔎 JavaDoc 추가 예시
+/** + * 라운드 연장 결정을 처리하는 서비스 인터페이스. + */ public interface RoundDecisionService { + /** + * 사용자의 다음 라운드 참여 의사를 결정합니다. + * + * @param userId 결정을 내리는 사용자 ID + * @param challengeId 대상 챌린지 ID + * @param request 연장 여부 (CONTINUE 또는 DROP) + * + * @throws com.hrr.backend.global.exception.CustomException + * - CHALLENGE_NOT_FOUND: 챌린지가 존재하지 않는 경우 + * - USER_NOT_PARTICIPATING: 사용자가 챌린지 참가자가 아닌 경우 + * - DECISION_PERIOD_NOT_STARTED: 결정 가능 기간이 아닌 경우 + * - ALREADY_DECIDED: 이미 결정한 경우 + * + * <p>결정 가능 기간: 라운드 종료 3일 전부터 1일 전까지</p> + */ void decideNextRound(Long userId, Long challengeId, RoundDecisionRequestDto request); }참고: Clean Code에서 강조하는 "코드는 의도를 표현해야 한다"는 원칙은 인터페이스 문서화에서 더욱 중요합니다.
src/main/java/com/hrr/backend/domain/round/service/RoundDropService.java (1)
5-7: 인터페이스 문서화 추가 권장메서드명이 명확하긴 하지만, 공개 API로서 다음 정보를 포함하는 JavaDoc 추가를 권장합니다:
- "non-continuers"의 정의 (연장 의사를 밝히지 않았거나 하차 의사를 표시한 챌린저)
- drop 작업의 구체적인 동작
- 호출 시점 및 멱등성 보장 여부
🔎 문서화 예시
+/** + * 지정된 종료일의 라운드에서 연장하지 않는 챌린저들을 정리합니다. + * + * @param endDate 라운드 종료일 (해당 날짜에 종료되는 라운드들이 대상) + */ void dropNonContinuersAt(LocalDate endDate);src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (2)
47-49: NPE 방어 로직 개선이 필요합니다.Line 47의 조건문에서
challenge.getCurrentRound()null 체크와getId()호출이 동일 라인에 있어, 단락 평가(short-circuit)로 안전하지만 가독성이 떨어집니다.🔎 가독성 개선 제안
- // 멱등성: 이미 currentRound가 바뀌었으면 스킵 - if (challenge.getCurrentRound() == null || !challenge.getCurrentRound().getId().equals(round.getId())) { - continue; - } + // 멱등성: 이미 currentRound가 바뀌었으면 스킵 + Round currentRound = challenge.getCurrentRound(); + if (currentRound == null || !currentRound.getId().equals(round.getId())) { + continue; + }
57-65: 루프 내 반복적인 UPDATE 쿼리 최적화 필요N+1 쿼리 문제가 확인되었습니다. 현재 코드는 드랍 대상 참가자마다 개별 UPDATE 쿼리를 실행하므로, 드랍 인원이 많을수록 데이터베이스 부하가 증가합니다.
권장 해결책:
- 드랍된 인원 수를 루프에서 집계하고, 단일 UPDATE 쿼리로 일괄 처리
- ChallengeRepository에
decreaseCurrentParticipantCountBy(Long id, int count)메서드를 추가하거나- Challenge 엔티티에서 currentParticipants를 메모리에서 감소시킨 후 한 번에 저장
🔎 구현 예시
for (RoundRecord rr : records) { if (rr.getNextRoundIntent() == NextRoundIntent.CONTINUE) continue; UserChallenge uc = rr.getUserChallenge(); if (uc.getStatus() != ChallengeJoinStatus.JOINED) continue; uc.updateStatus(ChallengeJoinStatus.DROPPED); - challengeRepository.decreaseCurrentParticipantCount(challenge.getId()); } + + // 드랍된 인원 수를 집계하여 한 번에 감소 + long droppedCount = records.stream() + .filter(rr -> rr.getNextRoundIntent() != NextRoundIntent.CONTINUE) + .filter(rr -> rr.getUserChallenge().getStatus() == ChallengeJoinStatus.JOINED) + .count(); + if (droppedCount > 0) { + challengeRepository.decreaseCurrentParticipantCountBy(challenge.getId(), (int)droppedCount); + }ChallengeRepository에 추가:
@Modifying @Query("UPDATE Challenge c " + "SET c.currentParticipants = c.currentParticipants - :count " + "WHERE c.id = :id AND c.currentParticipants >= :count") int decreaseCurrentParticipantCountBy(@Param("id") Long id, @Param("count") int count);참고: 현재
decreaseCurrentParticipantCountBy메서드는 존재하지 않으므로 추가 구현이 필수입니다.src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
72-76: 중복된 null 체크를 제거하세요.Line 74의
intent == null체크는 불필요합니다.RoundDecisionRequestDto의intent필드가@NotNull로 검증되므로, 이 시점에서request.intent()는 절대 null이 될 수 없습니다.Jakarta Validation이 이미 null을 거부하므로, 이 체크는 중복입니다.
🔎 중복 제거 제안
// intent 유효성 (UNDECIDED로 요청하는 것 방지) NextRoundIntent intent = request.intent(); - if (intent == null || intent == NextRoundIntent.UNDECIDED) { + if (intent == NextRoundIntent.UNDECIDED) { throw new GlobalException(ErrorCode.ROUND_DECISION_INTENT_INVALID); }참고: DTO validation이 null을 방어하므로 비즈니스 로직에서는
UNDECIDED검증에만 집중할 수 있습니다.src/test/java/com/hrr/backend/domain/round/RoundFlowUnitTest.java (2)
91-124: 결정 기간 검증 로직 확인 필요Line 113의 주석과 코드를 보면 결정 기간이
endDate - 2(open)부터endDate - 1(close)까지, 즉 하루만 허용됩니다. 하지만 Line 113에서endDate = today + 4로 설정하면:
- open = today + 2
- close = today + 3
그런데 Line 118의 조건
today.isBefore(decisionOpenDate)는today < today+2이므로 true가 되어 예외가 발생합니다. 이 경우 "결정 기간이 아직 시작하지 않음"을 테스트하는 것으로 보입니다.테스트 이름이 "결정 기간이 아니면"으로 포괄적인데, 다음을 명확히 구분하는 것이 좋겠습니다:
- 결정 기간 시작 전 (현재 테스트)
- 결정 기간 종료 후
🔎 결정 기간 종료 후 테스트 추가 제안
@Test @DisplayName("decideNextRound: 결정 기간이 종료된 후면 ROUND_DECISION_PERIOD_NOT_OPEN 예외") void decideNextRound_throws_whenAfterDecisionWindow() { // given Long userId = 1L; Long challengeId = 10L; LocalDate today = LocalDate.now(); Challenge challenge = mock(Challenge.class); Round currentRound = mock(Round.class); UserChallenge uc = mock(UserChallenge.class); when(challengeRepository.findById(challengeId)).thenReturn(Optional.of(challenge)); when(userChallengeRepository.findByUserIdAndChallengeId(userId, challengeId)).thenReturn(Optional.of(uc)); when(uc.getStatus()).thenReturn(ChallengeJoinStatus.JOINED); when(challenge.getCurrentRound()).thenReturn(currentRound); when(currentRound.getStartDate()).thenReturn(today.minusDays(10)); // 결정 기간 종료 후: end=today-1 -> open=today-3, close=today-2 (이미 지난 기간) when(currentRound.getEndDate()).thenReturn(today.minusDays(1)); RoundDecisionRequestDto request = new RoundDecisionRequestDto(NextRoundIntent.CONTINUE); // when & then assertThatThrownBy(() -> roundDecisionService.decideNextRound(userId, challengeId, request)) .isInstanceOf(GlobalException.class) .extracting("errorCode") .isEqualTo(ErrorCode.ROUND_DECISION_PERIOD_NOT_OPEN); verifyNoInteractions(roundRecordRepository); }
272-322: 다음 라운드 생성 및 전환 로직이 잘 검증되었습니다.다음 라운드 생성, RoundRecord 생성, currentRound 교체의 전체 흐름을 포괄적으로 테스트하고 있습니다. Mock 설정도 세밀하게 되어 있습니다.
다만, Line 310에서
existsByUserChallengeAndRound가 false를 반환하도록 설정했는데, 실제로는 여러 명의 continuer가 있을 수 있습니다. 현재 테스트는 continuer가 1명인 경우만 다루고 있으므로, 여러 명의 continuer를 처리하는 케이스도 추가하면 더욱 완벽할 것 같습니다.src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (1)
80-83: 다음 라운드 조회/생성 로직에서 잠재적 경쟁 조건이 있습니다.
findByChallengeIdAndRoundNumber로 조회 후 없으면 생성하는 패턴(orElseGet)은 일반적으로 괜찮지만, 동시성 환경에서는 문제가 될 수 있습니다:
- 동시 실행 시나리오: 같은 endDate를 가진 라운드가 여러 개이고, 모두 같은 챌린지의 다음 라운드를 생성하려 할 때
- 결과: 중복 생성 시도로 인한 DB unique constraint violation 가능
현재 코드에서는 이런 상황이 발생할 가능성이 낮아 보이지만(한 챌린지는 보통 하나의 현재 라운드만 가짐), 더 안전하게 처리하고 싶다면 다음을 고려하세요:
🔎 개선 제안: DB 레벨 유니크 제약과 재시도
Option 1: DB에
(challenge_id, round_number)유니크 제약이 있다면, 중복 생성 시도 시 예외를 잡아 재조회:Round nextRound; try { nextRound = roundRepository .findByChallengeIdAndRoundNumber(challenge.getId(), endedRound.getRoundNumber() + 1) .orElseGet(() -> roundRepository.save(roundConverter.toNextRoundEntity(challenge, endedRound))); } catch (DataIntegrityViolationException e) { // 동시 생성으로 인한 중복, 재조회 nextRound = roundRepository .findByChallengeIdAndRoundNumber(challenge.getId(), endedRound.getRoundNumber() + 1) .orElseThrow(() -> new IllegalStateException("Round should exist after concurrent creation")); }Option 2: 비관적 락 사용 (challenge에 대한 배타적 락)
하지만 현재 구조상 문제가 발생할 가능성이 매우 낮다면, 있는 그대로 유지해도 충분합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/main/java/com/hrr/backend/domain/challenge/entity/Challenge.javasrc/main/java/com/hrr/backend/domain/round/controller/RoundController.javasrc/main/java/com/hrr/backend/domain/round/converter/RoundConverter.javasrc/main/java/com/hrr/backend/domain/round/dto/RoundDecisionRequestDto.javasrc/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.javasrc/main/java/com/hrr/backend/domain/round/service/RoundDecisionService.javasrc/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.javasrc/main/java/com/hrr/backend/domain/round/service/RoundDropService.javasrc/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.javasrc/main/java/com/hrr/backend/domain/round/service/RoundLifecycleService.javasrc/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.javasrc/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.javasrc/test/java/com/hrr/backend/domain/challenge/service/ChallengeServiceProfileTest.javasrc/test/java/com/hrr/backend/domain/round/RoundFlowUnitTest.javasrc/test/resources/application-test.yml
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (2)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
Service(23-85)src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (1)
Service(23-72)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (2)
src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (1)
Service(23-72)src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (1)
Service(23-109)
src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (2)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
Service(23-85)src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (1)
Service(23-109)
src/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java (1)
src/main/java/com/hrr/backend/domain/round/converter/RoundConverter.java (1)
Component(11-50)
🔇 Additional comments (24)
src/test/java/com/hrr/backend/domain/challenge/service/ChallengeServiceProfileTest.java (1)
50-53: 의존성 주입 구조 개선 👍
ChallengeConverter를 생성자 기반으로 초기화하여 실제 프로덕션 코드의 의존성 구조를 테스트에서도 정확히 반영한 점이 좋습니다!다만,
S3UrlUtil을 모킹만 하고 stubbing을 하지 않은 상태에서ChallengeConverter내부에서 해당 유틸을 호출하면null이 반환되어NullPointerException이 발생할 수 있습니다. 현재 테스트 케이스들이 이미 통과하고 있다면 문제없지만, 향후ChallengeConverter에서 S3 URL 생성 로직이 추가되는 경우를 대비해 필요한 stubbing을 추가하는 것을 고려해보세요.예시: S3UrlUtil stubbing
필요한 경우
setUp()메서드나 각 테스트에서:given(s3UrlUtil.generatePresignedUrl(anyString())) .willReturn("https://s3.example.com/test-image.jpg");src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
70-71: 멱등성 보장을 위한 탁월한 구현입니다! 👌스케줄러 재실행 시나리오를 대비한
existsByUserChallengeAndRound메서드는 훌륭한 방어책입니다. 더욱 인상적인 점은 RoundRecord 엔티티에(round_id, user_challenge_id)복합 유니크 제약조건(uk_round_user)이 이미 정의되어 있다는 것입니다.이 조합은 두 겹의 방어를 제공합니다:
- 애플리케이션 레벨:
existsByUserChallengeAndRound로 중복 생성 사전 방지- 데이터베이스 레벨: 유니크 제약조건으로 동시성 문제까지 원천 차단
유니크 제약조건이 자동으로 인덱스를 생성하므로,
exists쿼리는 효율적으로 실행될 것입니다. 멱등성 설계가 견고하게 갖춰져 있습니다.src/test/resources/application-test.yml (2)
16-18: 테스트 환경에서 스케줄링 비활성화 👍테스트 실행 중 스케줄러가 자동으로 동작하는 것을 방지하여 테스트의 결정성(determinism)을 보장합니다. 좋은 선택입니다!
34-62: 외부 서비스 테스트 설정이 적절합니다환경 변수와 더미 기본값을 사용하여 테스트 격리를 잘 구현했습니다. 실제 외부 API 호출을 방지하면서도 설정 구조는 프로덕션과 일관성을 유지합니다.
src/main/java/com/hrr/backend/domain/round/converter/RoundConverter.java (1)
39-48: 다음 라운드 생성 로직이 명확합니다이전 라운드의 종료일 다음 날을 시작일로 설정하고,
ROUND_WEEKS상수를 재사용하여 일관성을 유지합니다.toFirstRoundEntity와 패턴이 일치하여 가독성도 좋습니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
95-97: 라운드 의사결정 에러 코드가 체계적입니다새로운 에러 코드들이 기존 패턴을 잘 따르고 있으며, 각각의 검증 시나리오(현재 라운드 여부, 결정 기간, 입력값)를 명확하게 구분합니다. 메시지도 사용자 친화적이네요!
src/main/java/com/hrr/backend/domain/round/dto/RoundDecisionRequestDto.java (1)
6-8: 깔끔한 DTO 설계입니다!불변 레코드와 Jakarta Validation을 활용한 간결하고 명확한 구조입니다. 요청 검증이 적절히 설정되어 있습니다.
src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (1)
32-71: 전체 흐름은 견고합니다.멱등성 체크, 상태 검증, 그리고 라운드별 에러 격리 처리가 잘 구현되어 있습니다. 배치 처리의 안정성을 고려한 좋은 설계입니다.
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (2)
60-70: 결정 기간 로직 검증을 확인하세요.Lines 64-68의 결정 기간 계산이 요구사항과 일치하는지 확인이 필요합니다.
현재 로직:
decisionOpenDate = endDate - 2decisionCloseDate = endDate - 1- 조건:
[openDate, closeDate)→ endDate - 2일 하루만 허용하지만 PR 요약에는 "라운드 시작 3일 전에 연장 여부를 묻고, 응답 기간은 하루"라고 명시되어 있습니다.
- 라운드 시작 3일 전 =
nextStart - 3 = endDate + 1 - 3 = endDate - 2✓- 응답 기간 하루 =
[endDate - 2, endDate - 1)✓로직은 정확하지만, 주석(Lines 60-63)이 계산 과정을 잘 설명하고 있어 좋습니다. 추가로 비즈니스 요구사항과의 매핑을 주석에 명시하면 더 명확할 것입니다.
💡 주석 보강 제안
- // 결정 기간 체크: (다음 라운드 시작 3일 전 ~ 2일 전) => 딱 하루만 - // nextStart = endDate + 1 - // open = nextStart - 3 = endDate - 2 - // close = nextStart - 2 = endDate - 1 + // 결정 기간 체크: 다음 라운드 시작 3일 전 하루 동안만 허용 + // - 요구사항: 라운드 시작(nextStart) 3일 전에 연장 여부 결정, 응답 기간 1일 + // - nextStart = endDate + 1 + // - 결정일(open) = nextStart - 3 = endDate - 2 + // - 마감일(close) = nextStart - 2 = endDate - 1 + // - 허용 범위: [결정일, 마감일) = endDate - 2일 당일만 LocalDate decisionOpenDate = currentRound.getEndDate().minusDays(2); LocalDate decisionCloseDate = currentRound.getEndDate().minusDays(1);
78-84: JPA 영속성 컨텍스트를 잘 활용하고 있습니다.Line 81에서
rr.updateNextRoundIntent(intent)호출 후 명시적인save()없이 변경 사항이 자동으로 DB에 반영됩니다. 이는 JPA 더티 체킹(dirty checking)을 올바르게 활용한 것입니다.Line 82-83의 주석도 이 부분을 명확히 설명하고 있어 좋습니다.
src/main/java/com/hrr/backend/domain/round/controller/RoundController.java (1)
24-33: 깔끔한 컨트롤러 설계입니다!인증, 검증, 서비스 위임의 책임 분리가 명확하고, RESTful 엔드포인트 구조가 적절합니다.
@Valid어노테이션으로 DTO 검증이 자동화되어 있어 좋습니다.src/test/java/com/hrr/backend/domain/round/RoundFlowUnitTest.java (7)
41-49: 테스트 클래스 구조가 깔끔합니다! 👍세 개의 서비스 구현체를 하나의 테스트 클래스에서 테스트하고 있습니다. 각 서비스가 명확히 구분되어 있고 주석으로 섹션이 나뉘어 있어 가독성이 좋습니다.
55-89: UNDECIDED intent 검증 테스트가 잘 작성되었습니다.유효하지 않은 intent(UNDECIDED)에 대한 예외 처리를 정확히 검증하고 있으며,
verifyNoInteractions(roundRecordRepository)로 부작용이 없음을 확인하는 것도 좋습니다.한 가지 제안: Line 73-75의 주석이 코드의 의도를 잘 설명하고 있지만, 테스트 시나리오에 집중하기 위해 given 블록에 더 간결하게 표현할 수 있습니다.
127-161: 정상 케이스 검증이 명확합니다! ✨결정 기간 내에 유효한 intent로 요청했을 때
RoundRecord.updateNextRoundIntent()가 정확히 호출되는지 검증하고 있습니다. Mock 설정도 적절하고, verify 구문으로 기대 동작을 명확히 확인합니다.
167-208: 드랍 로직 테스트가 견고합니다.CONTINUE와 STOP을 구분하여 STOP만 드랍 처리하고, 참가자 수 감소가 정확히 1회만 호출되는지 검증(
times(1))하는 것이 좋습니다. 멱등성을 위한 currentRound 일치 확인도 포함되어 있습니다.
210-234: 멱등성 검증 테스트가 훌륭합니다! 🎯currentRound가 이미 변경된 경우 드랍 처리를 스킵하는 로직을 검증하고 있으며,
never()matcher로 불필요한 DB 호출이 없음을 확인합니다. 이런 멱등성 체크는 스케줄러 기반 배치에서 매우 중요합니다.
240-270: 챌린지 종료 케이스 검증이 정확합니다.CONTINUE가 없을 때 챌린지가 FINISHED로 변경되고, 불필요한 라운드 생성이 없음을
never()matcher로 확인하는 것이 좋습니다.
324-365: 기존 라운드 재사용 테스트로 멱등성을 확보했습니다! 💯이미 존재하는 nextRound를 재사용하는 경로를 테스트하여, 중복 생성을 방지하는 멱등성 로직을 검증합니다.
never()matcher로 불필요한 save나 conversion이 없음을 확인하는 것이 매우 좋습니다.Line 355에서
existsByUserChallengeAndRound가 true를 반환하므로 RoundRecord save도 스킵되는 것을 검증합니다.src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (6)
47-60: 멱등성 체크와 상태 검증이 잘 구현되었습니다! 👍Line 51-55의 currentRound 일치 확인과 Line 58-60의 ONGOING 상태 검증으로 중복 처리를 방지하고 있습니다. 스케줄러 기반 배치에서 필수적인 안전장치입니다.
62-78: 비즈니스 로직이 명확하게 구현되었습니다.JOINED 상태의 참가자 중 CONTINUE intent를 가진 사람만 필터링하는 로직이 깔끔합니다. Line 63의 주석으로 드랍 스케줄러와의 책임 분리도 명확히 했습니다.
continuer가 없을 때 챌린지를 종료하는 것도 자연스럽습니다.
85-96: RoundRecord 생성 로직의 멱등성이 보장됩니다.Line 90의
existsByUserChallengeAndRound체크로 중복 생성을 방지하고 있어 좋습니다. 재실행 시에도 안전합니다.
98-103: 라운드 전환 로깅이 유용합니다! 📝
changeCurrentRound호출 후 로그로 전환 내역을 남기는 것이 모니터링과 디버깅에 도움이 됩니다. 라운드 번호까지 포함되어 있어 명확합니다.
105-108: 챌린지 종료 헬퍼 메서드가 간결합니다.단일 책임을 가진 헬퍼 메서드로 잘 분리되어 있습니다. 로그도 적절합니다.
28-30: ChallengeRepository 의존성이 누락되었습니다.코드를 보면
ChallengeRepository를 사용하지 않고 있는데,RoundDropServiceImpl(relevant snippets)에서는challengeRepository.decreaseCurrentParticipantCount()를 호출합니다.현재
RoundLifecycleServiceImpl에서는 참가자 수를 변경하지 않고 있는데, 비즈니스 로직상 올바른지 확인이 필요합니다:
- 드랍은
RoundDropServiceImpl이 담당: Line 63 주석에 따르면 STOP/UNDECIDED는 S-1일 23:59에 드랍 스케줄러가 처리한다고 명시되어 있습니다.- 라이프사이클은 전환만 담당: 이 서비스는 라운드 종료 시 CONTINUE인 사람들만 다음 라운드로 이동시키는 역할입니다.
이 책임 분리가 의도된 것이라면 문제없지만, 두 스케줄러의 실행 순서와 타이밍을 명확히 문서화하는 것이 좋겠습니다.
다음을 확인해 주세요:
- 드랍 스케줄러(
RoundDropServiceImpl.dropNonContinuersAt)와 라이프사이클 스케줄러(RoundLifecycleServiceImpl.processRoundsEndedAt)의 실행 순서가 보장되는지- 두 스케줄러가 같은 endDate에 대해 실행될 때 시간차가 있는지 (예: 드랍은 23:59, 라이프사이클은 00:00?)
- 만약 순서가 보장되지 않는다면,
processSingleEndedRound내에서 드랍 처리 후 라운드 전환을 하는 것이 더 안전할 수 있습니다스케줄러 설정 파일을 확인하여 실행 순서를 검증하는 것을 권장합니다.
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java
Outdated
Show resolved
Hide resolved
| // [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용) | ||
| if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) { |
There was a problem hiding this comment.
코드 포맷팅을 수정하세요.
Line 67의 주석이 들여쓰기 없이 컬럼 0에서 시작하고 있습니다. Java 코드 컨벤션에 맞게 들여쓰기를 적용하세요.
🔎 포맷팅 수정
-// [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용)
- if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) {
+ // [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용)
+ if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) {📝 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.
| // [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용) | |
| if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) { | |
| // [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용) | |
| if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) { |
🤖 Prompt for AI Agents
In
@src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java
around lines 67-68, The inline comment before the if check in
RoundDecisionServiceImpl is at column 0; re-indent the comment so it aligns with
the surrounding code block (match the indentation of the following if statement
that checks today.isBefore(decisionOpenDate) ||
!today.isBefore(decisionCloseDate)), preserving the comment text and formatting
to conform to Java indentation conventions.
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java
Show resolved
Hide resolved
| @Scheduled(cron = "0 10 0 * * *") | ||
| public void closeEndedRounds() { | ||
| LocalDate yesterday = LocalDate.now().minusDays(1); |
There was a problem hiding this comment.
타임존 명시 필요
스케줄러의 cron 표현식과 LocalDate.now()가 모두 명시적인 타임존 없이 시스템 기본값에 의존합니다.
문제점:
- PR 요구사항에서 Asia/Seoul 타임존 기준 23:59 drop, 00:10 lifecycle 처리가 명시되어 있음
- JVM 타임존이 다른 환경(예: UTC)에서 배포 시 잘못된 날짜로 라운드를 처리할 위험
- "어제 종료된 라운드"의 기준일이 의도와 다를 수 있음
해결 방안:
- Cron 표현식에
zone속성 추가 LocalDate.now()를LocalDate.now(ZoneId.of("Asia/Seoul"))로 변경
🔎 타임존을 명시하는 수정안
- @Scheduled(cron = "0 10 0 * * *")
+ @Scheduled(cron = "0 10 0 * * *", zone = "Asia/Seoul")
public void closeEndedRounds() {
- LocalDate yesterday = LocalDate.now().minusDays(1);
+ LocalDate yesterday = LocalDate.now(java.time.ZoneId.of("Asia/Seoul")).minusDays(1);
log.info("[RoundLifecycleScheduler] 종료 라운드 처리 시작. endDate={}", yesterday);
roundLifecycleService.processRoundsEndedAt(yesterday);
}참고: Spring Framework 5.3+ 문서의 @Scheduled 어노테이션 zone 속성을 확인하세요.
📝 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.
| @Scheduled(cron = "0 10 0 * * *") | |
| public void closeEndedRounds() { | |
| LocalDate yesterday = LocalDate.now().minusDays(1); | |
| @Scheduled(cron = "0 10 0 * * *", zone = "Asia/Seoul") | |
| public void closeEndedRounds() { | |
| LocalDate yesterday = LocalDate.now(java.time.ZoneId.of("Asia/Seoul")).minusDays(1); |
🤖 Prompt for AI Agents
In @src/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java
around lines 21-23, The scheduled task closeEndedRounds currently relies on
system default timezone for both the @Scheduled cron and LocalDate.now(); update
the @Scheduled annotation to include zone = "Asia/Seoul" and replace
LocalDate.now() with LocalDate.now(ZoneId.of("Asia/Seoul")) inside
closeEndedRounds (and add the java.time.ZoneId import) so the cron and date
calculations consistently use the Asia/Seoul timezone.
…hrr-server into feat/#192-challenge-round-period-extend
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In
@src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java:
- Around line 78-79: The commented-out save call and explanatory comment in
RoundDecisionServiceImpl should be removed: findByUserChallengeAndRoundId is
correctly defined and returns the entity used as rr, and because the method is
@Transactional, changes made by updateNextRoundIntent on rr will be persisted
via JPA dirty checking at transaction commit; delete the commented "save"
invocation and the accompanying comment to clean up the code and leave a concise
inline note (or rely on VCS history) if necessary.
In
@src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java:
- Line 38: The findAllByEndDate repository method is loading Round entities with
a lazy challenge relation outside the transactional context, causing
LazyInitializationException; update the RoundRepository's findAllByEndDate to
use a JPQL JOIN FETCH for r.challenge (i.e., replace the simple finder with a
@Query that selects r JOIN FETCH r.challenge WHERE r.endDate = :endDate and bind
the endDate param) so the challenge is eagerly fetched within the repository
call while keeping the method signature returning List<Round>.
♻️ Duplicate comments (1)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
67-68: 코드 포맷팅을 수정하세요.Line 67의 주석이 들여쓰기 없이 컬럼 0에서 시작하고 있습니다. Java 코드 컨벤션에 맞게 적절한 들여쓰기를 적용하세요.
🔎 포맷팅 수정
-// [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용) - if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) { + // [openDate, closeDate) 만 허용 (즉, openDate 당일만 허용) + if (today.isBefore(decisionOpenDate) || !today.isBefore(decisionCloseDate)) {
🧹 Nitpick comments (7)
src/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.java (1)
21-27: 타임존 이슈가 해결되었습니다! 👍이전 리뷰에서 지적된 타임존 불일치 문제가 Line 24에서
LocalDate.now(ZoneId.of("Asia/Seoul"))를 사용하여 정확히 수정되었습니다. 스케줄러의zone="Asia/Seoul"설정과 일관되게 동작합니다.추가 개선 제안:
RoundLifecycleScheduler와 이 스케줄러 모두 동일한 타임존을 사용하므로, 코드 중복을 줄이고 일관성을 높이기 위해 타임존을 상수로 추출하는 것을 고려해보세요.🔎 타임존 상수 추출 예시
공통 상수 클래스 생성:
public final class SchedulerConstants { public static final ZoneId ZONE_SEOUL = ZoneId.of("Asia/Seoul"); public static final String ZONE_SEOUL_ID = "Asia/Seoul"; private SchedulerConstants() {} }그런 다음 두 스케줄러에서 사용:
+import static com.hrr.backend.global.scheduler.SchedulerConstants.ZONE_SEOUL; +import static com.hrr.backend.global.scheduler.SchedulerConstants.ZONE_SEOUL_ID; - @Scheduled(cron = "0 59 23 * * *", zone = "Asia/Seoul") + @Scheduled(cron = "0 59 23 * * *", zone = ZONE_SEOUL_ID) public void dropNonContinuers() { - LocalDate today = LocalDate.now(ZoneId.of("Asia/Seoul")); + LocalDate today = LocalDate.now(ZONE_SEOUL);src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (3)
6-6: 추상화된 인터페이스 사용을 권장합니다.
DataSourceTransactionManager는 JDBC에 특화된 구현체입니다.PlatformTransactionManager인터페이스를 사용하면 더 나은 추상화와 유연성을 얻을 수 있습니다.🔎 제안: PlatformTransactionManager로 변경
-import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.transaction.PlatformTransactionManager;- private final DataSourceTransactionManager transactionManager; + private final PlatformTransactionManager transactionManager;이렇게 하면 JPA, JTA 등 다른 트랜잭션 매니저로 교체가 필요할 때 코드 변경 없이 설정만으로 대응할 수 있습니다.
Spring 공식 문서 참고: https://docs.spring.io/spring-framework/reference/data-access/transaction/strategies.html
Also applies to: 33-33
40-40: TransactionTemplate을 필드로 선언하여 재사용하세요.메서드 내에서 매번
TransactionTemplate을 생성하는 것보다, 필드로 선언하여 재사용하는 것이 더 효율적입니다.🔎 제안: 필드로 초기화
private final RoundRepository roundRepository; private final RoundRecordRepository roundRecordRepository; private final RoundConverter roundConverter; - private final DataSourceTransactionManager transactionManager; + private final PlatformTransactionManager transactionManager; + private final TransactionTemplate transactionTemplate; + + public RoundLifecycleServiceImpl( + RoundRepository roundRepository, + RoundRecordRepository roundRecordRepository, + RoundConverter roundConverter, + PlatformTransactionManager transactionManager) { + this.roundRepository = roundRepository; + this.roundRecordRepository = roundRecordRepository; + this.roundConverter = roundConverter; + this.transactionManager = transactionManager; + this.transactionTemplate = new TransactionTemplate(transactionManager); + } @Override public void processRoundsEndedAt(LocalDate endDate) { List<Round> endedRounds = roundRepository.findAllByEndDate(endDate); - TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); - for (Round endedRound : endedRounds) {또는
@RequiredArgsConstructor유지를 원하신다면@PostConstruct를 사용할 수도 있습니다.
87-89: 다음 라운드 생성 시 동시성 경합 가능성을 인지하세요.
findByChallengeIdAndRoundNumber+orElseGet(save)패턴에서, 동일한 챌린지에 대해 여러 프로세스가 동시에 실행된다면 중복 라운드 생성을 시도할 수 있습니다.현재 배치 처리 특성상 동시 실행 가능성이 낮고, 데이터베이스 제약조건이 중복을 방지하므로 실질적 문제는 아닙니다. 하지만 만약 높은 동시성이 예상된다면, 다음을 고려하세요:
- 데이터베이스에 UNIQUE 제약조건 추가 (challengeId, roundNumber)
- 예외 처리 시 duplicate key 예외를 특별히 처리하여 재조회
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (3)
1-29: 타임존을 클래스 상수로 정의하여 일관성을 높이세요.Line 50에서
ZoneId.of("Asia/Seoul")을 직접 사용하고 있습니다. 매직 스트링을 제거하고 클래스 레벨 상수로 정의하면 다른 서비스들(예:RoundDropScheduler,RoundLifecycleServiceImpl)과의 일관성을 확보할 수 있습니다.🔎 상수 정의 제안
@Service @RequiredArgsConstructor public class RoundDecisionServiceImpl implements RoundDecisionService { + private static final ZoneId ZONE_SEOUL = ZoneId.of("Asia/Seoul"); + private final ChallengeRepository challengeRepository; private final UserChallengeRepository userChallengeRepository; private final RoundRecordRepository roundRecordRepository;그리고 Line 50에서 사용:
- LocalDate today = LocalDate.now(ZoneId.of("Asia/Seoul")); + LocalDate today = LocalDate.now(ZONE_SEOUL);
52-58: 라운드 기간 체크 로직을 검토하세요.결정 기간(Line 64-65)이
endDate - 2(즉, 라운드 종료 2일 전)이므로, Line 56-57의today.isAfter(endDate)체크는 결정 기간 검증 시에는 항상 false가 됩니다.방어적 코딩 차원에서 유지하는 것도 좋지만, 로직의 단순성을 위해 결정 기간 체크만으로 충분할 수 있습니다. 라운드 시작 전 체크(Line 53-55)는 유용하므로 유지하는 것을 권장합니다.
82-83: 주석 처리된 코드는 제거하세요.영속성 컨텍스트의 변경 감지(dirty checking)로 충분하다는 설명은 유용하지만, 주석 처리된
save호출은 혼란을 줄 수 있습니다. 주석 처리된 코드는 버전 관리 시스템에 기록되므로 제거하는 것이 좋습니다.설명이 필요하다면 주석만 남기거나, 더 간결하게 작성하세요.
🔎 제안
옵션 1: 주석 처리된 코드만 제거
rr.updateNextRoundIntent(intent); - // rr은 영속 상태라 save 없어도 되지만, 명시적으로 해도 OK - // roundRecordRepository.save(rr); + // @Transactional에 의해 변경 감지(dirty checking)가 자동으로 적용됨 }옵션 2: 주석 완전히 제거 (변경 감지는 Spring/JPA 개발자에게 일반적인 지식)
rr.updateNextRoundIntent(intent); - // rr은 영속 상태라 save 없어도 되지만, 명시적으로 해도 OK - // roundRecordRepository.save(rr); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.javasrc/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.javasrc/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.javasrc/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.java (1)
src/main/java/com/hrr/backend/global/scheduler/RoundLifecycleScheduler.java (1)
Component(13-27)
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (2)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (1)
Service(23-85)src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (1)
Service(23-72)
src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (2)
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (1)
Service(25-115)src/main/java/com/hrr/backend/domain/round/service/RoundDropServiceImpl.java (1)
Service(23-72)
🔇 Additional comments (5)
src/main/java/com/hrr/backend/global/scheduler/RoundDropScheduler.java (1)
1-20: 클래스 구조와 의존성 주입이 올바르게 구성되었습니다.Spring 스케줄러 컴포넌트로서 필요한 어노테이션과 임포트가 적절하게 선언되었고, Lombok을 활용한 생성자 주입이 깔끔하게 적용되었습니다.
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (2)
35-51: 훌륭한 개선입니다! 이전 리뷰의 트랜잭션 문제를 효과적으로 해결했습니다.
TransactionTemplate을 사용하여 각 라운드를 독립적인 트랜잭션으로 처리하도록 변경한 점이 excellent합니다. 이제:
- 부분 실패 시에도 성공한 라운드는 정상적으로 커밋됩니다
- 재실행 시 멱등성 체크가 제대로 작동합니다
- 에러 격리가 명확해졌습니다
이전 리뷰에서 제안한
@Transactional(propagation = REQUIRES_NEW)대신 프로그래매틱 방식을 선택하셨는데, 동일한 효과를 달성하는 유효한 대안입니다.
96-98: 멱등성 체크가 적절합니다. 👍
existsByUserChallengeAndRound체크로 재실행 시 중복 레코드 생성을 방지하는 방어 로직이 잘 구현되어 있습니다.src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java (2)
72-76: 깔끔한 유효성 검증이네요! 👍
UNDECIDED로의 명시적 요청을 방지하는 로직이 비즈니스 의도를 정확히 반영하고 있습니다.
41-43: 현재 에러 코드 사용이 적절합니다.
USER_CHALLENGE_NOT_FOUND의 메시지는 "해당 챌린지에 참가하지 않았습니다."로, JOINED 상태가 아닐 때의 상황을 정확하게 표현하고 있습니다. ErrorCode 전체를 검토한 결과,USER_NOT_PARTICIPATING이나USER_CHALLENGE_INVALID_STATUS같은 더 구체적인 에러 코드는 존재하지 않으며, 현재 구현은 의미적으로도 코드 메시지 측면에서도 적절합니다.
| RoundRecord rr = roundRecordRepository.findByUserChallengeAndRoundId(uc, currentRound.getId()) | ||
| .orElseThrow(() -> new GlobalException(ErrorCode.ROUND_RECORD_NOT_FOUND)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "RoundRecordRepository" --type fRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 158
🏁 Script executed:
# RoundRecordRepository 파일 내용 확인
find . -name "*RoundRecordRepository*" -type f | head -5Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 160
🏁 Script executed:
# Round 관련 엔티티 구조 파악
fd -e java -path "*/domain/round/*" | grep -E "(Repository|Entity)" | head -20Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 246
🏁 Script executed:
cat -n src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.javaRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 3394
🏁 Script executed:
# RoundRecord 엔티티 파일 찾기
fd "RoundRecord" --extension java | grep -v Repository | grep -v TestRepository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 144
🏁 Script executed:
# UserChallenge 엔티티 파일 찾기
fd "UserChallenge" --extension java | grep -E "(Entity|domain)" | head -3Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 303
🏁 Script executed:
cat -n src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java | sed -n '70,90p'Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 792
🏁 Script executed:
# RoundRecord 엔티티 구조 확인 (필드 이름 검증용)
cat -n src/main/java/com/hrr/backend/domain/round/entity/RoundRecord.java | head -50Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 1915
주석 처리된 코드를 정리하세요.
findByUserChallengeAndRoundId는 이미 @Query로 명시적으로 정의되어 있어 정상 작동합니다. 다만 라인 83의 주석 처리된 save 호출과 설명 주석은 정리가 필요합니다.
@Transactional 메서드 내에서 updateNextRoundIntent로 엔티티를 변경하면 트랜잭션 종료 시 Dirty Checking에 의해 자동으로 저장되므로, 주석 처리된 save 호출은 삭제하고 설명 주석도 코드 리뷰나 문서에서 명확히 정리하는 것이 좋습니다. 주석 처리된 코드는 버전 관리 히스토리를 의도하지 않는 한 제거하는 것이 코드 가독성을 높입니다.
🤖 Prompt for AI Agents
In
@src/main/java/com/hrr/backend/domain/round/service/RoundDecisionServiceImpl.java
around lines 78-79, The commented-out save call and explanatory comment in
RoundDecisionServiceImpl should be removed: findByUserChallengeAndRoundId is
correctly defined and returns the entity used as rr, and because the method is
@Transactional, changes made by updateNextRoundIntent on rr will be persisted
via JPA dirty checking at transaction commit; delete the commented "save"
invocation and the accompanying comment to clean up the code and leave a concise
inline note (or rely on VCS history) if necessary.
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (2)
35-57: 트랜잭션 격리 패턴은 좋지만, TransactionTemplate 재사용을 고려하세요.각 라운드를 독립 트랜잭션으로 처리하는 패턴과 detached 엔티티 방지를 위한 재조회 로직이 과거 리뷰 피드백을 잘 반영했습니다. 다만, Line 40에서 메서드 호출마다
TransactionTemplate인스턴스를 새로 생성하고 있는데, 필드로 추출하여 재사용하는 것이 더 효율적입니다.🔎 TransactionTemplate을 필드로 추출하는 리팩토링 제안
@Service @RequiredArgsConstructor @Slf4j public class RoundLifecycleServiceImpl implements RoundLifecycleService { private final RoundRepository roundRepository; private final RoundRecordRepository roundRecordRepository; private final RoundConverter roundConverter; - private final DataSourceTransactionManager transactionManager; + private final TransactionTemplate transactionTemplate; + + public RoundLifecycleServiceImpl( + RoundRepository roundRepository, + RoundRecordRepository roundRecordRepository, + RoundConverter roundConverter, + DataSourceTransactionManager transactionManager + ) { + this.roundRepository = roundRepository; + this.roundRecordRepository = roundRecordRepository; + this.roundConverter = roundConverter; + this.transactionTemplate = new TransactionTemplate(transactionManager); + } @Override public void processRoundsEndedAt(LocalDate endDate) { List<Round> endedRounds = roundRepository.findAllByEndDate(endDate); - TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); - for (Round endedRound : endedRounds) { // ... 나머지 로직
82-84: [선택사항] 데이터베이스 레벨 필터링으로 성능 개선 가능.현재 Java 스트림으로
CONTINUE의도를 필터링하고 있습니다. 참가자 수가 많을 경우RoundRecordRepository에JOINED+CONTINUE조건을 함께 조회하는 쿼리 메서드를 추가하면 불필요한 데이터 전송을 줄일 수 있습니다. 현재 구현도 정상 동작하므로 선택적 개선사항입니다.🔎 데이터베이스 레벨 필터링 제안
RoundRecordRepository에 다음 메서드 추가:
@Query("SELECT rr FROM RoundRecord rr " + "JOIN FETCH rr.userChallenge uc " + "JOIN FETCH uc.user " + "JOIN FETCH uc.challengeSetting " + "WHERE rr.round = :round " + "AND uc.status = :status " + "AND rr.nextRoundIntent = :intent") List<RoundRecord> findAllByRoundWithUserAndSettingAndIntent( @Param("round") Round round, @Param("status") ChallengeJoinStatus status, @Param("intent") NextRoundIntent intent );그리고 서비스 코드 수정:
-List<RoundRecord> records = roundRecordRepository.findAllByRoundWithUserAndSetting( - endedRound, - ChallengeJoinStatus.JOINED -); - -List<RoundRecord> continuers = records.stream() - .filter(rr -> rr.getNextRoundIntent() == NextRoundIntent.CONTINUE) - .toList(); +List<RoundRecord> continuers = roundRecordRepository.findAllByRoundWithUserAndSettingAndIntent( + endedRound, + ChallengeJoinStatus.JOINED, + NextRoundIntent.CONTINUE +);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/hrr/backend/domain/round/repository/RoundRepository.javasrc/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java
🔇 Additional comments (5)
src/main/java/com/hrr/backend/domain/round/repository/RoundRepository.java (2)
44-53: 과거 리뷰 피드백이 정확히 반영되었습니다!JOIN FETCH를 통해
challenge와currentRound를 즉시 로딩하도록 수정하여,LazyInitializationException이 발생하지 않도록 개선되었습니다. LEFT JOIN FETCH 사용도 적절합니다(currentRound는 null일 수 있음).
55-62: 트랜잭션 내 재조회 패턴을 위한 메서드 추가가 적절합니다.detached 엔티티 문제를 방지하기 위해 트랜잭션 내부에서 필요한 연관 관계를 즉시 로딩하는 전용 메서드를 추가한 것은 좋은 설계입니다. 메서드명도 로딩되는 연관 관계를 명확히 나타내고 있습니다.
src/main/java/com/hrr/backend/domain/round/service/RoundLifecycleServiceImpl.java (3)
1-34: 의존성 구조와 트랜잭션 관리 방식이 적절합니다.과거 리뷰에서 권장했던 독립 트랜잭션 처리를 위해
DataSourceTransactionManager와TransactionTemplate을 사용하는 구조로 설계되었습니다. 클래스 레벨@Transactional을 사용하지 않고 프로그래매틱 트랜잭션을 선택한 것이 각 라운드별 격리를 위한 올바른 접근입니다.
117-120: 명확한 헬퍼 메서드입니다.챌린지 종료 처리를 담당하는 단순하고 집중된 메서드입니다. 메서드명
finishChallengeOnly가 다른 정리 작업은 수행하지 않음을 명확히 나타냅니다.
59-115:findAllByRoundWithUserAndSetting메서드가 UserChallenge를 제대로 즉시 로딩하고 있습니다.쿼리 구현을 확인해보니
JOIN FETCH rr.userChallenge uc로 UserChallenge를 명시적으로 즉시 로딩하고 있으며, 추가로 User와 NotificationSetting까지 함께 로딩하고 있습니다. 이렇게 구성하면 라인 99의getUserChallenge()호출 시 지연 로딩이 발생하지 않아 N+1 문제가 없습니다.전체적으로 비즈니스 로직이 견고하게 구현되었습니다. 멱등성 체크(라인 65, 102), 상태 검증(라인 70), 스트림 기반의 깔끔한 필터링(라인 82-84), orElseGet 패턴의 find-or-create 로직(라인 93-95) 등 각 단계가 잘 설계되어 있습니다.
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
새 기능
개선
테스트
설정
✏️ Tip: You can customize this high-level summary in your review settings.