[volume-1] TDD 기반 1주차 과제 (회원, 포인트) 기능 구현 #41
Conversation
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
Walkthrough사용자 및 포인트 도메인을 위한 계층화된 아키텍처를 구현합니다. 엔티티 모델(UserModel, PointModel), 리포지토리 인터페이스 및 구현체, 비즈니스 로직 서비스 및 파사드, REST API 컨트롤러, DTO, API 명세를 추가하고 엔드-투-엔드 테스트를 포함합니다. Changes
Sequence DiagramssequenceDiagram
participant Client
participant Controller
participant Facade
participant Service
participant Repository
participant Database
rect rgb(200, 220, 255)
Note over Client,Database: 사용자 회원가입 흐름
Client->>Controller: POST /api/v1/users/signup
Controller->>Facade: signup(userId, email, birthDate)
Facade->>Service: signUp(UserModel)
Service->>Repository: find(userId)
Repository->>Database: Query by userId
Database-->>Repository: null (새로운 사용자)
Service->>Repository: save(UserModel)
Repository->>Database: Insert User
Database-->>Repository: UserModel
Service-->>Facade: UserModel
Facade-->>Controller: UserInfo
Controller-->>Client: 201 Created + UserResponse
end
rect rgb(220, 255, 220)
Note over Client,Database: 포인트 충전 흐름
Client->>Controller: POST /api/v1/points/charge (X-USER-ID)
Controller->>Facade: chargePoint(userId, amount)
Facade->>Service: getUser(userId)
Service->>Repository: find(userId)
Repository->>Database: Query by userId
Database-->>Repository: UserModel
Service-->>Facade: UserModel
Facade->>Service: charge(PointModel with amount)
Service->>Repository: findPoint(user)
Repository->>Database: Query PointModel by user
Database-->>Repository: PointModel
Service->>Service: point.charge(amount)
Service->>Repository: save(PointModel)
Repository->>Database: Update point value
Database-->>Repository: PointModel
Facade->>Service: findPoint(updated)
Facade-->>Controller: PointInfo
Controller-->>Client: 200 OK + PointResponse
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 주의가 필요한 영역:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (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: 8
🧹 Nitpick comments (6)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (1)
59-65: 검증 에러 처리가 올바르게 구현되었습니다.
@Valid또는@Validated로 발생하는 검증 실패를 적절히 처리하고 있습니다.선택적으로, 사용자 경험 개선을 위해 필드명과 에러 메시지를 함께 표시하는 것을 고려해볼 수 있습니다.
선택적 개선안: 필드명을 포함한 에러 메시지
다음과 같이 필드명을 포함하면 더 명확한 에러 메시지를 제공할 수 있습니다:
@ExceptionHandler public ResponseEntity<ApiResponse<?>> handleBadRequest(MethodArgumentNotValidException e) { String errorMessage = e.getBindingResult().getFieldErrors().stream() - .map(FieldError::getDefaultMessage) + .map(error -> String.format("%s: %s", error.getField(), error.getDefaultMessage())) .collect(Collectors.joining(", ")); return failureResponse(ErrorType.BAD_REQUEST, errorMessage); }apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java (1)
16-18: ManyToOne 관계에 fetch 전략 지정을 권장합니다.Fetch 전략이 명시되지 않으면 기본값인
EAGER가 적용되어 N+1 쿼리 문제가 발생할 수 있습니다. PointModel 조회 시 UserModel이 항상 필요한지 검토 후 적절한 전략을 설정하세요.-@ManyToOne +@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_model_id") private UserModel user;apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
18-22: 체크리스트 주석을 제거하세요.완료된 테스트 항목의 체크리스트는 코드에서 제거하는 것이 좋습니다. PR 설명이나 이슈 트래커에서 관리하는 것을 권장합니다.
- /* - - [ ] ID 가 `영문 및 숫자 10자 이내` 형식에 맞지 않으면, User 객체 생성에 실패한다. - - [ ] 이메일이 `xx@yy.zz` 형식에 맞지 않으면, User 객체 생성에 실패한다. - - [ ] 생년월일이 `yyyy-MM-dd` 형식에 맞지 않으면, User 객체 생성에 실패한다. - */ -apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (2)
48-63: 테스트 데이터 설정이 불명확합니다.Line 53에서
pointService.charge()를 사용하여 테스트 데이터를 설정하고 있습니다. 이는 테스트 대상 메서드와 설정 로직이 섞여 테스트 의도를 불명확하게 만듭니다. 통합 테스트에서는 repository를 직접 사용하여 설정하는 것이 더 명확합니다.UserModel user = new UserModel("userId", "email@email.com", "1999-01-01"); userRepository.save(user); PointModel pointModel = new PointModel(user, 10); -pointService.charge(pointModel); +pointRepository.save(pointModel);
67-79: 테스트 시나리오가 실제 사용 패턴과 다릅니다.User를 저장하지 않은 상태에서 PointModel을 조회하는 테스트입니다. 실제로는 존재하지 않는 userId로 조회하는 시나리오를 테스트해야 하므로, 다른 userId를 가진 User를 사용하거나 문자열 userId로 직접 조회하는 방식이 더 명확합니다.
apps/commerce-api/src/main/java/com/loopers/application/point/PointInfo.java (1)
6-12: 애플리케이션 계층에서 도메인 엔티티 노출을 피해주세요.
지금PointInfo가UserModel전체를 그대로 들고 있어서 상위 계층(PointFacade→PointV1Dto)이 도메인 엔티티에 직접 접근하게 됩니다. 응답에 실제로 쓰는 값은 userId/point 정도이므로, 애플리케이션 DTO를UserInfo혹은 단순 userId와 같은 경량 타입으로 변환해 두면 계층 분리가 명확해지고 직렬화·테스트 부담도 줄어듭니다. 예시는 아래와 같이 바꿔 볼 수 있습니다(호출부도 함께 조정 필요).-public record PointInfo(Long id, UserModel user, int point) { - public static PointInfo from(PointModel model) { - return new PointInfo(model.getId(), model.getUser(), model.getPoint()); +public record PointInfo(Long id, String userId, int point) { + public static PointInfo from(PointModel model) { + return new PointInfo(model.getId(), model.getUser().getUserId(), model.getPoint()); } }
📜 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/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.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/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (3)
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/UserV1ApiE2ETest.java (3)
DisplayName(62-113)DisplayName(115-160)SpringBootTest(26-161)
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/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/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/domain/point/PointServiceIntegrationTest.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/user/UserServiceIntegrationTest.java (3)
DisplayName(52-93)DisplayName(95-129)SpringBootTest(25-130)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/main/java/com/loopers/application/user/UserFacade.java (2)
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/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/test/java/com/loopers/domain/point/PointModelTest.java (2)
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/interfaces/api/PointV1ApiE2ETest.java (2)
DisplayName(62-109)DisplayName(111-162)
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/interfaces/api/PointV1ApiE2ETest.java (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.java (1)
PointV1Dto(6-20)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Controller.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/interfaces/api/point/PointV1Dto.java (1)
PointV1Dto(6-20)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (1)
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/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/application/point/PointFacade.java (6)
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/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/test/java/com/loopers/domain/user/UserModelTest.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 (2)
DisplayName(52-93)DisplayName(95-129)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.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/application/user/UserFacade.java (1)
RequiredArgsConstructor(10-28)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)
🔇 Additional comments (12)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (2)
11-13: LGTM!새로운 예외 핸들러를 지원하기 위해 필요한 임포트가 정확히 추가되었습니다.
52-57: LGTM!필수 헤더 누락에 대한 예외 처리가 기존 파라미터 누락 핸들러와 일관된 패턴으로 잘 구현되었습니다. 명확한 에러 메시지를 제공합니다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
1-123: 단위 테스트 범위가 적절합니다.UserModel의 생성자 검증 로직을 철저하게 테스트하고 있으며, TDD 관점에서 적절한 범위입니다. 각 검증 규칙에 대한 테스트 케이스가 잘 구성되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java (1)
1-10: 인프라 계층 구현이 적절합니다.Spring Data JPA의 표준 패턴을 올바르게 따르고 있으며, 커스텀 쿼리 메서드 명명 규칙도 적절합니다.
apps/commerce-api/src/test/java/com/loopers/domain/point/PointServiceIntegrationTest.java (1)
21-41: 통합 테스트의 계층 분리가 적절합니다.PointService의 통합 테스트로서 적절한 범위를 테스트하고 있습니다. Repository와 Service 계층의 상호작용을 검증하며, SpyBean을 활용한 검증 방식도 합리적입니다. 다만 일부 테스트 데이터 설정 방식은 개선 여지가 있습니다.
apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.java (1)
1-10: 도메인 계층 Repository 인터페이스가 적절합니다.도메인 계층에서 인프라 계층에 대한 의존성을 역전시키는 표준 패턴을 올바르게 적용했습니다. 메서드 시그니처도 명확하고 일관성이 있습니다.
apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java (1)
1-13: 애플리케이션 계층 DTO 구현이 우수합니다.Record를 활용한 간결한 DTO 구현이며, 도메인 모델에서 애플리케이션 계층으로의 매핑을 명확하게 분리했습니다. 계층 간 책임 분리가 잘 이루어져 있습니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointJpaRepository.java (1)
1-11: 인프라 계층 구현이 적절합니다.Spring Data JPA를 활용한 표준적인 repository 구현입니다.
findByUser메서드는 연관관계를 통한 조회를 명확하게 표현하고 있습니다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1ApiSpec.java (1)
1-28: API 명세가 잘 정의되어 있습니다.OpenAPI 어노테이션이 적절하게 사용되었고, API 계약이 명확하게 정의되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java (1)
7-15: DTO 매핑 로직이 적절합니다.정적 팩토리 메서드를 사용한 변환 패턴이 깔끔하게 구현되어 있습니다.
apps/commerce-api/src/main/java/com/loopers/infrastructure/point/PointRepositoryImpl.java (1)
11-25: 리포지토리 구현이 적절합니다.도메인 계층과 인프라 계층을 명확히 분리하는 리포지토리 패턴이 잘 적용되어 있습니다. JPA 리포지토리로의 위임도 깔끔합니다.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1ApiSpec.java (1)
1-31: API 명세가 명확하게 정의되어 있습니다.포인트 API의 계약이 OpenAPI 어노테이션과 함께 잘 문서화되어 있습니다. 헤더 기반 사용자 식별 방식도 일관되게 적용되었습니다.
Comment on lines
+18
to
+31
| 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); | ||
|
|
||
| if (point == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "포인트 정보가 없습니다."); | ||
| } | ||
|
|
||
| return PointInfo.from(point); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
포인트 조회 로직의 비즈니스 요구사항을 명확히 해주세요.
현재 구현에서는 포인트 정보가 없으면 예외를 던지는데, 신규 유저의 경우 포인트 레코드가 아직 생성되지 않았을 수 있습니다. 이 경우 0 포인트를 반환하는 것이 더 적절할 수 있습니다.
또한 Line 23에서 new PointModel(user, 0)로 생성하는 부분은 설계상 어색합니다. PointService.findPoint가 실제로는 user만 필요로 하는데 PointModel을 파라미터로 받고 있습니다.
다음을 확인해주세요:
- 포인트 레코드가 없는 신규 유저의 경우 어떻게 처리해야 하는지 비즈니스 요구사항 확인
PointService.findPoint(UserModel user)시그니처로 리팩토링 검토
---
🏁 Script executed:
```shell
#!/bin/bash
# Find PointService class and findPoint method
find . -type f -name "*.java" | xargs grep -l "class PointService"
Length of output: 168
🏁 Script executed:
#!/bin/bash
# Search for findPoint method definition
rg -n "findPoint\s*\(" --type=java -A 3 -B 3Length of output: 7228
🏁 Script executed:
#!/bin/bash
# Find PointModel class definition
find . -type f -name "*.java" | xargs grep -l "class PointModel"Length of output: 153
🏁 Script executed:
#!/bin/bash
# Find all usages of findPoint
rg -n "\.findPoint\s*\(" --type=java -B 2 -A 2Length of output: 3467
🏁 Script executed:
#!/bin/bash
# Check PointModel constructor
rg -n "PointModel\s*\(" --type=java -A 2Length of output: 3712
PointService.findPoint 메서드의 API 설계를 개선하세요.
코드 검증 결과, PointService.findPoint가 PointModel을 받지만 실제로는 user만 사용합니다(line 21: point.getUser()). 반면 호출되는 PointRepository.findPoint는 UserModel을 직접 받습니다(line 35에서 이미 이렇게 사용됨).
필수 개선사항:
- PointService.findPoint(PointModel point) → findPoint(UserModel user)로 리팩토링 필요
- PointFacade 24번, 41번 줄에서 불필요한 new PointModel(user, 0) 제거
- PointRepository와 일관된 인터페이스로 정렬
비즈니스 로직 검토:
현재 getPoint에서 포인트 레코드가 없으면 예외 발생(line 26-27). charge 메서드는 신규 유저 포인트를 자동 생성하므로, getPoint도 0으로 반환하거나 일관된 동작으로 정의하세요.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java
around lines 18–31, refactor to call pointService.findPoint with a UserModel
(not a PointModel) and remove the unnecessary new PointModel(user, 0) at lines
24 and 41; update PointService signature from findPoint(PointModel) to
findPoint(UserModel) and align PointRepository to accept UserModel as well, then
change getPoint so that when the service returns null it returns a PointInfo
representing zero balance for the user (e.g., create/return a PointInfo for user
with 0) instead of throwing NOT_FOUND, ensuring the service/repository
interfaces remain consistent.
Comment on lines
+33
to
+43
| 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); | ||
|
|
||
| PointModel charged = pointService.findPoint(new PointModel(user, 0)); | ||
| return PointInfo.from(charged); | ||
| } |
There was a problem hiding this comment.
Null 안전성 문제와 중복 코드를 개선해주세요.
Line 42에서 charged가 null인지 확인하지 않고 PointInfo.from(charged)를 호출하면 NPE가 발생할 수 있습니다. pointService.charge는 void를 반환하고 내부에서 저장을 처리하지만, 이후 조회 시 이론적으로 null이 반환될 수 있습니다.
또한 Lines 34-37의 사용자 검증 로직은 getPoint 메서드(Lines 19-22)와 중복됩니다.
다음과 같이 개선을 권장합니다:
+ private UserModel validateAndGetUser(String userId) {
+ UserModel user = userService.getUser(userId);
+ if (user == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다.");
+ }
+ return user;
+ }
+
public PointInfo chargePoint(String userId, int amount) {
- UserModel user = userService.getUser(userId);
- if (user == null) {
- throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다.");
- }
+ UserModel user = validateAndGetUser(userId);
PointModel pointModel = new PointModel(user, amount);
pointService.charge(pointModel);
PointModel charged = pointService.findPoint(new PointModel(user, 0));
+ if (charged == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "포인트 충전 후 조회에 실패했습니다.");
+ }
return PointInfo.from(charged);
}🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java
around lines 33 to 43, avoid duplicating user validation and prevent a possible
NPE: reuse the existing getPoint/getUser validation method instead of repeating
userService.getUser(...) so the same not-found handling is used, and after
charging, check that the returned charged PointModel from
pointService.findPoint(...) is not null; if it is null throw an appropriate
CoreException (e.g., ErrorType.NOT_FOUND with a clear message) before calling
PointInfo.from(charged).
Comment on lines
+47
to
+56
| 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.
잔액 부족 검증 로직이 잘못되었습니다.
Line 52의 point < 0 검증은 차감 후 잔액이 음수가 되는지를 확인하지 못합니다. 현재 로직으로는 차감 전 잔액이 양수이기만 하면 차감 후 음수가 되어도 통과합니다.
다음 diff를 적용하여 잔액 부족 검증을 수정하세요:
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 이상이어야 합니다.");
+ if (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, the insufficient-balance check currently tests point < 0
before deduction which misses cases where amount > point; change the validation
to either verify amount is not greater than current point (e.g., if (amount >
point) throw ...) or compute the resulting balance and ensure it is non-negative
(e.g., if (point - amount < 0) throw ...), keeping the negative-amount guard
as-is and then perform the subtraction only after the correct
insufficient-balance check.
Comment on lines
+9
to
+45
| @Entity | ||
| @Table(name = "user") | ||
| public class UserModel extends BaseEntity { | ||
|
|
||
| private String userId; | ||
| private String email; | ||
| private String birthDate; | ||
|
|
||
| protected UserModel() {} | ||
|
|
||
| public UserModel(String userId, String email, String birthDate) { | ||
|
|
||
| if ( userId == null || userId.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "UserId는 비어있을 수 없습니다."); | ||
| } | ||
| if ( !userId.matches("^[a-zA-Z0-9_-]{1,10}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "ID 가 `영문 및 숫자 10자 이내` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| if ( email == null || email.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일은 비어있을 수 없습니다."); | ||
| } | ||
| if ( !email.matches("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,6}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일이 `xx@yy.zz` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| if ( birthDate == null || birthDate.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 비어있을 수 없습니다."); | ||
| } | ||
| if ( !birthDate.matches("^\\d{4}-\\d{2}-\\d{2}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "생년월일이 `yyyy-MM-dd` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| this.userId = userId; | ||
| this.email = email; | ||
| this.birthDate = birthDate; | ||
| } |
There was a problem hiding this comment.
테이블 이름 충돌: user는 표준 SQL 예약어
Hibernate가 스키마를 생성할 때 user를 그대로 사용하면 H2·MySQL·PostgreSQL 등 주요 DB에서 예약어 충돌로 DDL 실행이 실패합니다. 애플리케이션 부팅/테스트가 바로 깨지므로 안전한 이름(예: users)으로 변경해 주세요.
-@Table(name = "user")
+@Table(name = "users")🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java around
lines 9 to 45, the @Table(name = "user") uses the SQL reserved word "user" which
causes DDL failures on many DBs; change the table name to a safe identifier (for
example "users") by updating the @Table annotation, and then update any JPQL/SQL
queries, repository definitions, tests, and migration scripts that reference
"user" to the new name so schema generation and runtime queries remain
consistent.
Comment on lines
+21
to
+23
| public Optional<UserModel> findById(Long id) { | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
구현 누락: findById가 항상 빈 Optional 반환
UserRepository.findById 계약을 어기고 있어 어느 계층에서 호출해도 무조건 미존재로 처리됩니다. 이 상태로는 ID 기반 조회 로직과 연쇄 검증이 전부 실패하므로 즉시 수정이 필요합니다. 아래처럼 JPA 리포지토리에 위임해 주세요.
@Override
public Optional<UserModel> findById(Long id) {
- return Optional.empty();
+ return userJpaRepository.findById(id);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java
around lines 21-23, the findById method currently always returns
Optional.empty(); replace this with a delegation to the JPA repository (e.g.,
call userJpaRepository.findById(id)) and map the returned User entity to the
UserModel before returning (wrap mapping result in Optional), ensuring null-safe
handling and preserving the contract of UserRepository.findById.
Comment on lines
+16
to
+19
| public record ChargeRequest( | ||
| @Min(value = 1, message = "포인트는 1 이상이어야 합니다.") | ||
| int amount | ||
| ) {} |
There was a problem hiding this comment.
🧩 Analysis chain
API 계층과 도메인 계층의 검증 규칙이 불일치합니다.
ChargeRequest는 최소값을 1로 제한하지만, PointModel 생성자와 charge() 메서드는 0을 허용합니다. 이는 혼란을 야기할 수 있으며, 비즈니스 요구사항에 따라 일관성 있게 정렬되어야 합니다.
다음 스크립트로 도메인 계층의 검증 로직을 확인하세요:
🏁 Script executed:
#!/bin/bash
# PointModel의 포인트 검증 로직 확인
ast-grep --pattern 'if ($_ < 0) {
$$$
}'Length of output: 1378
더 정확한 분석을 위해 PointModel의 메서드 구조를 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# PointModel의 메서드 정의 확인
cat apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java
Length of output: 1414
API와 도메인 계층의 포인트 검증 규칙 불일치 및 use() 메서드 검증 버그
-
API/도메인 검증 불일치:
ChargeRequest는 @min(value = 1)로 최소값 1을 강제하지만,PointModel의 생성자(line 25-27)와charge()메서드(line 41-43)는 0을 허용합니다(< 0만 검사). -
use() 메서드 검증 버그 (line 49-51):
amount < 0검사로 0을 허용하지만, 에러 메시지는 "사용 금액은 0보다 커야 합니다"로 0보다 큰 값을 요구합니다. 검사 조건을amount <= 0으로 수정하거나, 메시지를 수정해야 합니다.
비즈니스 요구사항을 명확히 한 후 검증 규칙을 통일하세요.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/point/PointV1Dto.java
around lines 16–19 and corresponding domain code (PointModel constructor lines
~25–27, charge() ~41–43, use() ~49–51), the API requires amount >=1 but the
domain allows 0 (checks only amount < 0) and use() has a mismatched
check/message; unify the validation to a single business rule (either allow 0 or
require >0), then apply it consistently: if the business rule is "amount must be
>=1", keep @Min(1) on ChargeRequest and change PointModel constructor, charge()
and use() to reject amount <= 0 (use <= 0 checks) and update error messages
accordingly; if the rule allows 0, change the @Min and API message to @Min(0)
(or remove the annotation) and make domain checks allow 0; finally fix use() so
the condition matches its message (use <= 0 when message says must be >0, or
adjust message).
Comment on lines
+24
to
+26
| 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 필드가 API에서 폐기되고 있습니다.
UserV1Dto.SignupRequest는 gender를 필수 값으로 받고 있지만, Line 24에서 userFacade.signup으로 전달할 때 userId, email, birthDate만 넘기고 있어서 gender 정보가 영구적으로 소실됩니다. 이는 API 계약 상 “필수 입력”을 받고도 저장/활용하지 못해 클라이언트가 기대하는 동작과 실제 동작이 어긋나는 주요 문제입니다. UserModel 및 관련 도메인 계층에서 gender를 보존하도록 확장하거나, 불필요하다면 요청 DTO에서 제거하여 계약을 일치시켜 주세요.
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java
around lines 24-26, the SignupRequest includes a required gender field but the
call to userFacade.signup(...) only passes userId, email, and birthDate, causing
gender to be dropped; either update the controller to pass request.gender() into
userFacade.signup and then update the UserFacade, UserModel/domain constructors
and persistence layer to accept and persist gender, or if gender is truly
deprecated remove it from SignupRequest and all related DTOs to keep the API
contract consistent—implement whichever path fits the domain, update method
signatures and tests accordingly, and ensure mapping/conversion and database
schema accommodate the change.
Comment on lines
+17
to
+26
| 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.
🧩 Analysis chain
SignupRequest의 gender 필드가 사용되지 않습니다.
SignupRequest에 gender 필드가 정의되어 있고 @NotBlank 검증도 적용되어 있지만, 실제로 이 필드는 어디에서도 사용되지 않습니다.
UserFacade.signup 메서드를 보면 userId, email, birthDate만 파라미터로 받고 있으며, UserResponse와 UserModel에도 gender 필드가 없습니다.
다음 중 하나로 수정이 필요합니다:
- gender 필드가 필요하다면:
UserModel,UserInfo,UserResponse,UserFacade.signup시그니처를 모두 업데이트 - gender 필드가 불필요하다면:
SignupRequest에서 제거
비즈니스 요구사항을 확인하여 올바른 방향으로 수정해주세요.
🏁 Script executed:
#!/bin/bash
# Search for UserFacade.signup method signature and implementation
find . -type f -name "*.java" | xargs grep -l "UserFacade" | head -5Length of output: 166
🏁 Script executed:
#!/bin/bash
# Find and display UserFacade class with signup method
rg -n "class UserFacade" -A 50 --type java | head -100Length of output: 1942
🏁 Script executed:
#!/bin/bash
# Search for signup method implementation
rg -n "def signup|void signup|signup\s*\(" -B 2 -A 10 --type java | head -150Length of output: 5123
🏁 Script executed:
#!/bin/bash
# Search for UserModel definition
rg -n "class UserModel|record UserModel" -A 20 --type javaLength of output: 4683
🏁 Script executed:
#!/bin/bash
# Search for UserResponse definition
rg -n "class UserResponse|record UserResponse" -A 20 --type javaLength of output: 2341
🏁 Script executed:
#!/bin/bash
# Search all references to gender field
rg -n "gender" --type java -B 2 -A 2 | head -100Length of output: 1346
🏁 Script executed:
#!/bin/bash
# Check UserInfo class
rg -n "class UserInfo|record UserInfo" -A 20 --type javaLength of output: 961
SignupRequest에서 gender 필드를 제거하거나 전체 레이어에 추가해야 합니다.
SignupRequest의 gender 필드는 @NotBlank 검증을 받지만, 실제로 어느 계층에서도 사용되지 않습니다. 현재 데이터 흐름을 확인한 결과:
UserV1Controller.signup()라인 24:userFacade.signup(request.userId(), request.email(), request.birthDate())로 호출 - gender 미전달UserFacade.signup()시그니처:(String userId, String email, String birthDate)- gender 파라미터 없음UserModel: gender 필드 없음UserInfo,UserResponse: gender 필드 없음
비즈니스 요구사항에 따라 다음 중 하나로 수정하세요:
- gender 필드가 필요하면:
UserModel,UserInfo,UserResponse,UserFacade.signup()시그니처를 모두 업데이트 - gender 필드가 불필요하면:
SignupRequest에서 제거
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java
around lines 17 to 26, the SignupRequest includes a gender field annotated with
@NotBlank but no other layers consume it; either remove gender from
SignupRequest or propagate it across layers. If gender is not needed, delete the
gender field and its validation from SignupRequest and update any callers if
present; if gender is required, add gender to UserModel, UserInfo, UserResponse
and update UserFacade.signup() signature and all call sites (including
UserV1Controller.signup()) to accept and pass the gender value, ensuring
DTO↔domain mapping is updated accordingly.
hyujikoh
added a commit
that referenced
this pull request
Dec 14, 2025
refactor(common): BaseEntity 제네릭 타입 개선
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
📌 Summary
💬 리뷰 포인트
TestRestTemplate), AI의 도움을 일부 받아 작성했는데, 이 접근 방식이 올바른지 중점적으로 리뷰 부탁드립니다.(아직 구조를 제대로 이해하지 못했습니다.)✅ Checklist
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항