Merged
Conversation
요약Walkthrough발주처 등록 기능의 전체 스택을 구현합니다. REST API 엔드포인트, 서비스 레이어, 요청/응답 DTO, 도메인 엔티티, 성공 메시지, 그리고 컨트롤러 및 서비스 테스트를 포함하며, 도메인 필드를 Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as VendorController
participant Service as VendorService
participant UserRepo as UserRepository
participant VendorRepo as VendorRepository
Client->>Controller: POST /api/v1/vendor<br/>(CreateVendorRequest, UserPrincipal)
activate Controller
Controller->>Service: createVendor(request, userId)
activate Service
Service->>UserRepo: findById(userId)
activate UserRepo
UserRepo-->>Service: User
deactivate UserRepo
Note over Service: User 존재 여부 확인<br/>(없으면 예외 발생)
Service->>Service: toEntity(request, user)
Note over Service: Vendor 엔티티 생성
Service->>VendorRepo: save(vendor)
activate VendorRepo
VendorRepo-->>Service: Vendor (with ID)
deactivate VendorRepo
Service->>Service: VendorResponse.from(vendor)
Service-->>Controller: VendorResponse
deactivate Service
Controller-->>Client: ApiResponse<VendorResponse><br/>(200 OK, CREATE_VENDOR_SUCCESS)
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes 주의깊게 검토해야 할 영역:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 1
🧹 Nitpick comments (7)
src/main/java/com/almang/inventory/vendor/dto/request/CreateVendorRequest.java (1)
7-12: 요청 DTO에 길이 검증 추가를 고려해 보세요
@NotBlank,@NotNull로 필수값 검증은 잘 잡혀 있습니다. 다만, 엔티티에서name과contact_point컬럼이length = 30으로 제한되어 있는데, DTO 레벨에서는 길이 제한이 없어 과도하게 긴 문자열이 들어오면 DB 레벨에서만 에러가 날 수 있습니다.- Jakarta Bean Validation 레벨에서 길이를 함께 제한해 두면, 컨트롤러 단에서 더 일관된 400 응답을 줄 수 있습니다 (Spring Validation / Jakarta Bean Validation 레퍼런스 참고).
예시:
import com.almang.inventory.vendor.domain.VendorChannel; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; public record CreateVendorRequest( - @NotBlank String name, + @NotBlank @Size(max = 30) String name, @NotNull VendorChannel channel, - @NotBlank String contactPoint, + @NotBlank @Size(max = 30) String contactPoint, String note ) {}
max값은 실제 DB 스키마와 반드시 동일하게 맞춰 주세요.src/main/java/com/almang/inventory/vendor/controller/VendorController.java (1)
30-42: 인증 주체 null 방어와 로깅 정보 확장을 고려해 보세요
- 현재 구현은
@AuthenticationPrincipal CustomUserPrincipal userPrincipal이 항상 존재한다고 가정하고userPrincipal.getId()를 호출합니다. 보안 설정상 이 엔드포인트가 반드시 인증 필요로 묶여 있다면 문제 없지만, 설정 변경이나 오작동 시 NPE 로그만 남을 수 있습니다.
- 필요하다면,
userPrincipal == null인 경우 공통 예외(UNAUTHORIZED등)로 치환하는 방어 코드를 두는 것도 한 방법입니다. 이 패턴은 Spring Security 레퍼런스의@AuthenticationPrincipal사용 예시와 함께 보시면 좋습니다.- 디버깅 측면에서는
userId뿐 아니라 발주처 이름까지 남겨두면 추적이 더 쉬워집니다.예시:
Long userId = userPrincipal.getId(); log.info("[VendorController] 발주처 등록 요청 - userId: {}, name: {}", userId, request.name());현재 요구사항 기준으로는 필수는 아니지만, 운영 단계에서 도움이 될 만한 개선 포인트라 참고만 해 두시면 좋겠습니다.
src/test/java/com/almang/inventory/vendor/controller/VendorControllerTest.java (1)
52-139: 컨트롤러 테스트 구성이 탄탄합니다 — 401 케이스만 추가를 고민해 보세요
- 성공 케이스와
USER_NOT_FOUND,INVALID_INPUT_VALUE예외까지 MockMvc로 잘 커버하고 있어서, 컨트롤러 단 계약(HTTP status, 메시지, JSON 구조)을 검증하는 테스트 구성이 좋습니다.- 보안 설정이 이 엔드포인트를 인증 필수로 강제한다면, 추가로 “인증 없이 호출하면 401이 떨어지는지” 를 검증하는 테스트를 하나 더 두면 전체 플로우가 더 완결됩니다.
예시:
@Test void 발주처_등록_시_미인증이면_401_반환한다() throws Exception { CreateVendorRequest request = new CreateVendorRequest( "테스트 발주처", VendorChannel.KAKAO, "010-1111-2222", "비고" ); mockMvc.perform(post("/api/v1/vendor") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isUnauthorized()); }현재 테스트만으로도 기능 요구사항은 잘 커버되고 있으니, 위 케이스는 여유 있을 때 추가해도 충분합니다.
src/test/java/com/almang/inventory/vendor/service/VendorServiceTest.java (4)
35-55: Store/User 생성 헬퍼를 공용 테스트 픽스처로 승격하는 것 고려
newStore,newUser헬퍼가 잘 추상화되어 있어서 읽기 좋습니다. 다만 다른 테스트 클래스에서도 비슷한 상점/유저 생성 로직이 반복된다면,
StoreTestFixture,UserTestFixture같은 공용 테스트 픽스처/빌더 클래스로 분리해 두면:
- 도메인 필드 변경 시 수정 지점을 한 곳으로 모을 수 있고
- “테스트 데이터가 실제 서비스에서 사용하는 패턴과 일관적인지”를 관리하기 더 쉬워집니다.
지금 상태도 충분히 동작에는 문제가 없으므로, 중복이 보이기 시작할 때 리팩터링 대상으로 잡아두면 좋겠습니다.
57-85: 엔티티 상태까지 조금 더 폭넓게 검증하면 매핑 이슈를 잘 잡을 수 있습니다해피 패스 테스트가 응답 DTO와 저장된
Vendor의 핵심 필드를 잘 검증하고 있어서 기본적인 흐름 확인에는 충분합니다.
다만 매핑 누락/오타 같은 회귀를 더 잘 잡고 싶다면, 마지막 부분에서 아래처럼 엔티티의 다른 필드도 함께 확인해 보는 것도 좋습니다.
saved.getChannel(),saved.getContactPoint(),saved.getNote(),saved.isActivated()등도 기대값과 일치하는지 검증이렇게 하면
VendorResponse↔Vendor간 매핑이 변경되었을 때도 테스트가 빨리 깨져서 원인 파악이 수월해집니다.
87-103: 예외 메시지 문자열 대신 에러 코드 필드를 검증하면 더 견고합니다현재는
hasMessageContaining(ErrorCode.USER_NOT_FOUND.getMessage())로 검증하고 있어서, 에러 메시지(i18n, 문구 수정 등)가 바뀌면 테스트가 쉽게 깨질 수 있습니다.
BaseException이ErrorCode필드를 노출한다면, 아래처럼 “코드” 자체를 검증하는 쪽이 더 안전합니다.예시:
assertThatThrownBy(() -> vendorService.createVendor(request, notExistUserId)) .isInstanceOf(BaseException.class) .extracting("errorCode") .isEqualTo(ErrorCode.USER_NOT_FOUND);또는
getErrorCode()게터가 있다면extracting(BaseException::getErrorCode)를 사용하는 방식도 가능합니다.
이렇게 하면 메시지 포맷 변경과 무관하게 비즈니스 에러 시맨틱을 안정적으로 보장할 수 있습니다.
1-128: 전체적으로: 핵심 플로우·에러 케이스를 잘 커버하는 서비스 테스트입니다
- 성공/실패(사용자 미존재)·옵션 필드(null 비고)까지 주요 시나리오를 분리해서 검증하고 있고,
- 테스트 데이터 생성 헬퍼도 적절히 분리되어 읽기와 수정이 쉬운 편입니다.
위에 남긴 몇 가지 리팩터 제안(에러 코드 검증, 엔티티 필드 검증 확장, 픽스처 공유)은 선택적으로 적용하셔도 되고, 현 상태로도 기능 구현 품질을 확인하는 데는 충분해 보입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java(1 hunks)src/main/java/com/almang/inventory/vendor/controller/VendorController.java(1 hunks)src/main/java/com/almang/inventory/vendor/domain/Vendor.java(1 hunks)src/main/java/com/almang/inventory/vendor/dto/request/CreateVendorRequest.java(1 hunks)src/main/java/com/almang/inventory/vendor/dto/response/VendorResponse.java(1 hunks)src/main/java/com/almang/inventory/vendor/service/VendorService.java(1 hunks)src/test/java/com/almang/inventory/product/service/ProductServiceTest.java(1 hunks)src/test/java/com/almang/inventory/vendor/controller/VendorControllerTest.java(1 hunks)src/test/java/com/almang/inventory/vendor/service/VendorServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-20T10:43:47.489Z
Learnt from: JoonKyoLee
Repo: almang2/inventory-server PR: 33
File: src/main/java/com/almang/inventory/product/domain/Product.java:62-66
Timestamp: 2025-11-20T10:43:47.489Z
Learning: In the almang2/inventory-server repository, Product entity update methods (e.g., updateVendor in src/main/java/com/almang/inventory/product/domain/Product.java) do not require null checks on vendor parameters because ProductService validates vendor existence via findVendorByIdAndValidateAccess before calling update methods, and the vendor field has nullable=false constraint ensuring this.vendor is never null for persisted entities.
Applied to files:
src/main/java/com/almang/inventory/vendor/service/VendorService.javasrc/main/java/com/almang/inventory/vendor/dto/request/CreateVendorRequest.javasrc/test/java/com/almang/inventory/product/service/ProductServiceTest.javasrc/main/java/com/almang/inventory/vendor/controller/VendorController.javasrc/main/java/com/almang/inventory/vendor/domain/Vendor.javasrc/test/java/com/almang/inventory/vendor/service/VendorServiceTest.javasrc/main/java/com/almang/inventory/vendor/dto/response/VendorResponse.javasrc/test/java/com/almang/inventory/vendor/controller/VendorControllerTest.java
🔇 Additional comments (7)
src/main/java/com/almang/inventory/vendor/domain/Vendor.java (1)
38-39:activated필드 리네이밍 일관성 좋습니다
- DB 컬럼명을
"is_activate"로 유지하면서 자바 필드명을activated로 바꾼 선택이 적절합니다. 기존 스키마는 깨지지 않고, 도메인 모델에서는 보다 자연스러운 불리언 이름을 쓰게 되었습니다.- Lombok
@Getter/@Builder조합에서isActivated()/.activated(true)시그니처로 노출되므로, 테스트와 다른 코드에서의 사용도 혼동 없이 이어질 수 있습니다.src/test/java/com/almang/inventory/product/service/ProductServiceTest.java (1)
50-60: Vendor 빌더 필드명 변경 반영이 정확합니다
Vendor.builder().activated(true)로 수정하여, 엔티티 필드명 변경과 테스트 코드가 잘 정렬됐습니다.newVendor(Store)가 여러 테스트에서 공통으로 사용되기 때문에, 이 한 곳만 맞춰주면 전체 테스트 일관성이 유지되는 구조도 좋습니다.src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
30-32: 발주처 성공 메시지 enum 추가가 패턴과 잘 맞습니다
CREATE_VENDOR_SUCCESS("발주처 등록 성공")이 기존CREATE_*_SUCCESS네이밍 및 한국어 메시지 스타일과 자연스럽게 일치합니다.- 컨트롤러에서 공통
ApiResponse메시지로 재사용하기에도 명확한 이름입니다.src/main/java/com/almang/inventory/vendor/dto/response/VendorResponse.java (1)
6-25: VendorResponse 매핑이 단순하고 유지보수에 유리합니다
VendorResponse를record로 정의하고,from(Vendor)정적 팩토리로 매핑을 한 곳에 모은 구조가 좋습니다. 필드 추가/변경 시 이 메서드만 보면 되므로, API 스펙 관리가 수월해집니다.vendor.isActivated()와vendor.getStore().getId()를 그대로 사용해 활성 여부와 소속 상점을 노출하는 것도, 현재 도메인 모델을 잘 반영하고 있습니다.src/main/java/com/almang/inventory/vendor/service/VendorService.java (1)
36-50: 도메인 매핑과 사용자 검증 흐름이 기존 패턴과 잘 맞습니다
toEntity에서user.getStore()를 통해 발주처를 항상 사용자 소속 상점에 귀속시키는 구조는, 상품 관련 서비스에서 사용하던 접근 제어 패턴과도 잘 어울립니다.findUserById에서USER_NOT_FOUND를 바로 던지도록 한 것도 서비스 레이어에서 선 검증 후 도메인 메서드를 호출한다는 기존 합의와 일관됩니다. Based on learningssrc/test/java/com/almang/inventory/vendor/service/VendorServiceTest.java (2)
25-33: 서비스 레이어 통합 테스트 구성, 현재 요구사항에 잘 맞습니다
@SpringBootTest+@Transactional+@ActiveProfiles("test")조합으로 실제 DB·리포지토리까지 포함한 end-to-end 흐름을 검증하고 있어서, 새로 추가된 발주처 등록 기능의 안정성을 확인하기에 충분한 수준입니다.
추후 성능이나 테스트 속도가 이슈가 되면, 그때@DataJpaTest/슬라이스 테스트로 분리하는 방향을 검토해도 좋겠습니다.
105-128: 비고(null) 케이스까지 분리해서 테스트한 점이 스펙을 잘 드러냅니다비고가 없을 때
note가null로 유지되면서도 발주처 생성이 성공해야 한다는 요구사항이 테스트 이름과 검증 코드에 잘 드러나 있습니다.
해피 패스(비고 있음)와 옵션 필드(비고 없음)를 별도 테스트로 나눈 구조도 가독성과 유지보수 측면에서 깔끔합니다.
Comment on lines
+24
to
+34
| @Transactional | ||
| public VendorResponse createVendor(CreateVendorRequest request, Long userId) { | ||
| User user = findUserById(userId); | ||
|
|
||
| log.info("[VendorService] 발주처 생성 요청 - userId: {}", user.getId()); | ||
| Vendor vendor = toEntity(request, user); | ||
| Vendor saved = vendorRepository.save(vendor); | ||
|
|
||
| log.info("[VendorService] 발주처 생성 성공 - vendorId: {}", vendor.getId()); | ||
| return VendorResponse.from(saved); | ||
| } |
There was a problem hiding this comment.
저장 후 로그에서는 saved.getId() 를 사용하는 것이 더 안전합니다
vendorRepository.save(vendor)호출 이후 로그에서vendor.getId()를 사용하고 있는데, JPA 구현체에 따라 아직 ID 가 세팅되지 않았거나, 가독성 측면에서 “저장된 엔티티” 인지 모호할 수 있습니다.- 바로 위에서
saved를 사용하고 있으므로, 실제로는saved.getId()를 사용하는 편이 더 명확합니다.
예시 수정:
- Vendor vendor = toEntity(request, user);
- Vendor saved = vendorRepository.save(vendor);
-
- log.info("[VendorService] 발주처 생성 성공 - vendorId: {}", vendor.getId());
- return VendorResponse.from(saved);
+ Vendor vendor = toEntity(request, user);
+ Vendor saved = vendorRepository.save(vendor);
+
+ log.info("[VendorService] 발주처 생성 성공 - vendorId: {}", saved.getId());
+ return VendorResponse.from(saved);기능에는 직접 영향이 없지만, 운영 로그를 신뢰 가능하게 만들기 위해서는 이렇게 정리해 두는 편이 좋습니다.
🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/vendor/service/VendorService.java around
lines 24 to 34, the post-save log uses vendor.getId() instead of the persisted
entity's ID; change the log call to reference saved.getId() (i.e., replace
vendor.getId() with saved.getId()) so the log reports the ID from the entity
returned by vendorRepository.save().
This was referenced Nov 21, 2025
Merged
Merged
Merged
This was referenced Nov 22, 2025
Merged
This was referenced Nov 30, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 작업 내용
📝 적용 범위
/vendor📌 참고 사항
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.