[volume-10] Spring Batch 기반 주간/월간 랭킹 시스템 구현 #396
Open
move-wook wants to merge 2 commits into
Open
Conversation
product_metrics 테이블을 읽어 가중치 기반 Top 100 랭킹을 집계하고 mv_product_rank_weekly / mv_product_rank_monthly 테이블에 저장한다. - WeeklyRankingJobConfig: Chunk-Oriented (chunkSize=100, Top 100) Reader: JpaPagingItemReader<ProductMetrics> Processor: 가중치 계산 (view×0.1 + like×0.2 + order×0.7) Writer: 기존 기간 DELETE → Top 100 INSERT - MonthlyRankingJobConfig: 동일 패턴 (월간 집계) - MvProductRankWeekly, MvProductRankMonthly 엔티티 + JPA Repository - JobParameter: requestDate (yyyy-MM-dd) 실행: ./gradlew :apps:commerce-batch:bootRun \ --args='--spring.batch.job.name=weeklyRankingJob --requestDate=2026-04-12' Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
기존 Redis ZSET 기반 일간 랭킹에 더해, 배치가 적재한 MV 테이블을 조회하여 주간/월간 랭킹을 제공한다. - RankingV1Controller: @RequestParam period 추가 (기본값: daily) - RankingFacade: period별 분기 - daily → Redis ZSET (기존 R9) - weekly → mv_product_rank_weekly DB 조회 - monthly → mv_product_rank_monthly DB 조회 - MvRankingRepository (읽기 전용) + Impl - MvProductRankWeekly/Monthly 엔티티 + JPA Repository GET /api/v1/rankings?period=weekly&date=20260412&page=1&size=20 GET /api/v1/rankings?period=monthly&date=20260412&page=1&size=20 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough일일 랭킹에 주간/월간 랭킹을 추가하는 기능이 확장되었다. 컨트롤러에서 period 파라미터를 수락하고, 퍼사드에서 기간별 데이터 소스를 분기하는 로직이 도입되었으며, 배치 작업 두 개가 추가되어 주간/월간 랭킹을 주기적으로 계산하여 저장한다. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as RankingV1Controller
participant Facade as RankingFacade
participant Repo as MvRankingRepository<br/>(구현체)
participant JpaRepo as MvProductRankWeekly<br/>JpaRepository
participant DB as Database
Client->>Controller: GET /api/v1/rankings?period=weekly&date=20260401
Controller->>Controller: targetDate 파싱, period="weekly" 기본값
Controller->>Facade: getRankings("weekly", "20260401", page, size)
Facade->>Facade: switch(period) → "weekly" 분기
Facade->>Facade: date 파싱 → periodStart(월), periodEnd(일)
Facade->>Repo: findWeeklyRanking(periodStart, periodEnd, offset, size)
Repo->>JpaRepo: findByPeriodStartAndPeriodEndOrderByRankPositionAsc(...)
JpaRepo->>DB: SELECT * WHERE periodStart=? AND periodEnd=?<br/>ORDER BY rankPosition ASC LIMIT size OFFSET offset
DB-->>JpaRepo: MvProductRankWeekly 레코드
JpaRepo-->>Repo: 결과 반환
Repo->>Repo: RankedProduct로 변환 (productId, score)
Repo-->>Facade: List<RankedProduct>
Facade->>Repo: countWeeklyRanking(periodStart, periodEnd)
Repo->>JpaRepo: countByPeriodStartAndPeriodEnd(...)
JpaRepo->>DB: SELECT COUNT(*) WHERE periodStart=? AND periodEnd=?
DB-->>JpaRepo: 총 개수
JpaRepo-->>Repo: count
Repo-->>Facade: totalSize
Facade->>Facade: buildPageInfo(date, ranked, totalSize, offset, page, size)
Facade->>Facade: RankedProduct → 상품/브랜드 매핑, RankingPageInfo 조립
Facade-->>Controller: RankingPageInfo
Controller-->>Client: ApiResponse<RankingV1Dto.PageResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 변경사항이 배치, API, 도메인 계층에 걸쳐 있으며, 새로운 엔티티 정의(중복), 배치 작업 설정(복잡한 청크 처리 및 주기 계산), 퍼사드의 메서드 시그니처 변경과 분기 로직이 혼재되어 있다. 특히 배치 작업 두 개의 적절한 기간 경계 계산(주간 월-일, 월간 1-말일) 및 데이터 정합성을 검증해야 하며, 환경 간 중복된 엔티티 정의가 유지보수상 부담을 야기할 수 있다. 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: 10
🧹 Nitpick comments (6)
apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
47-50:ZonedDateTime.now()타임존 명시 권장
ZonedDateTime.now()는 JVM 기본 타임존을 사용하므로, 서버 환경에 따라 저장되는 시간이 달라질 수 있다.운영 관점:
- 멀티 리전 또는 서버 간 타임존 설정이 다른 경우
created_at값이 일관되지 않아 디버깅이 어려워진다.♻️ 수정안
`@PrePersist` private void prePersist() { - this.createdAt = ZonedDateTime.now(); + this.createdAt = ZonedDateTime.now(java.time.ZoneOffset.UTC); }추가 테스트: 단위 테스트에서
@PrePersist호출 후createdAt의 타임존이 UTC인지 검증 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java` around lines 47 - 50, prePersist in MvProductRankMonthly sets createdAt with ZonedDateTime.now() which uses the JVM default zone; change it to produce an explicit UTC timestamp (e.g., use ZonedDateTime.now(ZoneOffset.UTC) or Instant.now().atZone(ZoneOffset.UTC)) in the prePersist method so createdAt is always stored in UTC, and add a unit test that invokes prePersist and asserts createdAt's zone is UTC (or that it equals an expected UTC instant).apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
55-59: 유효하지 않은 period 값에 대한 처리 명확화 필요
default분기가daily로 fallback되어 잘못된 period 값(예: "yearly", 오타)도 daily로 처리된다.운영 영향: 클라이언트 오류를 감지하지 못하고 잘못된 데이터 제공.
수정안: 명시적으로 "daily" case를 추가하고, default에서는 에러 반환 권장.
♻️ 명시적 분기 처리
switch (period) { case "weekly" -> { /* ... */ } case "monthly" -> { /* ... */ } - default -> { + case "daily" -> { // daily — Redis ZSET ranked = rankingRepository.findTopN(date, offset, size); totalSize = rankingRepository.getTotalSize(date); } + default -> throw new CoreException(ErrorType.INVALID_PERIOD); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java` around lines 55 - 59, The current switch in RankingFacade falls back to daily for any unknown period, causing invalid period values (e.g., "yearly") to be silently treated as daily; update the switch to include an explicit "daily" case that calls rankingRepository.findTopN(date, offset, size) and rankingRepository.getTotalSize(date), and change the default branch to throw a clear IllegalArgumentException (or return an error response) indicating an unsupported period value so callers receive a proper error instead of incorrect results.apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java (2)
47-50:ZonedDateTime.now()사용 시 타임존 불일치 가능성
ZonedDateTime.now()는 시스템 기본 타임존을 사용한다. 배치 서버가 여러 대이거나 타임존이 다른 환경에서 실행될 경우createdAt값이 일관되지 않을 수 있다.운영 영향: 로그 분석, 데이터 정합성 검증 시 혼란 유발.
수정안: 명시적 타임존(예: UTC) 사용 권장.
♻️ 명시적 타임존 사용
`@PrePersist` private void prePersist() { - this.createdAt = ZonedDateTime.now(); + this.createdAt = ZonedDateTime.now(java.time.ZoneOffset.UTC); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java` around lines 47 - 50, The prePersist method in MvProductRankWeekly sets createdAt with ZonedDateTime.now(), which uses the system default timezone and can vary across servers; change it to use an explicit timezone (e.g., ZonedDateTime.now(ZoneOffset.UTC) or ZonedDateTime.now(ZoneId.of("UTC"))) inside the prePersist method so createdAt is consistently stored in UTC; update any imports/usage accordingly and ensure the createdAt field type remains ZonedDateTime to preserve timezone information.
11-51: commerce-api 모듈과 동일 테이블 매핑 엔티티 중복
MvProductRankMonthly와 마찬가지로MvProductRankWeekly도 두 모듈에 중복 정의되어 있다. 앞서 언급한 유지보수 리스크가 동일하게 적용된다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java` around lines 11 - 51, The MvProductRankWeekly entity class is duplicated across commerce-batch and commerce-api (same issue as MvProductRankMonthly); remove the duplicate definition here and reference a single shared entity to avoid divergence: either delete this MvProductRankWeekly class from apps/commerce-batch and import the canonical entity from the common/shared module (or from commerce-api if it’s the chosen source), update any code that constructs or persists MvProductRankWeekly to use the shared type, and ensure the `@Entity` mapping, constructor and `@PrePersist` createdAt behavior remain in the centralized class.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java (1)
36-99: WeeklyRankingJobConfig와 코드 중복 - 공통화 권장두 Job 설정의 차이점:
- Job/Step 이름
- period 계산 로직 (주간 vs 월간)
- Repository 타입
나머지 Reader, Processor, 가중치 상수, 구조가 모두 동일하다.
수정안: 공통 추상 클래스 또는 설정 빌더로 추출하여 유지보수성 향상.
♻️ 공통화 예시
// AbstractRankingJobConfig.java public abstract class AbstractRankingJobConfig { protected static final int CHUNK_SIZE = 100; protected static final int TOP_N = 100; protected static final double WEIGHT_VIEW = 0.1; protected static final double WEIGHT_LIKE = 0.2; protected static final double WEIGHT_ORDER = 0.7; protected JpaPagingItemReader<ProductMetrics> createReader( EntityManagerFactory emf, String readerName) { return new JpaPagingItemReaderBuilder<ProductMetrics>() .name(readerName) .entityManagerFactory(emf) .queryString("SELECT pm FROM ProductMetrics pm ORDER BY pm.id ASC") .pageSize(CHUNK_SIZE) .build(); } protected ItemProcessor<ProductMetrics, RankedProductScore> createProcessor() { return metrics -> new RankedProductScore( metrics.getProductId(), metrics.getViewCount() * WEIGHT_VIEW + metrics.getLikesCount() * WEIGHT_LIKE + metrics.getOrderCount() * WEIGHT_ORDER ); } }🤖 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 36 - 99, MonthlyRankingJobConfig duplicates logic from the weekly job (reader, processor, constants); extract common pieces into an abstract base or helper (e.g., create AbstractRankingJobConfig with shared constants CHUNK_SIZE/TOP_N/WEIGHT_* and protected methods createReader(EntityManagerFactory, String) and createProcessor()) and have MonthlyRankingJobConfig/WeeklyRankingJobConfig extend/use it; update monthlyRankingStep/monthlyProductMetricsReader/monthlyRankingScoreProcessor to delegate to the shared createReader/createProcessor and keep only job-specific differences (JOB_NAME, STEP_NAME, period logic, repository type, and writer bean like monthlyRankingWriter).apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java (1)
11-38: 공유 엔티티 모듈 도입 또는 의도적 분리 문서화 필요 - 모듈 간 스키마 동기화 리스크 존재동일한 테이블(
mv_product_rank_monthly)을 매핑하는 엔티티가commerce-api와commerce-batch모듈에 각각 존재하며, 설계 차이가 있다. commerce-batch는 데이터 생성 시 생성자와@PrePersist를 포함하고, commerce-api는 보유하지 않는다.운영 리스크:
- 스키마 변경 시(컬럼 추가/삭제/수정) 두 모듈 모두 동기화 필요
- 한쪽만 수정하면 데이터 불일치 또는 마이그레이션 오류 발생 가능
권장 사항:
shared-domain모듈로 엔티티를 통합하거나, 의도적인 모듈 격리라면 스키마 변경 체크리스트와 함께 문서화하고 CI/CD에서 엔티티 정의 일관성 검증 로직 추가 필요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java` around lines 11 - 38, There are duplicate, inconsistent entity definitions for MvProductRankMonthly across modules which risks schema drift; consolidate the entity into a single shared module or align implementations: move the MvProductRankMonthly class into a shared-domain module and reference it from commerce-api and commerce-batch (or if intentional, document the isolation and add CI checks), ensure the shared entity includes the same constructor and lifecycle hooks (e.g., the `@PrePersist` behavior and any constructors present in commerce-batch) and remove the divergent class from commerce-api so both modules use the identical MvProductRankMonthly definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 39-60: The date parsing in RankingFacade (calls to
LocalDate.parse(date, DATE_FORMAT) used in the "weekly" and "monthly" branches)
can throw DateTimeParseException and currently bubbles up as a 500; wrap the
parse in a try/catch for DateTimeParseException and convert it to the
repository's unified error type by throwing a CoreException with a clear message
(e.g., "invalid date format" and include the original exception as the cause);
apply this handling to both places where LocalDate.parse(date, DATE_FORMAT) is
used so malformed dates produce a controlled CoreException response instead of
an uncaught 500.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`:
- Around line 14-38: The MvProductRankWeekly entity is missing the business
constructor and the `@PrePersist` callback that sets createdAt (causing
compile/runtime errors and NOT NULL violations); add a public constructor on
MvProductRankWeekly that accepts and assigns productId, rankPosition, score,
periodStart, and periodEnd (keeping the existing protected no-args constructor),
and add a `@PrePersist-annotated` method (e.g., initCreatedAt or prePersist) that
sets createdAt = ZonedDateTime.now() if createdAt is null, mirroring the pattern
used in MvProductRankMonthly so batch writers can new MvProductRankWeekly(...)
and persists succeed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.java`:
- Around line 20-23: The method findWeeklyRanking currently does offset / size
which will throw ArithmeticException when size == 0; add defensive validation at
the start of findWeeklyRanking to ensure size > 0 (and optionally offset >= 0)
and throw a clear IllegalArgumentException (or custom BadRequest) if the check
fails; then compute the page index and call
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc with
PageRequest.of(pageIndex, size) as before. Ensure you reference the same method
names (findWeeklyRanking,
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc,
PageRequest.of) so the guard is applied before any division happens.
- Around line 19-24: findWeeklyRanking currently treats the incoming offset as a
page by doing PageRequest.of(offset / size, size), which misbehaves for
non-multiple offsets; fix by changing the contract to accept page + size (or
alternatively implement a query that supports OFFSET/LIMIT directly).
Concretely: update the method signature of findWeeklyRanking(LocalDate
periodStart, LocalDate periodEnd, int page, int size) and call
PageRequest.of(page, size) inside the implementation (update all call sites such
as RankingFacade which currently computes offset to pass the correct page), or
if you prefer keeping an absolute offset, replace the repository call with a
native query that applies SQL OFFSET :offset LIMIT :size and map results to
RankedProduct. Ensure references to
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc and any
callers are updated to the new semantic.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`:
- Around line 22-30: getRankings in RankingV1Controller currently accepts any
string for the period param and silently falls back to daily; add explicit
validation at the start of RankingV1Controller.getRankings to accept only
"daily", "weekly", or "monthly" (case-sensitive or normalize to lower-case
first), and if invalid throw a CoreException (or return a 400 ApiResponse) with
a clear message like "Invalid period: must be one of [daily, weekly, monthly]";
ensure you reference the incoming period value in the message, do not alter
rankingFacade.getRankings behavior, and add a unit/integration test that posts
an invalid period (e.g., "Weekly" or "DAILY" or "foo") and asserts a 400
response and the proper error message.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`:
- Around line 101-128: monthlyRankingWriter currently deletes and saves per
chunk which causes only the last chunk's results to persist; remove the call to
mvRepository.deleteByPeriodStartAndPeriodEnd from monthlyRankingWriter and
instead implement a StepExecutionListener that does the delete once in
beforeStep, and change monthlyRankingWriter to accumulate chunk scores into a
shared StepExecution executionContext priority queue (or List) keyed by e.g.
"monthlyTopQueue" so each chunk adds its RankedProductScore items (use TOP_N
logic to keep the queue bounded), then in afterStep (the same listener) compute
the global top N from that shared queue and persist MvProductRankMonthly entries
(using scored.productId(), rank, scored.score(), periodStart, periodEnd);
reference monthlyRankingWriter, mvRepository.deleteByPeriodStartAndPeriodEnd,
MvProductRankMonthly, TOP_N, and the StepExecution beforeStep/afterStep methods
to locate and apply the change.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`:
- Around line 101-130: The weeklyRankingWriter ItemWriter currently computes Top
N per chunk and calls mvRepository.deleteByPeriodStartAndPeriodEnd inside the
writer, causing earlier chunks to be erased; change the flow so the writer only
emits/persists chunk items (no global delete or final MV save) and perform
global aggregation and persistence in a StepExecutionListener.afterStep (or
accumulate all items into the StepExecution's ExecutionContext during write and
perform final Top N computation and a single
mvRepository.deleteByPeriodStartAndPeriodEnd + mvRepository.save(...) loop
there), ensuring use of weeklyRankingWriter, TOP_N,
mvRepository.deleteByPeriodStartAndPeriodEnd, and a new/updated afterStep method
to do the single global delete and insert.
- Around line 117-118: The delete-then-save sequence in WeeklyRankingJobConfig
(mvRepository.deleteByPeriodStartAndPeriodEnd(...) followed by saves) is
vulnerable to race conditions; wrap that critical section in a distributed lock
or make the operation atomic. Specifically, add a locking mechanism (e.g., Redis
lock or DB-level lock) around the call site in WeeklyRankingJobConfig so that
before calling mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart,
periodEnd) you acquire a lock keyed by the period (and release it after all
saves complete), or replace the delete+save flow with an atomic DB-side
operation (UPSERT/MERGE/REPLACE INTO via a new repository method) or enforce
single execution via JobParametersIncrementer/unique JobParameters to prevent
concurrent job runs; update the mvRepository usage or Job configuration
accordingly and add integration tests that simulate concurrent executions to
validate integrity.
In
`@apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.java`:
- Around line 9-12: The derived delete method deleteByPeriodStartAndPeriodEnd in
MvProductRankMonthlyJpaRepository triggers a select-then-delete loop and causes
N+1 queries; replace it with a single bulk delete by adding a `@Modifying`
`@Transactional` `@Query` (JPQL or native) that executes "DELETE FROM
MvProductRankMonthly m WHERE m.periodStart = :periodStart AND m.periodEnd =
:periodEnd" (keep method name or rename to deleteBulkByPeriodStartAndPeriodEnd)
in the MvProductRankMonthlyJpaRepository interface, and update/integrate tests
to assert row counts before/after and verify via query logs that one DELETE
statement is executed.
In
`@apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.java`:
- Around line 9-12: The derived delete method deleteByPeriodStartAndPeriodEnd in
MvProductRankWeeklyJpaRepository causes inefficient per-entity delete; replace
it with a bulk delete using a `@Modifying` `@Query` annotated method (e.g. a JPQL
"delete from MvProductRankWeekly m where m.periodStart = :periodStart and
m.periodEnd = :periodEnd") and ensure the method is executed within a
transaction (add `@Transactional` at the repository or service layer) and include
`@Param` bindings or return an int/long for affected rows to confirm deletion.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 55-59: The current switch in RankingFacade falls back to daily for
any unknown period, causing invalid period values (e.g., "yearly") to be
silently treated as daily; update the switch to include an explicit "daily" case
that calls rankingRepository.findTopN(date, offset, size) and
rankingRepository.getTotalSize(date), and change the default branch to throw a
clear IllegalArgumentException (or return an error response) indicating an
unsupported period value so callers receive a proper error instead of incorrect
results.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java`:
- Around line 11-38: There are duplicate, inconsistent entity definitions for
MvProductRankMonthly across modules which risks schema drift; consolidate the
entity into a single shared module or align implementations: move the
MvProductRankMonthly class into a shared-domain module and reference it from
commerce-api and commerce-batch (or if intentional, document the isolation and
add CI checks), ensure the shared entity includes the same constructor and
lifecycle hooks (e.g., the `@PrePersist` behavior and any constructors present in
commerce-batch) and remove the divergent class from commerce-api so both modules
use the identical MvProductRankMonthly definition.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`:
- Around line 36-99: MonthlyRankingJobConfig duplicates logic from the weekly
job (reader, processor, constants); extract common pieces into an abstract base
or helper (e.g., create AbstractRankingJobConfig with shared constants
CHUNK_SIZE/TOP_N/WEIGHT_* and protected methods
createReader(EntityManagerFactory, String) and createProcessor()) and have
MonthlyRankingJobConfig/WeeklyRankingJobConfig extend/use it; update
monthlyRankingStep/monthlyProductMetricsReader/monthlyRankingScoreProcessor to
delegate to the shared createReader/createProcessor and keep only job-specific
differences (JOB_NAME, STEP_NAME, period logic, repository type, and writer bean
like monthlyRankingWriter).
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.java`:
- Around line 47-50: prePersist in MvProductRankMonthly sets createdAt with
ZonedDateTime.now() which uses the JVM default zone; change it to produce an
explicit UTC timestamp (e.g., use ZonedDateTime.now(ZoneOffset.UTC) or
Instant.now().atZone(ZoneOffset.UTC)) in the prePersist method so createdAt is
always stored in UTC, and add a unit test that invokes prePersist and asserts
createdAt's zone is UTC (or that it equals an expected UTC instant).
In
`@apps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`:
- Around line 47-50: The prePersist method in MvProductRankWeekly sets createdAt
with ZonedDateTime.now(), which uses the system default timezone and can vary
across servers; change it to use an explicit timezone (e.g.,
ZonedDateTime.now(ZoneOffset.UTC) or ZonedDateTime.now(ZoneId.of("UTC"))) inside
the prePersist method so createdAt is consistently stored in UTC; update any
imports/usage accordingly and ensure the createdAt field type remains
ZonedDateTime to preserve timezone information.
- Around line 11-51: The MvProductRankWeekly entity class is duplicated across
commerce-batch and commerce-api (same issue as MvProductRankMonthly); remove the
duplicate definition here and reference a single shared entity to avoid
divergence: either delete this MvProductRankWeekly class from
apps/commerce-batch and import the canonical entity from the common/shared
module (or from commerce-api if it’s the chosen source), update any code that
constructs or persists MvProductRankWeekly to use the shared type, and ensure
the `@Entity` mapping, constructor and `@PrePersist` createdAt behavior remain in
the centralized class.
🪄 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: 96f14d58-f326-4f7b-9df6-f3e76fc39739
📒 Files selected for processing (16)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/MvRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankedProductScore.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankMonthly.javaapps/commerce-batch/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.java
Comment on lines
+39
to
+60
| switch (period) { | ||
| case "weekly" -> { | ||
| LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT); | ||
| LocalDate periodStart = targetDate.with(DayOfWeek.MONDAY); | ||
| LocalDate periodEnd = targetDate.with(DayOfWeek.SUNDAY); | ||
| ranked = mvRankingRepository.findWeeklyRanking(periodStart, periodEnd, offset, size); | ||
| totalSize = mvRankingRepository.countWeeklyRanking(periodStart, periodEnd); | ||
| } | ||
| case "monthly" -> { | ||
| LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT); | ||
| YearMonth yearMonth = YearMonth.from(targetDate); | ||
| LocalDate periodStart = yearMonth.atDay(1); | ||
| LocalDate periodEnd = yearMonth.atEndOfMonth(); | ||
| ranked = mvRankingRepository.findMonthlyRanking(periodStart, periodEnd, offset, size); | ||
| totalSize = mvRankingRepository.countMonthlyRanking(periodStart, periodEnd); | ||
| } | ||
| default -> { | ||
| // daily — Redis ZSET | ||
| ranked = rankingRepository.findTopN(date, offset, size); | ||
| totalSize = rankingRepository.getTotalSize(date); | ||
| } | ||
| } |
There was a problem hiding this comment.
날짜 파싱 예외 처리 미흡 - 500 에러 노출 가능
LocalDate.parse(date, DATE_FORMAT)에서 잘못된 형식의 date가 입력되면 DateTimeParseException이 발생하여 500 에러로 노출된다.
운영 영향: 사용자에게 불친절한 에러 메시지 노출, 에러 모니터링 시 불필요한 노이즈 발생.
수정안: CoreException으로 변환하여 일관된 에러 응답 제공. (이 레포지토리의 통합 에러 처리 패턴 준수)
🛡️ 에러 처리 추가
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
+import java.time.format.DateTimeParseException;
case "weekly" -> {
- LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT);
+ LocalDate targetDate = parseDate(date);
LocalDate periodStart = targetDate.with(DayOfWeek.MONDAY);
// ...
}
+private LocalDate parseDate(String date) {
+ try {
+ return LocalDate.parse(date, DATE_FORMAT);
+ } catch (DateTimeParseException e) {
+ throw new CoreException(ErrorType.INVALID_DATE_FORMAT);
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 39 - 60, The date parsing in RankingFacade (calls to
LocalDate.parse(date, DATE_FORMAT) used in the "weekly" and "monthly" branches)
can throw DateTimeParseException and currently bubbles up as a 500; wrap the
parse in a try/catch for DateTimeParseException and convert it to the
repository's unified error type by throwing a CoreException with a clear message
(e.g., "invalid date format" and include the original exception as the cause);
apply this handling to both places where LocalDate.parse(date, DATE_FORMAT) is
used so malformed dates produce a controlled CoreException response instead of
an uncaught 500.
Comment on lines
+14
to
+38
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class MvProductRankWeekly { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(name = "product_id", nullable = false) | ||
| private Long productId; | ||
|
|
||
| @Column(name = "rank_position", nullable = false) | ||
| private Integer rankPosition; | ||
|
|
||
| @Column(name = "score", nullable = false) | ||
| private Double score; | ||
|
|
||
| @Column(name = "period_start", nullable = false) | ||
| private LocalDate periodStart; | ||
|
|
||
| @Column(name = "period_end", nullable = false) | ||
| private LocalDate periodEnd; | ||
|
|
||
| @Column(name = "created_at", nullable = false, updatable = false) | ||
| private ZonedDateTime createdAt; | ||
| } |
There was a problem hiding this comment.
생성자와 @PrePersist 콜백 누락으로 배치 저장 실패 예상
MvProductRankMonthly에는 비즈니스 필드를 초기화하는 생성자와 createdAt을 설정하는 @PrePersist 콜백이 구현되어 있으나, 이 엔티티에는 둘 다 누락되어 있다.
운영 관점:
- 배치 Writer에서
new MvProductRankWeekly(...)호출 시 컴파일 에러 또는 런타임 에러 발생한다. createdAt이 null인 상태로 persist 시도 시NOT NULL constraint위반으로 배치 Job 실패한다.
🐛 수정안
`@Column`(name = "created_at", nullable = false, updatable = false)
private ZonedDateTime createdAt;
+
+public MvProductRankWeekly(Long productId, Integer rankPosition, Double score, LocalDate periodStart, LocalDate periodEnd) {
+ this.productId = productId;
+ this.rankPosition = rankPosition;
+ this.score = score;
+ this.periodStart = periodStart;
+ this.periodEnd = periodEnd;
+}
+
+@PrePersist
+private void prePersist() {
+ this.createdAt = ZonedDateTime.now();
+}추가 테스트: 배치 Job 통합 테스트에서 주간 랭킹 저장 후 createdAt 필드가 정상 설정되었는지 검증 필요하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`
around lines 14 - 38, The MvProductRankWeekly entity is missing the business
constructor and the `@PrePersist` callback that sets createdAt (causing
compile/runtime errors and NOT NULL violations); add a public constructor on
MvProductRankWeekly that accepts and assigns productId, rankPosition, score,
periodStart, and periodEnd (keeping the existing protected no-args constructor),
and add a `@PrePersist-annotated` method (e.g., initCreatedAt or prePersist) that
sets createdAt = ZonedDateTime.now() if createdAt is null, mirroring the pattern
used in MvProductRankMonthly so batch writers can new MvProductRankWeekly(...)
and persists succeed.
Comment on lines
+19
to
+24
| @Override | ||
| public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) { | ||
| return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc( | ||
| periodStart, periodEnd, PageRequest.of(offset / size, size) | ||
| ).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList(); | ||
| } |
There was a problem hiding this comment.
offset → page 변환 로직 오류 - 페이지네이션 결과 불일치 가능
PageRequest.of(offset / size, size) 변환은 offset이 size의 배수일 때만 정확하다.
문제 시나리오:
offset=5, size=10→page=0→ 실제로는 09번째 항목 반환 (기대: 514번째)offset=15, size=10→page=1→ 실제로는 1019번째 항목 반환 (기대: 1524번째)
운영 영향: API 호출자가 예상과 다른 랭킹 데이터를 받게 되어 UX 불일치 발생.
수정안: 호출부(RankingFacade)에서 항상 offset = (page - 1) * size로 계산하므로 현재는 문제없으나, 인터페이스 계약과 구현의 의미론적 불일치가 있다. 인터페이스를 (int page, int size)로 변경하거나, native query로 OFFSET/LIMIT 직접 사용을 권장한다.
♻️ 인터페이스 시그니처 변경 제안
public interface MvRankingRepository {
- List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);
- List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);
+ List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size);
+ List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size);
long countWeeklyRanking(LocalDate periodStart, LocalDate periodEnd);
long countMonthlyRanking(LocalDate periodStart, LocalDate periodEnd);
}그리고 구현체:
public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size) {
return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
- periodStart, periodEnd, PageRequest.of(offset / size, size)
+ periodStart, periodEnd, PageRequest.of(page, size)
).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.java`
around lines 19 - 24, findWeeklyRanking currently treats the incoming offset as
a page by doing PageRequest.of(offset / size, size), which misbehaves for
non-multiple offsets; fix by changing the contract to accept page + size (or
alternatively implement a query that supports OFFSET/LIMIT directly).
Concretely: update the method signature of findWeeklyRanking(LocalDate
periodStart, LocalDate periodEnd, int page, int size) and call
PageRequest.of(page, size) inside the implementation (update all call sites such
as RankingFacade which currently computes offset to pass the correct page), or
if you prefer keeping an absolute offset, replace the repository call with a
native query that applies SQL OFFSET :offset LIMIT :size and map results to
RankedProduct. Ensure references to
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc and any
callers are updated to the new semantic.
Comment on lines
+20
to
+23
| public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) { | ||
| return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc( | ||
| periodStart, periodEnd, PageRequest.of(offset / size, size) | ||
| ).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList(); |
There was a problem hiding this comment.
size=0 입력 시 ArithmeticException 발생 가능
offset / size 연산에서 size=0이면 ArithmeticException이 발생한다. 상위 레이어에서 검증이 있더라도 방어적 코딩이 필요하다.
🛡️ 방어 코드 추가 제안
`@Override`
public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) {
+ if (size <= 0) {
+ return Collections.emptyList();
+ }
return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
periodStart, periodEnd, PageRequest.of(offset / size, size)
).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.java`
around lines 20 - 23, The method findWeeklyRanking currently does offset / size
which will throw ArithmeticException when size == 0; add defensive validation at
the start of findWeeklyRanking to ensure size > 0 (and optionally offset >= 0)
and throw a clear IllegalArgumentException (or custom BadRequest) if the check
fails; then compute the page index and call
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc with
PageRequest.of(pageIndex, size) as before. Ensure you reference the same method
names (findWeeklyRanking,
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc,
PageRequest.of) so the guard is applied before any division happens.
Comment on lines
22
to
30
| public ApiResponse<RankingV1Dto.PageResponse> getRankings( | ||
| @RequestParam(defaultValue = "daily") String period, | ||
| @RequestParam(required = false) String date, | ||
| @RequestParam(defaultValue = "1") int page, | ||
| @RequestParam(defaultValue = "20") int size | ||
| ) { | ||
| String targetDate = (date != null && !date.isBlank()) ? date : LocalDate.now().format(DATE_FORMAT); | ||
| RankingPageInfo pageInfo = rankingFacade.getRankings(targetDate, page, size); | ||
| RankingPageInfo pageInfo = rankingFacade.getRankings(period, targetDate, page, size); | ||
| return ApiResponse.success(RankingV1Dto.PageResponse.from(pageInfo)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: CoreException 및 ErrorType 사용 패턴 확인
# 기존 Controller에서 유효성 검증 후 CoreException throw 패턴이 있는지 확인
rg -n "throw new CoreException" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ -C2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# RankingV1Controller 전체 코드 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1541
🏁 Script executed:
# RankingFacade 구현 확인
cat -n apps/commerce-api/src/main/java/com/loopers/application/facade/RankingFacade.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 198
🏁 Script executed:
# 다른 Controller에서의 파라미터 검증 패턴 확인
rg -A5 "@RequestParam" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6730
🏁 Script executed:
# CoreException 사용 패턴 검색 (전체 codebase)
rg "throw new CoreException" --type=java -C2 | head -30Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2934
🏁 Script executed:
# Bean Validation 또는 검증 로직이 있는 컨트롤러 예제 확인
rg "@Valid|@Pattern|@NotNull" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# RankingFacade 파일 위치 찾기
find apps/commerce-api -name "*RankingFacade*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 167
🏁 Script executed:
# getRankings 메서드 검색
rg "def getRankings|void getRankings|getRankings(" --type=java apps/commerce-api -A8 | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 225
🏁 Script executed:
# period 파라미터를 다루는 로직 전체 찾기
rg "period" --type=java apps/commerce-api -B2 -A2 | grep -A3 -B3 "getRankings\|switch\|period"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 13429
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4914
🏁 Script executed:
# ErrorType 열거형 확인
rg "enum ErrorType|INVALID_PARAMETER" --type=java -A2 apps/commerce-api | head -30Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 470
🏁 Script executed:
# RankingV1Controller 테스트 파일 확인
find apps/commerce-api -name "*RankingV1ControllerTest*" -o -name "*RankingTest*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 다른 컨트롤러의 파라미터 검증 테스트 패턴
rg "invalid.*parameter|Invalid.*parameter" --type=java -B2 -A2 apps/commerce-api | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# ErrorType 전체 내용 확인
cat -n apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1046
🏁 Script executed:
# 파라미터 검증으로 CoreException을 throw하는 예제 찾기
rg "throw new CoreException.*INVALID\|throw new CoreException.*BAD_REQUEST" --type=java -B2 -A1 apps/commerce-api | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
period 파라미터 유효성 검증 누락
period 파라미터가 daily, weekly, monthly 외의 값(예: Weekly, DAILY, 오타)인 경우 RankingFacade의 switch default로 인해 조용히 daily 결과를 반환한다.
운영 관점:
- 사용자가 오타나 대소문자 실수로 주간 랭킹을 요청했으나 일간 랭킹을 받는 상황이 발생할 수 있다.
- API 응답만으로는 어떤 period 데이터가 반환되었는지 구분이 어려워 디버깅이 곤란하다.
- 잘못된 입력에 대해 400 에러를 반환하는 것이 RESTful API 설계 원칙에 부합한다.
♻️ 명시적 검증 추가 권장
Controller에서 검증 후 CoreException throw:
+import java.util.Set;
+
`@RestController`
`@RequestMapping`("/api/v1/rankings")
`@RequiredArgsConstructor`
public class RankingV1Controller {
private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyyMMdd");
+ private static final Set<String> VALID_PERIODS = Set.of("daily", "weekly", "monthly");
private final RankingFacade rankingFacade;
`@GetMapping`
public ApiResponse<RankingV1Dto.PageResponse> getRankings(
`@RequestParam`(defaultValue = "daily") String period,
`@RequestParam`(required = false) String date,
`@RequestParam`(defaultValue = "1") int page,
`@RequestParam`(defaultValue = "20") int size
) {
+ if (!VALID_PERIODS.contains(period)) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "지원하지 않는 period입니다: " + period);
+ }
String targetDate = (date != null && !date.isBlank()) ? date : LocalDate.now().format(DATE_FORMAT);추가 테스트: 잘못된 period 값 입력 시 400 응답과 에러 메시지를 검증하는 테스트 케이스 추가 필요하다.
🤖 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 22 - 30, getRankings in RankingV1Controller currently accepts any
string for the period param and silently falls back to daily; add explicit
validation at the start of RankingV1Controller.getRankings to accept only
"daily", "weekly", or "monthly" (case-sensitive or normalize to lower-case
first), and if invalid throw a CoreException (or return a 400 ApiResponse) with
a clear message like "Invalid period: must be one of [daily, weekly, monthly]";
ensure you reference the incoming period value in the message, do not alter
rankingFacade.getRankings behavior, and add a unit/integration test that posts
an invalid period (e.g., "Weekly" or "DAILY" or "foo") and asserts a 400
response and the proper error message.
Comment on lines
+101
to
+128
| @StepScope | ||
| @Bean | ||
| public ItemWriter<RankedProductScore> monthlyRankingWriter( | ||
| @Value("#{jobParameters['requestDate']}") String requestDate | ||
| ) { | ||
| return items -> { | ||
| LocalDate date = LocalDate.parse(requestDate, DateTimeFormatter.ofPattern("yyyy-MM-dd")); | ||
| YearMonth yearMonth = YearMonth.from(date); | ||
| LocalDate periodStart = yearMonth.atDay(1); | ||
| LocalDate periodEnd = yearMonth.atEndOfMonth(); | ||
|
|
||
| List<RankedProductScore> allScores = new ArrayList<>(items.getItems()); | ||
| allScores.sort(Comparator.comparingDouble(RankedProductScore::score).reversed()); | ||
|
|
||
| List<RankedProductScore> topN = allScores.stream().limit(TOP_N).toList(); | ||
|
|
||
| mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart, periodEnd); | ||
|
|
||
| int rank = 1; | ||
| for (RankedProductScore scored : topN) { | ||
| mvRepository.save(new MvProductRankMonthly( | ||
| scored.productId(), rank++, scored.score(), periodStart, periodEnd | ||
| )); | ||
| } | ||
|
|
||
| log.info("월간 랭킹 저장 완료: period={} ~ {}, count={}", periodStart, periodEnd, topN.size()); | ||
| }; | ||
| } |
There was a problem hiding this comment.
WeeklyRankingJobConfig와 동일한 치명적 로직 오류
WeeklyRankingJobConfig에서 지적한 chunk 단위 Top 100 계산 문제가 동일하게 존재한다. 각 chunk 실행 시 이전 데이터가 삭제되어 최종적으로 마지막 chunk의 데이터만 남는다.
동일한 수정이 필요하다.
🤖 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 101 - 128, monthlyRankingWriter currently deletes and saves per
chunk which causes only the last chunk's results to persist; remove the call to
mvRepository.deleteByPeriodStartAndPeriodEnd from monthlyRankingWriter and
instead implement a StepExecutionListener that does the delete once in
beforeStep, and change monthlyRankingWriter to accumulate chunk scores into a
shared StepExecution executionContext priority queue (or List) keyed by e.g.
"monthlyTopQueue" so each chunk adds its RankedProductScore items (use TOP_N
logic to keep the queue bounded), then in afterStep (the same listener) compute
the global top N from that shared queue and persist MvProductRankMonthly entries
(using scored.productId(), rank, scored.score(), periodStart, periodEnd);
reference monthlyRankingWriter, mvRepository.deleteByPeriodStartAndPeriodEnd,
MvProductRankMonthly, TOP_N, and the StepExecution beforeStep/afterStep methods
to locate and apply the change.
Comment on lines
+101
to
+130
| @StepScope | ||
| @Bean | ||
| public ItemWriter<RankedProductScore> weeklyRankingWriter( | ||
| @Value("#{jobParameters['requestDate']}") String requestDate | ||
| ) { | ||
| return items -> { | ||
| LocalDate date = LocalDate.parse(requestDate, DateTimeFormatter.ofPattern("yyyy-MM-dd")); | ||
| LocalDate periodStart = date.with(DayOfWeek.MONDAY); | ||
| LocalDate periodEnd = date.with(DayOfWeek.SUNDAY); | ||
|
|
||
| // 모든 chunk의 데이터를 모아서 정렬 후 Top N 저장 | ||
| List<RankedProductScore> allScores = new ArrayList<>(items.getItems()); | ||
| allScores.sort(Comparator.comparingDouble(RankedProductScore::score).reversed()); | ||
|
|
||
| List<RankedProductScore> topN = allScores.stream().limit(TOP_N).toList(); | ||
|
|
||
| // 기존 데이터 삭제 | ||
| mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart, periodEnd); | ||
|
|
||
| // Top N 저장 | ||
| int rank = 1; | ||
| for (RankedProductScore scored : topN) { | ||
| mvRepository.save(new MvProductRankWeekly( | ||
| scored.productId(), rank++, scored.score(), periodStart, periodEnd | ||
| )); | ||
| } | ||
|
|
||
| log.info("주간 랭킹 저장 완료: period={} ~ {}, count={}", periodStart, periodEnd, topN.size()); | ||
| }; | ||
| } |
There was a problem hiding this comment.
치명적 로직 오류: chunk 단위 Top 100 계산으로 전역 랭킹 손실
ItemWriter는 chunk 단위로 호출된다(100개씩). 현재 구현은 각 chunk에서:
- 해당 chunk의 아이템만 정렬
- 해당 chunk에서 top 100 추출
- 기존 전체 데이터 삭제
- 해당 chunk의 top 100 저장
결과: 마지막 chunk의 데이터만 남고, 이전 chunk의 고득점 상품은 모두 삭제된다.
예시: 상품 1000개, chunk size 100
- Chunk 1 (상품 1-100): 삭제 후 저장
- Chunk 2 (상품 101-200): Chunk 1 데이터 삭제 후 저장
- ...
- Chunk 10 (상품 901-1000): 최종적으로 이 chunk의 top 100만 남음
수정안:
- Step 종료 후
StepExecutionListener.afterStep()에서 전역 집계 수행 - 또는
ExecutionContext에 누적 저장 후 마지막에 일괄 처리
🐛 수정 방안: ExecutionContext 활용
+ private static final String SCORES_KEY = "allScores";
+
`@StepScope`
`@Bean`
public ItemWriter<RankedProductScore> weeklyRankingWriter(
`@Value`("#{jobParameters['requestDate']}") String requestDate
) {
- return items -> {
+ List<RankedProductScore> accumulated = new ArrayList<>();
+
+ return new ItemWriter<>() {
+ `@Override`
+ public void write(Chunk<? extends RankedProductScore> items) {
+ accumulated.addAll(items.getItems());
+ }
+ };
+ }
+
+ // StepExecutionListener에서 afterStep 시점에 전역 정렬 및 저장 로직 구현 필요또는 Tasklet 기반으로 전체 데이터를 한 번에 처리하는 방식을 권장한다.
🤖 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 101 - 130, The weeklyRankingWriter ItemWriter currently computes
Top N per chunk and calls mvRepository.deleteByPeriodStartAndPeriodEnd inside
the writer, causing earlier chunks to be erased; change the flow so the writer
only emits/persists chunk items (no global delete or final MV save) and perform
global aggregation and persistence in a StepExecutionListener.afterStep (or
accumulate all items into the StepExecution's ExecutionContext during write and
perform final Top N computation and a single
mvRepository.deleteByPeriodStartAndPeriodEnd + mvRepository.save(...) loop
there), ensuring use of weeklyRankingWriter, TOP_N,
mvRepository.deleteByPeriodStartAndPeriodEnd, and a new/updated afterStep method
to do the single global delete and insert.
Comment on lines
+117
to
+118
| // 기존 데이터 삭제 | ||
| mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart, periodEnd); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if there's any distributed lock or job execution constraint
echo "=== Checking JobListener for locking mechanism ==="
fd -t f "JobListener.java" apps/commerce-batch --exec cat {}
echo ""
echo "=== Checking for Redis lock or distributed lock usage ==="
rg -n "lock|Lock|LOCK" apps/commerce-batch/src/main/java --type java
echo ""
echo "=== Checking Spring Batch job instance uniqueness ==="
rg -n "JobParametersIncrementer|preventRestart|allowStartIfComplete" apps/commerce-batch/src/main/java --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1942
동시 실행 시 경합 조건으로 인한 랭킹 데이터 손실 위험
deleteByPeriodStartAndPeriodEnd 실행 후 save 호출 사이에 다른 배치 인스턴스가 동일 period로 실행되면 데이터 손실이 발생한다. 현재 코드에 분산 락이나 Job 실행 제약이 없어 이 문제는 실제 위험이다.
운영 영향: 스케줄러 오설정, 수동 재실행, 또는 여러 배치 인스턴스 운영 시 랭킹 데이터 정합성이 훼손되고 이를 감지하기 어렵다.
해결 방안:
- Redis 또는 DB 레벨 분산 락(예:
SELECT ... FOR UPDATE) 적용 - 또는
REPLACE INTO/MERGE구문으로 DELETE-INSERT를 원자적으로 처리 - Job Parameter에 Unique constraint 추가하거나
JobParametersIncrementer설정으로 중복 실행 방지
추가 테스트: 동시 실행 시나리오에서 데이터 무결성 검증, 실패 케이스 복구 절차 확인 필요하다.
🤖 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 117 - 118, The delete-then-save sequence in WeeklyRankingJobConfig
(mvRepository.deleteByPeriodStartAndPeriodEnd(...) followed by saves) is
vulnerable to race conditions; wrap that critical section in a distributed lock
or make the operation atomic. Specifically, add a locking mechanism (e.g., Redis
lock or DB-level lock) around the call site in WeeklyRankingJobConfig so that
before calling mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart,
periodEnd) you acquire a lock keyed by the period (and release it after all
saves complete), or replace the delete+save flow with an atomic DB-side
operation (UPSERT/MERGE/REPLACE INTO via a new repository method) or enforce
single execution via JobParametersIncrementer/unique JobParameters to prevent
concurrent job runs; update the mvRepository usage or Job configuration
accordingly and add integration tests that simulate concurrent executions to
validate integrity.
Comment on lines
+9
to
+12
| public interface MvProductRankMonthlyJpaRepository extends JpaRepository<MvProductRankMonthly, Long> { | ||
| List<MvProductRankMonthly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | ||
| void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Derived Delete 쿼리의 성능 문제
deleteByPeriodStartAndPeriodEnd는 Spring Data JPA의 derived delete로, 내부적으로 SELECT 후 개별 DELETE를 실행한다. 월간 랭킹 100건 삭제 시 최대 101개의 쿼리가 발생한다.
운영 관점:
- 배치 실행 시간 증가 및 DB 부하 발생한다.
- 트랜잭션 시간이 길어져 락 경합 가능성이 높아진다.
♻️ 벌크 삭제 쿼리로 변경 권장
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
+
public interface MvProductRankMonthlyJpaRepository extends JpaRepository<MvProductRankMonthly, Long> {
List<MvProductRankMonthly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd);
- void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd);
+
+ `@Modifying`
+ `@Query`("DELETE FROM MvProductRankMonthly m WHERE m.periodStart = :periodStart AND m.periodEnd = :periodEnd")
+ void deleteByPeriodStartAndPeriodEnd(`@Param`("periodStart") LocalDate periodStart, `@Param`("periodEnd") LocalDate periodEnd);
}추가 테스트: 통합 테스트에서 삭제 전후 row count를 검증하고, 쿼리 로그로 단일 DELETE 문 실행 확인 필요하다.
📝 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 interface MvProductRankMonthlyJpaRepository extends JpaRepository<MvProductRankMonthly, Long> { | |
| List<MvProductRankMonthly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | |
| void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd); | |
| } | |
| import org.springframework.data.jpa.repository.JpaRepository; | |
| import org.springframework.data.jpa.repository.Modifying; | |
| import org.springframework.data.jpa.repository.Query; | |
| import org.springframework.data.repository.query.Param; | |
| import java.time.LocalDate; | |
| import java.util.List; | |
| public interface MvProductRankMonthlyJpaRepository extends JpaRepository<MvProductRankMonthly, Long> { | |
| List<MvProductRankMonthly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | |
| `@Modifying` | |
| `@Query`("DELETE FROM MvProductRankMonthly m WHERE m.periodStart = :periodStart AND m.periodEnd = :periodEnd") | |
| void deleteByPeriodStartAndPeriodEnd(`@Param`("periodStart") LocalDate periodStart, `@Param`("periodEnd") LocalDate periodEnd); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankMonthlyJpaRepository.java`
around lines 9 - 12, The derived delete method deleteByPeriodStartAndPeriodEnd
in MvProductRankMonthlyJpaRepository triggers a select-then-delete loop and
causes N+1 queries; replace it with a single bulk delete by adding a `@Modifying`
`@Transactional` `@Query` (JPQL or native) that executes "DELETE FROM
MvProductRankMonthly m WHERE m.periodStart = :periodStart AND m.periodEnd =
:periodEnd" (keep method name or rename to deleteBulkByPeriodStartAndPeriodEnd)
in the MvProductRankMonthlyJpaRepository interface, and update/integrate tests
to assert row counts before/after and verify via query logs that one DELETE
statement is executed.
Comment on lines
+9
to
+12
| public interface MvProductRankWeeklyJpaRepository extends JpaRepository<MvProductRankWeekly, Long> { | ||
| List<MvProductRankWeekly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | ||
| void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Derived Delete 쿼리의 성능 문제
월간 리포지토리와 동일하게 deleteByPeriodStartAndPeriodEnd가 derived delete로 구현되어 있어 동일한 성능 문제가 발생한다.
♻️ 벌크 삭제 쿼리로 변경 권장
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
+
public interface MvProductRankWeeklyJpaRepository extends JpaRepository<MvProductRankWeekly, Long> {
List<MvProductRankWeekly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd);
- void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd);
+
+ `@Modifying`
+ `@Query`("DELETE FROM MvProductRankWeekly w WHERE w.periodStart = :periodStart AND w.periodEnd = :periodEnd")
+ void deleteByPeriodStartAndPeriodEnd(`@Param`("periodStart") LocalDate periodStart, `@Param`("periodEnd") LocalDate periodEnd);
}📝 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 interface MvProductRankWeeklyJpaRepository extends JpaRepository<MvProductRankWeekly, Long> { | |
| List<MvProductRankWeekly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | |
| void deleteByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd); | |
| } | |
| import org.springframework.data.jpa.repository.Modifying; | |
| import org.springframework.data.jpa.repository.Query; | |
| import org.springframework.data.repository.query.Param; | |
| public interface MvProductRankWeeklyJpaRepository extends JpaRepository<MvProductRankWeekly, Long> { | |
| List<MvProductRankWeekly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(LocalDate periodStart, LocalDate periodEnd); | |
| `@Modifying` | |
| `@Query`("DELETE FROM MvProductRankWeekly w WHERE w.periodStart = :periodStart AND w.periodEnd = :periodEnd") | |
| void deleteByPeriodStartAndPeriodEnd(`@Param`("periodStart") LocalDate periodStart, `@Param`("periodEnd") LocalDate periodEnd); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/MvProductRankWeeklyJpaRepository.java`
around lines 9 - 12, The derived delete method deleteByPeriodStartAndPeriodEnd
in MvProductRankWeeklyJpaRepository causes inefficient per-entity delete;
replace it with a bulk delete using a `@Modifying` `@Query` annotated method (e.g. a
JPQL "delete from MvProductRankWeekly m where m.periodStart = :periodStart and
m.periodEnd = :periodEnd") and ensure the method is executed within a
transaction (add `@Transactional` at the repository or service layer) and include
`@Param` bindings or return an int/long for affected rows to confirm deletion.
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
GROUP BY + ORDER BY로 매번 집계하면 데이터 증가 시 성능 저하 + API 부하.period파라미터로 daily/weekly/monthly 모두 제공.🧭 Context & Decision
문제 정의
ranking:all:{yyyyMMdd}는 TTL 2일이라 7일/30일치 데이터 유지 불가선택지와 결정
1. 데이터 소스 — Redis ZSET vs DB product_metrics
2. 배치 처리 방식 — Tasklet vs Chunk-Oriented
3. MV 테이블 구조 — 단일 테이블 vs 주간/월간 분리
mv_product_rank+ period_type 컬럼 → 테이블 단순, 쿼리 복잡mv_product_rank_weekly+mv_product_rank_monthly분리 → 인덱스/쿼리 단순4. 랭킹 API 분기 — 별도 Controller vs period 파라미터
/api/v1/rankings/weekly,/api/v1/rankings/monthly등 별도 엔드포인트period파라미터 추가추후 개선 여지
🏗️ Design Overview
변경 범위
주요 컴포넌트 책임
commerce-batch (집계)
WeeklyRankingJobConfig: Chunk-Oriented Job. chunkSize=100, Top 100 저장MonthlyRankingJobConfig: 동일 구조 (월간)JpaPagingItemReader<ProductMetrics>— product_metrics 전체 읽기commerce-api (조회)
RankingFacade.getRankings(period, date, page, size): period 분기MvRankingRepository: MV 테이블 조회 (페이징 지원)MV 테이블 구조
🔁 Flow Diagram
배치 집계 흐름
sequenceDiagram autonumber participant Scheduler as 배치 실행<br/>(requestDate) participant Job as WeeklyRankingJob participant Reader as JpaPagingItemReader participant Processor as RankingScoreProcessor participant Writer as RankingWriter participant DB as product_metrics participant MV as mv_product_rank_weekly Scheduler->>Job: 실행 (requestDate=2026-04-12) Job->>Reader: Chunk(100) 단위로 읽기 Reader->>DB: SELECT pm FROM ProductMetrics<br/>ORDER BY id ASC LIMIT 100 DB-->>Reader: ProductMetrics[] Reader-->>Processor: items Processor->>Processor: 가중치 계산<br/>view×0.1 + like×0.2 + order×0.7 Processor-->>Writer: RankedProductScore[] Writer->>Writer: score 정렬 → Top 100 Writer->>MV: DELETE WHERE period_start=2026-04-06<br/>AND period_end=2026-04-12 Writer->>MV: INSERT rank 1~100 Writer-->>Job: 완료랭킹 API 조회 흐름
flowchart LR A[GET /api/v1/rankings<br/>?period=weekly&date=20260412] --> B[RankingV1Controller] B --> C{RankingFacade<br/>period 분기} C -->|daily| D[Redis ZSET<br/>ZREVRANGE] C -->|weekly| E[mv_product_rank_weekly<br/>WHERE period_start ~ period_end] C -->|monthly| F[mv_product_rank_monthly<br/>WHERE period_start ~ period_end] D --> G[Product/Brand Aggregation] E --> G F --> G G --> H[RankingPageInfo 응답]📊 가중치 & 기간 계산
기간 계산
🚀 실행 방법
배치 Job 실행
API 호출
변경 목적
Redis ZSET 2일 TTL 제약으로 주간/월간 집계가 불가능한 한계를 극복하기 위해, Spring Batch로 ProductMetrics를 가중치(view×0.1, like×0.2, order×0.7) 기반으로 집계하여 MV(Materialized View) 테이블에 Top 100을 저장하는 시스템을 구현했습니다.
핵심 변경점
리스크/주의사항
yyyy-MM-dd형식을 사용하고, API/RankingFacade에서는yyyyMMdd형식을 사용합니다. RankingV1Controller에서 클라이언트의yyyyMMdd요청을 배치 requestDateyyyy-MM-dd형식으로 변환하는 로직이 필요한데, 현재 코드에서 이 변환 위치가 명확하지 않습니다. requestDate 파라미터 전달 시 정확한 포맷 변환을 어느 계층에서 담당하는지 확인이 필요합니다.List<RankedProductScore> allScores = new ArrayList<>(items.getItems())로 메모리에 올린 후 정렬합니다. 대량의 ProductMetrics 처리 시 메모리 부족 가능성이 있으며, 처리 데이터 규모에 따른 메모리 테스트가 필요합니다.테스트/검증 방법
./gradlew :apps:commerce-batch:bootRun --args='--spring.batch.job.name=weeklyRankingJob --requestDate=2026-04-12'명령으로 실행하여 MV 테이블 데이터 저장 및 Top 100 정렬 결과 확인 필요period=weekly&date=20260412형식이 배치의yyyy-MM-dd형식으로 정확히 변환되어 주간/월간 period 범위 계산이 올바른지 검증 필요