fix: gate solo sessions and report cancel failure - #75
Draft
dev-yunseong wants to merge 1 commit into
Draft
Conversation
Solo session entry points trusted the client: PVE played any scenarioId without checking whether it was unlocked, and both PVE and practice skipped the deck and busy-state preconditions the PVP path already enforces. Queue cancellation always answered 200 even when the user had just been dequeued into a live match, and the scheduler matched only one pair per five-second tick regardless of queue depth. Closes #62 Closes #63 Closes #65 Closes #66 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 20, 2026
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.
요약
PVE/연습 세션 진입에 덱 검증과 대기중/플레이중 상태 검사를 추가하고, PVE는 해당 시나리오가 해금(ACTIVE/FINISHED)되어 있는지 서버에서 검증한 뒤에만 세션을 만든다. 대기열 취소는 실제 ZREM 개수를 확인해 이미 매칭된 경우 409를 반환하며 markOnline을 건너뛴다. 스케줄러 틱마다 최대 20쌍까지 반복 매칭한다.
대상 이슈
Closes #62
Closes #63
Closes #65
Closes #66
검증
Ran
./gradlew buildin the worktree: compileJava/compileKotlin/compileTestJava/compileTestKotlin/bootJar all succeeded;:testreported "30 tests completed, 3 failed". The 3 failures are DataControllerTest (x2) and CardControllerTest — @SpringBootTest context load failures (ConfigurationPropertiesBindException -> NumberFormatException, missing env like PORT/DB). Confirmed pre-existing bygit stash -uand re-running./gradlew teston clean origin/main state: same 3 failures, BUILD FAILED; thengit stash pop. My new tests:./gradlew test --tests com.wordonline.matching.matching.service.GameMatchServiceTest-> BUILD SUCCESSFUL, XML result tests="5" failures="0" errors="0". Committed 58958bc and pushedfix/62to origin (new branch, push output confirmed).테스트
— 5 plain Mockito/JUnit5 tests (new src/test/kotlin source dir, compiled by the existing kotlin-jvm plugin; no new dependencies): PVE rejected when scenario locked, practice rejected when OnMatching, practice rejected without valid deck, removeFromQueue false + no markOnline when ZREM count 0, true + markOnline when count 1. The tryMatching loop is not unit-tested (needs live Redis + game server).
리뷰어 참고
Root-cause placement: the unlock check lives in AdventureService.isScenarioUnlocked (runs the same idempotent activation the GET /api/adventures read path runs, then reads the user_scenarios row), so a brand-new user who never fetched /api/adventures is not falsely blocked on scenario 1, and any future PVE caller routes through the same gate. New derived query UserScenarioRepository.findByUserIdAndScenarioId.
#65 busy check reuses the existing UserService.getStatus, which already folds isInQueue + sessionRecoveryStore + live-room lookup into OnMatching/OnPlaying — no new duplicate logic. Side effect worth reviewing: practice/PVE start now depends on the game-server room-list call inside getStatus succeeding; if the game server is unreachable the request fails, but createSession would fail anyway. Deck validation reuses the existing validateSelectedDeck (still IllegalStateException -> 500 via GlobalExceptionHandler, same as the PVP path today; I did not change that mapping). New rejections use IllegalArgumentException -> 400.
#63: MatchingQueueRepository.remove now returns the ZREM count (Mono instead of Mono); the two other call sites (enqueue failure path, tryMatching finally block) use awaitSingleOrNull and are unaffected. DELETE /api/match/queue/me now returns 409 when nothing was removed — client-visible API change, Unity client needs to handle 409. markOnline is skipped in that case so a user already marked OnPlaying is not clobbered.
#66: implemented the throughput half only — up to MAX_PAIRS_PER_TICK = 20 pairs per 5s tick (marked with a
ponytail:comment). The "notify the user on queue expiry" half is not implemented: there is no push channel in this module, and the existing GET /api/match/queue/me/exist polling endpoint already returns 404 for an expired entry. A real push notification would be a new mechanism, not a mechanical patch. Pairs are still created sequentially inside one coroutine, so 20 pairs can outlast a 5s tick and overlap the next tick; that overlap was already possible before this change and is safe because dequeueBestPair removes both users from Redis atomically-per-pair before session creation.멀티 에이전트 코드 리뷰에서 검증된 결함을 수정한 것. 격리된 워크트리에서 작업하고 모듈 빌드로 확인함.