[5주차] - Read Optimization - 박주닮 - #46
Conversation
Walkthrough제품 엔터티에 likeCount 필드를 추가하고 관련 DB 인덱스들을 선언했으며, 페이지 요청에 brandId 필터를 추가해 컨트롤러 → PageCommand → Repository로 전달되도록 변경했습니다. LikeService가 좋아요 추가/삭제 시 ProductService를 호출해 product.likeCount를 갱신합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ProductV1Controller
participant DTO as PageRequestDto
participant Command as PageCommand
participant Repo as ProductQuerydslRepository
participant DB
Client->>Controller: GET /products?brandId=123&...
Controller->>DTO: 바인딩
Controller->>Command: new PageCommand(..., brandId=123)
Command->>Repo: fetchProducts(command)
Repo->>DB: SELECT ... WHERE deleted_at IS NULL AND brand_id = 123
DB-->>Repo: rows + totalCount
Repo-->>Controller: 페이지 결과 반환
Controller-->>Client: 200 OK
sequenceDiagram
participant Client
participant Controller as LikeController
participant LikeSvc as LikeService
participant LikeRepo as LikeRepository
participant ProductSvc as ProductService
participant ProductRepo as ProductRepository
Client->>Controller: POST /products/456/likes
Controller->>LikeSvc: addLike(productId=456, userId=...)
LikeSvc->>LikeRepo: save(Like...)
LikeRepo-->>LikeSvc: savedLike
LikeSvc->>ProductSvc: getProduct(456)
ProductSvc->>ProductRepo: findById(456)
ProductRepo-->>ProductSvc: product
LikeSvc->>ProductSvc: request incrementLikeCount()
ProductSvc->>ProductRepo: save(updated product)
ProductRepo-->>ProductSvc: savedProduct
LikeSvc-->>Controller: success
Controller-->>Client: 201 Created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt (1)
8-18: 브랜드 ID 인덱스 추가 방향은 적절해 보입니다
brandId로 필터링하는 조회/조인이 많다면,idx_product_brand_id인덱스 추가는 읽기 성능 개선에 도움이 될 것 같습니다. 다만 다음 두 가지는 한 번만 확인 부탁드립니다.
실제 컬럼명 일치 여부 확인 필요
- 현재 필드 정의는
@Column(nullable = false)만 있고name을 지정하지 않아, JPA 물리 네이밍 전략에 따라 컬럼명이 정해집니다.columnList = "brand_id"가 실제 DDL 상 컬럼명과 반드시 일치하는지(예: snake_case 전략 사용 여부, 커스텀 NamingStrategy 적용 여부)를 확인해 주세요. 불일치 시 스키마 생성/검증 단계에서 문제가 생길 수 있습니다.DB 마이그레이션 도구 사용 시 인덱스 스크립트 반영
- 운영 환경에서
spring.jpa.hibernate.ddl-auto를 끄고 Flyway/Liquibase 등으로 스키마를 관리한다면, 이 애노테이션 변경만으로는 실제 DB 인덱스가 생성되지 않습니다.- 동일한 인덱스를 마이그레이션 스크립트에도 추가해 두어야 운영 DB와 엔티티 정의가 일치합니다.
추가적인 인덱스 설계 관점(선택 사항)
- 향후에 정렬/페이지네이션 패턴이 명확해지면, 예를 들어
WHERE brand_id = ? ORDER BY id DESC와 같은 쿼리가 많다면(brand_id, id)복합 인덱스도 후보가 될 수 있습니다. 지금은 단일 컬럼 인덱스로 시작하고, 실제 쿼리 패턴과 실행 계획을 보면서 추가/수정하는 방식이 좋아 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/common/PageRequestDto.kt (1)
17-19: 선택적 개선: brandId 유효성 검증 추가 고려브랜드 ID 필터가 추가되었습니다. 현재는 null 허용이지만, 값이 제공될 경우 양수인지 검증하는 것을 고려해보세요.
다음과 같이 init 블록에 검증을 추가할 수 있습니다:
init { require(pageNumber >= 0) { "페이지 번호는 0 이상이어야 합니다." } require(pageSize > 0) { "페이지 크기는 1 이상이어야 합니다." } require(pageSize <= 100) { "페이지 크기는 100 이하여야 합니다." } + brandId?.let { require(it > 0) { "브랜드 ID는 양수여야 합니다." } } }apps/commerce-api/src/main/kotlin/com/loopers/domain/common/PageCommand.kt (1)
7-7: 선택적 개선: brandId 유효성 검증 추가 고려도메인 계층에서도 brandId가 제공될 경우 양수인지 검증하는 것을 고려해보세요.
다음과 같이 init 블록에 검증을 추가할 수 있습니다:
init { require(pageNumber >= 0) { "페이지 번호는 0 이상이어야 합니다." } require(pageSize > 0) { "페이지 크기는 1 이상이어야 합니다." } require(pageSize <= 100) { "페이지 크기는 100 이하여야 합니다." } + brandId?.let { require(it > 0) { "브랜드 ID는 양수여야 합니다." } } }apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/product/ProductQuerydslRepository.kt (1)
20-23: 선택적 개선: where 조건 순서 일관성count 쿼리의 where 조건 순서가 메인 쿼리(lines 44-47)와 다릅니다. 기능적으로는 동일하지만, 일관성을 위해 순서를 맞추는 것을 고려해보세요.
다음과 같이 순서를 통일할 수 있습니다:
.where( - pageCommand.brandId?.let { product.brandId.eq(it) }, product.deletedAt.isNull, + pageCommand.brandId?.let { product.brandId.eq(it) }, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/commerce-api/src/main/kotlin/com/loopers/domain/common/PageCommand.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/product/ProductQuerydslRepository.kt(4 hunks)apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/common/PageRequestDto.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/product/ProductV1Controller.kt(1 hunks)
🔇 Additional comments (4)
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/product/ProductV1Controller.kt (1)
71-71: LGTM!브랜드 ID 필터가 요청에서 커맨드로 올바르게 전달되고 있습니다.
apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/product/ProductQuerydslRepository.kt (3)
68-68: 정렬 필드 매핑 업데이트됨정렬 필드가
product.likeCount를 사용하도록 업데이트되었습니다. 이는 프로젝션 변경과 일관성이 있지만, lines 26-51에서 언급한likeCount필드의 존재 및 동기화 검증이 선행되어야 합니다.
26-51: I'll help you verify the concerns raised in this review comment about the denormalizedlikeCountfield. Let me search the codebase for the Product entity definition and related like operations.
<function_calls>
#!/bin/bashFind Product entity definition
fd -e kt -type f | xargs rg -l "class Product" | head -5
</function_calls>
<function_calls>
#!/bin/bashSearch for Product class and likeCount field
rg -A 30 "class Product\b" --type kt | head -100
</function_calls>
<function_calls>
#!/bin/bashCheck for likeCount field in codebase
rg "likeCount" --type kt -B 3 -A 3
</function_calls>
<function_calls>
#!/bin/bashSearch for like operations (create, delete, etc.)
rg -n "(Like|like)" --type kt | grep -i "fun|class|data class" | head -20
</function_calls>
<function_calls>
#!/bin/bashFind files in commerce-api infrastructure area
fd -type f -name "*.kt" | grep -E "(product|like)" | head -20
</function_calls>
20-23: 브랜드 ID 인덱스가 올바르게 추가되었습니다검증 완료: Product.kt 파일에서
@Index(name = "idx_product_brand_id", columnList = "brand_id")인덱스가@Table어노테이션에 정의되어 있습니다(lines 13-18). 이 인덱스는 ProductQuerydslRepository의findProducts()메서드에서brandId로 필터링하는 쿼리(lines 21, 46)의 성능을 보장합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/commerce-api/src/main/kotlin/com/loopers/domain/like/LikeService.kt (1)
19-28: 동시성 제어가 필요합니다 - 좋아요 카운트 업데이트에서 Lost Update 문제 발생 가능.여러 사용자가 동시에 같은 상품에 좋아요를 추가할 경우,
likeCount가 정확하게 증가하지 않을 수 있습니다.시나리오:
- 트랜잭션 A: 좋아요 저장 → 상품 조회(count=10) → count를 11로 증가
- 트랜잭션 B: 좋아요 저장 → 상품 조회(count=10) → count를 11로 증가
- 결과: count=11 (기대값: 12)
해결 방안:
Product엔티티에@Version필드를 추가하여 JPA 낙관적 잠금을 사용하거나, 비관적 잠금(@Lock(LockModeType.PESSIMISTIC_WRITE))을 사용해야 합니다.@Transactional fun addLike(userId: Long, productId: Long) { if (likeRepository.existsByUserIdAndProductId(userId, productId)) { return } likeRepository.save(Like.of(userId, productId)) - - val product = productService.getProduct(productId) + val product = productService.getProductWithLock(productId) product.incrementLikeCount() }ProductService에 비관적 잠금을 사용하는 메서드 추가:
@Lock(LockModeType.PESSIMISTIC_WRITE) @Query("SELECT p FROM Product p WHERE p.id = :id") fun findByIdWithLock(@Param("id") id: Long): Product?apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt (1)
1-32: Database migration scripts are missing and must be created before this change can be deployed to production.The verification confirms a critical issue: The project uses
spring.jpa.hibernate.ddl-auto: nonein production (seemodules/jpa/src/main/resourcesjpa.yml), which means Hibernate will not automatically create or update the database schema. Without migration scripts for the newlike_countcolumn and indexes, deploying this Product entity change to production will fail with column not found errors.Additionally, the project has no Flyway or Liquibase dependency configured in
apps/commerce-api/build.gradle.kts, and no SQL migration files exist in the repository.Required actions:
- Create a database migration script (V*.sql using Flyway or equivalent) to add the
like_countcolumn with default value 0 and the two indexes (idx_product_brand_id,idx_product_like_count)- Set up Flyway or Liquibase in the project if not already present
- Ensure migrations are applied before deploying this schema change to any environment using
ddl-auto: none
🧹 Nitpick comments (1)
apps/commerce-api/src/test/kotlin/com/loopers/domain/like/LikeServiceTest.kt (1)
20-40: 테스트 검증이 적절하지만, 동시성 테스트가 누락되었습니다.현재 테스트는 정상 흐름을 검증하지만,
LikeService.kt에서 식별된 동시성 문제를 확인하는 테스트가 없습니다. 여러 스레드에서 동시에 같은 상품에 좋아요를 추가하는 시나리오를 테스트하는 것을 권장합니다.동시성 테스트 예시를 생성하시겠습니까?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt(2 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/like/Like.kt(2 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/like/LikeService.kt(2 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt(2 hunks)apps/commerce-api/src/test/kotlin/com/loopers/domain/like/LikeServiceTest.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt (2)
apps/commerce-api/src/main/kotlin/com/loopers/domain/brand/Brand.kt (1)
validateName(30-34)apps/commerce-api/src/main/kotlin/com/loopers/domain/order/OrderItem.kt (1)
validatePrice(73-77)
🔇 Additional comments (6)
apps/commerce-api/src/main/kotlin/com/loopers/domain/like/Like.kt (1)
19-21: 좋아요 조회 성능 개선을 위한 인덱스 추가가 적절합니다.
product_id컬럼에 인덱스를 추가하여 특정 상품의 좋아요 수를 조회하는 쿼리의 성능이 개선됩니다.LikeRepository.countByProductId()같은 조회 쿼리에서 이 인덱스를 활용할 수 있습니다.apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt (2)
24-25: 읽기 성능 최적화를 위해 비정규화된 필드를 사용합니다.
product.likeCount를 직접 사용하여LikeService를 통한 집계 쿼리를 제거했습니다. 이는 읽기 성능을 크게 개선하지만,LikeService에서 좋아요 추가/삭제 시Product.likeCount를 올바르게 동기화하는 것이 중요합니다.
51-52: 일관성 있게 비정규화된 필드를 사용합니다.
updateProduct에서도 동일하게product.likeCount를 사용하여 읽기 최적화 패턴을 일관되게 적용했습니다.apps/commerce-api/src/test/kotlin/com/loopers/domain/like/LikeServiceTest.kt (1)
17-18: 테스트가 새로운 의존성을 올바르게 반영했습니다.
ProductService목(mock)을 추가하고LikeService에 주입하여 새로운 동작을 테스트할 수 있도록 구성했습니다.apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt (2)
30-31: 좋아요 수를 저장하는 필드가 적절하게 추가되었습니다.
likeCount필드를 추가하여 읽기 성능을 최적화했습니다. 기본값 0과nullable = false제약 조건이 적절합니다.
13-19:idx_product_like_count인덱스 재검토 결과: 인덱스는 필요합니다.검증 결과,
likeCount컬럼은 실제로 정렬 쿼리에 사용 중입니다:
- ProductV1Controller의
/api/v1/products/search엔드포인트에서 클라이언트가 sort 파라미터로likeCount를 지정하여 인기 상품순으로 조회할 수 있습니다 (lines 57-85).- ProductQuerydslRepository에서
likeCount가 유효한 정렬 필드로 구현되어 있습니다 (line 68).따라서 현재 인덱싱 전략은 적절합니다:
idx_product_brand_id: 브랜드별 필터링에 유용 ✓idx_product_like_count: 대중적 쿼리 패턴(정렬)을 지원하므로 유지 권장 ✓쓰기 성능 영향이 우려되면 향후 데이터 규모와 실제 정렬 쿼리 사용 빈도를 모니터링하여 필요 시 재평가하시기 바랍니다.
| package com.loopers.domain.like | ||
|
|
||
| import com.loopers.domain.product.ProductService | ||
| import org.springframework.stereotype.Component | ||
| import org.springframework.transaction.annotation.Transactional | ||
|
|
||
| @Component | ||
| class LikeService( | ||
| private val likeRepository: LikeRepository, | ||
| private val productService: ProductService, | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full LikeService.kt file to understand the changes
find . -name "LikeService.kt" -type fRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Find Product entity and like_count field references
find . -name "Product.kt" -o -name "Product.java" -type fRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Search for migration files and SQL schemas
find . -name "*.sql" -o -name "*migration*" -o -name "*schema*" | head -20Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Look for data initialization or test data files
find . -name "data.sql" -o -name "test-data.sql" -o -name "*init*.sql" -type fRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# List repository structure to understand layout
ls -laRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1563
🏁 Script executed:
#!/bin/bash
# Check git status and recent changes to understand PR scope
git log --oneline -10Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Search for all .kt files in the repository
find . -name "*.kt" -type f | head -30Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 2542
🏁 Script executed:
#!/bin/bash
# Search for SQL and database related files
find . -name "*.sql" -o -name "*.xml" | grep -E "(sql|database|schema|migration)" | head -20Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check all git branches
git branch -aRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Check current branch and status
git statusRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Look for any like-related files in the commerce-api
find ./apps/commerce-api -name "*ike*" -o -name "*product*" -o -name "*like*" 2>/dev/nullRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check git diff to see what's being changed in this PR
git diff --name-only HEADRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check for untracked files
git ls-files --others --exclude-standardRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Check out coderabbit_46 branch and look for LikeService.kt
git checkout coderabbit_46 2>&1
find . -name "LikeService.kt" -o -name "Product.kt" 2>/dev/null | head -20Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Check git log to see commits in the PR
git log --oneline -20Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Show git diff with stats
git diff main... --stat | head -30Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Look for all changes related to like or product
git diff main... --name-only | grep -E "(like|product|Like|Product)" -iRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/antml/bash
# Show files that differ between main and coderabbit_46
git log --oneline main..coderabbit_46Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1362
🏁 Script executed:
#!/bin/bash
# Read the LikeService.kt file
cat -n ./apps/commerce-api/src/main/kotlin/com/loopers/domain/like/LikeService.ktRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1471
🏁 Script executed:
#!/bin/bash
# Read the Product.kt file
cat -n ./apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.ktRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 2763
🏁 Script executed:
#!/bin/bash
# Search for migration files related to like or product
find . -path "*/db/migration/*" -o -path "*/resources/db/*" -o -name "*Like*" -o -name "*Product*" | grep -E "\.(sql|xml)" | head -20Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Look for JPA entity files
find ./apps/commerce-api -name "*Entity.kt" -o -name "*Jpa*.kt" | grep -E "(like|product)" -iRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 273
🏁 Script executed:
#!/bin/bash
# Look for Flyway or Liquibase migration files
find . -path "*/db/migration*" -type dRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Search for all SQL files in the project
find . -name "*.sql" -type f 2>/dev/null | grep -v node_modulesRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check the resources directory structure
find ./apps/commerce-api/src/main/resources -type f 2>/dev/null | head -30Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 144
🏁 Script executed:
#!/antml/bash
# Look for schema or initialization files
ls -la ./apps/commerce-api/src/main/resources/ 2>/dev/null || echo "resources dir not found"Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Read application.yml to understand JPA configuration
cat ./apps/commerce-api/src/main/resources/application.ymlRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1049
🏁 Script executed:
#!/bin/bash
# Search for CommandLineRunner or ApplicationRunner beans that might handle initialization
rg -n "CommandLineRunner|ApplicationRunner|@PostConstruct" --type=ktRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Look for any test data or initialization classes
find ./apps/commerce-api -name "*Init*" -o -name "*Fixture*" -o -name "*Seed*" 2>/dev/nullRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Check the commit that added the likeCount denormalization to see if migration was included
git show 89cedec --statRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Check the commit for like feature implementation
git show bbe4e98 --statRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1095
🏁 Script executed:
#!/bin/bash
# Find and read jpa.yml
find . -name "jpa.yml" -type fRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Search for any file that contains jpa configuration
rg -l "hibernate.ddl-auto|spring.jpa" --type=yamlRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Look at the modules structure for shared JPA config
ls -la ./modules/jpa/src/main/resources/ 2>/dev/null || echo "no resources"Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 257
🏁 Script executed:
#!/bin/bash
# Check if there are any schema initialization files
find ./modules -name "*.sql" -o -name "*schema*" -o -name "*init*" 2>/dev/nullRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Look at the full Product entity to understand initialization
git show 89cedec:apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt | head -40Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 1062
🏁 Script executed:
#!/bin/bash
# Read jpa.yml configuration
cat ./modules/jpa/src/main/resources/jpa.ymlRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 2236
🏁 Script executed:
#!/bin/bash
# Check the full git history of Product.kt to see when likeCount was added
git log --oneline --all -- "**/Product.kt" | head -10Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Look at the LikeService test to understand the expected behavior
find ./apps/commerce-api -name "*LikeServiceTest*" -o -name "*LikeService*Test*" 2>/dev/nullRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check when the Like entity was first introduced
git log --oneline --all -- "**/Like.kt" | head -10Repository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Read the LikeServiceTest to understand test coverage
cat -n ./apps/commerce-api/src/test/kotlin/com/loopers/domain/like/LikeServiceTest.ktRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 3313
🏁 Script executed:
#!/bin/bash
# Check the detailed changes in commit 89cedec to see if migration was mentioned
git show 89cedecRepository: Loopers-dev-lab/loopers-spring-kotlin-template
Length of output: 8050
기존 데이터의 일관성 확보가 필수입니다. 이 변경사항을 배포하기 전에 데이터 마이그레이션이 반드시 필요합니다.
이 커밋 (89cedec)은 상품 좋아요 수를 Product.likeCount 필드로 비정규화하는 변경입니다. 그러나 배포 환경 (dev/qa/prd)에서는 hibernate.ddl-auto: none으로 설정되어 있어 수동 마이그레이션이 필수입니다.
문제점:
- 변경 후 신규 좋아요는
Product.likeCount가 올바르게 증가/감소됩니다 - 하지만 기존 데이터의 경우, 새로운
like_count컬럼이 기본값 0으로 초기화되어 데이터 불일치 발생 - 이미 존재하는 Like 레코드들이
Product.likeCount에 반영되지 않음 - 좋아요 수로 정렬하는 기능 (
idx_product_like_count인덱스 사용)이 부정확한 결과 반환
필요한 마이그레이션:
UPDATE loopers_product p
SET like_count = (
SELECT COUNT(*)
FROM loopers_like l
WHERE l.product_id = p.id
);🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/like/LikeService.kt
around lines 1–11: the commit denormalizes like counts into Product.likeCount
but does not populate existing rows; before deploying you must add and run an
idempotent DB migration that sets product.like_count = count of existing
loopers_like rows per product, ensure the migration is managed by the project's
migration tool (Flyway/Liquibase) and added to the release, run it in dev/qa/prd
(back up DB first), verify counts and the idx_product_like_count-driven sorting
are correct after migration, and only then deploy the code that relies on
Product.likeCount.
| @Transactional | ||
| fun removeLike(userId: Long, productId: Long) { | ||
| val like = likeRepository.findByUserIdAndProductId(userId, productId) ?: return | ||
| likeRepository.delete(like) | ||
|
|
||
| val product = productService.getProduct(productId) | ||
| product.decrementLikeCount() | ||
| } |
There was a problem hiding this comment.
동일한 동시성 문제가 좋아요 취소에도 존재합니다.
removeLike에서도 addLike와 동일한 Lost Update 문제가 발생할 수 있습니다. 동일한 해결 방안을 적용해야 합니다.
@Transactional
fun removeLike(userId: Long, productId: Long) {
val like = likeRepository.findByUserIdAndProductId(userId, productId) ?: return
likeRepository.delete(like)
-
- val product = productService.getProduct(productId)
+ val product = productService.getProductWithLock(productId)
product.decrementLikeCount()
}| fun incrementLikeCount() { | ||
| this.likeCount++ | ||
| } | ||
|
|
||
| fun decrementLikeCount() { | ||
| if (this.likeCount > 0) { | ||
| this.likeCount-- | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
증가/감소 메서드 구현이 적절합니다.
incrementLikeCount와 decrementLikeCount 메서드가 명확하게 구현되었습니다. 특히 decrementLikeCount의 음수 방지 로직은 데이터 무결성을 보호합니다.
다만, 동시성 제어를 위해 Product 엔티티에 낙관적 잠금을 추가하는 것을 권장합니다.
+@Version
+var version: Long = 0
+
@Column(nullable = false)
var likeCount: Long = 0,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/product/Product.kt
around lines 39 to 47, the increment/decrement methods are fine but the entity
lacks optimistic locking; add a version field annotated with JPA's @Version
(e.g., a Long) to the Product entity, import the annotation, ensure the field is
persisted and initialized appropriately, update the database schema/migration to
include the version column, and keep the existing methods as-is so concurrent
updates will throw OptimisticLockException for callers to handle.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductService.kt (1)
71-75:deleteProductmethod missing cache eviction configuration causes data inconsistencyThe
deleteProductmethod (lines 71-75) lacks cache eviction annotations, creating a stale cache issue:
- Deleted products remain accessible via the
"Product"cache if previously retrieved- Deleted products may persist in the
"Products"paginated cache- This inconsistency contradicts the pattern established in
createProduct(line 43-56) andupdateProduct(line 58-69)The method should evict both caches, following the same pattern as
updateProduct:+ @Caching( + evict = [ + CacheEvict(cacheNames = ["Products"], allEntries = true), + CacheEvict(cacheNames = ["Product"], key = "#id"), + ], + ) @Transactional fun deleteProduct(id: Long) { val product = getProduct(id) product.delete() }This ensures cache consistency across create, update, and delete operations.
🧹 Nitpick comments (3)
apps/commerce-api/src/main/kotlin/com/loopers/domain/common/PageCommand.kt (1)
7-7: brandId 필드 추가를 승인하되, 선택적 유효성 검사를 고려하세요.brandId 필드가 잘 추가되었습니다. 다만, null이 아닌 경우 양수 검증을 추가하면 더 방어적인 코드가 됩니다.
필요시 다음 diff를 적용하여 유효성 검사를 추가할 수 있습니다:
init { require(pageNumber >= 0) { "페이지 번호는 0 이상이어야 합니다." } require(pageSize > 0) { "페이지 크기는 1 이상이어야 합니다." } require(pageSize <= 100) { "페이지 크기는 100 이하여야 합니다." } + brandId?.let { require(it > 0) { "브랜드 ID는 양수여야 합니다." } } }apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductService.kt (2)
34-41: PageCommand 기반 캐시 키(cacheKey) 구성 요소 누락 여부 확인 필요
getProductById(pageCommand)를"Products"캐시에pageCommand.cacheKey()로 키잉하는 것은 좋습니다만,cacheKey()안에 page, size, sort, brandId 등 페이징/필터링/정렬에 영향을 주는 필드가 모두 포함돼 있는지 꼭 확인해야 합니다. 누락 시 서로 다른 요청이 같은 캐시 엔트리를 공유하는 문제가 생길 수 있습니다.
43-63: 생성/수정 시 목록·단건 캐시 무효화 전략은 일관적이나, 비용·확장성은 고려 여지 있음
createProduct에서"Products"전체를allEntries = true로 비우는 것은 구현이 단순해 장점이 있지만, 캐시 키가 많아질 경우 비용이 커질 수 있습니다.updateProduct에서"Products"전체 + 해당"Product"(id 기반)를 함께 evict 하는 전략은 일관성이 좋습니다.현재 트래픽 규모에서는 문제 없을 수 있으나, 나중에 캐시 키 조합이 늘어날 경우 더 세분화된 invalidation 전략(예: 특정 필터 조합만 비우는 방식)을 고려해 볼 만합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/commerce-api/src/main/kotlin/com/loopers/domain/common/PageCommand.kt(2 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductService.kt(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-09T10:41:39.297Z
Learnt from: ghojeong
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 25
File: apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductRepository.kt:1-12
Timestamp: 2025-11-09T10:41:39.297Z
Learning: In this codebase, domain repository interfaces are allowed to use Spring Data's org.springframework.data.domain.Page and org.springframework.data.domain.Pageable types. This is an accepted architectural decision and should not be flagged as a DIP violation.
Applied to files:
apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductService.kt
🔇 Additional comments (2)
apps/commerce-api/src/main/kotlin/com/loopers/domain/product/ProductService.kt (2)
7-9: Spring Cache 관련 import 구성은 사용처와 잘 맞습니다.
Cacheable,CacheEvict,Caching사용이 서비스 레벨 캐싱 도입 의도와 일관되어 보입니다. 프로젝트 전반에서 동일한 캐시 설정/매니저를 사용하는지만 한 번만 확인해 주세요.
19-27: 단건 조회 캐시 도입 시 삭제/갱신 경로와의 일관성 재점검 권장
getProduct(id)에"Product"캐시를 붙인 것은 읽기 비중이 높다면 괜찮은 선택입니다. 다만 이후 모든 쓰기 경로(업데이트, 삭제 등)에서 해당 id에 대한"Product"캐시가 반드시 무효화되도록 설계되었는지 전체 흐름을 한 번 더 점검해 주세요. (아래deleteProduct코멘트 참고)
| fun cacheKey(): String { | ||
| return "${brandId}_${pageNumber}_${pageSize}_$sort" | ||
| } |
There was a problem hiding this comment.
캐시 키 생성 로직에 안정성 문제가 있습니다.
cacheKey() 메서드에 다음 문제가 있습니다:
- sort 리스트의 toString() 사용: 리스트의 기본 toString()은 예측 가능하고 안정적인 캐시 키 형식을 보장하지 않습니다.
- 정렬 조건 순서 민감성: 동일한 정렬 조건이 다른 순서로 제공되면 의미상 동일한 쿼리임에도 다른 캐시 키가 생성되어 캐시 미스가 발생합니다.
이는 캐시 효율성을 저하시키고 불필요한 캐시 미스를 유발합니다.
다음 diff를 적용하여 안정적인 캐시 키를 생성하세요:
fun cacheKey(): String {
- return "${brandId}_${pageNumber}_${pageSize}_$sort"
+ val sortKey = sort
+ .sortedWith(compareBy({ it.field }, { it.direction }))
+ .joinToString(",") { "${it.field}:${it.direction}" }
+ return "${brandId}_${pageNumber}_${pageSize}_${sortKey}"
}📝 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.
| fun cacheKey(): String { | |
| return "${brandId}_${pageNumber}_${pageSize}_$sort" | |
| } | |
| fun cacheKey(): String { | |
| val sortKey = sort | |
| .sortedWith(compareBy({ it.field }, { it.direction })) | |
| .joinToString(",") { "${it.field}:${it.direction}" } | |
| return "${brandId}_${pageNumber}_${pageSize}_${sortKey}" | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/common/PageCommand.kt
around lines 17 to 19, the cacheKey() uses the sort list's default toString()
which is order-sensitive and non-deterministic; change it to build a
deterministic, order-insensitive string by normalizing and serializing the sort
criteria: null-safe handle brandId/pageNumber/pageSize, sort each sort-entry by
a stable key (e.g., property name + direction), map each entry to a consistent
"field:direction" token, sort those tokens alphabetically, join with a delimiter
(e.g., comma) and use that joined string in the cache key so identical logical
sorts produce the same key regardless of input order.
1 participant
📌 Summary
브랜드 기반 페이징·정렬과 좋아요 기능 추가, 인덱스 및 캐시 적용으로 조회 성능이 개선
💬 Review Points
안녕하세요. 캐시 적용 및 Materialized View 설계 관련해서 궁금한 점이 있어 질문드립니다.
수정이 잦은 도메인에도 페이징 리스트 캐시를 적용하시는지 궁금합니다.
예를 들어 상품이나 게시글처럼 생성/수정/삭제가 빈번하고, 변경 사항이 리스트에 즉시 반영돼야 하는 경우에도 캐시를 활용하시는지요?
또한, 실시간성이 필요한 리스트 캐시에 대해 보편적으로 어떤 전략을 사용하시는지 궁금합니다.
Materialized View 개념에 대해 확인하고 싶습니다.
제가 이해하기로 Materialized View는 일반 MySQL View처럼 읽을 때마다 쿼리를 실행하는 게 아니라, 조회 결과를 별도 테이블 형태로 물리적으로 저장해두고, 데이터 변경 시마다 이를 갱신하는 구조라고 알고 있습니다.
그럼 Materialized View는 라이브러리나 Mysql에 있는 개념이 아니라 직접 Materialized View 테이블을 일반 테이블처럼 설계하고, 변경 이벤트에 맞춰 갱신 로직을 구현하는 방식인 것인가요??
✅ Checklist
🗞️ Coupon 도메인
🔖 Index
❤️ Structure
⚡ Cache
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.