Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@
import com.loopers.domain.brand.BrandService;
import com.loopers.domain.product.Product;
import com.loopers.domain.product.ProductService;
import com.loopers.domain.ranking.MvRankingRepository;
import com.loopers.domain.ranking.RankedProduct;
import com.loopers.domain.ranking.RankingRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;

import java.time.DayOfWeek;
import java.time.LocalDate;
import java.time.YearMonth;
import java.time.format.DateTimeFormatter;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand All @@ -18,18 +23,55 @@
@RequiredArgsConstructor
public class RankingFacade {

private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyyMMdd");

private final RankingRepository rankingRepository;
private final MvRankingRepository mvRankingRepository;
private final ProductService productService;
private final BrandService brandService;

public RankingPageInfo getRankings(String date, int page, int size) {
public RankingPageInfo getRankings(String period, String date, int page, int size) {
int offset = (page - 1) * size;
List<RankedProduct> ranked = rankingRepository.findTopN(date, offset, size);

List<RankedProduct> ranked;
long totalSize;

switch (period) {
case "weekly" -> {
LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT);
LocalDate periodStart = targetDate.with(DayOfWeek.MONDAY);
LocalDate periodEnd = targetDate.with(DayOfWeek.SUNDAY);
ranked = mvRankingRepository.findWeeklyRanking(periodStart, periodEnd, offset, size);
totalSize = mvRankingRepository.countWeeklyRanking(periodStart, periodEnd);
}
case "monthly" -> {
LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT);
YearMonth yearMonth = YearMonth.from(targetDate);
LocalDate periodStart = yearMonth.atDay(1);
LocalDate periodEnd = yearMonth.atEndOfMonth();
ranked = mvRankingRepository.findMonthlyRanking(periodStart, periodEnd, offset, size);
totalSize = mvRankingRepository.countMonthlyRanking(periodStart, periodEnd);
}
default -> {
// daily — Redis ZSET
ranked = rankingRepository.findTopN(date, offset, size);
totalSize = rankingRepository.getTotalSize(date);
}
}
Comment on lines +39 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

날짜 파싱 예외 처리 미흡 - 500 에러 노출 가능

LocalDate.parse(date, DATE_FORMAT)에서 잘못된 형식의 date가 입력되면 DateTimeParseException이 발생하여 500 에러로 노출된다.

운영 영향: 사용자에게 불친절한 에러 메시지 노출, 에러 모니터링 시 불필요한 노이즈 발생.

수정안: CoreException으로 변환하여 일관된 에러 응답 제공. (이 레포지토리의 통합 에러 처리 패턴 준수)

🛡️ 에러 처리 추가
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
+import java.time.format.DateTimeParseException;

