[FEAT] 번호 인증 검사 기능 구현 #15
Conversation
Return 타입으로 Response DTO를 선언했습니다.
요청 데이터 기반 도메인 객체와 조회 데이터 기반 도메인 객체의 동일여부에 따라 검증 여부를 판별할 수 있도록 합니다. - PhoneVerification 객체에 대해 `@EqualsAndHashCode` 어노테이션을 추가하여 필드값을 통한 비교가 가능하도록 했습니다. * AuthRequest 객체에서 Command로 변환하는 메서드도 정의했습니다. * 누락된 Repository 내 인증 이력 조회가 없을 경우에 대한 실패 코드 정의 작업 내용을 추가했습니다.
- 내부 `PhoneVerification.VerificationCode` Record 또한 객체로서 생성되어 HashCode 값이 상이하기 때문에 발생한 테스트 불통과 문제를 해결했습니다.
|
기존 interface의 default 접근제어자 메서드들에 접근 가능한 환경을 개선했습니다. - 구현 클래스가 많아지기 때문에 내부적으로 도메인별 패키징을 추가했습니다.
Transaction의 성격이 강한 메서드 명보다 직관적인 메서드 명으로 변경했습니다.
|
코드 리뷰는 오프라인으로 진행했기 때문에 생략하겠슴다~~~ 1.제 생각에는요 다만 verify 메서드에서 두 가지의 역할을 하고 있음이 분명하기 때문에, SRP를 유지가 목적이라면 Event 발행 구조를 적용해도 될 것 같지만 따라서 제 의견은 현행 유지. 입니다. 2.만약 한 API 엔드포인트에서 여러 유스케이스를 호출하는 구조라면, 유즈케이스 설계가 잘못된 경우라고 생각합니다! |
| public final class AuthResponse {} | ||
| public final class AuthResponse { | ||
|
|
||
| public record VerifyResult(@JsonProperty("isVerified") boolean isVerified) {} |
There was a problem hiding this comment.
p3
회원가입에서 번호 인증을 사용하게 되면 운영진이 제공해준 사용자 정보를 식별할 수 있는 값도 전달되어야 할 것 같은데 어떻게 생각하시나요? 컨트롤러를 분리하는게 좋을지 response에서 필드를 추가해 재사용하는게 좋을지 고민되어 일단 response에 남겨봅니다!
There was a problem hiding this comment.
오호옷!!
식별 데이터라 하심은
임원진으로부터 전달받은 "신규회원 csv를 저장한 테이블"에서의 ID를 넘겨주자는 거죠?!
전 좋은 것 같습니다!!
그래야만 소셜 인증 요청 시, 인증된 csv 내 신규회원 식별 값을 함께 넘겨 해당 DB에서 삭제까지 용이하게 할 수 있겠군요
(저희가 논의했을 땐, 소셜인증까지 완료했을 때만 csv 저장 데이터를 삭제하자고 했었으니까!!)
There was a problem hiding this comment.
넵 근데 고려되어야 할 점이 회원가입 시점에서는 csv 파일을 저장한 id를 넘겨주셔야하지만 소셜계정 변경을 위해 번호인증을 한 경우에는 실제 유저의 id를 넘겨주시거나 다른 값(현재 사용되고 있는 socialPlaltformId 등)을 전달 해야할 것 같은데 어떻게 생각하시나요?
src/main/java/sopt/makers/authentication/database/PhoneVerificationRepositoryImpl.java
Show resolved
Hide resolved
src/main/java/sopt/makers/authentication/database/PhoneVerificationRepositoryImpl.java
Show resolved
Hide resolved
.../sopt/makers/authentication/database/rdb/repository/auth/PhoneVerificationJpaRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/sopt/makers/authentication/usecase/auth/service/VerifyVerificationService.java
Show resolved
Hide resolved
helper 레이어를 추가함으로서 기존 Repository 인터페이스 내에서 default 메서드 정의로 예외 처리하던 구조를 helper 객체로 이관했습니다. - Repository 객체는 실제 쿼리 실행 역할만 수행할 수 있도록 개선했습니다. - [리뷰](#15 (comment))
src/main/java/sopt/makers/authentication/usecase/auth/service/VerifyVerificationService.java
Show resolved
Hide resolved
Related Issue 🚀
Work Description ✏️
(Jpa 테스트의 경우, 리뷰를 받는 과정에서 추가 진행할 예정입니다.)
PR Point 📸
※ Verify(검증) 메서드 역할 분리
verify()메서드 구현체 내에서 데이터 삭제까지 진행delete()메서드 추가 구현하여 함수 분리@sung-silver @hyunw9
위 사항에 대한 여러분들의 의견이 궁금합니다!! 가감없이 의견 공유 주세요!!
※ Usercase 메서드 반환 타입
본 PR에서는 구현한
verify()메서드의 반환 타입으로AuthResponse.VerifyResultDTO 객체를 반환하여해당 반환 객체를 Response Body Data로 주입하도록 구현했습니다.
본래 계층 간 협력 관계에서 협력 메시지 객체를 사용하도록 하여 도메인 객체를 외부로부터 보존하기 위해 위 방법을 채택했습니다!
이 때, 아래와 같은 상황이 우려되었습니다.
" 한 개의 API 메서드 내부에서 복수의 Usecase 메서드를 호출하는 경우 "
Usecase의 메서드 반환 타입이 모두 Response Body DTO 객체라면
복수의 Usecase 메서드를 호출했을 때, " 복수의 Response Body DTO를 재조합하여 새로운 Response Body DTO 객체를 생성 "해야하는 상황이 발생할 것으로 예상되었습니다.
@sung-silver @hyunw9
이러한 상황은 바람직한 구조는 아니라고 판단되어 여러분들과 논의를 해보면 좋을 것 같아 가져오게 되었습니다!!
편하게 의견 공유 주세요!!