Issue:(#9, #10) cors 설정 변경 및 온보딩 구현 #11
Conversation
Walkthrough이번 변경 사항은 여러 구성 요소의 코드를 수정 및 개선한 PR입니다. Gradle 종속성 버전이 다운그레이드되고, 사용자 권한 및 식별 관련 메서드들이 업데이트되었으며, API 응답의 스키마 주석이 추가되었습니다. 또한, CORS 및 보안 설정에 새 엔드포인트와 인증 규칙이 도입되었고, JWT 토큰 생성 과정에 사용자 역할이 포함되도록 변경되었습니다. Swagger 관련 설정과 Unauthorized 예외 처리는 제거되었으며, 회원 정보 조회 및 온보딩 기능을 위한 새로운 Facade, 서비스, 컨트롤러, DTO, 테스트 클래스가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant OC as OnboardingController
participant OF as OnboardingFacade
participant OS as OnboardingService
participant MR as MemberRepository
Client->>OC: POST /v1/member/onboarding
OC->>OF: onboarding(customUserDetails, onboardingRequest)
OF->>OS: onboarding(id, onboardingRequest)
OS->>MR: findById(id)
MR-->>OS: Member 데이터 반환
OS-->>OF: 업데이트된 Member 정보
OF-->>OC: 처리 결과 반환
OC-->>Client: ApiResponse(Boolean)
sequenceDiagram
participant Client as 클라이언트
participant MIC as MemberInfoController
participant MIF as MemberInfoFacade
participant MIS as MemberInfoService
participant MR as MemberRepository
Client->>MIC: GET /v1/member/my-info
MIC->>MIF: getMemberInfo(customUserDetails)
MIF->>MIS: getGuestInfo(id)
MIS->>MR: findById(id)
MR-->>MIS: Member 데이터 반환
MIS-->>MIF: Member 정보 반환
MIF-->>MIC: GuestInfoResponse 생성
MIC-->>Client: ApiResponse(Member 정보)
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 11
🔭 Outside diff range comments (1)
src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt (1)
62-70: 🛠️ Refactor suggestionUserDTO와 Member 클래스 간의 속성명 불일치
UserDTO는 여전히
name속성을 사용하는 반면, Member 클래스는 이제nickname을 사용합니다. 이로 인해 혼란이 생길 수 있습니다.val userDto = UserDTO( username = oAuth2Response.getProviderId(), - name = oAuth2Response.getName(), + nickname = oAuth2Response.getName(), email = oAuth2Response.getEmail(), profileImage = oAuth2Response.getProfileImage(), role = Role.MEMBER.name, registrationId = registrationId, userId = savedMember.id, )UserDTO 클래스에서도
name을nickname으로 변경하는 것이 일관성을 유지하는 데 도움이 될 것입니다.
🧹 Nitpick comments (9)
src/main/kotlin/gomushin/backend/member/domain/value/Role.kt (1)
3-6: KDoc 문서화 추가를 고려해보세요.각 역할(GUEST, MEMBER)의 목적과 권한을 설명하는 KDoc 문서를 추가하면 다른 개발자들이 코드를 더 쉽게 이해할 수 있을 것입니다.
enum class Role { + /** + * 온보딩을 완료하지 않은 초기 사용자 역할 + */ GUEST, + /** + * 온보딩을 완료하고 모든 기능을 사용할 수 있는 역할 + */ MEMBER, }src/main/kotlin/gomushin/backend/core/oauth/CustomOAuth2User.kt (1)
31-33: Role enum 타입 활용 검토가 필요합니다.현재 getRole() 메서드는 String 타입을 반환하고 있는데, 새로 추가된 Role enum을 활용하여 타입 안전성을 높이는 것을 고려해보세요. 이렇게 하면 컴파일 시점에서 타입 오류를 발견할 수 있습니다.
- fun getRole() : String { - return userDto.role - } + fun getRole() : Role { + return Role.valueOf(userDto.role) + }또는 UserDTO 클래스에서 role 필드의 타입을 String에서 Role로 변경하는 것도 고려해볼 수 있습니다.
src/main/kotlin/gomushin/backend/member/application/OnboardingFacade.kt (1)
7-13: 온보딩 기능을 위한 파사드 패턴 적용온보딩 기능을 위한 파사드 클래스가 적절히 구현되었습니다. 파사드 패턴을 통해 온보딩 관련 복잡한 로직을 캡슐화하고 컨트롤러에 단순한 인터페이스를 제공합니다.
한 가지 제안: 파사드 메서드가 단순히 서비스 메서드를 호출하는 것만으로는 파사드의 이점이 제한될 수 있습니다. 필요한 경우 트랜잭션 관리나 추가 비즈니스 로직을 포함하는 것을 고려해 보세요.
src/main/kotlin/gomushin/backend/member/domain/service/MemberInfoService.kt (1)
1-18: 회원 정보 조회 서비스 구현회원 정보를 조회하는 서비스 클래스가 적절하게 구현되었습니다. Spring 표준 아키텍처 패턴을 따르고 있으며, 데이터 조회는 읽기 전용 트랜잭션으로 표시되어 있습니다.
한 가지 주의할 점은 예외 메시지 "sarangggun.member.not-exist-member"에 오타가 있는 것 같습니다('g'가 한 글자 더 들어감). 메시지 키를 확인하고 올바르게 수정하는 것이 좋겠습니다.
- fun getGuestInfo(id: Long): Member = - memberRepository.findById(id).orElseThrow { BadRequestException("sarangggun.member.not-exist-member") } + fun getGuestInfo(id: Long): Member = + memberRepository.findById(id).orElseThrow { BadRequestException("saranggun.member.not-exist-member") }src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt (1)
1-26: 회원 정보 조회 컨트롤러 구현회원 정보를 조회하는 컨트롤러가 적절하게 구현되었습니다. REST 패턴을 준수하고 있으며, Swagger 문서화를 위한 어노테이션도 잘 적용되어 있습니다. 현재 인증된 사용자의 정보를 가져오기 위해
@AuthenticationPrincipal을 사용하는 것도 좋은 방법입니다.반환 타입이
ApiResponse<Any>로 설정되어 있는데, 타입 안전성을 높이기 위해 실제 반환되는 객체 타입으로 지정하는 것이 좋을 것 같습니다.- fun get( - @AuthenticationPrincipal customUserDetails: CustomUserDetails - ): ApiResponse<Any> { + fun get( + @AuthenticationPrincipal customUserDetails: CustomUserDetails + ): ApiResponse<GuestInfoResponse> {src/test/kotlin/gomushin/backend/member/application/MemberInfoFacadeTest.kt (2)
52-62: GUEST 사용자에 대한 정보 조회가 적절히 테스트되고 있습니다.회원 정보 조회 테스트가 잘 구현되어 있습니다. 하지만 현재의 테스트는 GuestInfoResponse로의 변환과 닉네임 검증만 수행하고 있습니다.
개선 가능한 부분:
- 61번 라인에서 result를 GuestInfoResponse로 캐스팅할 때, 타입 검사를 먼저 수행하는 것이 더 안전할 것 같습니다.
- 닉네임뿐만 아니라 응답의 다른 속성들도 검증하면 좋을 것 같습니다.
// then verify(memberInfoService).getGuestInfo(1L) -assertEquals(member.nickname, (result as GuestInfoResponse).nickname) +val response = result as? GuestInfoResponse +requireNotNull(response) { "Response should be of type GuestInfoResponse" } +assertEquals(member.nickname, response.nickname) +// 다른 필드들에 대한 검증도 추가
32-50: setUp 메서드가 효율적으로 구성되어 있습니다.테스트 전 초기화를 위한 @beforeeach 메서드가 잘 구현되어 있습니다. 하지만 mock 객체 설정 방식에 일관성이 부족합니다.
개선 사항:
- customUserDetails를 mock()으로 생성하는 대신, @mock 어노테이션을 사용하면 코드가 더 일관성 있어 보일 것 같습니다.
@ExtendWith(MockitoExtension::class) class MemberInfoFacadeTest { @Mock private lateinit var memberInfoService: MemberInfoService + @Mock + private lateinit var customUserDetails: CustomUserDetails @InjectMocks private lateinit var memberInfoFacade: MemberInfoFacade - private lateinit var customUserDetails: CustomUserDetails private lateinit var member: Member @BeforeEach fun setUp() { member = Member( id = 1L, nickname = "테스트 닉네임", email = "[email protected]", birthDate = null, profileImageUrl = null, provider = Provider.KAKAO, role = Role.GUEST, ) val authorities = mutableListOf<GrantedAuthority>(SimpleGrantedAuthority("ROLE_GUEST")) - customUserDetails = mock(CustomUserDetails::class.java) `when`(customUserDetails.getId()).thenReturn(1L) `when`(customUserDetails.authorities).thenReturn(authorities) }src/main/kotlin/gomushin/backend/member/domain/service/OnboardingService.kt (1)
16-17: 예외 처리 개선 필요현재 구현에서는 회원이 존재하지 않을 경우 BadRequestException을 발생시키고 있습니다. 하지만 이는 클라이언트 오류보다는 서버 측 리소스를 찾을 수 없는 상황이므로 NotFoundException이 더 적절할 수 있습니다.
-val member = memberRepository.findById(id).orElseThrow { BadRequestException("sarangggun.member.not-exist-member") } +val member = memberRepository.findById(id).orElseThrow { NotFoundException("sarangggun.member.not-exist-member") }참고: NotFoundException 클래스가 프로젝트에 정의되어 있는지 확인하고, 없다면 추가해야 합니다.
src/main/kotlin/gomushin/backend/core/jwt/infrastructure/JwtTokenProviderImpl.kt (1)
25-26: JWT 토큰에 역할 정보가 추가되었습니다. 유효성 검사를 고려하세요.사용자 역할을 JWT 토큰에 포함시키는 것은 좋은 변경 사항입니다. 이를 통해 권한 검사가 더 효율적으로 이루어질 수 있습니다.
다음과 같이 역할 매개변수에 대한 유효성 검사를 추가하는 것이 좋습니다:
override fun provideAccessToken(userId: Long, role: String): String { + require(role.isNotBlank()) { "역할은 비어 있을 수 없습니다." } return createToken(userId, role, ACCESS_TOKEN_EXPIRATION, Type.ACCESS) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
build.gradle.kts(1 hunks)src/main/kotlin/gomushin/backend/core/CustomUserDetails.kt(1 hunks)src/main/kotlin/gomushin/backend/core/common/web/response/ApiResponse.kt(2 hunks)src/main/kotlin/gomushin/backend/core/configuration/security/CustomCorsConfiguration.kt(1 hunks)src/main/kotlin/gomushin/backend/core/configuration/security/SecurityConfiguration.kt(3 hunks)src/main/kotlin/gomushin/backend/core/configuration/swagger/SwaggerConfiguration.kt(0 hunks)src/main/kotlin/gomushin/backend/core/infrastructure/exception/UnauthorizedException.kt(0 hunks)src/main/kotlin/gomushin/backend/core/infrastructure/filter/JwtAuthenticationFilter.kt(2 hunks)src/main/kotlin/gomushin/backend/core/jwt/JwtTokenProvider.kt(1 hunks)src/main/kotlin/gomushin/backend/core/jwt/infrastructure/JwtTokenProviderImpl.kt(3 hunks)src/main/kotlin/gomushin/backend/core/oauth/CustomOAuth2User.kt(1 hunks)src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt(1 hunks)src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt(4 hunks)src/main/kotlin/gomushin/backend/core/service/CustomUserDetailsService.kt(1 hunks)src/main/kotlin/gomushin/backend/member/application/MemberInfoFacade.kt(1 hunks)src/main/kotlin/gomushin/backend/member/application/OnboardingFacade.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(2 hunks)src/main/kotlin/gomushin/backend/member/domain/repository/MemberRepository.kt(0 hunks)src/main/kotlin/gomushin/backend/member/domain/service/MemberInfoService.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/service/OnboardingService.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/value/Role.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/OnboardingController.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/dto/request/OnboardingRequest.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/dto/response/GuestInfoResponse.kt(1 hunks)src/test/kotlin/gomushin/backend/member/application/MemberInfoFacadeTest.kt(1 hunks)src/test/kotlin/gomushin/backend/member/application/OnboardingFacadeTest.kt(1 hunks)src/test/kotlin/gomushin/backend/member/domain/service/MemberInfoServiceTest.kt(1 hunks)src/test/kotlin/gomushin/backend/member/domain/service/OnboardingServiceTest.kt(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/kotlin/gomushin/backend/member/domain/repository/MemberRepository.kt
- src/main/kotlin/gomushin/backend/core/infrastructure/exception/UnauthorizedException.kt
- src/main/kotlin/gomushin/backend/core/configuration/swagger/SwaggerConfiguration.kt
🔇 Additional comments (30)
src/main/kotlin/gomushin/backend/member/domain/value/Role.kt (1)
3-6: Enum 클래스 구현이 적절합니다.이 Role enum 클래스는 GUEST와 MEMBER 두 가지 역할을 명확하게 정의하고 있어 적절합니다. PR 설명에 언급된 것처럼 사용자가 처음 등록할 때는 GUEST 역할이 되고, 온보딩 과정을 완료하면 MEMBER 역할로 전환된다는 비즈니스 로직을 잘 표현하고 있습니다.
src/main/kotlin/gomushin/backend/core/oauth/CustomOAuth2User.kt (1)
31-33: OAuth2User에 역할 정보 제공 기능이 추가되었습니다.새로 추가된 getRole() 메서드를 통해 사용자의 역할 정보를 문자열로 반환하도록 구현되었습니다. 이는 JWT 토큰 생성 시 사용자 역할 정보를 포함하고, Spring Security의 인증/인가 처리에 활용될 것으로 보입니다.
build.gradle.kts (1)
37-37:❓ Verification inconclusive
SpringDoc OpenAPI 의존성 버전이 다운그레이드되었습니다.
springdoc-openapi-starter-webmvc-ui 의존성이 2.8.1에서 2.7.0으로 다운그레이드되었습니다. 이는 의도적인 결정인지 확인이 필요합니다.
혹시 이 버전 다운그레이드가 필요한 이유가 있는지요? 일반적으로 의존성 버전은 가능한 최신 버전을 사용하는 것이 보안 및 기능적인 측면에서 좋습니다. 다운그레이드의 이유가 특정 호환성 문제 해결이라면 주석으로 이유를 남겨두는 것이 좋습니다.
🏁 Script executed:
#!/bin/bash # SpringDoc OpenAPI 버전 관련 정보 확인 echo "현재 선택된 버전: 2.7.0" echo "이전 버전: 2.8.1" # 두 버전의 Maven Central 정보 확인 echo -e "\n2.7.0 버전 정보:" curl -s "https://search.maven.org/solrsearch/select?q=g:org.springdoc+AND+a:springdoc-openapi-starter-webmvc-ui+AND+v:2.7.0&wt=json" | jq '.response.docs[0]' echo -e "\n2.8.1 버전 정보:" curl -s "https://search.maven.org/solrsearch/select?q=g:org.springdoc+AND+a:springdoc-openapi-starter-webmvc-ui+AND+v:2.8.1&wt=json" | jq '.response.docs[0]' # 해당 패키지의 최신 버전 확인 echo -e "\n최신 버전 정보:" curl -s "https://search.maven.org/solrsearch/select?q=g:org.springdoc+AND+a:springdoc-openapi-starter-webmvc-ui&rows=1&wt=json" | jq '.response.docs[0]'Length of output: 1836
확인 요청: SpringDoc OpenAPI 의존성 버전 다운그레이드에 대한 의도 검증
- 현재
build.gradle.kts파일 37번째 줄에서springdoc-openapi-starter-webmvc-ui의존성이 2.8.1에서 2.7.0으로 다운그레이드되어 적용되었습니다.- Maven Central의 정보에 따르면, 2.7.0과 2.8.1 버전 모두 유효하며, 최신 버전은 2.8.6 입니다.
- 해당 다운그레이드가 특정 호환성 이슈를 해결하기 위한 의도적인 결정인지, 아니면 실수로 인한 변경인지 확인 부탁드립니다. 만약 의도한 변경이라면 관련 이유(예: 호환성 문제 등)를 주석으로 남겨두시길 권장합니다.
src/main/kotlin/gomushin/backend/member/presentation/dto/request/OnboardingRequest.kt (1)
6-12: 온보딩 요청 DTO가 적절하게 정의되었습니다.닉네임과 생일 정보를 받는 OnboardingRequest DTO가 잘 정의되어 있습니다. Swagger 문서화를 위한 @Schema 어노테이션도 적절히 사용되었습니다.
src/main/kotlin/gomushin/backend/core/jwt/JwtTokenProvider.kt (1)
4-4: 인터페이스 변경이 잘 되었습니다.토큰 생성 시 사용자 역할(role)을 포함하도록
provideAccessToken메서드 시그니처가 적절히 수정되었습니다. 이는 PR 목표인 온보딩 과정에서의 역할 관리를 위한 필수적인 변경사항입니다.src/main/kotlin/gomushin/backend/core/infrastructure/filter/JwtAuthenticationFilter.kt (1)
45-45: 포맷팅이 개선되었습니다.코드 가독성을 위한 빈 줄 추가는 좋은 변경입니다.
src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt (1)
3-6: API 경로 상수화 구현이 좋습니다.API 경로를 상수로 정의한 것은 유지보수성과 일관성을 높이는 좋은 방법입니다. 향후 더 많은 API 경로가 추가될 때도 이 패턴을 유지하면 좋겠습니다.
온보딩 엔드포인트(
ONBOARDING)와 회원 정보 조회 엔드포인트(MY_INFO)가 명확하게 정의되어 있습니다. 이는 PR 목표인 온보딩 구현을 위한 좋은 기반입니다.src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt (3)
37-37: 토큰 생성 시 역할 정보 추가사용자의 역할 정보를 토큰에 포함시키는 변경사항입니다. 이는 PR의 목적인 온보딩 프로세스와 역할 관리(GUEST → MEMBER)를 지원하기 위한 중요한 변경입니다.
39-39: 미등록 사용자의 토큰에도 역할 정보 추가사용자가 시스템에 등록되지 않은 경우에도 OAuth2User에서 가져온 역할 정보를 토큰에 포함시키는 것으로 변경되었습니다. 이는 일관된 인증 흐름을 제공합니다.
43-43: 리다이렉트 URL 변경개발 환경(localhost)에서 프로덕션 환경(Vercel)으로 리다이렉트 URL이 변경되었습니다. 프로덕션 환경에 배포하기 위한 준비 단계로 보입니다.
src/main/kotlin/gomushin/backend/member/presentation/dto/response/GuestInfoResponse.kt (1)
5-11: 게스트 정보 응답 DTO 구현게스트 사용자의 정보를 반환하기 위한 응답 DTO가 적절히 구현되었습니다. 팩토리 메서드 패턴을 사용하여 Member 엔티티에서 간편하게 변환할 수 있는
of메서드를 제공하는 것이 좋습니다.다만, 현재는 닉네임만 포함하고 있는데, 향후 게스트에 대한 추가 정보가 필요한 경우 확장이 용이합니다.
src/main/kotlin/gomushin/backend/core/common/web/response/ApiResponse.kt (1)
8-8: API 문서에서 오류 필드 숨김 처리Swagger/OpenAPI 문서에서 오류 필드를 숨기도록
@Schema(hidden = true)주석을 추가했습니다. 이는 API 문서를 더 깔끔하게 유지하고 클라이언트에게 필요한 정보만 노출하는 좋은 관행입니다.이 변경은 API 응답 형식을 변경하지 않고 문서화에만 영향을 미칩니다.
Also applies to: 24-24
src/main/kotlin/gomushin/backend/core/configuration/security/SecurityConfiguration.kt (3)
5-5: 이전에 주석 처리되었던 CustomSuccessHandler 가져오기 추가주석 처리되었던 CustomSuccessHandler 가져오기가 활성화되었습니다. 이는 OAuth2 로그인 성공 시 JWT 토큰 발급과 관련된 핸들러로 보이며, 53번 라인에서 사용되고 있습니다.
21-21: 매개변수 뒤에 쉼표 추가매개변수 목록의 마지막 항목 뒤에 쉼표를 추가했습니다. 이는 Kotlin 스타일 가이드를 따르는 것으로, 향후 추가 매개변수를 추가할 때 코드 변경을 최소화하는 좋은 습관입니다.
70-71: 권한 관리 규칙 추가온보딩 엔드포인트(
/v1/member/onboarding)에 대한 접근 권한을 "GUEST" 역할로 제한하고, 기타 모든 요청에 인증을 요구하는 규칙을 추가했습니다. 이는 PR의 목표(GUEST 역할을 가진 사용자만 온보딩 API에 접근할 수 있도록 함)와 일치합니다.src/test/kotlin/gomushin/backend/member/application/OnboardingFacadeTest.kt (1)
1-35: 테스트 코드가 기본적인 요구사항을 충족합니다.이 테스트는 OnboardingFacade가 OnboardingService에 요청을 올바르게 위임하는지 확인하고 있습니다. 기본적인 테스트 구조와, given-when-then 패턴을 잘 따르고 있습니다.
다만, 다음과 같은 개선 사항을 고려해 볼 수 있습니다:
- 예외 상황에 대한 테스트 케이스가 없습니다. 오류 시나리오도 테스트하면 좋을 것 같습니다.
- 만약 Facade에서 데이터 변환 로직이 있다면, 반환값에 대한 검증도 추가하면 좋을 것 같습니다.
src/test/kotlin/gomushin/backend/member/domain/service/MemberInfoServiceTest.kt (1)
1-48: 테스트 구조가 잘 갖추어져 있습니다.MemberInfoService의 getGuestInfo 메서드에 대한 기본적인 테스트가 잘 작성되어 있습니다. Mockito를 활용한 의존성 주입과 테스트 구조가 적절합니다.
개선 가능한 부분:
- 현재는 성공 케이스만 테스트하고 있는데, 회원을 찾을 수 없는 경우와 같은 예외 상황에 대한 테스트도 추가하면 좋을 것 같습니다.
- 회원의 역할(Role)이 GUEST가 아닌 경우에 대한 동작 검증도 필요할 수 있습니다.
src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt (3)
10-10: Role 열거형 도입을 통한 역할 관리 개선Role 열거형을 사용하여 역할 관리를 개선한 것이 좋습니다. 이는 하드코딩된 문자열 대신 타입 안전성을 제공합니다.
36-36: Member 클래스의 속성명 변경 반영기존
name속성에서nickname으로의 변경이 일관되게 적용되었습니다.
46-46: 역할 지정 개선하드코딩된 "ROLE_USER" 대신
Role.MEMBER.name을 사용하여 일관성과 유지보수성이 향상되었습니다.src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (5)
16-17: 속성명 변경이 적절히 수행됨
name에서nickname으로의 속성명 변경이 잘 수행되었습니다. 이는 사용자 식별에 더 적합한 용어입니다.
19-20: email 필드에 non-null 제약 조건 추가이메일 필드에
nullable = false제약 조건을 추가한 것은 데이터 무결성을 위해 좋은 변경입니다.
22-24: birthDate 필드 추가사용자의 생년월일을 저장하기 위한 필드가 추가되었습니다. 이는 온보딩 과정에서 수집되는 정보로 적절합니다.
32-34: 역할 관리를 위한 필드 추가Role 열거형을 사용하여 사용자 역할을 관리하는 것은 좋은 접근 방식입니다. 또한 기본값으로
Role.GUEST를 설정한 것은 PR 설명에 부합합니다.
16-50: 커플 정보 필드 부재PR 설명에 따르면 "유저 커플에 연결되지 않은 경우 GUEST 역할을 가진다"고 되어 있습니다. 그러나 현재 Member 엔티티에는 커플 정보를 저장하는 필드가 보이지 않습니다.
커플 정보와 관련된 필드나 관계가 다른 파일에 정의되어 있는지 확인해 보세요. 만약 없다면, 다음과 같은 관계를 추가하는 것이 좋을 수 있습니다:
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "couple_id") var couple: Couple? = null또는:
@Column(name = "couple_id") var coupleId: Long? = null이를 통해 사용자가 커플에 연결되어 있는지 여부를 추적할 수 있습니다.
src/main/kotlin/gomushin/backend/core/CustomUserDetails.kt (3)
11-13: 역할 기반 권한 부여 개선역할에 기반한 권한 부여 메커니즘이 잘 구현되었습니다.
SimpleGrantedAuthority를 사용하여 멤버의 역할을 기반으로 권한을 생성하는 것은 Spring Security의 모범 사례입니다.
19-21: 사용자 식별자 필드 업데이트Member 엔티티에서
name이nickname으로 변경됨에 따라getUsername()메서드도 적절히 업데이트되었습니다.
12-12:✅ Verification successful
프리픽스 중복 가능성 확인 필요
"ROLE_${member.role.name}"의 구현에서, Role 열거형의 이름에 이미 "ROLE_" 프리픽스가 포함되어 있는지 확인이 필요합니다. 그렇지 않으면 괜찮지만, 만약 이미 포함되어 있다면 "ROLE_ROLE_MEMBER"와 같은 중복이 발생할 수 있습니다.
🏁 Script executed:
#!/bin/bash # Role 열거형에 이미 "ROLE_" 프리픽스가 포함되어 있는지 확인 rg -l "enum\s+class\s+Role" | xargs cat | grep -A 10 "enum class Role"Length of output: 115
검증 결과: 접두어 중복 문제 없음
역할(Role) 열거형의 값이 "GUEST"와 "MEMBER"처럼 접두어 없이 정의되어 있으므로,
CustomUserDetails.kt의 다음 코드return mutableListOf(SimpleGrantedAuthority("ROLE_${member.role.name}"))는 올바른 구현입니다. "ROLE_" 접두어가 enum 값에 포함되어 있지 않으므로 중복 문제가 발생하지 않습니다.
src/main/kotlin/gomushin/backend/core/jwt/infrastructure/JwtTokenProviderImpl.kt (1)
59-59: 역할에 대한 클레임이 추가되었습니다.JWT 토큰에 역할 클레임을 추가한 것은 권한 관리에 있어 좋은 접근 방식입니다.
src/main/kotlin/gomushin/backend/core/service/CustomUserDetailsService.kt (1)
18-21: 사용자 ID 기반 조회 메서드가 더 간결해졌습니다.
createUserDetail메서드 제거로 코드가 더 간결해졌고,CustomUserDetails를 직접 생성하는 방식으로 변경된 것은 좋은 리팩토링입니다.다만, 여기서도 같은 예외 메시지를 사용하고 있어 이전 코멘트에서 언급한 것과 같은 보안 고려사항이 적용됩니다.
src/main/kotlin/gomushin/backend/member/presentation/dto/request/OnboardingRequest.kt
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/core/configuration/security/CustomCorsConfiguration.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/core/infrastructure/filter/JwtAuthenticationFilter.kt
Outdated
Show resolved
Hide resolved
| package gomushin.backend.member.domain.service | ||
|
|
||
| import gomushin.backend.member.domain.entity.Member | ||
| import gomushin.backend.member.domain.repository.MemberRepository | ||
| import gomushin.backend.member.domain.value.Provider | ||
| import gomushin.backend.member.domain.value.Role | ||
| import gomushin.backend.member.presentation.dto.request.OnboardingRequest | ||
| import org.junit.jupiter.api.Assertions.assertEquals | ||
| import org.junit.jupiter.api.extension.ExtendWith | ||
| import org.mockito.Mock | ||
| import org.mockito.Mockito.verify | ||
| import org.mockito.Mockito.`when` | ||
| import org.mockito.junit.jupiter.MockitoExtension | ||
| import java.time.LocalDate | ||
| import java.util.* | ||
| import kotlin.test.Test | ||
|
|
||
| @ExtendWith(MockitoExtension::class) | ||
| class OnboardingServiceTest { | ||
| @Mock | ||
| private lateinit var memberRepository: MemberRepository | ||
|
|
||
| private val onboardingService: OnboardingService by lazy { | ||
| OnboardingService(memberRepository) | ||
| } | ||
|
|
||
| @Test | ||
| fun `onboarding 성공 케이스`() { | ||
| // given | ||
| val memberId = 1L | ||
| val existingMember = Member( | ||
| id = memberId, | ||
| nickname = "원래 닉네임", | ||
| email = "[email protected]", | ||
| birthDate = LocalDate.of(1990, 1, 1), | ||
| profileImageUrl = null, | ||
| provider = Provider.KAKAO, | ||
| role = Role.GUEST, | ||
| ) | ||
|
|
||
| val onboardingRequest = OnboardingRequest( | ||
| nickname = "새로운 닉네임", | ||
| birthDate = LocalDate.of(2000, 1, 1) | ||
| ) | ||
|
|
||
| `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(existingMember)) | ||
|
|
||
| // when | ||
| onboardingService.onboarding(memberId, onboardingRequest) | ||
|
|
||
| // then | ||
| assertEquals("새로운 닉네임", existingMember.nickname) | ||
| assertEquals(LocalDate.of(2000, 1, 1), existingMember.birthDate) | ||
| assertEquals(Role.MEMBER, existingMember.role) | ||
|
|
||
| verify(memberRepository).findById(memberId) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
온보딩 기능의 핵심 동작이 테스트되고 있습니다.
온보딩 서비스의 동작을 검증하는 테스트가 적절히 작성되어 있으며, 회원의 닉네임, 생년월일 업데이트 및 역할 변경(GUEST → MEMBER)을 확인하고 있습니다.
다음과 같은 개선 사항을 고려해 보세요:
- findById 호출 여부는 검증하고 있지만, 변경된 회원 정보를 저장하는 memberRepository.save() 메서드 호출 여부는 검증되지 않았습니다. 이 부분도 verify로 확인하면 좋을 것 같습니다.
- 회원을 찾을 수 없는 경우나, 이미 GUEST가 아닌 역할을 가진 회원에 대한 예외 케이스도 테스트하면 더 완성도 높은 테스트가 될 것 같습니다.
// then
assertEquals("새로운 닉네임", existingMember.nickname)
assertEquals(LocalDate.of(2000, 1, 1), existingMember.birthDate)
assertEquals(Role.MEMBER, existingMember.role)
verify(memberRepository).findById(memberId)
+verify(memberRepository).save(existingMember)📝 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.
| package gomushin.backend.member.domain.service | |
| import gomushin.backend.member.domain.entity.Member | |
| import gomushin.backend.member.domain.repository.MemberRepository | |
| import gomushin.backend.member.domain.value.Provider | |
| import gomushin.backend.member.domain.value.Role | |
| import gomushin.backend.member.presentation.dto.request.OnboardingRequest | |
| import org.junit.jupiter.api.Assertions.assertEquals | |
| import org.junit.jupiter.api.extension.ExtendWith | |
| import org.mockito.Mock | |
| import org.mockito.Mockito.verify | |
| import org.mockito.Mockito.`when` | |
| import org.mockito.junit.jupiter.MockitoExtension | |
| import java.time.LocalDate | |
| import java.util.* | |
| import kotlin.test.Test | |
| @ExtendWith(MockitoExtension::class) | |
| class OnboardingServiceTest { | |
| @Mock | |
| private lateinit var memberRepository: MemberRepository | |
| private val onboardingService: OnboardingService by lazy { | |
| OnboardingService(memberRepository) | |
| } | |
| @Test | |
| fun `onboarding 성공 케이스`() { | |
| // given | |
| val memberId = 1L | |
| val existingMember = Member( | |
| id = memberId, | |
| nickname = "원래 닉네임", | |
| email = "[email protected]", | |
| birthDate = LocalDate.of(1990, 1, 1), | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.GUEST, | |
| ) | |
| val onboardingRequest = OnboardingRequest( | |
| nickname = "새로운 닉네임", | |
| birthDate = LocalDate.of(2000, 1, 1) | |
| ) | |
| `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(existingMember)) | |
| // when | |
| onboardingService.onboarding(memberId, onboardingRequest) | |
| // then | |
| assertEquals("새로운 닉네임", existingMember.nickname) | |
| assertEquals(LocalDate.of(2000, 1, 1), existingMember.birthDate) | |
| assertEquals(Role.MEMBER, existingMember.role) | |
| verify(memberRepository).findById(memberId) | |
| } | |
| } | |
| package gomushin.backend.member.domain.service | |
| import gomushin.backend.member.domain.entity.Member | |
| import gomushin.backend.member.domain.repository.MemberRepository | |
| import gomushin.backend.member.domain.value.Provider | |
| import gomushin.backend.member.domain.value.Role | |
| import gomushin.backend.member.presentation.dto.request.OnboardingRequest | |
| import org.junit.jupiter.api.Assertions.assertEquals | |
| import org.junit.jupiter.api.extension.ExtendWith | |
| import org.mockito.Mock | |
| import org.mockito.Mockito.verify | |
| import org.mockito.Mockito.`when` | |
| import org.mockito.junit.jupiter.MockitoExtension | |
| import java.time.LocalDate | |
| import java.util.* | |
| import kotlin.test.Test | |
| @ExtendWith(MockitoExtension::class) | |
| class OnboardingServiceTest { | |
| @Mock | |
| private lateinit var memberRepository: MemberRepository | |
| private val onboardingService: OnboardingService by lazy { | |
| OnboardingService(memberRepository) | |
| } | |
| @Test | |
| fun `onboarding 성공 케이스`() { | |
| // given | |
| val memberId = 1L | |
| val existingMember = Member( | |
| id = memberId, | |
| nickname = "원래 닉네임", | |
| email = "[email protected]", | |
| birthDate = LocalDate.of(1990, 1, 1), | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.GUEST, | |
| ) | |
| val onboardingRequest = OnboardingRequest( | |
| nickname = "새로운 닉네임", | |
| birthDate = LocalDate.of(2000, 1, 1) | |
| ) | |
| `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(existingMember)) | |
| // when | |
| onboardingService.onboarding(memberId, onboardingRequest) | |
| // then | |
| assertEquals("새로운 닉네임", existingMember.nickname) | |
| assertEquals(LocalDate.of(2000, 1, 1), existingMember.birthDate) | |
| assertEquals(Role.MEMBER, existingMember.role) | |
| verify(memberRepository).findById(memberId) | |
| verify(memberRepository).save(existingMember) | |
| } | |
| } |
src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/member/domain/service/OnboardingService.kt
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/member/presentation/OnboardingController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/core/service/CustomUserDetailsService.kt
Show resolved
Hide resolved
| @@ -34,7 +34,7 @@ dependencies { | |||
| runtimeOnly("mysql:mysql-connector-java:8.0.33") | |||
|
|
|||
| // swagger | |||
| implementation("org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.1") | |||
| implementation("org.springdoc:springdoc-openapi-starter-webmvc-ui:2.7.0") | |||
There was a problem hiding this comment.
버전을 내린 이유가 궁금해!
아 이건 Swagger 응답이 좀 이상하게 나와서 버전 문제인 줄 알고 내렸거든.
근데 버전 문제는 아니었어
이건 내가 내렸다 안 올렸네
스웨거 2.8.1 이나 2.7.0 은 사용하는데 별 차이는 없으니까 그냥 가도 될 것 같아
| @@ -12,7 +12,8 @@ class CustomCorsConfiguration { | |||
| @Bean | |||
| fun corsConfigurationSource(): CorsConfigurationSource { | |||
| val configuration = CorsConfiguration() | |||
| configuration.allowedOrigins = listOf("http://localhost:5173", "http://localhost:8080") | |||
| configuration.allowedOrigins = | |||
| listOf("http://localhost:5173", "http://localhost:8080", "https://frontend-sarang.vercel.app") | |||
There was a problem hiding this comment.
프론트에서 로컬로 개발할 때 5173포트쓰나? 보통 3000번 포트 쓰지 않아?
There was a problem hiding this comment.
프론트에서 로컬로 개발할 때 5173포트쓰나? 보통 3000번 포트 쓰지 않아?
프론트 포트 5173이래
| } | ||
|
|
||
| response!!.addCookie(createCookie("access_token", accessToken)) | ||
| response.sendRedirect("http://localhost:8080") // TODO: 프론트엔드 주소로 변경 , 환경변수 처리 | ||
| response.sendRedirect("https://frontend-sarang.vercel.app") |
There was a problem hiding this comment.
처음에 프론트는 로컬환경에서 개발할건데 이렇게 하면 프론트입자에서 로컬에 테스트 할때 토큰을 못 받지 않나? 바로 배포주소로 리디렉션되니깐?
There was a problem hiding this comment.
처음에 프론트는 로컬환경에서 개발할건데 이렇게 하면 프론트입자에서 로컬에 테스트 할때 토큰을 못 받지 않나? 바로 배포주소로 리디렉션되니깐?
아 맞네 이것도 프론트 주소로 바꿀게
근데 우리 배포가 이미 둘 다 https 면 프론트에서 로컬로 쿠키를 못 받는데…
이건 고민 해봐야겠다
| private val memberRepository: MemberRepository | ||
| ) { | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
여긴 왜 readonly=true를 안 쓰는 거야?
There was a problem hiding this comment.
여긴 왜 readonly=true를 안 쓰는 거야?
JPA 변경감지를 통해서 Member를 업데이트 하는 코드인데, 여기에 readonly = true 를 달면 엔티티 스냅샷을 안만들거든.
그러면 변경감지가 안되니까 업데이트도 안돼.
간단하게 말하면 조회 메서드가 아닌 업데이트 메서드라서 안썼다~
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
✏️ 관련 이슈
🎸 기타 사항 or 추가 코멘트
처음 회원가입 후 유저 커플 연결 안되어있으면 ROLE이 GUEST야 그래서 온보딩까지 끝내야 MEMBER로 변해
그래서 온보딩 API는 Role이 GUEST만 사용 가능해
코드보고 이해 안되는 부분 있으면 물어봐
Summary by CodeRabbit