refactor/db-lock

[TreeStone94]

No description provided.

[claude]

PR 리뷰: refactor/db-lock

낙관적/비관적 락을 구조화하려는 방향은 좋습니다. 다만 몇 가지 버그와 설계 개선 포인트를 공유드립니다.

---

🔴 Critical: Self-invocation으로 인한 @transactional 미적용

파일: DecreaseStockWithOptimisticLockUseCase.java

// execute()에서 같은 클래스의 메서드를 직접 호출
return decreaseStock(input); // ← Spring AOP 프록시를 우회함

Spring의 트랜잭션은 AOP 프록시 기반으로 동작합니다. 같은 클래스 내에서 this.decreaseStock()을 호출하면 프록시를 거치지 않아 @Transactional이 무시됩니다. 결과적으로 낙관적 락이 실제로 동작하지 않습니다.

해결 방법: decreaseStock() 로직을 별도 클래스(예: InventoryProcessor 또는 DecreaseStockHelper)로 분리하여 외부 빈으로 주입받아 호출해야 합니다.

// 별도 클래스로 분리
@Service
public class DecreaseStockTransactionalHelper {
    @Transactional
    public Inventory decreaseStockWithOptimisticLock(Long productId) { ... }
}

// DecreaseStockWithOptimisticLockUseCase에서 주입받아 사용
private final DecreaseStockTransactionalHelper helper;

---

🟠 Major: UseCase가 UseCase를 직접 의존 (아키텍처 위반)

파일: CreateOrderUseCase.java

private final DecreaseStockWithPessimisticLockUseCase decreaseStockWithPessimisticLockUseCase;

CLAUDE.md 아키텍처 원칙에 따르면 UseCase는 Domain + Port 계층에만 의존해야 합니다. UseCase가 다른 UseCase를 직접 주입받으면 레이어 간 결합이 생기고, 락 전략을 바꿀 때마다 CreateOrderUseCase도 수정해야 하는 문제가 발생합니다.

현재 구조에서 CreateOrderUseCase가 재고 감소 책임을 갖는다면, InventoryRepository를 직접 사용하거나 공통 로직을 Processor 레이어로 추출하는 것이 적합합니다.

---

🟠 Major: output != null 검사 불필요 + 잘못된 에러 메시지

파일: CreateOrderUseCase.java

if (output != null) {
    ...
} else {
    throw new IllegalArgumentException("Product not found"); // ← 실제로는 재고 문제인데 메시지가 틀림
}

DecreaseStockWithPessimisticLockUseCase.execute()는 절대 null을 반환하지 않습니다. 재고가 없으면 내부에서 예외를 던집니다. 따라서 이 null 체크는 의미가 없고, else 블록의 "Product not found" 메시지도 실제 상황(재고 없음)과 맞지 않습니다.

// 개선: null 체크 제거
DecreaseStockWithPessimisticLockUseCase.Output output = decreaseStockWithPessimisticLockUseCase.execute(
    new DecreaseStockWithPessimisticLockUseCase.Input(input.productId())
);
User user = userRepository.findById(input.userId)
    .orElseThrow(() -> new IllegalArgumentException("User not found"));
orderRepository.save(Order.builder().product(output.product()).user(user).build());

---

🟡 Minor: 주석 처리된 코드 정리 필요

파일: CreateOrderUseCase.java

미사용 import 3개와 주석 처리된 코드 블록이 남아 있습니다. 실험용 코드라면 삭제하거나, 전략 비교를 문서화하는 목적이라면 별도 문서나 테스트로 관리하는 것이 낫습니다. 주석으로 남긴 코드는 가독성을 떨어뜨립니다.

---

🟡 Minor: @Version 초기값

파일: Inventory.java

@Builder.Default
private Long version = 1L; // ← 관례적으로 0L 또는 null 사용

JPA @Version 필드는 관례상 0L로 시작합니다. 1L로 시작하면 기능은 동작하지만, 버전 값의 의미가 불명확해집니다(저장 전인데 이미 version=1). 0L 또는 아예 초기값 없이 null로 두는 것이 권장됩니다.

---

🟡 Minor: Thread.sleep()으로 인한 웹 스레드 블로킹

파일: DecreaseStockWithOptimisticLockUseCase.java

Thread.sleep(50L * retryCount); // 서블릿 스레드를 최대 150ms 블로킹

