Conversation
Walkthrough사용자 인증 기반 PATCH /api/v1/inventory/{inventoryId} 엔드포인트와 서비스/도메인/DTO/테스트를 추가하여 재고의 선택적(부분) 수동 변경을 처리하도록 구현했습니다. 또한 성공/오류 enum에 관련 상수들을 추가했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client as 사용자
participant Controller as InventoryController
participant Service as InventoryService
participant Context as UserContextProvider
participant Repo as InventoryRepository
participant Domain as Inventory
Client->>Controller: PATCH /api/v1/inventory/{id} + UpdateInventoryRequest
Controller->>Service: updateInventory(id, request, userId)
Service->>Context: resolve(userId)
Context-->>Service: UserStoreContext
rect rgb(220,240,255)
Note over Service,Repo: 접근 제어 검사
Service->>Repo: findInventoryByIdAndValidateAccess(id, storeId)
Repo-->>Service: Inventory or throw ACCESS_DENIED
end
rect rgb(240,220,255)
Note over Service: 상품 일치 검증
Service->>Service: validateProductMatch(inventory.productId, request.productId)
alt mismatch
Service-->>Controller: throw INVENTORY_PRODUCT_MISMATCH
end
end
rect rgb(240,255,220)
Note over Service,Domain: 부분 업데이트 수행
Service->>Domain: updateManually(displayStock, warehouseStock, ...)
Domain-->>Domain: null-safe 필드 적용
end
Service->>Repo: save(inventory)
Repo-->>Service: saved Inventory
Service->>Service: InventoryResponse.from(inventory)
Service-->>Controller: InventoryResponse
Controller-->>Client: ApiResponse(200, data, UPDATE_INVENTORY_SUCCESS)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 특히 검토할 항목:
권장 참고문서: Spring Validation ( 짧고 위트 있는 칭찬: 잘했어요 — 작은 변화로도 안전 경계를 잘 챙겼습니다. 🎉 Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/test/java/com/almang/inventory/inventory/service/InventoryServiceTest.java (1)
96-145: 테스트 커버리지가 우수합니다! 👏성공 케이스와 주요 예외 케이스들이 잘 테스트되고 있습니다.
BigDecimal비교에isEqualByComparingTo를 사용한 것도 정확합니다.다음 엣지 케이스에 대한 테스트도 추가하면 더욱 견고해집니다:
- 부분 업데이트 테스트: 일부 필드만 업데이트하는 경우 (예: displayStock만 업데이트하고 나머지는 null)
- 경계값 테스트: 0 값 업데이트, 매우 큰 값 업데이트
- reorderTriggerPoint 범위 검증: 0 미만 또는 1 초과 값에 대한 예외 처리
예시 테스트:
@Test void 재고_수동_수정시_일부_필드만_업데이트할_수_있다() { // given Store store = newStore("부분수정상점"); User user = newUser(store, "partialUser"); Vendor vendor = newVendor(store, "발주처"); Product product = newProduct(store, vendor, "상품", "P001"); inventoryService.createInventory(product); Inventory inventory = inventoryRepository.findByProduct_Id(product.getId()).orElseThrow(); BigDecimal originalWarehouse = inventory.getWarehouseStock(); BigDecimal newDisplay = BigDecimal.valueOf(5.0); UpdateInventoryRequest request = new UpdateInventoryRequest( product.getId(), newDisplay, // displayStock만 업데이트 null, // warehouseStock은 유지 null, null, null ); // when InventoryResponse response = inventoryService.updateInventory(inventory.getId(), request, user.getId()); // then assertThat(response.displayStock()).isEqualByComparingTo(newDisplay); assertThat(response.warehouseStock()).isEqualByComparingTo(originalWarehouse); }src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java (1)
7-7: productId 필드의 필요성을 재검토해보세요.
inventoryId로 이미 재고를 특정하는데, 요청 바디에productId를 추가로 요구하고 서비스에서 일치 여부를 검증하는 설계가 다소 복잡해 보입니다.현재 구조의 장점:
- 클라이언트가 잘못된 재고를 수정하는 것을 방지 (이중 확인)
- 명시적인 의도 확인
대안 고려사항:
productId없이inventoryId만으로 업데이트- 응답에
productId를 포함하여 클라이언트가 확인하도록 함현재 설계가 보안이나 UX 측면에서 명확한 이유가 있다면 괜찮지만, 그렇지 않다면 API를 단순화하는 것도 고려해볼 만합니다. 프론트엔드 개발자와 논의해보세요.
src/main/java/com/almang/inventory/inventory/service/InventoryService.java (1)
85-101: 서비스 로직이 잘 구현되었습니다! ✅사용자 컨텍스트 기반 접근 제어, 상품 일치 검증, 트랜잭션 관리가 모두 적절하게 구현되었습니다. 로깅도 잘 되어 있어 운영 중 추적이 용이할 것 같습니다.
선택적 필드 업데이트를 위해 모든 필드를 nullable로 받는데, 요청이 모든 필드를 null로 보내는 경우(실제로 아무것도 업데이트하지 않는 경우)에 대한 검증을 추가하면 더욱 견고해집니다:
if (request.displayStock() == null && request.warehouseStock() == null && request.outgoingReserved() == null && request.incomingReserved() == null && request.reorderTriggerPoint() == null) { throw new BaseException(ErrorCode.INVALID_INPUT_VALUE); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java(1 hunks)src/main/java/com/almang/inventory/global/exception/ErrorCode.java(1 hunks)src/main/java/com/almang/inventory/inventory/controller/InventoryController.java(1 hunks)src/main/java/com/almang/inventory/inventory/domain/Inventory.java(1 hunks)src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java(1 hunks)src/main/java/com/almang/inventory/inventory/dto/response/InventoryResponse.java(1 hunks)src/main/java/com/almang/inventory/inventory/service/InventoryService.java(4 hunks)src/test/java/com/almang/inventory/inventory/service/InventoryServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T18:12:13.172Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.172Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/main/java/com/almang/inventory/inventory/controller/InventoryController.javasrc/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.javasrc/test/java/com/almang/inventory/inventory/service/InventoryServiceTest.javasrc/main/java/com/almang/inventory/inventory/domain/Inventory.javasrc/main/java/com/almang/inventory/inventory/service/InventoryService.java
📚 Learning: 2025-11-20T10:43:47.502Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.502Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/main/java/com/almang/inventory/inventory/controller/InventoryController.javasrc/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.javasrc/test/java/com/almang/inventory/inventory/service/InventoryServiceTest.javasrc/main/java/com/almang/inventory/inventory/domain/Inventory.javasrc/main/java/com/almang/inventory/inventory/service/InventoryService.java
🔇 Additional comments (6)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
65-66: 새로운 에러 코드 추가가 적절합니다! 👍재고 접근 제어와 상품 불일치에 대한 에러 코드가 잘 정의되었습니다. HTTP 상태 코드도 의미에 맞게 설정되어 있습니다.
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
62-63: 성공 메시지 추가 완료! ✨재고 수동 수정 성공 메시지가 적절하게 추가되었습니다.
src/main/java/com/almang/inventory/inventory/dto/response/InventoryResponse.java (1)
15-25: 응답 DTO 구현이 깔끔합니다! 🎯정적 팩토리 메서드 패턴을 사용하여
Inventory엔티티에서 응답 객체로의 매핑이 명확하게 구현되었습니다.src/main/java/com/almang/inventory/inventory/service/InventoryService.java (1)
119-127: 접근 제어 검증 로직이 정확합니다! 🔒재고가 속한 상점과 사용자의 상점을 비교하여 접근을 제어하는 로직이 올바르게 구현되었습니다.
src/main/java/com/almang/inventory/inventory/controller/InventoryController.java (1)
31-45: 컨트롤러 구현이 훌륭합니다! 🚀RESTful API 설계 원칙을 잘 따르고 있으며, 인증, 검증, 로깅이 적절하게 구현되었습니다. Swagger 문서화도 잘 되어 있어 API 사용자에게 도움이 될 것입니다.
src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java (1)
6-13: 검증 어노테이션 추가가 필요합니다.현재
productId에만@NotNull검증이 있지만, 재고 수량과reorderTriggerPoint에 대한 범위 검증이 누락되었습니다. 음수 재고나 범위를 벗어난reorderTriggerPoint값이 전달될 수 있습니다.Inventory.java의
updateManually메서드 리뷰 코멘트를 참조하세요.⛔ Skipped due to learnings
Learnt from: JoonKyoLee Repo: almang2/inventory-server PR: 66 File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53 Timestamp: 2025-11-22T18:12:13.172Z Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates. <!--Learnt from: JoonKyoLee Repo: almang2/inventory-server PR: 33 File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66 Timestamp: 2025-11-20T10:43:47.502Z Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
| public void updateManually( | ||
| BigDecimal displayStock, | ||
| BigDecimal warehouseStock, | ||
| BigDecimal outgoingReserved, | ||
| BigDecimal incomingReserved, | ||
| BigDecimal reorderTriggerPoint | ||
| ) { | ||
| if (displayStock != null) { | ||
| this.displayStock = displayStock; | ||
| } | ||
| if (warehouseStock != null) { | ||
| this.warehouseStock = warehouseStock; | ||
| } | ||
| if (outgoingReserved != null) { | ||
| this.outgoingReserved = outgoingReserved; | ||
| } | ||
| if (incomingReserved != null) { | ||
| this.incomingReserved = incomingReserved; | ||
| } | ||
| if (reorderTriggerPoint != null) { | ||
| this.reorderTriggerPoint = reorderTriggerPoint; | ||
| } | ||
| } |
There was a problem hiding this comment.
재고 값에 대한 검증이 필요합니다.
현재 updateManually 메서드는 음수 재고 값에 대한 검증을 수행하지 않습니다. 재고 수량(displayStock, warehouseStock, outgoingReserved, incomingReserved)이 음수가 되면 데이터 무결성 문제가 발생할 수 있습니다.
또한 reorderTriggerPoint는 Store 엔티티의 defaultCountCheckThreshold와 마찬가지로 0과 1 사이의 값이어야 하는데, 이에 대한 범위 검증이 없습니다.
서비스 계층에서 요청 DTO에 다음과 같은 검증 어노테이션을 추가하는 것을 권장합니다:
public record UpdateInventoryRequest(
@NotNull Long productId,
@DecimalMin(value = "0.0", message = "매대 재고는 0 이상이어야 합니다.")
BigDecimal displayStock,
@DecimalMin(value = "0.0", message = "창고 재고는 0 이상이어야 합니다.")
BigDecimal warehouseStock,
@DecimalMin(value = "0.0", message = "출고 예약은 0 이상이어야 합니다.")
BigDecimal outgoingReserved,
@DecimalMin(value = "0.0", message = "입고 예약은 0 이상이어야 합니다.")
BigDecimal incomingReserved,
@DecimalMin(value = "0.0", message = "재주문 기준점은 0 이상이어야 합니다.")
@DecimalMax(value = "1.0", message = "재주문 기준점은 1 이하여야 합니다.")
BigDecimal reorderTriggerPoint
) {}참고: Jakarta Bean Validation 공식 문서
🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/inventory/domain/Inventory.java around
lines 92-114, updateManually currently accepts negative inventory values and
does not enforce that reorderTriggerPoint is between 0 and 1; add validation for
each non-null BigDecimal: for displayStock, warehouseStock, outgoingReserved,
incomingReserved ensure value.compareTo(BigDecimal.ZERO) >= 0 and throw a clear
IllegalArgumentException if negative; for reorderTriggerPoint ensure
value.compareTo(BigDecimal.ZERO) >= 0 and value.compareTo(BigDecimal.ONE) <= 0
and throw an IllegalArgumentException if out of range; also add or recommend
Bean Validation annotations on the service-layer UpdateInventoryRequest DTO
(e.g., @DecimalMin("0.0") and @DecimalMax("1.0") for reorderTriggerPoint) so
incoming requests are validated before reaching the domain.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/almang/inventory/inventory/controller/InventoryControllerTest.java (1)
1-178: 컨트롤러 테스트 시나리오 구성이 좋습니다. 성공 케이스에서 한 필드만 더 체크하면 완성도가 올라갑니다.
- 성공 테스트(
재고_수동_수정에_성공한다)에서UpdateInventoryRequest와InventoryResponse모두outgoingReserved = 0.500을 세팅하고 있지만, 응답 검증에서는displayStock,warehouseStock,incomingReserved,reorderTriggerPoint만 확인하고 있습니다.
매핑 누락/실수에 대비하려면outgoingReserved도 같이 검증하는 편이 안전합니다.예시 diff는 아래처럼 추가하면 됩니다.
- .andExpect(jsonPath("$.data.warehouseStock").value(10.000)) - .andExpect(jsonPath("$.data.incomingReserved").value(3.000)) - .andExpect(jsonPath("$.data.reorderTriggerPoint").value(0.25)); + .andExpect(jsonPath("$.data.warehouseStock").value(10.000)) + .andExpect(jsonPath("$.data.outgoingReserved").value(0.500)) + .andExpect(jsonPath("$.data.incomingReserved").value(3.000)) + .andExpect(jsonPath("$.data.reorderTriggerPoint").value(0.25));
- 그 외에,
- 성공/도메인 예외/검증 예외를 나눠서 테스트하신 구조는 좋은 패턴입니다. “행복/불행 플로우를 잘 쪼개 두신 느낌”이라 유지보수에 유리해 보입니다.
- 검증 실패 테스트에서는 현재도 충분하지만, 필요하다면
verify(inventoryService, never()).updateInventory(anyLong(), any(), anyLong());같은 검증을 추가해 “검증 실패 시 서비스가 호출되지 않는다”는 의도를 더 명확히 표현할 수도 있습니다(선택 사항).Spring
MockMvc+@WebMvcTest조합은 버전에 따라 설정 차이가 있을 수 있으니, 현재 사용 중인 Spring Boot/Validation 버전에서 JSON 레코드 바인딩 + 검증이 기대한 대로 동작하는지만 한 번 더 확인해 주시면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java(1 hunks)src/test/java/com/almang/inventory/inventory/controller/InventoryControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T18:12:13.172Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 66
File: src/main/java/com/almang/inventory/order/domain/OrderItem.java:41-53
Timestamp: 2025-11-22T18:12:13.172Z
Learning: In the almang2/inventory-server repository, OrderItem entity update methods (updateQuantity, updatePrice in src/main/java/com/almang/inventory/order/domain/OrderItem.java) do not require null checks because OrderService will validate parameters before calling these update methods, following the same pattern as Product entity updates.
<!--
Applied to files:
src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java
📚 Learning: 2025-11-20T10:43:47.502Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.502Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java
🔇 Additional comments (1)
src/main/java/com/almang/inventory/inventory/dto/request/UpdateInventoryRequest.java (1)
1-16: 부분 업데이트용 DTO 설계가 요구사항과 잘 맞습니다만, API 규칙을 명시해 두면 더 좋겠습니다.
productId만@NotNull로 강제하고 나머지 재고 값들은BigDecimal+@PositiveOrZero/@DecimalMin·@DecimalMax조합으로,
값이 들어오면 범위 검증을 하고(null이면 검증을 건너뛰는) Bean Validation 특성을 잘 활용하셨습니다. 부분 수동 변경 요구사항과 구조가 잘 맞습니다. 짧게 칭찬하자면, “검증 애노테이션 조합, 딱 알맞게 잘 뽑으셨네요.” 🙂- 다만 외부 클라이언트 입장에서는 “필드를 생략/nullable 로 보내면 어떤 의미인지(해당 항목 미변경인지, 0으로 리셋인지)” 가 헷갈릴 수 있습니다.
API 문서나 스웨거 스펙에
productId는 필수, 나머지 필드는 선택이며null이면 기존 값을 유지한다
와 같이 한 줄 정도를 명시해 두면 이후 클라이언트/프론트 협업 시 혼선을 줄일 수 있습니다.Bean Validation(Jakarta Validation)에서는 대부분의 제약 애노테이션이
null값은 유효한 값으로 취급하고(@NotNull만 별도), 이 패턴을 의도하신 것이라면 현재 구현 그대로 유지하시면 됩니다. 혹시라도 특정 필드를 “항상 있어야 하는 값”으로 바꾸고 싶다면@NotNull추가 여부만 한 번 더 점검해 주세요.
✨ 작업 내용
📝 적용 범위
/inventory📌 참고 사항
Summary by CodeRabbit
새로운 기능
유효성 검사
오류 처리 / 권한
테스트
✏️ Tip: You can customize this high-level summary in your review settings.