[volume-10] 랭킹 시스템 구현#392
Open
najang wants to merge 6 commits intoLoopers-dev-lab:najangfrom
Open
Conversation
(product_id, date) 복합 unique 제약으로 일별 단위 집계가 가능하도록 구조 전환. findByProductId → findByProductIdAndDate 로 변경하여 Consumer upsert도 date 기반으로 수정. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mv_product_rank_weekly / mv_product_rank_monthly 테이블에 매핑되는 도메인 모델과 Repository 인터페이스, JpaRepository 구현체를 추가. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
product_metrics를 집계하여 mv_product_rank_weekly / mv_product_rank_monthly를 갱신하는 WeeklyRankingJob, MonthlyRankingJob을 Spring Batch로 구현. RankingScoreCalculator로 like/order 가중치 점수 계산 로직 분리. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /api/v1/rankings?period=weekly|monthly 로 MV 기반 집계 랭킹 조회 지원. period 미입력 시 기존 daily(Redis ZSET) 동작 유지. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SLF4J placeholder 형식으로 로그 수정, afterJob에서 ExitStatus.FAILED 감지 시 에러 로그 출력 및 추후 Slack 알람 연동을 위한 TODO 추가. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WeeklyRankingJobE2ETest, MonthlyRankingJobE2ETest, RankingPeriodV1ApiE2ETest 추가. ProductMetrics date 추가에 따른 기존 streamer 테스트 수정. CommerceBatchApplicationTest에 spring.batch.job.enabled=false 설정 추가. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough주기 기반 상품 랭킹 조회 기능을 도입했다. RankingFacade는 요청된 주기(일일/주간/월간)에 따라 Redis ZSET, 주간/월간 물리화 뷰에서 랭킹 데이터를 조회한다. ProductMetrics는 상품-날짜 복합키로 변경되고, 배치 작업으로 주간/월간 집계 데이터를 생성한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as RankingV1Controller
participant Facade as RankingFacade
participant DailyRepo as Daily Repository<br/>(Redis)
participant WeeklyRepo as MvProductRankWeekly<br/>Repository
participant MonthlyRepo as MvProductRankMonthly<br/>Repository
participant ProductSvc as ProductService
Client->>Controller: GET /api/v1/rankings?period=weekly&page=0&size=20
activate Controller
Controller->>Controller: resolvedPeriod = "weekly"<br/>targetDate = null
Controller->>Facade: findRankings("weekly", null, 0, 20)
deactivate Controller
activate Facade
alt period == "weekly"
Facade->>WeeklyRepo: findTop(0, 20)
activate WeeklyRepo
WeeklyRepo-->>Facade: List<MvProductRankWeekly>
deactivate WeeklyRepo
else period == "monthly"
Facade->>MonthlyRepo: findTop(0, 20)
activate MonthlyRepo
MonthlyRepo-->>Facade: List<MvProductRankMonthly>
deactivate MonthlyRepo
else period == "daily"
Facade->>DailyRepo: findByScore(date, 0, 20)
activate DailyRepo
DailyRepo-->>Facade: List<RankingData>
deactivate DailyRepo
end
Facade->>ProductSvc: findAllByIds(productIds)
activate ProductSvc
ProductSvc-->>Facade: Map<Long, Product>
deactivate ProductSvc
Facade->>Facade: 제외된 상품 필터링 및<br/>RankingInfo 생성 (rank offset 계산)
Facade-->>Controller: List<RankingInfo>
deactivate Facade
activate Controller
Controller-->>Client: 200 OK {period, date, content, page, size}
deactivate Controller
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java (1)
31-37:⚠️ Potential issue | 🟡 MinorRankingV1ApiSpec.java 추가하여 API 스펙을 문서화해야 한다.
현재 모든 API 엔드포인트는 해당 *ApiSpec.java 인터페이스 파일로 Swagger/OpenAPI 스펙을 제공한다 (ProductV1ApiSpec, BrandV1ApiSpec, CartV1ApiSpec 등). RankingV1 엔드포인트는 이 패턴을 따르지 않고 있다. 새로 추가된
period필드는 API 응답 구조 변경이므로 명시적으로 문서화되어야 한다.다음을 수행하라:
- RankingV1ApiSpec.java 인터페이스 생성
@Tag,@Operation,@Parameter애노테이션으로 getRankings 메서드 문서화- period, date 파라미터와 응답의 period 필드를 명시적으로 기술
이를 통해 클라이언트 통합 시 API 계약이 명확해지며, 기존 패턴과의 일관성이 유지된다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java` around lines 31 - 37, Create a new RankingV1ApiSpec interface that documents the existing ranking endpoint consistent with other ApiSpec files: annotate the interface/class with `@Tag` and add a getRankings method annotated with `@Operation` and `@Parameter` for the request parameters (including period and date) and describe the response structure to include the new period field; reference the existing RankingPageResponse record and ensure the response schema documents its content, period, date, page and size fields so Swagger/OpenAPI shows the period response property and the period/date request parameters for clients integrating with getRankings.
🧹 Nitpick comments (4)
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/LikeEventConsumer.java (1)
53-56: 타임존 하드코딩은 운영 확장 시 문제가 될 수 있다.
ZoneId.of("Asia/Seoul")이 하드코딩되어 있다. 단일 리전 운영에서는 문제없으나, 다중 리전 확장 시 이벤트 날짜 계산이 불일치할 수 있다.♻️ 설정 기반 타임존 주입 제안
// application.yml 또는 환경변수로 관리 `@Value`("${app.ranking.timezone:Asia/Seoul}") private String rankingTimezone;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/LikeEventConsumer.java` around lines 53 - 56, The code hardcodes ZoneId.of("Asia/Seoul") when computing LocalDate eventDate in LikeEventConsumer (around the LocalDate eventDate assignment and processLikeEvent call); change this to use a configurable timezone injected from configuration (e.g., a new field rankingTimezone or `@Value`("${app.ranking.timezone:Asia/Seoul}") rankingTimezone) and use ZoneId.of(rankingTimezone) when converting Instant.ofEpochMilli(record.timestamp()) to LocalDate so the event date honors the configured region rather than a hardcoded "Asia/Seoul".apps/commerce-batch/src/test/resources/schema/ranking-tables.sql (1)
12-30: MV 테이블 간 기간 컬럼 네이밍이 불일치한다.
mv_product_rank_weekly는year_month_week,mv_product_rank_monthly는ranking_period를 사용한다. 이 불일치로 인해:
- 쿼리 작성 시 혼란: 동일 목적의 컬럼에 다른 이름 사용
- 유지보수 부담: 코드 리뷰 및 디버깅 시 인지 부하 증가
♻️ 일관된 네이밍 제안
CREATE TABLE IF NOT EXISTS mv_product_rank_weekly ( product_id BIGINT PRIMARY KEY, like_count INT NOT NULL, order_count INT NOT NULL, score DOUBLE NOT NULL, - year_month_week VARCHAR(10) NOT NULL, + ranking_period VARCHAR(10) NOT NULL, updated_at DATETIME NOT NULL );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/resources/schema/ranking-tables.sql` around lines 12 - 30, The two materialized view tables mv_product_rank_weekly and mv_product_rank_monthly use inconsistent column names for the period field (year_month_week vs ranking_period); rename one to match the other so both tables use a consistent column name (e.g., change year_month_week to ranking_period or vice versa) and update any dependent queries, DTOs or ETL code that reference mv_product_rank_weekly.year_month_week or mv_product_rank_monthly.ranking_period (search for these exact symbols) to the chosen unified name to avoid confusion and maintenance overhead.apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java (1)
55-61: 예외 로깅 방식에 운영 관점의 개선이 필요하다.
getAllFailureExceptions()를 직접 로깅하면 다음 문제가 발생한다:
- 로그 파싱 어려움:
List.toString()은 전체 스택 트레이스를 한 줄로 출력하여 로그 수집 도구에서 파싱이 어렵다.- 민감 정보 노출 가능성: 예외 메시지에 SQL 파라미터, 사용자 식별자 등이 포함될 수 있다.
- 중복 로깅:
StepMonitorListener.afterStep()에서 이미 동일한 예외를 로깅하고 있어 로그 볼륨이 증가한다.♻️ 개선안
if (ExitStatus.FAILED.getExitCode().equals(jobExecution.getExitStatus().getExitCode())) { - log.error("[{}] Job FAILED — ExitCode: {}, Exceptions: {}", + log.error("[{}] Job FAILED — ExitCode: {}, ExceptionCount: {}", jobExecution.getJobInstance().getJobName(), jobExecution.getExitStatus().getExitCode(), - jobExecution.getAllFailureExceptions()); + jobExecution.getAllFailureExceptions().size()); + // 개별 예외는 StepMonitorListener에서 이미 로깅되므로 여기서는 요약만 출력 // TODO: Slack 알람 연동 }🤖 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 55 - 61, 현재 JobListener의 FAILED 블록이 jobExecution.getAllFailureExceptions() 전체를 그대로 로깅하고 있어 파싱·민감정보·중복 문제를 발생시키므로, JobListener (class JobListener)에서 jobExecution.getAllFailureExceptions() 대신 예외의 전체 스택이나 원문 메시지를 직접 출력하지 않고 예외 타입 목록과 간단한 요약(예: exception class names, 개수, 첫번째 예외의 짧은 메시지(필요 시 마스킹/잘라내기) 및 jobName/exitCode)을 로깅하도록 변경하고, 중복 로그를 막기 위해 StepMonitorListener.afterStep()에서 이미 로깅된 경우에는 여기서 로깅을 생략하거나 중복 방지 플래그(예: jobExecution.getExecutionContext()에 "exceptionsLogged" 표식)를 설정/확인하도록 구현하세요; 대상 식별자는 jobExecution.getAllFailureExceptions(), JobListener 클래스, StepMonitorListener.afterStep(), jobExecution.getExecutionContext().apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventConsumer.java (1)
53-55: 타임존 하드코딩에 대한 운영 고려가 필요하다.
ZoneId.of("Asia/Seoul")이 하드코딩되어 있다. 현재는 문제없으나 향후 멀티 리전 지원 시 설정 외부화가 필요할 수 있다.LikeEventConsumer와 동일한 패턴이므로 현재는 수용 가능하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventConsumer.java` around lines 53 - 55, The code in OrderEventConsumer uses a hardcoded ZoneId.of("Asia/Seoul") when computing LocalDate (Instant.ofEpochMilli(record.timestamp()).atZone(...).toLocalDate()); refactor to externalize the timezone like LikeEventConsumer does: read a configured timezone (from application properties/env or a injected config bean) into a ZoneId (or ZoneId supplier) and use that variable in the atZone(...) call, providing a sensible fallback (e.g., system default) if the config is missing so multi-region deployments can change zone without code edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductMetrics.java`:
- Around line 51-54: 생성자 ProductMetrics(Long productId, LocalDate date, int
likeCount)에서 productId와 date에 대한 즉시 null 방어가 없어 실패가 지연되므로, 생성 시작 시 두 파라미터를 체크하고
null이면 명시적인 런타임 예외(예: IllegalArgumentException)를 던지도록 수정하세요; 예외 메시지는 어떤 필드가
null인지 명확히 표기하고 호출 지점이 쉽게 추적되게 하며, 이 동작을 검증하는 단위 테스트로 new ProductMetrics(null,
today, 0) 및 new ProductMetrics(productId, null, 0)가 즉시 예외를 던지는지를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsRepositoryImpl.java`:
- Around line 18-20: The method findByProductId in ProductMetricsRepositoryImpl
uses LocalDate.now() which can produce a different date than LikeEventConsumer
(which uses ZoneId.of("Asia/Seoul")), causing off-by-one-day lookups across
timezones; change the date computation to use the explicit KST zone (e.g.,
LocalDate.now(ZoneId.of("Asia/Seoul")) or inject a ZoneId/Clock configured to
"Asia/Seoul") and pass that date to
productMetricsJpaRepository.findByProductIdAndDate(productId, date); also add
tests exercising the KST midnight boundary (e.g., times around 00:00–01:00 KST)
to verify correct behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`:
- Around line 39-49: Normalize and validate the incoming period in
RankingV1Controller: replace the current resolvedPeriod assignment so only
"weekly" and "monthly" are allowed; any other value (including null/blank) must
be normalized to "daily" before computing targetDate and before calling
rankingFacade.findRankings, and compute targetDate only when resolvedPeriod
equals "daily" (use LocalDate.now() if date is missing); ensure the same
normalized resolvedPeriod and computed targetDate are used in the ApiResponse so
response fields always match the actual lookup (symbols: resolvedPeriod,
targetDate, rankingFacade.findRankings, RankingV1Controller), and add E2E tests
for period=foo and period=foo&date=20260401 to assert response.period/date align
with the lookup.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`:
- Around line 50-83: 현재 monthlyTruncateStep가 먼저 mv_product_rank_monthly을 바로 비워
두어 aggregate 실패 시 데이터 공백이 발생하므로, monthlyTruncateStep/monthlyAggregateStep 흐름을
변경해 원자성을 확보하세요: 월간 집계는 먼저 임시 테이블(예: mv_product_rank_monthly_tmp 또는 식별자 필드가 있는
스냅샷 테이블)에 monthlyReader/monthlyProcessor/monthlyWriter로 적재하고, 잡이 정상 완료되면 트랜잭션 또는
교체 스텝에서 기존 mv_product_rank_monthly를 새 스냅샷으로 교체(swap/rename 또는 식별자 업데이트)하도록 구현
변경하세요(대상 메서드/빈: monthlyTruncateStep, monthlyAggregateStep, monthlyWriter,
monthlyReader, monthlyProcessor). 또한 monthlyWriter 실패를 주입했을 때 기존 월간 랭킹이 유지되는 통합
테스트를 추가해 교체 로직이 실패 시 롤백되거나 기존 데이터가 보존되는지 검증하세요.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`:
- Around line 52-84: The current flow deletes mv_product_rank_weekly in
weeklyTruncateStep before aggregation, which risks leaving the live table empty
on any downstream failure; change the job to write into a staging table or
snapshot (e.g., mv_product_rank_weekly_tmp or
mv_product_rank_weekly_snapshot_<id>) during
weeklyReader/weeklyProcessor/weeklyWriter execution and only swap/rename or
update the live mv_product_rank_weekly in a final atomic step (or use
transactional swap) after weeklyAggregateStep completes successfully; update
weeklyTruncateStep to no longer truncate the live table, add a final commit/swap
step (name it e.g. weeklySwapStep) wired into weeklyRankingJob after
weeklyAggregateStep, and add an integration test that injects a failure into
weeklyWriter to assert the original mv_product_rank_weekly remains unchanged.
- Around line 126-128: The code mixes calendar year and ISO week-based year when
building yearMonthWeek: replace monday.getYear() with
monday.get(WeekFields.ISO.weekBasedYear()) so both year and week come from ISO
week fields (use WeekFields.ISO.weekBasedYear() together with
WeekFields.ISO.weekOfWeekBasedYear()); update the construction of yearMonthWeek
in WeeklyRankingJobConfig (variables targetDate, target, monday, yearMonthWeek,
DATE_FORMATTER) accordingly, and add a unit test that asserts dates from
2024-12-28 through 2025-01-05 all map to "2025-W01".
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`:
- Around line 138-166: The test weeklyRankingJob_truncatesBeforeWrite only
asserts that old MV row (product_id=999) is removed and misses verifying the
JobExecution success and that the new aggregated row for product_id=1 was
written; update the test to capture the JobExecution returned by
jobLauncherTestUtils.launchJob(params) (or use
jobLauncherTestUtils.launchJobAndWait) and assert its status is COMPLETED, then
query mv_product_rank_weekly for product_id=1 and assert the expected row exists
(and values like like_count/order_count/score if appropriate); additionally add
a separate negative test that injects a writer failure (e.g., simulate exception
in the writer step or use a test-specific bean) to assert that on writer failure
the original mv_product_rank_weekly row is preserved.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/product/ProductMetrics.java`:
- Around line 49-60: Add immediate null and validity checks in the
ProductMetrics constructors: inside both ProductMetrics(Long productId,
LocalDate date, int likeCount) and ProductMetrics(Long productId, LocalDate
date, int likeCount, int orderCount) call Objects.requireNonNull(productId) and
Objects.requireNonNull(date) to fail fast, and validate likeCount and orderCount
are non-negative (throw IllegalArgumentException on negative). Update
constructors to assign only after these checks and add unit tests asserting that
null productId/date and negative counts cause immediate exceptions.
In
`@apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/OrderEventConsumerTest.java`:
- Line 61: Replace uses of any(LocalDate.class) in stubs for
productMetricsJpaRepository.findByProductIdAndDate(...) with a fixed LocalDate
(e.g., LocalDate today) so the test pins the expected date; update all
occurrences where you currently call
when(productMetricsJpaRepository.findByProductIdAndDate(eq(100L),
any(LocalDate.class))) to use eq(today) (or eq(LocalDate.of(...))) and likewise
update verifications that assert repository.save(...) was called to capture the
saved ProductMetrics and assert its getDate() equals today; do this in
OrderEventConsumerTest where you stub/verify productMetricsJpaRepository so date
mismatches are caught deterministically.
---
Outside diff comments:
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java`:
- Around line 31-37: Create a new RankingV1ApiSpec interface that documents the
existing ranking endpoint consistent with other ApiSpec files: annotate the
interface/class with `@Tag` and add a getRankings method annotated with `@Operation`
and `@Parameter` for the request parameters (including period and date) and
describe the response structure to include the new period field; reference the
existing RankingPageResponse record and ensure the response schema documents its
content, period, date, page and size fields so Swagger/OpenAPI shows the period
response property and the period/date request parameters for clients integrating
with getRankings.
---
Nitpick comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.java`:
- Around line 55-61: 현재 JobListener의 FAILED 블록이
jobExecution.getAllFailureExceptions() 전체를 그대로 로깅하고 있어 파싱·민감정보·중복 문제를 발생시키므로,
JobListener (class JobListener)에서 jobExecution.getAllFailureExceptions() 대신 예외의
전체 스택이나 원문 메시지를 직접 출력하지 않고 예외 타입 목록과 간단한 요약(예: exception class names, 개수, 첫번째
예외의 짧은 메시지(필요 시 마스킹/잘라내기) 및 jobName/exitCode)을 로깅하도록 변경하고, 중복 로그를 막기 위해
StepMonitorListener.afterStep()에서 이미 로깅된 경우에는 여기서 로깅을 생략하거나 중복 방지 플래그(예:
jobExecution.getExecutionContext()에 "exceptionsLogged" 표식)를 설정/확인하도록 구현하세요; 대상
식별자는 jobExecution.getAllFailureExceptions(), JobListener 클래스,
StepMonitorListener.afterStep(), jobExecution.getExecutionContext().
In `@apps/commerce-batch/src/test/resources/schema/ranking-tables.sql`:
- Around line 12-30: The two materialized view tables mv_product_rank_weekly and
mv_product_rank_monthly use inconsistent column names for the period field
(year_month_week vs ranking_period); rename one to match the other so both
tables use a consistent column name (e.g., change year_month_week to
ranking_period or vice versa) and update any dependent queries, DTOs or ETL code
that reference mv_product_rank_weekly.year_month_week or
mv_product_rank_monthly.ranking_period (search for these exact symbols) to the
chosen unified name to avoid confusion and maintenance overhead.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/LikeEventConsumer.java`:
- Around line 53-56: The code hardcodes ZoneId.of("Asia/Seoul") when computing
LocalDate eventDate in LikeEventConsumer (around the LocalDate eventDate
assignment and processLikeEvent call); change this to use a configurable
timezone injected from configuration (e.g., a new field rankingTimezone or
`@Value`("${app.ranking.timezone:Asia/Seoul}") rankingTimezone) and use
ZoneId.of(rankingTimezone) when converting
Instant.ofEpochMilli(record.timestamp()) to LocalDate so the event date honors
the configured region rather than a hardcoded "Asia/Seoul".
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventConsumer.java`:
- Around line 53-55: The code in OrderEventConsumer uses a hardcoded
ZoneId.of("Asia/Seoul") when computing LocalDate
(Instant.ofEpochMilli(record.timestamp()).atZone(...).toLocalDate()); refactor
to externalize the timezone like LikeEventConsumer does: read a configured
timezone (from application properties/env or a injected config bean) into a
ZoneId (or ZoneId supplier) and use that variable in the atZone(...) call,
providing a sensible fallback (e.g., system default) if the config is missing so
multi-region deployments can change zone without code edits.
🪄 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: debddd06-86ee-4ac9-a074-67bb59ef502a
📒 Files selected for processing (32)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductMetrics.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthlyRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeeklyRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingPeriodV1ApiE2ETest.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MvRankRow.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductMetricsAggregateRow.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingScoreCalculator.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/listener/JobListener.javaapps/commerce-batch/src/test/java/com/loopers/CommerceBatchApplicationTest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.javaapps/commerce-batch/src/test/resources/schema/ranking-tables.sqlapps/commerce-streamer/src/main/java/com/loopers/domain/product/ProductMetrics.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/product/ProductMetricsJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/LikeEventConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventConsumer.javaapps/commerce-streamer/src/test/java/com/loopers/domain/product/ProductMetricsOptimisticLockTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/LikeEventConsumerTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/OrderEventConsumerTest.java
Comment on lines
+51
to
54
| public ProductMetrics(Long productId, LocalDate date, int likeCount) { | ||
| this.productId = productId; | ||
| this.date = date; | ||
| this.likeCount = likeCount; |
There was a problem hiding this comment.
생성자에서 productId/date null 방어가 없어 장애 전파 지점이 늦다.
운영 관점에서 잘못된 이벤트/호출이 들어오면 DB flush 시점에야 실패해 소비자 재시도 누적으로 지연이 커질 수 있다. Line 51-54에서 생성 즉시 null 검증으로 fail-fast 처리하는 것이 안전하다. 추가 테스트로 new ProductMetrics(null, today, 0) 및 new ProductMetrics(productId, null, 0)가 즉시 예외를 던지는지 검증해야 한다.
수정 예시
+import java.util.Objects;
+
public ProductMetrics(Long productId, LocalDate date, int likeCount) {
- this.productId = productId;
- this.date = date;
+ this.productId = Objects.requireNonNull(productId, "productId must not be null");
+ this.date = Objects.requireNonNull(date, "date must not be null");
this.likeCount = likeCount;
this.orderCount = 0;
}As per coding guidelines **/*.java: null 처리와 예외 흐름이 명확한지 점검한다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public ProductMetrics(Long productId, LocalDate date, int likeCount) { | |
| this.productId = productId; | |
| this.date = date; | |
| this.likeCount = likeCount; | |
| public ProductMetrics(Long productId, LocalDate date, int likeCount) { | |
| this.productId = Objects.requireNonNull(productId, "productId must not be null"); | |
| this.date = Objects.requireNonNull(date, "date must not be null"); | |
| this.likeCount = likeCount; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductMetrics.java`
around lines 51 - 54, 생성자 ProductMetrics(Long productId, LocalDate date, int
likeCount)에서 productId와 date에 대한 즉시 null 방어가 없어 실패가 지연되므로, 생성 시작 시 두 파라미터를 체크하고
null이면 명시적인 런타임 예외(예: IllegalArgumentException)를 던지도록 수정하세요; 예외 메시지는 어떤 필드가
null인지 명확히 표기하고 호출 지점이 쉽게 추적되게 하며, 이 동작을 검증하는 단위 테스트로 new ProductMetrics(null,
today, 0) 및 new ProductMetrics(productId, null, 0)가 즉시 예외를 던지는지를 추가하세요.
Comment on lines
18
to
20
| public Optional<ProductMetrics> findByProductId(Long productId) { | ||
| return productMetricsJpaRepository.findByProductId(productId); | ||
| return productMetricsJpaRepository.findByProductIdAndDate(productId, LocalDate.now()); | ||
| } |
There was a problem hiding this comment.
LocalDate.now() 사용으로 타임존 불일치 문제가 발생할 수 있다.
LocalDate.now()는 JVM 기본 타임존을 사용하지만, LikeEventConsumer는 ZoneId.of("Asia/Seoul")을 명시적으로 사용한다. 서버 타임존이 UTC인 경우:
- 문제 시나리오: KST 00:30에 Streamer가
2026-04-16날짜로 데이터 저장 → API 서버(UTC)는2026-04-15로 조회 → 좋아요 수 0 반환 - 운영 영향: 자정 전후 시간대에 좋아요 수가 갑자기 0으로 표시되는 현상 발생
🐛 수정안: 명시적 타임존 사용
+import java.time.ZoneId;
+
`@RequiredArgsConstructor`
`@Component`
public class ProductMetricsRepositoryImpl implements ProductMetricsRepository {
+ private static final ZoneId RANKING_ZONE = ZoneId.of("Asia/Seoul");
private final ProductMetricsJpaRepository productMetricsJpaRepository;
`@Override`
public Optional<ProductMetrics> findByProductId(Long productId) {
- return productMetricsJpaRepository.findByProductIdAndDate(productId, LocalDate.now());
+ return productMetricsJpaRepository.findByProductIdAndDate(productId, LocalDate.now(RANKING_ZONE));
}
}추가 테스트 권장: 타임존 경계 시간대(KST 00:00-01:00)에서의 동작을 검증하는 테스트 케이스를 추가한다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Optional<ProductMetrics> findByProductId(Long productId) { | |
| return productMetricsJpaRepository.findByProductId(productId); | |
| return productMetricsJpaRepository.findByProductIdAndDate(productId, LocalDate.now()); | |
| } | |
| import java.time.ZoneId; | |
| `@RequiredArgsConstructor` | |
| `@Component` | |
| public class ProductMetricsRepositoryImpl implements ProductMetricsRepository { | |
| private static final ZoneId RANKING_ZONE = ZoneId.of("Asia/Seoul"); | |
| private final ProductMetricsJpaRepository productMetricsJpaRepository; | |
| `@Override` | |
| public Optional<ProductMetrics> findByProductId(Long productId) { | |
| return productMetricsJpaRepository.findByProductIdAndDate(productId, LocalDate.now(RANKING_ZONE)); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductMetricsRepositoryImpl.java`
around lines 18 - 20, The method findByProductId in ProductMetricsRepositoryImpl
uses LocalDate.now() which can produce a different date than LikeEventConsumer
(which uses ZoneId.of("Asia/Seoul")), causing off-by-one-day lookups across
timezones; change the date computation to use the explicit KST zone (e.g.,
LocalDate.now(ZoneId.of("Asia/Seoul")) or inject a ZoneId/Clock configured to
"Asia/Seoul") and pass that date to
productMetricsJpaRepository.findByProductIdAndDate(productId, date); also add
tests exercising the KST midnight boundary (e.g., times around 00:00–01:00 KST)
to verify correct behavior.
Comment on lines
+39
to
+49
| String resolvedPeriod = (period == null || period.isBlank()) ? "daily" : period; | ||
| String targetDate = "daily".equalsIgnoreCase(resolvedPeriod) | ||
| ? (date == null || date.isBlank() ? LocalDate.now().format(DATE_FORMATTER) : date) | ||
| : null; | ||
|
|
||
| List<RankingInfo> rankings = rankingFacade.findRankings(targetDate, page, size); | ||
| List<RankingInfo> rankings = rankingFacade.findRankings(resolvedPeriod, targetDate, page, size); | ||
| List<RankingV1Dto.RankingResponse> content = rankings.stream() | ||
| .map(RankingV1Dto.RankingResponse::from) | ||
| .toList(); | ||
|
|
||
| return ApiResponse.success(new RankingV1Dto.RankingPageResponse(content, targetDate, page, size)); | ||
| return ApiResponse.success(new RankingV1Dto.RankingPageResponse(content, resolvedPeriod, targetDate, page, size)); |
There was a problem hiding this comment.
비정상 period 값이 실제 조회 경로와 응답 본문을 어긋나게 만든다.
period=foo면 Line 40-42에서 targetDate는 null이 되지만, Line 44는 RankingFacade의 daily fallback으로 조회된다. 결과적으로 실제 데이터는 daily인데 응답의 period는 "foo"이고 date도 비어 있어 캐시 키, 클라이언트 분기, 운영 지표가 잘못 분류될 수 있다. weekly/monthly만 허용하고 나머지는 "daily"로 정규화해야 한다. period=foo와 period=foo&date=20260401 케이스를 추가해 응답 필드와 실제 조회 결과가 항상 일치하는지 E2E로 검증하는 편이 안전하다.
As per coding guidelines **/*Controller*.java: Controller는 요청 검증과 응답 조립에 집중한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`
around lines 39 - 49, Normalize and validate the incoming period in
RankingV1Controller: replace the current resolvedPeriod assignment so only
"weekly" and "monthly" are allowed; any other value (including null/blank) must
be normalized to "daily" before computing targetDate and before calling
rankingFacade.findRankings, and compute targetDate only when resolvedPeriod
equals "daily" (use LocalDate.now() if date is missing); ensure the same
normalized resolvedPeriod and computed targetDate are used in the ApiResponse so
response fields always match the actual lookup (symbols: resolvedPeriod,
targetDate, rankingFacade.findRankings, RankingV1Controller), and add E2E tests
for period=foo and period=foo&date=20260401 to assert response.period/date align
with the lookup.
Comment on lines
+50
to
+83
| @Bean(JOB_NAME) | ||
| public Job monthlyRankingJob() { | ||
| return new JobBuilder(JOB_NAME, jobRepository) | ||
| .start(monthlyTruncateStep(null)) | ||
| .next(monthlyAggregateStep(null)) | ||
| .listener(jobListener) | ||
| .build(); | ||
| } | ||
|
|
||
| @JobScope | ||
| @Bean(STEP_TRUNCATE) | ||
| public Step monthlyTruncateStep(@Value("#{jobParameters['targetDate']}") String targetDate) { | ||
| Tasklet tasklet = (contribution, chunkContext) -> { | ||
| new org.springframework.jdbc.core.JdbcTemplate(dataSource) | ||
| .update("DELETE FROM mv_product_rank_monthly"); | ||
| return RepeatStatus.FINISHED; | ||
| }; | ||
| return new StepBuilder(STEP_TRUNCATE, jobRepository) | ||
| .tasklet(tasklet, transactionManager) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } | ||
|
|
||
| @JobScope | ||
| @Bean(STEP_AGGREGATE) | ||
| public Step monthlyAggregateStep(@Value("#{jobParameters['targetDate']}") String targetDate) { | ||
| return new StepBuilder(STEP_AGGREGATE, jobRepository) | ||
| .<ProductMetricsAggregateRow, MvRankRow>chunk(CHUNK_SIZE, transactionManager) | ||
| .reader(monthlyReader(targetDate)) | ||
| .processor(monthlyProcessor(targetDate)) | ||
| .writer(monthlyWriter()) | ||
| .listener(stepMonitorListener) | ||
| .listener(chunkListener) | ||
| .build(); |
There was a problem hiding this comment.
본 테이블 선삭제는 월간 랭킹 공백을 남길 수 있다.
여기서도 truncate step이 먼저 끝나고 aggregate step이 이어지므로, 집계 도중 장애가 나면 mv_product_rank_monthly가 빈 상태로 남는다. 월간 API가 이 테이블을 직접 읽는 구조라서 일시 장애가 곧 사용자-facing 공백으로 전파된다. 임시 테이블 적재 후 swap 하거나, 새 스냅샷을 별도 식별자로 기록한 뒤 성공 시점에만 전환하도록 바꿔 원자성을 확보하는 편이 안전하다. writer 실패를 주입했을 때 기존 월간 랭킹이 유지되는 통합 테스트도 필요하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`
around lines 50 - 83, 현재 monthlyTruncateStep가 먼저 mv_product_rank_monthly을 바로 비워
두어 aggregate 실패 시 데이터 공백이 발생하므로, monthlyTruncateStep/monthlyAggregateStep 흐름을
변경해 원자성을 확보하세요: 월간 집계는 먼저 임시 테이블(예: mv_product_rank_monthly_tmp 또는 식별자 필드가 있는
스냅샷 테이블)에 monthlyReader/monthlyProcessor/monthlyWriter로 적재하고, 잡이 정상 완료되면 트랜잭션 또는
교체 스텝에서 기존 mv_product_rank_monthly를 새 스냅샷으로 교체(swap/rename 또는 식별자 업데이트)하도록 구현
변경하세요(대상 메서드/빈: monthlyTruncateStep, monthlyAggregateStep, monthlyWriter,
monthlyReader, monthlyProcessor). 또한 monthlyWriter 실패를 주입했을 때 기존 월간 랭킹이 유지되는 통합
테스트를 추가해 교체 로직이 실패 시 롤백되거나 기존 데이터가 보존되는지 검증하세요.
Comment on lines
+52
to
+84
| public Job weeklyRankingJob() { | ||
| return new JobBuilder(JOB_NAME, jobRepository) | ||
| .start(weeklyTruncateStep(null)) | ||
| .next(weeklyAggregateStep(null)) | ||
| .listener(jobListener) | ||
| .build(); | ||
| } | ||
|
|
||
| @JobScope | ||
| @Bean(STEP_TRUNCATE) | ||
| public Step weeklyTruncateStep(@Value("#{jobParameters['targetDate']}") String targetDate) { | ||
| Tasklet tasklet = (contribution, chunkContext) -> { | ||
| new org.springframework.jdbc.core.JdbcTemplate(dataSource) | ||
| .update("DELETE FROM mv_product_rank_weekly"); | ||
| return RepeatStatus.FINISHED; | ||
| }; | ||
| return new StepBuilder(STEP_TRUNCATE, jobRepository) | ||
| .tasklet(tasklet, transactionManager) | ||
| .listener(stepMonitorListener) | ||
| .build(); | ||
| } | ||
|
|
||
| @JobScope | ||
| @Bean(STEP_AGGREGATE) | ||
| public Step weeklyAggregateStep(@Value("#{jobParameters['targetDate']}") String targetDate) { | ||
| return new StepBuilder(STEP_AGGREGATE, jobRepository) | ||
| .<ProductMetricsAggregateRow, MvRankRow>chunk(CHUNK_SIZE, transactionManager) | ||
| .reader(weeklyReader(targetDate)) | ||
| .processor(weeklyProcessor(targetDate)) | ||
| .writer(weeklyWriter()) | ||
| .listener(stepMonitorListener) | ||
| .listener(chunkListener) | ||
| .build(); |
There was a problem hiding this comment.
본 테이블 선삭제는 배치 일시 실패를 바로 사용자 장애로 만든다.
여기서는 truncate step이 먼저 커밋되고 나서 aggregate step이 실행된다. 그 다음 reader/processor/writer 어디에서라도 실패하면 mv_product_rank_weekly는 빈 상태로 남고, 주간 랭킹 API는 그대로 공백을 노출하게 된다. 운영 관점에서는 일시적인 DB 오류가 곧 사용자-visible 장애로 번진다. 임시 테이블에 모두 적재한 뒤 swap 하거나, 새 스냅샷을 별도 식별자로 저장한 후 성공 시점에만 전환하도록 바꿔 원자성을 확보하는 편이 안전하다. writer 실패를 주입했을 때 기존 주간 랭킹이 유지되는 통합 테스트도 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`
around lines 52 - 84, The current flow deletes mv_product_rank_weekly in
weeklyTruncateStep before aggregation, which risks leaving the live table empty
on any downstream failure; change the job to write into a staging table or
snapshot (e.g., mv_product_rank_weekly_tmp or
mv_product_rank_weekly_snapshot_<id>) during
weeklyReader/weeklyProcessor/weeklyWriter execution and only swap/rename or
update the live mv_product_rank_weekly in a final atomic step (or use
transactional swap) after weeklyAggregateStep completes successfully; update
weeklyTruncateStep to no longer truncate the live table, add a final commit/swap
step (name it e.g. weeklySwapStep) wired into weeklyRankingJob after
weeklyAggregateStep, and add an integration test that injects a failure into
weeklyWriter to assert the original mv_product_rank_weekly remains unchanged.
Comment on lines
+126
to
+128
| LocalDate target = LocalDate.parse(targetDate, DATE_FORMATTER); | ||
| LocalDate monday = target.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | ||
| String yearMonthWeek = monday.getYear() + "-W" + String.format("%02d", monday.get(java.time.temporal.WeekFields.ISO.weekOfWeekBasedYear())); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ISO week-based year와 calendar year가 달라지는 경계일을 확인한다.
python - <<'PY'
from datetime import date
for d in [date(2024, 12, 30), date(2025, 1, 1), date(2025, 1, 5)]:
iso_year, iso_week, iso_weekday = d.isocalendar()
print(f"{d} -> iso_year={iso_year}, iso_week={iso_week}")
PY
# 현재 구현이 calendar year(getYear)와 ISO week를 조합하는지 확인한다.
rg -n "getYear\\(\\)|weekOfWeekBasedYear" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 357
🏁 Script executed:
# 전체 메서드 컨텍스트 확인
sed -n '100,150p' apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java
# 테스트 파일 확인
find . -name "*WeeklyRankingJob*Test*.java" -o -name "*WeeklyRankingJob*.java" | grep -i test
# 파일 크기 확인
wc -l apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java
# weekOfWeekBasedYear 또는 유사한 ISO week 계산이 다른 곳에도 있는지 확인
rg -n "WeekFields|weekOfWeekBasedYear|weekBasedYear" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2626
🏁 Script executed:
# 테스트 파일 확인 - 특히 연말 경계 케이스가 있는지 확인
cat -n apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java
# 전체 파일 내용 확인 (151줄이므로 전체 보기 가능)
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 15715
🏁 Script executed:
# MvRankRow 클래스 확인 - period 필드가 yearMonthWeek을 받는지 확인
rg -A 5 "class MvRankRow" apps/commerce-batch/src/main/java/
# SQL에서 :period 파라미터를 제대로 받는지 확인
rg ":period" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 443
🏁 Script executed:
# MvRankRow 클래스 찾기
fd -name "MvRankRow.java" apps/commerce-batch/
# record나 class 정의 확인
rg -B 2 -A 8 "MvRankRow" apps/commerce-batch/src/main/java/ --max-count=5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 11880
ISO 주차 계산에서 calendar year와 ISO week-based year를 혼용하여 연말 날짜의 year_month_week이 잘못 기록된다.
monday.getYear()는 달력 연도(2024년 12월 30일 → 2024)이고, WeekFields.ISO.weekOfWeekBasedYear()는 ISO 주 기준 연도(2024년 12월 30일 → 2025)이다. 이 둘을 조합하면 2024년 12월 30일이 2024-W01로 저장되지만, ISO 표준에서는 같은 날이 2025년 W01에 속해야 한다. 운영 중 연말 재집계, 장애 분석, 수동 보정 시 같은 주가 서로 다른 식별자로 갈라지는 데이터 무결성 문제가 발생한다.
weekBasedYear()를 사용하여 year와 week를 모두 ISO 표준에 맞춰야 하며, 2024년 12월 28일부터 2025년 1월 5일까지의 날짜가 모두 2025-W01로 매핑되는 단위 테스트를 추가해야 한다.
수정 예시
- String yearMonthWeek = monday.getYear() + "-W" + String.format("%02d", monday.get(java.time.temporal.WeekFields.ISO.weekOfWeekBasedYear()));
+ var weekFields = java.time.temporal.WeekFields.ISO;
+ String yearMonthWeek = monday.get(weekFields.weekBasedYear())
+ + "-W"
+ + String.format("%02d", monday.get(weekFields.weekOfWeekBasedYear()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`
around lines 126 - 128, The code mixes calendar year and ISO week-based year
when building yearMonthWeek: replace monday.getYear() with
monday.get(WeekFields.ISO.weekBasedYear()) so both year and week come from ISO
week fields (use WeekFields.ISO.weekBasedYear() together with
WeekFields.ISO.weekOfWeekBasedYear()); update the construction of yearMonthWeek
in WeeklyRankingJobConfig (variables targetDate, target, monday, yearMonthWeek,
DATE_FORMATTER) accordingly, and add a unit test that asserts dates from
2024-12-28 through 2025-01-05 all map to "2025-W01".
Comment on lines
+138
to
+166
| @DisplayName("재실행 시 기존 mv_product_rank_weekly 데이터를 삭제하고 새로 적재한다.") | ||
| @Test | ||
| void weeklyRankingJob_truncatesBeforeWrite() throws Exception { | ||
| // arrange: 이전 배치 결과가 남아있다고 가정 | ||
| jdbcTemplate.update( | ||
| "INSERT INTO mv_product_rank_weekly (product_id, like_count, order_count, score, year_month_week, updated_at) " + | ||
| "VALUES (?, ?, ?, ?, ?, NOW())", | ||
| 999L, 1, 1, 0.9, "2020-W01" | ||
| ); | ||
| LocalDate monday = LocalDate.now().with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | ||
| jdbcTemplate.update( | ||
| "INSERT INTO product_metrics (product_id, date, like_count, order_count) VALUES (?, ?, ?, ?)", | ||
| 1L, monday, 5, 3 | ||
| ); | ||
|
|
||
| jobLauncherTestUtils.setJob(job); | ||
|
|
||
| // act | ||
| var params = new JobParametersBuilder() | ||
| .addString("targetDate", monday.format(DATE_FORMATTER)) | ||
| .addLong("runId", System.nanoTime()) | ||
| .toJobParameters(); | ||
| jobLauncherTestUtils.launchJob(params); | ||
|
|
||
| // assert: product 999 (이전 데이터)는 삭제되고 product 1만 남음 | ||
| Integer count999 = jdbcTemplate.queryForObject( | ||
| "SELECT COUNT(*) FROM mv_product_rank_weekly WHERE product_id = ?", Integer.class, 999L | ||
| ); | ||
| assertThat(count999).isEqualTo(0); |
There was a problem hiding this comment.
truncatesBeforeWrite 테스트가 부분 실패를 놓친다.
지금은 이전 데이터 삭제만 확인해서 truncate step 이후 aggregate step이 실패해도 테스트가 통과할 수 있다. 운영에서는 “기존 데이터만 지우고 새 데이터는 못 적재한” 회귀를 막아야 하므로 JobExecution 완료 상태와 product 1의 신규 적재까지 함께 검증해야 한다. 가능하면 writer 실패를 주입한 케이스를 추가해 실패 시 기존 MV 데이터 보존 여부도 확인하는 편이 안전하다.
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/ranking/WeeklyRankingJobE2ETest.java`
around lines 138 - 166, The test weeklyRankingJob_truncatesBeforeWrite only
asserts that old MV row (product_id=999) is removed and misses verifying the
JobExecution success and that the new aggregated row for product_id=1 was
written; update the test to capture the JobExecution returned by
jobLauncherTestUtils.launchJob(params) (or use
jobLauncherTestUtils.launchJobAndWait) and assert its status is COMPLETED, then
query mv_product_rank_weekly for product_id=1 and assert the expected row exists
(and values like like_count/order_count/score if appropriate); additionally add
a separate negative test that injects a writer failure (e.g., simulate exception
in the writer step or use a test-specific bean) to assert that on writer failure
the original mv_product_rank_weekly row is preserved.
Comment on lines
+49
to
60
| public ProductMetrics(Long productId, LocalDate date, int likeCount) { | ||
| this.productId = productId; | ||
| this.date = date; | ||
| this.likeCount = likeCount; | ||
| this.orderCount = 0; | ||
| } | ||
|
|
||
| public ProductMetrics(Long productId, int likeCount, int orderCount) { | ||
| public ProductMetrics(Long productId, LocalDate date, int likeCount, int orderCount) { | ||
| this.productId = productId; | ||
| this.date = date; | ||
| this.likeCount = likeCount; | ||
| this.orderCount = orderCount; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
생성자에서 productId와 date 불변식을 바로 강제하는 편이 안전하다.
지금은 @Column(nullable = false)에만 기대고 있어서 null 값이 들어오면 flush 시점에야 실패한다. 스트리머에서는 이런 지연 실패가 곧 재시도 누적과 원인 파악 지연으로 이어진다. 생성자에서 Objects.requireNonNull(productId) / Objects.requireNonNull(date)로 즉시 실패시키고, 가능하면 음수 카운트도 함께 막는 편이 안전하다. productId == null, date == null 입력이 즉시 거부되는 테스트를 추가하는 것이 좋다.
As per coding guidelines **/domain/**/*.java: null 처리, 불변성, 캡슐화를 점검한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/product/ProductMetrics.java`
around lines 49 - 60, Add immediate null and validity checks in the
ProductMetrics constructors: inside both ProductMetrics(Long productId,
LocalDate date, int likeCount) and ProductMetrics(Long productId, LocalDate
date, int likeCount, int orderCount) call Objects.requireNonNull(productId) and
Objects.requireNonNull(date) to fail fast, and validate likeCount and orderCount
are non-negative (throw IllegalArgumentException on negative). Update
constructors to assign only after these checks and add unit tests asserting that
null productId/date and negative counts cause immediate exceptions.
| Acknowledgment acknowledgment = mock(Acknowledgment.class); | ||
| when(eventHandledJpaRepository.existsByTopicAndEventId("order-events", "evt-1")).thenReturn(false); | ||
| when(productMetricsJpaRepository.findByProductId(100L)).thenReturn(Optional.empty()); | ||
| when(productMetricsJpaRepository.findByProductIdAndDate(eq(100L), any(LocalDate.class))).thenReturn(Optional.empty()); |
There was a problem hiding this comment.
날짜 인자를 any(LocalDate.class)로 두면 회귀를 놓칠 수 있다.
운영 관점에서 잘못된 날짜로 upsert되면 일별 집계가 분산되어 주/월 배치 랭킹이 왜곡된다. Line 61, Line 82, Line 122는 today 고정값으로 스텁/검증하여 날짜 전달 오류를 즉시 잡도록 수정하는 것이 안전하다. 추가로 “이벤트 처리 시 저장된 ProductMetrics.date가 기대 날짜와 동일한지”를 검증하는 테스트를 보강하는 것이 좋다.
수정 예시
+LocalDate today = LocalDate.now();
-when(productMetricsJpaRepository.findByProductIdAndDate(eq(100L), any(LocalDate.class))).thenReturn(Optional.empty());
+when(productMetricsJpaRepository.findByProductIdAndDate(eq(100L), eq(today))).thenReturn(Optional.empty());
verify(productMetricsJpaRepository, times(1)).save(
- argThat(m -> m.getProductId().equals(100L) && m.getOrderCount() == 1)
+ argThat(m -> m.getProductId().equals(100L) && m.getDate().equals(today) && m.getOrderCount() == 1)
);As per coding guidelines **/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.
Also applies to: 82-82, 122-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/OrderEventConsumerTest.java`
at line 61, Replace uses of any(LocalDate.class) in stubs for
productMetricsJpaRepository.findByProductIdAndDate(...) with a fixed LocalDate
(e.g., LocalDate today) so the test pins the expected date; update all
occurrences where you currently call
when(productMetricsJpaRepository.findByProductIdAndDate(eq(100L),
any(LocalDate.class))) to use eq(today) (or eq(LocalDate.of(...))) and likewise
update verifications that assert repository.save(...) was called to capture the
saved ProductMetrics and assert its getDate() equals today; do this in
OrderEventConsumerTest where you stub/verify productMetricsJpaRepository so date
mismatches are caught deterministically.
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
product_metrics일간 데이터를 주기적으로 집계하고, 조회 전용 Materialized View 테이블에 적재한다. 기존 Ranking API는period파라미터로 일간/주간/월간을 선택할 수 있도록 확장한다.🧭 Context & Decision
#1. Batch Job 전략 — Truncate-then-Insert
목표: 배치를 재실행해도 MV 테이블에 중복 데이터가 쌓이지 않도록 멱등성을 보장해야 한다.
고민: 기존 데이터를
UPSERT로 갱신하는 방법과, 전체 삭제 후 재삽입하는 두 가지 선택지가 있었다.결정: Step 1(Truncate Tasklet) → Step 2(Chunk 집계·적재) 순서로 두 Step을 체이닝했다. MV 테이블은 랭킹 조회 전용이므로 배치 실행 중 잠깐 비어있어도 API는 빈 목록을 반환할 뿐 오류는 발생하지 않는다.
#2. 점수 계산 일관성 — RankingScoreCalculator 공유
목표: Streamer(실시간 랭킹)와 Batch(주간/월간 랭킹)의 점수 계산 공식이 달라지면 일간 → 주간 집계 결과 간 순위가 역전될 수 있다.
고민: Streamer 모듈의 가중치(
like: 0.2,order: 0.7)를 Batch 모듈에서 하드코딩하면, 한쪽만 수정될 경우 공식이 어긋난다.결정:
RankingScoreCalculator를 commerce-batch 내 공유 상수로 두고, Reader SQL의 ORDER BY 절과 Processor 모두 이 상수를 참조하도록 했다. Streamer는 동일한 가중치 값을 독립적으로 유지한다 (모듈 의존 없이 값만 일치).#3. Ranking API 확장 — period 파라미터 분기
목표: 기존
GET /api/v1/rankings?date=yyyyMMddAPI를 깨지 않으면서 주간/월간 랭킹을 추가한다.결정:
period쿼리 파라미터를 optional로 추가하고, 미입력 시"daily"로 폴백했다. Facade에서 period 값에 따라 Redis ZSET(daily) 또는 MV JPA 조회(weekly/monthly)로 분기한다. 기존 클라이언트는 period 없이 호출하면 기존 동작 그대로 유지된다.🏗️ Design Overview
모듈 책임
commerce-batchcommerce-apicommerce-streamer주요 컴포넌트 책임
WeeklyRankingJobConfigMonthlyRankingJobConfigRankingScoreCalculatorMvProductRankWeeklyMvProductRankMonthlyRankingFacadeRankingV1Controller🔁 Flow Diagram
Flow 1 — Weekly Batch Job 실행 흐름
sequenceDiagram autonumber participant Scheduler as Scheduler / CLI participant BatchJob as WeeklyRankingJob participant DB as MySQL (product_metrics) participant MV as mv_product_rank_weekly Scheduler->>BatchJob: launch(targetDate=20250414) BatchJob->>MV: DELETE FROM mv_product_rank_weekly (Truncate Step) BatchJob->>DB: SELECT product_id, SUM(like_count), SUM(order_count)<br/>WHERE date BETWEEN monday AND sunday<br/>GROUP BY product_id ORDER BY score DESC LIMIT 100 loop Chunk (1000건 단위) DB-->>BatchJob: ProductMetricsAggregateRow BatchJob->>BatchJob: Processor: score 계산, period 레이블(yyyy-Www) 생성 BatchJob->>MV: INSERT INTO mv_product_rank_weekly (batch) end BatchJob-->>Scheduler: COMPLETEDFlow 2 — Ranking API period 분기 흐름
sequenceDiagram autonumber participant Client participant Controller as RankingV1Controller participant Facade as RankingFacade participant Redis participant MV as MV JPA Repository Client->>Controller: GET /api/v1/rankings?period=weekly Controller->>Facade: findRankings("weekly", date, page, size) alt period = "weekly" Facade->>MV: mvWeeklyRepository.findTop(page, size) MV-->>Facade: List<MvProductRankWeekly> else period = "monthly" Facade->>MV: mvMonthlyRepository.findTop(page, size) MV-->>Facade: List<MvProductRankMonthly> else period = "daily" (기본값) Facade->>Redis: rankingRedisRepository.findTopN(date, page, size) Redis-->>Facade: ZSet Tuples end Facade-->>Controller: List<RankingInfo> Controller-->>Client: ApiResponse<RankingPageResponse>✅ Checklist
🧱 Step 1 — Spring Batch Job 구현
WeeklyRankingJob 구현
targetDate파라미터 기반으로 해당 주(월~일) 범위 자동 계산JdbcCursorItemReader→ItemProcessor→JdbcBatchItemWriter(ChunkSize=1000)WeeklyRankingJobE2ETest— 집계 정확성, 기간 외 데이터 제외, 재실행 truncateMonthlyRankingJob 구현
targetDate파라미터 기반으로 해당 월(1일~말일) 범위 자동 계산MonthlyRankingJobE2ETest— 집계 정확성, 기간 외 데이터 제외, 재실행 truncateRankingScoreCalculator 공식 통일
LIKE_WEIGHT=0.2,ORDER_WEIGHT=0.7(Streamer 가중치와 동일)🏗️ Step 2 — Materialized View 설계
mv_product_rank_weekly 테이블 설계 및 Domain 매핑
product_id(PK),like_count,order_count,score,year_month_week,updated_atMvProductRankWeeklyJPA Entity (읽기 전용),MvProductRankWeeklyRepositorymv_product_rank_monthly 테이블 설계 및 Domain 매핑
product_id(PK),like_count,order_count,score,ranking_period,updated_atMvProductRankMonthlyJPA Entity (읽기 전용),MvProductRankMonthlyRepository🧩 Step 3 — Ranking API 확장
periodoptional 파라미터 추가, 미입력 시"daily"폴백 (하위 호환)RankingFacade.findRankings(period, date, page, size)분기 로직RankingPageResponse에period필드 추가RankingPeriodV1ApiE2ETest— weekly/monthly 데이터 반환, daily 폴백 동작👀 리뷰 포인트
1. Score 계산 위치 — DB에서 계산해도 괜찮은가요?
처음에는 SQL ORDER BY 절에서 정렬용 score를 계산하고, Processor에서 저장용 score를
RankingScoreCalculator.calculate()로 따로 계산하는 구조였습니다.같은 공식이 두 곳에 흩어져 있다는 걸 인지하고, SQL SELECT 절에 score를 포함시켜 Processor가
row.score()만 읽도록 수정했습니다.덕분에 코드 중복은 사라졌는데, 한편으로는 점수 공식 같은 비즈니스 로직이 SQL에 담기는 게 맞는지 의문이 생겼습니다.
실무에서는 이런 집계 계산을 DB 쿼리에 두는 것이 일반적인가요, 아니면 Application Layer(Java)에서 처리하는 경우가 더 많은가요?
📄
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java📄
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ProductMetricsAggregateRow.java2. weekly/monthly 응답에서 date를 null로 내려주는 것이 좋은 설계인가요?
period=weekly또는monthly일 때 date 파라미터가 의미 없는 값으로 응답에 포함되고 있었습니다.이를 null로 바꿔서 "이 period에서는 날짜 개념이 없다"는 것을 명시적으로 표현했습니다.
그런데 null을 반환하면 클라이언트가 period마다 null 여부를 분기해야 한다는 부담이 생깁니다.
엔드포인트를 period별로 분리하거나(
/rankings/daily,/rankings/weekly), period별 응답 DTO를 나누는 방식도 있는데요.실무에서 한 엔드포인트가 파라미터에 따라 응답 구조가 달라지는 경우, 어떤 방식을 선호하시는지 의견이 궁금합니다.
📄
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java📄
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java3. ChunkSize 튜닝 시점 — 언제 어떤 지표를 보고 결정하시나요?
현재
CHUNK_SIZE = 1000이지만 Reader SQL에LIMIT 100이 고정되어 있어, 실제로 Chunk 경계에 도달하지 않습니다.지금은 TOP 100만 다루니 실질적인 문제는 없지만, 향후 집계 대상이 수만 건으로 늘어난다면 ChunkSize를 다시 고민해야 할 것 같습니다.
처음 배치를 설계할 때 ChunkSize의 초깃값을 어떻게 결정하시나요?
그리고 운영 중에 튜닝이 필요하다고 판단하는 시점은 어떤 지표(처리 시간, 메모리 사용량, 트랜잭션 롤백 비용 등)를 기준으로 하시는지 의견이 궁금합니다.
📄
apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java(L41)변경 요약
변경 목적: 기존 Redis ZSET 기반의 일일 랭킹만으로는 주간/월간 트렌드 분석이 불가능하고 대규모 실시간 집계로 인한 부하가 발생하는 문제를 해결하기 위해 Spring Batch 기반의 MaterializedView 집계 방식 도입.
핵심 변경점: (1) ProductMetrics에
date필드 추가, (product_id, date) 복합 유니크 제약 적용 / (2)mv_product_rank_weekly,mv_product_rank_monthlyMV 테이블 및 JPA 엔티티/레포지토리 추가 / (3) RankingScoreCalculator 클래스로 가중치 통일(LIKE_WEIGHT=0.2, ORDER_WEIGHT=0.7) / (4) Ranking API에 period 파라미터 추가(daily/weekly/monthly, 기본값 daily) / (5) Batch는 DELETE → INSERT 전략으로 멱등성 보장, 청크 크기 1000.리스크/주의사항: Batch 작업 중 실패 시 MV 테이블이 truncate 상태로 남아 재시작 필수 → JobListener의 실패 감지 로직이 추가되었으나 자동 재시도 메커니즘 부재 / ProductMetricsRepositoryImpl에서 findByProductId 조회 시 현재 LocalDate.now()만 사용하므로 과거 데이터는 조회 불가 → 기존 데이터 마이그레이션이 필요한지 확인 필요 / Batch 작업 스케줄(언제 실행되는지)이 PR에 명시되지 않아 배포 후 운영 계획 필요.
테스트/검증 방법: WeeklyRankingJobE2ETest, MonthlyRankingJobE2ETest로 Batch 집계 결과와 점수 계산 검증 / RankingPeriodV1ApiE2ETest로 period 파라미터별 응답 검증(MV에 직접 INSERT로 테스트) / 스트리머 쪽 이벤트 컨슈머 테스트 수정으로 date 기반 저장 검증.