Skip to content
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

[refactor] User, Auth에 대한 DDD구조로 리팩토링을 진행한다. #91

Merged
merged 22 commits into from
Nov 27, 2024

Conversation

juuuunny
Copy link
Contributor

✅ PR 유형

어떤 변경 사항이 있었나요?

  • 새로운 기능 추가
  • 버그 수정
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
  • 코드 리팩토링
  • 주석 추가 및 수정
  • 문서 수정
  • 빌드 부분 혹은 패키지 매니저 수정
  • 파일 혹은 폴더명 수정
  • 파일 혹은 폴더 삭제

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • User, Auth에 대한 DDD구조로 리팩토링을 진행한다.

✏️ 관련 이슈

본인이 작업한 내용이 어떤 Issue Number와 관련이 있는지만 작성해주세요

user 와 auth 도메인이 분리되면서, auth에는 domain layer가 존재하지 않는것으로 보이는데요.
domain은 존재하는데, domain이 없을 수 있을까요? 단순히 domain == Entity 라는것을 넘어서서, 도메인 레이어에는 해당 도메인의 정책이 들어가게 됩니다.
auth라면 인증에 대한것들이 들어가게 되겠죠? (이런 로직들이 도메인의 존재 이유가 되며, 다른 레이어에 의존하면 안되기 때문에 가장 변화에 안전한 domain layer에 위치하게 됩니다.)

domain 이 없는 domain은 정책이 없는 domain같은 느낌이 드는것 같아요.

현재 Auth 도메인은 외부 소셜 인증 프로바이더 연동 및 JWT 생성/검증 같은 기술적 책임에 초점을 맞추고 있습니다.
도메인 관점에서 바라볼 때 Auth는 인증 과정을 처리하는 도메인 역할을 하고 있지만, 구체적인 정책은 다른 도메인(User 등)에 속하는 부분이라고 생각하고 있습니다. 이는 인증 정책이 Auth의 책임이라기보다, 사용자 관리의 연장선으로 보인다고 생각이 들었기에
Auth를 User 도메인에 속하도록 하며 리팩토링하였습니다.

public class UserMapper {

    public static User toDomain(UserEntity userEntity) {

        if (userEntity == null) {
            return null;
        }

        return User.toDomain(
                userEntity.getId(),
                userEntity.getProvider(),
                userEntity.getProviderId(),
                userEntity.getEmail(),
                userEntity.getNickname()
        );
    }

    public static UserEntity toEntity(User user) {
        return UserEntity.toEntity(
                user.getId(),
                user.getProvider(),
                user.getProviderId(),
                user.getEmail(),
                user.getNickname()
        );
    }
}

또한, 위와 같이 Mapper에서 DB로부터 해당 정보를 찾을 수 없을 경우 toDomain을 통해 도메인으로 변환하는 과정에서 NullPointerException 오류가 터져 엔티티에서 도메인으로 변환하는 부분은 null인지 여부를 파악하는 부분을 추가하였습니다.


🎸 기타 사항 or 추가 코멘트

@juuuunny juuuunny self-assigned this Nov 26, 2024
@juuuunny juuuunny added 🔄 refactor 코드 리팩토링 😆 JUNHYEONG 준형 Issue or PR labels Nov 26, 2024
@juuuunny
Copy link
Contributor Author

추가적으로 아직 적용하진 않았지만 고민중에 있는 부분이 쿠키에 토큰을 추가해주는 부분은 외부로직으로 controller 측에서 해주는 것으로 하였는데 oAuth2LoginSuccessHandler에서 기존 유저와 신규 유저일 시에는 현재 애플리케이션 서비스에서 해당 부분을 실행하고 있습니다.
이 것과 관련하여 handler인 oAuth2LoginSuccessHandler에서 해주는 것으로 바꾸어도 괜찮을까요?

저는 쿠키에 추가해주는 부분은 handler에서 해도 괜찮다곤 생각합니다..!!