이 프로젝트의 목표가 1,000+ 동시 요청 처리인 점을 고려하면, 요청 스레드를 직접 sleep시키는 방식은 스레드 풀 고갈을 유발할 수 있습니다. 낙관적 락 재시도에서 Thread.sleep을 사용한다면 재시도 횟수와 대기 시간을 신중히 설계해야 합니다.

또한 MAX_RETRY_COUNT=3은 1,000명이 동시에 경쟁하는 환경에서는 대부분의 요청이 실패할 수 있습니다. 비관적 락 대비 낙관적 락은 경합이 적은 상황에 더 유리하다는 점도 고려해 주세요.

---

🟡 Minor: RuntimeException 미처리

파일: GlobalExceptionHandler.java

DecreaseStockWithOptimisticLockUseCase가 최대 재시도 초과 시 RuntimeException을 던지는데, GlobalExceptionHandler에는 이를 처리하는 핸들러가 없습니다. 현재는 500 에러로 노출됩니다. 재시도 초과용 커스텀 예외나 핸들러를 추가하는 것을 권장합니다.

---

🔵 테스트 없음

동시성 제어는 테스트가 가장 중요한 영역 중 하나입니다. 현재 단위 테스트와 동시성 통합 테스트(e.g., CountDownLatch, ExecutorService를 활용한 동시 요청 시뮬레이션)가 없습니다. 적어도 다음 케이스는 테스트로 검증하면 좋겠습니다:

• 재고 0일 때 예외 발생 여부
• 동시 요청 시 재고 음수 방지 (낙관적/비관적 락 각각)
• 낙관적 락 재시도 로직 동작 여부

---

전반적으로 DB 락 전략의 구조를 UseCase로 분리한 접근은 방향이 맞습니다. Self-invocation 버그와 아키텍처 의존성 방향 문제를 우선 해결하시면 좋을 것 같습니다.

[claude]

코드 리뷰

DB 락 전략(낙관적/비관적)을 단계별로 도입하고 GlobalExceptionHandler를 추가한 좋은 시도입니다. 몇 가지 버그와 개선 포인트를 남깁니다.

---

🔴 Critical: 트랜잭션 분리로 인한 데이터 정합성 문제

OrderController.java

// 재고 감소 (별도 트랜잭션 - 커밋됨)
DecreaseStockWithOptimisticLockUseCase.Output output = decreaseStockWithOptimisticLockUseCase.execute(...);

// 주문 생성 (별도 트랜잭션)
createOrderUseCase.execute(createOrderUseCaseInput);

재고 감소와 주문 생성이 각각 별개의 트랜잭션으로 동작합니다. 재고 감소가 커밋된 후 주문 생성이 실패하면, 재고는 줄었는데 주문 기록은 없는 상황이 발생합니다.

이 두 작업은 하나의 트랜잭션 안에서 처리되어야 합니다. CreateOrderUseCase 내부에서 재고 감소까지 함께 처리하거나, 상위에서 트랜잭션을 묶는 구조가 필요합니다.

---

🔴 Critical: 미사용 빈 주입 + 주석 처리된 코드

OrderController.java

private final DecreaseStockUseCase decreaseStockUseCase;                         // 실제로 사용 안 함
private final DecreaseStockWithOptimisticLockUseCase decreaseStockWithOptimisticLockUseCase;
private final DecreaseStockWithPessimisticLockUseCase decreaseStockWithPessimisticLockUseCase; // 실제로 사용 안 함

