Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@
import life.mosu.mosuserver.domain.user.entity.UserJpaEntity;
import life.mosu.mosuserver.domain.user.entity.UserRole;
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.global.processor.StepProcessor;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import lombok.RequiredArgsConstructor;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -22,32 +19,29 @@ public class OAuthUserPersistenceProcessor implements StepProcessor<OAuthUserInf
@Override
@Transactional
public UserJpaEntity process(final OAuthUserInfo info) {
try {
return userRepository.findByPhoneNumber(info.phoneNumber())
.map(existingUser -> {
existingUser.updateOAuthUser(
info.gender(),
info.name(),
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()),
info.birthDay(),
info.marketingAgreed());
return existingUser;
})
.orElseGet(() -> {
final UserJpaEntity newUser = UserJpaEntity.builder()
.loginId(info.email())
.gender(info.gender())
.name(info.name())
.birth(info.birthDay())
.phoneNumber(PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.userRole(UserRole.ROLE_PENDING)
.provider(AuthProvider.KAKAO)
.agreedToMarketing(info.marketingAgreed())
.build();
return userRepository.save(newUser);
});
} catch (DataIntegrityViolationException ex) {
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
}
return userRepository.findByPhoneNumber(
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.map(existingUser -> {
existingUser.updateOAuthUser(
info.gender(),
info.name(),
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()),
info.birthDay(),
info.marketingAgreed());
return existingUser;
})
.orElseGet(() -> {
final UserJpaEntity newUser = UserJpaEntity.builder()
.loginId(info.email())
.gender(info.gender())
.name(info.name())
.birth(info.birthDay())
.phoneNumber(PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.userRole(UserRole.ROLE_PENDING)
.provider(AuthProvider.KAKAO)
.agreedToMarketing(info.marketingAgreed())
.build();
return userRepository.save(newUser);
});
Comment on lines +22 to +45

Choose a reason for hiding this comment

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

high

이 메서드에는 두 가지 개선점이 있습니다.

  1. PhoneNumberUtil.formatPhoneNumber(info.phoneNumber())가 여러 번 호출되고 있습니다. 포맷된 전화번호를 변수에 저장하여 재사용하면 코드가 더 깔끔해지고 중복 호출을 피할 수 있습니다.

  2. 이전 코드에 있던 DataIntegrityViolationException에 대한 try-catch 블록이 제거되었습니다. 사용자를 전화번호로 먼저 조회하더라도, 다른 사용자가 동일한 loginId(이메일)를 가지고 있을 경우 userRepository.save()에서 DataIntegrityViolationException이 발생할 수 있습니다. 이 예외를 처리하여 USER_ALREADY_EXISTS와 같은 명확한 오류를 반환하는 것이 좋습니다.

아래와 같이 수정하는 것을 제안합니다. 이 변경을 적용하려면 제거된 DataIntegrityViolationException, CustomRuntimeException, ErrorCode import를 다시 추가해야 합니다.

        final String formattedPhoneNumber = PhoneNumberUtil.formatPhoneNumber(info.phoneNumber());
        try {
            return userRepository.findByPhoneNumber(formattedPhoneNumber)
                    .map(existingUser -> {
                        existingUser.updateOAuthUser(
                                info.gender(),
                                info.name(),
                                formattedPhoneNumber,
                                info.birthDay(),
                                info.marketingAgreed());
                        return existingUser;
                    })
                    .orElseGet(() -> {
                        final UserJpaEntity newUser = UserJpaEntity.builder()
                                .loginId(info.email())
                                .gender(info.gender())
                                .name(info.name())
                                .birth(info.birthDay())
                                .phoneNumber(formattedPhoneNumber)
                                .userRole(UserRole.ROLE_PENDING)
                                .provider(AuthProvider.KAKAO)
                                .agreedToMarketing(info.marketingAgreed())
                                .build();
                        return userRepository.save(newUser);
                    });
        } catch (org.springframework.dao.DataIntegrityViolationException ex) {
            throw new life.mosu.mosuserver.global.exception.CustomRuntimeException(life.mosu.mosuserver.global.exception.ErrorCode.USER_ALREADY_EXISTS);
        }

}
}