아이템 등록 사용량 한도와 게스트 권한 정리 - #904
Merged
Merged
Conversation
- 게스트 계정이 POST /auth/guest 로 입력값 없이 무한 발급되는 것을 dev 에서 실측 확인(3연속 호출에 서로 다른 계정 3개). userId 를 키로 쓰는 어떤 사용량 제한도 계정을 갈아타면 리셋되므로, 비용이 드는 행위의 소유자를 회원으로 못박아 소셜 계정 생성 비용이 sybil 방어를 하게 한다 - 위시리스트는 이미 requireMember 로 회원 전용인데 토너먼트만 그 원칙에서 빠져 있었다. 새 원칙이 아니라 기존 원칙의 일관 적용이다 - 게스트에게 열어 두는 것: 참여·아이템 추가·플레이. 플레이(recordMatch)는 외부 호출이 없어 비용이 0 이고, 초대받아 바로 참여하는 흐름이 서비스의 핵심이다 - createFromPlayLink 는 게이트를 두지 않는다. 거기서 만들어지는 CLONE 은 아이템 추가가 막혀 있어(clonedTournamentCannotAddItems) 추출 비용을 만들 수 없다 - requireMember 는 findActiveById 가 아니라 findById + Elvis 로 둔다. 처음 findActiveById 로 두었더니 users 행 없이 인증만으로 호출되던 기존 계약이 404 로 바뀌어 토너먼트 테스트 151개가 깨졌다. 게스트는 발급이 곧 users 행 생성이라 행 부재를 통과시켜도 게이트에 구멍이 나지 않는다(FCM 토큰 등록의 rejectIfWithdrawnForUpdate 와 같은 결) - 동시성·presign 테스트 5종이 owner 를 GUEST 로 만들어 토너먼트를 생성하고 있었다. 각 테스트의 관심사는 경합·발급 흐름이지 게스트 권한이 아니므로 owner 만 MEMBER 로 정정했고, play-link 테스트의 cloner 는 GUEST 로 남겨 게스트 클론 경로가 열려 있음을 함께 검증한다
- LLM 을 태우는 등록 경로(위시 링크·이미지, 토너먼트 링크·이미지, 위시 재추출)에만 quota 를 건다. 조회·플레이 등 비용이 0 인 경로는 대상이 아니다 - 세는 단위를 요청 수가 아니라 큐에 넣는 item 수로 잡았다. 이미지 등록은 한 요청이 최대 5장이고 장당 LLM 호출이 1회씩 붙어, 요청 수로 세면 링크 1건과 이미지 5장이 같은 비용으로 취급돼 실제 호출량이 5배까지 벌어진다 - 토너먼트 축은 요청자가 아니라 토너먼트 오너의 몫에서 차감한다. 참여자에 게스트가 섞이는데 게스트 계정은 무한 발급되므로 요청자 기준으로 세면 계정을 갈아타며 리셋할 수 있다. 오너는 반드시 회원이라 소셜 계정 생성 비용이 그 우회를 막는다 - 위시와 토너먼트를 별개 키로 분리했다. 한 축으로 합치면 친구들이 내 토너먼트에 아이템을 넣은 만큼 내가 내 위시리스트를 못 쓰게 된다 - 오너 체감을 줄이는 방법으로 차감 가중치(0.5 등)를 검토했으나 한도를 키우는 쪽으로 정했다. 비용과 카운터가 어긋나면 메트릭으로 실제 호출량을 읽을 수 없어진다. 차감은 1:1 로 두고 tournament-limit 을 wish-limit 보다 크게 잡는다 - 이미지 v2 는 presign 시점에 차감하고 confirm 은 차감하지 않는다. confirm 이 안 와도 폴링 백스톱이 pending 을 회수해 큐에 넣으므로, confirm 에서만 세면 그 경로가 통째로 한도를 우회한다 - 저장소는 Bucket4j 대신 StringRedisTemplate + Lua 고정 윈도우로 두었다. Bucket4j 는 버킷 상태를 객체로 직렬화해 저장해 무중단 배포 중 구·신버전 호환성을 테스트로 고정해야 하는데(테스트 규약의 직렬화/호환성 분류), 필요한 것은 창당 N 개라는 카운터뿐이다 - 판정과 차감을 한 Lua 로 원자화하고 거부 시에는 INCRBY 를 하지 않는다. 거부분까지 누적하면 한도에 걸린 사용자가 재시도할수록 카운터가 올라 창이 끝나도 넘긴 상태로 시작한다 - Redis 장애는 fail-open 으로 통과시킨다. 한도 인프라 때문에 등록이 멈추는 것보다 낫고, Redis 가 죽으면 refresh 토큰 저장소도 함께 죽어 그 창에서 대량 호출이 지속되기 어렵다 - ErrorCategory 에 TOO_MANY_REQUESTS(429)를 신설하고 RetryAfter 인터페이스를 구현한 예외만 Retry-After 헤더를 받게 했다. 예외 클래스 전체에 nullable 필드를 다는 대신 타입으로 가려, 재시도 시점을 모르는 예외에 0 같은 거짓값이 실리지 않는다 - 토너먼트 429 문구는 오너의 사용량을 드러내지 않는다. 이 응답은 참여 게스트도 받는데 남의 사용량은 요청자에게 알릴 정보가 아니다 - 폴링 백스톱 테스트가 게스트로 토너먼트를 만들고 있어 owner 를 MEMBER 로 정정했다(앞선 커밋의 회원 전용 게이트에 걸리던 것)
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
- 구현은 처음부터 "큐에 넣는 item 수" 를 세어 LLM 여부와 무관했는데, 주석과 설정 문서만 LLM 중심으로 쓰여 있어 기준이 좁게 읽혔다. 코드 동작 변경 없이 문서만 실제 기준에 맞춘다 - 등록 1건은 파싱이 파서로 풀려 LLM 을 안 타도 fetch 대역·residential proxy 요청(HEADLESS_FIRST 사이트)·헤드리스 렌더러 시간·이미지 저장·DB 행 영구 증가를 소모한다. 프록시는 사용량 과금이라 LLM 과 별개로 돈이 나간다 - 그래서 경로별 차등을 두지 않는다. 등록 시점엔 파서로 풀릴지 LLM 으로 갈지 알 수 없고, 사이트가 마크업을 바꾸면 어제 파서로 풀리던 링크가 오늘 LLM 을 탄다 - 실제 소비량에 맞춘 사후 정산은 후속 과제로 남긴다는 점을 주석에 명시
- 판정을 "누적 + 요청량 > 한도" 에서 "누적 >= 한도" 로 바꾼다. 요청량은 이제 판정에 쓰지 않고 차감에만 쓴다 - 남은 몫이 2 인데 이미지 5장을 요청하면 기존에는 전량 거부였다. 사용자는 자기 잔액을 모르니 왜 막혔는지 알 수 없고, 몇 장으로 줄여야 통과하는지 안내할 방법도 없었다. 잔액 방식은 마지막 한 번이 항상 성공하고 그 다음부터 막혀, "이번 창의 몫을 다 썼다" 는 경계가 사용자에게 명확해진다 - 부분 성공 계약을 고민할 필요가 사라진다. 요청은 통째로 통과하거나 통째로 거부된다 - 대가로 누적이 한도를 넘어 잔액이 음수가 될 수 있다. 다만 창당 최대 소비가 (한도 + 1회 최대 요청량)으로 바운드되어 무한 초과가 아니다. 한도 10 · 이미지 5장 기준 최악 -4 이고, 그 뒤로는 전부 거부된다 - 후속 과제(#910)의 사후 정산도 같은 구조에 얹힌다. 파싱 후 확정된 실제 소비를 음수 쪽에 더하면 되고 판정 로직은 그대로다
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentItemImagePresignedIntegrationTest.kt (1)
290-301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win신분 값을 문자열 리터럴 대신 enum 에서 가져오면 더 안전하다.
insertMember는"MEMBER"리터럴을 DB 에 넣고,token()은IdentityType.MEMBER로 JWT 를 만든다. 두 값이 같은 개념을 두 경로로 표현한다. 이 PR 은 회원/게스트 구분을 권한 게이트로 승격시켰다. 따라서 두 값이 어긋나면 테스트는 통과하면서 실제로는 잘못된 신분을 검증한다.같은 파일의 다른 테스트(
ItemQuotaIntegrationTest.insertUser)는identityType.name을 쓴다. 여기도 맞추면 표현이 하나로 모인다.♻️ 제안 diff
private fun insertMember(userId: UUID) { jdbcTemplate.update( "INSERT INTO users (id, nickname, identity_type, created_at, updated_at) VALUES (?, ?, ?, NOW(6), NOW(6))", uuidToBytes(userId), userId.toString().take(10), - "MEMBER", + IdentityType.MEMBER.name, ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentItemImagePresignedIntegrationTest.kt` around lines 290 - 301, Update insertMember to use IdentityType.MEMBER.name instead of the hardcoded "MEMBER" database value, matching the token() identity and the pattern used by ItemQuotaIntegrationTest.insertUser.src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt (2)
84-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win변수명이 담긴 값과 다르다.
retryAfter라는 이름이지만 값은currentCount가 돌려준 quota 카운터다.Retry-After헤더 검증은 Line 82에서 이미 끝났다. 이 이름은 다음에 이 테스트를 읽는 사람이 헤더 검증으로 오독하게 만든다. 검증 로직 자체는 정확하다.♻️ 제안 diff
- val retryAfter = requireNotNull(currentCount(ItemQuotaScope.WISH, userId)) + val count = requireNotNull(currentCount(ItemQuotaScope.WISH, userId)) // 거부된 요청은 카운터를 올리지 않는다 — 올리면 재시도할수록 창이 끝나도 한도를 넘긴 채 시작한다. - assertEquals(properties.wishLimit.toLong(), retryAfter) + assertEquals(properties.wishLimit.toLong(), count)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt` around lines 84 - 86, Rename the local variable assigned from currentCount(ItemQuotaScope.WISH, userId) from retryAfter to a name that reflects the quota counter value, and update the following assertEquals reference. Leave the existing validation logic unchanged.
107-137: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winpresign → confirm 이중 차감 방지 테스트가 빠져 있다.
TournamentItemService.confirmImageRegistration과 위시 쪽 confirm 은 "presign 에서 이미 차감했으므로 다시 차감하지 않는다" 를 계약으로 삼는다. 이 계약이 깨지면 오너 몫이 이미지 장수만큼 두 번 소모된다. 사용자에게는 한도가 절반으로 줄어든 것처럼 보인다. 조용히 퇴행하는 종류의 버그다.현재 테스트는 presign 이후 카운터만 확인하고, confirm 이후 카운터는 확인하지 않는다. 이 파일은 이미 presign 요청을 보내므로, confirm 까지 이어서 카운터 불변을 단언하면 계약이 고정된다.
`@Test` fun `presign 으로 차감한 뒤 confirm 은 추가로 차감하지 않는다`() { // presign 으로 N 장 차감 → imageKey 획득 → 업로드 stub 통과 → confirm 200 // confirm 전후 currentCount 가 동일함을 단언한다. }confirm 은 S3 존재 확인(
StubImageStorage)에 의존하므로, 위시·토너먼트 중 stub 설정이 더 단순한 경로 하나만 검증해도 충분하다. 제가 이 테스트를 작성해 드릴까요?경로 지침의 "핵심 비즈니스 규칙, 예외 케이스, 경계값 검증이 충분한지" 항목을 근거로 남긴다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt` around lines 107 - 137, Add an integration test in ItemQuotaIntegrationTest covering the existing presign flow through confirm, using the simpler wish or tournament path and StubImageStorage setup. Capture currentCount after presign, complete a successful confirm with the returned imageKey, then assert the quota count is unchanged after confirmation.Source: Path instructions
src/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt (1)
258-267: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win세 429 응답에
Retry-After헤더를 OpenAPI 스키마로 선언하세요.
GlobalExceptionHandler는 실제 응답에Retry-After를 추가하지만, 현재 세@ApiResponse에는 설명 문구만 있습니다. 따라서 생성된 OpenAPI 계약에 헤더가 노출되지 않습니다.Header(name = "Retry-After", schema = Schema(type = "integer", format = "int64"))를 세 응답에 추가하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt` around lines 258 - 267, Update the three 429 ApiResponse definitions in TournamentItemApi to declare the Retry-After response header using an integer int64 schema. Add the header to each response’s headers collection while preserving the existing descriptions and response content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaException.kt`:
- Around line 29-36: Update ItemQuotaException.exceeded to validate that
retryAfterSeconds is positive (greater than zero) before constructing the
exception, while preserving the existing TOO_MANY_REQUESTS category validation
and return behavior.
In `@src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaGuard.kt`:
- Around line 30-45: Replace the runCatching/getOrElse handling in
ItemQuotaGuard’s quota-check flow with a try/catch that catches only Exception
around store.tryConsume. Preserve the existing fail-open warning and return
behavior for ordinary Redis access exceptions, while allowing Error subclasses
such as OutOfMemoryError and LinkageError to propagate.
In `@src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaProperties.kt`:
- Around line 39-41: Update the window validation in ItemQuotaProperties so it
accepts only durations whose millisecond value is between 1 and Long.MAX_VALUE
inclusive, rejecting sub-millisecond durations and values that overflow
Duration.toMillis(). Add tests covering both valid boundaries and out-of-range
values, including the ArithmeticException overflow case.
In `@src/main/kotlin/com/depromeet/piki/tournament/service/TournamentService.kt`:
- Around line 71-74: Update requireMember to load the user with
findByIdForUpdate, then require both isActive() and IdentityType.MEMBER before
allowing tournament creation; reject missing, withdrawn, or non-member users
with the appropriate TournamentException. Add an integration test covering
withdrawn users and the withdraw/create race.
In `@src/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.kt`:
- Around line 22-27: Update the four 429 responses in WishlistApi to declare a
Retry-After header using an integer int64 schema, matching the header emitted by
GlobalExceptionHandler. Add or update the OpenAPI specification test to assert
this header is present on all four responses.
In `@src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt`:
- Around line 108-112: Validate the requested contentTypes before invoking
itemQuotaGuard.consume in the wishlist image-presign flow. Use the existing
ImagePresignService validation mechanism or its established equivalent, reject
unsupported MIME types with the existing 400 input behavior, and only deduct
quota for validated content types before calling presignRawUploads.
- Around line 276-280: refreshWishItem에서 itemQuotaGuard.consume 호출을 persistence
내부의 wish 행 락 이후 소유권·유형·상태·진행 여부가 검증된 뒤 새 ItemSnapshot.pending(...)을 저장하는 분기로
이동하세요. quota 초과 시 snapshot을 저장하지 않도록 하고, Redis 차감 후 DB 저장 실패 시 보상하거나 예약 정책을
적용하세요. 성공한 READY refresh만 1을 소비하고 진행 중 재시도·FAILED·이미지·타인 위시·공유 합류·동시 refresh는
소비하지 않도록 관련 테스트를 추가·수정하세요.
In
`@src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt`:
- Around line 214-218: Update the assertion in ItemQuotaIntegrationTest so it
verifies that TournamentErrorCode.ITEM_QUOTA_EXCEEDED.message does not contain
owner-usage information or other prohibited usage terms, rather than merely
containing “토너먼트”. Import assertFalse for the negative assertion and remove
assertTrue if it is no longer used elsewhere in the test.
In
`@src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaPropertiesTest.kt`:
- Around line 18-26: ItemQuotaProperties의 window 검증을 밀리초 단위 양수로 강화해
Duration.ofNanos(1)처럼 toMillis()가 0이 되는 값을 생성 시 거부하세요. ItemQuotaPropertiesTest에
1ms 미만 창이 IllegalArgumentException을 발생시키는 경계값 테스트를 추가하고, 기존 0 및 음수 검증은 유지하세요.
---
Nitpick comments:
In
`@src/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt`:
- Around line 258-267: Update the three 429 ApiResponse definitions in
TournamentItemApi to declare the Retry-After response header using an integer
int64 schema. Add the header to each response’s headers collection while
preserving the existing descriptions and response content.
In
`@src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt`:
- Around line 84-86: Rename the local variable assigned from
currentCount(ItemQuotaScope.WISH, userId) from retryAfter to a name that
reflects the quota counter value, and update the following assertEquals
reference. Leave the existing validation logic unchanged.
- Around line 107-137: Add an integration test in ItemQuotaIntegrationTest
covering the existing presign flow through confirm, using the simpler wish or
tournament path and StubImageStorage setup. Capture currentCount after presign,
complete a successful confirm with the returned imageKey, then assert the quota
count is unchanged after confirmation.
In
`@src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentItemImagePresignedIntegrationTest.kt`:
- Around line 290-301: Update insertMember to use IdentityType.MEMBER.name
instead of the hardcoded "MEMBER" database value, matching the token() identity
and the pattern used by ItemQuotaIntegrationTest.insertUser.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 05de020f-ab0d-4f63-be8e-954caae3772c
📒 Files selected for processing (34)
src/main/kotlin/com/depromeet/piki/common/exception/CommonErrorCode.ktsrc/main/kotlin/com/depromeet/piki/common/exception/ErrorCategory.ktsrc/main/kotlin/com/depromeet/piki/common/exception/GlobalExceptionHandler.ktsrc/main/kotlin/com/depromeet/piki/common/exception/RetryAfter.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaException.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaGuard.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaProperties.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaScope.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaVerdict.ktsrc/main/kotlin/com/depromeet/piki/common/ratelimit/RedisItemQuotaStore.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentApi.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentApiExamples.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApiExamples.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentErrorCode.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentException.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentItemService.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentService.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApiExamples.ktsrc/main/kotlin/com/depromeet/piki/wishlist/domain/WishErrorCode.ktsrc/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.ktsrc/main/resources/application.ymlsrc/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaExceptionTest.ktsrc/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaPropertiesTest.ktsrc/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaStoreIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/image/service/PendingUploadPollingIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentFromPlayLinkConcurrencyIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentItemImageAddConcurrencyIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentItemImagePresignedIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentStartConcurrencyIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentWishAddConcurrencyIntegrationTest.kt
Comment on lines
+276
to
280
| // 재추출도 파싱을 한 번 더 돌리므로 신규 등록과 같은 비용이다 — 1 로 차감한다. | ||
| // refresh 계약 검증(링크 없음·FAILED 항목 등)은 persistence 안쪽이라 여기선 앞서 깎이는데, 그 두 사유는 | ||
| // 클라가 refresh 버튼을 띄우지 않는 상태라 정상 흐름에서 반복 호출되지 않는다. | ||
| itemQuotaGuard.consume(ItemQuotaScope.WISH, userId, 1, WishErrorCode.ITEM_QUOTA_EXCEEDED) | ||
| return wishPersistenceService.refresh(userId = userId, wishId = wishId) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(WishlistService|WishPersistenceService|WishlistApi|ItemQuota|Quota).*\.kt$|wishlist|quota' | head -200
printf '%s\n' '--- symbols ---'
rg -n -S 'fun refresh|refresh\(|itemQuotaGuard|ItemQuotaScope|ITEM_QUOTA_EXCEEDED|PENDING|PROCESSING|FAILED' src/main/kotlin 2>/dev/null | head -300
printf '%s\n' '--- repository status ---'
git status --shortRepository: TeamPiKi/core
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -e
for f in \
src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt \
src/main/kotlin/com/depromeet/piki/wishlist/service/WishPersistenceService.kt \
src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaGuard.kt \
src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaScope.kt \
src/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.kt \
src/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt \
src/test/kotlin/com/depromeet/piki/wishlist/controller/WishRefreshConcurrencyIntegrationTest.kt
do
echo "--- $f ---"
wc -l "$f"
done
echo '--- WishlistService refresh ---'
sed -n '245,290p' src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt
echo '--- WishPersistenceService refresh ---'
sed -n '185,245p' src/main/kotlin/com/depromeet/piki/wishlist/service/WishPersistenceService.kt
echo '--- ItemQuotaGuard ---'
cat -n src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaGuard.kt
echo '--- ItemQuotaScope ---'
cat -n src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaScope.kt
echo '--- WishlistApi refresh contract ---'
sed -n '350,470p' src/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.kt
echo '--- refresh test references ---'
rg -n -C 4 'refresh|quota|429|FAILED|PENDING|PROCESSING' \
src/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt \
src/test/kotlin/com/depromeet/piki/wishlist/controller/WishRefreshConcurrencyIntegrationTest.ktRepository: TeamPiKi/core
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -e
echo '--- quota classes and usages ---'
rg -n -C 8 'class RedisItemQuotaStore|interface RedisItemQuotaStore|fun tryConsume|`@Transactional`|refreshWishItem|ItemQuotaGuard' \
src/main/kotlin/com/depromeet/piki/common/ratelimit \
src/main/kotlin/com/depromeet/piki/wishlist \
src/test/kotlin/com/depromeet/piki/common/ratelimit \
src/test/kotlin/com/depromeet/piki/wishlist | head -500
echo '--- quota files ---'
git ls-files src/main/kotlin/com/depromeet/piki/common/ratelimit | sort
for f in $(git ls-files src/main/kotlin/com/depromeet/piki/common/ratelimit | sort); do
echo "--- $f ---"
sed -n '1,220p' "$f"
done
echo '--- transaction config and service declaration ---'
rg -n -C 5 'class WishlistService|class WishPersistenceService|`@EnableTransactionManagement`|PlatformTransactionManager|RedisTransaction|transaction' \
src/main/kotlin src/test/kotlin | head -500Repository: TeamPiKi/core
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
echo '--- Redis quota store ---'
cat -n src/main/kotlin/com/depromeet/piki/common/ratelimit/RedisItemQuotaStore.kt
echo '--- quota properties ---'
cat -n src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaProperties.kt
echo '--- quota config references ---'
rg -n -C 4 'item-quota|itemQuota|quota.enabled|wish-limit|wishLimit|ITEM_QUOTA' src/main/resources src/main/kotlin src/test/resources 2>/dev/null | head -250
echo '--- quota integration test scenarios ---'
rg -n -C 6 'refresh|register|consume|quota|count|429|Redis' src/test/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaIntegrationTest.kt
echo '--- read-only call-order verifier ---'
python3 - <<'PY'
from pathlib import Path
service = Path("src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt").read_text()
persistence = Path("src/main/kotlin/com/depromeet/piki/wishlist/service/WishPersistenceService.kt").read_text()
quota = Path("src/main/kotlin/com/depromeet/piki/common/ratelimit/ItemQuotaGuard.kt").read_text()
refresh_service = service[service.index("fun refreshWishItem"):service.index(" // 멱등 삭제")]
refresh_persistence = persistence[persistence.index("fun refresh("):]
checks = {
"quota_before_persistence_call": refresh_service.index("itemQuotaGuard.consume") < refresh_service.index("wishPersistenceService.refresh"),
"persistence_locks_wish_first": refresh_persistence.index("wishRepository.findByIdForUpdate") < refresh_persistence.index("itemSnapshotRepository.findById"),
"in_progress_returns_without_snapshot": "if (activeSnapshot.isInProgress()) return" in refresh_persistence,
"failed_rejected": "if (activeSnapshot.isFailed()) throw" in refresh_persistence,
"new_snapshot_saved": "itemSnapshotRepository.save(ItemSnapshot.pending" in refresh_persistence,
"quota_is_external_to_db": "store.tryConsume" in quota and "runCatching" in quota,
}
for k, v in checks.items():
print(f"{k}={v}")
if not all(checks.values()):
raise SystemExit("invariant failed")
PYRepository: TeamPiKi/core
Length of output: 42280
새 PENDING snapshot을 생성할 때만 refresh quota를 차감하세요.
현재 refreshWishItem은 상태 검증과 무관하게 Redis quota를 먼저 차감합니다. 존재하지 않는 위시, 타인 위시, 이미지 위시, FAILED 위시, PENDING·PROCESSING 재시도, 공유 중인 추출에 합류하는 요청도 quota를 소비합니다. Redis 차감은 DB 롤백으로 복구되지 않으므로, 반복 재시도 후 정상 READY refresh가 429로 거부될 수 있습니다.
wish 행 락 안에서 소유권·상태·진행 중 여부를 확인하고, 새 ItemSnapshot.pending(...)을 저장하는 분기에서만 quota를 차감하세요. quota 초과 시 snapshot을 저장하지 않아야 하며, Redis 차감 후 DB 저장 실패에 대한 보상 또는 예약 정책도 정의해야 합니다. 성공 refresh 1회, 진행 중 재시도·FAILED·이미지·타인 위시·공유 합류·동시 refresh의 quota 소비량을 테스트로 고정하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt`
around lines 276 - 280, refreshWishItem에서 itemQuotaGuard.consume 호출을 persistence
내부의 wish 행 락 이후 소유권·유형·상태·진행 여부가 검증된 뒤 새 ItemSnapshot.pending(...)을 저장하는 분기로
이동하세요. quota 초과 시 snapshot을 저장하지 않도록 하고, Redis 차감 후 DB 저장 실패 시 보상하거나 예약 정책을
적용하세요. 성공한 READY refresh만 1을 소비하고 진행 중 재시도·FAILED·이미지·타인 위시·공유 합류·동시 refresh는
소비하지 않도록 관련 테스트를 추가·수정하세요.
Source: Path instructions
Collaborator
Author
There was a problem hiding this comment.
Rejected - 반영하지 않았습니다.
지적의 취지(거부되는 refresh 요청이 몫을 깎으면 안 된다)는 타당합니다. 다만 "새 PENDING snapshot 을 만들 때만" 이라는 판정은 wishPersistenceService.refresh 의 락 안에서 최종 확정됩니다. 거기서 차감하면 Redis 호출이 트랜잭션 안으로 들어가는데, 이는 CLAUDE.md 의 "외부 호출은 트랜잭션 밖에서" 와 충돌합니다 (커넥션을 잡은 채 외부 I/O 를 기다리게 됨).
멱등 재요청(이미 PROCESSING 인 항목에 refresh 연타)에도 차감되는 문제는 실재하므로, 파싱 완료 후 실제 소비를 정산하는 후속 과제 #910 에서 함께 다루겠습니다. 그쪽이 "실제로 파싱이 돌았는가" 를 사후에 아는 유일한 지점입니다.
resolve 하지 않고 두니 검토 후 판단해 주세요.
There was a problem hiding this comment.
@m-a-king, 설명을 확인했습니다.
wish 행 락 안에서 Redis를 호출하라는 기존 제안은 CLAUDE.md의 외부 I/O 규칙과 충돌합니다. 이 부분은 현재 PR에 적절한 수정 방향이 아닙니다.
다만 현재 구현은 PENDING·PROCESSING 상태의 멱등 재요청과 이후 거부되는 요청에도 quota를 소비합니다. 이 동작은 실제 사용자 quota를 소진시킬 수 있습니다. 해당 문제를 실제 파싱 완료 후 정산하는 #910에서 처리한다는 범위 판단은 이해했습니다.
변경이 이 PR에는 포함되지 않았으므로, 후속 작업의 추적성을 위해 스레드는 열린 상태로 유지하겠습니다.
🐇 …
✏️ Learnings added
Learnt from: m-a-king
Repo: TeamPiKi/core PR: 904
File: src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt:280-284
Timestamp: 2026-08-12T08:50:13.153Z
Learning: 이 저장소에서는 `CLAUDE.md` 규칙에 따라 Redis 등 외부 I/O를 DB 트랜잭션과 `FOR UPDATE` 행 락 내부에서 호출하지 않는다. `WishPersistenceService.refresh`는 행 락 안에서 새 PENDING snapshot 생성 여부를 최종 판정한다.
Learnt from: m-a-king
Repo: TeamPiKi/core PR: 904
File: src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt:280-284
Timestamp: 2026-08-12T08:50:13.153Z
Learning: 위시리스트 refresh의 quota 정산은 현재 요청 시점 차감 방식이다. PENDING 또는 PROCESSING 상태의 멱등 재요청에도 quota가 소비되는 문제는 실제 파싱 완료 후 소비를 정산하는 후속 이슈 `#910에서` 처리한다.
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.
You are interacting with an AI system.
- ItemQuotaException.exceeded 가 retryAfterSeconds 를 검증하지 않아 0·음수로도 만들어졌다. 지금 유일한 호출자(RedisItemQuotaStore)가 최소 1초를 보장하지만 그건 그쪽 사정이라, 어느 호출자가 오든 유효한 Retry-After 가 나가도록 팩토리 불변식으로 못박는다 - ItemQuotaGuard 의 fail-open 을 runCatching 에서 catch(Exception) 으로 바꾼다. runCatching 은 Throwable 을 잡아 OutOfMemoryError 같은 치명적 Error 까지 삼키는데, 그 상황에서 요청을 계속 받으면 장애를 키운다 - 창 길이 검증을 Duration 양수에서 toMillis() 양수로 바꾼다. Redis PEXPIRE 가 ms 단위라 0.5ms 같은 값은 양수여도 환산이 0 이 되어 창이 즉시 만료되고, 매 요청이 새 창을 열어 한도가 조용히 무제한이 된다 - 토너먼트 생성 게이트가 탈퇴(tombstone) 계정을 통과시켰다. anonymize 는 닉네임·프로필만 비우고 identityType 은 MEMBER 로 남기므로 identityType 만 보면 죽은 계정이 토너먼트를 만든다. 위시가 findActiveById 로 막는 것과 같은 사유(#691)라 deletedAt 도 함께 본다. 그에 따라 409 응답 문서화·example 도 복구 - 이미지 presign 경로가 content-type 검증 전에 차감하고 있었다. 지원하지 않는 MIME 을 보낸 요청이 몫을 깎고 400 을 받는 순서라, v1(ProductImage.of 로 먼저 거름)과 맞춰 검증을 차감 앞으로 당긴다 - 429 응답에 Retry-After 헤더를 OpenAPI 로 선언한다. 값은 이미 내려가고 있었지만 description 에만 적혀 있어 클라가 스펙으로 읽을 수 없었다 - 토너먼트 429 문구 테스트가 "토너먼트가 들어있다" 만 확인해 의도한 규칙("오너의 사용량을 드러내지 않는다")을 전혀 검증하지 못했다. 금지 단어 부재로 고정해, 문구를 "오너의 남은 사용량이 0이에요" 로 바꾸면 깨지게 한다 - 새로 건 require 3종(재시도 시점 양수·창 1ms 하한·탈퇴 계정 차단)에 검증 테스트를 함께 추가 - reject 1건: "refresh 는 새 PENDING snapshot 을 만들 때만 차감하라" 는 지적은 반영하지 않았다. 그 판정이 persistence 의 락 안에서 최종 확정되는데, 거기서 차감하면 Redis 호출이 트랜잭션 안으로 들어가 CLAUDE.md 의 "외부 호출은 트랜잭션 밖" 과 충돌한다. 멱등 재요청(이미 PROCESSING)에도 차감되는 문제는 실재하므로 사후 정산(#910)에서 함께 다룬다
- presign 에서 차감하고 confirm 은 0 이라는 계약이 코드 주석에만 있고 테스트로 고정돼 있지 않았다. CodeRabbit nitpick 지적대로 성공 경로(발급 2장 → confirm 201)를 태워 카운터가 2에서 안 움직이는지 단언한다 - 공유 stub 특성상 exists 동작을 이 테스트가 명시 세팅한다 (다른 테스트가 false 로 바꿔둔 상태를 물려받지 않게) - 카운터 값을 담은 변수명이 retryAfter 로 잘못돼 있던 것을 정정하고, 폴링 테스트의 신분 리터럴을 IdentityType.MEMBER.name 으로 바꾼다
Collaborator
Author
|
CodeRabbit 리뷰 대응 정리입니다. 인라인 thread 9건 — 8건 accept( review body nitpick 4건 — 3건 반영(
reject 한 1건( |
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.
Situation
nginx IP 레이트리밋(#332)이 있지만 IP 는 거친 1차 방어다. 비싼 추출 경로는 전부 인증이 필요하니 계정 단위로 정밀하게 걸 수 있다는 것이 이슈의 출발점이었다.
설계에 들어가면서 세 가지가 드러났다.
요청 1건과 실제 소비가 1:1 이 아니다. 이미지 등록은 한 요청이 최대 5장이고 장마다 추출이 따로 돈다. 요청 수로 세면 링크 1건과 이미지 5장이 같은 비용으로 취급된다.
비용은 LLM 만이 아니다. 파싱이 파서로 풀려 LLM 을 안 타도 아래가 그대로 소모된다. 특히 residential proxy 는 사용량 과금이라 LLM 과 별개로 돈이 나간다.
게스트 계정이 무한 발급된다.
POST /api/v1/auth/guest는 인증도, 입력값도, 기기 식별자도 받지 않는다. dev 에 3연속 호출해 서로 다른 계정 3개가 즉시 나오는 것을 확인했다. 그러면 userId 를 키로 쓰는 어떤 한도도 계정을 갈아타면 리셋된다. 그리고 토너먼트 아이템 등록은 게스트에게 열려 있었다.Task
Action
우회를 막는 방법 선택
세 안을 저울질했다.
채택 안의 핵심은 OAuth provider 가 대신 sybil 방어를 해준다는 점이다. 카카오/구글/애플 계정을 대량 생성하는 것은 그 자체로 비싸고, 각 provider 가 자기 abuse 방어에 훨씬 많은 비용을 쓴다. 우리가 IP 대역과 기기 지문을 직접 관리하는 것보다 낫다.
공격 비용의 병목이 IP 에서 전화번호로 옮겨간 셈이라 VPN 이나 residential proxy 로는 우회되지 않는다. 이 선택으로 IP 키와 deviceId 억제가 둘 다 불필요해졌다.
게스트 권한 정리
원칙은 소비는 게스트, 생산은 회원이다. 위시리스트는 이미 회원 전용이었고 토너먼트만 그 원칙에서 빠져 있어, 새 원칙이 아니라 기존 원칙의 일관 적용이다.
한도 설계
차감 단위는 큐에 넣는 item 수다. 링크 1건은 1, 이미지 5장은 5 를 소모한다.
판정은 잔액 방식이다. 남은 몫이 있으면 요청 크기와 무관하게 통과시키고, 넘긴 만큼은 다음 요청이 갚는다. 요청량은 판정에 쓰지 않고 차감에만 쓴다.
누적 + 요청량 > 한도면 거부누적 >= 한도면 거부대가로 잔액이 음수가 될 수 있지만 창당 최대 소비가 (한도 + 1회 최대 요청량)으로 바운드된다. 한도 10, 이미지 5장 기준 최악 -4 이고 그 뒤로는 전부 거부되므로 무한 초과가 아니다.
경로별 차등은 두지 않는다. 파서로 풀리는 사이트가 훨씬 싼 건 맞지만, 등록 시점엔 어느 경로로 풀릴지 알 수 없다. 29cm 가 파서로 풀린다는 건 실측이지 보장이 아니고, 사이트가 마크업을 바꾸면 그날부터 LLM fallback 으로 간다. 실제 소비량에 맞춘 정산은 파싱이 끝난 뒤에야 가능해서 후속 과제(#910)로 뺐다.
토너먼트 축은 요청자가 아니라 토너먼트 오너의 몫에서 깎는다. 참여자에 게스트가 섞이는데 요청자 기준으로 세면 계정을 갈아타며 리셋할 수 있다. 오너는 반드시 회원이라 그 우회가 성립하지 않는다.
오너가 체감하지 않게 하는 방법으로 차감 가중치(0.5 등)를 검토했으나 한도를 키우는 쪽으로 정했다. 실제 비용은 게스트가 넣든 오너가 넣든 1인데 0.5 로 세면 카운터가 실제 소비량과 어긋나 메트릭으로 읽을 수 없게 된다. 차감은 1:1 로 두고 토너먼트 한도를 위시보다 크게 잡았다.
위시와 토너먼트를 별개 키로 분리했다. 한 축으로 합치면 친구들이 내 토너먼트에 아이템을 넣은 만큼 내가 내 위시리스트를 못 쓰게 된다.
quota:item:wish:{userId}quota:item:tournament:{ownerId}이미지 v2 는 presign 시점에 차감하고 confirm 은 차감하지 않는다. confirm 이 안 와도 폴링 백스톱이 pending 을 회수해 큐에 넣으므로, confirm 에서만 세면 그 경로가 통째로 한도를 우회한다.
구현
저장소는 Bucket4j 대신 Lua 고정 윈도우로 두었다. Bucket4j 는 버킷 상태를 객체로 직렬화해 Redis 에 저장해서 무중단 배포 중 구버전과 신버전의 호환성을 테스트로 고정해야 한다(테스트 규약의 직렬화/호환성 분류). 필요한 것은 창당 N 개라는 카운터뿐이고, 문자열만 저장하면 그 부담이 사라진다. 기존 Redis 사용(
RedisRefreshTokenStore)과도 같은 방식이다.판정과 차감을 한 스크립트로 원자화하고, 거부 시에는 카운터를 올리지 않는다. 거부분까지 누적하면 한도에 걸린 사용자가 재시도할수록 카운터가 올라 창이 끝나도 넘긴 상태로 시작한다.
Redis 장애는 통과시킨다(fail-open). 한도 인프라 때문에 등록이 멈추는 것보다 낫고, Redis 가 죽으면 refresh 토큰 저장소도 함께 죽어 그 창에서 대량 호출이 지속되기 어렵다.
429 응답에
Retry-After를 싣는다.ErrorCategory.TOO_MANY_REQUESTS를 신설하고,RetryAfter인터페이스를 구현한 예외만 헤더를 받는다. 예외 클래스 전체에 nullable 필드를 두는 대신 타입으로 가려서, 재시도 시점을 모르는 예외에 0 같은 거짓값이 실리지 않는다.토너먼트 429 문구는 오너의 사용량을 드러내지 않는다. 이 응답은 참여 게스트도 받는데, 남의 사용량은 요청자에게 알릴 정보가 아니다.
인터셉터 대신 서비스에서 부르는 이유
처음에는 인터셉터와 어노테이션 조합을 생각했으나 두 가지가 걸렸다. 차감 주체가 요청자가 아닐 수 있어(토너먼트는 tournamentId 로 오너를 찾아야 안다) 핸들러 진입 시점에는 모르고, 차감량이 이미지 장수에 달려 있어 인터셉터에서 세려면 multipart 본문을 두 번 읽어야 한다.
Result
ITEM_QUOTA_*환경변수로 배포 없이 조정할 수 있고,enabled=false로 통째로 끌 수 있다. 실측 없이 정한 초기값이라 429 발생률을 보며 조일 것을 전제로 한다CommonErrorCode.SERVER_BUSY가 "실제 발생은 후속 이슈" 라며 비워둔 자리가 정확히 그것이다piki_llmzone(IP별 분당 30 = 시간당 1,800)이 잉여가 된다. 같은 경로에 계정별 시간당 10 이 걸리므로 180배 느슨한 쪽은 닿을 일이 없다. 제거는 계정별 한도가 실제로 도는 것을 확인한 뒤가 안전해서 계정별 한도로 대체된 nginx piki_llm zone 제거 #928 로 분리했다. IP별 초당 20 인piki_general은 성격이 달라(미인증 스캔까지 인증 필터 이전에 끊는다) 그대로 둔다users행을 무한히 만든다. 기준을 "외부 비용" 으로 넓히면 이들도 대상이라 별도로 다룰 필요가 있다연관 이슈
Summary by CodeRabbit
새 기능
429 Too Many Requests와 재시도 가능 시간이 안내됩니다.문서
Retry-After헤더 동작을 문서화했습니다.버그 수정