[FEAT] 사장님 - 예약내역 목록 및 상세 조회 API 구현#15
Conversation
- status(PENDING/CONFIRMED/CANCELLED) 에 따라 진행예정/확정/취소 상태의 예약을 하나의 API로 처리
Walkthrough사장님 대상 예약 목록(커서 페이징) 및 상세 조회 API를 추가했다. 컨트롤러·서비스·레포지토리·도메인(Reservation, DateRange 등)·DTO·페이징 유틸·에러코드·스웨거·WebConfig 등록을 포함한 관련 구현을 도입했다. Changes
Sequence Diagram(s)sequenceDiagram
actor Owner as 사장님
participant C as OwnerController
participant S as OwnerService
participant V as OwnerValidator
participant RS as OwnerReservationService
participant RR as ReservationRepository
participant UR as UserRepository
Owner->>C: GET /owners/me/reservations?status=&cursor=&size=
C->>S: getOwnerReservations(request, ownerId)
S->>V: validateOwner(ownerId)
V-->>S: OK
S->>RS: getOwnerReservations(ownerId, status, lastCursor, pageSize)
RS->>RR: findOwnerReservationsByStatusWithCursor(...)
RR-->>RS: Slice<Reservation>
RS->>UR: findAllByUserIdInAndRoleAndStatus(...)
UR-->>RS: List<User>
RS-->>S: CursorPagingResponse<OwnerReservationHistoryResponse>
S-->>C: BaseResponse(200, payload)
C-->>Owner: 200 OK
sequenceDiagram
actor Owner as 사장님
participant C as OwnerController
participant S as OwnerService
participant V as OwnerValidator
participant RS as OwnerReservationService
participant RR as ReservationRepository
Owner->>C: GET /owners/me/reservations/{reservationId}
C->>S: getReservationDetail(ownerId, reservationId)
S->>V: validateOwner(ownerId)
V-->>S: OK
S->>RS: getReservationDetail(ownerId, reservationId)
RS->>RR: findByIdWithDetails(reservationId)
alt found
RR-->>RS: Reservation(+member,+foodTruck,+owner)
RS-->>S: OwnerReservationDetailResponse
S-->>C: BaseResponse(200, payload)
C-->>Owner: 200 OK
else not found
RR-->>RS: empty
RS-->>S: throw EntityNotFound(RESERVATION_NOT_FOUND)
S-->>C: ErrorResponse(404)
C-->>Owner: 404 Not Found
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java (1)
60-66: 오너 예약 응답 설명 상수 추가 LGTM. Set 불변화로 후속 변이 방지 제안.생성자에서 공통 에러를 addAll로 주입하므로, 외부에서 참조해 변이되면 문서 일관성이 깨질 수 있습니다. 불변 Set으로 래핑을 권장합니다.
생성자 예시(참고 코드):
import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; SwaggerResponseDescription(Set<ErrorCode> errorCodeList) { Set<ErrorCode> s = new LinkedHashSet<>(errorCodeList); s.addAll(Set.of( API_NOT_FOUND, API_METHOD_NOT_ALLOWED, API_SERVER_ERROR, API_MISSING_PARAM, API_INVALID_PARAM, API_INVALID_TYPE )); this.errorCodeList = Collections.unmodifiableSet(s); }src/main/java/konkuk/chacall/global/common/dto/CursorPagingResponse.java (1)
5-12: 필드 네이밍 혼선: lastCursor 보다는 nextCursor가 의미에 부합합니다.서비스에서 “다음 페이지 요청용 커서”를 반환한다면 명확히 nextCursor로 이름을 맞추는 편이 실수를 줄입니다. 또한 PR 요약(“isLastPage”)과 코드(hasNext)가 불일치하니 용어를 통일해 주세요.
제안 diff:
-public record CursorPagingResponse<T>( - List<T> content, - Long lastCursor, - boolean hasNext -) { - public static <T> CursorPagingResponse<T> of(List<T> content, Long lastCursor, boolean hasNext) { - return new CursorPagingResponse<>(content, lastCursor, hasNext); +public record CursorPagingResponse<T>( + List<T> content, + Long nextCursor, + boolean hasNext +) { + public static <T> CursorPagingResponse<T> of(List<T> content, Long nextCursor, boolean hasNext) { + return new CursorPagingResponse<>(content, nextCursor, hasNext); } }src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java (1)
20-21: IN 절에 빈 리스트 전달 시 DB 벤더에 따라 SQL 오류/빈 결과 혼선 가능. 호출부 가드 권장.
findAllByUserIdIn...는userIds가 비면 일부 DB에서IN ()구문 오류가 납니다. 서비스에서 빈 리스트면 조기 반환하도록 가드해 주세요.src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java (1)
41-45: NPE 방지용 null 가드 추가 제안.초기화 이전 엔터티 사용 등에서
reservationDate가 null일 수 있습니다.- public List<String> getFormattedDateTimeInfos() { - return this.reservationDate.getDates().stream() - .map(date -> date.toString() + " " + this.operationHour) - .toList(); - } + public List<String> getFormattedDateTimeInfos() { + if (this.reservationDate == null || this.reservationDate.getDates() == null) { + return List.of(); + } + return this.reservationDate.getDates().stream() + .map(date -> date.toString() + " " + this.operationHour) + .toList(); + }src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationHistoryResponse.java (1)
23-33: 중복 접근 최소화를 위한 소폭 정리 제안(가독성).같은 값 접근을 로컬 변수로 묶으면 읽기 쉬워집니다.
- public static OwnerReservationHistoryResponse of(Reservation reservation, User member) { - List<String> dateTimeList = reservation.getReservationInfo().getFormattedDateTimeInfos(); - - return new OwnerReservationHistoryResponse( - reservation.getReservationId(), - member.getProfileImageUrl(), - member.getName(), - reservation.getReservationInfo().getReservationAddress(), - dateTimeList - ); - } + public static OwnerReservationHistoryResponse of(Reservation reservation, User member) { + var info = reservation.getReservationInfo(); + List<String> dateTimeList = info.getFormattedDateTimeInfos(); + return new OwnerReservationHistoryResponse( + reservation.getReservationId(), + member.getProfileImageUrl(), + member.getName(), + info.getReservationAddress(), + dateTimeList + ); + }src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
95-101: 페이지 크기 상한 미적용 — size 과다 요청 방지 필요size 파라미터에 상한이 없어 대용량 응답/메모리 사용 위험이 있습니다. 서버 단에서 상한을 적용하거나 DTO에 @max를 추가해 방어적으로 제한하세요.
간단한 상한 적용 예시:
- return ownerReservationService.getOwnerReservations(ownerId, request.status(), request.getCursorOrDefault(), request.getPageSizeOrDefault()); + int pageSize = Math.min(100, request.getPageSizeOrDefault()); + return ownerReservationService.getOwnerReservations(ownerId, request.status(), request.getCursorOrDefault(), pageSize);src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationDetailResponse.java (3)
29-31: 금액을 문자열로 반환하지 말고 수치형으로 유지"원"을 붙인 문자열은 정렬/통계/다국어 처리에 불리합니다. 금액은 수치형(Integer/Long/BigDecimal)으로 제공하고, 표시 형식은 클라이언트에 위임하세요.
- @Schema(description = "지불된 예약금액", - example = "50000원") - String deposit, + @Schema(description = "예약금(원 단위, 숫자)", + example = "50000") + Integer deposit, ... - reservation.getReservationInfo().getReservationDeposit() + "원", + reservation.getReservationInfo().getReservationDeposit(),Also applies to: 53-53
32-35: 전기 사용 여부는 불리언 필드로 제공"가능/불가능" 문자열은 i18n과 소비자 로직에 불리합니다. 불리언(useElectricity)을 직접 반환하고, 필요 시 클라이언트에서 라벨링하세요.
- @Schema(description = "전기 사용 가능 여부", - example = "가능") - String electricityInfo, + @Schema(description = "전기 사용 가능 여부", + example = "true") + boolean useElectricity, ... - String electricity = reservation.getReservationInfo().isUseElectricity() ? "가능" : "불가능"; ... - electricity, + reservation.getReservationInfo().isUseElectricity(),Also applies to: 44-44
10-16: nullable 스키마 명시 필요(프로필/이름)프로필 이미지/이름은 도메인에 따라 null일 수 있습니다. API 스키마에 nullable=true를 명시하거나, 널을 허용하지 않으려면 기본값/대체 문자열을 제공하세요.
- @Schema(description = "상대방(손님)의 프로필 이미지 URL", - example = "https://image.url/path/profile.jpg") + @Schema(description = "상대방(손님)의 프로필 이미지 URL", + example = "https://image.url/path/profile.jpg", + nullable = true) String userProfileImage, - @Schema(description = "상대방의 이름 또는 닉네임", - example = "김차콜") + @Schema(description = "상대방의 이름 또는 닉네임", + example = "김차콜", + nullable = true) String username,Also applies to: 47-48
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (1)
21-27: size 상한 추가(@max) 권장과도한 size 요청을 방지하기 위해 상한을 명시해 주세요(예: 100). 스키마에도 maximum을 반영하면 문서/검증이 일치합니다.
import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Max; ... @Schema(description = "한 페이지에 조회할 개수", example = "30", defaultValue = "20", minimum = "1", + maximum = "100", nullable = true) - @Min(value = 1, message = "size 는 1 이상이어야 합니다.") + @Min(value = 1, message = "size 는 1 이상이어야 합니다.") + @Max(value = 100, message = "size 는 100 이하이어야 합니다.") Integer sizesrc/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
15-17: 엔티티에 @Builder/@AllArgsConstructor 사용 시 주의JPA 엔티티에 빌더/전체 생성자를 노출하면 불완전/불변 위반 상태로 생성될 수 있습니다. 정적 팩토리(필수 필드 강제)로의 전환을 고려하세요.
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
36-47: 빈 페이지 단락 처리로 불필요한 조회 제거결과가 비어있을 때 userRepository 호출과 매핑을 건너뛰면 쿼리 1회를 줄일 수 있습니다.
List<Reservation> ownerReservationList = ownerReservationSlice.getContent(); + if (ownerReservationList.isEmpty()) { + return CursorPagingResponse.of(List.of(), null, false); + } + // 손님 ID 리스트로 User 정보 한 번에 조회 Map<Long, User> customerMap = getCustomerMap(ownerReservationList);
69-76: getCustomerMap 최적화: 빈 목록 처리 + 중복 ID 제거빈 목록 방어와 distinct로 IN 절/메모리 사용을 줄이세요. 빈 IN() 방지도 됩니다.
private Map<Long, User> getCustomerMap(List<Reservation> reservations) { - List<Long> customerIds = reservations.stream() - .map(reservation -> reservation.getMember().getUserId()) - .toList(); + if (reservations.isEmpty()) { + return Map.of(); + } + List<Long> customerIds = reservations.stream() + .map(reservation -> reservation.getMember().getUserId()) + .distinct() + .toList(); return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream() .collect(Collectors.toMap(User::getUserId, user -> user)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(3 hunks)src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java(3 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(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationHistoryResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java(2 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java(2 hunks)src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java(2 hunks)src/main/java/konkuk/chacall/global/common/dto/CursorPagingResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(2 hunks)src/main/java/konkuk/chacall/global/config/WebConfig.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(19-110)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (2)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(19-110)src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
RequiredArgsConstructor(25-89)
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java (1)
Getter(14-46)
🔇 Additional comments (11)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java (1)
24-24: 타입 변경(ReservationDateList) 적합. 마이그레이션/컨버터 null 처리만 재확인 요청.DB 기존 레코드에 값 누락이 있을 경우 NPE 가능성이 있으니
ReservationDateListConverter에서 null ↔ 빈 목록 변환이 안전한지 확인해 주세요.src/main/java/konkuk/chacall/global/config/WebConfig.java (1)
17-20: ArgumentResolver 등록 LGTM.스프링 MVC 확장 포인트 사용이 적절합니다. 추가 리졸버와의 우선순위 이슈가 생기면 Ordering만 고려해 주세요.
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (1)
27-32: FETCH JOIN 기반 상세 조회 LGTM.모두 To-One 경로라 중복 행 위험이 낮습니다. 향후 To-Many가 추가되면 DISTINCT 고려가 필요합니다.
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
48-54: 90001(RESERVATION_NOT_FOUND) 중복 없음 — 정의 및 사용처만 확인됨발견 위치:
- src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java:53 — 정의
- src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java:52 — 사용
- src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java:65 — 사용
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/OwnerReservationDetailResponse.java (1)
41-45: 최대 2개 일정 보장 여부 확인설명에는 최대 2개라고 되어 있으나, 코드 상 하한/상한 체크가 없습니다. 상위 도메인 로직(ReservationInfo/예약 생성 로직)에서 2개 제한이 보장되는지 확인 바랍니다.
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (2)
146-158: LGTM — 무한 스크롤 조회 엔드포인트 구조 적절@ParameterObject + @Valid 바인딩과 CursorPagingResponse 래핑이 명확합니다. 서비스로의 위임도 적절합니다.
164-164: 잘못된 경로 매핑 — 선행 슬래시 누락현재 "me/reservations/{reservationId}"로 선언되어 클래스 매핑("/owners")과 결합 시 "/ownersme/..."가 됩니다. 선행 슬래시를 추가해야 합니다.
- @GetMapping("me/reservations/{reservationId}") + @GetMapping("/me/reservations/{reservationId}")Likely an incorrect or invalid review comment.
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (1)
39-45: 확인 — cursor 기본값(Long.MAX_VALUE) 전략이 Repository 쿼리와 일치ReservationRepository의 쿼리가 'r.reservationId < :lastCursor' 및 'ORDER BY r.reservationId DESC'를 사용하므로 getCursorOrDefault()의 기본값(Long.MAX_VALUE)은 적절합니다.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
33-41: DB 마이그레이션·인덱스 확인 필요파일: src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (33–41)
- user_id, food_truck_id가 nullable=false이므로 운영 DB에 NOT NULL 및 FK 제약 존재 여부 확인.
- 인덱스 필요: reservations(user_id), reservations(food_truck_id) — owner-scope 조회가 빈번하므로 food_truck_id 인덱스 필수.
- 기존 데이터에 NULL 값 존재 여부 확인(백필 필요 시 마이그레이션 준비).
검증 보조: 제출한 스크립트가 "No files were searched"로 실패했습니다. 마이그레이션 파일(V*.sql 또는 Liquibase/Flyway changelog)이나 DDL을 첨부하거나, 로컬에서 아래 명령을 실행한 결과를 붙여넣기:
rg -n --hidden --glob '!/node_modules/' -i "(reservations).?(user_id|food_truck_id).?(foreign key|not null|index)|V..*.sql|db/migration|changelog|liquibase|flyway" -C2src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
61-64: 커서 페이지네이션 정렬 미지정 — id DESC로 정렬 명시 필요위치: src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (lines 61–64)
정렬 미지정 시 DB 기본 정렬에 의존해 nextCursor 계산이 일관되지 않을 수 있음.권장 변경:
- .findOwnerReservationsByStatusWithCursor(ownerId, status, lastCursor, PageRequest.of(0, pageSize)); + .findOwnerReservationsByStatusWithCursor( + ownerId, + status, + lastCursor, + PageRequest.of(0, pageSize, org.springframework.data.domain.Sort.by(org.springframework.data.domain.Sort.Direction.DESC, "id")) + );리포지토리 쿼리도 아래 조건 충족 확인:
- ORDER BY id DESC
- lastCursor 존재 시 WHERE id < :lastCursor (id DESC와 일치하는 커서 필터)
현재 자동 검증 불가(저장소에서 파일/쿼리 미발견) — 위 변경 적용 후 리포지토리 쿼리와 통합 테스트로 커서 일관성 검증 필요.
41-43: 검토 완료 — DTO 매핑(N+1) 위험 없음
- ReservationInfo.getFormattedDateTimeInfos()는 Embeddable(ReservationInfo) + ReservationDateListConverter로 값 타입(문자열 → List)을 반환하므로 추가 엔티티 조회 쿼리가 발생하지 않습니다.
- 고객 조회는 getCustomerMap()에서 customerIds로 userRepository.findAllByUserIdInAndRoleAndStatus(...)로 한 번에 배치 조회하므로 mapToReservationHistory()에서 사용자 관련 N+1이 발생하지 않습니다.
- 상세 조회는 reservationRepository.findByIdWithDetails(...)를 사용하고 있어 연관 데이터 조회 경로가 분리되어 있습니다.
| public OwnerReservationDetailResponse getReservationDetail(Long reservationId) { | ||
| // ID로 예약 정보와 연관된 모든 데이터 한 번에 조회 | ||
| Reservation reservation = reservationRepository.findByIdWithDetails(reservationId) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND)); | ||
|
|
||
| // DTO 로 변환하여 반환 | ||
| return OwnerReservationDetailResponse.of(reservation, reservation.getMember()); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
상세 조회: 소유자 검증 누락(블로커) — reservationId만으로 접근 가능
ownerId를 인자로 받지 않아 다른 사장님의 예약 상세를 열람할 수 있습니다. 서비스/리포지토리에서 ownerId 조건을 포함해 권한을 강제하세요. (대안: 조회 후 ownerId 불일치 시 404/403 처리)
아래처럼 시그니처/쿼리 메서드 사용을 수정 제안:
- public OwnerReservationDetailResponse getReservationDetail(Long reservationId) {
- // ID로 예약 정보와 연관된 모든 데이터 한 번에 조회
- Reservation reservation = reservationRepository.findByIdWithDetails(reservationId)
- .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND));
+ public OwnerReservationDetailResponse getReservationDetail(Long ownerId, Long reservationId) {
+ // ID + 소유자 조건으로 단건 조회 (권한 보호)
+ Reservation reservation = reservationRepository.findByIdWithDetailsAndOwner(reservationId, ownerId)
+ .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND));
// DTO 로 변환하여 반환
return OwnerReservationDetailResponse.of(reservation, reservation.getMember());
}OwnerService 호출부도 함께 변경 필요:
- return ownerReservationService.getReservationDetail(reservationId);
+ return ownerReservationService.getReservationDetail(ownerId, reservationId);검증 스크립트:
🏁 Script executed:
#!/bin/bash
# getReservationDetail 호출부 전수 확인
rg -nP '\bgetReservationDetail\s*\(' -C2Length of output: 2960
권한 검증 누락(치명적) — OwnerReservationService에서 ownerId 미전달
OwnerService가 ownerId로 사장 검증(ownerValidator.validateAndGetOwner)을 수행한 뒤 내부에서 ownerReservationService.getReservationDetail(reservationId)만 호출해 ownerId가 전달되지 않습니다. 결과적으로 다른 소유자의 예약 상세를 조회할 수 있습니다.
영향 위치:
- src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (getReservationDetail, ~49–56)
- src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (getReservationDetail, ~103–108)
수정 제안(권장 — repository 레벨에서 owner 필터 적용):
- public OwnerReservationDetailResponse getReservationDetail(Long reservationId) {
- Reservation reservation = reservationRepository.findByIdWithDetails(reservationId)
- .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND));
+ public OwnerReservationDetailResponse getReservationDetail(Long ownerId, Long reservationId) {
+ Reservation reservation = reservationRepository.findByIdWithDetailsAndOwner(reservationId, ownerId)
+ .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND));
return OwnerReservationDetailResponse.of(reservation, reservation.getMember());
}OwnerService 호출 수정:
- return ownerReservationService.getReservationDetail(reservationId);
+ return ownerReservationService.getReservationDetail(ownerId, reservationId);대안(임시): repository 변경이 어렵다면 기존 조회 후 reservation의 ownerId와 비교해 불일치 시 404/403 반환하라.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java
around lines 49–56, the method getReservationDetail(reservationId) does not
verify the owner and allows other owners' reservations to be viewed; change the
API to accept the ownerId (or principal owner) and query the repository using a
method that filters by reservationId and ownerId (e.g.,
findByIdWithDetailsAndOwnerId) or, if repository change isn't possible, after
loading the reservation compare reservation.getOwnerId() to the supplied ownerId
and throw EntityNotFoundException or an authorization exception on mismatch;
also update the caller in
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (around
lines 103–108) to pass the validated ownerId into
OwnerReservationService.getReservationDetail.
| return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream() | ||
| .collect(Collectors.toMap(User::getUserId, user -> user)); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
비활성/탈퇴 회원 제외로 인한 누락/NPE 가능성
BaseStatus.ACTIVE로 제한하면 과거 예약의 탈퇴/정지 회원이 매핑되지 않아 customerMap.get(...)이 null이 될 수 있습니다. 현재 mapToReservationHistory는 null 대비가 없어 NPE/불완전 응답 위험이 있습니다.
대안:
- 이력 화면 특성상 상태 필터를 제거(또는 허용 상태 다중 허용)해 모든 관련 User를 로드.
- 또는
mapToReservationHistory에서 null 시 “탈퇴 회원” 등 마스킹 처리.
가능하다면 아래처럼 상태 필터를 제거한 리포지토리 메서드를 사용(메서드가 없다면 추가):
- return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream()
+ return userRepository.findAllByUserIdInAndRole(customerIds, Role.MEMBER).stream()
.collect(Collectors.toMap(User::getUserId, user -> user));🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
TARGET="src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java"
if [ -f "$TARGET" ]; then
echo "=== SHOW: $TARGET ==="
sed -n '1,240p' "$TARGET"
else
echo "=== $TARGET not found; searching for OwnerReservationService ==="
rg -n --hidden --no-ignore 'OwnerReservationService' -S || true
fi
echo
echo "=== SEARCH: mapToReservationHistory ==="
rg -n --hidden --no-ignore 'mapToReservationHistory' -S -C3 || true
echo
echo "=== SEARCH: repository query methods (findAllByUserIdInAndRoleAndStatus / findAllByUserIdInAndRole) ==="
rg -n --hidden --no-ignore 'findAllByUserIdInAndRoleAndStatus|findAllByUserIdInAndRole' -S -C3 || true
echo
echo "=== SEARCH: UserRepository interface / definitions ==="
rg -n --hidden --no-ignore 'interface .*UserRepository|class .*UserRepository' -S -C3 || true
rg -n --hidden --no-ignore 'findAllByUserIdInAndRole' -S -C3 || true
echo
echo "=== SEARCH: customerMap usages ==="
rg -n --hidden --no-ignore 'customerMap\.get\(|customerMap\[' -S -C3 || trueLength of output: 11603
NPE 위험 — 비활성/탈퇴 회원 누락으로 인한 NullPointerException 가능
검증: OwnerReservationService#getCustomerMap가 BaseStatus.ACTIVE로 User를 필터링하고 있어(파일: src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java, lines ~71–76) 탈퇴/정지 회원이 누락되면 mapToReservationHistory에서 customerMap.get(...)이 null이 되어 NPE가 발생할 수 있음. UserRepository에는 findAllByUserIdInAndRoleAndStatus(...)만 선언되어 있으며, findAllByUserIdInAndRole(...)는 존재하지 않음 (src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java).
해결안(둘 중 하나 적용):
- 권장 — 상태 필터 제거: UserRepository에 List findAllByUserIdInAndRole(List, Role) 메서드를 추가하고 getCustomerMap에서 사용.
- return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream()
+ return userRepository.findAllByUserIdInAndRole(customerIds, Role.MEMBER).stream()
.collect(Collectors.toMap(User::getUserId, user -> user));- 대체(빠른 안정화): mapToReservationHistory에서 customer == null일 때 “탈퇴 회원” 등으로 마스킹하거나 OwnerReservationHistoryResponse.of(...)가 null을 안전하게 처리하도록 변경.
조치 필요: 우선적으로 수정 필수.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java
around lines 74–76, getCustomerMap currently filters users by BaseStatus.ACTIVE
which can omit suspended/withdrawn users and lead to NPEs when
mapToReservationHistory does customerMap.get(...) for missing users; fix by
adding a repository method List<User> findAllByUserIdInAndRole(List<Long>
userIds, Role role) to
src/main/java/konkuk/chacall/domain/user/domain/repository/UserRepository.java
and change getCustomerMap to call that method (remove the status filter) so all
users with the role are returned; alternatively for a quick stabilization, make
mapToReservationHistory handle a null customer (e.g., substitute a "withdrawn
user" placeholder or make OwnerReservationHistoryResponse.of(...) accept null
safely) until repository change is implemented.
| private List<OwnerReservationHistoryResponse> mapToReservationHistory(List<Reservation> reservations, Map<Long, User> customerMap) { | ||
| return reservations.stream() | ||
| .map(reservation -> { | ||
| User customer = customerMap.get(reservation.getMember().getUserId()); | ||
| return OwnerReservationHistoryResponse.of(reservation, customer); | ||
| }) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
customer null 처리 없음 — 방어 로직/마스킹 필요
customerMap.get(...) 결과가 null일 때의 처리(마스킹/대체 DTO)가 없습니다. 팩토리 메서드에서 null 허용 또는 별도 ofWithdrawnMember(...) 같은 대체 생성 경로를 두는 것을 권장합니다.
예시(개념):
- null이면 이름: “탈퇴 회원”, 연락처 마스킹 등 기본값으로 DTO 생성
- 또는 사전에 모든 userId 존재 여부를 검증해 누락 시 경고 로그 후 기본값 처리
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java
around lines 81 to 88, the code does not handle a null customer when
customerMap.get(...) returns null; add defensive handling by detecting null and
passing a fallback/masked DTO to OwnerReservationHistoryResponse (either provide
an overloaded factory like ofWithdrawnMember(...) or change the existing factory
to accept null and build a masked response with name "탈퇴 회원" and masked
contact), or validate beforehand and log a warning for any missing userId, then
use the fallback; ensure the mapping returns the fallback response instead of
allowing a null customer to propagate.
| @Query("SELECT r FROM Reservation r " + | ||
| "WHERE r.foodTruck.owner.userId = :ownerId " + | ||
| "AND r.reservationStatus = :status " + | ||
| "AND r.reservationId < :lastCursor " + | ||
| "ORDER BY r.reservationId DESC") | ||
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | ||
| @Param("ownerId") Long ownerId, | ||
| @Param("status") ReservationStatus status, | ||
| @Param("lastCursor") Long lastCursor, | ||
| Pageable pageable); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
첫 페이지에서 lastCursor=null인 경우 결과가 0건이 되는 버그 가능성.
현재 JPQL은 r.reservationId < :lastCursor 조건이므로 lastCursor가 null이면 전부 필터링됩니다. (첫 페이지가 비게 됨)
해결 옵션:
- 선호: 서비스 레벨에서
lastCursor == null이면Long.MAX_VALUE(또는 가장 큰 ID)로 대체. - 대안: JPQL에 null 허용 조건 추가(인덱스 활용은 다소 저하).
JPQL 수정안(간단):
- @Query("SELECT r FROM Reservation r " +
- "WHERE r.foodTruck.owner.userId = :ownerId " +
- "AND r.reservationStatus = :status " +
- "AND r.reservationId < :lastCursor " +
- "ORDER BY r.reservationId DESC")
+ @Query("SELECT r FROM Reservation r " +
+ "WHERE r.foodTruck.owner.userId = :ownerId " +
+ "AND r.reservationStatus = :status " +
+ "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " +
+ "ORDER BY r.reservationId DESC")운영 팁: 본 쿼리 경로에 아래 복합 인덱스를 고려해 주세요.
- (owner_id, reservation_status, reservation_id DESC) 또는 DESC 미지원 DB는 ASC 인덱스 + 조건 보완.
중요: lastCursor가 null이면 첫 페이지가 0건으로 나오는 버그 — 수정 필요
파일: src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java (Lines 15-24)
설명: JPQL의 r.reservationId < :lastCursor는 :lastCursor가 NULL이면 조건이 항상 거짓이 되어 모든 행이 필터링됩니다.
해결 옵션:
- 선호: 서비스 레이어에서
lastCursor == null이면Long.MAX_VALUE(또는 가장 큰 ID)로 대체. - 대안: JPQL에 NULL 허용 조건 추가(인덱스 효율 일부 저하).
- @Query("SELECT r FROM Reservation r " +
- "WHERE r.foodTruck.owner.userId = :ownerId " +
- "AND r.reservationStatus = :status " +
- "AND r.reservationId < :lastCursor " +
- "ORDER BY r.reservationId DESC")
+ @Query("SELECT r FROM Reservation r " +
+ "WHERE r.foodTruck.owner.userId = :ownerId " +
+ "AND r.reservationStatus = :status " +
+ "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " +
+ "ORDER BY r.reservationId DESC")운영 팁: 복합 인덱스 고려 — (owner_id, reservation_status, reservation_id DESC). DESC 미지원 DB는 ASC 인덱스 + 쿼리/조건 보완.
📝 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.reservationId < :lastCursor " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | |
| @Param("ownerId") Long ownerId, | |
| @Param("status") ReservationStatus status, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); | |
| @Query("SELECT r FROM Reservation r " + | |
| "WHERE r.foodTruck.owner.userId = :ownerId " + | |
| "AND r.reservationStatus = :status " + | |
| "AND (:lastCursor IS NULL OR r.reservationId < :lastCursor) " + | |
| "ORDER BY r.reservationId DESC") | |
| Slice<Reservation> findOwnerReservationsByStatusWithCursor( | |
| @Param("ownerId") Long ownerId, | |
| @Param("status") ReservationStatus status, | |
| @Param("lastCursor") Long lastCursor, | |
| Pageable pageable); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java
around lines 15-24, the JPQL predicate r.reservationId < :lastCursor returns no
rows when lastCursor is null; change the calling service to detect lastCursor ==
null and pass Long.MAX_VALUE (or the current max id) as the parameter so the
query behaves like an unbounded upper cursor; alternatively (if you must change
repository), modify the query to allow null by adding OR :lastCursor IS NULL to
the WHERE clause (aware this may reduce index efficiency).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/java/konkuk/chacall/global/common/dto/PagingRequest.java (4)
8-12: cursor 유효성 보강
cursor에 음수가 들어오면 의미가 모호합니다. ID 기반 커서라면 양수만 허용하는 것이 안전합니다. OpenAPI에도 최소값을 명시해 주세요.해당 범위에 적용할 diff:
- @Schema(description = "마지막으로 조회된 데이터의 ID (다음 페이지 요청 시 사용)", - example = "120", - nullable = true) - Long cursor, + @Schema(description = "마지막으로 조회된 데이터의 ID (다음 페이지 요청 시 사용)", + example = "120", + minimum = "1", + nullable = true) + @jakarta.validation.constraints.Positive(message = "cursor 는 1 이상이어야 합니다.") + Long cursor,추가 import(파일 상단에 한 줄 추가):
import jakarta.validation.constraints.Positive;
19-19: DEFAULT_SIZE 구성화 제안상수 20은 무난하지만, 운영 중 튜닝이 필요할 수 있습니다.
@ConfigurationProperties로pagination.default-size/pagination.max-size를 주입받도록 변경을 권장합니다(기본값은 20/100 유지).
25-27: 레코드 관례에 맞춘 메서드 네이밍 소폭 정리 제안레코드 컴포넌트와 일관성을 위해
getSizeOrDefault()/getCursorOrDefault()대신sizeOrDefault()/cursorOrDefault()처럼 접두어 없이 두는 형태가 가독성이 더 좋습니다. 기능 변화는 없습니다.
21-27: 쿼리 전제(정렬: reservationId DESC, 조건: reservationId < cursor) 명시 또는 null 기반 처리로 변경검증: src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java 의 findOwnerReservationsByStatusWithCursor 쿼리가
AND r.reservationId < :lastCursor및ORDER BY r.reservationId DESC를 사용함(확인). PagingRequest.getCursorOrDefault()는 null을 Long.MAX_VALUE로 대체함.
- 권장(우선): 리포지토리 쿼리를
(:lastCursor IS NULL OR r.reservationId < :lastCursor)로 변경하고 서비스/컨트롤러에는 nullable Long을 그대로 전달하도록 변경. (위치: ReservationRepository.java 17–19, PagingRequest.java 21–27)- 대안(유지 시): PagingRequest.getCursorOrDefault 동작과 “정렬: reservationId DESC, 비교: reservationId < cursor” 전제를 JavaDoc/Swagger에 명확히 문서화.
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (2)
17-18: pagingRequest null 시 NPE 가능성 — 널-세이프 헬퍼 추가
pagingRequest가 선택 값이면 서비스에서request.pagingRequest().getSizeOrDefault()같은 접근 시 NPE가 납니다. DTO 내부에 기본 객체를 반환하는 헬퍼를 추가해 주세요.추가 메서드 예시(레코드 바디에 추가):
@Schema(hidden = true) public PagingRequest pagingOrDefault() { return pagingRequest == null ? new PagingRequest(null, null) : pagingRequest; }서비스/리포지토리에서는
request.pagingOrDefault().getSizeOrDefault()처럼 사용.
17-18: PagingRequest 필드에 @Schema 설명 추가 — 컨트롤러는 @ParameterObject 적용됨OwnerController의 getOwnerReservations 파라미터에 이미 @Valid @ParameterObject가 적용되어 있습니다 (src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java:152).
적용할 diff:
- @Valid - PagingRequest pagingRequest + @Valid + @Schema(description = "커서/사이즈 페이징 파라미터 (예: pagingRequest.cursor, pagingRequest.size)") + PagingRequest pagingRequest
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(3 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/PagingRequest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (1)
10-15: 확인 완료 — allowableValues가 ReservationStatus와 일치합니다.
allowableValues {"PENDING","CONFIRMED","CANCELLED"}는 src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationStatus.java의 상수(PENDING, CONFIRMED, CANCELLED)와 1:1 일치하므로 수정 불필요합니다.
| @Schema(description = "한 페이지에 조회할 개수", | ||
| defaultValue = "20", | ||
| minimum = "1") | ||
| @Min(value = 1, message = "size 는 1 이상이어야 합니다.") | ||
| Integer size |
There was a problem hiding this comment.
page size 상한(cap) 부재 — 대량 요청/DoS 리스크
요청자가 size를 매우 크게 넣을 수 있어 DB 부하와 응답 지연을 유발할 수 있습니다. 서버 측에서 상한을 강제하세요. 권장: @max 추가 및 스키마 maximum 명시.
해당 범위에 적용할 diff:
- @Schema(description = "한 페이지에 조회할 개수",
- defaultValue = "20",
- minimum = "1")
- @Min(value = 1, message = "size 는 1 이상이어야 합니다.")
+ @Schema(description = "한 페이지에 조회할 개수",
+ defaultValue = "20",
+ minimum = "1",
+ maximum = "100",
+ nullable = true)
+ @Min(value = 1, message = "size 는 1 이상이어야 합니다.")
+ @jakarta.validation.constraints.Max(value = 100, message = "size 는 100 이하이어야 합니다.")
Integer size추가 import(파일 상단에 한 줄 추가):
import jakarta.validation.constraints.Max;🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/dto/PagingRequest.java around
lines 13 to 17, the page size field has no upper bound which allows excessively
large requests; add a server-side cap by importing
jakarta.validation.constraints.Max at the top of the file and annotate the size
field with @Max (e.g., @Max(value = 100, message = "size 는 100 이하이어야 합니다.")) and
update the @Schema to include maximum = "100" (choose an appropriate cap for
your service) so validation and OpenAPI schema both enforce the limit.
- DateRange 객체를 통해 예약 일정을 startDate, endDate 로 나누어서 관리하도록 구현 - 입력 형식 / 응답 형식은 모두 yyyy.MM.dd 로 통일
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/konkuk/chacall/global/common/converter/ReservationDateListConverter.java (1)
17-19: 주석의 DB 포맷 예시가 실제 구현과 불일치현재 주석은 단일 날짜 리스트 예시(
yyyy-MM-dd, ...)로 표기되어 있으나, 구현은 범위 포맷(start~end의 쉼표 구분)입니다. 문서-코드 불일치를 수정해주세요.-/** - * DB: "yyyy-MM-dd,yyyy-MM-dd,..." <-> 도메인: ReservationDateList - */ +/** + * DB: "yyyy-MM-dd~yyyy-MM-dd,yyyy-MM-dd~yyyy-MM-dd" <-> 도메인: ReservationDateList + */src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationDateList.java (3)
21-21: DB 저장 포맷을 ISO로 통일 권장도메인 내 DOT 포맷 상수는 UI 표현에만 사용하고, DB 저장은 ISO로 고정하는 편이 안전합니다. 아래와 같이 ISO 상수를 추가하고 저장 시 ISO를 사용하도록 변경을 제안합니다.
- private static final DateTimeFormatter DOT = DateTimeFormatter.ofPattern("yyyy.MM.dd"); + private static final DateTimeFormatter DOT = DateTimeFormatter.ofPattern("yyyy.MM.dd"); // UI 표현 + private static final DateTimeFormatter ISO = DateTimeFormatter.ISO_LOCAL_DATE; // DB 저장
54-70: toStorageString는 ISO로 출력하도록 변경DB 내 일관성 및 하위호환(기존 ISO 데이터) 유지를 위해 저장용 포맷을 ISO로 고정합니다.
public String toStorageString() { if (isEmpty()) return ""; StringBuilder sb = new StringBuilder(); for (int i = 0; i < ranges.size(); i++) { DateRange range = ranges.get(i); - sb.append(range.startDate().format(DOT)) + sb.append(range.startDate().format(ISO)) .append("~") - .append(range.endDate().format(DOT)); + .append(range.endDate().format(ISO)); if (i < ranges.size() - 1) sb.append(","); } return sb.toString(); }
33-52: 입력 파서에 DOT→ISO 폴백을 추가하여 견고성 확보FE/운영 데이터 이행 동안 DOT/ISO 혼재 가능성이 있어 파서를 유연하게 만듭니다. 메서드명을 일반화하고 폴백을 추가합니다.
- @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - public static ReservationDateList fromJson(List<String> rawRanges) { + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static ReservationDateList fromJson(List<String> rawRanges) { if (rawRanges == null || rawRanges.isEmpty()) return ReservationDateList.of(List.of()); List<DateRange> list = new ArrayList<>(); for (String range : rawRanges) { if (range == null || range.isBlank()) continue; String[] dates = range.split("~"); if (dates.length != 2) { throw new DomainRuleException(ErrorCode.INVALID_DATE_INPUT); } - LocalDate start = parseDot(dates[0].trim()); - LocalDate end = parseDot(dates[1].trim()); + LocalDate start = parseDate(dates[0].trim()); + LocalDate end = parseDate(dates[1].trim()); list.add(new DateRange(start, end)); } return ReservationDateList.of(list); } @@ - private static LocalDate parseDot(String s) { - try { - return LocalDate.parse(s, DOT); - } catch (DateTimeParseException e) { - throw new DomainRuleException(ErrorCode.INVALID_DATE_INPUT); - } - } + private static LocalDate parseDate(String s) { + try { + return LocalDate.parse(s, DOT); + } catch (DateTimeParseException e1) { + try { + return LocalDate.parse(s, ISO); + } catch (DateTimeParseException e2) { + throw new DomainRuleException(ErrorCode.INVALID_DATE_INPUT); + } + } + }Also applies to: 72-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/konkuk/chacall/domain/reservation/domain/value/DateRange.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationDateList.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java(3 hunks)src/main/java/konkuk/chacall/global/common/converter/ReservationDateListConverter.java(2 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationDateList.java (1)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java (1)
Getter(15-50)
🔇 Additional comments (4)
src/main/java/konkuk/chacall/domain/reservation/domain/value/DateRange.java (1)
10-17: @embeddable 레코드 사용 적절값 객체 불변성, 컬럼 명시, 레코드 적용 모두 적절합니다. JPA/Hibernate의 불변 임베더블 매핑과도 합치합니다.
src/main/java/konkuk/chacall/global/common/converter/ReservationDateListConverter.java (1)
29-31: DB 저장 포맷 정합성 재확인 필요현재
toStorageString()이 DOT 포맷을 사용하면, 기존 ISO로 저장된 레코드와 혼재되어 운영/리포트에서 혼란을 초래할 수 있습니다. 저장 포맷을 ISO로 통일할지, 혹은 마이그레이션을 수행할지 결정 부탁드립니다.저장 포맷을 ISO로 통일한다면, 아래
ReservationDateList변경안을 함께 적용해주세요.src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationDateList.java (1)
23-31: 불변 컬렉션 처리 및 팩토리 메서드 구성 깔끔
List.copyOf로 방어적 복사,isEmpty()제공 등 API 표면이 명료합니다. 이 부분은 그대로 가도 좋습니다.src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
50-55: 예약 도메인 에러코드 추가 적절: 도메인/HTTP 매핑 일관성 양호메시지/HTTP 상태/코드 범위는 컨벤션과 일치합니다. 다만 새 숫자 코드(90001~90003) 중복 여부를 확인해 주세요 — 제공하신 스크립트 실행 결과 "NO CODES FOUND"가 나와 패턴 불일치로 추출에 실패한 것으로 보입니다. 아래 대체 명령으로 src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java 파일의 두번째 인자(코드)를 재검사해 주세요.
#!/bin/bash file=src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java rg -oP '\(\s*HttpStatus\.[A-Z_]+\s*,\s*\K\d+' "$file" | sort | uniq -c \ | awk 'BEGIN{f=0} $1>1{print "DUP:",$0; f=1} END{if(!f) print "NO DUPLICATES"}'
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
와우 제가 누락한 것까지 엄청 꼼꼼히 추가해주셨네요! 역시 꼼신 (꼼꼼의 신이라는 뜻)
CursorBasedResponse 추가된 것 너무 좋습니다! 유용하게 사용하겠습니다.
쿼리도 흠잡을 데 없이 완벽하십니다 역시~ 👍🏻
몇가지 가독성 + 유지보수면에서 리뷰 좀 남겼는데 확인해주시면 감사하겠습니다~
| /** | ||
| * 예약 목록에 포함된 손님 정보 Map 조회 | ||
| */ | ||
| private Map<Long, User> getCustomerMap(List<Reservation> reservations) { | ||
| List<Long> customerIds = reservations.stream() | ||
| .map(reservation -> reservation.getMember().getUserId()) | ||
| .toList(); | ||
|
|
||
| return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream() | ||
| .collect(Collectors.toMap(User::getUserId, user -> user)); | ||
| } |
| // 미리 조회한 User 정보를 사용해 DTO 로 변환 | ||
| List<OwnerReservationHistoryResponse> responses = mapToReservationHistory(ownerReservationList, customerMap); | ||
|
|
||
| Long nextCursor = responses.isEmpty() ? null : responses.get(responses.size() - 1).reservationId(); |
There was a problem hiding this comment.
p2: nextCursor 구하는 로직은 아마 모든 응답에 대해서 공통적일 것 같은데 Response에서 어떤 필드를 cursor로 잡을지만 함수형 인터페이스로 전달하고 CursorPagingResponse 내부에서 nextCursor를 구하는거 어떨까요??
return CursorPagingResponse.of(responses, OwnerReservationHistoryResponse::reservationId, ownerReservationSlice.hasNext());
package konkuk.chacall.global.common.dto;
import java.util.List;
public record CursorPagingResponse<T>(
List<T> content,
Long lastCursor,
boolean hasNext
) {
public static <T> CursorPagingResponse<T> of(List<T> content, CursorExtractor<T> extractor, boolean hasNext) {
Long lastCursor = content.isEmpty() ? null : extractor.extractCursor(content.get(content.size() - 1));
return new CursorPagingResponse<>(content, lastCursor, hasNext);
}
}package konkuk.chacall.global.common.dto;
@FunctionalInterface
public interface CursorExtractor<T> {
Long extractCursor(T lastElement);
}There was a problem hiding this comment.
오호 아마 로직상 커서를 식별자로만 잡을 것 같아서 이런 부분을 고려하진 않았었는데, 이 부분은 적용해보면 좋을 것 같습니다 반영해볼게요
| public record CursorPagingResponse<T>( | ||
| List<T> content, | ||
| Long lastCursor, | ||
| boolean hasNext | ||
| ) { | ||
| public static <T> CursorPagingResponse<T> of(List<T> content, Long lastCursor, boolean hasNext) { | ||
| return new CursorPagingResponse<>(content, lastCursor, hasNext); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
p3: 혹시 정렬조건이 최신순이 아닌 경우는 어떻게 해야할까요?? lastCursor를 String으로 선언해두고 모든 값을 toString()으로 변환하는 차선책도 있을 것 같긴한데 어떻게 생각하시나요??
There was a problem hiding this comment.
음 근데 와이어프레임을 다시 확인해보니 아직까지는 그런 정렬 조건에 대한 요구사항은 딱히 없네요,, 나중에 요구사항이 추가되면 도입해도 늦지 않을 것 같습니다!
| @ExceptionDescription(SwaggerResponseDescription.OWNER_GET_RESERVATION_HISTORY) | ||
| @GetMapping("/me/reservations") | ||
| public BaseResponse<CursorPagingResponse<OwnerReservationHistoryResponse>> getOwnerReservations( | ||
| @Valid @ParameterObject final GetReservationHistoryRequest request, |
There was a problem hiding this comment.
혹시 여기서 @ParamterObject는 어떤 역할인가요??
There was a problem hiding this comment.
보통 RequestBody 로 요청이 가는 것들은 @RequestBody 로 요청을 받으면 되는데, RequestParam, 즉 쿼리파라미터로 요청이 가는 것들을 요청 DTO 로 매핑해서 관리하려면 @ModelAttribute 를 활용해야합니다.
하지만 @ModelAttribute 를 사용하면, 스웨거 상에서 마치 Request Body 로 요청을 받는 것처럼 UI 가 보여지더라구요.
그래서 이렇게 쿼리파라미터를 요청 DTO 로 매핑해서 사용하면서, 스웨거 상에서도 파라미터로 값을 받는 것처럼 보여지게하려면 이 @ParameterObject 를 명시해주면 된다고 합니다!
There was a problem hiding this comment.
오호 저는 쿼리 파라미터를 @RequestParam으로 사용했었는데 dto로 편리하게 관리할 수 있군요! 하나 배워갑니다~
| @Schema(description = "조회할 예약 상태", | ||
| requiredMode = Schema.RequiredMode.REQUIRED, | ||
| example = "PENDING", | ||
| allowableValues = {"PENDING", "CONFIRMED", "CANCELLED"}) |
There was a problem hiding this comment.
requiredMode와 allowableValues는 처음 보네요. 한번 활용해보겠습니다~
| // '예약자(손님)'와의 연관관계 (N:1) | ||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "user_id", nullable = false) | ||
| private User member; | ||
|
|
||
| // '예약된 푸드트럭'과의 연관관계 (N:1) | ||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "food_truck_id", nullable = false) | ||
| private FoodTruck foodTruck; |
| public long getCursorOrDefault() { | ||
| return (this.cursor == null) ? Long.MAX_VALUE : this.cursor; | ||
| } | ||
|
|
||
| public int getSizeOrDefault() { | ||
| return (this.size == null) ? DEFAULT_SIZE : this.size; | ||
| } |
There was a problem hiding this comment.
현재 구현해주신 방식은 getCursorOrDefault(), getSizeOrDefault() 메서드를 통해 null 방어를 잘 해주고 있어서 사용상 문제가 없을 것 같습니다.
다만 개인적으로는 record의 constructor를 오버라이드해서 생성 시점에 기본값을 주입하는 방식이 조금 더 직관적이지 않을까 싶은데 어떤가욥
이렇게 하면 이후에 별도의 getXXXOrDefault() 메서드를 호출하지 않아도 되고, DTO 자체가 이미 기본값을 가진 상태로 다뤄지기 때문에 서비스 코드 가독성이 조금이나마 올라가지 않을까 싶습니다.
public PagingRequest {
if (cursor == null) {
cursor = Long.MAX_VALUE;
}
if (size == null) {
size = DEFAULT_SIZE;
}
}There was a problem hiding this comment.
엇 그리고 추가적으로 직접 호출을 해보니까 cursor와 size를 모두 null로 두고 요청을 보낼 경우 Request 객체 자체에 NPE가 발생하는데 이 점은 어쩔 수 없는 상황인가요?? gpt한테 물어보니까 jsoncreator 머시기라고 하는데 조금 복잡해지는 것 같네요,,
There was a problem hiding this comment.
우선 생성자를 오버라이드하는 방식 좋은 것 같습니다!!
현재 언급해주신 이슈를 저도 방금 확인했네요.. 확인해보니 PagingRequest 객체가 다른 요청 DTO 의 내부에 포함되어있는 구조가 되다보니, 모든 필드가 null 로 전달될 경우 아예 객체 생성이 되지 않는다고 해요.
몇 가지 해결책이 있었는데,
- 모든 요청 dto 내부에, 자신이 필드로 갖는 PagingRequest 객체가 null 일 경우, new PagingRequest(null, null) 로 실제 객체를 만들어주는 메서드를 만든다.
-> 이 방식은 모든 요청 DTO 에 중복된 코드가 들어가야해서 비효율적이라 판단해 배제했습니다.
- PageRequest 객체를 요청 DTO와 분리해서 관리한다.
public BaseResponse<CursorPagingResponse<OwnerReservationHistoryResponse>> getOwnerReservations(
@Valid @ParameterObject final GetReservationHistoryRequest reservationRequest,
@Valid @ParameterObject final PagingRequest pagingRequest
@Parameter(hidden = true) @UserId final Long ownerId
) {
return BaseResponse.ok(ownerService.getOwnerReservations(
request,
ownerId));
} -> 이 방식을 활용하면 모든 필드에 null 이 들어가도 객체 자체는 정상적으로 생성이 되는 것 같아서 문제가 터지진 않을 것 같습니다. 다만 개인적인 취향이긴 한데, 하나의 요청 DTO 에서 pageRequest 객체도 관리해주는 것이 좀 더 깔끔하다는 느낌이 들어서 선호도는 제 개인적으로는 2순위였습니다.
- PagingRequest 객체 자체가 null 일시, new PagingRequest(null, null)을 생성해주는 디폴트 메서드를 갖는 인터페이스를 구현한다.
이 방식은 무한 스크롤 로직이 존재하는 API 에 대한 요청 DTO들은 모두 이 인터페이스를 구현하도록 함으로써, 실제로 서비스 로직에서 활용할 때에는 이 디폴트 메서드를 활용해서 NPE 를 막는 형식입니다.
실제 활용 예시는 아래와 같습니다.
개인적으로 이 3번째 방식이, 요청 DTO 가 무한 스크롤에 사용된다는 것도 시각적으로 바로 확인이 가능하고, 모든 요청 DTO 마다 공통된 로직을 만들 필요도 없기 때문에 개인적으로 가장 좋은 방식이라고 생각하여 이 방식으로 해결을 해보려고 합니다!
There was a problem hiding this comment.
오호 dto가 인터페이스를 구현해서 default 메서드로 기본 객체를 주입해주는 방식 좋은 것 같습니다~!! 👍🏻
| @Configuration | ||
| @RequiredArgsConstructor | ||
| public class WebConfig implements WebMvcConfigurer { | ||
|
|
||
| private final UserIdArgumentResolver userIdArgumentResolver; | ||
|
|
||
| @Override | ||
| public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) { | ||
| resolvers.add(userIdArgumentResolver); | ||
| } | ||
| } |
| public OwnerReservationDetailResponse getReservationDetail(Long reservationId) { | ||
| // ID로 예약 정보와 연관된 모든 데이터 한 번에 조회 | ||
| Reservation reservation = reservationRepository.findByIdWithDetails(reservationId) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND)); | ||
|
|
||
| // DTO 로 변환하여 반환 | ||
| return OwnerReservationDetailResponse.of(reservation, reservation.getMember()); | ||
| } |
There was a problem hiding this comment.
오 코드래빗 리뷰 좋은 것 같습니다! 해당 예약을 조회한 후에 그 예약이 정말 Owner가 소유하고 있는 예약인지 검증하는 것도 좋아보이네요!
두 가지 방법이 있을 것 같습니다.
- 조회할 때 두 fk로 조회
- reservationId로만 조회 후 Reservation 도메인 계층 내부에서 권한 검증
There was a problem hiding this comment.
오호 이 두 방식은 뭐로 구현하던 크게 상관 없을 것 같네용.
다만 생각해봤을 때, 2번 방식으로 Owner 가 소유한 예약인지를 확인하면, Reservation 도메인 내부에서 검증을 할 시, ownerId.equals(this.foodTruck.getOwner().getUserId())) 이런 형태로 검증이 들어갈 것 같은데, 어차피 findByIdWithDefails() 메서드를 사용했을 때 fetch join 을 통해서 Owner 정보까지 모두 끌어오기 때문에, getOwner() 메서드를 사용한다하더라도 지연로딩 없이, 즉 추가 쿼리 없이 검증이 가능할 것 같아서 이 방식을 활용하면 좋을 것 같습니다! 반영해보겠습니당
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
53-57: RESERVATION_NOT_OWNED(403) 노출 정책 재검토미소유 리소스에 403을 반환하면 “대상이 존재함”을 유추할 수 있습니다. 소유자 범위 조회로 404(RESERVATION_NOT_FOUND)만 반환하도록 조정하면 ID 열거 위험을 낮출 수 있습니다. 운영 정책상 403이 필요하다면 관리자/감사 로그로만 구분하고 외부 응답은 404로 은닉하는 것을 권장합니다.
검증 스크립트(코드 번호 중복 확인):
#!/bin/bash # ErrorCode 내 코드 번호 중복 여부 확인 rg -n 'HttpStatus\.[A-Z_]+,\s*(\d{5})' src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java \ | gawk -F '[(), ]+' '{print $NF}' \ | sort | uniq -c | gawk '$1>1{print "DUP:",$0} END{if(NR==0) print "NO_MATCH"}'src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
106-112: 예약 상세 조회 — 존재 여부 노출(정보 유출) 취약점: owner 스코프로 단일 조회하고 404로 은닉하십시오현재 서비스 계층(OwnerReservationService)에서 reservationId로 먼저 조회 후 isOwnedBy(ownerId)로 403을 던집니다. 이 흐름은 “해당 ID의 예약이 존재한다”는 사실을 공격자에게 노출합니다(리소스 ID 열거 가능). 레포지토리에서 ownerId와 reservationId를 함께 조건으로 조회하여, 미소유/미존재 모두 404(RESERVATION_NOT_FOUND)로 응답하세요.
아래와 같이 서비스/레포지토리 수정 제안을 드립니다(참고용):
*** src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java @@ - public OwnerReservationDetailResponse getReservationDetail(Long ownerId, Long reservationId) { - // ID로 예약 정보와 연관된 모든 데이터 한 번에 조회 - Reservation reservation = reservationRepository.findByIdWithDetails(reservationId) - .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND)); - - if(!reservation.isOwnedBy(ownerId)) { - throw new BusinessException(ErrorCode.RESERVATION_NOT_OWNED); - } - - // DTO 로 변환하여 반환 - return OwnerReservationDetailResponse.of(reservation, reservation.getMember()); - } + public OwnerReservationDetailResponse getReservationDetail(Long ownerId, Long reservationId) { + Reservation reservation = reservationRepository + .findByIdWithDetailsAndOwnerScope(reservationId, ownerId) + .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND)); + return OwnerReservationDetailResponse.of(reservation, reservation.getMember()); + }// 예시: ReservationRepository (참고용) @Query(""" select r from Reservation r join fetch r.member m join fetch r.truck t left join fetch r.schedules s left join fetch r.operatingTime ot where r.id = :reservationId and t.owner.id = :ownerId """) Optional<Reservation> findByIdWithDetailsAndOwnerScope(@Param("reservationId") Long reservationId, @Param("ownerId") Long ownerId);src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
84-91: customer null 방어 처리 추가 권장customerMap에 없는 경우를 대비해 null 고객을 마스킹/대체하도록 DTO 생성로직에 분기 처리를 추가하세요(예: “탈퇴 회원”, 연락처 마스킹 등). 기존 팩토리가 null을 허용하지 않으면 오버로드된 팩토리 추가를 권장합니다.
72-79: 비활성/탈퇴 회원 누락으로 인한 NPE/불완전 응답 위험 — 상태필터 제거 및 방어 코드 추가BaseStatus.ACTIVE로 필터링하면 과거 이력의 정지/탈퇴 회원이 customerMap에 없어서 이후 매핑 시 NPE/불완전 응답이 발생할 수 있습니다. 중복 ID 제거와 빈 목록 쇼트서킷도 함께 권장합니다.
적용 제안(diff):
- private Map<Long, User> getCustomerMap(List<Reservation> reservations) { - List<Long> customerIds = reservations.stream() - .map(reservation -> reservation.getMember().getUserId()) - .toList(); - - return userRepository.findAllByUserIdInAndRoleAndStatus(customerIds, Role.MEMBER, BaseStatus.ACTIVE).stream() - .collect(Collectors.toMap(User::getUserId, user -> user)); - } + private Map<Long, User> getCustomerMap(List<Reservation> reservations) { + List<Long> customerIds = reservations.stream() + .map(r -> r.getMember().getUserId()) + .filter(id -> id != null) + .distinct() + .toList(); + if (customerIds.isEmpty()) return Map.of(); + // 상태 필터 제거: 과거 이력 보존 목적 + return userRepository.findAllByUserIdInAndRole(customerIds, Role.MEMBER).stream() + .collect(Collectors.toMap(User::getUserId, user -> user)); + }참고: 위 변경은 UserRepository에
List<User> findAllByUserIdInAndRole(List<Long> userIds, Role role)메서드 추가가 필요합니다. 당장 리포지토리 수정이 어렵다면, 아래 84–91 라인에서 null 고객 방어 처리도 병행하세요.
🧹 Nitpick comments (10)
src/main/java/konkuk/chacall/global/common/dto/CursorExtractor.java (1)
3-6: null-안전 및 문서화 보강 제안
- lastElement가 null일 가능성, extractor가 null을 반환하는 경우에 대한 계약을 Javadoc으로 명시해 주세요.
- 대안: java.util.function.Function<T, Long>을 받아도 충분합니다(의존성 축소). 현재 명명적 이점이 크지 않다면 교체 고려 부탁드립니다.
src/main/java/konkuk/chacall/global/common/dto/HasPaging.java (1)
6-8: 기본값 생성 위치를 PagingRequest로 이동해 결합도 낮추기인터페이스에서 new PagingRequest(null, null)를 직접 생성하면 PagingRequest의 생성자/기본값 정책 변경 시 여기까지 영향이 전파됩니다.
- 제안: PagingRequest에 DEFAULT 상수나 defaults() 정적 팩토리를 두고 여기서는 그 팩토리만 호출하세요.
예)
- pagingOrDefault() → return Optional.ofNullable(pagingRequest()).orElse(PagingRequest.defaults());
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (3)
7-7: 와일드카드 import 지양팀 컨벤션/체크스타일에 따라 * import가 실패 원인이 될 수 있습니다. 사용되는 DTO만 명시 import 해 주세요.
97-104: 페이지 파라미터 유효성 보장 점검request.pagingOrDefault()로 null 대체는 좋습니다. size 최소/최대(@Min/@max 등)와 cursor 음수 방지 검증이 PagingRequest에 존재하는지 확인 부탁드립니다. 존재하지 않으면 여기서도 가드해 주세요.
97-104: 메서드 파라미터 순서 일관화OwnerService 내 메서드들에서 (request, ownerId)와 (ownerId, …, request)가 혼재합니다. API 일관성을 위해 통일을 권장합니다.
src/main/java/konkuk/chacall/global/common/dto/CursorPagingResponse.java (2)
10-17: of(...)에서 null-가드와 편의 생성자 추가 제안
- extractor에 대한 명시적 null-가드를 추가해 방어적 프로그래밍을 확보하세요.
- 빈 페이지 응답을 만드는 empty() 편의 생성자를 두면 호출부 단순화에 도움이 됩니다.
적용 예시:
package konkuk.chacall.global.common.dto; import java.util.List; +import java.util.Objects; public record CursorPagingResponse<T>( List<T> content, Long lastCursor, boolean hasNext ) { public static <T> CursorPagingResponse<T> of( List<T> content, CursorExtractor<T> extractor, boolean hasNext ) { - Long lastCursor = (content == null || content.isEmpty()) ? null : extractor.extractCursor(content.get(content.size() - 1)); + Objects.requireNonNull(extractor, "extractor must not be null"); + Long lastCursor = (content == null || content.isEmpty()) + ? null + : extractor.extractCursor(content.get(content.size() - 1)); return new CursorPagingResponse<>(content, lastCursor, hasNext); } + + public static <T> CursorPagingResponse<T> empty() { + return new CursorPagingResponse<>(List.of(), null, false); + } }
5-9: 커서 자료형 고정(Long)에 대한 트레이드오프 인지현재 요구사항에 부합하지만, 향후 정렬/커서 전략이 변경(예: createdAt DESC, 복합키, 문자열 커서)되면 제네릭형 커서 또는 String 커서로의 확장이 필요할 수 있습니다. 지금은 유지하되 확장 포인트로 기록만 남겨두시죠.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (2)
45-47: isOwnedBy NPE/지연로딩 안전성 보완foodTruck 또는 owner가 null일 경우 NPE가 나고, fetch join이 누락되면 불필요한 추가 쿼리가 발생할 수 있습니다. null‑safe하게 방어해 500을 피하고, 불일치 시 false를 반환하도록 하는 편이 안전합니다.
적용 제안(diff):
- public boolean isOwnedBy(Long ownerId) { - return foodTruck.getOwner().getUserId().equals(ownerId); - } + public boolean isOwnedBy(Long ownerId) { + if (ownerId == null || this.foodTruck == null) return false; + var owner = this.foodTruck.getOwner(); + return owner != null && owner.getUserId() != null && owner.getUserId().equals(ownerId); + }
29-31: pdfUrl 저장전략 재검토(키 저장 권장)서명된 URL/도메인 변경에 대비하려면 전체 URL 대신 스토리지 키(경로)만 보관하고, 응답 시에만 서명 URL을 생성하는 패턴이 운영상 안정적입니다. 길이 500은 일부 서명 URL에 부족할 수 있어 키 보관 전환을 추천합니다.
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
48-56: 권한 검증을 리포지토리 레벨로 위임 + 404 은닉 권장현재는 단건 조회 후 도메인에서 소유자 검증(403)이지만, 존재 유무를 은닉하고 쿼리 단계에서 오너 조건을 포함하면 보안/성능이 개선됩니다. 404로 일관 처리도 고려해 주세요.
적용 제안(diff):
- Reservation reservation = reservationRepository.findByIdWithDetails(reservationId) - .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND)); - - if(!reservation.isOwnedBy(ownerId)) { - throw new BusinessException(ErrorCode.RESERVATION_NOT_OWNED); - } + Reservation reservation = reservationRepository.findByIdWithDetailsAndOwner(reservationId, ownerId) + .orElseThrow(() -> new EntityNotFoundException(ErrorCode.RESERVATION_NOT_FOUND));비고:
findByIdWithDetailsAndOwner추가 필요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(3 hunks)src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/CursorExtractor.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/CursorPagingResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/HasPaging.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/PagingRequest.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/chacall/global/common/dto/PagingRequest.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(21-113)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
RequiredArgsConstructor(26-92)
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
src/main/java/konkuk/chacall/domain/reservation/domain/value/ReservationInfo.java (1)
Getter(15-50)
🔇 Additional comments (3)
src/main/java/konkuk/chacall/global/common/dto/CursorExtractor.java (1)
3-6: 간결하고 목적에 맞는 함수형 인터페이스 정의, 좋습니다커서 추출 책임을 명시적으로 분리해 가독성이 좋습니다.
src/main/java/konkuk/chacall/domain/reservation/domain/model/Reservation.java (1)
45-47: 확인: 상세 조회 쿼리에 foodTruck.owner FETCH JOIN 포함
ReservationRepository.findByIdWithDetails 쿼리에 JOIN FETCH ft.owner가 있어 isOwnedBy 호출 시 추가 쿼리/LAZY 초기화 예외 우려 없음.src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
64-67: 확인 완료 — 쿼리에 명시적 정렬 있음
ReservationRepository의 @query에ORDER BY r.reservationId DESC가 있어 lastCursor 기반 커서 정렬과 일치하므로 누락/중복 우려 없음. 위치: src/main/java/konkuk/chacall/domain/reservation/domain/repository/ReservationRepository.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (2)
8-8: 와일드카드 import 지양 (팀 컨벤션 일관성).request DTO는 명시적 import가 가독성과 정적 분석에 유리합니다.
다음과 같이 교체 제안:
-import konkuk.chacall.domain.owner.presentation.dto.request.*; +import konkuk.chacall.domain.owner.presentation.dto.request.GetReservationHistoryRequest; +import konkuk.chacall.domain.owner.presentation.dto.request.RegisterBankAccountRequest; +import konkuk.chacall.domain.owner.presentation.dto.request.RegisterChatTemplateRequest; +import konkuk.chacall.domain.owner.presentation.dto.request.UpdateBankAccountRequest; +import konkuk.chacall.domain.owner.presentation.dto.request.UpdateChatTemplateRequest;
160-172: 상세 조회 경로의 슬래시 표기 일관화 제안.동작상 문제는 없으나, 파일 내 다른 매핑과 동일하게 선행 슬래시를 붙여 일관성을 유지하면 좋습니다.
다음 수정 제안:
- @GetMapping("me/reservations/{reservationId}") + @GetMapping("/me/reservations/{reservationId}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (2)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(21-113)src/main/java/konkuk/chacall/domain/owner/application/reservation/OwnerReservationService.java (1)
RequiredArgsConstructor(26-92)
🔇 Additional comments (6)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (6)
10-11: 응답 DTO import 적절.
필요한 타입만 명시되어 있으며 의존성 최소화 측면에서 문제 없습니다.
16-16: 커서 페이징 래퍼 import OK.
컨트롤러에서 제네릭 타입 안정적으로 노출됩니다.
20-20: @ParameterObject 추가 적절.
쿼리 파라미터를 Swagger에서 파라미터로 표시하려는 의도에 부합합니다.
66-66: 엔드포인트 매핑 변경 무해.
경로 포맷 일관성 유지됩니다.
71-71: 포맷팅 변경만 존재.
동작 영향 없음.
123-123: 채팅 템플릿 수정 매핑 OK.
기존 패턴과 일관적입니다.
| @Operation( | ||
| summary = "사장님 예약 내역 목록 조회 (무한 스크롤)", | ||
| description = "사장님의 예약 내역 목록을 조회합니다.") | ||
| @ExceptionDescription(SwaggerResponseDescription.OWNER_GET_RESERVATION_HISTORY) | ||
| @GetMapping("/me/reservations") | ||
| public BaseResponse<CursorPagingResponse<OwnerReservationHistoryResponse>> getOwnerReservations( | ||
| @Valid @ParameterObject final GetReservationHistoryRequest request, | ||
| @Parameter(hidden = true) @UserId final Long ownerId | ||
| ) { | ||
| return BaseResponse.ok(ownerService.getOwnerReservations( | ||
| request, | ||
| ownerId)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
목록 조회 엔드포인트: 바인딩/검증 흐름 OK, 예외 매핑만 확인 필요.
- GET 쿼리 객체에 @ParameterObject + @Valid 조합 적절합니다.
- 단, @ModelAttribute 바인딩 검증 실패 시 BindException이 발생하므로, 글로벌 예외 처리에서 BindException을 BaseResponse 규격으로 매핑하는지 확인 부탁드립니다. 없다면 400 응답 포맷이 일관되지 않을 수 있습니다.
다음 스크립트로 예외 처리 및 UserId 리졸버 등록을 점검하세요.
🏁 Script executed:
#!/bin/bash
# 1) 글로벌 예외 처리기와 BindException 핸들링 확인
rg -nP --type=java '@ControllerAdvice' -C3
rg -nP --type=java 'BindException|MethodArgumentNotValidException' -C3
# 2) @UserId ArgumentResolver 등록 여부 확인
rg -nP --type=java 'class\s+UserIdArgumentResolver' -C3
rg -nP --type=java 'addArgumentResolvers\s*\(' -C3
# 3) 요청 DTO 유효성 애너테이션 존재 여부(선택)
fd -a 'GetReservationHistoryRequest\.java' | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,200p" {}'Length of output: 5470
목록 조회 엔드포인트: BindException 핸들러 추가 필요 (검증·리졸버 점검 완료)
- @ParameterObject + @Valid 조합은 적절합니다. DTO 유효성 애너테이션 확인: src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/GetReservationHistoryRequest.java (status: @NotNull, pagingRequest: @Valid).
- UserId ArgumentResolver 등록 확인: src/main/java/konkuk/chacall/global/common/security/resolver/UserIdArgumentResolver.java 및 src/main/java/konkuk/chacall/global/config/WebConfig.java (addArgumentResolvers에 추가됨).
- 문제점: 글로벌 예외처리기에는 MethodArgumentNotValidException 핸들러만 존재하고 BindException에 대한 @ExceptionHandler가 없습니다 (src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java). GET(@ParameterObject) 바인딩 검증 실패는 BindException이므로, BindException을 BaseResponse(400) 포맷으로 매핑하도록 핸들러를 추가하거나 기존 검증 핸들러를 확장하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java
around lines 146-158: the controller uses @ParameterObject + @Valid which on GET
requests can trigger BindException on binding failures, but your global handler
only handles MethodArgumentNotValidException; open
src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java
and add an @ExceptionHandler(BindException.class) method that mirrors the
existing MethodArgumentNotValidException handling—extract validation errors,
construct a BaseResponse with HTTP 400 and the same error payload shape, and
return it so GET @ParameterObject validation failures are mapped to the expected
BaseResponse(400) format.
2 participants
#️⃣연관된 이슈
📝작업 내용
사장님 - 예약 내역 목록 조회 및 상세 조회 API 를 구현하였습니다.
무한스크롤을 위한 공통 DTO 구현
모든 리스트 기반 조회 로직에는 무한스크롤이 적용될 예정이라고 합니다. 따라서 무한스크롤에 필요한 커서, 마지막 페이지 여부 등을 공통으로 관리해주는 DTO 를 구현하였습니다. 이 때 응답에는 pageSize 를 별도로 명시하지 않았습니다. 프론트단에서 따로 활용할 여지가 없다고 판단했기 때문입니다.
다른 무한스크롤 로직에도 이 dto 를 가져다가 사용하시면 될 것 같습니다
추가로 무한스크롤을 구현할 경우, 동일한 값을 매번 요청에 포함시켜야할 필요성이 존재했습니다. cursor, size 값이 그 대표적인 예시인데, 이 두 가지 값들은 null 을 넘겨줄 경우 서버단에서 자동으로 기본값을 채워주도록 구현하고자 했습니다.
다만 만약 페이징 관련 요청을 공통되게 처리해주는 dto 가 없다면, 이 기본값 설정 로직을 모든 페이징이 필요한 API의 요청 dto 마다 구현해주어야했습니다.
따라서 그러한 중복 로직을 막고자 PagingRequest 라는 페이징 관련 요청만을 담당하는 DTO 를 따로 만들고, 이 dto 를 실제로 서비스로직에 활용될 요청 DTO 에 포함시키도록 구현하였습니다.
따라서 추후 페이징 관련 로직 구현하실 때 이 부분도 참고하시면 좋을 것 같습니다
예약 내역 목록 조회
예약 내역 목록 조회에서는 다음과 같은 고민거리가 있었습니다.
하나의 예약에는 최대 2개의 예약 일정이 존재할 수 있고, 오직 한 개의 운영 시간대가 존재한다.
예약 일정은 2개인데 운영 시간대는 하나라서 이 부분을 어떻게 처리해야하나 PM 님께 여쭤보니, 두 일정 모두 하나의 운영 시간대를 따르도록 해주면 된다라는 피드백을 받게 되어서 해당 방식으로 구현을 진행하였습니다.
이 때 응답을 손쉽게 만들 수 있도록 ReservationInfo 클래스 내부에 일정 + 운영 시간대를 합쳐서 List 로 반환해주는 메서드를 만들어두었습니다.
추가로 페이징 로직이 들어가다보니 조회시에 fetch join 을 사용할 수 없게 되었습니다. 따라서 N+1 문제를 해결하기 위해 다음과 같은 흐름을 따라주었습니다.
사장님의 예약 목록을 우선 조회해온다. (페이징) -> 쿼리 1회조회해온 예약에서 멤버들의 id 들을 추출한 후, 추출한 멤버의 id 를 기반으로 DB에서 해당 id 를 갖는 멤버를 모두 조회한다 -> 쿼리 1회이렇게 조회해온 데이터들을 기반으로 응답을 내려준다.이 방식이 아니라 실제로 지연 로딩을 활용할 경우,
사장님 예약 목록 조회하는 쿼리 1회각 예약 목록에서 멤버 조회(getMember()) 를 통한 쿼리 N회 (페이지 개수 만큼)가 필요했으나, 위와 같이 로직을 변경함으로써 쿼리 횟수를 2회로 줄였습니다.
예약 상세 내역 조회
이 기능은 별다른 이슈는 없었습니다. 그저 reservationId 를 받아 예약 객체를 조회하면 되었는데, 이 때는 별도의 페이징이 존재하지 않기 때문에 fetch join 을 적용함으로써 쿼리가 1회만 발생하도록 구현하였습니다.
스크린샷 (선택)
💬리뷰 요구사항(선택)
Summary by CodeRabbit