Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@
@Component
public class ProductAssembler {

public List<ProductInfo> toInfos(List<Product> products, List<Brand> brands) {
public Map<Long, ProductInfo> toInfoMap(List<Product> products, List<Brand> brands) {
Map<Long, Brand> brandMap = brands.stream().collect(Collectors.toMap(Brand::getId, b -> b));

return products.stream()
.map(product -> {
Brand brand = brandMap.get(product.brandId());
return ProductInfo.of(product, brand != null ? brand.name() : null);
}).toList();
.collect(Collectors.toMap(
Product::getId,
product -> {
Brand brand = brandMap.get(product.brandId());
return ProductInfo.of(product, brand != null ? brand.name() : null);
}
));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.loopers.application.product;

import java.util.List;
import java.util.Map;
import java.util.Optional;

public interface ProductCacheStore {

long DETAIL_TTL_MINUTES = 5;
long LIST_TTL_SECONDS = 30;

Optional<ProductInfo> get(Long productId);

void put(Long productId, ProductInfo productInfo);

void evict(Long productId);

record ProductListCacheEntry(List<Long> productIds, int totalPages) {}

Optional<ProductListCacheEntry> getList(Long brandId, String sort, int page, int size);

void putList(Long brandId, String sort, int page, int size, List<Long> productIds, int totalPages);

Map<Long, ProductInfo> multiGet(List<Long> productIds);
Comment on lines +16 to +24
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

목록 캐시를 무효화할 계약이 필요하다.

현재 인터페이스는 상세 캐시만 evict할 수 있어서, 삭제/가격 변경 뒤에도 목록의 productIds 캐시는 TTL 동안 그대로 남는다. 운영에서는 삭제된 상품이 목록에 남거나 PRICE_ASC·LIKE_COUNT 결과가 실제 DB 순서와 달라질 수 있으므로, 브랜드/정렬 단위의 목록 캐시 무효화 API를 추가하거나 key scan 대신 namespace version 기반 일괄 폐기 전략으로 바꿔야 한다. 삭제·가격 변경·좋아요 수 변경 직후 다음 목록 조회가 이전 page ID 캐시를 재사용하지 않는 테스트를 추가해야 한다.

🤖 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/product/ProductCacheStore.java`
around lines 16 - 24, Product list caches aren't evicted today (only detail
evict exists), so add a contract on ProductCacheStore to invalidate list-level
entries and update callers/tests to use it: extend the ProductCacheStore
interface with a method such as evictList(Long brandId, String sort) (or
evictListsForBrand(Long brandId) if you need brand-wide), ensure implementations
of getList/putList/ProductListCacheEntry respect a namespace/version or
implement the new evictList to remove/increment namespace for that brand+sort,
and call this new method from the product deletion/price-change/like-change
flows where evict(Long productId) is currently used; add a unit/integration test
that performs delete/price/like and then verifies subsequent getList does not
return stale productIds.

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,19 @@
import com.loopers.domain.like.LikeRepository;
import com.loopers.domain.product.Product;
import com.loopers.domain.product.ProductRepository;
import com.loopers.domain.product.ProductSortType;
import com.loopers.domain.product.vo.Price;
import com.loopers.domain.product.vo.Stock;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import com.loopers.support.page.PageResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.Optional;
import java.util.Map;
import java.util.Objects;

@RequiredArgsConstructor
@Component
Expand All @@ -28,6 +27,7 @@ public class ProductFacade {
private final ProductRepository productRepository;
private final LikeRepository likeRepository;
private final ProductAssembler productAssembler;
private final ProductQueryService productQueryService;

@Transactional
public void register(String name, String description, Integer stock, Integer price, Long brandId) {
Expand All @@ -49,6 +49,8 @@ public void update(Long productId, String name, String description, Integer stoc
Stock stockVo = Stock.from(stock);
Price priceVo = Price.from(price);
product.update(name, description, stockVo, priceVo);

productQueryService.evict(productId);
}

@Transactional(readOnly = true)
Expand All @@ -60,42 +62,25 @@ public PageResponse<ProductInfo> getList(Pageable pageable) {
List<Long> brandIds = productList.stream().map(Product::brandId).toList();
List<Brand> brands = brandRepository.findAllByIdIn(brandIds);

List<ProductInfo> productInfos = productAssembler.toInfos(productList, brands);
Map<Long, ProductInfo> infoMap = productAssembler.toInfoMap(productList, brands);
List<ProductInfo> productInfos = productList.stream().map(p -> infoMap.get(p.getId())).filter(Objects::nonNull).toList();
return new PageResponse<>(productInfos, products.page(), products.size(), products.totalPages());
}

@Transactional(readOnly = true)
public PageResponse<ProductInfo> getList(Pageable pageable, Long brandId, String sort) {
PageRequest pageRequest = PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), ProductSortType.from(sort).getSort());
PageResponse<Product> products;
if (brandId == null) {
products = productRepository.findAll(pageRequest);
} else {
products = productRepository.findAllByBrandId(brandId, pageRequest);
}

List<Product> productList = products.content();
if (productList.isEmpty()) return new PageResponse<>(List.of(), products.page(), products.size(), products.totalPages());

List<Long> brandIds = productList.stream().map(Product::brandId).toList();
List<Brand> brands = brandRepository.findAllByIdIn(brandIds);

List<ProductInfo> productInfos = productAssembler.toInfos(productList, brands);
return new PageResponse<>(productInfos, products.page(), products.size(), products.totalPages());
return productQueryService.getList(pageable, brandId, sort);
}

@Transactional(readOnly = true)
public ProductInfo getDetail(Long productId) {
Product product = productRepository.findById(productId)
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 상품입니다."));

Optional<Brand> optionalBrand = brandRepository.findById(product.brandId());
return ProductInfo.of(product, optionalBrand.map(Brand::name).orElse(null));
return productQueryService.getDetail(productId);
}

@Transactional
public void delete(Long productId) {
likeRepository.deleteAllByProductId(productId);
productRepository.deleteById(productId);
productQueryService.evict(productId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package com.loopers.application.product;

import com.loopers.domain.brand.Brand;
import com.loopers.domain.brand.BrandRepository;
import com.loopers.domain.product.Product;
import com.loopers.domain.product.ProductRepository;
import com.loopers.domain.product.ProductSortType;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import com.loopers.support.page.PageResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

@RequiredArgsConstructor
@Component
public class ProductQueryService {

private final ProductRepository productRepository;
private final BrandRepository brandRepository;
private final ProductAssembler productAssembler;
private final ProductCacheStore productCacheStore;

@Transactional(readOnly = true)
public PageResponse<ProductInfo> getList(Pageable pageable, Long brandId, String sort) {
int page = pageable.getPageNumber();
int size = pageable.getPageSize();
PageRequest pageRequest = PageRequest.of(page, size, ProductSortType.from(sort).getSort());

return productCacheStore.getList(brandId, sort, page, size)
.map(entry -> resolveFromCache(entry, page, size))
.orElseGet(() -> resolveFromDatabase(brandId, pageRequest, sort, page, size));
Comment on lines +33 to +40
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

캐시 키에는 정규화된 sort 값을 써야 한다.

DB 조회는 ProductSortType.from(sort)로 정규화하지만, 캐시 조회/저장은 원본 sort 문자열을 그대로 사용한다. 그래서 null, latest, LATEST처럼 같은 의미의 요청이 서로 다른 list key로 분산되어 hit rate가 떨어지고 DB 부하 저감 효과가 약해지므로, ProductSortType을 한 번만 해석한 뒤 sortType.name() 같은 정규화된 값을 getList/putList 전체에 사용해야 한다. null·소문자·대문자 입력이 같은 캐시 엔트리를 재사용하는 테스트를 추가해야 한다.

패치 예시
 `@Transactional`(readOnly = true)
 public PageResponse<ProductInfo> getList(Pageable pageable, Long brandId, String sort) {
     int page = pageable.getPageNumber();
     int size = pageable.getPageSize();
-    PageRequest pageRequest = PageRequest.of(page, size, ProductSortType.from(sort).getSort());
+    ProductSortType sortType = ProductSortType.from(sort);
+    String normalizedSort = sortType.name();
+    PageRequest pageRequest = PageRequest.of(page, size, sortType.getSort());
 
-    return productCacheStore.getList(brandId, sort, page, size)
+    return productCacheStore.getList(brandId, normalizedSort, page, size)
             .map(entry -> resolveFromCache(entry, page, size))
-            .orElseGet(() -> resolveFromDatabase(brandId, pageRequest, sort, page, size));
+            .orElseGet(() -> resolveFromDatabase(brandId, pageRequest, normalizedSort, page, size));
 }

Also applies to: 72-89

🤖 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/product/ProductQueryService.java`
around lines 33 - 40, In getList, normalize the sort once with ProductSortType
sortType = ProductSortType.from(sort) and use sortType.name() (or another
canonical value) for all cache interactions (productCacheStore.getList and
productCacheStore.putList) instead of the raw sort string so requests like
null/latest/LATEST share the same cache key; apply the same change in the other
block (resolveFromDatabase/resolveFromCache paths, lines ~72-89) and add a
unit/integration test that calls the service with null, lowercase and uppercase
sort inputs and asserts they hit the same cache entry.

}

private PageResponse<ProductInfo> resolveFromCache(ProductCacheStore.ProductListCacheEntry entry, int page, int size) {
List<Long> productIds = entry.productIds();
int totalPages = entry.totalPages();
if (productIds.isEmpty()) {
return new PageResponse<>(List.of(), page, size, totalPages);
}

Map<Long, ProductInfo> cachedInfos = productCacheStore.multiGet(productIds);
List<Long> missingIds = productIds.stream().filter(id -> !cachedInfos.containsKey(id)).toList();
if (missingIds.isEmpty()) {
return new PageResponse<>(productIds.stream().map(cachedInfos::get).toList(), page, size, totalPages);
}

Map<Long, ProductInfo> allInfoMap = new HashMap<>(cachedInfos);
List<Product> missingProducts = productRepository.findAllByIdIn(missingIds);
List<Long> missingBrandIds = missingProducts.stream().map(Product::brandId).distinct().toList();
List<Brand> missingBrands = brandRepository.findAllByIdIn(missingBrandIds);
Map<Long, ProductInfo> missingInfoMap = productAssembler.toInfoMap(missingProducts, missingBrands);
allInfoMap.putAll(missingInfoMap);
missingInfoMap.forEach(productCacheStore::put);

List<ProductInfo> orderedInfos = productIds.stream()
.map(allInfoMap::get)
.filter(Objects::nonNull)
.toList();

return new PageResponse<>(orderedInfos, page, size, totalPages);
}

private PageResponse<ProductInfo> resolveFromDatabase(Long brandId, PageRequest pageRequest, String sort, int page, int size) {
PageResponse<Product> products = brandId == null
? productRepository.findAll(pageRequest)
: productRepository.findAllByBrandId(brandId, pageRequest);

List<Product> productList = products.content();
if (productList.isEmpty()) {
productCacheStore.putList(brandId, sort, page, size, List.of(), products.totalPages());
return new PageResponse<>(List.of(), products.page(), products.size(), products.totalPages());
}

List<Long> brandIds = productList.stream().map(Product::brandId).toList();
List<Brand> brands = brandRepository.findAllByIdIn(brandIds);
Map<Long, ProductInfo> infoMap = productAssembler.toInfoMap(productList, brands);

List<Long> productIds = productList.stream().map(Product::getId).toList();
productCacheStore.putList(brandId, sort, page, size, productIds, products.totalPages());
infoMap.forEach((id, info) -> productCacheStore.put(id, info));

List<ProductInfo> orderedInfos = productIds.stream().map(infoMap::get).filter(Objects::nonNull).toList();
return new PageResponse<>(orderedInfos, products.page(), products.size(), products.totalPages());
}

@Transactional(readOnly = true)
public ProductInfo getDetail(Long productId) {
Optional<ProductInfo> cached = productCacheStore.get(productId);
if (cached.isPresent()) {
return cached.get();
}

Product product = productRepository.findById(productId)
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 상품입니다."));

Optional<Brand> optionalBrand = brandRepository.findById(product.brandId());
ProductInfo productInfo = ProductInfo.of(product, optionalBrand.map(Brand::name).orElse(null));

productCacheStore.put(productId, productInfo);

return productInfo;
}

public void evict(Long productId) {
productCacheStore.evict(productId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@

public enum ProductSortType {
LATEST(Sort.by(Sort.Direction.DESC, "createdAt")), // 또는 "createdAt"
PRICE_ASC(Sort.by(Sort.Direction.ASC, "price"));
PRICE_ASC(Sort.by(Sort.Direction.ASC, "price")),
LIKE_COUNT(Sort.by(Sort.Direction.DESC, "likeCount"));
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

LIKE_COUNT 정렬에 보조 키를 추가해야 한다.

likeCount만으로 정렬하면 동률 구간에서 DB가 임의 순서를 반환하므로, 운영 중 페이지 경계에서 상품 중복/누락이 발생할 수 있다. 현재 PR처럼 페이지별 productIds를 캐시하면 이 비결정성이 그대로 캐시되어 같은 요청도 다른 ID 집합을 반환할 수 있으므로, id DESC 또는 createdAt DESC를 보조 정렬로 붙여 순서를 고정해야 한다. 같은 likeCount를 가진 상품이 여러 개 있을 때 1페이지/2페이지 결과가 반복 호출에도 안정적으로 유지되는 테스트를 추가해야 한다.

패치 예시
-    LIKE_COUNT(Sort.by(Sort.Direction.DESC, "likeCount"));
+    LIKE_COUNT(
+            Sort.by(Sort.Direction.DESC, "likeCount")
+                    .and(Sort.by(Sort.Direction.DESC, "id"))
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
PRICE_ASC(Sort.by(Sort.Direction.ASC, "price")),
LIKE_COUNT(Sort.by(Sort.Direction.DESC, "likeCount"));
PRICE_ASC(Sort.by(Sort.Direction.ASC, "price")),
LIKE_COUNT(
Sort.by(Sort.Direction.DESC, "likeCount")
.and(Sort.by(Sort.Direction.DESC, "id"))
);
🤖 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/product/ProductSortType.java`
around lines 9 - 10, The LIKE_COUNT sort (constant LIKE_COUNT in
ProductSortType) is unstable because it only sorts by likeCount; update it to
append a deterministic secondary key (e.g., id DESC or createdAt DESC) so ties
are resolved consistently (use Sort.by(...).and(...) to combine likeCount DESC
with id DESC or createdAt DESC), and add a unit/integration test that seeds
multiple products with identical likeCount and asserts paginated results (page 1
then page 2) remain stable across repeated requests.


private final Sort sort;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package com.loopers.infrastructure.product;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.loopers.application.product.ProductCacheStore;
import com.loopers.application.product.ProductInfo;
import lombok.RequiredArgsConstructor;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.stereotype.Component;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

@RequiredArgsConstructor
@Component
public class RedisProductCacheStore implements ProductCacheStore {

private static final String DETAIL_KEY_PREFIX = "product:detail:";
private static final String LIST_KEY_PREFIX = "product:list:";

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

@Override
public Optional<ProductInfo> get(Long productId) {
try {
String cached = redisTemplate.opsForValue().get(DETAIL_KEY_PREFIX + productId);
if (cached != null) {
return Optional.of(objectMapper.readValue(cached, ProductInfo.class));
}
} catch (Exception ignored) {
}
return Optional.empty();
}

@Override
public void put(Long productId, ProductInfo productInfo) {
try {
String json = objectMapper.writeValueAsString(productInfo);
redisTemplate.opsForValue().set(DETAIL_KEY_PREFIX + productId, json, DETAIL_TTL_MINUTES, TimeUnit.MINUTES);
} catch (Exception ignored) {
}
}

@Override
public void evict(Long productId) {
redisTemplate.delete(DETAIL_KEY_PREFIX + productId);
Comment on lines +47 to +49
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

evictmultiGet도 Redis 장애를 비치명적으로 처리해야 한다.

이 클래스는 get/put/getList/putList만 실패를 흡수하고, evictmultiGet은 Redis 예외를 그대로 전파한다. 운영에서는 Redis 일시 장애만으로 상품 수정/삭제 트랜잭션이 롤백되거나 목록 조회가 DB fallback 없이 500으로 끝날 수 있으므로, 두 메서드도 no-op/빈 맵으로 폴백하고 실패 로그·메트릭을 남기도록 맞춰야 한다. redisTemplate.delete(...)multiGet(...)이 예외를 던질 때 쓰기 API는 성공하고 읽기 API는 DB로 폴백하는 테스트를 추가해야 한다.

패치 예시
 public void evict(Long productId) {
-    redisTemplate.delete(DETAIL_KEY_PREFIX + productId);
+    try {
+        redisTemplate.delete(DETAIL_KEY_PREFIX + productId);
+    } catch (Exception e) {
+        log.warn("Failed to evict product cache. productId={}", productId, e);
+    }
 }
 
 `@Override`
 public Map<Long, ProductInfo> multiGet(List<Long> productIds) {
-    List<String> keys = productIds.stream()
-            .map(id -> DETAIL_KEY_PREFIX + id)
-            .toList();
-    List<String> values = redisTemplate.opsForValue().multiGet(keys);
     Map<Long, ProductInfo> result = new HashMap<>();
-    if (values == null) return result;
-    for (int i = 0; i < productIds.size(); i++) {
-        String value = values.get(i);
-        if (value != null) {
-            try {
-                result.put(productIds.get(i), objectMapper.readValue(value, ProductInfo.class));
-            } catch (Exception ignored) {
+    try {
+        List<String> keys = productIds.stream()
+                .map(id -> DETAIL_KEY_PREFIX + id)
+                .toList();
+        List<String> values = redisTemplate.opsForValue().multiGet(keys);
+        if (values == null) return result;
+        for (int i = 0; i < productIds.size(); i++) {
+            String value = values.get(i);
+            if (value != null) {
+                try {
+                    result.put(productIds.get(i), objectMapper.readValue(value, ProductInfo.class));
+                } catch (Exception e) {
+                    log.warn("Failed to deserialize product cache. productId={}", productIds.get(i), e);
+                }
             }
         }
+    } catch (Exception e) {
+        log.warn("Failed to read product caches. productIds={}", productIds, e);
     }
     return result;
 }

Also applies to: 74-90

🤖 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/product/RedisProductCacheStore.java`
around lines 47 - 49, RedisProductCacheStore currently swallows Redis failures
only for get/put/getList/putList but lets evict and multiGet throw; wrap both
evict(Long) and multiGet(Collection<Long>) calls to redisTemplate.delete(...)
and redisTemplate.multiGet(...) in try-catch blocks that log the exception via
the existing logger, increment a failure metric, and perform safe fallbacks
(evict: no-op on failure; multiGet: return an empty map/empty collection so
callers fall back to DB). Update method bodies in RedisProductCacheStore to
catch RuntimeException (or Redis-specific exceptions), emit the same log/metric
style used by get/put methods, and ensure behavior parity with write APIs
(tests: add cases asserting that when redisTemplate.delete/multiGet throw, write
operations still succeed and read operations fall back to DB/return empty map).

}

@Override
public Optional<ProductListCacheEntry> getList(Long brandId, String sort, int page, int size) {
try {
String cached = redisTemplate.opsForValue().get(listKey(brandId, sort, page, size));
if (cached != null) {
return Optional.of(objectMapper.readValue(cached, ProductListCacheEntry.class));
}
} catch (Exception ignored) {
}
return Optional.empty();
}

@Override
public void putList(Long brandId, String sort, int page, int size, List<Long> productIds, int totalPages) {
try {
String json = objectMapper.writeValueAsString(new ProductListCacheEntry(productIds, totalPages));
redisTemplate.opsForValue().set(listKey(brandId, sort, page, size), json, LIST_TTL_SECONDS, TimeUnit.SECONDS);
} catch (Exception ignored) {
}
}

@Override
public Map<Long, ProductInfo> multiGet(List<Long> productIds) {
List<String> keys = productIds.stream()
.map(id -> DETAIL_KEY_PREFIX + id)
.toList();
List<String> values = redisTemplate.opsForValue().multiGet(keys);
Map<Long, ProductInfo> result = new HashMap<>();
if (values == null) return result;
for (int i = 0; i < productIds.size(); i++) {
String value = values.get(i);
if (value != null) {
try {
result.put(productIds.get(i), objectMapper.readValue(value, ProductInfo.class));
} catch (Exception ignored) {
}
}
}
return result;
}

private String listKey(Long brandId, String sort, int page, int size) {
return LIST_KEY_PREFIX + "brandId=" + brandId + ":sort=" + sort + ":page=" + page + ":size=" + size;
}
}
Loading