refactor: 인증 도메인 리팩토링 #42
Conversation
📝 Walkthrough""" Walkthrough인증 도메인 전반에 걸쳐 리팩토링이 이루어졌습니다. 주요 변경 사항은 인증 및 사용자 관련 서비스, DTO, VO, 예외 처리, 리포지토리, 전략 패턴 등에서 요청/응답 객체 도입, 반환 타입 및 메서드 시그니처 일치화, 예외 명확화, soft-delete 적용, 불필요한 코드 제거 및 네이밍 명확화 등입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthUseCase
participant SocialAuthService
participant AuthStrategy
participant UserAuthService
participant UserDomainService
Client->>AuthController: 소셜 로그인 요청(SocialLoginRequest)
AuthController->>AuthUseCase: signIn(socialLoginRequest)
AuthUseCase->>SocialAuthService: resolveStrategy + authenticate
SocialAuthService->>AuthStrategy: authenticate(credentials)
AuthStrategy-->>SocialAuthService: UserCreateInfoResponse
SocialAuthService-->>AuthUseCase: UserCreateInfoResponse
AuthUseCase->>UserAuthService: findOrCreateUser(FindOrCreateUserRequest)
UserAuthService->>UserDomainService: findUserByProviderTypeAndProviderId
alt 사용자 없음
UserAuthService->>UserDomainService: create(...)
else 삭제된 사용자
UserAuthService->>UserDomainService: restoreDeletedUser
end
UserDomainService-->>UserAuthService: UserIdentityVO
UserAuthService-->>AuthUseCase: CreateUserResponse
AuthUseCase->>AuthUseCase: GenerateTokenPairRequest 생성
AuthUseCase-->>Client: TokenPairResponse
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 변경사항 내에서 명확히 범위를 벗어난 기능적 코드 변경은 발견되지 않았습니다.) ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (27)
apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthControllerApi.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt(0 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserCreateInfoResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserProfileResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/auth/service/SocialAuthService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/service/UserAuthService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/strategy/AppleAuthStrategy.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/auth/strategy/AuthCredentials.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/strategy/AuthStrategy.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/auth/strategy/KakaoAuthStrategy.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(0 hunks)domain/src/main/kotlin/org/yapp/domain/user/User.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/UserRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/exception/UserErrorCode.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/exception/UserNotFoundException.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/vo/UserIdentity.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfile.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt(1 hunks)
💤 Files with no reviewable changes (2)
- apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt
- apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt
🧰 Additional context used
🪛 detekt (1.23.8)
domain/src/main/kotlin/org/yapp/domain/user/exception/UserNotFoundException.kt
[warning] 8-9: The class or object UserNotFoundException is empty.
(detekt.empty-blocks.EmptyClassBlock)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (43)
apis/src/main/kotlin/org/yapp/apis/auth/strategy/AuthCredentials.kt (1)
1-1: 패키지 구조 개선이 적절합니다.인증 전략 관련 클래스들을
dto패키지에서strategy패키지로 이동한 것은 관심사 분리 원칙에 따라 코드 구조를 개선한 좋은 변경사항입니다.apis/src/main/kotlin/org/yapp/apis/auth/service/SocialAuthService.kt (1)
4-4: import 경로 업데이트가 올바릅니다.
AuthCredentials의 패키지 이동에 따른 import 경로 업데이트가 정확하게 반영되었습니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt (1)
5-7: import 경로 업데이트가 일관되게 적용되었습니다.인증 전략 관련 클래스들(
AppleAuthCredentials,AuthCredentials,KakaoAuthCredentials)의 import 경로가 모두 올바르게strategy패키지로 업데이트되었습니다.apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthControllerApi.kt (1)
1-109: @RequestMapping 매핑이 AuthController 구현체에 포함되어 있습니다인터페이스에서 제거된
@RequestMapping("/api/v1/auth")애노테이션은AuthController클래스 상단에 선언되어 있어 URL 매핑이 정상적으로 동작합니다. 추가 조치는 필요하지 않습니다.domain/src/main/kotlin/org/yapp/domain/user/exception/UserNotFoundException.kt (1)
1-10: 적절한 예외 클래스 구현입니다.
UserNotFoundException이CommonException을 상속받아 표준적인 예외 처리 패턴을 따르고 있습니다. 생성자에서UserErrorCode와 선택적 메시지를 받아 부모 클래스에 전달하는 구조가 적절합니다.정적 분석 도구의 "empty class block" 경고는 false positive입니다. 클래스가 생성자를 가지고 있어 실제로는 비어있지 않습니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserProfileResponse.kt (2)
5-5: 도메인 값 객체 import 추가 확인됨
UserProfile값 객체를 사용하기 위한 import가 적절히 추가되었습니다.
39-46: 팩토리 메서드 리팩토링 승인개별 파라미터를 받던
of메서드를UserProfile객체를 받는from메서드로 변경한 것은 좋은 개선입니다. 이는 캡슐화를 향상시키고 파라미터 오류 가능성을 줄여줍니다.apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthController.kt (2)
25-25: 컨트롤러 로직 단순화 승인중간 변환 단계를 제거하고 요청 객체를 직접 전달하는 것은 좋은 개선입니다. 이는 관심사의 분리를 향상시키고 컨트롤러의 복잡성을 줄여줍니다.
31-31: 토큰 갱신 로직 단순화 승인
validRefreshToken()호출을 제거하고 전체 요청 객체를 직접 전달하는 것은 적절한 리팩토링입니다. 검증 로직이 유스케이스 레이어로 이동된 것으로 보입니다.apis/src/main/kotlin/org/yapp/apis/auth/strategy/AuthStrategy.kt (2)
3-3: 응답 DTO로의 import 변경 승인
UserCreateInfo에서UserCreateInfoResponse로의 import 변경은 리팩토링 목표와 일치합니다.
14-14: AuthStrategyauthenticate반환 타입 업데이트 완료 확인
AuthStrategy인터페이스(AuthStrategy.kt)와 구현체(KakaoAuthStrategy.kt,AppleAuthStrategy.kt)의authenticate메서드 모두UserCreateInfoResponse로 일관되게 변경된 것을 확인했습니다.- 더 이상의 추가 검증이나 수정은 필요하지 않습니다.
apis/src/main/kotlin/org/yapp/apis/auth/strategy/AppleAuthStrategy.kt (3)
5-5: 응답 DTO import 변경 승인
UserCreateInfoResponse로의 import 변경이 적절히 이루어졌습니다.
24-24: 인터페이스 구현 메서드 시그니처 변경 승인
AuthStrategy인터페이스 변경에 맞게 반환 타입이UserCreateInfoResponse로 적절히 변경되었습니다.
46-54: 헬퍼 메서드 리팩토링 승인
createUserInfo메서드의 반환 타입과 구현이 새로운 DTO 구조에 맞게 잘 변경되었습니다.UserCreateInfoResponse.of()팩토리 메서드 사용이 적절합니다.apis/src/main/kotlin/org/yapp/apis/auth/strategy/KakaoAuthStrategy.kt (3)
5-5: 응답 DTO import 변경 승인
UserCreateInfoResponse로의 import 변경이 적절히 이루어졌습니다.
25-25: 인터페이스 구현 메서드 시그니처 변경 승인
AuthStrategy인터페이스 변경에 맞게 반환 타입이UserCreateInfoResponse로 적절히 변경되었습니다.AppleAuthStrategy와 일관된 구현입니다.
47-55: 헬퍼 메서드 리팩토링 승인
createUserInfo메서드의 반환 타입과 구현이 새로운 DTO 구조에 맞게 잘 변경되었습니다. 다른 전략 구현체들과 일관된 패턴을 보여줍니다.domain/src/main/kotlin/org/yapp/domain/user/vo/UserIdentity.kt (1)
16-24: 팩토리 메서드 구현이 적절합니다nullable
User객체를 받아UserIdentity를 생성하는 팩토리 메서드가 잘 구현되어 있습니다. null 체크와 객체 생성 로직이 명확합니다.domain/src/main/kotlin/org/yapp/domain/user/User.kt (1)
67-68: 한국어 주석이 적절합니다향후 다른 역할 부여 시 사용한다는 주석이 명확하고 적절합니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt (3)
8-11: Swagger 스키마 문서화가 적절합니다API 문서화를 위한 Swagger 애노테이션이 잘 작성되어 있습니다. 이름과 설명이 명확합니다.
12-18: DTO 설계 패턴이 우수합니다private constructor와 companion object 패턴을 사용하여 객체 생성을 제어하는 설계가 훌륭합니다. 한국어 설명과 예시 값도 적절합니다.
19-26: 팩토리 메서드 구현이 깔끔합니다
UserIdentity로부터CreateUserResponse를 생성하는 팩토리 메서드가 간결하고 명확합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserCreateInfoResponse.kt (3)
6-9: Swagger 문서화가 상세하고 적절합니다소셜 로그인을 통한 신규 사용자 정보 DTO에 대한 설명이 명확하고 구체적입니다.
10-41: 필드 설계가 적절합니다nullable과 non-nullable 필드가 소셜 로그인 특성에 맞게 잘 구분되어 있습니다. 소셜 제공자마다 제공하는 정보가 다를 수 있음을 고려한 설계입니다.
42-58: 입력값 검증이 우수합니다팩토리 메서드에서
providerId.trim()을 통해 공백을 제거하는 것은 훌륭한 방어적 프로그래밍 기법입니다. 외부 API로부터 받은 데이터의 품질을 보장합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt (2)
12-18: 최소한의 정보 노출 설계가 우수합니다인증에 필요한 최소한의 정보(ID와 역할)만을 노출하는 설계가 보안상 좋습니다. private constructor 패턴도 적절합니다.
19-26: 팩토리 메서드 구현이 일관성 있습니다다른 Response DTO들과 동일한 패턴의 팩토리 메서드로 일관성이 있고 구현이 명확합니다.
domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfile.kt (1)
7-12: UserProfile 클래스 구조가 잘 설계되었습니다Value Object 패턴을 적절히 적용하여 불변성과 캡슐화를 보장했습니다. private constructor와 companion object factory method 조합이 좋습니다.
domain/src/main/kotlin/org/yapp/domain/user/exception/UserErrorCode.kt (1)
6-16: 에러 코드 enum 구현이 깔끔하고 확장 가능합니다BaseErrorCode 인터페이스를 구현하여 일관성 있는 에러 처리 구조를 제공합니다. 향후 다른 사용자 관련 에러 코드 추가가 용이합니다.
infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserRepository.kt (1)
16-18: 논리적 삭제를 고려한 존재성 검사 메소드가 잘 구현되었습니다
deletedAt필드가 null인 경우만 존재한다고 판단하는 로직이 논리적 삭제 패턴에 적합합니다. 메소드 명도 명확하게 의도를 전달합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt (1)
33-48: factory method 구현이 적절합니다companion object의 factory method들이 잘 구현되어 있고, 서로 다른 response 타입에서 GenerateTokenPairRequest를 생성할 수 있어 유연합니다.
domain/src/main/kotlin/org/yapp/domain/user/UserRepository.kt (2)
11-11: nullable 반환 타입 변경이 적절합니다findById 메소드가 User?를 반환하도록 변경된 것이 명시적인 null 처리를 가능하게 하여 더 안전합니다.
19-22: 논리적 삭제를 고려한 존재성 검사 메소드가 잘 설계되었습니다메소드명이 명확하게 의도를 전달하고, deletedAt 필드가 null인 경우만 존재한다고 판단하는 로직이 논리적 삭제 패턴에 완벽히 부합합니다.
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt (2)
26-28: 적절한 nullable 반환 타입 변경입니다.
findById메서드가 nullableUser?를 반환하도록 변경된 것은 도메인 계층의 명시적인 예외 처리 방식과 일치합니다.
30-36: 활성 사용자 확인을 위한 명확한 메서드 추가삭제되지 않은 사용자만을 확인하는
existsByEmailAndDeletedAtIsNull와existsByIdAndDeletedAtIsNull메서드 추가가 적절합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt (1)
44-50: 기본값 생성 로직이 적절합니다이메일과 닉네임의 기본값 생성 로직이 명확하고 안전하게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/auth/service/UserAuthService.kt (2)
18-32: 도메인과 API 계층 간의 명확한 변환Value Object를 DTO로 변환하는 메서드들이 적절하게 구현되었습니다. 명시적인 예외 처리도 좋습니다.
34-51: 사용자 찾기/생성 로직의 개선된 구조기존 사용자, 삭제된 사용자 복원, 신규 사용자 생성의 순서가 논리적이고 명확합니다.
apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt (2)
26-39: 명확한 DTO 체이닝 구조각 단계별로 적절한 DTO 변환이 이루어지며, 책임이 명확하게 분리되어 있습니다.
42-54: 토큰 재발급 로직의 개선
TokenRefreshRequestDTO 사용과 명시적인 사용자 인증 정보 조회가 적절합니다.domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt (3)
16-24: Value Object와 명시적 예외 처리의 적절한 사용
UserProfile과UserIdentityValue Object 반환 및UserNotFoundException예외 처리가 도메인 계층의 책임을 명확히 합니다.
36-42: 개선된 메서드 명명
existsActiveUserByIdAndDeletedAtIsNull와existsActiveUserByEmailAndDeletedAtIsNull메서드명이 의도를 명확히 표현합니다.
44-63: 명시적 파라미터를 사용한 사용자 생성
SocialUserProfile대신 개별 파라미터를 받는 방식이 더 명확하고 유연합니다.
| import org.yapp.apis.auth.dto.response.UserProfileResponse | ||
| import org.yapp.globalutils.exception.ErrorResponse | ||
| import java.util.UUID | ||
| import java.util.* |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
wildcard import 사용을 재고해보세요.
java.util.* wildcard import 대신 java.util.UUID를 명시적으로 import하는 것이 더 명확하고 예측 가능합니다.
다음과 같이 수정하는 것을 권장합니다:
-import java.util.*
+import java.util.UUID🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthControllerApi.kt at
line 20, replace the wildcard import 'java.util.*' with an explicit import of
'java.util.UUID' to improve code clarity and maintainability.
There was a problem hiding this comment.
@minwoo1999 네, 맞습니다! 명시적인 import가 코드 가독성과 유지보수성에 더 좋겠네요.
이전에 학습한 내용에 따르면, IntelliJ IDEA에서도 와일드카드 임포트를 방지하는 설정을 통해 이런 상황을 미연에 방지할 수 있습니다. File > Settings > Editor > Code Style > Java > Imports 탭에서 "Class count to use import with '*'" 값을 높게 설정하시면 도움이 될 것 같습니다.
해당 변경사항을 적용해드리겠습니다.
Task: Replace wildcard import with explicit import
- Replace
java.util.*withjava.util.UUIDin AuthControllerApi.kt
🧠 Learnings used
Learnt from: hoonyworld
PR: YAPP-Github/Reed-Server#31
File: apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt:13-13
Timestamp: 2025-07-08T14:24:44.340Z
Learning: IntelliJ IDEA에서 와일드카드 임포트를 방지하는 설정 방법: File > Settings > Editor > Code Style > Java > Imports 탭에서 "Class count to use import with '*'" 값을 높게 설정(예: 500)하면 해당 임계값 이하에서는 와일드카드 임포트가 자동으로 사용되지 않음. 와일드카드 임포트 관련 리뷰 시 이 IDE 설정 방법도 함께 안내하면 좋음.
Learnt from: hoonyworld
PR: YAPP-Github/Reed-Server#31
File: apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt:13-13
Timestamp: 2025-07-08T14:24:44.340Z
Learning: hoonyworld가 와일드카드 임포트 관련하여 IDE 설정을 통한 자동 최적화 방법을 추천했습니다. 향후 임포트 관련 리뷰 시 IDE 설정 방법도 함께 안내하는 것이 좋겠습니다.
⏳ Preparing changes...
There was a problem hiding this comment.
A summary of the changes CodeRabbit can apply:
- Update the AuthControllerApi.kt file by replacing the wildcard import of java.util.* with an explicit import of java.util.UUID to improve code clarity and maintainability.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
| init { | ||
| requireNotNull(id) { "User ID must not be null." } | ||
| requireNotNull(role) { "User role must not be null." } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
불필요한 null 체크 제거 필요
id와 role 프로퍼티가 이미 non-nullable 타입으로 선언되어 있으므로, init 블록의 requireNotNull 체크는 불필요합니다. 컴파일러가 이미 null 값을 허용하지 않습니다.
- init {
- requireNotNull(id) { "User ID must not be null." }
- requireNotNull(role) { "User role must not be null." }
- }📝 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.
| init { | |
| requireNotNull(id) { "User ID must not be null." } | |
| requireNotNull(role) { "User role must not be null." } | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/vo/UserIdentity.kt at lines 11 to
14, remove the init block containing requireNotNull checks for id and role, as
these properties are already declared with non-nullable types, making the null
checks redundant and unnecessary.
| fun create( | ||
| email: String, | ||
| nickname: String, | ||
| profileImageUrl: String?, | ||
| providerType: ProviderType, | ||
| providerId: String, | ||
| createdAt: LocalDateTime, | ||
| updatedAt: LocalDateTime, | ||
| deletedAt: LocalDateTime? = null | ||
| ): User { | ||
| return User( | ||
| id = UUID.randomUUID(), | ||
| email = email, | ||
| nickname = nickname, | ||
| profileImageUrl = profileImageUrl, | ||
| providerType = providerType, | ||
| providerId = providerId, | ||
| role = Role.USER, | ||
| createdAt = createdAt, | ||
| updatedAt = updatedAt, | ||
| deletedAt = deletedAt | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
기본 사용자 생성을 위한 팩토리 메서드 추가를 승인합니다
기본 Role.USER로 사용자를 생성하는 간편한 팩토리 메서드가 잘 구현되어 있습니다. 일반적인 사용자 생성 시나리오에서 유용할 것입니다.
create와 createWithRole 메서드 간의 코드 중복을 줄이기 위해 다음과 같이 개선할 수 있습니다:
fun create(
email: String,
nickname: String,
profileImageUrl: String?,
providerType: ProviderType,
providerId: String,
createdAt: LocalDateTime,
updatedAt: LocalDateTime,
deletedAt: LocalDateTime? = null
): User {
- return User(
- id = UUID.randomUUID(),
- email = email,
- nickname = nickname,
- profileImageUrl = profileImageUrl,
- providerType = providerType,
- providerId = providerId,
- role = Role.USER,
- createdAt = createdAt,
- updatedAt = updatedAt,
- deletedAt = deletedAt
- )
+ return createWithRole(
+ email = email,
+ nickname = nickname,
+ profileImageUrl = profileImageUrl,
+ providerType = providerType,
+ providerId = providerId,
+ role = Role.USER,
+ createdAt = createdAt,
+ updatedAt = updatedAt,
+ deletedAt = deletedAt
+ )
}📝 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 create( | |
| email: String, | |
| nickname: String, | |
| profileImageUrl: String?, | |
| providerType: ProviderType, | |
| providerId: String, | |
| createdAt: LocalDateTime, | |
| updatedAt: LocalDateTime, | |
| deletedAt: LocalDateTime? = null | |
| ): User { | |
| return User( | |
| id = UUID.randomUUID(), | |
| email = email, | |
| nickname = nickname, | |
| profileImageUrl = profileImageUrl, | |
| providerType = providerType, | |
| providerId = providerId, | |
| role = Role.USER, | |
| createdAt = createdAt, | |
| updatedAt = updatedAt, | |
| deletedAt = deletedAt | |
| ) | |
| } | |
| fun create( | |
| email: String, | |
| nickname: String, | |
| profileImageUrl: String?, | |
| providerType: ProviderType, | |
| providerId: String, | |
| createdAt: LocalDateTime, | |
| updatedAt: LocalDateTime, | |
| deletedAt: LocalDateTime? = null | |
| ): User { | |
| return createWithRole( | |
| email = email, | |
| nickname = nickname, | |
| profileImageUrl = profileImageUrl, | |
| providerType = providerType, | |
| providerId = providerId, | |
| role = Role.USER, | |
| createdAt = createdAt, | |
| updatedAt = updatedAt, | |
| deletedAt = deletedAt | |
| ) | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/User.kt between lines 43 and 65,
the create method duplicates code similar to createWithRole. To reduce
duplication, refactor create to call createWithRole with Role.USER as the role
parameter, passing all other arguments through. This centralizes user creation
logic and avoids repeating the User constructor call.
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt
Show resolved
Hide resolved
| init { | ||
| require(email.isNotBlank()) { "email은 비어 있을 수 없습니다." } | ||
| require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."} | ||
| require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
validation 로직에서 일관성 문제가 있습니다
provider.name으로 검증하는 것보다 enum 자체의 유효성을 검증하는 것이 더 적절할 수 있습니다.
- require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." }
+ // ProviderType은 enum이므로 이미 유효한 값이 보장됨또한 에러 메시지 형식을 일관성 있게 맞춰주세요:
- require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."}
+ require(nickname.isNotBlank()) { "nickname은 비어 있을 수 없습니다." }📝 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.
| init { | |
| require(email.isNotBlank()) { "email은 비어 있을 수 없습니다." } | |
| require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."} | |
| require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." } | |
| } | |
| init { | |
| require(email.isNotBlank()) { "email은 비어 있을 수 없습니다." } | |
| require(nickname.isNotBlank()) { "nickname은 비어 있을 수 없습니다." } | |
| // ProviderType은 enum이므로 이미 유효한 값이 보장됨 | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfile.kt around lines 13
to 17, the validation logic inconsistently checks provider.name instead of
validating the enum provider directly. Change the validation to check the
provider enum's validity rather than its name string. Also, update the error
messages to have a consistent format matching the other require statements.
| companion object { | ||
| fun newInstance(user: User?): UserProfile { | ||
| requireNotNull(user) { "User는 null일 수 없습니다." } | ||
|
|
||
| return UserProfile( | ||
| id = user.id, | ||
| email = user.email, | ||
| nickname = user.nickname, | ||
| provider = user.providerType | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
factory method 구현이 적절하지만 개선이 필요합니다
requireNotNull 사용은 좋지만, 실제 사용 시 null인 경우 더 구체적인 예외 타입을 사용하는 것이 좋겠습니다.
companion object {
- fun newInstance(user: User?): UserProfile {
- requireNotNull(user) { "User는 null일 수 없습니다." }
+ fun newInstance(user: User?): UserProfile {
+ user ?: throw IllegalArgumentException("User는 null일 수 없습니다.")또는 null safety를 위해 non-null parameter로 변경하는 것도 고려해보세요:
- fun newInstance(user: User?): UserProfile {
- requireNotNull(user) { "User는 null일 수 없습니다." }
+ fun newInstance(user: User): UserProfile {📝 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.
| companion object { | |
| fun newInstance(user: User?): UserProfile { | |
| requireNotNull(user) { "User는 null일 수 없습니다." } | |
| return UserProfile( | |
| id = user.id, | |
| email = user.email, | |
| nickname = user.nickname, | |
| provider = user.providerType | |
| ) | |
| } | |
| } | |
| } | |
| companion object { | |
| fun newInstance(user: User?): UserProfile { | |
| - requireNotNull(user) { "User는 null일 수 없습니다." } | |
| + user ?: throw IllegalArgumentException("User는 null일 수 없습니다.") | |
| return UserProfile( | |
| id = user.id, | |
| email = user.email, | |
| nickname = user.nickname, | |
| provider = user.providerType | |
| ) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfile.kt around lines 19
to 31, the factory method newInstance uses requireNotNull with a generic
exception for null user input. Improve this by either changing the user
parameter to be non-nullable to enforce null safety at compile time or throw a
more specific exception type when user is null to provide clearer error
handling.
| private val code: String, | ||
| private val message: String | ||
| ) : BaseErrorCode { | ||
| USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_001", "유저 정보를 찾을 수 없습니다."); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
세미콜론 제거를 권장합니다
Kotlin에서는 세미콜론이 필요하지 않으므로 제거하는 것이 좋습니다.
- USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_001", "유저 정보를 찾을 수 없습니다.");
+ USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_001", "유저 정보를 찾을 수 없습니다.")📝 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.
| USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_001", "유저 정보를 찾을 수 없습니다."); | |
| USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_001", "유저 정보를 찾을 수 없습니다.") |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/exception/UserErrorCode.kt at
line 11, remove the semicolon at the end of the USER_NOT_FOUND enum entry, as
Kotlin does not require semicolons and it is recommended to omit them for
cleaner code.
| fun validUserId(): UUID = userId!! | ||
| fun validRole(): Role = role!! | ||
|
|
There was a problem hiding this comment.
!! 연산자 사용으로 인한 런타임 예외 위험
nullable 필드를 non-null로 강제 변환하는 !! 연산자는 런타임 KotlinNullPointerException을 발생시킬 수 있습니다.
위의 필드를 non-null로 변경하면 이 메소드들도 불필요해집니다:
- fun validUserId(): UUID = userId!!
- fun validRole(): Role = role!!📝 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 validUserId(): UUID = userId!! | |
| fun validRole(): Role = role!! |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt
around lines 30 to 32, the functions validUserId() and validRole() use the !!
operator to force unwrap nullable fields, which risks runtime
KotlinNullPointerException. To fix this, change the userId and role fields to be
non-nullable types so that these functions become unnecessary and can be removed
safely.
| data class GenerateTokenPairRequest private constructor( | ||
| @field:NotBlank(message = "userId must not be null") | ||
| @Schema( | ||
| description = "User ID", | ||
| example = "a1b2c3d4-e5f6-7890-1234-56789abcdef0" | ||
| ) | ||
| val userId: UUID? = null, | ||
|
|
||
| @field:NotBlank(message = "role must not be null") | ||
| @Schema( | ||
| description = "User role", | ||
| example = "USER" | ||
| ) | ||
| val role: Role? = null | ||
| ) { |
There was a problem hiding this comment.
validation annotation 사용이 부적절합니다
@notblank는 String에만 사용할 수 있는 annotation인데 UUID와 Role에 사용되었습니다. 또한 nullable 필드에 @notblank를 사용하는 것도 일관성이 없습니다.
- @field:NotBlank(message = "userId must not be null")
+ @field:NotNull(message = "userId must not be null")
@Schema(
description = "User ID",
example = "a1b2c3d4-e5f6-7890-1234-56789abcdef0"
)
- val userId: UUID? = null,
+ val userId: UUID,
- @field:NotBlank(message = "role must not be null")
+ @field:NotNull(message = "role must not be null")
@Schema(
description = "User role",
example = "USER"
)
- val role: Role? = null
+ val role: Role📝 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.
| data class GenerateTokenPairRequest private constructor( | |
| @field:NotBlank(message = "userId must not be null") | |
| @Schema( | |
| description = "User ID", | |
| example = "a1b2c3d4-e5f6-7890-1234-56789abcdef0" | |
| ) | |
| val userId: UUID? = null, | |
| @field:NotBlank(message = "role must not be null") | |
| @Schema( | |
| description = "User role", | |
| example = "USER" | |
| ) | |
| val role: Role? = null | |
| ) { | |
| data class GenerateTokenPairRequest private constructor( | |
| @field:NotNull(message = "userId must not be null") | |
| @Schema( | |
| description = "User ID", | |
| example = "a1b2c3d4-e5f6-7890-1234-56789abcdef0" | |
| ) | |
| val userId: UUID, | |
| @field:NotNull(message = "role must not be null") | |
| @Schema( | |
| description = "User role", | |
| example = "USER" | |
| ) | |
| val role: Role | |
| ) { | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt
between lines 14 and 28, the @NotBlank annotation is incorrectly applied to UUID
and Role fields, which are not Strings, and also to nullable fields. Remove the
@NotBlank annotations from these fields and replace them with appropriate
validation annotations such as @NotNull to ensure the fields are not null, or
handle validation logic suitable for their types.
| @field:NotBlank(message = "providerType은 필수입니다.") | ||
| @Schema( | ||
| description = "소셜 로그인 제공자", | ||
| example = "KAKAO" | ||
| ) val providerType: ProviderType? = null, | ||
|
|
||
| @field:NotBlank(message = "providerId는 필수입니다.") | ||
| @Schema( | ||
| description = "소셜 제공자에서 발급한 식별자", | ||
| example = "12345678901234567890" | ||
| ) val providerId: String? = null |
There was a problem hiding this comment.
Nullable 필드와 @notblank 검증의 잠재적 런타임 예외 위험
providerType과 providerId 필드가 nullable로 선언되어 있으면서 @NotBlank 검증과 !! 연산자를 사용하는 것은 위험합니다. 검증이 실행되지 않거나 우회될 경우 NPE가 발생할 수 있습니다.
다음과 같이 non-nullable 필드로 변경하거나 안전한 접근 방식을 사용하세요:
- @field:NotBlank(message = "providerType은 필수입니다.")
- @Schema(
- description = "소셜 로그인 제공자",
- example = "KAKAO"
- ) val providerType: ProviderType? = null,
+ @Schema(
+ description = "소셜 로그인 제공자",
+ example = "KAKAO"
+ ) val providerType: ProviderType,
- @field:NotBlank(message = "providerId는 필수입니다.")
- @Schema(
- description = "소셜 제공자에서 발급한 식별자",
- example = "12345678901234567890"
- ) val providerId: String? = null
+ @field:NotBlank(message = "providerId는 필수입니다.")
+ @Schema(
+ description = "소셜 제공자에서 발급한 식별자",
+ example = "12345678901234567890"
+ ) val providerId: String
- fun validProviderType(): ProviderType = providerType!!
- fun validProviderId(): String = providerId!!
+ fun validProviderType(): ProviderType = providerType
+ fun validProviderId(): String = providerIdAlso applies to: 52-53
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt
around lines 32 to 42 and also lines 52 to 53, the fields providerType and
providerId are declared as nullable but annotated with @NotBlank, which can
cause runtime NullPointerExceptions if validation is bypassed. To fix this,
change these fields to be non-nullable by removing the '?' and ensure they are
always initialized with non-null values, or alternatively, implement safe null
checks before using them to avoid unsafe !! operators.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt (1)
8-11: Swagger 문서화 언어 일관성 검토 필요이미 이전 리뷰에서 지적된 것처럼, 다른 DTO들은 한국어 description을 사용하는데 이 파일은 영어를 사용하고 있습니다. 프로젝트 전반의 일관성을 위해 한국어 사용을 고려해보세요.
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt (2)
19-20: 검증 annotation 사용이 부적절합니다이전 리뷰에서 지적된 것처럼, @notblank는 String에만 사용할 수 있는 annotation인데 UUID와 Role에 사용되었습니다. 또한 nullable 필드에 @notblank를 사용하는 것도 일관성이 없습니다.
Also applies to: 26-27
30-32: !! 연산자 사용으로 인한 런타임 예외 위험이전 리뷰에서 지적된 것처럼, nullable 필드를 non-null로 강제 변환하는 !! 연산자는 런타임 KotlinNullPointerException을 발생시킬 수 있습니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt (1)
37-38: Nullable 필드와 @notblank 검증의 잠재적 런타임 예외 위험이전 리뷰에서 지적된 것처럼,
providerType과providerId필드가 nullable로 선언되어 있으면서@NotBlank검증과!!연산자를 사용하는 것은 위험합니다.Also applies to: 44-45
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/request/TokenRefreshRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/AuthResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (9)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt (3)
5-7: 리팩토링에 따른 import 경로 변경이 적절합니다.AuthCredentials 관련 클래스들을 strategy 패키지로 이동시킨 것이 인증 전략 패턴의 구조적 개선에 부합합니다. 패키지 구조가 더 명확해졌습니다.
17-21: Schema 어노테이션 포맷팅이 개선되었습니다.단일 라인에서 다중 라인 형식으로 변경하여 가독성이 향상되었습니다. API 문서화에 필요한 모든 속성들이 명확하게 정의되어 있습니다.
25-29: OAuth 토큰 필드의 Schema 어노테이션이 잘 정의되었습니다.다중 라인 형식으로 변경하여 가독성이 향상되었고, 설명과 예시가 적절하게 제공되어 API 문서의 품질이 개선되었습니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt (1)
8-33: 훌륭한 구현입니다!Response DTO 구현이 우수합니다:
- Private constructor와 companion object factory method 패턴 사용
- 한국어 Swagger 문서화로 프로젝트 일관성 유지
- 도메인 객체에서 DTO로 올바른 데이터 변환
- 명확한 책임 분리 (응답 전용 DTO)
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt (1)
12-33: Response DTO 구현 구조가 우수합니다CreateUserResponse와 동일한 패턴으로 일관성 있게 구현되었습니다:
- Private constructor와 factory method 패턴 사용
- 도메인 객체에서 적절한 데이터 추출
- 명확한 책임 분리
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/GenerateTokenPairRequest.kt (1)
34-47: Factory method 구현이 깔끔합니다Response DTO들로부터 Request DTO를 생성하는 factory method들이 잘 구현되어 있습니다:
- CreateUserResponse와 UserAuthInfoResponse 모두 지원
- 명확한 데이터 변환 로직
- 타입 안전성 보장
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/AuthResponse.kt (1)
24-25: 메소드 시그니처 포맷팅 변경 승인단순한 포맷팅 변경으로 기능에 영향을 주지 않습니다. 코드의 가독성도 유지됩니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindOrCreateUserRequest.kt (2)
47-53: 기본값 처리 로직이 우수합니다
getOrDefaultEmail()과getOrDefaultNickname()메소드가 잘 구현되어 있습니다:
- Null 안전성을 고려한 기본값 생성
- 빈 문자열 체크도 포함
- 도메인 로직에 맞는 기본값 생성 (이메일 형식, 닉네임 생성기 사용)
58-71: Factory method 구현이 깔끔합니다
UserCreateInfoResponse로부터 Request DTO를 생성하는 factory method가 잘 구현되어 있습니다:
- 모든 필드를 적절히 매핑
- 명확한 데이터 변환 로직
- 타입 안전성 보장
| @field:NotBlank(message = "Refresh token is required") | ||
| val refreshToken: String? = null | ||
| ) { | ||
| fun validRefreshToken(): String = refreshToken!! |
There was a problem hiding this comment.
🛠️ Refactor suggestion
불필요한 !! 제거로 NPE 위험 해소
위 필드가 non-null 로 변경되면 아래 메서드는 단순 반환으로 충분합니다.
-fun validRefreshToken(): String = refreshToken!!
+fun validRefreshToken(): String = refreshTokenCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/auth/dto/request/TokenRefreshRequest.kt at
line 19, the method validRefreshToken() uses the not-null assertion operator
(!!) on refreshToken, which can cause a NullPointerException. Since the
refreshToken field should be non-null, remove the !! operator and simply return
refreshToken directly to eliminate the unnecessary null check and avoid
potential NPE.
| @field:NotBlank(message = "Refresh token is required") | ||
| val refreshToken: String? = null |
There was a problem hiding this comment.
@NotBlank 와 nullable 타입이 상충됩니다.
@NotBlank 는 값이 null 이면 즉시 제약 위반이 발생하도록 설계되었습니다. 그런데 현재 refreshToken 타입이 String?(nullable) 이고 기본값까지 null 로 지정되어 있어, DTO 인스턴스가 생성되는 순간부터 제약 위반 상태가 됩니다. 또한 이후 !! 연산자 사용을 강제하게 되어 NPE 위험이 존재합니다.
- @field:NotBlank(message = "Refresh token is required")
- val refreshToken: String? = null
+ @field:NotBlank(message = "Refresh token is required")
+ val refreshToken: String이렇게 변경하면
- 컴파일 단계에서 null 전달을 차단하여 불필요한 런타임 검증을 줄이고,
- 하단
validRefreshToken()의!!도 제거할 수 있습니다.
DTO 생성 방식(예:@Builder, 정적 팩토리 등)이 있다면 해당 부분 역시 non-null 서명으로 맞춰주세요.
📝 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.
| @field:NotBlank(message = "Refresh token is required") | |
| val refreshToken: String? = null | |
| @field:NotBlank(message = "Refresh token is required") | |
| val refreshToken: String |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/auth/dto/request/TokenRefreshRequest.kt at
lines 16-17, the refreshToken property is marked as nullable String? with a
default null value but annotated with @NotBlank, which conflicts because
@NotBlank expects a non-null value. Change the type of refreshToken to a
non-nullable String without a default value to enforce non-null at compile time,
remove the default null, and update any related builder or factory methods to
match this non-null signature. This will eliminate the need for !! operators and
reduce runtime validation.
minwoo1999
left a comment
minwoo1999
left a comment
There was a problem hiding this comment.
"고생 많으셨습니다! 리뷰는 간단한 부분만 남겼으니 확인 부탁드릴게요 🙇♂️
동훈님의 생각 말씀 주시면 감사하겠습니다"
| import org.yapp.apis.auth.dto.response.CreateUserResponse | ||
| import org.yapp.apis.auth.dto.response.UserAuthInfoResponse | ||
| import org.yapp.globalutils.auth.Role | ||
| import java.util.* |
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import org.yapp.domain.user.vo.UserIdentity | ||
| import org.yapp.globalutils.auth.Role | ||
| import java.util.* |
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import org.yapp.domain.user.vo.UserIdentity | ||
| import org.yapp.globalutils.auth.Role | ||
| import java.util.* |
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import org.yapp.domain.user.ProviderType | ||
| import org.yapp.domain.user.vo.UserProfile | ||
| import java.util.* |
| import org.yapp.globalutils.auth.Role | ||
| import java.util.UUID | ||
|
|
||
| data class UserIdentity( |
There was a problem hiding this comment.
"해당 객체가 VO(Value Object)라는 점이 한눈에 잘 드러나지 않는 것 같아, 클래스 이름에 VO와 같은 접미사를 붙여 명시해보는 건 어떨까요? 유지보수 시 역할을 좀 더 명확히 구분하는 데 도움이 될 것 같습니다!"
| import org.yapp.domain.user.User | ||
| import java.util.UUID | ||
|
|
||
| data class UserProfile private constructor( |
There was a problem hiding this comment.
"해당 객체가 VO(Value Object)라는 점이 한눈에 잘 드러나지 않는 것 같아, 클래스 이름에 VO와 같은 접미사를 붙여 명시해보는 건 어떨까요? 유지보수 시 역할을 좀 더 명확히 구분하는 데 도움이 될 것 같습니다!"
| fun findByEmail(email: String): UserEntity? | ||
| fun existsByIdAndDeletedAtIsNull(id: UUID): Boolean | ||
|
|
||
| fun existsByEmailAndDeletedAtIsNull(email: String): Boolean |
There was a problem hiding this comment.
"DeletedAt이 null인지 반복적으로 체크하다 보면 중복된 코드가 계속 생길 수 있을 것 같아요.
@where(clause = "deleted_at IS NULL") 어노테이션을 entity 클래스에 적용하면, 기본적으로 해당 조건이 자동으로 적용돼 중복 없이 처리할 수 있어서 더 깔끔할 것 같습니다!"
There was a problem hiding this comment.
Hibernate 6에서 @where에서 @SQLRestriction로 바뀌었다고 해서 해당 설정으로 적용하겠습니다 ㅎㅎ
감사합니다!!
| fun findByProviderTypeAndProviderId(providerType: ProviderType, providerId: String): UserEntity? | ||
|
|
||
| fun findByEmail(email: String): UserEntity? | ||
| fun existsByIdAndDeletedAtIsNull(id: UUID): Boolean |
There was a problem hiding this comment.
"DeletedAt이 null인지 반복적으로 체크하다 보면 중복된 코드가 계속 생길 수 있을 것 같아요.
@where(clause = "deleted_at IS NULL") 어노테이션을 entity 클래스에 적용하면, 기본적으로 해당 조건이 자동으로 적용돼 중복 없이 처리할 수 있어서 더 깔끔할 것 같습니다!"
There was a problem hiding this comment.
Hibernate 6에서 @where에서 @SQLRestriction로 바뀌었다고 해서 해당 설정으로 적용하겠습니다 ㅎㅎ
감사합니다!!
| import org.yapp.globalutils.auth.Role | ||
| import org.yapp.globalutils.util.TimeProvider | ||
| import java.util.UUID | ||
| import java.util.* |
| deletedAt: LocalDateTime? = null | ||
| ): User { | ||
| return User( | ||
| id = UUID.randomUUID(), |
There was a problem hiding this comment.
"랜덤 UUID를 직접 사용하는 것보다는, 저희 GlobalUtils 안에 있는 uuidV7을 사용해서 생성하면 더 좋을 것 같아요! 통일된 방식으로 관리할 수 있어서 추후 유지보수나 로그 추적 측면에서도 이점이 있을 것 같습니다."
There was a problem hiding this comment.
오 제가 이 부분은 놓쳤네요!! 반영해두겠습니다!
… 리팩토링 - ID, EMAIL, PROVIDERID는 동일한 값을 가지면 같은 객체임
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt (1)
14-23: Swagger 문서화 언어 일관성을 맞춰주세요.과거 리뷰에서 지적된 것처럼 Swagger 어노테이션에서 한국어와 영어가 혼재되어 있습니다. 프로젝트 전반의 일관성을 위해 언어를 통일해주세요.
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt (1)
8-11: Swagger 문서화 언어 일관성을 맞춰주세요.과거 리뷰에서 지적된 것처럼 다른 DTO들과 언어 일관성을 맞춰야 합니다. 프로젝트 전반의 일관성을 위해 한국어 또는 영어로 통일해주세요.
domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt (1)
14-14: 불필요한 provider.name 검증 제거 필요ProviderType이 enum이므로 name이 항상 존재하여 이 검증은 불필요합니다.
domain/src/main/kotlin/org/yapp/domain/user/User.kt (1)
44-66: 팩토리 메서드의 코드 중복 개선 필요이전 리뷰에서 지적된 create와 createWithRole 메서드 간의 코드 중복이 여전히 존재합니다.
다음과 같이 개선할 수 있습니다:
fun create( email: String, nickname: String, profileImageUrl: String?, providerType: ProviderType, providerId: String, createdAt: LocalDateTime, updatedAt: LocalDateTime, deletedAt: LocalDateTime? = null ): User { - return User( - id = Id.newInstance(UuidGenerator.create()), - email = Email.newInstance(email), - nickname = nickname, - profileImageUrl = profileImageUrl, - providerType = providerType, - providerId = ProviderId.newInstance(providerId), - role = Role.USER, - createdAt = createdAt, - updatedAt = updatedAt, - deletedAt = deletedAt - ) + return createWithRole( + email = email, + nickname = nickname, + profileImageUrl = profileImageUrl, + providerType = providerType, + providerId = providerId, + role = Role.USER, + createdAt = createdAt, + updatedAt = updatedAt, + deletedAt = deletedAt + ) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (28)
apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserProfileResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/auth/service/UserAuthService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/book/Book.kt(4 hunks)domain/src/main/kotlin/org/yapp/domain/book/BookDomainService.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/book/BookRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/exception/BookAlreadyExistsException.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/exception/BookErrorCode.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/exception/BookNotFoundException.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/vo/BookInfoVO.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/user/User.kt(6 hunks)domain/src/main/kotlin/org/yapp/domain/user/vo/UserIdentityVO.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt(3 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/book/repository/JpaBookRepository.kt(0 hunks)infra/src/main/kotlin/org/yapp/infra/book/repository/impl/BookRepositoryImpl.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/external/redis/entity/RefreshToken.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt(3 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/entity/UserBookEntity.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt(1 hunks)
💤 Files with no reviewable changes (1)
- infra/src/main/kotlin/org/yapp/infra/book/repository/JpaBookRepository.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
apis/src/main/kotlin/org/yapp/apis/auth/service/UserAuthService.kt (1)
domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt (1)
createNewUser(44-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (58)
infra/src/main/kotlin/org/yapp/infra/external/redis/entity/RefreshToken.kt (3)
15-15: 생성자를 private로 변경한 것은 좋은 설계입니다.외부에서 직접 인스턴스를 생성할 수 없도록 하여 팩토리 메서드를 통한 객체 생성을 강제함으로써 캡슐화를 강화했습니다.
26-28: 도메인 모델 변환 시 인라인 value class 사용이 적절합니다.
newInstance팩토리 메서드를 사용하여 primitive 타입을 도메인 특화 value class로 변환하는 것은 타입 안전성과 유효성 검증을 강화하는 좋은 접근입니다.
36-38: 도메인 newInstance 팩토리 메서드 예외 처리 확인 완료
- RefreshToken.Id · UserId: non-nullable UUID를 받아 래핑하므로 별도 검증 불필요
- RefreshToken.Token:
require(value.isNotBlank())로 공백 검증 적용도메인 객체의 primitive 추출 및 예외 처리가 모두 적절히 구현되었습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt (2)
1-1: 패키지 구조 리팩토링이 올바르게 적용되었습니다.userbook 도메인을 별도 패키지로 분리하는 것은 도메인 경계를 명확히 하는 좋은 접근입니다.
4-4: import 경로 변경이 적절합니다.패키지 구조 변경에 맞춰 UserBookEntity의 import 경로가 올바르게 업데이트되었습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (2)
1-1: 패키지 구조 변경이 일관되게 적용되었습니다.repository 구현체의 패키지 경로가 인터페이스와 일관되게 업데이트되었습니다.
6-7: import 경로들이 새로운 패키지 구조에 맞게 올바르게 변경되었습니다.UserBookEntity와 JpaUserBookRepository의 import 경로가 적절히 업데이트되었습니다.
domain/src/main/kotlin/org/yapp/domain/book/exception/BookNotFoundException.kt (1)
1-8: 도메인 레벨 예외 처리가 올바르게 구현되었습니다.BookNotFoundException 클래스가 적절히 설계되었습니다:
- CommonException을 상속받아 일관된 예외 계층 구조를 유지
- BookErrorCode를 통한 명확한 에러 코드 관리
- 선택적 메시지 파라미터로 유연성 제공
이는 PR 목적에 언급된 도메인 서비스 레벨에서의 에러 핸들링 강화와 일치합니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/UserBookResponse.kt (3)
4-4: VO 타입 변경이 적절합니다.UserBookVO에서 UserBookInfoVO로의 변경은 목적별 VO 설계 원칙에 부합합니다.
22-22: 파라미터 타입 변경이 올바르게 적용되었습니다.from 메서드의 파라미터 타입이 UserBookInfoVO로 정확히 변경되었습니다.
25-27: 인라인 값 클래스 사용이 적절합니다.id, userId, bookIsbn 프로퍼티에
.value접근을 통해 인라인 값 클래스의 실제 값을 추출하는 것은 타입 안전성을 높이는 좋은 접근입니다.domain/src/main/kotlin/org/yapp/domain/book/exception/BookAlreadyExistsException.kt (1)
1-8: 도메인 예외 클래스가 일관되게 구현되었습니다.BookAlreadyExistsException 클래스가 적절히 설계되었습니다:
- BookNotFoundException과 동일한 패턴을 따라 일관성 유지
- CommonException 상속으로 예외 계층 구조 일관성 확보
- BookErrorCode를 통한 명확한 에러 분류
이는 도메인 레벨에서의 체계적인 예외 관리를 위한 좋은 접근입니다.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (2)
7-7: import 변경이 적절합니다.
UserBookVO에서UserBookInfoVO로의 변경이 PR 목표인 "목적 지향적인 VO 설계"와 일치합니다. 더 구체적이고 명확한 책임을 가진 VO 네이밍입니다.
28-29: 타입 변경이 일관되게 적용되었습니다.
UserBookInfoVO로의 타입 변경이 일관되게 적용되어 도메인 리팩토링의 의도를 잘 반영하고 있습니다.domain/src/main/kotlin/org/yapp/domain/user/vo/UserIdentityVO.kt (2)
6-9: 목적 지향적인 VO 설계가 잘 구현되었습니다.사용자 신원 정보만을 담당하는 명확한 책임을 가진 VO로, PR 목표인 "응답 DTO에 필요한 필드만 포함하는 목적 지향적인 VO"의 좋은 예시입니다.
11-16: 팩토리 메서드 패턴이 적절하게 적용되었습니다.도메인 엔티티에서 VO를 생성하는 팩토리 메서드가 깔끔하게 구현되어 도메인과 VO 간의 변환 로직이 명확합니다.
domain/src/main/kotlin/org/yapp/domain/book/vo/BookInfoVO.kt (2)
5-6: 강한 타입 적용이 도메인 설계를 개선했습니다.
isbn필드의 타입을String에서Book.Isbn으로 변경하여 도메인 레벨에서 ISBN 검증과 타입 안전성을 확보한 것이 좋습니다.
21-24: 팩토리 메서드가 새로운 타입 시스템에 맞게 업데이트되었습니다.
BookInfoVO를 반환하도록 메서드가 적절히 수정되어 일관성이 유지되고 있습니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookCreateResponse.kt (2)
3-3: import 변경이 도메인 리팩토링과 일치합니다.
BookInfoVO로의 변경이 도메인 레이어의 VO 네이밍 변경과 일관되게 적용되었습니다.
13-15: 인라인 값 클래스 처리가 올바르게 구현되었습니다.
bookVO.isbn.value를 통해Book.Isbn인라인 값 클래스에서 실제 값을 추출하는 로직이 정확하게 구현되어 도메인 모델의 변경사항을 잘 반영했습니다.infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt (2)
56-56: 엔티티-도메인 변환에서 값 객체 캡슐화가 올바르게 구현되었습니다.
Book.Isbn.newInstance(this.isbn)을 통해 문자열을 도메인 값 객체로 적절히 래핑하여 도메인 경계를 유지하고 있습니다.
70-70: 도메인-엔티티 변환에서 값 추출이 정확하게 처리되었습니다.
book.isbn.value를 통해 도메인 값 객체에서 실제 값을 추출하는 로직이 올바르게 구현되어 영속성 레이어와 도메인 레이어 간의 일관성을 보장합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserProfileResponse.kt (1)
39-46: 팩토리 메서드 패턴과 VO 사용이 잘 적용되었습니다.
of에서from으로 메서드명 변경과UserProfileVO를 사용하는 패턴이 도메인 분리와 가독성 향상에 도움이 됩니다. 도메인 값 객체를 통해 응답 DTO를 생성하는 방식이 일관성 있고 명확합니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/response/CreateUserResponse.kt (1)
12-32: 팩토리 메서드 패턴과 불변성 보장이 잘 구현되었습니다.private constructor와 companion object의
from메서드를 사용하여 객체 생성을 제어하고,UserIdentityVO를 통해 도메인 값 객체와 응답 DTO 간의 명확한 변환 로직을 제공합니다.infra/src/main/kotlin/org/yapp/infra/book/repository/impl/BookRepositoryImpl.kt (1)
14-20: 인터페이스 변경사항에 맞춰 구현이 잘 되어 있습니다.
findByIdOrNull을 사용하여 안전한 조회를 제공하고,existsById메서드도 JPA 레포지토리의 메서드를 적절히 위임하여 구현했습니다. 도메인 인터페이스와 일관성 있는 구현입니다.apis/src/main/kotlin/org/yapp/apis/auth/dto/response/UserAuthInfoResponse.kt (1)
12-33: 일관된 응답 DTO 패턴이 잘 적용되었습니다.
CreateUserResponse와 유사한 패턴을 사용하여 private constructor와 companion object의from메서드로 객체 생성을 제어하고,UserIdentityVO를 통해 도메인 값 객체와의 명확한 변환을 제공합니다. 인증 도메인 전반에 걸쳐 일관된 패턴이 적용되어 있습니다.domain/src/main/kotlin/org/yapp/domain/userbook/vo/UserBookInfoVO.kt (2)
7-10: 도메인 특화 타입 적용으로 타입 안전성 향상primitive type에서 domain-specific inline value class로 변경하여 타입 안전성과 도메인 무결성이 크게 향상되었습니다. 이는 DDD 원칙을 잘 적용한 개선사항입니다.
32-32: 클래스명 변경으로 목적성 명확화UserBookVO에서 UserBookInfoVO로 변경하여 해당 VO의 목적이 사용자 도서 정보 조회에 특화되어 있음을 명확히 표현했습니다.
domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt (1)
17-28: companion object factory method 구현이 적절함nullable User를 받고 requireNotNull로 검증하는 패턴이 적절하며, 도메인 객체에서 VO로의 변환 로직도 명확합니다.
domain/src/main/kotlin/org/yapp/domain/book/exception/BookErrorCode.kt (1)
6-17: 도메인별 에러 코드 enum 구현이 우수함BaseErrorCode 인터페이스를 구현하여 일관된 에러 처리 구조를 제공하고, 도메인별로 분리된 에러 코드 관리가 잘 되어 있습니다. HTTP 상태 코드와 메시지도 적절합니다.
infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt (3)
17-17: soft-delete 필터링 구현이 적절함@SQLRestriction을 통해 전역적으로 soft-delete된 레코드를 제외하는 구현이 적절합니다. 이는 데이터 무결성과 비즈니스 로직을 보장합니다.
55-65: domain-specific inline value class 적용으로 타입 안전성 향상Entity에서 Domain으로의 변환 시 inline value class를 사용하여 타입 안전성이 크게 향상되었습니다. newInstance() 패턴을 통해 validation도 함께 수행됩니다.
68-81: Domain에서 Entity로의 변환 로직 적절함inline value class의 .value 접근을 통해 primitive type을 추출하는 로직이 적절하며, 변환 과정에서 데이터 무결성이 보장됩니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookDomainService.kt (3)
3-3: import 변경으로 일관성 유지UserBookVO에서 UserBookInfoVO로 import 변경이 적절하며, 전체 코드베이스의 일관성을 유지합니다.
19-19: 메서드 리턴 타입 변경으로 명확성 향상UserBookVO에서 UserBookInfoVO로 리턴 타입을 변경하여 메서드의 목적이 더욱 명확해졌습니다.
Also applies to: 35-35
32-32: factory method 호출 변경이 적절함UserBookInfoVO.newInstance() 호출로 변경하여 새로운 VO 타입과 일관성을 유지하고 있습니다.
Also applies to: 37-37
domain/src/main/kotlin/org/yapp/domain/book/Book.kt (3)
9-9: ISBN 도메인 모델링 개선을 승인합니다primitive String 타입을 Isbn inline value class로 변경한 것은 타입 안전성과 도메인 무결성을 높이는 좋은 개선사항입니다.
32-32: 팩토리 메서드의 일관성 있는 업데이트를 승인합니다create 메서드에서 Isbn.newInstance를 사용하여 value object를 생성하는 것이 올바릅니다.
72-81: ISBN 검증 로직 개선을 승인합니다ISBN의 10자리 또는 13자리 숫자 검증 로직이 정확하게 구현되었습니다. 정규식 패턴이 올바르고 에러 메시지도 명확합니다.
domain/src/main/kotlin/org/yapp/domain/book/BookDomainService.kt (3)
13-16: 도메인 서비스의 예외 기반 에러 처리 개선을 승인합니다nullable 반환 대신 BookNotFoundException을 던지는 것은 더 명확한 에러 처리를 제공하고 호출자가 null 체크를 할 필요가 없어집니다.
27-29: 중복 생성 방지 로직 추가를 승인합니다save 메서드에 existsById 검사를 추가하여 중복 생성을 방지하는 것은 데이터 무결성을 보장하는 좋은 개선사항입니다.
14-14: BookRepository.findById 사용에 문제 없습니다BookRepository는 Spring Data JPA의 JpaRepository를 상속하고 있어 별도 선언 없이도 findById(id: String) 메서드를 제공합니다.
따라서 BookDomainService 내에서 findById 를 호출하도록 변경한 것은 의도한 대로 동작하며, 기존 findByIsbn 커스텀 메서드는 인터페이스에 남아있지 않습니다.domain/src/main/kotlin/org/yapp/domain/user/User.kt (3)
55-55: UuidGenerator 사용 개선을 승인합니다이전 리뷰 코멘트에서 제안된 UuidGenerator.create() 사용이 올바르게 반영되었습니다.
127-135: 이메일 검증 로직을 승인합니다이메일 형식 검증을 위한 정규식이 올바르게 구현되었고, 에러 메시지도 명확합니다.
138-145: ProviderId 검증 로직을 승인합니다ProviderId의 blank 검증 로직이 적절하게 구현되었습니다.
infra/src/main/kotlin/org/yapp/infra/userbook/entity/UserBookEntity.kt (3)
1-1: 패키지 구조 개선을 승인합니다패키지 선언이
org.yapp.infra.userbook.entity로 변경되어 더 명확한 구조를 제공합니다.
57-68: 도메인 매핑 로직의 inline value class 적용을 승인합니다toDomain() 메서드에서 primitive 타입을 inline value class로 래핑하는 로직이 올바르게 구현되었습니다.
73-75: 엔티티 매핑 로직의 value 추출을 승인합니다fromDomain() 메서드에서 inline value class의 .value 속성을 추출하는 로직이 올바르게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/auth/service/UserAuthService.kt (4)
4-12: DTO 기반 인터페이스 개선을 승인합니다API 레이어에서 DTO를 사용하여 도메인 객체와의 분리를 명확히 한 것은 좋은 아키텍처 개선사항입니다.
35-52: 사용자 조회/생성 로직 개선을 승인합니다findOrCreateUser 메서드에서 활성 사용자 조회, 삭제된 사용자 복원, 신규 사용자 생성의 순서로 처리하는 로직이 명확하고 적절합니다.
54-69: 사용자 생성 로직의 이메일 중복 검사를 승인합니다createNewUser 메서드에서 활성 사용자의 이메일 중복을 검사하는 로직이 올바르게 구현되었습니다.
30-33: FindUserIdentityRequest.validUserId() 검증 로직 일관성 확인
FindUserIdentityRequest.validUserId()는userId!!를 통해 null을 보장하는 간단한 Non-Null Assertion 패턴으로 구현되어 있습니다.- 이와 동일한 패턴이
TokenGenerateRequest,SocialLoginRequest등 다른 DTO에서도 사용되고 있어, 별도 추가 검증 로직 없이 유지해도 무방합니다.- (참고) NPE 발생 시 HTTP 400으로 매핑되는 예외 처리 정책이 프로젝트 전반에 일관되게 적용되는지만 한 번 더 확인해 주세요.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (6)
8-10: 강력한 타입 안전성을 위한 value class 도입이 훌륭합니다.원시 타입 대신 Id, UserId, BookIsbn과 같은 인라인 value class를 사용하는 것은 도메인 모델링에서 매우 좋은 접근입니다. 타입 안전성을 높이고 잘못된 매개변수 전달을 컴파일 타임에 방지할 수 있습니다.
36-38: 팩토리 메서드를 통한 value class 생성이 적절합니다.create 메서드에서 newInstance 팩토리 메서드를 사용하여 value class 인스턴스를 생성하는 것은 캡슐화 원칙을 잘 따르고 있습니다. 특히 BookIsbn의 경우 내부 검증 로직을 통과하게 됩니다.
50-52: reconstruct 메서드의 매개변수 타입 일관성이 좋습니다.reconstruct 메서드가 value class 타입을 직접 받도록 변경된 것은 일관성 있는 접근입니다. 이미 검증된 value class를 재구성할 때 사용하는 메서드의 특성상 적절한 선택입니다.
78-83: Id value class의 단순한 구현이 적절합니다.UUID를 감싸는 Id value class는 @JvmInline 어노테이션으로 성능 최적화를 하고 있고, 단순한 팩토리 메서드를 제공합니다. 도메인 식별자로서의 역할을 명확히 하는 좋은 설계입니다.
85-90: UserId value class의 구현이 일관성 있습니다.Id와 동일한 패턴을 따르는 UserId value class는 사용자 식별자의 타입 안전성을 보장합니다. 일관된 설계 패턴을 유지하고 있어 좋습니다.
92-100: BookIsbn의 검증 로직이 올바르게 구현되었습니다.ISBN 형식 검증을 위한 정규식
^(\\d{10}|\\d{13})$이 정확하게 10자리 또는 13자리 숫자를 검증하고 있습니다. 한국어 오류 메시지도 코드베이스의 언어 정책과 일치합니다.
| init { | ||
| require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."} | ||
| require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
User.Email 타입 검증 누락
User.Email이 inline value class로 변경되었으므로 이미 내부에서 검증이 수행되지만, 일관성을 위해 email 검증도 고려해보세요.
init {
+ require(email.value.isNotBlank()) { "email은 비어 있을 수 없습니다." }
require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."}
require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." }
}📝 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.
| init { | |
| require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."} | |
| require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." } | |
| } | |
| init { | |
| require(email.value.isNotBlank()) { "email은 비어 있을 수 없습니다." } | |
| require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."} | |
| require(provider.name.isNotBlank()) { "providerType은 비어 있을 수 없습니다." } | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt around lines
12 to 15, the init block validates nickname and provider name but lacks
validation for the email field. Since User.Email is now an inline value class
with its own validation, add an explicit require check for the email property
here to maintain consistency, ensuring the email is not blank or invalid
according to your domain rules.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindUserIdentityRequest.kt (1)
20-20: !! 연산자 사용 시 NPE 위험이 있습니다.
validUserId()메서드에서!!연산자를 사용하면 예상치 못한 NPE가 발생할 수 있습니다. 더 안전한 방법을 고려해보세요.다음과 같이 개선할 수 있습니다:
- fun validUserId(): UUID = userId!! + fun validUserId(): UUID = requireNotNull(userId) { "userId must not be null" }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindUserIdentityRequest.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (3)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindUserIdentityRequest.kt (3)
1-6: 패키지 선언과 임포트가 적절합니다.필요한 라이브러리들이 올바르게 임포트되었고, 패키지 구조가 인증 도메인 리팩토링 목적에 부합합니다.
8-11: Swagger 스키마 어노테이션이 잘 작성되었습니다.API 문서화 개선을 위한 스키마 어노테이션이 명확하게 작성되어 있어 협업 효율성이 향상될 것입니다.
22-26: 팩토리 메서드가 잘 구현되었습니다.
UserIdResponse로부터FindUserIdentityRequest를 생성하는 팩토리 메서드가 명확하고 간결하게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/FindUserIdentityRequest.kt
Show resolved
Hide resolved
minwoo1999
left a comment
minwoo1999
left a comment
There was a problem hiding this comment.
고생 많으셨습니다! 🙇♂️
이번 변경 범위가 꽤 넓다 보니, 전체적으로 API 통합 테스트가 필요해 보입니다.
시나리오별로 전체 API 호출 흐름을 따라가며 테스트를 한번 진행해보면 좋을 것 같습니다.
시간 되실 때 한 번 확인 부탁드리며,
언제든 필요하시면 말씀 주세요. 감사합니다 😊
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import jakarta.validation.constraints.NotNull | ||
| import org.yapp.apis.auth.dto.response.UserIdResponse | ||
| import java.util.* |
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import jakarta.validation.constraints.NotBlank | ||
| import jakarta.validation.constraints.NotNull | ||
| import java.util.* |
| import org.yapp.apis.auth.dto.response.UserIdResponse | ||
| import org.yapp.domain.token.TokenDomainRedisService | ||
| import org.yapp.domain.token.RefreshToken | ||
| import java.util.* |
| value class Isbn(val value: String) { | ||
| companion object { | ||
| fun newInstance(value: String): Isbn { | ||
| require(value.matches(Regex("^(\\d{10}|\\d{13})$"))) { "ISBN은 10자리 또는 13자리 숫자여야 합니다." } |
There was a problem hiding this comment.
global-utils에 있느 validator에 추가해서 사용해도 좋을듯요!
ISBN의 패턴은 일정하니 전역 global-util의 validator로 빼도 좋아 보입니다~
There was a problem hiding this comment.
| @JvmInline | ||
| value class Isbn(val value: String) { | ||
| companion object { |
There was a problem hiding this comment.
별도의 클래스로 빼지않고 내부에 정의하신 이유가 궁금합니다~
There was a problem hiding this comment.
간단하게 언급 드리자면, 도메인에 강하게 종속된 개념이기 때문에 inner class로 정의했습니다!
자세한 내용은 아래에서 적어놨습니다 ㅎㅎ
#42 (comment)
| value class BookIsbn(val value: String) { | ||
| companion object { | ||
| fun newInstance(value: String): BookIsbn { | ||
| require(value.matches(Regex("^(\\d{10}|\\d{13})$"))) { "ISBN은 10자리 또는 13자리 숫자여야 합니다." } |
There was a problem hiding this comment.
이부분도 global-utils로 빼도 좋을 것 같아요~
| import org.yapp.domain.userbook.BookStatus | ||
| import org.yapp.domain.userbook.UserBook | ||
| import java.sql.Types | ||
| import java.util.* |
| import org.springframework.data.jpa.repository.JpaRepository | ||
| import org.yapp.infra.user.entity.UserBookEntity | ||
| import org.yapp.infra.userbook.entity.UserBookEntity | ||
| import java.util.* |
| import org.yapp.infra.user.repository.JpaUserBookRepository | ||
| import org.yapp.infra.userbook.entity.UserBookEntity | ||
| import org.yapp.infra.userbook.repository.JpaUserBookRepository | ||
| import java.util.* |
| fun findUserProfileById(id: UUID): UserProfileVO { | ||
| val user = userRepository.findById(id) ?: throw UserNotFoundException(UserErrorCode.USER_NOT_FOUND) | ||
| return UserProfileVO.newInstance(user) | ||
| } | ||
|
|
||
| fun findById(id: UUID): User? = | ||
| userRepository.findById(id) | ||
| fun findUserIdentityById(id: UUID): UserIdentityVO { | ||
| val user = userRepository.findById(id) ?: throw UserNotFoundException(UserErrorCode.USER_NOT_FOUND) | ||
| return UserIdentityVO.newInstance(user) | ||
| } | ||
|
|
||
| fun findByEmail(email: String): User? = | ||
| userRepository.findByEmail(email) | ||
| fun findUserByProviderTypeAndProviderId(providerType: ProviderType, providerId: String): UserIdentityVO? { | ||
| return userRepository.findByProviderTypeAndProviderId(providerType, providerId) | ||
| ?.let { UserIdentityVO.newInstance(it) } | ||
| } | ||
|
|
||
| fun findByProviderTypeAndProviderId(providerType: ProviderType, providerId: String): User? = | ||
| userRepository.findByProviderTypeAndProviderId(providerType, providerId) | ||
| fun findUserByProviderTypeAndProviderIdIncludingDeleted(providerType: ProviderType, providerId: String): UserIdentityVO? { | ||
| return userRepository.findByProviderTypeAndProviderIdIncludingDeleted(providerType, providerId) | ||
| ?.let { UserIdentityVO.newInstance(it) } | ||
| } |
개인톡으로 말씀드리긴 했는데, 아카이빙 하면 좋을 것 같아서 남깁니다 ㅎㅎ 결국 VO를 생성하려면, 해당 도메인이 존재함이 보장된 상태에서 newInstance 메서드에 인자로 전달되어야 한다고 생각합니다. 그렇지 않으면 VO 내부에서는 해당 도메인이 반드시 존재한다는 전제 하에 getter를 호출하게 되는데, 실제로 객체가 존재하지 않으면 null이 전달되어 init 블록의 require 조건에서 IllegalArgumentException이 발생하게 됩니다. 이 경우, 의미 있는 예외인 UserNotFoundException으로 처리되지 않아, 에러 핸들링이 어려워지는 문제가 있었습니다. 따라서 도메인의 존재 여부나 무결성에 대한 책임은 도메인 서비스 레이어에서,
좋습니다!
IntelliJ에서 import문 관련 설정도 해봤는데, 여전히 같은 현상이 나타나네요 ㅠㅠ
UserId, Isbn, Email 등의 값 객체를 각각 해당 도메인 클래스의 내부 클래스로 선언했습니다. ISBN은 일반적으로 Book 도메인 외부에서 단독으로 사용될 일이 거의 없으며, Book과 함께 관리되고 사용되는 식별자입니다. 물론, Isbn이 향후 다른 도메인에서도 재사용될 가능성이 있다면 별도의 클래스로 분리하는 것이 더 바람직할 수 있습니다.
이거 너무 좋아요~ |
|
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
domain/src/main/kotlin/org/yapp/domain/book/Book.kt (1)
73-81: 도메인 값 객체 구현이 올바릅니다Isbn 인라인 값 클래스의 구현이 잘 되어 있습니다:
@JvmInline어노테이션으로 런타임 오버헤드 최소화newInstance팩토리 메서드로 검증 로직 캡슐화- 적절한 에러 메시지 제공
과거 리뷰 댓글에서 논의된 바와 같이, 도메인에 강하게 종속된 개념으로 inner class 설계가 적절합니다.
domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (1)
4-4: IsbnValidator 활용이 적절합니다UserBook 도메인에서도 동일한 ISBN 검증 로직을 재사용하여 일관성을 유지하고 있습니다.
과거 리뷰 댓글에서 언급된 바와 같이 global-utils로 추출된 것이 올바른 접근입니다.
domain/src/main/kotlin/org/yapp/domain/user/User.kt (1)
45-67: 팩토리 메서드 중복 코드 개선 필요이전 리뷰에서 지적된
create와createWithRole메서드 간의 코드 중복 문제가 여전히 남아있습니다.fun create( email: String, nickname: String, profileImageUrl: String?, providerType: ProviderType, providerId: String, createdAt: LocalDateTime, updatedAt: LocalDateTime, deletedAt: LocalDateTime? = null ): User { - return User( - id = Id.newInstance(UuidGenerator.create()), - email = Email.newInstance(email), - nickname = nickname, - profileImageUrl = profileImageUrl, - providerType = providerType, - providerId = ProviderId.newInstance(providerId), - role = Role.USER, - createdAt = createdAt, - updatedAt = updatedAt, - deletedAt = deletedAt - ) + return createWithRole( + email = email, + nickname = nickname, + profileImageUrl = profileImageUrl, + providerType = providerType, + providerId = providerId, + role = Role.USER, + createdAt = createdAt, + updatedAt = updatedAt, + deletedAt = deletedAt + ) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/Book.kt(4 hunks)domain/src/main/kotlin/org/yapp/domain/user/User.kt(6 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt(2 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (14)
global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt (1)
6-8: 유효성 검사 메서드 구현이 올바릅니다기본적인 ISBN 형식 검증 로직이 정확하게 구현되었습니다. 전역 유틸리티로 추출하여 재사용성을 높인 것도 좋은 설계입니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
28-30: 코드 포맷팅 개선이 적절합니다메서드 시그니처를 멀티라인으로 변경하여 가독성을 향상시켰습니다. 클래스 내 다른 메서드들과 일관성 있는 포맷팅을 유지하고 있습니다.
domain/src/main/kotlin/org/yapp/domain/book/Book.kt (3)
3-3: IsbnValidator 도입이 적절합니다전역 유틸리티에서 ISBN 유효성 검사 로직을 가져와 도메인 레벨에서 활용하는 것이 좋은 설계입니다. 검증 로직의 재사용성과 일관성을 보장합니다.
10-10: 타입 안전성 강화가 우수합니다기존 String 타입을 Isbn 인라인 값 클래스로 교체하여 타입 안전성을 크게 향상시켰습니다. 컴파일 타임에 타입 오류를 잡을 수 있게 되었습니다.
33-33: 팩토리 메서드 업데이트가 정확합니다
create메서드에서Isbn.newInstance(isbn)를 사용하여 문자열을 검증된 Isbn 객체로 변환하는 것이 올바른 접근입니다.domain/src/main/kotlin/org/yapp/domain/userbook/UserBook.kt (4)
9-11: 도메인 식별자 타입 안전성 강화
id,userId,bookIsbn프로퍼티를 각각 전용 인라인 값 클래스로 교체하여 타입 안전성을 크게 향상시켰습니다. 잘못된 식별자 타입 전달을 컴파일 타임에 방지할 수 있습니다.
37-39: 팩토리 메서드의 값 객체 생성이 정확합니다각 값 객체의
newInstance팩토리 메서드를 올바르게 활용하여 검증된 도메인 객체를 생성하고 있습니다.
94-101: BookIsbn 검증 로직이 우수합니다BookIsbn 인라인 값 클래스에서 IsbnValidator를 활용한 검증 로직이 잘 구현되어 있습니다:
- 팩토리 메서드에서 검증 수행
- 명확한 에러 메시지 제공
- 도메인 무결성 보장
79-91: 단순 래퍼 값 객체들이 적절합니다
Id와UserId인라인 값 클래스들이 간단하면서도 효과적으로 구현되어 있습니다. 별도의 검증 로직이 필요 없는 UUID 타입에 대해서는 단순 래퍼로 충분합니다.domain/src/main/kotlin/org/yapp/domain/user/User.kt (5)
4-5: 글로벌 유틸리티 통합 승인UuidGenerator와 EmailValidator의 통합이 잘 이루어졌습니다. 이전 리뷰 의견이 적절히 반영되어 통일된 방식으로 UUID 생성과 이메일 검증을 관리할 수 있게 되었습니다.
24-25: Value Object 도입 승인primitive 타입을 type-safe한 value object로 변경한 것이 좋습니다. 컴파일 타임에 타입 안정성을 보장하고 도메인 로직을 명확하게 표현할 수 있습니다.
Also applies to: 29-29
124-129: Id Value Object 구현 승인UUID를 래핑하는 Id value class의 구현이 깔끔합니다.
newInstance팩토리 메서드를 통해 일관된 생성 패턴을 유지하고 있습니다.
131-139: Email Value Object 검증 로직 승인EmailValidator와의 통합이 잘 이루어졌습니다. 이메일 형식 검증을 value object 생성 시점에 수행하여 도메인 무결성을 보장하고 있습니다.
95-119: Reconstruct 메서드 승인이미 검증된 value object들을 받아 User를 재구성하는 메서드가 잘 구현되어 있습니다. 데이터베이스에서 조회한 데이터를 도메인 객체로 변환할 때 유용할 것입니다.
| package org.yapp.globalutils.validator | ||
|
|
||
| object IsbnValidator { | ||
| private val ISBN_REGEX = Regex("^(\\d{10}|\\d{13})$") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
ISBN 유효성 검사 로직 개선 제안
현재 구현은 단순히 10자리 또는 13자리 숫자인지만 확인하지만, 실제 ISBN 유효성 검사에는 체크 디지트(check digit) 검증이 포함되어야 합니다.
더 정확한 ISBN 유효성 검사를 위해 다음과 같은 개선을 고려해보세요:
object IsbnValidator {
- private val ISBN_REGEX = Regex("^(\\d{10}|\\d{13})$")
+ private val ISBN_10_REGEX = Regex("^\\d{9}[\\dX]$")
+ private val ISBN_13_REGEX = Regex("^\\d{13}$")
fun isValidIsbn(isbn: String): Boolean {
- return isbn.matches(ISBN_REGEX)
+ val cleanIsbn = isbn.replace("-", "").replace(" ", "")
+ return when (cleanIsbn.length) {
+ 10 -> isValidIsbn10(cleanIsbn)
+ 13 -> isValidIsbn13(cleanIsbn)
+ else -> false
+ }
+ }
+
+ private fun isValidIsbn10(isbn: String): Boolean {
+ if (!isbn.matches(ISBN_10_REGEX)) return false
+ // ISBN-10 체크 디지트 검증 로직 추가
+ return true
+ }
+
+ private fun isValidIsbn13(isbn: String): Boolean {
+ if (!isbn.matches(ISBN_13_REGEX)) return false
+ // ISBN-13 체크 디지트 검증 로직 추가
+ return true
}
}📝 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.
| private val ISBN_REGEX = Regex("^(\\d{10}|\\d{13})$") | |
| object IsbnValidator { | |
| private val ISBN_10_REGEX = Regex("^\\d{9}[\\dX]$") | |
| private val ISBN_13_REGEX = Regex("^\\d{13}$") | |
| fun isValidIsbn(isbn: String): Boolean { | |
| val cleanIsbn = isbn.replace("-", "").replace(" ", "") | |
| return when (cleanIsbn.length) { | |
| 10 -> isValidIsbn10(cleanIsbn) | |
| 13 -> isValidIsbn13(cleanIsbn) | |
| else -> false | |
| } | |
| } | |
| private fun isValidIsbn10(isbn: String): Boolean { | |
| if (!isbn.matches(ISBN_10_REGEX)) return false | |
| // ISBN-10 체크 디지트 검증 로직 추가 | |
| return true | |
| } | |
| private fun isValidIsbn13(isbn: String): Boolean { | |
| if (!isbn.matches(ISBN_13_REGEX)) return false | |
| // ISBN-13 체크 디지트 검증 로직 추가 | |
| return true | |
| } | |
| } |
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt
at line 4, the current ISBN_REGEX only checks if the input is 10 or 13 digits
long without validating the check digit. To improve accuracy, implement a
function that calculates and verifies the ISBN check digit according to ISBN-10
and ISBN-13 standards, and update the validation logic to use this function
instead of just the regex.
| fun isValidEmail(email: String): Boolean { | ||
| return email.matches(EMAIL_REGEX) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
이메일 검증 로직 개선 제안
현재 구현은 기본적인 검증만 수행하고 있습니다. 더 robust한 검증을 위해 다음과 같은 개선을 고려해보세요:
fun isValidEmail(email: String): Boolean {
- return email.matches(EMAIL_REGEX)
+ return email.isNotBlank() &&
+ email.length <= 254 && // RFC 5321 제한
+ email.matches(EMAIL_REGEX) &&
+ !email.startsWith(".") &&
+ !email.endsWith(".") &&
+ !email.contains("..")
}또는 더 포괄적인 검증을 위해 Apache Commons Validator와 같은 검증된 라이브러리 사용을 고려해보세요.
📝 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 isValidEmail(email: String): Boolean { | |
| return email.matches(EMAIL_REGEX) | |
| } | |
| fun isValidEmail(email: String): Boolean { | |
| return email.isNotBlank() && | |
| email.length <= 254 && // RFC 5321 제한 | |
| email.matches(EMAIL_REGEX) && | |
| !email.startsWith(".") && | |
| !email.endsWith(".") && | |
| !email.contains("..") | |
| } |
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.kt
around lines 6 to 8, the current email validation uses a simple regex that may
not cover all valid email formats. Improve the validation by either enhancing
the regex pattern to cover more edge cases or replace the custom validation with
a well-tested library like Apache Commons Validator to ensure more robust and
comprehensive email format checking.
| package org.yapp.globalutils.validator | ||
|
|
||
| object EmailValidator { | ||
| private val EMAIL_REGEX = "^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$".toRegex() |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
이메일 정규식 패턴 검증 필요
현재 사용된 정규식 패턴이 RFC 5322 표준을 완전히 준수하는지 확인이 필요합니다. 특히 다음과 같은 케이스들을 고려해야 합니다:
- 국제화 도메인 이름 (IDN) 지원
- 더 복잡한 이메일 형식 (예: 따옴표로 감싸진 로컬 파트)
- 도메인 검증 로직
다음 스크립트로 이메일 정규식의 유효성을 검증해보세요:
🏁 Script executed:
#!/bin/bash
# 이메일 정규식 패턴 테스트를 위한 검증 스크립트
# 테스트 케이스들
test_emails=(
"[email protected]"
"[email protected]"
"[email protected]"
"[email protected]"
"test@localhost"
"invalid.email"
"test@"
"@example.com"
"[email protected]"
"한글@example.com"
)
# Kotlin 코드로 테스트 실행
cat > EmailValidatorTest.kt << 'EOF'
object EmailValidator {
private val EMAIL_REGEX = "^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$".toRegex()
fun isValidEmail(email: String): Boolean {
return email.matches(EMAIL_REGEX)
}
}
fun main() {
val testEmails = arrayOf(
"[email protected]",
"[email protected]",
"[email protected]",
"[email protected]",
"test@localhost",
"invalid.email",
"test@",
"@example.com",
"[email protected]",
"한글@example.com"
)
testEmails.forEach { email ->
println("$email: ${EmailValidator.isValidEmail(email)}")
}
}
EOF
# kotlinc로 컴파일하고 실행
if command -v kotlinc &> /dev/null; then
kotlinc EmailValidatorTest.kt -include-runtime -d EmailValidatorTest.jar
java -jar EmailValidatorTest.jar
else
echo "Kotlin compiler not available - manual testing recommended"
fiLength of output: 365
아래 Python3 스크립트로 동일한 정규식 패턴을 테스트해 주세요. sandbox에 kotlinc가 없어 Kotlin에서 테스트할 수 없으므로, Python을 이용해 결과를 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
python3 << 'EOF'
import re
# Kotlin EmailValidator와 동일한 정규식 패턴
pattern = re.compile(r"^[A-Za-z0-9_!#$%&'*+/=?`{|}~^.\-]+@[A-Za-z0-9.\-]+$")
# 테스트 케이스 리스트
test_emails = [
"[email protected]",
"[email protected]",
"[email protected]",
"[email protected]",
"test@localhost",
"invalid.email",
"test@",
"@example.com",
"[email protected]",
"한글@example.com"
]
# 실행 및 결과 출력
for email in test_emails:
print(f"{email}: {bool(pattern.match(email))}")
EOFLength of output: 249
이메일 정규식 패턴 수정 필요
현재 패턴은 연속된 점("..")을 허용하고, 도메인 검증이 약하며, 국제화(IDN)나 따옴표 로컬 파트를 지원하지 않습니다. 아래 항목을 검토·수정해주세요:
• 위치
global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.kt
– 4행private val EMAIL_REGEX = …
• 제안 수정 예시
- private val EMAIL_REGEX = "^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$".toRegex()
+ // 연속된 점 차단, 도메인에 최소 한 개의 점 요구, TLD 최소 2글자
+ private val EMAIL_REGEX = (
+ "^(?!.*\\.\\.)" + // 연속된 점 금지
+ "[A-Za-z0-9_!#$%&'*+/=?`{|}~^.-]+" +
+ "@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?" +
+ "(?:\\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)+$" // 도메인에 최소 1개 ‘.’ + TLD
+ ).toRegex()• 대안 라이브러리 사용 고려
- Apache Commons Validator
EmailValidator.getInstance(true).isValid(email) - JavaMail
InternetAddress
📝 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.
| private val EMAIL_REGEX = "^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$".toRegex() | |
| // 연속된 점 차단, 도메인에 최소 한 개의 점 요구, TLD 최소 2글자 | |
| private val EMAIL_REGEX = ( | |
| "^(?!.*\\.\\.)" + // 연속된 점 금지 | |
| "[A-Za-z0-9_!#$%&'*+/=?`{|}~^.-]+" + // 로컬 파트 | |
| "@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?" + // 도메인 레이블 | |
| "(?:\\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)+$" // 최소 1개 도메인 ‘.’ + TLD | |
| ).toRegex() |
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.kt
at line 4, the current email regex pattern is insufficient as it allows
consecutive dots, has weak domain validation, and lacks support for
internationalized domain names and quoted local parts. To fix this, replace the
regex with a more robust pattern that disallows consecutive dots, properly
validates domain parts, and supports IDN and quoted local parts, or
alternatively, replace the regex validation entirely by integrating a
well-tested library such as Apache Commons Validator's EmailValidator or
JavaMail's InternetAddress for email validation.
| @JvmInline | ||
| value class ProviderId(val value: String) { | ||
| companion object { | ||
| fun newInstance(value: String): ProviderId { | ||
| require(value.isNotBlank()) { "Provider ID는 필수입니다." } | ||
| return ProviderId(value) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
ProviderId 검증 로직 개선 제안
현재 단순히 blank 체크만 하고 있습니다. Provider별로 다른 검증 규칙이 필요할 수 있습니다.
@JvmInline
value class ProviderId(val value: String) {
companion object {
fun newInstance(value: String): ProviderId {
require(value.isNotBlank()) { "Provider ID는 필수입니다." }
+ require(value.length <= 255) { "Provider ID는 255자를 초과할 수 없습니다." }
return ProviderId(value)
}
}
}추후 특정 Provider에 대한 ID 형식 검증이 필요하다면 ProviderType과 함께 검증하는 방식도 고려해볼 수 있습니다.
📝 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.
| @JvmInline | |
| value class ProviderId(val value: String) { | |
| companion object { | |
| fun newInstance(value: String): ProviderId { | |
| require(value.isNotBlank()) { "Provider ID는 필수입니다." } | |
| return ProviderId(value) | |
| } | |
| } | |
| } | |
| @JvmInline | |
| value class ProviderId(val value: String) { | |
| companion object { | |
| fun newInstance(value: String): ProviderId { | |
| require(value.isNotBlank()) { "Provider ID는 필수입니다." } | |
| require(value.length <= 255) { "Provider ID는 255자를 초과할 수 없습니다." } | |
| return ProviderId(value) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/User.kt around lines 141 to 149,
the ProviderId validation currently only checks for blank values. To improve
this, enhance the validation logic to support different rules based on the
ProviderType by passing ProviderType as a parameter to the newInstance method
and implementing conditional validation for each provider's ID format. This will
allow flexible and extensible validation for various providers in the future.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
(추가)
변경 사항
변경 내용
Domain Layer:
Infrastructure Layer:
Application Layer & Presentation Layer:
코드예시
@Schema( name = "UserProfileResponse", description = "Response containing user profile information" ) data class UserProfileResponse( @Schema( description = "Unique identifier of the user", example = "c4d46ff7-9f1b-4c5f-8262-9fa2f982a7f4" ) val id: UUID, @Schema( description = "User email address", example = "[email protected]" ) val email: String, @Schema( description = "User nickname", example = "HappyPanda" ) val nickname: String, @Schema( description = "Social login provider type", example = "KAKAO" ) val provider: ProviderType ) { companion object { fun from(userProfileVO: UserProfileVO): UserProfileResponse { return UserProfileResponse( id = userProfileVO.id.value, // value로 실제 타입을 꺼냄 email = userProfileVO.email.value, // value로 실제 타입을 꺼냄 nickname = userProfileVO.nickname, provider = userProfileVO.provider ) } } }🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
도메인 서비스에 에러핸들링 도입
반대로 인증 실패, 권한 없음 등 AuthException은 애플리케이션 계층에서 처리하는 방식이 관심사의 분리를 명확하게 유지하면서도 책임 있는 설계라고 판단했습니다.
VO는 responseDTO의 필드만 갖도록 설계
누락된 스웨거 명세
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
변경 사항
버그 수정 및 개선
문서화