Round5: 성능 개선, 캐시 #146
Round5: 성능 개선, 캐시 #146
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough쿠폰, 가격 연산, 제품 캐시(Redis)와 관련 인프라를 추가하고 좋아요/메트릭/주문/포인트 흐름에 트랜잭션·락·캐시 무결성 로직을 도입하며 대량 데이터 생성·성능 테스트를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant OrderFacade
participant ProductService
participant CouponService
participant PointService
participant OrderService
participant Database
Client->>OrderFacade: 주문 요청 (items, couponId?)
OrderFacade->>ProductService: 제품 조회 (ID 목록)
ProductService->>Database: 제품 조회
Database-->>ProductService: 제품들
OrderFacade->>OrderFacade: 원가 계산
alt couponId 존재
OrderFacade->>CouponService: applyCoupon(couponId, userId, originalPrice)
CouponService->>Database: 쿠폰 조회 (for update)
Database-->>CouponService: 쿠폰
CouponService->>Database: 쿠폰 사용 업데이트
CouponService-->>OrderFacade: DiscountResult
else
OrderFacade->>OrderFacade: DiscountResult(원가)
end
OrderFacade->>PointService: checkAndDeductPoint(userId, finalPrice)
PointService->>Database: 포인트 조회 (for update)
Database-->>PointService: 포인트
PointService->>Database: 포인트 차감
PointService-->>OrderFacade: 완료
OrderFacade->>OrderService: createOrder(productQuantityMap, userId, DiscountResult)
OrderService->>Database: 주문 저장 (order, items, 재고감소 등)
Database-->>OrderService: 저장 완료
OrderService-->>OrderFacade: 주문 생성 결과
OrderFacade-->>Client: 응답 (주문 완료)
sequenceDiagram
actor User
participant LikeProductFacade
participant LikeProductService
participant ProductMetricsService
participant ProductCacheService
participant Database
User->>LikeProductFacade: likeProduct(userId, productId)
LikeProductFacade->>LikeProductService: likeProduct (트랜잭션)
LikeProductService->>Database: findByUserIdAndProductId (낙관적 잠금)
Database-->>LikeProductService: like 엔티티(Optional)
LikeProductService->>Database: 저장/복원/삭제
LikeProductService-->>LikeProductFacade: LikeResult (liked, beforeLiked)
alt 상태 변경 발생
LikeProductFacade->>ProductMetricsService: incrementLikeCount / decrementLikeCount (트랜잭션)
ProductMetricsService->>Database: findByProductIdForUpdate
ProductMetricsService->>Database: metrics 저장
ProductMetricsService-->>LikeProductFacade: 완료
LikeProductFacade->>ProductCacheService: invalidateProductDetail / invalidateProductList
ProductCacheService->>Redis: delete key(s)
end
LikeProductFacade-->>User: 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 검토 시 주의할 항목:
Possibly related PRs
Suggested labels
시
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (34)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.java (1)
95-107: 하드코딩된 productId를 동적 ID로 변경해야 합니다.테스트가
productId = 1L을 하드코딩하고 있지만, setup 메서드에서는 동적으로 생성된 ID를productId1변수에 저장합니다. 이는 다음과 같은 문제를 야기할 수 있습니다:
- 데이터베이스에 기존 데이터가 있거나 테스트 실행 순서가 변경되면 auto-increment ID가 1이 아닐 수 있음
- 같은 클래스의 다른 테스트들(lines 143-145)은 올바르게
productId1,productId2,productId3를 사용함- 테스트 격리(test isolation)가 깨지고 취약한(brittle) 테스트가 됨
다음 diff를 적용하여 수정하세요:
@DisplayName("존재하는 상품 ID로 조회하면 상품을 반환한다. (Happy Path)") @Test void should_returnProduct_when_productExists() { // arrange - Long productId = 1L; + Long productId = productId1; // act Product result = productService.getProductById(productId); // assert - verify(spyProductRepository).findById(1L); + verify(spyProductRepository).findById(productId1); assertThat(result).isNotNull(); - assertThat(result.getId()).isEqualTo(1L); + assertThat(result.getId()).isEqualTo(productId1); assertThat(result.getName()).isEqualTo("상품1"); }
🧹 Nitpick comments (21)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.java (1)
54-61: 페이지size필드 의미 변경에 따른 API 호환성 확인 제안
size인자에page.getSize()대신page.getNumberOfElements()를 넘기면서, 필드 이름은 그대로지만 의미가 “요청된 페이지 크기”에서 “현재 페이지의 실제 요소 개수”로 바뀌었습니다. 자체적으로는 더 직관적인 값이긴 한데, 이미 이 DTO를 사용하는 클라이언트가size를 “pageSize”로 가정하고 있다면 미묘한 브레이킹 체인지가 될 수 있습니다.
- 이 변경이 의도한 스펙인지(예: 다른 DTO와의 정합성 맞추기) 한 번 더 확인해 주시고,
- 외부/프런트 클라이언트가 존재한다면 스펙 문서 및 계약이 이 의미 변화와 일치하는지 검토를 부탁드립니다.
- 만약 둘 다 필요하다면
size(page size)와count(numberOfElements)처럼 필드를 분리하는 것도 옵션일 수 있습니다.apps/commerce-api/src/main/java/com/loopers/application/like/product/LikeProductFacade.java (1)
38-46: 트랜잭션 경계 설정은 적절하나, metrics 업데이트의 동시성 문제 검토 필요
likeProductService.likeProduct()와productMetricsService.incrementLikeCount()가 같은 트랜잭션 내에서 실행되는 것은 좋습니다. 그러나ProductMetricsService.incrementLikeCount()에서 read-then-write 패턴이 사용되고 있어, 동시 요청 시 lost update 문제가 발생할 수 있습니다.Based on learnings, 이 코드베이스에서는 좋아요 기능에 비관적 락(
findByIdForUpdate)을 사용하는 것이 권장됩니다.ProductMetrics조회 시에도 동일한 락 전략 적용을 검토해 주세요.apps/commerce-api/src/test/java/com/loopers/application/like/product/LikeProductFacadeIntegrationTest.java (2)
94-113:userEntityIds생성 시 불필요한 DB 재조회 - 리팩토링 권장
userIds생성 시 이미savedUser객체를 가지고 있었으나,userEntityIds를 생성할 때 다시 DB에서 조회하고 있습니다. 테스트 데이터 양이 적어 성능 영향은 크지 않지만, 코드를 간결하게 개선할 수 있습니다.- userIds = IntStream.range(1, 11) - .mapToObj(i -> { - User user = User.create("user" + i, "user" + i + "@test.com", "1993-03-13", "male"); - User savedUser = userJpaRepository.save(user); - return savedUser.getUserId(); - }) - .toList(); - - // userIds와 같은 순서로 userEntityIds 생성 (같은 사용자) - userEntityIds = IntStream.range(1, 11) - .mapToObj(i -> { - // userIds.get(i-1)에 해당하는 사용자의 entityId를 찾기 - String userId = userIds.get(i - 1); - return userJpaRepository.findByUserId(userId) - .orElseThrow() - .getId(); - }) - .toList(); + List<User> savedUsers = IntStream.range(1, 11) + .mapToObj(i -> { + User user = User.create("user" + i, "user" + i + "@test.com", "1993-03-13", "male"); + return userJpaRepository.save(user); + }) + .toList(); + + userIds = savedUsers.stream().map(User::getUserId).toList(); + userEntityIds = savedUsers.stream().map(User::getId).toList();
142-146: 테스트 코드에서System.out.println대신 로거 사용 권장테스트 실패 메시지를
System.out.println으로 출력하고 있습니다. 로거를 사용하면 로그 레벨 제어와 테스트 출력 관리가 용이합니다.apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.java (1)
133-146: 테스트 일관성을 위해 동적 ID를 사용하세요.arrange 단계에서
List.of(1L, 2L, 3L)을 하드코딩하고 있지만, assert 단계에서는 동적 변수productId1,productId2,productId3를 사용합니다. 테스트 전체에서 일관되게 동적 ID를 사용하는 것이 좋습니다.다음 diff를 적용하여 개선하세요:
@DisplayName("존재하는 상품 ID들로 조회하면 상품 맵을 반환한다. (Happy Path)") @Test void should_returnProductMap_when_productsExist() { // arrange - List<Long> productIds = List.of(1L, 2L, 3L); + List<Long> productIds = List.of(productId1, productId2, productId3); // act Map<Long, Product> result = productService.getProductMapByIds(productIds);apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java (1)
67-69: 빈 주문 아이템에 대한 방어 로직 고려
productQuantityMap이 비어있는 경우reduce의 초기값인new Price(0)이 반환됩니다. 이로 인해 0원 주문이 생성될 수 있습니다.request.validate()에서 이미 처리되고 있다면 무시해도 됩니다.apps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponTest.java (1)
193-331: nullPrice입력에 대한 테스트 케이스 추가 고려
applyDiscount메서드에nullPrice가 전달되는 경우의 테스트가 없습니다. 프로덕션 코드에서 null 체크를 하고 있다면 해당 동작을 검증하는 테스트 추가를 권장합니다.@DisplayName("null 가격으로 쿠폰을 적용할 경우, 예외가 발생한다. (Exception)") @Test void should_throwException_when_priceIsNull() { // arrange Coupon coupon = Coupon.issueFixed(1L, 5000); Price originalPrice = null; // act & assert assertThrows(CoreException.class, () -> coupon.applyDiscount(originalPrice)); }apps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.java (1)
19-38: 매핑 로직 중복 개선 고려
new BrandInfo(brand.getId(), brand.getName())매핑이 3곳에서 반복됩니다.BrandInfo에 정적 팩토리 메서드를 추가하면 유지보수성이 향상됩니다.
BrandInfo레코드에 추가:public record BrandInfo(Long id, String name) { public static BrandInfo from(Brand brand) { return new BrandInfo(brand.getId(), brand.getName()); } }그 후 파사드에서 활용:
-return new BrandInfo(brand.getId(), brand.getName()); +return BrandInfo.from(brand);apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java (1)
3-3: 사용되지 않는 import입니다.
Priceimport가 이 파일에서 사용되지 않습니다. AI 요약에 따르면calculateTotalAmount메서드가 제거되면서 남은 것으로 보입니다.-import com.loopers.domain.common.vo.Price;apps/commerce-api/src/main/java/com/loopers/application/product/ProductSearchRequest.java (2)
40-49: 정렬 문자열 파싱이 취약할 수 있습니다.
sort.toString().split(":")[0]방식은 Spring의Sort.toString()출력 형식에 의존합니다. 출력 형식이 변경되거나 예상치 못한 형식이 들어오면 잘못된 결과를 반환할 수 있습니다.
SortAPI를 직접 사용하여 정렬 속성을 검증하는 것이 더 안전합니다:private void validate() { Sort sort = pageable().getSort(); - String sortStr = sort.toString().split(":")[0]; - if (!(sort.isUnsorted() - || sortStr.equals("like_desc") - || sortStr.equals("price_asc") - || sortStr.equals("latest"))) { + if (sort.isUnsorted()) { + return; + } + Set<String> allowedSorts = Set.of("like_desc", "price_asc", "latest"); + for (Sort.Order order : sort) { + if (!allowedSorts.contains(order.getProperty())) { throw new IllegalArgumentException("유효하지 않은 정렬 기준입니다."); } + } }
31-38: Filter 레코드 구조가 적절합니다.향후 확장을 위한 주석 처리된 필드들이 있어 확장성이 고려되었습니다.
brandIds가 null일 경우를 대비해 생성자에서 방어적 처리를 추가하면 더 안전합니다.public record Filter( List<Long> brandIds ) { + public Filter { + brandIds = brandIds != null ? List.copyOf(brandIds) : List.of(); + } }apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceIntegrationTest.java (1)
51-53: import 문을 사용하여 코드 가독성을 개선하세요.
com.loopers.domain.common.vo.DiscountResult를 FQN(Fully Qualified Name)으로 사용하고 있습니다. import 문을 추가하면 코드가 더 깔끔해집니다.파일 상단에 import 추가:
import com.loopers.domain.common.vo.DiscountResult;그리고 사용 부분을 간소화:
-com.loopers.domain.common.vo.DiscountResult discountResult = new com.loopers.domain.common.vo.DiscountResult( - new Price(40000) // 10000 * 2 + 20000 * 1 -); +DiscountResult discountResult = new DiscountResult( + new Price(40000) // 10000 * 2 + 20000 * 1 +);apps/commerce-api/src/main/java/com/loopers/domain/order/Order.java (1)
68-74: 중복된 orderItems 유효성 검사가 있습니다.
create(Long userId, List<OrderItem> orderItems)메서드에서orderItemsnull/empty 검사를 수행하고, 이후 호출되는 생성자에서도 동일한 검사를 수행합니다. 중복을 제거하거나, 이 메서드에서는 검사를 제거하고 생성자에서만 처리하는 것이 좋습니다.public static Order create(Long userId, List<OrderItem> orderItems) { - if (orderItems == null || orderItems.isEmpty()) { - throw new CoreException(ErrorType.BAD_REQUEST, "주문 항목은 최소 1개 이상이어야 합니다."); - } Price originalPrice = new Price(orderItems.stream().mapToInt(OrderItem::getTotalPrice).sum()); return create(userId, orderItems, new DiscountResult(originalPrice)); }apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java (3)
165-212: 동시성 테스트와 유사한 테스트가 중복됩니다.이 테스트(
concurrencyTest_stockShouldBeProperlyDecreasedWhenOrdersCreated, Line 165)와 Line 965의concurrencyTest_stockShouldBeProperlyDecreasedWhenConcurrentOrders테스트가 동일한 시나리오를 검증합니다. 중복 테스트를 제거하거나 하나로 통합하는 것을 권장합니다.
192-194: 테스트에서System.out.println대신 로거 사용을 권장합니다.동시성 테스트에서 실패 메시지를
System.out.println으로 출력하고 있습니다. 테스트 환경에서도 SLF4J 로거를 사용하면 로그 레벨 관리와 테스트 리포트 통합이 용이합니다.+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class OrderFacadeIntegrationTest { + private static final Logger log = LoggerFactory.getLogger(OrderFacadeIntegrationTest.class); // ... - System.out.println("실패: " + e.getMessage()); + log.warn("주문 실패: {}", e.getMessage());
283-310: 포인트 부족 시 예외 테스트와 롤백 테스트가 중복됩니다.
should_throwException_when_pointInsufficient(Line 283)와should_rollback_when_pointInsufficient(Line 870)가 유사한 시나리오를 테스트합니다. 하나의 테스트에서 예외 발생과 롤백을 함께 검증하는 것이 효율적입니다.apps/commerce-api/src/test/java/com/loopers/performance/data/BulkDataGenerator.java (1)
18-34: 대량 ID 리스트 유지로 인한 메모리 사용 고려
generateRound5Data에서productGenerator.generateProducts가 돌려주는List<Long> productIds를 전부 메모리에 들고 있다가size()만 사용하는데, 상품 수가 수십만·수백만 건 이상으로 커지면 불필요한 메모리 사용이 커질 수 있습니다. 단순 개수만 필요하다면 개수만 반환하는 전용 API를 두거나, 이 메서드에서는 ID 리스트를 굳이 유지하지 않는 방향으로 리팩터링을 한 번 고려해 보셔도 좋겠습니다. 테스트용이라도System.out.println대신 Logger를 쓰면 로그 레벨로 필터링하기도 수월합니다.apps/commerce-api/build.gradle.kts (1)
14-17: Faker 의존성 범위(test 전용 여부) 한 번 더 점검 제안현재 변경 내용만 보면
javafaker는src/test하위(ProductDataGenerator등)에서만 사용되고 있어서,implementation대신testImplementation만 남겨도 충분해 보입니다. 이렇게 하면 프로덕션 클래스패스/아티팩트에 불필요한 의존성이 포함되지 않아 의존성 관리가 조금 더 깔끔해집니다. 만약 이후 main 소스에서도 사용할 계획이라면 지금 구조도 괜찮습니다.또, snakeyaml을 exclude 해두셨는데, 스프링 부트 쪽에서 끌어오는 snakeyaml 버전과 함께 사용할 때
javafaker가 YAML 로딩 시 문제 없이 동작하는지 테스트 실행으로 한 번만 확인 부탁드립니다.Also applies to: 27-29
apps/commerce-api/src/test/java/com/loopers/performance/data/ProductDataGenerator.java (3)
21-27: Faker/Random 시드 주입으로 재현 가능한 시나리오도 대비 가능테스트마다 데이터가 달라지는 것은 장점이지만, 성능 수치를 여러 번 비교해야 하는 경우에는 동일한 데이터 셋을 다시 만들어야 할 때가 있습니다. 현재
Faker와Random을 필드에서 바로 생성하고 있어서 시드 제어가 어려우므로, 생성자 인자로 시드를 받거나 테스트에서 주입 가능한 팩토리/빌더를 두어 재현 가능한 시나리오도 선택할 수 있게 해 두면 활용성이 더 높아질 것 같습니다.
34-77: 대용량 생성 시 단일 트랜잭션·ID 리스트로 인한 메모리/락 부담 가능성
generateProducts전체에@Transactional이 걸려 있고, 모든 생성 결과를productIds리스트에 담고 있어서, 생성 건수가 매우 커질 경우 하나의 트랜잭션 안에서 영속성 컨텍스트와 ID 리스트가 모두 커질 수 있습니다. 성능 테스트 규모를 크게 가져갈 계획이라면:
- 트랜잭션 경계를 배치 단위로 나누는지,
- 혹은
productFacade.createProductBulk내부에서 적절히flush/clear하거나 별도 트랜잭션(REQUIRES_NEW등)으로 처리하는지를 한 번 점검해 보시면 장시간 테스트에서의 메모리 사용과 락 유지 시간을 줄이는 데 도움이 될 것 같습니다.
79-109:brandCount·batchSize에 대한 간단한 파라미터 검증 추가 제안현재
ensureBrandsExist는brandCount와batchSize가 0 이하인 경우에 대한 명시적인 가드가 없습니다. 잘못된 값이 들어오면Pageable.ofSize(0)같은 부분이나, 이후brandIds.get(random.nextInt(brandIds.size()))에서 예외가 발생할 수 있어 보입니다. 성능 테스트용 유틸이긴 하지만, 초기에brandCount > 0,batchSize > 0정도를 assert/gard 해 두면 잘못된 호출을 더 빨리 잡는 데 도움이 될 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
apps/commerce-api/build.gradle.kts(2 hunks)apps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/brand/BrandInfo.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/like/product/LikeProductFacade.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java(3 hunks)apps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/order/OrderRequest.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/product/ProductCreateRequest.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/application/product/ProductSearchRequest.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/common/vo/DiscountResult.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/common/vo/Price.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponService.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponType.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProduct.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProductRepository.java(0 hunks)apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProductService.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetrics.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsRepository.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsService.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/order/Order.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java(3 hunks)apps/commerce-api/src/main/java/com/loopers/domain/point/PointRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java(3 hunks)apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java(4 hunks)apps/commerce-api/src/main/java/com/loopers/domain/supply/Supply.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/supply/SupplyRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/supply/SupplyService.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandRepositoryImpl.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponRepositoryImpl.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductRepositoryImpl.java(0 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/metrics/product/ProductMetricsRepositoryImpl.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/supply/SupplyRepositoryImpl.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/like/product/LikeProductV1Dto.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Dto.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/application/like/product/LikeProductFacadeIntegrationTest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java(13 hunks)apps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponTest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/domain/order/OrderItemTest.java(2 hunks)apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceIntegrationTest.java(3 hunks)apps/commerce-api/src/test/java/com/loopers/domain/order/OrderTest.java(2 hunks)apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.java(2 hunks)apps/commerce-api/src/test/java/com/loopers/interfaces/api/LikeProductV1ApiE2ETest.java(2 hunks)apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.java(2 hunks)apps/commerce-api/src/test/java/com/loopers/performance/data/BulkDataGenerator.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/performance/data/ProductDataGenerator.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/performance/fixture/PerformanceTestFixture.java(1 hunks)
💤 Files with no reviewable changes (2)
- apps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductRepositoryImpl.java
- apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProductRepository.java
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-27T09:09:24.961Z
Learnt from: sky980221
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 121
File: apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java:22-24
Timestamp: 2025-11-27T09:09:24.961Z
Learning: Product 엔티티 (apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java)는 유즈케이스별로 의도적으로 다른 락 전략을 사용한다: 좋아요 기능에는 비관적 락(findByIdForUpdate)을, 재고 차감에는 낙관적 락(Version + 재시도)을 사용한다.
Applied to files:
apps/commerce-api/src/test/java/com/loopers/interfaces/api/LikeProductV1ApiE2ETest.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductCreateRequest.javaapps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetrics.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/test/java/com/loopers/performance/data/ProductDataGenerator.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/product/Product.javaapps/commerce-api/src/main/java/com/loopers/application/like/product/LikeProductFacade.javaapps/commerce-api/src/test/java/com/loopers/application/like/product/LikeProductFacadeIntegrationTest.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsService.javaapps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProductService.javaapps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProduct.java
📚 Learning: 2025-11-17T14:23:34.490Z
Learnt from: yeonsu00
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 85
File: apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java:54-60
Timestamp: 2025-11-17T14:23:34.490Z
Learning: In the Point domain entity at apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java, the deduct() method does not validate balance sufficiency. The balance validation is intentionally performed at the service layer in PointService.deductPoint() using hasSufficientAmount() before calling deduct(). This is the team's design decision to handle validation at the service layer rather than within the entity.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/point/PointRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/common/vo/Price.javaapps/commerce-api/src/main/java/com/loopers/domain/point/PointService.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java
📚 Learning: 2025-11-20T13:22:51.519Z
Learnt from: kilian-develop
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 98
File: core/domain/src/main/java/com/loopers/core/domain/order/DefaultCoupon.java:15-62
Timestamp: 2025-11-20T13:22:51.519Z
Learning: In the order domain coupon implementation, DefaultCoupon serves as an internal helper/DTO class to carry AbstractCoupon's base fields during database reconstruction. It is used as an intermediate object when converting CouponEntity to specific coupon types (RateDiscountCoupon or AmountDiscountCoupon). The actual discount information (rate/amount) is stored in separate tables (rate_discount_coupon and amount_discount_coupon) and fetched via RateDiscountCouponJpaRepository and AmountDiscountCouponJpaRepository. DefaultCoupon is never returned to callers directly.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponJpaRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponTest.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponService.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.javaapps/commerce-api/src/main/java/com/loopers/domain/order/Order.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponType.javaapps/commerce-api/src/test/java/com/loopers/domain/order/OrderTest.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/domain/common/vo/DiscountResult.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java
📚 Learning: 2025-11-09T10:41:39.297Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 25
File: apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductRepository.kt:1-12
Timestamp: 2025-11-09T10:41:39.297Z
Learning: In this codebase, domain repository interfaces are allowed to use Spring Data's org.springframework.data.domain.Page and org.springframework.data.domain.Pageable types. This is an accepted architectural decision and should not be flagged as a DIP violation.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/metrics/product/ProductMetricsRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/application/like/product/LikeProductFacade.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsService.java
📚 Learning: 2025-11-25T02:35:20.910Z
Learnt from: CR
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 0
File: .codeguide/loopers-1-week.md:0-0
Timestamp: 2025-11-25T02:35:20.910Z
Learning: Applies to **/*.e2e.(test|spec).{ts,tsx,js,jsx} : User info retrieval E2E test must return 404 Not Found response when attempting to retrieve information for a non-existent user ID
Applied to files:
apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java
🧬 Code graph analysis (2)
apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java (4)
apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java (1)
RequiredArgsConstructor(20-65)apps/commerce-api/src/main/java/com/loopers/domain/supply/SupplyService.java (1)
RequiredArgsConstructor(14-45)apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponService.java (1)
RequiredArgsConstructor(11-34)apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java (1)
RequiredArgsConstructor(18-51)
apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProduct.java (3)
apps/commerce-api/src/main/java/com/loopers/support/error/CoreException.java (1)
Getter(5-19)apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java (1)
Entity(14-53)apps/commerce-api/src/main/java/com/loopers/domain/supply/Supply.java (1)
Entity(13-49)
🔇 Additional comments (56)
apps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsRepository.java (1)
16-19: LGTM!
save와saveAll메서드 추가가 적절합니다.saveAll의 입력으로Collection을 사용하여 유연성을 제공하고, 반환 타입으로List를 사용하여 명확한 계약을 제공합니다.apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProduct.java (2)
10-21: 인덱스 및 유니크 제약조건 추가가 적절합니다.
(product_id, deleted_at)복합 인덱스와(user_id, product_id)유니크 제약조건은 쿼리 성능과 데이터 무결성을 보장합니다.
24-26: 낙관적 락(Optimistic Lock) 전략 변경 확인 필요기존 학습 내용에 따르면 "좋아요 기능에는 비관적 락(findByIdForUpdate)"을 사용하도록 되어 있었습니다. 이번 변경으로
@Version기반 낙관적 락으로 전환되는데, 이 전략 변경이 의도된 것인지 확인해 주세요.낙관적 락 사용 시 동시 좋아요 요청에서
OptimisticLockException발생 가능성이 있으며, 호출하는 쪽에서 재시도 로직이 필요할 수 있습니다.apps/commerce-api/src/main/java/com/loopers/domain/like/product/LikeProductService.java (2)
16-23: LGTM - 트랜잭션 경계 설정
likeProduct메서드에@Transactional추가가 적절합니다.findByUserIdAndProductId조회 후restore또는save작업이 하나의 트랜잭션 내에서 원자적으로 수행됩니다.
25-29: LGTM!
unlikeProduct메서드의@Transactional추가가 적절합니다.참고:
LikeProductFacade에서도 동일 메서드에@Transactional이 있다면, 기본 전파(REQUIRED)로 인해 외부 트랜잭션에 참여하게 됩니다. 트랜잭션 경계를 Facade 레벨에서만 관리하는 것도 고려해 볼 수 있습니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeProductJpaRepository.java (1)
13-14: Unable to verify OptimisticLockException handling due to repository access issuesThe repository could not be cloned to verify whether the calling services (LikeProductService) implement retry logic or exception handling for
OptimisticLockException.Please manually verify:
- LikeProduct entity has
@Versionfield (should be present based on the import of@Lock)- LikeProductService or any facade calling
findByUserIdAndProductIdincludes exception handling forOptimisticLockExceptionor uses@Retryableannotation- If concurrent modifications occur, the retry mechanism gracefully handles version conflicts
apps/commerce-api/src/main/java/com/loopers/infrastructure/metrics/product/ProductMetricsRepositoryImpl.java (1)
34-42: LGTM!JPA Repository로의 위임 패턴이 일관되게 적용되었습니다. 인터페이스 계약을 올바르게 구현하고 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/metrics/product/ProductMetricsService.java (1)
47-51: I'll help you verify the review comment about the lost update vulnerability in theProductMetricsService.incrementLikeCount()method. Let me start by examining the codebase to confirm the concerns and validate the recommendation.
<function_calls>
#!/bin/bashFind ProductMetrics entity and repository files
fd "ProductMetrics" --type=f | grep -E ".(java|kt)$"
</function_calls>
<function_calls>
#!/bin/bashSearch for ProductMetricsRepository implementation
rg -n "class ProductMetricsRepository|interface ProductMetricsRepository" --type=java -A 10
</function_calls>
<function_calls>
#!/bin/bashSearch for pessimistic locking methods in ProductMetrics context
rg -n "ForUpdate|LockModeType|PESSIMISTIC" --type=java -C 2 | head -100
</function_calls>apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointJpaRepository.java (1)
15-17: 비관적 쓰기 락 구현이 적절합니다.포인트 차감과 같은 read-modify-write 작업에
PESSIMISTIC_WRITE락을 사용하는 것은 동시성 문제를 방지하는 올바른 접근입니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponJpaRepository.java (1)
11-16: 쿠폰 리포지토리 구현이 적절합니다.비관적 락 메서드와 일반 조회 메서드를 모두 제공하여 유즈케이스별로 적절한 락 전략을 선택할 수 있습니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.java (1)
31-31: 요청 검증 추가가 적절합니다.userId 검증 이후 요청 데이터를 검증하여 잘못된 요청이 비즈니스 로직에 도달하기 전에 차단합니다.
apps/commerce-api/src/main/java/com/loopers/application/order/OrderInfo.java (1)
17-17: 최종 가격 사용이 적절합니다.할인이 적용된 최종 가격(
getFinalPrice)을 사용하는 것이 주문 정보 표시에 적합합니다.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java (1)
19-21: 저장 메서드 추가가 적절합니다.단건 저장(
save)과 배치 저장(saveAll) 메서드를 추가하여 다양한 영속화 시나리오를 지원합니다.apps/commerce-api/src/test/java/com/loopers/interfaces/api/LikeProductV1ApiE2ETest.java (1)
103-103: 메서드 이름 변경이 잘 반영되었습니다.
saveSupply에서save로의 메서드 이름 변경이 테스트 코드에 올바르게 적용되었습니다.Also applies to: 113-113
apps/commerce-api/src/main/java/com/loopers/domain/point/PointRepository.java (1)
8-8: LGTM! 동시성 제어를 위한 중요한 개선입니다.포인트 차감/충전 시 비관적 잠금을 통해 동시 수정 문제를 방지합니다. 메서드 네이밍도 명확하고 기존
findByUserId와 잘 구분됩니다.apps/commerce-api/src/main/java/com/loopers/application/brand/BrandInfo.java (1)
3-7: LGTM! 깔끔한 DTO 구현입니다.브랜드 정보를 전달하기 위한 간단하고 명확한 record입니다. 애플리케이션 레이어의 파사드에서 사용하기에 적합합니다.
apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.java (1)
83-83: LGTM! SupplyService API 변경에 맞게 테스트를 업데이트했습니다.
saveSupply에서save로 메서드명이 간소화되었고, 테스트가 올바르게 업데이트되었습니다. 새로 추가된 배치 저장 기능(saveAll)과 일관된 API 설계입니다.Also applies to: 93-93
apps/commerce-api/src/main/java/com/loopers/infrastructure/supply/SupplyRepositoryImpl.java (1)
37-40: LGTM! 배치 저장으로 성능을 개선했습니다.Supply 엔티티의 배치 저장 기능을 추가하여 다수의 공급 정보를 한 번에 저장할 수 있습니다. 이는 데이터베이스 왕복 횟수를 줄여 성능을 향상시킵니다.
apps/commerce-api/src/test/java/com/loopers/domain/order/OrderTest.java (1)
135-202: LGTM! DiscountResult 통합에 대한 포괄적인 테스트 커버리지입니다.세 가지 중요한 시나리오를 모두 테스트합니다:
- 쿠폰이 적용된 주문 생성 (Happy Path)
- null DiscountResult 예외 처리 (Exception)
- 쿠폰 없이 주문 생성 (Happy Path)
각 테스트는 명확한 네이밍과 적절한 검증을 포함하고 있어 유지보수가 용이합니다.
apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponType.java (1)
3-6: I encountered a technical issue: the repository failed to clone, which prevents me from executing the verification script requested in the original review comment. Without access to the codebase, I cannot confirm whetherCouponTypeis actually being used or if it's orphaned code.Based on the verification request in the original comment being incomplete, here is the rewritten review comment:
Clear enum definition for coupon types.
The
FIXEDandPERCENTAGEconstants provide a clear distinction between fixed-amount and percentage-based discounts. However, verify that this enum is referenced elsewhere in the codebase to ensure it's not orphaned code. Check usages in the Coupon domain entity and any related service classes.apps/commerce-api/src/main/java/com/loopers/domain/supply/SupplyRepository.java (1)
16-16: 배치 저장 기능 추가가 적절합니다.성능 개선을 위한 배치 저장 메서드 추가가 PR 목적과 일치하며, 구현이 적절합니다.
</review_comment_end>
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (1)
20-23: 비관적 락을 사용한 조회 메서드 추가가 적절합니다.포인트 잔액 업데이트 시 동시성 문제를 방지하기 위한 for-update 조회 메서드가 올바르게 구현되었습니다.
</review_comment_end>
apps/commerce-api/src/main/java/com/loopers/domain/supply/SupplyService.java (2)
38-40: 메서드 이름 단순화가 적절합니다.
saveSupply에서save로의 이름 변경이 더 간결하고 명확합니다.</review_comment_end>
42-44: 배치 저장 메서드 추가가 성능 개선에 기여합니다.여러 Supply 엔티티를 한 번에 저장할 수 있는
saveAll메서드가 PR의 성능 개선 목적과 일치합니다.</review_comment_end>
apps/commerce-api/src/test/java/com/loopers/domain/order/OrderItemTest.java (1)
34-34: 테스트가 도메인 모델 변경을 올바르게 반영합니다.
getPrice()에서getPricePerItem()으로의 API 변경이 테스트에 정확히 적용되었으며, 의미가 더 명확해졌습니다.</review_comment_end>
Also applies to: 86-86
apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponRepository.java (1)
5-9: 쿠폰 리포지토리 설계가 적절합니다.비관적 락을 사용한
findByIdAndUserIdForUpdate메서드가 쿠폰 사용 시 동시성 문제를 방지하며, userId 검증을 통해 소유권 확인도 함께 수행합니다.</review_comment_end>
apps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandRepositoryImpl.java (1)
29-42: 브랜드 리포지토리 기능 확장이 적절합니다.단일/배치 저장 메서드와 페이징 조회 메서드가 추가되어 성능 개선 및 확장성을 지원합니다. 구현이 JPA 리포지토리에 올바르게 위임되고 있습니다.
</review_comment_end>
apps/commerce-api/src/main/java/com/loopers/domain/common/vo/Price.java (2)
14-16: 가격 덧셈 메서드가 올바르게 구현되었습니다.두 가격을 더하는 로직이 간결하고 명확합니다.
</review_comment_end>
18-21: 가격 차감 메서드가 음수 방지 처리를 적절히 수행합니다.
Math.max(0, ...)를 사용하여 결과가 음수가 되는 것을 방지하는 것이 적절합니다.</review_comment_end>
apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandRepository.java (1)
15-19: LGTM! 저장소 API 확장이 적절합니다.단일/배치 저장 및 페이지네이션 조회 메서드 추가가 깔끔하게 구현되었습니다. Spring Data의 Page/Pageable 타입 사용은 이 코드베이스의 아키텍처 결정에 부합합니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java (1)
24-29: LGTM! ProductSearchRequest 도입이 좋은 리팩터링입니다.
Pageable을ProductSearchRequest로 감싸서 파사드 계층에 전달하는 방식은 향후 필터링, 정렬 등 추가 검색 조건을 확장하기에 적합한 설계입니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponRepositoryImpl.java (1)
10-24: LGTM! 표준적인 저장소 구현입니다.JPA 저장소로의 단순 위임 패턴이 올바르게 적용되었습니다.
findByIdAndUserIdForUpdate메서드는 쿠폰 사용 시 동시성 제어를 위한 비관적 락을 사용하는 것으로 보입니다.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java (1)
39-47: LGTM! 저장 메서드 추가가 적절합니다.단일 및 배치 저장 메서드가 JPA 저장소로 올바르게 위임됩니다. Product 엔티티는 유즈케이스별로 다른 락 전략을 사용하므로, 이 범용 저장 메서드는 락이 필요하지 않은 시나리오에 적합합니다.
Based on learnings, Product 엔티티는 좋아요 기능에는 비관적 락을, 재고 차감에는 낙관적 락을 사용합니다.
apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.java (1)
18-31: LGTM! 필드명 변경으로 의미가 더 명확해졌습니다.
price를pricePerItem으로 변경하여 OrderItem 컨텍스트에서 "개당 가격"이라는 의미가 더 명확해졌습니다. 팩토리 메서드는 범용 파라미터명price를 유지하는 것이 적절합니다.apps/commerce-api/src/main/java/com/loopers/application/order/OrderRequest.java (1)
10-40: LGTM! 쿠폰 지원 및 검증 로직이 잘 구현되었습니다.추가된 기능들이 적절합니다:
couponId필드로 쿠폰 적용 지원validate()메서드의 포괄적인 검증 (빈 항목, 수량, 중복 상품ID, 쿠폰ID)toItemQuantityMap()메서드로 편리한 수량 맵 생성
toItemQuantityMap()에서 중복 키로 인한IllegalStateException은validate()의 중복 검사(line 34-36)로 방지됩니다.apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java (2)
31-34: LGTM! 트랜잭션 처리가 적절합니다.단일 Brand 저장 시
@Transactional어노테이션이 올바르게 적용되었습니다.
36-38: LGTM! 읽기 전용 메서드입니다.페이지네이션 조회 메서드는 읽기 전용이므로
@Transactional없이 구현하는 것이 적절합니다.apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (2)
25-26: 동시성 안전을 위한 row-locking 패턴 적용 - LGTM
findByUserIdForUpdate를 사용하여 포인트 조회 시 행 잠금을 적용한 것은 동시성 환경에서 race condition을 방지하는 좋은 개선입니다.Also applies to: 39-40
37-43: I'll verify the review comment by examining the actual implementation of the Point entity and PointService to check if the balance validation is properly performed.
<function_calls>
#!/bin/bashFind the PointService file
fd -t f "PointService.java"
Also find Point.java
fd -t f "Point.java" | grep -E "(domain|point)"
</function_calls>Now let me examine the PointService and Point implementations:
<function_calls>
#!/bin/bashRead PointService.java
echo "=== PointService.java ==="
cat -n "apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java"
</function_calls><function_calls>
#!/bin/bashRead Point.java
echo "=== Point.java ==="
cat -n "apps/commerce-api/src/main/java/com/loopers/domain/point/Point.java"
</function_calls><function_calls>
#!/bin/bashSearch for hasSufficientAmount usage
echo "=== hasSufficientAmount usage ==="
rg -n "hasSufficientAmount" --type java -C 3
</function_calls><function_calls>
#!/bin/bashSearch for deduct method calls to understand calling pattern
echo "=== deduct method calls ==="
rg -n ".deduct(" --type java -C 3
</function_calls>apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java (1)
79-84: LGTM - 할인 결과 추출 헬퍼 메서드쿠폰 적용 로직을 별도 메서드로 분리하여 가독성이 향상되었습니다.
couponId가 null인 경우 기본DiscountResult를 반환하는 처리가 적절합니다.apps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponTest.java (1)
16-332: 포괄적인 쿠폰 테스트 커버리지 - LGTM
Coupon엔티티에 대한 테스트가 체계적으로 구성되어 있습니다:
- Happy path, 예외 케이스, 엣지 케이스가 모두 포함됨
@Nested클래스로 논리적 그룹화@ParameterizedTest를 활용한 경계값 테스트- AAA(Arrange-Act-Assert) 패턴 준수
apps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.java (1)
13-39: 전반적인 구조 LGTM
- 트랜잭션 경계가 적절히 설정됨 (
@Transactional,@Transactional(readOnly = true))- Lombok
@RequiredArgsConstructor를 통한 생성자 주입 사용- 파사드 패턴으로 도메인 서비스와 응용 계층을 적절히 분리
apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java (1)
27-35: I encountered a repository clone failure when attempting to verify the concerns in the review comment. This prevents me from examining theProduct.javafile to check itshashCodeandequalsimplementation, as well as verifying the actual usage pattern in the codebase.To proceed, I would need:
- Direct file content access: The
Product.javafile from the repository to inspect its entity structure and whether it implementshashCode()andequals()methods- Context verification: Whether the
Map<Product, Integer>pattern appears elsewhere in the codebase or ifMap<Long, Integer>(productId-based) is the preferred approachCould you provide either:
- The content of
apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java, or- Confirmation that the repository can be accessed for verification?
Alternatively, if you have insights into whether the
Productentity implements properhashCodeandequalsmethods, that would help determine if the original review concern is valid.apps/commerce-api/src/main/java/com/loopers/domain/common/vo/DiscountResult.java (1)
7-27: LGTM! 할인 결과를 캡슐화하는 잘 설계된 value object입니다.레코드의 불변성과 compact constructor를 활용한 유효성 검증이 적절합니다. 특히 쿠폰 ID 없이 할인 금액이 존재할 수 없다는 비즈니스 규칙과 가격 일관성 검증이 도메인 무결성을 잘 보장합니다.
apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java (1)
30-64: LGTM! 새 메서드들이 기존 패턴을 잘 따르고 있습니다.
getProductsByIds는 기존getProductMapByIds와 일관성 있는 API를 제공하고,save/saveAll메서드는 쓰기 작업에 적절한 트랜잭션 경계를 설정했습니다.apps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.java (2)
27-58: LGTM! 팩토리 메서드와 유효성 검증이 잘 구현되었습니다.정률 할인의 100% 상한선 검증, userId 유효성 검증 등 도메인 규칙이 명확하게 적용되어 있습니다. 엔티티 생성을 팩토리 메서드로 캡슐화한 것도 좋은 설계입니다.
60-80: LGTM! 할인 적용 로직이 잘 구현되었습니다.단일 사용 보장(
used플래그), FIXED 할인 시 원가 상한선 적용(Math.min), PERCENTAGE 계산 시 오버플로우 방지(long캐스팅) 등 엣지 케이스가 적절히 처리되었습니다.apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponService.java (1)
28-33: LGTM! 비관적 락을 사용한 쿠폰 적용 로직이 적절합니다.
findByIdAndUserIdForUpdate를 통한 비관적 락으로 동시성 문제(쿠폰 이중 사용)를 방지하고 있습니다. 학습된 내용에 따르면 이 코드베이스는 유즈케이스별로 다른 락 전략을 사용하는데, 쿠폰 적용처럼 단일 사용이 중요한 경우 비관적 락이 적합합니다.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (1)
101-110: LGTM! ProductSearchRequest로의 리팩토링이 적절합니다.
Pageable에서ProductSearchRequest로 변경하여 검색 조건을 더 명확하게 캡슐화했습니다. AI 요약에 따르면 이 요청 객체는 허용된 정렬 조건에 대한 유효성 검증도 포함하고 있어 API 계층의 안정성이 향상됩니다.apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceIntegrationTest.java (1)
42-63: 테스트 로직이 올바르게 구현되었습니다.
DiscountResult를 사용한 주문 생성 테스트가 새로운 API 시그니처에 맞게 잘 업데이트되었습니다.apps/commerce-api/src/main/java/com/loopers/domain/order/Order.java (2)
53-65: DiscountResult를 사용한 Order 생성 로직이 잘 구현되었습니다.
DiscountResult에서 가격 정보를 추출하여 Order를 생성하는 패턴이 명확합니다.
72-72: I encountered a repository access failure that prevents me from directly verifying the code. The cloning operation failed, which means I cannot inspect:
- The
Priceclass definition and its type signature- The
OrderItem.getTotalPrice()method return type- The context of line 72 in
Order.java- Actual usage patterns and typical data ranges
Integer overflow concern cannot be verified due to repository access failure.
The review comment raises a theoretically valid concern—
IntStream.sum()accumulates asintand can overflow with large values. However, without accessing the codebase, I cannot confirm:
- Whether
Priceconstructor acceptsintorlong- The typical magnitude of order item prices
- Whether this is a practical risk in the business context
apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java (3)
381-426: 쿠폰 적용 테스트가 잘 구현되었습니다.정액 쿠폰 할인 시나리오를 테스트하며, 주문 생성, 재고 차감, 포인트 차감, 쿠폰 사용 상태를 모두 검증합니다. 테스트 주석도 명확하게 작성되어 있습니다.
513-548: 트랜잭션 롤백 테스트가 잘 구현되었습니다.존재하지 않는 쿠폰 사용 시 롤백 동작을 검증하며, 초기 상태를 캡처하여 롤백 후 상태와 비교하는 패턴이 적절합니다.
910-963: 쿠폰 동시성 테스트가 잘 설계되었습니다.Pessimistic Lock을 통해 동시에 같은 쿠폰을 사용하려 할 때 하나만 성공하는 것을 검증합니다.
successCount와failureCount를 사용한 검증 방식이 명확합니다.apps/commerce-api/src/test/java/com/loopers/performance/fixture/PerformanceTestFixture.java (1)
27-32: 성능 테스트용 Fixture 흐름이 명확하고 재사용성 좋습니다
prepareTestData에서 테이블 truncate → 대량 데이터 생성 순서가 명확하고, (productCount, brandCount)를 파라미터로 받아 테스트별로 다른 스케일을 쉽게 설정할 수 있어서 성능 테스트 베이스 클래스로 적절해 보입니다. 이 패턴을 상속 테스트들에서 공통으로 쓰기에도 좋아 보입니다.
| Map<Long, Integer> productIdQuantityMap = request.toItemQuantityMap(); | ||
|
|
||
| Map<Long, Integer> productIdQuantityMap = request.items().stream() | ||
| .collect(Collectors.toMap(OrderItemRequest::productId, OrderItemRequest::quantity)); | ||
| productIdQuantityMap.forEach(supplyService::checkAndDecreaseStock); | ||
|
|
||
| Map<Long, Product> productMap = productService.getProductMapByIds(productIdQuantityMap.keySet()); | ||
| List<Product> products = productService.getProductsByIds(productIdQuantityMap.keySet()); | ||
| Map<Product, Integer> productQuantityMap = products.stream().collect(Collectors.toMap( | ||
| Function.identity(), | ||
| product -> productIdQuantityMap.get(product.getId()) | ||
| )); |
There was a problem hiding this comment.
재고 차감 전 상품 존재 여부 검증 필요
현재 흐름에서 supplyService::checkAndDecreaseStock이 productService.getProductsByIds 보다 먼저 호출됩니다. 만약 요청된 상품 중 일부가 존재하지 않는다면, 재고는 이미 차감되었지만 이후 로직에서 문제가 발생할 수 있습니다.
또한, getProductsByIds가 요청한 모든 상품을 반환하는지 검증하지 않고 있어, 존재하지 않는 상품 ID가 포함된 경우 누락된 채로 주문이 생성될 수 있습니다.
request.validate();
Map<Long, Integer> productIdQuantityMap = request.toItemQuantityMap();
+List<Product> products = productService.getProductsByIds(productIdQuantityMap.keySet());
+if (products.size() != productIdQuantityMap.size()) {
+ throw new CoreException(ErrorType.NOT_FOUND, "일부 상품을 찾을 수 없습니다.");
+}
+
productIdQuantityMap.forEach(supplyService::checkAndDecreaseStock);
-List<Product> products = productService.getProductsByIds(productIdQuantityMap.keySet());
Map<Product, Integer> productQuantityMap = products.stream().collect(Collectors.toMap(
Function.identity(),
product -> productIdQuantityMap.get(product.getId())
));📝 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.
| Map<Long, Integer> productIdQuantityMap = request.toItemQuantityMap(); | |
| Map<Long, Integer> productIdQuantityMap = request.items().stream() | |
| .collect(Collectors.toMap(OrderItemRequest::productId, OrderItemRequest::quantity)); | |
| productIdQuantityMap.forEach(supplyService::checkAndDecreaseStock); | |
| Map<Long, Product> productMap = productService.getProductMapByIds(productIdQuantityMap.keySet()); | |
| List<Product> products = productService.getProductsByIds(productIdQuantityMap.keySet()); | |
| Map<Product, Integer> productQuantityMap = products.stream().collect(Collectors.toMap( | |
| Function.identity(), | |
| product -> productIdQuantityMap.get(product.getId()) | |
| )); | |
| Map<Long, Integer> productIdQuantityMap = request.toItemQuantityMap(); | |
| List<Product> products = productService.getProductsByIds(productIdQuantityMap.keySet()); | |
| if (products.size() != productIdQuantityMap.size()) { | |
| throw new CoreException(ErrorType.NOT_FOUND, "일부 상품을 찾을 수 없습니다."); | |
| } | |
| productIdQuantityMap.forEach(supplyService::checkAndDecreaseStock); | |
| Map<Product, Integer> productQuantityMap = products.stream().collect(Collectors.toMap( | |
| Function.identity(), | |
| product -> productIdQuantityMap.get(product.getId()) | |
| )); |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java
around lines 57 to 65, the code decreases stock before verifying that all
requested product IDs actually exist and that getProductsByIds returned every
requested product; change the flow to first load products by IDs, verify the
returned collection size (or returned IDs) matches the requested
productIdQuantityMap keys and throw a clear exception if any IDs are missing,
then construct the product-to-quantity map from the verified products and only
after that call supplyService.checkAndDecreaseStock for each product (or perform
the stock decrement inside the same transaction after verification) so stock is
never decreased for non-existent products and orders cannot be created with
missing products.
| public record ProductCreateRequest( | ||
| String name, | ||
| Long brandId, | ||
| Price price, | ||
| Stock stock, | ||
| Integer likeCount | ||
| ) { |
There was a problem hiding this comment.
likeCount를 생성 요청에 포함하는 것이 적절한지 검토가 필요합니다.
좋아요 수(likeCount)는 일반적으로 사용자 행동에 의해 증가하는 메트릭이며, 상품 생성 시점에 임의로 설정할 수 있는 값이 아닙니다. 이는 좋아요 수 조작 가능성을 열어둘 수 있습니다.
생성 시점에 likeCount를 0으로 초기화하고, 이 필드를 요청에서 제거하는 것을 고려해보세요.
</review_comment_end>
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/product/ProductCreateRequest.java
around lines 6 to 12, the request currently accepts likeCount which should not
be client-provided; remove the likeCount field from the ProductCreateRequest
record and update any callers/constructors to stop passing it; ensure product
creation logic (service/repository layer) initializes likeCount to 0 (or
defaults it in the Product entity) and update API docs/tests/contracts
accordingly so likeCount is not accepted from clients at creation time.
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
Show resolved
Hide resolved
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
Show resolved
Hide resolved
| public List<Brand> saveAll(Collection<Brand> brands) { | ||
| return brandRepository.saveAll(brands); | ||
| } |
There was a problem hiding this comment.
배치 저장 메서드에 @transactional이 누락되었습니다.
saveAll() 메서드에 @Transactional 어노테이션이 없습니다. 반면 단일 저장 메서드인 save()(line 31)에는 @Transactional이 적용되어 있습니다.
배치 작업은 원자성(atomicity)을 보장하기 위해 트랜잭션으로 묶여야 합니다. 하나의 Brand 저장이 실패할 경우 전체 배치가 롤백되어야 데이터 일관성이 유지됩니다.
다음 diff를 적용하여 수정하세요:
+ @Transactional
public List<Brand> saveAll(Collection<Brand> brands) {
return brandRepository.saveAll(brands);
}📝 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.
| public List<Brand> saveAll(Collection<Brand> brands) { | |
| return brandRepository.saveAll(brands); | |
| } | |
| @Transactional | |
| public List<Brand> saveAll(Collection<Brand> brands) { | |
| return brandRepository.saveAll(brands); | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/brand/BrandService.java
around lines 40-42, the batch saveAll(Collection<Brand>) method is missing a
@Transactional annotation so failures during batch persist won't roll back the
entire operation; add the @Transactional annotation to the saveAll method
(importing org.springframework.transaction.annotation.Transactional if not
already present) so the batch executes within a single transaction and will
fully roll back on exceptions.
| page.getTotalElements(), | ||
| page.getNumber(), | ||
| page.getSize() | ||
| page.getNumberOfElements() |
There was a problem hiding this comment.
필드명과 값의 의미가 불일치합니다.
ProductV1Dto와 동일한 문제입니다. size 필드에 getNumberOfElements()를 할당하여 의미가 불일치합니다.
원래대로 되돌리려면:
- page.getNumberOfElements()
+ page.getSize()또는 필드명을 변경하려면:
public record ProductsResponse(
List<LikeProductV1Dto.ProductResponse> content,
int totalPages,
long totalElements,
int number,
- int size
+ int numberOfElements
) {📝 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.
| page.getNumberOfElements() | |
| page.getSize() |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/like/product/LikeProductV1Dto.java
around line 44, the `size` field is being assigned `page.getNumberOfElements()`
which mismatches the field name; either assign the correct page size (use
`page.getSize()` or the appropriate paging API method) to `size`, or rename the
field to reflect number of elements (e.g., `numberOfElements`) and keep
`page.getNumberOfElements()` — update all usages accordingly.
| page.getTotalElements(), | ||
| page.getNumber(), | ||
| page.getSize() | ||
| page.getNumberOfElements() |
There was a problem hiding this comment.
필드명과 값의 의미가 불일치합니다.
size 필드에 getNumberOfElements()를 할당하고 있습니다:
getSize(): 요청된 페이지 크기 (예: 10, 고정값)getNumberOfElements(): 현재 페이지의 실제 요소 수 (마지막 페이지에서 더 적을 수 있음)
이는 API 소비자에게 혼란을 줄 수 있습니다. 다음 중 하나를 선택해주세요:
getSize()로 되돌려 일관된 페이지 크기를 반환- 필드명을
numberOfElements로 변경하여 의미를 명확히
원래대로 되돌리려면:
- page.getNumberOfElements()
+ page.getSize()또는 필드명을 변경하려면:
public record ProductsPageResponse(
List<ProductResponse> content,
int totalPages,
long totalElements,
int number,
- int size
+ int numberOfElements
) {📝 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.
| page.getNumberOfElements() | |
| page.getSize() |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.java
around line 42, the DTO assigns page.getNumberOfElements() to the size field
which misrepresents the requested page size; either revert to page.getSize() so
size consistently reflects the requested page size, or rename the field to
numberOfElements (and update its getter/setter/JSON name) so it accurately
represents the actual number of elements on the current page—pick one option and
implement the corresponding change across the DTO and any serialization/consumer
code.
...api/src/test/java/com/loopers/application/like/product/LikeProductFacadeIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
🔖 Index @coderabbitai 위 체크리스트가 맞는지 확인해줘. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 727 🏁 Script executed: Length of output: 7537 @LenKIM, 체크리스트를 확인한 결과를 알려드립니다: 🔖 Index
❤️ Structure
⚡ Cache
요약: Index와 Structure는 대부분 완료되었으나, 성능 비교 결과가 누락되었고, Cache는 전혀 구현되지 않았습니다. 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 21220 🏁 Script executed: Length of output: 9671 @looper88888, 체크리스트를 확인한 결과를 알려드립니다: 🔖 Index
❤️ Structure
⚡ Cache
요약: Structure 섹션은 완료되었으나, Index 섹션의 성능 비교와 Cache 섹션 전체가 미완료 상태입니다. 🧠 Learnings used |
|
@coderabbitai 다시 리뷰해줘 |
|
@JVHE, 네, PR 전체를 다시 리뷰하겠습니다. 변경사항을 종합적으로 검토하여 피드백을 드리겠습니다. 🧠 Learnings used✅ Actions performedReview triggered.
|
2 participants
📌 Summary
현재 작업 진행중으로, 캐시 작업과 인덱스 작업 전후 결과 비교가 남았습니다.
💬 Review Points
✅ Checklist
📎 References
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.