Conversation
WalkthroughAdds weekly and monthly ranking support alongside daily across commerce-api, updates DTOs, services, repositories, and controllers; introduces YearWeek handling and formatter. Adds commerce-batch application with entities, repositories, services, tasklets, scheduler, and a split-flow batch job aggregating daily to weekly/monthly. Documentation and HTTP examples updated. Build files add threeten-extra. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as RankingV1Controller
participant Fac as RankingFacade
participant Svc as RankingService (API)
participant Repo as RankingRepository
participant Prod as ProductService
rect rgb(235,245,255)
note over C,API: Daily/Weekly/Monthly search
C->>API: GET /api/v1/rankings/{daily|weekly|monthly}
API->>Fac: search{Daily|Weekly|Monthly}(Request)
Fac->>Svc: search{Daily|Weekly|Monthly}(Command)
Svc->>Repo: searchRanks(date|yearWeek|yearMonth, pageable)
Repo-->>Svc: Page<SearchRanks>
Svc-->>Fac: RankingResult.{Daily|Weekly|Monthly}
Fac->>Prod: getProductDetail(productId...) loop
Prod-->>Fac: ProductDetail list
Fac-->>API: RankingOutput.{Daily|Weekly|Monthly}
API-->>C: ApiResponse<RankingResponse.{Daily|Weekly|Monthly}>
end
sequenceDiagram
autonumber
participant S as Scheduler
participant JL as JobLauncher
participant J as product_ranking_job
participant D as Daily Tasklet
participant WD as Weekly Decider
participant WS as Weekly Tasklet
participant MD as Monthly Decider
participant MS as Monthly Tasklet
participant BS as RankingService (Batch)
participant BR as RankingRepository (Batch)
participant R as Redis
S->>JL: run(J, params: date=yesterday)
JL->>J: start
J->>D: execute
D->>R: ZREVRANGE metric.product.all:<date> WITHSCORES
R-->>D: top items
D->>BS: aggregateDaily(entries)
BS->>BR: merge daily rows
BR-->>BS: ok
J->>WD: decide(date)
WD-->>J: RUN or SKIP
alt Weekly RUN
J->>WS: execute
WS->>BS: getDaily(start..end)
BS->>BR: findDailyRanking
BR-->>BS: list
WS->>BS: aggregateWeekly(productIds)
end
J->>MD: decide(date)
MD-->>J: RUN or SKIP
alt Monthly RUN
J->>MS: execute
MS->>BS: getDaily(start..end)
BS->>BR: findDailyRanking
BR-->>BS: list
MS->>BS: aggregateMonthly(productIds)
end
J-->>JL: completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
34-39: Consider bulk fetching product details for performanceThe current implementation makes N individual calls to
getProductDetailfor N products. This could cause performance issues with database round trips.Would you like me to suggest an implementation that fetches all product details in a single query? This would significantly improve performance by reducing database round trips from N to 1.
Also applies to: 55-60, 76-81
🧹 Nitpick comments (19)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingDaily.java (1)
58-82: Consider consistency in validation patterns across ranking entities.The validation logic here differs slightly from the weekly/monthly counterparts in the relevant code snippets:
- Score field: This entity includes a
scorefield with validation (score <= 0.0), butProductRankingWeeklyandProductRankingMonthlydon't have score fields- Date handling: This entity uses
LocalDatedirectly, while weekly/monthly entities derive their temporal fields from thestandardDateparameterConsider documenting why the daily ranking includes score while weekly/monthly do not, and whether this architectural difference is intentional for the aggregation logic.
apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java (1)
4-4: Remove unused import.The import
ProductRankingWeeklyis not used in this monthly repository interface.Apply this diff to remove the unused import:
-import com.loopers.domain.ranking.ProductRankingWeekly;apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
32-33: Update Javadoc comment to reflect monthly rather than weeklyThe Javadoc comment says "한 해의 주차" (week of the year) but this field represents YearMonth for monthly rankings.
Apply this diff to fix the comment:
- * 한 해의 주차 + * 연월apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java (2)
39-40: Consider using a more descriptive Redis key prefix.The hardcoded key prefix
"metric.product.all:"could benefit from being externalized to configuration or at least documented with a comment explaining the key structure.Consider extracting this to a constant:
+ private static final String REDIS_KEY_PREFIX = "metric.product.all:"; + @Override public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) { String day = date.format(DateTimeFormatter.BASIC_ISO_DATE); - String all = "metric.product.all:" + day; + String all = REDIS_KEY_PREFIX + day;
44-44: Consider parameterizing the top 100 limit.The hardcoded limit of 99 (top 100 items) should be configurable to allow flexibility in ranking size requirements.
Consider adding a configurable property:
+ @Value("${ranking.daily.limit:99}") + private int rankingLimit; + // Top 100 ZSetOperations<String, String> zSet = stringRedisTemplate.opsForZSet(); - Set<ZSetOperations.TypedTuple<String>> tuples = zSet.reverseRangeWithScores(all, 0, 99); + Set<ZSetOperations.TypedTuple<String>> tuples = zSet.reverseRangeWithScores(all, 0, rankingLimit);apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingRepository.java (1)
8-8: Consider adding pagination support for daily ranking queries.The
findDailyRankingmethod returns aList<ProductRankingDaily>without pagination, which could become problematic with large datasets over extended date ranges.Consider adding a paginated version:
public interface RankingRepository { List<ProductRankingDaily> findDailyRanking(LocalDate startDate, LocalDate endDate); + + Page<ProductRankingDaily> findDailyRanking(LocalDate startDate, LocalDate endDate, Pageable pageable);apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
20-24: Code duplication with CommerceStreamerApplication.The timezone initialization logic is duplicated from
CommerceStreamerApplication.java. Consider extracting this to a common configuration component.Consider creating a shared configuration class:
@Component public class TimezoneConfiguration { @PostConstruct public void initializeTimezone() { TimeZone.setDefault(TimeZone.getTimeZone("Asia/Seoul")); } }Then import this component in both applications instead of duplicating the logic.
apps/commerce-batch/src/main/resources/application.yml (1)
17-22: Consider adjusting the task executor pool configuration for batch processing.The current thread pool configuration (core-size: 8, max-size: 16, queue-capacity: 8) has a very small queue capacity relative to the pool sizes. This could lead to task rejections under moderate load since the queue will fill quickly before new threads are spawned.
For batch processing workloads, consider a larger queue capacity or using an unbounded queue if memory permits:
task: execution: pool: core-size: 8 max-size: 16 - queue-capacity: 8 + queue-capacity: 100 # Or consider Integer.MAX_VALUE for unboundedapps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
30-30: Fix incorrect Javadoc comment.The comment "한 해의 주차" (week of the year) is incorrect for a monthly field. It should describe months, not weeks.
/** - * 한 해의 주차 + * 연월 */apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java (2)
65-66: Consider limiting the number of products for performanceThe comment mentions "Top 100" but the code doesn't enforce this limit. If there are thousands of products, this could cause memory/performance issues.
Apply this diff to limit to top 100:
List<Long> productIds = productScores.entrySet() .stream() .sorted(Comparator .<Map.Entry<Long, Double>>comparingDouble(Map.Entry::getValue).reversed() .thenComparing(Map.Entry::getKey)) .map(Map.Entry::getKey) + .limit(100) .toList();
43-43: Verify getDaily handles month-long ranges efficiently
Found: getDaily -> rankingRepository.findDailyRanking(start,end) -> ProductRankingDailyJpaRepository.findByDateBetween(start,end) which returns List; MonthlyRankingAggregationTasklet then aggregates in-memory. (files: apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingService.java, apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java, apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingDailyJpaRepository.java, apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java)
Risk & fix: date is indexed but the current flow loads all rows into memory — for large volumes switch to DB-side aggregation (repo method that SELECTs product_id, SUM(score) GROUP BY product_id ORDER BY SUM DESC LIMIT 100) or use streaming/pagination to avoid high memory/CPU.
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java (1)
82-82: Unnecessary Long.valueOf() for Integer to Long conversionUsing
Long.valueOf(item.getRank())is verbose. Consider using auto-boxing or a direct cast if getRank() returns an int/Integer.Apply this diff:
- Long.valueOf(item.getRank()) + item.getRank().longValue()Or if getRank() returns a primitive int:
- Long.valueOf(item.getRank()) + (long) item.getRank()Also applies to: 97-97
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
23-84: Code duplication across three ranking methodsThe three methods (searchDaily, searchWeekly, searchMonthly) have identical logic patterns. This violates DRY principle and makes maintenance harder.
Consider extracting common logic into a generic method:
private <TCommand, TResult, TOutput> TOutput searchRanking( TCommand command, Function<TCommand, TResult> searchFunction, Function<TResult, Boolean> isEmpty, Function<TResult, TOutput> emptyMapper, Function<TResult, List<Long>> getProductIds, BiFunction<TResult, List<ProductResult.GetProductDetail>, TOutput> mapper ) { TResult ranks = searchFunction.apply(command); if (isEmpty.apply(ranks)) { return emptyMapper.apply(ranks); } List<ProductResult.GetProductDetail> details = getProductIds.apply(ranks) .stream() .map(productId -> productService.getProductDetail(productId) .orElseThrow(() -> new BusinessException(CommonErrorType.NOT_FOUND)) ) .toList(); return mapper.apply(ranks, details); }However, given the specific types involved, a simpler approach might be to create a base method with common logic.
apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java (1)
66-67: Top 100 comment is misleading - no limit is enforced.The comment says "Top 100" but the code passes all
productIdswithout limiting to 100 items.Either enforce the limit or update the comment:
// Top 100 - rankingService.aggregateWeekly(new RankingCommand.AggregateWeekly(date, productIds)); + List<Long> top100ProductIds = productIds.size() > 100 ? productIds.subList(0, 100) : productIds; + rankingService.aggregateWeekly(new RankingCommand.AggregateWeekly(date, top100ProductIds));apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingService.java (1)
24-41: Consider batch processing for better performance.The daily aggregation iterates and persists rankings one by one, which could be inefficient for large datasets. Consider batch inserts for better performance.
@Transactional public void aggregateDaily(RankingCommand.AggregateDaily command) { List<Map.Entry<Long, Double>> entries = command.entries(); if (CollectionUtils.isEmpty(entries)) { return; } + List<ProductRankingDaily> rankings = new ArrayList<>(); for (int i = 0; i < entries.size(); i++) { ProductRankingDaily ranking = ProductRankingDaily.builder() .date(command.date()) .productId(entries.get(i).getKey()) .rank(i + 1) .score(entries.get(i).getValue()) .build(); - - rankingRepository.merge(ranking); + rankings.add(ranking); } + rankingRepository.mergeAll(rankings); // Implement batch merge in repository }The same optimization applies to
aggregateWeeklyandaggregateMonthlymethods.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
71-72: Document or change week calculation (ISO-8601 vs calendar-year)File: apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java lines 71–72
LocalDate dateAtMonday = standardDate.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); this.yearWeek = YearWeek.from(dateAtMonday);Verified: anchoring to the previous-or-same Monday then YearWeek.from(...) follows ISO-8601 week-based-year — e.g., 2024-12-30..2025-01-01 produce YearWeek 2025-W01.
- If ISO-8601 semantics are intended: add a comment and unit tests covering year-boundary dates.
- If you expect the week to belong to the date's calendar year: change the calculation (e.g., derive week/year via WeekFields.of(Locale) or compute week-number within standardDate.getYear()) and add tests.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingResponse.java (1)
34-77: Consider extracting common response structure to reduce duplication.The
SearchWeeklyandSearchMonthlyclasses are identical in structure, differing only in their names. While this explicit approach provides type safety, you could reduce duplication by using a generic base class or a parameterized type.Consider this alternative approach using composition:
@Getter @Builder @RequiredArgsConstructor(access = AccessLevel.PRIVATE) public static class PagedRankingResponse { private final Integer totalPages; private final Long totalItems; private final Integer page; private final Integer size; private final List<Item> items; protected static PagedRankingResponse fromOutput( Integer totalPages, Long totalItems, Integer page, Integer size, List<? extends RankingOutput.Item> items) { return builder() .totalPages(totalPages) .totalItems(totalItems) .page(page) .size(size) .items(items.stream().map(Item::from).toList()) .build(); } } public static class SearchWeekly extends PagedRankingResponse { public static SearchWeekly from(RankingOutput.SearchWeekly output) { PagedRankingResponse base = fromOutput( output.totalPages(), output.totalItems(), output.page(), output.size(), output.items() ); return new SearchWeekly(base); } // Constructor that copies from base... }apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingOutput.java (1)
34-78: Consider extracting common pagination logic.Similar to the response layer,
SearchWeeklyandSearchMonthlyhave identical structure and factory methods. Consider reducing duplication while maintaining type safety.You could reduce duplication by extracting a common factory method:
private static <T> List<Item> mapDetailsToItems(List<ProductResult.GetProductDetail> details) { return details.stream().map(Item::from).toList(); } public static SearchWeekly from(RankingResult.SearchWeekly ranks, List<ProductResult.GetProductDetail> details) { return new SearchWeekly( ranks.totalPages(), ranks.totalItems(), ranks.page(), ranks.size(), mapDetailsToItems(details) ); }apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java (1)
36-84: Consider consolidating duplicate Item records.The nested
Itemrecords inSearchWeeklyandSearchMonthlyare identical to the one inSearchDaily. Consider extracting a single sharedItemrecord to reduce duplication.Apply this refactor to consolidate the Item records:
public record RankingResult() { + + public record Item( + Long productId, + Long rank + ) { + } public record SearchDaily( Integer totalPages, Long totalItems, Integer page, Integer size, List<Item> items ) { public static SearchDaily from(Page<RankingQueryResult.SearchRanks> page, Pageable pageable) { return new SearchDaily( page.getTotalPages(), page.getTotalElements(), pageable.getPageNumber(), pageable.getPageSize(), page.map(result -> new Item(result.productId(), result.rank())).toList() ); } - - public record Item( - Long productId, - Long rank - ) { - } } // Similar changes for SearchWeekly and SearchMonthly...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
README.md(1 hunks)apps/commerce-api/build.gradle.kts(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingInput.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingOutput.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/config/web/WebConfig.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/config/web/converter/YearWeekFormatter.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingCommand.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingRepository.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingService.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java(3 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingResponse.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/test/assertion/ConcurrentAssertion.java(1 hunks)apps/commerce-batch/build.gradle.kts(1 hunks)apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingDaily.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingCommand.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingRepository.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingResult.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingService.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingDailyJpaRepository.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/interfaces/scheduler/ranking/RankingScheduler.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java(1 hunks)apps/commerce-batch/src/main/java/com/loopers/support/CommonJobListener.java(1 hunks)apps/commerce-batch/src/main/resources/application.yml(1 hunks)build.gradle.kts(1 hunks)http/commerce-api/rank.http(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-11T17:37:17.705Z
Learnt from: ImSejin
PR: loopers-bootcamp/e-commerce#9
File: apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java:28-30
Timestamp: 2025-09-11T17:37:17.705Z
Learning: In the RankingResult.SearchRanks class at apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java, the pageable() method intentionally uses the page value directly in PageRequest.of(page, size) without converting from 1-based to 0-based indexing, as confirmed by the maintainer.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java
🧬 Code graph analysis (16)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
Getter(11-49)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java (4)
Getter(16-28)Getter(32-44)Getter(48-60)NoArgsConstructor(13-62)
apps/commerce-api/src/main/java/com/loopers/config/web/WebConfig.java (1)
apps/commerce-api/src/main/java/com/loopers/config/web/converter/YearWeekFormatter.java (1)
YearWeekFormatter(8-20)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingInput.java (1)
k6/api/v1/products/search-products.js (1)
page(46-46)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingCommand.java (1)
k6/api/v1/products/search-products.js (1)
page(46-46)
apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
apps/commerce-streamer/src/main/java/com/loopers/CommerceStreamerApplication.java (1)
SpringBootApplication(10-24)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (2)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingDaily.java (1)
Getter(12-84)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
Getter(15-77)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java (4)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingService.java (1)
Slf4j(13-49)apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java (1)
Slf4j(25-64)apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java (1)
Slf4j(26-73)apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java (1)
Slf4j(27-74)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingDaily.java (2)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
Getter(13-74)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
Getter(15-77)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java (3)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingResponse.java (5)
NoArgsConstructor(8-103)Getter(11-30)Getter(34-53)Getter(57-76)Getter(80-101)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
Getter(11-49)apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
Getter(10-48)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java (3)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java (2)
Slf4j(116-129)Slf4j(131-144)apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java (1)
Slf4j(25-64)apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java (1)
Slf4j(27-74)
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java (1)
apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java (1)
Repository(24-231)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java (2)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java (2)
Slf4j(116-129)Slf4j(131-144)apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java (1)
Slf4j(25-64)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (2)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingDaily.java (1)
Getter(12-84)apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (1)
Getter(13-74)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/DailyRankingAggregationTasklet.java (3)
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java (2)
Slf4j(116-129)Slf4j(131-144)apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java (1)
Slf4j(26-73)apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java (1)
Slf4j(27-74)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
Getter(10-48)apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java (4)
Getter(16-28)Getter(32-44)Getter(48-60)NoArgsConstructor(13-62)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingResponse.java (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java (3)
Getter(16-28)Getter(32-44)Getter(48-60)apps/commerce-api/src/main/java/com/loopers/domain/product/ProductResult.java (4)
Getter(11-52)Getter(41-51)Getter(98-128)Getter(110-127)
🔇 Additional comments (41)
apps/commerce-api/src/test/java/com/loopers/test/assertion/ConcurrentAssertion.java (1)
73-76: Excellent addition of explicit InterruptedException handling.This change properly handles thread interruption by restoring the interrupt status and tracking the interruption as an error. This is a best practice for concurrent code as it preserves the interruption signal for higher-level code that might need to respond to cancellation requests.
README.md (1)
30-30: LGTM! Documentation correctly reflects the new commerce-batch app.The addition of the
commerce-batchentry accurately documents the new Spring Batch application added as part of this PR's ranking system implementation.build.gradle.kts (1)
57-58: LGTM! threeten-extra dependency added correctly.ThreeTen-Extra provides additional date-time classes that complement those in JDK 8, including YearWeek as "an immutable date-time object that represents the combination of a week-based-year and week-of-week-based-year". This dependency enables the YearWeek and YearMonth types used throughout the ranking system implementation.
apps/commerce-api/build.gradle.kts (1)
20-20: LGTM! Dependency properly leverages the root-level version management.The threeten-extra dependency correctly references the version managed at the root level in
build.gradle.kts, maintaining consistency across modules.apps/commerce-batch/build.gradle.kts (1)
1-22: LGTM! Well-structured dependencies for batch processing.The dependency configuration appropriately includes:
- Core modules (JPA, Redis) for data access
- Spring Batch runtime and testing support
- threeten-extra for YearWeek/YearMonth types
- QueryDSL processors for advanced querying
- Test fixtures for reusing test infrastructure
All dependencies align with the batch-oriented ranking aggregation implementation.
apps/commerce-batch/src/main/java/com/loopers/support/CommonJobListener.java (1)
1-34: LGTM! Well-implemented Spring Batch job listener.The implementation provides comprehensive job lifecycle logging with appropriate log levels:
- Info-level logging for start and completion events
- Error-level logging for job failures with exit codes
- Proper separation of concerns for batch job monitoring
http/commerce-api/rank.http (1)
1-14: LGTM! HTTP examples align well with the new ranking endpointsThe HTTP examples correctly demonstrate the three new ranking endpoints (daily, weekly, monthly) with appropriate parameter formats and authentication headers. The date formats match the expected patterns:
- Daily: ISO date format (2025-09-10)
- Weekly: ISO week format (2025-W36)
- Monthly: Year-month format (2025-08)
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java (1)
12-13: Confirm whether secondary ORDER BY p.id is requiredQuery currently orders by p.rank, p.id. A unique constraint on (year_week, ref_product_id) prevents duplicate product rows but does not guarantee rank values are unique — verify how ranks are assigned (batch/job/entity). If rank ties are impossible, remove p.id; if ties can occur, keep p.id to ensure deterministic ordering/pagination.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java (2)
36-37: Decide & enforce temporal validation for SearchWeekly.yearWeekSearchDaily.date is annotated @PastOrPresent and SearchMonthly.yearMonth is annotated @past, but SearchWeekly.yearWeek is only @NotNull — either add @PastOrPresent or @past to align with intended semantics, or confirm and document that future weeks are allowed.
Location: apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingRequest.java — SearchWeekly.yearWeek
52-53: @past is correct for SearchMonthly — Monthly aggregation only runs when the job’s date parameter is the first day of the month (triggered by the scheduler on the 2nd of each month), aggregating the previous month, so SearchMonthly should exclude the current YearMonth.apps/commerce-api/src/main/java/com/loopers/config/web/WebConfig.java (1)
8-16: Consider adding a YearMonthFormatter for consistencyWebConfig currently registers only YearWeekFormatter (apps/commerce-api/src/main/java/com/loopers/config/web/WebConfig.java). No YearMonth formatter or conclusive YearMonth usage was found in the repo; if monthly endpoints accept java.time.YearMonth, register registry.addFormatter(new YearMonthFormatter()) here or annotate controller params with @DateTimeFormat(pattern = "yyyy-MM").
apps/commerce-api/src/main/java/com/loopers/config/web/converter/YearWeekFormatter.java (2)
8-19: LGTM! Clean and straightforward formatter implementation.The formatter correctly implements Spring's
Formatter<YearWeek>interface with proper delegation toYearWeek.parse()andtoString(). The locale-agnostic approach is appropriate since ISO-8601 year-week format is standardized.
3-4: Verify threeten-extra dependency version.I can't access the project's build files — paste the pom.xml or build.gradle (or provide a repo URL) so I can confirm whether org.threeten.extra:threeten-extra is declared and which version is used; ensure it's a current compatible release.
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingInput.java (2)
10-15: LGTM! Proper renaming for clarity.The renaming from
SearchRankingstoSearchDailyimproves API clarity and aligns with the expanded daily/weekly/monthly ranking structure.
19-33: LGTM! Consistent structure across time periods.The new
SearchWeeklyandSearchMonthlyrecords follow a consistent pattern with appropriate temporal types (YearWeekandYearMonth). The structure maintains consistency with pagination parameters.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingRepository.java (1)
10-15: LGTM! Consistent merge operation interface.The merge methods provide a consistent interface for persisting different ranking period types, with boolean return values indicating operation success.
apps/commerce-batch/src/main/java/com/loopers/interfaces/scheduler/ranking/RankingScheduler.java (1)
24-24: Verify the cron schedule aligns with PR requirements.The cron expression
"5 0 0 * * *"runs at 00:00:05 daily, which matches the PR objectives. The 5-second delay helps avoid potential conflicts with other midnight processes.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java (1)
13-14: LGTM! Proper JPA query with sorting.The query correctly uses JPQL syntax and includes proper ordering by rank and ID for consistent results. The parameterized query prevents SQL injection.
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingResult.java (2)
11-22: LGTM! Clean factory method implementation.The
from()method provides a clean way to transformProductRankingDailyentities to the result format using streams. The implementation is concise and follows good functional programming practices.
24-30: LGTM! Well-structured nested record.The
Itemrecord properly encapsulates the daily ranking data with appropriate field types. The nested structure keeps related concepts together while maintaining clarity.apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.java (1)
15-15: Resolved — mysqlMainDataSource bean presentBean defined in modules/jpa/src/main/java/com/loopers/config/jpa/DataSourceConfig.java (method mysqlMainDataSource, annotated @bean and @primary, returns HikariDataSource); @EnableBatchProcessing(dataSourceRef = "mysqlMainDataSource") is valid.
apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingCommand.java (1)
1-43: LGTM! Clean command structure for daily/weekly/monthly rankings.The command records are well-organized with clear separation of concerns. The use of
YearWeekfrom threeten-extra and standardYearMonthappropriately models the time periods for rankings.apps/commerce-batch/src/main/resources/application.yml (1)
24-28: LGTM! Safe defaults for batch job configuration.Good practice to have the batch job disabled by default with
NONEas the fallback job name. This prevents accidental job execution in non-intended environments.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/RankingCommand.java (1)
7-39: LGTM! Well-structured command records for batch operations.The command records clearly define the input data for different ranking aggregation operations. The use of
Map.Entry<Long, Double>for daily entries appropriately captures product-score pairs.apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java (1)
1-48: LGTM! Well-structured weekly ranking entity.The entity is properly configured with appropriate JPA annotations, constraints, and indexes. The use of
YearWeekfrom threeten-extra is a good choice for modeling weekly periods.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.java (1)
11-36: LGTM! Well-structured API specificationThe API specification cleanly separates daily, weekly, and monthly ranking endpoints with appropriate request/response types and clear operation descriptions.
apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java (2)
75-87: Incorrect — repository method exists.
The method findByYearWeek(YearWeek, Pageable) is defined in apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java:13, so the original comment is incorrect.Likely an incorrect or invalid review comment.
90-102: Incorrect — repository method existsproductRankingMonthlyJpaRepository.findByYearMonth(YearMonth, Pageable) is defined at apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java:14 — the "Missing repository method" claim is incorrect.
Likely an incorrect or invalid review comment.
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java (1)
53-58: LGTM! Good use of parallel execution.The configuration properly uses Spring Batch's split functionality to run weekly and monthly aggregations in parallel after daily aggregation completes, which is an efficient design.
apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/RankingRepositoryImpl.java (1)
27-36: merge() return-value handling is correct — no change needed.The ProductRankingDaily/Weekly/Monthly repositories use @Modifying methods that return int and execute INSERT ... ON CONFLICT DO UPDATE, which yields 1 for both insert and update; comparing == 1 is valid.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java (3)
19-33: LGTM! Clean implementation of daily rankings endpoint.The refactored daily rankings endpoint maintains good separation of concerns with proper request/response mapping and delegation to the facade layer.
35-49: LGTM! Well-structured weekly rankings endpoint.The weekly rankings implementation follows the same clean pattern as the daily endpoint, with appropriate use of
YearWeekfor time-based queries.
51-65: LGTM! Consistent monthly rankings implementation.The monthly rankings endpoint maintains consistency with the other time-period endpoints, properly utilizing
YearMonthfor temporal scoping.apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingService.java (3)
25-31: LGTM! Clear daily search implementation.The method correctly builds a pageable request and delegates to the repository with appropriate date filtering.
33-39: LGTM! Weekly search correctly implemented.The method properly uses
YearWeekfor weekly aggregation and follows the same clean pattern as the daily search.
41-47: LGTM! Monthly search follows established pattern.The monthly search implementation correctly uses
YearMonthand maintains consistency with the other time-period methods.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingResponse.java (2)
11-30: LGTM! Clean refactoring of daily response with centralized item mapping.The refactored
SearchDailyresponse correctly delegates item mapping to the sharedItem.from()method, reducing code duplication.
80-101: LGTM! Well-structured shared Item class.The extracted
Itemclass with its centralizedfrom()factory method effectively eliminates duplication and provides a clean mapping from the output layer.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingOutput.java (2)
10-30: LGTM! Clean daily output implementation.The renamed
SearchDailyrecord properly uses the centralizedItem::frommapping to reduce duplication.
82-99: LGTM! Clean Item mapper implementation.The
Itemrecord with itsfrom()factory method provides a clear and type-safe mapping fromProductResult.GetProductDetail.apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingResult.java (1)
10-32: LGTM! Well-structured daily result with proper pagination handling.The
SearchDailyrecord correctly uses the pageable's page number directly, which aligns with the documented learning about intentional 0-based indexing.
| */ | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false) |
There was a problem hiding this comment.
Fix incorrect column name for the monthly ranking entity's primary key.
The column name product_ranking_weekly_id appears to be a copy-paste error from the weekly entity. It should be product_ranking_monthly_id for consistency.
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
- @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false)
+ @Column(name = "product_ranking_monthly_id", nullable = false, updatable = false)
private Long id;📝 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.
| @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false) | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "product_ranking_monthly_id", nullable = false, updatable = false) | |
| private Long id; |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java
around line 26, the JPA @Column annotation uses the wrong column name
"product_ranking_weekly_id" (copy-paste error); change it to
"product_ranking_monthly_id" and keep nullable=false, updatable=false as-is so
the primary key column name matches the monthly entity.
| import org.threeten.extra.YearWeek; | ||
|
|
||
| import java.time.LocalDate; | ||
| import java.time.YearMonth; |
There was a problem hiding this comment.
🧩 Analysis chain
Consider adding JPA AttributeConverters for YearWeek and YearMonth.
JPA offers the AttributeConverter for mapping custom types. When using JPA, the AttributeConverter interface can define the conversion logic between Java YearMonth/YearWeek types and database column types. Since JPA doesn't support YearMonth and YearWeek out-of-the-box, you'll need converters for proper persistence.
Run the following script to check if AttributeConverters are already implemented:
Also applies to: 17-20
🏁 Script executed:
#!/bin/bash
# Description: Check for existing JPA AttributeConverters for YearWeek and YearMonth types
echo "Searching for YearWeek AttributeConverter..."
rg -nP --type=java "AttributeConverter.*YearWeek|YearWeek.*AttributeConverter" -A 5 -B 2
echo -e "\nSearching for YearMonth AttributeConverter..."
rg -nP --type=java "AttributeConverter.*YearMonth|YearMonth.*AttributeConverter" -A 5 -B 2
echo -e "\nSearching for @Converter annotations..."
rg -nP --type=java "@Converter" -A 10 -B 2
echo -e "\nSearching for any existing converters..."
ast-grep --pattern 'class $_ implements AttributeConverter<$_, $_>'Length of output: 16236
Add JPA AttributeConverters for YearWeek and YearMonth
No converters found for YearWeek or YearMonth — add YearMonthConverter and YearWeekConverter (implement AttributeConverter<YearMonth,String> and AttributeConverter<YearWeek,String>, annotate @converter) in apps/commerce-api/src/main/java/com/loopers/config/jpa/converter/ to persist as ISO strings (YYYY-MM / YYYY-Www).
| @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false) | ||
| private Long id; |
There was a problem hiding this comment.
Fix incorrect column name in @column annotation
The column is named product_ranking_weekly_id but this is the ProductRankingMonthly entity. This appears to be a copy-paste error from the weekly entity.
Apply this diff to fix the column name:
- @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false)
+ @Column(name = "product_ranking_monthly_id", nullable = false, updatable = false)📝 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.
| @Column(name = "product_ranking_weekly_id", nullable = false, updatable = false) | |
| private Long id; | |
| @Column(name = "product_ranking_monthly_id", nullable = false, updatable = false) | |
| private Long id; |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingMonthly.java
around lines 28-29, the @Column annotation uses the wrong column name
product_ranking_weekly_id; change it to product_ranking_monthly_id to match this
entity (keep nullable = false and updatable = false as-is).
| @Query(""" | ||
| insert into ProductRankingDaily (date, productId, rank, createdAt, updatedAt) | ||
| values (:date, :productId, :rank, :createdAt, :updatedAt) | ||
| on conflict (date, productId) do update set | ||
| rank = :rank, | ||
| updatedAt = :updatedAt | ||
| """) |
There was a problem hiding this comment.
Fix the native SQL query to use correct column names.
The @Query annotation uses entity property names instead of database column names. Since this is a native SQL query (indicated by the INSERT statement), you must use the actual database column names.
Based on the entity mapping patterns seen in other files, apply this fix:
@Query("""
- insert into ProductRankingDaily (date, productId, rank, createdAt, updatedAt)
+ insert into product_rankings_daily (date, ref_product_id, rank, created_at, updated_at)
values (:date, :productId, :rank, :createdAt, :updatedAt)
- on conflict (date, productId) do update set
+ on conflict (date, ref_product_id) do update set
rank = :rank,
- updatedAt = :updatedAt
+ updated_at = :updatedAt
""")Additionally, you should add nativeQuery = true to the @Query annotation since this is native SQL:
@Modifying
- @Query("""
+ @Query(value = """
insert into product_rankings_daily (date, ref_product_id, rank, created_at, updated_at)
values (:date, :productId, :rank, :createdAt, :updatedAt)
on conflict (date, ref_product_id) do update set
rank = :rank,
updated_at = :updatedAt
- """)
+ """, nativeQuery = true)📝 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.
| @Query(""" | |
| insert into ProductRankingDaily (date, productId, rank, createdAt, updatedAt) | |
| values (:date, :productId, :rank, :createdAt, :updatedAt) | |
| on conflict (date, productId) do update set | |
| rank = :rank, | |
| updatedAt = :updatedAt | |
| """) | |
| @Query(value = """ | |
| insert into product_rankings_daily (date, ref_product_id, rank, created_at, updated_at) | |
| values (:date, :productId, :rank, :createdAt, :updatedAt) | |
| on conflict (date, ref_product_id) do update set | |
| rank = :rank, | |
| updated_at = :updatedAt | |
| """, nativeQuery = true) |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingDailyJpaRepository.java
around lines 18 to 24, the @Query uses entity property names instead of DB
column names and is missing nativeQuery=true; change the SQL to use actual
column names (e.g. product_id, created_at, updated_at) in both the INSERT and ON
CONFLICT clauses and add nativeQuery = true to the @Query annotation; also
update the ON CONFLICT DO UPDATE assignment to reference the excluded values
(e.g. set rank = EXCLUDED.rank, updated_at = EXCLUDED.updated_at) so the native
upsert behaves correctly.
| @Modifying | ||
| @Query(""" | ||
| insert into ProductRankingMonthly (yearMonth, productId, rank, createdAt, updatedAt) | ||
| values (:yearMonth, :productId, :rank, :createdAt, :updatedAt) | ||
| on conflict (yearMonth, productId) do update set | ||
| rank = :rank, | ||
| updatedAt = :updatedAt | ||
| """) | ||
| int merge( | ||
| @Param("yearMonth") YearMonth yearMonth, | ||
| @Param("productId") Long productId, | ||
| @Param("rank") Integer rank, | ||
| @Param("createdAt") ZonedDateTime createdAt, | ||
| @Param("updatedAt") ZonedDateTime updatedAt | ||
| ); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Verify "on conflict" syntax compatibility with your database.
The query uses PostgreSQL-specific on conflict ... do update syntax. Ensure this is compatible with your target database system.
Run the following script to check the database configuration and verify syntax compatibility:
🏁 Script executed:
#!/bin/bash
# Description: Check database configuration and JPA dialect settings
echo "Checking database configuration in application files..."
fd -e yml -e yaml -e properties --exec rg -nH "spring\.datasource|hibernate\.dialect|database" {}
echo -e "\nSearching for similar upsert/merge queries..."
rg -nP --type=java "on conflict|merge into|upsert" -A 3 -B 1
echo -e "\nChecking for database-specific annotations or configurations..."
rg -nP --type=java "@Query.*merge|@Query.*conflict" -A 5 -B 2Length of output: 7597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Searching for PostgreSQL JDBC driver or 'postgres' mentions in build files and docs..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' "org\.postgresql|postgresql|jdbc:postgresql|jdbc:postgres" -S || true
echo -e "\n2) Searching for spring datasource / JPA database-platform / hibernate dialect entries..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' "spring\.datasource\.url|spring\.datasource\.driver-class-name|spring\.jpa\.database-platform|hibernate\.dialect|spring\.datasource" -S || true
echo -e "\n3) Searching for Docker / compose / k8s / workflows mentioning postgres or image: postgres..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' "image:\s*postgres|postgres:|postgresql:|POSTGRES_USER|POSTGRES_PASSWORD|postgres" -S || true
echo -e "\n4) Search in .github workflows and Dockerfiles for DATABASE_URL / PGHOST / PGUSER / POSTGRES env vars..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' "DATABASE_URL|PGHOST|PGUSER|POSTGRES|POSTGRES_DB|POSTGRES_USER|POSTGRES_PASSWORD" -S || true
echo -e "\n5) Search for 'on conflict' usage count (sanity)..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' "on conflict" -S || true
echo -e "\n6) Search README / docs for 'Postgres' mentions..."
rg -n --hidden --no-ignore --glob '!**/target/**' --glob '!**/build/**' -g 'README*' "postgres|postgresql" -S || trueLength of output: 1231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "A) Search for Postgres/JDBC driver mentions..."
rg -n --hidden -uu -S "org\.postgresql|jdbc:postgresql|jdbc:postgres|postgresql" || true
echo -e "\nB) Search for spring datasource / JPA / hibernate dialect settings..."
rg -n --hidden -uu -S "spring\.datasource|spring\.jpa\.database-platform|hibernate\.dialect|spring\.datasource\.url|spring\.datasource\.driver-class-name|spring\.jpa" || true
echo -e "\nC) Search for 'on conflict' occurrences (sanity)..."
rg -n --hidden -uu -S "on conflict" || true
echo -e "\nD) Search for Docker/compose/workflow envs or images mentioning Postgres..."
rg -n --hidden -uu -S "image:\s*postgres|postgres:|postgresql:|POSTGRES_|POSTGRES_DB|PGHOST|DATABASE_URL" || true
echo -e "\nE) Show modules/jpa application-jpa.yml (if exists)..."
if [ -f "modules/jpa/src/main/resources/application-jpa.yml" ]; then
echo "---- modules/jpa/src/main/resources/application-jpa.yml ----"
sed -n '1,240p' modules/jpa/src/main/resources/application-jpa.yml || true
else
echo "modules/jpa/src/main/resources/application-jpa.yml not found"
fi
echo -e "\nF) Show top-level README mentions of Postgres (if any)..."
rg -n --hidden -uu -S "postgres|postgresql" README* || trueLength of output: 5899
'ON CONFLICT' is PostgreSQL-only — fix required
modules/jpa/src/main/resources/application-jpa.yml configures mysql-jpa (com.mysql.cj.jdbc.Driver, jdbc:mysql://...), but multiple @query SQL use "on conflict" (e.g. apps/commerce-batch/.../ProductRankingMonthlyJpaRepository.java, ProductRankingDailyJpaRepository.java, ProductRankingWeeklyJpaRepository.java; apps/commerce-api/.../OutboxJpaRepository.java, InboxJpaRepository.java, LikedProductJpaRepository.java; apps/commerce-streamer/.../ProductMetricDailyJpaRepository.java). "ON CONFLICT" will fail on MySQL — either convert these queries to MySQL-compatible upsert syntax (INSERT ... ON DUPLICATE KEY UPDATE / INSERT IGNORE) or switch the datasource/dialect to PostgreSQL and update driver/config.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingMonthlyJpaRepository.java
around lines 16 to 31, the SQL uses PostgreSQL's "ON CONFLICT" which will fail
against the project's MySQL datasource; replace the PostgreSQL upsert with
MySQL-compatible syntax (INSERT ... ON DUPLICATE KEY UPDATE) or switch the
datasource/dialect to PostgreSQL. Specifically, change the query to use INSERT
... ON DUPLICATE KEY UPDATE setting rank=:rank, updatedAt=:updatedAt, ensure a
unique constraint/index exists on (yearMonth, productId) so the duplicate-key
upsert works, and verify any @Query/nativeQuery flags and parameter names remain
correct after the change.
| insert into ProductRankingWeekly (yearWeek, productId, rank, createdAt, updatedAt) | ||
| values (:yearWeek, :productId, :rank, :createdAt, :updatedAt) | ||
| on conflict (yearWeek, productId) do update set | ||
| rank = :rank, | ||
| updatedAt = :updatedAt |
There was a problem hiding this comment.
🧩 Analysis chain
Verify table and column names match database schema
When using native queries, table and column names must match the actual database schema exactly (case-sensitive in PostgreSQL). Ensure that ProductRankingWeekly, yearWeek, productId, etc., match your database naming convention.
🏁 Script executed:
#!/bin/bash
# Check for database migration files or entity definitions to verify the table schema
fd -e sql -e java | xargs rg -A5 -B5 "CREATE TABLE.*ProductRankingWeekly|product_ranking_weekly" --ignore-caseLength of output: 4730
Fix native SQL identifiers to match DB table/column names
Postgres folds unquoted identifiers to lower‑case; the entity uses snake_case (e.g. @column(name = "product_ranking_weekly_id")), so the native query's CamelCase/camelCase identifiers will not match the actual table/columns.
Action (files): apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java (lines ~16–20). Verify against apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductRankingWeekly.java.
Suggested fix — use snake_case (or quote identifiers). Example:
insert into product_ranking_weekly (year_week, product_id, rank, created_at, updated_at)
values (:yearWeek, :productId, :rank, :createdAt, :updatedAt)
on conflict (year_week, product_id) do update set
rank = EXCLUDED.rank,
updated_at = EXCLUDED.updated_at
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankingWeeklyJpaRepository.java
around lines 16–20, the native INSERT uses CamelCase identifiers that won't
match Postgres lower‑cased snake_case column/table names; update the SQL to use
the actual snake_case table and column names from ProductRankingWeekly (e.g.
product_ranking_weekly, year_week, product_id, created_at, updated_at), and in
the ON CONFLICT DO UPDATE use EXCLUDED.rank and EXCLUDED.updated_at (or properly
quoted identifiers) so the conflict update references the incoming values
correctly.
| public void launchRankingJob() throws Exception { | ||
| LocalDate yesterday = LocalDate.now().minusDays(1); | ||
| JobParameters jobParameters = new JobParametersBuilder() | ||
| .addLocalDate("date", yesterday) | ||
| .toJobParameters(); | ||
| jobLauncher.run(job, jobParameters); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and logging for job execution.
The scheduler method lacks error handling and logging, which are crucial for monitoring batch job execution in production.
Apply this diff to add proper error handling and logging:
+ @Slf4j
@Scheduled(cron = "5 0 0 * * *", zone = "Asia/Seoul")
public void launchRankingJob() throws Exception {
+ try {
+ log.info("Starting daily ranking job for date: {}", LocalDate.now().minusDays(1));
LocalDate yesterday = LocalDate.now().minusDays(1);
JobParameters jobParameters = new JobParametersBuilder()
.addLocalDate("date", yesterday)
.toJobParameters();
jobLauncher.run(job, jobParameters);
+ log.info("Successfully launched daily ranking job");
+ } catch (Exception e) {
+ log.error("Failed to launch daily ranking job", e);
+ throw e;
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/interfaces/scheduler/ranking/RankingScheduler.java
around lines 25 to 31, the launchRankingJob method currently runs the job
without any logging or exception handling; add start/finish info logs, capture
the JobExecution returned by jobLauncher.run, log the execution id and status on
success, and wrap the run call in a try/catch to log any Exception (including
the stack/exception message) with context; in the catch block record a failure
log and either rethrow a wrapped exception or handle the failure according to
existing application patterns (e.g., update metrics or alerting) so job failures
are observable in production.
| LocalDate startDate = date.with(TemporalAdjusters.firstDayOfMonth()).minusMonths(1); | ||
| LocalDate endDate = date.minusDays(1); |
There was a problem hiding this comment.
Date calculation logic may not handle edge cases correctly
The current logic calculates the previous month's range incorrectly. Line 40 gets the first day of the current month, then subtracts a month. This could fail for edge cases like March 31st (which would try to get February 31st before adjusting).
Apply this diff for more robust date calculations:
- LocalDate startDate = date.with(TemporalAdjusters.firstDayOfMonth()).minusMonths(1);
- LocalDate endDate = date.minusDays(1);
+ // Get the previous month first, then find its first day
+ LocalDate previousMonth = date.minusMonths(1);
+ LocalDate startDate = previousMonth.with(TemporalAdjusters.firstDayOfMonth());
+ LocalDate endDate = previousMonth.with(TemporalAdjusters.lastDayOfMonth());📝 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.
| LocalDate startDate = date.with(TemporalAdjusters.firstDayOfMonth()).minusMonths(1); | |
| LocalDate endDate = date.minusDays(1); | |
| // Get the previous month first, then find its first day | |
| LocalDate previousMonth = date.minusMonths(1); | |
| LocalDate startDate = previousMonth.with(TemporalAdjusters.firstDayOfMonth()); | |
| LocalDate endDate = previousMonth.with(TemporalAdjusters.lastDayOfMonth()); |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/job/ranking/MonthlyRankingAggregationTasklet.java
around lines 40-41, the date-range calculation for the previous month should be
made robust by using YearMonth to avoid edge-case invalid dates; derive
YearMonth.from(date).minusMonths(1), then set startDate to that Year's month
first day and endDate to that month's last day (atEndOfMonth) so you always get
a valid first and last day for the previous month.
| public FlowExecutionStatus decide(JobExecution jobExecution, StepExecution stepExecution) { | ||
| LocalDate date = jobExecution.getJobParameters().getLocalDate("date"); | ||
| String status = date.getDayOfWeek() == DayOfWeek.MONDAY ? "RUN" : "SKIP"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the weekly decider logic aligns with the tasklet's date calculation.
The decider runs weekly aggregation on Mondays, but given the date calculation issue in WeeklyRankingAggregationTasklet, ensure both components agree on which week's data to process.
🏁 Script executed:
#!/bin/bash
# Check how the date parameter is passed to the job
echo "Checking RankingScheduler to see how date is set..."
fd -e java "RankingScheduler" --exec grep -B 5 -A 10 "JobParameters\|date"
echo -e "\nChecking if there's documentation about the date parameter semantics..."
fd README -e md --exec grep -i -A 3 -B 3 "ranking\|date\|weekly"Length of output: 1161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "-> Locating ProductRankingJobConfig.java"
pfile=$(fd -e java ProductRankingJobConfig.java --strip-cwd-prefix || true)
pfile=$(echo "$pfile" | head -n1 || true)
if [ -n "$pfile" ]; then
echo "FOUND: $pfile"
sed -n '1,260p' "$pfile"
else
echo "NOT FOUND by filename; searching for class declaration..."
rg -n --hidden "class\s+ProductRankingJobConfig" -S || true
fi
echo; echo "-> Locating WeeklyRankingAggregationTasklet.java"
tfile=$(fd -e java WeeklyRankingAggregationTasklet.java --strip-cwd-prefix || true)
tfile=$(echo "$tfile" | head -n1 || true)
if [ -n "$tfile" ]; then
echo "FOUND: $tfile"
sed -n '1,400p' "$tfile"
else
echo "NOT FOUND by filename; searching for 'Weekly' + 'Aggregation' occurrences..."
rg -n --hidden "Weekly.*Aggregation|Aggregation.*Weekly" -S || true
fi
echo; echo "-> Searching for usages of the job parameter 'date'"
rg -n --hidden "\\.getLocalDate\\(\"date\"\\)|addLocalDate\\(\"date\"\\)|getJobParameters\\(\\)\\.getLocalDate\\(\"date\"\\)" -S || trueLength of output: 8997
Align 'date' semantics between RankingScheduler and deciders (weekly/monthly)
- RankingScheduler passes job param "date" = LocalDate.now().minusDays(1) (yesterday). WeeklyDecider returns RUN only when date.getDayOfWeek() == MONDAY, so the weekly flow will SKIP on Mondays (scheduler passes Sunday). WeeklyRankingAggregationTasklet computes start/end with previousOrSame(MONDAY)/previousOrSame(SUNDAY) and expects date to represent the week-end (Sunday).
- Fix (pick one):
- A) Keep scheduler passing yesterday and change WeeklyDecider to check for SUNDAY (and change MonthlyDecider to check for last-day-of-month), or
- B) Pass LocalDate.now() from RankingScheduler and keep deciders as MONDAY / day==1.
- Files to update: apps/commerce-batch/src/main/java/com/loopers/interfaces/scheduler/ranking/RankingScheduler.java; apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java; apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java.
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/job/ranking/ProductRankingJobConfig.java
around lines 119-121, the WeeklyDecider currently checks date.getDayOfWeek() ==
MONDAY while RankingScheduler passes date = LocalDate.now().minusDays(1)
(yesterday), causing weekly runs to SKIP on Mondays; align semantics by
implementing option A: keep the scheduler passing yesterday and change deciders
to treat the passed date as the period-end. Concretely, update WeeklyDecider to
check for DayOfWeek.SUNDAY instead of MONDAY; update MonthlyDecider to detect
month-end by comparing date to date.with(TemporalAdjusters.lastDayOfMonth()) (or
date.getDayOfMonth() == date.lengthOfMonth()); leave RankingScheduler unchanged;
review WeeklyRankingAggregationTasklet to ensure its
previousOrSame(MONDAY)/previousOrSame(SUNDAY) logic remains correct given date
represents the week-end and adjust only if any inverted assumptions remain.
| // 지난 주 월요일 ~ 일요일 | ||
| LocalDate startDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | ||
| LocalDate endDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.SUNDAY)); | ||
|
|
There was a problem hiding this comment.
Potential logic error in weekly date range calculation.
The date range calculation uses previousOrSame for both start and end dates, which creates incorrect ranges. When date is a Monday, this produces Monday-Sunday of the current week, not the previous week as the comment suggests.
Apply this fix to correctly calculate the previous week's range:
- // 지난 주 월요일 ~ 일요일
- LocalDate startDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY));
- LocalDate endDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.SUNDAY));
+ // 지난 주 월요일 ~ 일요일
+ LocalDate endDate = date.minusDays(1); // Previous day (Sunday if date is Monday)
+ LocalDate startDate = endDate.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY));The current logic would aggregate the wrong week's data when executed on Monday.
📝 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.
| // 지난 주 월요일 ~ 일요일 | |
| LocalDate startDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | |
| LocalDate endDate = date.with(TemporalAdjusters.previousOrSame(DayOfWeek.SUNDAY)); | |
| // 지난 주 월요일 ~ 일요일 | |
| LocalDate endDate = date.minusDays(1); // Previous day (Sunday if date is Monday) | |
| LocalDate startDate = endDate.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); |
🤖 Prompt for AI Agents
In
apps/commerce-batch/src/main/java/com/loopers/job/ranking/WeeklyRankingAggregationTasklet.java
around lines 40-43, the code uses previousOrSame for both start and end which
yields the current week when run on Monday; change the logic to compute the
previous week's range by shifting the computed boundaries one week back (e.g.,
compute startDate as date.with(previousOrSame(MONDAY)).minusWeeks(1) and compute
endDate either as startDate.plusDays(6) or as
date.with(previousOrSame(SUNDAY)).minusWeeks(1)) so the tasklet consistently
covers the full previous week.
종합 피드백 (9월 24일 2025)
음.. 보통 무기한 ttl로 잡고 write-through로 하는 패턴은 간헐적으로 있는것 같습니다. 개인적으로는 거의 write-through로 하는 경우가 많은것 같고 혹은 caffein cache를통해 지속적으로 refresh하는 경우도 많은것 같아요! 자주 봅시다 세진님! Implementation Quest: PASSWriting Quest: PASS |
1 participant
📌 Summary
일간/주간/월간 랭킹 영속화
주간/월간 랭킹 API 구현
Redis에서 조회한 일간 랭킹과 달리 RDB에서 주간/월간 랭킹을 조회했습니다.
💬 Review Points
일간 랭킹 테이블 대신 실시간 랭킹의 TTL을 한 달 이상으로 한다면?
맨 처음에 실시간으로 집계되는 일간 랭킹이 레디스에 있으니 이를 활용하자고 생각했습니다.
하지만 바로 문제가 생겼습니다. TTL이 3일로 지정되어 있었기 때문입니다.
TTL을 한 달 이상으로 설정하면 해결은 되겠으나, 메모리를 상당히 많이 차지할 거 같습니다.
분기별/연간 랭킹 요구사항이 들어온다면 TTL을 또 늘려야 하니 비효율적일 거 같다고 생각합니다.
그럼에도 불구하고, 실무에서 레디스에 집계되는 실시간 랭킹의 TTL을 배치를 위해 길게 잡는 경우가 있을까요?
✅ Checklist
🧱 Spring Batch
🧩 Ranking API
Summary by CodeRabbit
New Features
Documentation