Conversation
Walkthrough상품 수정(PATCH) 기능이 추가되고, Product 엔티티에 부분 업데이트용 가드 메서드들이 도입되었습니다. DTO는 CreateProductResponse가 ProductResponse로 통합되고 UpdateProductRequest가 추가되었으며, 컨트롤러·서비스에 업데이트 흐름과 관련 성공/오류 상수들이 추가·수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ProductController
participant Service as ProductService
participant Repo as Repository
participant Entity as Product
Client->>Controller: PATCH /api/v1/product/{productId} (UpdateProductRequest)
activate Controller
Controller->>Service: updateProduct(productId, request, userId)
activate Service
Service->>Repo: findUserById(userId) / validate
Service->>Repo: findProductById(productId)
alt product not found
Service-->>Controller: throw PRODUCT_NOT_FOUND
else
Service->>Repo: findVendorByIdAndValidateAccess(vendorId, user)
alt vendor not found
Service-->>Controller: throw VENDOR_NOT_FOUND
else vendor access denied
Service-->>Controller: throw VENDOR_ACCESS_DENIED / STORE_ACCESS_DENIED
else
rect rgba(100,180,120,0.12)
Service->>Entity: updateVendor(...)
Service->>Entity: updateBasicInfo(...)
Service->>Entity: updateWeights(...)
Service->>Entity: updatePrices(...)
Service->>Entity: updateActivation(...)
end
Service->>Repo: save(product)
Service-->>Controller: ProductResponse.from(product)
end
end
Controller-->>Client: 200 OK + ApiResponse<ProductResponse> (SuccessMessage.UPDATE_PRODUCT_SUCCESS)
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 주의할 파일/영역:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-20T10:43:47.489ZApplied to files:
🔇 Additional comments (10)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/main/java/com/almang/inventory/product/dto/request/UpdateProductRequest.java (1)
11-12: 필드 검증이 누락되었습니다.
name과code필드에 유효성 검증이 없습니다. 빈 문자열이나 공백만 있는 값이 허용될 수 있습니다.생성 시에는 어떤 검증을 사용하는지 확인하고 동일한 수준의 검증을 적용하는 것이 좋습니다.
CreateProductRequest의 검증 수준과 일관성을 맞추기 위해 다음 검증을 추가하세요:public record UpdateProductRequest( @NotNull Long vendorId, + @NotBlank String name, + @NotBlank String code, - String name, - String code, ProductUnit unit,src/main/java/com/almang/inventory/product/domain/Product.java (2)
92-102: 타입 일관성을 개선하면 더 명확한 코드가 됩니다.메서드 파라미터는
Integer(wrapper)인데 엔티티 필드는int(primitive)로 선언되어 있습니다 (lines 54, 57, 60). 현재는 null 체크로 문제가 없지만, 타입을 일치시키면 코드 의도가 더 명확해집니다.옵션 1: 엔티티 필드를 wrapper 타입으로 변경 (nullable 가격을 허용하려는 경우)
-private int costPrice; -private int retailPrice; -private int wholesalePrice; +private Integer costPrice; +private Integer retailPrice; +private Integer wholesalePrice;옵션 2: primitive를 유지하려면, null인 경우 업데이트를 건너뛰는 현재 로직이 적절합니다. 이 경우 추가 조치는 불필요하지만, 차후 리팩토링 시 고려해보세요.
📚 참고: JPA Best Practices - Wrapper vs Primitive Types
104-108: 타입 일관성 개선을 고려해보세요.파라미터는
Boolean(wrapper)이지만 엔티티 필드는boolean(primitive, line 51)입니다. 필드가@Column(nullable = false)로 선언되어 있으므로, 파라미터도 primitiveboolean으로 통일하는 것이 더 명확할 수 있습니다.-public void updateActivation(Boolean isActivate) { - if (isActivate != null) { - this.isActivate = isActivate; - } +public void updateActivation(boolean isActivate) { + this.isActivate = isActivate; }단, 이렇게 변경하면 호출하는 쪽에서 항상 값을 전달해야 하므로, 부분 업데이트 시나리오에 따라 현재 구현이 더 적합할 수도 있습니다. 비즈니스 요구사항에 맞게 선택하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
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/product/controller/ProductController.java(3 hunks)src/main/java/com/almang/inventory/product/domain/Product.java(1 hunks)src/main/java/com/almang/inventory/product/dto/request/UpdateProductRequest.java(1 hunks)src/main/java/com/almang/inventory/product/dto/response/ProductResponse.java(2 hunks)src/main/java/com/almang/inventory/product/service/ProductService.java(3 hunks)src/test/java/com/almang/inventory/product/controller/ProductControllerTest.java(4 hunks)src/test/java/com/almang/inventory/product/service/ProductServiceTest.java(3 hunks)
🔇 Additional comments (8)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
31-33: LGTM!에러 코드가 기존 패턴과 일관성 있게 추가되었습니다. 👍
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
26-26: LGTM!성공 메시지가 적절하게 추가되었습니다.
src/test/java/com/almang/inventory/product/controller/ProductControllerTest.java (1)
195-383: LGTM!품목 수정 기능에 대한 테스트 커버리지가 훌륭합니다. 성공 케이스뿐만 아니라 다양한 예외 상황(품목 없음, 발주처 없음, 권한 없음, 검증 실패)을 모두 검증하고 있습니다. 🎯
src/main/java/com/almang/inventory/product/dto/response/ProductResponse.java (1)
7-38: LGTM!
ProductResponse로 이름을 변경하고productId를 포함하도록 개선되었습니다. 생성과 수정 API가 동일한 응답 타입을 사용하게 되어 일관성이 높아졌습니다.src/main/java/com/almang/inventory/product/service/ProductService.java (1)
82-91: 발주처 접근 권한 검증 로직이 잘 추출되었습니다.중복 코드를 제거하고 재사용 가능한 메서드로 분리한 점이 좋습니다. 생성과 수정에서 동일한 검증 로직을 사용할 수 있게 되었네요. 👍
src/main/java/com/almang/inventory/product/controller/ProductController.java (1)
48-62: LGTM!PATCH 엔드포인트가 적절하게 구현되었습니다. 검증, 로깅, 에러 처리가 모두 포함되어 있고, 기존 생성 API와 일관된 구조를 유지하고 있습니다.
src/main/java/com/almang/inventory/product/domain/Product.java (2)
68-78: 깔끔한 null-safe 구현입니다! 👍각 파라미터에 대한 null 체크가 적절하게 적용되어 있어 부분 업데이트를 안전하게 수행할 수 있습니다.
80-90: 구조가 일관성 있게 잘 구현되었습니다.null 체크를 통한 부분 업데이트 패턴이 명확하게 적용되어 있습니다.
| import java.math.BigDecimal; | ||
|
|
||
| public record UpdateProductRequest( | ||
| @NotNull Long vendorId, |
There was a problem hiding this comment.
PATCH 엔드포인트의 부분 업데이트 시맨틱과 일치하지 않습니다.
vendorId에 @NotNull 제약이 있어서 발주처를 변경하지 않는 부분 업데이트 시에도 항상 vendorId를 제공해야 합니다. PATCH 방식은 변경하려는 필드만 전달하는 것이 일반적입니다.
현재 구조는 PUT(전체 교체)에 가깝습니다. 다음 중 하나를 선택하세요:
- PATCH 시맨틱을 유지하려면
vendorId를 nullable로 변경하고, null인 경우 업데이트하지 않도록 서비스 로직 수정 - 전체 교체 방식(PUT)으로 변경하고 모든 필수 필드에 validation 추가
참고: RFC 5789(PATCH Method for HTTP) 및 REST API 설계 모범 사례를 확인하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/almang/inventory/product/dto/request/UpdateProductRequest.java
around line 10, the @NotNull on vendorId forces clients to always supply
vendorId which breaks PATCH semantics; remove the @NotNull so vendorId is
nullable and update the service layer to only apply vendorId changes when
request.getVendorId() != null (i.e., check for null before setting or calling
repository update), adjust any controller validation groups to allow partial
updates, and add/modify tests to cover partial updates where vendorId is
omitted.
src/test/java/com/almang/inventory/product/service/ProductServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
34-34: 코드 포맷팅 일관성을 개선하세요.생성자 호출 시 쉼표 뒤에 공백이 누락되었습니다. 파일 내 다른 상수 선언들(10, 14, 16번 라인 등)과 일관성을 유지하기 위해 공백을 추가해주세요.
적용 예시:
- PRODUCT_NOT_FOUND(HttpStatus.NOT_FOUND,"품목을 찾을 수 없습니다."), + PRODUCT_NOT_FOUND(HttpStatus.NOT_FOUND, "품목을 찾을 수 없습니다."),참고로
HttpStatus.NOT_FOUND사용은 완벽합니다! 👍 품목을 찾을 수 없을 때 404를 반환하는 것이 정확한 HTTP 시맨틱입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java(2 hunks)src/main/java/com/almang/inventory/product/service/ProductService.java(3 hunks)src/test/java/com/almang/inventory/product/service/ProductServiceTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-20T10:43:47.489Z
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.489Z
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/test/java/com/almang/inventory/product/service/ProductServiceTest.javasrc/main/java/com/almang/inventory/product/service/ProductService.java
🔇 Additional comments (11)
src/test/java/com/almang/inventory/product/service/ProductServiceTest.java (6)
10-11: 임포트 및 반환 타입 변경 잘 반영되었습니다!
ProductResponse로의 통합이 깔끔하게 이루어졌네요.Also applies to: 95-95
200-252: 완벽한 업데이트 성공 케이스 테스트입니다!발주처 변경을 포함한 모든 필드 업데이트를 검증하고 있어, 업데이트 기능의 정확성을 확실하게 보장합니다.
254-281: 존재하지 않는 품목 예외 처리 테스트 적절합니다!
283-326: 존재하지 않는 발주처 예외 처리 테스트 적절합니다!
328-372: 다른 상점의 발주처로 수정 시도 테스트가 잘 구현되었습니다!접근 권한 검증이 제대로 동작하는지 확인하는 중요한 보안 테스트입니다.
374-430: 이전 리뷰에서 요청한 테스트 케이스가 완벽하게 추가되었습니다!다른 상점의 품목을 수정하려는 시도를 차단하는 중요한 보안 검증 테스트입니다. 두 개의 독립적인 상점 환경을 구성하고, 교차 접근을 시도한 뒤 적절한 예외(
STORE_ACCESS_DENIED)가 발생하는지 확인하고 있어 완벽합니다.src/main/java/com/almang/inventory/product/service/ProductService.java (5)
7-8: 응답 DTO 통합이 깔끔하게 적용되었습니다!
ProductResponse로의 일관된 변경이 잘 이루어졌네요.Also applies to: 29-29, 37-37
40-61: 이전 리뷰의 크리티컬 보안 이슈가 완벽하게 수정되었습니다!Lines 45-47의 상점 소유권 검증이 추가되어, 다른 상점의 품목을 수정할 수 없도록 보호하고 있습니다. 검증 순서도 훌륭합니다:
- 품목 소유권 확인 (fail-fast)
- 발주처 접근 권한 확인
- 업데이트 수행
이는 OWASP의 Broken Access Control (A01:2021) 취약점을 제대로 방어하는 구현입니다. 👍
참고: OWASP Top 10:2021 - A01 Broken Access Control
63-64: 헬퍼 메서드 재사용으로 코드 중복이 제거되었습니다!DRY 원칙을 잘 따르고 있네요.
87-96: 발주처 검증 헬퍼 메서드가 잘 구현되었습니다!존재 여부와 접근 권한을 한 곳에서 검증하여 코드 재사용성이 높아졌습니다.
98-101: 품목 조회 헬퍼 메서드가 적절하게 구현되었습니다!일관된 예외 처리 패턴을 따르고 있네요.
src/main/java/com/almang/inventory/global/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
1 participant
✨ 작업 내용
📝 적용 범위
/product📌 참고 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정 / 개선
테스트
✏️ Tip: You can customize this high-level summary in your review settings.