주석 처리된 코드(// DecreaseStockUseCase.Output output = ..., // DecreaseStockWithPessimisticLockUseCase.Output output = ...)와 함께, 사용하지 않는 빈 2개가 그대로 주입되어 있습니다. 주석 코드는 코드베이스에 남기지 않는 것이 원칙입니다. 전략 선택이 필요하다면 프로파일 또는 설정 값으로 분기하는 것이 깔끔합니다.

---

🟡 Warning: RuntimeException을 400으로 처리하는 것은 부적절

GlobalExceptionHandler.java

@ExceptionHandler(RuntimeException.class)
public ResponseEntity<String> handleRuntimeException(RuntimeException e) {
    return ResponseEntity.badRequest().body(e.getMessage());  // 400
}

400 Bad Request는 클라이언트의 잘못된 요청을 의미합니다. RuntimeException은 서버 내부 오류를 포함하는 매우 광범위한 예외입니다. 최대 재시도 초과 같은 서버 측 실패는 500 Internal Server Error로 내려야 합니다.

또한, DecreaseStockWithOptimisticLockUseCase에서 던지는 RuntimeException("재고 감소 실패: 최대 재시도 횟수 초과")도 커스텀 예외(예: StockDecreaseFailedException)로 분리하고, 핸들러에서 적절한 HTTP 상태 코드(409 Conflict 또는 503 Service Unavailable)를 반환하는 것이 더 명확합니다.

---

🟡 Warning: 도달 불가능한 코드

DecreaseStockWithOptimisticLockUseCase.java

throw new IllegalStateException("도달할 수 없는 코드");

컴파일러 오류를 피하기 위해 추가한 코드로 보입니다. while 루프 안에서 항상 return 또는 예외로 빠져나오기 때문에 실제로는 도달하지 않지만, 이런 패턴은 코드 가독성을 해칩니다.

재시도 로직을 아래처럼 구조화하면 이 코드를 제거할 수 있습니다:

public Output execute(Input input) {
    for (int retry = 0; retry < MAX_RETRY_COUNT; retry++) {
        try {
            return new Output(inventoryProcessor.decreaseStockWithOptimisticLock(input.productId()).getProduct());
        } catch (ObjectOptimisticLockingFailureException e) {
            if (retry == MAX_RETRY_COUNT - 1) {
                throw new RuntimeException("재고 감소 실패: 최대 재시도 횟수 초과", e);
            }
            try {
                Thread.sleep(50L * (retry + 1));
            } catch (InterruptedException ie) {
                Thread.currentThread().interrupt();
                throw new RuntimeException("재시도 중 인터럽트 발생", ie);
            }
        }
    }
    throw new IllegalStateException("unreachable"); // 이제 진짜 도달 불가
}

---

🟡 Warning: @Lock(LockModeType.OPTIMISTIC) 명시적 선언 불필요할 수 있음

InventoryRepository.java

@Lock(LockModeType.OPTIMISTIC)
@Query("SELECT i FROM Inventory i WHERE i.product.id = :productId")
Optional<Inventory> findByProductIdWithOptimisticLock(Long productId);

Inventory 엔티티에 @Version 필드가 이미 있으면, JPA는 해당 엔티티를 조회할 때 자동으로 낙관적 락을 적용합니다. @Lock(OPTIMISTIC)을 명시적으로 지정하는 것은 대부분의 경우 중복입니다(OPTIMISTIC_FORCE_INCREMENT가 필요한 경우는 별개). 의도를 문서화하는 목적이라면 주석으로 설명하는 것이 더 명확합니다.

---

🟡 Warning: CreateOrderUseCase.Input에 도메인 엔티티 직접 전달

CreateOrderUseCase.java

public record Input(@NotNull Long userId, @NotNull Product product) {}

UseCase의 Input에 JPA 관리 엔티티(Product)를 직접 담으면, 해당 엔티티의 영속성 컨텍스트 상태에 따라 예상치 못한 동작이 생길 수 있습니다. productId를 전달하고 UseCase 내부에서 조회하거나, 트랜잭션을 하나로 합치는 방향을 권장합니다.

---

🟢 Minor: 에러 메시지 불일치

DecreaseStockUseCase와 DecreaseStockWithOptimisticLockUseCase에서는 "재고를 찾을 수 없습니다", DecreaseStockWithPessimisticLockUseCase에서는 "재고를 찾을 수 없습니다." (마침표 포함)로 일관성이 없습니다.

---

🟢 Minor: 테스트 없음

동시성 제어 전략의 핵심은 실제로 동시 요청이 들어왔을 때 올바르게 동작하는지를 검증하는 것입니다. 현재 PR에는 테스트가 전혀 없습니다. 최소한 아래 케이스에 대한 테스트가 있으면 좋겠습니다:

• 낙관적 락: 동시 요청 시 버전 충돌 발생 후 재시도 성공 시나리오
• 비관적 락: 순차 처리로 재고 음수 방지 시나리오
• 재시도 횟수 초과 시 예외 발생 시나리오

---

잘된 점

• InventoryProcessor를 통해 낙관적 락의 트랜잭션 경계를 올바르게 분리한 점 (재시도 시 매번 새 트랜잭션 생성)
• CreateOrderBody 분리로 Controller의 path variable + body 조합 처리가 명확해진 점
• 백오프 전략(점진적 대기시간)을 적용한 점

[claude]

PR 리뷰: refactor/db-lock

전반적으로 DB 락 전략을 구체적으로 구현하려는 방향은 좋습니다. 다만 몇 가지 중요한 문제들이 있어 피드백 드립니다.

---

🔴 치명적 버그

1. 낙관적 락 재시도 로직이 실제로 동작하지 않음

가장 심각한 문제입니다.

OrderProcessor.createOrderWithStockDecrease()는 @Transactional을 가지고 있고, 그 안에서 DecreaseStockWithOptimisticLockUseCase를 호출합니다. InventoryProcessor.decreaseStockWithOptimisticLock()도 @Transactional이 있지만 기본 전파 방식(REQUIRED)이기 때문에 외부 트랜잭션(OrderProcessor)에 참여합니다.

낙관적 락의 ObjectOptimisticLockingFailureException은 트랜잭션이 커밋되는 시점에 발생합니다. 즉:

OrderProcessor.createOrderWithStockDecrease() 시작 → 트랜잭션 A 시작
  → InventoryProcessor.decreaseStockWithOptimisticLock() 실행 (트랜잭션 A에 참여)
  → CreateOrderUseCase.execute() 실행 (트랜잭션 A에 참여)
  → 트랜잭션 A 커밋 시점에 OptimisticLockingFailureException 발생
     ↑ 이미 DecreaseStockWithOptimisticLockUseCase의 try-catch 블록 밖

DecreaseStockWithOptimisticLockUseCase의 재시도 로직은 절대 실행되지 않습니다.

해결 방향: InventoryProcessor에서 Propagation.REQUIRES_NEW를 사용하거나, 재시도 로직이 있는 UseCase가 트랜잭션 컨텍스트 바깥에서 실행되어야 합니다.

// InventoryProcessor
@Transactional(propagation = Propagation.REQUIRES_NEW) // 매 재시도마다 새 트랜잭션
public Inventory decreaseStockWithOptimisticLock(Long productId) { ... }

---

🟠 설계 문제

2. CLAUDE.md 아키텍처 원칙 위반 - Processor 조기 생성

CLAUDE.md: "3개 이상의 UseCase에서 검증된 중복이 있을 때만 도입. 절대 미리 만들지 말 것"

• OrderProcessor: 단 2개의 UseCase(DecreaseStockWithOptimisticLockUseCase + CreateOrderUseCase)를 조합하기 위해 생성됨
• InventoryProcessor: 현재 DecreaseStockWithOptimisticLockUseCase 한 곳에서만 사용됨

이 정도 조합은 Processor 없이 UseCase 내부나 Controller에서 직접 처리해도 충분합니다.

3. GlobalExceptionHandler의 RuntimeException 처리가 잘못됨

@ExceptionHandler(RuntimeException.class)
public ResponseEntity<String> handleRuntimeException(RuntimeException e) {
    return ResponseEntity.badRequest().body(e.getMessage()); // 400 반환
}

RuntimeException의 모든 하위 타입이 클라이언트 오류(400)는 아닙니다. DB 연결 실패, NPE, 재고 감소 실패 등이 모두 400으로 반환됩니다. 재고 감소 재시도 초과 시 던지는 RuntimeException("재고 감소 실패: 최대 재시도 횟수 초과")도 마찬가지입니다 — 이건 서버 측 처리 실패이므로 409 Conflict나 503 Service Unavailable이 더 적합합니다.

최소한 RuntimeException 핸들러는 500으로 변경하거나, 도메인 전용 예외 클래스(예: OutOfStockException, StockDecreaseFailedException)를 도입해 각각 적절한 HTTP 상태코드를 반환해야 합니다.

4. CreateOrderUseCase.Input에 도메인 객체 직접 전달

public record Input(@NotNull Long userId, @NotNull Product product) {}

Product 도메인 객체를 Input으로 받으면 UseCase 간 결합도가 높아집니다. productId를 받아서 UseCase 내부에서 직접 로딩하거나, 영속성 컨텍스트 문제를 인지하고 의도적으로 설계한 것이라면 주석으로 이유를 명시해주세요.

---

🟡 코드 품질

5. 데드 코드

다음 두 클래스가 추가되었으나 어디서도 사용되지 않습니다:

• DecreaseStockUseCase — 락 없는 재고 감소. 실수로 호출될 경우 동시성 문제 재발
• DecreaseStockWithPessimisticLockUseCase — 비관적 락 버전

학습/비교 목적이라면 주석으로 명확히 표시하거나, 실제로 사용하는 엔드포인트를 함께 추가해주세요.

6. @Lock(LockModeType.OPTIMISTIC) 불필요

@Lock(LockModeType.OPTIMISTIC)
@Query("SELECT i FROM Inventory i WHERE i.product.id = :productId")
Optional<Inventory> findByProductIdWithOptimisticLock(Long productId);

Inventory에 @Version 필드가 있으면 이미 낙관적 락이 자동으로 동작합니다. @Lock(LockModeType.OPTIMISTIC)은 명시적으로 추가해도 동작은 같으며 혼란만 줄 수 있습니다. 오히려 기존 findByProductId()를 그대로 사용해도 낙관적 락이 적용됩니다.

7. 에러 메시지 불일치

• DecreaseStockWithPessimisticLockUseCase: "재고를 찾을 수 없습니다." (마침표 있음)
• InventoryProcessor, DecreaseStockUseCase: "재고를 찾을 수 없습니다" (마침표 없음)

일관성을 맞춰주세요.

8. @NotNull 검증이 UseCase 내부에서 동작하지 않음

public record Input(@NotNull Long productId) {}

jakarta.validation.constraints.NotNull은 @Validated 없이는 UseCase 내부에서 검증이 실행되지 않습니다. Controller에서 @Valid로 처리되는 DTO만 실제로 검증됩니다. UseCase의 @NotNull은 문서화 목적으로만 동작합니다.

9. Thread.sleep 백오프 전략

Thread.sleep(50L * retryCount); // 50ms, 100ms, 150ms

고부하 상황에서 수백~수천 개의 스레드가 동시에 sleep 상태가 되면 스레드 풀 고갈로 이어질 수 있습니다. 백오프 전략은 의도는 좋지만 선형 백오프보다 지수 백오프(exponential backoff) + jitter 조합이 일반적으로 더 효과적입니다.

---

요약

우선순위	항목
🔴 즉시 수정	낙관적 락 재시도 로직 트랜잭션 문제 (실제로 동작 안 함)
🟠 수정 권장	GlobalExceptionHandler RuntimeException → 500, Processor 조기 추상화
🟡 개선 고려	데드 코드 정리, 불필요한 @Lock 제거, 에러 메시지 통일

[claude]

PR 리뷰: refactor/db-lock

전반적으로 비관적 락(Pessimistic Lock)과 낙관적 락(Optimistic Lock) 전략을 도입하려는 방향은 좋습니다. 다만 몇 가지 중요한 버그와 개선 사항이 있어 공유드립니다.

---

🔴 Critical Bug

1. CreateOrderWithOptimisticLockUseCase가 실제로는 비관적 락을 사용하고 있음

클래스 이름은 Optimistic Lock이라고 되어 있지만, 내부에서는 findByProductIdWithPessimisticLock을 호출하고 있습니다.

// CreateOrderWithOptimisticLockUseCase.java:25
Inventory inventory = inventoryRepository.findByProductIdWithPessimisticLock(input.productId())

이름이 약속하는 것과 구현이 완전히 다릅니다. 두 전략이 모두 비관적 락을 쓰고 있어, 낙관적 락 구현이 실질적으로 없는 상태입니다.

수정 방향: findByProductIdWithOptimisticLock으로 변경하고, 낙관적 락 충돌 시 발생하는 ObjectOptimisticLockingFailureException을 처리하는 재시도 로직(retry)이 필요합니다.

---

🟠 Major Issues

2. findByProductIdWithOptimisticLock이 실제로 사용되지 않음

Repository에 메서드를 추가했지만, 어떤 UseCase에서도 호출하지 않습니다. 위 버그(#1)와 직결된 문제입니다.

3. GlobalExceptionHandler에서 503 상태코드 오용

@ExceptionHandler(RuntimeException.class)
public ResponseEntity<String> handleRuntimeException(RuntimeException e) {
    return ResponseEntity.status(503).body(e.getMessage());
}

503 (Service Unavailable)은 서버가 일시적으로 요청을 처리할 수 없는 인프라/가용성 문제에 사용해야 합니다. 일반적인 RuntimeException에 503을 반환하면 클라이언트가 잘못된 신호를 받게 됩니다. 낙관적 락 충돌(OptimisticLockException)의 경우 409 Conflict 또는 429 Too Many Requests가 더 적합합니다.

4. Controller가 CreateOrderWithPessimisticLockUseCase를 전혀 노출하지 않음

// OrderController.java
private final CreateOrderWithOptimisticLockUseCase createOrderWithOptimisticLockUseCase;

CreateOrderWithPessimisticLockUseCase는 만들어졌지만 컨트롤러에 연결되지 않아 사용할 방법이 없습니다. 두 전략을 비교 테스트하려는 프로젝트 목적에 맞지 않습니다.

---

🟡 Minor Issues

5. 예외 메시지 언어 불일치

같은 파일 내에서 한국어와 영어가 혼용되고 있습니다.

.orElseThrow(() -> new IllegalArgumentException("재고를 찾을 수 없습니다"));
.orElseThrow(() -> new IllegalArgumentException("User not found"));

프로젝트 전체적으로 언어를 통일하는 것이 좋습니다.

6. 예외 메시지가 클라이언트에 그대로 노출됨 (보안)

return ResponseEntity.badRequest().body(e.getMessage());

내부 예외 메시지를 그대로 응답 body로 반환하면, 내부 구현 세부사항이 외부에 노출될 수 있습니다. 특히 RuntimeException 핸들러는 예상치 못한 내부 오류 메시지까지 클라이언트에 노출할 수 있습니다. 공통 에러 응답 DTO를 사용하는 것을 권장합니다.

public record ErrorResponse(String code, String message) {}

7. IllegalStateException을 400으로 처리하는 것이 적절한지 재검토 필요

IllegalStateException은 재고 부족처럼 409 Conflict가 더 적절한 시나리오에서 발생할 수 있습니다. 단순히 400으로 묶기보다, 도메인 전용 예외 클래스(OutOfStockException 등)를 만들어 상황에 맞는 HTTP 상태코드를 반환하는 것이 더 명확합니다.

---

✅ 잘 된 점

• @Version 필드를 Inventory에 추가하여 낙관적 락의 기반을 마련한 것
• Optional.isPresent() 분기 대신 orElseThrow()로 리팩토링하여 코드가 간결해진 것
• GlobalExceptionHandler를 common/exception 패키지에 배치한 구조
• Input 파라미터 순서를 (userId, productId)로 통일한 것

---

요약 체크리스트

• CreateOrderWithOptimisticLockUseCase에서 findByProductIdWithOptimisticLock 사용
• 낙관적 락 충돌에 대한 재시도 로직 추가
• GlobalExceptionHandler의 503 상태코드 수정
• CreateOrderWithPessimisticLockUseCase를 컨트롤러에 연결 (또는 endpoint 분리)
• 에러 메시지 언어 통일
• 테스트 코드 추가 (두 락 전략의 동시성 테스트)

[claude]

PR 리뷰: refactor/db-lock

비관적 락 / 낙관적 락을 분리하고 글로벌 예외 핸들러를 추가한 방향은 좋습니다. 다만 몇 가지 개선이 필요한 부분이 있어 피드백 드립니다.

---

🔴 주요 이슈

1. 낙관적 락에 재시도(Retry) 로직 없음 — CreateOrderWithOptimisticLockUseCase

낙관적 락의 핵심 동작 방식은 충돌 감지 후 재시도입니다. 현재 구현에서는 ObjectOptimisticLockingFailureException이 발생하면 그대로 409 에러가 클라이언트에 전달됩니다. 이는 낙관적 락의 이점을 살리지 못합니다.

재시도를 아예 지원하지 않을 것이라면, 클라이언트에게 '재시도가 필요하다'는 명확한 응답(예: 409 + 안내 메시지)은 있어야 하지만, 근본적으로는 Spring Retry의 @retryable을 활용한 자동 재시도 구현을 권장합니다.

---

2. GlobalExceptionHandler에서 RuntimeException을 너무 광범위하게 409로 처리

현재 코드:

• IllegalArgumentException → 400
• IllegalStateException → 400
• RuntimeException → 409 (모두!)

RuntimeException 하위에는 NullPointerException, DataIntegrityViolationException, TransactionSystemException 등 수백 가지 예외가 있습니다. 이들이 모두 409로 처리되는 건 잘못된 시맨틱입니다.

의도가 낙관적 락 충돌 처리라면 다음과 같이 명시적으로 분리해야 합니다:

• ObjectOptimisticLockingFailureException → 409 Conflict
• 그 외 예상치 못한 RuntimeException → 500 Internal Server Error

---

🟡 개선 권장 사항

3. 예외 메시지를 클라이언트에 직접 노출 — 보안 주의

e.getMessage()를 그대로 응답 body로 반환하면, 운영 환경에서 내부 구현 세부사항이 노출될 수 있습니다. 표준 에러 응답 객체(ErrorResponse record 등)를 별도로 만들어 사용하는 것을 권장합니다.

---

4. 사용자 조회 순서 문제 — CreateOrderWithOptimisticLockUseCase (PessimisticLock도 동일)

현재 순서:

1. 재고 조회
2. 재고 감소 (decreaseStockQuantity)
3. 사용자 조회 ← 여기서 실패하면?

사용자가 존재하지 않을 경우, 이미 재고를 감소시킨 후 예외가 발생합니다. 트랜잭션 롤백으로 정합성 문제는 없지만, 불필요한 DB 작업을 줄이기 위해 사용자 검증을 먼저 수행하는 것이 좋습니다.

권장 순서: 사용자 조회 → 재고 조회 → 재고 감소

---

5. @lock(LockModeType.OPTIMISTIC) 중복 여부

Inventory 엔티티에 @Version 필드가 이미 존재하면, JPA는 자동으로 낙관적 락을 적용합니다. @lock(LockModeType.OPTIMISTIC)은 기술적으로 불필요합니다. 메서드명에서 이미 의도가 드러나므로 어노테이션을 제거해도 무방합니다.

---

6. URL에 구현 세부사항 노출

• POST /order/with-optimistic-lock
• POST /order/with-pessimistic-lock

학습/실험 목적의 프로젝트에서는 이해할 수 있지만, 실제 API 설계에서 내부 구현 방식(어떤 락을 사용하는지)이 URL에 드러나는 것은 적절하지 않습니다.

---

🟢 잘된 점

• 기존 if (inventory.isPresent()) 패턴을 orElseThrow()로 깔끔하게 개선한 점
• 에러 메시지를 한국어로 통일한 점
• Input 필드 순서를 userId, productId로 두 UseCase 간 일관되게 맞춘 점
• @Version 필드에 @Builder.Default를 붙여 Lombok 빌더와의 호환성을 챙긴 점

---

📝 테스트 커버리지

이번 PR에 테스트가 포함되지 않았습니다. 특히 동시성 시나리오가 핵심인 기능이므로, 아래 케이스들에 대한 테스트가 있으면 더 좋겠습니다:

• 낙관적 락 충돌 발생 시 예외 처리 확인
• 비관적 락으로 재고 감소가 정확히 동작하는지 확인
• 재고가 없을 때 주문 거부 확인

[TreeStone94]

@claude 사용자 조회 순서 문제 변경해줘

[claude]

Claude finished @TreeStone94's task in 1m 20s —— View job

---

사용자 조회 순서 변경 완료

• 작업 파악
• 현재 코드 분석
• CreateOrderWithOptimisticLockUseCase 수정
• CreateOrderWithPessimisticLockUseCase 수정
• 커밋 & 푸시

두 UseCase 모두 사용자 조회 순서를 재고 조회 앞으로 변경했습니다.

변경 전:

재고 조회 → 재고 감소 → 사용자 조회 → 주문 저장

변경 후:

사용자 조회 → 재고 조회 → 재고 감소 → 주문 저장

존재하지 않는 사용자로 요청 시, 재고에 불필요한 DB 작업(조회 + 락)을 하기 전에 빠르게 실패합니다. 트랜잭션 롤백으로 데이터 정합성은 기존에도 보장되었지만, 이제 불필요한 DB 작업을 줄일 수 있습니다.

[claude]

PR 리뷰: refactor/db-lock

낙관적/비관적 락 구현 전반적으로 방향성은 좋습니다. 다만 몇 가지 수정이 필요한 사항들을 공유드립니다.

---

Critical: @EnableRetry 누락

@retryable이 동작하려면 반드시 @EnableRetry가 설정되어 있어야 합니다. 현재 ShopApiApplication 이나 별도 설정 클래스 어디에도 @EnableRetry가 없기 때문에, @retryable과 @recover는 완전히 무시되고 ObjectOptimisticLockingFailureException 발생 시 재시도 없이 바로 예외가 클라이언트에게 전달됩니다.

ShopApiApplication.java 또는 별도 Config 클래스에 아래를 추가해야 합니다:

@SpringBootApplication
@EnableRetry
public class ShopApiApplication { ... }

---

Warning: @retryable + @transactional 동일 클래스 사용 시 프록시 순서 주의

CreateOrderWithOptimisticLockUseCase에 @transactional(클래스 레벨)과 @retryable(메서드 레벨)이 함께 선언되어 있습니다. Spring Retry의 기본 Advisor order(1)가 @transactional의 기본 order보다 높아 현재는 우연히 올바르게 동작할 수 있지만, 이는 암묵적인 설정에 의존하는 불안정한 구조입니다.

권장 패턴: 트랜잭션 처리 메서드를 별도 빈으로 분리하여 명시적으로 프록시 순서를 보장하는 것이 안전합니다.

// 재시도 담당 (outer)
@Slf4j @Service @RequiredArgsConstructor
public class CreateOrderWithOptimisticLockUseCase {
    private final OrderTransactionService orderTransactionService;

    @Retryable(retryFor = ObjectOptimisticLockingFailureException.class, maxAttempts = 3, backoff = @Backoff(delay = 100))
    public void execute(Input input) {
        orderTransactionService.createOrder(input);
    }

    @Recover
    public void recover(ObjectOptimisticLockingFailureException e, Input input) { ... }
}

// 트랜잭션 담당 (별도 빈)
@Service @RequiredArgsConstructor @Transactional
class OrderTransactionService {
    public void createOrder(CreateOrderWithOptimisticLockUseCase.Input input) { ... }
}

---

Warning: @recover 메서드에도 @transactional 적용됨

클래스 레벨에 @transactional이 선언되어 있어 recover() 메서드도 트랜잭션 내에서 실행됩니다. recover()는 DB 작업을 하지 않고 예외만 던지므로 불필요한 트랜잭션이 열립니다. @transactional(propagation = Propagation.NOT_SUPPORTED)를 recover()에 추가하거나, 위의 분리 패턴을 적용하면 해결됩니다.

---

Warning: GlobalExceptionHandler - RuntimeException 범용 핸들러 문제

@ExceptionHandler(RuntimeException.class)
public ResponseEntity<String> handleRuntimeException(RuntimeException e) {
    return ResponseEntity.status(409).body(e.getMessage());
}

RuntimeException을 catch-all로 사용하면 예상치 못한 예외(NPE, ClassCastException 등)도 전부 409로 응답하게 됩니다. 의도하지 않은 예외는 500으로 응답해야 정상입니다. 낙관적 락 재시도 실패를 위한 전용 예외 클래스를 만드는 것이 좋습니다.

public class OptimisticLockRetryExhaustedException extends RuntimeException {
    public OptimisticLockRetryExhaustedException(String message) { super(message); }
}

// recover()에서:
throw new OptimisticLockRetryExhaustedException("잠시 후 다시 시도해주세요.");

// GlobalExceptionHandler에서:
@ExceptionHandler(OptimisticLockRetryExhaustedException.class)
public ResponseEntity<String> handleOptimisticLockRetryExhausted(OptimisticLockRetryExhaustedException e) {
    return ResponseEntity.status(409).body(e.getMessage());
}

---

Minor: 에러 메시지 불일치

두 UseCase 사이에 동일한 상황의 에러 메시지가 다릅니다.

UseCase	에러 메시지
CreateOrderWithOptimisticLockUseCase	"재고를 찾을 수 없습니다" (마침표 없음)
CreateOrderWithPessimisticLockUseCase	"재고를 찾을 수 없습니다." (마침표 있음)

또한 StockQuantity.decreaseStockQuantity()의 예외 메시지는 영어("Stock quantity cannot be less than zero")인데, 나머지는 한국어입니다. 일관성 있게 통일해 주세요.

---

Minor: @lock(LockModeType.OPTIMISTIC) 명시 의미 재검토

Inventory에 @Version이 선언된 이상 JPA는 기본적으로 낙관적 락을 적용합니다. findByProductIdWithOptimisticLock에 @lock(LockModeType.OPTIMISTIC)을 추가하는 것은 기능적으로는 문제없지만 중복입니다. 만약 낙관적 락 + 강제 버전 증가가 필요하다면 OPTIMISTIC_FORCE_INCREMENT를 사용하고, 그렇지 않다면 제거해도 무방합니다. 의도를 명확히 주석으로 남겨주면 더 좋겠습니다.

---

Minor: 동시성 테스트 부재

이 PR의 핵심 목적이 고동시성 처리인데, 이를 검증하는 테스트가 없습니다. CountDownLatch 또는 ExecutorService를 활용한 동시성 통합 테스트를 추가하면 실제 락 동작을 검증하고 회귀를 방지할 수 있습니다.

---

잘된 점

• 비관적/낙관적 락을 별도 UseCase로 명확하게 분리한 것은 CLAUDE.md의 SRP 원칙에 잘 맞습니다.
• GlobalExceptionHandler 추가로 예외 처리를 중앙화한 것은 좋은 방향입니다.
• 사용자 검증을 재고 조회 이전에 수행하도록 순서를 최적화한 것도 좋습니다.
• @Builder.Default를 활용한 version 필드 초기화는 적절합니다.