-
Notifications
You must be signed in to change notification settings - Fork 14
[volume-1] 회원 가입, 내 정보 조회, 포인트 조회, 포인트 충전 구현 #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: choco3193
Are you sure you want to change the base?
Conversation
워크스루사용자 관리 기능을 위한 완전한 계층형 아키텍처를 도입했습니다. 도메인 엔티티, 저장소, 서비스, 애플리케이션 파사드, REST API 컨트롤러, 요청/응답 DTO를 포함하며, 단위 테스트, 통합 테스트, 엔드-투-엔드 테스트까지 구현했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant Controller as UserV1Controller
participant Facade as UserFacade
participant Service as UserService
participant Repo as UserRepository
participant DB as Database
Note over Client,DB: 사용자 등록 흐름
Client->>Controller: POST /api/v1/users/register
Controller->>Facade: registerUser(userId, email, birth, gender)
Facade->>Service: registerUser(userId, email, birth, gender)
Service->>Repo: existsByUserId(userId)
Repo->>DB: 쿼리
DB-->>Repo: false
Service->>Repo: save(User)
Repo->>DB: INSERT
DB-->>Repo: User
Repo-->>Service: User
Service-->>Facade: User
Facade->>Facade: convert to UserInfo
Facade-->>Controller: UserInfo
Controller->>Controller: map to UserResponse
Controller-->>Client: ApiResponse(UserResponse)
Note over Client,DB: 포인트 충전 흐름
Client->>Controller: POST /api/v1/users/chargePoint
Controller->>Facade: chargePoint(userId, amount)
Facade->>Service: chargePointByUserId(userId, amount)
Service->>Repo: findByUserId(userId)
Repo->>DB: 쿼리
DB-->>Repo: User
Repo-->>Service: User
Service->>Service: user.chargePoint(amount)
Service->>Repo: save(User)
Repo->>DB: UPDATE
DB-->>Repo: User
Repo-->>Service: User
Service-->>Facade: Int (updated point)
Facade-->>Controller: Int
Controller-->>Client: ApiResponse(Int)
코드 리뷰 추정 시간🎯 3 (보통) | ⏱️ ~20분 추가 검토 필요 영역:
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt (2)
29-29: 포인트 저장 설계에 대한 의견 (작성자 질문 관련)현재 User 엔티티에 포인트를 직접 저장하는 방식은 MVP 단계에서는 단순하고 구현이 빠른 장점이 있습니다. 하지만 다음 단계를 고려하면:
현재 설계의 장단점:
- 장점: 구현이 단순하고 조회 성능이 좋음
- 단점: 포인트 변경 이력 추적 불가, 감사(audit) 기능 없음, 트랜잭션 히스토리 부재
향후 고려사항:
서비스가 성장하면 별도의PointAccount또는PointHistory애그리게잇으로 분리하는 것을 권장합니다. 이를 통해 충전/사용 내역 추적, 정산, 감사 기능을 구현할 수 있습니다.현재 단계에서는 이 설계가 적절하지만, 백로그에 리팩토링 항목을 추가하시는 것을 추천합니다.
31-37: 생년월일 검증 보완 권장현재 생년월일 검증이 형식(yyyy-MM-dd)만 확인하고 있어 의미상 유효하지 않은 날짜(예: "9999-99-99", "2025-13-32", 미래 날짜)도 통과합니다.
다음과 같이 의미 검증을 추가하는 것을 권장합니다:
init { if (!validateUserId(userId)) throw CoreException(ErrorType.BAD_REQUEST, "invalid user id") if (!validateEmail(email)) throw CoreException(ErrorType.BAD_REQUEST, "invalid email") - if (!validateBirth(birth)) throw CoreException(ErrorType.BAD_REQUEST, "invalid birth") + if (!validateBirth(birth)) throw CoreException(ErrorType.BAD_REQUEST, "invalid birth") + // 실제 날짜로 파싱 가능한지 검증 + try { + val birthDate = java.time.LocalDate.parse(birth) + if (birthDate.isAfter(java.time.LocalDate.now())) { + throw CoreException(ErrorType.BAD_REQUEST, "birth date cannot be in the future") + } + } catch (e: java.time.format.DateTimeParseException) { + throw CoreException(ErrorType.BAD_REQUEST, "invalid birth date") + } }apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1Dto.kt (1)
20-20: 포인트 충전 금액 검증 개선 권장도메인에서 음수/0 검증을 하고 있지만, API 레벨에서 조기 검증하면 더 나은 사용자 경험을 제공할 수 있습니다.
Jakarta Validation 어노테이션 추가를 고려하세요:
+import jakarta.validation.constraints.Positive + -data class ChargePointRequest(val userId: String, val amount: Int) +data class ChargePointRequest( + val userId: String, + @field:Positive(message = "충전 금액은 양수여야 합니다") + val amount: Int +)apps/commerce-api/src/test/kotlin/com/loopers/domain/user/UserTest.kt (1)
11-79: 테스트 커버리지 보완 권장실패 케이스는 잘 작성되어 있지만, 성공 케이스와 경계값 테스트가 부족합니다.
다음 테스트 추가를 권장합니다:
@DisplayName("유효한 정보로 유저를 생성하면, 성공한다.") @Test fun createsUser_whenAllInputsAreValid() { // arrange & act val user = User( userId = "validId123", email = "[email protected]", birth = "1990-01-01", gender = Gender.MALE ) // assert assertThat(user.userId).isEqualTo("validId123") assertThat(user.point).isEqualTo(0) } @DisplayName("유효한 포인트로 충전하면, 포인트가 증가한다.") @Test fun increasesPoint_whenChargingWithValidAmount() { // arrange val user = User(userId = "testId", email = "[email protected]", birth = "1990-01-01", gender = Gender.MALE) // act user.chargePoint(100) val finalPoint = user.chargePoint(50) // assert assertThat(finalPoint).isEqualTo(150) assertThat(user.point).isEqualTo(150) } @DisplayName("userId 경계값(10자)으로 유저를 생성하면, 성공한다.") @Test fun createsUser_whenUserIdIsMaxLength() { // arrange & act val user = User(userId = "abcdefgh12", email = "[email protected]", birth = "1990-01-01", gender = Gender.MALE) // assert assertThat(user.userId).hasSize(10) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/commerce-api/src/main/kotlin/com/loopers/application/user/UserFacade.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/application/user/UserInfo.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserRepository.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserService.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/user/UserJpaRepository.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/user/UserRepositoryImpl.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1ApiSpec.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1Controller.kt(1 hunks)apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1Dto.kt(1 hunks)apps/commerce-api/src/test/kotlin/com/loopers/domain/user/UserServiceIntegrationTest.kt(1 hunks)apps/commerce-api/src/test/kotlin/com/loopers/domain/user/UserTest.kt(1 hunks)apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/UserV1ApiE2ETest.kt(1 hunks)
🔇 Additional comments (28)
apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt (2)
11-28: 캡슐화가 잘 되어 있습니다.Protected setter를 통해 외부에서의 직접 수정을 방지하고 있어 도메인 불변성을 유지하고 있습니다.
67-72: 성별 enum이 적절합니다.NONE, MALE, FEMALE, OTHER로 충분한 옵션을 제공하고 있습니다.
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1Dto.kt (2)
7-7: DTO 구조가 적절합니다.도메인 레이어에서 검증을 수행하는 구조이므로 DTO에 별도 검증 어노테이션이 없어도 괜찮습니다.
8-19: 매핑 로직이 명확합니다.UserInfo를 UserResponse로 변환하는 로직이 간단하고 명확합니다.
apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/user/UserJpaRepository.kt (1)
6-9: 리포지토리 인터페이스가 적절합니다.Spring Data JPA의 쿼리 메서드 네이밍 규칙을 잘 따르고 있으며, 자동 구현이 올바르게 동작할 것입니다.
apps/commerce-api/src/main/kotlin/com/loopers/application/user/UserInfo.kt (1)
6-22: 사용자 정보와 포인트 분리 설계가 좋습니다.UserInfo에서 point 필드를 제외하고 별도 API로 조회하도록 한 설계는 관심사의 분리 측면에서 적절합니다. 향후 포인트 정보에 대한 접근 제어를 다르게 가져갈 수 있는 유연성도 확보됩니다.
apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserRepository.kt (1)
3-7: 리포지토리 추상화가 깔끔합니다.도메인 레이어에서 인터페이스를 정의하고 인프라 레이어에서 구현하는 구조로 의존성이 잘 정리되어 있습니다.
apps/commerce-api/src/main/kotlin/com/loopers/infrastructure/user/UserRepositoryImpl.kt (1)
7-22: 리포지토리 구현이 적절합니다.인프라 레이어에서 도메인 인터페이스를 구현하면서 JPA 리포지토리로 단순 위임하는 구조가 명확합니다.
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1ApiSpec.kt (3)
10-14: 회원 가입 API 스펙이 적절합니다.요청/응답 타입과 설명이 명확합니다.
16-23: 회원 조회 API 스펙이 적절합니다.필수 파라미터로 설계되어 있고 문서화가 잘 되어 있습니다.
34-38: 포인트 충전 API 스펙이 적절합니다.요청 객체를 통한 충전 로직이 명확하게 정의되어 있습니다.
apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserService.kt (3)
12-22: 회원 가입 로직이 적절합니다.중복 ID 검사와 트랜잭션 처리가 올바르게 구현되어 있습니다.
24-26: 조회 로직이 간결하고 명확합니다.
28-30: 포인트 조회 로직이 적절합니다.apps/commerce-api/src/main/kotlin/com/loopers/application/user/UserFacade.kt (4)
13-16: Facade 패턴이 적절하게 적용되었습니다.도메인 객체를 DTO로 변환하는 책임 분리가 명확합니다.
18-22: Null 처리와 예외 처리가 적절합니다.Elvis 연산자를 활용한 깔끔한 에러 처리입니다.
24-27: 일관된 에러 처리 패턴이 유지되고 있습니다.
29-31: 간결한 위임 구조입니다.다만
UserService.chargePointByUserId의 트랜잭션 이슈가 해결되어야 정상 동작합니다.apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/UserV1ApiE2ETest.kt (3)
41-90: 회원 가입 테스트 케이스가 적절합니다.성공 케이스와 검증 실패 케이스를 모두 커버하고 있습니다.
92-131: 사용자 조회 테스트가 적절합니다.존재/미존재 케이스를 모두 검증하고 있습니다.
133-170: 포인트 조회 테스트가 적절합니다.헤더 검증과 초기 포인트 값(0) 확인이 포함되어 있습니다.
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1Controller.kt (4)
21-28: 회원 가입 엔드포인트가 적절하게 구현되었습니다.입력 검증과 계층 분리가 명확합니다.
30-36: 사용자 조회 엔드포인트가 간결합니다.
38-46: 헤더 검증 로직이 적절합니다.필수 헤더에 대한 명시적인 검증이 구현되어 있습니다.
48-52: 포인트 충전 엔드포인트가 간결합니다.다만
UserService.chargePointByUserId의 트랜잭션 이슈가 해결되어야 정상 동작합니다.apps/commerce-api/src/test/kotlin/com/loopers/domain/user/UserServiceIntegrationTest.kt (3)
28-58: 회원 가입 테스트가 적절합니다.성공 케이스와 중복 ID 예외 케이스를 모두 검증하고 있습니다.
60-94: 사용자 조회 테스트가 적절합니다.존재/미존재 케이스를 모두 커버하고 있습니다.
96-127: 포인트 조회 테스트가 적절합니다.
| fun chargePoint(point: Int): Int { | ||
| if (point <= 0) throw CoreException(ErrorType.BAD_REQUEST, "invalid point") | ||
|
|
||
| this.point += point | ||
| return this.point | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동시성 제어 필요 (Critical) 및 재저장 질문에 대한 답변
1. 동시성 문제 (Race Condition):
현재 구현은 동시에 여러 포인트 충전 요청이 들어올 때 업데이트 손실이 발생할 수 있습니다.
예시:
- 스레드 A: point=100 읽음 → +50 → 150으로 저장
- 스레드 B: point=100 읽음 → +30 → 130으로 저장
- 결과: 130 (스레드 A의 +50이 손실됨)
2. 재저장 관련 질문 답변:
JPA의 dirty checking 메커니즘이 @Transactional 내에서 엔티티 변경을 자동으로 감지하여 저장하므로, 명시적인 save() 호출은 불필요합니다. 다만, 반드시 서비스 레이어에서 @Transactional 안에서 호출되어야 합니다.
해결 방법:
낙관적 잠금을 위한 @Version 필드 추가를 권장합니다:
@Entity
@Table(name = "users")
class User(
userId: String,
email: String,
birth: String,
gender: Gender,
) : BaseEntity() {
+ @Version
+ var version: Long = 0
+ protected set
+
var userId: String = userId
protected set이렇게 하면 동시 수정 시 OptimisticLockException이 발생하여 데이터 무결성을 보호할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt around
lines 39 to 44, the chargePoint method is vulnerable to race conditions when
concurrent requests update the same entity; fix by adding an optimistic lock
version field (annotated @Version) to the User entity class, ensure all
modifications occur inside a service-layer method annotated with @Transactional
(so JPA dirty-checking will flush changes), and handle OptimisticLockException
by retrying the charge operation a bounded number of times or propagating a
clear error; do not add explicit save() calls—let the transaction commit persist
changes.
| companion object { | ||
| private fun validateUserId(userId: String): Boolean { | ||
| val userIdValidator = Regex("^[A-Za-z0-9]{1,10}$") | ||
|
|
||
| return userIdValidator.matches(userId) | ||
| } | ||
|
|
||
| private fun validateEmail(email: String): Boolean { | ||
| val emailValidator = Regex("^[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*@[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*\\.[a-zA-Z]{2,3}$") | ||
|
|
||
| return emailValidator.matches(email) | ||
| } | ||
|
|
||
| private fun validateBirth(birth: String): Boolean { | ||
| val birthValidator = Regex("^\\d{4}-\\d{2}-\\d{2}$") | ||
|
|
||
| return birthValidator.matches(birth) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
데이터베이스 제약조건 및 인덱스 추가 권장
검증 로직은 잘 구현되어 있지만, 데이터베이스 레벨의 보장이 필요합니다.
User 엔티티에 다음 어노테이션을 추가하세요:
@Entity
-@Table(name = "users")
+@Table(
+ name = "users",
+ indexes = [Index(name = "idx_user_id", columnList = "userId")],
+ uniqueConstraints = [UniqueConstraint(name = "uk_user_id", columnNames = ["userId"])]
+)
class User(그리고 userId 필드에:
+ @Column(unique = true, nullable = false, length = 10)
var userId: String = userId
protected set이유:
uniqueConstraints: 애플리케이션 레벨 검증만으로는 동시성 상황에서 중복 방지 불가Index:findByUserId쿼리 성능 최적화
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt around
lines 46–64, add database-level uniqueness and an index: annotate the User
entity class with a @Table that declares a UniqueConstraint on the userId column
and an @Index for userId; also annotate the userId field to be non-nullable and
unique at the column level (e.g., @Column(nullable = false, unique = true) or
equivalent) so the DB enforces uniqueness and the findByUserId query is indexed
for performance.
| fun chargePointByUserId(userId: String, point: Int): Int { | ||
| val user = userRepository.findByUserId(userId) | ||
| ?: throw CoreException(ErrorType.NOT_FOUND, "user not found") | ||
|
|
||
| user.chargePoint(point) | ||
| return user.point | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
트랜잭션 경계가 누락되어 포인트 변경사항이 저장되지 않습니다.
chargePoint() 호출로 엔티티가 변경되지만, @Transactional 어노테이션이 없고 명시적인 save() 호출도 없어서 변경사항이 데이터베이스에 반영되지 않습니다.
작성자 질문에 대한 답변:
- User 엔티티에 포인트를 저장하는 것은 현재 요구사항에서는 적절합니다. 다만 향후 포인트 이력 추적, 트랜잭션 관리 등이 필요해지면 별도의
PointTransaction엔티티를 고려해야 합니다. - 포인트 충전 후 명시적인 저장이 필요한지에 대해: JPA의 더티 체킹(dirty checking)을 활용하려면
@Transactional을 추가해야 하며, 그렇지 않으면userRepository.save(user)를 명시적으로 호출해야 합니다.
해결 방법 1 (권장): @Transactional 추가
+ @Transactional
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
return user.point
}해결 방법 2: 명시적 저장
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
+ userRepository.save(user)
return user.point
}📝 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.
| fun chargePointByUserId(userId: String, point: Int): Int { | |
| val user = userRepository.findByUserId(userId) | |
| ?: throw CoreException(ErrorType.NOT_FOUND, "user not found") | |
| user.chargePoint(point) | |
| return user.point | |
| } | |
| @Transactional | |
| fun chargePointByUserId(userId: String, point: Int): Int { | |
| val user = userRepository.findByUserId(userId) | |
| ?: throw CoreException(ErrorType.NOT_FOUND, "user not found") | |
| user.chargePoint(point) | |
| return user.point | |
| } |
| fun chargePointByUserId(userId: String, point: Int): Int { | |
| val user = userRepository.findByUserId(userId) | |
| ?: throw CoreException(ErrorType.NOT_FOUND, "user not found") | |
| user.chargePoint(point) | |
| return user.point | |
| } | |
| fun chargePointByUserId(userId: String, point: Int): Int { | |
| val user = userRepository.findByUserId(userId) | |
| ?: throw CoreException(ErrorType.NOT_FOUND, "user not found") | |
| user.chargePoint(point) | |
| userRepository.save(user) | |
| return user.point | |
| } |
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserService.kt
around lines 32-38, the method mutates the User entity via
user.chargePoint(point) but lacks a transactional boundary or an explicit save,
so the change isn't persisted; fix by annotating the method (or the service
class) with @Transactional and import
org.springframework.transaction.annotation.Transactional so JPA dirty-checking
will flush the updated point, or alternatively call userRepository.save(user)
after chargePoint(point) if you prefer explicit saves; ensure the chosen
approach is consistently applied and the method still returns user.point.
| @Operation( | ||
| summary = "회원 포인트 조회", | ||
| description = "ID로 회원 포인트를 조회합니다.", | ||
| ) | ||
| fun getPoint( | ||
| @Schema(name = "회원 ID", description = "조회할 회원의 ID") | ||
| userId: String?, | ||
| ): ApiResponse<Int> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파라미터 nullable 불일치 확인 필요
getPoint의 userId 파라미터가 nullable(String?)로 선언되어 있는데, 이는 getUser의 non-nullable userId와 일관성이 없습니다.
만약 인증 컨텍스트에서 userId를 가져오는 등의 이유로 의도적으로 optional이라면, 이를 주석이나 문서에 명시하는 것이 좋습니다. 그렇지 않다면 non-nullable로 변경해야 합니다.
일관성을 위해 다음과 같이 수정하는 것을 권장합니다:
fun getPoint(
@Schema(name = "회원 ID", description = "조회할 회원의 ID")
- userId: String?,
+ userId: String,
): ApiResponse<Int>📝 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.
| @Operation( | |
| summary = "회원 포인트 조회", | |
| description = "ID로 회원 포인트를 조회합니다.", | |
| ) | |
| fun getPoint( | |
| @Schema(name = "회원 ID", description = "조회할 회원의 ID") | |
| userId: String?, | |
| ): ApiResponse<Int> | |
| @Operation( | |
| summary = "회원 포인트 조회", | |
| description = "ID로 회원 포인트를 조회합니다.", | |
| ) | |
| fun getPoint( | |
| @Schema(name = "회원 ID", description = "조회할 회원의 ID") | |
| userId: String, | |
| ): ApiResponse<Int> |
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1ApiSpec.kt
around lines 25–32, the getPoint parameter userId is declared nullable (String?)
but getUser uses a non-nullable userId; make this consistent by changing userId
to non-nullable String if it must always be provided, and update the @Schema
annotation to reflect required=true; if nullability was intentional, add a KDoc
comment and update the OpenAPI/@Schema to document that the parameter is
optional and how null is handled.
| @DisplayName("포인트 충전시, ") | ||
| @Nested | ||
| inner class ChargePoint { | ||
| @DisplayName("해당 ID의 회원이 존재하지 않을 경우, Null이 반환된다.") | ||
| @Test | ||
| fun returnNull_whenUserNotExists() { | ||
| // arrange | ||
| val userId = "testId" | ||
|
|
||
| // act | ||
| val exception = assertThrows<CoreException> { | ||
| userService.chargePointByUserId(userId, 50) | ||
| } | ||
|
|
||
| // assert | ||
| assertThat(exception.errorType).isEqualTo(ErrorType.NOT_FOUND) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
포인트 충전 성공 케이스 테스트가 누락되었습니다.
현재는 사용자를 찾지 못하는 경우만 테스트하고 있습니다. 실제 포인트 충전이 성공하는 케이스와 영속성 검증이 필요합니다. 이 테스트가 있었다면 UserService.chargePointByUserId의 트랜잭션 이슈를 발견할 수 있었을 것입니다.
다음 테스트를 추가하세요:
@DisplayName("해당 ID의 회원이 존재할 경우, 포인트가 충전되고 충전된 총 포인트가 반환된다.")
@Test
fun chargePoint_whenUserExists() {
// arrange
val user = userJpaRepository.save(User(userId = "testId", email = "[email protected]", birth = "2025-10-25", gender = Gender.OTHER))
val chargeAmount = 1000
// act
val result = userService.chargePointByUserId(user.userId, chargeAmount)
// assert
assertAll(
{ assertThat(result).isEqualTo(chargeAmount) },
// 실제 DB에 저장되었는지 확인
{ assertThat(userJpaRepository.findById(user.id).get().point).isEqualTo(chargeAmount) }
)
}🤖 Prompt for AI Agents
In
apps/commerce-api/src/test/kotlin/com/loopers/domain/user/UserServiceIntegrationTest.kt
around lines 129-146, the test suite only covers the "user not found" error case
and is missing a success case that verifies point charging and persistence; add
a new test that saves a User via userJpaRepository with a known userId, calls
userService.chargePointByUserId(user.userId, chargeAmount), asserts the returned
value equals the charged amount, and then reloads the user from
userJpaRepository.findById(user.id) to assert the user's point field equals the
charged amount (use assertAll to group assertions).
| @DisplayName("POST /api/v1/users/chargePoint") | ||
| @Nested | ||
| inner class ChargePoint { | ||
| @DisplayName("존재하는 유저가 1000원을 충전할 경우, 충전된 보유 총량을 응답으로 반환한다.") | ||
| @Test | ||
| fun returnsUserPoint_whenUserExists() { | ||
| // arrange | ||
| val user = userJpaRepository.save(User(userId = "testId", email = "[email protected]", birth = "2025-10-25", gender = Gender.OTHER)) | ||
| val req = UserV1Dto.ChargePointRequest( | ||
| userId = user.userId, | ||
| amount = 1000, | ||
| ) | ||
|
|
||
| // act | ||
| val responseType = object : ParameterizedTypeReference<ApiResponse<Int>>() {} | ||
| val res = testRestTemplate.exchange(ENDPOINT_CHARGEPOINT, HttpMethod.POST, HttpEntity(req), responseType) | ||
|
|
||
| // assert | ||
| assertAll( | ||
| { assertThat(res.statusCode.is2xxSuccessful).isTrue() }, | ||
| { assertThat(res.body?.data).isEqualTo(1000) }, | ||
| ) | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 유저로 요청할 경우, 404 Not Found 응답을 반환한다.") | ||
| @Test | ||
| fun throwNotFound_whenUserNotExists() { | ||
| // arrange | ||
| val req = UserV1Dto.ChargePointRequest( | ||
| userId = "testId", | ||
| amount = 1000, | ||
| ) | ||
|
|
||
| // act | ||
| val responseType = object : ParameterizedTypeReference<ApiResponse<Int>>() {} | ||
| val res = testRestTemplate.exchange(ENDPOINT_CHARGEPOINT, HttpMethod.POST, HttpEntity(req), responseType) | ||
|
|
||
| // assert | ||
| assertAll( | ||
| { assertThat(res.statusCode.is4xxClientError).isTrue }, | ||
| { assertThat(res.statusCode).isEqualTo(HttpStatus.NOT_FOUND) }, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
포인트 충전 테스트가 포함되어 있으나 영속성 검증을 추가하세요.
현재 테스트는 반환된 포인트 값만 확인하고 있습니다. UserService.chargePointByUserId의 트랜잭션 이슈로 인해 실제로 데이터베이스에 저장되지 않을 수 있습니다.
테스트에 데이터베이스 재조회를 추가하여 영속성을 검증하세요:
// assert
assertAll(
{ assertThat(res.statusCode.is2xxSuccessful).isTrue() },
{ assertThat(res.body?.data).isEqualTo(1000) },
+ // 실제 DB에 저장되었는지 확인
+ { assertThat(userJpaRepository.findById(user.id).get().point).isEqualTo(1000) },
)🤖 Prompt for AI Agents
In
apps/commerce-api/src/test/kotlin/com/loopers/interfaces/api/UserV1ApiE2ETest.kt
around lines 172 to 215, the charge-point tests only assert the HTTP response
and do not verify the database was actually updated; add a persistence
verification by reloading the user from userJpaRepository (or calling findById)
after the POST and assert the user's persisted point total matches the expected
value (ensure you use the saved user's id, handle optional result, and consider
repository.flush() or transaction boundaries if needed).
📌 Summary
💬 Review Points
포인트를 유저가 갖고 있도록 했는데, 적절한 설계일지
포인트 충전 후 유저 정보를 디비에 다시 저장하지 않았는데 괜찮을지
의견을 듣고 싶습니다.
✅ Checklist
Summary by CodeRabbit
릴리스 노트
새로운 기능