     case "weekly" -> {
-        LocalDate targetDate = LocalDate.parse(date, DATE_FORMAT);
+        LocalDate targetDate = parseDate(date);
         LocalDate periodStart = targetDate.with(DayOfWeek.MONDAY);
         // ...
     }
     
+private LocalDate parseDate(String date) {
+    try {
+        return LocalDate.parse(date, DATE_FORMAT);
+    } catch (DateTimeParseException e) {
+        throw new CoreException(ErrorType.INVALID_DATE_FORMAT);
+    }
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 39 - 60, The date parsing in RankingFacade (calls to
LocalDate.parse(date, DATE_FORMAT) used in the "weekly" and "monthly" branches)
can throw DateTimeParseException and currently bubbles up as a 500; wrap the
parse in a try/catch for DateTimeParseException and convert it to the
repository's unified error type by throwing a CoreException with a clear message
(e.g., "invalid date format" and include the original exception as the cause);
apply this handling to both places where LocalDate.parse(date, DATE_FORMAT) is
used so malformed dates produce a controlled CoreException response instead of
an uncaught 500.


if (ranked.isEmpty()) {
return new RankingPageInfo(date, Collections.emptyList(), 0, page, size);
}

return buildPageInfo(date, ranked, totalSize, offset, page, size);
}

public Long getProductRank(String date, Long productId) {
Long rank = rankingRepository.findRank(date, productId);
return rank != null ? rank + 1 : null;
}

private RankingPageInfo buildPageInfo(String date, List<RankedProduct> ranked, long totalSize, int offset, int page, int size) {
List<Long> productIds = ranked.stream().map(RankedProduct::productId).toList();
List<Product> products = productService.getByIds(productIds);
Map<Long, Product> productMap = products.stream()
Expand All @@ -52,12 +94,6 @@ public RankingPageInfo getRankings(String date, int page, int size) {
items.add(RankingInfo.of(rank++, rp.score(), product, brandName));
}

long totalSize = rankingRepository.getTotalSize(date);
return new RankingPageInfo(date, items, totalSize, page, size);
}

public Long getProductRank(String date, Long productId) {
Long rank = rankingRepository.findRank(date, productId);
return rank != null ? rank + 1 : null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.loopers.domain.ranking;

import jakarta.persistence.*;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

import java.time.LocalDate;
import java.time.ZonedDateTime;

@Entity
@Table(name = "mv_product_rank_monthly")
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class MvProductRankMonthly {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(name = "product_id", nullable = false)
private Long productId;

@Column(name = "rank_position", nullable = false)
private Integer rankPosition;

@Column(name = "score", nullable = false)
private Double score;

@Column(name = "period_start", nullable = false)
private LocalDate periodStart;

@Column(name = "period_end", nullable = false)
private LocalDate periodEnd;

@Column(name = "created_at", nullable = false, updatable = false)
private ZonedDateTime createdAt;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.loopers.domain.ranking;

import jakarta.persistence.*;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

import java.time.LocalDate;
import java.time.ZonedDateTime;

@Entity
@Table(name = "mv_product_rank_weekly")
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class MvProductRankWeekly {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(name = "product_id", nullable = false)
private Long productId;

@Column(name = "rank_position", nullable = false)
private Integer rankPosition;

@Column(name = "score", nullable = false)
private Double score;

@Column(name = "period_start", nullable = false)
private LocalDate periodStart;

@Column(name = "period_end", nullable = false)
private LocalDate periodEnd;

@Column(name = "created_at", nullable = false, updatable = false)
private ZonedDateTime createdAt;
}
Comment on lines +14 to +38
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

생성자와 @PrePersist 콜백 누락으로 배치 저장 실패 예상

MvProductRankMonthly에는 비즈니스 필드를 초기화하는 생성자와 createdAt을 설정하는 @PrePersist 콜백이 구현되어 있으나, 이 엔티티에는 둘 다 누락되어 있다.

운영 관점:

  • 배치 Writer에서 new MvProductRankWeekly(...) 호출 시 컴파일 에러 또는 런타임 에러 발생한다.
  • createdAt이 null인 상태로 persist 시도 시 NOT NULL constraint 위반으로 배치 Job 실패한다.
🐛 수정안
 `@Column`(name = "created_at", nullable = false, updatable = false)
 private ZonedDateTime createdAt;
+
+public MvProductRankWeekly(Long productId, Integer rankPosition, Double score, LocalDate periodStart, LocalDate periodEnd) {
+    this.productId = productId;
+    this.rankPosition = rankPosition;
+    this.score = score;
+    this.periodStart = periodStart;
+    this.periodEnd = periodEnd;
+}
+
+@PrePersist
+private void prePersist() {
+    this.createdAt = ZonedDateTime.now();
+}

추가 테스트: 배치 Job 통합 테스트에서 주간 랭킹 저장 후 createdAt 필드가 정상 설정되었는지 검증 필요하다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/ranking/MvProductRankWeekly.java`
around lines 14 - 38, The MvProductRankWeekly entity is missing the business
constructor and the `@PrePersist` callback that sets createdAt (causing
compile/runtime errors and NOT NULL violations); add a public constructor on
MvProductRankWeekly that accepts and assigns productId, rankPosition, score,
periodStart, and periodEnd (keeping the existing protected no-args constructor),
and add a `@PrePersist-annotated` method (e.g., initCreatedAt or prePersist) that
sets createdAt = ZonedDateTime.now() if createdAt is null, mirroring the pattern
used in MvProductRankMonthly so batch writers can new MvProductRankWeekly(...)
and persists succeed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.loopers.domain.ranking;

import java.time.LocalDate;
import java.util.List;

public interface MvRankingRepository {

List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);

List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);

long countWeeklyRanking(LocalDate periodStart, LocalDate periodEnd);

long countMonthlyRanking(LocalDate periodStart, LocalDate periodEnd);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.loopers.infrastructure.ranking;

import com.loopers.domain.ranking.MvProductRankMonthly;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;

import java.time.LocalDate;
import java.util.List;

public interface MvProductRankMonthlyJpaRepository extends JpaRepository<MvProductRankMonthly, Long> {
List<MvProductRankMonthly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
LocalDate periodStart, LocalDate periodEnd, Pageable pageable);
long countByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.loopers.infrastructure.ranking;

import com.loopers.domain.ranking.MvProductRankWeekly;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;

import java.time.LocalDate;
import java.util.List;

public interface MvProductRankWeeklyJpaRepository extends JpaRepository<MvProductRankWeekly, Long> {
List<MvProductRankWeekly> findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
LocalDate periodStart, LocalDate periodEnd, Pageable pageable);
long countByPeriodStartAndPeriodEnd(LocalDate periodStart, LocalDate periodEnd);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.loopers.infrastructure.ranking;

import com.loopers.domain.ranking.MvRankingRepository;
import com.loopers.domain.ranking.RankedProduct;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Repository;

import java.time.LocalDate;
import java.util.List;

@Repository
@RequiredArgsConstructor
public class MvRankingRepositoryImpl implements MvRankingRepository {

private final MvProductRankWeeklyJpaRepository weeklyRepository;
private final MvProductRankMonthlyJpaRepository monthlyRepository;

@Override
public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) {
return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
periodStart, periodEnd, PageRequest.of(offset / size, size)
).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
Comment on lines +20 to +23
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

size=0 입력 시 ArithmeticException 발생 가능

offset / size 연산에서 size=0이면 ArithmeticException이 발생한다. 상위 레이어에서 검증이 있더라도 방어적 코딩이 필요하다.

🛡️ 방어 코드 추가 제안
 `@Override`
 public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) {
+    if (size <= 0) {
+        return Collections.emptyList();
+    }
     return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
             periodStart, periodEnd, PageRequest.of(offset / size, size)
     ).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.java`
around lines 20 - 23, The method findWeeklyRanking currently does offset / size
which will throw ArithmeticException when size == 0; add defensive validation at
the start of findWeeklyRanking to ensure size > 0 (and optionally offset >= 0)
and throw a clear IllegalArgumentException (or custom BadRequest) if the check
fails; then compute the page index and call
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc with
PageRequest.of(pageIndex, size) as before. Ensure you reference the same method
names (findWeeklyRanking,
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc,
PageRequest.of) so the guard is applied before any division happens.

}
Comment on lines +19 to +24
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

offset → page 변환 로직 오류 - 페이지네이션 결과 불일치 가능

PageRequest.of(offset / size, size) 변환은 offsetsize의 배수일 때만 정확하다.

문제 시나리오:

  • offset=5, size=10page=0 → 실제로는 09번째 항목 반환 (기대: 514번째)
  • offset=15, size=10page=1 → 실제로는 1019번째 항목 반환 (기대: 1524번째)

운영 영향: API 호출자가 예상과 다른 랭킹 데이터를 받게 되어 UX 불일치 발생.

수정안: 호출부(RankingFacade)에서 항상 offset = (page - 1) * size로 계산하므로 현재는 문제없으나, 인터페이스 계약과 구현의 의미론적 불일치가 있다. 인터페이스를 (int page, int size)로 변경하거나, native query로 OFFSET/LIMIT 직접 사용을 권장한다.

♻️ 인터페이스 시그니처 변경 제안
 public interface MvRankingRepository {
-    List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);
-    List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size);
+    List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size);
+    List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size);
     long countWeeklyRanking(LocalDate periodStart, LocalDate periodEnd);
     long countMonthlyRanking(LocalDate periodStart, LocalDate periodEnd);
 }

