Conversation
Walkthrough입고 조회 기능을 구현하며, 사용자 인증을 통한 상점별 접근 제어 검증을 포함한 새로운 GET 엔드포인트를 추가합니다. 성공/오류 응답 구조를 정의하고 엔드-투-엔드 테스트 커버리지를 확보합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as ReceiptController
participant Service as ReceiptService
participant Repo as ReceiptRepository<br/>(JPA)
participant DB as Database
User->>Controller: GET /api/v1/receipt/{id}<br/>(with Auth)
activate Controller
Controller->>Service: getReceipt(receiptId, userId)
activate Service
Service->>Service: userStore = getUserStore(userId)
alt User Not Found
Service-->>Controller: throw USER_NOT_FOUND
else User Found
Service->>Repo: findById(receiptId)
activate Repo
Repo->>DB: SELECT receipt
Repo-->>Service: Optional<Receipt>
deactivate Repo
alt Receipt Not Found
Service-->>Controller: throw RECEIPT_NOT_FOUND
else Receipt Found
alt Receipt.store != userStore
Service-->>Controller: throw RECEIPT_ACCESS_DENIED
else Access Granted
Service-->>Controller: ReceiptResponse
end
end
end
deactivate Service
Controller-->>User: ApiResponse<ReceiptResponse>
deactivate Controller
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12분 검토 포인트:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (5)
src/main/java/com/almang/inventory/receipt/controller/ReceiptController.java (1)
56-68: 단일 입고 조회 엔드포인트 구현이 기존 패턴과 잘 정렬되어 있습니다.
GET /api/v1/receipt/{receiptId}가 기존/from-order/{orderId}핸들러와 동일한 구조(로그 →userId추출 → 서비스 위임 →ApiResponse.success)를 따르고 있어 유지보수성이 좋습니다.
교육적 관점에서 한 가지 제안만 드리면, Swagger 문서를 더 풍부하게 만들고 싶을 때@Operation(summary = "...", description = "...")정도를 메서드에 추가하면 API 문서에서 조회 용도를 한눈에 파악하기 더 좋아집니다(이미 다른 컨트롤러에서 사용 중이라면 특히 일관성 측면에서 유리).src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java (1)
298-400: 입고 조회 컨트롤러 테스트 커버리지가 깔끔하게 완성되었습니다.성공 케이스 +
USER_NOT_FOUND+RECEIPT_NOT_FOUND+RECEIPT_ACCESS_DENIED까지 HTTP 상태, 메시지, 데이터 필드를 모두 검증하고 있어 컨트롤러 레이어의 행위가 충분히 보호되고 있습니다. 잘 뽑으셨습니다.
개선 아이디어를 하나만 덧붙이면,ReceiptItemResponse/ReceiptResponse생성 코드가 from‑order/단일 조회 테스트들 사이에 거의 동일하게 반복되므로, 별도 헬퍼 메서드나 테스트 전용 fixture 클래스로 추출하면 중복이 줄고 이후 필드가 변경되었을 때 테스트 수정 범위도 줄일 수 있습니다.src/main/java/com/almang/inventory/receipt/service/ReceiptService.java (1)
71-81: 입고 단건 조회 로직은 적절하지만, 트랜잭션 옵션·로그 개선 여지가 있습니다.구현 자체는
findUserById→Store추출 →findReceiptByIdAndValidateAccess→ReceiptResponse.from흐름이 명확하고,RECEIPT_NOT_FOUND/RECEIPT_ACCESS_DENIED매핑도 의도에 잘 맞습니다. 다만 다음 두 가지를 고민해보시면 좋겠습니다.
읽기 전용 트랜잭션으로 정렬
getReceiptFromOrder는@Transactional(readOnly = true)인데,getReceipt는 일반@Transactional입니다.- 둘 다 읽기 전용 조회이므로,
getReceipt도@Transactional(readOnly = true)로 맞추면 JPA의 dirty checking 부담을 줄이고 의도를 더 분명히 표현할 수 있습니다. (참고: Spring Framework Reference - Transaction Management 섹션의 read-only 옵션 설명)로그 정보 확장
- 요청 로그에서
receiptId가 빠져 있어, 404/403 발생 시 어떤 ID에 대한 요청이었는지 추적이 살짝 어려울 수 있습니다.- 예를 들어 아래처럼 receiptId도 함께 남기면 운영 중 디버깅 가독성이 좋아집니다.
- log.info("[ReceiptService] 입고 조회 요청 - userId: {}, storeId: {}", user.getId(), store.getId()); + log.info("[ReceiptService] 입고 조회 요청 - userId: {}, storeId: {}, receiptId: {}", + user.getId(), store.getId(), receiptId);
- 그리고
findReceiptByIdAndValidateAccess쪽의Receipt receipt = receiptRepository...(공백 두 칸)는 사소하지만 한 번 정리해 두면 코드 스타일도 더 깨끗해집니다.위 두 가지는 기능 동작에는 영향 없고, 성능·운영/가독성 측면에서의 추천 사항입니다.
Also applies to: 145-153
src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java (2)
268-320: 발주기반 입고 조회 예외 시나리오를 잘 커버하고 있습니다.
발주기반_입고_조회시_입고가_존재하지_않으면_...,...다른_상점의_발주면_...,...사용자가_존재하지_않으면...세 테스트가 각각RECEIPT_NOT_FOUND,ORDER_ACCESS_DENIED,USER_NOT_FOUND를 정확하게 검증하고 있어서 서비스 로직의 예외 플로우가 안정적으로 보호되고 있습니다.
조금 더 견고하게 만들고 싶다면,BaseException에getErrorCode()같은 접근자가 있다면 현재처럼hasMessageContaining(...)대신 에러 코드 자체를 비교하도록 바꾸는 것을 고려해 보세요. 메시지 리팩터링에 덜 민감하고, 로직 의도를 더 직접적으로 표현할 수 있습니다.
322-428: 입고 단건 조회 서비스 테스트 구성도 탄탄합니다.
입고_조회에_성공한다에서 실제 영속화된Receipt와ReceiptItem을 기반으로 응답 필드를 검증하고, 이어서 세 개 테스트에서RECEIPT_NOT_FOUND,USER_NOT_FOUND,RECEIPT_ACCESS_DENIED각각을 체크해 주셔서 새로 추가된getReceipt경로가 거의 완전히 커버되고 있습니다. 아주 교과서적인 서비스 단 테스트 구성입니다.
위의 발주기반 테스트와 마찬가지로, 가능하다면 예외 검증을 메시지 포함 대신 에러 코드 비교 방식으로 통일하면 이후 메시지 변경(번역, 문구 수정 등)에 더 유연해질 수 있습니다. 또한Receipt+ReceiptItem생성 패턴이 여러 테스트에서 반복되니, 별도 private 헬퍼 메서드로 추출하면 테스트 본문이 “시나리오 설명”에만 집중할 수 있어 가독성이 한층 좋아집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/receipt/controller/ReceiptController.java(1 hunks)src/main/java/com/almang/inventory/receipt/service/ReceiptService.java(2 hunks)src/test/java/com/almang/inventory/receipt/controller/ReceiptControllerTest.java(1 hunks)src/test/java/com/almang/inventory/receipt/service/ReceiptServiceTest.java(4 hunks)
🔇 Additional comments (2)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
50-54: 입고 조회 성공 메시지 추가 일관성 좋습니다.
GET_RECEIPT_SUCCESS("입고 조회 성공")이름/메시지가 기존GET_*_SUCCESS패턴과 잘 맞고, 컨트롤러/테스트에서도 동일하게 사용되어 응답 규격이 깔끔하게 맞춰졌습니다. 별도 수정 필요 없어 보입니다.src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
48-52: RECEIPT_ACCESS_DENIED 코드 추가 방향 적절합니다.
RECEIPT_ACCESS_DENIED(HttpStatus.FORBIDDEN, "해당 상점의 입고가 아닙니다.")가 기존*_ACCESS_DENIED들과 상태 코드/메시지 스타일이 통일되어 있고, 서비스/컨트롤러/테스트에서 잘 활용되고 있어 예외 설계가 명확합니다. 현재 수준에서 추가 변경은 없어도 되겠습니다.
1 participant
✨ 작업 내용
📝 적용 범위
/receipt📌 참고 사항
Summary by CodeRabbit
새로운 기능
버그 수정
테스트
✏️ Tip: You can customize this high-level summary in your review settings.