-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YS-68] feat: 연구자 회원가입 API 구현 #19
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` 인 경우, 예외 반환
DEV_YML 반영해야 해서 merge 바로 하면 터집니다! approve 받으면 제가 반영 후에 merge하도록 하겠습니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
연구자 회원가입 기능은 고려할 부분도 많고 구현할 작업도 복잡했을 텐데, 매끄럽게 잘 작성해주셔서 정말 좋습니다! 👍 코드를 보며 고민하셨을 지점들이 느껴지네요.
코드를 쭉 보면서 이번 변경사항에 대해 궁금한 점이나 개선되었으면 하는 부분들 코멘트로 남겼습니다. 확인해주시면 감사합니다, 고생 많으셨어요! 👏
@@ -42,3 +42,4 @@ application-local.yml | |||
|
|||
### Kotlin ### | |||
.kotlin | |||
/src/main/resources/application.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 파일을 .gitignore
에 추가한 이유는, 로컬에서 테스트용으로 추가한 민감한 정보가 포함될 수 있기 때문일까요?
.gitignore
에 추가해도 괜찮지만, 만약 로컬 환경에서 시크릿 값을 사용하려는 목적이라면, 기존에 존재하는 template-application-local.yml
파일을 활용해 로컬 설정을 진행해도 편해서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 주로 git add
명령어를 쓸 때 git add src/
를 쓰는데, 변경사항이 자잘하게 적용된 application.yml
파일이 자꾸 스테이징되어서 항상 git restore --staged
로 스테이징된 파일을 취소하다 보니 불편해서 .gitignore
에 추가하게 되었어요 😅
혹시나 해서 올라가는 불상사가 있을 수도 있어서요!
@@ -7,6 +7,7 @@ import org.springframework.boot.context.properties.ConfigurationPropertiesScan | |||
import org.springframework.cloud.openfeign.EnableFeignClients | |||
import org.springframework.context.annotation.ComponentScan | |||
import org.springframework.context.annotation.FilterType | |||
import org.springframework.data.jpa.repository.config.EnableJpaAuditing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용하지 않는 import
는 지워주세요~
|
||
fun toSendResDto() : EmailSendResponse{ | ||
return EmailSendResponse( | ||
isSucceess = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분과 아래 함수에 오타가 있습니다! isSuccess
로 수정해주세요~
private val emailVerificationUseCase: EmailVerificationUseCase | ||
) { | ||
@Transactional | ||
fun sendEmail(req: EmailSendRequest) : EmailSendResponse{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서비스 파일을 쭉 보니까 Request에 대한 변수명이 req, input, ~~Request 등으로 다르게 사용되고 있더라고요. 이 부분을 통일하면 가독성이 더 좋아질 것 같아요.
지금 당장 수정할 필요는 없고, 오늘 회의 때 천천히 얘기해보면 좋을 것 같습니다. 👏
fun researcherSignup(input: ResearcherSignupRequest) : SignupResponse{ | ||
if(!input.emailVerified) { | ||
println("Email verification failed: ${input.univEmail}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 이 부분은 로컬 테스트용 코드인가요, 아니면 로깅을 위한 코드인가요?
로컬 테스트용이라면 삭제하는 것이 좋을 것 같고, 로깅을 위한 용도라면 println
보다는 로깅 라이브러리를 사용하는 것이 더 적합할 것 같습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로컬 테스트용이었습니다! 삭제한다는 걸 깜빡하고 반영을 못했네요!! 😅
꼼꼼하게 읽어주셔서 감사합니다! 반영하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여러 가지를 고려할 부분이 많아서 구현하기 힘들었을텐데 고생 많으셨습니다!
코드 잘 작성되었는데, 한 유즈케이스 안에 로직이 집중되다 보니 코드가 길어진 것 같습니다. 해당 유즈케이스에서 사용되는 private method는 EmailUtils와 같은 별도의 유틸리티 클래스로 분리해서 사용하는 것도 좋을 것 같습니다. 이를 통해 코드의 재사용성과 가독성이 향상될 것 같아요!
|
||
@Tag(name = "이메일 인증 API - /v1/email") | ||
@RestController | ||
@RequestMapping("/v1/email") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메일과 관련한 작업을 처리하는 api들이니 emails처럼 복수형을 쓰는 건 어떨까요?
description = "연구자 회원가입 시, 학교 메일 인증 코드를 전송하는 API입니다." | ||
) | ||
fun sendCode(@RequestBody @Valid emailSendRequest: EmailSendRequest) | ||
: ApiResponse<EmailSendResponse> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각해보니 AuthController
와 SignupController
는 ApiResponse
로 감싸지 않고 바로 응답하네요... 이번주 안으로 리팩터링해야겠어요... 🥲
exception.message shouldBe "Verification information is not found" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예외 메시지를 하드코딩하기 보다는 CodeNotCorrectException().message
이런식으로 처리하는 건 어떨까요? 나중에 예외 메시지를 변경해야 한다면 한 곳에서만 수정하면 돼서 유지보수성을 높일 수 있다고 생각해서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
외부 의존성을 Gateway 패턴으로 추상화한 점 좋습니다. 👍
env["java.naming.factory.initial"] = "com.sun.jndi.dns.DnsContextFactory" | ||
val ctx = InitialDirContext(env) | ||
val attributes: Attributes = ctx.getAttributes(domain, arrayOf("MX")) | ||
val mxRecords = attributes.get("MX") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오! 반영하겠습니다 ㅎㅎ
- `/v1/email`→ `/v1/emails` 로 엔드포인트 조정 - 이메일 도메인 검증 중 getter 삭제 후 인덱싱 방법으로 재조정 - MemberResponse 필드값 `isSuccess` typo 수정 - 기존 검증 로직 EmailUtils 패키지로 구조 조정
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 제안드린 코멘트 잘 반영되었다고 생각이 들어 approve합니다. 수고하셨습니다~ 👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굳 좋습니다! 나중에 이메일 템플릿 생성 로직 같은 기능이 추가될 가능성을 고려한다면, 조금 더 SRP 원칙에 따라 세분화하는 방향도 생각해볼 수 있을 것 같아요. 하지만 이는 지금 당장 고민하기보다는 확장 필요성이 생겼을 때 고려해도 충분할 것 같아요. 👍
* feat: implements participant signup - 참여자 회원가입 API 구현 - 구글 OAuth 에외처리 로직 일부 수정 * test: add test codes for SignupMapper and ParticipantSignupUseCase - 테스트 커버리지 충족을 위해 테스트 코드 추가 - `SignupMapperTest` 코드 추가 - `ParticipantSignupUseCase` 코드 추가 * fix: resolve duplicate requested authentication - 실험자 회원가입 시, Authentication 중복 생성 코드 제거 * refact: move DB Transaction to seperate responsibility to satisfy clean architecture principal - DB 트랜잭션 책임 관련: UseCase → Service 계층으로 위임 * refact: Applay automatic Component Scan for UseCase classes - UseCase 클래스가 비즈니스 로직의 핵심 계층: 자동으로 컴포넌트 스캔 대상에 포함되도록 어노테이션 제거 - 관리와 확장성을 고려하여 클린 아키텍처를 적용한 구현 방식 * style: rename API endpoint signup to meet the convention rule - 회원가입 API endpoint 관련하여 기존 `/join` → `/signup` 으로 개선 - 코드 컨벤션을 위한 API endpoint 개선 * refact: update annotations to validate DateTime Format - 생년월일 데이터 포맷 관련: 기존: `@NotBlank` → `@NotNull` `@Past` `@DateTimeFormat` * refact: rename mapper function to match its usecase - dto → entity 변환하는 용례에 따라, 기존: `toAddressInfoDto` → `toAddressInfo` 로 리네이밍 * fix: reflect updated codes to test code - dto 함수 리네이밍 반영한 `SingupMapperTest` 버그 해결 * feat: implement researcher member signup - 연구자 회원가입 기본 로직 초안 구현 - `emailVerified` 필드 값이 false이면, `EmailNotValidateException` 반환하도록 구현 * test: update test codes for adjust additional logic - 세부 조정사항: 기존 MemberEntity에 있던 `birthDate` → ParticipantEntity 에 추가 - 이에 따른 기존 test code 수정하여 반영 * test: add test codes for AuthenticationUtils - `AuthenticationUtils` 에 대한 test code 작성 * test: add test codes to UseCases and Service related to signup logic - `SignupService`에 대한 test code 작성 - `CreateResearcherUsecase`,`ParticipantSignupUsecase` test code 작성&수정 * feat: add verification entity - `VerificationEntity`, `VerificationStatus` 도메인 엔티티 추가 - `VerificationRepository` 레포지토리 추가 * feat: implement Email Send Code and Verification using SMTP - SMTP 활용하여 메일 인증 코드 발송 로직 구현 - 유효한 이메일 도메인인지 확인(실제 존재하는지 여부) - 학교 이메일 여부 확인 로직 구현 - 학교 이메일 코드 전송 로직 구현 * fix: add exception codes for Email Verification logic - 인증코드 만료 시에도 재요청 가능하도록 로직 수정 - 이미 승인된 대학 이메일에 대해서는 '이미 승인 완료'라고 예외 처리 * docs: add application-yml file to .gitignore * fix: resolve merge conflicts with dev branch * chore: move packages structure - `usecase/SignUpUseCase/email` 패키지 밑에 메일 인증 관련 UseCase 재배ġ * fix: adjust test codes for non-failed test configuration - 테스트가 Fail하는 현상을 막기 위해, 몇 가지 조치를 취했습니다. * test: add test codes for Email send verification logic - `application/usecase/email/` 패키지 하위 UseCase에 대한 테스트 코드 작성 * feat: add email verification condition to researcherSignup - 연구자 회원가입 전 인증된 학교 메일에 대해서만 가입 가능하게끔 변경 - `ResearcherSignupRequest` 에서 `emailVerified=false` 인 경우, 예외 반환 * fix: add test codes for SignupServiceTest to meet the code coverage * refact: reflect updation from the code review - `/v1/email`→ `/v1/emails` 로 엔드포인트 조정 - 이메일 도메인 검증 중 getter 삭제 후 인덱싱 방법으로 재조정 - MemberResponse 필드값 `isSuccess` typo 수정 - 기존 검증 로직 EmailUtils 패키지로 구조 조정
💡 작업 내용
[Task Flow]
/v1/members/signup/participants
/v1/emails/send
→/v1/emails/verify
이메일 발송 API -
/v1/emails/send
이메일 코드 인증 로직 API -
/v1/emails/verify
✅ 셀프 체크리스트
🙋🏻 확인해주세요
➡️ 나중에 1차 MVP 끝난 이후에, 해당 이슈에 대해서 비동기 처리 방식이나 MQ 도입 등의 성능 최적화를 할 수 있는 방안을 마련해야 할 것 같습니다!
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-68