[volume-10] Spring Batch 기반 주간/월간 랭킹 시스템 구현 - 윤유탁#394
Open
yoon-yoo-tak wants to merge 4 commits intoLoopers-dev-lab:yoon-yoo-takfrom
Open
[volume-10] Spring Batch 기반 주간/월간 랭킹 시스템 구현 - 윤유탁#394yoon-yoo-tak wants to merge 4 commits intoLoopers-dev-lab:yoon-yoo-takfrom
yoon-yoo-tak wants to merge 4 commits intoLoopers-dev-lab:yoon-yoo-takfrom
Conversation
- ledger → staging(upsert) → MV TOP-100 파이프라인 신규 구성 - Step 0 DELETE + Step 1 Chunk 누적 + Step 2 단일 서브쿼리 ROW_NUMBER 로 tie-break 결정성 확보 - WeekRange VO (ISO 1월 4일 앵커) + JobParametersValidator 로 포맷 사전 검증 - chunk 내 product_id pre-aggregate 로 ON DUPLICATE KEY UPDATE 드라이버 의존성 제거 - Testcontainers MySQL 기반 E2E 테스트 (tie-break, 멱등성, 이전 주차 보호 포함) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- 주간 Job 과 대칭 구조 (Step 0/1/2), product_id pre-aggregate 는 Weekly Writer 의 static helper 공용 - MonthRange VO 가 YearMonth 기반으로 윤년/30·31일 월 자동 처리 - year_month_key 컬럼명 사용: MySQL 8.0 parser 가 unquoted year_month 를 거부(ERROR 1064)하는 제약 회피 - MonthlyJobParametersValidator + E2E 로 경계/포맷/TOP-100 컷 커버 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- 현재 주 포함 최근 4주 / 현재 월 포함 최근 3개월만 보존하고 그 이전 staging row 삭제
- MV 테이블은 건드리지 않으며 E2E 테스트로 회귀 가드
- LocalDate.now(KST) 명시로 JVM TZ 의존성 제거
- JobListener 의 \${...} interpolation 버그 수정 (SLF4J placeholder) + afterJob 에 batchStatus/exitStatus 기록
- CommerceBatchApplicationTest 가 auto-run 을 우회하도록 spring.batch.job.enabled=false 추가
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- GET /api/v1/rankings 에 period(DAILY|WEEKLY|MONTHLY) 쿼리 파라미터 추가 - ApplicationService switch 분기: DAILY=Redis graceful degradation, WEEKLY/MONTHLY=MV 조회 (정확성 우선, 예외 전파) - WeeklyRankingRepository / MonthlyRankingRepository + JdbcTemplate 구현체 - Controller @validated + @Min/@Max/@pattern 으로 수동 if/throw 제거, size 상한 100 강제 - FakePeriodRankingRepository + Weekly/Monthly Fake + RankingQueryServiceImpl 테스트 확장 (fail-loud 검증 포함) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRankingQueryService API를 일일 중심에서 기간 기반 통합 엔드포인트로 리팩토링하고, 주간 및 월간 랭킹 저장소 구현 및 배치 작업을 추가하며, 관련된 스키마 엔티티 및 마이그레이션을 도입한다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant RankingV1Controller
participant RankingQueryService
participant DailyRepository as Redis<br/>(Daily)
participant WeeklyRepo as WeeklyRankingRepository<br/>(MV)
participant MonthlyRepo as MonthlyRankingRepository<br/>(MV)
Client->>RankingV1Controller: GET /ranking?date=...&period=WEEKLY&page=0&size=20
RankingV1Controller->>RankingV1Controller: 날짜 파싱, LocalDate 생성
RankingV1Controller->>RankingQueryService: getRanking(WEEKLY, baseDate, 0, 20)
alt 기간별 라우팅
alt DAILY
RankingQueryService->>DailyRepository: Redis 조회 (date)
DailyRepository-->>RankingQueryService: 일일 랭킹 반환
else WEEKLY
RankingQueryService->>RankingQueryService: baseDate → year-week 키 변환<br/>(2026-W15)
RankingQueryService->>WeeklyRepo: findTopN("2026-W15", offset, limit)
WeeklyRepo->>WeeklyRepo: SQL 실행: mv_product_rank_weekly 쿼리
WeeklyRepo-->>RankingQueryService: 상위 N개 제품 랭킹
RankingQueryService->>WeeklyRepo: countByPeriod("2026-W15")
WeeklyRepo-->>RankingQueryService: 전체 카운트
else MONTHLY
RankingQueryService->>RankingQueryService: baseDate → year-month 키 변환<br/>(2026-04)
RankingQueryService->>MonthlyRepo: findTopN("2026-04", offset, limit)
MonthlyRepo->>MonthlyRepo: SQL 실행: mv_product_rank_monthly 쿼리
MonthlyRepo-->>RankingQueryService: 상위 N개 제품 랭킹
RankingQueryService->>MonthlyRepo: countByPeriod("2026-04")
MonthlyRepo-->>RankingQueryService: 전체 카운트
end
end
RankingQueryService->>RankingQueryService: PageResult 구성
RankingQueryService-->>RankingV1Controller: PageResult<ProductRanking>
RankingV1Controller-->>Client: ApiResponse{items, totalPages, ...}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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: 11
🧹 Nitpick comments (14)
apps/commerce-api/src/test/java/com/loopers/domain/ranking/FakeRankingRepository.java (1)
20-22: 강제 실패 상태 해제 API를 명시적으로 추가하는 것이 좋다운영 관점이라기보다 테스트 운영 관점에서, Line 20-22의
failWith()만 존재하면 동일 fake 인스턴스를 재사용하는 테스트 구성에서 실패 상태가 잔류해 플래키 테스트를 유발할 수 있다.
수정안으로clearFailure()를 추가해 의도를 명시하고, 테스트에서 실패 주입/해제를 명확히 분리하는 편이 안전하다.
추가 테스트로failWith()호출 후 예외가 발생하고,clearFailure()이후 동일 메서드가 정상 동작하는 시나리오를 검증하는 것을 권장한다.♻️ 제안 diff
public class FakeRankingRepository implements RankingRepository { @@ public void failWith(RuntimeException exception) { this.forcedFailure = exception; } + + public void clearFailure() { + this.forcedFailure = null; + }Also applies to: 26-28, 44-46, 66-68
🤖 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/domain/ranking/FakeRankingRepository.java` around lines 20 - 22, Add an explicit clearFailure() API to FakeRankingRepository that resets the forcedFailure field (the counterpart to failWith(RuntimeException exception)) so tests can inject and then clear failures without leaking state; update the class methods that set forcedFailure (failWith) to be paired with clearFailure, and add unit tests that assert an exception is thrown after failWith(...) and that the same call succeeds after clearFailure() is invoked; ensure the new method name clearFailure and the existing forcedFailure and failWith symbols are used to locate and modify the implementation and tests.apps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/MonthRangeTest.java (1)
88-117:ofPreviousMonth(null)예외 케이스 테스트가 누락되었다.
MonthRange.ofPreviousMonth은Assert.notNull로 null을 검증하지만, 테스트에서 null 입력에 대한 예외 발생 검증이 없다.WeekRangeTest와 동일한 누락 패턴이다.🧪 null 입력 테스트 추가 제안
`@Test` void throwsException_whenBaseDateIsNull() { assertThatThrownBy(() -> MonthRange.ofPreviousMonth(null)) .isInstanceOf(IllegalArgumentException.class); }🤖 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/batch/domain/ranking/MonthRangeTest.java` around lines 88 - 117, Add a null-input test inside the OfPreviousMonth nested test class to assert that MonthRange.ofPreviousMonth(null) throws an IllegalArgumentException; specifically, create a test method (e.g., throwsException_whenBaseDateIsNull) that calls MonthRange.ofPreviousMonth(null) wrapped in assertThatThrownBy(...) and checks isInstanceOf(IllegalArgumentException.class) so the null-validation branch in MonthRange.ofPreviousMonth is covered.apps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/MonthRange.java (1)
15-19: WeekRange와 MonthRange 간BASIC_ISO상수가 중복된다.두 클래스 모두 동일한
DateTimeFormatter.BASIC_ISO_DATE상수를 정의한다. 현재는 문서에서 WeekRange를 참조하고 있으나, 포맷이 변경될 경우 양쪽 모두 수정해야 하는 위험이 있다. 공통 상수 클래스로 추출하는 것을 고려할 수 있다.향후 유지보수 시 공통 상수 클래스(예:
RankingDateConstants)로 추출하여 단일 변경점을 유지하는 방안을 검토할 수 있다.🤖 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/domain/ranking/MonthRange.java` around lines 15 - 19, MonthRange and WeekRange both declare the same BASIC_ISO constant; extract this shared DateTimeFormatter into a single common constant (e.g., create a RankingDateConstants class or similar) and update MonthRange and WeekRange to reference RankingDateConstants.BASIC_ISO (or whatever static name you choose) instead of declaring their own BASIC_ISO; ensure the new constant uses DateTimeFormatter.BASIC_ISO_DATE and update imports/usages in the MonthRange and WeekRange classes.apps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/WeekRangeTest.java (2)
83-89: W53이 존재하는 연도에 대한 테스트가 누락되었다.
throwsException_whenWeekNumberOutOfRange에서 2026-W54는 테스트하지만, W53이 실제로 존재하는 연도(예: 2020년, 2015년)에 대한 정상 케이스 테스트가 없다. ISO 8601에서 일부 연도는 53주를 가지므로, 이에 대한 검증이 필요하다.🧪 W53 존재 연도 테스트 추가 제안
`@Test` void handlesYearWith53Weeks() { // 2020년은 ISO 기준 53주를 가진 해 WeekRange range = WeekRange.of("2020-W53"); assertThat(range.start()).isEqualTo(LocalDate.of(2020, 12, 28)); assertThat(range.end()).isEqualTo(LocalDate.of(2021, 1, 3)); }🤖 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/batch/domain/ranking/WeekRangeTest.java` around lines 83 - 89, The test suite is missing a positive case for years that legitimately have 53 ISO weeks; add a new unit test (e.g., handlesYearWith53Weeks) that calls WeekRange.of("2020-W53") and asserts the expected start and end LocalDate values for that week; ensure the new test uses the same WeekRange.start() and WeekRange.end() accessors as existing tests and verifies that "2020-W53" does not throw and yields start == LocalDate.of(2020,12,28) and end == LocalDate.of(2021,1,3) (adjust dates if your WeekRange logic expects different boundaries).
92-121:ofPreviousWeek(null)예외 케이스 테스트가 누락되었다.
WeekRange.ofPreviousWeek은Assert.notNull로 null을 검증하지만, 테스트에서 null 입력에 대한 예외 발생 검증이 없다. 방어 코드가 의도대로 동작하는지 확인하는 테스트가 필요하다.🧪 null 입력 테스트 추가 제안
`@Test` void throwsException_whenBaseDateIsNull() { assertThatThrownBy(() -> WeekRange.ofPreviousWeek(null)) .isInstanceOf(IllegalArgumentException.class); }🤖 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/batch/domain/ranking/WeekRangeTest.java` around lines 92 - 121, The test suite is missing a null-input assertion for WeekRange.ofPreviousWeek; add a unit test in WeekRangeTest (inside the OfPreviousWeek nested class) that invokes WeekRange.ofPreviousWeek(null) and asserts an IllegalArgumentException is thrown (e.g., using AssertJ's assertThatThrownBy(() -> WeekRange.ofPreviousWeek(null)).isInstanceOf(IllegalArgumentException.class)); this verifies the existing Assert.notNull defensive check in the ofPreviousWeek method behaves as expected.apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java (1)
30-33:getLong(START_TIME_KEY)호출 시 키 부재 처리가 없다.
ExecutionContext.getLong(key)는 키가 존재하지 않으면 기본값0L을 반환한다.beforeJob이 실행되지 않거나 예외로 인해startTime이 저장되지 않은 경우,endTime - 0으로 잘못된 duration이 계산되어 운영 로그 분석에 혼란을 줄 수 있다.♻️ 키 존재 여부 확인 또는 기본값 처리 제안
`@AfterJob` void afterJob(JobExecution jobExecution) { String jobName = jobExecution.getJobInstance().getJobName(); - var startTime = jobExecution.getExecutionContext().getLong(START_TIME_KEY); + var context = jobExecution.getExecutionContext(); + var startTime = context.containsKey(START_TIME_KEY) + ? context.getLong(START_TIME_KEY) + : jobExecution.getStartTime() != null + ? jobExecution.getStartTime().atZone(KST).toInstant().toEpochMilli() + : System.currentTimeMillis(); var endTime = System.currentTimeMillis();🤖 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/listener/JobListener.java` around lines 30 - 33, In afterJob(JobExecution jobExecution) the call to ExecutionContext.getLong(START_TIME_KEY) can return 0L when the key is missing, producing a bogus duration; update afterJob to first check the ExecutionContext for the presence of START_TIME_KEY (or use containsKey/contains) and handle the missing case by logging a warning/error and skipping or using a sensible fallback instead of computing endTime - 0, referencing the afterJob method, START_TIME_KEY constant and the beforeJob that should set the value; ensure you do not assume beforeJob always ran and avoid emitting misleading duration metrics when the start time is absent.apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/WeeklyJobParametersValidatorTest.java (1)
25-34: 빈 문자열(blank) 파라미터에 대한 테스트가 누락되었다.
throwsWhenMissing은 파라미터가 없는 경우만 테스트한다.year_week가 빈 문자열("")이나 공백(" ")인 경우에도JobParametersInvalidException이 발생하는지 검증이 필요하다. Validator의yearWeek.isBlank()분기를 커버하지 않는다.🧪 blank 파라미터 테스트 추가 제안
`@Test` `@DisplayName`("빈 문자열 year_week 는 JobParametersInvalidException 을 던진다.") void throwsWhenBlank() { JobParameters params = new JobParametersBuilder() .addString("year_week", " ") .toJobParameters(); assertThatThrownBy(() -> validator.validate(params)) .isInstanceOf(JobParametersInvalidException.class) .hasMessageContaining("year_week"); }🤖 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/batch/job/ranking/validator/WeeklyJobParametersValidatorTest.java` around lines 25 - 34, Add a new unit test to cover the validator's isBlank() branch: in WeeklyJobParametersValidatorTest create a test (e.g., throwsWhenBlank) that builds JobParameters with addString("year_week", " ") (and/or "") and asserts that validator.validate(params) throws JobParametersInvalidException with a message containing "year_week"; this complements the existing throwsWhenMissing test and ensures the validator's blank-string path is exercised.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidator.java (1)
9-31:WeeklyJobParametersValidator와 구조가 동일하여 중복이 발생한다.두 validator의 로직이 파라미터 이름과 Range 클래스만 다르고 검증 흐름은 동일하다. 현재 수준에서는 허용 가능하나, 향후 검증 규칙 추가 시 양쪽을 모두 수정해야 하는 위험이 있다.
필요 시 제네릭 기반 추상 클래스나 함수형 인터페이스를 통해 공통 로직을 추출할 수 있다:
// 예시: 공통 유틸리티 메서드 public static void validateRequiredParam( JobParameters params, String key, Consumer<String> parser ) throws JobParametersInvalidException { ... }🤖 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/validator/MonthlyJobParametersValidator.java` around lines 9 - 31, MonthlyJobParametersValidator duplicates WeeklyJobParametersValidator: both implement the same validation flow differing only by the parameter name and Range parser. Refactor by extracting the common logic into a reusable abstraction (e.g., an AbstractJobParametersValidator or a static utility validateRequiredParam(JobParameters, String, Function<String, ?>/Consumer<String>) ) and have MonthlyJobParametersValidator call that common method passing PARAM_YEAR_MONTH and MonthRange::of (or override a protected getParamKey()/parse method in the abstract class); update MonthlyJobParametersValidator to delegate to the new shared implementation and remove the duplicated validation code.apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidatorTest.java (1)
36-54: 공백year_month경계값 테스트를 추가하는 것이 안전하다.Line 36-54 기준으로 현재 실패 케이스는 잘 잡고 있지만, 운영에서 수동 실행/스케줄 파라미터 오입력으로 가장 자주 발생하는
""," "케이스 회귀 보호가 비어 있다.
수정안으로 공백 입력 시JobParametersInvalidException을 검증하는 파라미터화 테스트를 추가하는 것이 좋다.
추가 테스트로@ParameterizedTest+@ValueSource(strings = {"", " "})케이스를 권장한다.제안 코드
+import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; ... + `@ParameterizedTest` + `@ValueSource`(strings = {"", " "}) + `@DisplayName`("공백 year_month 는 JobParametersInvalidException 을 던진다.") + void throwsWhenYearMonthBlank(String yearMonth) { + JobParameters params = new JobParametersBuilder() + .addString("year_month", yearMonth) + .toJobParameters(); + assertThatThrownBy(() -> validator.validate(params)) + .isInstanceOf(JobParametersInvalidException.class) + .hasMessageContaining("year_month"); + }As per coding guidelines "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.".
🤖 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/batch/job/ranking/validator/MonthlyJobParametersValidatorTest.java` around lines 36 - 54, Add a parameterized test in MonthlyJobParametersValidatorTest that asserts validator.validate(...) throws JobParametersInvalidException for blank year_month inputs; create a `@ParameterizedTest` with `@ValueSource`(strings = {"", " "}) and for each value build JobParameters via new JobParametersBuilder().addString("year_month", value).toJobParameters() and assertThatThrownBy(() -> validator.validate(params)).isInstanceOf(JobParametersInvalidException.class) to cover empty and whitespace-only boundary cases.sql/V12__create_mv_product_rank_monthly.sql (1)
8-8:ranking_position에 기간 내 유일 제약을 두는 편이 운영 안정성에 유리하다.운영 관점에서 동일
year_month_key에 같은ranking_position이 중복 저장되면 페이지 경계에서 중복/누락 응답이 발생하고 장애 분석이 어려워진다. 인덱스는 조회 최적화만 보장하므로 무결성은 DB 제약으로 고정하는 편이 안전하다.UNIQUE (year_month_key, ranking_position)로 바꾸고, 월간 MV 적재 테스트에 중복 순위 삽입 시 제약 위반을 검증하는 케이스를 추가하는 것을 권장한다.변경 제안
- KEY idx_mv_product_rank_monthly_position (year_month_key, ranking_position) + UNIQUE KEY uk_mv_product_rank_monthly_period_position (year_month_key, ranking_position)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sql/V12__create_mv_product_rank_monthly.sql` at line 8, 현재 인덱스 KEY idx_mv_product_rank_monthly_position (year_month_key, ranking_position)은 조회용 인덱스일 뿐 무결성을 보장하지 않으니, 복수의 레코드가 같은 year_month_key와 ranking_position으로 들어오는 것을 막기 위해 인덱스 정의를 UNIQUE (year_month_key, ranking_position)로 변경하고 MV 적재 로직/스크립트에서 중복 순위 삽입 시 제약 위반이 발생하는지를 검증하는 테스트 케이스를 추가하세요; 관련 식별자는 idx_mv_product_rank_monthly_position, year_month_key, ranking_position 및 월간 MV 적재 테스트(또는 해당 로더 함수)를 참고해 수정 및 테스트를 구현하면 됩니다.apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/step/WeeklyStagingUpsertWriterTest.java (1)
14-67: 예외 경로 테스트가 없어 배치 장애 회귀를 놓칠 수 있다.운영에서는 비정상 데이터 한 건(
productId == null,delta == NaN/Infinity)으로 청크 전체가 실패할 수 있는데, 현재 테스트는 정상 경로만 검증하고 있어 실패 정책 회귀를 조기에 잡기 어렵다.
수정안은aggregateByProductId의 입력 검증 정책을 명시하고, 본 테스트 클래스에 예외 케이스를 추가하는 것이다.
추가 테스트는null productId입력 시 예외,NaN/Infinity delta입력 시 예외, 기존 정상 케이스 유지 검증으로 구성하면 충분하다.테스트 보강 예시
+ `@Test` + `@DisplayName`("product_id 가 null 이면 즉시 예외를 던진다.") + void rejectsNullProductId() { + List<StagingDelta> items = List.of( + new StagingDelta(null, 1.0) + ); + assertThatThrownBy(() -> WeeklyStagingUpsertWriter.aggregateByProductId(items)) + .isInstanceOf(IllegalArgumentException.class); + } + + `@Test` + `@DisplayName`("delta 가 NaN/Infinity 이면 즉시 예외를 던진다.") + void rejectsNonFiniteDelta() { + assertThatThrownBy(() -> WeeklyStagingUpsertWriter.aggregateByProductId( + List.of(new StagingDelta(1L, Double.NaN)))) + .isInstanceOf(IllegalArgumentException.class); + }🤖 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/batch/job/ranking/step/WeeklyStagingUpsertWriterTest.java` around lines 14 - 67, The tests lack negative-path coverage for invalid StagingDelta inputs; update the contract for WeeklyStagingUpsertWriter.aggregateByProductId to reject invalid inputs (throw IllegalArgumentException for null productId and for delta values that are NaN or infinite), then add unit tests in WeeklyStagingUpsertWriterTest that assert aggregateByProductId(List.of(new StagingDelta(null, 1.0))) throws, and that aggregateByProductId(List.of(new StagingDelta(1L, Double.NaN)), and similarly for Double.POSITIVE_INFINITY/NEGATIVE_INFINITY, also keep existing positive tests unchanged to ensure normal behavior; use the symbols WeeklyStagingUpsertWriter.aggregateByProductId, StagingDelta, productId, and delta to locate code to change.apps/commerce-batch/src/main/java/com/loopers/batch/job/cleanup/step/StagingCleanupTasklet.java (1)
38-46: 정리 작업 원자성을 코드에 명시하는 편이 운영 안정성에 유리하다.향후 Step 설정 변경이나 재사용 경로에서 트랜잭션 경계가 약해지면 주간만 삭제되고 월간은 남는 부분 성공 상태가 발생할 수 있다.
execute에@Transactional(propagation = Propagation.REQUIRED)를 명시해 의도를 고정하라. 추가 테스트로 두 번째 DELETE에서 예외를 발생시켜 첫 번째 DELETE가 롤백되는지 통합 테스트를 추가하라.수정 예시
+import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; @@ `@Override` + `@Transactional`(propagation = Propagation.REQUIRED) public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) {🤖 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/cleanup/step/StagingCleanupTasklet.java` around lines 38 - 46, Add an explicit transaction boundary to ensure both deletes are atomic by annotating the execute method in StagingCleanupTasklet with `@Transactional`(propagation = Propagation.REQUIRED) so the weekly and monthly deletes (DELETE_WEEKLY_SQL and DELETE_MONTHLY_SQL invocations inside execute) run in one transaction and will roll back together on failure; also add an integration test that triggers an exception after the first jdbcTemplate.update call (simulate failure on the second DELETE) and assert that the first delete was rolled back.apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/RankingQueryServiceImplTest.java (1)
110-136: ISO 주차/월 경계값 테스트를 보강하는 편이 안전하다.운영에서 연말·연초 키 변환이 틀리면 주간/월간 랭킹이 정상 데이터가 있어도 빈 결과로 보일 수 있다.
weeklyMapsBaseDateToIsoWeek/monthlyMapsBaseDateToYearMonth에 연도 전환 케이스를 추가하라. 추가 테스트로2025-12-31,2026-01-01같은 날짜가 기대 키(예:YYYY-Www,YYYY-MM)로 정확히 매핑되는지 검증하라.🤖 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/RankingQueryServiceImplTest.java` around lines 110 - 136, Add boundary tests to the existing weeklyMapsBaseDateToIsoWeek and monthlyMapsBaseDateToYearMonth tests to cover year-turn transitions: for weekly, insert scores into weeklyRankingRepository with the expected ISO-week keys for 2025-12-31 and 2026-01-01 (e.g., "2025-W53" or the correct ISO week string) and call rankingQueryService.getRanking(RankingPeriod.WEEKLY, LocalDate.of(2025,12,31), ...) and LocalDate.of(2026,1,1) to assert the returned PageResult contains those scores; for monthly, insert scores into monthlyRankingRepository with "2025-12" and "2026-01" and call rankingQueryService.getRanking(RankingPeriod.MONTHLY, LocalDate.of(...), ...) for both boundary dates to assert correct mapping to year-month keys and expected productIds and totals. Ensure you use the same repository methods (weeklyRankingRepository.addScore, monthlyRankingRepository.addScore) and the same test helpers (rankingQueryService.getRanking, ProductRanking assertions) used in the existing tests.apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/MonthlyRankingJobE2ETest.java (1)
89-188: 월간 정렬의 tie-break 계약을 검증하는 케이스가 빠져 있다.월간 MV 는
score DESC, product_id ASC가 계약인데, 현재 테스트 데이터는 모두 점수가 달라서product_id ASC가 빠져도 전부 통과한다. 운영에서는 동점 상품의 월간 순위가 비결정적으로 흔들리면 캐시 비교와 장애 재현이 어려워진다. 주간 테스트와 같은 방식으로 동점 상품 3개를 넣고10L → 20L → 30L순서로ranking_position이 생성되는 케이스를 추가하는 편이 안전하다. 추가로 100위와 101위가 동점인 경우에도product_id기준으로 컷이 고정되는지 검증해야 한다.추가하면 좋은 테스트 예시다
+ `@Test` + void tieBreakByProductIdAsc() throws Exception { + insertLedger(30L, "20260315", 5.0); + insertLedger(10L, "20260315", 5.0); + insertLedger(20L, "20260315", 5.0); + + JobExecution exec = jobLauncherTestUtils.launchJob(params(YEAR_MONTH)); + assertThat(exec.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()); + + List<Map<String, Object>> rows = jdbcTemplate.queryForList( + "SELECT product_id, ranking_position FROM mv_product_rank_monthly " + + "WHERE year_month_key = ? ORDER BY ranking_position", + YEAR_MONTH + ); + + assertThat(rows).hasSize(3); + assertThat(rows.get(0)).containsEntry("product_id", 10L); + assertThat(rows.get(1)).containsEntry("product_id", 20L); + assertThat(rows.get(2)).containsEntry("product_id", 30L); + }🤖 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/batch/job/ranking/MonthlyRankingJobE2ETest.java` around lines 89 - 188, Add two tests in MonthlyRankingJobE2ETest to verify the MV tie-break contract (score DESC, product_id ASC): (1) insert three ledgers with identical score for product_ids 10L, 20L, 30L using insertLedger(...), run job via jobLauncherTestUtils.launchJob(params(YEAR_MONTH)) and assert mv_product_rank_monthly ranking_position 1..3 map to product_id 10L, 20L, 30L respectively; (2) create >100 products where the 100th and 101st have the same score (use insertLedger for each pid), run the job and assert COUNT(*) = 100 and that between the tied pair the one with the smaller product_id is included at ranking_position 100 and the other is excluded, using jdbcTemplate queries against mv_product_rank_monthly as in existing tests.
🤖 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/infrastructure/ranking/schema/MonthlyRankingMvSchema.java`:
- Around line 18-23: Update the class-level javadoc in MonthlyRankingMvSchema to
accurately reflect the repository's true schema management: state that DDL
(including indexes and JPA `@Index` annotations) is NOT applied by JPA ddl-auto
but by the internal DB migration pipeline, remove/clarify the old Flyway/sql/V12
wording, and add a clear WARNING (mirroring WeeklyRankingMvSchema) instructing
maintainers to update the migration SQL, the commerce-batch Entity, and this API
Entity in lockstep when changing the monthly MV schema (mention the
year_month_key naming rationale and reference WeeklyRankingMvSchema). Also add a
short note recommending automated equivalence checks (columns, types/lengths,
PKs, indexes) between the Entity and the reference SQL to prevent drift.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/schema/WeeklyRankingMvSchema.java`:
- Around line 20-22: Update the JavaDoc on WeeklyRankingMvSchema to explicitly
state that an external migration pipeline (not ddl-auto) is the source of truth
and that this class/entity is read-only / used only for mapping the existing
materialized view (e.g., reference sql/V10__create_mv_product_rank_weekly.sql as
a prepared migration file), clarifying not to rely on
spring.jpa.hibernate.ddl-auto for schema changes; then add a profile-based test
(e.g., a new test class like ProductionProfileSchemaConfigTest) that loads the
production/operational Spring profile and asserts spring.jpa.hibernate.ddl-auto
is not set to a schema-creating value (validate it is none/validate/update
disabled) to prevent accidental schema generation in deployment.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`:
- Around line 12-18: The controller currently uses Bean Validation annotations
and automatic enum binding which causes framework exceptions; remove the
`@Validated` annotation and all Jakarta constraint annotations and stop relying on
automatic RankingPeriod binding in RankingV1Controller, then implement
controller-side validation methods parsePeriod(String), validatePage(int),
validateSize(int) (reusing the existing parseDate(String) pattern) to
parse/validate period, page and size and throw CoreException with
ErrorType.BAD_REQUEST on any invalid input; ensure the controller calls
parseDate(), parsePeriod(), validatePage(), validateSize() before processing and
let ApiControllerAdvice handle CoreException, and add integration tests that
assert invalid inputs (period=INVALID, page=-1, size=101, date=20260230) all
produce the unified CoreException → ApiControllerAdvice error response.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/ranking/FakePeriodRankingRepository.java`:
- Around line 35-37: In FakePeriodRankingRepository, avoid directly casting
offset to int; validate that offset and limit are non-negative, return an empty
result when offset >= sorted.size(), and perform the conversion with
Math.toIntExact(offset) to guard against overflow. Concretely, in the method
using variables offset, limit, startIndex, endExclusive and sorted.size(), first
check if (offset < 0 || limit < 0) and handle as invalid (e.g., throw or return
empty per test semantics), then if (offset >= sorted.size()) return an empty
list, otherwise compute int startIndex = Math.toIntExact(offset) and int
endExclusive = Math.min(startIndex + Math.toIntExact(limit), sorted.size())
before iterating.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/MonthlyRankingMvSchema.java`:
- Around line 17-19: The file MonthlyRankingMvSchema.java contains a misleading
comment stating the app manages schema via Hibernate ddl-auto; update the header
comment to state the schema is managed by the external DB migration pipeline
(referencing sql/V12__create_mv_product_rank_monthly.sql as the Flyway
reference) and remove any implication that JPA ddl-auto is used; additionally,
ensure the batch module configuration disables ddl-auto (check application
properties / DataSource config used by MonthlyRankingMvSchema) and add an
integration test that boots the batch module with ddl-auto disabled and runs the
migration SQL to verify the materialized view, tables and indexes (the V12
script) are created successfully.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/WeeklyRankingMvSchema.java`:
- Around line 19-22: Update the header comment in WeeklyRankingMvSchema.java to
remove the assertion that Hibernate ddl-auto manages all environments and
instead state that schema and index changes (including JPA `@Index` definitions)
are managed by the internal DB migration pipeline; explicitly list the
synchronization responsibilities (batch entity, API entity, and reference SQL
file sql/V10__create_mv_product_rank_weekly.sql) and add a note suggesting CI
validation to assert entity metadata (columns, PKs, indexes) matches the
reference SQL for the weekly MV; keep the class name WeeklyRankingMvSchema as
context for where this policy applies.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/WeeklyRankingStagingSchema.java`:
- Around line 16-18: Update the comment in WeeklyRankingStagingSchema to stop
presenting Hibernate's ddl-auto as the authoritative schema owner: reword the
lines referencing "Hibernate {`@code` ddl-auto}" and the SQL file {`@code`
sql/V11__create_mv_product_rank_weekly_staging.sql} to state that the SQL is a
reference used by the project's internal DB migration pipeline (not managed by
JPA ddl-auto) and avoid implying JPA creates indexes/PKs; additionally, add a
note recommending a smoke test that programmatically compares the batch module
entity metadata (entities referenced by the WeeklyRankingStagingSchema) against
the reference SQL (sql/V11__create_mv_product_rank_weekly_staging.sql) to
validate columns, primary keys, and indexes.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyStagingToMvTasklet.java`:
- Around line 52-55: Add fail-fast validation at the start of
MonthlyStagingToMvTasklet.execute: assert the incoming yearMonth is non-blank
using Assert.hasText(yearMonth, "...") and validate/parse it via
MonthRange.of(yearMonth) (or equivalent) so invalid or misformatted year_month
immediately throws; only proceed to call jdbcTemplate.update(DELETE_SQL, ...)
and jdbcTemplate.update(INSERT_SQL, ...) after validation succeeds. Also add
unit tests that pass blank and malformed year_month values to execute and assert
that an exception is thrown and that jdbcTemplate.update is never invoked.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyStagingToMvTasklet.java`:
- Around line 53-56: Add input validation at the start of
WeeklyStagingToMvTasklet.execute: assert that yearWeek is non-blank (use
Assert.hasText(yearWeek,...)) and validate its format by calling
WeekRange.of(yearWeek) so invalid or blank values throw immediately and prevent
JDBC updates; update tests for WeeklyStagingToMvTasklet to assert that blank or
malformed inputs (e.g., "2026W15") cause an exception and that
jdbcTemplate.update is not invoked when validation fails.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyStagingUpsertWriter.java`:
- Around line 59-63: The aggregateByProductId method should validate inputs to
avoid NPEs and bad numeric values: in aggregateByProductId(List<? extends
StagingDelta> items) iterate items and for each StagingDelta (referenced by
item.productId() and item.delta()) assert productId != null and delta is finite
(not null, not NaN, not Infinite); if a check fails, throw an explicit
IllegalArgumentException with a clear message indicating which field and value
caused the failure. Update unit tests to cover null productId, NaN and Infinite
delta cases (expect IllegalArgumentException) and a normal case to verify
correct aggregation.
In `@sql/V10__create_mv_product_rank_weekly.sql`:
- Line 8: Change the non-unique index definition for
idx_mv_product_rank_weekly_position into a UNIQUE KEY to enforce DB-level
uniqueness on (year_week, ranking_position) and prevent duplicate ranks: update
the table definition in V10__create_mv_product_rank_weekly.sql replacing KEY
idx_mv_product_rank_weekly_position (year_week, ranking_position) with UNIQUE
KEY idx_mv_product_rank_weekly_position (year_week, ranking_position). Also add
an integration test that attempts to insert two rows with the same year_week and
ranking_position against the materialized view/table and asserts that the
operation fails with a duplicate key (unique constraint) error to prevent
regressions.
---
Nitpick comments:
In
`@apps/commerce-api/src/test/java/com/loopers/domain/ranking/FakeRankingRepository.java`:
- Around line 20-22: Add an explicit clearFailure() API to FakeRankingRepository
that resets the forcedFailure field (the counterpart to
failWith(RuntimeException exception)) so tests can inject and then clear
failures without leaking state; update the class methods that set forcedFailure
(failWith) to be paired with clearFailure, and add unit tests that assert an
exception is thrown after failWith(...) and that the same call succeeds after
clearFailure() is invoked; ensure the new method name clearFailure and the
existing forcedFailure and failWith symbols are used to locate and modify the
implementation and tests.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/RankingQueryServiceImplTest.java`:
- Around line 110-136: Add boundary tests to the existing
weeklyMapsBaseDateToIsoWeek and monthlyMapsBaseDateToYearMonth tests to cover
year-turn transitions: for weekly, insert scores into weeklyRankingRepository
with the expected ISO-week keys for 2025-12-31 and 2026-01-01 (e.g., "2025-W53"
or the correct ISO week string) and call
rankingQueryService.getRanking(RankingPeriod.WEEKLY, LocalDate.of(2025,12,31),
...) and LocalDate.of(2026,1,1) to assert the returned PageResult contains those
scores; for monthly, insert scores into monthlyRankingRepository with "2025-12"
and "2026-01" and call rankingQueryService.getRanking(RankingPeriod.MONTHLY,
LocalDate.of(...), ...) for both boundary dates to assert correct mapping to
year-month keys and expected productIds and totals. Ensure you use the same
repository methods (weeklyRankingRepository.addScore,
monthlyRankingRepository.addScore) and the same test helpers
(rankingQueryService.getRanking, ProductRanking assertions) used in the existing
tests.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/MonthRange.java`:
- Around line 15-19: MonthRange and WeekRange both declare the same BASIC_ISO
constant; extract this shared DateTimeFormatter into a single common constant
(e.g., create a RankingDateConstants class or similar) and update MonthRange and
WeekRange to reference RankingDateConstants.BASIC_ISO (or whatever static name
you choose) instead of declaring their own BASIC_ISO; ensure the new constant
uses DateTimeFormatter.BASIC_ISO_DATE and update imports/usages in the
MonthRange and WeekRange classes.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/cleanup/step/StagingCleanupTasklet.java`:
- Around line 38-46: Add an explicit transaction boundary to ensure both deletes
are atomic by annotating the execute method in StagingCleanupTasklet with
`@Transactional`(propagation = Propagation.REQUIRED) so the weekly and monthly
deletes (DELETE_WEEKLY_SQL and DELETE_MONTHLY_SQL invocations inside execute)
run in one transaction and will roll back together on failure; also add an
integration test that triggers an exception after the first jdbcTemplate.update
call (simulate failure on the second DELETE) and assert that the first delete
was rolled back.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidator.java`:
- Around line 9-31: MonthlyJobParametersValidator duplicates
WeeklyJobParametersValidator: both implement the same validation flow differing
only by the parameter name and Range parser. Refactor by extracting the common
logic into a reusable abstraction (e.g., an AbstractJobParametersValidator or a
static utility validateRequiredParam(JobParameters, String, Function<String,
?>/Consumer<String>) ) and have MonthlyJobParametersValidator call that common
method passing PARAM_YEAR_MONTH and MonthRange::of (or override a protected
getParamKey()/parse method in the abstract class); update
MonthlyJobParametersValidator to delegate to the new shared implementation and
remove the duplicated validation code.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java`:
- Around line 30-33: In afterJob(JobExecution jobExecution) the call to
ExecutionContext.getLong(START_TIME_KEY) can return 0L when the key is missing,
producing a bogus duration; update afterJob to first check the ExecutionContext
for the presence of START_TIME_KEY (or use containsKey/contains) and handle the
missing case by logging a warning/error and skipping or using a sensible
fallback instead of computing endTime - 0, referencing the afterJob method,
START_TIME_KEY constant and the beforeJob that should set the value; ensure you
do not assume beforeJob always ran and avoid emitting misleading duration
metrics when the start time is absent.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/MonthRangeTest.java`:
- Around line 88-117: Add a null-input test inside the OfPreviousMonth nested
test class to assert that MonthRange.ofPreviousMonth(null) throws an
IllegalArgumentException; specifically, create a test method (e.g.,
throwsException_whenBaseDateIsNull) that calls MonthRange.ofPreviousMonth(null)
wrapped in assertThatThrownBy(...) and checks
isInstanceOf(IllegalArgumentException.class) so the null-validation branch in
MonthRange.ofPreviousMonth is covered.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/WeekRangeTest.java`:
- Around line 83-89: The test suite is missing a positive case for years that
legitimately have 53 ISO weeks; add a new unit test (e.g.,
handlesYearWith53Weeks) that calls WeekRange.of("2020-W53") and asserts the
expected start and end LocalDate values for that week; ensure the new test uses
the same WeekRange.start() and WeekRange.end() accessors as existing tests and
verifies that "2020-W53" does not throw and yields start ==
LocalDate.of(2020,12,28) and end == LocalDate.of(2021,1,3) (adjust dates if your
WeekRange logic expects different boundaries).
- Around line 92-121: The test suite is missing a null-input assertion for
WeekRange.ofPreviousWeek; add a unit test in WeekRangeTest (inside the
OfPreviousWeek nested class) that invokes WeekRange.ofPreviousWeek(null) and
asserts an IllegalArgumentException is thrown (e.g., using AssertJ's
assertThatThrownBy(() ->
WeekRange.ofPreviousWeek(null)).isInstanceOf(IllegalArgumentException.class));
this verifies the existing Assert.notNull defensive check in the ofPreviousWeek
method behaves as expected.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/MonthlyRankingJobE2ETest.java`:
- Around line 89-188: Add two tests in MonthlyRankingJobE2ETest to verify the MV
tie-break contract (score DESC, product_id ASC): (1) insert three ledgers with
identical score for product_ids 10L, 20L, 30L using insertLedger(...), run job
via jobLauncherTestUtils.launchJob(params(YEAR_MONTH)) and assert
mv_product_rank_monthly ranking_position 1..3 map to product_id 10L, 20L, 30L
respectively; (2) create >100 products where the 100th and 101st have the same
score (use insertLedger for each pid), run the job and assert COUNT(*) = 100 and
that between the tied pair the one with the smaller product_id is included at
ranking_position 100 and the other is excluded, using jdbcTemplate queries
against mv_product_rank_monthly as in existing tests.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/step/WeeklyStagingUpsertWriterTest.java`:
- Around line 14-67: The tests lack negative-path coverage for invalid
StagingDelta inputs; update the contract for
WeeklyStagingUpsertWriter.aggregateByProductId to reject invalid inputs (throw
IllegalArgumentException for null productId and for delta values that are NaN or
infinite), then add unit tests in WeeklyStagingUpsertWriterTest that assert
aggregateByProductId(List.of(new StagingDelta(null, 1.0))) throws, and that
aggregateByProductId(List.of(new StagingDelta(1L, Double.NaN)), and similarly
for Double.POSITIVE_INFINITY/NEGATIVE_INFINITY, also keep existing positive
tests unchanged to ensure normal behavior; use the symbols
WeeklyStagingUpsertWriter.aggregateByProductId, StagingDelta, productId, and
delta to locate code to change.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidatorTest.java`:
- Around line 36-54: Add a parameterized test in
MonthlyJobParametersValidatorTest that asserts validator.validate(...) throws
JobParametersInvalidException for blank year_month inputs; create a
`@ParameterizedTest` with `@ValueSource`(strings = {"", " "}) and for each value
build JobParameters via new JobParametersBuilder().addString("year_month",
value).toJobParameters() and assertThatThrownBy(() ->
validator.validate(params)).isInstanceOf(JobParametersInvalidException.class) to
cover empty and whitespace-only boundary cases.
In
`@apps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/WeeklyJobParametersValidatorTest.java`:
- Around line 25-34: Add a new unit test to cover the validator's isBlank()
branch: in WeeklyJobParametersValidatorTest create a test (e.g.,
throwsWhenBlank) that builds JobParameters with addString("year_week", " ")
(and/or "") and asserts that validator.validate(params) throws
JobParametersInvalidException with a message containing "year_week"; this
complements the existing throwsWhenMissing test and ensures the validator's
blank-string path is exercised.
In `@sql/V12__create_mv_product_rank_monthly.sql`:
- Line 8: 현재 인덱스 KEY idx_mv_product_rank_monthly_position (year_month_key,
ranking_position)은 조회용 인덱스일 뿐 무결성을 보장하지 않으니, 복수의 레코드가 같은 year_month_key와
ranking_position으로 들어오는 것을 막기 위해 인덱스 정의를 UNIQUE (year_month_key,
ranking_position)로 변경하고 MV 적재 로직/스크립트에서 중복 순위 삽입 시 제약 위반이 발생하는지를 검증하는 테스트 케이스를
추가하세요; 관련 식별자는 idx_mv_product_rank_monthly_position, year_month_key,
ranking_position 및 월간 MV 적재 테스트(또는 해당 로더 함수)를 참고해 수정 및 테스트를 구현하면 됩니다.
🪄 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: f969a616-d9c3-499f-af18-603a5cdd0d07
📒 Files selected for processing (56)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryService.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MonthlyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/PeriodRankingRepository.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/MonthlyRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/RankingQueryServiceImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/schema/MonthlyRankingMvSchema.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/schema/WeeklyRankingMvSchema.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/test/java/com/loopers/domain/ranking/FakeMonthlyRankingRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/ranking/FakePeriodRankingRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/ranking/FakeRankingRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/ranking/FakeWeeklyRankingRepository.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/ranking/RankingQueryServiceImplTest.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/MonthRange.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/WeekRange.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/MonthlyRankingMvSchema.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/MonthlyRankingStagingSchema.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/RankingScoreLedgerSchema.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/WeeklyRankingMvSchema.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/schema/WeeklyRankingStagingSchema.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/cleanup/StagingCleanupJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/cleanup/step/StagingCleanupTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/dto/DailyLedgerRow.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/dto/StagingDelta.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/DeleteMonthlyStagingTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/DeleteWeeklyStagingTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/LedgerToStagingProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyLedgerReaderConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyStagingToMvTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyStagingUpsertWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyLedgerReaderConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyStagingToMvTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyStagingUpsertWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidator.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/validator/WeeklyJobParametersValidator.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.javaapps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/MonthRangeTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/domain/ranking/WeekRangeTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/cleanup/StagingCleanupJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/cleanup/step/StagingCleanupTaskletTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/WeeklyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/step/WeeklyStagingUpsertWriterTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/MonthlyJobParametersValidatorTest.javaapps/commerce-batch/src/test/java/com/loopers/batch/job/ranking/validator/WeeklyJobParametersValidatorTest.javasql/V10__create_mv_product_rank_weekly.sqlsql/V11__create_mv_product_rank_weekly_staging.sqlsql/V12__create_mv_product_rank_monthly.sqlsql/V13__create_mv_product_rank_monthly_staging.sql
Comment on lines
+18
to
+23
| * 현재 프로젝트는 {@code ddl-auto} 로 DDL 을 관리한다. {@code sql/V12} 는 Flyway 이관 시 참조 스키마. | ||
| * | ||
| * <p><b>WARNING</b>: commerce-batch 쪽 Entity, 이 Entity, 참조 SQL 세 곳에서 같은 테이블을 각자 정의한다. | ||
| * {@link WeeklyRankingMvSchema} 의 WARNING 참조. {@code year_month_key} 네이밍 근거는 | ||
| * commerce-batch 쪽 {@code MonthlyRankingMvSchema} 주석 참조. | ||
| */ |
There was a problem hiding this comment.
스키마 관리 설명 불일치로 모듈 간 DDL 동기화 절차가 흔들릴 수 있다.
운영에서 문서 기준이 분산되면 월간 랭킹 테이블 변경 시 배치/API 간 반영 누락 가능성이 커진다.
수정안은 현재 저장소의 실제 스키마 관리 체계를 기준으로 주석을 정리하고, 참조 SQL과 양 모듈 엔티티 동시 수정 규칙을 더 명확히 적는 것이다.
추가 테스트는 월간 MV 엔티티 정의와 기준 SQL 정의 간 동등성 검증(컬럼 타입/길이/PK/인덱스) 자동화를 권장한다.
Based on learnings: In the Loopers-dev-lab/loop-pack-be-l2-vol3-java project, database index creation (including JPA Index annotations) is NOT managed by JPA ddl-auto and is handled via an internal separate DB migration pipeline.
🤖 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/schema/MonthlyRankingMvSchema.java`
around lines 18 - 23, Update the class-level javadoc in MonthlyRankingMvSchema
to accurately reflect the repository's true schema management: state that DDL
(including indexes and JPA `@Index` annotations) is NOT applied by JPA ddl-auto
but by the internal DB migration pipeline, remove/clarify the old Flyway/sql/V12
wording, and add a clear WARNING (mirroring WeeklyRankingMvSchema) instructing
maintainers to update the migration SQL, the commerce-batch Entity, and this API
Entity in lockstep when changing the monthly MV schema (mention the
year_month_key naming rationale and reference WeeklyRankingMvSchema). Also add a
short note recommending automated equivalence checks (columns, types/lengths,
PKs, indexes) between the Entity and the reference SQL to prevent drift.
Comment on lines
+20
to
+22
| * <p>현재 프로젝트는 Flyway 를 쓰지 않고 {@code ddl-auto} 로 DDL 을 관리한다. | ||
| * {@code sql/V10__create_mv_product_rank_weekly.sql} 은 Flyway 이관 시 참조 스키마로 준비된 파일. | ||
| * |
There was a problem hiding this comment.
DDL 관리 방식 설명이 저장소 운영 정책과 충돌한다.
Line 20-22 설명은 운영자가 ddl-auto를 스키마 소스로 오인하게 만들어 배포 시 스키마 드리프트 대응 판단을 잘못하게 할 수 있다.
수정안으로 JavaDoc에서 “외부 마이그레이션 파이프라인이 소스 오브 트루스이고, 이 Entity는 읽기/매핑 목적”임을 명확히 적는 것이 필요하다.
추가 테스트로는 프로파일 기반 설정 검증 테스트를 두어 운영 프로파일에서 spring.jpa.hibernate.ddl-auto가 스키마 생성 모드가 아님을 고정하는 것이 좋다.
제안 코드
- * <p>현재 프로젝트는 Flyway 를 쓰지 않고 {`@code` ddl-auto} 로 DDL 을 관리한다.
- * {`@code` sql/V10__create_mv_product_rank_weekly.sql} 은 Flyway 이관 시 참조 스키마로 준비된 파일.
+ * <p>현재 프로젝트의 스키마 변경은 별도 DB 마이그레이션 파이프라인에서 관리한다.
+ * 이 Entity 는 런타임 매핑 일관성을 위한 선언이며, 스키마 관리의 단일 기준은 외부 마이그레이션이다.Based on learnings "In the Loopers-dev-lab/loop-pack-be-l2-vol3-java project, database index creation ... is handled via an internal, separate DB migration pipeline ...".
📝 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
| * <p>현재 프로젝트는 Flyway 를 쓰지 않고 {@code ddl-auto} 로 DDL 을 관리한다. | |
| * {@code sql/V10__create_mv_product_rank_weekly.sql} 은 Flyway 이관 시 참조 스키마로 준비된 파일. | |
| * | |
| * <p>현재 프로젝트의 스키마 변경은 별도 DB 마이그레이션 파이프라인에서 관리한다. | |
| * 이 Entity 는 런타임 매핑 일관성을 위한 선언이며, 스키마 관리의 단일 기준은 외부 마이그레이션이다. | |
| * |
🤖 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/schema/WeeklyRankingMvSchema.java`
around lines 20 - 22, Update the JavaDoc on WeeklyRankingMvSchema to explicitly
state that an external migration pipeline (not ddl-auto) is the source of truth
and that this class/entity is read-only / used only for mapping the existing
materialized view (e.g., reference sql/V10__create_mv_product_rank_weekly.sql as
a prepared migration file), clarifying not to rely on
spring.jpa.hibernate.ddl-auto for schema changes; then add a profile-based test
(e.g., a new test class like ProductionProfileSchemaConfigTest) that loads the
production/operational Spring profile and asserts spring.jpa.hibernate.ddl-auto
is not set to a schema-creating value (validate it is none/validate/update
disabled) to prevent accidental schema generation in deployment.
Comment on lines
+12
to
18
| import jakarta.validation.constraints.Max; | ||
| import jakarta.validation.constraints.Min; | ||
| import jakarta.validation.constraints.Pattern; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.validation.annotation.Validated; | ||
| import org.springframework.web.bind.annotation.GetMapping; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether commerce-api already normalizes validation/binding exceptions
# and inspect the current controller/advice wiring.
fd -i 'ApiControllerAdvice.java' apps/commerce-api/src/main/java -x sed -n '1,260p' {}
echo
rg -n --type=java 'ConstraintViolationException|MethodArgumentTypeMismatchException|MethodArgumentNotValidException' apps/commerce-api/src/main/java
echo
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 12456
컨트롤러에서 Bean Validation을 제거하고 도메인 레이어/수동 검증을 통해 CoreException 경로로 통일한다.
현재 구현은 @Validated, @Pattern, @Min, @Max, 그리고 RankingPeriod enum 바인딩을 사용하여 잘못된 입력을 HandlerMethodValidationException, MethodArgumentTypeMismatchException으로 처리하고 있다. 이 프로젝트는 Bean Validation 대신 도메인 레이어 또는 컨트롤러 내부 수동 검증을 통해 모든 에러를 CoreException 경로로 흘려보내는 것을 원칙으로 한다. 현재 패턴은 이 원칙을 위반한다.
date/period/page/size 검증을 모두 컨트롤러 메서드 내부에서 수행하고, 실패 시 CoreException으로 변환하도록 수정한다. 예를 들어 parseDate() 메서드와 유사하게 parsePeriod(), validatePage(), validateSize()를 구현한 후, 검증 실패 시 적절한 ErrorType.BAD_REQUEST로 CoreException을 발생시킨다. @Validated 어노테이션과 모든 검증 제약 어노테이션을 제거한다.
추가로 period=INVALID, page=-1, size=101, date=20260230 등의 케이스가 모두 동일한 CoreException → ApiControllerAdvice 경로를 통해 일관된 에러 응답이 내려가는 통합 테스트를 추가한다.
🤖 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/RankingV1Controller.java`
around lines 12 - 18, The controller currently uses Bean Validation annotations
and automatic enum binding which causes framework exceptions; remove the
`@Validated` annotation and all Jakarta constraint annotations and stop relying on
automatic RankingPeriod binding in RankingV1Controller, then implement
controller-side validation methods parsePeriod(String), validatePage(int),
validateSize(int) (reusing the existing parseDate(String) pattern) to
parse/validate period, page and size and throw CoreException with
ErrorType.BAD_REQUEST on any invalid input; ensure the controller calls
parseDate(), parsePeriod(), validatePage(), validateSize() before processing and
let ApiControllerAdvice handle CoreException, and add integration tests that
assert invalid inputs (period=INVALID, page=-1, size=101, date=20260230) all
produce the unified CoreException → ApiControllerAdvice error response.
Comment on lines
+35
to
+37
| int startIndex = (int) offset; | ||
| int endExclusive = Math.min(startIndex + limit, sorted.size()); | ||
| for (int i = startIndex; i < endExclusive; i++) { |
There was a problem hiding this comment.
offset 형변환 경계 처리가 없어 테스트 신뢰성이 깨질 수 있다.
Line 35-37에서 long을 int로 바로 캐스팅하면 음수/초대형 입력 시 인덱스 예외나 왜곡된 페이지 계산이 발생한다. 테스트 더블이 여기서 깨지면 서비스 로직 회귀와 구분이 어려워 운영 장애 원인 분석 비용이 커진다.
수정안으로 offset < 0, limit < 0을 먼저 차단하고, offset >= sorted.size()는 빈 결과를 반환하며, 변환은 Math.toIntExact(offset)로 안전 처리하는 것이 좋다.
추가 테스트로 offset=-1, offset=Long.MAX_VALUE, limit=-1, offset==size 케이스를 포함하는 것을 권장한다.
제안 코드
public List<ProductRanking> findTopN(String periodKey, long offset, int limit) {
if (forcedFailure != null) {
throw forcedFailure;
}
+ if (offset < 0 || limit < 0) {
+ throw new IllegalArgumentException("offset/limit must be non-negative");
+ }
Map<Long, Double> scores = store.getOrDefault(periodKey, Map.of());
List<Map.Entry<Long, Double>> sorted = scores.entrySet().stream()
.sorted(Map.Entry.<Long, Double>comparingByValue(Comparator.reverseOrder())
.thenComparing(Map.Entry::getKey))
.toList();
+ if (offset >= sorted.size() || limit == 0) {
+ return List.of();
+ }
List<ProductRanking> result = new ArrayList<>();
- int startIndex = (int) offset;
- int endExclusive = Math.min(startIndex + limit, sorted.size());
+ int startIndex = Math.toIntExact(offset);
+ int endExclusive = (int) Math.min((long) startIndex + limit, sorted.size());
for (int i = startIndex; i < endExclusive; i++) {
Map.Entry<Long, Double> entry = sorted.get(i);
result.add(new ProductRanking(entry.getKey(), entry.getValue(), i + 1L));
}
return result;
}🤖 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/domain/ranking/FakePeriodRankingRepository.java`
around lines 35 - 37, In FakePeriodRankingRepository, avoid directly casting
offset to int; validate that offset and limit are non-negative, return an empty
result when offset >= sorted.size(), and perform the conversion with
Math.toIntExact(offset) to guard against overflow. Concretely, in the method
using variables offset, limit, startIndex, endExclusive and sorted.size(), first
check if (offset < 0 || limit < 0) and handle as invalid (e.g., throw or return
empty per test semantics), then if (offset >= sorted.size()) return an empty
list, otherwise compute int startIndex = Math.toIntExact(offset) and int
endExclusive = Math.min(startIndex + Math.toIntExact(limit), sorted.size())
before iterating.
Comment on lines
+17
to
+19
| * <p>현재 앱의 모든 환경은 Hibernate {@code ddl-auto} 로 스키마를 관리한다. | ||
| * {@code sql/V12__create_mv_product_rank_monthly.sql} 은 Flyway 이관 시 사용할 참조 스키마. | ||
| * |
There was a problem hiding this comment.
스키마 관리 방식 주석이 실제 운영 정책과 상충한다.
운영 관점에서 주석이 잘못되면 배포/장애 대응 시 ddl-auto 의존으로 오판할 수 있어 위험하다. 이 구간은 “외부 DB migration pipeline(SQL 스크립트) 기반 관리”로 정정하고, 배치 모듈에서 ddl-auto를 비활성화한 상태로도 마이그레이션 SQL만 적용하면 테이블/인덱스가 생성되는지 확인하는 통합 테스트를 추가하는 편이 안전하다.
주석 정정 제안
- * <p>현재 앱의 모든 환경은 Hibernate {`@code` ddl-auto} 로 스키마를 관리한다.
- * {`@code` sql/V12__create_mv_product_rank_monthly.sql} 은 Flyway 이관 시 사용할 참조 스키마.
+ * <p>이 프로젝트의 스키마 변경은 내부 DB migration pipeline(SQL 스크립트)으로 관리한다.
+ * {`@code` sql/V12__create_mv_product_rank_monthly.sql} 은 해당 운영 스키마의 기준 정의다.Based on learnings: In the Loopers-dev-lab/loop-pack-be-l2-vol3-java project, database index creation is managed via an internal external DB migration pipeline, not JPA ddl-auto.
📝 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
| * <p>현재 앱의 모든 환경은 Hibernate {@code ddl-auto} 로 스키마를 관리한다. | |
| * {@code sql/V12__create_mv_product_rank_monthly.sql} 은 Flyway 이관 시 사용할 참조 스키마. | |
| * | |
| * <p>이 프로젝트의 스키마 변경은 내부 DB migration pipeline(SQL 스크립트)으로 관리한다. | |
| * {`@code` sql/V12__create_mv_product_rank_monthly.sql} 은 해당 운영 스키마의 기준 정의다. | |
| * |
🤖 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/infrastructure/schema/MonthlyRankingMvSchema.java`
around lines 17 - 19, The file MonthlyRankingMvSchema.java contains a misleading
comment stating the app manages schema via Hibernate ddl-auto; update the header
comment to state the schema is managed by the external DB migration pipeline
(referencing sql/V12__create_mv_product_rank_monthly.sql as the Flyway
reference) and remove any implication that JPA ddl-auto is used; additionally,
ensure the batch module configuration disables ddl-auto (check application
properties / DataSource config used by MonthlyRankingMvSchema) and add an
integration test that boots the batch module with ddl-auto disabled and runs the
migration SQL to verify the materialized view, tables and indexes (the V12
script) are created successfully.
Comment on lines
+16
to
+18
| * <p>현재 앱의 모든 환경은 Hibernate {@code ddl-auto} 로 스키마를 관리한다. | ||
| * {@code sql/V11__create_mv_product_rank_weekly_staging.sql} 은 Flyway 이관 시 사용할 참조 스키마. | ||
| * |
There was a problem hiding this comment.
스키마 관리 주체 설명이 현재 저장소 운영 방식과 어긋나 혼선을 유발한다.
운영 관점에서 DDL 소유권 문서가 실제와 다르면 변경 시점에 잘못된 배포 절차를 따라 스키마 드리프트가 발생할 수 있다.
수정안은 해당 문구를 “내부 DB 마이그레이션 파이프라인 기준”으로 정정하고, ddl-auto를 단일 진실 공급원처럼 표현하지 않도록 바꾸는 것이다.
추가 테스트는 배치 모듈 스키마 엔티티와 기준 SQL 간 컬럼/PK/인덱스 일치 여부를 검증하는 스모크 테스트(메타데이터 비교)로 보완하면 된다.
Based on learnings: In the Loopers-dev-lab/loop-pack-be-l2-vol3-java project, database index creation (including JPA Index annotations) is NOT managed by JPA ddl-auto and is handled via an internal separate DB migration pipeline.
🤖 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/infrastructure/schema/WeeklyRankingStagingSchema.java`
around lines 16 - 18, Update the comment in WeeklyRankingStagingSchema to stop
presenting Hibernate's ddl-auto as the authoritative schema owner: reword the
lines referencing "Hibernate {`@code` ddl-auto}" and the SQL file {`@code`
sql/V11__create_mv_product_rank_weekly_staging.sql} to state that the SQL is a
reference used by the project's internal DB migration pipeline (not managed by
JPA ddl-auto) and avoid implying JPA creates indexes/PKs; additionally, add a
note recommending a smoke test that programmatically compares the batch module
entity metadata (entities referenced by the WeeklyRankingStagingSchema) against
the reference SQL (sql/V11__create_mv_product_rank_weekly_staging.sql) to
validate columns, primary keys, and indexes.
Comment on lines
+52
to
+55
| public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) { | ||
| int deleted = jdbcTemplate.update(DELETE_SQL, yearMonth); | ||
| int inserted = jdbcTemplate.update(INSERT_SQL, yearMonth, yearMonth); | ||
| log.info("[MonthlyRankingJob] promoted staging to MV. yearMonth={}, deleted={}, inserted={}", |
There was a problem hiding this comment.
year_month 검증 누락으로 무음 실패가 발생할 수 있다.
운영에서 잘못된 파라미터가 주입되면 DELETE/INSERT가 0건이어도 Step이 성공 종료되어 월간 MV 미갱신 장애를 늦게 인지하게 된다. execute 시작 시 fail-fast 검증(Assert.hasText, MonthRange.of)을 추가하라. 추가 테스트로 year_month가 blank/형식 오류일 때 예외가 발생하고 JdbcTemplate 호출이 일어나지 않음을 검증하라.
수정 예시
+import com.loopers.batch.domain.ranking.MonthRange;
+import org.springframework.util.Assert;
@@
`@Override`
`@Transactional`(propagation = Propagation.REQUIRED)
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) {
+ Assert.hasText(yearMonth, "year_month must not be blank");
+ MonthRange.of(yearMonth);
+
int deleted = jdbcTemplate.update(DELETE_SQL, yearMonth);
int inserted = jdbcTemplate.update(INSERT_SQL, yearMonth, yearMonth);🤖 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/step/MonthlyStagingToMvTasklet.java`
around lines 52 - 55, Add fail-fast validation at the start of
MonthlyStagingToMvTasklet.execute: assert the incoming yearMonth is non-blank
using Assert.hasText(yearMonth, "...") and validate/parse it via
MonthRange.of(yearMonth) (or equivalent) so invalid or misformatted year_month
immediately throws; only proceed to call jdbcTemplate.update(DELETE_SQL, ...)
and jdbcTemplate.update(INSERT_SQL, ...) after validation succeeds. Also add
unit tests that pass blank and malformed year_month values to execute and assert
that an exception is thrown and that jdbcTemplate.update is never invoked.
Comment on lines
+53
to
+56
| public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) { | ||
| int deleted = jdbcTemplate.update(DELETE_SQL, yearWeek); | ||
| int inserted = jdbcTemplate.update(INSERT_SQL, yearWeek, yearWeek); | ||
| log.info("[WeeklyRankingJob] promoted staging to MV. yearWeek={}, deleted={}, inserted={}", |
There was a problem hiding this comment.
year_week 검증 누락으로 주간 MV 갱신 실패를 성공으로 오인할 수 있다.
운영 중 파라미터 오입력 시 삭제/삽입이 0건으로 끝나도 성공 처리되어 주간 랭킹 공백이 누적될 수 있다. execute 초기에 Assert.hasText(yearWeek, ...)와 WeekRange.of(yearWeek) 검증을 넣어 즉시 실패시키라. 추가 테스트로 blank/형식 오류(2026W15 등) 입력 시 예외 발생과 DB update 미호출을 검증하라.
수정 예시
+import com.loopers.batch.domain.ranking.WeekRange;
+import org.springframework.util.Assert;
@@
`@Override`
`@Transactional`(propagation = Propagation.REQUIRED)
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) {
+ Assert.hasText(yearWeek, "year_week must not be blank");
+ WeekRange.of(yearWeek);
+
int deleted = jdbcTemplate.update(DELETE_SQL, yearWeek);
int inserted = jdbcTemplate.update(INSERT_SQL, yearWeek, yearWeek);🤖 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/step/WeeklyStagingToMvTasklet.java`
around lines 53 - 56, Add input validation at the start of
WeeklyStagingToMvTasklet.execute: assert that yearWeek is non-blank (use
Assert.hasText(yearWeek,...)) and validate its format by calling
WeekRange.of(yearWeek) so invalid or blank values throw immediately and prevent
JDBC updates; update tests for WeeklyStagingToMvTasklet to assert that blank or
malformed inputs (e.g., "2026W15") cause an exception and that
jdbcTemplate.update is not invoked when validation fails.
Comment on lines
+59
to
+63
| static Map<Long, Double> aggregateByProductId(List<? extends StagingDelta> items) { | ||
| Map<Long, Double> aggregated = new LinkedHashMap<>(); | ||
| for (StagingDelta item : items) { | ||
| aggregated.merge(item.productId(), item.delta(), Double::sum); | ||
| } |
There was a problem hiding this comment.
집계 단계 입력 검증 부재로 런타임 NPE가 발생해 배치 재시도 루프를 유발할 수 있다.
운영 관점에서 비정상 레코드가 섞이면 현재 구현은 바인딩 시점에 NPE로 실패해 원인 파악이 늦고, 청크 재시도로 처리량이 급락할 수 있다.
수정안은 집계 단계에서 productId와 delta를 선검증해 명시적 IllegalArgumentException으로 실패시키는 것이다.
추가 테스트는 null productId, NaN/Infinity delta 입력 시 예외 발생과 정상 집계 회귀를 함께 검증해야 한다.
수정 예시
static Map<Long, Double> aggregateByProductId(List<? extends StagingDelta> items) {
Map<Long, Double> aggregated = new LinkedHashMap<>();
for (StagingDelta item : items) {
+ if (item.productId() == null) {
+ throw new IllegalArgumentException("productId must not be null");
+ }
+ if (!Double.isFinite(item.delta())) {
+ throw new IllegalArgumentException("delta must be finite");
+ }
aggregated.merge(item.productId(), item.delta(), Double::sum);
}
return aggregated;
}🤖 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/step/WeeklyStagingUpsertWriter.java`
around lines 59 - 63, The aggregateByProductId method should validate inputs to
avoid NPEs and bad numeric values: in aggregateByProductId(List<? extends
StagingDelta> items) iterate items and for each StagingDelta (referenced by
item.productId() and item.delta()) assert productId != null and delta is finite
(not null, not NaN, not Infinite); if a check fails, throw an explicit
IllegalArgumentException with a clear message indicating which field and value
caused the failure. Update unit tests to cover null productId, NaN and Infinite
delta cases (expect IllegalArgumentException) and a normal case to verify
correct aggregation.
| score DOUBLE NOT NULL, | ||
| created_at DATETIME(6) NOT NULL, | ||
| PRIMARY KEY (year_week, product_id), | ||
| KEY idx_mv_product_rank_weekly_position (year_week, ranking_position) |
There was a problem hiding this comment.
주차별 ranking_position 유일성 제약이 없어 페이징 결과가 불안정해질 수 있다
Line 8이 일반 인덱스만 두고 있어 동일 year_week에 중복 순위가 들어와도 DB가 차단하지 못한다. 운영 시 재처리/버그 상황에서 중복 순위가 누적되면 조회 정렬의 결정성이 깨지고 페이지 결과가 흔들릴 수 있다.
수정안은 (year_week, ranking_position)를 UNIQUE KEY로 승격해 데이터 무결성을 DB 레벨에서 강제하는 것이다.
추가 테스트로 동일 year_week + 동일 ranking_position 2건 삽입 시 duplicate key 오류가 발생하는 통합 테스트를 넣어 회귀를 막아야 한다.
🐛 제안 diff
- KEY idx_mv_product_rank_weekly_position (year_week, ranking_position)
+ UNIQUE KEY uq_mv_product_rank_weekly_position (year_week, ranking_position)📝 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
| KEY idx_mv_product_rank_weekly_position (year_week, ranking_position) | |
| UNIQUE KEY uq_mv_product_rank_weekly_position (year_week, ranking_position) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sql/V10__create_mv_product_rank_weekly.sql` at line 8, Change the non-unique
index definition for idx_mv_product_rank_weekly_position into a UNIQUE KEY to
enforce DB-level uniqueness on (year_week, ranking_position) and prevent
duplicate ranks: update the table definition in
V10__create_mv_product_rank_weekly.sql replacing KEY
idx_mv_product_rank_weekly_position (year_week, ranking_position) with UNIQUE
KEY idx_mv_product_rank_weekly_position (year_week, ranking_position). Also add
an integration test that attempts to insert two rows with the same year_week and
ranking_position against the materialized view/table and asserts that the
operation fails with a duplicate key (unique constraint) error to prevent
regressions.
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.
Round 10: Spring Batch 기반 주간/월간 랭킹 집계 + API period 확장
📌 Summary
ranking_score_ledger→ Redis ZSET 파이프라인으로 실시간성에 적합하지만, 주간/월간 랭킹을 같은 방식으로 유지하면 Redis 메모리 비용 + "정확성 > 실시간성" 요구를 맞추기 어렵습니다. 주 경계에서 늦게 도착하는 이벤트(watermark) 처리도 ZSET 누적 구조에선 모호해집니다.ranking_score_ledger (bucket_type=DAY)를 source of truth 로 두고, Spring Batch 가 주간/월간 MV(mv_product_rank_weekly,mv_product_rank_monthly) 를 생성하도록 한다. API 는period=DAILY|WEEKLY|MONTHLY쿼리 파라미터 하나로 세 가지 기간을 통합 서빙한다.WeeklyRankingJob/MonthlyRankingJob/StagingCleanupJob이 CLI 기반으로 실행되고,/api/v1/rankings?period=...가 DAILY 는 Redis, WEEKLY/MONTHLY 는 MV 를 조회. 멱등성 (Step 0 DELETE → Step 1+=누적) + tie-break 결정성 (ROW_NUMBER 단일 정렬) 이 E2E 로 회귀 가드됨.🧭 Context & Decision
1. 왜
product_metrics가 아닌ranking_score_ledger를 소스로 썼나?Quest 문서는
product_metrics를 소스로 지정했지만, 실제 스키마는product_idUNIQUE + 날짜 컬럼 없음 = 누적(cumulative) 통계 테이블입니다. 시계열 집계(주간·월간)의 source 로 부적합합니다.반면 Round 9 에서 도입한
ranking_score_ledger는(bucket_type, bucket_key, product_id)단위로base_points를 저장하는 일자별 스냅샷이며, 이미 가중치가 적용된 값이라 주간은SUM(base_points) WHERE bucket_type='DAY' AND bucket_key BETWEEN ? AND ?한 줄로 만들어집니다.Quest 의 의도는 "일간 집계 기반 주간/월간" 이라는 데이터 흐름이지
product_metrics테이블 이름 자체가 아니라고 판단하여 ledger 를 소스로 채택했습니다.2. 멀티 Step Chunk-Oriented 구조 + Step 0 DELETE 멱등성
주간 Job 은 세 단계로 나뉩니다.
DELETE FROM mv_product_rank_weekly_staging WHERE year_week = ?— 해당 주차의 이전 실행 흔적을 완전히 제거.JdbcPagingItemReader로 ledger 의 raw row 스트리밍 →WeeklyStagingUpsertWriter가 chunk 내product_id별로 사전 합산 →INSERT ... AS new ON DUPLICATE KEY UPDATE score = score + new.score로 staging 에 누적.@Transactional):DELETE mv WHERE year_week = ?+INSERT ... SELECT ... ROW_NUMBER() OVER (ORDER BY score DESC, product_id ASC)로 TOP-100 을 MV 에 적재.이 구조의 핵심은 Step 0 DELETE 덕분에
+=누적이 항상 0 에서 시작한다는 점입니다. 재실행 시 (a) 실패 후 재시작 (b) 운영자 실수 (c) 의도적 재계산 — 어느 시나리오에서도 결과가 동일하게 수렴합니다. Spring Batch 의 restart 메커니즘에 의존하지 않아 운영자가 복잡한 내부 동작을 이해할 필요 없이 "새run.id로 다시 돌리면 된다" 는 단순한 SOP 로 처리 가능합니다.3. Tie-break 결정성: 단일 서브쿼리 ROW_NUMBER
초기 설계는 아래처럼 window ORDER BY 와 outer ORDER BY + LIMIT 을 함께 썼습니다.
이건 두 ORDER BY 가 정확히 일치해야만
ranking_position 1..100이 성립한다는 숨은 불변식에 기대는 구조입니다. 서브쿼리로 감싸 outer 에서WHERE ranking_position <= 100으로 컷하는 형태로 바꿔 정렬 일관성 위반 가능성을 원천 차단했습니다.동점 시
product_id ASC로 결정성을 확보. E2E 테스트tieBreakByProductIdAsc로 회귀 가드.4. Writer 의 chunk 내 pre-aggregate
Reader 의 sortKey 가
(bucket_key, product_id)여서 같은 상품의 7일치 row 는 서로 다른 chunk 에 분산될 수 있지만, 한 chunk 안에도 동일product_id가 중복 등장할 수 있습니다.ON DUPLICATE KEY UPDATE가 batch 내 동일 PK 를 accumulate 해주는 동작은 Connector/J 버전과rewriteBatchedStatements옵션에 미묘하게 의존하므로, Writer 가Map.merge로 사전 합산한 뒤 batch 에 고유 PK 만 넣는 방식으로 드라이버 의존성을 원천 차단했습니다.5. DAILY graceful degradation vs WEEKLY/MONTHLY fail-loud
RankingQueryServiceImpl의 DAILY 분기는 Round 9 의 Redis graceful degradation (try/catch + 빈 결과 반환) 을 유지하지만, WEEKLY/MONTHLY 분기는 예외를 전파합니다.6. 외부 cron 기반 실행 (
@Scheduled포기)commerce-batch는SpringApplication.exit()기반 CLI 패턴으로 설계되어@Scheduled(앱이 계속 떠 있어야 함) 와 충돌합니다. 초기 설계는@Scheduled였지만 실제 모듈 구조와 맞지 않아 외부 cron / K8s CronJob 으로 선회했습니다.이 선회로 파생된 리스크(타임존 강제, 동시 실행 방지, watermark 의존성)는 앱 레벨 가드를 추가하는 대신
.docs/week10/OPERATIONS.md에 ⚠ 마크 + 필수 체크리스트(K8sconcurrencyPolicy: Forbid,CRON_TZ=Asia/Seoul,RankingLedgerSyncSchedulersync lag 모니터링 SQL)로 명시했습니다.7. 스키마 정의의 3중 진실 공급원 — 의식적 수용
MV/staging 테이블은 (1)
sql/V10~V13참조 스키마, (2)commerce-batch의 schema-only@Entity, (3)commerce-api의 schema-only@Entity세 곳에 정의됩니다. 프로젝트에 Flyway 가 아직 도입되지 않아ddl-auto로 관리 중이고,commerce-batch가commerce-streamer의RankingScoreLedger엔티티를 의존하지 않는 모듈 경계를 유지하려다 보니 중복이 발생했습니다.근본 해결(Flyway 전환)은 프로젝트 전체 ddl-auto 패턴을 바꾸는 범위라 Week 10 외로 미루고, 각 엔티티에 WARNING 주석 +
@link상호참조 로 drift 방지 체크리스트를 코드 상에 명시했습니다.🏗️ Design Overview
변경 범위
commerce-batch에 Job 3개 추가)mv_product_rank_weekly,mv_product_rank_weekly_staging,mv_product_rank_monthly,mv_product_rank_monthly_staging(복합 PK +ranking_position인덱스)/api/v1/rankings?period=DAILY|WEEKLY|MONTHLYweeklyRankingJob,monthlyRankingJob,rankingStagingCleanupJob주요 컴포넌트 책임
WeekRange/MonthRange(VO): ISO 8601 주차 /YearMonth기반 기간 식별자.ranking_score_ledger.bucket_key와yyyyMMdd포맷 공유 계약을 주석으로 명시.Delete{Weekly|Monthly}StagingTasklet: Step 0. 멱등성 리셋.Assert.hasText이중 방어.{Weekly|Monthly}LedgerReaderConfig:JdbcPagingItemReader로 ledger 스트리밍. sortKey(bucket_key, product_id)는 ledger unique index 를 covering.LedgerToStagingProcessor: Reader 결과를StagingDelta(productId, basePoints)로 변환 (공용).{Weekly|Monthly}StagingUpsertWriter:aggregateByProductId로 chunk 내 중복 PK 사전 합산 후ON DUPLICATE KEY UPDATE ... AS newupsert. Monthly 는 Weekly 의 static helper 재사용.{Weekly|Monthly}StagingToMvTasklet: Step 2.@Transactional+ 단일 서브쿼리 ROW_NUMBER 로 TOP-100 적재.{Weekly|Monthly}JobParametersValidator: 기동 시점에year_week/year_month포맷을 검증하여 Spring 컨텍스트 로딩 실패가 아닌 Job 레벨JobParametersInvalidException으로 실패 경로를 통일.StagingCleanupTasklet: 현재 주 포함 최근 4주 / 현재 월 포함 최근 3개월 staging 보존. MV 는 건드리지 않음.LocalDate.now(KST)명시.RankingQueryServiceImpl:switch(period)로 DAILY → Redis, WEEKLY/MONTHLY → JdbcTemplate 기반 MV 조회 분기. 예외 정책 비대칭.🔁 Flow Diagram
Main Flow —
weeklyRankingJobsequenceDiagram autonumber participant Cron as 외부 cron participant App as commerce-batch participant Validator as JobParametersValidator participant Step0 as Step 0 (Tasklet) participant Step1 as Step 1 (Chunk) participant Step2 as Step 2 (Tasklet) participant Ledger as ranking_score_ledger participant Staging as mv_product_rank_weekly_staging participant MV as mv_product_rank_weekly Cron->>App: --job.name=weeklyRankingJob year_week=2026-W15 run.id=$(date +%s%3N) App->>Validator: validate(JobParameters) Validator-->>App: OK (포맷/존재 주차 검증 통과) App->>Step0: execute Step0->>Staging: DELETE WHERE year_week='2026-W15' App->>Step1: execute (chunk size=1000) loop raw ledger 스트리밍 Step1->>Ledger: SELECT product_id, bucket_key, base_points WHERE bucket_type='DAY' AND bucket_key BETWEEN '20260406' AND '20260412' Ledger-->>Step1: page Step1->>Step1: aggregateByProductId(chunk) Step1->>Staging: INSERT ... AS new ON DUPLICATE KEY UPDATE score = score + new.score end App->>Step2: execute (@Transactional) Step2->>MV: DELETE WHERE year_week='2026-W15' Step2->>Staging: SELECT ROW_NUMBER() OVER (ORDER BY score DESC, product_id ASC) ... LIMIT 100 Step2->>MV: INSERT TOP-100 with ranking_position App-->>Cron: exit(0)API Flow
sequenceDiagram autonumber participant Client participant Controller as RankingV1Controller (@Validated) participant Service as RankingQueryService participant Redis participant MV as mv_product_rank_{weekly|monthly} Client->>Controller: GET /api/v1/rankings?period=WEEKLY&date=20260407&page=0&size=20 Controller->>Controller: Bean Validation (@Min/@Max/@Pattern) Controller->>Service: getRanking(WEEKLY, 2026-04-07, 0, 20) alt period == DAILY Service->>Redis: ZREVRANGE ranking:day:20260407 (graceful degradation) else period == WEEKLY / MONTHLY Service->>MV: SELECT ... WHERE year_week='2026-W15' LIMIT 20 OFFSET 0 (fail-loud) end Service-->>Controller: PageResult<ProductRanking> Controller->>Controller: ProductApplicationService.getByIds (N+1 방지) Controller-->>Client: RankingPageResponse✅ Tests
commerce-batch: 63 tests, 0 failuresWeekRangeTest/MonthRangeTest— ISO 경계, 윤년, 부존재 주차/월 거부WeeklyRankingJobE2ETest/MonthlyRankingJobE2ETest— happy path, 재실행 멱등성, 이전 주차 보호, 빈 소스, TOP-100 컷, tie-break 결정성StagingCleanupJobE2ETest/StagingCleanupTaskletTest— 보존 경계, MV 불변성, 연말 경계 문자열 비교Weekly/MonthlyJobParametersValidatorTest— 포맷/존재 주차·월 검증WeeklyStagingUpsertWriterTest— chunk 내 동일 PK 합산 (pre-aggregate)commerce-api: 521 tests, 0 failuresRankingQueryServiceImplTest— DAILY/WEEKLY/MONTHLY 분기 + DAILY graceful + WEEKLY/MONTHLY fail-loud모든 Job 테스트는 Testcontainers MySQL 8.0 기반으로 실제 DB 엔진에서
ON DUPLICATE KEY UPDATE,ROW_NUMBER()같은 MySQL 고유 문법을 검증합니다 (H2 speculation 회피).🚨 Known Limitations (문서화된 취약점)
OPERATIONS.md§2.6 ⚠concurrencyPolicy: Forbid또는flock필수 (인프라 계약)OPERATIONS.md§2.5 ⚠OPERATIONS.md§4JobListener.afterJob()에 webhook 추가DECIMAL전환은 차기 과제