@juuuunny juuuunny changed the base branch from develop to refactor-v1 November 26, 2024 17:34
@bbbang105
Copy link
Member

추가적으로 아직 적용하진 않았지만 고민중에 있는 부분이 쿠키에 토큰을 추가해주는 부분은 외부로직으로 controller 측에서 해주는 것으로 하였는데 oAuth2LoginSuccessHandler에서 기존 유저와 신규 유저일 시에는 현재 애플리케이션 서비스에서 해당 부분을 실행하고 있습니다. 이 것과 관련하여 handler인 oAuth2LoginSuccessHandler에서 해주는 것으로 바꾸어도 괜찮을까요?

저는 쿠키에 추가해주는 부분은 handler에서 해도 괜찮다곤 생각합니다..!!

기존 유저일 때는 핸들러에서만 처리하니 그렇게 해야할 것 같아요!!
다만 신규 유저(온보딩)는 제가 보기에는 컨트롤러에서 signupUser를 호출해서 처리하는 것 같아서 이 때는 컨트롤러에서 처리할 수 있지 않을까요?? 제가 잘못 이해한 것이라면 알려주세요!!

@juuuunny
Copy link
Contributor Author

추가적으로 아직 적용하진 않았지만 고민중에 있는 부분이 쿠키에 토큰을 추가해주는 부분은 외부로직으로 controller 측에서 해주는 것으로 하였는데 oAuth2LoginSuccessHandler에서 기존 유저와 신규 유저일 시에는 현재 애플리케이션 서비스에서 해당 부분을 실행하고 있습니다. 이 것과 관련하여 handler인 oAuth2LoginSuccessHandler에서 해주는 것으로 바꾸어도 괜찮을까요?
저는 쿠키에 추가해주는 부분은 handler에서 해도 괜찮다곤 생각합니다..!!

기존 유저일 때는 핸들러에서만 처리하니 그렇게 해야할 것 같아요!! 다만 신규 유저(온보딩)는 제가 보기에는 컨트롤러에서 signupUser를 호출해서 처리하는 것 같아서 이 때는 컨트롤러에서 처리할 수 있지 않을까요?? 제가 잘못 이해한 것이라면 알려주세요!!

아 신규 유저일 때 어세스토큰, 리프레시 토큰 받는 것은 signupUser에서 호츌하기에 그건 맞지만
oAuth2LoginSuccessHandler에서 신규 유저일 때 레지스터 토큰을 쿠키에 넣어주는 부분이 있지 않나요?? 그 레지스터 토큰 넣어주는 것을 핸들러에서 해도 될지 궁금합니다!!

Copy link
Member

@bbbang105 bbbang105 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리팩토링 고생하셨습니다 👍🏻👍🏻

