Repository navigation
Volume 7 - #333
Merged
Merged
Volume 7#333
Conversation
- LikeFacade 는 좋아요 저장까지만 담당하고 LikeAdded/RemovedEvent 발행 - 집계(like_count)는 @async + @TransactionalEventListener(AFTER_COMMIT) 로 분리 → 집계 실패와 무관하게 좋아요는 성공(실패 격리) - 단일 스레드 executor 로 추가/취소 이벤트 순서 보장(감소 가드 뒤바뀜 방지) - 집계를 관찰하는 테스트는 Awaitility 로 최종 일관성 검증 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- OrderFacade → OrderPlacedEvent, PaymentFacade → Payment Completed/Failed 이벤트 발행 - UserActivityLogListener: 좋아요/주문/결제 등 유저 행동을 AFTER_COMMIT 로 서버 로깅 (실제 커밋된 행동만 기록, 로깅 실패가 본 로직에 영향 없음) - PaymentNotificationListener: 결제 결과 알림을 @async 로 응답 지연과 분리 - 주 로직/부가 로직 경계와 트랜잭션 상관관계를 이벤트 phase 로 표현 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- OutboxKafkaConfig: acks=all, enable.idempotence=true 전용 KafkaTemplate - outbox_event 테이블 + OutboxEventRecorder(@eventlistener, 도메인 변경과 같은 트랜잭션에 기록) - OutboxRelay(@scheduled): PENDING 을 key(productId/orderId)로 발행 후 SENT 마킹, 실패 시 재시도(At-Least-Once) - 좋아요/조회 → catalog-events, 주문(라인 포함) → order-events - test 프로파일에서 릴레이 비활성화, Outbox 기록/릴레이 단위·통합 테스트 추가 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CatalogEventConsumer/OrderEventConsumer: 관심사별 Consumer Group 분리, manual ack(배치 처리 후 커밋) - MetricsEventHandler: 좋아요/조회/판매량을 product_metrics 에 upsert - event_handled(event_id PK) 기반 멱등 처리 — 중복 수신에도 한 번만 반영 - 지표 반영과 처리 기록을 한 트랜잭션으로 커밋 - 멱등/upsert 통합 테스트 추가 (브로커 없이 핸들러 직접 검증) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- API 는 발급을 직접 하지 않고 요청만 접수(REQUESTED) 후 coupon-issue-requests 로 발행(Outbox)
- CouponIssueRequestConsumer → CouponIssuanceService 가 실제 발급
- 선착순 수량 제한: coupon_stock 원자적 조건부 UPDATE 로 초과 발급 원천 차단
- 중복 발급 방지 + 이미 처리된 요청 멱등 스킵
- requestId 로 발급 결과 폴링(GET /coupon-issue-requests/{requestId})
- 동시성 테스트: 100장 한정에 200명 동시 요청 → 정확히 100건만 발급
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 집계가 커밋 후 비동기로 일어나므로 ProductLikeCounter 에서 집계 반영 뒤 상세 캐시를 evict - 캐시 무효화 통합 테스트는 최종 일관성(Awaitility)으로 검증 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 Walkthrough변경 목적: 좋아요/결제/주문/쿠폰 처리에서 도메인 이벤트 기반의 비동기 후처리와 Outbox/Kafka 연계를 도입해, 본 트랜잭션은 성공시키고 집계·알림은 이후 처리되도록 바꾸는 것입니다. Walkthroughcommerce-api에 트랜잭셔널 아웃박스 패턴과 Kafka 기반 비동기 이벤트 파이프라인을 도입해 좋아요/주문/결제/조회 도메인 이벤트를 발행하고, 좋아요 카운트 집계를 비동기로 전환했다. 선착순 쿠폰 발급 기능(요청 접수/폴링 API, Kafka 컨슈머, 재고 원자적 차감)을 신규 추가했으며, commerce-streamer에는 카탈로그/주문 이벤트를 소비해 product_metrics를 멱등적으로 집계하는 컨슈머를 추가했다. Changes이벤트 기반 아키텍처 전환
선착순 쿠폰 발급 기능
commerce-streamer 지표 집계
Sequence Diagram(s)sequenceDiagram
participant LikeFacade
participant ApplicationEventPublisher
participant OutboxEventRecorder
participant OutboxEventRepository
participant OutboxRelay
participant Kafka
LikeFacade->>ApplicationEventPublisher: publishEvent(LikeAddedEvent)
ApplicationEventPublisher->>OutboxEventRecorder: on(LikeAddedEvent)
OutboxEventRecorder->>OutboxEventRepository: save(OutboxEvent PENDING)
OutboxRelay->>OutboxEventRepository: findPending(batchSize)
OutboxRelay->>Kafka: send(topic, partitionKey, payload)
Kafka-->>OutboxRelay: ack
OutboxRelay->>OutboxEventRepository: markSent()
sequenceDiagram
participant Client
participant CouponIssueV1Controller
participant CouponIssueFacade
participant CouponIssueRequestConsumer
participant CouponIssuanceService
Client->>CouponIssueV1Controller: POST /coupons/{id}/issue-requests
CouponIssueV1Controller->>CouponIssueFacade: requestIssue(couponId, userId)
CouponIssueFacade-->>Client: requestId (202 ACCEPTED)
CouponIssueRequestConsumer->>CouponIssuanceService: issue(requestId, couponId, userId)
CouponIssuanceService->>CouponIssuanceService: 재고 원자적 차감 및 중복 검사
Client->>CouponIssueV1Controller: GET /coupon-issue-requests/{requestId}
CouponIssueV1Controller->>CouponIssueFacade: getStatus(requestId)
CouponIssueFacade-->>Client: 발급 상태 응답
메타데이터
관련 이슈: 명시된 이슈 링크 없음. 관련 PR: 명시된 관련 PR 없음. 제안 라벨: enhancement, kafka, coupon, event-driven, streamer 제안 리뷰어: 백엔드 도메인/인프라 담당자 및 동시성 로직 검토 가능한 시니어 엔지니어 운영 관점에서 다음 사항을 반드시 확인해야 한다. 왜 문제인지: 또한
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 16
🧹 Nitpick comments (14)
apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.java (1)
31-39: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win집계 실패에 대한 재시도/관측 수단이 없다.
Exception을 잡아log.warn만 남기고 종료하므로, 집계 실패가 누적되면like_count가 실제 좋아요 수와 영구히 어긋날 수 있다. 이는 PR에서 의도한 "eventual consistency" 트레이드오프 자체는 타당하나, 실패를 감지·복구할 운영 수단(재시도 큐, DLQ, 실패 카운트 메트릭/알람)이 전혀 없다는 점은 장애 대응 관점에서 보완이 필요하다. 현재 구조로는 실패가 로그에만 남고, 로그를 모니터링하지 않으면 좋아요 수 불일치를 영구히 인지하지 못한다.추가 테스트 제안:
productLikeCounter.increase()가 예외를 던지는 경우를 모킹하여 리스너가 실패를 감지·기록하는 경로를 검증하는 단위 테스트를 추가할 것을 권장한다.Also applies to: 41-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.java` around lines 31 - 39, `LikeCountAggregationListener.onLikeAdded`는 예외를 잡고 `log.warn`만 남긴 뒤 종료해 실패 복구/관측 수단이 없습니다; `productLikeCounter.increase()` 실패 시 재시도 큐나 DLQ로 넘기거나 실패 카운트 메트릭/알람을 남기도록 처리 흐름을 보완하고, `onLikeAdded`의 실패 경로를 검증하는 테스트를 추가하세요.apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueFlowTest.java (1)
62-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win동시 동일 유저 중복 요청 시나리오에 대한 테스트가 없다.
duplicateIssue_isIdempotent는 동일requestId를 순차적으로 두 번 처리하는 케이스만 검증한다. 위UserCouponRepository에서 지적한 것처럼, 서로 다른requestId로 동일 유저가 동시에 요청하는 경우existsByCouponIdAndUserId체크와save사이의 레이스로 중복 발급될 가능성이 있는데, 이 케이스는 커버되지 않는다.운영 관점: 회귀 발생 시 이를 조기에 잡아낼 안전망이 없다. 별도 파일(
CouponIssueConcurrencyTest)이 "200명 요청/100장 한정" 시나리오만 다룬다면 이는 서로 다른 유저 간 초과발급 방지 검증이지, 동일 유저 중복발급 방지 검증은 아니다.동일 유저가 서로 다른
requestId로 동시 요청하는 케이스에 대한 통합/동시성 테스트 추가를 권장한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueFlowTest.java` around lines 62 - 72, Add a concurrency test for the same user sending duplicate issuance requests with different requestIds, since duplicateIssue_isIdempotent only covers sequential retries with one requestId. Extend CouponIssueFlowTest or add a focused concurrent test around CouponIssueFacade.requestIssue and issuanceService.issue to run two parallel requests for the same USER_ID and assert only one coupon issuance/stock decrement occurs. Use the existing UserCouponRepository and CouponStockRepository checks to verify the race between existsByCouponIdAndUserId and save is safely handled.apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java (1)
47-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win상태 전이 가드가 엔티티 내부에 없다.
markIssued()/markFailed()가 현재 상태를 검증하지 않고 무조건 전이시킨다. 호출부인CouponIssuanceService.issue()에서isPending()을 먼저 체크하고 있지만, 이는 도메인 엔티티가 아닌 애플리케이션 서비스가 상태 머신 불변식을 책임지는 구조다. 향후 다른 호출 경로가 추가되면isPending()체크 없이 상태가 덮어써질 위험이 있다(예: 이미 ISSUED인 요청이 FAILED로 재전이).운영 관점: 이 문제가 실제로 발생하면 이미 발급 완료된 요청의 상태가 FAILED로 뒤바뀌어 폴링 API 응답이 잘못되고, 사용자가 실제로는 쿠폰을 받았는데도 실패로 오인해 재요청하는 등 데이터 정합성 이슈로 이어질 수 있다.
엔티티 자체에서 전이 유효성을 검증하도록 방어 코드를 추가하고, 잘못된 전이 시도에 대한 단위 테스트를 추가할 것을 제안한다.
♻️ 제안 diff
public void markIssued() { + if (this.status != CouponIssueStatus.REQUESTED) { + throw new CoreException(ErrorType.CONFLICT, "이미 처리된 요청입니다. status=" + this.status); + } this.status = CouponIssueStatus.ISSUED; } public void markFailed(String reason) { + if (this.status != CouponIssueStatus.REQUESTED) { + throw new CoreException(ErrorType.CONFLICT, "이미 처리된 요청입니다. status=" + this.status); + } this.status = CouponIssueStatus.FAILED; this.reason = reason; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java` around lines 47 - 58, Add state-transition guards inside CouponIssueRequest so markIssued() and markFailed(String reason) only allow transitions from REQUESTED, instead of blindly overwriting status; if the current state is not pending/requested, fail fast with an appropriate domain exception. Keep CouponIssuanceService.issue() as a caller-side check if needed, but move the invariant enforcement into the entity methods markIssued, markFailed, and isPending so the domain model owns the rule. Also add unit tests covering invalid transitions such as calling markFailed on an already ISSUED request and calling markIssued after FAILED.apps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponIssueV1Controller.java (1)
29-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
couponId경로 변수에 대한 Bean Validation이 없다.음수/0 값의
couponId가 그대로 서비스 레이어까지 전달된다. 컨트롤러 단에서@Positive등으로 조기 검증하면 불필요한 서비스 호출과 예외 흐름을 줄일 수 있다.As per path instructions, Java Controller 리뷰 기준에 따라 "Controller는 요청 검증(Bean Validation)과 응답 조립에 집중하고 비즈니스 로직은 Service로 이동한다".
♻️ 제안
+import jakarta.validation.constraints.Positive; + public ApiResponse<CouponIssueV1Dto.IssueRequestResponse> request( `@RequestHeader`(value = "X-Loopers-UserId", required = false) Long userId, - `@PathVariable` Long couponId + `@PathVariable` `@Positive` Long couponId ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponIssueV1Controller.java` around lines 29 - 40, The CouponIssueV1Controller.request endpoint is missing Bean Validation for the couponId path variable, so invalid non-positive IDs can reach the service layer. Add a controller-level validation annotation such as `@Positive` to the couponId parameter in request, keeping CouponIssueV1Controller focused on request validation and response assembly while CouponIssueV1Facade handles the business flow.Source: Path instructions
apps/commerce-streamer/src/test/java/com/loopers/application/metrics/MetricsEventHandlerTest.java (1)
18-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win경계/실패 케이스 테스트가 부족하다.
현재는 LIKE_ADDED, 중복 eventId, ORDER_PLACED 정상 흐름만 검증한다.
MetricsEventHandler의LIKE_REMOVED,PRODUCT_VIEWED, 알 수 없는 eventType(default 분기),lines가 null 인 ORDER_PLACED 분기에 대한 테스트가 없어 회귀 발생 시 감지가 어렵다. 앞서 지적한 catalog/order 컨슈머 동시 처리 시 lost-update 시나리오에 대한 검증도 없다.As per path instructions, 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-streamer/src/test/java/com/loopers/application/metrics/MetricsEventHandlerTest.java` around lines 18 - 65, Add missing boundary/failure coverage in MetricsEventHandlerTest for the MetricsEventHandler flows: verify LIKE_REMOVED and PRODUCT_VIEWED handling, assert the default/unknown eventType branch behavior, and add an ORDER_PLACED case where lines is null to confirm the failure path. Also add a concurrency-focused test around handleCatalog and handleOrder to exercise the lost-update scenario when both consumers process the same product at once, using the existing handler and repository symbols to locate the relevant paths.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java (1)
1-27: 🚀 Performance & Scalability | 🔵 Trivial단일 스레드 풀을 여러 이벤트 리스너가 공유할 경우 상호 지연 가능성.
PR 요약상 활동 로그, 결제 알림 리스너도 같은
eventTaskExecutor를 사용할 가능성이 있다. 좋아요 집계처럼 순서 보장이 필요한 처리와, 순서와 무관한 로깅/알림 처리가 같은 단일 스레드에서 직렬화되면, 알림 처리 지연이 좋아요 집계 지연으로 전이될 수 있다.향후 트래픽 증가 시 관심사별로 실행자를 분리(좋아요 집계 전용 단일 스레드 + 로그/알림용 별도 풀)하는 것을 검토할 것을 제안한다. 이는 현재 시점에 필수는 아니지만 확장성 관점에서 미리 염두에 둘 사항이다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java` around lines 1 - 27, `AsyncConfig.eventTaskExecutor()` currently defines a single-thread executor that may be shared by multiple event listeners, so unrelated tasks like activity logs or payment notifications can block ordered like-count processing. Split this into separate executors by concern: keep one single-thread FIFO executor for order-sensitive domain events (for example, like aggregation) and add another executor for non-order-sensitive work such as logging/notifications. Update the listeners to reference the appropriate executor bean names so `eventTaskExecutor` is not a shared bottleneck.apps/commerce-api/src/test/java/com/loopers/application/outbox/OutboxRelayTest.java (1)
46-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win첫 이벤트 실패 시 이후 이벤트 처리 중단(순서 보장) 시나리오 테스트가 누락되었다.
OutboxRelay의 핵심 설계 의도는 "발행 실패 시 이후 메시지도 처리를 중단해 파티션 키 순서를 보장한다"는 것이다(연관 파일OutboxRelay.java주석 참고). 현재 테스트는 단일 이벤트에 대한 성공/실패만 검증하여, 두 번째 이벤트가 첫 실패 이후 실제로 처리되지 않는지는 검증하지 못한다. 이는 좋아요/주문 이벤트의 순서 역전을 방지하는 핵심 안전장치이므로 회귀 테스트로 고정해 둘 필요가 있다.수정안: 2개 이상의 PENDING 이벤트를
findPending이 반환하도록 목킹하고, 첫 이벤트 발행 실패 시 두 번째 이벤트에 대해kafkaTemplate.send(...)가 호출되지 않았음을verify(kafkaTemplate, never())로 검증하는 테스트를 추가한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/test/java/com/loopers/application/outbox/OutboxRelayTest.java` around lines 46 - 57, Add a regression test for the OutboxRelay sequential-stop behavior: the current keepsPending_onFailure test only covers one event and misses the “stop processing after first publish failure” contract. In OutboxRelayTest, use OutboxRelay.relay() with findPending() returning at least two PENDING OutboxEvent items, make the first kafkaTemplate.send(...) fail, and verify the second event is not sent by asserting kafkaTemplate.send(...) is never called for it. Use the existing OutboxRelay, outboxRepository, kafkaTemplate, and keepsPending_onFailure setup patterns to keep the new test aligned with the current class behavior.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.java (1)
51-70: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win필수 필드 null 검증 부재 — DB 제약조건에만 의존한다.
eventId,topic,partitionKey,payload등은nullable = false로만 보호되어 있어, 잘못된 값이 들어오면 영속화(flush) 시점에서야 예외가 발생한다. Outbox 릴레이 배치 처리 중 이런 예외가 발생하면 원인 추적이 어렵고, 배치 전체가 스킵될 위험이 있다. 생성 시점에 즉시 검증하도록 개선하는 편이 운영 디버깅에 유리하다.🛡️ 제안: 생성자 방어적 검증 추가
private OutboxEvent(String eventId, String aggregateType, String aggregateId, String eventType, String topic, String partitionKey, String payload) { - this.eventId = eventId; + this.eventId = Objects.requireNonNull(eventId, "eventId must not be null"); this.aggregateType = aggregateType; this.aggregateId = aggregateId; this.eventType = eventType; - this.topic = topic; - this.partitionKey = partitionKey; - this.payload = payload; + this.topic = Objects.requireNonNull(topic, "topic must not be null"); + this.partitionKey = Objects.requireNonNull(partitionKey, "partitionKey must not be null"); + this.payload = Objects.requireNonNull(payload, "payload must not be null"); this.status = OutboxStatus.PENDING; }관련 단위 테스트로 null 인자 전달 시 즉시 예외가 발생하는지 검증하는 테스트를 추가할 것을 제안한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.java` around lines 51 - 70, The OutboxEvent factory currently relies only on DB constraints for required fields, so add constructor/create-time null checks in OutboxEvent and fail fast when eventId, aggregateType, aggregateId, eventType, topic, partitionKey, or payload are null. Update the private OutboxEvent constructor and the static create method to validate inputs immediately, and add unit tests covering null arguments to confirm the exception is thrown at creation time rather than on flush.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxEventRecorder.java (1)
85-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
CoreException생성 시 원인 예외(cause)를 보존해야 한다.
JsonProcessingException을CoreException으로 변환하면서 cause를 전달하지 않는다.CoreException이Throwable cause를 받는 생성자를 지원한다면, 직렬화 실패의 근본 원인(어떤 필드/타입이 문제인지)이 스택트레이스에서 사라져 장애 발생 시 원인 파악이 어려워진다. 이 예외는 좋아요/주문/쿠폰 도메인 트랜잭션 자체를 롤백시키는 중요한 실패 지점이므로 원인 보존이 특히 필요하다.} catch (JsonProcessingException e) { - throw new CoreException(ErrorType.INTERNAL_ERROR, "이벤트 직렬화 실패: " + e.getMessage()); + throw new CoreException(ErrorType.INTERNAL_ERROR, "이벤트 직렬화 실패: " + e.getMessage(), e); }As per path instructions, "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxEventRecorder.java` around lines 85 - 91, The serialize(Object message) method in OutboxEventRecorder currently wraps JsonProcessingException into CoreException without preserving the cause. Update the exception conversion to use the CoreException constructor that accepts a Throwable cause so the original JsonProcessingException is retained, and keep the user-facing message concise while preserving the underlying stacktrace for debugging. If there is logging around this path, separate the human-readable error message from the detailed diagnostic log so the root cause remains visible without changing the outward message.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java (1)
10-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
(status, id)복합 인덱스 존재 여부 확인이 필요하다.
findByStatusOrderByIdAsc는 릴레이 스케줄러가 매 초 단위로 호출하는 핫패스 쿼리다. 아웃박스 테이블은 시간이 지날수록 SENT 행이 누적되는 append-only 구조이므로,status컬럼(및 정렬 기준인id)에 대한 복합 인덱스가 없으면 PENDING 조회가 점점 풀스캔에 가까워져 릴레이 주기 지연 및 DB 부하로 이어질 수 있다.마이그레이션에
(status, id)복합 인덱스가 포함되어 있는지 확인하고, 없다면 추가할 것을 권장한다. 부하 테스트로 아웃박스 행 수 증가에 따른 조회 지연 추이를 확인하는 것도 필요하다.As per path instructions, "정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java` around lines 10 - 13, The hot-path query in OutboxEventJpaRepository.findByStatusOrderByIdAsc should be backed by a composite index on (status, id), since it filters by status and orders by id while the outbox table grows append-only. Check the migration/schema for an existing (status, id) index; if it is missing, add it in the database migration so the relay scheduler can keep using indexed lookups instead of degrading into scans as SENT rows accumulate. Also verify the index is actually used for this repository method under large data volumes.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java (1)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win이벤트 발행 책임을 Facade로 이동하는 것을 고려할 필요가 있다.
조회수 집계용 이벤트 발행은 부가 로직으로, 컨트롤러가 직접
ApplicationEventPublisher를 들고 발행하면 컨트롤러가 요청 검증/응답 조립 이상의 책임을 지게 된다.productFacade.getProductDetail()호출 지점(Facade/Service 계층)에서 이벤트를 발행하도록 옮기면, 추후 다른 진입점(배치, 관리자 API 등)에서도 동일 이벤트 발행 로직을 재사용할 수 있고 컨트롤러 테스트 시 이벤트 발행 여부까지 목업할 필요가 없어진다.As per path instructions, "Controller는 요청 검증과 응답 조립에 집중하고 비즈니스 로직은 Service로 이동한다."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java` around lines 42 - 43, 컨트롤러가 직접 ProductViewedEvent를 발행하는 책임을 갖고 있어 요청 처리와 비즈니스 부가 로직이 섞여 있습니다. ProductV1Controller의 `productFacade.getProductDetail()` 호출 흐름으로 이벤트 발행을 옮기고, `ProductFacade` 또는 관련 Service에서 조회수 집계용 `ProductViewedEvent`를 발행하도록 분리하세요. 이렇게 하면 컨트롤러는 검증/응답 조립에 집중하고, 다른 진입점에서도 동일한 이벤트 발행 로직을 재사용할 수 있습니다.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/application/outbox/message/CatalogEventMessage.java (1)
7-13: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win메시지 계약 진화 대비 미흡하다.
CouponIssueMessage에는@JsonIgnoreProperties(ignoreUnknown = true)가 적용되어 있으나, 동일하게 Kafka 메시지 계약 역할을 하는 이 레코드에는 누락되어 있다. 운영 관점에서 프로듀서가 필드를 추가하는 시점에 소비자(commerce-streamer)가 재배포되지 않으면 역직렬화 실패로 컨슈머가 멈출 수 있다. 스키마 호환성을 위해 동일하게 적용하는 것을 권장한다. 추가로eventType이 문자열이라 오탈자 위험이 있으므로, enum 도입도 함께 검토할 것을 제안한다.package com.loopers.application.outbox.message; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; /** * catalog-events 토픽 메시지(JSON 계약). 소비자(commerce-streamer)가 동일 필드로 역직렬화한다. * eventType: LIKE_ADDED | LIKE_REMOVED | PRODUCT_VIEWED */ +@JsonIgnoreProperties(ignoreUnknown = true) public record CatalogEventMessage(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/application/outbox/message/CatalogEventMessage.java` around lines 7 - 13, The CatalogEventMessage Kafka contract is missing forward-compatible JSON handling, so add the same ignore-unknown behavior used by CouponIssueMessage to the CatalogEventMessage record to prevent deserialization failures when producers add fields. Update the record definition itself so commerce-streamer consumers can safely ignore extra payload properties, and consider replacing the eventType string with an enum-backed type in the same message contract to reduce typo risk.apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueConcurrencyTest.java (1)
61-100: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win초과 요청(100건)의 최종 상태 검증이 빠져있다.
ISSUED카운트만 100건으로 검증하고, 재고 소진으로 실패한 나머지 100건이 실제로FAILED(또는 그에 준하는 종결 상태)로 전이되는지는 검증하지 않는다. 실패 요청이REQUESTED에 방치되는 회귀를 이 테스트가 잡아내지 못할 수 있으므로, 실패 상태 카운트 검증을 추가하는 것을 권장한다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueConcurrencyTest.java` around lines 61 - 100, The concurrency test only verifies that 100 requests end up as ISSUED, but it does not assert the remaining 100 over-limit requests transition to a terminal failure state. Update firstComeLimit_neverOverIssues and its helper countIssued() usage to also count the non-issued requests from requestRepository and verify they are all FAILED (or the expected terminal status) rather than staying REQUESTED, so the test covers both successful issuance and rejected overflow requests.apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java (1)
30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win토픽명 하드코딩 대신
Topics상수 사용을 권장한다.동일 PR 계열에서 이미
Topics상수 클래스가 도입되었다. 컨슈머에서도 문자열 리터럴 대신 상수를 참조하면 프로듀서/컨슈머 간 토픽명 드리프트를 방지할 수 있다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java` around lines 30 - 35, `CouponIssueRequestConsumer`의 `@KafkaListener`에서 토픽명을 문자열 리터럴로 직접 쓰고 있어 프로듀서/컨슈머 간 불일치 위험이 있습니다. `Topics` 상수 클래스에 정의된 쿠폰 발행 요청 토픽 상수를 참조하도록 바꾸고, `KafkaListener` 설정에서는 해당 상수로 토픽명을 연결해 하드코딩을 제거하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java`:
- Around line 56-58: 쿠폰 조회 실패 시 트랜잭션 롤백만 되고 `CouponIssueRequestConsumer`가 예외를 삼킨
뒤 ack해서 요청이 `REQUESTED`에 고정되는 문제를 수정하세요. `CouponIssuanceService`의 쿠폰 조회/발급 흐름에서
`couponRepository.findById(couponId)` 실패를 `markFailed("NOT_FOUND")`로 같은 처리 경로
안에서 반영하거나, 최소한 이 예외가 소비자에서 재시도 또는 DLQ로 넘어가도록 `CouponIssueRequestConsumer` 예외 처리와
ack 방식을 바꾸세요. `couponId`가 없거나 삭제된 쿠폰인 경우 `REQUESTED -> FAILED` 전이와 메시지 ack/DLQ
동작을 검증하는 통합 테스트를 추가하세요.
- Around line 43-59: Replace the check-then-act duplicate guard in
CouponIssuanceService with a DB-enforced uniqueness path: add a unique
constraint on user_coupon(coupon_id, user_id), remove reliance on
existsByCouponIdAndUserId for correctness, and handle
DataIntegrityViolationException around
userCouponRepository.save(coupon.issue(userId)) by marking the request as
ALREADY_ISSUED. Keep the stock reservation flow in CouponIssuanceService intact,
and add a concurrency test for concurrent issuance of the same couponId/userId
ensuring only one save succeeds and the rest are recorded as failed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.java`:
- Around line 25-29: `ProductLikeCounter.increase` and `decrease` currently
combine `@CacheEvict` with `@Transactional`, which can evict the cache before
the database transaction commits. Move the `CacheConfig.PRODUCT_DETAIL` eviction
in both methods to an after-commit hook such as
`TransactionSynchronizationManager.afterCommit`, or switch to a
transaction-aware cache manager, and keep the atomic repository update methods
like `productRepository.increaseLikeCount` in the transactional block. Add an
integration test around `ProductLikeCounter` to verify the cache is only
invalidated after commit and that stale `productDetail` data cannot be
repopulated during the transaction.
- Around line 27-29: The ProductLikeCounter increase/decrease flow currently
ignores whether ProductJpaRepository.updateLikeCount actually affected any rows,
so soft-deleted or missing products can fail silently. Update the repository
methods (increaseLikeCount/decreaseLikeCount) to return the affected-row count,
then have ProductLikeCounter and LikeCountAggregationListener treat 0 as a
failure by throwing an exception or emitting a clear warning log. Add tests
covering increase/decrease for deleted products and non-existent products to
verify the failure is propagated.
In
`@apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxRelay.java`:
- Around line 45-57: The current OutboxRelay loop stops the entire batch on a
single send failure, so unrelated events from other topics or aggregates can be
blocked by one poison message. Update OutboxRelay.relay-style processing around
findPending(batchSize) and the send(...).get() try/catch so failures only stop
or skip the affected partitionKey/aggregate group, while the rest of pending
continues to publish. Also add a bounded retry or FAILED/dead-letter handling
path for permanently failing events, and cover it with a test showing one stuck
event does not prevent other outbox events from being sent.
- Around line 42-58: OutboxRelay.relay() currently blocks indefinitely on
outboxKafkaTemplate.send(...).get() inside a `@Transactional` method and stops the
whole batch on the first failure, which can hold scheduler threads/DB
transactions too long and block unrelated partitionKey events. Update relay() to
use a bounded wait on send with a timeout, then handle failures so only the
affected partitionKey or event range is paused while other pending OutboxEvent
items continue processing. Add tests around OutboxRelay.relay(),
outboxRepository.findPending, and outboxKafkaTemplate.send to verify broker
delay, timeout handling, and that non-failing events still get marked sent.
In `@apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java`:
- Around line 17-26: ThreadPoolTaskExecutor in AsyncConfig.eventTaskExecutor()
is missing graceful shutdown settings, so queued event tasks may be dropped on
app stop. Update the executor configuration to enable waiting for queued tasks
to finish on shutdown and set a bounded termination wait, keeping the existing
eventTaskExecutor bean and its current pool/thread settings intact. Add or
adjust a test/verification around shutdown behavior to confirm pending tasks are
drained before termination.
- Around line 17-26: The `eventTaskExecutor` in `AsyncConfig` currently relies
on the default rejection behavior, so once the queue is full `@Async` event
processing can fail silently after `@TransactionalEventListener(AFTER_COMMIT)`.
Update this bean to set an explicit `RejectedExecutionHandler` on
`ThreadPoolTaskExecutor`—prefer `CallerRunsPolicy` for backpressure, or a custom
handler that records/retries rejected events—and keep the behavior aligned with
the `eventTaskExecutor()` lifecycle. Add a test that fills the executor queue
and verifies overflow events are either processed or explicitly surfaced instead
of being lost.
In `@apps/commerce-api/src/main/java/com/loopers/config/OutboxKafkaConfig.java`:
- Around line 27-34: `OutboxKafkaConfig.outboxProducerFactory` only sets a few
producer defaults, so add an explicit
`ProducerConfig.DELIVERY_TIMEOUT_MS_CONFIG` alongside the existing
ack/idempotence settings and keep the retry/request timeout values aligned. Also
update the outbox send call sites that use `send(...).get()` to use a bounded
`get(timeout, TimeUnit)` so broker outages cannot block indefinitely, and add an
integration test around the outbox publish flow that verifies the operation
fails within the configured time when the broker is unavailable.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCouponRepository.java`:
- Around line 11-12: The current duplicate-issuance guard in
UserCouponRepository.existsByCouponIdAndUserId is only check-then-act and can
race under concurrent requests. Add a database-level unique constraint on
UserCouponModel for the (couponId, userId) pair, then update the coupon issuance
flow to catch the duplicate-key exception and map it to ALREADY_ISSUED. Also add
a concurrency test that uses different requestId values but the same
couponId/userId to verify only one UserCoupon is persisted.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/order/event/OrderPlacedEvent.java`:
- Around line 9-13: OrderPlacedEvent의 lines 필드가 외부 리스트를 그대로 참조해 발행 후 변경될 수 있으니,
record의 compact constructor에서 방어적 복사를 적용해 불변으로 고정하세요. OrderPlacedEvent 생성 시 전달된
lines를 직접 보관하지 말고, 불변 리스트로 변환해 내부 상태가 이후 변경되지 않도록 하세요. Line 중첩 record는 그대로 두되,
OrderFacade 등 호출 측 변경에 영향을 받지 않도록 OrderPlacedEvent 본문에서 복사/불변화 처리를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentFailedEvent.java`:
- Line 6: `PaymentFailedEvent`의 `reason`이 운영 로그에 원문 그대로 남는 문제를 수정해야 합니다.
`PaymentNotificationListener`와 `UserActivityLogListener`의 `onPaymentFailed`에서
`reason`을 직접 출력하지 말고, 오류 코드로 매핑하거나 마스킹된 값만 로그에 남기도록 변경하세요. 관련 로그 포맷을 `reason`
문자열에 의존하지 않게 바꾸고, `onPaymentFailed` 테스트를 추가해 원문 `reason`이 로그 메시지에 포함되지 않는지
검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponJpaRepository.java`:
- Around line 12-13: `UserCouponJpaRepository`의 `existsByCouponIdAndUserId`만으로는
동시 발급 중복을 막을 수 없으니, `user_coupon` 엔티티/매핑에 `(couponId, userId)` 유니크 제약과 복합 인덱스를
추가하고 저장 시 중복 키 충돌을 `ALREADY_ISSUED`로 변환하도록 처리하세요. 해당 제약은 `UserCoupon`(또는 이에 대응하는
JPA 엔티티)에서 적용되도록 확인하고, 발급 로직이 이를 잡아 서비스 예외로 매핑하는지 점검하세요. 또한 동시 발급 상황에서 한 건만 저장되는
통합 테스트와 유니크 제약 위반이 `ALREADY_ISSUED`로 처리되는 테스트를 추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java`:
- Around line 36-46: The batch consumer in CouponIssueRequestConsumer is
swallowing all exceptions, which hides deserialization and non-business failures
from retry/DLQ handling. Refactor consume() to catch only expected business
exceptions from couponIssuanceService.issue(), and let unexpected errors from
ObjectMapper.readValue or other runtime failures propagate or be routed to DLQ
instead of always logging and skipping. Keep acknowledgment.acknowledge() only
after the batch is fully handled, and add tests covering mixed success plus
deserialization/runtime failure cases to ensure failed records are marked FAILED
or sent to DLQ without duplicating successful coupon issuance.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/metrics/MetricsEventHandler.java`:
- Around line 29-68: MetricsEventHandler still does a non-atomic
read-modify-write in getOrCreate, then mutates and saves in handleCatalog and
handleOrder, which can lose increments or fail on concurrent first insert for
the same productId. Fix this by making ProductMetrics updates atomic with either
optimistic locking on ProductMetrics plus retry, or a database-side
upsert/increment path in metricsRepository so creation and accumulation happen
in one safe flow. Keep the existing handleCatalog, handleOrder, and getOrCreate
entry points, but ensure concurrent events for the same productId cannot
overwrite each other, and add tests covering parallel catalog/order updates and
first-create contention.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java`:
- Around line 34-45: The batch consumer in CatalogEventConsumer.consume
currently logs per-record failures but still unconditionally calls
acknowledgment.acknowledge() after the loop, which can permanently skip failed
events. Update the error-handling flow so failed records are either routed to a
DLQ via Spring Kafka error handling (for example, DefaultErrorHandler with
DeadLetterPublishingRecoverer) or prevent committing past the failed offset by
only acknowledging successful records. Keep the fix centered on consume and its
acknowledgment behavior, and add a test covering deserialize/handler failures to
verify the failed record is retried or sent to DLQ instead of being skipped.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.java`:
- Around line 31-39: `LikeCountAggregationListener.onLikeAdded`는 예외를 잡고
`log.warn`만 남긴 뒤 종료해 실패 복구/관측 수단이 없습니다; `productLikeCounter.increase()` 실패 시 재시도
큐나 DLQ로 넘기거나 실패 카운트 메트릭/알람을 남기도록 처리 흐름을 보완하고, `onLikeAdded`의 실패 경로를 검증하는 테스트를
추가하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/application/outbox/message/CatalogEventMessage.java`:
- Around line 7-13: The CatalogEventMessage Kafka contract is missing
forward-compatible JSON handling, so add the same ignore-unknown behavior used
by CouponIssueMessage to the CatalogEventMessage record to prevent
deserialization failures when producers add fields. Update the record definition
itself so commerce-streamer consumers can safely ignore extra payload
properties, and consider replacing the eventType string with an enum-backed type
in the same message contract to reduce typo risk.
In
`@apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxEventRecorder.java`:
- Around line 85-91: The serialize(Object message) method in OutboxEventRecorder
currently wraps JsonProcessingException into CoreException without preserving
the cause. Update the exception conversion to use the CoreException constructor
that accepts a Throwable cause so the original JsonProcessingException is
retained, and keep the user-facing message concise while preserving the
underlying stacktrace for debugging. If there is logging around this path,
separate the human-readable error message from the detailed diagnostic log so
the root cause remains visible without changing the outward message.
In `@apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java`:
- Around line 1-27: `AsyncConfig.eventTaskExecutor()` currently defines a
single-thread executor that may be shared by multiple event listeners, so
unrelated tasks like activity logs or payment notifications can block ordered
like-count processing. Split this into separate executors by concern: keep one
single-thread FIFO executor for order-sensitive domain events (for example, like
aggregation) and add another executor for non-order-sensitive work such as
logging/notifications. Update the listeners to reference the appropriate
executor bean names so `eventTaskExecutor` is not a shared bottleneck.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java`:
- Around line 47-58: Add state-transition guards inside CouponIssueRequest so
markIssued() and markFailed(String reason) only allow transitions from
REQUESTED, instead of blindly overwriting status; if the current state is not
pending/requested, fail fast with an appropriate domain exception. Keep
CouponIssuanceService.issue() as a caller-side check if needed, but move the
invariant enforcement into the entity methods markIssued, markFailed, and
isPending so the domain model owns the rule. Also add unit tests covering
invalid transitions such as calling markFailed on an already ISSUED request and
calling markIssued after FAILED.
In `@apps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.java`:
- Around line 51-70: The OutboxEvent factory currently relies only on DB
constraints for required fields, so add constructor/create-time null checks in
OutboxEvent and fail fast when eventId, aggregateType, aggregateId, eventType,
topic, partitionKey, or payload are null. Update the private OutboxEvent
constructor and the static create method to validate inputs immediately, and add
unit tests covering null arguments to confirm the exception is thrown at
creation time rather than on flush.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java`:
- Around line 10-13: The hot-path query in
OutboxEventJpaRepository.findByStatusOrderByIdAsc should be backed by a
composite index on (status, id), since it filters by status and orders by id
while the outbox table grows append-only. Check the migration/schema for an
existing (status, id) index; if it is missing, add it in the database migration
so the relay scheduler can keep using indexed lookups instead of degrading into
scans as SENT rows accumulate. Also verify the index is actually used for this
repository method under large data volumes.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponIssueV1Controller.java`:
- Around line 29-40: The CouponIssueV1Controller.request endpoint is missing
Bean Validation for the couponId path variable, so invalid non-positive IDs can
reach the service layer. Add a controller-level validation annotation such as
`@Positive` to the couponId parameter in request, keeping CouponIssueV1Controller
focused on request validation and response assembly while CouponIssueV1Facade
handles the business flow.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.java`:
- Around line 42-43: 컨트롤러가 직접 ProductViewedEvent를 발행하는 책임을 갖고 있어 요청 처리와 비즈니스 부가
로직이 섞여 있습니다. ProductV1Controller의 `productFacade.getProductDetail()` 호출 흐름으로 이벤트
발행을 옮기고, `ProductFacade` 또는 관련 Service에서 조회수 집계용 `ProductViewedEvent`를 발행하도록
분리하세요. 이렇게 하면 컨트롤러는 검증/응답 조립에 집중하고, 다른 진입점에서도 동일한 이벤트 발행 로직을 재사용할 수 있습니다.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java`:
- Around line 30-35: `CouponIssueRequestConsumer`의 `@KafkaListener`에서 토픽명을 문자열
리터럴로 직접 쓰고 있어 프로듀서/컨슈머 간 불일치 위험이 있습니다. `Topics` 상수 클래스에 정의된 쿠폰 발행 요청 토픽 상수를
참조하도록 바꾸고, `KafkaListener` 설정에서는 해당 상수로 토픽명을 연결해 하드코딩을 제거하세요.
In
`@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueConcurrencyTest.java`:
- Around line 61-100: The concurrency test only verifies that 100 requests end
up as ISSUED, but it does not assert the remaining 100 over-limit requests
transition to a terminal failure state. Update firstComeLimit_neverOverIssues
and its helper countIssued() usage to also count the non-issued requests from
requestRepository and verify they are all FAILED (or the expected terminal
status) rather than staying REQUESTED, so the test covers both successful
issuance and rejected overflow requests.
In
`@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueFlowTest.java`:
- Around line 62-72: Add a concurrency test for the same user sending duplicate
issuance requests with different requestIds, since duplicateIssue_isIdempotent
only covers sequential retries with one requestId. Extend CouponIssueFlowTest or
add a focused concurrent test around CouponIssueFacade.requestIssue and
issuanceService.issue to run two parallel requests for the same USER_ID and
assert only one coupon issuance/stock decrement occurs. Use the existing
UserCouponRepository and CouponStockRepository checks to verify the race between
existsByCouponIdAndUserId and save is safely handled.
In
`@apps/commerce-api/src/test/java/com/loopers/application/outbox/OutboxRelayTest.java`:
- Around line 46-57: Add a regression test for the OutboxRelay sequential-stop
behavior: the current keepsPending_onFailure test only covers one event and
misses the “stop processing after first publish failure” contract. In
OutboxRelayTest, use OutboxRelay.relay() with findPending() returning at least
two PENDING OutboxEvent items, make the first kafkaTemplate.send(...) fail, and
verify the second event is not sent by asserting kafkaTemplate.send(...) is
never called for it. Use the existing OutboxRelay, outboxRepository,
kafkaTemplate, and keepsPending_onFailure setup patterns to keep the new test
aligned with the current class behavior.
In
`@apps/commerce-streamer/src/test/java/com/loopers/application/metrics/MetricsEventHandlerTest.java`:
- Around line 18-65: Add missing boundary/failure coverage in
MetricsEventHandlerTest for the MetricsEventHandler flows: verify LIKE_REMOVED
and PRODUCT_VIEWED handling, assert the default/unknown eventType branch
behavior, and add an ORDER_PLACED case where lines is null to confirm the
failure path. Also add a concurrency-focused test around handleCatalog and
handleOrder to exercise the lost-update scenario when both consumers process the
same product at once, using the existing handler and repository symbols to
locate the relevant paths.
🪄 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 Plus
Run ID: 1f0b8a2e-4f35-4bfb-820b-c0f299a40f93
📒 Files selected for processing (73)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/CommerceApiApplication.javaapps/commerce-api/src/main/java/com/loopers/application/activity/UserActivityLogListener.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueFacade.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueInfo.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxEventRecorder.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxRelay.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/Topics.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/message/CatalogEventMessage.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/message/CouponIssueMessage.javaapps/commerce-api/src/main/java/com/loopers/application/outbox/message/OrderEventMessage.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentFacade.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentNotificationListener.javaapps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.javaapps/commerce-api/src/main/java/com/loopers/config/OutboxKafkaConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequestRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueStatus.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponStock.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponStockRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCouponRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/event/CouponIssueRequestedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/like/event/LikeAddedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/like/event/LikeRemovedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/order/event/OrderPlacedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEventRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxStatus.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentCompletedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentFailedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/product/event/ProductViewedEvent.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponStockJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponStockRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponIssueV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponIssueV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/product/ProductV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/application/coupon/CouponIssueFlowTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/outbox/OutboxRecordingTest.javaapps/commerce-api/src/test/java/com/loopers/application/outbox/OutboxRelayTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductCacheIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/LikeV1ApiE2ETest.javaapps/commerce-streamer/src/main/java/com/loopers/application/metrics/MetricsEventHandler.javaapps/commerce-streamer/src/main/java/com/loopers/domain/handled/HandledEvent.javaapps/commerce-streamer/src/main/java/com/loopers/domain/handled/HandledEventRepository.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetricsRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/handled/HandledEventJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/handled/HandledEventRepositoryImpl.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsRepositoryImpl.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/message/CatalogEventMessage.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/message/OrderEventMessage.javaapps/commerce-streamer/src/main/resources/application.ymlapps/commerce-streamer/src/test/java/com/loopers/application/metrics/MetricsEventHandlerTest.javaapps/commerce-streamer/src/test/resources/docker-java.properties
Comment on lines
+43
to
+59
| // 중복 발급 방지 (같은 유저, 같은 쿠폰) | ||
| if (userCouponRepository.existsByCouponIdAndUserId(couponId, userId)) { | ||
| request.markFailed("ALREADY_ISSUED"); | ||
| return; | ||
| } | ||
|
|
||
| // 선착순 수량 제한 — 원자적 차감 (남아있을 때만 성공) | ||
| int acquired = couponStockRepository.issueOne(couponId); | ||
| if (acquired == 0) { | ||
| request.markFailed("SOLD_OUT"); | ||
| return; | ||
| } | ||
|
|
||
| CouponModel coupon = couponRepository.findById(couponId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, | ||
| "[id = " + couponId + "] 쿠폰을 찾을 수 없습니다.")); | ||
| userCouponRepository.save(coupon.issue(userId)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== target file outline ==\n'
ast-grep outline apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java --view expanded || true
printf '\n== target file lines ==\n'
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java | cat -n
printf '\n== search for user coupon entity/repository and unique constraints ==\n'
rg -n "class UserCoupon|`@Table`|uniqueConstraints|coupon_id|user_id|existsByCouponIdAndUserId|save\\(coupon\\.issue|markFailed\\(\"ALREADY_ISSUED\"\\)" apps/commerce-api/src/main/java || true
printf '\n== outline likely related files ==\n'
for f in \
apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCoupon.java \
apps/commerce-api/src/main/java/com/loopers/infrastructure/persistence/coupon/UserCouponRepository.java \
apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueRequestConsumer.java
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --view expanded || true
fi
done
printf '\n== relevant file contents if present ==\n'
for f in \
apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCoupon.java \
apps/commerce-api/src/main/java/com/loopers/infrastructure/persistence/coupon/UserCouponRepository.java \
apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueRequestConsumer.java
do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,260p' "$f" | cat -n
fi
done
printf '\n== search for exception handling / ack policy ==\n'
rg -n "ack|Acknowledg|retry|DLQ|dead letter|errorHandler|DefaultErrorHandler|`@KafkaListener`|`@RabbitListener`|consume|listener" apps/commerce-api/src/main/java || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 34003
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== UserCouponModel ==\n'
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCouponModel.java | cat -n
printf '\n== UserCouponRepository and implementation ==\n'
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCouponRepository.java | cat -n
echo '---'
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponRepositoryImpl.java | cat -n
echo '---'
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponJpaRepository.java | cat -n
printf '\n== CouponIssueRequestConsumer ==\n'
sed -n '1,240p' apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java | cat -n
printf '\n== search for db migration / schema around user_coupon ==\n'
rg -n "user_coupon|coupon_id.*user_id|user_id.*coupon_id|UniqueConstraint|unique constraint|uniqueConstraints" . || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 9634
중복 발급은 DB 유니크 제약으로 막아야 한다
existsByCouponIdAndUserId는 check-then-act라 동시 소비나 재시도 상황에서 중복 발급을 막지 못한다. 현재 consumer가 예외를 잡고 ack만 하므로, 저장 단계에서 충돌이 나면 요청 상태가 정리되지 않은 채 남을 수 있다. user_coupon(coupon_id, user_id)에 유니크 제약을 추가하고 save의 DataIntegrityViolationException을 ALREADY_ISSUED로 변환해 마킹해야 한다. 같은 couponId/userId를 동시에 여러 번 처리했을 때 한 건만 발급되고 나머지는 실패로 끝나는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java`
around lines 43 - 59, Replace the check-then-act duplicate guard in
CouponIssuanceService with a DB-enforced uniqueness path: add a unique
constraint on user_coupon(coupon_id, user_id), remove reliance on
existsByCouponIdAndUserId for correctness, and handle
DataIntegrityViolationException around
userCouponRepository.save(coupon.issue(userId)) by marking the request as
ALREADY_ISSUED. Keep the stock reservation flow in CouponIssuanceService intact,
and add a concurrency test for concurrent issuance of the same couponId/userId
ensuring only one save succeeds and the rest are recorded as failed.
Source: Path instructions
Comment on lines
+56
to
+58
| CouponModel coupon = couponRepository.findById(couponId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, | ||
| "[id = " + couponId + "] 쿠폰을 찾을 수 없습니다.")); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 컨슈머의 예외 처리/ack 정책 확인
fd CouponIssueRequestConsumer.java --exec cat {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1994
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 서비스 본문과 상태 전이 경로 확인
FILE="apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java"
echo "== $FILE =="
cat -n "$FILE" | sed -n '1,220p'
echo
echo "== request.markFailed / REQUESTED references =="
rg -n "markFailed|REQUESTED|markSuccess|markCompleted|issueOne|findById\\(" apps/commerce-api/src/main/java/com/loopers -SRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 11031
쿠폰 미존재 실패를 REQUESTED에 남기지 않도록 처리해야 한다
CouponIssuanceService.java:56-60에서couponRepository.findById(couponId)가 실패하면 트랜잭션이 롤백되고, 현재CouponIssueRequestConsumer는 예외를 잡아 로그만 남긴 뒤 ack 하므로 해당 요청이 영구히REQUESTED로 남는다.markFailed("NOT_FOUND")를 같은 트랜잭션 안에서 호출하도록 분기하거나, 최소한 이 예외는 재시도/DLQ로 보내도록 소비자 흐름을 바꿔 상태가 고정되지 않게 해야 한다.couponId가 잘못되었거나 쿠폰이 삭제된 경우에REQUESTED -> FAILED전이와 메시지 ack/DLQ 동작을 검증하는 통합 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java`
around lines 56 - 58, 쿠폰 조회 실패 시 트랜잭션 롤백만 되고 `CouponIssueRequestConsumer`가 예외를
삼킨 뒤 ack해서 요청이 `REQUESTED`에 고정되는 문제를 수정하세요. `CouponIssuanceService`의 쿠폰 조회/발급
흐름에서 `couponRepository.findById(couponId)` 실패를 `markFailed("NOT_FOUND")`로 같은 처리
경로 안에서 반영하거나, 최소한 이 예외가 소비자에서 재시도 또는 DLQ로 넘어가도록 `CouponIssueRequestConsumer` 예외
처리와 ack 방식을 바꾸세요. `couponId`가 없거나 삭제된 쿠폰인 경우 `REQUESTED -> FAILED` 전이와 메시지
ack/DLQ 동작을 검증하는 통합 테스트를 추가하세요.
Source: Path instructions
Comment on lines
+25
to
+29
| @CacheEvict(cacheNames = CacheConfig.PRODUCT_DETAIL, key = "#productId") | ||
| @Transactional | ||
| public void increase(Long productId) { | ||
| productRepository.increaseLikeCount(productId); // 원자적 UPDATE (동시 증가에도 안전) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files | rg 'apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter\.java|CacheConfig|`@EnableCaching`|`@EnableTransactionManagement`|CacheManager|transactionAware|ProductLikeCounter'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 329
🏁 Script executed:
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.java && printf '\n---CACHECONFIG---\n' && sed -n '1,260p' apps/commerce-api/src/main/java/com/loopers/config/CacheConfig.java && printf '\n---ENABLE SEARCH---\n' && rg -n "`@EnableCaching`|`@EnableTransactionManagement`|transactionAware|CacheManager|RedisCacheManager|ConcurrentMapCacheManager|TransactionAwareCacheManagerProxy" apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4427
🏁 Script executed:
rg -n "`@CacheEvict`|PRODUCT_DETAIL|productDetail|increaseLikeCount|decreaseLikeCount|LoggingCacheErrorHandler|CacheErrorHandler" apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4142
🏁 Script executed:
sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/config/LoggingCacheErrorHandler.java && printf '\n---LIKEFACADE---\n' && sed -n '1,220p' apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java && printf '\n---PRODUCTFACADE---\n' && sed -n '1,180p' apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6239
increase와 decrease의 캐시 무효화는 커밋 이후로 분리해야 한다다. @CacheEvict와 @Transactional을 같은 메서드에 두면 무효화가 커밋보다 먼저 실행될 수 있어, 동시 조회가 이전 productDetail을 다시 캐시에 넣고 최대 5분 동안 오래된 값이 노출될 수 있다다. TransactionSynchronizationManager.afterCommit으로 옮기거나 transaction-aware cache manager를 쓰고, 커밋 전후 캐시 상태를 검증하는 통합 테스트를 추가해야 한다다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.java`
around lines 25 - 29, `ProductLikeCounter.increase` and `decrease` currently
combine `@CacheEvict` with `@Transactional`, which can evict the cache before
the database transaction commits. Move the `CacheConfig.PRODUCT_DETAIL` eviction
in both methods to an after-commit hook such as
`TransactionSynchronizationManager.afterCommit`, or switch to a
transaction-aware cache manager, and keep the atomic repository update methods
like `productRepository.increaseLikeCount` in the transactional block. Add an
integration test around `ProductLikeCounter` to verify the cache is only
invalidated after commit and that stale `productDetail` data cannot be
repopulated during the transaction.
Comment on lines
+27
to
+29
| public void increase(Long productId) { | ||
| productRepository.increaseLikeCount(productId); // 원자적 UPDATE (동시 증가에도 안전) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A8 'increaseLikeCount|decreaseLikeCount' apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java 2>/dev/nullRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 3091
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ProductJpaRepository ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java
printf '\n== ProductRepositoryImpl ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java
printf '\n== LikeCountAggregationListener ==\n'
fd -a 'LikeCountAggregationListener.java' apps/commerce-api/src/main/java || true
if [ -f apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.java ]; then
cat -n apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountAggregationListener.java
fi
printf '\n== Search for affected rows / return values / exceptions ==\n'
rg -n -A4 -B4 'affected rows|row count|update .*count|increaseLikeCount\(|decreaseLikeCount\(|catch \(|try \{' apps/commerce-api/src/main/java/com/loopers -g '!**/target/**'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 35935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Product deletion / removal paths ==\n'
rg -n -A3 -B3 'deleteProduct|removeProduct|deleteById|delete\(|remove\(' apps/commerce-api/src/main/java/com/loopers -g '!**/target/**'
printf '\n== ProductLikeCounter ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.java
printf '\n== Repository interface for like count return types ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.java
printf '\n== Search for tests around like count / listener ==\n'
rg -n -A4 -B4 'LikeCountAggregationListener|increaseLikeCount|decreaseLikeCount|ProductLikeCounter' apps/commerce-api/src/test apps/commerce-api/src/main/java/com/loopers -g '!**/target/**'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 18470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Product delete/disable/search ==\n'
rg -n -A3 -B3 'Product.*delete|delete.*Product|soft delete|deactivate|disabled|isDeleted|deletedAt|removedAt' apps/commerce-api/src/main/java/com/loopers apps/commerce-api/src/test/java/com/loopers -g '!**/target/**'
printf '\n== Product service/facade/repository APIs ==\n'
fd -a 'Product*Facade.java' apps/commerce-api/src/main/java/com/loopers
fd -a 'Product*Service.java' apps/commerce-api/src/main/java/com/loopers
fd -a 'Product*Controller.java' apps/commerce-api/src/main/java/com/loopers
printf '\n== Product domain/infrastructure files ==\n'
fd -a 'Product*.java' apps/commerce-api/src/main/java/com/loopers | sed -n '1,200p'
printf '\n== ProductModel ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 10605
0건 UPDATE는 실패로 전파해야 한다
increaseLikeCount/decreaseLikeCount가 affected rows를 무시하면 soft delete 된 상품이나 누락된 상품의 집계가 0건 UPDATE로 조용히 사라진다. ProductJpaRepository가 업데이트 건수를 반환하고 0건이면 예외를 던지거나 경고 로그를 남겨 LikeCountAggregationListener에서 실패가 남도록 바꿔야 한다. 삭제된 상품과 존재하지 않는 상품에 대한 증가·감소 테스트를 추가해야 한다다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/ProductLikeCounter.java`
around lines 27 - 29, The ProductLikeCounter increase/decrease flow currently
ignores whether ProductJpaRepository.updateLikeCount actually affected any rows,
so soft-deleted or missing products can fail silently. Update the repository
methods (increaseLikeCount/decreaseLikeCount) to return the affected-row count,
then have ProductLikeCounter and LikeCountAggregationListener treat 0 as a
failure by throwing an exception or emitting a clear warning log. Add tests
covering increase/decrease for deleted products and non-existent products to
verify the failure is propagated.
Comment on lines
+42
to
+58
| @Scheduled(fixedDelayString = "${loopers.outbox.relay.interval-ms:1000}") | ||
| @Transactional | ||
| public void relay() { | ||
| List<OutboxEvent> pending = outboxRepository.findPending(batchSize); | ||
| for (OutboxEvent event : pending) { | ||
| try { | ||
| // key=partitionKey 로 파티션 라우팅 → 같은 상품/주문의 순서 보장. | ||
| // .get() 으로 브로커 ack(acks=all) 를 기다려 발행 성공을 확인한 뒤 SENT 로 마킹. | ||
| outboxKafkaTemplate.send(event.getTopic(), event.getPartitionKey(), event.getPayload()).get(); | ||
| event.markSent(); // dirty checking → 커밋 시 UPDATE | ||
| } catch (Exception e) { | ||
| log.warn("[Outbox] 발행 실패 eventId={} topic={} → 다음 주기 재시도", | ||
| event.getEventId(), event.getTopic(), e); | ||
| break; // 순서 보장을 위해 이후 발행도 중단 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'OutboxKafkaConfig' --exec cat -n {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== OutboxRelay ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxRelay.java
printf '\n== Search timeouts ==\n'
rg -n "request\.timeout\.ms|delivery\.timeout\.ms|sendTimeout|timeout" apps/commerce-api/src/main/java apps/commerce-api/src/test/java -g '!**/target/**' || true
printf '\n== Outbox repository ==\n'
fd -i 'OutboxRepository' apps/commerce-api/src/main/java --exec sh -c 'echo "--- {}"; cat -n "{}"' \;
printf '\n== Outbox event ==\n'
fd -i 'OutboxEvent' apps/commerce-api/src/main/java --exec sh -c 'echo "--- {}"; cat -n "{}"' \;Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 13397
OutboxRelay.relay()에 명시적 타임아웃과 실패 격리가 필요하다.
@Transactional안에서send(...).get()을 무제한 대기하면 브로커 지연 시 스케줄러 스레드와 DB 트랜잭션을 오래 점유해 커넥션 풀 고갈과 전체 릴레이 지연으로 이어진다.- 첫 실패에서
break하므로 뒤의 pending 이벤트가 모두 정체된다. 현재는id오름차순 전역 배치이므로, 한 이벤트가 반복 실패하면 서로 다른partitionKey의 이벤트도 함께 막힌다. get(timeout)을 적용하고, 실패 시에는 동일partitionKey범위로만 중단하거나 나머지 이벤트는 계속 처리하도록 분리해야 한다. 브로커 지연과 영구 실패 상황에서 트랜잭션 종료 시간과 이후 이벤트 처리 여부를 검증하는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/application/outbox/OutboxRelay.java`
around lines 42 - 58, OutboxRelay.relay() currently blocks indefinitely on
outboxKafkaTemplate.send(...).get() inside a `@Transactional` method and stops the
whole batch on the first failure, which can hold scheduler threads/DB
transactions too long and block unrelated partitionKey events. Update relay() to
use a bounded wait on send with a timeout, then handle failures so only the
affected partitionKey or event range is paused while other pending OutboxEvent
items continue processing. Add tests around OutboxRelay.relay(),
outboxRepository.findPending, and outboxKafkaTemplate.send to verify broker
delay, timeout handling, and that non-failing events still get marked sent.
| /** | ||
| * 결제가 실패했다는 "사실"(Event). | ||
| */ | ||
| public record PaymentFailedEvent(Long paymentId, Long orderId, Long userId, String reason) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n[1] PaymentFailedEvent and reason usages\n'
rg -n --hidden --glob '!**/build/**' --glob '!**/target/**' \
'PaymentFailedEvent|PaymentNotificationListener|reason\)' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
printf '\n[2] Files containing PaymentNotificationListener by name\n'
fd -a 'PaymentNotificationListener.*' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
printf '\n[3] Show PaymentFailedEvent file\n'
sed -n '1,120p' apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentFailedEvent.java || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n[1] Search for PaymentNotificationListener and PaymentFailedEvent usages\n'
rg -n --hidden --glob '!**/build/**' --glob '!**/target/**' \
'PaymentNotificationListener|PaymentFailedEvent|new PaymentFailedEvent|reason\)' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
printf '\n[2] Locate listener file names\n'
fd -a 'PaymentNotificationListener.*' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
printf '\n[3] Open PaymentFailedEvent\n'
cat -n apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentFailedEvent.java || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
apps/commerce-api/src/main/java/com/loopers/application/payment/PaymentNotificationListener.java \
apps/commerce-api/src/main/java/com/loopers/application/activity/UserActivityLogListener.java
do
echo "===== $f ====="
cat -n "$f"
echo
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4307
reason을 그대로 로그에 남기지 않도록 수정해야 한다.
PaymentNotificationListener와 UserActivityLogListener가 reason을 원문 그대로 출력하므로, PG 원본 오류나 내부 식별자가 운영 로그에 남을 수 있다. 로그는 접근 범위가 넓어 노출 피해가 크므로, 원본 문자열 대신 오류 코드로 치환하거나 마스킹한 값만 기록해야 한다. onPaymentFailed에서 원문 reason이 그대로 포함되지 않는지 확인하는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentFailedEvent.java`
at line 6, `PaymentFailedEvent`의 `reason`이 운영 로그에 원문 그대로 남는 문제를 수정해야 합니다.
`PaymentNotificationListener`와 `UserActivityLogListener`의 `onPaymentFailed`에서
`reason`을 직접 출력하지 말고, 오류 코드로 매핑하거나 마스킹된 값만 로그에 남기도록 변경하세요. 관련 로그 포맷을 `reason`
문자열에 의존하지 않게 바꾸고, `onPaymentFailed` 테스트를 추가해 원문 `reason`이 로그 메시지에 포함되지 않는지
검증하세요.
Comment on lines
+12
to
+13
|
|
||
| boolean existsByCouponIdAndUserId(Long couponId, Long userId); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# UserCouponModel 엔티티 및 관련 마이그레이션에서 인덱스/유니크 제약 확인
fd -e java -i usercouponmodel --exec cat {}
fd -e sql -i migration --exec grep -l -i "user_coupon" {} \;Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] Locate repository and migration files"
git ls-files | rg 'UserCouponJpaRepository\.java|UserCouponModel\.java|migration|schema|flyway|liquibase|user_coupon' || true
echo
echo "[2] Show repository definition"
repo_file="$(git ls-files | rg 'UserCouponJpaRepository\.java$' | head -n 1)"
if [ -n "${repo_file:-}" ]; then
cat -n "$repo_file"
fi
echo
echo "[3] Search for user_coupon table/index/unique definitions"
git ls-files | rg '\.(sql|java|yml|yaml)$' | while read -r f; do
rg -n -i 'user_coupon|coupon_id|user_id|unique|index' "$f" || true
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] UserCouponModel annotations"
file="apps/commerce-api/src/main/java/com/loopers/domain/coupon/UserCouponModel.java"
cat -n "$file" | sed -n '1,120p'
echo
echo "[2] Search repository and migration/DDL files for user_coupon table definition"
git ls-files | rg '\.(sql|java|yml|yaml)$' | while read -r f; do
rg -n -i 'create table.*user_coupon|table.*user_coupon|user_coupon[^a-zA-Z0-9_]|unique.*user_id.*coupon_id|coupon_id.*user_id|index.*user_id.*coupon_id|coupon_id.*user_id' "$f" || true
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] Migration-related files"
git ls-files | rg '(^|/)(db|database|schema|migration|migrations|flyway|liquibase)(/|$)|\.(sql|yaml|yml)$' || true
echo
echo "[2] user_coupon DDL or index definitions in migration-related files"
git ls-files | rg '(^|/)(db|database|schema|migration|migrations|flyway|liquibase)(/|$)|\.(sql|yaml|yml)$' | while read -r f; do
rg -n -i 'user_coupon|user_id|coupon_id|unique|index|create table' "$f" || true
doneRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1642
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] usages of existsByCouponIdAndUserId"
rg -n "existsByCouponIdAndUserId" apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
echo
echo "[2] coupon issuance service / facade"
rg -n "issue\\(|requestIssue\\(|UserCouponJpaRepository|existsByCouponIdAndUserId" apps/commerce-api/src/main/java/com/loopers -A 4 -B 4 || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 20650
(coupon_id, user_id) 유니크 제약과 복합 인덱스를 추가해야 한다다
현재 existsByCouponIdAndUserId로 중복 발급을 막고 있지만, DB 제약이 없으면 동시 요청에서 두 트랜잭션이 모두 통과해 중복 쿠폰이 저장될 수 있다. user_coupon에 (coupon_id, user_id) 유니크 제약과 복합 인덱스를 추가하고, 중복 키 충돌은 ALREADY_ISSUED로 처리하라. 동시 발급 시 중복이 1건만 저장되는 통합 테스트와 유니크 제약 위반 처리 테스트를 추가하라.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/UserCouponJpaRepository.java`
around lines 12 - 13, `UserCouponJpaRepository`의 `existsByCouponIdAndUserId`만으로는
동시 발급 중복을 막을 수 없으니, `user_coupon` 엔티티/매핑에 `(couponId, userId)` 유니크 제약과 복합 인덱스를
추가하고 저장 시 중복 키 충돌을 `ALREADY_ISSUED`로 변환하도록 처리하세요. 해당 제약은 `UserCoupon`(또는 이에 대응하는
JPA 엔티티)에서 적용되도록 확인하고, 발급 로직이 이를 잡아 서비스 예외로 매핑하는지 점검하세요. 또한 동시 발급 상황에서 한 건만 저장되는
통합 테스트와 유니크 제약 위반이 `ALREADY_ISSUED`로 처리되는 테스트를 추가하세요.
Source: Path instructions
Comment on lines
+36
to
+46
| public void consume(List<ConsumerRecord<String, byte[]>> records, Acknowledgment acknowledgment) { | ||
| for (ConsumerRecord<String, byte[]> record : records) { | ||
| try { | ||
| CouponIssueMessage message = objectMapper.readValue(record.value(), CouponIssueMessage.class); | ||
| couponIssuanceService.issue(message.requestId(), message.couponId(), message.userId()); | ||
| } catch (Exception e) { | ||
| log.error("[coupon-issue-requests] 처리 실패 offset={} → 스킵", record.offset(), e); | ||
| } | ||
| } | ||
| acknowledgment.acknowledge(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java' \
'apps/commerce-api/src/main/java/com/loopers/application/*' \
'apps/commerce-api/src/main/java/com/loopers/domain/*' \
'apps/commerce-api/src/test/java/**/*Coupon*' 2>/dev/null || true
printf '\n== consumer outline ==\n'
ast-grep outline apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java --view expanded || true
printf '\n== consumer source (numbered) ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java
printf '\n== search state transitions and issue() ==\n'
rg -n "issue\\(|REQUESTED|ISSUED|FAILED|CouponIssueRequest" apps/commerce-api/src/main/java apps/commerce-api/src/test/java
printf '\n== service candidates ==\n'
fd -a 'CouponIssuanceService.java|CouponIssueRequestService.java|CouponIssueRequest.*java' apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 26857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CouponIssuanceService ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssuanceService.java
printf '\n== CouponIssueRequest entity ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java
printf '\n== CouponIssueStatus ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueStatus.java
printf '\n== consumer/container config references ==\n'
rg -n "BATCH_LISTENER|AckMode|Acknowledgment|manual ack|enableAutoCommit|DefaultErrorHandler|DeadLetter" apps/commerce-api/src/main/java apps/commerce-api/src/test/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CouponIssueMessage ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/application/outbox/message/CouponIssueMessage.java
printf '\n== KafkaConfig ==\n'
cat -n apps/commerce-api/src/main/java/com/loopers/confg/kafka/KafkaConfig.java
printf '\n== coupon consumer-related tests ==\n'
rg -n "CouponIssueRequestConsumer|coupon-issue-requests|BATCH_LISTENER|Acknowledgment|DefaultErrorHandler|DeadLetter" apps/commerce-api/src/test/java apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== kafka config candidates ==\n'
fd -a 'KafkaConfig.java|*Kafka*Config*.java|*ErrorHandler*.java|*DeadLetter*.java' apps/commerce-api/src/main/java apps/commerce-api/src/test/java
printf '\n== error handler / DLQ / deserializer references ==\n'
rg -n "DefaultErrorHandler|DeadLetterPublishingRecoverer|ErrorHandlingDeserializer|CommonErrorHandler|SeekToCurrent|BackOff|Retry|DLT|dead letter|DLQ|AckMode|BATCH_LISTENER" apps/commerce-api/src/main/java apps/commerce-api/src/test/java
printf '\n== spring kafka config files ==\n'
rg -n "ConcurrentKafkaListenerContainerFactory|DefaultErrorHandler|ConsumerFactory|ProducerFactory|KafkaTemplate" apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== kafka-related files ==\n'
fd -a -g '*Kafka*Config*.java' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
fd -a -g '*ErrorHandler*.java' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
fd -a -g '*DeadLetter*.java' apps/commerce-api/src/main/java apps/commerce-api/src/test/java || true
printf '\n== kafka config / listener factory references ==\n'
rg -n "ConcurrentKafkaListenerContainerFactory|DefaultErrorHandler|DeadLetterPublishingRecoverer|ErrorHandlingDeserializer|CommonErrorHandler|AckMode|BATCH_LISTENER|KafkaTemplate" apps/commerce-api/src/main/java apps/commerce-api/src/test/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2165
배치 소비자에서 예외를 삼키지 말고 재시도/DLQ 경로로 넘겨야 한다.
CouponIssuanceService.issue()는 중복/품절 같은 비즈니스 실패를 이미FAILED로 전이하지만, 이 consumer가Exception을 전부 잡아 버리면 역직렬화 실패나 서비스 밖의 런타임 오류는 상태 전이 없이 지나가REQUESTED가 남는다.- 배치 마지막에만
acknowledge()하므로ack이전에 프로세스가 종료되면 앞서 성공한 레코드까지 다시 전달될 수 있다.Exception은 유형별로 분리해 비즈니스 실패만 서비스에서 처리하고, 그 외 오류는 재던지거나 DLQ로 보내도록 바꾸는 편이 낫다. - 추가 테스트로는 배치에 정상 레코드와 역직렬화 실패/런타임 예외 레코드가 섞인 경우, 성공 레코드가 중복 발급되지 않고 실패 레코드가
FAILED또는 DLQ로 흘러가는지 확인하라.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/consumer/CouponIssueRequestConsumer.java`
around lines 36 - 46, The batch consumer in CouponIssueRequestConsumer is
swallowing all exceptions, which hides deserialization and non-business failures
from retry/DLQ handling. Refactor consume() to catch only expected business
exceptions from couponIssuanceService.issue(), and let unexpected errors from
ObjectMapper.readValue or other runtime failures propagate or be routed to DLQ
instead of always logging and skipping. Keep acknowledgment.acknowledge() only
after the batch is fully handled, and add tests covering mixed success plus
deserialization/runtime failure cases to ensure failed records are marked FAILED
or sent to DLQ without duplicating successful coupon issuance.
Comment on lines
+29
to
+68
| public void handleCatalog(CatalogEventMessage message) { | ||
| if (alreadyHandled(message.eventId())) { | ||
| return; | ||
| } | ||
| ProductMetrics metrics = getOrCreate(message.productId()); | ||
| switch (message.eventType()) { | ||
| case "LIKE_ADDED" -> metrics.addLike(1, message.occurredAt()); | ||
| case "LIKE_REMOVED" -> metrics.addLike(-1, message.occurredAt()); | ||
| case "PRODUCT_VIEWED" -> metrics.addView(message.occurredAt()); | ||
| default -> log.warn("[metrics] 알 수 없는 catalog eventType={}", message.eventType()); | ||
| } | ||
| metricsRepository.save(metrics); | ||
| handledEventRepository.save(HandledEvent.of(message.eventId(), message.eventType())); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void handleOrder(OrderEventMessage message) { | ||
| if (alreadyHandled(message.eventId())) { | ||
| return; | ||
| } | ||
| if ("ORDER_PLACED".equals(message.eventType()) && message.lines() != null) { | ||
| for (OrderEventMessage.Line line : message.lines()) { | ||
| ProductMetrics metrics = getOrCreate(line.productId()); | ||
| metrics.addSales(line.quantity(), message.occurredAt()); | ||
| metricsRepository.save(metrics); | ||
| } | ||
| } else { | ||
| log.warn("[metrics] 알 수 없는 order eventType={}", message.eventType()); | ||
| } | ||
| handledEventRepository.save(HandledEvent.of(message.eventId(), message.eventType())); | ||
| } | ||
|
|
||
| private boolean alreadyHandled(String eventId) { | ||
| return handledEventRepository.existsByEventId(eventId); | ||
| } | ||
|
|
||
| private ProductMetrics getOrCreate(Long productId) { | ||
| return metricsRepository.findByProductId(productId) | ||
| .orElseGet(() -> ProductMetrics.create(productId)); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ProductMetrics/BaseEntity 에 낙관적 락(`@Version`) 존재 여부 확인
fd -a 'BaseEntity.java' apps/commerce-streamer | xargs -r cat -n
fd -a 'ProductMetrics.java' apps/commerce-streamer | xargs -r rg -n '`@Version`'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files 'apps/commerce-streamer/src/main/java/com/loopers/application/metrics/*'
echo
echo "== MetricsEventHandler =="
cat -n apps/commerce-streamer/src/main/java/com/loopers/application/metrics/MetricsEventHandler.java
echo
echo "== ProductMetrics =="
cat -n apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java
echo
echo "== BaseEntity candidates =="
fd -a 'BaseEntity.java' apps/commerce-streamer || true
echo
echo "== `@Version` in metrics package =="
rg -n '`@Version`|`@OptimisticLock`|version' apps/commerce-streamer/src/main/java/com/loopers -g '*.java' || true
echo
echo "== repositories =="
rg -n 'interface .*Repository|save\\(|findByProductId|increment|update ' apps/commerce-streamer/src/main/java/com/loopers -g '*.java' || trueRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 6416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== consumer/listener config =="
rg -n 'ConsumerGroup|groupId|`@KafkaListener`|`@RabbitListener`|concurrency|partitionKey|productId' apps/commerce-streamer/src/main/java -g '*.java' || true
echo
echo "== handled event entity/repository =="
fd -a 'HandledEvent.java' apps/commerce-streamer | xargs -r cat -n
echo
fd -a 'HandledEventRepository.java' apps/commerce-streamer | xargs -r cat -n
echo
echo "== BaseEntity search in repo =="
rg -n 'class BaseEntity|`@Version`' apps/commerce-streamer -g '*.java' || true
echo
echo "== ProductMetrics repository =="
fd -a 'ProductMetricsRepository.java' apps/commerce-streamer | xargs -r cat -nRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5801
MetricsEventHandler의 read-modify-write 경합을 제거해야 한다 apps/commerce-streamer/src/main/java/com/loopers/application/metrics/MetricsEventHandler.java의 getOrCreate → 수정 → save는 catalog-metrics-consumer와 order-metrics-consumer가 서로 다른 groupId로 같은 product_id를 동시에 갱신할 때 마지막 커밋이 앞선 증분을 덮어쓰고, 최초 생성 경합에서는 product_id 유니크 제약 위반으로 처리 실패를 만든다다. 운영에서는 상품 지표가 간헐적으로 실제보다 작게 집계되어 재처리로도 원인 추적이 어렵다다. @Version + 재시도 또는 DB 원자적 upsert/증분 쿼리로 바꿔 신규 생성과 누적 갱신을 하나의 원자 경로로 묶어야 한다다. 같은 productId에 대해 handleCatalog와 handleOrder를 병렬 실행했을 때 지표가 누락 없이 합산되는지, 최초 insert 경합 시 한 건만 생성되고 나머지가 재시도되는지 테스트를 추가해야 한다다
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/metrics/MetricsEventHandler.java`
around lines 29 - 68, MetricsEventHandler still does a non-atomic
read-modify-write in getOrCreate, then mutates and saves in handleCatalog and
handleOrder, which can lose increments or fail on concurrent first insert for
the same productId. Fix this by making ProductMetrics updates atomic with either
optimistic locking on ProductMetrics plus retry, or a database-side
upsert/increment path in metricsRepository so creation and accumulation happen
in one safe flow. Keep the existing handleCatalog, handleOrder, and getOrCreate
entry points, but ensure concurrent events for the same productId cannot
overwrite each other, and add tests covering parallel catalog/order updates and
first-create contention.
Comment on lines
+34
to
+45
| public void consume(List<ConsumerRecord<String, byte[]>> records, Acknowledgment acknowledgment) { | ||
| for (ConsumerRecord<String, byte[]> record : records) { | ||
| try { | ||
| CatalogEventMessage message = objectMapper.readValue(record.value(), CatalogEventMessage.class); | ||
| metricsEventHandler.handleCatalog(message); | ||
| } catch (Exception e) { | ||
| // 개별 메시지 처리 실패는 로깅 후 스킵(포이즌 메시지가 파티션을 막지 않게). 필요 시 DLQ. | ||
| log.error("[catalog-events] 처리 실패 offset={} → 스킵", record.offset(), e); | ||
| } | ||
| } | ||
| acknowledgment.acknowledge(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
배치 일부 실패 시에도 전체 오프셋을 커밋해 이벤트가 영구 유실된다.
루프 내부에서 개별 레코드 처리 실패(역직렬화 오류, DB 제약 위반, 일시적 DB 장애 등)는 로그만 남기고 무시되는데, 루프 종료 후 acknowledgment.acknowledge() 를 무조건 호출한다(Line 44). 재시도나 DLQ 라우팅이 없으므로 실패한 이벤트는 다시 소비되지 않고 그대로 사라진다. MAX_POLLING_SIZE=3000 기준 대량 배치를 처리하는 구조상, 일시적 DB 부하나 앞서 지적한 lost-update 성 제약 위반이 발생하면 다수 이벤트가 조용히 누락될 수 있다.
product_metrics 는 "결과적 일관성" 을 전제로 설계되었으나, 이 구조에서는 결과적으로도 반영되지 않는 영구 유실이 발생해 설계 의도와 어긋난다.
수정안: 실패 레코드를 DLQ 로 라우팅(Spring Kafka DefaultErrorHandler + DeadLetterPublishingRecoverer)하거나, 실패 지점 이전까지만 부분 커밋하도록 개선해야 한다.
추가 테스트: 역직렬화 예외/핸들러 예외를 유발하는 레코드를 포함한 배치에 대해, 실패 레코드가 재처리되거나 DLQ 로 전송되는지(단순 스킵+커밋되지 않는지) 검증하는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java`
around lines 34 - 45, The batch consumer in CatalogEventConsumer.consume
currently logs per-record failures but still unconditionally calls
acknowledgment.acknowledge() after the loop, which can permanently skip failed
events. Update the error-handling flow so failed records are either routed to a
DLQ via Spring Kafka error handling (for example, DefaultErrorHandler with
DeadLetterPublishingRecoverer) or prevent committing past the failed offset by
only acknowledging successful records. Keep the fix centered on consume and its
acknowledgment behavior, and add a test covering deserialize/handler failures to
verify the failed record is retried or sent to DLQ instead of being skipped.
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
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.
🧭 Context & Decision
문제 정의
LikeFacade.like()가 한@Transactional안에서 (1) 좋아요 행 저장과 (2)product.like_count집계(원자적 UPDATE)를 함께 처리하고 있었다. 둘이 한 트랜잭션에 묶여 있어, 집계 쪽이 실패하면 좋아요 자체가 롤백된다.선택지와 결정
@TransactionalEventListener(AFTER_COMMIT)+@Transactional(REQUIRES_NEW)동기 리스너좋아요 커밋 후 별도 트랜잭션에서 집계. "집계 실패 무관 성공"은 만족한다. 하지만 동기 실행 시 원래 커넥션을 쥔 채 두 번째 커넥션(REQUIRES_NEW)을 요구한다. 고동시성에서 각 스레드가 커넥션 1개를 잡고 2번째를 기다리며 Hikari 풀 데드락 → 타임아웃 → (실패를 삼키는 try/catch 때문에) 집계 유실. 실측에서 50 기대에 8만 반영됐다.
@Async+AFTER_COMMIT비동기 리스너 (채택)좋아요 트랜잭션이 커넥션을 반납한 뒤 별도 스레드에서 집계 → 커넥션을 한 번에 하나만 사용 → 풀 데드락 없음. 집계 UPDATE는 원자적이라 동시성에도 안전. 순서 보장을 위해 단일 스레드 executor(core=max=1) 를 쓴다(추가/취소가 뒤바뀌어 감소 가드
like_count > 0에 걸리는 문제 방지).LikeCountAggregationListener를@Async("eventTaskExecutor") @TransactionalEventListener(AFTER_COMMIT)로 두고, 실제 UPDATE는ProductLikeCounter(별도@Transactional빈)로 위임.@Async와@Transactional을 한 메서드에 겹칠 때의 인터셉터 순서 모호성을 피하기 위해 "언제 실행할지(리스너)"와 "무엇을 트랜잭션으로 처리할지(빈)"를 분리했다.Awaitility로 "최종적으로 N" 검증으로 바꿨다.@Async는 재시작 시 큐에 남은 이벤트가 유실될 수 있다. Step 2에서 이 경계를 Transactional Outbox → Kafka → commerce-streamer로 승격해, 발행 내구성(At-Least-Once)과 시스템 간 전파를 확보했다.🤔 고민한 점 / 막혔던 부분
AFTER_COMMIT이면 다 되는 줄 알았다. 커밋 이후니까 좋아요는 안전하다고 봤는데, 정작REQUIRES_NEW가 원래 커넥션을 아직 쥔 상태에서 두 번째 커넥션을 요구한다는 걸 실측(50→8)으로야 알았다. "동기 AFTER_COMMIT"과 "비동기"는 커넥션 점유 관점에서 전혀 다른 얘기였다.like()가 상세 캐시를 evict하지만, 집계는 그 후 비동기로 일어나 캐시에 옛날 값(0) 이 다시 적재됐다. 집계 반영 지점(ProductLikeCounter)에서 한 번 더 캐시를 evict해 정합성을 맞췄다. "좋아요 수가 바로 보여야 하나?"라는 질문의 답을 코드로 내린 셈이다.