[volume-10] Spring Batch 기반 주간/월간 랭킹 시스템 구현 #399
Open
jsj1215 wants to merge 9 commits intoLoopers-dev-lab:jsj1215from
Open
[volume-10] Spring Batch 기반 주간/월간 랭킹 시스템 구현 #399jsj1215 wants to merge 9 commits intoLoopers-dev-lab:jsj1215from
jsj1215 wants to merge 9 commits intoLoopers-dev-lab:jsj1215from
Conversation
- WeeklyRankingJobConfig, MonthlyRankingJobConfig: JdbcCursorItemReader + Chunk 기반 집계 Job - SQL GROUP BY 집계 + LN(1+x) 가중치 공식을 Reader 단에서 처리해 Chunk 구조에 자연스럽게 적합 - DELETE + INSERT 트랜잭션으로 base_date 단위 원자적 MV 갱신 (슬라이딩 윈도우 방식) - JobListener, ChunkListener: Job 소요 시간 및 Chunk 처리 건수 로깅 추가 - RankingWeightsConfig: view/like/orderAmount 가중치 설정값 바인딩 - application.yml: job.name 파라미터 기반 단일 Job 실행 + 외부 트리거(K8s CronJob/Jenkins) 지원 - WeeklyRankingJobE2ETest, MonthlyRankingJobE2ETest: Testcontainers 기반 E2E 검증 - scripts/run-weekly-ranking.sh, run-monthly-ranking.sh: Jenkins/Cron 트리거 실행 스크립트 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- WeeklyRankingRepository, MonthlyRankingRepository: 도메인 인터페이스 정의 (DIP) - MvProductRankWeekly, MvProductRankMonthly: JPA 엔티티 (복합 PK MvProductRankId) - rank 예약어 충돌 방지를 위해 @column(name = "`rank`") 백틱 인용 적용 - WeeklyRankingRepositoryImpl, MonthlyRankingRepositoryImpl: JPA 기반 구현체 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- WeeklyRankingFacade, MonthlyRankingFacade: MV 테이블 조회 후 RankingAssembler 에 위임 - date null 시 KST 기준 어제 날짜 기본값 (배치 적재 기준일과 일치) - RankingAssembler: entries → 가시성 필터 → RankingPageResult 조립 로직 단일화 - 일간/주간/월간 Facade 의 Shotgun Surgery 방지 - RankingPageResult: effectiveDate + total + items 를 Facade 에서 하나로 묶어 반환 - Controller 가 Facade 내부 메서드를 직접 호출하는 구조 제거 - RankingPageQuery: 세 Controller 공통 파라미터 파싱·보정 로직 단일화 - GET /api/v1/rankings/weekly, /monthly 엔드포인트 추가 (별도 엔드포인트 방식 채택) - RankingAssemblerTest, WeeklyRankingFacadeTest, MonthlyRankingFacadeTest 작성 - WeeklyRankingV1ControllerTest, MonthlyRankingV1ControllerTest, E2E 테스트 작성 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- RankingFacade: RankingPageResult 반환으로 변경, RankingAssembler 주입으로 ProductFacade 직접 의존 제거 - getDailyTotal, today() 등 내부 메서드 외부 노출 제거 - RankingV1Controller: RankingPageQuery.of() 한 줄로 파라미터 파싱 통일 - 주간/월간 Controller 와 동일한 패턴으로 일관성 확보 - RankingFacadeTest: 반환 타입 RankingPageResult 기반으로 검증 방식 수정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (6)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough일일/주간/월간 랭킹 조회 API 및 배치 작업을 추가하였다. 새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as RankingV1Controller<br/>(Weekly/Monthly)
participant Facade as Weekly/Monthly<br/>RankingFacade
participant Repository as Weekly/Monthly<br/>RankingRepository
participant Database as MV Table<br/>(mv_product_rank_weekly/monthly)
participant Assembler as RankingAssembler
participant ProductFacade
Client->>Controller: GET /api/v1/rankings/{weekly|monthly}?date=yyyyMMdd&page=1&size=20
Controller->>Controller: RankingPageQuery.of(dateStr, page, size)
Controller->>Facade: get{Weekly|Monthly}Ranking(date, page, size)
alt date == null
Facade->>Facade: baseDate = Clock.now(KST) - 1 day
else
Facade->>Facade: baseDate = date
end
Facade->>Repository: getTotal(baseDate)
Repository->>Database: COUNT WHERE base_date = ?
Repository-->>Facade: total
Facade->>Repository: getTopN(baseDate, pageOneBased, size)
Repository->>Database: SELECT ... WHERE base_date = ? ORDER BY rank ASC LIMIT offset,size
Repository-->>Facade: entries
Facade->>Assembler: assemble(baseDate, total, entries)
alt entries empty
Assembler-->>Facade: RankingPageResult(baseDate, total, [])
else
Assembler->>ProductFacade: findVisibleByIds(productIds)
ProductFacade-->>Assembler: visibleProductsMap
Assembler->>Assembler: filter & map to RankingItemInfo
Assembler-->>Facade: RankingPageResult(baseDate, total, items)
end
Facade-->>Controller: RankingPageResult
Controller-->>Client: ApiResponse.success(RankingPageResponse)
sequenceDiagram
participant Job as Weekly/Monthly<br/>RankingJob
participant Reader as JdbcCursorItemReader
participant DB as product_metrics_hourly
participant Writer as Weekly/Monthly<br/>RankingItemWriter
participant Repo as JdbcMvProductRankRepository
participant MV as mv_product_rank_weekly/monthly
Job->>Reader: open(targetDate)
loop chunks
Reader->>DB: SELECT product_id, SUM(...) AS score WHERE bucket_hour IN window GROUP BY product_id ORDER BY score DESC LIMIT TOP_N
DB-->>Reader: stream ProductMetricsAggregate
Reader->>Job: chunk(items)
Job->>Writer: write(chunk)
Writer->>Writer: accumulate rows, assign rank 1..N
end
Job->>Writer: afterStep()
Writer->>Repo: replace{Weekly|Monthly}Ranking(baseDate = targetDate - 1, rows)
Repo->>MV: DELETE FROM mv_table WHERE base_date = ?
Repo->>MV: INSERT INTO mv_table (...) VALUES (...)*N
MV-->>Repo: OK
Repo-->>Writer: OK
Job->>Job: 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 |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java (2)
27-27:⚠️ Potential issue | 🟠 MajorstartTime 키 부재 시 조용한 실패(silent failure)가 발생한다.
getLong("startTime")은 키가 없을 때 0L을 반환하므로, beforeJob이 실패하거나 ExecutionContext가 예기치 않게 초기화된 경우 잘못된 duration 계산으로 이어진다. 운영 중 로그 분석 시 혼란을 야기하고, 실제 문제(beforeJob 미실행)를 은폐한다.🛡️ 명시적 검증 로직 추가 제안
`@AfterJob` void afterJob(JobExecution jobExecution) { var startTime = jobExecution.getExecutionContext().getLong("startTime"); + if (startTime == 0L) { + log.warn("Job '{}' startTime이 ExecutionContext에 없음. beforeJob 실행 여부를 확인하라.", + jobExecution.getJobInstance().getJobName()); + return; + } 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` at line 27, In JobListener, don't call jobExecution.getExecutionContext().getLong("startTime") blindly—first verify the key exists (e.g., check jobExecution.getExecutionContext().containsKey("startTime") or retrieve the object and check for null) and if it's missing log an explicit error via your logger (including job id/name) and skip or mark the duration as unknown rather than using 0L; update the code paths around the startTime lookup (the after-job duration calculation) to handle the missing key safely rather than producing a silent/incorrect duration.
43-51:⚠️ Potential issue | 🟠 Major다중 라인 로깅이 운영 모니터링을 저해하고 Job 실행 상태 누락이 장애 대응을 어렵게 한다.
현재 afterJob의 로깅은 여러 줄에 걸쳐 있어 로그 집계 도구(ELK, Splunk 등)에서 각 줄이 별도 엔트리로 처리되며, 파싱과 알림 규칙 설정이 복잡해진다. 또한 Job 실행 결과(BatchStatus, ExitStatus)가 로깅되지 않아 운영자가 성공 여부를 즉시 파악할 수 없다. 실패 원인 파악을 위해서는 Job 상태와 Exit Code가 필수적이다.
♻️ 단일 라인 구조화된 로깅 + Job 상태 추가
- var message = String.format( - """ - *Start Time:* %s - *End Time:* %s - *Total Time:* %d시간 %d분 %d초 - """, startDateTime, endDateTime, hours, minutes, seconds - ).trim(); - - log.info(message); + var status = jobExecution.getStatus(); + var exitCode = jobExecution.getExitStatus().getExitCode(); + + log.info("Job '{}' 완료 - Status: {}, ExitCode: {}, StartTime: {}, EndTime: {}, Duration: {}h {}m {}s", + jobExecution.getJobInstance().getJobName(), + status, + exitCode, + startDateTime, + endDateTime, + hours, + minutes, + seconds);변경 후:
- 단일 라인으로 로그 파싱이 용이하고 집계 도구에서 정확히 처리된다.
- Job 실행 상태를 즉시 확인할 수 있다.
- 로그 기반 알림 설정(예: Status=FAILED 감지)이 가능해진다.
🤖 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 43 - 51, The current multi-line message in JobListener.afterJob makes log aggregation hard and lacks job status; replace the triple-quoted multi-line String.format with a single-line, structured log that includes BatchStatus and ExitStatus along with startDateTime, endDateTime and total time (hours, minutes, seconds) so it prints as one log entry (e.g., key=value or JSON style). Update the code that builds "message" in the afterJob method to produce a single-line string and ensure you log job.getStatus() and job.getExitStatus().getExitCode()/getExitDescription() (or equivalent) so monitoring/alerts can detect FAILED states.
🧹 Nitpick comments (12)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPageResult.java (1)
16-21:items리스트의 방어적 복사를 고려하라.
List<RankingItemInfo> items가 외부에서 전달된 mutable 리스트를 그대로 참조하면, 호출자가 리스트를 수정할 경우 불변성이 깨진다. record의 불변 보장을 완전히 하려면 compact constructor에서List.copyOf(items)로 방어적 복사를 수행하는 것이 안전하다.🛡️ 방어적 복사 예시
public record RankingPageResult( LocalDate effectiveDate, long total, List<RankingItemInfo> items ) { + public RankingPageResult { + items = items == null ? List.of() : List.copyOf(items); + } }🤖 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/RankingPageResult.java` around lines 16 - 21, The record RankingPageResult currently stores the passed-in List<RankingItemInfo> directly; add a compact constructor for RankingPageResult that performs a defensive copy via List.copyOf(items) (and optionally null-check items) so the record's items field is immutable even if the caller passes a mutable list; update the record's constructor to assign this.items = List.copyOf(items) to enforce immutability.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRow.java (1)
16-20:productIdnull 방어 또는 primitive 타입 전환을 검토하라.
productId가Long래퍼 타입으로 선언되어 null이 허용된다. ItemWriter에서 SQL 결과를 기반으로 생성하므로 DB에서 null이 반환될 가능성은 낮으나, 방어적 코딩 관점에서 compact constructor에서 null 체크를 추가하거나 primitivelong으로 변경하는 것이 안전하다. nullproductId가 MV 테이블에 INSERT되면 FK 제약 위반 또는 API 조회 시 NPE가 발생할 수 있다.🛡️ 방어적 생성자 추가 예시
public record MvProductRankRow( Long productId, int rank, double score ) { + public MvProductRankRow { + if (productId == null) { + throw new IllegalArgumentException("productId must not be null"); + } + if (rank <= 0) { + throw new IllegalArgumentException("rank must be positive"); + } + } }🤖 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/MvProductRankRow.java` around lines 16 - 20, The record MvProductRankRow currently uses a nullable Long productId which can lead to NPEs or DB FK violations; update it by either changing the productId field to a primitive long or add a compact constructor in MvProductRankRow that checks Objects.requireNonNull(productId, "productId must not be null") (or throws IllegalArgumentException) to defensively validate values produced by the ItemWriter/SQL result before creating the record.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingPageQuery.java (1)
38-41:formattedDate()에서 nulleffectiveDate처리를 검토하라.
effectiveDate.format(YYYYMMDD)는effectiveDate가 null이면 NPE를 발생시킨다. Facade가 항상 non-nulleffectiveDate를 반환한다면 문제없으나, 방어적 코딩 관점에서 null 체크를 추가하거나 Javadoc에@param제약을 명시하는 것이 안전하다.🛡️ 방어 코드 예시
/** effectiveDate 를 yyyyMMdd 형식 문자열로 변환한다. */ String formattedDate(LocalDate effectiveDate) { + if (effectiveDate == null) { + throw new IllegalArgumentException("effectiveDate must not be null"); + } return effectiveDate.format(YYYYMMDD); }🤖 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/RankingPageQuery.java` around lines 38 - 41, The formattedDate(LocalDate effectiveDate) method can NPE if effectiveDate is null; update formattedDate to defensively handle null (e.g., return null or an empty/default string or throw a clear IllegalArgumentException) or document the non-null contract via Javadoc `@param` on formattedDate to state it must not be null; locate the method formattedDate and adjust the implementation and/or add Javadoc so callers and maintainers (and the YYYYMMDD formatter reference) are clear about the null behavior.apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.java (1)
47-60: replace 메서드가 호출 횟수에 민감하여 운영 설정 변화 시 결과 유실 위험이 있다.Line 49-60은 호출할 때마다 전체 삭제 후 재삽입하므로, Step이 다중 Chunk로 실행되거나 재시도 경로에서 동일
baseDate로 재호출되면 앞선 결과가 지워져 최종 데이터가 왜곡될 수 있다. 운영 관점에서 설정 드리프트 한 번으로 랭킹 품질이 즉시 깨지는 구조다.deleteByBaseDate(baseDate)와insertBatch(...)를 분리해 Step 시작 시 1회 삭제하고, Writer는 Chunk마다 append만 수행하도록 계약을 분리하는 것이 안전하다. 추가 테스트로chunkSize < TOP_N조건의 E2E를 넣어 최종 row 수와 rank 연속성(1..N)을 검증해야 한다.🔧 제안 diff
- public void replaceWeeklyRanking(LocalDate baseDate, List<MvProductRankRow> rows) { - // 해당 base_date 의 기존 랭킹 전체 삭제 후 새 데이터 삽입 - jdbcTemplate.update(DELETE_WEEKLY, baseDate); - insertBatch(INSERT_WEEKLY, baseDate, rows); - } + public void deleteWeeklyRanking(LocalDate baseDate) { + jdbcTemplate.update(DELETE_WEEKLY, baseDate); + } + + public void appendWeeklyRanking(LocalDate baseDate, List<MvProductRankRow> rows) { + insertBatch(INSERT_WEEKLY, baseDate, rows); + } - public void replaceMonthlyRanking(LocalDate baseDate, List<MvProductRankRow> rows) { - jdbcTemplate.update(DELETE_MONTHLY, baseDate); - insertBatch(INSERT_MONTHLY, baseDate, rows); - } + public void deleteMonthlyRanking(LocalDate baseDate) { + jdbcTemplate.update(DELETE_MONTHLY, baseDate); + } + + public void appendMonthlyRanking(LocalDate baseDate, List<MvProductRankRow> rows) { + insertBatch(INSERT_MONTHLY, baseDate, rows); + }🤖 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/infrastructure/ranking/JdbcMvProductRankRepository.java` around lines 47 - 60, The current replaceWeeklyRanking/replaceMonthlyRanking methods delete all rows for baseDate on every call, which causes data loss when steps run in multiple chunks or on retry; extract the deletion into a new method deleteByBaseDate(LocalDate baseDate) (or expose an existing DELETE_WEEKLY/DELETE_MONTHLY path) and remove the jdbcTemplate.update(DELETE_*) call from replaceWeeklyRanking and replaceMonthlyRanking so they only call insertBatch(INSERT_*, baseDate, rows) (i.e., writer becomes append-only); ensure the step orchestration calls deleteByBaseDate once at step start, and add an E2E test for chunkSize < TOP_N verifying final row count and rank continuity 1..N.apps/commerce-api/src/test/java/com/loopers/application/ranking/MonthlyRankingFacadeTest.java (1)
67-80: 기본 날짜 테스트에서getTotal()조회 기준일도 함께 고정하라.현재 테스트는
getTopN()만 검증해서getTotal()이 다른 날짜로 조회되어도 통과한다. 운영에서는 total 메타데이터와 목록의 기준일이 어긋나면 페이지 계산과 응답 일관성이 깨진다.verify(monthlyRankingRepository).getTotal(YESTERDAY)를 추가하고, 날짜 생략 경로에서getTotal()과getTopN()이 같은 기준일을 쓰는지 함께 검증하는 편이 안전하다. 추가로 null-date 경로에서 두 호출이 모두YESTERDAY를 사용하는 테스트를 유지해야 회귀를 막을 수 있다.🤖 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/MonthlyRankingFacadeTest.java` around lines 67 - 80, The test nullDateUsesYesterdayForQuery only verifies monthlyRankingRepository.getTopN but not getTotal, so add verification that monthlyRankingRepository.getTotal(YESTERDAY) is called as well to ensure both list and total use the same effective date; update the test around facade.getMonthlyRanking(null, 1, 20) to verify monthlyRankingRepository.getTotal(YESTERDAY) (and keep the existing getTopN/YESTERDAY check) so null-date code path uses YESTERDAY for both metadata and list retrieval.apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java (1)
47-59: 기본 날짜 테스트에서getTotal()조회 키도 같이 검증하라.현재 테스트는
getTopN()만 확인해서getTotal()이 다른 키를 사용해도 통과한다. 운영에서는 total 값과 실제 목록이 서로 다른 날짜를 가리키면 페이지 메타데이터가 틀어져 장애 분석이 어려워진다.verify(rankingRepository).getTotal("ranking:all:20260409")를 추가해 두 조회가 같은 날짜 키를 쓰는지 고정하는 편이 안전하다. 추가로 날짜 생략 경로에서getTotal()과getTopN()이 동일한 KST 기준 날짜를 사용하는지 검증하는 테스트를 보강해야 한다.🤖 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 47 - 59, Update the nullDateDefaultsToToday test in RankingFacadeTest to also verify that rankingRepository.getTotal is called with the same KST date key as getTopN; specifically, after calling facade.getDailyRanking(null, 1, 20) add a verification like verify(rankingRepository).getTotal(eq("ranking:all:20260409")) so both getTopN(...) and getTotal(...) use the identical key and the test will fail if they diverge (refer to methods getDailyRanking, rankingRepository.getTopN, and rankingRepository.getTotal).apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/MonthlyRankingV1ControllerTest.java (1)
157-161: 에러 응답 테스트가 상태코드만 검증해 계약 회귀를 놓칠 수 있다운영 관점에서 클라이언트는 상태코드뿐 아니라 표준 에러 바디 포맷에도 의존하므로, 현재처럼 400만 검증하면 에러 코드/메시지 포맷 붕괴를 조기에 탐지하기 어렵다.
BAD_REQUEST에 해당하는 공통 에러 필드(코드/메시지/성공 여부 등)까지 함께 검증하도록 확장하는 것을 권장한다. 추가로date=2026-04-11외에 비숫자 문자열(abcd) 케이스도 동일 포맷으로 응답하는지 테스트를 보강하면 안전하다.
As per coding guidelines,**/*Test*.java: 단위/통합 테스트는 실패 케이스와 예외 흐름을 포함해 검증해야 한다.🤖 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/MonthlyRankingV1ControllerTest.java` around lines 157 - 161, Extend the badDateFormat() test to assert the standardized error response body (e.g., check JSON fields like "code", "message", and "success"/"ok") in addition to status 400 by using mockMvc.andExpect(jsonPath(...)) against the controller's error payload; update the existing test that calls mockMvc.perform(get(ENDPOINT).param("date", "2026-04-11")) to assert those fields and add a separate test (e.g., badDateFormat_NonNumeric) that performs get(ENDPOINT).param("date", "abcd") and verifies it returns the same 400 plus identical error body fields so both numeric-format and non-numeric invalid dates are covered.apps/commerce-api/src/test/java/com/loopers/application/ranking/WeeklyRankingFacadeTest.java (1)
69-80: null-date 경로에서total조회 기준일 검증이 빠져 있다운영 관점에서
total과items의 기준일이 어긋나면 페이지 메타데이터가 실제 목록과 불일치해 API 신뢰도가 떨어진다. 현재 테스트는getTopN만 검증하므로getTotal날짜 회귀를 놓칠 수 있다.getTotal(YESTERDAY)호출과rankingAssembler.assemble(YESTERDAY, total, entries)전달값까지 함께 검증하도록 보강하는 것을 권장한다. 추가로 null-date 케이스에서getTotal과getTopN이 동일 기준일을 사용한다는 전용 테스트를 유지하면 회귀 방지에 효과적이다.As per coding guidelines, `**/*Test*.java`: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검해야 한다.보강 예시 diff
void nullDateUsesYesterdayForQuery() { // given + when(weeklyRankingRepository.getTotal(YESTERDAY)).thenReturn(0L); when(weeklyRankingRepository.getTopN(any(), anyInt(), anyInt())).thenReturn(List.of()); when(rankingAssembler.assemble(any(), anyLong(), any())) .thenReturn(new RankingPageResult(YESTERDAY, 0L, List.of())); @@ // then — FIXED clock 은 2026-04-12, 어제 = 2026-04-11 + verify(weeklyRankingRepository).getTotal(YESTERDAY); verify(weeklyRankingRepository).getTopN(YESTERDAY, 1, 20); assertThat(result.effectiveDate()).isEqualTo(YESTERDAY); }🤖 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/WeeklyRankingFacadeTest.java` around lines 69 - 80, The test currently only verifies weeklyRankingRepository.getTopN for the null-date path; stub weeklyRankingRepository.getTotal(...) to return a known total (e.g., 5L) and assert that getTotal was called with the same effective date (YESTERDAY), and also verify rankingAssembler.assemble(...) was invoked with YESTERDAY and that same total value so items and metadata share the same date/total; keep existing verification of getTopN(YESTERDAY, 1, 20) and the asserted result.effectiveDate() as before.apps/commerce-api/src/test/java/com/loopers/interfaces/api/WeeklyRankingV1ApiE2ETest.java (1)
120-142: 가시성 필터링 이후에도 원래 rank가 유지되는지 함께 고정하는 편이 안전하다.현재 시나리오는 hidden 상품 제거 후에도 남은 상품이 기존
rank=2를 유지해야 하는 케이스인데, Line 140-Line 141은productId만 검증한다. 운영에서는 조립 로직이 응답 인덱스로 rank를 다시 계산해도 이 테스트가 통과해서, 저장된 MV rank와 API 응답 rank가 어긋나는 회귀를 놓치게 된다. 남은 item의rank()==2L도 함께 검증하고, 가능하면 deleted 상품 케이스까지 별도 추가해 계약을 고정하는 편이 좋다. As per coding guidelines**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.🤖 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/WeeklyRankingV1ApiE2ETest.java` around lines 120 - 142, The test WeeklyRankingV1ApiE2ETest.visibilityFilter currently only asserts productId after removing a hidden product, which misses verifying the preserved MV rank; update the assertions to also check the remaining item's rank equals 2L (use RankingV1Dto.RankingPageResponse.body.items().get(0).rank()) and add a separate test for a deleted product scenario to assert both productId and its expected rank, using the existing seedWeeklyRanking(...) setup to reproduce the cases.apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java (3)
149-177: 슬라이딩 윈도우 경계값(포함/제외) 테스트가 빠져 오프바이원 회귀 위험이 있다현재는 윈도우 내부/외부만 검증하며, Line 152 주석의 실제 계약인
[targetDate-7일 00:00, targetDate 00:00)경계는 확인하지 않는다. 운영에서는 경계 비교자 하나 차이로 집계 누락/과집계가 발생하므로, 시작 시각 포함과 종료 시각 제외를 명시적으로 테스트해야 한다. 추가 테스트로targetDate.minusDays(7).atStartOfDay()포함,targetDate.atStartOfDay()제외 케이스를 넣는 것이 좋다.추가 테스트 예시
+ `@DisplayName`("슬라이딩 윈도우 경계값: 시작 시각은 포함되고 종료 시각은 제외된다.") + `@Test` + void includesStartBoundaryAndExcludesEndBoundary() throws Exception { + LocalDate targetDate = LocalDate.of(2026, 4, 23); + LocalDateTime startBoundary = targetDate.minusDays(7).atStartOfDay(); // 포함 + LocalDateTime endBoundary = targetDate.atStartOfDay(); // 제외 + + insertMetrics(1L, startBoundary, 0, 0, 0, 1000.0); + insertMetrics(2L, endBoundary, 0, 0, 0, 999999.0); + jobLauncherTestUtils.setJob(job); + + var jobExecution = jobLauncherTestUtils.launchJob(new JobParametersBuilder() + .addLocalDate("targetDate", targetDate) + .toJobParameters()); + + LocalDate baseDate = targetDate.minusDays(1); + List<Map<String, Object>> rows = jdbcTemplate.queryForList( + "SELECT product_id FROM mv_product_rank_weekly WHERE base_date = ?", + baseDate + ); + + assertAll( + () -> assertThat(jobExecution.getExitStatus().getExitCode()) + .isEqualTo(ExitStatus.COMPLETED.getExitCode()), + () -> assertThat(rows.stream().map(r -> ((Number) r.get("product_id")).longValue())) + .contains(1L) + .doesNotContain(2L) + ); + }As per coding guidelines,
**/*Test*.java:단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다규칙에 따라 경계 시각 테스트를 보강해야 한다.🤖 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/weekly/WeeklyRankingJobE2ETest.java` around lines 149 - 177, The test excludesDataOutsideWindow only checks an inside and outside point but misses the exact sliding-window boundaries; add assertions (or a separate test) that explicitly insert metrics at targetDate.minusDays(7).atStartOfDay() and targetDate.atStartOfDay() using insertMetrics (e.g., product ids like 3L and 4L), run the job via jobLauncherTestUtils.launchJob with the same targetDate, then query mv_product_rank_weekly for baseDate and assert the metric at targetDate.minusDays(7).atStartOfDay() is included while the metric at targetDate.atStartOfDay() is excluded (use the same jdbcTemplate query and assert rows contain the expected product id(s) and sizes). Ensure the new assertions reference the existing variables/methods: targetDate, insertMetrics, jobLauncherTestUtils, baseDate, and mv_product_rank_weekly.
244-247: 동적 테이블명 문자열 결합은 테스트 코드에서도 확산되면 위험하다Line 244-247은 테이블명을 SQL 문자열로 결합한다. 현재는 상수 호출이라 즉시 취약점은 아니지만, 패턴이 재사용되면 SQL 인젝션/오타 리스크가 커지고 테스트 유지보수가 어려워진다. 주간 전용 헬퍼로 고정하거나 화이트리스트 검증을 추가하는 편이 안전하다. 추가 테스트로 허용되지 않은 테이블명이 들어오면 예외가 발생하는 케이스를 넣어 재발을 막는 것이 좋다.
변경 제안
- private int countByBaseDate(String table, LocalDate baseDate) { + private int countWeeklyByBaseDate(LocalDate baseDate) { return jdbcTemplate.queryForObject( - "SELECT COUNT(*) FROM `" + table + "` WHERE base_date = ?", + "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE base_date = ?", Integer.class, baseDate ); }🤖 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/weekly/WeeklyRankingJobE2ETest.java` around lines 244 - 247, countByBaseDate currently concatenates the table name into the SQL string which can lead to injection/maintenance issues; change countByBaseDate to validate table against a whitelist of allowed table names (or map an enum of allowed weekly tables) before using it in jdbcTemplate.queryForObject, and throw an IllegalArgumentException for invalid names; update or add tests to assert that allowed table names return counts and that disallowed names raise the expected exception; reference the countByBaseDate method and the jdbcTemplate.queryForObject usage when applying the fix.
227-234: TOP 100 검증이 개수만 확인하여 랭킹 회귀를 놓칠 수 있다Line 227-234는 100건 여부만 확인하므로, 정렬/컷오프 로직이 깨져도 운영에서 잘못된 상품이 노출될 수 있다. 상위 상품 포함, 101위 제외, rank 연속성(1~100)까지 함께 검증하도록 수정하는 것이 안전하다. 추가로 “정확히 100개일 때 전량 포함” 케이스도 별도 테스트로 보강해야 한다.
변경 제안
- int count = jdbcTemplate.queryForObject( - "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE base_date = ?", - Integer.class, baseDate - ); - assertThat(count).isEqualTo(100); + List<Map<String, Object>> rows = jdbcTemplate.queryForList( + "SELECT product_id, `rank` FROM mv_product_rank_weekly WHERE base_date = ? ORDER BY `rank`", + baseDate + ); + assertAll( + () -> assertThat(rows).hasSize(100), + () -> assertThat(((Number) rows.get(0).get("product_id")).longValue()).isEqualTo(1L), + () -> assertThat(rows.stream() + .map(r -> ((Number) r.get("product_id")).longValue())) + .doesNotContain(101L), + () -> assertThat(((Number) rows.get(99).get("rank")).intValue()).isEqualTo(100) + );As per coding guidelines,
**/*Test*.java:단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다를 충족하도록 컷오프 정확성 검증을 추가해야 한다.🤖 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/weekly/WeeklyRankingJobE2ETest.java` around lines 227 - 234, The test currently only asserts the count is 100 for mv_product_rank_weekly; extend it to verify cutoff correctness by querying the actual ranked rows for base_date (use the same baseDate/jdbcTemplate and table mv_product_rank_weekly), assert that ranks present are exactly the integers 1..100 with no gaps (rank continuity), assert that known expected top items (IDs or SKUs you can derive from the test setup) are included within ranks 1..100, and assert that the item that should be rank 101 is not present; additionally add a separate test case that seeds exactly 100 candidates and verifies they are all included and ranked 1..100.
🤖 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/test/java/com/loopers/application/ranking/RankingAssemblerTest.java`:
- Line 103: The test currently only verifies that productFacade.findVisibleByIds
was never called with List.of(), which misses any other unwanted calls; replace
that specific verify in RankingAssemblerTest (the emptyEntries case) with
verifyNoInteractions(productFacade) to assert productFacade was not called at
all, and ensure the emptyEntries test explicitly documents/keeps this regression
check so no arguments to productFacade are invoked during that case.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/MonthlyRankingJobConfig.java`:
- Around line 119-123: The step currently skips DELETE when the reader returns 0
items because write() is never invoked; update MonthlyRankingJobConfig so the
baseDate rows are always cleared: either add a StepExecutionListener.beforeStep
implementation that calls replaceMonthlyRanking(baseDate) (or the underlying DAO
delete) before the step runs, or modify the monthlyRankingItemWriter to perform
the DELETE unconditionally (independent of chunk size) before writing, or
configure the step to invoke write() for empty chunks; reference the StepBuilder
usage, monthlyRankingReader(...), monthlyRankingItemWriter,
replaceMonthlyRanking, and implement or attach a
StepExecutionListener.beforeStep to guarantee pre-step deletion. Ensure you add
the E2E test "existing MV data + rerun same targetDate with 0 metrics -> MV
becomes 0 rows" to validate stale-data removal.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/step/MonthlyRankingItemWriter.java`:
- Around line 37-45: The write() method in MonthlyRankingItemWriter currently
assumes the `@Value-injected` field targetDate is non-null, causing an NPE at
LocalDate baseDate = targetDate.minusDays(1); add an explicit null check at the
start of write(Chunk<? extends ProductMetricsAggregate> chunk) that throws an
IllegalStateException (or IllegalArgumentException) with a clear message like
"Missing job parameter 'targetDate' for MonthlyRankingItemWriter" when
targetDate == null; update any related tests to assert this explicit exception
is thrown when the job is executed without the targetDate parameter.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.java`:
- Around line 47-60: The current WeeklyRankingItemWriter.write() resets rank and
calls mvProductRankRepository.replaceWeeklyRanking(baseDate, rows) on every
chunk, causing earlier chunks to be lost if write() runs multiple times; modify
WeeklyRankingItemWriter to accumulate rows and maintain a persistent rank offset
(e.g., a field like List<MvProductRankRow> accumulatedRows and int nextRank)
updated in write(ProductMetricsAggregate chunk) by appending rows with rank =
nextRank++ rather than resetting, and move the single
replaceWeeklyRanking(baseDate, rows) call to an after-step/close hook (or
implement flush() / `@AfterStep`) so repository replacement happens exactly once
using accumulatedRows; also add a unit/integration test that calls write() twice
(with two chunks) and asserts ranks are contiguous (1..N) and
mvProductRankRepository.replaceWeeklyRanking() is invoked only once.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.java`:
- Around line 121-126: The current flow leaves stale weekly-ranking rows when
the reader returns zero items because write() never runs and deletion only
occurs inside replaceWeeklyRanking; update the job to ensure existing base_date
rows are removed even on empty reads by either adding a beforeStep listener that
deletes rows for the current targetDate (hooking into weeklyRankingStep and
using weeklyRankingReader to obtain the targetDate) or by changing
weeklyRankingItemWriter to run the replace/delete logic when the chunk is empty
(detect empty collection and call replaceWeeklyRanking for the targetDate), and
add an E2E test that covers "existing MV rows + metrics 0 rows + rerun same
targetDate => existing data cleared".
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.java`:
- Around line 11-28: Update the MvProductRankRepository interface Javadoc to
explicitly document the contract for replaceWeeklyRanking and
replaceMonthlyRanking: state that rows may be empty (no inserts will be
performed; DELETE for baseDate still runs) and that neither baseDate nor rows
may be null. Add `@NonNull` annotations (or javax.annotation.Nonnull) to the
method parameters (baseDate and rows) and mention that implementations will
validate and fail fast. Then modify the implementing class(es) that implement
replaceWeeklyRanking/replaceMonthlyRanking to perform explicit null checks on
baseDate and rows (throw IllegalArgumentException or NullPointerException with
clear messages) before proceeding, keeping the existing behavior of skipping
batchUpdate when rows.isEmpty(). Ensure references include
MvProductRankRepository, replaceWeeklyRanking, replaceMonthlyRanking, and
MvProductRankRow.
In `@scripts/run-monthly-ranking.sh`:
- Around line 30-38: The script enables strict errexit (`set -euo pipefail`) so
when the `java -jar ...` command in run-monthly-ranking.sh fails the script
exits before capturing EXIT_CODE and logging; to fix, temporarily disable
errexit (e.g., `set +e`) immediately before running the `java` invocation, run
the java command, capture its exit code into the existing EXIT_CODE variable,
perform the echo log using EXIT_CODE, and then re-enable errexit (`set -e`) if
needed; apply the same pattern to run-weekly-ranking.sh so batch failures always
produce the exit log for monitoring.
In `@scripts/run-weekly-ranking.sh`:
- Line 26: The TARGET_DATE default uses the system date which varies by
container timezone; change the assignment that sets TARGET_DATE to respect a
fixed timezone by reading a BATCH_TZ env var (defaulting to "Asia/Seoul") and
invoking date under that TZ (e.g., use TZ="$BATCH_TZ" when computing the
default) so the script yields KST-based dates regardless of host timezone;
update any related references that assume TARGET_DATE to preserve behavior when
an explicit arg is provided, and add tests that run the script with no args
under TZ=UTC and TZ=Asia/Seoul to assert the computed TARGET_DATE is the same
KST date.
---
Outside diff comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java`:
- Line 27: In JobListener, don't call
jobExecution.getExecutionContext().getLong("startTime") blindly—first verify the
key exists (e.g., check
jobExecution.getExecutionContext().containsKey("startTime") or retrieve the
object and check for null) and if it's missing log an explicit error via your
logger (including job id/name) and skip or mark the duration as unknown rather
than using 0L; update the code paths around the startTime lookup (the after-job
duration calculation) to handle the missing key safely rather than producing a
silent/incorrect duration.
- Around line 43-51: The current multi-line message in JobListener.afterJob
makes log aggregation hard and lacks job status; replace the triple-quoted
multi-line String.format with a single-line, structured log that includes
BatchStatus and ExitStatus along with startDateTime, endDateTime and total time
(hours, minutes, seconds) so it prints as one log entry (e.g., key=value or JSON
style). Update the code that builds "message" in the afterJob method to produce
a single-line string and ensure you log job.getStatus() and
job.getExitStatus().getExitCode()/getExitDescription() (or equivalent) so
monitoring/alerts can detect FAILED states.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPageResult.java`:
- Around line 16-21: The record RankingPageResult currently stores the passed-in
List<RankingItemInfo> directly; add a compact constructor for RankingPageResult
that performs a defensive copy via List.copyOf(items) (and optionally null-check
items) so the record's items field is immutable even if the caller passes a
mutable list; update the record's constructor to assign this.items =
List.copyOf(items) to enforce immutability.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingPageQuery.java`:
- Around line 38-41: The formattedDate(LocalDate effectiveDate) method can NPE
if effectiveDate is null; update formattedDate to defensively handle null (e.g.,
return null or an empty/default string or throw a clear
IllegalArgumentException) or document the non-null contract via Javadoc `@param`
on formattedDate to state it must not be null; locate the method formattedDate
and adjust the implementation and/or add Javadoc so callers and maintainers (and
the YYYYMMDD formatter reference) are clear about the null behavior.
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/MonthlyRankingFacadeTest.java`:
- Around line 67-80: The test nullDateUsesYesterdayForQuery only verifies
monthlyRankingRepository.getTopN but not getTotal, so add verification that
monthlyRankingRepository.getTotal(YESTERDAY) is called as well to ensure both
list and total use the same effective date; update the test around
facade.getMonthlyRanking(null, 1, 20) to verify
monthlyRankingRepository.getTotal(YESTERDAY) (and keep the existing
getTopN/YESTERDAY check) so null-date code path uses YESTERDAY for both metadata
and list retrieval.
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java`:
- Around line 47-59: Update the nullDateDefaultsToToday test in
RankingFacadeTest to also verify that rankingRepository.getTotal is called with
the same KST date key as getTopN; specifically, after calling
facade.getDailyRanking(null, 1, 20) add a verification like
verify(rankingRepository).getTotal(eq("ranking:all:20260409")) so both
getTopN(...) and getTotal(...) use the identical key and the test will fail if
they diverge (refer to methods getDailyRanking, rankingRepository.getTopN, and
rankingRepository.getTotal).
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/WeeklyRankingFacadeTest.java`:
- Around line 69-80: The test currently only verifies
weeklyRankingRepository.getTopN for the null-date path; stub
weeklyRankingRepository.getTotal(...) to return a known total (e.g., 5L) and
assert that getTotal was called with the same effective date (YESTERDAY), and
also verify rankingAssembler.assemble(...) was invoked with YESTERDAY and that
same total value so items and metadata share the same date/total; keep existing
verification of getTopN(YESTERDAY, 1, 20) and the asserted
result.effectiveDate() as before.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/MonthlyRankingV1ControllerTest.java`:
- Around line 157-161: Extend the badDateFormat() test to assert the
standardized error response body (e.g., check JSON fields like "code",
"message", and "success"/"ok") in addition to status 400 by using
mockMvc.andExpect(jsonPath(...)) against the controller's error payload; update
the existing test that calls mockMvc.perform(get(ENDPOINT).param("date",
"2026-04-11")) to assert those fields and add a separate test (e.g.,
badDateFormat_NonNumeric) that performs get(ENDPOINT).param("date", "abcd") and
verifies it returns the same 400 plus identical error body fields so both
numeric-format and non-numeric invalid dates are covered.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/WeeklyRankingV1ApiE2ETest.java`:
- Around line 120-142: The test WeeklyRankingV1ApiE2ETest.visibilityFilter
currently only asserts productId after removing a hidden product, which misses
verifying the preserved MV rank; update the assertions to also check the
remaining item's rank equals 2L (use
RankingV1Dto.RankingPageResponse.body.items().get(0).rank()) and add a separate
test for a deleted product scenario to assert both productId and its expected
rank, using the existing seedWeeklyRanking(...) setup to reproduce the cases.
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRow.java`:
- Around line 16-20: The record MvProductRankRow currently uses a nullable Long
productId which can lead to NPEs or DB FK violations; update it by either
changing the productId field to a primitive long or add a compact constructor in
MvProductRankRow that checks Objects.requireNonNull(productId, "productId must
not be null") (or throws IllegalArgumentException) to defensively validate
values produced by the ItemWriter/SQL result before creating the record.
In
`@apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.java`:
- Around line 47-60: The current replaceWeeklyRanking/replaceMonthlyRanking
methods delete all rows for baseDate on every call, which causes data loss when
steps run in multiple chunks or on retry; extract the deletion into a new method
deleteByBaseDate(LocalDate baseDate) (or expose an existing
DELETE_WEEKLY/DELETE_MONTHLY path) and remove the jdbcTemplate.update(DELETE_*)
call from replaceWeeklyRanking and replaceMonthlyRanking so they only call
insertBatch(INSERT_*, baseDate, rows) (i.e., writer becomes append-only); ensure
the step orchestration calls deleteByBaseDate once at step start, and add an E2E
test for chunkSize < TOP_N verifying final row count and rank continuity 1..N.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java`:
- Around line 149-177: The test excludesDataOutsideWindow only checks an inside
and outside point but misses the exact sliding-window boundaries; add assertions
(or a separate test) that explicitly insert metrics at
targetDate.minusDays(7).atStartOfDay() and targetDate.atStartOfDay() using
insertMetrics (e.g., product ids like 3L and 4L), run the job via
jobLauncherTestUtils.launchJob with the same targetDate, then query
mv_product_rank_weekly for baseDate and assert the metric at
targetDate.minusDays(7).atStartOfDay() is included while the metric at
targetDate.atStartOfDay() is excluded (use the same jdbcTemplate query and
assert rows contain the expected product id(s) and sizes). Ensure the new
assertions reference the existing variables/methods: targetDate, insertMetrics,
jobLauncherTestUtils, baseDate, and mv_product_rank_weekly.
- Around line 244-247: countByBaseDate currently concatenates the table name
into the SQL string which can lead to injection/maintenance issues; change
countByBaseDate to validate table against a whitelist of allowed table names (or
map an enum of allowed weekly tables) before using it in
jdbcTemplate.queryForObject, and throw an IllegalArgumentException for invalid
names; update or add tests to assert that allowed table names return counts and
that disallowed names raise the expected exception; reference the
countByBaseDate method and the jdbcTemplate.queryForObject usage when applying
the fix.
- Around line 227-234: The test currently only asserts the count is 100 for
mv_product_rank_weekly; extend it to verify cutoff correctness by querying the
actual ranked rows for base_date (use the same baseDate/jdbcTemplate and table
mv_product_rank_weekly), assert that ranks present are exactly the integers
1..100 with no gaps (rank continuity), assert that known expected top items (IDs
or SKUs you can derive from the test setup) are included within ranks 1..100,
and assert that the item that should be rank 101 is not present; additionally
add a separate test case that seeds exactly 100 candidates and verifies they are
all included and ranked 1..100.
🪄 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: bdb29600-6ce5-45f9-96dc-c20ddc82db82
📒 Files selected for processing (43)
apps/commerce-api/src/main/java/com/loopers/application/ranking/MonthlyRankingFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingAssembler.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingPageResult.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/WeeklyRankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MonthlyRankingRepository.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/MvProductRankId.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/WeeklyRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/MonthlyRankingV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingPageQuery.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/WeeklyRankingV1Controller.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/MonthlyRankingFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/RankingAssemblerTest.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/WeeklyRankingFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/MonthlyRankingV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/WeeklyRankingV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/MonthlyRankingV1ControllerTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ranking/WeeklyRankingV1ControllerTest.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/step/MonthlyRankingItemWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/ChunkListener.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.javaapps/commerce-batch/src/main/java/com/loopers/config/RankingWeightsConfig.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRow.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/ProductMetricsAggregate.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.javaapps/commerce-batch/src/main/resources/application.ymlapps/commerce-batch/src/test/java/com/loopers/job/monthly/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.javaapps/commerce-batch/src/test/resources/application.ymlscripts/run-monthly-ranking.shscripts/run-weekly-ranking.sh
Comment on lines
+11
to
+28
| public interface MvProductRankRepository { | ||
|
|
||
| /** | ||
| * 주간 MV 테이블을 해당 baseDate 기준으로 교체한다 (DELETE + INSERT). | ||
| * | ||
| * @param baseDate 기준일 (targetDate - 1일) | ||
| * @param rows rank 1 부터 순서대로 정렬된 집계 결과 | ||
| */ | ||
| void replaceWeeklyRanking(LocalDate baseDate, List<MvProductRankRow> rows); | ||
|
|
||
| /** | ||
| * 월간 MV 테이블을 해당 baseDate 기준으로 교체한다 (DELETE + INSERT). | ||
| * | ||
| * @param baseDate 기준일 (targetDate - 1일) | ||
| * @param rows rank 1 부터 순서대로 정렬된 집계 결과 | ||
| */ | ||
| void replaceMonthlyRanking(LocalDate baseDate, List<MvProductRankRow> rows); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# JDBC 구현체에서 빈 리스트 처리 로직 확인
ast-grep --pattern $'void replaceWeeklyRanking($_, $_) {
$$$
}'
ast-grep --pattern $'void replaceMonthlyRanking($_, $_) {
$$$
}'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 1. MvProductRankRepository 구현체 찾기
find . -type f -name "*.java" | xargs grep -l "implements MvProductRankRepository"
# 2. 인터페이스 직접 참조하는 파일 찾기
rg "MvProductRankRepository" --type java -lRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 576
🏁 Script executed:
# 읽기할 파일 크기 확인
wc -l ./apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.java
# 구현체 내용 전체 읽기
cat -n ./apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3607
🏁 Script executed:
# 호출 지점 확인
head -30 ./apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1222
🏁 Script executed:
# 전체 WeeklyRankingItemWriter 내용 확인
wc -l ./apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.java
cat -n ./apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3042
인터페이스 수준의 Javadoc에 파라미터 계약을 명시하고 null 방어 로직을 추가하라.
구현체의 Javadoc(lines 62-68)은 빈 리스트 처리를 명확히 문서화하고 있다: "rows가 비어 있으면 batchUpdate를 호출하지 않아도 되지만 JdbcTemplate이 내부에서 빈 배치를 건너뛰므로 별도 처리가 필요 없다." 그러나 인터페이스 수준의 Javadoc은 이 중요한 계약을 명시하지 않으므로, 인터페이스 사용자는 empty list 시 DELETE 가 실행되는지 불분명하다.
더 중요한 문제는 null 파라미터 처리의 부재다. baseDate 가 null 이면 DELETE WHERE base_date = NULL 은 행을 선택하지 않으며(silent failure), rows 가 null 이면 insertBatch 에서 NullPointerException 이 발생한다. 인터페이스 메서드에 @NonNull 애노테이션을 추가하거나 Javadoc 에 "null 파라미터는 허용하지 않음" 을 명시하고, 구현체에서 명시적 null check 를 추가하여 호출자에게 빠른 피드백을 제공하는 것이 바람직하다.
🤖 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/MvProductRankRepository.java`
around lines 11 - 28, Update the MvProductRankRepository interface Javadoc to
explicitly document the contract for replaceWeeklyRanking and
replaceMonthlyRanking: state that rows may be empty (no inserts will be
performed; DELETE for baseDate still runs) and that neither baseDate nor rows
may be null. Add `@NonNull` annotations (or javax.annotation.Nonnull) to the
method parameters (baseDate and rows) and mention that implementations will
validate and fail fast. Then modify the implementing class(es) that implement
replaceWeeklyRanking/replaceMonthlyRanking to perform explicit null checks on
baseDate and rows (throw IllegalArgumentException or NullPointerException with
clear messages) before proceeding, keeping the existing behavior of skipping
batchUpdate when rows.isEmpty(). Ensure references include
MvProductRankRepository, replaceWeeklyRanking, replaceMonthlyRanking, and
MvProductRankRow.
Comment on lines
+30
to
+38
| java -jar "${JAR_PATH}" \ | ||
| --spring.profiles.active="${SPRING_PROFILE}" \ | ||
| --job.name=monthlyRankingJob \ | ||
| targetDate="${TARGET_DATE}" | ||
|
|
||
| EXIT_CODE=$? | ||
|
|
||
| echo "[$(date '+%Y-%m-%d %H:%M:%S')] monthlyRankingJob finished | exitCode=${EXIT_CODE}" | ||
| exit ${EXIT_CODE} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 주간 랭킹 스크립트에 동일 패턴이 있는지 확인
cat scripts/run-weekly-ranking.sh 2>/dev/null | grep -A5 "java -jar" || echo "파일 없음 또는 패턴 없음"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 235
🏁 Script executed:
cat -n scripts/run-monthly-ranking.shRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1386
🏁 Script executed:
cat -n scripts/run-weekly-ranking.shRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1370
set -e와 종료 코드 캡처 로직이 충돌하므로 배치 실패 시 로그가 출력되지 않는다.
Line 22의 set -euo pipefail이 활성화된 상태에서 Line 30-33의 java 명령이 0이 아닌 종료 코드를 반환하면 스크립트가 즉시 종료된다. 따라서 Line 35(EXIT_CODE=$?)에 도달하지 않으며, Line 37의 종료 로그도 출력되지 않는다. 배치 실패 시 모니터링 로그가 남지 않아 장애 추적이 어렵다.
🔧 수정안
java -jar "${JAR_PATH}" \
--spring.profiles.active="${SPRING_PROFILE}" \
--job.name=monthlyRankingJob \
- targetDate="${TARGET_DATE}"
-
-EXIT_CODE=$?
+ targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$?
echo "[$(date '+%Y-%m-%d %H:%M:%S')] monthlyRankingJob finished | exitCode=${EXIT_CODE}"
exit ${EXIT_CODE}run-weekly-ranking.sh에도 동일한 패턴이 있으므로 함께 수정해야 한다. 수정 후 다음 방법으로 검증한다:
테스트 방안:
- 배치 명령을 의도적으로 실패하도록 테스트하여 종료 로그가 정상 출력되는지 확인한다.
- Jenkins 또는 Cron 환경에서 로그가
/var/log/batch/디렉토리에 정상 기록되는지 확인한다.
📝 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
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --job.name=monthlyRankingJob \ | |
| targetDate="${TARGET_DATE}" | |
| EXIT_CODE=$? | |
| echo "[$(date '+%Y-%m-%d %H:%M:%S')] monthlyRankingJob finished | exitCode=${EXIT_CODE}" | |
| exit ${EXIT_CODE} | |
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --job.name=monthlyRankingJob \ | |
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? | |
| echo "[$(date '+%Y-%m-%d %H:%M:%S')] monthlyRankingJob finished | exitCode=${EXIT_CODE}" | |
| exit ${EXIT_CODE} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/run-monthly-ranking.sh` around lines 30 - 38, The script enables
strict errexit (`set -euo pipefail`) so when the `java -jar ...` command in
run-monthly-ranking.sh fails the script exits before capturing EXIT_CODE and
logging; to fix, temporarily disable errexit (e.g., `set +e`) immediately before
running the `java` invocation, run the java command, capture its exit code into
the existing EXIT_CODE variable, perform the echo log using EXIT_CODE, and then
re-enable errexit (`set -e`) if needed; apply the same pattern to
run-weekly-ranking.sh so batch failures always produce the exit log for
monitoring.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- RankingMvCleanupListener: beforeStep에서 baseDate 행 사전 삭제 - Writer: write() 청크 누적 후 afterStep에서 replaceRanking 1회 호출 - targetDate 누락 시 NPE 대신 명시적 IllegalStateException 발생 - E2E: metrics 0건 재실행 시 stale 데이터 정리 검증 케이스 추가 - E2E: failsWithoutTargetDate에 예외 메시지 포함 검증 추가 - 단위 테스트: write() 2회 호출 시 rank 연속성 및 치환 1회 검증 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 인터페이스 Javadoc에 null 불가 및 빈 리스트 시 DELETE만 실행됨을 명시 - 구현체 4개 메서드에 Objects.requireNonNull 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- set -e 환경에서 java 실패 시 EXIT_CODE 미캡처 문제를 && EXIT_CODE=0 || EXIT_CODE=$? 패턴으로 해결 - BATCH_TZ 환경변수(기본값 Asia/Seoul)로 targetDate 기본값을 KST 기준으로 고정 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Actions performedReview triggered.
|
7 similar comments
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingAssemblerTest.java (3)
36-38: 테스트 시각값을 고정해 플래키 가능성을 줄여야 한다Line [38]의
ZonedDateTime.now()는 현재는 직접 assert하지 않더라도, 향후 전체 객체 비교나 스냅샷 테스트가 추가되면 CI 시간대/실행 시점에 따라 간헐 실패를 유발할 수 있다. 테스트 더블은 고정값을 사용해 결정적으로 유지하는 편이 운영 회귀 탐지에 유리하다. 고정ZonedDateTime상수를 두고stubProduct에서 재사용하도록 수정하는 것을 권장한다. 추가로 happyPath에서createdAt매핑도 한 번 assert해 두면 변환 누락 회귀를 조기에 탐지할 수 있다.제안 diff
class RankingAssemblerTest { private static final LocalDate BASE_DATE = LocalDate.of(2026, 4, 11); + private static final ZonedDateTime FIXED_CREATED_AT = + ZonedDateTime.parse("2026-04-11T00:00:00+09:00[Asia/Seoul]"); @@ private ProductInfo stubProduct(Long id) { return new ProductInfo(id, 1L, "브랜드", "상품" + id, 10000, 9000, 2500, 0, - ProductStatus.ON_SALE, "Y", ZonedDateTime.now()); + ProductStatus.ON_SALE, "Y", FIXED_CREATED_AT); } }🤖 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/RankingAssemblerTest.java` around lines 36 - 38, Replace the non-deterministic ZonedDateTime.now() in the test stub by defining a fixed ZonedDateTime constant (e.g., TEST_CREATED_AT) and use it inside stubProduct (which returns ProductInfo) so createdAt is stable across runs; then update the happyPath test to assert that the assembled object's createdAt matches TEST_CREATED_AT to catch any mapping regressions. Ensure references: stubProduct, ProductInfo, ZonedDateTime.now(), happyPath, and createdAt are updated accordingly.
84-90: 가시성 필터 테스트에 메타데이터 회귀 검증을 추가해야 한다현재 visibilityFilter는 items 축소만 확인한다. 필터링 로직 리팩터링 중
effectiveDate/total이 의도치 않게 변형되면 API 응답 메타데이터가 깨져 페이지네이션/집계 표시가 잘못될 수 있다. 같은 테스트에서effectiveDate == BASE_DATE,total == 3L를 함께 검증하는 것이 안전하다. 추가로 “모든 상품이 비가시인 경우에도 items는 비고 total은 원본 유지” 케이스를 별도 테스트로 넣으면 회귀 방어력이 올라간다.As per coding guidelines,
**/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."🤖 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/RankingAssemblerTest.java` around lines 84 - 90, Update the visibility filter test in RankingAssemblerTest that calls assembler.assemble(BASE_DATE, 3L, entries) to also assert that result.effectiveDate() equals BASE_DATE and result.total() equals 3L to ensure metadata did not regress; additionally add a new unit test (e.g., in RankingAssemblerTest) that constructs entries such that all products are invisible, calls assemble(BASE_DATE, 3L, entries), asserts result.items() is empty while result.total() still equals the original total (3L) and result.effectiveDate() equals BASE_DATE to guard against metadata mutation when items are filtered out.
53-56: 외부 호출 횟수 검증을 넣어 성능 회귀를 조기 탐지해야 한다현재는 반환값 중심 검증만 있어, 향후 구현 변경으로
findVisibleByIds가 중복 호출되어도 테스트가 통과할 수 있다. 운영에서는 불필요한 조회 증가가 지연/부하로 이어질 수 있다. happyPath와 visibilityFilter에서 호출 1회 및 추가 호출 없음(verifyNoMoreInteractions)을 명시해 두는 것을 권장한다. 추가 테스트로 entries 크기가 큰 케이스에서도 동일하게 1회 호출되는지 확인하면 성능 회귀 방지에 도움이 된다.As per coding guidelines,
**/*Test*.java: "통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다."Also applies to: 80-81
🤖 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/RankingAssemblerTest.java` around lines 53 - 56, The tests in RankingAssemblerTest (happyPath and visibilityFilter) only assert return values but do not verify external call counts; add Mockito verifications to ensure productFacade.findVisibleByIds(...) is called exactly once in each test (use verify(productFacade).findVisibleByIds(List.of(1L,2L)) or an appropriate argument matcher) and then call verifyNoMoreInteractions(productFacade) to assert no additional calls; also add an extra test case with a larger entries list that asserts the same single call behavior to catch performance regressions.apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java (1)
273-276: 동적 테이블명 문자열 결합 대신 고정 쿼리 헬퍼로 단순화하는 것이 안전하다.운영 관점에서 현재는 테스트 코드라도 유틸 재사용 시 오용 여지가 생기고, SQL 오류가 런타임까지 지연될 수 있다.
countByBaseDate는 주간 MV 전용 메서드로 고정하거나 enum 화이트리스트로 제한하는 리팩터를 권장한다. 추가 테스트로 고정 헬퍼(countWeeklyByBaseDate)를 사용하는 기존 케이스가 동일하게 통과하는지 확인하면 된다.제안 변경안
- private int countByBaseDate(String table, LocalDate baseDate) { + private int countWeeklyByBaseDate(LocalDate baseDate) { return jdbcTemplate.queryForObject( - "SELECT COUNT(*) FROM `" + table + "` WHERE base_date = ?", + "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE base_date = ?", Integer.class, baseDate ); }🤖 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/weekly/WeeklyRankingJobE2ETest.java` around lines 273 - 276, The method countByBaseDate currently concatenates the table name into a SQL string which is unsafe; replace it with a fixed helper for the weekly MV (e.g., introduce countWeeklyByBaseDate(LocalDate baseDate) that queries a hard-coded weekly table name) or restrict table names via an enum whitelist and a safe switch in the existing method; update tests to call the new countWeeklyByBaseDate and verify behavior remains identical. Ensure you reference the existing countByBaseDate method during the change and remove dynamic table string concatenation so SQL is constructed only with fixed table identifiers or validated enum values.
🤖 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/monthly/MonthlyRankingJobConfig.java`:
- Around line 91-92: The SQL ORDER BY is non-deterministic for ties; update the
query in MonthlyRankingJobConfig (the SQL building that currently uses "ORDER BY
score DESC") to add a deterministic tie-breaker such as "product_id ASC" (i.e.,
"ORDER BY score DESC, product_id ASC") so identical scores always yield a stable
order, and add an E2E test that creates tied-score products hitting the LIMIT
boundary and runs the ranking job twice to assert the returned order is
identical across runs.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/step/MonthlyRankingItemWriter.java`:
- Around line 55-60: Modify afterStep(StepExecution stepExecution) so it only
calls mvProductRankRepository.replaceMonthlyRanking(targetDate.minusDays(1),
accumulated) when stepExecution.getStatus() == BatchStatus.COMPLETED; if not
completed, simply return stepExecution.getExitStatus() without mutating MV.
Update references in MonthlyRankingItemWriter to import/use
org.springframework.batch.core.BatchStatus for the check. Add two tests: a unit
test that sets StepExecution to FAILED and asserts replaceMonthlyRanking is not
invoked, and an E2E test that forces a step failure and verifies existing MV
rows remain unchanged after afterStep runs.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.java`:
- Around line 94-95: The SQL ordering in WeeklyRankingJobConfig currently uses
"ORDER BY score DESC" which yields non-deterministic ties; update the query
generator in WeeklyRankingJobConfig (where the LIMIT and ORDER BY are assembled)
to add a deterministic tie-breaker such as "product_id ASC" (i.e., ORDER BY
score DESC, product_id ASC) so identical scores produce stable ordering, and add
an E2E test that runs the ranking job twice (or replays the same input) to
assert the top-N boundary is identical to catch LIMIT-boundary tie cases.
- Around line 135-136: Protect the call to replaceWeeklyRanking() by checking
the step outcome in the StepExecutionListener.afterStep() implementation: only
invoke replaceWeeklyRanking() when stepExecution.getStatus() ==
BatchStatus.COMPLETED (handle empty-result replace as part of a successful
completion), so intermediate failures (e.g., after the first chunk) do not
overwrite existing MV rows; update the listener wired via
weeklyRankingCleanupListener() and the usage around weeklyRankingItemWriter
accordingly, and add a unit/integration test covering "existing MV present +
forced failure after first chunk => existing rows preserved, replace not
called"; apply the same fix to MonthlyRankingItemWriter's afterStep handling.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java`:
- Around line 153-181: Add an explicit boundary test in WeeklyRankingJobE2ETest
to verify inclusion of targetDate.minusDays(7).atStartOfDay() and exclusion of
targetDate.atStartOfDay(): use insertMetrics(...) to create two products with
timestamps at targetDate.minusDays(7).atStartOfDay() (should be included) and
targetDate.atStartOfDay() (should be excluded), launch the job via
jobLauncherTestUtils.launchJob(new
JobParametersBuilder().addLocalDate("targetDate",
targetDate).toJobParameters()), then query mv_product_rank_weekly with
jdbcTemplate.queryForList(...) for baseDate = targetDate.minusDays(1) and assert
that only the product from the minus-7 start-of-day timestamp is present (use
the existing assert pattern and ExitStatus checks). Ensure you reference the
existing helper insertMetrics, jobLauncherTestUtils, and jdbcTemplate usage so
the new assertions plug into the current test scaffolding.
- Around line 41-45: The test setup only clears business tables in setUp(),
which leaves Spring Batch metadata (BATCH_JOB_INSTANCE, BATCH_JOB_EXECUTION,
BATCH_JOB_EXECUTION_CONTEXT, BATCH_STEP_EXECUTION, etc.) and can cause
JobInstanceAlreadyCompleteException; update setUp() to truncate those batch meta
tables or ensure every launchJob() call includes a unique JobParameter (e.g.,
run.id) to avoid reuse, then strengthen the excludesDataOutsideWindow() test to
explicitly assert boundary timestamps at targetDate -7 days 00:00 and targetDate
00:00 are handled correctly; finally refactor countByBaseDate() to stop
concatenating the table name into SQL directly by using a parameterized helper
or a small utility that safely maps allowed table names to prepared statements
to improve readability and safety.
In `@scripts/run-monthly-ranking.sh`:
- Around line 31-34: The script passes the wrong batch property flag
(--job.name=monthlyRankingJob) so Spring won't see spring.batch.job.name; update
the java invocation in scripts/run-monthly-ranking.sh to pass
--spring.batch.job.name=monthlyRankingJob (keeping other arguments like
"${JAR_PATH}", --spring.profiles.active="${SPRING_PROFILE}" and
targetDate="${TARGET_DATE}" intact) so the application recognizes the job name
reliably across environments and tests.
In `@scripts/run-weekly-ranking.sh`:
- Around line 31-34: The script passes the wrong flag (--job.name) so
WeeklyRankingJobConfig's `@ConditionalOnProperty`(name = "spring.batch.job.name",
...) never matches; change the invocation in the weekly script to use
--spring.batch.job.name=weeklyRankingJob (and apply the same fix in
scripts/run-monthly-ranking.sh for the monthly job), then run a smoke test to
confirm the --spring.batch.job.name property is delivered to the JVM and the
correct Job bean (WeeklyRankingJobConfig) is loaded.
---
Nitpick comments:
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingAssemblerTest.java`:
- Around line 36-38: Replace the non-deterministic ZonedDateTime.now() in the
test stub by defining a fixed ZonedDateTime constant (e.g., TEST_CREATED_AT) and
use it inside stubProduct (which returns ProductInfo) so createdAt is stable
across runs; then update the happyPath test to assert that the assembled
object's createdAt matches TEST_CREATED_AT to catch any mapping regressions.
Ensure references: stubProduct, ProductInfo, ZonedDateTime.now(), happyPath, and
createdAt are updated accordingly.
- Around line 84-90: Update the visibility filter test in RankingAssemblerTest
that calls assembler.assemble(BASE_DATE, 3L, entries) to also assert that
result.effectiveDate() equals BASE_DATE and result.total() equals 3L to ensure
metadata did not regress; additionally add a new unit test (e.g., in
RankingAssemblerTest) that constructs entries such that all products are
invisible, calls assemble(BASE_DATE, 3L, entries), asserts result.items() is
empty while result.total() still equals the original total (3L) and
result.effectiveDate() equals BASE_DATE to guard against metadata mutation when
items are filtered out.
- Around line 53-56: The tests in RankingAssemblerTest (happyPath and
visibilityFilter) only assert return values but do not verify external call
counts; add Mockito verifications to ensure productFacade.findVisibleByIds(...)
is called exactly once in each test (use
verify(productFacade).findVisibleByIds(List.of(1L,2L)) or an appropriate
argument matcher) and then call verifyNoMoreInteractions(productFacade) to
assert no additional calls; also add an extra test case with a larger entries
list that asserts the same single call behavior to catch performance
regressions.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java`:
- Around line 273-276: The method countByBaseDate currently concatenates the
table name into a SQL string which is unsafe; replace it with a fixed helper for
the weekly MV (e.g., introduce countWeeklyByBaseDate(LocalDate baseDate) that
queries a hard-coded weekly table name) or restrict table names via an enum
whitelist and a safe switch in the existing method; update tests to call the new
countWeeklyByBaseDate and verify behavior remains identical. Ensure you
reference the existing countByBaseDate method during the change and remove
dynamic table string concatenation so SQL is constructed only with fixed table
identifiers or validated enum values.
🪄 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: c2cf4ce2-7010-4907-af66-dd108c75adac
📒 Files selected for processing (14)
apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingAssemblerTest.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/step/MonthlyRankingItemWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/RankingMvCleanupListener.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.javaapps/commerce-batch/src/test/java/com/loopers/job/monthly/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/monthly/step/MonthlyRankingItemWriterTest.javaapps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/weekly/step/WeeklyRankingItemWriterTest.javascripts/run-monthly-ranking.shscripts/run-weekly-ranking.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankRepository.java
- apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.java
- apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/JdbcMvProductRankRepository.java
- apps/commerce-batch/src/test/java/com/loopers/job/monthly/MonthlyRankingJobE2ETest.java
Comment on lines
+91
to
+92
| ORDER BY score DESC | ||
| LIMIT %d |
There was a problem hiding this comment.
동점 제품의 정렬 기준이 없어 TOP 100 경계가 실행마다 흔들린다
ORDER BY score DESC만 있으면 동점일 때 DB가 임의 순서를 반환하므로, 같은 데이터로 재집계해도 rank와 page 결과가 바뀔 수 있다. 운영에서는 캐시 비교, 재현, CS 대응이 어려워진다. ORDER BY score DESC, product_id ASC 같은 결정적 tie-breaker를 추가하고, 동일 score 상품이 LIMIT 경계에 걸린 케이스를 두 번 실행해 순서가 고정되는지 검증하는 E2E를 추가하는 편이 좋다.
🔧 제안 diff
- ORDER BY score DESC
+ ORDER BY score DESC, product_id ASC
LIMIT %d🤖 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/monthly/MonthlyRankingJobConfig.java`
around lines 91 - 92, The SQL ORDER BY is non-deterministic for ties; update the
query in MonthlyRankingJobConfig (the SQL building that currently uses "ORDER BY
score DESC") to add a deterministic tie-breaker such as "product_id ASC" (i.e.,
"ORDER BY score DESC, product_id ASC") so identical scores always yield a stable
order, and add an E2E test that creates tied-score products hitting the LIMIT
boundary and runs the ranking job twice to assert the returned order is
identical across runs.
Comment on lines
+55
to
+60
| public ExitStatus afterStep(StepExecution stepExecution) { | ||
| if (targetDate == null) { | ||
| return stepExecution.getExitStatus(); | ||
| } | ||
| mvProductRankRepository.replaceMonthlyRanking(targetDate.minusDays(1), accumulated); | ||
| return stepExecution.getExitStatus(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "MonthlyRankingItemWriter.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 185
🏁 Script executed:
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/monthly/step/MonthlyRankingItemWriter.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2902
🌐 Web query:
Spring Batch StepExecutionListener afterStep lifecycle failed step
💡 Result:
In Spring Batch, the StepExecutionListener's afterStep(StepExecution stepExecution) method is always called after a step's processing logic completes, regardless of whether the step succeeded or failed. This includes failed steps. Official documentation confirms: "Called after execution of the step's processing logic (whether successful or failed)." The method receives the StepExecution object, where you can check stepExecution.getStatus (e.g., FAILED) or stepExecution.getExitStatus to determine the outcome and perform appropriate actions, such as logging failures or modifying the exit status. You can implement it as: public ExitStatus afterStep(StepExecution stepExecution) { if (stepExecution.getStatus == BatchStatus.FAILED) { // Handle failed step } return stepExecution.getExitStatus; // or modify as needed } In recent versions (e.g., 5.x+), the step status is persisted before calling afterStep, so stepExecution reflects the final state (COMPLETED or FAILED). Exceptions thrown in afterStep are logged but do not affect the step execution.
Citations:
- 1: https://docs.spring.io/spring-batch/docs/current/api/org/springframework/batch/core/StepExecutionListener.html
- 2: https://docs.spring.io/spring-batch/reference/step/chunk-oriented-processing/intercepting-execution.html
- 3: https://docs.spring.io/spring-batch/reference/5.2/step/chunk-oriented-processing/intercepting-execution.html
- 4: Incorrect Step status in StepExecutionListener#afterStep spring-projects/spring-batch#4362
실패한 Step에서도 월간 ranking MV를 부분 데이터로 치환한다
Spring Batch의 afterStep()는 Step 성공 여부와 관계없이 항상 호출된다. 현재 구현은 상태를 검사하지 않고 무조건 replaceMonthlyRanking()을 실행하므로, 청크 처리 중 예외가 발생하면 누적된 부분 데이터가 그대로 MV 테이블에 공개된다. 예를 들어 전체 10개 청크 중 5개 청크까지 처리 후 실패 시, 절반의 불완전한 ranking이 데이터베이스에 저장되어 사용자가 오류를 인지하지 못한 채 잘못된 데이터를 조회하게 된다.
stepExecution.getStatus() == BatchStatus.COMPLETED일 때만 replaceMonthlyRanking()을 실행하고, 실패 시 기존 MV 데이터를 유지해야 한다. 추가로 다음 테스트를 작성한다: (1) StepExecution을 FAILED 상태로 설정하고 afterStep() 호출 후 replaceMonthlyRanking()이 실행되지 않음을 단위 테스트로 검증, (2) 기존 MV 데이터가 존재한 상태에서 Step 강제 실패 후에도 기존 rows가 유지되는 것을 E2E 테스트로 확인한다.
수정 제안
+import org.springframework.batch.core.BatchStatus;
import org.springframework.batch.core.ExitStatus;
import org.springframework.batch.core.StepExecution;
import org.springframework.batch.core.StepExecutionListener;
@@
`@Override`
public ExitStatus afterStep(StepExecution stepExecution) {
- if (targetDate == null) {
+ if (targetDate == null || stepExecution.getStatus() != BatchStatus.COMPLETED) {
return stepExecution.getExitStatus();
}
mvProductRankRepository.replaceMonthlyRanking(targetDate.minusDays(1), accumulated);
return stepExecution.getExitStatus();
}🤖 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/monthly/step/MonthlyRankingItemWriter.java`
around lines 55 - 60, Modify afterStep(StepExecution stepExecution) so it only
calls mvProductRankRepository.replaceMonthlyRanking(targetDate.minusDays(1),
accumulated) when stepExecution.getStatus() == BatchStatus.COMPLETED; if not
completed, simply return stepExecution.getExitStatus() without mutating MV.
Update references in MonthlyRankingItemWriter to import/use
org.springframework.batch.core.BatchStatus for the check. Add two tests: a unit
test that sets StepExecution to FAILED and asserts replaceMonthlyRanking is not
invoked, and an E2E test that forces a step failure and verifies existing MV
rows remain unchanged after afterStep runs.
Comment on lines
+94
to
+95
| ORDER BY score DESC | ||
| LIMIT %d |
There was a problem hiding this comment.
동점 제품의 정렬 기준이 없어 TOP 100 경계가 실행마다 흔들린다
ORDER BY score DESC만 있으면 동점일 때 DB가 임의 순서를 반환하므로, 같은 데이터로 재집계해도 rank와 page 결과가 바뀔 수 있다. 운영에서는 캐시 비교, 재현, CS 대응이 어려워진다. ORDER BY score DESC, product_id ASC 같은 결정적 tie-breaker를 추가하고, 동일 score 상품이 LIMIT 경계에 걸린 케이스를 두 번 실행해 순서가 고정되는지 검증하는 E2E를 추가하는 편이 좋다.
🔧 제안 diff
- ORDER BY score DESC
+ ORDER BY score DESC, product_id ASC
LIMIT %d📝 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
| ORDER BY score DESC | |
| LIMIT %d | |
| ORDER BY score DESC, product_id ASC | |
| LIMIT %d |
🤖 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/weekly/WeeklyRankingJobConfig.java`
around lines 94 - 95, The SQL ordering in WeeklyRankingJobConfig currently uses
"ORDER BY score DESC" which yields non-deterministic ties; update the query
generator in WeeklyRankingJobConfig (where the LIMIT and ORDER BY are assembled)
to add a deterministic tie-breaker such as "product_id ASC" (i.e., ORDER BY
score DESC, product_id ASC) so identical scores produce stable ordering, and add
an E2E test that runs the ranking job twice (or replays the same input) to
assert the top-N boundary is identical to catch LIMIT-boundary tie cases.
Comment on lines
+135
to
+136
| .listener(weeklyRankingCleanupListener()) | ||
| .listener(weeklyRankingItemWriter) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "WeeklyRankingItemWriter.java" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 183
🏁 Script executed:
find . -type f -name "WeeklyRankingJobConfig.java" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 177
🏁 Script executed:
find . -type f -name "*WeeklyRanking*Test*.java" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 560
🏁 Script executed:
cat -n ./apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/step/WeeklyRankingItemWriter.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2895
🏁 Script executed:
cat -n ./apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.java | head -150Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6938
🏁 Script executed:
cat -n ./apps/commerce-batch/src/test/java/com/loopers/job/weekly/step/WeeklyRankingItemWriterTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3163
🏁 Script executed:
find . -name "pom.xml" -o -name "build.gradle" | grep -E "(batch|commerce-batch)" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
cat ./apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java 2>/dev/null | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3898
🏁 Script executed:
find . -name "pom.xml" | head -1 | xargs grep -A 5 "spring-batch" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
grep -r "BatchStatus\|afterStep" ./apps/commerce-batch/src --include="*.java" -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6089
afterStep()에서 Step 실패 여부를 확인하고 replaceWeeklyRanking() 호출을 보호해야 한다
현재 코드는 Spring Batch의 StepExecutionListener.afterStep()가 Step 성공/실패와 무관하게 호출되는 특성을 반영하지 않는다. 첫 번째 청크 처리 후 중간 예외가 발생하면 누적된 부분 데이터로 기존 MV 테이블을 덮어쓰게 되어 데이터 손상이 발생한다.
afterStep() 메서드에서 stepExecution.getStatus() == BatchStatus.COMPLETED 조건을 추가하여 성공 시에만 replaceWeeklyRanking()을 호출하고, 빈 결과 처리는 성공 시점의 empty replace로 통일해야 한다. 또한 "기존 MV 존재 + 첫 청크 후 강제 실패 → 기존 rows 유지, replace 미호출" 케이스를 테스트에 추가해야 한다.
참고: MonthlyRankingItemWriter에서도 동일한 취약점이 존재한다.
🤖 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/weekly/WeeklyRankingJobConfig.java`
around lines 135 - 136, Protect the call to replaceWeeklyRanking() by checking
the step outcome in the StepExecutionListener.afterStep() implementation: only
invoke replaceWeeklyRanking() when stepExecution.getStatus() ==
BatchStatus.COMPLETED (handle empty-result replace as part of a successful
completion), so intermediate failures (e.g., after the first chunk) do not
overwrite existing MV rows; update the listener wired via
weeklyRankingCleanupListener() and the usage around weeklyRankingItemWriter
accordingly, and add a unit/integration test covering "existing MV present +
forced failure after first chunk => existing rows preserved, replace not
called"; apply the same fix to MonthlyRankingItemWriter's afterStep handling.
Comment on lines
+25
to
+31
| public void beforeStep(StepExecution stepExecution) { | ||
| LocalDate targetDate = stepExecution.getJobParameters().getLocalDate("targetDate"); | ||
| if (targetDate == null) { | ||
| throw new IllegalStateException("JobParameter 'targetDate' is required"); | ||
| } | ||
| LocalDate baseDate = targetDate.minusDays(1); | ||
| deleteAction.accept(baseDate); |
There was a problem hiding this comment.
Step 시작 전에 MV를 삭제하면 실패 시 마지막 정상 스냅샷까지 사라진다
beforeStep()에서 baseDate를 선삭제하면, 집계 도중 reader/writer 예외가 발생해도 이전 성공 결과가 이미 제거된 상태가 된다. 운영에서는 재처리 전까지 빈 랭킹이나 불완전한 랭킹이 노출되는 장애로 이어진다. 사전 DELETE는 제거하고, 성공한 Step 종료 시점에만 DELETE+INSERT를 한 트랜잭션으로 수행하도록 바꾸는 편이 안전하다. 추가 테스트로 "기존 MV 존재 + Step 강제 실패 + 동일 baseDate 조회 시 이전 rows 유지" E2E 케이스를 넣는 것이 좋다.
Comment on lines
+41
to
+45
| @BeforeEach | ||
| void setUp() { | ||
| jdbcTemplate.execute("DELETE FROM mv_product_rank_weekly"); | ||
| jdbcTemplate.execute("DELETE FROM product_metrics_hourly"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# launchJob 호출부와 run.id 사용 여부 확인
rg -n --type=java 'launchJob\(' -C2
rg -n --type=java 'addLong\("run.id"' -C2
# 배치 메타테이블 정리 코드 존재 여부 확인
rg -n --type=java 'DELETE FROM BATCH_|DELETE FROM batch_|BATCH_JOB|BATCH_STEP' -C2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 14453
🏁 Script executed:
# 파일 라인 수 확인 후 주요 부분 읽기
wc -l apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java
# setUp() 메서드 전체와 필드 선언 확인 (41-45 라인 포함 전후 컨텍스트)
sed -n '1,80p' apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java | cat -n
# countByBaseDate 메서드 구현 확인
sed -n '270,280p' apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java | cat -n
# 경계값 관련 테스트 (excludesDataOutsideWindow) 확인
sed -n '153,185p' apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java | cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5694
배치 메타데이터 정리가 없어 테스트가 플래키해질 수 있다.
setUp()이 비즈니스 테이블만 정리하고 Spring Batch 메타데이터(BATCH_JOB_INSTANCE, BATCH_JOB_EXECUTION 등)를 정리하지 않는다. CI 재시도나 테스트 순서 변경 시 동일한 JobParameters가 남아 JobInstanceAlreadyCompleteException이 발생하거나 비결정적으로 실패할 수 있다. setUp()에서 배치 메타테이블을 정리하거나, 각 launchJob() 호출 시마다 고유한 run.id를 추가하는 방식으로 개선한다.
추가로 경계값 테스트를 보강한다. excludesDataOutsideWindow() 테스트에서 윈도우 시작(targetDate-7일 00:00)과 끝(targetDate 00:00) 정확히의 경계값을 검증해야 한다. 현재는 내부/외부만 검증하고 경계 시각은 누락되어 있어 오프바이원 회귀를 놓칠 수 있다.
마지막으로 countByBaseDate() 메서드에서 테이블명을 문자열 결합으로 SQL에 직접 삽입하는 부분을 개선한다. 동적 테이블명 대신 파라미터화된 쿼리나 유틸 메서드로 분리하여 가독성을 높인다.
🤖 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/weekly/WeeklyRankingJobE2ETest.java`
around lines 41 - 45, The test setup only clears business tables in setUp(),
which leaves Spring Batch metadata (BATCH_JOB_INSTANCE, BATCH_JOB_EXECUTION,
BATCH_JOB_EXECUTION_CONTEXT, BATCH_STEP_EXECUTION, etc.) and can cause
JobInstanceAlreadyCompleteException; update setUp() to truncate those batch meta
tables or ensure every launchJob() call includes a unique JobParameter (e.g.,
run.id) to avoid reuse, then strengthen the excludesDataOutsideWindow() test to
explicitly assert boundary timestamps at targetDate -7 days 00:00 and targetDate
00:00 are handled correctly; finally refactor countByBaseDate() to stop
concatenating the table name into SQL directly by using a parameterized helper
or a small utility that safely maps allowed table names to prepared statements
to improve readability and safety.
Comment on lines
+153
to
+181
| @DisplayName("슬라이딩 윈도우 바깥 데이터는 집계에서 제외된다.") | ||
| @Test | ||
| void excludesDataOutsideWindow() throws Exception { | ||
| // given — 윈도우: [targetDate-7일 00:00, targetDate 00:00) | ||
| LocalDate targetDate = LocalDate.of(2026, 4, 20); | ||
| LocalDateTime outsideWindow = targetDate.minusDays(8).atTime(12, 0); // 윈도우 시작보다 하루 이전 | ||
| LocalDateTime insideWindow = targetDate.minusDays(6).atTime(12, 0); // 윈도우 내 | ||
| insertMetrics(1L, outsideWindow, 0, 0, 0, 50000.0); // 집계 제외 | ||
| insertMetrics(2L, insideWindow, 0, 0, 0, 1000.0); // 집계 포함 | ||
| jobLauncherTestUtils.setJob(job); | ||
|
|
||
| // when | ||
| var jobExecution = jobLauncherTestUtils.launchJob(new JobParametersBuilder() | ||
| .addLocalDate("targetDate", targetDate) | ||
| .toJobParameters()); | ||
|
|
||
| // then — product 2만 MV 에 적재되어야 한다 | ||
| LocalDate baseDate = targetDate.minusDays(1); | ||
| List<Map<String, Object>> rows = jdbcTemplate.queryForList( | ||
| "SELECT product_id, `rank` FROM mv_product_rank_weekly WHERE base_date = ? ORDER BY `rank`", | ||
| baseDate | ||
| ); | ||
| assertAll( | ||
| () -> assertThat(jobExecution.getExitStatus().getExitCode()) | ||
| .isEqualTo(ExitStatus.COMPLETED.getExitCode()), | ||
| () -> assertThat(rows).hasSize(1), | ||
| () -> assertThat(((Number) rows.get(0).get("product_id")).longValue()).isEqualTo(2L) | ||
| ); | ||
| } |
There was a problem hiding this comment.
슬라이딩 윈도우 경계값 검증이 없어 오프바이원 회귀를 놓칠 수 있다.
운영 관점에서 경계 포함/제외가 틀리면 일자별 랭킹이 지속적으로 왜곡될 수 있다. targetDate.minusDays(7).atStartOfDay()는 포함되고 targetDate.atStartOfDay()는 제외되는지를 명시적으로 검증하는 케이스를 추가하는 것이 좋다. 추가 테스트로 두 시각에 각각 메트릭을 넣고 포함/제외 결과를 단정하면 회귀 탐지력이 올라간다.
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/job/weekly/WeeklyRankingJobE2ETest.java`
around lines 153 - 181, Add an explicit boundary test in WeeklyRankingJobE2ETest
to verify inclusion of targetDate.minusDays(7).atStartOfDay() and exclusion of
targetDate.atStartOfDay(): use insertMetrics(...) to create two products with
timestamps at targetDate.minusDays(7).atStartOfDay() (should be included) and
targetDate.atStartOfDay() (should be excluded), launch the job via
jobLauncherTestUtils.launchJob(new
JobParametersBuilder().addLocalDate("targetDate",
targetDate).toJobParameters()), then query mv_product_rank_weekly with
jdbcTemplate.queryForList(...) for baseDate = targetDate.minusDays(1) and assert
that only the product from the minus-7 start-of-day timestamp is present (use
the existing assert pattern and ExitStatus checks). Ensure you reference the
existing helper insertMetrics, jobLauncherTestUtils, and jdbcTemplate usage so
the new assertions plug into the current test scaffolding.
Comment on lines
+31
to
+34
| java -jar "${JAR_PATH}" \ | ||
| --spring.profiles.active="${SPRING_PROFILE}" \ | ||
| --job.name=monthlyRankingJob \ | ||
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 먼저 스크립트 파일의 정확한 내용을 확인
find . -name "run-monthly-ranking.sh" -type f | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 117
🏁 Script executed:
# 배치 관련 스프링 설정 파일 찾기
find . -type f \( -name "*.java" -o -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \) | xargs rg "spring.batch.job" -l 2>/dev/null | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 894
🏁 Script executed:
# 스크립트들에서 사용 중인 job.name 또는 spring.batch.job.name 패턴 확인
fd -e "sh" scripts/ | xargs rg "job\.name|spring\.batch\.job\.name" -A2 -B2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 8101
🏁 Script executed:
# BatchConfiguration 클래스 검색 (월간 Job 설정을 제어하는 부분)
rg "monthlyRankingJob|spring\.batch\.job\.name" -A5 -B5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 19677
배치 선택 프로퍼티 이름이 일치하지 않아 월간 Job이 실행되지 않을 수 있다
스크립트는 --job.name=monthlyRankingJob을 전달하지만, 배치 작업 설정은 @ConditionalOnProperty(name = "spring.batch.job.name", ...)으로 spring.batch.job.name 프로퍼티만 인식한다. application.yml의 spring.batch.job.name: ${job.name:NONE} 매핑이 있어도, YAML 프로퍼티 해석 시점과 커맨드라인 인자 로드 순서에 따라 실패할 수 있다. 특히 E2E 테스트는 @TestPropertySource(properties = "spring.batch.job.name=...")로 직접 설정하며, 이는 올바른 프로퍼티 이름이 무엇인지 명확히 보여준다. --spring.batch.job.name=monthlyRankingJob으로 수정하여 운영 환경에서 프로퍼티 해석 실패를 근본적으로 차단한다.
수정 제안
java -jar "${JAR_PATH}" \
--spring.profiles.active="${SPRING_PROFILE}" \
- --job.name=monthlyRankingJob \
+ --spring.batch.job.name=monthlyRankingJob \
targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$?📝 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
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --job.name=monthlyRankingJob \ | |
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? | |
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --spring.batch.job.name=monthlyRankingJob \ | |
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/run-monthly-ranking.sh` around lines 31 - 34, The script passes the
wrong batch property flag (--job.name=monthlyRankingJob) so Spring won't see
spring.batch.job.name; update the java invocation in
scripts/run-monthly-ranking.sh to pass --spring.batch.job.name=monthlyRankingJob
(keeping other arguments like "${JAR_PATH}",
--spring.profiles.active="${SPRING_PROFILE}" and targetDate="${TARGET_DATE}"
intact) so the application recognizes the job name reliably across environments
and tests.
Comment on lines
+31
to
+34
| java -jar "${JAR_PATH}" \ | ||
| --spring.profiles.active="${SPRING_PROFILE}" \ | ||
| --job.name=weeklyRankingJob \ | ||
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for spring.batch.job.name and job.name usage patterns
rg "spring\.batch\.job\.name|--job\.name" --type-add "shell:*.sh" --type-add "java:*.java,*.kt"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2180
🏁 Script executed:
# Find job configuration files and Bean declarations
fd -e java -e kt | xargs rg -l "Job|BatchConfiguration|@Bean" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1734
🏁 Script executed:
# Check the run-weekly-ranking.sh script for context
cat -n scripts/run-weekly-ranking.shRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1432
🏁 Script executed:
# Look for any Spring Batch job config in the project
find . -type f \( -name "*Batch*" -o -name "*Job*" \) \( -name "*.java" -o -name "*.kt" -o -name "*.xml" \) | grep -v node_modules | grep -v ".git"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1037
🏁 Script executed:
# Check the WeeklyRankingJobConfig to confirm the property condition
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/weekly/WeeklyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 8487
🏁 Script executed:
# Verify CommerceBatchApplication to understand job startup
cat -n apps/commerce-batch/src/main/java/com/loopers/CommerceBatchApplication.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 972
🏁 Script executed:
# Check if there are any integration tests that simulate script execution
cat -n apps/commerce-batch/src/test/java/com/loopers/job/weekly/WeeklyRankingJobE2ETest.java | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2140
배치 선택 프로퍼티 이름이 잘못되어 주간 Job이 활성화되지 않는다
스크립트는 --job.name=weeklyRankingJob을 전달하지만, WeeklyRankingJobConfig의 @ConditionalOnProperty(name = "spring.batch.job.name", ...)는 이 프로퍼티 조건을 충족할 수 없다. 결과적으로 Job Bean이 로드되지 않아 배치 실행이 실패하거나 잘못된 Job이 실행될 수 있다.
--spring.batch.job.name=weeklyRankingJob으로 수정하고, 스모크 테스트를 통해 스크립트 실행 시 해당 프로퍼티가 애플리케이션에 전달되는지 검증해야 한다. 같은 이유로 scripts/run-monthly-ranking.sh도 동일한 수정이 필요하다.
수정 사항
java -jar "${JAR_PATH}" \
--spring.profiles.active="${SPRING_PROFILE}" \
- --job.name=weeklyRankingJob \
+ --spring.batch.job.name=weeklyRankingJob \
targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$?📝 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
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --job.name=weeklyRankingJob \ | |
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? | |
| java -jar "${JAR_PATH}" \ | |
| --spring.profiles.active="${SPRING_PROFILE}" \ | |
| --spring.batch.job.name=weeklyRankingJob \ | |
| targetDate="${TARGET_DATE}" && EXIT_CODE=0 || EXIT_CODE=$? |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/run-weekly-ranking.sh` around lines 31 - 34, The script passes the
wrong flag (--job.name) so WeeklyRankingJobConfig's `@ConditionalOnProperty`(name
= "spring.batch.job.name", ...) never matches; change the invocation in the
weekly script to use --spring.batch.job.name=weeklyRankingJob (and apply the
same fix in scripts/run-monthly-ranking.sh for the monthly job), then run a
smoke test to confirm the --spring.batch.job.name property is delivered to the
JVM and the correct Job bean (WeeklyRankingJobConfig) is loaded.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
commerce-batch에 주간·월간 랭킹 배치 Job을 구현하고,commerce-api에/api/v1/rankings/weekly,/api/v1/rankings/monthly엔드포인트를 추가했습니다. 59건 신규 테스트 ALL PASS.Context & Decision
문제 정의
product_metrics_hourly테이블을 전체 스캔해서 GROUP BY로 집계하면, 월간 기준으로 상품 1개당 시간 단위 row가 720개씩 쌓여 있어 상품 수 × 720 row를 매 요청마다 계산해야 합니다. 트래픽이 늘수록 DB 부하가 그대로 비례해서 증가합니다.선택지와 결정
선택 1. 배치 처리 구조
Spring Batch에서 데이터를 처리하는 방법은 크게 두 가지입니다.
결정: B — Chunk
Chunk는 "1건씩 읽고 1건씩 가공한다"는 구조인데, 여기서 읽어야 할 데이터는 여러 시간 단위 row를 GROUP BY로 합친 집계값입니다. 구조가 어색해 보였지만, SQL에서 GROUP BY를 미리 처리하면 Reader 입장에서는 product_id 1개 = 집계 결과 1행으로 받아볼 수 있어서 Chunk 구조와 자연스럽게 맞아떨어졌습니다.
선택 2. 데이터를 읽는 방식 (ItemReader)
결정: B — JdbcCursorItemReader
읽어야 할 데이터가 TOP 100건뿐이고, 하루 1회 새벽에 실행하는 배치라 실행 시간이 짧습니다. 커넥션을 오래 잡고 있을 위험이 낮고, PagingItemReader의 복잡한 정렬 키 설정을 감수할 이유가 없다고 봤습니다.
선택 3. 기간 집계 기준
"이번 주 랭킹"을 어떻게 정의할지가 핵심이었습니다.
결정: B — 슬라이딩 윈도우 (어제 기준 직전 7일/30일)
배치는 새벽에 실행되는데, 오늘 날짜 기준으로 집계하면 당일 0시부터 배치 실행 시각까지만 쌓인 불완전한 데이터가 포함됩니다. 기준일을 하루 전(targetDate - 1)으로 잡으면 전날까지 완전히 쌓인 데이터만 쓰니까 항상 안정적인 랭킹이 나옵니다.
선택 4. 집계 결과를 저장하는 방식
배치가 새 랭킹을 계산했을 때 기존 데이터를 어떻게 교체할지 결정해야 했습니다.
결정: B — DELETE + INSERT
UPSERT의 문제는 재실행 시 이전 레코드가 남는다는 점입니다. 예를 들어 1차 실행 중 실패해서 상품 A가 절반만 저장된 상태에서 2차 실행을 하면, 상품 A가 이번 TOP 100 밖으로 밀렸을 때 1차 레코드가 그대로 남습니다. 재실행을 안전하게 하려면 어차피 DELETE를 앞에 붙여야 하는데, 그러면 DELETE + INSERT와 구조가 같아져서 UPSERT를 쓸 이유가 없어집니다.
선택 5. API 엔드포인트 구조
?period=weekly파라미터 추가 — 변경 범위가 작지만, 하나의 메서드 안에서 daily/weekly/monthly를 분기 처리해야 합니다./rankings/weekly,/rankings/monthly) — 각 엔드포인트가 하나의 역할만 가집니다.결정: B — 별도 엔드포인트
daily는 Redis, weekly/monthly는 집계 결과 테이블로 데이터 소스가 완전히 다릅니다. A 방식이면 period 값에 따라 내부 분기가 생기고, 나중에 기간별로 다른 요구사항이 붙을 때마다 복잡도가 계속 쌓입니다. 데이터 성격이 다르면 URL로 구분하는 게 맞다고 봤습니다.
선택 6. 스케줄링 방식
결정: 외부 스케줄링 (Jenkins / K8s CronJob)
@Scheduled를 쓰면 서버가 여러 대 떠 있을 때 같은 Job이 동시에 실행될 수 있습니다. 배치 앱은 독립 실행 단위로 두고 스케줄 제어는 외부에 맡겼습니다.--job.name파라미터로 실행할 Job을 지정하고,@ConditionalOnProperty로 해당 Job에 필요한 Bean만 로드되도록 해서 Job 간 간섭을 차단했습니다.Design Overview
변경 범위
commerce-batch(신규 Job),commerce-api(Ranking API 확장)commerce-batch:WeeklyRankingJobConfig,MonthlyRankingJobConfig,WeeklyRankingItemWriter,MonthlyRankingItemWriter,JobListener,StepMonitorListener,ChunkListener,domain.ranking패키지 (MvProductRankRepository,MvProductRankRow,ProductMetricsAggregate),JdbcMvProductRankRepositorycommerce-api:WeeklyRankingFacade,MonthlyRankingFacade,RankingAssembler,RankingPageResult,RankingPageQuery,WeeklyRankingV1Controller,MonthlyRankingV1Controller,WeeklyRankingRepository,MonthlyRankingRepository,MvProductRankWeekly,MvProductRankMonthly및 JPA 구현체RankingFacade와RankingV1Controller도RankingAssembler·RankingPageQuery를 쓰도록 변경.RankingFacadeTest는 Mock 대상이 바뀐 부분 반영.주요 컴포넌트 역할
commerce-batch
WeeklyRankingJobConfig/MonthlyRankingJobConfig: Job·Step·Reader를 정의합니다. SQL 집계, 슬라이딩 윈도우 날짜 계산,TOP_N상수로 SQLLIMIT과 chunk size를 같은 값으로 묶어 관리합니다.WeeklyRankingItemWriter/MonthlyRankingItemWriter: Chunk로 받은 데이터에 순서대로 1위부터 rank를 붙이고, DELETE + INSERT로 집계 결과 테이블을 교체합니다.JobListener/StepMonitorListener/ChunkListener: 실행 시작·종료 시각, 처리 건수, 실패 원인을 로그로 남깁니다. Step이 실패하면 Slack 같은 외부 채널로 알림을 보낼 수 있는 자리입니다.JdbcMvProductRankRepository: DELETE + INSERT를@Transactional로 원자적으로 처리합니다. Batch 트랜잭션 안에서는 합류(REQUIRED), 외부 호출 시 자체 트랜잭션 생성.commerce-api
WeeklyRankingFacade/MonthlyRankingFacade: MV 테이블 조회 후 상품 가시성 필터링과 결과 조립을RankingAssembler에 위임합니다.date=null이면 KST 기준 어제 날짜를 기본값으로 사용합니다.RankingAssembler: DB에서 꺼낸 랭킹 목록에서 삭제·숨김 상품을 걸러내고, 페이지 결과 객체로 만들어 반환합니다. 일간·주간·월간 Facade가 모두 공통으로 씁니다.RankingPageResult:effectiveDate + total + items를 Facade가 하나로 묶어 반환하는 VO. Controller가 Facade 내부 메서드를 직접 호출하는 구조를 제거합니다.RankingPageQuery: 날짜 문자열 파싱, 페이지·사이즈 범위 보정 같은 공통 파라미터 처리를 한 곳에 모아뒀습니다. 세 Controller가 공유합니다.WeeklyRankingV1Controller/MonthlyRankingV1Controller: 각각/api/v1/rankings/weekly,/api/v1/rankings/monthly엔드포인트를 담당합니다.구현 기능
1. WeeklyRankingJobConfig / MonthlyRankingJobConfig — Chunk 기반 집계 Job
JdbcCursorItemReader로product_metrics_hourly에서 슬라이딩 윈도우 기간의 데이터를 SQL GROUP BY로 집계해 읽습니다.LN(1 + SUM(view_count)) * ? + LN(1 + SUM(like_count)) * ? + LN(1 + SUM(order_amount)) * ?공식으로 score를 계산해 일간 랭킹과 동일한 가중치 체계를 유지합니다.TOP_N = 100상수로 SQLLIMIT과 chunk size를 묶어 단일 Chunk 전제를 코드로 명시합니다.2. WeeklyRankingItemWriter / MonthlyRankingItemWriter — rank 부여 + MV 교체
score 내림차순으로 정렬된 집계 결과를 받아 1위부터 순서대로 rank를 붙입니다.
JdbcMvProductRankRepository.replace*()를 호출해 해당base_date의 MV 데이터를 DELETE + INSERT로 원자적으로 교체합니다.3. JobListener / StepMonitorListener / ChunkListener — 배치 모니터링
JobListener는 시작 시각을ExecutionContext에 저장하고 종료 시 시간/분/초 단위 소요 시간을 로깅합니다.StepMonitorListener는 Step 실패 시 예외 메시지를 로깅하고ExitStatus.FAILED를 반환합니다 (Slack 알림 연동 포인트 주석 처리).ChunkListener는 Chunk 완료마다 readCount/writeCount를 로깅합니다.4. RankingAssembler — 공통 조립 파이프라인
entries → productIds 추출 → findVisibleByIds → RankingItemInfo.of → RankingPageResult파이프라인을 단일 위치로 통합합니다. 가시성 필터 정책이 바뀌면 세 Facade를 모두 수정하는 Shotgun Surgery를 방지합니다.KST상수도 이곳에서 관리하여 Facade 간 일관성을 보장합니다.5. WeeklyRankingV1Controller / MonthlyRankingV1Controller — 주간/월간 랭킹 API
/api/v1/rankings/weekly?date=yyyyMMdd&page=1&size=20/api/v1/rankings/monthly?date=yyyyMMdd&page=1&size=20date생략 시 KST 어제 날짜 기본값.page=0이면 1로 보정,size범위 초과 시 MAX_SIZE(100)로 클램핑. 잘못된 날짜 포맷은 400 반환.6. scripts/run-weekly-ranking.sh / run-monthly-ranking.sh — 외부 트리거 스크립트
Jenkins/K8s CronJob에서 호출하는 실행 스크립트입니다.
targetDate인자 생략 시 오늘 날짜 자동 설정.JAR_PATH,SPRING_PROFILE환경변수로 경로·프로필 재정의 가능.set -euo pipefail로 오류 발생 시 즉시 종료 및 exit code 전달.Flow Diagram
Batch Flow (새벽 배치 실행)
API Flow (랭킹 조회)
테스트
신규 테스트 요약 (59건 ALL PASS)
RankingAssemblerTestWeeklyRankingFacadeTestMonthlyRankingFacadeTestWeeklyRankingV1ControllerTestMonthlyRankingV1ControllerTestWeeklyRankingJobE2ETestMonthlyRankingJobE2ETestWeeklyRankingV1ApiE2ETestMonthlyRankingV1ApiE2ETest기존 테스트 변경
RankingFacadeTestRankingAssembler로 변경,RankingPageResult반환 타입 반영RankingFacade가 직접 조합하던 로직을RankingAssembler에 위임하면서 테스트 구조도 같이 변경테스트 전략 포인트
Batch E2E와 API E2E를 분리한 이유
API E2E 테스트는 MV 테이블에 직접 데이터를 넣어서 배치 실행 없이 동작합니다. 배치가 실패하거나 스펙이 바뀌어도 API 테스트는 독립적으로 돌릴 수 있고, 실패 원인도 "배치 문제"인지 "API 문제"인지 바로 알 수 있습니다.
excludesDataOutsideWindow테스트가 있는 이유슬라이딩 윈도우 경계 조건(
targetDate - 7일/targetDate - 30일)은 오프셋 값 하나만 잘못 써도 조용히 틀립니다. 윈도우 밖 데이터가 score가 아무리 높아도 MV에 적재되지 않는다는 걸 직접 검증해서 이 종류의 버그를 잡아냅니다.failsWithoutTargetDate가 FAILED로 끝나는 건 의도된 동작targetDate없이 실행하면ItemReader에서 NPE가 발생하고, Spring Batch가 이를 잡아ExitStatus=FAILED로 전환합니다. 파라미터 없이 배치를 돌렸을 때 조용히 COMPLETED 되는 버그를 방지하는 역할입니다. 향후JobParametersValidator를 따로 구현하면 더 명확한 오류 메시지를 남길 수 있습니다.Checklist
변경 목적: 요청 시마다 product_metrics_hourly를 집계하던 비용(예: 월간은 상품수×720 행)을 줄이기 위해 주간/월간 랭킹을 일일 배치로 선계산해 MV 테이블에 저장하고 API는 단순 조회로 제공하도록 구현.
핵심 변경점: commerce-batch에 JdbcCursorItemReader+Chunk 기반 주간/월간 배치(슬라이딩 윈도우: targetDate-1 기준 7/30일, TOP_N=100, LN(1+metric) 가중 스코어)와 DELETE+INSERT 교체 전략, RankingMvCleanupListener/Chunk/Job 리스너 및 step-accumulate writer 추가. commerce-api에는 Weekly/Monthly Facade·Controller·RankingAssembler·RankingPageQuery·MV JPA 엔티티/레포지토리와 스크립트(run-weekly/monthly) 추가, BATCH_TZ로 타임존 결정 일관화.
리스크/주의사항: 배치는 외부 스케줄러(Jenkins/K8s CronJob)에서 실행 필요—실행 누락 시 최신성 보장 불가. targetDate 미존재 시 실패(IllegalStateException) 처리 및 MV 교체(DELETE 후 INSERT) 동작으로 데이터 손실/경합 가능성에 유의. 페이지/size는 클램핑 적용. 운영 모니터링(실패 알림/재시도 정책) 및 스케줄러 권장 방식 확인 필요.
테스트/검증: 단위·배치 E2E·API E2E 포함 59개 테스트 추가(윈도우 경계, 가시성 필터링, 재실행 시 MV 교체, TOP_N 제한 등 검증) 및 모두 통과. 배치 스크립트/운영 스케줄 통합은 별도(수동) 검증이 필요해 보이는데, 외부 스케줄러 연동 방식(예: Jenkins vs K8s CronJob) 중 우선 적용할 환경을 알려주실 수 있습니까?