Conversation
Walkthrough컨트롤러 문서와 서비스 로직을 수정하여 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repo
Note over Controller,Service `#DDEBF7`: getProductionPlans 요청 흐름 (요지)
Client->>Controller: GET /production-plans?includeRecentDays=X
Controller->>Service: getProductionPlans(includeRecentDays)
alt includeRecentDays == -1
Service->>Service: cutoffDate = ancientDate
else
Service->>Service: cutoffDate = now - includeRecentDays
end
Service->>Repo: query production plans (apply cutoffDate)
Repo-->>Service: result set
Service->>Service: map DTOs (round progressRate to 2 decimals)
Service-->>Controller: production plan DTOs
Controller-->>Client: 200 OK (JSON)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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,
vivivim 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: 0
🧹 Nitpick comments (1)
src/main/java/com/sampoom/factory/api/part/service/PartOrderService.java (1)
583-586: 매직 값 대신 상수 사용을 권장합니다.
includeRecentDays=-1로직이 올바르게 구현되었으며, 모든 데이터를 조회하려는 의도가 명확합니다. 다만LocalDateTime.of(1900, 1, 1, 0, 0)같은 하드코딩된 날짜는 가독성과 유지보수성을 위해 클래스 상수로 추출하는 것이 좋습니다.다음과 같이 리팩토링할 수 있습니다:
+ private static final LocalDateTime ANCIENT_DATE = LocalDateTime.of(1900, 1, 1, 0, 0); + public PageResponseDto<PartOrderResponseDto> getProductionPlans(Long factoryId, List<PartOrderPriority> priorities, String query, Long categoryId, Long groupId, int page, int size, int includeRecentDays) { // ... // includeRecentDays가 -1이면 모든 IN_PROGRESS 데이터 포함, 아니면 최근 며칠간만 LocalDateTime cutoffDate = includeRecentDays == -1 ? - LocalDateTime.of(1900, 1, 1, 0, 0) : // 아주 오래된 날짜로 설정하여 모든 데이터 포함 + ANCIENT_DATE : // 아주 오래된 날짜로 설정하여 모든 데이터 포함 LocalDateTime.now().minusDays(includeRecentDays);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/sampoom/factory/api/part/controller/PartOrderController.java(1 hunks)src/main/java/com/sampoom/factory/api/part/service/PartOrderService.java(3 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/factory/api/part/service/PartOrderService.java (2)
645-645: 진행률 반올림 로직이 정확합니다.
progressRate를 소수점 둘째 자리까지 반올림하는 로직이 올바르게 구현되었습니다. 두 DTO 변환 메서드(toResponseDto,toProductionPlanResponseDto)에서 일관되게 적용되어 있습니다.
692-692: 진행률 반올림 로직이 일관되게 적용되었습니다.
toProductionPlanResponseDto메서드에서도 동일한 반올림 로직이 적용되어 일관성이 유지됩니다.src/main/java/com/sampoom/factory/api/part/controller/PartOrderController.java (1)
94-94: API 문서가 구현 내용을 정확히 반영합니다.
includeRecentDays=-1로 전체 데이터를 조회할 수 있다는 설명이 추가되어 API 사용자가 이 기능을 쉽게 이해하고 활용할 수 있습니다.
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 (2)
src/main/java/com/sampoom/factory/api/part/service/PartOrderCodeGenerator.java (2)
34-46: 파싱 실패 시 중복 코드가 생성될 수 있습니다.파싱 실패 시
nextSequence가 1로 초기화되는데, 해당 날짜에 이미 주문 코드가 존재하는 경우 중복 코드가 생성될 위험이 있습니다. 파싱 실패 시 예외를 던지거나 더 안전한 처리 방식이 필요합니다.다음과 같이 수정하는 것을 권장합니다:
if (lastOrderCode != null && !lastOrderCode.isEmpty()) { // WO-251109-001에서 마지막 3자리 숫자 추출 String[] parts = lastOrderCode.split("-"); if (parts.length == 3) { try { int lastSequence = Integer.parseInt(parts[2]); nextSequence = lastSequence + 1; } catch (NumberFormatException e) { - log.warn("주문 코드 파싱 실패: {}", lastOrderCode); + log.error("주문 코드 파싱 실패: {}", lastOrderCode, e); + throw new IllegalStateException("기존 주문 코드 파싱 실패: " + lastOrderCode, e); } } }
25-53: 동시성 문제로 중복 코드가 생성될 수 있습니다.두 개 이상의 요청이 동시에
generateOrderCode()를 호출하면 같은lastOrderCode를 읽고 동일한 코드를 생성할 수 있습니다. 이는 데이터 무결성에 심각한 영향을 미칩니다.다음 중 하나의 방법으로 해결할 수 있습니다:
옵션 1 (권장): 데이터베이스 유니크 제약조건 + 재시도 로직
@Service @RequiredArgsConstructor public class PartOrderCodeGenerator { private static final int MAX_RETRY = 3; public String generateOrderCode() { for (int attempt = 0; attempt < MAX_RETRY; attempt++) { try { String code = generateOrderCodeInternal(); // 실제 저장 시 DB 유니크 제약조건이 중복을 방지 return code; } catch (DataIntegrityViolationException e) { if (attempt == MAX_RETRY - 1) throw e; log.warn("주문 코드 중복으로 재시도: attempt {}", attempt + 1); } } throw new IllegalStateException("주문 코드 생성 실패"); } private String generateOrderCodeInternal() { // 기존 로직 } }옵션 2: synchronized 키워드 사용 (간단하지만 확장성 제한)
- public String generateOrderCode() { + public synchronized String generateOrderCode() {옵션 3: 비관적 락 사용
PartOrderRepository에 락을 사용하는 쿼리 메서드를 추가합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/sampoom/factory/api/part/service/PartOrderCodeGenerator.java(2 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/factory/api/part/service/PartOrderCodeGenerator.java (3)
20-23: 문서화가 명확합니다.새로운 날짜 기반 형식(yyMMdd)에 대한 설명이 명확하고 예시도 적절합니다.
29-32: 날짜 기반 조회 로직이 올바릅니다.날짜 코드 접두사를 사용한 조회 로직이 정확하며, 고정 폭 숫자 형식이므로 문자열 정렬로 최대 시퀀스를 찾는 방식이 적절합니다.
48-49: 시퀀스 오버플로우 검증 및 처리 로직이 필요합니다.3자리 시퀀스(%03d)는 최대 999까지만 지원하지만, 999를 초과하면 검증 없이 "WO-251109-1000" 형태의 4자리 코드가 생성되어 의도한 형식을 위반합니다. 다음 중 하나의 조치가 필요합니다:
- 일일 주문량이 999개를 초과하는 경우 시퀀스 자릿수 확대 (4자리 이상)
- 999개 도달 시 예외 발생 및 거부
- 비즈니스 요구사항 확인 후 적절한 처리 전략 결정
src/main/java/com/sampoom/factory/api/part/service/PartOrderCodeGenerator.java
Show resolved
Hide resolved
3 participants
📝 Summary
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
릴리스 노트
개선 사항
문서