[7팀 안유진] 10주차 과제 제출 (주간/월간 랭킹 배치 시스템 구축)#211
Open
anewjean wants to merge 8 commits into
Open
Conversation
- ProductMetricsModel에 date: LocalDate 컬럼 추가 - UNIQUE 인덱스를 product_id → (product_id, date)로 변경 - ProductMetricsSnapshot에 date 필드 추가 - 주간/월간 랭킹 집계를 위한 일별 데이터 관리 기반 마련 🤖 Generated with Claude Code
- UPSERT SQL에 date 컬럼 추가 - RESET 로직 제거 (날짜별 독립 row이므로 불필요) - RedisReader에서 snapshot에 date 포함하여 반환 - JobConfig 주석 정리 🤖 Generated with Claude Code
- DDL에 date 컬럼 추가, UNIQUE KEY를 (product_id, date)로 변경 - 쿼리에 date 조건 추가 - '빠진 상품 0 초기화' → '다른 날짜 데이터 미영향' 테스트로 변경 - RESET 롤백 테스트 → UPSERT 실패 시 기존 행 보존 테스트로 변경 🤖 Generated with Claude Code
- MvProductRankWeeklyModel, MvProductRankMonthlyModel 엔티티 생성 - RankingPeriod enum (DAILY, WEEKLY, MONTHLY) 추가 - MvProductRankRepository 인터페이스 및 구현체 추가 - JPA Repository (weekly, monthly) 추가
- weeklyRankJob: Tasklet(DELETE) → Chunk(JdbcCursorItemReader + INSERT) TOP 100 - monthlyRankJob: 주간과 동일 구조, WHERE 조건만 이번 달 1일~오늘 - metricsCleanupJob: Tasklet으로 60일 이전 product_metrics 삭제 - 주간 경계: 일요일~토요일, 월간 경계: 1일~말일 - Score 가중치: view×0.1 + like×0.2 + order×0.7
- Controller에 period 파라미터 추가 (기본값: daily) - GetRankingCriteria에 period 필드 추가 - UseCase에서 when(period) 분기로 데이터 소스 선택 - daily → Redis ZSET, weekly/monthly → DB MV 테이블 - ApiSpec에 period 파라미터 반영
- WeeklyRankJobE2ETest: 합산 적재, 멱등성, 범위 밖 제외, 빈 메트릭 - MonthlyRankJobE2ETest: 합산 적재, 범위 밖 제외, 멱등성 - MetricsCleanupJobE2ETest: 60일 이전 삭제 + 최근 보존 - RankingV1ApiE2ETest: period=weekly/monthly 조회, 잘못된 period 400
📝 WalkthroughWalkthrough랭킹 기능을 일일 기준에서 주간, 월간으로 확장했다. RankingPeriod 열거형을 추가하고 주간/월간 MV 테이블과 JPA 저장소를 도입했으며, 배치 작업을 통해 데이터를 생성한다. API에 기간 파라미터를 추가하여 요청 시 데이터 소스를 선택하도록 변경했다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as RankingV1Controller
participant UseCase as UserGetRankingUseCase
participant PeriodRepo as MvProductRankRepository
participant RedisReader as ProductRankRedisReader
participant ProductSvc as ProductService
participant BrandSvc as BrandService
participant DB as Database
Client->>Controller: GET /api/v1/rankings?period=weekly&page=0&size=10
Controller->>Controller: parsePeriod("weekly") → RankingPeriod.WEEKLY
Controller->>UseCase: getRanking(criteria: GetRankingCriteria)
alt period == WEEKLY
UseCase->>PeriodRepo: getWeeklyRanking(page=0, size=10)
PeriodRepo->>DB: SELECT * FROM mv_product_rank_weekly ORDER BY score DESC
DB-->>PeriodRepo: RankRow list (productId, score)
PeriodRepo-->>UseCase: List<RankedProductEntry>
UseCase->>PeriodRepo: getWeeklyTotalCount()
PeriodRepo->>DB: SELECT COUNT(*) FROM mv_product_rank_weekly
DB-->>PeriodRepo: totalCount
else period == DAILY
UseCase->>RedisReader: getRankingPage(date, page, size)
RedisReader->>DB: Query Redis ZSET
DB-->>RedisReader: RankedProductEntry list
RedisReader-->>UseCase: List<RankedProductEntry>
UseCase->>RedisReader: getTotalCount(date)
end
UseCase->>ProductSvc: getProducts(productIds)
ProductSvc-->>UseCase: Map<Long, Product>
UseCase->>BrandSvc: getBrands(brandIds)
BrandSvc-->>UseCase: Map<Long, Brand>
UseCase->>UseCase: assembleRankedProductResults(entries, products, brands)
UseCase-->>Controller: RankingPageResponse
Controller-->>Client: ApiResponse<RankingPageResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserGetRankingUseCase.kt (1)
72-82:⚠️ Potential issue | 🟡 Minor주석과 반환값이 WEEKLY/MONTHLY 분기와 맞지 않다.
- 주석은 여전히 "ZSET entries", "Redis ZCARD 기준" 등 일일 경로 기준으로 쓰여 있으나, 이제 분기에 따라 MV 테이블이 소스가 된다. 유지보수자가 오독할 수 있으니 분기별 의미를 갱신한다.
GetRankingResult.date = criteria.date는 DAILY에선 맞지만 WEEKLY/MONTHLY에선 "요청 시점의 today"를 그대로 반환하여 응답 의미가 모호해진다. 응답에 기간 정보(period, periodStart/periodEnd) 또는 MV 생성 시각을 포함하는 쪽이 클라이언트 캐싱·디버깅에 유리하다.hasNext = entries.size >= criteria.size는 MV 경로에서totalCount를 이미 알고 있으므로(criteria.page + 1) * criteria.size < totalCount로 정확히 계산 가능하다. DAILY 경로의 soft-delete 보정 이유가 MV에는 적용되지 않는다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserGetRankingUseCase.kt` around lines 72 - 82, Update the stale comment to describe that DAILY uses Redis entries but WEEKLY/MONTHLY use materialized view (MV) data, and then change the returned GetRankingResult so it does not always set date = criteria.date: for MV branches (WEEKLY/MONTHLY) populate a clear period identifier (e.g., period/periodStart/periodEnd or the MV generation timestamp) instead of criteria.date so the client can cache/interpret the response; finally, compute hasNext differently per data source—keep entries.size >= criteria.size for the DAILY (Redis entries) path but for MV paths set hasNext = (criteria.page + 1) * criteria.size < totalCount (using totalCount determined from the MV) rather than entries.size, and ensure you reference GetRankingResult, criteria.date, hasNext, entries.size and totalCount when making these changes.
🧹 Nitpick comments (14)
apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/MetricsCleanupJobConfig.kt (1)
32-48: LGTM에 가깝지만, 실패 관측성과 재실행 안전성에 대한 보강을 권고한다.Job/Step 구성은 의도대로 동작한다. 다만 운영 관점에서 다음 두 가지를 점검하길 권한다.
RunIdIncrementer는 매 실행마다 파라미터를 다르게 만들어 실패한 이전 실행을 "재시작"이 아닌 "새 인스턴스"로 처리한다. 재시작(resume) 의미가 필요 없다면 현재 그대로가 맞지만, 실패 누적을 모니터링하려면 별도 지표가 필요하다.- 본 Step은 리소스 집약적
DELETE이므로StepMonitorListener외에 삭제 건수/소요 시간 같은 메트릭을 Micrometer로 노출하면 운영 상황을 빠르게 진단할 수 있다.추가 테스트: 대량 삭제 시나리오(예: 수만 건)에서 Step이 타임아웃 또는 트랜잭션 타임아웃 없이 완료되는지 부하 테스트를 분리 수행할 것을 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/MetricsCleanupJobConfig.kt` around lines 32 - 48, The job currently uses RunIdIncrementer in metricsCleanupJob which creates a new job instance each run (preventing restart semantics); decide whether to remove/replace RunIdIncrementer or document/emit restart-aware metrics if you need resume behavior, and in metricsCleanupStep/metricsCleanupTasklet instrument the heavy DELETE work with Micrometer (register a Counter for deleted records and a Timer or LongTaskTimer for duration) and emit those metrics from metricsCleanupTasklet (and/or via stepMonitorListener) so operators can monitor delete counts and execution time; also ensure the tasklet is idempotent and consider chunking/transaction boundaries or adjusting transactionManager/step configuration to avoid transaction/timeouts during large-batch deletes.apps/commerce-batch/src/test/kotlin/com/loopers/job/metricscleanup/MetricsCleanupJobE2ETest.kt (1)
120-140: "변화 없음" 검증이 약하다.현재는 삽입한 1건이 그대로 1건이면 통과하지만, 삭제가 잘못 수행되어 같은 행이 삭제된 뒤 테스트 훅이 다시 넣는다면 감지가 어렵다. 또한 "삭제할 게 없다"는 시맨틱을 명확히 하려면 삽입 건을 여러 개로 늘려 총량 불변을 확인하는 편이 좋다.
수정안: 최근 일자 데이터를 2건 이상 삽입해 총량/식별자 동일성을 함께 검증한다.
🧪 제안 패치
- seedMetrics(PRODUCT_ID_1, LocalDate.now()) + seedMetrics(PRODUCT_ID_1, LocalDate.now()) + seedMetrics(PRODUCT_ID_2, LocalDate.now().minusDays(59)) ... - assertThat(count).isEqualTo(1L) + assertThat(count).isEqualTo(2L)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/kotlin/com/loopers/job/metricscleanup/MetricsCleanupJobE2ETest.kt` around lines 120 - 140, Update the shouldCompleteWhenNothingToDelete test so it seeds at least two recent-date metric rows and then verifies both that the total row count remains the same and that the set of identifiers for PRODUCT_ID_1 is unchanged after running the job; specifically, in the test function shouldCompleteWhenNothingToDelete call seedMetrics(PRODUCT_ID_1, LocalDate.now()) twice (or a helper to insert two rows), run jobLauncherTestUtils.launchJob(jobParameters()), then use jdbcTemplate.queryForObject to assert COUNT(*) == 2L and additionally query the identifying column(s) (e.g., ids or timestamps) for PRODUCT_ID_1 and assert the returned set equals the pre-run set to ensure no row was deleted-and-reinserted.apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/step/MetricsCleanupTasklet.kt (1)
27-35: 대용량 단일 DELETE의 운영 리스크 점검이 필요하다.60일 이상 누적된
product_metrics의 삭제 대상이 수십만~수백만 건에 이를 경우, 단일DELETE문은 다음과 같은 운영 문제를 일으킬 수 있다.
- 장시간 트랜잭션과 언두 로그 팽창으로 복제 지연(replication lag)이 증가한다.
- 인덱스/갭 락으로 인해 동시에 실행되는
ProductMetricsSyncJob의 UPSERT가 블로킹되거나 데드락이 발생할 수 있다.- 실패 시 전체 롤백되어 재시도 비용이 크다.
운영 안정성을 위해 배치 분할 삭제(chunked delete) 방식으로 바꾸고, 필요 시 주 인덱스(
date)에 대한 커버링 인덱스가 있는지 확인하는 것을 권장한다.♻️ 제안 패치: LIMIT 루프로 분할 삭제
override fun execute(contribution: StepContribution, chunkContext: ChunkContext): RepeatStatus { val cutoffDate = LocalDate.now().minusDays(RETENTION_DAYS) - val deletedCount = jdbcTemplate.update( - "DELETE FROM product_metrics WHERE date < ?", - cutoffDate, - ) - log.info("{}일 이전 메트릭 {}건 삭제 (기준일: {})", RETENTION_DAYS, deletedCount, cutoffDate) + var totalDeleted = 0 + while (true) { + val deleted = jdbcTemplate.update( + "DELETE FROM product_metrics WHERE date < ? LIMIT ?", + cutoffDate, DELETE_BATCH_SIZE, + ) + totalDeleted += deleted + if (deleted < DELETE_BATCH_SIZE) break + } + log.info("{}일 이전 메트릭 {}건 삭제 (기준일: {})", RETENTION_DAYS, totalDeleted, cutoffDate) return RepeatStatus.FINISHED }companion object { private const val RETENTION_DAYS = 60L + private const val DELETE_BATCH_SIZE = 1_000 }추가 테스트로 다음을 권장한다.
- 배치 크기 이상의 행(예:
DELETE_BATCH_SIZE + 1)이 삭제 대상일 때 모두 삭제되는지 검증- 실행 중 신규 INSERT가 발생해도 기대한 삭제 건수가 집계되는지 검증
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/step/MetricsCleanupTasklet.kt` around lines 27 - 35, The current execute method in MetricsCleanupTasklet performs a single large DELETE via jdbcTemplate.update("DELETE FROM product_metrics WHERE date < ?", cutoffDate) which can cause long transactions, replication lag, locks and rollbacks for millions of rows; change it to a chunked-delete loop that repeatedly issues limited-size deletes (e.g., DELETE FROM product_metrics WHERE date < ? LIMIT DELETE_BATCH_SIZE) until the returned deletedCount for a batch is zero, logging each batch and the total deleted, and ensure DELETE_BATCH_SIZE and RETENTION_DAYS are configurable; also verify the date column has an appropriate covering index to make the batched deletes efficient.apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserRankingCriteria.kt (1)
6-11:date가WEEKLY/MONTHLY에서 무시되는 필드라는 사실이 타입에 드러나지 않는다.
UserGetRankingUseCase가period에 따라date를 사용/무시하도록 분기한다는 점은 본 criteria 타입만 봐서는 알 수 없다. 운영·유지보수 관점에서, 호출자가 주간/월간 요청에 엉뚱한 날짜를 넘겨도 조용히 무시되어 디버깅이 어려워지고, 향후 "주간 기준일 지정" 요건이 추가되면 의미가 더 모호해진다. 최소한 KDoc으로period == DAILY일 때만 유효함을 명시하고, 장기적으로는DailyCriteria/WeeklyCriteria/MonthlyCriteria분리(sealed class) 또는date: LocalDate?로 타입 수준에서 표현하는 것을 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserRankingCriteria.kt` around lines 6 - 11, GetRankingCriteria currently has a non-null date that is ignored when period != RankingPeriod.DAILY, which is not obvious to callers; update the API to make this explicit by either (A) adding KDoc above GetRankingCriteria explaining "date is only used when period == DAILY", or (B) refactor the type into a sealed hierarchy (e.g., DailyCriteria(date: LocalDate), WeeklyCriteria, MonthlyCriteria) or change date to nullable (date: LocalDate?) and validate in UserGetRankingUseCase depending on period; reference GetRankingCriteria, UserGetRankingUseCase and RankingPeriod when implementing the chosen change and add validation/logging to surface misuse.apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankRepositoryImpl.kt (1)
14-46: 주간/월간 구현이 거의 동일한 중복 코드다.페이징 생성 → 조회 →
RankedProductEntry매핑 흐름이 완전히 동일하다. 향후 정렬 키·랭크 계산식이 바뀌면 두 군데를 모두 맞춰야 해 실수 여지가 크다(실제로 현재 오프-바이-원이 두 곳에 복제돼 있다). 제네릭/함수 추출로 일원화하기 바란다.♻️ 중복 제거 예시
+ private fun <T> fetchRanking( + page: Int, + size: Int, + loader: (PageRequest) -> List<T>, + productIdOf: (T) -> Long, + scoreOf: (T) -> Double, + ): List<RankedProductEntry> { + val offset = page.toLong() * size + return loader(PageRequest.of(page, size)) + .mapIndexed { index, model -> + RankedProductEntry( + productId = productIdOf(model), + score = scoreOf(model), + rank = offset + index + 1, + ) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankRepositoryImpl.kt` around lines 14 - 46, Extract the duplicated paging→query→map flow in getWeeklyRanking and getMonthlyRanking into a single private helper (e.g., fetchRankingPage or mapRankingPage) that takes a Pageable and a repository accessor (or a function reference to findAllOrderByScoreDesc) and returns List<RankedProductEntry>; inside the helper compute offset, call the provided repository method (weeklyJpaRepository.findAllOrderByScoreDesc or monthlyJpaRepository.findAllOrderByScoreDesc), then mapIndexed to build RankedProductEntry(productId, score, rank = offset + index). Replace getWeeklyRanking and getMonthlyRanking to create the PageRequest and call the new helper with the appropriate repository/function. Ensure helper is reused to remove the off-by-one duplication and keep getWeeklyTotalCount/getMonthlyTotalCount unchanged.apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankMonthlyModel.kt (1)
8-22: MV 테이블에product_id고유 제약 부재가 운영상 리스크다.배치는
Tasklet(DELETE) + Chunk(INSERT)로 멱등성을 확보한다고 했으나, 주간/월간 배치가 중복 실행되거나 DELETE 이후 INSERT 단계에서 재시도/부분 실패가 겹치면 동일product_id가 여러 행으로 누적될 수 있다. 이 경우 API의 TOP 100 조회에서 같은 상품이 복수 랭크로 노출되는 장애로 이어진다. 엔티티와 DDL에@Table(..., uniqueConstraints = [UniqueConstraint(columnNames = ["product_id"])])를 걸어 DB 레벨에서 방어하는 것을 권장한다. 추가로, 배치 E2E에 "연속 2회 실행 시 행 수 불변" 케이스를 보강하기 바란다.♻️ 고유 제약 추가 제안
-import jakarta.persistence.Column -import jakarta.persistence.Entity -import jakarta.persistence.Table +import jakarta.persistence.Column +import jakarta.persistence.Entity +import jakarta.persistence.Table +import jakarta.persistence.UniqueConstraint `@Entity` -@Table(name = "mv_product_rank_monthly") +@Table( + name = "mv_product_rank_monthly", + uniqueConstraints = [UniqueConstraint(name = "uk_mv_product_rank_monthly_product_id", columnNames = ["product_id"])], +) class MvProductRankMonthlyModel(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankMonthlyModel.kt` around lines 8 - 22, The MV table lacks a DB-level unique constraint on product_id which can lead to duplicate rows; update the MvProductRankMonthlyModel class's `@Table` annotation to include UniqueConstraint(columnNames = ["product_id"]) and add a corresponding DB migration/DDL change to enforce uniqueness at the database level; ensure the batch insert logic (the Tasklet DELETE + Chunk INSERT) is compatible with the constraint (add idempotent upsert/replace or handle unique-violation errors if necessary) and strengthen the batch E2E by adding a test that runs the monthly/weekly job twice in succession and asserts the row count (and top-100 results) remain unchanged.apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.kt (1)
11-18: Swagger 스펙에period의 기본값/허용값을 파라미터 메타로 표현한다.현재는
description문자열에만daily|weekly|monthly가 적혀 있어 Swagger UI에서 enum 드롭다운/기본값 힌트가 제공되지 않는다. 운영/협업 관점에서 계약이 문서에 구조적으로 드러나지 않으면 잘못된 값으로 400을 유발하기 쉽다.@Parameter(schema =@Schema(allowableValues = ["daily","weekly","monthly"], defaultValue = "daily"))를period에 부착하거나, 타입을RankingPeriodenum으로 바꾸는 것을 고려한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.kt` around lines 11 - 18, Add structured Swagger metadata for the period parameter on the getRanking method: either annotate the period parameter with `@Parameter`(schema = `@Schema`(allowableValues = ["daily","weekly","monthly"], defaultValue = "daily")) so Swagger shows an enum dropdown and default, or change the parameter type from String to a new or existing RankingPeriod enum (values daily, weekly, monthly) and let the framework emit the enum schema automatically; update any callers/serializers to use RankingPeriod if you choose the enum approach.apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/step/WeeklyRankDeleteTasklet.kt (2)
14-19:Tasklet에@StepScope가 굳이 필요한지 재검토한다.해당 태스크릿은 JobParameters/StepExecutionContext에 의존하지 않고 고정 SQL만 실행한다.
@StepScope는 프록시 생성과 스텝 생명주기 바인딩 비용만 추가할 뿐 이득이 없다. 싱글톤 컴포넌트로 충분하다.@ConditionalOnProperty만 남기면 의도(해당 잡 실행 시에만 빈 생성)가 유지된다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/step/WeeklyRankDeleteTasklet.kt` around lines 14 - 19, WeeklyRankDeleteTasklet currently has an unnecessary `@StepScope`; remove the `@StepScope` annotation from the WeeklyRankDeleteTasklet class so it becomes a regular singleton Spring bean, keep the existing `@ConditionalOnProperty` and the constructor-injected jdbcTemplate and Tasklet implementation (no other code changes required).
22-26: MV 전체 DELETE는 테이블 규모가 커지면 장시간 락/언두 로그를 유발한다.현재 요구사항(TOP 100)에서는 규모가 작지만, 운영 중 TOP N 확장·디버그용 증분 적재 등으로 행 수가 늘어나면
DELETE FROM ...단일 문은 트랜잭션 로그 증가·복제 지연·락 대기 위험이 있다. 다음을 검토한다.
TRUNCATE TABLE mv_product_rank_weekly로 전환(자동 커밋 + 메타데이터 연산, InnoDB에서 훨씬 빠름). 단 FK/트리거 제약 여부 확인 필요.- 유지할 경우 청크 단위 DELETE(
LIMIT) + 반복을 적용하거나 파티셔닝 도입.또한 DELETE와 후속 INSERT가 별도 스텝이므로 실패 시점에 따라 MV가 빈 상태로 방치될 수 있다(주간 배치 실패 → 사용자 응답이 빈 페이지). 운영 관점에서 실패 시 롤백/재시도 전략(예: staging 테이블에 적재 후 rename swap)을 한 번 더 고민해본다.
추가 테스트: 대량 건수(예: 10만 건) 시 스텝 타임아웃/실패 처리 시나리오를 로컬 부하 테스트로 확인한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/step/WeeklyRankDeleteTasklet.kt` around lines 22 - 26, The current WeeklyRankDeleteTasklet.execute uses a single jdbcTemplate.update("DELETE FROM mv_product_rank_weekly") which can cause long locks/undo log growth; change this to a safer strategy: either (1) use TRUNCATE TABLE mv_product_rank_weekly if there are no FK constraints/triggers and autocommit behavior is acceptable (verify FK/triggers first), or (2) implement a chunked delete loop in WeeklyRankDeleteTasklet.execute that repeatedly issues limited deletes (e.g., DELETE ... LIMIT N) via jdbcTemplate until zero rows are removed, committing between chunks to avoid long transactions; additionally consider switching the flow to a staging-table swap (load new rows into mv_product_rank_weekly_staging then RENAME/atomic swap) to prevent the MV being left empty on failure and add retry/rollback handling around the step.apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/RankingV1ApiE2ETest.kt (2)
261-291: 주간/월간 MV 테스트가 기간 경계(boundary)와 페이징을 검증하지 않는다.운영 관점에서 주간/월간 전환은 "기간 파라미터 → 올바른 데이터 소스" 계약이 핵심이므로, 단순 정렬 검증만으로는 회귀를 놓친다. 다음 케이스를 추가한다.
period=weekly호출 시 Redis(일일) ZSET을 시드해도 응답에 포함되지 않는다(소스 분기 검증).period=monthly에 101건 이상을 시드해 TOP 100 제한 및 페이징(page=1)이 동작한다.period대소문자 혼용(Weekly,MONTHLY) 동작 명세화(컨트롤러parsePeriod가uppercase()처리하므로 허용되는 것으로 보이는데, 스펙 문서(daily|weekly|monthly)와 불일치하지 않는지 확인 후 정책을 테스트로 고정한다).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/RankingV1ApiE2ETest.kt` around lines 261 - 291, Add tests to verify period boundary and paging: 1) for weekly ensure seeding daily Redis ZSET (use seedMvDaily or equivalent) does not affect the response from the RankingV1Api when calling "$ENDPOINT?period=weekly" by asserting only weekly-seeded items appear; 2) for monthly seed >100 items (use registerProduct in a loop and seedMvMonthly) and assert top-100 limit and that page=1 returns the next items (verify sizes and ordering); and 3) add a case-insensitivity test for period strings (e.g., "Weekly", "MONTHLY") to confirm controller.parsePeriod behavior is fixed/expected; reference the existing test helpers registerProduct, seedMvWeekly, seedMvDaily/seedMvMonthly and controller.parsePeriod to locate code to extend.
341-353: MV 시드를 raw SQL로 하드코딩하면 스키마 변경 시 테스트가 조용히 깨진다.
mv_product_rank_weekly/mv_product_rank_monthly스키마(컬럼명, NOT NULL 제약, 추가 컬럼)가 향후 변경되면 E2E 테스트만 SQL 레벨에서 실패한다. 운영 관점에서 E2E는 도메인 경로(리포지토리/배치)를 통해 상태를 준비하는 편이 회귀를 더 안전하게 잡는다. 최소한MvProductRankWeeklyJpaRepository/MvProductRankMonthlyJpaRepository를 주입받아 엔티티 저장으로 대체하는 것을 검토한다. 또한 현재 쿼리는deleted_at등 추후 추가될 수 있는 NOT NULL 컬럼에 취약하므로, INSERT 대상 컬럼이 최소 집합임을 주석이나@Sql로 명시하는 것도 고려한다.추가 테스트 제안: JPA 리포지토리 기반 시드로 전환 후, 해당 리포지토리의 단위 테스트에서 컬럼 매핑을 별도 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/RankingV1ApiE2ETest.kt` around lines 341 - 353, The tests use raw SQL in seedMvWeekly and seedMvMonthly via jdbcTemplate inserting into mv_product_rank_weekly/monthly which will break silently on schema changes; replace these methods to inject and use MvProductRankWeeklyJpaRepository and MvProductRankMonthlyJpaRepository (create and save the appropriate MvProductRankWeekly / MvProductRankMonthly entities) so seeding goes through JPA/domain model, or if you cannot switch immediately, at minimum document the exact inserted columns and/or move the SQL into an `@Sql` file that explicitly lists columns to avoid NOT NULL/added-column breakage; update calls to remove jdbcTemplate usage and ensure tests import the repositories and use repository.save(...) instead of direct INSERT.apps/commerce-batch/src/test/kotlin/com/loopers/job/monthlyrank/MonthlyRankJobE2ETest.kt (1)
48-77: 주간 테스트와 동일하게 DDL 중복 관리 문제가 있다.
CREATE TABLE IF NOT EXISTS방식은 WeeklyRankJobE2ETest와 중복이며 스키마 드리프트 위험이 있다. 공통 테스트 유틸(또는 Flyway/Liquibase 마이그레이션) 한 곳으로 일원화한다. 상세 근거는WeeklyRankJobE2ETest코멘트 참조.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/kotlin/com/loopers/job/monthlyrank/MonthlyRankJobE2ETest.kt` around lines 48 - 77, The duplicated inline DDL inside MonthlyRankJobE2ETest.createTablesIfNotExists (which creates product_metrics and mv_product_rank_monthly) should be removed and replaced with a single centralized setup: either call the shared test utility used by WeeklyRankJobE2ETest or invoke the Flyway/Liquibase migration runner to create those tables; update MonthlyRankJobE2ETest to delegate table creation to that common helper (or migration) so schema creation for product_metrics and mv_product_rank_monthly is maintained in one place and avoids drift.apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/step/MonthlyRankDeleteTasklet.kt (1)
1-27: WeeklyRankDeleteTasklet과 동일 구조의 중복 — 공통화한다.두 태스크릿은 테이블명/로그 문구만 다르고 로직이 동일하다. 운영 관점에서 향후 DELETE 전략(예: TRUNCATE 전환, 청크 DELETE, staging swap)을 바꿀 때 두 곳을 모두 수정해야 해 누락 위험이 있다. 다음처럼 파라미터화된 공통 태스크릿 또는 팩토리 메서드로 통합을 권한다.
class MvRankDeleteTasklet( private val jdbcTemplate: JdbcTemplate, private val tableName: String, private val logLabel: String, ) : Tasklet { /* ... */ }또한 주간 태스크릿에 남긴
@StepScope불필요성, TRUNCATE 대안, 실패 시 MV 공백 노출 리스크도 동일하게 적용된다(주간 태스크릿 코멘트 참조).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/step/MonthlyRankDeleteTasklet.kt` around lines 1 - 27, Create a single parameterized Tasklet class (e.g., MvRankDeleteTasklet) and replace MonthlyRankDeleteTasklet (and the analogous WeeklyRankDeleteTasklet) with bean definitions that supply tableName and logLabel to it; implement MvRankDeleteTasklet to accept JdbcTemplate, tableName, and logLabel, call jdbcTemplate.update("DELETE FROM " + tableName) and log using logLabel, keep `@StepScope` on the parameterized tasklet or on factory methods that produce it, and update job/step configuration to instantiate MvRankDeleteTasklet with "mv_product_rank_monthly" and the existing Korean log label so both monthly and weekly steps reuse the same logic.apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/WeeklyRankJobConfig.kt (1)
1-127: 주간/월간 배치 설정 중복 제거를 권장한다.
WeeklyRankJobConfig와MonthlyRankJobConfig는 SQL 상수, 스텝 구성, 리더/라이터 구조가 거의 동일하다. 기간(Period) 파라미터와 날짜 범위 계산 전략만 다르므로, 공통 추상(예:AbstractRankJobConfig또는RankRangeResolver전략 + 공통 팩토리)을 추출하면 다음 운영 이점이 있다.
- SQL 수정(예: 위 동점 정렬 키 추가, score 가중치 변경)을 한 곳에서만 반영.
- 추후 "일 단위 재계산 배치" 등 기간이 추가될 때 중복 코드 없이 확장.
- 테스트 복잡도 감소(범위 계산만 단위 테스트로 분리 가능).
추가 테스트: 추출된
RankRangeResolver에 대해 주/월/임의 기간 각각의 경계 일자 계산 단위 테스트를 둔다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/WeeklyRankJobConfig.kt` around lines 1 - 127, Duplicate setup between WeeklyRankJobConfig and MonthlyRankJobConfig should be refactored: create an abstract base (e.g., AbstractRankJobConfig) that holds shared constants (READER_SQL, WRITER_SQL, CHUNK_SIZE), common beans (weekly/monthly chunk step creation logic using chunk<RankRow,RankRow>, weeklyRankWriter() logic), and job wiring (JobBuilder start/next/listener), and delegate period-specific behavior (job name constant, delete step bean name, and the date-range calculation used in weeklyRankReader()/monthlyRankReader()) to subclasses via an abstract method or a RankRangeResolver strategy; move the row mapping and item writer creation into the base (retain RankRow data class), and have WeeklyRankJobConfig and MonthlyRankJobConfig only implement the period-specific preparedStatementSetter values (start/end dates) and supply their JOB_NAME and DELETE_STEP_NAME.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankRepository.kt`:
- Line 3: MvProductRankRepository currently imports
com.loopers.infrastructure.catalog.RankedProductEntry causing a dependency
inversion; move the RankedProductEntry data type into the domain package
com.loopers.domain.ranking and update all infra/application implementations to
depend on the new domain type. Specifically: create/move RankedProductEntry into
com.loopers.domain.ranking, change the import in the MvProductRankRepository
interface to reference the new domain type, then update
MvProductRankRepositoryImpl, ProductRankRedisReader, and UserGetRankingUseCase
to import com.loopers.domain.ranking.RankedProductEntry and use that type;
run/adjust tests to ensure the Redis implementation compiles against the
relocated type.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.kt`:
- Around line 9-10: The ORDER BY in
MvProductRankMonthlyJpaRepository.findAllOrderByScoreDesc is unstable because it
only sorts by m.score DESC; change the JPQL to include a deterministic
tiebreaker (e.g., add ", m.id ASC" or ", m.productId ASC") so
MvProductRankMonthlyModel rows with equal scores have a consistent order across
pages, and update/ add an E2E test that calls findAllOrderByScoreDesc(pageable)
for a dataset with many identical scores to assert no duplicates or omissions
across paginated pages.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankRepositoryImpl.kt`:
- Around line 14-25: getWeeklyRanking currently computes 0-based ranks (rank =
offset + index); change it to 1-based by adding +1 (i.e., rank = offset + index
+ 1) and apply the identical fix in the monthly ranking method
(getMonthlyRanking) where rank is computed, and in ProductRankRedisReader
replace rank = start + index with rank = start + index + 1; also add E2E
assertions that the first item of page=0 has rank == 1 and the first item of
page=1 has rank == size + 1 to validate the off-by-one fix.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1Controller.kt`:
- Around line 31-38: The period query handling is inconsistent with date: when
clients send ?period= (empty string) parsePeriod("") returns an error while
parseDate treats blank as missing; to fix, make empty period behave like the
default "daily" by either updating parsePeriod to check for
isNullOrBlank/isBlank and return the DAILY period (preferred) or normalize the
value before calling parsePeriod in RankingV1Controller (e.g., treat
period.isBlank() as "daily"); update the parsePeriod function (or the
controller's request-param handling) so that parsePeriod/RankingV1Controller
treat blank strings the same as a missing parameter and reference the
parsePeriod function and the GetRankingCriteria creation in RankingV1Controller
to locate the change.
- Around line 53-59: In parsePeriod, avoid swallowing the
IllegalArgumentException: when RankingPeriod.valueOf(raw.uppercase()) fails,
wrap and rethrow a CoreException including the original exception as the cause
(or at minimum log e) so the stack/context is preserved; also use
raw.uppercase(Locale.ROOT) to avoid locale issues and validate/handle blank or
whitespace-only inputs (return a BAD_REQUEST CoreException with a clear message)
so empty/whitespace and mixed-case inputs are spec'd and handled consistently.
Ensure references: parsePeriod, RankingPeriod.valueOf, CoreException,
ErrorType.BAD_REQUEST.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/step/MetricsCleanupTasklet.kt`:
- Around line 28-33: MetricsCleanupTasklet uses LocalDate.now() which depends on
the JVM default timezone and can cause cutoff-date drift and concurrency issues
with other jobs; change MetricsCleanupTasklet to accept an injected Clock (use
LocalDate.now(clock)) so the timezone is explicit, ensure the cleanup job is
scheduled or wired to run after WeeklyRankJobConfig/MonthlyRankJobConfig readers
and ProductMetricsSyncJobConfig upserts complete (enforce ordering via the
scheduler or job Flow), and add a distributed lock or mutual-exclusion mechanism
at the job level to prevent concurrent DELETE/READ/UPSERT conflicts; also add
unit tests that construct Clocks with UTC and KST to validate cutoffDate
calculation.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/MonthlyRankJobConfig.kt`:
- Around line 42-50: The SQL in READER_SQL inside MonthlyRankJobConfig.kt uses
only ORDER BY score DESC which yields nondeterministic ordering for tied scores;
update the query to add a deterministic secondary sort key (e.g., ORDER BY score
DESC, product_id ASC) so ties break consistently, and then run the suggested
test: insert many equal-score products and execute the job twice to confirm the
MV row set and order are stable.
- Around line 88-108: monthlyRankReader currently computes LocalDate.now() at
bean creation so it’s a singleton-bound fixed date; change monthlyRankReader to
a `@StepScope` bean and inject a JobParameter (e.g., targetDate or
targetDateMillis) to compute the date range at step runtime (use
ZoneId.of("Asia/Seoul") or accept an explicit timezone param) instead of
LocalDate.now(), then use that runtime-derived startOfMonth and endOfMonth in
the preparedStatementSetter for READER_SQL; also update any WRITER_SQL usage to
accept an application-provided timestamp/parameter instead of relying on DB
NOW() so both reader (monthlyRankReader) and writer use the same injected target
date/time.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/WeeklyRankJobConfig.kt`:
- Around line 89-109: weeklyRankReader currently evaluates LocalDate.now() at
bean creation (singleton) and computes a start/end that yields week-to-date;
change weeklyRankReader to be step-scoped and compute dates at execution time
(same approach used in MonthlyRankJobConfig): annotate weeklyRankReader with
`@StepScope` (or otherwise defer creation), accept an execution reference date via
JobParameters or a Clock injected at runtime, compute the completed-week range
for Intent A by taking
referenceDate.with(previousOrSame(DayOfWeek.SUNDAY)).minusWeeks(1) as start and
start.plusDays(6) (or start.plusWeeks(1).minusDays(1)) as end, then pass those
two dates to the preparedStatementSetter instead of the current
startOfWeek/today values; update the SQL parameter binding in weeklyRankReader's
preparedStatementSetter accordingly and add tests that run on different weekdays
to verify the same previous-week (Sunday–Saturday) range is used.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/job/metricscleanup/MetricsCleanupJobE2ETest.kt`:
- Around line 48-65: The test currently defines DDL inline in
createTableIfNotExists() inside MetricsCleanupJobE2ETest which risks schema
drift and mismatches with the real ProductMetrics JPA entity/Flyway migrations
(e.g., indexes, nullability, soft-delete semantics); replace the inline CREATE
TABLE by configuring the test to use the application schema (enable JPA
ddl-auto=create-drop for tests or run Flyway migrations in the test container)
and change createTableIfNotExists()/jdbcTemplate usage to only insert/clean test
data, and add an explicit test that verifies soft-delete behavior against the
ProductMetrics entity (i.e., assert whether deleted_at rows are
excluded/included according to the entity’s soft-delete configuration).
- Around line 87-118: Add a boundary-case metric dated exactly 60 days ago to
the shouldDeleteOldDataAndPreserveRecent test: call seedMetrics with a new
PRODUCT_ID (e.g., PRODUCT_ID_3) and date = LocalDate.now().minusDays(60), then
launch the job and update assertions to expect the boundary record to be
preserved (adjust the COUNT expectation and assert that returned product_ids
include both PRODUCT_ID_2 and the boundary PRODUCT_ID_3 while PRODUCT_ID_1 (61
days) is deleted); modify queries/assertions in
shouldDeleteOldDataAndPreserveRecent to reflect these expectations so the
cutoff-exclusive behavior is validated.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/job/monthlyrank/MonthlyRankJobE2ETest.kt`:
- Around line 144-161: The test only asserts row count after two runs; update
MonthlyRankJobE2ETest.shouldBeIdempotent to capture a snapshot of the row(s)
after the first job run (querying mv_product_rank_monthly for product_id and
score) and then run the job a second time and re-query the same columns,
asserting the product_id and score values are equal to the snapshot; use the
existing seedMetrics and jobLauncherTestUtils.launchJob calls and compare
results fetched via jdbcTemplate (select product_id, score from
mv_product_rank_monthly) instead of only COUNT(*) to ensure the score is
unchanged across runs.
- Around line 125-142: The test should avoid relying on LocalDate.now() by
fixing the job's reference month via JobParameters or an injectable Clock;
update the shouldExcludeDataOutsideMonthRange test to supply a deterministic
targetMonth to jobLauncherTestUtils (use the existing jobParameters() helper to
add "targetMonth") or make the batch read an injected Clock and set a fixed
clock in the test, then seed both lastMonth
(LocalDate.of(targetYear,targetMonth, lastDay)) and a future date (next month's
1st) via seedMetrics(PRODUCT_ID_1, ...), launch the job, and assert
mv_product_rank_monthly count is 0; reference the test method
shouldExcludeDataOutsideMonthRange, jobParameters(), and seedMetrics(...) when
making changes.
- Around line 100-123: The test should assert deterministic aggregate results
instead of only hasSizeGreaterThanOrEqualTo(2): after calling cleanUp() and
seeding via seedMetrics(...) in shouldAggregateMonthlyMetricsIntoMv(), assert
jobExecution.exitStatus is COMPLETED and then query mv_product_rank_monthly
ORDER BY score DESC to assert exact row count, exact top product_id(s) and their
score numeric values (matching the 0.1/0.2/0.7 weighting) to catch weight
regressions; also add a new test that seeds >100 distinct products and asserts
only the top 100 rows are written to the MV (verifying the monthly MV cap).
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/job/weeklyrank/WeeklyRankJobE2ETest.kt`:
- Around line 51-80: The test currently creates tables directly via
createTablesIfNotExists (using jdbcTemplate and "CREATE TABLE IF NOT EXISTS"),
which risks schema drift; remove or disable that method and instead ensure tests
use the same migration path as production by configuring the test profile to run
Flyway/Liquibase or JPA ddl-auto=create-drop, or apply the same migration SQL
via Testcontainers init scripts so WeeklyRankJobE2ETest uses the real
migrations; update test setup to run migrations before tests (or mount migration
scripts into the container), and optionally add a small assertion comparing
information_schema.columns to the production migration to lock column parity.
- Around line 132-149: The test should not rely on LocalDate.now() across two
launchJob calls; make the job use an injected Clock (or accept a date via
JobParameters) and fix the test to supply a deterministic date: replace usages
of LocalDate.now() in the test (seedMetrics(...) and jobParameters()) with a
fixed LocalDate (e.g., LocalDate.of(...)) or supply a fixed Clock to the
WeeklyRankJob implementation and in shouldBeIdempotent() pass that fixed Clock
so both jobLauncherTestUtils.launchJob(jobParameters()) calls run against the
same week boundary deterministically.
- Around line 103-130: The test uses a loose assert
hasSizeGreaterThanOrEqualTo(2) and a date-dependent branch that makes it flaky;
change assertions to expect exactly two rows and verify ordering and exact score
values (compute expectedScore = view*0.1 + like*0.2 + order*0.7 for PRODUCT_ID_1
and PRODUCT_ID_2 and assert results[0]["product_id"], results[0]["score"],
results[1]["product_id"], results[1]["score"] match the expected order and
numeric scores), and remove the if (today != startOfWeek) variability by
injecting a fixed date into the test run (use a Clock or pass a deterministic
job parameter used by seedMetrics and
jobLauncherTestUtils.launchJob/jobParameters) so seedMetrics(PRODUCT_ID_1, ...),
seedMetrics(PRODUCT_ID_2, ...) always seed the same days.
- Around line 151-168: The test shouldExcludeDataOutsideWeekRange uses a brittle
date calculation (previousOrSame(SUNDAY).minusDays(1)) that only seeds last
Saturday and breaks on certain "today" values; change it to explicitly seed
boundary dates outside the target week using clear helpers: compute
lastWeekStart =
LocalDate.now().with(TemporalAdjusters.previousOrSame(DayOfWeek.SUNDAY)).minusWeeks(1)
(the earliest day of last week) and nextWeekStart =
LocalDate.now().with(TemporalAdjusters.next(DayOfWeek.SUNDAY)) (the first day of
next week), then call seedMetrics(PRODUCT_ID_1, lastWeekStart, ...) and
seedMetrics(PRODUCT_ID_1, nextWeekStart, ...) (or add a separate seed for each)
and keep the same assertions to ensure mv_product_rank_weekly remains empty;
replace the previous minusDays(1) logic in shouldExcludeDataOutsideWeekRange
with these explicit dates to avoid flakiness.
---
Outside diff comments:
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserGetRankingUseCase.kt`:
- Around line 72-82: Update the stale comment to describe that DAILY uses Redis
entries but WEEKLY/MONTHLY use materialized view (MV) data, and then change the
returned GetRankingResult so it does not always set date = criteria.date: for MV
branches (WEEKLY/MONTHLY) populate a clear period identifier (e.g.,
period/periodStart/periodEnd or the MV generation timestamp) instead of
criteria.date so the client can cache/interpret the response; finally, compute
hasNext differently per data source—keep entries.size >= criteria.size for the
DAILY (Redis entries) path but for MV paths set hasNext = (criteria.page + 1) *
criteria.size < totalCount (using totalCount determined from the MV) rather than
entries.size, and ensure you reference GetRankingResult, criteria.date, hasNext,
entries.size and totalCount when making these changes.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserRankingCriteria.kt`:
- Around line 6-11: GetRankingCriteria currently has a non-null date that is
ignored when period != RankingPeriod.DAILY, which is not obvious to callers;
update the API to make this explicit by either (A) adding KDoc above
GetRankingCriteria explaining "date is only used when period == DAILY", or (B)
refactor the type into a sealed hierarchy (e.g., DailyCriteria(date: LocalDate),
WeeklyCriteria, MonthlyCriteria) or change date to nullable (date: LocalDate?)
and validate in UserGetRankingUseCase depending on period; reference
GetRankingCriteria, UserGetRankingUseCase and RankingPeriod when implementing
the chosen change and add validation/logging to surface misuse.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankMonthlyModel.kt`:
- Around line 8-22: The MV table lacks a DB-level unique constraint on
product_id which can lead to duplicate rows; update the
MvProductRankMonthlyModel class's `@Table` annotation to include
UniqueConstraint(columnNames = ["product_id"]) and add a corresponding DB
migration/DDL change to enforce uniqueness at the database level; ensure the
batch insert logic (the Tasklet DELETE + Chunk INSERT) is compatible with the
constraint (add idempotent upsert/replace or handle unique-violation errors if
necessary) and strengthen the batch E2E by adding a test that runs the
monthly/weekly job twice in succession and asserts the row count (and top-100
results) remain unchanged.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankRepositoryImpl.kt`:
- Around line 14-46: Extract the duplicated paging→query→map flow in
getWeeklyRanking and getMonthlyRanking into a single private helper (e.g.,
fetchRankingPage or mapRankingPage) that takes a Pageable and a repository
accessor (or a function reference to findAllOrderByScoreDesc) and returns
List<RankedProductEntry>; inside the helper compute offset, call the provided
repository method (weeklyJpaRepository.findAllOrderByScoreDesc or
monthlyJpaRepository.findAllOrderByScoreDesc), then mapIndexed to build
RankedProductEntry(productId, score, rank = offset + index). Replace
getWeeklyRanking and getMonthlyRanking to create the PageRequest and call the
new helper with the appropriate repository/function. Ensure helper is reused to
remove the off-by-one duplication and keep
getWeeklyTotalCount/getMonthlyTotalCount unchanged.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.kt`:
- Around line 11-18: Add structured Swagger metadata for the period parameter on
the getRanking method: either annotate the period parameter with
`@Parameter`(schema = `@Schema`(allowableValues = ["daily","weekly","monthly"],
defaultValue = "daily")) so Swagger shows an enum dropdown and default, or
change the parameter type from String to a new or existing RankingPeriod enum
(values daily, weekly, monthly) and let the framework emit the enum schema
automatically; update any callers/serializers to use RankingPeriod if you choose
the enum approach.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/RankingV1ApiE2ETest.kt`:
- Around line 261-291: Add tests to verify period boundary and paging: 1) for
weekly ensure seeding daily Redis ZSET (use seedMvDaily or equivalent) does not
affect the response from the RankingV1Api when calling "$ENDPOINT?period=weekly"
by asserting only weekly-seeded items appear; 2) for monthly seed >100 items
(use registerProduct in a loop and seedMvMonthly) and assert top-100 limit and
that page=1 returns the next items (verify sizes and ordering); and 3) add a
case-insensitivity test for period strings (e.g., "Weekly", "MONTHLY") to
confirm controller.parsePeriod behavior is fixed/expected; reference the
existing test helpers registerProduct, seedMvWeekly, seedMvDaily/seedMvMonthly
and controller.parsePeriod to locate code to extend.
- Around line 341-353: The tests use raw SQL in seedMvWeekly and seedMvMonthly
via jdbcTemplate inserting into mv_product_rank_weekly/monthly which will break
silently on schema changes; replace these methods to inject and use
MvProductRankWeeklyJpaRepository and MvProductRankMonthlyJpaRepository (create
and save the appropriate MvProductRankWeekly / MvProductRankMonthly entities) so
seeding goes through JPA/domain model, or if you cannot switch immediately, at
minimum document the exact inserted columns and/or move the SQL into an `@Sql`
file that explicitly lists columns to avoid NOT NULL/added-column breakage;
update calls to remove jdbcTemplate usage and ensure tests import the
repositories and use repository.save(...) instead of direct INSERT.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/MetricsCleanupJobConfig.kt`:
- Around line 32-48: The job currently uses RunIdIncrementer in
metricsCleanupJob which creates a new job instance each run (preventing restart
semantics); decide whether to remove/replace RunIdIncrementer or document/emit
restart-aware metrics if you need resume behavior, and in
metricsCleanupStep/metricsCleanupTasklet instrument the heavy DELETE work with
Micrometer (register a Counter for deleted records and a Timer or LongTaskTimer
for duration) and emit those metrics from metricsCleanupTasklet (and/or via
stepMonitorListener) so operators can monitor delete counts and execution time;
also ensure the tasklet is idempotent and consider chunking/transaction
boundaries or adjusting transactionManager/step configuration to avoid
transaction/timeouts during large-batch deletes.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/step/MetricsCleanupTasklet.kt`:
- Around line 27-35: The current execute method in MetricsCleanupTasklet
performs a single large DELETE via jdbcTemplate.update("DELETE FROM
product_metrics WHERE date < ?", cutoffDate) which can cause long transactions,
replication lag, locks and rollbacks for millions of rows; change it to a
chunked-delete loop that repeatedly issues limited-size deletes (e.g., DELETE
FROM product_metrics WHERE date < ? LIMIT DELETE_BATCH_SIZE) until the returned
deletedCount for a batch is zero, logging each batch and the total deleted, and
ensure DELETE_BATCH_SIZE and RETENTION_DAYS are configurable; also verify the
date column has an appropriate covering index to make the batched deletes
efficient.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/step/MonthlyRankDeleteTasklet.kt`:
- Around line 1-27: Create a single parameterized Tasklet class (e.g.,
MvRankDeleteTasklet) and replace MonthlyRankDeleteTasklet (and the analogous
WeeklyRankDeleteTasklet) with bean definitions that supply tableName and
logLabel to it; implement MvRankDeleteTasklet to accept JdbcTemplate, tableName,
and logLabel, call jdbcTemplate.update("DELETE FROM " + tableName) and log using
logLabel, keep `@StepScope` on the parameterized tasklet or on factory methods
that produce it, and update job/step configuration to instantiate
MvRankDeleteTasklet with "mv_product_rank_monthly" and the existing Korean log
label so both monthly and weekly steps reuse the same logic.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/step/WeeklyRankDeleteTasklet.kt`:
- Around line 14-19: WeeklyRankDeleteTasklet currently has an unnecessary
`@StepScope`; remove the `@StepScope` annotation from the WeeklyRankDeleteTasklet
class so it becomes a regular singleton Spring bean, keep the existing
`@ConditionalOnProperty` and the constructor-injected jdbcTemplate and Tasklet
implementation (no other code changes required).
- Around line 22-26: The current WeeklyRankDeleteTasklet.execute uses a single
jdbcTemplate.update("DELETE FROM mv_product_rank_weekly") which can cause long
locks/undo log growth; change this to a safer strategy: either (1) use TRUNCATE
TABLE mv_product_rank_weekly if there are no FK constraints/triggers and
autocommit behavior is acceptable (verify FK/triggers first), or (2) implement a
chunked delete loop in WeeklyRankDeleteTasklet.execute that repeatedly issues
limited deletes (e.g., DELETE ... LIMIT N) via jdbcTemplate until zero rows are
removed, committing between chunks to avoid long transactions; additionally
consider switching the flow to a staging-table swap (load new rows into
mv_product_rank_weekly_staging then RENAME/atomic swap) to prevent the MV being
left empty on failure and add retry/rollback handling around the step.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/WeeklyRankJobConfig.kt`:
- Around line 1-127: Duplicate setup between WeeklyRankJobConfig and
MonthlyRankJobConfig should be refactored: create an abstract base (e.g.,
AbstractRankJobConfig) that holds shared constants (READER_SQL, WRITER_SQL,
CHUNK_SIZE), common beans (weekly/monthly chunk step creation logic using
chunk<RankRow,RankRow>, weeklyRankWriter() logic), and job wiring (JobBuilder
start/next/listener), and delegate period-specific behavior (job name constant,
delete step bean name, and the date-range calculation used in
weeklyRankReader()/monthlyRankReader()) to subclasses via an abstract method or
a RankRangeResolver strategy; move the row mapping and item writer creation into
the base (retain RankRow data class), and have WeeklyRankJobConfig and
MonthlyRankJobConfig only implement the period-specific preparedStatementSetter
values (start/end dates) and supply their JOB_NAME and DELETE_STEP_NAME.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/job/metricscleanup/MetricsCleanupJobE2ETest.kt`:
- Around line 120-140: Update the shouldCompleteWhenNothingToDelete test so it
seeds at least two recent-date metric rows and then verifies both that the total
row count remains the same and that the set of identifiers for PRODUCT_ID_1 is
unchanged after running the job; specifically, in the test function
shouldCompleteWhenNothingToDelete call seedMetrics(PRODUCT_ID_1,
LocalDate.now()) twice (or a helper to insert two rows), run
jobLauncherTestUtils.launchJob(jobParameters()), then use
jdbcTemplate.queryForObject to assert COUNT(*) == 2L and additionally query the
identifying column(s) (e.g., ids or timestamps) for PRODUCT_ID_1 and assert the
returned set equals the pre-run set to ensure no row was deleted-and-reinserted.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/job/monthlyrank/MonthlyRankJobE2ETest.kt`:
- Around line 48-77: The duplicated inline DDL inside
MonthlyRankJobE2ETest.createTablesIfNotExists (which creates product_metrics and
mv_product_rank_monthly) should be removed and replaced with a single
centralized setup: either call the shared test utility used by
WeeklyRankJobE2ETest or invoke the Flyway/Liquibase migration runner to create
those tables; update MonthlyRankJobE2ETest to delegate table creation to that
common helper (or migration) so schema creation for product_metrics and
mv_product_rank_monthly is maintained in one place and avoids drift.
🪄 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: c075c05e-618f-4bee-a6df-88e247be11d6
📒 Files selected for processing (21)
apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserGetRankingUseCase.ktapps/commerce-api/src/main/kotlin/com/loopers/application/ranking/UserRankingCriteria.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankMonthlyModel.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankRepository.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/MvProductRankWeeklyModel.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingPeriod.ktapps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.ktapps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankRepositoryImpl.ktapps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.ktapps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.ktapps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1Controller.ktapps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/RankingV1ApiE2ETest.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/MetricsCleanupJobConfig.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/metricscleanup/step/MetricsCleanupTasklet.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/MonthlyRankJobConfig.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/monthlyrank/step/MonthlyRankDeleteTasklet.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/WeeklyRankJobConfig.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/weeklyrank/step/WeeklyRankDeleteTasklet.ktapps/commerce-batch/src/test/kotlin/com/loopers/job/metricscleanup/MetricsCleanupJobE2ETest.ktapps/commerce-batch/src/test/kotlin/com/loopers/job/monthlyrank/MonthlyRankJobE2ETest.ktapps/commerce-batch/src/test/kotlin/com/loopers/job/weeklyrank/WeeklyRankJobE2ETest.kt
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
🔁 전체 흐름
사용자가 주문/조회/좋아요 등의 행위를 하면 API가 Kafka로 이벤트를 발행합니다. Streamer는 이를 소비해서 Redis ZSET에 실시간으로 반영합니다. 이 Redis ZSET이 일간 랭킹의 SoT(Single Source of Truth) 입니다.
일간 배치가 돌면서 Redis ZSET의 데이터를 product_metrics 테이블에 일별 스냅샷으로 적재합니다. 이 테이블이 주간/월간 집계의 원천 데이터가 됩니다.
product_metrics의 일별 데이터를 기반으로, 주간 배치는 이번 주 일요일부터 오늘까지, 월간 배치는 이번 달 1일부터 오늘까지의 데이터를 합산하여 각각mv_product_rank_weekly,mv_product_rank_monthly에 TOP 100을 저장합니다. 둘 다 매일 배치를 돌리며, Tasklet(DELETE) + Chunk(Reader→Writer) 구조로 멱등하게 갱신합니다.daily는 Redis ZSET에서 실시간으로 제공하고,weekly/monthly는 각각의 MV 테이블에서 배치로 미리 계산해둔 결과를 내려줍니다.🏗️ 아키텍처
flowchart TD subgraph "실시간 계층" API["API 요청"] --> Kafka --> Streamer Streamer --> ZSET["Redis ZSET<br/>(일간, SoT)"] end subgraph "배치 계층" ZSET -->|"일간 배치<br/>(Tasklet)"| PM["product_metrics<br/>(일별 데이터)"] PM -->|"주간 배치 (매일)<br/>(Tasklet+Chunk)"| MW["mv_product_rank_weekly<br/>(TOP 100)"] PM -->|"월간 배치 (매일)<br/>(Tasklet+Chunk)"| MM["mv_product_rank_monthly<br/>(TOP 100)"] PM -->|"정리 배치<br/>(Tasklet)"| DEL["60일 이전 삭제"] end subgraph "API 조회" C["GET /api/v1/rankings?period="] C -->|"daily"| ZSET C -->|"weekly"| MW C -->|"monthly"| MM end style ZSET fill:#ffecb3 style PM fill:#e3f2fd style MW fill:#c8e6c9 style MM fill:#c8e6c9배치 Job 구조 (주간 기준)
sequenceDiagram autonumber participant Job as weeklyRankJob participant S1 as Step 1 (Tasklet) participant DB as MySQL participant S2 as Step 2 (Chunk) Job->>S1: 실행 S1->>DB: DELETE FROM mv_product_rank_weekly Note over S1,DB: 100건 DELETE — 딱 한 번 Job->>S2: 실행 S2->>DB: SELECT ... GROUP BY product_id<br/>ORDER BY score DESC LIMIT 100 Note over S2,DB: JdbcCursorItemReader — 커서로 한 건씩 loop chunk 단위 (100건) S2->>DB: INSERT INTO mv_product_rank_weekly end Note over S2,DB: Writer는 INSERT만 — chunk 반복해도 안전API 분기 흐름
sequenceDiagram autonumber participant Client participant Controller participant UseCase participant Redis as Redis ZSET participant WeeklyDB as mv_weekly participant MonthlyDB as mv_monthly Client->>Controller: GET /rankings?period=weekly Controller->>UseCase: execute(criteria) alt period = DAILY UseCase->>Redis: getRankingPage(date, page, size) else period = WEEKLY UseCase->>WeeklyDB: getWeeklyRanking(page, size) else period = MONTHLY UseCase->>MonthlyDB: getMonthlyRanking(page, size) end UseCase->>UseCase: Product/Brand bulk 조회 + 응답 조립 UseCase-->>Controller: GetRankingResult Controller-->>Client: ApiResponse🧭 리뷰 포인트
1. 매일 REDIS에서 일간 데이터를 읽어서 주간 MV 누적 → TOP 100 데이터를 못 만듦
처음에는 단순히 매일 배치가 Redis 일간 ZSET을 읽어서 주간 MV에 누적하면 되겠다고 생각했습니다.
그런데 quest에서 "MV에 TOP 100만 저장하라"는 요구사항이 있었습니다.
매일 누적하면서 TOP 100만 남기면, 다음 날 순위가 올라온 상품의 기존 누적 데이터가 이미 삭제되어 있습니다. 합산할 기반이 없어집니다.
→ 매일 누적이 아니라, 기간 전체를 한번에 합산해야 한다는 결론.
→ REDIS의 ZSET TTL은 2일로 설정했는데 7일치 일별 데이터를 어디서 읽어오지?
2. 기존의
product_metrics테이블에 date 컬럼 추가일별 데이터가 필요하기 때문에 일별 스냅샷을 따로 두기로 했습니다. 하지만 기존의
product_metrics에 date만 추가하면 일별 스냅샷 테이블이 되고, 이전 주차의 설계가 잘못 되었음을 알았습니다. 기존product_metrics는product_idUNIQUE로 상품당 전체 누적치만 저장하고 있었습니다.3. DELETE 후 INSERT 방시으로 멱등 처리
주간 배치를 주 1회만 돌리면, 이번 주에는 지난주 확정 데이터만 보게 됩니다. 또, "이번 주 현재까지의 TOP 100"을 보여주려면 매일 배치를 돌려야 하는데, UPSERT(
val = val + ?)는 재실행 시 값이 두 배가 됩니다.DELETE 후 INSERT 방식이면?
몇 번을 돌려도 결과가 동일합니다. MV에 저장되는 데이터가 최대 100건이라 DELETE/INSERT 비용도 무시할 수준입니다.
이 때, Chunk-Oriented에서 Writer는 chunk 단위로 반복 호출됩니다. chunk size 50으로 100건을 처리하면 Writer가 2번 호출되는데 두번 째 chunk에서는 첫 번째의 50건이 소실됩니다.
따라서 Writer에서 DELETE를 하지 않고 Job을 2개 Step으로 분리해 DELETE를 한 번만 실행되도록 했습니다.
flowchart LR subgraph "weeklyRankJob" S1["Step 1<br/>Tasklet<br/>DELETE FROM<br/>mv_product_rank_weekly"] S2["Step 2<br/>Chunk<br/>Reader(SQL GROUP BY)<br/>→ Writer(INSERT)"] S1 --> S2 endStep 1(Tasklet)에서 DELETE를 딱 한 번, Step 2(Chunk)에서는 INSERT만하면 이 구조면 Writer가 몇 번 호출되든 안전합니다.
📊 최종 설계 결정 요약
📁 변경 파일
배치 Job (commerce-batch) — 신규 6개
weeklyrank/WeeklyRankJobConfig.ktweeklyrank/step/WeeklyRankDeleteTasklet.ktmonthlyrank/MonthlyRankJobConfig.ktmonthlyrank/step/MonthlyRankDeleteTasklet.ktmetricscleanup/MetricsCleanupJobConfig.ktmetricscleanup/step/MetricsCleanupTasklet.ktMV 엔티티 + Repository (commerce-api) — 신규 7개
domain/ranking/RankingPeriod.ktdomain/ranking/MvProductRankWeeklyModel.ktdomain/ranking/MvProductRankMonthlyModel.ktdomain/ranking/MvProductRankRepository.ktinfrastructure/ranking/MvProductRankWeeklyJpaRepository.ktinfrastructure/ranking/MvProductRankMonthlyJpaRepository.ktinfrastructure/ranking/MvProductRankRepositoryImpl.ktAPI 확장 (commerce-api) — 수정 4개
RankingV1Controller.ktperiod파라미터 추가 (기본값: daily)RankingV1ApiSpec.ktUserRankingCriteria.ktperiod: RankingPeriod필드 추가UserGetRankingUseCase.ktwhen(period)분기 — daily→Redis, weekly/monthly→DB테스트 — 신규 3개 + 수정 1개
WeeklyRankJobE2ETest.ktMonthlyRankJobE2ETest.ktMetricsCleanupJobE2ETest.ktRankingV1ApiE2ETest.kt✅ 테스트 결과
변경 목적
기존 일일 랭킹(Redis ZSET)에 주간/월간 랭킹 지원을 추가하고, 배치 작업으로 일일 스냅샷 데이터(product_metrics)를 집계하여 물화 뷰(MV) 생성 및 오래된 데이터 정리
핵심 변경점
리스크/주의사항
테스트/검증
E2E 테스트로 배치 완료, MV 데이터 정렬 검증, 멱등성(중복 실행 시 결과 동일), 기간 밖 데이터 제외, API period 파라미터 정상 동작 및 invalid 값 400 에러 반환 확인