링크 등록 재시도가 한도를 깎고 겹친 항목을 못 가리키던 문제 해소 - #999
Merged
Merged
Conversation
- 지금까지 이 프로젝트의 모든 에러 응답은 data 가 null 이었다. fail() 팩토리가 그렇게 고정돼 있어, 사유(code)만으로 클라가 다음 행동을 정할 수 없는 경우에도 맥락을 줄 방법이 없었다 - 예외가 응답 계약의 일부를 나르는 패턴은 RetryAfter(재시도 시점을 아는 예외만 헤더를 싣는다)가 이미 쓰고 있어, 그것과 대칭인 인터페이스로 뒀다. 핸들러가 as? 로 가려 맥락을 아는 예외만 data 를 싣는다 - RetryAfter 와 달리 값을 nullable 로 둔 이유: 이 프로젝트의 도메인 예외는 사유가 클래스가 아니라 팩토리 단위로 갈려(WishException 하나가 모든 위시 사유를 다룬다) 구현 여부로 가릴 수 없다. 대신 null 이면 data 를 싣지 않아 기존 에러 응답 모양이 그대로 유지된다 - example 헬퍼(OpenApiExamples.add)도 payload 를 싣도록 함께 맞췄다. 핸들러와 lockstep 이 이 헬퍼의 존재 이유라, 한쪽만 싣게 두면 문서가 실제 응답과 어긋난다
- 응답이 유실된 뒤 클라가 같은 URL 로 재시도하면 두 가지가 어긋났다. 차감이 중복 판정보다 앞이라 409 로 끝나는 재시도도 몫을 소비했고(사용자는 담지도 못한 채 한도만 잃는다), 409 를 받은 클라는 어느 위시와 겹쳤는지 몰라 목록을 다시 조회해야 했다 - 차감 앞에 사전 중복 검사를 두고 최종 판정은 기존처럼 락 안에 남겼다. 이 코드베이스가 이미 쓰는 "사전 확인 + 최종 판정" 패턴이다. 차감을 등록 성공 뒤로 옮기는 안은 한도 검사 자체가 사후가 되어 몫이 0 인 사용자도 첫 요청이 통과하므로 택하지 않았다 - 409 응답에 겹친 리소스의 id 를 싣는다(위시는 wishId, 토너먼트는 tournamentItemId). status·code·detail 이 그대로라 기존 클라는 그대로 동작하는 하위 호환 확장이다 - 409 를 200 get-or-create 로 바꾸는 안도 검토했다. 이미 담긴 상품을 또 담는 것은 리소스 상태와의 충돌이라 409 가 의미상 맞고, 에러에 data 를 못 싣는다는 구현 제약 때문에 status 의미를 왜곡하는 것은 본말전도라 채택하지 않았다. #470 이 from-play-link 를 200 으로 바꾼 것과는 성격이 다르다 - 클론 생성은 여러 번 해도 되는 행위지만 같은 상품을 위시에 두 번 담는 것은 막아야 할 상태다 - data 에 응답 객체 전체를 싣는 안은 도메인 예외가 컨트롤러 DTO 를 참조하게 되어 계층이 뒤집힌다. 클라가 필요로 하는 것은 그 위시로의 이동이라 id 하나면 충분하고, 필드 추가는 비파괴라 요구가 생기면 넓힐 수 있다 - 다건 위시 담기의 409 에는 id 를 싣지 않는다. 요청 안의 중복과 기존 출전분과의 중복을 함께 다루고 후자도 여러 건일 수 있어 가리킬 아이템 하나를 고를 수 없다 - 작업 중 addItemFromLink 의 409 문서에 중복 아이템 사유가 통째로 빠져 있던 것을 발견해 함께 정정했다(응답 전수 문서화 규칙 위반) - 차감이 판정 뒤로 가면서 status 우선순위가 하나 바뀐다. 이미 담긴 상품이어도 몫이 없으면 429 가 나가던 것이 409 로 바뀌며, 중복은 한도와 무관한 사실이라 이쪽이 정확하다. 그 우선순위를 테스트로 고정했다
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 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 |
- ItemQuotaException 이 같은 문제(사유 하나가 추가 응답 데이터를 실어야 함)에 이미 답을 냈고 그 주석이 "도메인 예외에 nullable 필드를 떠안기지 말라"고 적어 뒀는데, 앞선 커밋이 그 선례를 거슬렀다. AlreadyRegisteredException 하나를 두고 code 는 도메인이 넘기는 형태로 맞춘다 - 그 결과 payload 가 non-null 이 되어 RetryAfter 와 같이 "구현 여부 = data 유무" 가 성립하고, WishException·TournamentException 의 나머지 20여 사유가 쓰지 않는 필드를 지지 않는다 - payload 타입을 Map<String, Any> 에서 ExistingWish·ExistingTournamentItem 으로 바꿨다. 응답 필드명이 프로퍼티에서 나오므로 문자열 키 상수 두 개가 사라지고, rename 이 컴파일로 드러난다 - duplicateTournamentItem 의 기본 인자를 없앴다. 다건 위시 담기가 id 를 못 주는 것은 의도인데, 기본값이 있으면 그 의도와 미이관이 시그니처로 구분되지 않는다 - raw link 중복 판정이 사전 확인과 최종 판정에 글자 그대로 복붙돼 있던 것을 duplicatedByRawLink 로 뽑아 두 곳이 공유한다. 링크 동등성 규칙이 바뀌어도 한쪽만 낡을 수 없다 - 사전 확인 메서드를 rejectIfAlreadyRegistered·rejectIfAlreadyAdded 로 바꿔 호출부가 throw 를 조립하지 않게 했다(verifyCanAddItems·rejectIfWithdrawnForUpdate 와 같은 결) - existingTournamentItemIdByItemId 가 중간 맵을 두 번 만들던 것을 associate 두 번으로, 위시 조회 관용구를 existingWishId 헬퍼로 접었다 - payload 근거를 네 파일에 반복해 적었던 주석을 ErrorPayload 정본 하나 + 포인터로 줄이고, example 상수가 주석대로 실제로 다른 example 과 묶이게 리터럴을 상수로 바꿨다
12 tasks
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
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
응답이 유실된 뒤 클라가 같은 URL 로 재시도하면 두 가지가 어긋났다.
토너먼트 링크 담기도 같은 구조였다.
Task
서버가 재시도와 "사용자가 진짜 두 번 담으려는 시도"를 구분할 수 없다는 것이 출발점이었다. 링크라는 자연 키가 이미 있는데 그 판정 결과를 실패로만 표현하고 있었다.
두 가지를 정해야 했다. 차감을 어디로 옮길지, 그리고 겹친 항목을 어떻게 알려줄지.
Action
차감 순서
차감 앞에 사전 중복 검사를 두고 최종 판정은 기존처럼 락 안에 남겼다. 이 코드베이스가 이미 쓰는 "사전 확인 + 최종 판정" 패턴이다. 락 밖 조회라 근사치이고, 병합 경합 창에서만 차감이 낭비된다.
409 를 유지하고 data 를 실었다
status 를 200 get-or-create 로 바꾸는 안을 먼저 검토했지만 채택하지 않았다.
ApiResponseBody.fail()이data = null로 고정이라 이걸 여는 것이 선행 작업이었다. 예외가 응답 계약의 일부를 나르는 패턴은RetryAfter(재시도 시점을 아는 예외만 헤더를 싣는다)가 이미 쓰고 있어, 그것과 대칭인ErrorPayload를 두고 핸들러가as?로 가리게 했다.payload 를 어느 클래스가 나를지는 한 번 되돌렸다. 처음에는
WishException·TournamentException에 nullable 필드로 얹었는데,ItemQuotaException이 같은 문제(사유 하나가 추가 데이터를 실어야 함)에 이미 답을 냈고 그 주석이 피하라고 적어 둔 형태였다.그 선례대로
AlreadyRegisteredException하나를 두고 code 는 도메인이 넘긴다. 그 결과payload가 non-null 이 되어RetryAfter와 같이 구현 여부가 곧 data 유무가 되고, 두 도메인 예외의 나머지 20여 사유가 쓰지 않는 필드를 지지 않는다. 타입도Map<String, Any>대신ExistingWish/ExistingTournamentItem이라 응답 필드명이 프로퍼티에서 나온다(문자열 키 상수가 사라지고 rename 이 컴파일로 드러난다).example 헬퍼도 함께 맞췄다. 핸들러와 lockstep 이 그 헬퍼의 존재 이유라 한쪽만 싣게 두면 문서가 실제 응답과 어긋난다.
응답 계약
WishItemResponseWISH-009{ "wishId": 1024 }WISH-010{ "tournamentItemId": 1 }로 대칭이다./items/wish)의 409 에는 싣지 않는다. 요청 안의 중복과 기존 출전분과의 중복을 함께 다루고 후자도 여러 건일 수 있어 가리킬 항목 하나를 고를 수 없다.작업 중 발견해 함께 고친 것
addItemFromLink의 409 문서에 중복 아이템 사유가 통째로 빠져 있었다. 응답 전수 문서화 규칙 위반이라 description 과 example 을 함께 채웠다.Result
refresh경로 주석이 같은 구멍을 이미 인정하고 있어, 사유가 늘 때마다 사전 검사를 복제하지 않으려면ItemQuotaGuard에 환불 경로가 필요하다. 그건 fail-open 창이라는 새 트레이드오프를 지므로 아이템 등록 한도를 실제 소비량 기반 사후 정산으로 정밀화 #910 과 함께 볼 별건으로 남긴다.연관 이슈