Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughKafka 의존성을 추가하고 아웃박스 패턴과 이벤트 기반 아키텍처를 도입했다. 쿠폰 발급, 좋아요, 주문 생성 시 이벤트를 발행하며, 별도의 스트리머 애플리케이션에서 이 이벤트를 소비하여 처리한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CommerceAPI as Commerce API
participant OutboxDB as Outbox DB
participant Kafka
participant Streamer as Commerce Streamer
participant AppDB as App DB
rect rgba(100, 150, 200, 0.5)
Note over Client,AppDB: 쿠폰 발급 요청 흐름
Client->>CommerceAPI: POST /api/v1/coupons/{couponId}/issue
CommerceAPI->>CommerceAPI: 사용자 인증
CommerceAPI->>AppDB: CouponIssueRequest 생성
CommerceAPI->>OutboxDB: OutboxEvent 저장<br/>(eventType: COUPON_ISSUE_REQUESTED)
CommerceAPI->>Client: CouponIssueInfo 반환
end
rect rgba(150, 150, 100, 0.5)
Note over CommerceAPI,Streamer: 아웃박스 릴레이 및 스트리머 처리
CommerceAPI->>OutboxDB: findPendingEvents() 1초마다
CommerceAPI->>Kafka: publish to COUPON_ISSUE_REQUESTS
Kafka->>Streamer: consume COUPON_ISSUE_REQUESTS
Streamer->>Streamer: 멱등성 확인 (eventId)
Streamer->>AppDB: Coupon 조회 (비관적 락)
Streamer->>AppDB: 재고 검증
Streamer->>AppDB: IssuedCoupon 저장
Streamer->>AppDB: CouponIssueRequest markSuccess()
Streamer->>AppDB: EventHandled 기록
end
rect rgba(200, 150, 150, 0.5)
Note over Client,AppDB: 좋아요 이벤트 흐름
Client->>CommerceAPI: POST /api/v1/likes
CommerceAPI->>AppDB: Like 엔티티 생성
CommerceAPI->>OutboxDB: OutboxEvent 저장<br/>(eventType: LIKE_ADDED)
CommerceAPI->>CommerceAPI: LikeAddedEvent 발행 (인프로세스)
CommerceAPI->>AppDB: ProductService.increaseLikes()
CommerceAPI->>Client: 응답 반환
end
rect rgba(150, 200, 150, 0.5)
Note over Kafka,Streamer: 스트리머의 Catalog 이벤트 처리
Kafka->>Streamer: consume CATALOG_EVENTS<br/>(LIKE_ADDED/LIKE_REMOVED)
Streamer->>Streamer: 멱등성 확인 (eventId)
Streamer->>AppDB: ProductMetrics 업데이트<br/>(likeCount 증감)
Streamer->>AppDB: EventHandled 기록
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 주요 복잡도 요인:
운영 관점 주의사항:
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/kafka/src/main/resources/kafka.yml (1)
24-24:⚠️ Potential issue | 🔴 CriticalConsumer
value-serializer가 아닌value-deserializer를 사용해야 한다.Consumer 설정에서
value-serializer로 지정되어 있으나, Consumer는 메시지를 역직렬화하므로value-deserializer가 올바른 키이다. 이 오류로 인해 Kafka Consumer가 메시지 수신 시 역직렬화 실패가 발생할 수 있다.🐛 수정안
consumer: group-id: loopers-default-consumer key-deserializer: org.apache.kafka.common.serialization.StringDeserializer - value-serializer: org.apache.kafka.common.serialization.ByteArrayDeserializer + value-deserializer: org.apache.kafka.common.serialization.ByteArrayDeserializer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/kafka/src/main/resources/kafka.yml` at line 24, The Kafka consumer property key is incorrect: replace the consumer config key "value-serializer" with "value-deserializer" in kafka.yml so the consumer uses deserialization; keep the deserializer class value as org.apache.kafka.common.serialization.ByteArrayDeserializer and update any equivalent "key-serializer"/"key-deserializer" entries if present to match consumer semantics.
🟡 Minor comments (11)
apps/commerce-api/src/main/java/com/loopers/domain/order/OrderCreatedEvent.java-5-6 (1)
5-6:⚠️ Potential issue | 🟡 Minor팩토리 메서드에 null 방어가 필요하다.
운영에서 예외가 발생했을 때 원인 메시지 없는 NPE는 장애 추적 시간을 늘린다. 수정안으로
from시작 지점에서Objects.requireNonNull(order, "...")로 실패를 명확히 해야 한다. 추가 테스트로OrderCreatedEvent.from(null)호출 시 의도한 메시지로 즉시 실패하는지 검증해야 한다. As per coding guidelines**/*.java: "null 처리 ... 안정성을 점검한다."수정 예시 diff
package com.loopers.domain.order; +import java.util.Objects; + public record OrderCreatedEvent(Long orderId, Long userId, Long totalAmount) { public static OrderCreatedEvent from(Order order) { + Objects.requireNonNull(order, "order must not be null"); return new OrderCreatedEvent(order.getId(), order.getUserId(), order.getTotalAmount()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderCreatedEvent.java` around lines 5 - 6, Add a null-check at the start of the factory method OrderCreatedEvent.from(Order) by calling Objects.requireNonNull(order, "order must not be null") so a clear NPE message is thrown instead of an ambiguous one; update the method in class OrderCreatedEvent and add a unit test that calls OrderCreatedEvent.from(null) asserting a NullPointerException with the same message to verify behavior.apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.java-9-14 (1)
9-14:⚠️ Potential issue | 🟡 Minor
coupon_issue_requests테이블에coupon_id컬럼 인덱스를 추가하다.
findByCouponId와countByCouponIdAndStatus쿼리가 모두coupon_id컬럼을 조건으로 사용하는데, 현재 인덱스가 정의되지 않았다. 데이터 증가에 따라 전체 테이블 스캔으로 인한 성능 저하가 발생한다.수정안:
coupon_id단일 인덱스 추가:CREATE INDEX idx_coupon_issue_requests_coupon_id ON coupon_issue_requests(coupon_id)- 선택적으로
(coupon_id, status)복합 인덱스 고려 (countByCouponIdAndStatus최적화)추가 테스트:
- 쿠폰 발급 대량 요청 시나리오에서 쿼리 응답 시간 측정
- 쿠폰_이슈_요청 테이블 100만 행 이상 상황에서 인덱스 적용 전후 성능 비교
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.java` around lines 9 - 14, Add a DB index for coupon_id because repository methods findByCouponId and countByCouponIdAndStatus (in CouponIssueRequestJpaRepository) filter on coupon_id; apply the single-column index with SQL like CREATE INDEX idx_coupon_issue_requests_coupon_id ON coupon_issue_requests(coupon_id), and optionally add a composite index for (coupon_id, status) to further optimize countByCouponIdAndStatus; after applying the migration, run load tests (bulk coupon issue and a >1M rows scenario) to compare query latency before/after.modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaTopics.java-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor패키지명
confg는config의 오타로, 8개 파일에서 참조 중이다.현재 패키지명이
com.loopers.confg.kafka로 되어 있으나, 표준 명명 관례에 따르면com.loopers.config.kafka로 수정되어야 한다. 이 오타는 다음과 같은 문제를 초래한다:
- 코드 검색(grep, IDE 검색) 시 혼란 초래
- 개발자의 오타 가능성으로 인한 실수 유발
- 패키지명 일관성 부족
검증 결과, 다음 8개 파일에서 해당 패키지를 참조하고 있으므로 패키지명 변경 시 일괄 수정이 필요하다:
- modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaConfig.java
- apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java
- apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java
- apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponFacade.java
- apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/DemoKafkaConsumer.java
- apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java
- apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java
모든 참조 파일의 import 문을 수정한 후, 각 모듈의 빌드 및 단위 테스트를 실행하여 패키지 변경으로 인한 영향이 없는지 확인해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaTopics.java` at line 1, The package declaration "com.loopers.confg.kafka" is misspelled and must be renamed to "com.loopers.config.kafka"; update the package line in KafkaTopics.java and change all import statements that reference com.loopers.confg.kafka to com.loopers.config.kafka across the codebase (e.g., imports in KafkaConfig, OrderFacade, LikeFacade, CouponFacade, DemoKafkaConsumer, CouponIssueConsumer, CatalogEventConsumer), then rebuild and run unit tests for affected modules to ensure no breakage from the refactor.apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemJpaRepository.java-11-12 (1)
11-12:⚠️ Potential issue | 🟡 Minor
order_id컬럼에 명시적 인덱스 추가 필요
findByOrderIdIn메서드가 생성하는WHERE order_id IN (...)쿼리는 현재 orderId 컬럼에 인덱스가 없으면 order_items 테이블 전체를 스캔한다. OrderFacade에서 대량의 주문에 대한 아이템을 조회하는 패턴이므로, 쿼리 성능 저하로 이어질 수 있다.OrderItem 엔티티의 orderId 필드에
@Index애너테이션을 추가하거나,@Table의 indexes 파라미터에 인덱스를 정의하는 것을 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemJpaRepository.java` around lines 11 - 12, The repository method findByOrderIdIn generates WHERE order_id IN (...) queries that will cause full table scans if the OrderItem.orderId column is not indexed; update the OrderItem entity to add an index on the orderId field (either by annotating the field with `@Index` or adding an entry to the `@Table`(indexes = ... ) annotation) so the DB will create an index for order_id and improve performance when OrderFacade queries many orders.apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/Coupon.java-23-24 (1)
23-24:⚠️ Potential issue | 🟡 Minor
totalQuantity필드 타입과 생성자 파라미터 타입이 불일치한다.필드는
Integer(nullable)이고 생성자는int(non-null)를 받는다. DB 컬럼이nullable=false이므로 필드도int로 통일하거나, 생성자에서 null 검증을 추가해야 한다. 기존 데이터에 NULL이 있을 경우 조회 시 NPE 위험이 있다.🛠️ int 타입으로 통일 제안
- `@Column`(name = "total_quantity", nullable = false) - private Integer totalQuantity; + `@Column`(name = "total_quantity", nullable = false) + private int totalQuantity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/Coupon.java` around lines 23 - 24, The field totalQuantity is declared as Integer but the constructor takes an int, causing a mismatch and potential NPE; fix by making the types consistent—preferably change the field declaration totalQuantity from Integer to int (and update any getters/setters and usages) and update the constructor parameter to int if not already, or alternatively keep Integer and add a null-check in the Coupon constructor to validate/convert a null to a safe default; adjust the field, constructor (Coupon), and any accessor methods to match the chosen non-null primitive type.apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java-64-90 (1)
64-90:⚠️ Potential issue | 🟡 Minor주문 생성 테스트가 핵심 outbox 계약을 놓치고 있다.
현재 검증은
publishEvent(any(OrderCreatedEvent.class))만 있어서, outbox 저장이 빠지거나topic/key/payload가 잘못 나가도 통과한다. 운영에서는 주문은 생성됐는데 Kafka 후속 처리만 멈추는 회귀를 CI가 잡지 못한다.outboxEventPublisher.publish(...)의 인자를 정확히 검증하고,OrderCreatedEvent도any()대신 실제orderId를 캡처해 확인해야 한다. 추가로 outbox 저장 또는 주문 생성이 예외를 던질 때 두 publisher가 호출되지 않는 실패 케이스 테스트를 넣어야 한다. As per coding guidelines, "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java` around lines 64 - 90, The test only verifies eventPublisher.publishEvent(any(OrderCreatedEvent.class)) and misses asserting the outbox contract and exact OrderCreatedEvent contents; update publishesOrderCreatedEvent_whenOrderCreated to capture and assert outboxEventPublisher.publish(...) is called with the correct topic/key/payload (use an ArgumentCaptor for the outbox payload) and replace any(OrderCreatedEvent.class) with a captured OrderCreatedEvent to assert the orderId matches the created Order (order.getId()); also add a failure-path test that simulates orderService.createOrder(...) or the outbox save throwing an exception and assert that neither eventPublisher.publishEvent(...) nor outboxEventPublisher.publish(...) are invoked in that exception case.apps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.java-57-98 (1)
57-98:⚠️ Potential issue | 🟡 Minor좋아요 테스트도 outbox와 payload를 함께 고정해야 한다.
현재 두 테스트 모두
publishEvent(any(...))만 확인해서, outbox 저장이 빠지거나 잘못된productId/userId로 발행돼도 통과한다. 운영에서는 좋아요 DB 상태와 Kafka 기반 지표 갱신이 어긋나는 회귀를 CI가 놓친다.outboxEventPublisher.publish(...)의topic/key/eventType/data와LikeAddedEvent/LikeRemovedEvent의 실제 필드를 캡처해 검증해야 한다. 추가로likeService가 예외를 던질 때 두 publisher가 호출되지 않는 실패 케이스도 넣어야 한다. As per coding guidelines, "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.java` around lines 57 - 98, Update the LikeFacadeTest tests to assert concrete event payloads instead of using any(...): for publishesLikeAddedEvent_whenLikeAdded and publishesLikeRemovedEvent_whenLikeRemoved, capture the arguments passed to eventPublisher.publishEvent and outboxEventPublisher.publish (use ArgumentCaptor) and assert LikeAddedEvent/LikeRemovedEvent fields (userId/loginId, productId) match the arranged User and Product, and assert outboxEventPublisher.publish was called with the expected topic, key, eventType and serialized data matching the event payload; additionally add failure-case tests where likeService (or likeFacade internals) throws and assert neither eventPublisher.publishEvent nor outboxEventPublisher.publish is invoked. Ensure you reference likeFacade.addLike, likeFacade.removeLike, eventPublisher, outboxEventPublisher.publish, LikeAddedEvent and LikeRemovedEvent when locating code to change.apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java-58-69 (1)
58-69:⚠️ Potential issue | 🟡 MinorJSON 필드 null 체크가 누락되어 있다.
node.get("eventId"),node.get("data")등이 null 반환 시 NPE가 발생한다.CatalogEventConsumer와 동일하게 방어적 null 체크를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java` around lines 58 - 69, Add defensive null checks in CouponIssueConsumer where you read JSON fields: verify node.get("eventId") and node.get("data") are non-null before calling asText()/asLong(), and likewise ensure data.get("requestId"), data.get("couponId"), and data.get("userId") are non-null before using asLong(); if any required field is missing, log a debug/error with the eventId (if available) and return early (similar to CatalogEventConsumer) so eventHandledRepository.existsById and subsequent processing never receive nulls.apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java-53-65 (1)
53-65:⚠️ Potential issue | 🟡 MinorJSON 필드 접근 시 NPE 위험이 있다.
node.get("eventId"),node.get("eventType"),node.get("data")등이 null을 반환할 수 있다. 잘못된 형식의 메시지가 들어오면NullPointerException이 발생하여 해당 레코드 처리가 실패하고, DLQ로 전송되지 않은 채 로그만 남게 된다. 운영 시 원인 파악이 어려워진다.수정안: null 체크 및 명확한 예외 처리
String rawPayload = new String((byte[]) record.value()); JsonNode node = objectMapper.readTree(rawPayload); -String eventId = node.get("eventId").asText(); -String eventType = node.get("eventType").asText(); +JsonNode eventIdNode = node.get("eventId"); +JsonNode eventTypeNode = node.get("eventType"); +if (eventIdNode == null || eventTypeNode == null) { + throw new IllegalArgumentException("필수 필드 누락: eventId 또는 eventType"); +} +String eventId = eventIdNode.asText(); +String eventType = eventTypeNode.asText();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java` around lines 53 - 65, The code in CatalogEventConsumer reads JSON fields (eventId, eventType, occurredAt, data, productId) without null checks which can cause NPEs; update the parsing in the consumer (around variables eventId, eventType, occurredAt, data, productId and the objectMapper.treeToValue call) to first validate node.hasNonNull("eventId"), node.hasNonNull("eventType"), node.hasNonNull("data") and node.hasNonNull("occurredAt") and that data.hasNonNull("productId"); if any required field is missing or of the wrong type, throw a clear runtime exception (e.g., IllegalArgumentException) including the raw node/payload and a short reason so the message is rejected consistently (DLQ-friendly) instead of causing an NPE; also wrap objectMapper.treeToValue(… ZonedDateTime.class) in a try/catch to rethrow a descriptive exception on parse errors.apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java-42-47 (1)
42-47:⚠️ Potential issue | 🟡 Minor동일 타임스탬프의 이벤트가 누락될 수 있다.
eventTime.isAfter(this.updatedAt)조건은 동일 시각의 이벤트를 무시한다. 밀리초 단위로 동시에 발생한 좋아요 이벤트가 있으면 두 번째 이벤트가 처리되지 않는다. 빈도는 낮지만 데이터 정합성 이슈가 될 수 있다.수정안: 동일 시각 허용
public void increaseLikeCount(ZonedDateTime eventTime) { - if (eventTime.isAfter(this.updatedAt)) { + if (!eventTime.isBefore(this.updatedAt)) { this.likeCount++; this.updatedAt = eventTime; } }단, 이 경우 중복 처리 방지는
EventHandled테이블의 멱등성 체크에 의존해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java` around lines 42 - 47, The current condition in increaseLikeCount(ZonedDateTime eventTime) uses eventTime.isAfter(this.updatedAt) which ignores events with an identical timestamp; change the condition to accept equal timestamps (e.g., eventTime.isAfter(this.updatedAt) || eventTime.isEqual(this.updatedAt) or use !eventTime.isBefore(this.updatedAt)) so that events with the same ZonedDateTime are applied; keep in mind duplicate protection should be enforced by the EventHandled idempotency check rather than this timestamp comparison.apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerConcurrencyTest.java-82-87 (1)
82-87:⚠️ Potential issue | 🟡 Minor예외를 무시하면 테스트 실패 원인 파악이 어렵다.
catch (Exception ignored)로 모든 예외를 삼키면, 예상치 못한 오류(예: DB 연결 실패, 잘못된 직렬화)가 발생해도 테스트가 통과할 수 있다. 실제 동시성 문제가 아닌 환경 문제로 발급 수가 적어져도 테스트가 성공으로 처리된다.수정안: 예외 수집 및 검증
+List<Exception> errors = Collections.synchronizedList(new ArrayList<>()); + for (ConsumerRecord<Object, Object> record : records) { executor.submit(() -> { try { startLatch.await(); consumer.consume(List.of(record), ack); - } catch (Exception ignored) { + } catch (Exception e) { + errors.add(e); } finally { doneLatch.countDown(); } }); } + +// Assert 섹션에 추가 +assertThat(errors).isEmpty();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerConcurrencyTest.java` around lines 82 - 87, The test swallows all exceptions in the worker thread (catch (Exception ignored)) which hides failures; change the thread lambda in CouponIssueConsumerConcurrencyTest so it captures exceptions instead of ignoring them (e.g., add a ConcurrentLinkedQueue<Throwable> or AtomicReference<Throwable> declared outside the lambda), push any caught exception from the catch block into that collection when invoking consumer.consume(List.of(record), ack), and after awaiting doneLatch check/throw/assert that the collection is empty (or rethrow the first captured Throwable) so test fails on real errors; keep doneLatch.countDown() in finally.
🧹 Nitpick comments (15)
apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java (1)
33-39: 상태 전이에 도메인 규칙 가드가 필요하다.운영에서 재처리나 중복 소비가 발생하면
SUCCESS가FAILED로 덮어써져 집계/모니터링/고객 응대 데이터가 왜곡될 수 있다. 수정안으로PENDING -> SUCCESS|FAILED만 허용하고, 이미 종결된 상태에서의 변경은 no-op 또는 예외로 차단하는 전이 규칙을 메서드에 추가해야 한다. 추가 테스트로 전이 매트릭스 테스트(PENDING->SUCCESS,PENDING->FAILED,SUCCESS->FAILED,FAILED->SUCCESS, 동일 상태 재호출)를 명시해 회귀를 방지해야 한다. As per coding guidelines**/domain/**/*.java: "도메인 규칙과 인프라 관심사가 섞이면 분리하도록 제안한다."수정 예시 diff
public class CouponIssueRequest extends BaseEntity { @@ public void markSuccess() { - this.status = CouponIssueStatus.SUCCESS; + if (this.status == CouponIssueStatus.SUCCESS) { + return; + } + if (this.status != CouponIssueStatus.PENDING) { + throw new IllegalStateException("Invalid transition: " + this.status + " -> SUCCESS"); + } + this.status = CouponIssueStatus.SUCCESS; } public void markFailed() { - this.status = CouponIssueStatus.FAILED; + if (this.status == CouponIssueStatus.FAILED) { + return; + } + if (this.status != CouponIssueStatus.PENDING) { + throw new IllegalStateException("Invalid transition: " + this.status + " -> FAILED"); + } + this.status = CouponIssueStatus.FAILED; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.java` around lines 33 - 39, The markSuccess and markFailed methods in CouponIssueRequest must enforce domain transition guards so only PENDING -> SUCCESS or PENDING -> FAILED are allowed; update markSuccess() and markFailed() to check this.status == CouponIssueStatus.PENDING and either set the new status or else be a no-op (or throw a DomainException per project convention) to prevent overwriting terminal states, keep logic inside CouponIssueRequest (domain layer) and do not mix infra concerns; also add unit tests for the transition matrix covering PENDING->SUCCESS, PENDING->FAILED, SUCCESS->FAILED, FAILED->SUCCESS and repeated calls to the same transition to prevent regressions.apps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1Dto.java (1)
8-22:userId를 API 응답에 포함하는 것이 적절한지 검토가 필요하다.
userId는 사용자 식별 정보(PII)에 해당할 수 있다. 클라이언트가 본인의 발급 요청만 조회한다면userId를 응답에 포함할 필요가 없을 수 있다. 타인의 요청 정보를 열람할 수 있는 API라면userId노출이 보안 문제가 될 수 있다.추가 테스트: 권한 없는 사용자가 타인의
requestId로 조회 시userId가 노출되는지 확인하는 인가 테스트가 필요하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/coupon/CouponV1Dto.java` around lines 8 - 22, IssueResponse currently exposes userId (from CouponIssueInfo) which may be PII; remove the userId field from the IssueResponse record and from the static IssueResponse.from(CouponIssueInfo) mapping so API responses no longer include user identifiers, and instead rely on server-side authorization to return only the requesting user's data or add an explicit authorizedUserId check in the controller/service layer before constructing IssueResponse; update any callers that construct IssueResponse to use the new constructor signature and add/adjust authorization tests to ensure unauthenticated/unauthorized requests cannot see other users' request details.apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java (1)
47-49: 빈/널orderIds가드가 없어 불필요한 조회 부하를 만들 수 있다.운영 관점에서 배치/집계 호출이 빈 목록을 자주 전달하면 의미 없는 DB 호출이 누적될 수 있다.
수정안은null또는 빈 목록 입력 시 즉시List.of()를 반환하고, 그 외에는 조회를 수행하도록 가드를 추가하는 방식이다.
추가 테스트로null/빈 목록 입력 시 빈 결과 반환 및 저장소 미호출, 정상 목록 입력 시 기존 조회 동작 유지를 검증해야 한다.As per coding guidelines: "null 처리, 방어적 복사, 불변성 ... 점검한다."수정 예시 diff
`@Transactional`(readOnly = true) public List<OrderItem> getOrderItemsByOrderIds(List<Long> orderIds) { + if (orderIds == null || orderIds.isEmpty()) { + return List.of(); + } return orderItemRepository.findByOrderIds(orderIds); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.java` around lines 47 - 49, Add a null/empty guard in getOrderItemsByOrderIds to avoid unnecessary DB calls: if the incoming orderIds is null or orderIds.isEmpty() return List.of() immediately, otherwise call orderItemRepository.findByOrderIds(orderIds); also add unit tests asserting that null and empty inputs return an empty list and do not invoke orderItemRepository, and that a non-empty list delegates to orderItemRepository.findByOrderIds as before.apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemRepositoryImpl.java (1)
21-24:orderIds가 null이거나 빈 리스트일 때의 동작 확인 필요.
orderIds가 null인 경우 JPA 레벨에서 NullPointerException이 발생할 수 있다. 또한, 매우 큰 리스트가 전달될 경우 IN 절의 DB별 제한(Oracle 1000개 등)에 걸릴 수 있다. 호출부에서 방어 로직이 있는지 확인하거나, 여기서 early return을 추가하는 것이 운영 안정성 측면에서 권장된다.♻️ 방어 로직 추가 예시
`@Override` public List<OrderItem> findByOrderIds(List<Long> orderIds) { + if (orderIds == null || orderIds.isEmpty()) { + return List.of(); + } return orderItemJpaRepository.findByOrderIdIn(orderIds); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemRepositoryImpl.java` around lines 21 - 24, In OrderItemRepositoryImpl.replace the current findByOrderIds(List<Long> orderIds) body to guard against null/empty and oversized IN lists: return Collections.emptyList() immediately when orderIds is null or empty, and for large lists split orderIds into chunks (e.g., <= 900) and invoke orderItemJpaRepository.findByOrderIdIn(...) per chunk, aggregating results into a single List<OrderItem> to return; ensure you reference the existing method findByOrderIds and the repository call orderItemJpaRepository.findByOrderIdIn when implementing this logic.modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaTopics.java (1)
3-7: 상수 전용 클래스는 인스턴스화 방지를 위해 private 생성자를 추가해야 한다.
KafkaTopics는 상수만 포함하는 유틸리티 클래스이므로, 인스턴스 생성을 방지하고final로 상속을 막는 것이 관례이다.♻️ 유틸리티 클래스 패턴 적용
-public class KafkaTopics { +public final class KafkaTopics { + private KafkaTopics() { + // prevent instantiation + } + public static final String CATALOG_EVENTS = "catalog-events"; public static final String ORDER_EVENTS = "order-events"; public static final String COUPON_ISSUE_REQUESTS = "coupon-issue-requests"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/kafka/src/main/java/com/loopers/confg/kafka/KafkaTopics.java` around lines 3 - 7, Make KafkaTopics a non-instantiable utility class by declaring the class final and adding a private constructor (e.g., private KafkaTopics() { throw new AssertionError("no instances"); }) to prevent instantiation; keep the existing constants (CATALOG_EVENTS, ORDER_EVENTS, COUPON_ISSUE_REQUESTS) unchanged and ensure no public constructors remain.apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/Coupon.java (1)
28-31: 생성자에서 totalQuantity 음수 검증이 누락되었다.선착순 쿠폰 발급에서
totalQuantity가 음수이면 Redis INCR gate 로직과 재고 비교가 오작동할 수 있다. 도메인 불변성 보장을 위해 생성자에서 방어 코드를 추가하는 것이 좋다.🛡️ 음수 검증 추가
public Coupon(String name, int totalQuantity) { + if (totalQuantity < 0) { + throw new IllegalArgumentException("totalQuantity must be non-negative"); + } this.name = name; this.totalQuantity = totalQuantity; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/Coupon.java` around lines 28 - 31, 생성자 Coupon(String name, int totalQuantity)에서 totalQuantity가 음수일 경우 도메인 불변성이 깨지지 않도록 방어 로직을 추가하세요; Coupon 클래스의 생성자(Coupon)에서 전달받은 totalQuantity를 검사하여 음수이면 IllegalArgumentException(또는 도메인에 맞는 예외)으로 즉시 던지거나 0으로 정규화하는 방식으로 처리하고, 이 검사 후에만 this.totalQuantity에 값을 할당하도록 수정하세요.apps/commerce-api/src/main/java/com/loopers/application/event/OrderEventHandler.java (1)
14-19: @async 메서드의 예외 처리 전략이 부재하다.
@Async메서드에서 발생한 예외는 기본적으로 콘솔에 출력되고 무시된다. 현재는 단순 로깅이라 치명적이지 않지만, 향후 로직 추가 시 장애 추적이 어려워질 수 있다.
AsyncUncaughtExceptionHandler를 구성하거나, 메서드 내 try-catch로 예외를 명시적으로 처리하는 것이 운영 안정성에 도움이 된다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/event/OrderEventHandler.java` around lines 14 - 19, The `@Async` method OrderEventHandler.handle lacks exception handling; either wrap the method body in a try-catch that logs errors with context (orderId, userId, totalAmount) and any stacktrace, or register an AsyncUncaughtExceptionHandler bean (and wire it into your AsyncConfigurer) to centrally handle/log uncaught exceptions from `@Async` methods; pick one approach, update OrderEventHandler.handle or add the AsyncUncaughtExceptionHandler implementation and ensure exceptions include the event details for traceability.apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/IssuedCoupon.java (1)
37-41: 생성자에서 couponId, userId null 검증이 누락되었다.둘 다
nullable=false컬럼이지만 생성자에서 검증이 없다. null 입력 시 DB 저장 단계에서 예외가 발생하므로, 빠른 실패(fail-fast)를 위해 생성자에서 검증하는 것이 디버깅에 유리하다.🛡️ null 검증 추가
public IssuedCoupon(Long couponId, Long userId) { + if (couponId == null || userId == null) { + throw new IllegalArgumentException("couponId and userId must not be null"); + } this.couponId = couponId; this.userId = userId; this.issuedAt = ZonedDateTime.now(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/IssuedCoupon.java` around lines 37 - 41, The IssuedCoupon(Long couponId, Long userId) constructor lacks null checks for couponId and userId; add fail-fast validation at the start of the constructor (in the IssuedCoupon class constructor) to throw IllegalArgumentException (or NullPointerException per project convention) with clear messages like "couponId must not be null" and "userId must not be null" before assigning fields and setting issuedAt so nulls are rejected earlier than the DB layer.apps/commerce-streamer/src/main/java/com/loopers/domain/outbox/EventHandled.java (1)
25-28: eventId null 검증이 없고, ZonedDateTime.now() 호출이 테스트 격리를 어렵게 한다.
eventId가 null이면 DB 저장 시 NPE 또는 constraint violation이 발생한다. 생성자에서 방어 코드 추가가 필요하다.ZonedDateTime.now()직접 호출은 단위 테스트에서 시간 제어가 어렵다.Clock주입 또는 팩토리 메서드 패턴을 고려할 수 있다.🛡️ null 검증 추가 예시
public EventHandled(String eventId) { + if (eventId == null || eventId.isBlank()) { + throw new IllegalArgumentException("eventId must not be null or blank"); + } this.eventId = eventId; this.handledAt = ZonedDateTime.now(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/domain/outbox/EventHandled.java` around lines 25 - 28, The EventHandled constructor lacks null validation for eventId and directly calls ZonedDateTime.now(), making tests brittle; update the EventHandled(String eventId) constructor to validate eventId (e.g., Objects.requireNonNull or throw IllegalArgumentException) and introduce time-source injection by adding an overload or factory that accepts a java.time.Clock (e.g., EventHandled(String eventId, Clock clock)) and set handledAt using ZonedDateTime.now(clock), providing a default constructor or factory that delegates to Clock.systemDefaultZone() for production use so tests can supply a fixed Clock.apps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/IssuedCouponJpaRepository.java (1)
8-10:countByCouponId쿼리 성능 모니터링 권장
IssuedCoupon엔티티에(coupon_id, user_id)복합 유니크 제약이 정의되어 있으나,coupon_id단독 인덱스가 없다.countByCouponId쿼리는 복합 인덱스를 활용하거나 테이블 스캔을 수행하므로, 데이터 증가 시 성능 저하 가능성이 있다.현재는 Redis INCR 게이트가 주요 동시성 제어 역할을 하고
countByCouponId는 백업 검증으로만 사용되므로 즉각적인 문제가 아니다. 운영 중 발급 데이터가 대규모로 증가하면issued_coupons테이블의coupon_id컬럼에 인덱스를 추가하여 COUNT 쿼리 성능을 개선할 필요가 있다.향후 단계에서 데이터 증가 추이를 모니터링하고, 필요시 인덱스 추가 또는 쿼리 최적화를 검토할 것을 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/IssuedCouponJpaRepository.java` around lines 8 - 10, The countByCouponId repository method on IssuedCoupon can suffer table-scan as there is no single-column index on coupon_id; monitor growth of issued_coupons and, if COUNT queries become slow, add a DB index on coupon_id (or a covering index) to support countByCouponId, and consider adding metrics/alerts around execution time of IssuedCouponJpaRepository.countByCouponId and query plans; also confirm the existing (coupon_id, user_id) unique constraint still supports existsByCouponIdAndUserId without change.apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerConcurrencyTest.java (1)
91-92: ExecutorService 종료 처리가 불완전하다.
executor.shutdown()은 새 작업 제출만 막고, 실행 중인 작업 완료를 기다리지 않는다.doneLatch.await()가 타임아웃되면 백그라운드 스레드가 여전히 DB에 접근할 수 있어 다음 테스트에 영향을 줄 수 있다.수정안
-doneLatch.await(30, TimeUnit.SECONDS); -executor.shutdown(); +boolean completed = doneLatch.await(30, TimeUnit.SECONDS); +executor.shutdown(); +executor.awaitTermination(5, TimeUnit.SECONDS); +assertThat(completed).isTrue();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerConcurrencyTest.java` around lines 91 - 92, The test currently only calls executor.shutdown() after doneLatch.await(...), which doesn't wait for running tasks to finish and can leave background threads touching the DB; update the teardown to (1) check the boolean result of doneLatch.await(30, TimeUnit.SECONDS), (2) call executor.shutdown() and then block on executor.awaitTermination(...) for a short timeout, and (3) if termination times out, call executor.shutdownNow() to force interruption; reference the existing doneLatch.await(...) and executor.shutdown() calls when making the change so the test reliably stops background threads before finishing.apps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.java (1)
28-30: 동시 업데이트 보호를 위한 낙관적 잠금 고려가 필요하다.다중 스케줄러 인스턴스 환경에서 동일 이벤트에 대한 동시 업데이트가 발생할 수 있다.
FOR UPDATE SKIP LOCKED가 repository에서 적용되더라도,@Version필드를 추가하면 추가적인 동시성 보호와 lost update 감지가 가능하다.수정안
`@Column`(name = "status", nullable = false, length = 20) private OutboxStatus status = OutboxStatus.PENDING; + +@Version +private Long version;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.java` around lines 28 - 30, OutboxEvent 엔티티에 낙관적 잠금을 추가하세요: 클래스 OutboxEvent에 `@Version이` 붙은 Long 타입 필드(예: private Long version;)를 추가해 JPA의 Optimistic Locking을 활성화하고, 필요한 getter/setter를 생성해 주입되도록 하세요; 이렇게 하면 repository의 FOR UPDATE SKIP LOCKED와 함께 동시 업데이트 시 lost update 감지(OptimisticLockException)를 받을 수 있습니다.apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponConcurrencyTest.java (2)
83-89: 예외를 무시하면 동시성 테스트의 신뢰성이 떨어진다.
catch (Exception ignored)패턴은 테스트 중 발생한 오류를 숨긴다. 앞서 리뷰한CouponIssueConsumerConcurrencyTest와 동일하게 예외 수집 후 검증하는 방식을 권장한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponConcurrencyTest.java` around lines 83 - 89, The test currently swallows exceptions in the worker threads (catch (Exception ignored)) which hides failures; update CouponConcurrencyTest to collect exceptions from each thread (e.g., a ConcurrentLinkedQueue<Throwable> or AtomicReference<Throwable>) inside the catch block when calling couponFacade.requestIssue(loginId, rawPassword, coupon.getId()), and after doneLatch.await() assert that the collection is empty or rethrow the first captured exception so failures are visible (mirror the approach used in CouponIssueConsumerConcurrencyTest); ensure startLatch and doneLatch usage remains the same but remove silent ignores and perform verification on the collected exceptions at the end of the test.
117-127: 실제 컨슈머 코드를 테스트하지 않고 시뮬레이션만 수행한다.이 테스트는
CouponIssueRequest의 상태를 직접markSuccess()/markFailed()로 변경하여 "컨슈머 순차 처리를 시뮬레이션"한다. 실제CouponIssueConsumer의 비즈니스 로직(중복 발급 체크, 수량 확인,IssuedCoupon저장 등)은 테스트되지 않는다.테스트 이름
exactly100SucceedAnd100Fail_whenProcessedSequentially가 기대하는 바와 실제 검증 내용이 다르다.두 가지 방향을 고려할 수 있다:
- 테스트 이름/설명을 "요청 상태 변경 시나리오 검증"으로 명확히 하거나
- 실제 Consumer를 호출하는 통합 테스트로 변경 (commerce-streamer 모듈의
CouponIssueConsumerConcurrencyTest처럼)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/coupon/CouponConcurrencyTest.java` around lines 117 - 127, The test exactly100SucceedAnd100Fail_whenProcessedSequentially is only simulating consumer behavior by directly calling CouponIssueRequest.markSuccess()/markFailed() and saving via couponIssueRequestJpaRepository, so it doesn't exercise the actual business logic in CouponIssueConsumer (duplication checks, inventory decrement, IssuedCoupon persistence). Either rename the test to reflect it's validating "request state change simulation" or convert it into an integration-style test that invokes the real consumer (e.g., dispatch requests into the same path used by CouponIssueConsumer or reuse the approach from commerce-streamer’s CouponIssueConsumerConcurrencyTest) so the actual methods and classes (CouponIssueConsumer, CouponIssueRequest processing flow, IssuedCoupon persistence) are exercised rather than directly mutating request state.apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayScheduler.java (1)
24-30: 배치 전체를 단일 트랜잭션으로 처리하면 장애 격리가 어렵다.100개 이벤트를 단일 트랜잭션에서 처리 중 마지막 이벤트에서 예외가 발생하면 전체 배치가 롤백된다. PR 설명에 언급된 "이벤트별 독립 트랜잭션(FOR UPDATE SKIP LOCKED)" 설계와 현재 구현이 일치하지 않는다.
수정안: 이벤트별 독립 트랜잭션 적용
-@Scheduled(fixedDelay = 1000) -@Transactional -public void relay() { +@Scheduled(fixedDelay = 1000) +public void relay() { List<OutboxEvent> pendingEvents = outboxEventRepository.findPendingEvents(BATCH_SIZE); if (pendingEvents.isEmpty()) { return; } for (OutboxEvent event : pendingEvents) { - try { - // ... 처리 로직 - } catch (Exception e) { - log.error("[Outbox] 릴레이 처리 실패: eventId={}", event.getEventId(), e); - } + relayEvent(event.getId()); // 별도 트랜잭션 메서드 호출 } - - log.debug("[Outbox] {}건 발행 완료", pendingEvents.size()); } + +@Transactional +public void relayEvent(Long eventId) { + // FOR UPDATE SKIP LOCKED로 개별 조회 후 처리 +}별도 빈의 메서드 호출 또는
TransactionTemplate을 사용해야 프록시 우회 문제를 방지할 수 있다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayScheduler.java` around lines 24 - 30, The relay() method in OutboxRelayScheduler currently loads a batch with outboxEventRepository.findPendingEvents(BATCH_SIZE) and processes them in one `@Transactional` scope, causing whole-batch rollback on a single failure; change to process each OutboxEvent in its own transaction by: (1) modifying the repository query to acquire rows with FOR UPDATE SKIP LOCKED (or add a repository method that does), and (2) moving per-event processing into a separate transactional boundary (either a new `@Service` bean method, e.g., OutboxRelayProcessor.processEvent(OutboxEvent) called from relay(), or use TransactionTemplate to execute per-event work), so each event commit/rollback is isolated and the proxy is not bypassed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a640f4f-cf16-4558-bcac-961fce2dc9ba
📒 Files selected for processing (56)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponFacade.javaapps/commerce-api/src/main/java/com/loopers/application/coupon/CouponIssueInfo.javaapps/commerce-api/src/main/java/com/loopers/application/event/LikeEventHandler.javaapps/commerce-api/src/main/java/com/loopers/application/event/OrderEventHandler.javaapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.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/CouponRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/coupon/CouponService.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeAddedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/like/LikeRemovedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderCreatedEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderItemRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/order/OrderService.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEvent.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEventPayload.javaapps/commerce-api/src/main/java/com/loopers/domain/outbox/OutboxEventPublisher.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/product/ProductService.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/CouponJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/coupon/CouponRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/order/OrderItemRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayScheduler.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRepositoryImpl.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/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/application/coupon/CouponConcurrencyTest.javaapps/commerce-api/src/test/java/com/loopers/application/like/LikeFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.javaapps/commerce-streamer/src/main/java/com/loopers/domain/coupon/Coupon.javaapps/commerce-streamer/src/main/java/com/loopers/domain/coupon/CouponIssueRequest.javaapps/commerce-streamer/src/main/java/com/loopers/domain/coupon/CouponIssueStatus.javaapps/commerce-streamer/src/main/java/com/loopers/domain/coupon/IssuedCoupon.javaapps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaapps/commerce-streamer/src/main/java/com/loopers/domain/outbox/EventHandled.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/CouponIssueRequestJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/CouponJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/IssuedCouponJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/infrastructure/outbox/EventHandledJpaRepository.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.javaapps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.javaapps/commerce-streamer/src/test/java/com/loopers/interfaces/consumer/CouponIssueConsumerConcurrencyTest.javamodules/kafka/src/main/java/com/loopers/confg/kafka/KafkaTopics.javamodules/kafka/src/main/resources/kafka.yml
| @Transactional(readOnly = true) | ||
| public List<CouponInfo.IssuedInfo> getMyCoupons(String loginId, String rawPassword) { | ||
| User user = userService.authenticate(loginId, rawPassword); | ||
| return issuedCouponRepository.findAllByUserId(user.getId()).stream() | ||
| .map(CouponInfo.IssuedInfo::from) | ||
| .toList(); | ||
| public CouponIssueInfo getIssueResult(String loginId, String rawPassword, Long requestId) { | ||
| userService.authenticate(loginId, rawPassword); | ||
| CouponIssueRequest request = couponService.getRequest(requestId); | ||
| return CouponIssueInfo.from(request); |
There was a problem hiding this comment.
발급 요청 조회에 소유자 검증이 빠져 있다.
Line 41에서 인증만 하고 request.getUserId()를 비교하지 않아, 다른 사용자가 requestId만 알면 발급 성공/실패 상태를 조회할 수 있다. 이는 쿠폰 발급 이력 노출로 이어지는 권한 우회다. authenticate(...) 결과의 사용자와 요청 소유자를 비교하고 불일치 시 기존 패턴대로 NOT_FOUND를 반환해야 한다. 추가로 다른 사용자로 getIssueResult를 호출했을 때 CoreException(ErrorType.NOT_FOUND)가 발생하는 테스트를 넣어야 한다.
수정 예시
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
...
`@Transactional`(readOnly = true)
public CouponIssueInfo getIssueResult(String loginId, String rawPassword, Long requestId) {
- userService.authenticate(loginId, rawPassword);
+ User user = userService.authenticate(loginId, rawPassword);
CouponIssueRequest request = couponService.getRequest(requestId);
+ if (!request.getUserId().equals(user.getId())) {
+ throw new CoreException(ErrorType.NOT_FOUND, "쿠폰 발급 요청을 찾을 수 없습니다.");
+ }
return CouponIssueInfo.from(request);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/coupon/CouponFacade.java`
around lines 39 - 43, getIssueResult currently only authenticates the caller but
does not verify ownership of the CouponIssueRequest, allowing any authenticated
user who knows requestId to read another user's issuance info; update
CouponFacade.getIssueResult to compare the authenticated user's id (from
userService.authenticate(...) return or context) with request.getUserId() and
throw CoreException(ErrorType.NOT_FOUND) when they differ (matching the existing
NOT_FOUND pattern), and add a unit/integration test that calls getIssueResult as
a different user and asserts CoreException(ErrorType.NOT_FOUND) is thrown.
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| @Async | ||
| public void handleLikeAdded(LikeAddedEvent event) { | ||
| productService.increaseLikes(event.productId()); | ||
| log.info("[UserBehavior] 좋아요 추가 - userId={}, productId={}", event.userId(), event.productId()); | ||
| } | ||
|
|
||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| @Async | ||
| public void handleLikeRemoved(LikeRemovedEvent event) { | ||
| productService.decreaseLikes(event.productId()); | ||
| log.info("[UserBehavior] 좋아요 취소 - userId={}, productId={}", event.userId(), event.productId()); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring Framework, when a listener uses both @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) and @async, is the event persisted or retried automatically if the listener throws or the process stops after the transaction commits?
💡 Result:
No.
@TransactionalEventListener(phase = AFTER_COMMIT)only defers invocation until after the surrounding transaction successfully commits; it does not make the event durable (it’s still an in-memory application event). (docs.spring.io)- Adding
@Asyncjust hands the listener execution off to a TaskExecutor on another thread. If the JVM/process stops after the DB transaction commits but before (or during) that async execution, the event is simply lost—there’s nothing persisted for Spring to replay. (Spring’s transaction-bound event support is for timing, not delivery guarantees.) (docs.spring.io) - If the async listener throws, the original transaction is already committed, so it won’t roll anything back and Spring won’t automatically retry the listener; the exception is merely handled via async exception/error-handling mechanisms (e.g., executor/error handler /
AsyncUncaughtExceptionHandler). (docs.spring.io)
If you need “persist + retry”, you have to add it yourself (e.g., transactional outbox table + background publisher, or publish to a durable broker like JMS/Kafka with retries).
Citations:
- 1: https://docs.spring.io/spring-framework/docs/6.0.0-RC2/reference/pdf/data-access.pdf
- 2: https://docs.spring.io/spring-framework/docs/6.0.0-RC2/reference/pdf/data-access.pdf
- 3: https://docs.spring.io/spring-framework/docs/5.0.2.RELEASE/kdoc-api/spring-framework/org.springframework.context.event/-simple-application-event-multicaster/set-error-handler.html
🏁 Script executed:
# First, let's examine the full file
find . -name "LikeEventHandler.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
# Check for event definitions and the full handler file
fd "LikeEventHandler" --type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
# Search for typical logging patterns with userId in the codebase
rg "userId.*log\.|log\..*userId" --type java -B2 -A2 | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3458
🏁 Script executed:
# Check if there are any logging guidelines or patterns in similar event handlers
fd "EventHandler" --type f -path "*/application/event/*" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 253
🏁 Script executed:
# Let's view the full LikeEventHandler.java file
cat -n apps/commerce-api/src/main/java/com/loopers/application/event/LikeEventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1559
🏁 Script executed:
# Check if there are exception handlers or error handling patterns in similar event handlers
cat -n apps/commerce-api/src/main/java/com/loopers/application/event/OrderEventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 931
🏁 Script executed:
# Check if there's a security or logging policy document
find . -name "*security*" -o -name "*logging*" -o -name "*SECURITY*" | grep -E "\.(md|txt|properties|yml|yaml)$" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 134
🏁 Script executed:
# Check the logging configuration
cat -n ./supports/logging/src/main/resources/logging.ymlRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 137
🏁 Script executed:
# Search for any documentation or guidelines about logging sensitive data
find . -name "*.md" -type f | xargs grep -l "userId\|sensitive\|로깅\|민감" 2>/dev/null | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 130
🏁 Script executed:
# Check if there are integration tests for event handlers
find . -path "*/test/*" -name "*EventHandler*" -type f | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check the event classes to understand the data structure
find . -name "LikeAddedEvent.java" -o -name "LikeRemovedEvent.java" | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 318
🏁 Script executed:
# Check the ProductService to understand what increaseLikes/decreaseLikes do
find . -name "ProductService.java" -type f | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3052
🏁 Script executed:
# Check for any retry mechanisms or recovery patterns in the codebase
rg "Retryable|@Retry|RetryTemplate|resilience4j" --type java | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check the logback configuration for logging levels
cat -n ./supports/logging/src/main/resources/logback.xml 2>/dev/null | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check if there are other async handlers and how they handle exceptions
rg "@Async" --type java -B5 -A10 | grep -A10 "EventHandler\|Consumer" | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3574
🏁 Script executed:
# Search for TaskExecutor configuration
find . -name "*.java" -type f | xargs grep -l "TaskExecutor\|AsyncConfigur" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check the Product entity to understand likes field
find . -name "Product.java" -path "*/domain/product/*" -type f | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3722
비동기 이벤트 리스너로는 좋아요 카운트 갱신이 유실될 수 있다.
@TransactionalEventListener(AFTER_COMMIT)와 @Async 조합에서는 DB 트랜잭션 커밋 이후 별도 스레드에서 productService.increaseLikes/decreaseLikes를 실행한다. 이 구간에 예외 발생, 스레드 풀 포화, 프로세스 종료 등이 발생하면 재시도 없이 이벤트가 소실되어 좋아요 관계 데이터와 Product.likesCount가 영구히 어긋날 수 있다. 조회 경로에 직접 사용되는 카운트라면 같은 트랜잭션에서 동기 반영하거나, 꼭 비동기여야 한다면 transactional outbox 패턴이나 Kafka 같은 재처리 가능한 메커니즘으로 전환해야 한다.
추가로 핸들러에 예외 처리가 없어서, productService.increaseLikes/decreaseLikes 실패 시 로그나 모니터링 없이 조용히 실패한다. 예외를 잡아서 로깅하고, 리스너 실패/애플리케이션 재시작 시나리오에서 좋아요 수가 보정되는지 확인하는 통합 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/event/LikeEventHandler.java`
around lines 20 - 31, The current `@TransactionalEventListener` methods
handleLikeAdded and handleLikeRemoved call
productService.increaseLikes/decreaseLikes asynchronously (`@Async`) after commit
which risks lost updates; either make the updates synchronous inside the
committing transaction (remove `@Async` and run in same thread) or replace the
async event with a durable, replayable mechanism (implement transactional outbox
or publish to Kafka) so retries are possible; additionally wrap calls to
productService.increaseLikes/decreaseLikes in try/catch inside these handlers to
log failures and surface metrics/alerts, and add an integration test that
simulates handler failure/restart to verify likesCount is eventually
consistent/repaired.
| log.info("[UserBehavior] 좋아요 추가 - userId={}, productId={}", event.userId(), event.productId()); | ||
| } | ||
|
|
||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | ||
| @Async | ||
| public void handleLikeRemoved(LikeRemovedEvent event) { | ||
| productService.decreaseLikes(event.productId()); | ||
| log.info("[UserBehavior] 좋아요 취소 - userId={}, productId={}", event.userId(), event.productId()); |
There was a problem hiding this comment.
INFO 로그에 사용자 식별자를 남기지 마십시오.
Line 24와 Line 31은 고빈도 좋아요 이벤트마다 userId와 productId를 INFO로 남긴다. 운영에서는 로그 비용이 급증하고 사용자 식별자가 과도하게 축적된다. INFO에는 집계 가능한 metric이나 저카디널리티 정보만 남기고, userId는 제거하거나 최소한 DEBUG로 낮추는 편이 안전하다. 추가로 로그 appender 테스트로 INFO 메시지에 userId가 포함되지 않는지 확인해야 한다. As per coding guidelines, "로깅 시 민감정보 노출 가능성을 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/event/LikeEventHandler.java`
around lines 24 - 31, The INFO logs in handleLikeRemoved (and the corresponding
like-add handler) include per-user identifiers (event.userId()), which must be
removed or demoted to DEBUG to avoid high-cardinality sensitive data in INFO;
update the log calls in the methods handleLikeRemoved (and the like-add handler
where log.info("[UserBehavior] 좋아요 추가 ...") is used) to omit event.userId()
(leave productId or other low-cardinality fields only) or change the level to
log.debug(...) if the userId must be retained for troubleshooting; additionally
add or update a log-appender test to assert that INFO-level messages from these
handlers do not contain userId values.
| @EnableAsync | ||
| @EnableScheduling | ||
| @Configuration | ||
| public class AsyncConfig {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: 현재 코드베이스에 명시적 Async/Scheduler 실행기 구성이 있는지 확인한다.
# 기대 결과:
# - ThreadPoolTaskExecutor / ThreadPoolTaskScheduler 빈 정의가 존재해야 한다.
# - `@Async` 사용처가 늘어났다면 실행기 명시가 확인되어야 한다.
echo '--- Async/Scheduling annotations ---'
rg -n '@EnableAsync|@EnableScheduling|@Async|@Scheduled'
echo '--- Executor/Scheduler bean definitions ---'
rg -n 'ThreadPoolTaskExecutor|ThreadPoolTaskScheduler|TaskExecutor|SchedulingConfigurer|AsyncConfigurer|AsyncUncaughtExceptionHandler'
echo '--- Relevant config files ---'
fd -t f '.*Config\.java$|.*Scheduler.*\.java$|.*EventHandler.*\.java$'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1883
🏁 Script executed:
# AsyncConfig 내용 확인
cat -n apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 448
🏁 Script executed:
# `@Async` 메서드들이 executor를 지정하는지 확인
rg -A 5 '@Async' apps/commerce-api/src/main/java/com/loopers/application/event/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2300
🏁 Script executed:
# OutboxRelayScheduler 구현 확인
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/outbox/OutboxRelayScheduler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2077
기본 Async/Scheduling 실행기 의존은 운영 리스크가 크고, 비동기 메서드의 예외 처리가 누락되어 있다.
@EnableAsync와 @EnableScheduling이 활성화되었으나 명시적인 ThreadPoolTaskExecutor, ThreadPoolTaskScheduler, AsyncConfigurer가 정의되지 않아 Spring의 기본 설정을 사용 중이다. 트래픽 급증 시 스레드/큐 제어와 백프레셔가 불명확해 스레드 풀 고갈 또는 이벤트 처리 지연으로 이어질 수 있다.
추가로 LikeEventHandler의 handleLikeAdded(), handleLikeRemoved()와 OrderEventHandler의 handle() 메서드는 @Async 어노테이션만 있고 예외 처리가 없어서 비동기 실행 중 예외가 발생하면 무시된다. OutboxRelayScheduler는 @Scheduled(fixedDelay = 1000)으로 배치 처리하되, Kafka 발행 실패 시 재시도 메커니즘이 없고 기본 scheduler를 사용한다.
수정안은 다음과 같다:
AsyncConfig에ThreadPoolTaskExecutor(corePoolSize, maxPoolSize, queueCapacity, rejectedExecutionHandler 명시)와ThreadPoolTaskScheduler(poolSize, errorHandler 명시)를 빈으로 등록한다.AsyncUncaughtExceptionHandler를 구현하여 비동기 메서드의 예외를 적절히 로깅하고 처리한다.@Async메서드에서 executor를 명시하거나(@Async("executorName")), 최소한 메서드 레벨에서 try-catch로 예외를 처리하여 장애 전파를 방지한다.OutboxRelayScheduler의 발행 실패 이벤트는 재시도 큐에 등록하거나 상태를 별도로 추적하여 나중에 재처리한다.
추가 테스트로 컨텍스트 테스트에서 지정 실행기 빈이 실제로 바인딩되는지 확인하고, 부하 테스트에서 동시 비동기 작업 100개 이상 발생 시 큐 포화, 거부, 예외 처리 동작을 검증해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.java` around
lines 7 - 10, AsyncConfig currently relies on Spring defaults and lacks custom
executors and an uncaught exception handler; add beans for a
ThreadPoolTaskExecutor (set corePoolSize, maxPoolSize, queueCapacity and a
RejectedExecutionHandler) and a ThreadPoolTaskScheduler (set poolSize and an
ErrorHandler) and implement AsyncConfigurer or provide an
AsyncUncaughtExceptionHandler bean to log/handle async exceptions. Update `@Async`
usages: either annotate methods to use the named executor (e.g.,
`@Async`("yourExecutor")) or wrap body of LikeEventHandler.handleLikeAdded,
LikeEventHandler.handleLikeRemoved, and OrderEventHandler.handle in try/catch to
log and surface errors; for OutboxRelayScheduler, switch scheduled execution to
the custom scheduler and add a retry/queueing mechanism for publish failures
(persist failed outbox entries or push to a retry queue/state for later
reprocessing). Also add context/unit tests to assert the custom
executor/scheduler beans are used and add a load test scenario for >100
concurrent async tasks to validate queue/rejection behavior.
| public class Coupon extends BaseEntity { | ||
|
|
||
| @Column(name = "name", nullable = false, length = 100) | ||
| private String name; | ||
|
|
||
| @Column(name = "total_quantity", nullable = false) | ||
| private Integer totalQuantity; | ||
|
|
||
| protected Coupon() {} | ||
|
|
||
| public Coupon(String name, Integer totalQuantity) { | ||
| if (name == null || name.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "쿠폰 이름은 비어있을 수 없습니다."); | ||
| } | ||
| if (totalQuantity == null || totalQuantity <= 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "발급 수량은 0보다 커야 합니다."); | ||
| } | ||
| this.name = name; | ||
| this.totalQuantity = totalQuantity; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# BaseEntity의 equals/hashCode 구현 확인
ast-grep --pattern $'class BaseEntity {
$$$
equals($_) {
$$$
}
$$$
}'
rg -n "equals|hashCode" --type=java -A 5 -B 2 | rg -A 7 "BaseEntity"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# BaseEntity 파일 찾기
find . -name "BaseEntity.java" -type f 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 147
🏁 Script executed:
cat -n ./modules/jpa/src/main/java/com/loopers/domain/BaseEntity.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2291
🏁 Script executed:
# Verify if equals/hashCode might be generated by Lombok elsewhere
rg -n "@EqualsAndHashCode|@Data" ./apps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
Coupon 엔티티에 equals/hashCode 메서드를 구현해야 한다.
BaseEntity는 equals/hashCode를 정의하지 않으므로 기본 객체 동일성(identity comparison)을 사용한다. 이는 JPA 엔티티에서 중대한 문제다. 새로 생성된 쿠폰(id=0L)과 영속화된 쿠폰(생성된 id)은 동일한 데이터를 가지고 있어도 절대 같다고 판정되지 않는다.
이 문제는 다음 상황에서 버그를 유발한다:
Set<Coupon>연산: 중복 삽입Map<Coupon, V>연산: 키 조회 실패- 비즈니스 로직의 엔티티 동등성 검사
해결책: equals/hashCode를 식별자 기반(id)으로 구현하거나, 비즈니스 키(예: name)를 사용한 일관된 전략을 정의한다. 변경 후 Set/Map 컬렉션 및 equals 비교 시나리오에 대한 단위 테스트를 추가한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/java/com/loopers/domain/coupon/Coupon.java` around
lines 14 - 34, The Coupon entity lacks equals/hashCode causing identity-based
comparisons to break collection and business logic; implement equals(Object) and
hashCode() on Coupon using a consistent identifier strategy (preferably the
entity id when non-null, falling back to a business key like name for transient
instances) and ensure they reference the id field from BaseEntity and the name
field in Coupon; update tests to cover Set<Coupon>/Map<Coupon,V> behavior and
equals comparisons between transient (id null/0) and persisted instances.
| public void increaseLikeCount(ZonedDateTime eventTime) { | ||
| if (eventTime.isAfter(this.updatedAt)) { | ||
| this.likeCount++; | ||
| this.updatedAt = eventTime; | ||
| } | ||
| } | ||
|
|
||
| public void decreaseLikeCount(ZonedDateTime eventTime) { | ||
| if (eventTime.isAfter(this.updatedAt)) { | ||
| if (this.likeCount > 0) { | ||
| this.likeCount--; | ||
| } | ||
| this.updatedAt = eventTime; | ||
| } | ||
| } | ||
|
|
||
| public void increaseOrderCount(ZonedDateTime eventTime) { | ||
| if (eventTime.isAfter(this.updatedAt)) { | ||
| this.orderCount++; | ||
| this.updatedAt = eventTime; | ||
| } | ||
| } |
There was a problem hiding this comment.
동시 접근 시 카운터 업데이트 손실(lost update)이 발생할 수 있다.
CatalogEventConsumer가 ProductMetrics를 조회하고 increaseLikeCount() 등을 호출하는 과정에서, 동일 productId에 대한 동시 요청이 있으면 this.likeCount++ 연산이 race condition에 노출된다. 낙관적 잠금(@Version)이나 비관적 잠금 없이는 동시 업데이트 시 일부 증감이 무시된다.
운영 시 좋아요 수가 실제와 불일치하는 문제가 발생할 수 있다.
수정안: 낙관적 잠금 추가
`@Column`(name = "updated_at", nullable = false)
private ZonedDateTime updatedAt;
+
+@Version
+private Long version;또는 @Lock(LockModeType.PESSIMISTIC_WRITE)를 repository 메서드에 적용하는 방안도 고려할 수 있다. PR 설명에 "OptimisticLock 재시도 최대 3회"가 언급되어 있으므로 @Version 적용이 의도된 설계로 보인다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java`
around lines 42 - 63, ProductMetrics' counter methods (increaseLikeCount,
decreaseLikeCount, increaseOrderCount) are vulnerable to lost updates under
concurrent access; add JPA optimistic locking by introducing a version field
annotated with `@Version` in the ProductMetrics entity, and implement a retry
wrapper where the repository/service that invokes these methods (e.g.,
CatalogEventConsumer -> ProductMetrics repository/service) catches
OptimisticLockException and retries the fetch-modify-save cycle up to 3 times
before failing; ensure the retry logic re-reads the entity each attempt so the
increment/decrement applies to the latest state.
| @Lock(LockModeType.PESSIMISTIC_WRITE) | ||
| @Query("SELECT c FROM Coupon c WHERE c.id = :id") | ||
| Optional<Coupon> findByIdWithLock(Long id); |
There was a problem hiding this comment.
findByIdWithLock 메서드에 lock timeout 힌트가 누락되었다.
Pessimistic lock 획득 시 timeout 설정이 없으면 데드락 상황에서 무한 대기가 발생할 수 있다. 이 프로젝트의 다른 WithLock 메서드들은 @QueryHints(@QueryHint(name = "jakarta.persistence.lock.timeout", value = "3000"))를 일관되게 적용하고 있다. 운영 환경에서 lock 경합 시 장애 확산을 방지하려면 반드시 timeout을 설정해야 한다.
🔒️ 수정안
+import org.springframework.data.jpa.repository.QueryHints;
+import jakarta.persistence.QueryHint;
public interface CouponJpaRepository extends JpaRepository<Coupon, Long> {
`@Lock`(LockModeType.PESSIMISTIC_WRITE)
+ `@QueryHints`(`@QueryHint`(name = "jakarta.persistence.lock.timeout", value = "3000"))
`@Query`("SELECT c FROM Coupon c WHERE c.id = :id")
Optional<Coupon> findByIdWithLock(Long id);
}Based on learnings: "In PR #148, ALL WithLock repository methods have QueryHints with jakarta.persistence.lock.timeout applied consistently."
📝 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.
| @Lock(LockModeType.PESSIMISTIC_WRITE) | |
| @Query("SELECT c FROM Coupon c WHERE c.id = :id") | |
| Optional<Coupon> findByIdWithLock(Long id); | |
| `@Lock`(LockModeType.PESSIMISTIC_WRITE) | |
| `@QueryHints`(`@QueryHint`(name = "jakarta.persistence.lock.timeout", value = "3000")) | |
| `@Query`("SELECT c FROM Coupon c WHERE c.id = :id") | |
| Optional<Coupon> findByIdWithLock(Long id); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/coupon/CouponJpaRepository.java`
around lines 13 - 15, The findByIdWithLock repository method is missing a lock
timeout hint; add a QueryHints annotation to the findByIdWithLock method (the
Optional<Coupon> findByIdWithLock(Long id) method) with a QueryHint name
"jakarta.persistence.lock.timeout" and value "3000" so the pessimistic write
lock won’t wait indefinitely during deadlocks; ensure the new `@QueryHints`(...)
is applied alongside `@Lock`(LockModeType.PESSIMISTIC_WRITE) and `@Query`("SELECT c
FROM Coupon c WHERE c.id = :id").
| package com.loopers.infrastructure.metrics; | ||
|
|
||
| import com.loopers.domain.metrics.ProductMetrics; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| public interface ProductMetricsJpaRepository extends JpaRepository<ProductMetrics, Long> {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: ProductMetrics 엔티티의 동시성 제어 메커니즘 확인
ast-grep --pattern $'@Entity
$$$
class ProductMetrics {
$$$
}'Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# ProductMetrics 엔티티 위치 찾기
fd -e java | xargs grep -l "class ProductMetrics"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
#!/bin/bash
# CatalogEventConsumer 위치 찾기
fd -e java | xargs grep -l "CatalogEventConsumer"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 179
🏁 Script executed:
#!/bin/bash
# ProductMetrics 엔티티 내용 확인 (컨텍스트 포함)
rg "class ProductMetrics" -A 30 -B 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3832
🏁 Script executed:
#!/bin/bash
# ProductMetrics 엔티티 전체 내용 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2162
🏁 Script executed:
#!/bin/bash
# CatalogEventConsumer 내용 확인
cat -n apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4051
🏁 Script executed:
#!/bin/bash
# ProductMetrics 업데이트 로직 검색 (서비스 레이어)
rg "productMetricsJpaRepository|ProductMetrics" -B 3 -A 3 --type java | grep -E "(class|interface|@Service|@Transactional|save|update)"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4639
동시성 제어 메커니즘이 부재하여 Lost Update 발생 위험이 있다.
ProductMetrics 엔티티에 @Version 필드가 없고, CatalogEventConsumer의 timestamp 기반 검사(eventTime.isAfter(updatedAt))는 데이터베이스 수준의 동시성 제어가 아니다. 여러 Consumer 인스턴스나 Kafka 파티션이 동시에 같은 productId의 metrics를 업데이트할 때, 다음과 같은 Race Condition이 발생한다:
- Thread A, B가 동일한 ProductMetrics 로드 (likeCount=5, updatedAt=t1)
- Thread A: likeCount 6으로 증가, updatedAt=t2로 저장
- Thread B: 여전히 old updatedAt=t1 기반 비교, likeCount 6으로 증가
- 결과: likeCount=7이 되어야 하지만 6이 됨 (Lost Update)
수정안: ProductMetrics에 @Version 필드를 추가하거나, Repository에서 findById() 시 PESSIMISTIC_WRITE 락을 적용한다.
수정 예시
`@Entity`
`@Table`(name = "product_metrics")
`@Getter`
public class ProductMetrics {
// ...
`@Version`
private Long version;
// ...
}또는 Repository에 custom 메서드 추가:
`@Lock`(LockModeType.PESSIMISTIC_WRITE)
`@Query`("SELECT p FROM ProductMetrics p WHERE p.productId = :productId")
Optional<ProductMetrics> findByIdWithLock(`@Param`("productId") Long productId);추가로 높은 동시성 환경에서는 UPDATE product_metrics SET like_count = like_count + 1 WHERE product_id = ? 형태의 벌크 업데이트도 고려해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/infrastructure/metrics/ProductMetricsJpaRepository.java`
around lines 1 - 6, The ProductMetrics updates are vulnerable to lost-updates
because there is no DB-level concurrency control; add an optimistic lock field
(annotate a Long version field with `@Version` in the ProductMetrics entity) or
add a pessimistic-locking finder in ProductMetricsJpaRepository (e.g. declare
findByIdWithLock annotated with `@Lock`(LockModeType.PESSIMISTIC_WRITE) and use
that from CatalogEventConsumer instead of the plain findById), and alternatively
consider performing an atomic bulk update (UPDATE ... SET like_count =
like_count + 1 WHERE product_id = ?) from the consumer to avoid race conditions.
| @Transactional | ||
| public void consume(List<ConsumerRecord<Object, Object>> records, Acknowledgment acknowledgment) { | ||
| for (ConsumerRecord<Object, Object> record : records) { | ||
| try { | ||
| processRecord(record); | ||
| } catch (Exception e) { | ||
| log.error("[CatalogConsumer] 처리 실패: topic={}, partition={}, offset={}", | ||
| record.topic(), record.partition(), record.offset(), e); | ||
| } | ||
| } | ||
| acknowledgment.acknowledge(); | ||
| } |
There was a problem hiding this comment.
배치 전체를 단일 트랜잭션으로 처리하면 부분 롤백 문제가 발생한다.
@Transactional이 consume() 메서드 전체를 감싸므로, 한 레코드에서 RuntimeException(예: DB constraint violation)이 발생하면 try-catch를 벗어나 전체 트랜잭션이 롤백된다. 이 경우 이미 성공적으로 처리된 다른 레코드의 EventHandled 저장도 함께 롤백되어, 다음 폴링에서 중복 처리된다.
PR 설명의 "멱등성" 보장이 운영 환경에서 깨질 수 있다.
수정안: 레코드별 독립 트랜잭션 적용
별도 서비스 클래스에 @Transactional(propagation = REQUIRES_NEW) 메서드를 만들거나, TransactionTemplate을 사용하여 레코드별 트랜잭션 격리를 보장해야 한다.
// 별도 서비스에서 호출
`@Transactional`(propagation = Propagation.REQUIRES_NEW)
public void processRecordInNewTx(ConsumerRecord<Object, Object> record) throws Exception {
// 처리 로직
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CatalogEventConsumer.java`
around lines 36 - 47, The consume() method is currently annotated with
`@Transactional` causing the entire batch to rollback on a single record failure;
remove `@Transactional` from CatalogEventConsumer.consume and instead invoke a new
per-record transactional boundary (e.g., create a new service method like
processRecordInNewTx(ConsumerRecord<Object,Object>) annotated with
`@Transactional`(propagation = Propagation.REQUIRES_NEW) or execute per-record
logic via TransactionTemplate). Ensure consume() catches exceptions from
processRecord/processRecordInNewTx, logs the error (as already done), and
continues processing other records so successful EventHandled persists
independently of failed records.
| // 수량 확인 | ||
| long issuedCount = issuedCouponRepository.countByCouponId(couponId); | ||
| if (issuedCount >= coupon.getTotalQuantity()) { | ||
| log.info("[CouponConsumer] 쿠폰 소진: couponId={}, issuedCount={}, totalQuantity={}", | ||
| couponId, issuedCount, coupon.getTotalQuantity()); | ||
| request.markFailed(); | ||
| eventHandledRepository.save(new EventHandled(eventId)); | ||
| return; | ||
| } | ||
|
|
||
| // 발급 처리 | ||
| issuedCouponRepository.save(new IssuedCoupon(couponId, userId)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# IssuedCoupon 테이블의 unique constraint 또는 Redis 사용 여부 확인
rg -n "unique|UNIQUE" --type java -g "*IssuedCoupon*"
rg -n "RedisTemplate|StringRedisTemplate" --type java -g "*CouponIssue*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 250
🏁 Script executed:
#!/bin/bash
# 1. CouponIssueConsumer.java의 전체 consume() 메서드 확인
echo "=== CouponIssueConsumer consume() method ==="
rg -A 50 "public void consume" apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java | head -80
# 2. IssuedCoupon.java 전체 확인
echo -e "\n=== IssuedCoupon entity ==="
cat apps/commerce-streamer/src/main/java/com/loopers/domain/coupon/IssuedCoupon.java
# 3. DataIntegrityViolationException 핸들링 확인
echo -e "\n=== Exception handling in commerce-streamer ==="
rg -n "DataIntegrityViolationException|UniqueConstraint" apps/commerce-streamer/src/main/java/com/loopers/
# 4. Redis 사용 여부 확인
echo -e "\n=== Redis usage in commerce-streamer ==="
rg -n "redis|Redis|INCR" apps/commerce-streamer/src/main/java/com/loopers/ --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3884
쿠폰 초과 발급 위험: 총 수량 확인과 저장 사이 TOCTOU 경쟁 조건 존재
countByCouponId()로 현재 발급 수를 조회(라인 85)한 후 IssuedCoupon 저장(라인 94) 사이에 다른 트랜잭션이 동일 쿠폰을 발급할 수 있다. IssuedCoupon 테이블의 Unique Constraint는 (coupon_id, user_id)로 정의되어 있어 동일 사용자의 중복 발급만 방지하며, 서로 다른 사용자의 동시 발급으로 인한 총 수량 초과는 막지 못한다. 또한 findByIdWithLock(couponId)로 획득한 Coupon 행 락은 IssuedCoupon 테이블의 INSERT를 제어하지 않는다.
운영 영향: 동시 요청이 많은 인기 쿠폰의 경우, 총 수량 한도를 초과하여 발급될 수 있으며, 이는 정산 오류 및 고객 불만으로 이어진다.
필수 수정 사항:
방안 1: IssuedCoupon 저장 시 unique constraint 위반 예외 처리 추가
try {
issuedCouponRepository.save(new IssuedCoupon(couponId, userId));
} catch (DataIntegrityViolationException e) {
log.warn("[CouponConsumer] 쿠폰 발급 실패 (중복 또는 초과): couponId={}, userId={}", couponId, userId, e);
request.markFailed();
eventHandledRepository.save(new EventHandled(eventId));
return;
}방안 2: Redis INCR을 이용한 원자적 수량 확인 (권장)
Long currentCount = redisTemplate.opsForValue().increment("coupon:" + couponId + ":issued");
if (currentCount > coupon.getTotalQuantity()) {
redisTemplate.opsForValue().decrement("coupon:" + couponId + ":issued");
log.info("[CouponConsumer] 쿠폰 소진: couponId={}, totalQuantity={}", couponId, coupon.getTotalQuantity());
request.markFailed();
eventHandledRepository.save(new EventHandled(eventId));
return;
}
// DB 저장 진행
issuedCouponRepository.save(new IssuedCoupon(couponId, userId));추가 테스트 요구: 동시 발급 시나리오(예: totalQuantity=10, 동시 요청 20개)에서 정확히 10개만 발급되는지 검증하는 동시성 테스트 추가 필요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-streamer/src/main/java/com/loopers/interfaces/consumer/CouponIssueConsumer.java`
around lines 85 - 96, The code in CouponIssueConsumer uses
issuedCouponRepository.countByCouponId(...) then
issuedCouponRepository.save(...) causing a TOCTOU race that can exceed
coupon.getTotalQuantity(); fix by making the save operation resilient to
concurrent inserts: either wrap issuedCouponRepository.save(new
IssuedCoupon(couponId, userId)) in a try/catch that catches
DataIntegrityViolationException (or the JPA/sql unique/constraint exception) and
on catch log the failure and call request.markFailed() and
eventHandledRepository.save(new EventHandled(eventId)), or implement an atomic
counter (e.g., Redis INCR on key "coupon:{couponId}:issued") before DB insert
and decrement on overflow as an alternative; update the logic around
countByCouponId, findByIdWithLock, issuedCouponRepository.save,
request.markFailed, and eventHandledRepository.save accordingly and add
concurrency tests to validate totalQuantity enforcement.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java (3)
15-15: 미사용 import 제거 필요
ProductCacheService가 import되어 있으나 코드에서 사용되지 않는다. 이전 버전에서 캐시 무효화 로직이 제거되면서 남은 것으로 보인다. 미사용 import는 빌드 시간에는 영향 없으나, 코드 가독성과 의존성 추적을 방해한다.🧹 미사용 import 제거
-import com.loopers.infrastructure.product.ProductCacheService;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java` at line 15, Remove the unused import of ProductCacheService from LikeFacade.java: delete the line "import com.loopers.infrastructure.product.ProductCacheService;" and confirm there are no references to ProductCacheService in the LikeFacade class (e.g., constructor, fields, or methods) before committing; run a quick build or static-import check to ensure no remaining usages.
54-54: 반환값 무시에 대한 명시적 의도 표현 권장
productService.getProduct(productId)호출 시 반환값을 무시하고 있다. 상품 존재 여부 검증 목적임을 알 수 있으나, 코드만 봐서는 의도가 불명확하다. 장애 상황에서 디버깅 시 혼란을 줄 수 있다.💡 의도를 명시하는 방법
- productService.getProduct(productId); + productService.getProduct(productId); // 상품 존재 검증또는 검증 전용 메서드를 사용하는 것이 더 명확하다:
productService.validateProductExists(productId);Also applies to: 68-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java` at line 54, The call productService.getProduct(productId) is currently ignoring its return value which obscures intent; change this to an explicit existence check or a clearer helper such as productService.validateProductExists(productId) (or assign the result to a variable and assert/throw if null) so readers and debuggers know the call is for validation; update both occurrences (the call at productService.getProduct(productId) and the similar call around line 68) to use the validation method or an explicit null-check with a clear exception/log message.
4-4: 미사용 import 제거 필요:ProductCacheServiceline 15의
ProductCacheServiceimport는 코드에서 사용되지 않는다. 불필요한 의존성을 제거하여 클래스 복잡도를 낮춘다.패키지명 일관성:
confg→config리팩토링 권장
com.loopers.confg.kafka패키지명이 다른 모듈의com.loopers.config규칙과 불일치한다. kafka 모듈의 패키지명을com.loopers.config.kafka로 통일하면 향후 유지보수 시 패키지명 오타로 인한 혼동을 방지할 수 있다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java` at line 4, Remove the unused import ProductCacheService from LikeFacade and any other classes where it's not referenced to eliminate dead dependency; then rename the misspelled package com.loopers.confg.kafka to com.loopers.config.kafka in the kafka module and update all imports (e.g., references to KafkaTopics) and package declarations (including the import line in LikeFacade referencing com.loopers.confg.kafka.KafkaTopics) to the corrected com.loopers.config.kafka so package names are consistent across modules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java`:
- Around line 56-62: Add robust async error handling and drift detection:
implement an AsyncUncaughtExceptionHandler in AsyncConfig to log and emit
monitoring metrics for uncaught exceptions from `@Async` methods; wrap
LikeEventHandler.handleLikeAdded and handleLikeRemoved bodies in try-catch that
logs both system and user-facing messages and emits failure metrics when
productService.increaseLikes()/decreaseLikes() or other processing fails; ensure
the caller sequence that publishes outboxEventPublisher.publish(...) and
eventPublisher.publishEvent(new LikeAddedEvent(...)) remains unchanged but
consider emitting a correlated trace/id on both events for debugging; and add a
nightly drift detection job that compares Product.likesCount and
ProductMetrics.likeCount, emits alerts when they diverge, and provides a manual
reconciliation flag in the job for operators.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java`:
- Around line 67-93: Test only verifies eventPublisher.publishEvent and misses
verifying outbox publishing; add assertions to ensure
outboxEventPublisher.publish(...) is called once with an OutboxEvent whose
type/key/payload correspond to the OrderCreatedEvent produced by
orderFacade.createOrder, and keep verifying
eventPublisher.publishEvent(OrderCreatedEvent). Update the
publishesOrderCreatedEvent_whenOrderCreated test to verify
outboxEventPublisher.publish was invoked (use argument capture or matchers to
assert eventType, key and payload contents), and add a separate test that
simulates outboxEventPublisher.publish throwing (when createOrder is called) to
assert the expected rollback/exception propagation behavior per your policy.
Ensure you reference orderFacade.createOrder, eventPublisher.publishEvent,
outboxEventPublisher.publish, OrderCreatedEvent and OutboxEvent when locating
the code to change.
---
Nitpick comments:
In
`@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java`:
- Line 15: Remove the unused import of ProductCacheService from LikeFacade.java:
delete the line "import com.loopers.infrastructure.product.ProductCacheService;"
and confirm there are no references to ProductCacheService in the LikeFacade
class (e.g., constructor, fields, or methods) before committing; run a quick
build or static-import check to ensure no remaining usages.
- Line 54: The call productService.getProduct(productId) is currently ignoring
its return value which obscures intent; change this to an explicit existence
check or a clearer helper such as
productService.validateProductExists(productId) (or assign the result to a
variable and assert/throw if null) so readers and debuggers know the call is for
validation; update both occurrences (the call at
productService.getProduct(productId) and the similar call around line 68) to use
the validation method or an explicit null-check with a clear exception/log
message.
- Line 4: Remove the unused import ProductCacheService from LikeFacade and any
other classes where it's not referenced to eliminate dead dependency; then
rename the misspelled package com.loopers.confg.kafka to
com.loopers.config.kafka in the kafka module and update all imports (e.g.,
references to KafkaTopics) and package declarations (including the import line
in LikeFacade referencing com.loopers.confg.kafka.KafkaTopics) to the corrected
com.loopers.config.kafka so package names are consistent across modules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9dc6212-46da-4ae8-b706-f527a61be706
📒 Files selected for processing (5)
apps/commerce-api/build.gradle.ktsapps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java
✅ Files skipped from review due to trivial changes (2)
- apps/commerce-api/build.gradle.kts
- apps/commerce-api/src/main/resources/application.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/commerce-api/src/main/java/com/loopers/application/order/OrderFacade.java
| outboxEventPublisher.publish( | ||
| KafkaTopics.CATALOG_EVENTS, | ||
| productId.toString(), | ||
| "LIKE_ADDED", | ||
| Map.of("productId", productId) | ||
| ); | ||
| eventPublisher.publishEvent(new LikeAddedEvent(user.getId(), productId)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: LikeEventHandler의 에러 처리 및 재시도 로직 확인
echo "=== LikeEventHandler 에러 처리 확인 ==="
ast-grep --pattern $'@Async
public void $METHOD($$$) {
$$$
}'
echo ""
echo "=== `@Async` 메서드의 예외 처리 설정 확인 ==="
rg -n "AsyncUncaughtExceptionHandler|@Async" --type java -C 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2961
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/application/event/LikeEventHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1559
🏁 Script executed:
rg -n "AsyncUncaughtExceptionHandler|ListeningExecutorService|TaskExecutor" --type java -A 5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg -n "increaseLikes|decreaseLikes" --type java -A 10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6136
🏁 Script executed:
rg -n "EnableAsync|AsyncConfigurer|Executor" --type java -A 10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 17569
🏁 Script executed:
head -100 apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2527
🏁 Script executed:
rg -n "@Transactional|@Retry|@Recover" apps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.java -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1633
🏁 Script executed:
rg -n "reconcile|drift|monitor|metric" --type java -i -lRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 534
🏁 Script executed:
cat -n apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1721
🏁 Script executed:
rg -n "likeCount|likesCount" --type java apps/commerce-streamer/src/main/java/com/loopers/domain/metrics/ProductMetrics.java -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 888
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/config/AsyncConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 448
@async 이벤트 실패 시 에러 처리 및 모니터링 메커니즘 필요
Outbox 이벤트(Kafka)와 Spring 이벤트를 이중 발행하는 구조다. 두 시스템이 서로 다른 DB 필드를 업데이트하므로 중복 문제는 없으나, 운영 관점에서 다음과 같은 위험이 있다.
문제점:
LikeEventHandler의@Async메서드(handleLikeAdded,handleLikeRemoved)에 에러 처리가 없다. 예외 발생 시 호출자에게 전파되지 않으며, 전역AsyncUncaughtExceptionHandler도 설정되어 있지 않아 실패가 침묵한다.productService.increaseLikes()또는decreaseLikes()실패 시Product.likesCount가 업데이트되지 않으면서ProductMetrics.likeCount와 정합성이 깨진다.- 두 카운트 간 drift를 감지하고 알림하는 메트릭 또는 배치가 보이지 않는다.
수정 방안:
AsyncConfig에AsyncUncaughtExceptionHandler구현하여 async 실패를 로깅 및 모니터링한다.LikeEventHandler메서드 내에 try-catch를 추가하여 실패 사유를 명확히 기록한다(로그: 시스템 메시지, 사용자 메시지 분리).- 야간 배치로
Product.likesCount와ProductMetrics.likeCount의 drift를 감지하고, 불일치 시 알람을 발생시킨다. - 필요 시 수동 재조정 프로세스를 마련한다.
테스트:
LikeEventHandler에서productService.increaseLikes()실패 시나리오를 통합 테스트에 포함한다.- 두 카운트 간 차이 발생 시 drift 감지 로직을 검증한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/java/com/loopers/application/like/LikeFacade.java`
around lines 56 - 62, Add robust async error handling and drift detection:
implement an AsyncUncaughtExceptionHandler in AsyncConfig to log and emit
monitoring metrics for uncaught exceptions from `@Async` methods; wrap
LikeEventHandler.handleLikeAdded and handleLikeRemoved bodies in try-catch that
logs both system and user-facing messages and emits failure metrics when
productService.increaseLikes()/decreaseLikes() or other processing fails; ensure
the caller sequence that publishes outboxEventPublisher.publish(...) and
eventPublisher.publishEvent(new LikeAddedEvent(...)) remains unchanged but
consider emitting a correlated trace/id on both events for debugging; and add a
nightly drift detection job that compares Product.likesCount and
ProductMetrics.likeCount, emits alerts when they diverge, and provides a manual
reconciliation flag in the job for operators.
| @DisplayName("주문 생성 시 OrderCreatedEvent가 발행된다.") | ||
| @Test | ||
| void publishesOrderCreatedEvent_whenOrderCreated() { | ||
| // Arrange | ||
| String loginId = "testuser"; | ||
| String rawPassword = "Test1234!"; | ||
| User user = new User(loginId, "encrypted", "홍길동", "19900101", "test@example.com"); | ||
| user.restoreBalance(100000L); | ||
| Product product = new Product(0L, "테스트상품", 10000, 100, "설명", "img.jpg"); | ||
| Brand brand = new Brand("테스트브랜드", "설명"); | ||
| Order order = new Order(user.getId(), "ORD-20240101-TEST", 10000L); | ||
| List<OrderRequest.OrderItemRequest> items = List.of(new OrderRequest.OrderItemRequest(1L, 1)); | ||
|
|
||
| given(userService.authenticate(loginId, rawPassword)).willReturn(user); | ||
| given(productService.getProduct(1L)).willReturn(product); | ||
| given(brandService.getBrandsByIds(any())).willReturn(List.of(brand)); | ||
| given(orderService.generateOrderNumber()).willReturn("ORD-20240101-TEST"); | ||
| given(orderService.createOrder(any(), any(), any())).willReturn(order); | ||
| given(orderService.createOrderItem(any(), any(), any(), any(), any(), any(), any())).willReturn(null); | ||
| given(orderService.getOrderItems(any())).willReturn(List.of()); | ||
|
|
||
| // Act | ||
| orderFacade.createOrder(loginId, rawPassword, items); | ||
|
|
||
| // Assert | ||
| verify(eventPublisher).publishEvent(any(OrderCreatedEvent.class)); | ||
| } |
There was a problem hiding this comment.
아웃박스 발행 경로 검증이 빠져 있어 운영 회귀를 놓칠 수 있다
현재 검증은 eventPublisher.publishEvent(...)만 확인하므로, outboxEventPublisher.publish(...)가 누락/오동작해도 테스트가 통과한다. 운영에서는 이 회귀가 발생하면 주문 후속 처리(스트리머 소비)가 조용히 끊겨 장애 탐지가 늦어진다.
수정안으로 아웃박스 발행 호출 자체와 핵심 인자(최소 1회 호출, 이벤트 타입/키/페이로드)를 함께 검증해야 한다.
추가 테스트로는 정상 발행 1건 검증 외에, 아웃박스 발행 예외 시 기대 동작(롤백/예외 전파 정책)을 명시적으로 검증하는 실패 케이스를 포함해야 한다.
수정 예시 diff
@@
// Assert
+ verify(outboxEventPublisher).publish(any(), any(), any(), any());
verify(eventPublisher).publishEvent(any(OrderCreatedEvent.class));As per coding guidelines **/*Test*.java: "단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/test/java/com/loopers/application/order/OrderFacadeTest.java`
around lines 67 - 93, Test only verifies eventPublisher.publishEvent and misses
verifying outbox publishing; add assertions to ensure
outboxEventPublisher.publish(...) is called once with an OutboxEvent whose
type/key/payload correspond to the OrderCreatedEvent produced by
orderFacade.createOrder, and keep verifying
eventPublisher.publishEvent(OrderCreatedEvent). Update the
publishesOrderCreatedEvent_whenOrderCreated test to verify
outboxEventPublisher.publish was invoked (use argument capture or matchers to
assert eventType, key and payload contents), and add a separate test that
simulates outboxEventPublisher.publish throwing (when createOrder is called) to
assert the expected rollback/exception propagation behavior per your policy.
Ensure you reference orderFacade.createOrder, eventPublisher.publishEvent,
outboxEventPublisher.publish, OrderCreatedEvent and OutboxEvent when locating
the code to change.
1 participant
📌 Summary
🧭 Context & Decision
왜 Transactional Outbox인가
비즈니스 데이터 변경과 Kafka 발행의 원자성이 핵심 과제였습니다.
kafkaTemplate.send()를 트랜잭션 내에서 직접 호출하면 DB 롤백 시 이미 발행된 메시지를 회수할 수 없고, 트랜잭션 밖에서 호출하면 커밋 성공 후 발행 실패로 메시지가유실됩니다.
Outbox 패턴은 이벤트를 비즈니스 데이터와 같은 트랜잭션에
outbox_events테이블로 저장하고, 별도 스케줄러가 polling하여 Kafka로 발행합니다. DB 커밋이 곧 발행 보장이 되므로, at-least-once 시맨틱이 자연스럽게 달성됩니다.선택지와 결정
Outbox 발행 방식: 전체 배치 vs 개별 트랜잭션
초기 구현에서는 배치 단위로 조회 → 순차 전송 → 성공한 것만 모아서 한 번에 saveAll하는 방식이었습니다. 이 방식은 한 건의 발행 실패가 나머지 성공 건의 상태 반영을 지연시키고, 실패한 이벤트를 재시도와 최종 실패로 분류할 수 없는
문제가 있었습니다.
최종적으로 이벤트마다 독립 트랜잭션(
FOR UPDATE SKIP LOCKED)으로 변경했습니다. 각 이벤트를 비관적 락으로 잠근 뒤 Kafka 전송 → 성공 시markPublished()/ 실패 시incrementRetryCount()→ 커밋하는 구조입니다. 5회 초과실패 시
FAILED로 마킹하여 수동 개입 대상을 명확히 식별할 수 있습니다.Consumer 오프셋: auto commit vs manual ack
enable.auto.commit=false+AckMode.MANUAL을 선택했습니다. 자동 커밋은 처리 완료 전에 오프셋이 커밋되어 장애 시 메시지가 유실될 수 있기 때문입니다. 수동 커밋은 비즈니스 로직 + DLQ 전송까지 완료된 후에만 ack하므로, 처리 실패시 재배달이 보장됩니다.
선착순 쿠폰: DB 락 vs Redis INCR gate
DB 비관적 락은 Kafka Consumer가 단일 스레드(concurrency=1)로 순차 처리하므로 불필요한 오버헤드입니다. Redis INCR은 원자적 연산으로 DB 트랜잭션 밖에서 수량을 빠르게 체크하고, 초과 시 즉시 차단합니다. DB 커밋 실패 시 DECR로
보상하며, Redis는 gate 역할이고 DB가 source of truth입니다.
🏗️Design Overview
변경 범위
주요 컴포넌트 책임
Producer 측 (commerce-api)
OutboxEventListener@TransactionalEventListener(BEFORE_COMMIT)로 도메인 이벤트를 수신하여outbox_events에 저장합니다. 비즈니스 데이터와 같은 트랜잭션에서 실행되어 원자성을 보장합니다.OutboxPublisherFOR UPDATE SKIP LOCKED로 다중 인스턴스 환경에서 중복 발행을 방지하고, 개별 실패 처리 + 연속 실패 시 조기 종료(circuit breaker)를 적용했습니다.|
|
ProductViewedEventPublisher| 조회 이벤트는 유실 허용 가능하므로 Outbox를 거치지 않고 Kafka에 직접 발행합니다. 고빈도 조회마다 Outbox INSERT를 방지합니다. |Consumer 측 (commerce-streamer)
CatalogEventConsumercatalog-events토픽을 batch(500건, concurrency=3)로 소비합니다. LIKED/UNLIKED/PRODUCT_VIEWED 이벤트를 수신하여ProductMetrics의 좋아요 수/조회수를 갱신합니다.OrderEventConsumerorder-events토픽을 batch로 소비합니다. PAYMENT_COMPLETED 이벤트 수신 시ProductMetrics의 판매수를 갱신합니다.CouponIssueConsumercoupon-issue-requests토픽을 single(concurrency=1)로 순차 소비합니다. 순서 보장이 필요한 선착순 쿠폰 발급을 처리합니다.FcfsCouponIssueServiceMetricsApplicationServiceProductMetrics갱신을 담당합니다. OptimisticLock 충돌 시 최대 3회 재시도하며,EventHandled저장과 비즈니스 로직을 같은 트랜잭션에서 수행하여 멱등성을 보장합니다.🔁 Flow Diagram
Transactional Outbox 발행 흐름
sequenceDiagram participant App as ApplicationService participant DB participant OutboxListener as OutboxEventListener participant Publisher as OutboxPublisher participant Kafka App->>DB: 비즈니스 데이터 저장 App->>App: eventPublisher.publishEvent() OutboxListener->>DB: outbox_events INSERT (같은 TX) Note over DB: TX 커밋 → 비즈니스 + Outbox 원자적 저장 loop 1초 주기 polling Publisher->>DB: findPendingIds (LIMIT 100) Publisher->>DB: findByIdForUpdate (SKIP LOCKED) Publisher->>Kafka: send(topic, key, payload) alt 성공 Publisher->>DB: markPublished() else 실패 Publisher->>DB: incrementRetryCount() Note over Publisher: 5회 초과 시 markFailed() → 수동 확인 필요 end end선착순 쿠폰 발급 흐름
sequenceDiagram participant API as CouponController participant DB as commerce-api DB participant Kafka participant Consumer as CouponIssueConsumer participant Redis participant StreamerDB as commerce-streamer DB API->>DB: CouponIssueRequest(PENDING) + Outbox 저장 (같은 TX) API-->>API: 202 Accepted 반환 Note over Kafka: OutboxPublisher polling → 발행 Kafka->>Consumer: COUPON_ISSUE_REQUESTED 수신 Consumer->>Redis: INCR coupon:fcfs:{couponId}:count alt count <= maxQuantity Consumer->>StreamerDB: TX { 쿠폰 발급 + issuedCount++ + request.markSuccess() + EventHandled } alt DB 실패 Consumer->>Redis: DECR (보상) Consumer->>Consumer: DLQ 전송 end else count > maxQuantity Consumer->>Redis: DECR (보상) Consumer->>StreamerDB: request.markFailed("선착순 마감") endConsumer 에러 처리 흐름 (Batch)
flowchart TD A[poll 500건] --> B{레코드 순회} B -->|성공| C[비즈니스 처리 + EventHandled 저장] B -->|실패| D[DLQ 동기 전송 5s timeout] D -->|DLQ 성공| B D -->|DLQ 실패| E[RuntimeException throw] C --> B B -->|순회 완료| F[acknowledgment.acknowledge] E --> G[ack 미호출 → 전체 배치 재배달]🛡️ 신뢰성 보장 전략
Producer 신뢰성
Consumer 신뢰성
재처리 안전성
오프셋 되감기를 통한 재처리 시, EventHandled 테이블에 이미 처리된 eventId가 남아 있으므로 중복 처리가 방지됩니다. 단, 잘못된 처리를 보정하기 위한 재처리에서는 EventHandled 레코드를 먼저 삭제해야 skip 없이 재처리됩니다.
테스트/검증
변경 목적
동기 처리에 묶인 도메인 이벤트(좋아요/주문 등)의 비동기화와 선착순 쿠폰 발급의 동시성 제어를 위해 Transactional Outbox 패턴 기반 Kafka 이벤트 파이프라인을 도입했습니다.
핵심 변경점
Producer(commerce-api): Outbox 엔티티, 1초 주기 폴링 스케줄러(OutboxRelayScheduler), OutboxEventPublisher를 추가하여 비즈니스 트랜잭션과 이벤트 저장의 원자성을 확보. 좋아요/주문 이벤트는 Outbox 저장 후 Kafka 발행 및 Spring 비동기 리스너 호출.
Consumer(commerce-streamer): Catalog/Coupon 이벤트 컨슈머, EventHandled 테이블 기반 멱등성 처리, 선착순 쿠폰은 DB 비관적 락(PESSIMISTIC_WRITE)으로 동시성 제어.
신뢰성 설정: Kafka Producer는
acks=all,enable.idempotence=true,retries=3; Consumer는 수동 오프셋 커밋(enable.auto.commit=false,ack-mode: manual).주의사항 및 확인 필요 항목
markSent()를 Kafka 비동기 콜백 내에서 호출하지 않고 즉시 실행하므로, 실제 Kafka 전송 실패 시 메시지 유실 가능성 있음테스트/검증 방법
동시성 테스트(CouponConcurrencyTest, CouponIssueConsumerConcurrencyTest)로 쿠폰 발급 제한을 검증했으나, Outbox 메시지 유실 시나리오와 Kafka 재전송 실패 시 보상 처리에 대한 테스트가 부재합니다. Outbox 폴링 방식의 메시지 순서 보장 및 중복 처리에 대한 검증 필요.