Skip to content
Merged
Show file tree
Hide file tree
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 @@ -32,7 +32,7 @@ public class NotifyService {
retryFor = {CustomRuntimeException.class}
)
public void notify(LunaNotificationEvent event) {
String phone = retrievePhoneNumberByUserId(event.userId());
String phone = retrievePhoneNumberByUserId(event.userId()).replaceFirst("^.", "");
LunaNotificationVariable notifyVariable = notifyVariableFactory.create(event);
NotifySender<LunaNotificationVariable> sender = senderResolver.resolve(event.status());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ private static OAuthUserInfo ofKakao(final Map<String, Object> attributes,
String phoneNumber = Optional.ofNullable((String) account.get("phone_number"))
.map(p -> p.replace("+82 ", "0"))
.orElse(null);

Gender gender = Optional.ofNullable((String) account.get("gender"))
.map(g -> g.equalsIgnoreCase("male") ? Gender.MALE : Gender.FEMALE)
.orElse(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import life.mosu.mosuserver.domain.user.entity.UserRole;
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import life.mosu.mosuserver.global.processor.StepProcessor;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -23,7 +24,7 @@ public UserJpaEntity process(final OAuthUserInfo info) {
existingUser.updateOAuthUser(
info.gender(),
info.name(),
info.phoneNumber(),
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()),
info.birthDay(),
info.marketingAgreed());
return existingUser;
Expand All @@ -34,7 +35,7 @@ public UserJpaEntity process(final OAuthUserInfo info) {
.gender(info.gender())
.name(info.name())
.birth(info.birthDay())
.phoneNumber(info.phoneNumber())
.phoneNumber(PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.userRole(UserRole.ROLE_PENDING)
.provider(AuthProvider.KAKAO)
.agreedToMarketing(info.marketingAgreed())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public ChangePasswordResponse changePassword(ChangePasswordRequest request,
String phoneNumber) {

String rgxPhone = PhoneNumberUtil.formatPhoneNumberWithHyphen(phoneNumber);

UserJpaEntity user = userJpaRepository.findByPhoneNumber(rgxPhone)
UserJpaEntity user = userJpaRepository.findByPhoneNumber(
PhoneNumberUtil.formatPhoneNumber(rgxPhone))
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_NOT_FOUND));

user.changePassword(passwordEncode(encoder, request.newPassword()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,8 @@ public void grantUserRole() {
public void changePassword(String newPassword) {
this.password = newPassword;
}

public String getPhoneNumber() {
return phoneNumber.replaceFirst("^.", "");
}
Comment on lines +121 to +123

Choose a reason for hiding this comment

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

critical

Overriding a getter for a JPA entity field to return a modified value is an anti-pattern. It can cause unexpected behavior with persistence frameworks and breaks the principle of least astonishment. A getter should return the raw field value. It's recommended to rename this method to something like getRawPhoneNumber() and restore the default getter for phoneNumber (e.g., by letting Lombok generate it).

Additionally, the current implementation phoneNumber.replaceFirst("^.", "") will throw a NullPointerException if phoneNumber is null, as the field is nullable.

Here is a safer implementation with a more appropriate method name:

    public String getRawPhoneNumber() {
        if (phoneNumber == null || phoneNumber.isEmpty()) {
            return phoneNumber;
        }
        return phoneNumber.substring(1);
    }

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,12 @@ public static String formatPhoneNumberWithHyphen(String phoneNumber) {
phoneNumber.substring(3, 7),
phoneNumber.substring(7));
}

public static String formatGuestPhoneNumber(String phoneNumber) {
return String.format("G%s", phoneNumber);
}

public static String formatPhoneNumber(String phoneNumber) {
return String.format("U%s", phoneNumber);
}
Comment on lines +23 to +25

Choose a reason for hiding this comment

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

medium

To avoid duplicating the logic for removing the phone number prefix (as seen in NotifyService and UserJpaEntity), and to improve readability over replaceFirst("^.", ""), consider adding a utility method here to handle that.

This centralizes the logic and makes the code easier to maintain. You can then use PhoneNumberUtil.stripPrefix() in other parts of the code.

    public static String formatPhoneNumber(String phoneNumber) {
        return String.format("U%s", phoneNumber);
    }

    public static String stripPrefix(String phoneNumber) {
        if (phoneNumber == null || phoneNumber.isEmpty()) {
            return phoneNumber;
        }
        return phoneNumber.substring(1);
    }

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import life.mosu.mosuserver.global.annotation.NotBlankPhoneNumberPattern;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import life.mosu.mosuserver.presentation.common.FileRequest;

public record ApplicationGuestRequest(
Expand Down Expand Up @@ -44,7 +45,7 @@ public UserJpaEntity toUserJpaEntity() {
.password(randomPassword)
.name(userName)
.birth(birth)
.phoneNumber(phoneNumber)
.phoneNumber(PhoneNumberUtil.formatGuestPhoneNumber(phoneNumber))
.gender(validGender())
.userRole(UserRole.ROLE_USER)
.provider(AuthProvider.MOSU)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import life.mosu.mosuserver.global.annotation.LoginIdPattern;
import life.mosu.mosuserver.global.annotation.NotBlankPhoneNumberPattern;
import life.mosu.mosuserver.global.annotation.PasswordPattern;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import org.springframework.security.crypto.password.PasswordEncoder;

public record SignUpAccountRequest(
Expand Down Expand Up @@ -58,7 +59,7 @@ public UserJpaEntity toAuthEntity(PasswordEncoder passwordEncoder) {
.agreedToMarketing(serviceTermRequest.agreedToMarketing())
.gender(Gender.fromName(gender))
.name(userName)
.phoneNumber(phoneNumber)
.phoneNumber(PhoneNumberUtil.formatPhoneNumber(phoneNumber))
.birth(birth)
.provider(AuthProvider.MOSU)
.userRole(UserRole.ROLE_PENDING)
Expand Down