Conversation
재고 붙여서 전달
|
Warning Rate limit exceeded@vivivim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 13 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)
WalkthroughKafka의 part-forecast 토픽으로 들어온 이벤트를 처리하기 위한 DTO와 컨슈머, 재고 서비스 로직을 추가하여 예측 페이로드에 현재 재고를 첨부하고 업데이트된 이벤트를 아웃박스로 보냅니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Kafka as Kafka Broker
participant Consumer as ForecastEventConsumer
participant Mapper as EventPayloadMapper
participant Service as InventoryService
participant Repo as InventoryRepository
participant EventSvc as EventService
Kafka->>Consumer: 메시지 전달 (consume(message))
activate Consumer
Consumer->>Consumer: ObjectMapper.readValue(message)
Note over Consumer: JSON -> Event (eventType, payload)
Consumer->>Mapper: getPayloadClass(eventType)
Mapper-->>Consumer: ForecastPayload.class
Consumer->>Service: attachStocksToForecast(Event<ForecastPayload>)
activate Service
Service->>Service: payload 타입 검증
Service->>Repo: findByWarehouseIdAndPartId(warehouseId, partId)
Repo-->>Service: Inventory (quantity)
Service->>Service: payload.setStock(inventory.getQuantity())
Service->>EventSvc: serialize & save outbox ("part-forecast-events", payload)
EventSvc-->>Service: 저장 완료
Service-->>Consumer: 성공 리턴
deactivate Service
Consumer->>Consumer: log.info("success")
deactivate Consumer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
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/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
304-332: 읽기 전용 트랜잭션에서 쓰기 작업 수행메서드가
@Transactional(readOnly = true)로 표시되어 있지만, 329번 라인에서eventService.setEventOutBox()를 호출하여 데이터베이스에 쓰기 작업을 수행합니다. 읽기 전용 트랜잭션에서는 쓰기 작업이 커밋되지 않거나 런타임 예외가 발생할 수 있습니다.다음과 같이 수정하세요:
-@Transactional(readOnly = true) +@Transactional public <T> void attachStocksToForecast(Event<T> event) {
🧹 Nitpick comments (5)
src/main/java/com/sampoom/backend/api/inventory/dto/ForecastPayload.java (2)
7-11: @builder와 @Setter 동시 사용 확인
@Builder와@Setter를 함께 사용하면 빌더로 생성한 불변 객체처럼 보이지만 실제로는 가변 객체가 됩니다.InventoryService의 328번 라인에서payload.setStock()을 호출하므로 의도된 설계로 보이지만, 이는 예측하기 어려운 동작을 유발할 수 있습니다.더 명확한 설계를 위해 다음을 고려해보세요:
stock필드만 setter를 제공하거나withStock(Integer stock)같은 명시적인 메서드를 추가하는 방법-@Setter @Builder @NoArgsConstructor @AllArgsConstructor public class ForecastPayload { private Long warehouseId; private Long partId; private Integer demandQuantity; private LocalDateTime demandMonth; + @Setter private Integer stock; }
16-16: 필드명과 타입 불일치 고려
demandMonth필드명은 월 단위 데이터를 암시하지만 타입은LocalDateTime입니다. 일/시간 정보도 포함되는 경우demandDate또는demandDateTime으로 명명하는 것이 더 명확할 수 있습니다.src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java (2)
27-33: JSON 이중 파싱 최적화 고려메시지를 두 번 파싱합니다(27-28번 라인에서 eventType 추출, 30-33번 라인에서 전체 역직렬화). 이는 성능에 영향을 줄 수 있습니다.
다음과 같이 개선할 수 있습니다:
try { JsonNode root = objectMapper.readTree(message); String eventType = root.get("eventType").asText(); Class<?> payloadClass = eventPayloadMapper.getPayloadClass(eventType); - Event<?> event = objectMapper.readValue( - message, - objectMapper.getTypeFactory().constructParametricType(Event.class, payloadClass) - ); + Event<?> event = objectMapper.convertValue( + root, + objectMapper.getTypeFactory().constructParametricType(Event.class, payloadClass) + );
35-38: 이벤트 타입 문자열 상수화 고려
"PartForecast"문자열이 하드코딩되어 있습니다. 이벤트 타입을 상수로 정의하면 유지보수가 쉬워집니다.private static final String PART_FORECAST_EVENT = "PartForecast"; // ... if (PART_FORECAST_EVENT.equals(eventType)) {src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
330-331: 코드 스타일 일관성
else블록에 중괄호가 없습니다. 가독성과 일관성을 위해 중괄호를 추가하는 것이 좋습니다.- } else - throw new BadRequestException(ErrorStatus.INVALID_PAYLOAD_TYPE.getMessage()); + } else { + throw new BadRequestException(ErrorStatus.INVALID_PAYLOAD_TYPE.getMessage()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/sampoom/backend/api/inventory/dto/ForecastPayload.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(2 hunks)src/main/java/com/sampoom/backend/api/part/event/EventPayloadMapper.java(2 hunks)src/main/java/com/sampoom/backend/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
Service(40-333)
⏰ 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 (2)
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java (1)
32-32: LGTM!에러 상수가 다른 에러 정의들과 일관되게 구현되었습니다.
src/main/java/com/sampoom/backend/api/part/event/EventPayloadMapper.java (1)
5-5: LGTM!
PartForecast이벤트 타입 매핑이 기존 패턴과 일관되게 추가되었습니다.Also applies to: 31-32
src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java
Outdated
Show resolved
Hide resolved
트랜잭셔널 수정 이벤트 저장하고 있는데 웬 리드온리..
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
319-320: 이벤트 파라미터에 대한 방어적 null 검증은 선택사항입니다.현재 코드는 Kafka 컨슈머의 정상 동작 범위에서 안전합니다.
ObjectMapper.readValue()는 역직렬화 실패 시 예외를 발생시키므로 null을 반환하지 않습니다. ForecastEventConsumer에서 event 객체가 정상적으로 생성되면 InventoryService의attachStocksToForecast()메서드는 null 이벤트를 받지 않습니다.다만 추가적인 방어 계층을 원한다면, 다음과 같이 검증을 추가할 수 있습니다:
@Transactional public <T> void attachStocksToForecast(Event<T> event) { + if (event == null || event.getPayload() == null) { + throw new BadRequestException(ErrorStatus.INVALID_PAYLOAD_TYPE.getMessage()); + } if (event.getPayload() instanceof ForecastPayload payload) {이는 선택적 개선사항이며, 코드의 견고성을 높이고자 할 때 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/sampoom/backend/api/inventory/event/ForecastEventConsumer.java
🔇 Additional comments (4)
src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (4)
14-14: LGTM!새로운 메서드의 타입 파라미터를 위한 필요한 import입니다.
321-321: 패턴 매칭 사용이 적절합니다.Modern Java의 instanceof 패턴 매칭을 사용하여 타입 체크와 캐스팅을 간결하게 처리했습니다.
328-329: 이벤트 enrichment 패턴이 올바르게 구현되었습니다.payload에 재고 정보를 설정한 후 event를 직렬화하여 outbox로 전송하는 로직이 적절합니다. 기존 코드(line 259)와 일관된 패턴을 따르고 있습니다.
330-331: 예외 처리가 적절합니다.payload 타입이
ForecastPayload가 아닌 경우에 대한 예외 처리가 올바르게 구현되었습니다. 에러 상태와 예외 타입이 적절합니다.
📝 Summary
재고 붙여서 전달
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
New Features
Chores