Skip to content

fix: claim quest before granting reward - #77

Draft
dev-yunseong wants to merge 1 commit into
mainfrom
fix/68
Draft

fix: claim quest before granting reward#77
dev-yunseong wants to merge 1 commit into
mainfrom
fix/68

Conversation

@dev-yunseong

@dev-yunseong dev-yunseong commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

요약

보상 지급 전에 퀘스트를 선점하도록 순서를 뒤집었습니다. setCompletedByUserIdAndQuestIdAND state = 'IN_PROGRESS' 조건을 추가하고 반환 타입을 Mono<Long>(영향 행 수)으로 바꿔, 실제로 1행 이상을 선점한 요청만 rewardGiver.giveWithReward를 호출합니다. 또한 CardRewardGiver의 INSERT를 ON CONFLICT (card_id, user_id) DO UPDATE SET count = user_cards.count + :count upsert로 바꿔, 이미 보유한 카드를 보상으로 줄 때 유니크 제약 위반으로 배치 전체가 롤백되지 않게 했습니다.

대상 이슈

Closes #68

검증

Ran ./gradlew build in the worktree: compiled clean, :test reported "26 tests completed, 3 failed" — the 3 failures are DataControllerTest (x2) and CardControllerTest (x1), all Spring context load failures (ConfigurationPropertiesBindException -> NumberFormatException from missing env config, e.g. PORT/DATABASE_URL). Confirmed pre-existing by git stash + ./gradlew test on the untouched origin/main tree: "25 tests completed, 3 failed" with exactly the same 3 test names. Then git stash pop. Ran ./gradlew test --tests com.wordonline.matching.quest.service.QuestServiceTest on my change: BUILD SUCCESSFUL (all 4 quest service tests including the new one pass). The Postgres upsert SQL itself was not executed against a live DB — no DB is available in this environment.

테스트

— added checkQuestsWithRewards_SkipsRewardWhenAlreadyClaimed (@DisplayName "퀘스트_선점_실패시_보상_미지급"): the claim UPDATE returns 0 rows, the result list must be empty, and verify(rewardGiver, never()).giveWithReward(...). Matches the existing Mockito + StepVerifier style in that file. Also updated the existing checkQuestsWithRewards_Success stubs from Mono.empty() to Mono.just(1L) for the new return type.

리뷰어 참고

Both defects in the issue were verified in code before editing. (1) setCompletedByUserIdAndQuestId really had no state predicate and returned Mono<Void>, and rewards really ran first. (2) database/migration/V000_20260406__init_tables.sql:363 confirms user_cards ... unique (card_id, user_id), and CardRewardGiver really saved a null-id UserCard (always an INSERT). The class-level @Transactional on QuestService is unchanged, so batch-wide rollback semantics stay as-is — the fix removes the cause of the constraint violation rather than narrowing the transaction.

Implementation notes: the new UserCardRepository.addCount upsert follows the existing ON CONFLICT ... DO UPDATE pattern already used in UserScenarioRepository, and the Mono<Long> row-count return type matches the existing @Query UPDATE methods in UserRepository. ON CONFLICT (card_id, user_id) is written in the constraint's declared column order. The claim-then-give ordering relies on Postgres READ COMMITTED re-evaluating the WHERE clause after a blocking row lock is released, which is what makes the second concurrent transaction see 0 affected rows.

Out of scope / not changed: DecorationRewardGiver and MagicRewardGiver still insert directly. user_decorations has no unique constraint in the migration so it duplicates silently rather than erroring, and user_magics does have uq_user_magics_user_id_magic_id — the claim guard now prevents the concurrent double-grant for both, but a quest whose reward is an already-owned magic would still raise a duplicate key. Issue 68 named only the card path, so I left those alone; worth a separate issue if magic rewards can overlap the initUserMagic DEFAULT grant.

Residual risk: the upsert SQL was never run against a real Postgres instance here, so a typo-level SQL error would only surface at runtime. No migration is required — the unique (card_id, user_id) index the upsert infers on already exists.


멀티 에이전트 코드 리뷰에서 검증된 결함을 수정한 것. 격리된 워크트리에서 작업하고 모듈 빌드로 확인함.

Rewards were granted before the completion UPDATE, and that UPDATE had no
state condition and discarded its row count, so two concurrent check
requests both granted the same quest reward. The card reward then hit the
user_cards (card_id, user_id) unique constraint and the class-level
transaction rolled back every reward in the batch. The UPDATE now requires
state = 'IN_PROGRESS' and returns the affected row count so a reward is only
given when this request actually claimed the quest, and the card grant is an
upsert so rewarding an already-owned card increments the count instead of
failing.

Closes #68

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix fix bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

퀘스트 완료에 상태 가드가 없고 보상 지급이 멱등하지 않음

1 participant