그리고 구현체:

 public List<RankedProduct> findWeeklyRanking(LocalDate periodStart, LocalDate periodEnd, int page, int size) {
     return weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
-            periodStart, periodEnd, PageRequest.of(offset / size, size)
+            periodStart, periodEnd, PageRequest.of(page, size)
     ).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/ranking/MvRankingRepositoryImpl.java`
around lines 19 - 24, findWeeklyRanking currently treats the incoming offset as
a page by doing PageRequest.of(offset / size, size), which misbehaves for
non-multiple offsets; fix by changing the contract to accept page + size (or
alternatively implement a query that supports OFFSET/LIMIT directly).
Concretely: update the method signature of findWeeklyRanking(LocalDate
periodStart, LocalDate periodEnd, int page, int size) and call
PageRequest.of(page, size) inside the implementation (update all call sites such
as RankingFacade which currently computes offset to pass the correct page), or
if you prefer keeping an absolute offset, replace the repository call with a
native query that applies SQL OFFSET :offset LIMIT :size and map results to
RankedProduct. Ensure references to
weeklyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc and any
callers are updated to the new semantic.


@Override
public List<RankedProduct> findMonthlyRanking(LocalDate periodStart, LocalDate periodEnd, int offset, int size) {
return monthlyRepository.findByPeriodStartAndPeriodEndOrderByRankPositionAsc(
periodStart, periodEnd, PageRequest.of(offset / size, size)
).stream().map(mv -> new RankedProduct(mv.getProductId(), mv.getScore())).toList();
}

@Override
public long countWeeklyRanking(LocalDate periodStart, LocalDate periodEnd) {
return weeklyRepository.countByPeriodStartAndPeriodEnd(periodStart, periodEnd);
}

@Override
public long countMonthlyRanking(LocalDate periodStart, LocalDate periodEnd) {
return monthlyRepository.countByPeriodStartAndPeriodEnd(periodStart, periodEnd);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ public class RankingV1Controller {

@GetMapping
public ApiResponse<RankingV1Dto.PageResponse> getRankings(
@RequestParam(defaultValue = "daily") String period,
@RequestParam(required = false) String date,
@RequestParam(defaultValue = "1") int page,
@RequestParam(defaultValue = "20") int size
) {
String targetDate = (date != null && !date.isBlank()) ? date : LocalDate.now().format(DATE_FORMAT);
RankingPageInfo pageInfo = rankingFacade.getRankings(targetDate, page, size);
RankingPageInfo pageInfo = rankingFacade.getRankings(period, targetDate, page, size);
return ApiResponse.success(RankingV1Dto.PageResponse.from(pageInfo));
Comment on lines 22 to 30
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: CoreException 및 ErrorType 사용 패턴 확인
# 기존 Controller에서 유효성 검증 후 CoreException throw 패턴이 있는지 확인

rg -n "throw new CoreException" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ -C2

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# RankingV1Controller 전체 코드 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 1541


🏁 Script executed:

# RankingFacade 구현 확인
cat -n apps/commerce-api/src/main/java/com/loopers/application/facade/RankingFacade.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 198


🏁 Script executed:

# 다른 Controller에서의 파라미터 검증 패턴 확인
rg -A5 "@RequestParam" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ | head -50

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 6730


🏁 Script executed:

# CoreException 사용 패턴 검색 (전체 codebase)
rg "throw new CoreException" --type=java -C2 | head -30

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 2934


🏁 Script executed:

# Bean Validation 또는 검증 로직이 있는 컨트롤러 예제 확인
rg "@Valid|@Pattern|@NotNull" --type=java apps/commerce-api/src/main/java/com/loopers/interfaces/api/ | head -20

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# RankingFacade 파일 위치 찾기
find apps/commerce-api -name "*RankingFacade*" -type f

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 167


🏁 Script executed:

# getRankings 메서드 검색
rg "def getRankings|void getRankings|getRankings(" --type=java apps/commerce-api -A8 | head -40

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 225


🏁 Script executed:

# period 파라미터를 다루는 로직 전체 찾기
rg "period" --type=java apps/commerce-api -B2 -A2 | grep -A3 -B3 "getRankings\|switch\|period"

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 13429


🏁 Script executed:

cat -n apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 4914


🏁 Script executed:

# ErrorType 열거형 확인
rg "enum ErrorType|INVALID_PARAMETER" --type=java -A2 apps/commerce-api | head -30

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 470


🏁 Script executed:

# RankingV1Controller 테스트 파일 확인
find apps/commerce-api -name "*RankingV1ControllerTest*" -o -name "*RankingTest*" -type f

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# 다른 컨트롤러의 파라미터 검증 테스트 패턴
rg "invalid.*parameter|Invalid.*parameter" --type=java -B2 -A2 apps/commerce-api | head -40

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# ErrorType 전체 내용 확인
cat -n apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 1046


🏁 Script executed:

# 파라미터 검증으로 CoreException을 throw하는 예제 찾기
rg "throw new CoreException.*INVALID\|throw new CoreException.*BAD_REQUEST" --type=java -B2 -A1 apps/commerce-api | head -20

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


period 파라미터 유효성 검증 누락

period 파라미터가 daily, weekly, monthly 외의 값(예: Weekly, DAILY, 오타)인 경우 RankingFacade의 switch default로 인해 조용히 daily 결과를 반환한다.

운영 관점:

  • 사용자가 오타나 대소문자 실수로 주간 랭킹을 요청했으나 일간 랭킹을 받는 상황이 발생할 수 있다.
  • API 응답만으로는 어떤 period 데이터가 반환되었는지 구분이 어려워 디버깅이 곤란하다.
  • 잘못된 입력에 대해 400 에러를 반환하는 것이 RESTful API 설계 원칙에 부합한다.
♻️ 명시적 검증 추가 권장

Controller에서 검증 후 CoreException throw:

+import java.util.Set;
+
 `@RestController`
 `@RequestMapping`("/api/v1/rankings")
 `@RequiredArgsConstructor`
 public class RankingV1Controller {

     private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyyMMdd");
+    private static final Set<String> VALID_PERIODS = Set.of("daily", "weekly", "monthly");

     private final RankingFacade rankingFacade;

     `@GetMapping`
     public ApiResponse<RankingV1Dto.PageResponse> getRankings(
             `@RequestParam`(defaultValue = "daily") String period,
             `@RequestParam`(required = false) String date,
             `@RequestParam`(defaultValue = "1") int page,
             `@RequestParam`(defaultValue = "20") int size
     ) {
+        if (!VALID_PERIODS.contains(period)) {
+            throw new CoreException(ErrorType.BAD_REQUEST, "지원하지 않는 period입니다: " + period);
+        }
         String targetDate = (date != null && !date.isBlank()) ? date : LocalDate.now().format(DATE_FORMAT);

추가 테스트: 잘못된 period 값 입력 시 400 응답과 에러 메시지를 검증하는 테스트 케이스 추가 필요하다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java`
around lines 22 - 30, getRankings in RankingV1Controller currently accepts any
string for the period param and silently falls back to daily; add explicit
validation at the start of RankingV1Controller.getRankings to accept only
"daily", "weekly", or "monthly" (case-sensitive or normalize to lower-case
first), and if invalid throw a CoreException (or return a 400 ApiResponse) with
a clear message like "Invalid period: must be one of [daily, weekly, monthly]";
ensure you reference the incoming period value in the message, do not alter
rankingFacade.getRankings behavior, and add a unit/integration test that posts
an invalid period (e.g., "Weekly" or "DAILY" or "foo") and asserts a 400
response and the proper error message.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package com.loopers.batch.job.ranking;

