[Volume 5] 인덱스, 캐시 기반 조회 성능 최적화#224
Open
Praesentia-YKM wants to merge 5 commits intoLoopers-dev-lab:Praesentia-YKMfrom
Open
[Volume 5] 인덱스, 캐시 기반 조회 성능 최적화#224Praesentia-YKM wants to merge 5 commits intoLoopers-dev-lab:Praesentia-YKMfrom
Praesentia-YKM wants to merge 5 commits intoLoopers-dev-lab:Praesentia-YKMfrom
Conversation
- ProductService: getById() → getProduct() 리네임, getProductForAdmin() 추가 - ProductSortType: LATEST 값 추가, ProductRepositoryImpl에 LATEST 케이스 처리 - BrandFacade: deleteAllByBrandId() → softDeleteByBrandId() 호출 수정 - ProductRepositoryImpl: 중복 save() 메서드 제거 - 테스트 5개 파일: 메서드명 불일치 일괄 수정 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ProductFacade에 @Cacheable/@CacheEvict 적용 (상품 상세 캐시) - LikeTransactionService에 좋아요 변경 시 캐시 무효화 추가 - RedisCacheConfig, CustomCacheErrorHandler 추가 - 고아 클래스 ProductListCacheService 삭제 - 중복 admin/ 패키지 및 interfaces/auth/ 패키지 정리 - import 경로 수정 및 getter 이름 불일치 해결 - 캐시 통합 테스트 추가 (ProductCacheIntegrationTest, LikeTransactionCacheTest) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- QueryDSL BooleanBuilder로 brandId 동적 필터 + 정렬(좋아요순 등) 구현 - 복합 인덱스 4개 추가 (brand+deleted+like, deleted+like, deleted+created, deleted+price) - N+1 문제 해결: 목록 조회 시 배치 쿼리 패턴(Map<Long, Model>) 적용 - likes 테이블 유니크 제약조건 추가 (uk_likes_user_product) - StockService 메서드 참조 오류 수정 (productId → getProductId) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # apps/commerce-api/src/main/java/com/loopers/application/brand/BrandService.java # apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java # apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java # apps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.java # apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java
📝 WalkthroughWalkthroughRedis 캐싱을 도입하여 제품 상세 정보를 캐싱하고, 데이터베이스에 다중 인덱스를 추가하며, 관리자 API 엔드포인트를 제거하고 클라이언트 API로 통합한다. 좋아요 기능에 unique 제약 조건을 추가하고, API 계층에서 DTO 타입과 위치를 리팩터링한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProductV1Controller
participant ProductFacade
participant CacheManager
participant ProductService
participant BrandService
participant StockService
participant Database as DB
Client->>ProductV1Controller: GET /api/v1/products/{id}
ProductV1Controller->>ProductFacade: getProduct(productId)
ProductFacade->>CacheManager: 캐시 조회 (productDetail)
alt 캐시 히트
CacheManager-->>ProductFacade: ProductDetail 반환
else 캐시 미스
ProductFacade->>ProductService: getById(productId)
ProductService->>Database: SELECT * FROM product WHERE id=? AND deleted_at IS NULL
Database-->>ProductService: ProductModel
ProductFacade->>BrandService: getBrand(brandId)
BrandService->>Database: SELECT * FROM brand WHERE id=? AND deleted_at IS NULL
Database-->>BrandService: BrandModel
ProductFacade->>StockService: getByProductId(productId)
StockService->>Database: SELECT * FROM stock WHERE product_id=?
Database-->>StockService: StockModel
ProductFacade->>CacheManager: 캐시 저장 (productDetail, 10min TTL)
CacheManager-->>ProductFacade: 저장 완료
end
ProductFacade-->>ProductV1Controller: ProductDetail
ProductV1Controller-->>Client: ApiResponse<ProductDetailResponse>
sequenceDiagram
participant Client
participant LikeV1Controller
participant LikeTransactionService
participant ProductService
participant ProductRepository
participant CacheManager
participant Database as DB
Client->>LikeV1Controller: POST /api/v1/likes/{productId}
LikeV1Controller->>LikeTransactionService: doLike(userId, productId)
LikeTransactionService->>ProductRepository: incrementLikeCount(productId)
ProductRepository->>Database: UPDATE product SET like_count = like_count + 1 WHERE id=?
Database-->>ProductRepository: 1 (affected rows)
LikeTransactionService->>CacheManager: evictProductDetailCache(productId)
CacheManager->>CacheManager: productDetail 캐시 항목 제거
CacheManager-->>LikeTransactionService: 제거 완료
LikeTransactionService-->>LikeV1Controller: 완료
LikeV1Controller-->>Client: 좋아요 등록 완료
Note over LikeTransactionService,CacheManager: 다음 조회 시 캐시 미스 → DB 재조회
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 검토 핵심 영역:
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java (1)
54-86:⚠️ Potential issue | 🟡 Minor소유자 불일치 시 예외 발생 테스트가 누락되었다.
통합 테스트(
OrderFacadeIntegrationTest)에서는 다른 사용자의 주문 조회 시BAD_REQUEST예외가 발생함을 검증하지만, 단위 테스트에서는 해당 케이스가 없다.getOrder(id, userId)시그니처 변경의 핵심 목적인 소유자 검증 로직이 단위 테스트에서 검증되지 않으면, 해당 로직 변경 시 조기 탐지가 어렵다.추가 테스트 케이스 제안
`@DisplayName`("본인 주문이 아니면 BAD_REQUEST 예외가 발생한다") `@Test` void throwsOnUserIdMismatch() { // arrange Long id = 1L; Long ownerId = 1L; Long requesterId = 999L; OrderModel order = new OrderModel(ownerId, new Money(10000), Money.ZERO, null); ReflectionTestUtils.setField(order, "id", id); given(orderRepository.findById(id)).willReturn(Optional.of(order)); // act CoreException exception = assertThrows(CoreException.class, () -> { orderService.getOrder(id, requesterId); }); // assert assertThat(exception.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java` around lines 54 - 86, Add a unit test to OrderServiceTest that verifies owner mismatch throws BAD_REQUEST: in the GetOrder nested class create a test (e.g., throwsOnUserIdMismatch) that sets up an OrderModel with ownerId (use ReflectionTestUtils to set id), stubs orderRepository.findById(id) to return that order, then call orderService.getOrder(id, requesterId) where requesterId != ownerId and assert the thrown CoreException has ErrorType.BAD_REQUEST; reference OrderModel, orderRepository.findById, and orderService.getOrder to locate where to add the test.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (1)
119-126:⚠️ Potential issue | 🟡 Minor
getBrandName()에서 모든 예외를 로깅 없이 삼키면 운영 중 장애 원인 파악이 어렵다.브랜드 조회 실패 시
null을 반환하는 것은 의도된 폴백이지만, 예외를 로깅하지 않으면 왜brandName이null인지 파악할 수 없다. 코딩 가이드라인에 따라 예외 발생 시 로깅을 추가해야 한다.🛠️ 수정 제안
+import lombok.extern.slf4j.Slf4j; +@Slf4j `@RequiredArgsConstructor` `@Component` public class ProductFacade { // ... private String getBrandName(Long brandId) { try { BrandModel brand = brandService.getBrandForAdmin(brandId); return brand.getName(); } catch (Exception e) { + log.warn("브랜드 조회 실패 brandId={}: {}", brandId, e.getMessage()); return null; } } }🤖 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/ProductFacade.java` around lines 119 - 126, The getBrandName(Long brandId) method currently swallows all exceptions silently; modify its catch block to log the failure including the brandId and exception stacktrace before returning null (use the class logger, e.g., the existing or add a private static final Logger for ProductFacade). Specifically, keep the intent of returning null on failure but call logger.warn or logger.error with a clear message like "Failed to fetch brand for id={}" and include the caught Exception (from the catch(Exception e)) while referencing BrandModel and brandService.getBrandForAdmin to locate the try/catch to update.apps/commerce-api/src/main/java/com/loopers/application/like/LikeTransactionService.java (1)
30-49:⚠️ Potential issue | 🟠 Major직접 캐시 무효화 경로가 오류 처리를 우회하여 Redis 장애 시 트랜잭션 실패를 야기한다.
cache.evict(productId)직접 호출은CustomCacheErrorHandler를 거치지 않는다. 어노테이션 기반@CacheEvict는handleCacheEvictError()로 로깅 후 계속 진행하지만, 직접 호출은 Redis 예외가 그대로doLike/doUnlike메서드로 전파되어 트랜잭션 실패로 이어진다. 또한 Redis I/O가 DB 트랜잭션 락 구간에 포함되므로 응답 시간이 증가한다.무효화를 Spring 캐시 에러 처리 경로로 통합하려면
@CacheEvict(cacheNames = "productDetail", key = "#productId")어노테이션을 메서드에 적용하거나, 또는@TransactionalEventListener(phase = AFTER_COMMIT)으로 이벤트 기반 무효화로 옮기고 별도 메서드에서 어노테이션을 사용해야 한다. 추가로 (1) 롤백 시 무효화가 발생하지 않는지, (2) Redis 장애 상태에서도 like/unlike가 성공하는지를 검증하는 테스트를 추가해야 한다.🤖 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/like/LikeTransactionService.java` around lines 30 - 49, The direct cache.evict(productId) call inside evictProductDetailCache bypasses the CustomCacheErrorHandler and can propagate Redis exceptions into the doLike/doUnlike transactions; replace this direct eviction with a Spring-managed eviction path by moving cache eviction out of the DB transaction and using a method annotated with `@CacheEvict`(cacheNames = "productDetail", key = "#productId") or publish an event and handle eviction in a separate listener annotated with `@TransactionalEventListener`(phase = AFTER_COMMIT) so that evictions run after commit and go through the cache error handler (update doLike/doUnlike to publish the event or call the `@CacheEvict` method instead of calling evictProductDetailCache, and remove/direct-evict usage of cache.evict in evictProductDetailCache).
🧹 Nitpick comments (16)
docs/mentoring (3)
187-196: Write-Through/Write-Around 정의가 표준 용어와 상이함문서의 Write-Around 정의("DB 먼저 쓰고 캐시 갱신")는 일반적인 캐싱 문헌의 정의와 다르다. 표준 정의에서:
- Write-Through: 캐시와 DB에 동기적으로 동시 쓰기
- Write-Around: DB에만 쓰기, 캐시는 TTL 만료 후 읽기 시 채움
멘토의 설명은 실무적으로 유효하나, 면접이나 외부 문서 참조 시 혼란을 방지하기 위해 표준 정의와의 차이점을 명시하는 것이 좋다.
**Write-Through vs Write-Around** +> **참고**: 아래 정의는 멘토의 실무 관점 설명이며, 일반적인 캐싱 문헌의 표준 정의와 다소 차이가 있을 수 있음. + | 패턴 | 흐름 | 위험성 |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mentoring` around lines 187 - 196, Update the "Write-Through vs Write-Around" section to use standard caching definitions: state that Write-Through writes synchronously to both cache and DB, and Write-Around writes only to the DB (cache is populated on subsequent reads or TTL expiry); then keep the pragmatic recommendation (prefer Write-Around for DB as source-of-truth) but add an explicit note clarifying that this is a practical deviation from the textbook definitions and explain when to prefer Write-Through for strict consistency.
788-798: 캐시 체크리스트에 장애 폴백 테스트 항목 추가 권장PR에서
CustomCacheErrorHandler를 통해 Redis 장애 시 DB 폴백을 구현했다. 운영 환경에서 이 폴백 경로가 정상 동작하는지 검증하는 것이 중요하다. 체크리스트에 해당 항목을 추가하면 완전성이 높아진다.- [ ] 에러 케이스 처리 (잘못된 데이터 캐싱 금지) +- [ ] 장애 폴백 테스트 (Redis 다운 시 DB 조회 정상 동작 확인) - [ ] 모니터링 설정 (Hit Rate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mentoring` around lines 788 - 798, Add a checklist item to validate the Redis → DB fallback path by explicitly testing the CustomCacheErrorHandler's behavior under Redis failures: specify tests to simulate Redis errors, verify that requests fall back to DB and return correct data, assert no bad data is cached, and include monitoring/alert validation for fallback occurrences so the checklist covers operational validation of the DB fallback path.
170-185: Evict 전략을 "하수"로 분류한 부분에 대한 맥락 보완 필요PR 구현에서는
@CacheEvict를 사용한 Evict 전략을 채택했으나, 문서에서는 이를 "하수"로 분류하고 있다. 실무에서 Evict 전략이 적절한 케이스가 존재한다:
- 캐시 데이터 구조가 복잡하여 부분 갱신이 어려운 경우
- 쓰기 빈도가 낮고 읽기 빈도가 높은 경우
- 캐시 미스 비용이 수용 가능한 수준인 경우
현재 PR의
ProductDetail캐싱은 여러 서비스 호출을 조합한 복합 DTO이므로, Evict 후 재조회가 더 안전한 선택일 수 있다. 문서에 이러한 trade-off 맥락을 추가하면 학습자의 이해도가 높아진다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mentoring` around lines 170 - 185, The document currently labels the Evict strategy as "하수" without acknowledging valid use-cases; update the mentoring docs to add a concise trade-off paragraph explaining when Evict is appropriate (e.g., complex cache structures where partial updates are hard, low write/high read workloads, and acceptable cache-miss cost) and reference the PR's concrete case (the ProductDetail composite DTO and the use of `@CacheEvict`) to justify why Evict can be safer there; keep the text short, neutral, and add one or two bullet-style examples clarifying when to prefer Put vs Evict.apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java (1)
33-35:OrderItemRepositorymock이 사용되지 않는다.
@Mock OrderItemRepository orderItemRepository가 선언되었지만 어떤 테스트에서도 사용되지 않는다. 불필요한 mock은 테스트 의존성을 혼란스럽게 하고, InjectMocks 주입 시 예상치 못한 부작용을 유발할 수 있다.실제로 필요한 테스트가 있다면 해당 테스트를 추가하고, 그렇지 않다면 제거하라.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java` around lines 33 - 35, The OrderItemRepository mock declared in OrderServiceTest is unused; either remove the field or add tests that exercise code paths relying on it (e.g., when OrderService interacts with order items). If you choose removal, delete the `@Mock` private OrderItemRepository orderItemRepository declaration from OrderServiceTest; if you choose to keep it, add one or more tests that call the OrderService method(s) that use OrderItemRepository and verify interactions (use when(...) to stub repository responses and verify(orderItemRepository).save(...)/findByOrderId(...) as appropriate) so the mock is actually used during InjectMocks injection.apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java (1)
47-52: 쿠폰 서비스 mock이 선언되었으나 테스트에서 사용되지 않는다.
CouponIssueService와CouponServicemock이 추가되었지만, 모든 테스트에서couponIssueId로null만 전달한다. 쿠폰 적용 경로가 단위 테스트에서 검증되지 않으면 다음 장애 시나리오를 사전에 탐지할 수 없다:
- 쿠폰 할인 계산 오류
couponIssue.validateOwner()실패 시 예외 처리coupon.validateUsable()실패 시 예외 처리- 쿠폰 사용 처리(
couponIssue.use()) 호출 검증mock을 선언했다면 해당 경로를 테스트하거나, 현재 범위 외라면 mock 선언을 제거하고 TODO 주석으로 추후 추가 계획을 명시하라.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java` around lines 47 - 52, The tests declare mocks for CouponIssueService and CouponService but never exercise coupon logic (couponIssueId passed as null); either add unit tests that exercise the coupon path or remove the unused mocks and add a TODO. To fix: for positive and negative coupon scenarios, stub CouponIssueService.lookup(couponIssueId) and CouponService.lookup(...) (or methods used in OrderFacade) to return mocked CouponIssue and Coupon objects, verify calls to couponIssue.validateOwner(), coupon.validateUsable(), and couponIssue.use(), and assert discount calculation; otherwise delete the CouponIssueService and CouponService `@Mock` declarations from OrderFacadeTest and add a TODO comment indicating coupon behavior tests will be added later. Reference the CouponIssueService, CouponService, couponIssueId, couponIssue.validateOwner(), coupon.validateUsable(), and couponIssue.use() to locate relevant code.apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java (1)
46-134: 쿠폰 적용 통합 테스트가 누락되었다.
placeOrder메서드는couponIssueId파라미터를 받지만, 모든 테스트에서null만 전달하고 있다. 운영 환경에서 쿠폰 할인 계산 오류, 쿠폰 중복 사용, 쿠폰 소유자 검증 실패 등의 장애가 발생할 경우 사전 탐지가 어렵다.다음 테스트 케이스 추가를 권장한다:
- 쿠폰 적용 시 할인 금액이 정확히 계산되는지
- 타인의 쿠폰 사용 시 예외 발생
- 이미 사용된 쿠폰 재사용 시 예외 발생
- 쿠폰 사용 후 주문 실패 시 쿠폰 상태 롤백 여부
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java` around lines 46 - 134, Add integration tests to the PlaceOrder nested class that exercise coupon handling via orderFacade.placeOrder(couponIssueId): create helper(s) to issue a coupon to a user (e.g., createCouponIssue(userId, couponDef)) and add tests that (1) apply a valid coupon and assert totalAmount() reflects the discount, (2) attempt to use another user's coupon and assert a CoreException with ErrorType.BAD_REQUEST/NOT_FOUND as appropriate, (3) attempt to reuse an already-consumed coupon and assert a CoreException, and (4) simulate an order failure after coupon consumption and assert the couponIssue status is rolled back to unused; reference placeOrder, OrderResult, OrderItemCommand, and any coupon service methods (e.g., couponIssue creation/claim/check) to locate where to wire the tests.apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.java (1)
22-31:adminLdap파라미터가 사용되지 않는다.
@AdminUser String adminLdap파라미터를 캡처하지만 Facade 호출 시 전달하지 않는다. 운영 관점에서 관리자 조회 이력 감사(audit log), 접근 권한 세분화 등을 위해 해당 정보가 필요할 수 있다.의도적인 설계라면 주석으로 명시하거나, 향후 감사 로그 구현 시 활용할 수 있도록 Facade 시그니처에 포함하는 것을 고려하라.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.java` around lines 22 - 31, 메서드 getAll에서 캡처한 `@AdminUser` String adminLdap가 사용되지 않으므로 감사/권한 목적으로 이 값을 전달하거나 의도를 명시해야 합니다; 수정안으로는 OrderAdminV1Controller# getAll 시 orderFacade.getAllForAdmin(pageable) 대신 orderFacade.getAllForAdmin(adminLdap, pageable)처럼 adminLdap를 Facade로 전달하도록 orderFacade의 시그니처(또는 오버로드)를 업데이트하거나, 정말로 필요 없다면 컨트롤러에서 해당 파라미터를 제거하고 불필요 파라미터임을 주석으로 남겨 의도를 분명히 하세요.apps/commerce-api/src/test/java/com/loopers/interfaces/api/order/OrderV1ApiE2ETest.java (1)
60-63: 응답 역참조 전에 상태/본문 단언을 추가하는 것이 안전하다Line 62처럼 즉시
getBody().data()를 역참조하면 실패 시 NPE로 종료되어 운영 관점의 장애 원인(HTTP 상태/응답 내용) 추적이 어려워진다. 상태코드와 본문 null 여부를 먼저 단언한 뒤id를 반환하도록 수정하는 편이 좋다.
추가 테스트로 관리자 상품 생성 실패(예: 잘못된 brandId) 시에도 테스트가 NPE가 아닌 명확한 단언 메시지로 실패하는지 검증해야 한다.수정 예시 diff
private Long createProduct(String name, int price, Long brandId, int stock) { var req = new ProductAdminV1Dto.CreateRequest(name, "설명", price, brandId, stock); - return testRestTemplate.exchange("/api-admin/v1/products", HttpMethod.POST, + ResponseEntity<ApiResponse<ProductAdminV1Dto.ProductAdminDetailResponse>> response = + testRestTemplate.exchange("/api-admin/v1/products", HttpMethod.POST, new HttpEntity<>(req, adminHeaders()), - new ParameterizedTypeReference<ApiResponse<ProductAdminV1Dto.ProductAdminDetailResponse>>() {}).getBody().data().id(); + new ParameterizedTypeReference<ApiResponse<ProductAdminV1Dto.ProductAdminDetailResponse>>() {}); + + assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().data()).isNotNull(); + return response.getBody().data().id(); }As per coding guidelines,
**/*Test*.java: 통합 테스트는 플래키 가능성과 테스트 신뢰성을 점검해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/order/OrderV1ApiE2ETest.java` around lines 60 - 63, The current call that directly dereferences getBody().data().id() (via testRestTemplate.exchange(...).getBody().data().id()) can NPE and hide HTTP status/body info; change the code in OrderV1ApiE2ETest to first assert the ResponseEntity status is OK and that getBody() and getBody().data() are non-null before extracting id, using the same testRestTemplate.exchange(...) response object and adminHeaders() helper, and include a separate test that posts an invalid brandId to the "/api-admin/v1/products" endpoint and asserts a clear error status and message (not NPE) to validate failure behavior.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java (1)
28-28: 무제한 List 조회는 브랜드별 대량 데이터에서 메모리·GC·긴 트랜잭션 위험을 만든다Line 28 메서드는 결과 건수 상한이 없어 운영 데이터가 커질수록 장애 가능성이 증가한다.
수정안은 기존 Line 30의 페이징 메서드만 사용하도록 무제한 메서드를 제거하거나, 대량 변경 목적이면 조회 대신 벌크 업데이트 쿼리로 전환하는 것이다.
추가 테스트는 대량 브랜드 데이터 시나리오에서 페이징 루프(또는 벌크 업데이트)로 처리되어 단건 트랜잭션 메모리 사용이 폭증하지 않는지 검증하는 것이다.As per coding guidelines
**/*Repository*.java: "대량 데이터에서의 병목을 점검한다."🤖 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/ProductJpaRepository.java` at line 28, The findAllByBrandIdAndDeletedAtIsNull(Long brandId) method in ProductJpaRepository allows unbounded list retrieval which risks OOM/GC/long transactions for large brands; remove this unpaged method (delete the signature) and ensure callers use the existing paged variant findAllByBrandIdAndDeletedAtIsNull(Long brandId, Pageable pageable) or replace callers that perform mass changes with a bulk update query (e.g., a `@Modifying` JPQL/SQL method) in ProductJpaRepository; add an integration test that simulates large-brand data processed via a paging loop or a bulk update to verify per-transaction memory stays bounded.apps/commerce-api/src/test/java/com/loopers/interfaces/api/like/LikeV1ApiE2ETest.java (1)
53-53: E2E 헬퍼에서 응답 본문 즉시 역참조는 실패 원인 가시성을 떨어뜨린다Line 53은 API 실패 시 NPE로 종료될 수 있어 실제 장애 원인(상태코드/에러바디) 추적이 어려워진다.
수정안은createProduct헬퍼에서 먼저 상태코드와 body/data null 여부를 명시적으로 검증한 뒤 id를 반환하는 것이다.
추가 테스트는 잘못된 관리자 헤더 또는 유효하지 않은 요청에서 헬퍼가 명확한 assertion 메시지로 실패하는지 확인하는 것이다.헬퍼 안정성 보강 예시
private Long createProduct(String name, int price, Long brandId) { var req = new ProductAdminV1Dto.CreateRequest(name, "설명", price, brandId, 10); - return testRestTemplate.exchange("/api-admin/v1/products", HttpMethod.POST, new HttpEntity<>(req, adminHeaders()), - new ParameterizedTypeReference<ApiResponse<ProductAdminV1Dto.ProductAdminDetailResponse>>() {}).getBody().data().id(); + var response = testRestTemplate.exchange( + "/api-admin/v1/products", + HttpMethod.POST, + new HttpEntity<>(req, adminHeaders()), + new ParameterizedTypeReference<ApiResponse<ProductAdminV1Dto.ProductAdminDetailResponse>>() {} + ); + assertThat(response.getStatusCode().is2xxSuccessful()).isTrue(); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().data()).isNotNull(); + return response.getBody().data().id(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/like/LikeV1ApiE2ETest.java` at line 53, The current E2E helper dereferences the response body immediately (see the ParameterizedTypeReference call that ends with .getBody().data().id()), which causes NPEs on API failures and hides real failure details; update the createProduct helper to first assert/validate the HTTP status code and that getBody() and data() are non-null, log or include the error/body when failing, then return the id only after those checks, and add tests that assert the helper fails with clear messages for invalid admin headers and for invalid request payloads so failures surface status and error body instead of NPEs.apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java (2)
115-133:updateStock()캐시 무효화 테스트가 누락되었다.
ProductFacade.updateStock()에@CacheEvict가 적용되어 있으나 이에 대한 테스트가 없다. 재고 변경 시 캐시가 올바르게 무효화되는지 검증하는 테스트를 추가해야 한다.🧪 추가 테스트 제안
`@Test` `@DisplayName`("재고 수정 시 상세 캐시가 삭제된다") void 재고_수정_시_상세_캐시가_삭제된다() { // given BrandModel brand = brandJpaRepository.save(new BrandModel("테스트브랜드", "설명")); ProductModel product = productJpaRepository.save( new ProductModel("테스트상품", "상품설명", new Money(10000), brand.getId()) ); stockJpaRepository.save(new StockModel(product.getId(), 100)); productFacade.getProduct(product.getId()); assertThat(cacheManager.getCache("productDetail").get(product.getId())).isNotNull(); // when productFacade.updateStock(product.getId(), 50); // then assertThat(cacheManager.getCache("productDetail").get(product.getId())).isNull(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java` around lines 115 - 133, The test suite is missing a test that verifies ProductFacade.updateStock() evicts the "productDetail" cache; add a new test method in ProductCacheIntegrationTest (e.g., 재고_수정_시_상세_캐시가_삭제된다) that mirrors the existing delete test: create and save BrandModel and ProductModel, seed StockModel, call productFacade.getProduct(product.getId()) and assert cacheManager.getCache("productDetail").get(product.getId()) isNotNull, then call productFacade.updateStock(product.getId(), <newQty>) and finally assert the same cache entry is null to confirm `@CacheEvict` on ProductFacade.updateStock() works.
32-33: Redis 컨테이너 이미지 버전을 고정해야 CI 안정성이 보장된다.
redis:latest태그는 빌드 시점에 따라 다른 버전이 사용될 수 있어 플래키 테스트의 원인이 된다. 특정 버전(예:redis:7.2)으로 고정하여 재현 가능한 테스트 환경을 유지해야 한다.♻️ 수정 제안
static final GenericContainer<?> redisContainer = - new GenericContainer<>(DockerImageName.parse("redis:latest")).withExposedPorts(6379); + new GenericContainer<>(DockerImageName.parse("redis:7.2-alpine")).withExposedPorts(6379);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java` around lines 32 - 33, The test uses an unstable image tag; update the redis container declaration (redisContainer / GenericContainer and DockerImageName.parse usage) to pin a specific Redis version (e.g., change "redis:latest" to "redis:7.2" or another CI-approved immutable tag) so CI tests are reproducible; locate the static final GenericContainer<?> redisContainer and replace the image string with the chosen fixed version, and run tests to confirm compatibility.apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java (1)
9-9:PageRequestimport가 사용되지 않는다.코드에서
PageRequest를 사용하는 부분이 보이지 않는다. 불필요한 import는 제거해야 한다.♻️ 수정 제안
-import org.springframework.data.domain.PageRequest;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java` at line 9, Remove the unused import org.springframework.data.domain.PageRequest from ProductV1Controller.java; locate the import statement (PageRequest) at the top of the file and delete it, then recompile or run the linter to ensure no other unused imports remain and clean up any additional unnecessary imports if reported.apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java (1)
94-122:getProduct()실패 케이스 테스트가 누락되었다.코딩 가이드라인에 따라 단위 테스트는 실패 케이스와 예외 흐름을 포함해야 한다. 상품 미존재, 재고 미존재 등의 예외 시나리오 테스트가 필요하다.
🧪 추가 테스트 제안
`@DisplayName`("존재하지 않는 상품을 조회하면 NOT_FOUND 예외를 던진다") `@Test` void throwsWhenProductNotFound() { // given Long productId = 999L; when(productService.getById(productId)) .thenThrow(new CoreException(ErrorType.NOT_FOUND, "상품을 찾을 수 없습니다.")); // when & then assertThrows(CoreException.class, () -> productFacade.getProduct(productId)); } `@DisplayName`("브랜드 조회 실패 시 brandName이 null로 반환된다") `@Test` void returnNullBrandNameWhenBrandNotFound() { // given Long productId = 1L; Long brandId = 1L; ProductModel product = new ProductModel("에어맥스", "러닝화", new Money(129000), brandId); StockModel stock = new StockModel(productId, 100); when(productService.getById(productId)).thenReturn(product); when(brandService.getBrandForAdmin(brandId)) .thenThrow(new CoreException(ErrorType.NOT_FOUND, "브랜드를 찾을 수 없습니다.")); when(stockService.getByProductId(productId)).thenReturn(stock); // when ProductDetail result = productFacade.getProduct(productId); // then assertThat(result.brandName()).isNull(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java` around lines 94 - 122, Add missing failure-case unit tests in ProductFacadeTest for productFacade.getProduct: create a test that when productService.getById(productId) throws a CoreException(ErrorType.NOT_FOUND, ...) then assertThrows(CoreException.class, () -> productFacade.getProduct(productId)); add a test where productService.getById(productId) returns a ProductModel but brandService.getBrandForAdmin(brandId) throws CoreException and stockService.getByProductId(productId) returns StockModel, then call productFacade.getProduct(productId) and assert that result.brandName() is null; optionally add a third test where stockService.getByProductId(productId) throws or returns null and assert the expected StockStatus/exception from productFacade.getProduct; reference ProductFacadeTest, productFacade.getProduct, productService.getById, brandService.getBrandForAdmin, and stockService.getByProductId to locate where to add these tests.apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java (1)
35-42: 좋아요 목록 조회가 다시 N+1 조회로 늘어난다.현재 구현은
likes.map(...)내부에서productService.getById를 반복 호출하므로 페이지 크기만큼 추가 조회가 발생한다. 운영에서는 좋아요가 많은 사용자일수록 응답 시간이 선형으로 늘고 DB 부하가 커진다. 상품 ID를 먼저 모아 배치 조회한 뒤Map<Long, ProductModel>로 조합하는 방식으로 바꾸는 편이 안전하다. 추가 테스트로는 좋아요 N건 조회 시 product 조회가 1회의 배치 호출로 고정되는지, 또는 SQL 수가 페이지 크기에 비례하지 않는지 검증하는 통합 테스트를 넣어야 한다.apps/commerce-api/src/main/java/com/loopers/application/brand/BrandService.java (1)
71-75: 빈 ID 목록은 즉시 반환해서 빈 페이지 비용을 제거하는 편이 낫다.상품 목록 최적화 경로에서는 빈 페이지도 자주 발생하는데, 현재 구현은 그 경우에도 repository 호출과 stream/map 생성 비용을 지불한다. 운영에서는 필터 결과가 없는 요청이 많을수록 무의미한 DB round-trip이 누적된다.
ids.isEmpty()일 때Map.of()를 즉시 반환하도록 가드를 두는 편이 안전하다. 추가 테스트로는 빈 목록 입력 시 빈 Map을 반환하고brandRepository.findAllByIdIn이 호출되지 않는지 검증해야 한다.수정 예시
`@Transactional`(readOnly = true) public Map<Long, BrandModel> getByIds(List<Long> ids) { + if (ids.isEmpty()) { + return Map.of(); + } return brandRepository.findAllByIdIn(ids) .stream() .collect(Collectors.toMap(BrandModel::getId, Function.identity())); }🤖 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/brand/BrandService.java` around lines 71 - 75, The getByIds method in BrandService should short-circuit when the input ids list is empty to avoid an unnecessary repository call and collection creation: add a guard at the start of BrandService.getByIds that returns Map.of() when ids == null or ids.isEmpty(), then keep the existing brandRepository.findAllByIdIn(ids)... collect(...) logic for non-empty input. Also add a unit test that calls getByIds with an empty list and asserts an empty Map is returned and that brandRepository.findAllByIdIn was not invoked (mock verification).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java`:
- Around line 106-109: Add the same transactional boundary to the delete path as
used by update: annotate ProductFacade.delete with `@Transactional` (matching the
transaction settings used on update) so cache eviction via `@CacheEvict` happens
inside the transaction and will not be applied if productService.delete() fails;
ensure you import the annotation and keep the existing `@CacheEvict` on the
delete(Long productId) method so behavior mirrors update().
In `@apps/commerce-api/src/main/java/com/loopers/domain/like/LikeModel.java`:
- Around line 12-14: Add a pre-migration Flyway script that finds and removes
any duplicate (user_id, product_id) rows then adds the unique constraint
referenced by LikeModel (uk_likes_user_product), update LikeToggleService to
handle race inserts by catching DataIntegrityViolationException around the
create/restore path and treating it as a successful idempotent like (return
existing/created state rather than propagating), add a
DataIntegrityViolationException handler to ApiControllerAdvice that maps
constraint-violation cases to the consistent API response (200 with current like
state or an appropriate 4xx domain mapping as discussed), and add an integration
test that fires two concurrent like requests against LikeToggleService/endpoint
verifying a single like is created and a 200 response with likeCount incremented
by 1.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java`:
- Around line 14-19: ProductModel's current table indexes miss brand_id for
created_at/price sorts causing filesort when findAll() filters by brandId and
sorts by CREATED_DESC/PRICE_*. Add composite indexes (brand_id, deleted_at,
created_at) and (brand_id, deleted_at, price), remove or reconsider
idx_product_deleted_like to reduce write-amplification on like_count, and update
any index-related docs/tests; also add EXPLAIN-based CI checks for findAll()
variants (with/without brand filter and each sort) to assert index usage instead
of filesort.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java`:
- Around line 61-68: The query uses a single-column OrderSpecifier (built via
toOrderSpecifier) which allows non-deterministic ordering on ties; modify to
always append the primary key (product.id or ProductModel.id) as a secondary
OrderSpecifier so sorting is deterministic (update toOrderSpecifier to return
either a List<OrderSpecifier<?>> or have callers combine the primary
OrderSpecifier with an OrderSpecifier for id before calling
queryFactory.orderBy), change usages around
queryFactory.selectFrom(product).orderBy(...) in ProductRepositoryImpl to pass
both specifiers, and add an integration test that inserts multiple products with
identical createdAt/price/likeCount and asserts sequential page fetches (page 1
then page 2) contain no duplicates or gaps.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.java`:
- Line 17: Update the OrderV1ApiSpec#getMyOrders signature to exactly match the
controller for /api/v1/orders: change the return type and parameters of
getMyOrders (currently ApiResponse<OrderV1Dto.OrderSummaryResponse>
getMyOrders(MemberModel member, Pageable pageable)) to the controller's contract
(e.g., return a collection/array response type and include the startAt/endAt
query parameters instead of Pageable if the controller uses explicit query
params), so the interface signature and types (OrderV1ApiSpec, getMyOrders,
MemberModel, Pageable, OrderV1Dto.OrderSummaryResponse) are identical to the
implemented controller; then add an OpenAPI snapshot/contract test that requests
/api/v1/orders and asserts the response schema is an array and that startAt and
endAt are documented as query parameters.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductAdminV1Controller.java`:
- Around line 48-53: The controller parses sortType into ProductSortType but
never uses it; update the flow to pass the parsed sort to service/facade: add a
ProductSortType parameter to ProductService.getProductsForAdmin(Long brandId,
ProductSortType sortType, Pageable pageable) and to
ProductFacade.getProductsForAdmin(...), update their implementations to apply
the sort instead of always using CREATED_DESC, and then call
productFacade.getProductsForAdmin(null, sort, pageable) from
ProductAdminV1Controller so the client-provided ProductSortType is actually
applied.
In
`@apps/commerce-api/src/test/java/com/loopers/application/like/LikeTransactionCacheTest.java`:
- Around line 33-38: The test currently hard-codes redis:latest and starts the
GenericContainer manually in a static block; change the GenericContainer<?>
redisContainer declaration in LikeTransactionCacheTest to use a pinned image
(e.g., DockerImageName.parse("redis:7.0")) and annotate the field with
Testcontainers' `@Container` so JUnit/Testcontainers manages lifecycle (remove the
static initializer and the explicit redisContainer.start() call); keep the
withExposedPorts(6379) configuration and run the tests repeatedly to validate
cache behavior against the pinned image.
In
`@apps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceIntegrationTest.java`:
- Around line 163-167: The test ProductServiceIntegrationTest currently only
asserts total >=1 and the first product name, which can miss other-brand items;
update the assertions after calling productFacade.getProducts(nikeId,
ProductSortType.CREATED_DESC, PageRequest.of(0,10)) to: (1) assert that
result.getTotalElements() equals the expected exact count for nikeId (use the
known fixture count), (2) iterate result.getContent() and assert every
ProductDetail.brandId() equals nikeId, and (3) add a complementary call using
adidasId to assert zero nike products returned (result.getTotalElements() == 0
and all brandId() == adidasId when expected), thereby ensuring the brand filter
is fully validated.
- Around line 147-150: Tighten the soft-delete assertions in
ProductServiceIntegrationTest: instead of only asserting
result.getTotalElements() >= 2 after calling productService.getProducts(...,
ProductSortType.CREATED_DESC, PageRequest.of(0, 10)), assert the exact expected
count (e.g. equals(expectedActiveCount)) and add an explicit assertion that the
known soft-deleted entity ID is not present in result.getContent() (e.g.
noneMatch(p -> p.getId().equals(deletedId))). Ensure the test arranges a
predictable dataset (create active vs soft-deleted products or reference fixture
IDs) so the exact count and exclusion check are reliable.
In
`@apps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceTest.java`:
- Around line 235-253: Existing tests only verify that
productService.incrementLikeCount/decrementLikeCount invoke
productRepository.incrementLikeCount/decrementLikeCount, so they miss the
failure boundary when the repository reports 0 rows updated; add tests for both
methods that stub productRepository.incrementLikeCount(id) and
productRepository.decrementLikeCount(id) to return 0 and then assert the service
behavior (either expect a thrown exception or a defined no-op/validation result
per the service contract), and also keep the positive-case tests where the
repository returns 1 to assert success; locate and modify the tests around
productService.incrementLikeCount, productService.decrementLikeCount, and the
mocked productRepository.incrementLikeCount/productRepository.decrementLikeCount
to cover both 1-rows and 0-rows scenarios.
In
`@apps/commerce-api/src/test/java/com/loopers/config/RedisCacheConfigTest.java`:
- Around line 30-40: Update the test so it verifies the TTL for the
"productDetail" cache instead of only checking non-null: retrieve the
RedisCacheManager (cacheManager), call
redisCacheManager.getCacheConfigurations().get("productDetail") and assert that
its getTtlFunction() is a FixedDurationTtlFunction whose duration() equals 10
minutes; also add a negative test that fails when the configuration is missing.
Use the existing test method productDetail_cache_configuration_should_exist (or
create a new assertion in it) and reference getCacheConfigurations(),
get("productDetail"), getTtlFunction(), and FixedDurationTtlFunction.duration()
to locate the configuration and perform the assertions.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceTest.java`:
- Around line 53-72: Add a negative test in LikeServiceTest that covers the
non-existent case: mock likeRepository.findByUserIdAndProductId(userId,
productId) to return Optional.empty() and assert that
likeService.findByUserIdAndProductId(userId, productId) also returns an empty
Optional; place it alongside the existing findsByUserIdAndProductId test (e.g.,
a test named returnsEmptyWhenNotFound) to ensure the service preserves
Optional.empty() from the repository and prevents regression of idempotent
"unlike" behavior.
In `@docs/mentoring`:
- Line 22: Update the header text "### 1.2 커버링 인덱스 (Covered Index)" to use the
correct English term "Covering Index" (i.e., change "(Covered Index)" to
"(Covering Index)") so the section title reads "### 1.2 커버링 인덱스 (Covering
Index)"; ensure any other occurrences of "Covered Index" in the same document
are replaced with "Covering Index" for consistency.
In `@docs/sql/seed-data.sql`:
- Around line 7-10: 현재 스크립트는 SET FOREIGN_KEY_CHECKS = 0; 이후 stock, product,
brand만 TRUNCATE 하여 product를 참조하는 하위 테이블(예: 좋아요/likes, 리뷰/review, product_images
등)의 레코드가 남아 재시드 시 ID 재사용으로 데이터 오염이 발생할 수 있습니다; 수정 방법은 TRUNCATE 순서를 부모보다 먼저 하위
테이블을 모두 비우도록 변경하거나(하위 테이블 이름을 명시해 부모 TRUNCATE 전에 실행), 또는 TRUNCATE 실행 전 product를
참조하는 하위 테이블의 COUNT(*)가 0인지 확인하는 검증 쿼리를 추가하고 조건 불충분 시 스크립트를 중단하도록 하십시오; 또한 검증 실패
시 어떤 테이블에서 건수가 남아있는지 명확히 로그/출력하도록 하세요.
- Around line 62-71: Seed randomness makes results non-deterministic; replace
RAND() usage by deterministic formulas based on loop index i (e.g., set
price_val = 1000 + ((i * 7919) % 999001) and like_val = (i * 9973) % 10001) and
make created_at unique per row using TIMESTAMPADD (e.g., created_at =
TIMESTAMPADD(SECOND, i, now_ts) or subtracting i to spread timestamps); update
assignments for price_val, like_val and created_at in the loops (look for
variables price_val, like_val, created_at and RAND() usage) and add verification
queries after seeding to assert distribution reproducibility such as SELECT
SUM(price) AS price_checksum, SUM(like_count) AS like_checksum FROM products;
and SELECT COUNT(DISTINCT created_at) FROM products; and run the same seed twice
in tests to ensure checksums and distinct created_at counts match; apply the
same deterministic changes where RAND() appears in the other loop blocks
referenced (lines ~79-86 and ~197-202).
- Around line 2-3: Update the file header to state the minimum MySQL version as
"MySQL 8.0.18 or later" and/or replace every EXPLAIN ANALYZE usage (search for
the literal "EXPLAIN ANALYZE") with plain EXPLAIN to maintain compatibility with
< 8.0.18; additionally add a runtime version check by inserting a SELECT
VERSION() near the top of the script to make it clear which server version is
being used when running the seed script and then verify all EXPLAIN ANALYZE
queries (if you keep them) run successfully on 8.0.18+.
In
`@modules/redis/src/main/java/com/loopers/config/redis/CustomCacheErrorHandler.java`:
- Around line 12-27: 로그에서 예외 원인과 키 노출 문제가 있으니 CustomCacheErrorHandler의
handleCacheGetError, handleCachePutError, handleCacheEvictError,
handleCacheClearError를 수정해 예외 메시지 대신 예외 객체 전체를 로거에 전달해(cause/stack 보존) 기록하고, 원본
key는 그대로 출력하지 않도록 마스킹 또는 해시하는 유틸(예: maskKey(String) 또는 hashKey(Object))를 적용해
기록하도록 변경하고, 해당 변경을 검증하는 로그 캡처 테스트를 추가해 원문 key 미노출 및 예외 스택 포함 여부를 확인하세요.
In `@modules/redis/src/main/java/com/loopers/config/redis/RedisCacheConfig.java`:
- Around line 29-37: The current Redis cache config creates a local ObjectMapper
with LaissezFaireSubTypeValidator and DefaultTyping.EVERYTHING which over-trusts
type metadata; change RedisCacheConfig to accept the application's Jackson
ObjectMapper bean via constructor injection (reuse global Jackson config used
elsewhere) and remove activateDefaultTyping usage, or at minimum switch to
ObjectMapper.DefaultTyping.NON_FINAL; alternatively register specific
GenericJackson2JsonRedisSerializer instances per cache/DTO (e.g., ProductDetail)
rather than a global EVERYTHING policy; update tests to cover ProductDetail
round-trip serialization and ensure cache read failures due to unexpected type
metadata trigger the CustomCacheErrorHandler fallback to DB.
---
Outside diff comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeTransactionService.java`:
- Around line 30-49: The direct cache.evict(productId) call inside
evictProductDetailCache bypasses the CustomCacheErrorHandler and can propagate
Redis exceptions into the doLike/doUnlike transactions; replace this direct
eviction with a Spring-managed eviction path by moving cache eviction out of the
DB transaction and using a method annotated with `@CacheEvict`(cacheNames =
"productDetail", key = "#productId") or publish an event and handle eviction in
a separate listener annotated with `@TransactionalEventListener`(phase =
AFTER_COMMIT) so that evictions run after commit and go through the cache error
handler (update doLike/doUnlike to publish the event or call the `@CacheEvict`
method instead of calling evictProductDetailCache, and remove/direct-evict usage
of cache.evict in evictProductDetailCache).
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java`:
- Around line 119-126: The getBrandName(Long brandId) method currently swallows
all exceptions silently; modify its catch block to log the failure including the
brandId and exception stacktrace before returning null (use the class logger,
e.g., the existing or add a private static final Logger for ProductFacade).
Specifically, keep the intent of returning null on failure but call logger.warn
or logger.error with a clear message like "Failed to fetch brand for id={}" and
include the caught Exception (from the catch(Exception e)) while referencing
BrandModel and brandService.getBrandForAdmin to locate the try/catch to update.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java`:
- Around line 54-86: Add a unit test to OrderServiceTest that verifies owner
mismatch throws BAD_REQUEST: in the GetOrder nested class create a test (e.g.,
throwsOnUserIdMismatch) that sets up an OrderModel with ownerId (use
ReflectionTestUtils to set id), stubs orderRepository.findById(id) to return
that order, then call orderService.getOrder(id, requesterId) where requesterId
!= ownerId and assert the thrown CoreException has ErrorType.BAD_REQUEST;
reference OrderModel, orderRepository.findById, and orderService.getOrder to
locate where to add the test.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/brand/BrandService.java`:
- Around line 71-75: The getByIds method in BrandService should short-circuit
when the input ids list is empty to avoid an unnecessary repository call and
collection creation: add a guard at the start of BrandService.getByIds that
returns Map.of() when ids == null or ids.isEmpty(), then keep the existing
brandRepository.findAllByIdIn(ids)... collect(...) logic for non-empty input.
Also add a unit test that calls getByIds with an empty list and asserts an empty
Map is returned and that brandRepository.findAllByIdIn was not invoked (mock
verification).
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java`:
- Line 28: The findAllByBrandIdAndDeletedAtIsNull(Long brandId) method in
ProductJpaRepository allows unbounded list retrieval which risks OOM/GC/long
transactions for large brands; remove this unpaged method (delete the signature)
and ensure callers use the existing paged variant
findAllByBrandIdAndDeletedAtIsNull(Long brandId, Pageable pageable) or replace
callers that perform mass changes with a bulk update query (e.g., a `@Modifying`
JPQL/SQL method) in ProductJpaRepository; add an integration test that simulates
large-brand data processed via a paging loop or a bulk update to verify
per-transaction memory stays bounded.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.java`:
- Around line 22-31: 메서드 getAll에서 캡처한 `@AdminUser` String adminLdap가 사용되지 않으므로
감사/권한 목적으로 이 값을 전달하거나 의도를 명시해야 합니다; 수정안으로는 OrderAdminV1Controller# getAll 시
orderFacade.getAllForAdmin(pageable) 대신 orderFacade.getAllForAdmin(adminLdap,
pageable)처럼 adminLdap를 Facade로 전달하도록 orderFacade의 시그니처(또는 오버로드)를 업데이트하거나, 정말로 필요
없다면 컨트롤러에서 해당 파라미터를 제거하고 불필요 파라미터임을 주석으로 남겨 의도를 분명히 하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java`:
- Line 9: Remove the unused import org.springframework.data.domain.PageRequest
from ProductV1Controller.java; locate the import statement (PageRequest) at the
top of the file and delete it, then recompile or run the linter to ensure no
other unused imports remain and clean up any additional unnecessary imports if
reported.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.java`:
- Around line 46-134: Add integration tests to the PlaceOrder nested class that
exercise coupon handling via orderFacade.placeOrder(couponIssueId): create
helper(s) to issue a coupon to a user (e.g., createCouponIssue(userId,
couponDef)) and add tests that (1) apply a valid coupon and assert totalAmount()
reflects the discount, (2) attempt to use another user's coupon and assert a
CoreException with ErrorType.BAD_REQUEST/NOT_FOUND as appropriate, (3) attempt
to reuse an already-consumed coupon and assert a CoreException, and (4) simulate
an order failure after coupon consumption and assert the couponIssue status is
rolled back to unused; reference placeOrder, OrderResult, OrderItemCommand, and
any coupon service methods (e.g., couponIssue creation/claim/check) to locate
where to wire the tests.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java`:
- Around line 47-52: The tests declare mocks for CouponIssueService and
CouponService but never exercise coupon logic (couponIssueId passed as null);
either add unit tests that exercise the coupon path or remove the unused mocks
and add a TODO. To fix: for positive and negative coupon scenarios, stub
CouponIssueService.lookup(couponIssueId) and CouponService.lookup(...) (or
methods used in OrderFacade) to return mocked CouponIssue and Coupon objects,
verify calls to couponIssue.validateOwner(), coupon.validateUsable(), and
couponIssue.use(), and assert discount calculation; otherwise delete the
CouponIssueService and CouponService `@Mock` declarations from OrderFacadeTest and
add a TODO comment indicating coupon behavior tests will be added later.
Reference the CouponIssueService, CouponService, couponIssueId,
couponIssue.validateOwner(), coupon.validateUsable(), and couponIssue.use() to
locate relevant code.
In
`@apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java`:
- Around line 115-133: The test suite is missing a test that verifies
ProductFacade.updateStock() evicts the "productDetail" cache; add a new test
method in ProductCacheIntegrationTest (e.g., 재고_수정_시_상세_캐시가_삭제된다) that mirrors
the existing delete test: create and save BrandModel and ProductModel, seed
StockModel, call productFacade.getProduct(product.getId()) and assert
cacheManager.getCache("productDetail").get(product.getId()) isNotNull, then call
productFacade.updateStock(product.getId(), <newQty>) and finally assert the same
cache entry is null to confirm `@CacheEvict` on ProductFacade.updateStock() works.
- Around line 32-33: The test uses an unstable image tag; update the redis
container declaration (redisContainer / GenericContainer and
DockerImageName.parse usage) to pin a specific Redis version (e.g., change
"redis:latest" to "redis:7.2" or another CI-approved immutable tag) so CI tests
are reproducible; locate the static final GenericContainer<?> redisContainer and
replace the image string with the chosen fixed version, and run tests to confirm
compatibility.
In
`@apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java`:
- Around line 94-122: Add missing failure-case unit tests in ProductFacadeTest
for productFacade.getProduct: create a test that when
productService.getById(productId) throws a CoreException(ErrorType.NOT_FOUND,
...) then assertThrows(CoreException.class, () ->
productFacade.getProduct(productId)); add a test where
productService.getById(productId) returns a ProductModel but
brandService.getBrandForAdmin(brandId) throws CoreException and
stockService.getByProductId(productId) returns StockModel, then call
productFacade.getProduct(productId) and assert that result.brandName() is null;
optionally add a third test where stockService.getByProductId(productId) throws
or returns null and assert the expected StockStatus/exception from
productFacade.getProduct; reference ProductFacadeTest, productFacade.getProduct,
productService.getById, brandService.getBrandForAdmin, and
stockService.getByProductId to locate where to add these tests.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.java`:
- Around line 33-35: The OrderItemRepository mock declared in OrderServiceTest
is unused; either remove the field or add tests that exercise code paths relying
on it (e.g., when OrderService interacts with order items). If you choose
removal, delete the `@Mock` private OrderItemRepository orderItemRepository
declaration from OrderServiceTest; if you choose to keep it, add one or more
tests that call the OrderService method(s) that use OrderItemRepository and
verify interactions (use when(...) to stub repository responses and
verify(orderItemRepository).save(...)/findByOrderId(...) as appropriate) so the
mock is actually used during InjectMocks injection.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/like/LikeV1ApiE2ETest.java`:
- Line 53: The current E2E helper dereferences the response body immediately
(see the ParameterizedTypeReference call that ends with .getBody().data().id()),
which causes NPEs on API failures and hides real failure details; update the
createProduct helper to first assert/validate the HTTP status code and that
getBody() and data() are non-null, log or include the error/body when failing,
then return the id only after those checks, and add tests that assert the helper
fails with clear messages for invalid admin headers and for invalid request
payloads so failures surface status and error body instead of NPEs.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/order/OrderV1ApiE2ETest.java`:
- Around line 60-63: The current call that directly dereferences
getBody().data().id() (via testRestTemplate.exchange(...).getBody().data().id())
can NPE and hide HTTP status/body info; change the code in OrderV1ApiE2ETest to
first assert the ResponseEntity status is OK and that getBody() and
getBody().data() are non-null before extracting id, using the same
testRestTemplate.exchange(...) response object and adminHeaders() helper, and
include a separate test that posts an invalid brandId to the
"/api-admin/v1/products" endpoint and asserts a clear error status and message
(not NPE) to validate failure behavior.
In `@docs/mentoring`:
- Around line 187-196: Update the "Write-Through vs Write-Around" section to use
standard caching definitions: state that Write-Through writes synchronously to
both cache and DB, and Write-Around writes only to the DB (cache is populated on
subsequent reads or TTL expiry); then keep the pragmatic recommendation (prefer
Write-Around for DB as source-of-truth) but add an explicit note clarifying that
this is a practical deviation from the textbook definitions and explain when to
prefer Write-Through for strict consistency.
- Around line 788-798: Add a checklist item to validate the Redis → DB fallback
path by explicitly testing the CustomCacheErrorHandler's behavior under Redis
failures: specify tests to simulate Redis errors, verify that requests fall back
to DB and return correct data, assert no bad data is cached, and include
monitoring/alert validation for fallback occurrences so the checklist covers
operational validation of the DB fallback path.
- Around line 170-185: The document currently labels the Evict strategy as "하수"
without acknowledging valid use-cases; update the mentoring docs to add a
concise trade-off paragraph explaining when Evict is appropriate (e.g., complex
cache structures where partial updates are hard, low write/high read workloads,
and acceptable cache-miss cost) and reference the PR's concrete case (the
ProductDetail composite DTO and the use of `@CacheEvict`) to justify why Evict can
be safer there; keep the text short, neutral, and add one or two bullet-style
examples clarifying when to prefer Put vs Evict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f143572a-4c3d-43e8-ae90-22749ebbaa7e
⛔ Files ignored due to path filters (9)
.claude/commands/analyze-transaction.mdis excluded by!**/*.mdand included by**docs/blog/index-tuning-blog-draft.mdis excluded by!**/*.mdand included by**docs/blog/index-tuning-research.mdis excluded by!**/*.mdand included by**docs/plans/2026-03-12-query-optimization.mdis excluded by!**/*.mdand included by**docs/plans/2026-03-12-redis-cache.mdis excluded by!**/*.mdand included by**docs/requirements/round5-requirement.mdis excluded by!**/*.mdand included by**docs/나만의 동시성 제어 판단기준.pngis excluded by!**/*.png,!**/*.pngand included by**pr-body.mdis excluded by!**/*.mdand included by**research.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (74)
.http/cache-test.httpapps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.javaapps/commerce-api/src/main/java/com/loopers/application/brand/BrandInfo.javaapps/commerce-api/src/main/java/com/loopers/application/brand/BrandService.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeTransactionService.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductDetail.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/application/stock/StockService.javaapps/commerce-api/src/main/java/com/loopers/config/WebMvcConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/BrandRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeModel.javaapps/commerce-api/src/main/java/com/loopers/domain/product/Money.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductSortType.javaapps/commerce-api/src/main/java/com/loopers/domain/stock/StockRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/brand/BrandJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/stock/StockRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/auth/LoginMemberArgumentResolver.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/BrandAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/admin/BrandAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/admin/BrandAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/like/LikeV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/admin/OrderAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/order/admin/OrderAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/admin/ProductAdminV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/admin/ProductAdminV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminInfo.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminUser.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminUserArgumentResolver.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/LoginMember.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/LoginMemberArgumentResolver.javaapps/commerce-api/src/test/java/com/loopers/application/ConcurrencyIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/brand/BrandFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/brand/BrandServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/brand/BrandServiceTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeTransactionCacheTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductServiceTest.javaapps/commerce-api/src/test/java/com/loopers/application/stock/StockServiceTest.javaapps/commerce-api/src/test/java/com/loopers/config/CustomCacheErrorHandlerTest.javaapps/commerce-api/src/test/java/com/loopers/config/RedisCacheConfigTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/order/OrderServiceTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/brand/BrandV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/like/LikeV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/order/OrderV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/product/ProductV1ApiE2ETest.javadocs/mentoringdocs/sql/seed-data.sqlmodules/redis/src/main/java/com/loopers/config/redis/CustomCacheErrorHandler.javamodules/redis/src/main/java/com/loopers/config/redis/RedisCacheConfig.java
💤 Files with no reviewable changes (12)
- apps/commerce-api/src/main/java/com/loopers/interfaces/auth/LoginMemberArgumentResolver.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/admin/ProductAdminV1Controller.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminInfo.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/admin/BrandAdminV1Dto.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminUser.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/brand/admin/BrandAdminV1Controller.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/admin/ProductAdminV1Dto.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/admin/OrderAdminV1Controller.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminUserArgumentResolver.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/admin/OrderAdminV1Dto.java
- apps/commerce-api/src/main/java/com/loopers/domain/product/Money.java
- apps/commerce-api/src/main/java/com/loopers/interfaces/auth/LoginMember.java
Comment on lines
+106
to
109
| @CacheEvict(cacheNames = "productDetail", key = "#productId") | ||
| public void delete(Long productId) { | ||
| productService.delete(productId); | ||
| } |
There was a problem hiding this comment.
delete() 메서드에 @Transactional 누락으로 캐시 무효화 타이밍 불일치 가능성이 있다.
@CacheEvict는 메서드 실행 전 또는 후에 캐시를 무효화하는데, 현재 트랜잭션 없이 실행되면 productService.delete()가 실패해도 캐시가 이미 무효화될 수 있다. update() 메서드와 동일하게 @Transactional을 추가해야 한다.
🐛 수정 제안
`@CacheEvict`(cacheNames = "productDetail", key = "#productId")
+@Transactional
public void delete(Long productId) {
productService.delete(productId);
}🤖 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/ProductFacade.java`
around lines 106 - 109, Add the same transactional boundary to the delete path
as used by update: annotate ProductFacade.delete with `@Transactional` (matching
the transaction settings used on update) so cache eviction via `@CacheEvict`
happens inside the transaction and will not be applied if
productService.delete() fails; ensure you import the annotation and keep the
existing `@CacheEvict` on the delete(Long productId) method so behavior mirrors
update().
Comment on lines
+12
to
+14
| @Table(name = "likes", uniqueConstraints = { | ||
| @UniqueConstraint(name = "uk_likes_user_product", columnNames = {"user_id", "product_id"}) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# likes 유니크 제약 관련 사전 정리/예외 처리 존재 여부 점검(읽기 전용)
rg -n -C3 'uk_likes_user_product|unique.*user_id.*product_id|likes' --type sql --type java
rg -n -C3 'DataIntegrityViolationException|ConstraintViolationException|DuplicateKeyException|CoreException' apps/commerce-api/src/main/java/com/loopers/application/like
rg -n -C3 '멱등|idempotent|likeIsIdempotent|두 번 좋아요' apps/commerce-api/src/test/javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 26366
🏁 Script executed:
# LikeFacade.like() 메서드 구현 확인
cat -n apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java | head -40
# LikeService 구현 확인
find apps/commerce-api -name "LikeService.java" -type f -exec cat -n {} \;
# DB 마이그레이션 스크립트 검색
find . -path "*/db/migration*" -o -path "*/flyway*" -o -path "*/liquibase*" | grep -i like || true
# SQL 파일 검색
find . -name "*.sql" -type f | xargs grep -l "likes\|uk_likes" 2>/dev/null || trueRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3127
🏁 Script executed:
# LikeTransactionService 찾기
find apps/commerce-api -name "LikeTransactionService.java" -type f -exec cat -n {} \;
# 광범위한 마이그레이션 파일 검색
find . -type f \( -name "*.sql" -o -name "V*.sql" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | head -20
# resources 디렉토리에서 마이그레이션 파일 찾기
find apps/commerce-api/src -type d -name "*migration*" -o -name "*flyway*" -o -name "*liquibase*"
# 전체 SQL 파일 목록
find apps/commerce-api -name "*.sql" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2852
🏁 Script executed:
# LikeToggleService 찾기
find apps/commerce-api -name "LikeToggleService.java" -type f -exec cat -n {} \;
# Flyway 마이그레이션 디렉토리 구조
find apps/commerce-api -type d -name "db" -o -type d -name "migration" -o -type d -name "migrations"
# resources 경로 전체 탐색
find apps/commerce-api/src/main/resources -type f | head -30Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1497
🏁 Script executed:
# application.yml에서 Flyway 설정 확인
cat apps/commerce-api/src/main/resources/application.yml | grep -A 10 -i flyway
# 전체 resources 디렉토리 구조
find apps/commerce-api/src/main/resources -type f -o -type d | sort
# 전역 예외 처리 확인
find apps/commerce-api -name "*ExceptionHandler*" -o -name "*ErrorHandler*" -o -name "*ControllerAdvice*" | xargs cat -n 2>/dev/null | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5658
유니크 제약 추가 시 배포 위험과 동시성 예외 처리 누락
Line 12-14 변경은 도메인 규칙 강화 방향이 맞으나, 운영 환경 배포 시 두 가지 위험이 있다.
-
기존 중복 데이터로 인한 DDL 실패: 마이그레이션 전에
(user_id, product_id)중복 데이터가 1건이라도 있으면 스키마 적용이 실패한다. Flyway 마이그레이션이 구성되지 않은 것으로 보이므로, 제약 추가 전 사전 정리 마이그레이션이 필수다. -
동시 요청 시 예외 처리 누락: LikeToggleService는 순차 호출에서 멱등성을 보장한다(
existing.isEmpty()체크 후 create/restore/ignore 결정). 하지만 동시 요청 T1, T2가 모두empty상태를 조회한 후 동시에 insert를 시도하면DataIntegrityViolationException이 발생한다. 현재 ApiControllerAdvice에는 이 예외를 처리하는 핸들러가 없어 500 에러로 노출된다.
수정안:
- (1) 마이그레이션 파일 추가: 중복 제거 후 제약 추가
- (2)
DataIntegrityViolationException핸들러 추가: 제약 위반 시 멱등 성공(200) 또는 도메인 예외(4xx)로 일관 매핑 - (3) 동시 좋아요 2건 요청 통합 테스트 추가: 200 응답 + likeCount 1 증가 확인
🤖 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/like/LikeModel.java`
around lines 12 - 14, Add a pre-migration Flyway script that finds and removes
any duplicate (user_id, product_id) rows then adds the unique constraint
referenced by LikeModel (uk_likes_user_product), update LikeToggleService to
handle race inserts by catching DataIntegrityViolationException around the
create/restore path and treating it as a successful idempotent like (return
existing/created state rather than propagating), add a
DataIntegrityViolationException handler to ApiControllerAdvice that maps
constraint-violation cases to the consistent API response (200 with current like
state or an appropriate 4xx domain mapping as discussed), and add an integration
test that fires two concurrent like requests against LikeToggleService/endpoint
verifying a single like is created and a 200 response with likeCount incremented
by 1.
Comment on lines
+14
to
+19
| @Table(name = "product", indexes = { | ||
| @Index(name = "idx_product_brand_deleted_like", columnList = "brand_id, deleted_at, like_count"), | ||
| @Index(name = "idx_product_deleted_like", columnList = "deleted_at, like_count"), | ||
| @Index(name = "idx_product_deleted_created", columnList = "deleted_at, created_at"), | ||
| @Index(name = "idx_product_deleted_price", columnList = "deleted_at, price") | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 정렬 조합과 인덱스 정합성 확인(읽기 전용)
rg -n -C4 'enum ProductSortType|LATEST|PRICE|LIKE' apps/commerce-api/src/main/java
rg -n -C6 'orderBy|likeCount|createdAt|price|brandId|deletedAt' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java
rg -n -C3 '@Index|columnList' apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4902
🏁 Script executed:
# ProductRepositoryImpl의 모든 쿼리 메서드 확인
rg -n 'public|private' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java | head -30
# ProductModel의 전체 인덱스 구조 확인
cat -n apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java | head -30
# JPA Repository 인터페이스 메서드 확인
rg -n 'findBy|findAll' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2635
🏁 Script executed:
# ProductModel의 update 메서드 확인
rg -n 'incrementLikeCount|decrementLikeCount|UPDATE|update' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java -A5
# ProductModel의 필드 수정 패턴 확인
rg -n 'deletedAt|likeCount|price|deleted_at|like_count' apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1623
브랜드 필터와 함께 created_at/price로 정렬할 때 filesort 발생 위험이 있다
현재 인덱스 설계에서 idx_product_deleted_created와 idx_product_deleted_price는 brand_id를 포함하지 않아, 브랜드별 조회와 함께 정렬할 때 인덱스 스킵으로 인한 filesort가 발생한다. findAll() 메서드가 brandId 필터와 CREATED_DESC/PRICE_ASC/DESC 정렬을 동시에 처리하므로 대량 데이터 조회 시 성능 저하가 필연적이다.
수정안
(brand_id, deleted_at, created_at)및(brand_id, deleted_at, price)인덱스 추가- 중복성을 낮추기 위해
idx_product_deleted_like(현재 like_count만 정렬하는 쿼리에 사용) 제거 검토 - like_count는 호트 업데이트 컬럼이므로 인덱스 개수 최소화가 쓰기 증폭 감소에 필수적
추가 테스트
- EXPLAIN 기반 CI 회귀 점검: 각 정렬 타입별(brand 필터 있음/없음)로 index_type이 ALL(filesort)이 아닌지 확인
- 프로덕션 같은 데이터 규모에서 브랜드별 조회 쿼리 응답 시간 기준선 설정 후 모니터링
🤖 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/ProductModel.java`
around lines 14 - 19, ProductModel's current table indexes miss brand_id for
created_at/price sorts causing filesort when findAll() filters by brandId and
sorts by CREATED_DESC/PRICE_*. Add composite indexes (brand_id, deleted_at,
created_at) and (brand_id, deleted_at, price), remove or reconsider
idx_product_deleted_like to reduce write-amplification on like_count, and update
any index-related docs/tests; also add EXPLAIN-based CI checks for findAll()
variants (with/without brand filter and each sort) to assert index usage instead
of filesort.
Comment on lines
+61
to
+68
| OrderSpecifier<?> orderSpecifier = toOrderSpecifier(product, sortType); | ||
|
|
||
| List<ProductModel> content = queryFactory.selectFrom(product) | ||
| .where(where) | ||
| .orderBy(orderSpecifier) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()) | ||
| .fetch(); |
There was a problem hiding this comment.
동일 정렬값에서 페이지 순서가 고정되지 않는다.
현재 정렬이 단일 컬럼만 사용하므로 createdAt, price, likeCount 동률이 많은 순간 DB가 tie-break를 보장하지 않는다. 운영에서는 페이지 이동이나 무한 스크롤에서 상품이 중복되거나 누락되는 간헐적 이슈로 이어진다. 각 정렬에 id를 보조 정렬키로 항상 추가해 전체 순서를 고정하는 편이 안전하다. 수정 후에는 동일 createdAt, 동일 가격, 동일 likeCount 데이터를 여러 페이지에 배치하고 1페이지와 2페이지를 연속 조회해 중복·누락이 없는지 검증하는 통합 테스트를 추가하는 편이 좋다.
수정 예시
- OrderSpecifier<?> orderSpecifier = toOrderSpecifier(product, sortType);
+ OrderSpecifier<?>[] orderSpecifiers = toOrderSpecifiers(product, sortType);
List<ProductModel> content = queryFactory.selectFrom(product)
.where(where)
- .orderBy(orderSpecifier)
+ .orderBy(orderSpecifiers)
.offset(pageable.getOffset())
.limit(pageable.getPageSize())
.fetch();
@@
- private OrderSpecifier<?> toOrderSpecifier(QProductModel product, ProductSortType sortType) {
+ private OrderSpecifier<?>[] toOrderSpecifiers(QProductModel product, ProductSortType sortType) {
return switch (sortType) {
- case LATEST, CREATED_DESC -> product.createdAt.desc();
- case PRICE_ASC -> product.price.value.asc();
- case PRICE_DESC -> product.price.value.desc();
- case LIKES_DESC -> product.likeCount.desc();
+ case LATEST, CREATED_DESC -> new OrderSpecifier<?>[] { product.createdAt.desc(), product.id.desc() };
+ case PRICE_ASC -> new OrderSpecifier<?>[] { product.price.value.asc(), product.id.asc() };
+ case PRICE_DESC -> new OrderSpecifier<?>[] { product.price.value.desc(), product.id.desc() };
+ case LIKES_DESC -> new OrderSpecifier<?>[] { product.likeCount.desc(), product.id.desc() };
};
}As per coding guidelines, **/*Repository*.java: 정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다.
Also applies to: 78-84
🤖 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/ProductRepositoryImpl.java`
around lines 61 - 68, The query uses a single-column OrderSpecifier (built via
toOrderSpecifier) which allows non-deterministic ordering on ties; modify to
always append the primary key (product.id or ProductModel.id) as a secondary
OrderSpecifier so sorting is deterministic (update toOrderSpecifier to return
either a List<OrderSpecifier<?>> or have callers combine the primary
OrderSpecifier with an OrderSpecifier for id before calling
queryFactory.orderBy), change usages around
queryFactory.selectFrom(product).orderBy(...) in ProductRepositoryImpl to pass
both specifiers, and add an integration test that inserts multiple products with
identical createdAt/price/likeCount and asserts sequential page fetches (page 1
then page 2) contain no duplicates or gaps.
|
|
||
| @Operation(summary = "내 주문 목록 조회", description = "내 주문 목록을 조회합니다.") | ||
| ApiResponse<Page<OrderV1Dto.OrderSummaryResponse>> getMyOrders(MemberModel member, Pageable pageable); | ||
| ApiResponse<OrderV1Dto.OrderSummaryResponse> getMyOrders(MemberModel member, Pageable pageable); |
There was a problem hiding this comment.
API 스펙과 실제 엔드포인트 계약을 일치시켜야 한다
Line 17의 반환 타입/파라미터가 실제 컨트롤러와 다르다. 운영 관점에서 Swagger 스펙 기반 클라이언트 생성 시 잘못된 요청/응답 계약이 배포되어 호출 장애를 유발할 수 있다. 스펙 인터페이스를 실제 구현과 동일하게 맞춰야 한다.
추가 테스트로 OpenAPI 스냅샷(또는 contract) 테스트를 추가해 /api/v1/orders의 응답이 배열인지, startAt/endAt 쿼리 파라미터가 문서화되는지 검증해야 한다.
수정 예시 diff
import com.loopers.domain.member.MemberModel;
import com.loopers.interfaces.api.ApiResponse;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
-import org.springframework.data.domain.Page;
-import org.springframework.data.domain.Pageable;
+import java.time.LocalDate;
+import java.util.List;
`@Tag`(name = "Order V1 API", description = "고객 주문 API")
public interface OrderV1ApiSpec {
@@
- ApiResponse<OrderV1Dto.OrderSummaryResponse> getMyOrders(MemberModel member, Pageable pageable);
+ ApiResponse<List<OrderV1Dto.OrderSummaryResponse>> getMyOrders(
+ MemberModel member,
+ LocalDate startAt,
+ LocalDate endAt
+ );📝 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
| ApiResponse<OrderV1Dto.OrderSummaryResponse> getMyOrders(MemberModel member, Pageable pageable); | |
| ApiResponse<List<OrderV1Dto.OrderSummaryResponse>> getMyOrders( | |
| MemberModel member, | |
| LocalDate startAt, | |
| LocalDate endAt | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/order/OrderV1ApiSpec.java`
at line 17, Update the OrderV1ApiSpec#getMyOrders signature to exactly match the
controller for /api/v1/orders: change the return type and parameters of
getMyOrders (currently ApiResponse<OrderV1Dto.OrderSummaryResponse>
getMyOrders(MemberModel member, Pageable pageable)) to the controller's contract
(e.g., return a collection/array response type and include the startAt/endAt
query parameters instead of Pageable if the controller uses explicit query
params), so the interface signature and types (OrderV1ApiSpec, getMyOrders,
MemberModel, Pageable, OrderV1Dto.OrderSummaryResponse) are identical to the
implemented controller; then add an OpenAPI snapshot/contract test that requests
/api/v1/orders and asserts the response schema is an array and that startAt and
endAt are documented as query parameters.
Comment on lines
+2
to
+3
| -- 대량 데이터 시딩 SQL (MySQL 8.0) | ||
| -- 브랜드 100개, 상품 100,000개, 재고 100,000개 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the seed-data.sql file
fd seed-data.sqlRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 107
🏁 Script executed:
# First check file size
wc -l docs/sql/seed-data.sql
# Read the header (lines 1-10)
head -20 docs/sql/seed-data.sql
# Search for EXPLAIN ANALYZE in the file
rg "EXPLAIN ANALYZE" docs/sql/seed-data.sql -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 765
🏁 Script executed:
# Read lines 180-209 to see EXPLAIN ANALYZE usage
sed -n '180,209p' docs/sql/seed-data.sqlRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 658
🌐 Web query:
MySQL EXPLAIN ANALYZE minimum version support
💡 Result:
EXPLAIN ANALYZE is supported starting with MySQL Server 8.0.18 (it was introduced in that release). It executes the statement and reports actual runtime statistics, with output provided in TREE format (initially described for SELECT). [1][2]
Minimum version: 8.0.18. [1][2]
Sources:
[1] MySQL 8.0 “What Is New” (notes EXPLAIN ANALYZE implemented in 8.0.18) (docs.oracle.com)
[2] MySQL blog post announcing EXPLAIN ANALYZE with MySQL 8.0.18 (dev.mysql.com)
Citations:
- 1: https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/mysql-nutshell.html?utm_source=openai
- 2: https://dev.mysql.com/blog-archive/mysql-explain-analyze/?utm_source=openai
MySQL 버전 명시를 MySQL 8.0.18 이상으로 상향 조정해야 한다.
현재 파일 헤더는 MySQL 8.0으로만 지정되어 있으나, 184-209줄의 EXPLAIN ANALYZE는 MySQL 8.0.18부터 지원되는 기능이다. 8.0.0~8.0.17 버전에서는 구문 오류가 발생하므로, 운영 환경에서 스크립트 자체 결함으로 오인되어 불필요한 디버깅 시간이 낭비될 수 있다.
수정안:
- 헤더를 "MySQL 8.0.18 이상"으로 변경하거나
- EXPLAIN ANALYZE를 단순 EXPLAIN로 변경하여 호환성 확대
추가 테스트:
- 지원 최소 버전(8.0.18 선택 시)에서 모든 EXPLAIN ANALYZE 쿼리 실행 확인
- 스크립트 시작 시 SELECT VERSION() 출력 추가
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sql/seed-data.sql` around lines 2 - 3, Update the file header to state
the minimum MySQL version as "MySQL 8.0.18 or later" and/or replace every
EXPLAIN ANALYZE usage (search for the literal "EXPLAIN ANALYZE") with plain
EXPLAIN to maintain compatibility with < 8.0.18; additionally add a runtime
version check by inserting a SELECT VERSION() near the top of the script to make
it clear which server version is being used when running the seed script and
then verify all EXPLAIN ANALYZE queries (if you keep them) run successfully on
8.0.18+.
Comment on lines
+7
to
+10
| SET FOREIGN_KEY_CHECKS = 0; | ||
| TRUNCATE TABLE stock; | ||
| TRUNCATE TABLE product; | ||
| TRUNCATE TABLE brand; |
There was a problem hiding this comment.
하위 참조 테이블 정리가 빠져 있어 재시드 시 데이터가 오염된다.
product와 brand만 비우면 기존 좋아요 같은 하위 행이 남아, TRUNCATE 후 재사용된 상품 ID에 잘못 붙을 수 있다. 운영에서는 목록/상세 검증 결과가 왜곡되고, 좋아요 유니크 제약 검증도 신뢰하기 어렵다. product를 참조하는 하위 테이블을 부모보다 먼저 함께 비우거나, 최소한 이 스크립트가 빈 상태 전제에서만 실행되도록 사전 검사를 넣는 편이 안전하다. 추가로 시드 직후 이 스크립트가 생성하지 않는 product 하위 테이블의 건수가 0인지 확인하는 검증 쿼리를 넣어야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sql/seed-data.sql` around lines 7 - 10, 현재 스크립트는 SET FOREIGN_KEY_CHECKS
= 0; 이후 stock, product, brand만 TRUNCATE 하여 product를 참조하는 하위 테이블(예: 좋아요/likes,
리뷰/review, product_images 등)의 레코드가 남아 재시드 시 ID 재사용으로 데이터 오염이 발생할 수 있습니다; 수정 방법은
TRUNCATE 순서를 부모보다 먼저 하위 테이블을 모두 비우도록 변경하거나(하위 테이블 이름을 명시해 부모 TRUNCATE 전에 실행), 또는
TRUNCATE 실행 전 product를 참조하는 하위 테이블의 COUNT(*)가 0인지 확인하는 검증 쿼리를 추가하고 조건 불충분 시
스크립트를 중단하도록 하십시오; 또한 검증 실패 시 어떤 테이블에서 건수가 남아있는지 명확히 로그/출력하도록 하세요.
Comment on lines
+62
to
+71
| SET now_ts = NOW(6); | ||
| SET sql_text = ''; | ||
|
|
||
| WHILE i <= 100000 DO | ||
| -- 브랜드 순환 배정: 1~100 | ||
| SET brand_id_val = ((i - 1) % 100) + 1; | ||
| -- price: 1,000 ~ 1,000,000 | ||
| SET price_val = FLOOR(1000 + RAND() * 999001); | ||
| -- like_count: 0 ~ 10,000 | ||
| SET like_val = FLOOR(RAND() * 10001); |
There was a problem hiding this comment.
상품 시드 분포가 비재현적이라 성능 회귀를 신뢰하기 어렵다.
현재 price와 like_count는 실행마다 달라지고, created_at은 모든 상품이 동일하다. 이 상태에서는 PR 전후 인덱스 비교가 같은 데이터 위에서 이뤄지지 않고, 최신 정렬 쿼리는 사실상 동률 정렬만 측정하게 된다. RAND() 대신 i 기반의 결정적 분포를 사용하고, created_at도 TIMESTAMPADD로 행마다 다르게 생성해야 한다. 추가로 빈 DB에서 두 번 시드한 뒤 가격/좋아요 분포 체크섬이 동일한지와 COUNT(DISTINCT created_at)가 충분히 큰지 검증하는 쿼리를 넣어야 한다.
Also applies to: 79-86, 197-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sql/seed-data.sql` around lines 62 - 71, Seed randomness makes results
non-deterministic; replace RAND() usage by deterministic formulas based on loop
index i (e.g., set price_val = 1000 + ((i * 7919) % 999001) and like_val = (i *
9973) % 10001) and make created_at unique per row using TIMESTAMPADD (e.g.,
created_at = TIMESTAMPADD(SECOND, i, now_ts) or subtracting i to spread
timestamps); update assignments for price_val, like_val and created_at in the
loops (look for variables price_val, like_val, created_at and RAND() usage) and
add verification queries after seeding to assert distribution reproducibility
such as SELECT SUM(price) AS price_checksum, SUM(like_count) AS like_checksum
FROM products; and SELECT COUNT(DISTINCT created_at) FROM products; and run the
same seed twice in tests to ensure checksums and distinct created_at counts
match; apply the same deterministic changes where RAND() appears in the other
loop blocks referenced (lines ~79-86 and ~197-202).
Comment on lines
+12
to
+27
| log.warn("Cache GET failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | ||
| } | ||
|
|
||
| @Override | ||
| public void handleCachePutError(RuntimeException exception, Cache cache, Object key, Object value) { | ||
| log.warn("Cache PUT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | ||
| } | ||
|
|
||
| @Override | ||
| public void handleCacheEvictError(RuntimeException exception, Cache cache, Object key) { | ||
| log.warn("Cache EVICT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | ||
| } | ||
|
|
||
| @Override | ||
| public void handleCacheClearError(RuntimeException exception, Cache cache) { | ||
| log.warn("Cache CLEAR failed - cache: {}, error: {}", cache.getName(), exception.getMessage()); |
There was a problem hiding this comment.
캐시 장애 로그에서 원인 유실과 키 노출 위험이 동시에 존재한다
현재 로그는 exception.getMessage()만 남겨 장애 원인 추적이 어려우며, key 원문 출력으로 식별자 노출 가능성이 있다. 운영 장애 대응 시간과 보안 리스크를 동시에 키우는 패턴이다. 예외 객체를 그대로 로깅해 cause를 보존하고, key는 마스킹/해시 처리해 기록해야 한다. 추가로 로그 캡처 테스트를 통해 원문 key 미노출과 예외 스택 포함 여부를 검증해야 한다.
수정 예시
public class CustomCacheErrorHandler implements CacheErrorHandler {
+ private String cacheName(Cache cache) {
+ return cache == null ? "unknown" : cache.getName();
+ }
+
+ private String keyHash(Object key) {
+ return key == null ? "null" : Integer.toHexString(String.valueOf(key).hashCode());
+ }
`@Override`
public void handleCacheGetError(RuntimeException exception, Cache cache, Object key) {
- log.warn("Cache GET failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage());
+ log.warn("Cache GET failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception);
}
`@Override`
public void handleCachePutError(RuntimeException exception, Cache cache, Object key, Object value) {
- log.warn("Cache PUT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage());
+ log.warn("Cache PUT failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception);
}
`@Override`
public void handleCacheEvictError(RuntimeException exception, Cache cache, Object key) {
- log.warn("Cache EVICT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage());
+ log.warn("Cache EVICT failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception);
}
`@Override`
public void handleCacheClearError(RuntimeException exception, Cache cache) {
- log.warn("Cache CLEAR failed - cache: {}, error: {}", cache.getName(), exception.getMessage());
+ log.warn("Cache CLEAR failed - cache: {}", cacheName(cache), exception);
}
}As per coding guidelines **/*.java: 예외 처리 시 cause를 보존하고, 로깅 시 민감정보 노출 가능성을 점검해야 한다.
📝 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
| log.warn("Cache GET failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | |
| } | |
| @Override | |
| public void handleCachePutError(RuntimeException exception, Cache cache, Object key, Object value) { | |
| log.warn("Cache PUT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | |
| } | |
| @Override | |
| public void handleCacheEvictError(RuntimeException exception, Cache cache, Object key) { | |
| log.warn("Cache EVICT failed - cache: {}, key: {}, error: {}", cache.getName(), key, exception.getMessage()); | |
| } | |
| @Override | |
| public void handleCacheClearError(RuntimeException exception, Cache cache) { | |
| log.warn("Cache CLEAR failed - cache: {}, error: {}", cache.getName(), exception.getMessage()); | |
| public class CustomCacheErrorHandler implements CacheErrorHandler { | |
| private static final Logger log = LoggerFactory.getLogger(CustomCacheErrorHandler.class); | |
| private String cacheName(Cache cache) { | |
| return cache == null ? "unknown" : cache.getName(); | |
| } | |
| private String keyHash(Object key) { | |
| return key == null ? "null" : Integer.toHexString(String.valueOf(key).hashCode()); | |
| } | |
| `@Override` | |
| public void handleCacheGetError(RuntimeException exception, Cache cache, Object key) { | |
| log.warn("Cache GET failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception); | |
| } | |
| `@Override` | |
| public void handleCachePutError(RuntimeException exception, Cache cache, Object key, Object value) { | |
| log.warn("Cache PUT failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception); | |
| } | |
| `@Override` | |
| public void handleCacheEvictError(RuntimeException exception, Cache cache, Object key) { | |
| log.warn("Cache EVICT failed - cache: {}, keyHash: {}", cacheName(cache), keyHash(key), exception); | |
| } | |
| `@Override` | |
| public void handleCacheClearError(RuntimeException exception, Cache cache) { | |
| log.warn("Cache CLEAR failed - cache: {}", cacheName(cache), exception); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/redis/src/main/java/com/loopers/config/redis/CustomCacheErrorHandler.java`
around lines 12 - 27, 로그에서 예외 원인과 키 노출 문제가 있으니 CustomCacheErrorHandler의
handleCacheGetError, handleCachePutError, handleCacheEvictError,
handleCacheClearError를 수정해 예외 메시지 대신 예외 객체 전체를 로거에 전달해(cause/stack 보존) 기록하고, 원본
key는 그대로 출력하지 않도록 마스킹 또는 해시하는 유틸(예: maskKey(String) 또는 hashKey(Object))를 적용해
기록하도록 변경하고, 해당 변경을 검증하는 로그 캡처 테스트를 추가해 원문 key 미노출 및 예외 스택 포함 여부를 확인하세요.
Comment on lines
+29
to
+37
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| objectMapper.registerModule(new JavaTimeModule()); | ||
| objectMapper.activateDefaultTyping( | ||
| LaissezFaireSubTypeValidator.instance, | ||
| ObjectMapper.DefaultTyping.EVERYTHING, | ||
| JsonTypeInfo.As.PROPERTY | ||
| ); | ||
|
|
||
| GenericJackson2JsonRedisSerializer jsonSerializer = new GenericJackson2JsonRedisSerializer(objectMapper); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*redis*" -name "RedisCacheConfig.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 161
🏁 Script executed:
cat -n modules/redis/src/main/java/com/loopers/config/redis/RedisCacheConfig.java | head -70Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3176
🏁 Script executed:
# Look for ObjectMapper bean definitions in the application
fd -t f "\.java$" | xargs grep -l "@Bean\|@Configuration" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 837
🏁 Script executed:
# Search for cache configuration and ObjectMapper usage
rg "ObjectMapper|activateDefaultTyping|LaissezFaireSubTypeValidator" -A 3 -B 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6935
🏁 Script executed:
cat -n supports/jackson/src/main/java/com/loopers/config/jackson/JacksonConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2377
🏁 Script executed:
# Look for CustomCacheErrorHandler to understand fallback mechanism
rg "CustomCacheErrorHandler" -A 10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 10283
🏁 Script executed:
# Search for existing cache tests
fd -t f ".*Test\.java$" | xargs grep -l "RedisCacheConfig\|cacheManager\|GenericJackson2JsonRedisSerializer" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 349
🏁 Script executed:
# Search for ProductDetail entity/DTO
rg "class ProductDetail|record ProductDetail" -A 5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1514
🏁 Script executed:
# Check if ObjectMapper is injected elsewhere in cache configs
rg "cacheManager.*ObjectMapper" -B 3 -A 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/config/RedisCacheConfigTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1735
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2814
🏁 Script executed:
# Check if tests validate deserialization with unexpected type metadata
rg "defaultTyping|polymorphic|EVERYTHING|LaissezFaire" --type java -A 3 -B 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1894
🏁 Script executed:
# Check if ObjectMapper is available as a bean anywhere
rg "@Bean.*ObjectMapper|ObjectMapper.*@Bean" -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Verify if ObjectMapper is available as a bean or if Jackson2ObjectMapperBuilderCustomizer provides automatic bean creation
rg "ObjectMapper" supports/jackson/src/main/java/com/loopers/config/jackson/JacksonConfig.java -A 5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 626
🏁 Script executed:
# Check how Kafka and other modules inject ObjectMapper
rg "ObjectMapper.*objectMapper\)" modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java -B 3 -A 1Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 256
🏁 Script executed:
# Look at the rest of ProductCacheIntegrationTest to see what cache tests actually exist
cat -n apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.java | tail -80Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3491
전역 polymorphic typing 설정으로 캐시 역직렬화 범위가 과도하게 넓어진다.
LaissezFaireSubTypeValidator.instance와 DefaultTyping.EVERYTHING 조합은 캐시 payload의 타입 메타데이터를 사실상 무제한 신뢰한다. 운영 환경에서 Redis 데이터 오염이나 공유 인프라 사고 시 예상하지 않은 타입이 역직렬화되어 장애 반경과 보안 노출면이 커진다. 현재 CustomCacheErrorHandler가 캐시 읽기 실패를 삼키고 로그 기록하므로 기본적인 보호는 제공하지만, 악의적이거나 잘못된 type metadata에 대한 방어는 불충분하다.
개선 방안: 애플리케이션 ObjectMapper bean을 주입받아 (Kafka 모듈처럼 생성자 주입으로 가능) 중앙화된 Jackson 설정을 재사용하고, DefaultTyping.NON_FINAL 또는 캐시 대상 DTO별 구체 serializer로 범위를 좁힌다. 수정 후에는 다음 테스트를 추가한다: (1) ProductDetail round-trip 직렬화 검증, (2) 예상 밖 type metadata 역직렬화 실패 시 캐시 조회 예외 처리 및 DB fallback 동작 확인.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/redis/src/main/java/com/loopers/config/redis/RedisCacheConfig.java`
around lines 29 - 37, The current Redis cache config creates a local
ObjectMapper with LaissezFaireSubTypeValidator and DefaultTyping.EVERYTHING
which over-trusts type metadata; change RedisCacheConfig to accept the
application's Jackson ObjectMapper bean via constructor injection (reuse global
Jackson config used elsewhere) and remove activateDefaultTyping usage, or at
minimum switch to ObjectMapper.DefaultTyping.NON_FINAL; alternatively register
specific GenericJackson2JsonRedisSerializer instances per cache/DTO (e.g.,
ProductDetail) rather than a global EVERYTHING policy; update tests to cover
ProductDetail round-trip serialization and ensure cache read failures due to
unexpected type metadata trigger the CustomCacheErrorHandler fallback to DB.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
상품 상세 조회 Redis 캐시 적용
📌 Summary
🧭 Context & Decision
문제 정의
ProductFacade.getProduct()가 매번 ProductService + BrandService + StockService를 호출하여 DB 쿼리 3회 발생선택지와 결정
1. 캐시 적용 레이어
2. 직렬화 방식
@class타입 정보 포함3. 캐시 무효화 전략
4. 목록 캐시
🏗️ Design Overview
변경 범위
영향 받는 모듈/도메인:
modules/redis,apps/commerce-api(product, like, brand, order)신규 추가:
RedisCacheConfig.javaCustomCacheErrorHandler.javaProductCacheIntegrationTest.javaLikeTransactionCacheTest.javaRedisCacheConfigTest.javaCustomCacheErrorHandlerTest.java.http/cache-test.http제거/대체:
ProductListCacheService.java+ 테스트interfaces/api/brand/admin/패키지ConflictingBeanDefinitioninterfaces/api/product/admin/패키지interfaces/api/order/admin/패키지interfaces/auth/패키지interfaces/api/auth/와 중복주요 컴포넌트 책임
RedisCacheConfigCustomCacheErrorHandlerProductFacade@Cacheable로 상품 상세 캐시 적용,@CacheEvict로 수정/삭제 시 캐시 무효화LikeTransactionServiceproductDetail캐시를 수동 evict캐시 설정 요약
productDetail@Cacheable/@CacheEvictdefaultRedisCacheConfig🔁 Flow Diagram
Main Flow — 상품 상세 조회 (캐시 미스 → 히트)
sequenceDiagram autonumber participant Client participant Facade as ProductFacade participant Redis participant DB Note over Client,DB: 첫 번째 조회 (캐시 미스) Client->>Facade: GET /api/v1/products/42 Facade->>Redis: get("productDetail::42") Redis-->>Facade: null (미스) Facade->>DB: Product + Brand + Stock 조회 (쿼리 3회) DB-->>Facade: ProductDetail Facade->>Redis: set("productDetail::42", ProductDetail, TTL=10분) Facade-->>Client: 200 OK + ProductDetail Note over Client,DB: 두 번째 조회 (캐시 히트) Client->>Facade: GET /api/v1/products/42 Facade->>Redis: get("productDetail::42") Redis-->>Facade: ProductDetail (히트) Note over Facade,DB: DB 접근 없음 (쿼리 0회) Facade-->>Client: 200 OK + ProductDetail캐시 무효화 Flow — 수정/삭제
sequenceDiagram autonumber participant Admin participant Facade as ProductFacade participant DB participant Redis Admin->>Facade: PUT /api-admin/v1/products/42 Facade->>DB: 상품 수정 DB-->>Facade: OK Facade->>Redis: evict("productDetail::42") Redis-->>Facade: 삭제 완료 Facade-->>Admin: 200 OK Note over Admin,Redis: 다음 조회 시 캐시 미스 → DB에서 새 데이터 가져옴캐시 무효화 Flow — 좋아요
sequenceDiagram autonumber participant User participant LikeTxn as LikeTransactionService participant DB participant Redis User->>LikeTxn: POST /api/v1/likes (상품 42) LikeTxn->>DB: 좋아요 INSERT + like_count += 1 DB-->>LikeTxn: OK LikeTxn->>Redis: CacheManager.evict("productDetail", 42) Redis-->>LikeTxn: 삭제 완료 LikeTxn-->>User: 200 OK Note over User,Redis: 캐시의 likeCount가 stale → evict → 다음 조회 시 새 값예외 Flow — Redis 장애
sequenceDiagram autonumber participant Client participant Facade as ProductFacade participant Redis participant Handler as CacheErrorHandler participant DB Client->>Facade: GET /api/v1/products/42 Facade->>Redis: get("productDetail::42") Redis--xFacade: ❌ Connection Refused Facade->>Handler: handleCacheGetError() Handler-->>Handler: log.warn() + 예외 삼킴 Facade->>DB: Product + Brand + Stock 조회 (정상 fallback) DB-->>Facade: ProductDetail Facade-->>Client: 200 OK (느리지만 정상 동작)상품 목록 조회 쿼리 최적화
📌 Summary
🧭 Context & Decision
문제 정의
ProductFacade.getProducts()에서 페이지 내 상품마다getBrandName()+getByProductId()개별 호출 → 상품 20개 기준 쿼리 41회(1 + 20 + 20)선택지와 결정
1. 동적 쿼리 구현 방식
2. N+1 해결 전략
3. 인덱스 설계
idx_product_brand_deleted_likebrand_id, deleted_at, like_countidx_product_deleted_likedeleted_at, like_countidx_product_deleted_createddeleted_at, created_atidx_product_deleted_pricedeleted_at, price4. 좋아요 중복 방지
🏗️ Design Overview
변경 범위
영향 받는 모듈/도메인:
apps/commerce-api(product, brand, stock, like)주요 변경:
ProductModel.java@Table(indexes = ...)4개 복합 인덱스 추가LikeModel.java@UniqueConstraint추가 (user_id, product_id)ProductRepository.javafindAll(Long brandId, Pageable, ProductSortType)시그니처 추가ProductRepositoryImpl.javaBooleanBuilder+JPAQueryFactory동적 쿼리 구현ProductService.javagetProducts(brandId, sortType, pageable)/getProductsForAdmin(brandId, pageable)추가BrandService.javagetByIds(List<Long>)배치 조회 메서드 추가StockService.javagetProductId메서드 참조 수정ProductFacade.java주요 컴포넌트 책임
ProductRepositoryImplProductFacadeBrandService.getByIds()findAllByIdInIN 쿼리 후Map<Long, BrandModel>반환StockService.getByProductIds()findAllByProductIdInIN 쿼리 후Map<Long, StockModel>반환🔁 Flow Diagram
Main Flow — 상품 목록 조회 (N+1 해결 후)
sequenceDiagram autonumber participant Client participant Facade as ProductFacade participant ProductRepo as ProductRepositoryImpl participant BrandSvc as BrandService participant StockSvc as StockService participant DB Client->>Facade: GET /api/v1/products?brandId=1&sort=LIKES_DESC Facade->>ProductRepo: findAll(brandId=1, pageable, LIKES_DESC) Note over ProductRepo,DB: QueryDSL 동적 쿼리 ProductRepo->>DB: SELECT * FROM product<br/>WHERE deleted_at IS NULL AND brand_id = 1<br/>ORDER BY like_count DESC<br/>LIMIT 20 OFFSET 0 DB-->>ProductRepo: List<ProductModel> (1회) ProductRepo-->>Facade: Page<ProductModel> Note over Facade: brandIds, productIds 수집 Facade->>BrandSvc: getByIds([1]) BrandSvc->>DB: SELECT * FROM brand WHERE id IN (1) DB-->>BrandSvc: Map<Long, BrandModel> (2회) Facade->>StockSvc: getByProductIds([101, 102, ...]) StockSvc->>DB: SELECT * FROM stock WHERE product_id IN (101, 102, ...) DB-->>StockSvc: Map<Long, StockModel> (3회) Note over Facade: Map으로 O(1) 매핑 Facade-->>Client: 200 OK + Page<ProductDetail> Note over Client,DB: 총 쿼리 3회 (기존: 1 + N + N = 41회)QueryDSL 동적 쿼리 구조
flowchart TD A[요청: brandId, sortType, pageable] --> B{BooleanBuilder} B --> C[deletedAt IS NULL ← 고정] B --> D{brandId != null?} D -->|YES| E[AND brand_id = ?] D -->|NO| F[조건 없음] A --> G{sortType} G -->|LATEST / CREATED_DESC| H[ORDER BY created_at DESC] G -->|PRICE_ASC| I[ORDER BY price ASC] G -->|PRICE_DESC| J[ORDER BY price DESC] G -->|LIKES_DESC| K[ORDER BY like_count DESC] C --> L[JPAQueryFactory.selectFrom] E --> L F --> L H --> L I --> L J --> L K --> L L --> M[.offset + .limit → 페이징] M --> N[PageImpl 반환]변경 개요
Redis Cache-Aside 패턴과 QueryDSL 기반 동적 쿼리, 복합 인덱스를 통해 상품 조회 성능을 종합적으로 최적화합니다.
핵심 변경점
productDetail캐시명으로 10분 TTL로 캐싱; update/delete/updateStock/like/unlike 시 @CacheEvict로 무효화리스크 및 주의사항
Breaking Change 위험: 기존 admin 엔드포인트 3개 완전 제거(BrandAdminV1Controller, ProductAdminV1Controller, OrderAdminV1Controller) 및 해당 DTO 패키지 재구성으로 기존 API 클라이언트 영향 가능. 테스트 파일 52줄이 임포트 경로 변경됨 → 실제 운영 환경 admin API 클라이언트 의존성 확인 필요. Redis 장애 시 CustomCacheErrorHandler가 예외를 삼켜 DB 폴백하는데, 캐시 무효화 예외도 무시되는지 확인 필요.
테스트 및 검증 방법