[volume-10] Batch 기반 주간·월간 랭킹 시스템 구현 - 박시온#416
Open
hey-sion wants to merge 7 commits intoLoopers-dev-lab:hey-sionfrom
Open
[volume-10] Batch 기반 주간·월간 랭킹 시스템 구현 - 박시온#416hey-sion wants to merge 7 commits intoLoopers-dev-lab:hey-sionfrom
hey-sion wants to merge 7 commits intoLoopers-dev-lab:hey-sionfrom
Conversation
📝 WalkthroughWalkthrough결제 완료 이벤트 페이로드를 재설계하여 개별 상품 정보를 포함하도록 변경하고, 다중 유형 스냅샷 기반 랭킹 시스템을 추가하며, 일일 상품 메트릭스 추적을 도입하고, 배치 기반 순위 집계 작업을 구현했다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Controller as RankingV1Controller
participant Facade as RankingFacade
participant SnapshotRepo as ProductRankSnapshotQueryRepository
participant DB as 데이터베이스
Client->>Controller: GET /api/v1/rankings?rankingType=WEEKLY&date=2026-04-01
Controller->>Controller: resolveDate(date, WEEKLY)
Controller->>Facade: getRankings(2026-04-01, WEEKLY, page, size)
Facade->>SnapshotRepo: findRankings(WEEKLY, 2026-04-01, offset, size)
SnapshotRepo->>DB: SELECT * FROM product_rank_snapshot<br/>WHERE ranking_type='WEEKLY' AND rank_date='2026-04-01'<br/>ORDER BY rank_position ASC<br/>LIMIT size OFFSET offset
DB-->>SnapshotRepo: ProductRankSnapshot 리스트
SnapshotRepo-->>Facade: 조회 결과 반환
Facade->>Facade: 각 스냅샷을 RankingProductInfo로 매핑
Facade-->>Controller: RankingPageResult 반환
Controller-->>Client: JSON 응답 (rank, score, productName 등)
Note over Facade,DB: 날짜가 null인 경우:<br/>findLatestRankDate(WEEKLY)로<br/>최신 순위 날짜를 조회
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductUnlikedEventHandler.java (1)
29-33:⚠️ Potential issue | 🟡 Minor
ProductViewedEventHandler와 동일한 원자성·타임존 이슈가 적용된다.집계/일별 두 decrement upsert의 트랜잭션 경계와
Clock타임존 고정에 대해 루트 코멘트의 지적이 동일하게 해당된다. 추가로, 일별 테이블의decrementLikeCount도 집계와 동일하게 음수 방지(GREATEST(..., 0)) 정책이 적용되어 있는지 확인 부탁한다. 일별 테이블은 "델타" 성격이라 음수 허용이 의도일 수 있으나(좋아요 취소가 이전 일자의 증가를 상쇄), 취소만 들어온 날의 일별 값이 0 아래로 내려가면 주간/월간 집계에서 의도치 않은 마이너스 기여가 발생할 수 있다. 정책을 문서화하고 테스트로 고정하는 편이 안전하다.추가 테스트: 같은 날 like가 없는 상태에서 unlike만 수신했을 때 일별
like_count가 어떤 값이 되는지 검증하는 테스트.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductUnlikedEventHandler.java` around lines 29 - 33, ProductUnlikedEventHandler.handle currently calls productMetricsRepository.decrementLikeCount and productMetricsDailyRepository.decrementLikeCount separately and uses LocalDate.now(clock) — make the two decrements atomic and timezone-consistent: wrap both calls in a single transaction (e.g., annotate handle with `@Transactional` or call a transactional service method) and ensure the same injected Clock is used to compute LocalDate once at the start (store LocalDate now = LocalDate.now(clock) and pass it to productMetricsDailyRepository). Also verify/modify productMetricsDailyRepository.decrementLikeCount to guard against negative counts (use database-side GREATEST(...,0) or equivalent) or document the delta semantics and add a unit/integration test (e.g., ProductUnlikedEventHandlerTest_unlikeWithoutPriorLike_resultsInZeroDailyCount) that asserts an unlike on a day with no prior like does not drive daily/aggregate counts below zero.
🧹 Nitpick comments (20)
apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.java (1)
14-48:@Modifying에clearAutomatically/flushAutomatically지정을 검토한다네 개의 네이티브 upsert 모두
@Modifying만 지정되어 있다. 현재 호출부(이벤트 핸들러)는 쓰기 직후 읽지 않으므로 당장은 문제없으나, 추후 동일 트랜잭션 내에서 엔티티 조회가 섞이면 1차 캐시가 최신값을 반영하지 못해 stale read가 발생할 수 있다. 네이티브 upsert는 특성상 영속성 컨텍스트에 반영되지 않으므로clearAutomatically = true, flushAutomatically = true를 함께 지정하는 편이 안전하다.♻️ 제안 diff
- `@Modifying` + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`(value = """ INSERT INTO product_metrics_daily ... """, nativeQuery = true)
**/*Repository*.java가이드라인에서 "영속성 컨텍스트 오염 가능성을 점검한다"는 지침을 따른 제안이다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.java` around lines 14 - 48, The native upsert methods (incrementLikeCount, decrementLikeCount, incrementViewCount, incrementOrderLineCountAndAmount) use `@Modifying` without clearing or flushing the persistence context, which can cause stale reads if the same transaction later queries these entities; update each `@Modifying` annotation to include clearAutomatically = true and flushAutomatically = true so the persistence context is flushed and cleared after the native query executes, ensuring subsequent reads see the upsert results.apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDailyId.java (1)
7-7:serialVersionUID명시를 권장한다
Serializable을 구현하는@IdClass타입에서serialVersionUID가 없으면 JVM/컴파일러별 자동 생성값에 의존하게 되어, 필드/애노테이션 변경이 있을 때 직렬화 호환성 경고 및 드물게 세션 복제·2차 캐시 직렬화 과정에서 잡기 어려운 이슈가 발생할 수 있다.♻️ 제안 diff
public class ProductMetricsDailyId implements Serializable { + private static final long serialVersionUID = 1L; private LocalDate metricDate; private Long productId;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDailyId.java` at line 7, ProductMetricsDailyId implements Serializable but lacks a serialVersionUID; add a private static final long serialVersionUID field to the ProductMetricsDailyId class (e.g. private static final long serialVersionUID = 1L;) to ensure stable serialization for this `@IdClass` type and avoid JVM/compiler-dependent generated values when fields/annotations change.modules/jpa/src/main/java/com/loopers/domain/ranking/RankingType.java (1)
3-14: enum 값 자체에days를 필드로 보유하는 방식을 검토한다
getDays()의switch는 향후QUARTERLY,YEARLY등이 추가될 때 한 곳을 더 수정해야 하며, 값과 로직이 분리되어 있다. enum 생성자에서days를 불변 필드로 보유하면 추가 케이스 누락 시 컴파일이 강제되지 않는switch대비 선언부에서 일원화된다.♻️ 제안 diff
-public enum RankingType { - DAILY, - WEEKLY, - MONTHLY; - - public int getDays() { - return switch (this) { - case DAILY -> 1; - case WEEKLY -> 7; - case MONTHLY -> 30; - }; - } -} +public enum RankingType { + DAILY(1), + WEEKLY(7), + MONTHLY(30); + + private final int days; + + RankingType(int days) { + this.days = days; + } + + public int getDays() { + return days; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/jpa/src/main/java/com/loopers/domain/ranking/RankingType.java` around lines 3 - 14, The enum RankingType uses a switch in getDays(), separating data from declaration; refactor RankingType to store a final int days field set via a private constructor on each constant (e.g., DAILY(1), WEEKLY(7), MONTHLY(30)) and change getDays() to return this.days; update the enum constants to pass their day values and remove the switch so adding new constants requires supplying the days at declaration time.apps/commerce-streamer/src/test/java/com/loopers/infrastructure/ProductMetricsDailyRepositoryTest.java (2)
141-156: 금액 산술은long기반으로 표현해야 실제 상품 가격 기준의 오버플로를 방지할 수 있다
int unitPrice = 3000; int quantity = 3;이후long orderAmount = unitPrice * quantity는 곱셈이int에서 수행된 뒤long으로 확장된다. 예제 값은 문제없지만 실제 상용 가격(고가 상품 x 수량)에서는int오버플로 후 음수가long에 저장될 수 있다. 테스트는 프로덕션 코드의 계산 컨벤션을 암묵적으로 규정하는 역할도 하므로, 금액은 처음부터long으로 표현하는 편이 안전하다.♻️ 제안 diff
- int unitPrice = 3000; - int quantity = 3; - long orderAmount = unitPrice * quantity; + long unitPrice = 3000L; + long quantity = 3L; + long orderAmount = unitPrice * quantity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/java/com/loopers/infrastructure/ProductMetricsDailyRepositoryTest.java` around lines 141 - 156, Summary: Use long arithmetic for price/quantity to avoid int overflow in ProductMetricsDailyRepositoryTest. In ProductMetricsDailyRepositoryTest update the test variables so multiplication is performed in long (e.g., change unitPrice and/or quantity to long literals or cast before multiplication) and compute orderAmount as a long from the start (replace int unitPrice, int quantity, long orderAmount = unitPrice * quantity with long-aware declarations and calculation) so the test reflects the production convention and avoids int overflow when calling incrementOrderLineCountAndAmount(PRODUCT_ID, TODAY, orderAmount).
46-135: 경계·실패 케이스 보강을 권장한다
**/*Test*.java가이드라인은 경계값/실패 케이스/예외 흐름 커버리지를 요구한다. 현재 테스트는 성공 누적 시나리오 위주로, 다음 케이스를 추가할 것을 권장한다.
- 다른
productIdx 같은metricDate에서 별도 행으로 생성되고 상호 간섭이 없는지 (ODKU 키 동작 회귀 방지)incrementOrderLineCountAndAmount에orderAmount = 0또는 매우 큰 값(Long.MAX_VALUE근사)을 넣었을 때 컬럼 타입 한계 검증- 음수 누적 이후
incrementLikeCount로 0/양수 복귀 회귀 (이벤트 역전 시나리오)As per coding guidelines: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/java/com/loopers/infrastructure/ProductMetricsDailyRepositoryTest.java` around lines 46 - 135, 테스트가 정상 누적 시나리오만 검증하고 경계·실패 케이스가 없어 요구사항을 충족하지 못하므로, ProductMetricsDailyRepository 관련 테스트들에 경계/예외 케이스를 추가하세요: 다른 PRODUCT_ID에 대해 같은 날짜(TODAY)로 호출해 별도 행이 생성되고 서로 간섭하지 않는지 검증하는 케이스를 추가하고(incrementViewCount / incrementLikeCount 사용), incrementOrderLineCountAndAmount(PRODUCT_ID, TODAY, amount)에 대해 amount=0 및 매우 큰 값(예: Long.MAX_VALUE 근사)으로 호출했을 때 orderAmount/컬럼 타입 한계를 검사하는 테스트를 추가하며, decrementLikeCount로 음수로 만든 후 incrementLikeCount로 0 또는 양수로 복귀하는 흐름(likeAndUnlike 역전 시나리오)을 assert하는 테스트를 각각 구현하세요; 각 테스트는 findMetrics(...) 호출로 결과를 조회하고 getViewCount/getLikeCount/getOrderLineCount/getOrderAmount 검증을 수행하도록 만드세요.apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDaily.java (1)
26-36: 기본값 초기화 방식을 DB 레벨에서도 명시하고, PK 컬럼 순서는 이미 최적화되어 있다.현재 이 엔티티의 모든 쓰기 경로는
INSERT ... ON DUPLICATE KEY UPDATE네이티브 쿼리를 통해 진행되므로,Long래퍼 필드의 기본값(= 0L)은 Java 차원의 초기화일 뿐이며 JPA merge/리플렉션 경로에서의 NPE 위험은 실질적으로 없다. 다만 향후 JPAsave경로가 추가될 가능성에 대비하여,@ColumnDefault("0")어노테이션을 컬럼에 함께 선언하면 DB 레벨에서도 기본값이 보장되어 방어적이다.PK 컬럼 순서(
metric_date,product_id)는 이미 올바르게 설계되어 있다. 배치 작업에서WHERE metric_date BETWEEN ...로 범위 조회할 때metric_date가 클러스터드 인덱스의 선두 컬럼이므로 효율적인 range scan이 가능하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDaily.java` around lines 26 - 36, Add DB-level default values for numeric metric columns in ProductMetricsDaily by annotating each wrapper field (likeCount, viewCount, orderLineCount, orderAmount) with `@ColumnDefault`("0") so the database guarantees 0 even if a JPA save/insert path is used later; update imports to include org.hibernate.annotations.ColumnDefault and leave the existing Long types and current PK ordering (metric_date, product_id) unchanged.modules/jpa/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotJpaRepository.java (1)
14-19: 벌크 DELETE는clearAutomatically = true부여를 권장한다.
@ModifyingJPQL 벌크 DELETE는 1차 캐시(영속성 컨텍스트)를 건너뛴다. 동일 트랜잭션 내에서 이후ProductRankSnapshot을 로드/머지(예:JpaItemWriter가 동일EntityManager를 공유하는 경로)하면 이미 삭제된 엔티티가 스테일 상태로 남아있을 위험이 있다. 배치 잡은beforeStep에서 DELETE → reader/writer에서 INSERT 순이므로 현재 구조상 실질 충돌은 낮지만, 재사용 가능성과 회귀 안전성을 위해clearAutomatically = true(+ 필요 시flushAutomatically = true) 부여를 권장한다. 또한 호출 측에서 트랜잭션을 열고 있는지(배치StepExecutionListener의beforeStep은 기본적으로 스텝 트랜잭션 밖에서 동작) 확인이 필요하다.추가 테스트:
DELETE후 동일 트랜잭션에서(rankingType, rankDate)로 재조회 시 빈 결과가 반환되는지, 그리고 재집계 멱등성(같은 날짜 재실행 시 중복 없이 동일 결과) 검증.♻️ 제안
- `@Modifying` + `@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("DELETE FROM ProductRankSnapshot p WHERE p.rankingType = :rankingType AND p.rankDate = :rankDate") void deleteByRankingTypeAndRankDate( `@Param`("rankingType") RankingType rankingType, `@Param`("rankDate") LocalDate rankDate );
RankingAggregationJobConfig에서beforeStepDELETE가 어떤 트랜잭션 경계 안에서 실행되는지(별도TransactionTemplate사용 여부 포함) 확인 부탁한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/jpa/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotJpaRepository.java` around lines 14 - 19, The JPQL bulk DELETE method deleteByRankingTypeAndRankDate in ProductRankSnapshotJpaRepository should be annotated to avoid stale entities in the persistence context: add clearAutomatically = true (and add flushAutomatically = true if you want immediate flush) to the `@Modifying` annotation so the EntityManager clears the 1st-level cache after the bulk delete; also verify in RankingAggregationJobConfig whether the beforeStep DELETE runs inside a transaction (or uses TransactionTemplate) and add/adjust transaction boundaries as needed and add tests that after delete a subsequent find by (rankingType, rankDate) in the same transaction returns empty and that re-running aggregation is idempotent.apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.java (1)
135-144:orderedProducts비어있는 경우의 이벤트 발행 정책을 재검토 부탁한다.
orderService.getOrderItems(...)가 어떤 이유로 빈 리스트를 반환할 경우(동시성 이슈, 보상 로직과의 경합 등), 빈orderedProducts로PAYMENT_COMPLETED이벤트가 그대로 발행된다. 소비 측(PaymentCompletedEventHandler)은 빈 리스트를 무시하므로 기능상 문제는 없으나, 운영 관점에서는 "결제 성공 이벤트는 발행되었는데 메트릭이 전혀 오르지 않는" 관측 공백이 생긴다. 빈 리스트인 경우 경고 로깅을 남기거나, 방어적으로 발행을 스킵하고 메트릭/알림으로 상위에 신호를 주는 편을 권장한다.또한
OrderedProduct.of(item.productId(), item.price(), item.quantity())가 이벤트 페이로드에 금액/수량을 싣게 되므로, 아웃박스 직렬화·로깅 레벨에서 가격 정보가 과도하게 기록되지 않는지(카프카 토픽 접근 권한, 로깅 마스킹) 운영 레벨 점검을 권장한다.추가 테스트:
orderItems가 빈 리스트일 때 이벤트 발행/로깅 동작 검증 테스트.🤖 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/payment/PaymentFacade.java` around lines 135 - 144, 현재 orderService.getOrderItems(...)가 빈 리스트를 반환하면 orderedProducts가 비어 있는 상태로 outboxEventPublisher.publish(...)가 호출되어 관측 공백이 생깁니다; PaymentFacade에서 orderedProducts가 비어있는지 검사하여 빈 경우에는 outboxEventPublisher.publish을 건너뛰고 대신 processLogger.warn(...) 또는 logger.warn을 남기고(메시지에 payment.getId(), payment.getOrderId() 포함) 모니터링용 카운터/메트릭을 증가시켜 알림을 보내도록 변경하세요(또는 비동기 재시도/재검증 로직을 호출하도록 방어적 처리를 추가). 변경 대상 식별자: orderService.getOrderItems, orderedProducts, outboxEventPublisher.publish, PaymentCompletedEventPayload, PaymentCompletedEventHandler; 또한 orderItems가 빈 리스트일 때 이벤트 발행/로깅 동작을 검증하는 단위 테스트를 추가하세요.apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankSnapshotQueryRepository.java (1)
7-10: 페이지네이션 파라미터 방어와 도메인 계약 명확화를 권장한다.인터페이스 자체는 깔끔하나,
offset/size가 음수거나 과도하게 클 때의 계약이 명시되어 있지 않다. 운영 관점에서는 Facade/구현체 어느 한쪽에서 반드시 상한(예:size <= 100)과 음수 차단을 강제해야 대량 스캔으로 인한 성능 저하를 방지할 수 있다. 또한findLatestRankDate가 반환하는 날짜의 의미(가장 최근 집계 성공일 vs 오늘 포함 여부)를 javadoc으로 명시해두면 WEEKLY/MONTHLY 조회 시 "아직 배치가 돌지 않은 상태"의 처리(예:Optional.empty→ 빈 응답)가 호출부에서 일관되게 이루어진다.추가 테스트:
offset < 0,size = 0,size > topN(=100)입력 시의 동작 계약을 검증하는 테스트.🤖 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/ProductRankSnapshotQueryRepository.java` around lines 7 - 10, Add defensive pagination contract and clarify semantics: update the ProductRankSnapshotQueryRepository interface by documenting (Javadoc) that findRankings(RankingType, LocalDate, int offset, int size) requires offset >= 0, size > 0 and size <= 100 (or configurable TOP_N) and that implementations must validate/normalize inputs (throw IllegalArgumentException or clamp) to prevent large scans; also document findLatestRankDate(RankingType) to specify whether it returns the most recent successful aggregation date (excluding today if batch not run) and when Optional.empty is returned. Then implement the validation in the repository implementation(s) that back findRankings and add unit tests covering offset < 0, size == 0, and size > 100 to assert the defined behavior.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotQueryRepositoryImpl.java (2)
6-6: 사용되지 않는ProductRankSnapshotJpaRepositoryimport가 포함되어 있다.이 파일은
EntityManager만 사용하므로ProductRankSnapshotJpaRepositoryimport는 불필요하다. 정적 분석이 가리키는 항목은 아니지만, PR 병합 전 제거를 권장한다.🤖 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/ProductRankSnapshotQueryRepositoryImpl.java` at line 6, 파일 ProductRankSnapshotQueryRepositoryImpl에 불필요하게 포함된 ProductRankSnapshotJpaRepository import를 제거하세요; 이 클래스는 EntityManager만 사용하므로 ProductRankSnapshotJpaRepository 참조를 삭제(또는 불필요한 임포트 자동 정리 실행)하여 정리합니다.
35-42:SELECT MAX(...)쿼리의 반환 처리 단순화를 권장한다.
SELECT MAX(p.rankDate)는 집계 쿼리이므로 항상 정확히 1개 행(값이 없을 때는 단일null)을 반환한다. 따라서result.isEmpty()분기는 실제로 발생하지 않으며getSingleResult()와Optional.ofNullable(...)조합이 의도를 더 명확히 드러낸다. 기능상 문제는 없어 필수는 아니다.제안 수정안
- List<LocalDate> result = entityManager.createQuery( - "SELECT MAX(p.rankDate) FROM ProductRankSnapshot p " + - "WHERE p.rankingType = :rankingType", LocalDate.class) - .setParameter("rankingType", rankingType) - .getResultList(); - return result.isEmpty() ? Optional.empty() : Optional.ofNullable(result.get(0)); + LocalDate result = entityManager.createQuery( + "SELECT MAX(p.rankDate) FROM ProductRankSnapshot p " + + "WHERE p.rankingType = :rankingType", LocalDate.class) + .setParameter("rankingType", rankingType) + .getSingleResult(); + return Optional.ofNullable(result);🤖 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/ProductRankSnapshotQueryRepositoryImpl.java` around lines 35 - 42, The method findLatestRankDate currently executes entityManager.createQuery(..., LocalDate.class).getResultList() and checks isEmpty(), but since SELECT MAX(p.rankDate) always returns exactly one row (possibly null) simplify by using getSingleResult() on the Query returned by entityManager.createQuery and then wrap that result with Optional.ofNullable(...); update the code in findLatestRankDate to call getSingleResult() instead of getResultList() and return Optional.ofNullable(theSingleResult) so intent is clearer.modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankSnapshot.java (1)
22-31: 스냅샷 조회 패턴 대비 인덱스 커버리지에 대한 운영상 의견을 남긴다.
uk_type_date_product와idx_type_date_rank는 쓰기 중복 방지 및 페이지네이션 조회에 적절하다. 다만 상품 상세에서 최신 주·월간 순위를 역조회하려는 요구가 생기면(product_id, ranking_type, rank_date)보조 인덱스가 필요할 수 있으므로 API 추가 시점에 함께 고려한다. 스키마는 별도 마이그레이션 파이프라인으로 관리되는 점을 전제로 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankSnapshot.java` around lines 22 - 31, 현재 인덱스 구성(UniqueConstraint uk_type_date_product, Index idx_type_date_rank) 는 쓰기 중복 방지와 페이지네이션에 적절하지만, 상품 상세에서 최신 주/월간 순위를 역조회할 경우를 대비해 보조 인덱스가 필요합니다; 마이그레이션 시 product_rank_snapshot 테이블에 (product_id, ranking_type, rank_date) 순서의 보조 인덱스 추가(또는 API 추가 시점에 함께 생성) 방안을 마련하고, 해당 인덱스 이름과 역할을 명시하여 스키마 마이그레이션 스크립트에 반영하세요.apps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.java (1)
38-45: 라인 아이템마다 2회의 DB 왕복이 발생하여 주문 규모가 커질수록 처리 지연이 누적된다.각
OrderedProduct마다productMetricsRepository.incrementOrderLineCount와productMetricsDailyRepository.incrementOrderLineCountAndAmount가 동기 호출되어 N×2회의 라운드트립이 발생한다. 대형 주문/버스트 트래픽 시 컨슈머 지연과 리밸런싱 유발 가능성이 있다. 즉시 수정은 필수 아니나, 향후 배치 upsert(동일 (productId, today) 키에 대해 집계 후 1회 실행)로 모으는 것을 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.java` around lines 38 - 45, Per-order loop causes 2 DB roundtrips per OrderedProduct (productMetricsRepository.incrementOrderLineCount and productMetricsDailyRepository.incrementOrderLineCountAndAmount), so aggregate updates by productId (and today) before persisting: build a map keyed by product.getProductId() that sums quantity and amount (price*quantity), then replace the per-item calls in PaymentCompletedEventHandler with two batched operations—one to increment order line counts in productMetricsRepository with aggregated counts and one to upsert/increment daily metrics in productMetricsDailyRepository using the aggregated (productId, today) counts and amounts—so each unique productId results in a single DB call instead of N×2 calls.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (1)
32-38:rankingType이null일 때의 방어 로직이 없어 NPE가 발생할 수 있다.Controller에서 기본값
"DAILY"를 주고 있어 현재 경로에서는 문제가 없지만, 파사드는 다른 호출자(향후 내부 스케줄러·테스트 등)로부터 재사용될 가능성이 있으므로 방어가 필요하다.rankingType == null인 경우 즉시CoreException으로 실패시키거나DAILY로 폴백하는 것이 바람직하다. 추가 테스트로는rankingType = null전달 시 정의된 예외가 던져지는지를 검증한다.🤖 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 32 - 38, Add a null-check for the rankingType parameter in RankingFacade.getRankings and fail fast: if rankingType == null throw a CoreException with a clear message (e.g., "rankingType must not be null") instead of letting an NPE occur; update callers/tests to assert that passing null to getRankings raises the CoreException and ensure existing flows still call getDailyRankings(date, page, size) for RankingType.DAILY and getSnapshotRankings(rankingType, date, page, size) otherwise.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java (1)
44-52:null반환으로 상태 신호를 전달하는 대신 의도를 명시적으로 드러내는 편이 좋다.
resolveDate가DAILY가 아닐 때null을 반환하고 Facade가 "null이면 최신 rank_date 조회" 규약으로 해석하는 암묵 계약은 유지 비용이 크다. 호출부에서Optional<LocalDate>를 반환하거나, Facade의getSnapshotRankings가LocalDate없는 오버로드를 별도 제공하도록 리팩터링하면 경로 분기가 명확해진다. 현재 단계에서는 필수는 아니다.🤖 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 44 - 52, The resolveDate method currently returns null for non-DAILY ranking types which encodes a hidden contract consumed by getSnapshotRankings; change resolveDate(LocalDate date, RankingType rankingType) to return Optional<LocalDate> (or alternatively add an overload of getSnapshotRankings that takes no date) so callers handle presence/absence explicitly: update callers of resolveDate and the invocation of getSnapshotRankings to branch on Optional.isPresent() (or call the new no-date overload) instead of relying on null; reference symbols: resolveDate, RankingType.DAILY, and getSnapshotRankings.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java (2)
82-89:@Bean메서드에서 step-scope 대상에null을 넘겨 호출하는 패턴은 표준적이지만, 의도를 드러내는 주석이 있으면 좋다.
aggregateRankingStep(null)및deleteOldRankingStepListener(null, null)호출은@ConfigurationCGLIB 프록시가 실제 step-scoped 빈(프록시)을 해석해 주는 Spring Batch의 표준 패턴이다. 기능상 문제는 없으나, 신규 합류자 혼동을 막기 위해 "late-binding: StepScope proxy will resolve job parameters" 주석을 남기면 유지보수성이 향상된다.🤖 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/RankingAggregationJobConfig.java` around lines 82 - 89, Add an explicit clarifying comment where step-scoped beans are invoked with null (e.g., the aggregateRankingStep(null) call inside rankingAggregationJob and the deleteOldRankingStepListener(null, null) usages) stating that passing null is intentional because Spring Batch uses a StepScope proxy which will late-bind and resolve job parameters at runtime (suggested text: "late-binding: StepScope proxy will resolve job parameters at runtime"). Place the comment immediately above or inline with the null-invocation to make the intent clear to new contributors without changing behavior.
45-72: 가중치 동일 합산식을 두 번 기술하여 유지보수 과정에서 ORDER BY와 SELECT의 식이 어긋날 위험이 있다.
score계산식이 SELECT와ROW_NUMBER() OVER(ORDER BY ...)양쪽에 중복되어 있고, 파라미터 바인딩도 6개로 분리되어 있다. 향후 가중치 항목(예:cart_count)을 추가할 때 한쪽만 바뀌면 정렬과 저장 스코어가 불일치해 순위가 틀어진다. 내부 서브쿼리에서score를 컬럼으로 산출한 뒤ROW_NUMBER() OVER (ORDER BY score DESC, product_id ASC)로 참조하도록 재작성하면 중복이 제거되고 파라미터도 3개로 줄어든다.추가 제안:
product_metrics_daily의metric_date인덱스(및 월간 스캔 최적화를 위한(metric_date, product_id)복합 인덱스) 존재 여부를 점검한다. 30일 윈도 스캔이 풀스캔으로 동작하면 배치 시간이 선형 증가한다.🤖 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/RankingAggregationJobConfig.java` around lines 45 - 72, READER_SQL의 SELECT에서 score 식과 ROW_NUMBER() OVER(ORDER BY ...)에 동일한 가중치 합산식을 중복 사용하고 파라미터가 6개로 분리되어 있어 불일치 위험이 있으니, 내부 서브쿼리에서 (예: SELECT ... , (SUM(pm.view_count) * ? + SUM(pm.like_count) * ? + SUM(pm.order_amount) * ?) AS score) 형태로 score 컬럼을 먼저 계산한 뒤 외부에서 ROW_NUMBER() OVER(ORDER BY score DESC, pm.product_id ASC)로 참조하도록 쿼리를 재작성하여 중복식을 제거하고 바인딩 파라미터를 3개로 줄이세요; 또한 쿼리 성능을 위해 product_metrics_daily의 metric_date (가능하면 (metric_date, product_id) 복합 인덱스) 존재 여부를 점검하세요.apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.java (1)
260-409: 스냅샷 API 경계 케이스 커버리지를 한 가지 더 보강한다.WEEKLY/MONTHLY 기본 경로·빈 결과·페이지네이션은 잘 다뤄졌으나, 다음 한 가지 보강을 권장한다:
rankingType파라미터에 잘못된 값(예:HOURLY)을 넘겼을 때의 응답 포맷 검증. 이는RankingV1Controller의 enum 바인딩 실패 시 통합 에러 응답으로 노출되는지 회귀를 막기 위함이다. 또한rankDate가 동일한 두 스냅샷에 대한rankPosition정렬의 결정성을 확인하는 케이스가 있으면 배치·API 간 정렬 계약이 고정된다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.java` around lines 260 - 409, Add two E2E tests inside the GetSnapshotRankings nested class: (1) a negative test that calls RANKING_ENDPOINT with an invalid rankingType value (e.g. "?rankingType=HOURLY&size=10") and asserts the controller returns the standardized error ApiResponse (HTTP 400/Bad Request and the expected error body shape) to verify enum binding failure is surfaced as a consistent error from RankingV1Controller, and (2) a determinism test that saves two ProductRankSnapshot entries with the same rankDate and the same score but different rankPosition using saveSnapshot(...) and asserts the returned list from RANKING_ENDPOINT + "?rankingType=WEEKLY&size=10" is ordered deterministically by rankPosition (e.g. lower rankPosition appears first); use existing helpers (saveSnapshot, saveBrand, saveProduct, RANKING_ENDPOINT, testRestTemplate) and mirror assertions style used in other tests.apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java (1)
177-279: 스냅샷 시나리오 단위 테스트는 적절하나 Mockito 의존성이 과도하다.
snapshotQueryRepository에 대한 when/then stubbing이 사실상 Facade 매핑 로직만 검증하는 구조가 되어 있어, 리포지토리 JPQL의 올바름은 별도 슬라이스 테스트나 E2E에 의존한다. 현재 E2E가 커버하므로 수용 가능하다. 경계 보강 제안: (1)productFacade가 스냅샷 경로에서 호출되지 않음을verifyNoInteractions로 고정, (2)page < 1또는size <= 0에 대한 입력 검증 동작이 Facade·Controller 중 어디에 있는지 계약을 테스트로 고정한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java` around lines 177 - 279, Add explicit interaction checks and input-validation contract tests: in the SnapshotRankings tests add verifyNoInteractions(productFacade) after each arrange/act to ensure productFacade is not called when using snapshotQueryRepository (reference SnapshotRankings and snapshotQueryRepository/productFacade), and add two unit tests that call rankingFacade.getRankings with page < 1 and with size <= 0 asserting the agreed contract (e.g., expect IllegalArgumentException or a specific validation exception); if the facade lacks input validation, implement parameter checks in RankingFacade.getRankings to throw that exception for invalid page/size so the new tests pass.apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java (1)
62-62: 고정END_DATE는 현재 유효하지만, 실제 운영 동작(어제 기준 집계)과의 괴리를 드러낼 테스트가 없다.운영 관점에서 배치는 기본적으로
endDate = yesterday로 돌도록 설계되어 있다고 PR 설명에 기술되어 있으나, 본 테스트는 모든 케이스에서 고정된2026-04-14만 사용한다. 파라미터 기본값 해석(예: 파라미터 누락 시 어제로 치환되는지, 시간대 계산이 서버 타임존에 좌우되지 않는지) 검증이 누락되어, 운영 환경 타임존 이슈가 테스트에서 포착되지 않는다.최소한
endDate파라미터를 생략하고launchJob을 실행했을 때 기본값이LocalDate.now().minusDays(1)(또는 구현 사양)로 치환되어 정상 동작하는지 확인하는 테스트를 한 건 추가할 것을 제안한다. 현재 테스트를 수정하는 것이 아니라 신규 케이스 추가만으로 충분하다.As per coding guidelines (
**/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java` at line 62, Add a new test in RankingAggregationJobE2ETest that verifies the default endDate behavior when the parameter is omitted: call launchJob (the same method used by existing tests) without passing an endDate and assert the job uses LocalDate.now().minusDays(1) (or the implementation's specified default) as the effective end date (compare stored/queried aggregation boundaries or job args), ensuring you account for server timezone by normalizing to LocalDate or injecting a Clock if available; reference END_DATE only for existing fixed-case tests and keep this new test separate to validate the implicit "yesterday" default.
🤖 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/interfaces/api/ranking/RankingV1Controller.java`:
- Line 30: The controller currently binds rankingType as the enum RankingType
which causes Spring to throw MethodArgumentTypeMismatchException (bypassing
ApiControllerAdvice) for invalid values; change the RankingV1Controller endpoint
to accept rankingType as String, validate/convert it to RankingType using domain
rules and throw new CoreException(ErrorType.BAD_REQUEST, ...) on invalid values,
or alternatively update ApiControllerAdvice to map
MethodArgumentTypeMismatchException -> CoreException so all enum binding
failures are normalized; add an E2E test that calls the endpoint with
rankingType=INVALID and asserts a 400 with the unified error body.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java`:
- Around line 148-169: Extract the anonymous StepExecutionListener into a
dedicated `@Component` class named DeleteOldRankingListener that implements
StepExecutionListener; inject ProductRankSnapshotJpaRepository and read job
parameters inside beforeStep (use stepExecution.getJobParameters() to obtain
rankingType and endDate), annotate the beforeStep method with `@Transactional`,
and update the deleteOldRankingStepListener bean to return/lookup this component
(or register it as the listener) so transactions run on a Spring-managed bean;
ensure you still call
productRankSnapshotJpaRepository.deleteByRankingTypeAndRankDate(rankingType,
actualEndDate) from the new beforeStep.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java`:
- Around line 119-138: The teardown currently calls truncateExternalTables →
databaseCleanUp.truncateAllTables → cleanUpBatchMetadata inside `@AfterEach` so a
failure in an earlier step can skip later cleanup (causing
JobInstanceAlreadyCompleteException on subsequent runs) and SET
FOREIGN_KEY_CHECKS may not apply as expected due to session/transaction scope;
modify the `@AfterEach` to ensure each cleanup step runs independently by either
(a) wrapping each of truncateExternalTables, databaseCleanUp.truncateAllTables,
and cleanUpBatchMetadata in their own try/catch (or assertDoesNotThrow) so
failures don’t prevent the next step, or (b) perform a single transactional
block that disables FK checks once at the connection/session level and truncates
all external and batch tables together before re-enabling FK checks; update
truncateExternalTables and cleanUpBatchMetadata accordingly to use the chosen
approach and ensure FK toggling is applied at the correct scope.
- Around line 382-455: Add an assertion to each exclusion test
(excludesDeletedProducts, excludesHiddenProducts,
excludesProductsOfDeletedBrand) that the remaining ProductRankSnapshot has
rankPosition == 1 to verify re-ranking after exclusions; update the asserts
block in each test to check results.stream().anyMatch(r ->
r.getProductId().equals(1L) && r.getRankPosition()==1) (or equivalent) and leave
the existing noneMatch checks untouched—no changes to seeding are required
because the seeds already give the excluded product a higher metric.
- Around line 77-117: The test uses CREATE TABLE IF NOT EXISTS inside
createExternalTables() which can hide schema drift and causes cross-test
flakiness; change test setup to guarantee an exact schema by dropping and
recreating the tables each run (call DROP TABLE IF EXISTS for brands, products,
product_metrics_daily before creating them) or replace createExternalTables()
with a single shared `@Sql` script executed in `@BeforeEach` so the test has a
deterministic schema; ensure the setup includes the expected columns
(visibility, deleted_at, order_line_count) and the product_metrics_daily PRIMARY
KEY order (metric_date, product_id) so the test assertions match the real
schema.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.java`:
- Around line 41-44: PaymentCompletedEventHandler 내부에서 OrderedProduct의
product.getPrice()와 product.getQuantity()가 null일 경우 자동 언박싱 NPE와 잠재적 산술 오버플로우가
발생하므로, product 처리 전에 null 가드를 추가해 price 또는 quantity가 null이면 해당 제품 항목을 스킵하고
productMetricsDailyRepository.incrementOrderLineCountAndAmount 호출을 하지 않도록 변경하고
WARN 로그를 남기세요; 또한 더 안전하게 계산하려면 quantity를 명시적으로 long으로 캐스트하고 orderAmount를 long으로
계산해 전달하도록 수정하고(price: product.getPrice().longValue(), quantity:
product.getQuantity().longValue()), 해당 동작에 대한 단위 테스트를 추가해 null price/quantity를
가진 OrderedProduct가 섞여 있어도 나머지 항목은 정상 처리되는지 검증하세요.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductViewedEventHandler.java`:
- Around line 29-33: The two repository calls in
ProductViewedEventHandler.handle are already covered by the transaction on
EventProcessingService.process, so do not add another transactional boundary;
instead ensure you leave the two calls to
productMetricsRepository.incrementViewCount and
productMetricsDailyRepository.incrementViewCount(metricDate) as-is under the
existing transaction. Replace the use of Clock.systemDefaultZone() with an
injected Clock tied to an explicit timezone (e.g.,
Clock.system(ZoneId.of("Asia/Seoul")) or a configurable bean) so metric_date is
deterministic across JVMs, and add a unit/integration test that injects a fixed
Clock at the day-boundary (e.g., UTC 23:59:59 / KST 00:00:00) to assert correct
metric_date computation for productMetricsDailyRepository.incrementViewCount.
In `@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetrics.java`:
- Line 23: Add an explicit `@Column` declaration to the
ProductMetrics.orderLineCount field to match ProductMetricsDaily (use
`@Column`(name = "order_line_count", nullable = false) on the orderLineCount field
in class ProductMetrics) so the mapping is explicit and consistent; after
changing the entity, coordinate with the external migration pipeline to ensure a
migration/DDL is applied or a backfill from any existing order_count column is
included, confirm migration runs before app deployment, and verify no historical
metric data will be lost by the column rename.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.java`:
- Around line 22-28: The current decrementLikeCount repository method inserts a
new row with like_count = -1 which can persist negative likes; change the
behavior so negative scores cannot be persisted by either: (A) modifying the
batch aggregation to clamp like totals with GREATEST(SUM(like_count), 0) when
writing to product_rank_snapshot, or (B) changing decrementLikeCount (the
repository method) to not INSERT when the row doesn't exist — replace the INSERT
... ON DUPLICATE KEY UPDATE pattern in decrementLikeCount with an UPDATE-only
statement (so no new row is created), or (C) migrate like handling to an
idempotent product_like_daily(user_id, product_id, metric_date) table and
compute deltas during aggregation; after choosing one approach, add a regression
E2E test in RankingAggregationJobE2ETest that ensures an arriving unlike before
a like does not produce a negative score in the final snapshot.
---
Duplicate comments:
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductUnlikedEventHandler.java`:
- Around line 29-33: ProductUnlikedEventHandler.handle currently calls
productMetricsRepository.decrementLikeCount and
productMetricsDailyRepository.decrementLikeCount separately and uses
LocalDate.now(clock) — make the two decrements atomic and timezone-consistent:
wrap both calls in a single transaction (e.g., annotate handle with
`@Transactional` or call a transactional service method) and ensure the same
injected Clock is used to compute LocalDate once at the start (store LocalDate
now = LocalDate.now(clock) and pass it to productMetricsDailyRepository). Also
verify/modify productMetricsDailyRepository.decrementLikeCount to guard against
negative counts (use database-side GREATEST(...,0) or equivalent) or document
the delta semantics and add a unit/integration test (e.g.,
ProductUnlikedEventHandlerTest_unlikeWithoutPriorLike_resultsInZeroDailyCount)
that asserts an unlike on a day with no prior like does not drive
daily/aggregate counts below zero.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.java`:
- Around line 135-144: 현재 orderService.getOrderItems(...)가 빈 리스트를 반환하면
orderedProducts가 비어 있는 상태로 outboxEventPublisher.publish(...)가 호출되어 관측 공백이 생깁니다;
PaymentFacade에서 orderedProducts가 비어있는지 검사하여 빈 경우에는 outboxEventPublisher.publish을
건너뛰고 대신 processLogger.warn(...) 또는 logger.warn을 남기고(메시지에 payment.getId(),
payment.getOrderId() 포함) 모니터링용 카운터/메트릭을 증가시켜 알림을 보내도록 변경하세요(또는 비동기 재시도/재검증 로직을
호출하도록 방어적 처리를 추가). 변경 대상 식별자: orderService.getOrderItems, orderedProducts,
outboxEventPublisher.publish, PaymentCompletedEventPayload,
PaymentCompletedEventHandler; 또한 orderItems가 빈 리스트일 때 이벤트 발행/로깅 동작을 검증하는 단위 테스트를
추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 32-38: Add a null-check for the rankingType parameter in
RankingFacade.getRankings and fail fast: if rankingType == null throw a
CoreException with a clear message (e.g., "rankingType must not be null")
instead of letting an NPE occur; update callers/tests to assert that passing
null to getRankings raises the CoreException and ensure existing flows still
call getDailyRankings(date, page, size) for RankingType.DAILY and
getSnapshotRankings(rankingType, date, page, size) otherwise.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankSnapshotQueryRepository.java`:
- Around line 7-10: Add defensive pagination contract and clarify semantics:
update the ProductRankSnapshotQueryRepository interface by documenting (Javadoc)
that findRankings(RankingType, LocalDate, int offset, int size) requires offset
>= 0, size > 0 and size <= 100 (or configurable TOP_N) and that implementations
must validate/normalize inputs (throw IllegalArgumentException or clamp) to
prevent large scans; also document findLatestRankDate(RankingType) to specify
whether it returns the most recent successful aggregation date (excluding today
if batch not run) and when Optional.empty is returned. Then implement the
validation in the repository implementation(s) that back findRankings and add
unit tests covering offset < 0, size == 0, and size > 100 to assert the defined
behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotQueryRepositoryImpl.java`:
- Line 6: 파일 ProductRankSnapshotQueryRepositoryImpl에 불필요하게 포함된
ProductRankSnapshotJpaRepository import를 제거하세요; 이 클래스는 EntityManager만 사용하므로
ProductRankSnapshotJpaRepository 참조를 삭제(또는 불필요한 임포트 자동 정리 실행)하여 정리합니다.
- Around line 35-42: The method findLatestRankDate currently executes
entityManager.createQuery(..., LocalDate.class).getResultList() and checks
isEmpty(), but since SELECT MAX(p.rankDate) always returns exactly one row
(possibly null) simplify by using getSingleResult() on the Query returned by
entityManager.createQuery and then wrap that result with
Optional.ofNullable(...); update the code in findLatestRankDate to call
getSingleResult() instead of getResultList() and return
Optional.ofNullable(theSingleResult) so intent is clearer.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`:
- Around line 44-52: The resolveDate method currently returns null for non-DAILY
ranking types which encodes a hidden contract consumed by getSnapshotRankings;
change resolveDate(LocalDate date, RankingType rankingType) to return
Optional<LocalDate> (or alternatively add an overload of getSnapshotRankings
that takes no date) so callers handle presence/absence explicitly: update
callers of resolveDate and the invocation of getSnapshotRankings to branch on
Optional.isPresent() (or call the new no-date overload) instead of relying on
null; reference symbols: resolveDate, RankingType.DAILY, and
getSnapshotRankings.
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java`:
- Around line 177-279: Add explicit interaction checks and input-validation
contract tests: in the SnapshotRankings tests add
verifyNoInteractions(productFacade) after each arrange/act to ensure
productFacade is not called when using snapshotQueryRepository (reference
SnapshotRankings and snapshotQueryRepository/productFacade), and add two unit
tests that call rankingFacade.getRankings with page < 1 and with size <= 0
asserting the agreed contract (e.g., expect IllegalArgumentException or a
specific validation exception); if the facade lacks input validation, implement
parameter checks in RankingFacade.getRankings to throw that exception for
invalid page/size so the new tests pass.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.java`:
- Around line 260-409: Add two E2E tests inside the GetSnapshotRankings nested
class: (1) a negative test that calls RANKING_ENDPOINT with an invalid
rankingType value (e.g. "?rankingType=HOURLY&size=10") and asserts the
controller returns the standardized error ApiResponse (HTTP 400/Bad Request and
the expected error body shape) to verify enum binding failure is surfaced as a
consistent error from RankingV1Controller, and (2) a determinism test that saves
two ProductRankSnapshot entries with the same rankDate and the same score but
different rankPosition using saveSnapshot(...) and asserts the returned list
from RANKING_ENDPOINT + "?rankingType=WEEKLY&size=10" is ordered
deterministically by rankPosition (e.g. lower rankPosition appears first); use
existing helpers (saveSnapshot, saveBrand, saveProduct, RANKING_ENDPOINT,
testRestTemplate) and mirror assertions style used in other tests.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java`:
- Around line 82-89: Add an explicit clarifying comment where step-scoped beans
are invoked with null (e.g., the aggregateRankingStep(null) call inside
rankingAggregationJob and the deleteOldRankingStepListener(null, null) usages)
stating that passing null is intentional because Spring Batch uses a StepScope
proxy which will late-bind and resolve job parameters at runtime (suggested
text: "late-binding: StepScope proxy will resolve job parameters at runtime").
Place the comment immediately above or inline with the null-invocation to make
the intent clear to new contributors without changing behavior.
- Around line 45-72: READER_SQL의 SELECT에서 score 식과 ROW_NUMBER() OVER(ORDER BY
...)에 동일한 가중치 합산식을 중복 사용하고 파라미터가 6개로 분리되어 있어 불일치 위험이 있으니, 내부 서브쿼리에서 (예: SELECT
... , (SUM(pm.view_count) * ? + SUM(pm.like_count) * ? + SUM(pm.order_amount) *
?) AS score) 형태로 score 컬럼을 먼저 계산한 뒤 외부에서 ROW_NUMBER() OVER(ORDER BY score DESC,
pm.product_id ASC)로 참조하도록 쿼리를 재작성하여 중복식을 제거하고 바인딩 파라미터를 3개로 줄이세요; 또한 쿼리 성능을 위해
product_metrics_daily의 metric_date (가능하면 (metric_date, product_id) 복합 인덱스) 존재
여부를 점검하세요.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java`:
- Line 62: Add a new test in RankingAggregationJobE2ETest that verifies the
default endDate behavior when the parameter is omitted: call launchJob (the same
method used by existing tests) without passing an endDate and assert the job
uses LocalDate.now().minusDays(1) (or the implementation's specified default) as
the effective end date (compare stored/queried aggregation boundaries or job
args), ensuring you account for server timezone by normalizing to LocalDate or
injecting a Clock if available; reference END_DATE only for existing fixed-case
tests and keep this new test separate to validate the implicit "yesterday"
default.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.java`:
- Around line 38-45: Per-order loop causes 2 DB roundtrips per OrderedProduct
(productMetricsRepository.incrementOrderLineCount and
productMetricsDailyRepository.incrementOrderLineCountAndAmount), so aggregate
updates by productId (and today) before persisting: build a map keyed by
product.getProductId() that sums quantity and amount (price*quantity), then
replace the per-item calls in PaymentCompletedEventHandler with two batched
operations—one to increment order line counts in productMetricsRepository with
aggregated counts and one to upsert/increment daily metrics in
productMetricsDailyRepository using the aggregated (productId, today) counts and
amounts—so each unique productId results in a single DB call instead of N×2
calls.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDaily.java`:
- Around line 26-36: Add DB-level default values for numeric metric columns in
ProductMetricsDaily by annotating each wrapper field (likeCount, viewCount,
orderLineCount, orderAmount) with `@ColumnDefault`("0") so the database guarantees
0 even if a JPA save/insert path is used later; update imports to include
org.hibernate.annotations.ColumnDefault and leave the existing Long types and
current PK ordering (metric_date, product_id) unchanged.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDailyId.java`:
- Line 7: ProductMetricsDailyId implements Serializable but lacks a
serialVersionUID; add a private static final long serialVersionUID field to the
ProductMetricsDailyId class (e.g. private static final long serialVersionUID =
1L;) to ensure stable serialization for this `@IdClass` type and avoid
JVM/compiler-dependent generated values when fields/annotations change.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.java`:
- Around line 14-48: The native upsert methods (incrementLikeCount,
decrementLikeCount, incrementViewCount, incrementOrderLineCountAndAmount) use
`@Modifying` without clearing or flushing the persistence context, which can cause
stale reads if the same transaction later queries these entities; update each
`@Modifying` annotation to include clearAutomatically = true and
flushAutomatically = true so the persistence context is flushed and cleared
after the native query executes, ensuring subsequent reads see the upsert
results.
In
`@apps/commerce-streamer/src/test/java/com/loopers/infrastructure/ProductMetricsDailyRepositoryTest.java`:
- Around line 141-156: Summary: Use long arithmetic for price/quantity to avoid
int overflow in ProductMetricsDailyRepositoryTest. In
ProductMetricsDailyRepositoryTest update the test variables so multiplication is
performed in long (e.g., change unitPrice and/or quantity to long literals or
cast before multiplication) and compute orderAmount as a long from the start
(replace int unitPrice, int quantity, long orderAmount = unitPrice * quantity
with long-aware declarations and calculation) so the test reflects the
production convention and avoids int overflow when calling
incrementOrderLineCountAndAmount(PRODUCT_ID, TODAY, orderAmount).
- Around line 46-135: 테스트가 정상 누적 시나리오만 검증하고 경계·실패 케이스가 없어 요구사항을 충족하지 못하므로,
ProductMetricsDailyRepository 관련 테스트들에 경계/예외 케이스를 추가하세요: 다른 PRODUCT_ID에 대해 같은
날짜(TODAY)로 호출해 별도 행이 생성되고 서로 간섭하지 않는지 검증하는 케이스를 추가하고(incrementViewCount /
incrementLikeCount 사용), incrementOrderLineCountAndAmount(PRODUCT_ID, TODAY,
amount)에 대해 amount=0 및 매우 큰 값(예: Long.MAX_VALUE 근사)으로 호출했을 때 orderAmount/컬럼 타입
한계를 검사하는 테스트를 추가하며, decrementLikeCount로 음수로 만든 후 incrementLikeCount로 0 또는 양수로
복귀하는 흐름(likeAndUnlike 역전 시나리오)을 assert하는 테스트를 각각 구현하세요; 각 테스트는 findMetrics(...)
호출로 결과를 조회하고 getViewCount/getLikeCount/getOrderLineCount/getOrderAmount 검증을
수행하도록 만드세요.
In
`@modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankSnapshot.java`:
- Around line 22-31: 현재 인덱스 구성(UniqueConstraint uk_type_date_product, Index
idx_type_date_rank) 는 쓰기 중복 방지와 페이지네이션에 적절하지만, 상품 상세에서 최신 주/월간 순위를 역조회할 경우를 대비해
보조 인덱스가 필요합니다; 마이그레이션 시 product_rank_snapshot 테이블에 (product_id, ranking_type,
rank_date) 순서의 보조 인덱스 추가(또는 API 추가 시점에 함께 생성) 방안을 마련하고, 해당 인덱스 이름과 역할을 명시하여 스키마
마이그레이션 스크립트에 반영하세요.
In `@modules/jpa/src/main/java/com/loopers/domain/ranking/RankingType.java`:
- Around line 3-14: The enum RankingType uses a switch in getDays(), separating
data from declaration; refactor RankingType to store a final int days field set
via a private constructor on each constant (e.g., DAILY(1), WEEKLY(7),
MONTHLY(30)) and change getDays() to return this.days; update the enum constants
to pass their day values and remove the switch so adding new constants requires
supplying the days at declaration time.
In
`@modules/jpa/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotJpaRepository.java`:
- Around line 14-19: The JPQL bulk DELETE method deleteByRankingTypeAndRankDate
in ProductRankSnapshotJpaRepository should be annotated to avoid stale entities
in the persistence context: add clearAutomatically = true (and add
flushAutomatically = true if you want immediate flush) to the `@Modifying`
annotation so the EntityManager clears the 1st-level cache after the bulk
delete; also verify in RankingAggregationJobConfig whether the beforeStep DELETE
runs inside a transaction (or uses TransactionTemplate) and add/adjust
transaction boundaries as needed and add tests that after delete a subsequent
find by (rankingType, rankDate) in the same transaction returns empty and that
re-running aggregation is idempotent.
🪄 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: d423d32d-a84f-4872-9305-d58d9f0505ac
📒 Files selected for processing (27)
apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/metrics/ProductMetricsReadModel.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/ProductRankSnapshotQueryRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotQueryRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/test/java/com/loopers/application/payment/PaymentFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.javaapps/commerce-batch/src/main/resources/application.ymlapps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.javaapps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.javaapps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductLikedEventHandler.javaapps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductUnlikedEventHandler.javaapps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductViewedEventHandler.javaapps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetrics.javaapps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDaily.javaapps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDailyId.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsRepository.javaapps/commerce-streamer/src/test/java/com/loopers/infrastructure/ProductMetricsDailyRepositoryTest.javamodules/event-contract/src/main/java/com/loopers/event/payload/PaymentCompletedEventPayload.javamodules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankSnapshot.javamodules/jpa/src/main/java/com/loopers/domain/ranking/RankingType.javamodules/jpa/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotJpaRepository.javamodules/jpa/src/main/resources/jpa.yml
💤 Files with no reviewable changes (2)
- apps/commerce-api/src/test/java/com/loopers/application/payment/PaymentFacadeTest.java
- modules/event-contract/src/main/java/com/loopers/event/payload/PaymentCompletedEventPayload.java
| @GetMapping | ||
| public ApiResponse<List<RankingV1Dto.RankingResponse>> getRankings( | ||
| @RequestParam(required = false) @DateTimeFormat(pattern = "yyyyMMdd") LocalDate date, | ||
| @RequestParam(defaultValue = "DAILY") RankingType rankingType, |
There was a problem hiding this comment.
rankingType 입력이 잘못된 경우 표준 에러 포맷을 벗어난다.
enum 바인딩 실패 시 Spring이 MethodArgumentTypeMismatchException을 던지고 ApiControllerAdvice에서 CoreException 기반의 통일된 응답 포맷 대신 기본 500/400 포맷이 반환될 수 있다. 운영 관점에서 클라이언트가 에러 코드를 일관되게 처리하지 못하면 장애 재현·디버깅이 어렵다.
수정안: String으로 받은 뒤 도메인 규칙으로 검증하여 CoreException(ErrorType.BAD_REQUEST, ...)로 변환하거나, @ControllerAdvice에 타입 미스매치 → CoreException 매핑을 추가한다. 추가 테스트: rankingType=INVALID 요청 시 400과 표준 에러 바디가 반환되는지 E2E로 검증한다.
As per coding guidelines, "enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response 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/interfaces/api/ranking/RankingV1Controller.java`
at line 30, The controller currently binds rankingType as the enum RankingType
which causes Spring to throw MethodArgumentTypeMismatchException (bypassing
ApiControllerAdvice) for invalid values; change the RankingV1Controller endpoint
to accept rankingType as String, validate/convert it to RankingType using domain
rules and throw new CoreException(ErrorType.BAD_REQUEST, ...) on invalid values,
or alternatively update ApiControllerAdvice to map
MethodArgumentTypeMismatchException -> CoreException so all enum binding
failures are normalized; add an E2E test that calls the endpoint with
rankingType=INVALID and asserts a 400 with the unified error body.
Comment on lines
+148
to
+169
| @StepScope | ||
| @Bean | ||
| public StepExecutionListener deleteOldRankingStepListener( | ||
| @Value("#{jobParameters['rankingType']}") String rankingTypeStr, | ||
| @Value("#{jobParameters['endDate']}") LocalDate endDate | ||
| ) { | ||
| return new StepExecutionListener() { | ||
| @Override | ||
| @Transactional | ||
| public void beforeStep(StepExecution stepExecution) { | ||
| RankingType rankingType = RankingType.valueOf(rankingTypeStr); | ||
| LocalDate actualEndDate = endDate != null ? endDate : LocalDate.now().minusDays(1); | ||
| log.info("기존 랭킹 데이터 삭제: rankingType={}, rankDate={}", rankingType, actualEndDate); | ||
| productRankSnapshotJpaRepository.deleteByRankingTypeAndRankDate(rankingType, actualEndDate); | ||
| } | ||
|
|
||
| @Override | ||
| public ExitStatus afterStep(StepExecution stepExecution) { | ||
| return stepExecution.getExitStatus(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep --pattern $'@Bean
public StepExecutionListener $_($$$) {
$$$
}'
rg -nP -C3 '@Transactional' apps/commerce-batch/src/main/javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1138
🏁 Script executed:
#!/bin/bash
# Check the structure of batch module and locate application layer
find apps/commerce-batch -type f -name "*.java" | grep -E "(Service|Application)" | head -20
# Check RankingAggregationJobConfig location and surrounding classes
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java | head -50
# Search for `@Transactional` usage in batch module
echo "=== `@Transactional` in batch module ==="
rg -n "@Transactional" apps/commerce-batch/src --type java
# Check if there's an application layer service for ranking in batch
echo "=== Looking for ranking-related services ==="
rg -n "Ranking.*Service\|RankingApplication" apps/commerce-batch/src --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3184
🏁 Script executed:
#!/bin/bash
# Check directory structure of commerce-batch
echo "=== Commerce-batch module structure ==="
find apps/commerce-batch/src/main/java -type d | sort
# Check if there's any application layer in batch
echo "=== Looking for application layer in batch ==="
ls -la apps/commerce-batch/src/main/java/com/loopers/batch/ 2>/dev/null || echo "Directory listing..."
find apps/commerce-batch/src/main/java -name "*Application.java" -o -name "*ApplicationService.java" -o -name "*Service.java" | head -10
# Check module definition and architecture
echo "=== Check commerce-batch pom.xml ==="
cat apps/commerce-batch/pom.xml | grep -A5 -B5 "artifactId\|name" | head -20
# Look at other batch job configurations to compare patterns
echo "=== Other batch job configurations ==="
find apps/commerce-batch/src/main/java -name "*JobConfig.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1273
🏁 Script executed:
#!/bin/bash
# Check DemoJobConfig for comparison
echo "=== DemoJobConfig content ==="
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/DemoJobConfig.java | head -100
# Check if DemoJobConfig uses `@Transactional`
rg -n "@Transactional" apps/commerce-batch/src/main/java/com/loopers/batch/job/demo/ --type java
# Look for batch test files to understand the transaction testing
echo "=== Batch tests related to ranking ==="
find apps/commerce-batch/src/test -name "*Ranking*" -type f
# Check infrastructure layer references to see how transactions are typically handled
echo "=== ProductRankSnapshotJpaRepository location ==="
find . -name "ProductRankSnapshotJpaRepository.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2739
🏁 Script executed:
#!/bin/bash
# Check the E2E test for ranking
echo "=== RankingAggregationJobE2ETest ==="
cat -n apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java | head -150
# Check ProductRankSnapshotJpaRepository
echo "=== ProductRankSnapshotJpaRepository ==="
cat -n modules/jpa/src/main/java/com/loopers/infrastructure/ranking/ProductRankSnapshotJpaRepository.java
# Check if `@Modifying` is present
rg -n "@Modifying\|deleteByRankingTypeAndRankDate" modules/jpa/src/main/java/com/loopers/infrastructure/ranking/ --type java -A2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 8276
🏁 Script executed:
#!/bin/bash
# Get the rest of the test file
echo "=== Full RankingAggregationJobE2ETest ==="
cat apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java
# Search for other StepExecutionListener implementations
echo -e "\n=== StepExecutionListener implementations ==="
rg -n "implements StepExecutionListener\|extends.*StepExecutionListener" --type java
# Check batch listener classes
echo -e "\n=== Batch listener files ==="
ls -la apps/commerce-batch/src/main/java/com/loopers/batch/listener/
cat apps/commerce-batch/src/main/java/com/loopers/batch/listener/*.java | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 36378
🏁 Script executed:
#!/bin/bash
# Search for tests that verify transaction rollback or exception handling
echo "=== Tests with 'rollback', 'exception', 'fail' keywords in ranking ==="
grep -rn "rollback\|exception\|fail\|Exception\|@Test.*[Ee]xception" apps/commerce-batch/src/test/java/com/loopers/job/ranking/ --include="*.java"
# Check if there are any tests that verify deletion failure scenarios
echo -e "\n=== Search for step failure or retry tests ==="
grep -rn "StepExecution\|beforeStep\|afterStep\|failure\|error" apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java | head -20
# Look for Transactional test configuration
echo -e "\n=== Search for `@Transactional` in batch tests ==="
rg -n "@Transactional" apps/commerce-batch/src/test --type java
# Check Spring version to understand AOP behavior
echo -e "\n=== Check Spring Boot version ==="
cd apps/commerce-batch && cat pom.xml | grep -A2 -B2 "spring-boot\|spring.version" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1801
🏁 Script executed:
#!/bin/bash
# Find pom.xml files
echo "=== Finding pom.xml files ==="
find . -name "pom.xml" -type f | head -5
# Check root pom for Spring version
echo -e "\n=== Root pom version ==="
cat pom.xml | grep -A5 "spring-boot-starter-parent" | head -10
# Check the actual RankingAggregationJobConfig more thoroughly
echo -e "\n=== Full RankingAggregationJobConfig (critical parts) ==="
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java | sed -n '140,180p'
# Search for any error handling around the delete operation
echo -e "\n=== Error handling context around deletion ==="
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/RankingAggregationJobConfig.java | sed -n '145,175p'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3639
익명 내부 클래스의 @Transactional은 작동하지만, 별도 클래스로 추출하여 운영 안정성을 높여야 한다.
현재 코드에서 StepExecutionListener를 익명 클래스로 반환하며 beforeStep에 @Transactional을 선언했다. 테스트("reAggregation_replacesExistingSnapshots_noDuplicates")가 통과하는 것으로 보아 트랜잭션이 실제로 적용되고 있으나, Spring의 공식 문서에서는 익명 내부 클래스 메서드에 @Transactional을 적용하는 것을 권장하지 않는다. 또한 현재 테스트는 정상 흐름만 검증하고 예외 발생 시 롤백 동작을 확인하지 않는다.
운영상 우려: deleteByRankingTypeAndRankDate 실행 중 예외가 발생하면 트랜잭션 롤백 여부가 불명확해지며, 부분 삭제 후 재시도 시 기존 레코드와의 UNIQUE 제약 조건 충돌이 재발할 수 있다.
수정안: @Component로 선언된 별도의 클래스(DeleteOldRankingListener)로 추출하고, 명시적으로 @Transactional을 선언한다. 추가 테스트를 작성하여 beforeStep 실행 중 런타임 예외 발생 시 롤백이 정상 동작하고, Step 재시도 시 중복 없이 재집계되는지 검증한다.
🤖 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/RankingAggregationJobConfig.java`
around lines 148 - 169, Extract the anonymous StepExecutionListener into a
dedicated `@Component` class named DeleteOldRankingListener that implements
StepExecutionListener; inject ProductRankSnapshotJpaRepository and read job
parameters inside beforeStep (use stepExecution.getJobParameters() to obtain
rankingType and endDate), annotate the beforeStep method with `@Transactional`,
and update the deleteOldRankingStepListener bean to return/lookup this component
(or register it as the listener) so transactions run on a Spring-managed bean;
ensure you still call
productRankSnapshotJpaRepository.deleteByRankingTypeAndRankDate(rankingType,
actualEndDate) from the new beforeStep.
Comment on lines
+77
to
+117
| private void createExternalTables() { | ||
| transactionTemplate.executeWithoutResult(status -> { | ||
| entityManager.createNativeQuery(""" | ||
| CREATE TABLE IF NOT EXISTS brands ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| name VARCHAR(255) NOT NULL, | ||
| description VARCHAR(255), | ||
| created_at DATETIME(6) NOT NULL, | ||
| updated_at DATETIME(6) NOT NULL, | ||
| deleted_at DATETIME(6) | ||
| ) | ||
| """).executeUpdate(); | ||
|
|
||
| entityManager.createNativeQuery(""" | ||
| CREATE TABLE IF NOT EXISTS products ( | ||
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| brand_id BIGINT NOT NULL, | ||
| name VARCHAR(255) NOT NULL, | ||
| description VARCHAR(255), | ||
| price INT NOT NULL, | ||
| stock_quantity INT NOT NULL DEFAULT 0, | ||
| visibility VARCHAR(20) NOT NULL, | ||
| created_at DATETIME(6) NOT NULL, | ||
| updated_at DATETIME(6) NOT NULL, | ||
| deleted_at DATETIME(6) | ||
| ) | ||
| """).executeUpdate(); | ||
|
|
||
| entityManager.createNativeQuery(""" | ||
| CREATE TABLE IF NOT EXISTS product_metrics_daily ( | ||
| product_id BIGINT NOT NULL, | ||
| metric_date DATE NOT NULL, | ||
| view_count BIGINT NOT NULL DEFAULT 0, | ||
| like_count BIGINT NOT NULL DEFAULT 0, | ||
| order_line_count BIGINT NOT NULL DEFAULT 0, | ||
| order_amount BIGINT NOT NULL DEFAULT 0, | ||
| PRIMARY KEY (metric_date, product_id) | ||
| ) | ||
| """).executeUpdate(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
CREATE TABLE IF NOT EXISTS는 스키마 드리프트를 은폐할 수 있다.
운영 관점에서, 다른 모듈(예: commerce-api)의 JPA 엔티티 또는 마이그레이션 스크립트가 먼저 brands/products/product_metrics_daily를 생성해둔 상태라면, 이 테스트의 IF NOT EXISTS는 조용히 스킵된다. 그 결과 본 테스트가 가정하는 컬럼 집합(예: visibility, deleted_at, order_line_count)이 실제 운영 스키마와 어긋나더라도 테스트가 통과해 장애를 뒤늦게 포착하게 된다. 또한 테스트 클래스가 테이블을 생성만 하고 DROP하지 않으므로, 다른 테스트 클래스가 다른 스키마 정의로 같은 이름의 테이블을 쓰는 경우 실행 순서에 따른 플래키 문제도 발생할 수 있다.
운영상 안전을 위해 (1) @BeforeEach에서 매번 DROP TABLE IF EXISTS 후 CREATE TABLE로 다시 만들거나, (2) 아예 공용 테스트 SQL 스크립트(@Sql)를 단일 진실 공급원으로 두어 모든 배치/스트리머/ API 테스트가 같은 스키마를 공유하도록 정리할 것을 제안한다. 추가 테스트로는 product_metrics_daily의 PK 순서(metric_date, product_id)가 실제 리더 쿼리 기준과 일치하는지 명시적으로 확인하는 케이스(예: 동일 일자의 대량 데이터 삽입 후 기대 정렬 확인)가 있으면 좋겠다.
♻️ 수정 예시
private void createExternalTables() {
transactionTemplate.executeWithoutResult(status -> {
+ entityManager.createNativeQuery("DROP TABLE IF EXISTS product_metrics_daily").executeUpdate();
+ entityManager.createNativeQuery("DROP TABLE IF EXISTS products").executeUpdate();
+ entityManager.createNativeQuery("DROP TABLE IF EXISTS brands").executeUpdate();
entityManager.createNativeQuery("""
- CREATE TABLE IF NOT EXISTS brands (
+ CREATE TABLE brands (As per coding guidelines (**/*Test*.java: "통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java`
around lines 77 - 117, The test uses CREATE TABLE IF NOT EXISTS inside
createExternalTables() which can hide schema drift and causes cross-test
flakiness; change test setup to guarantee an exact schema by dropping and
recreating the tables each run (call DROP TABLE IF EXISTS for brands, products,
product_metrics_daily before creating them) or replace createExternalTables()
with a single shared `@Sql` script executed in `@BeforeEach` so the test has a
deterministic schema; ensure the setup includes the expected columns
(visibility, deleted_at, order_line_count) and the product_metrics_daily PRIMARY
KEY order (metric_date, product_id) so the test assertions match the real
schema.
Comment on lines
+119
to
+138
| private void truncateExternalTables() { | ||
| transactionTemplate.executeWithoutResult(status -> { | ||
| entityManager.createNativeQuery("SET FOREIGN_KEY_CHECKS = 0").executeUpdate(); | ||
| entityManager.createNativeQuery("TRUNCATE TABLE product_metrics_daily").executeUpdate(); | ||
| entityManager.createNativeQuery("TRUNCATE TABLE products").executeUpdate(); | ||
| entityManager.createNativeQuery("TRUNCATE TABLE brands").executeUpdate(); | ||
| entityManager.createNativeQuery("SET FOREIGN_KEY_CHECKS = 1").executeUpdate(); | ||
| }); | ||
| } | ||
|
|
||
| private void cleanUpBatchMetadata() { | ||
| transactionTemplate.executeWithoutResult(status -> { | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_STEP_EXECUTION_CONTEXT").executeUpdate(); | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_STEP_EXECUTION").executeUpdate(); | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_JOB_EXECUTION_CONTEXT").executeUpdate(); | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_JOB_EXECUTION_PARAMS").executeUpdate(); | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_JOB_EXECUTION").executeUpdate(); | ||
| entityManager.createNativeQuery("DELETE FROM BATCH_JOB_INSTANCE").executeUpdate(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
외부 테이블 → 배치 메타데이터 순서의 정리 로직에 누락 케이스가 존재한다.
운영 관점에서 @AfterEach는 성공 경로에서만 실행되며, setUp()에서 테이블 생성이 실패하거나 launchJob 도중 예외가 전파되어도 동일하게 호출되지만, 현재 순서(truncateExternalTables → databaseCleanUp.truncateAllTables → cleanUpBatchMetadata)는 앞 단계가 실패하면 뒤 단계가 실행되지 않는다. 특히 cleanUpBatchMetadata가 누락되면 다음 테스트가 동일 JobParameters로 재실행될 때 JobInstanceAlreadyCompleteException으로 폭주한다. 또한 SET FOREIGN_KEY_CHECKS는 세션 스코프이므로 트랜잭션 범위에 따라 효과가 예상과 다를 수 있다.
운영 안정성을 위해 각 단계를 독립적으로 try/catch 또는 assertDoesNotThrow로 분리하거나, @AfterEach를 단일 트랜잭션 안에서 FK 체크 해제 후 모든 테이블을 한 번에 TRUNCATE 하도록 묶는 것을 제안한다. 추가 테스트로는 테스트가 의도적으로 실패했을 때 다음 테스트가 영향 없이 동작하는지 확인하는 격리 회귀 테스트(파라미터화된 실패 시나리오)를 추가할 것을 권장한다.
♻️ 수정 예시
`@AfterEach`
void tearDown() {
- truncateExternalTables();
- databaseCleanUp.truncateAllTables();
- cleanUpBatchMetadata();
+ // 앞 단계 실패가 뒷 단계(특히 BATCH_* 초기화)를 막지 않도록 독립 실행한다.
+ try { truncateExternalTables(); } catch (Exception ignored) {}
+ try { databaseCleanUp.truncateAllTables(); } catch (Exception ignored) {}
+ cleanUpBatchMetadata();
}As per coding guidelines (**/*Test*.java: "테스트 데이터 준비/정리 전략을 점검한다").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java`
around lines 119 - 138, The teardown currently calls truncateExternalTables →
databaseCleanUp.truncateAllTables → cleanUpBatchMetadata inside `@AfterEach` so a
failure in an earlier step can skip later cleanup (causing
JobInstanceAlreadyCompleteException on subsequent runs) and SET
FOREIGN_KEY_CHECKS may not apply as expected due to session/transaction scope;
modify the `@AfterEach` to ensure each cleanup step runs independently by either
(a) wrapping each of truncateExternalTables, databaseCleanUp.truncateAllTables,
and cleanUpBatchMetadata in their own try/catch (or assertDoesNotThrow) so
failures don’t prevent the next step, or (b) perform a single transactional
block that disables FK checks once at the connection/session level and truncates
all external and batch tables together before re-enabling FK checks; update
truncateExternalTables and cleanUpBatchMetadata accordingly to use the chosen
approach and ensure FK toggling is applied at the correct scope.
Comment on lines
+382
to
+455
| @DisplayName("삭제된 상품은 랭킹에서 제외된다") | ||
| @Test | ||
| void excludesDeletedProducts() throws Exception { | ||
| // arrange | ||
| seedTestDataWithDeletedProduct(); | ||
|
|
||
| // act | ||
| var jobExecution = jobLauncherTestUtils.launchJob( | ||
| new JobParametersBuilder() | ||
| .addString("rankingType", "WEEKLY") | ||
| .addLocalDate("endDate", END_DATE) | ||
| .toJobParameters() | ||
| ); | ||
|
|
||
| // assert | ||
| assertThat(jobExecution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()); | ||
|
|
||
| List<ProductRankSnapshot> results = productRankSnapshotJpaRepository.findAll(); | ||
| assertAll( | ||
| () -> assertThat(results).hasSize(1), | ||
| () -> assertThat(results).anyMatch(r -> r.getProductId().equals(1L)), | ||
| () -> assertThat(results).noneMatch(r -> r.getProductId().equals(99L)) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("비노출(HIDDEN) 상품은 랭킹에서 제외된다") | ||
| @Test | ||
| void excludesHiddenProducts() throws Exception { | ||
| // arrange | ||
| seedTestDataWithHiddenProduct(); | ||
|
|
||
| // act | ||
| var jobExecution = jobLauncherTestUtils.launchJob( | ||
| new JobParametersBuilder() | ||
| .addString("rankingType", "WEEKLY") | ||
| .addLocalDate("endDate", END_DATE) | ||
| .toJobParameters() | ||
| ); | ||
|
|
||
| // assert | ||
| assertThat(jobExecution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()); | ||
|
|
||
| List<ProductRankSnapshot> results = productRankSnapshotJpaRepository.findAll(); | ||
| assertAll( | ||
| () -> assertThat(results).hasSize(1), | ||
| () -> assertThat(results).anyMatch(r -> r.getProductId().equals(1L)), | ||
| () -> assertThat(results).noneMatch(r -> r.getProductId().equals(98L)) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("삭제된 브랜드의 상품은 랭킹에서 제외된다") | ||
| @Test | ||
| void excludesProductsOfDeletedBrand() throws Exception { | ||
| // arrange | ||
| seedTestDataWithDeletedBrand(); | ||
|
|
||
| // act | ||
| var jobExecution = jobLauncherTestUtils.launchJob( | ||
| new JobParametersBuilder() | ||
| .addString("rankingType", "WEEKLY") | ||
| .addLocalDate("endDate", END_DATE) | ||
| .toJobParameters() | ||
| ); | ||
|
|
||
| // assert | ||
| assertThat(jobExecution.getExitStatus().getExitCode()).isEqualTo(ExitStatus.COMPLETED.getExitCode()); | ||
|
|
||
| List<ProductRankSnapshot> results = productRankSnapshotJpaRepository.findAll(); | ||
| assertAll( | ||
| () -> assertThat(results).hasSize(1), | ||
| () -> assertThat(results).anyMatch(r -> r.getProductId().equals(1L)), | ||
| () -> assertThat(results).noneMatch(r -> r.getProductId().equals(97L)) | ||
| ); | ||
| } |
There was a problem hiding this comment.
제외 케이스 검증에서 rankPosition 재정렬 여부를 함께 확인하는 것이 좋다.
운영 관점에서 삭제/비노출/삭제된 브랜드 상품이 단순히 집계에서 빠지는지만 검증하면, 리더 쿼리의 ROW_NUMBER()가 이 제외 조건과 같은 WHERE에서 실행되지 않는 잠재 버그(예: 서브쿼리 단계에서 필터가 누락되어 rankPosition이 2부터 시작하는 케이스)를 놓칠 수 있다. 현재 단정은 hasSize(1)과 anyMatch/noneMatch만 있어, 남은 1건의 rankPosition이 1인지 확인하지 않는다.
세 테스트(excludesDeletedProducts, excludesHiddenProducts, excludesProductsOfDeletedBrand)에 rankPosition == 1 단정을 추가하고, 제외 대상 상품에 더 높은 메트릭을 심어 두어 “제외되지 않았다면 1등이었을 상품”이 실제로 1등 자리에서 배제되는지까지 검증할 것을 권장한다. 현재 시드가 이미 제외 상품에 더 높은 값을 넣고 있으므로 단정만 추가하면 충분하다.
♻️ 수정 예시 (excludesDeletedProducts 기준, 나머지 두 테스트에도 동일 적용)
List<ProductRankSnapshot> results = productRankSnapshotJpaRepository.findAll();
assertAll(
() -> assertThat(results).hasSize(1),
() -> assertThat(results).anyMatch(r -> r.getProductId().equals(1L)),
- () -> assertThat(results).noneMatch(r -> r.getProductId().equals(99L))
+ () -> assertThat(results).noneMatch(r -> r.getProductId().equals(99L)),
+ () -> assertThat(results.get(0).getRankPosition()).isEqualTo(1)
);As per coding guidelines (**/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/RankingAggregationJobE2ETest.java`
around lines 382 - 455, Add an assertion to each exclusion test
(excludesDeletedProducts, excludesHiddenProducts,
excludesProductsOfDeletedBrand) that the remaining ProductRankSnapshot has
rankPosition == 1 to verify re-ranking after exclusions; update the asserts
block in each test to check results.stream().anyMatch(r ->
r.getProductId().equals(1L) && r.getRankPosition()==1) (or equivalent) and leave
the existing noneMatch checks untouched—no changes to seeding are required
because the seeds already give the excluded product a higher metric.
Comment on lines
+41
to
+44
| long orderAmount = product.getPrice() * product.getQuantity(); | ||
| productMetricsDailyRepository.incrementOrderLineCountAndAmount( | ||
| product.getProductId(), today, orderAmount | ||
| ); |
There was a problem hiding this comment.
price/quantity null 입력 시 NPE 및 int 오버플로 위험이 존재한다.
OrderedProduct.price(Long)와 quantity(Integer)는 박싱 타입이므로 업스트림에서 null로 전달될 경우 자동 언박싱 NPE가 발생하여 이벤트 처리 파이프라인이 멈출 수 있다. 또한 getQuantity()가 Integer지만 곱셈 전 long으로 승격되므로 현재는 안전하나, 향후 price가 int로 변경되면 int * int 오버플로가 숨겨진다. 운영 관점에서는 결제 스트리머가 한 건의 잘못된 페이로드로 재시도 루프에 빠지는 것이 치명적이다.
수정안: 필수 필드 null 가드를 추가하고 명시적으로 long으로 캐스팅하며, 누락된 페이로드에 대한 WARN 로그를 남긴다. 테스트로는 price/quantity가 null인 OrderedProduct가 섞여도 나머지 항목이 정상 처리되는지(또는 전체가 skip되는지 정책에 맞춰) 검증하는 케이스를 추가한다.
제안 수정안
- long orderAmount = product.getPrice() * product.getQuantity();
+ if (product.getPrice() == null || product.getQuantity() == null) {
+ log.warn("결제 완료 이벤트 페이로드에 price/quantity 누락: productId={}", product.getProductId());
+ continue;
+ }
+ long orderAmount = product.getPrice() * (long) product.getQuantity();📝 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
| long orderAmount = product.getPrice() * product.getQuantity(); | |
| productMetricsDailyRepository.incrementOrderLineCountAndAmount( | |
| product.getProductId(), today, orderAmount | |
| ); | |
| if (product.getPrice() == null || product.getQuantity() == null) { | |
| log.warn("결제 완료 이벤트 페이로드에 price/quantity 누락: productId={}", product.getProductId()); | |
| continue; | |
| } | |
| long orderAmount = product.getPrice() * (long) product.getQuantity(); | |
| productMetricsDailyRepository.incrementOrderLineCountAndAmount( | |
| product.getProductId(), today, orderAmount | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/PaymentCompletedEventHandler.java`
around lines 41 - 44, PaymentCompletedEventHandler 내부에서 OrderedProduct의
product.getPrice()와 product.getQuantity()가 null일 경우 자동 언박싱 NPE와 잠재적 산술 오버플로우가
발생하므로, product 처리 전에 null 가드를 추가해 price 또는 quantity가 null이면 해당 제품 항목을 스킵하고
productMetricsDailyRepository.incrementOrderLineCountAndAmount 호출을 하지 않도록 변경하고
WARN 로그를 남기세요; 또한 더 안전하게 계산하려면 quantity를 명시적으로 long으로 캐스트하고 orderAmount를 long으로
계산해 전달하도록 수정하고(price: product.getPrice().longValue(), quantity:
product.getQuantity().longValue()), 해당 동작에 대한 단위 테스트를 추가해 null price/quantity를
가진 OrderedProduct가 섞여 있어도 나머지 항목은 정상 처리되는지 검증하세요.
Comment on lines
29
to
33
| public void handle(Event<ProductViewedEventPayload> event) { | ||
| productMetricsRepository.incrementViewCount(event.getPayload().getProductId()); | ||
| Long productId = event.getPayload().getProductId(); | ||
| productMetricsRepository.incrementViewCount(productId); | ||
| productMetricsDailyRepository.incrementViewCount(productId, LocalDate.now(clock)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. ProductViewedEventHandler 파일 확인
fd -t f ProductViewedEventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 184
🏁 Script executed:
# 2. TimeConfig 파일 찾기
fd -t f -i TimeConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 223
🏁 Script executed:
# 3. EventHandler 인터페이스 및 디스패처 찾기
fd -t f -i "EventHandler*.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1102
🏁 Script executed:
# 4. 저장소 구현 찾기
fd -t f -i "*MetricsRepository*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 388
🏁 Script executed:
# 저장소 찾기 (올바른 glob 패턴)
fd -t f --glob "*MetricsRepository*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 178
🏁 Script executed:
# ProductViewedEventHandler 전체 내용
cat -n apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductViewedEventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1591
🏁 Script executed:
# TimeConfig 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/config/TimeConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 486
🏁 Script executed:
# EventHandler 인터페이스 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/application/EventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 393
🏁 Script executed:
# ProductMetricsRepository 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2180
🏁 Script executed:
# ProductMetricsDailyRepository 찾기 및 확인
fd -t f --glob "*ProductMetricsDailyRepository*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 286
🏁 Script executed:
# EventHandler 디스패처 찾기
rg -t java "EventHandler" --glob "*Dispatcher*" -lRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 233
🏁 Script executed:
# EventHandler 호출처 찾기 (일반적인 패턴)
rg -t java "\.handle\(" apps/commerce-streamer/src --glob "*[Cc]onsumer*" --glob "*[Dd]ispatcher*" -lRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# ProductMetricsDailyRepository 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2833
🏁 Script executed:
# EventHandler 호출 지점 더 넓게 검색
rg -t java "\.handle\(" apps/commerce-streamer/src/main --with-filename | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 212
🏁 Script executed:
# ProductViewedEventHandler 테스트 파일 찾기
fd -t f --glob "*ProductViewedEventHandler*Test*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 전체 handler 구현 패턴 확인 (다른 handler들이 `@Transactional을` 사용하는지)
rg -t java "@Transactional" apps/commerce-streamer/src/main/java/com/loopers/application/handler/ -A 5 | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# EventProcessingService 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/application/EventProcessingService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1629
🏁 Script executed:
# 다른 핸들러들도 `@Transactional이` 없는지 확인
rg -t java "public void handle" apps/commerce-streamer/src/main/java/com/loopers/application/handler/ -B 5 | grep -E "(class|@Transactional|public void handle)" | head -30Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1621
두 저장소 호출이 이미 트랜잭션 내에서 실행된다.
EventProcessingService.process() 메서드가 @Transactional로 선언되어 있고(라인 23), 이 메서드 내에서 handler.handle(event)가 호출되므로(라인 35), ProductViewedEventHandler.handle()의 두 저장소 호출은 이미 동일한 트랜잭션 경계 내에서 실행된다. 따라서 incrementViewCount()와 incrementViewCount(metricDate) 중 하나가 실패하면 둘 다 롤백되며, 트랜잭션 경계를 별도로 추가할 필요는 없다.
다만 Clock.systemDefaultZone()을 사용함에 따라 스트리머 JVM의 로컬 타임존에 일별 버킷이 종속되는 점은 여전히 운영 리스크다. 일일 스냅샷이 metric_date에 의존하고 배치 작업의 타임존이 다르면 일별 집계가 의도한 경계와 어긋날 수 있다. Clock.system(ZoneId.of("Asia/Seoul"))로 명시적 존을 고정하거나, 운영 문서에 타임존을 명시할 것을 권장한다.
추가 테스트:
- 고정
Clock(예: UTC 23:59:59 / KST 00:00:00)으로 day-boundary 이벤트가 의도한metric_date로 집계되는지 검증하는 테스트.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/handler/ProductViewedEventHandler.java`
around lines 29 - 33, The two repository calls in
ProductViewedEventHandler.handle are already covered by the transaction on
EventProcessingService.process, so do not add another transactional boundary;
instead ensure you leave the two calls to
productMetricsRepository.incrementViewCount and
productMetricsDailyRepository.incrementViewCount(metricDate) as-is under the
existing transaction. Replace the use of Clock.systemDefaultZone() with an
injected Clock tied to an explicit timezone (e.g.,
Clock.system(ZoneId.of("Asia/Seoul")) or a configurable bean) so metric_date is
deterministic across JVMs, and add a unit/integration test that injects a fixed
Clock at the day-boundary (e.g., UTC 23:59:59 / KST 00:00:00) to assert correct
metric_date computation for productMetricsDailyRepository.incrementViewCount.
|
|
||
| @Column(nullable = false) | ||
| private Long orderCount = 0L; | ||
| private Long orderLineCount = 0L; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 리포지토리/코드 내 잔존 order_count 사용처 확인
rg -nP -C2 'order_count|orderCount' --type=java --type=sql --type=yaml --type=yml
# 마이그레이션성 SQL 파일 잔존 여부
fd -e sqlRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 112
🏁 Script executed:
# ProductMetrics.java 파일 찾기
fd -i 'ProductMetrics.java'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 160
🏁 Script executed:
# 마이그레이션 관련 디렉토리 및 파일 탐색
find . -type d -iname '*migration*' -o -type d -iname '*db*' | head -20
fd -e sql -e ddl | head -30Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# order_count / orderCount 검색 (더 정확한 패턴)
rg 'order_count|orderCount' --type java -n
rg 'order_count|orderCount' --type sql -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 233
🏁 Script executed:
# ProductMetrics와 관련된 모든 파일 검색
fd ProductMetricsRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 708
🏁 Script executed:
# ProductMetrics.java 내용 읽기
cat -n apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetrics.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 802
🏁 Script executed:
# ProductMetricsReadModel.java 내용 읽기
cat -n apps/commerce-api/src/main/java/com/loopers/domain/metrics/ProductMetricsReadModel.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 882
🏁 Script executed:
# order_count 또는 orderCount 검색 (type 필터 제거)
rg 'order_count|orderCount' -n -C2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 마이그레이션 관련 파일 검색 - 구체적 경로들 확인
find . -path "*/migration*" -o -path "*/sql*" -o -path "*/ddl*" 2>/dev/null | grep -E "\.(sql|ddl|yaml|yml)$"
ls -la apps/commerce-streamer/src/main/resources/ 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 260
🏁 Script executed:
# ProductMetricsDailyRepository 등 다른 관련 파일에서 order 관련 컬럼 사용 여부 확인
cat apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsRepository.java
cat apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetricsDaily.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2839
🏁 Script executed:
# 전체 리포지토리에서 "product_metrics" 테이블 참조 확인
rg 'product_metrics|ProductMetrics' -n | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 9001
🏁 Script executed:
# git 이력으로 이전 필드명 확인
git log -p --all -S 'orderCount' -- '*.java' | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
ProductMetrics 필드의 @Column 선언을 명시적으로 지정하고 외부 마이그레이션 확인이 필요하다
orderLineCount 필드에 @Column(name = "order_line_count")를 명시적으로 선언해야 한다. ProductMetricsDaily에서는 이미 명시적으로 선언되어 있지만, ProductMetrics에서는 @Column(nullable = false) 만 사용 중으로 일관성이 없다. Spring의 기본 naming strategy가 orderLineCount를 order_line_count로 변환하므로 현재는 의도한 대로 동작하지만, 스키마 관리의 명확성을 위해 두 클래스 모두 동일하게 명시해야 한다.
// ProductMetrics 수정 필요
`@Column`(name = "order_line_count", nullable = false)
private Long orderLineCount = 0L;더불어 이 프로젝트가 외부 마이그레이션 파이프라인으로 스키마를 관리하므로, 다음 사항을 배포 전 확인하라:
- 기존
order_count컬럼에서order_line_count컬럼으로의 데이터 이관(백필) DDL 포함 여부 - 마이그레이션과 애플리케이션 배포의 순서 (마이그레이션 선행 필수)
- 컬럼명 변경으로 인한 기존 누적값 손실 가능성 검토
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-streamer/src/main/java/com/loopers/domain/ProductMetrics.java`
at line 23, Add an explicit `@Column` declaration to the
ProductMetrics.orderLineCount field to match ProductMetricsDaily (use
`@Column`(name = "order_line_count", nullable = false) on the orderLineCount field
in class ProductMetrics) so the mapping is explicit and consistent; after
changing the entity, coordinate with the external migration pipeline to ensure a
migration/DDL is applied or a backfill from any existing order_count column is
included, confirm migration runs before app deployment, and verify no historical
metric data will be lost by the column rename.
Comment on lines
+22
to
+28
| @Modifying | ||
| @Query(value = """ | ||
| INSERT INTO product_metrics_daily (product_id, metric_date, like_count, view_count, order_line_count, order_amount) | ||
| VALUES (:productId, :metricDate, -1, 0, 0, 0) | ||
| ON DUPLICATE KEY UPDATE like_count = like_count - 1 | ||
| """, nativeQuery = true) | ||
| void decrementLikeCount(@Param("productId") Long productId, @Param("metricDate") LocalDate metricDate); |
There was a problem hiding this comment.
decrementLikeCount가 신규 행을 -1로 생성하는 동작은 운영 관점에서 주의가 필요하다
이벤트 스트림에서 unlike가 like보다 먼저 도달하는 순서 역전이나 at-least-once에 따른 중복 unlike가 발생하면 like_count가 음수 상태로 product_metrics_daily에 영속되고, 이후 배치가 해당 날짜 구간을 product_rank_snapshot에 집계할 때 음수 기여분이 그대로 스냅샷에 남는다. 일별 Redis ZSET 방식과 달리 스냅샷은 TTL로 자동 회복되지 않으므로, 운영 중 영구적인 스코어 왜곡 가능성이 있다.
최소한 다음 중 하나를 검토하라.
- 배치 집계 SQL에서
GREATEST(SUM(like_count), 0)형태로 하한 클램핑 decrementLikeCount시 기존 행이 없으면 INSERT를 스킵하도록 구조 변경(ON DUPLICATE KEY UPDATE만 유효한 update 쿼리로 치환)- 또는 like 이벤트는 delta 방식 대신 멱등 테이블(
product_like_daily(product_id, user_id, metric_date))로 관리 후 집계
추가 테스트 제안: "unlike 이벤트가 like보다 먼저 도착한 경우, 배치 집계 결과의 score/likeCount가 음수로 반영되지 않는다"는 회귀 테스트를 RankingAggregationJobE2ETest에 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricsDailyRepository.java`
around lines 22 - 28, The current decrementLikeCount repository method inserts a
new row with like_count = -1 which can persist negative likes; change the
behavior so negative scores cannot be persisted by either: (A) modifying the
batch aggregation to clamp like totals with GREATEST(SUM(like_count), 0) when
writing to product_rank_snapshot, or (B) changing decrementLikeCount (the
repository method) to not INSERT when the row doesn't exist — replace the INSERT
... ON DUPLICATE KEY UPDATE pattern in decrementLikeCount with an UPDATE-only
statement (so no new row is created), or (C) migrate like handling to an
idempotent product_like_daily(user_id, product_id, metric_date) table and
compute deltas during aggregation; after choosing one approach, add a regression
E2E test in RankingAggregationJobE2ETest that ensures an arriving unlike before
a like does not produce a negative score in the final snapshot.
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.
Round 10 - Spring Batch 기반 주간/월간 랭킹 집계 시스템
📌 Summary
product_metrics_daily(일별 집계)를 기반으로 주간(7일)/월간(30일) 롤링 윈도우 집계를 수행하고, 스냅샷 테이블(product_rank_snapshot)에 적재하여 API로 제공합니다.rankingType파라미터 확장(DAILY/WEEKLY/MONTHLY)의 전체 파이프라인을 구현했습니다.🧭 Context & Decision
① product_metrics_daily — 테이블 추가 생성
기존
product_metrics는 상품당 1행으로 누적값만 저장하며,ProductMetricsReadModel(commerce-api)이 상품 목록 좋아요순 정렬에 사용 중입니다. 일별 구조로 변경하면 상품당 여러 행이 생겨 기존 JOIN이 깨집니다.결정: 기존
product_metrics(누적)는 유지하고,product_metrics_daily를 별도 생성하여 일별 집계를 쌓습니다.설계 포인트:
(metric_date, product_id): 배치 SQL이metric_date BETWEEN으로 기간 범위 조회하므로metric_date가 선행 컬럼이어야 range scan 가능. 스트리머 upsert는 두 컬럼 모두 등호(=) 조건이라 PK 순서에 영향받지 않음like_count음수 허용: delta 방식. unlike 시 -1이 되어야 7일/30일 SUM 집계 시 Redis 일간 랭킹의 점수 차감 방식과 일치 (월요일 like(+1) → 화요일 unlike(-1) → 주간 SUM = 0)order_amount추가: 일간 랭킹과 동일한 score 공식(view×0.1 + like×0.2 + order_amount×0.7)을 주간/월간에도 적용하기 위함.order_amount산출에 price×quantity가 필요하므로PaymentCompletedEventHandler가orderedProducts를 데이터 소스로 사용하도록 변경.해당 변경으로
productId는orderedProducts에서 꺼낼 수 있으므로, payload의 불필요한 필드인productIds를 제거함③ Chunk-Oriented Processing — Reader에서 집계 완료
Processor를 생략한 이유: Reader의 집계 쿼리(GROUP BY + JOIN + ROW_NUMBER())가 모든 계산을 완료합니다. Processor가 담당할 역할이 단순 필드 매핑뿐이므로, RowMapper에서 엔티티를 직접 생성하여 Processor를 생략했습니다.
DB에서 GROUP BY로 합산하는 이유: 상품 100만개(대량의 데이터 가정) × 7일 = 700만건의 raw 데이터를 애플리케이션으로 읽어와 합산하는 것보다, DB에서 GROUP BY로 합산 후 최대 100건만 가져오는 것이 메모리와 네트워크 모두 효율적입니다.
④ 멱등성 — beforeStep DELETE + UNIQUE 제약
DELETE를 별도 Step으로 분리하지 않는 이유: DELETE Step 성공 → INSERT Step 중간 실패 → 재시작 시 DELETE Step이 스킵(이미 성공 상태)되어 부분 INSERT 위에 중복 INSERT가 발생할 수 있습니다.
StepExecutionListener.beforeStep()에서 DELETE를 수행하면, Step 재시작 시에도 항상 DELETE가 먼저 실행되어 멱등성이 보장됩니다.추가로
(ranking_type, rank_date, product_id)UNIQUE 제약을 안전망으로 설정했습니다.⑤ Reader SQL 설계
product_id ASC: 결정적(deterministic) 정렬HAVING rank <= 100이 동작하지 않음p.deleted_at IS NULL,p.visibility = 'VISIBLE',b.deleted_at IS NULL로 삭제/비노출 상품 및 삭제된 브랜드 상품 제외⑥ 랭킹 API 확장 — Facade 분기
RankingFacade에서rankingType에 따라 분기합니다. DAILY는 기존 Redis 로직을 그대로 사용하고, WEEKLY/MONTHLY는 스냅샷 테이블에서 DB 조회합니다.⑦ 롤링 윈도우 — 주간 정의
롤링 윈도우 선택. 실제 이커머스에서 사용자는 "2주차 랭킹"이 아니라 "최근 일주일간 인기 상품"을 원합니다. 가능한 최신 데이터를 제공하는 것이 UX 관점에서 자연스럽습니다.
🔁 Flow Diagram
전체 데이터 흐름
배치 Job 실행 흐름
sequenceDiagram participant Scheduler as Cron / Manual participant Job as rankingAggregationJob participant Step as aggregateRankingStep participant DB as MySQL Scheduler->>Job: launch(rankingType=WEEKLY, endDate=2026-04-14) Job->>Step: execute Note over Step: beforeStep Step->>DB: DELETE FROM product_rank_snapshot<br/>WHERE ranking_type='WEEKLY' AND rank_date='2026-04-14' Note over Step: Reader (JdbcCursorItemReader) Step->>DB: SELECT ... FROM product_metrics_daily<br/>JOIN products, brands<br/>WHERE metric_date BETWEEN '2026-04-08' AND '2026-04-14'<br/>GROUP BY product_id ... TOP 100 Note over Step: Writer (JpaItemWriter, chunk=100) Step->>DB: INSERT INTO product_rank_snapshot (100건) Step-->>Job: COMPLETED🧪 Verification
배치 E2E 테스트 —
RankingAggregationJobE2ETestweeklyAggregation_aggregates7DaysmonthlyAggregation_aggregates30DaysweeklyAggregation_excludesDataOutsideWindowmonthlyAggregation_excludesDataOutsideWindowreAggregation_replacesExistingSnapshots_noDuplicatesexcludesDeletedProductsexcludesHiddenProductsexcludesProductsOfDeletedBrandtiedScore_orderedByProductIdAsc@TestPropertySource에 가중치(view=0.1, like=0.2, order=0.7) 명시하여 테스트 self-containedisCloseTo(expected, within(0.01))로 부동소수점 안전하게 비교findAll()결과는rankPosition기준 정렬 후 검증 (JPA findAll 정렬 미보장)스트리머 통합 테스트 —
ProductMetricsDailyRepositoryTestAPI E2E 테스트 —
RankingV1ApiE2ETest확장rankingType=WEEKLY→ 스냅샷 데이터 반환rankingType=MONTHLY→ 스냅샷 데이터 반환⏭ 미구현 / 향후 과제
@Value로 빈 생성 시 1회 주입. DB 테이블 기반으로 런타임 변경 가능하도록 개선✅ Checklist
🧱 Spring Batch
🧩 Ranking API
✅ 변경 범위
새로 생성한 파일
domain/ProductMetricsDaily.javadomain/ProductMetricsDailyId.javainfrastructure/ProductMetricsDailyRepository.javadomain/ranking/RankingType.javadomain/ranking/ProductRankSnapshot.javainfrastructure/ranking/ProductRankSnapshotJpaRepository.javabatch/job/ranking/RankingAggregationJobConfig.javadomain/ranking/ProductRankSnapshotQueryRepository.javainfrastructure/ranking/ProductRankSnapshotQueryRepositoryImpl.java수정한 파일
PaymentCompletedEventHandlerproduct_metrics_daily에order_amount적재 추가,orderedProducts활용ProductViewedEventHandlerproduct_metrics_dailyview_count 적재 추가ProductLikedEventHandlerproduct_metrics_dailylike_count 적재 추가ProductUnlikedEventHandlerproduct_metrics_dailylike_count 감소 적재 추가PaymentCompletedEventPayloaduserId,productIds) 제거ProductMetrics/ProductMetricsRepositoryorderCount→orderLineCount변경RankingV1ControllerrankingType파라미터 추가 (default: DAILY)RankingFacade