feat: admin 행사 티켓 라운드+타입 수정 API - #143 #148
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
Walkthrough관리자 API의 일부 생성 응답 상태가 OK→CREATED로 변경되고, 이벤트 상세 응답에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as AdminClient
participant C as AdminTicketController
participant S as AdminTicketService
participant RR as TicketRoundRetriever
participant RU as AdminTicketRoundUpdater
participant TR as AdminTicketTypeRetriever
participant TS as AdminTicketTypeSaver
participant TU as AdminTicketTypeUpdater
Admin->>C: PATCH /admin/tickets/round-with-type (updateRequest)
C->>S: updateTicketRoundWithType(updateRequest)
S->>RR: getTicketRoundEntityById(ticketRoundId)
RR-->>S: TicketRoundEntity
S->>RU: updateTicketRound(entity, name, salesStart, salesEnd)
loop for each ticketType in request.ticketTypes
alt id == null
S->>TS: saveTicketType(roundId, name, price, total, start, end)
else id != null
S->>TR: getTicketTypeEntityById(typeId)
TR-->>S: TicketTypeEntity
alt ticketType.roundId != roundId
S-->>C: throw BAD_REQUEST_MISMATCH_TICKET_TYPE_ROUND
else
S->>TU: updateTicketType(entity, name, price, total, start, end)
end
end
end
S-->>C: 200 OK
C-->>Admin: 200 OK
sequenceDiagram
autonumber
participant Sch as ReservationSessionCleanupScheduler
participant Repo as ReservationSessionRepository
participant Rem as ReservationSessionRemover
participant Redis as RedisStore
Sch->>Repo: findAllByCreatedAtBefore(expireThreshold)
Repo-->>Sch: expiredSessions
opt rollback ticket counts
Sch->>Redis: increment remain counts per session
end
Sch->>Rem: deleteAllInBatch(expiredSessions)
Rem-->>Sch: done
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 (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (4)
🔇 Additional comments (4)
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 |
sjk4618
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionCleanupScheduler.java (1)
41-64: 성공한 예약의 티켓 롤백 위험쿼리 변경으로
findAllByCreatedAtBefore는 성공 여부와 관계없이 모든 만료된 세션을 반환합니다. 38-39번 라인에서 성공한 세션을 먼저 삭제하므로 일반적으로는 문제가 없지만, 다음과 같은 엣지 케이스에서 데이터 손상이 발생할 수 있습니다:
- 트랜잭션 커밋 전 타이밍 이슈
- 첫 번째 삭제가 부분적으로 실패한 경우
- 동시성 문제로 성공 세션이 중복 처리되는 경우
성공한 예약의 티켓이 Redis에서 잘못 롤백되면 티켓 수량이 부정확해집니다.
다음 diff를 적용하여 롤백 로직에 안전 장치를 추가하세요:
final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllByCreatedAtBefore(expireThreshold); final Map<Long, Integer> rollbackMap = new HashMap<>(); final List<String> expiredOrderIds = expiredOrFailedSessions.stream() + .filter(session -> !session.isSuccessful()) // 성공한 세션 제외 .map(ReservationSessionEntity::getOrderId) .toList();추가로, 명확성을 위해 41번 라인의 주석을 다음과 같이 수정하는 것을 권장합니다:
- // 7분 지난 세션 -> 롤백 및 삭제 + // 7분 지난 미성공 세션 -> 티켓 롤백 및 삭제 (성공 세션은 이미 삭제됨)
🧹 Nitpick comments (10)
src/main/java/com/permitseoul/permitserver/domain/admin/guest/api/controller/AdminGuestController.java (1)
26-38: 선택적 개선: 생성된 게스트 정보 반환 고려현재 구현은 정상적으로 작동하지만, REST 모범 사례에 따르면 201 CREATED 응답 시 생성된 리소스의 세부 정보를 응답 본문에 포함하는 것이 일반적입니다. 이를 통해 클라이언트가 추가 GET 요청 없이 생성된 리소스의 ID나 기타 정보를 바로 사용할 수 있습니다.
이 개선은 현재 PR의 범위를 벗어날 수 있으므로, 필요시 별도 이슈로 추적하는 것을 권장합니다.
예시 개선 방안:
@PostMapping() public ResponseEntity<BaseResponse<?>> addGuest( @RequestBody @Valid GuestAddRequest guestAddRequest ) { - adminGuestService.addGuest( + var createdGuest = adminGuestService.addGuest( guestAddRequest.guestName(), guestAddRequest.guestType(), guestAddRequest.affiliation(), guestAddRequest.phoneNumber(), guestAddRequest.email() ); - return ApiResponseUtil.success(SuccessCode.CREATED); + return ApiResponseUtil.success(SuccessCode.CREATED, createdGuest); }서비스 메서드도 생성된 게스트 정보를 반환하도록 수정 필요:
public GuestResponse addGuest(String guestName, GuestType guestType, String affiliation, String phoneNumber, String email) { // 게스트 생성 로직 Guest savedGuest = guestRepository.save(guest); return GuestResponse.from(savedGuest); }src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionCleanupScheduler.java (2)
27-27: 새 의존성의 가치를 재검토하세요.
ReservationSessionRemover는 단순히 repository의deleteAllInBatch를 위임하는 얇은 래퍼입니다. 이 컴포넌트가 추가적인 로직이나 추상화 없이 단순 위임만 수행한다면, 직접 repository를 사용하는 것이 더 명확할 수 있습니다.아키텍처적 일관성을 위한 것이라면, 최소한 같은 파일 내에서 일관되게 적용하세요 (line 64에서는 여전히 repository를 직접 사용).
39-39: 일관성 없는 삭제 방식.Line 39에서는
reservationSessionRemover를 사용하지만, line 64에서는reservationSessionRepository를 직접 사용합니다. 같은 파일 내에서 삭제 방식이 일관되지 않습니다.두 위치 모두 같은 방식을 사용하도록 통일하세요.
src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionRemover.java (2)
10-18: 불필요한 래퍼 컴포넌트를 재검토하세요.이 컴포넌트는
ReservationSessionRepository.deleteAllInBatch에 대한 단순 위임만 수행하며, 추가 로직, 검증, 에러 처리, 로깅 등이 전혀 없습니다.문제점:
- 아키텍처 레이어는 가치를 추가해야 하는데, 이 컴포넌트는 단순 위임만 합니다.
- 불필요한 간접 참조로 인해 코드 탐색이 어려워집니다.
ReservationSessionCleanupScheduler에서도 일관되게 사용되지 않습니다 (line 64는 여전히 repository 직접 사용).다음 중 하나를 선택하세요:
- 권장: 이 컴포넌트를 제거하고 스케줄러에서 repository를 직접 사용
- 배치 삭제 시 추가 로직(로깅, 검증, 메트릭)을 추가하여 컴포넌트의 존재 가치 부여
- 컴포넌트를 유지한다면 스케줄러의 line 64도 이 컴포넌트를 통하도록 수정하여 일관성 확보
10-17: 추상화 레이어의 필요성 검토
ReservationSessionRemover는 현재 리포지토리의deleteAllInBatch를 단순히 위임하는 thin wrapper입니다. 추가 로직이나 비즈니스 규칙이 없다면 이 추상화가 불필요한 보일러플레이트가 될 수 있습니다.다음을 고려하세요:
- 향후 삭제 전/후 로직 추가 계획이 있다면 현재 구조 유지
- 단순 위임만 필요하다면 스케줄러에서 리포지토리를 직접 사용하는 것이 더 간단할 수 있음
필요시 컴포넌트를 제거하고 스케줄러에서 직접 리포지토리 사용:
- private final ReservationSessionRemover reservationSessionRemover; + // ReservationSessionRemover 제거 - reservationSessionRemover.deleteAllInBatch(successSessions); + reservationSessionRepository.deleteAllInBatch(successSessions);src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeCoreException.java (1)
3-4: 생성자 추가를 고려해보세요.기본 예외 클래스가 적절하게 정의되어 있습니다. 다만, 메시지나 원인(cause)을 전달할 수 있는 생성자를 추가하면 하위 예외 클래스에서 더 유연하게 활용할 수 있습니다.
필요한 경우 다음과 같은 생성자를 추가할 수 있습니다:
public abstract class AdminTicketTypeCoreException extends RuntimeException { + protected AdminTicketTypeCoreException(String message) { + super(message); + } + + protected AdminTicketTypeCoreException(String message, Throwable cause) { + super(message, cause); + } }src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (1)
138-162: 티켓 타입 삭제 처리 고려현재 구현은 티켓 타입의 추가와 수정만 처리하며, 삭제는 처리하지 않습니다. 기존 티켓 타입이 요청에서 누락되면 데이터베이스에 남아있게 됩니다.
의도된 동작인지 확인이 필요합니다:
- 의도된 동작이라면: 문제없음
- 삭제를 지원해야 한다면: 요청에 없는 기존 티켓 타입을 식별하여 삭제하는 로직 추가 필요
추가로, 이미 판매된 티켓이 있는 경우 수정을 제한해야 하는지도 검토하시기 바랍니다.
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/dto/req/TicketRoundWithTypeUpdateRequest.java (2)
27-28: 빈 티켓 타입 리스트 검증 추가 권장
@NotNull은 null만 방지하며, 빈 리스트([])는 허용합니다. 티켓 라운드에 최소 1개의 티켓 타입이 필요하다면@NotEmpty또는@Size(min=1)어노테이션을 추가하는 것이 좋습니다.다음 diff를 적용하세요:
+import jakarta.validation.constraints.NotEmpty; public record TicketRoundWithTypeUpdateRequest( @NotNull(message = "티켓라운드 ID는 필수입니다.") Long ticketRoundId, @NotBlank(message = "티켓라운드 이름은 필수입니다.") String ticketRoundName, @NotNull(message = "티켓라운드 판매 시작일은 필수입니다.") @JsonFormat(pattern = "yyyy.MM.dd HH:mm") LocalDateTime ticketRoundSalesStartDate, @NotNull(message = "티켓라운드 판매 종료일은 필수입니다.") @JsonFormat(pattern = "yyyy.MM.dd HH:mm") LocalDateTime ticketRoundSalesEndDate, @NotNull(message = "티켓타입 리스트는 필수입니다.") + @NotEmpty(message = "최소 1개의 티켓타입이 필요합니다.") List<TicketTypeUpdateRequest> ticketTypes ) {
12-54: 날짜 순서 검증 고려현재 구현은 개별 필드만 검증하며, 다음과 같은 날짜 순서는 검증하지 않습니다:
ticketRoundSalesStartDate < ticketRoundSalesEndDatestartDate < endDate(각 티켓 타입)이러한 검증은 서비스 계층에서 수행되거나, 커스텀 클래스 레벨 validator를 추가하여 처리할 수 있습니다. 현재 구조가 의도된 것이라면 문제없지만, DTO 레벨에서 검증하면 더 명확한 에러 메시지를 제공할 수 있습니다.
예시 (클래스 레벨 validator 추가):
import jakarta.validation.Constraint; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; import jakarta.validation.Payload; import java.lang.annotation.*; @Documented @Constraint(validatedBy = ValidDateRangeValidator.class) @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @interface ValidDateRange { String message() default "시작일은 종료일보다 이전이어야 합니다."; Class<?>[] groups() default {}; Class<? extends Payload>[] payload() default {}; } class ValidDateRangeValidator implements ConstraintValidator<ValidDateRange, TicketRoundWithTypeUpdateRequest> { @Override public boolean isValid(TicketRoundWithTypeUpdateRequest value, ConstraintValidatorContext context) { if (value == null) return true; return value.ticketRoundSalesStartDate().isBefore(value.ticketRoundSalesEndDate()); } }그 후 record에 적용:
+@ValidDateRange public record TicketRoundWithTypeUpdateRequest(src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/repository/ReservationSessionRepository.java (1)
32-32: 쿼리 명확성 개선 권장쿼리 메서드명이
findAllBySuccessfulFalseAndCreatedAtBefore에서findAllByCreatedAtBefore로 변경되어successful=false필터가 제거되었습니다. 이는 스케줄러에서 성공 세션을 먼저 삭제하는 전제에 의존하게 되어 코드 의도가 불명확해집니다.다음 중 하나를 고려하세요:
- 명시적 필터 유지 (권장): 기존 쿼리 메서드를 유지하여 의도를 명확히 하고 실수를 방지
- 문서화 강화: 메서드에 JavaDoc을 추가하여 이 쿼리가 성공 세션 삭제 이후에만 호출되어야 함을 명시
옵션 1: 기존 쿼리 메서드 복원
+ List<ReservationSessionEntity> findAllBySuccessfulFalseAndCreatedAtBefore(final LocalDateTime time); + List<ReservationSessionEntity> findAllByCreatedAtBefore(final LocalDateTime time);스케줄러에서는
findAllBySuccessfulFalseAndCreatedAtBefore를 사용하도록 변경.옵션 2: JavaDoc 추가
+ /** + * 주어진 시간 이전에 생성된 모든 세션을 조회합니다. + * 주의: 이 메서드는 successful=true인 세션이 이미 삭제된 후에 호출되어야 합니다. + * 그렇지 않으면 성공한 예약의 티켓이 잘못 롤백될 수 있습니다. + */ List<ReservationSessionEntity> findAllByCreatedAtBefore(final LocalDateTime time);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/main/java/com/permitseoul/permitserver/domain/admin/event/api/controller/AdminEventController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/event/api/dto/res/AdminEventDetailResponse.java(4 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/guest/api/controller/AdminGuestController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/guestticket/api/controller/AdminGuestTicketController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/controller/AdminTicketController.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/dto/req/TicketRoundWithTypeUpdateRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java(3 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundSaver.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeCoreException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionCleanupScheduler.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionRemover.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/repository/ReservationSessionRepository.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservationticket/core/component/ReservationTicketRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticketround/core/domain/entity/TicketRoundEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/tickettype/core/domain/entity/TicketTypeEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/java/com/permitseoul/permitserver/domain/admin/event/api/controller/AdminEventController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRetriever.java (1)
Component(12-35)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundSaver.java (1)
Component(11-24)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.java (1)
Component(9-20)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java (1)
AdminTicketTypeNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticketround/core/exception/TicketRoundIllegalArgumentException.java (1)
TicketRoundIllegalArgumentException(3-4)src/main/java/com/permitseoul/permitserver/domain/ticketround/core/exception/TicketRoundNotFoundException.java (1)
TicketRoundNotFoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/controller/AdminTicketController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/guest/api/controller/AdminGuestController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/guestticket/api/controller/AdminGuestTicketController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
Component(13-36)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java (1)
AdminTicketTypeNotFoundException(3-4)
🔇 Additional comments (18)
src/main/java/com/permitseoul/permitserver/domain/reservationticket/core/component/ReservationTicketRetriever.java (1)
33-33: 파라미터 일관성 개선 확인
final수정자를 추가하여 클래스 내 메서드 간 파라미터 선언 스타일이 일관되었습니다 (라인 22의findAllByOrderId와 동일한 패턴). 파라미터의 불변성을 보장하는 좋은 코드 품질 개선입니다.src/main/java/com/permitseoul/permitserver/domain/admin/guest/api/controller/AdminGuestController.java (1)
37-37: REST 의미론에 부합 — 201 CREATED 반환 적절POST 엔드포인트에서 리소스 생성 시 HTTP 201 CREATED를 반환하는 것은 REST 모범 사례입니다.
다만, 기존 클라이언트 코드가 명시적으로 200 OK를 확인할 수 있으므로 프론트엔드나 통합 클라이언트에서 해당 엔드포인트의 상태 코드 의존성을 직접 검토해 주세요.src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundSaver.java (1)
1-1: 패키지 구조 개선이 적절합니다.컴포넌트 파일을
.component패키지로 이동하여 코드 구조를 개선한 변경사항입니다. 이는 PR의 다른 컴포넌트 파일들(AdminTicketRoundUpdater, AdminTicketTypeUpdater 등)과 일관된 패턴을 따릅니다.src/main/java/com/permitseoul/permitserver/domain/admin/guestticket/api/controller/AdminGuestTicketController.java (1)
29-29: REST 의미론에 맞는 상태 코드 개선입니다.게스트 티켓 생성 엔드포인트가 이제 201 CREATED를 반환하여 REST 모범 사례를 따릅니다. 이는 PR의 다른 생성 엔드포인트 변경사항(예: AdminEventController)과 일관됩니다.
src/main/java/com/permitseoul/permitserver/domain/admin/event/api/controller/AdminEventController.java (1)
41-41: REST 의미론에 맞는 상태 코드 개선입니다.이벤트 생성 엔드포인트가 이제 201 CREATED를 반환하여 HTTP 표준에 부합합니다. 이는 PR 전반의 일관된 개선 사항입니다.
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1)
30-30: 적절한 에러 코드 추가입니다.티켓 타입과 라운드 간의 불일치를 검증하기 위한 에러 코드가 추가되었습니다. 에러 코드 번호(40016)와 메시지가 적절하며, 티켓 라운드/타입 수정 API의 검증 로직에 활용될 것으로 보입니다.
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRetriever.java (1)
1-1: 패키지 리팩토링 확인 완료컴포넌트들을
core.component패키지로 일관되게 정리한 것이 좋습니다. 기능적 변경 없이 구조 개선만 이루어졌습니다.src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java (1)
1-4: 예외 클래스 구현 확인 완료티켓 타입 조회 실패 시 사용할 전용 예외 클래스가 적절히 정의되었습니다.
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
32-35: 엔티티 직접 반환 확인이 메서드는 기존 메서드들과 달리
TicketTypeEntity를 직접 반환합니다. 업데이트 플로우에서 엔티티를 직접 수정하기 위한 의도로 보이며, 트랜잭션 읽기 전용 설정도 적절합니다.src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java (1)
21-28: 티켓 타입 저장 메서드 추가 확인개별 파라미터로 티켓 타입을 생성하고 저장하는 메서드가 적절히 구현되었습니다. 업데이트 플로우에서 새로운 티켓 타입 생성 시 유용하게 사용될 것으로 보입니다.
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/controller/AdminTicketController.java (2)
49-50: HTTP 상태 코드 개선생성(POST) 엔드포인트의 응답 코드를
CREATED로 변경한 것은 RESTful API 관례에 부합하는 좋은 개선입니다.
52-59: 티켓 라운드+타입 수정 엔드포인트 추가새로운 PATCH 엔드포인트가 적절히 구현되었습니다.
@Valid어노테이션으로 입력 검증도 적용되어 있습니다.src/main/java/com/permitseoul/permitserver/domain/admin/event/api/dto/res/AdminEventDetailResponse.java (1)
3-3: 이벤트 타입 필드 추가응답 DTO에
eventType필드가 적절히 추가되었고, 팩토리 메서드와 생성자가 일관되게 업데이트되었습니다.Also applies to: 15-15, 34-34, 52-52
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (1)
1-17: 티켓 라운드 업데이터 컴포넌트 추가엔티티의
update()메서드에 위임하는 간결한 구현입니다. JPA의 변경 감지(dirty checking)를 활용하여 업데이트가 처리되며, 서비스 레이어에서 트랜잭션을 관리하는 기존 패턴과 일관됩니다.src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java (2)
15-16: Import 경로 업데이트컴포넌트 패키지 재구성에 따라 import 경로가 올바르게 업데이트되었습니다.
97-97: 이벤트 타입 정보 전달응답 DTO에 이벤트 타입 정보가 적절히 전달되고 있습니다.
src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionCleanupScheduler.java (2)
27-27: LGTM!세션 삭제 로직을 별도 컴포넌트로 분리하여 관심사를 명확히 했습니다.
1-66: 티켓 라운드+타입 수정 API 파일이 PR에 포함되어 있습니다
AdminTicketTypeUpdater.java,AdminTicketRoundUpdater.java,TicketRoundWithTypeUpdateRequest.java등 관련 컴포넌트가 이미 반영되어 있으니, 예약 세션 정리 로직 리팩토링과 별개로 해당 API 구현 검토를 진행해주세요.Likely an incorrect or invalid review comment.
Comment on lines
127
to
170
| @Transactional | ||
| public void updateTicketRoundWithType(final TicketRoundWithTypeUpdateRequest updateRequest) { | ||
| try { | ||
| final TicketRoundEntity ticketRoundEntity = ticketRoundRetriever.findTicketRoundEntityById(updateRequest.ticketRoundId()); | ||
| adminTicketRoundUpdater.updateTicketRound( | ||
| ticketRoundEntity, | ||
| updateRequest.ticketRoundName(), | ||
| updateRequest.ticketRoundSalesStartDate(), | ||
| updateRequest.ticketRoundSalesEndDate() | ||
| ); | ||
|
|
||
| for(TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest ticketTypeUpdateRequest: updateRequest.ticketTypes()) { | ||
| if(ticketTypeUpdateRequest.id() == null) { | ||
| adminTicketTypeSaver.saveTicketType( | ||
| ticketRoundEntity.getTicketRoundId(), | ||
| ticketTypeUpdateRequest.name(), | ||
| ticketTypeUpdateRequest.price(), | ||
| ticketTypeUpdateRequest.totalCount(), | ||
| ticketTypeUpdateRequest.startDate(), | ||
| ticketTypeUpdateRequest.endDate() | ||
| ); | ||
| } else { | ||
| final TicketTypeEntity ticketTypeEntity = adminTicketTypeRetriever.getTicketTypeEntityById(ticketTypeUpdateRequest.id()); | ||
| if(ticketTypeEntity.getTicketRoundId() != ticketRoundEntity.getTicketRoundId()) { | ||
| throw new AdminApiException(ErrorCode.BAD_REQUEST_MISMATCH_TICKET_TYPE_ROUND); | ||
| } | ||
| adminTicketTypeUpdater.updateTicketType( | ||
| ticketTypeEntity, | ||
| ticketTypeUpdateRequest.name(), | ||
| ticketTypeUpdateRequest.price(), | ||
| ticketTypeUpdateRequest.totalCount(), | ||
| ticketTypeUpdateRequest.startDate(), | ||
| ticketTypeUpdateRequest.endDate() | ||
| ); | ||
| } | ||
| } | ||
| } catch (TicketRoundNotFoundException e) { | ||
| throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_ROUND); | ||
| } catch (TicketRoundIllegalArgumentException | TicketTypeIllegalException e) { | ||
| throw new AdminApiException(ErrorCode.BAD_REQUEST_DATE_TIME_ERROR); | ||
| } catch (AdminTicketTypeNotFoundException e) { | ||
| throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_TYPE); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
날짜 범위 검증 추가 권장
다음과 같은 비즈니스 규칙 검증이 누락되어 있습니다:
- 티켓 라운드 판매 시작일 < 종료일
- 티켓 타입 시작일 < 종료일
- 티켓 타입 기간이 티켓 라운드 판매 기간 내에 포함되는지
adminTicketRoundUpdater 또는 adminTicketTypeUpdater에서 이러한 검증을 수행하는지 확인하세요. 만약 검증이 없다면, 잘못된 데이터가 저장될 수 있습니다.
다음 스크립트를 실행하여 Updater 컴포넌트에서 날짜 검증을 수행하는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Updater 컴포넌트에서 날짜 범위 검증 로직 확인
# AdminTicketRoundUpdater의 updateTicketRound 메서드 검색
ast-grep --pattern $'class AdminTicketRoundUpdater {
$$$
updateTicketRound($$$) {
$$$
}
$$$
}'
# AdminTicketTypeUpdater의 updateTicketType 메서드 검색
ast-grep --pattern $'class AdminTicketTypeUpdater {
$$$
updateTicketType($$$) {
$$$
}
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# AdminTicketRoundUpdater 클래스와 updateTicketRound 메서드 위치 확인
rg -nP "class\s+AdminTicketRoundUpdater" -C3
rg -nP "updateTicketRound\s*\(" -C3
# AdminTicketTypeUpdater 클래스와 updateTicketType 메서드 위치 확인
rg -nP "class\s+AdminTicketTypeUpdater" -C3
rg -nP "updateTicketType\s*\(" -C3Length of output: 6617
🏁 Script executed:
#!/bin/bash
# AdminTicketRoundUpdater.updateTicketRound 전체 내용 확인
sed -n '1,200p' src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java
# AdminTicketTypeUpdater.updateTicketType 전체 내용 확인
sed -n '1,200p' src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.javaLength of output: 1494
🏁 Script executed:
#!/bin/bash
# TicketRoundEntity 파일 경로 찾기
ENTITY_ROUND_FILE=$(rg -l "class\s\+TicketRoundEntity" -g '*.java')
echo "== $ENTITY_ROUND_FILE =="
# TicketRoundEntity.update 메서드 내용 출력
sed -n '1,200p' "$ENTITY_ROUND_FILE"
# TicketTypeEntity 파일 경로 찾기
ENTITY_TYPE_FILE=$(rg -l "class\s\+TicketTypeEntity" -g '*.java')
echo "== $ENTITY_TYPE_FILE =="
# TicketTypeEntity.update 메서드 내용 출력
sed -n '1,200p' "$ENTITY_TYPE_FILE"Length of output: 101
TicketRoundEntity와 TicketTypeEntity의 update(...) 구현을 확인하기 위해 다음 스크립트를 실행하세요:
🏁 Script executed:
#!/bin/bash
# TicketRoundEntity 파일 경로 찾기 및 내용 확인
ENTITY_ROUND_FILE=$(find src -type f -name 'TicketRoundEntity.java' | head -n1)
echo "== $ENTITY_ROUND_FILE =="
sed -n '1,200p' "$ENTITY_ROUND_FILE"
# TicketTypeEntity 파일 경로 찾기 및 내용 확인
ENTITY_TYPE_FILE=$(find src -type f -name 'TicketTypeEntity.java' | head -n1)
echo "== $ENTITY_TYPE_FILE =="
sed -n '1,200p' "$ENTITY_TYPE_FILE"Length of output: 7308
티켓 타입 기간이 라운드 판매 기간 내 포함 여부 검증 추가
updateTicketRoundWithType에서 티켓 타입의 시작일이 ticketRoundEntity.getSalesStartAt()보다 빠르거나, 종료일이 ticketRoundEntity.getSalesEndAt()보다 늦은 경우 예외를 발생시키는 로직을 추가하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java
around lines 127 to 170, add validation inside the loop over
updateRequest.ticketTypes() that ensures each ticketType's startDate is not
before ticketRoundEntity.getSalesStartAt() and each ticketType's endDate is not
after ticketRoundEntity.getSalesEndAt(); if a ticket type violates these bounds,
throw TicketTypeIllegalException so the existing catch maps it to
BAD_REQUEST_DATE_TIME_ERROR. Insert the check before calling
adminTicketTypeSaver.saveTicketType(...) or
adminTicketTypeUpdater.updateTicketType(...), comparing
ticketTypeUpdateRequest.startDate()/endDate() against
ticketRoundEntity.getSalesStartAt()/getSalesEndAt() and throwing the exception
on failure.
...in/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java
Outdated
Show resolved
Hide resolved
Comment on lines
+9
to
+19
| @Component | ||
| public class AdminTicketTypeUpdater { | ||
|
|
||
| public void updateTicketType(final TicketTypeEntity ticketTypeEntity, | ||
| final String name, | ||
| final BigDecimal price, | ||
| final int totalCount, | ||
| final LocalDateTime startAt, | ||
| final LocalDateTime endAt) { | ||
| ticketTypeEntity.update(name, price, totalCount, startAt, endAt); | ||
| } |
There was a problem hiding this comment.
트랜잭션 어노테이션이 누락되었습니다.
엔티티 업데이트 작업을 수행하는 컴포넌트에 @Transactional 어노테이션이 없습니다. JPA의 dirty checking이 제대로 동작하려면 트랜잭션 경계 내에서 실행되어야 합니다.
다음과 같이 수정하세요:
+import org.springframework.transaction.annotation.Transactional;
+
@Component
public class AdminTicketTypeUpdater {
+ @Transactional
public void updateTicketType(final TicketTypeEntity ticketTypeEntity,
final String name,
final BigDecimal price,
final int totalCount,
final LocalDateTime startAt,
final LocalDateTime endAt) {
ticketTypeEntity.update(name, price, totalCount, startAt, endAt);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Component | |
| public class AdminTicketTypeUpdater { | |
| public void updateTicketType(final TicketTypeEntity ticketTypeEntity, | |
| final String name, | |
| final BigDecimal price, | |
| final int totalCount, | |
| final LocalDateTime startAt, | |
| final LocalDateTime endAt) { | |
| ticketTypeEntity.update(name, price, totalCount, startAt, endAt); | |
| } | |
| import org.springframework.transaction.annotation.Transactional; | |
| @Component | |
| public class AdminTicketTypeUpdater { | |
| @Transactional | |
| public void updateTicketType(final TicketTypeEntity ticketTypeEntity, | |
| final String name, | |
| final BigDecimal price, | |
| final int totalCount, | |
| final LocalDateTime startAt, | |
| final LocalDateTime endAt) { | |
| ticketTypeEntity.update(name, price, totalCount, startAt, endAt); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.java
around lines 9 to 19, the component performing entity updates lacks a
transactional boundary so JPA dirty checking may not flush changes; annotate the
class or the updateTicketType method with @Transactional
(org.springframework.transaction.annotation.Transactional) so the update runs
inside a transaction, and ensure the Spring transaction management is enabled in
the application configuration.
| // 실패인 세션들 + 7분 지난 세션 -> 롤백 및 삭제 | ||
| final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllBySuccessfulFalseAndCreatedAtBefore(expireThreshold); | ||
| // 7분 지난 세션 -> 롤백 및 삭제 | ||
| final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllByCreatedAtBefore(expireThreshold); |
There was a problem hiding this comment.
심각한 로직 버그: 성공한 세션이 롤백될 수 있습니다.
쿼리가 findAllBySuccessfulFalseAndCreatedAtBefore에서 findAllByCreatedAtBefore로 변경되면서 successful=false 필터가 제거되었습니다.
문제점:
- 이전 로직: 만료된 실패한 세션만 조회 → 티켓 롤백 → 삭제
- 현재 로직: 만료된 모든 세션 조회 (성공/실패 무관) → 티켓 롤백 → 삭제
영향:
- 성공한 예약의 오래된 세션이 line 39에서 삭제되더라도, 트랜잭션 내에서 line 42의 쿼리가 실행되는 순서에 따라 여전히 조회될 수 있습니다.
- 성공한 세션의 티켓이 롤백되면, 이미 확정된 예약의 티켓 카운트가 잘못 증가하여 데이터 정합성이 깨집니다.
- Line 41의 주석도 부정확합니다 ("7분 지난 세션"이 아니라 "7분 지난 실패/미완료 세션"이어야 함).
다음 diff를 적용하여 실패한 세션만 롤백되도록 수정하세요:
- // 7분 지난 세션 -> 롤백 및 삭제
- final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllByCreatedAtBefore(expireThreshold);
+ // 만료된 실패/미완료 세션 -> 롤백 및 삭제
+ final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllBySuccessfulFalseAndCreatedAtBefore(expireThreshold);그리고 repository에서도 메서드 이름을 다시 되돌려야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllByCreatedAtBefore(expireThreshold); | |
| // 만료된 실패/미완료 세션 -> 롤백 및 삭제 | |
| final List<ReservationSessionEntity> expiredOrFailedSessions = reservationSessionRepository.findAllBySuccessfulFalseAndCreatedAtBefore(expireThreshold); |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/reservationsession/core/component/ReservationSessionCleanupScheduler.java
around line 42, the query currently fetches all sessions by createdAt and
thereby risks rolling back tickets for successful sessions; change the call back
to the failing-only variant (e.g.,
reservationSessionRepository.findAllBySuccessfulFalseAndCreatedAtBefore(expireThreshold))
so only unsuccessful/unfinished sessions are selected, update the repository
method name accordingly to match that signature, and correct the comment on line
41 to state "7 minutes past failed/unfinished sessions" (or similar) to reflect
the actual behavior.
...oul/permitserver/domain/reservationsession/core/repository/ReservationSessionRepository.java
Outdated
Show resolved
Hide resolved
Comment on lines
49
to
56
| public void update(final String ticketRoundName, | ||
| final LocalDateTime ticketRoundSalesStartDate, | ||
| final LocalDateTime ticketRoundSalesEndDate) { | ||
| validateDateTIme(ticketRoundSalesStartDate, ticketRoundSalesEndDate); | ||
| this.ticketRoundTitle = ticketRoundName; | ||
| this.salesStartAt = ticketRoundSalesStartDate; | ||
| this.salesEndAt = ticketRoundSalesEndDate; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
파라미터 네이밍 일관성을 개선하세요.
업데이트 메서드가 적절하게 구현되어 있습니다. 다만, 파라미터명(ticketRoundName)과 필드명(ticketRoundTitle)이 일치하지 않아 혼동을 줄 수 있습니다.
일관성을 위해 다음과 같이 수정하는 것을 권장합니다:
-public void update(final String ticketRoundName,
+public void update(final String ticketRoundTitle,
final LocalDateTime ticketRoundSalesStartDate,
final LocalDateTime ticketRoundSalesEndDate) {
validateDateTIme(ticketRoundSalesStartDate, ticketRoundSalesEndDate);
- this.ticketRoundTitle = ticketRoundName;
+ this.ticketRoundTitle = ticketRoundTitle;
this.salesStartAt = ticketRoundSalesStartDate;
this.salesEndAt = ticketRoundSalesEndDate;
}또한, 이 메서드를 호출하는 AdminTicketRoundUpdater 컴포넌트에 @Transactional 어노테이션이 있는지 확인하세요. JPA dirty checking이 제대로 동작하려면 트랜잭션 경계가 필요합니다.
다음 스크립트로 호출 컨텍스트를 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# AdminTicketRoundUpdater 컴포넌트의 트랜잭션 설정 확인
fd -e java AdminTicketRoundUpdater | xargs rg -A 10 -B 2 "class AdminTicketRoundUpdater"Length of output: 465
파라미터 네이밍 일관화 및 트랜잭션 경계 추가
TicketRoundEntity#update의 파라미터명을ticketRoundTitle로 변경해 필드명과 일치시키세요.AdminTicketRoundUpdater#updateTicketRound에@Transactional을 추가해 JPA 더티 체킹이 정상 작동하도록 하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/ticketround/core/domain/entity/TicketRoundEntity.java
around lines 49 to 56, rename the method parameter ticketRoundName to
ticketRoundTitle so the parameter name matches the entity field and avoids
confusion; update all call sites accordingly. Also add @Transactional to the
AdminTicketRoundUpdater#updateTicketRound method (in its defining class) so JPA
dirty-checking will be applied and entity changes are persisted without an
explicit save.
Comment on lines
+82
to
+89
| public void adjustTicketTypeTotalAndRemainCount(final int requestTotalCount) { | ||
| final int differenceCount = requestTotalCount - this.totalTicketCount; | ||
| this.remainTicketCount += differenceCount; | ||
| this.totalTicketCount = requestTotalCount; | ||
| if (this.remainTicketCount < 0) { | ||
| this.remainTicketCount = 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
티켓 수량 조정 로직에 잠재적 데이터 무결성 문제가 있습니다.
총 티켓 수를 줄일 때, 이미 판매된 티켓 수보다 적은 값으로 설정하면 remainTicketCount가 음수가 된 후 0으로 클램핑됩니다. 이는 오버셀링(overselling) 상태를 감지하지 못하고 조용히 무시합니다.
예시:
- 초기 상태: totalTicketCount=100, remainTicketCount=30 (70장 판매됨)
- 업데이트: requestTotalCount=50으로 변경
- 결과: remainTicketCount가 -20이 된 후 0으로 클램핑되어, 실제로는 20장이 오버셀링된 상태임을 알 수 없게 됩니다.
다음과 같이 검증 로직을 추가하세요:
public void adjustTicketTypeTotalAndRemainCount(final int requestTotalCount) {
final int differenceCount = requestTotalCount - this.totalTicketCount;
+ final int newRemainCount = this.remainTicketCount + differenceCount;
+
+ // 판매된 티켓 수보다 적은 총 수량으로 변경할 수 없음
+ if (newRemainCount < 0) {
+ throw new TicketTypeInsufficientCountException();
+ }
+
- this.remainTicketCount += differenceCount;
+ this.remainTicketCount = newRemainCount;
this.totalTicketCount = requestTotalCount;
- if (this.remainTicketCount < 0) {
- this.remainTicketCount = 0;
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public void adjustTicketTypeTotalAndRemainCount(final int requestTotalCount) { | |
| final int differenceCount = requestTotalCount - this.totalTicketCount; | |
| this.remainTicketCount += differenceCount; | |
| this.totalTicketCount = requestTotalCount; | |
| if (this.remainTicketCount < 0) { | |
| this.remainTicketCount = 0; | |
| } | |
| } | |
| public void adjustTicketTypeTotalAndRemainCount(final int requestTotalCount) { | |
| final int differenceCount = requestTotalCount - this.totalTicketCount; | |
| final int newRemainCount = this.remainTicketCount + differenceCount; | |
| // 판매된 티켓 수보다 적은 총 수량으로 변경할 수 없음 | |
| if (newRemainCount < 0) { | |
| throw new TicketTypeInsufficientCountException(); | |
| } | |
| this.remainTicketCount = newRemainCount; | |
| this.totalTicketCount = requestTotalCount; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/tickettype/core/domain/entity/TicketTypeEntity.java
around lines 82 to 89, the adjustTicketTypeTotalAndRemainCount method currently
clamps negative remainTicketCount to zero which hides overselling; compute
soldCount = this.totalTicketCount - this.remainTicketCount and if
requestTotalCount < soldCount throw a validation exception (e.g.,
IllegalArgumentException or a domain-specific exception) to prevent reducing
total below already sold tickets, otherwise set this.totalTicketCount =
requestTotalCount and this.remainTicketCount = requestTotalCount - soldCount
(which will be >= 0).
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
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항