[REFACTOR] 일괄 주문 API 재고 처리 로직 개선 #41
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageHandler.kt (1)
27-30: 멀티 인스턴스 동시 처리 대비 예약(lock) 없어서 중복 처리 위험
findPendingAndRetryableMessages가 SKIP LOCKED/PROCESSING 상태 전이를 사용하지 않으면 다중 노드에서 중복 처리됩니다.권장:
- 선택 쿼리:
FOR UPDATE SKIP LOCKED(Spring Data + 네이티브) 또는- 상태 전이:
INIT/SEND_FAIL → PROCESSING을 CAS 업데이트 후 처리.예시(리포지토리 외부 파일 제안):
@Query( value = """ SELECT * FROM outbox_messages WHERE status IN ('INIT','SEND_FAIL') ORDER BY id ASC LIMIT :limit FOR UPDATE SKIP LOCKED """, nativeQuery = true ) fun lockNextBatch(@Param("limit") limit: Int): List<OutboxMessage>
🧹 Nitpick comments (20)
gradle/spring.gradle (1)
14-14: 운영 가시성 강화: Kafka 메트릭 노출 준비DLQ/재처리 관점에서 컨슈머 랙·재시도/실패 메트릭 관찰이 중요합니다. Actuator 및 Prometheus 레지스트리 추가를 고려해 주세요.
dependencies { ... implementation "org.springframework.kafka:spring-kafka" + implementation "org.springframework.boot:spring-boot-starter-actuator" + runtimeOnly "io.micrometer:micrometer-registry-prometheus" }추가 후
/actuator/metrics,/actuator/prometheus로 Kafka 관련 메트릭 노출 여부를 확인해 주세요.src/main/kotlin/com/dh/baro/core/exception/ConflictException.kt (1)
3-3: open 처리 적절. cause 보존 생성자 추가 제안추후 원인 추적을 위해
cause포함 생성자를 함께 제공하면 좋습니다.적용 예:
-open class ConflictException(message: String) : RuntimeException(message) +open class ConflictException : RuntimeException { + constructor(message: String) : super(message) + constructor(message: String, cause: Throwable) : super(message, cause) +}src/main/kotlin/com/dh/baro/core/dlt/FailedMessage.kt (2)
7-9: 조회 패턴 대비 인덱스 추가 제안운영 분석/리스팅을 고려하면
(topic, consumer_group, created_at)조합 또는created_at단독 인덱스가 유용합니다.적용 예:
-@Table(name = "failed_messages") +@Table( + name = "failed_messages", + indexes = [ + Index(name = "idx_failed_msg_created_at", columnList = "created_at"), + Index(name = "idx_failed_msg_topic_group_created_at", columnList = "topic, consumer_group, created_at") + ] +)
20-24: DB 이식성: TEXT 지정 대신 @lob 권장
columnDefinition = "TEXT"는 DB 벤더 종속적입니다.@Lob로 교체하면 이식성이 좋아집니다.errorMessage도 동일.-@Column(name = "payload", nullable = false, columnDefinition = "TEXT") -val payload: String, +@Lob +@Column(name = "payload", nullable = false) +val payload: String, -@Column(name = "error_message", nullable = false, columnDefinition = "TEXT") -val errorMessage: String, +@Lob +@Column(name = "error_message", nullable = false) +val errorMessage: String,src/main/kotlin/com/dh/baro/core/dlt/FailedMessageRepository.kt (1)
5-5: 단순 CRUD 인터페이스 적절운영성 보완을 위해 보관 기간 만료 일괄 삭제 메서드 추가를 고려해 주세요(예:
deleteByCreatedAtBefore).src/main/kotlin/com/dh/baro/order/domain/Order.kt (1)
65-75: 취소 사유(reason) 파라미터가 사용되지 않습니다API/이벤트에서 취소 사유를 요구한다면 현재 엔티티 메서드가 이를 무시합니다. 사유를 저장하지 않을 계획이면 파라미터를 제거하거나, 사유 저장(필드/이벤트 발행)에 반영하세요.
가능한 최소 수정 예시:
- fun cancel(reason: String) { + fun cancel() { if (status == OrderStatus.CANCELLED) { throw IllegalStateException(ErrorMessage.ORDER_ALREADY_CANCELLED.format(id)) } if (status in listOf(OrderStatus.SHIPPED, OrderStatus.DELIVERED)) { throw IllegalStateException(ErrorMessage.ORDER_CANCEL_INVALID_STATUS.format(status, id)) } status = OrderStatus.CANCELLED }src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt (2)
40-41: 재고 부족은 error 보다는 warn/info가 적절운영 알람 노이즈를 줄이기 위해 도메인상 기대 가능한 경로(재고 부족)는 warn으로, 시스템 오류만 error로 로깅하세요. 위 패치에 반영했습니다.
25-27: 토픽/그룹 ID 하드코딩 — 구성 또는 중앙상수 사용 권장KafkaConfig.kt에 ORDER_EVENTS_TOPIC / INVENTORY_EVENTS_TOPIC 상수가 이미 정의되어 있습니다. 리스너가 리터럴을 직접 사용하므로 운영에서 토픽/그룹 변경 시 재빌드가 필요합니다.
단기(간단): KafkaConfig 상수 재사용 — 파일: src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt
예: @KafkaListener(topics = [KafkaConfig.ORDER_EVENTS_TOPIC], groupId = "product-service")권장(운영 편의): 토픽·그룹을 프로퍼티로 외부화하고 application-*.yaml에 키 추가
예: @KafkaListener(topics = ["${kafka.topics.order-events}"], groupId = "${kafka.consumer.group-id:product-service}")나머지 리스너들에도 동일 정책 적용 필요.
src/main/kotlin/com/dh/baro/product/infra/redis/InventoryRedisRepository.kt (1)
33-42: Lambda 파라미터 이름을 더 명확하게 변경하는 것이 좋겠습니다
resultHandler라는 이름은 다소 일반적입니다. 재고 부족 여부를 판단하는 로직임을 명확히 하기 위해isSuccessful또는validateResult같은 이름이 더 적절할 것 같습니다.return executeScript( script = batchStockDeductionScript, items = mergedItems, - autoInitializeFromDb = true -) { result -> + autoInitializeFromDb = true, + validateResult = { result -> when (result) { INSUFFICIENT_STOCK -> throw IllegalArgumentException(ErrorMessage.INSUFFICIENT_STOCK.message) else -> result >= 0 } -} +})src/main/kotlin/com/dh/baro/core/dlt/DltHandler.kt (2)
39-39: 하드코딩된 재시도 횟수를 설정 가능하게 만들어야 합니다
retryCount = 3이 하드코딩되어 있는데, 이는 환경별로 다르게 설정해야 할 수 있습니다. 설정 파일이나 환경 변수를 통해 구성 가능하도록 개선이 필요합니다.+@Value("\${kafka.dlt.retry-count:3}") +private val defaultRetryCount: Int = 3 + @DltHandler @Transactional fun handleDltMessage( // ... 파라미터들 ... ) { // ... 기존 코드 ... val failedMessage = FailedMessage.create( topic = topic, eventId = eventId, payload = payload, errorMessage = errorMessage, - retryCount = 3, + retryCount = defaultRetryCount, consumerGroup = group )
52-66: eventId 추출 실패 시 0L 반환은 문제가 될 수 있습니다eventId가 없거나 추출 실패 시 0L을 반환하는데, 이는 실제 0번 이벤트와 구분이 안 됩니다. NULL 또는 음수값을 사용하거나, 랜덤 UUID를 생성하는 것이 더 안전할 것 같습니다.
private fun extractEventId(message: Any): Long { return try { when (message) { is Map<*, *> -> { - (message["eventId"] as? Number)?.toLong() ?: 0L + (message["eventId"] as? Number)?.toLong() ?: -1L } else -> { val jsonNode = objectMapper.valueToTree<com.fasterxml.jackson.databind.JsonNode>(message) - jsonNode.get("eventId")?.asLong() ?: 0L + jsonNode.get("eventId")?.asLong() ?: -1L } } } catch (e: Exception) { - log.warn("eventId 추출 실패, 기본값 사용: $e") - 0L + log.warn("eventId 추출 실패, 음수값 사용: $e") + -1L } }src/main/kotlin/com/dh/baro/order/infra/IdempotencyService.kt (2)
36-39: TTL 설정을 환경별로 조정 가능하게 만들어야 합니다24시간 고정 TTL은 모든 환경에 적합하지 않을 수 있습니다. 개발/스테이징 환경에서는 더 짧게, 프로덕션에서는 더 길게 설정이 필요할 수 있습니다.
+@Value("\${idempotency.ttl-hours:24}") +private val ttlHours: Long = 24 + companion object { private const val IDEMPOTENCY_KEY_PREFIX = "idempotency:" - private val TTL_DURATION = Duration.ofHours(24) + private val TTL_DURATION = Duration.ofHours(ttlHours) }
12-15: hasKey 체크 제거 및 tryMarkAsProcessing 단일화 권장
setIfAbsent는 키 존재 여부를 atomic하게 처리하므로,isAlreadyProcessed호출 없이tryMarkAsProcessing만으로도 레이스 없이 중복 처리를 방지할 수 있습니다.isAlreadyProcessed메서드는 논리상 중복이므로 제거를 검토하세요.src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (2)
53-55: Optional을 사용하는 것보다 Kotlin의 null 안전성을 활용하는 것이 좋습니다Java의 Optional 대신 Kotlin의 nullable 타입과 엘비스 연산자를 사용하면 더 간결하고 관용적인 코드가 됩니다.
-val order = orderRepository.findById(inventoryInsufficientEvent.orderId) - .orElseThrow { IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(inventoryInsufficientEvent.orderId)) } +val order = orderRepository.findByIdOrNull(inventoryInsufficientEvent.orderId) + ?: throw IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(inventoryInsufficientEvent.orderId))
66-69: 롤백 실패 시 로그만 남기는 것으로 충분한지 검토 필요재고 롤백이 실패했을 때 단순 로그만 남기고 있는데, 이는 데이터 일관성 문제를 야기할 수 있습니다. 알림이나 추가적인 보상 로직이 필요할 수 있습니다.
롤백 실패 시 관리자 알림이나 별도의 보상 큐로 메시지를 전송하는 로직을 추가하시겠습니까?
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt (3)
37-38: 예외를 삼키고(false 반환) 로깅이 없어 장애 분석 곤란최소 WARN 로그는 남기고, 메시지 ID/이벤트 타입을 포함하세요.
+import org.slf4j.LoggerFactory ... ) { + private val log = LoggerFactory.getLogger(OutboxMessageRouter::class.java) ... } catch (e: Exception) { - false + log.warn("Outbox routing failed. msgId={}, type={}", msg.id, msg.eventType, e) + false }
35-36: 알 수 없는 이벤트 타입은 재시도보다 즉시 사망(DLQ/DEAD) 처리 고려현재는 일반 실패와 동일하게 재시도되어 의미 없는 3회 반복 후 DEAD로 전이됩니다. Unknown 타입은 재시도 무의미하므로 별도 예외를 던져 핸들러에서 즉시 DEAD로 표시하도록 프로토콜을 분리하는 것을 권장합니다.
핸들러(OutboxMessageHandler)가 예외 종류별로
markSendFail()vsmarkDead()를 구분 처리할 수 있는지 확인 부탁드립니다. 필요 시 전용 예외(UnknownEventTypeException) 정의를 제안드릴 수 있습니다.
43-45: 이벤트 타입 문자열 상수 드리프트 위험 — 공용 Enum/Sealed class로 통합 권장
OutboxMessage생성 측과 라우터 간 하드코딩 문자열이 어긋나면 영구 실패가 납니다. 공용 Enum 또는 sealed hierarchy + 매핑 함수로 단일 출처를 유지하세요.src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt (2)
63-68: defaultTopic 하드코딩은 발화 사고 위험명시적 토픽 지정이 안전합니다. 기본 토픽을 제거하거나 프로퍼티로 외부화하세요.
- val template = KafkaTemplate(producerFactory()) - template.defaultTopic = "default-topic" - return template + return KafkaTemplate(producerFactory())
106-127: 운영 팁: 미생성 토픽으로 인한 부팅 실패 방지프로비저닝 시점 차이로 부팅 실패를 피하려면 필요 시 아래 옵션 고려.
val factory = ConcurrentKafkaListenerContainerFactory<String, Any>() factory.consumerFactory = consumerFactory() factory.containerProperties.ackMode = ContainerProperties.AckMode.MANUAL factory.setConcurrency(3) // 동시 처리 스레드 수 + factory.containerProperties.isMissingTopicsFatal = false
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
gradle.properties(1 hunks)gradle/spring.gradle(1 hunks)src/main/kotlin/com/dh/baro/core/ErrorMessage.kt(1 hunks)src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt(1 hunks)src/main/kotlin/com/dh/baro/core/dlt/DltHandler.kt(1 hunks)src/main/kotlin/com/dh/baro/core/dlt/FailedMessage.kt(1 hunks)src/main/kotlin/com/dh/baro/core/dlt/FailedMessageRepository.kt(1 hunks)src/main/kotlin/com/dh/baro/core/exception/ConflictException.kt(1 hunks)src/main/kotlin/com/dh/baro/core/exception/InventoryInsufficientException.kt(1 hunks)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageHandler.kt(1 hunks)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt(1 hunks)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageScheduler.kt(1 hunks)src/main/kotlin/com/dh/baro/order/application/OrderFacade.kt(3 hunks)src/main/kotlin/com/dh/baro/order/application/event/OrderCancelledEvent.kt(1 hunks)src/main/kotlin/com/dh/baro/order/application/event/OrderPlacedEvent.kt(1 hunks)src/main/kotlin/com/dh/baro/order/domain/Order.kt(1 hunks)src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt(1 hunks)src/main/kotlin/com/dh/baro/order/domain/service/OrderServiceV2.kt(2 hunks)src/main/kotlin/com/dh/baro/order/infra/IdempotencyService.kt(1 hunks)src/main/kotlin/com/dh/baro/order/infra/InventoryEventBeforeListener.kt(0 hunks)src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt(1 hunks)src/main/kotlin/com/dh/baro/order/presentation/OrderController.kt(1 hunks)src/main/kotlin/com/dh/baro/product/application/event/InventoryDeductionCompletedEvent.kt(1 hunks)src/main/kotlin/com/dh/baro/product/application/event/InventoryInsufficientEvent.kt(1 hunks)src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt(2 hunks)src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt(1 hunks)src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt(0 hunks)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt(0 hunks)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt(0 hunks)src/main/kotlin/com/dh/baro/product/infra/redis/InventoryRedisRepository.kt(3 hunks)
💤 Files with no reviewable changes (4)
- src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt
- src/main/kotlin/com/dh/baro/order/infra/InventoryEventBeforeListener.kt
- src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt
- src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codrin2
PR: S-BARO/server#17
File: src/main/kotlin/com/dh/baro/look/presentation/dto/LookDetailResponse.kt:38-59
Timestamp: 2025-08-12T13:52:29.449Z
Learning: codrin2 prefers direct field access for immutable id properties (val) rather than using getters, reasoning that immutable fields pose less encapsulation risk.
🧬 Code graph analysis (19)
src/main/kotlin/com/dh/baro/product/application/event/InventoryInsufficientEvent.kt (4)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt (2)
inventoryService(7-18)item(14-16)src/main/kotlin/com/dh/baro/product/domain/InventoryItem.kt (1)
productId(3-6)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt (1)
orderService(11-25)
src/main/kotlin/com/dh/baro/product/application/event/InventoryDeductionCompletedEvent.kt (4)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt (3)
inventoryService(7-18)item(14-16)handleEvent(12-17)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt (2)
orderService(11-25)EVENT_ASYNC_TASK_EXECUTOR(17-24)src/main/kotlin/com/dh/baro/product/domain/InventoryItem.kt (1)
productId(3-6)
src/main/kotlin/com/dh/baro/core/dlt/FailedMessageRepository.kt (2)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRepository.kt (1)
findPendingAndRetryableMessages(7-15)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt (1)
name(7-66)
src/main/kotlin/com/dh/baro/core/exception/ConflictException.kt (3)
src/main/kotlin/com/dh/baro/core/exception/ForbiddenException.kt (1)
message(3-3)src/main/kotlin/com/dh/baro/core/exception/UnauthorizedException.kt (1)
message(3-3)src/main/kotlin/com/dh/baro/core/advice/GlobalExceptionHandler.kt (1)
HttpStatus(131-136)
src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt (3)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt (3)
inventoryService(7-18)item(14-16)handleEvent(12-17)src/main/kotlin/com/dh/baro/product/domain/Product.kt (1)
deductStock(106-112)src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
addItemsToOrder(42-60)
src/main/kotlin/com/dh/baro/order/presentation/OrderController.kt (2)
src/main/kotlin/com/dh/baro/order/presentation/swagger/OrderSwagger.kt (2)
summary(26-97)name(18-206)src/main/kotlin/com/dh/baro/cart/presentation/CartController.kt (1)
HttpStatus(29-34)
src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt (1)
src/main/kotlin/com/dh/baro/order/presentation/dto/OrderSummary.kt (1)
using(10-26)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageHandler.kt (2)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt (2)
name(7-66)markSendSuccess(36-38)src/main/kotlin/com/dh/baro/core/outbox/OutboxBatchJob.kt (1)
outboxMessageHandler(8-28)
src/main/kotlin/com/dh/baro/order/domain/service/OrderServiceV2.kt (1)
src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
confirmOrder(62-66)
src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt (1)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt (1)
kafkaTemplate(12-46)
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (5)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt (2)
orderService(11-25)EVENT_ASYNC_TASK_EXECUTOR(17-24)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt (1)
inventoryService(7-18)src/main/kotlin/com/dh/baro/order/infra/InventoryEventBeforeListener.kt (1)
inventoryRedisRepository(11-26)src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
productRepository(14-67)src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt (1)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt (1)
name(7-66)
src/main/kotlin/com/dh/baro/order/application/event/OrderPlacedEvent.kt (2)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt (1)
orderService(11-25)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (2)
src/main/kotlin/com/dh/baro/order/presentation/dto/OrderSummary.kt (1)
using(10-26)src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
productRepository(14-67)
src/main/kotlin/com/dh/baro/core/dlt/FailedMessage.kt (2)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt (2)
name(7-66)getId(34-34)src/main/kotlin/com/dh/baro/core/BaseTimeEntity.kt (1)
name(10-34)
src/main/kotlin/com/dh/baro/order/application/OrderFacade.kt (1)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)
src/main/kotlin/com/dh/baro/product/infra/redis/InventoryRedisRepository.kt (1)
src/main/kotlin/com/dh/baro/order/infra/InventoryEventBeforeListener.kt (1)
inventoryRedisRepository(11-26)
src/main/kotlin/com/dh/baro/core/exception/InventoryInsufficientException.kt (5)
src/main/kotlin/com/dh/baro/product/domain/Product.kt (1)
deductStock(106-112)src/main/kotlin/com/dh/baro/product/domain/InventoryItem.kt (1)
productId(3-6)src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
addItemsToOrder(42-60)src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (2)
cmd(115-129)shouldThrow(157-161)src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventHandler.kt (1)
inventoryService(7-18)
src/main/kotlin/com/dh/baro/order/application/event/OrderCancelledEvent.kt (1)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryDeductionRequestedEvent.kt (1)
orderId(5-9)
🔇 Additional comments (19)
gradle/spring.gradle (1)
14-14: spring-kafka 추가 LGTM의존성 추가는 타당합니다. Boot BOM이 버전 정합성도 관리합니다.
통합 테스트를 강화하려면 Embedded Kafka를 위한 테스트 의존성 추가를 권장합니다.
dependencies { ... implementation "org.springframework.kafka:spring-kafka" + testImplementation "org.springframework.kafka:spring-kafka-test" ... }gradle.properties (1)
41-42:axonVersion속성 사용처 확인 필요
- Gradle 스크립트(rg 검색)에서
axonVersion참조가 없습니다. 실제로 빌드 의존성에 반영되는지 검토해 주세요.- 참고: Axon Framework 4.10.3은 Spring Boot 3.4/Spring Framework 6.2와 호환됩니다.
src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt (1)
9-10: CANCELED → CANCELLED 변경은 역호환성 파괴 — DB/매핑/마이그레이션 필요레포 검색 결과: 코드 내에 "CANCELED" 직접 언급은 발견되지 않았고, 다음 파일들에서 @Enumerated(EnumType.STRING)로 매핑된 필드가 확인되었습니다. DB에 기존 'CANCELED' 값이 남아있다면 애플리케이션이 역직렬화 실패(예: IllegalArgumentException)를 일으킬 수 있습니다. 다음 중 하나를 적용하세요.
- DB 마이그레이션: UPDATE ... WHERE order_status = 'CANCELED' → 'CANCELLED'
- JPA AttributeConverter로 DB 값 "CANCELED"를 OrderStatus.CANCELLED로 매핑
- (임시) CANCELED 상수 복구 후 @deprecated 처리
확인된 파일(@Enumerated(EnumType.STRING)):
- src/main/kotlin/com/dh/baro/order/domain/Order.kt:28
- src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt:25
- src/main/kotlin/com/dh/baro/identity/domain/User.kt:28
- src/main/kotlin/com/dh/baro/identity/domain/SocialAccount.kt:21
- src/main/kotlin/com/dh/baro/identity/domain/Store.kt:34
권장 임시완충안(diff):
enum class OrderStatus { PENDING, ORDERED, PAID, SHIPPED, DELIVERED, - COMPLETED, - CANCELLED, + COMPLETED, + @Deprecated("Use CANCELLED instead; kept for backward compatibility") + CANCELED, + CANCELLED, }또는 Converter 예시:
@Converter(autoApply = true) class OrderStatusConverter : AttributeConverter<OrderStatus, String> { override fun convertToDatabaseColumn(attr: OrderStatus?) = attr?.name override fun convertToEntityAttribute(dbData: String?): OrderStatus? = when (dbData) { "CANCELED" -> OrderStatus.CANCELLED else -> dbData?.let { OrderStatus.valueOf(it) } } }잔존 사용처(코드) 및 DB 값('CANCELED' 존재 여부) 확인 후 적용 방식 결정해서 반영하세요.
src/main/kotlin/com/dh/baro/product/application/event/InventoryInsufficientEvent.kt (1)
5-10: 승인 — Outbox/Kafka 매핑 정합성 확인됨ProductEventListener.kt에서 eventType = "INVENTORY_INSUFFICIENT"로 발행되고, OutboxMessageRouter.kt의 INVENTORY_INSUFFICIENT_EVENT(="INVENTORY_INSUFFICIENT") 분기에서 payload를 InventoryInsufficientEvent로 역직렬화해 KafkaConfig.INVENTORY_EVENTS_TOPIC으로 전송합니다. 매핑/타입 정합성 문제 없음.
src/main/kotlin/com/dh/baro/order/application/event/OrderPlacedEvent.kt (1)
6-11: 이벤트 모델 적절 — Kafka 역직렬화 설정 확인 완료IdGenerator 기반 eventId와 items 리스트 포함 설계는 적절합니다. src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt에서 JsonDeserializer.TRUSTED_PACKAGES="com.dh.baro", JsonDeserializer.VALUE_DEFAULT_TYPE=OrderPlacedEvent, JsonDeserializer.TYPE_MAPPINGS에 OrderPlacedEvent 매핑이 설정되어 있어 타입 매핑·신뢰 패키지 관련 추가 조치 불필요합니다.
src/main/kotlin/com/dh/baro/product/application/event/InventoryDeductionCompletedEvent.kt (1)
6-11: 완료 이벤트 구조 OK — Kafka 매핑 반영 확인됨주문/유저/아이템 및 eventId 구성 표준적이며, src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt에서 import(라인 4)와 타입 매핑(라인 132: "InventoryDeductionCompletedEvent" → InventoryDeductionCompletedEvent::class.java.name)이 확인되었습니다.
src/main/kotlin/com/dh/baro/order/application/event/OrderCancelledEvent.kt (1)
5-10: 취소 이벤트 도입 — 서비스 계층에서 이벤트 생성·아웃박스 기록 일관성 필요OrderCancelledEvent 정의: src/main/kotlin/com/dh/baro/order/application/event/OrderCancelledEvent.kt.
리포지토리 검색 결과(이벤트 참조)는 정의 파일 외에 호출 지점을 찾지 못함.
도메인에서 reason을 소비하지 않으므로, Order.cancel(reason) 흐름에서 서비스 계층이 이벤트 생성 및(outbox) 기록을 항상 호출하도록 구현·합의 필요.src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt (1)
27-35: 멱등 처리 마커 TTL 확인 — 검증 완료src/main/kotlin/com/dh/baro/order/infra/IdempotencyService.kt에서 tryMarkAsProcessing는 setIfAbsent(..., TTL_DURATION)으로 TTL(Duration.ofHours(24))을 설정하고, markProcessingAsCompleted도 TTL_DURATION으로 키를 갱신하며 실패 시 removeProcessingState로 키를 삭제하므로 영구 잠금 우려는 없습니다.
src/main/kotlin/com/dh/baro/order/domain/Order.kt (1)
65-75: Enum 명칭 변경(CANCELED → CANCELLED) — DB 마이그레이션 여부 확인 필요검증 결과: 코드베이스는 이미 CANCELLED로 통일됨(확인 파일: src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt, src/main/kotlin/com/dh/baro/order/domain/Order.kt, src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt, src/main/kotlin/com/dh/baro/core/ErrorMessage.kt). Order.status에 @Enumerated(EnumType.STRING) 사용이 확인되며, 마이그레이션/DDL/Flyway/Liquibase 파일은 발견되지 않음.
조치: 배포된 DB에 레거시 값 'CANCELED'가 남아 있는지 확인(예: SELECT DISTINCT status FROM orders). 존재하면 마이그레이션 스크립트(예: UPDATE orders SET status='CANCELLED' WHERE status='CANCELED') 또는 정식 Flyway/Liquibase 마이그레이션을 추가할 것.
src/main/kotlin/com/dh/baro/order/application/OrderFacade.kt (1)
51-56: 검증결과: 원 코멘트 일부 부정확 — 아웃박스 영속·재시도 로직이 구현되어 있음
- 사실: OrderEventListener.saveToOutbox는 @TransactionalEventListener(phase = TransactionPhase.BEFORE_COMMIT)로 아웃박스 메시지를 동일 트랜잭션 내에 저장합니다. (src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt)
- 카프카 전송/재시도 흐름 확인: OutboxMessageScheduler(@scheduled) → OutboxBatchJob(run, NOT_SUPPORTED) → OutboxMessageHandler(handleInNewTx, REQUIRES_NEW) → OutboxMessageRouter(kafkaTemplate.send(...).get(1s)). 실패 시 tryCount 증가 → SEND_FAIL/DEAD 처리 및 로그(재시도 로직 존재). (src/main/kotlin/com/dh/baro/core/outbox/*)
- 권고(선택): 멀티 인스턴스 환경에서 중복 전송 방지를 위한 메시지 선점/락(예: 상태 선점 또는 SELECT ... FOR UPDATE SKIP LOCKED) 도입 검토, kafka send 타임아웃(1s) 재검토, DEAD 메시지에 대한 모니터링·알림 추가 권장.
Likely an incorrect or invalid review comment.
src/main/kotlin/com/dh/baro/product/infra/redis/InventoryRedisRepository.kt (1)
45-56: rollbackStocks 메서드가 public API로 노출되었습니다재고 롤백은 보상 트랜잭션의 일부로 올바르게 구현된 것으로 보입니다. 멱등성 처리와 함께 사용되어 중복 롤백을 방지하는 점이 좋습니다.
src/main/kotlin/com/dh/baro/core/ErrorMessage.kt (1)
47-50: 새로운 에러 메시지들이 적절하게 추가되었습니다주문 취소 및 보상 트랜잭션 관련 에러 메시지들이 명확하고 구조화되어 있습니다. 특히 상태별 취소 불가능 메시지는 디버깅에 유용할 것으로 보입니다.
src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt (3)
3-3: InventoryInsufficientException 도입이 적절합니다재고 부족 예외를 전용 예외 타입으로 분리한 것은 좋은 개선입니다. 이벤트 기반 아키텍처에서 예외 타입별 처리가 명확해집니다.
Also applies to: 35-35
20-22: rollbackStocks 메서드 추가가 적절합니다보상 트랜잭션을 위한 명시적인 롤백 메서드 추가는 이벤트 소싱 패턴에 잘 부합합니다.
25-29: deductStocksFromDatabase는 @transactional로 묶여 있어 부분 실패 시 DB 변경은 전체 롤백됩니다 — Outbox/Idempotency 처리 의도 확인 필요
- InventoryService.deductStocksFromDatabase는 @transactional이고 deductStockFromDB가 런타임 예외(InventoryInsufficientException)를 던지므로, 아이템 하나 실패 시 전체 트랜잭션이 롤백됩니다. (src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt)
- ProductEventListener.handleOrderPlacedEvent도 @transactional이므로 catch 블록의 outboxMessageRepository.save(...)와 idempotencyService.removeProcessingState(...)가 동일 트랜잭션에 포함됩니다. 현재 예외 재던짐으로 인해 outbox 저장은 롤백되어 이벤트가 남지 않습니다 — outbox를 실패 케이스에도 보존하려면 outbox 저장을 별도 트랜잭션(REQUIRES_NEW)으로 분리하거나 트랜잭션 경계를 재설계하세요. (src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt)
- 격리 수준은 명시되어 있지 않음. 현재의 조건부 UPDATE(deductStock)는 대부분의 DB에서 원자적으로 동작하므로 기본 격리로도 동시성에 대해 안전한 편이나, 고경쟁 시 명시적 잠금(findByIdForUpdate) 또는 추가 검토 권장.
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (3)
31-39: Outbox 패턴 구현이 적절합니다트랜잭션 커밋 전에 Outbox 메시지를 저장하여 이벤트 발행의 신뢰성을 보장하는 것이 좋습니다.
44-50: 멱등성 처리 로직이 견고합니다이미 처리된 이벤트와 처리 중인 이벤트를 구분하여 중복 처리를 방지하는 로직이 잘 구현되었습니다.
72-75: 예외 발생 시 멱등성 상태 제거가 적절합니다처리 중 예외 발생 시 멱등성 키를 제거하여 재시도를 가능하게 하는 것이 좋은 전략입니다.
src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt (1)
48-52: max.in.flight.requests.per.connection을 5로 상향 검토 — 브로커 버전(>=1.1) 확인 필요Kafka 1.1+에서는 idempotent 프로듀서가 max.in.flight.requests.per.connection ≤5(기본 5)까지 안전합니다. 처리량 병목이면 1→5로 변경, 순서 엄격 필요시 1 유지.
File: src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt Lines: 48-52
- ProducerConfig.MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION to 1, + ProducerConfig.MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION to 5,
| // JSON 역직렬화 설정 | ||
| JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro", | ||
| JsonDeserializer.VALUE_DEFAULT_TYPE to OrderPlacedEvent::class.java.name, | ||
| JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(), | ||
| JsonDeserializer.USE_TYPE_INFO_HEADERS to false | ||
| ) |
There was a problem hiding this comment.
역직렬화 설정 불일치: 헤더 타입 정보 무시 + 기본 타입 고정으로 다형 이벤트가 잘못 파싱됨
프로듀서는 기본적으로 타입 헤더를 추가하지만(JsonSerializer 기본), 컨슈머는 USE_TYPE_INFO_HEADERS=false로 이를 무시하고 OrderPlacedEvent로 고정 역직렬화합니다. 다른 이벤트(InventoryInsufficientEvent 등)는 오매핑/실패됩니다. 헤더 사용으로 전환하세요.
- JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro",
- JsonDeserializer.VALUE_DEFAULT_TYPE to OrderPlacedEvent::class.java.name,
- JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(),
- JsonDeserializer.USE_TYPE_INFO_HEADERS to false
+ JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro",
+ JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(),
+ JsonDeserializer.USE_TYPE_INFO_HEADERS to true프로듀서 쪽도 명시적으로 타입 헤더 추가를 고정(가시성)하는 것을 권장합니다:
ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG to JsonSerializer::class.java,
+ JsonSerializer.ADD_TYPE_INFO_HEADERS to true,
// 타입 매핑 설정
JsonSerializer.TYPE_MAPPINGS to buildTypeMapping()📝 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.
| // JSON 역직렬화 설정 | |
| JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro", | |
| JsonDeserializer.VALUE_DEFAULT_TYPE to OrderPlacedEvent::class.java.name, | |
| JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(), | |
| JsonDeserializer.USE_TYPE_INFO_HEADERS to false | |
| ) | |
| // JSON 역직렬화 설정 | |
| JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro", | |
| JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(), | |
| JsonDeserializer.USE_TYPE_INFO_HEADERS to true | |
| ) |
| // JSON 역직렬화 설정 | |
| JsonDeserializer.TRUSTED_PACKAGES to "com.dh.baro", | |
| JsonDeserializer.VALUE_DEFAULT_TYPE to OrderPlacedEvent::class.java.name, | |
| JsonDeserializer.TYPE_MAPPINGS to buildTypeMapping(), | |
| JsonDeserializer.USE_TYPE_INFO_HEADERS to false | |
| ) | |
| ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG to JsonSerializer::class.java, | |
| JsonSerializer.ADD_TYPE_INFO_HEADERS to true, | |
| // 타입 매핑 설정 | |
| JsonSerializer.TYPE_MAPPINGS to buildTypeMapping() |
🤖 Prompt for AI Agents
In src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt around lines 98 to
103, the consumer config forces a fixed default type and disables type-info
headers which causes polymorphic events (e.g., InventoryInsufficientEvent) to be
mis-parsed; change JsonDeserializer.USE_TYPE_INFO_HEADERS to true and remove or
unset JsonDeserializer.VALUE_DEFAULT_TYPE so the deserializer uses the message
type header, and ensure the producer explicitly enables type headers
(JsonSerializer.ADD_TYPE_INFO_HEADERS = true) so the JVM type is written into
the record headers for correct polymorphic deserialization.
| class InventoryInsufficientException( | ||
| val productId: Long, | ||
| val requestedQuantity: Int, | ||
| ) : ConflictException(ErrorMessage.OUT_OF_STOCK.format(productId)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
도메인 특화 예외 도입은 좋습니다. 다만 호출부 정합성 점검 필요
예외 타입이 ConflictException에서 InventoryInsufficientException으로 바뀌면 테스트/핸들러 매핑도 함께 바뀌어야 합니다. 또한 필요한 경우 가용 재고(available) 등 추가 정보를 포함하면 분석/로깅에 도움이 됩니다.
호출부 전환/테스트 수정 후보 탐색:
🏁 Script executed:
#!/bin/bash
# OUT_OF_STOCK 메시지를 직접 포맷팅하는 잔존 호출부 확인
rg -nP 'ConflictException\s*\(\s*ErrorMessage\.OUT_OF_STOCK\.format\(' -S -C2
# 예외 매핑(ControllerAdvice 등)에서 InventoryInsufficientException 처리 여부 확인
rg -nP 'InventoryInsufficientException' -C3Length of output: 3479
InventoryInsufficientException로 예외 통일 및 핸들러·테스트 갱신 필요
실행 결과 일부 호출부가 여전히 ConflictException(ErrorMessage.OUT_OF_STOCK.format(...))을 던지고 있습니다 — 다음 수정이 필요합니다:
- src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt:48
현재: throw ConflictException(ErrorMessage.OUT_OF_STOCK.format(item.product.id))
변경: throw InventoryInsufficientException(productId, 요청수량) (컨텍스트의 요청 수량 변수 사용) - src/main/kotlin/com/dh/baro/product/domain/Product.kt:108
현재: throw ConflictException(ErrorMessage.OUT_OF_STOCK.format(id))
변경: throw InventoryInsufficientException(id, orderQuantity) - src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt:35 는 이미 InventoryInsufficientException 사용 중 — 일관성 확인 완료
- ControllerAdvice/예외 매핑 및 단위·통합 테스트 갱신: InventoryInsufficientException는 ConflictException을 상속하므로 기존 ConflictException 기반 핸들러는 동작하지만, 예외 타입별 로깅/응답(요청수량·가용재고 등)을 활용하려면 핸들러와 테스트를 명시적으로 갱신하세요.
- 권장(선택): InventoryInsufficientException에 available(가용재고) 같은 필드를 추가해 로그/분석 정보를 풍부하게 만드세요.
🤖 Prompt for AI Agents
In src/main/kotlin/com/dh/baro/core/exception/InventoryInsufficientException.kt
around lines 5-8, the project should consistently throw
InventoryInsufficientException instead of constructing ConflictException
directly: update OrderService at
src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt line ~48 to
throw InventoryInsufficientException(productId, requestedQuantity) using the
local request quantity variable; update
src/main/kotlin/com/dh/baro/product/domain/Product.kt line ~108 to throw
InventoryInsufficientException(id, orderQuantity) instead of ConflictException;
verify InventoryService at
src/main/kotlin/com/dh/baro/product/domain/service/InventoryService.kt line ~35
remains using InventoryInsufficientException; then update
ControllerAdvice/exception mapping and unit/integration tests to handle
InventoryInsufficientException explicitly (adjust logging/response to include
requestedQuantity and optionally available stock), and consider adding an
available field to InventoryInsufficientException to enrich logs/analytics.
| val isSuccess = outboxMessageRouter.route(outboxMessage) | ||
|
|
||
| if (isSuccess) { | ||
| outboxMessageRepository.delete(outboxMessage) | ||
| } else { | ||
| outboxMessage.markSendFail() | ||
| outboxMessageRepository.save(outboxMessage) | ||
|
|
There was a problem hiding this comment.
라우팅 예외 전파 시 재시도 카운트 미증가 가능. try/catch로 실패 경로 일원화 권장
현재 route가 예외를 던지면 트랜잭션 롤백되어 markSendFail()이 누락될 수 있습니다. 예외 포함 실패 경로를 동일 처리하세요.
- val isSuccess = outboxMessageRouter.route(outboxMessage)
-
- if (isSuccess) {
- outboxMessageRepository.delete(outboxMessage)
- } else {
- outboxMessage.markSendFail()
- outboxMessageRepository.save(outboxMessage)
- }
+ try {
+ val isSuccess = outboxMessageRouter.route(outboxMessage)
+ if (isSuccess) {
+ // (선택) 상태 표준화: 성공 마킹 후 삭제
+ // outboxMessage.markSendSuccess()
+ outboxMessageRepository.delete(outboxMessage)
+ return
+ }
+ outboxMessage.markSendFail()
+ outboxMessageRepository.save(outboxMessage)
+ } catch (ex: Exception) {
+ outboxMessage.markSendFail()
+ outboxMessageRepository.save(outboxMessage)
+ log.warn("Outbox route failed. eventType={}, id={}", outboxMessage.eventType, outboxMessage.id, ex)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val isSuccess = outboxMessageRouter.route(outboxMessage) | |
| if (isSuccess) { | |
| outboxMessageRepository.delete(outboxMessage) | |
| } else { | |
| outboxMessage.markSendFail() | |
| outboxMessageRepository.save(outboxMessage) | |
| try { | |
| val isSuccess = outboxMessageRouter.route(outboxMessage) | |
| if (isSuccess) { | |
| // (선택) 상태 표준화: 성공 마킹 후 삭제 | |
| // outboxMessage.markSendSuccess() | |
| outboxMessageRepository.delete(outboxMessage) | |
| return | |
| } | |
| outboxMessage.markSendFail() | |
| outboxMessageRepository.save(outboxMessage) | |
| } catch (ex: Exception) { | |
| outboxMessage.markSendFail() | |
| outboxMessageRepository.save(outboxMessage) | |
| log.warn("Outbox route failed. eventType={}, id={}", outboxMessage.eventType, outboxMessage.id, ex) | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageHandler.kt around lines
19 to 26, the current logic calls outboxMessageRouter.route(outboxMessage) and
only handles the failure path when it returns false; if route throws an
exception the transaction may roll back and markSendFail()/save() never run.
Wrap the route call in a try/catch so both boolean-false and exception cases
funnel to the same failure handling: on success delete the message; on failure
(either route returned false or an exception was caught) call
outboxMessage.markSendFail(), persist the change with
outboxMessageRepository.save(outboxMessage), and log the exception in the catch;
rethrow only if caller semantics require it, otherwise swallow after logging to
ensure retry count increments.
| @Transactional | ||
| fun changeOrderStatus(orderId: Long, status: OrderStatus): Order { | ||
| val order = orderRepository.findById(orderId) | ||
| .orElseThrow { IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId)) } | ||
| order.changeStatus(status) | ||
| return orderRepository.save(order) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
임의 상태 전환 노출 주의. 도메인 제약 검증/테스트 확인 필요
changeStatus가 유효 전이만 허용하는지(예: PENDING→ORDERED, DELIVERED→COMPLETED) 확인 바랍니다. 예외 메시지는 OK.
테스트/구현 확인 스크립트:
스타일 합치기(기존 서비스와 동일하게 findByIdOrNull 사용)도 제안드립니다.
- val order = orderRepository.findById(orderId)
- .orElseThrow { IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId)) }
+ val order = orderRepository.findByIdOrNull(orderId)
+ ?: throw IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId))🏁 Script executed:
#!/bin/bash
rg -nP 'fun\s+changeStatus\(' -C3
rg -nP 'OrderStatus\.' -C1 src/testLength of output: 807
조치 필요 — Order.changeStatus에 상태 전이 검증이 없음
- 핵심: src/main/kotlin/com/dh/baro/order/domain/Order.kt의 fun changeStatus(newStatus: OrderStatus)가 this.status = newStatus로 단순 대입되어 도메인 전이 제약을 전혀 검증하지 않음 (약력: lines 77–79). 조치: 허용된 상태 전이만 통과시키는 검증 로직을 도메인에 추가하고, 불가능한 전이는 명시적 예외로 차단할 것.
- 테스트 불일치: src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt의 테스트 설명("초기 상태는 ORDERED 이다")과 어설션(OrderStatus.PENDING)이 상충(lines 108–110). 조치: 테스트 설명 또는 기대값을 정정하고 상태 전이 규칙을 반영한 테스트를 추가/수정할 것.
- 선택적 스타일 제안: 서비스에서 findByIdOrNull로 통일 권장 — 변경 예시(선택):
- val order = orderRepository.findById(orderId)
- .orElseThrow { IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId)) }
+ val order = orderRepository.findByIdOrNull(orderId)
+ ?: throw IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId))📝 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.
| @Transactional | |
| fun changeOrderStatus(orderId: Long, status: OrderStatus): Order { | |
| val order = orderRepository.findById(orderId) | |
| .orElseThrow { IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId)) } | |
| order.changeStatus(status) | |
| return orderRepository.save(order) | |
| } | |
| @Transactional | |
| fun changeOrderStatus(orderId: Long, status: OrderStatus): Order { | |
| val order = orderRepository.findByIdOrNull(orderId) | |
| ?: throw IllegalArgumentException(ErrorMessage.ORDER_NOT_FOUND.format(orderId)) | |
| order.changeStatus(status) | |
| return orderRepository.save(order) | |
| } |
🤖 Prompt for AI Agents
In src/main/kotlin/com/dh/baro/order/domain/service/OrderServiceV2.kt around
lines 37–43, changeOrderStatus delegates a raw state assignment to
Order.changeStatus which currently does no transition validation; update the
domain method (src/main/kotlin/com/dh/baro/order/domain/Order.kt, lines ~77–79)
to enforce allowed state transitions and throw a clear domain exception (e.g.,
InvalidOrderStateTransitionException) for disallowed transitions, then keep
service calling changeStatus and save; also correct the unit test mismatch in
src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (lines ~108–110) so
the test description and asserted expected state align with the domain rules (or
add tests covering allowed/forbidden transitions); optionally standardize
repository fetches by using findByIdOrNull in the service and handle null ->
IllegalArgumentException as before.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/resources/application-dev.yaml (1)
14-14: SERVER_URL 환경변수 불일치(실행 시 플레이스홀더 미해결 위험)dev 설정은
${DEV_SERVER_URL}를 참조하지만 배포 스크립트는SERVER_URL만 주입합니다.${SERVER_URL}로 통일하세요.-server: - url: ${DEV_SERVER_URL} +server: + url: ${SERVER_URL}.github/workflows/deploy-prod.yaml (1)
82-113: Prod 배포 워크플로에 MongoDB 환경변수 누락 — 애플리케이션 부팅 실패 가능검증 결과: .github/workflows/deploy-dev.yaml에는 MONGODB_* 환경변수(MONGODB_USERNAME, MONGODB_PASSWORD, MONGODB_HOST, MONGODB_PORT, MONGODB_DATABASE, MONGODB_AUTH_SOURCE)가 존재(라인 110–115)하지만 .github/workflows/deploy-prod.yaml에는 없음. prod 배포 스크립트에 동일한 env 주입 필요.
-e KAKAO_CLIENT_ID=${{ secrets.KAKAO_CLIENT_ID }} \ -e SERVER_URL=${{ secrets.PROD_SERVER_URL }} \ + -e MONGODB_USERNAME=${{ secrets.MONGODB_USERNAME }} \ + -e MONGODB_PASSWORD=${{ secrets.MONGODB_PASSWORD }} \ + -e MONGODB_HOST=${{ secrets.MONGODB_HOST }} \ + -e MONGODB_PORT=${{ secrets.MONGODB_PORT }} \ + -e MONGODB_DATABASE=${{ secrets.MONGODB_DATABASE }} \ + -e MONGODB_AUTH_SOURCE=${{ secrets.MONGODB_AUTH_SOURCE }} \ -e KAFKA_HOST=${{ secrets.KAFKA_HOST }} \src/main/resources/application-prod.yaml (1)
14-14: SERVER_URL 환경변수 불일치 — 런타임 플레이스홀더 미해결 위험 (긴급 수정 필요)src/main/resources/application-prod.yaml(14)은 ${PROD_SERVER_URL}, application-dev.yaml(14)은 ${DEV_SERVER_URL}을 사용하고 있으나 CI 배포는 컨테이너 런타임에 -e SERVER_URL=... 으로 주입합니다 (.github/workflows/deploy-prod.yaml:109, .github/workflows/deploy-dev.yaml:109). 이 불일치로 런타임에 플레이스홀더가 미해결되어 부팅 실패할 수 있습니다.
조치(택1):
- 권장: YAML을 통일하여 둘 다 ${SERVER_URL} 사용 — src/main/resources/application-{prod,dev}.yaml 수정.
- 대안: 배포 스크립트에서 컨테이너 런타임에 PROD_SERVER_URL/DEV_SERVER_URL을 -e로 주입하도록 변경.
-server: - url: ${PROD_SERVER_URL} +server: + url: ${SERVER_URL}application-dev.yaml도 동일하게 ${DEV_SERVER_URL} → ${SERVER_URL}으로 정리하세요.
🧹 Nitpick comments (14)
src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (2)
142-147: 사양 설명과 검증이 불일치합니다it 설명은 “각 항목을 그대로 저장”이라고 했지만, 검증은 items.size == 1(동일 상품 합산 저장)을 기대합니다. 의도에 맞게 설명을 정정하는 편이 명확합니다.
다음 변경을 제안합니다.
- it("각 항목을 그대로 저장하고 합산하여 재고를 차감한다") { + it("동일 상품은 합산 저장하고 재고를 차감한다") { val order = orderService.createOrder(cmd) - order.items.shouldHaveSize(1) // p1 하나만 + order.items.shouldHaveSize(1) // 동일 상품(p1)은 합산되어 1개 라인 productRepository.findById(p1.id).get().getQuantity() shouldBe 2 // 5-(1+2) }
70-73: DataJpaTest에선 별도 정리 불요(트랜잭션 롤백)@DataJpaTest는 각 테스트 종료 시 트랜잭션 롤백이 기본입니다. afterEach의 deleteAll은 중복 정리일 가능성이 높습니다. 유지보수성 위해 제거를 권장합니다.
- afterEach { - orderRepository.deleteAll() - productRepository.deleteAll() - } + // @DataJpaTest의 기본 롤백으로 정리되므로 afterEach 정리는 생략합니다..github/workflows/deploy-prod.yaml (1)
43-44: Gradle 빌드 단계의 PROD_SERVER_URL 시스템 프로퍼티는 컨테이너 런타임과 무관런타임에서 yaml 플레이스홀더를 해석하므로, 서버 URL은 컨테이너 env로만 일치하면 충분합니다. 위 SERVER_URL 통일 후 빌드 단계의
-DPROD_SERVER_URL=...는 제거해도 됩니다(선택).src/main/kotlin/com/dh/baro/core/dlq/FailedMessage.kt (1)
9-31: 운영 분석 강화를 위한 메타데이터(파티션/오프셋/오리지널 토픽) 및 인덱스 추가 제안검색·대시보드(기간/토픽/그룹) 필터에 대비해 컬럼과 인덱스를 권장합니다.
@Table(name = "failed_messages") class FailedMessage private constructor( @@ - @Column(name = "topic", nullable = false, length = 100) + @Column(name = "topic", nullable = false, length = 100) val topic: String, @@ - @Column(name = "retry_count", nullable = false) + @Column(name = "retry_count", nullable = false) val retryCount: Int, - @Column(name = "consumer_group", nullable = false, length = 50) + @Column(name = "consumer_group", nullable = false, length = 50) val consumerGroup: String, + + @Column(name = "partition", nullable = false) + val partition: Int = -1, + + @Column(name = "offset", nullable = false) + val offset: Long = -1, ) : BaseTimeEntity() {스키마 인덱스(토픽, 그룹, 생성일자):
-@Table(name = "failed_messages") +@Table( + name = "failed_messages", + indexes = [ + Index(name = "idx_failed_topic_created_at", columnList = "topic, created_at"), + Index(name = "idx_failed_group_created_at", columnList = "consumer_group, created_at") + ] +)팩토리에도 추가 파라미터/기본값 반영 필요.
src/main/kotlin/com/dh/baro/core/dlq/FailedMessageRepository.kt (1)
5-5: LGTM기본 CRUD 요건 충족. 필요 시 기간/토픽 기반 조회 메서드 추가를 권장합니다.
fun findByTopicAndCreatedAtBetween(topic: String, from: Instant, to: Instant, pageable: Pageable): Page<FailedMessage>src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt (2)
43-45: 성공 로그 레벨이 error로 설정됨성공 저장은 warn/info가 적절합니다.
- log.error("DLT 메시지 저장 완료: topic=$topic, eventId=$eventId") + log.warn("DLT 메시지 저장 완료: topic=$topic, eventId=$eventId")
39-41: 재시도 횟수 하드코딩(3)컨테이너 에러핸들러 설정(FixedBackOff)과 분리될 수 있습니다. 헤더(
KafkaHeaders.DLT_EXCEPTION_STACKTRACE등)만으로는 시도 횟수를 알기 어려우므로,KafkaHeaders.DELIVERY_ATTEMPT혹은 커스텀 헤더를 함께 기록하는 방향을 고려하세요. 설정값 주입으로 동기화하는 것도 방법입니다.- retryCount = 3, + retryCount = retryAttempts, // @Value로 주입 또는 헤더 파싱 결과src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt (4)
98-103: 다형 이벤트 역직렬화 설정 적절
TRUSTED_PACKAGES+TYPE_MAPPINGS+USE_TYPE_INFO_HEADERS=true조합은 안정적입니다. 프로듀서에서 타입 헤더 추가를 명시적으로 고정하면 운영 가시성이 좋아집니다(아래 참고).ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG to JsonSerializer::class.java, + JsonSerializer.ADD_TYPE_INFO_HEADERS to true, // 타입 매핑 설정 JsonSerializer.TYPE_MAPPINGS to buildTypeMapping()
47-52: 주석의 “Exactly-Once Semantics”는 과장 표현 — 트랜잭션 미설정현재 설정은 프로듀서 idempotence(순서/중복 최소화) 수준이며, EOS를 보장하려면 트랜잭션 ID 프리픽스와 템플릿 트랜잭션 사용이 필요합니다. 주석 정정 또는 트랜잭션 활성화를 권장합니다.
- // 안전성 설정 (Exactly-Once Semantics) + // 안전성 설정 (Idempotence; EOS를 원하면 transactionIdPrefix 설정 및 트랜잭션 사용 필요) ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG to true, ProducerConfig.ACKS_CONFIG to "all", ProducerConfig.RETRIES_CONFIG to Int.MAX_VALUE, ProducerConfig.MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION to 1,트랜잭션 활성화 예:
- fun producerFactory(): ProducerFactory<String, Any> { - val configProps = buildProducerProperties() - val producerFactory = DefaultKafkaProducerFactory<String, Any>(configProps) - return producerFactory - } + fun producerFactory(): ProducerFactory<String, Any> { + val configProps = buildProducerProperties() + val pf = DefaultKafkaProducerFactory<String, Any>(configProps) + pf.setTransactionIdPrefix("baro-tx-") + return pf + }
112-121: DLQ 토픽 네이밍 일관·폴백 처리 적절원본 토픽 매핑과 폴백
-dlq일치합니다. 운영 규칙 문서화만 추가해주세요(선택).
63-68: defaultTopic 설정은 불필요해 보임명시 토픽 전송만 사용 중이면 혼란 방지를 위해 제거를 권장합니다.
- val template = KafkaTemplate(producerFactory()) - template.defaultTopic = "default-topic" - return template + return KafkaTemplate(producerFactory())src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (3)
58-60: 상태 전이 검증 및 낙관적 락(Optimistic Lock) 고려현재 상태 확인 없이 무조건 CANCELLED로 전이합니다. 중복 이벤트/경합 시 불일치 가능성이 큽니다. 허용 전이만 수행하도록 가드하고, Order에 @Version을 두고 OptimisticLock 예외에 대한 재시도 정책을 적용하세요.
33-41: Outbox 메시지에 key/correlationId 등 메타데이터 포함 권장라우팅/파티셔닝/추적을 위해 orderId 기반 key, correlationId/causationId를 OutboxMessage에 함께 저장하는 것을 권장합니다. 키 기준 파티셔닝은 동일 주문 이벤트 순서를 보장합니다.
46-52: 관측성: 아이템포턴시 스킵/완료/해제 로그에 컨텍스트 포함운영 추적을 위해 eventId/orderId를 포함한 구조적 로그 추가를 권장합니다. 장애 원인 역추적이 쉬워집니다.
예시:
- if (!idempotencyService.tryMarkAsProcessing(inventoryInsufficientEvent.eventId)) { - return - } + if (!idempotencyService.tryMarkAsProcessing(inventoryInsufficientEvent.eventId)) { + log.debug("Idempotency skip (already processed/in-flight). eventId={}, orderId={}", + inventoryInsufficientEvent.eventId, inventoryInsufficientEvent.orderId) + return + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
-
.github/workflows/deploy-dev.yaml(1 hunks) -
.github/workflows/deploy-prod.yaml(1 hunks) -
src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt(1 hunks) -
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt(1 hunks) -
src/main/kotlin/com/dh/baro/core/dlq/FailedMessage.kt(1 hunks) -
src/main/kotlin/com/dh/baro/core/dlq/FailedMessageRepository.kt(1 hunks) -
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt(1 hunks) -
src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt(1 hunks) -
src/main/resources/application-dev.yaml(1 hunks) -
src/main/resources/application-prod.yaml(1 hunks) -
src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/com/dh/baro/product/infra/ProductEventListener.kt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codrin2
PR: S-BARO/server#17
File: src/main/kotlin/com/dh/baro/look/presentation/dto/LookDetailResponse.kt:38-59
Timestamp: 2025-08-12T13:52:29.449Z
Learning: codrin2 prefers direct field access for immutable id properties (val) rather than using getters, reasoning that immutable fields pose less encapsulation risk.
🧬 Code graph analysis (4)
src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (2)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (2)
confirmOrder(53-63)name(11-78)src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt (1)
PENDING(3-10)
src/main/kotlin/com/dh/baro/core/dlq/FailedMessageRepository.kt (4)
src/main/kotlin/com/dh/baro/identity/domain/repository/StoreRepository.kt (1)
interface StoreRepository : JpaRepository<Store, Long>(6-6)src/main/kotlin/com/dh/baro/identity/domain/repository/UserRepository.kt (1)
interface UserRepository : JpaRepository<User, Long>(6-6)src/main/kotlin/com/dh/baro/product/domain/repository/CategoryRepository.kt (1)
interface CategoryRepository : JpaRepository<Category, Long>(6-6)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRepository.kt (1)
findPendingAndRetryableMessages(7-15)
src/main/kotlin/com/dh/baro/core/dlq/FailedMessage.kt (2)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessage.kt (1)
name(7-66)src/main/kotlin/com/dh/baro/core/BaseTimeEntity.kt (1)
name(10-34)
src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt (1)
src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt (1)
kafkaTemplate(12-46)
🔇 Additional comments (8)
src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (1)
109-109: 결론: 철자 혼재 없음 — 'CANCELLED'로 통일됨레포 검색 결과 'CANCELED' 철자 없음; 다음 파일들에서 'CANCELLED' 사용 확인됨:
- src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt:10 — CANCELLED
- src/main/kotlin/com/dh/baro/order/domain/Order.kt:66, 74 — OrderStatus.CANCELLED 사용(비교/변경)
- src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt:58 — order.changeStatus(OrderStatus.CANCELLED)
.github/workflows/deploy-prod.yaml (1)
110-110: Kafka 호스트 주입 추가 적절애플리케이션 설정(${KAFKA_HOST}:9092)과 일치합니다.
.github/workflows/deploy-dev.yaml (2)
116-116: Kafka 호스트 주입 추가 적절dev/prod 구성과 합치합니다.
109-109: SERVER_URL 사용 vs dev yaml의 DEV_SERVER_URL 불일치dev yaml은
${DEV_SERVER_URL}를 참조하지만 여기서는SERVER_URL만 주입합니다. yaml을${SERVER_URL}로 통일하는 방향을 권장합니다(별도 dev yaml 코멘트에 패치 포함).#!/bin/bash rg -nP 'DEV_SERVER_URL|SERVER_URL' src/main/resources/application-dev.yaml .github/workflows/deploy-dev.yaml -C2src/main/kotlin/com/dh/baro/core/dlq/FailedMessage.kt (1)
7-55: 엔터티 설계 전반 LGTM불변 필드 + 팩토리 패턴, BaseTimeEntity 상속과 IdGenerator 사용 모두 일관적입니다. codrin2의 불변 id 직접 접근 선호와도 부합합니다.
DLT 분석 시 파티션/오프셋 조회가 자주 필요합니다. 아래 제안 참고해 확장 여부 검토 바랍니다.
src/main/resources/application-dev.yaml (1)
40-42: Kafka 부트스트랩 서버 주입 OK로컬/개발 환경 변수 주입과 일치합니다.
src/main/resources/application-prod.yaml (1)
40-42: KAFKA_HOST 주입 확인 (prod/dev) src/main/resources/application-prod.yaml의 kafka.bootstrap-servers가 ${KAFKA_HOST}:9092로 외부화되어 있으며, 워크플로에 -e KAFKA_HOST가 설정되어 있습니다 (.github/workflows/deploy-prod.yaml:110, .github/workflows/deploy-dev.yaml:116).src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (1)
43-45: Kafka 역직렬화 타입 매핑·DLT 설정 확인됨buildTypeMapping()에 InventoryInsufficientEvent 매핑 포함, ErrorHandlingDeserializer/JsonDeserializer(JsonDeserializer.TRUSTED_PACKAGES="com.dh.baro", USE_TYPE_INFO_HEADERS=true) 및 JsonSerializer/TYPE_MAPPINGS, DefaultErrorHandler + DeadLetterPublishingRecoverer(FixedBackOff(1000L,3L))가 src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt에 구성되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (1)
53-63: confirmOrder가 상태 전이를 수행하지 않음 — OrderService에서 호출되어 긴급 수정 필요확인: Order.confirmOrder()는 현재 status != ORDERED 검사 후 다시 ORDERED로 설정해 사실상 상태 전이가 없습니다. OrderService.confirmOrder(orderId)에서 호출됩니다 (src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt:65).
선택지(택1):
- 생성→확정 2단계가 필요하면: 생성 시 상태를 CREATED(또는 PENDING)로 두고, Order.confirmOrder()는 status == CREATED 검사 → status = ORDERED로 전이하도록 수정(아이템 검사 유지). (수정 지점: src/main/kotlin/com/dh/baro/order/domain/Order.kt — confirmOrder).
- 생성 시 곧바로 ORDERED가 맞다면: confirmOrder 제거 또는 Deprecated 처리하고 호출부(OrderService)에서 confirm 호출 제거/대체.
임시 방편(명시적 no-op으로 표시): deprecated + no-op 패치 제안(호출부가 남아있을 때 혼동을 줄이기 위함) — 아래 패치 스니펫 적용 가능.
- fun confirmOrder() { - if (status != OrderStatus.ORDERED) { - throw IllegalStateException(ErrorMessage.ORDER_CONFIRM_INVALID_STATUS.format(status)) - } - - if (items.isEmpty()) { - throw IllegalStateException(ErrorMessage.ORDER_CONFIRM_NO_ITEMS.message) - } - - status = OrderStatus.ORDERED - } + @Deprecated("생성 시 이미 ORDERED 상태입니다. 오케스트레이션 플로우에서는 changeStatus(...) 사용을 권장합니다.") + fun confirmOrder() { + if (items.isEmpty()) { + throw IllegalStateException(ErrorMessage.ORDER_CONFIRM_NO_ITEMS.message) + } + // no-op + }정밀 수정 필요 위치: src/main/kotlin/com/dh/baro/order/domain/Order.kt (confirmOrder, ~라인 53–63) 및 호출부 src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (confirm 호출, ~라인 62–66).
🧹 Nitpick comments (2)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (2)
65-75: cancel(reason) — 멱등성(중복 이벤트)·사유 저장 미흡
- 이미 CANCELLED인 경우 예외를 던지면, 중복 취소 이벤트가 DLQ로 쌓일 수 있습니다(본 PR의 멱등성 목표와 상충). 멱등하게 조용히 반환하는 편이 안전합니다.
- 전달받은 reason이 저장/노출되지 않습니다(감사/CS 분석 곤란).
멱등성만 먼저 반영하는 최소 수정:
- if (status == OrderStatus.CANCELLED) { - throw IllegalStateException(ErrorMessage.ORDER_ALREADY_CANCELLED.format(id)) - } + if (status == OrderStatus.CANCELLED) { + // idempotent: 이미 취소된 주문 + return + }추가로 취소 사유 영속화를 권장합니다(DDL 동반 필요). 예시:
// (파일 상단 필드 선언부에 추가) @Column(name = "cancel_reason", length = 200) var cancelReason: String? = nullstatus = OrderStatus.CANCELLED + this.cancelReason = reason운영 반영 시 컬럼 추가 및 인덱스/마스킹 정책(PII 여부) 점검이 필요합니다.
91-91: 중복 기본값 설정 제거 제안(드리프트 방지)프로퍼티 기본값이 이미 ORDERED인데, 팩토리에서 다시 지정합니다. 한 곳으로 수렴해 향후 기본 상태 변경 시 드리프트를 막는 편이 좋습니다.
다음처럼 팩토리에서 status 인자를 제거하세요.
Order( id = IdGenerator.generate(), userId = userId, totalPrice = BigDecimal.ZERO, shippingAddress = shippingAddress, - status = OrderStatus.ORDERED, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/kotlin/com/dh/baro/order/domain/Order.kt(4 hunks)src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/com/dh/baro/order/domain/OrderStatus.kt
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codrin2
PR: S-BARO/server#17
File: src/main/kotlin/com/dh/baro/look/presentation/dto/LookDetailResponse.kt:38-59
Timestamp: 2025-08-12T13:52:29.449Z
Learning: codrin2 prefers direct field access for immutable id properties (val) rather than using getters, reasoning that immutable fields pose less encapsulation risk.
🧬 Code graph analysis (1)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (3)
src/main/kotlin/com/dh/baro/order/presentation/dto/OrderSummary.kt (1)
using(10-26)src/test/kotlin/com/dh/baro/order/domain/OrderServiceTest.kt (1)
order(108-110)src/main/kotlin/com/dh/baro/order/domain/service/OrderService.kt (1)
productRepository(14-67)
🔇 Additional comments (1)
src/main/kotlin/com/dh/baro/order/domain/Order.kt (1)
30-30: 초기 상태를 ORDERED로 변경 — PENDING 참조 재검증 필요 (검색 실패)리포지토리에서 제공하신 rg 스크립트 실행 결과 "No files were searched"가 반환되어 PENDING 사용처를 자동 확인하지 못했습니다.
파일: src/main/kotlin/com/dh/baro/order/domain/Order.kt — var status: OrderStatus = OrderStatus.ORDERED (생성 직후 PENDING을 기대하던 테스트/클라이언트/DB 영향 가능).조치: 리포지토리 루트에서 아래 스크립트를 실행해 PENDING 참조(코드/문자열/enum/DB)를 재검증하고, 테스트·외부계약·운영 DB 마이그레이션을 검토하세요.
#!/bin/bash set -euo pipefail # 리포지토리 루트에서 실행 — PENDING 참조 재검증용 echo "== 리포지토리 루트 ==" git rev-parse --show-toplevel 2>/dev/null || pwd echo "== 상위 파일 일부 ==" find . -maxdepth 2 -type f | head -n 50 || true echo "== rg 버전 ==" rg --version || true echo "== OrderStatus.PENDING 또는 'PENDING' 문자열 검색 (무시파일 무시, 숨김 포함) ==" rg -n --hidden -uu -S -C2 "OrderStatus\.PENDING|['\"]PENDING['\"]" || echo "== no matches for OrderStatus.PENDING or 'PENDING' ==" echo "== PENDING 전체 단어 검색 ==" rg -n --hidden -uu -S -w -C2 'PENDING' || echo "== no matches for whole-word PENDING ==" echo "== OrderStatus/enum/DB컬럼 관련 검색 (OrderStatus, order_status, ORDERED, CANCELED 등) ==" rg -n --hidden -uu -S -C2 '\bOrderStatus\b|\border_status\b|ORDERED|CANCELED|CANCELLED' || true echo "== rg가 계속 'No files were searched'를 반환하면 --debug로 원인 확인 ==" rg --debug 'PENDING' || true echo "== 완료 =="
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt (4)
46-46: 성공 로그를 error 레벨로 기록함 — info로 낮추세요정상 저장 완료 로그는 info가 적절합니다.
적용 diff:
- log.error("DLT 메시지 저장 완료: topic=$topic, eventId=$eventId") + log.info("DLT 메시지 저장 완료: topic=$topicToPersist, dltTopic=$dltTopic, eventId=$eventId")
53-68: String JSON에서 eventId를 읽지 못하는 케이스 보완String이면 valueToTree로는 문자열 노드가 되어 eventId를 못 읽습니다. readTree 분기 추가 및 경고 로그에 예외 스택을 함께 남기세요.
적용 diff:
private fun extractEventId(message: Any): Long { return try { when (message) { is Map<*, *> -> { (message["eventId"] as? Number)?.toLong() ?: 0L } + is String -> { + val jsonNode = objectMapper.readTree(message) + jsonNode.get("eventId")?.asLong() ?: 0L + } else -> { val jsonNode = objectMapper.valueToTree<com.fasterxml.jackson.databind.JsonNode>(message) jsonNode.get("eventId")?.asLong() ?: 0L } } } catch (e: Exception) { - log.warn("eventId 추출 실패, 기본값 사용: $e") + log.warn("eventId 추출 실패, 기본값 사용", e) 0L } }
21-24: 토픽/그룹 하드코딩 제거(환경별 설정화)topics/groupId를 상수/설정으로 분리하면 운영 환경 전개가 쉬워집니다(예: application-*.yml 혹은 KafkaConfig의 상수 사용).
예시:
-@KafkaListener(topics = ["order-events.DLT", "inventory-events.DLT"], groupId = "dlt-handler") +@KafkaListener(topics = [KafkaTopics.ORDER_EVENTS_DLT, KafkaTopics.INVENTORY_EVENTS_DLT], groupId = KafkaConsumerGroups.DLT_HANDLER)
48-50: 실패 시 예외만 로깅 — 메트릭/알림 연계 권장운영 가시성 강화를 위해 실패 카운터(metric: dlt_handler.persist.failed, tags: topic, consumerGroup, exceptionFqcn)를 추가하고 알림 룰에 연결하세요.
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (3)
46-52: 멱등성 단락(Return) 시 관찰 가능성 추가 권장중복 처리 스킵/동시 처리 충돌로 인한 스킵을 로그로 남기면 운영 가시성이 좋아집니다.
다음처럼 최소 로그를 추가해 주세요:
if (idempotencyService.isAlreadyProcessed(inventoryInsufficientEvent.eventId)) { - return + log.info("InventoryInsufficientEvent already processed. eventId={}", inventoryInsufficientEvent.eventId) + return } if (!idempotencyService.tryMarkAsProcessing(inventoryInsufficientEvent.eventId)) { - return + log.info("Another worker is processing event. Skipping. eventId={}", inventoryInsufficientEvent.eventId) + return }
58-60: 불필요한 save 호출 제거 제안 (JPA 더티 체킹 활용)동일 트랜잭션 내
order는 영속 상태이므로status변경만으로 커밋 시 업데이트됩니다.orderRepository.save(order)는 중복 호출일 가능성이 큽니다.- order.changeStatus(OrderStatus.CANCELLED) - orderRepository.save(order) + order.changeStatus(OrderStatus.CANCELLED)
76-79: 예외 분류(재시도 vs 비재시도) 및 DLT 가시성 강화현재 모든 예외를 재전파합니다. 재시도 가능 예외(일시적 인프라 장애 등)와 비재시도 예외(잘못된 이벤트, 주문 미존재 등)를 구분하고, DLT 전송 시 이벤트 키/헤더/원인 예외를 함께 기록·전달해 주세요.
운영 팁:
- DefaultErrorHandler에 재시도/비재시도 예외 분류, 백오프, DLT 퍼블리셔 구성
- DLT 토픽 보존/모니터링 대시보드(consume lag, 실패 레이트) 추가
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt(1 hunks)src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codrin2
PR: S-BARO/server#17
File: src/main/kotlin/com/dh/baro/look/presentation/dto/LookDetailResponse.kt:38-59
Timestamp: 2025-08-12T13:52:29.449Z
Learning: codrin2 prefers direct field access for immutable id properties (val) rather than using getters, reasoning that immutable fields pose less encapsulation risk.
🧬 Code graph analysis (1)
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (4)
src/main/kotlin/com/dh/baro/product/infra/event/InventoryEventAfterListener.kt (2)
orderService(11-25)EVENT_ASYNC_TASK_EXECUTOR(17-24)src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt (1)
inventoryEventHandler(9-29)src/main/kotlin/com/dh/baro/order/application/OrderFacade.kt (1)
userService(18-89)src/main/kotlin/com/dh/baro/order/domain/Order.kt (1)
name(11-78)
🔇 Additional comments (6)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt (1)
25-29: retryCount 하드코딩(3) 제거 — KafkaHeaders.DELIVERY_ATTEMPT 사용하드코딩된 retryCount = 3을 제거하고 @Header(KafkaHeaders.DELIVERY_ATTEMPT)에서 읽어 재시도 횟수로 반영하세요. DELIVERY_ATTEMPT는 Spring‑Kafka 2.5+에서 도입되었고, DLT에 해당 헤더가 포함되려면 container의 deliveryAttemptHeader 활성화 및 DeadLetterPublishingRecoverer가 헤더를 DLT로 복사하도록 구성되어야 합니다.
- @Header(value = KafkaHeaders.DLT_EXCEPTION_MESSAGE, required = false) exceptionMessage: String?, + @Header(value = KafkaHeaders.DLT_EXCEPTION_MESSAGE, required = false) exceptionMessage: String?, + @Header(value = KafkaHeaders.DELIVERY_ATTEMPT, required = false) deliveryAttempt: Int?, @Header(value = KafkaHeaders.DLT_ORIGINAL_CONSUMER_GROUP, required = false) consumerGroup: String? @@ - retryCount = 3, + // DELIVERY_ATTEMPT는 구현/버전에 따라 1부터 시작할 수 있습니다. 필요시 -1 보정 검토. + retryCount = (deliveryAttempt ?: 0),검증 항목(레포에서 확인 필요):
- build.gradle* / gradle/spring.gradle에서 org.springframework.kafka:spring-kafka 버전 확인(>= 2.5 권장).
- src/main/kotlin/com/dh/baro/core/config/KafkaConfig.kt에서 ConcurrentKafkaListenerContainerFactory / ContainerProperties.deliveryAttemptHeader 활성화 여부 확인.
- DeadLetterPublishingRecoverer(또는 recoverer 구성)에서 kafka_deliveryAttempt 헤더가 DLT로 복사되는지 확인.
src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (5)
68-75: 롤백 실패 시 예외 전파 + 성공 시에만 완료 마킹 — 이전 코멘트 해결됨이전 리뷰의 지적대로 실패 시 예외를 던지고 성공 후에만
markProcessingAsCompleted하는 흐름으로 수정되어 재시도/DTL 경로가 보장됩니다. 잘 반영되었습니다.
3-21: IdempotencyService import 불필요 — 동일 패키지에 선언됨
IdempotencyService는 src/main/kotlin/com/dh/baro/order/infra/IdempotencyService.kt에 package com.dh.baro.order.infra로 선언되어 있어 OrderEventListener와 동일 패키지이므로 import 추가 불필요.Likely an incorrect or invalid review comment.
33-41: 확인 완료 — ORDER_PLACED_EVENT 정의 및 라우팅 존재src/main/kotlin/com/dh/baro/core/outbox/OutboxMessageRouter.kt에 const val ORDER_PLACED_EVENT = "ORDER_PLACED"가 정의되어 있으며, 같은 파일의 route(msg: OutboxMessage) 내부 when 분기(ORDER_PLACED_EVENT -> ...)에서 payload를 OrderPlacedEvent로 역직렬화해 Kafka로 전송합니다. 추가 조치 불필요.
61-66: 매핑 필드 확인 완료 — 문제 없음OrderItem이 productId: Long, quantity: Int로 정의되어 있고 InventoryItem도 동일한 타입(productId: Long, quantity: Int)을 사용하므로 해당 매핑은 올바릅니다.
43-46: 검증 필요 — Kafka 리스너(역직렬화 · DLT · 트랜잭션)아래 항목을 확인하세요.
- 컨테이너 팩토리 / 역직렬화 — src/main/kotlin/com/dh/baro/order/infra/OrderEventListener.kt (라인 43–46)에서 @KafkaListener가 올바른 containerFactory를 참조하거나, 애플리케이션의 ConcurrentKafkaListenerContainerFactory/ConsumerFactory가 InventoryInsufficientEvent로 역직렬화되도록(JsonDeserializer 또는 ErrorHandlingDeserializer 등) 구성되어 있는지 확인.
- DLT · 에러 핸들러 — DefaultErrorHandler / DeadLetterPublishingRecoverer 또는 CommonErrorHandler 등이 등록되어 실패 메시지가 DLT로 전송되고 재시도/백오프가 적용되는지 확인.
- 트랜잭션 · 오프셋 — DB 트랜잭션과 Kafka 오프셋 커밋이 함께 롤백되도록 KafkaTransactionManager 또는 ChainedTransactionManager, producer.transaction-id-prefix 설정과 listener의 AckMode/트랜잭션 연동이 적절히 구성되어 있는지 확인.
- 참고 — idempotencyService.isAlreadyProcessed 체크가 있으나, 위 설정이 없으면 중복 처리·오프셋 불일치 문제가 남을 수 있음.
로컬에서 검색해 확인할 명령(리포지토리 루트에서 실행):
rg -n -C3 'DefaultErrorHandler|DeadLetterPublishingRecoverer|CommonErrorHandler|SeekToCurrentErrorHandler|ConcurrentKafkaListenerContainerFactory|DefaultKafkaConsumerFactory|JsonDeserializer|ErrorHandlingDeserializer|KafkaTransactionManager|ChainedKafkaTransactionManager|transaction-id-prefix|dead[- ]?letter|dlt|inventory-events' || true rg -n -C3 '@KafkaListener' src/main || true rg -n -C3 'spring.kafka|producer.transaction-id-prefix|ackMode|enableAutoCommit' -g 'application*.yml' -g 'application*.yaml' -g 'application*.properties' || true
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt (3)
40-40: 고정된 retryCount 값을 동적으로 개선하는 것을 고려하세요.현재
retryCount = 3으로 하드코딩되어 있지만, 실제 재시도 횟수는 Kafka 헤더에서 추출할 수 있습니다.다음과 같이 실제 재시도 횟수를 동적으로 추출하도록 개선할 수 있습니다:
@Transactional - @KafkaListener(topics = ["order-events.DLT", "inventory-events.DLT"], groupId = "dlt-handler") + @KafkaListener(topics = ["order-events.DLT", "inventory-events.DLT"], groupId = "dlt-handler") fun handleDltMessage( consumerRecord: ConsumerRecord<String, Any>, @Header(KafkaHeaders.RECEIVED_TOPIC) topic: String, @Header(value = KafkaHeaders.DLT_EXCEPTION_MESSAGE, required = false) exceptionMessage: String?, - @Header(value = KafkaHeaders.DLT_ORIGINAL_CONSUMER_GROUP, required = false) consumerGroup: String? + @Header(value = KafkaHeaders.DLT_ORIGINAL_CONSUMER_GROUP, required = false) consumerGroup: String?, + @Header(value = "spring_kafka_retry_attempt", required = false) retryAttempt: String? ) { try { val eventId = extractEventId(consumerRecord.value()) val payload = objectMapper.writeValueAsString(consumerRecord.value()) val errorMessage = exceptionMessage ?: "Unknown error" val group = consumerGroup ?: "unknown-group" + val actualRetryCount = retryAttempt?.toLongOrNull() ?: 3L val failedMessage = FailedMessage.create( topic = topic, eventId = eventId, payload = payload, errorMessage = errorMessage, - retryCount = 3, + retryCount = actualRetryCount, consumerGroup = group )
52-67: eventId 추출 로직의 안정성을 향상시키세요.현재 구현은 Map과 JSON 객체를 모두 처리하지만, 예외 발생 시 기본값 0L을 반환합니다. Spring Kafka에서 DLT로 전송된 메시지의 경우 원본 값이 복원되므로, 더 명확한 예외 처리가 필요합니다.
다음과 같이 더 구체적인 예외 처리와 로깅을 추가하는 것을 권장합니다:
private fun extractEventId(message: Any): Long { return try { when (message) { is Map<*, *> -> { - (message["eventId"] as? Number)?.toLong() ?: 0L + val eventId = message["eventId"] + when (eventId) { + is Number -> eventId.toLong() + is String -> eventId.toLongOrNull() ?: run { + log.warn("eventId가 숫자로 변환될 수 없습니다: $eventId") + 0L + } + null -> { + log.warn("eventId 필드가 존재하지 않습니다") + 0L + } + else -> { + log.warn("지원하지 않는 eventId 타입: ${eventId::class.simpleName}") + 0L + } + } } else -> { val jsonNode = objectMapper.valueToTree<com.fasterxml.jackson.databind.JsonNode>(message) - jsonNode.get("eventId")?.asLong() ?: 0L + jsonNode.get("eventId")?.asLong() ?: run { + log.warn("JSON에서 eventId를 찾을 수 없습니다") + 0L + } } } } catch (e: Exception) { - log.warn("eventId 추출 실패, 기본값 사용: $e") + log.warn("eventId 추출 실패, 기본값 0L 사용. 메시지 타입: ${message::class.simpleName}", e) 0L } }
45-45: 로그 메시지 개선을 고려하세요.성공 로그가
log.error로 출력되고 있어서 실제 오류와 구분하기 어렵습니다.다음과 같이 적절한 로그 레벨을 사용하는 것을 권장합니다:
- log.error("DLT 메시지 저장 완료: topic=$topic, eventId=$eventId") + log.info("DLT 메시지 저장 완료: topic=$topic, eventId=$eventId, consumerGroup=$group")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: codrin2
PR: S-BARO/server#17
File: src/main/kotlin/com/dh/baro/look/presentation/dto/LookDetailResponse.kt:38-59
Timestamp: 2025-08-12T13:52:29.449Z
Learning: codrin2 prefers direct field access for immutable id properties (val) rather than using getters, reasoning that immutable fields pose less encapsulation risk.
🔇 Additional comments (1)
src/main/kotlin/com/dh/baro/core/dlq/DltHandler.kt (1)
13-68: Spring Kafka의 DLT 처리 패턴을 잘 구현했습니다.이벤트 기반 아키텍처에서 메시지 손실 방지와 실패 분석을 위한 DLT 핸들러가 적절히 구현되었습니다. DLT 핸들러는 재처리에 실패한 메시지들의 영속화를 담당하며, Spring Kafka의 @DltHandler 어노테이션과 함께 필수 헤더들을 올바르게 추출하고 있습니다.
codrin2
changed the title
Issue Number
#40
As-Is
To-Be
이벤트 기반 시스템 구축
InventoryInsufficientEvent를 통한 주문 취소 및 재고 롤백 복구 로직 구현.기능 확장 및 리팩터링
FailedMessage엔티티 추가로 실패 메시지 영속화 및 분석 기반 마련.전체 커밋 요약
feat:V3주문 API, 재고 배치 차감, 카프카 컨슈머, 멱등성 서비스 등 핵심 기능 추가.fix: 카프카 설정 이벤트 타입 매핑 누락 수정.✅ Check List
📸 Test Screenshot
Additional Description