Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough판매 중(on_sale) 상품이 있는 카테고리·브랜드만 반환하도록 저장소 쿼리와 서비스 필터링을 강화했습니다. 또한 SPECIAL_SALE 타입 상품을 일부 쿼리에서 제외하고, CategoryConverter에 서브카테고리 리스트를 외부로부터 주입하는 오버로드를 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 2
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/com/ongil/backend/domain/category/service/CategoryService.java (1)
100-109:⚠️ Potential issue | 🟠 Major캐시 없는 N+1 쿼리 — 매 요청마다 실행
getAllCategories와 달리getRecommendedSubCategories는 Redis 캐시가 적용되어 있지 않습니다. 요청마다findAllSubCategories()로 모든 하위 카테고리를 불러온 뒤, 각 서브카테고리에 대해existsByCategoryIdAndOnSaleTrue를 개별 호출합니다. Java 스트림의 지연 평가 덕분에limit(count)이후에는 중단되지만, 앞쪽 하위 카테고리 대부분에 판매 중인 상품이 없을 경우 여전히 대다수 카테고리에 대해 DB 쿼리가 실행됩니다.
getAllCategories와 동일하게, 판매 중인 상품을 가진 카테고리 ID를 단일 쿼리로 일괄 조회하여 Set 기반 필터링으로 교체하거나, 이 메서드 결과도 적절한 TTL로 캐싱하는 것을 권장합니다.public List<CategorySimpleResponse> getRecommendedSubCategories(int count) { List<Category> subCategories = categoryRepository.findAllSubCategories(); + Set<Long> activeCategoryIds = productRepository.findCategoryIdsWithOnSaleProducts(); return subCategories.stream() - .filter(category -> productRepository.existsByCategoryIdAndOnSaleTrue(category.getId())) + .filter(category -> activeCategoryIds.contains(category.getId())) .limit(count) .map(categoryConverter::toSimpleResponse) .collect(Collectors.toList()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java` around lines 100 - 109, getRecommendedSubCategories suffers from N+1 queries because it calls findAllSubCategories() then existsByCategoryIdAndOnSaleTrue(...) per category; replace the per-category existence checks with a single batched query (e.g., add/use a repository method like productRepository.findDistinctCategoryIdsByOnSaleTrue() to return a Set<Long>) and then filter the subCategories stream by set membership before limit/count, or alternatively cache the resulting recommended subcategory IDs/result with a TTL similar to getAllCategories; update getRecommendedSubCategories to use the batched Set lookup (and/or caching) instead of calling existsByCategoryIdAndOnSaleTrue for each category.
🧹 Nitpick comments (3)
src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java (2)
18-18:LIMIT 3하드코딩 대신 상수로 분리하는 것을 고려하세요.같은 값이 여러 쿼리(카테고리, 브랜드 등)에 분산될 경우 유지보수가 어려워집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java` at line 18, The native query currently hardcodes "LIMIT 3"; extract that magic number into a compile-time constant (e.g., static final String RANDOM_LIMIT = "LIMIT 3" or a shared int constant in a RepositoryConstants class) and replace "LIMIT 3" with that constant in the `@Query` annotation in BrandRepository (and other repositories like category/brand where the same limit appears) so updates are centralized; if the constant is used across multiple repositories place it in a shared constants class (e.g., RepositoryConstants.NUM_RANDOM_RESULTS) and reference that symbol from each repository.
16-18:ORDER BY RAND()는 대용량 테이블에서 성능 문제를 유발하며, EXISTS 서브쿼리에 인덱스가 필요합니다.
ORDER BY RAND()는 전체 행을 임시 테이블에 올린 뒤 정렬하므로 O(n log n) 비용이 발생합니다. 브랜드 수가 적은 지금은 큰 문제가 없으나, 규모가 커질 경우 병목이 됩니다.products테이블에(brand_id, on_sale)복합 인덱스가 없다면 EXISTS 서브쿼리마다 풀 스캔이 발생합니다.DB 마이그레이션 스크립트에 아래 인덱스 추가를 권장합니다:
CREATE INDEX idx_products_brand_on_sale ON products (brand_id, on_sale);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java` around lines 16 - 18, The native query in BrandRepository uses ORDER BY RAND() which will not scale and the EXISTS subquery needs an index; add a composite index on products(brand_id, on_sale) (e.g., idx_products_brand_on_sale) to avoid full scans, and replace ORDER BY RAND() with a scalable random-selection strategy (for example, sample by random id ranges, use DB-supported TABLESAMPLE/BERNOULLI, or precomputed random values and then join/filter to pick 3 brands) in the query used by BrandRepository to avoid sorting the entire result set.src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java (1)
199-226: JPQL에 완전 한정 열거형(FQCN) 하드코딩 →@Param파라미터화 권장현재 쿼리 문자열에
com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE를 직접 삽입하고 있습니다. 패키지 경로가 변경될 경우 런타임까지 오류가 드러나지 않으며, 이미 파일 상단에ProductTypeimport도 있어 파라미터 방식이 더 일관성 있습니다.♻️ 파라미터 방식으로 변경하는 예시
`@Query`("SELECT p FROM Product p " + "WHERE p.onSale = true " + - "AND p.productType <> com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE " + + "AND p.productType <> :excludedType " + "ORDER BY (COALESCE(p.viewCount, 0) + COALESCE(p.cartCount, 0)) DESC") -List<Product> findPopularProducts(Pageable pageable); +List<Product> findPopularProducts( + `@Param`("excludedType") ProductType excludedType, + Pageable pageable);호출 측에서
productRepository.findPopularProducts(ProductType.SPECIAL_SALE, pageable)처럼 전달하면 됩니다.findRecommendedProducts도 동일하게 적용합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java` around lines 199 - 226, The JPQL currently hardcodes the FQCN com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE in ProductRepository queries; change both findPopularProducts and findRecommendedProducts to accept a ProductType parameter and use a named JPQL parameter (e.g. :excludedProductType) instead of the FQCN, add `@Param`("excludedProductType") ProductType excludedProductType to the method signatures, update the `@Query` strings to reference :excludedProductType in the productType <> comparison, and update call sites to pass ProductType.SPECIAL_SALE when invoking these repository methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java`:
- Line 17: The EXISTS subquery in BrandRepository that checks for products on
sale only uses "p.on_sale = true" and misses excluding SPECIAL_SALE items;
update the EXISTS clause in the repository query used for recommended brands to
also exclude products with type SPECIAL_SALE (e.g., add "AND p.type !=
'SPECIAL_SALE'" or the equivalent enum/constant reference) so it matches the
behavior of findPopularProducts/findProductsByCategory and prevents brands with
only SPECIAL_SALE items from appearing in recommendations.
In
`@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`:
- Around line 49-64: The current code calls
productRepository.existsByCategoryIdAndOnSaleTrue for each subcategory causing
an N+M query storm; instead batch-fetch the set of category IDs that have
on-sale products in one query (e.g. add a repository method like
findDistinctCategoryIdsByCategoryIdInAndOnSaleTrue(Collection<Long>) returning
List<Long>), build a Set<Long> availableCategoryIds, then stream
parentCategories and for each parent filter its getSubCategories() by
availableCategoryIds (map each to categoryConverter.toSubCategoryResponse) and
only collect parents with non-empty filteredSubs; also replace the map(...
return null ...).filter(...) pattern with flatMap (or filter + map) to avoid
returning nulls — reference parentCategories,
productRepository.existsByCategoryIdAndOnSaleTrue,
categoryConverter.toSubCategoryResponse, and categoryConverter.toResponse when
making the change.
---
Outside diff comments:
In
`@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`:
- Around line 100-109: getRecommendedSubCategories suffers from N+1 queries
because it calls findAllSubCategories() then
existsByCategoryIdAndOnSaleTrue(...) per category; replace the per-category
existence checks with a single batched query (e.g., add/use a repository method
like productRepository.findDistinctCategoryIdsByOnSaleTrue() to return a
Set<Long>) and then filter the subCategories stream by set membership before
limit/count, or alternatively cache the resulting recommended subcategory
IDs/result with a TTL similar to getAllCategories; update
getRecommendedSubCategories to use the batched Set lookup (and/or caching)
instead of calling existsByCategoryIdAndOnSaleTrue for each category.
---
Nitpick comments:
In
`@src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java`:
- Line 18: The native query currently hardcodes "LIMIT 3"; extract that magic
number into a compile-time constant (e.g., static final String RANDOM_LIMIT =
"LIMIT 3" or a shared int constant in a RepositoryConstants class) and replace
"LIMIT 3" with that constant in the `@Query` annotation in BrandRepository (and
other repositories like category/brand where the same limit appears) so updates
are centralized; if the constant is used across multiple repositories place it
in a shared constants class (e.g., RepositoryConstants.NUM_RANDOM_RESULTS) and
reference that symbol from each repository.
- Around line 16-18: The native query in BrandRepository uses ORDER BY RAND()
which will not scale and the EXISTS subquery needs an index; add a composite
index on products(brand_id, on_sale) (e.g., idx_products_brand_on_sale) to avoid
full scans, and replace ORDER BY RAND() with a scalable random-selection
strategy (for example, sample by random id ranges, use DB-supported
TABLESAMPLE/BERNOULLI, or precomputed random values and then join/filter to pick
3 brands) in the query used by BrandRepository to avoid sorting the entire
result set.
In
`@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java`:
- Around line 199-226: The JPQL currently hardcodes the FQCN
com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE in
ProductRepository queries; change both findPopularProducts and
findRecommendedProducts to accept a ProductType parameter and use a named JPQL
parameter (e.g. :excludedProductType) instead of the FQCN, add
`@Param`("excludedProductType") ProductType excludedProductType to the method
signatures, update the `@Query` strings to reference :excludedProductType in the
productType <> comparison, and update call sites to pass
ProductType.SPECIAL_SALE when invoking these repository methods.
src/main/java/com/ongil/backend/domain/brand/repository/BrandRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/category/service/CategoryService.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/ongil/backend/domain/category/service/CategoryService.java (1)
107-113:getRecommendedSubCategories— 매번 동일한 결과 반환 가능성
findAllSubCategories()의 기본 정렬 순서 그대로filter→limit이 적용되므로,activeCategoryIds필터링 후에도 항상 같은count개의 카테고리가 반환될 수 있습니다. "추천" 의미에 맞게 사전에Collections.shuffle()을 적용하거나, 저장소 쿼리에 랜덤 정렬을 추가하는 것을 검토해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java` around lines 107 - 113, getRecommendedSubCategories currently applies filter → limit on the deterministic list returned by findAllSubCategories(), so the same top N items may be returned every time; to make recommendations non-deterministic, shuffle or randomize the subCategories collection before filtering/limiting (or adjust the repository query to return a randomized order). Locate getRecommendedSubCategories and before applying filter/limit on subCategories (the result of findAllSubCategories()), call Collections.shuffle(subCategories) or apply a repository-level random order, then proceed to filter by activeCategoryIds (from productRepository.findCategoryIdsWithOnSaleProducts()) and map with categoryConverter::toSimpleResponse so results vary across calls.src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java (1)
199-226: [선택적 개선] JPQL 인라인 FQCN → 바인딩 파라미터로 대체 가능두 쿼리 모두
com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE을 쿼리 문자열에 직접 포함합니다.@Param으로 바인딩하면 FQCN 중복 없이 타입 안전하게 처리할 수 있습니다.♻️ 리팩토링 예시 (findPopularProducts 기준)
`@Query`("SELECT p FROM Product p " + "WHERE p.onSale = true " + - "AND p.productType <> com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE " + + "AND p.productType <> :excludeType " + "ORDER BY (COALESCE(p.viewCount, 0) + COALESCE(p.cartCount, 0)) DESC") -List<Product> findPopularProducts(Pageable pageable); +List<Product> findPopularProducts( + `@Param`("excludeType") ProductType excludeType, + Pageable pageable +);호출부:
productRepository.findPopularProducts(ProductType.SPECIAL_SALE, pageable)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java` around lines 199 - 226, Both JPQL queries in findPopularProducts and findRecommendedProducts inline the fully-qualified enum constant com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE; change these to use a bind parameter to avoid FQCN duplication and improve type-safety: update the JPQL to replace the literal with "AND p.productType <> :excludedType", add a parameter of type ProductType (annotated with `@Param`("excludedType")) to both repository methods (findPopularProducts and findRecommendedProducts), and update any call sites to pass ProductType.SPECIAL_SALE when invoking these methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`:
- Around line 93-101: The loop in getRandomCategories triggers N+1 DB calls
because getTopProductThumbnail() calls findTopByCategoryIdOrderByPopularity()
per category; instead, pre-filter shuffledCategories using the same
activeCategoryIds obtained from findCategoryIdsWithOnSaleProducts() and
batch-fetch top thumbnails for the candidate category IDs (limit to count + a
small buffer), e.g., add a repository method like
findTopThumbnailsByCategoryIds(List<Long> categoryIds) and call it once for the
selected category IDs, then map thumbnails by categoryId and use that map inside
the existing loop (referencing getRandomCategories, getTopProductThumbnail,
shuffledCategories, count, activeCategoryIds,
categoryConverter.toRandomResponse) so you only perform ~1 query for thumbnails
rather than one per category.
---
Duplicate comments:
In
`@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`:
- Around line 53-68: The stream currently maps parentCategories to null for
parents without any active subcategories and then filters out nulls—replace this
anti-pattern by using flatMap: for each parent in the parentCategories stream,
build the filteredSubs list (using
parent.getSubCategories().stream().filter(...).map(categoryConverter::toSubCategoryResponse).collect(...))
and then return either Stream.empty() when filteredSubs.isEmpty() or
Stream.of(categoryConverter.toResponse(parent, filteredSubs)); this removes the
need to return null and the subsequent .filter(r -> r != null) while preserving
use of activeCategoryIds, categoryConverter.toSubCategoryResponse and
categoryConverter.toResponse.
---
Nitpick comments:
In
`@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`:
- Around line 107-113: getRecommendedSubCategories currently applies filter →
limit on the deterministic list returned by findAllSubCategories(), so the same
top N items may be returned every time; to make recommendations
non-deterministic, shuffle or randomize the subCategories collection before
filtering/limiting (or adjust the repository query to return a randomized
order). Locate getRecommendedSubCategories and before applying filter/limit on
subCategories (the result of findAllSubCategories()), call
Collections.shuffle(subCategories) or apply a repository-level random order,
then proceed to filter by activeCategoryIds (from
productRepository.findCategoryIdsWithOnSaleProducts()) and map with
categoryConverter::toSimpleResponse so results vary across calls.
In
`@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java`:
- Around line 199-226: Both JPQL queries in findPopularProducts and
findRecommendedProducts inline the fully-qualified enum constant
com.ongil.backend.domain.product.enums.ProductType.SPECIAL_SALE; change these to
use a bind parameter to avoid FQCN duplication and improve type-safety: update
the JPQL to replace the literal with "AND p.productType <> :excludedType", add a
parameter of type ProductType (annotated with `@Param`("excludedType")) to both
repository methods (findPopularProducts and findRecommendedProducts), and update
any call sites to pass ProductType.SPECIAL_SALE when invoking these methods.
Comment on lines
+93
to
+101
| List<CategoryRandomResponse> result = new ArrayList<>(); | ||
| for (Category category : shuffledCategories) { | ||
| if (result.size() >= count) break; | ||
| String thumbnailUrl = getTopProductThumbnail(category); | ||
| if (thumbnailUrl != null) { | ||
| result.add(categoryConverter.toRandomResponse(category, thumbnailUrl)); | ||
| } | ||
| } | ||
| return result; |
Contributor
There was a problem hiding this comment.
getRandomCategories 루프 내 N+1 — 카테고리당 DB 쿼리 1회 발생
getTopProductThumbnail()이 내부적으로 findTopByCategoryIdOrderByPopularity()를 호출하므로, 최악의 경우 전체 카테고리 수만큼 SELECT가 실행됩니다. getAllCategories()에서는 findCategoryIdsWithOnSaleProducts()로 N+1을 해결했지만, 이 메서드에는 동일한 사전 필터링이 적용되지 않았습니다.
activeCategoryIds로 사전 필터링하면 실제 DB 호출을 필요한 count개 근처로 줄일 수 있습니다.
🔧 수정 예시
public List<CategoryRandomResponse> getRandomCategories(int count) {
List<Category> allCategories = categoryRepository.findAllByOrderByDisplayOrder();
+ Set<Long> activeCategoryIds = new HashSet<>(productRepository.findCategoryIdsWithOnSaleProducts());
List<Category> shuffledCategories = new ArrayList<>(allCategories);
Collections.shuffle(shuffledCategories);
List<CategoryRandomResponse> result = new ArrayList<>();
for (Category category : shuffledCategories) {
if (result.size() >= count) break;
+ // 판매 중인 상품이 없는 카테고리는 DB 조회 없이 건너뜀
+ Long targetId = category.getParentCategory() == null
+ ? (category.getSubCategories().isEmpty() ? null : category.getSubCategories().get(0).getId())
+ : category.getId();
+ if (targetId == null || !activeCategoryIds.contains(targetId)) continue;
String thumbnailUrl = getTopProductThumbnail(category);
if (thumbnailUrl != null) {
result.add(categoryConverter.toRandomResponse(category, thumbnailUrl));
}
}
return result;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/ongil/backend/domain/category/service/CategoryService.java`
around lines 93 - 101, The loop in getRandomCategories triggers N+1 DB calls
because getTopProductThumbnail() calls findTopByCategoryIdOrderByPopularity()
per category; instead, pre-filter shuffledCategories using the same
activeCategoryIds obtained from findCategoryIdsWithOnSaleProducts() and
batch-fetch top thumbnails for the candidate category IDs (limit to count + a
small buffer), e.g., add a repository method like
findTopThumbnailsByCategoryIds(List<Long> categoryIds) and call it once for the
selected category IDs, then map thumbnails by categoryId and use that map inside
the existing loop (referencing getRandomCategories, getTopProductThumbnail,
shuffledCategories, count, activeCategoryIds,
categoryConverter.toRandomResponse) so you only perform ~1 query for thumbnails
rather than one per category.
This was referenced Feb 19, 2026
Merged
Merged
1 task
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
1 participant
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.
🔍️ 작업 내용
feat: 추천 상품 특가 제외 및 상품 없는 카테고리/브랜드 필터링
✨ 상세 설명
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
릴리스 노트
버그 수정
신규 기능