[PJT] 2. ZIGLOG - MemberService에서의 문제

Park Yeongseo·2024년 1월 17일
1

Project

목록 보기
3/5
post-thumbnail

1. MemberService 인터페이스

MemberService 인터페이스는 회원 정보 조회, 수정과 관련된 서비스를 제공하는 인터페이스다.

public interface MemberService {  
  
    /**  
     * DTO 변환 로직을 포함하는 인터페이스  
     */  
    //사용자 닉네임 및 프로필 이미지를 변경  
    UserPublicInfoResponseDto modifyUserInfo(Member member, ModifyUserRequestDto modifyUserRequestDto) throws  UserNotFoundException, InvalidUserModificationRequestException;  
  
    //사용자의 공개 정보를 닉네임을 통해 조회  
    UserPublicInfoResponseDto getUserPublicInfoByNickname(String nickname) throws UserNotFoundException;  
  
    //닉네임 변경 시의 유효성을 검사  
    NicknameValidationResponseDto validateNickname(Member member, NicknameDto nicknameDto);  
  
    //현재 로그인한 회원의 정보를 조회  
    MyInfoResponseDto getLoginUserInfo(Member member) throws UserNotFoundException, FolderNotFoundException;  
  
    /**  
     * 엔티티와 그 필드만을 사용하는 함수 인터페이스  
     */  
    Member findUserByEmail(String email) throws UserNotFoundException;  
    Member findUserByNickname(String nickname) throws UserNotFoundException;  
    void modifyUserNickname(Member member, String nickname) throws UserNotFoundException, InvalidUserModificationRequestException;  
    void modifyUserProfile(Member member, String profileUrl) throws UserNotFoundException;  
    boolean isValidNickname(Member member, String nickname);  
    boolean isValidNicknameFormat(String nickname);  
    boolean isNotDuplicatedNickname(String nickname);  
  
    //테스트를 위한 회원가입 인터페이스. 실제로 사용되지는 않음  
    Member signUp(String email, String nickname) throws Exception;  
}

이 인터페이스에는 DTO를 인풋 또는 아웃풋으로 갖는 메서드들과, 엔티티와 그 필드만을 사용하는 메서드들이 있다. 하나의 메서드로 통합할 수 있음에도 이렇게 메서드들을 분리한 것은 위쪽의 DTO를 사용하는 함수들의 입출력 양식이 변하더라도 아래의 공통 기능들을 사용할 수 있게 하기 위함이다.

컨트롤러에서 호출해서 사용하는 위쪽 메서드들과, 기본적인 공통 기능들에 해당하는 아래쪽의 메서드들을 별개의 클래스로 분리해서 사용하는 것도 괜찮을 것 같기도 하다.

1-1. 닉네임 유효성 검사 메서드 수정 필요

아래는 사용자 닉네임을 변경할 때의 유효성 검사를 위한 기능들로, 형식 확인과 중복 확인을 거쳐 입력된 닉네임이 사용가능한지의 여부를 반환한다.

	NicknameValidationResponseDto validateNickname(Member member, NicknameDto nicknameDto);
    
    boolean isValidNickname(Member member, String nickname);  
    boolean isValidNicknameFormat(String nickname);  
    boolean isNotDuplicatedNickname(String nickname);  

