-
Notifications
You must be signed in to change notification settings - Fork 0
[round-9] 실시간 상품 랭킹 API 구현 #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fe081d9
6d8ed98
43d189a
a2eae10
1e0d48d
2d02575
e58c0df
f273c2d
bf60749
cb93541
fbbdae5
595dc1f
70a2afd
960fe06
6b2b272
c050b75
f3e5dce
e069460
e95ade6
801df03
56aa885
9cc890d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package com.loopers.application.ranking; | ||
|
|
||
| import com.loopers.domain.product.ProductResult; | ||
| import com.loopers.domain.product.ProductService; | ||
| import com.loopers.domain.ranking.RankingCommand; | ||
| import com.loopers.domain.ranking.RankingResult; | ||
| import com.loopers.domain.ranking.RankingService; | ||
| import com.loopers.support.error.BusinessException; | ||
| import com.loopers.support.error.CommonErrorType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.util.CollectionUtils; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class RankingFacade { | ||
|
|
||
| private final RankingService rankingService; | ||
| private final ProductService productService; | ||
|
|
||
| public RankingOutput.SearchRankings searchRankings(RankingInput.SearchRankings input) { | ||
| RankingCommand.SearchRanks rankCommand = new RankingCommand.SearchRanks(input.date(), input.page(), input.size()); | ||
| RankingResult.SearchRanks ranks = rankingService.searchRanks(rankCommand); | ||
|
|
||
| if (CollectionUtils.isEmpty(ranks.items())) { | ||
| return RankingOutput.SearchRankings.empty(ranks); | ||
| } | ||
|
|
||
| List<ProductResult.GetProductDetail> details = ranks.items() | ||
| .stream() | ||
| .map(rank -> productService.getProductDetail(rank.productId()) | ||
| .orElseThrow(() -> new BusinessException(CommonErrorType.NOT_FOUND)) | ||
| ) | ||
| .toList(); | ||
|
|
||
| return RankingOutput.SearchRankings.from(ranks, details); | ||
|
Comment on lines
+31
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEliminate N+1 detail fetches (Top 1000 = 1000 calls). Batch-fetch product details to reduce latency and load. Also decide behavior on missing details (skip vs. fail-all). - List<ProductResult.GetProductDetail> details = ranks.items()
- .stream()
- .map(rank -> productService.getProductDetail(rank.productId())
- .orElseThrow(() -> new BusinessException(CommonErrorType.NOT_FOUND))
- )
- .toList();
+ List<Long> productIds = ranks.items().stream().map(RankingResult.SearchRanks.Item::productId).toList();
+ // Prefer batch API to avoid N calls
+ List<ProductResult.GetProductDetail> details = productService.getProductDetails(productIds);
+ // Option A (strict): fail if any missing
+ if (details.size() != productIds.size()) {
+ throw new BusinessException(CommonErrorType.NOT_FOUND);
+ }
+ // Option B (lenient): filter missing and still return (requires output mapping to handle gaps)Outside-this-file support (add to ProductService): public List<ProductResult.GetProductDetail> getProductDetails(List<Long> productIds) { /* batch cache + repo */ }🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- rg: getProductDetail occurrences ----"
rg -n --hidden --no-ignore -S "getProductDetail\(" -C3 || true
echo
echo "---- rg: getProductDetails occurrences ----"
rg -n --hidden --no-ignore -S "getProductDetails\(" -C3 || true
echo
echo "---- rg: ProductService definition (interface/class) ----"
rg -n --hidden --no-ignore -S "interface\s+ProductService\b|class\s+ProductService\b" -C3 || true
echo
echo "---- rg: productService variable usages ----"
rg -n --hidden --no-ignore -S "\bproductService\b" -C3 || true
echo
echo "---- Show RankingFacade.java (first 300 lines) ----"
if [ -f "apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java" ]; then
sed -n '1,300p' apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java || true
else
echo "File not found: apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java"
fi
echo
echo "---- Show ProductService files (if found) ----"
rg -n --hidden --no-ignore -S "ProductService" -g '!**/target/**' -C2 || trueLength of output: 43208 Eliminate N+1 product-detail fetch — use a batch API ProductService currently only exposes getProductDetail(Optional); add a batch getProductDetails(List) and update RankingFacade to use it. Decide strict (fail-all) vs lenient (skip-missing) behavior. Location: apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (stream mapping) - List<ProductResult.GetProductDetail> details = ranks.items()
- .stream()
- .map(rank -> productService.getProductDetail(rank.productId())
- .orElseThrow(() -> new BusinessException(CommonErrorType.NOT_FOUND))
- )
- .toList();
+ List<Long> productIds = ranks.items().stream().map(RankingResult.SearchRanks.Item::productId).toList();
+ // Prefer batch API to avoid N calls
+ List<ProductResult.GetProductDetail> details = productService.getProductDetails(productIds);
+ // Option A (strict): fail if any missing
+ if (details.size() != productIds.size()) {
+ throw new BusinessException(CommonErrorType.NOT_FOUND);
+ }
+ // Option B (lenient): filter missing and still return (requires output mapping to handle gaps)Outside-this-file support (add to ProductService): public List<ProductResult.GetProductDetail> getProductDetails(List<Long> productIds) { /* batch cache + repo */ }🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package com.loopers.application.ranking; | ||
|
|
||
| import java.time.LocalDate; | ||
|
|
||
| public record RankingInput() { | ||
|
|
||
| public record SearchRankings( | ||
| LocalDate date, | ||
| Integer page, | ||
| Integer size | ||
| ) { | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,51 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.loopers.application.ranking; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.loopers.domain.product.ProductResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.loopers.domain.ranking.RankingResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public record RankingOutput() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public record SearchRankings( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Integer totalPages, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long totalItems, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Integer page, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Integer size, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<Item> items | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static SearchRankings empty(RankingResult.SearchRanks result) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new SearchRankings(result.totalPages(), result.totalItems(), result.page(), result.size(), List.of()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static SearchRankings from(RankingResult.SearchRanks ranks, List<ProductResult.GetProductDetail> details) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new SearchRankings( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ranks.totalPages(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ranks.totalItems(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ranks.page(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ranks.size(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| details.stream() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(detail -> new Item( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.productId(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.productName(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.basePrice(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.likeCount(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.brandId(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| detail.brandName() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .toList() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusivePreserve ranking order when joining details (essential)
Apply: @@
- public static SearchRankings from(RankingResult.SearchRanks ranks, List<ProductResult.GetProductDetail> details) {
- return new SearchRankings(
- ranks.totalPages(),
- ranks.totalItems(),
- ranks.page(),
- ranks.size(),
- details.stream()
- .map(detail -> new Item(
- detail.productId(),
- detail.productName(),
- detail.basePrice(),
- detail.likeCount(),
- detail.brandId(),
- detail.brandName()
- ))
- .toList()
- );
- }
+ public static SearchRankings from(RankingResult.SearchRanks ranks, List<ProductResult.GetProductDetail> details) {
+ var safeDetails = details == null ? List.<ProductResult.GetProductDetail>of() : details;
+ var byId = safeDetails.stream()
+ .collect(java.util.stream.Collectors.toMap(
+ ProductResult.GetProductDetail::productId,
+ java.util.function.Function.identity(),
+ (a, b) -> a,
+ java.util.LinkedHashMap::new));
+
+ var items = ranks.productIds().stream() // assume this exposes ranked product IDs
+ .map(byId::get)
+ .filter(java.util.Objects::nonNull)
+ .map(detail -> new Item(
+ detail.productId(),
+ detail.productName(),
+ detail.basePrice(),
+ detail.likeCount(),
+ detail.brandId(),
+ detail.brandName()
+ ))
+ .toList();
+
+ return new SearchRankings(
+ ranks.totalPages(),
+ ranks.totalItems(),
+ ranks.page(),
+ ranks.size(),
+ items
+ );
+ }Additional imports needed: +import java.util.LinkedHashMap;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.stream.Collectors;If Preserve ranking order when joining details (essential) from(...) currently streams details directly and ignores the rank order in ranks — map details by productId and emit items in ranks order. File: apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingOutput.java Apply: @@
- public static SearchRankings from(RankingResult.SearchRanks ranks, List<ProductResult.GetProductDetail> details) {
- return new SearchRankings(
- ranks.totalPages(),
- ranks.totalItems(),
- ranks.page(),
- ranks.size(),
- details.stream()
- .map(detail -> new Item(
- detail.productId(),
- detail.productName(),
- detail.basePrice(),
- detail.likeCount(),
- detail.brandId(),
- detail.brandName()
- ))
- .toList()
- );
- }
+ public static SearchRankings from(RankingResult.SearchRanks ranks, List<ProductResult.GetProductDetail> details) {
+ var safeDetails = details == null ? List.<ProductResult.GetProductDetail>of() : details;
+ var byId = safeDetails.stream()
+ .collect(java.util.stream.Collectors.toMap(
+ ProductResult.GetProductDetail::productId,
+ java.util.function.Function.identity(),
+ (a, b) -> a,
+ java.util.LinkedHashMap::new));
+
+ var items = ranks.productIds().stream() // assume this exposes ranked product IDs
+ .map(byId::get)
+ .filter(java.util.Objects::nonNull)
+ .map(detail -> new Item(
+ detail.productId(),
+ detail.productName(),
+ detail.basePrice(),
+ detail.likeCount(),
+ detail.brandId(),
+ detail.brandName()
+ ))
+ .toList();
+
+ return new SearchRankings(
+ ranks.totalPages(),
+ ranks.totalItems(),
+ ranks.page(),
+ ranks.size(),
+ items
+ );
+ }Additional imports needed: +import java.util.LinkedHashMap;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.stream.Collectors;If SearchRanks uses a different accessor than productIds(), swap accordingly. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public record Item( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long productId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String productName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Integer basePrice, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long likeCount, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Long brandId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String brandName | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid system-default time; inject Clock to fix date-boundary bugs.
Using LocalDate.now() ties behavior to the host JVM timezone and can mis-rank around midnight or on misconfigured servers. Inject a Clock and use LocalDate.now(clock).
Apply:
If the API later accepts a date parameter, thread it into this facade and prefer the provided date over now(clock).