[volume-5] 상품 목록 조회 성능, 좋아요 수 정렬 구조 개선 및 인덱스,캐시 적용#197
Open
jsj1215 wants to merge 6 commits intoLoopers-dev-lab:jsj1215from
Open
[volume-5] 상품 목록 조회 성능, 좋아요 수 정렬 구조 개선 및 인덱스,캐시 적용#197jsj1215 wants to merge 6 commits intoLoopers-dev-lab:jsj1215from
jsj1215 wants to merge 6 commits intoLoopers-dev-lab:jsj1215from
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- LikeService에서 ProductRepository 의존 제거, 반환타입 Like → boolean 변경 - 좋아요 수 증감 책임을 상위 레이어(Facade)로 이동 - ProductLikeSummary 엔티티 및 refreshAll() 배치 쿼리 추가 - 좋아요 관련 테스트 코드 업데이트 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ProductCacheStore 인터페이스 및 Redis/Caffeine 구현체 추가 - Cache-Aside 패턴으로 상품 목록/상세 조회 캐시 적용 - 캐시 복원용 restoreFromCache 팩토리 메서드 추가 (Product, Brand, ProductOption, BaseEntity) - 애플리케이션 기동 시 캐시 웜업 (ProductCacheWarmUp) - 상품 수정/삭제 시 캐시 무효화 (evictDetail) - Materialized View 기반 상품 검색 쿼리 추가 - 상품 테이블 인덱스 및 FK NO_CONSTRAINT 최적화 - 캐시 성능 비교 및 E2E 테스트 추가 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 캐시 전략별 상품 조회 엔드포인트 추가 (Redis, Local Cache, No Cache)
- 좋아요 취소 HTTP 메서드 PUT → DELETE 변경
- 내 좋아요 목록 경로 /users/{userId}/likes → /me/likes 변경
- OrderItem FK NO_CONSTRAINT 적용
- 상품 검색 통합 테스트 추가
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ProductService 좋아요 수 증감 단위 테스트 추가 (음수 방지 경로 포함) - LikeService refreshLikeSummary 단위 테스트 추가 - ProductService 좋아요 수 증감 통합 테스트 추가 (DB 레벨 음수 방지 검증) - 좋아요 등록/취소 후 likeCount 반영 확인 E2E 테스트 추가 - unlike 엔드포인트 HTTP 메서드를 PUT에서 DELETE로 수정 (컨트롤러와 일치) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRedis 및 Caffeine 로컬 캐시를 통한 상품 조회 캐싱 기능을 도입하고, 좋아요 기능을 리팩토링하여 LikeService 반환값을 boolean으로 변경하며 ProductLikeSummary 물화 뷰를 추가한다. 상품 캐시 무효화, 다양한 캐시 전략 엔드포인트, 광범위한 성능 및 통합 테스트를 함께 포함한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as ProductV1Controller
participant Facade as ProductFacade
participant RedisCache as ProductRedisCacheStore
participant LocalCache as ProductLocalCacheStore
participant Service as ProductService
participant DB as Database
rect rgb(200, 150, 100, 0.5)
Note over Client,DB: Redis Cache-Aside Pattern
Client->>API: GET /api/v1/products
API->>Facade: getProducts(condition, pageable)
Facade->>RedisCache: getSearch(condition, pageable)
alt Cache Hit
RedisCache-->>Facade: Optional<Page<Product>>
Facade-->>API: Page<ProductInfo>
API-->>Client: 200 OK (cached)
else Cache Miss
RedisCache-->>Facade: Optional.empty()
Facade->>Service: search(condition, pageable)
Service->>DB: query products
DB-->>Service: Page<Product>
Service-->>Facade: Page<Product>
Facade->>RedisCache: setSearch(condition, pageable, page)
RedisCache-->>Facade: void
Facade-->>API: Page<ProductInfo>
API-->>Client: 200 OK (from DB)
end
end
rect rgb(100, 200, 150, 0.5)
Note over Client,DB: Local Cache-Aside Pattern
Client->>API: GET /api/v1/products/local-cache
API->>Facade: getProductsWithLocalCache(condition, pageable)
Facade->>LocalCache: getSearch(condition, pageable)
alt Cache Hit
LocalCache-->>Facade: Optional<Page<Product>>
else Cache Miss
LocalCache-->>Facade: Optional.empty()
Facade->>Service: search(condition, pageable)
Service->>DB: query products
DB-->>Service: Page<Product>
Service-->>Facade: Page<Product>
Facade->>LocalCache: setSearch(condition, pageable, page)
LocalCache-->>Facade: void
end
Facade-->>API: Page<ProductInfo>
API-->>Client: 200 OK
end
rect rgb(150, 100, 200, 0.5)
Note over Client,DB: Like/Unlike with Cache Invalidation
Client->>API: POST /api/v1/products/{id}/likes
API->>Facade: like(memberId, productId)
Facade->>Service: findById(productId)
Service->>DB: query product
DB-->>Service: Product
Service-->>Facade: Product
Facade->>LikeService: like(memberId, productId)
LikeService->>DB: save like record
DB-->>LikeService: boolean (changed?)
LikeService-->>Facade: true/false
alt If Changed (true)
Facade->>Service: incrementLikeCount(productId)
Service->>DB: UPDATE likeCount + 1
DB-->>Service: void
Facade->>RedisCache: evictDetail(productId)
RedisCache-->>Facade: void
end
Facade-->>API: void
API-->>Client: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 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 for PR comments
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 |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (19)
modules/jpa/src/main/java/com/loopers/domain/BaseEntity.java-78-82 (1)
78-82:⚠️ Potential issue | 🟠 Majornull 파라미터 처리 시 DB 제약 위반 가능성이 있다.
createdAt이 null로 전달되면updatedAt도 null이 되어@Column(nullable = false)제약과 충돌한다. Brand.restoreFromCache에서 실제로restoreBase(id, null)을 호출하고 있다.운영 위험:
- 복원된 엔티티가 dirty checking으로 flush되면 NOT NULL 제약 위반으로 예외 발생
- 복원 엔티티를 merge/persist 시도 시 런타임 오류
- 장애 상황에서 원인 추적 어려움
수정안:
방어적 처리 추가
protected void restoreBase(Long id, ZonedDateTime createdAt) { + if (id == null) { + throw new IllegalArgumentException("id must not be null for cache restoration"); + } this.id = id; this.createdAt = createdAt; - this.updatedAt = createdAt; + this.updatedAt = createdAt != null ? createdAt : ZonedDateTime.now(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/jpa/src/main/java/com/loopers/domain/BaseEntity.java` around lines 78 - 82, The restoreBase method in BaseEntity allows a null createdAt (Brand.restoreFromCache calls restoreBase(id, null)), which can violate `@Column`(nullable = false) on createdAt/updatedAt; update BaseEntity.restoreBase(Long id, ZonedDateTime createdAt) to defensively handle null createdAt by assigning a non-null timestamp (e.g., ZonedDateTime.now()) when createdAt is null, then set this.updatedAt = this.createdAt and this.id = id so restored entities never leave createdAt/updatedAt as null; keep method name restoreBase and ensure Brand.restoreFromCache usage remains valid.apps/commerce-api/src/main/java/com/loopers/domain/brand/Brand.java-65-69 (1)
65-69:⚠️ Potential issue | 🟠 Major캐시 복원 시 Brand 상태(status)가 누락되어 비즈니스 로직 오류가 발생할 수 있다.
현재 구현은
status를 복원하지 않아 원래 ACTIVE 상태의 Brand도 PENDING으로 복원된다.brand.isActive()검증이 필요한 로직에서 오동작할 수 있다.운영 위험:
- 캐시에서 복원된 Brand를 사용하는 상품 조회 시 브랜드 활성화 검증 실패
- 캐시 hit/miss 여부에 따라 동작이 달라지는 비결정적 버그 발생
수정안:
status 파라미터 추가
- public static Brand restoreFromCache(Long id, String name) { - Brand brand = new Brand(name, ""); + public static Brand restoreFromCache(Long id, String name, BrandStatus status) { + Brand brand = new Brand(name, ""); + brand.status = status; brand.restoreBase(id, null); return brand; }🤖 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/brand/Brand.java` around lines 65 - 69, The restoreFromCache method currently omits restoring the Brand status, causing restored brands to default to PENDING; update Brand.restoreFromCache to accept a status parameter (e.g., BrandStatus or String status) and set the instance status after construction (or call an existing setter) before calling restoreBase so that brand.isActive() behaves correctly; reference the Brand.restoreFromCache method and the Brand class constructor/restoreBase to locate where to add the new parameter and assignment.apps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.java-22-23 (1)
22-23:⚠️ Potential issue | 🟠 MajorOrderItem → Order 외래키 제약 제거의 정당성이 없다.
캐시 복원 시나리오는 Product, Brand, ProductOption에만 적용되며, OrderItem은 캐시 복원 대상이 아니다. Order 엔티티에 CascadeType.ALL과 orphanRemoval=true가 설정되어 있어도, 이는 JPA 수준의 정합성만 보장하고 DB 수준의 참조 무결성은 보호하지 않는다. NO_CONSTRAINT 제약을 제거하면 다음과 같은 운영 위험이 발생한다:
- 직접 DB 작업(원본 SQL, 저장 프로시저)을 통한 orphan OrderItem 레코드 생성
- 다른 서비스가 동일 DB에 접근할 때 잘못된 order_id 삽입 방지 불가
- 장애 상황에서 데이터 정합성 추적 어려움
Product→Brand의 NO_CONSTRAINT는 캐시 복원 특성 때문에 정당하지만, OrderItem→Order는 그러한 근거가 없다. 외래키 제약을 유지하거나, 의도적 설계라면 코드 주석으로 명확히 기록해야 한다.
🤖 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/order/OrderItem.java` around lines 22 - 23, The OrderItem→Order mapping currently disables the DB foreign-key via foreignKey = `@ForeignKey`(ConstraintMode.NO_CONSTRAINT) in the OrderItem entity; either restore the DB-level FK by removing the foreignKey override (letting JPA generate the FK) or explicitly reintroduce a proper foreignKey declaration so the DB enforces referential integrity for the OrderItem.order relationship, and if disabling it was intentional, add a clear code comment on the OrderItem class and the `@JoinColumn` specifying the precise, documented rationale (including why OrderItem is excluded from cache-restore concerns) and any compensating controls.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java-108-117 (1)
108-117:⚠️ Potential issue | 🟠 Major0건 업데이트를 경고로만 삼키면
likeCount정합성이 깨진다.
incrementLikeCount()는 update count를 버리고,decrementLikeCount()도0건을 경고로만 처리한다.updatedRows == 0은 "이미 0"뿐 아니라 대상 상품 없음/삭제됨도 포함할 수 있는데, 지금 구조에서는 상위 유스케이스가 롤백이나 보상 처리를 할 수 없다. 이번 PR에서likeCount가 정렬 기준이므로 이런 누락은 곧 목록 순서와 응답 숫자 오류로 이어진다. 두 메서드 모두 결과를 호출자에게 반환하거나CoreException으로 승격해 실패를 상위 트랜잭션에 전파하고, 로그 메시지도 원인을 단정하지 않도록 바꾸는 편이 안전하다. 추가로0건 업데이트가 발생하면 좋아요 등록/취소 유스케이스가 실패 또는 보상 처리되는 통합 테스트를 넣는 것이 좋다.Based on learnings: commerce-api 모듈은 오류를
CoreException→ApiControllerAdvice경로로 통일한다.🤖 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/ProductService.java` around lines 108 - 117, The incrementLikeCount and decrementLikeCount methods currently swallow update counts which can hide missing-product or concurrency failures; change them to propagate failure to the caller by returning the update result or throwing the module's CoreException so the upper transaction/compensation can react. Specifically, modify incrementLikeCount(productId) and decrementLikeCount(productId) (and their calls to productRepository.incrementLikeCount/decrementLikeCount) to either (a) return the int updatedRows to the caller or (b) throw a CoreException when updatedRows == 0, and update the log message in decrementLikeCount to avoid assuming the cause (e.g., "like count update affected 0 rows for productId={}"). Ensure the thrown exception flows through the existing CoreException → ApiControllerAdvice path so callers and integration tests can assert failure/compensation behavior.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductCacheStore.java-15-31 (1)
15-31:⚠️ Potential issue | 🟠 Major검색 캐시 무효화 경로가 없어 비노출·삭제 상품이 TTL 동안 목록에 남을 수 있다.
evictDetail()만 있고 검색 캐시를 비울 수단이 없어, 상품 삭제나displayYn/status변경 직후에도 이미 캐시된 0~2페이지가 최대 TTL 동안 그대로 반환된다.likeCount는 정책적으로 eventual consistency를 허용하더라도, 비노출/삭제 상품이 공개 목록에 남는 것은 운영 이슈가 된다. 검색 캐시도 namespace version bump나 bulk eviction API를 Port에 추가하고, 관리자 숨김/삭제 후 첫 페이지에서 해당 상품이 즉시 사라지는 E2E 테스트를 넣는 편이 안전하다.🤖 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/ProductCacheStore.java` around lines 15 - 31, The ProductCacheStore currently only exposes evictDetail(...) so search caches set via setSearch(...) (and returned by getSearch(...) for ProductSearchCondition + Pageable) can remain stale after deletions or display/status changes; add a search-cache eviction mechanism to the Port: introduce a method like evictSearch(ProductSearchCondition condition) and/or evictSearchNamespaceVersion(ProductSearchCondition condition) or a bulkEvictSearchByProductId(Long productId) to invalidate cached pages affected by a product change, implement calls to this new API from the admin hide/delete and product update flows, and add an E2E that hides/deletes a product and asserts the first search page no longer contains it immediately (use ProductCacheStore.getSearch/setSearch, ProductSearchCondition and Pageable to locate affected entries).apps/commerce-api/src/test/java/com/loopers/domain/product/ProductSearchIntegrationTest.java-45-55 (1)
45-55:⚠️ Potential issue | 🟠 Major10만 건 시드를 공용 DB에 주입하는 방식은 테스트 격리를 깨기 쉽다.
@SpringBootTest에서BeforeAll로 대량 데이터를 넣고AfterAll에만 정리하면, 이 클래스가 실패하거나 병렬 실행될 때 다른 통합 테스트가 같은 DB 상태를 공유한다. CI에서는 실행 순서에 따라 원인 파악이 어려운 플래키 테스트가 되고, 일반 회귀 테스트도 10만 건 적재 비용을 계속 부담하게 된다. 이 케이스는@Tag("performance")나 전용 프로필로 분리하고, 최소한 클래스 시작 전에 정리 후 시드를 주입해 고립시키는 편이 안전하다. 추가로 랜덤 클래스 순서나 병렬 실행에서도 독립적으로 통과하는지 검증 전략을 두는 것이 좋다.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/domain/product/ProductSearchIntegrationTest.java` around lines 45 - 55, The test injects a large shared seed in ProductSearchIntegrationTest via `@BeforeAll` and only cleans up in `@AfterAll`, which breaks isolation and causes flakiness; modify ProductSearchIntegrationTest so setUp and tearDown enforce isolation by cleaning the DB before loading seeds (call databaseCleanUp.truncateAllTables or equivalent at start of setUp), or move heavy seed loading out of default integration tests by annotating the class with `@Tag`("performance") or using a dedicated test profile, and consider loading the 100k records into an isolated resource (Testcontainers or a dedicated DB) or into `@BeforeEach` with transactional rollback to avoid cross-test leakage; ensure methods referenced are setUp(), tearDown(), and the class-level annotations (e.g., `@BeforeAll/`@AfterAll) are updated accordingly so tests remain independent under parallel or randomized execution.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java-164-180 (1)
164-180:⚠️ Potential issue | 🟠 Major동률 tie-breaker가 없어 페이지 경계가 흔들린다.
LIKE_DESC에서mvLikeCount.desc()하나만 사용하면 같은 likeCount를 가진 상품들의 상대 순서가 비결정적이다. 이 상태에서는 페이지 0/1을 연속 조회할 때 중복/누락이 생길 수 있고, 캐시가 그 불안정한 페이지를 그대로 고정할 수 있다. 고유한 2차 정렬 키를 추가해 순서를 고정하고, 동일 likeCount 다건 데이터셋에서 페이지 간 overlap이 없고 반복 호출 결과도 안정적인지 통합 테스트를 넣는 편이 안전하다.수정 예시
- OrderSpecifier<?> orderSpecifier = switch (condition.sort()) { + OrderSpecifier<?> primaryOrder = switch (condition.sort()) { case PRICE_ASC -> product.price.asc(); case LIKE_DESC -> mvLikeCount.desc(); default -> product.createdAt.desc(); }; @@ - .orderBy(orderSpecifier) + .orderBy(primaryOrder, product.id.desc())🤖 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 164 - 180, The LIKE_DESC branch uses only mvLikeCount.desc() causing non-deterministic ties; make ordering deterministic by adding stable secondary keys (e.g., product.createdAt.desc() then product.id.asc()) when condition.sort() == LIKE_DESC: update the switch/OrderSpecifier logic for LIKE_DESC to include the tie-breakers and ensure the final query.orderBy(...) includes those additional OrderSpecifier instances (mvLikeCount.desc(), product.createdAt.desc(), product.id.asc()) so pagination is stable and repeatable.apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductRedisCacheApiE2ETest.java-72-76 (1)
72-76:⚠️ Potential issue | 🟠 Major첫 테스트가 워밍업된 Redis 상태에서 시작될 수 있다.
Line 72-76은
@AfterEach만 정리해서, PR에 포함된 캐시 warm-up이 첫 테스트 시작 시점의 Redis 키를 남길 수 있다. 이 상태에서 Line 168-191, 263-266 같은 개수/빈 집합 단언은 첫 실행에서 플래키해지므로,@BeforeEach에서도redisCleanUp.truncateAll()을 호출해 각 테스트를 빈 캐시에서 시작시키는 편이 안전하다. 추가 테스트로 클래스의 첫 번째 테스트만 단독 실행해도 동일하게 통과하는지 확인해야 한다.As per coding guidelines, "통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다."
Also applies to: 167-191, 263-266
🤖 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/ProductRedisCacheApiE2ETest.java` around lines 72 - 76, The tests can start with warmed Redis keys because cleanup runs only in tearDown; modify ProductRedisCacheApiE2ETest to also call redisCleanUp.truncateAll() in a `@BeforeEach` setup method so every test begins with an empty cache (mirror the existing tearDown logic that calls databaseCleanUp.truncateAll() and redisCleanUp.truncateAll()); ensure the new `@BeforeEach` is applied before tests that make assertions around empty sets/counts (see assertions around lines referenced for context) and then run the class’s first test standalone to confirm no flakiness.apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.java-319-344 (1)
319-344:⚠️ Potential issue | 🟠 Major좋아요/취소 응답을 검증하지 않아 거짓 양성이 가능하다.
Line 320-330에서 POST와 DELETE 결과를 확인하지 않으면, 두 호출이 모두 실패해도 초기값 0 때문에 마지막 단언이 그대로 통과할 수 있다. 운영 관점에서는 인증이나 라우팅 회귀가 숨어버리므로, 두 응답의 HTTP 200을 먼저 단언하고 중간 조회로
1 -> 0전이를 확인하는 편이 안전하다. 추가 테스트로 인증 헤더를 일부러 깨뜨렸을 때 이 케이스가 반드시 실패하는지 확인해야 한다.🤖 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/ProductV1ApiE2ETest.java` around lines 319 - 344, The test currently fires two calls to testRestTemplate.exchange for POST and DELETE on ENDPOINT_PRODUCTS + "/" + product.getId() + "/likes" but never asserts their responses; add assertions on both exchange responses (ensure HTTP 200/OK and non-null body) and additionally fetch the product after the POST to assert likeCount == 1 before issuing the DELETE and then fetch again to assert likeCount == 0; use the existing ParameterizedTypeReference/response handling pattern and also add one negative test that supplies an invalid memberAuthHeaders() to confirm authentication causes a failure.apps/commerce-api/src/test/java/com/loopers/interfaces/api/CachePerformanceComparisonTest.java-55-55 (1)
55-55:⚠️ Potential issue | 🟠 Major순서 의존형 벤치마크라 단독 실행 시 전제가 무너진다.
Line 55의 순서 고정과 Line 164, 211, 271, 299의 전제조건 때문에 각 hit 테스트가 이전 테스트가 캐시를 채워줬다는 가정에 의존한다. 운영 관점에서는 메서드 단독 실행이나 재시도 시 측정값이 달라져 회귀 신호를 잃으므로, 각 테스트가 시작할 때 캐시 초기화와 예열을 자체 수행하도록 분리하는 편이 안전하다. 추가 테스트로 각 케이스를 단독 실행하거나 순서를 섞어도 같은 전제조건에서 통과하는지 확인해야 한다.
As per coding guidelines, "통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다."
Also applies to: 163-165, 209-212, 269-275, 297-303
🤖 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/CachePerformanceComparisonTest.java` at line 55, The test class CachePerformanceComparisonTest relies on TestMethodOrder and cross-test cache state for hit-rate benchmarks; remove the class-level TestMethodOrder annotation and modify each cache-dependent test (the hit scenario tests referenced at lines 164, 211, 271, 299) to perform its own setup: explicitly clear/reset the cache and perform any required warm-up/preload at the start of the test (or in a `@BeforeEach` helper called by each test) so each test is independently repeatable; ensure setup/teardown is implemented in methods inside CachePerformanceComparisonTest (e.g., a private resetCache() and warmUpCache() invoked by each hit test) and add an extra test that runs a hit scenario in isolation to verify the behavior when tests execute in any order.apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.java-174-230 (1)
174-230:⚠️ Potential issue | 🟠 Major원자적 업데이트 회귀를 막기에는 동시성 검증이 부족하다.
Line 178-229의 케이스는 모두 단일 스레드라서, 구현이 다시 read-modify-write로 바뀌어도 계속 통과한다. 운영에서는 동시 좋아요 순간에 lost update가 숨어버리므로,
ExecutorService와 별도 트랜잭션으로 다중 스레드 increment/decrement 후 최종likeCount를 검증하는 통합 테스트를 추가하는 편이 안전하다. 추가 테스트로 100회 동시 increment 결과가 정확히 100이고, 0에서 동시 decrement해도 음수가 되지 않는 케이스를 넣어야 한다.🤖 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/product/ProductServiceIntegrationTest.java` around lines 174 - 230, Add a multi-threaded integration test using ExecutorService to verify atomic increments/decrements: create a product, submit 100 concurrent tasks (via ExecutorService) each calling productService.incrementLikeCount(productId) in its own transaction (use TransactionTemplate or ensure the service method is `@Transactional`), wait for completion, then entityManager.flush()/clear() and assert productService.findById(id).getLikeCount() == 100; add a second test that submits many concurrent productService.decrementLikeCount(productId) calls starting from 0 (or from a small count) and assert after flush/clear that Product.getLikeCount() is never negative (== 0 when starting from 0). Ensure each task executes in an independent transaction so this catches lost-update race conditions.apps/commerce-api/src/main/java/com/loopers/config/LocalCacheConfig.java-25-27 (1)
25-27:⚠️ Potential issue | 🟠 Major상품 상세 캐시 TTL이 PR 목표와 다르다.
Line 27이
Duration.ofMinutes(5)라서, PR에 적은 상세 TTL 10분보다 miss가 더 자주 발생하고 상세 조회 DB 부하가 다시 올라간다. 10분이 의도라면 설정을 10분으로 맞추고, 5분이 최종값이라면 PR 설명과 성능 기준도 함께 수정하는 편이 안전하다. 추가 테스트로 상세 캐시가 10분 전에는 hit이고 10분 이후에만 miss가 되는 만료 검증을 넣어 두는 것이 좋다.수정 예시
- buildCache("productDetail", Duration.ofMinutes(5), 5_000) // 상품 상세 조회용 + buildCache("productDetail", Duration.ofMinutes(10), 5_000) // 상품 상세 조회용🤖 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/config/LocalCacheConfig.java` around lines 25 - 27, The product detail cache TTL in LocalCacheConfig is set to Duration.ofMinutes(5) but the PR states 10 minutes; update the buildCache call for "productDetail" in LocalCacheConfig to use Duration.ofMinutes(10) (or, if 5 minutes is intended, update the PR description and performance criteria instead), and add/adjust a unit/integration test that exercises the cache created by buildCache("productDetail", ...) to assert hits before 10 minutes and a miss after 10 minutes to validate expiration behavior.apps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeRepositoryImpl.java-85-88 (1)
85-88:⚠️ Potential issue | 🟠 Major배치/스케줄러 호출 경로의 트랜잭션 경계 누락으로 인한 runtime 실패 위험이 있다.
Line 85-88의
refreshLikeSummary()는 자체@Transactional이 없으면서 javadoc에 "주기적(배치/스케줄러)으로 호출된다"고 명시되어 있다. 그러나 배치/스케줄러 작업은 상위@Transactional을 제공하지 않으므로, 내부의@Modifying쿼리 실행 시TransactionRequiredException으로 실패한다. 테스트는 통합 테스트의 클래스 레벨@Transactional덕분에 통과하지만, 실제 배치 실행 시에는 예외가 발생한다.수정안:
LikeService.refreshLikeSummary()에@Transactional을 추가하거나,- ProductFacade에 배치용 래퍼 메서드를 추가하여
@Transactional으로 감싸고, 배치 스케줄러가 해당 메서드를 호출하도록 변경하는 방식을 권장한다.추가 테스트:
- 배치 컨텍스트(트랜잭션 없음)에서
refreshLikeSummary()호출이 성공적으로 동작하는지 검증하는 테스트를 추가한다. (TransactionTemplate으로 트랜잭션 미포함 환경 시뮬레이션)🤖 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/like/LikeRepositoryImpl.java` around lines 85 - 88, refreshLikeSummary() currently executes a `@Modifying` JPA operation but has no transaction boundary and will fail when invoked from a scheduler; wrap it in a transaction by either annotating LikeService.refreshLikeSummary() with `@Transactional` or add a new `@Transactional` batch wrapper method on ProductFacade and invoke that from the scheduler, then add an integration test that calls refreshLikeSummary() from a non-transactional context (use TransactionTemplate to simulate) to verify it succeeds outside of class-level test transactions.apps/commerce-api/src/main/java/com/loopers/application/product/AdminProductFacade.java-55-60 (1)
55-60:⚠️ Potential issue | 🟠 Major상세 캐시만 비우면 목록 캐시가 계속 stale하다.
현재 변경으로는 update/delete 이후에도 검색 캐시 0~2페이지가 TTL 동안 그대로 남아서, 비노출·삭제·가격 변경·정렬 순서 변경이 목록 응답에 늦게 반영된다. 운영에서는 숨김 처리한 상품이 계속 노출되거나 정렬 결과가 틀어질 수 있으니, 관련 목록 캐시 전체를 함께 비우거나 버전 키를 올리는 방식으로 검색 캐시도 무효화해야 한다. 추가로 캐시 hit 상태에서 상품 비노출/삭제 후 목록 조회가 즉시 반영되는 E2E 테스트를 추가해야 한다.
Also applies to: 64-70
🤖 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/AdminProductFacade.java` around lines 55 - 60, The product update/delete currently only evicts the detail cache (productCacheStore.evictDetail) causing list/search caches to stay stale; modify AdminProductFacade's update (where productService.update is called) and the corresponding delete flow to also invalidate the relevant product list/search caches—either call a new productCacheStore.evictList(...) / evictSearchPages(...) method or increment a product list version key in productCacheStore so page 0-2 results are invalidated; after implementing cache invalidation add an E2E test that updates/hides/deletes a product and asserts that a subsequent list/search request immediately reflects the change (no stale results).apps/commerce-api/src/test/java/com/loopers/domain/like/ProductLikeSummaryIntegrationTest.java-117-150 (1)
117-150:⚠️ Potential issue | 🟠 Major좋아요가 0건이 되는 전이 케이스가 빠져 있다.
현재 unlike 검증은 3→2만 다뤄서, refresh 로직이 0건 상품의 summary row를 제거하지 못해도 그대로 통과한다. 한 상품을 1→0으로 만든 뒤 refresh 후 LIKE_DESC 결과에서 해당 상품이 0건으로 반영되는지까지 명시적으로 검증해야 한다. 추가로 “모든 좋아요 취소 후 refresh” 케이스를 별도 테스트로 넣어 stale summary 회귀를 막아야 한다.
🤖 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/like/ProductLikeSummaryIntegrationTest.java` around lines 117 - 150, The test currently only covers a decrement from 3→2 and misses the edge where a product's likes go 1→0; update ProductLikeSummaryIntegrationTest to include a case where a product receives a like then an unlike (so its count becomes 0) before calling likeService.refreshLikeSummary(), then assert that productService.searchWithMaterializedView(...) reflects the 0-like state (either excluded from LIKE_DESC results or present with like count 0 as your contract dictates). Also add a separate test method that removes all likes from all users for a product, calls likeService.refreshLikeSummary(), and asserts there is no stale summary row (e.g., product not appearing in LIKE_DESC results or having zero count) to prevent regressions; reference the existing test method reflectsUnlikesAfterRefresh, likeService.refreshLikeSummary(), and productService.searchWithMaterializedView(...) when locating where to add these checks.apps/commerce-api/src/test/java/com/loopers/domain/like/LikeConcurrencyTest.java-191-235 (1)
191-235:⚠️ Potential issue | 🟠 Major‘하나만 성공’이라는 계약이 실제 assert에 반영되지 않는다.
두 테스트는 최종
likeCount만 맞으면 통과해서, 중복 요청이 전부 성공 처리되거나 일부가 lock timeout 같은 인프라 오류로 실패해도 회귀를 놓칠 수 있다. 기대 계약을 하나로 고정해 정확히 검증해야 한다. 중복 요청을 거절하는 정책이면successCount == 1과 나머지의 예외 타입 또는 반환값을 assert하고, idempotent no-op 정책이면 테스트명과 assertions를 그에 맞게 바꾸는 편이 낫다. 추가로 실패 결과를 수집해 비즈니스 중복 오류 외의 예외가 섞이면 즉시 실패하도록 보강해야 한다. As per coding guidelines**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.Also applies to: 239-289
🤖 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/like/LikeConcurrencyTest.java` around lines 191 - 235, The test onlyOneLikeSucceeds_whenConcurrentLikeBySameMember in LikeConcurrencyTest currently only asserts final likeCount and total attempts, so it may pass even if multiple threads succeeded or unexpected infra errors occurred; update the test to assert successCount == 1 and that all failures are the expected business duplicate error (or, if the policy is idempotent no-op, change the test name/assertions to reflect that), by: recording thrown exceptions inside the executor (collecting them into a concurrent list), asserting successCount.get() == 1, verifying failCount.get() == threadCount - 1, and iterating the collected exceptions to fail the test if any exception is not the expected DuplicateLikeException (or the agreed business error type) so unexpected infra exceptions immediately fail; look for references to productFacade.like(...) and productService.findById(...) and update the assertions and exception handling in onlyOneLikeSucceeds_whenConcurrentLikeBySameMember (and similarly for the related test at lines 239-289).apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java-49-75 (1)
49-75:⚠️ Potential issue | 🟠 Major비교용 캐시 엔드포인트를 메인 공개 API에 두면 안 된다.
/local-cache와/no-cache는 테스트·비교 목적이지만 현재 프로덕션 컨트롤러에 그대로 매핑되어 있다. 운영에서는 외부 호출자가/no-cache로 캐시를 우회해 DB 부하를 의도적으로 키울 수 있고,/local-cache는 노드별 결과 차이를 공개 API 계약으로 굳혀 버린다. 이 경로들은 로컬/테스트 프로필 전용 컨트롤러나 내부 관리 경로로 분리하고, 운영 프로필에서는 매핑 자체가 생성되지 않게 해 달라. 추가로 prod profile에서는 접근이 차단되고 test/local profile에서만 노출되는 통합 테스트를 넣어 달라.Also applies to: 88-104
🤖 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` around lines 49 - 75, Remove the test/comparison endpoints from the public ProductV1Controller and relocate them into a dedicated test-only controller class (e.g., ProductV1CacheTestController) annotated with a profile or conditional so it only exists in non-prod environments (use `@Profile`({"local","test"}) or `@ConditionalOnProperty` with a test-only flag). Specifically, delete getProductsWithLocalCache and getProductsNoCache from ProductV1Controller and re-implement those methods in the new controller class, calling productFacade.getProductsWithLocalCache and productFacade.getProductsNoCache respectively; ensure the new controller carries `@RequestMapping`("/...") and the profile/conditional annotation so in production the mappings are not registered. Finally, add an integration test that boots the prod profile and asserts those paths are not exposed and a test/local profile test that they are accessible.apps/commerce-api/src/main/java/com/loopers/domain/like/LikeService.java-63-75 (1)
63-75:⚠️ Potential issue | 🟠 MajorDELETE 의미와 unlike 계약이 어긋난다.
현재는 좋아요 row가 한 번도 없으면
BAD_REQUEST를 던지고, row가 있지만 이미N이면false를 반환한다. 운영에서는 같은 "이미 관계가 없음" 상태가 저장 형태에 따라 400과 성공으로 갈려 모바일 재시도와 중복 호출이 불안정해진다. row가 없을 때도false로 처리해 no-op으로 수렴시키거나, 정말 404가 필요하다면 facade에서 상품 존재성을 먼저 고정해 API 계약을 명확히 해 달라. 추가로 "한 번도 좋아요하지 않은 상품에 대한 DELETE"가 count 변화 없이 성공적으로 처리되는 테스트를 추가해 달라. As per coding guidelines**/*Service*.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/domain/like/LikeService.java` around lines 63 - 75, The unlike method currently throws CoreException(BAD_REQUEST) when no Like row exists, which breaks idempotency; change LikeService.unlike to treat a missing like (likeRepository.findByMemberIdAndProductId returning empty) as a no-op and return false instead of throwing, keep the existing branch that returns false when like.isLiked() is already false, invoke like.unlike() and likeRepository.save(like) only when transitioning Y→N; also add a unit test that calls the unlike path for a product never liked (verifying no DB count change and a false/OK result) to ensure DELETE semantics are idempotent and stable.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java-100-115 (1)
100-115:⚠️ Potential issue | 🟠 Major좋아요 수 변경 후 캐시 정합성이 깨진다.
여기서는 DB의
likeCount만 바꾸고 Redis/로컬 목록·상세 캐시를 전혀 비우거나 갱신하지 않는다. 운영에서는 사용자가 좋아요 직후에도 상세의 좋아요 수와 좋아요순 정렬 결과가 TTL 동안 예전 값으로 남아 정렬 신뢰도가 무너진다. 상태가 실제로 변경된 경우에는 커밋 후 상세 캐시를 즉시 무효화하고, 좋아요순 목록 캐시는 영향 범위의 키를 evict 또는 write-through 하도록 후속 처리를 추가해 달라. 추가로 캐시를 미리 적재한 뒤 like/unlike를 수행했을 때 상세 count와 좋아요순 첫 페이지가 즉시 반영되는 E2E 테스트를 넣어 달라.🤖 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 100 - 115, The like/unlike methods call productService.incrementLikeCount/decrementLikeCount after likeService.like/unlike but do not evict or update cached product detail and "likes-sorted" list entries, causing stale data; update ProductFacade.like and ProductFacade.unlike to perform cache invalidation/update only after the transaction successfully commits (e.g., register a TransactionSynchronization or publish a transactional event) so that on changed==true you evict the product detail cache key and evict/update any affected "likes-sorted" list keys (or write-through the updated first-page result) using your cache layer (CacheManager/ProductCacheService); also add an E2E test that preloads the detail and likes-first-page cache, calls ProductFacade.like and ProductFacade.unlike, then asserts the detail likeCount and the first-page ordering reflect the change immediately.
🟡 Minor comments (3)
.gitignore-48-49 (1)
48-49:⚠️ Potential issue | 🟡 Minor
**/generated/패턴 범위 확장에 대한 운영 관점 검토현재 리포지토리에서 추적 중인 파일 중
generated경로를 포함한 파일은 없으므로, 이번 패턴 변경으로 기존 추적 파일이 무시되지는 않는다. 다만 원칙적으로.gitignore패턴은 필요한 범위만큼만 설정하는 것이 운영상 바람직하다.
**/generated/패턴은 리포지토리의 모든 레벨에서generated디렉토리를 무시하므로, 향후 다음과 같은 상황에서 의도하지 않은 파일이 무시될 수 있다:
- 테스트 픽스처 또는 문서 예제에 포함된 생성 샘플 코드
- 다른 도구의 생성 산출물
QueryDSL 생성 위치가 명확하다면 더 구체적인 패턴 사용을 권장한다:
/src/main/generated/ /src/test/generated/또는 빌드 디렉토리 하위로만 제한:
**/build/generated/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 48 - 49, The current broad ignore pattern "**/generated/" is too wide and may unintentionally ignore generated folders elsewhere; replace it with more specific patterns such as /src/main/generated/ and /src/test/generated/ or restrict to build outputs like **/build/generated/ (keep references to the existing pattern "**/generated/" when editing .gitignore and remove or replace it with one or more of the suggested concrete patterns to limit the ignore scope).apps/commerce-api/src/main/java/com/loopers/domain/product/ProductOption.java-57-61 (1)
57-61:⚠️ Potential issue | 🟡 MinorBaseEntity.restoreBase 호출 시 타임스탬프가 null로 설정된다.
restoreBase(id, null)호출로createdAt,updatedAt이 모두 null이 된다. BaseEntity 리뷰에서 언급한 것과 동일한 문제가 발생한다.추가로, 캐시 데이터에
createdAt이 포함되어 있다면 해당 값을 파라미터로 전달하여 완전한 상태 복원이 가능하도록 개선해야 한다.🤖 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/ProductOption.java` around lines 57 - 61, The restoreFromCache method currently calls restoreBase(id, null) causing createdAt/updatedAt to be null; update ProductOption.restoreFromCache to accept cached timestamp(s) (at minimum createdAt, optionally updatedAt) as parameter(s) and pass them into BaseEntity.restoreBase instead of null so the entity is fully restored; locate the restoreFromCache method in ProductOption and change its signature to include the timestamp parameter(s) and forward those values to restoreBase.apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceTest.java-434-445 (1)
434-445:⚠️ Potential issue | 🟡 Minor경고 로그 분기를 검증하지 않아 테스트 이름이 실제 보장과 다르다.
이 테스트는
decrementLikeCount(1L)호출만 확인하고, DisplayName이 말하는 "경고 로그를 남긴다"는 행동은 검증하지 않는다. 이 상태에서는 warn 분기가 제거되어도 테스트가 계속 통과해 운영 중 이상 징후를 놓치기 쉽다.OutputCaptureExtension이나 LogCaptor로 warn 1회를 검증하거나, 로그를 검증하지 않을 계획이면 테스트 이름을 호출 검증 수준으로 낮추는 편이 정확하다. 추가로updatedRows == 1일 때는 warn이 남지 않는 케이스도 함께 넣어 두는 것이 좋다.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/domain/product/ProductServiceTest.java` around lines 434 - 445, The test does not actually assert the warning log behavior described in the DisplayName; update the test method doesNotDecrement_whenLikeCountIsZero() to also verify the warn log was emitted when productRepository.decrementLikeCount(1L) returns 0 by capturing logs (use JUnit's OutputCaptureExtension or LogCaptor) and asserting a single warn message, and add a complementary test for the updatedRows == 1 case to assert no warn is logged; ensure you reference productService.decrementLikeCount(1L) and productRepository.decrementLikeCount(1L) in the updated assertions.
🧹 Nitpick comments (16)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductNoCacheApiE2ETest.java (3)
72-76: 테스트 격리를 위해@BeforeEach에서도정리 작업이 필요하다.현재
@AfterEach에서만정리하므로 테스트 실패 시 이전 테스트 데이터가 잔류할 수 있다. 테스트 순서나 병렬 실행에 따라 flaky 테스트가 될 수 있다.수정안
+ `@BeforeEach` + void setUp() { + databaseCleanUp.truncateAllTables(); + redisCleanUp.truncateAll(); + } + `@AfterEach` void tearDown() { databaseCleanUp.truncateAllTables(); redisCleanUp.truncateAll(); }🤖 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/ProductNoCacheApiE2ETest.java` around lines 72 - 76, Add setup cleanup to ensure test isolation by invoking the same cleanup logic before each test: create a `@BeforeEach` method in ProductNoCacheApiE2ETest that calls databaseCleanUp.truncateAllTables() and redisCleanUp.truncateAll(); this mirrors the existing tearDown() method (which remains as `@AfterEach`) so stale data is removed even if a previous test fails.
156-162: NPE 방어 코드가 필요하다.
response.getBody()가 null을 반환할 경우 Line 160-161에서 NPE가 발생한다. 테스트 실패 시 원인 파악이 어려워진다.수정안
assertAll( () -> assertThat(firstResponse.getStatusCode()).isEqualTo(HttpStatus.OK), () -> assertThat(secondResponse.getStatusCode()).isEqualTo(HttpStatus.OK), - () -> assertThat(secondResponse.getBody().data()) - .isEqualTo(firstResponse.getBody().data()), + () -> { + assertThat(firstResponse.getBody()).isNotNull(); + assertThat(secondResponse.getBody()).isNotNull(); + assertThat(secondResponse.getBody().data()) + .isEqualTo(firstResponse.getBody().data()); + }, () -> assertThat(redisTemplate.keys("product:search:*")).isEmpty());🤖 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/ProductNoCacheApiE2ETest.java` around lines 156 - 162, The test currently dereferences response bodies directly (secondResponse.getBody(), firstResponse.getBody()) which can NPE; update ProductNoCacheApiE2ETest to assert the response bodies are not null before calling data(): add null-check assertions (e.g., assertThat(firstResponse.getBody()).as("firstResponse body").isNotNull() and similarly for secondResponse) and only then compare data() values and check redisTemplate keys, so failures show clear messages instead of NPEs.
213-225: 404 테스트에서 응답 본문 검증이 누락되어 있다.HTTP 상태 코드만 검증하고 에러 응답 본문(error code, message 등)을 검증하지 않는다. 에러 응답 형식이 변경되어도 테스트가 통과할 수 있다.
수정안
// then assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(response.getBody()).isNotNull(); + // error code 또는 message 검증 추가🤖 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/ProductNoCacheApiE2ETest.java` around lines 213 - 225, The test fail_whenProductNotFound_thenReturns404 in ProductNoCacheApiE2ETest only asserts the HTTP status; extend it to also assert the response body (the ApiResponse<ProductV1Dto.ProductDetailResponse> instance in variable response) contains the expected error payload (e.g., non-null body, correct error code and error message fields) so changes to error body format will be caught; locate the testRestTemplate.exchange call and the response/responseType variables and add assertions against response.getBody().getErrorCode() / getMessage() (or the actual ApiResponse error-field names) to validate the error response content.apps/commerce-api/src/test/java/com/loopers/domain/like/LikeCountQueryPerformanceTest.java (4)
41-44: 성능 테스트를 일반 테스트와 분리하는 것이 권장된다.
@SpringBootTest와 대용량 시드 데이터를 사용하는 성능 테스트가 일반 테스트와 함께 실행되면 CI 시간이 크게 증가한다.권장 구성:
@Tag("performance")또는@ActiveProfiles("performance")적용- CI에서 기본 테스트와 분리 실행
- 별도
src/test-performance소스셋 구성 고려수정안
`@SpringBootTest` `@TestInstance`(TestInstance.Lifecycle.PER_CLASS) `@TestMethodOrder`(MethodOrderer.OrderAnnotation.class) +@Tag("performance") class LikeCountQueryPerformanceTest {🤖 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/like/LikeCountQueryPerformanceTest.java` around lines 41 - 44, This performance test class LikeCountQueryPerformanceTest is annotated with `@SpringBootTest` and uses large seed data which will slow CI; mark it as a performance-only test by adding a test tag or profile (e.g., annotate the class with `@Tag`("performance") or `@ActiveProfiles`("performance")) and update CI to exclude or run tests with that tag/profile separately (optionally move to a dedicated test source set such as src/test-performance); ensure existing annotations on LikeCountQueryPerformanceTest (e.g., `@SpringBootTest`, `@TestInstance`, `@TestMethodOrder`) remain but the new tag/profile isolates execution.
61-70: 시드 데이터 로드 실패 시 에러 메시지가 불명확하다.
seed-data.sql또는seed-likes-data.sql파일이 없거나 SQL 오류 발생 시 예외 메시지만으로 원인 파악이 어렵다. 또한 대용량 시드 데이터(10만건) 로드 시간이 길어 CI 타임아웃이 발생할 수 있다.권장사항:
- 시드 파일 존재 여부 사전 검증
- 로드 시간 로깅 추가
- 성능 테스트를 별도 CI 파이프라인 또는 프로파일로 분리
🤖 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/like/LikeCountQueryPerformanceTest.java` around lines 61 - 70, In setUp() update the seeding to first verify the ClassPathResource exists for "seed-data.sql" and "seed-likes-data.sql" before calling ScriptUtils.executeSqlScript, add timing/logging around each ScriptUtils.executeSqlScript call to log start, end, duration and any caught SQLException details (use the existing test logger), and mark this heavy test (LikeCountQueryPerformanceTest) with a dedicated JUnit profile/tag (e.g., `@Tag`("performance") or a Spring profile) so it can be excluded from default CI and run in a separate performance pipeline; reference the setUp() method and the ScriptUtils.executeSqlScript/ClassPathResource usages when making these changes.
77-107: 성능 테스트에 명시적 임계값 검증이 없어 회귀를 감지할 수 없다.현재 테스트는 결과를 출력만 하고 성능 임계값을 검증하지 않는다. 성능 저하가 발생해도 테스트가 통과하여 회귀가 감지되지 않는다.
운영 관점 권장사항:
- 평균 응답 시간에 대한 상한선 assertion 추가
- CI에서 성능 테스트를 별도 프로파일로 분리 (
@ActiveProfiles("performance"))- 결과를 System.out 대신 테스트 리포터 또는 메트릭 시스템으로 전송
임계값 검증 예시
double avgMs = (totalElapsed / (double) iterations) / 1_000_000.0; System.out.printf("[비정규화] 전체 + 좋아요순: 평균 %.2fms (%d회)%n", avgMs, iterations); + + // 성능 회귀 감지를 위한 임계값 검증 + assertThat(avgMs) + .as("비정규화 좋아요순 조회 평균 응답 시간이 임계값(50ms)을 초과함") + .isLessThan(50.0);🤖 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/like/LikeCountQueryPerformanceTest.java` around lines 77 - 107, Add an explicit performance threshold assertion and isolate the test with a performance profile: in the denormalized_allProducts_sortByLikeDesc test, after computing avgMs, assert avgMs is below a chosen threshold (e.g., assertTrue(avgMs <= MAX_ALLOWED_MS) or use assertThat(avgMs).isLessThan(MAX_ALLOWED_MS)), and replace the System.out.printf with sending the metric to a test reporter or logger; also annotate the test class or method with `@ActiveProfiles`("performance") so CI can run these separately. Ensure references: method denormalized_allProducts_sortByLikeDesc, ProductService.search call used in warm-up and measured loop, local variables iterations and avgMs, and a new constant MAX_ALLOWED_MS to hold the threshold.
49-59: 사용하지 않는 의존성 주입이 있다.
LikeService(Line 50)와EntityManager(Line 59)가 주입되었으나 테스트에서 사용되지 않는다. 불필요한 의존성은 테스트 컨텍스트 로딩 시간을 증가시키고 코드 가독성을 저하시킨다.수정안
`@Autowired` private ProductService productService; - `@Autowired` - private LikeService likeService; - `@Autowired` private DatabaseCleanUp databaseCleanUp; `@Autowired` private DataSource dataSource; - - `@Autowired` - private EntityManager entityManager;🤖 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/like/LikeCountQueryPerformanceTest.java` around lines 49 - 59, Remove the unused injected dependencies to avoid unnecessary test context setup: delete the `@Autowired` fields likeService and entityManager (symbols: LikeService likeService, EntityManager entityManager) from LikeCountQueryPerformanceTest so only required beans (e.g., DatabaseCleanUp, DataSource) remain; ensure no remaining references to these symbols in the test class and run tests to confirm nothing else depends on them.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheStore.java (1)
37-69: 가변 도메인 객체를 그대로 캐싱하면 같은 JVM에서 캐시 오염에 취약하다.이 구현은
Product와Page<Product>를 복사하지 않고 같은 객체 참조를 캐시에 저장하고 다시 반환한다.Product와ProductOption은 변경 메서드를 가진 가변 객체라서, 한 요청에서 실수로 상태를 바꾸면 같은 JVM의 다음 요청이 명시적인 cache put 없이도 오염된 값을 읽을 수 있다. Redis 구현처럼 스냅샷 DTO를 저장하거나, 최소한 get/set 시restoreFromCache기반의 방어적 복사본을 사용해 캐시 객체를 불변화하는 편이 안전하다. 추가로 캐시에서 꺼낸 객체를 수정한 뒤 재조회해도 원본 캐시 값이 바뀌지 않는 테스트를 넣는 것이 좋다.As per coding guidelines
**/*.java: 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/infrastructure/product/ProductLocalCacheStore.java` around lines 37 - 69, The cache stores mutable Product and Page<Product> instances directly, risking JVM-level cache contamination; modify ProductLocalCacheStore to perform defensive copying on both put and get: when setting (setDetail, setSearch) store an immutable/snapshot DTO or a deep-copy of Product/Page<Product>, and when getting (getDetail, getSearch) return a restored defensive copy (use an existing restoreFromCache or implement a toSnapshot/fromSnapshot conversion on Product and Page wrapper) so callers never receive the original cached mutable object; add a unit test that mutates a returned Product and re-reads from the cache to assert the cached value did not change.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRedisCacheStore.java (1)
49-122: hot key miss 이후 재생성을 조율하는 장치는 확인이 필요하다.제공된 store 코드는 plain get/set만 수행하므로, 상위
ProductFacade도 일반적인 cache-aside라면 인기 목록 0페이지나 상세 키 만료 시 동시 요청이 한꺼번에 DB fallback으로 내려간다. 이 PR의 목표가 DB 부하 완화인 만큼 miss 경로에 single-flight, stale-while-revalidate, 또는 최소한 TTL jitter가 있는지 확인하고, 없다면 per-key lock을 추가하는 편이 안전하다. 추가로 캐시 만료 직후 동시 요청 N개에서 실제 DB 조회가 1회로 수렴하는지 검증하는 동시성 테스트를 넣는 것이 좋다. 상위 레이어에 이미 miss coalescing이 있다면 이 코멘트는 닫아도 된다.🤖 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/ProductRedisCacheStore.java` around lines 49 - 122, The ProductRedisCacheStore methods (getDetail/setDetail and getSearch/setSearch) perform plain get/set without any hot-key coordination, so implement per-key miss coalescing or jittered TTL to prevent stampedes: add a per-key lock or single-flight mechanism around cache miss handling in getDetail and getSearch (use detailKey(productId) and buildSearchKey(condition, pageable) to derive locks), optionally emit a "revalidating" marker and serve stale data while refresh runs in background (use setDetail/setSearch to write refreshed values), and add small random jitter to DETAIL_TTL/SEARCH_TTL to avoid synchronized expiries; also add a concurrency test that hits getDetail and getSearch concurrently to assert only one DB fallback occurs.apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductLocalCacheApiE2ETest.java (1)
119-144: 캐시 히트 테스트가 캐시 사용 자체를 증명하지 못한다.두 구간 모두 첫 번째와 두 번째 응답이 같다는 것만 확인해서, 캐시가 빠져도 DB가 같은 데이터를 반환하면 계속 통과한다. 운영 관점에서는 Caffeine 회귀를 CI가 놓칠 수 있으므로, 첫 호출 뒤
productSearch/productDetail엔트리 존재나 native cache stats를 직접 단언하는 편이 안전하다. 추가 테스트로 캐시 어노테이션을 제거했을 때 이 테스트가 실패하는지 확인해 두면 회귀 방지력이 높아진다.Also applies to: 255-283
🤖 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/ProductLocalCacheApiE2ETest.java` around lines 119 - 144, The test only compares response bodies and doesn't prove the cache was used; update ProductLocalCacheApiE2ETest (method success_whenCacheHit) to explicitly assert the cache state after the first request by querying the application's CacheManager/Caffeine cache for the expected keys (e.g., "productSearch" and/or "productDetail" entries) or inspecting native Caffeine stats to confirm a cache entry/put occurred, then assert a cache hit/increment on the second request; additionally add a complementary test that removes or disables the cache annotation (or clears the cache manager) and verifies the same endpoint no longer hits the cache to catch regressions.apps/commerce-api/src/main/java/com/loopers/domain/like/LikeRepository.java (1)
15-17: MV 갱신 책임은LikeRepository계약에서 분리하는 편이 낫다.Line 17의
refreshLikeSummary()는 Like 저장소의 기본 영속화가 아니라 투영/비정규화 유지 작업이라, 도메인 계약이 특정 구현 전략에 묶인다. 운영 관점에서는 나중에 이벤트 기반 동기화나 배치 잡으로 바꿀 때 호출부 전반을 다시 수정해야 하므로, 별도ProductLikeSummaryRepository나 애플리케이션 유스케이스로 분리하는 편이 안전하다. 추가 테스트도 Like 저장소 계약 테스트와 summary refresh 통합 테스트를 분리해 유지하는 것이 좋다.As per coding guidelines, "도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다."
🤖 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/LikeRepository.java` around lines 15 - 17, The repository interface LikeRepository currently mixes persistence with projection duties via refreshLikeSummary(); remove refreshLikeSummary() from LikeRepository and introduce a separate interface (e.g., ProductLikeSummaryRepository) or an application-level use-case/service to own the MV/projection operation, move the refreshLikeSummary contract there, update all callers to use the new ProductLikeSummaryRepository (or the new application service) instead of LikeRepository, and split/adjust tests so persistence tests exercise LikeRepository.save(...) and projection/integration tests exercise the new summary refresh contract.apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.java (1)
241-272: DELETE 전환에 맞는 실패 경로 검증이 빠져 있다.Line 266에서 메서드를
DELETE로 바꿨는데, 현재 블록은 성공 케이스만 확인한다. 운영 관점에서는 보안 설정이나 라우팅이POST와 다르게 동작해도 회귀를 놓칠 수 있으므로, 무인증 요청과 아직 좋아요하지 않은 상품에 대한DELETE계약을 함께 고정하는 편이 안전하다. 추가 테스트로DELETE+ no headers가 401을 반환하는지, 그리고 선행 좋아요 없이 취소했을 때의 응답을 명시적으로 검증해야 한다.As per coding guidelines, "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."
🤖 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/ProductV1ApiE2ETest.java` around lines 241 - 272, Add two failure-path tests alongside the existing Unlike.success_whenAuthenticated(): one that issues a DELETE to ENDPOINT_PRODUCTS + "/" + product.getId() + "/likes" with no headers (using testRestTemplate.exchange) and asserts HttpStatus.UNAUTHORIZED, and another that issues an authenticated DELETE (using memberAuthHeaders()) against a product that was never liked and asserts the controller's contract for idempotent delete (explicitly assert the expected status, e.g., HttpStatus.OK or HttpStatus.NO_CONTENT per the controller) using the same ParameterizedTypeReference<ApiResponse<Void>> pattern; place these tests in the Unlike nested class to cover unauthenticated and not-previously-liked failure/edge cases.apps/commerce-api/src/main/java/com/loopers/config/ProductCacheWarmUp.java (1)
37-45: 모든 인스턴스가 같은 웜업을 수행하면 배포 시점에 burst가 생긴다.현재 구현은 replica 수만큼 0~2페이지와 상세 조회를 동시에 재생하므로, 롤링 배포나 오토스케일 시 cold-cache 부하가 DB와 Redis에 한꺼번에 몰릴 수 있다. 운영 안정성을 우선하면 분산 락, leader-only 실행, 또는 프로퍼티 기반 토글 중 하나로 1개 인스턴스만 웜업하도록 제한하는 편이 낫다. 추가로 락 미획득 시 facade 호출이 발생하지 않는 단위 테스트와, 락 획득 시에만 웜업이 실행되는 검증을 넣어야 한다.
Also applies to: 47-83
🤖 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/config/ProductCacheWarmUp.java` around lines 37 - 45, The current ProductCacheWarmUp.warmUp method triggers warming on every instance causing bursty DB/Redis load; change it so only one instance runs warmUpSearchCache and warmUpDetailCache by adding a leader-only guard (e.g., acquire a distributed lock or check a single-writer property) before calling those methods in ProductCacheWarmUp; ensure the lock acquisition path calls the warm-up methods and the failure-to-acquire path returns without invoking any facade/service methods. Also add unit tests verifying that when the lock is not acquired no facade/service calls are made and when the lock is acquired the warmUpSearchCache and warmUpDetailCache flows execute.apps/commerce-api/src/test/java/com/loopers/interfaces/api/product/ProductV1ControllerTest.java (1)
158-176: DELETE 좋아요 취소의 인증 실패 경로도 고정해 두는 편이 안전하다.HTTP verb를 PUT에서 DELETE로 바꾼 직후라, 인증 인터셉터나 request mapping이 메서드별로 어긋나도 현재 테스트는 놓친다. 무인증 DELETE 요청이 401을 반환하는 케이스와, 가능하면 잘못된 자격증명 케이스까지 추가해서 보안 회귀를 막아야 한다. 추가 테스트로 헤더 없는
DELETE /api/v1/products/{productId}/likes와 invalid credentials 시나리오를 넣는 편이 좋다. 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/product/ProductV1ControllerTest.java` around lines 158 - 176, Add negative-path tests under the Unlike nested class in ProductV1ControllerTest: add one test that performs DELETE /api/v1/products/1/likes with no HEADER_LOGIN_ID/HEADER_LOGIN_PW and asserts 401 Unauthorized, and another that calls DELETE with invalid credentials (mock memberService.authenticate("bad", "creds") to return null or throw the same authentication failure your controller uses) and assert 401; in both tests verify productFacade.unlike is not invoked (verify(productFacade, never()).unlike(...)). Ensure the new tests use the same request setup as the existing returnsOk_whenAuthenticated() test but omit or mock bad headers and authentication to lock down the failure paths.apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductCacheEvictionE2ETest.java (1)
122-126: E2E 범위라면 관리자 변경도 HTTP로 태우는 편이 안전하다.현재 테스트는 사용자 조회만 HTTP이고 수정/삭제는
AdminProductFacade를 직접 호출한다. 이 상태면 관리자 컨트롤러의 라우팅, 인증, DTO 역직렬화, 예외 매핑이 깨져도 테스트가 통과해 운영 회귀를 놓칠 수 있다. 실제 관리자 API로 수정/삭제를 호출하도록 바꾸거나, facade 직접 호출이 의도라면 테스트명을 통합 테스트로 낮춰 범위를 명확히 해 달라. 추가로 동일 시나리오를 관리자 HTTP 엔드포인트 기준으로 검증하는 케이스를 보강해 달라.Also applies to: 163-163
🤖 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/ProductCacheEvictionE2ETest.java` around lines 122 - 126, The test ProductCacheEvictionE2ETest currently calls AdminProductFacade.updateProduct directly which bypasses controller routing, auth, DTO deserialization and exception mapping; change the test to exercise the real admin HTTP API instead: replace the direct AdminProductFacade.updateProduct(...) call with an HTTP request to the admin product update endpoint using the same client used for user flows (e.g., TestRestTemplate/MockMvc with auth headers and request DTO), or if you intentionally want a narrower integration test, rename the test from an E2E to an integration test to reflect the reduced scope; additionally add a separate E2E test that verifies the admin HTTP update/delete endpoints (routing, auth, DTO mapping and evictions) so the original scenario is covered via the controller path.apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java (1)
170-194: 로컬 캐시 경로의 miss 분기가 비어 있다.지금은 hit만 검증해서 캐시가 비었을 때 DB fallback과
setSearch호출이 깨져도 테스트가 잡지 못한다. 로컬 캐시를 별도 경로로 노출한 만큼 miss 후 재적재까지 같이 잠가 두는 편이 안전하다.localCacheStore.getSearch가Optional.empty()를 반환할 때productService.search와localCacheStore.setSearch가 호출되는 테스트를 추가해 달라. 필요하면 상세 조회(getProductWithLocalCache)도 같은 방식으로 보강하는 편이 좋다. 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/application/product/ProductFacadeTest.java` around lines 170 - 194, Add a test that covers the local-cache miss path for getProductsWithLocalCache: in ProductFacadeTest create a scenario where localCacheStore.getSearch(condition, pageable) returns Optional.empty(), stub productService.search(condition, pageable) to return a Page<Product> (e.g., PageImpl of the product), then call facade.getProductsWithLocalCache(condition, pageable) and assert the returned content, verify productService.search was called once and localCacheStore.setSearch(condition, pageable, returnedPage) was invoked; apply the same pattern to getProductWithLocalCache (stub localCacheStore.get and verify productService.findById and localCacheStore.set were called) to ensure miss+reload flows are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30a5757c-c341-4ef7-845a-d70497d63c09
📒 Files selected for processing (41)
.gitignoreapps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/product/AdminProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/config/LocalCacheConfig.javaapps/commerce-api/src/main/java/com/loopers/config/ProductCacheWarmUp.javaapps/commerce-api/src/main/java/com/loopers/config/WebMvcConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/brand/Brand.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeService.javaapps/commerce-api/src/main/java/com/loopers/domain/like/ProductLikeSummary.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderItem.javaapps/commerce-api/src/main/java/com/loopers/domain/product/Product.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductCacheStore.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductOption.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/LikeRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/like/ProductLikeSummaryJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductLocalCacheStore.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRedisCacheStore.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.javaapps/commerce-api/src/test/java/com/loopers/application/product/AdminProductFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeCountQueryPerformanceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/ProductLikeSummaryIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductSearchIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/CachePerformanceComparisonTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductCacheEvictionE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductLocalCacheApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductNoCacheApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductRedisCacheApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/product/ProductV1ControllerTest.javamodules/jpa/src/main/java/com/loopers/domain/BaseEntity.java
Comment on lines
+58
to
60
| // 상품 수정 시 상세 캐시를 명시적으로 삭제 -> 삭제 후 최초 사용자가 조회시 캐싱. | ||
| productCacheStore.evictDetail(productId); | ||
| return ProductDetailInfo.from(product); |
There was a problem hiding this comment.
캐시 무효화를 트랜잭션 내부에서 직접 호출하는 것은 위험하다.
지금 방식은 커밋 전에 상세 캐시를 비워서 동시 조회가 이전 committed 데이터를 다시 캐시에 채울 수 있고, eviction 예외가 나면 상품 수정/삭제 자체의 가용성까지 같이 떨어뜨린다. 무효화는 커밋 이후 훅으로 분리하고, 실패는 로깅·재시도 대상으로 격리해서 DB 변경 성공 여부와 분리하는 편이 안전하다. 추가로 “커밋 전 동시 상세 조회가 stale cache를 재생성하지 않는다”와 “캐시 삭제 실패가 나도 DB 변경은 커밋된다” 통합 테스트를 넣어야 한다.
Also applies to: 69-70
🤖 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/AdminProductFacade.java`
around lines 58 - 60, The code calls productCacheStore.evictDetail(productId)
directly inside AdminProductFacade within the transaction, which can evict
before commit and make DB-change failures impact availability; refactor to
perform cache eviction after successful commit by registering an after-commit
hook or publishing an after-commit event (e.g., use
TransactionSynchronizationManager.registerSynchronization or an
`@TransactionalEventListener` / ApplicationEvent for
ProductUpdated/ProductDeleted) and move the eviction logic into that listener
which catches exceptions, logs them, and schedules retries rather than throwing;
ensure the symbols to change are productCacheStore.evictDetail(productId)
invocations in AdminProductFacade and add an after-commit listener class/method
to perform eviction and retry/logging, and add integration tests asserting (1)
concurrent read during commit does not repopulate stale cache and (2) cache
eviction failure does not prevent DB commit.
Comment on lines
+10
to
+18
| @Modifying | ||
| @Query(value = """ | ||
| REPLACE INTO product_like_summary (product_id, like_count, updated_at) | ||
| SELECT l.product_id, COUNT(*), NOW() | ||
| FROM likes l | ||
| WHERE l.like_yn = 'Y' | ||
| GROUP BY l.product_id | ||
| """, nativeQuery = true) | ||
| void refreshAll(); |
There was a problem hiding this comment.
좋아요가 0건이 된 상품의 summary row가 제거되지 않는다.
현재 쿼리는 like_yn = 'Y' 집계 결과에 존재하는 product_id만 갱신하므로, 예전에 좋아요가 있다가 0건이 된 상품의 기존 row는 그대로 남는다. 운영에서는 refresh 이후에도 오래된 like_count가 정렬에 반영되어 LIKE_DESC 결과가 잘못될 수 있으니, 같은 트랜잭션에서 summary 테이블을 비우고 INSERT ... SELECT로 다시 채우거나, upsert와 함께 missing row 삭제를 추가해야 한다. 추가로 “모든 좋아요 취소 후 refresh 시 해당 상품이 0건으로 반영된다” 통합 테스트를 넣어야 한다. 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/like/ProductLikeSummaryJpaRepository.java`
around lines 10 - 18, The refreshAll() native query in
ProductLikeSummaryJpaRepository only REPLACEs aggregated rows and leaves stale
summary rows for products whose like_count dropped to 0; modify refreshAll() to
run in one transaction that first clears product_like_summary (TRUNCATE or
DELETE) and then INSERT ... SELECT FROM likes WHERE like_yn='Y' to repopulate,
or alternately perform an upsert plus a DELETE of product_ids not present in the
aggregation; ensure the operation is transactional and touches
product_like_summary and likes in the same method, and add an integration test
that cancels all likes for a product then calls refreshAll() asserting the
product has no summary row (like_count = 0 / absent) afterwards.
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.
📌 Summary
likeCount도입으로 JOIN 없는 단일 테이블 정렬 구현, Redis Cache-Aside 패턴 적용으로 Cache Hit 시 DB 조회 대비 약 2.1배 응답 개선🧭 Context & Decision
문제 정의
선택지와 결정
인덱스 전략: 단일 인덱스 vs 복합 인덱스
like_count,created_at,price): 현재 데이터 분포(ON_SALE 85%, Y 90%)에서는 옵티마이저가 잘 선택하지만, 데이터 분포 변화(시즌 종료, 재고 소진 등)에 취약(status, display_yn, 정렬컬럼)복합 인덱스 3개: 필터+정렬을 하나의 인덱스에서 처리하여 filesort 제거 + LIMIT 조기 종료 가능status선두 복합 인덱스 3개 + 기존 FK(brand_id) 인덱스 활용좋아요 수 구조: 비정규화 vs Materialized View
Product.likeCount): 원자적 UPDATE로 즉시 반영, 복합 인덱스 활용 가능캐시 전략: Local Cache (Caffeine) vs Redis Cache
🏗️ Design Overview
변경 범위
commerce-api— Product, Like 도메인ProductRedisCache— Redis Cache-Aside 패턴 구현체LocalCacheConfig— Caffeine 로컬 캐시 설정 (비교 학습용)ProductLikeSummary— MV 비교 실험용 엔티티주요 컴포넌트 책임
ProductRedisCache: Redis 조회/저장/삭제를 try-catch로 감싸 장애 시 DB fallback 보장. TTL(목록 3분, 상세 10분), 페이지 제한(0~2페이지), 캐시 키 생성을 관리ProductFacade: Cache-Aside 패턴의 분기 로직 — Cache Hit 시 캐시 반환, Miss 시 DB 조회 후 Redis 저장. 로컬 캐시는@Cacheable어노테이션으로 별도 구현AdminProductFacade: 상품 수정/삭제 시evictProductDetail()호출로 즉시 캐시 무효화LikeService: 좋아요 등록/취소 시productRepository.incrementLikeCount()/decrementLikeCount()로 원자적 카운트 동기화Product(Entity):likeCount필드 + 복합 인덱스(status, display_yn, like_count DESC)등 3개 정의ProductRepositoryImpl: QueryDSL 기반 검색 쿼리 — 비정규화likeCount직접 정렬 + MV 방식 비교용searchWithMaterializedView()구현구현 기능
1. 인덱스 적용 —
Productbrand_id는 FK가 아닌 의미적 레퍼런스 ID이므로 단일 인덱스를 직접 추가하여 브랜드 필터 조회 시 1~2ms 유지.status선두 복합 인덱스 3개로 전체 조회 + 정렬 시 filesort 제거 및 LIMIT 조기 종료 달성.2. 비정규화
likeCount필드 —ProductProduct 엔티티에
likeCount컬럼을 추가하여 Like 테이블 JOIN 없이 단일 테이블에서 좋아요순 정렬 가능. 복합 인덱스(status, display_yn, like_count DESC)와 결합하여 0.2ms 정렬 달성.3. 원자적 좋아요 카운트 동기화 —
LikeService좋아요 등록/취소 시
SET like_count = like_count + 1원자적 UPDATE로 Lost Update 방지.likeCount > 0조건으로 음수 방지.4. Redis Cache-Aside 패턴 —
ProductRedisCache모든 Redis 조회/저장/삭제를 try-catch로 감싸 장애 시 DB fallback 보장. 상품 상세 TTL 10분(evict 안전망), 목록 TTL 3분(유일한 갱신 수단), 3페이지(page 0~2)까지만 캐싱.
5. Facade 캐시 분기 로직 —
ProductFacade목록 조회: 캐싱 가능 페이지 → Redis 조회 → Hit 시 반환 / Miss 시 DB 조회 + Redis 저장. 4페이지(page=3) 이상 조회 시 캐시 미사용으로 Redis 메모리 절약.
6. 관리자 수정/삭제 시 캐시 무효화 —
AdminProductFacade상품 수정/삭제 시
evictProductDetail(productId)호출로 Redis에서 해당 상세 캐시 즉시 삭제. 모든 서버에서 다음 요청 시 최신 데이터 조회 보장.7. 로컬 캐시 (Caffeine) 비교용 API —
LocalCacheConfig+ProductFacadeCaffeine 로컬 캐시를
@Cacheable로 적용한 별도 API 엔드포인트. Redis와의 정량적 성능 비교(응답속도, Cache Miss 비용, Evict 후 재조회)를 위해 구현.8. 상품 검색 QueryDSL —
ProductRepositoryImpl비정규화
product.likeCount를 직접 정렬에 사용하여 JOIN 없는 단일 테이블 쿼리 구현. MV 방식 비교용searchWithMaterializedView()도 함께 구현.9. REST API 엔드포인트 —
ProductV1Controller/api/v1/products/api/v1/products/local-cache/api/v1/products/{id}/api/v1/products/{id}/local-cache/api/v1/products/{id}/likes/api/v1/products/{id}/likes🔁 Flow Diagram
Main Flow — 상품 목록 조회 (Redis Cache-Aside)
sequenceDiagram autonumber participant Client participant Controller participant Facade participant RedisCache participant Service participant DB Client->>Controller: GET /api/v1/products?sort=LIKE_DESC Controller->>Facade: getProducts(condition, pageable) alt page <= 2 (캐싱 대상) Facade->>RedisCache: getSearchResult(key) alt Cache Hit RedisCache-->>Facade: cached Page<ProductInfo> Facade-->>Controller: 캐시 데이터 반환 else Cache Miss RedisCache-->>Facade: Optional.empty() Facade->>Service: search(condition, pageable) Service->>DB: QueryDSL (복합 인덱스 활용) DB-->>Service: Page<Product> Service-->>Facade: Page<Product> Facade->>RedisCache: setSearchResult(key, result, TTL=3min) Facade-->>Controller: DB 데이터 반환 end else page > 2 (캐싱 미대상) Facade->>Service: search(condition, pageable) Service->>DB: QueryDSL DB-->>Service: Page<Product> Service-->>Facade: Page<Product> Facade-->>Controller: DB 데이터 반환 end Controller-->>Client: ApiResponse<Page<ProductResponse>>좋아요 등록 + 카운트 동기화 Flow
sequenceDiagram autonumber participant Client participant Controller participant Facade participant LikeService participant LikeRepo participant ProductRepo participant DB Client->>Controller: POST /api/v1/products/{id}/likes Controller->>Facade: like(productId, userId) Facade->>LikeService: like(productId, userId) LikeService->>LikeRepo: findByProductIdAndUserId() alt 신규 좋아요 LikeService->>LikeRepo: save(new Like) else 기존 취소 → 재등록 LikeService->>LikeRepo: like.like() (N→Y) end LikeService->>ProductRepo: incrementLikeCount(productId) Note over ProductRepo,DB: SET like_count = like_count + 1 (원자적) ProductRepo->>DB: UPDATE product DB-->>ProductRepo: affected rows LikeService-->>Facade: void Facade-->>Controller: void Controller-->>Client: 200 OK관리자 상품 수정 + 캐시 무효화 Flow
sequenceDiagram autonumber participant Admin participant AdminFacade participant ProductService participant RedisCache participant DB Admin->>AdminFacade: updateProduct(productId, command) AdminFacade->>ProductService: update(productId, command) ProductService->>DB: UPDATE product DB-->>ProductService: updated ProductService-->>AdminFacade: Product AdminFacade->>RedisCache: evictProductDetail(productId) Note over RedisCache: DEL product:detail:{productId} RedisCache-->>AdminFacade: void AdminFacade-->>Admin: ProductDetailInfo✅ 과제 체크리스트
📊 성능 개선 전후 비교
인덱스 — 10만건 데이터 기준
캐시 — E2E 환경 측정 (50회 반복 평균)
Cache Miss 비용 — 첫 요청 시 (캐시 비어있는 상태)
Redis Cache Miss 시 No Cache 대비 약 3배 느린 이유: DB 조회 비용에 JSON 직렬화 + Redis SET 네트워크 왕복 비용이 추가되기 때문. 단, 이후 반복 호출은 Cache Hit(5.83ms)으로 빠르게 응답하므로 첫 요청의 추가 비용은 후속 Hit으로 상쇄된다.
🧪 테스트
ProductLocalCacheApiE2ETestProductRedisCacheApiE2ETestCachePerformanceComparisonTestProductSearchIntegrationTestProductLikeSummaryIntegrationTestLikeCountQueryPerformanceTestProductFacadeTestAdminProductFacadeTest리뷰포인트
1. 좋아요 수 정렬 — 비정규화 vs Materialized View
좋아요순 정렬을 위해 비정규화와 MV를 비교했습니다. EXPLAIN으로 직접 확인해보니 MV 방식에서는 약 49,518건을 스캔하고 filesort가 발생했는데, 비정규화에서는 복합 인덱스 덕분에 20건만 읽고 0.2ms에 끝나서 차이가 컸습니다.
MV를 선택했을 때 필터 조건들은 product 테이블에 있고 정렬 기준(
like_count)은 MV테이블에 있어서 하나의 복합 인덱스로 같이 처리할 수가 없어서 두 방식에 성능차이가 존재하는 것 같습니다.그래서 인덱스 설계와 잘 맞는 비정규화를 선택했는데, 이 판단이 괜찮은 선택이었는지 궁금합니다. 그리고 실무에서 MV를 실제로 많이 사용하는 편인지, 사용한다면 어떤 경우에 쓰는지도 여쭤보고 싶습니다.
2. Redis 캐시 TTL — 목록 3분, 상세 10분으로 설정
상품 목록 캐시는 키 조합이 검색어-정렬-브랜드ID-page-size로 너무 많아서 명시적으로 삭제하기가 어려워서, TTL(3분) 만료에만 의존하고 있습니다.
반면 상품 상세 캐시는
product:detail:{id}로 키가 명확해서 수정/삭제 시 바로 삭제할 수 있으니까 TTL을 길게(10분) 잡아서 히트율을 높이는 방향으로 했습니다.이렇게 하면 관리자가 상품 가격을 수정했을 때, 상세 페이지는 바로 반영되지만 목록에서는 3분간 변경전 가격이 보일 수 있는데, 이 정도 지연이 실무에서 허용 가능한 수준인지, 아니면 성능 이슈가 있더라도 목록 캐시도 명시적으로 삭제해야 하는 상황이 있는지 궁금합니다!
TTL 설정에 대한 경험이 없다보니 이 값이 적절한지 궁금합니다. 실무에서는 이런 TTL 값을 처음 정할 때 어떤 기준으로 접근하시는지 조언 부탁드립니다!
3. Redis Cache Miss 비용 증가에 대한 대응
Cache Hit 시에는 확실히 응답이 빨라지지만, Cache Miss 시에는 DB 조회에 JSON 직렬화 + Redis SET 비용이 추가되어 캐시가 없을 때보다 오히려 느려지는 구간이 존재합니다.
이후 반복 요청의 Cache Hit으로 상쇄된다고 판단했지만, TTL 만료 시점에 동시 다발적으로 Cache Miss가 발생하는 상황(Cache Stampede)이 우려됩니다.
실무에서 이런 Cache Miss 비용 증가를 감수하는 편인지, 캐시 워밍이나 비동기 저장 같은 별도 대응을 하는 편인지 궁금합니다!
변경 목적
상품 목록 조회 성능 병목 해소를 위해 좋아요 수 비정규화(likeCount), 복합 인덱스 최적화, Redis/Caffeine 캐시 적용을 통해 DB 부하 감소 및 정렬 성능 개선.
핵심 변경사항
리스크/주의사항
테스트/검증