Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import com.loopers.domain.product.ProductRepository;
import com.loopers.domain.ranking.RankingEntry;
import com.loopers.domain.ranking.RankingRepository;
import com.loopers.domain.ranking.mv.MvProductRankMonthly;
import com.loopers.domain.ranking.mv.MvProductRankWeekly;
import com.loopers.domain.ranking.mv.MvRankingRepository;
import com.loopers.infrastructure.ranking.RankingCacheStore;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -32,14 +35,19 @@ public class RankingFacade {
private final ProductRepository productRepository;
private final BrandRepository brandRepository;
private final RankingCacheStore rankingCacheStore;
private final MvRankingRepository mvRankingRepository;

@Transactional(readOnly = true)
public List<RankingDto.RankingItemInfo> getRankings(String date, int page, int size) {
return rankingCacheStore.getRankings(date, page, size)
public List<RankingDto.RankingItemInfo> getRankings(String period, String date, int page, int size) {
return rankingCacheStore.getRankings(period, date, page, size)
.orElseGet(() -> {
List<RankingDto.RankingItemInfo> result = loadRankings(date, page, size);
List<RankingDto.RankingItemInfo> result = switch (period) {
case "weekly" -> loadMvRankings(date, page, size, true);
case "monthly" -> loadMvRankings(date, page, size, false);
default -> loadRankings(date, page, size);
};
if (!result.isEmpty()) {
rankingCacheStore.putRankings(date, page, size, result);
rankingCacheStore.putRankings(period, date, page, size, result);
Comment on lines +41 to +50
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

허용되지 않은 perioddaily로 묵살하지 말아야 한다.

현재는 "weekly", "monthly" 외의 모든 값이 default로 일간 랭킹으로 내려가고, 잘못된 period가 그대로 캐시 키에도 반영된다. 운영에서는 잘못된 호출을 조용히 성공 처리해 계약 위반을 숨기고, 캐시 오염까지 남긴다. 캐시 조회 전에 daily|weekly|monthly만 허용하도록 검증하고, 그 외 값은 CoreException으로 명시적으로 거절해 달라. period=foo 요청이 표준 오류 응답으로 실패하고 캐시/저장소가 호출되지 않는 테스트도 추가해 달라. Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format.

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

In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 41 - 50, Reject invalid period values before any cache/store access
in RankingFacade.getRankings: validate that the incoming period string is one of
"daily","weekly","monthly" at the top of getRankings(), and if not throw a
CoreException so the error flows to ApiControllerAdvice; only after validation
perform rankingCacheStore.getRankings(...) and routing to
loadMvRankings/loadRankings as currently implemented; update any unit tests to
assert that a request with period="foo" results in a CoreException-driven API
error and that rankingCacheStore.put/get and backing loaders are not invoked for
that call.

}
return result;
});
Expand Down Expand Up @@ -86,6 +94,52 @@ private List<RankingDto.RankingItemInfo> loadRankings(String date, int page, int
return result;
}

private List<RankingDto.RankingItemInfo> loadMvRankings(String date, int page, int size, boolean weekly) {
LocalDate aggregatedAt = LocalDate.parse(date, DATE_FORMAT);
Comment on lines +97 to +98
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

주간/월간 날짜 파싱 예외를 그대로 노출하면 응답 형식이 깨진다.

여기서 LocalDate.parse가 던지는 DateTimeParseException이 그대로 새면 일간 경로와 다르게 공통 오류 응답 형식을 보장하지 못한다. 날짜 검증/파싱을 공통 resolver로 올리거나 이 지점에서 CoreException으로 변환해 달라. period=weekly|monthly에서 잘못된 날짜 형식이 들어왔을 때 표준 오류 코드와 메시지로 내려가는 테스트를 추가해 달라. Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format.

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

In
`@apps/commerce-api/src/main/java/com/loopers/application/ranking/RankingFacade.java`
around lines 97 - 98, The method loadMvRankings currently calls
LocalDate.parse(date, DATE_FORMAT) which can throw DateTimeParseException and
bypass the unified error response; catch DateTimeParseException inside
loadMvRankings (or the calling path) and rethrow a CoreException with a clear
error code/message so ApiControllerAdvice produces the standard response, or
move the parsing into a shared resolver that validates/returns LocalDate and
throws CoreException on failure; update tests to cover invalid date inputs for
period=weekly and period=monthly to assert the standard error code/message is
returned.


List<Long> productIds;
List<Integer> ranks;
List<Double> scores;

if (weekly) {
var mvRanks = mvRankingRepository.findWeeklyRankings(aggregatedAt, page, size);
productIds = mvRanks.stream().map(MvProductRankWeekly::getProductId).toList();
ranks = mvRanks.stream().map(MvProductRankWeekly::getRank).toList();
scores = mvRanks.stream().map(MvProductRankWeekly::getScore).toList();
} else {
var mvRanks = mvRankingRepository.findMonthlyRankings(aggregatedAt, page, size);
productIds = mvRanks.stream().map(MvProductRankMonthly::getProductId).toList();
ranks = mvRanks.stream().map(MvProductRankMonthly::getRank).toList();
scores = mvRanks.stream().map(MvProductRankMonthly::getScore).toList();
}

if (productIds.isEmpty()) {
return List.of();
}

Map<Long, Product> productMap = productRepository.findAllByIds(Set.copyOf(productIds)).stream()
.collect(Collectors.toMap(Product::getId, p -> p));

Set<Long> brandIds = productMap.values().stream()
.map(Product::getBrandId)
.collect(Collectors.toSet());

Map<Long, String> brandNameMap = brandRepository.findAllByIds(brandIds).stream()
.collect(Collectors.toMap(Brand::getId, Brand::getName));

List<RankingDto.RankingItemInfo> result = new ArrayList<>();
for (int i = 0; i < productIds.size(); i++) {
Product product = productMap.get(productIds.get(i));
if (product == null) {
continue;
}
String brandName = brandNameMap.getOrDefault(product.getBrandId(), "");
ProductDto.ProductInfo productInfo = ProductDto.ProductInfo.of(product, brandName);
result.add(RankingDto.RankingItemInfo.of(ranks.get(i), scores.get(i), productInfo));
}
return result;
}

public RankingDto.ProductRankInfo getProductRank(Long productId, String date) {
String key = RANKING_KEY_PREFIX + date;
Long rank = rankingRepository.getRank(key, productId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.loopers.domain.ranking.mv;

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

import java.time.LocalDate;

@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 = "score", nullable = false)
private double score;

@Column(name = "ranking", nullable = false)
private int rank;

@Column(name = "view_count", nullable = false)
private long viewCount;

@Column(name = "like_count", nullable = false)
private long likeCount;

@Column(name = "sales_count", nullable = false)
private long salesCount;

@Column(name = "aggregated_at", nullable = false)
private LocalDate aggregatedAt;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.loopers.domain.ranking.mv;

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

import java.time.LocalDate;

@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 = "score", nullable = false)
private double score;

@Column(name = "ranking", nullable = false)
private int rank;

@Column(name = "view_count", nullable = false)
private long viewCount;

@Column(name = "like_count", nullable = false)
private long likeCount;

@Column(name = "sales_count", nullable = false)
private long salesCount;

@Column(name = "aggregated_at", nullable = false)
private LocalDate aggregatedAt;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.loopers.domain.ranking.mv;

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

public interface MvRankingRepository {

List<MvProductRankWeekly> findWeeklyRankings(LocalDate aggregatedAt, int page, int size);

List<MvProductRankMonthly> findMonthlyRankings(LocalDate aggregatedAt, int page, int size);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,37 @@
public class RankingCacheStore {

private static final String RANKING_CACHE_KEY_PREFIX = "ranking:cache:";
private static final Duration RANKING_CACHE_TTL = Duration.ofMinutes(5);
private static final Duration DAILY_CACHE_TTL = Duration.ofMinutes(5);
private static final Duration WEEKLY_MONTHLY_CACHE_TTL = Duration.ofHours(1);

private final RedisTemplate<String, String> redisTemplate;
private final ObjectMapper objectMapper;

public Optional<List<RankingDto.RankingItemInfo>> getRankings(String date, int page, int size) {
public Optional<List<RankingDto.RankingItemInfo>> getRankings(String period, String date, int page, int size) {
try {
String json = redisTemplate.opsForValue().get(cacheKey(date, page, size));
String json = redisTemplate.opsForValue().get(cacheKey(period, date, page, size));
if (json == null) return Optional.empty();
return Optional.of(objectMapper.readValue(json, new TypeReference<>() {}));
} catch (Exception e) {
log.warn("랭킹 캐시 조회 실패 - date:{} page:{}", date, page, e);
log.warn("랭킹 캐시 조회 실패 - period:{} date:{} page:{}", period, date, page, e);
return Optional.empty();
}
}

public void putRankings(String date, int page, int size, List<RankingDto.RankingItemInfo> rankings) {
public void putRankings(String period, String date, int page, int size, List<RankingDto.RankingItemInfo> rankings) {
try {
String json = objectMapper.writeValueAsString(rankings);
redisTemplate.opsForValue().set(cacheKey(date, page, size), json, RANKING_CACHE_TTL);
redisTemplate.opsForValue().set(cacheKey(period, date, page, size), json, ttlFor(period));
} catch (Exception e) {
log.warn("랭킹 캐시 저장 실패 - date:{} page:{}", date, page, e);
log.warn("랭킹 캐시 저장 실패 - period:{} date:{} page:{}", period, date, page, e);
}
}

private String cacheKey(String date, int page, int size) {
return RANKING_CACHE_KEY_PREFIX + "date=" + date + ":page=" + page + ":size=" + size;
private Duration ttlFor(String period) {
return "daily".equals(period) ? DAILY_CACHE_TTL : WEEKLY_MONTHLY_CACHE_TTL;
}

private String cacheKey(String period, String date, int page, int size) {
return RANKING_CACHE_KEY_PREFIX + "period=" + period + ":date=" + date + ":page=" + page + ":size=" + size;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.loopers.infrastructure.ranking.mv;

import com.loopers.domain.ranking.mv.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> findByAggregatedAtOrderByRankAsc(LocalDate aggregatedAt, Pageable pageable);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.loopers.infrastructure.ranking.mv;

import com.loopers.domain.ranking.mv.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> findByAggregatedAtOrderByRankAsc(LocalDate aggregatedAt, Pageable pageable);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.loopers.infrastructure.ranking.mv;

import com.loopers.domain.ranking.mv.MvProductRankMonthly;
import com.loopers.domain.ranking.mv.MvProductRankWeekly;
import com.loopers.domain.ranking.mv.MvRankingRepository;
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 MvRankingRepositoryAdapter implements MvRankingRepository {

private final MvProductRankWeeklyJpaRepository weeklyJpaRepository;
private final MvProductRankMonthlyJpaRepository monthlyJpaRepository;

@Override
public List<MvProductRankWeekly> findWeeklyRankings(LocalDate aggregatedAt, int page, int size) {
return weeklyJpaRepository.findByAggregatedAtOrderByRankAsc(aggregatedAt, PageRequest.of(page, size));
}

@Override
public List<MvProductRankMonthly> findMonthlyRankings(LocalDate aggregatedAt, int page, int size) {
return monthlyJpaRepository.findByAggregatedAtOrderByRankAsc(aggregatedAt, PageRequest.of(page, size));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ public class RankingV1Controller {

@GetMapping("/api/v1/rankings")
public ApiResponse<RankingV1Dto.RankingPageResponse> getRankings(
@RequestParam(defaultValue = "daily") String period,
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

period 파라미터 검증 부재로 인한 운영 이슈 가능성이 있다.

period 파라미터가 문자열로 받아지며, RankingFacade의 switch문에서 대소문자 구분("weekly", "monthly")을 한다. 사용자가 "Weekly" 또는 "DAILY" 등 대소문자가 다른 값을 입력하면 silent하게 daily 랭킹으로 fallback되어, 운영 환경에서 오동작 디버깅이 어려워질 수 있다.

수정안:

  1. period.toLowerCase()로 정규화하거나
  2. 유효하지 않은 period 값에 대해 명시적 예외를 던지도록 도메인 레이어에서 검증

추가 테스트: "Weekly", "MONTHLY" 등 대소문자 변형 입력에 대한 E2E 테스트 추가 권장

🔧 정규화 적용 예시
     `@GetMapping`("/api/v1/rankings")
     public ApiResponse<RankingV1Dto.RankingPageResponse> getRankings(
         `@RequestParam`(defaultValue = "daily") String period,
         `@RequestParam`(required = false) String date,
         `@RequestParam`(defaultValue = "20") int size,
         `@RequestParam`(defaultValue = "0") int page
     ) {
         String resolvedDate = (date != null) ? date : todayDate();
-        List<RankingDto.RankingItemInfo> rankings = rankingFacade.getRankings(period, resolvedDate, page, size);
+        List<RankingDto.RankingItemInfo> rankings = rankingFacade.getRankings(period.toLowerCase(), resolvedDate, page, size);
         return ApiResponse.success(RankingV1Dto.RankingPageResponse.of(rankings, page, size));
     }
🤖 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 26, The controller currently accepts a String period
(RankingV1Controller parameter period) but does not normalize or validate it,
causing the RankingFacade switch to be case-sensitive and silently fall back to
daily; update RankingV1Controller to normalize period (e.g., period =
period.toLowerCase(Locale.ROOT)) before passing to RankingFacade or validate
against allowed values and throw a clear exception for invalid input, and add
E2E tests for case variants like "Weekly" and "MONTHLY" to ensure correct
behavior.

@RequestParam(required = false) String date,
@RequestParam(defaultValue = "20") int size,
@RequestParam(defaultValue = "0") int page
) {
String resolvedDate = (date != null) ? date : todayDate();
List<RankingDto.RankingItemInfo> rankings = rankingFacade.getRankings(resolvedDate, page, size);
List<RankingDto.RankingItemInfo> rankings = rankingFacade.getRankings(period, resolvedDate, page, size);
return ApiResponse.success(RankingV1Dto.RankingPageResponse.of(rankings, page, size));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.loopers.domain.product.ProductRepository;
import com.loopers.domain.ranking.RankingEntry;
import com.loopers.domain.ranking.RankingRepository;
import com.loopers.domain.ranking.mv.MvRankingRepository;
import com.loopers.infrastructure.ranking.RankingCacheStore;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand All @@ -30,6 +31,7 @@ class RankingFacadeTest {
private ProductRepository productRepository;
private BrandRepository brandRepository;
private RankingCacheStore rankingCacheStore;
private MvRankingRepository mvRankingRepository;
private RankingFacade rankingFacade;

private final String todayKey = "ranking:all:" +
Expand All @@ -41,9 +43,10 @@ void setUp() {
productRepository = mock(ProductRepository.class);
brandRepository = mock(BrandRepository.class);
rankingCacheStore = mock(RankingCacheStore.class);
rankingFacade = new RankingFacade(rankingRepository, productRepository, brandRepository, rankingCacheStore);
mvRankingRepository = mock(MvRankingRepository.class);
rankingFacade = new RankingFacade(rankingRepository, productRepository, brandRepository, rankingCacheStore, mvRankingRepository);

when(rankingCacheStore.getRankings(anyString(), anyInt(), anyInt())).thenReturn(Optional.empty());
when(rankingCacheStore.getRankings(anyString(), anyString(), anyInt(), anyInt())).thenReturn(Optional.empty());
}

@DisplayName("랭킹 페이지 조회 시 상품 정보가 포함된 랭킹 목록을 반환한다")
Expand All @@ -65,7 +68,7 @@ void getRankings_returnsRankingWithProductInfo() {
setId(brand, 1L);
when(brandRepository.findAllByIds(anyCollection())).thenReturn(List.of(brand));

List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings(date, 0, 20);
List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings("daily", date, 0, 20);

assertThat(result).hasSize(2);
assertThat(result.get(0).rank()).isEqualTo(1);
Expand All @@ -80,7 +83,7 @@ void getRankings_returnsRankingWithProductInfo() {
void getRankings_emptyZset_returnsEmptyList() {
when(rankingRepository.getTopRankings(anyString(), anyLong(), anyLong())).thenReturn(List.of());

List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings("20260405", 0, 20);
List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings("daily", "20260405", 0, 20);

assertThat(result).isEmpty();
}
Expand Down Expand Up @@ -117,7 +120,7 @@ void getRankings_page1_calculatesCorrectStart() {
String key = "ranking:all:" + date;
when(rankingRepository.getTopRankings(eq(key), eq(20L), anyLong())).thenReturn(List.of());

rankingFacade.getRankings(date, 1, 20);
rankingFacade.getRankings("daily", date, 1, 20);

verify(rankingRepository).getTopRankings(eq(key), eq(20L), anyLong());
}
Expand All @@ -128,9 +131,9 @@ void getRankings_cacheHit_skipsZsetAndDb() {
List<RankingDto.RankingItemInfo> cached = List.of(
new RankingDto.RankingItemInfo(1, 5.0, 10L, "상품A", "브랜드", BigDecimal.valueOf(10000))
);
when(rankingCacheStore.getRankings("20260405", 0, 20)).thenReturn(Optional.of(cached));
when(rankingCacheStore.getRankings("daily", "20260405", 0, 20)).thenReturn(Optional.of(cached));

List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings("20260405", 0, 20);
List<RankingDto.RankingItemInfo> result = rankingFacade.getRankings("daily", "20260405", 0, 20);

assertThat(result).hasSize(1);
assertThat(result.get(0).productName()).isEqualTo("상품A");
Expand All @@ -153,9 +156,9 @@ void getRankings_cacheMiss_savesToCache() {
setId(brand, 1L);
when(brandRepository.findAllByIds(anyCollection())).thenReturn(List.of(brand));

rankingFacade.getRankings(date, 0, 20);
rankingFacade.getRankings("daily", date, 0, 20);

verify(rankingCacheStore).putRankings(eq(date), eq(0), eq(20), anyList());
verify(rankingCacheStore).putRankings(eq("daily"), eq(date), eq(0), eq(20), anyList());
}

private void setId(Object entity, Long id) {
Expand Down
Loading