[YS-116] refact: 회원가입 로직 클린 아키텍처 따르도록 코드 리팩터링 #23
Conversation
- 참여자 회원가입 API 구현 - 구글 OAuth 에외처리 로직 일부 수정
- 테스트 커버리지 충족을 위해 테스트 코드 추가 - `SignupMapperTest` 코드 추가 - `ParticipantSignupUseCase` 코드 추가
- 실험자 회원가입 시, Authentication 중복 생성 코드 제거
architecture principal - DB 트랜잭션 책임 관련: UseCase → Service 계층으로 위임
- UseCase 클래스가 비즈니스 로직의 핵심 계층: 자동으로 컴포넌트 스캔 대상에 포함되도록 어노테이션 제거 - 관리와 확장성을 고려하여 클린 아키텍처를 적용한 구현 방식
- 회원가입 API endpoint 관련하여 기존 `/join` → `/signup` 으로 개선 - 코드 컨벤션을 위한 API endpoint 개선
- 생년월일 데이터 포맷 관련: 기존: `@NotBlank` → `@NotNull` `@Past` `@DateTimeFormat`
- dto → entity 변환하는 용례에 따라, 기존: `toAddressInfoDto` → `toAddressInfo` 로 리네이밍
- dto 함수 리네이밍 반영한 `SingupMapperTest` 버그 해결
- 연구자 회원가입 기본 로직 초안 구현 - `emailVerified` 필드 값이 false이면, `EmailNotValidateException` 반환하도록 구현
- 세부 조정사항: 기존 MemberEntity에 있던 `birthDate` → ParticipantEntity 에 추가 - 이에 따른 기존 test code 수정하여 반영
- `AuthenticationUtils` 에 대한 test code 작성
- `SignupService`에 대한 test code 작성 - `CreateResearcherUsecase`,`ParticipantSignupUsecase` test code 작성&수정
- `VerificationEntity`, `VerificationStatus` 도메인 엔티티 추가 - `VerificationRepository` 레포지토리 추가
- SMTP 활용하여 메일 인증 코드 발송 로직 구현 - 유효한 이메일 도메인인지 확인(실제 존재하는지 여부) - 학교 이메일 여부 확인 로직 구현 - 학교 이메일 코드 전송 로직 구현
- 인증코드 만료 시에도 재요청 가능하도록 로직 수정 - 이미 승인된 대학 이메일에 대해서는 '이미 승인 완료'라고 예외 처리
- `usecase/SignUpUseCase/email` 패키지 밑에 메일 인증 관련 UseCase 재배ġ
- 테스트가 Fail하는 현상을 막기 위해, 몇 가지 조치를 취했습니다.
- `application/usecase/email/` 패키지 하위 UseCase에 대한 테스트 코드 작성
- 연구자 회원가입 전 인증된 학교 메일에 대해서만 가입 가능하게끔 변경 - `ResearcherSignupRequest` 에서 `emailVerified=false` 인 경우, 예외 반환
- `/v1/email`→ `/v1/emails` 로 엔드포인트 조정 - 이메일 도메인 검증 중 getter 삭제 후 인덱싱 방법으로 재조정 - MemberResponse 필드값 `isSuccess` typo 수정 - 기존 검증 로직 EmailUtils 패키지로 구조 조정
… into feat/YS-114
…DIP principal - 클린 아키텍처 원칙 준수를 위한 기존 이메일 인증 로직 리팩토링 - `VerificationGateway` `VerificationGatewayImpl` 추가 - `VerificationModel` 추가
- researcherSignup 시 쿼리가 2번 날아가 정합성 깨지는 현상 - → Researcher,Participant / Member 도메인 @OnetoOne으로 맵핑 전략 수정
저장된 Researcher 및 Member 엔티티의 ID가 정상적으로 반영되지 않는 문제를 해결하였습니다. - createResearcher에서 저장된 엔티티의 반환값을 사용하도록 수정 - researcherGateway.save 호출 시 반환 값이 최신 ID를 확인하도록 확인
|
| .securityMatcher( "/v1/**") | ||
| .csrf { it.disable() } | ||
| .cors(Customizer.withDefaults()) | ||
| .sessionManagement { | ||
| it.sessionCreationPolicy(SessionCreationPolicy.STATELESS) | ||
| } | ||
| .authorizeHttpRequests { | ||
| it.anyRequest().authenticated() | ||
| it.anyRequest().permitAll() // 모든 요청 허용 |
There was a problem hiding this comment.
/v1/members/**와 같이 인증이 필요한 URI를 구체적으로 명시하고 authenticated()로 제한하는 것이 보안적으로 더 적합하다고 생각합니다.
현재 설정(/v1/**에 대해 permitAll())은 모든 요청을 허용하는데, 혹시 이렇게 변경하신 이유가 있을까요? 인증이 필요한 URI를 점진적으로 추가하는 방식이 더 안전할 것이라 생각해서요!
There was a problem hiding this comment.
밑의 답변 참고해주세요! 임시로 넣은 부분입니다!
| val path = request.servletPath | ||
|
|
||
| if(path.startsWith("/v1/members/signup") || path.startsWith("/v1/emails") || path.startsWith("/v1/auth")) { | ||
| filterChain.doFilter(request, response) | ||
| return | ||
| } |
There was a problem hiding this comment.
이 파일에 해당 uri를 넣는 것보다는 WebSecurityConfig의 authSecurityFilterChain에서 .securityMatcher("/v1/auth/**", "/v1/members") 이런식으로 인증을 생략하도록 하면 될 거 같습니다!
There was a problem hiding this comment.
처음에는 그렇게 시도했는데, 계속 인증 토큰 관련해서 오류가 떠서 인가에 대한 마지막 선택지로 넣었던 것 같아요.
해당 부분 다시 시도해보고 말씀 드리겠습니다.
| @@ -31,7 +31,7 @@ object EmailUtils{ | |||
| "handong.edu", | |||
| "ewhain.net" | |||
| ) | |||
| return email.endsWith("@ac.kr") || eduDomains.any { email.endsWith(it) } | |||
| return email.endsWith(".ac.kr") || eduDomains.any { email.endsWith(it) } | |||
| class MemberEntity( | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "id") | ||
| val id: Long, | ||
| val id: Long = 0L, |
| ) = Participant( | ||
| id = 0, |
There was a problem hiding this comment.
사소하지만 Researcher는 0L로 되어있어서 통일하면 좋을 거 같습니다!
그리고 Member 엔티티에서 0L로 초기화하고 있던데 혹시 에러가 나서 그런거라면 Member에 대한 model도 0L로 초기화하면 되지 않을까요??
| object ParticipantConverter { | ||
| fun toModel(entity: ParticipantEntity): Participant { | ||
| return Participant( | ||
| member = Member( | ||
| id = entity.member.id, | ||
| contactEmail = entity.member.contactEmail, | ||
| oauthEmail = entity.member.oauthEmail, | ||
| provider = entity.member.provider, | ||
| role = entity.member.role, | ||
| name = entity.member.name, | ||
| status = entity.member.status |
There was a problem hiding this comment.
MemberEntity에 선언된 toDomain/fromDomain 메서드와 역할이 비슷해 보이는데, 의도한 설계가 동일한지 궁금합니다!
엔티티가 없는 VerificationConverter의 경우에는 어쩔 수 없겠지만, Participant와 Researcher처럼 관련 엔티티가 있는 경우 각 엔티티에 선언하면 더 깔끔하고 직관적일 거 같은데 어떠실까요? 😊
| : ApiResponse<EmailSendResponse> { | ||
| val input = EmailMapper.toEmailCodeSendUseCaseInput(emailSendRequest) | ||
| val output = emailService.sendEmail(input) | ||
| val response = EmailMapper.toEmailSendResponse(output) | ||
| return ApiResponse.onSuccess(response) |
There was a problem hiding this comment.
응답 형식은 ApiResponse로 감싸지 않고 바로 응답하기로 결정했었습니다! 😊
아래 API와 함께 이 부분 해당 결정에 맞게 수정 부탁드릴게요~!
| object EmailMapper { | ||
|
|
||
| fun toEmailCodeSendUseCaseInput(request: EmailSendRequest): EmailCodeSendUseCase.Input { | ||
| return EmailCodeSendUseCase.Input( | ||
| univEmail = request.univEmail | ||
| ) | ||
| } |
There was a problem hiding this comment.
컨트롤러 안에서 UseCase의 Input과 Output을 하나씩 매칭하기에는 코드가 너무 길어져서 mapper를 따로 둔걸까요? 그렇다면 이 방법이 깔끔해보이네요. 👍
| class ParticipantSignupUseCase ( | ||
| private val participantRepository: ParticipantRepository, | ||
| private val jwtTokenProvider: JwtTokenProvider | ||
| ): UseCase<ParticipantSignupRequest, SignupResponse> | ||
| private val participantGateway: ParticipantGateway, |
There was a problem hiding this comment.
실험자 회원가입 UseCase처럼 피험자의 경우에도 앞에 Create 붙이면 좋을 거 같습니다~
There was a problem hiding this comment.
리뷰할 부분이 많아서 Mapper에 있는 내용을 깊게 살펴보진 못했지만, GetMemberByIdUseCase처럼 domain 패키지내의 Member를 이용하면 MemberResponse에 필요한 값을 직접 넘겨줄 수 있지 않을까 생각이 들어요!
만약 이렇게 처리할 수 있다면, 매번 별도의 함수를 생성해서 처리하는 것보다 더 재사용성이 높고 효율적인 코드가 될 것 같습니다! 각 엔티티에 관한 domain 클래스를 적극적으로 활용하면 중복을 줄이고 코드가 더 깔끔해질 거라고 생각합니다. 🙂
💡 작업 내용
1-3 번 작업은 다음을 포함합니다.
domain/modelGateway정의✅ 셀프 체크리스트
🙋🏻 확인해주세요
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-116