[VOLUME-10] Spring Batch 를 이용한 주간, 월간 랭킹 구현#395
Open
hyejin0810 wants to merge 2 commits intoLoopers-dev-lab:hyejin0810from
Open
[VOLUME-10] Spring Batch 를 이용한 주간, 월간 랭킹 구현#395hyejin0810 wants to merge 2 commits intoLoopers-dev-lab:hyejin0810from
hyejin0810 wants to merge 2 commits intoLoopers-dev-lab:hyejin0810from
Conversation
- modules/jpa에 ProductRankWeekly, ProductRankMonthly MV 엔티티 추가 - commerce-batch에 weeklyRankingJob, monthlyRankingJob 구현 - 3단계 구성: Clear(Tasklet) → Aggregate(Chunk) → Rank부여(Tasklet) - 스코어: like×0.2 + order×0.7 + view×0.1 - TOP 100만 MV 테이블에 유지 - commerce-api Ranking API에 type=daily/weekly/monthly 파라미터 추가 - product_metrics 테스트 데이터 시드 스크립트 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ProductRankWeekly/Monthly에 assignRank() 메서드 추가 - RankingFacadeTest에 WEEKLY/MONTHLY 랭킹 조회 및 개수 조회 케이스 6개 추가 - WeeklyRankingJobE2ETest / MonthlyRankingJobE2ETest 신규 작성 - k6/06_ranking.js에 주간(E)/월간(F) 랭킹 부하 시나리오 추가 - CouponFacadeTest, OrderFacadeTest, OrderConcurrencyTest 컴파일 오류 수정 (변경된 Facade 생성자 시그니처 및 createOrder 파라미터에 맞게 업데이트) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough주간 및 월간 랭킹 기능을 추가하기 위해 Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant RankingV1Controller
participant RankingFacade
participant RankingCacheService
participant ProductRankWeeklyJpaRepository
participant ProductRankMonthlyJpaRepository
participant ProductService
participant BrandService
Client->>RankingV1Controller: GET /api/v1/rankings?type=weekly&page=0&size=10
activate RankingV1Controller
RankingV1Controller->>RankingV1Controller: type 파라미터를 RankingType으로 변환
RankingV1Controller->>RankingFacade: getRankings(date, page, size, RankingType.WEEKLY)
deactivate RankingV1Controller
activate RankingFacade
alt RankingType.WEEKLY
RankingFacade->>ProductRankWeeklyJpaRepository: findByYearWeekOrderByRankPosition(yearWeek)
activate ProductRankWeeklyJpaRepository
ProductRankWeeklyJpaRepository-->>RankingFacade: List<ProductRankWeekly> 반환
deactivate ProductRankWeeklyJpaRepository
RankingFacade->>RankingFacade: 페이지네이션 처리 (skip/limit)
RankingFacade->>ProductService: 상품 ID 목록으로 상품 정보 조회
activate ProductService
ProductService-->>RankingFacade: List<Product> 반환
deactivate ProductService
RankingFacade->>BrandService: 브랜드 정보 조회
activate BrandService
BrandService-->>RankingFacade: Map<Long, Brand> 반환
deactivate BrandService
RankingFacade->>RankingFacade: 상품/브랜드 매핑 및 RankingInfo 조립
else RankingType.MONTHLY
RankingFacade->>ProductRankMonthlyJpaRepository: findByYearMonthOrderByRankPosition(yearMonth)
activate ProductRankMonthlyJpaRepository
ProductRankMonthlyJpaRepository-->>RankingFacade: List<ProductRankMonthly> 반환
deactivate ProductRankMonthlyJpaRepository
RankingFacade->>RankingFacade: 페이지네이션 처리 (skip/limit)
RankingFacade->>ProductService: 상품 정보 조회
RankingFacade->>BrandService: 브랜드 정보 조회
RankingFacade->>RankingFacade: 결과 조립
end
RankingFacade-->>RankingV1Controller: List<RankingInfo> 반환
deactivate RankingFacade
RankingV1Controller-->>Client: ApiResponse<RankingPageResponse> 200 OK
sequenceDiagram
participant Batch Scheduler
participant MonthlyRankingJobConfig
participant ProductMetricsJpaRepository
participant MonthlyRankingItemProcessor
participant ProductRankMonthlyJpaRepository
participant MonthlyRankingRankTasklet
participant Database
Batch Scheduler->>MonthlyRankingJobConfig: monthlyRankingJob 실행 (targetDate=20260416)
activate MonthlyRankingJobConfig
MonthlyRankingJobConfig->>MonthlyRankingJobConfig: Clear Step (monthlyRankingClearStep)
activate Database
Database->>Database: DELETE FROM mv_product_rank_monthly WHERE period_month = '2026-04'
deactivate Database
MonthlyRankingJobConfig->>ProductMetricsJpaRepository: 모든 ProductMetrics 페이지 단위 조회
activate ProductMetricsJpaRepository
ProductMetricsJpaRepository-->>MonthlyRankingJobConfig: List<ProductMetrics> (chunk=1000)
deactivate ProductMetricsJpaRepository
MonthlyRankingJobConfig->>MonthlyRankingItemProcessor: process(ProductMetrics item)
activate MonthlyRankingItemProcessor
MonthlyRankingItemProcessor->>MonthlyRankingItemProcessor: score 계산 (like*0.2 + order*0.7 + view*0.1)
MonthlyRankingItemProcessor->>MonthlyRankingItemProcessor: ProductRankMonthly 생성 (yearMonth='2026-04')
MonthlyRankingItemProcessor-->>MonthlyRankingJobConfig: ProductRankMonthly 반환
deactivate MonthlyRankingItemProcessor
MonthlyRankingJobConfig->>ProductRankMonthlyJpaRepository: save(ProductRankMonthly)
activate ProductRankMonthlyJpaRepository
ProductRankMonthlyJpaRepository-->>MonthlyRankingJobConfig: 저장 완료
deactivate ProductRankMonthlyJpaRepository
MonthlyRankingJobConfig->>MonthlyRankingJobConfig: Rank Step (monthlyRankingRankStep)
activate Database
Database->>Database: UPDATE mv_product_rank_monthly SET rank_position = ROW_NUMBER() OVER (ORDER BY score DESC)
Database->>Database: DELETE FROM mv_product_rank_monthly WHERE rank_position > 100 AND period_month = '2026-04'
deactivate Database
deactivate MonthlyRankingJobConfig
Batch Scheduler-->>Batch Scheduler: 배치 작업 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponFacadeTest.java (1)
95-117:⚠️ Potential issue | 🟠 Major
getIssueResult에 권한/실패 경계 테스트가 없어 보안 회귀를 막기 어렵다운영/보안 관점에서 현재는 성공 경로만 검증하여 타 사용자 요청 조회 허용 같은 권한 우회 회귀를 탐지하지 못한다. 수정안으로 인증 사용자와 요청 소유자가 다를 때 권한 예외를 반환하도록 명시하고, 테스트에서 해당 예외 흐름을 검증해야 한다. 추가 테스트로 (1) 다른 사용자로 조회 시 권한 오류, (2) 존재하지 않는 요청 ID 조회 시 NOT_FOUND, (3) 인증 실패 시 하위 서비스 미호출을 포함하는 것이 필요하다.
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-api/src/test/java/com/loopers/application/coupon/CouponFacadeTest.java` around lines 95 - 117, The test only covers the success path for CouponFacade.getIssueResult; add unit tests for the failure/authorization boundaries: (1) when an authenticated user is not the owner of the CouponIssueRequest returned by couponService.getRequest(1L) the facade should throw a permission/authorization exception and the test should assert that, (2) when couponService.getRequest(requestId) returns null or not found the facade should throw a NOT_FOUND exception and the test should assert that, and (3) when userService.authenticate(loginId, rawPassword) fails (returns null or throws) ensure couponService is not called and the facade surfaces the authentication failure—implement corresponding test methods in CouponFacadeTest invoking getIssueResult and stubbing userService and couponService behaviors to assert exceptions and verifyNoInteractions/verify on couponService where appropriate.
🧹 Nitpick comments (23)
apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java (1)
82-109: 토큰 검증 실패 케이스 테스트 누락.현재 테스트는
queueCacheService.validateToken(any(), any()).willReturn(true)로 토큰 검증을 항상 통과시킨다. 그러나 토큰 검증 실패 시BAD_REQUEST예외가 발생하는 경로에 대한 테스트가 없다.운영 관점에서 토큰 검증 실패는 실제 발생 가능한 시나리오이며, 이에 대한 테스트가 없으면 해당 예외 처리 로직의 회귀를 감지하지 못할 수 있다.
추가 테스트 케이스 제안
`@DisplayName`("유효하지 않은 토큰으로 주문 생성 시 BAD_REQUEST 예외가 발생한다.") `@Test` void throwsBadRequest_whenTokenIsInvalid() { // Arrange String loginId = "testuser"; String rawPassword = "Test1234!"; User user = new User(loginId, "encrypted", "홍길동", "19900101", "test@example.com"); List<OrderRequest.OrderItemRequest> items = List.of(new OrderRequest.OrderItemRequest(1L, 1)); given(userService.authenticate(loginId, rawPassword)).willReturn(user); given(queueCacheService.validateToken(any(), any())).willReturn(false); // Act & Assert CoreException exception = assertThrows(CoreException.class, () -> orderFacade.createOrder(loginId, rawPassword, "INVALID_TOKEN", items, null) ); assertThat(exception.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); }🤖 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/order/OrderFacadeTest.java` around lines 82 - 109, Add a new unit test that covers the token-validation-failure path: in the test arrange phase stub queueCacheService.validateToken(...) to return false (keep userService.authenticate(...) returning a User), then call orderFacade.createOrder(...) with an invalid token and assert that it throws a CoreException whose getErrorType() equals ErrorType.BAD_REQUEST; reference the existing test class and method naming style (e.g., throwsBadRequest_whenTokenIsInvalid) and reuse orderFacade.createOrder and queueCacheService.validateToken stubs to locate where to add this test.apps/commerce-api/src/test/java/com/loopers/application/order/OrderConcurrencyTest.java (1)
97-131: 실패 원인 검증 부재로 테스트 신뢰도 저하 가능.현재 테스트는
failCount만 확인하고 실패 원인(예외 타입/메시지)을 검증하지 않는다. 이 테스트의 목적은 "동일 쿠폰 동시 사용 시 1건만 성공"을 검증하는 것인데, 실패가 쿠폰 충돌(CONFLICT) 때문인지, 토큰 만료나 다른 이유 때문인지 구분하지 못한다.운영 관점에서 실패 원인이 변경되어도 테스트가 통과하면 실제 동시성 제어 로직의 회귀를 감지하지 못할 수 있다.
실패 원인 검증 추가 제안
AtomicInteger successCount = new AtomicInteger(0); AtomicInteger failCount = new AtomicInteger(0); +List<Exception> exceptions = Collections.synchronizedList(new ArrayList<>()); // Act for (int i = 0; i < threadCount; i++) { executor.submit(() -> { latch.countDown(); try { latch.await(); orderFacade.createOrder( "couponuser", RAW_PASSWORD, entryToken, List.of(new OrderRequest.OrderItemRequest(product.getId(), 1)), issuedCoupon.getId() ); successCount.incrementAndGet(); } catch (Exception e) { failCount.incrementAndGet(); + exceptions.add(e); } }); }Assert 부분에 다음을 추가한다:
assertThat(exceptions).hasSize(1); assertThat(exceptions.get(0)).isInstanceOf(CoreException.class); CoreException coreException = (CoreException) exceptions.get(0); assertThat(coreException.getErrorType()).isEqualTo(ErrorType.CONFLICT);🤖 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/order/OrderConcurrencyTest.java` around lines 97 - 131, The test currently only counts failures but doesn't verify their cause; update OrderConcurrencyTest to capture exceptions thrown by the concurrent tasks (e.g., collect thrown Exception objects into a thread-safe list/queue inside the executor loop where orderFacade.createOrder(...) is called) and after executor termination assert that exactly one exception was recorded, that it's an instance of CoreException, and that ((CoreException) thatException).getErrorType() equals ErrorType.CONFLICT; keep the existing assertions on successCount and IssuedCoupon.getStatus() to ensure only one successful use.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingClearTasklet.java (1)
24-25: 날짜 포맷터는 상수 하나로 통일하는 편이 낫다.Line 24 상수를 선언해 두고 Line 45에서 새 포맷터를 다시 만들면 패턴 변경 시 누락 위험이 생긴다.
LocalDate.parse(targetDate, DATE_FORMATTER)로 통일하고, 단위 테스트로toYearWeek("20260101")정상 변환과 잘못된 포맷 입력 예외를 함께 검증하는 것이 좋다.Also applies to: 45-45
🤖 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/step/WeeklyRankingClearTasklet.java` around lines 24 - 25, The code creates a duplicated DateTimeFormatter instead of reusing the class constant; update WeeklyRankingClearTasklet to use the existing DATE_FORMATTER (replace any new DateTimeFormatter.ofPattern(...) usages, e.g., inside the toYearWeek/parse logic) and change LocalDate.parse(targetDate, ...) to use DATE_FORMATTER; also add unit tests for WeeklyRankingClearTasklet.toYearWeek that assert correct conversion for "20260101" and that invalid formats throw the expected exception.apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankMonthlyJpaRepository.java (1)
10-12: 조회 결과 상한을 저장소 레벨에서 강제하는 편이 안전하다.Line 10 메서드는 월별 전체 행을 반환하므로, 배치 정리 단계 이상 시 API가 과다 행을 메모리로 적재해 응답 지연과 장애 전파를 유발할 수 있다.
Top100 + rankPosition not null조건을 저장소 메서드에 반영해 방어선을 추가하는 것이 좋다. 추가로 저장소 통합 테스트에서 150건 시드 후 100건만 반환되고rankPosition오름차순이 보장되는지 검증하는 것이 좋다.변경 예시
public interface ProductRankMonthlyJpaRepository extends JpaRepository<ProductRankMonthly, Long> { - List<ProductRankMonthly> findByYearMonthOrderByRankPosition(String yearMonth); + List<ProductRankMonthly> findTop100ByYearMonthAndRankPositionIsNotNullOrderByRankPositionAsc(String yearMonth); - long countByYearMonth(String yearMonth); + long countByYearMonthAndRankPositionLessThanEqual(String yearMonth, long maxRankPosition); }As per coding guidelines
**/*Repository*.java: 대량 데이터에서의 병목을 점검한다.🤖 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/ProductRankMonthlyJpaRepository.java` around lines 10 - 12, Update ProductRankMonthlyJpaRepository to limit query results at the repository level by adding a new/findByYearMonthTop100ByRankPositionNotNullOrderByRankPosition (or equivalent) method that returns only the Top 100 rows for a given yearMonth and filters out null rankPosition values (replace the existing findByYearMonthOrderByRankPosition usage where appropriate); keep countByYearMonth as-is for totals but ensure consumers use the new bounded method to avoid loading all rows. Also add a repository integration test that seeds 150 ProductRankMonthly records, calls the new repository method, and asserts exactly 100 rows are returned and that their rankPosition values are in ascending order to verify the limit and ordering behavior.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingItemProcessor.java (2)
22-23:toYearMonth()반복 호출 - 동일 Job 내에서 매 아이템마다 재계산
targetDate는 Job 파라미터로 고정된 값이므로,yearMonth를 매process()호출마다 재계산할 필요가 없다. 대량 데이터 처리 시 불필요한LocalDate.parse()호출이 반복된다.수정안: yearMonth 캐싱
+ private String cachedYearMonth; + `@Override` public ProductRankMonthly process(ProductMetrics item) { - String yearMonth = MonthlyRankingClearTasklet.toYearMonth(targetDate); + if (cachedYearMonth == null) { + cachedYearMonth = MonthlyRankingClearTasklet.toYearMonth(targetDate); + } double score = item.getLikeCount() * WEIGHT_LIKE🤖 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/step/MonthlyRankingItemProcessor.java` around lines 22 - 23, The code recomputes yearMonth inside ProductRankMonthlyProcessor.process(ProductMetrics) by calling MonthlyRankingClearTasklet.toYearMonth(targetDate) for every item; compute and cache yearMonth once (e.g., initialize a private final String yearMonth field in the processor's constructor or set it in a `@BeforeStep` method) using the existing targetDate, then reference that cached yearMonth inside process(...) to avoid repeated LocalDate.parse() calls.
14-16: 가중치 상수 중복 -WeeklyRankingItemProcessor와 동일한 값 사용
WEIGHT_LIKE,WEIGHT_ORDER,WEIGHT_VIEW상수가WeeklyRankingItemProcessor에도 동일하게 정의되어 있다. 가중치 정책 변경 시 두 곳을 동시에 수정해야 하며, 불일치 발생 시 주간/월간 랭킹 간 점수 계산 차이가 발생할 수 있다.수정안: 공통 상수 클래스 추출
public final class RankingScoreWeights { public static final double WEIGHT_LIKE = 0.2; public static final double WEIGHT_ORDER = 0.7; public static final double WEIGHT_VIEW = 0.1; private RankingScoreWeights() {} public static double calculate(long likeCount, long orderCount, long viewCount) { return likeCount * WEIGHT_LIKE + orderCount * WEIGHT_ORDER + viewCount * WEIGHT_VIEW; } }🤖 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/step/MonthlyRankingItemProcessor.java` around lines 14 - 16, MonthlyRankingItemProcessor defines WEIGHT_LIKE, WEIGHT_ORDER, WEIGHT_VIEW duplicated from WeeklyRankingItemProcessor; extract these into a single shared final class (e.g., RankingScoreWeights) that exposes the three constants and a calculate(likeCount, orderCount, viewCount) helper, then remove the duplicated constants from MonthlyRankingItemProcessor and WeeklyRankingItemProcessor and replace their local calculations with calls to RankingScoreWeights.WEIGHT_* or RankingScoreWeights.calculate(...) so both processors use the same canonical weights.scripts/seed-metrics.sql (1)
17-17:TRUNCATE TABLE사용 시 FK 제약조건 주의
TRUNCATE TABLE product_metrics는 외래 키 제약조건이 있을 경우 실패할 수 있다.product_metrics테이블이 다른 테이블과 FK 관계가 없다면 문제없으나, 향후 스키마 변경 시 주의가 필요하다.안전하게 하려면
DELETE FROM product_metrics를 사용하거나SET FOREIGN_KEY_CHECKS=0후TRUNCATE를 실행할 수 있다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/seed-metrics.sql` at line 17, TRUNCATE TABLE product_metrics can fail if there are foreign key constraints; change the operation to a safe alternative: either replace "TRUNCATE TABLE product_metrics" with "DELETE FROM product_metrics" to respect FK constraints, or explicitly wrap the truncate with disabling FK checks (SET FOREIGN_KEY_CHECKS=0; TRUNCATE TABLE product_metrics; SET FOREIGN_KEY_CHECKS=1;) so future schema changes with FKs won't break the seed script; update the statement involving product_metrics accordingly.modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankWeekly.java (2)
46-57:LocalDateTime.now()사용으로 테스트 어려움
of()팩토리 메서드에서LocalDateTime.now()를 직접 호출하면 단위 테스트에서 시간 값을 검증하기 어렵다. 또한 배치 실행 중 아이템 간updatedAt값이 미세하게 달라질 수 있다.수정안: Clock 주입 또는 파라미터화
- public static ProductRankWeekly of(Long productId, double score, long likeCount, long orderCount, long viewCount, String yearWeek) { + public static ProductRankWeekly of(Long productId, double score, long likeCount, long orderCount, long viewCount, String yearWeek, LocalDateTime updatedAt) { ProductRankWeekly entity = new ProductRankWeekly(); // ... - entity.updatedAt = LocalDateTime.now(); + entity.updatedAt = updatedAt; return entity; }호출 측에서 일관된 시간을 전달하도록 변경한다.
🤖 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/ProductRankWeekly.java` around lines 46 - 57, The factory method ProductRankWeekly.of sets updatedAt using LocalDateTime.now(), which makes tests fragile and produces slightly different timestamps across batch-created entities; change the API to accept a LocalDateTime (or a Clock) from the caller and assign that to updatedAt (e.g., add an overload or new parameter to of to accept updatedAt or a Clock), update any callers to pass a deterministic time in tests or a common clock during batch runs, and ensure the existing field ProductRankWeekly.updatedAt is assigned from that passed-in value rather than calling LocalDateTime.now() inside the method.
14-62:equals()/hashCode()미구현JPA 엔티티에서
equals()/hashCode()가 구현되지 않으면Set이나Map에서 예기치 않은 동작이 발생할 수 있다. 식별자(id) 기반 구현을 권장한다.현재 사용 패턴(batch write → read by period)에서는 큰 문제가 없을 수 있으나, 향후 확장 시 고려가 필요하다.
🤖 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/ProductRankWeekly.java` around lines 14 - 62, The entity ProductRankWeekly lacks equals() and hashCode(), which can cause incorrect behavior when instances are used in Sets/Maps; implement equals() and hashCode() in the ProductRankWeekly class using the primary key id (treating transient/new entities without id as not equal), e.g., have equals compare class and non-null id equality and have hashCode return a stable value derived from id (handling null id safely), and ensure these methods replace default Object implementations so collections behave correctly.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingItemProcessor.java (1)
21-26:toYearWeek()반복 호출 및 가중치 상수 중복
MonthlyRankingItemProcessor와 동일한 문제가 존재한다:
yearWeek계산이 매 아이템마다 반복됨 (LocalDate.parse()포함)- 가중치 상수가 중복 정의됨
공통 상수 클래스 추출 및
yearWeek캐싱을 권장한다. 상세 수정안은MonthlyRankingItemProcessor리뷰 코멘트 참조.🤖 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/step/WeeklyRankingItemProcessor.java` around lines 21 - 26, The process method in WeeklyRankingItemProcessor repeatedly calls WeeklyRankingClearTasklet.toYearWeek (which parses targetDate per item) and duplicates weight constants (WEIGHT_LIKE, WEIGHT_ORDER, WEIGHT_VIEW); compute yearWeek once and reuse it (cache it in the processor instance or compute in the step setup and reference the cached value in process(ProductMetrics)), and move the weight constants into a shared constants class (e.g., RankingWeights) used by both WeeklyRankingItemProcessor and MonthlyRankingItemProcessor so WEIGHT_LIKE/WEIGHT_ORDER/WEIGHT_VIEW are not duplicated.modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankMonthly.java (1)
14-62:ProductRankWeekly와 높은 코드 중복
ProductRankMonthly와ProductRankWeekly는yearWeek/yearMonth필드와 컬럼명만 다르고 나머지 구조가 동일하다. 동일한 문제점(LocalDateTime.now(),equals()/hashCode()미구현)도 공유한다.추상 베이스 클래스 또는 공통 인터페이스 추출을 고려할 수 있으나, 현재 규모에서는 유지보수 부담이 크지 않다면 허용 가능한 수준이다.
리팩토링 방향 예시
`@MappedSuperclass` public abstract class ProductRankBase { `@Id` `@GeneratedValue`(strategy = GenerationType.IDENTITY) private Long id; `@Column`(name = "product_id", nullable = false) private Long productId; `@Column`(name = "rank_position", nullable = false) private Integer rankPosition; // ... 공통 필드 public abstract String getPeriod(); }🤖 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/ProductRankMonthly.java` around lines 14 - 62, ProductRankMonthly duplicates ProductRankWeekly and shares issues (direct LocalDateTime.now() usage and missing equals()/hashCode()); extract a `@MappedSuperclass` like ProductRankBase containing common fields (id, productId, rankPosition, score, likeCount, orderCount, viewCount, updatedAt) and common methods (assignRank), move shared static factory logic out or make a protected factory in ProductRankBase, let concrete classes (ProductRankMonthly/ProductRankWeekly) only define their period field and column name (yearMonth/yearWeek) and implement an abstract getPeriod(); replace direct LocalDateTime.now() in of() with an injectable Clock or accept updatedAt as a parameter so tests can control time, and implement equals() and hashCode() in the base class (or each entity) using id and productId to ensure correct identity semantics.k6/06_ranking.js (2)
295-307: Weekly 시나리오에서rank필드 검증이 누락되었다.시나리오 A(
scenarioRankingRead)에서는rank 필드 존재검증이 있으나, 시나리오 E/F에서는 누락되었다. 일관성을 위해 동일한 검증을 추가하거나, 의도적으로 생략한 경우 주석으로 명시해야 한다.♻️ rank 필드 검증 추가
const ok = check(res, { '[E] status 200': (r) => r.status === 200, '[E] data.content 배열': (r) => { try { return Array.isArray(JSON.parse(r.body).data.content); } catch { return false; } }, + '[E] rank 필드 존재': (r) => { + try { + const content = JSON.parse(r.body).data.content; + return content.length === 0 || content[0].rank !== undefined; + } catch { return false; } + }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/06_ranking.js` around lines 295 - 307, The weekly ranking scenario function scenarioWeeklyRankingRead is missing the validation for each item's rank field; update the check in scenarioWeeklyRankingRead to include a predicate that parses JSON.parse(res.body).data.content and verifies every element has a defined numeric (or non-null) rank (same style as scenarioRankingRead), add that predicate to the check map so weeklyRankingFailRate reflects rank validation, or if omission is intentional add a clear comment in scenarioWeeklyRankingRead explaining why rank is skipped.
293-307: Weekly/Monthly 시나리오 실행 전 batch job 실행이 필요함을 문서화해야 한다.
scenarioWeeklyRankingRead와scenarioMonthlyRankingRead는 MV 테이블(mv_product_rank_weekly,mv_product_rank_monthly)에서 데이터를 조회한다. batch job이 미리 실행되지 않으면 빈 결과가 반환되어 의미 있는 부하 테스트가 되지 않는다.파일 상단 주석의 "사전 조건"에 batch job 실행 요구사항을 추가해야 한다.
📝 문서 추가 제안
* 사전 조건: * - 서버 실행 중 (commerce-api, commerce-streamer, Redis, Kafka) * - user1~user100 계정 등록 완료 (bash scripts/seed-users.sh) * - 상품 데이터 존재 (productId 1~20) +* - 주간/월간 랭킹 시나리오(E, F) 실행 시 batch job 선행 실행 필요: +* - java -jar commerce-batch.jar --spring.batch.job.name=weeklyRankingJob targetDate=YYYYMMDD +* - java -jar commerce-batch.jar --spring.batch.job.name=monthlyRankingJob targetDate=YYYYMMDD🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@k6/06_ranking.js` around lines 293 - 307, Add a precondition note at the top-of-file comment stating that the weekly/monthly ranking scenarios require the batch job that populates the materialized view tables to be run beforehand; explicitly mention that scenarioWeeklyRankingRead and scenarioMonthlyRankingRead query mv_product_rank_weekly and mv_product_rank_monthly respectively and will return empty results if the batch job hasn’t been executed, so include instructions to run the batch job (or link to its trigger) before executing these scenarios.apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java (2)
32-33: 주입된productRankWeeklyJpaRepository가 사용되지 않는다.테스트 결과 검증에 사용하려고 주입한 것으로 보이나 실제 assertion에서 사용되지 않는다. 의도대로 결과 검증에 활용하거나, 불필요하면 제거해야 한다.
🤖 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 32 - 33, The injected productRankWeeklyJpaRepository field in WeeklyRankingJobE2ETest is unused; either use it to assert persisted results or remove the injection. Locate the private field productRankWeeklyJpaRepository and, if verifying DB state, query it (e.g., findAll/findBy... on ProductRankWeeklyJpaRepository) and add assertions against expected weekly ranks after the job runs; otherwise delete the field and any related unused imports to keep the test clean.
49-67: Job 성공 테스트에서 실제 결과 검증이 누락되었다.
ExitStatus.COMPLETED만 확인하고ProductRankWeeklyJpaRepository에 데이터가 정상적으로 적재되었는지 검증하지 않는다. Job이 성공 상태로 종료되더라도 aggregate/rank step에서 0건 처리 시 실제 ranking 데이터는 없을 수 있다.
- seed 데이터(ProductMetrics)가 없으면 빈 결과로 COMPLETED될 수 있어 테스트 신뢰도가 낮다.
- 테스트 DB에 seed 데이터를 준비하고, job 실행 후 repository 조회로 결과를 검증해야 한다.
🧪 결과 검증 추가 예시
// Assert assertAll( () -> assertThat(jobExecution).isNotNull(), () -> assertThat(jobExecution.getExitStatus().getExitCode()) - .isEqualTo(ExitStatus.COMPLETED.getExitCode()) + .isEqualTo(ExitStatus.COMPLETED.getExitCode()), + () -> { + // seed 데이터가 있다면 ranking 결과 검증 + var rankings = productRankWeeklyJpaRepository.findByYearWeekOrderByRankPosition("2026-W16"); + assertThat(rankings).isNotEmpty(); + assertThat(rankings.get(0).getRankPosition()).isEqualTo(1); + } );🤖 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 49 - 67, The test WeeklyRankingJobE2ETest currently only asserts ExitStatus.COMPLETED; seed the DB with minimal ProductMetrics records before launching the job (use existing repositories or a test fixture), run the job via jobLauncherTestUtils.launchJob(jobParameters), then query ProductRankWeeklyJpaRepository to assert expected records were created (count > 0 and/or specific field values) to ensure the aggregate/rank step actually produced ranking data; use the same test setup/teardown pattern as other tests to persist seed data and clean it up after the test.apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java (2)
80-88: 전체 조회 후 in-memory pagination은 데이터 증가 시 비효율적이다.현재 TOP_N이 100으로 제한되어 있어 문제없지만, 향후 데이터 보관 정책 변경이나 페이지 크기 증가 시 전체 데이터를 메모리에 로드하는 것은 비효율적이다. DB-level pagination(Spring Data의
Pageable사용)을 적용하면 확장성이 개선된다.♻️ DB-level pagination 적용 예시
Repository에 Pageable 지원 메서드 추가:
Page<ProductRankWeekly> findByYearWeek(String yearWeek, Pageable pageable);Facade 호출 변경:
- List<ProductRankWeekly> ranked = productRankWeeklyJpaRepository - .findByYearWeekOrderByRankPosition(yearWeek); - - int offset = (page - 1) * size; - List<ProductRankWeekly> paged = ranked.stream() - .skip(offset).limit(size).toList(); + Pageable pageable = PageRequest.of(page - 1, size, Sort.by("rankPosition")); + Page<ProductRankWeekly> pagedResult = productRankWeeklyJpaRepository + .findByYearWeek(yearWeek, pageable); + List<ProductRankWeekly> paged = pagedResult.getContent();🤖 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 80 - 88, The getWeeklyRankings method currently loads all ProductRankWeekly into memory then paginates; change it to use DB-level pagination by adding a repository method like findByYearWeek(String yearWeek, Pageable pageable) on productRankWeeklyJpaRepository and call it from getWeeklyRankings with a PageRequest built from page-1 and size (and sort by rankPosition) to return only the requested page (use Page<ProductRankWeekly> and .getContent()). Update getWeeklyRankings to accept/convert the page params accordingly and remove the in-memory skip/limit logic.
99-108: stream 내 null return 패턴은 가독성이 낮다.
map내에서 null을 반환하고filter(r -> r != null)로 제거하는 패턴은 동작하지만,Objects::nonNull또는Optional을 활용하면 의도가 더 명확해진다. 단, 현재 코드도 정상 동작하므로 선택적 개선 사항이다.♻️ Objects::nonNull 사용
return paged.stream() .map(r -> { Product product = productMap.get(r.getProductId()); if (product == null) return null; Brand brand = brandMap.get(product.getBrandId()); if (brand == null) return null; return RankingInfo.of(r.getRankPosition(), ProductInfo.from(product, brand)); }) - .filter(r -> r != null) + .filter(Objects::nonNull) .toList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java` around lines 99 - 108, In RankingFacade, avoid returning null inside the stream map and then filtering; instead make the intent explicit by replacing the map+filter pattern on paged.stream() with either mapping to Optional (e.g., Optional.ofNullable(Product).flatMap(...) and flatMap(Optional::stream)) or keep the current mapping but use Objects::nonNull in the filter (replace filter(r -> r != null) with filter(Objects::nonNull)); update the lambda that produces RankingInfo.of(...) (referenced as r -> { Product product = productMap.get(r.getProductId()); Brand brand = brandMap.get(product.getBrandId()); ... }) to return an Optional or allow the non-null filter to be Objects::nonNull for clearer intent.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java (1)
51-60:JobParametersValidator등록을 권장한다.
targetDate파라미터 누락 시 job이 실행되다가 step 내부에서 NPE로 실패한다.DefaultJobParametersValidator또는 커스텀 validator를 등록하면 job 시작 전에 검증하여 불필요한 리소스 소비와 불명확한 오류 메시지를 방지할 수 있다.♻️ JobParametersValidator 추가 예시
+import org.springframework.batch.core.job.DefaultJobParametersValidator; ... `@Bean`(JOB_NAME) public Job weeklyRankingJob() { return new JobBuilder(JOB_NAME, jobRepository) .incrementer(new RunIdIncrementer()) + .validator(new DefaultJobParametersValidator( + new String[]{"targetDate"}, // required + new String[]{} // optional + )) .start(weeklyRankingClearStep()) .next(weeklyRankingAggregateStep()) .next(weeklyRankingRankStep()) .listener(jobListener) .build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java` around lines 51 - 60, The job lacks a JobParametersValidator so a missing targetDate causes an NPE inside steps; update the weeklyRankingJob bean to register a JobParametersValidator (e.g., DefaultJobParametersValidator or a small custom validator) on the JobBuilder for JOB_NAME used in weeklyRankingJob so the job fails fast on missing/invalid parameters before executing weeklyRankingClearStep/weeklyRankingAggregateStep/weeklyRankingRankStep; ensure the validator requires "targetDate" (and any other required params) and is wired alongside the existing RunIdIncrementer and jobListener.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java (1)
51-60:JobParametersValidator등록을 권장한다 - WeeklyRankingJobConfig와 동일한 이슈이다.Weekly job과 동일하게
targetDate파라미터 검증을 job 시작 전에 수행하도록 validator 등록을 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java` around lines 51 - 60, Add a JobParametersValidator to the MonthlyRankingJobConfig monthlyRankingJob() bean to validate the targetDate parameter before job start (same as WeeklyRankingJobConfig). Update the JobBuilder for JOB_NAME to call .validator(...) with the existing or a new implementation of JobParametersValidator that enforces presence/format of "targetDate", ensuring monthlyRankingClearStep(), monthlyRankingAggregateStep(), monthlyRankingRankStep() run only when parameters are valid and keep jobListener attached.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingRankTasklet.java (1)
32-44: UPDATE 쿼리 결과(affected rows)를 로깅하면 운영 모니터링에 유용하다.현재 DELETE 결과만 로깅되고 있다. UPDATE가 0건인 경우 해당
period_week에 데이터가 없거나 aggregation step이 실패했음을 의미할 수 있다. affected rows를 로깅하면 job 실행 결과 검증 및 알람 설정에 도움이 된다.♻️ UPDATE 결과 로깅 추가
- jdbcTemplate.update( + int updated = jdbcTemplate.update( """ UPDATE mv_product_rank_weekly w ... """, yearWeek, yearWeek ); + log.info("[WeeklyRankingRankTasklet] 순위 부여 완료: {}건", updated);🤖 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/step/WeeklyRankingRankTasklet.java` around lines 32 - 44, The UPDATE statement executed via jdbcTemplate.update in WeeklyRankingRankTasklet currently ignores the returned affected-row count; capture the int result from the jdbcTemplate.update call (e.g., int updated = jdbcTemplate.update(...)) and log it alongside the yearWeek (using the existing logger/processLogger) so operators can detect 0-updates and trigger alerts or investigate aggregation failures; also consider logging at WARN when updated == 0 and INFO otherwise to match the existing DELETE-result logging behavior.apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java (2)
51-88: product 또는 brand가 조회되지 않는 경우의 필터링 테스트가 누락되었다.
RankingFacade에서 product 또는 brand가 null인 경우 결과에서 제외하는 로직이 있으나(if (product == null) continue;), 이를 검증하는 테스트가 없다. 삭제된 상품이나 브랜드가 있을 때 정상적으로 필터링되는지 확인해야 한다.🤖 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 51 - 88, The test suite is missing a case that verifies RankingFacade.getRankings filters out entries when product or brand lookups return null; add a test in the GetRankings nested class that arranges rankingCacheService.getProductIds("20260408", 0, 20) to return an id list containing entries whose corresponding productService.getProductsByIds(...) or brandService.getBrandsByIds(...) will not include (or will return null for) that id, then call rankingFacade.getRankings("20260408", 1, 20, RankingType.DAILY) and assert the result excludes the missing product/brand (e.g., result is empty or size reduced), ensuring you reference the existing mocks rankingCacheService, productService, brandService and the method getRankings to validate the filtering behavior.
109-145: Weekly ranking의 pagination 테스트가 누락되었다.Daily ranking에는
page=2테스트(assignsRankFromOffset_whenPage2Requested)가 있으나, Weekly/Monthly에는 pagination 동작 검증이 없다. in-memory pagination 로직(skip/limit)이 올바르게 동작하는지 확인하는 테스트가 필요하다.🧪 Weekly pagination 테스트 예시
`@DisplayName`("주간 랭킹 page=2 요청 시 올바른 데이터가 반환된다.") `@Test` void returnsWeeklyRankings_whenPage2Requested() { // Arrange: rank 1~25 데이터 준비 List<ProductRankWeekly> rankings = IntStream.rangeClosed(1, 25) .mapToObj(i -> { var r = ProductRankWeekly.of((long) i, 100.0 - i, 10L, 5L, 20L, "2026-W16"); r.assignRank(i); return r; }).toList(); given(productRankWeeklyJpaRepository.findByYearWeekOrderByRankPosition("2026-W16")) .willReturn(rankings); // ... product/brand mocking // Act List<RankingInfo> result = rankingFacade.getRankings("20260416", 2, 20, RankingType.WEEKLY); // Assert assertThat(result).hasSize(5); // 21~25번 assertThat(result.get(0).rank()).isEqualTo(21L); }🤖 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 109 - 145, Add a new test method returnsWeeklyRankings_whenPage2Requested that verifies pagination by creating 25 ProductRankWeekly entries (use ProductRankWeekly.of(...) and call assignRank(i) for i=1..25), mock productRankWeeklyJpaRepository.findByYearWeekOrderByRankPosition("2026-W16") to return that list, mock productService.getProductsByIds(...) and brandService.getBrandsByIds(...) to return matching Product and Brand objects for the IDs used, call rankingFacade.getRankings("20260416", 2, 20, RankingType.WEEKLY), and assert the result has size 5 and the first returned RankingInfo.rank() equals 21L to confirm skip/limit pagination works.apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingRankTasklet.java (1)
32-44: UPDATE 결과 로깅 추가를 권장한다.WeeklyRankingRankTasklet과 동일하게 UPDATE affected rows를 로깅하면 운영 시 데이터 정합성 확인에 유용하다.
🤖 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/step/MonthlyRankingRankTasklet.java` around lines 32 - 44, Capture the integer return value from the jdbcTemplate.update call in MonthlyRankingRankTasklet (e.g., int updated = jdbcTemplate.update(...)) and add a log statement (using the class logger) similar to WeeklyRankingRankTasklet to print the affected rows and the yearMonth parameter — e.g., log.info("Monthly rank UPDATE affected {} rows for periodMonth={}", updated, yearMonth) so operators can verify data consistency after the UPDATE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 148-159: Both toYearWeek and toYearMonth call
LocalDate.parse(date, DATE_FORMATTER) and let DateTimeParseException bubble up
causing 500 responses; wrap the parse call in a try-catch for
DateTimeParseException inside each method (toYearWeek and toYearMonth) and
convert it into a client-facing exception (e.g., throw new
IllegalArgumentException or a custom InvalidDateFormatException with a clear
message that includes the invalid input) so the controller can map it to a 400
Bad Request.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`:
- Line 34: 현재 RankingV1Controller에서 RankingType rankingType =
RankingType.valueOf(type.toUpperCase()); 호출 시 잘못된 type 값이면
IllegalArgumentException이 발생하여 CoreException 기반의 일관된 에러 처리 경로를 우회합니다; 수정 방법은
RankingV1Controller에서 valueOf 호출을 try-catch로 감싸고 IllegalArgumentException을 잡아
CoreException(또는 적절한 400 Bad Request 상태를 표현하는 예외)으로 변환해 던지거나, 대안으로 RankingType에
정적 팩토리 메서드(e.g., RankingType.from(String))를 추가해 유효성 검증 후 유효하지 않으면 CoreException을
던지도록 구현해 일관된 400 응답이 반환되게 하세요.
In
`@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponFacadeTest.java`:
- Line 71: The test currently only asserts result is not null in
CouponFacadeTest; replace and augment these weak assertions by asserting
CouponIssueInfo's core fields (e.g., requestId, couponTemplateId, userId,
status) equal the expected values from the fixture/stub used to produce the
result (reference the result variable and the method under test in
CouponFacadeTest), and add a complementary negative test that stubs the
dependency to return mismatched/different request data and verifies those field
assertions fail (or that the facade returns the correct error/exception) to
cover regression and edge/failure flows.
- Around line 62-66: The test uses loose stubbing allowing wrong args to
pass—replace any()/implicit user ids by declaring a concrete userId variable and
use eq(userId)/eq(1L) in the stubs for userService.authenticate(...) and
couponService.createRequest(...), then add explicit verifications like
verify(couponService).createRequest(eq(1L), eq(userId)) and add a negative test
case that asserts a call with an incorrect template id fails; update references
in this file to CouponIssueRequest, userService.authenticate,
couponService.createRequest and the verify(couponService).createRequest
assertion accordingly.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingRankTasklet.java`:
- Around line 22-30: The execute method in MonthlyRankingRankTasklet calls
MonthlyRankingClearTasklet.toYearMonth(targetDate) without validating
targetDate, which can cause an NPE; add an explicit null/empty check for the
injected field targetDate at the start of execute (same pattern as
WeeklyRankingRankTasklet) and if missing, log an error via log.error and fail
the step early by throwing an IllegalArgumentException or returning an
appropriate failure RepeatStatus; update references inside execute and ensure
toYearMonth is only called when targetDate is non-null/non-empty.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingClearTasklet.java`:
- Around line 36-39: The current WeeklyRankingClearTasklet deletes existing
weekly rows via jdbcTemplate.update("DELETE FROM mv_product_rank_weekly WHERE
period_week = ?", yearWeek) which can leave a gap if downstream Aggregate/Rank
steps fail; change flow to use a staging table and atomic swap (or a run_id
active-pointer) instead: stop deleting in WeeklyRankingClearTasklet, write new
results into a staging table (e.g., mv_product_rank_weekly_staging) keyed by
period_week and run_id, then after the Aggregate/Rank steps succeed perform an
atomic replace of mv_product_rank_weekly from staging (via transactional
DELETE/INSERT, table rename, or flip an active flag by run_id) in a final
commit/step (refer to WeeklyRankingClearTasklet, the Aggregate/Rank step names,
and any run_id handling), and add an E2E test that forces Aggregate step failure
while prior week data exists to assert the original mv_product_rank_weekly
remains unchanged.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingRankTasklet.java`:
- Around line 22-30: The code calls
WeeklyRankingClearTasklet.toYearWeek(targetDate) without validating the injected
field targetDate, risking an NPE; update WeeklyRankingRankTasklet.execute to
explicitly check that targetDate is not null/blank and throw a clear
IllegalArgumentException (or JobParametersInvalidException) with a descriptive
message (e.g. "Missing job parameter 'targetDate'") before calling toYearWeek,
and alternatively register a JobParametersValidator for the job configuration to
enforce presence of 'targetDate' at job startup; reference the targetDate field,
execute(...) method, and WeeklyRankingClearTasklet.toYearWeek when making the
change.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.java`:
- Around line 49-67: The test success_whenTargetDateParamIsGiven currently only
asserts ExitStatus.COMPLETED; enhance it to query
productRankMonthlyJpaRepository after launching the job (using
jobLauncherTestUtils and the existing JobParametersBuilder) and assert that rows
were created, that each result has yearMonth equal to "2026-04" (or derived from
the targetDate param), and that rankPosition values are assigned (e.g., > 0);
locate these changes in the same test method and add assertions that check
results.size() > 0, the yearMonth field on returned ProductRankMonthly entities,
and rankPosition presence/validity (using
productRankMonthlyJpaRepository.findByYearMonthOrderByRankPosition or equivalent
repository method).
- Around line 35-47: Add explicit job parameter validation in
MonthlyRankingJobConfig by registering a DefaultJobParametersValidator in the
monthlyRankingJob() bean (require "targetDate") so missing parameters produce a
JobParametersInvalidException instead of a NPE from
MonthlyRankingClearTasklet.toYearMonth; then update the tests: keep
fails_whenTargetDateParamIsMissing but rely on the validator to trigger failure,
and modify success_whenTargetDateParamIsGiven to launch the job with
JobParametersBuilder.addString("targetDate", "20260416") and assert both
ExitStatus.COMPLETED and that productRankMonthlyJpaRepository.findAll() is not
empty to verify data persistence.
---
Outside diff comments:
In
`@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponFacadeTest.java`:
- Around line 95-117: The test only covers the success path for
CouponFacade.getIssueResult; add unit tests for the failure/authorization
boundaries: (1) when an authenticated user is not the owner of the
CouponIssueRequest returned by couponService.getRequest(1L) the facade should
throw a permission/authorization exception and the test should assert that, (2)
when couponService.getRequest(requestId) returns null or not found the facade
should throw a NOT_FOUND exception and the test should assert that, and (3) when
userService.authenticate(loginId, rawPassword) fails (returns null or throws)
ensure couponService is not called and the facade surfaces the authentication
failure—implement corresponding test methods in CouponFacadeTest invoking
getIssueResult and stubbing userService and couponService behaviors to assert
exceptions and verifyNoInteractions/verify on couponService where appropriate.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`:
- Around line 80-88: The getWeeklyRankings method currently loads all
ProductRankWeekly into memory then paginates; change it to use DB-level
pagination by adding a repository method like findByYearWeek(String yearWeek,
Pageable pageable) on productRankWeeklyJpaRepository and call it from
getWeeklyRankings with a PageRequest built from page-1 and size (and sort by
rankPosition) to return only the requested page (use Page<ProductRankWeekly> and
.getContent()). Update getWeeklyRankings to accept/convert the page params
accordingly and remove the in-memory skip/limit logic.
- Around line 99-108: In RankingFacade, avoid returning null inside the stream
map and then filtering; instead make the intent explicit by replacing the
map+filter pattern on paged.stream() with either mapping to Optional (e.g.,
Optional.ofNullable(Product).flatMap(...) and flatMap(Optional::stream)) or keep
the current mapping but use Objects::nonNull in the filter (replace filter(r ->
r != null) with filter(Objects::nonNull)); update the lambda that produces
RankingInfo.of(...) (referenced as r -> { Product product =
productMap.get(r.getProductId()); Brand brand =
brandMap.get(product.getBrandId()); ... }) to return an Optional or allow the
non-null filter to be Objects::nonNull for clearer intent.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankMonthlyJpaRepository.java`:
- Around line 10-12: Update ProductRankMonthlyJpaRepository to limit query
results at the repository level by adding a
new/findByYearMonthTop100ByRankPositionNotNullOrderByRankPosition (or
equivalent) method that returns only the Top 100 rows for a given yearMonth and
filters out null rankPosition values (replace the existing
findByYearMonthOrderByRankPosition usage where appropriate); keep
countByYearMonth as-is for totals but ensure consumers use the new bounded
method to avoid loading all rows. Also add a repository integration test that
seeds 150 ProductRankMonthly records, calls the new repository method, and
asserts exactly 100 rows are returned and that their rankPosition values are in
ascending order to verify the limit and ordering behavior.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderConcurrencyTest.java`:
- Around line 97-131: The test currently only counts failures but doesn't verify
their cause; update OrderConcurrencyTest to capture exceptions thrown by the
concurrent tasks (e.g., collect thrown Exception objects into a thread-safe
list/queue inside the executor loop where orderFacade.createOrder(...) is
called) and after executor termination assert that exactly one exception was
recorded, that it's an instance of CoreException, and that ((CoreException)
thatException).getErrorType() equals ErrorType.CONFLICT; keep the existing
assertions on successCount and IssuedCoupon.getStatus() to ensure only one
successful use.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java`:
- Around line 82-109: Add a new unit test that covers the
token-validation-failure path: in the test arrange phase stub
queueCacheService.validateToken(...) to return false (keep
userService.authenticate(...) returning a User), then call
orderFacade.createOrder(...) with an invalid token and assert that it throws a
CoreException whose getErrorType() equals ErrorType.BAD_REQUEST; reference the
existing test class and method naming style (e.g.,
throwsBadRequest_whenTokenIsInvalid) and reuse orderFacade.createOrder and
queueCacheService.validateToken stubs to locate where to add this test.
In
`@apps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.java`:
- Around line 51-88: The test suite is missing a case that verifies
RankingFacade.getRankings filters out entries when product or brand lookups
return null; add a test in the GetRankings nested class that arranges
rankingCacheService.getProductIds("20260408", 0, 20) to return an id list
containing entries whose corresponding productService.getProductsByIds(...) or
brandService.getBrandsByIds(...) will not include (or will return null for) that
id, then call rankingFacade.getRankings("20260408", 1, 20, RankingType.DAILY)
and assert the result excludes the missing product/brand (e.g., result is empty
or size reduced), ensuring you reference the existing mocks rankingCacheService,
productService, brandService and the method getRankings to validate the
filtering behavior.
- Around line 109-145: Add a new test method
returnsWeeklyRankings_whenPage2Requested that verifies pagination by creating 25
ProductRankWeekly entries (use ProductRankWeekly.of(...) and call assignRank(i)
for i=1..25), mock
productRankWeeklyJpaRepository.findByYearWeekOrderByRankPosition("2026-W16") to
return that list, mock productService.getProductsByIds(...) and
brandService.getBrandsByIds(...) to return matching Product and Brand objects
for the IDs used, call rankingFacade.getRankings("20260416", 2, 20,
RankingType.WEEKLY), and assert the result has size 5 and the first returned
RankingInfo.rank() equals 21L to confirm skip/limit pagination works.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`:
- Around line 51-60: Add a JobParametersValidator to the MonthlyRankingJobConfig
monthlyRankingJob() bean to validate the targetDate parameter before job start
(same as WeeklyRankingJobConfig). Update the JobBuilder for JOB_NAME to call
.validator(...) with the existing or a new implementation of
JobParametersValidator that enforces presence/format of "targetDate", ensuring
monthlyRankingClearStep(), monthlyRankingAggregateStep(),
monthlyRankingRankStep() run only when parameters are valid and keep jobListener
attached.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingItemProcessor.java`:
- Around line 22-23: The code recomputes yearMonth inside
ProductRankMonthlyProcessor.process(ProductMetrics) by calling
MonthlyRankingClearTasklet.toYearMonth(targetDate) for every item; compute and
cache yearMonth once (e.g., initialize a private final String yearMonth field in
the processor's constructor or set it in a `@BeforeStep` method) using the
existing targetDate, then reference that cached yearMonth inside process(...) to
avoid repeated LocalDate.parse() calls.
- Around line 14-16: MonthlyRankingItemProcessor defines WEIGHT_LIKE,
WEIGHT_ORDER, WEIGHT_VIEW duplicated from WeeklyRankingItemProcessor; extract
these into a single shared final class (e.g., RankingScoreWeights) that exposes
the three constants and a calculate(likeCount, orderCount, viewCount) helper,
then remove the duplicated constants from MonthlyRankingItemProcessor and
WeeklyRankingItemProcessor and replace their local calculations with calls to
RankingScoreWeights.WEIGHT_* or RankingScoreWeights.calculate(...) so both
processors use the same canonical weights.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingRankTasklet.java`:
- Around line 32-44: Capture the integer return value from the
jdbcTemplate.update call in MonthlyRankingRankTasklet (e.g., int updated =
jdbcTemplate.update(...)) and add a log statement (using the class logger)
similar to WeeklyRankingRankTasklet to print the affected rows and the yearMonth
parameter — e.g., log.info("Monthly rank UPDATE affected {} rows for
periodMonth={}", updated, yearMonth) so operators can verify data consistency
after the UPDATE.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingClearTasklet.java`:
- Around line 24-25: The code creates a duplicated DateTimeFormatter instead of
reusing the class constant; update WeeklyRankingClearTasklet to use the existing
DATE_FORMATTER (replace any new DateTimeFormatter.ofPattern(...) usages, e.g.,
inside the toYearWeek/parse logic) and change LocalDate.parse(targetDate, ...)
to use DATE_FORMATTER; also add unit tests for
WeeklyRankingClearTasklet.toYearWeek that assert correct conversion for
"20260101" and that invalid formats throw the expected exception.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingItemProcessor.java`:
- Around line 21-26: The process method in WeeklyRankingItemProcessor repeatedly
calls WeeklyRankingClearTasklet.toYearWeek (which parses targetDate per item)
and duplicates weight constants (WEIGHT_LIKE, WEIGHT_ORDER, WEIGHT_VIEW);
compute yearWeek once and reuse it (cache it in the processor instance or
compute in the step setup and reference the cached value in
process(ProductMetrics)), and move the weight constants into a shared constants
class (e.g., RankingWeights) used by both WeeklyRankingItemProcessor and
MonthlyRankingItemProcessor so WEIGHT_LIKE/WEIGHT_ORDER/WEIGHT_VIEW are not
duplicated.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingRankTasklet.java`:
- Around line 32-44: The UPDATE statement executed via jdbcTemplate.update in
WeeklyRankingRankTasklet currently ignores the returned affected-row count;
capture the int result from the jdbcTemplate.update call (e.g., int updated =
jdbcTemplate.update(...)) and log it alongside the yearWeek (using the existing
logger/processLogger) so operators can detect 0-updates and trigger alerts or
investigate aggregation failures; also consider logging at WARN when updated ==
0 and INFO otherwise to match the existing DELETE-result logging behavior.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/WeeklyRankingJobConfig.java`:
- Around line 51-60: The job lacks a JobParametersValidator so a missing
targetDate causes an NPE inside steps; update the weeklyRankingJob bean to
register a JobParametersValidator (e.g., DefaultJobParametersValidator or a
small custom validator) on the JobBuilder for JOB_NAME used in weeklyRankingJob
so the job fails fast on missing/invalid parameters before executing
weeklyRankingClearStep/weeklyRankingAggregateStep/weeklyRankingRankStep; ensure
the validator requires "targetDate" (and any other required params) and is wired
alongside the existing RunIdIncrementer and jobListener.
In
`@apps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.java`:
- Around line 32-33: The injected productRankWeeklyJpaRepository field in
WeeklyRankingJobE2ETest is unused; either use it to assert persisted results or
remove the injection. Locate the private field productRankWeeklyJpaRepository
and, if verifying DB state, query it (e.g., findAll/findBy... on
ProductRankWeeklyJpaRepository) and add assertions against expected weekly ranks
after the job runs; otherwise delete the field and any related unused imports to
keep the test clean.
- Around line 49-67: The test WeeklyRankingJobE2ETest currently only asserts
ExitStatus.COMPLETED; seed the DB with minimal ProductMetrics records before
launching the job (use existing repositories or a test fixture), run the job via
jobLauncherTestUtils.launchJob(jobParameters), then query
ProductRankWeeklyJpaRepository to assert expected records were created (count >
0 and/or specific field values) to ensure the aggregate/rank step actually
produced ranking data; use the same test setup/teardown pattern as other tests
to persist seed data and clean it up after the test.
In `@k6/06_ranking.js`:
- Around line 295-307: The weekly ranking scenario function
scenarioWeeklyRankingRead is missing the validation for each item's rank field;
update the check in scenarioWeeklyRankingRead to include a predicate that parses
JSON.parse(res.body).data.content and verifies every element has a defined
numeric (or non-null) rank (same style as scenarioRankingRead), add that
predicate to the check map so weeklyRankingFailRate reflects rank validation, or
if omission is intentional add a clear comment in scenarioWeeklyRankingRead
explaining why rank is skipped.
- Around line 293-307: Add a precondition note at the top-of-file comment
stating that the weekly/monthly ranking scenarios require the batch job that
populates the materialized view tables to be run beforehand; explicitly mention
that scenarioWeeklyRankingRead and scenarioMonthlyRankingRead query
mv_product_rank_weekly and mv_product_rank_monthly respectively and will return
empty results if the batch job hasn’t been executed, so include instructions to
run the batch job (or link to its trigger) before executing these scenarios.
In
`@modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankMonthly.java`:
- Around line 14-62: ProductRankMonthly duplicates ProductRankWeekly and shares
issues (direct LocalDateTime.now() usage and missing equals()/hashCode());
extract a `@MappedSuperclass` like ProductRankBase containing common fields (id,
productId, rankPosition, score, likeCount, orderCount, viewCount, updatedAt) and
common methods (assignRank), move shared static factory logic out or make a
protected factory in ProductRankBase, let concrete classes
(ProductRankMonthly/ProductRankWeekly) only define their period field and column
name (yearMonth/yearWeek) and implement an abstract getPeriod(); replace direct
LocalDateTime.now() in of() with an injectable Clock or accept updatedAt as a
parameter so tests can control time, and implement equals() and hashCode() in
the base class (or each entity) using id and productId to ensure correct
identity semantics.
In `@modules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankWeekly.java`:
- Around line 46-57: The factory method ProductRankWeekly.of sets updatedAt
using LocalDateTime.now(), which makes tests fragile and produces slightly
different timestamps across batch-created entities; change the API to accept a
LocalDateTime (or a Clock) from the caller and assign that to updatedAt (e.g.,
add an overload or new parameter to of to accept updatedAt or a Clock), update
any callers to pass a deterministic time in tests or a common clock during batch
runs, and ensure the existing field ProductRankWeekly.updatedAt is assigned from
that passed-in value rather than calling LocalDateTime.now() inside the method.
- Around line 14-62: The entity ProductRankWeekly lacks equals() and hashCode(),
which can cause incorrect behavior when instances are used in Sets/Maps;
implement equals() and hashCode() in the ProductRankWeekly class using the
primary key id (treating transient/new entities without id as not equal), e.g.,
have equals compare class and non-null id equality and have hashCode return a
stable value derived from id (handling null id safely), and ensure these methods
replace default Object implementations so collections behave correctly.
In `@scripts/seed-metrics.sql`:
- Line 17: TRUNCATE TABLE product_metrics can fail if there are foreign key
constraints; change the operation to a safe alternative: either replace
"TRUNCATE TABLE product_metrics" with "DELETE FROM product_metrics" to respect
FK constraints, or explicitly wrap the truncate with disabling FK checks (SET
FOREIGN_KEY_CHECKS=0; TRUNCATE TABLE product_metrics; SET FOREIGN_KEY_CHECKS=1;)
so future schema changes with FKs won't break the seed script; update the
statement involving product_metrics accordingly.
🪄 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: fd144b59-d661-4129-a1b2-a9efa5fdbc65
📒 Files selected for processing (27)
apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.javaapps/commerce-api/src/main/java/com/loopers/application/ranking/RankingType.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankMonthlyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/ProductRankWeeklyJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-api/src/test/java/com/loopers/application/coupon/CouponFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/ranking/RankingFacadeTest.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/step/MonthlyRankingClearTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingItemProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingRankTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingClearTasklet.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingItemProcessor.javaapps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingRankTasklet.javaapps/commerce-batch/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankMonthlyJpaRepository.javaapps/commerce-batch/src/main/java/com/loopers/infrastructure/ranking/ProductRankWeeklyJpaRepository.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.javaapps/commerce-batch/src/test/java/com/loopers/job/ranking/WeeklyRankingJobE2ETest.javak6/06_ranking.jsmodules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankMonthly.javamodules/jpa/src/main/java/com/loopers/domain/ranking/ProductRankWeekly.javascripts/seed-metrics.sql
Comment on lines
+148
to
159
| private String toYearWeek(String date) { | ||
| LocalDate localDate = LocalDate.parse(date, DATE_FORMATTER); | ||
| WeekFields weekFields = WeekFields.ISO; | ||
| int week = localDate.get(weekFields.weekOfWeekBasedYear()); | ||
| int year = localDate.get(weekFields.weekBasedYear()); | ||
| return year + "-W" + String.format("%02d", week); | ||
| } | ||
|
|
||
| private String toYearMonth(String date) { | ||
| LocalDate localDate = LocalDate.parse(date, DATE_FORMATTER); | ||
| return localDate.format(DateTimeFormatter.ofPattern("yyyy-MM")); | ||
| } |
There was a problem hiding this comment.
잘못된 date 형식 입력 시 DateTimeParseException이 그대로 전파된다.
date 파라미터가 yyyyMMdd 형식이 아닌 경우 LocalDate.parse에서 DateTimeParseException이 발생하여 500 에러로 응답된다. Controller 또는 Facade에서 적절한 예외 변환(400 Bad Request)이 필요하다.
🛡️ 예외 처리 추가 예시
private String toYearWeek(String date) {
+ try {
LocalDate localDate = LocalDate.parse(date, DATE_FORMATTER);
WeekFields weekFields = WeekFields.ISO;
int week = localDate.get(weekFields.weekOfWeekBasedYear());
int year = localDate.get(weekFields.weekBasedYear());
return year + "-W" + String.format("%02d", week);
+ } catch (DateTimeParseException e) {
+ throw new CoreException(ErrorType.INVALID_DATE_FORMAT, "date 형식이 올바르지 않습니다: " + date);
+ }
}🤖 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 148 - 159, Both toYearWeek and toYearMonth call
LocalDate.parse(date, DATE_FORMATTER) and let DateTimeParseException bubble up
causing 500 responses; wrap the parse call in a try-catch for
DateTimeParseException inside each method (toYearWeek and toYearMonth) and
convert it into a client-facing exception (e.g., throw new
IllegalArgumentException or a custom InvalidDateFormatException with a clear
message that includes the invalid input) so the controller can map it to a 400
Bad Request.
| @RequestParam(defaultValue = "1") int page | ||
| ) { | ||
| String rankingDate = (date != null) ? date : LocalDate.now().format(DATE_FORMATTER); | ||
| RankingType rankingType = RankingType.valueOf(type.toUpperCase()); |
There was a problem hiding this comment.
RankingType.valueOf() 예외 처리 누락으로 일관성 없는 에러 응답 발생 가능
type.toUpperCase()가 유효하지 않은 값(예: "invalid")일 경우 IllegalArgumentException이 발생하며, 이는 CoreException을 통한 표준 에러 처리 경로를 우회하여 일관성 없는 에러 응답을 반환한다.
운영 관점에서 클라이언트가 잘못된 type 파라미터를 전달했을 때 500 에러가 아닌 명확한 400 Bad Request 응답이 필요하다.
수정안
- RankingType rankingType = RankingType.valueOf(type.toUpperCase());
+ RankingType rankingType;
+ try {
+ rankingType = RankingType.valueOf(type.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new CoreException(ErrorType.INVALID_INPUT_VALUE, "Invalid ranking type: " + type);
+ }또는 RankingType에 정적 팩토리 메서드를 추가하여 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/interfaces/api/ranking/RankingV1Controller.java`
at line 34, 현재 RankingV1Controller에서 RankingType rankingType =
RankingType.valueOf(type.toUpperCase()); 호출 시 잘못된 type 값이면
IllegalArgumentException이 발생하여 CoreException 기반의 일관된 에러 처리 경로를 우회합니다; 수정 방법은
RankingV1Controller에서 valueOf 호출을 try-catch로 감싸고 IllegalArgumentException을 잡아
CoreException(또는 적절한 400 Bad Request 상태를 표현하는 예외)으로 변환해 던지거나, 대안으로 RankingType에
정적 팩토리 메서드(e.g., RankingType.from(String))를 추가해 유효성 검증 후 유효하지 않으면 CoreException을
던지도록 구현해 일관된 400 응답이 반환되게 하세요.
Comment on lines
+62
to
66
| CouponIssueRequest request = new CouponIssueRequest(1L, user.getId()); | ||
|
|
||
| given(userService.authenticate(loginId, rawPassword)).willReturn(user); | ||
| given(couponTemplateRepository.findById(1L)).willReturn(Optional.of(template)); | ||
| given(issuedCouponRepository.existsByUserIdAndCouponTemplateId(any(), any())).willReturn(false); | ||
| given(issuedCouponRepository.save(any())).willAnswer(inv -> inv.getArgument(0)); | ||
| given(couponService.createRequest(1L, user.getId())).willReturn(request); | ||
|
|
There was a problem hiding this comment.
인자 검증이 느슨해 잘못된 사용자/요청 매핑 회귀를 놓칠 수 있다
운영 관점에서 현재 스텁은 any() 사용과 불명확한 userId 설정 때문에 파사드가 잘못된 인자를 넘겨도 테스트가 통과할 수 있어 장애를 사전에 차단하지 못한다. 수정안으로 userId를 명시적으로 고정하고 eq(...) 기반 스텁/검증으로 호출 인자를 강하게 고정하는 것이 필요하다. 추가 테스트로 verify(couponService).createRequest(1L, userId) 및 잘못된 템플릿 ID 전달 시 실패 검증을 추가하는 것이 좋다.
권장 수정 예시
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+import org.springframework.test.util.ReflectionTestUtils;
User user = new User(loginId, "encoded", "홍길동", "19900101", "test@naver.com");
-CouponIssueRequest request = new CouponIssueRequest(1L, user.getId());
+ReflectionTestUtils.setField(user, "id", 10L);
+Long userId = 10L;
+CouponIssueRequest request = new CouponIssueRequest(1L, userId);
-given(couponService.createRequest(1L, user.getId())).willReturn(request);
+given(couponService.createRequest(eq(1L), eq(userId))).willReturn(request);
...
+verify(couponService).createRequest(1L, userId);
-given(couponService.createRequest(any(), any()))
+given(couponService.createRequest(eq(999L), eq(userId)))
.willThrow(new CoreException(ErrorType.NOT_FOUND, "쿠폰을 찾을 수 없습니다."));As per coding guidelines **/*Test*.java: "Mock 남용으로 의미가 약해지면 테스트 방향을 재정렬하도록 제안한다."
Also applies to: 83-84, 106-110
🤖 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/coupon/CouponFacadeTest.java`
around lines 62 - 66, The test uses loose stubbing allowing wrong args to
pass—replace any()/implicit user ids by declaring a concrete userId variable and
use eq(userId)/eq(1L) in the stubs for userService.authenticate(...) and
couponService.createRequest(...), then add explicit verifications like
verify(couponService).createRequest(eq(1L), eq(userId)) and add a negative test
case that asserts a call with an incorrect template id fails; update references
in this file to CouponIssueRequest, userService.authenticate,
couponService.createRequest and the verify(couponService).createRequest
assertion accordingly.
| CouponIssueInfo result = couponFacade.requestIssue(loginId, rawPassword, 1L); | ||
|
|
||
| // Assert | ||
| assertThat(result).isNotNull(); |
There was a problem hiding this comment.
성공 케이스 단언이 notNull만 있어 회귀 탐지력이 낮다
운영 관점에서 Line [71], Line [115]의 단언은 결과 객체가 비어 있지 않다는 사실만 확인하므로 필드 매핑 오류나 잘못된 상태 반환이 배포 전 탐지되지 않는다. 수정안으로 CouponIssueInfo의 핵심 필드(예: 요청 ID, 쿠폰 템플릿 ID, 사용자 ID, 상태)를 정확 값으로 단언해야 한다. 추가 테스트로 의도적으로 다른 요청 데이터를 반환하도록 스텁한 뒤 필드 단언이 실패하는지 확인하는 케이스를 넣는 것이 좋다.
As per coding guidelines **/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."
Also applies to: 115-115
🤖 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/coupon/CouponFacadeTest.java`
at line 71, The test currently only asserts result is not null in
CouponFacadeTest; replace and augment these weak assertions by asserting
CouponIssueInfo's core fields (e.g., requestId, couponTemplateId, userId,
status) equal the expected values from the fixture/stub used to produce the
result (reference the result variable and the method under test in
CouponFacadeTest), and add a complementary negative test that stubs the
dependency to return mismatched/different request data and verifies those field
assertions fail (or that the facade returns the correct error/exception) to
cover regression and edge/failure flows.
Comment on lines
+22
to
+30
| @Value("#{jobParameters['targetDate']}") | ||
| private String targetDate; | ||
|
|
||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| @Override | ||
| public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) { | ||
| String yearMonth = MonthlyRankingClearTasklet.toYearMonth(targetDate); | ||
| log.info("[MonthlyRankingRankTasklet] 랭킹 순위 부여 시작: yearMonth={}", yearMonth); |
There was a problem hiding this comment.
targetDate null 체크 누락 - WeeklyRankingRankTasklet과 동일한 이슈이다.
jobParameters['targetDate'] 누락 시 toYearMonth(null) 호출에서 NPE가 발생한다. Weekly tasklet과 동일하게 명시적 validation 추가가 필요하다.
🛡️ targetDate null 방어 코드 제안
`@Override`
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) {
+ if (targetDate == null || targetDate.isBlank()) {
+ throw new IllegalArgumentException("targetDate 파라미터가 필요합니다.");
+ }
String yearMonth = MonthlyRankingClearTasklet.toYearMonth(targetDate);🤖 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/step/MonthlyRankingRankTasklet.java`
around lines 22 - 30, The execute method in MonthlyRankingRankTasklet calls
MonthlyRankingClearTasklet.toYearMonth(targetDate) without validating
targetDate, which can cause an NPE; add an explicit null/empty check for the
injected field targetDate at the start of execute (same pattern as
WeeklyRankingRankTasklet) and if missing, log an error via log.error and fail
the step early by throwing an IllegalArgumentException or returning an
appropriate failure RepeatStatus; update references inside execute and ensure
toYearMonth is only called when targetDate is non-null/non-empty.
Comment on lines
+36
to
+39
| int deleted = jdbcTemplate.update( | ||
| "DELETE FROM mv_product_rank_weekly WHERE period_week = ?", | ||
| yearWeek | ||
| ); |
There was a problem hiding this comment.
선삭제 방식은 배치 실패 시 랭킹 공백을 만든다.
Line 36에서 기존 데이터를 먼저 삭제하면 Aggregate 또는 Rank 스텝 장애 시 해당 주차 랭킹이 0건으로 노출되어 운영 장애로 이어진다. staging 테이블 적재/랭크 완료 후 본 테이블 교체(또는 run_id 기반 활성 포인터 전환) 방식으로 원자적 전환을 적용하는 것이 필요하다. 추가 테스트로, 기존 주간 데이터가 있는 상태에서 Aggregate 스텝을 강제로 실패시키고 기존 데이터가 유지되는 E2E 케이스를 반드시 넣는 것이 좋다.
🤖 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/step/WeeklyRankingClearTasklet.java`
around lines 36 - 39, The current WeeklyRankingClearTasklet deletes existing
weekly rows via jdbcTemplate.update("DELETE FROM mv_product_rank_weekly WHERE
period_week = ?", yearWeek) which can leave a gap if downstream Aggregate/Rank
steps fail; change flow to use a staging table and atomic swap (or a run_id
active-pointer) instead: stop deleting in WeeklyRankingClearTasklet, write new
results into a staging table (e.g., mv_product_rank_weekly_staging) keyed by
period_week and run_id, then after the Aggregate/Rank steps succeed perform an
atomic replace of mv_product_rank_weekly from staging (via transactional
DELETE/INSERT, table rename, or flip an active flag by run_id) in a final
commit/step (refer to WeeklyRankingClearTasklet, the Aggregate/Rank step names,
and any run_id handling), and add an E2E test that forces Aggregate step failure
while prior week data exists to assert the original mv_product_rank_weekly
remains unchanged.
Comment on lines
+22
to
+30
| @Value("#{jobParameters['targetDate']}") | ||
| private String targetDate; | ||
|
|
||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| @Override | ||
| public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) { | ||
| String yearWeek = WeeklyRankingClearTasklet.toYearWeek(targetDate); | ||
| log.info("[WeeklyRankingRankTasklet] 랭킹 순위 부여 시작: yearWeek={}", yearWeek); |
There was a problem hiding this comment.
targetDate null 체크 누락으로 NPE 발생 가능하다.
jobParameters['targetDate']가 누락된 채로 job이 실행되면 toYearWeek(null) 호출 시 LocalDate.parse(null, ...)에서 NPE가 발생한다. E2E 테스트에서 파라미터 누락 시 FAILED 상태를 기대하지만, 예외 메시지가 불명확하여 운영 시 디버깅이 어렵다.
- 명시적 validation과 의미 있는 예외 메시지를 추가하거나,
JobParametersValidator를 job 설정에 등록하여 early fail을 보장해야 한다.
🛡️ targetDate null 방어 코드 제안
`@Override`
public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) {
+ if (targetDate == null || targetDate.isBlank()) {
+ throw new IllegalArgumentException("targetDate 파라미터가 필요합니다.");
+ }
String yearWeek = WeeklyRankingClearTasklet.toYearWeek(targetDate);
log.info("[WeeklyRankingRankTasklet] 랭킹 순위 부여 시작: yearWeek={}", yearWeek);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/WeeklyRankingRankTasklet.java`
around lines 22 - 30, The code calls
WeeklyRankingClearTasklet.toYearWeek(targetDate) without validating the injected
field targetDate, risking an NPE; update WeeklyRankingRankTasklet.execute to
explicitly check that targetDate is not null/blank and throw a clear
IllegalArgumentException (or JobParametersInvalidException) with a descriptive
message (e.g. "Missing job parameter 'targetDate'") before calling toYearWeek,
and alternatively register a JobParametersValidator for the job configuration to
enforce presence of 'targetDate' at job startup; reference the targetDate field,
execute(...) method, and WeeklyRankingClearTasklet.toYearWeek when making the
change.
Comment on lines
+35
to
+47
| @DisplayName("targetDate 파라미터 없이 실행하면 monthlyRankingJob이 실패한다.") | ||
| @Test | ||
| void fails_whenTargetDateParamIsMissing() throws Exception { | ||
| // Arrange | ||
| jobLauncherTestUtils.setJob(job); | ||
|
|
||
| // Act | ||
| var jobExecution = jobLauncherTestUtils.launchJob(); | ||
|
|
||
| // Assert | ||
| assertThat(jobExecution.getExitStatus().getExitCode()) | ||
| .isEqualTo(ExitStatus.FAILED.getExitCode()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# MonthlyRankingJobConfig에 validator 설정 여부 확인
rg -n "validator|JobParametersValidator" apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# MonthlyRankingClearTasklet 파일 위치 찾기
find apps/commerce-batch -name "*MonthlyRankingClearTasklet*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 185
🏁 Script executed:
# MonthlyRankingJobConfig 전체 내용 확인
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5575
🏁 Script executed:
# 테스트 클래스 전체 내용 확인
cat -n apps/commerce-batch/src/test/java/com/loopers/job/ranking/MonthlyRankingJobE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2969
🏁 Script executed:
cat -n apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/step/MonthlyRankingClearTasklet.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2036
JobParametersValidator 미설정으로 인한 불안정한 테스트 및 불완전한 검증
현재 fails_whenTargetDateParamIsMissing 테스트는 targetDate 누락 시 FAILED 상태를 기대하지만, 명시적 파라미터 검증 메커니즘이 없다. MonthlyRankingClearTasklet.toYearMonth() 메서드에서 null 체크 없이 LocalDate.parse(targetDate, ...)를 호출하므로, 파라미터 누락 시 NullPointerException이 발생하여 간접적으로 FAILED 상태가 된다. 이는 파라미터 검증 실패와 런타임 예외를 구분할 수 없으며, 향후 null-safe 처리 추가 시 테스트가 예기치 않게 실패할 수 있다.
또한 성공 케이스 테스트 success_whenTargetDateParamIsGiven은 Job 완료 상태만 확인하고 실제 처리된 데이터를 검증하지 않는다. productRankMonthlyJpaRepository가 주입되었으나 테스트에서 사용되지 않으므로, 실제 랭킹 데이터가 저장되었는지 확인할 수 없다.
MonthlyRankingJobConfig에 JobParametersValidator 추가:
`@Bean`(JOB_NAME)
public Job monthlyRankingJob() {
return new JobBuilder(JOB_NAME, jobRepository)
.incrementer(new RunIdIncrementer())
.validator(new DefaultJobParametersValidator(
new String[]{"targetDate"},
new String[]{}
))
.start(monthlyRankingClearStep())
.next(monthlyRankingAggregateStep())
.next(monthlyRankingRankStep())
.listener(jobListener)
.build();
}성공 케이스 테스트에 데이터 검증 추가:
`@DisplayName`("targetDate 파라미터와 함께 실행하면 monthlyRankingJob이 성공한다.")
`@Test`
void success_whenTargetDateParamIsGiven() throws Exception {
// Arrange
jobLauncherTestUtils.setJob(job);
var jobParameters = new JobParametersBuilder()
.addString("targetDate", "20260416")
.toJobParameters();
// Act
var jobExecution = jobLauncherTestUtils.launchJob(jobParameters);
// Assert
assertAll(
() -> assertThat(jobExecution.getExitStatus().getExitCode())
.isEqualTo(ExitStatus.COMPLETED.getExitCode()),
() -> assertThat(productRankMonthlyJpaRepository.findAll())
.isNotEmpty()
);
}🤖 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/MonthlyRankingJobE2ETest.java`
around lines 35 - 47, Add explicit job parameter validation in
MonthlyRankingJobConfig by registering a DefaultJobParametersValidator in the
monthlyRankingJob() bean (require "targetDate") so missing parameters produce a
JobParametersInvalidException instead of a NPE from
MonthlyRankingClearTasklet.toYearMonth; then update the tests: keep
fails_whenTargetDateParamIsMissing but rely on the validator to trigger failure,
and modify success_whenTargetDateParamIsGiven to launch the job with
JobParametersBuilder.addString("targetDate", "20260416") and assert both
ExitStatus.COMPLETED and that productRankMonthlyJpaRepository.findAll() is not
empty to verify data persistence.
Comment on lines
+49
to
+67
| @DisplayName("targetDate 파라미터와 함께 실행하면 monthlyRankingJob이 성공한다.") | ||
| @Test | ||
| void success_whenTargetDateParamIsGiven() throws Exception { | ||
| // Arrange | ||
| jobLauncherTestUtils.setJob(job); | ||
| var jobParameters = new JobParametersBuilder() | ||
| .addString("targetDate", "20260416") | ||
| .toJobParameters(); | ||
|
|
||
| // Act | ||
| var jobExecution = jobLauncherTestUtils.launchJob(jobParameters); | ||
|
|
||
| // Assert | ||
| assertAll( | ||
| () -> assertThat(jobExecution).isNotNull(), | ||
| () -> assertThat(jobExecution.getExitStatus().getExitCode()) | ||
| .isEqualTo(ExitStatus.COMPLETED.getExitCode()) | ||
| ); | ||
| } |
There was a problem hiding this comment.
성공 테스트에서 실제 데이터 검증 누락
success_whenTargetDateParamIsGiven 테스트는 ExitStatus.COMPLETED만 확인하고, 실제로 ProductRankMonthly 데이터가 올바르게 생성되었는지 검증하지 않는다. productRankMonthlyJpaRepository가 주입되어 있으나 사용되지 않는다.
배치 Job이 완료되었더라도 데이터가 올바르게 적재되지 않을 수 있으므로, 최소한 다음을 검증해야 한다:
- 생성된 row 수
yearMonth값 일치 여부rankPosition할당 여부
추가 검증 예시
// Assert
assertAll(
() -> assertThat(jobExecution.getExitStatus().getExitCode())
.isEqualTo(ExitStatus.COMPLETED.getExitCode()),
() -> {
var results = productRankMonthlyJpaRepository.findByYearMonthOrderByRankPosition("2026-04");
assertThat(results).isNotEmpty();
assertThat(results.get(0).getRankPosition()).isGreaterThan(0);
}
);🤖 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/MonthlyRankingJobE2ETest.java`
around lines 49 - 67, The test success_whenTargetDateParamIsGiven currently only
asserts ExitStatus.COMPLETED; enhance it to query
productRankMonthlyJpaRepository after launching the job (using
jobLauncherTestUtils and the existing JobParametersBuilder) and assert that rows
were created, that each result has yearMonth equal to "2026-04" (or derived from
the targetDate param), and that rankPosition values are assigned (e.g., > 0);
locate these changes in the same test method and add assertions that check
results.size() > 0, the yearMonth field on returned ProductRankMonthly entities,
and rankPosition presence/validity (using
productRankMonthlyJpaRepository.findByYearMonthOrderByRankPosition or equivalent
repository method).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
🧭 Context & Decision
일간 ZSET vs 주간/월간 MV 분리 이유
🏗️ Design Overview
🔄 Batch Flow
1. Clear Tasklet
2. Aggregate Step
3. Rank Tasklet
✅ Requirements Checklist
📊 Load Test (k6)
주요 결과
응답 시간
ZREVRANK 오버헤드