Skip to content

[TMT-226] feat: 공개 리뷰 상세·삭제 실구현 — 티켓 회수 · 사진 완전 삭제 · 집계 되돌림 - #73

Open
mingdodev wants to merge 3 commits into
mainfrom
feat/TMT-226-review-detail-delete
Open

[TMT-226] feat: 공개 리뷰 상세·삭제 실구현 — 티켓 회수 · 사진 완전 삭제 · 집계 되돌림#73
mingdodev wants to merge 3 commits into
mainfrom
feat/TMT-226-review-detail-delete

Conversation

@mingdodev

Copy link
Copy Markdown
Member

Related Issue

TMT-226 — [리뷰] GET·DELETE /v1/reviews/{reviewId} · 명세 v2 F·G·I §6-3·§6-4

Why

티켓 회수를 되돌리기 작업의 맨 앞에 뒀다. 회수할 AVAILABLE 티켓이 0장이면 그 이전에 아무것도
건드리지 않은 채로 409가 나가야 한다. 이미 그룹 가입에 쓴 티켓은 돌아오지 않으니 삭제 자체를 막는 것이
규칙이고(R7), 순서를 뒤로 두면 집계·공유를 먼저 되돌린 뒤 롤백에 의존하게 된다.

어느 티켓을 회수하는지는 한 문장의 조건부 UPDATE로 정했다. 읽어서 고르고 쓰면 동시 요청이 같은 장을
두 번 회수한다. 티켓은 서로 구분되지 않지만, 이 리뷰가 발급한 장이 아직 AVAILABLE이면 그것부터 고르도록
ORDER BY에 넣었다 — 회수 로그를 봤을 때 인과가 읽히는 편이 낫다.

S3 객체 삭제는 커밋 후 이벤트로 뺐다. 트랜잭션 안에서 지우면 롤백돼도 객체는 이미 사라져 살아 있는
리뷰의 사진이 깨진다. 반대 순서에서 실패하면 아무도 참조하지 않는 객체만 남는다 — 화면에 영향이 없고
나중에 걷어낼 수 있어 이쪽을 골랐다. ReviewCommittedEvent(TMT-232)와 같은 방식이다.

티켓 409용 예외를 tmt-common에 새로 만들었다. 같은 형태를 mock의 TicketRequiredException이 이미
쓰고 있지만, mock 코드를 application·persistence가 참조하면 안 되고 실구현이 mock 패키지에 의존하는
것도 피해야 해서 별도 타입을 뒀다. 그룹 가입 실구현이 들어올 때 같은 예외를 재사용하면 된다.

티켓과 명세가 갈리는 곳은 명세를 따랐다. 티켓 승인 기준은 "남의 리뷰 삭제가 403"인데 명세 §6-4는
REVIEW_NOT_FOUND(404)다. 없는 리뷰와 타인의 리뷰가 같은 응답이어야 존재 여부가 새지 않는다(규약 §3-2).
GET도 같다.

What

공개 리뷰 상세와 리뷰 삭제가 실 DB 위에서 돈다. ReviewMockController가 서빙하던 두 엔드포인트를
넘겨받고 그 mock을 지웠다.

  • GET /v1/reviews/{reviewId} — 인증 선택. 비로그인도 조회되고 그때 isMine은 false다 (G2).
    review 행이 있는 것만 대상이라 미완성 저장은 조회되지 않는다 (R8). aiSummary는 요약 행이 없으면
    null (A2). 사진·태그·요약은 카드와 같은 배치 조회(ReviewCardLookupPort)를 재사용해서, 상세와 카드가
    서로 다른 순서·URL 규칙을 갖지 않는다.
  • DELETE /v1/reviews/{reviewId} — 티켓 1장 회수, 그룹 공유 해제와 그룹 지표 재계산, 매장
    review_count·rating_sum 차감, 사진(save_photo·media_asset·S3 객체) 완전 삭제, 리뷰·저장 소프트
    삭제가 한 트랜잭션이다 (TX-5). 저장으로 되돌아가지 않는다 (R6).
  • 회수할 티켓이 0장이면 REVIEW_DELETE_TICKET_REQUIRED(409)로 거부하고 본문에
    ticket: { requiredCount, availableCount, shortageCount }를 함께 싣는다 (규약 §3-2).

스키마 변경은 없다. 새 마이그레이션도 docs/DB-SCHEMA.md 갱신도 필요하지 않았다 (명세 §7과 일치).
production 의존성도 추가하지 않았다.

How

