Conversation
Walkthrough상점 등록 기능을 구현하는 변경사항입니다. REST 컨트롤러, 요청/응답 DTO, 서비스 계층 검증 로직을 추가하고, 글로벌 에러 코드 및 성공 메시지 상수를 확장하며, 도메인 엔티티의 검증 범위를 업데이트합니다. 컨트롤러 및 서비스 계층에 대한 통합 테스트를 포함합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AdminController
participant AdminService
participant StoreRepository
participant Database
Client->>AdminController: POST /api/v1/admin/store<br/>(CreateStoreRequest)
activate AdminController
AdminController->>AdminService: createStore(request)
activate AdminService
Note over AdminService: 검증: 상점명 길이 ≤ 20
Note over AdminService: 검증: 임계치 0 ~ 1 범위
AdminService->>AdminService: toEntity(request)<br/>Store 엔티티 생성
AdminService->>StoreRepository: save(store)
activate StoreRepository
StoreRepository->>Database: INSERT Store
Database-->>StoreRepository: storeId 반환
StoreRepository-->>AdminService: 저장된 Store
deactivate StoreRepository
AdminService->>AdminService: CreateStoreResponse.from(store)
AdminService-->>AdminController: CreateStoreResponse
deactivate AdminService
AdminController-->>Client: ApiResponse<CreateStoreResponse><br/>(status 200, message)
deactivate AdminController
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 주의 필요 영역:
Possibly related PRs
Poem
피드백 포인트: 당신의 구현은 체계적이고 균형잡혀 있습니다. 요청 검증, 서비스 로직, 응답 변환이 깔끔하게 분리되어 있고, 테스트 커버리지도 우수합니다. 다만 다음을 고려하시면 더욱 견고해질 것 같습니다:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 (8)
src/main/java/com/almang/inventory/global/exception/ErrorCode.java (1)
14-16: 임계치 에러 메시지 범위를 더 명확하게 표현하면 좋겠습니다
DEFAULT_COUNT_CHECK_THRESHOLD_NOT_IN_RANGE의 메시지"기본 임계치는 0과 1 사이여야 합니다."는 사용자가 0, 1을 포함하는지 헷갈릴 수 있습니다. 도메인과 엔티티 제약(@DecimalMin("0.00"),@DecimalMax("1.00"))은 0 이상 1 이하를 허용하므로, 아래처럼 바꿔 두면 향후 혼동을 줄일 수 있습니다.DEFAULT_COUNT_CHECK_THRESHOLD_NOT_IN_RANGE( HttpStatus.BAD_REQUEST, "기본 임계치는 0 이상 1 이하(0.00 ~ 1.00)여야 합니다." ),이렇게 하면 에러 메시지만 봐도 허용 범위를 바로 이해할 수 있습니다.
src/main/java/com/almang/inventory/admin/dto/request/CreateStoreRequest.java (1)
7-10: 요청 DTO 설계는 충분히 깔끔합니다 (추가 검증 이관은 선택 사항)현재 구조(
@NotBlank+@NotNull)만으로도 컨트롤러 단의 필수 값 검증은 잘 되어 있고, 범위 검증은 서비스에서ErrorCode와 함께 처리하는 패턴이라 일관성 있습니다.다만, 설계 방향을 조금 더 단순화하고 싶다면(추후 고려용):
defaultCountCheckThreshold에@DecimalMin("0.00"),@DecimalMax("1.00")를 붙여 형식·범위 검증을 DTO 레벨로 당기고,- 서비스에서는 순수 비즈니스 로직만 집중하도록 역할을 분리하는 방법도 있습니다.
이 경우 컨트롤러에서 발생하는 검증 에러가 현재처럼
ErrorCode.INVALID_INPUT_VALUE로 귀결되는지, 혹은 스토어 전용 에러 코드로 내려야 하는지 정책을 먼저 정해야 해서, 지금 당장 변경보다는 “다음 리팩터링 후보” 정도로 두시면 좋겠습니다.src/main/java/com/almang/inventory/admin/controller/AdminController.java (1)
28-38: 리소스 생성 시 HTTP 201 및 Location 헤더도 고려해 볼 만합니다현재 구현은 내부 API 기준으로 실용적인 형태이고, 동작 상 문제는 없습니다. 다만 HTTP 관점에서 조금 더 엄밀하게 가고 싶다면:
POST /store로 리소스를 생성할 때
- 상태 코드를
200 OK대신201 Created로,Location헤더에 새로 생성된 상점의 URI(/api/v1/admin/store/{id}등)를 포함하는 패턴을 사용할 수 있습니다(RFC 7231, 리소스 생성 규약).
예를 들어:
return ResponseEntity.status(HttpStatus.CREATED) .body(ApiResponse.success(SuccessMessage.CREATE_STORE_SUCCESS.getMessage(), response));처럼 상태 코드만 조정해도, 나중에 외부용 API로 확장할 때 재사용성이 좋아집니다. 지금 구조도 충분히 괜찮으니, 설계 방향에 따라 선택적으로 적용하시면 될 것 같습니다.
src/test/java/com/almang/inventory/admin/controller/AdminControllerTest.java (1)
37-84: 컨트롤러 기본 흐름 테스트는 좋고, 비즈니스 예외 케이스를 하나 더 추가해 보면 좋겠습니다성공 케이스와
@NotBlank검증 실패 케이스를 각각 별도 테스트로 나눈 점이 가독성도 좋고,ErrorCode.INVALID_INPUT_VALUE응답 구조까지 잘 검증하고 있습니다. 딱 필요한 것만 깔끔하게 테스트한 느낌입니다.추가로 생각해 볼 만한 점은:
- 서비스에서 던지는
BaseException(예:STORE_NAME_IS_LONG,DEFAULT_COUNT_CHECK_THRESHOLD_NOT_IN_RANGE)이
컨트롤러/글로벌 예외 처리기를 통해 어떤 HTTP 응답으로 매핑되는지,- 이를 MockMvc 레벨에서 한 케이스 정도라도 검증해 두는 것
입니다. 이렇게 하면 “입력 형식 검증 실패(Bean Validation)”와 “비즈니스 규칙 위반(BaseException)”이 서로 다른 응답 패턴을 가진다는 것을 테스트로 문서화할 수 있어, 후속 리팩터링 시 안전망이 조금 더 두꺼워집니다.
src/test/java/com/almang/inventory/admin/service/AdminServiceTest.java (1)
30-95: 예외 검증을 메시지 문자열 대신 ErrorCode 기준으로 하면 더 견고합니다테스트 케이스 구성이 매우 좋습니다. 정상 플로우 + 각 경계 조건(이름 20자 초과, 임계치 <0, >1)을 모두 커버하고 있어서 서비스 레벨 규칙을 잘 문서화하고 있습니다.
다만 예외 검증 부분에서:
assertThatThrownBy(() -> adminService.createStore(request)) .isInstanceOf(BaseException.class) .hasMessageContaining(ErrorCode.STORE_NAME_IS_LONG.getMessage());처럼 메시지 문자열에 의존하고 있는데, 향후 메시지 문구를 조금만 바꿔도(오타 수정, 표현 변경 등) 테스트가 깨질 수 있습니다.
만약
BaseException이getErrorCode()같은 접근자를 제공한다면, 아래와 같이 ErrorCode 자체를 검증하는 쪽이 리팩터링에 더 강합니다.assertThatThrownBy(() -> adminService.createStore(request)) .isInstanceOf(BaseException.class) .extracting("errorCode") .isEqualTo(ErrorCode.STORE_NAME_IS_LONG);또는 커스텀 Assert/AssertJ extension을 두어
assertThatBaseException(e).hasErrorCode(...)와 같은 형태로 감싸 주면, 테스트가 더 읽기 쉬워집니다. BaseException에 이런 필드가 없다면, 다음 기회에 추가를 검토해 보셔도 좋겠습니다.src/main/java/com/almang/inventory/admin/service/AdminService.java (3)
22-33: createStore 흐름은 깔끔합니다만, 저장 결과 활용을 조금 더 명확히 해보면 좋겠습니다.현재
storeRepository.save(store)의 반환값을saved에 담고 있지만, 로그에서는store.getId()를 사용하고 있습니다. JPA 구현체 대부분에서store와saved가 동일 레퍼런스를 가리키지만, 의미적으로는 “저장된 결과”를 사용하는 쪽이 의도가 더 분명합니다.아래처럼 살짝 정리하면 가독성이 좋아집니다.
- Store store = toEntity(request); - Store saved = storeRepository.save(store); - - log.info("[AdminService] 상점 생성 성공 - storeId: {}", store.getId()); - return CreateStoreResponse.from(saved); + Store store = toEntity(request); + Store saved = storeRepository.save(store); + + log.info("[AdminService] 상점 생성 성공 - storeId: {}", saved.getId()); + return CreateStoreResponse.from(saved);작동에는 문제 없지만, 앞으로 유지보수하는 사람이 의도를 읽기 더 쉬워질 것 같습니다.
35-46: 검증 로직에 null 방어를 추가하거나 DTO 레벨 제약을 명시하는 것을 추천드립니다.현재 구현은 기능적으로는 명확하지만,
storeName이나defaultCountCheckThreshold가 null로 들어올 경우 NPE가 발생할 수 있습니다 (storeName.length(),compareTo).두 가지 방향 중 하나를 선택해서 정리해보는 걸 권장합니다.
DTO 레벨에서 Bean Validation으로 막기 (선호)
CreateStoreRequest에@NotNull,@NotBlank,@Size(max = 20),@DecimalMin("0"),@DecimalMax("1")등을 선언하고, 컨트롤러에서@Valid를 사용합니다.- 이렇게 하면 서비스 단에서는 “이미 형식 검증은 끝났다”는 가정하에 비즈니스 규칙만 체크할 수 있습니다.
- 참고: Spring Validation / Bean Validation(JSR-380) 공식 문서.
서비스 레벨에서 null-safe 검증 추가
예시:private void validateStoreName(String storeName) { if (storeName == null || storeName.isBlank()) { throw new BaseException(ErrorCode.STORE_NAME_IS_EMPTY); } if (storeName.length() > 20) { throw new BaseException(ErrorCode.STORE_NAME_IS_LONG); } } private void validateDefaultCountCheckThreshold(BigDecimal threshold) { if (threshold == null) { throw new BaseException(ErrorCode.DEFAULT_COUNT_CHECK_THRESHOLD_NOT_IN_RANGE); } if (threshold.compareTo(BigDecimal.ZERO) < 0 || threshold.compareTo(BigDecimal.ONE) > 0) { throw new BaseException(ErrorCode.DEFAULT_COUNT_CHECK_THRESHOLD_NOT_IN_RANGE); } }또한
20,0,1같은 매직 넘버는 상수(STORE_NAME_MAX_LENGTH,DEFAULT_THRESHOLD_MIN,DEFAULT_THRESHOLD_MAX)로 분리해 두면 재사용과 변경에 더 유리합니다.
48-53: 엔티티 생성 책임을 도메인 쪽으로 옮기면 이후 확장이 더 쉬워질 것 같습니다.
toEntity에서isActivate(true)를 직접 설정하는 방식은 지금은 단순하지만, “상점 생성 시 기본 활성화 여부”라는 규칙을 서비스 계층이 알고 있는 셈입니다. 이 규칙이 비즈니스적으로 바뀌면 서비스 코드를 계속 수정해야 합니다.도메인에 팩토리/정적 메서드를 두는 패턴을 고려해 볼 수 있습니다.
// Store 엔티티 예시 public static Store createActive(String name, BigDecimal defaultCountCheckThreshold) { return Store.builder() .name(name) .isActivate(true) .defaultCountCheckThreshold(defaultCountCheckThreshold) .build(); }그리고 서비스에서는:
- private Store toEntity(CreateStoreRequest request) { - return Store.builder() - .name(request.name()) - .isActivate(true) - .defaultCountCheckThreshold(request.defaultCountCheckThreshold()) - .build(); - } + private Store toEntity(CreateStoreRequest request) { + return Store.createActive( + request.name(), + request.defaultCountCheckThreshold() + ); + }지금 구현도 충분히 읽기 좋고 동작에는 문제가 없지만, 도메인 규칙이 늘어날수록 이런 방식이 유지보수에 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/almang/inventory/admin/controller/AdminController.java(1 hunks)src/main/java/com/almang/inventory/admin/dto/request/CreateStoreRequest.java(1 hunks)src/main/java/com/almang/inventory/admin/dto/response/CreateStoreResponse.java(1 hunks)src/main/java/com/almang/inventory/admin/service/AdminService.java(1 hunks)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/store/domain/Store.java(1 hunks)src/test/java/com/almang/inventory/admin/controller/AdminControllerTest.java(1 hunks)src/test/java/com/almang/inventory/admin/service/AdminServiceTest.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
9-11: 상점 성공 메시지 분리 좋습니다STORE 섹션을 따로 두고
CREATE_STORE_SUCCESS를 추가한 구조가 명확하고, 도메인별로 메시지를 확장하기에도 좋아 보입니다. 별다른 이슈 없이 그대로 사용해도 될 것 같습니다.src/main/java/com/almang/inventory/store/domain/Store.java (1)
38-45: 도메인 설명과 검증 범위를 맞춘 변경이 잘 되어 있습니다기본 알림 임계치 주석을
0.00 ~ 1.00으로 명확히 하고,@DecimalMax("1.00")로 올린 덕분에:
- ErrorCode의 메시지,
- 서비스 레벨 검증 로직,
- 엔티티 제약
이 모두 같은 범위를 바라보게 된 점이 좋습니다. 도메인 규칙이 코드 여러 층에서 일관되게 표현된 상태라 유지보수에도 유리할 것 같습니다.
src/main/java/com/almang/inventory/admin/dto/response/CreateStoreResponse.java (1)
6-19: 매핑 전용 정적 팩토리 메서드 패턴이 깔끔합니다
CreateStoreResponse를 record로 두고,from(Store store)에서만 엔티티 → DTO 변환을 담당하게 한 구조가 명확합니다. 덕분에:
- 서비스에서 매핑 로직이 새지 않고,
- 나중에 응답 스펙이 바뀌어도 이 한 곳만 수정하면 되어 추적이 쉬워집니다.
현재 구현은 그대로 사용해도 무방해 보입니다.
1 participant
✨ 작업 내용
📝 적용 범위
/admin📌 참고 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.