[volume-1] TDD 기반 1주차 과제 (회원, 포인트) 기능 구현 #40
Conversation
개요사용자 및 포인트 관리를 위한 도메인 계층, 서비스 계층, 애플리케이션 계층, 인프라 계층, API 계층의 완전한 아키텍처를 구현합니다. 사용자 회원가입/조회, 포인트 충전/조회 기능을 포함한 REST API 엔드포인트를 추가하고, 포괄적인 유닛/통합/E2E 테스트를 작성합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant PointV1Controller
participant PointFacade
participant PointService
participant UserService
participant PointJpaRepository as PointRepository
participant UserJpaRepository as UserRepository
Client->>PointV1Controller: POST /api/v1/points/charge<br/>(X-USER-ID, amount)
PointV1Controller->>PointFacade: chargePoint(userId, amount)
PointFacade->>UserService: 사용자 존재 확인
UserService->>UserRepository: find(userId)
UserRepository-->>UserService: UserModel | null
alt User Not Found
UserService-->>PointFacade: throw NOT_FOUND
PointFacade-->>PointV1Controller: CoreException
PointV1Controller-->>Client: 404 Not Found
else User Found
PointFacade->>PointService: charge(PointModel)
PointService->>UserRepository: find(user)
UserRepository-->>PointService: UserModel
PointService->>PointRepository: findByUser(user)
PointRepository-->>PointService: PointModel | empty
alt Point Exists
PointService->>PointService: 포인트 누적
else Point Not Exists
PointService->>PointService: 새 PointModel 생성
end
PointService->>PointRepository: save(pointModel)
PointRepository-->>PointService: PointModel
PointService-->>PointFacade: 완료
PointFacade->>PointService: findPoint(user)
PointService-->>PointFacade: PointModel
PointFacade->>PointFacade: PointInfo로 변환
PointFacade-->>PointV1Controller: PointInfo
PointV1Controller->>PointV1Controller: PointV1Dto.PointResponse로 변환
PointV1Controller-->>Client: 200 OK + response
end
예상 코드 리뷰 노력🎯 3 (중간) | ⏱️ ~25 분 특히 주의가 필요한 영역:
관련 가능성 있는 PR
시 🐰
Pre-merge checks and finishing touches✅ Passed checks (2 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: 6
🧹 Nitpick comments (18)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (1)
59-65: 검증 에러 처리가 정상적으로 동작합니다.
MethodArgumentNotValidException핸들러가 올바르게 구현되어 있으며, 필드 검증 실패 시 적절한 에러 메시지를 반환합니다.다만, 다음 사항을 고려해볼 수 있습니다:
- 여러 검증 오류가 발생할 경우 모든 메시지를 ", "로 연결하면 클라이언트가 개별 필드 오류를 구분하기 어려울 수 있습니다
- 향후 프론트엔드 요구사항에 따라 필드별 구조화된 에러 응답을 고려할 수 있습니다
현재 구현은 기본적인 사용 사례에 충분하며, 필요시 추후 개선할 수 있습니다.
참고: null 안전성 향상을 위해 다음과 같이 개선할 수도 있습니다 (선택사항):
@ExceptionHandler public ResponseEntity<ApiResponse<?>> handleBadRequest(MethodArgumentNotValidException e) { String errorMessage = e.getBindingResult().getFieldErrors().stream() - .map(FieldError::getDefaultMessage) + .map(error -> error.getDefaultMessage() != null ? error.getDefaultMessage() : "검증 오류") .collect(Collectors.joining(", ")); return failureResponse(ErrorType.BAD_REQUEST, errorMessage); }apps/commerce-api/src/main/java/com/loopers/application/point/PointInfo.java (1)
6-9: 애플리케이션 DTO에서 엔티티를 직접 노출하지 않는 편이 좋겠습니다.
현재 PointInfo가 UserModel을 그대로 들고 있어 애플리케이션 레이어에서 영속 엔티티가 노출되고 있습니다. 상위 계층(API DTO, 테스트 등)에서 실수로 엔티티를 재사용하면 지연 로딩 문제나 의도치 않은 상태 변경이 발생해 레이어 경계가 흐려집니다. 필요한 값(userId 등)만 복사하거나 UserInfo 같은 얇은 뷰 모델을 별도로 두는 방향을 검토해 주세요.apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1ApiSpec.java (1)
15-18: Swagger 요청 본문 정의가 누락되어 문서가 어색해질 수 있습니다.
인터페이스 파라미터에 @RequestBody(또는 @parameter) 없이 @Schema만 사용하면 springdoc-openapi가 요청 본문으로 인식하지 못해 Swagger UI에 query parameter처럼 표시될 수 있습니다. 문서를 인터페이스에서 관리하려면 다음과 같이 명시해 주세요.-import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.parameters.RequestBody; ... - ApiResponse<UserV1Dto.UserResponse> signup( - @Schema(name = "회원가입 요청", description = "회원가입할 유저의 정보") - UserV1Dto.SignupRequest request - ); + ApiResponse<UserV1Dto.UserResponse> signup( + @RequestBody( + description = "회원가입할 유저의 정보" + ) + UserV1Dto.SignupRequest request + );apps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.java (3)
13-13: 미사용 import를 제거하세요.
java.time.LocalDate가 import되어 있지만 사용되지 않습니다.
16-16: DisplayName이 불완전합니다."포인트 " 다음에 설명이 누락되어 있습니다. 예: "포인트 생성" 또는 "포인트 검증"과 같이 명확한 이름을 지정하세요.
34-35: 미완성 코드를 정리하세요.주석 처리된 "//포인트 사용하기"는 향후 구현 예정인 것으로 보입니다. TODO 주석으로 변경하거나 제거하는 것이 좋습니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
30-31: 파일 끝의 빈 줄을 정리하세요.파일 끝에 불필요한 빈 줄이 있습니다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (3)
7-7: 미사용 import를 제거하세요.
org.apache.catalina.User가 import되어 있지만 사용되지 않습니다.
12-12: 미사용 import를 제거하세요.
org.springdoc.api.ErrorMessage가 import되어 있지만 사용되지 않습니다.
17-19: 미사용 static import를 정리하세요.Mockito의
any,verify,timesimport는 사용되지만, 파일에서 실제로 필요한 것만 남기고 정리하는 것이 좋습니다. Line 67에서만 사용되고 있습니다.apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (2)
26-27: 미사용 필드를 제거하세요.
PointJpaRepository필드가 선언되어 있지만 테스트에서 사용되지 않습니다.
83-97: 포인트 충전 테스트 커버리지를 확장하는 것을 권장합니다.현재는 존재하지 않는 유저에 대한 실패 케이스만 테스트하고 있습니다. 다음 케이스를 추가하면 더 완전한 테스트가 됩니다:
- 새로운 유저(포인트 레코드 없음)에게 충전하는 경우
- 기존 포인트가 있는 유저에게 추가 충전하는 경우 (누적 검증)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/PointV1ApiE2ETest.java (1)
78-80: 반복되는 ParameterizedTypeReference 생성을 리팩토링하세요.동일한
ParameterizedTypeReference<ApiResponse<PointV1Dto.PointResponse>>생성 코드가 Lines 78-80, 99-101, 128-130, 152-154에서 반복됩니다.테스트 클래스의 상수로 추출하면 가독성이 향상됩니다:
+ private static final ParameterizedTypeReference<ApiResponse<PointV1Dto.PointResponse>> POINT_RESPONSE_TYPE = + new ParameterizedTypeReference<>() {}; + @DisplayName("포인트 조회에 성공할 경우, 보유 포인트를 응답으로 반환한다.") @Test void returnsPoint_whenValidUserIdHeaderIsProvided() { // ... - ParameterizedTypeReference<ApiResponse<PointV1Dto.PointResponse>> responseType = new ParameterizedTypeReference<>() {}; ResponseEntity<ApiResponse<PointV1Dto.PointResponse>> response = - testRestTemplate.exchange(ENDPOINT_GET, HttpMethod.GET, new HttpEntity<>(headers), responseType); + testRestTemplate.exchange(ENDPOINT_GET, HttpMethod.GET, new HttpEntity<>(headers), POINT_RESPONSE_TYPE);동일한 패턴을 다른 테스트 메서드에도 적용하세요.
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (2)
18-31: 레이어 간 책임 분리를 개선할 수 있습니다.현재
PointFacade와PointService모두에서 사용자 존재 여부를 검증하고 있어 중복이 발생합니다:
- Facade의 Line 19-22: 사용자 검증
- Service의
findPoint(PointService.java Line 22-25): 동일한 검증 수행또한 Line 23에서 조회를 위해
new PointModel(user, 0)으로 더미 값을 전달하는 것은 API 설계상 어색합니다. PointModel 생성자는 포인트 충전/사용을 위한 금액을 받도록 설계되었는데, 조회 시 0을 전달하는 것은 의미상 맞지 않습니다.다음 중 하나의 방식으로 개선을 고려해보세요:
방안 1 (권장):
PointService.findPoint의 시그니처를 변경하여 userId만 받도록 수정-public PointModel findPoint(PointModel point) { - UserModel requestUser = point.getUser(); - var foundUser = userRepository.find(requestUser.getUserId()); +public PointModel findPoint(String userId) { + var foundUser = userRepository.find(userId); if (foundUser.isEmpty()) { return null; } return pointRepository.findPoint(foundUser.get()).orElse(null); }그리고 Facade에서:
public PointInfo getPoint(String userId) { - UserModel user = userService.getUser(userId); - if (user == null) { - throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다."); - } - PointModel pointModel = new PointModel(user, 0); - PointModel point = pointService.findPoint(pointModel); + PointModel point = pointService.findPoint(userId); if (point == null) { throw new CoreException(ErrorType.NOT_FOUND, "포인트 정보가 없습니다."); } return PointInfo.from(point); }방안 2: UserService의 검증 로직을 신뢰하고 PointService에서는 사용자 검증을 제거하여 단일 책임 원칙을 준수
33-43: 중복된 사용자 검증 로직을 제거하고 개선할 수 있습니다.Lines 34-37의 사용자 검증 로직이
getPoint메서드(Lines 19-22)와 완전히 동일하며,PointService.charge(PointService.java Line 32-33)에서도 동일한 검증을 수행합니다.또한 Line 41에서 충전 후 재조회를 위해 다시
new PointModel(user, 0)을 생성하는데, 이는 불필요한 객체 생성입니다.개선 방안:
public PointInfo chargePoint(String userId, int amount) { - UserModel user = userService.getUser(userId); - if (user == null) { - throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다."); - } - PointModel pointModel = new PointModel(user, amount); - pointService.charge(pointModel); + pointService.charge(userId, amount); - PointModel charged = pointService.findPoint(new PointModel(user, 0)); + PointModel charged = pointService.findPoint(userId); return PointInfo.from(charged); }이렇게 하려면
PointService.charge의 시그니처도 변경이 필요합니다:-public void charge(PointModel point) { - UserModel user = point.getUser(); - var foundUser = userRepository.find(user.getUserId()) +public void charge(String userId, int amount) { + var foundUser = userRepository.find(userId) .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "유저가 존재하지 않습니다.")); var existing = pointRepository.findPoint(foundUser); if (existing.isPresent()) { - existing.get().charge(point.getPoint()); + existing.get().charge(amount); pointRepository.save(existing.get()); return; } - pointRepository.save(new PointModel(foundUser, point.getPoint())); + pointRepository.save(new PointModel(foundUser, amount)); }이렇게 하면 Facade 레이어에서 불필요한 UserModel 조회와 PointModel 생성을 제거할 수 있습니다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (3)
18-22: TODO 주석을 제거하거나 업데이트하세요.Lines 18-22의 체크리스트 항목들은 이미 아래 테스트들로 구현되었으므로 주석을 제거하거나 완료 표시로 업데이트하는 것이 좋습니다.
24-24: 주석의 오타를 수정하세요."입력한 아이디가 빈칸칸이거나" → "입력한 아이디가 빈칸이거나"
14-122: 검증 실패 케이스는 잘 커버되었으나, 정상 케이스 테스트가 누락되었습니다.현재 테스트는 모두 검증 실패 케이스만 다루고 있습니다. 올바른 입력값으로
UserModel객체가 성공적으로 생성되는 정상 케이스 테스트를 추가하는 것을 권장합니다.예시:
@DisplayName("유효한 입력값으로 User 객체 생성에 성공한다.") @Test void createsUserModel_whenAllInputsAreValid() { // arrange & act UserModel userModel = new UserModel("user123", "user123@example.com", "1999-01-01"); // assert assertAll( () -> assertThat(userModel.getUserId()).isEqualTo("user123"), () -> assertThat(userModel.getEmail()).isEqualTo("user123@example.com"), () -> assertThat(userModel.getBirthDate()).isEqualTo("1999-01-01") ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/application/point/PointInfo.java(1 hunks)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/point/PointModel.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/point/PointRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.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/point/PointJpaRepository.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.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/ApiControllerAdvice.java(2 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1ApiSpec.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Controller.java(1 hunks)apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.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/point/PointModelTest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.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/PointV1ApiE2ETest.java(1 hunks)apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-31T14:57:08.821Z
Learnt from: minor7295
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 22
File: apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointsV1Controller.java:24-34
Timestamp: 2025-10-31T14:57:08.821Z
Learning: In the loopers-spring-java-template repository, the X-USER-ID header-based user identification is temporarily used for learning purposes and will be replaced with proper authentication (Spring Security) in a future PR. Do not flag this as a security issue until proper authentication is implemented.
Applied to files:
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
📚 Learning: 2025-10-31T02:20:33.770Z
Learnt from: kilian-develop
Repo: Loopers-dev-lab/loopers-spring-java-template PR: 15
File: core/domain/src/main/java/com/loopers/core/domain/user/vo/UserIdentifier.java:16-27
Timestamp: 2025-10-31T02:20:33.770Z
Learning: In UserIdentifier and similar value objects, when the constructor performs only null-checking while the static create() method performs full validation (regex, length, etc.), this is an intentional pattern for schema evolution. The constructor is used by the persistence layer to reconstruct domain objects from the database (no validation needed for already-validated legacy data), while create() is used by the application layer to create new domain objects (with validation for new data). This allows backward compatibility when validation rules change in production without requiring migration of all existing database records.
Applied to files:
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java
🧬 Code graph analysis (16)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java (1)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java (1)
Entity(12-57)
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (2)
apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (3)
DisplayName(43-81)DisplayName(83-97)SpringBootTest(21-98)apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
DisplayName(14-122)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/PointV1ApiE2ETest.java (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.java (1)
PointV1Dto(6-20)apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (1)
SpringBootTest(26-161)
apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java (3)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
RequiredArgsConstructor(10-32)apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
RequiredArgsConstructor(11-38)
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (2)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (1)
RequiredArgsConstructor(12-43)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (3)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (1)
RequiredArgsConstructor(12-44)apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java (1)
RequiredArgsConstructor(10-28)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
RequiredArgsConstructor(10-32)
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (3)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (1)
RequiredArgsConstructor(12-44)apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (1)
RequiredArgsConstructor(12-43)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
RequiredArgsConstructor(10-32)
apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (2)
apps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.java (1)
DisplayName(16-36)apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (3)
DisplayName(52-93)DisplayName(95-129)SpringBootTest(25-130)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (5)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (1)
RequiredArgsConstructor(12-43)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (1)
RequiredArgsConstructor(11-25)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
RequiredArgsConstructor(10-32)apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Controller.java (1)
RequiredArgsConstructor(10-37)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java (1)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java (1)
Entity(9-59)
apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (4)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (1)
RequiredArgsConstructor(12-44)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (1)
RequiredArgsConstructor(11-25)apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
RequiredArgsConstructor(10-32)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java (1)
UserV1Dto(6-27)apps/commerce-api/src/test/java/com/loopers/interfaces/api/PointV1ApiE2ETest.java (3)
DisplayName(62-109)DisplayName(111-162)SpringBootTest(23-163)
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (3)
apps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.java (1)
DisplayName(16-36)apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (2)
DisplayName(43-81)DisplayName(83-97)apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (2)
DisplayName(52-93)DisplayName(95-129)
apps/commerce-api/src/test/java/com/loopers/domain/point/PointModelTest.java (1)
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
DisplayName(14-122)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (3)
apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java (1)
RequiredArgsConstructor(10-28)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java (1)
UserV1Dto(6-27)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Controller.java (4)
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java (1)
RequiredArgsConstructor(12-44)apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (1)
RequiredArgsConstructor(12-43)apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
RequiredArgsConstructor(11-31)apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.java (1)
PointV1Dto(6-20)
🔇 Additional comments (21)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (2)
11-13: LGTM! Import 문이 적절하게 추가되었습니다.새로운 예외 핸들러에서 사용하는 클래스들이 올바르게 import되었습니다.
52-57: LGTM! 헤더 누락 예외 처리가 적절합니다.
MissingRequestHeaderException핸들러가 기존 핸들러들과 동일한 패턴으로 구현되어 일관성이 있으며, 명확한 에러 메시지를 제공합니다.apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java (1)
5-12: 단순하면서도 명확한 DTO 변환입니다레코드로 DTO를 구성하고, 팩토리 메서드에서 도메인 엔티티를 그대로 매핑한 구조가 계층 간 책임을 잘 분리해 줍니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointJpaRepository.java (1)
9-11: 필요한 조회 메서드를 정확히 노출했습니다사용자 단위 포인트 조회가 도메인 요구사항과 정확히 맞물려 있고, Spring Data JPA 메서드 시그니처도 적절하게 정의되어 있습니다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (1)
56-92: 회원가입 통합 테스트가 잘 작성되었습니다.회원가입 시나리오가 포괄적으로 테스트되고 있습니다:
- 저장 동작 spy 검증
- 중복 ID 검증
테스트 커버리지가 적절합니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.java (2)
16-19: 충전 금액 검증 정책을 확인하세요.
ChargeRequest는@Min(value = 1)로 최소 1 이상을 요구하지만,PointModel의 생성자와charge메서드는 0 이상을 허용합니다.
- API 레벨에서 0원 충전을 막는 것이 의도라면 일관성이 있습니다.
- 그러나 도메인 모델과 API 검증 정책이 다르므로, 이것이 의도된 설계인지 확인이 필요합니다.
비즈니스 요구사항에 따라 0원 충전이 허용되어야 한다면
@Min(value = 0)으로 변경하거나, 도메인 모델에서도 1 이상만 허용하도록 통일하세요.
7-14: DTO 구조가 잘 설계되었습니다.Record와 static factory 메서드를 사용한 깔끔한 DTO 설계입니다.
apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java (1)
23-30: 생성자 검증이 올바르게 구현되었습니다.음수 포인트를 차단하는 검증 로직이 적절합니다.
apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (1)
43-80: 포인트 조회 테스트가 잘 작성되었습니다.존재하는 유저와 존재하지 않는 유저에 대한 시나리오를 모두 커버하고 있습니다.
apps/commerce-api/src/test/java/com/loopers/interfaces/api/PointV1ApiE2ETest.java (2)
62-109: E2E 테스트 구현이 올바르게 작성되었습니다.PR 설명에서 언급하신 E2E 테스트 구현 방식에 대한 피드백:
잘된 점:
TestRestTemplate과ParameterizedTypeReference를 사용한 접근법이 정확합니다.- 헤더 설정, 요청/응답 매핑이 적절합니다.
- 2xx, 4xx 상태 코드 검증이 올바릅니다.
@SpringBootTest(webEnvironment = RANDOM_PORT)를 사용한 실제 HTTP 요청 테스트가 적합합니다.이것이 Spring Boot에서 권장하는 E2E 테스트 방식입니다.
111-162: 포인트 충전 E2E 테스트가 포괄적입니다.성공 케이스와 실패 케이스(존재하지 않는 유저)를 모두 검증하고 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/point/PointService.java (2)
20-27: 포인트 조회 로직이 올바르게 구현되었습니다.사용자 검증 후 포인트를 조회하는 흐름이 적절합니다. 다만
findPoint가 사용자가 없는 경우와 포인트가 없는 경우 모두 null을 반환하여 구분할 수 없는데, 현재 요구사항에서는 문제가 되지 않습니다. 향후 세밀한 에러 처리가 필요하다면 별도 예외를 던지는 것을 고려할 수 있습니다.
30-42: 충전 로직이 올바르게 구현되었습니다.기존 포인트가 있으면 업데이트하고, 없으면 새로 생성하는 로직이 적절합니다. Line 37의
charge()메서드를 통한 도메인 로직 캡슐화도 좋은 설계입니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
29-37: 사용자 조회 엔드포인트가 올바르게 구현되었습니다.경로 변수로 userId를 받아 처리하는 구조가 RESTful 설계 원칙에 부합합니다.
apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (4)
26-50: E2E 테스트 설정이 올바르게 구현되었습니다.
TestRestTemplate사용 방식과DatabaseCleanUp을 통한 테스트 격리가 적절합니다. PR 설명에서 E2E 테스트 구현이 어려웠다고 하셨는데, 실제 구현은 매우 잘 되어 있습니다.RANDOM_PORT설정과@AfterEach에서의 데이터 정리는 E2E 테스트의 모범 사례입니다.
67-89: 회원가입 성공 테스트가 잘 작성되었습니다.
ParameterizedTypeReference를 사용한 제네릭 타입 처리와assertAll을 통한 다중 검증이 적절합니다.
93-112: 필수 필드 누락 테스트가 의도와 다르게 동작할 수 있습니다.이 테스트는 gender 필드가 없을 때 400 응답을 기대하는데,
UserV1Controller에서 gender가 실제로 사용되지 않는 문제와 연관됩니다. gender 필드의 처리 방향이 결정되면 이 테스트도 함께 검토가 필요합니다.UserV1Controller.java의 gender 필드 처리 방향에 따라 이 테스트를 업데이트하세요:
- gender를 저장하기로 했다면: 테스트 유지
- gender를 제거하기로 했다면: 다른 필수 필드(예: userId, email)로 테스트 변경
120-159: 사용자 조회 관련 E2E 테스트가 성공/실패 케이스를 모두 커버하고 있습니다.존재하는 사용자 조회와 존재하지 않는 사용자 조회 모두 테스트하여 엔드포인트의 동작을 충분히 검증하고 있습니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1ApiSpec.java (1)
9-31: API 명세가 명확하고 잘 문서화되었습니다.Swagger 애노테이션을 통한 API 문서화가 적절하며, X-USER-ID 헤더 파라미터에 대한 설명도 명확합니다. 인터페이스를 통한 API 계약 정의는 좋은 설계 패턴입니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (1)
13-25: 리포지토리 구현이 올바른 레이어 분리 패턴을 따르고 있습니다.도메인 인터페이스(
PointRepository)를 인프라스트럭처 레이어에서 구현하고, JPA 리포지토리에 위임하는 구조가 적절합니다. 이는 도메인 레이어가 인프라에 의존하지 않도록 하는 클린 아키텍처 원칙에 부합합니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Controller.java (1)
17-36: 컨트롤러 구현이 적절한 레이어 책임을 따르고 있습니다.컨트롤러는 HTTP 요청/응답 처리만 담당하고, 비즈니스 로직은
PointFacade에 위임하는 구조가 올바릅니다. X-USER-ID 헤더를 통한 사용자 식별과@Valid를 통한 요청 검증도 적절합니다.
| public void use(int amount) { | ||
|
|
||
| if (amount < 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0보다 커야 합니다."); | ||
| } | ||
| if (point < 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다."); | ||
| } | ||
| this.point -= amount; | ||
| } |
There was a problem hiding this comment.
use 메서드에 잔액 부족 검증이 누락되었습니다.
현재 구현의 문제점:
- Critical: Line 55에서
this.point -= amount를 실행하기 전에point >= amount검증이 없습니다. 잔액이 부족해도 차감되어 음수가 될 수 있습니다. - Minor: Line 49-51의 에러 메시지가 "사용 금액은 0보다 커야 합니다."인데, 실제 검증은
amount < 0으로 0 이상을 허용합니다. 메시지를 "사용 금액은 0 이상이어야 합니다."로 수정하세요. - Optional: Line 52-54의
point < 0검증은 불필요합니다. 생성자와 charge에서 이미 음수를 차단하므로 point는 항상 0 이상입니다.
다음 diff를 적용하세요:
public void use(int amount) {
if (amount < 0) {
- throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0보다 커야 합니다.");
+ throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0 이상이어야 합니다.");
}
- if (point < 0) {
- throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다.");
+ if (this.point < 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 use(int amount) { | |
| if (amount < 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0보다 커야 합니다."); | |
| } | |
| if (point < 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다."); | |
| } | |
| this.point -= amount; | |
| } | |
| public void use(int amount) { | |
| if (amount < 0) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0 이상이어야 합니다."); | |
| } | |
| if (this.point < amount) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트 잔액이 부족합니다."); | |
| } | |
| this.point -= amount; | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java
around lines 47 to 56, add a balance-check before subtracting so the method
validates that amount is non-negative and that point >= amount (throw
CoreException with an appropriate ErrorType and message on failure), change the
error message for the amount check from "사용 금액은 0보다 커야 합니다." to "사용 금액은 0 이상이어야
합니다.", and remove the redundant point < 0 check (since constructor/charge
already prevent negative balances); then perform this.point -= amount.
| @Entity | ||
| @Table(name = "user") |
There was a problem hiding this comment.
예약어 테이블명으로 인한 SQL 오류 가능성
user는 PostgreSQL, MySQL, H2 등에서 예약어라서 Hibernate가 select ... from user 같은 쿼리를 생성할 때 즉시 SQLSyntaxError 가 발생합니다. 테스트나 운영 DB 어느 쪽이든 영향을 받으니 테이블명을 users 같이 예약어가 아닌 이름으로 바꾸거나, 반드시 해당 이름을 써야 한다면 @Table(name = "\"user\"")처럼 ANSI 인용을 강제해야 합니다.
-@Table(name = "user")
+@Table(name = "users")📝 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.
| @Entity | |
| @Table(name = "user") | |
| @Entity | |
| @Table(name = "users") |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java around
lines 9-10, the entity is mapped to the reserved table name "user" which can
cause SQLSyntaxError on PostgreSQL/MySQL/H2; change the mapping to a
non-reserved name (e.g., @Table(name = "users")) or, if you must keep "user",
force ANSI quoting (e.g., @Table(name = "\"user\"")), and then update any
related JPQL/native queries, migrations, and tests to use the new table name.
| public UserModel signUp(UserModel userModel) { | ||
| Optional<UserModel> user = userRepository.find(userModel.getUserId()); | ||
|
|
||
| if (user.isPresent()) { | ||
| throw new CoreException(ErrorType.CONFLICT, "[userId = " + userModel.getUserId() + "] 아이디가 중복되었습니다."); | ||
| } | ||
| return userRepository.save(userModel); | ||
| } |
There was a problem hiding this comment.
동시 가입 시 중복 체크-저장 사이 경쟁 상태가 발생합니다.
현재는 userRepository.find(...)로 중복을 확인한 뒤 save를 호출하지만, 두 요청이 동시에 들어오면 둘 다 Optional이 비어 있다고 판단하고 각각 save를 시도할 수 있습니다. DB에 userId 유니크 제약이 없다면 중복 레코드가 쌓이고, 제약이 있더라도 DataIntegrityViolationException이 발생해 500으로 튈 위험이 큽니다. 저장 구간을 예외 처리로 감싸 CONFLICT로 변환하고, userId에 유니크 인덱스를 두는 식으로 방어 로직을 추가해 주세요.
+import org.springframework.dao.DataIntegrityViolationException;
...
public UserModel signUp(UserModel userModel) {
Optional<UserModel> user = userRepository.find(userModel.getUserId());
if (user.isPresent()) {
throw new CoreException(ErrorType.CONFLICT, "[userId = " + userModel.getUserId() + "] 아이디가 중복되었습니다.");
}
- return userRepository.save(userModel);
+ try {
+ return userRepository.save(userModel);
+ } catch (DataIntegrityViolationException e) {
+ throw new CoreException(ErrorType.CONFLICT, "[userId = " + userModel.getUserId() + "] 아이디가 중복되었습니다.", e);
+ }
}🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
around lines 23 to 30, there is a race between the find() duplicate check and
save() that can produce duplicate users or a 500 on constraint violation; wrap
the save call in a try/catch that catches the persistence exception thrown on
unique-constraint violations (e.g., DataIntegrityViolationException /
PersistenceException) and translate it into a CoreException with
ErrorType.CONFLICT and a descriptive message including the userId, and also
ensure the userId has a unique index at the DB/schema level (add a unique
constraint in the JPA entity or DB migration) so the service is protected even
under concurrent requests.
| @Override | ||
| public Optional<UserModel> findById(Long id) { | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
findById 구현이 플레이스홀더입니다.
findById 메서드가 항상 Optional.empty()를 반환합니다. 이 메서드가 실제로 사용되지 않는다면 문제없지만, 향후 사용 시 버그가 될 수 있습니다. 현재 과제 범위에서 필요하지 않다면 TODO 주석을 추가하거나, 사용할 계획이라면 올바른 구현을 제공하세요.
🏁 Script executed:
#!/bin/bash
# Description: findById 메서드가 실제로 호출되는지 확인
# UserRepository.findById 호출을 검색
rg -nP --type=java '\bfindById\s*\(' -A 2 -B 2Length of output: 1726
Let me search the codebase more specifically for how UserRepository is being used.
Let me search for where UserRepository is injected and used in the codebase:
This will help identify which classes are using UserRepository and what methods they're calling.
🏁 Script executed:
#!/bin/bash
# Search for UserRepository bean injection and usage
rg -nP --type=java '(UserRepository|@Autowired.*User|userRepository\s*\.)' -A 2 -B 2 | head -100Length of output: 10177
🏁 Script executed:
#!/bin/bash
# Search for UserRepository injection points and method calls
rg -nP --type=java 'private.*UserRepository|@Autowired' -A 3 | head -80Length of output: 7245
🏁 Script executed:
#!/bin/bash
# Search for all Java files in the codebase
fd -e java --type f | head -20Length of output: 1593
🏁 Script executed:
#!/bin/bash
# Search for Service or Controller classes that might use UserRepository
rg -nP --type=java '@Service|@Component|@Controller' -l | head -20Length of output: 974
UserRepository.findById는 사용되지 않는 플레이스홀더입니다. 인터페이스에서 제거하거나, 구현하거나, TODO 주석을 추가하세요.
코드베이스를 검색한 결과, UserRepository.findById() 메서드는 어디에도 호출되지 않습니다. UserService에서는 find(userId) 및 save() 메서드만 사용되며, 테스트 파일에서도 findById를 호출하는 곳이 없습니다. 현재 이 메서드는 항상 Optional.empty()를 반환하는 플레이스홀더 상태이므로, 다음 중 하나를 수행해야 합니다:
- 인터페이스와 구현에서 제거
- 필요한 경우 실제 구현 제공
- 향후 사용 계획이 있으면 TODO 주석 추가
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java
around lines 20-23, the findById(Long) method is a placeholder that always
returns Optional.empty() and is not used anywhere; either remove it from the
UserRepository interface and this implementation (also remove unused imports and
update any references), or if you intend to keep it, replace the body with a
proper implementation that queries the underlying data source and returns
Optional.ofNullable(result), or at minimum add a TODO comment explaining why
it’s a stub and when it will be implemented; pick one of these actions and apply
it consistently to both the interface and implementation so the codebase remains
clean and compiles.
| @PostMapping("/signup") | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| @Override | ||
| public ApiResponse<UserV1Dto.UserResponse> signup( | ||
| @Valid @RequestBody UserV1Dto.SignupRequest request | ||
| ) { | ||
| UserInfo info = userFacade.signup(request.userId(), request.email(), request.birthDate()); | ||
| UserV1Dto.UserResponse response = UserV1Dto.UserResponse.from(info); | ||
| return ApiResponse.success(response); | ||
| } |
There was a problem hiding this comment.
필수 입력 필드인 gender가 처리되지 않고 있습니다.
Line 24에서 request.userId(), request.email(), request.birthDate()만 전달하고 있으며, request.gender()는 검증은 되지만 사용되지 않습니다. UserV1Dto.SignupRequest에서 gender를 @NotBlank로 필수 검증하고 있지만(UserV1Dto.java Line 17), 실제로는 저장되지 않아 사용자에게 혼란을 줄 수 있습니다.
다음 중 하나를 선택하세요:
- gender를 실제로 저장한다면, UserFacade와 UserModel에 gender 파라미터 추가
-UserInfo info = userFacade.signup(request.userId(), request.email(), request.birthDate());
+UserInfo info = userFacade.signup(request.userId(), request.email(), request.gender(), request.birthDate());- gender를 저장하지 않는다면, 요청 DTO에서 gender 필드와 검증 제거
public record SignupRequest(
@NotBlank(message = "userId는 필수입니다.")
String userId,
@NotBlank(message = "email은 필수입니다.")
String email,
- @NotBlank(message = "gender는 필수입니다.")
- String gender,
@NotBlank(message = "birthDate는 필수입니다.")
String birthDate
) {}Committable suggestion skipped: line range outside the PR's diff.
| public record SignupRequest( | ||
| @NotBlank(message = "userId는 필수입니다.") | ||
| String userId, | ||
| @NotBlank(message = "email은 필수입니다.") | ||
| String email, | ||
| @NotBlank(message = "gender는 필수입니다.") | ||
| String gender, | ||
| @NotBlank(message = "birthDate는 필수입니다.") | ||
| String birthDate | ||
| ) {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
SignupRequest의 gender 필드가 사용되지 않습니다.
SignupRequest에 gender 필드가 있지만, UserModel 생성자는 userId, email, birthDate만 받습니다.
- 향후 사용을 위해 수집하는 것이라면 주석으로 명시하세요.
- 현재 과제에서 필요없다면 제거하는 것이 좋습니다.
- 실제로 저장해야 한다면
UserModel에 gender 필드를 추가하세요.
🏁 Script executed:
#!/bin/bash
# Find UserModel class definition and its constructor
find . -type f -name "*.java" | head -20
rg -n "class UserModel" --type=java -A 20Length of output: 6317
🏁 Script executed:
#!/bin/bash
# Search for SignupRequest usage and gender field references
rg -n "SignupRequest" --type=java -B 2 -A 5Length of output: 4842
🏁 Script executed:
#!/bin/bash
# Search for mapper or service methods that might process gender
rg -n "gender" --type=java -B 3 -A 3Length of output: 1819
🏁 Script executed:
#!/bin/bash
# Check the UserV1Dto file content
find . -path "*/user/UserV1Dto.java" -exec cat {} \;Length of output: 784
SignupRequest의 gender 필드를 처리하세요.
검증 결과, gender 필드가 클라이언트로부터 수집되고 @notblank로 검증되지만, UserV1Controller에서 userFacade.signup()을 호출할 때 전달되지 않으며, UserModel에도 해당 필드가 없습니다.
다음 중 하나를 선택하세요:
gender필드를UserModel에 추가하고 저장 로직을 구현SignupRequest에서gender필드 제거- 향후 사용 목적이라면 명확한 주석 추가
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java
around lines 17-26, the SignupRequest includes a validated gender field that is
never passed into userFacade.signup() nor stored in UserModel; either add gender
to the domain and persistence layers and propagate it through the controller to
userFacade.signup(), updating UserModel, mapper/DTO conversions, repository save
logic and any database schema if needed, or remove the gender field from
SignupRequest (and its validation) if not used; if you intend to keep it for
future use, add a clear TODO comment explaining why it's not yet persisted and
link to the ticket that will implement storage.
refactor(order): 주문 및 결제 이벤트 핸들러 리팩토링
2 participants
📌 Summary
💬 리뷰 포인트
TestRestTemplate), AI의 도움을 일부 받아 작성했는데, 이 접근 방식이 올바른지 중점적으로 리뷰 부탁드립니다.(아직 구조를 제대로 이해하지 못했습니다.)✅ Checklist
Summary by CodeRabbit
릴리스 노트