[Volume - 10] Spring Batch 기반 주간/월간 랭킹 집계 시스템 구현#411
Open
ukukdin wants to merge 1 commit intoLoopers-dev-lab:ukukdinfrom
Open
[Volume - 10] Spring Batch 기반 주간/월간 랭킹 집계 시스템 구현#411ukukdin wants to merge 1 commit intoLoopers-dev-lab:ukukdinfrom
ukukdin wants to merge 1 commit intoLoopers-dev-lab:ukukdinfrom
Conversation
- WeeklyRankingJob, MonthlyRankingJob: Chunk-Oriented Processing으로 product_metrics 집계 - mv_product_rank_weekly, mv_product_rank_monthly MV 테이블 설계 및 적재 - Ranking API에 period(DAILY/WEEKLY/MONTHLY) 파라미터 추가 - PeriodRankingRepository 도메인 인터페이스 분리 (SRP 준수) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough주간 및 월간 랭킹 기능을 추가한다. 새로운 Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as RankingController
participant UseCase as RankingQueryUseCase
participant Service as RankingQueryService
participant PeriodRepo as PeriodRankingRepository
participant Database as DB
Client->>Controller: GET /rankings?period=WEEKLY&date=...&page=0&size=10
Controller->>UseCase: getRankings(date, 0, 10, WEEKLY)
UseCase->>Service: getRankings(date, 0, 10, WEEKLY)
alt period == WEEKLY
Service->>PeriodRepo: getWeeklyRankings(date, offset, size)
PeriodRepo->>Database: SELECT * FROM mv_product_rank_weekly WHERE yearWeek=?
Database-->>PeriodRepo: List<RankedProduct>
Service->>PeriodRepo: getWeeklyTotalCount(date)
PeriodRepo->>Database: COUNT(*) FROM mv_product_rank_weekly WHERE yearWeek=?
Database-->>PeriodRepo: totalCount
else period == MONTHLY
Service->>PeriodRepo: getMonthlyRankings(date, offset, size)
PeriodRepo->>Database: SELECT * FROM mv_product_rank_monthly WHERE yearMonth=?
Database-->>PeriodRepo: List<RankedProduct>
Service->>PeriodRepo: getMonthlyTotalCount(date)
PeriodRepo->>Database: COUNT(*) FROM mv_product_rank_monthly WHERE yearMonth=?
Database-->>PeriodRepo: totalCount
else period == DAILY
Service->>Service: 기존 RankingRepository 사용
end
Service->>Service: 제품 정보 매핑 및 PageResult 구성
Service-->>UseCase: PageResult<RankingItemInfo>
UseCase-->>Controller: PageResult<RankingItemInfo>
Controller-->>Client: 200 OK with RankingItemResponse list
sequenceDiagram
participant Batch as Spring Batch Job
participant Cleanup as MonthlyCleanupTasklet
participant Reader as JpaPagingItemReader
participant Processor as ItemProcessor
participant Writer as ItemWriter
participant Database as DB
Batch->>Cleanup: Execute monthlyRankingCleanupStep
Cleanup->>Database: DELETE FROM mv_product_rank_monthly WHERE yearMonth=?
Database-->>Cleanup: rows deleted
Batch->>Reader: 청크 단위로 ProductMetricsEntity 읽기
Reader->>Database: SELECT * FROM product_metrics ORDER BY (score formula) LIMIT 1000
Database-->>Reader: ProductMetricsEntity batch
Reader->>Processor: process(metric)
Processor->>Processor: 가중치 점수 계산(뷰:0.2 + 좋아요:0.3 + 주문:0.5)<br/>순차 랭킹 할당
Processor-->>Writer: ProductRankMonthlyEntity
Writer->>Database: BATCH INSERT INTO mv_product_rank_monthly
Database-->>Writer: Success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 주간/월간 랭킹 지원으로 다층 구조 변경이 발생한다. API 계층(컨트롤러, 서비스, 저장소), 배치 계층(설정, 엔티티, 저장소), 도메인 계층(열거형, 유틸리티)에 걸쳐 총 20개 이상의 파일이 추가되었다. 배치 작업 설정의 복잡한 로직(청크 처리, 점수 계산, 순차적 랭킹), 날짜 형식 변환 로직의 중복(두 모듈의 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: 15
♻️ Duplicate comments (1)
apps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java (1)
19-24:⚠️ Potential issue | 🔴 CriticalAPI 쪽과 동일한 주 기준 연도 버그가 재현된다.
int year = date.getYear();가 아니라date.get(weekFields.weekBasedYear())를 써야 연말/연초 경계에서 API와 키가 일치한다. 배치가"2024-W01"로 저장하고 API가"2025-W01"로 조회하면 주간 랭킹이 비어버린다. 즉 운영 관점에서는 사용자 체감 장애가 된다.🐛 제안 수정
public static String toYearWeek(LocalDate date) { WeekFields weekFields = WeekFields.of(Locale.KOREA); - int year = date.getYear(); - int week = date.get(weekFields.weekOfWeekBasedYear()); + int year = date.get(weekFields.weekBasedYear()); + int week = date.get(weekFields.weekOfWeekBasedYear()); return String.format("%d-W%02d", year, week); }
- 추가 테스트: 동일 경계일자 집합(
2023-01-01,2024-12-30,2024-12-31,2025-01-01,2027-01-01등)에 대해 배치·API 두 Resolver의 반환값이 바이트 단위로 동일함을 확인하는 계약 테스트를 권고한다.🤖 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/support/RankingPeriodKeyResolver.java` around lines 19 - 24, toYearWeek currently uses date.getYear() causing week-based-year mismatches at year boundaries; replace the year calculation with date.get(weekFields.weekBasedYear()) in the toYearWeek method (using the existing WeekFields weekFields variable) so the produced key matches the API's week-based year; also add a small contract test comparing this RankingPeriodKeyResolver.toYearWeek outputs against the API resolver for boundary dates like 2023-01-01, 2024-12-30, 2024-12-31, 2025-01-01, 2027-01-01 to ensure byte-for-byte equality.
🧹 Nitpick comments (9)
apps/commerce-api/src/main/java/com/loopers/domain/repository/PeriodRankingRepository.java (1)
8-17: 주/월 메서드 쌍이 분기되어 있어 향후 기간(일별 스냅샷, 분기 등) 추가 시 인터페이스 폭발이 우려된다.PR 설명에 일별 스냅샷 도입 여지가 언급되어 있다. 현재 구조에서는 기간이 늘어날 때마다
getXxxRankings,getXxxTotalCount가 2개씩 추가되어야 한다.RankingPeriod를 인자로 받는 단일 API로 통일하면 라우팅 책임을 구현체가 가져가고,RankingQueryService에서 분기도 줄어든다.List<RankedProduct> getRankings(RankingPeriod period, LocalDate date, int offset, int size); long getTotalCount(RankingPeriod period, LocalDate date);
DAILY라우팅은 상위 서비스에서 유지하더라도, 적어도WEEKLY/MONTHLY를 단일 메서드로 묶으면 테스트 표면도 절반으로 줄어든다.- 추가 테스트: 각
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/domain/repository/PeriodRankingRepository.java` around lines 8 - 17, The PeriodRankingRepository currently exposes period-specific methods (getWeeklyRankings/getWeeklyTotalCount and getMonthlyRankings/getMonthlyTotalCount) which will scale poorly as new periods are added; change the interface PeriodRankingRepository to a period-agnostic API by replacing those four methods with two methods: getRankings(RankingPeriod period, LocalDate date, int offset, int size) and getTotalCount(RankingPeriod period, LocalDate date), update implementing classes to route based on the RankingPeriod enum (WEEKLY/MONTHLY/etc.), and adjust callers (e.g., RankingQueryService) to pass a RankingPeriod rather than calling period-specific methods; add parameterized tests per RankingPeriod to cover each branch.apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductMetricsEntity.java (1)
14-32: 읽기 전용 소스 테이블 여부를 명확히 할 것을 권고한다.이 엔티티는 배치 Reader 입력 전용으로 보이나, 쓰기 가능한 JPA 엔티티로 노출되어 있어 실수로
save/merge호출 시product_metrics원본이 훼손될 여지가 있다. 운영 관점에서는 집계 원천 데이터가 배치에서 변질되면 후속 랭킹 전체가 오염된다.
- 해당 배치에서 쓰기가 필요 없다면
@org.hibernate.annotations.Immutable부여, Setter 미노출(@Getter만 유지되어 현재는 안전함), 그리고 Reader에서readOnly힌트 적용을 권고한다.- 추가 테스트: 이 엔티티 대상
EntityManager.persist/merge호출이 배치 코드 어디에도 없음을 검증하는 ArchUnit 또는 grep 기반 CI 체크를 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductMetricsEntity.java` around lines 14 - 32, ProductMetricsEntity is exposed as a mutable JPA entity but is used only as batch Reader input; mark it immutable by adding `@org.hibernate.annotations.Immutable` on the ProductMetricsEntity class, ensure no setters are added (keep only getters), and update the batch Reader to pass the JPA readOnly hint (e.g., setHint("org.hibernate.readOnly", true) or use readOnly on the JpaPagingItemReader) so Hibernate treats instances as read-only; additionally add a CI/ArchUnit or grep check to assert no code calls EntityManager.persist/merge or repository.save for ProductMetricsEntity to prevent accidental writes.apps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java (1)
15-17:parseDate실패 경로가 Batch Job 파라미터 에러 메시지에 그대로 노출된다.
LocalDate.parse가 던지는DateTimeParseException은 런타임으로 전파되어 Job 실패 로그에 남게 되는데, 잘못된 포맷(예:2025-01-01)을 받으면 원인이 모호한 스택이 쌓인다. 운영 관점에서 스케줄러/운영 도구에서 재시도 여부를 판단하기 어렵다.
- 호출 측에서 형식 기대값(
yyyyMMdd)을 포함한 명확한 예외 메시지를 던지거나, 이 메서드에서 try/catch로 래핑하여IllegalArgumentException으로 변환할 것을 권고한다(cause 보존).- 추가 테스트:
null, 빈 문자열,yyyy-MM-dd포맷 입력에 대한 실패 케이스 테스트를 권고한다.🤖 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/support/RankingPeriodKeyResolver.java` around lines 15 - 17, parseDate currently calls LocalDate.parse(requestDate, DATE_FORMAT) which lets DateTimeParseException propagate into job failure logs; change parseDate to validate requestDate (null/empty) and wrap the LocalDate.parse call in a try/catch catching DateTimeParseException (and NullPointerException if you prefer) and rethrow an IllegalArgumentException that includes the expected format (DATE_FORMAT pattern, e.g. "yyyyMMdd") and an example (e.g. "20250101"), preserving the original exception as the cause; update tests to cover null, empty, and wrong-format inputs (e.g. "yyyy-MM-dd") to assert the IllegalArgumentException and that the cause is the DateTimeParseException.apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankMonthlyEntity.java (1)
48-71:updatedAt을LocalDateTime.now()로 직접 찍으면 테스트 불가/시간대 혼선이 발생한다.생성자와
updateRanking양쪽에서LocalDateTime.now()를 직접 호출하면, (1) 서버 로컬 타임존에 따라 값이 달라져 배포 환경 간 불일치가 나고, (2) 테스트에서 결정적 시간 주입이 불가능해진다. 운영 관점에서는 MV 갱신 시각 기반 모니터링/알람(예: "N시간 이상 업데이트 없음")이 시간대에 따라 오탐할 수 있다.
@PrePersist/@PreUpdate로 위임하거나, 외부에서Clock또는LocalDateTime을 주입받는 시그니처를 권고한다. UTC 통일이 프로젝트 기준이면LocalDateTime.now(ZoneOffset.UTC)로 통일할 것을 권고한다.- 추가 테스트: 고정
Clock기반으로updateRanking호출 시 정확한updatedAt값이 설정되는지 단위 테스트 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankMonthlyEntity.java` around lines 48 - 71, The constructor ProductRankMonthlyEntity(Long productId, String yearMonth, ...) and the updateRanking(...) method currently set updatedAt = LocalDateTime.now(), which prevents deterministic tests and can cause timezone inconsistencies; change this by removing direct LocalDateTime.now() calls and instead (a) delegate timestamping to JPA lifecycle callbacks (`@PrePersist/`@PreUpdate) that set updatedAt, or (b) add a time-provider to the API (inject java.time.Clock or accept a LocalDateTime parameter) and use Clock.systemUTC() or Clock injected in production to produce UTC timestamps; update usages and tests to pass a fixed Clock (or explicit LocalDateTime) so unit tests can assert updatedAt deterministically.apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/repository/ProductRankMonthlyJpaRepository.java (1)
11-13:@Modifying에clearAutomatically=true추가를 권고한다.JPQL bulk DELETE 후 persistence context 동기화를 명시적으로 처리하면 향후 코드 변경 시 의도치 않은 데이터 불일치를 방지할 수 있다. 현 아키텍처에서는 cleanup과 aggregate가 별도 Step으로 분리되어 transaction 경계가 명확하나, 방어적 코딩 관점에서 다음과 같이 수정한다.
수정안
- `@Modifying` + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("DELETE FROM ProductRankMonthlyEntity e WHERE e.yearMonth = :yearMonth") void deleteByYearMonth(`@Param`("yearMonth") String yearMonth);동일 지적은
ProductRankWeeklyJpaRepository의deleteByYearWeek메서드에도 적용된다. 두 저장소 모두 수정을 권고한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/repository/ProductRankMonthlyJpaRepository.java` around lines 11 - 13, JPQL bulk DELETEs should clear the persistence context to avoid stale entities: update the `@Modifying` annotation on ProductRankMonthlyJpaRepository.deleteByYearMonth and likewise on ProductRankWeeklyJpaRepository.deleteByYearWeek to include clearAutomatically = true (i.e., `@Modifying`(clearAutomatically = true)), so the persistence context is synchronized automatically after the bulk delete operation.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryUseCase.java (1)
10-13: 인터페이스가 두 가지 오버로드를 노출하면서 호출자 분산 위험이 발생한다.
RankingQueryService의 구현부에서 3-인자 메서드가 이미 4-인자 메서드로 위임하고 있어 로직 중복은 피해졌으나, 인터페이스 수준에서 여전히 두 시그니처를 모두 노출하면 향후 신규 호출자가 불완전한 3-인자 경로를 선택할 여지가 있다. 현재RankingController는 4-인자만 사용 중이며, 3-인자 호출처는 발견되지 않는다.
- 3-인자 오버로드를
@Deprecated처리하여 단계적 제거 의도를 명시하거나, 호출처 전환이 완료되었다면 인터페이스에서 제거할 것을 권고한다.- 위임 패턴이 이미 구현되어 있으므로, 기본값
DAILY경로의 동작이 4-인자 호출과 동일함을 보장하는 통합 테스트를 추가하여 회귀를 방지할 것을 강력히 권고한다.🤖 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/RankingQueryUseCase.java` around lines 10 - 13, 인터페이스 RankingQueryUseCase가 3-인자 getRankings(LocalDate date, int page, int size)와 4-인자 getRankings(LocalDate date, int page, int size, RankingPeriod period)을 둘다 노출해 호출자 분산 위험이 있으니 3-인자 오버로드를 `@Deprecated로` 표시하거나 호출처 전환이 끝났다면 인터페이스에서 삭제하고, 기존 3-인자 구현(현재는 4-인자에 위임하는 구현)이 동일 동작을 보장하는지 확인하기 위해 통합 테스트를 추가하여 getRankings(date,page,size) 호출 결과가 getRankings(date,page,size, RankingPeriod.DAILY)와 동일함을 검증하십시오.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaPeriodRankingRepository.java (1)
25-45: offset 기반 시그니처와 내부 페이지 계산이 의미론적으로 어긋난다.인터페이스 시그니처는
offset, size이지만 구현은PageRequest.of(offset / size, size)로 "offset이 반드시 size의 배수"임을 암묵적으로 가정한다. 현재 유일한 호출부가offset = page * size로 계산하므로 우연히 동작하지만, 향후 다른 호출자가 임의 offset을 넘기면 엉뚱한 페이지의 데이터를 반환하는 silent bug가 된다. 또한size = 0이면ArithmeticException이 바로 터진다.두 가지 중 하나를 권장한다:
- 인터페이스를
(date, page, size)로 바꿔 계산 의도를 드러낸다(가독성 ↑, 오용 ↓).offset을 유지한다면size <= 0과offset % size != 0을 가드해CoreException또는IllegalArgumentException으로 명시적으로 실패시킨다.테스트로는
offset=5, size=10같은 비정렬 입력에서의 동작,size=0입력 시 표준 에러 포맷 반환을 커버해야 한다.🤖 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/JpaPeriodRankingRepository.java` around lines 25 - 45, The implementation of getWeeklyRankings and getMonthlyRankings assumes offset is a multiple of size by doing PageRequest.of(offset / size, size) and will throw on size==0; update both methods (getWeeklyRankings and getMonthlyRankings) to validate inputs when keeping the (date, offset, size) signature: if size <= 0 or offset < 0 or offset % size != 0 throw an IllegalArgumentException (or CoreException used elsewhere) with a clear message, then compute int page = offset / size and call PageRequest.of(page, size); add unit tests covering non-multiple offset (e.g., offset=5,size=10) and size=0 to assert the exception is thrown.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryService.java (2)
52-65: switch 분기가 도메인 지식으로 가득해 확장 시 수정 포인트가 분산된다.현재
WEEKLY/MONTHLY/default(DAILY)분기가 서비스에 하드코딩되어 있어 추후YEARLY등 기간이 추가되면 이 파일과PeriodRankingRepository를 동시에 수정해야 한다.RankingPeriod에 전략을 위임(period.getRankings(...)/period.getTotalCount(...))하거나,Map<RankingPeriod, RankingProvider>형태로 Spring 빈 주입으로 해결하면 OCP를 지킬 수 있다.운영 영향은 없으나 장기 유지보수를 위한 권장 리팩터이다.
🤖 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/RankingQueryService.java` around lines 52 - 65, The switch in RankingQueryService embeds period-specific logic (WEEKLY/MONTHLY/DAILY) causing scattered changes when adding new periods; refactor by delegating ranking retrieval to a period-specific strategy: add methods like getRankings(date, offset, size) and getTotalCount(date) on RankingPeriod or introduce a RankingProvider interface and wire a Map<RankingPeriod, RankingProvider> via Spring, then replace the switch in RankingQueryService with a single call period.getRankings(...) / period.getTotalCount(...) or lookup providerMap.get(period).getRankings(...) so new periods (e.g. YEARLY) only require adding a new strategy bean and not modifying the service, and remove direct repository calls (periodRankingRepository / rankingRepository) from the switch.
41-43: 기존 daily API 경로 회귀 테스트가 필요하다.3-인자 오버로드가 내부적으로
RankingPeriod.DAILY로 위임하도록 변경되었으므로, 기존 DAILY 호출 경로가 기존 동작(Redis ZSET 기반, totalCount 원시값 사용)과 완전히 동일한 응답을 반환하는지 보장하는 회귀 테스트가 있으면 좋다. 특히 DAILY의totalCount가 필터 미적용인 트레이드오프(기존 설계)가 그대로 유지되는지 단위 테스트에 명시하여 의도치 않은 변경을 방지할 것을 권장한다.🤖 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/RankingQueryService.java` around lines 41 - 43, Add a regression unit test for RankingQueryService that calls the 3-arg overload getRankings(LocalDate date, int page, int size) and asserts it delegates to RankingPeriod.DAILY and returns the exact legacy behavior: use the Redis ZSET-backed path (mock or spy the Redis ZSET/data source used by the service) and verify the returned PageResult<RankingItemInfo> items match the ZSET-derived results and that totalCount equals the unfiltered/raw total (not filtered by any applied filters). In the test, explicitly exercise RankingQueryService.getRankings(...), assert the service invoked the DAILY branch (or that getRankings(date, page, size, RankingPeriod.DAILY) would be used), mock the underlying repository/Redis client to return a known ZSET and raw count, and assert the PageResult.totalCount matches that raw count to prevent regressions.
🤖 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/RankingQueryService.java`:
- Around line 46-71: The getRankings method in RankingQueryService lacks input
guards for invalid pagination/period values; add validation at the start of
getRankings to check if size <= 0 or page < 0 or period == null and throw a
CoreException(BAD_REQUEST) with a user-facing message (e.g., "Invalid pagination
parameters") while preserving the original cause for logs and using a separate
detailed log message; ensure the thrown exception is created from the service
layer (not controller) and references the same behavior for both
periodRankingRepository and rankingRepository code paths so downstream
PageRequest creation and the totalCount/size division cannot divide by zero or
receive negative offsets.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/model/ranking/RankingPeriod.java`:
- Around line 3-7: Add a specific handler for
MethodArgumentTypeMismatchException in GlobalExceptionHandler: implement an
`@ExceptionHandler`(MethodArgumentTypeMismatchException.class) method that returns
400 BAD_REQUEST with a standardized body like
{"code":"INVALID_PARAMETER","message":"..."} and ensure the message references
the invalid parameter name/value; reference the GlobalExceptionHandler class and
MethodArgumentTypeMismatchException symbol. Also add integration tests for
RankingController covering period=invalid, period=daily (lowercase),
period=DAILY (valid), and omitted period (uses default) to assert 400 for
invalid values and correct 200 responses for valid/default cases, using
RankingPeriod to validate expected behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/model/ranking/RankingPeriodKeyResolver.java`:
- Around line 8-23: The RankingPeriodKeyResolver logic (methods toYearWeek and
toYearMonth in class RankingPeriodKeyResolver) is duplicated between
apps/commerce-api and apps/commerce-batch; extract these methods and the
YEAR_MONTH_FORMAT constant into a shared module (e.g., commerce-domain or
commerce-support) and have both modules reference the single shared
RankingPeriodKeyResolver, or if extraction isn’t feasible immediately, add a
contract test that invokes RankingPeriodKeyResolver.toYearWeek(LocalDate) and
toYearMonth(LocalDate) from both API and batch with a common dataset to assert
identical outputs; update imports/usages to reference the shared resolver and
remove the duplicate class.
- Around line 14-19: The toYearWeek method currently mixes date.getYear() with
weekFields.weekOfWeekBasedYear(), which yields wrong keys around year
boundaries; change the year source to the week-based year via
date.get(weekFields.weekBasedYear()) so toYearWeek(LocalDate) returns
String.format("%d-W%02d", weekBasedYear, week) using
WeekFields.of(Locale.KOREA), and add a parameterized test that asserts
toYearWeek produces identical keys for known boundary dates (e.g., 2023-01-01,
2024-12-30, 2024-12-31, 2025-01-01) to ensure API and batch resolver parity.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/repository/PeriodRankingRepository.java`:
- Around line 10-16: PeriodRankingRepository의
getWeeklyRankings/getMonthlyRankings가 반환하는 RankedProduct.rank의 기준(0-based인지
1-based인지)과 정렬·동점 처리 규칙이 명시되어 있지 않아 호출측에서 오프바이원 버그가 발생할 수 있으니 인터페이스 Javadoc에
rank는 0-based 기준임을 명시하고 정렬 보장(예: ranking 오름차순, 동점 시 productId로 tiebreaker)과
offset과의 관계(첫 요소의 rank == offset)까지 기술하세요; 동일 규약을 RankingRepository에도 반영해 문서
일관성을 유지하고, 두 Repository 구현체(PeriodRankingRepository와 RankingRepository)에 대해 "첫
번째 요소의 rank == offset" 불변을 검증하는 단위 테스트를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/ProductRankMonthlyJpaEntity.java`:
- Around line 14-45: Add the missing unique constraint and timestamp automation
to ProductRankMonthlyJpaEntity: annotate the class with `@Table`(uniqueConstraints
= `@UniqueConstraint`(columnNames = {"productId","yearMonth"})) to match the batch
entity, and ensure updatedAt is set automatically by adding either a constructor
that initializes updatedAt = LocalDateTime.now() or lifecycle callbacks
`@PrePersist` and `@PreUpdate` methods that assign updatedAt before insert/update;
optionally mark the entity `@Immutable` or restrict the repository to read-only if
it must not be modified.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/ProductRankWeeklyJpaEntity.java`:
- Around line 23-24: 엔티티의 yearWeek 컬럼 길이가 잘못되어(현재 length = 7) "2024-W01" 같은 8자
키가 절단되므로 ProductRankWeeklyJpaEntity의 yearWeek 속성(`@Column`)을 length = 8으로 늘리고 동일한
변경을 apps/commerce-batch의 ProductRankWeeklyEntity(또는 해당 엔티티의 yearWeek 필드)에 동일하게
적용하세요; 또한 관련 스키마/마이그레이션(혹은 JPA 스키마 생성 설정)이 있다면 동기화하고, 변경 후
RankingPeriodKeyResolver.toYearWeek()로 생성한 키로 배치 INSERT 후 API 조회가 정상 동작하는 통합
테스트를 추가해 검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaPeriodRankingRepository.java`:
- Line 29: The mapping in JpaPeriodRankingRepository that constructs new
RankedProduct using e.getRanking() - 1L can produce negative ranks when
entity.ranking is 0 or negative; change the mapping to clamp the computed rank
to >= 0 (e.g. use Math.max(0, e.getRanking() - 1)) or otherwise ensure
RankedProduct receives a non-negative rank, and add defensive checks/guards in
the entity constructor or the batch ItemProcessor to enforce ranking >= 1 (and
add a unit test asserting the batch writer/ItemProcessor treats ranking as
1-based) so RankingQueryService.toRankingItemInfo no longer receives and adjusts
negative ranks.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankWeeklyJpaRepository.java`:
- Around line 13-17: Add a composite index on (yearWeek, ranking) to the
ProductRankWeeklyJpaEntity (and mirror it on ProcessRankWeeklyEntity) by
annotating the entity `@Table` with an `@Index` (e.g. name="idx_year_week_ranking",
columnList="yearWeek, ranking"); ensure the column names in the index match the
entity field/column names, redeploy/migrate so the DDL is applied in the
environment, and validate with EXPLAIN that queries from
JpaPeriodRankingRepository (which currently builds PageRequest without a Sort)
use an index scan and not filesort; adjust the repository paging/sort callers
only if they would conflict with the JPQL ORDER BY.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankMonthlyEntity.java`:
- Around line 13-25: The unique constraint uses Java field names that Hibernate
will physical-name to snake_case, causing a mismatch; update the
`@UniqueConstraint` on ProductRankMonthlyEntity to reference the actual DB column
names (e.g., "product_id" and "year_month") or alternatively set explicit
`@Column`(name = "year_month")/@Column(name = "product_id") on the fields
productId and yearMonth so the constraint matches, and make the same change in
ProductRankWeeklyEntity; also add a DataJpaTest with Testcontainers Postgres and
spring.jpa.hibernate.ddl-auto=validate to catch naming/constraint mismatches.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankWeeklyEntity.java`:
- Around line 24-25: ProductRankWeeklyEntity의 yearWeek 컬럼 길이가 현재 length = 7으로
선언되어 있어 RankingPeriodKeyResolver.toYearWeek(...)가 생성하는 "YYYY-Www" (예:
"2026-W15", 길이 8) 값을 저장할 때 잘림 또는 예외가 발생할 수 있으니 ProductRankWeeklyEntity의 yearWeek
길이를 8으로 수정하고, ProductRankMonthlyEntity.yearMonth(포맷 "yyyy-MM", 길이 7)도 포맷과 일치하는지
확인·조정하여 불일치가 없도록 하세요; 또한 RankingPeriodKeyResolver.toYearWeek(...)에 대한 단위 테스트(반환
문자열 길이 및 포맷 검증)와 Testcontainers MySQL 기반의 주간 배치 통합 테스트를 추가해 실제 DB insert가 성공하는지
검증하십시오.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`:
- Around line 120-145: The current weeklyRankingProcessor captures an in-memory
AtomicInteger rankCounter that only increments and thus produces
incorrect/duplicated ranks on chunk retries/skips; to fix, remove the per-item
mutable rankCounter from weeklyRankingProcessor and instead produce stable ranks
by either (1) changing the reader to load the top N items into memory once and
sort/assign ranks inside the processor before emitting (suitable for
TOP_RANK_LIMIT ~100), (2) persist and restore the last index via
StepExecutionContext inside the step to resume a deterministic counter across
retries, or (3) prefer computing ranks in the query layer (use ROW_NUMBER()
OVER(...) in the repository) and map the DB-provided rank into
ProductRankWeeklyEntity; apply the same change pattern to
MonthlyRankingJobConfig where a similar rankCounter is used.
- Around line 59-67: The job deletes existing weekly data in weeklyCleanupStep
before weeklyAggregateStep, risking data loss if aggregation fails; change to an
atomic swap or safer sequence: implement Option A by writing aggregates to a
staging target (e.g., new yearWeek_tmp key or staging table) inside the method
backing weeklyAggregateStep, then add a final swap/rename step (e.g.,
weeklySwapStep) that runs only after successful aggregate to replace live data,
or implement Option B by changing the writer used by weeklyAggregateStep to
perform upserts (INSERT ... ON DUPLICATE KEY UPDATE) instead of
delete-then-insert and delete stale rows afterwards; also add
retry/fault-tolerant settings (faultTolerant().retryLimit(...)) on
weeklyAggregateStep and add a regression test that injects an exception into
weeklyAggregateStep to assert original data remains intact, and apply the same
changes to MonthlyRankingJobConfig (monthly cleanup/aggregate steps) and update
job builder (weeklyRankingJob/monthlyRankingJob) to include the new
swap/finalization step and listener for failures.
- Around line 105-118: The JPQL in weeklyMetricsReader builds the score using
string-concatenated WEIGHT_* constants and lacks a deterministic tie-breaker,
causing non-deterministic TOP_N and ranking; change the query to use named
parameters for weights (e.g. :weightView, :weightLike, :weightOrder) instead of
concatenating WEIGHT_* and bind those parameters when constructing the
JpaPagingItemReader, and add a stable tie-breaker to the ORDER BY such as
"m.productId ASC"; apply the same fix to MonthlyRankingJobConfig so both readers
use parameterized weights and a deterministic ORDER BY.
---
Duplicate comments:
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java`:
- Around line 19-24: toYearWeek currently uses date.getYear() causing
week-based-year mismatches at year boundaries; replace the year calculation with
date.get(weekFields.weekBasedYear()) in the toYearWeek method (using the
existing WeekFields weekFields variable) so the produced key matches the API's
week-based year; also add a small contract test comparing this
RankingPeriodKeyResolver.toYearWeek outputs against the API resolver for
boundary dates like 2023-01-01, 2024-12-30, 2024-12-31, 2025-01-01, 2027-01-01
to ensure byte-for-byte equality.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryService.java`:
- Around line 52-65: The switch in RankingQueryService embeds period-specific
logic (WEEKLY/MONTHLY/DAILY) causing scattered changes when adding new periods;
refactor by delegating ranking retrieval to a period-specific strategy: add
methods like getRankings(date, offset, size) and getTotalCount(date) on
RankingPeriod or introduce a RankingProvider interface and wire a
Map<RankingPeriod, RankingProvider> via Spring, then replace the switch in
RankingQueryService with a single call period.getRankings(...) /
period.getTotalCount(...) or lookup providerMap.get(period).getRankings(...) so
new periods (e.g. YEARLY) only require adding a new strategy bean and not
modifying the service, and remove direct repository calls
(periodRankingRepository / rankingRepository) from the switch.
- Around line 41-43: Add a regression unit test for RankingQueryService that
calls the 3-arg overload getRankings(LocalDate date, int page, int size) and
asserts it delegates to RankingPeriod.DAILY and returns the exact legacy
behavior: use the Redis ZSET-backed path (mock or spy the Redis ZSET/data source
used by the service) and verify the returned PageResult<RankingItemInfo> items
match the ZSET-derived results and that totalCount equals the unfiltered/raw
total (not filtered by any applied filters). In the test, explicitly exercise
RankingQueryService.getRankings(...), assert the service invoked the DAILY
branch (or that getRankings(date, page, size, RankingPeriod.DAILY) would be
used), mock the underlying repository/Redis client to return a known ZSET and
raw count, and assert the PageResult.totalCount matches that raw count to
prevent regressions.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryUseCase.java`:
- Around line 10-13: 인터페이스 RankingQueryUseCase가 3-인자 getRankings(LocalDate date,
int page, int size)와 4-인자 getRankings(LocalDate date, int page, int size,
RankingPeriod period)을 둘다 노출해 호출자 분산 위험이 있으니 3-인자 오버로드를 `@Deprecated로` 표시하거나 호출처
전환이 끝났다면 인터페이스에서 삭제하고, 기존 3-인자 구현(현재는 4-인자에 위임하는 구현)이 동일 동작을 보장하는지 확인하기 위해 통합
테스트를 추가하여 getRankings(date,page,size) 호출 결과가 getRankings(date,page,size,
RankingPeriod.DAILY)와 동일함을 검증하십시오.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/repository/PeriodRankingRepository.java`:
- Around line 8-17: The PeriodRankingRepository currently exposes
period-specific methods (getWeeklyRankings/getWeeklyTotalCount and
getMonthlyRankings/getMonthlyTotalCount) which will scale poorly as new periods
are added; change the interface PeriodRankingRepository to a period-agnostic API
by replacing those four methods with two methods: getRankings(RankingPeriod
period, LocalDate date, int offset, int size) and getTotalCount(RankingPeriod
period, LocalDate date), update implementing classes to route based on the
RankingPeriod enum (WEEKLY/MONTHLY/etc.), and adjust callers (e.g.,
RankingQueryService) to pass a RankingPeriod rather than calling period-specific
methods; add parameterized tests per RankingPeriod to cover each branch.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaPeriodRankingRepository.java`:
- Around line 25-45: The implementation of getWeeklyRankings and
getMonthlyRankings assumes offset is a multiple of size by doing
PageRequest.of(offset / size, size) and will throw on size==0; update both
methods (getWeeklyRankings and getMonthlyRankings) to validate inputs when
keeping the (date, offset, size) signature: if size <= 0 or offset < 0 or offset
% size != 0 throw an IllegalArgumentException (or CoreException used elsewhere)
with a clear message, then compute int page = offset / size and call
PageRequest.of(page, size); add unit tests covering non-multiple offset (e.g.,
offset=5,size=10) and size=0 to assert the exception is thrown.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductMetricsEntity.java`:
- Around line 14-32: ProductMetricsEntity is exposed as a mutable JPA entity but
is used only as batch Reader input; mark it immutable by adding
`@org.hibernate.annotations.Immutable` on the ProductMetricsEntity class, ensure
no setters are added (keep only getters), and update the batch Reader to pass
the JPA readOnly hint (e.g., setHint("org.hibernate.readOnly", true) or use
readOnly on the JpaPagingItemReader) so Hibernate treats instances as read-only;
additionally add a CI/ArchUnit or grep check to assert no code calls
EntityManager.persist/merge or repository.save for ProductMetricsEntity to
prevent accidental writes.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankMonthlyEntity.java`:
- Around line 48-71: The constructor ProductRankMonthlyEntity(Long productId,
String yearMonth, ...) and the updateRanking(...) method currently set updatedAt
= LocalDateTime.now(), which prevents deterministic tests and can cause timezone
inconsistencies; change this by removing direct LocalDateTime.now() calls and
instead (a) delegate timestamping to JPA lifecycle callbacks
(`@PrePersist/`@PreUpdate) that set updatedAt, or (b) add a time-provider to the
API (inject java.time.Clock or accept a LocalDateTime parameter) and use
Clock.systemUTC() or Clock injected in production to produce UTC timestamps;
update usages and tests to pass a fixed Clock (or explicit LocalDateTime) so
unit tests can assert updatedAt deterministically.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/repository/ProductRankMonthlyJpaRepository.java`:
- Around line 11-13: JPQL bulk DELETEs should clear the persistence context to
avoid stale entities: update the `@Modifying` annotation on
ProductRankMonthlyJpaRepository.deleteByYearMonth and likewise on
ProductRankWeeklyJpaRepository.deleteByYearWeek to include clearAutomatically =
true (i.e., `@Modifying`(clearAutomatically = true)), so the persistence context
is synchronized automatically after the bulk delete operation.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java`:
- Around line 15-17: parseDate currently calls LocalDate.parse(requestDate,
DATE_FORMAT) which lets DateTimeParseException propagate into job failure logs;
change parseDate to validate requestDate (null/empty) and wrap the
LocalDate.parse call in a try/catch catching DateTimeParseException (and
NullPointerException if you prefer) and rethrow an IllegalArgumentException that
includes the expected format (DATE_FORMAT pattern, e.g. "yyyyMMdd") and an
example (e.g. "20250101"), preserving the original exception as the cause;
update tests to cover null, empty, and wrong-format inputs (e.g. "yyyy-MM-dd")
to assert the IllegalArgumentException and that the cause is the
DateTimeParseException.
🪄 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: 15cc28ec-320d-4ef1-823a-e35a13283c01
⛔ Files ignored due to path filters (2)
claudedocs/10weeks/lec10.mdis excluded by!**/*.mdand included by**claudedocs/10weeks/todo.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (19)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryService.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingQueryUseCase.javaapps/commerce-api/src/main/java/com/loopers/domain/model/ranking/RankingPeriod.javaapps/commerce-api/src/main/java/com/loopers/domain/model/ranking/RankingPeriodKeyResolver.javaapps/commerce-api/src/main/java/com/loopers/domain/repository/PeriodRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/JpaPeriodRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/ProductRankMonthlyJpaEntity.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/ProductRankWeeklyJpaEntity.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductMetricsEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankMonthlyEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankWeeklyEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/repository/ProductRankMonthlyJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/repository/ProductRankWeeklyJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java
Comment on lines
+46
to
71
| public PageResult<RankingItemInfo> getRankings(LocalDate date, int page, int size, RankingPeriod period) { | ||
| int offset = page * size; | ||
| List<RankedProduct> rankedProducts = rankingRepository.getTopRankings(date, offset, size); | ||
|
|
||
| List<RankedProduct> rankedProducts; | ||
| long totalCount; | ||
|
|
||
| switch (period) { | ||
| case WEEKLY -> { | ||
| rankedProducts = periodRankingRepository.getWeeklyRankings(date, offset, size); | ||
| totalCount = periodRankingRepository.getWeeklyTotalCount(date); | ||
| } | ||
| case MONTHLY -> { | ||
| rankedProducts = periodRankingRepository.getMonthlyRankings(date, offset, size); | ||
| totalCount = periodRankingRepository.getMonthlyTotalCount(date); | ||
| } | ||
| default -> { | ||
| rankedProducts = rankingRepository.getTopRankings(date, offset, size); | ||
| totalCount = rankingRepository.getTotalCount(date); | ||
| } | ||
| } | ||
|
|
||
| if (rankedProducts.isEmpty()) { | ||
| return new PageResult<>(Collections.emptyList(), page, size, 0, 0); | ||
| } | ||
|
|
||
| long totalCount = rankingRepository.getTotalCount(date); | ||
| int totalPages = (int) Math.ceil((double) totalCount / size); |
There was a problem hiding this comment.
입력 가드 부재로 size/page 경계값에서 500이 발생한다.
현재 메서드는 size <= 0, page < 0에 대한 사전 검증이 없다. 아래 경로에서 즉시 런타임 예외가 발생한다:
size = 0: 하위JpaPeriodRankingRepository에서PageRequest.of(offset / size, size)→ArithmeticException, 또한 71행totalCount / size도 동일.page < 0:offset = page * size가 음수가 되어PageRequest.of에서IllegalArgumentException.
이 서비스는 컨트롤러 외에도 재사용될 수 있고, 컨트롤러가 Bean Validation을 쓰지 않는 정책(학습)이므로 유효성은 도메인/서비스 계층에서 막아야 한다. 진입부에서 CoreException(BAD_REQUEST)로 일관된 에러 응답을 반환하도록 가드를 추가할 것을 권장한다.
🛡️ 제안 diff
public PageResult<RankingItemInfo> getRankings(LocalDate date, int page, int size, RankingPeriod period) {
+ if (size <= 0 || page < 0 || period == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "invalid ranking query parameters");
+ }
int offset = page * size;추가 테스트로 size=0, page=-1, period=null 각각에 대해 표준 에러 포맷이 반환되는지 확인하는 단위 테스트를 제안한다.
As per coding guidelines("예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다.") — 예외 메시지는 사용자용/로그용을 분리해 민감한 쿼리 파라미터를 그대로 노출하지 않도록 주의한다.
🤖 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/RankingQueryService.java`
around lines 46 - 71, The getRankings method in RankingQueryService lacks input
guards for invalid pagination/period values; add validation at the start of
getRankings to check if size <= 0 or page < 0 or period == null and throw a
CoreException(BAD_REQUEST) with a user-facing message (e.g., "Invalid pagination
parameters") while preserving the original cause for logs and using a separate
detailed log message; ensure the thrown exception is created from the service
layer (not controller) and references the same behavior for both
periodRankingRepository and rankingRepository code paths so downstream
PageRequest creation and the totalCount/size division cannot divide by zero or
receive negative offsets.
Comment on lines
+3
to
+7
| public enum RankingPeriod { | ||
| DAILY, | ||
| WEEKLY, | ||
| MONTHLY | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=java -C3 'RankingPeriod\s+period'
rg -nP --type=java -C5 '@RequestParam.*RankingPeriod'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4467
🏁 Script executed:
#!/bin/bash
# Find ApiControllerAdvice or similar exception handlers
fd -t f -e java | xargs rg -l 'ApiControllerAdvice|@ControllerAdvice' | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Check full RankingPeriod enum with all annotations
cat -n apps/commerce-api/src/main/java/com/loopers/domain/model/ranking/RankingPeriod.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 241
🏁 Script executed:
#!/bin/bash
# Search for RankingController tests
find . -path '*test*' -name '*RankingController*' -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Search for custom converters or JsonCreator for enum handling
rg -nP --type=java '@JsonCreator|Converter.*Ranking|ConfigureWebMvc' apps/commerce-api/srcRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# Search for any ControllerAdvice or exception handler more broadly
rg -nP --type=java 'ControllerAdvice|RestControllerAdvice|ExceptionHandler' apps/commerce-api/src/mainRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1473
🏁 Script executed:
#!/bin/bash
# Search for all test files
find . -type f -name '*.java' | grep -i test | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1808
🏁 Script executed:
#!/bin/bash
# Check RankingController file completely
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingController.java | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2068
🏁 Script executed:
#!/bin/bash
# Check the full GlobalExceptionHandler to see how Exception is handled
cat -n apps/commerce-api/src/main/java/com/loopers/support/error/GlobalExceptionHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3933
🏁 Script executed:
#!/bin/bash
# Search specifically for RankingController tests
find . -type f -name '*.java' | xargs rg -l 'RankingController' | grep -i testRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
MethodArgumentTypeMismatchException에 대한 에러 처리 추가가 필수이다.
@RequestParam(defaultValue = "DAILY") RankingPeriod period 바인딩 시 잘못된 값("invalid", 소문자 "daily" 등)이 전달되면, Spring의 기본 Enum 변환기가 MethodArgumentTypeMismatchException을 발생시킨다. 현재 GlobalExceptionHandler는 이 예외에 대한 명시적 핸들러가 없어 제네릭 Exception 핸들러로 폴백되어 500 INTERNAL_ERROR를 반환한다. 이는 클라이언트 입력 오류(4xx)를 서버 오류(5xx)로 분류하는 것으로, 운영 모니터링과 클라이언트 에러 처리 전략이 일관되지 않는다.
다음을 적용할 것을 권고한다.
-
MethodArgumentTypeMismatchException 핸들러 추가: GlobalExceptionHandler에
@ExceptionHandler(MethodArgumentTypeMismatchException.class)추가하여 400 BAD_REQUEST로 응답하고, 응답 형식을{"code": "INVALID_PARAMETER", "message": "..."}로 표준화한다. -
테스트 케이스 추가:
period=invalid,period=daily(소문자),period=DAILY(정상),period누락(기본값 동작) 네 케이스에 대한 RankingController 통합 테스트를 추가한다.
🤖 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/model/ranking/RankingPeriod.java`
around lines 3 - 7, Add a specific handler for
MethodArgumentTypeMismatchException in GlobalExceptionHandler: implement an
`@ExceptionHandler`(MethodArgumentTypeMismatchException.class) method that returns
400 BAD_REQUEST with a standardized body like
{"code":"INVALID_PARAMETER","message":"..."} and ensure the message references
the invalid parameter name/value; reference the GlobalExceptionHandler class and
MethodArgumentTypeMismatchException symbol. Also add integration tests for
RankingController covering period=invalid, period=daily (lowercase),
period=DAILY (valid), and omitted period (uses default) to assert 400 for
invalid values and correct 200 responses for valid/default cases, using
RankingPeriod to validate expected behavior.
Comment on lines
+8
to
+23
| public final class RankingPeriodKeyResolver { | ||
|
|
||
| private static final DateTimeFormatter YEAR_MONTH_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM"); | ||
|
|
||
| private RankingPeriodKeyResolver() {} | ||
|
|
||
| public static String toYearWeek(LocalDate date) { | ||
| WeekFields weekFields = WeekFields.of(Locale.KOREA); | ||
| int year = date.getYear(); | ||
| int week = date.get(weekFields.weekOfWeekBasedYear()); | ||
| return String.format("%d-W%02d", year, week); | ||
| } | ||
|
|
||
| public static String toYearMonth(LocalDate date) { | ||
| return date.format(YEAR_MONTH_FORMAT); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
동일 로직이 commerce-batch 쪽 RankingPeriodKeyResolver와 완전히 중복된다.
apps/commerce-batch/src/main/java/com/loopers/batch/support/RankingPeriodKeyResolver.java와 주/월 키 산출 로직이 똑같이 존재한다. 한쪽만 수정되면 API와 배치가 서로 다른 키를 만들게 되어 랭킹이 비는 장애가 날 수 있다. 운영 관점에서 "어떤 경우에도 두 모듈이 같은 키를 생산해야 한다"는 불변을 코드 레벨에서 강제해야 한다.
- 공통 모듈(
commerce-domain/commerce-support등)로 추출해 두 모듈이 동일 구현을 참조하도록 리팩터링할 것을 권고한다. - 추출이 즉시 어렵다면, 최소한 두 Resolver 간 동일 출력 보장 계약 테스트(contract test)를 배치/ 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/domain/model/ranking/RankingPeriodKeyResolver.java`
around lines 8 - 23, The RankingPeriodKeyResolver logic (methods toYearWeek and
toYearMonth in class RankingPeriodKeyResolver) is duplicated between
apps/commerce-api and apps/commerce-batch; extract these methods and the
YEAR_MONTH_FORMAT constant into a shared module (e.g., commerce-domain or
commerce-support) and have both modules reference the single shared
RankingPeriodKeyResolver, or if extraction isn’t feasible immediately, add a
contract test that invokes RankingPeriodKeyResolver.toYearWeek(LocalDate) and
toYearMonth(LocalDate) from both API and batch with a common dataset to assert
identical outputs; update imports/usages to reference the shared resolver and
remove the duplicate class.
Comment on lines
+14
to
+19
| public static String toYearWeek(LocalDate date) { | ||
| WeekFields weekFields = WeekFields.of(Locale.KOREA); | ||
| int year = date.getYear(); | ||
| int week = date.get(weekFields.weekOfWeekBasedYear()); | ||
| return String.format("%d-W%02d", year, week); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Java WeekFields weekOfWeekBasedYear weekBasedYear difference Locale.KOREA
💡 Result:
In Java's java.time.temporal.WeekFields, weekOfWeekBasedYear returns a TemporalField for the week number within the week-based year, and weekBasedYear returns the TemporalField for the week-based year itself. These fields define a week-based-year system where each week belongs to exactly one year: Week 1 is the first week starting on the first day-of-week (e.g., Monday in ISO) with at least the minimal days in the first week (e.g., 4). The first/last weeks may overlap calendar years. Key difference from standard year/week-of-year: week-based-year can differ from the proleptic calendar year near year boundaries. Example from docs (Monday first, min 4 days): 2008-12-31 (Wed) is week 1 of 2009 in one config, week 53 of 2008 in another. For Locale.KOREA (ko_KR), WeekFields.of(Locale.KOREA) uses locale-specific firstDayOfWeek (Sunday, as Korean weekdays start with 일요일) and minimalDaysInFirstWeek (likely 1, similar to US). This affects week numbering: e.g., first days of calendar year may belong to prior week-based-year's last week. Usage example: WeekFields wf = WeekFields.of(Locale.KOREA); LocalDate date = LocalDate.of(2026, 1, 1); int wby = date.get(wf.weekBasedYear); // May be 2025 if Jan 1 in last week of prior WB-year int wowby = date.get(wf.weekOfWeekBasedYear); // e.g., 52 or 53 No explicit numerical "difference" found; the fields work together for week-date representation (WB-year + week-of-WB-year + day-of-week), differing from ChronoField.YEAR + weekOfYear by strict week-year alignment.
Citations:
- 1: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/time/temporal/WeekFields.html
- 2: https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/time/temporal/WeekFields.html
- 3: https://docs.oracle.com/javase/8/docs/api/java/time/temporal/WeekFields.html
- 4: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/temporal/WeekFields.html
- 5: https://zetcode.com/java/temporal-weekfields-class
- 6: https://www.geeksforgeeks.org/java/weekfields-of-method-in-java-with-examples/
- 7: https://stackoverflow.com/questions/52365194/how-to-get-the-first-day-of-week-for-a-certain-locale-country
date.getYear()와 weekOfWeekBasedYear()의 조합이 연말/연초 경계에서 잘못된 키를 생성한다.
weekOfWeekBasedYear()는 주(week) 기준 연도에 속하는 주차를 반환하므로, 짝이 되는 연도는 date.getYear()(달력 연도)가 아니라 weekFields.weekBasedYear()이어야 한다. Locale.KOREA(일요일 시작, 최소 1일)에서 연말/연초 경계일자의 달력 연도와 주 기준 연도가 달라질 수 있으며, 현재 구현은 "2025-W01"을 생성해야 할 날짜에 "2024-W01"을 생성할 수 있다. API의 RankingPeriodKeyResolver와 배치 Writer가 다른 키를 생성하면 API 조회 시 해당 주차 데이터가 조회되지 않아 사용자에게 "주간 랭킹 없음"으로 노출되는 장애를 초래한다.
🐛 제안 수정
public static String toYearWeek(LocalDate date) {
WeekFields weekFields = WeekFields.of(Locale.KOREA);
- int year = date.getYear();
- int week = date.get(weekFields.weekOfWeekBasedYear());
+ int year = date.get(weekFields.weekBasedYear());
+ int week = date.get(weekFields.weekOfWeekBasedYear());
return String.format("%d-W%02d", year, week);
}연말/연초 경계일자(2023-01-01, 2024-12-30, 2024-12-31, 2025-01-01 등)에 대해 API·배치 두 Resolver가 동일한 키를 생성하는지 검증하는 파라미터화 테스트를 추가해야 한다.
🤖 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/model/ranking/RankingPeriodKeyResolver.java`
around lines 14 - 19, The toYearWeek method currently mixes date.getYear() with
weekFields.weekOfWeekBasedYear(), which yields wrong keys around year
boundaries; change the year source to the week-based year via
date.get(weekFields.weekBasedYear()) so toYearWeek(LocalDate) returns
String.format("%d-W%02d", weekBasedYear, week) using
WeekFields.of(Locale.KOREA), and add a parameterized test that asserts
toYearWeek produces identical keys for known boundary dates (e.g., 2023-01-01,
2024-12-30, 2024-12-31, 2025-01-01) to ensure API and batch resolver parity.
Comment on lines
+10
to
+16
| List<RankedProduct> getWeeklyRankings(LocalDate date, int offset, int size); | ||
|
|
||
| long getWeeklyTotalCount(LocalDate date); | ||
|
|
||
| List<RankedProduct> getMonthlyRankings(LocalDate date, int offset, int size); | ||
|
|
||
| long getMonthlyTotalCount(LocalDate date); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
저장된 랭크의 1-based ↔ 0-based 변환 규약을 인터페이스 문서로 명시할 것을 권고한다.
동일 RankedProduct 레코드를 반환하지만, 일간(RankingRepository)과 기간(PeriodRankingRepository)의 rank 의미가 각각 "Redis 0-based"와 "MV 1-based → 0-based 변환값"으로 다르다(요약 정보 기준). 문서화가 없으면 호출측에서 +1 또는 -1 실수로 오프바이원 버그가 자라기 쉽다. 운영 관점에서는 "1위 상품이 2위로 보임" 같은 사용자 가시 장애가 된다.
- 인터페이스 Javadoc에
rank의 0-based 규약과 정렬 보장(동점 처리, ranking asc, productId tiebreaker 등)을 명시할 것을 권고한다. - 추가 테스트: 두 Repository 구현체에 대해 "첫 번째 요소의 rank == offset" 불변 검증 테스트를 권고한다.
🤖 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/repository/PeriodRankingRepository.java`
around lines 10 - 16, PeriodRankingRepository의
getWeeklyRankings/getMonthlyRankings가 반환하는 RankedProduct.rank의 기준(0-based인지
1-based인지)과 정렬·동점 처리 규칙이 명시되어 있지 않아 호출측에서 오프바이원 버그가 발생할 수 있으니 인터페이스 Javadoc에
rank는 0-based 기준임을 명시하고 정렬 보장(예: ranking 오름차순, 동점 시 productId로 tiebreaker)과
offset과의 관계(첫 요소의 rank == offset)까지 기술하세요; 동일 규약을 RankingRepository에도 반영해 문서
일관성을 유지하고, 두 Repository 구현체(PeriodRankingRepository와 RankingRepository)에 대해 "첫
번째 요소의 rank == offset" 불변을 검증하는 단위 테스트를 추가하세요.
Comment on lines
+24
to
+25
| @Column(nullable = false, length = 7) | ||
| private String yearWeek; |
There was a problem hiding this comment.
yearWeek 컬럼 길이 부족으로 저장 실패 가능성이 있다.
RankingPeriodKeyResolver.toYearWeek(...)는 String.format("%d-W%02d", year, week) 형식으로 "2026-W15"와 같이 8자 문자열을 생성한다. 그러나 현재 컬럼은 length = 7로 선언되어 있어, MySQL에서 VARCHAR(7)로 매핑되는 순간 insert 시 데이터 절단 혹은 DataException이 발생한다. 운영 관점에서 주간 배치가 전부 실패하는 장애로 이어질 수 있는 사안이다.
동일한 이슈가 ProductRankMonthlyEntity의 yearMonth(형식 "yyyy-MM", 7자)에도 반대 방향으로 영향을 줄 수 있으니 선언 길이와 실제 포맷의 정합성을 함께 점검해 달라.
추가로 다음 테스트를 권장한다.
RankingPeriodKeyResolver.toYearWeek(...)반환 문자열 길이에 대한 단위 테스트- 주간 배치를 Testcontainers MySQL 기반으로 실행해 실제 컬럼에 insert 되는지 검증하는 통합 테스트
🛠️ 제안 수정안
- `@Column`(nullable = false, length = 7)
+ `@Column`(nullable = false, length = 8)
private String yearWeek;📝 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
| @Column(nullable = false, length = 7) | |
| private String yearWeek; | |
| `@Column`(nullable = false, length = 8) | |
| private String yearWeek; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/entity/ProductRankWeeklyEntity.java`
around lines 24 - 25, ProductRankWeeklyEntity의 yearWeek 컬럼 길이가 현재 length = 7으로
선언되어 있어 RankingPeriodKeyResolver.toYearWeek(...)가 생성하는 "YYYY-Www" (예:
"2026-W15", 길이 8) 값을 저장할 때 잘림 또는 예외가 발생할 수 있으니 ProductRankWeeklyEntity의 yearWeek
길이를 8으로 수정하고, ProductRankMonthlyEntity.yearMonth(포맷 "yyyy-MM", 길이 7)도 포맷과 일치하는지
확인·조정하여 불일치가 없도록 하세요; 또한 RankingPeriodKeyResolver.toYearWeek(...)에 대한 단위 테스트(반환
문자열 길이 및 포맷 검증)와 Testcontainers MySQL 기반의 주간 배치 통합 테스트를 추가해 실제 DB insert가 성공하는지
검증하십시오.
Comment on lines
+59
to
+67
| @Bean(JOB_NAME) | ||
| public Job weeklyRankingJob() { | ||
| return new JobBuilder(JOB_NAME, jobRepository) | ||
| .incrementer(new RunIdIncrementer()) | ||
| .start(weeklyCleanupStep()) | ||
| .next(weeklyAggregateStep()) | ||
| .listener(jobListener) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
Cleanup → Aggregate 간 트랜잭션 경계 분리로 데이터 유실 위험이 있다.
weeklyCleanupStep은 이미 커밋된 상태에서 weeklyAggregateStep이 실행된다. 만약 Aggregate 단계에서 Reader/Writer 예외, DB 일시 장애, 배포 타이밍 이슈 등으로 실패할 경우, 기존 주간 랭킹은 이미 삭제되었고 신규 랭킹은 적재되지 않은 공백 상태로 API가 빈 결과를 반환하게 된다. 운영 관점에서 사용자 가시적 장애다.
개선 방안을 제안한다.
- 옵션 A: Aggregate를 먼저 새 키(예:
yearWeek_tmp또는 staging 테이블)에 적재한 뒤, 최종 단계에서 swap/rename 하는 패턴으로 원자성 확보 - 옵션 B:
upsert(JPA saveAll 대신INSERT ... ON DUPLICATE KEY UPDATE) + 작업 종료 후 stale row만 삭제하는 순서로 변경 - 옵션 C: 최소한 Aggregate 실패 시 재시도(
faultTolerant().retryLimit(...))/실패 알림을 보강하고, 실패 시 이전 데이터 자동 복구가 불가함을 운영 런북에 명시
추가 테스트: Aggregate 단계에서 강제 예외를 주입했을 때 기존 데이터가 삭제된 채 남지 않도록 하는 회귀 테스트를 권장한다.
동일한 구조가 MonthlyRankingJobConfig에도 그대로 존재하므로 함께 수정해 달라.
🤖 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 59 - 67, The job deletes existing weekly data in weeklyCleanupStep
before weeklyAggregateStep, risking data loss if aggregation fails; change to an
atomic swap or safer sequence: implement Option A by writing aggregates to a
staging target (e.g., new yearWeek_tmp key or staging table) inside the method
backing weeklyAggregateStep, then add a final swap/rename step (e.g.,
weeklySwapStep) that runs only after successful aggregate to replace live data,
or implement Option B by changing the writer used by weeklyAggregateStep to
perform upserts (INSERT ... ON DUPLICATE KEY UPDATE) instead of
delete-then-insert and delete stale rows afterwards; also add
retry/fault-tolerant settings (faultTolerant().retryLimit(...)) on
weeklyAggregateStep and add a regression test that injects an exception into
weeklyAggregateStep to assert original data remains intact, and apply the same
changes to MonthlyRankingJobConfig (monthly cleanup/aggregate steps) and update
job builder (weeklyRankingJob/monthlyRankingJob) to include the new
swap/finalization step and listener for failures.
Comment on lines
+105
to
+118
| public JpaPagingItemReader<ProductMetricsEntity> weeklyMetricsReader() { | ||
| return new JpaPagingItemReaderBuilder<ProductMetricsEntity>() | ||
| .name("weeklyMetricsReader") | ||
| .entityManagerFactory(entityManagerFactory) | ||
| .queryString( | ||
| "SELECT m FROM ProductMetricsEntity m " + | ||
| "ORDER BY (m.viewCount * " + WEIGHT_VIEW + | ||
| " + m.likeCount * " + WEIGHT_LIKE + | ||
| " + m.orderCount * " + WEIGHT_ORDER + ") DESC" | ||
| ) | ||
| .pageSize(TOP_RANK_LIMIT) | ||
| .maxItemCount(TOP_RANK_LIMIT) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
ORDER BY에 안정적 타이브레이커가 없어 결과가 비결정적이다.
현재 JPQL은 가중 점수 단일 키로만 정렬한다. 동일 점수를 가진 상품이 다수 존재하는 경우, DB/인덱스 상태에 따라 TOP 100 선정과 rank 부여가 실행마다 달라질 수 있다. 주간/월간 랭킹은 사용자 노출 데이터이므로 같은 입력에 대해 같은 순위가 보장되어야 한다.
또한 현재 점수 계산식을 JPQL 문자열에 WEIGHT_* 상수를 문자열 연결로 삽입하고 있어, double 값이 로케일에 따라 포맷되거나 향후 가중치 변경 시 Reader와 Processor 사이의 계산식 불일치가 발생할 수 있다. 두 곳에서 동일 수식을 문자열로 중복 관리하는 것도 유지보수 리스크다.
개선 방안을 제안한다.
- ORDER BY에
m.productId ASC등 고유 타이브레이커 추가 - 가중치는
:weightView등 named parameter로 바인딩하거나, 계산 로직을 Processor 한 곳에만 두고 Reader는 원천 정렬(예: 주요 단일 컬럼 정렬 + 전체 로딩 후 Processor 내 정렬)로 단순화
추가 테스트: 동점 데이터를 포함한 fixture로 TOP 100 및 ranking 값이 결정적으로 재현되는지 검증한다.
동일 이슈가 MonthlyRankingJobConfig(Line 105-118)에도 존재한다.
🛠️ 제안 수정안 (타이브레이커 추가)
.queryString(
"SELECT m FROM ProductMetricsEntity m " +
"ORDER BY (m.viewCount * " + WEIGHT_VIEW +
" + m.likeCount * " + WEIGHT_LIKE +
- " + m.orderCount * " + WEIGHT_ORDER + ") DESC"
+ " + m.orderCount * " + WEIGHT_ORDER + ") DESC, m.productId ASC"
)📝 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 JpaPagingItemReader<ProductMetricsEntity> weeklyMetricsReader() { | |
| return new JpaPagingItemReaderBuilder<ProductMetricsEntity>() | |
| .name("weeklyMetricsReader") | |
| .entityManagerFactory(entityManagerFactory) | |
| .queryString( | |
| "SELECT m FROM ProductMetricsEntity m " + | |
| "ORDER BY (m.viewCount * " + WEIGHT_VIEW + | |
| " + m.likeCount * " + WEIGHT_LIKE + | |
| " + m.orderCount * " + WEIGHT_ORDER + ") DESC" | |
| ) | |
| .pageSize(TOP_RANK_LIMIT) | |
| .maxItemCount(TOP_RANK_LIMIT) | |
| .build(); | |
| } | |
| public JpaPagingItemReader<ProductMetricsEntity> weeklyMetricsReader() { | |
| return new JpaPagingItemReaderBuilder<ProductMetricsEntity>() | |
| .name("weeklyMetricsReader") | |
| .entityManagerFactory(entityManagerFactory) | |
| .queryString( | |
| "SELECT m FROM ProductMetricsEntity m " + | |
| "ORDER BY (m.viewCount * " + WEIGHT_VIEW + | |
| " + m.likeCount * " + WEIGHT_LIKE + | |
| " + m.orderCount * " + WEIGHT_ORDER + ") DESC, m.productId ASC" | |
| ) | |
| .pageSize(TOP_RANK_LIMIT) | |
| .maxItemCount(TOP_RANK_LIMIT) | |
| .build(); | |
| } |
🤖 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 105 - 118, The JPQL in weeklyMetricsReader builds the score using
string-concatenated WEIGHT_* constants and lacks a deterministic tie-breaker,
causing non-deterministic TOP_N and ranking; change the query to use named
parameters for weights (e.g. :weightView, :weightLike, :weightOrder) instead of
concatenating WEIGHT_* and bind those parameters when constructing the
JpaPagingItemReader, and add a stable tie-breaker to the ORDER BY such as
"m.productId ASC"; apply the same fix to MonthlyRankingJobConfig so both readers
use parameterized weights and a deterministic ORDER BY.
Comment on lines
+120
to
+145
| @StepScope | ||
| @Bean | ||
| public ItemProcessor<ProductMetricsEntity, ProductRankWeeklyEntity> weeklyRankingProcessor( | ||
| @Value("#{jobParameters['requestDate']}") String requestDate | ||
| ) { | ||
| AtomicInteger rankCounter = new AtomicInteger(0); | ||
| String yearWeek = resolveYearWeek(requestDate); | ||
|
|
||
| return metrics -> { | ||
| int rank = rankCounter.incrementAndGet(); | ||
| double score = metrics.getViewCount() * WEIGHT_VIEW | ||
| + metrics.getLikeCount() * WEIGHT_LIKE | ||
| + metrics.getOrderCount() * WEIGHT_ORDER; | ||
|
|
||
| return new ProductRankWeeklyEntity( | ||
| metrics.getProductId(), | ||
| yearWeek, | ||
| metrics.getLikeCount(), | ||
| metrics.getOrderCount(), | ||
| metrics.getViewCount(), | ||
| metrics.getTotalSalesAmount(), | ||
| score, | ||
| rank | ||
| ); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Processor의 rankCounter는 재시작/재시도에 안전하지 않다.
@StepScope Bean으로 AtomicInteger rankCounter = new AtomicInteger(0)를 캡처하지만, Spring Batch에서 청크 재시도(faultTolerant 도입 시)나 스킵 발생 시 동일 아이템이 다시 처리되어도 카운터는 증가만 한다. 현재 구성(CHUNK_SIZE=1000, TOP_RANK_LIMIT=100)에서는 단일 청크라 우연히 문제가 드러나지 않으나, 향후 청크 크기 조정·fault-tolerant 옵션 추가 시 ranking 값이 어긋나거나 중복될 수 있다. 운영 관점에서 표시 순위가 틀어지는 사용자 가시적 결함으로 이어진다.
개선 방안을 제안한다.
- Reader에서 전체 상위 N건을 한 번에 메모리에 올린 뒤 Processor에서 정렬·순위 부여(현재 TOP 100 규모에서는 실용적)
- 또는
StepExecutionContext에 마지막 처리 인덱스를 저장해 재시작 시 복원 - 또는 ranking을 DB 윈도우 함수(
ROW_NUMBER() OVER (...))로 한 번에 산출하는 쿼리 기반 적재로 전환
추가 테스트: 스킵/리트라이 시나리오에서 ranking이 1..N 연속·유일한지 검증하는 테스트를 권장한다.
MonthlyRankingJobConfig(Line 120-145)도 동일하다.
🤖 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 120 - 145, The current weeklyRankingProcessor captures an in-memory
AtomicInteger rankCounter that only increments and thus produces
incorrect/duplicated ranks on chunk retries/skips; to fix, remove the per-item
mutable rankCounter from weeklyRankingProcessor and instead produce stable ranks
by either (1) changing the reader to load the top N items into memory once and
sort/assign ranks inside the processor before emitting (suitable for
TOP_RANK_LIMIT ~100), (2) persist and restore the last index via
StepExecutionContext inside the step to resume a deterministic counter across
retries, or (3) prefer computing ranks in the query layer (use ROW_NUMBER()
OVER(...) in the repository) and map the DB-provided rank into
ProductRankWeeklyEntity; apply the same change pattern to
MonthlyRankingJobConfig where a similar rankCounter is used.
Comment on lines
+147
to
+157
| @StepScope | ||
| @Bean | ||
| public ItemWriter<ProductRankWeeklyEntity> weeklyRankingWriter( | ||
| @Value("#{jobParameters['requestDate']}") String requestDate | ||
| ) { | ||
| return chunk -> { | ||
| List<? extends ProductRankWeeklyEntity> items = chunk.getItems(); | ||
| weeklyRepository.saveAll(items); | ||
| log.info("주간 랭킹 {} 건 저장 완료", items.size()); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Writer에 주입된 requestDate가 사용되지 않는다.
weeklyRankingWriter(@value("#{jobParameters['requestDate']}") String requestDate)에서 requestDate가 전혀 사용되지 않는다. 불필요한 SpEL 평가 비용이 발생하고, 독자에게 "Writer가 날짜에 의존한다"는 잘못된 신호를 준다. 운영 관점에서는 "writer가 실제로 무엇에 의존하는가"를 로그/모니터링 시 오해하게 만든다.
로깅에 yearWeek를 남기고 싶다면 실제로 사용하도록 바꾸거나, 그렇지 않다면 파라미터를 제거해 달라.
MonthlyRankingJobConfig(Line 147-157)도 동일하다.
🛠️ 제안 수정안
`@StepScope`
`@Bean`
- public ItemWriter<ProductRankWeeklyEntity> weeklyRankingWriter(
- `@Value`("#{jobParameters['requestDate']}") String requestDate
- ) {
+ public ItemWriter<ProductRankWeeklyEntity> weeklyRankingWriter(
+ `@Value`("#{jobParameters['requestDate']}") String requestDate
+ ) {
+ String yearWeek = resolveYearWeek(requestDate);
return chunk -> {
List<? extends ProductRankWeeklyEntity> items = chunk.getItems();
weeklyRepository.saveAll(items);
- log.info("주간 랭킹 {} 건 저장 완료", items.size());
+ log.info("주간 랭킹 저장 완료: yearWeek={}, count={}", yearWeek, items.size());
};
}
ukukdin
changed the title
Author
|
질문사항주간/월간 랭킹 기간은 크게 두 가지로 정의할 수 있을 것 같습니다.a 사용자 입장에서는 슬라이딩 윈도우가 더 자연스러울 것 같은데, 실무에서는 어떤 기준으로 선택하나요? 비용 트레이드오프 판단 기준 2. MV 테이블 설계 차이 3. 짧은 주기 배치 vs 실시간 스트림 |
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일간 집계 테이블을 Chunk-Oriented Processing으로 읽어 주간/월간 TOP 100 랭킹을 Materialized View(조회 전용 테이블)에 적재한다. API는period파라미터로 일간/주간/월간 랭킹을 단일 엔드포인트에서 제공한다.weeklyRankingJob,monthlyRankingJob두 Batch Job이 Cleanup Tasklet → Chunk Step(Reader/Processor/Writer) 구조로 동작하며,mv_product_rank_weekly,mv_product_rank_monthlyMV 테이블에 적재한다. API는period=DAILY일 때 Redis ZSET,WEEKLY/MONTHLY일 때 MV 테이블을 조회한다.🧭 Context & Decision
문제 정의
product_metrics테이블에 누적 집계(viewCount, likeCount, orderCount, totalSalesAmount)가 존재하지만 주간/월간 랭킹으로 가공되지 않고 있다.ranking:all:{yyyyMMdd})로 설계되어 있어 7일치/30일치 ZUNIONSTORE는 키 개수 × 상품 수만큼의 비용이 든다.requestDate파라미터를 받아 해당 주/월의 TOP 100 랭킹을 MV 테이블에 적재한다.GET /api/v1/rankings?period=WEEKLY&date=20260417으로 주간 랭킹이 조회된다.period=DAILY)이 그대로 동작한다.선택지와 결정
1. 배치 처리 모델 — Chunk-Oriented vs Tasklet 단독
최종 결정: B — 표준 Chunk-Oriented Processing 채택.
근거: Round 10의 학습 목표가 "Spring Batch의 Chunk-Oriented Processing"이므로 B가 가장 적합하다. 실무에서도
product_metrics가 수십만 건으로 늘어나면 A의 메모리 전량 적재는 OOM 위험이 있고, C는 Batch 프레임워크의 장점(청크 단위 재시작,StepExecution모니터링)을 포기하게 된다.Cleanup Step은 Tasklet으로 분리: 기존 데이터 삭제는 단발성 작업이므로 Reader/Processor/Writer가 필요 없다.
DELETE FROM mv_product_rank_weekly WHERE yearWeek = :yearWeek를 Tasklet 한 줄로 처리하고, 집계는 Chunk Step에서 처리한다. 이렇게 Job = Cleanup Tasklet Step → Aggregate Chunk Step으로 구성했다.2. MV 테이블 설계 — rank를 저장할 것인가, 조회 시 계산할 것인가
최종 결정: A —
ranking컬럼에 1~100 순위를 미리 저장.근거: MV의 본질은 "복잡한 집계 쿼리를 미리 계산해둔 조회 전용 구조"다. 조회 시마다 정렬하면 MV의 의미가 반감된다. 배치 Job이 score 내림차순으로 읽으면서 Processor에서
AtomicInteger로 순위를 부여하므로 적재 비용은 무시할 수 있다. 조회는WHERE yearWeek = ? ORDER BY ranking ASC로 인덱스 scan만 하면 된다.3. 데이터 소스 분리 — 일간(Redis) vs 주간/월간(DB MV)
최종 결정: C — period별 데이터 소스 분리.
근거: 일간 랭킹은 이벤트 발생 즉시 Redis ZSET에 반영되는 실시간 시스템이다. 이걸 DB 조회로 바꾸면 Round 9의 설계를 부정하는 셈이다. 반면 주간/월간은 하루 한 번 배치로 충분하므로 Redis에 올릴 이유가 없다. 조회 경로가 두 갈래로 나뉘지만, 이는
RankingRepository(일간/Redis)와PeriodRankingRepository(주간·월간/JPA) 인터페이스 분리로 깔끔하게 해결된다.4. 인터페이스 설계 — RankingRepository 확장 vs 분리
RankingRepository에 weekly/monthly 메서드 추가PeriodRankingRepository별도 분리최종 결정: B —
PeriodRankingRepository도메인 인터페이스 분리.근거: A를 먼저 구현해봤는데,
RedisRankingRepository에ProductRankWeeklyJpaRepository를 주입해야 하는 상황이 되었다. 이름은 "Redis"인데 JPA를 알고 있는 건 명백한 SRP 위반이다.RankingRepository는 "일간 실시간 랭킹"이라는 하나의 변경 원인을 가지고,PeriodRankingRepository는 "기간별 배치 집계 랭킹"이라는 다른 변경 원인을 가진다. 분리가 맞다.5. yearWeek 키 형식 — ISO 주차 vs 한국 로케일
최종 결정:
WeekFields.of(Locale.KOREA)기반의yyyy-Www형식(예:2026-W16).근거: ISO 8601의
WeekFields.ISO는 월요일 시작이지만, 한국 비즈니스에서는 일요일 시작이 일반적이다.Locale.KOREA를 사용하면 한국 기준 주차를 자동 계산한다. 키 형식은RankingPeriodKeyResolver유틸에 한 곳으로 모아 batch/api 양쪽에서 동일하게 사용한다.추후 개선 여지
product_metrics는 누적 카운터이므로, 주간 배치와 월간 배치가 동일한 시점의 스냅샷을 읽는다. 일별 스냅샷(product_metrics_daily)을 도입하면 진정한 기간별 차분 집계(이번 주 좋아요 수 = 이번 주 - 지난 주)가 가능하다.requestDate파라미터로 수동 실행한다. Spring Scheduler나 K8s CronJob으로 주간(매주 월요일 00:05), 월간(매월 1일 00:05)에 자동 실행하도록 확장할 수 있다.🏗️ Design Overview
변경 범위
apps/commerce-batch— 주간/월간 랭킹 배치 Job 신규apps/commerce-api— 랭킹 API period 파라미터 확장, MV 조회 경로 추가WeeklyRankingJobConfig,MonthlyRankingJobConfig,ProductMetricsEntity,ProductRankWeeklyEntity,ProductRankMonthlyEntity,RankingPeriodKeyResolverRankingPeriodenum,RankingPeriodKeyResolver,PeriodRankingRepository(domain interface),JpaPeriodRankingRepository(infrastructure),ProductRankWeeklyJpaEntity,ProductRankMonthlyJpaEntity주요 컴포넌트 책임
WeeklyRankingJobConfigMonthlyRankingJobConfigJpaPagingItemReaderproduct_metrics에서 score 가중합 내림차순 TOP 100 페이징 조회ItemProcessorAtomicInteger기반 순위 부여 + MV 엔티티 변환ItemWritermv_product_rank_weekly/mv_product_rank_monthly)에saveAll적재RankingPeriodKeyResolverLocalDate→yearWeek(2026-W16) /yearMonth(2026-04) 변환 유틸 (중복 제거)PeriodRankingRepositoryRankingRepository(일간/Redis)와 분리하여 ISP/SRP 준수JpaPeriodRankingRepositoryPeriodRankingRepositoryJPA 구현체 — MV 테이블에서 yearWeek/yearMonth 기준 페이징 조회RankingQueryServiceDAILY→RankingRepository(Redis),WEEKLY/MONTHLY→PeriodRankingRepository(JPA MV)RankingControllerperiod파라미터 추가:GET /api/v1/rankings?period=DAILY|WEEKLY|MONTHLY&date=yyyyMMdd&page=0&size=20🔁 Flow Diagram
배치 집계 흐름 (주간/월간 공통)
sequenceDiagram autonumber participant Scheduler as Cron / 수동 실행 participant Job as Spring Batch Job participant Tasklet as Cleanup Tasklet participant Reader as JpaPagingItemReader participant DB as product_metrics participant Processor as ItemProcessor participant Writer as ItemWriter participant MV as mv_product_rank_weekly/monthly Scheduler->>Job: weeklyRankingJob (requestDate=20260417) Note over Job,MV: Step 1 — Cleanup (Tasklet) Job->>Tasklet: 실행 Tasklet->>MV: DELETE WHERE yearWeek = '2026-W16' Tasklet-->>Job: FINISHED Note over Job,MV: Step 2 — Aggregate (Chunk-Oriented) Job->>Reader: open() loop 청크 단위 (pageSize=100, maxItemCount=100) Reader->>DB: SELECT * FROM product_metrics ORDER BY score DESC (TOP 100) DB-->>Reader: ProductMetricsEntity 청크 Reader->>Processor: 청크 전달 Processor->>Processor: score 계산 + ranking 순위 부여 Processor-->>Writer: ProductRankWeeklyEntity 청크 Writer->>MV: saveAll() — MV 테이블 적재 end Job-->>Scheduler: Job COMPLETEDAPI 조회 흐름 (period별 라우팅)
sequenceDiagram autonumber participant Client participant Controller as RankingController participant Service as RankingQueryService participant Redis as RankingRepository (Redis ZSET) participant JPA as PeriodRankingRepository (JPA MV) participant ProductDB as ProductRepository participant BrandDB as BrandRepository Client->>Controller: GET /api/v1/rankings?period=WEEKLY&date=20260417&page=0&size=20 Controller->>Service: getRankings(date, page, size, WEEKLY) alt period = DAILY Service->>Redis: getTopRankings(date, offset, size) Redis-->>Service: RankedProduct[] (Redis ZREVRANGE) else period = WEEKLY Service->>JPA: getWeeklyRankings(date, offset, size) JPA-->>Service: RankedProduct[] (mv_product_rank_weekly) else period = MONTHLY Service->>JPA: getMonthlyRankings(date, offset, size) JPA-->>Service: RankedProduct[] (mv_product_rank_monthly) end Service->>ProductDB: findAllActiveByIds(productIds) ProductDB-->>Service: Product[] Service->>BrandDB: findAllByIds(brandIds) BrandDB-->>Service: Brand[] Service->>Service: RankedProduct + Product + Brand → RankingItemInfo 조합 Service-->>Controller: PageResult<RankingItemInfo> Controller-->>Client: PageResponse<RankingItemResponse>아키텍처 레이어 의존성 (Hexagonal)
graph TD subgraph interfaces RC[RankingController] end subgraph application RQS[RankingQueryService] end subgraph domain RR[RankingRepository] PRR[PeriodRankingRepository] end subgraph infrastructure Redis[RedisRankingRepository] JPA[JpaPeriodRankingRepository] end RC --> RQS RQS --> RR RQS --> PRR Redis -.implements.-> RR JPA -.implements.-> PRR style RR fill:#e1f5fe style PRR fill:#e1f5fe style Redis fill:#fff3e0 style JPA fill:#fff3e0✅ Checklist
🧱 Spring Batch
weeklyRankingJob,monthlyRankingJob이requestDateJob Parameter로 동작,@ConditionalOnProperty로 Job 이름 기반 활성화JpaPagingItemReader→ItemProcessor→ItemWriterChunk 방식mv_product_rank_weekly(yearWeek + ranking),mv_product_rank_monthly(yearMonth + ranking) 테이블에 score/rank 사전 계산하여 적재🧩 Ranking API
period=DAILY(Redis ZSET),WEEKLY/MONTHLY(MV 테이블)로 데이터 소스 분리, 단일 엔드포인트에서 제공🤖 Generated with Claude Code