[YS-256] feat: 회원가입 연락 받을 이메일 중복 비허용 로직 추가 #80
Conversation
- 회원가입 시 연락 이메일 중복 가입 방지를 위한 조회 구현
Walkthrough이 PR은 회원 서비스에 중복 이메일 검증 기능을 추가합니다. 새로운 Changes
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt (1)
9-9: 성능 최적화를 위한 인덱스 추가를 검토해주세요.
contactEmail컬럼에 대한 조회가 자주 발생할 것으로 예상되므로, 해당 컬럼에 인덱스를 추가하는 것이 좋을 것 같습니다. 또한 동시성 제어를 위해@Lock(LockModeType.PESSIMISTIC_WRITE)어노테이션 추가도 고려해주세요.src/main/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCase.kt (2)
18-21: 에러 처리와 로깅을 보강해주세요.
- 디버깅과 모니터링을 위한 로깅 추가가 필요합니다.
- 예외 발생 시 중복된 이메일 값을 포함하면 디버깅에 도움이 될 것 같습니다.
override fun execute(input: Input): Output { - if(memberGateway.findByContactEmail(input.contactEmail) == null) return Output(success = true) - else throw SignupContactEmailDuplicateException + logger.debug("Verifying contact email: ${input.contactEmail}") + val existingMember = memberGateway.findByContactEmail(input.contactEmail) + + return if (existingMember == null) { + logger.debug("Email verification successful: ${input.contactEmail}") + Output(success = true) + } else { + logger.debug("Duplicate email found: ${input.contactEmail}") + throw SignupContactEmailDuplicateException.withEmail(input.contactEmail) + } }
7-9: 클래스 레벨 로거 선언을 추가해주세요.+import org.slf4j.LoggerFactory + class VerifyContactEmailUseCase( private val memberGateway: MemberGateway -): UseCase<VerifyContactEmailUseCase.Input, VerifyContactEmailUseCase.Output> { +): UseCase<VerifyContactEmailUseCase.Input, VerifyContactEmailUseCase.Output> { + private val logger = LoggerFactory.getLogger(javaClass)src/test/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCaseTest.kt (1)
11-41: 테스트 케이스 보완 제안현재 구현된 테스트는 기본적인 성공/실패 케이스를 잘 커버하고 있습니다. 다음과 같은 추가 테스트 케이스 구현을 고려해보시기 바랍니다:
- 잘못된 이메일 형식 검증
- 공백이나 null 값 처리
- 대소문자 구분 여부 검증
src/main/kotlin/com/dobby/backend/application/service/MemberService.kt (1)
36-39: 트랜잭션 설정 최적화 필요이메일 중복 확인은 읽기 전용 작업이므로, 현재 설정된 트랜잭션 어노테이션을 최적화할 수 있습니다.
다음과 같이 수정하는 것을 추천드립니다:
- @Transactional + @Transactional(readOnly = true) fun validateDuplicatedContactEmail(input: VerifyContactEmailUseCase.Input) : VerifyContactEmailUseCase.Output{ return verifyContactEmailUseCase.execute(input) }src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt (1)
51-62: HTTP 상태 코드 개선 필요이메일 검증 API의 응답이 더 명확한 HTTP 상태 코드를 사용할 수 있습니다:
- 이메일 사용 가능: 200 OK
- 이메일 중복: 409 Conflict (현재 403 대신)
또한 API 응답 형식을 더 구체적으로 정의하여
DefaultResponse대신 전용 응답 클래스를 사용하는 것을 고려해보세요.src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt (2)
7-7: 와일드카드 임포트 사용 검토 필요와일드카드 임포트(
.*)를 사용하면 코드의 명확성이 떨어질 수 있습니다. 특히 여러 패키지에서 동일한 이름의 클래스를 사용할 경우 혼란을 야기할 수 있습니다. 명시적인 임포트 사용을 고려해 주세요.
61-71: 이메일 검증 매핑 구현이 잘 되었습니다!새로 추가된 이메일 검증 매핑 함수들이 기존 코드 스타일을 잘 따르고 있으며, 간단하고 명확하게 구현되었습니다.
다만, 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
DefaultResponse의 success 필드만 사용하는 것이 충분한지 검토가 필요합니다.- 실패 케이스에 대한 추가 정보를 제공하는 것이 사용자 경험 향상에 도움이 될 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/kotlin/com/dobby/backend/application/service/MemberService.kt(2 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/exception/DobbyException.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt(2 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ContactEmailVerificationRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCaseTest.kt(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/main/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCase.kt (2)
Learnt from: chock-cho
PR: YAPP-Github/25th-Web-Team-2-BE#67
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/EmailCodeSendUseCase.kt:44-56
Timestamp: 2025-01-29T16:14:08.389Z
Learning: The VerificationRepository in this codebase implements a robust concurrency control mechanism with:
1. Pessimistic write locking (@Lock(LockModeType.PESSIMISTIC_WRITE)) on findByUnivEmail
2. Proper transaction boundaries using @Modifying annotations
3. Automatic timestamp updates in native queries for code updates
Learnt from: chock-cho
PR: YAPP-Github/25th-Web-Team-2-BE#67
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/EmailCodeSendUseCase.kt:44-56
Timestamp: 2025-01-29T16:14:08.389Z
Learning: The email verification system in this codebase uses pessimistic locking (@Lock(LockModeType.PESSIMISTIC_WRITE)) on findByUnivEmail method to handle concurrent verification requests.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ContactEmailVerificationRequest.kt (1)
7-12: 적절한 유효성 검사와 문서화가 구현되었습니다!이메일 형식 검증과 필수값 검사가 잘 구현되어 있으며, 한글 에러 메시지도 명확합니다.
src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt (1)
9-9: 동시성 제어 메커니즘 추가 검토가 필요합니다.이전 이메일 검증 구현에서 사용된
@Lock(LockModeType.PESSIMISTIC_WRITE)메커니즘을 이 메소드에도 적용하는 것이 좋을 것 같습니다. 동시에 여러 사용자가 같은 이메일로 가입을 시도할 경우를 대비하여 동시성 제어가 필요합니다.
src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/dobby/backend/domain/exception/DobbyException.kt
Outdated
Show resolved
Hide resolved
| if(memberGateway.findByContactEmail(input.contactEmail) == null) return Output(success = true) | ||
| else throw SignupContactEmailDuplicateException |
There was a problem hiding this comment.
Member 엔티티를 반환하는 findBy 보다는 Boolean 값을 반환하는 exists를 이용해 필요한 데이터만 가져오는 건 어떨까요?
There was a problem hiding this comment.
음, 제가 exists가 아니라 findBy 로 조회한 이유는 해당 부분이 지난 PR에서 매칭 공고 이메일 전송 시 멤버 이름을 조회하는 로직 에 활용하기 위해서 통합적으로 해당 메서드를 활용한 것이었는데요. (memberName을 가져올 때 findBy 메서드로 가져올 수 있게 하기 위해서)
오늘 올리신 PR 보니까 '참여자 수정' 에서의 중복 이메일 검증 로직에서 이미 exists 메서드를 이용하신 걸로 확인했습니다. 그래서 그거랑 통합해도 될 것 같네요!
There was a problem hiding this comment.
아하, 제가 생각하기에는 해당 유즈케이스에서는 중복된 이메일이 존재하는지만 확인하면 되므로, Member 엔티티를 조회하는 findBy보다 existsBy를 사용하여 최소한의 연산만 수행하도록 최적화 가능할 거라 기대해 코멘트를 남겼습니다!
| @@ -51,7 +51,7 @@ data object ResearcherNotFoundException : ClientException("ME0003", "Researcher | |||
| data object ParticipantNotFoundException : ClientException("ME0004", "Participant Not Found.", HttpStatus.NOT_FOUND) | |||
| data object EmailNotValidateException : ClientException("ME0005", "You should validate your school email first", HttpStatus.BAD_REQUEST) | |||
| data object SignupOauthEmailDuplicateException : ClientException("ME0006", "You've already joined with requested oauth email", HttpStatus.CONFLICT) | |||
|
|
|||
| data object SignupContactEmailDuplicateException: ClientException("ME0007", "You've already joined with requested contact email", HttpStatus.CONFLICT) | |||
There was a problem hiding this comment.
해당 예외는 회원가입 뿐만 아니라 회원 정보 수정 시에도 필요합니다. 그래서 조금 더 범용적으로 아래처럼 통일하는 건 어떨까요??
data object ContactEmailDuplicateException: ClientException("ME0007", "This contact email is already in use.", HttpStatus.CONFLICT)
| @PostMapping("/signup/validate/contact-email") | ||
| @Operation( | ||
| summary = "연락 받을 이메일 주소 검증 API - 회원가입 시 필수 API", |
There was a problem hiding this comment.
해당 API는 회원 정보를 수정할 때도 사용할 거 같습니다. 그래서 API URI를 /validate/contact-email 로 변경하는 방향은 어떨까요?
There was a problem hiding this comment.
회원가입 시 중복 이메일 검증 부분과 참여자 회원정보 수정은 필터 검증 부분에서 헤더의 액세스 토큰의 유무가 달라질 것같은데, 인증을 어떻게 처리해야 할까요? 필터단에서 건드리면 api 하나로 통합할 수 있을 것 같은데, 지수님께서 생각하신 방향성이 이게 맞을까요??🙋♀️
There was a problem hiding this comment.
중복 이메일 검증 API를 OAuth 로그인처럼 아예 인증없이 사용할 수 있게 처리하면 된다고 생각하는데 어떻게 생각하실까요?
프론트에서 해당 API를 회원정보 수정 기능에서 활용할 때 단순히 이메일 검증 용도로 사용할 테니 저희 서비스 회원인지 토큰으로 검증하는 부분은 생략해도 좋을 거 같아요!
There was a problem hiding this comment.
생각해보니 회원정보 수정 기능에서 검증할 때는 본인 이메일에 대해서는 검증 범위에 빼야하네요... 이걸 하나의 API로 처리할지 두 개의 API로 처리할지는 서버 회의때 얘기해봅시다!
There was a problem hiding this comment.
오늘 서버 회의를 통해 두 개의 API로 분리하기로 결정했습니다~
There was a problem hiding this comment.
@chock-cho 수정님 혹시 email을 리퀘스트 바디로 받는 것보다 파라미터로 받고 Get 요청으로 처리하는 건 어떨까요? email 하나에 대해 검증만 수행하는 API라 파라미터로 처리해도 좋을 거 같아서요!
There was a problem hiding this comment.
와우 그렇네요!!. 검증 로직은 어차피 조회하는 로직밖에 없기 때문에 POST보다 GET 요청이 좋겠군요. 쿼리 파라미터+GET으로 수정하겠습니다:)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt (1)
51-62: API 구현이 잘 되었습니다!이전 리뷰 논의사항들이 모두 반영되어 있습니다:
- GET 메소드 사용
- 쿼리 파라미터로 이메일 전달
- 인증 없이 호출 가능
API 문서화를 조금 더 개선하면 좋을 것 같습니다. description에 다음 내용을 추가하는 것은 어떨까요?:
- description = "연락 받을 이메일이 사용 가능한지 검증해주는 API입니다. 사용가능하면 true, 아니면 예외를 던집니다." + description = """ + 연락 받을 이메일이 사용 가능한지 검증해주는 API입니다. 사용가능하면 true, 아니면 예외를 던집니다. + - 성공 응답: 200 OK (이메일 사용 가능) + - 실패 응답: 403 CONFLICT (이미 사용 중인 이메일) + """src/test/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCaseTest.kt (3)
11-15: 클래스 레벨 KDoc 문서화를 추가하면 좋을 것 같습니다.테스트 클래스의 목적과 책임을 명확히 하기 위해 KDoc 문서를 추가하는 것이 좋습니다.
다음과 같이 문서화를 추가해보세요:
+/** + * 회원가입 시 연락 받을 이메일 중복 검증 유스케이스 테스트 + * + * @see VerifyContactEmailUseCase + */ class VerifyContactEmailUseCaseTest : BehaviorSpec({
16-28: 이메일 유효성 검증 테스트 케이스를 추가하면 좋을 것 같습니다.현재 테스트는 기본적인 시나리오만 다루고 있습니다. 다음과 같은 엣지 케이스들도 고려해보시면 좋을 것 같습니다:
- 잘못된 이메일 형식
- 빈 문자열
- 최대 길이 초과
29-41: 예외 메시지 검증을 추가하면 좋을 것 같습니다.예외가 발생하는 것을 확인하는 것 외에도, 적절한 에러 메시지가 포함되어 있는지 검증하면 좋을 것 같습니다.
다음과 같이 수정해보세요:
then("ContactEmailDuplication 예외가 발생해야 한다") { - shouldThrow<ContactEmailDuplicateException> { + val exception = shouldThrow<ContactEmailDuplicateException> { verifyContactEmailUseCase.execute(input) } + exception.message shouldBe "이미 등록된 이메일입니다: ${input.contactEmail}" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/exception/DobbyException.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt(2 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCaseTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt
- src/main/kotlin/com/dobby/backend/domain/exception/DobbyException.kt
- src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt
- src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt
- src/main/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCase.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt (1)
27-29: 구현이 깔끔하고 목적에 부합합니다!
existsByContactEmail메서드가 회원가입 시 연락 이메일 중복 검증이라는 요구사항을 잘 구현하고 있습니다. 구현이 간단명료하고 기존 코드 스타일을 잘 따르고 있습니다.src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt (1)
4-5: 임포트 변경이 적절해 보입니다!member 관련 DTO들을 와일드카드로 임포트하는 것이 코드를 더 간결하게 만들어줍니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/VerifyContactEmailUseCaseTest.kt (1)
1-10: 패키지 구조와 임포트가 잘 구성되어 있습니다!테스트에 필요한 모든 의존성이 적절하게 임포트되어 있으며, 패키지 구조도 도메인 계층을 잘 반영하고 있습니다.
| @@ -51,7 +51,7 @@ data object ResearcherNotFoundException : ClientException("ME0003", "Researcher | |||
| data object ParticipantNotFoundException : ClientException("ME0004", "Participant Not Found.", HttpStatus.NOT_FOUND) | |||
| data object EmailNotValidateException : ClientException("ME0005", "You should validate your school email first", HttpStatus.BAD_REQUEST) | |||
| data object SignupOauthEmailDuplicateException : ClientException("ME0006", "You've already joined with requested oauth email", HttpStatus.CONFLICT) | |||
|
|
|||
| data object ContactEmailDuplicateException: ClientException("ME0007", "This contact email is already in use", HttpStatus.CONFLICT) | |||
There was a problem hiding this comment.
저번 PR에서랑 이번 PR에서 추가된 예외들은 노션에 있는 예외 코드 정리에 업데이트 해주시면 감사합니다~
| fun emailAvailableCheck( | ||
| @RequestParam contactEmail: String |
There was a problem hiding this comment.
validateContactEmailForSignUp 이라는 함수명으로 변경하는 것은 어떨까요? 추후에 회원정보 수정을 위한 API도 추가해야 해서 함수명으로 구분이 가능하면 좋을 거 같아요!
* feat: implement gateway to add duplicate signup verification - 회원가입 시 연락 이메일 중복 가입 방지를 위한 조회 구현 * feat: define contactEmailVerificationRequest dto * feat: define application level and exception * feat: define presentation level(controller, mapper) * test: add test codes for VerifyContactEmailUseCase * refact: rename appropriate parameter naming * refact: rename appropriate exception message to meet the convention * refact: convert find method to exists method for optimization * fix: fix test codes to reflect updation * fix: convert PostMapping to GetMapping * fix: add unique constraint in field value to prevent race condition
* feat: implement gateway to add duplicate signup verification - 회원가입 시 연락 이메일 중복 가입 방지를 위한 조회 구현 * feat: define contactEmailVerificationRequest dto * feat: define application level and exception * feat: define presentation level(controller, mapper) * test: add test codes for VerifyContactEmailUseCase * refact: rename appropriate parameter naming * refact: rename appropriate exception message to meet the convention * refact: convert find method to exists method for optimization * fix: fix test codes to reflect updation * fix: convert PostMapping to GetMapping * fix: add unique constraint in field value to prevent race condition
💡 작업 내용
/signup/validate/contact-email) 했습니다.✅ 셀프 체크리스트
🙋🏻 확인해주세요
case 1) 가입 가능한 이메일일 때 - 200 OK
case 2) 이미 가입하여 불가능한 이메일일 때 - 403 CONFLICT
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-256
Summary by CodeRabbit
Summary by CodeRabbit
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-256