Comment on lines 48 to 62
/**
* 전화번호 형식을 010-XXXX-XXXX 형태로 변경한다.
* @param phoneNumber 휴대폰 번호
* @return 변경된 형식의 전화번호
*/
private static String formatPhoneNumber(String phoneNumber) {
String pattern = "^010(\\d{4})(\\d{4})$";
Pattern regex = Pattern.compile(pattern);
Matcher matcher = regex.matcher(phoneNumber.replaceAll("-", "")); // 입력값에서 "-" 제거
if (matcher.matches()) {
return "010-" + matcher.group(1) + "-" + matcher.group(2);
} else {
throw new CustomException(UserErrorStatus._BAD_REQUEST_PHONE_NUMBER_TYPE);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인 내부 로직으로 둔 건 괜찮은 것 같아요!!
다만 현재는 호출해서 사용하는 유틸성으로 느껴져서

formatPhoneNumber 메서드는 private으로 변경하고,

public void validateAndFormatPhoneNumber(String phoneNumber) {
    this.phoneNumber = formatPhoneNumber(phoneNumber);
}

이렇게 외부에서 바로 검증 및 포맷 후 지정할 수 있게 만드는 건 어떨까요??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateAndFormatPhoneNumber와 같이 포맷 후 저장하는 메서드를 따로 만들고 service에서 호출하는 식으로 말씀해주신거죠??

그렇다면 여기서 바꾸는 것보다 service쪽에서 위와 같이 validateAndFormatPhoneNumber를 호출해서 저장할 수 있게 하는 것 좋은 것 같아요!

Comment on lines +10 to +11
@Component
public class SmsManager {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니당 👍🏻

Comment on lines 10 to 12
if (userEntity == null) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요 부분은 Mapper 마다 있는 것 같던데, null을 반환하게 되면 이후 어떻게 되나요??
호출한 쪽에서 에러 처리를 진행하나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Repository
@RequiredArgsConstructor
public class StadiumRepositoryImpl implements StadiumRepository {

    private final StadiumJpaRepository stadiumJpaRepository;
    private final StadiumCustomRepository stadiumCustomRepository;

    @Override
    public Stadium findStadiumByName(String stadiumName) {
        return stadiumJpaRepository.findByName(stadiumName)
                .map(StadiumMapper::toDomain)
                .orElseThrow(() -> new CustomException(StadiumErrorStatus._NOT_FOUND_STADIUM));
    }
}

위 부분은 stadium 도메인의 레포지토리Impl에서 예외 처리를 해주는 부분인데 원래 대부분은 저렇게 DB에 없을 경우 찾은 다음 바로 예외 처리를 해주는 것이 좋아보이는데

/**
     * OAuth2 사용자 정보 추출 및 신규 여부 확인.
     *
     * @param token OAuth2 인증 토큰
     * @return 신규 사용자 여부
     */
    @Transactional(readOnly = true)
    public boolean isNewUser(OAuth2AuthenticationToken token) {
        OAuth2UserInfo oAuth2UserInfo = extractOAuth2UserInfo(token);
        return userRepository.findUserByProviderId(oAuth2UserInfo.getProviderId()) == null;
    }

-> 대표적인 예시를 들자면 UserRepostiroy에서 findUserByProviderId에서는 User 또는 null을 반환해주어 신규 사용자인지 여부를 파악할 수 있는 부분이 있는데요.
만약 db에 providerId에 해당하는 유저가 없을 경우에는 예외 처리를 해주는 것 대신에 null로 반환을 해주어야 합니다.
본래처럼 그대로 DB에 없어 null이 나올 때 UserMapper.toDomain(UserEntity userEntity) 안에 userEntity를 넣어주게되면 NullPointerException이 터지더라구요!
그래서 이렇게 null 여부를 추가해주었습니다.

이건 사실 에러처리를 위해서기보단 null인지 아니면 객체로 나왔는지에 따라 다르게 로직 처리를 위해 해주었습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 상황과 관련해서 Mapper에서 null을 반환하는 것이 아닌 repositoryImpl에서 null 여부를 파악하는 것으로 변경하였습니다!

Comment on lines 15 to 22

_BAD_REQUEST_PHONE_NUMBER_TYPE(HttpStatus.BAD_REQUEST, "USER-003", "잘못된 휴대폰 번호 형식입니다. 010-0000-0000 형식으로 입력해주세요."),
_EXPIRED_AUTH_CODE(HttpStatus.UNAUTHORIZED, "USER-004", "인증코드가 만료되었습니다. 인증코드를 재발급해주세요."),
_MISS_MATCH_AUTH_CODE(HttpStatus.BAD_REQUEST, "USER-005", "인증코드가 일치하지 않습니다."),

_EXISTING_USER_ACCOUNT_KAKAO(HttpStatus.FORBIDDEN, "USER-006", "이미 회원가입된 카카오 계정이 존재합니다."),
_EXISTING_USER_ACCOUNT_GOOGLE(HttpStatus.FORBIDDEN, "USER-007", "이미 회원가입된 구글 계정이 존재합니다."),
_EXISTING_USER_ACCOUNT_NAVER(HttpStatus.FORBIDDEN, "USER-008", "이미 회원가입된 네이버 계정이 존재합니다."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기 줄바꿈 추가된 별다른 이유가 있나요??!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이거는 다시 수정하겠습니다!

@@ -8,13 +8,15 @@

@Getter
@RequiredArgsConstructor
public enum UserErrorStatus implements BaseErrorCode {
public enum UserErrorStatus implements BaseErrorCode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 클래스명 앞에 공백 하나 있는 것 같아요

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 Q클래스 파일도 계속 커밋해야하는걸까요??!
빌드할 때 자동으로 생성되는 것 같아서요!

Copy link
Contributor Author

@juuuunny juuuunny Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이거는 전에 QUserEntity가 만들어져야 하는데 QUser가 계속 만들어지며 빌드 에러가 계속 생겨서 수정하면서 생겨난 부분이라 다음부턴 안 생성될거에요

Comment on lines 38 to 41
TokenResponseDto tokenResponseDto = userApplicationService.signupUser(registerToken, request);
CookieUtil.setAuthCookies(response, tokenResponseDto.accessToken(), tokenResponseDto.refreshToken(),
tokenResponseDto.accessTokenExpiration(), tokenResponseDto.refreshTokenExpiration());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니당 👍🏻

Comment on lines 27 to 33
* 레지스터 토큰에서 추출한 정보들과 닉네임으로 회원가입을 진행한다.
* @param registerToken 쿠키로부터 받은 레지스터 토큰
* @param request 닉네임
* @return void
*/
@PostMapping("/signup")
public ResponseEntity<ApiResponse<Void>> signupUser(
public ResponseEntity<ApiResponse<TokenResponseDto>> signupUser(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 반환 값이 void인가요 TokenResponseDto인가요??
리턴 보면은 status만 반환하는 것 같아서요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아직 수정이 안됐네요
TokenResponseDto로 수정할게요!

@bbbang105
Copy link
Member

추가적으로 아직 적용하진 않았지만 고민중에 있는 부분이 쿠키에 토큰을 추가해주는 부분은 외부로직으로 controller 측에서 해주는 것으로 하였는데 oAuth2LoginSuccessHandler에서 기존 유저와 신규 유저일 시에는 현재 애플리케이션 서비스에서 해당 부분을 실행하고 있습니다. 이 것과 관련하여 handler인 oAuth2LoginSuccessHandler에서 해주는 것으로 바꾸어도 괜찮을까요?
저는 쿠키에 추가해주는 부분은 handler에서 해도 괜찮다곤 생각합니다..!!

기존 유저일 때는 핸들러에서만 처리하니 그렇게 해야할 것 같아요!! 다만 신규 유저(온보딩)는 제가 보기에는 컨트롤러에서 signupUser를 호출해서 처리하는 것 같아서 이 때는 컨트롤러에서 처리할 수 있지 않을까요?? 제가 잘못 이해한 것이라면 알려주세요!!

아 신규 유저일 때 어세스토큰, 리프레시 토큰 받는 것은 signupUser에서 호츌하기에 그건 맞지만 oAuth2LoginSuccessHandler에서 신규 유저일 때 레지스터 토큰을 쿠키에 넣어주는 부분이 있지 않나요?? 그 레지스터 토큰 넣어주는 것을 핸들러에서 해도 될지 궁금합니다!!

현재로써는 그게 최선일 것 같아요..! 아니면 처리해주는 클래스를 하나 더 만들어야 할 것 같은데.. 좀 더 찾아봐야할 것 같습니다!!

@juuuunny juuuunny merged commit 311213c into refactor-v1 Nov 27, 2024
@juuuunny juuuunny deleted the refactor/#78/user-refactoring branch November 27, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😆 JUNHYEONG 준형 Issue or PR 🔄 refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] : 코드 리뷰를 기반으로 1차 리팩토링을 진행한다
2 participants