[FEAT] 푸드트럭 메뉴 등록/조회 API & 푸드트럭 관련 간단한 CRUD 구현#34
Conversation
Walkthrough푸드트럭 이름 중복 체크와 점주 메뉴 등록/조회/표시 상태 변경, 관리자 푸드트럭 승인 흐름이 추가되었고 페이징/정렬 추상화가 변경되었습니다. 서비스·컨트롤러·레포지토리·도메인·DTO·에러코드·스웨거 항목들이 확장·수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Owner
participant API as OwnerController
participant SVC as OwnerService
participant MFS as MyFoodTruckMenuService
participant FRepo as FoodTruckRepository
participant MRepo as MenuRepository
rect rgb(245,248,255)
note over Owner,API: 점주 메뉴 조회 (커서 페이징/정렬)
Owner->>API: GET /me/food-trucks/{id}/menus
API->>SVC: getMyFoodTruckMenus(ownerId, foodTruckId, req)
SVC->>MFS: delegate
MFS->>FRepo: findByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn(...)
alt 승인됨
MFS->>MRepo: findMenusAsc/Desc(foodTruckId, cursor, pageable)
MRepo-->>MFS: Slice<Menu>
MFS-->>SVC: CursorPagingResponse<MyFoodTruckMenuResponse>
SVC-->>API: 200
else 미승인
MFS-->>SVC: throw FOOD_TRUCK_NOT_APPROVED
SVC-->>API: 403
end
end
sequenceDiagram
actor Admin
participant API as UserController
participant US as UserService
participant AS as AdminService
participant FRepo as FoodTruckRepository
participant FT as FoodTruck
rect rgb(245,255,245)
note over Admin,API: 관리자 푸드트럭 승인
Admin->>API: PATCH /admin/food-trucks/{id}/approval {status}
API->>US: approveFoodTruckStatus(userId, id, req)
US->>AS: delegate
AS->>FRepo: findById(id)
alt 존재
FRepo-->>AS: FoodTruck
AS->>FT: approveFoodTruck(req.status)
FT-->>AS: 상태 갱신 또는 예외
AS-->>US: void
US-->>API: 200
else 없음
FRepo-->>AS: not found
AS-->>US: throw FOOD_TRUCK_NOT_FOUND
US-->>API: 404
end
end
sequenceDiagram
actor Owner
participant API as FoodTruckController
participant SVC as FoodTruckService
participant CMD as FoodTruckCommandService
rect rgb(255,248,240)
note over Owner,API: 이름 중복 체크
Owner->>API: POST /food-trucks/duplicate-check {name}
API->>SVC: isNameDuplicated(ownerId, request)
SVC->>CMD: isNameDuplicated(request.name)
CMD-->>SVC: boolean duplicated
SVC-->>API: FoodTruckNameDuplicateCheckResponse.of(duplicated)
API-->>Owner: 200
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/infra/FoodTruckSearchRepositoryImpl.java (1)
119-130: SortType 파라미터 추가 및 정렬 분기 필수
- FoodTruckSearchRequest에 SortType sort 필드 추가
request.pagingOrDefault(sort)로 초기 커서 분기 (NEWEST→Long.MAX_VALUE, OLDEST→0)- repository에서 sort에 따라
where.and(foodTruck.foodTruckId.lt/gt(cursor))및orderBy(...desc()/asc())분기- 다음 페이지 커서도 정렬 방향에 맞춰 올바르게 반환
🧹 Nitpick comments (19)
src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java (3)
206-210: GET 메뉴 목록 요청 파라미터에 @Valid 누락쿼리 파라미터 레코드의 내부 필드(CursorPagingRequest 등) 검증이 동작하지 않습니다. @Valid 추가를 권장합니다.
- public BaseResponse<CursorPagingResponse<MyFoodTruckMenuResponse>> getMenus ( - @PathVariable final Long foodTruckId, - @ParameterObject final MyFoodTruckMenuListRequest request, - @Parameter(hidden = true) @UserId final Long ownerId) { + public BaseResponse<CursorPagingResponse<MyFoodTruckMenuResponse>> getMenus ( + @PathVariable final Long foodTruckId, + @Valid @ParameterObject final MyFoodTruckMenuListRequest request, + @Parameter(hidden = true) @UserId final Long ownerId) {
217-225: POST 등록 응답: 201 Created + Location 헤더 고려현재 200 OK + 빈 바디를 반환합니다. 생성 API는 리소스 ID 반환 또는 Location 헤더 포함 201 Created가 REST 관례상 더 명확합니다. 기존 컨벤션 유지 필요 시 무시해도 됩니다.
232-241: 상태 변경 API 명세 일관성엔드포인트를 PATCH /menus/{menuId}/status로 단순화하면 리소스 지향적입니다. 현재 경로도 기능상 문제는 없습니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java (2)
56-58: 메서드 명명 개선 제안parsingMenuPrice → formatPrice 등 의미를 더 잘 드러내는 이름을 권장합니다.
- public String parsingMenuPrice() { + public String formatPrice() { return price + "원"; }
46-53: 중복 기본값 지정 제거 가능@Builder.Default로 ON을 지정했으므로 create()에서 .menuViewedStatus(MenuViewedStatus.ON) 설정은 중복입니다.
return Menu.builder() .name(name) .price(price) .description(description) .imageUrl(imageUrl) - .menuViewedStatus(MenuViewedStatus.ON) .foodTruck(foodTruck) .build();src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterMenuRequest.java (2)
24-27: photoUrl 길이 검증 누락 (DB 제약과 불일치 가능)엔티티 imageUrl 컬럼 길이는 2083입니다. DTO에도 동일한 최대 길이 제약을 추가하세요. URL 형식 검증(@url)도 고려해볼 만합니다.
@Schema(description = "대표 메뉴 이미지 URL", example = "https://cdn.example.com/menus/bulgogi-burger.jpg") @NotBlank(message = "사진은 반드시 1장 등록해야 합니다.") + @Size(max = 2083, message = "사진 URL은 최대 2083자까지 입력 가능합니다.") String photoUrl
7-27: 필드명 일관성(photoUrl vs imageUrl)DTO(photoUrl)와 엔티티(imageUrl) 필드명이 달라 매핑 계층에서 혼선을 줄 수 있습니다. 통일을 권장합니다(변경 시 API 호환성 검토).
src/main/java/konkuk/chacall/global/common/dto/SortType.java (1)
17-19: Jackson 직렬화/역직렬화 어노테이션 확인
SortType은 EnumValueConverter를 통해 query parameter 바인딩이 “최신순”/“오래된순” 값으로 동작합니다. 다만 @JsonValue/@JsonCreator가 없어 JSON 직렬화/역직렬화 시 NEWEST/OLDEST 이름(name)만 사용되니, 의도한 출력·입력 포맷인지 검토 후 필요한 어노테이션을 추가해주세요.src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java (1)
97-112: 승인 전이 로직 OK. 다만 메서드명이 의미를 혼동시킵니다.현재 전이는 PENDING→OFF/REJECTED, REJECTED→OFF만 허용합니다. 메서드명이 approveFoodTruck이어서 “승인=ON”으로 오해될 수 있습니다. 도메인 용어에 맞춰 changeApprovalStatus 등으로 리네이밍하거나 Javadoc으로 의도를 명확히 해주세요.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java (1)
6-10: 입력 길이 제약 추가 권장DB 컬럼 길이(기본 255)에 맞춰 길이 검증을 추가해 과도한 입력/DB 예외를 사전에 방지하세요.
적용 예시:
import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; public record FoodTruckNameDuplicateCheckRequest( @Schema(description = "중복 여부를 확인할 푸드트럭 이름", example = "차콜 꼬치") - @NotBlank(message = "푸드트럭 이름은 필수입니다.") + @NotBlank(message = "푸드트럭 이름은 필수입니다.") + @Size(max = 255, message = "푸드트럭 이름은 255자 이하여야 합니다.") String name ) {}src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateMenuStatusRequest.java (1)
7-11: 허용값(OpenAPI) 명시로 스웨거 가독성 개선스키마에 허용값(ON/OFF)을 명시하면 클라이언트 사용성이 좋아집니다.
public record UpdateMenuStatusRequest( - @Schema(description = "변경할 메뉴 표시 여부", example = "OFF") + @Schema(description = "변경할 메뉴 표시 여부", example = "OFF", allowableValues = {"ON","OFF"}) @NotNull(message = "메뉴 상태는 필수입니다.") MenuViewedStatus status ) {}src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (1)
42-52: 중복 체크 API의 HTTP 메서드 재검토(선택 사항)단순 조회성(is duplicated) 작업이라면 GET + query param로의 전환을 고려할 수 있습니다. 추후 CDN/캐시/멱등성 측면에서 이점이 있습니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/MenuRepository.java (2)
19-29: 첫 페이지(lastCursor 미지정) 처리 안전성 보강현재 JPQL이
m.menuId < :lastCursor/> :lastCursor전제로 작성되어 lastCursor가 null인 경우 예외가 납니다. 서비스 단에서 센티넬 값을 주입한다면 OK지만, 레포지토리 레벨에서도 null 안전 처리를 고려하면 방어적입니다.다음과 같이 coalesce/조건을 추가하면 null에 안전합니다(첫 페이지만 약간의 옵티마이저 영향 가능):
- and m.menuId < :lastCursor + and (:lastCursor is null or m.menuId < :lastCursor)- and m.menuId > :lastCursor + and (:lastCursor is null or m.menuId > :lastCursor)서비스에서 기본 커서(예: DESC=Long.MAX_VALUE, ASC=0)를 항상 주입하는지 확인 부탁드립니다.
Also applies to: 30-39
19-29: 커서 페이징 쿼리 성능 가이드(운영 조언)해당 패턴은
(food_truck_id, menu_id)복합 인덱스가 있으면 효율이 좋습니다. 현재 PK(menu_id)만 있으면food_truck_id조건과 결합해 최적화가 제한될 수 있습니다. 운영 DB에 다음 인덱스를 고려하세요.예:
create index idx_menu_foodtruckid_menuid on menu(food_truck_id, menu_id);Also applies to: 30-39
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
149-156: 소유권 및 상태 검증 책임 위치 확인 필요Owner 검증만 수행 중입니다. 실제 메뉴 등록 시 대상 푸드트럭 소유권/상태(예: APPROVED) 검증이 MyFoodTruckMenuService 내부에 존재하는지 확인 부탁드립니다. 없다면 여기서 선제 검증 또는 해당 서비스로 이전 권장.
src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/MyFoodTruckMenuResponse.java (2)
21-31: 메뉴 표시 상태 NPE 방어 로직 추가 권장
menu.getMenuViewedStatus()가 null이면.name()에서 NPE 가능합니다. 기본값(예: "OFF")으로 폴백하세요.적용 예시:
- menu.getMenuViewedStatus().name() + (menu.getMenuViewedStatus() != null ? menu.getMenuViewedStatus().name() : "OFF")
12-13: 가격은 포맷 문자열과 숫자 원값을 병행 노출 고려(선택 사항)정렬/통계 등을 위해 Long(원화) 등 원시 금액 필드 추가를 권장합니다. 현재는 "12000원" 문자열만 제공되어 클라이언트 정렬 시 비용이 큽니다.
src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (1)
37-39: 이름 비교 전 공백 정규화 권장(선택 사항)앞/뒤 공백만 다른 이름을 동일하게 취급하려면 trim 후 비교를 권장합니다.
- public boolean isNameDuplicated(String name) { - return foodTruckRepository.existsByNameIgnoreCase(name); - } + public boolean isNameDuplicated(String name) { + String normalized = name == null ? "" : name.trim(); + return foodTruckRepository.existsByNameIgnoreCase(normalized); + }src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java (1)
23-33: JPQL IN 구문 괄호 권장(호환성/가독성 개선)
in :statuses도 동작하나, 벤더/툴링 간 혼선을 줄이기 위해in (:statuses)표기를 권장합니다.- and f.foodTruckStatus in :statuses + and f.foodTruckStatus in (:statuses)- and f.foodTruckStatus in :statuses + and f.foodTruckStatus in (:statuses)Also applies to: 35-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/MenuRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/infra/FoodTruckSearchRepositoryImpl.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/value/MenuViewedStatus.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/ApproveFoodTruckStatusRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/FoodTruckNameDuplicateCheckResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/member/application/MemberService.java(2 hunks)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(5 hunks)src/main/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/MyFoodTruckMenuListRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterMenuRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateMenuStatusRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/MyFoodTruckMenuResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/CursorPagingRequest.java(0 hunks)src/main/java/konkuk/chacall/global/common/dto/HasPaging.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/SortType.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(2 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/konkuk/chacall/global/common/dto/CursorPagingRequest.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (1)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java (1)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
RequiredArgsConstructor(23-166)
src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java (2)
src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java (1)
Builder(16-120)src/main/java/konkuk/chacall/domain/member/domain/SavedFoodTruck.java (1)
Builder(9-36)
🔇 Additional comments (11)
src/main/java/konkuk/chacall/domain/foodtruck/domain/value/MenuViewedStatus.java (1)
5-14: enum 설계 깔끔합니다상태 상수와 사용자 가독용 value 노출 구성이 명확합니다.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/FoodTruckNameDuplicateCheckResponse.java (1)
5-12: 단순 응답 DTO 적절합니다정적 팩터리 메서드 포함 구성 좋습니다.
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
30-33: 주입된 커맨드 서비스 활용이 간결합니다.
클래스 기본 readOnly 설정을 유지하면서 필요한 구간만 쓰기 트랜잭션으로 재정의하신 구성, 명확하게 느껴집니다.src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java (1)
93-109: 스웨거 오류 매핑 확장이 알맞습니다.
점주 메뉴 플로우에서 발생 가능한 예외들을 빠짐없이 연결해 두셔서 문서 신뢰도가 올라가겠네요.src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java (1)
97-112: ON 전이 미지원 의도 확인 요청공개 조회에서는 ON 상태만 노출됩니다(FoodTruckSearchRepositoryImpl Line 116). 본 메서드는 OFF/REJECTED로만 전이시켜 운영자 승인 이후에도 공개 상태(ON)로는 가지 못합니다. ON 전이는 별도 플로우(예: 운영자/점주 활성화 API)로 처리되는 것이 의도인지 확인 부탁드립니다.
src/main/java/konkuk/chacall/domain/member/application/MemberService.java (1)
65-68: NEWEST 기본 정렬 적용: LGTM기본 정렬을 최신순으로 강제하는 변경은 의도에 부합합니다. 하위 레이어에서 SortType에 따라 정렬/커서 비교(ASC: gt, DESC: lt)를 실제로 분기하는지 확인만 부탁드립니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/MenuRepository.java (1)
41-47: LGTM — 스코프 제한 조회 메소드 추가 적절푸드트럭 범위 내 특정 메뉴 조회 메소드 시그니처/JPQL 모두 적절합니다.
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
78-99: LGTM — 도메인별 에러코드 확장 적절신규 코드들이 의미/HTTP 상태/범위가 명확합니다. 스웨거 응답/예외 매퍼에 해당 코드들이 연결되는지 최종 확인만 부탁드립니다.
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (1)
141-147: LGTM — 위임/트랜잭션 경계 분리 적절조회는 readOnly, 변경은 @transactional로 명확히 구분되어 있습니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java (1)
48-49: 케이스 민감도/정규화 확인 요청
existsByNameIgnoreCase는 DB Collation/정규화 정책에 따라 동작이 달라질 수 있습니다(한글 포함). 트리밍/중복 스페이스, 특수문자 변형 등에 대한 요구사항이 있다면 추가 정규화가 필요합니다.src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (1)
41-47: @transactional 중복 적용 불필요
FoodTruckService의approveFoodTruckStatus메서드에 이미@Transactional이 선언되어 있어FoodTruckCommandService에 별도 추가할 필요가 없습니다.Likely an incorrect or invalid review comment.
.../java/konkuk/chacall/domain/user/presentation/dto/request/ApproveFoodTruckStatusRequest.java
Show resolved
Hide resolved
| @Operation( | ||
| summary = "푸드트럭 승인 상태 변경", | ||
| description = "운영자 - 푸드트럭 승인 상태를 변경합니다." | ||
| ) | ||
| @ExceptionDescription(SwaggerResponseDescription.APPROVE_FOOD_TRUCK_STATUS) | ||
| @PatchMapping("/{foodTruckId}/approval") | ||
| public BaseResponse<Void> approveFoodTruckStatus ( | ||
| @PathVariable final Long foodTruckId, | ||
| @Valid @RequestBody final ApproveFoodTruckStatusRequest request | ||
| ) { | ||
| foodTruckService.approveFoodTruckStatus(foodTruckId, request); | ||
| return BaseResponse.ok(null); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
운영자 전용 승인 엔드포인트에 권한 가드 추가 필요
승인 상태 변경 API는 운영자 전용이라고 주석에 명시되어 있으나, 메소드/컨트롤러에 권한 제약이 없습니다. 최소 @PreAuthorize 등으로 롤 가드가 필요합니다.
적용 예시(메소드 어노테이션 추가):
@ExceptionDescription(SwaggerResponseDescription.APPROVE_FOOD_TRUCK_STATUS)
@PatchMapping("/{foodTruckId}/approval")
+ @PreAuthorize("hasRole('OPERATOR')")
public BaseResponse<Void> approveFoodTruckStatus (
@PathVariable final Long foodTruckId,
@Valid @RequestBody final ApproveFoodTruckStatusRequest request
) {
foodTruckService.approveFoodTruckStatus(foodTruckId, request);
return BaseResponse.ok(null);
}필요한 import(파일 상단에 추가):
import org.springframework.security.access.prepost.PreAuthorize;권한 가드 일관성 점검 스크립트:
🏁 Script executed:
#!/bin/bash
# FoodTruckController 승인 엔드포인트에 PreAuthorize가 있는지 확인 및 유사 패턴 조사
rg -n -C2 --type=java '@PatchMapping\("/food-trucks/\{foodTruckId\}/approval"\)'
rg -n -C2 --type=java '@PreAuthorize\(' src/main/java/konkuk/chacallLength of output: 0
🏁 Script executed:
#!/bin/bash
# FoodTruckController 승인 엔드포인트에 @PreAuthorize 어노테이션 적용 여부 재확인
rg -n -C1 --type=java '@PatchMapping\("/\{foodTruckId\}/approval"\)' src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java
rg -n -C1 --type=java '@PreAuthorize' src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.javaLength of output: 188
운영자 전용 엔드포인트에 @PreAuthorize 추가 필요
@PatchMapping("/{foodTruckId}/approval")
+@PreAuthorize("hasRole('OPERATOR')")필요한 import: org.springframework.security.access.prepost.PreAuthorize
[src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java:59]
📝 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.
| @Operation( | |
| summary = "푸드트럭 승인 상태 변경", | |
| description = "운영자 - 푸드트럭 승인 상태를 변경합니다." | |
| ) | |
| @ExceptionDescription(SwaggerResponseDescription.APPROVE_FOOD_TRUCK_STATUS) | |
| @PatchMapping("/{foodTruckId}/approval") | |
| public BaseResponse<Void> approveFoodTruckStatus ( | |
| @PathVariable final Long foodTruckId, | |
| @Valid @RequestBody final ApproveFoodTruckStatusRequest request | |
| ) { | |
| foodTruckService.approveFoodTruckStatus(foodTruckId, request); | |
| return BaseResponse.ok(null); | |
| } | |
| @Operation( | |
| summary = "푸드트럭 승인 상태 변경", | |
| description = "운영자 - 푸드트럭 승인 상태를 변경합니다." | |
| ) | |
| @ExceptionDescription(SwaggerResponseDescription.APPROVE_FOOD_TRUCK_STATUS) | |
| @PatchMapping("/{foodTruckId}/approval") | |
| @PreAuthorize("hasRole('OPERATOR')") | |
| public BaseResponse<Void> approveFoodTruckStatus ( | |
| @PathVariable final Long foodTruckId, | |
| @Valid @RequestBody final ApproveFoodTruckStatusRequest request | |
| ) { | |
| foodTruckService.approveFoodTruckStatus(foodTruckId, request); | |
| return BaseResponse.ok(null); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java
around lines 54 to 66, the patch endpoint for changing food truck approval lacks
authorization; add the import
org.springframework.security.access.prepost.PreAuthorize and annotate the
controller method with @PreAuthorize (e.g., @PreAuthorize("hasRole('ADMIN')"))
directly above the method signature to restrict access to operators/admins,
keeping the method body unchanged.
...ain/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java
Outdated
Show resolved
Hide resolved
...ain/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java
Outdated
Show resolved
Hide resolved
...in/java/konkuk/chacall/domain/owner/presentation/dto/request/MyFoodTruckMenuListRequest.java
Show resolved
Hide resolved
| default CursorPagingRequest pagingOrDefault(SortType sort) { | ||
| CursorPagingRequest req = cursorPagingRequest(); | ||
|
|
||
| if (req == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, 20) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, 20); | ||
| } | ||
|
|
||
| if (req.cursor() == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, req.size()) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, req.size()); | ||
| } | ||
|
|
||
| if (req.size() == null) { | ||
| return new CursorPagingRequest(req.cursor(), 20); | ||
| } | ||
|
|
||
| return req; // 이미 커서 존재 | ||
| } |
There was a problem hiding this comment.
size=null일 때 잘못된 기본값 적용 (치명적 버그)
req.cursor()==null 분기에서 size가 null인 경우 그대로 null을 전달해 이후 처리에서 NPE/검증 실패가 발생할 수 있습니다. cursor/size를 한 번에 안전하게 기본값으로 보정하세요.
- default CursorPagingRequest pagingOrDefault(SortType sort) {
- CursorPagingRequest req = cursorPagingRequest();
-
- if (req == null) {
- return (sort == SortType.OLDEST)
- ? new CursorPagingRequest(0L, 20)
- : new CursorPagingRequest(Long.MAX_VALUE, 20);
- }
-
- if (req.cursor() == null) {
- return (sort == SortType.OLDEST)
- ? new CursorPagingRequest(0L, req.size())
- : new CursorPagingRequest(Long.MAX_VALUE, req.size());
- }
-
- if (req.size() == null) {
- return new CursorPagingRequest(req.cursor(), 20);
- }
-
- return req; // 이미 커서 존재
- }
+ default CursorPagingRequest pagingOrDefault(SortType sort) {
+ final CursorPagingRequest req = cursorPagingRequest();
+ final long defaultCursor = (sort == SortType.OLDEST) ? 0L : Long.MAX_VALUE;
+
+ if (req == null) {
+ return new CursorPagingRequest(defaultCursor, 20);
+ }
+
+ final Long cursor = (req.cursor() != null) ? req.cursor() : defaultCursor;
+ final Integer size = (req.size() != null) ? req.size() : 20;
+
+ return new CursorPagingRequest(cursor, size);
+ }📝 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.
| default CursorPagingRequest pagingOrDefault(SortType sort) { | |
| CursorPagingRequest req = cursorPagingRequest(); | |
| if (req == null) { | |
| return (sort == SortType.OLDEST) | |
| ? new CursorPagingRequest(0L, 20) | |
| : new CursorPagingRequest(Long.MAX_VALUE, 20); | |
| } | |
| if (req.cursor() == null) { | |
| return (sort == SortType.OLDEST) | |
| ? new CursorPagingRequest(0L, req.size()) | |
| : new CursorPagingRequest(Long.MAX_VALUE, req.size()); | |
| } | |
| if (req.size() == null) { | |
| return new CursorPagingRequest(req.cursor(), 20); | |
| } | |
| return req; // 이미 커서 존재 | |
| } | |
| default CursorPagingRequest pagingOrDefault(SortType sort) { | |
| final CursorPagingRequest req = cursorPagingRequest(); | |
| final long defaultCursor = (sort == SortType.OLDEST) ? 0L : Long.MAX_VALUE; | |
| if (req == null) { | |
| return new CursorPagingRequest(defaultCursor, 20); | |
| } | |
| final Long cursor = (req.cursor() != null) ? req.cursor() : defaultCursor; | |
| final Integer size = (req.size() != null) ? req.size() : 20; | |
| return new CursorPagingRequest(cursor, size); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/dto/HasPaging.java around lines 6
to 26, when req.cursor() == null the current branch can pass a null size through
(if req.size() is null) causing NPEs; change the method to normalize both cursor
and size at once: derive an effective size = (req != null && req.size() != null)
? req.size() : 20 and an effective cursor = (req == null || req.cursor() ==
null) ? (sort == SortType.OLDEST ? 0L : Long.MAX_VALUE) : req.cursor(), then
return a new CursorPagingRequest(effectiveCursor, effectiveSize); also handle
the case where req is null by using the same defaults.
There was a problem hiding this comment.
코래 말대로 cursor가 null일 때, size가 null이면 null이 넘어가지 않을까 우려되었는데 생각해보니 cursor가 null이면서 size가 null인 상황은 이미 앞에서 req == null인 분기로 예외처리가 되겠네요! 고려하지 않아도 될 것 같습니다.
There was a problem hiding this comment.
@buzz0331 좋은 분석이지만, 한 가지 놓친 부분이 있는 것 같습니다!
req == null 분기는 CursorPagingRequest 객체 자체가 null일 때만 처리됩니다. 하지만 문제가 되는 시나리오는 다음과 같습니다:
// 이런 경우가 문제입니다
CursorPagingRequest req = new CursorPagingRequest(null, null);이 경우:
req == null❌ (객체는 존재하므로 false)req.cursor() == null✅ (두 번째 분기로 진입)new CursorPagingRequest(0L, req.size())👈 여기서req.size()가 null
즉, 객체는 존재하지만 필드들이 null인 경우는 첫 번째 분기에서 걸러지지 않아서 여전히 size=null이 전달될 수 있습니다.
현재 코드에서는 cursor=null, size=null인 유효한 CursorPagingRequest 객체가 들어오면 NPE 위험이 있으니, 안전하게 처리하는 것이 좋을 것 같습니다!
/| /|
( :v: )
|(_)|
^ ^
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (2)
3-5: 와일드카드(*) import와 개별 import 혼용으로 인한 중복 제거request/response 패키지를 와일드카드로 import하면서, 동일 패키지의 타입(MyFoodTruckMenuListRequest, MyFoodTruckMenuResponse)을 개별 import로도 중복 선언하고 있습니다. 한쪽만 유지하세요. 현재 스타일을 유지한다면 아래처럼 개별 import를 제거하는 편이 변경 범위가 작습니다.
-import konkuk.chacall.domain.owner.presentation.dto.request.MyFoodTruckMenuListRequest; -import konkuk.chacall.domain.owner.presentation.dto.response.MyFoodTruckMenuResponse;참고: 장기적으로는 와일드카드 대신 명시적 import만 사용하는 일관된 규칙을 채택하면(예: Checkstyle/Spotless 설정) 타입 충돌 가능성을 줄일 수 있습니다.
Also applies to: 11-12
149-156: 등록 API는 생성 리소스 식별자/응답 반환 고려현재 void 반환입니다. 클라이언트 사용성(리디렉션, 후속 요청, 낙관적 업데이트)을 위해 생성된 메뉴 ID 또는 응답 DTO를 반환하는 것을 권장합니다. 엔드포인트 계약에 맞춘 선택적 개선 사항입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(5 hunks)src/main/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java
🔇 Additional comments (4)
src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java (4)
25-25: 클래스 레벨 readOnly 트랜잭션 설정 적절읽기 기본 + 쓰기 메서드에 한정된 @transactional 오버라이드 구성이 명확하고 안전합니다.
112-114: 예약 목록 기본 정렬을 NEWEST로 고정: 사양/클라이언트 정합성 확인 필요기본 정렬이 최신순으로 고정되었습니다. 동일 도메인의 다른 페이징 API 기본값과 맞는지, 그리고 정렬 방향에 따른 기본 커서(seed) 값이 올바르게 세팅되는지(예: asc일 때는 가장 작은 값, desc일 때는 가장 큰 값) 확인 부탁드립니다.
158-165: 이전 리뷰 지적(주석 오타) 반영 확인 — LGTM해당 구간의 주석이 실제 동작(메뉴 상태 변경)과 일치하도록 수정되었습니다.
141-165: MyFoodTruckMenuService에 필요한 검증 및 no-op 차단 로직이 모두 구현돼 있습니다
ownerId↔foodTruckId 검증(existsByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn), menuId↔foodTruckId 검증(findByMenuIdAndFoodTruckId), 동일 상태 전환 시 changeViewedStatus에서 예외 발생이 정상 작동합니다.
| public CursorPagingResponse<FoodTruckResponse> getFoodTrucks(FoodTruckSearchRequest request) { | ||
| return foodTruckSearchService.getFoodTrucks(request); | ||
| return foodTruckCommandService.getFoodTrucks(request); | ||
| } | ||
|
|
||
| public FoodTruckNameDuplicateCheckResponse isNameDuplicated(FoodTruckNameDuplicateCheckRequest request) { | ||
| return FoodTruckNameDuplicateCheckResponse.of( | ||
| foodTruckCommandService.isNameDuplicated(request.name())); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) { | ||
| foodTruckCommandService.approveFoodTruckStatus(foodTruckId, request); | ||
| } |
There was a problem hiding this comment.
p2: 여기 api에서 유저 권한 검증이 필요해보이네욥!
제가 api를 제대로 이해한게 맞다면 각 api의 권한은 다음과 같을 것 같습니다. 이에 따라서 Role enum에 관리자를 의미하는 것을 하나 추가해야 될 것 같네요,,
- 푸드트럭 조회 : 일반 유저 권한
- 푸드트럭 이름 중복 체크 : 사장님 권한
- 푸드트럭 승인 상태 변경 : 관리자 권한
There was a problem hiding this comment.
이 부분에서 관리자를 위해 권한을 만들면 고민해야할 부분이 존재해서 일단 미뤄두긴 했었습니다!
왜냐면 관리자 권한을 만들고 검증에 활용할 경우, 관리자 권한을 검증하기 위해서 관리자용 토큰을 만들어야할텐데, 이를 위해서는 관리자용 토큰 발급 API 같은 걸 별도로 뚫어둬야할 것 같더라구요.
그래서 이 부분에 대한 이야기가 선행되어야할 것 같았습니당. 관리자를 위한 토큰 처리는 어떻게 하는 게 좋을까요?
API 를 별도로 뚫거나, 로컬에서 실제 배포 서버와 jwt secret key 를 동일하게 설정해두고, 관리자 권한을 갖는 토큰을 만들어서 제공해주는 식도 가능할 것 같긴 한데 약간 고민이네유
There was a problem hiding this comment.
음 근데 현재 저희가 토큰에 Role 정보를 담고있지 않으니 관리자 또한 그냥 User 테이블에 row에 관리자라는 ENUM으로 정의해두면 되지 않을까여?? 어차피 관리자 계정은 하나뿐일테니 저희가 임의로 RDS에 넣어두고 사용하면 되지 않을까 싶긴한데.. 어떻게 생각하시나요?
There was a problem hiding this comment.
아 지금 토큰에 role 을 안담는구나 그럼 일단 그런 형태로 가시죵
| } | ||
|
|
||
| public boolean isNameDuplicated(String name) { | ||
| return foodTruckRepository.existsByNameIgnoreCase(name); |
There was a problem hiding this comment.
요구사항이 푸드트럭 이름을 대소문자를 구분하지 않는건가욥?? 예를 들어, Foodtruck이 이미 존재할때, foodtruck이 등록될 수 없는??
There was a problem hiding this comment.
이 부분에 대해서는 별 생각없이 구현하긴 했는데 PM 님께 한 번 여쭤보겠습니다~
| public void approveFoodTruck(FoodTruckStatus targetFoodTruckStatus) { | ||
|
|
||
| // 운영자 - 승인 대기 -> 승인 OR 승인 거부 | ||
| if (this.foodTruckStatus == FoodTruckStatus.PENDING && (targetFoodTruckStatus == FoodTruckStatus.OFF || targetFoodTruckStatus == FoodTruckStatus.REJECTED)) { | ||
| this.foodTruckStatus = targetFoodTruckStatus; | ||
| return; | ||
| } | ||
|
|
||
| // 운영자 - 승인 거부 -> 승인 | ||
| if (this.foodTruckStatus == FoodTruckStatus.REJECTED && targetFoodTruckStatus == FoodTruckStatus.OFF) { | ||
| this.foodTruckStatus = targetFoodTruckStatus; | ||
| return; | ||
| } | ||
|
|
||
| throw new DomainRuleException(ErrorCode.INVALID_FOOD_TRUCK_STATUS_TRANSITION); | ||
| } |
| default CursorPagingRequest pagingOrDefault(SortType sort) { | ||
| CursorPagingRequest req = cursorPagingRequest(); | ||
|
|
||
| if (req == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, 20) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, 20); | ||
| } | ||
|
|
||
| if (req.cursor() == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, req.size()) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, req.size()); | ||
| } | ||
|
|
||
| if (req.size() == null) { | ||
| return new CursorPagingRequest(req.cursor(), 20); | ||
| } | ||
|
|
||
| return req; // 이미 커서 존재 |
There was a problem hiding this comment.
오호 정렬 조건도 dto 내부에서 처리되도록 하셨네요. CursorPagingRequest 안에 필드로 SortType을 넣지 않고 파라미터로 받으신 이유는 기존에 구현되어 있는 조회 로직의 수정을 최소화하려는 목적인건가욥??
There was a problem hiding this comment.
네 맞습니다! sortType 을 내부로 넣을 경우 기존 코드들에서 변경이 다소 많아질 것 같더라구요.
현재와 같이 구현하면 기존에 존재하던, 최신순, 오래된순 정렬을 고려하지 않아도 되던 API 들은 수정할 것 없이 제 기능에만 분기처리를 해주면 되었어서 변경을 최소화하고자 현재와같이 구현했습니다!
| default CursorPagingRequest pagingOrDefault(SortType sort) { | ||
| CursorPagingRequest req = cursorPagingRequest(); | ||
|
|
||
| if (req == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, 20) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, 20); | ||
| } | ||
|
|
||
| if (req.cursor() == null) { | ||
| return (sort == SortType.OLDEST) | ||
| ? new CursorPagingRequest(0L, req.size()) | ||
| : new CursorPagingRequest(Long.MAX_VALUE, req.size()); | ||
| } | ||
|
|
||
| if (req.size() == null) { | ||
| return new CursorPagingRequest(req.cursor(), 20); | ||
| } | ||
|
|
||
| return req; // 이미 커서 존재 | ||
| } |
There was a problem hiding this comment.
코래 말대로 cursor가 null일 때, size가 null이면 null이 넘어가지 않을까 우려되었는데 생각해보니 cursor가 null이면서 size가 null인 상황은 이미 앞에서 req == null인 분기로 예외처리가 되겠네요! 고려하지 않아도 될 것 같습니다.
| public CursorPagingResponse<MyFoodTruckMenuResponse> getMyFoodTruckMenus(Long ownerId, Long foodTruckId, MyFoodTruckMenuListRequest request) { | ||
| SortType sort = SortType.fromNullable(request.sort()); | ||
| CursorPagingRequest pagingRequest = request.pagingOrDefault(sort); | ||
| Pageable pageable = PageRequest.of(0, pagingRequest.size()); | ||
|
|
||
| // 본인 소유인지, 푸드트럭이 승인 완료된 상태인지 검증 | ||
| if (!foodTruckRepository.existsByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn(foodTruckId, ownerId, List.of(FoodTruckStatus.ON, FoodTruckStatus.OFF))) { | ||
| throw new BusinessException(ErrorCode.FOOD_TRUCK_NOT_APPROVED); | ||
| } | ||
|
|
||
| Slice<Menu> menuSlice = switch (sort) { | ||
| case NEWEST -> menuRepository.findMenusDesc(foodTruckId, pagingRequest.cursor(), pageable); | ||
| case OLDEST -> menuRepository.findMenusAsc(foodTruckId, pagingRequest.cursor(), pageable); | ||
| }; | ||
|
|
||
| List<MyFoodTruckMenuResponse> content = menuSlice.getContent().stream() | ||
| .map(MyFoodTruckMenuResponse::from) | ||
| .toList(); | ||
|
|
||
| return CursorPagingResponse.of(content, MyFoodTruckMenuResponse::menuId, menuSlice.hasNext()); | ||
| } | ||
|
|
||
| public void registerMenu(Long ownerId, Long foodTruckId, RegisterMenuRequest request) { | ||
|
|
||
| // 본인 소유인지, 푸드트럭이 승인 완료된 상태인지 검증 | ||
| FoodTruck foodTruck = foodTruckRepository.findByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn( | ||
| foodTruckId, ownerId, List.of(FoodTruckStatus.ON, FoodTruckStatus.OFF)) | ||
| .orElseThrow(() -> new BusinessException(ErrorCode.FOOD_TRUCK_NOT_APPROVED)); | ||
|
|
||
| Menu menu = Menu.create( | ||
| request.name(), | ||
| request.price(), | ||
| request.description(), | ||
| request.photoUrl(), | ||
| foodTruck); | ||
|
|
||
| menuRepository.save(menu); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (1)
17-22: 승인 서비스 메서드에 트랜잭션 경계를 명시하세요
approveFoodTruckStatus가 엔티티 상태를 변경하지만 현재는 호출자가 트랜잭션을 열어 줄 것이라는 가정에 의존합니다. 서비스 자체에@Transactional을 붙여 두어야 다른 진입점에서도 안전하게 동작합니다.+import org.springframework.transaction.annotation.Transactional; ... - public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) { + @Transactional + public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) {src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (1)
16-16: 네이밍: 읽기 로직은 Query/Read 서비스로 분리 권장CommandService가 조회(getFoodTrucks)도 담당하고 있어 혼재되어 보입니다. Query/Command 분리를 검토해 주세요.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (3)
51-56: 소유자 권한 가드 추가 권장(@PreAuthorize)Service에서 OwnerValidator로 2차 검증하지만, 컨트롤러 레벨 롤 가드가 방어선과 문서화에 유용합니다.
@PostMapping("/duplicate-check") + @PreAuthorize("hasRole('OWNER')") public BaseResponse<FoodTruckNameDuplicateCheckResponse> isNameDuplicated( @Valid @RequestBody final FoodTruckNameDuplicateCheckRequest request, @Parameter(hidden = true) @UserId final Long ownerId ) {파일 상단 import(선택):
import org.springframework.security.access.prepost.PreAuthorize;
51-56: 중복 체크는 멱등(GET)으로 노출 권장단순 조회 성격이므로 GET /food-trucks/duplicate-check?name=... 형태가 REST 관점에서 자연스럽습니다. 추후 캐싱/레이트리밋 적용도 용이합니다.
26-26: 사용되지 않는 로거 제거 제안@slf4j가 사용되지 않습니다. 불필요한 어노테이션 제거를 권장합니다.
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
31-36: 이름 중복 체크: 수정 시 자기 자신의 트럭 제외 지원 고려향후 이름 변경 API에서 자기 리소스 제외(excludeId) 파라미터를 허용하지 않으면 오탐이 발생합니다. 확장 여지를 열어두길 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/user/application/UserService.java(3 hunks)src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java(1 hunks)src/main/java/konkuk/chacall/domain/user/presentation/UserController.java(2 hunks)src/main/java/konkuk/chacall/domain/user/presentation/dto/request/ApproveFoodTruckStatusRequest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (2)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (1)
Tag(18-66)src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
RequiredArgsConstructor(15-39)
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
Service(17-44)
🔇 Additional comments (8)
src/main/java/konkuk/chacall/domain/user/presentation/dto/request/ApproveFoodTruckStatusRequest.java (1)
7-10: Swagger 예제 허용 상태와 일치시키세요도메인 전환 규칙이
PENDING → OFF/REJECTED,REJECTED → OFF로 제한되어 있는데도 예제값을ON으로 두면 문서만 보고 호출한 클라이언트가 항상 예외를 맞습니다. 허용 상태 중 하나로 예제를 맞춰 주세요.- @Schema(description = "변경할 푸드트럭 승인 상태", example = "ON") + @Schema(description = "변경할 푸드트럭 승인 상태", example = "OFF")src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
40-43: AdminService 위임으로 수정 트랜잭션 범위가 명확해졌습니다Line 41에서 클래스 기본 readOnly 트랜잭션을 해제하고 AdminService에 상태 변경을 위임한 구조가 깔끔합니다. 이런 식으로 쓰기 전용 흐름을 명시해 두면 추후 유지보수 시에도 의도가 명확히 드러날 것 같아요.
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (1)
53-65: 운영자 전용 권한 검증 위치를 한 번 확인해 주세요Line 58의
/users/admin/...엔드포인트에는 별도의 권한/역할 어노테이션이나@UserId와 같은 인증 정보 주입이 없어 보입니다. Spring Security 설정 혹은 인터셉터에서 이 경로를 운영자 권한으로 제한하고 있는지 확인 부탁드립니다. 그렇지 않다면 일반 사용자도 호출할 수 있는 여지가 있습니다.src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (2)
34-36: 이름 중복 체크: 대소문자 무시 요구사항 확정 + 입력 정규화(트림) 제안IgnoreCase 사용은 합리적입니다. 다만 요구사항(대소문자 구분 여부) 확정을 부탁드립니다. 또한 앞뒤 공백 차이로 다른 문자열로 판정되는 것을 막기 위해 트림을 권장합니다.
적용 예:
- public boolean isNameDuplicated(String name) { - return foodTruckRepository.existsByNameIgnoreCase(name); - } + public boolean isNameDuplicated(final String name) { + final String normalized = name == null ? null : name.trim(); + return foodTruckRepository.existsByNameIgnoreCase(normalized); + }DTO에 @notblank 등 검증이 있다면 null/blank는 사전 차단됩니다. 이전 코멘트 맥락.
20-32: FoodTruckSearchRepositoryImpl#getFoodTrucks는 foodTruckId 기준 단일 정렬만 사용
createdAt 기반 2차 정렬이나 ASC/DESC 전환에 따른 커서 키 불일치 이슈는 없습니다.Likely an incorrect or invalid review comment.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (1)
38-43: 푸드트럭 조회가 로그인 필수인지 확인 필요현재 @userid memberId를 요구하고 Service에서 회원 검증을 수행합니다. 해당 엔드포인트가 공개 조회용이라면 인증 강제가 맞는지 기획 확인 부탁드립니다(공개/마이페이지 엔드포인트 분리 여부).
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (2)
20-24: Validator 주입 및 CommandService 교체 LGTM권한 경계가 Service 레이어에서 선제 검증되어 일관적입니다.
25-29: 회원 검증 후 조회 위임 구조 적절읽기 트랜잭션(readOnly)과 멤버 검증 흐름이 명확합니다.
| public boolean isNameDuplicated(String name) { | ||
| return foodTruckRepository.existsByNameIgnoreCase(name); | ||
| } |
There was a problem hiding this comment.
TOCTOU 위험: API 레벨 중복체크만으로는 무결성 불충분 → DB 유니크 제약 필요
등록 시 경쟁 상태에서 중복이 삽입될 수 있습니다. 이름 정규화(소문자/트림) 기준으로 DB 유니크 제약을 추가해 데이터 무결성을 보강하세요.
예시(PostgreSQL):
-- 공백 트림만 고려(소문자 기반)
CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS ux_food_truck_name_ci
ON food_truck (lower(name));
-- 가능하면 citext 사용
-- ALTER TABLE food_truck ALTER COLUMN name TYPE citext;
-- CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS ux_food_truck_name_citext ON food_truck (name);정규화 정책(트림/다중 공백 축약 등)에 맞춘 함수 인덱스를 선택하세요.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java
around lines 34-36, the current API-level duplicate name check is vulnerable to
TOCTOU races; add a database-level unique constraint/index that enforces the
service's normalization policy (e.g., lowercasing and trimming/whitespace
normalization) so concurrent inserts cannot create duplicates. Update the schema
by creating a functional unique index that matches your normalization (for
example an index on lower(trim(...)) or convert the column to citext and add a
unique index), and ensure your persistence code handles unique-constraint
violations (catch the DB exception and translate to a domain-level
duplicate-name error) so the service returns a proper response when the DB
rejects duplicates.
| @Operation( | ||
| summary = "푸드트럭 이름 중복 체크", | ||
| description = "푸드트럭 이름 중복 여부를 체크합니다." | ||
| ) | ||
| @ExceptionDescription(SwaggerResponseDescription.DEFAULT) | ||
| @PostMapping("/duplicate-check") | ||
| public BaseResponse<FoodTruckNameDuplicateCheckResponse> isNameDuplicated( | ||
| @Valid @RequestBody final FoodTruckNameDuplicateCheckRequest request, | ||
| @Parameter(hidden = true) @UserId final Long ownerId | ||
| ) { | ||
| return BaseResponse.ok(foodTruckService.isNameDuplicated(ownerId, request)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
운영자 승인 엔드포인트 권한 가드 존재 여부 점검
해당 파일엔 없지만, PR 맥락상 승인 API가 UserController 등으로 이동했습니다. @PreAuthorize가 누락되지 않았는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 승인 엔드포인트(@PatchMapping .../approval)와 권한가드(@PreAuthorize) 동시 존재 여부 점검
rg -n -C2 --type=java '@PatchMapping\(".*approval"\)'
rg -n -C2 --type=java '@PreAuthorize\('
# 같은 파일 내에 둘 다 있는지 수동 대조Length of output: 638
운영자 승인 엔드포인트에 권한 가드(@PreAuthorize) 추가 필요
UserController의 approveFoodTruckStatus 메서드
(src/main/java/konkuk/chacall/domain/user/presentation/UserController.java:58)
– @PatchMapping("/admin/food-trucks/{foodTruckId}/approval")에 @PreAuthorize(예: hasRole('ADMIN')) 어노테이션이 누락되어 있습니다. 운영자 권한 검증을 추가하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/user/presentation/UserController.java
around line 58, the admin approval endpoint for food trucks is missing an
authorization guard; add @PreAuthorize("hasRole('ADMIN')") above the
@PatchMapping("/admin/food-trucks/{foodTruckId}/approval") method, import
org.springframework.security.access.prepost.PreAuthorize, and ensure method
security is enabled in configuration (e.g.,
@EnableGlobalMethodSecurity(prePostEnabled = true) or the equivalent for your
Spring Security version) so only users with the ADMIN role can call this
endpoint.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java
| @Query(""" | ||
| select f | ||
| from FoodTruck f | ||
| where f.foodTruckId = :foodTruckId | ||
| and f.owner.userId = :ownerId | ||
| and f.foodTruckStatus in :statuses | ||
| """) | ||
| Optional<FoodTruck> findByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn( | ||
| @Param("foodTruckId") Long foodTruckId, | ||
| @Param("ownerId") Long ownerId, | ||
| @Param("statuses") Collection<FoodTruckStatus> statuses); | ||
|
|
||
| @Query(""" | ||
| select (count(f) > 0) | ||
| from FoodTruck f | ||
| where f.foodTruckId = :foodTruckId | ||
| and f.owner.userId = :ownerId | ||
| and f.foodTruckStatus in :statuses | ||
| """) | ||
| boolean existsByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn( | ||
| @Param("foodTruckId") Long foodTruckId, | ||
| @Param("ownerId") Long ownerId, | ||
| @Param("statuses") Collection<FoodTruckStatus> statuses | ||
| ); |
There was a problem hiding this comment.
빈 컬렉션 파라미터에 대한 방어 로직 추가 필요
f.foodTruckStatus in :statuses 조건이 두 메서드에 공통으로 들어가 있는데, statuses가 빈 컬렉션이면 Hibernate/JPA가 IN () 구문을 만들면서 SQLGrammarException을 던지는 사례가 다수 보고되어 있습니다. 호출부에서 상태 목록을 항상 한 개 이상 담도록 강제하거나, 빈 목록이면 즉시 실패/조기 반환하도록 방어 로직을 추가해야 합니다.(github.com)
- 오름차순 / 내림차순 모두 구현
ksg1227
force-pushed
the
feat/#32-food-truck-menu-save-and-get
branch
from
05dff25 to
266d990
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/user/domain/model/User.java (1)
66-71: 운영자 권한 검증 메서드 추가: OK
validateOwner,validateMember와 일관된 예외 모델을 유지합니다. 메시지 로깅/감사 필요 시userId를 원인에 함께 남기는 것도 고려해볼 수 있습니다(클라이언트 노출은 유지).src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java (1)
97-112: 승인 전이 규칙 구현은 명확합니다. 의미를 드러내는 네이밍/문서화 보완 권장
PENDING -> OFF | REJECTED,REJECTED -> OFF제약이 코드에 잘 반영되었습니다. 다만OFF가 “승인(Approved but Off)” 상태를 의미한다는 도메인 맥락이 코드만 보면 직관적이지 않습니다. 메서드/상태명 또는 Javadoc/주석으로 의미를 명확히 해두면 유지보수 가독성이 좋아집니다.예시:
- public void approveFoodTruck(FoodTruckStatus targetFoodTruckStatus) { + /** + * 운영자의 승인 판단을 적용한다. + * 허용 전이: + * - PENDING -> OFF(승인) 또는 REJECTED(승인거부) + * - REJECTED -> OFF(재승인) + * 그 외에는 INVALID_FOOD_TRUCK_STATUS_TRANSITION + */ + public void approveFoodTruck(FoodTruckStatus targetFoodTruckStatus) {또는 메서드명을
applyApprovalDecision처럼 바꾸는 것도 고려 가능합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/MenuRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/infra/FoodTruckSearchRepositoryImpl.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/value/MenuViewedStatus.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/FoodTruckNameDuplicateCheckResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/member/application/MemberService.java(2 hunks)src/main/java/konkuk/chacall/domain/owner/application/OwnerService.java(5 hunks)src/main/java/konkuk/chacall/domain/owner/application/myfoodtruckmenu/MyFoodTruckMenuService.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/MyFoodTruckMenuListRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterMenuRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateMenuStatusRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/MyFoodTruckMenuResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/user/application/UserService.java(3 hunks)src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java(1 hunks)src/main/java/konkuk/chacall/domain/user/application/validator/AdminValidator.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/model/Role.java(1 hunks)src/main/java/konkuk/chacall/domain/user/domain/model/User.java(1 hunks)src/main/java/konkuk/chacall/domain/user/presentation/UserController.java(2 hunks)src/main/java/konkuk/chacall/domain/user/presentation/dto/request/ApproveFoodTruckStatusRequest.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/CursorPagingRequest.java(0 hunks)src/main/java/konkuk/chacall/global/common/dto/HasPaging.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/SortType.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(2 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/konkuk/chacall/global/common/dto/CursorPagingRequest.java
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/infra/FoodTruckSearchRepositoryImpl.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/RegisterMenuRequest.java
- src/main/java/konkuk/chacall/domain/owner/presentation/OwnerController.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/response/MyFoodTruckMenuResponse.java
- src/main/java/konkuk/chacall/domain/foodtruck/domain/value/MenuViewedStatus.java
- src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/UpdateMenuStatusRequest.java
- src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/FoodTruckNameDuplicateCheckRequest.java
- src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/MenuRepository.java
- src/main/java/konkuk/chacall/domain/user/presentation/dto/request/ApproveFoodTruckStatusRequest.java
- src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/konkuk/chacall/domain/user/application/validator/AdminValidator.java (1)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (2)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (1)
Tag(18-67)src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
RequiredArgsConstructor(15-39)
src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java (2)
src/main/java/konkuk/chacall/domain/member/domain/Rating.java (1)
Entity(14-49)src/main/java/konkuk/chacall/domain/foodtruck/domain/model/FoodTruck.java (1)
Builder(16-120)
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (2)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
Service(18-48)
🔇 Additional comments (9)
src/main/java/konkuk/chacall/domain/member/application/MemberService.java (1)
12-12: SortType 도입 import: OK기본 정렬 정책을 코드로 명시한 점 좋습니다.
src/main/java/konkuk/chacall/domain/user/domain/model/Role.java (1)
7-11: ADMIN 상수 추가로 인한 호환성/보안 매핑 확인
- 현재
@Enumerated(EnumType.STRING)이라 DB ordinal 변경 위험은 없습니다. OK.- 보안 계층(Spring Security 등)에서 권한 문자열이
ROLE_ADMIN을 기대하는지, 도메인Role.ADMIN과의 매핑 규칙을 한번 점검해주세요(권한 주입/프리Authorize 표현식 등).src/main/java/konkuk/chacall/domain/user/application/validator/AdminValidator.java (1)
17-22: 관리자 검증 로직 모듈화 👍ACTIVE 상태 조회 뒤
validateAdmin()으로 역할을 위임해 서비스 레이어가 간결해졌습니다. 잘 반영해주셨습니다.src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/FoodTruckNameDuplicateCheckResponse.java (1)
5-11: 응답 DTO 구성이 명확합니다boolean 필드 하나에 스키마 메타데이터를 붙이고
of정적 팩토리로 일관성 있게 감싸준 점이 좋습니다.src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
42-47: 관리자 승인 흐름 연결 방식이 자연스럽습니다
@Transactional범위를 재정의하고 관리자 검증 뒤 전용 서비스에 위임한 구조 덕분에 책임이 명확해졌습니다.src/main/java/konkuk/chacall/domain/foodtruck/application/command/FoodTruckCommandService.java (1)
34-36: API 중복검사만으로는 동시 등록 경쟁을 막지 못합니다
existsByName()을 선행 검사로 두면 두 요청이 동시에 통과해 중복 레코드가 생길 수 있습니다. 이름 정규화 정책(소문자/trim 등)에 맞춘 DB 유니크 제약과, 위반 시DataIntegrityViolationException을 잡아 도메인 오류로 매핑하는 처리가 필요합니다. 이전 리뷰에서 지적된 사항이 아직 반영되지 않아 재차 남겨둡니다.src/main/java/konkuk/chacall/domain/owner/presentation/dto/request/MyFoodTruckMenuListRequest.java (1)
12-27: Swagger 예시값과 실제 enum 바인딩이 불일치합니다현재 Spring MVC는
SortType의 enum 이름(예:NEWEST)만 바인딩합니다. PR에서Converter<String, SortType>이나ConverterFactory가 등록되지 않아 문서 예시인 “최신순/오래된순”으로 호출하면 400이 발생합니다. 예시·allowableValues를 enum 이름으로 맞추거나, 한글 값을 지원할 커스텀 컨버터를 추가해 주세요. 이전 리뷰 피드백이어서 재차 남깁니다.src/main/java/konkuk/chacall/global/common/dto/HasPaging.java (1)
6-25: 커서 기본값 분기에서 size=null 그대로 전달됩니다.
cursorPagingRequest()가new CursorPagingRequest(null, null)처럼 필드만 null인 객체를 돌려주면 Line 16 분기로 들어가면서req.size()가 그대로 null로 전달되어 후속 로직에서 NPE나 검증 실패가 발생합니다. 커서·사이즈를 동시에 정규화해 주셔야 합니다. 아래처럼 기본 커서와 기본 사이즈를 먼저 계산한 뒤, 보정된 값으로 새 요청을 만들어 주세요.- CursorPagingRequest req = cursorPagingRequest(); - - if (req == null) { - return (sort == SortType.OLDEST) - ? new CursorPagingRequest(0L, 20) - : new CursorPagingRequest(Long.MAX_VALUE, 20); - } - - if (req.cursor() == null) { - return (sort == SortType.OLDEST) - ? new CursorPagingRequest(0L, req.size()) - : new CursorPagingRequest(Long.MAX_VALUE, req.size()); - } - - if (req.size() == null) { - return new CursorPagingRequest(req.cursor(), 20); - } - - return req; + final CursorPagingRequest req = cursorPagingRequest(); + final long defaultCursor = (sort == SortType.OLDEST) ? 0L : Long.MAX_VALUE; + final int defaultSize = 20; + + if (req == null) { + return new CursorPagingRequest(defaultCursor, defaultSize); + } + + final long cursor = (req.cursor() != null) ? req.cursor() : defaultCursor; + final int size = (req.size() != null) ? req.size() : defaultSize; + + return new CursorPagingRequest(cursor, size);src/main/java/konkuk/chacall/domain/foodtruck/domain/model/Menu.java (1)
60-66: null 상태 입력 방어 로직 추가 필요
menuViewedStatus컬럼이nullable = false인데도 null 값에 대한 방어가 없어, 잘못된 요청이 들어오면 JPA flush 시점에 DB 제약 위반으로 500이 발생합니다. 도메인 규칙 위반을 명시적으로 막아야 합니다.public void changeViewedStatus(MenuViewedStatus targetViewedStatus) { - if(this.menuViewedStatus == targetViewedStatus) { + if (targetViewedStatus == null) { + throw new DomainRuleException(ErrorCode.INVALID_MENU_STATUS_TRANSITION); + } + + if (this.menuViewedStatus == targetViewedStatus) { throw new DomainRuleException(ErrorCode.INVALID_MENU_STATUS_TRANSITION); } this.menuViewedStatus = targetViewedStatus; }
| CursorPagingRequest cursorPagingRequest = request.pagingOrDefault(SortType.NEWEST); | ||
|
|
||
| return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
기본 정렬 NEWEST 설정은 좋지만, SortType이 서비스로 전달되지 않습니다
pagingOrDefault(SortType.NEWEST)로 기본값을 정했지만 아래 호출에서 cursor, size만 넘겨 정렬 정보가 손실될 수 있습니다. 서비스/레포지토리 단에서 정렬이 반영되는지 확인하거나 시그니처를 확장하는 것을 권장합니다.
두 가지 대안 중 하나를 검토해주세요.
- return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size());
+ // 대안 A: CursorPagingRequest 전체를 넘기도록 서비스 시그니처 확장
+ return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest);- return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size());
+ // 대안 B: 정렬 타입을 명시적으로 전달
+ return memberReservationService.getMemberReservations(
+ member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size(), cursorPagingRequest.sortType()
+ );검증 스크립트(서비스 시그니처 확인):
🏁 Script executed:
#!/bin/bash
# MemberReservationService의 getMemberReservations 시그니처와 SortType 사용 여부 점검
rg -nP --type=java -C3 '(class\s+MemberReservationService\b)|(\bgetMemberReservations\s*\()|SortType' src/main/javaLength of output: 17079
🏁 Script executed:
#!/bin/bash
# ReservationRepository 구현에서 정렬 처리 확인
file=$(find src/main/java -name "ReservationRepository.java")
echo "Repository file: $file"
rg -n -C5 "findMemberReservationsByStatusWithCursor" "$file"Length of output: 645
MemberService.getMemberReservations 호출부에 SortType 전달 누락
MemberReservationService.getMemberReservations(User, ReservationViewType, Long, int) 시그니처 및 ReservationRepository.findMemberReservationsByStatusWithCursor의 JPQL이 ORDER BY r.reservationId DESC로 하드코딩되어 있어 SortType이 반영되지 않습니다. 서비스 또는 레포지토리 시그니처를 확장하세요.
대안 A: CursorPagingRequest 전체 전달
- return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size());
+ return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest);대안 B: SortType 명시적 전달
- return memberReservationService.getMemberReservations(member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size());
+ return memberReservationService.getMemberReservations(
+ member, request.viewType(), cursorPagingRequest.cursor(), cursorPagingRequest.size(), cursorPagingRequest.sortType()
+ );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/member/application/MemberService.java
around lines 65-68, the call to memberReservationService.getMemberReservations
does not pass SortType so the repository always uses ORDER BY r.reservationId
DESC; update the call and service/repository signatures to propagate sort
information. Change MemberReservationService.getMemberReservations to accept
either the whole CursorPagingRequest or an explicit SortType parameter
(preferred: add a SortType arg), update MemberService to pass request.sortType()
(or the CursorPagingRequest) and then update ReservationRepository/its JPQL to
respect the passed SortType (e.g., branch query/order or build dynamic ORDER BY)
so the requested sort order is applied.
| public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) { | ||
| FoodTruck foodTruck = foodTruckRepository.findById(foodTruckId) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.FOOD_TRUCK_NOT_FOUND)); | ||
|
|
||
| foodTruck.approveFoodTruck(request.status()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
쓰기 로직에 @transactional 추가 필요
현재는 상위 UserService.approveFoodTruckStatus(메서드 수준 @transactional)에서 호출되어 안전하지만, 본 서비스가 다른 경로에서 호출될 경우 변경사항이 flush되지 않을 수 있습니다. 방어적으로 트랜잭션을 부여해 주세요.
package konkuk.chacall.domain.user.application.admin;
import konkuk.chacall.domain.foodtruck.domain.model.FoodTruck;
import konkuk.chacall.domain.foodtruck.domain.repository.FoodTruckRepository;
import konkuk.chacall.domain.user.presentation.dto.request.ApproveFoodTruckStatusRequest;
import konkuk.chacall.global.common.exception.EntityNotFoundException;
import konkuk.chacall.global.common.exception.code.ErrorCode;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
@Service
@RequiredArgsConstructor
public class AdminService {
private final FoodTruckRepository foodTruckRepository;
+ @Transactional
public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) {
FoodTruck foodTruck = foodTruckRepository.findById(foodTruckId)
.orElseThrow(() -> new EntityNotFoundException(ErrorCode.FOOD_TRUCK_NOT_FOUND));
foodTruck.approveFoodTruck(request.status());
}
}📝 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 void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) { | |
| FoodTruck foodTruck = foodTruckRepository.findById(foodTruckId) | |
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.FOOD_TRUCK_NOT_FOUND)); | |
| foodTruck.approveFoodTruck(request.status()); | |
| } | |
| package konkuk.chacall.domain.user.application.admin; | |
| import konkuk.chacall.domain.foodtruck.domain.model.FoodTruck; | |
| import konkuk.chacall.domain.foodtruck.domain.repository.FoodTruckRepository; | |
| import konkuk.chacall.domain.user.presentation.dto.request.ApproveFoodTruckStatusRequest; | |
| import konkuk.chacall.global.common.exception.EntityNotFoundException; | |
| import konkuk.chacall.global.common.exception.code.ErrorCode; | |
| import lombok.RequiredArgsConstructor; | |
| import org.springframework.stereotype.Service; | |
| import org.springframework.transaction.annotation.Transactional; | |
| @Service | |
| @RequiredArgsConstructor | |
| public class AdminService { | |
| private final FoodTruckRepository foodTruckRepository; | |
| @Transactional | |
| public void approveFoodTruckStatus(Long foodTruckId, ApproveFoodTruckStatusRequest request) { | |
| FoodTruck foodTruck = foodTruckRepository.findById(foodTruckId) | |
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.FOOD_TRUCK_NOT_FOUND)); | |
| foodTruck.approveFoodTruck(request.status()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java
around lines 17 to 22, the approveFoodTruckStatus method mutates an entity but
lacks its own @Transactional annotation, which can cause changes not to be
flushed when called outside a transactional caller; add a method-level
@Transactional (importing
org.springframework.transaction.annotation.Transactional) to ensure the
persistence context is active and changes are flushed/committed, or
alternatively annotate the class if all methods should be transactional, keeping
propagation and readOnly defaults appropriate for a write operation.
| @Operation( | ||
| summary = "푸드트럭 승인 상태 변경", | ||
| description = "운영자 - 푸드트럭 승인 상태를 변경합니다." | ||
| ) | ||
| @ExceptionDescription(SwaggerResponseDescription.APPROVE_FOOD_TRUCK_STATUS) | ||
| @PatchMapping("/admin/food-trucks/{foodTruckId}/approval") | ||
| public BaseResponse<Void> approveFoodTruckStatus ( | ||
| @PathVariable final Long foodTruckId, | ||
| @Valid @RequestBody final ApproveFoodTruckStatusRequest request | ||
| ) { | ||
| userService.approveFoodTruckStatus(foodTruckId, request); | ||
| return BaseResponse.ok(null); | ||
| } |
|
|
||
| public boolean isNameDuplicated(String name) { | ||
| return foodTruckRepository.existsByNameIgnoreCase(name); | ||
| return foodTruckRepository.existsByName(name); |
|
|
||
| // 본인 소유인지, 푸드트럭이 승인 완료된 상태인지 검증 | ||
| if(!foodTruckRepository.existsByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn(ownerId, foodTruckId, List.of(FoodTruckStatus.ON, FoodTruckStatus.OFF))) { | ||
| if (!foodTruckRepository.existsByFoodTruckIdAndOwnerIdAndFoodTruckStatusIn(foodTruckId, ownerId, List.of(FoodTruckStatus.ON, FoodTruckStatus.OFF))) { |
| public void validateAdmin() { | ||
| if (this.role != Role.ADMIN) { | ||
| throw new DomainRuleException(ErrorCode.USER_FORBIDDEN, | ||
| new IllegalArgumentException("사용자는 운영자 권한이 없습니다. " + this.role)); | ||
| } | ||
| } | ||
|
|
2 participants
#️⃣연관된 이슈
📝작업 내용
푸드트럭 메뉴 등록/조회, 그리고 간단한 CRUD 기능들을 구현하였습니다.
나의 푸드트럭 메뉴 등록
푸드트럭 메뉴 등록은 간단한 insert 작업이었기에 이슈는 없었습니다.
나의 푸드트럭 메뉴 조회
나의 푸드트럭 메뉴 조회에는 두 가지정도의 중요한 이슈가 존재했습니다.
우선 나의 푸드트럭 메뉴를 조회할 때에는 최신순, 혹은 오래된순으로 순서를 바꿔가며 데이터를 보여줄 필요가 있었습니다. 또한 여기에 페이징 로직이 적용되어야했는데, 저희의 페이징 로직 구현 방식을 확인해보면, CursorPagingRequest 객체가 null 일경우, cursor 값과 size 값을 디폴트로 Long.MAX_VALUE, 20 으로 설정해주고있었습니다.
이 때 최신순으로 데이터를 보여줄 경우, cursor 의 디폴트값을 Long.MAX_VALUE 로 하더라도 foodTruck.id < cursor 와 같은 형식으로 조회가 들어가기 때문에 문제가 없었습니다.
하지만 오래된 순으로 데이터를 보여줄 경우, foodTruck.id > cursor 와 같이, PK 가 낮은 쪽부터 오른차순으로 데이터를 보여줄 필요가 있었습니다. 이 케이스에서 만약 cursor의 기본값을 Long.MAX_VALUE 로 설정해버린다면,
foodTruck.id > Long.MAX_VALUE와 같은 형식으로 조건이 들어가버려서 데이터를 정상적으로 조회할 수 없는 문제가 발생했습니다.그래서 우선 현재는 SortType 이라는 enum 을 만들어서 오름차순, 내림차순으로 정렬 형식을 정해두고 이 SortType 에 맞게 디폴트값을 지정해주는 식으로 구현하였습니다.
그 다음으로 일반 유저가 메뉴를 조회할 때와 사장님이 자신의 푸드트럭의 메뉴를 조회할 때에는 화면에 보여주어야할 데이터에 차이가 존재했습니다.
사장님의 경우, 자신의 푸드트럭에 등록된 메뉴들을 노출시킬수도 있고, 고객으로부터 감출수 있는 기능이 존재했고, 해당 버튼을 보여줄 필요가 있었습니다.
따라서 이 메뉴 표시 여부를 응답값에 포함시켜주어야해서, 본래 푸드트럭 도메인 하위에 푸드트럭 메뉴 조회 API 를 만들어두었던 것에서,
푸드트럭 도메인 -> 푸드트럭 메뉴 조회 (일반 유저용) && 사장님 도메인 -> [마이페이지] 나의 푸드트럭 메뉴 조회 API 로 총 2개의 API 를 만들어두었습니다.
나머지 CRUD
푸드트럭 이름 중복 체크-> 이슈가 전혀 없었기에 패스하겠습니다.푸드트럭 메뉴 표시 여부 변경-> 앞서 말했던 것처럼 사장님이 자신의 푸드트럭에 등록된 메뉴의 표시 여부를 변경할 때 사용하는 API 입니다.표시(ON) -> 표시(ON) 혹은 미표시(OFF) -> 미표시(OFF) 와 같이 동일한 상태로 변경하는 케이스는 허용하지 않도록 구현하였습니다.
푸드트럭 승인 여부 변경-> 운영자가 사용할 기능입니다.푸드트럭 상태 변경이라는 API 하나로 운영자 & 사장님 (푸드트럭 자체 표시 여부 변경(메뉴 아님)) 이 모두 활용할 수 있도록 구현할까 하다가, 사장님이 푸드트럭의 상태를 OFF -> ON 으로 바꾸려면 푸드트럭의 모든 정보를 기입했는지 여부를 판단한 뒤에 상태 변경이 가능하도록 할 생각이어서 우선은 API 를 두 개로 분리할 생각입니다. 따라서 현재는
상태로의 변경만 가능하도록 구현해둔 상태입니다.
스크린샷 (선택)
💬리뷰 요구사항(선택)
현재 푸드트럭 승인 여부 변경 API 는 운영자를 위한 기능인데, 추후에 푸드트럭의 표시 여부를 변경하는 API 가 추가될 것 같습니다.
이 푸드트럭 표시 여부 는 사장님이 고객에게 자신의 푸드트럭을 노출 시킬지 말지를 결정하는 칼럼이라서, 이 부분과는 차이를 두어야한다고 판단했습니다.
그래서 생각이 들었던 게, 차라리 admin 과 같은 도메인을 따로 하나 만들어서 푸드트럭을 승인해주는 API 를 이쪽으로 옮겨버릴까 싶기도 합니다.
뭔가 모두다 푸드트럭 도메인 아래에 있으면 약간 헷갈릴수도 있겠더라구요. 그래서 이 부분에 대한 현준님의 의견이 궁금합니다!
Summary by CodeRabbit