Demo 테스트 #12
Demo 테스트 #12
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough사용자 관리 기능을 위한 완전한 계층 구조를 구현합니다. JPA 엔티티(UserModel), 도메인 서비스(UserService), 리포지토리 패턴, 애플리케이션 파사드(UserFacade), REST API 컨트롤러, DTOs, 그리고 포괄적인 테스트 스위트를 추가하여 사용자 생성, 조회, 포인트 충전 기능을 제공합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as UserV1Controller
participant Facade as UserFacade
participant Service as UserService
participant Repo as UserRepositoryImpl
participant JpaRepo as UserJpaRepository
participant DB as Database
Client->>Controller: POST /api/v1/users
Controller->>Facade: createUser(id, email, birthDate, gender)
Facade->>Service: createUser(id, email, birthDate, gender)
Service->>Repo: existsById(id)
Repo->>JpaRepo: existsByUserId(id)
JpaRepo->>DB: SELECT * FROM users WHERE user_id=?
DB-->>JpaRepo: (empty or found)
JpaRepo-->>Repo: boolean result
Repo-->>Service: boolean result
alt ID 이미 존재
Service-->>Facade: throw CoreException(BAD_REQUEST)
Facade-->>Controller: exception
Controller-->>Client: 400 Bad Request
else ID 미존재 (정상)
Service->>Service: new UserModel(id, email, birthDate, gender)
Service->>Repo: save(userModel)
Repo->>JpaRepository: save(userModel)
JpaRepository->>DB: INSERT INTO users ...
DB-->>JpaRepository: id
JpaRepository-->>Repo: UserModel
Repo-->>Service: UserModel
Service-->>Facade: UserModel
Facade->>Facade: UserInfo.from(userModel)
Facade-->>Controller: UserInfo
Controller->>Controller: UserV1Dto.UserResponse.from(userInfo)
Controller-->>Client: 200 OK {id, email, birthDate, gender, point}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
156-192: 포인트 충전 테스트가 잘 작성되었습니다.0 이하의 값과 양수 충전 케이스가 모두 테스트되었으며, 예외 타입과 메시지도 적절히 검증하고 있습니다.
추가로 다음 엣지 케이스들을 테스트하면 더 견고한 코드가 됩니다:
@DisplayName("여러 번 충전하면, 포인트가 누적된다.") @Test void accumulatesPoints_whenChargingMultipleTimes() { // arrange UserModel userModel = new UserModel("user123", "test@example.com", "1990-01-01", "M"); // act userModel.chargePoint(1000); userModel.chargePoint(500); // assert assertThat(userModel.getPoint()).isEqualTo(1500); } @DisplayName("Integer.MAX_VALUE에 가까운 포인트 충전 시 오버플로우 처리를 확인한다.") @Test void handlesIntegerOverflow_whenChargingNearMaxValue() { // 오버플로우 방지 로직이 있다면 테스트 }apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java (2)
8-8: Bean Validation 어노테이션 사용을 고려하세요.현재 요청 검증이 컨트롤러에서 수동으로 이루어지고 있습니다 (UserV1Controller Line 24-26). Bean Validation 어노테이션을 사용하면 검증 로직을 선언적으로 표현할 수 있고 코드 중복을 줄일 수 있습니다.
+import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.NotBlank; + -public record UserCreateRequest(String id, String email, String birthDate, String gender) {} +public record UserCreateRequest( + @NotBlank String id, + @Email @NotBlank String email, + @NotBlank String birthDate, + @NotBlank String gender +) {}그런 다음 컨트롤러에서
@Valid어노테이션을 사용하세요:public ApiResponse<UserV1Dto.UserResponse> createUser( @Valid @RequestBody UserV1Dto.UserCreateRequest request )
28-28: PointChargeRequest에도 검증 추가를 고려하세요.충전 금액에 대한 검증도 추가하면 좋습니다.
+import jakarta.validation.constraints.Positive; + -public record PointChargeRequest(Integer amount) {} +public record PointChargeRequest(@Positive Integer amount) {}apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
19-31: Bean Validation 사용을 권장합니다.수동 검증 대신 DTO에 Bean Validation 어노테이션을 사용하면 더 선언적이고 유지보수하기 쉬운 코드가 됩니다. UserV1Dto.java에 대한 리뷰 코멘트를 참고하세요.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (2)
40-64: 입력 검증 테스트를 추가하는 것을 권장합니다.현재 유효한 입력에 대한 테스트만 포함되어 있습니다. 다음과 같은 검증 테스트를 추가하면 더 견고한 테스트 커버리지를 확보할 수 있습니다:
- null 또는 빈 문자열 입력 처리
- 잘못된 날짜 형식 (예: "invalid-date")
- 잘못된 이메일 형식
- 잘못된 성별 값 (예: "X")
다음과 같은 테스트를 추가할 수 있습니다:
@DisplayName("유효하지 않은 이메일 형식으로 가입하면, 예외가 발생한다.") @Test void throwsException_whenInvalidEmailIsProvided() { // arrange String id = "user123"; String invalidEmail = "invalid-email"; String birthDate = "1990-01-01"; String gender = "M"; // act & assert assertThrows(CoreException.class, () -> { userService.createUser(id, invalidEmail, birthDate, gender); }); } @DisplayName("null ID로 가입하면, 예외가 발생한다.") @Test void throwsException_whenNullIdIsProvided() { assertThrows(CoreException.class, () -> { userService.createUser(null, "test@example.com", "1990-01-01", "M"); }); }
193-214: 엣지 케이스 테스트를 추가하는 것을 권장합니다.현재 정상적인 포인트 충전만 테스트하고 있습니다. 다음과 같은 엣지 케이스에 대한 테스트를 추가하면 더 견고한 구현을 보장할 수 있습니다:
- 음수 포인트 충전 시도 (거부되어야 함)
- 0 포인트 충전
- 매우 큰 값으로 인한 Integer 오버플로우
- 동시성 시나리오 (여러 스레드가 동시에 같은 사용자의 포인트 충전)
예시 테스트:
@DisplayName("음수 포인트를 충전하면, 예외가 발생한다.") @Test void throwsException_whenNegativeAmountIsProvided() { // arrange String id = "user123"; userJpaRepository.save(new UserModel(id, "test@example.com", "1990-01-01", "M")); // act & assert assertThrows(CoreException.class, () -> { userService.chargePoint(id, -1000); }); } @DisplayName("Integer 최대값을 초과하는 포인트 충전 시, 오버플로우를 방지한다.") @Test void preventsOverflow_whenChargingMaxIntegerPoints() { // arrange String id = "user123"; UserModel user = userJpaRepository.save( new UserModel(id, "test@example.com", "1990-01-01", "M") ); userService.chargePoint(id, Integer.MAX_VALUE - 100); // act & assert assertThrows(CoreException.class, () -> { userService.chargePoint(id, 200); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1ApiSpec.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java(1 hunks)http/commerce-api/user-v1.http(1 hunks)
🔇 Additional comments (13)
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java (1)
8-8: LGTM!
existsByUserId는boolean을 반환하므로 null-safety 문제가 없으며 올바르게 구현되었습니다.apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.java (1)
5-9: LGTM!도메인 레이어의 리포지토리 인터페이스가 잘 설계되었습니다:
findById가Optional을 반환하여 null-safety 보장- 인프라 레이어의 구현 세부사항을 추상화
- 리포지토리 패턴의 표준 관례 준수
apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java (1)
6-16: LGTM!애플리케이션 레이어의 DTO로 Java record를 사용한 것은 적절한 선택입니다:
- 불변 객체로 스레드 안전성 보장
- 정적 팩토리 메서드
from()으로 도메인 모델에서 깔끔하게 변환- 이미 검증된
UserModel로부터 생성되므로 추가 검증 불필요- 필드명 변경(
userId→id)은 API 응답 관점에서 더 간결함apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
17-152: Let me search for the actual UserModel implementation to verify if gender validation exists:사용자 생성 검증 테스트가 ID, 이메일, 생년월일에 대해 포괄적으로 작성되었습니다.
ID, 이메일, 생년월일의 모든 주요 검증 규칙이 테스트되었으며, AAA 패턴을 잘 따르고 있습니다. 다만 성별(gender) 필드에 대한 검증 규칙이 UserModel에 존재하는지 확인하고, 존재한다면 해당 검증을 테스트하는 케이스를 추가하는 것을 권장합니다.
apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (1)
1-250: E2E 테스트 커버리지가 훌륭합니다.테스트가 성공 케이스와 실패 케이스를 모두 포괄적으로 커버하고 있으며, 중첩 클래스로 잘 구조화되어 있습니다. 데이터베이스 정리 로직도 적절하게 구현되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
15-23: 중복 ID 검증 로직이 잘 구현되어 있습니다.사용자 생성 전 ID 중복을 검사하고 적절한 예외를 던지는 로직이 잘 구현되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1ApiSpec.java (1)
1-50: API 스펙이 Swagger 문서화와 함께 명확하게 정의되어 있습니다.REST API 계약이 깔끔하게 정의되어 있으며, 각 엔드포인트에 대한 설명이 적절합니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
1-30: 리포지토리 구현이 깔끔합니다.도메인 리포지토리 인터페이스를 JPA 리포지토리로 적절히 어댑팅하고 있으며, Optional을 올바르게 사용하고 있습니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
66-75: chargePoint 엔드포인트 구현이 깔끔합니다.파사드를 통해 포인트 충전을 수행하고 적절한 응답을 반환하고 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java (2)
28-50: 도메인 검증 로직이 잘 구현되어 있습니다.ID 검증이 포괄적이며 적절한 오류 메시지와 함께 예외를 던지고 있습니다. 패턴 매칭과 길이 제한이 적절합니다.
52-70: 이메일 및 생년월일 검증이 적절합니다.이메일 정규식과 날짜 파싱 로직이 잘 구현되어 있으며, 적절한 예외 처리가 되어 있습니다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (2)
18-32: 테스트 구조가 잘 설계되어 있습니다.통합 테스트를 위한 Spring Boot 설정과 데이터베이스 정리를 통한 테스트 격리가 적절하게 구현되어 있습니다.
177-189: 예외 처리 방식이 적절합니다.존재하지 않는 사용자에 대해 NOT_FOUND 예외를 발생시키는 것은 올바른 접근입니다. 이 패턴을
getUserById및getUserPoint메서드에도 적용하면 API의 일관성이 향상됩니다.
| public UserInfo getUserById(String id) { | ||
| UserModel user = userService.getUserById(id); | ||
| return user != null ? UserInfo.from(user) : null; | ||
| } |
There was a problem hiding this comment.
null 반환 대신 일관된 예외 처리를 고려하세요.
getUserById는 사용자가 없을 때 null을 반환하지만, chargePoint (Line 28)는 서비스 레이어에서 예외를 던집니다. 이러한 불일치는 API 레이어(UserV1Controller)에서 null 체크와 예외 처리를 혼용하게 만들어 오류 처리 로직을 복잡하게 만듭니다.
모든 메서드가 사용자를 찾을 수 없을 때 일관되게 예외를 던지도록 변경을 고려하세요:
public UserInfo getUserById(String id) {
UserModel user = userService.getUserById(id);
- return user != null ? UserInfo.from(user) : null;
+ if (user == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다.");
+ }
+ return UserInfo.from(user);
}🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java
around lines 18 to 21, the method currently returns null when a user is not
found which is inconsistent with other methods that throw exceptions; change
getUserById to throw a consistent not-found exception instead of returning null
by checking the UserModel result and, when absent, throw an appropriate runtime
(e.g., UserNotFoundException or a standard NotFound/EntityNotFoundException)
with a clear message; ensure callers (controllers) no longer need to null-check
and update method signature or javadocs if needed to reflect the exception-based
behavior.
| public Integer getUserPoint(String id) { | ||
| return userService.getUserPoint(id); | ||
| } |
There was a problem hiding this comment.
getUserPoint도 null 대신 예외를 던지도록 수정하세요.
getUserPoint도 null을 반환하는데, 이는 동일한 일관성 문제를 야기합니다.
public Integer getUserPoint(String id) {
- return userService.getUserPoint(id);
+ Integer point = userService.getUserPoint(id);
+ if (point == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다.");
+ }
+ return point;
}🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java
around lines 23 to 25, change the method so it does not return null when
userService.getUserPoint(id) yields null: call userService.getUserPoint(id),
check the result for null, and if null throw an appropriate runtime or domain
exception (e.g., new UserNotFoundException("No points for user: "+id) or new
IllegalStateException("No points for user: "+id)); otherwise return the integer
value. Ensure you import or create the chosen exception type as needed.
| public void chargePoint(Integer amount) { | ||
| if (amount == null || amount <= 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "충전할 포인트는 0보다 큰 정수여야 합니다."); | ||
| } | ||
| this.point += amount; | ||
| } |
There was a problem hiding this comment.
정수 오버플로우 가능성을 확인하세요.
point += amount 연산에서 정수 오버플로우가 발생할 수 있습니다. 포인트가 Integer.MAX_VALUE에 가까울 때 충전하면 음수로 래핑될 수 있습니다.
오버플로우 검증을 추가하세요:
public void chargePoint(Integer amount) {
if (amount == null || amount <= 0) {
throw new CoreException(ErrorType.BAD_REQUEST, "충전할 포인트는 0보다 큰 정수여야 합니다.");
}
+ if (this.point > Integer.MAX_VALUE - amount) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "포인트 충전 한도를 초과했습니다.");
+ }
this.point += amount;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void chargePoint(Integer amount) { | |
| if (amount == null || amount <= 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "충전할 포인트는 0보다 큰 정수여야 합니다."); | |
| } | |
| this.point += amount; | |
| } | |
| public void chargePoint(Integer amount) { | |
| if (amount == null || amount <= 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "충전할 포인트는 0보다 큰 정수여야 합니다."); | |
| } | |
| if (this.point > Integer.MAX_VALUE - amount) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트 충전 한도를 초과했습니다."); | |
| } | |
| this.point += amount; | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java around
lines 72 to 77, the chargePoint method can overflow when adding amount to
this.point; before doing the addition, validate that this.point is non-null (or
treat null as 0), and check that (long)this.point + (long)amount <=
Integer.MAX_VALUE (or use Math.addExact in a try/catch) and if it would overflow
throw a CoreException(ErrorType.BAD_REQUEST, "충전할 포인트가 최대값을 초과합니다."); only after
that safe check perform the addition.
| @Transactional(readOnly = true) | ||
| public UserModel getUserById(String id) { | ||
| return userRepository.findById(id) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
null 반환 대신 일관된 예외 처리 패턴을 사용하세요.
getUserById는 null을 반환하지만, chargePoint (Line 40-41)는 예외를 던집니다. 서비스 레이어 전체에서 일관된 오류 처리 전략을 사용하는 것이 좋습니다.
사용자를 찾을 수 없을 때 일관되게 예외를 던지도록 수정하세요:
@Transactional(readOnly = true)
public UserModel getUserById(String id) {
return userRepository.findById(id)
- .orElse(null);
+ .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다."));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
around lines 25 to 29, change the null-returning getUserById to throw a
consistent not-found exception instead of returning null: replace the
orElse(null) with orElseThrow(...) to throw a domain-appropriate exception
(e.g., UserNotFoundException or an existing EntityNotFoundException) that
includes the user id; if UserNotFoundException does not exist, add it as a
simple runtime exception, and update any callers (like chargePoint) to rely on
this exception rather than handling null.
| @Transactional(readOnly = true) | ||
| public Integer getUserPoint(String id) { | ||
| UserModel user = userRepository.findById(id) | ||
| .orElse(null); | ||
| return user != null ? user.getPoint() : null; | ||
| } |
There was a problem hiding this comment.
getUserPoint도 예외 던지기로 통일하세요.
동일한 일관성 문제가 있습니다.
@Transactional(readOnly = true)
public Integer getUserPoint(String id) {
- UserModel user = userRepository.findById(id)
- .orElse(null);
- return user != null ? user.getPoint() : null;
+ UserModel user = userRepository.findById(id)
+ .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다."));
+ return user.getPoint();
}🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
around lines 31 to 36, the method currently returns null when a user is not
found; make it consistent with the rest of the service by throwing an exception
instead. Replace the orElse(null) usage with orElseThrow(...) and throw an
appropriate exception (e.g., EntityNotFoundException or a custom
UserNotFoundException) including the user id in the message, then return
user.getPoint(); this preserves the return type but ensures missing users raise
an error rather than returning null.
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| public interface UserJpaRepository extends JpaRepository<UserModel, Long> { | ||
| UserModel findByUserId(String userId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
findByUserId가 Optional을 반환하도록 변경하세요.
현재 findByUserId는 UserModel을 직접 반환하므로 사용자를 찾지 못할 경우 null을 반환할 수 있습니다. 이는 NullPointerException 위험과 null-safety 문제를 야기합니다.
Spring Data JPA의 관례에 따라 조회 메서드는 Optional로 래핑하여 반환하는 것이 안전합니다. 도메인 레이어의 UserRepository.findById도 Optional을 반환하고 있으므로 일관성을 위해서도 변경이 필요합니다.
다음과 같이 수정하세요:
- UserModel findByUserId(String userId);
+ Optional<UserModel> findByUserId(String userId);이 변경에 따라 UserRepositoryImpl의 구현도 함께 업데이트해야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UserModel findByUserId(String userId); | |
| Optional<UserModel> findByUserId(String userId); |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java
around line 7, change the repository signature so the finder returns
Optional<UserModel> instead of UserModel to avoid nulls; update the method
declaration to Optional<UserModel> findByUserId(String userId) and then update
UserRepositoryImpl to handle Optional (e.g., use orElseThrow/map or propagate
Optional) so callers and the domain layer remain null-safe and consistent with
UserRepository.findById.
| @GetMapping("/{userId}/points") | ||
| @Override | ||
| public ApiResponse<UserV1Dto.PointResponse> getUserPoint( | ||
| @PathVariable String userId, | ||
| @RequestHeader(value = "X-USER-ID", required = false) String headerUserId | ||
| ) { | ||
| if (headerUserId == null || headerUserId.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "X-USER-ID 헤더가 필요합니다."); | ||
| } | ||
|
|
||
| Integer point = userFacade.getUserPoint(userId); | ||
| if (point == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다."); | ||
| } | ||
|
|
||
| UserV1Dto.PointResponse response = UserV1Dto.PointResponse.from(point); | ||
| return ApiResponse.success(response); | ||
| } |
There was a problem hiding this comment.
X-USER-ID 헤더가 실제로 사용되지 않습니다.
X-USER-ID 헤더의 존재 여부만 확인하고 있지만, 실제로 headerUserId와 userId 경로 파라미터를 비교하는 인증/인가 로직이 없습니다. 이는 사용자가 다른 사용자의 포인트를 조회할 수 있는 보안 취약점입니다.
헤더 값을 경로 파라미터와 비교하는 검증을 추가하세요:
public ApiResponse<UserV1Dto.PointResponse> getUserPoint(
@PathVariable String userId,
@RequestHeader(value = "X-USER-ID", required = false) String headerUserId
) {
if (headerUserId == null || headerUserId.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "X-USER-ID 헤더가 필요합니다.");
}
+ if (!headerUserId.equals(userId)) {
+ throw new CoreException(ErrorType.FORBIDDEN, "본인의 포인트만 조회할 수 있습니다.");
+ }
Integer point = userFacade.getUserPoint(userId);🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java
around lines 47 to 64, the controller currently only checks that X-USER-ID
header exists but does not verify it matches the {userId} path parameter; add an
authorization check that compares headerUserId (trimmed) to userId and if they
differ throw a CoreException with an appropriate error type (e.g.,
ErrorType.FORBIDDEN or ErrorType.UNAUTHORIZED) and message, so only the user
matching the header can access their points; keep the existing null/blank header
check and use equals() for comparison (consider normalizing casing/trimming)
before calling userFacade.getUserPoint.
| void returnsUserInfo_whenValidIdIsProvided() { | ||
| // arrange | ||
| String id = "user123"; | ||
| String email = "test@example.com"; | ||
| String birthDate = "1990-01-01"; | ||
| String gender = "M"; | ||
|
|
||
| UserModel savedUser = userJpaRepository.save( | ||
| new UserModel(id, email, birthDate, gender) | ||
| ); | ||
|
|
||
| // act | ||
| UserModel result = userService.getUserById(id); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(result).isNotNull(), | ||
| () -> assertThat(result.getUserId()).isEqualTo(id), | ||
| () -> assertThat(result.getEmail()).isEqualTo(email), | ||
| () -> assertThat(result.getBirthDate().toString()).isEqualTo(birthDate), | ||
| () -> assertThat(result.getGender()).isEqualTo(gender) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 사용자 ID를 주면, null을 반환한다.") | ||
| @Test | ||
| void returnsNull_whenInvalidIdIsProvided() { | ||
| // arrange | ||
| String invalidId = "nonexistent"; | ||
|
|
||
| // act | ||
| UserModel result = userService.getUserById(invalidId); | ||
|
|
||
| // assert | ||
| assertThat(result).isNull(); | ||
| } |
There was a problem hiding this comment.
null 반환 대신 Optional 또는 예외 처리를 고려하세요.
존재하지 않는 사용자를 조회할 때 null을 반환하는 것은 다음과 같은 문제가 있습니다:
chargePoint메서드는 NOT_FOUND 예외를 던지는데,getUserById는 null을 반환하여 일관성이 없습니다 (line 184-188 참조)- 호출자가 null 체크를 빠뜨리면 NullPointerException 발생 위험이 있습니다
- Java의 모던 관례는
Optional<UserModel>반환 또는 예외 발생입니다
다음 중 하나로 변경하는 것을 권장합니다:
옵션 1: Optional 사용
Optional<UserModel> result = userService.getUserById(id);
assertThat(result).isPresent();
assertThat(result.get().getUserId()).isEqualTo(id);옵션 2: 예외 발생 (chargePoint와 일관성 유지)
assertThrows(CoreException.class, () -> {
userService.getUserById(invalidId);
});🤖 Prompt for AI Agents
In
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java
around lines 95-130, the tests currently expect getUserById to return null for
missing users which is inconsistent and unsafe; update the code and tests to use
one consistent behavior: either (A) change the service signature to return
Optional<UserModel> and update the happy-path test to assert that the Optional
is present and validate result.get() fields, and update the invalid-id test to
assertThat(optional).isEmpty(); or (B) keep a non-Optional signature but change
the service to throw a CoreException (to match chargePoint) and update the
invalid-id test to use assertThrows(CoreException.class, () ->
userService.getUserById(invalidId)); ensure tests import and use Optional or
assertThrows accordingly.
| void returnsUserPoint_whenValidIdIsProvided() { | ||
| // arrange | ||
| String id = "user123"; | ||
| String email = "test@example.com"; | ||
| String birthDate = "1990-01-01"; | ||
| String gender = "M"; | ||
|
|
||
| UserModel savedUser = userJpaRepository.save( | ||
| new UserModel(id, email, birthDate, gender) | ||
| ); | ||
|
|
||
| // act | ||
| Integer result = userService.getUserPoint(id); | ||
|
|
||
| // assert | ||
| assertThat(result).isEqualTo(0); | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 사용자 ID를 주면, null을 반환한다.") | ||
| @Test | ||
| void returnsNull_whenInvalidIdIsProvided() { | ||
| // arrange | ||
| String invalidId = "nonexistent"; | ||
|
|
||
| // act | ||
| Integer result = userService.getUserPoint(invalidId); | ||
|
|
||
| // assert | ||
| assertThat(result).isNull(); | ||
| } |
There was a problem hiding this comment.
null 반환 설계가 일관성이 없습니다.
getUserPoint 메서드도 존재하지 않는 사용자에 대해 null을 반환하고 있습니다. 이는 앞서 언급한 getUserById의 문제와 동일하며, chargePoint가 NOT_FOUND 예외를 던지는 것과 일관성이 없습니다.
Optional<Integer> 반환 또는 예외 발생으로 변경하여 API의 일관성을 개선하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java
around lines 139 to 168: the test shows getUserPoint returns null for a
nonexistent user which is inconsistent with chargePoint throwing a NOT_FOUND
error; update the service to be consistent by either (A) changing getUserPoint
to return Optional<Integer> and update callers/tests to expect Optional.empty()
for missing users, or (B) have getUserPoint throw the same
NOT_FOUND/EntityNotFoundException for missing users and update the test to
expect that exception; ensure the method signature, implementation, and all
affected tests are changed accordingly so behavior is consistent across the API.
http/commerce-api/user-v1.http
Outdated
| GET http://localhost:8080/api/v1/users/user123/points | ||
| X-USER-ID: user123 |
There was a problem hiding this comment.
🧩 Analysis chain
X-USER-ID 헤더의 필요성을 검토하세요.
포인트 조회 API에서 URL 경로에 이미 userId가 포함되어 있는데, 별도로 X-USER-ID 헤더를 전송하고 있습니다. 이는 다음과 같은 문제를 야기할 수 있습니다:
- 헤더와 경로 파라미터의 값이 다를 경우 불일치 발생 가능
- 중복된 정보 전달로 인한 혼란
- 서버에서 두 값 중 어느 것을 우선시할지 불명확
만약 X-USER-ID 헤더가 인증/권한 검증용이라면 명확한 네이밍(예: X-AUTH-USER-ID)을 사용하고, 서버에서 경로의 userId와 헤더 값이 일치하는지 검증하는 로직이 필요합니다.
다음 스크립트로 서버 구현에서 헤더 검증 로직이 있는지 확인하세요:
Also applies to: 28-29
🏁 Script executed:
#!/bin/bash
# Description: Check if X-USER-ID header is validated against path parameter
# Search for X-USER-ID header usage in controller/service code
rg -n "X-USER-ID" --type java -C 5
# Search for point query endpoint implementation
ast-grep --pattern $'@GetMapping($$$)
$$$points$$$
$$$'Length of output: 5988
X-USER-ID 헤더와 경로 파라미터의 일치성 검증이 필요합니다.
서버 코드 검증 결과, X-USER-ID 헤더는 null/blank 여부만 검증되고 있습니다. 경로의 userId와 헤더의 headerUserId 값이 일치하는지 검증하는 로직이 누락되어 있어, 다음 문제를 야기할 수 있습니다:
- 요청에서 헤더와 경로 값이 다를 경우 불일치 발생 (예:
header="user123",path="user456") - 인증된 사용자와 실제 요청 리소스 소유자가 다를 수 있는 보안 취약점
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java 줄 53-55에서 다음과 같이 검증 로직을 추가하세요:
if (!userId.equals(headerUserId)) {
throw new CoreException(ErrorType.FORBIDDEN, "요청 사용자와 경로의 사용자 ID가 일치하지 않습니다.");
}Also applies to: 28-29
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java
around lines 28-29 and 53-55, add a validation that compares the path userId to
the X-USER-ID header value and reject mismatches; specifically, after verifying
headerUserId is non-null/blank, check if userId.equals(headerUserId) and if not
throw a CoreException with ErrorType.FORBIDDEN and message "요청 사용자와 경로의 사용자 ID가
일치하지 않습니다." so requests where the header and path disagree are forbidden.
…figuration options
* Feature/refactor user (#7) * refactor: 사용자 및 포인트에 대한 단위 테스트 수정 * refactor: point도메인을 user 도메인으로 통합 * refactor: user 도메인 관련 통합테스트 로직 수정 * refactor: user 도메인 관련 서비스 로직 구현 * refactor: user 도메인 관련 E2E 테스트 로직 수정 * refactor: user 도메인 관련 API 구현 * Feature/brand product (#8) * test: product 도메인 단위 테스트 추가 * feat: product, brand 도메인 구현 * Feature/like (#9) * feat: like 도메인 구현 * test: like에 대한 단위 테스트 코드 추가 * feat: like에 대한 서비스 로직 추가 * Feature/order (#10) * Feature/like (#9) * feat: like 도메인 구현 * test: like에 대한 단위 테스트 코드 추가 * feat: like에 대한 서비스 로직 추가 * test: order 도메인에 대한 단위 테스트 추가 * feat: order 도메인 구현 * test: 주문에 대한 통합 테스트 코드 추가 * feat: 주문에 대한 서비스 로직 구현 * Feature/product detail (#11) * test: 상품 정보에 대한 도메인 서비스 테스트 코드 추가 * feat: 상품 정보 도메인 서비스 구현 * feat: 상품 정보 조회 서비스 로직 구현 * feat: 테이블명에 `이 포함되는 경우에도 DatabaseCleanUp로직에서 오류 발생하지 않도록 수정 (#12) * refactor: 설계 문서 업데이트 및 구현 내용 반영 (#13)
hotfix: JPQL 내 테이블명 -> 엔티티로 변경
3 participants
📌 Summary
💬 Review Points
✅ Checklist
📎 References
Summary by CodeRabbit
릴리스 노트
새 기능
테스트