Merged
Conversation
Walkthrough사용자 정보 조회 기능을 구현하기 위해 성공 메시지 열거형, REST 컨트롤러, 서비스 계층, 응답 DTO를 추가하고, 성공 및 오류 시나리오를 다루는 단위 및 통합 테스트를 작성했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant UserRepo as UserRepository
participant StoreRepo as StoreRepository
participant DB
Client->>Controller: POST /api/v1/users/me<br/>(with auth token)
activate Controller
Controller->>Service: getUserProfile(userId)
deactivate Controller
activate Service
Service->>UserRepo: findById(userId)
activate UserRepo
UserRepo->>DB: SELECT * FROM user
DB-->>UserRepo: User
UserRepo-->>Service: User
deactivate UserRepo
Note over Service: User가 null이면<br/>USER_NOT_FOUND 예외 발생
Service->>Service: user.getStore()
Service->>StoreRepo: (Store 정보 접근)
Service-->>Controller: UserProfileResponse
deactivate Service
Controller-->>Client: ApiResponse<UserProfileResponse><br/>HTTP 200 ✓
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 추가 검토 필요 영역:
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 (6)
src/main/java/com/almang/inventory/user/dto/response/UserProfileResponse.java (1)
1-21: DTO 구조와 매핑은 명확하지만, 팩토리 메서드 시그니처를 단순화할 여지가 있습니다.
- 현재
from(User user, Store store)에서 하는 일은user.getStore()를 호출하는 것과 거의 동일해서, 서비스 단에서 굳이Store를 분리해 넘길 필요는 없어 보입니다.
→public static UserProfileResponse from(User user)형태로 단순화하고, 내부에서Store store = user.getStore();를 사용하면 호출부가 더 깔끔해집니다.- 한 가지 확인 포인트는
User–Store연관관계입니다. 도메인에서store가optional = false로 보장되지 않는다면,store.getName()에서 NPE 가능성이 생깁니다. 연관관계에서 필수 여부를 강제하거나, 여기서 방어 로직(예:store == null ? null : store.getName())을 두는지 검토해 보시면 좋겠습니다.전반적인 필드 구성과 응답 구조는 요구사항과 잘 맞습니다. 딱 필요한 정보만 뽑아서 주는 점이 좋네요.
src/test/java/com/almang/inventory/user/service/UserServiceTest.java (2)
22-68: 통합 테스트 구성과 픽스처 설계가 깔끔합니다.
@SpringBootTest+@Transactional+@ActiveProfiles("test")조합으로 실제 빈/DB 환경에서 조회 로직을 검증하는 구조가 잘 잡혀 있습니다.newStore(),newUser(Store store)헬퍼로 픽스처 생성을 캡슐화한 것도 테스트 가독성과 재사용성 측면에서 좋은 선택입니다.- 성공 케이스에서 문자열 리터럴 대신
savedUser/store객체의 값을 그대로 검증해도(예:assertThat(response.username()).isEqualTo(savedUser.getUsername())) 변경에 조금 더 강한 테스트가 됩니다. 선택 사항이지만, 추후 도메인 변경 시 테스트 유지보수성이 올라갑니다.잘 구성된 통합 테스트라 “서비스 단의 스펙”을 읽기에도 좋습니다.
70-79: 예외 검증 시 메시지 문자열 의존도를 줄이면 테스트가 더 견고해집니다.현재는 아래와 같이 예외 메시지 텍스트를 부분 문자열로 검증하고 있습니다.
assertThatThrownBy(() -> userService.getUserProfile(notExistUserId)) .isInstanceOf(BaseException.class) .hasMessageContaining(ErrorCode.USER_NOT_FOUND.getMessage());
- 메시지는 번역/표현 변경에 의해 쉽게 바뀔 수 있어, 도메인 규칙보다는 “문구”에 테스트가 종속되는 형태입니다.
BaseException이getErrorCode()와 같은 접근자를 제공한다면,extracting("errorCode").isEqualTo(ErrorCode.USER_NOT_FOUND)식으로 에러 코드 중심으로 검증하는 방식을 추천드립니다. 도메인 에러 정책에 더 직접적으로 매핑됩니다.참고로 AssertJ 공식 문서의
extracting,satisfies등 기능을 활용하면 도메인 예외 검증을 더 풍부하게 표현할 수 있습니다.src/main/java/com/almang/inventory/user/service/UserService.java (1)
20-37: 조회 로직은 명확하지만,Store널 처리와 로깅 레벨/위치에 대해 한 번 점검해 보면 좋겠습니다.
getUserProfile에서User user = findUserById(userId);후Store store = user.getStore();를 곧바로 사용하고 있습니다. 도메인에서User.store가 항상 존재한다는 제약이 없다면, 여기서 NPE가 발생할 수 있습니다.
- 연관관계 매핑에서
optional = false/nullable = false로 강제하거나,- 이 메서드에서
store == null인 경우에 대한 별도 처리(예: 다른 에러 코드, 기본값)를 두는 방식을 검토해 주세요.- 로그가 컨트롤러(
UserController)와 서비스 양쪽에서 모두 INFO 레벨로 남고 있어, 트래픽이 많은 경우 로그 노이즈가 될 수 있습니다.
- “요청/응답 추적”을 컨트롤러 레이어에 집중시키고, 서비스는 도메인적으로 의미 있는 이벤트에만 로그를 남기거나,
- 서비스 로그를 DEBUG 레벨로 내리는 것도 한 가지 선택지입니다.
트랜잭션을
readOnly = true로 분리한 점과, 조회 + 예외 처리를findUserById로 캡슐화한 구조는 서비스 레이어 설계 측면에서 좋습니다.src/test/java/com/almang/inventory/user/controller/UserControllerTest.java (1)
41-71: 성공 케이스 테스트가 잘 짜여 있지만, 서비스 호출 인자를 검증하면 버그 탐지력이 더 좋아집니다.
CustomUserPrincipal에서userId = 1L을 세팅하고 있지만,when(userService.getUserProfile(anyLong()))로 스텁을 설정해서 컨트롤러가 잘못된 ID를 넘겨도 테스트가 통과합니다.- 아래와 같이
verify를 추가하면, 컨트롤러가 인증 주체의 ID를 정확히 넘기는지까지 보장할 수 있습니다.// given when(userService.getUserProfile(userId)).thenReturn(response); // when & then 이후 검증 verify(userService).getUserProfile(userId);전반적으로 JSON 구조와 메시지까지 꼼꼼히 검증하고 있어서, API 계약을 문서처럼 보여주는 좋은 테스트입니다.
src/main/java/com/almang/inventory/user/controller/UserController.java (1)
18-39: 엔드포인트 설계는 명확하지만, HTTP 메서드 선택과 인증 주체 널 처리에 대해 한 번 더 고민해 볼 만합니다.
- 현재
@PostMapping("/me")로 “조회”를 처리하고 있는데, REST 관점에서는@GetMapping("/me")가 더 자연스럽습니다.
- 캐싱, 프록시, 클라이언트 라이브러리(예: OpenAPI 코드 생성기) 입장에서도 조회성 요청은
GET으로 노출하는 것이 일반적인 베스트 프랙티스입니다.- 만약 기존 클라이언트 제약 등으로 인해
POST를 유지해야 한다면, Swagger 문서 쪽 설명에 그 이유를 명시해 두는 것도 좋겠습니다.@AuthenticationPrincipal CustomUserPrincipal userPrincipal이 항상 non-null이라는 전제가 Spring Security 설정으로 보장되는지 한 번 확인해 주세요.
- 만약 익명 접근 가능성이나 잘못된 설정이 있다면,
userPrincipal == null일 때 별도 예외를 던지는 방어 코드도 고려할 수 있습니다.
ApiResponse.success와SuccessMessage.GET_USER_PROFILE_SUCCESS를 사용해 응답 포맷과 메시지를 한 곳에서 관리하는 구조는 깔끔합니다. 딱 보기 좋은 표준 컨트롤러입니다.
📜 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/user/controller/UserController.java(1 hunks)src/main/java/com/almang/inventory/user/dto/response/UserProfileResponse.java(1 hunks)src/main/java/com/almang/inventory/user/service/UserService.java(1 hunks)src/test/java/com/almang/inventory/user/controller/UserControllerTest.java(1 hunks)src/test/java/com/almang/inventory/user/service/UserServiceTest.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
8-18:GET_USER_PROFILE_SUCCESS추가 방향 적절합니다.기존 성공 메시지 패턴(한글 설명, 과거 시제/완료형)과 잘 맞고, 컨트롤러에서 재사용하기에도 명확한 이름입니다. 추가적인 수정 포인트는 없어 보입니다.
src/test/java/com/almang/inventory/user/controller/UserControllerTest.java (1)
73-93: 에러 케이스 테스트 구성 좋습니다.
UserService.getUserProfile에서BaseException(USER_NOT_FOUND)가 발생했을 때, HTTP 404 상태, 에러 메시지,data필드 부재까지 모두 검증하고 있어 에러 응답 스펙이 명확하게 고정됩니다.- 성공/실패 모두 같은 엔드포인트(
/api/v1/users/me)를 타게 해서, 보안 컨텍스트·응답 래핑(ApiResponse)을 실제와 동일하게 검증하고 있는 점도 좋습니다.이 수준의 컨트롤러 테스트면 회귀 버그를 잡는 데 꽤 든든하겠습니다.
Merged
This was referenced Nov 20, 2025
Merged
Merged
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.
✨ 작업 내용
📝 적용 범위
/user📌 참고 사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트