구현부

    @Override
    public NicknameValidationResponseDto validateNickname(Member member, NicknameDto nicknameDto) {
        return NicknameValidationResponseDto.toDto(isValidNickname(member, nicknameDto.getNickname()));

    @Override
    public boolean isValidNickname(Member member, String nickname) {
        //기존에 사용하던 닉네임과 동일한지 여부를 확인하는 로직이 추가돼야 함
        if (member.getNickname().equals(nickname)) return true;
        if (!isValidNicknameFormat(nickname)) return false;
        return isNotDuplicatedNickname(nickname);
    }

	@Override
    public boolean isValidNicknameFormat(String nickname){
        String regex = "^[a-zA-Z0-9가-힣]{1,18}$";
        return nickname.matches(regex);
    }

	@Override
    public boolean isNotDuplicatedNickname(String nickname){
        return !memberRepository.existsMemberByNickname(nickname);
    }

앞서 말한 것과 같이 NicknameValidationResponseDto를 반환 타입으로 갖는 메서드 validateNickname()은 DTO를 입력 및 출력으로 가지고 있다. 이 NicknameValidationResponseDto 클래스는 해당 닉네임이 유효한지의 여부만을 필드로 가지는 래퍼 클래스다.

@Getter  
public class NicknameValidationResponseDto {  
    Boolean isValid;  
    private NicknameValidationResponseDto(Boolean isValid){  
        this.isValid = isValid;  
    }  
  
    public static NicknameValidationResponseDto toDto(Boolean isValid){  
        return new NicknameValidationResponseDto(isValid);  
    }  
}

isValidNickname(), isNotDuplicateNickname()isValidNickname()에서의 형식 확인, 중복 확인을 맡고 있는 내부 로직들이다. 이 함수들은 테스트를 하는 경우를 제외하고는 밖으로 노출되지 않는다. 원래는 private이었지만 이 각각에 대해서도 테스트를 하기 위해 접근 제한자를 private에서 public으로 바꿨다.

하지만 밖으로 노출되어 사용되지 않을 함수들을 인터페이스에 명시할 필요는 없으며, 서비스 단의 단위 테스트 또한 각각의 내부 로직 구현 함수들에 대해서 진행하기보다는 외부로 드러나는 isValidNickname()에 대해서만 진행하도록 변경하는 게 좋았을 것 같다.

지금은 각 함수들이 boolean을 반환해 해당 닉네임을 사용할 수 있는지의 여부만 알 수 있지만, 닉네임 유효성 검사에 실패했을 때 무엇 때문에 실패했는지를 알고 싶어질 경우가 있을 수도 있다. 각각의 함수가 다른 예외를 던지게 하고 isValidNickname()에서 받아 던지는 방식을 사용하면 되겠지만, 굳이 예외로 규정해 처리할 필요까지는 없는 것 같기도 하다.

1-2. 회원가입

해당 로직의 구현은 다음과 같이 돼있다. 그런데 사실 현재의 프로젝트에서 회원가입은 인증 서비스 단에서 이루어지고 있고(OAuth2 인증 후 가입되지 않은 회원인 경우를 처리하는 과정), 아래 메서드는 서비스 단위 테스트에서 임시 회원을 추가하기 위해서만 사용되고 있다. 따로 검증 로직이 포함되어 있지도 않다.

그런데 인증은 스프링 시큐리티 인증 필터에 맡기더라도 회원과 관련된 CRUD는 MemberService에서 처리하는 게 맞을 것 같다. 인증 및 회원 관리와 관련된 코드들을 전체적으로 바꿔야할 필요가 있어 보인다.

@Override //테스트에서만 사용됨  
public Member signUp(String email, String nickname) throws Exception{  
    Member member = memberRepository.save(  
            Member.builder()  
                    .email(email)  
                    .nickname(nickname)  
                    .password(UUID.randomUUID().toString())  
                    .build());  
  
    Folder root = folderRepository.save(Folder.builder()  
            .title("root")  
            .owner(member)  
            .build());  
  
    member.getFolders().add(root);  
    return member;  
}

2. N+1 문제

getLoginUserInfo()에서의 N+1문제

@Override  
public MyInfoResponseDto getLoginUserInfo(Member member) throws UserNotFoundException, FolderNotFoundException {  
    Member memberPersist = memberRepository.findById(member.getId()).orElseThrow(UserNotFoundException::new);  
  
    for (Folder folder : memberPersist.getFolders()){  
        if (folder.getParent() == null) return MyInfoResponseDto.toDto(memberPersist, folder);  
    }  
    throw new FolderNotFoundException();  
}

로그인한 사용자의 정보를 가져오는 메서드다. 반환되는 응답 DTO MyInfoResponseDto는 다음과 같다.

public class MyInfoResponseDto {  
  
    private String profileUrl;  
    private String nickname;  
    private Long rootFolderId;  
  
    public static MyInfoResponseDto toDto(Member member, Folder folder) {  
        return MyInfoResponseDto.builder()  
                .profileUrl(member.getProfileUrl())  
                .nickname(member.getNickname())  
                .rootFolderId(folder.getId())  
                .build();  
    }  
}

for문에서는 사용자 소유의 폴더들 중 부모 폴더가 없는, 즉 루트 폴더를 찾아서 반환하는데, 이렇게 하기 위해서 사용자 소유의 폴더 리스트를 모두 순회한다.폴더 리스트는 lazy fetch로, 처음 회원을 조회해올 때에는 쿼리가 발생하지 않지만, 리스트 내의 각 원소들에 접근할 때마다 쿼리가 새로 발생한다. 즉 N+1 문제가 발생한다.

public class Member {  
	//...
	
    //회원 입장에서는 양방향 쓸 일 없이. 루트 폴더만 알면 됨. 수정 필요  
    @OneToMany(mappedBy = "owner", cascade = CascadeType.REMOVE)  
    @Builder.Default  
    private List<Folder> folders = new ArrayList<>();  
  
    //...  
}
public class Folder {  
	//...   

    @ManyToOne(fetch = FetchType.LAZY, optional = false)  
    @JoinColumn(name = "MEMBER_ID")  
    private Member owner;  

	//...  
}

루트 폴더의 여부를 확인하기 위해서 해당 폴더의 부모가 null인지를 확인하는 방식을 채택한 것도 깔끔한 것 같지 않다. 루트 폴더와 그렇지 않은 것들을 다른 방식으로 구분지을 수 있게 만드는 게 좋을 것 같다.

profile
꾸준함, 기본기, 성찰, 공유

1개의 댓글

comment-user-thumbnail
2024년 1월 18일

아하!

답글 달기