Conversation
입고 / 출고 나눔 그리고 이벤트 박스 페이로드 스트링 형식으로
ItemDto -> PartDeltaDto
|
Warning Rate limit exceeded@vivivim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughEventOutbox의 Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as InventoryController
participant Service as InventoryService
participant EventSvc as EventService
participant Repo as InventoryRepository
participant DB as Database
Client->>Controller: PATCH /inventory/delivery (DeliveryReqDto)
Controller->>Service: deliveryProcess(deliveryReqDto)
Service->>Repo: UPDATE inventories (warehouseId, items)
Repo->>DB: SQL UPDATE
DB-->>Repo: OK
Service->>Service: checkRop(deliveryReqDto)
Service->>EventSvc: serializePayload(lackItems)
alt serialize OK
EventSvc-->>Service: JSON string
Service->>EventSvc: setEventOutBox(topic, payload)
EventSvc->>DB: INSERT EventOutbox (PENDING)
DB-->>EventSvc: OK
else serialize FAIL
EventSvc-->>Service: throw BadRequestException (FAIL_SERIALIZE)
Service-->>Controller: error
end
Controller-->>Client: 200 OK / error
sequenceDiagram
actor Client
participant Controller as InventoryController
participant Service as InventoryService
participant Repo as InventoryRepository
participant DB as Database
Client->>Controller: PATCH /inventory/stocking (PartUpdateReqDto)
Controller->>Service: stockingParts(partUpdateReqDto)
Service->>Repo: UPDATE inventories (warehouseId, items)
Repo->>DB: SQL UPDATE
DB-->>Repo: OK
Note over Service: stocking 경로는 checkRop/주문확정 흐름 없음
Controller-->>Client: 200 OK
sequenceDiagram
participant RopSvc as RopService
participant RopRepo as RopRepository
participant DB as Database
RopSvc->>RopRepo: existsByInventory_Id(inventoryId)
RopRepo->>DB: SELECT EXISTS ...
alt exists == true
DB-->>RopRepo: true
RopRepo-->>RopSvc: true
RopSvc->>RopSvc: throw BadRequestException (ROP_ALREADY_EXIST)
else exists == false
DB-->>RopRepo: false
RopRepo-->>RopSvc: false
RopSvc->>DB: create ROP
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분
Possibly related PRs
Suggested labels
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
33Auto-Bot
requested review from
CHOOSLA,
Lee-Jong-Jin,
Sangyoon98,
taemin3 and
yangjiseonn
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/sampoom/backend/api/inventory/repository/InventoryRepository.java (1)
35-44: 정수 나눗셈 절삭 문제 검증 필요
average_daily계산에서 SQL 정수 나눗셈은 나머지를 버리고 정수 부분만 반환합니다.Line 37의
p.safety_stock/5에서safety_stock값이 5 미만이면 결과는 0이 됩니다. 예를 들어safety_stock이 3이면 3/5 = 0입니다. 이는 재고 관리 로직에 문제를 야기할 수 있습니다:
averageDaily = 0일 때 ROP 계산식(line 41):rop = 0 * 5 + safetyStock = safetyStock- 이는 의도한 재발주점(Reorder Point)이 safety_stock만 되어 정상 작동하지 않음을 의미합니다.
이를 해결하려면 나누기 전에 DECIMAL로 캐스팅해야 합니다:
- SELECT :branchId, p.id, p.safety_stock*2, 'ENOUGH', p.safety_stock/5, 5, p.safety_stock*5, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + SELECT :branchId, p.id, p.safety_stock*2, 'ENOUGH', CAST(p.safety_stock AS DECIMAL)/5, 5, p.safety_stock*5, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP추가로 확인 필요:
- Part 엔티티에서
safety_stock의 최소값 제약이 있는지- 이 변경이 PR 목표(ItemDto → PartDeltaDto 변환)와 어떻게 연관되는지
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
121-159: 예외 메시지 보안 노출 문제 및 직렬화 코드 중복 개선 필요검증 결과:
메서드 시그니처 변경 확인됨: InventoryController 71번 줄에서
checkRop이PartUpdateReqDto로 올바르게 호출되고 있습니다. 추가 호출부는 없습니다.보안 문제 확인됨 (Line 149): 예외 메시지를 통한 정보 노출은 보안 취약점(CWE-209)으로, 애플리케이션이 민감한 정보를 사용자에게 노출할 수 있습니다.
e.getMessage()를 직접 연결하면 원격 사용자에게 민감한 정보를 포함할 수 있는 상세한 오류 메시지가 노출됩니다.권장사항: ObjectMapper는 설정 후 스레드 안전하므로 매번 새로 생성하기보다 재사용하는 것이 최적의 방법입니다. 직렬화 로직을 공유 유틸리티 메서드로 이동하고, 예외 메시지는 서버 로그에만 남기며 클라이언트에는 일반적인 메시지를 반환하세요.
수정 사항:
- Line 149:
e.getMessage()제거, 클라이언트에는 일반 오류 메시지 반환- 직렬화 로직을
PayloadSerializationUtil등 공유 유틸리티로 추출- 전체 프로젝트에서 이 패턴 통일
♻️ Duplicate comments (1)
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
98-116: 동일한 직렬화 패턴 중복이 메서드는
setOrderWarehouseEvent(Lines 77-96)와 동일한 직렬화 패턴을 사용합니다. 앞서 제안한 공통 헬퍼 메서드를 사용하여 두 메서드의 중복을 제거하세요.
🧹 Nitpick comments (4)
src/main/java/com/sampoom/backend/api/rop/dto/OrderToFactoryDto.java (1)
3-4: 미사용 import 제거 권장
PartDeltaDto로 변경되면서 Line 4의ItemDtoimport가 더 이상 사용되지 않는 것으로 보입니다.import com.sampoom.backend.api.inventory.dto.PartDeltaDto; -import com.sampoom.backend.api.order.dto.ItemDto; import lombok.AllArgsConstructor;src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java (1)
7-11: 필수 입력 검증 추가 권장요청 DTO에 필수 필드 검증이 없어 null 값으로 인한 런타임 오류가 발생할 수 있습니다. 특히
warehouseId와items리스트는 필수 값이어야 합니다.다음 diff를 적용하여 검증을 추가하세요:
package com.sampoom.backend.api.inventory.dto; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotNull; import lombok.Getter; import java.util.List; @Getter public class PartUpdateReqDto { + @NotNull private Long warehouseId; + @NotEmpty + @Valid private List<PartDeltaDto> items; }컨트롤러 메서드에서
@Valid어노테이션도 추가해야 합니다:public ResponseEntity<ApiResponse<Void>> deliveryParts(@Valid @RequestBody PartUpdateReqDto partUpdateReqDto)src/main/java/com/sampoom/backend/api/rop/service/RopService.java (1)
30-34: 중복 ROP 체크 로직 개선 권장중복 체크가 추가된 것은 좋으나, 몇 가지 개선이 필요합니다:
- 에러 메시지 포맷팅: Line 33에서 문자열 연결 시 공백이나 구분자가 없어 읽기 어려울 수 있습니다.
- 일괄 처리 실패: 스트림 중간에 예외가 발생하면 전체 배치가 실패합니다. 이것이 의도된 동작인지 확인이 필요합니다.
옵션 1 (현재 로직 유지 - 하나라도 중복 시 전체 실패):
.map(i -> { if (ropRepository.existsByInventory_Id(i.getId())) - throw new BadRequestException(ErrorStatus.ROP_ALREADY_EXIST.getMessage() + i.getPart().getName()); + throw new BadRequestException(ErrorStatus.ROP_ALREADY_EXIST.getMessage() + ": " + i.getPart().getName());옵션 2 (중복 건너뛰기 - 중복이 아닌 것만 생성):
List<Rop> rops = inventories.stream() + .filter(i -> !ropRepository.existsByInventory_Id(i.getId())) .map(i -> { - if (ropRepository.existsByInventory_Id(i.getId())) - throw new BadRequestException(ErrorStatus.ROP_ALREADY_EXIST.getMessage() + i.getPart().getName()); - int ropValue = i.getAverageDaily() * i.getLeadTime() + i.getPart().getSafetyStock();비즈니스 요구사항에 맞는 옵션을 선택하세요.
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java (1)
25-26: 에러 메시지 형식의 일관성을 고려하세요.새로 추가된 두 에러 상수의 메시지 형식이 일관되지 않습니다:
ROP_ALREADY_EXIST: 마침표 없음FAIL_SERIALIZE: 마침표 있음파일 전체의 에러 메시지 형식을 통일하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/sampoom/backend/api/event/entity/EventOutbox.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/dto/PartDeltaDto.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/repository/InventoryRepository.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(7 hunks)src/main/java/com/sampoom/backend/api/order/event/SalesEventConsumer.java(1 hunks)src/main/java/com/sampoom/backend/api/order/service/OrderService.java(4 hunks)src/main/java/com/sampoom/backend/api/rop/dto/OrderToFactoryDto.java(2 hunks)src/main/java/com/sampoom/backend/api/rop/repository/RopRepository.java(1 hunks)src/main/java/com/sampoom/backend/api/rop/service/RopService.java(2 hunks)src/main/java/com/sampoom/backend/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java (1)
src/main/java/com/sampoom/backend/api/inventory/dto/PartDeltaDto.java (1)
Getter(9-18)
src/main/java/com/sampoom/backend/api/rop/service/RopService.java (1)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(5-14)
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(5-14)
src/main/java/com/sampoom/backend/api/inventory/dto/PartDeltaDto.java (2)
src/main/java/com/sampoom/backend/api/rop/dto/OrderToFactoryDto.java (1)
Getter(12-19)src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java (1)
Getter(7-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (9)
src/main/java/com/sampoom/backend/api/rop/repository/RopRepository.java (1)
39-39: LGTM!재주문점 중복 체크를 위한 메서드가 올바르게 추가되었습니다. Spring Data JPA 네이밍 규칙을 따르며,
RopService의 중복 검증 로직에서 적절히 사용됩니다.src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java (1)
67-79: 엔드포인트 로직 승인출고와 입고를 명확하게 분리한 설계가 좋습니다. 출고 시에만 재주문점(
checkRop) 확인을 수행하는 로직이 비즈니스 요구사항과 일치합니다.src/main/java/com/sampoom/backend/api/inventory/dto/PartDeltaDto.java (1)
9-18: LGTM!클래스명을
PartDeltaDto로 변경하여 더 명확한 의미를 전달하고,@Builder어노테이션 추가로 인스턴스 생성이 편리해졌습니다. 필드 검증(@NotNull)도 적절히 적용되어 있습니다.src/main/java/com/sampoom/backend/api/rop/dto/OrderToFactoryDto.java (1)
17-18: 검증 완료: 모든OrderToFactoryDto생성 위치에서 올바른 타입 사용 확인됨검증 결과,
ItemDto에서PartDeltaDto로의 변경이 올바르게 적용되었습니다:
- 유일한 생성 위치:
InventoryService.checkRop()메서드 (144-147줄)- 타입 일치 확인:
lackItems는List<PartDeltaDto>타입이며,OrderToFactoryDto.items필드와 일치- 항목 추가: 모든
PartDeltaDto객체가 올바르게 빌더 패턴으로 생성됨PR에서 요청한 변경사항이 완벽하게 구현되었습니다.
src/main/java/com/sampoom/backend/api/order/event/SalesEventConsumer.java (1)
1-29:sales-events컨슈머 비활성화 의도 확인 필수전체
SalesEventConsumer클래스가 주석 처리되어 비활성화되었으며, 검증 결과sales-events토픽을 처리하는 다른 활성 컨슈머가 존재하지 않습니다. 애플리케이션이 여전히 이 토픽에 메시지를 발행하는 경우 메시지가 누적될 수 있습니다.다음을 명확히 해주세요:
- 이 컨슈머를 완전히 제거할 계획인가요, 아니면 임시 비활성화인가요?
sales-events토픽이 여전히 활성 상태인가요?- 이 변경이 프로덕션 환경에 영향을 미치지 않는지 확인했나요?
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
3-4: 변경사항 확인 완료Jackson ObjectMapper를 사용한 직렬화 기능 추가가 적절하게 구현되었습니다.
@RequiredArgsConstructor를 통한 의존성 주입도 올바릅니다.Also applies to: 14-14, 32-32
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (3)
3-4: 직렬화 기능 추가 확인 완료Jackson ObjectMapper를 사용한 직렬화 기능이 적절하게 추가되었습니다.
Also applies to: 46-46
71-118: 메서드 시그니처 변경이 올바르게 적용되었습니다호출 코드 검증 완료:
InventoryController.java의 두 엔드포인트(/delivery,/stocking)에서 모두PartUpdateReqDto를 올바르게 전달하고 있습니다. 메서드 시그니처 변경이 완벽하게 반영되었으며, 레거시 호출부는 발견되지 않았습니다.
197-197: 엔티티 필드명 변경이 올바르게 적용되었습니다검증 결과:
Part엔티티에standardCost필드 존재 확인 ✓ (@Column(name = "standard_cost"))- 구 필드
partValue는 엔티티에서 완전히 제거됨InventoryService.java:197의 변경이 올바름: 새로운 엔티티 필드(getStandardCost())를 DTO 필드(partValue)로 정확히 매핑- DTO 계층(
PartResDto)은 API 하위 호환성을 위해 기존 필드명 유지코드가 정상적으로 구현되어 있습니다.
src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java
Show resolved
Hide resolved
src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java
Show resolved
Hide resolved
이미 스트링으로 해놔서 인코딩 없이 보내는 것으로 수정
직렬화, 아웃박스 저장 함수 따로 뺌
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/sampoom/backend/api/event/service/EventPublisher.java (1)
42-46: 오해의 소지가 있는 에러 메시지 수정.직렬화 로직이 제거되었으므로 "Failed to serialize event" 메시지가 더 이상 정확하지 않습니다. 에러 메시지를 현재 로직에 맞게 수정하세요.
} catch (Exception e) { event.setStatus(EventStatus.FAILED); eventOutboxRepository.save(event); - log.error("❌ Failed to serialize event: {}", event.getId(), e); + log.error("❌ Failed to send event: {}", event.getId(), e); }
🧹 Nitpick comments (1)
src/main/java/com/sampoom/backend/api/event/service/EventPublisher.java (1)
3-3: 사용하지 않는 import 제거.
ObjectMapper가 더 이상 사용되지 않으므로 import를 제거하세요.-import com.fasterxml.jackson.databind.ObjectMapper;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/sampoom/backend/api/event/service/EventPublisher.java(1 hunks)src/main/java/com/sampoom/backend/api/event/service/EventService.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java(2 hunks)src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(7 hunks)src/main/java/com/sampoom/backend/api/order/service/OrderService.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/sampoom/backend/api/event/service/EventService.java (3)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(5-14)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
Service(38-191)src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
Service(26-96)
src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java (1)
src/main/java/com/sampoom/backend/api/inventory/dto/PartDeltaDto.java (1)
Getter(9-18)
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(5-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (4)
src/main/java/com/sampoom/backend/api/event/service/EventPublisher.java (1)
30-30: 직렬화 로직 제거는 타당합니다.검증 결과,
EventOutbox.payload는 String 타입이며 DB에 JSONB로 저장됩니다.EventService.serializePayload()에서ObjectMapper.writeValueAsString()으로 직렬화된 후 저장되므로,EventPublisher에서 페이로드를 직접 전송하는 것이 정상입니다.코드 변경은 문제 없습니다.
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
77-94: 이전 리뷰의 개선 사항이 부분적으로 반영됨
EventService로 직렬화 로직을 중앙화하여 코드 중복과 예외 메시지 노출 문제가 해결되었습니다. 그러나 직렬화 실패 시 로그를 남기는 부분은 여전히 누락되어 있습니다 (EventService.serializePayload 참조).
EventService에 로깅을 추가하는 것을 권장합니다.src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java (1)
68-80: 이전 리뷰의 검증 어노테이션 추가 완료
@Valid어노테이션이 추가되어 요청 DTO의 유효성 검증이 수행됩니다. 다만,PartUpdateReqDto의items필드에도 적절한 검증 어노테이션이 필요합니다 (PartUpdateReqDto.java 리뷰 참조).src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
143-149: 이벤트 발행 로직 개선 완료
EventService를 사용하여 이벤트 직렬화 및 발행을 중앙화한 것은 좋은 개선입니다. 코드 중복이 제거되고 일관성이 향상되었습니다.
src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java
Show resolved
Hide resolved
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java
Show resolved
Hide resolved
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
123-128: 빈 리스트에 대한 검사 누락으로 IndexOutOfBoundsException 위험Line 127에서
inventories.get(0)을 호출하기 전에 리스트가 비어있는지 확인하지 않습니다.partUpdateReqDto.getItems()의 모든 ID가 데이터베이스에 존재하지 않는 경우,inventories가 빈 리스트가 되어IndexOutOfBoundsException이 발생합니다.다음과 같이 개선하세요:
public void checkRop(PartUpdateReqDto partUpdateReqDto) { List<Inventory> inventories = inventoryRepository.findByBranch_IdAndPart_IdIn(partUpdateReqDto.getWarehouseId(), partUpdateReqDto.getItems().stream().map(PartDeltaDto::getId).collect(Collectors.toList())); + + if (inventories.isEmpty()) { + return; // 또는 적절한 예외를 던지거나 로그를 남김 + } + List<PartDeltaDto> lackItems = new ArrayList<>(); String warehouseName = inventories.get(0).getBranch().getName();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (3)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (3)
46-46: LGTM! 새로운 의존성 주입이 적절합니다.
EventService와ObjectMapper가 이벤트 직렬화 및 발행 로직을 중앙화하기 위해 추가되었습니다.@RequiredArgsConstructor를 통한 생성자 주입 방식이 올바르게 적용되었습니다.Also applies to: 48-48
73-76: 이전 리뷰 코멘트가 반영되었습니다.Line 74의 null 체크가 이전 리뷰에서 지적된 대로 추가되어
NullPointerException위험이 해결되었습니다.
77-78: LGTM! 창고 존재 여부 검증이 추가되었습니다.
updateParts메서드에서 재고 업데이트를 진행하기 전에 창고(branch)의 존재 여부를 검증하는 로직이 적절하게 추가되었습니다.
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (3)
74-74: Null 검사가 추가되어 이전 리뷰 이슈가 해결되었습니다.이전 리뷰에서 지적된 NPE 위험이 Line 74의 null 검사 추가로 해결되었습니다.
partUpdateReqDto.getItems()가 null이거나 비어있는 경우 모두 처리됩니다.
126-127: 빈 리스트 검사가 추가되어 이전 리뷰 이슈가 해결되었습니다.이전 리뷰에서 지적된 IndexOutOfBoundsException 위험이 Lines 126-127의 isEmpty 검사 추가로 해결되었습니다. 이제 빈 리스트인 경우 NotFoundException을 명확하게 던지고, Line 130에서 안전하게
inventories.get(0)을 호출할 수 있습니다.
134-152: 🔴 중대한 이슈: ROP 중복 주문 방지 로직이 여전히 누락되었습니다.이전 리뷰에서 지적된 중복 주문 방지 메커니즘이 여전히 구현되지 않았습니다:
- Line 134-135에서
autoOrderStatus = ACTIVE인 ROP를 조회합니다.- Lines 147-151에서 주문 이벤트를 생성하지만, 해당 ROP의 autoOrderStatus를 INACTIVE로 업데이트하지 않습니다.
- 결과:
checkRop이 다시 호출되면 동일한 ROP가 여전히 ACTIVE 상태이므로 같은 재고에 대해 중복 주문이 생성됩니다.Line 151 이후에 다음 로직을 추가하여 처리된 ROP의 상태를 업데이트하세요:
// 주문서 발행 if (!lackItems.isEmpty()) { String json = eventService.serializePayload(OrderToFactoryDto.builder() .warehouseName(warehouseName) .items(lackItems) .build()); eventService.setEventOutBox("order-to-factory-events", json); + + // ROP 상태 업데이트하여 중복 주문 방지 + for (Inventory inventory : inventories) { + if (inventory.getQuantity() <= ropRepository.findByInventory_Id(inventory.getId()) + .orElseThrow(() -> new NotFoundException(ErrorStatus.ROP_NOT_FOUND.getMessage())).getRop()) { + Rop rop = ropRepository.findWithInventoryByInventory_IdAndAutoOrderStatusAndIsDeletedFalse( + inventory.getId(), Status.ACTIVE).orElse(null); + if (rop != null) { + rop.setAutoOrderStatus(Status.INACTIVE); + ropRepository.save(rop); + } + } + } }또는 더 효율적으로, Line 133-143 루프에서 lackItem에 추가된 ROP를 추적하여 한 번에 업데이트할 수 있습니다:
- List<PartDeltaDto> lackItems = new ArrayList<>(); + List<PartDeltaDto> lackItems = new ArrayList<>(); + List<Rop> ropsToDeactivate = new ArrayList<>(); String warehouseName = inventories.get(0).getBranch().getName(); // 재고 없는 것들 수집 for (Inventory inventory : inventories) { Rop rop = ropRepository.findWithInventoryByInventory_IdAndAutoOrderStatusAndIsDeletedFalse(inventory.getId(), Status.ACTIVE) .orElseThrow(() -> new NotFoundException(ErrorStatus.ROP_NOT_FOUND.getMessage())); if (inventory.getQuantity() <= rop.getRop()) { lackItems.add(PartDeltaDto.builder() .id(inventory.getPart().getId()) .delta(inventory.getMaxStock() - inventory.getQuantity()) .build()); + ropsToDeactivate.add(rop); } } // 주문서 발행 if (!lackItems.isEmpty()) { String json = eventService.serializePayload(OrderToFactoryDto.builder() .warehouseName(warehouseName) .items(lackItems) .build()); eventService.setEventOutBox("order-to-factory-events", json); + + // ROP 상태 업데이트하여 중복 주문 방지 + for (Rop rop : ropsToDeactivate) { + rop.setAutoOrderStatus(Status.INACTIVE); + } + ropRepository.saveAll(ropsToDeactivate); }
🧹 Nitpick comments (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
3-4: ObjectMapper 필드가 사용되지 않습니다.Line 48에 ObjectMapper 필드를 선언했지만, 이 서비스 클래스 내에서 직접 사용되지 않고 있습니다. EventService가 내부적으로 직렬화를 처리하므로 이 필드는 불필요합니다.
다음 diff를 적용하여 사용하지 않는 의존성을 제거하세요:
-import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper;private final EventService eventService; private final BranchRepository branchRepository; -private final ObjectMapper objectMapper; @PersistenceContextAlso applies to: 48-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(7 hunks)
🔇 Additional comments (2)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (2)
191-191: <!-- [scratchpad_start]
[task review_verification_1/1]
Verify PartValue to StandardCost change at line 191[observations]
- Line 191 confirms: .partValue(inv.getPart().getStandardCost())
- toResponse method maps Inventory → PartResDto
- Part entity has only standardCost field (no partValue field exists)
- Third search produced no output: Part.java contains no getPartValue or partValue references
- PartResDto.partValue DTO field receives Part.standardCost value
[analysis]
- Part.partValue field does not exist in Part entity
- If original code called .partValue(inv.getPart().getPartValue()), it would fail
- This is a valid mapping: Part.standardCost → PartResDto.partValue
- The change maps an existing field to an existing DTO field
- Cannot verify business semantics (whether standardCost is correct value for partValue field)
[done]
Verified that Part.standardCost exists and is correctly mapped
Unable to confirm if semantic mapping aligns with business requirements
StandardCost를 partValue 필드에 매핑하는 것이 의도된 변경인지 확인 필요
검증 결과:
- Part 엔티티에는
standardCost필드만 존재하고partValue필드는 없습니다- 현재 코드
.partValue(inv.getPart().getStandardCost())는 기술적으로 유효한 매핑입니다- 그러나 PartResDto의
partValue필드에 Part의standardCost값을 매핑하는 것이 비즈니스 요구사항에 정확히 부합하는지는 검증할 수 없습니다다음을 확인하세요:
- 이 변경이 요구사항 및 비즈니스 로직에 부합하는지 검토
standardCost와partValue의 의미가 일치하는지 확인
147-147: 예외 처리는 올바르게 구현되었습니다.EventService의
serializePayload()메서드는 checked exception인JsonProcessingException을 unchecked exception인BadRequestException(→BaseException→RuntimeException)으로 적절하게 래핑하고 있습니다.InventoryService.checkRop()의@Transactional어노테이션은 unchecked exception 발생 시 자동으로 트랜잭션을 롤백하므로, 현재 예외 처리 구조는 올바르고 추가 조정이 필요하지 않습니다.
출고 요청을 보낼 때 해당 주문 아이디를 함게 보내야 합니다.
출고 시 배송완료 해버림
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
143-161: [중요] ROP 중복 주문 방지 로직 여전히 누락이전 리뷰에서 지적된 중복 주문 방지 로직이 아직 구현되지 않았습니다. Line 143에서
ACTIVE상태의 ROP를 조회하여 주문 이벤트를 생성(Lines 156-160)하지만, ROP의autoOrderStatus를INACTIVE로 업데이트하지 않습니다.결과:
checkRop이 재호출되면 동일한 ROP가 여전히ACTIVE상태이므로 같은 재고에 대해 중복 주문이 생성됩니다.다음 diff를 적용하여 이벤트 발행 후 ROP 상태를 업데이트하세요:
// 주문서 발행 if (!lackItems.isEmpty()) { String json = eventService.serializePayload(OrderToFactoryDto.builder() .warehouseName(warehouseName) .items(lackItems) .build()); eventService.setEventOutBox("order-to-factory-events", json); + + // ROP 상태를 INACTIVE로 변경하여 중복 주문 방지 + for (Inventory inventory : inventories) { + if (inventory.getQuantity() <= ropRepository.findWithInventoryByInventory_IdAndAutoOrderStatusAndIsDeletedFalse( + inventory.getId(), Status.ACTIVE).orElseThrow().getRop()) { + Rop rop = ropRepository.findWithInventoryByInventory_IdAndAutoOrderStatusAndIsDeletedFalse( + inventory.getId(), Status.ACTIVE).orElseThrow(); + rop.setAutoOrderStatus(Status.INACTIVE); + ropRepository.save(rop); + } + } }
🧹 Nitpick comments (2)
src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java (1)
5-6: 미사용 import 제거 권장컨트롤러에서
OrderStatus와OrderService를 직접 사용하지 않습니다. 이들은InventoryService내부에서만 사용되므로 제거하는 것이 좋습니다.src/main/java/com/sampoom/backend/api/inventory/dto/DeliveryReqDto.java (1)
16-16: items 필드에 대한 검증 추가 권장
items필드에@NotNull및@NotEmpty어노테이션이 누락되어 있습니다. 서비스 레이어에서 null/empty 체크를 수행하고 있지만, DTO 레벨에서 검증하는 것이 더 나은 방어적 프로그래밍 방식입니다.다음 diff를 적용하세요:
+import jakarta.validation.constraints.NotEmpty; + @Getter public class DeliveryReqDto { @NotNull(message = "창고 아이디는 필수입니다.") private Long warehouseId; @NotNull(message = "주문 아이디는 필수입니다.") private Long orderId; + @NotNull(message = "부품 목록은 필수입니다.") + @NotEmpty(message = "부품 목록이 비어있습니다.") private List<PartDeltaDto> items; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java(2 hunks)src/main/java/com/sampoom/backend/api/inventory/dto/DeliveryReqDto.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(8 hunks)src/main/java/com/sampoom/backend/api/order/service/OrderService.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/sampoom/backend/api/order/service/OrderService.java (1)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(5-14)
src/main/java/com/sampoom/backend/api/inventory/dto/DeliveryReqDto.java (1)
src/main/java/com/sampoom/backend/api/inventory/dto/PartUpdateReqDto.java (1)
Getter(9-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (6)
src/main/java/com/sampoom/backend/api/inventory/controller/InventoryController.java (1)
70-81: 입력 검증 추가 완료!이전 리뷰에서 요청한
@Valid어노테이션이 두 엔드포인트 모두에 추가되었습니다. 이제 요청 DTO의 제약 조건 검증이 정상적으로 수행됩니다.src/main/java/com/sampoom/backend/api/order/service/OrderService.java (2)
77-85: EventService로의 리팩토링 완료!이전 리뷰에서 지적된 코드 중복과 예외 처리 문제가
EventService를 통한 중앙화로 해결되었습니다. 직렬화 로직이 재사용 가능한 서비스로 추출되어 유지보수성이 개선되었습니다.
87-94: 메서드 가시성 변경 및 리팩토링 적절
setOrderStatusEvent가public으로 변경되어InventoryService에서 호출 가능하게 되었고,EventService를 활용한 일관된 이벤트 발행 패턴을 따릅니다.src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (3)
74-79: 출고 프로세스 통합 로직 적절출고 프로세스를 하나의 트랜잭션으로 묶어 재고 업데이트, ROP 체크, 주문 상태 이벤트 발행을 순차적으로 처리합니다. 로직 분리가 명확하고 트랜잭션 경계 설정이 적절합니다.
82-129: 입력 검증 및 벌크 업데이트 개선 완료!이전 리뷰에서 요청한 null/empty 검증이 추가되었고(Line 83), 추가로 branch 존재 여부 확인(Lines 86-87)과 중복 ID 체크(Lines 89-95)가 구현되어 데이터 무결성이 강화되었습니다. 네이티브 쿼리를 사용한 벌크 업데이트도 효율적입니다.
132-162: 빈 리스트 검증 추가 완료 및 이벤트 처리 개선이전 리뷰에서 요청한 빈 리스트 검증(Lines 135-136)과
EventService를 통한 직렬화 로직 개선이 완료되었습니다.
3 participants
📝 Summary
부품 업데이트 -> 입고 / 출고 나눔
이벤트 박스 페이로드 스트링 형식으로
재주문점 도달 시 주문 요청(ItemDto -> PartDeltaDto)
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
새로운 기능
버그 수정
리팩토링
제거