[volume-10] 주간/월간 랭킹 - 김선민#417
Open
seonminKim1122 wants to merge 4 commits intoLoopers-dev-lab:seonminKim1122from
Open
[volume-10] 주간/월간 랭킹 - 김선민#417seonminKim1122 wants to merge 4 commits intoLoopers-dev-lab:seonminKim1122from
seonminKim1122 wants to merge 4 commits intoLoopers-dev-lab:seonminKim1122from
Conversation
- WeeklyRankingJob: targetDate(월요일) 기준 해당 주 7일 product_metrics 집계 - MonthlyRankingJob: targetYearMonth(yyyyMM) 기준 해당 월 전체 집계 - Chunk-Oriented 처리 (JdbcCursorItemReader → RankingProcessor → JpaItemWriter) - score = like*0.2 + view*0.1 + 0.7*log1p(sales), 상위 100개만 MV 테이블에 적재 - MvProductRankWeekly, MvProductRankMonthly 엔티티 추가 - E2E 테스트 작성 (파라미터 누락/빈 메트릭/100개 이하/100개 초과 케이스) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- GET /api/v1/rankings?period=DAILY|WEEKLY|MONTHLY 지원 (기본값: DAILY) - DAILY: 기존 Redis ZSET 기반 일간 랭킹 유지 - WEEKLY: mv_product_rank_weekly 테이블 기반 (date 속한 주의 월요일 기준) - MONTHLY: mv_product_rank_monthly 테이블 기반 (date 속한 월의 1일 기준) - commerce-batch SQL에 LIMIT 100 추가 및 TruncateTasklet을 기간별 삭제로 변경 (멱등성) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- JpaItemWriter → JdbcBatchItemWriter: TRUNCATE → INSERT 패턴에서 merge()의 불필요한 SELECT 제거 - JpaTransactionManager → DataSourceTransactionManager로 단순화 - MvProductRankWeekly, MvProductRankMonthly를 JPA 엔티티에서 순수 POJO로 변환 (jakarta.persistence 의존성 제거) - ProductAggregation에 score 필드 추가, SQL에서 score 계산 후 Processor 재계산 로직 제거 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough랭킹 조회에 주간 및 월간 기간 지원을 추가하였다. RankingFacade.getPage 시그니처가 RankingPeriod 파라미터를 받도록 변경되어 기간별(DAILY, WEEKLY, MONTHLY)로 baseDate를 계산하고 대응 리포지토리에서 페이징·카운트를 조회한다. 주간/월간 MV 엔티티·리포지토리·배치(job/processor/tasklet)와 관련 통합·E2E 테스트가 추가되었다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as RankingController
participant Facade as RankingFacade
participant Repo as Weekly/Monthly/ Daily<br/>RankingRepository
participant JpaRepo as Weekly/Monthly<br/>RankingJpaRepository
participant DB as Database
Client->>Controller: GET /api/v1/rankings?date=...&period=WEEKLY
Controller->>Facade: getPage(date, WEEKLY, page, size)
Note over Facade: period=WEEKLY\nbaseDate = date.previousOrSame(MONDAY)
Facade->>Repo: findProductIdsByBaseDate(baseDate, offset, limit)
Repo->>JpaRepo: findProductIdsByBaseDate(baseDate, pageRequest)
JpaRepo->>DB: SELECT product_id FROM mv_product_rank_weekly\nWHERE base_date=? ORDER BY rank ASC
DB-->>JpaRepo: List<Long>
JpaRepo-->>Repo: List<Long>
Repo-->>Facade: List<Long>
Facade->>Repo: countByBaseDate(baseDate)
Repo->>JpaRepo: countByBaseDate(baseDate)
JpaRepo->>DB: SELECT COUNT(*) FROM mv_product_rank_weekly WHERE base_date=?
DB-->>JpaRepo: long
JpaRepo-->>Repo: long
Repo-->>Facade: long
Facade-->>Controller: PageResponse<RankingInfo>
Controller-->>Client: HTTP 200 + JSON
sequenceDiagram
participant Scheduler as Spring Batch Scheduler
participant Job as WeeklyRankingJob
participant Truncate as TruncateWeeklyMvTasklet
participant Reader as JdbcCursorItemReader
participant Processor as WeeklyRankingProcessor
participant Writer as JdbcBatchItemWriter
participant DB as Database
Scheduler->>Job: launch(targetDate=Monday)
Job->>Truncate: execute()
Truncate->>DB: DELETE FROM mv_product_rank_weekly WHERE base_date=?
DB-->>Truncate: rowsDeleted
Truncate-->>Job: FINISHED
Job->>Reader: read(product_metrics for 7 days)
Reader->>DB: SELECT ... WHERE metric_hour >= start AND metric_hour < end
DB-->>Reader: ProductAggregation stream
loop chunk
Reader->>Processor: process(item)
Processor-->>Writer: MvProductRankWeekly
end
Writer->>DB: INSERT INTO mv_product_rank_weekly (...)
DB-->>Writer: success
Writer-->>Job: FINISHED
Job-->>Scheduler: Job Execution Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (18)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/RankingApiE2ETest.java (1)
191-197: 주간 테스트의baseDate계산이 하드코딩되어 있어 회귀 탐지력이 떨어진다.
LocalDate.of(2026, 4, 13)이 실제로 그 주의 월요일임을 테스트 스스로 검증하지 못하므로, 달력 경계(연말/연초 주) 회귀를 잡아내지 못한다. 운영 관점에서 주차 계산 버그는 전체 주간 랭킹이 비어 보이는 장애로 이어지므로, 월요일 계산을TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)로 도출하거나 ISO 주 경계(예: 2025-12-29 주, 2027-01-04 주)를 다루는 케이스 추가를 권고한다.🧪 보완 테스트 예시
// arrange LocalDate wednesday = LocalDate.of(2026, 12, 30); // 연말 경계 LocalDate monday = wednesday.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); // ... monday 기준 저장 및 period=WEEKLY 로 조회🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/RankingApiE2ETest.java` around lines 191 - 197, The test hardcodes LocalDate monday = LocalDate.of(2026, 4, 13) which risks missing week-boundary regressions; change the setup to compute the week's Monday dynamically by deriving a reference date (e.g., a Wednesday or test-specific date) and calling with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)) to produce the monday used when creating MvProductRankWeekly via weeklyRankingJpaRepository.save(...) and when asserting weekly queries; also add at least one additional case using a boundary reference date (e.g., end-of-year like 2026-12-30) to ensure ISO/week-boundary behavior is covered.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
8-36: commerce-api의 JPA 엔티티와 이름·필드가 중복되어 혼동 위험이 있다.동일 패키지명
com.loopers.domain.ranking에 동일 클래스명MvProductRankMonthly가 commerce-api(@Entity)와 commerce-batch(POJO) 양쪽에 존재한다. 두 모듈이 같은 클래스패스에 합쳐지는 일은 없겠으나, IDE 검색/리팩터링 시 혼동이 쉽고 스키마 변경 시 두 쪽을 동기화하지 않으면 배치 INSERT 컬럼과 API 조회 컬럼이 어긋나 무음 장애(silent data drift)가 발생한다. 운영 관점에서 권고는 다음과 같다:
- 배치 측 POJO는
BatchMvProductRankMonthly등으로 이름을 다르게 하거나, 패키지를com.loopers.batch.domain.ranking으로 분리한다.- 컬럼 매핑(JdbcBatchItemWriter의 INSERT SQL)과 엔티티
@Column이 동기화되도록 통합 테스트(배치 E2E)에서 스키마 대비 검증을 추가한다.추가로
equals/hashCode/toString이 없어 디버깅 로그/단위 테스트 비교 시 식별성이 낮다. 불변 값 객체이므로record로 전환하면 한 번에 해결된다(동일 PR의ProductAggregation과 일관성도 확보). As per coding guidelines: "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다. equals/hashCode는 식별자 기반 또는 값 기반으로 일관되게 설계한다."♻️ record 전환 예시
-@Getter -public class MvProductRankMonthly { - - private final Long productId; - private final int rank; - private final double score; - private final long totalLike; - private final long totalOrder; - private final long totalView; - private final long totalSales; - private final LocalDate baseDate; - - private MvProductRankMonthly(Long productId, int rank, double score, - long totalLike, long totalOrder, long totalView, - long totalSales, LocalDate baseDate) { - ... - } - - public static MvProductRankMonthly of(...) { ... } -} +public record MvProductRankMonthly( + Long productId, + int rank, + double score, + long totalLike, + long totalOrder, + long totalView, + long totalSales, + LocalDate baseDate +) { + public static MvProductRankMonthly of(Long productId, int rank, double score, + long totalLike, long totalOrder, long totalView, + long totalSales, LocalDate baseDate) { + return new MvProductRankMonthly(productId, rank, score, totalLike, totalOrder, totalView, totalSales, baseDate); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java` around lines 8 - 36, The POJO MvProductRankMonthly in com.loopers.domain.ranking collides with the commerce-api JPA entity of the same name and lacks equals/hashCode/toString; rename or relocate this batch value object (e.g., BatchMvProductRankMonthly or package com.loopers.batch.domain.ranking) and update all references to the constructor/factory method (MvProductRankMonthly.of and the private constructor) to the new type, or convert it into a Java record to get immutability and generated equals/hashCode/toString; additionally add an integration test that verifies the batch INSERT JDBC column order/names used by JdbcBatchItemWriter match the commerce-api `@Column` mapping to prevent silent schema drift.http/commerce-api/ranking.http (1)
1-21:{{today}}변수 선언이 누락되어 있다.1번 요청은 "오늘 날짜"를 의도하는 주석이 있으나 실제 URL에는
date파라미터가 없고, 주석과 상충된다. 2·3·4번 요청의date=20260416도 환경 변수({{date}})가 아닌 하드코딩이라 연·월이 바뀌면 수동 수정이 필요하다. http-client 환경 파일 또는GET .../rankings?date={{today}}형태로 통일해 문서와 실제 호출을 일치시키기를 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@http/commerce-api/ranking.http` around lines 1 - 21, Declare a reusable date environment variable and use it in the ranking requests: add a {{today}} (or generic {{date}}) variable to the HTTP client environment file and update the GET calls to /api/v1/rankings to pass ?date={{today}} (replace hardcoded 20260416 instances with {{today}} or {{date}}); also modify the first request comment/URL so the “오늘 날짜” example actually includes the date query param (?date={{today}}) to keep documentation and requests consistent.apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingPeriod.java (1)
3-7: API 문서에 enum 파라미터의 대문자 고정 요구사항을 명시해야 한다.Spring의 기본 enum 바인딩은
Enum.valueOf()를 사용하므로 대소문자를 엄격히 구분한다.period=daily,period=Daily같은 소문자 또는 혼합 형식은 400으로 처리된다. 현재ranking.http에 대문자 예제만 제시되어 있으나 "대문자 고정" 요구사항을 명시적으로 문서화하면 외부 클라이언트의 실수로 인한 운영 중 400 에러 증가를 사전에 방지할 수 있다. 공개 API 명세에도 동일하게 반영하기를 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingPeriod.java` around lines 3 - 7, Document that the RankingPeriod enum (values: DAILY, WEEKLY, MONTHLY) must be provided in uppercase in external requests because Spring binds enums via Enum.valueOf() (case-sensitive); update the API spec and the ranking.http example to explicitly state "use uppercase values: DAILY, WEEKLY, MONTHLY" and include both the accepted exact values and a note that lowercase/mixed-case (e.g., "daily" or "Daily") will cause a 400 Bad Request so clients can avoid this error.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingJpaRepository.java (1)
14-17: 동일 코드가 Monthly와 중복되며, 동점 score에 대한 정렬 안정성이 보장되지 않는다.
ORDER BY w.rank ASC단일 정렬은 rank 값이 중복될 경우(배치의 rank 채번 버그 또는 동일 score 타이브레이크 미적용으로 인해 동일 rank가 나올 가능성) 페이지 간 결과가 비결정적이 된다. Processor는AtomicInteger로 순번을 매기므로 현재는 중복 rank가 발생하지 않지만, 인프라 레이어 쿼리는 방어적으로 보조 정렬 키(예:productId)를 두는 편이 좋다.♻️ 제안 수정
- `@Query`("SELECT w.productId FROM MvProductRankWeekly w WHERE w.baseDate = :baseDate ORDER BY w.rank ASC") + `@Query`("SELECT w.productId FROM MvProductRankWeekly w WHERE w.baseDate = :baseDate ORDER BY w.rank ASC, w.productId ASC") List<Long> findProductIdsByBaseDate(`@Param`("baseDate") LocalDate baseDate, Pageable pageable);Monthly 쪽도 동일하게 반영이 필요하다.
추가 테스트: 동일
baseDate에 rank가 (가상으로) 중복일 때 페이지네이션 결과가 결정적이 되는지 검증하는 단위 테스트.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingJpaRepository.java` around lines 14 - 17, The query in findProductIdsByBaseDate on MvProductRankWeekly uses a single ORDER BY w.rank ASC which is non-deterministic when ranks tie; update the JPQL to add a stable secondary sort (e.g., ORDER BY w.rank ASC, w.productId ASC) in WeeklyRankingJpaRepository and make the same change in the Monthly repository/query, then add a unit test that inserts rows with the same baseDate and duplicate rank values and verifies pagination of findProductIdsByBaseDate returns a deterministic, consistent ordering across pages.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (1)
8-37: 값 객체로서는 무난하나, 동일성/검증이 누락되어 있다.불변 필드 + 정적 팩토리 구성은 적절하다. 다만 운영/디버깅 관점에서 다음을 권장한다.
productId != null,rank >= 1,totalLike/Order/View/Sales >= 0등 팩토리 단계의 사전조건 검증이 없다. Processor의 버그(예: rank=0에서 시작)가 MV에 그대로 적재되어도 런타임에서 드러나지 않는다.- DTO/값 객체 성격상
equals/hashCode/toString이 로그/테스트 단계에서 유용하다. 현재는 Lombok@Getter만 있어 실패 케이스 진단이 번거롭다.추가 테스트: 팩토리 사전조건 검증 단위 테스트(음수 카운터, rank<=0, null productId).
As per coding guidelines, "불변성과 캡슐화를 점검한다. ... equals/hashCode는 식별자 기반 또는 값 기반으로 일관되게 설계한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java` around lines 8 - 37, Add precondition validation in MvProductRankWeekly.of (and/or the private constructor) to enforce productId != null, rank >= 1, and totalLike/totalOrder/totalView/totalSales >= 0 (throw IllegalArgumentException with clear messages on violation), and implement value semantics by overriding equals(Object), hashCode(), and toString() based on all fields (or at least productId and baseDate plus rank/score as needed) so the value object has predictable identity and readable logs/tests; update unit tests to cover null productId, rank<=0, and negative counters.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.java (1)
26-26:RankingPeriodenum의 대소문자 변환 처리를 추가하여 사용성을 개선하도록 제안한다.Spring의 기본
StringToEnumConverter는 대소문자를 구분하므로period=daily,period=Weekly같은 입력이 실패한다. 현재ApiControllerAdvice가MethodArgumentTypeMismatchException을 이미 적절히 처리하여 표준 에러 응답 포맷을 보장하므로 응답 형식 일관성 문제는 해결되어 있으나, 운영 관점에서 클라이언트 오입력 허용성을 높이기 위해 다음 중 하나를 권장한다.
RankingPeriod에@JsonCreator를 추가하여 대소문자를 무시하고 파싱하거나,Converter<String, RankingPeriod>를 구현해 동일한 효과를 낸다.- 미지원 값은 자동으로
CoreException(또는MethodArgumentTypeMismatchException→ 400)으로 변환되도록 처리한다.추가 테스트:
RankingApiE2ETest에 (1) 소문자period=daily, (2) 혼합 대소문자period=Weekly, (3) 미지원 값period=HOURLY세 케이스의 상태 코드 일관성을 검증하는 테스트를 추가한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.java` at line 26, Add case-insensitive parsing for the RankingPeriod enum so requests like period=daily or period=Weekly succeed: modify the RankingPeriod enum to include a `@JsonCreator` static factory that normalizes input (e.g., trim, toUpperCase) and maps to the appropriate enum constant (throwing an IllegalArgumentException for unsupported values), or alternatively implement a Converter<String, RankingPeriod> bean to do the same; ensure this change works with the `@RequestParam` in RankingController and keeps MethodArgumentTypeMismatchException/CoreException behavior for completely invalid inputs. Also add three assertions in RankingApiE2ETest verifying responses for (1) period=daily, (2) period=Weekly, and (3) unsupported period=HOURLY to confirm success for case-insensitive inputs and consistent 400 handling for invalid values.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java (1)
28-31:targetYearMonth파싱 실패 시 예외 메시지 보강이 필요하다.잘못된 파라미터(예:
2026-04,202613, 빈 문자열)가 들어오면DateTimeParseException이 그대로 전파되어 원인 파라미터 값과 기대 포맷이 로그에서 즉시 식별되지 않는다. 운영 시 재실행을 위해 어떤 값을 넘겼는지 찾아야 하므로 원문과 기대 포맷을 포함해IllegalArgumentException으로 감싸고 cause를 보존하는 편이 낫다. Weekly 쪽(WeeklyRankingJobConfig)에서 월요일 검증에IllegalArgumentException을 쓰는 것과도 일관된다.♻️ 수정안
`@Value`("#{jobParameters['targetYearMonth']}") public void setTargetYearMonth(String targetYearMonth) { - this.baseDate = YearMonth.parse(targetYearMonth, DateTimeFormatter.ofPattern("yyyyMM")).atDay(1); + try { + this.baseDate = YearMonth.parse(targetYearMonth, DateTimeFormatter.ofPattern("yyyyMM")).atDay(1); + } catch (RuntimeException e) { + throw new IllegalArgumentException( + "targetYearMonth 형식이 올바르지 않다(기대: yyyyMM, 입력: " + targetYearMonth + ")", e); + } }추가 테스트:
targetYearMonth를 빈 문자열/2026-04/202613으로 넘겼을 때 Job이 즉시 실패하고 원문 값이 메시지에 포함되는지 확인하는 케이스를MonthlyRankingJobE2ETest에 보강한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java` around lines 28 - 31, The setTargetYearMonth method in MonthlyRankingProcessor should catch DateTimeParseException when parsing targetYearMonth and rethrow an IllegalArgumentException that includes the original parameter value and the expected pattern ("yyyyMM"), preserving the caught exception as the cause so logs show both the bad input and the underlying parse error; update MonthlyRankingJobE2ETest to add cases passing "", "2026-04", and "202613" and assert the job fails immediately with an IllegalArgumentException whose message contains the submitted value and the expected format.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
66-83: baseDate 계산 로직 중복을RankingPeriod로 응집하는 편이 유지보수에 유리하다.
resolveProductIds와resolveCount에서 WEEKLY/MONTHLY의 baseDate 산출 로직이 동일하게 두 번 등장한다. 향후 "주간 기준을 일요일로 바꾼다" 같은 변경이 들어오면 한쪽만 수정되는 실수가 발생하기 쉽고, 배치 측WeeklyRankingJobConfig가 요구하는 "월요일" 규칙(배치에서 월요일이 아니면 예외)과 API 쪽 규칙이 어긋나면 MV 조회가 조용히 0건을 반환하는 운영 장애로 이어진다.규칙을
RankingPeriod에resolveBaseDate(LocalDate)로 둬 한 곳에서 관리하면 응집도가 올라가고 테스트 포인트도 단일화된다.♻️ 수정안
private List<Long> resolveProductIds(LocalDate date, RankingPeriod period, long offset, long limit) { return switch (period) { case DAILY -> rankingRepository.findProductIdsByRank(date, offset, limit); - case WEEKLY -> weeklyRankingRepository.findProductIdsByBaseDate( - date.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)), offset, limit); - case MONTHLY -> monthlyRankingRepository.findProductIdsByBaseDate( - date.withDayOfMonth(1), offset, limit); + case WEEKLY -> weeklyRankingRepository.findProductIdsByBaseDate(period.resolveBaseDate(date), offset, limit); + case MONTHLY -> monthlyRankingRepository.findProductIdsByBaseDate(period.resolveBaseDate(date), offset, limit); }; } private long resolveCount(LocalDate date, RankingPeriod period) { return switch (period) { case DAILY -> rankingRepository.countByDate(date); - case WEEKLY -> weeklyRankingRepository.countByBaseDate( - date.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY))); - case MONTHLY -> monthlyRankingRepository.countByBaseDate(date.withDayOfMonth(1)); + case WEEKLY -> weeklyRankingRepository.countByBaseDate(period.resolveBaseDate(date)); + case MONTHLY -> monthlyRankingRepository.countByBaseDate(period.resolveBaseDate(date)); }; }
RankingPeriod예시:public enum RankingPeriod { DAILY { `@Override` public LocalDate resolveBaseDate(LocalDate d) { return d; } }, WEEKLY { `@Override` public LocalDate resolveBaseDate(LocalDate d) { return d.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); } }, MONTHLY { `@Override` public LocalDate resolveBaseDate(LocalDate d) { return d.withDayOfMonth(1); } }; public abstract LocalDate resolveBaseDate(LocalDate date); }추가 테스트:
RankingPeriod단위 테스트에서 월요일/일요일/월말 경계값으로 baseDate 산출을 검증한다.[As per coding guidelines] "도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java` around lines 66 - 83, Extract the base-date calculation into a new abstract method on RankingPeriod (e.g. add public abstract LocalDate resolveBaseDate(LocalDate date) and implement DAILY/WEEKLY/MONTHLY variants) and update resolveProductIds and resolveCount to call period.resolveBaseDate(date) before passing the value to weeklyRankingRepository/monthlyRankingRepository; ensure both methods use the same resolved baseDate and add unit tests for RankingPeriod.resolveBaseDate covering boundary cases (e.g. Monday/Sunday/month-end).apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (1)
15-71:MvProductRankWeekly와MvProductRankMonthly의 구조 중복을 공통화하는 것을 권장한다.두 엔티티는
id,productId,rank,score,totalLike/Order/View/Sales,baseDate,created_at, 생성자/팩토리/@PrePersist까지 거의 완전히 동일하다. 한쪽에만 반영되는 드리프트(예: 컬럼 추가, 타임존 수정, 검증 로직 추가)가 발생하면 조용히 주간/월간 동작이 갈라진다.
MappedSuperclass(예:AbstractMvProductRank)로 공통 필드/생성자/PrePersist를 올리고 테이블만 분리하는 방식, 또는 하나의 엔티티에@Enumerated(EnumType.STRING) RankingPeriod를 두고@Table을 분기하는 방식 중 하나를 택하면 유지보수 비용이 크게 줄어든다.[As per coding guidelines] "도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다." — 중복된 인프라 매핑/감사 필드 관심사를 상위 타입으로 모을 것을 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java` around lines 15 - 71, Extract the duplicated fields, constructors, factory method and `@PrePersist` into a `@MappedSuperclass` named e.g. AbstractMvProductRank that declares id, productId, rank, score, totalLike/Order/View/Sales, baseDate, createdAt, a protected constructor and a public static factory helper (or protected constructor + subclass factory), and the prePersist method; then have MvProductRankWeekly and MvProductRankMonthly become simple `@Entity` classes that extend AbstractMvProductRank and keep only their `@Table/`@Entity annotations and any period-specific behavior. Ensure annotations currently on the fields (e.g., `@Id`, `@Column` names, `@GeneratedValue`) move to the superclass, adjust visibility of constructors (make them protected) and update the static of(...) factory on subclasses to call the superclass constructor or helper so persisting and `@PrePersist` behavior remain identical.apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java (1)
118-210: 주간 일요일/월간 월말 경계 테스트가 빠져 있다.현재 WEEKLY는 수요일 → 월요일만, MONTHLY는 15일 → 1일만 확인한다. 정작 회귀가 가장 잘 나오는 케이스는 경계에 몰려 있다.
- WEEKLY: 일요일(예:
2026-04-19)을 넣었을 때 같은 주 시작 월요일인2026-04-13으로 해석되는지(일요일을 다음 주로 간주하는 실수 방지).- WEEKLY: 월요일 자체(예:
2026-04-13)를 넣었을 때 그대로2026-04-13이 쓰이는지.- MONTHLY: 월말(예:
2026-04-30)과 월 1일(2026-04-01)에서 모두2026-04-01이 쓰이는지.- MONTHLY: 연말 경계(
2026-12-31→2026-12-01)에서도withDayOfMonth(1)이 기대대로 동작하는지.추가 테스트 제안:
`@Test` void getPage_weekly_resolvesSundayToMondayOfSameWeek() { LocalDate sunday = LocalDate.of(2026, 4, 19); LocalDate monday = LocalDate.of(2026, 4, 13); when(weeklyRankingRepository.findProductIdsByBaseDate(monday, 0L, 20L)).thenReturn(List.of()); rankingFacade.getPage(sunday, RankingPeriod.WEEKLY, 1, 20); verify(weeklyRankingRepository).findProductIdsByBaseDate(monday, 0L, 20L); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java` around lines 118 - 210, Add boundary tests in RankingFacadeTest to verify base-date calculation around week/month edges: call rankingFacade.getPage with Sunday (e.g., LocalDate.of(2026,4,19)) and assert weeklyRankingRepository.findProductIdsByBaseDate is invoked with that week’s Monday (LocalDate.of(2026,4,13)), call it with Monday itself (LocalDate.of(2026,4,13)) and verify same Monday is used, call it with month-end (e.g., 2026-04-30) and month-start (2026-04-01) and verify monthlyRankingRepository.findProductIdsByBaseDate is invoked with the 1st (withDayOfMonth(1)), and add a year-end case (2026-12-31 -> 2026-12-01); use the existing mocking pattern (when(...).thenReturn(...), verify(...)) and the same methods: rankingFacade.getPage, weeklyRankingRepository.findProductIdsByBaseDate, monthlyRankingRepository.findProductIdsByBaseDate.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java (2)
96-104:DataSourceTransactionManager를 매 Step 빌드마다 new 로 생성한다.
monthlyAggregateAndRankStep()이 호출될 때마다new DataSourceTransactionManager(dataSource)가 생성된다. 단일 Job 기준 큰 영향은 없지만,PlatformTransactionManager는 보통 싱글톤 빈으로 공유하여 동일DataSource의 트랜잭션 전파/중첩 호환을 보장하는 것이 관용적이다. Bean 으로 노출하고 주입받아 재사용할 것을 권고한다.♻️ 제안 수정
+ `@Bean` + public DataSourceTransactionManager batchDataSourceTransactionManager(DataSource dataSource) { + return new DataSourceTransactionManager(dataSource); + } + `@JobScope` `@Bean`(STEP_AGGREGATE_AND_RANK) - public Step monthlyAggregateAndRankStep() { + public Step monthlyAggregateAndRankStep(DataSourceTransactionManager batchDataSourceTransactionManager) { return new StepBuilder(STEP_AGGREGATE_AND_RANK, jobRepository) - .<ProductAggregation, MvProductRankMonthly>chunk(CHUNK_SIZE, new DataSourceTransactionManager(dataSource)) + .<ProductAggregation, MvProductRankMonthly>chunk(CHUNK_SIZE, batchDataSourceTransactionManager) .reader(monthlyMetricsItemReader(null))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java` around lines 96 - 104, monthlyAggregateAndRankStep currently constructs a new DataSourceTransactionManager for each Step build (new DataSourceTransactionManager(dataSource)); instead, define a singleton PlatformTransactionManager/DataSourceTransactionManager bean and inject it into this config, then use that injected transactionManager in the StepBuilder (replace new DataSourceTransactionManager(dataSource) with the injected transactionManager) so the same transaction manager instance backs monthlyAggregateAndRankStep and preserves proper propagation/transaction semantics for the shared dataSource.
48-63:ORDER BY score DESC + LIMIT 100동점 처리 전략이 비결정적이다.동일
score가 다수일 때 MySQL 은 정렬 안정성을 보장하지 않으므로 재실행 시 상위 100개가 달라질 수 있다. 또한 윈도우 함수를 쓰지 않고 애플리케이션에서rank를 부여하므로, 동점 상품의rank도 매 실행마다 뒤바뀔 수 있다. 운영 관점에서 랭킹 변동이 사용자 혼란과 캐시 무효화 이슈로 번질 수 있다. 아래 중 하나를 고려한다.
ORDER BY score DESC, product_id ASC로 타이브레이커 보강.- MySQL 8 의
DENSE_RANK()/RANK()윈도우 함수로 동점 시맨틱을 명시.추가 테스트 제안: 동일 score 다수 케이스에 대해 재실행 시
product_id순으로 rank 가 고정되는지 검증.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java` around lines 48 - 63, 쿼리의 ORDER BY가 score만 사용되어 동점시 결과가 비결정적이므로 MonthlyRankingJobConfig의 MONTHLY_METRICS_SQL을 수정해 동점 타이브레이커를 추가하거나 윈도우 함수를 사용하도록 변경하세요; 간단한 fix는 ORDER BY 절에 ", product_id ASC"를 추가해 동일 score일 때 product_id 순으로 결과가 고정되도록 하고, 대안으로 MySQL 8 이상에서는 DENSE_RANK()/RANK()를 사용해 명시적 랭크를 계산하는 방식을 도입할 수 있습니다; 변경 후에는 동일 score 다수 케이스에 대해 재실행 시 product_id 순으로 상위 100개와 애플리케이션의 rank 부여가 결정적인지 검증하는 테스트를 추가하세요.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java (3)
71-79: Job 레벨JobParametersValidator적용을 권고한다.현재는 필수 파라미터 검증이 Step 2 의 Reader 초기화 시점에만 존재하여, Step 1 (
truncateWeeklyMvStep) 이 잘못된/누락된targetDate로 선행 실행될 여지가 있다(예:base_date = NULL로 DELETE → 0건 영향이지만 감사 로그 관점에서 혼란). Job 빌더에DefaultJobParametersValidator를 달아 Step 진입 전에 실패시키는 것이 안전하다..validator(new DefaultJobParametersValidator(new String[]{"targetDate"}, new String[]{"run.id"}))추가 테스트 제안: 파라미터 누락 시
truncateWeeklyMvStep이 실행되기 전에 Job 이 FAILED 로 종료되는지, 그리고mv_product_rank_weekly의 기존 데이터가 그대로 보존되는지 검증.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java` around lines 71 - 79, Add a JobParametersValidator at the job level in weeklyRankingJob() so the job fails early on missing/invalid parameters; update the JobBuilder for JOB_NAME to call .validator(new DefaultJobParametersValidator(new String[]{"targetDate"}, new String[]{"run.id"})) so missing targetDate causes the Job to fail before truncateWeeklyMvStep() runs (leaving mv_product_rank_weekly intact), keeping the existing .start(truncateWeeklyMvStep()).listener(jobListener) and .next(weeklyAggregateAndRankStep()) flow unchanged.
43-62:CHUNK_SIZE=10과 동점자 정렬 안정성에 대한 재검토가 필요하다.
- SQL 에서
LIMIT 100으로 상위 100건을 제한하므로 chunk 10 은 총 10 개 커밋/트랜잭션이 발생한다. 실패 복구가 특별히 필요 없는 단일 INSERT 패턴이라면 chunk 를 100(또는 50) 으로 키우거나 Skip/Retry 정책을 명시적으로 설계하는 편이 운영상 더 명확하다.ORDER BY score DESC만으로는 동점 시 순서가 비결정적이다. Processor 에서AtomicInteger로 rank 를 부여하므로 재실행 시 동점자 간rank가 뒤바뀔 수 있다.ORDER BY score DESC, product_id ASC로 타이브레이커를 명시할 것을 권고한다.추가 테스트 제안: 동일 score 를 가진 다수 상품에 대해 Job 을 두 번 실행해도
(product_id, rank)가 동일한지 단언하는 케이스.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java` around lines 43 - 62, CHUNK_SIZE is set to 10 while the SQL limits results to 100, causing 10 separate commits and non-deterministic ranking for ties; either increase CHUNK_SIZE (e.g., to 100 or 50) or explicitly document/design a Skip/Retry policy for the chunked insert behavior, and make the SQL deterministic by adding a tiebreaker to WEEKLY_METRICS_SQL (change ORDER BY score DESC to ORDER BY score DESC, product_id ASC) so that Processor/AtomicInteger rank assignment is stable across reruns; also add a unit/integration test that runs the job twice on products with identical scores and asserts (product_id, rank) equality.
95-103:DataSourceTransactionManager인스턴스를 매 Step 호출마다 새로 생성한다.
MonthlyRankingJobConfig와 동일 패턴이다.@JobScope로 인해 Job 실행마다 Step 이 재생성되고, 그때마다new DataSourceTransactionManager(dataSource)가 만들어진다. 비용이 크지는 않지만 Spring 관용을 벗어나고, 필요 시 트랜잭션 매니저에 전파 옵션 등을 튜닝할 때 산재된 new 호출을 모두 고쳐야 한다. 빈으로 노출해 공용화할 것을 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java` around lines 95 - 103, 현재 weeklyAggregateAndRankStep() 내부에서 매 호출마다 new DataSourceTransactionManager(dataSource)로 트랜잭션 매니저를 생성하고 있는데, JobScope로 인해 Step이 재생성될 때마다 인스턴스가 불필요하게 만들어집니다; DataSourceTransactionManager를 `@Bean으로` 등록(예: dataSourceTransactionManager)하고 WeeklyRankingJobConfig에서 생성자나 필드 주입으로 해당 빈을 참조하도록 변경한 뒤 weeklyAggregateAndRankStep()의 chunk(...) 호출에서 new 대신 주입된 트랜잭션 매니저(예: dataSourceTransactionManager)를 사용하도록 수정하세요; 동일 패턴인 MonthlyRankingJobConfig의 구현 방식과 일관되게 적용해 전역 설정(전파 옵션 등) 유지·튜닝이 용이하도록 하세요.apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java (2)
41-46: 테스트 격리 전략 보완을 권고한다.
@BeforeEach에서DELETE FROM ...만 수행하므로 다른 테스트 클래스와 동일 DB 를 공유하면 상호 간섭 가능성이 있다. 운영 관점에서 플래키 테스트의 주원인이 되므로 테스트 클래스 단위의 트랜잭션 롤백 또는 Testcontainers 의 per-class 격리, 혹은@Sql(executionPhase = AFTER_TEST_METHOD, scripts=...)을 통한 정리 전략을 보강하는 것이 좋다. 또한BATCH_JOB_INSTANCE등 Spring Batch 메타 테이블은 정리하지 않으므로, 동일JobParameters재실행 시JobInstanceAlreadyCompleteException이 발생하지 않도록JobLauncherTestUtils의getUniqueJobParameters()사용 여부를 확인하기 바란다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java` around lines 41 - 46, The test currently only deletes mv_product_rank_weekly and product_metrics in WeeklyRankingJobE2ETest.setUp(), which leaves shared DB state (including Spring Batch meta tables like BATCH_JOB_INSTANCE) and can cause cross-test flakiness; update the test class to enforce per-class isolation by either (a) annotating the class/methods to run with a transactional rollback (use `@Transactional` on the test and ensure the framework rolls back after each test) or (b) run the test with Testcontainers per-class DB lifecycle, or (c) add `@Sql`(executionPhase = AFTER_TEST_METHOD, scripts=...) to fully clean application tables and Spring Batch meta tables, and ensure job runs supply unique parameters by using JobLauncherTestUtils.getUniqueJobParameters() so JobInstanceAlreadyCompleteException cannot occur.
76-99: 경계값 테스트가 누락되어 있다.집계 범위가
[targetDate, targetDate+7일)임에도 경계 테스트가 없어 회귀 시 off-by-one 을 잡아낼 수 없다. 운영 관점에서 주 경계는 실제 장애 가능성이 가장 높은 지점이다. 아래와 같이 포함/제외 경계를 명확히 검증하는 케이스 추가를 권고한다.
metric_hour = targetDate 00:00:00(포함되어야 함)metric_hour = targetDate.minusSeconds(1)(제외되어야 함)metric_hour = targetDate.plusDays(7).minusSeconds(1)(포함되어야 함)metric_hour = targetDate.plusDays(7)(제외되어야 함)deleted_at IS NOT NULL인 행이 제외되는지 확인하는 케이스🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java` around lines 76 - 99, Add boundary tests to WeeklyRankingJobE2ETest to verify the inclusive/exclusive aggregation window [targetDate, targetDate+7days) and deletion filtering: create test cases (or extend fewProducts_allRankedInOrder) that call insertMetrics(...) with metric_hour equal to targetDate 00:00:00 (should be included), targetDate.minusSeconds(1) (should be excluded), targetDate.plusDays(7).minusSeconds(1) (included), and targetDate.plusDays(7) (excluded), and also insert a row with deleted_at non-null to assert it is ignored; after launching the job via jobLauncherTestUtils.launchJob(params) use jdbcTemplate queries (mv_product_rank_weekly and any raw metrics table) and assertions (countMv(), rank lookup, etc.) to assert each boundary behaves as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 37-45: In getPage(LocalDate date, RankingPeriod period, int page,
int size) add explicit null checks for period and date (e.g.,
Objects.requireNonNull or throw a CoreException with a clear error code/message)
before any use (the switch(period) and any TemporalAdjusters calls) so NPEs are
avoided and errors follow ApiControllerAdvice handling; update
resolveProductIds/resolveCount callers if needed to accept the validated values;
and add a unit test asserting getPage(null, null, 1, 20) throws the expected
exception type and message.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`:
- Around line 45-70: The createdAt assignment in MvProductRankWeekly currently
uses ZonedDateTime.now() (see createdAt field and prePersist()), which relies on
the JVM default TZ; change prePersist() to set createdAt with an explicit zone
(e.g., ZonedDateTime.now(ZoneId.of("Asia/Seoul")) or
ZonedDateTime.now(ZoneOffset.UTC)) or refactor both MvProductRankWeekly and
MvProductRankMonthly to inherit a single `@MappedSuperclass/BaseEntity` or project
Auditing convention that centralizes createdAt handling; after changing, add an
integration test in JpaWeeklyRankingRepositoryTest to assert that the persisted
created_at uses the chosen timezone/offset.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaMonthlyRankingRepository.java`:
- Around line 18-22: In findProductIdsByBaseDate (and mirror the same fix in
JpaWeeklyRankingRepository) add defensive validation: if limit <= 0 throw an
IllegalArgumentException (or similar), and if offset % limit != 0 throw an
IllegalArgumentException to explicitly reject non-multiple offsets; compute
pageNumber using Math.toIntExact(offset / limit) and build
PageRequest.of(pageNumber, Math.toIntExact(limit)) to avoid silent
truncation/overflow; finally add unit tests that assert behavior for limit=0 and
for non-multiple offset values (e.g., offset=3, limit=2) to codify the expected
exceptions.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepository.java`:
- Around line 18-22: The conversion from offset/limit in
JpaWeeklyRankingRepository.findProductIdsByBaseDate is unsafe: validate that
limit > 0 (throw IllegalArgumentException or return empty) to avoid division by
zero/invalid PageRequest, compute the page number using floor division to handle
non-multiple offsets (use Math.floorDiv(offset, limit)) and clamp the result to
an int (e.g. (int)Math.min(Integer.MAX_VALUE, Math.floorDiv(offset, limit)))
before calling PageRequest.of, and apply the same defensive change to the
corresponding MonthlyRankingRepository implementation; add unit tests for
offset=3,limit=2, limit=0 and extreme overflow offsets.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepositoryTest.java`:
- Around line 38-119: Add a test that verifies filtering by baseDate by
inserting rows with two different LocalDate values and asserting only rows
matching the target date are returned; specifically, in the test class use
weeklyRankingJpaRepository.save(MvProductRankWeekly.of(..., target)) and save
another MvProductRankWeekly with a different date (other), then call
weeklyRankingRepository.findProductIdsByBaseDate(target, 0, 10) and assert it
contains only the IDs for the target date and call
weeklyRankingRepository.countByBaseDate(target) and assert it equals 1 to ensure
the repository methods (findProductIdsByBaseDate and countByBaseDate) properly
filter by baseDate.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/RankingApiE2ETest.java`:
- Around line 167-180: The test period_invalidValue_returns400 currently only
asserts the HTTP status and should also verify the error response body to
prevent format regressions; update the assertions in
RankingApiE2ETest.period_invalidValue_returns400 to assert
response.getBody().meta().result() equals ApiResponse.Metadata.Result.FAIL and
response.getBody().meta().errorCode() equals "BAD_REQUEST" (in addition to the
existing HttpStatus.BAD_REQUEST check), using assertAll or equivalent to group
the assertions against the
ResponseEntity<ApiResponse<PageResponse<RankingDto.Response>>> variable named
response.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java`:
- Around line 76-104: The job currently allows truncateMonthlyMvStep() to run
before parameters are validated, risking DELETE with null/invalid base_date;
modify monthlyRankingJob() to set a JobParametersValidator (e.g., new
DefaultJobParametersValidator(new String[]{"targetYearMonth"}, new
String[]{"run.id"})) on the JobBuilder (before calling .start(...)) so
missing/invalid targetYearMonth is rejected at job launch; ensure you reference
JOB_NAME / monthlyRankingJob(), keep RunIdIncrementer, and validate that
truncateMonthlyMvStep() and monthlyMetricsItemReader(null) are never executed
when validation fails.
- Around line 108-114: Add explicit validation for the targetYearMonth job
parameter before calling YearMonth.parse in monthlyMetricsItemReader: check for
null/blank and validate format using DateTimeFormatter.ofPattern("yyyyMM"), and
if invalid throw an IllegalArgumentException with a clear message (preserve
underlying DateTimeParseException as the cause) so the failure is identifiable;
implement a JobParametersValidator (follow the pattern in WeeklyRankingJobConfig
— e.g., MonthlyRankingJobParametersValidator) and register it as a bean to
enforce this check at job start; update tests to assert that missing
targetYearMonth causes the job to FAIL with getAllFailureExceptions() containing
an IllegalArgumentException and that malformed values like "2026-04" produce the
format validation failure.
- Around line 139-145: The INSERT SQL in MonthlyRankingJobConfig (.sql(...))
uses the reserved word rank unquoted causing MySQL syntax errors; update that
VALUES/column list to quote the rank identifier with backticks (`rank`) and
likewise update any JPQL/SQL ORDER BY or WHERE usages (e.g.,
findProductIdsByBaseDate, WeeklyRankingJpaRepository ORDER BY w.rank,
MonthlyRankingJpaRepository ORDER BY m.rank, and any E2E test SQL like WHERE
rank = 1) to reference `rank` consistently; as a follow-up, plan a schema/DTO
rename (e.g., ranking_order or position) across
MvProductRankMonthly/MvProductRankWeekly and all queries to remove reliance on a
reserved identifier.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java`:
- Around line 22-41: The current AtomicInteger rankCounter (field rankCounter
and its use in process(ProductAggregation)) can drift on chunk rollback/retry;
either move rank assignment out of the processor (assign just before insert in
the writer or let the DB use ROW_NUMBER() on insert) or add guards to
reset/decrement the counter on chunk failures by implementing
ItemReadListener#onReadError and ChunkListener#afterChunkError to restore
rankCounter to the last committed value; additionally add a clear comment near
rankCounter/setTargetYearMonth explaining the retry assumption and add an
integration test that simulates writer failure to assert
mv_product_rank_monthly.rank remains a contiguous 1..N sequence.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/TruncateMonthlyMvTasklet.java`:
- Around line 29-32: The setter setTargetYearMonth currently calls
YearMonth.parse(...) directly which will NPE on null and leak
DateTimeParseException; replace that inline parsing with a shared utility (e.g.,
a new parseTargetYearMonth(String) method) that first checks for null/empty and
throws a new IllegalArgumentException("Missing required job parameter:
targetYearMonth") with the original cause preserved when applicable, then wraps
DateTimeParseException into an IllegalArgumentException with a clear user-facing
message like "Invalid targetYearMonth format; expected yyyyMM" while passing the
caught exception as the cause; update setTargetYearMonth,
monthlyMetricsItemReader, and any Processor usages to call this new utility to
remove duplication and add tests for missing param and bad formats (e.g.,
"2026-04", "202613").
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/TruncateWeeklyMvTasklet.java`:
- Around line 25-32: TruncateWeeklyMvTasklet.execute currently deletes rows
using the raw job parameter baseDate and doesn't verify it's a Monday, risking
accidental deletes before the Reader (weeklyMetricsItemReader) fails; add a
pre-check in TruncateWeeklyMvTasklet.execute to throw an exception if
baseDate.getDayOfWeek() != MONDAY (same logic as weeklyMetricsItemReader) so the
Tasklet fails fast and avoids deleting unexpected rows, and when calling
jdbcTemplate.update convert the LocalDate to java.sql.Date (e.g.,
java.sql.Date.valueOf(baseDate)) or otherwise bind an explicit SQL DATE to avoid
JDBC driver SQLFeatureNotSupportedException.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/WeeklyRankingProcessor.java`:
- Around line 21-38: The current rankCounter field in WeeklyRankingProcessor
resets on step restart causing duplicate/missing ranks; either make the reader
restart-safe and ensure truncation runs, or persist/restore rank—apply one of
these fixes: (A) set weeklyMetricsItemReader.saveState(false) so the reader
always re-reads from start and set
truncateWeeklyMvStep.allowStartIfComplete(true) so truncation always runs before
ranking; or (B) implement rankCounter as an ItemStream on WeeklyRankingProcessor
(implement open/update/close using ExecutionContext) to restore the counter on
restart; apply the same change to MonthlyRankingProcessor and add tests that
force an exception mid-step then restart to assert final MV ranks are unique,
continuous integers with no duplicates/missing values.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java`:
- Around line 108-115: The weeklyMetricsItemReader in WeeklyRankingJobConfig
currently assumes targetDate is non-null and will NPE; add a null guard at the
start of weeklyMetricsItemReader to detect a missing `@Value` job parameter and
throw an IllegalArgumentException with a clear message like "targetDate job
parameter is required" (preserve cause if any), or alternatively enforce the
parameter at the job level by registering a JobParametersValidator that
validates presence and Monday-ness of "targetDate" before the step runs; ensure
the existing Monday check remains and update tests to assert
IllegalArgumentException is raised when targetDate is null or not a Monday.
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.java`:
- Around line 3-10: ProductAggregation fields are wrapper types but the
RowMapper currently uses rs.getLong()/rs.getDouble(), which converts SQL NULL to
0, so change the mapping to detect NULLs explicitly (use
rs.getObject(columnName, Long.class) / rs.getObject(columnName, Double.class) or
call rs.getLong()/rs.getDouble() and then check rs.wasNull()) and populate
ProductAggregation with null when appropriate; alternatively, update the SQL
aggregation to use COALESCE(SUM(...), 0) for each numeric column (including the
score expression) if you intend to treat missing aggregates as 0 — choose one
approach and apply consistently in the RowMapper that constructs new
ProductAggregation(...) so NULL vs 0 semantics are preserved.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.java`:
- Around line 40-119: Add four E2E tests in MonthlyRankingJobE2ETest to cover
idempotency, boundary, month-isolation, and soft-delete: 1) idempotency: run job
twice with the same JobParameters ("targetYearMonth" "202604") using
jobLauncherTestUtils.launchJob(...) and assert countMv() does not double (ensure
TruncateMonthlyMvTasklet deletes only base_date rows). 2) boundary:
insertMetrics at 2026-04-01T00:00 and at 2026-05-01T00:00, run the job and
assert only the 2026-04-01 row appears (verifies >= firstDay AND < nextMonth
behavior). 3) month isolation: insert product_metrics rows for 2026-03-xx and
2026-05-xx plus 2026-04-xx, run for 202604 and assert only April productIds are
in mv (use insertMetrics and countMv/queries). 4) soft-delete: insertMetrics
with deleted_at non-null for some product(s), run job and assert those products
are not aggregated into mv. Use existing helpers (insertMetrics, countMv,
jdbcTemplate queries) and JobParametersBuilder to build the targetYearMonth
param.
- Around line 90-96: The test's native SQL uses an unescaped MySQL reserved word
"rank" causing potential parse failures; update the jdbcTemplate.queryForObject
call in MonthlyRankingJobE2ETest (the code that assigns rank1ProductId via
jdbcTemplate.queryForObject on mv_product_rank_monthly) to wrap the column name
in backticks (i.e. use `\`rank\``) so the WHERE clause matches the entity
mapping; run the test and ensure countMv() and assertions remain unchanged.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`:
- Around line 48-56: The test failsWithoutTargetDate is brittle because absence
of targetDate currently triggers a NullPointerException from
weeklyMetricsItemReader instead of a clear parameter validation failure; either
add a JobParametersValidator to the job so missing targetDate triggers a
JobParametersInvalidException (then assert that JobExecution fails with
JobParametersInvalidException or check JobExecution#getAllFailureExceptions()
contains that type/message), or minimally strengthen the test to assert that
execution.getAllFailureExceptions() contains an IllegalArgumentException caused
by the missing targetDate (referencing the TruncateWeeklyMvTasklet and
weeklyMetricsItemReader failure path if you choose the minimal change).
- Around line 92-93: The SELECT uses the reserved identifier `rank` causing
syntax errors; update the schema and all SQL to use a non-reserved column name
(e.g., rename `rank` to `rank_no` or `ranking`) and update references in the
test query (WeeklyRankingJobE2ETest), the INSERTs/DDL in WeeklyRankingJobConfig,
and any other SQL/DTOs that reference `rank`; alternatively, if you must keep
the name temporarily, escape the identifier with backticks everywhere (e.g.,
`rank`) but prefer renaming for a permanent fix. Also add a new E2E test that
calls the reader/job with a non-Monday targetDate (e.g., 2026-04-14) and asserts
the job ends FAILED and throws IllegalArgumentException to validate the Monday
boundary check in the reader.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 66-83: Extract the base-date calculation into a new abstract
method on RankingPeriod (e.g. add public abstract LocalDate
resolveBaseDate(LocalDate date) and implement DAILY/WEEKLY/MONTHLY variants) and
update resolveProductIds and resolveCount to call period.resolveBaseDate(date)
before passing the value to weeklyRankingRepository/monthlyRankingRepository;
ensure both methods use the same resolved baseDate and add unit tests for
RankingPeriod.resolveBaseDate covering boundary cases (e.g.
Monday/Sunday/month-end).
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`:
- Around line 15-71: Extract the duplicated fields, constructors, factory method
and `@PrePersist` into a `@MappedSuperclass` named e.g. AbstractMvProductRank that
declares id, productId, rank, score, totalLike/Order/View/Sales, baseDate,
createdAt, a protected constructor and a public static factory helper (or
protected constructor + subclass factory), and the prePersist method; then have
MvProductRankWeekly and MvProductRankMonthly become simple `@Entity` classes that
extend AbstractMvProductRank and keep only their `@Table/`@Entity annotations and
any period-specific behavior. Ensure annotations currently on the fields (e.g.,
`@Id`, `@Column` names, `@GeneratedValue`) move to the superclass, adjust visibility
of constructors (make them protected) and update the static of(...) factory on
subclasses to call the superclass constructor or helper so persisting and
`@PrePersist` behavior remain identical.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingPeriod.java`:
- Around line 3-7: Document that the RankingPeriod enum (values: DAILY, WEEKLY,
MONTHLY) must be provided in uppercase in external requests because Spring binds
enums via Enum.valueOf() (case-sensitive); update the API spec and the
ranking.http example to explicitly state "use uppercase values: DAILY, WEEKLY,
MONTHLY" and include both the accepted exact values and a note that
lowercase/mixed-case (e.g., "daily" or "Daily") will cause a 400 Bad Request so
clients can avoid this error.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingJpaRepository.java`:
- Around line 14-17: The query in findProductIdsByBaseDate on
MvProductRankWeekly uses a single ORDER BY w.rank ASC which is non-deterministic
when ranks tie; update the JPQL to add a stable secondary sort (e.g., ORDER BY
w.rank ASC, w.productId ASC) in WeeklyRankingJpaRepository and make the same
change in the Monthly repository/query, then add a unit test that inserts rows
with the same baseDate and duplicate rank values and verifies pagination of
findProductIdsByBaseDate returns a deterministic, consistent ordering across
pages.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.java`:
- Line 26: Add case-insensitive parsing for the RankingPeriod enum so requests
like period=daily or period=Weekly succeed: modify the RankingPeriod enum to
include a `@JsonCreator` static factory that normalizes input (e.g., trim,
toUpperCase) and maps to the appropriate enum constant (throwing an
IllegalArgumentException for unsupported values), or alternatively implement a
Converter<String, RankingPeriod> bean to do the same; ensure this change works
with the `@RequestParam` in RankingController and keeps
MethodArgumentTypeMismatchException/CoreException behavior for completely
invalid inputs. Also add three assertions in RankingApiE2ETest verifying
responses for (1) period=daily, (2) period=Weekly, and (3) unsupported
period=HOURLY to confirm success for case-insensitive inputs and consistent 400
handling for invalid values.
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java`:
- Around line 118-210: Add boundary tests in RankingFacadeTest to verify
base-date calculation around week/month edges: call rankingFacade.getPage with
Sunday (e.g., LocalDate.of(2026,4,19)) and assert
weeklyRankingRepository.findProductIdsByBaseDate is invoked with that week’s
Monday (LocalDate.of(2026,4,13)), call it with Monday itself
(LocalDate.of(2026,4,13)) and verify same Monday is used, call it with month-end
(e.g., 2026-04-30) and month-start (2026-04-01) and verify
monthlyRankingRepository.findProductIdsByBaseDate is invoked with the 1st
(withDayOfMonth(1)), and add a year-end case (2026-12-31 -> 2026-12-01); use the
existing mocking pattern (when(...).thenReturn(...), verify(...)) and the same
methods: rankingFacade.getPage,
weeklyRankingRepository.findProductIdsByBaseDate,
monthlyRankingRepository.findProductIdsByBaseDate.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/RankingApiE2ETest.java`:
- Around line 191-197: The test hardcodes LocalDate monday = LocalDate.of(2026,
4, 13) which risks missing week-boundary regressions; change the setup to
compute the week's Monday dynamically by deriving a reference date (e.g., a
Wednesday or test-specific date) and calling
with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)) to produce the monday
used when creating MvProductRankWeekly via weeklyRankingJpaRepository.save(...)
and when asserting weekly queries; also add at least one additional case using a
boundary reference date (e.g., end-of-year like 2026-12-30) to ensure
ISO/week-boundary behavior is covered.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java`:
- Around line 96-104: monthlyAggregateAndRankStep currently constructs a new
DataSourceTransactionManager for each Step build (new
DataSourceTransactionManager(dataSource)); instead, define a singleton
PlatformTransactionManager/DataSourceTransactionManager bean and inject it into
this config, then use that injected transactionManager in the StepBuilder
(replace new DataSourceTransactionManager(dataSource) with the injected
transactionManager) so the same transaction manager instance backs
monthlyAggregateAndRankStep and preserves proper propagation/transaction
semantics for the shared dataSource.
- Around line 48-63: 쿼리의 ORDER BY가 score만 사용되어 동점시 결과가 비결정적이므로
MonthlyRankingJobConfig의 MONTHLY_METRICS_SQL을 수정해 동점 타이브레이커를 추가하거나 윈도우 함수를 사용하도록
변경하세요; 간단한 fix는 ORDER BY 절에 ", product_id ASC"를 추가해 동일 score일 때 product_id 순으로
결과가 고정되도록 하고, 대안으로 MySQL 8 이상에서는 DENSE_RANK()/RANK()를 사용해 명시적 랭크를 계산하는 방식을 도입할 수
있습니다; 변경 후에는 동일 score 다수 케이스에 대해 재실행 시 product_id 순으로 상위 100개와 애플리케이션의 rank 부여가
결정적인지 검증하는 테스트를 추가하세요.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java`:
- Around line 28-31: The setTargetYearMonth method in MonthlyRankingProcessor
should catch DateTimeParseException when parsing targetYearMonth and rethrow an
IllegalArgumentException that includes the original parameter value and the
expected pattern ("yyyyMM"), preserving the caught exception as the cause so
logs show both the bad input and the underlying parse error; update
MonthlyRankingJobE2ETest to add cases passing "", "2026-04", and "202613" and
assert the job fails immediately with an IllegalArgumentException whose message
contains the submitted value and the expected format.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java`:
- Around line 71-79: Add a JobParametersValidator at the job level in
weeklyRankingJob() so the job fails early on missing/invalid parameters; update
the JobBuilder for JOB_NAME to call .validator(new
DefaultJobParametersValidator(new String[]{"targetDate"}, new
String[]{"run.id"})) so missing targetDate causes the Job to fail before
truncateWeeklyMvStep() runs (leaving mv_product_rank_weekly intact), keeping the
existing .start(truncateWeeklyMvStep()).listener(jobListener) and
.next(weeklyAggregateAndRankStep()) flow unchanged.
- Around line 43-62: CHUNK_SIZE is set to 10 while the SQL limits results to
100, causing 10 separate commits and non-deterministic ranking for ties; either
increase CHUNK_SIZE (e.g., to 100 or 50) or explicitly document/design a
Skip/Retry policy for the chunked insert behavior, and make the SQL
deterministic by adding a tiebreaker to WEEKLY_METRICS_SQL (change ORDER BY
score DESC to ORDER BY score DESC, product_id ASC) so that
Processor/AtomicInteger rank assignment is stable across reruns; also add a
unit/integration test that runs the job twice on products with identical scores
and asserts (product_id, rank) equality.
- Around line 95-103: 현재 weeklyAggregateAndRankStep() 내부에서 매 호출마다 new
DataSourceTransactionManager(dataSource)로 트랜잭션 매니저를 생성하고 있는데, JobScope로 인해 Step이
재생성될 때마다 인스턴스가 불필요하게 만들어집니다; DataSourceTransactionManager를 `@Bean으로` 등록(예:
dataSourceTransactionManager)하고 WeeklyRankingJobConfig에서 생성자나 필드 주입으로 해당 빈을
참조하도록 변경한 뒤 weeklyAggregateAndRankStep()의 chunk(...) 호출에서 new 대신 주입된 트랜잭션 매니저(예:
dataSourceTransactionManager)를 사용하도록 수정하세요; 동일 패턴인 MonthlyRankingJobConfig의 구현
방식과 일관되게 적용해 전역 설정(전파 옵션 등) 유지·튜닝이 용이하도록 하세요.
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java`:
- Around line 8-36: The POJO MvProductRankMonthly in com.loopers.domain.ranking
collides with the commerce-api JPA entity of the same name and lacks
equals/hashCode/toString; rename or relocate this batch value object (e.g.,
BatchMvProductRankMonthly or package com.loopers.batch.domain.ranking) and
update all references to the constructor/factory method (MvProductRankMonthly.of
and the private constructor) to the new type, or convert it into a Java record
to get immutability and generated equals/hashCode/toString; additionally add an
integration test that verifies the batch INSERT JDBC column order/names used by
JdbcBatchItemWriter match the commerce-api `@Column` mapping to prevent silent
schema drift.
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`:
- Around line 8-37: Add precondition validation in MvProductRankWeekly.of
(and/or the private constructor) to enforce productId != null, rank >= 1, and
totalLike/totalOrder/totalView/totalSales >= 0 (throw IllegalArgumentException
with clear messages on violation), and implement value semantics by overriding
equals(Object), hashCode(), and toString() based on all fields (or at least
productId and baseDate plus rank/score as needed) so the value object has
predictable identity and readable logs/tests; update unit tests to cover null
productId, rank<=0, and negative counters.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`:
- Around line 41-46: The test currently only deletes mv_product_rank_weekly and
product_metrics in WeeklyRankingJobE2ETest.setUp(), which leaves shared DB state
(including Spring Batch meta tables like BATCH_JOB_INSTANCE) and can cause
cross-test flakiness; update the test class to enforce per-class isolation by
either (a) annotating the class/methods to run with a transactional rollback
(use `@Transactional` on the test and ensure the framework rolls back after each
test) or (b) run the test with Testcontainers per-class DB lifecycle, or (c) add
`@Sql`(executionPhase = AFTER_TEST_METHOD, scripts=...) to fully clean application
tables and Spring Batch meta tables, and ensure job runs supply unique
parameters by using JobLauncherTestUtils.getUniqueJobParameters() so
JobInstanceAlreadyCompleteException cannot occur.
- Around line 76-99: Add boundary tests to WeeklyRankingJobE2ETest to verify the
inclusive/exclusive aggregation window [targetDate, targetDate+7days) and
deletion filtering: create test cases (or extend fewProducts_allRankedInOrder)
that call insertMetrics(...) with metric_hour equal to targetDate 00:00:00
(should be included), targetDate.minusSeconds(1) (should be excluded),
targetDate.plusDays(7).minusSeconds(1) (included), and targetDate.plusDays(7)
(excluded), and also insert a row with deleted_at non-null to assert it is
ignored; after launching the job via jobLauncherTestUtils.launchJob(params) use
jdbcTemplate queries (mv_product_rank_weekly and any raw metrics table) and
assertions (countMv(), rank lookup, etc.) to assert each boundary behaves as
expected.
In `@http/commerce-api/ranking.http`:
- Around line 1-21: Declare a reusable date environment variable and use it in
the ranking requests: add a {{today}} (or generic {{date}}) variable to the HTTP
client environment file and update the GET calls to /api/v1/rankings to pass
?date={{today}} (replace hardcoded 20260416 instances with {{today}} or
{{date}}); also modify the first request comment/URL so the “오늘 날짜” example
actually includes the date query param (?date={{today}}) to keep documentation
and requests consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2e38cdb-0aac-4c5a-8e93-e09a8ab40bbb
📒 Files selected for processing (27)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MonthlyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/RankingPeriod.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/WeeklyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaMonthlyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MonthlyRankingJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaMonthlyRankingRepositoryTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepositoryTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/RankingApiE2ETest.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/TruncateMonthlyMvTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/TruncateWeeklyMvTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/WeeklyRankingProcessor.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.javahttp/commerce-api/ranking.http
Comment on lines
+37
to
+45
| public PageResponse<RankingInfo> getPage(LocalDate date, RankingPeriod period, int page, int size) { | ||
| long offset = (long) (page - 1) * size; | ||
| List<Long> productIds = rankingRepository.findProductIdsByRank(date, offset, (long) size); | ||
| List<Long> productIds = resolveProductIds(date, period, offset, (long) size); | ||
|
|
||
| if (productIds.isEmpty()) { | ||
| return new PageResponse<>(List.of(), page, size, 0); | ||
| } | ||
|
|
||
| long count = rankingRepository.countByDate(date); | ||
| long count = resolveCount(date, period); |
There was a problem hiding this comment.
period/date 인자의 null 방어를 명시해야 한다.
period가 null이면 switch (period)에서 NullPointerException이 발생하고 ApiControllerAdvice 경로가 아닌 일반 500으로 응답되어 오류 포맷이 어긋난다. Controller에서 기본값 DAILY를 보장한다고 하지만, 내부 호출자(스케줄러·테스트 등)를 고려해 퍼사드 진입부에서 Objects.requireNonNull 또는 CoreException으로 명시적으로 처리하는 편이 안전하다. date 역시 null 시 TemporalAdjusters에서 NPE가 발생한다.
추가 테스트: getPage(null, null, 1, 20) 호출 시 기대한 예외 타입이 던져지는지 단위 테스트를 보강한다.
🛡️ 수정안
`@Transactional`(readOnly = true)
public PageResponse<RankingInfo> getPage(LocalDate date, RankingPeriod period, int page, int size) {
+ Objects.requireNonNull(date, "date");
+ Objects.requireNonNull(period, "period");
long offset = (long) (page - 1) * size;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 37 - 45, In getPage(LocalDate date, RankingPeriod period, int page,
int size) add explicit null checks for period and date (e.g.,
Objects.requireNonNull or throw a CoreException with a clear error code/message)
before any use (the switch(period) and any TemporalAdjusters calls) so NPEs are
avoided and errors follow ApiControllerAdvice handling; update
resolveProductIds/resolveCount callers if needed to accept the validated values;
and add a unit test asserting getPage(null, null, 1, 20) throws the expected
exception type and message.
Comment on lines
+45
to
+70
| @Column(name = "created_at", nullable = false, updatable = false) | ||
| private ZonedDateTime createdAt; | ||
|
|
||
| private MvProductRankWeekly(Long productId, int rank, double score, | ||
| long totalLike, long totalOrder, long totalView, | ||
| long totalSales, LocalDate baseDate) { | ||
| this.productId = productId; | ||
| this.rank = rank; | ||
| this.score = score; | ||
| this.totalLike = totalLike; | ||
| this.totalOrder = totalOrder; | ||
| this.totalView = totalView; | ||
| this.totalSales = totalSales; | ||
| this.baseDate = baseDate; | ||
| } | ||
|
|
||
| public static MvProductRankWeekly of(Long productId, int rank, double score, | ||
| long totalLike, long totalOrder, long totalView, | ||
| long totalSales, LocalDate baseDate) { | ||
| return new MvProductRankWeekly(productId, rank, score, totalLike, totalOrder, totalView, totalSales, baseDate); | ||
| } | ||
|
|
||
| @PrePersist | ||
| private void prePersist() { | ||
| this.createdAt = ZonedDateTime.now(); | ||
| } |
There was a problem hiding this comment.
createdAt의 타임존을 명시적으로 고정하는 편이 운영 혼선을 줄인다.
ZonedDateTime.now()는 JVM의 기본 타임존에 의존한다. API/배치 서버의 타임존이 달라지거나 컨테이너 이미지 변경으로 UTC↔Asia/Seoul이 섞이면 동일 baseDate의 행에 대해 created_at만 시차가 들어간 값이 쓰여 장애 분석 로그에서 혼란을 준다. 가능한 ZonedDateTime.now(ZoneId.of("Asia/Seoul")) 또는 ZoneOffset.UTC로 고정하거나, 프로젝트에 BaseEntity/Auditing이 있다면 그쪽 규약을 따르는 것이 좋다.
또한 동일한 타임스탬프 규약이 MvProductRankMonthly에도 반복되므로, 공통 @MappedSuperclass로 뽑으면 규약을 한 곳에서 강제할 수 있다(아래 중복 관련 코멘트 참조).
추가 테스트: 엔티티 저장 후 created_at이 기대 타임존으로 저장되는지 검증하는 통합 테스트를 JpaWeeklyRankingRepositoryTest에 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`
around lines 45 - 70, The createdAt assignment in MvProductRankWeekly currently
uses ZonedDateTime.now() (see createdAt field and prePersist()), which relies on
the JVM default TZ; change prePersist() to set createdAt with an explicit zone
(e.g., ZonedDateTime.now(ZoneId.of("Asia/Seoul")) or
ZonedDateTime.now(ZoneOffset.UTC)) or refactor both MvProductRankWeekly and
MvProductRankMonthly to inherit a single `@MappedSuperclass/BaseEntity` or project
Auditing convention that centralizes createdAt handling; after changing, add an
integration test in JpaWeeklyRankingRepositoryTest to assert that the persisted
created_at uses the chosen timezone/offset.
Comment on lines
+18
to
+22
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | ||
| int pageNumber = (int) (offset / limit); | ||
| PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit); | ||
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd JpaWeeklyRankingRepository.java | xargs -I{} sed -n '1,80p' {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1007
🏁 Script executed:
rg "offset\s*/\s*limit|PageRequest\.of" --type java -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 14089
🏁 Script executed:
fd "Repository" --extension java | xargs grep -l "offset.*limit\|limit.*offset" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 943
🏁 Script executed:
cat apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaMonthlyRankingRepositoryTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4146
🏁 Script executed:
cat apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepositoryTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4127
limit=0 입력 시 ArithmeticException 발생, 비배수 offset 계산 오류 위험
pageNumber = (int) (offset / limit)은 limit == 0일 때 0으로 나누기 예외를 발생시켜 500 에러가 된다. 또한 파사드에서 항상 배수 형태로 입력된다는 가정 하에 작동하지만, 인프라 계층만 보면 이는 전제일 뿐이므로 경계 방어가 필요하다. JpaWeeklyRankingRepository도 동일한 패턴을 가지고 있다.
수정: limit <= 0 검증 및 offset % limit != 0 확인, Math.toIntExact() 사용으로 오버플로우 방지. 현재 테스트는 offset=2, limit=2 같은 안전한 경우만 검증하므로, limit=0 및 비배수 offset 입력 시 동작을 명세하는 단위 테스트를 추가해야 한다.
♻️ 제안 수정
- `@Override`
- public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) {
- int pageNumber = (int) (offset / limit);
- PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit);
- return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest);
- }
+ `@Override`
+ public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) {
+ if (limit <= 0) {
+ return List.of();
+ }
+ if (offset % limit != 0) {
+ throw new IllegalArgumentException(
+ "offset(" + offset + ") must be a multiple of limit(" + limit + ")");
+ }
+ int pageNumber = Math.toIntExact(offset / limit);
+ PageRequest pageRequest = PageRequest.of(pageNumber, Math.toIntExact(limit));
+ return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest);
+ }📝 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.
Suggested change
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | |
| int pageNumber = (int) (offset / limit); | |
| PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit); | |
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | |
| } | |
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | |
| if (limit <= 0) { | |
| return List.of(); | |
| } | |
| if (offset % limit != 0) { | |
| throw new IllegalArgumentException( | |
| "offset(" + offset + ") must be a multiple of limit(" + limit + ")"); | |
| } | |
| int pageNumber = Math.toIntExact(offset / limit); | |
| PageRequest pageRequest = PageRequest.of(pageNumber, Math.toIntExact(limit)); | |
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaMonthlyRankingRepository.java`
around lines 18 - 22, In findProductIdsByBaseDate (and mirror the same fix in
JpaWeeklyRankingRepository) add defensive validation: if limit <= 0 throw an
IllegalArgumentException (or similar), and if offset % limit != 0 throw an
IllegalArgumentException to explicitly reject non-multiple offsets; compute
pageNumber using Math.toIntExact(offset / limit) and build
PageRequest.of(pageNumber, Math.toIntExact(limit)) to avoid silent
truncation/overflow; finally add unit tests that assert behavior for limit=0 and
for non-multiple offset values (e.g., offset=3, limit=2) to codify the expected
exceptions.
Comment on lines
+18
to
+22
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | ||
| int pageNumber = (int) (offset / limit); | ||
| PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit); | ||
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | ||
| } |
There was a problem hiding this comment.
offset/limit → PageRequest 변환이 정렬되지 않은 offset에서 부정확하다.
pageNumber = (int) (offset / limit)은 offset이 limit의 배수일 때만 정확하다. 현재 호출처(RankingFacade.getPage)는 offset=(page-1)*size로 항상 정렬되어 있어 우연히 동작하나, 계약상 offset은 임의 값을 허용한다. 향후 커서/스킵 방식 호출이 생기면 페이지 경계를 잘라먹어 랭킹이 누락되는 장애로 이어질 수 있다. 또한 limit <= 0에 대한 방어가 없어 ArithmeticException(division by zero) 또는 IllegalArgumentException(PageRequest.of는 size>=1을 요구)로 500이 발생한다. 운영 관점에서 사용자가 size=0을 주면 500을 뱉게 되므로 입력 검증이 필요하다.
🛠 제안 수정안
`@Override`
public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) {
- int pageNumber = (int) (offset / limit);
- PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit);
- return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest);
+ if (limit <= 0) {
+ return List.of();
+ }
+ if (offset % limit != 0) {
+ throw new IllegalArgumentException("offset must be a multiple of limit: offset=" + offset + ", limit=" + limit);
+ }
+ int pageNumber = Math.toIntExact(offset / limit);
+ PageRequest pageRequest = PageRequest.of(pageNumber, Math.toIntExact(limit));
+ return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest);
}추가 테스트: offset=3, limit=2(비정렬) 케이스, limit=0 케이스, offset=Integer.MAX_VALUE+1L(오버플로우) 케이스.
참고로 MonthlyRankingRepository 구현체도 동일한 변환 로직을 가질 가능성이 높으니 일관 수정이 필요하다.
📝 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.
Suggested change
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | |
| int pageNumber = (int) (offset / limit); | |
| PageRequest pageRequest = PageRequest.of(pageNumber, (int) limit); | |
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | |
| } | |
| public List<Long> findProductIdsByBaseDate(LocalDate baseDate, long offset, long limit) { | |
| if (limit <= 0) { | |
| return List.of(); | |
| } | |
| if (offset % limit != 0) { | |
| throw new IllegalArgumentException("offset must be a multiple of limit: offset=" + offset + ", limit=" + limit); | |
| } | |
| int pageNumber = Math.toIntExact(offset / limit); | |
| PageRequest pageRequest = PageRequest.of(pageNumber, Math.toIntExact(limit)); | |
| return jpaRepository.findProductIdsByBaseDate(baseDate, pageRequest); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepository.java`
around lines 18 - 22, The conversion from offset/limit in
JpaWeeklyRankingRepository.findProductIdsByBaseDate is unsafe: validate that
limit > 0 (throw IllegalArgumentException or return empty) to avoid division by
zero/invalid PageRequest, compute the page number using floor division to handle
non-multiple offsets (use Math.floorDiv(offset, limit)) and clamp the result to
an int (e.g. (int)Math.min(Integer.MAX_VALUE, Math.floorDiv(offset, limit)))
before calling PageRequest.of, and apply the same defensive change to the
corresponding MonthlyRankingRepository implementation; add unit tests for
offset=3,limit=2, limit=0 and extreme overflow offsets.
Comment on lines
+38
to
+119
| @DisplayName("findProductIdsByBaseDate() 를 호출할 때,") | ||
| @Nested | ||
| class FindProductIdsByBaseDate { | ||
|
|
||
| @DisplayName("rank 오름차순으로 productId 목록을 반환한다.") | ||
| @Test | ||
| void returnsInRankOrder() { | ||
| // arrange | ||
| LocalDate baseDate = LocalDate.of(2026, 4, 13); | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of(100L, 1, 300.0, 10, 5, 100, 5000, baseDate)); | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of(200L, 2, 200.0, 8, 3, 80, 3000, baseDate)); | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of(300L, 3, 100.0, 5, 1, 50, 1000, baseDate)); | ||
|
|
||
| // act | ||
| List<Long> result = weeklyRankingRepository.findProductIdsByBaseDate(baseDate, 0, 3); | ||
|
|
||
| // assert | ||
| assertThat(result).containsExactly(100L, 200L, 300L); | ||
| } | ||
|
|
||
| @DisplayName("offset 과 limit 에 따라 해당 페이지만 반환한다.") | ||
| @Test | ||
| void withOffsetAndLimit() { | ||
| // arrange | ||
| LocalDate baseDate = LocalDate.of(2026, 4, 13); | ||
| for (int rank = 1; rank <= 5; rank++) { | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of((long) rank * 10, rank, 100.0 - rank, 1, 1, 1, 1000, baseDate)); | ||
| } | ||
|
|
||
| // act | ||
| List<Long> result = weeklyRankingRepository.findProductIdsByBaseDate(baseDate, 2, 2); | ||
|
|
||
| // assert | ||
| assertThat(result).containsExactly(30L, 40L); | ||
| } | ||
|
|
||
| @DisplayName("해당 baseDate 데이터가 없으면 빈 목록을 반환한다.") | ||
| @Test | ||
| void returnsEmpty_whenNoData() { | ||
| // arrange | ||
| LocalDate baseDate = LocalDate.of(2026, 4, 13); | ||
|
|
||
| // act | ||
| List<Long> result = weeklyRankingRepository.findProductIdsByBaseDate(baseDate, 0, 10); | ||
|
|
||
| // assert | ||
| assertThat(result).isEmpty(); | ||
| } | ||
| } | ||
|
|
||
| @DisplayName("countByBaseDate() 를 호출할 때,") | ||
| @Nested | ||
| class CountByBaseDate { | ||
|
|
||
| @DisplayName("해당 baseDate 의 전체 row 수를 반환한다.") | ||
| @Test | ||
| void returnsTotalCount() { | ||
| // arrange | ||
| LocalDate baseDate = LocalDate.of(2026, 4, 13); | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of(1L, 1, 100.0, 1, 1, 1, 1000, baseDate)); | ||
| weeklyRankingJpaRepository.save(MvProductRankWeekly.of(2L, 2, 90.0, 1, 1, 1, 900, baseDate)); | ||
|
|
||
| // act | ||
| long count = weeklyRankingRepository.countByBaseDate(baseDate); | ||
|
|
||
| // assert | ||
| assertThat(count).isEqualTo(2); | ||
| } | ||
|
|
||
| @DisplayName("해당 baseDate 데이터가 없으면 0을 반환한다.") | ||
| @Test | ||
| void returnsZero_whenNoData() { | ||
| // arrange | ||
| LocalDate baseDate = LocalDate.of(2026, 4, 13); | ||
|
|
||
| // act | ||
| long count = weeklyRankingRepository.countByBaseDate(baseDate); | ||
|
|
||
| // assert | ||
| assertThat(count).isZero(); | ||
| } | ||
| } |
There was a problem hiding this comment.
다른 baseDate 데이터가 섞여 있을 때 필터링되는지에 대한 테스트가 필요하다.
현재는 모든 케이스가 단일 baseDate만 저장하고 조회한다. 그러나 배치가 주차별 데이터를 누적 적재하는 구조상 한 테이블에 여러 baseDate가 공존하는 상태가 실제 운영 상태다. 쿼리에 WHERE baseDate = :baseDate가 빠지거나 <=로 변경되는 회귀가 들어와도 현재 테스트로는 포착되지 않는다.
추가 테스트:
`@Test`
void doesNotReturnOtherBaseDateRows() {
LocalDate target = LocalDate.of(2026, 4, 13);
LocalDate other = LocalDate.of(2026, 4, 6);
weeklyRankingJpaRepository.save(MvProductRankWeekly.of(1L, 1, 100.0, 1, 1, 1, 100, target));
weeklyRankingJpaRepository.save(MvProductRankWeekly.of(2L, 1, 200.0, 1, 1, 1, 100, other));
assertThat(weeklyRankingRepository.findProductIdsByBaseDate(target, 0, 10)).containsExactly(1L);
assertThat(weeklyRankingRepository.countByBaseDate(target)).isEqualTo(1);
}[As per coding guidelines] "쿼리 조건 누락/과다 조회 ... 을 점검한다." — 조건 누락 회귀를 잡기 위한 필터 격리 테스트가 필요하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/JpaWeeklyRankingRepositoryTest.java`
around lines 38 - 119, Add a test that verifies filtering by baseDate by
inserting rows with two different LocalDate values and asserting only rows
matching the target date are returned; specifically, in the test class use
weeklyRankingJpaRepository.save(MvProductRankWeekly.of(..., target)) and save
another MvProductRankWeekly with a different date (other), then call
weeklyRankingRepository.findProductIdsByBaseDate(target, 0, 10) and assert it
contains only the IDs for the target date and call
weeklyRankingRepository.countByBaseDate(target) and assert it equals 1 to ensure
the repository methods (findProductIdsByBaseDate and countByBaseDate) properly
filter by baseDate.
Comment on lines
+3
to
+10
| public record ProductAggregation( | ||
| Long productId, | ||
| Double score, | ||
| Long totalLike, | ||
| Long totalOrder, | ||
| Long totalView, | ||
| Long totalSales | ||
| ) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\bProductAggregation\s*\(' --type=java
rg -nP -C3 'rs\.getLong|rs\.getDouble|getObject\(.*Long|getObject\(.*Double' --type=java -g '**/ranking/**'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6348
🏁 Script executed:
rg -nP 'class.*Processor|ItemProcessor' --type=java -g '**/ranking/**' -A 5
rg -nP 'MvProductRank' --type=java -g '**'
rg -nP 'item\.score\(\)|item\.totalLike\(\)' --type=javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 15707
🏁 Script executed:
cat -n apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.java
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/WeeklyRankingProcessor.java
rg -nP 'SELECT.*SUM|SELECT.*COUNT' --type=java -g '**/ranking/**' -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5489
🏁 Script executed:
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java | head -140
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java | head -140Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 14924
NULL 처리 방식을 명시적으로 설계할 것을 권고한다.
현재 RowMapper에서 rs.getLong()·rs.getDouble()을 직접 호출할 때, JDBC 표준에 따라 NULL 컬럼은 0 또는 0.0으로 자동 치환된다. 이는 ProductAggregation 레코드의 래퍼 타입 설계 의도와 불일치한다. DB 집계 결과에서 NULL(데이터 부재)과 0(실제 값)을 구분하지 못하면, 운영 중 랭킹 산출 과정을 추적하기 어렵고 집계 정확성을 보장할 수 없다.
RowMapper에서 rs.wasNull() 또는 getObject() + null 체크를 통해 NULL 여부를 명시적으로 확인하거나, SQL에서 COALESCE(SUM(...), 0) 형태로 NULL을 사전에 처리하도록 개선할 것을 권고한다. 특히 score 계산식이 NULL을 포함하는 경우 전체 결과가 NULL이 될 수 있으므로, 집계 대상이 없는 경우의 처리 방식을 명확히 정의하기를 제안한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.java`
around lines 3 - 10, ProductAggregation fields are wrapper types but the
RowMapper currently uses rs.getLong()/rs.getDouble(), which converts SQL NULL to
0, so change the mapping to detect NULLs explicitly (use
rs.getObject(columnName, Long.class) / rs.getObject(columnName, Double.class) or
call rs.getLong()/rs.getDouble() and then check rs.wasNull()) and populate
ProductAggregation with null when appropriate; alternatively, update the SQL
aggregation to use COALESCE(SUM(...), 0) for each numeric column (including the
score expression) if you intend to treat missing aggregates as 0 — choose one
approach and apply consistently in the RowMapper that constructs new
ProductAggregation(...) so NULL vs 0 semantics are preserved.
Comment on lines
+40
to
+119
| @BeforeEach | ||
| void setUp() { | ||
| jobLauncherTestUtils.setJob(job); | ||
| jdbcTemplate.execute("DELETE FROM mv_product_rank_monthly"); | ||
| jdbcTemplate.execute("DELETE FROM product_metrics"); | ||
| } | ||
|
|
||
| @DisplayName("targetYearMonth 파라미터 없으면 Job 이 실패한다.") | ||
| @Test | ||
| void failsWithoutTargetDate() throws Exception { | ||
| // arrange & act | ||
| JobExecution execution = jobLauncherTestUtils.launchJob(); | ||
|
|
||
| // assert | ||
| assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.FAILED.getExitCode()); | ||
| } | ||
|
|
||
| @DisplayName("집계 대상 메트릭이 없으면 MV 테이블은 비어 있다.") | ||
| @Test | ||
| void emptyMetrics_noRankingStored() throws Exception { | ||
| // arrange | ||
| JobParameters params = new JobParametersBuilder() | ||
| .addString("targetYearMonth", "202604") | ||
| .toJobParameters(); | ||
|
|
||
| // act | ||
| JobExecution execution = jobLauncherTestUtils.launchJob(params); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | ||
| () -> assertThat(countMv()).isZero() | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("상품이 100개 이하이면 모두 랭킹에 기록되고, rank 1 이 가장 높은 score 를 가진다.") | ||
| @Test | ||
| void fewProducts_allRankedInOrder() throws Exception { | ||
| // arrange | ||
| insertMetrics(1L, LocalDateTime.of(2026, 4, 16, 0, 0), 1, 1, 1, 1_000); | ||
| insertMetrics(2L, LocalDateTime.of(2026, 4, 16, 0, 0), 5, 5, 5, 50_000); | ||
|
|
||
| JobParameters params = new JobParametersBuilder() | ||
| .addString("targetYearMonth", "202604") | ||
| .toJobParameters(); | ||
|
|
||
| // act | ||
| JobExecution execution = jobLauncherTestUtils.launchJob(params); | ||
|
|
||
| // assert | ||
| Long rank1ProductId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_monthly WHERE rank = 1", Long.class); | ||
| assertAll( | ||
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | ||
| () -> assertThat(countMv()).isEqualTo(2), | ||
| () -> assertThat(rank1ProductId).isEqualTo(2L) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("상품이 100개 초과이면 상위 100개만 기록된다.") | ||
| @Test | ||
| void manyProducts_top100Only() throws Exception { | ||
| // arrange | ||
| for (long i = 1; i <= 110; i++) { | ||
| insertMetrics(i, LocalDateTime.of(2026, 4, 16, 0, 0), 1, 1, 1, i * 1_000); | ||
| } | ||
|
|
||
| JobParameters params = new JobParametersBuilder() | ||
| .addString("targetYearMonth", "202604") | ||
| .toJobParameters(); | ||
|
|
||
| // act | ||
| JobExecution execution = jobLauncherTestUtils.launchJob(params); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | ||
| () -> assertThat(countMv()).isEqualTo(100) | ||
| ); | ||
| } |
There was a problem hiding this comment.
멱등성·기간 경계·삭제 필터 케이스가 누락되어 있다.
현재 Job의 운영 계약 중 아래 항목이 검증되지 않아 회귀가 발생해도 포착이 어렵다.
- 멱등성: 동일
targetYearMonth로 두 번 실행해도mv_product_rank_monthly행 수가 2배가 되지 않는지(=TruncateMonthlyMvTasklet가 기대대로 해당base_date만 삭제하는지). - 기간 경계:
targetYearMonth=202604일 때metric_hour가2026-04-01 00:00:00은 포함되고2026-05-01 00:00:00은 제외되는지(쿼리의>= firstDay AND < nextMonth계약 검증). - 이전 월 격리: 3월/5월
product_metrics를 섞어 넣었을 때 4월 실행 결과에는 해당 productId가 등장하지 않는지. - soft-delete 필터:
deleted_at IS NOT NULL인product_metrics행은 집계에서 제외되는지.
운영에서 가장 흔한 사고 경로(재실행 중복, 경계 한 시간 오차, 월 섞임, soft-delete 누락)를 커버해야 한다.
필요하면 위 네 케이스에 대한 테스트 스켈레톤을 만들어 낼 수 있다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.java`
around lines 40 - 119, Add four E2E tests in MonthlyRankingJobE2ETest to cover
idempotency, boundary, month-isolation, and soft-delete: 1) idempotency: run job
twice with the same JobParameters ("targetYearMonth" "202604") using
jobLauncherTestUtils.launchJob(...) and assert countMv() does not double (ensure
TruncateMonthlyMvTasklet deletes only base_date rows). 2) boundary:
insertMetrics at 2026-04-01T00:00 and at 2026-05-01T00:00, run the job and
assert only the 2026-04-01 row appears (verifies >= firstDay AND < nextMonth
behavior). 3) month isolation: insert product_metrics rows for 2026-03-xx and
2026-05-xx plus 2026-04-xx, run for 202604 and assert only April productIds are
in mv (use insertMetrics and countMv/queries). 4) soft-delete: insertMetrics
with deleted_at non-null for some product(s), run job and assert those products
are not aggregated into mv. Use existing helpers (insertMetrics, countMv,
jdbcTemplate queries) and JobParametersBuilder to build the targetYearMonth
param.
Comment on lines
+90
to
+96
| Long rank1ProductId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_monthly WHERE rank = 1", Long.class); | ||
| assertAll( | ||
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | ||
| () -> assertThat(countMv()).isEqualTo(2), | ||
| () -> assertThat(rank1ProductId).isEqualTo(2L) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is RANK a reserved word in MySQL 8.0 and does unquoted rank work as a column name in WHERE clauses?
💡 Result:
Yes, RANK is a reserved word in MySQL 8.0, added as reserved in version 8.0.2. No, unquoted rank does not work as a column name in WHERE clauses; it must be quoted with backticks (e.g., rank) because reserved words cannot be used as unquoted identifiers.
Citations:
- 1: https://dev.mysql.com/doc/mysqld-version-reference/en/keywords-8-0.html
- 2: https://dev.mysql.com/doc/refman/8.0/en/keywords.html
- 3: "rank" becomes a mysql reserved keyword in version 8 Piwigo/Piwigo#919
- 4: https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/keywords.html
- 5: https://stackoverflow.com/questions/77930527/migrating-to-mysql-8-calculating-the-rank-error
테스트 쿼리에서 MySQL 예약어 rank를 백틱으로 이스케이프해야 한다.
MySQL 8.0.2부터 RANK는 공식 예약어로 등록되었다. 엔티티 매핑에서는 `@Column`(name = "`rank`")로 백틱을 적용했으나, 이 테스트의 네이티브 쿼리에서는 WHERE rank = 1로 이스케이프 없이 사용 중이다. 운영 환경의 MySQL 버전과 sql_mode 설정에 따라 파싱 오류나 쿼리 실행 실패를 초래할 수 있으므로, 엔티티와 동일하게 백틱으로 감싸 일관성을 유지해야 한다.
수정안
- "SELECT product_id FROM mv_product_rank_monthly WHERE rank = 1", Long.class);
+ "SELECT product_id FROM mv_product_rank_monthly WHERE `rank` = 1", Long.class);📝 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.
Suggested change
| Long rank1ProductId = jdbcTemplate.queryForObject( | |
| "SELECT product_id FROM mv_product_rank_monthly WHERE rank = 1", Long.class); | |
| assertAll( | |
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | |
| () -> assertThat(countMv()).isEqualTo(2), | |
| () -> assertThat(rank1ProductId).isEqualTo(2L) | |
| ); | |
| Long rank1ProductId = jdbcTemplate.queryForObject( | |
| "SELECT product_id FROM mv_product_rank_monthly WHERE `rank` = 1", Long.class); | |
| assertAll( | |
| () -> assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()), | |
| () -> assertThat(countMv()).isEqualTo(2), | |
| () -> assertThat(rank1ProductId).isEqualTo(2L) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.java`
around lines 90 - 96, The test's native SQL uses an unescaped MySQL reserved
word "rank" causing potential parse failures; update the
jdbcTemplate.queryForObject call in MonthlyRankingJobE2ETest (the code that
assigns rank1ProductId via jdbcTemplate.queryForObject on
mv_product_rank_monthly) to wrap the column name in backticks (i.e. use
`\`rank\``) so the WHERE clause matches the entity mapping; run the test and
ensure countMv() and assertions remain unchanged.
Comment on lines
+48
to
+56
| @DisplayName("targetDate 파라미터 없으면 Job 이 실패한다.") | ||
| @Test | ||
| void failsWithoutTargetDate() throws Exception { | ||
| // arrange & act | ||
| JobExecution execution = jobLauncherTestUtils.launchJob(); | ||
|
|
||
| // assert | ||
| assertThat(execution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.FAILED.getExitCode()); | ||
| } |
There was a problem hiding this comment.
failsWithoutTargetDate 의 실패 경로가 불명확하다.
현재 구현상 targetDate 가 없으면 1) TruncateWeeklyMvTasklet 은 base_date = NULL 로 DELETE(0건 영향, 정상 종료) → 2) weeklyMetricsItemReader 가 null 에서 getDayOfWeek() 호출 시 NullPointerException 으로 실패한다. 즉 테스트가 "FAILED" 인 것만 검증하면 NPE 로 인한 실패인지 검증 로직에 의한 실패인지 구분할 수 없고, 향후 리더를 변경하면 테스트가 조용히 무의미해진다. 아래 중 하나로 강화할 것을 권고한다.
JobParametersValidator로 필수 파라미터 검증을 선행하도록 변경 후 테스트에서JobParametersInvalidException또는JobExecution#getAllFailureExceptions()의 예외 타입/메시지를 함께 단언.- 최소한 현재 구현에서라도
execution.getAllFailureExceptions()의 예외 원인이IllegalArgumentException인지 검증하도록 테스트를 보강(단, 1안을 선행 권고).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`
around lines 48 - 56, The test failsWithoutTargetDate is brittle because absence
of targetDate currently triggers a NullPointerException from
weeklyMetricsItemReader instead of a clear parameter validation failure; either
add a JobParametersValidator to the job so missing targetDate triggers a
JobParametersInvalidException (then assert that JobExecution fails with
JobParametersInvalidException or check JobExecution#getAllFailureExceptions()
contains that type/message), or minimally strengthen the test to assert that
execution.getAllFailureExceptions() contains an IllegalArgumentException caused
by the missing targetDate (referencing the TruncateWeeklyMvTasklet and
weeklyMetricsItemReader failure path if you choose the minimal change).
Comment on lines
+92
to
+93
| Long rank1ProductId = jdbcTemplate.queryForObject( | ||
| "SELECT product_id FROM mv_product_rank_weekly WHERE rank = 1", Long.class); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is the word "RANK" a reserved keyword in MySQL 8.0, and does using it as an unquoted column name cause a syntax error?
💡 Result:
Yes, "RANK" is a reserved keyword in MySQL 8.0, added as reserved in version 8.0.2. Using it as an unquoted column name causes a syntax error (error 1064), as shown in official documentation and multiple examples where CREATE TABLE or SELECT statements fail without backticks but succeed when quoted with backticks (RANK). Reserved keywords require quoting (backticks) to be used as identifiers like column names.
Citations:
- 1: https://dev.mysql.com/doc/mysqld-version-reference/en/keywords-8-0.html
- 2: https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/keywords.html
- 3: https://www.tutorialspoint.com/can-we-use-rank-as-column-name-with-mysql8
- 4: https://stackoverflow.com/questions/53570132/table-cannot-be-created-in-mysql-error-1064
🏁 Script executed:
# Find WeeklyRankingJobConfig file
find . -type f -name "*WeeklyRankingJobConfig*" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 185
🏁 Script executed:
# Search for INSERT statements with rank column in the WeeklyRankingJobConfig
rg -A 5 -B 5 "mv_product_rank_weekly" --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 11094
🏁 Script executed:
# Look for Reader implementation and boundary validation
rg -A 10 "IllegalArgumentException" apps/commerce-batch/src/main/java/com/loopers/job/ranking/ --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 183
🏁 Script executed:
# Look for table definition or schema file
find . -type f \( -name "*.sql" -o -name "*.ddl" \) | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Search for Reader class with boundary validation
rg -B 5 -A 15 "ItemReader" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/ --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 10452
🏁 Script executed:
# Search for targetDate validation in WeeklyRankingJobConfig
rg -B 3 -A 3 "targetDate\|validateDate\|DayOfWeek\|MONDAY" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/ --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
SELECT 쿼리의 rank 컬럼을 백틱으로 감싸거나 컬럼명을 리네이밍해야 한다.
rank는 MySQL 8.0.2부터 예약어로 등재되어 있으므로 식별자로 사용하려면 반드시 백틱으로 감싸야 한다. 현재 테스트의 SELECT 쿼리와 소스 코드의 INSERT 문(WeeklyRankingJobConfig)이 모두 백틱 없이 rank를 사용하고 있어 MySQL 8.0 이상에서 ERROR 1064가 발생하여 테스트가 실패한다.
운영 관점에서 백틱 처리는 임시방편이며, MySQL 마이너 버전 업그레이드 시마다 동일 장애가 재발할 수 있다. rank_no나 ranking 등으로 컬럼명을 리네이밍하는 것이 근본적인 해결책이다.
추가로 Reader의 월요일 경계 검증이 동작하는지 확인하는 테스트를 작성한다.
월요일이 아닌 targetDate(예: 2026-04-14)를 전달하면 Job이 FAILED로 종료되고 IllegalArgumentException이 발생하는지 검증해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`
around lines 92 - 93, The SELECT uses the reserved identifier `rank` causing
syntax errors; update the schema and all SQL to use a non-reserved column name
(e.g., rename `rank` to `rank_no` or `ranking`) and update references in the
test query (WeeklyRankingJobE2ETest), the INSERTs/DDL in WeeklyRankingJobConfig,
and any other SQL/DTOs that reference `rank`; alternatively, if you must keep
the name temporarily, escape the identifier with backticks everywhere (e.g.,
`rank`) but prefer renaming for a permanent fix. Also add a new E2E test that
calls the reader/job with a non-Monday targetDate (e.g., 2026-04-14) and asserts
the job ends FAILED and throws IllegalArgumentException to validate the Monday
boundary check in the reader.
chunk retry 시 AtomicInteger 누적으로 rank가 어긋나는 버그를 수정. ROW_NUMBER() OVER 서브쿼리로 DB가 rank를 결정하도록 변경하고, MySQL 예약어 충돌 방지를 위해 rank 컬럼에 백틱 적용. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java (1)
1-153:WeeklyRankingJobConfig와 구조·SQL 이 거의 동일하여 중복이 크다.집계 SQL 본문(
WEEKLY_METRICS_SQL/MONTHLY_METRICS_SQL), INSERT 템플릿, Writer 빌더, Step/Job 배선 형태가 기간 경계 산출만 다르고 거의 동일하다. 향후 스코어 공식, 컬럼 추가, 예약어 이슈(rank) 같은 변경이 생길 때 두 파일 모두 수정해야 하므로 누락 시 결함이 발생할 위험이 있다.운영/유지보수 관점에서 공통 부분을 추출하는 것을 권고한다: 예컨대 집계 SQL 의 SELECT/FROM 본문을 상수로 공유하고, 기간에 따른
[start, end)경계만 전략(enum/Period 객체)으로 주입하는 형태. Writer 역시 테이블명만 파라미터화한 헬퍼로 묶을 수 있다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java` around lines 1 - 153, MonthlyRankingJobConfig duplicates structure and SQL with WeeklyRankingJobConfig; extract shared parts to remove duplication: move the common SQL body (MONTHLY_METRICS_SQL/WEEKLY_METRICS_SQL SELECT/FROM/score expression) and the INSERT template into a shared helper or constants class, create a PeriodStrategy (enum or class) to compute start/end DateTimes used by monthlyMetricsItemReader (reference monthlyMetricsItemReader and its use of targetYearMonth) and its weekly counterpart, and add a reusable writer factory method that builds JdbcBatchItemWriter given a table/bean mapping used by monthlyRankingItemWriter; update MonthlyRankingJobConfig to call the shared SQL constant, use the PeriodStrategy for boundary calculation, and obtain its writer via the writer factory so only period-specific wiring remains in MonthlyRankingJobConfig (and similarly refactor WeeklyRankingJobConfig).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java`:
- Around line 87-95: The truncate step uses ResourcelessTransactionManager so
the jdbcTemplate.delete in truncateMonthlyMvTasklet is not bound to a DB
transaction; replace the step's transaction manager by wiring a
DataSourceTransactionManager (or otherwise ensure truncate and aggregate run in
the same DB transaction/single step) so DELETE is atomic with the Step outcome;
update truncateMonthlyMvStep to use DataSourceTransactionManager instead of
ResourcelessTransactionManager (and apply the same fix to
WeeklyRankingJobConfig), or alternatively refactor to a single Step that
performs truncate+aggregate/atomic swap to avoid leaving MV empty on partial
failure.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java`:
- Around line 76-84: Register and attach a JobParametersValidator to the
weeklyRankingJob (JOB_NAME) so the job-level validation fails fast when
targetDate is missing or malformed before truncateWeeklyMvStep() or
weeklyAggregateAndRankStep() run; add a validator bean (or configure
DefaultJobParametersValidator/custom validator) that requires "targetDate" and
enforces the expected format, then call .validator(yourValidator) on the
JobBuilder in weeklyRankingJob() to prevent step execution when parameters are
invalid and enable the described E2E test that ensures job fails immediately and
truncateWeeklyMvStep has no execution record.
---
Nitpick comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java`:
- Around line 1-153: MonthlyRankingJobConfig duplicates structure and SQL with
WeeklyRankingJobConfig; extract shared parts to remove duplication: move the
common SQL body (MONTHLY_METRICS_SQL/WEEKLY_METRICS_SQL SELECT/FROM/score
expression) and the INSERT template into a shared helper or constants class,
create a PeriodStrategy (enum or class) to compute start/end DateTimes used by
monthlyMetricsItemReader (reference monthlyMetricsItemReader and its use of
targetYearMonth) and its weekly counterpart, and add a reusable writer factory
method that builds JdbcBatchItemWriter given a table/bean mapping used by
monthlyRankingItemWriter; update MonthlyRankingJobConfig to call the shared SQL
constant, use the PeriodStrategy for boundary calculation, and obtain its writer
via the writer factory so only period-specific wiring remains in
MonthlyRankingJobConfig (and similarly refactor WeeklyRankingJobConfig).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bed00a4-1b68-42c1-a66f-a2014161bdf2
📒 Files selected for processing (5)
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/WeeklyRankingProcessor.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.java
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductAggregation.java
- apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/step/WeeklyRankingProcessor.java
- apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/step/MonthlyRankingProcessor.java
Comment on lines
+87
to
+95
| // Step 1: 이전 집계 결과 전체 삭제 (재실행 시 중복 방지) | ||
| @JobScope | ||
| @Bean(STEP_TRUNCATE) | ||
| public Step truncateMonthlyMvStep() { | ||
| return new StepBuilder(STEP_TRUNCATE, jobRepository) | ||
| .tasklet(truncateMonthlyMvTasklet, new ResourcelessTransactionManager()) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
ResourcelessTransactionManager + DELETE 조합으로 실패 복구성이 떨어진다.
truncateMonthlyMvTasklet 은 jdbcTemplate.update("DELETE ...") 로 실제 DB 에 쓰기를 수행하지만, Step 트랜잭션 매니저가 ResourcelessTransactionManager 라 Spring Batch 프레임워크 관점의 트랜잭션이 실제 DB 트랜잭션과 연동되지 않는다. 결과적으로 Tasklet 내부 예외가 발생해도 Step 이 rollback 처리된 것으로 기록되지만 DELETE 는 auto-commit 으로 이미 반영되어 MV 가 텅 빈 상태로 남을 수 있다. 이후 monthlyAggregateAndRankStep 이 실패하면 재시도 전까지 해당 baseDate 의 랭킹 조회가 0건이 되어 사용자 영향이 발생한다.
운영 관점 대안은 (1) Tasklet 트랜잭션 매니저를 DataSourceTransactionManager 로 교체해 DELETE 와 Step 트랜잭션을 일치시키거나, (2) truncate+aggregate 를 단일 Step 또는 단일 트랜잭션 범위에서 수행하거나, (3) 스왑 테이블/INSERT ... ON DUPLICATE KEY UPDATE 로 원자적 교체하는 것이다. 동일 이슈가 WeeklyRankingJobConfig 에도 존재한다.
추가 테스트 제안: truncate 성공 후 aggregate 에서 의도적 예외 주입 시 MV 가 비어있는 채로 남지 않는지(또는 이전 데이터가 보존되는지) 검증하는 E2E 케이스.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/monthly/MonthlyRankingJobConfig.java`
around lines 87 - 95, The truncate step uses ResourcelessTransactionManager so
the jdbcTemplate.delete in truncateMonthlyMvTasklet is not bound to a DB
transaction; replace the step's transaction manager by wiring a
DataSourceTransactionManager (or otherwise ensure truncate and aggregate run in
the same DB transaction/single step) so DELETE is atomic with the Step outcome;
update truncateMonthlyMvStep to use DataSourceTransactionManager instead of
ResourcelessTransactionManager (and apply the same fix to
WeeklyRankingJobConfig), or alternatively refactor to a single Step that
performs truncate+aggregate/atomic swap to avoid leaving MV empty on partial
failure.
Comment on lines
+76
to
+84
| @Bean(JOB_NAME) | ||
| public Job weeklyRankingJob() { | ||
| return new JobBuilder(JOB_NAME, jobRepository) | ||
| .incrementer(new RunIdIncrementer()) | ||
| .start(truncateWeeklyMvStep()) | ||
| .next(weeklyAggregateAndRankStep()) | ||
| .listener(jobListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Job 레벨 파라미터 검증기 미등록으로 truncate 선행 위험이 있다.
truncateWeeklyMvStep → weeklyAggregateAndRankStep 구성에서 targetDate 가 부재/형식 오류여도 Step 이 기동되어 base_date = ? 에 null 또는 예외 상황이 전파된다. 월간 Job 과 동일한 톤으로 JobParametersValidator 를 등록해 Step 기동 전에 실패시키는 것을 권고한다.
public Job weeklyRankingJob() {
return new JobBuilder(JOB_NAME, jobRepository)
.incrementer(new RunIdIncrementer())
+ .validator(new DefaultJobParametersValidator(
+ new String[]{"targetDate"}, new String[]{"run.id"}))
.start(truncateWeeklyMvStep())
.next(weeklyAggregateAndRankStep())
.listener(jobListener)
.build();
}추가 테스트 제안: 파라미터 누락 시 Job 이 즉시 FAILED 되고 truncateWeeklyMvStep 실행 이력이 생성되지 않는지 E2E 로 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/weekly/WeeklyRankingJobConfig.java`
around lines 76 - 84, Register and attach a JobParametersValidator to the
weeklyRankingJob (JOB_NAME) so the job-level validation fails fast when
targetDate is missing or malformed before truncateWeeklyMvStep() or
weeklyAggregateAndRankStep() run; add a validator bean (or configure
DefaultJobParametersValidator/custom validator) that requires "targetDate" and
enforces the expected format, then call .validator(yourValidator) on the
JobBuilder in weeklyRankingJob() to prevent step execution when parameters are
invalid and enable the described E2E test that ensures job fails immediately and
truncateWeeklyMvStep has no execution record.
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.
📌 Summary
GET /api/v1/rankingsAPI와 수동/전체 집계로만 관리되던 랭킹 데이터를 주기적으로 자동 집계하는 배치가 없었음period파라미터를 추가해 일간/주간/월간 랭킹을 단일 엔드포인트에서 제공commerce-batch에WeeklyRankingJob/MonthlyRankingJob구현,GET /api/v1/rankings?period=DAILY|WEEKLY|MONTHLY지원🧭 Context & Decision
문제 정의
GET /api/v1/rankings는 Redis ZSET 기반 일간 랭킹만 제공. 주간/월간 랭킹 집계 배치 없음mv_product_rank_weekly,mv_product_rank_monthly테이블이 비어 있음선택지와 결정
[1] 집계 구조 선택 (원장 직접 집계 vs 계층형 집계)
product_metrics(원장)를 직접 읽어 7일/30일치 한 번에 GROUP BY[2] ItemReader 방식 선택 (JdbcCursorItemReader vs JdbcPagingItemReader)
JdbcCursorItemReader— 커넥션 유지하며 커서로 한 줄씩 스트리밍, SQL 직접 작성 가능JdbcPagingItemReader— 페이지 단위로 쿼리 반복, 읽기 완료 후 커넥션 반환JdbcCursorItemReader)GROUP BY+ 복잡한 score 기반ORDER BY집계 쿼리를 SQL 그대로 표현할 수 있어JdbcCursorItemReader가 더 적합JdbcPagingItemReader전환 고려[3] ItemWriter 방식 선택 (JpaItemWriter vs JdbcBatchItemWriter)
JpaItemWriter— JPA 엔티티로 merge/persist, 영속성 컨텍스트 관리JdbcBatchItemWriter— SQL INSERT 직접 작성, JDBC batch로 실행JdbcBatchItemWriter)JpaItemWriter는merge()특성상 INSERT 전 불필요한 SELECT가 발생함. 단순 INSERT만 필요한 상황에서 영속성 컨텍스트 오버헤드가 불필요하여JdbcBatchItemWriter로 선택🏗️ Design Overview
변경 범위
commerce-batch(랭킹 Job),commerce-api(Ranking API, 도메인/인프라)commerce-batch:WeeklyRankingJobConfig,MonthlyRankingJobConfig, 각 Processor/Tasklet,MvProductRankWeekly,MvProductRankMonthly,ProductAggregationcommerce-api:RankingPeriod,MvProductRankWeekly,MvProductRankMonthly,WeeklyRankingRepository,MonthlyRankingRepository,JpaWeeklyRankingRepository,JpaMonthlyRankingRepository,WeeklyRankingJpaRepository,MonthlyRankingJpaRepository주요 컴포넌트 책임
WeeklyRankingJobConfig/MonthlyRankingJobConfig: Job 흐름 정의 (truncate → aggregate & rank),@ConditionalOnProperty로 Job별 활성화WeeklyRankingProcessor/MonthlyRankingProcessor:ProductAggregation→MvProductRankWeekly|Monthly변환,AtomicInteger로 rank 부여TruncateWeeklyMvTasklet/TruncateMonthlyMvTasklet: 실행 대상 기간 데이터만 삭제 (멱등성 보장)RankingFacade:period파라미터로 분기 — DAILY는 Redis ZSET, WEEKLY/MONTHLY는 MV 테이블 조회🔁 Flow Diagram
Main Flow — Spring Batch (주간 기준, 월간 동일)
Main Flow — Ranking API (WEEKLY 기준, MONTHLY 동일)
Main Flow — Ranking API (DAILY)
변경 목적: Spring Batch로 주간/월간 랭킹을 자동 집계해 MV 테이블에 적재하고, 단일 Ranking API에 period 파라미터(DAILY|WEEKLY|MONTHLY)를 추가해 기간별 조회를 제공함.
핵심 변경점: Weekly/Monthly Batch(JobConfig, Processor, Tasklet, JdbcCursorItemReader/Writer) 추가로 product_metrics를 직접 집계해 mv_product_rank_weekly/monthly에 상위 100개를 저장하고, API 레이어에 RankingPeriod enum 및 기간별 조회 로직(주: 해당 주의 월요일 기준, 월: 해당 월 1일 기준) 추가.
리스크/주의사항: Weekly Job은 targetDate가 월요일인지 검증하며(아니면 예외), Monthly는 targetYearMonth(yyyyMM)를 요구; 배치가 실시간이 아니므로 데이터 신선도(실행 주기/스케줄)와 대량 데이터 집계 성능(쿼리·인덱스)이 우려됨.
버그 수정 요약: chunk retry 시 AtomicInteger 누적으로 인한 rank 불일치 문제를 ROW_NUMBER() 기반 DB 순위 부여로 해결(멱등성·정합성 향상).
테스트/검증 방법: 단위/통합 및 E2E 테스트 추가(RankingFacadeTest, JpaWeekly/Monthly repository 테스트, Weekly/Monthly Job E2E, RankingApiE2ETest)로 날짜 변환·페이징·상위100 제한·빈 결과 계약·잘못된 period 검증을 포함함.
확인 요청: 배치 실행 스케줄(예: cron/kubernetes CronJob)과 운영 시 리소스/인덱스 튜닝 책임(데이터베이스·운영 설정)은 이 PR에 포함되나요, 아니면 별도 작업으로 계획되어 있나요?