Repository navigation
[Volume-7] 이벤트 기반 아키텍처 구현 - #318
Merged
Merged
Conversation
📝 Walkthrough변경 목적: 이벤트 기반 아키텍처로 내부 후속 처리와 외부 전파를 분리하고, 유실 허용 여부에 따라 Walkthrough좋아요/주문/결제/조회에 도메인 이벤트 기반 부가효과 처리(비동기 리스너)를 도입하고, outbox 패턴으로 Kafka에 전파하는 구조를 commerce-api에 추가했다. commerce-streamer에 지표 집계(product_metrics)와 멱등 처리(event_handled), DLQ 격리를 갖춘 Kafka 소비자를 신설했다. 또한 한도 있는 쿠폰의 동기 발급을 차단하고 요청 접수→비동기 처리→폴링 방식의 선착순 발급 흐름(CouponIssueRequest, CouponIssueProcessor)을 구현했다. Changes도메인 이벤트 및 부가효과 리스너 분리
Outbox 패턴 기반 Kafka 전파(commerce-api)
선착순 쿠폰 발급(요청-비동기 처리)
commerce-streamer 지표 집계 및 멱등 처리
인프라 설정 및 설계 문서
Sequence Diagram(s)sequenceDiagram
participant Client
participant CouponV1Controller
participant CouponApplicationService
participant CouponIssueRequestRepository
participant OutboxEventJpaRepository
participant OutboxRelay
participant Kafka
participant CouponIssueConsumer
participant CouponIssueProcessor
participant CouponIssueJdbcRepository
Client->>CouponV1Controller: POST /coupons/{templateId}/issue-requests
CouponV1Controller->>CouponApplicationService: requestIssue(userId, templateId)
CouponApplicationService->>CouponIssueRequestRepository: save(PENDING)
CouponApplicationService->>OutboxEventJpaRepository: save(pending outbox)
CouponApplicationService-->>Client: requestId
OutboxRelay->>OutboxEventJpaRepository: findTop100ByStatus(PENDING)
OutboxRelay->>Kafka: send(CouponIssueMessage)
OutboxRelay->>OutboxEventJpaRepository: markPublished()
Kafka->>CouponIssueConsumer: consume(CouponIssueMessage)
CouponIssueConsumer->>CouponIssueProcessor: handle(message)
CouponIssueProcessor->>CouponIssueJdbcRepository: findRequest/tryConsumeSlot/insertUserCoupon
CouponIssueProcessor->>CouponIssueJdbcRepository: updateStatus(SUCCESS|SOLD_OUT|ALREADY_ISSUED|FAILED)
CouponIssueConsumer->>Kafka: acknowledge()
Client->>CouponV1Controller: GET /coupons/issue-requests/{requestId}
CouponV1Controller->>CouponApplicationService: getIssueRequest(userId, requestId)
CouponApplicationService->>CouponIssueRequestRepository: findByRequestId(userId 필터)
CouponApplicationService-->>Client: status
sequenceDiagram
participant LikeApplicationService
participant EventPublisher
participant LikeCountEventListener
participant OutboxEventListener
participant OutboxRelay
participant Kafka
participant CatalogEventConsumer
participant ProductMetricsApplicationService
LikeApplicationService->>EventPublisher: publishEvent(ProductLikedEvent)
EventPublisher->>OutboxEventListener: onProductLiked (BEFORE_COMMIT)
OutboxEventListener->>OutboxEventListener: save pending outbox row
Note over LikeApplicationService: 트랜잭션 커밋
EventPublisher->>LikeCountEventListener: onLiked (AFTER_COMMIT, REQUIRES_NEW)
LikeCountEventListener->>LikeCountEventListener: increaseLikeCount
OutboxRelay->>Kafka: send(CatalogEventMessage)
Kafka->>CatalogEventConsumer: consume(CatalogEventMessage)
CatalogEventConsumer->>ProductMetricsApplicationService: apply(message)
ProductMetricsApplicationService->>ProductMetricsApplicationService: 멱등 검사 후 like_count 갱신
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 10
🧹 Nitpick comments (13)
apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentCompletedEvent.java (1)
11-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value금액 필드가 원시
Long타입으로 노출되어 있다.
amount가Long원시 타입으로 정의되어 있어, 다른 도메인 코드(Money.of(...))에서 사용하는Money값 객체와 경계가 어긋난다. 통화/스케일 검증 로직이 이벤트 전파 경로에서 누락될 위험이 있고, 추후 통화 단위가 추가되거나 금액 계산 규칙이 바뀔 때 이 이벤트만 별도로 수정해야 하는 유지보수 부담이 생긴다. 직렬화(Kafka outbox) 편의를 위해 원시 타입을 쓴 것으로 보이나, 이벤트 생성 시점에Money에서long변환을 명시적으로 하도록 팩토리 메서드를 두는 방안을 고려해볼 만하다.테스트 관점에서도
PaymentCompletedEvent를 생성하는 경계값(음수/0 금액) 케이스가 있는지 확인이 필요하다.As per path instructions, "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다."
🤖 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/PaymentCompletedEvent.java` around lines 11 - 12, `PaymentCompletedEvent` currently exposes `amount` as a raw `Long`, which breaks the `Money` value-object boundary used elsewhere. Update the event construction path so the domain still works with `Money` and only converts to `long` at the event factory/creation point, adding a dedicated factory or mapper on `PaymentCompletedEvent` or the caller that uses `Money.of(...)`. Also add/adjust tests around the `PaymentCompletedEvent` creation flow to cover boundary values like zero and negative amounts.Source: Path instructions
apps/commerce-api/src/main/java/com/loopers/application/product/ProductApplicationService.java (1)
60-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ProductViewedEvent발행을 공통 메서드로 추출하라. 캐시 히트/미스 모두 동일한 조회 로그를 남기므로, 한쪽만 수정되면 운영 지표와 이벤트 스키마가 어긋난다. 공통 헬퍼로 묶고, 캐시 히트/미스 각각에서ProductViewedEvent가 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/application/product/ProductApplicationService.java` around lines 60 - 63, `ProductViewedEvent` 발행 로직이 캐시 히트 경로에만 직접 들어가 있어 히트/미스 간 동작이 흩어져 있습니다. `ProductApplicationService`의 조회 흐름에서 공통 헬퍼(예: 조회 이벤트 발행 메서드)로 `eventPublisher.publishEvent(new ProductViewedEvent(...))`를 추출하고, 캐시 히트와 미스 모두 그 헬퍼를 통해 동일하게 1회씩만 발행되도록 정리하세요. `getProductById`(또는 해당 조회 메서드)와 `ProductViewedEvent`를 기준으로 위치를 찾고, 캐시 히트/미스 각각에서 이벤트가 정확히 한 번 발생하는 테스트를 추가해 검증하세요.apps/commerce-api/src/test/java/com/loopers/application/event/EventPhaseIntegrationTest.java (1)
88-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
awaitLikeCount폴링 로직이 4개 테스트 파일에 중복 구현되어 있다.동일한
Thread.sleep(100)기반 최대 100회 폴링 로직이LikeConcurrencyIntegrationTest,ProductModifyLikeConcurrencyIntegrationTest,ProductApplicationServiceIntegrationTest에도 거의 동일하게 반복된다. 운영 관점에서는, 이후 타임아웃/폴링 간격을 조정해야 할 때 4곳을 모두 손봐야 하고 일부만 수정되면 테스트 간 플래키 허용치가 서로 달라져 CI 신뢰도가 저하될 수 있다.공용 테스트 유틸(
abstract IntegrationTestSupport또는 정적 헬퍼)로 추출하거나,Awaitility(Apache 2.0, 최신 4.3.1) 도입으로 폴링 보일러플레이트를 제거하고 조건 미충족 시 자동으로 상세 실패 메시지를 남기도록 하는 편이 유지보수와 디버깅 모두에 유리하다.♻️ 공용 유틸 추출 예시 (Awaitility 미사용 시)
public final class LikeCountAwaiter { private LikeCountAwaiter() {} public static long await(ProductJpaRepository repo, Long productId, long expected) { long counter = -1L; for (int i = 0; i < 100; i++) { counter = repo.findById(productId).orElseThrow().getLikeCount(); if (counter == expected) return counter; try { Thread.sleep(100); } catch (InterruptedException e) { Thread.currentThread().interrupt(); break; } } return counter; } }Awaitility가 이미 프로젝트 테스트 의존성에 포함되어 있는지 확인이 필요하다.
🤖 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/event/EventPhaseIntegrationTest.java` around lines 88 - 98, The awaitLikeCount polling logic is duplicated across multiple integration tests and should be centralized. Extract the repeated `Thread.sleep(100)`/100-iteration wait loop from `EventPhaseIntegrationTest` and the matching helper in `LikeConcurrencyIntegrationTest`, `ProductModifyLikeConcurrencyIntegrationTest`, and `ProductApplicationServiceIntegrationTest` into a shared test utility or an `IntegrationTestSupport` helper, or replace it with `Awaitility` if already available. Keep the shared helper responsible for waiting on `productJpaRepository.findById(productId).orElseThrow().getLikeCount()` until the expected value is reached so timeout/interval changes only need to be made in one place.apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEvent.java (1)
41-43: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
status컬럼 조회용 인덱스 부재 검토가 필요하다.
OutboxRelay.relay()가findTop100ByStatusOrderByIdAsc(PENDING)으로 폴링하는데, 엔티티/테이블에status(및id) 조합 인덱스가 명시되어 있지 않다. outbox 테이블이 커지면 PENDING 행이 소수라도 전체 스캔 비용이 누적돼 폴링 주기(1초)마다 부하가 커진다.@Table(indexes =@Index(columnList = "status, id"))를 추가하거나 마이그레이션에서 복합 인덱스를 걸 것을 제안한다. 대량 데이터 시나리오에 대한 성능 테스트도 함께 확인이 필요하다.♻️ 제안
`@Entity` -@Table(name = "outbox_events") +@Table(name = "outbox_events", indexes = `@Index`(name = "idx_outbox_status_id", columnList = "status, id")) `@NoArgsConstructor`(access = AccessLevel.PROTECTED) public class OutboxEvent extends BaseEntity {🤖 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/OutboxEvent.java` around lines 41 - 43, The Outbox polling query in OutboxRelay.relay() uses findTop100ByStatusOrderByIdAsc(PENDING), but OutboxEvent does not declare an index for the status/id access pattern. Update the OutboxEvent entity to add a composite index on status and id via `@Table`(indexes = `@Index`(columnList = "status, id")) (or equivalent migration) so the polling query can avoid repeated full scans as the table grows. If there is already a schema migration flow, keep the entity and migration index definitions aligned.apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java (1)
9-10: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
(status, id)복합 인덱스 존재 여부를 확인해야 한다.
OutboxRelay가@Scheduled(fixedDelay = 1000)로 1초마다 이 쿼리를 호출한다.status로 필터링 후id오름차순 정렬하는 쿼리인데, outbox 테이블에 발행 완료(PUBLISHED) 행이 누적될수록 인덱스가 없으면 매 폴링마다 풀스캔에 가까운 비용이 발생해 운영 환경에서 DB 부하 및 relay 지연으로 이어질 수 있다.status단일 컬럼 인덱스만으로는id정렬 비용이 남으므로,(status, id)복합 인덱스를 엔티티/마이그레이션에 추가하는 것을 권장한다. 대량 적재 시나리오를 재현하는 성능 테스트도 함께 추가하면 좋다.As per path instructions,
**/*Repository*.java: "쿼리 조건 누락/과다 조회, 정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다."🤖 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 9 - 10, The OutboxRelay query in OutboxEventJpaRepository relies on filtering by status and ordering by id, so verify that the outbox table has an index matching the query shape. Add or confirm a composite (status, id) index on the OutboxEvent entity or via migration, since a status-only index will still leave sorting costly as PUBLISHED rows accumulate. If needed, review related repository/query definitions and add a performance test that exercises the findTop100ByStatusOrderByIdAsc path with large outbox volume.Source: Path instructions
apps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductMetricsApplicationService.java (1)
40-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
switch문에 미매칭 케이스에 대한 방어(default)가 없다.
CatalogEventType을 셀렉터로 쓰는 이 코드는 값을 반환하지 않는 switch statement이며, arrow 문법이라도 switch expression과 달리 컴파일러가 enum exhaustiveness를 강제하지 않는다. 즉 향후CatalogEventType에 새 값이 추가되고 이 switch가 갱신되지 않으면, 어떤 카운터도 증가시키지 않은 채eventHandled만 기록되어 해당 이벤트는 재처리 기회 없이 영구히 유실된다.수정안:
default분기에서 예외를 던지거나 최소한 경고 로그를 남겨 미매칭 시 조용히 넘어가지 않도록 한다.
추가 테스트: 알 수 없는 타입 유입 시의 동작(예외/로그)을 검증하는 단위 테스트를 추가할 것을 권장한다.♻️ 제안 diff
switch (message.type()) { case PRODUCT_LIKED -> metrics.increaseLike(); case PRODUCT_UNLIKED -> metrics.decreaseLike(); case PRODUCT_VIEWED -> metrics.increaseView(); + default -> throw new IllegalStateException("처리할 수 없는 CatalogEventType: " + message.type()); }🤖 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/ProductMetricsApplicationService.java` around lines 40 - 44, The switch statement in ProductMetricsApplicationService does not guard against unmapped CatalogEventType values, so unknown future event types can be silently ignored. Update the switch on message.type() to include a default branch that either throws an exception or logs a warning before returning, and make sure the handling remains explicit in this method. Add a unit test around ProductMetricsApplicationService to verify the default path behavior when an unexpected CatalogEventType is received.apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java (1)
74-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
increaseSales에 수량 유효성 검사가 없다.운영 관점: 컨슈머가 전달하는
quantity가 음수이거나 0인 경우(주문 라인아이템 파싱 오류, 취소성 이벤트 오분류 등)에도 그대로salesCount에 누적되어 지표가 왜곡될 수 있다.decreaseLike는 0 미만 방지 로직을 두면서increaseSales는 방어가 없어 일관성도 떨어진다.수정안: quantity가 1 이상인지 검증하고, 위반 시 예외를 던져 상위 계층(Kafka 에러 핸들링)이 DLQ로 격리하도록 한다.
추가 테스트:increaseSales(0),increaseSales(-1)호출 시 예외가 발생하는 단위 테스트를ProductMetricsTest에 추가할 것을 권장한다.🛡️ 제안 diff
public void increaseSales(int quantity) { + if (quantity <= 0) { + throw new IllegalArgumentException("quantity 는 1 이상이어야 합니다: " + quantity); + } this.salesCount += quantity; }🤖 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/domain/metrics/ProductMetrics.java` around lines 74 - 76, `ProductMetrics.increaseSales` has no validation for invalid quantities, so zero or negative values can corrupt `salesCount`. Update `increaseSales(int quantity)` to validate that `quantity` is at least 1 and throw an exception when it is not, keeping the behavior consistent with the defensive checks used elsewhere in `ProductMetrics` such as `decreaseLike`. Also add unit tests in `ProductMetricsTest` covering `increaseSales(0)` and `increaseSales(-1)` to verify the exception behavior.apps/commerce-streamer/src/test/java/com/loopers/domain/metrics/ProductMetricsTest.java (1)
26-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win0에서 추가 감소 시 플로어 방어 검증 누락
decreaseLikeFloorsAtZero는 1→0 감소만 확인하고, 이미 0인 상태에서 한 번 더decreaseLike()를 호출했을 때 음수로 내려가지 않는지는 검증하지 않는다.ProductMetrics.decreaseLike()의 핵심 방어 로직(0 이하일 때 무시)이 실제로는 테스트되지 않은 셈이다.As per path instructions, "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."
`@Test` `@DisplayName`("decreaseLike 는 0 미만으로 내려가지 않는다.") void decreaseLikeFloorsAtZero() { ProductMetrics metrics = ProductMetrics.of(1L); metrics.decreaseLike(); assertThat(metrics.getLikeCount()).isZero(); + metrics.decreaseLike(); + assertThat(metrics.getLikeCount()).isZero(); }🤖 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/domain/metrics/ProductMetricsTest.java` around lines 26 - 32, The current ProductMetricsTest only verifies the 1-to-0 case, so the 0-floor guard in ProductMetrics.decreaseLike() is not exercised. Update decreaseLikeFloorsAtZero to also call decreaseLike() again after likeCount reaches zero and assert it stays zero, using ProductMetrics.of() and decreaseLike() as the key symbols to locate the test.Source: Path instructions
apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.java (1)
95-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
recordListenerFactory와couponRecordListenerFactory의 consumer 설정 빌드 로직 중복두 팩토리 모두
MAX_POLL_RECORDS_CONFIG부터MAX_POLL_INTERVAL_MS_CONFIG까지 동일한 6줄을 그대로 복붙했다. 한쪽만 값이 바뀌면 두 리스너의 폴링 특성이 암묵적으로 갈라질 위험이 있다. 공통Map<String,Object> buildConsumerConfig(KafkaProperties)헬퍼로 추출을 권한다.♻️ 제안 리팩터
+ private Map<String, Object> buildTunedConsumerConfig(KafkaProperties kafkaProperties) { + Map<String, Object> consumerConfig = new HashMap<>(kafkaProperties.buildConsumerProperties()); + consumerConfig.put(ConsumerConfig.MAX_POLL_RECORDS_CONFIG, KafkaConfig.MAX_POLLING_SIZE); + consumerConfig.put(ConsumerConfig.FETCH_MIN_BYTES_CONFIG, KafkaConfig.FETCH_MIN_BYTES); + consumerConfig.put(ConsumerConfig.FETCH_MAX_WAIT_MS_CONFIG, KafkaConfig.FETCH_MAX_WAIT_MS); + consumerConfig.put(ConsumerConfig.SESSION_TIMEOUT_MS_CONFIG, KafkaConfig.SESSION_TIMEOUT_MS); + consumerConfig.put(ConsumerConfig.HEARTBEAT_INTERVAL_MS_CONFIG, KafkaConfig.HEARTBEAT_INTERVAL_MS); + consumerConfig.put(ConsumerConfig.MAX_POLL_INTERVAL_MS_CONFIG, KafkaConfig.MAX_POLL_INTERVAL_MS); + return consumerConfig; + }두
@Bean메서드에서 위 헬퍼를 호출하도록 교체한다.🤖 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/KafkaErrorHandlingConfig.java` around lines 95 - 148, The consumer configuration setup in recordListenerFactory and couponRecordListenerFactory is duplicated, which makes the two Kafka listener factories drift over time. Extract the shared KafkaProperties-to-consumerConfig mapping into a single helper such as buildConsumerConfig(KafkaProperties), move the repeated MAX_POLL_RECORDS_CONFIG through MAX_POLL_INTERVAL_MS_CONFIG assignments there, and have both `@Bean` methods call that helper before creating their DefaultKafkaConsumerFactory.apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CatalogEventCollectorIntegrationTest.java (1)
150-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKafka producer 생성 및 폴링 대기 로직이 여러 통합 테스트 파일에서 반복됨
newProducer(),awaitLikeCount()/awaitViewCount()와 동일한 패턴이CatalogEventDlqIntegrationTest,OrderSalesCollectorIntegrationTest,ProductMetricsConcurrentCreateIntegrationTest에도 그대로 복제되어 있다. 공통KafkaIntegrationTestSupport베이스 클래스나 유틸로 추출하면 향후 타임아웃/프로듀서 설정 변경 시 한 곳만 수정하면 된다.🤖 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/interfaces/consumer/CatalogEventCollectorIntegrationTest.java` around lines 150 - 193, `newProducer()`, `awaitLikeCount()`, and `awaitViewCount()` are duplicated across multiple integration tests; extract the shared Kafka producer setup and polling helpers into a common support class or utility such as `KafkaIntegrationTestSupport`. Move the reusable logic there, then update `CatalogEventCollectorIntegrationTest` and the other affected tests to call the shared methods so future timeout or producer config changes only need to be made in one place.apps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductSalesApplicationService.java (1)
39-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winswitch에 default 분기 부재 — 향후 OrderEventType 추가 시 조용한 미집계 위험.
현재
OrderEventType에는PRODUCT_SOLD하나뿐이라 컴파일과 동작에는 문제가 없다. 다만 향후 타입이 추가되면 이 switch는 아무 동작도 하지 않은 채 44행에서event_handled에는 정상 기록되어, 해당 이벤트는 영구히 재처리되지 않고 집계 누락만 남는다.운영 관점: 신규 이벤트 타입 추가 시 집계 로직 추가를 누락해도 장애 신호(예외/로그) 없이 조용히 데이터가 누락된다. default 분기에서 명시적으로 예외를 던지거나 경고 로그를 남기는 것을 제안한다. 추가로 신규 타입 처리 시 이 방어 로직이 실제로 걸리는지 검증하는 단위 테스트를 함께 추가할 것을 제안한다.
🛡️ 제안 diff
switch (message.type()) { case PRODUCT_SOLD -> metrics.increaseSales(message.quantity()); + default -> throw new IllegalStateException("지원하지 않는 OrderEventType: " + message.type()); }🤖 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/ProductSalesApplicationService.java` around lines 39 - 41, In ProductSalesApplicationService, the switch on message.type() only handles PRODUCT_SOLD and has no default branch, so new OrderEventType values could be silently ignored while still being marked handled. Update the switch to include an explicit default in this method so unexpected types fail fast with an exception or at least emit a clear warning log, and make sure the behavior is tied to the message.type() handling in ProductSalesApplicationService. Add a unit test that covers an unrecognized OrderEventType and verifies the new default path is triggered.apps/commerce-streamer/src/main/java/com/loopers/application/coupon/CouponIssueProcessor.java (1)
40-70: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win상태 문자열 리터럴("PENDING","ALREADY_ISSUED","FAILED","SOLD_OUT","SUCCESS")이 코드 곳곳에 하드코딩돼 있다.
commerce-api의
CouponIssueStatusenum과 모듈이 분리돼 있어 직접 공유는 어렵지만, 동일 리터럴이 이 파일과 테스트 파일에 반복돼 있어 오타 시 컴파일 타임에 잡히지 않는다. 예컨대"SOLD_OUT"을"SOLDOUT"으로 잘못 쓰면 상태 조회 API가 이를 매핑하지 못해 폴링 응답이 깨질 수 있는데, 이는 런타임에만 드러난다. streamer 모듈 내 상수(enum 또는static final)로 추출해 재사용하는 편을 권장한다.🤖 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/coupon/CouponIssueProcessor.java` around lines 40 - 70, The status values in CouponIssueProcessor are hardcoded string literals, so replace the repeated values with a single shared source of truth inside the streamer module. Introduce a local enum or static final constants and use them in the request status check, existsUserCoupon/tryConsumeSlot flow, and every updateStatus call so the identifiers stay consistent. Update any related tests to reference the same symbols instead of repeating the raw strings, using CouponIssueProcessor and its status handling as the main place to locate the change.apps/commerce-streamer/src/test/java/com/loopers/application/coupon/CouponIssueProcessorIntegrationTest.java (1)
115-127: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winunique 제약 경합(최후 방어선) 시나리오에 대한 테스트 케이스가 없다.
현재
alreadyIssued_whenAlreadyHasCoupon은 요청 상태(existsUserCoupon)를 통한 정상 경로만 검증한다.CouponIssueProcessor의 (5) 발급 단계에서insertUserCoupon이 unique 제약 위반으로 실패하는 경합 상황(파티션 직렬화가 깨진 극단 케이스)에 대한 테스트가 빠져 있다.user_coupons에 선행 삽입 후tryConsumeSlot이 성공한 상태에서insertUserCoupon을 호출해 이 경로를 재현하는 테스트를 추가하면, 위에서 지적한 슬롯 누수/예외 처리 이슈를 회귀 방지할 수 있다.🤖 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/coupon/CouponIssueProcessorIntegrationTest.java` around lines 115 - 127, The current test only covers the normal already-issued path via existsUserCoupon, but it does not exercise the final defense path where CouponIssueProcessor.handle reaches insertUserCoupon and hits a unique-constraint conflict after tryConsumeSlot succeeds. Add a regression test in CouponIssueProcessorIntegrationTest that pre-seeds a user_coupon row, makes the slot consumption succeed, then forces insertUserCoupon to fail with the unique violation to verify the processor returns ALREADY_ISSUED and does not consume an extra slot. Use the existing handle, seedRequest, statusOf, and issuedCountOf helpers to locate and assert this collision scenario.
🤖 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/CouponApplicationService.java`:
- Around line 96-102: `CouponApplicationService.serialize()`에서
`JsonProcessingException`을 문자열만 담아 다시 던지는 대신 원본 cause를 보존하도록 수정하세요.
`CoreException`에 `Throwable cause`를 받는 생성자를 추가하고, `serialize()`의 catch 블록에서 해당
생성자로 `JsonProcessingException`을 전달하도록 `objectMapper.writeValueAsString` 실패 처리를
바꾸세요. 관련 식별자는 `serialize`, `CoreException`, `ErrorType.INTERNAL_ERROR`입니다. 또한
직렬화 실패 테스트를 보강해 던져진 `CoreException`의 `getCause()`가 `JsonProcessingException`인지
검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeCountEventListener.java`:
- Around line 36-52: `LikeCountEventListener`의 `void `@Async`` 이벤트 핸들러인 `onLiked`와
`onUnliked`에서 발생하는 예외가 호출자에게 전파되지 않으므로, `likeCountExecutor`에
`AsyncUncaughtExceptionHandler`를 연결해 실패를 집계·알림하도록 보강하세요. 예외 발생 시 핸들러가 실제로 호출되는지
검증하는 테스트를 추가하고, 필요하면 `likeCountExecutor` 설정과 예외 처리 흐름이 함께 동작하도록 확인하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueStatus.java`:
- Line 4: The Javadoc in CouponIssueStatus is describing the wrong consumer for
status transitions; update the documentation to match the actual architecture by
replacing the “발급 소비자(api 호스팅)” wording with the service/component that באמת
handles processing, namely commerce-streamer’s CouponIssueProcessor. Keep the
comment focused on who performs the PENDING-to-final-state transition so readers
can identify the correct ownership during troubleshooting.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventListener.java`:
- Around line 78-84: In OutboxEventListener.serialize(), the
JsonProcessingException handling should preserve the original cause and stop
exposing Jackson details in the client-facing message. Update CoreException to
support a cause constructor, then throw it here with a fixed user-safe message
like “요청 처리 중 오류가 발생했습니다.” while passing the caught exception as the cause. Make
sure ApiControllerAdvice still returns only the safe customMessage, and add a
test around serialize() failure to verify the response excludes the Jackson
error text while the underlying exception remains attached in logs/stack trace.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxKafkaConfig.java`:
- Around line 24-29: In OutboxKafkaConfig, update outboxKafkaTemplate to stop
using the no-arg KafkaProperties.buildProducerProperties() call and switch to
the Spring Boot 3.4.4-compatible buildProducerProperties(SslBundles) overload by
injecting SslBundles into the bean method; pass null when SSL is not configured.
Keep the existing ProducerConfig serializer setup and KafkaTemplate creation
unchanged, then add coverage in commerce-api for context loading and successful
KafkaTemplate<String, String> bean creation.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelay.java`:
- Around line 35-53: The `OutboxRelay.relay()` method holds a single
`@Transactional` scope while doing blocking
`outboxKafkaTemplate.send(...).get(...)` calls, which can keep a DB connection
busy for too long. Move Kafka publishing out of the transaction boundary, then
persist `markPublished()` in a separate short transaction only for successfully
sent events, using `OutboxRelay`, `outboxKafkaTemplate`, and
`outboxEventJpaRepository` as the key touchpoints. If needed, split the work
into separate methods or a separate bean to avoid self-invocation/proxy issues,
and keep retry/error handling around the send path rather than inside the DB
transaction.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/coupon/CouponIssueProcessor.java`:
- Around line 17-19: `CouponIssueProcessor.handle` currently lets the
`insertUserCoupon` unique-key failure escape as a generic database error, which
turns an expected duplicate issue into retry/DLT flow. Wrap the issuance step in
explicit handling for `DataIntegrityViolationException`, and on that path
transition directly to `ALREADY_ISSUED` instead of treating it as failure.
Because the slot increment happens earlier in the same transaction, add/use a
`CouponIssueJdbcRepository.releaseSlot` compensation and invoke it when the
unique constraint is hit so the `tryConsumeSlot` count is not wasted; also add a
test covering the duplicate `user_coupons` case through `handle()`.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.java`:
- Around line 72-87: `dlqErrorHandler()` currently builds a
`DefaultErrorHandler` without committing recovered records, so poison messages
sent to the DLT can be consumed again and duplicated. Update the
`dlqErrorHandler(KafkaTemplate<Object, Object>)` setup in
`KafkaErrorHandlingConfig` to enable recovered-offset commits by calling
`setCommitRecovered(true)` on the `handler`, so the offset advances after DLT
publication and the consumer moves on instead of redelivering the same record.
In `@docs/week2/04-erd.md`:
- Around line 140-145: The `outbox_events.topic` contract in the ERD is missing
the coupon publish path, so update the `outbox_events` schema description to
include `coupon-issue-requests` or generalize the allowed values to the full
Round 7 outbox topic set. Use the `outbox_events` table definition in the ERD
markdown to make the contract match the existing coupon issuance flow, and then
regenerate the ERD/sequence snapshots to confirm the coupon path is reflected.
- Line 414: 블록쿼트 내부의 빈 줄로 인해 markdownlint MD028 경고가 발생하고 있으니, 해당 인용문을 처리하는 부분에서
빈 줄을 제거하거나 블록쿼트를 하나로 붙이도록 수정하세요. `docs/week2/04-erd.md`의 `<details>` 안에 있는 인용/예시
텍스트를 찾아 빈 줄 없이 연속된 블록쿼트 형태로 정리하면 됩니다. 수정 후 `markdownlint docs/week2/04-erd.md`를
다시 실행해 경고가 사라졌는지 확인하세요.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/product/ProductApplicationService.java`:
- Around line 60-63: `ProductViewedEvent` 발행 로직이 캐시 히트 경로에만 직접 들어가 있어 히트/미스 간
동작이 흩어져 있습니다. `ProductApplicationService`의 조회 흐름에서 공통 헬퍼(예: 조회 이벤트 발행 메서드)로
`eventPublisher.publishEvent(new ProductViewedEvent(...))`를 추출하고, 캐시 히트와 미스 모두 그
헬퍼를 통해 동일하게 1회씩만 발행되도록 정리하세요. `getProductById`(또는 해당 조회 메서드)와
`ProductViewedEvent`를 기준으로 위치를 찾고, 캐시 히트/미스 각각에서 이벤트가 정확히 한 번 발생하는 테스트를 추가해
검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentCompletedEvent.java`:
- Around line 11-12: `PaymentCompletedEvent` currently exposes `amount` as a raw
`Long`, which breaks the `Money` value-object boundary used elsewhere. Update
the event construction path so the domain still works with `Money` and only
converts to `long` at the event factory/creation point, adding a dedicated
factory or mapper on `PaymentCompletedEvent` or the caller that uses
`Money.of(...)`. Also add/adjust tests around the `PaymentCompletedEvent`
creation flow to cover boundary values like zero and negative amounts.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEvent.java`:
- Around line 41-43: The Outbox polling query in OutboxRelay.relay() uses
findTop100ByStatusOrderByIdAsc(PENDING), but OutboxEvent does not declare an
index for the status/id access pattern. Update the OutboxEvent entity to add a
composite index on status and id via `@Table`(indexes = `@Index`(columnList =
"status, id")) (or equivalent migration) so the polling query can avoid repeated
full scans as the table grows. If there is already a schema migration flow, keep
the entity and migration index definitions aligned.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.java`:
- Around line 9-10: The OutboxRelay query in OutboxEventJpaRepository relies on
filtering by status and ordering by id, so verify that the outbox table has an
index matching the query shape. Add or confirm a composite (status, id) index on
the OutboxEvent entity or via migration, since a status-only index will still
leave sorting costly as PUBLISHED rows accumulate. If needed, review related
repository/query definitions and add a performance test that exercises the
findTop100ByStatusOrderByIdAsc path with large outbox volume.
In
`@apps/commerce-api/src/test/java/com/loopers/application/event/EventPhaseIntegrationTest.java`:
- Around line 88-98: The awaitLikeCount polling logic is duplicated across
multiple integration tests and should be centralized. Extract the repeated
`Thread.sleep(100)`/100-iteration wait loop from `EventPhaseIntegrationTest` and
the matching helper in `LikeConcurrencyIntegrationTest`,
`ProductModifyLikeConcurrencyIntegrationTest`, and
`ProductApplicationServiceIntegrationTest` into a shared test utility or an
`IntegrationTestSupport` helper, or replace it with `Awaitility` if already
available. Keep the shared helper responsible for waiting on
`productJpaRepository.findById(productId).orElseThrow().getLikeCount()` until
the expected value is reached so timeout/interval changes only need to be made
in one place.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/coupon/CouponIssueProcessor.java`:
- Around line 40-70: The status values in CouponIssueProcessor are hardcoded
string literals, so replace the repeated values with a single shared source of
truth inside the streamer module. Introduce a local enum or static final
constants and use them in the request status check,
existsUserCoupon/tryConsumeSlot flow, and every updateStatus call so the
identifiers stay consistent. Update any related tests to reference the same
symbols instead of repeating the raw strings, using CouponIssueProcessor and its
status handling as the main place to locate the change.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductMetricsApplicationService.java`:
- Around line 40-44: The switch statement in ProductMetricsApplicationService
does not guard against unmapped CatalogEventType values, so unknown future event
types can be silently ignored. Update the switch on message.type() to include a
default branch that either throws an exception or logs a warning before
returning, and make sure the handling remains explicit in this method. Add a
unit test around ProductMetricsApplicationService to verify the default path
behavior when an unexpected CatalogEventType is received.
In
`@apps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductSalesApplicationService.java`:
- Around line 39-41: In ProductSalesApplicationService, the switch on
message.type() only handles PRODUCT_SOLD and has no default branch, so new
OrderEventType values could be silently ignored while still being marked
handled. Update the switch to include an explicit default in this method so
unexpected types fail fast with an exception or at least emit a clear warning
log, and make sure the behavior is tied to the message.type() handling in
ProductSalesApplicationService. Add a unit test that covers an unrecognized
OrderEventType and verifies the new default path is triggered.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java`:
- Around line 74-76: `ProductMetrics.increaseSales` has no validation for
invalid quantities, so zero or negative values can corrupt `salesCount`. Update
`increaseSales(int quantity)` to validate that `quantity` is at least 1 and
throw an exception when it is not, keeping the behavior consistent with the
defensive checks used elsewhere in `ProductMetrics` such as `decreaseLike`. Also
add unit tests in `ProductMetricsTest` covering `increaseSales(0)` and
`increaseSales(-1)` to verify the exception behavior.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.java`:
- Around line 95-148: The consumer configuration setup in recordListenerFactory
and couponRecordListenerFactory is duplicated, which makes the two Kafka
listener factories drift over time. Extract the shared
KafkaProperties-to-consumerConfig mapping into a single helper such as
buildConsumerConfig(KafkaProperties), move the repeated MAX_POLL_RECORDS_CONFIG
through MAX_POLL_INTERVAL_MS_CONFIG assignments there, and have both `@Bean`
methods call that helper before creating their DefaultKafkaConsumerFactory.
In
`@apps/commerce-streamer/src/test/java/com/loopers/application/coupon/CouponIssueProcessorIntegrationTest.java`:
- Around line 115-127: The current test only covers the normal already-issued
path via existsUserCoupon, but it does not exercise the final defense path where
CouponIssueProcessor.handle reaches insertUserCoupon and hits a
unique-constraint conflict after tryConsumeSlot succeeds. Add a regression test
in CouponIssueProcessorIntegrationTest that pre-seeds a user_coupon row, makes
the slot consumption succeed, then forces insertUserCoupon to fail with the
unique violation to verify the processor returns ALREADY_ISSUED and does not
consume an extra slot. Use the existing handle, seedRequest, statusOf, and
issuedCountOf helpers to locate and assert this collision scenario.
In
`@apps/commerce-streamer/src/test/java/com/loopers/domain/metrics/ProductMetricsTest.java`:
- Around line 26-32: The current ProductMetricsTest only verifies the 1-to-0
case, so the 0-floor guard in ProductMetrics.decreaseLike() is not exercised.
Update decreaseLikeFloorsAtZero to also call decreaseLike() again after
likeCount reaches zero and assert it stays zero, using ProductMetrics.of() and
decreaseLike() as the key symbols to locate the test.
In
`@apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CatalogEventCollectorIntegrationTest.java`:
- Around line 150-193: `newProducer()`, `awaitLikeCount()`, and
`awaitViewCount()` are duplicated across multiple integration tests; extract the
shared Kafka producer setup and polling helpers into a common support class or
utility such as `KafkaIntegrationTestSupport`. Move the reusable logic there,
then update `CatalogEventCollectorIntegrationTest` and the other affected tests
to call the shared methods so future timeout or producer config changes only
need to be made in one place.
🪄 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: 87ce6720-fbec-4829-953a-6a39b5c93f99
📒 Files selected for processing (78)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponApplicationService.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponInfo.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueMessage.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeApplicationService.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeCountEventListener.javaapps/commerce-api/src/main/java/com/loopers/application/log/UserActionLogListener.javaapps/commerce-api/src/main/java/com/loopers/application/notification/NotificationEventListener.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderApplicationService.javaapps/commerce-api/src/main/java/com/loopers/application/payment/PaymentResultApplier.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductApplicationService.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/CouponTemplate.javaapps/commerce-api/src/main/java/com/loopers/domain/like/event/ProductLikedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/like/event/ProductUnlikedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/order/event/OrderPlacedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/payment/event/PaymentCompletedEvent.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/outbox/CatalogEventMessage.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/CatalogEventType.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OrderEventMessage.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OrderEventType.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEvent.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxEventListener.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxKafkaConfig.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelay.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxStatus.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/ProductViewedEventPublisher.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/config/AsyncConfig.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/config/KafkaTopicConfig.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1ApiSpec.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1Dto.javaapps/commerce-api/src/test/java/com/loopers/application/event/EventPhaseIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeConcurrencyIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductApplicationServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductModifyLikeConcurrencyIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponIssueRequestTest.javaapps/commerce-api/src/test/java/com/loopers/domain/coupon/CouponTemplateTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/OutboxRelayIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/outbox/PaymentSalesOutboxIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/infrastructure/payment/PgPaymentGatewayResilienceIntegrationTest.javaapps/commerce-streamer/src/main/java/com/loopers/application/coupon/CouponIssueProcessor.javaapps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductMetricsApplicationService.javaapps/commerce-streamer/src/main/java/com/loopers/application/metrics/ProductSalesApplicationService.javaapps/commerce-streamer/src/main/java/com/loopers/domain/eventlog/EventHandled.javaapps/commerce-streamer/src/main/java/com/loopers/domain/eventlog/EventHandledId.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/CouponIssueJdbcRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/eventlog/EventHandledJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventMessage.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventType.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueMessage.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaTopicConfig.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventMessage.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderEventType.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/OrderSalesConsumer.javaapps/commerce-streamer/src/test/java/com/loopers/application/coupon/CouponIssueProcessorIntegrationTest.javaapps/commerce-streamer/src/test/java/com/loopers/domain/metrics/ProductMetricsTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CatalogEventCollectorIntegrationTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CatalogEventDlqIntegrationTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/OrderSalesCollectorIntegrationTest.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/ProductMetricsConcurrentCreateIntegrationTest.javadocs/week2/01-requirements.mddocs/week2/02-sequence-diagrams.mddocs/week2/03-class-diagram.mddocs/week2/04-erd.mdmodules/kafka/src/main/resources/kafka.ymlmodules/kafka/src/testFixtures/java/com/loopers/testcontainers/KafkaTestContainersConfig.java
Comment on lines
+96
to
+102
| private String serialize(CouponIssueMessage message) { | ||
| try { | ||
| return objectMapper.writeValueAsString(message); | ||
| } catch (JsonProcessingException e) { | ||
| throw new CoreException(ErrorType.INTERNAL_ERROR, "outbox payload 직렬화에 실패했습니다: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -a 'CoreException.java' --exec cat -n {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 833
직렬화 예외에 원본 cause를 보존해야 한다
CouponApplicationService.serialize() 에서 JsonProcessingException 을 문자열만 담아 다시 던지면 outbox payload 직렬화 실패의 원인을 운영 로그에서 추적하기 어렵다. CoreException 에 Throwable cause 를 받는 생성자를 추가해 원본 예외를 전달하고, 직렬화 실패 단위 테스트에서 getCause() 가 JsonProcessingException 인지 확인하라.
🤖 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/CouponApplicationService.java`
around lines 96 - 102, `CouponApplicationService.serialize()`에서
`JsonProcessingException`을 문자열만 담아 다시 던지는 대신 원본 cause를 보존하도록 수정하세요.
`CoreException`에 `Throwable cause`를 받는 생성자를 추가하고, `serialize()`의 catch 블록에서 해당
생성자로 `JsonProcessingException`을 전달하도록 `objectMapper.writeValueAsString` 실패 처리를
바꾸세요. 관련 식별자는 `serialize`, `CoreException`, `ErrorType.INTERNAL_ERROR`입니다. 또한
직렬화 실패 테스트를 보강해 던져진 `CoreException`의 `getCause()`가 `JsonProcessingException`인지
검증하세요.
Source: Path instructions
Comment on lines
+36
to
+52
| @Async("likeCountExecutor") | ||
| @Transactional(propagation = Propagation.REQUIRES_NEW) | ||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void onLiked(ProductLikedEvent event) { | ||
| if (productRepository.increaseLikeCount(event.productId()) == 0) { | ||
| log.warn("좋아요 집계 반영 실패(대상 상품 없음/삭제) — likeCount 드리프트 가능. productId={}", event.productId()); | ||
| } | ||
| } | ||
|
|
||
| @Async("likeCountExecutor") | ||
| @Transactional(propagation = Propagation.REQUIRES_NEW) | ||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| public void onUnliked(ProductUnlikedEvent event) { | ||
| if (productRepository.decreaseLikeCount(event.productId()) == 0) { | ||
| log.warn("좋아요 취소 집계 반영 실패(대상 상품 없음/삭제) — likeCount 드리프트 가능. productId={}", event.productId()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd AsyncConfig.java --exec cat -n {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2618
🏁 Script executed:
#!/bin/bash
rg -n "AsyncUncaughtExceptionHandler|AsyncConfigurer|implements AsyncConfigurer|setAsyncUncaughtExceptionHandler|`@EnableAsync`" apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 275
likeCountExecutor의 예외 처리 경로를 보강해야 한다
likeCountExecutor는 바운드 큐와 CallerRunsPolicy가 있어 적재 폭주/OOM 우려는 낮다. 다만 void @Async`` 리스너는 예외가 호출자에게 전파되지 않아 likeCount 갱신 실패가 운영에서 묻힐 수 있다. AsyncUncaughtExceptionHandler로 실패를 집계·알림 처리하고, 예외 발생 시 해당 핸들러가 호출되는 테스트를 추가하라.
🤖 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/LikeCountEventListener.java`
around lines 36 - 52, `LikeCountEventListener`의 `void `@Async`` 이벤트 핸들러인
`onLiked`와 `onUnliked`에서 발생하는 예외가 호출자에게 전파되지 않으므로, `likeCountExecutor`에
`AsyncUncaughtExceptionHandler`를 연결해 실패를 집계·알림하도록 보강하세요. 예외 발생 시 핸들러가 실제로 호출되는지
검증하는 테스트를 추가하고, 필요하면 `likeCountExecutor` 설정과 예외 처리 흐름이 함께 동작하도록 확인하세요.
| package com.loopers.domain.coupon; | ||
|
|
||
| /** | ||
| * 선착순 쿠폰 발급 요청의 상태. 접수 시 PENDING 으로 저장되고, 발급 소비자(api 호스팅)가 처리 결과로 전이시킨다. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Javadoc의 소비자 설명이 실제 아키텍처와 불일치한다.
발급 소비자(api 호스팅)로 서술되어 있으나, PR 설계상 실제 상태 전이는 별도 서비스인 commerce-streamer의 CouponIssueProcessor가 담당한다. 운영 중 장애 대응 시 담당 서비스를 잘못 지목해 트러블슈팅이 지연될 수 있으므로 문서를 정정해야 한다.
📝 제안
- * 선착순 쿠폰 발급 요청의 상태. 접수 시 PENDING 으로 저장되고, 발급 소비자(api 호스팅)가 처리 결과로 전이시킨다.
+ * 선착순 쿠폰 발급 요청의 상태. 접수 시 PENDING 으로 저장되고, 발급 소비자(commerce-streamer)가 처리 결과로 전이시킨다.📝 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
| * 선착순 쿠폰 발급 요청의 상태. 접수 시 PENDING 으로 저장되고, 발급 소비자(api 호스팅)가 처리 결과로 전이시킨다. | |
| * 선착순 쿠폰 발급 요청의 상태. 접수 시 PENDING 으로 저장되고, 발급 소비자(commerce-streamer)가 처리 결과로 전이시킨다. |
🤖 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/CouponIssueStatus.java`
at line 4, The Javadoc in CouponIssueStatus is describing the wrong consumer for
status transitions; update the documentation to match the actual architecture by
replacing the “발급 소비자(api 호스팅)” wording with the service/component that באמת
handles processing, namely commerce-streamer’s CouponIssueProcessor. Keep the
comment focused on who performs the PENDING-to-final-state transition so readers
can identify the correct ownership during troubleshooting.
Comment on lines
+78
to
+84
| private String serialize(Object message) { | ||
| try { | ||
| return objectMapper.writeValueAsString(message); | ||
| } catch (JsonProcessingException e) { | ||
| throw new CoreException(ErrorType.INTERNAL_ERROR, "outbox payload 직렬화에 실패했습니다: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# CoreException 생성자가 cause(Throwable)를 지원하는지 확인
fd -t f 'CoreException.java' --exec cat -n {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 833
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# CoreException과 관련 예외 처리 경로를 함께 확인한다.
fd -t f 'OutboxEventListener.java' 'CoreException.java' 'ErrorType.java' '.*ControllerAdvice.*' '.*ExceptionHandler.*' apps/commerce-api/src/main/java com -x sh -c 'echo "===== $1 ====="; cat -n "$1"; echo' sh {}
# serialize() 주변도 함께 본다.
file=$(fd -t f 'OutboxEventListener.java' apps/commerce-api/src/main/java | head -n 1)
if [ -n "${file:-}" ]; then
echo "===== OutboxEventListener excerpt ====="
sed -n '1,140p' "$file" | cat -n
fi
# CoreException 생성자 사용처와 메시지 노출 가능성을 확인한다.
rg -n "new CoreException\(|customMessage|errorType.getMessage\(|`@ControllerAdvice`|ExceptionHandler|ResponseEntityExceptionHandler" apps/commerce-api/src/main/javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 37055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# API 예외 처리에서 CoreException 메시지가 어떻게 노출되는지 확인한다.
advice_file=$(fd -t f 'ApiControllerAdvice.java' apps/commerce-api/src/main/java | head -n 1)
if [ -n "${advice_file:-}" ]; then
echo "===== ApiControllerAdvice ====="
sed -n '1,220p' "$advice_file" | cat -n
fi
# CoreException의 getter/상속 구조를 확인한다.
core_file=$(fd -t f 'CoreException.java' apps/commerce-api/src/main/java | head -n 1)
if [ -n "${core_file:-}" ]; then
echo "===== CoreException ====="
sed -n '1,120p' "$core_file" | cat -n
fi
# outbox 직렬화 실패와 관련된 로그/예외 처리 패턴을 넓게 본다.
rg -n "outbox payload 직렬화|log\.(error|warn|info)|new CoreException\(ErrorType\.INTERNAL_ERROR|getMessage\(\)" apps/commerce-api/src/main/java/com/loopersRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 14610
직렬화 실패 예외에 cause를 연결하고 응답 메시지를 분리해야 한다
CoreException은 cause를 받지 못해JsonProcessingException의 스택이 끊기고,ApiControllerAdvice가customMessage를 그대로 응답하므로e.getMessage()를 넣으면 내부 직렬화 정보가 클라이언트로 노출된다.CoreException에 cause 생성자를 추가한 뒤, 이 경로는"요청 처리 중 오류가 발생했습니다."같은 고정 메시지로 던지고 상세 원인은 스택트레이스에만 남겨라.serialize()실패 시 API 응답에 Jackson 메시지가 포함되지 않고, 로그에는 원인 예외가 남는 테스트를 추가해야 한다.
🤖 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/OutboxEventListener.java`
around lines 78 - 84, In OutboxEventListener.serialize(), the
JsonProcessingException handling should preserve the original cause and stop
exposing Jackson details in the client-facing message. Update CoreException to
support a cause constructor, then throw it here with a fixed user-safe message
like “요청 처리 중 오류가 발생했습니다.” while passing the caught exception as the cause. Make
sure ApiControllerAdvice still returns only the safe customMessage, and add a
test around serialize() failure to verify the response excludes the Jackson
error text while the underlying exception remains attached in logs/stack trace.
Source: Path instructions
Comment on lines
+24
to
+29
| public KafkaTemplate<String, String> outboxKafkaTemplate(KafkaProperties kafkaProperties) { | ||
| Map<String, Object> config = new HashMap<>(kafkaProperties.buildProducerProperties()); | ||
| config.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); | ||
| config.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, StringSerializer.class); | ||
| return new KafkaTemplate<>(new DefaultKafkaProducerFactory<>(config)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 프로젝트에 선언된 Spring Boot 버전 확인
rg -n "springBootVersion|org.springframework.boot" --type=kotlin -g '*.gradle.kts' -g 'gradle.properties'
fd 'gradle.properties' -x cat {}Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Gradle/Spring Boot 버전 선언 위치와 outbox 설정 파일 확인
git ls-files '*gradle.kts' 'gradle.properties' 'apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxKafkaConfig.java' | sed 's#^`#FILE` #'
echo '--- gradle version refs ---'
rg -n "springBootVersion|org\.springframework\.boot|spring-boot" \
--glob '*.gradle.kts' --glob 'gradle.properties' --glob 'settings.gradle.kts' \
.
echo '--- outbox config ---'
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxKafkaConfig.java
echo '--- SslBundles / buildProducerProperties usage ---'
rg -n "buildProducerProperties\\(|SslBundles" apps/commerce-api src .Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5655
KafkaProperties.buildProducerProperties() 호출을 SslBundles 기반으로 교체해야 한다.
gradle.properties의 springBootVersion=3.4.4 기준으로 무인자 오버로드가 없어 OutboxKafkaConfig의 빈 생성이 빌드 단계에서 깨진다. SslBundles를 주입해 buildProducerProperties(sslBundles)로 바꾸고, SSL 설정이 없으면 null을 전달하라. 변경 후에는 commerce-api 컨텍스트 로딩 테스트와 KafkaTemplate<String, String> 빈 생성 테스트를 추가하라.
🤖 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/OutboxKafkaConfig.java`
around lines 24 - 29, In OutboxKafkaConfig, update outboxKafkaTemplate to stop
using the no-arg KafkaProperties.buildProducerProperties() call and switch to
the Spring Boot 3.4.4-compatible buildProducerProperties(SslBundles) overload by
injecting SslBundles into the bean method; pass null when SSL is not configured.
Keep the existing ProducerConfig serializer setup and KafkaTemplate creation
unchanged, then add coverage in commerce-api for context loading and successful
KafkaTemplate<String, String> bean creation.
Comment on lines
+35
to
+53
| @Scheduled(fixedDelay = 1000) | ||
| @Transactional | ||
| public void relay() { | ||
| List<OutboxEvent> pending = outboxEventJpaRepository.findTop100ByStatusOrderByIdAsc(OutboxStatus.PENDING); | ||
| for (OutboxEvent event : pending) { | ||
| try { | ||
| outboxKafkaTemplate.send(event.getTopic(), event.getAggregateId(), event.getPayload()) | ||
| .get(PUBLISH_TIMEOUT_SECONDS, TimeUnit.SECONDS); | ||
| event.markPublished(); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| log.warn("outbox relay 중단됨. eventId={}", event.getEventId()); | ||
| return; | ||
| } catch (Exception e) { | ||
| log.warn("outbox relay 발행 실패 — 다음 주기 재시도. eventId={}, topic={}", | ||
| event.getEventId(), event.getTopic(), e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
@Transactional 범위 안에서 순차 블로킹 Kafka 전송을 수행해 DB 커넥션 점유 시간이 과도하게 길어질 수 있다.
relay()는 하나의 트랜잭션 안에서 최대 100건에 대해 outboxKafkaTemplate.send(...).get(5, TimeUnit.SECONDS)를 순차 호출한다. 브로커 지연·타임아웃이 반복되면 최악의 경우 한 번의 스케줄 실행에서 최대 500초 동안 DB 커넥션 1개를 점유하게 된다. fixedDelay 특성상 다음 실행은 겹치지 않지만, 이 시간 동안 해당 커넥션은 커넥션 풀에서 회수되지 않아 다른 요청/배치가 사용할 수 있는 자원을 줄이고, 장애 상황에서 회복을 더디게 만든다. Kafka 전송(외부 I/O)과 DB 상태 갱신(로컬 트랜잭션)의 경계를 분리해, 전송은 트랜잭션 밖에서 수행하고 성공한 건만 별도의 짧은 트랜잭션으로 markPublished를 커밋하는 구조를 권장한다. 부하/장애 재현 통합 테스트(브로커 응답 지연 시 커넥션 점유 시간 측정)도 함께 고려할 필요가 있다.
♻️ 제안 (개념적 방향, self-invocation 프록시 문제는 별도 빈 분리로 회피 필요)
- `@Scheduled`(fixedDelay = 1000)
- `@Transactional`
- public void relay() {
- List<OutboxEvent> pending = outboxEventJpaRepository.findTop100ByStatusOrderByIdAsc(OutboxStatus.PENDING);
- for (OutboxEvent event : pending) {
- try {
- outboxKafkaTemplate.send(event.getTopic(), event.getAggregateId(), event.getPayload())
- .get(PUBLISH_TIMEOUT_SECONDS, TimeUnit.SECONDS);
- event.markPublished();
- } catch (...) { ... }
- }
- }
+ `@Scheduled`(fixedDelay = 1000)
+ public void relay() {
+ List<OutboxEvent> pending = outboxEventJpaRepository.findTop100ByStatusOrderByIdAsc(OutboxStatus.PENDING);
+ for (OutboxEvent event : pending) {
+ try {
+ outboxKafkaTemplate.send(event.getTopic(), event.getAggregateId(), event.getPayload())
+ .get(PUBLISH_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+ outboxPublishWriter.markPublished(event.getId()); // 짧은 트랜잭션 분리, 별도 빈에서 수행
+ } catch (...) { ... }
+ }
+ }🤖 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/OutboxRelay.java`
around lines 35 - 53, The `OutboxRelay.relay()` method holds a single
`@Transactional` scope while doing blocking
`outboxKafkaTemplate.send(...).get(...)` calls, which can keep a DB connection
busy for too long. Move Kafka publishing out of the transaction boundary, then
persist `markPublished()` in a separate short transaction only for successfully
sent events, using `OutboxRelay`, `outboxKafkaTemplate`, and
`outboxEventJpaRepository` as the key touchpoints. If needed, split the work
into separate methods or a separate bean to avoid self-invocation/proxy issues,
and keep retry/error handling around the send path rather than inside the DB
transaction.
Comment on lines
+17
to
+19
| * <p><b>동시성</b>: key=templateId 파티션 직렬화(한 템플릿=한 파티션=단일 스레드)를 전제로 하되, 슬롯 차감은 | ||
| * 조건부 원자 UPDATE({@link CouponIssueJdbcRepository#tryConsumeSlot})라 파티션 가정이 깨져도 초과 발급이 없다. | ||
| * 멱등 판정·중복·차감·발급·상태 전이를 <b>한 트랜잭션</b>에 묶어 부분 반영을 막는다.</p> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
insertUserCoupon의 unique 제약 위반(최후 방어선) 발동 시 예외를 명시적으로 처리하지 않는다.
주석(라인 17-19)에서 "파티션 가정이 깨져도" user_coupons unique 제약이 최후 방어선이라고 명시했지만, 실제 (5) 발급 단계(라인 68-71)에서는 insertUserCoupon 호출을 감싸는 예외 처리가 없다. 이 경우 발생하는 시나리오는 다음과 같다:
- unique 위반 →
DataIntegrityViolationException전파 → 트랜잭션 전체 롤백(직전tryConsumeSlot의issued_count증가도 함께 롤백) → ack 미호출 → Kafka 재전달. - 재전달 시점에는 (2)의
existsUserCoupon이 참이 되어ALREADY_ISSUED로 정상 귀결되긴 하지만, 이는 "우연히 재시도가 이 케이스를 해결해주는" 구조에 불과하다.
운영 관점 문제: 이 경합은 설계상 이미 예견된(즉, 드물지만 정상적인) 상황인데도 일반 장애와 동일한 재시도→DLT 파이프라인을 타게 된다. 재시도 횟수 설정이 낮으면 정상적인 중복 요청이 DLT로 격리되어 수동 개입이 필요해질 수 있다. DataIntegrityViolationException을 명시적으로 catch해 즉시 ALREADY_ISSUED로 전이하는 편이 결정적이고 운영 부담이 적다.
추가 테스트: user_coupons에 동일 (user_id, template_id) 행을 미리 삽입한 뒤 handle()을 호출해 unique 위반 경로에서 슬롯이 낭비되지 않고 ALREADY_ISSUED로 확정되는지 검증하는 케이스가 필요하다.
🔧 제안 수정안
+import org.springframework.dao.DataIntegrityViolationException;
+
// (5) 발급 — 슬롯을 확보했으니 쿠폰을 발급하고 요청을 SUCCESS 로 전이.
- couponIssueJdbcRepository.insertUserCoupon(userId, template, now);
- couponIssueJdbcRepository.updateStatus(message.requestId(), "SUCCESS", now);
+ try {
+ couponIssueJdbcRepository.insertUserCoupon(userId, template, now);
+ } catch (DataIntegrityViolationException e) {
+ // unique(user_id, template_id) 위반 = 파티션 직렬화 가정이 깨진 경합의 최후 방어선.
+ // 이미 소비한 슬롯을 되돌려야 실제 발급 가능 수량이 issue_limit 아래로 새지 않는다.
+ couponIssueJdbcRepository.releaseSlot(templateId, now);
+ couponIssueJdbcRepository.updateStatus(message.requestId(), "ALREADY_ISSUED", now);
+ return;
+ }
+ couponIssueJdbcRepository.updateStatus(message.requestId(), "SUCCESS", now);CouponIssueJdbcRepository에 issued_count를 원자적으로 1 감소시키는 releaseSlot 메서드 추가가 함께 필요하다.
Also applies to: 49-52, 68-71
🤖 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/coupon/CouponIssueProcessor.java`
around lines 17 - 19, `CouponIssueProcessor.handle` currently lets the
`insertUserCoupon` unique-key failure escape as a generic database error, which
turns an expected duplicate issue into retry/DLT flow. Wrap the issuance step in
explicit handling for `DataIntegrityViolationException`, and on that path
transition directly to `ALREADY_ISSUED` instead of treating it as failure.
Because the slot increment happens earlier in the same transaction, add/use a
`CouponIssueJdbcRepository.releaseSlot` compensation and invoke it when the
unique constraint is hit so the `tryConsumeSlot` count is not wasted; also add a
test covering the duplicate `user_coupons` case through `handle()`.
Comment on lines
+72
to
+87
| @Bean | ||
| public CommonErrorHandler dlqErrorHandler(KafkaTemplate<Object, Object> deadLetterKafkaTemplate) { | ||
| // 목적지를 소스 토픽 기준으로 분기한다: <원본토픽>.DLT, 파티션 = -1(producer 가 결정). | ||
| // 이 팩토리를 catalog·order collector 가 공유하므로, 소스 토픽에서 DLT 이름을 유도해야 격리가 뒤섞이지 않는다. | ||
| // (기본 리졸버는 소스 파티션 번호를 그대로 지정해, 발행 시점에 producer 메타데이터가 없으면 유실될 수 있다.) | ||
| DeadLetterPublishingRecoverer recoverer = new DeadLetterPublishingRecoverer( | ||
| deadLetterKafkaTemplate, | ||
| (record, exception) -> new TopicPartition(record.topic() + DLT_SUFFIX, -1)); | ||
| DefaultErrorHandler handler = new DefaultErrorHandler(recoverer, new FixedBackOff(1000L, 2L)); | ||
| // 결정적 poison(필수값 누락/타입 위반 등 IllegalArgumentException 계열)은 재시도해도 반드시 실패한다 | ||
| // → 재시도 없이 즉시 DLT 격리해 파이프라인 지연을 줄인다. 일시 장애(DB 순단 등)만 backOff 를 태운다. | ||
| // (역직렬화/변환 실패인 DeserializationException·MessageConversionException 등은 DefaultErrorHandler 가 | ||
| // 이미 기본 not-retryable 로 취급한다 — 여기에 도메인 판정 축을 하나 더 얹는 것.) | ||
| handler.addNotRetryableExceptions(IllegalArgumentException.class); | ||
| return handler; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files
git ls-files 'apps/commerce-streamer/src/main/java/**/KafkaErrorHandlingConfig.java' \
'apps/commerce-streamer/src/main/java/**/Kafka*Config.java' \
'apps/commerce-streamer/src/test/**' | sed -n '1,200p'
echo '--- outline KafkaErrorHandlingConfig ---'
ast-grep outline apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.java --view expanded
echo '--- relevant listener factory references ---'
rg -n "MANUAL_IMMEDIATE|setCommitRecovered|DefaultErrorHandler|DeadLetterPublishingRecoverer|FixedBackOff|CommonErrorHandler|ackMode|AckMode" apps/commerce-streamer/src/main/java apps/commerce-streamer/src/testRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect the config and tests around it.
sed -n '1,240p' apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/KafkaErrorHandlingConfig.java
echo '--- consumer factories ---'
rg -n "couponRecordListenerFactory|recordListenerFactory|AckMode\.MANUAL_IMMEDIATE|AckMode\.RECORD|consumerFactory|ConcurrentKafkaListenerContainerFactory" apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer -A6 -B6
echo '--- tests mentioning DLQ / error handler behavior ---'
rg -n "DLT|DLQ|dead letter|poison|DefaultErrorHandler|commitRecovered|retry" apps/commerce-streamer/src/test -A4 -B4Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 35067
🌐 Web query:
Spring Kafka DefaultErrorHandler setCommitRecovered MANUAL_IMMEDIATE commitRecovered documentation
💡 Result:
In Spring Kafka, the setCommitRecovered(true) method on the DefaultErrorHandler is used to ensure that the offset of a successfully recovered record is committed [1][2][3]. Key points regarding its use: Requirement: The listener container must be configured with ContainerProperties.AckMode.MANUAL_IMMEDIATE [1][3][4]. If this mode is not used, the property may not function as expected [5]. Behavior: When set to true, the error handler will commit the offset of the record after it has been successfully processed by the configured recoverer [1][3][4]. Sync vs Async: Whether the commit operation is synchronous or asynchronous depends on the container's syncCommits property [1][3]. This feature is particularly useful when you want to explicitly advance the offset for messages that could not be processed normally but were successfully handled by a recoverer (e.g., sent to a Dead Letter Topic), effectively marking them as done so they are not re-processed [4][6]. Example usage: defaultErrorHandler.setCommitRecovered(true); Documentation references can be found in the official Spring for Apache Kafka API and reference guides under DefaultErrorHandler [7][1][3].
Citations:
- 1: https://docs.spring.io/spring-kafka/api/org/springframework/kafka/listener/DefaultErrorHandler.html
- 2: https://docs.spring.io/spring-kafka/docs/3.3.13/api/org/springframework/kafka/listener/DefaultErrorHandler.html
- 3: https://docs.spring.io/spring-kafka/docs/current/api/org/springframework/kafka/listener/DefaultErrorHandler.html
- 4: https://docs.spring.io/spring-kafka/reference/4.0-SNAPSHOT/kafka/annotation-error-handling.html
- 5: Custom CommonErrorHandler, ackMode = MANUAL_IMMEDIATE, offset not committed after recovery. Bug / by design? spring-projects/spring-kafka#4182
- 6: ack-mode = MANUAL_IMMEDIATE, enable.auto.commit = false and @RetryableTopic not working as expected spring-projects/spring-kafka#2495
- 7: https://docs.spring.io/spring-kafka/reference/kafka/annotation-error-handling.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- coupon consumer / tests ---'
rg -n "CouponIssue|couponRecordListenerFactory|MANUAL_IMMEDIATE|DLT|dead letter|poison|acknowledge|ackMode" apps/commerce-streamer/src/test apps/commerce-streamer/src/main/java -A4 -B4
echo '--- specific coupon integration test file ---'
sed -n '1,260p' apps/commerce-streamer/src/test/java/com/loopers/application/coupon/CouponIssueProcessorIntegrationTest.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CatalogEventDlqIntegrationTest.java')
text = p.read_text()
for needle in ['setCommitRecovered', 'MANUAL_IMMEDIATE', 'DLT_ORIGINAL', 'poison', 'awaitDltRecordWithEventId']:
print(f'[{needle}]', needle in text)
PYRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 295
dlqErrorHandler()에 setCommitRecovered(true)를 추가해야 한다다
couponRecordListenerFactory()가 AckMode.MANUAL_IMMEDIATE인데 recovered 레코드의 오프셋 커밋이 꺼져 있어서, 재시도 소진 후 DLT로 보낸 poison 메시지가 다시 소비되어 같은 레코드가 계속 재전달되고 DLT가 중복 적재될 수 있다. handler.setCommitRecovered(true)를 넣어 DLT 격리 후 오프셋이 전진하도록 해야 한다. 쿠폰 컨슈머에 대해 poison 메시지 1건을 보낸 뒤 DLT로 이동한 다음 같은 레코드가 다시 소비되지 않고 다음 메시지로 진행하는 통합 테스트를 추가해야 한다다
🤖 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/KafkaErrorHandlingConfig.java`
around lines 72 - 87, `dlqErrorHandler()` currently builds a
`DefaultErrorHandler` without committing recovered records, so poison messages
sent to the DLT can be consumed again and duplicated. Update the
`dlqErrorHandler(KafkaTemplate<Object, Object>)` setup in
`KafkaErrorHandlingConfig` to enable recovered-offset commits by calling
`setCommitRecovered(true)` on the `handler`, so the offset advances after DLT
publication and the consumer moves on instead of redelivering the same record.
Comment on lines
+140
to
+145
| outbox_events { | ||
| bigint id PK | ||
| varchar(36) event_id UK "NOT NULL / 전역 유일 키(소비자 멱등 기준, payload에도 동봉)" | ||
| varchar(100) aggregate_id "NOT NULL / Kafka 파티셔닝 키(= productId 등)" | ||
| varchar(100) topic "NOT NULL / catalog-events | order-events" | ||
| text payload "NOT NULL / 발행 메시지 JSON 원문" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
outbox_events.topic 계약에 쿠폰 토픽을 추가해야 한다.
현재 ERD는 토픽을 catalog-events | order-events로만 적고 있어, 같은 Round 7 흐름의 coupon-issue-requests 발행 경로와 계약이 어긋난다. 문서가 운영자와 스키마 소비자에게 잘못된 허용 토픽 집합을 주면, 쿠폰 outbox가 누락된 것으로 오해해 릴레이/모니터링 설정을 잘못 맞출 수 있다. coupon-issue-requests를 목록에 추가하거나, 아예 “Round 7 outbox 토픽 전체”로 일반화해야 한다.
수정 예시
- varchar(100) topic "NOT NULL / catalog-events | order-events"
+ varchar(100) topic "NOT NULL / catalog-events | order-events | coupon-issue-requests"수정 후 ERD/시퀀스 문서 스냅샷을 다시 생성해 쿠폰 발행 경로가 반영됐는지 확인해야 한다.
📝 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
| outbox_events { | |
| bigint id PK | |
| varchar(36) event_id UK "NOT NULL / 전역 유일 키(소비자 멱등 기준, payload에도 동봉)" | |
| varchar(100) aggregate_id "NOT NULL / Kafka 파티셔닝 키(= productId 등)" | |
| varchar(100) topic "NOT NULL / catalog-events | order-events" | |
| text payload "NOT NULL / 발행 메시지 JSON 원문" | |
| outbox_events { | |
| bigint id PK | |
| varchar(36) event_id UK "NOT NULL / 전역 유일 키(소비자 멱등 기준, payload에도 동봉)" | |
| varchar(100) aggregate_id "NOT NULL / Kafka 파티셔닝 키(= productId 등)" | |
| varchar(100) topic "NOT NULL / catalog-events | order-events | coupon-issue-requests" | |
| text payload "NOT NULL / 발행 메시지 JSON 원문" |
🤖 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 `@docs/week2/04-erd.md` around lines 140 - 145, The `outbox_events.topic`
contract in the ERD is missing the coupon publish path, so update the
`outbox_events` schema description to include `coupon-issue-requests` or
generalize the allowed values to the full Round 7 outbox topic set. Use the
`outbox_events` table definition in the ERD markdown to make the contract match
the existing coupon issuance flow, and then regenerate the ERD/sequence
snapshots to confirm the coupon path is reflected.
| **동시성(집계) — 파티션 직렬화 + 컬럼 단위 쓰기** — `product_metrics`엔 `@Version`이 없다. **행이 이미 존재하는 갱신(update) 경합** 기준으로 락이 필요 없다: 같은 `product_id` 이벤트는 `aggregate_id` 키로 같은 파티션→단일 소비자 스레드로만 처리되므로(파티션 직렬화), 한 카운터의 `find→증감→save`가 직렬화된다. **단, 카운터마다 writer가 다르다** — like/view는 `product-metrics` collector가, sales는 `product-sales` collector가 같은 행을 건드린다(다른 스레드). 각 카운터는 단일 writer지만 행은 다중 writer라, **`@DynamicUpdate`(변경 컬럼만 UPDATE)** 로 `like_count`·`sales_count`가 서로를 덮어쓰지 않게 해 교차 lost update를 막는다(`@Version` 낙관락 대신 컬럼 단위 쓰기 — `inventories`의 lost update 방지와 같은 패턴). 멱등 원장의 복합 PK `(event_id, handler)`는 하나의 원장을 두 collector가 공유하되 멱등 판정을 handler별로 분리해, 같은 이벤트를 서로 다른 소비자가 각각 한 번씩 처리하게 한다. | ||
|
|
||
| > **행 최초 생성(insert) 경합은 별개** — "락 불필요"는 위처럼 *갱신* 경합에 대한 것이다. 두 collector는 모두 `findById().orElseGet(ProductMetrics.of)` 구조라, **같은 신규 상품의 첫 catalog 이벤트와 첫 order 이벤트가 동시에 오면 둘 다 INSERT를 시도**할 수 있다. 이때는 `product_id`가 PK라 **한쪽만 성공하고 다른 쪽은 무결성 위반→Kafka 재시도→기존 행 UPDATE**로 자가 치유된다(손실 없음, 최초 1회의 좁은 창). 더 단단하게 가려면 앱 레벨 find-or-create 대신 **DB 원자 업서트**(`INSERT … ON DUPLICATE KEY UPDATE col = col + ?`)로 경합 창 자체를 없앨 수 있으나, 도메인 객체 변경(load→mutate→save)과 `@DynamicUpdate` 방식을 SQL 업서트로 바꾸는 큰 변경이라 현재는 **재시도 자가 치유 + 동시 최초 생성 보존 테스트**로 근거를 세우고 업서트는 대안으로 남긴다. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
블록쿼트 내부의 빈 줄을 제거해야 한다.
현재 빈 줄 때문에 markdownlint MD028 경고가 발생한다. 이 경고는 docs CI를 실패시켜 이번 변경을 막을 수 있다. 인용문을 한 블록쿼트로 붙이거나 빈 줄을 삭제해야 한다.
수정 예시
-> **행 최초 생성(insert) 경합은 별개** — ...
-
-> **왜 FK가 없나** — ...수정 후 markdownlint docs/week2/04-erd.md를 다시 돌려야 한다.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 414-414: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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 `@docs/week2/04-erd.md` at line 414, 블록쿼트 내부의 빈 줄로 인해 markdownlint MD028 경고가
발생하고 있으니, 해당 인용문을 처리하는 부분에서 빈 줄을 제거하거나 블록쿼트를 하나로 붙이도록 수정하세요.
`docs/week2/04-erd.md`의 `<details>` 안에 있는 인용/예시 텍스트를 찾아 빈 줄 없이 연속된 블록쿼트 형태로 정리하면
됩니다. 수정 후 `markdownlint docs/week2/04-erd.md`를 다시 실행해 경고가 사라졌는지 확인하세요.
Source: Linters/SAST tools
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
문제 정의
현재 동작/제약:
issueLimit을 넘지 않아야 하고, 한 사용자는 같은 템플릿 쿠폰을 한 장만 받을 수 있어야 한다.문제(또는 리스크):
성공 기준(완료 정의):
ApplicationEvent로 분리한다.issueLimit을 초과하지 않고, 사용자는requestId로 처리 결과를 조회할 수 있다.선택지와 결정
고려한 대안:
commerce-api에 배치commerce-streamer에 배치하고JdbcTemplate으로 최소 SQL 처리최종 결정:
ApplicationEvent와@TransactionalEventListener로 분리한다.Transactional Outbox + Kafka로 처리한다.commerce-streamer는catalog-events,order-events를 소비해product_metrics를 갱신하고,event_handled로 멱등 처리한다.commerce-streamer가coupon-issue-requests를 소비해 발급을 확정한다.JdbcTemplate조건부 원자 UPDATE로 방어하고, 중복 발급은user_couponsunique constraint와 요청 상태로 방어한다.트레이드오프:
product_metrics는 eventual consistency 모델이라 원천 데이터와 일시적으로 다를 수 있다.추후 개선 여지(있다면):
🤔 고민한 점 / 막혔던 부분
EDA 설계에 시간이 오래 걸려, Kafka Config를 직접 만지며 파티션·컨슈머·ack 옵션을 조절해보는 경험까지는 못 가본 것이 아쉬웠습니다.
또한 선착순 쿠폰을 commerce-streamer에서 처리하기로 하면서, 도메인 재사용 대신 JdbcTemplate 최소 SQL을 택했습니다. 도메인 클래스를 복제하진 않았지만, 발급 규칙(한도·스냅샷·만료일) 자체가 api의 도메인 코드와 streamer의 SQL 두 곳에 나뉘어 존재하게 됐습니다. 컴파일러가 둘의 일치를 보장해주지 못해, 한쪽만 바뀌면 조용히 어긋날 수 있다는 점이 마음에 걸렸고, 이 드리프트를 어떻게 후속 관리할지가 남은 고민입니다.
🙋 기타