[volume-9] Product Ranking with Redis#228
Merged
Kimjipang merged 86 commits intoLoopers-dev-lab:Kimjipangfrom Jan 1, 2026
Merged
[volume-9] Product Ranking with Redis#228Kimjipang merged 86 commits intoLoopers-dev-lab:Kimjipangfrom
Kimjipang merged 86 commits intoLoopers-dev-lab:Kimjipangfrom
Conversation
회원 가입시 User 저장이 수행된다. ( spy 검증 ) 이미 가입된 ID 로 회원가입 시도 시, 실패한다.
회원 가입이 성공할 경우, 생성된 유저 정보를 응답으로 반환한다.
회원 가입 시에 성별이 없을 경우, `400 Bad Request` 응답을 반환한다.
해당 ID 의 회원이 존재할 경우, 회원 정보가 반환된다. 해당 ID 의 회원이 존재하지 않을 경우, null 이 반환된다. 내 정보 조회에 성공할 경우, 해당하는 유저 정보를 응답으로 반환한다. 존재하지 않는 ID 로 조회할 경우, `404 Not Found` 응답을 반환한다.
해당 ID 의 회원이 존재할 경우, 보유 포인트가 반환된다. 해당 ID 의 회원이 존재하지 않을 경우, null 이 반환된다. 포인트 조회에 성공할 경우, 보유 포인트를 응답으로 반환한다. `X-USER-ID` 헤더가 없을 경우, `400 Bad Request` 응답을 반환한다.
0 이하의 정수로 포인트를 충전 시 실패한다. 존재하지 않는 유저 ID 로 충전을 시도한 경우, 실패한다. 존재하는 유저가 1000원을 충전할 경우, 충전된 보유 총량을 응답으로 반환한다. 존재하지 않는 유저로 요청할 경우, `404 Not Found` 응답을 반환한다.
[volume-3] 도메인 모델링 및 구현
This reverts commit 0074ea9.
…9-revert-86-3round Revert "Revert "[volume-3] 도메인 모델링 및 구현""
…-3round Revert "[volume-3] 도메인 모델링 및 구현"
Round3: Product, Brand, Like, Order
…-round3 Revert "Round3: Product, Brand, Like, Order"
* Redis ZSET으로부터 상품의 랭킹 정보를 조회합니다. - 순위(rank) - 점수(score)
* 상품 상세 정보 조회 시 랭킹 정보도 함께 조회하도록 수정하면서 기존 API 수정 - 반환타입 수정 - Facade 수정 - DTO 수정
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRedis 기반 일일 상품 순위 시스템을 도입하고, Kafka 컨슈머를 통해 이벤트 기반 순위 계산을 구현합니다. 새로운 순위 API 엔드포인트와 전역 예외 처리 메커니즘이 추가되어 상품 조회 시 실시간 순위 데이터를 반환합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ProductAPI as Product API<br/>(Commerce API)
participant ProductFacade as ProductFacade
participant Redis as Redis
participant RankingReader as RankingRedisReader
User->>ProductAPI: GET /api/v1/products/{id}
ProductAPI->>ProductFacade: findProductById(id)
ProductFacade->>RankingReader: getDailyRanking(date, id)
RankingReader->>Redis: zScore, zRevRank, zCard<br/>(ranking:all:YYYYMMDD)
Redis-->>RankingReader: score, revRank, total
RankingReader->>RankingReader: rank = revRank + 1
RankingReader-->>ProductFacade: RankingInfo{date, score, rank, total}
ProductFacade->>ProductFacade: combine Product + RankingInfo
ProductFacade-->>ProductAPI: ProductRankingInfo
ProductAPI-->>User: ApiResponse(ProductRankingResponse)
sequenceDiagram
autonumber
participant KafkaProducer as Event Producer
participant Kafka as Kafka Broker
participant KafkaConsumer as KafkaOutboxConsumer
participant DB as PostgreSQL
participant Redis as Redis
KafkaProducer->>Kafka: Publish ProductViewed/Liked/<br/>SALES event
Kafka->>KafkaConsumer: Consume message
KafkaConsumer->>DB: Query/Upsert ProductMetric<br/>(event_type, metric_value)
DB-->>KafkaConsumer: ProductMetric
KafkaConsumer->>KafkaConsumer: Calculate score based<br/>on event weights<br/>(VIEWED:0.1, LIKED:0.3, SALES:0.6)
KafkaConsumer->>Redis: ZINCRBY ranking:all:YYYYMMDD<br/>productId score
Redis-->>KafkaConsumer: Updated score
KafkaConsumer->>Redis: EXPIRE ranking:all:YYYYMMDD<br/>2 days
sequenceDiagram
autonumber
actor User
participant RankingAPI as Ranking API<br/>(Commerce Streamer)
participant RankingController as RankingV1Controller
participant RankingFacade as RankingFacade
participant Redis as Redis
User->>RankingAPI: GET /api/v1/rankings?page=0&size=10
RankingAPI->>RankingController: getDailyProductRanking(size, page)
RankingController->>RankingFacade: getDailyProductRanking(page, size)
RankingFacade->>Redis: ZCARD ranking:all:YYYYMMDD<br/>(get total)
Redis-->>RankingFacade: total count
RankingFacade->>RankingFacade: Validate & normalize<br/>page/size parameters
RankingFacade->>Redis: ZREVRANGE ranking:all:YYYYMMDD<br/>start end WITHSCORES
Redis-->>RankingFacade: Ranked products with scores
RankingFacade->>RankingFacade: Build ProductRankingResponse<br/>list with rank, productId, score
RankingFacade-->>RankingController: ProductRankingPageResponse
RankingController->>RankingController: Wrap in ApiResponse.success()
RankingController-->>User: ApiResponse(ProductRankingPageResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 5
🧹 Nitpick comments (6)
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java (3)
75-75: 매 배치마다expire()호출 최적화 고려.현재 모든 배치 처리 시마다
expire()를 호출합니다. Redis가 이를 효율적으로 처리하지만, 키가 이미 존재할 때만 TTL을 갱신하는 조건부 로직을 추가하거나, 초기 생성 시에만 TTL을 설정하는 방식을 고려해볼 수 있습니다.
42-42: 메서드명이 확장된 기능을 반영하지 않습니다.
productViewedListener는 이제product-viewed,product-liked,product-sales세 가지 이벤트를 처리합니다.productEventListener또는productMetricListener와 같이 범위를 반영하는 이름으로 변경을 권장합니다.
80-86:weight()메서드를private으로 변경 권장.내부에서만 사용되는 헬퍼 메서드이므로
private접근 제한자를 사용하여 캡슐화를 강화하는 것이 좋습니다.🔎 제안
- double weight(ProductEventType eventType) { + private double weight(ProductEventType eventType) {apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.java (1)
5-8: Swagger API 문서화 어노테이션 누락.
ProductV1ApiSpec과 달리@Tag,@Operation어노테이션이 없습니다. API 문서화 일관성을 위해 추가를 권장합니다.🔎 제안
+import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.tags.Tag; +@Tag(name = "Ranking V1 API", description = "상품 순위 API 입니다.") public interface RankingV1ApiSpec { + @Operation(summary = "일별 상품 순위 조회") ApiResponse<RankingV1Dto.ProductRankingPageResponse> getDailyProductRanking(int size, int page); }apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.java (1)
19-26: 입력 파라미터에 기본값 및 유효성 검증 추가 권장
size와page파라미터에 기본값이 없어 클라이언트가 생략 시MissingServletRequestParameterException이 발생합니다. 또한size에 상한 제한이 없어 매우 큰 값 요청 시 성능 문제가 발생할 수 있습니다.🔎 기본값 및 유효성 검증 적용 예시
@GetMapping @Override public ApiResponse<RankingV1Dto.ProductRankingPageResponse> getDailyProductRanking( - @RequestParam int size, - @RequestParam int page + @RequestParam(defaultValue = "20") int size, + @RequestParam(defaultValue = "1") int page ) { + if (size > 100) size = 100; // 상한 제한 RankingV1Dto.ProductRankingPageResponse response = rankingFacade.getDailyProductRanking(page, size); return ApiResponse.success(response); }apps/commerce-streamer/src/main/java/com/loopers/application/RankingFacade.java (1)
42-44:totalPages중복 계산
totalPages가 라인 43과 66에서 동일하게 계산됩니다. 변수를 앞에서 한 번만 계산하면 코드 중복을 줄일 수 있습니다.🔎 리팩터링 제안
long totalElements = (total == null) ? 0 : total; +int totalPages = (int) Math.ceil((double) totalElements / size); if (totalElements == 0) { - return new RankingV1Dto.ProductRankingPageResponse(date, page, size, 0, 0, List.of()); + return new RankingV1Dto.ProductRankingPageResponse(date, page, size, 0, totalPages, List.of()); } long start = (long) (page - 1) * size; long end = start + size - 1; if (start >= totalElements) { - int totalPages = (int) Math.ceil((double) totalElements / size); return new RankingV1Dto.ProductRankingPageResponse(date, page, size, totalElements, totalPages, List.of()); } // ... -int totalPages = (int) Math.ceil((double) totalElements / size); return new RankingV1Dto.ProductRankingPageResponse(date, page, size, totalElements, totalPages, items);Also applies to: 66-66
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductRankingInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/RankingInfo.javaapps/commerce-api/src/main/java/com/loopers/application/product/RankingRedisReader.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.javaapps/commerce-streamer/src/main/java/com/loopers/application/RankingFacade.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiResponse.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1ApiSpec.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Controller.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/support/error/CoreException.javaapps/commerce-streamer/src/main/java/com/loopers/support/error/ErrorType.java
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-27T09:09:24.961Z
Learnt from: sky980221
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 121
File: apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java:22-24
Timestamp: 2025-11-27T09:09:24.961Z
Learning: Product 엔티티 (apps/commerce-api/src/main/java/com/loopers/domain/product/Product.java)는 유즈케이스별로 의도적으로 다른 락 전략을 사용한다: 좋아요 기능에는 비관적 락(findByIdForUpdate)을, 재고 차감에는 낙관적 락(Version + 재시도)을 사용한다.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
📚 Learning: 2025-12-19T09:30:12.459Z
Learnt from: HongChangMo
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 198
File: apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentEventListener.java:0-0
Timestamp: 2025-12-19T09:30:12.459Z
Learning: In the loopers-spring-java-template repository's commerce-api module, when entities are managed within a transactional context (e.g., Transactional methods), prefer relying on JPA dirty checking for automatic persistence rather than explicit save() calls. Both Payment and Order entities in PaymentEventListener use this pattern, with state changes automatically flushed on transaction commit.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
📚 Learning: 2025-12-19T20:59:57.713Z
Learnt from: toongri
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 68
File: docs/week8/round8-detailed-design.md:151-178
Timestamp: 2025-12-19T20:59:57.713Z
Learning: In the Loopers-dev-lab/loopers-spring-kotlin-template repository's Kafka event pipeline, only 5 domain events are intentionally published to Kafka via CloudEventEnvelopeFactory: OrderPaidEventV1, LikeCreatedEventV1, LikeCanceledEventV1, ProductViewedEventV1, and StockDepletedEventV1. Other domain events (OrderCreatedEventV1, OrderCanceledEventV1, PaymentCreatedEventV1, PaymentPaidEventV1, PaymentFailedEventV1) are internal-only and intentionally not mapped in resolveMetadata(), which correctly returns null for them to exclude them from Outbox publication.
Applied to files:
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java
📚 Learning: 2025-12-18T13:24:51.650Z
Learnt from: kilian-develop
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 190
File: apps/commerce-streamer/src/main/java/com/loopers/applications/streamer/consumer/product/IncreaseProductViewKafkaConsumer.java:25-35
Timestamp: 2025-12-18T13:24:51.650Z
Learning: Adopt centralized error handling for Kafka consumers by using the EventInboxAspect to intercept methods annotated with InboxEvent. Ensure that service methods annotated with InboxEvent save failed EventInbox entries and log errors, avoiding duplicating error handling logic in individual consumers. Apply this pattern broadly to similar consumer/service layers within the commerce-streamer module.
Applied to files:
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java
📚 Learning: 2025-12-19T23:39:20.851Z
Learnt from: toongri
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 68
File: apps/commerce-streamer/src/main/kotlin/com/loopers/interfaces/consumer/product/ProductEventConsumer.kt:0-0
Timestamp: 2025-12-19T23:39:20.851Z
Learning: In Loopers-dev-lab/loopers-spring-kotlin-template, toongri prefers pragmatic idempotency handling in Kafka consumers: when idempotency key persistence fails after successful business logic execution, log a warning and continue (accepting low risk of duplicates on retry) rather than rolling back business data. This keeps business logic decoupled from idempotency store (which might be Redis/external system, not RDB), avoids over-engineering for rare edge cases, and prioritizes core business value over auxiliary metadata. Transaction-based atomicity between business data and idempotency keys is considered excessive when duplicate event probability is very low.
Applied to files:
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java
📚 Learning: 2025-12-19T21:30:16.024Z
Learnt from: toongri
Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 68
File: apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/outbox/OutboxEventListener.kt:0-0
Timestamp: 2025-12-19T21:30:16.024Z
Learning: In the Loopers-dev-lab/loopers-spring-kotlin-template Kafka event pipeline, Like events (LikeCreatedEventV1, LikeCanceledEventV1) intentionally use aggregateType="Like" with aggregateId=productId. The aggregateId serves as a partitioning/grouping key (not a unique Like entity identifier), ensuring all like events for the same product go to the same partition for ordering guarantees and aligning with ProductStatisticService's product-based aggregation logic. Using individual like_id would scatter events across partitions and break the statistics aggregation pattern.
Applied to files:
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java
🧬 Code graph analysis (6)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.java (1)
ProductV1Dto(11-74)
apps/commerce-streamer/src/main/java/com/loopers/application/RankingFacade.java (1)
apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java (1)
RankingV1Dto(5-20)
apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiResponse.java (1)
apps/pg-simulator/src/main/kotlin/com/loopers/interfaces/api/ApiResponse.kt (3)
meta(3-32)result(7-19)SUCCESS(12-12)
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java (1)
apps/commerce-streamer/src/main/java/com/loopers/infrastructure/ProductMetricRepositoryImpl.java (1)
Component(9-23)
apps/commerce-streamer/src/main/java/com/loopers/support/error/CoreException.java (1)
apps/pg-simulator/src/main/kotlin/com/loopers/support/error/CoreException.kt (1)
errorType(3-6)
apps/commerce-streamer/src/main/java/com/loopers/support/error/ErrorType.java (2)
apps/commerce-streamer/src/main/java/com/loopers/support/error/CoreException.java (1)
Getter(5-19)apps/pg-simulator/src/main/kotlin/com/loopers/support/error/ErrorType.kt (1)
status(5-11)
🔇 Additional comments (14)
apps/commerce-streamer/src/main/java/com/loopers/support/error/ErrorType.java (1)
7-19: LGTM!
pg-simulator모듈의 Kotlin 버전ErrorType과 일관된 구조입니다. Lombok을 활용한 깔끔한 enum 구현이며, 에러 메타데이터(상태, 코드, 메시지)가 잘 정의되어 있습니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java (1)
44-51: API 응답 타입 변경으로 인한 하위 호환성 검토 필요.
findProductById엔드포인트가ProductResponse에서ProductRankingResponse로 변경되었습니다. 기존 응답에 있던likeCount필드가 제거되고rank,score필드가 추가되었습니다. API 소비자에게 breaking change가 될 수 있으므로 확인이 필요합니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1ApiSpec.java (1)
17-18: LGTM!컨트롤러 구현과 일관되게 인터페이스 스펙이 업데이트되었습니다.
apps/commerce-api/src/main/java/com/loopers/application/product/RankingInfo.java (1)
3-8: LGTM!순위 정보를 담는 immutable record로 적절하게 설계되었습니다.
date를String대신LocalDate로 사용하면 타입 안전성이 향상되지만, Redis와의 연동 시 문자열 변환이 필요하므로 현재 구조도 합리적입니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Dto.java (1)
12-24:likeCount필드 누락 확인 필요.
ProductRankingInfo에는likeCount필드가 포함되어 있지만,ProductRankingResponse에는 매핑되지 않았습니다. 의도적인 설계라면 문제없지만, 기존ProductResponse와의 일관성 측면에서 확인이 필요합니다.apps/commerce-api/src/main/java/com/loopers/application/product/ProductRankingInfo.java (1)
7-19: LGTM!
Product와RankingInfo를 결합하는 팩토리 메서드가 깔끔하게 구현되었습니다.RankingInfo가 null일 경우의 처리가 호출 측에서 보장되는지 확인이 필요할 수 있습니다.apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ranking/RankingV1Dto.java (1)
5-19: LGTM!DTO 레코드 구조가 적절하며, 페이징 메타데이터와 랭킹 응답 구조가 잘 설계되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/application/product/RankingRedisReader.java (1)
24-30: Redis 파이프라인 사용 방식 적절함3개의 Redis 명령을 단일 파이프라인으로 실행하여 네트워크 라운드트립을 최소화했습니다. 효율적인 구현입니다.
apps/commerce-streamer/src/main/java/com/loopers/support/error/CoreException.java (1)
5-18: LGTM!
CoreException구현이pg-simulator의 Kotlin 버전과 일관성 있게 작성되었습니다. 생성자 위임 패턴과 메시지 처리가 적절합니다.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java (2)
56-57:@Transactional과@Cacheable조합 시 쓰기 작업 주의이 메서드는 캐시 가능하지만
OutboxEvent를 저장하는 쓰기 작업도 수행합니다. 캐시 히트 시OutboxEvent가 저장되지 않아 조회 이벤트가 누락될 수 있습니다.캐시 히트 시에도 조회 이벤트 발행이 필요한지 확인해 주세요. 필요하다면 캐시 로직과 이벤트 발행 로직을 분리하는 것이 좋습니다.
74-74: 타임존 불일치 문제
RankingFacade는ZoneId.of("Asia/Seoul")을 사용하지만, 여기서는 시스템 기본 타임존을 사용합니다. 서버 타임존 설정에 따라 날짜 키가 불일치할 수 있습니다.🔎 일관된 타임존 사용
+private static final ZoneId KST = ZoneId.of("Asia/Seoul"); + try { - String date = LocalDate.now().format(DateTimeFormatter.BASIC_ISO_DATE); + String date = LocalDate.now(KST).format(DateTimeFormatter.BASIC_ISO_DATE); ranking = rankingRedisReader.getDailyRanking(date, product.getId());⛔ Skipped due to learnings
Learnt from: jikimee64 Repo: Loopers-dev-lab/loopers-spring-kotlin-template PR: 71 File: apps/commerce-streamer/src/main/kotlin/com/loopers/application/ranking/RankingFacade.kt:167-173 Timestamp: 2025-12-22T16:33:50.678Z Learning: In the Loopers-dev-lab/loopers-spring-kotlin-template repository, using system default timezone (ZonedDateTime.now() without explicit ZoneId) in date key calculations is an intentional design choice. The deployment ensures all instances share the same timezone configuration.apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiResponse.java (1)
3-31: LGTM!
ApiResponse구현이pg-simulator의 Kotlin 버전과 일관성 있게 작성되었습니다. Java record를 활용한 깔끔한 구조입니다.apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (2)
26-30:CoreException핸들러 구현 적절함
CoreException을 적절히 처리하고ErrorType의 상태 코드와 메시지를 활용하여 일관된 응답을 반환합니다.
49-92: JSON 파싱 오류 상세 처리 우수
InvalidFormatException,MismatchedInputException,JsonMappingException을 구분하여 필드 경로와 예상 타입 등 상세한 오류 메시지를 제공합니다. 사용자 경험 향상에 도움이 됩니다.
Comment on lines
+73
to
+76
| try { | ||
| String date = LocalDate.now().format(DateTimeFormatter.BASIC_ISO_DATE); | ||
| ranking = rankingRedisReader.getDailyRanking(date, product.getId()); | ||
| } catch (Exception ignored) {} |
There was a problem hiding this comment.
예외를 무시하는 catch (Exception ignored) 패턴 개선 필요
모든 예외를 조용히 삼키면 Redis 연결 실패, 직렬화 오류 등 중요한 문제를 디버깅하기 어렵습니다. 최소한 로깅을 추가하세요.
🔎 로깅 추가 예시
try {
String date = LocalDate.now().format(DateTimeFormatter.BASIC_ISO_DATE);
ranking = rankingRedisReader.getDailyRanking(date, product.getId());
-} catch (Exception ignored) {}
+} catch (Exception e) {
+ log.warn("Failed to fetch ranking for product {}: {}", product.getId(), e.getMessage());
+}🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java
around lines 73 to 76, replace the silent catch block "catch (Exception ignored)
{}" with an explicit catch that logs the error and relevant context (product id
and date) and only swallows expected recoverable exceptions; e.g., catch
Exception as e -> use the class logger to log at warn/error with a message like
"Failed to read daily ranking for productId=<id> date=<date>" and include the
exception, or narrow to specific exceptions (e.g., Redis or serialization
exceptions) if appropriate, so failures are visible while preserving the
existing fallback behavior.
| Long revRank0 = (Long) results.get(1); | ||
| Long total = (Long) results.get(2); | ||
|
|
||
| Integer rank = (revRank0 == null) ? null : Math.toIntExact(revRank0 + 1); |
There was a problem hiding this comment.
Math.toIntExact 오버플로우 가능성
revRank0가 Integer.MAX_VALUE를 초과하면 ArithmeticException이 발생합니다. 상품 수가 많은 환경에서는 안전한 변환이 필요합니다.
🔎 안전한 변환 제안
-Integer rank = (revRank0 == null) ? null : Math.toIntExact(revRank0 + 1);
+Integer rank = (revRank0 == null) ? null : (int) Math.min(revRank0 + 1, Integer.MAX_VALUE);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Integer rank = (revRank0 == null) ? null : Math.toIntExact(revRank0 + 1); | |
| Integer rank = (revRank0 == null) ? null : (int) Math.min(revRank0 + 1, Integer.MAX_VALUE); |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/product/RankingRedisReader.java
around line 36, the use of Math.toIntExact(revRank0 + 1) can throw
ArithmeticException if revRank0 + 1 exceeds Integer.MAX_VALUE; replace this with
a safe conversion that first checks for null, computes revRank0 + 1 as a long,
and clamps the result to Integer.MAX_VALUE (or Integer.MIN_VALUE if negative
underflow is possible), then cast to int; also optionally log or metric the
clamping event for observability.
Comment on lines
+58
to
+62
| items.add(new RankingV1Dto.ProductRankingResponse( | ||
| rank++, | ||
| Long.parseLong(member), | ||
| score | ||
| )); |
There was a problem hiding this comment.
Long.parseLong 예외 처리 누락
Redis에 잘못된 형식의 member 데이터가 존재할 경우 NumberFormatException이 발생할 수 있습니다. 방어적 코드를 추가하는 것이 좋습니다.
🔎 예외 처리 추가 예시
for (var t : tuples) {
String member = t.getValue();
Double score = t.getScore();
if (member == null || score == null) continue;
+ Long productId;
+ try {
+ productId = Long.parseLong(member);
+ } catch (NumberFormatException e) {
+ continue; // 잘못된 데이터는 스킵
+ }
+
items.add(new RankingV1Dto.ProductRankingResponse(
rank++,
- Long.parseLong(member),
+ productId,
score
));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| items.add(new RankingV1Dto.ProductRankingResponse( | |
| rank++, | |
| Long.parseLong(member), | |
| score | |
| )); | |
| for (var t : tuples) { | |
| String member = t.getValue(); | |
| Double score = t.getScore(); | |
| if (member == null || score == null) continue; | |
| Long productId; | |
| try { | |
| productId = Long.parseLong(member); | |
| } catch (NumberFormatException e) { | |
| continue; // 잘못된 데이터는 스킵 | |
| } | |
| items.add(new RankingV1Dto.ProductRankingResponse( | |
| rank++, | |
| productId, | |
| score | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
apps/commerce-streamer/src/main/java/com/loopers/application/RankingFacade.java
around lines 58 to 62, parsing Redis member with Long.parseLong may throw
NumberFormatException for malformed values; wrap the parse in a try-catch that
catches NumberFormatException, log a warning (including the offending member
value and context), and skip that entry (do not add a ProductRankingResponse) so
processing continues for other members; ensure the counter/rank logic remains
consistent when skipping invalid members.
Comment on lines
94
to
103
| @ExceptionHandler | ||
| public ResponseEntity<ApiResponse<?>> handleBadRequest(ServerWebInputException e) { | ||
| String missingParams = extractMissingParameter(e.getReason() != null ? e.getReason() : ""); | ||
| if (!missingParams.isEmpty()) { | ||
| String message = String.format("필수 요청 값 '%s'가 누락되었습니다.", missingParams); | ||
| return failureResponse(ErrorType.BAD_REQUEST, message); | ||
| } else { | ||
| return failureResponse(ErrorType.BAD_REQUEST, null); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Build Configuration Files ==="
fd -e gradle -e xml -e properties | head -20
echo ""
echo "=== Spring Dependencies in build.gradle ==="
fd build.gradle | xargs cat 2>/dev/null
echo ""
echo "=== Checking pom.xml ==="
fd pom.xml | xargs cat 2>/dev/null | head -100Repository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 9932
🏁 Script executed:
#!/bin/bash
echo "=== Checking ApiControllerAdvice.java imports and class definition ==="
cat apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java | head -20
echo ""
echo "=== Checking if ServerWebInputException is used ==="
rg "ServerWebInputException" apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaRepository: Loopers-dev-lab/loopers-spring-java-template
Length of output: 1387
ServerWebInputException은 WebFlux 전용 예외로 이 프로젝트에서는 호출되지 않습니다
이 프로젝트는 spring-boot-starter-web을 사용하는 Spring MVC(Servlet 기반) 애플리케이션입니다. ServerWebInputException은 Spring WebFlux에서만 발생하는 예외이므로 이 핸들러(94-103줄)는 절대 호출되지 않는 데드 코드입니다.
Servlet MVC에서 요청 매개변수 오류를 처리하려면 MissingServletRequestParameterException(이미 구현됨) 또는 HttpMessageNotReadableException(이미 구현됨)을 사용하십시오. 이 핸들러를 제거하거나, WebFlux 환경이 추가되는 경우에만 별도 모듈에 배치하세요.
🤖 Prompt for AI Agents
In
apps/commerce-streamer/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java
around lines 94-103, the handler for ServerWebInputException is dead code
because this project uses Spring MVC (servlet) not WebFlux; remove this
exception handler (or move it to a separate WebFlux-specific module) and rely on
the existing MissingServletRequestParameterException and
HttpMessageNotReadableException handlers to handle servlet request binding
errors instead.
Comment on lines
41
to
45
| @Transactional | ||
| public void productViewedListener( | ||
| List<ConsumerRecord<String, String>> messages, | ||
| Acknowledgment acknowledgment | ||
| ) throws JsonProcessingException { |
There was a problem hiding this comment.
배치 처리 중 단일 메시지 실패 시 전체 배치가 실패합니다.
JsonProcessingException이 for 루프 바깥에서 throw되므로, 하나의 메시지 파싱 실패 시 전체 배치가 처리되지 않습니다. 개별 메시지 실패를 격리하여 처리하는 것을 권장합니다.
🔎 개선 제안
@Transactional
public void productViewedListener(
List<ConsumerRecord<String, String>> messages,
Acknowledgment acknowledgment
- ) throws JsonProcessingException {
+ ) {
Map<Long, Double> scoreDelta = new HashMap<>();
for (var record : messages) {
- OutboxEvent value = objectMapper.readValue(record.value(), OutboxEvent.class);
+ try {
+ OutboxEvent value = objectMapper.readValue(record.value(), OutboxEvent.class);
+ // ... process message
+ } catch (JsonProcessingException e) {
+ log.error("Failed to parse message: {}", record.value(), e);
+ // 개별 메시지 실패 시 로깅 후 계속 처리
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaOutboxConsumer.java
around lines 41-45, the method currently declares and can throw
JsonProcessingException outside the message loop so a single bad message aborts
the whole transactional batch; change the handling so JSON parsing and
processing for each ConsumerRecord is wrapped in its own try/catch that catches
JsonProcessingException (and other per-message errors), logs the failure (and/or
emits metrics or forwards the raw message to a DLQ), and continues processing
the remaining messages; ensure transactional boundaries are adjusted so
successful messages are not rolled back by a single-message failure (e.g.,
perform per-message processing in a separate transaction or remove the
method-level @Transactional and manage transactions per message) and do not
rethrow JsonProcessingException from the outer method.
**AS-IS** commerce-streamer에서 랭킹 정보 API 정의 **TO-BE** commerce-streamer에서는 집계만 하도록 하기 위해 commerce-api 모듈로 랭킹 정보 API 수정
refactor: 랭킹 정보 API path 수정
- 주간/월간 랭킹 정보 집계를 용이하게 하기 위해 metric_date 컬럼 추가
refactor: product_metrics 테이블 컬럼 수정
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
💬 Review Points
Q1)
현재 상품 상세 조회에 캐시를 걸어두어서 설정한 시간 내에는 outbox 테이블에 row가 쌓이지 않기에 kafka broker에 메시지 발행이 안되는 구조입니다.
abusing을 막기 위해서는 조회할 때마다 메시지 발행이 되면 안되겠지만, 그렇다고 캐싱을 하는 기간동안 막아두는 것 또한 score가 제대로 쌓이지는 않을 거 같습니다.
보통 위와 같은 상황에서 어떻게 outbox에 row를 쌓나요?
✅ Checklist
📈 Ranking Consumer
⚾ Ranking API
📎 References