Conversation
Walkthrough엑셀 업로드 파싱 책임을 분리하는 리팩토링입니다. 새로운 Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Service as RetailService
participant Parser as RetailExcelParser
participant DB as Repository
participant Product as ProductService
Client->>Service: 엑셀 업로드 (InputStream)
Service->>Parser: parse(inputStream)
Parser->>Parser: 워크북 로드 & 헤더 스킵
loop 각 데이터 행
Parser->>Parser: 셀 추출 & 유형 변환<br/>(Code, Name, Qty, Sales)
alt 코드 없음 OR 수량 ≤ 0
Parser->>Parser: 행 건너뛰기 (스킵 처리)
else 유효한 행
Parser->>Parser: RetailExcelRowDto 생성
end
end
Parser-->>Service: List<RetailExcelRowDto>
Service->>Service: soldDate 계산, 기존 Retail 소프트 삭제
loop 각 파싱된 DTO
Service->>Product: 상품 조회 (code)
alt 상품 없음
Service->>Service: 스킵 항목 수집
else 상품 존재
Service->>Product: 재고 확인 & 감소
Service->>Service: Retail 엔티티 생성 (큐에 추가)
end
end
Service->>DB: saveAll(Retail 목록)
Service-->>Client: {processed: int, skipped: List}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 사유: 파싱 로직의 복잡성(셀 타입 변환, null/형식 처리)과 서비스 리팩토링 범위를 고려하면 중간 수준입니다. 다만 변경이 4개 파일에 분산되어 있고, 각 컴포넌트의 책임 경계를 검증해야 합니다. Possibly related PRs
Suggested reviewers
Poem
리뷰 포인트잘 구성된 리팩토링입니다. 몇 가지 검증 사항을 제안드립니다:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 (4)
src/main/java/com/almang/inventory/retail/service/RetailService.java (2)
72-77: POI 라이브러리의 다른 예외 처리를 고려해주세요.현재
IOException만 catch하고 있지만, Apache POI의WorkbookFactory.create()는InvalidFormatException이나EncryptedDocumentException등 다른 예외도 발생시킬 수 있습니다. 이러한 예외가 발생하면 500 에러로 전파됩니다.🔎 제안: 더 넓은 예외 처리
List<RetailExcelRowDto> rows; try (InputStream inputStream = file.getInputStream()) { rows = retailExcelParser.parse(inputStream); - } catch (IOException e) { + } catch (IOException | RuntimeException e) { + log.error("[RetailService] 엑셀 파일 파싱 실패 - fileName: {}", file.getOriginalFilename(), e); throw new BaseException(ErrorCode.EXCEL_PARSE_ERROR); }또는 파서 내부에서 예외를 통일하여
IOException으로 감싸는 방법도 있습니다.
111-123: 예외 처리 범위가 넓어 의도치 않은 예외를 삼킬 수 있습니다.
BaseException을 catch하면DISPLAY_STOCK_NOT_ENOUGH외의 다른 예외도 모두 "재고 부족"으로 처리됩니다. 예상치 못한 예외가 발생할 경우 디버깅이 어려워질 수 있습니다.🔎 제안: ErrorCode 확인 추가
try { inventory.decreaseDisplay(quantity); } catch (BaseException e) { + if (e.getErrorCode() != ErrorCode.DISPLAY_STOCK_NOT_ENOUGH) { + throw e; // 예상치 못한 예외는 재throw + } // 재고 부족 시 해당 상품을 스킵하고 계속 진행 - // decreaseDisplay() 메서드는 DISPLAY_STOCK_NOT_ENOUGH 예외를 던짐 BigDecimal currentStock = inventory.getDisplayStock();src/main/java/com/almang/inventory/retail/parser/RetailExcelParser.java (2)
79-90: FORMULA 및 BLANK 셀 타입 처리가 누락되었습니다.
STRING과NUMERIC만 명시적으로 처리하고 있어,FORMULA셀(예:=A1&B1)이나BLANK셀은 빈 문자열로 처리됩니다. 실제 Excel 파일에서 수식을 사용하는 경우 의도치 않은 동작이 발생할 수 있습니다.🔎 제안: FORMULA 셀 처리 추가
private String getCellValueAsString(Cell cell, DataFormatter formatter) { if (cell == null) { return null; } if (cell.getCellType() == CellType.STRING) { return cell.getStringCellValue().trim(); } if (cell.getCellType() == CellType.NUMERIC) { return formatter.formatCellValue(cell).trim(); // "00123" 보존 } + if (cell.getCellType() == CellType.FORMULA) { + // 수식의 캐시된 결과값 사용 + return formatter.formatCellValue(cell).trim(); + } + if (cell.getCellType() == CellType.BLANK) { + return null; + } return ""; }
DataFormatter.formatCellValue()는 수식의 캐시된 결과를 반환하므로, 일관성을 위해formatter.formatCellValue(cell)를 기본으로 사용하는 것도 고려해보세요.
92-146: 수치 파싱 메서드에서도 FORMULA 셀 처리가 필요합니다.
getCellValueAsBigDecimal과getCellValueAsInteger에서도FORMULA셀이 기본값(ZERO/null)으로 처리됩니다. 또한,formatter파라미터가 전달되지만 실제로 사용되지 않습니다.🔎 제안: FORMULA 셀 처리 및 formatter 활용
private BigDecimal getCellValueAsBigDecimal(Cell cell, DataFormatter formatter) { if (cell == null) { return BigDecimal.ZERO; } + CellType effectiveType = cell.getCellType() == CellType.FORMULA + ? cell.getCachedFormulaResultType() + : cell.getCellType(); - if (cell.getCellType() == CellType.NUMERIC) { + if (effectiveType == CellType.NUMERIC) { return BigDecimal.valueOf(cell.getNumericCellValue()).stripTrailingZeros(); } - if (cell.getCellType() == CellType.STRING) { + if (effectiveType == CellType.STRING) { // ... 기존 로직 } return BigDecimal.ZERO; }
getCellValueAsInteger에도 동일한 패턴을 적용하세요.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/almang/inventory/global/exception/ErrorCode.javasrc/main/java/com/almang/inventory/retail/dto/excel/RetailExcelRowDto.javasrc/main/java/com/almang/inventory/retail/parser/RetailExcelParser.javasrc/main/java/com/almang/inventory/retail/service/RetailService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/almang/inventory/retail/parser/RetailExcelParser.java (1)
src/main/java/com/almang/inventory/retail/service/RetailService.java (1)
Slf4j(35-203)
🔇 Additional comments (4)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
12-12: 깔끔한 에러 코드 추가입니다! 👍
BAD_REQUEST상태 코드 선택이 적절합니다. 클라이언트가 제공한 파일의 파싱 실패는 클라이언트 측 오류로 분류하는 것이 올바릅니다.src/main/java/com/almang/inventory/retail/dto/excel/RetailExcelRowDto.java (1)
5-11: 간결하고 불변성을 보장하는 Record 사용, 좋습니다! 🎯
RetailExcelRowDto가 파싱된 행 데이터를 명확하게 담고 있습니다.actualSales가Integer(nullable)인 점은 파서의null반환 동작과 일치합니다.참고로, 향후 확장성을 고려하여 validation annotation(
@NotNull,@Positive등)을 추가하는 것도 검토해볼 만합니다. 현재는 파서에서 검증하므로 당장은 필요하지 않습니다.src/main/java/com/almang/inventory/retail/service/RetailService.java (1)
48-141: 파싱 책임 분리가 잘 되었습니다! 🚀Service가 도메인 로직(재고 차감, 엔티티 생성, 저장)에만 집중하고, 파싱은
RetailExcelParser에 위임하는 구조가 명확합니다. soft delete 시 상세 로그를 남기는 것도 복구 시 유용합니다.src/main/java/com/almang/inventory/retail/parser/RetailExcelParser.java (1)
19-77: 파싱 로직이 잘 분리되었습니다! 깔끔한 구조입니다. 👏
- 컬럼 인덱스를 상수로 정의하여 유지보수성이 좋습니다.
try-with-resources로Workbook리소스를 안전하게 관리합니다.- 헤더 스킵, 빈 코드/수량 스킵 정책이 명확하게 구현되어 있습니다.
한 가지 참고:
sheet.getSheetAt(0)은 항상 첫 번째 시트를 사용하므로, 다중 시트 Excel 업로드 시 사용자에게 첫 번째 시트만 처리된다는 안내가 필요할 수 있습니다.
1 participant
✨ 작업 내용
📝 적용 범위
/retail📌 참고 사항
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.