[7팀 - 김현우] Round9 제출#179
[7팀 - 김현우] Round9 제출#179UjiinEatingTangerines wants to merge 5 commits intoLoopers-dev-lab:UjiinEatingTangerinesfrom
Conversation
CatalogEventType에 ORDER_COMPLETED를 추가하고, OrderCompletedEvent에 상품별 정보(productId, quantity)를 포함하여 CatalogEventOutboxAppender가 주문 상품별로 Outbox 레코드를 생성하도록 구현. ProductMetricsModel은 ORDER_COMPLETED 이벤트 수신 시 salesCount를 갱신한다.
ProductMetricsEventHandler가 이벤트 처리 시 RankingScorePolicy의 가중치(조회 0.1, 좋아요 0.2, 주문 0.7)를
적용하여 ZINCRBY로 일간 ZSET(ranking:all:{yyyyMMdd})에 점수를 누적한다.
키 최초 생성 시 TTL 2일을 설정하며, 기존 멱등성 체크가 ZSET 갱신에도 동일하게 적용된다.
GET /api/v1/rankings?date=yyyyMMdd&size=20&page=1 엔드포인트를 통해 일간 랭킹을 페이지 단위로
조회할 수 있으며, 상품 정보(이름, 가격, 브랜드명)가 Aggregation되어 제공된다.
상품 상세 조회(GET /api/v1/products/{id}) 시에도 오늘 기준 순위가 함께 반환되고,
순위에 없는 상품은 null로 표시된다.
Fake Repository 기반으로 이벤트 발행부터 ZSET 점수 반영, 조회까지의 전체 흐름을 검증한다. 주문 1건(0.7)이 좋아요 3건(0.6)보다 높은 순위를 갖는지, 날짜가 변경되어도 이전 날짜의 랭킹이 독립적으로 조회되는지, 좋아요 취소 시 점수가 정상 감소하는지를 확인한다.
매일 23:50에 실행하여 오늘 랭킹 점수의 10%를 내일 키에 미리 복사하는 Spring Batch Job을 구현. 자정 이후 빈 랭킹이 노출되는 콜드 스타트 문제를 완화하며, carry-over 가중치는 ranking.carry-over.weight 설정으로 조정할 수 있다.
📝 WalkthroughWalkthrough주문 완료 이벤트에 주문 항목 정보를 추가하고, 상품 상세 조회에 랭킹을 포함하며, Redis 기반 랭킹 시스템을 구현한다. 배치에서 일일 랭킹을 다음 날로 전이하고, 스트리머에서 이벤트 기반 점수 계산을 통해 랭킹을 실시간 업데이트한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as RankingV1Controller
participant Facade as RankingFacade
participant Service as RankingService
participant Repository as RankingRepository
participant Redis
participant ProductSvc as ProductService
participant BrandSvc as BrandService
Client->>Controller: GET /api/v1/rankings?date=20260410
Controller->>Facade: getRankings(date, page, size)
Facade->>Service: getTopRankings(date, page, size)
Service->>Repository: getTopN(key, offset, size)
Repository->>Redis: ZREVRANGE ranking:all:20260410
Redis-->>Repository: [(productId, score), ...]
Repository-->>Service: RankingEntry list
Service->>Repository: getTotalCount(key)
Repository->>Redis: ZCARD ranking:all:20260410
Redis-->>Repository: count
Repository-->>Service: total count
Service-->>Facade: RankingPage(entries, totalElements, ...)
Facade->>ProductSvc: findAllByIds(productIds)
ProductSvc-->>Facade: ProductModel list
Facade->>BrandSvc: findAllByIds(brandIds)
BrandSvc-->>Facade: BrandModel list
Facade->>Facade: map(RankingItemInfo)
Facade-->>Controller: RankingPageInfo
Controller-->>Client: ApiResponse(RankingPageInfo)
sequenceDiagram
participant EventStream as Kafka/EventStream
participant Handler as ProductMetricsEventHandler
participant Metrics as ProductMetricsJpaRepository
participant Policy as RankingScorePolicy
participant RankingRepo as RankingRepository
participant Redis
EventStream->>Handler: CatalogEventMessage(ORDER_COMPLETED)
Handler->>Metrics: persist metrics update
Metrics-->>Handler: saved
Handler->>Policy: calculateIncrement(ORDER_COMPLETED, delta)
Policy-->>Handler: score (delta * 0.7)
Handler->>RankingRepo: incrementScore(key, productId, score)
RankingRepo->>Redis: ZINCRBY ranking:all:20260410 score productId
Redis-->>RankingRepo: new score
RankingRepo->>Redis: getExpire(key)
alt No expiration set
RankingRepo->>Redis: EXPIRE key 172800
Redis-->>RankingRepo: success
end
RankingRepo-->>Handler: success
Handler-->>EventStream: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55분 검토 사항:
운영 관점 고려사항:
Possibly related PRs
Suggested reviewers
🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (15)
apps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt (1)
6-12: 랭킹 키 생성 규칙은 모듈 공용으로 단일화하는 편이 안전하다.운영 관점에서 배치/스트리머/API가 키 포맷을 각각 보유하면 일부 모듈만 변경될 때 키 불일치가 발생하고, 적재와 조회가 서로 다른 키를 보게 되어 랭킹 공백 장애로 이어진다. 수정안은
RankingKeyGenerator를 공용 모듈로 이동해 단일 소스를 사용하도록 정리하는 것이다. 추가 테스트로는 고정 날짜 입력에 대해 각 모듈이 동일 키(ranking:all:yyyyMMdd)를 반환하는 계약 테스트를 두는 것이다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt` around lines 6 - 12, The RankingKeyGenerator implementation (object RankingKeyGenerator with KEY_PREFIX, DATE_FORMATTER and dailyKey(LocalDate)) must be extracted to a shared/common module and referenced by batch/streamer/api to ensure a single source of truth; move the object into the common library, update callers to import and use RankingKeyGenerator.dailyKey(date) instead of local copies, and remove duplicated KEY_PREFIX/DATE_FORMATTER definitions in other modules; also add a small contract test that calls RankingKeyGenerator.dailyKey(fixedDate) from each module (or the shared API) and asserts the returned string equals "ranking:all:yyyyMMdd" for the fixed date to guarantee parity.apps/commerce-batch/src/main/resources/application.yml (1)
20-22: Carry-over 가중치는 환경변수 오버라이드 형태로 여는 편이 운영에 유리하다.운영 관점에서 랭킹 왜곡 이슈가 발생하면 재배포 없이 즉시 튜닝할 수 있어야 장애 대응 속도가 확보된다. 수정안은 정적 리터럴 대신 환경변수 기본값 패턴을 사용하는 것이다. 추가 테스트로는
RANKING_CARRY_OVER_WEIGHT주입 시 서비스가 해당 값을 실제로 사용함을 설정 바인딩 테스트로 검증하는 것이다.🔧 제안 수정안
ranking: carry-over: - weight: 0.1 + weight: ${RANKING_CARRY_OVER_WEIGHT:0.1}As per coding guidelines
**/application*.yml: "타임아웃, 커넥션 풀, 로깅 레벨 등 운영에 영향을 주는 설정 변경은 근거와 영향 범위를 요구한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/main/resources/application.yml` around lines 20 - 22, Replace the static literal for ranking.carry-over.weight with an environment-variable-backed default (e.g. use the Spring placeholder pattern ${RANKING_CARRY_OVER_WEIGHT:0.1}) so the weight can be overridden at runtime without redeploy; then add/update a configuration binding test that sets the RANKING_CARRY_OVER_WEIGHT env var (or System property) and asserts the application binding (the property that maps to ranking.carry-over.weight in your config class) actually reflects the injected value to ensure runtime override works.apps/commerce-streamer/src/main/resources/application.yml (1)
38-42: 랭킹 가중치도 운영 오버라이드 경로를 열어 두는 것이 안전하다.운영 관점에서 이벤트 편향이나 점수 폭주가 발생하면 즉시 가중치를 조정해야 하는데, 현재 정적 값만으로는 대응 시간이 길어진다. 수정안은 환경변수 기본값 패턴으로 바꾸고, 정책 클래스에서 음수/비정상 합계에 대한 시작 시 검증을 추가하는 것이다. 추가 테스트로는 잘못된 가중치 주입 시 애플리케이션 기동이 실패하는 설정 검증 테스트를 두는 것이다.
🔧 제안 수정안
ranking: weight: - view: 0.1 - like: 0.2 - order: 0.7 + view: ${RANKING_WEIGHT_VIEW:0.1} + like: ${RANKING_WEIGHT_LIKE:0.2} + order: ${RANKING_WEIGHT_ORDER:0.7}As per coding guidelines
**/application*.yml: "타임아웃, 커넥션 풀, 로깅 레벨 등 운영에 영향을 주는 설정 변경은 근거와 영향 범위를 요구한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/resources/application.yml` around lines 38 - 42, Change the static ranking weights in application.yml to environment-variable-backed defaults (e.g. view: ${RANKING_WEIGHT_VIEW:0.1}, like: ${RANKING_WEIGHT_LIKE:0.2}, order: ${RANKING_WEIGHT_ORDER:0.7}) so ops can override without rebuilding, and add startup validation in the config/policy layer: in the class that binds these properties (e.g., RankingProperties) or the policy class (e.g., RankingPolicy) implement a `@PostConstruct` (or equivalent init) check that throws IllegalStateException if any weight is negative or if the total weight is <= 0; also add a unit/integration test (e.g., RankingConfigValidationTest) that injects bad values and asserts the application/context fails to start to catch misconfiguration early.apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeTest.kt (1)
31-38: 테스트 의존성 주입이 프로덕션 코드와 동기화되었다.
RankingService의존성 추가로 테스트 구성이 올바르게 업데이트되었다. 다만getProductDetail테스트에서 반환된ProductInfo에ranking필드 검증이 없다.relaxed = true모킹으로 인해null이 반환될 것이므로, 기존 테스트에assertThat(result.ranking).isNull()검증을 추가하면 의도치 않은 회귀를 방지할 수 있다.💡 기존 테스트에 ranking 필드 검증 추가
// assert assertThat(result.name).isEqualTo(PRODUCT_NAME) assertThat(result.price).isEqualTo(PRODUCT_PRICE) assertThat(result.brandName).isEqualTo(BRAND_NAME) assertThat(result.likesCount).isEqualTo(5L) + assertThat(result.ranking).isNull() verify(exactly = 1) { productService.findById(PRODUCT_ID) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeTest.kt` around lines 31 - 38, The test adds a RankingService dependency (mockk(relaxed = true)) but the getProductDetail test doesn't assert the ProductInfo.ranking value; update the getProductDetail test in ProductFacadeTest to explicitly verify the ranking field returned by ProductFacade.getProductDetail (e.g., add assertThat(result.ranking).isNull()) so the relaxed mock's null ranking cannot silently regress behavior.apps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.kt (1)
10-13:quantity필드에 대한 유효성 검증 부재.
quantity가 0 또는 음수일 경우 랭킹 점수 계산에서 의도치 않은 결과가 발생할 수 있다. 이벤트 생성 시점 또는 소비 시점에서require(quantity > 0)검증을 추가하면 잘못된 데이터 유입을 조기에 차단할 수 있다.💡 init 블록에서 유효성 검증
data class OrderCompletedItem( val productId: Long, val quantity: Int, -) +) { + init { + require(quantity > 0) { "quantity must be positive: $quantity" } + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.kt` around lines 10 - 13, OrderCompletedItem 데이터 클래스의 quantity 필드에 유효성 검증이 빠져 있어 0 또는 음수 입력이 가능하므로, OrderCompletedItem에 init 블록을 추가해 require(quantity > 0)로 검증을 수행하여 생성 시점에 잘못된 값을 차단하고(예: IllegalArgumentException 발생), 필요하면 이벤트 소비 지점(consumer)에서도 동일 검증을 중복으로 적용해 방어적으로 처리하세요.apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt (1)
6-13: 공통 모듈로 추출하거나 동기화 검증 테스트를 추가해야 한다.현재 두 모듈의
RankingKeyGenerator구현은 일치하고 있으나, 코드 중복으로 인한 장기적 유지보수 리스크가 있다.운영 관점의 문제:
- commerce-api와 commerce-streamer가 각각 독립적으로 같은 로직을 관리하면, 미래에 한쪽만 포맷을 변경할 가능성이 높다
- 변경 누락 시 Redis 키 포맷이 불일치하여 데이터 조회 실패 상황이 발생할 수 있다
♻️ 해결안: 공통 모듈로 추출
modules/ranking-common같은 공유 모듈에 단일 정의하고 양쪽에서 의존하도록 변경한다.modules/ranking-common/ src/main/kotlin/com/loopers/ranking/RankingKeyGenerator.ktcommerce-api와 commerce-streamer는 이 모듈을 의존성으로 추가한다.
✅ 단기 대안: 동기화 검증 테스트
두 모듈의 키 포맷이 항상 일치함을 자동으로 검증하는 통합 테스트를 추가한다.
`@Test` fun rankingKeyFormat_shouldBeConsistentAcrosModules() { val testDate = LocalDate.of(2026, 4, 10) val apiKey = com.loopers.domain.ranking.RankingKeyGenerator.dailyKey(testDate) val streamerKey = com.loopers.streamer.domain.ranking.RankingKeyGenerator.dailyKey(testDate) assertEquals(apiKey, streamerKey, "키 포맷이 모듈 간 불일치") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt` around lines 6 - 13, RankingKeyGenerator 구현이 commerce-api와 다른 모듈에도 중복되어 있어 Redis 키 포맷 불일치 위험이 있으므로 RankingKeyGenerator 객체(특히 dailyKey, KEY_PREFIX, DATE_FORMATTER)를 공통 모듈로 추출하거나 당장 동기화 검증 테스트를 추가해 일관성을 보장하세요; 이상적인 해결책은 modules/ranking-common에 단일 RankingKeyGenerator 정의를 두고 양쪽에서 의존하도록 변경하며(참조: RankingKeyGenerator.dailyKey, KEY_PREFIX, DATE_FORMATTER), 단기 대안으로는 테스트에서 com.loopers.domain.ranking.RankingKeyGenerator.dailyKey와 다른 모듈의 RankingKeyGenerator.dailyKey를 동일한 LocalDate로 호출해 assertEquals로 검증하는 동기화 검증 테스트를 추가하십시오.apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingScorePolicy.kt (1)
7-19: 가중치 바인딩과 검증은 도메인 객체 밖으로 분리하는 편이 안전하다.현재 정책 객체가
@Value기본값에 직접 의존해 설정 누락을 조용히 삼키고, 음수나 비정상 가중치도 시작 시점에 막지 못한다.@ConfigurationProperties로 가중치를 묶어 검증한 뒤 순수한RankingScorePolicy에 주입하고, 잘못된 설정에서 애플리케이션이 즉시 실패하는 바인딩 테스트를 추가하는 편이 유지보수와 운영 안정성에 유리하다. 운영 설정이 누락되었을 때 기본값으로 계속 기동하는 것이 의도인지도 함께 확인이 필요하다. As per coding guidelines "비즈니스 규칙은 도메인에 두고, 인프라 관심사가 섞이면 분리하도록 제안한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingScorePolicy.kt` around lines 7 - 19, The RankingScorePolicy currently binds weights via `@Value` (viewWeight, likeWeight, orderWeight) which hides missing/invalid configs and allows negative/invalid weights; extract these into a `@ConfigurationProperties` class (e.g., RankingProperties) annotated with `@Validated` and appropriate constraints (e.g., `@Min`(0), `@NotNull`) to validate bindings at startup, inject that properties bean into RankingScorePolicy (remove `@Value` usage) so the policy remains a pure domain component, and add a binding test that supplies invalid/missing values to assert the application fails fast on bad configuration.apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt (1)
6-12: 랭킹 키 포맷은 모듈별 복제보다 공용 계약으로 고정하는 편이 안전하다.이 문자열 포맷이 이제 API·streamer·batch 사이의 저장소 계약이 되었는데 생성기를 모듈마다 따로 두면 prefix나 formatter가 한쪽만 바뀌어도 적재와 조회가 조용히 어긋난다. 공용 모듈로 승격해 단일 구현만 재사용하고, 계약 테스트도 한 곳에서만 유지하도록 정리하는 편이 유지보수와 운영 검증에 유리하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt` around lines 6 - 12, 현재 각 모듈에 따로 존재하는 랭킹 키 포맷 구현(RankingKeyGenerator의 KEY_PREFIX, DATE_FORMATTER, dailyKey)을 공용 계약으로 고정해야 합니다; RankingKeyGenerator를 공통 라이브러리(또는 shared module)로 승격해 단일 구현만 노출하고 각 모듈은 이 공용 클래스를 사용하도록 참조를 변경하며, 기존 로컬 구현들을 제거/대체하고 단일 위치에서 접두사(KEY_PREFIX)와 포맷터(DATE_FORMATTER)를 관리하도록 마이그레이션을 수행하고 동시에 해당 공용 구현에 대해 계약 테스트(예: dailyKey 출력 형식 검증)를 추가해 모듈 간 저장소 호환성을 보장하세요.apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt (1)
53-54: 상세 조회의 추가 Redis 왕복 비용은 별도 계측 대상으로 두는 편이 좋다.캐시 히트 경로도 이제 Redis RTT 하나를 항상 추가하므로 상세 QPS가 높아지면 p95/p99가 랭킹 저장소 상태에 바로 묶인다.
product_detail.ranking.lookup수준의 지표와 타임아웃을 분리해 보고, 필요하면 짧은 TTL 캐시나 bulk/pipeline 전략을 검토할 수 있도록 부하 테스트를 함께 남기는 편이 좋다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt` around lines 53 - 54, 현재 product 상세 조회에서 rankingService.getProductRank 호출이 항상 Redis 왕복을 추가하므로 이 RTT를 별도 계측하고 타임아웃을 분리하도록 수정하세요: productDetail flow에서 rankingService.getProductRank 호출 주변에 별도 메트릭(product_detail.ranking.lookup)을 기록하고 호출 타임아웃/결과를 productInfo.copy(ranking = ranking)에 반영하기 전에 타임아웃 실패시 폴백(예: null 또는 캐시 없는 기본값)을 적용하도록 구현하세요; 또한 짧은 TTL 캐시 또는 bulk/pipeline 전략을 적용할 수 있도록 관련 코드(rankingService.getProductRank)와 호출부(productInfo.copy(...))에 측정 포인트와 구성 가능한 타임아웃/TTL을 추가하고, 변경사항에 대해 부하 테스트 시나리오를 남기세요.apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/ranking/RankingV1ControllerTest.kt (1)
43-45: 테스트에서!!대신 명시적 null 처리로 실패 원인을 선명하게 해야 한다.운영 관점에서
!!는 실패 시 NPE로 원인을 흐릴 수 있어 테스트 디버깅 시간을 늘린다.val data = requireNotNull(response.data)로 단일 지점에서 null을 검증하고 이후 단언을 수행하도록 바꾸는 편이 안전하다.
추가 테스트로data가 null일 때 메시지가 명확히 드러나는 실패 시나리오(또는isNotNull이후 지역 변수 고정)를 포함해 회귀를 방지하는 것이 좋다.
As per coding guidelines'**/*.kt': null-safety를 최우선으로 점검하고, '!!'는 불가피한 경우에만 허용하며 근거를 요구한다.Also applies to: 56-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/ranking/RankingV1ControllerTest.kt` around lines 43 - 45, Replace unsafe non-null assertions with explicit null checks: capture response.data into a local non-null variable using val data = requireNotNull(response.data) (or assertNotNull with a message) in RankingV1ControllerTest, then perform assertions on data.content and data.content[0].rank instead of using response.data!!. Also apply the same change to the second occurrence (the assertions at the 56-58 block) so failures produce a clear message instead of an NPE and make it easier to debug broken tests.apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt (1)
30-45: 삭제/누락 상품 필터링 후 콘텐츠와 메타데이터 불일치를 줄이는 보정이 필요하다.운영 관점에서
mapNotNull로 항목이 빠지면 실제content크기와totalElements/체감 페이지 밀도가 어긋나 사용자에게 “빈 슬롯”처럼 보일 수 있다. 최소한 누락 ID를 랭킹 저장소에서 정리하는 후속 경로(프루닝) 또는 재조회 보정 전략을 추가해 메타데이터와 실제 응답 간 편차를 줄이는 것이 좋다.
추가 테스트로 “랭킹 엔트리 3개 중 1개가 삭제 상품” 시나리오에서 (1) 응답 콘텐츠 정합성, (2) 후속 요청에서 정리 반영 여부를 검증해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt` around lines 30 - 45, 현재 mapNotNull으로 삭제/누락 상품을 필터링하면 content 크기와 응답 메타(totalElements, page/size 기반 밀도)가 불일치해 사용자에겐 빈 슬롯으로 보일 수 있으니, RankingFacade의 변환 로직(rankingPage.entries -> content, RankingPageInfo 생성)을 수정해 누락된 항목을 보정하도록 하세요: 한 가지 간단한 옵션은 mapNotNull 이후 실제 content.size를 사용해 totalElements를 보정하거나(즉 RankingPageInfo에 전달하는 totalElements를 rankingPage.totalElements 대신 rankingPage.totalElements - removedCount 또는 content.size 기반으로 조정) 다른 옵션은 누락이 감지되면 추가 조회/재시도 로직을 호출해 page 채우기(재조회 함수/리포지토리 메서드 사용)입니다; 관련 심볼: rankingPage.entries, mapNotNull, RankingItemInfo, RankingPageInfo, totalElements, content를 찾아 위 두 방식 중 한 가지로 구현하고 관련 단위테스트(예: 3개 중 1개 삭제된 케이스)도 추가하세요.apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeRankingTest.kt (1)
34-90:LocalDate.now()직접 의존으로 테스트 플래키 가능성이 있다운영 관점에서 자정 경계 CI 실행 시 날짜 불일치로 간헐 실패가 발생하면 배포 파이프라인 신뢰도가 떨어진다. 수정안으로
ProductFacade에Clock을 주입하고LocalDate.now(clock)를 사용해 테스트에서 고정 시계를 주입하는 방식으로 결정론을 확보하는 것이 바람직하다. 추가 테스트로Asia/Seoul기준 자정 직전/직후 고정 시계 케이스를 넣어 랭킹 조회 날짜가 의도대로 계산되는지 검증하기 바란다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeRankingTest.kt` around lines 34 - 90, Tests call rankingService.getProductRank(LocalDate.now(), ...) which makes them flaky; modify ProductFacade to accept a java.time.Clock (constructor injection) and replace any LocalDate.now() calls inside ProductFacade with LocalDate.now(clock), then update tests to pass a fixed Clock (e.g., Clock.fixed(..., ZoneId.of("Asia/Seoul"))) when creating ProductFacade so you can deterministically assert ranking lookups and add boundary tests around Asia/Seoul midnight to verify date calculation.apps/commerce-streamer/src/test/kotlin/com/loopers/domain/metrics/ProductMetricsModelTest.kt (1)
47-62: 이벤트 카운터 테스트가 정상 경로 중심이라 경계/실패 회귀를 잡기 어렵다운영 관점에서 취소/보정 이벤트가 들어올 때(예:
LIKE_CHANGED음수 delta) 카운터 오염이 발생하면 지표와 랭킹 품질이 흔들리는데, 현재 케이스만으로는 이를 조기에 탐지하기 어렵다. 수정안으로 음수/0 delta에 대한 정책(감소 허용 또는 예외)을 명시적으로 검증하는 테스트를 추가하는 편이 좋다. 추가 테스트로LIKE_CHANGED(delta=-1),ORDER_COMPLETED(delta=0 또는 음수)시 기대 동작을 각각 분리해 검증하기 바란다.As per coding guidelines
**/*Test*.kt: 단위 테스트는 행위/경계값/실패 케이스를 포함하는지 점검한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/kotlin/com/loopers/domain/metrics/ProductMetricsModelTest.kt` around lines 47 - 62, Add boundary/failure tests to verify how ProductMetricsModel.apply handles non-positive deltas: create new test cases similar to appliesAllEventTypes that call model.apply with CatalogEventType.LIKE_CHANGED delta = -1 and assert expected behavior on likesCount (either decremented to min 0 or throw), and separate cases for ORDER_COMPLETED with delta = 0 and delta < 0 asserting salesCount behavior; reference ProductMetricsModel.apply, CatalogEventType.LIKE_CHANGED, CatalogEventType.ORDER_COMPLETED, and the counters likesCount, viewsCount, salesCount to locate where to add these assertions and ensure tests document the intended policy (decrement vs exception).apps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingFlowIntegrationTest.kt (1)
183-189: 동점 점수에서topN()순서가 비결정적이라 테스트가 플래키해질 수 있다
ConcurrentHashMap순회 순서는 보장되지 않아서 동일 점수 상품이 생기면 현재 구현의 결과 순서가 실행마다 달라질 수 있다. 운영 Redis ZSET는 동점에도 결정적 정렬이 있으므로, fake도 같은 tie-break 규칙을 흉내 내도록 보조 정렬 키를 명시하는 편이 안전하다. 예를 들어 점수 내림차순 뒤에 실제 운영 규칙과 동일한 보조 정렬을 추가하고, 동일 점수 두 상품의 순서를 검증하는 테스트를 함께 넣어 달라. As per coding guidelines "통합 테스트는 DB/외부 의존성 격리와 플래키 가능성을 점검하고, 테스트 데이터 준비/정리가 명확한지 본다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingFlowIntegrationTest.kt` around lines 183 - 189, The topN function uses an iteration over store (a ConcurrentHashMap) which makes ordering for equal scores nondeterministic; change topN(key: String, count: Int) to apply a stable deterministic tie-breaker after sorting by score (e.g., sortByDescending { it.value } then secondary sortBy { it.key } or equivalent) so equal-score entries have a defined order matching Redis ZSET tie-break rules, and update the related integration test to assert the expected deterministic ordering for tied scores; refer to the topN function and the store map when implementing this change.apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverIntegrationTest.kt (1)
91-94: 테스트에서!!로 실패 원인을 NPE로 바꾸지 않는 편이 좋다여기서 null이 나오면 실제 회귀는 "carry-over 결과가 기록되지 않음"인데, 현재는 NPE로만 보여 원인 파악이 늦어진다.
assertThat(...).isNotNull뒤에requireNotNull로 메시지를 명확히 남기거나, 점수 존재를 먼저 검증하는 assertion으로 바꿔 달라. 추가로 대상 상품이 누락됐을 때 어떤 키와 상품이 비어 있었는지 드러나는 실패 메시지를 검증하는 보조 케이스를 넣어두면 디버깅이 쉬워진다. As per coding guidelines "null-safety를 최우선으로 점검하고, '!!'는 불가피한 경우에만 허용하며 근거를 요구한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverIntegrationTest.kt` around lines 91 - 94, Replace the unsafe "!!" usages on fakeRepo.getScore results with explicit null-safety assertions: first assertThat(fakeRepo.getScore(tomorrowKey, 101L)).withFailMessage("missing score for key=%s id=%s", tomorrowKey, 101L).isNotNull (and similarly for 202L and 303L), then call requireNotNull(...) with a clear message or use the non-null value from the assertion for further checks; locate calls via RankingKeyGenerator.dailyKey and fakeRepo.getScore and remove the raw "!!". Also add a small helper test that intentionally simulates a missing product and asserts the failure message (including the key and product id) to make debugging missing carry-over entries easier.
🤖 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/kotlin/com/loopers/application/event/CatalogEventOutboxAppender.kt`:
- Around line 35-44: The current CatalogEventOutboxAppender creates a random
eventId per OrderCompletedEvent item which breaks idempotency; change the
eventId generation inside the loop that builds CatalogEventMessage so it is
deterministically derived from the original event (e.g., use a stable id
composed of orderId + productId, e.g., via a name-based UUID or hashing) instead
of UUID.randomUUID(), update the code path that constructs CatalogEventMessage
in CatalogEventOutboxAppender and the append(...) usage to use that
deterministic id, and add a unit/integration test that replays the same
OrderCompletedEvent twice and asserts consumers (or the outbox) only
produce/apply one event per orderId+productId to verify idempotency.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.kt`:
- Around line 7-8: The OrderCompletedEvent currently sets orderItems:
List<OrderCompletedItem> = emptyList(), which permits creating events with no
items and causes silent data loss downstream; fix by removing the default value
from the OrderCompletedEvent constructor so orderItems must be provided, or
alternatively add defensive handling in
CatalogEventOutboxAppender.appendOrderCompleted: check
event.orderItems.isEmpty(), log a warning with orderId (or throw) and return
early to prevent silent loss, and add tests that assert the warning/exception
when orderItems is empty; locate symbols OrderCompletedEvent and
CatalogEventOutboxAppender.appendOrderCompleted to implement the change.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt`:
- Around line 53-54: Product 상세 조회에서 rankingService.getProductRank 호출 예외가 전체 응답을
깨뜨리지 않도록 ProductFacade의 rankingService.getProductRank 호출을 runCatching/try-catch로
감싸 예외 발생 시 null로 폴백하고(또는 타임아웃/서킷브레이커로 보호) productInfo.copy(ranking = null)로
내려주도록 수정하고, 또한 rankingService가 예외를 던질 때에도 상세 응답은 200을 반환하고 ranking이 null임을 검증하는
단위/통합 테스트를 추가하시오; 관련 식별자는 ProductFacade, rankingService.getProductRank,
productInfo.copy(...)와 ProductViewedEvent를 참고해 수정 위치와 테스트 시나리오를 찾으세요.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt`:
- Around line 20-22: The early-return in RankingFacade that does `return
RankingPageInfo.empty(page, size)` when `rankingPage.entries.isEmpty()` drops
`totalElements` to 0; change the logic so that when `rankingPage.entries` is
empty you still build/return a RankingPageInfo which preserves
`rankingPage.totalElements` (i.e., clear only entries but copy
`rankingPage.totalElements` into the returned page instead of forcing 0), and
add a unit test for `rankingService.getTopRankings` that simulates `entries =
empty` with `totalElements > 0` and asserts the response keeps the original
`totalElements`.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingService.kt`:
- Around line 10-15: Validate inputs and prevent Int overflow in getTopRankings:
add a precondition like require(page >= 1 && size >= 1) at the start of
getTopRankings, then compute offset using long-safe multiplication (e.g.,
Math.multiplyExact((page - 1).toLong(), size.toLong())) before passing to
rankingRepository.getTopN; keep using RankingKeyGenerator.dailyKey(date) and
rankingRepository.getTotalCount(key) unchanged. Also add unit tests covering
page=0, size=0 and page=Int.MAX_VALUE boundary cases to assert an exception or
safe behavior.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/RankingRepositoryImpl.kt`:
- Around line 16-20: The mapNotNull in RankingRepositoryImpl silently drops
invalid ZSET tuples (inside the lambda that reads tuple.value and tuple.score
and returns RankingEntry), so change it to detect when tuple.value can't be
parsed or tuple.score is null, emit a warning log via the class logger with
context (the raw tuple, key/zone info if available) and increment a dedicated
metric counter (e.g., ranking.invalid_zset_entry) before returning null; keep
returning null for now but add the metric/log so these anomalies are observable
and optionally trigger a corrective path (extra fetch or alert) later. Also add
unit tests for the mapping logic that feed a tuple with value="abc" and with
score=null and assert the warning log was emitted and the metric counter
incremented.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1Controller.kt`:
- Around line 18-25: Update RankingV1Controller.getRankings to validate query
params in the controller: annotate the controller with `@Validated`, replace
manual LocalDate.parse by letting Spring convert the date using `@RequestParam`
`@DateTimeFormat`(pattern="yyyyMMdd") date: LocalDate, and add
`@RequestParam`(defaultValue="20") `@Min`(1) size: Int and
`@RequestParam`(defaultValue="1") `@Min`(1) page: Int (or use `@Positive`) so invalid
values are rejected before reaching rankingFacade; add handling in
ApiControllerAdvice to map DateTimeParseException (and
MethodArgumentNotValidException/ConstraintViolationException if not already
handled) to a 400 ApiResponse; finally extend tests for RankingV1Controller to
include invalid date format and boundary/negative/zero page and size cases to
assert 400 responses.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/application/order/OrderFacadeOrderEventTest.kt`:
- Around line 71-85: Add an assertion that publishing occurs exactly once and
ensure failure paths don't publish: after calling orderFacade.createOrder(...)
verify exact invocation with verify(exactly = 1) {
applicationEventPublisher.publishEvent(any<OrderCompletedEvent>()) } to lock the
contract for OrderCompletedEvent emission, and add a separate test for the
createOrder failure scenario to assert applicationEventPublisher.publishEvent is
NOT called (verify(exactly = 0) or verify {
applicationEventPublisher.publishEvent(any()) wasNot Called }) so
duplicate/ghost events are prevented; refer to orderFacade.createOrder and
applicationEventPublisher.publishEvent / OrderCompletedEvent to locate the
changes.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/ranking/RankingV1ControllerTest.kt`:
- Around line 48-55: Update the "defaultsToPage1Size20" test to actually
exercise the default-parameter path by calling controller.getRankings(date =
"20260410") without passing page or size, and verify that
rankingFacade.getRankings(any(), 1, 20) is invoked (use the existing
RankingPageInfo.empty(1, 20) stub). Also add a separate test that calls
controller.getRankings(date = "20260410", page = 2, size = 50) and verifies
delegation to rankingFacade.getRankings(any(), 2, 50) to ensure explicit
parameters override defaults.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/batch/job/ranking/step/RankingCarryOverTasklet.kt`:
- Around line 21-25: Make requestDate explicitly nullable and validate it at the
start of RankingCarryOverTasklet.execute: if requestDate is null or blank, throw
an IllegalArgumentException with a clear message like "requestDate is required";
when parsing LocalDate.parse(requestDate, DateTimeFormatter.BASIC_ISO_DATE) wrap
parsing in try/catch and on DateTimeParseException throw an
IllegalArgumentException with a message describing the invalid format (e.g.
"requestDate must be in yyyyMMdd format, got: <value>"); keep calling
rankingCarryOverService.execute(baseDate) only after successful
validation/parse. Also add unit tests for execute to assert exceptions for (1)
missing/null requestDate and (2) incorrectly formatted values such as
"2026-04-10".
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingCarryOverService.kt`:
- Around line 21-33: The carry-over is not idempotent:
RankingCarryOverService.execute calls rankingCarryOverRepository.carryOver with
sourceKey/destKey and carryOverWeight which causes duplicate accumulation if run
twice for the same baseDate; change the implementation to guard executions by
either (a) recording an execution marker for baseDate before performing
carryOver and skip if already present, or (b) write to a temporary dest key
(e.g., destKey + ".tmp") and then atomically rename/swap to the final destKey so
the operation can be retried safely; update RankingCarryOverService.execute to
use this marker or temp-key swap and adjust rankingCarryOverRepository.carryOver
or add a new repository method to perform the atomic swap, and add an automated
idempotency test that runs execute(baseDate) twice and asserts the same final
score as a single run.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/infrastructure/ranking/RankingCarryOverRepositoryImpl.kt`:
- Around line 20-29: 현재 reverseRangeWithScores로 조회한 members를 members.forEach에서
redisTemplate.opsForZSet().incrementScore로 하나씩 호출해 네트워크 왕복이 발생하므로 배치 지연 위험이 큽니다;
수정 방법은 RankingCarryOverRepositoryImpl의 해당 블록에서 단일 파이프라인 또는 서버사이드 Lua 스크립트로 모든
incrementScore 호출을 묶어 네트워크 왕복을 O(1)로 줄이고(대안으로 members를 적절한 청크로 나눠 pipelined
execution 반복), 기존 변수명(sourceKey, destKey, carryOverWeight, members, tuple,
redisTemplate)과 메서드(reverseRangeWithScores, incrementScore)를 사용해 파이프라인/스크립트 방식으로
치환하며 대용량(예: 100k) 통합 테스트를 추가해 처리 시간과 타임아웃을 검증하세요.
- Around line 19-35: The carryOver method is not idempotent and re-applying it
re-accumulates scores; fix by acquiring an idempotency marker (SETNX) keyed by a
deterministic marker name derived from sourceKey and destKey at the start of
carryOver (e.g. "carryover:marker:$sourceKey:$destKey"), only proceed with
reading and incrementing scores if SETNX returns success, and if successful set
a sensible expire on that marker so it auto-expires (use
redisTemplate.opsForValue().setIfAbsent(...) and then expire); if SETNX fails
immediately return 0 without touching destKey. Ensure the existing TTL logic for
destKey remains unchanged, and add a unit/integration test that calls
RankingCarryOverRepositoryImpl.carryOver twice with identical args and asserts
the second call returns 0 and destKey's score did not change.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverIntegrationTest.kt`:
- Around line 103-123: FakeCarryOverRepository currently only accumulates scores
and ignores TTL behavior; update the fake to simulate Redis TTLs by adding a map
tracking key -> expiry (or remaining TTL). In carryOver(sourceKey, destKey,
carryOverWeight) if destKey is newly created set its TTL to 2 days, if destKey
already has an expiry keep it unchanged; add helper methods to seed and read TTL
(e.g., seedTtl(key, expiryMillis) / getTtl(key)) so tests can assert both "no
TTL -> set 2 days" and "existing TTL -> preserved" scenarios while keeping
existing seedScore/getScore and carryOver semantics.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverServiceTest.kt`:
- Around line 36-47: The test returns 0 without verifying the repository
contract; update the test for returnsZeroWhenSourceIsEmpty to assert that
repository.carryOver("ranking:all:20260410", "ranking:all:20260411", 0.1) is
invoked exactly once (use verify(exactly = 1) on repository.carryOver) and add
confirmVerified(repository) to ensure no other interactions; also add/duplicate
a boundary test calling service.execute with a month/year-end date (e.g.,
LocalDate.of(2026, 12, 31)) to validate the next-day key calculation logic.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/application/metrics/ProductMetricsEventHandler.kt`:
- Around line 38-40: The code calls rankingRepository.incrementScore(...) (using
RankingKeyGenerator.dailyKey and rankingScorePolicy.calculateIncrement)
immediately after productMetricsJpaRepository.save(...), which can cause
inconsistent state if Redis updates succeed but the DB commit fails; move the
Redis/ranking update out of the DB transaction by scheduling it in an
afterCommit hook or emitting an outbox event consumed by a separate ranking
updater (so incrementScore runs only after JPA commit), update the handler to
enqueue that post-commit action instead of calling
rankingRepository.incrementScore directly, and add an integration test that
simulates "Redis success then DB commit failure" to verify no
duplicate/misaligned increments occur.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/metrics/ProductMetricsModel.kt`:
- Around line 59-63: ORDER_COMPLETED 분기에서 event.delta에 대한 유효성 검증이 빠져 salesCount가
음수/0으로 오염될 수 있으니 CatalogEventType.ORDER_COMPLETED 처리부에서 반드시 event.delta > 0인
경우에만 salesCount에 누적하고, 그렇지 않으면 도메인 예외(예: InvalidEventDeltaException)를 던져 이벤트 적용을
차단하도록 변경하세요; 변경 대상은 ProductMetricsModel의 when (event.eventType) 분기 내
ORDER_COMPLETED 처리 로직이며, 단위테스트를 추가해 delta가 음수 또는 0일 때 예외가 발생하고 정상 양수일 때만
salesCount가 증가하는 케이스를 검증하세요.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/infrastructure/ranking/RankingRepositoryImpl.kt`:
- Around line 19-23: The current incrementScore method issues two Redis calls
(ZINCRBY and GETEXPIRE/EXPIRE) causing extra QPS; replace the separate
operations in RankingRepositoryImpl.incrementScore by executing an atomic Lua
script via redisTemplate.execute that performs ZINCRBY and sets TTL only if the
key has no TTL (i.e., check TTL == -1 and call EXPIRE inside the script) using
the existing TTL constant; keep the same method signature but call the script
instead of redisTemplate.opsForZSet().incrementScore / getExpire / expire. Also
add integration tests for concurrent updates validating (1) TTL on keys that
already have TTL is not reset and (2) TTL is set after the first increment on
keys without TTL.
In
`@apps/commerce-streamer/src/test/kotlin/com/loopers/application/metrics/ProductMetricsEventHandlerRankingTest.kt`:
- Around line 109-139: The test currently only verifies
rankingRepository.incrementScore() was called once but doesn't assert that
ProductMetricsModel in metricsStore wasn't overwritten; update the test for
staleEventDoesNotIncrementScore to also verify the metricsStore entry for
productId 30L remains the same after handler.handle(staleEvent) (i.e., unchanged
ProductMetricsModel state) and assert productMetricsJpaRepository.save() was
called only once (or not called for the stale event), and add an additional test
variant where the stale event has a larger delta to ensure both metricsStore and
rankingRepository remain unchanged when handler.handle(...) is invoked with that
staleEvent.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.kt`:
- Around line 10-13: OrderCompletedItem 데이터 클래스의 quantity 필드에 유효성 검증이 빠져 있어 0 또는
음수 입력이 가능하므로, OrderCompletedItem에 init 블록을 추가해 require(quantity > 0)로 검증을 수행하여
생성 시점에 잘못된 값을 차단하고(예: IllegalArgumentException 발생), 필요하면 이벤트 소비 지점(consumer)에서도
동일 검증을 중복으로 적용해 방어적으로 처리하세요.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt`:
- Around line 53-54: 현재 product 상세 조회에서 rankingService.getProductRank 호출이 항상
Redis 왕복을 추가하므로 이 RTT를 별도 계측하고 타임아웃을 분리하도록 수정하세요: productDetail flow에서
rankingService.getProductRank 호출 주변에 별도 메트릭(product_detail.ranking.lookup)을 기록하고
호출 타임아웃/결과를 productInfo.copy(ranking = ranking)에 반영하기 전에 타임아웃 실패시 폴백(예: null 또는
캐시 없는 기본값)을 적용하도록 구현하세요; 또한 짧은 TTL 캐시 또는 bulk/pipeline 전략을 적용할 수 있도록 관련
코드(rankingService.getProductRank)와 호출부(productInfo.copy(...))에 측정 포인트와 구성 가능한
타임아웃/TTL을 추가하고, 변경사항에 대해 부하 테스트 시나리오를 남기세요.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt`:
- Around line 30-45: 현재 mapNotNull으로 삭제/누락 상품을 필터링하면 content 크기와 응답
메타(totalElements, page/size 기반 밀도)가 불일치해 사용자에겐 빈 슬롯으로 보일 수 있으니, RankingFacade의
변환 로직(rankingPage.entries -> content, RankingPageInfo 생성)을 수정해 누락된 항목을 보정하도록
하세요: 한 가지 간단한 옵션은 mapNotNull 이후 실제 content.size를 사용해 totalElements를 보정하거나(즉
RankingPageInfo에 전달하는 totalElements를 rankingPage.totalElements 대신
rankingPage.totalElements - removedCount 또는 content.size 기반으로 조정) 다른 옵션은 누락이
감지되면 추가 조회/재시도 로직을 호출해 page 채우기(재조회 함수/리포지토리 메서드 사용)입니다; 관련 심볼:
rankingPage.entries, mapNotNull, RankingItemInfo, RankingPageInfo,
totalElements, content를 찾아 위 두 방식 중 한 가지로 구현하고 관련 단위테스트(예: 3개 중 1개 삭제된 케이스)도
추가하세요.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt`:
- Around line 6-13: RankingKeyGenerator 구현이 commerce-api와 다른 모듈에도 중복되어 있어 Redis
키 포맷 불일치 위험이 있으므로 RankingKeyGenerator 객체(특히 dailyKey, KEY_PREFIX,
DATE_FORMATTER)를 공통 모듈로 추출하거나 당장 동기화 검증 테스트를 추가해 일관성을 보장하세요; 이상적인 해결책은
modules/ranking-common에 단일 RankingKeyGenerator 정의를 두고 양쪽에서 의존하도록 변경하며(참조:
RankingKeyGenerator.dailyKey, KEY_PREFIX, DATE_FORMATTER), 단기 대안으로는 테스트에서
com.loopers.domain.ranking.RankingKeyGenerator.dailyKey와 다른 모듈의
RankingKeyGenerator.dailyKey를 동일한 LocalDate로 호출해 assertEquals로 검증하는 동기화 검증 테스트를
추가하십시오.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeRankingTest.kt`:
- Around line 34-90: Tests call rankingService.getProductRank(LocalDate.now(),
...) which makes them flaky; modify ProductFacade to accept a java.time.Clock
(constructor injection) and replace any LocalDate.now() calls inside
ProductFacade with LocalDate.now(clock), then update tests to pass a fixed Clock
(e.g., Clock.fixed(..., ZoneId.of("Asia/Seoul"))) when creating ProductFacade so
you can deterministically assert ranking lookups and add boundary tests around
Asia/Seoul midnight to verify date calculation.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeTest.kt`:
- Around line 31-38: The test adds a RankingService dependency (mockk(relaxed =
true)) but the getProductDetail test doesn't assert the ProductInfo.ranking
value; update the getProductDetail test in ProductFacadeTest to explicitly
verify the ranking field returned by ProductFacade.getProductDetail (e.g., add
assertThat(result.ranking).isNull()) so the relaxed mock's null ranking cannot
silently regress behavior.
In
`@apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/ranking/RankingV1ControllerTest.kt`:
- Around line 43-45: Replace unsafe non-null assertions with explicit null
checks: capture response.data into a local non-null variable using val data =
requireNotNull(response.data) (or assertNotNull with a message) in
RankingV1ControllerTest, then perform assertions on data.content and
data.content[0].rank instead of using response.data!!. Also apply the same
change to the second occurrence (the assertions at the 56-58 block) so failures
produce a clear message instead of an NPE and make it easier to debug broken
tests.
In
`@apps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt`:
- Around line 6-12: The RankingKeyGenerator implementation (object
RankingKeyGenerator with KEY_PREFIX, DATE_FORMATTER and dailyKey(LocalDate))
must be extracted to a shared/common module and referenced by batch/streamer/api
to ensure a single source of truth; move the object into the common library,
update callers to import and use RankingKeyGenerator.dailyKey(date) instead of
local copies, and remove duplicated KEY_PREFIX/DATE_FORMATTER definitions in
other modules; also add a small contract test that calls
RankingKeyGenerator.dailyKey(fixedDate) from each module (or the shared API) and
asserts the returned string equals "ranking:all:yyyyMMdd" for the fixed date to
guarantee parity.
In `@apps/commerce-batch/src/main/resources/application.yml`:
- Around line 20-22: Replace the static literal for ranking.carry-over.weight
with an environment-variable-backed default (e.g. use the Spring placeholder
pattern ${RANKING_CARRY_OVER_WEIGHT:0.1}) so the weight can be overridden at
runtime without redeploy; then add/update a configuration binding test that sets
the RANKING_CARRY_OVER_WEIGHT env var (or System property) and asserts the
application binding (the property that maps to ranking.carry-over.weight in your
config class) actually reflects the injected value to ensure runtime override
works.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverIntegrationTest.kt`:
- Around line 91-94: Replace the unsafe "!!" usages on fakeRepo.getScore results
with explicit null-safety assertions: first
assertThat(fakeRepo.getScore(tomorrowKey, 101L)).withFailMessage("missing score
for key=%s id=%s", tomorrowKey, 101L).isNotNull (and similarly for 202L and
303L), then call requireNotNull(...) with a clear message or use the non-null
value from the assertion for further checks; locate calls via
RankingKeyGenerator.dailyKey and fakeRepo.getScore and remove the raw "!!". Also
add a small helper test that intentionally simulates a missing product and
asserts the failure message (including the key and product id) to make debugging
missing carry-over entries easier.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.kt`:
- Around line 6-12: 현재 각 모듈에 따로 존재하는 랭킹 키 포맷 구현(RankingKeyGenerator의 KEY_PREFIX,
DATE_FORMATTER, dailyKey)을 공용 계약으로 고정해야 합니다; RankingKeyGenerator를 공통 라이브러리(또는
shared module)로 승격해 단일 구현만 노출하고 각 모듈은 이 공용 클래스를 사용하도록 참조를 변경하며, 기존 로컬 구현들을
제거/대체하고 단일 위치에서 접두사(KEY_PREFIX)와 포맷터(DATE_FORMATTER)를 관리하도록 마이그레이션을 수행하고 동시에 해당
공용 구현에 대해 계약 테스트(예: dailyKey 출력 형식 검증)를 추가해 모듈 간 저장소 호환성을 보장하세요.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingScorePolicy.kt`:
- Around line 7-19: The RankingScorePolicy currently binds weights via `@Value`
(viewWeight, likeWeight, orderWeight) which hides missing/invalid configs and
allows negative/invalid weights; extract these into a `@ConfigurationProperties`
class (e.g., RankingProperties) annotated with `@Validated` and appropriate
constraints (e.g., `@Min`(0), `@NotNull`) to validate bindings at startup, inject
that properties bean into RankingScorePolicy (remove `@Value` usage) so the policy
remains a pure domain component, and add a binding test that supplies
invalid/missing values to assert the application fails fast on bad
configuration.
In `@apps/commerce-streamer/src/main/resources/application.yml`:
- Around line 38-42: Change the static ranking weights in application.yml to
environment-variable-backed defaults (e.g. view: ${RANKING_WEIGHT_VIEW:0.1},
like: ${RANKING_WEIGHT_LIKE:0.2}, order: ${RANKING_WEIGHT_ORDER:0.7}) so ops can
override without rebuilding, and add startup validation in the config/policy
layer: in the class that binds these properties (e.g., RankingProperties) or the
policy class (e.g., RankingPolicy) implement a `@PostConstruct` (or equivalent
init) check that throws IllegalStateException if any weight is negative or if
the total weight is <= 0; also add a unit/integration test (e.g.,
RankingConfigValidationTest) that injects bad values and asserts the
application/context fails to start to catch misconfiguration early.
In
`@apps/commerce-streamer/src/test/kotlin/com/loopers/domain/metrics/ProductMetricsModelTest.kt`:
- Around line 47-62: Add boundary/failure tests to verify how
ProductMetricsModel.apply handles non-positive deltas: create new test cases
similar to appliesAllEventTypes that call model.apply with
CatalogEventType.LIKE_CHANGED delta = -1 and assert expected behavior on
likesCount (either decremented to min 0 or throw), and separate cases for
ORDER_COMPLETED with delta = 0 and delta < 0 asserting salesCount behavior;
reference ProductMetricsModel.apply, CatalogEventType.LIKE_CHANGED,
CatalogEventType.ORDER_COMPLETED, and the counters likesCount, viewsCount,
salesCount to locate where to add these assertions and ensure tests document the
intended policy (decrement vs exception).
In
`@apps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingFlowIntegrationTest.kt`:
- Around line 183-189: The topN function uses an iteration over store (a
ConcurrentHashMap) which makes ordering for equal scores nondeterministic;
change topN(key: String, count: Int) to apply a stable deterministic tie-breaker
after sorting by score (e.g., sortByDescending { it.value } then secondary
sortBy { it.key } or equivalent) so equal-score entries have a defined order
matching Redis ZSET tie-break rules, and update the related integration test to
assert the expected deterministic ordering for tied scores; refer to the topN
function and the store map when implementing this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c60cb037-697d-460d-8830-d70f49768623
📒 Files selected for processing (43)
.http/ranking/getRankings.httpapps/commerce-api/src/main/kotlin/com/loopers/application/event/CatalogEventOutboxAppender.ktapps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.ktapps/commerce-api/src/main/kotlin/com/loopers/application/order/OrderFacade.ktapps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.ktapps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductInfo.ktapps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.ktapps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingInfo.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingRepository.ktapps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingService.ktapps/commerce-api/src/main/kotlin/com/loopers/infrastructure/ranking/RankingRepositoryImpl.ktapps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/ranking/RankingV1Controller.ktapps/commerce-api/src/test/kotlin/com/loopers/application/event/CatalogEventOutboxAppenderOrderTest.ktapps/commerce-api/src/test/kotlin/com/loopers/application/order/OrderFacadeOrderEventTest.ktapps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeRankingTest.ktapps/commerce-api/src/test/kotlin/com/loopers/application/product/ProductFacadeTest.ktapps/commerce-api/src/test/kotlin/com/loopers/application/ranking/RankingFacadeTest.ktapps/commerce-api/src/test/kotlin/com/loopers/domain/ranking/RankingServiceTest.ktapps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/ranking/RankingV1ControllerTest.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/ranking/RankingCarryOverJobConfig.ktapps/commerce-batch/src/main/kotlin/com/loopers/batch/job/ranking/step/RankingCarryOverTasklet.ktapps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingCarryOverRepository.ktapps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingCarryOverService.ktapps/commerce-batch/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.ktapps/commerce-batch/src/main/kotlin/com/loopers/infrastructure/ranking/RankingCarryOverRepositoryImpl.ktapps/commerce-batch/src/main/resources/application.ymlapps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverIntegrationTest.ktapps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverServiceTest.ktapps/commerce-streamer/src/main/kotlin/com/loopers/application/metrics/ProductMetricsEventHandler.ktapps/commerce-streamer/src/main/kotlin/com/loopers/domain/metrics/ProductMetricsModel.ktapps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingKeyGenerator.ktapps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingRepository.ktapps/commerce-streamer/src/main/kotlin/com/loopers/domain/ranking/RankingScorePolicy.ktapps/commerce-streamer/src/main/kotlin/com/loopers/infrastructure/ranking/RankingRepositoryImpl.ktapps/commerce-streamer/src/main/resources/application.ymlapps/commerce-streamer/src/test/kotlin/com/loopers/application/metrics/ProductMetricsEventHandlerRankingTest.ktapps/commerce-streamer/src/test/kotlin/com/loopers/application/metrics/ProductMetricsEventHandlerTest.ktapps/commerce-streamer/src/test/kotlin/com/loopers/domain/metrics/ProductMetricsModelTest.ktapps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingFlowIntegrationTest.ktapps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingKeyGeneratorTest.ktapps/commerce-streamer/src/test/kotlin/com/loopers/domain/ranking/RankingScorePolicyTest.ktmodules/kafka/src/main/kotlin/com/loopers/config/kafka/event/CatalogEventMessage.kt
| event.orderItems.forEach { item -> | ||
| val message = CatalogEventMessage( | ||
| eventId = UUID.randomUUID().toString(), | ||
| productId = item.productId, | ||
| eventType = CatalogEventType.ORDER_COMPLETED, | ||
| delta = item.quantity.toLong(), | ||
| version = System.currentTimeMillis(), | ||
| occurredAt = ZonedDateTime.now(), | ||
| ) | ||
| append(message) |
There was a problem hiding this comment.
주문 항목별 eventId를 랜덤으로 만들면 중복 처리 방지가 깨진다.
현재는 같은 OrderCompletedEvent가 재발행되거나 재시도되어도 각 항목에 새 UUID가 발급되어 하위 소비자가 모두 신규 이벤트로 처리한다. 운영에서는 salesCount와 랭킹 점수가 함께 부풀 수 있으니 orderId + productId처럼 원본 이벤트에서 재현 가능한 식별자로 eventId를 만들고, 동일 주문 이벤트를 두 번 흘려도 한 번만 반영되는 테스트를 추가하는 편이 안전하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/event/CatalogEventOutboxAppender.kt`
around lines 35 - 44, The current CatalogEventOutboxAppender creates a random
eventId per OrderCompletedEvent item which breaks idempotency; change the
eventId generation inside the loop that builds CatalogEventMessage so it is
deterministically derived from the original event (e.g., use a stable id
composed of orderId + productId, e.g., via a name-based UUID or hashing) instead
of UUID.randomUUID(), update the code path that constructs CatalogEventMessage
in CatalogEventOutboxAppender and the append(...) usage to use that
deterministic id, and add a unit/integration test that replays the same
OrderCompletedEvent twice and asserts consumers (or the outbox) only
produce/apply one event per orderId+productId to verify idempotency.
| val orderItems: List<OrderCompletedItem> = emptyList(), | ||
| ) |
There was a problem hiding this comment.
orderItems의 기본값 emptyList()는 운영 장애를 유발할 수 있다.
운영 관점에서 문제가 되는 이유:
OrderCompletedEvent(orderId, userId, totalAmount)형태로orderItems없이 이벤트를 생성할 수 있다CatalogEventOutboxAppender.appendOrderCompleted는event.orderItems.forEach로 순회하므로, 빈 리스트면 outbox 메시지가 0건 생성된다- 결과적으로 주문은 완료되지만 랭킹 점수가 누적되지 않는 사일런트 데이터 유실이 발생한다
- 모니터링 없이는 문제 감지가 불가능하다
🛡️ 수정안: 기본값 제거 또는 방어 로직 추가
Option 1: 기본값 제거 (권장)
data class OrderCompletedEvent(
val orderId: Long,
val userId: Long,
val totalAmount: Long,
- val orderItems: List<OrderCompletedItem> = emptyList(),
+ val orderItems: List<OrderCompletedItem>,
)Option 2: 다운스트림에서 방어 로깅 추가
CatalogEventOutboxAppender.appendOrderCompleted에 추가:
`@EventListener`
fun appendOrderCompleted(event: OrderCompletedEvent) {
if (event.orderItems.isEmpty()) {
log.warn("OrderCompletedEvent with empty orderItems received: orderId=${event.orderId}")
return
}
// ...
}추가 테스트: orderItems가 빈 리스트일 때 경고 로그 또는 예외 발생을 검증하는 테스트 추가 필요
📝 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.
| val orderItems: List<OrderCompletedItem> = emptyList(), | |
| ) | |
| val orderItems: List<OrderCompletedItem>, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/event/UserActionEvents.kt`
around lines 7 - 8, The OrderCompletedEvent currently sets orderItems:
List<OrderCompletedItem> = emptyList(), which permits creating events with no
items and causes silent data loss downstream; fix by removing the default value
from the OrderCompletedEvent constructor so orderItems must be provided, or
alternatively add defensive handling in
CatalogEventOutboxAppender.appendOrderCompleted: check
event.orderItems.isEmpty(), log a warning with orderId (or throw) and return
early to prevent silent loss, and add tests that assert the warning/exception
when orderItems is empty; locate symbols OrderCompletedEvent and
CatalogEventOutboxAppender.appendOrderCompleted to implement the change.
| val ranking = rankingService.getProductRank(LocalDate.now(), productId) | ||
| return productInfo.copy(ranking = ranking) |
There was a problem hiding this comment.
선택적 랭킹 조회 실패가 상품 상세 전체를 깨뜨리면 안 된다.
ranking 필드는 nullable인데 현재는 Redis 조회 예외가 그대로 전파되어 상세 조회가 5xx로 실패하고, 그 전에 발행한 ProductViewedEvent만 반영되는 부분 성공도 생긴다. 랭킹 조회는 runCatching 등으로 null로 폴백하고 타임아웃/서킷브레이커를 두며, rankingService가 실패해도 상세 응답은 내려가고 ranking = null이 되는 테스트를 추가하는 편이 안전하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/product/ProductFacade.kt`
around lines 53 - 54, Product 상세 조회에서 rankingService.getProductRank 호출 예외가 전체
응답을 깨뜨리지 않도록 ProductFacade의 rankingService.getProductRank 호출을
runCatching/try-catch로 감싸 예외 발생 시 null로 폴백하고(또는 타임아웃/서킷브레이커로 보호)
productInfo.copy(ranking = null)로 내려주도록 수정하고, 또한 rankingService가 예외를 던질 때에도 상세
응답은 200을 반환하고 ranking이 null임을 검증하는 단위/통합 테스트를 추가하시오; 관련 식별자는 ProductFacade,
rankingService.getProductRank, productInfo.copy(...)와 ProductViewedEvent를 참고해 수정
위치와 테스트 시나리오를 찾으세요.
| if (rankingPage.entries.isEmpty()) { | ||
| return RankingPageInfo.empty(page, size) | ||
| } |
There was a problem hiding this comment.
빈 페이지 반환 시 totalElements를 0으로 덮어쓰면 페이징 메타가 깨질 수 있다.
운영 관점에서 요청 페이지가 범위를 벗어난 경우에도 전체 건수는 유지되어야 클라이언트가 정상적으로 페이지 이동/종료 판단을 할 수 있는데, 현재 조기 반환은 전체 건수 정보를 잃을 수 있다. entries만 비우고 totalElements는 rankingPage.totalElements를 그대로 전달하도록 수정하는 것이 안전하다.
추가 테스트로 rankingService.getTopRankings가 entries=empty, totalElements>0을 반환하는 케이스에서 응답의 totalElements가 유지되는지 검증해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt`
around lines 20 - 22, The early-return in RankingFacade that does `return
RankingPageInfo.empty(page, size)` when `rankingPage.entries.isEmpty()` drops
`totalElements` to 0; change the logic so that when `rankingPage.entries` is
empty you still build/return a RankingPageInfo which preserves
`rankingPage.totalElements` (i.e., clear only entries but copy
`rankingPage.totalElements` into the returned page instead of forcing 0), and
add a unit test for `rankingService.getTopRankings` that simulates `entries =
empty` with `totalElements > 0` and asserts the response keeps the original
`totalElements`.
| fun getTopRankings(date: LocalDate, page: Int, size: Int): RankingPage { | ||
| val key = RankingKeyGenerator.dailyKey(date) | ||
| val offset = ((page - 1) * size).toLong() | ||
|
|
||
| val entries = rankingRepository.getTopN(key, offset, size.toLong()) | ||
| val totalCount = rankingRepository.getTotalCount(key) |
There was a problem hiding this comment.
페이지/사이즈 검증 부재와 Int 오버플로로 오프셋 계산이 깨질 수 있다
운영 관점에서 page <= 0 또는 비정상 큰 값이 들어오면 음수/오버플로 오프셋으로 잘못된 랭킹 구간을 조회해 API 일관성이 깨진다. 수정안으로 입력을 require(page >= 1 && size >= 1)로 방어하고, 오프셋은 Long 기반 곱셈(Math.multiplyExact((page - 1).toLong(), size.toLong()))으로 계산해야 한다. 추가 테스트로 page=0, size=0, page=Int.MAX_VALUE 경계 케이스에서 예외 또는 안전 동작을 검증하기 바란다.
수정 예시
class RankingService(
private val rankingRepository: RankingRepository,
) {
fun getTopRankings(date: LocalDate, page: Int, size: Int): RankingPage {
+ require(page >= 1) { "page must be >= 1" }
+ require(size >= 1) { "size must be >= 1" }
val key = RankingKeyGenerator.dailyKey(date)
- val offset = ((page - 1) * size).toLong()
+ val offset = Math.multiplyExact((page - 1).toLong(), size.toLong())
val entries = rankingRepository.getTopN(key, offset, size.toLong())📝 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 getTopRankings(date: LocalDate, page: Int, size: Int): RankingPage { | |
| val key = RankingKeyGenerator.dailyKey(date) | |
| val offset = ((page - 1) * size).toLong() | |
| val entries = rankingRepository.getTopN(key, offset, size.toLong()) | |
| val totalCount = rankingRepository.getTotalCount(key) | |
| fun getTopRankings(date: LocalDate, page: Int, size: Int): RankingPage { | |
| require(page >= 1) { "page must be >= 1" } | |
| require(size >= 1) { "size must be >= 1" } | |
| val key = RankingKeyGenerator.dailyKey(date) | |
| val offset = Math.multiplyExact((page - 1).toLong(), size.toLong()) | |
| val entries = rankingRepository.getTopN(key, offset, size.toLong()) | |
| val totalCount = rankingRepository.getTotalCount(key) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/kotlin/com/loopers/domain/ranking/RankingService.kt`
around lines 10 - 15, Validate inputs and prevent Int overflow in
getTopRankings: add a precondition like require(page >= 1 && size >= 1) at the
start of getTopRankings, then compute offset using long-safe multiplication
(e.g., Math.multiplyExact((page - 1).toLong(), size.toLong())) before passing to
rankingRepository.getTopN; keep using RankingKeyGenerator.dailyKey(date) and
rankingRepository.getTotalCount(key) unchanged. Also add unit tests covering
page=0, size=0 and page=Int.MAX_VALUE boundary cases to assert an exception or
safe behavior.
| @DisplayName("원본 키가 비어있으면 0을 반환한다") | ||
| @Test | ||
| fun returnsZeroWhenSourceIsEmpty() { | ||
| val baseDate = LocalDate.of(2026, 4, 10) | ||
| every { | ||
| repository.carryOver("ranking:all:20260410", "ranking:all:20260411", 0.1) | ||
| } returns 0L | ||
|
|
||
| val count = service.execute(baseDate) | ||
|
|
||
| assertThat(count).isEqualTo(0) | ||
| } |
There was a problem hiding this comment.
빈 원본 케이스에서 저장소 호출 계약 검증이 누락되어 있다
운영 관점에서 서비스가 조기 return 0 하거나 잘못된 키/가중치로 호출해도 현재 테스트는 통과할 수 있어, 배치 carry-over 미동작 회귀를 CI에서 놓치게 된다. 수정안으로 verify(exactly = 1)로 repository.carryOver(...) 인자(원본/대상 키, weight)를 검증하고 confirmVerified(repository)를 추가하는 편이 안전하다. 추가 테스트로 월말/연말 경계 날짜(예: 2026-12-31)에서 다음 날 키 계산이 정확한지 검증하면 좋다.
As per coding guidelines **/*Test*.kt: 단위 테스트는 행위/경계값/실패 케이스를 포함하는지 점검한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-batch/src/test/kotlin/com/loopers/domain/ranking/RankingCarryOverServiceTest.kt`
around lines 36 - 47, The test returns 0 without verifying the repository
contract; update the test for returnsZeroWhenSourceIsEmpty to assert that
repository.carryOver("ranking:all:20260410", "ranking:all:20260411", 0.1) is
invoked exactly once (use verify(exactly = 1) on repository.carryOver) and add
confirmVerified(repository) to ensure no other interactions; also add/duplicate
a boundary test calling service.execute with a month/year-end date (e.g.,
LocalDate.of(2026, 12, 31)) to validate the next-day key calculation logic.
| val scoreIncrement = rankingScorePolicy.calculateIncrement(event.eventType, event.delta) | ||
| val key = RankingKeyGenerator.dailyKey(event.occurredAt.toLocalDate()) | ||
| rankingRepository.incrementScore(key, event.productId, scoreIncrement) |
There was a problem hiding this comment.
DB 트랜잭션 안에서 Redis를 직접 갱신하면 부분 성공 시 복구가 어렵다.
productMetricsJpaRepository.save(...) 뒤에 rankingRepository.incrementScore(...)를 같은 흐름에서 호출하면 Redis 반영 성공 후 JPA 커밋이 실패하는 순간 두 저장소가 영구히 어긋난다. 운영에서는 랭킹만 먼저 올라가고 동일 메시지 재처리 시 중복 가산까지 생길 수 있으니, 랭킹 갱신은 afterCommit 훅이나 별도 outbox consumer로 옮기고 "Redis 성공 후 DB 커밋 실패" 시나리오를 재현하는 통합 테스트를 추가하는 편이 안전하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/application/metrics/ProductMetricsEventHandler.kt`
around lines 38 - 40, The code calls rankingRepository.incrementScore(...)
(using RankingKeyGenerator.dailyKey and rankingScorePolicy.calculateIncrement)
immediately after productMetricsJpaRepository.save(...), which can cause
inconsistent state if Redis updates succeed but the DB commit fails; move the
Redis/ranking update out of the DB transaction by scheduling it in an
afterCommit hook or emitting an outbox event consumed by a separate ranking
updater (so incrementScore runs only after JPA commit), update the handler to
enqueue that post-commit action instead of calling
rankingRepository.incrementScore directly, and add an integration test that
simulates "Redis success then DB commit failure" to verify no
duplicate/misaligned increments occur.
| when (event.eventType) { | ||
| CatalogEventType.LIKE_CHANGED -> likesCount += event.delta | ||
| CatalogEventType.PRODUCT_VIEWED -> viewsCount += event.delta | ||
| CatalogEventType.ORDER_COMPLETED -> salesCount += event.delta | ||
| } |
There was a problem hiding this comment.
ORDER_COMPLETED의 delta 유효성 검증이 없어 판매 지표 오염 위험이 있다.
운영 관점에서 외부 이벤트 이상치(음수/0)가 유입되면 salesCount가 감소하거나 비정상 누적되어 랭킹과 분석 지표가 동시에 오염된다. 수정안은 ORDER_COMPLETED 분기에서 delta > 0을 강제하고 위반 시 도메인 예외로 차단하는 것이다. 추가 테스트로는 음수/0 delta 이벤트 적용 시 예외가 발생하고, 정상 양수 이벤트만 누적되는 케이스를 추가해야 한다.
🔧 제안 수정안
fun apply(event: CatalogEventMessage) {
when (event.eventType) {
CatalogEventType.LIKE_CHANGED -> likesCount += event.delta
CatalogEventType.PRODUCT_VIEWED -> viewsCount += event.delta
- CatalogEventType.ORDER_COMPLETED -> salesCount += event.delta
+ CatalogEventType.ORDER_COMPLETED -> {
+ require(event.delta > 0) { "ORDER_COMPLETED delta must be positive" }
+ salesCount += event.delta
+ }
}📝 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.
| when (event.eventType) { | |
| CatalogEventType.LIKE_CHANGED -> likesCount += event.delta | |
| CatalogEventType.PRODUCT_VIEWED -> viewsCount += event.delta | |
| CatalogEventType.ORDER_COMPLETED -> salesCount += event.delta | |
| } | |
| when (event.eventType) { | |
| CatalogEventType.LIKE_CHANGED -> likesCount += event.delta | |
| CatalogEventType.PRODUCT_VIEWED -> viewsCount += event.delta | |
| CatalogEventType.ORDER_COMPLETED -> { | |
| require(event.delta > 0) { "ORDER_COMPLETED delta must be positive" } | |
| salesCount += event.delta | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/domain/metrics/ProductMetricsModel.kt`
around lines 59 - 63, ORDER_COMPLETED 분기에서 event.delta에 대한 유효성 검증이 빠져
salesCount가 음수/0으로 오염될 수 있으니 CatalogEventType.ORDER_COMPLETED 처리부에서 반드시
event.delta > 0인 경우에만 salesCount에 누적하고, 그렇지 않으면 도메인 예외(예:
InvalidEventDeltaException)를 던져 이벤트 적용을 차단하도록 변경하세요; 변경 대상은 ProductMetricsModel의
when (event.eventType) 분기 내 ORDER_COMPLETED 처리 로직이며, 단위테스트를 추가해 delta가 음수 또는 0일
때 예외가 발생하고 정상 양수일 때만 salesCount가 증가하는 케이스를 검증하세요.
| override fun incrementScore(key: String, productId: Long, score: Double) { | ||
| redisTemplate.opsForZSet().incrementScore(key, productId.toString(), score) | ||
| if (redisTemplate.getExpire(key) == -1L) { | ||
| redisTemplate.expire(key, TTL) | ||
| } |
There was a problem hiding this comment.
TTL 확인 로직이 이벤트 핫패스의 Redis 부하를 키운다.
운영 관점에서 현재 방식은 이벤트마다 ZINCRBY + TTL 조회가 발생해 마스터 Redis QPS를 불필요하게 증가시키고, 트래픽 급증 시 Kafka consumer lag로 이어질 수 있다. TTL 설정은 원자적으로 한 번만 처리되도록 Lua 스크립트(또는 동등한 원자 연산 전략)로 합쳐 왕복 횟수를 줄이는 편이 안전하다.
추가 테스트로 (1) 동시성 상황에서 TTL이 설정된 키의 TTL이 리셋되지 않는지, (2) TTL 미설정 키에서 최초 증분 후 TTL이 설정되는지 통합 테스트를 보강해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/kotlin/com/loopers/infrastructure/ranking/RankingRepositoryImpl.kt`
around lines 19 - 23, The current incrementScore method issues two Redis calls
(ZINCRBY and GETEXPIRE/EXPIRE) causing extra QPS; replace the separate
operations in RankingRepositoryImpl.incrementScore by executing an atomic Lua
script via redisTemplate.execute that performs ZINCRBY and sets TTL only if the
key has no TTL (i.e., check TTL == -1 and call EXPIRE inside the script) using
the existing TTL constant; keep the same method signature but call the script
instead of redisTemplate.opsForZSet().incrementScore / getExpire / expire. Also
add integration tests for concurrent updates validating (1) TTL on keys that
already have TTL is not reset and (2) TTL is set after the first increment on
keys without TTL.
| @DisplayName("stale 이벤트는 metrics도 ZSET도 갱신하지 않는다") | ||
| @Test | ||
| fun staleEventDoesNotIncrementScore() { | ||
| val handledEventIds = mutableSetOf<String>() | ||
| val metricsStore = mutableMapOf<Long, ProductMetricsModel>() | ||
| stubRepositories(handledEventIds, metricsStore) | ||
|
|
||
| val latestEvent = catalogEvent( | ||
| eventId = "latest", | ||
| productId = 30L, | ||
| eventType = CatalogEventType.LIKE_CHANGED, | ||
| delta = 1, | ||
| version = 10, | ||
| occurredAt = ZonedDateTime.parse("2026-04-10T12:00:00+09:00"), | ||
| ) | ||
| val staleEvent = catalogEvent( | ||
| eventId = "stale", | ||
| productId = 30L, | ||
| eventType = CatalogEventType.LIKE_CHANGED, | ||
| delta = 1, | ||
| version = 5, | ||
| occurredAt = ZonedDateTime.parse("2026-04-10T11:00:00+09:00"), | ||
| ) | ||
|
|
||
| handler.handle(latestEvent) | ||
| handler.handle(staleEvent) | ||
|
|
||
| // latest는 1회, stale는 0회 | ||
| verify(exactly = 1) { | ||
| rankingRepository.incrementScore(any(), any(), any()) | ||
| } |
There was a problem hiding this comment.
stale 경로가 metrics 미갱신을 실제로 검증하지 않는다
현재 검증은 rankingRepository.incrementScore()가 1회만 호출됐는지만 본다. stale 이벤트가 랭킹은 건드리지 않더라도 ProductMetricsModel을 덮어쓰면 운영에서는 집계 수치와 랭킹이 서로 어긋난다. metricsStore의 최신 상태가 그대로 유지되는지 확인하고, 가능하면 productMetricsJpaRepository.save()도 1회만 호출됐는지 함께 검증해 달라. 추가로 stale 이벤트의 delta를 더 크게 바꾼 케이스에서도 metrics와 ranking이 모두 불변인지 테스트를 하나 더 넣어두는 편이 안전하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/test/kotlin/com/loopers/application/metrics/ProductMetricsEventHandlerRankingTest.kt`
around lines 109 - 139, The test currently only verifies
rankingRepository.incrementScore() was called once but doesn't assert that
ProductMetricsModel in metricsStore wasn't overwritten; update the test for
staleEventDoesNotIncrementScore to also verify the metricsStore entry for
productId 30L remains the same after handler.handle(staleEvent) (i.e., unchanged
ProductMetricsModel state) and assert productMetricsJpaRepository.save() was
called only once (or not called for the stale event), and add an additional test
variant where the stale event has a larger delta to ensure both metricsStore and
rankingRepository remain unchanged when handler.handle(...) is invoked with that
staleEvent.
📌 Summary
product_metrics에 유저 행동(조회, 좋아요)을 집계하는 구조를 구축했다. 하지만 이 집계 데이터를 기반으로 "오늘의 인기 상품"을 실시간으로 제공하는 랭킹 시스템이 없었다. RDB의GROUP BY + ORDER BY로 매번 랭킹을 계산하면 데이터가 쌓일수록 느려지고, 조회 빈도가 높은 랭킹 API 특성상 DB 과부하로 이어진다.GET /api/v1/rankings?date=yyyyMMdd&size=20&page=1로 상품 정보가 Aggregation된 일간 랭킹을 페이지 단위로 조회할 수 있다.GET /api/v1/products/{id}응답에 오늘 기준 순위(ranking필드)가 추가되었다. 주문 이벤트도 Kafka 파이프라인에 추가되어 조회(0.1)/좋아요(0.2)/주문(0.7) 가중치로 실시간 반영된다. 콜드 스타트 완화를 위해 매일 23:50에 전일 점수의 10%를 다음 날 키에 복사하는 배치 Job도 구현했다.🧭 Context & Decision
문제 정의
product_metrics테이블에 조회/좋아요 수가 집계되어 있지만, 이를 기반으로 "인기순 정렬"을 하려면SELECT ... ORDER BY weighted_score DESC를 매번 실행해야 한다. 상품 10만 건 기준 ~300ms로, 홈 메인에서 초당 수백 건의 랭킹 요청이 들어오면 DB가 감당할 수 없다. 또한 주문 이벤트는 Kafka 파이프라인에 포함되지 않아 구매 결정이 랭킹에 반영되지 않았다.선택지와 결정
1. 랭킹 저장소 선택
GROUP BY + ORDER BY— product_metrics 테이블에서 직접 계산2. 가중치 설계
ranking.weight.*설정으로 외부화하여 재배포 없이 조정 가능하도록 했다. 서비스 성장 시 A/B 테스트나 Feature Flag 기반 실시간 가중치 조절로 전환할 수 있다.3. 콜드 스타트 해결
🏗️ Design Overview
변경 범위
modules/kafka(CatalogEventType 확장),apps/commerce-api(Ranking API, 상품 상세 순위, 주문 이벤트 발행),apps/commerce-streamer(ZSET 적재),apps/commerce-batch(Carry-Over 배치)domain/ranking/(RankingKeyGenerator, RankingScorePolicy, RankingRepository),infrastructure/ranking/RankingRepositoryImpldomain/ranking/(RankingKeyGenerator, RankingRepository, RankingService),infrastructure/ranking/RankingRepositoryImpl,application/ranking/(RankingFacade, RankingInfo),interfaces/api/ranking/RankingV1Controllerdomain/ranking/(RankingKeyGenerator, RankingCarryOverRepository, RankingCarryOverService),infrastructure/ranking/RankingCarryOverRepositoryImpl,batch/job/ranking/(RankingCarryOverJobConfig, RankingCarryOverTasklet)OrderCompletedEvent에orderItems필드 추가(기존 리스너 호환),ProductInfo에ranking필드 추가(기본값 null로 호환),ProductFacade에RankingService주입.주요 컴포넌트 책임
RankingScorePolicy: 이벤트 타입(조회/좋아요/주문)과 delta를 받아 가중치를 곱한 점수 증분값을 반환한다. 가중치는 application.yml에서 외부 설정으로 관리한다.RankingKeyGenerator:LocalDate → "ranking:all:yyyyMMdd"변환을 담당한다. 세 앱(streamer, api, batch) 모두 동일한 키 형식을 사용해야 하므로 각 앱에 동일 로직이 존재한다(apps 간 의존 금지 원칙).ProductMetricsEventHandler(변경): 기존 DB 집계에 더해, 이벤트 처리 시RankingScorePolicy.calculateIncrement()→RankingRepository.incrementScore()로 ZSET 점수를 누적한다. 기존EventHandledModel기반 멱등성 체크가 ZSET 갱신에도 동일하게 적용된다.RankingRepositoryImpl(streamer):ZINCRBY로 점수를 누적하고, 키 최초 생성 시 TTL 2일을 설정한다. Master 전용 RedisTemplate을 사용한다.RankingRepositoryImpl(api):ZREVRANGE WITHSCORES(Top-N),ZREVRANK(개별 순위),ZCARD(전체 수)를 제공한다. Replica Preferred RedisTemplate으로 읽기 부하를 분산한다.RankingService: ZSET의 0-based 인덱스를 1-based 순위로 변환하고, offset 기반 페이지네이션을 처리한다.RankingFacade: 랭킹 데이터(productId + score)에ProductService.findAllByIds()+BrandService.findAllByIds()로 상품명, 가격, 브랜드명을 Aggregation하여 반환한다. 삭제된 상품은mapNotNull로 자동 필터링된다.RankingV1Controller:GET /api/v1/rankings?date=yyyyMMdd&size=20&page=1엔드포인트를 노출한다.ProductFacade(변경):getProductDetail()실행 후rankingService.getProductRank(LocalDate.now(), productId)로 오늘 기준 순위를 조회하여ProductInfo.copy(ranking = ...)으로 응답에 포함한다.CatalogEventOutboxAppender(변경):OrderCompletedEvent수신 시 주문 상품별로CatalogEventMessage(ORDER_COMPLETED)Outbox 레코드를 생성한다.RankingCarryOverService: 전일 ZSET의 모든 멤버를 carry-over 가중치(0.1)를 곱해 다음 날 키에 합산하여 콜드 스타트를 완화한다.RankingCarryOverJobConfig:rankingCarryOverJobSpring Batch Job으로, 매일 23:50 cron 실행 시requestDate파라미터의 날짜 기준으로 Carry-Over를 수행한다.🔁 Flow Diagram
Main Flow — 이벤트 → ZSET 적재 → API 조회
sequenceDiagram autonumber participant Client participant API as commerce-api participant Kafka participant Streamer as commerce-streamer participant Redis as Redis ZSET participant DB as MySQL Note over Client,DB: Step 1 — 유저 행동 이벤트 발행 Client->>API: 상품 조회 / 좋아요 / 주문 API->>DB: Outbox 테이블에 CatalogEventMessage 저장 API-->>Client: 200 OK Note over API,Kafka: Step 2 — Outbox Relay → Kafka API->>Kafka: OutboxRelayPublisher (1초 주기)<br/>catalog-events 토픽 발행 Note over Kafka,Redis: Step 3 — Consumer → DB 집계 + ZSET 적재 Kafka->>Streamer: DemoKafkaConsumer (배치 리스너) Streamer->>DB: ProductMetricsModel.apply()<br/>likesCount / viewsCount / salesCount 갱신 Streamer->>Redis: ZINCRBY ranking:all:{yyyyMMdd}<br/>{weight × delta} {productId} Note over Client,Redis: Step 4 — 랭킹 API 조회 Client->>API: GET /api/v1/rankings?date=20260410&size=20&page=1 API->>Redis: ZREVRANGE ranking:all:20260410 0 19 WITHSCORES Redis-->>API: [(productId, score), ...] API->>DB: ProductService.findAllByIds() + BrandService.findAllByIds() DB-->>API: 상품명, 가격, 브랜드명 API-->>Client: RankingPageInfo (rank, score, product 정보)Sub Flow — 상품 상세 조회 시 순위 포함
sequenceDiagram autonumber participant Client participant API as ProductFacade participant Cache as ProductCacheStore participant Redis as Redis ZSET participant DB as MySQL Client->>API: GET /api/v1/products/101 API->>Cache: getProductDetail(101) alt 캐시 히트 Cache-->>API: ProductCacheSnapshot else 캐시 미스 API->>DB: ProductService.findById(101) DB-->>API: ProductModel end API->>Redis: ZREVRANK ranking:all:{today} 101 Redis-->>API: 2 (0-based) → 3 (1-based) API-->>Client: ProductInfo { ..., ranking: 3 }Sub Flow — 콜드 스타트 Carry-Over (매일 23:50)
sequenceDiagram autonumber participant Batch as commerce-batch participant Redis as Redis ZSET Note over Batch,Redis: rankingCarryOverJob 실행 Batch->>Redis: ZREVRANGE ranking:all:20260410 0 -1 WITHSCORES Redis-->>Batch: 모든 (productId, score) 목록 loop 각 member에 대해 Batch->>Redis: ZINCRBY ranking:all:20260411<br/>{score × 0.1} {productId} end Batch->>Redis: EXPIRE ranking:all:20260411 172800 Note over Redis: 다음 날 키에 전일 점수의 10%가 미리 적재됨🔍 Review Points — 멘토님께 짚어주길 바라는 포인트
1. DB-Redis 일관성 트레이드오프
ProductMetricsEventHandler에서 DB 트랜잭션(metrics 갱신 + 멱등성 마킹)과 Redis ZINCRBY가 서로 다른 저장소 연산이므로, DB 커밋 후 Redis 실패 시 점수가 누락될 수 있습니다. 현재는 "랭킹 점수는 근사값이므로 허용 가능한 트레이드오프"로 판단했는데, 이 판단이 적절한지 리뷰 부탁드립니다.ProductMetricsEventHandler.kt(38~42라인)SELECT product_id, SUM(weighted_score) FROM product_metrics WHERE date = today GROUP BY product_id결과를 ZADD로 다시 넣으면 10만 건 기준 2~3초면 완료됩니다.2. 가중치 0.1 / 0.2 / 0.7의 설계 근거와 관리 전략
가중치를 "전환 퍼널의 깊이"에 따라 경험적으로 설정했습니다. 현재는 application.yml의
@Value주입으로 외부화했지만, 이것이 Level 2(외부 설정 서버) 수준입니다. 학습 자료에서 빅테크의 가중치 관리 성숙도 4단계(코드 상수 → 외부 설정 → Feature Flag → ML Pipeline)를 공부했는데, 현재 R9 규모에서 이 수준이 적절한지 피드백 부탁드립니다.RankingScorePolicy.ktRankingScorePolicyTest.kt— "주문 1건(0.7) > 좋아요 3건(0.6)" 검증3. Carry-Over 실행 시점과 Race Condition
학습 자료에서 "23:59:58에 Consumer가 오늘 키에 ZINCRBY를 실행하고, 00:00:00에 Scheduler가 ZUNIONSTORE를 실행하면 이미 쌓인 점수가 날아갈 수 있다"는 race condition을 공부했습니다. 현재 구현에서는 ZUNIONSTORE 대신 멤버를 순회하며 ZINCRBY로 합산하는 방식을 선택하여 덮어쓰기 문제를 회피했는데, 이 접근이 적절한지 리뷰 부탁드립니다.
RankingCarryOverRepositoryImpl.kt4. RankingKeyGenerator 코드 중복 (3개 앱)
RankingKeyGenerator가 commerce-streamer, commerce-api, commerce-batch 세 곳에 동일한 코드로 존재합니다. apps 간 의존 금지 원칙을 지키기 위한 선택인데, 키 형식이 변경되면 세 곳을 동시에 수정해야 하는 위험이 있습니다. 공유 모듈(예:modules/redis에 유틸로 추출)로 올리는 게 나은지, 현재처럼 단순 중복이 나은지 의견을 듣고 싶습니다.apps/commerce-streamer/.../RankingKeyGenerator.ktapps/commerce-api/.../RankingKeyGenerator.ktapps/commerce-batch/.../RankingKeyGenerator.kt5. 상품 상세 조회에서 순위 조회의 추가 비용
ProductFacade.getProductDetail()에서 기존 캐시/DB 조회 후 매번ZREVRANK1회를 추가로 호출합니다. Redis 조회라 μs 단위로 빠르지만, 상품 상세 페이지의 모든 요청에 Redis 왕복이 추가되는 것이므로 트래픽이 많아지면 부담이 될 수 있습니다. 순위 정보를 ProductCacheSnapshot에 포함시켜 캐싱하는 것과 현재 방식 중 어느 쪽이 나은지 의견 부탁드립니다.ProductFacade.kt(46~48라인)변경 요약
변경 목적
RDB의 GROUP BY + ORDER BY로 인한 성능 문제(10만건 기준 ~300ms)를 해결하고, Redis Sorted Set을 기반으로 일간 실시간 인기 상품 랭킹 시스템을 구축하여 롱테일 및 콜드 스타트 문제를 완화한다.
핵심 변경점
리스크/주의사항
테스트/검증 방법