Open
Conversation
📝 WalkthroughWalkthrough제품 조회 캐싱 기능을 추가하고 좋아요 수를 ProductModel에 비정규화하여 관리한다. 제품 목록 조회에 브랜드 필터링을 추가하고 정렬 일관성을 위해 ID로 이차 정렬한다. Redis 기반 캐시 설정과 캐시 무효화 전략을 구현하며, 좋아요 작업 시 제품 좋아요 수를 동기화한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Cache as Redis Cache
participant Facade as ProductFacade
participant DB as Database
participant Repository as ProductRepository
Client->>Facade: getProduct(productId)
Facade->>Cache: 캐시 확인 (productDetail)
alt 캐시 히트
Cache-->>Facade: ProductInfo 반환
else 캐시 미스
Facade->>Repository: findById(productId)
Repository->>DB: SELECT * FROM products
DB-->>Repository: ProductModel
Repository-->>Facade: ProductModel
Facade->>Cache: ProductInfo 저장
Cache-->>Facade: 저장 완료
end
Facade-->>Client: ProductInfo
sequenceDiagram
participant Client
participant Controller as ProductV1Controller
participant Facade as ProductFacade
participant Cache as Redis Cache
participant Service as LikeService
participant DB as Database
Client->>Controller: like(userId, productId)
Controller->>Service: like(userId, productId)
Service->>DB: findByIdForUpdate(productId)
DB-->>Service: ProductModel
Service->>DB: save(LikeModel)
DB-->>Service: LikeModel
Service->>DB: product.increaseLikeCount()
DB-->>Service: 완료
Service->>Cache: evict productDetail 캐시
Cache-->>Service: 무효화 완료
Service->>Cache: evict productList 캐시 (전체)
Cache-->>Service: 무효화 완료
Service-->>Controller: LikeModel
Controller-->>Client: 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 approve the review once all CodeRabbit's comments are resolved.Enable 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: 12
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/like/LikeServiceTest.java (1)
97-111:⚠️ Potential issue | 🟡 Minor충돌 경로에서도
likeCount불변을 같이 검증해달라.운영에서는 중복 좋아요 경합에서 카운트만 증가해도 목록 정렬과 상세 값이 어긋난다. 이 테스트는 예외 타입만 확인해서
increaseLikeCount()가 저장 이전으로 이동해도 놓친다. 수정안은 예외와 함께product.getLikeCount() == 0을 검증하는 것이고, 추가 테스트로이미 좋아요한 상품경로에도 같은 불변식을 고정해달라. 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/like/LikeServiceTest.java` around lines 97 - 111, Update the existing throwsConflictException_whenDataIntegrityViolationOccurs test to also assert the product.likeCount remains unchanged (e.g., assertThat(product.getLikeCount()).isEqualTo(0)) after likeService.like(userId, productId) throws the CoreException; ensure the product's initial like count is set before invoking likeService and keep the mocks (productRepository.findByIdForUpdate, likeRepository.findByUserIdAndProductId, likeRepository.save throwing DataIntegrityViolationException) the same. Add a new unit test for the "already liked" path (e.g., when likeRepository.findByUserIdAndProductId returns Optional.of(existingLike)) that asserts CoreException with ErrorType.CONFLICT is thrown by likeService.like and that product.getLikeCount() remains unchanged there as well. Ensure both tests reference the same product instance used by increaseLikeCount()/likeService.like so the invariant can be verified reliably.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java (1)
47-58:⚠️ Potential issue | 🔴 Critical
like_count컬럼 추가는 데이터베이스 마이그레이션 파일이 필수이다.저장소 전체에 SQL 마이그레이션 파일, Flyway, Liquibase 설정이 없고, 프로덕션 환경 설정에서
spring.jpa.hibernate.ddl-auto: none으로 되어 있다. 엔티티 코드는like_count NOT NULL을 추가했으나 기존 products 테이블의 행들에는 해당 컬럼이 없어, 배포 시 제약조건 위반으로 애플리케이션 시작이 실패한다.수정안: 데이터베이스 마이그레이션 전략을 수립하고, 다음 단계를 순차적으로 진행한다.
- 마이그레이션 파일 생성 (Flyway 또는 수동 SQL):
ALTER TABLE products ADD COLUMN like_count BIGINT DEFAULT 0;로 기존 행 채우기- 제약조건 추가:
ALTER TABLE products MODIFY COLUMN like_count BIGINT NOT NULL;- 인덱스 생성 (필요 시): 두 인덱스 생성 확인
추가 검증: 기존 데이터가 있는 상태에서 마이그레이션이 성공하고,
SELECT COUNT(*) FROM products WHERE like_count IS NULL;이 0을 반환하는지 확인한다.🤖 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 47 - 58, The new non-nullable likeCount field on ProductModel (private long likeCount) requires a DB migration before deploying; add a migration script (Flyway/Liquibase) to create the column with a default and backfill existing rows (e.g., ALTER TABLE products ADD COLUMN like_count BIGINT DEFAULT 0), then a follow-up migration to set the NOT NULL constraint (ALTER TABLE ... MODIFY/MATCH to NOT NULL) and optionally add indexes if needed; ensure the migration files are committed and CI runs them, and verify post-migration with a query like SELECT COUNT(*) FROM products WHERE like_count IS NULL to confirm zero nulls before enabling the entity change.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java (1)
19-21:⚠️ Potential issue | 🟠 Major
findByIdForUpdate메서드에 락 타임아웃 힌트가 누락되어 있다.
@Lock(LockModeType.PESSIMISTIC_WRITE)를 사용하지만@QueryHints로jakarta.persistence.lock.timeout을 설정하지 않았다. 운영 환경에서 데드락이나 락 경합 상황이 발생하면 트랜잭션이 무한 대기 상태에 빠져 스레드 풀이 고갈되고 서비스 장애로 이어질 수 있다.수정안
+ `@QueryHints`(`@QueryHint`(name = "jakarta.persistence.lock.timeout", value = "3000")) `@Lock`(LockModeType.PESSIMISTIC_WRITE) `@Query`("SELECT p FROM ProductModel p WHERE p.id = :id AND p.deletedAt IS NULL") Optional<ProductModel> findByIdForUpdate(Long id);PR
#148에서모든 WithLock 메서드에 일관되게 적용된 패턴이다. 동일한 수정을 진행하고, UserCouponJpaRepository의findByIdAndUserIdForUpdate메서드도 같은 방식으로 타임아웃 힌트를 추가하도록 한다.🤖 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` around lines 19 - 21, The PESSIMISTIC_WRITE method lacks a lock timeout hint; update ProductJpaRepository.findByIdForUpdate to include a QueryHint for "jakarta.persistence.lock.timeout" (set a sensible ms value consistent with the project's pattern, e.g., "3000") so the DB lock will fail fast instead of waiting indefinitely, and apply the same QueryHint addition to UserCouponJpaRepository.findByIdAndUserIdForUpdate; keep the existing `@Lock`(LockModeType.PESSIMISTIC_WRITE) and `@Query` unchanged, just add the `@QueryHints` wrapper with the timeout hint.
🧹 Nitpick comments (4)
apps/commerce-api/src/main/java/com/loopers/domain/like/LikeModel.java (1)
15-18:idx_likes_user_id는 현재 스키마에서 중복 인덱스에 가깝다.운영에서는 likes 쓰기마다 보조 인덱스를 하나 더 갱신하게 되어 핫패스 쓰기 비용만 늘어난다.
(user_id, product_id)유니크 인덱스가 이미user_id단독 조건을 선행 컬럼으로 커버하므로, 수정안은idx_likes_product_id만 유지하고idx_likes_user_id는 EXPLAIN 근거가 있을 때만 남기는 것이다. 추가 테스트로findByUserId와findByUserIdAndProductId에 대해 EXPLAIN 검증을 넣어 복합 유니크 인덱스가 계속 선택되는지 확인해달라.🤖 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 15 - 18, Remove the redundant single-column index idx_likes_user_id from the LikeModel mapping because the unique constraint on (user_id, product_id) already covers user_id as a leading column; keep only idx_likes_product_id. Update the entity/class where the indexes are declared (LikeModel) to drop `@Index`(name = "idx_likes_user_id", ...). Add tests that run EXPLAIN for the repository methods findByUserId and findByUserIdAndProductId to assert the query planner chooses the composite unique index (user_id, product_id) for those queries and only keep idx_likes_user_id if EXPLAIN proves it’s necessary. Ensure test assertions explicitly check the index name or index usage in the EXPLAIN output.apps/commerce-api/src/main/java/com/loopers/application/product/ProductPageInfo.java (1)
16-23: 페이지 콘텐츠는 방어적 복사로 고정하는 편이 안전하다.운영 관점에서 Line 18의
page.getContent()참조를 그대로 보관하면 상위 계층에서 리스트가 변경될 때 캐시 데이터/응답 일관성이 깨질 수 있다.
수정안으로List.copyOf(page.getContent())를 사용해 불변 스냅샷을 저장하는 것을 권장한다.권장 수정 예시
public static ProductPageInfo from(Page<ProductInfo> page) { return new ProductPageInfo( - page.getContent(), + List.copyOf(page.getContent()), page.getNumber(), page.getSize(), page.getTotalElements(), page.getTotalPages() ); }추가 테스트로 원본
Page의 content 리스트를 변경해도ProductPageInfo.content()가 변하지 않는 불변성 테스트를 넣어야 한다.As per coding guidelines
**/*.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/application/product/ProductPageInfo.java` around lines 16 - 23, from(Page<ProductInfo>) stores a direct reference to page.getContent(), which risks mutable external changes; change ProductPageInfo.from to wrap the content with an immutable defensive copy (e.g., List.copyOf(page.getContent())) when constructing the ProductPageInfo so the internal content is fixed, and ensure the ProductPageInfo constructor/field uses that copied list; also add a unit test that mutates the original Page content after creating ProductPageInfo and asserts ProductPageInfo.content() remains unchanged to verify immutability.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java (1)
12-13:brandId의 null 계약을 인터페이스에 명시하는 편이 안전하다.운영 관점에서 null 의미(전체 조회)가 인터페이스에 드러나지 않으면 구현체 추가 시 잘못된 쿼리 분기로 이어져 캐시 키/조회 결과 불일치가 발생할 수 있다.
수정안으로 Line 12, Line 13 시그니처에 nullable 계약을 명시하고 Javadoc으로null = 전체 브랜드를 고정하는 것을 권장한다.권장 수정 예시
+import org.springframework.lang.Nullable; ... - Page<ProductModel> findAll(Pageable pageable, Long brandId); - Page<ProductModel> findAllOrderByLikesDesc(Pageable pageable, Long brandId); + Page<ProductModel> findAll(Pageable pageable, `@Nullable` Long brandId); + Page<ProductModel> findAllOrderByLikesDesc(Pageable pageable, `@Nullable` Long brandId);추가 테스트로
brandId == null과brandId != null두 케이스에서 각각 전체 조회/브랜드 필터 조회로 분기되는 계약 테스트를 두는 것이 안전하다.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/domain/product/ProductRepository.java` around lines 12 - 13, 해당 인터페이스의 메서드 계약이 명시적이지 않아 brandId의 null 의미가 불분명하므로 ProductRepository의 시그니처들 findAll(Pageable pageable, Long brandId) 및 findAllOrderByLikesDesc(Pageable pageable, Long brandId)에 파라미터에 `@Nullable` 애노테이션을 추가하고 Javadoc을 추가해 "brandId == null이면 전체 브랜드 조회"로 명확히 문서화하세요; 또한 구현체 변경 시 규약이 깨지지 않도록 brandId == null(전체 조회)와 brandId != null(브랜드 필터) 두 케이스를 검증하는 단위 테스트를 작성해 계약을 보장하세요.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (1)
79-85:LIKES_DESC정렬이applySorting에서 처리되지 않아 암묵적 의존성이 존재한다.
LIKES_DESC의 경우applySorting이 정렬 조건 없이PageRequest를 반환하며, 실제 정렬은ProductJpaRepository의 JPQLORDER BY절에서 처리된다. 이는 Facade의 정렬 로직과 Repository의 쿼리 메서드 간에 암묵적 결합을 만든다.향후 유지보수 시
applySorting만 보고 정렬 로직을 파악하기 어려우며, Repository 메서드 변경 시 정렬이 누락될 위험이 있다.개선안 (명시적 정렬 포함)
case LIKES_DESC -> PageRequest.of(pageable.getPageNumber(), pageable.getPageSize()); + // Note: LIKES_DESC sorting is handled by repository JPQL (ORDER BY likeCount DESC, id DESC) + // to leverage idx_products_brand_deleted_like_id / idx_products_deleted_like_id indexes또는
ProductService.getAll()에서sortType에 따라 적절한 Repository 메서드를 선택하는 로직이 있다면, 해당 로직 근처에 주석으로 명시하는 것을 권장한다.🤖 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 79 - 85, applySorting currently returns an un-sorted PageRequest for ProductSortType.LIKES_DESC, creating an implicit dependency on ProductJpaRepository's JPQL ORDER BY; fix it by making the sorting explicit: update applySorting(ProductSortType) to return a PageRequest with the corresponding Sort for LIKES_DESC (e.g., Sort.by(Sort.Direction.DESC, "likes").and(Sort.by(Sort.Direction.DESC, "id"))) or, if sorting by a computed value in ProductJpaRepository is required, add a clear comment in ProductService.getAll (or next to ProductJpaRepository query method) explaining that LIKES_DESC is handled by the repository and ensure repository method names/JPQL remain in sync with ProductSortType to avoid implicit coupling.
🤖 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/ProductInfo.java`:
- Line 8: Remove the unsafe polymorphic type hint on ProductInfo by deleting the
`@JsonTypeInfo`(use = JsonTypeInfo.Id.CLASS) annotation from the ProductInfo
class, and instead harden CacheConfig by replacing activateDefaultTyping(...)
with an explicit PolymorphicTypeValidator that white-lists only trusted packages
(or register explicit serializers per cache) via
CacheConfig.getPolymorphicTypeValidator()/activateDefaultTyping changes; then
add a security regression test (e.g., extend ProductCacheTest) that attempts to
deserialize a manipulated cache payload containing a malicious "@class" and
asserts the deserialization is rejected/throws to ensure tampered cache entries
are not instantiated.
In `@apps/commerce-api/src/main/java/com/loopers/domain/like/LikeService.java`:
- Around line 26-27: The current `@CacheEvict` on LikeService evicts allEntries
from "productList" which causes full list cache thrashing; change it to evict
only the brand-scoped or versioned key instead of allEntries: replace
`@CacheEvict`(cacheNames = "productList", allEntries = true) with a targeted
eviction like `@CacheEvict`(cacheNames = "productList", key =
"'product:list:brand:' + `#product.brand.id`") or use a versioned key strategy
(e.g., include a productListVersionProvider token in the key and evict only that
token for the product's brand inside the like/unlike method such as likeProduct
or toggleLike), and add an automated test that likes a product of brand A and
asserts the cached entry for brand B's productList key remains present to verify
isolation.
- Around line 30-31: Add a 3000ms pessimistic lock timeout hint to the
repository method used by the hot path: update
ProductJpaRepository.findByIdForUpdate() to include `@QueryHints`(QueryHint(name =
"jakarta.persistence.lock.timeout", value = "3000")) while keeping
`@Lock`(LockModeType.PESSIMISTIC_WRITE) so concurrent
LikeService.like()/LikeService.unlike() callers don’t block indefinitely; also
add an integration test that issues concurrent like/unlike requests against the
same product and asserts that a lock timeout results in the expected predictable
exception mapping (verify the service/exception translator maps the lock timeout
to the agreed application exception).
- Around line 24-28: The current `@CacheEvict` on the like/unlike method in
LikeService executes before the transaction commit and can cause stale
recaching; change eviction to happen after commit by moving eviction logic to a
TransactionalEventListener(method like evictProductListAfterCommit annotated
with `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)) or
enable a transaction-aware cache manager (wrap RedisCacheManager with
TransactionAwareCacheManagerProxy or enable transaction-aware setting in
CacheConfig) and remove or replace the expensive allEntries = true on
"productList" with a more targeted eviction strategy (e.g., brand or
product-list segment key) to avoid huge hit-rate drops; finally add a
concurrency test that performs a like/unlike transaction while a concurrent read
triggers cache population to ensure the cache is not repopulated with the old
value before commit.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java`:
- Around line 23-28: Replace the direct call to ProductSortType.valueOf(...) in
ProductV1Controller with a safe conversion that throws a CoreException on
invalid input: add a static ProductSortType.fromString(String) method in
ProductSortType that catches IllegalArgumentException and rethrows new
CoreException(ErrorCode.INVALID_PARAMETER, "Invalid sort type: " + value), then
update the controller to call ProductSortType.fromString(sort) (keeping existing
parameter names and usage of productFacade.getAll and
ProductV1Dto.ProductListResponse) so bad sort values produce a 400-style
CoreException and consistent ApiControllerAdvice handling.
In `@apps/commerce-api/src/main/java/com/loopers/support/config/CacheConfig.java`:
- Around line 65-87: The cacheErrorHandler bean currently swallows EVICT/CLEAR
exceptions causing stale cache after successful writes; change
cacheErrorHandler() so GET/PUT continue to log and suppress, but
handleCacheEvictError(...) and handleCacheClearError(...) must not silently
swallow failures — either rethrow the RuntimeException to propagate failure to
the caller or enqueue an after-commit compensation/retry (use
TransactionSynchronizationManager.registerSynchronization after successful DB
commit to perform eviction with retry/backoff), and add integration tests that
force a Redis eviction exception to verify no stale key remains; update
references to handleCacheEvictError and handleCacheClearError and ensure any
transactional write paths trigger eviction in an afterCommit hook when chosen.
In
`@apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheTest.java`:
- Around line 56-77: The test ProductCacheTest.returnsCachedResult_onSecondCall
currently only asserts result equality, which can pass even if the DB is hit
again; update the test to (1) assert the cache entry for
ProductFacade.PRODUCT_DETAIL_CACHE exists after the first call via
cacheManager.getCache(...) and its key (product.getId()), and (2) use `@SpyBean`
on the repository or service used by productFacade (e.g., ProductRepository or
ProductService) to verify it was called exactly once across both
productFacade.getProduct(product.getId()) calls (i.e., first call triggers a
repository/service invocation and second does not); split into two focused tests
if needed: one to assert cache entry creation and one to assert
repository/service call count remains 1.
- Around line 200-242: Add tests that explicitly simulate cache operation
failures by injecting a spy or test-double CacheManager/Cache that throws on
get/put and verifying service falls back to DB: create two new tests (one for
productFacade.getProduct(productId) and one for
productFacade.getAll(pageable,...)) in the CacheMiss nested class that replace
the real CacheManager/Cache with a spy which throws a RuntimeException on
get/put, then assert the calls still return non-null results with correct data
(compare result.name() == "에어맥스" for detail and result.totalElements() == 1 for
list) to ensure DB fallback; locate setup where productFacade is wired and the
existing redisCleanUp.truncateAll() to swap in the test double and restore
original cache after each test.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceTest.java`:
- Around line 101-104: Add tests verifying brandId is forwarded: extend
ProductServiceTest to include cases where brandId != null for both
ProductSortType.LATEST and LIKES_DESC. For getAll(pageable, LATEST, brandId)
assert productRepository.findAll(pageable, brandId) is called and
productRepository.findAllOrderByLikesDesc(...) is never called; for
getAll(pageable, LIKES_DESC, brandId) assert
productRepository.findAllOrderByLikesDesc(pageable, brandId) is called and
productRepository.findAll(...) is never called; keep the existing brandId ==
null test as regression protection. Use the same pageable, mock productPage, and
verify/never with the repository mocks and productService.getAll(...) calls.
In
`@apps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductQueryExplainTest.java`:
- Around line 90-104: The test currently asserts on possibleKeys(result.get(0))
which only lists candidate indexes; update UC1 (and UC2 similarly) to assert on
the actual index used by the optimizer by calling key(result.get(0)) instead of
possibleKeys(...). Locate the test method explain_allProducts_sortByLatest in
ProductQueryExplainTest, use executeExplain to get result and then replace the
possibleKeys assertion with an assertion on key(result.get(0)) (e.g.,
assertThat(key(...)).contains("idx_products_deleted_created_id")), keeping
existing helpers like logExplainResult and the existing assertion style.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.java`:
- Around line 222-252: Add tests that assert deterministic tie-breaker ordering
when like counts are equal: in ProductV1ApiE2ETest create two products with the
same like count (ensure they are saved in a known ID order) and call the
existing endpoints (both the overall list and the brand-filtered endpoint used
in returnsProductListFilteredByBrandAndSortedByLikesDesc) with sort=likes_desc;
then assert that when likeCount ties occur the secondary order is consistent
(e.g., by product ID or insertion order) by checking the exact sequence of
names/ids. Update or add test methods (e.g., extends
returnsProductListFilteredByBrandAndSortedByLikesDesc and a new overall-list
test) to set up the tied likes via likeService.like(...) and include assertions
that verify the deterministic tie-breaker ordering.
- Around line 58-62: The tearDown in ProductV1ApiE2ETest calls
RedisCleanUp.truncateAll(), which invokes serverCommands().flushAll() and clears
the entire Redis instance causing cross-test flakiness; change Redis cleanup to
only delete keys with the test's cache prefix (or switch tests to use isolated
Redis DB/containers) by replacing RedisCleanUp.truncateAll() usage with a scoped
delete-by-prefix implementation (keep reference to RedisCleanUp.truncateAll()
and serverCommands().flushAll() to locate the code) and update the tearDown in
ProductV1ApiE2ETest to call the new scoped cleanup method; add a new E2E
integration test that runs ProductV1ApiE2ETest and the other E2E class in
parallel and asserts neither class’ cache keys are removed by the other to
verify isolation.
---
Outside diff comments:
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.java`:
- Around line 47-58: The new non-nullable likeCount field on ProductModel
(private long likeCount) requires a DB migration before deploying; add a
migration script (Flyway/Liquibase) to create the column with a default and
backfill existing rows (e.g., ALTER TABLE products ADD COLUMN like_count BIGINT
DEFAULT 0), then a follow-up migration to set the NOT NULL constraint (ALTER
TABLE ... MODIFY/MATCH to NOT NULL) and optionally add indexes if needed; ensure
the migration files are committed and CI runs them, and verify post-migration
with a query like SELECT COUNT(*) FROM products WHERE like_count IS NULL to
confirm zero nulls before enabling the entity change.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java`:
- Around line 19-21: The PESSIMISTIC_WRITE method lacks a lock timeout hint;
update ProductJpaRepository.findByIdForUpdate to include a QueryHint for
"jakarta.persistence.lock.timeout" (set a sensible ms value consistent with the
project's pattern, e.g., "3000") so the DB lock will fail fast instead of
waiting indefinitely, and apply the same QueryHint addition to
UserCouponJpaRepository.findByIdAndUserIdForUpdate; keep the existing
`@Lock`(LockModeType.PESSIMISTIC_WRITE) and `@Query` unchanged, just add the
`@QueryHints` wrapper with the timeout hint.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceTest.java`:
- Around line 97-111: Update the existing
throwsConflictException_whenDataIntegrityViolationOccurs test to also assert the
product.likeCount remains unchanged (e.g.,
assertThat(product.getLikeCount()).isEqualTo(0)) after likeService.like(userId,
productId) throws the CoreException; ensure the product's initial like count is
set before invoking likeService and keep the mocks
(productRepository.findByIdForUpdate, likeRepository.findByUserIdAndProductId,
likeRepository.save throwing DataIntegrityViolationException) the same. Add a
new unit test for the "already liked" path (e.g., when
likeRepository.findByUserIdAndProductId returns Optional.of(existingLike)) that
asserts CoreException with ErrorType.CONFLICT is thrown by likeService.like and
that product.getLikeCount() remains unchanged there as well. Ensure both tests
reference the same product instance used by increaseLikeCount()/likeService.like
so the invariant can be verified reliably.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java`:
- Around line 79-85: applySorting currently returns an un-sorted PageRequest for
ProductSortType.LIKES_DESC, creating an implicit dependency on
ProductJpaRepository's JPQL ORDER BY; fix it by making the sorting explicit:
update applySorting(ProductSortType) to return a PageRequest with the
corresponding Sort for LIKES_DESC (e.g., Sort.by(Sort.Direction.DESC,
"likes").and(Sort.by(Sort.Direction.DESC, "id"))) or, if sorting by a computed
value in ProductJpaRepository is required, add a clear comment in
ProductService.getAll (or next to ProductJpaRepository query method) explaining
that LIKES_DESC is handled by the repository and ensure repository method
names/JPQL remain in sync with ProductSortType to avoid implicit coupling.
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductPageInfo.java`:
- Around line 16-23: from(Page<ProductInfo>) stores a direct reference to
page.getContent(), which risks mutable external changes; change
ProductPageInfo.from to wrap the content with an immutable defensive copy (e.g.,
List.copyOf(page.getContent())) when constructing the ProductPageInfo so the
internal content is fixed, and ensure the ProductPageInfo constructor/field uses
that copied list; also add a unit test that mutates the original Page content
after creating ProductPageInfo and asserts ProductPageInfo.content() remains
unchanged to verify immutability.
In `@apps/commerce-api/src/main/java/com/loopers/domain/like/LikeModel.java`:
- Around line 15-18: Remove the redundant single-column index idx_likes_user_id
from the LikeModel mapping because the unique constraint on (user_id,
product_id) already covers user_id as a leading column; keep only
idx_likes_product_id. Update the entity/class where the indexes are declared
(LikeModel) to drop `@Index`(name = "idx_likes_user_id", ...). Add tests that run
EXPLAIN for the repository methods findByUserId and findByUserIdAndProductId to
assert the query planner chooses the composite unique index (user_id,
product_id) for those queries and only keep idx_likes_user_id if EXPLAIN proves
it’s necessary. Ensure test assertions explicitly check the index name or index
usage in the EXPLAIN output.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java`:
- Around line 12-13: 해당 인터페이스의 메서드 계약이 명시적이지 않아 brandId의 null 의미가 불분명하므로
ProductRepository의 시그니처들 findAll(Pageable pageable, Long brandId) 및
findAllOrderByLikesDesc(Pageable pageable, Long brandId)에 파라미터에 `@Nullable` 애노테이션을
추가하고 Javadoc을 추가해 "brandId == null이면 전체 브랜드 조회"로 명확히 문서화하세요; 또한 구현체 변경 시 규약이 깨지지
않도록 brandId == null(전체 조회)와 brandId != null(브랜드 필터) 두 케이스를 검증하는 단위 테스트를 작성해 계약을
보장하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 783681fa-6d89-4e16-992b-4e9211ab30f4
📒 Files selected for processing (26)
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductPageInfo.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeModel.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeService.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/ProductService.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/interfaces/api/product/ProductAdminV1ApiSpec.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/ProductV1ApiSpec.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/support/config/CacheConfig.javaapps/commerce-api/src/test/java/com/loopers/application/order/ConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheTest.javaapps/commerce-api/src/test/java/com/loopers/domain/like/LikeServiceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductServiceTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/product/ProductQueryExplainTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/LikeV1ApiE2ETest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/ProductV1ApiE2ETest.javahttp/commerce-api/product-v1.httpmodules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java
💤 Files with no reviewable changes (1)
- modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java
|
|
||
| import java.time.ZonedDateTime; | ||
|
|
||
| @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# JsonTypeInfo CLASS 사용 지점과 Jackson default typing 설정을 확인한다.
rg -n -C3 'JsonTypeInfo\\.Id\\.CLASS|activateDefaultTyping\\(|DefaultTyping\\.NON_FINAL'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 261
🏁 Script executed:
#!/bin/bash
# JsonTypeInfo CLASS 사용과 Jackson default typing 설정을 확인한다.
# Rust regex 문법에 맞게 수정
echo "=== 1. JsonTypeInfo.Id.CLASS 검색 ==="
rg -n 'JsonTypeInfo\.Id\.CLASS' -A 2 -B 2
echo -e "\n=== 2. activateDefaultTyping 검색 ==="
rg -n 'activateDefaultTyping' -A 2 -B 2
echo -e "\n=== 3. DefaultTyping.NON_FINAL 검색 ==="
rg -n 'DefaultTyping\.NON_FINAL' -A 2 -B 2
echo -e "\n=== 4. ProductInfo.java 전체 내용 ==="
find . -name 'ProductInfo.java' -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2686
🏁 Script executed:
#!/bin/bash
# ProductInfo.java 파일 직접 확인
find . -name 'ProductInfo.java' -type f -exec wc -l {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
#!/bin/bash
# ProductInfo.java 내용 확인
find . -path '*product/ProductInfo.java' -type f -exec cat -n {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1333
🏁 Script executed:
#!/bin/bash
# CacheConfig 검색
find . -name '*CacheConfig.java' -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 162
🏁 Script executed:
#!/bin/bash
# CacheConfig.java 전체 내용 확인
cat -n apps/commerce-api/src/main/java/com/loopers/support/config/CacheConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4769
🏁 Script executed:
#!/bin/bash
# 보안/역직렬화 관련 테스트 검색
find . -name '*Test*.java' -o -name '*test*.java' | xargs rg -l 'deserial|security|malicious|polymorphic|CacheConfig' 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 504
🏁 Script executed:
#!/bin/bash
# 다른 Jackson 보안 설정 확인
rg -n 'ObjectMapper|JsonMapper|disableDefaultTyping|StdTypeResolverBuilder|TypeResolverBuilder' apps/commerce-api/src/main/java/com/loopers/support/config/ -A 1 -B 1Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2413
🏁 Script executed:
#!/bin/bash
# ProductInfo와 ProductPageInfo 사용처 확인 - API 응답/캐시 계층
rg -l 'ProductInfo|ProductPageInfo' apps/commerce-api/src/main/java --type java | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 783
🏁 Script executed:
#!/bin/bash
# Jackson 및 Redis 보안 관련 설정 확인
rg -n 'getPolymorphicTypeValidator|LaissezFaireSubTypeValidator|BasicPolymorphicTypeValidator|allowIfBaseType|allowIfClassIs' apps/commerce-api/ -A 2 -B 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 728
🏁 Script executed:
#!/bin/bash
# 캐시 관련 테스트 또는 보안 테스트 확인
find apps/commerce-api/src/test -name '*.java' -exec grep -l 'Cache\|Redis\|polymorphic\|deserial' {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 335
🏁 Script executed:
#!/bin/bash
# Spring Boot 및 Jackson 버전 확인 (pom.xml 또는 build.gradle)
find . -name 'pom.xml' -o -name 'build.gradle' | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# 프로젝트 루트의 build 파일에서 Jackson 버전 확인
cat pom.xml 2>/dev/null | grep -A 2 -B 2 'jackson' || cat build.gradle 2>/dev/null | grep -i 'jackson'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# ProductCacheTest 내용 확인
cat -n apps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 11211
🏁 Script executed:
#!/bin/bash
# 프로젝트 빌드 파일 위치 확인
find . -maxdepth 3 -type f \( -name 'pom.xml' -o -name 'build.gradle*' -o -name 'gradle.properties' \)Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 449
🏁 Script executed:
#!/bin/bash
# Jackson 의존성 확인
find . -name 'pom.xml' -exec cat {} \; 2>/dev/null | grep -A 5 'jackson-databind'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# 대안: 모든 pom.xml 찾아 Jackson 버전 확인
find . -type f -name 'pom.xml' | xargs grep -h 'jackson' 2>/dev/null | grep -E 'version|artifactId' | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
캐시 역직렬화 공격 표면을 줄이기 위해 JsonTypeInfo.Id.CLASS 제거가 필요하다.
운영 관점에서 Redis 캐시 값 변조 시 의도하지 않은 타입 역직렬화 위험이 있다. LINE 8의 @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) 어노테이션은 CacheConfig의 activateDefaultTyping(getPolymorphicTypeValidator(), NON_FINAL, ...)과 함께 작동하면서 캐시 페이로드의 @class 필드를 통해 임의 클래스 인스턴스화를 허용한다. Redis 접근 제어가 완벽하지 않은 환경에서는 캐시 데이터 변조로 보안 침해가 발생할 수 있다.
수정안:
@JsonTypeInfo어노테이션 제거CacheConfig에서 명시적 패키지 화이트리스트 기반PolymorphicTypeValidator또는 캐시별 직렬화기 도입- 악의적인
@class값을 포함한 캐시 페이로드 역직렬화가 거부되는지 검증하는 보안 회귀 테스트 추가
ProductCacheTest.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/application/product/ProductInfo.java`
at line 8, Remove the unsafe polymorphic type hint on ProductInfo by deleting
the `@JsonTypeInfo`(use = JsonTypeInfo.Id.CLASS) annotation from the ProductInfo
class, and instead harden CacheConfig by replacing activateDefaultTyping(...)
with an explicit PolymorphicTypeValidator that white-lists only trusted packages
(or register explicit serializers per cache) via
CacheConfig.getPolymorphicTypeValidator()/activateDefaultTyping changes; then
add a security regression test (e.g., extend ProductCacheTest) that attempts to
deserialize a manipulated cache payload containing a malicious "@class" and
asserts the deserialization is rejected/throws to ensure tampered cache entries
are not instantiated.
Comment on lines
24
to
+28
| @Transactional | ||
| @Caching(evict = { | ||
| @CacheEvict(cacheNames = "productDetail", key = "'product:detail:' + #productId"), | ||
| @CacheEvict(cacheNames = "productList", allEntries = true) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring Framework, does @CacheEvicton a@transactional method run after method return or after transaction commit, and what is the recommended way to evict cache strictly after commit?
💡 Result:
- When does
@CacheEvictrun? By default,@CacheEvictperforms the eviction after the annotated method is successfully invoked/returns (beforeInvocation=falseby default). (docs.spring.io) - Is that “after transaction commit”? Not necessarily.
@CacheEvictis not inherently transaction-phase aware; without extra support it runs “after method invocation”, which can still be before the surrounding@Transactionaladvice actually commits, depending on advisor ordering.
Recommended way to evict strictly after commit
Use Spring’s transaction-aware cache support so evictions are deferred to the transaction after-commit callback:
- Wrap your
CacheManagerwithTransactionAwareCacheManagerProxy, or use a transaction-aware cache/decorator (or enable “transactionAware” on cache managers that support it). Spring’s transaction-aware cache support performsput/evict/clearonly in the after-commit phase of a successful transaction. (docs.spring.io) - If your cache manager is an
AbstractTransactionSupportingCacheManager, you can typically enable this by settingsetTransactionAware(true). (stackoverflow.com)
Citations:
- 1: https://docs.spring.io/spring-framework/docs/4.1.0.RELEASE/javadoc-api/org/springframework/cache/annotation/CacheEvict.html?utm_source=openai
- 2: https://docs.spring.io/spring-framework/docs/4.3.0.RC1_to_4.3.0.RC2/Spring%20Framework%204.3.0.RC2/org/springframework/cache/transaction/package-summary.html
- 3: https://stackoverflow.com/questions/35549440/how-can-i-cacheevict-or-cacheput-after-transaction-commit
🏁 Script executed:
cat apps/commerce-api/src/main/java/com/loopers/domain/like/LikeService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2994
🏁 Script executed:
rg "TransactionAwareCacheManagerProxy|setTransactionAware|@TransactionalEventListener" --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
fd "CacheConfig|CacheManager|cache" --extension java | xargs grep -l "setTransactionAware\|TransactionAware" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 69
🏁 Script executed:
fd "application\.(yml|yaml|properties)" --type f | xargs cat 2>/dev/null | grep -A 10 -i cacheRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
fd ".*Config\\.java" --type f | xargs grep -l "cache\|Cache" 2>/dev/null | head -5 | xargs catRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4146
🏁 Script executed:
rg "@Bean.*CacheManager|@Bean.*CacheConfig|setTransactionAware" --type java -A 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
트랜잭션 commit 이후 무효화를 보장하도록 after-commit 패턴으로 변경해야 한다.
@CacheEvict는 기본적으로 메서드 반환 직후에 동작하므로, @Transactional이 감싼 트랜잭션의 commit 시점과 일치하지 않는다. 실제 운영에서는 like/unlike 트랜잭션이 아직 commit되지 않았는데 캐시가 먼저 비워지면, 동시 조회가 이전 상태를 DB에서 읽어 캐시에 다시 채우게 되어 최대 5분 동안 stale data가 남을 수 있다.
수정안은 두 가지다:
ProductService.evictProductListAfterCommit()처럼@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)로 옮기기- 또는 CacheConfig에서
RedisCacheManager를TransactionAwareCacheManagerProxy로 감싸거나 transaction-aware 설정 활성화
추가로 productList 전체 무효화(allEntries = true)를 매 like/unlike마다 실행하면 1분 TTL 캐시의 hit rate가 급격히 떨어진다. 관리자 수정 빈도가 낮은 시점이라면 현재 설정이 받아들여질 수 있지만, 사용자 like 빈도가 높으면 선택적 무효화(예: 브랜드별)로 전환을 고려해야 한다.
commit 전 재캐시 방지 검증: 쓰기 트랜잭션 진행 중 동시 조회를 붙여 캐시가 먼저 비워진 후 old value로 재캐시되지 않음을 테스트해달라.
Also applies to: 48-52
🤖 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 24 - 28, The current `@CacheEvict` on the like/unlike method in
LikeService executes before the transaction commit and can cause stale
recaching; change eviction to happen after commit by moving eviction logic to a
TransactionalEventListener(method like evictProductListAfterCommit annotated
with `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)) or
enable a transaction-aware cache manager (wrap RedisCacheManager with
TransactionAwareCacheManagerProxy or enable transaction-aware setting in
CacheConfig) and remove or replace the expensive allEntries = true on
"productList" with a more targeted eviction strategy (e.g., brand or
product-list segment key) to avoid huge hit-rate drops; finally add a
concurrency test that performs a like/unlike transaction while a concurrent read
triggers cache population to ensure the cache is not repopulated with the old
value before commit.
Comment on lines
+26
to
+27
| @CacheEvict(cacheNames = "productDetail", key = "'product:detail:' + #productId"), | ||
| @CacheEvict(cacheNames = "productList", allEntries = true) |
There was a problem hiding this comment.
좋아요 한 번마다 productList 전체 eviction은 캐시 목표를 스스로 무너뜨린다.
운영에서는 인기 상품의 like/unlike 한 번마다 모든 브랜드·페이지·정렬 조합의 list cache가 함께 비워져 read burst가 다시 DB로 쏠린다. 관리자 저빈도 수정과 달리 사용자 좋아요는 고빈도이므로, 수정안은 brand 범위나 versioned key 방식으로 eviction 범위를 좁히는 것이다. 추가 테스트로 brand A 상품의 like가 brand B의 list cache key를 보존하는지 검증해달라. Based on learnings, full cache invalidation was accepted only for low-frequency admin writes with a short TTL.
Also applies to: 50-51
🤖 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 26 - 27, The current `@CacheEvict` on LikeService evicts allEntries
from "productList" which causes full list cache thrashing; change it to evict
only the brand-scoped or versioned key instead of allEntries: replace
`@CacheEvict`(cacheNames = "productList", allEntries = true) with a targeted
eviction like `@CacheEvict`(cacheNames = "productList", key =
"'product:list:brand:' + `#product.brand.id`") or use a versioned key strategy
(e.g., include a productListVersionProvider token in the key and evict only that
token for the product's brand inside the like/unlike method such as likeProduct
or toggleLike), and add an automated test that likes a product of brand A and
asserts the cached entry for brand B's productList key remains present to verify
isolation.
Comment on lines
+30
to
31
| ProductModel product = productRepository.findByIdForUpdate(productId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 상품입니다.")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd 'ProductJpaRepository.java' -x sed -n '1,220p' {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2327
🏁 Script executed:
fd 'ProductJpaRepository.java' -x wc -l {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 182
🏁 Script executed:
fd 'ProductRepositoryImpl.java' -x head -100 {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1643
🏁 Script executed:
rg '@Lock' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 265
🏁 Script executed:
fd 'ProductJpaRepository.java' -x cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2327
🏁 Script executed:
fd 'LikeService.java' -x sed -n '25,60p' {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1686
🏁 Script executed:
fd 'OrderJpaRepository.java' -x rg '@QueryHints|@Lock' -A 3 {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
fd 'OrderJpaRepository.java' -x head -100 {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1602
🏁 Script executed:
rg '@QueryHints' -A 2 --type java | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg 'jakarta\.persistence\.lock\.timeout' -A 1 -B 1 --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg '@QueryHints' -B 2 -A 3 --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg '@Lock' -B 1 -A 3 --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1472
ProductJpaRepository.findByIdForUpdate()에 락 타임아웃 힌트를 추가해야 한다.
이 메서드는 LikeService.like(), LikeService.unlike()에서 호출되는 핫 경로이다. 현재 @Lock(LockModeType.PESSIMISTIC_WRITE) 만 있고 @QueryHints(QueryHint(name = "jakarta.persistence.lock.timeout", value = "3000"))가 없으면, 동일 상품에 대한 동시 요청에서 일부 스레드가 락을 기다리다 무기한 대기 상태에 빠질 수 있다. 프로젝트 표준에 맞게 다른 WithLock 메서드들과 동일한 3000ms 타임아웃 힌트를 적용하고, 동일 상품 동시 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/domain/like/LikeService.java`
around lines 30 - 31, Add a 3000ms pessimistic lock timeout hint to the
repository method used by the hot path: update
ProductJpaRepository.findByIdForUpdate() to include `@QueryHints`(QueryHint(name =
"jakarta.persistence.lock.timeout", value = "3000")) while keeping
`@Lock`(LockModeType.PESSIMISTIC_WRITE) so concurrent
LikeService.like()/LikeService.unlike() callers don’t block indefinitely; also
add an integration test that issues concurrent like/unlike requests against the
same product and asserts that a lock timeout results in the expected predictable
exception mapping (verify the service/exception translator maps the lock timeout
to the agreed application exception).
Comment on lines
+23
to
+28
| @RequestParam(defaultValue = "latest") String sort, | ||
| @RequestParam(required = false) Long brandId | ||
| ) { | ||
| ProductSortType sortType = ProductSortType.valueOf(sort.toUpperCase()); | ||
| Page<ProductV1Dto.ProductResponse> response = productFacade.getAll(pageable, sortType) | ||
| .map(ProductV1Dto.ProductResponse::from); | ||
| return ApiResponse.success(response); | ||
| ProductPageInfo pageInfo = productFacade.getAll(pageable, sortType, brandId); | ||
| return ApiResponse.success(ProductV1Dto.ProductListResponse.from(pageInfo)); |
There was a problem hiding this comment.
sort 파라미터의 유효성 검증이 누락되어 잠재적 500 오류가 발생할 수 있다.
ProductSortType.valueOf(sort.toUpperCase())는 유효하지 않은 정렬 값에 대해 IllegalArgumentException을 발생시킨다. 이 경우 클라이언트에게 400 Bad Request 대신 500 Internal Server Error가 반환될 수 있다.
수정안 (도메인 레이어에서 처리)
ProductSortType에 안전한 변환 메서드를 추가하거나, 컨트롤러에서 예외를 CoreException으로 변환하여 일관된 에러 응답을 제공해야 한다.
// ProductSortType.java에 추가
public static ProductSortType fromString(String value) {
try {
return valueOf(value.toUpperCase());
} catch (IllegalArgumentException e) {
throw new CoreException(ErrorCode.INVALID_PARAMETER, "Invalid sort type: " + value);
}
}프로젝트 규칙상 CoreException을 통한 일관된 에러 처리가 요구된다. Based on learnings: "enforce unified error handling by routing errors through CoreException to 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/interfaces/api/product/ProductV1Controller.java`
around lines 23 - 28, Replace the direct call to ProductSortType.valueOf(...) in
ProductV1Controller with a safe conversion that throws a CoreException on
invalid input: add a static ProductSortType.fromString(String) method in
ProductSortType that catches IllegalArgumentException and rethrows new
CoreException(ErrorCode.INVALID_PARAMETER, "Invalid sort type: " + value), then
update the controller to call ProductSortType.fromString(sort) (keeping existing
parameter names and usage of productFacade.getAll and
ProductV1Dto.ProductListResponse) so bad sort values produce a 400-style
CoreException and consistent ApiControllerAdvice handling.
Comment on lines
+200
to
+242
| @DisplayName("캐시 미스 시에도 서비스가 정상 동작한다") | ||
| @Nested | ||
| class CacheMiss { | ||
|
|
||
| @DisplayName("Redis 캐시가 비어 있어도 상품 상세 조회가 DB fallback으로 정상 동작한다.") | ||
| @Test | ||
| void detailQuery_worksWithoutCache() { | ||
| // arrange | ||
| redisCleanUp.truncateAll(); | ||
| BrandModel brand = brandJpaRepository.save(new BrandModel("나이키", "스포츠 브랜드")); | ||
| ProductModel product = productJpaRepository.save( | ||
| new ProductModel(brand, "에어맥스", 150000L, "설명", 100, ProductStatus.ON_SALE) | ||
| ); | ||
|
|
||
| // act | ||
| ProductInfo result = productFacade.getProduct(product.getId()); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(result).isNotNull(), | ||
| () -> assertThat(result.name()).isEqualTo("에어맥스") | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("Redis 캐시가 비어 있어도 상품 목록 조회가 DB fallback으로 정상 동작한다.") | ||
| @Test | ||
| void listQuery_worksWithoutCache() { | ||
| // arrange | ||
| redisCleanUp.truncateAll(); | ||
| BrandModel brand = brandJpaRepository.save(new BrandModel("나이키", "스포츠 브랜드")); | ||
| productJpaRepository.save(new ProductModel(brand, "에어맥스", 150000L, "설명", 100, ProductStatus.ON_SALE)); | ||
|
|
||
| PageRequest pageable = PageRequest.of(0, 20); | ||
|
|
||
| // act | ||
| ProductPageInfo result = productFacade.getAll(pageable, ProductSortType.LATEST, null); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(result).isNotNull(), | ||
| () -> assertThat(result.totalElements()).isEqualTo(1) | ||
| ); | ||
| } |
There was a problem hiding this comment.
캐시 에러 fallback 요구사항이 테스트로 보장되지 않는다.
운영 관점에서 Redis 장애/직렬화 예외 상황이 검증되지 않으면 실제 장애 시 조회 실패가 발생해도 사전에 탐지하지 못한다.
수정안으로 CacheManager 또는 Cache를 스파이/테스트 더블로 주입해 get/put에서 예외를 발생시키고, 서비스가 DB fallback으로 정상 응답하는지 검증해야 한다.
추가 테스트로 상세/목록 각각에 대해 캐시 연산 예외 발생 시 응답 성공과 데이터 정합성(상품명, totalElements)을 함께 확인하는 예외 경로 테스트를 추가해야 한다.
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/ProductCacheTest.java`
around lines 200 - 242, Add tests that explicitly simulate cache operation
failures by injecting a spy or test-double CacheManager/Cache that throws on
get/put and verifying service falls back to DB: create two new tests (one for
productFacade.getProduct(productId) and one for
productFacade.getAll(pageable,...)) in the CacheMiss nested class that replace
the real CacheManager/Cache with a spy which throws a RuntimeException on
get/put, then assert the calls still return non-null results with correct data
(compare result.name() == "에어맥스" for detail and result.totalElements() == 1 for
list) to ensure DB fallback; locate setup where productFacade is wired and the
existing redisCleanUp.truncateAll() to swap in the test double and restore
original cache after each test.
Comment on lines
+101
to
+104
| given(productRepository.findAll(pageable, null)).willReturn(productPage); | ||
|
|
||
| // act | ||
| Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LATEST); | ||
| Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LATEST, null); |
There was a problem hiding this comment.
브랜드 필터 전달 검증이 없어 회귀를 놓칠 가능성이 크다.
Line [104], Line [122]가 모두 null만 사용하여, brandId 전달이 누락되어도 테스트가 통과한다. 운영 관점에서는 브랜드 필터가 무시되면 잘못된 목록/캐시 키 경로가 유지되어 고트래픽 구간에서 오염 데이터가 확산될 수 있다.
수정안:
brandId != null케이스를LATEST,LIKES_DESC각각 추가한다.- 각 케이스에서 올바른 저장소 메서드와 인자가 호출되는지
verify한다. - 반대 분기 메서드 미호출(
never)도 함께 검증한다.
추가 테스트:
getAll(pageable, LATEST, brandId)→findAll(pageable, brandId)호출 +findAllOrderByLikesDesc미호출.getAll(pageable, LIKES_DESC, brandId)→findAllOrderByLikesDesc(pageable, brandId)호출 +findAll미호출.brandId == null기존 테스트는 회귀 방지용으로 유지한다.
🔧 제안 코드
+import static org.mockito.Mockito.never;
...
`@DisplayName`("기본 정렬로 조회하면, 페이징된 상품 목록을 반환한다.")
`@Test`
void returnsProductList_whenDefaultSort() {
// arrange
Pageable pageable = PageRequest.of(0, 20);
+ Object brandId = 1L;
List<ProductModel> products = List.of(
new ProductModel(brand, "에어맥스", 150000L, "나이키 에어맥스", 100, ProductStatus.ON_SALE),
new ProductModel(brand, "에어포스", 120000L, "나이키 에어포스", 50, ProductStatus.ON_SALE)
);
Page<ProductModel> productPage = new PageImpl<>(products, pageable, products.size());
- given(productRepository.findAll(pageable, null)).willReturn(productPage);
+ given(productRepository.findAll(pageable, brandId)).willReturn(productPage);
// act
- Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LATEST, null);
+ Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LATEST, brandId);
// assert
assertThat(result.getContent()).hasSize(2);
+ verify(productRepository).findAll(pageable, brandId);
+ verify(productRepository, never()).findAllOrderByLikesDesc(any(), any());
}
`@DisplayName`("LIKES_DESC 정렬로 조회하면, 좋아요 수 기준 정렬된 목록을 반환한다.")
`@Test`
void returnsProductList_whenSortByLikesDesc() {
// arrange
Pageable pageable = PageRequest.of(0, 20);
+ Object brandId = 1L;
List<ProductModel> products = List.of(
new ProductModel(brand, "에어맥스", 150000L, "나이키 에어맥스", 100, ProductStatus.ON_SALE)
);
Page<ProductModel> productPage = new PageImpl<>(products, pageable, products.size());
- given(productRepository.findAllOrderByLikesDesc(pageable, null)).willReturn(productPage);
+ given(productRepository.findAllOrderByLikesDesc(pageable, brandId)).willReturn(productPage);
// act
- Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LIKES_DESC, null);
+ Page<ProductModel> result = productService.getAll(pageable, ProductSortType.LIKES_DESC, brandId);
// assert
assertThat(result.getContent()).hasSize(1);
- verify(productRepository).findAllOrderByLikesDesc(pageable, null);
+ verify(productRepository).findAllOrderByLikesDesc(pageable, brandId);
+ verify(productRepository, never()).findAll(any(), any());
}As per coding guidelines **/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."
Also applies to: 119-126
🤖 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 101 - 104, Add tests verifying brandId is forwarded: extend
ProductServiceTest to include cases where brandId != null for both
ProductSortType.LATEST and LIKES_DESC. For getAll(pageable, LATEST, brandId)
assert productRepository.findAll(pageable, brandId) is called and
productRepository.findAllOrderByLikesDesc(...) is never called; for
getAll(pageable, LIKES_DESC, brandId) assert
productRepository.findAllOrderByLikesDesc(pageable, brandId) is called and
productRepository.findAll(...) is never called; keep the existing brandId ==
null test as regression protection. Use the same pageable, mock productPage, and
verify/never with the repository mocks and productService.getAll(...) calls.
Comment on lines
+90
to
+104
| @DisplayName("UC1: 전체 + 최신순 — idx_products_deleted_created_id 사용 확인") | ||
| @Test | ||
| void explain_allProducts_sortByLatest() { | ||
| String sql = "EXPLAIN SELECT p.id, p.name, p.price, p.like_count " + | ||
| "FROM products p " + | ||
| "WHERE p.deleted_at IS NULL " + | ||
| "ORDER BY p.created_at DESC, p.id DESC " + | ||
| "LIMIT 20"; | ||
|
|
||
| List<Object[]> result = executeExplain(sql); | ||
| logExplainResult("UC1: 전체 + 최신순", result); | ||
|
|
||
| String possibleKeys = possibleKeys(result.get(0)); | ||
| assertThat(possibleKeys).contains("idx_products_deleted_created_id"); | ||
| } |
There was a problem hiding this comment.
UC1, UC2 테스트의 인덱스 검증이 불충분하다.
possibleKeys는 옵티마이저가 고려 가능한 인덱스 목록일 뿐, 실제 사용된 인덱스가 아니다. UC4에서는 key()를 통해 실제 사용된 인덱스를 검증하는 반면, UC1과 UC2는 possibleKeys만 확인하고 있다.
운영 환경에서 데이터 분포나 통계 변화에 따라 옵티마이저가 다른 인덱스를 선택할 수 있으며, 이 경우 테스트는 통과하지만 실제로는 filesort가 발생할 수 있다.
수정안
- String possibleKeys = possibleKeys(result.get(0));
- assertThat(possibleKeys).contains("idx_products_deleted_created_id");
+ String key = key(result.get(0));
+ assertThat(key).contains("idx_products_deleted_created_id");
+
+ String extra = extra(result.get(0));
+ assertThat(extra).doesNotContain("Using filesort");UC2도 동일한 패턴으로 수정이 필요하다. 추가로 CI 환경과 운영 환경의 MySQL 버전 및 설정 차이에 따른 실행 계획 변화 가능성도 고려해야 한다.
📝 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
| @DisplayName("UC1: 전체 + 최신순 — idx_products_deleted_created_id 사용 확인") | |
| @Test | |
| void explain_allProducts_sortByLatest() { | |
| String sql = "EXPLAIN SELECT p.id, p.name, p.price, p.like_count " + | |
| "FROM products p " + | |
| "WHERE p.deleted_at IS NULL " + | |
| "ORDER BY p.created_at DESC, p.id DESC " + | |
| "LIMIT 20"; | |
| List<Object[]> result = executeExplain(sql); | |
| logExplainResult("UC1: 전체 + 최신순", result); | |
| String possibleKeys = possibleKeys(result.get(0)); | |
| assertThat(possibleKeys).contains("idx_products_deleted_created_id"); | |
| } | |
| `@DisplayName`("UC1: 전체 + 최신순 — idx_products_deleted_created_id 사용 확인") | |
| `@Test` | |
| void explain_allProducts_sortByLatest() { | |
| String sql = "EXPLAIN SELECT p.id, p.name, p.price, p.like_count " + | |
| "FROM products p " + | |
| "WHERE p.deleted_at IS NULL " + | |
| "ORDER BY p.created_at DESC, p.id DESC " + | |
| "LIMIT 20"; | |
| List<Object[]> result = executeExplain(sql); | |
| logExplainResult("UC1: 전체 + 최신순", result); | |
| String key = key(result.get(0)); | |
| assertThat(key).contains("idx_products_deleted_created_id"); | |
| String extra = extra(result.get(0)); | |
| assertThat(extra).doesNotContain("Using filesort"); | |
| } |
🤖 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/infrastructure/product/ProductQueryExplainTest.java`
around lines 90 - 104, The test currently asserts on possibleKeys(result.get(0))
which only lists candidate indexes; update UC1 (and UC2 similarly) to assert on
the actual index used by the optimizer by calling key(result.get(0)) instead of
possibleKeys(...). Locate the test method explain_allProducts_sortByLatest in
ProductQueryExplainTest, use executeExplain to get result and then replace the
possibleKeys assertion with an assertion on key(result.get(0)) (e.g.,
assertThat(key(...)).contains("idx_products_deleted_created_id")), keeping
existing helpers like logExplainResult and the existing assertion style.
Comment on lines
58
to
62
| @AfterEach | ||
| void tearDown() { | ||
| databaseCleanUp.truncateAllTables(); | ||
| redisCleanUp.truncateAll(); | ||
| } |
There was a problem hiding this comment.
테스트 종료 시 FLUSHALL을 호출하는 정리는 병렬 실행을 깨뜨린다.
운영/CI에서 같은 Redis 인스턴스를 공유하면 현재 정리는 다른 테스트 클래스의 키까지 같이 지워 플래키를 만든다. RedisCleanUp.truncateAll()이 실제로 serverCommands().flushAll()을 호출하므로, 수정안은 테스트별 Redis DB/컨테이너를 분리하거나 현재 캐시 prefix만 삭제하도록 범위를 좁히는 것이다. 추가 테스트로 두 E2E 클래스를 병렬 실행했을 때 서로의 캐시 키가 지워지지 않는지 검증해달라.
🤖 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 58 - 62, The tearDown in ProductV1ApiE2ETest calls
RedisCleanUp.truncateAll(), which invokes serverCommands().flushAll() and clears
the entire Redis instance causing cross-test flakiness; change Redis cleanup to
only delete keys with the test's cache prefix (or switch tests to use isolated
Redis DB/containers) by replacing RedisCleanUp.truncateAll() usage with a scoped
delete-by-prefix implementation (keep reference to RedisCleanUp.truncateAll()
and serverCommands().flushAll() to locate the code) and update the tearDown in
ProductV1ApiE2ETest to call the new scoped cleanup method; add a new E2E
integration test that runs ProductV1ApiE2ETest and the other E2E class in
parallel and asserts neither class’ cache keys are removed by the other to
verify isolation.
Comment on lines
+222
to
+252
| @DisplayName("brandId 필터와 likes_desc 정렬을 함께 조회하면, 해당 브랜드 내 좋아요 순으로 반환한다.") | ||
| @Test | ||
| void returnsProductListFilteredByBrandAndSortedByLikesDesc() { | ||
| // arrange | ||
| BrandModel nike = brandJpaRepository.save(new BrandModel("나이키", "스포츠 의류 및 신발 브랜드")); | ||
| BrandModel adidas = brandJpaRepository.save(new BrandModel("아디다스", "스포츠 의류 및 신발 브랜드")); | ||
| ProductModel nike1 = productJpaRepository.save(new ProductModel(nike, "나이키-1", 100000L, "desc", 100, ProductStatus.ON_SALE)); | ||
| ProductModel nike2 = productJpaRepository.save(new ProductModel(nike, "나이키-2", 120000L, "desc", 100, ProductStatus.ON_SALE)); | ||
| ProductModel adidas1 = productJpaRepository.save(new ProductModel(adidas, "아디다스-1", 110000L, "desc", 100, ProductStatus.ON_SALE)); | ||
| likeService.like(10L, nike2.getId()); | ||
| likeService.like(11L, nike2.getId()); | ||
| likeService.like(20L, adidas1.getId()); | ||
|
|
||
| // act | ||
| ParameterizedTypeReference<ApiResponse<Map<String, Object>>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<Map<String, Object>>> response = testRestTemplate.exchange( | ||
| ENDPOINT_PRODUCTS + "?page=0&size=20&sort=likes_desc&brandId=" + nike.getId(), | ||
| HttpMethod.GET, | ||
| null, | ||
| responseType | ||
| ); | ||
|
|
||
| // assert | ||
| List<Map<String, Object>> content = (List<Map<String, Object>>) response.getBody().data().get("content"); | ||
| assertAll( | ||
| () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK), | ||
| () -> assertThat(content).hasSize(2), | ||
| () -> assertThat(content.get(0).get("name")).isEqualTo("나이키-2"), | ||
| () -> assertThat(content.get(1).get("name")).isEqualTo("나이키-1") | ||
| ); | ||
| } |
There was a problem hiding this comment.
likes_desc의 동률 정렬 검증이 아직 없다.
운영에서는 같은 likeCount를 가진 상품이 많을수록 2차 정렬이 빠지면 페이지마다 순서가 흔들린다. 이번 PR이 ID 보조 정렬로 일관성을 맞추려는 목적이라면, 수정안으로 동률 상품 두 개를 만들어 ID 기준 순서를 고정하는 테스트를 추가해달라. 추가 테스트는 전체 조회와 brandId 필터 조회 둘 다 포함하는 편이 안전하다. 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/ProductV1ApiE2ETest.java`
around lines 222 - 252, Add tests that assert deterministic tie-breaker ordering
when like counts are equal: in ProductV1ApiE2ETest create two products with the
same like count (ensure they are saved in a known ID order) and call the
existing endpoints (both the overall list and the brand-filtered endpoint used
in returnsProductListFilteredByBrandAndSortedByLikesDesc) with sort=likes_desc;
then assert that when likeCount ties occur the secondary order is consistent
(e.g., by product ID or insertion order) by checking the exact sequence of
names/ids. Update or add test methods (e.g., extends
returnsProductListFilteredByBrandAndSortedByLikesDesc and a new overall-list
test) to set up the tied likes via likeService.like(...) and include assertions
that verify the deterministic tie-breaker ordering.
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
🧭 Context & Decision
문제 정의
선택지와 결정
🏗️ Design Overview
변경 범위
주요 컴포넌트 책임
ProductJpaRepository: 브랜드 필터/좋아요순 정렬 쿼리를 인덱스 친화적으로 제공한다.LikeService:: like/unlike 트랜잭션에서 like_count 동기화와 캐시 무효화를 수행한다.ProductFacade:: 상품 상세/목록 캐시 조회, TTL 키 전략 기반 캐시 적재를 담당한다.🔁 Flow Diagram
Main Flow
sequenceDiagram autonumber participant Client participant API as Product API participant Facade as ProductFacade(Cache) participant Service as ProductService/LikeService participant Redis participant DB as MySQL Client->>API: GET /products?brandId=10&sort=likes_desc&page=0&size=20 API->>Facade: getAll(pageable, LIKES_DESC, brandId) Facade->>Redis: cache get(product:list:brand:10:sort:LIKES_DESC:page:0:size:20) alt Cache Hit Redis-->>Facade: cached page Facade-->>API: response API-->>Client: 200 OK else Cache Miss Redis-->>Facade: miss Facade->>Service: getAll(...) Service->>DB: SELECT ... WHERE deleted_at IS NULL AND brand_id=? ORDER BY like_count DESC,id DESC DB-->>Service: page result (index scan) Service-->>Facade: page result Facade->>Redis: cache put(TTL 1m) Facade-->>API: response API-->>Client: 200 OK end