Merged
Conversation
Contributor
WalkthroughOrderController에 GET /api/orders 엔드포인트가 추가되어 키워드·날짜·페이징 파라미터로 주문 이력을 조회합니다. 관련 DTO(요약형 응답), Converter, Repository(페이징 쿼리), Service 로직 및 전역 예외/에러 응답 생성자 변경이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as OrderController
participant Service as OrderService
participant Repo as OrderRepository
participant DB as Database
participant Conv as OrderConverter
Client->>Controller: GET /api/orders (keyword,startDate,endDate,page,size)
Controller->>Controller: validate params, build Pageable
Controller->>Service: getOrderHistory(userId, keyword, startDate, endDate, pageable)
Service->>Repo: findOrderHistoryWithCount(userId, keyword, startDateTime, endDateTime, pageable)
Repo->>DB: execute paged query (fetch join + count)
DB-->>Repo: rows + total count
Repo-->>Service: Page<Order>
Service->>Conv: toHistoryResponse(Page<Order>)
Conv-->>Service: OrderHistoryResponse
Service-->>Controller: OrderHistoryResponse
Controller-->>Client: 200 OK (DataResponse<OrderHistoryResponse>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ongil/backend/domain/order/controller/OrderController.java`:
- Around line 54-60: Validate the incoming page and size parameters in the
OrderController method before calling PageRequest.of: ensure page >= 0 and size
is between 1 and a safe MAX_PAGE_SIZE (declare a constant like MAX_PAGE_SIZE).
If validation fails, throw a 400-level error (e.g., ResponseStatusException with
HttpStatus.BAD_REQUEST) with a clear message. After validation (or after capping
size to MAX_PAGE_SIZE if you prefer to auto-limit), create the Pageable using
PageRequest.of(page, size, Sort.by(...)) and then call
orderService.getOrderHistory(...).
In
`@src/main/java/com/ongil/backend/domain/order/repository/OrderRepository.java`:
- Around line 83-108: The query in findOrderHistoryWithCount uses plain LEFT
JOIN so paging is fine but accessing order.getOrderItems() and
orderItem.getProduct() in OrderConverter.toSummaryDto triggers N+1 lazy loads;
fix by implementing the suggested 2-step approach: add a new repository method
findOrderHistoryIds(...) that pages only Order IDs (mirroring the current
predicates and countQuery), then add findOrdersWithItemsByIds(`@Param`("orderIds")
List<Long>) that uses LEFT JOIN FETCH o.orderItems and LEFT JOIN FETCH
oi.product to load full Order graph for those IDs and return List<Order>, and
update the service/flow to call the ID-page method first then fetch the full
orders by IDs; alternatively, you can apply `@BatchSize`(size = 20) on the
Order.orderItems collection to reduce queries (use either approach, reference
methods findOrderHistoryWithCount, findOrderHistoryIds, findOrdersWithItemsByIds
and OrderConverter.toSummaryDto).
In `@src/main/java/com/ongil/backend/domain/order/service/OrderService.java`:
- Around line 136-160: Add validation in getOrderHistory to ensure the resolved
date range is not inverted: after computing effectiveStartDate and
effectiveEndDate (and before converting to startDateTime/endDateTime or calling
orderRepository.findOrderHistoryWithCount), check that
effectiveStartDate.isAfter(effectiveEndDate) and if so throw a clear exception
(e.g., IllegalArgumentException or a domain-specific BadRequestException) with a
helpful message indicating startDate must be on or before endDate; use the
method name getOrderHistory and the variables
effectiveStartDate/effectiveEndDate in the check so callers get immediate
feedback instead of an empty result set.
🧹 Nitpick comments (2)
src/main/java/com/ongil/backend/domain/order/repository/OrderRepository.java (1)
65-81:FETCH JOIN+Page조합은 Hibernate 인메모리 페이지네이션을 유발합니다.이 메서드는 현재 사용되지 않는 데드 코드이지만, 만약 누군가 사용하게 되면 Hibernate가
HHH000104경고와 함께 전체 결과를 메모리에 로드한 후 페이지네이션을 적용합니다. 대규모 데이터에서 OOM 위험이 있습니다.현재
findOrderHistoryWithCount만 사용 중이므로, 혼동 방지를 위해 이 메서드를 제거하는 것을 권장합니다.🗑️ 사용되지 않는 메서드 제거
- // 주문 내역 조회 (기간 + 키워드 검색) - `@Query`("SELECT DISTINCT o FROM Order o " + - "LEFT JOIN FETCH o.orderItems oi " + - "LEFT JOIN FETCH oi.product p " + - "WHERE o.user.id = :userId " + - "AND o.createdAt >= :startDate " + - "AND o.createdAt <= :endDate " + - "AND (:keyword IS NULL OR :keyword = '' " + - "OR o.orderNumber LIKE CONCAT('%', :keyword, '%') " + - "OR p.name LIKE CONCAT('%', :keyword, '%'))") - Page<Order> findOrderHistory( - `@Param`("userId") Long userId, - `@Param`("keyword") String keyword, - `@Param`("startDate") LocalDateTime startDate, - `@Param`("endDate") LocalDateTime endDate, - Pageable pageable - );src/main/java/com/ongil/backend/domain/order/converter/OrderConverter.java (1)
116-136: 이미지 URL 추출 및 브랜드명 기본값 로직이OrderService.getOrderDetail과 중복됩니다 (DRY 위반).Lines 119-124의 이미지 URL 파싱(
split(",")[0].trim(),"default-image-url")과 브랜드명 기본값("일반 브랜드") 로직이OrderService의getOrderDetail메서드(Lines 93-100)에도 동일하게 존재합니다. 한쪽만 수정하면 불일치가 발생할 수 있습니다.
Product엔티티나 별도 유틸리티에 헬퍼 메서드를 두고, 두 곳 모두 재사용하는 것을 권장합니다.♻️ 헬퍼 메서드 추출 예시
// Product 엔티티 또는 별도 유틸리티 클래스에 추가 public String getFirstImageUrl() { if (this.imageUrls != null && !this.imageUrls.isBlank()) { return this.imageUrls.split(",")[0].trim(); } return "default-image-url"; } public String getBrandNameOrDefault() { return this.brand != null ? this.brand.getName() : "일반 브랜드"; }그런 다음 converter와 service 양쪽에서:
- String imageUrl = "default-image-url"; - if (product.getImageUrls() != null && !product.getImageUrls().isBlank()) { - imageUrl = product.getImageUrls().split(",")[0].trim(); - } - String brandName = product.getBrand() != null ? product.getBrand().getName() : "일반 브랜드"; + String imageUrl = product.getFirstImageUrl(); + String brandName = product.getBrandNameOrDefault();
src/main/java/com/ongil/backend/domain/order/controller/OrderController.java
Show resolved
Hide resolved
Comment on lines
+83
to
+108
| // 주문 내역 개수 조회 (countQuery 분리) | ||
| @Query(value = "SELECT DISTINCT o FROM Order o " + | ||
| "LEFT JOIN o.orderItems oi " + | ||
| "LEFT JOIN oi.product p " + | ||
| "WHERE o.user.id = :userId " + | ||
| "AND o.createdAt >= :startDate " + | ||
| "AND o.createdAt <= :endDate " + | ||
| "AND (:keyword IS NULL OR :keyword = '' " + | ||
| "OR o.orderNumber LIKE CONCAT('%', :keyword, '%') " + | ||
| "OR p.name LIKE CONCAT('%', :keyword, '%'))", | ||
| countQuery = "SELECT COUNT(DISTINCT o) FROM Order o " + | ||
| "LEFT JOIN o.orderItems oi " + | ||
| "LEFT JOIN oi.product p " + | ||
| "WHERE o.user.id = :userId " + | ||
| "AND o.createdAt >= :startDate " + | ||
| "AND o.createdAt <= :endDate " + | ||
| "AND (:keyword IS NULL OR :keyword = '' " + | ||
| "OR o.orderNumber LIKE CONCAT('%', :keyword, '%') " + | ||
| "OR p.name LIKE CONCAT('%', :keyword, '%'))") | ||
| Page<Order> findOrderHistoryWithCount( | ||
| @Param("userId") Long userId, | ||
| @Param("keyword") String keyword, | ||
| @Param("startDate") LocalDateTime startDate, | ||
| @Param("endDate") LocalDateTime endDate, | ||
| Pageable pageable | ||
| ); |
Contributor
There was a problem hiding this comment.
N+1 쿼리 문제: plain JOIN으로 페이지네이션은 정상이지만, 연관 엔티티 로딩 시 다수의 쿼리가 발생합니다.
findOrderHistoryWithCount는 FETCH JOIN 없이 plain LEFT JOIN을 사용하므로 SQL 레벨 페이지네이션은 정상 동작합니다. 하지만 이후 OrderConverter.toSummaryDto()에서 order.getOrderItems() → orderItem.getProduct() 접근 시 Lazy Loading이 발생하여, 페이지당 1(주문) + N(아이템) + M(상품) 쿼리가 실행됩니다 (pageSize=10이면 40+ 쿼리 가능).
권장 해결 방안:
- 2단계 쿼리: 먼저 주문 ID만 페이지네이션 조회 → 해당 ID들로
FETCH JOIN쿼리 - 또는
OrderItem컬렉션에@BatchSize(size = 20)적용
♻️ 2단계 쿼리 접근 예시
// 1단계: ID만 페이지네이션 조회
`@Query`(value = "SELECT DISTINCT o.id FROM Order o " +
"LEFT JOIN o.orderItems oi " +
"LEFT JOIN oi.product p " +
"WHERE o.user.id = :userId " +
"AND o.createdAt >= :startDate " +
"AND o.createdAt <= :endDate " +
"AND (:keyword IS NULL OR :keyword = '' " +
"OR o.orderNumber LIKE CONCAT('%', :keyword, '%') " +
"OR p.name LIKE CONCAT('%', :keyword, '%'))",
countQuery = "SELECT COUNT(DISTINCT o.id) FROM Order o " +
"LEFT JOIN o.orderItems oi " +
"LEFT JOIN oi.product p " +
"WHERE o.user.id = :userId " +
"AND o.createdAt >= :startDate " +
"AND o.createdAt <= :endDate " +
"AND (:keyword IS NULL OR :keyword = '' " +
"OR o.orderNumber LIKE CONCAT('%', :keyword, '%') " +
"OR p.name LIKE CONCAT('%', :keyword, '%'))")
Page<Long> findOrderHistoryIds(...);
// 2단계: FETCH JOIN으로 한번에 로딩
`@Query`("SELECT DISTINCT o FROM Order o " +
"LEFT JOIN FETCH o.orderItems oi " +
"LEFT JOIN FETCH oi.product p " +
"WHERE o.id IN :orderIds")
List<Order> findOrdersWithItemsByIds(`@Param`("orderIds") List<Long> orderIds);🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/order/repository/OrderRepository.java`
around lines 83 - 108, The query in findOrderHistoryWithCount uses plain LEFT
JOIN so paging is fine but accessing order.getOrderItems() and
orderItem.getProduct() in OrderConverter.toSummaryDto triggers N+1 lazy loads;
fix by implementing the suggested 2-step approach: add a new repository method
findOrderHistoryIds(...) that pages only Order IDs (mirroring the current
predicates and countQuery), then add findOrdersWithItemsByIds(`@Param`("orderIds")
List<Long>) that uses LEFT JOIN FETCH o.orderItems and LEFT JOIN FETCH
oi.product to load full Order graph for those IDs and return List<Order>, and
update the service/flow to call the ID-page method first then fetch the full
orders by IDs; alternatively, you can apply `@BatchSize`(size = 20) on the
Order.orderItems collection to reduce queries (use either approach, reference
methods findOrderHistoryWithCount, findOrderHistoryIds, findOrdersWithItemsByIds
and OrderConverter.toSummaryDto).
Comment on lines
+136
to
+160
| public OrderHistoryResponse getOrderHistory( | ||
| Long userId, | ||
| String keyword, | ||
| LocalDate startDate, | ||
| LocalDate endDate, | ||
| Pageable pageable | ||
| ) { | ||
| // 기본값 설정: endDate가 null이면 오늘, startDate가 null이면 1년 전 | ||
| LocalDate effectiveEndDate = (endDate != null) ? endDate : LocalDate.now(); | ||
| LocalDate effectiveStartDate = (startDate != null) ? startDate : effectiveEndDate.minusYears(1); | ||
|
|
||
| // LocalDate -> LocalDateTime 변환 | ||
| LocalDateTime startDateTime = effectiveStartDate.atStartOfDay(); | ||
| LocalDateTime endDateTime = effectiveEndDate.atTime(LocalTime.MAX); | ||
|
|
||
| Page<Order> orderPage = orderRepository.findOrderHistoryWithCount( | ||
| userId, | ||
| keyword, | ||
| startDateTime, | ||
| endDateTime, | ||
| pageable | ||
| ); | ||
|
|
||
| return orderConverter.toHistoryResponse(orderPage); | ||
| } |
Contributor
There was a problem hiding this comment.
startDate가 endDate 이후인 경우에 대한 검증이 없습니다.
클라이언트가 startDate=2026-03-01&endDate=2026-01-01처럼 역전된 날짜를 보내면, 빈 결과만 반환되고 에러 메시지가 없어 혼란을 줄 수 있습니다.
🛡️ 날짜 검증 추가 예시
LocalDate effectiveEndDate = (endDate != null) ? endDate : LocalDate.now();
LocalDate effectiveStartDate = (startDate != null) ? startDate : effectiveEndDate.minusYears(1);
+
+ if (effectiveStartDate.isAfter(effectiveEndDate)) {
+ throw new AppException(ErrorCode.INVALID_DATE_RANGE);
+ }🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/order/service/OrderService.java`
around lines 136 - 160, Add validation in getOrderHistory to ensure the resolved
date range is not inverted: after computing effectiveStartDate and
effectiveEndDate (and before converting to startDateTime/endDateTime or calling
orderRepository.findOrderHistoryWithCount), check that
effectiveStartDate.isAfter(effectiveEndDate) and if so throw a clear exception
(e.g., IllegalArgumentException or a domain-specific BadRequestException) with a
helpful message indicating startDate must be on or before endDate; use the
method name getOrderHistory and the variables
effectiveStartDate/effectiveEndDate in the check so callers get immediate
feedback instead of an empty result set.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ongil/backend/domain/order/controller/OrderController.java`:
- Around line 63-71: The controller is throwing AppException with custom
messages but GlobalExceptionHandler.handleAppException() only uses ErrorCode via
ErrorResponse.of(e.getErrorCode(), request), so custom messages are dropped;
either (A) preserve custom messages by adding an overloaded
ErrorResponse.of(ErrorCode, String message, HttpServletRequest) (or similar) and
update GlobalExceptionHandler.handleAppException(Exception e) to call
ErrorResponse.of(e.getErrorCode(), e.getMessage(), request) (use
AppException#getMessage()), or (B) if custom messages aren’t needed, change the
throws in OrderController (e.g., in OrderController methods validating
page/size/startDate/endDate) to new AppException(ErrorCode.INVALID_PARAMETER)
(remove the custom string) so the existing ErrorResponse.of(ErrorCode, request)
remains correct. Ensure you update references to AppException constructors or
ErrorResponse.of usages accordingly.
🧹 Nitpick comments (2)
src/main/java/com/ongil/backend/domain/order/controller/OrderController.java (2)
53-53:keyword파라미터에 대한 길이 제한을 고려해 보세요.
keyword에 길이 제한이 없어서 매우 긴 문자열이 전달될 경우 DB 쿼리(LIKE '%...%') 성능에 영향을 줄 수 있습니다. 간단한 상한(예: 100자) 검증을 추가하면 방어적으로 처리할 수 있습니다.🛡️ 간단한 검증 추가 예시
) { + if (keyword != null && keyword.length() > 100) { + throw new AppException(ErrorCode.INVALID_PARAMETER, "검색어는 100자 이하여야 합니다."); + } if (page < 0) {
41-41:MAX_PAGE_SIZE상수 선언 위치가 적절합니다.인스턴스 필드(
orderService) 다음에static final상수를 선언하는 것보다, 관례상 상수를 클래스 최상단(필드 선언 전)에 배치하는 것이 일반적입니다. 가독성 측면의 사소한 개선 사항입니다.♻️ 상수 위치 조정 예시
public class OrderController { + private static final int MAX_PAGE_SIZE = 100; + private final OrderService orderService; - - private static final int MAX_PAGE_SIZE = 100;
src/main/java/com/ongil/backend/domain/order/controller/OrderController.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍️ 작업 내용
Summary
주요 변경사항
1. 주문 내역 조회 API
GET /api/orders엔드포인트 추가2. 공통 에러 처리 개선 (팀원 공유)
변경 파일:
AppException.java- 커스텀 메시지 생성자 추가ErrorResponse.java- 오버로드 메서드 추가GlobalExceptionHandler.java- 커스텀 메시지 전달사용법: