[volume-10] Batch 기반 주간·월간 랭킹 시스템 구현 #415
[volume-10] Batch 기반 주간·월간 랭킹 시스템 구현 #415dfdf0202 wants to merge 2 commits intoLoopers-dev-lab:dfdf0202from
Conversation
📝 WalkthroughWalkthrough일일, 주간, 월간 상품 랭킹 조회 기능을 구현한다. RankingQuery 객체를 도입하여 기간 파라미터를 추가하고, 기간별로 각각의 서비스와 저장소에 분기하는 구조로 RankingFacade를 리팩토링한다. 배치 작업과 지표 수집 로직도 함께 확장된다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as RankingV1Controller
participant Facade as RankingFacade
participant Query as RankingQuery
participant DailyService as RankingService<br/>(Daily)
participant WeeklyService as WeeklyRankingService
participant MonthlyService as MonthlyRankingService
participant ProductService
participant Database as Repository
Client->>Controller: GET /api/v1/rankings<br/>(period=daily|weekly|monthly)
Controller->>Query: RankingQuery.of(period, date, page, size)<br/>[검증: page≥1, size∈[1,100]]
Query-->>Controller: RankingQuery 객체
Controller->>Facade: getRankings(query)
Facade->>Facade: switch(query.period())
alt period == DAILY
Facade->>DailyService: getTopRankings(date, page, size)
DailyService->>Database: rankingRepository.getTopRankings(...)
Database-->>DailyService: List<RankingEntry>
DailyService->>Database: rankingRepository.getTotalCount(...)
Database-->>DailyService: totalCount
DailyService-->>Facade: RankingSlice(entries, totalCount)
else period == WEEKLY
Facade->>WeeklyService: getRankings(date, page, size)
WeeklyService->>Database: weeklyRankingRepository.getTopRankings(periodKey, ...)
Database-->>WeeklyService: List<RankingEntry>
WeeklyService->>Database: weeklyRankingRepository.getTotalCount(periodKey)
Database-->>WeeklyService: totalCount
WeeklyService-->>Facade: RankingSlice(entries, totalCount)
else period == MONTHLY
Facade->>MonthlyService: getRankings(date, page, size)
MonthlyService->>Database: monthlyRankingRepository.getTopRankings(periodKey, ...)
Database-->>MonthlyService: List<RankingEntry>
MonthlyService->>Database: monthlyRankingRepository.getTotalCount(periodKey)
Database-->>MonthlyService: totalCount
MonthlyService-->>Facade: RankingSlice(entries, totalCount)
end
Facade->>ProductService: findAllByIds(productIds)
ProductService->>Database: productRepository.findByIds(...)
Database-->>ProductService: List<Product>
ProductService-->>Facade: products
Facade->>Facade: 각 RankingEntry와 Product 결합<br/>→ FindRankingItemResDto
Facade-->>Controller: FindRankingListResDto(items, totalCount, page, size)
Controller-->>Client: ApiResponse.success(dto)
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: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/service/RankingService.java (1)
29-36:⚠️ Potential issue | 🟡 Minor
getProductRanking/getProductRank메서드의 파라미터와 반환 타입을 도메인 경계에 맞게 통일해야 한다.같은 서비스 내에서
getTopRankings/getTotalCount는LocalDate파라미터를 받고RankingPeriodKeyFactory.toDailyKey를 적용하지만,getProductRanking/getProductRank는 여전히String date를 받는다. 호출자(ProductFacade)가yyyyMMdd포맷을 넘기더라도 포맷이 다를 경우(예:yyyy-MM-dd) Redis ZSET 키가 달라져 조회가 0건으로 조용히 실패하며, 이는 상품 상세 페이지의 "내 랭킹" 표시 누락으로 이어진다. 또한 반환 타입이RankingRepository.RankingEntry(리포지토리 중첩 타입)인데, 서비스에서 이미 임포트하고 있는 도메인RankingEntry가 별도로 존재하여 도메인 경계가 모호하다.수정 방안:
- 두 메서드의 파라미터를
LocalDate로 통일하고, 내부에서RankingPeriodKeyFactory.toDailyKey(date)를 적용한다.- 반환 타입을 도메인
com.loopers.domain.ranking.model.RankingEntry로 변경하고, 리포지토리 중첩 타입은 단계적으로 제거한다.- 기존 호출자 스택(
ProductFacade)의 파라미터 타입도LocalDate로 변경하여 포맷 실수를 방지한다.추가 테스트: 새
LocalDate기반 호출이 기존yyyyMMdd키와 동일하게 변환되는지, 그리고getProductRanking결과가 null일 때getProductRank가 올바르게 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/domain/ranking/service/RankingService.java` around lines 29 - 36, getProductRanking and getProductRank accept String and return the repository's nested RankingEntry which breaks domain boundaries and allows date-format bugs; change both method signatures in RankingService (getProductRanking, getProductRank) to take java.time.LocalDate, convert inside the methods using RankingPeriodKeyFactory.toDailyKey(date) before calling rankingRepository, and change return types to the domain model com.loopers.domain.ranking.model.RankingEntry (and have getProductRank return its rank or null when the domain entry is null); update callers such as ProductFacade to pass LocalDate instead of String and add tests verifying the LocalDate->toDailyKey produces the same yyyyMMdd key and that getProductRank returns null when no entry exists.apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java (1)
50-57:⚠️ Potential issue | 🟡 Minor
DataIntegrityViolationException처리를DUPLICATE_EVENT_ID에러로 분리하고, 로그 수준을 명확히 한다.
existsById조회와save사이에 경쟁 조건이 존재한다. Kafka 파티션 재할당이나 리밸런싱 시 동일eventId를 두 병렬 컨슈머가 처리할 수 있으며, 둘 다existsById체크를 통과한 후save를 시도해DataIntegrityViolationException을 유발한다. 현재 코드는 모든 예외를 동일하게 처리하므로, 정상적인 중복(PK 충돌)인지 비정상 장애인지 판별하기 어렵다.수정안:
processRecord메서드에서DataIntegrityViolationException을 명시적으로 catch하여 경고 수준 로그로 기록한다.- 다른 런타임 예외는 에러 수준으로 기록하고, 트랜잭션 롤백이 이루어지도록 한다.
catch (DataIntegrityViolationException e) { log.warn("중복 이벤트 처리됨 - eventId: {}, eventType: {}", event.eventId(), event.eventType()); } catch (Exception e) { log.error("catalog-events 처리 실패 - record: {}", record, e); throw e; // 트랜잭션 롤백 }
decrementLikeCount의 하한 방어는ProductMetricsEntity.decrementLikeCount()에서if (likeCount > 0)조건으로,ProductMetricsDailyJpaRepository.decrementLikeCount()에서GREATEST(like_count - 1, 0)SQL로 이미 보장되어 있다.FAVORITE_REMOVED가 최초 이벤트로 도착해도 likeCount 는 음수가 되지 않는다.추가 테스트:
- 동일
eventId중복 소비 시DataIntegrityViolationException발생 후 로그에 경고 메시지가 남는지 확인FAVORITE_REMOVED를 likeCount 가 0인 상태에서 처리했을 때 음수가 되지 않는지 확인🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java` around lines 50 - 57, The save of EventHandledEntity in CatalogEventConsumer.processRecord can throw DataIntegrityViolationException due to a race between existsById and save; update processRecord to catch DataIntegrityViolationException around the eventHandledRepository.save(EventHandledEntity.of(...)) call and log a warning (include event.eventId() and event.eventType()), then catch generic Exception separately, log an error including the record and exception, and rethrow the exception to ensure transaction rollback; keep the switch handling (incrementViewCount/incrementLikeCount/decrementLikeCount) unchanged and only adjust the exception handling surrounding eventHandledRepository.save.
🟡 Minor comments (11)
apps/commerce-api/src/main/java/com/loopers/domain/ranking/service/MonthlyRankingService.java-17-24 (1)
17-24:⚠️ Potential issue | 🟡 Minor
page,size입력 가드가 없어 페이지네이션 계산이 깨질 수 있다.운영 관점에서
page = 0또는 음수가 유입되면offset = (page - 1) * size가 음수가 되어 하위PageRequest.of(page, size)에서IllegalArgumentException이 발생하거나(음수 페이지),size = 0이면 하위 구현(MonthlyRankingRepositoryImpl)의 나눗셈에서 예외가 발생한다. 상위RankingFacade/컨트롤러에서 이미 검증하더라도, 도메인 서비스가 직접 테스트되는 경로와 향후 호출자 확장을 고려해 방어 가드를 두는 것이 안전하다.수정안:
♻️ 입력 검증 추가
public RankingSlice getRankings(LocalDate date, int page, int size) { + if (page < 1 || size < 1) { + throw new IllegalArgumentException("page must be >= 1 and size must be >= 1"); + } int offset = (page - 1) * size; String periodKey = RankingPeriodKeyFactory.toMonthlyKey(date);추가 테스트:
page = 0,page < 0,size <= 0,date = null경로에 대한 단위 테스트를 추가하기를 권한다. 동일 패턴은WeeklyRankingService/RankingService에도 공통 적용이 필요한지 점검하기를 권한다.🤖 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/service/MonthlyRankingService.java` around lines 17 - 24, The getRankings method in MonthlyRankingService lacks input guards: validate that date is not null, page >= 1 and size > 0 before computing offset and calling repository.getTopRankings/getTotalCount (and before calling RankingPeriodKeyFactory.toMonthlyKey); if any check fails throw a clear IllegalArgumentException (e.g., "page must be >= 1", "size must be > 0", "date must not be null"). Add corresponding unit tests covering page = 0, page < 0, size <= 0, and date = null to ensure the guards work; consider applying the same validation pattern to WeeklyRankingService/RankingService if they expose similar methods.apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/repository/MonthlyRankingJpaRepository.java-10-10 (1)
10-10:⚠️ Potential issue | 🟡 Minor
deleteByPeriodKey호출 시 대량 삭제 성능을 점검하고 필요시 벌크 DELETE로 개선하기를 권한다.Spring Batch 프레임워크는
StepBuilder.tasklet(tasklet, transactionManager)를 통해 자동으로 tasklet 전체를 트랜잭션으로 감싼다. 따라서deleteByPeriodKey→saveAll양쪽 모두 동일 트랜잭션 내에서 원자적으로 실행되며, 추가@Transactional선언이 필요하지 않다.다만 성능 관점에서 Spring Data 파생
deleteByPeriodKey메서드는 내부적으로 엔티티를 먼저 SELECT한 후 1건씩 DELETE를 수행한다. 현재 TOP 100 규모라면 문제가 없으나, 이후 상한이 늘어나거나 동시 실행 빈도가 증가하면 데이터베이스 I/O 병목이 될 수 있다.벌크 DELETE 쿼리로 개선하려면 다음과 같이 변경하기를 권한다:
개선 예시
`@Modifying` `@Query`("delete from MonthlyRankingEntity m where m.periodKey = :periodKey") void deleteByPeriodKey(`@Param`("periodKey") String periodKey);이렇게 변경하면 데이터베이스에서 단일 DELETE 쿼리로 처리되어 대량 데이터 삭제 시 성능이 크게 개선된다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/repository/MonthlyRankingJpaRepository.java` at line 10, The derived Spring Data method deleteByPeriodKey in MonthlyRankingJpaRepository performs selects and individual deletes which can be very slow for large volumes; replace it with a bulk DELETE JPQL by annotating the repository method (deleteByPeriodKey) with `@Modifying` and a `@Query` that executes "delete from MonthlyRankingEntity m where m.periodKey = :periodKey" and add `@Param`("periodKey") to the parameter so the removal is executed as a single bulk DELETE at the database level.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/MonthlyRankingRepositoryImpl.java-18-23 (1)
18-23:⚠️ Potential issue | 🟡 Minor
size = 0입력 시ArithmeticException가능성이 있다.운영 관점에서
page/size는 사용자 입력을 거쳐 유입되며, 업스트림 검증이 누락되거나 우회될 경우offset / size에서ArithmeticException: / by zero가 500으로 그대로 노출된다. 또한 호출자가offset을size의 배수가 아닌 값으로 전달할 경우page = offset / size로 인해 요청된 오프셋이 아닌 페이지 경계로 절삭되어 응답이 조용히 어긋난다.수정안:
♻️ 방어 로직 추가
`@Override` public List<RankingEntry> getTopRankings(String periodKey, int offset, int size) { - int page = offset / size; + if (size <= 0) { + return List.of(); + } + int page = offset / size; return repository.findByPeriodKeyOrderByRankNoAsc(periodKey, PageRequest.of(page, size)).stream() .map(entity -> new RankingEntry(entity.getProductId(), entity.getScore(), entity.getRankNo())) .toList(); }추가 테스트:
size = 0,offset이size의 배수가 아닌 경우,offset >= totalCount인 경우에 대한 경계 테스트를 추가하기를 권한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/MonthlyRankingRepositoryImpl.java` around lines 18 - 23, getTopRankings currently divides offset by size without validating inputs which can cause ArithmeticException when size == 0 and silently return a shifted window when offset is not a multiple of size; add input validation to throw IllegalArgumentException if size <= 0 or offset < 0, compute page using Math.floorDiv(offset, size), call repository.findByPeriodKeyOrderByRankNoAsc(periodKey, PageRequest.of(page, size)), then adjust the returned page content to honor the exact offset (use stream().skip(offset % size).limit(size) or equivalent) so callers always get 'size' entries starting at 'offset'; reference getTopRankings, repository.findByPeriodKeyOrderByRankNoAsc, PageRequest.of, and RankingEntry when making changes and add unit tests for size=0, non-multiple offsets, and offset >= totalCount.apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.java-14-14 (1)
14-14:⚠️ Potential issue | 🟡 Minor
period파라미터의 필수 여부·기본값·허용값 스펙이 문서에서 빠졌다.
required속성이 없어 Swagger UI 상 선택 파라미터로 보이고, 누락 시 API 동작(기본 daily인지 400인지)이 OpenAPI 스펙으로 드러나지 않는다. 또한daily|weekly|monthly외 값이 들어올 때의 계약이 불명확하다. 운영/클라이언트 개발자 관점에서 오인 호출과 의도치 않은 기간 기본값 사용을 유발한다.
- 수정안:
@Parameter(required = true, schema =@Schema(allowableValues = {"daily","weekly","monthly"}))로 명시하거나, 서버에서RankingQuery.of(period, ...)가 허용값 외 입력을CoreException(BAD_REQUEST)로 거절하는지 확인하고 description 에 기본값/에러 동작을 기술한다.- 추가 테스트: 알 수 없는
period값과 누락 케이스에 대한 컨트롤러 E2E 검증을 추가한다.🤖 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/RankingV1ApiSpec.java` at line 14, The OpenAPI spec for the "period" parameter is missing required/allowed-values/default behavior; update the `@Parameter` annotation for the period parameter in RankingV1ApiSpec (the method signature containing "String period") to explicitly declare required = true and a Schema with allowableValues = {"daily","weekly","monthly"} and include description text stating the default and error behavior, or alternatively ensure RankingQuery.of(period, ...) performs validation and throws CoreException(BAD_REQUEST) for unknown values and document that in the description; also add E2E controller tests that cover missing period and unknown period cases to ensure the API surface and runtime behavior match the spec.apps/commerce-api/src/main/java/com/loopers/domain/ranking/service/WeeklyRankingService.java-17-24 (1)
17-24:⚠️ Potential issue | 🟡 Minor
page입력 검증이 없어 잘못된 요청 시 500 으로 터진다.
page <= 0이면offset = (page-1)*size가 음수가 되어PageRequest.of(page, size)에서IllegalArgumentException이 발생하고, 글로벌 예외 처리를 거치지 않으면 사용자에게 500 으로 노출된다.size <= 0역시 동일하다. 프로젝트 규약상 입력 검증은 도메인/유스케이스 레이어에서CoreException으로 통일해야 한다.
- 수정안: 메서드 진입부에서
page < 1 || size < 1시CoreException(ErrorType.BAD_REQUEST, ...)로 빠르게 실패시키고, 로그 메시지와 사용자 메시지를 분리한다.- 추가 테스트:
page=0,page=-1,size=0에 대해CoreException이 발생하는지 확인하는 단위 테스트를 추가한다.제안 수정안
public RankingSlice getRankings(LocalDate date, int page, int size) { + if (page < 1 || size < 1) { + throw new CoreException(ErrorType.BAD_REQUEST, "page/size 는 1 이상이어야 합니다."); + } int offset = (page - 1) * size; String periodKey = RankingPeriodKeyFactory.toWeeklyKey(date);As per coding guidelines: "Java Service 리뷰 기준... 유스케이스 단위로 책임이 정리되어 있는지, 부수 효과가 명확한지 점검한다" 및 CoreException 통일 규약.
🤖 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/service/WeeklyRankingService.java` around lines 17 - 24, getRankings in WeeklyRankingService lacks validation for page/size so negative/zero values lead to IllegalArgumentException; at the start of getRankings add a guard that throws new CoreException(ErrorType.BAD_REQUEST, "user-facing message") when page < 1 || size < 1, and log a separate developer/debug message (not the user message) before throwing; keep the rest of the method (RankingPeriodKeyFactory.toWeeklyKey, repository.getTopRankings, repository.getTotalCount) unchanged; add unit tests asserting CoreException is thrown for page=0, page=-1 and size=0.apps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/repository/ProductMetricsDailyJpaRepository.java-41-53 (1)
41-53:⚠️ Potential issue | 🟡 Minor
decrementLikeCount가 존재하지 않는 행을 빈 레코드로 삽입해 MV/집계를 오염시킨다.현재 SQL은 대상
(metricDate, productId)행이 없을 때like_count=0, score=0의 빈 행을 INSERT 한다. 감소 연산의 의미상 “대상 없음”은 NOOP이 되어야 하는데, 이 구현은 이벤트 순서가 뒤집히거나 유실로 like가 한 번도 집계되지 않은 상품에 대해product_metrics_daily에 0행을 남긴다.운영 관점의 영향:
- 주간/월간 배치는
product_metrics_daily를group by product_id로 집계하는데, 0행이 다수 추가되면 group 수가 늘어 필요 없는 정렬/연산 비용이 발생한다.count(*)류 메트릭/모니터링이 왜곡된다.CASE WHEN like_count > 0보호로 score는 안전하지만, 의도하지 않은 행 증가는 그 자체로 장애 원인 추적을 어렵게 한다.🔧 제안 수정안
INSERT 경로를 없애고 UPDATE-only로 전환한다.
`@Modifying` `@Query`(value = """ - INSERT INTO product_metrics_daily - (metric_date, product_id, view_count, like_count, order_count, score, created_at, updated_at) - VALUES (:metricDate, :productId, 0, 0, 0, 0, NOW(6), NOW(6)) - ON DUPLICATE KEY UPDATE - score = CASE WHEN like_count > 0 THEN score + :scoreDelta ELSE score END, - like_count = GREATEST(like_count - 1, 0), - updated_at = NOW(6) + UPDATE product_metrics_daily + SET score = CASE WHEN like_count > 0 THEN score + :scoreDelta ELSE score END, + like_count = GREATEST(like_count - 1, 0), + updated_at = NOW(6) + WHERE metric_date = :metricDate AND product_id = :productId """, nativeQuery = true) void decrementLikeCount(...);추가 테스트: 대상 행이 존재하지 않을 때
decrementLikeCount호출 후 해당 row가 생성되지 않아야 한다는 단위 테스트를ProductMetricsDailyJpaRepositoryTest에 추가하기를 권장한다.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/main/java/com/loopers/infrastructure/metrics/repository/ProductMetricsDailyJpaRepository.java` around lines 41 - 53, The current decrementLikeCount query inserts a zeroed row when none exists; change it to an UPDATE-only statement: replace the INSERT ... ON DUPLICATE KEY UPDATE block in decrementLikeCount with an UPDATE product_metrics_daily SET score = CASE WHEN like_count > 0 THEN score + :scoreDelta ELSE score END, like_count = GREATEST(like_count - 1, 0), updated_at = NOW(6) WHERE metric_date = :metricDate AND product_id = :productId so no row is created when absent; keep the `@Modifying/`@Query annotations and parameters as-is and add a unit test in ProductMetricsDailyJpaRepositoryTest asserting that calling decrementLikeCount on a non-existent (metricDate, productId) does not create a row.k6/scripts/ranking-api-load-test.js-50-56 (1)
50-56:⚠️ Potential issue | 🟡 Minor빈 응답을 통과시키는 느슨한 check 로직을 강화해야 한다.
운영 관점에서 대상 기간에 데이터가 시드되지 않았거나 MV 교체 중 공백이 발생하면
rankings가 빈 배열로 내려올 수 있는데, 현재 체크는Array.isArray만 보기 때문에 200 + 빈 결과를 정상으로 판단한다. 이 경우 실제 조회 경로의 성능/정상성이 검증되지 않은 채 threshold만 충족되어 회귀를 놓치게 된다.🔧 제안 수정안
check(res, { [`${period} status is 200`]: (r) => r.status === 200, [`${period} has rankings array`]: (r) => { const body = r.json(); - return !!body && !!body.data && Array.isArray(body.data.rankings); + return !!body && !!body.data && Array.isArray(body.data.rankings) && body.data.rankings.length > 0; }, });추가 테스트: TARGET_DATE에 MV/ZSET 데이터가 비어 있을 때 threshold가 실패하도록 드라이런을 추가하고, 시드 스크립트 실행 후에만 k6를 기동하는 사전 조건 체크를 CI에 추가하기를 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/scripts/ranking-api-load-test.js` around lines 50 - 56, The current check in the check(res, {...}) block allows an empty rankings array to pass; update the second assertion so it verifies not just Array.isArray(body.data.rankings) but also that body.data.rankings.length > 0 (e.g., change the lambda in the `${period} has rankings array` check to parse body with r.json() and return !!body && !!body.data && Array.isArray(body.data.rankings) && body.data.rankings.length > 0), and optionally add a separate check/assertion that fails with a clear message when rankings is an empty array so CI/k6 fails when TARGET_DATE MV/ZSET are empty.apps/commerce-streamer/src/test/java/com/loopers/infrastructure/metrics/ProductMetricsDailyJpaRepositoryTest.java-44-74 (1)
44-74:⚠️ Potential issue | 🟡 Minor경계값과 실패 경로가 누락되어
decrementLikeCount보호 로직이 검증되지 않는다.네이티브 쿼리의 핵심 보호 장치는
CASE WHEN like_count > 0 ... ELSE score END와GREATEST(like_count - 1, 0)이다. 그러나 현재 테스트는like_count=1 → 0경로만 다루어 아래 케이스가 모두 미검증 상태이다. 운영 시 like 이벤트가 유실되거나 순서가 뒤집히면like_count=0상태에서 decrement가 유입되는 상황이 실제로 발생한다.
like_count=0상태에서 decrement →score변화 없음,like_count0 유지- 대상
(metricDate, productId)행이 없는 상태에서 decrement → INSERT 되는 기본 행이 score에 영향을 주지 않는지like_count >= 2에서 decrement → score 감소 + like_count 1 감소🔧 추가 테스트 제안
+ `@DisplayName`("좋아요 수가 0일 때 감소해도 score는 차감되지 않는다") + `@Test` + void keepScoreWhenLikeCountIsZero() { + LocalDate metricDate = LocalDate.of(2026, 4, 15); + productMetricsDailyJpaRepository.incrementViewCount(metricDate, 2L, 0.1d); + + productMetricsDailyJpaRepository.decrementLikeCount(metricDate, 2L, -0.2d); + + Long likeCount = jdbcTemplate.queryForObject( + "select like_count from product_metrics_daily where metric_date = ? and product_id = ?", + Long.class, metricDate, 2L); + Double score = jdbcTemplate.queryForObject( + "select score from product_metrics_daily where metric_date = ? and product_id = ?", + Double.class, metricDate, 2L); + assertAll( + () -> assertThat(likeCount).isEqualTo(0L), + () -> assertThat(score).isEqualTo(0.1d) + ); + }또한
@Transactional로 각 테스트가 롤백되는데@AfterEach에서truncateAllTables()를 호출하는 부분은 중복 정리이다. 테스트 데이터가 커밋되어야 하는 시나리오가 생기면@Transactional제거 + truncate 유지 중 하나로 일원화하기를 권장한다.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/metrics/ProductMetricsDailyJpaRepositoryTest.java` around lines 44 - 74, The current test decrementScoreBeforeLikeCount only covers the 1→0 path and doesn't verify the native query protection logic; add separate tests within the DecrementLikeCount nested class to cover: (1) decrement when like_count == 0 (call productMetricsDailyJpaRepository.decrementLikeCount for metricDate/productId with no prior increment and assert like_count stays 0 and score unchanged), (2) decrement when the row does not exist (call decrementLikeCount for a metricDate/productId that has no row and assert a row is inserted if expected and that score is unchanged), and (3) decrement when like_count >= 2 (pre-seed with productMetricsDailyJpaRepository.incrementLikeCount twice or insert a row, call decrementLikeCount and assert score decreased and like_count decremented by 1); also reconcile test cleanup by either removing `@AfterEach.truncateAllTables`() if tests are `@Transactional` and rollback, or remove `@Transactional` and keep truncateAllTables(), to avoid redundant cleanup.apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java-75-93 (1)
75-93:⚠️ Potential issue | 🟡 Minor시드 수식이 음수 카운터를 만들어 실제 스키마/의미와 불일치한다.
orderCount = 100 - productId는productId > 100인 경우 음수가 된다(예: productId=150 → orderCount=-50).product_metrics_daily컬럼이UNSIGNED라면 INSERT 자체가 실패하고, signed BIGINT라면 성공하지만 운영에서는 존재할 수 없는 음수 카운터로 배치 집계를 검증하게 되어 테스트 신뢰도가 떨어진다. 또한 targetDate(2026-04-15, ISO W16)의 주 경계인 2026-04-18~19가 시드 범위에서 누락되어weeklyStart/End가 포함하는 전체 주간을 커버하지 못한다.🔧 제안 수정안
- private void seedWeek(LocalDate baseDate) { - LocalDate start = baseDate.minusDays(2); - List<Integer> productIds = IntStream.rangeClosed(1, 150).boxed().toList(); - for (LocalDate date = start.minusDays(2); !date.isAfter(start.plusDays(4)); date = date.plusDays(1)) { + private void seedWeek(LocalDate baseDate) { + LocalDate weekStart = baseDate.with(java.time.DayOfWeek.MONDAY); + LocalDate weekEnd = weekStart.plusDays(6); + List<Integer> productIds = IntStream.rangeClosed(1, 150).boxed().toList(); + for (LocalDate date = weekStart; !date.isAfter(weekEnd); date = date.plusDays(1)) { for (Integer productId : productIds) { - long viewCount = 200 - productId; - long likeCount = 150 - productId; - long orderCount = 100 - productId; + long viewCount = Math.max(0L, 200L - productId); + long likeCount = Math.max(0L, 150L - productId); + long orderCount = Math.max(0L, 100L - productId);추가 테스트: 정렬이 전구간에서 단조 비증가임을 검증한다.
- assertThat(rows.get(0).getScore()).isGreaterThanOrEqualTo(rows.get(1).getScore()); + for (int i = 1; i < rows.size(); i++) { + assertThat(rows.get(i - 1).getScore()) + .as("rank %d score >= rank %d score", i, i + 1) + .isGreaterThanOrEqualTo(rows.get(i).getScore()); + }As per coding guidelines ("통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java` around lines 75 - 93, seedWeek currently generates negative counters (e.g., orderCount = 100 - productId) and misses the full target week window; update seedWeek to clamp metric values to zero (or use max(0, ...)) for viewCount/likeCount/orderCount so no negative counters are inserted, and expand the date loop so it covers the entire target week (include the weeklyStart/weeklyEnd range such as 2026-04-18–19) by adjusting start/stop bounds (refer to method seedWeek and variables start, date loop); after fixing, add an integration assertion that the computed weekly ranking is monotonic non-increasing across the full week to validate ordering across the entire seeded range.scripts/batch/run-ranking-batch-benchmark.sh-89-105 (1)
89-105:⚠️ Potential issue | 🟡 Minor동일 파라미터로 weeklyRankingJob을 2회 실행하므로 JobInstance 중복 위험을 점검해야 한다.
main의 2번째 단계("[2/6]")와 5번째 단계("[5/6]")에서 동일한targetDate로weeklyRankingJob을 실행한다.WeeklyRankingJobConfig에RunIdIncrementer가 설정되어 있더라도, Spring Batch의JobLauncherApplicationRunner경로에서는 incrementer가 자동으로 적용되지 않는 경우가 있어JobInstanceAlreadyCompleteException으로 5번째 실행이 실패할 수 있다. 이는 배치 작업의 반복 실행이 불가능해지는 운영 이슈로 이어진다.제안 수정안
- (cd "$ROOT_DIR" && ./gradlew :apps:commerce-batch:bootRun --args="--spring.batch.job.name=${job_name} --spring.jpa.hibernate.ddl-auto=none targetDate=${TARGET_DATE}" >/tmp/${job_name}.log 2>&1) + local run_id + run_id="$(date +%s%N)" + (cd "$ROOT_DIR" && ./gradlew :apps:commerce-batch:bootRun \ + --args="--spring.batch.job.name=${job_name} --spring.jpa.hibernate.ddl-auto=none targetDate=${TARGET_DATE} run.id=${run_id}" \ + >"/tmp/${job_name}.log" 2>&1)동일
targetDate로 2회 연속 실행하는 회귀 테스트를 스크립트 말미에 추가하여, 두 번째 실행도COMPLETED상태로 정상 종료되는지 검증하기를 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/batch/run-ranking-batch-benchmark.sh` around lines 89 - 105, The script currently invokes the same job twice for the same targetDate (the calls around steps "[2/6]" and "[5/6]"), which can trigger JobInstanceAlreadyCompleteException; update the script to explicitly re-run weeklyRankingJob with the same TARGET_DATE and verify the second run completes by capturing and checking the process exit/log for COMPLETED (use the existing measure_job function and the gradlew invocation that passes --spring.batch.job.name=weeklyRankingJob and targetDate=${TARGET_DATE}); if the second run fails, ensure the job is launched with a RunIdIncrementer-equivalent (or add a unique run-id arg) so repeated executions succeed and log the status for automated verification.apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/service/MetricsService.java-28-31 (1)
28-31:⚠️ Potential issue | 🟡 Minor
@Modifying의 flush 순서를 명시적으로 보장하라.
save()후 네이티브INSERT ... ON DUPLICATE KEY UPDATE쿼리가 같은 트랜잭션에서 실행되는데, 현재@Modifying은flushAutomatically파라미터를 명시하지 않아 기본값(false)이 적용된다. JPA의save()는 persistence context에 등록 후 트랜잭션 커밋 시점에 flush되지만,@Modifying(flushAutomatically=false)는 자동 flush를 하지 않으므로 엔티티 변경이 아직 DB에 반영되지 않은 상태에서 네이티브 쿼리가 먼저 실행될 수 있다.현재는 두 테이블이 별개이고(product_metrics vs product_metrics_daily) 네이티브 쿼리가 멱등성을 보장하므로(
ON DUPLICATE KEY UPDATE) 실질적인 정합성 문제는 없다. 하지만 코드 변경이나 쿼리 수정 시 순서 의존이 생길 여지가 있으므로, 다음 중 하나를 적용하라:
@Modifying(flushAutomatically = true, clearAutomatically = true)설정으로 명시적 flush 보장- 또는 현재 구현이 안전한 이유를 코드 주석으로 명시 (separate tables, idempotent upsert)
🤖 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/metrics/service/MetricsService.java` around lines 28 - 31, The current sequence in MetricsService (metricsRepository.save(metrics); dailyMetricsRepository.incrementViewCount(...)) can run the native upsert before the JPA save is flushed; update the `@Modifying` on the repository method backing incrementViewCount to `@Modifying`(flushAutomatically = true, clearAutomatically = true) so the persistence context is flushed/cleared before the native INSERT ... ON DUPLICATE KEY UPDATE executes, or alternatively add a concise code comment in MetricsService explaining why the current order is safe (separate tables + idempotent upsert) so future maintainers understand the intentional behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32220bbb-bb98-49ae-b54c-a85ec631a74d
📒 Files selected for processing (50)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/dto/FindRankingItemResDto.javaapps/commerce-api/src/main/java/com/loopers/domain/product/service/ProductService.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/model/RankingEntry.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/model/RankingPeriod.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/model/RankingQuery.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/model/RankingSlice.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/repository/MonthlyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/repository/WeeklyRankingRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/service/MonthlyRankingService.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/service/RankingService.javaapps/commerce-api/src/main/java/com/loopers/domain/ranking/service/WeeklyRankingService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/MonthlyRankingEntity.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/entity/WeeklyRankingEntity.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/MonthlyRankingJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/MonthlyRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/WeeklyRankingJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/repository/WeeklyRankingRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/main/java/com/loopers/support/util/RankingPeriodKeyFactory.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/domain/ranking/RankingPeriodTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/AggregatedRankingRow.javaapps/commerce-batch/src/main/java/com/loopers/batch/domain/ranking/RankingPeriodKeyFactory.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/metrics/entity/ProductMetricsDailyEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/entity/MonthlyRankingEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/entity/WeeklyRankingEntity.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/repository/MonthlyRankingJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/repository/WeeklyRankingJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/tasklet/RefreshMonthlyRankingTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/tasklet/RefreshWeeklyRankingTasklet.javaapps/commerce-batch/src/test/java/com/loopers/domain/ranking/RankingPeriodKeyFactoryTest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/repository/ProductMetricsDailyRepository.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/repository/ProductMetricsRepository.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/service/MetricsService.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/entity/ProductMetricsDailyEntity.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/repository/ProductMetricsDailyJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/repository/ProductMetricsDailyRepositoryImpl.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/repository/ProductMetricsRepositoryImpl.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.javaapps/commerce-streamer/src/test/java/com/loopers/domain/metrics/service/MetricsServiceTest.javaapps/commerce-streamer/src/test/java/com/loopers/infrastructure/metrics/ProductMetricsDailyJpaRepositoryTest.javak6/scripts/ranking-api-load-test.jsscripts/batch/run-ranking-batch-benchmark.sh
| public record RankingEntry( | ||
| Long productId, | ||
| double score, | ||
| long rank | ||
| ) { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
도메인 모델 통일 목적의 record 도입에 이견이 없다.
다만 컨텍스트 스니펫에 따르면 RankingRepository 인터페이스는 여전히 내부 RankingRepository.RankingEntry를 선언하고 있고 RankingRepositoryImpl.getProductRanking은 이미 신규 도메인 RankingEntry를 반환하는 상태다. 또한 RankingService.getProductRanking / getProductRank도 RankingRepository.RankingEntry를 그대로 참조하고 있어 컴파일 레벨에서는 맞더라도 타입 일원화가 미완이다. 운영 관점에서 두 동명 타입이 공존하면 이후 리팩터링/IDE 자동완성 시 잘못된 타입 참조로 회귀 버그가 유입될 위험이 있다.
- 수정안:
RankingRepository인터페이스의 내부RankingEntryrecord를 제거하고com.loopers.domain.ranking.model.RankingEntry로 교체,RankingService의 반환/변수 타입도 동일하게 교체한다. - 추가 테스트:
RankingService.getProductRanking단위 테스트에서 도메인RankingEntry가 반환되는지 타입을 명시적으로 검증한다.
🤖 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/model/RankingEntry.java`
around lines 3 - 8, Remove the duplicate inner record from the RankingRepository
interface and replace all references to RankingRepository.RankingEntry with the
new top-level com.loopers.domain.ranking.model.RankingEntry; update
RankingRepository method signatures and any implementing method like
RankingRepositoryImpl.getProductRanking to return the top-level RankingEntry,
and change RankingService methods (getProductRanking, getProductRank) and their
local variables/returns to use com.loopers.domain.ranking.model.RankingEntry as
well; finally add/adjust the unit test for RankingService.getProductRanking to
explicitly assert the returned element(s) are instances of the top-level
RankingEntry type.
| public static RankingQuery of(String period, String date, int page, int size) { | ||
| validate(page, size); | ||
| try { | ||
| return new RankingQuery(RankingPeriod.from(period), LocalDate.parse(date, DATE_FORMAT), page, size); | ||
| } catch (DateTimeParseException e) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "날짜 형식이 올바르지 않습니다. (yyyyMMdd)"); | ||
| } | ||
| } |
There was a problem hiding this comment.
date가 null일 때 NullPointerException이 발생한다.
LocalDate.parse(date, DATE_FORMAT)는 date가 null이면 DateTimeParseException이 아닌 NullPointerException을 던지므로, 현재 catch 블록으로 잡히지 않는다. 컨트롤러에서 @RequestParam을 required=false로 바인딩하거나 호출측에서 null을 넘기는 순간 ApiControllerAdvice를 거치지 않는 500 응답으로 이어질 수 있다. 운영 관점에서는 사용자 입력 검증 실패가 5xx로 기록되어 장애 알람/오탐을 유발한다.
date 역시 null/blank인 경우 "오늘 날짜" 등 기본값을 적용하거나 명시적으로 CoreException(BAD_REQUEST)를 던지도록 분기하고, 테스트에 null·빈 문자열·잘못된 포맷("2026-04-17", "20260431" 같은 존재하지 않는 날짜) 케이스를 추가해야 한다.
🛡️ 제안
public static RankingQuery of(String period, String date, int page, int size) {
validate(page, size);
+ if (date == null || date.isBlank()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "날짜는 필수 입력입니다. (yyyyMMdd)");
+ }
try {
return new RankingQuery(RankingPeriod.from(period), LocalDate.parse(date, DATE_FORMAT), page, size);
} catch (DateTimeParseException e) {
throw new CoreException(ErrorType.BAD_REQUEST, "날짜 형식이 올바르지 않습니다. (yyyyMMdd)");
}
}📝 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.
| public static RankingQuery of(String period, String date, int page, int size) { | |
| validate(page, size); | |
| try { | |
| return new RankingQuery(RankingPeriod.from(period), LocalDate.parse(date, DATE_FORMAT), page, size); | |
| } catch (DateTimeParseException e) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "날짜 형식이 올바르지 않습니다. (yyyyMMdd)"); | |
| } | |
| } | |
| public static RankingQuery of(String period, String date, int page, int size) { | |
| validate(page, size); | |
| if (date == null || date.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "날짜는 필수 입력입니다. (yyyyMMdd)"); | |
| } | |
| try { | |
| return new RankingQuery(RankingPeriod.from(period), LocalDate.parse(date, DATE_FORMAT), page, size); | |
| } catch (DateTimeParseException e) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "날짜 형식이 올바르지 않습니다. (yyyyMMdd)"); | |
| } | |
| } |
🤖 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/model/RankingQuery.java`
around lines 18 - 25, RankingQuery.of currently calls LocalDate.parse(date,
DATE_FORMAT) which throws NullPointerException when date is null and bypasses
the DateTimeParseException catch; update RankingQuery.of to first check if date
is null or blank and either apply a default (e.g., LocalDate.now()) or
immediately throw new CoreException(ErrorType.BAD_REQUEST, "날짜 형식이 올바르지 않습니다.
(yyyyMMdd)") per your desired behavior, then keep the LocalDate.parse inside the
try/catch to handle format errors (DateTimeParseException) and rethrow
CoreException; also add unit tests for null, empty, invalid format (e.g.,
"2026-04-17", "20260431") and a valid case to verify the new branching in
RankingQuery.of.
| insertWeeklyRanking("2026-W16", 1, first.getId(), 120.0d, 10, 20, 30); | ||
| insertWeeklyRanking("2026-W16", 2, second.getId(), 110.0d, 8, 18, 28); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# period_key 포맷을 산출하는 로직 확인
rg -nP --type=java -C3 '"\d{4}-W|W%02d|WeekFields|IsoFields\.WEEK_BASED_YEAR|WEEK_OF_WEEK_BASED_YEAR'
rg -nP --type=java -C3 'period_?key|periodKey' -g '!**/test/**'
ast-grep --pattern $'class RankingPeriod {
$$$
}'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 35980
🏁 Script executed:
cd apps/commerce-api && find . -name "RankingV1ApiE2ETest.java" -type f && wc -l src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 222
🏁 Script executed:
sed -n '140,180p' apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2072
🏁 Script executed:
# 월별 테스트 부분도 확인
sed -n '160,220p' apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2755
🏁 Script executed:
# RankingPeriodKeyFactory의 월별 포맷 확인
sed -n '19,25p' apps/commerce-api/src/main/java/com/loopers/support/util/RankingPeriodKeyFactory.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 342
🏁 Script executed:
# 월별 포맷 단위 테스트 확인
rg -nP --type=java -A5 'toMonthlyKey|2026-04' apps/commerce-batch/src/test/java/com/loopers/domain/ranking/RankingPeriodKeyFactoryTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 207
🏁 Script executed:
# 테스트의 insertWeeklyRanking/insertMonthlyRanking 메서드 구현 확인
sed -n '250,308p' apps/commerce-api/src/test/java/com/loopers/interfaces/api/RankingV1ApiE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2924
🏁 Script executed:
# 프로덕션 조회 경로에서 RankingPeriodKeyFactory 사용 여부 재확인
rg -nP --type=java 'RankingPeriodKeyFactory\.(toWeeklyKey|toMonthlyKey)' apps/commerce-api/src/main/java/ --no-headingRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 418
테스트 데이터의 period_key를 RankingPeriodKeyFactory를 사용하여 계산하도록 변경해야 한다.
현재 테스트는 insertWeeklyRanking("2026-W16", ...)과 insertMonthlyRanking("2026-04", ...)로 문자열을 직접 지정하고 있으나, 프로덕션 조회 경로(WeeklyRankingService.getRankings(), MonthlyRankingService.getRankings())는 모두 RankingPeriodKeyFactory를 통해 period_key를 산출한다. 현재 하드코딩된 값은 RankingPeriodKeyFactoryTest에서 검증된 올바른 포맷이지만, factory 로직이 변경될 때 테스트 데이터가 동기화되지 않으면 INSERT는 성공하고 데이터도 존재하지만 조회 조건이 맞지 않아 결과가 0건이 되는 조용한 실패(silent failure)가 발생할 수 있다.
test data setup 시 LocalDate.parse("20260415", DateTimeFormatter.BASIC_ISO_DATE)로부터 RankingPeriodKeyFactory.toWeeklyKey() 및 .toMonthlyKey()를 호출하여 period_key를 동적으로 산출하도록 변경하면, 테스트 데이터 일관성이 자동으로 유지되고 프로덕션 경로의 안정성을 명확히 보장할 수 있다. 또는 최소한 test 내 단언으로 "2026-W16".equals(RankingPeriodKeyFactory.toWeeklyKey(date))를 검증하는 방식으로 포맷 일관성을 명시적으로 확인해야 한다.
🤖 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 145 - 146, 현재 테스트가 insertWeeklyRanking("2026-W16", ...)와
insertMonthlyRanking("2026-04", ...)로 period_key를 하드코딩하고 있어
RankingPeriodKeyFactory의 형식 변경 시 조회와 불일치하는 조용한 실패가 발생할 수 있습니다; 수정 방법은 테스트 데이터 생성
시 LocalDate date = LocalDate.parse("20260415",
DateTimeFormatter.BASIC_ISO_DATE)로 날짜를 만들고
RankingPeriodKeyFactory.toWeeklyKey(date) 및
RankingPeriodKeyFactory.toMonthlyKey(date)로 period_key를 계산해
insertWeeklyRanking(...)과 insertMonthlyRanking(...)에 전달하거나, 적어도 테스트에
"2026-W16".equals(RankingPeriodKeyFactory.toWeeklyKey(date)) 같은 단언을 추가해 포맷 일관성을
보장하도록 변경하세요.
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Entity | ||
| @Table(name = "mv_product_rank_weekly") | ||
| @IdClass(WeeklyRankingEntity.WeeklyRankingId.class) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Hibernate 6.4 officially support Java record as @IdClass target, and are there known issues with missing no-arg constructor?
💡 Result:
No, Hibernate 6.4 does not officially support Java records as @IdClass targets. This feature was introduced in Hibernate 6.5. In Hibernate 6.4 and earlier versions, @IdClass implementations must follow Jakarta Persistence requirements: they need a no-arg constructor, getters/setters for all ID fields, and must implement Serializable, equals, and hashCode. Java records lack a no-arg constructor (they only have a canonical constructor with all components) and lack setters/getters, so they do not work and will fail at runtime or during mapping. Support for records as @IdClass was added in Hibernate 6.5, where Hibernate uses a special EmbeddableInstantiator to handle record instantiation without relying on a no-arg constructor. This is confirmed in official Hibernate release notes for 6.5.0.CR1 and 6.5.0.Final, as well as Hibernate JIRA issue HHH-15493 (linked in discussions), and secondary sources like Thorben Janssen's blog stating "Older versions, unfortunately, don’t support records as IdClasses" and specifying ">= 6.5.0". Hibernate 6.4 release notes, migration guides, and user guide (6.4.10.Final) do not mention this feature. The "known issue" with the no-arg constructor is not a bug but the expected behavior due to lack of support: attempts to use records will fail because Hibernate expects to instantiate the @IdClass via reflection using the no-arg constructor, which records do not provide.
Citations:
- 1: support java record class as @IdClass in hibernate quarkusio/quarkus#36641
- 2: https://docs.hibernate.org/orm/6.4/userguide/html_single/
- 3: https://thorben-janssen.com/idclass-record
- 4: https://in.relation.to/2024/03/20/orm-65cr1/
- 5: https://in.relation.to/2024/04/25/orm-650/
- 6: https://hibernate.org/orm/releases/6.4/
- 7: https://docs.hibernate.org/orm/6.4/migration-guide/
@IdClass 에 Java record 를 사용하는 것은 JPA 스펙상 및 버전 호환성 리스크가 있다.
JPA 스펙(§2.4)은 @IdClass 에 대해 public no-arg 생성자, equals/hashCode, Serializable 구현을 요구한다. Record 는 equals/hashCode/Serializable 은 만족하지만 no-arg 생성자가 존재하지 않는다. 현재 Hibernate 6.4 환경에서는 이 제약으로 인해 런타임 오류가 발생하거나 매핑에 실패할 수 있다. Record 를 @IdClass 로 지원하는 것은 Hibernate 6.5 이상에서만 공식 지원되며, 하위 버전이나 다른 JPA 구현체 전환 시 깨질 여지가 있다. 배치에서 주간 MV 적재가 실패하면 랭킹 조회 전 구간이 정지되므로 운영 리스크가 크다.
- 수정안:
WeeklyRankingId를 일반public static class로 바꾸고@NoArgsConstructor/@EqualsAndHashCode/Serializable을 명시적으로 구현하거나,@IdClass대신@Embeddable+@EmbeddedId패턴으로 전환한다. - 추가 테스트:
WeeklyRankingJpaRepository.saveAll→findByPeriodKeyOrderByRankNoAsc왕복 테스트를 Testcontainers MySQL 환경에서 보강해 ID 바인딩/선택 쿼리 파라미터 바인딩까지 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/infrastructure/ranking/entity/WeeklyRankingEntity.java`
at line 20, 현재 WeeklyRankingEntity의 IdClass인 WeeklyRankingId를 Java record로 사용하면
JPA 요구사항(no-arg constructor 등)을 만족하지 못해 하위 Hibernate나 다른 JPA 구현에서 런타임 오류가 발생할 수
있으니 WeeklyRankingId를 record에서 public static class로 바꿔 기본 생성자(no-arg)를 제공하고
Serializable을 구현하며 equals/hashCode를 명시(혹은 Lombok의 `@NoArgsConstructor` +
`@EqualsAndHashCode` 사용)하도록 수정하거나, 대안으로 `@IdClass` 대신 `@Embeddable` + `@EmbeddedId` 패턴으로
전환하세요; 변경 후 WeeklyRankingJpaRepository.saveAll → findByPeriodKeyOrderByRankNoAsc
왕복 테스트를 Testcontainers MySQL 환경에서 추가해 ID 바인딩과 선택 쿼리 파라미터 바인딩이 올바른지 검증하세요.
| repository.deleteByPeriodKey(periodKey); | ||
| if (!rows.isEmpty()) { | ||
| repository.saveAll(rows.stream() | ||
| .map(row -> MonthlyRankingEntity.of(periodKey, row)) | ||
| .toList()); | ||
| } | ||
| return RepeatStatus.FINISHED; |
There was a problem hiding this comment.
delete-then-insert 구조의 원자성과 0건 시나리오 보호가 필요하다.
운영 관점에서 다음 리스크가 있다.
- 집계 결과가 0건(예:
product_metrics_daily가 해당 기간에 비어 있거나 업스트림 지연)이면deleteByPeriodKey만 실행되고 재적재가 스킵되어 MV가 공백 상태로 덮어씌워진다. 직전 성공분까지 함께 사라져 API가 해당 기간을 빈 랭킹으로 응답한다. - 동일
periodKey에 대해 재실행이 겹치면 DELETE + INSERT가 충돌해 데드락/공백 구간이 발생할 수 있다.
🔧 제안 수정안
- repository.deleteByPeriodKey(periodKey);
- if (!rows.isEmpty()) {
- repository.saveAll(rows.stream()
- .map(row -> MonthlyRankingEntity.of(periodKey, row))
- .toList());
- }
+ if (rows.isEmpty()) {
+ log.warn("monthly ranking aggregation returned 0 rows for periodKey={}. Skip replace to preserve previous snapshot.", periodKey);
+ return RepeatStatus.FINISHED;
+ }
+ repository.deleteByPeriodKey(periodKey);
+ repository.saveAll(rows.stream()
+ .map(row -> MonthlyRankingEntity.of(periodKey, row))
+ .toList());추가 테스트: (1) product_metrics_daily가 비어 있을 때 기존 MV가 유지되는 시나리오, (2) 동일 periodKey로 2회 연속 실행 시 최종 상태가 멱등함을 검증하는 통합 테스트를 추가하기를 권장한다. 가능하다면 RefreshWeeklyRankingTasklet에도 동일 가드를 적용해야 한다.
📝 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.
| repository.deleteByPeriodKey(periodKey); | |
| if (!rows.isEmpty()) { | |
| repository.saveAll(rows.stream() | |
| .map(row -> MonthlyRankingEntity.of(periodKey, row)) | |
| .toList()); | |
| } | |
| return RepeatStatus.FINISHED; | |
| if (rows.isEmpty()) { | |
| log.warn("monthly ranking aggregation returned 0 rows for periodKey={}. Skip replace to preserve previous snapshot.", periodKey); | |
| return RepeatStatus.FINISHED; | |
| } | |
| repository.deleteByPeriodKey(periodKey); | |
| repository.saveAll(rows.stream() | |
| .map(row -> MonthlyRankingEntity.of(periodKey, row)) | |
| .toList()); | |
| return RepeatStatus.FINISHED; |
🤖 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/tasklet/RefreshMonthlyRankingTasklet.java`
around lines 66 - 72, The current delete-then-insert in
RefreshMonthlyRankingTasklet (repository.deleteByPeriodKey ->
repository.saveAll) can wipe existing MV when rows is empty and races when
re-run; change the execute logic to be transactional and guard deletes: if
rows.isEmpty() return RepeatStatus.FINISHED without calling
repository.deleteByPeriodKey, otherwise perform repository.deleteByPeriodKey and
repository.saveAll inside a single `@Transactional` boundary (or use a DB
temp-table swap/upsert) and add a serialization mechanism for the same periodKey
(DB row lock or advisory lock) to prevent concurrent delete/insert races; update
or add integration tests to cover empty-source and idempotent double-run
scenarios and consider applying the same guard to RefreshWeeklyRankingTasklet.
| repository.deleteByPeriodKey(periodKey); | ||
| if (!rows.isEmpty()) { | ||
| repository.saveAll(rows.stream() | ||
| .map(row -> WeeklyRankingEntity.of(periodKey, row)) | ||
| .toList()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# WeeklyRankingJpaRepository.deleteByPeriodKey 구현 확인 (bulk delete 여부, `@Modifying/clearAutomatically`)
fd -t f 'WeeklyRankingJpaRepository' apps/commerce-batch --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 566
🏁 Script executed:
# 1. RefreshWeeklyRankingTasklet 전체 코드 확인 (트랜잭션 범위, 예외 처리)
fd -t f 'RefreshWeeklyRankingTasklet' apps/commerce-batch --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3187
🏁 Script executed:
# 2. Job 설정 확인 (`@Bean`, Step 설정, 트랜잭션)
rg -t java 'RefreshWeeklyRankingTasklet|WeeklyRankingJob' apps/commerce-batch --context 5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 12500
🏁 Script executed:
# 3. 동일 파라미터로 Job 실행 제어에 관한 설정 확인
rg -t java 'RunIdIncrementer|JobParameters|targetDate' apps/commerce-batch --context 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 12183
🏁 Script executed:
# 4. 동시성 관련 테스트 또는 Lock 관련 설정 확인
rg -t java -i 'lock|concurrent|同時|duplicate' apps/commerce-batch/src/test --context 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# WeeklyRankingJobConfig의 step 정의 전체 확인
cat apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2255
🏁 Script executed:
# `@Transactional이` Step이나 다른 레벨에 적용되었는지 확인
rg -t java '@Transactional' apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/ --context 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 동시 실행 테스트 여부 확인
cat apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3850
🏁 Script executed:
# `@Modifying` 어노테이션이 다른 repository에서는 어떻게 사용되는지 확인
rg -t java '@Modifying' apps/commerce-batch --context 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 확인: 다른 Repository에서 `@Modifying` 사용 패턴
rg -t java -B2 'void delete|int delete|long delete' apps/commerce-batch --type-listRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 474
🏁 Script executed:
# `@Modifying` 사용 사례 확인
rg -t java '@Modifying' apps/commerce-batchRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# MonthlyRankingTasklet도 같은 패턴인지 확인 (동일한 버그가 있는지)
fd -t f 'RefreshMonthlyRankingTasklet' apps/commerce-batch --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3197
🏁 Script executed:
# MonthlyRankingJpaRepository도 같은 문제가 있는지 확인
fd -t f 'MonthlyRankingJpaRepository' apps/commerce-batch --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 572
WeeklyRankingJpaRepository.deleteByPeriodKey() 메서드에 @Modifying 어노테이션이 누락되어 있다.
Spring Data JPA는 bulk delete/update 연산에 @Modifying 어노테이션을 필수로 요구한다. 현재 구현에서는 메서드가 제대로 실행되지 않거나 조용히 실패할 수 있다. 동일한 문제가 MonthlyRankingJpaRepository.deleteByPeriodKey()에도 존재한다.
추가로 다음 동시성 문제를 운영 단계에서 고려하여야 한다:
-
동시 기동 시 경합 가능성: RunIdIncrementer는 동일한 Job instance 재실행만 방지하며, 같은 targetDate로 여러 Job이 동시에 기동되면 deleteByPeriodKey 간 경합이 발생한다. 운영에서 파라미터를 다르게 입력하여 중복 실행할 여지가 있다.
-
예외 발생 시 롤백: 현재 Step 레벨 트랜잭션 구성(StepBuilder.tasklet(..., transactionManager))으로는 saveAll 실패 시 deleteByPeriodKey도 함께 롤백되어 이전 데이터가 유지되는 것이 정상이다.
수정 사항:
- WeeklyRankingJpaRepository와 MonthlyRankingJpaRepository의 deleteByPeriodKey 메서드에
@Modifying어노테이션을 추가한다. - 동시 실행 방지를 위해 Job 파라미터(targetDate)를 Job instance identifier로 강제하거나, Step 레벨에서 advisory lock(예: MySQL GET_LOCK)을 적용한다.
추가 테스트: 동일 targetDate로 동시에 여러 번 기동하는 통합 테스트를 작성하여 최종 MV 행 수가 TOP_N으로 수렴하고 중복 없이 유지되는지 검증한다.
🤖 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/tasklet/RefreshWeeklyRankingTasklet.java`
around lines 66 - 71, Add the missing `@Modifying` annotation to the repository
bulk-delete methods: annotate WeeklyRankingJpaRepository.deleteByPeriodKey(...)
and MonthlyRankingJpaRepository.deleteByPeriodKey(...) with `@Modifying` (and
ensure `@Transactional` or appropriate transaction boundary is present) so Spring
Data JPA executes the bulk delete correctly; additionally, to prevent concurrent
runs for the same targetDate, enforce the Job instance identifier to include
targetDate (instead of relying solely on RunIdIncrementer) or add an advisory
lock around the Step (e.g., use MySQL GET_LOCK in the Step/Tasklet or wrap Step
execution with a lock utility referenced from the StepBuilder.tasklet(...,
transactionManager)), and keep the current step-level transaction so a saveAll
failure rolls back the prior delete.
| import com.loopers.infrastructure.metrics.entity.ProductMetricsEntity; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
| public interface ProductMetricsRepository { | ||
|
|
||
| Optional<ProductMetricsEntity> findById(Long productId); | ||
|
|
||
| ProductMetricsEntity save(ProductMetricsEntity metrics); | ||
|
|
||
| List<ProductMetricsEntity> findByUpdatedAtAfter(LocalDateTime since); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
도메인 계층이 인프라 엔티티에 직접 의존한다.
com.loopers.domain.metrics.repository.ProductMetricsRepository가 com.loopers.infrastructure.metrics.entity.ProductMetricsEntity를 반환/수신 타입으로 사용하고 있다. 도메인 → 인프라 방향의 의존이 생겨 레이어드 아키텍처의 의존 방향이 역전되며, 향후 JPA를 다른 저장 기술로 교체하거나 엔티티 구조를 변경할 때 도메인까지 영향을 받는다. 운영 관점에서는 당장 장애 요인은 아니지만, 모듈 간 결합도를 높여 리팩토링/테스트 격리에 비용을 발생시킨다.
도메인 모델(또는 값 객체)을 별도로 정의해 repository 인터페이스의 입출력으로 사용하고, 인프라 구현체(ProductMetricsRepositoryImpl)에서 엔티티 ↔ 도메인 변환을 수행하는 방식을 권장한다. 적용 시 MetricsService가 도메인 모델만 다루도록 함께 조정해야 하며, 변환 경로를 검증하는 매퍼 단위 테스트 추가가 필요하다.
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/main/java/com/loopers/domain/metrics/repository/ProductMetricsRepository.java`
around lines 3 - 15, Change ProductMetricsRepository to depend on a domain model
instead of the infrastructure entity: define a domain class (e.g.,
ProductMetrics or ProductMetricsDto) and update the ProductMetricsRepository
interface signatures (findById, save, findByUpdatedAtAfter) to use that domain
type; implement conversion logic in the infra implementation
(ProductMetricsRepositoryImpl) to map between ProductMetricsEntity and the new
domain model; update callers such as MetricsService to use the domain model
only; add unit tests for the mapper that converts ProductMetricsEntity <->
ProductMetrics to ensure correctness.
| @Transactional | ||
| public void incrementViewCount(Long productId) { | ||
| ProductMetricsEntity metrics = findOrCreate(productId); | ||
| if (version <= metrics.getVersion()) return; | ||
| metrics.incrementViewCount(); | ||
| metrics.updateVersion(version); | ||
| metricsRepository.save(metrics); | ||
| dailyMetricsRepository.incrementViewCount(LocalDate.now(), productId, VIEW_WEIGHT); | ||
| } | ||
|
|
||
| public void incrementLikeCount(Long productId, long version) { | ||
| @Transactional | ||
| public void incrementLikeCount(Long productId) { | ||
| ProductMetricsEntity metrics = findOrCreate(productId); | ||
| if (version <= metrics.getVersion()) return; | ||
| metrics.incrementLikeCount(); | ||
| metrics.updateVersion(version); | ||
| metricsRepository.save(metrics); | ||
| dailyMetricsRepository.incrementLikeCount(LocalDate.now(), productId, LIKE_WEIGHT); | ||
| } | ||
|
|
||
| public void decrementLikeCount(Long productId, long version) { | ||
| @Transactional | ||
| public void decrementLikeCount(Long productId) { | ||
| ProductMetricsEntity metrics = findOrCreate(productId); | ||
| if (version <= metrics.getVersion()) return; | ||
| metrics.decrementLikeCount(); | ||
| metrics.updateVersion(version); | ||
| metricsRepository.save(metrics); | ||
| dailyMetricsRepository.decrementLikeCount(LocalDate.now(), productId, -LIKE_WEIGHT); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void incrementOrderCount(Long productId) { | ||
| ProductMetricsEntity metrics = findOrCreate(productId); | ||
| metrics.incrementOrderCount(); | ||
| metricsRepository.save(metrics); | ||
| dailyMetricsRepository.incrementOrderCount(LocalDate.now(), productId, ORDER_WEIGHT); | ||
| } |
There was a problem hiding this comment.
version 제거로 멱등성이 오히려 약화되었다.
PR 설명은 "version 체크 제거 → 멱등성으로 대체"라고 기술하고 있으나, 현재 구현은 호출될 때마다 incrementXxxCount()와 dailyMetricsRepository.incrementXxxCount()를 무조건 증가시킨다. Kafka consumer는 at-least-once이므로 동일 이벤트 재전달 시 product_metrics 및 product_metrics_daily가 중복 가산된다. 기존 version 가드는 최소한 같은 버전 재처리를 탐지할 수 있었던 반면, 현재는 중복 처리 방지 장치가 전혀 없다.
학습 메모(approximate ranking, at-least-once 수용)에 따라 랭킹 자체의 근사성은 허용되지만, product_metrics_daily는 weekly/monthly 배치의 "기준 데이터"로 사용되므로 누적 중복이 월간 집계까지 전파된다. 다음 중 하나를 선택하는 것이 바람직하다.
- 이벤트
eventId기반 dedup 테이블/Redis Set(SETNX+ TTL)로 소비 레벨에서 멱등 처리 ProductMetricsEntity의version을 이벤트 순번으로 활용한 단조증가 가드 유지
추가 테스트: 동일 이벤트를 2회 소비했을 때 view_count/score가 1회치만 증가하는지 검증.
As per coding guidelines (**/*Service*.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/domain/metrics/service/MetricsService.java`
around lines 26 - 56, The current removal of version checks makes
incrementViewCount/incrementLikeCount/decrementLikeCount/incrementOrderCount
idempotency broken because each call always updates ProductMetricsEntity and
dailyMetricsRepository, causing duplicate increments on at-least-once Kafka
redeliveries; fix by implementing one of two options: (A) add consumer-level
dedup using eventId (e.g., check a dedup store like Redis SETNX with TTL before
any work and skip if already seen) and perform the SETNX check in the entry of
each affected methods
(incrementViewCount/incrementLikeCount/decrementLikeCount/incrementOrderCount)
before calling ProductMetricsEntity.* and dailyMetricsRepository.*, or (B)
restore a monotonic version guard on ProductMetricsEntity (store an event
sequence/version on ProductMetricsEntity, compare incoming event sequence to
existing version and only apply increments and persist if incoming > stored
version, updating the version atomically with the counters); also add
unit/integration tests asserting that processing the same event twice does not
change view_count/score in product_metrics and product_metrics_daily.
| @Column(nullable = false) | ||
| private LocalDateTime createdAt; | ||
|
|
||
| @Column(nullable = false) | ||
| private LocalDateTime updatedAt; |
There was a problem hiding this comment.
createdAt/updatedAt 자동 설정 장치가 보이지 않는다.
두 컬럼 모두 nullable = false이지만 @PrePersist/@PreUpdate 또는 JPA Auditing(@CreatedDate/@LastModifiedDate) 설정이 없어, 호출측에서 세팅을 누락하면 PropertyValueException으로 insert가 실패한다. 메트릭 적재 경로는 Kafka consume의 핫패스이므로, 한 곳에서라도 세팅을 빠뜨리면 컨슈머 락업/지속적인 재시도로 번질 수 있다.
생성/수정 시 자동으로 값이 채워지도록 @EntityListeners(AuditingEntityListener.class) + @CreatedDate/@LastModifiedDate를 적용하거나, 최소한 @PrePersist 훅에서 updatedAt = createdAt = LocalDateTime.now()를 보장해야 한다. 동시에 엔티티를 신규 생성하는 경로에 대한 persist 테스트에 createdAt/updatedAt 비-null 검증을 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/entity/ProductMetricsDailyEntity.java`
around lines 43 - 47, ProductMetricsDailyEntity currently declares non-nullable
fields createdAt and updatedAt but lacks automatic population; add automatic
timestamping by either annotating the entity with
`@EntityListeners`(AuditingEntityListener.class) and marking createdAt with
`@CreatedDate` and updatedAt with `@LastModifiedDate` (ensuring JPA auditing is
enabled), or implement `@PrePersist` and `@PreUpdate` lifecycle methods inside
ProductMetricsDailyEntity to set createdAt/updatedAt = LocalDateTime.now() on
persist/update; also update or add the persistence test for the entity creation
path to assert createdAt and updatedAt are non-null after persist to prevent
consumer hot-path failures.
| run_mysql() { | ||
| mysql -h "$MYSQL_HOST" -P "$MYSQL_PORT" -u "$MYSQL_USER" -p"$MYSQL_PWD" "$MYSQL_DB" "$@" | ||
| } |
There was a problem hiding this comment.
-p 플래그에 직접 비밀번호를 넘기면 프로세스 리스트로 유출된다.
mysql -p"$MYSQL_PWD"는 ps aux / auditd / 컨테이너 로그에 비밀번호가 그대로 노출된다. 벤치마크 스크립트가 CI 러너 또는 개발자 로컬에서 실행될 때 자격증명이 누설될 수 있다. MySQL 공식 권장 방식인 환경변수 MYSQL_PWD 주입 또는 --defaults-extra-file=<tmp.cnf> 기반으로 전환해야 한다.
🔒 제안 수정안
run_mysql() {
- mysql -h "$MYSQL_HOST" -P "$MYSQL_PORT" -u "$MYSQL_USER" -p"$MYSQL_PWD" "$MYSQL_DB" "$@"
+ MYSQL_PWD="$MYSQL_PWD" mysql -h "$MYSQL_HOST" -P "$MYSQL_PORT" -u "$MYSQL_USER" "$MYSQL_DB" "$@"
}추가로, 비밀번호가 기본값(application)으로 하드코딩되어 있어 운영 환경 자격증명 재사용을 강제하게 되므로, 기본값은 제거하고 미설정 시 스크립트가 즉시 실패하도록 가드(: "${MYSQL_PWD:?MYSQL_PWD is required}")를 두기를 권장한다.
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 32-34: run_mysql references arguments, but none are ever passed.
(SC2120)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/batch/run-ranking-batch-benchmark.sh` around lines 32 - 34, The
run_mysql helper leaks the password via the -p"$MYSQL_PWD" argument; change
run_mysql to avoid passing the password on the command line (use the MYSQL_PWD
environment variable or create a temporary --defaults-extra-file with
credentials and pass that file to mysql) and update the script to fail fast if
MYSQL_PWD is not set (remove any hardcoded default and add a guard like :
"${MYSQL_PWD:?MYSQL_PWD is required}" early in the script) so credentials are
never exposed in process lists or logs; locate the run_mysql function and the
place where MYSQL_PWD might be defaulted to implement these changes.
1 participant
📌 Summary
product_metrics_daily를 기준 데이터로 두고, Spring Batch가 주간/월간 TOP 100 랭킹을 계산해 MV 테이블에 적재하도록 구현합니다. API는period파라미터로 일간/주간/월간 랭킹을 동일한 엔드포인트에서 제공합니다.mv_product_rank_weekly,mv_product_rank_monthly에서 조회하고, 일간 랭킹은 기존 Redis 기반 흐름을 유지했습니다.🧭 Context & Decision
문제 정의
GROUP BY,SUM,ORDER BY로 계산하면 데이터가 늘어날수록 조회 부하가 커짐period=daily/weekly/monthly로 조회 가능targetDate기준 집계score를 주간/월간에서 합산핵심 결정 요약
product_metrics_dailyRefreshWeeklyRankingTasklet,RefreshMonthlyRankingTaskletmv_product_rank_weekly,mv_product_rank_monthlyRefresh...RankingTaskletsum(score)period파라미터RankingV1Controller,RankingFacadeRound 9 결함 개선
이번 작업을 진행하면서 지난 라운드에서 남아 있던 랭킹/이벤트 처리 관련 결함도 함께 정리했습니다.
MetricsService의 version 공유findAllByIds흐름으로 변경@Transactional(readOnly = true)적용각 결정 상세
1. 주간/월간 기준 데이터: raw event vs product_metrics_daily
의도
주간/월간 랭킹을 만들 때 어떤 데이터를 기준으로 다시 집계할지 결정해야 했습니다.
대안 비교
product_metrics_daily재집계최종 근거
이론적으로 raw event가 가장 원본에 가깝지만, 랭킹 배치에서 필요한 단위는 결국 날짜별 상품 메트릭입니다. 이번 과제에서는
product_metrics_daily를 랭킹 배치의 기준 데이터로 두고, 주간/월간 배치는 이 데이터를 다시 합산하도록 했습니다.2. daily는 Redis, weekly/monthly는 MV 테이블
의도
기간별 랭킹을 모두 같은 저장소에 둘지, 데이터 특성에 따라 나눌지 결정해야 했습니다.
대안 비교
최종 근거
일간 랭킹은 실시간성이 중요하므로 Redis ZSET을 유지했습니다. 반면 주간/월간 랭킹은 실시간성보다 정확성과 재처리가 중요하다고 봤습니다. 과제에서도 주간/월간 결과를 조회 전용 테이블에 저장하라고 되어 있으므로
mv_product_rank_weekly,mv_product_rank_monthly를 기준으로 구현했습니다.주간/월간 결과도 Redis에 캐싱할 수는 있지만, 이번 작업에서는 Batch + MV 테이블 구조를 먼저 완성하는 것을 우선했습니다.
3. Chunk 대신 Tasklet을 선택한 이유
의도
Spring Batch 구현을 chunk-oriented로 할지, tasklet으로 할지 결정해야 했습니다.
대안 비교
최종 근거
이번 배치는 원본 데이터를 한 건씩 가공하는 작업이라기보다, 특정 기간의 집계 결과를 구해서 해당 기간의 MV 스냅샷을 교체하는 작업입니다. 최종 결과도 TOP 100으로 제한되어 있어 chunk의 장점이 크게 드러나지 않는다고 봤습니다.
그래서 tasklet 안에서 기간 계산, 집계 조회, 기존 기간 데이터 삭제, 새 랭킹 저장을 한 흐름으로 처리했습니다.
4. score를 다시 계산하지 않고 sum(score)를 사용
의도
주간/월간 batch에서 score를 다시 계산할지, 일간 score를 합산할지 결정해야 했습니다.
대안 비교
sum(score)최종 근거
product_metrics_daily.score는 이벤트 적재 시점에 조회 0.1, 좋아요 0.2, 주문 0.7 가중치로 누적됩니다. 주간/월간 배치는 이 값을 기간 단위로 합산하면 되므로sum(score)를 사용했습니다.이 방식은 일간 랭킹과 주간/월간 랭킹이 같은 기준을 공유한다는 장점이 있습니다. 대신 가중치가 바뀌면 과거 일간 score를 어떻게 재계산할지에 대한 backfill 정책이 필요합니다. 이 부분은 추후 개선 과제로 남겼습니다.
🏗️ Design Overview
변경 범위
product_metrics_daily일간 메트릭 누적/api/v1/rankings에period파라미터 추가, 일간/주간/월간 분기주요 컴포넌트 책임
MetricsServiceProductMetricsDailyJpaRepositoryINSERT ... ON DUPLICATE KEY UPDATE로 일간 메트릭 원자적 누적WeeklyRankingJobConfigMonthlyRankingJobConfigRefreshWeeklyRankingTaskletRefreshMonthlyRankingTaskletWeeklyRankingRepositoryMonthlyRankingRepositoryRankingFacadeRankingQuery🔁 Flow Diagram
Main Flow: 이벤트 수집 → 일간 메트릭 → 주간/월간 랭킹
sequenceDiagram autonumber participant U as User participant API as commerce-api participant K as Kafka participant S as commerce-streamer participant DB as MySQL participant B as commerce-batch participant R as Redis U->>API: 상품 조회/좋아요/주문 API->>K: catalog/order event 발행 K->>S: Consumer 이벤트 소비 S->>DB: product_metrics_daily upsert S->>DB: product_metrics 누적 갱신 Note over S,R: 일간 랭킹 동기화 S->>R: Redis ZSET score 갱신 Note over B,DB: 주간/월간 Batch 실행 B->>DB: product_metrics_daily 기간 집계 B->>DB: mv_product_rank_weekly/monthly 교체 U->>API: GET /api/v1/rankings?period=weekly API->>DB: weekly MV 조회 API-->>U: 주간 랭킹 응답API Query Flow
flowchart TD A[GET /api/v1/rankings] --> B[RankingQuery 검증] B --> C{period} C -->|daily| D[Redis ZSET 조회] C -->|weekly| E[mv_product_rank_weekly 조회] C -->|monthly| F[mv_product_rank_monthly 조회] D --> G[상품 정보 조합] E --> G F --> G G --> H[랭킹 응답]✅ Test
Checklist
targetDate파라미터 기반으로 동작하도록 구현mv_product_rank_weekly,mv_product_rank_monthly조회 전용 테이블 설계 및 적재period,page,size입력 검증검증한 내용
product_metrics_daily가 날짜/상품 기준으로 누적되는지 검증period=daily/weekly/monthly조회와 잘못된 입력 검증실행한 테스트
리뷰 포인트
sum(score)재사용: streamer가 일간 score를 계산하고 batch는 이를 합산하도록 했습니다. 가중치 변경 시 backfill 정책이 필요하다는 단점이 있는데, score 공식의 단일 책임과 재처리 유연성 사이에서 더 나은 기준이 있을지 궁금합니다.product_metrics동시성 보강: 이번 배치 기준 데이터인product_metrics_daily는 atomic upsert로 처리했습니다. 기존 누적 테이블인product_metrics도 같은 방식으로 맞추는 것이 실무적으로 더 자연스러운지 검토 의견을 받고 싶습니다.정량 검증
로컬 Docker MySQL/Redis 기준으로
product_metrics_daily100,000건을 seed한 뒤 배치 실행 시간과 랭킹 API 응답 시간을 측정했습니다.Batch 실행 시간
weeklyRankingJobmonthlyRankingJob측정 조건:
targetDate=20260415product_metrics_daily100,000 rowsmv_product_rank_weekly,mv_product_rank_monthly각각 TOP 100 적재 확인Ranking API k6
daily,weekly,monthly랭킹 API를 동시에 호출하는 k6 시나리오로 측정했습니다. weekly/monthly는 실제 랭킹 아이템 payload가 반환되도록 TOP 100 상품 데이터를 함께 seed했습니다.실행 명령:
로컬 단일 장비에서 수행한 간이 측정이므로 운영 성능으로 일반화할 수는 없습니다. 다만 이번 PR의 목표였던 “주간/월간 랭킹을 요청 시점 계산이 아니라 MV 조회로 제공한다”는 방향이 응답 시간 측면에서도 문제가 없는지 확인하는 용도로 사용했습니다.
주간·월간 랭킹 Batch 시스템 구현
변경 목적: 기존 일간 랭킹(Redis ZSET)만 제공하던 시스템에 주간/월간 랭킹을 추가하고, 단일 API(
/api/v1/rankings?period=daily|weekly|monthly)로 통합하여 매 요청 시 기간 집계로 인한 부하 제거.핵심 변경점:
period파라미터 추가(기본값: daily) 및RankingQueryDTO 도입으로 요청 관리 계층화product_metrics_daily테이블을 기준 데이터로 하여 매 이벤트마다 일간 메트릭 누적(Kafka 컨슈머)mv_product_rank_weekly,mv_product_rank_monthly)에 적재MetricsService의 version 기반 낙관적 잠금 제거 후@Transactional로 변경, daily 메트릭도 함께 업데이트리스크/주의사항:
findAllByIds()사용으로 조회 가능한 상품만 반환(NOT_FOUND 예외 무음) → 삭제된 상품은 랭킹 결과에서 누락될 수 있음테스트/검증: E2E 테스트(
RankingV1ApiE2ETest,WeeklyRankingJobE2ETest,MonthlyRankingJobE2ETest)로 기간별 조회, TOP 100 정렬, 빈 결과 처리 검증; k6 로드 테스트 스크립트로 API 응답성 검증; 로컬 벤치마크(100k rows)에서 주간/월간 배치 약 3.54초 보고.