feat: 도어용 유저 티켓 검증 API 구현 - #135 #136
Merged
Conversation
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
📝 WalkthroughSummary by CodeRabbit
WalkthroughPOST /api/tickets/confirm 엔드포인트와 요청 DTO( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as 도어스캐너
participant C as TicketController
participant S as TicketService
participant R as TicketRetriever
participant Repo as TicketRepository
participant U as TicketUpdater
participant E as TicketEntity
Client->>C: POST /api/tickets/confirm\n{ticketCode, checkCode}
C->>S: confirmTicket(ticketCode, checkCode)
S->>R: findTicketEntityByTicketCode(ticketCode)
R->>Repo: findByTicketCode(ticketCode)
Repo-->>R: Optional<TicketEntity>
R-->>S: TicketEntity | throw TicketNotFoundException
S->>S: verifyTicketStatus/verifyTicketDate\nverifyTicketCheckCode
alt 검증 성공
S->>E: updateTicketStatus(USED) (set usedTime)
S->>U: persist status 변경
U-->>S: OK
S-->>C: 성공 응답
C-->>Client: 200 OK
else 체크코드 불일치
S-->>C: throw BAD_REQUEST_TICKET_CHECK_CODE_ERROR
C-->>Client: 400
else 이미 사용/취소됨
S-->>C: throw CONFLICT_ALREADY_USED_TICKET / BAD_REQUEST_CANCELED_TICKET
C-->>Client: 409 / 400
else 날짜/유효성 오류
S-->>C: throw DateTicketException
C-->>Client: 400
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
43-43: 메서드 이름을 역할에 맞게 정리하면 가독성이 올라갑니다POST
/api/tickets/confirm엔드포인트가 GET용getUserBuyTicketInfo와 동일한 이름을 쓰고 있어 읽는 사람이 쉽게 혼동합니다.confirmTicket등 동작을 설명하는 이름으로 바꿔두면 유지보수 시 의도가 훨씬 분명해집니다.src/main/java/com/permitseoul/permitserver/domain/tickettype/core/component/TicketTypeRetriever.java (1)
24-27: 엔티티 재사용으로 조회 중복 제거 권장
이미 존재하는findTicketTypeEntityById를 활용하면 중복 쿼리를 줄이고 유지보수성이 향상됩니다.다음과 같이 내부 메서드를 재사용하면 깔끔합니다:
@Transactional(readOnly = true) public TicketType findTicketTypeById(final long ticketTypeId) { - return TicketType.fromEntity(ticketTypeRepository.findById(ticketTypeId).orElseThrow(TicketTypeNotfoundException::new)); + return TicketType.fromEntity(findTicketTypeEntityById(ticketTypeId)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java(3 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/req/TicketConfirmRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/res/EventTicketInfoResponse.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/res/UserBuyTicketInfoResponse.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/ConflictTicketException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/DateTicketException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/IllegalTicketException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java(6 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/core/component/TicketRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/core/domain/entity/TicketEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/core/repository/TicketRepository.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/tickettype/core/component/TicketTypeRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/com/permitseoul/permitserver/domain/tickettype/core/component/TicketTypeRetriever.java (1)
src/main/java/com/permitseoul/permitserver/domain/tickettype/core/exception/TicketTypeNotfoundException.java (1)
TicketTypeNotfoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/ticket/core/component/TicketRetriever.java (1)
src/main/java/com/permitseoul/permitserver/domain/ticket/core/exception/TicketNotFoundException.java (1)
TicketNotFoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/ConflictTicketException.java (1)
src/main/java/com/permitseoul/permitserver/domain/reservation/api/exception/ConflictReservationException.java (1)
ConflictReservationException(5-9)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java (7)
src/main/java/com/permitseoul/permitserver/domain/event/core/exception/EventNotfoundException.java (1)
EventNotfoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/ConflictTicketException.java (1)
ConflictTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/DateTicketException.java (1)
DateTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/IllegalTicketException.java (1)
IllegalTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/NotFoundTicketException.java (1)
NotFoundTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/core/exception/TicketNotFoundException.java (1)
TicketNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticket/api/TicketExceptionHandler.java (1)
Order(12-19)
🔇 Additional comments (10)
src/main/java/com/permitseoul/permitserver/domain/ticket/core/repository/TicketRepository.java (1)
16-17: 새 티켓 코드 조회 메서드 확인 완료
티켓 코드 기반 확인 흐름에 꼭 필요한 리포지토리 쿼리가 추가되어 일관성이 좋습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/res/EventTicketInfoResponse.java (1)
1-1: 패키지 재배치 적절
응답 DTO를.res패키지로 모으는 방향과 잘 맞습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/core/component/TicketRetriever.java (1)
19-22: 티켓 코드 검색 로직 적합
티켓 코드가 존재하지 않을 때 즉시 예외를 던져 API 오류 시그널과 일관됩니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/req/TicketConfirmRequest.java (1)
5-11: 요청 DTO 구성 적절
필수 필드에@NotBlank를 적용해 유효성 검증 흐름을 확보한 점이 좋습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/DateTicketException.java (1)
5-9: 날짜 검증 예외 정의 확인
도어 검증 시 날짜 오류를 명확히 전파할 수 있도록 예외가 잘 분리되었습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/ConflictTicketException.java (1)
5-9: 상태 충돌 예외 정의 확인
이미 사용된 티켓 충돌 시그널을 위한 예외 계층이 깔끔하게 추가되었습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/IllegalTicketException.java (1)
5-8: 예외 계층 구조 일관성 유지
TicketApiException을 그대로 상속해 ErrorCode만 위임하도록 한 구조가 기존 패턴과 잘 맞습니다. 이후 예외 매핑 시 추가 조정 없이 재사용 가능해 보여요.src/main/java/com/permitseoul/permitserver/domain/ticket/api/dto/res/UserBuyTicketInfoResponse.java (1)
6-29: 응답 DTO 패키지/네이밍 정리 확인응답용 DTO를
res패키지로 이동하고 명확히Response로 명명해 의도가 분명해졌습니다. 중첩 레코드/enum 구조도 이전과 동일하게 유지되어 하위 호환성 문제가 없어 보입니다.src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (2)
28-29: 도어 검증용 400 에러 코드 추가 적절체크 코드 검증 실패를 별도 ErrorCode로 분리해 컨트롤러에서 명확히 매핑할 수 있게 된 점 좋습니다. 기존 400번대와 충돌도 없네요.
102-103: 이미 사용된 티켓 대응 409 코드 확인이미 사용/취소된 티켓 시나리오를 40906으로 분리해 API 응답을 세분화한 구성 문제없습니다. 메시지도 문맥에 맞습니다.
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java
Show resolved
Hide resolved
Comment on lines
112
to
126
| @Transactional | ||
| public void confirmTicket(final String ticketCode, final String checkCodeFromTicket) { | ||
| try { | ||
| final TicketEntity ticketEntity = ticketRetriever.findTicketEntityByTicketCode(ticketCode); | ||
| verifyTicketStatus(ticketEntity.getStatus()); | ||
|
|
||
| final TicketType ticketType = ticketTypeRetriever.findTicketTypeById(ticketEntity.getTicketTypeId()); | ||
| verifyTicketDate(ticketType.getTicketStartAt(), ticketType.getTicketEndAt()); | ||
|
|
||
| final Event event = eventRetriever.findEventById(ticketEntity.getEventId()); | ||
| verifyTicketCheckCode(event.getTicketCheckCode(), checkCodeFromTicket); | ||
|
|
||
|
|
||
| ticketUpdater.updateTicketStatus(ticketEntity, TicketStatus.USED); | ||
| } catch (TicketNotFoundException e) { |
There was a problem hiding this comment.
문 상태 검증 API에 치명적인 경쟁 조건 존재
confirmTicket에서 티켓을 조회해 상태를 검사한 뒤, 별도의 잠금 없이 ticketUpdater.updateTicketStatus로 상태를 USED로 변경하고 있습니다. 서로 다른 트랜잭션에서 동시에 동일 티켓을 스캔하면 두 요청 모두 조회 시점에 RESERVED 상태를 보게 되고, 동시에 verifyTicketStatus를 통과하여 모두 성공해 버립니다. 결국 같은 티켓이 여러 번 사용 완료 처리되는 TOCTOU 문제가 생기며, 이는 티켓 중복 사용을 방지하려는 이 API의 핵심 목적을 무너뜨립니다.
티켓 엔터티를 비관적 락으로 조회하거나, update 단계에서 현재 상태를 조건으로 삼아 한 번만 성공하도록 방어 로직을 추가하는 등 동시성 제어가 필요합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java
around lines 112 to 126, the confirmTicket method has a TOCTOU race: it reads
the ticket status then updates without concurrency control. Fix by ensuring
atomic status transition — either retrieve the TicketEntity with a pessimistic
lock inside the @Transactional method (e.g., use repository/findById with
PESSIMISTIC_WRITE) before verifying status, or make the update operation
conditional (execute an update that sets status=USED only where id=? and
status=RESERVED and check the affected-row count, throwing an exception if 0).
Ensure the method remains transactional and remove the separate non-atomic
read+write sequence so only one concurrent caller can successfully mark the
ticket USED.
Comment on lines
60
to
63
| public void updateTicketStatus(final TicketStatus status) { | ||
| this.status = status; | ||
| this.usedTime = LocalDateTime.now(); | ||
| } |
There was a problem hiding this comment.
상태 업데이트 시 usedTime이 항상 갱신되고 있습니다
updateTicketStatus는 USED 외의 다른 상태 전환(예: RESERVED, CANCELLED)에도 사용되는데, 현재 구현은 어떤 상태로 바꾸더라도 usedTime을 지금 시각으로 덮어씁니다. 그 결과 실제로 사용되지 않은 티켓도 사용된 것처럼 기록되어 도어 검증/통계 로직에 치명적인 오동작이 발생합니다. 반드시 USED일 때만 사용 시각을 남기고, 그 외 상태에서는 usedTime을 초기화하거나 유지하도록 분기해 주세요.
public void updateTicketStatus(final TicketStatus status) {
this.status = status;
- this.usedTime = LocalDateTime.now();
+ if (status == TicketStatus.USED) {
+ this.usedTime = LocalDateTime.now();
+ } else {
+ this.usedTime = null;
+ }
}🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/ticket/core/domain/entity/TicketEntity.java
around lines 60-63, change updateTicketStatus so that usedTime is set to
LocalDateTime.now() only when the new status equals TicketStatus.USED, and for
any other status clear usedTime (set to null) or leave it unchanged per domain
rule; implement an if/else branch to perform this conditional assignment so
non-used state transitions do not record a usage timestamp.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
43-48: 엔드포인트 목적에 맞는 메서드명으로 정리했으면 합니다Line 44에서
/confirm검증용 POST 엔드포인트가 기존 조회용 메서드와 동일한getUserBuyTicketInfo이름을 재사용하고 있어 가독성이 떨어집니다. 후속 유지보수를 위해confirmTicket등 역할을 드러내는 메서드명으로 분리하는 편이 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java(3 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java(6 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/core/domain/entity/TicketEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/permitseoul/permitserver/domain/ticket/core/domain/entity/TicketEntity.java
- src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java (7)
src/main/java/com/permitseoul/permitserver/domain/event/core/exception/EventNotfoundException.java (1)
EventNotfoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/ConflictTicketException.java (1)
ConflictTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/DateTicketException.java (1)
DateTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/IllegalTicketException.java (1)
IllegalTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/api/exception/NotFoundTicketException.java (1)
NotFoundTicketException(5-9)src/main/java/com/permitseoul/permitserver/domain/ticket/core/exception/TicketNotFoundException.java (1)
TicketNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticket/api/TicketExceptionHandler.java (1)
Order(12-19)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
🔇 Additional comments (1)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java (1)
112-125: 동시 요청 시 같은 티켓이 여러 번 사용 처리됩니다Line 124 기준,
findTicketEntityByTicketCode로 상태를 읽은 뒤 별도 동기화 없이ticketUpdater.updateTicketStatus를 호출하고 있어 두 트랜잭션이 동시에RESERVED상태를 보고 모두USED로 전이시키는 TOCTOU 경쟁 조건이 남아 있습니다. 이는 도어 검증 API의 핵심 목적인 중복 사용 방지를 무력화하므로, 조회 단계에서 비관적 락을 걸거나 업데이트를WHERE status = :expected조건과 함께 실행해 한 번만 성공하도록 보완해 주세요.
This was referenced Oct 16, 2025
This was referenced Dec 31, 2025
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.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항