import com.loopers.batch.listener.JobListener;
import com.loopers.batch.listener.StepMonitorListener;
import com.loopers.domain.metrics.ProductMetrics;
import com.loopers.domain.ranking.MvProductRankMonthly;
import com.loopers.infrastructure.ranking.MvProductRankMonthlyJpaRepository;
import jakarta.persistence.EntityManagerFactory;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.batch.core.Job;
import org.springframework.batch.core.Step;
import org.springframework.batch.core.configuration.annotation.JobScope;
import org.springframework.batch.core.configuration.annotation.StepScope;
import org.springframework.batch.core.job.builder.JobBuilder;
import org.springframework.batch.core.launch.support.RunIdIncrementer;
import org.springframework.batch.core.repository.JobRepository;
import org.springframework.batch.core.step.builder.StepBuilder;
import org.springframework.batch.item.ItemProcessor;
import org.springframework.batch.item.ItemWriter;
import org.springframework.batch.item.database.JpaPagingItemReader;
import org.springframework.batch.item.database.builder.JpaPagingItemReaderBuilder;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.transaction.PlatformTransactionManager;

import java.time.LocalDate;
import java.time.YearMonth;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;

@Slf4j
@ConditionalOnProperty(name = "spring.batch.job.name", havingValue = MonthlyRankingJobConfig.JOB_NAME)
@RequiredArgsConstructor
@Configuration
public class MonthlyRankingJobConfig {

public static final String JOB_NAME = "monthlyRankingJob";
private static final String STEP_NAME = "monthlyRankingStep";
private static final int CHUNK_SIZE = 100;
private static final int TOP_N = 100;

private static final double WEIGHT_VIEW = 0.1;
private static final double WEIGHT_LIKE = 0.2;
private static final double WEIGHT_ORDER = 0.7;

private final JobRepository jobRepository;
private final PlatformTransactionManager transactionManager;
private final JobListener jobListener;
private final StepMonitorListener stepMonitorListener;
private final EntityManagerFactory entityManagerFactory;
private final MvProductRankMonthlyJpaRepository mvRepository;

@Bean(JOB_NAME)
public Job monthlyRankingJob() {
return new JobBuilder(JOB_NAME, jobRepository)
.incrementer(new RunIdIncrementer())
.start(monthlyRankingStep())
.listener(jobListener)
.build();
}

@JobScope
@Bean(STEP_NAME)
public Step monthlyRankingStep() {
return new StepBuilder(STEP_NAME, jobRepository)
.<ProductMetrics, RankedProductScore>chunk(CHUNK_SIZE, transactionManager)
.reader(monthlyProductMetricsReader())
.processor(monthlyRankingScoreProcessor())
.writer(monthlyRankingWriter(null))
.listener(stepMonitorListener)
.build();
}

@StepScope
@Bean
public JpaPagingItemReader<ProductMetrics> monthlyProductMetricsReader() {
return new JpaPagingItemReaderBuilder<ProductMetrics>()
.name("monthlyProductMetricsReader")
.entityManagerFactory(entityManagerFactory)
.queryString("SELECT pm FROM ProductMetrics pm ORDER BY pm.id ASC")
.pageSize(CHUNK_SIZE)
.build();
}

@StepScope
@Bean
public ItemProcessor<ProductMetrics, RankedProductScore> monthlyRankingScoreProcessor() {
return metrics -> {
double score = metrics.getViewCount() * WEIGHT_VIEW
+ metrics.getLikesCount() * WEIGHT_LIKE
+ metrics.getOrderCount() * WEIGHT_ORDER;
return new RankedProductScore(metrics.getProductId(), score);
};
}

@StepScope
@Bean
public ItemWriter<RankedProductScore> monthlyRankingWriter(
@Value("#{jobParameters['requestDate']}") String requestDate
) {
return items -> {
LocalDate date = LocalDate.parse(requestDate, DateTimeFormatter.ofPattern("yyyy-MM-dd"));
YearMonth yearMonth = YearMonth.from(date);
LocalDate periodStart = yearMonth.atDay(1);
LocalDate periodEnd = yearMonth.atEndOfMonth();

List<RankedProductScore> allScores = new ArrayList<>(items.getItems());
allScores.sort(Comparator.comparingDouble(RankedProductScore::score).reversed());

List<RankedProductScore> topN = allScores.stream().limit(TOP_N).toList();

mvRepository.deleteByPeriodStartAndPeriodEnd(periodStart, periodEnd);

int rank = 1;
for (RankedProductScore scored : topN) {
mvRepository.save(new MvProductRankMonthly(
scored.productId(), rank++, scored.score(), periodStart, periodEnd
));
}

log.info("월간 랭킹 저장 완료: period={} ~ {}, count={}", periodStart, periodEnd, topN.size());
};
}
Comment on lines +101 to +128
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

WeeklyRankingJobConfig와 동일한 치명적 로직 오류

WeeklyRankingJobConfig에서 지적한 chunk 단위 Top 100 계산 문제가 동일하게 존재한다. 각 chunk 실행 시 이전 데이터가 삭제되어 최종적으로 마지막 chunk의 데이터만 남는다.

동일한 수정이 필요하다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/commerce-batch/src/main/java/com/loopers/batch/job/ranking/MonthlyRankingJobConfig.java`
around lines 101 - 128, monthlyRankingWriter currently deletes and saves per
chunk which causes only the last chunk's results to persist; remove the call to
mvRepository.deleteByPeriodStartAndPeriodEnd from monthlyRankingWriter and
instead implement a StepExecutionListener that does the delete once in
beforeStep, and change monthlyRankingWriter to accumulate chunk scores into a
shared StepExecution executionContext priority queue (or List) keyed by e.g.
"monthlyTopQueue" so each chunk adds its RankedProductScore items (use TOP_N
logic to keep the queue bounded), then in afterStep (the same listener) compute
the global top N from that shared queue and persist MvProductRankMonthly entries
(using scored.productId(), rank, scored.score(), periodStart, periodEnd);
reference monthlyRankingWriter, mvRepository.deleteByPeriodStartAndPeriodEnd,
MvProductRankMonthly, TOP_N, and the StepExecution beforeStep/afterStep methods
to locate and apply the change.

}
Loading