[FEAT] 예약 상태 변경에 따른 pdf 업로드/삭제 + 예약 견적서 수정시 pdf 삭제 후 재업로드 구현#33
Conversation
# Conflicts: # build.gradle # src/main/java/konkuk/chacall/domain/reservation/application/status/ReservationStatusService.java
Walkthrough예약 조회 필터를 상태 단일값에서 viewType(상태 집합)으로 전환하고, 예약 확정 시 Thymeleaf → OpenHTMLToPDF로 PDF를 생성해 S3에 업로드하고 CDN URL을 저장하도록 추가했습니다. 응답 DTO에 예약 상태 문자열을 추가하고, 취소된 예약의 pdfUrl은 null로 반환됩니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Controller as Controller
participant Service as ReservationService
participant View as ReservationViewType
participant Repo as ReservationRepository
Client->>Controller: GET /reservations?viewType=...
Controller->>Service: getReservations(viewType, cursor, size)
Service->>View: viewType.getStatuses()
View-->>Service: Set<ReservationStatus>
Service->>Repo: find...ByStatusWithCursor(userId, statuses, cursor, size)
Repo-->>Service: Slice<Reservation>
Service-->>Controller: CursorPagingResponse(..., reservationStatus, ...)
Controller-->>Client: 200 OK (reservationStatus, pdfUrl=null if CANCELLED)
sequenceDiagram
autonumber
participant StatusSvc as ReservationStatusService
participant PdfSvc as PdfService
participant Tpl as PdfTemplateRenderer
participant Gen as PdfGenerator
participant S3 as S3Service
participant CDN as CdnUrlResolver
StatusSvc->>StatusSvc: updateReservationStatusToConfirmed(reservation)
StatusSvc->>PdfSvc: renderAndUpload(reservation)
PdfSvc->>Tpl: render(reservation)
Tpl-->>PdfSvc: HTML
PdfSvc->>Gen: generate(HTML)
Gen-->>PdfSvc: PDF bytes
PdfSvc->>S3: uploadPdf(key, "application/pdf", bytes)
S3-->>PdfSvc: OK
PdfSvc->>CDN: resolve(key)
CDN-->>PdfSvc: https://cdn/.../key
PdfSvc-->>StatusSvc: pdfUrl
StatusSvc->>StatusSvc: reservation.setPdfUrl(pdfUrl)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
buzz0331
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/konkuk/chacall/domain/reservation/application/status/ReservationStatusService.java (1)
48-56: 취소 시 PDF S3 삭제 및 reservation.pdfUrl 널링 필요
- 검증 결과: 응답 레이어는 취소 상태에서 pdfUrl을 null로 마스킹함(OwnerReservationDetailResponse, MemberReservationDetailResponse — src/.../OwnerReservationDetailResponse.java, src/.../MemberReservationDetailResponse.java).
그러나 서비스 계층의 updateReservationStatusToCancelled 메서드(src/.../ReservationStatusService.java)에서는 S3 삭제/엔티티 널링이 없음.- 관련 구현: PdfService.deleteIfExists(...)가 존재함(src/.../global/common/storage/PdfService.java) 및 S3 삭제는 S3Service.deleteObject(...)로 구현되어 있음(src/.../global/common/storage/S3Service.java).
- 조치(필수): PR 목표(업로드/삭제)를 충족하려면 updateReservationStatusToCancelled에서 pdfService.deleteIfExists(reservation.getPdfUrl()) 호출 후 reservation.setPdfUrl(null) 처리(영속성 반영 확인) 또는 설계상 파일을 보존하려면 그 사유를 PR/커밋에 명시.
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationStatus.java (1)
33-45: COMPLETED 전이 경로 누락으로 상태 전환 불가능COMPLETED가 정의되었지만 TRANSITIONS에 포함되지 않아 어떤 상태에서도 COMPLETED로 전환할 수 없습니다. 의도대로라면 CONFIRMED → COMPLETED 전이를 추가하고 COMPLETED를 종단 상태로 등록하세요.
수정 제안(diff):
- // PENDING -> CONFIRMED_REQUESTED -> CONFIRMED -> CANCELLED_REQUESTED -> CANCELLED + // PENDING -> CONFIRMED_REQUESTED -> CONFIRMED -> (CANCELLED_REQUESTED -> CANCELLED | COMPLETED) private static final Map<ReservationStatus, Set<ReservationStatus>> TRANSITIONS; static { Map<ReservationStatus, Set<ReservationStatus>> m = new EnumMap<>(ReservationStatus.class); m.put(PENDING, EnumSet.of(CONFIRMED_REQUESTED)); m.put(CONFIRMED_REQUESTED, EnumSet.of(CONFIRMED)); - m.put(CONFIRMED, EnumSet.of(CANCELLED_REQUESTED)); + m.put(CONFIRMED, EnumSet.of(CANCELLED_REQUESTED, COMPLETED)); m.put(CANCELLED_REQUESTED, EnumSet.of(CANCELLED)); m.put(CANCELLED, EnumSet.noneOf(ReservationStatus.class)); + m.put(COMPLETED, EnumSet.noneOf(ReservationStatus.class)); TRANSITIONS = Map.copyOf(m); }
🧹 Nitpick comments (14)
build.gradle (1)
67-75: Spring Cloud AWS starter 추가 + 수동 S3 빈 구성은 중복입니다. 하나로 정리하세요.application-*.yml에서 S3AutoConfiguration을 exclude했고, AwsConfig에서 S3Client/S3Presigner를 직접 빈으로 등록합니다. 이 경우 spring-cloud-aws-starter-s3 의 자동구성이 비활성화되어 사실상 스타터를 쓰지 않습니다. 유지관리 단순화를 위해 아래 둘 중 하나로 정리하는 것을 권장합니다:
- 선택 A: 수동 빈 유지 → 스타터 제거
- 선택 B: 스타터 자동구성 사용 → AwsConfig 제거 + yml exclude 제거
불필요한 의존성은 제거하는 편이 성능/보안/공급망 관점에서 유리합니다.
다음을 적용하면 선택 A에 해당합니다:
- // S3 - implementation 'io.awspring.cloud:spring-cloud-aws-starter-s3:3.4.0'src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java (1)
11-21: 초 단위 타임스탬프는 동일 초 내 재시도 시 키 충돌/캐시 히트 위험. 밀리초 및 UTC 권장.
- 밀리초(SSS)까지 포함해 키 충돌 가능성을 낮추세요.
- 서버 로케일 의존 제거를 위해 UTC 고정 권장.
예시 수정:
+import java.time.ZoneId; +import java.time.ZonedDateTime; @@ - private final static DateTimeFormatter DATE_TIME_FORMATTER = - DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"); + private final static DateTimeFormatter DATE_TIME_FORMATTER = + DateTimeFormatter.ofPattern("yyyyMMdd-HHmmssSSS"); @@ - String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 + String timestamp = ZonedDateTime.now(ZoneId.of("UTC")).format(DATE_TIME_FORMATTER); // ex) 20250923-163045123대체안: UUID suffix 추가 또는 업로드 idempotency 보장 로직(중복 이벤트 시 동일 키 사용)도 고려 가능합니다.
src/main/java/konkuk/chacall/global/config/AwsConfig.java (1)
25-45: 자격증명 하드코딩(StaticCredentialsProvider)만 사용 → 운영/테스트 환경 호환성 저하. 기본 자격증명 체인 폴백을 추가하세요.
- access/secret 키 미설정 시 DefaultCredentialsProvider로 폴백하면 EC2/ECS/IRSA, 환경변수, 프로파일 등 표준 체인을 활용할 수 있습니다.
- 동일 로직을 S3Client/S3Presigner 모두에 적용.
아래처럼 개선 권장:
+import org.springframework.util.StringUtils; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; @@ public S3Client s3Client() { - return S3Client.builder() - .region(Region.of(region)) - .credentialsProvider( - StaticCredentialsProvider.create( - AwsBasicCredentials.create(accessKey, secretKey) - ) - ) - .build(); + AwsCredentialsProvider provider = + (StringUtils.hasText(accessKey) && StringUtils.hasText(secretKey)) + ? StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey, secretKey)) + : DefaultCredentialsProvider.create(); + return S3Client.builder() + .region(Region.of(region)) + .credentialsProvider(provider) + .build(); } @@ public S3Presigner s3Presigner() { - return S3Presigner.builder() - .region(Region.of(region)) - .credentialsProvider( - StaticCredentialsProvider.create( - AwsBasicCredentials.create(accessKey, secretKey))) - .build(); + AwsCredentialsProvider provider = + (StringUtils.hasText(accessKey) && StringUtils.hasText(secretKey)) + ? StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey, secretKey)) + : DefaultCredentialsProvider.create(); + return S3Presigner.builder() + .region(Region.of(region)) + .credentialsProvider(provider) + .build(); }추가로, prod 프로파일에서도 S3AutoConfiguration exclude 여부와 자격증명 공급 방식이 일관되는지 확인 부탁드립니다.
src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java (1)
35-37: baseUri 미지정으로 상대 리소스(이미지/CSS) 해석 불가템플릿에서 상대 경로 리소스를 사용할 계획이 있다면 baseUri를 지정하거나 UserAgentCallback을 설정하세요. 현재는 단순 텍스트만이라면 무방하나 확장 시 장애 포인트입니다.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (2)
33-35: pdfUrl에 대한 public Setter 노출은 도메인 무결성 위험PDF URL은 상태 변화(확정/취소)에 종속되므로 임의 변경을 막는 편이 안전합니다. Setter 제거 후 의도된 시점에만 갱신/삭제하는 도메인 메서드로 한정하세요.
적용 제안(diff):
- @Setter @Column(length = 500) private String pdfUrl; // 견적서 PDF URL추가 메서드(파일 내 적절한 위치에 배치):
// 상태 검증을 포함한 의도적 변경 전용 메서드 public void updatePdfUrlWhenConfirmed(String url) { if (this.reservationStatus != ReservationStatus.CONFIRMED) { throw new DomainRuleException(CANNOT_UPDATE_RESERVATION, new IllegalArgumentException("예약 확정 상태에서만 PDF URL을 설정할 수 있습니다.")); } this.pdfUrl = url; } public void clearPdfUrl() { this.pdfUrl = null; }
164-170: 취소 시 pdfUrl 처리 위치 명확화요구사항이 "취소된 예약 조회 시 pdfUrl = null 반환"이라면 DTO 레벨 마스킹인지 실제 필드 null화인지 일관된 정책이 필요합니다. 도메인에서 명시적으로 null 처리하려면 아래와 같이 보완 가능합니다.
예시(diff):
this.reservationStatus = newStatus; + if (newStatus == ReservationStatus.CANCELLED) { + this.pdfUrl = null; // 저장/응답 정책 중 택1 + }src/main/java/konkuk/chacall/global/common/storage/util/PdfTemplateRenderer.java (1)
22-34: 템플릿 엔진 초기화 구성 적절함클래스로더 리졸버, UTF-8, 비캐시 설정 모두 적절합니다. 운영 환경에서는 템플릿이 고정이라면 캐시 활성화 고려 여지 있습니다.
src/main/java/konkuk/chacall/domain/member/application/reservation/MemberReservationService.java (1)
3-3: 사용되지 않는 import 제거이 파일에서는 GetReservationHistoryRequest를 사용하지 않습니다. 불필요한 import는 제거해 주세요.
-import konkuk.chacall.domain.member.presentation.dto.request.GetReservationHistoryRequest;src/main/java/konkuk/chacall/global/common/storage/S3Service.java (1)
29-34: 서버사이드 암호화 및 캐시 헤더 설정 권장민감도와 규정 준수를 위해 SSE-S3(AES256) 적용을 권장합니다. 또한 CDN 캐싱 정책이 필요하다면 Cache-Control/Content-Disposition 설정을 고려해 주세요(브라우저 열람/다운로드 제어).
PutObjectRequest req = PutObjectRequest.builder() .bucket(bucket) .key(key) .metadata(metadata) .contentType(ct != null ? ct : "application/pdf") + .serverSideEncryption("AES256") + // .cacheControl("public,max-age=31536000,immutable") + // .contentDisposition("inline; filename=\"estimate.pdf\"") .build();src/main/java/konkuk/chacall/global/common/storage/util/CdnUrlResolver.java (1)
7-9: 불필요한 @requiredargsconstructor 제거 제안필수(final/@nonnull) 필드가 없어 @requiredargsconstructor는 효과가 없습니다. 제거하여 어노테이션 노이즈를 줄일 수 있습니다.
@Component -@RequiredArgsConstructor public class CdnUrlResolver {src/main/java/konkuk/chacall/domain/member/presentation/dto/request/GetReservationHistoryRequest.java (1)
8-10: 사용되지 않는 import 정리 제안ReservationStatus import는 사용되지 않습니다. 정리해 주세요.
-import konkuk.chacall.domain.reservation.domain.value.ReservationStatus; import konkuk.chacall.domain.reservation.domain.value.ReservationViewType;src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
70-77: 중복 ID 제거로 IN 절 최적화동일 회원의 여러 예약이 포함될 수 있어 IN 파라미터가 불필요하게 커집니다. distinct로 중복 제거하면 DB 부하를 줄일 수 있습니다.
- List<Long> customerIds = reservations.stream() - .map(reservation -> reservation.getMember().getUserId()) - .toList(); + List<Long> customerIds = reservations.stream() + .map(reservation -> reservation.getMember().getUserId()) + .distinct() + .toList();
62-65: member 지연로딩 N+1 가능성
getCustomerMap과mapToReservationHistory에서reservation.getMember()접근 시 N+1이 발생할 수 있습니다. 리포지토리의 목록 조회 메서드에@EntityGraph(attributePaths = {"member"})를 추가하는 것을 권장합니다. 리포지토리 파일에 제안 포함했습니다.src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (1)
15-31: OpenAPI 예시 갱신 — 바인딩은 한글 표시값(value) 기준으로 동작(Converter 확인됨)예시가 "예약 대기"로 남아있습니다. ReservationViewType의 표시값은 "진행 예정" 등이며, EnumValueConverter(@component)가 쿼리 바인딩에서 getValue()로 매칭하므로 OpenAPI example을 표시값으로 갱신하세요.
파일: src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java
- example = "예약 대기", + example = "진행 예정",선택 권고: JSON 직렬화/역직렬화를 일관화하려면 ReservationViewType에도 @jsonvalue 추가를 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/resources/fonts/NotoSansKR-Regular.ttfis excluded by!**/*.ttf
📒 Files selected for processing (28)
build.gradle(1 hunks)src/main/java/konkuk/chacall/domain/member/application/MemberService.java(1 hunks)src/main/java/konkuk/chacall/domain/member/application/reservation/MemberReservationService.java(2 hunks)src/main/java/konkuk/chacall/domain/member/presentation/dto/request/GetReservationHistoryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/member/presentation/dto/response/MemberReservationDetailResponse.java(3 hunks)src/main/java/konkuk/chacall/domain/member/presentation/dto/response/MemberReservationHistoryResponse.java(2 hunks)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java(4 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationDetailResponse.java(3 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationHistoryResponse.java(2 hunks)src/main/java/konkuk/chacall/domain/reservation/application/status/ReservationStatusService.java(3 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java(3 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java(2 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationStatus.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationViewType.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/PdfService.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/S3Service.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/util/CdnUrlResolver.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/util/PdfTemplateRenderer.java(1 hunks)src/main/java/konkuk/chacall/global/config/AwsConfig.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)src/main/resources/templates/reservation-estimate.html(1 hunks)src/test/resources/application-test.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java (1)
src/main/java/konkuk/chacall/global/common/storage/util/PdfTemplateRenderer.java (1)
Component(13-46)
src/main/java/konkuk/chacall/global/common/storage/util/PdfTemplateRenderer.java (1)
src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java (1)
Component(14-45)
🔇 Additional comments (18)
src/test/resources/application-test.yml (2)
69-70: JWT 만료 설정 OK주석과 값(30일)이 일치합니다.
88-91: S3 AutoConfiguration FQCN 검증 필요src/test/resources/application-test.yml (lines 88–91)에 설정된 exclude 값(io.awspring.cloud.autoconfigure.s3.S3AutoConfiguration)이 프로젝트에서 사용하는 Spring Cloud AWS 버전과 일치하는지 확인하세요.
- Spring Cloud AWS 2.x → org.springframework.cloud.aws.autoconfigure.s3.S3AutoConfiguration
- Spring Cloud AWS 3.x (awspring) → io.awspring.cloud.autoconfigure.s3.S3AutoConfiguration
빌드파일(pom.xml 또는 build.gradle(.kts))에서 의존성 그룹/버전 확인:
rg -nP '(io.awspring|spring-cloud-aws|org.springframework.cloud)'
불일치 시 exclude 값을 해당 FQCN으로 교체하거나 불필요하면 제거하세요.src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
110-110: viewType 전환 LGTM.컨트롤러/요청 DTO에서 status → viewType 변경 전파, 기본값/검증(Null 방지)도 함께 확인해 주세요.
src/main/resources/application-local.yml (1)
78-103: 확인 완료 — 폰트·템플릿 존재 및 S3 auto-config 제외 의도 확인됨
- 폰트: src/main/resources/fonts/NotoSansKR-Regular.ttf (존재)
- 템플릿: src/main/resources/templates/reservation-estimate.html (존재)
- app.pdf 참조: src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java, PdfTemplateRenderer.java
- S3 설정: src/main/java/konkuk/chacall/global/config/AwsConfig.java 에서 S3Client/S3Presigner 빈 생성, S3 사용처: src/main/java/konkuk/chacall/global/common/storage/S3Service.java — spring.autoconfigure.exclude로 io.awspring S3AutoConfiguration를 비활성화한 뒤 수동 빈 등록하는 구성은 의도된 설정으로 보임
위 확인으로 원래 코멘트의 우려사항(리소스 존재·오타, autoconfigure 제외)은 해결되었습니다.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (2)
82-89: 평가 가능 상태 재검토: COMPLETED 도입과의 정합성COMPLETED 상태가 추가되었는데, 여전히 CONFIRMED에서만 평가 허용입니다. 완료 후 평가가 의도라면 COMPLETED(또는 CONFIRMED/COMPLETED 둘 다)로 조건을 조정하세요.
변경 예시(diff):
- if (this.reservationStatus != ReservationStatus.CONFIRMED) { + if (this.reservationStatus != ReservationStatus.COMPLETED) { throw new DomainRuleException(CANNOT_RATE_UNCONFIRMED_RESERVATION); }또는 둘 다 허용:
- if (this.reservationStatus != ReservationStatus.CONFIRMED) { + if (this.reservationStatus != ReservationStatus.CONFIRMED + && this.reservationStatus != ReservationStatus.COMPLETED) { throw new DomainRuleException(CANNOT_RATE_UNCONFIRMED_RESERVATION); }
144-155: 수정 가능 상태 검증 추가 LGTM"예약 대기/예약 확정 요청"에서만 수정 허용하는 검증이 PR 목적과 일치합니다.
src/main/java/konkuk/chacall/domain/member/application/MemberService.java (1)
66-67: viewType 파라미터 전파 변경 LGTM + null/유효성 확인 필요호출부 정합성은 좋습니다. 다만
request.viewType()이 null이거나 권한 외 뷰타입(예: CONFIRM_REQUEST)이 넘어오는 경우의 처리(기본값/검증)를 확인하세요.
- Controller/DTO 레벨에서 NotNull/Enum 검증 여부
- Member 사용자에게 노출되지 않는 탭 값 요청 시 빈 결과/에러 중 의도된 동작
src/main/java/konkuk/chacall/global/common/storage/util/PdfTemplateRenderer.java (1)
17-18: 환경 변수/리소스 정합성 확인 필요검증 결과: 템플릿 파일이 src/main/resources/templates/reservation-estimate.html에 존재합니다. 저장소의 src/main/resources/**/*.yml 또는 *.properties에서 app.pdf.template 항목은 찾지 못했습니다 — 설정이 환경변수/CI 등 외부에서 관리되는지 확인하고, app.pdf.template 값이 'reservation-estimate'(확장자 제외)로 설정되어 있는지 맞춰야 합니다.
src/main/resources/templates/reservation-estimate.html (1)
23-31: 템플릿 바인딩 검증 — 'parsingIsUserElectricity'는 오타 아님; 게터/메서드 존재 확인ReservationInfo에 parsingIsUserElectricity(), parsingReservationDeposit(), getFormattedDateTimeInfos()가 정의되어 있습니다 (src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java). member/foodTruck의 name 게터 사용도 확인됨(여러 DTO에서 foodTruck.getName(), member.getName() 호출). 템플릿의 메서드 호출 vs 프로퍼티 접근 일관성은 스타일상의 선택입니다.
Likely an incorrect or invalid review comment.
src/main/java/konkuk/chacall/domain/member/presentation/dto/response/MemberReservationHistoryResponse.java (1)
20-23: 예약 상태 노출 추가 적절히스토리 응답에 표시용 예약 상태 문자열 추가가 명확합니다. 조회/정렬 로직에 영향 없이 UI 표시 개선에 도움이 됩니다.
Also applies to: 33-35
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationHistoryResponse.java (1)
22-24: 예약 상태 노출 추가 적절오너 히스토리 응답에도 표시용 상태 문자열을 포함한 점 좋습니다. 일관성 있게 멤버/오너 응답 모두 동일 포맷을 유지합니다.
Also applies to: 36-38
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationDetailResponse.java (1)
53-55: 취소 상태에서 pdfUrl 비노출 처리 적절요구사항(취소 시 pdfUrl 숨김)에 부합합니다. 동일 로직이 멤버 상세에도 반영되어 있어 일관적입니다.
src/main/java/konkuk/chacall/domain/member/presentation/dto/response/MemberReservationDetailResponse.java (1)
52-54: 취소 상태에서 pdfUrl 비노출 + 상태 문자열 추가 적절멤버 상세 응답도 취소 시 pdfUrl 비노출, 상태 문자열 포함이 잘 반영되었습니다.
Also applies to: 58-59
src/main/java/konkuk/chacall/domain/member/application/reservation/MemberReservationService.java (1)
26-36: 페이지 사이즈 가드 추가 및 커서-정렬 일관성 확인 필요
- pageSize ≤ 0 입력 시 PageRequest.of 호출에서 예외 발생 — 간단한 가드 추가 권장. viewType null 체크도 권장.
- 리포지토리 쿼리의 ORDER BY( reservation_id 의 ASC/DESC )와 커서 조건( reservation_id < lastCursor — DESC일 때, reservation_id > lastCursor — ASC일 때 )이 일치하는지 확인 필요(불일치 시 페이지 누락/중복 발생).
다음 패치 제안은 그대로 적용해 주세요:
public CursorPagingResponse<MemberReservationHistoryResponse> getMemberReservations(User member, ReservationViewType viewType, Long lastCursor, int pageSize) { + if (pageSize <= 0) { + throw new IllegalArgumentException("pageSize must be > 0"); + } + if (viewType == null) { + throw new IllegalArgumentException("viewType must not be null"); + } Slice<Reservation> memberReservationSlice = reservationRepository .findMemberReservationsByStatusWithCursor(member.getUserId(), viewType.getStatuses(), lastCursor, PageRequest.of(0, pageSize));src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationViewType.java (2)
31-34: UPCOMING에 CANCELLED_REQUESTED 포함 의도 확인“진행 예정”에
CANCELLED_REQUESTED(취소 요청)을 포함시키면 사용자 혼란이 있을 수 있습니다. “취소 내역” 또는 별도 탭으로 분류가 맞는지 PM 기획 확인 부탁드립니다.
41-44: COMPLETED 상태 전환 전략 확인
COMPLETED전용 매핑을 사용하려면 배치/도메인 이벤트로 확정 상태가 일정 경과 시COMPLETED로 전환되어야 합니다. 전환 로직이 없다면 과거 확정 건이 “진행 예정”에 남을 수 있습니다.src/main/java/konkuk/chacall/global/common/storage/PdfService.java (1)
29-37: 해결 — KeyUtils가 업로드마다 타임스탬프를 포함합니다KeyUtils.buildReservationPdfKey(Long)는 yyyyMMdd-HHmmss 포맷의 타임스탬프를 경로에 포함(src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java)하므로 업로드마다 고유 키가 생성되어 CloudFront의 구버전 노출 문제는 방지됩니다.
동일 초 내 다중 업로드 가능성이 있으면 밀리초(epoch ms) 또는 UUID를 키에 추가해 충돌을 완전 차단하세요.src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
34-46: 첫 페이지(lastCursor == null) 문제 없음 — CursorPagingRequest가 기본값(Long.MAX_VALUE)으로 처리함HasPaging.pagingOrDefault()가 null일 때 new CursorPagingRequest(null, null)을 생성하고, CursorPagingRequest의 compact constructor에서 cursor가 null이면 DEFAULT_CURSOR(Long.MAX_VALUE)로 대체되어 리포지토리에 null이 전달되지 않습니다.
참조: src/main/java/konkuk/chacall/global/common/dto/HasPaging.java, src/main/java/konkuk/chacall/global/common/dto/CursorPagingRequest.javaLikely an incorrect or invalid review comment.
| @Parameter( | ||
| name = "status", in = ParameterIn.QUERY, | ||
| description = "조회할 예약 상태", | ||
| name = "viewType", in = ParameterIn.QUERY, | ||
| description = "조회할 예약 내역 타입", | ||
| required = true, | ||
| schema = @Schema( | ||
| type = "string", | ||
| example = "예약 대기", | ||
| allowableValues = { |
There was a problem hiding this comment.
🧹 Nitpick
Swagger 예시값(example)과 허용값(allowableValues) 불일치
example이 "예약 대기"로 남아 있어 허용값과 불일치합니다. 혼선을 줄이기 위해 허용값 중 하나로 교체해 주세요.
@Parameter(
name = "viewType", in = ParameterIn.QUERY,
description = "조회할 예약 내역 타입",
required = true,
schema = @Schema(
type = "string",
- example = "예약 대기",
+ example = "진행 예정",
allowableValues = {
"진행 예정",
"확정 신청",
"완료 내역",
"취소 내역"
}
)📝 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.
| @Parameter( | |
| name = "status", in = ParameterIn.QUERY, | |
| description = "조회할 예약 상태", | |
| name = "viewType", in = ParameterIn.QUERY, | |
| description = "조회할 예약 내역 타입", | |
| required = true, | |
| schema = @Schema( | |
| type = "string", | |
| example = "예약 대기", | |
| allowableValues = { | |
| @Parameter( | |
| name = "viewType", in = ParameterIn.QUERY, | |
| description = "조회할 예약 내역 타입", | |
| required = true, | |
| schema = @Schema( | |
| type = "string", | |
| example = "진행 예정", | |
| allowableValues = { | |
| "진행 예정", | |
| "확정 신청", | |
| "완료 내역", | |
| "취소 내역" | |
| } | |
| ) |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/member/presentation/dto/request/GetReservationHistoryRequest.java
around lines 14 to 21, the Swagger @Parameter annotation has an example value
"예약 대기" that does not match the declared allowableValues; update the example to
exactly match one of the strings in the allowableValues list (replace "예약 대기"
with one of those allowed values) so the example and allowableValues are
consistent.
| reservation.updateStatus(request.reservationStatus()); | ||
|
|
||
| // 예약 상태가 CONFIRMED로 변경되면 견적서 PDF 렌더링 및 업로드 | ||
| String pdfUrl = pdfService.renderAndUpload(reservation); | ||
| reservation.setPdfUrl(pdfUrl); | ||
|
|
||
| return new ReservationStatusResponse(reservation.getReservationStatus().getValue()); | ||
| } |
There was a problem hiding this comment.
CONFIRMED 검증 부재 + 외부 I/O 실패 시 부분 적용 위험.
- 메소드명/의도상 CONFIRMED로 전환하는 API인데, 요청 값에 대한 CONFIRMED 검증이 없습니다. 방어 로직 추가를 권장합니다.
- 상태를 먼저 변경한 뒤 PDF 업로드를 시도하므로, 업로드 실패 시 “상태는 CONFIRMED이나 pdfUrl은 없음” 상태가 될 수 있습니다. 트랜잭션 경계 내에서 예외 시 롤백되도록 하고, 외부 I/O는 가급적 커밋 이후(afterCommit) 실행을 권장합니다.
다음과 같이 최소한의 방어/롤백을 추가해 주세요.
+import org.springframework.transaction.annotation.Transactional;
@@
- public ReservationStatusResponse updateReservationStatusToConfirmed(Long reservationId, UpdateReservationStatusRequest request, User owner) {
+ @Transactional
+ public ReservationStatusResponse updateReservationStatusToConfirmed(Long reservationId, UpdateReservationStatusRequest request, User owner) {
@@
- reservation.updateStatus(request.reservationStatus());
-
- // 예약 상태가 CONFIRMED로 변경되면 견적서 PDF 렌더링 및 업로드
- String pdfUrl = pdfService.renderAndUpload(reservation);
- reservation.setPdfUrl(pdfUrl);
+ // (1) 요청 상태 유효성 방어 (도메인 예외 타입으로 교체 권장)
+ if (request.reservationStatus() == null /* || request.reservationStatus() != ReservationStatus.CONFIRMED */) {
+ throw new IllegalArgumentException("예약 상태는 CONFIRMED만 허용됩니다.");
+ }
+
+ // (2) 상태 변경 + 외부 I/O 실패 시 되돌리기
+ var previousStatus = reservation.getReservationStatus();
+ reservation.updateStatus(request.reservationStatus());
+ try {
+ String pdfUrl = pdfService.renderAndUpload(reservation);
+ reservation.setPdfUrl(pdfUrl);
+ } catch (RuntimeException e) {
+ reservation.updateStatus(previousStatus);
+ throw e;
+ }운영 안정성을 위해선 PDF 생성/업로드는 트랜잭션 커밋 후 TransactionSynchronizationManager.afterCommit 또는 비동기 이벤트/잡으로 처리하는 것을 권장합니다(커밋 실패 시 불필요한 업로드 방지).
📝 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.
| reservation.updateStatus(request.reservationStatus()); | |
| // 예약 상태가 CONFIRMED로 변경되면 견적서 PDF 렌더링 및 업로드 | |
| String pdfUrl = pdfService.renderAndUpload(reservation); | |
| reservation.setPdfUrl(pdfUrl); | |
| return new ReservationStatusResponse(reservation.getReservationStatus().getValue()); | |
| } | |
| @Transactional | |
| public ReservationStatusResponse updateReservationStatusToConfirmed(Long reservationId, UpdateReservationStatusRequest request, User owner) { | |
| // (1) 요청 상태 유효성 방어 (도메인 예외 타입으로 교체 권장) | |
| if (request.reservationStatus() == null /* || request.reservationStatus() != ReservationStatus.CONFIRMED */) { | |
| throw new IllegalArgumentException("예약 상태는 CONFIRMED만 허용됩니다."); | |
| } | |
| // (2) 상태 변경 + 외부 I/O 실패 시 되돌리기 | |
| var previousStatus = reservation.getReservationStatus(); | |
| reservation.updateStatus(request.reservationStatus()); | |
| try { | |
| String pdfUrl = pdfService.renderAndUpload(reservation); | |
| reservation.setPdfUrl(pdfUrl); | |
| } catch (RuntimeException e) { | |
| reservation.updateStatus(previousStatus); | |
| throw e; | |
| } | |
| return new ReservationStatusResponse(reservation.getReservationStatus().getValue()); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/reservation/application/status/ReservationStatusService.java
around lines 39-46, add a guard that the incoming request.reservationStatus() is
CONFIRMED and reject/throw a validation exception if not; do not change the
entity status and commit until validation passes. Remove the synchronous PDF
render/upload from the same transaction that updates the status: either register
a TransactionSynchronizationManager.afterCommit callback to call
pdfService.renderAndUpload(reservation) and then persist/set the pdfUrl in a
separate transactional operation, or publish an asynchronous event/job that
performs PDF generation and updates the reservation.pdfUrl after the status
transaction commits; if you keep synchronous upload in the same method, ensure
failures throw before returning so the transaction rolls back (but preferred
approach is afterCommit/event to avoid external I/O inside the transaction).
| @Query("SELECT r FROM Reservation r " + | ||
| "WHERE r.foodTruck.owner.userId = :ownerId " + | ||
| "AND r.reservationStatus = :status " + | ||
| "AND r.reservationStatus IN :statuses " + | ||
| "AND r.reservationId < :lastCursor " + | ||
| "ORDER BY r.reservationId DESC") | ||
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | ||
| @Param("ownerId") Long ownerId, | ||
| @Param("status") ReservationStatus status, | ||
| @Param("statuses") Set<ReservationStatus> statuses, | ||
| @Param("lastCursor") Long lastCursor, | ||
| Pageable pageable); |
There was a problem hiding this comment.
lastCursor null 시 조회 실패 위험 → JPQL 안전 가드 필요
첫 페이지 호출에서 lastCursor가 null이면 현재 JPQL(r.reservationId < :lastCursor)은 에러가 납니다. 다음과 같이 null 가드를 추가하세요.
- @Query("SELECT r FROM Reservation r " +
- "WHERE r.foodTruck.owner.userId = :ownerId " +
- "AND r.reservationStatus IN :statuses " +
- "AND r.reservationId < :lastCursor " +
- "ORDER BY r.reservationId DESC")
+ @EntityGraph(attributePaths = {"member"})
+ @Query("SELECT r FROM Reservation r " +
+ "WHERE r.foodTruck.owner.userId = :ownerId " +
+ "AND r.reservationStatus IN :statuses " +
+ "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " +
+ "ORDER BY r.reservationId DESC")추가로 @EntityGraph(attributePaths = {"member"})로 N+1을 줄였습니다.
📝 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.
| @Query("SELECT r FROM Reservation r " + | |
| "WHERE r.foodTruck.owner.userId = :ownerId " + | |
| "AND r.reservationStatus = :status " + | |
| "AND r.reservationStatus IN :statuses " + | |
| "AND r.reservationId < :lastCursor " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | |
| @Param("ownerId") Long ownerId, | |
| @Param("status") ReservationStatus status, | |
| @Param("statuses") Set<ReservationStatus> statuses, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); | |
| @EntityGraph(attributePaths = {"member"}) | |
| @Query("SELECT r FROM Reservation r " + | |
| "WHERE r.foodTruck.owner.userId = :ownerId " + | |
| "AND r.reservationStatus IN :statuses " + | |
| "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | |
| @Param("ownerId") Long ownerId, | |
| @Param("statuses") Set<ReservationStatus> statuses, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java
around lines 18 to 27, the JPQL predicate r.reservationId < :lastCursor will
fail when lastCursor is null on the first page; change the query to guard
against null by using a conditional check such as "AND (:lastCursor IS NULL OR
r.reservationId < :lastCursor)" so the clause is skipped when lastCursor is
null, and add @EntityGraph(attributePaths = {"member"}) on the repository method
to fetch the related member and avoid N+1 queries.
| @Query("SELECT r FROM Reservation r " + | ||
| "WHERE r.member.userId = :memberId " + | ||
| "AND r.reservationStatus = :status " + | ||
| "AND r.reservationStatus IN :statuses " + | ||
| "AND r.reservationId < :lastCursor " + | ||
| "ORDER BY r.reservationId DESC") | ||
| Slice<Reservation> findMemberReservationsByStatusWithCursor( | ||
| @Param("memberId") Long memberId, | ||
| @Param("status") ReservationStatus status, | ||
| @Param("statuses") Set<ReservationStatus> statuses, | ||
| @Param("lastCursor") Long lastCursor, | ||
| Pageable pageable); |
There was a problem hiding this comment.
member 목록 조회도 null 커서 가드 필요
동일하게 null 커서를 안전하게 처리하세요.
- @EntityGraph(attributePaths = {"foodTruck"})
- @Query("SELECT r FROM Reservation r " +
- "WHERE r.member.userId = :memberId " +
- "AND r.reservationStatus IN :statuses " +
- "AND r.reservationId < :lastCursor " +
- "ORDER BY r.reservationId DESC")
+ @EntityGraph(attributePaths = {"foodTruck"})
+ @Query("SELECT r FROM Reservation r " +
+ "WHERE r.member.userId = :memberId " +
+ "AND r.reservationStatus IN :statuses " +
+ "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " +
+ "ORDER BY r.reservationId DESC")📝 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.
| @Query("SELECT r FROM Reservation r " + | |
| "WHERE r.member.userId = :memberId " + | |
| "AND r.reservationStatus = :status " + | |
| "AND r.reservationStatus IN :statuses " + | |
| "AND r.reservationId < :lastCursor " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findMemberReservationsByStatusWithCursor( | |
| @Param("memberId") Long memberId, | |
| @Param("status") ReservationStatus status, | |
| @Param("statuses") Set<ReservationStatus> statuses, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); | |
| @EntityGraph(attributePaths = {"foodTruck"}) | |
| @Query("SELECT r FROM Reservation r " + | |
| "WHERE r.member.userId = :memberId " + | |
| "AND r.reservationStatus IN :statuses " + | |
| "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findMemberReservationsByStatusWithCursor( | |
| @Param("memberId") Long memberId, | |
| @Param("statuses") Set<ReservationStatus> statuses, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java
around lines 37 to 46, the query does not guard against a null lastCursor;
change the JPQL to handle a nullable cursor by adding a condition like
"(:lastCursor IS NULL OR r.reservationId < :lastCursor)" so the query still
returns the first page when lastCursor is null, keep the lastCursor parameter as
Long (nullable) and ensure callers can pass null for the first page.
| CANNOT_UPDATE_RESERVATION(HttpStatus.BAD_REQUEST, 90008, "예약을 수정할 수 있는 상태가 아닙니다."), | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
에러코드 추가 LGTM. 코드 중복(값 충돌)만 점검하세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 에러 코드 숫자 중복 여부 점검
rg -nP '^\s*[A-Z0-9_]+\s*\(HttpStatus\.\w+,\s*(\d+),' -U | awk -F'[(), ]+' '{print $7, $1":"$0}' | sort -n | awk 'prev==$1{print "DUP:",$0} {prev=$1}'Length of output: 3380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java"
python3 - <<'PY'
import re
from collections import defaultdict
p=re.compile(r'^\s*([A-Z0-9_]+)\s*\(\s*HttpStatus\.\w+\s*,\s*([0-9]+)\s*,')
d=defaultdict(list)
try:
with open("src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java","r",encoding="utf-8") as f:
for i,l in enumerate(f,1):
m=p.search(l)
if m:
name=m.group(1); num=int(m.group(2))
d[num].append((i,name,l.strip()))
except FileNotFoundError:
print("ERROR: file not found:", "src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java")
raise SystemExit(2)
dups = {k:v for k,v in d.items() if len(v)>1}
if not dups:
print("NO_DUPLICATE_NUMBERS_FOUND")
else:
for k in sorted(dups):
print("DUPLICATE_NUMBER", k)
for ln,name,line in dups[k]:
print(f" {ln} {name}: {line}")
PYLength of output: 213
중복된 에러 코드 번호(40003) 발견 — 수정 필요
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java — INVALID_ENUM_VALUE (라인 15) 및 API_INVALID_TYPE (라인 22)가 동일한 숫자 40003을 사용합니다. 하나를 다른 고유 번호로 변경하거나 충돌을 제거하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java
around lines 15 and 22, the enum constants INVALID_ENUM_VALUE and
API_INVALID_TYPE both use the duplicate numeric code 40003; pick a new unique
integer for one of them (ensure it doesn't conflict with any other ErrorCode
values), update that enum entry to the new number and adjust any usages/tests
that expect the old code so everything compiles and tests pass.
| public String resolve(String objectKey) { | ||
| // CDN 도메인 기준으로 URL 구성 | ||
| // 예) https://cdn.example.com/reservations/1/estimate-1234.pdf | ||
| String normalized = objectKey.startsWith("/") ? objectKey.substring(1) : objectKey; | ||
| return String.format("%s/%s", trimRightSlash(cdnBaseUrl), normalized); | ||
| } |
There was a problem hiding this comment.
objectKey null 처리 누락으로 인한 NPE 가능성
resolve에서 objectKey가 null/blank면 NPE가 발생합니다. 가드를 추가해 주세요.
public String resolve(String objectKey) {
// CDN 도메인 기준으로 URL 구성
// 예) https://cdn.example.com/reservations/1/estimate-1234.pdf
- String normalized = objectKey.startsWith("/") ? objectKey.substring(1) : objectKey;
+ if (objectKey == null || objectKey.isBlank()) {
+ throw new IllegalArgumentException("objectKey must not be blank");
+ }
+ String normalized = objectKey.startsWith("/") ? objectKey.substring(1) : objectKey;
return String.format("%s/%s", trimRightSlash(cdnBaseUrl), normalized);
}📝 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.
| public String resolve(String objectKey) { | |
| // CDN 도메인 기준으로 URL 구성 | |
| // 예) https://cdn.example.com/reservations/1/estimate-1234.pdf | |
| String normalized = objectKey.startsWith("/") ? objectKey.substring(1) : objectKey; | |
| return String.format("%s/%s", trimRightSlash(cdnBaseUrl), normalized); | |
| } | |
| public String resolve(String objectKey) { | |
| // CDN 도메인 기준으로 URL 구성 | |
| // 예) https://cdn.example.com/reservations/1/estimate-1234.pdf | |
| if (objectKey == null || objectKey.isBlank()) { | |
| throw new IllegalArgumentException("objectKey must not be blank"); | |
| } | |
| String normalized = objectKey.startsWith("/") ? objectKey.substring(1) : objectKey; | |
| return String.format("%s/%s", trimRightSlash(cdnBaseUrl), normalized); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/util/CdnUrlResolver.java
around lines 14 to 19, the resolve method can NPE when objectKey is null or
blank; add an explicit guard at the start that checks if objectKey == null ||
objectKey.isBlank() and throw a clear IllegalArgumentException (e.g., "objectKey
must not be null or blank") to fail fast, then proceed with the existing
normalization and URL assembly logic.
| @Value("${app.pdf.font-path}") | ||
| private Resource fontResource; |
There was a problem hiding this comment.
🧩 Analysis chain
환경 변수 확인 필요: app.pdf.font-path
모든 프로필(yml/properties)에 app.pdf.font-path가 설정되어 있는지 점검해 주세요.
다음 스크립트로 확인 가능합니다:
🏁 Script executed:
#!/bin/bash
# app.pdf.font-path / app.pdf.template 설정과 템플릿 파일 존재 여부 확인
echo "== search properties =="
rg -n -C2 -g 'src/main/resources/**/*.yml' -g 'src/main/resources/**/*.properties' 'app\.pdf\.(font-path|template)'
echo
echo "== template file =="
fd -a 'reservation-estimate.html' src/main/resources/templatesLength of output: 118
필수: app.pdf.font-path 누락 — 설정 추가 또는 코드 예외 처리 필요
src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java에서 @value("${app.pdf.font-path}")로 주입되나, 리포지토리의 src/main/resources/**/*.yml/.properties에 해당 키가 없습니다. 템플릿은 src/main/resources/templates/reservation-estimate.html에 존재합니다. 모든 활성 프로파일에 app.pdf.font-path를 추가하거나 코드에 기본값/옵션 처리를 추가해(예: @value("${app.pdf.font-path:...}") 또는 Resource 예외 처리) 애플리케이션 시작 실패를 방지하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java
around lines 18-19, the @Value("${app.pdf.font-path}") injection assumes
app.pdf.font-path exists but no such key is present in src/main/resources for
active profiles; either add app.pdf.font-path to all relevant
application-*.yml/properties files pointing to the font resource, or make the
code resilient by providing a default or optional handling (e.g., change the
@Value to include a default path or inject an Optional/nullable Resource and add
fallback logic that loads a bundled font or throws a clear startup exception),
and ensure any file-loading code checks for resource existence and logs a
descriptive error if missing to avoid startup failures.
| public byte[] generate(String html) { | ||
| try (ByteArrayOutputStream out = new ByteArrayOutputStream(); | ||
| InputStream in = fontResource.getInputStream()) { | ||
| PdfRendererBuilder builder = new PdfRendererBuilder(); | ||
| builder.useFastMode(); | ||
|
|
||
| builder.useFont( | ||
| () -> in, | ||
| "NotoSansKR", | ||
| 400, | ||
| BaseRendererBuilder.FontStyle.NORMAL, | ||
| true | ||
| ); |
There was a problem hiding this comment.
useFont에 동일 InputStream을 재사용하는 버그 (런타임 실패 가능성 높음)
Supplier가 여러 번 호출될 수 있는데, 동일 InputStream in을 반환하고 있어 EOF/closed 스트림 예외가 발생할 수 있습니다. 매 호출마다 새로운 InputStream을 열도록 변경하세요.
적용 제안(diff):
- public byte[] generate(String html) {
- try (ByteArrayOutputStream out = new ByteArrayOutputStream();
- InputStream in = fontResource.getInputStream()) {
+ public byte[] generate(String html) {
+ try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
PdfRendererBuilder builder = new PdfRendererBuilder();
builder.useFastMode();
builder.useFont(
- () -> in,
+ () -> {
+ try {
+ return fontResource.getInputStream();
+ } catch (IOException e) {
+ throw new RuntimeException("폰트 로딩 실패", e);
+ }
+ },
"NotoSansKR",
400,
BaseRendererBuilder.FontStyle.NORMAL,
true
);📝 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.
| public byte[] generate(String html) { | |
| try (ByteArrayOutputStream out = new ByteArrayOutputStream(); | |
| InputStream in = fontResource.getInputStream()) { | |
| PdfRendererBuilder builder = new PdfRendererBuilder(); | |
| builder.useFastMode(); | |
| builder.useFont( | |
| () -> in, | |
| "NotoSansKR", | |
| 400, | |
| BaseRendererBuilder.FontStyle.NORMAL, | |
| true | |
| ); | |
| public byte[] generate(String html) { | |
| try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { | |
| PdfRendererBuilder builder = new PdfRendererBuilder(); | |
| builder.useFastMode(); | |
| builder.useFont( | |
| () -> { | |
| try { | |
| return fontResource.getInputStream(); | |
| } catch (IOException e) { | |
| throw new RuntimeException("폰트 로딩 실패", e); | |
| } | |
| }, | |
| "NotoSansKR", | |
| 400, | |
| BaseRendererBuilder.FontStyle.NORMAL, | |
| true | |
| ); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/util/PdfGenerator.java
around lines 21 to 33, the Supplier passed to builder.useFont reuses the same
InputStream `in`, causing EOF/closed stream errors when the Supplier is invoked
multiple times; change the Supplier to open and return a new InputStream each
time by calling fontResource.getInputStream() inside the Supplier lambda so each
invocation gets a fresh stream, and keep the surrounding try-with-resources to
close the ByteArrayOutputStream (but do not close the streams returned by the
Supplier there).
| --- | ||
| # S3 관련 설정 | ||
| cloud: | ||
| aws: | ||
| s3: | ||
| bucket: bucket-name | ||
| cloud-front-url: https://cdn.net | ||
| credentials: | ||
| access-key: aws_access_key | ||
| secret-key: aws_secret_key | ||
| region: | ||
| static: ap-northeast-2 | ||
| auto: false | ||
| stack: | ||
| auto: false | ||
|
|
There was a problem hiding this comment.
테스트 설정에 하드코딩된 AWS 자격증명 제거 필요
application-test.yml이라도 access-key/secret-key를 레포지토리에 하드코딩하면 보안/컴플라이언스 리스크가 큽니다. 환경변수 바인딩으로 전환하고 안전한 기본값을 두세요. 또한 LocalStack/Mock 빈으로의 전환을 권장합니다.
적용 제안(diff):
cloud:
aws:
s3:
- bucket: bucket-name
- cloud-front-url: https://cdn.net
+ bucket: ${AWS_S3_BUCKET:dummy-bucket}
+ cloud-front-url: ${CLOUDFRONT_URL:http://localhost:8080}
credentials:
- access-key: aws_access_key
- secret-key: aws_secret_key
+ access-key: ${AWS_ACCESS_KEY_ID:test}
+ secret-key: ${AWS_SECRET_ACCESS_KEY:test}
region:
- static: ap-northeast-2
+ static: ${AWS_REGION:ap-northeast-2}
auto: false
stack:
auto: false옵션:
- 통합테스트가 필요하면 LocalStack을 사용하고, 테스트 시 S3 클라이언트 생성 빈을 profile=test에서 Mock으로 대체하세요.
🤖 Prompt for AI Agents
In src/test/resources/application-test.yml around lines 71 to 86, remove the
hard-coded AWS credentials and replace them with environment-bound placeholders
(e.g., reference ACCESS_KEY and SECRET_KEY env vars with safe empty/default
values) so no secrets are committed; additionally, add a test profile
configuration to switch the S3 client to LocalStack or a Mock bean (or set a
flag like use-localstack: true) so integration tests use a local S3 emulator,
and update documentation/README to instruct setting the env vars or enabling the
test mock/localstack profile when running tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/resources/application-dev.yml (1)
76-91: S3/CloudFront 설정은 적절 — CLOUDFRONT_URL 포맷과 자격 증명 운용 점검 권장
- cloud-front-url에 트레일링 슬래시가 없도록 합의된 포맷인지 확인해 주세요(이중 슬래시 방지).
- dev에서 환경변수로 액세스 키를 주입하는 방식은 가능하나, 배포 환경에서는 IAM Role(Instance/Task Role) 사용을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/resources/application-dev.yml(1 hunks)src/test/resources/application-test.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/resources/application-test.yml
🔇 Additional comments (2)
src/main/resources/application-dev.yml (2)
74-75: JWT 액세스 토큰 만료 30일 설정(2592000000ms) 확인 필요dev 환경에서 30일이 의도된 값인지 확인해 주세요. 프로덕션/로컬과의 정책 일관성도 점검 권장.
93-97: dev에 PDF 설정 추가된 점 확인 — 이전 지적사항 해결됨이전 코멘트(“dev 프로파일에 PDF 설정 누락”)가 해결되었습니다. 폰트/템플릿 리소스가 실제 포함되어 있는지만 확인해 주세요.
다음 스크립트로 리소스 존재 여부를 빠르게 확인할 수 있습니다:
#!/bin/bash # fonts / template 리소스 확인 set -euo pipefail echo "Checking font file..." fd -a 'NotoSansKR-Regular.ttf' -S -H | rg -n 'src/main/resources/fonts/NotoSansKR-Regular\.ttf' || { echo "Missing: src/main/resources/fonts/NotoSansKR-Regular.ttf"; exit 1; } echo "Checking pdf template..." fd -a 'reservation-estimate.html' -S -H | rg -n 'src/main/resources/templates/reservation-estimate\.html' || { echo "Missing: src/main/resources/templates/reservation-estimate.html"; exit 1; } echo "OK: Required resources found."
| spring: | ||
| autoconfigure: | ||
| exclude: | ||
| - io.awspring.cloud.autoconfigure.s3.S3AutoConfiguration No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
S3 AutoConfiguration 제외는 합리적 — 수동 빈 등록 존재 여부 확인
자동구성을 배제했으므로 S3Client/AmazonS3 클라이언트 및 업로더 관련 빈을 직접 구성해야 합니다(AwsConfig 등). dev만 제외하는 전략이 의도인지, 다른 프로파일과의 정합성도 점검해 주세요.
다음 스크립트로 수동 빈 구성 존재 여부를 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# S3 클라이언트 빈 정의 존재 여부 점검(정적 확인)
rg -nP --type=java -C3 '(class|@Configuration)\s+.+|@Bean|S3Client|AmazonS3'Length of output: 77628
수동 S3 빈 등록 확인 및 프로파일별 중복 정의 방지
AwsConfig.java에서 S3Client·S3Presigner 빈(@bean)이 수동으로 등록되어 있습니다. application-dev.yml에서만 AutoConfiguration을 제외하고 있어 다른 프로파일에서는 AutoConfiguration이 활성화된 상태에서 빈 중복 오류가 발생할 수 있습니다.
· @ConditionalOnMissingBean 추가 또는
· 다른 프로파일에도 AutoConfiguration 제외／수동 빈 제한
src/main/java/konkuk/chacall/global/config/AwsConfig.java:25,37
🤖 Prompt for AI Agents
In src/main/resources/application-dev.yml lines 99-102 and related
src/main/java/konkuk/chacall/global/config/AwsConfig.java lines 25 and 37, the
S3 AutoConfiguration is only excluded for the dev profile while AwsConfig
manually registers S3Client and S3Presigner beans, which can cause
duplicate-bean errors in other profiles; fix by adding @ConditionalOnMissingBean
on the S3Client and S3Presigner @Bean methods in AwsConfig so they are only
created when no auto-configured beans exist, or alternatively ensure
AutoConfiguration exclusion or profile-restriction is applied consistently
across non-dev profiles (choose one approach and apply it to both S3Client and
S3Presigner registrations).
| // 3) 업로드 Key (예: /reservations/{reservationId}/estimate-<epoch>.pdf) | ||
| String objectKey = KeyUtils.buildReservationPdfKey(reservation.getReservationId()); | ||
|
|
||
| // 4) S3 업로드 | ||
| s3Service.uploadPdf(objectKey, "application/pdf", pdfBytes); |
There was a problem hiding this comment.
reservation.getReservationId() 호출은 존재하지 않습니다.
Reservation 엔티티에는 getReservationId()가 없고 getId()만 존재합니다. 현재 구현대로라면 컴파일이 되지 않으니, 키 생성 시 reservation.getId()를 사용하도록 수정해 주세요.
- String objectKey = KeyUtils.buildReservationPdfKey(reservation.getReservationId());
+ String objectKey = KeyUtils.buildReservationPdfKey(reservation.getId());📝 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.
| // 3) 업로드 Key (예: /reservations/{reservationId}/estimate-<epoch>.pdf) | |
| String objectKey = KeyUtils.buildReservationPdfKey(reservation.getReservationId()); | |
| // 4) S3 업로드 | |
| s3Service.uploadPdf(objectKey, "application/pdf", pdfBytes); | |
| // 3) 업로드 Key (예: /reservations/{reservationId}/estimate-<epoch>.pdf) | |
| String objectKey = KeyUtils.buildReservationPdfKey(reservation.getId()); | |
| // 4) S3 업로드 | |
| s3Service.uploadPdf(objectKey, "application/pdf", pdfBytes); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/PdfService.java around
lines 30 to 34, the code calls reservation.getReservationId() which does not
exist on the Reservation entity; replace that call with reservation.getId() when
building the S3 object key (i.e., pass reservation.getId() into
KeyUtils.buildReservationPdfKey) so the code compiles and the correct identifier
is used for the key.
ksg1227
left a comment
ksg1227
left a comment
There was a problem hiding this comment.
굿굿 아주 좋습니다! 요구사항을 꼼꼼히 잘 분석해주셨군요.
한 가지 궁금한 것은, 그럼 예약 견적서를 작성(수정)중인 상태에서는 pdf 가 만들어지지 않기 때문에, 예약 견적서 생성 혹은 수정시에는 pdf 업로드 혹은 수정이 필요 없는거죠?
| reservation.getReservationStatus() == ReservationStatus.CANCELLED ? // 예약 취소 상태이면 null 반환 | ||
| null : reservation.getPdfUrl(), |
There was a problem hiding this comment.
한 가지 궁금한 것이 있는데, 예약이 최초로 생성되었을 때의 상태인 PENDING 상태, 즉 예약 대기 상태는
이 진행 예정, 완료 내역, 취소 내역 중 어디에도 포함되지 않는 상태인거죠?
이걸 여쭤본 이유는, 일단 PENDING 상태의 경우는 당연히 PDF 가 아직 생성되지 않은 시점일 것 같아서, 혹시나 ReservationStatus 가 PENDING 인 예약을 상세 조회할 시 pdfUrl 이 null 임에도 불구하고 reservation.getPdfUrl() 메서드가 호출될 것 같다고 생각했습니다. 약간 예상과 다르게 동작하는 상황? 같은 느낌이었습니다. (물론 애초에 nullable 하게 유지하기 때문에 문제가 발생할 것 같진 않습니다!)
다만 앞서 여쭤본 것처럼 어차피 PENDING 상태인 예약은 예약 목록 조회에서 조회되지 않는 상황이라면, PENDING 상태인 예약에 대해서는 상세 조회를 할 일이 없기 때문에 현재 구현이 논리적으로도 올바른 것 같아서 한 번 여쭤봅니당
There was a problem hiding this comment.
네넵 우선 예약 내역 뷰에서의 예약 상태 그룹화는 다음과 같습니다.
- 진행 예정 -> 예약 확정 (예약 날짜 안지난 것), 예약 취소 요청
- 확정 신청 -> 예약 확정 요청
- 완료 내역 -> 예약 완료 (예약 확정 상태에서 예약 날짜 지난 것만)
- 취소 내역 -> 예약 취소
따라서, 예약 대기 상태는 단순히 사장님이 예약 견적서를 작성하자마자 Reservation 테이블에 row를 추가하기 위함이지 예약 목록에서는 보여지지 않을 예정입니다. 제가 요구사항을 이해하기론 예약 견적서 pdf를 다운가능한 곳은 '진행 예정'과 '완료 내역'에 존재하는 예약 뿐인 것으로 알고 있습니다. 따라서, 상균님이 염려하시는 pdfUrl이 null임에도 불구하고 reservation.getPdfUrl() 메서드가 호출되는 일은 로직상 발생하지 않을 것으로 생각됩니다!
제가 질문을 잘 이해한게 맞나요??
| public record GetReservationHistoryRequest( | ||
| @Parameter( | ||
| name = "status", in = ParameterIn.QUERY, | ||
| description = "조회할 예약 상태", | ||
| name = "viewType", in = ParameterIn.QUERY, | ||
| description = "조회할 예약 내역 타입", | ||
| required = true, | ||
| schema = @Schema( | ||
| type = "string", | ||
| example = "예약 대기", | ||
| allowableValues = { | ||
| "예약 대기", | ||
| "예약 확정 완료", | ||
| "예약 확정 요청", | ||
| "예약 취소 완료", | ||
| "예약 취소 요청" | ||
| "진행 예정", | ||
| "확정 신청", | ||
| "완료 내역", | ||
| "취소 내역" | ||
| } | ||
| ) | ||
| ) | ||
| @NotNull(message = "예약 상태는 필수입니다.") | ||
| ReservationStatus status | ||
| @NotNull(message = "예약 내역 타입은 필수입니다.") | ||
| ReservationViewType viewType |
| "AND r.reservationStatus IN :statuses " + | ||
| "AND r.reservationId < :lastCursor " + | ||
| "ORDER BY r.reservationId DESC") | ||
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | ||
| @Param("ownerId") Long ownerId, | ||
| @Param("status") ReservationStatus status, | ||
| @Param("statuses") Set<ReservationStatus> statuses, |
| private static final Map<ReservationViewType, Set<ReservationStatus>> STATUS_GROUPS; | ||
|
|
||
| static { | ||
| Map<ReservationViewType, Set<ReservationStatus>> viewTypeToStatus = new EnumMap<>(ReservationViewType.class); |
| @Configuration | ||
| public class AwsConfig { | ||
|
|
||
| @Value("${cloud.aws.credentials.access-key}") | ||
| private String accessKey; | ||
|
|
||
| @Value("${cloud.aws.credentials.secret-key}") | ||
| private String secretKey; | ||
|
|
||
| @Value("${cloud.aws.region.static}") | ||
| private String region; | ||
|
|
||
| @Bean | ||
| public S3Client s3Client() { | ||
| return S3Client.builder() | ||
| .region(Region.of(region)) | ||
| .credentialsProvider( | ||
| StaticCredentialsProvider.create( | ||
| AwsBasicCredentials.create(accessKey, secretKey) |
There was a problem hiding this comment.
이건 그냥 팁인데, awssdk 를 활용하면 이렇게 accessKey, secretKey 를 직접 @value로 명시하고, AwsBasicCredentials.create() 과 같은 작업을 안해도 자동으로 S3Client 를 만들어주기도 합니다!
implementation platform('software.amazon.awssdk:bom:2.25.8')
implementation 'software.amazon.awssdk:s3'@Configuration
public class S3Config {
@Bean
public S3Client s3Client() {
return S3Client.builder()
.region(Region.AP_NORTHEAST_2)
.credentialsProvider(EnvironmentVariableCredentialsProvider.create())
.build();
}
}cloud:
aws:
region:
static: ap-northeast-2 # 서울 리전
credentials:
access-key: ${AWS_ACCESS_KEY_ID}
secret-key: ${AWS_SECRET_ACCESS_KEY}
s3:
bucket: ${S3_BUCKET_NAME}
base-url: ${S3_BASE_URL}대신 이 방식을 활용하면 환경변수명을 반드시 AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY 로 설정해야하긴 하더라구요
쨌든 이러한 방식도 있다는 걸 알고계시면 좋을 것 같습니다~
There was a problem hiding this comment.
오 어쩐지 자꾸 Spring Boot가 default로 만들어주는 빈과 제가 수동으로 직접 만드는 빈 두개가 중복되게 만들어진다고 에러가 떠서 현재 yml 파일에 s3autoconfiguration을 배제해둔 상태인데 상균님이 알려주신 방식대로 하면 충돌이 없겠네요. 이참에 그냥 수정해보겠습니다.
| @Bean | ||
| public S3Presigner s3Presigner() { | ||
| return S3Presigner.builder() | ||
| .region(Region.of(region)) | ||
| .credentialsProvider( | ||
| StaticCredentialsProvider.create( | ||
| AwsBasicCredentials.create(accessKey, secretKey))) | ||
| .build(); | ||
| } | ||
| } No newline at end of file |
| public String extractKeyFromUrl(String url) { | ||
| if (url == null) return null; | ||
| String base = trimRightSlash(cdnBaseUrl) + "/"; | ||
| if (url.startsWith(base)) { | ||
| return url.substring(base.length()); | ||
| } | ||
| // 다른 형태면 그대로 키 파싱 불가 → null | ||
| return null; | ||
| } |
There was a problem hiding this comment.
cdn url 로부터 object key 를 추출하는 메서드인 것 같네요!
있으면 좋겠다라고 생각중이었는데 딱 구현해주셨군요 아주 굿입니다
네넵 예약 상태의 흐름을 보면, 예약 대기 -> 예약 확정 요청 -> 예약 확정 -> 예약 취소 요청 -> 예약 취소 흐름인데 예약 견적서 수정은 예약 대기, 예약 확정 요청 상태일 때만 가능하고, 예약 견적서 pdf가 만들어지는 것은 예약 확정 이후이기 때문에 예약 견적서 작성/수정 api 호출시에는 pdf가 없다고 생각해도 무방할 것 같습니다!! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (3)
33-35: pdfUrl에 public Setter 노출은 도메인 규칙을 약화합니다 — 접근 제한 또는 도메인 메서드로 한정해 주세요.외부 계층에서 임의 상태에서 pdfUrl을 변경할 수 있어(예: PENDING/CANCELLED에서도 주입), 상태 기반 불변식이 흔들릴 수 있습니다. 최소한 패키지 스코프로 제한하거나, 도메인 메서드(attach/clear)로만 수정되도록 하는 편이 안전합니다.
다음과 같이 접근 제한을 낮추는 것을 제안합니다:
- @Setter + @Setter(AccessLevel.PACKAGE)또는 Setter를 제거하고 도메인 메서드를 도입하세요(참고용):
public void attachPdfUrl(String pdfUrl) { if (this.reservationStatus != ReservationStatus.CONFIRMED) { throw new DomainRuleException(CANNOT_UPDATE_RESERVATION, new IllegalArgumentException("예약 확정 상태에서만 PDF를 첨부할 수 있습니다.")); } this.pdfUrl = pdfUrl; } public void clearPdfUrl() { this.pdfUrl = null; }
157-162: 검증 로직을 EnumSet/enum 메서드로 위임해 가독성과 확장성 개선 제안상태가 추가될 경우 분기 중복이 늘 수 있습니다. EnumSet 또는 enum에 정책 메서드(isUpdatable 등)로 위임하면 변경 내성이 좋아집니다.
간단 대안:
- if (!(this.reservationStatus == ReservationStatus.CONFIRMED_REQUESTED || this.reservationStatus == ReservationStatus.PENDING)) { + if (!java.util.EnumSet.of(ReservationStatus.CONFIRMED_REQUESTED, ReservationStatus.PENDING) + .contains(this.reservationStatus)) { throw new DomainRuleException(CANNOT_UPDATE_RESERVATION, new IllegalArgumentException("예약 상태가 '예약 대기' 또는 '예약 확정 요청' 상태일 때만 예약 정보를 수정할 수 있습니다.")); }또는 ReservationStatus에
isUpdatable()추가 후if (!this.reservationStatus.isUpdatable())로 위임하는 방식 권장.
167-169: 이중부정 네이밍으로 오해 위험 + 취소 확정 시 pdfUrl 정합성 처리 확인 필요
- isInValidStatusTransitionFrom(newStatus)의 의미가 직관적이지 않습니다. 실제 불리언 의미(유효/무효)를 재확인하고,
canTransitionTo/cannotTransitionTo등으로 명확화하는 것을 권장합니다.- 도메인 정책상 취소 확정(CANCELLED) 시 pdfUrl을 필드 차원에서도 비우는지 확인 필요합니다. 응답에서만 null 마스킹하면 다른 조회/인덱싱 경로에서 노출될 수 있습니다. 도메인에서 함께 정리하는 옵션을 제안드립니다.
취소 확정 시 정합성 강화를 위한 예시:
public void updateStatus(ReservationStatus newStatus) { // status 순서가 올바른지 검증 (PENDING -> CONFIRMED_REQUESTED -> CONFIRMED -> CANCELLED_REQUESTED -> CANCELLED) if (this.reservationStatus.isInValidStatusTransitionFrom(newStatus)) { throw new DomainRuleException(INVALID_RESERVATION_STATUS_TRANSITION, new IllegalArgumentException("예약 상태는 '예약 대기' -> '예약 확정 요청' -> '예약 확정' -> '예약 취소 요청' -> '예약 취소' 순서로만 변경할 수 있습니다." + " 현재 상태: " + this.reservationStatus + ", 변경하려는 상태: " + newStatus)); } this.reservationStatus = newStatus; + if (newStatus == ReservationStatus.CANCELLED) { + this.pdfUrl = null; // 취소 확정 시 파일 URL 정리 + } }질문:
- 현재 구현은 “취소 확정된 예약 조회 시 pdfUrl = null로 반환”을 DTO 레벨 마스킹으로 처리하나요, 아니면 실제 필드 값을 null로 정리하나요? 저장/노출 정책을 명확히 합의해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
build.gradle(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java(3 hunks)src/main/java/konkuk/chacall/global/config/AwsConfig.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)src/test/resources/application-test.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/global/config/AwsConfig.java
🔇 Additional comments (3)
src/main/resources/application-dev.yml (1)
76-96: dev 프로파일 S3/PDF 설정 추가가 요구사항과 잘 부합합니다.새로 도입된 업로드·PDF 서비스에서 기대하는 프로퍼티들이 모두 정의되어 있어, 이전에 지적됐던 누락 문제가 해소되었습니다.
src/main/resources/application-local.yml (1)
77-96: 로컬 프로필 S3/PDF 설정 추가 확인 완료
환경변수 바인딩만으로 자격 증명을 주입하도록 정리되어 있어 로컬 환경에서도 요구사항을 무리 없이 충족할 것으로 보입니다.src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
144-144: 업데이트 이전 상태 검증 추가 👍update(...) 시작 시 상태 검증을 선행하는 흐름이 명확하고 안전합니다.
| // S3 | ||
| implementation platform('software.amazon.awssdk:bom:2.25.8') | ||
| implementation 'software.amazon.awssdk:s3' | ||
|
|
There was a problem hiding this comment.
S3 동기 클라이언트를 위한 HTTP 클라이언트 의존성이 누락되어 있습니다
AWS SDK v2는 HTTP 클라이언트를 분리해두기 때문에 S3Client.create() 등 동기 클라이언트를 사용할 때 software.amazon.awssdk:url-connection-client(또는 다른 HTTP 클라이언트 모듈)을 클래스패스에 추가하지 않으면 실행 시 NoClassDefFoundError/IllegalStateException이 발생합니다. 함께 추가해주세요.
// S3
implementation platform('software.amazon.awssdk:bom:2.25.8')
implementation 'software.amazon.awssdk:s3'
+ implementation 'software.amazon.awssdk:url-connection-client'🤖 Prompt for AI Agents
In build.gradle around lines 67 to 70, the AWS SDK v2 S3 synchronous client is
missing a runtime HTTP client dependency; add the URL connection client
(implementation 'software.amazon.awssdk:url-connection-client') or another
supported HTTP client implementation to the same dependencies block so
S3Client.create() has an HTTP provider on the classpath (BOM manages the
version).
| bucket: ${S3_BUCKET_NAME} | ||
| cloud-front-url: ${CLOUD_FRONT_URL} | ||
| credentials: | ||
| access-key: ${AWS_ACCESS_KEY_ID} | ||
| secret-key: ${AWS_SECRET_ACCESS_KEY} | ||
| region: |
There was a problem hiding this comment.
테스트 프로파일에 기본값이 없어 실행이 즉시 실패함
${…}에 기본값이 없어서 CI/로컬에서 환경변수를 안 주면 애플리케이션이 부팅 단계에서 바로 터집니다. 테스트용 프로파일에서는 더미 값을 넣어 두고, 실제 값은 필요 시 별도 환경변수로 덮어쓰도록 해주세요.
bucket: ${S3_BUCKET_NAME}
cloud-front-url: ${CLOUD_FRONT_URL}
credentials:
- access-key: ${AWS_ACCESS_KEY_ID}
- secret-key: ${AWS_SECRET_ACCESS_KEY}
+ access-key: ${AWS_ACCESS_KEY_ID:test}
+ secret-key: ${AWS_SECRET_ACCESS_KEY:test}- bucket: ${S3_BUCKET_NAME}
- cloud-front-url: ${CLOUD_FRONT_URL}
+ bucket: ${S3_BUCKET_NAME:dummy-bucket}
+ cloud-front-url: ${CLOUD_FRONT_URL:http://localhost:8080}필요하면 ap-northeast-2도 ${AWS_REGION:ap-northeast-2}처럼 맞춰 주세요.
📝 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.
| bucket: ${S3_BUCKET_NAME} | |
| cloud-front-url: ${CLOUD_FRONT_URL} | |
| credentials: | |
| access-key: ${AWS_ACCESS_KEY_ID} | |
| secret-key: ${AWS_SECRET_ACCESS_KEY} | |
| region: | |
| bucket: ${S3_BUCKET_NAME:dummy-bucket} | |
| cloud-front-url: ${CLOUD_FRONT_URL:http://localhost:8080} | |
| credentials: | |
| access-key: ${AWS_ACCESS_KEY_ID:test} | |
| secret-key: ${AWS_SECRET_ACCESS_KEY:test} | |
| region: |
🤖 Prompt for AI Agents
In src/test/resources/application-test.yml around lines 76 to 81, the YAML uses
environment placeholders without defaults causing startup failures when CI/local
env vars are missing; update the placeholders to provide safe test defaults
(e.g. set bucket to a dummy name, cloud-front-url to a harmless URL, credentials
to dummy access/secret values) and give region a default like ap-northeast-2 so
tests boot even without env vars; ensure the syntax uses the ${ENV:default} form
so actual environment variables still override these test defaults.
2 participants
#️⃣연관된 이슈
closes #28
📝작업 내용
크게 3가지 작업을 진행했습니다.
/reservations/[pdf가 생성된 timestamp]/reservationId/estimate.pdf/reservations/20250924-023818/3/estimate.pdf)스크린샷 (선택)
PDF 견적서 (예시)
예약 목록 조회
💬리뷰 요구사항(선택)
Summary by CodeRabbit