Open
Conversation
Walkthrough경기 날짜/시간 기준으로 매칭 완료·실패를 판단하는 스케줄러와 관련 리포지토리·서비스·엔티티·테스트 및 그룹 생성 흐름이 추가됩니다. 스케줄러는 분 단위로 실행되어 대상 스케줄을 조회하고 그룹/멤버 상태를 일괄 갱신합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as MatchScheduler
participant Repo as SchedulerRepository
participant FailSvc as GroupMatchFailService
participant CompleteSvc as GroupMatchCompleteService
participant MemberRepo as GroupMemberRepository
participant DB as Database
Scheduler->>Repo: findAllDueSchedules(now)
Repo->>DB: SELECT MatchSchedule WHERE executed = false AND executeTime <= now
DB-->>Repo: MatchSchedule 목록
Repo-->>Scheduler: 대상 스케줄 반환
loop for each MatchSchedule
alt schedule.type == FAIL
Scheduler->>FailSvc: validateFailStatus(groupId)
FailSvc->>MemberRepo: countGroupMember(groupId)
MemberRepo->>DB: SELECT COUNT(...)
DB-->>MemberRepo: count
MemberRepo-->>FailSvc: count
alt count < 2
FailSvc->>MemberRepo: updateAllStatusFail(groupId)
MemberRepo->>DB: UPDATE GroupMember.status = MATCH_FAILED
MemberRepo->>DB: UPDATE Group.status = FAILED
end
else schedule.type == COMPLETE
Scheduler->>CompleteSvc: validateCompleteStatus(groupId)
CompleteSvc->>MemberRepo: countGroupMember(groupId)
MemberRepo->>DB: SELECT COUNT(...)
DB-->>MemberRepo: count
MemberRepo-->>CompleteSvc: count
alt group.isGroup && count >= 2
CompleteSvc->>MemberRepo: updateAllStatusComplete(groupId)
MemberRepo->>DB: UPDATE GroupMember.status = MATCHED
MemberRepo->>DB: UPDATE Group.status = COMPLETED
end
end
Scheduler->>Repo: markExecuted() (MatchSchedule.executed = true)
Repo->>DB: UPDATE MatchSchedule.executed = true
end
sequenceDiagram
participant Client as Caller
participant Executor as GroupExecutorV3
participant EM as EntityManager
participant ChatSvc as ChattingV2Service
participant SchedulerRepo as SchedulerRepository
participant DB as Database
Client->>Executor: createGroup(userId, gameId, isGroup)
Executor->>EM: find User by userId
EM->>DB: SELECT User
DB-->>EM: User
Executor->>EM: find GameInformation by gameId
EM->>DB: SELECT GameInformation
DB-->>EM: GameInformation
Executor->>ChatSvc: assignChatting()
ChatSvc-->>Executor: Chatting (or null -> error)
Executor->>EM: persist Group (with chatting)
EM->>DB: INSERT Group
DB-->>EM: generated id
Executor->>EM: persist GroupMember (leader, PENDING_REQUEST)
EM->>DB: INSERT GroupMember
Executor->>SchedulerRepo: save MatchSchedule(group, gameTime.minusMinutes(1), FAIL)
SchedulerRepo->>DB: INSERT MatchSchedule
Executor->>SchedulerRepo: save MatchSchedule(group, gameTime, COMPLETE)
SchedulerRepo->>DB: INSERT MatchSchedule
Executor-->>Client: return group.id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 7
🧹 Nitpick comments (7)
src/main/java/at/mateball/domain/group/core/GroupStatus.java (1)
37-43:from()메서드를 재사용하여 중복 코드 제거를 고려해보세요.
GroupMemberStatus와 마찬가지로from(int value)메서드를 재사용할 수 있습니다.♻️ 리팩토링 제안
public static String labelOf(int value) { - return Arrays.stream(values()) - .filter(status -> status.value == value) - .map(status -> status.label) - .findFirst() - .orElseThrow(() -> new BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM)); + return from(value).getLabel(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/at/mateball/domain/group/core/GroupStatus.java` around lines 37 - 43, Replace the manual lookup in labelOf(int value) with a call to the existing from(int value) to avoid duplication: call GroupStatus.from(value) to get the enum instance, then return its label; ensure you keep the same error behavior (the from method will throw BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM) on unknown values) and reference the enum methods labelOf and from in GroupStatus.src/main/java/at/mateball/domain/groupmember/GroupMemberStatus.java (1)
33-39:from()메서드를 재사용하여 중복 코드 제거를 고려해보세요.
from(int value)메서드와 동일한 스트림 로직이 중복되어 있습니다. 다음과 같이 간소화할 수 있습니다:♻️ 리팩토링 제안
public static String labelOf(int value) { - return Arrays.stream(values()) - .filter(status -> status.value == value) - .map(status -> status.label) - .findFirst() - .orElseThrow(() -> new BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM)); + return from(value).getLabel(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/at/mateball/domain/groupmember/GroupMemberStatus.java` around lines 33 - 39, Replace the duplicated stream logic in labelOf(int value) by delegating to the existing from(int value) method and returning the label from that enum instance (e.g., return from(value).getLabel()); ensure getLabel() (or the label field) is accessible and that from(int value) continues to throw BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM) for unknown values so behavior is preserved.src/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepositoryImpl.java (1)
26-36: 스케줄 처리 순서 보장을 위해 ORDER BY 추가 권장
findAllDueSchedules에서 조회된 스케줄의 처리 순서가 보장되지 않습니다. 동일 그룹에 대해 FAIL과 COMPLETE 스케줄이 동시에 due 상태가 되는 경우, 처리 순서에 따라 결과가 달라질 수 있습니다.♻️ executeTime 기준 정렬 추가 제안
return queryFactory .selectFrom(matchSchedule) .where( matchSchedule.executed.isFalse(), matchSchedule.executeTime.loe(now) ) + .orderBy(matchSchedule.executeTime.asc()) .fetch();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepositoryImpl.java` around lines 26 - 36, The query in findAllDueSchedules (SchedulerRepositoryImpl) does not guarantee processing order; modify the QueryDSL pipeline on QMatchSchedule.matchSchedule to append an orderBy using matchSchedule.executeTime.asc() (and optionally a tie-breaker like matchSchedule.id.asc() or createdAt.asc()) so due schedules are returned in deterministic chronological order before .fetch().src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.java (1)
40-42: 에러 로깅 시 lazy loading 예외 가능성catch 블록에서
s.getGroup().getId()호출 시, 이전 처리에서 예외가 발생한 경우 영속성 컨텍스트 상태에 따라LazyInitializationException이 발생할 수 있습니다. 안전한 로깅을 위해 스케줄 ID를 사용하거나, try 블록 진입 전에 groupId를 미리 추출하는 것이 좋습니다.♻️ 안전한 로깅을 위한 수정
for (MatchSchedule s : targets) { + Long groupId = s.getGroup().getId(); try { if (s.getType().isFail()) { - matchFailService.validateFailStatus(s.getGroup().getId()); + matchFailService.validateFailStatus(groupId); } else { - matchCompleteService.validateCompleteStatus(s.getGroup().getId()); + matchCompleteService.validateCompleteStatus(groupId); } s.markExecuted(); } catch (Exception e) { - log.error("스케줄 처리 실패 groupId={}", s.getGroup().getId(), e); + log.error("스케줄 처리 실패 groupId={}", groupId, e); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.java` around lines 40 - 42, catch 블록에서 s.getGroup().getId() 호출은 이전 예외로 인해 영속성 컨텍스트가 닫혀 LazyInitializationException을 일으킬 수 있으니, 예외 처리 전에 Group id를 미리 추출하거나(예: long groupId = s.getGroup().getId(); try { ... } ) 아니면 로그에 schedule id(s.getId())만 사용하도록 변경하세요; MatchScheduler의 해당 catch 블록에서 s.getGroup().getId() 참조를 제거하고 미리 저장한 groupId 또는 s.getId()로 대체하세요.src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java (2)
91-93: @DisplayName 중복Line 40과 Line 92에서 동일한
@DisplayName("스케줄 실행 시 FAIL 처리")가 사용되어 테스트 리포트에서 구분이 어렵습니다.♻️ 구분되는 DisplayName 사용
`@Test` - `@DisplayName`("스케줄 실행 시 FAIL 처리") + `@DisplayName`("스케줄 실행 시 FAIL 처리 - 생성자만 존재하고 승인 대기자가 있는 경우") void 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` around lines 91 - 93, The test class MatchSchedulerIntegrationTest has duplicate `@DisplayName`("스케줄 실행 시 FAIL 처리") entries (one is on the method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 and another earlier in the file), so update one of the `@DisplayName` annotations to a unique, descriptive string that distinguishes the scenarios (e.g., include the condition like "그룹원 생성자만 있고 승인 대기자 존재 시 FAIL 처리") so test reports are unambiguous; locate and change the duplicate `@DisplayName` on the method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 or the other test to a clearer label.
85-86: 스케줄 실행 후 GroupMember 상태 검증 추가 권장현재 테스트는
isExecuted()만 검증하고 있습니다. 스케줄러의 핵심 기능인GroupMember상태 변경(매칭 실패/완료)이 실제로 적용되었는지 검증하면 테스트 커버리지가 향상됩니다.♻️ 상태 검증 추가 예시 (FAIL 테스트)
// FAIL 테스트 then 블록에 추가 List<GroupMember> members = em.createQuery( "select gm from GroupMember gm where gm.group.id = :groupId", GroupMember.class) .setParameter("groupId", group.getId()) .getResultList(); for (GroupMember m : members) { assertThat(m.getStatus()).isEqualTo(GroupMemberStatus.MATCHING_FAILED.getValue()); }Also applies to: 141-142, 199-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` around lines 85 - 86, After asserting that MatchSchedule result.isExecuted() is true, also query GroupMember entities for the affected group (using the EntityManager em and the group's id from the test fixture) and assert their status values match the expected outcome; specifically, in the FAIL scenario assert each GroupMember.getStatus() equals GroupMemberStatus.MATCHING_FAILED.getValue(), and in the SUCCESS scenario assert they equal the success enum/value (e.g., GroupMemberStatus.MATCHED.getValue()); add these checks in the corresponding test then blocks next to the existing MatchSchedule assertion to verify the scheduler actually updated GroupMember state.src/main/java/at/mateball/domain/group/scheduler/MatchSchedule.java (1)
28-29:executed필드 초기화 중복 (nitpick)
executed필드가 line 29에서false로 초기화되고, 생성자(line 38)에서도false로 재설정됩니다. 동작에는 문제가 없지만, 생성자 내 중복 할당을 제거할 수 있습니다.♻️ 중복 초기화 제거
public MatchSchedule(Group group, LocalDateTime executeTime, ScheduleType type) { this.group = group; this.executeTime = executeTime; this.type = type; - this.executed = false; }Also applies to: 34-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/at/mateball/domain/group/scheduler/MatchSchedule.java` around lines 28 - 29, Remove the redundant initialization of the executed field in the MatchSchedule constructor: keep the field-level default (private boolean executed = false;) and delete the duplicate assignment (this.executed = false;) from the MatchSchedule(...) constructor body; apply the same cleanup for any other fields that are initialized both at declaration and again inside that constructor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/at/mateball/domain/group/core/GroupExecutorV3.java`:
- Around line 57-58: The two MatchSchedule entries saved in GroupExecutorV3
(MatchSchedule(..., gameDateTime.minusMinutes(1), ScheduleType.FAIL) and
MatchSchedule(..., gameDateTime, ScheduleType.COMPLETE)) can become due at the
same scheduler run and lead to nondeterministic processing because
SchedulerRepositoryImpl.findAllDueSchedules() has no ORDER BY; update the
repository query to enforce deterministic ordering (e.g., ORDER BY scheduledAt
ASC, scheduleType ASC or use a CASE to prioritize ScheduleType.FAIL before
COMPLETE) so due schedules are returned in a stable, intended order, or
alternatively adjust the FAIL timing creation in GroupExecutorV3 to guarantee at
least one scheduler interval gap—choose the repository ORDER BY approach and add
the ordering to SchedulerRepositoryImpl.findAllDueSchedules().
In
`@src/main/java/at/mateball/domain/group/scheduler/GroupMatchCompleteService.java`:
- Around line 27-29: The COMPLETE handling is skipped for non-group matches due
to the isGroup() guard in GroupMatchCompleteService.validateCompleteStatus(),
causing inconsistency with GroupExecutorV3.createGroup() which always creates a
COMPLETE schedule; remove the isGroup() condition so validateCompleteStatus uses
only the memberCount >= MIN_PARTICIPANT_FOR_COMPLETE check and still calls
groupMemberRepository.updateAllStatusComplete(groupId) (symbols:
GroupMatchCompleteService.validateCompleteStatus, GroupExecutorV3.createGroup,
schedulerRepository.save, MatchSchedule, ScheduleType.COMPLETE,
groupMemberRepository.updateAllStatusComplete, isGroup()) to ensure COMPLETE
schedules for non-group matches are processed the same as group matches; update
related tests to cover non-group COMPLETE processing.
In `@src/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.java`:
- Around line 53-59: 메서드 validateFailStatus(Long groupId)에 `@Transactional` 애노테이션을
추가하고, 1:1 매칭을 잘못 처리하지 않도록 그룹이 실제로 그룹인지 확인하도록 수정하세요:
GroupMatchFailService.validateFailStatus에서 groupId로 그룹을 로드(예:
groupRepository.findById(groupId) 또는 기존 그룹 조회 메서드 사용), Optional 처리 후 그룹의
isGroup()이 true인 경우에만 기존 로직(participantCount 계산 및
groupMemberRepository.updateAllStatusFail(groupId))을 실행하도록 변경하고, 그룹이 없을 때의 예외/무시
처리도 일관되게 적용하세요.
In `@src/main/java/at/mateball/domain/group/scheduler/MatchSchedule.java`:
- Around line 17-19: Group 삭제 시 MatchSchedule 엔티티가 orphan으로 남아 LAZY 로딩에서
s.getGroup().getId() 호출 시 NPE/제약 위반이 발생할 수 있으므로 MatchSchedule-Group 연관관계를 정리하세요:
Group 엔티티에 MatchSchedule에 대한 `@OneToMany`(mappedBy="group", cascade =
CascadeType.REMOVE 또는 CascadeType.ALL, orphanRemoval = true) 설정을 추가하거나,
GroupRepository.deleteAllByLeaderId() 호출 전에 MatchScheduleRepository에서 해당 그룹의
스케줄을 명시적으로 삭제하는 로직을 구현해 MatchScheduler.run()이 null 그룹을 참조하지 않도록 하세요 (참조할 식별자:
class MatchSchedule, field group, method MatchScheduler.run(), method
GroupRepository.deleteAllByLeaderId()).
In `@src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.java`:
- Around line 30-42: The current loop in MatchScheduler leaves failed schedules
unmarked causing indefinite reprocessing; update behavior by adding a retry
mechanism and explicit failure marking: extend MatchSchedule with fields
retryCount and maxRetries (and a markFailed()/setFailedReason() method),
increment retryCount when matchFailService.validateFailStatus(...) or
matchCompleteService.validateCompleteStatus(...) throws, and only call
s.markExecuted() on success or when retryCount > maxRetries; on final failure
call s.markFailed(...) (or set failed flag/reason) and log accordingly. Ensure
MatchScheduler references these symbols (MatchSchedule.retryCount/maxRetries,
markFailed, markExecuted) and persist the updated state so retries stop after
maxRetries.
In
`@src/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepository.java`:
- Around line 7-8: The repository generic is wrong: SchedulerRepository
currently extends JpaRepository<Group, Long> but it actually persists
MatchSchedule instances; change the generic to JpaRepository<MatchSchedule,
Long> and add/import the MatchSchedule entity so save/load calls from
SchedulerRepository (and usages in GroupExecutorV3 and tests that call
schedulerRepository.save(new MatchSchedule(...))) type-check and work at
runtime; keep the SchedulerCustom extension unchanged.
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java`:
- Around line 173-176: The test incorrectly reuses userB for both GroupMember
instances (m1 and m2) and omits the leader record; update the test so m1 and m2
reference two distinct users (e.g., keep m1 as new GroupMember(userB, group,
true, GroupMemberStatus.PENDING_REQUEST.getValue(), true) and change m2 to use a
different user variable like userC or userD) and add a GroupMember for the
leader (create and persist a GroupMember for userA with the leader flag/status
appropriate to your domain) so the dataset models three distinct participants
(leader + two members) matching the business logic.
---
Nitpick comments:
In `@src/main/java/at/mateball/domain/group/core/GroupStatus.java`:
- Around line 37-43: Replace the manual lookup in labelOf(int value) with a call
to the existing from(int value) to avoid duplication: call
GroupStatus.from(value) to get the enum instance, then return its label; ensure
you keep the same error behavior (the from method will throw
BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM) on unknown values) and
reference the enum methods labelOf and from in GroupStatus.
In `@src/main/java/at/mateball/domain/group/scheduler/MatchSchedule.java`:
- Around line 28-29: Remove the redundant initialization of the executed field
in the MatchSchedule constructor: keep the field-level default (private boolean
executed = false;) and delete the duplicate assignment (this.executed = false;)
from the MatchSchedule(...) constructor body; apply the same cleanup for any
other fields that are initialized both at declaration and again inside that
constructor.
In `@src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.java`:
- Around line 40-42: catch 블록에서 s.getGroup().getId() 호출은 이전 예외로 인해 영속성 컨텍스트가 닫혀
LazyInitializationException을 일으킬 수 있으니, 예외 처리 전에 Group id를 미리 추출하거나(예: long
groupId = s.getGroup().getId(); try { ... } ) 아니면 로그에 schedule id(s.getId())만
사용하도록 변경하세요; MatchScheduler의 해당 catch 블록에서 s.getGroup().getId() 참조를 제거하고 미리 저장한
groupId 또는 s.getId()로 대체하세요.
In
`@src/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepositoryImpl.java`:
- Around line 26-36: The query in findAllDueSchedules (SchedulerRepositoryImpl)
does not guarantee processing order; modify the QueryDSL pipeline on
QMatchSchedule.matchSchedule to append an orderBy using
matchSchedule.executeTime.asc() (and optionally a tie-breaker like
matchSchedule.id.asc() or createdAt.asc()) so due schedules are returned in
deterministic chronological order before .fetch().
In `@src/main/java/at/mateball/domain/groupmember/GroupMemberStatus.java`:
- Around line 33-39: Replace the duplicated stream logic in labelOf(int value)
by delegating to the existing from(int value) method and returning the label
from that enum instance (e.g., return from(value).getLabel()); ensure getLabel()
(or the label field) is accessible and that from(int value) continues to throw
BusinessException(BusinessErrorCode.BAD_REQUEST_ENUM) for unknown values so
behavior is preserved.
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java`:
- Around line 91-93: The test class MatchSchedulerIntegrationTest has duplicate
`@DisplayName`("스케줄 실행 시 FAIL 처리") entries (one is on the method
그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 and another earlier in the file), so update
one of the `@DisplayName` annotations to a unique, descriptive string that
distinguishes the scenarios (e.g., include the condition like "그룹원 생성자만 있고 승인
대기자 존재 시 FAIL 처리") so test reports are unambiguous; locate and change the
duplicate `@DisplayName` on the method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 or the
other test to a clearer label.
- Around line 85-86: After asserting that MatchSchedule result.isExecuted() is
true, also query GroupMember entities for the affected group (using the
EntityManager em and the group's id from the test fixture) and assert their
status values match the expected outcome; specifically, in the FAIL scenario
assert each GroupMember.getStatus() equals
GroupMemberStatus.MATCHING_FAILED.getValue(), and in the SUCCESS scenario assert
they equal the success enum/value (e.g., GroupMemberStatus.MATCHED.getValue());
add these checks in the corresponding test then blocks next to the existing
MatchSchedule assertion to verify the scheduler actually updated GroupMember
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1231789d-fbf2-403e-ad95-ba35c022d644
📒 Files selected for processing (14)
src/main/java/at/mateball/domain/group/core/GroupExecutorV3.javasrc/main/java/at/mateball/domain/group/core/GroupStatus.javasrc/main/java/at/mateball/domain/group/scheduler/GroupMatchCompleteService.javasrc/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.javasrc/main/java/at/mateball/domain/group/scheduler/MatchSchedule.javasrc/main/java/at/mateball/domain/group/scheduler/MatchScheduler.javasrc/main/java/at/mateball/domain/group/scheduler/ScheduleType.javasrc/main/java/at/mateball/domain/group/scheduler/repository/SchedulerCustom.javasrc/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepository.javasrc/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepositoryImpl.javasrc/main/java/at/mateball/domain/groupmember/GroupMemberStatus.javasrc/main/java/at/mateball/domain/groupmember/core/repository/querydsl/GroupMemberRepositoryCustom.javasrc/main/java/at/mateball/domain/groupmember/core/repository/querydsl/GroupMemberRepositoryImpl.javasrc/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java
Comment on lines
+57
to
+58
| schedulerRepository.save(new MatchSchedule(group, gameDateTime.minusMinutes(1), ScheduleType.FAIL)); | ||
| schedulerRepository.save(new MatchSchedule(group, gameDateTime, ScheduleType.COMPLETE)); |
There was a problem hiding this comment.
FAIL과 COMPLETE 스케줄 동시 실행 가능성
FAIL 스케줄은 gameDateTime - 1분, COMPLETE 스케줄은 gameDateTime에 등록됩니다. 스케줄러가 1분 간격으로 실행되므로, 스케줄러 실행 시점에 따라 두 스케줄이 동시에 due 상태가 될 수 있습니다. SchedulerRepositoryImpl.findAllDueSchedules()에 ORDER BY가 없으면 처리 순서가 보장되지 않아 의도치 않은 결과가 발생할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/at/mateball/domain/group/core/GroupExecutorV3.java` around
lines 57 - 58, The two MatchSchedule entries saved in GroupExecutorV3
(MatchSchedule(..., gameDateTime.minusMinutes(1), ScheduleType.FAIL) and
MatchSchedule(..., gameDateTime, ScheduleType.COMPLETE)) can become due at the
same scheduler run and lead to nondeterministic processing because
SchedulerRepositoryImpl.findAllDueSchedules() has no ORDER BY; update the
repository query to enforce deterministic ordering (e.g., ORDER BY scheduledAt
ASC, scheduleType ASC or use a CASE to prioritize ScheduleType.FAIL before
COMPLETE) so due schedules are returned in a stable, intended order, or
alternatively adjust the FAIL timing creation in GroupExecutorV3 to guarantee at
least one scheduler interval gap—choose the repository ORDER BY approach and add
the ordering to SchedulerRepositoryImpl.findAllDueSchedules().
Comment on lines
+53
to
+59
| public void validateFailStatus(Long groupId) { | ||
| int participantCount = groupMemberRepository.countGroupMember(groupId).count(); | ||
|
|
||
| if (participantCount < MIN_PARTICIPANT_FOR_COMPLETE) { | ||
| groupMemberRepository.updateAllStatusFail(groupId); | ||
| } | ||
| } |
There was a problem hiding this comment.
@Transactional 어노테이션 누락 및 isGroup 조건 검사 누락
두 가지 문제가 있습니다:
-
@transactional 누락:
GroupMatchCompleteService.validateCompleteStatus()는@Transactional이 있지만, 이 메서드에는 없습니다. 일관성을 위해 추가해야 합니다. -
isGroup 조건 누락:
GroupMatchCompleteService에서는group.isGroup() && memberCount >= MIN_PARTICIPANT_FOR_COMPLETE조건을 검사하지만, 이 메서드에서는isGroup검사가 없습니다. 이로 인해 1:1 직접 매칭(non-group)도 실패 처리될 수 있습니다.
🐛 수정 제안
+@Transactional
public void validateFailStatus(Long groupId) {
+ Group group = groupRepository.findById(groupId)
+ .orElseThrow(() -> new BusinessException(BusinessErrorCode.GROUP_NOT_FOUND));
+
int participantCount = groupMemberRepository.countGroupMember(groupId).count();
- if (participantCount < MIN_PARTICIPANT_FOR_COMPLETE) {
+ if (group.isGroup() && participantCount < MIN_PARTICIPANT_FOR_COMPLETE) {
groupMemberRepository.updateAllStatusFail(groupId);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.java`
around lines 53 - 59, 메서드 validateFailStatus(Long groupId)에 `@Transactional`
애노테이션을 추가하고, 1:1 매칭을 잘못 처리하지 않도록 그룹이 실제로 그룹인지 확인하도록 수정하세요:
GroupMatchFailService.validateFailStatus에서 groupId로 그룹을 로드(예:
groupRepository.findById(groupId) 또는 기존 그룹 조회 메서드 사용), Optional 처리 후 그룹의
isGroup()이 true인 경우에만 기존 로직(participantCount 계산 및
groupMemberRepository.updateAllStatusFail(groupId))을 실행하도록 변경하고, 그룹이 없을 때의 예외/무시
처리도 일관되게 적용하세요.
Collaborator
Author
There was a problem hiding this comment.
맞습니다, 일대일과 그룹매칭 모두 MIN_PARTICIPANT_FOR_COMPLETE 이하일때 fail 로직이 실행되어야 하므로, 그룹여부는 검증하지 않았습니다.
There was a problem hiding this comment.
The PR description image shows test execution results for the MatchScheduler, with FAIL and COMPLETE scenarios running successfully — including cases for both a solo creator and a creator with a pending requester being marked as failed, and a group with two participants marked as complete.
@hyeeum, 설명 감사합니다! 일대일 매칭과 그룹 매칭 모두 참여자 수가 부족할 경우 동일하게 실패 처리되어야 한다는 점은 이해했습니다. isGroup 검사를 생략한 것은 의도된 설계로 철회합니다.
다만, @Transactional 누락 이슈는 여전히 유효합니다. validateFailStatus()는 updateAllStatusFail()을 호출하여 DB 쓰기 작업을 수행하는데, @Transactional이 없으면 트랜잭션 경계가 보장되지 않습니다. (참고로 GroupMatchCompleteService.validateCompleteStatus()에는 @Transactional이 있습니다.) 추가를 검토해 주세요.
`@Transactional`
public void validateFailStatus(Long groupId) {
int participantCount = groupMemberRepository.countGroupMember(groupId).count();
if (participantCount < MIN_PARTICIPANT_FOR_COMPLETE) {
groupMemberRepository.updateAllStatusFail(groupId);
}
}✏️ Learnings added
Learnt from: hyeeum
Repo: MATEBALL/MATEBALL-SERVER PR: 250
File: src/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.java:53-59
Timestamp: 2026-03-20T10:10:35.829Z
Learning: In MATEBALL-SERVER, `GroupMatchFailService.validateFailStatus()` intentionally does NOT check `isGroup()` before running fail logic. Both 1:1 and group matches are treated the same for failure: if participantCount < MIN_PARTICIPANT_FOR_COMPLETE (2), the match is marked as failed. The `isGroup` check in `GroupMatchCompleteService.validateCompleteStatus()` is specific to the complete path only.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: hyeeum
Repo: MATEBALL/MATEBALL-SERVER PR: 238
File: src/main/java/at/mateball/domain/group/core/validator/DateValidator.java:25-33
Timestamp: 2026-03-17T04:16:00.534Z
Learning: In MATEBALL-SERVER, the "+2 days" restriction (`BAD_REQUEST_DATE`) in `DateValidator.validate()` (src/main/java/at/mateball/domain/group/core/validator/DateValidator.java) is intentionally commented out as of PR `#238` because this exception condition is currently not applicable across any caller (including GroupService.validateRequest and GameInformationService.getGameInformation), not just the V3 matching creation API. Do not flag the commented-out `BAD_REQUEST_DATE` logic as a missing or improperly scoped validation.
Learnt from: hyeeum
Repo: MATEBALL/MATEBALL-SERVER PR: 238
File: src/main/java/at/mateball/domain/group/core/GroupExecutorV3.java:31-33
Timestamp: 2026-03-17T04:15:08.936Z
Learning: In MATEBALL-SERVER (GroupV3Service + GroupExecutorV3), game existence validation is performed in the service layer before calling GroupExecutorV3.createGroup(). Specifically, groupV3RepositoryCustom.getMatchValidationInfo(userId, gameId) is called first, and if it returns null, a BusinessException(BusinessErrorCode.GAME_NOT_FOUND) is thrown — so GroupExecutorV3 does not need its own null check for GameInformation.
Learnt from: hyeeum
Repo: MATEBALL/MATEBALL-SERVER PR: 238
File: src/main/java/at/mateball/domain/group/core/service/GroupV3Service.java:224-224
Timestamp: 2026-03-17T04:15:33.223Z
Learning: In MATEBALL-SERVER, the Monday restriction (`BAD_REQUEST_MONDAY`) and "+2 days" restriction (`BAD_REQUEST_DATE`) in `DateValidator` (src/main/java/at/mateball/domain/group/core/validator/DateValidator.java) are intentionally commented out as of PR `#238`, because these exception conditions are currently not applicable to the V3 matching creation API. Only the past-date check (`BAD_REQUEST_PAST`) remains active. Do not flag the commented-out rules as missing validation.
Learnt from: hyeeum
Repo: MATEBALL/MATEBALL-SERVER PR: 235
File: src/main/java/at/mateball/domain/group/core/service/GroupV3Service.java:215-234
Timestamp: 2026-03-15T20:04:35.817Z
Learning: In the MATEBALL-SERVER project, the group leader (생성자) is also added as a record in the groupMember table. Therefore, when implementing chatting access checks in GroupV3Service.getChattingUrl(), memberStatus will not be null for the leader, and there is no need to add a separate leader-check using leaderId before calling GroupMemberStatus.from(data.memberStatus()).
Comment on lines
+17
to
+19
| @ManyToOne(fetch = FetchType.LAZY, optional = false) | ||
| @JoinColumn(name = "group_id", nullable = false) | ||
| private Group group; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Group 삭제 로직 확인
rg -n "deleteGroup|delete.*Group" --type=java -C3Repository: MATEBALL/MATEBALL-SERVER
Length of output: 745
🏁 Script executed:
# Find and examine the Group entity definition
fd -t f "Group.java" --type=file | grep -E "domain/group|entity" | head -5Repository: MATEBALL/MATEBALL-SERVER
Length of output: 122
🏁 Script executed:
# Search for Group entity class definition
rg -n "class Group" --type=java -A 30 | head -100Repository: MATEBALL/MATEBALL-SERVER
Length of output: 11902
🏁 Script executed:
# Check for any cascade or orphan delete configurations related to MatchSchedule
rg -n "MatchSchedule" --type=java -B 2 -A 2Repository: MATEBALL/MATEBALL-SERVER
Length of output: 13209
🏁 Script executed:
# Look for deletion logic related to MatchSchedule
rg -n "delete.*MatchSchedule|MatchSchedule.*delete" --type=java -C 3Repository: MATEBALL/MATEBALL-SERVER
Length of output: 50
🏁 Script executed:
cat -n src/main/java/at/mateball/domain/group/core/Group.javaRepository: MATEBALL/MATEBALL-SERVER
Length of output: 1821
🏁 Script executed:
# Check if there's any explicit cleanup logic for MatchSchedule deletion in services
rg -n "delete|remove" src/main/java/at/mateball/domain/group/scheduler/ --type=java -B 2 -A 2Repository: MATEBALL/MATEBALL-SERVER
Length of output: 50
🏁 Script executed:
# Check MatchScheduler implementation to understand how it handles missing groups
cat -n src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.javaRepository: MATEBALL/MATEBALL-SERVER
Length of output: 1798
🏁 Script executed:
# Check how lazy loading behaves when Group is deleted - look for FK constraint details
rg -n "foreign key|FOREIGN KEY|constraint" --type=sql --type=ddl 2>/dev/null || echo "No SQL files found"Repository: MATEBALL/MATEBALL-SERVER
Length of output: 86
🏁 Script executed:
# Look for any test that validates Group deletion behavior
rg -n "deleteAllByLeaderId|testDelete|Group.*delete" src/test --type=java -B 2 -A 5Repository: MATEBALL/MATEBALL-SERVER
Length of output: 50
🏁 Script executed:
# Check if there are any migration files or schema definitions that show FK constraints
fd -type f -name "*.sql" -o -name "*schema*" -o -name "*migration*" | head -20Repository: MATEBALL/MATEBALL-SERVER
Length of output: 238
🏁 Script executed:
# Look for any repository delete methods related to Group
rg -n "delete" src/main/java/at/mateball/domain/group/core/repository --type=java -B 1 -A 3Repository: MATEBALL/MATEBALL-SERVER
Length of output: 872
🏁 Script executed:
# Check GroupMatchFailService to see how it handles the Group
cat -n src/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.java | head -50Repository: MATEBALL/MATEBALL-SERVER
Length of output: 2216
🏁 Script executed:
# Check if there are any database migration files or schema files
find . -name "*.sql" -o -name "*.ddl" -o -name "*schema*" 2>/dev/null | grep -v node_modules | head -20Repository: MATEBALL/MATEBALL-SERVER
Length of output: 50
🏁 Script executed:
# Look at the test file to see what happens with orphaned schedules
cat -n src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java | head -100Repository: MATEBALL/MATEBALL-SERVER
Length of output: 3981
🏁 Script executed:
# Check how the scheduler actually uses the group object
rg -n "getGroup\(\)" src/main/java/at/mateball/domain/group/scheduler/ --type=java -B 1 -A 1Repository: MATEBALL/MATEBALL-SERVER
Length of output: 1025
🏁 Script executed:
# Check if there's any validation or FK constraint setup in persistence configuration
rg -n "hibernate|JPA|constraint|foreign" src/main/resources --type=properties -iRepository: MATEBALL/MATEBALL-SERVER
Length of output: 102
🏁 Script executed:
# Look for any additional methods in GroupMatchFailService that might handle cleanup
cat -n src/main/java/at/mateball/domain/group/scheduler/GroupMatchFailService.javaRepository: MATEBALL/MATEBALL-SERVER
Length of output: 2578
🏁 Script executed:
# Look for any usage of deleteAllByLeaderId to understand when/if groups are actually deleted
rg -n "deleteAllByLeaderId" --type=java -B 2 -A 2Repository: MATEBALL/MATEBALL-SERVER
Length of output: 1029
🏁 Script executed:
# Check application.yml or application.properties for JPA/Hibernate settings
find . -name "application*.yml" -o -name "application*.properties" 2>/dev/null | xargs cat 2>/dev/null | grep -i "hibernate\|jpa\|constraint" | head -20Repository: MATEBALL/MATEBALL-SERVER
Length of output: 140
🏁 Script executed:
# Check if there's any explicit handling in MatchScheduler for deleted groups
cat -n src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.javaRepository: MATEBALL/MATEBALL-SERVER
Length of output: 1798
그룹 삭제 시 MatchSchedule 레코드가 orphan으로 남을 수 있는 설계 이슈
Group 엔티티에 MatchSchedule에 대한 @OneToMany 관계와 cascade delete 설정이 없습니다. Group이 삭제될 때(예: GroupRepository.deleteAllByLeaderId()), 해당 그룹을 참조하는 MatchSchedule 레코드는 고아 레코드로 남게 됩니다.
MatchScheduler.run()이 이러한 orphan 레코드를 처리하려 할 때 s.getGroup().getId()를 호출하면 LAZY 로딩으로 인해 null 참조 또는 제약 조건 위반 예외가 발생할 수 있습니다. 현재 try-catch 블록이 이를 포착하지만, 근본적으로는 Group 엔티티에서 @OneToMany(cascade = CascadeType.DELETE) 관계를 추가하거나 MatchSchedule 삭제 로직을 명시적으로 구현해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/at/mateball/domain/group/scheduler/MatchSchedule.java` around
lines 17 - 19, Group 삭제 시 MatchSchedule 엔티티가 orphan으로 남아 LAZY 로딩에서
s.getGroup().getId() 호출 시 NPE/제약 위반이 발생할 수 있으므로 MatchSchedule-Group 연관관계를 정리하세요:
Group 엔티티에 MatchSchedule에 대한 `@OneToMany`(mappedBy="group", cascade =
CascadeType.REMOVE 또는 CascadeType.ALL, orphanRemoval = true) 설정을 추가하거나,
GroupRepository.deleteAllByLeaderId() 호출 전에 MatchScheduleRepository에서 해당 그룹의
스케줄을 명시적으로 삭제하는 로직을 구현해 MatchScheduler.run()이 null 그룹을 참조하지 않도록 하세요 (참조할 식별자:
class MatchSchedule, field group, method MatchScheduler.run(), method
GroupRepository.deleteAllByLeaderId()).
Comment on lines
+30
to
+42
| for (MatchSchedule s : targets) { | ||
| try { | ||
| if (s.getType().isFail()) { | ||
| matchFailService.validateFailStatus(s.getGroup().getId()); | ||
| } else { | ||
| matchCompleteService.validateCompleteStatus(s.getGroup().getId()); | ||
| } | ||
|
|
||
| s.markExecuted(); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("스케줄 처리 실패 groupId={}", s.getGroup().getId(), e); | ||
| } |
There was a problem hiding this comment.
예외 발생 시 스케줄 재처리 동작 명확화 필요
현재 구조에서 예외 발생 시 markExecuted()가 호출되지 않아 다음 스케줄러 실행 시 재처리됩니다. 이 동작이 의도된 것인지, 또는 실패한 스케줄도 executed로 마킹하고 별도 실패 처리를 해야 하는지 확인이 필요합니다. 의도된 재시도라면 무한 재시도 방지를 위한 retry count나 실패 상태 추가를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/at/mateball/domain/group/scheduler/MatchScheduler.java` around
lines 30 - 42, The current loop in MatchScheduler leaves failed schedules
unmarked causing indefinite reprocessing; update behavior by adding a retry
mechanism and explicit failure marking: extend MatchSchedule with fields
retryCount and maxRetries (and a markFailed()/setFailedReason() method),
increment retryCount when matchFailService.validateFailStatus(...) or
matchCompleteService.validateCompleteStatus(...) throws, and only call
s.markExecuted() on success or when retryCount > maxRetries; on final failure
call s.markFailed(...) (or set failed flag/reason) and log accordingly. Ensure
MatchScheduler references these symbols (MatchSchedule.retryCount/maxRetries,
markFailed, markExecuted) and persist the updated state so retries stop after
maxRetries.
Comment on lines
+7
to
+8
| @Repository | ||
| public interface SchedulerRepository extends JpaRepository<Group, Long>, SchedulerCustom { |
There was a problem hiding this comment.
치명적 버그: JpaRepository 제네릭 타입이 잘못 선언되었습니다.
SchedulerRepository가 JpaRepository<Group, Long>을 상속하고 있지만, 실제로는 MatchSchedule 엔티티를 관리합니다. GroupExecutorV3.java(Line 57-58)와 테스트 코드에서 schedulerRepository.save(new MatchSchedule(...))를 호출하고 있어, 런타임에 타입 불일치로 인한 오류가 발생합니다.
🐛 수정 제안
-import at.mateball.domain.group.core.Group;
+import at.mateball.domain.group.scheduler.MatchSchedule;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
`@Repository`
-public interface SchedulerRepository extends JpaRepository<Group, Long>, SchedulerCustom {
+public interface SchedulerRepository extends JpaRepository<MatchSchedule, Long>, SchedulerCustom {
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/at/mateball/domain/group/scheduler/repository/SchedulerRepository.java`
around lines 7 - 8, The repository generic is wrong: SchedulerRepository
currently extends JpaRepository<Group, Long> but it actually persists
MatchSchedule instances; change the generic to JpaRepository<MatchSchedule,
Long> and add/import the MatchSchedule entity so save/load calls from
SchedulerRepository (and usages in GroupExecutorV3 and tests that call
schedulerRepository.save(new MatchSchedule(...))) type-check and work at
runtime; keep the SchedulerCustom extension unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java (3)
85-86: 비즈니스 로직 검증 부재테스트가
isExecuted()만 검증하고 있어, 스케줄러가 실제로 올바른 비즈니스 로직을 수행했는지 확인하지 못합니다. FAIL 처리 시 Group 상태와 GroupMember 상태가 기대대로 변경되었는지도 검증해야 합니다.♻️ 추가 검증 제안
MatchSchedule result = em.find(MatchSchedule.class, schedule.getId()); assertThat(result.isExecuted()).isTrue(); + Group resultGroup = em.find(Group.class, group.getId()); + assertThat(resultGroup.getStatus()).isEqualTo(GroupStatus.FAILED.getValue()); + + GroupMember resultLeader = em.find(GroupMember.class, leader.getId()); + assertThat(resultLeader.getStatus()).isEqualTo(GroupMemberStatus.MATCH_FAILED.getValue()); + System.out.println("\n================ 🔥 FAIL 테스트 종료 ================\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` around lines 85 - 86, The test only asserts MatchSchedule result.isExecuted(); extend it to also verify that the FAIL outcome updated related business state: after loading MatchSchedule (result = em.find(MatchSchedule.class, schedule.getId())), load the associated Group and its GroupMember entities and assert the Group's status equals the expected failed/disbanded enum/value and that each affected GroupMember's status/fields reflect the failure (e.g., isActive=false or status==EXPECTED_MEMBER_STATUS). Use the same identifiers from the test (MatchSchedulerIntegrationTest, result / schedule.getId(), MatchSchedule) to locate where to add these assertions and use the Group and GroupMember getters/fields already present in the domain model to perform the checks.
199-200: COMPLETE 비즈니스 로직 검증 추가 필요FAIL 테스트와 마찬가지로, COMPLETE 처리 후 Group 상태와 GroupMember 상태가 기대대로 변경되었는지 검증해야 합니다.
♻️ 추가 검증 제안
MatchSchedule result = em.find(MatchSchedule.class, schedule.getId()); assertThat(result.isExecuted()).isTrue(); + Group resultGroup = em.find(Group.class, group.getId()); + assertThat(resultGroup.getStatus()).isEqualTo(GroupStatus.COMPLETE.getValue()); + + // 참여자 상태 검증 + List<GroupMember> resultMembers = em.createQuery( + "select gm from GroupMember gm where gm.group.id = :groupId", + GroupMember.class) + .setParameter("groupId", group.getId()) + .getResultList(); + assertThat(resultMembers).allMatch(m -> m.getStatus() == GroupMemberStatus.MATCH_COMPLETE.getValue()); + System.out.println("\n================ 🚀 COMPLETE 테스트 종료 ================\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` around lines 199 - 200, 테스트에서 COMPLETE 처리 이후 MatchSchedule의 isExecuted 검증 외에 Group 및 GroupMember 상태 변화도 검증하도록 추가하세요: 현재 조회하는 MatchSchedule(result) 이후에 연관된 Group 엔티티와 해당 Group의 멤버들(GroupMember)을 em.find 또는 JPQL로 로드하여 기대 상태(예: Group의 상태 필드 값, 각 GroupMember의 상태 또는 참여 플래그 등)를 구체적으로 assertThat으로 검증하도록 작성하고, schedule 객체의 연관 참조가 lazy이면 필요한 초기화(fetch)나 트랜잭션 경계 유지(예: em.refresh 또는 쿼리로 재조회)를 적용해 테스트가 안정적으로 실패/성공하도록 만드세요.
173-176: 리더 상태값 검토 필요COMPLETE 시나리오에서 리더(
m1)의 상태가PENDING_REQUEST입니다. 일반적으로 COMPLETE 처리 대상은 이미 매칭 완료된 상태여야 하므로,MATCHED또는NEW_REQUEST가 더 적절할 수 있습니다. 비즈니스 요구사항에 맞는 상태값인지 확인하세요.또한 FAIL 테스트에서는
GroupMember.leader()팩토리 메서드를 사용하지만, 이 테스트에서는 생성자를 직접 사용하고 있습니다. 일관성을 위해 팩토리 메서드 사용을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` around lines 173 - 176, The leader member m1 is created with GroupMemberStatus.PENDING_REQUEST which is incorrect for the COMPLETE scenario; change m1 to use an appropriate matched state (e.g., GroupMemberStatus.MATCHED or NEW_REQUEST per business rules) and create it via the GroupMember.leader(...) factory method for consistency (replace the direct constructor call that creates m1 with a call to GroupMember.leader(userA, group, <appropriate status>, ...)), keeping m2 creation and em.persist calls as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java`:
- Around line 92-93: The test method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 in
MatchSchedulerIntegrationTest has a duplicate `@DisplayName`; change its
`@DisplayName` value to a unique, descriptive string (different from the other
FAIL test) so test reports distinguish the scenarios—update the annotation above
the method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 to a new name reflecting its
specific scenario.
- Line 40: Two tests in MatchSchedulerIntegrationTest share the same
`@DisplayName`("스케줄 실행 시 FAIL 처리"), making results ambiguous; update one of the
duplicates to a unique, descriptive name that reflects its scenario (e.g.,
include context like "when job X fails" or "with missing dependency") by editing
the `@DisplayName` annotation on the second test in the
MatchSchedulerIntegrationTest class so each test has a distinct display name.
---
Nitpick comments:
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java`:
- Around line 85-86: The test only asserts MatchSchedule result.isExecuted();
extend it to also verify that the FAIL outcome updated related business state:
after loading MatchSchedule (result = em.find(MatchSchedule.class,
schedule.getId())), load the associated Group and its GroupMember entities and
assert the Group's status equals the expected failed/disbanded enum/value and
that each affected GroupMember's status/fields reflect the failure (e.g.,
isActive=false or status==EXPECTED_MEMBER_STATUS). Use the same identifiers from
the test (MatchSchedulerIntegrationTest, result / schedule.getId(),
MatchSchedule) to locate where to add these assertions and use the Group and
GroupMember getters/fields already present in the domain model to perform the
checks.
- Around line 199-200: 테스트에서 COMPLETE 처리 이후 MatchSchedule의 isExecuted 검증 외에
Group 및 GroupMember 상태 변화도 검증하도록 추가하세요: 현재 조회하는 MatchSchedule(result) 이후에 연관된
Group 엔티티와 해당 Group의 멤버들(GroupMember)을 em.find 또는 JPQL로 로드하여 기대 상태(예: Group의 상태
필드 값, 각 GroupMember의 상태 또는 참여 플래그 등)를 구체적으로 assertThat으로 검증하도록 작성하고, schedule
객체의 연관 참조가 lazy이면 필요한 초기화(fetch)나 트랜잭션 경계 유지(예: em.refresh 또는 쿼리로 재조회)를 적용해 테스트가
안정적으로 실패/성공하도록 만드세요.
- Around line 173-176: The leader member m1 is created with
GroupMemberStatus.PENDING_REQUEST which is incorrect for the COMPLETE scenario;
change m1 to use an appropriate matched state (e.g., GroupMemberStatus.MATCHED
or NEW_REQUEST per business rules) and create it via the GroupMember.leader(...)
factory method for consistency (replace the direct constructor call that creates
m1 with a call to GroupMember.leader(userA, group, <appropriate status>, ...)),
keeping m2 creation and em.persist calls as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 653942e1-c99e-4f52-ac95-dc45f1e45c4a
📒 Files selected for processing (1)
src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java
|
|
||
| // ===================== FAIL ===================== | ||
| @Test | ||
| @DisplayName("스케줄 실행 시 FAIL 처리") |
There was a problem hiding this comment.
중복된 @DisplayName
Line 92의 두 번째 테스트와 동일한 @DisplayName을 사용하고 있습니다. 테스트 결과를 구분하기 어려우므로 각 시나리오에 맞는 고유한 이름을 사용하세요.
🐛 DisplayName 수정 제안
- `@DisplayName`("스케줄 실행 시 FAIL 처리")
+ `@DisplayName`("스케줄 실행 시 FAIL 처리 - 그룹원이 생성자 혼자일 때")
void 그룹원이_생성자_혼자일때_FAIL_테스트() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java` at
line 40, Two tests in MatchSchedulerIntegrationTest share the same
`@DisplayName`("스케줄 실행 시 FAIL 처리"), making results ambiguous; update one of the
duplicates to a unique, descriptive name that reflects its scenario (e.g.,
include context like "when job X fails" or "with missing dependency") by editing
the `@DisplayName` annotation on the second test in the
MatchSchedulerIntegrationTest class so each test has a distinct display name.
Comment on lines
+92
to
+93
| @DisplayName("스케줄 실행 시 FAIL 처리") | ||
| void 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트() { |
There was a problem hiding this comment.
중복된 @DisplayName (2차)
첫 번째 FAIL 테스트와 동일한 @DisplayName입니다. 시나리오 구분을 위해 고유 이름을 사용하세요.
🐛 DisplayName 수정 제안
- `@DisplayName`("스케줄 실행 시 FAIL 처리")
+ `@DisplayName`("스케줄 실행 시 FAIL 처리 - 생성자 혼자 + 승인 대기자 존재")
void 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트() {📝 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.
Suggested change
| @DisplayName("스케줄 실행 시 FAIL 처리") | |
| void 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트() { | |
| `@DisplayName`("스케줄 실행 시 FAIL 처리 - 생성자 혼자 + 승인 대기자 존재") | |
| void 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트() { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/at/mateball/scheduler/MatchSchedulerIntegrationTest.java`
around lines 92 - 93, The test method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 in
MatchSchedulerIntegrationTest has a duplicate `@DisplayName`; change its
`@DisplayName` value to a unique, descriptive string (different from the other
FAIL test) so test reports distinguish the scenarios—update the annotation above
the method 그룹원이_생성자_혼자면서_승인_대기자가_존재할때_FAIL_테스트 to a new name reflecting its
specific scenario.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 이슈 번호
closed #249
✅ 어떻게 이슈를 해결했나요?
경기시간 기준 -> complete 스케쥴 처리
findAllDueSchedules: 아직 실행되지 않은 스케쥴에 한해 로직이 실행됩니다.GroupMatchFailServicefail 이 되는 경우: 스케쥴러가 접근했을때, 매칭 참여자(isParticipant=ture)가 한 명인 경우-> 이때, 매칭생성자뿐만 아니라 그룹에 요청했던 요청자까지도 함께 매칭실패 로 상태가 변경됩니다.
GroupMatchCompleteServicecomplete 이 되는 경우: 스케쥴러가 접근했을때, 매칭 참여자(isParticipant=true)가 두 명 이상인 경우MatchSchedulerIntegrationTest을 통해, 총 3개의 경우의 수를 테스트해볼 수 있습니다. <- 다만 기존 "매칭생성"브랜치에서 작업했던 내용이라, 실행은 매칭생성 병합 후에 가능합니다.. ㅠㅠ❤️ To 다진 / To 헤음
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트