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
@@ -1,29 +1,38 @@
package com.loopers.application.brand;

import com.loopers.domain.model.brand.event.BrandDeletedEvent;
import com.loopers.domain.model.brand.event.BrandProductsDeletedEvent;
import com.loopers.domain.model.product.Product;
import com.loopers.domain.repository.ProductRepository;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;

import java.util.ArrayList;
import java.util.List;

@Component
public class BrandDeletedEventHandler {

private final ProductRepository productRepository;
private final ApplicationEventPublisher eventPublisher;

public BrandDeletedEventHandler(ProductRepository productRepository) {
public BrandDeletedEventHandler(ProductRepository productRepository,
ApplicationEventPublisher eventPublisher) {
this.productRepository = productRepository;
this.eventPublisher = eventPublisher;
}

@EventListener
public void handle(BrandDeletedEvent event) {
List<Product> products = productRepository.findAllByBrandId(event.brandId());
List<Long> deletedProductIds = new ArrayList<>();
for (Product product : products) {
if (!product.isDeleted()) {
productRepository.save(product.delete());
deletedProductIds.add(product.getId());
}
}
eventPublisher.publishEvent(new BrandProductsDeletedEvent(event.brandId(), deletedProductIds));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import com.loopers.domain.model.like.event.ProductLikedEvent;
import com.loopers.domain.model.like.event.ProductUnlikedEvent;
import com.loopers.domain.model.product.Product;
import com.loopers.domain.repository.ProductRepository;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;

Expand All @@ -20,17 +17,11 @@ public LikeEventHandler(ProductRepository productRepository) {

@EventListener
public void handle(ProductLikedEvent event) {
Product product = productRepository.findActiveByIdWithLock(event.productId())
.orElseThrow(() -> new CoreException(ErrorType.PRODUCT_NOT_FOUND));
Product updated = product.increaseLikeCount();
productRepository.save(updated);
productRepository.incrementLikeCount(event.productId());
}

@EventListener
public void handle(ProductUnlikedEvent event) {
Product product = productRepository.findActiveByIdWithLock(event.productId())
.orElseThrow(() -> new CoreException(ErrorType.PRODUCT_NOT_FOUND));
Product updated = product.decreaseLikeCount();
productRepository.save(updated);
productRepository.decrementLikeCount(event.productId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.loopers.domain.model.common.DomainEventPublisher;
import com.loopers.domain.model.like.Like;
import com.loopers.domain.model.product.Product;
import com.loopers.domain.model.user.UserId;
import com.loopers.domain.repository.LikeRepository;
import com.loopers.domain.repository.ProductRepository;
Expand All @@ -28,7 +27,7 @@ public LikeService(LikeRepository likeRepository, ProductRepository productRepos

@Override
public void like(UserId userId, Long productId) {
findProductWithLock(productId);
validateProductExists(productId);

if (likeRepository.existsByUserIdAndProductId(userId, productId)) {
return;
Expand All @@ -41,7 +40,7 @@ public void like(UserId userId, Long productId) {

@Override
public void unlike(UserId userId, Long productId) {
findProductWithLock(productId);
validateProductExists(productId);

likeRepository.findByUserIdAndProductId(userId, productId)
.ifPresent(like -> {
Expand All @@ -51,8 +50,8 @@ public void unlike(UserId userId, Long productId) {
});
}

private Product findProductWithLock(Long productId) {
return productRepository.findActiveByIdWithLock(productId)
private void validateProductExists(Long productId) {
productRepository.findActiveById(productId)
.orElseThrow(() -> new CoreException(ErrorType.PRODUCT_NOT_FOUND));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,17 @@
import com.loopers.domain.repository.OrderRepository;
import com.loopers.domain.repository.ProductRepository;
import com.loopers.domain.repository.UserCouponRepository;
import com.loopers.infrastructure.cache.CacheConfig;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.cache.Cache;
import org.springframework.cache.CacheManager;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;

import java.math.BigDecimal;
import java.util.Comparator;
Expand All @@ -24,20 +31,24 @@
@Transactional
public class OrderService implements CreateOrderUseCase, CancelOrderUseCase, UpdateDeliveryAddressUseCase {

private static final Logger log = LoggerFactory.getLogger(OrderService.class);

private final OrderRepository orderRepository;
private final ProductRepository productRepository;
private final CouponRepository couponRepository;
private final UserCouponRepository userCouponRepository;
private final DomainEventPublisher eventPublisher;
private final CacheManager cacheManager;

public OrderService(OrderRepository orderRepository, ProductRepository productRepository,
CouponRepository couponRepository, UserCouponRepository userCouponRepository,
DomainEventPublisher eventPublisher) {
DomainEventPublisher eventPublisher, CacheManager cacheManager) {
this.orderRepository = orderRepository;
this.productRepository = productRepository;
this.couponRepository = couponRepository;
this.userCouponRepository = userCouponRepository;
this.eventPublisher = eventPublisher;
this.cacheManager = cacheManager;
}

@Override
Expand Down Expand Up @@ -83,6 +94,35 @@ public void createOrder(UserId userId, OrderCommand command) {
discountAmount, command.couponId());

orderRepository.save(order);

// 4. 트랜잭션 커밋 후 영향받은 상품의 캐시만 개별 무효화
List<Long> affectedProductIds = sortedItems.stream()
.map(CreateOrderUseCase.OrderItemCommand::productId)
.toList();
registerAfterCommitCacheEviction(affectedProductIds);
}

private void registerAfterCommitCacheEviction(List<Long> productIds) {
if (!TransactionSynchronizationManager.isSynchronizationActive()) {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_DETAIL);
if (cache != null) {
productIds.forEach(cache::evict);
}
return;
}
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override
public void afterCommit() {
try {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_DETAIL);
if (cache != null) {
productIds.forEach(cache::evict);
}
} catch (RuntimeException e) {
log.warn("주문 후 캐시 무효화 실패 - productIds: {}, error: {}", productIds, e.getMessage());
}
}
});
}

private Money processCoupon(UserId userId, Long userCouponId, List<OrderLine> orderLines) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import com.loopers.domain.model.product.Product;
import com.loopers.domain.repository.BrandRepository;
import com.loopers.domain.repository.ProductRepository;
import com.loopers.infrastructure.cache.CacheConfig;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.util.List;
Expand All @@ -26,6 +28,7 @@ public ProductQueryService(ProductRepository productRepository, BrandRepository
}

@Override
@Cacheable(value = CacheConfig.PRODUCT_DETAIL, key = "#productId")
public ProductDetailInfo getProduct(Long productId) {
Product product = productRepository.findActiveById(productId)
.orElseThrow(() -> new CoreException(ErrorType.PRODUCT_NOT_FOUND));
Expand All @@ -48,6 +51,7 @@ public ProductDetailInfo getProduct(Long productId) {
}

@Override
@Cacheable(value = CacheConfig.PRODUCT_LIST, key = "'brand:' + #brandId + ':sort:' + #sort + ':page:' + #page + ':size:' + #size")
public PageResult<ProductSummaryInfo> getProducts(Long brandId, String sort, int page, int size) {
Comment on lines 53 to 55
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

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
# PRODUCT_LIST 캐시 eviction 로직 확인
rg -n "PRODUCT_LIST" --type=java -A 5 -B 2

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

Length of output: 6442


🏁 Script executed:

#!/bin/bash
# ProductCacheEvictHandler 호출 위치 확인
rg -n "ProductCacheEvictHandler\|evictAllProductCaches\|evictProductListAfterCommit" --type=java -B 3 -A 3

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

Length of output: 67


🏁 Script executed:

#!/bin/bash
# ProductService의 캐시 무효화 호출 위치 확인
rg -n "evictProductListAfterCommit" --type=java -B 5 -A 2

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

Length of output: 4100


🏁 Script executed:

#!/bin/bash
# 전체 ProductService 내용 확인 (특히 mutation 메서드들)
cat -n apps/commerce-api/src/main/java/com/loopers/application/product/ProductService.java | head -150

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

Length of output: 5115


PRODUCT_LIST 캐시 전체 무효화 전략 개선 필요하다.

원래 지적한 brandId=null 캐시 키 문제는 실제로는 이슈가 아니다. evictProductListAfterCommit()cache.clear()로 전체 캐시를 무효화하기 때문이다. 다만, 이 전략에는 성능상 문제가 있다.

상품 생성/수정/삭제 시마다 전체 PRODUCT_LIST 캐시를 비운다. 캐시는 page/size/sort 조합마다 별도 엔트리를 가지므로, 단일 상품 변경으로도 모든 페이지 결과가 무효화된다. TTL이 1분으로 매우 짧은 상황에서 이 캐시의 효과를 의심할 수 있다.

대신 변경된 상품의 brandId에 해당하는 캐시 키만 선택적으로 무효화하거나, 캐시 키 설계를 단순화하여 범위 무효화를 지원하도록 개선하라. 현재 PRODUCT_DETAIL은 선택적 evict(productId)를 사용하는데, PRODUCT_LIST도 동일한 패턴으로 개선할 수 있다.

성능 영향을 모니터링하고, 캐시 히트율이 낮으면 TTL 조정이나 캐시 전략 재검토를 수행하라.

🤖 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 53 - 55, The PRODUCT_LIST cache is currently fully cleared by
evictProductListAfterCommit(), causing poor performance; change the strategy to
evict only keys related to the affected brand or use a simpler namespace so
range invalidation is possible: modify evictProductListAfterCommit() to compute
and evict entries for CacheConfig.PRODUCT_LIST scoped to the affected brandId
(or call cache.evict("'brand:' + brandId + ':*'") / use a cache API that
supports pattern eviction) instead of cache.clear(), and update getProducts(...)
and any producers (create/update/delete flows) to supply the brandId to that
evict call similar to how PRODUCT_DETAIL is evicted by id so only relevant list
entries are removed.

Copy link
Author

Choose a reason for hiding this comment

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

현 시점에서는 적절한 판단이라고 생각이 든다. 실제로 개선이 필요한 부분 혹은 시점은 어드민 수정이 빈번해지거나 캐시 히트율이 낮게 측정될 때라고 생각이 듭니다.

Copy link

Choose a reason for hiding this comment

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

@ukukdin, 동의한다. 현재 TTL이 1분으로 짧고 쓰기 빈도가 낮은 상황에서는 cache.clear()로 전체 무효화하는 단순 전략이 운영 부담 없이 정합성을 보장하는 적절한 선택이다.

다만, 향후 개선 시점 판단을 위해 아래 두 가지 지표를 사전에 Micrometer로 수집해 두길 권장한다:

  • cache.gets + cache.puts 태그로 캐시 히트율 추적
  • 상품 생성/수정/삭제 빈도 (어드민 트래픽)

이 수치가 임계점을 초과할 때 brandId 범위 무효화 전략으로 전환하면 충분하다.


✏️ Learnings added
Learnt from: ukukdin
Repo: Loopers-dev-lab/loop-pack-be-l2-vol3-java PR: 203
File: apps/commerce-api/src/main/java/com/loopers/application/product/ProductQueryService.java:53-55
Timestamp: 2026-03-13T05:44:03.072Z
Learning: In Loopers-dev-lab/loop-pack-be-l2-vol3-java PR `#203`, the PRODUCT_LIST cache eviction strategy in ProductService.evictProductListAfterCommit() intentionally uses cache.clear() (full cache invalidation) rather than selective eviction by brandId. This is a deliberate trade-off: TTL is 1 minute and admin write frequency is currently low, so full invalidation is considered acceptable. Selective eviction by brandId is deferred until admin modifications become frequent or cache hit rates are measured to be low via Micrometer metrics.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: jsj1215
Repo: Loopers-dev-lab/loop-pack-be-l2-vol3-java PR: 142
File: apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java:80-94
Timestamp: 2026-03-06T06:41:42.198Z
Learning: In `ProductRepositoryImpl.java` (Loopers-dev-lab/loop-pack-be-l2-vol3-java), `findOptionByIdWithLock()` uses `entityManager.find(ProductOption.class, optionId, LockModeType.PESSIMISTIC_WRITE)` without a lock timeout hint. The call flow is: `OrderService.prepareOrderItems()` → `findProductOnly()` (loads only Product, not ProductOption) → `deductStock()` → `findOptionByIdWithLock()`. Since ProductOption is never loaded into the 1st-level cache before this point, there is no stale read risk. However, the lock timeout hint (`jakarta.persistence.lock.timeout`) is missing, risking infinite wait. Fix: use the 4-parameter `entityManager.find()` overload with `Map.of("jakarta.persistence.lock.timeout", 3000L)`.

Learnt from: Namjin-kimm
Repo: Loopers-dev-lab/loop-pack-be-l2-vol3-java PR: 152
File: apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java:86-91
Timestamp: 2026-03-06T09:00:23.795Z
Learning: In Loopers-dev-lab/loop-pack-be-l2-vol3-java PR `#152`, OrderFacade.java's deadlock prevention for multi-product orders relies on MySQL InnoDB's implicit behavior of scanning IN-clause PK queries in ascending order (B+Tree clustered index traversal). This is a deliberate design decision acknowledged by the author as a trade-off: InnoDB generally locks rows in PK ascending order for IN queries, but this is not a formal SQL standard guarantee. If the query optimizer changes execution plans (e.g., full table scan), lock order may change and deadlocks could occur. The safe alternative is adding explicit ORDER BY id to the SELECT FOR UPDATE query in ProductJpaRepository.findAllByIdsForUpdate(). InnoDB does auto-detect and roll back deadlocked transactions, so hangs are avoided but transaction failures still occur.

Learnt from: madirony
Repo: Loopers-dev-lab/loop-pack-be-l2-vol3-java PR: 0
File: :0-0
Timestamp: 2026-03-06T04:36:52.939Z
Learning: In PR `#148` of Loopers-dev-lab/loop-pack-be-l2-vol3-java, ALL WithLock repository methods — including `OrderJpaRepository.findByIdWithLock` — have `QueryHints(QueryHint(name = "jakarta.persistence.lock.timeout", value = "3000"))` applied consistently. No WithLock method is missing the timeout hint. This was confirmed by reading the full file content of OrderJpaRepository.java.

Learnt from: yoon-yoo-tak
Repo: Loopers-dev-lab/loop-pack-be-l2-vol3-java PR: 6
File: apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java:9-10
Timestamp: 2026-02-04T05:52:09.963Z
Learning: 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. Do not introduce MethodArgumentNotValidException handlers or Bean Validation handling, as that would create inconsistent error handling patterns. This guidance applies to all Java files under the commerce-api module (source code under apps/commerce-api/src/main/java/).

PageResult<Product> products = productRepository.findAllActive(brandId, sort, page, size);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,38 @@
package com.loopers.application.product;

import com.loopers.application.product.CreateProductUseCase;
import com.loopers.application.product.DeleteProductUseCase;
import com.loopers.application.product.UpdateProductUseCase;
import com.loopers.domain.model.product.Price;
import com.loopers.domain.model.product.Product;
import com.loopers.domain.model.product.ProductName;
import com.loopers.domain.model.product.Stock;
import com.loopers.domain.repository.BrandRepository;
import com.loopers.domain.repository.ProductRepository;
import com.loopers.infrastructure.cache.CacheConfig;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.cache.Cache;
import org.springframework.cache.CacheManager;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;

@Service
@Transactional
public class ProductService implements CreateProductUseCase, UpdateProductUseCase, DeleteProductUseCase {

private static final Logger log = LoggerFactory.getLogger(ProductService.class);

private final ProductRepository productRepository;
private final BrandRepository brandRepository;
private final CacheManager cacheManager;

public ProductService(ProductRepository productRepository, BrandRepository brandRepository) {
public ProductService(ProductRepository productRepository, BrandRepository brandRepository,
CacheManager cacheManager) {
this.productRepository = productRepository;
this.brandRepository = brandRepository;
this.cacheManager = cacheManager;
}

@Override
Expand All @@ -35,6 +44,8 @@ public void createProduct(ProductCreateCommand command) {
Product product = Product.create(command.brandId(), ProductName.of(command.name()),
Price.of(command.price()), salePriceVo, Stock.of(command.stock()), command.description());
productRepository.save(product);

evictProductListAfterCommit();
}

@Override
Expand All @@ -44,17 +55,61 @@ public void updateProduct(ProductUpdateCommand command) {
Product updated = product.update(ProductName.of(command.name()), Price.of(command.price()),
salePriceVo, Stock.of(command.stock()), command.description());
productRepository.save(updated);

evictProductAfterCommit(command.productId());
evictProductListAfterCommit();
}

@Override
public void deleteProduct(Long productId) {
Product product = findProduct(productId);
Product deleted = product.delete();
productRepository.save(deleted);

evictProductAfterCommit(productId);
evictProductListAfterCommit();
}

private Product findProduct(Long productId) {
return productRepository.findActiveById(productId)
.orElseThrow(() -> new CoreException(ErrorType.PRODUCT_NOT_FOUND));
}

private void evictProductAfterCommit(Long productId) {
if (!TransactionSynchronizationManager.isSynchronizationActive()) {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_DETAIL);
if (cache != null) cache.evict(productId);
return;
}
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override
public void afterCommit() {
try {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_DETAIL);
if (cache != null) cache.evict(productId);
} catch (RuntimeException e) {
log.warn("상품 캐시 무효화 실패 - productId: {}, error: {}", productId, e.getMessage());
}
}
});
}

private void evictProductListAfterCommit() {
if (!TransactionSynchronizationManager.isSynchronizationActive()) {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_LIST);
if (cache != null) cache.clear();
return;
}
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override
public void afterCommit() {
try {
Cache cache = cacheManager.getCache(CacheConfig.PRODUCT_LIST);
if (cache != null) cache.clear();
} catch (RuntimeException e) {
log.warn("상품 목록 캐시 무효화 실패 - error: {}", e.getMessage());
}
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.loopers.domain.model.brand.event;

import java.util.List;

public record BrandProductsDeletedEvent(
Long brandId,
List<Long> deletedProductIds
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,8 @@ public interface ProductRepository {
PageResult<Product> findAllActive(Long brandId, String sort, int page, int size);

List<Product> findAllByBrandId(Long brandId);

void incrementLikeCount(Long productId);

void decrementLikeCount(Long productId);
}
Loading