컨트롤러(tmt-input-http) → 유스케이스(tmt-application) → 어댑터(tmt-output-persistence/postgres)
경계를 그대로 따랐다. 삭제 쪽 흐름은 이렇다.

  1. ReviewQueryPort.findReviewForDeletion — 삭제 안 된 리뷰 + 소유자 + 되돌릴 rating. 없거나 남의
    것이면 여기서 404.
  2. GroupJoinTicketPort.revokeOneForReview — 조건부 UPDATE 한 문장. 0행이면 TicketShortageException.
  3. 공유 그룹 id를 내리기 전에 조회 → unshareByReview → 그룹별 refreshShareStats. 순서를 바꾸면
    어느 그룹이었는지 알 방법이 없다.
  4. PlaceStatsPort.removeReview — 이미 있던 것을 그대로 썼다. 음수 방지 술어까지 포함해 손대지 않았다.
  5. save_photo를 먼저 지우고 media_asset을 지운다. 스키마에 ON DELETE CASCADE가 없어서(명세 §7의
    8/31 정정) 애플리케이션이 순서대로 지운다.
  6. review·save는 조건부 UPDATE로 소프트 삭제 (D6). 이미 지워진 행의 시각을 덮어쓰지 않는다.
  7. 커밋 후 ReviewPhotosDeletedEvent 리스너가 S3 객체를 지운다.

기존 동작을 바꾼 부분: ExceptionAdviceTicketShortageException 핸들러가 추가됐다. mock의
MockTicketExceptionAdvice(@Order(HIGHEST_PRECEDENCE))는 별개 예외 타입을 보므로 그대로 두었고
그룹 가입 mock은 영향받지 않는다.

계약이 어떻게 달라지나 (Confluence 변경 이력은 릴리즈 태그 시점에 한 줄로 묶는다는 합의에 따라 여기 적는다):

항목 mock 실구현 영향도
reviewId 표기 review_1 rv_1 Breaking — 다만 TMT-228에서 정한 실구현 표기이고, 이미 머지된 카드·가게 상세가 rv_를 내리고 있어 이번에 상세/삭제가 거기에 맞춰진 것이다
그 외 응답 필드 동일 없음
조회 대상 mock 인메모리 시드 실 DB mock 시드로 만든 리뷰 id는 더 이상 조회되지 않는다

필드명·타입·place_/user_/sp_ 접두와 createdAt 문자열 표기는 mock과 같다.

Notes for Reviewer

집중해서 볼 곳

  • GroupJoinTicketRepository.revokeOneForReview의 네이티브 UPDATE. 서브쿼리로 1장을 고르고 바깥에서도
    status = 'AVAILABLE'을 다시 거는 이유가 동시 회수 방지다.
  • ReviewDeletionService의 단계 순서. 특히 3번(공유 그룹 id를 먼저 조회)과 5번(save_photo → media_asset).

명세에 없어서 임의로 정한 것

  • 회수 대상 티켓 선택 규칙 — 명세는 "회수할 AVAILABLE 티켓이 없으면 거부"까지만 말한다. 이 리뷰가
    발급한 장을 우선, 없으면 가장 오래된 AVAILABLE 장을 고르도록 정했다.
  • review_ai_summary 행은 지우지 않는다 — 리뷰가 소프트 삭제라 FK가 살아 있고, 조회는 deleted_at으로
    막힌다. 하드 삭제 정책이 생기면 같이 정리할 대상이다.
  • 잘못된 형식의 reviewId(rv_ 접두가 아니거나 숫자가 아닌 값)는 404parsePlaceId가 이미 쓰던
    방식과 같다. 존재 여부를 새지 않는 쪽이다.
  • S3 삭제 실패는 경고 로그만 남기고 삼킨다 — 커밋된 삭제를 되돌릴 수 없고, 남는 것은 참조 없는 객체뿐이다.

