본문 바로가기
개인 공부용 프로젝트/expert.refactor.project

Lv.3-4 & Lv.6 전체적인 리팩토링 일기

by pon9 2024. 12. 31.
과정 미리보기 ㅋㅋ

문제

1. response가 과도하게 많아 관리하기 힘드며, entity에서 response로 변환하는 과정을 response에 대한 생성자로 처리해 유지보수 하기 힘들다.

 

AuthService
UserService

2. service계층에서 처리하고 있는 일이 너무 많다. token 생성, entity -> dto 로 변환작업, 비즈니스 로직, request 에
대한 validate로직.. SRP를 완벽하게 위반하고 있다.
 

CommentService

3. 예외처리가 미흡하다.

 
 

1. 과도하게 많은 response 압축과 변환과정 간소화

public interface Mapper<E, S> {

    S toDto(E entity);
}

@Component
public class UserMapper implements Mapper<User, UserResponse> {

    @Override
    public UserResponse toDto(User entity) {
        return new UserResponse(
                entity.getId(),
                entity.getEmail()
        );
    }
}

@Getter
@JsonInclude(JsonInclude.Include.NON_NULL)
@RequiredArgsConstructor
public class UserResponse {

    private final Long id;
    private final String email;
}

Mapper 인터페이스를 두어 entity -> dto 로직을 통일되게 만들었다.
여러 개의 responseDto를 하나로 줄이고 JsonInclude를 사용해 값이 null이라면 직렬화 되지 않도록 만들었다.
 
그런데 아직 다형성을 이용해 여러 형태의 값을 반환하도록 하는 로직은 만들지 않아서 테스트 해보고 필요한 값만 반환하도록 살짝 수정해야 한다.
 
 

2. 굉장하게 위반된 SRP 풀어주기

진지하게 굉장히 엄청나게 위반됐다

@Value
public class SignupVo {

    String email;
    String password;
    UserRole userRole;

    public SignupVo(String email, String password, String userRole) {
        this.email = email;
        this.password = password;
        this.userRole = validateAndConvert(userRole);

        validate();
    }

    private void validate() {
        if (email == null || !email.matches("^[\\w.-]+@[\\w.-]+\\.[a-zA-Z]{2,}$")) {
            throw new IllegalArgumentException("유효하지 않은 이메일 형식입니다.");
        }
        if (password == null || password.length() < 8 || !password.matches(".*\\d.*") || !password.matches(".*[A-Z].*")) {
            throw new IllegalArgumentException("비밀번호는 8자 이상이어야 하며, 숫자와 대문자를 포함해야 합니다.");
        }
        if (userRole == null) {
            throw new IllegalArgumentException("유저 역할은 필수입니다.");
        }
    }

    private UserRole validateAndConvert(String role) {
        if (role == null || role.isBlank()) {
            throw new IllegalArgumentException("UserRole은 비어 있을 수 없습니다.");
        }
        return Arrays.stream(UserRole.values())
                .filter(r -> r.name().equalsIgnoreCase(role))
                .findFirst()
                .orElseThrow(() -> new IllegalArgumentException("유효하지 않은 UserRole입니다."));
    }
}

우선 requestDto를 value object로 만들어서 생성 시에 validate 로직을 수행하도록 만들어 service 계층의 코드를 간소화시켰다.

@Component
@RequiredArgsConstructor
public class AuthManager {

    private final AuthService authService;
    private final TokenProvider tokenProvider;

    public String handleSignup(SignupVo vo) {
        User user = authService.signup(vo);
        return tokenProvider.createToken(user.getId(), user.getEmail(), user.getUserRole());
    }

    public String handleSignin(SigninVo vo) {
        User user = authService.signin(vo);
        return tokenProvider.createToken(user.getId(), user.getEmail(), user.getUserRole());
    }
}

signup과 signin과정에서 token이 함께 생성되는데 이것을 manager클래스로 따로 만들어 authservice가 너무 많은 책임을 갖지 않도록 풀어주었다
 
제일 고민되었던 건 UserRole enum인데, 원래 enum안에 validate 로직이 존재하고 userrole을 어딘가에서 생성 / 사용 할 때마다 해당 로직이 호출되었다.
하지만 이미 회원가입 된 사람의(이미 검증된 userrole에 대한) token 생성 등의 로직에서 해당 validate가 실행되는 것은 불필요하다고 느꼈기에, SRP도 준수할 겸 value object에서만 validate를 수행하도록 하였다.
 
근데 고민하다가 이름을 그냥 vo에서 dto로 바꿨다. 지금 내 value 클래스는 dto로서의 역할을 더 수행하는 듯 하다
아무튼 이름만 바꿔줬당 ㅇㅅㅇ
 
 

3. 검증 과정 추가(existsById)

@Component
public class EntityValidator {

    public static <T> void isExistsById(JpaRepository<T, Long> repository, Long id, String errorMessage) {
        if(id == null){
            throw new InvalidRequestException("id is null");
        }
        if(!repository.existsById(id)) {
            throw new InvalidRequestException(errorMessage);
        }
    }
}

public List<Comment> getComments(long todoId) {
    EntityValidator.isExistsById(todoRepository, todoId, "Todo not found");
    return commentRepository.findAllByTodoId(todoId);
}

필요한 검증 로직이 보이지 않아서 추가했다. getComment를 하는데 해당Todo가 존재하지 않으면 굉장히 곤란할 것이다. generic을 사용해 여러 곳에서 편리하게 사용하도록 해봤다.
 
 

완성 된 패키지 구조

 
application: 비즈니스 로직만을 담당한다.
domain: 도메인 객체를 정의하고, 그의 생성 로직만을 담당한다.
infrastructure: 기술적인, global하게 적용되는 것들을 담고있다.
interfaces: 사용자와 직접 소통하는 객체를 선언하고, 요청을 처리하며 반환하는 책임만 갖고 있다.
 

/*TODO
전역 예외처리 추가, 테스트코드 작성
주석, aop와 인터셉터를 이용한 로깅
양방향 없애며 생긴 fk 딜리트 로직 추가하기
response 반환값 재설정 하기
 */

이제 이거만(?) 하면 된당 ㅎ 책임분리를 잘(한거 맞겠지) 해둬서 단순 추가 수정만 하면 될 것이다.