Conversation
|
Caution Review failedThe pull request is closed. Walkthrough검색 대표 키워드 추출 메서드 추가 및 ProductService 연동, 리뷰 도메인에 드래프트 상태 분기 도입과 의류 카테고리 및 사이즈/재질 보조 타입 필드 추가 등 리뷰 생성·조회 흐름이 draft-aware 하게 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ReviewService as ReviewCommandService
participant Repo as ReviewRepository
participant DB
Client->>ReviewService: initializeReview(orderItemId)
ReviewService->>Repo: findByOrderItemIdAndReviewTypeAndReviewStatus(orderItemId, INITIAL, DRAFT)
alt draft exists
Repo-->>ReviewService: Optional<DRAFT review>
ReviewService-->>Client: return draft.id
else no draft
ReviewService->>DB: save new DRAFT review (saveAndFlush)
alt concurrent insert causes DataIntegrityViolation
DB-->>ReviewService: DataIntegrityViolationException
ReviewService->>Repo: findByOrderItemIdAndReviewTypeAndReviewStatus(..., DRAFT)
Repo-->>ReviewService: Optional<DRAFT review>
ReviewService-->>Client: return existing draft.id
else success
DB-->>ReviewService: persisted review.id
ReviewService-->>Client: return new review.id
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/ongil/backend/domain/review/service/ReviewQueryService.java (1)
152-177: 🛠️ Refactor suggestion | 🟠 MajorN+1 쿼리 문제: 주문 아이템별 개별 조회
각
OrderItem마다 최대 4번의 DB 쿼리가 실행됩니다:
existsBy...(INITIAL, COMPLETED)findBy...(INITIAL, DRAFT)existsBy...(ONE_MONTH, COMPLETED)findBy...(ONE_MONTH, DRAFT)주문 아이템이 많은 사용자의 경우 심각한 성능 저하가 발생할 수 있습니다.
getPendingReviewCount(Line 183)에서는 이미findReviewedOrderItemIds로 배치 조회하고 있으므로, 동일한 패턴을 여기에도 적용하는 것이 좋습니다.♻️ 배치 조회 기반 리팩토링 제안
public List<PendingReviewResponse> getPendingReviews(Long userId) { List<PendingReviewResponse> pendingReviews = new ArrayList<>(); List<OrderItem> orderItems = orderItemRepository.findByOrderUserIdWithProduct(userId); + List<Long> orderItemIds = orderItems.stream().map(OrderItem::getId).toList(); + if (orderItemIds.isEmpty()) return pendingReviews; + + // 배치로 COMPLETED 리뷰 존재 여부 확인 + List<Long> initialCompletedIds = reviewRepository.findReviewedOrderItemIds(orderItemIds, ReviewType.INITIAL); + List<Long> oneMonthCompletedIds = reviewRepository.findReviewedOrderItemIds(orderItemIds, ReviewType.ONE_MONTH); + + // 배치로 DRAFT 리뷰 조회 (새로운 repository 메서드 필요) + // Map<Long, Review> initialDrafts = ... + // Map<Long, Review> oneMonthDrafts = ... for (OrderItem orderItem : orderItems) { LocalDateTime orderDate = orderItem.getOrder().getCreatedAt(); - if (!reviewRepository.existsByOrderItemIdAndReviewTypeAndReviewStatus(orderItem.getId(), ReviewType.INITIAL, ReviewStatus.COMPLETED)) { - Review draftReview = reviewRepository.findByOrderItemIdAndReviewTypeAndReviewStatus( - orderItem.getId(), ReviewType.INITIAL, ReviewStatus.DRAFT).orElse(null); - pendingReviews.add(reviewConverter.toPendingReviewResponse(orderItem, ReviewType.INITIAL, draftReview)); + if (!initialCompletedIds.contains(orderItem.getId())) { + // DRAFT 조회는 배치 맵에서 가져옴 + pendingReviews.add(reviewConverter.toPendingReviewResponse(orderItem, ReviewType.INITIAL, /* draftFromMap */ null)); } // ... ONE_MONTH도 동일 패턴 }src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java (1)
73-79:⚠️ Potential issue | 🔴 Critical
buildAnswerSummary와buildMyReviewAnswerSummary에서 null 답변에 대한 NPE 위험 존재
sizeAnswer,colorAnswer,materialAnswer는 Review 엔티티에서 nullable 필드이며, 이들 메서드는 null 체크 없이.getDisplayName()을 직접 호출합니다.현재 리포지토리 쿼리는 모두
ReviewStatus.COMPLETED필터링을 명시적으로 적용하므로 DRAFT 리뷰는 리스트 조회에 포함되지 않습니다. 하지만 COMPLETED 상태의 리뷰도 답변 필드가 null일 수 있으므로,buildInitialFirstAnswers(라인 153에서 null 체크)와의 일관성을 맞추기 위해 양쪽 메서드 모두 null 체크를 추가해야 합니다.
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java`:
- Around line 141-145: Extract the duplicated mapping into a private helper
method (e.g., private ClothingCategory resolveClothingCategory(Product product))
inside ReviewConverter that returns
Optional.ofNullable(product.getCategory()).map(Category::getParentCategory).map(Category::getName).map(ClothingCategory::fromDisplayName).orElse(null);
then replace the inlined expressions in buildProductInfo and
buildPendingProductInfo with
.clothingCategory(resolveClothingCategory(product)). Ensure the helper is
non-public, null-safe, and referenced by both methods to remove duplication.
In
`@src/main/java/com/ongil/backend/domain/review/dto/response/PendingReviewResponse.java`:
- Around line 51-53: The ClothingCategory enum is currently serialized as its
enum name (e.g., OUTER, TOP) but should return the human-friendly Korean
displayName; update the ClothingCategory enum by annotating its displayName
getter or field with `@JsonValue` so Jackson serializes the enum using
displayName, ensuring API responses (and the
PendingReviewResponse.clothingCategory property) return the expected Korean
strings.
In
`@src/main/java/com/ongil/backend/domain/review/dto/response/ReviewDetailResponse.java`:
- Around line 142-143: The `@Schema` description for the field
materialSecondaryType in ReviewDetailResponse is missing a closing parenthesis;
update the `@Schema` annotation on the materialSecondaryType field to include the
closing ')' so the description reads "소재 2차 질문 방향 (POSITIVE / NEGATIVE / null)".
In
`@src/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.java`:
- Around line 205-206: The repository method
findByOrderItemIdAndReviewTypeAndReviewStatus can return duplicates under
concurrent requests because there's no DB unique constraint or pessimistic
locking; update to prevent duplicate DRAFT reviews by either: add a unique
constraint on the Review entity `@Table` for (order_item_id, review_type,
review_status); or annotate the repository method
findByOrderItemIdAndReviewTypeAndReviewStatus with
`@Lock`(LockModeType.PESSIMISTIC_WRITE); or apply a pessimistic lock during the
lookup inside ReviewCommandService.initializeReview before creating a new Review
so only one request can create the DRAFT for the same
orderItemId+reviewType+reviewStatus.
In `@src/main/java/com/ongil/backend/domain/search/service/SearchService.java`:
- Around line 118-129: The current loop over SearchHit<ProductDocument> (hits)
always inserts doc.getCategoryName() then getName() then getBrandName(), which
biases category values ahead of other fields even if a brand match had a higher
ES score; change to build a list of (score, value) pairs from each hit (use
hit.getScore() and the non-null results of
ProductDocument.getCategoryName()/getName()/getBrandName()), sort that flat list
by score descending, then iterate it to add values into the LinkedHashSet
suggestions (to dedupe while preserving the score-based order) and finally
return the first 12; update the code around the suggestions variable and the
hits loop accordingly.
🧹 Nitpick comments (6)
src/main/java/com/ongil/backend/domain/product/service/ProductService.java (1)
195-199: 검색 사이드 이펙트 로직 개선 확인대표 키워드 추출을
SearchService로 위임하여 SRP를 잘 준수하고 있고, null/blank 방어 로직도 적절합니다.다만,
extractRepresentativeKeyword(query)는 ES 쿼리를 한 번 더 발생시킵니다.getProductIdsByQuery(query)호출 시 이미 ES를 조회했으므로, 첫 조회 결과를 재활용하면 불필요한 네트워크 왕복을 줄일 수 있습니다.src/main/java/com/ongil/backend/domain/search/service/SearchService.java (1)
170-196: 대표 키워드 추출 로직 검토오타가 포함된 검색어도 ES가 매칭한 top hit에서 정규화된 키워드를 추출하는 접근이 합리적입니다.
contains체크 실패 시 fallback (categoryName > brandName)으로 처리하는 것도 적절합니다.두 가지 개선 포인트:
query를normalize()없이 사용:getProductIdsByQuery에서는normalize(query)를 거치지만, 이 메서드는 raw query를 그대로 사용합니다. autocomplete 필드 특성상 큰 문제는 아니나, 공백 전처리 등 일관성 차원에서 확인이 필요합니다.
contains기반 매칭의 한계:"나이키 신발".toLowerCase().contains("나이키")→ true이지만,"나이킹".toLowerCase().contains("나이키")→ false. 오타 케이스에서는 항상 fallback을 타게 되는데, fallback이 카테고리명을 반환하므로 brand 검색 시 대표 키워드가 brand가 아닌 category가 됩니다.♻️ Levenshtein 거리 기반 매칭 또는 ES explain API 활용 제안
// 가장 점수 높은 문서에서 query와 가장 유사한 필드 값 반환 String q = query.toLowerCase(); -if (top.getBrandName() != null && top.getBrandName().toLowerCase().contains(q)) return top.getBrandName(); -if (top.getCategoryName() != null && top.getCategoryName().toLowerCase().contains(q)) return top.getCategoryName(); -if (top.getName() != null && top.getName().toLowerCase().contains(q)) return top.getName(); +// boost 순서(brand > category > name)를 활용하여, +// ES가 어떤 필드에서 매칭했는지 highlight 또는 explain으로 판단하는 방안 고려 +if (top.getBrandName() != null && top.getBrandName().toLowerCase().contains(q)) return top.getBrandName(); +if (top.getCategoryName() != null && top.getCategoryName().toLowerCase().contains(q)) return top.getCategoryName(); +if (top.getName() != null && top.getName().toLowerCase().contains(q)) return top.getName();src/main/java/com/ongil/backend/domain/review/converter/ReviewWriteConverter.java (1)
37-47: 매직 문자열 대신 enum 또는 상수 사용 권장
"POSITIVE"/"NEGATIVE"문자열이 여러 곳에서 반복됩니다. 오타 위험이 있고, 타입 안전성이 없습니다.♻️ enum 또는 상수 활용 제안
+// 예: 별도 enum 또는 상수 정의 +private static final String POSITIVE = "POSITIVE"; +private static final String NEGATIVE = "NEGATIVE"; + String sizeSecondaryType = null; if (needsSizeQ) { SizeAnswer sizeAnswer = review.getSizeAnswer(); sizeSecondaryType = (sizeAnswer == SizeAnswer.LOOSE || sizeAnswer == SizeAnswer.TOO_BIG_NEED_ALTERATION) - ? "POSITIVE" : "NEGATIVE"; + ? POSITIVE : NEGATIVE; } String materialSecondaryType = null; if (needsMaterialQ) { - materialSecondaryType = review.getMaterialAnswer().isPositive() ? "POSITIVE" : "NEGATIVE"; + materialSecondaryType = review.getMaterialAnswer().isPositive() ? POSITIVE : NEGATIVE; }src/main/java/com/ongil/backend/domain/review/dto/response/PendingReviewResponse.java (1)
21-26: DTO 필드 추가 확인
reviewId와reviewStatus필드가 DRAFT 상태의 리뷰 정보를 프론트엔드에 전달하기 위해 적절히 추가되었습니다. Schema 설명도 명확합니다.
reviewStatus가String타입인 반면, 내부적으로는ReviewStatusenum을 사용하고 있습니다. 프론트엔드와의 계약이 확정되어 있다면 괜찮지만, 타입 안전성을 위해ReviewStatusenum을 직접 사용하는 것도 고려해볼 수 있습니다.src/main/java/com/ongil/backend/domain/review/dto/response/ReviewStep1Response.java (1)
5-5: 사용되지 않는@Schemaimport
io.swagger.v3.oas.annotations.media.Schema가 import되었지만, 이 클래스의 어떤 필드에도@Schema어노테이션이 적용되어 있지 않습니다. 같은 도메인의 다른 DTO(ReviewDetailResponse,PendingReviewResponse)는 모든 필드에@Schema를 사용하고 있으므로, import를 제거하거나 각 필드에@Schema어노테이션을 추가하여 일관성을 맞추는 것을 권장합니다.src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java (1)
152-177:sizeSecondaryType/materialSecondaryType판별 로직이ReviewWriteConverter와 중복됩니다Lines 160-169의
sizeSecondaryType/materialSecondaryType산출 로직이ReviewWriteConverter.toStep1Response(Snippet 2, lines 36-48)와 동일합니다. 두 곳에서 판별 조건이 독립적으로 유지되면, 향후 조건 변경 시 한쪽만 수정하는 실수가 발생할 수 있습니다.공통 유틸리티 메서드(예:
SizeAnswer.getSecondaryType(),MaterialAnswer.getSecondaryType())로 enum에 위임하거나, 공유 헬퍼로 추출하는 것을 권장합니다.
src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java
Outdated
Show resolved
Hide resolved
| @Schema(description = "상위 카테고리") | ||
| private ClothingCategory clothingCategory; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ClothingCategory enum file
fd "ClothingCategory.java" -type fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Find ClothingCategory enum file
fd "ClothingCategory.java" -t fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Read ClothingCategory enum
cat -n src/main/java/com/ongil/backend/domain/review/enums/ClothingCategory.java
echo ""
echo "=== Check for Jackson annotations ==="
grep -n "JsonValue\|JsonFormat\|JsonProperty" src/main/java/com/ongil/backend/domain/review/enums/ClothingCategory.java || echo "No Jackson annotations found"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 1583
🏁 Script executed:
#!/bin/bash
# Read PendingReviewResponse.java
cat -n src/main/java/com/ongil/backend/domain/review/dto/response/PendingReviewResponse.java
echo ""
echo "=== Check for Jackson annotations on clothingCategory field ==="
grep -B3 -A3 "clothingCategory" src/main/java/com/ongil/backend/domain/review/dto/response/PendingReviewResponse.java || echo "Not found"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 2109
ClothingCategory enum을 displayName으로 직렬화되도록 수정 필요
ClothingCategory에 Jackson 직렬화 설정이 없어서, API 응답 시 enum의 이름(OUTER, TOP 등)이 그대로 노출됩니다. 그런데 enum 내 displayName 필드가 한글 값("아우터", "상의" 등)을 가지고 있어서, 프론트엔드에서 기대하는 형식과 불일치할 가능성이 높습니다.
displayName 필드에 @JsonValue 어노테이션을 추가하여 직렬화 시 한글명이 반환되도록 수정하세요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/ongil/backend/domain/review/dto/response/PendingReviewResponse.java`
around lines 51 - 53, The ClothingCategory enum is currently serialized as its
enum name (e.g., OUTER, TOP) but should return the human-friendly Korean
displayName; update the ClothingCategory enum by annotating its displayName
getter or field with `@JsonValue` so Jackson serializes the enum using
displayName, ensuring API responses (and the
PendingReviewResponse.clothingCategory property) return the expected Korean
strings.
src/main/java/com/ongil/backend/domain/review/dto/response/ReviewDetailResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.java
Show resolved
Hide resolved
| // ES 점수 순서 그대로 카테고리/상품명/브랜드 모두 수집 | ||
| LinkedHashSet<String> suggestions = new LinkedHashSet<>(); | ||
| for (SearchHit<ProductDocument> hit : hits) { | ||
| ProductDocument doc = hit.getContent(); | ||
|
|
||
| if (doc.getCategoryName() != null) categorySet.add(doc.getCategoryName()); | ||
| if (doc.getBrandName() != null) brandSet.add(doc.getBrandName()); | ||
| if (doc.getCategoryName() != null) suggestions.add(doc.getCategoryName()); | ||
| if (doc.getName() != null) suggestions.add(doc.getName()); | ||
| if (doc.getBrandName() != null) suggestions.add(doc.getBrandName()); | ||
| } | ||
|
|
||
| List<String> suggestions = new ArrayList<>(categorySet); | ||
| suggestions.addAll(brandSet); | ||
|
|
||
| return suggestions.stream() | ||
| .limit(12) | ||
| .toList(); |
There was a problem hiding this comment.
자동완성 결과의 정렬 순서 주의
LinkedHashSet을 사용하여 ES 점수 기반으로 hit 간 순서는 보존됩니다. 그러나 각 hit 내부에서 categoryName → name → brandName 순으로 항상 고정 삽입되므로, 실제 결과는 순수한 ES 점수 순서가 아닙니다.
예를 들어, 1번 hit의 brandName이 가장 높은 점수 기여를 했더라도 categoryName이 먼저 추가됩니다. 의도된 동작이라면 괜찮지만, PR 목적("ES score-based relevance sorting")과 약간 차이가 있을 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/search/service/SearchService.java`
around lines 118 - 129, The current loop over SearchHit<ProductDocument> (hits)
always inserts doc.getCategoryName() then getName() then getBrandName(), which
biases category values ahead of other fields even if a brand match had a higher
ES score; change to build a list of (score, value) pairs from each hit (use
hit.getScore() and the non-null results of
ProductDocument.getCategoryName()/getName()/getBrandName()), sort that flat list
by score descending, then iterate it to add values into the LinkedHashSet
suggestions (to dedupe while preserving the score-based order) and finally
return the first 12; update the code around the suggestions variable and the
hits loop accordingly.
1 participant
🔍️ 작업 내용
✨ 상세 설명
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
최적화