검증한 것 / 못 한 것

  • ./gradlew ktlintFormat./gradlew build 통과. 새 테스트 15개(application 11 · input-http 4)가 전부
    통과한다. 승인 기준 대응: 티켓 0장 거부 + 응답의 티켓 상태 / 매장 집계 차감 / 그룹 공유 해제·지표 재계산 /
    타인·없는 리뷰 404 / 비로그인 isMine: false / aiSummary null.
  • 못 한 것: 이 레포에 DB 통합 테스트 환경이 아직 없어(TMT-295, PR [TMT-295] test: persistence 통합 테스트 환경 — Testcontainers PostGIS #70 대기) 네이티브 쿼리 3종은
    실 DB에서 실행 검증하지 못했다. 티켓 회수 UPDATE·리뷰 상세 조인·삭제 조회가 여기 해당한다. 스키마
    변경이 없어 ddl-auto: validate 기동은 영향받지 않지만, 리뷰 시 쿼리 자체를 눈으로 봐 주면 좋겠다.
  • 애플리케이션 컨텍스트 기동으로 빈 배선(ReviewQueryAdapter·ReviewCommandAdapter·리스너)을 확인하지
    못했다 — tmt-bootstrap에 테스트 소스가 없다.

범위 밖으로 남긴 것

  • ReviewMockController만 지웠다. 다른 mock 컨트롤러와 공유 헬퍼는 그대로다. 확인해 보니 이번 삭제로
    안 쓰이게 된 헬퍼는 없다 — MockAiSummaryStore·MockMediaUrls·MockTicketLedger·ReviewFormRules
    전부 다른 mock이 계속 쓴다.
  • 공통 응답 DTO(ReviewCardResponse·Author 등)는 읽기만 했다. 시그니처 변경 없음.
  • 그룹 가입(H §2-2)의 티켓 409는 아직 mock이다. 새로 만든 TicketShortageException을 그쪽 실구현이
    재사용할 수 있다.

Prompt Log

mock을 실구현으로 옮기는 선례(SaveController·PlaceDetailController)를 먼저 읽고 그 경계를 그대로
따르는 방향으로 잡았다.

처음에는 삭제를 "리뷰 행을 지우고 사진을 정리한다" 정도로 봤는데, 명세 §5-2가 임시저장 삭제와 리뷰 삭제를
티켓 때문에 가른다고 명시한 것을 보고 티켓 회수를 흐름의 첫 단계로 올렸다. 되돌리기의 순서 자체가 이
티켓의 내용이라는 판단이다.

ON DELETE CASCADE로 자식 행이 정리된다고 가정하고 시작했다가, 명세 §7의 2026-08-31 정정과 V1__init.sql
대조해 스키마에 CASCADE가 한 건도 없음을 확인하고 save_photo를 명시적으로 먼저 지우는 쪽으로 고쳤다.

검토했다 접은 대안이 둘 있다. (1) 티켓 회수를 countAvailable 조회 후 UPDATE로 나누는 방식 — 동시
요청이 같은 장을 두 번 회수할 수 있어 한 문장으로 합쳤다. (2) S3 삭제를 트랜잭션 안에 두는 방식 —
MediaPurgeService가 "S3 먼저, 행 나중"을 쓰길래 같은 순서를 고려했으나, TTL 정리와 달리 여기서는 롤백 시
살아 있는 리뷰의 사진이 사라지는 경로가 생겨 커밋 후 이벤트로 뺐다.

LIMIT 1
)
""",
nativeQuery = true,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

후속 — TMT-295(#70) 머지 후 통합 테스트를 추가할 예정입니다.

이 조건부 UPDATE는 Fake로 검증이 안 됩니다. 회수할 티켓을 한 문장으로 고르고 갱신하는데, 갱신 행 수가 0인 경우(동시 삭제·이미 소비됨)를 실제 트랜잭션 경합 없이는 재현할 수 없습니다. TMT-227의 티켓 소비와 정확히 같은 종류라 그쪽과 같은 바닥 위에서 함께 다루는 게 맞습니다.

리뷰 상세 조인과 삭제용 조회도 실 DB 실행 검증이 남아 있습니다.

@wnsvy607 wnsvy607 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

되돌리기 순서(티켓 → 공유/지표 → 집계 → 사진 → 소프트 삭제 → 커밋 후 S3), 404로 존재 감추기, CASCADE 부재 대응, @TransactionalEventListener 분리(롤백 시 살아있는 사진 보호)까지 설계 판단이 일관되게 좋고 근거가 코드에 남아 있습니다. 유일한 블로커 하나만 고치면 머지 가능합니다.

[must] ReviewDeletionService.kt:66softDeleteReview의 반환값(갱신 행 수, 포트 시그니처가 Int인 이유)을 버리고 있어, 같은 리뷰에 대한 동시 중복 DELETE의 특정 인터리빙에서 매장 집계 이중 차감 + 티켓 한 장 추가 회수가 커밋될 수 있습니다. 시나리오: tx2의 findReviewForDeletion이 tx1 커밋 전에 리뷰를 보고, revokeOneForReview가 tx1 커밋 후 실행되면 — 이 리뷰가 발급한 티켓은 이미 REVOKED라 서브쿼리의 CASE … ELSE 1 폴백이 사용자의 다른 AVAILABLE 티켓을 골라 성공합니다. 이후 removeReview가 한 번 더 차감되고(음수 가드는 이중 차감을 못 막음), softDeleteReview는 0행을 돌려주지만 무시된 채 커밋돼요. 수정은 두 줄입니다:

if (reviewCommandPort.softDeleteReview(reviewId) == 0) throw TmtException(ErrorCode.REVIEW_NOT_FOUND)

예외가 트랜잭션 전체(티켓·집계)를 롤백해 경합이 닫힙니다. 이 레포가 D6·티켓 회수에서 세운 "조건부 UPDATE의 행 수가 정본" 원칙을 이 지점에만 적용하지 않은 형태예요.

[want] mock/MockTicketExceptionAdvice.kt:23ReviewDeleteTicketRequiredException은 삭제된 ReviewMockController가 유일한 사용처였고 이제 선언만 남은 데드 코드입니다(grep 확인). 본문의 "안 쓰이게 된 헬퍼는 없다"와 어긋나요. 그룹 가입 mock이 쓰는 나머지는 남기고 이 클래스만 지우면 됩니다.

[q] revokeOneForReview의 서브쿼리가 uncorrelated라 두 동시 삭제가 같은 '가장 오래된' 티켓을 고르면 두 번째가 잔여 티켓이 있어도 스퓨리어스 409가 납니다(손상 없음, 재시도 회복 — 이때 응답 availableCount가 1 이상으로 나가는 부수 효과 포함). 이미 라인에 TMT-295 경합 테스트 코멘트를 달아두셨으니 그 목록에 이 케이스도 명시해 주세요.

[q] S3 삭제 실패를 warn으로 삼킨 시점엔 media_asset 행이 이미 하드 삭제돼 고아 객체를 DB로 열거할 수단이 없습니다(정리 인덱스는 STAGED 전용). "나중에 걷어낼 수 있어"는 현재로선 S3 인벤토리 대조 전제예요. 고아 수용이 팀 결정이면 그대로, 아니면 tombstone/지연 큐가 후속 티켓감입니다.

@mingdodev

Copy link
Copy Markdown
Member Author

[must]·[want] 반영했습니다. 경합 시나리오가 정확했습니다 — ORDER BY CASE ... ELSE 1 폴백이 이 리뷰가 발급한 티켓이 아닌 다른 장을 골라 성공한다는 게 핵심이었네요. 포트 주석에 "이미 삭제된 행이면 0을 돌려준다"고 적어놓고 정작 호출부가 안 보고 있었습니다.

if (reviewCommandPort.softDeleteReview(reviewId) == 0) throw TmtException(ErrorCode.REVIEW_NOT_FOUND)

[want] 데드 코드ReviewDeleteTicketRequiredException만 지웠습니다. 실구현이 TicketShortageException(common)을 쓰면서 남은 것이고, TicketRequiredException·GroupJoinTicketRequiredException은 그룹 가입 mock이 계속 씁니다. 본문의 "안 쓰이게 된 헬퍼 없다"가 틀렸습니다.

테스트로 못 잡는 부분이 있어 적어둡니다

회귀 테스트를 넣다가 알게 된 건데, Fake로는 롤백을 검증할 수 없습니다. 두 번째 삭제가 예외를 던져도 Fake 포트에는 removeReview 호출이 그대로 남아 있어 "집계가 안 깎였다"를 단언할 수 없습니다. 그래서 테스트는 가드가 REVIEW_NOT_FOUND를 던지는 것까지만 확인하고, 되돌림 무효화는 TMT-295 통합 테스트로 넘긴다고 주석에 남겼습니다.

[q] 두 건

스퓨리어스 409 — uncorrelated 서브쿼리라 동시 삭제 둘이 같은 '가장 오래된' 티켓을 고르면 두 번째가 잔여 티켓이 있어도 409가 나는 것, 그리고 그때 응답 availableCount가 1 이상으로 나가는 부수 효과까지 — TMT-295 라인 코멘트 목록에 추가하겠습니다.

S3 고아 객체 — 맞습니다. media_asset이 하드 삭제된 뒤라 DB로 열거할 수단이 없고, 정리 인덱스는 STAGED 전용이라 안 걸립니다. 지금은 S3 인벤토리 대조가 전제인 셈이고요. 팀 결정 사항으로 올리겠습니다 — 수용이면 그대로, 아니면 tombstone이나 지연 큐가 후속 티켓입니다.

./gradlew build 통과.

@mingdodev
mingdodev requested a review from wnsvy607 September 1, 2026 12:52

@wnsvy607 wnsvy607 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

softDeleteReview(...) == 0 → REVIEW_NOT_FOUND로 경합이 닫힌 것 확인했고, 주석이 "왜 0행이 정본인지"를 다음 사람이 알 수 있게 잘 설명합니다. 재삭제 테스트 추가, 데드 예외 클래스 제거, 그리고 save를 함께 내리는 이유(R6 우회 차단)를 주석으로 남긴 것까지 — 제가 #69에서 [q]로 걸었던 "리뷰 삭제가 save도 내리는가"에 대한 답이 코드에 박혔습니다. approve합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants