Lv.6 리팩토링을 리팩토링하기

1. dto에서 검증로직을 수행하는 것은 뭔가 조금 애매?

@Value
public class SigninRequestDto {

    String email;
    String password;

    public SigninRequestDto(String email, String password) {
        this.email = email;
        this.password = password;

        validate();
    }

    private void validate() {
        if (email == null || !email.matches("^[\\w.-]+@[\\w.-]+\\.[a-zA-Z]{2,}$")) {
            throw new IllegalArgumentException("유효하지 않은 이메일 형식입니다.");
        }
        if (password == null || password.isBlank()) {
            throw new IllegalArgumentException("비밀번호는 필수입니다.");
        }
    }
}

dto는 값을 전달만 했으면 좋겠다!!!!!! 는 의견이 있었다. 맞는말이다. 지금 dto간에 중복되는 validate 로직도 많고.. 앞으로 requestDto가 늘어난다면 복잡해질거다.

해결법은, service 계층에 validate로직을 두고 controller 에서 그를 주입받아 사용하는 것이다 !

@Component
public class RequestValidator {

    public UserRole userRoleConvert(String userRole) {
        return Arrays.stream(UserRole.values())
                .filter(r -> r.name().equalsIgnoreCase(userRole))
                .findFirst()
                .orElseThrow(() -> new IllegalArgumentException("유효하지 않은 UserRole입니다."));
    }

    public void password(String password) {
        if (password == null || password.length() < 8 || !password.matches(".*\\d.*") || !password.matches(".*[A-Z].*")) {
            throw new IllegalArgumentException("비밀번호는 8자 이상이어야 하며, 숫자와 대문자를 포함해야 합니다.");
        }
    }

    public void email(String email) {
        if (email == null || !email.matches("^[\\w.-]+@[\\w.-]+\\.[a-zA-Z]{2,}$")) {
            throw new IllegalArgumentException("유효하지 않은 이메일 형식입니다.");
        }
    }

    public void isPasswordNull(String password) {
        if (password == null || password.isBlank()) {
            throw new IllegalArgumentException("비밀번호는 필수입니다.");
        }
    }

    public void isUserRoleNull(String userRole) {
        if (userRole == null || userRole.isBlank()) {
            throw new IllegalArgumentException("유저 역할은 필수입니다.");
        }
    }

    public void changePassword(String oldPassword, String newPassword) {
        if (oldPassword == null || oldPassword.isBlank()) {
            throw new IllegalArgumentException("현재 비밀번호는 필수입니다.");
        }
        if (newPassword == null || newPassword.isBlank()) {
            throw new IllegalArgumentException("새 비밀번호는 필수입니다.");
        }
        if(newPassword.equals(oldPassword)) {
            throw new IllegalArgumentException("현재 비밀번호와 새 비밀번호가 같을 수 없습니다.");
        }
    }
}

값의 검증만을 담당하는 requestValidator를 서비스 계층에 만들었다!

이제 controller 에서 이를 주입받아 사용해보자.

@RestController
@RequiredArgsConstructor
public class AuthController {

    private final AuthManager authManager;
    private final RequestValidator requestValidator;

    @PostMapping("/auth/signup")
    public ResponseEntity<String> signup(
            @Valid @RequestBody final SignupRequestDto dto
    ) {
        requestValidator.isPasswordNull(dto.password());
        requestValidator.isUserRoleNull(dto.userRole());
        requestValidator.email(dto.email());
        requestValidator.password(dto.password());
        requestValidator.userRole(dto.userRole());

        String token = authManager.handleSignup(dto);
        return ResponseEntity.ok(token);
    }

    @PostMapping("/auth/signin")
    public ResponseEntity<String> signin(
            @Valid @RequestBody final SigninRequestDto dto
    ) {
        requestValidator.email(dto.email());
        requestValidator.isPasswordNull(dto.password());

        String token = authManager.handleSignin(dto);
        return ResponseEntity.ok(token);
    }
}

흠.. 근데 코드의 길이가 너무 길어졌다. 어떻게 해결할 수 있을까? 저 긴 로직을 어떻게든 치워버리고싶다.

해결법은, 이전에 사용했던 authmanager처럼 validatemanager를 따로 생성하는 거다!

@Component
@RequiredArgsConstructor
public class ValidateManager {

    private final RequestValidator requestValidator;

    public void validateSignup(String email, String password, UserRole userRole) {
        requestValidator.isPasswordNull(password);
        requestValidator.isUserRoleNull(userRole);
        requestValidator.email(email);
        requestValidator.password(password);
        requestValidator.userRole(userRole);
    }

    public void validateSignin(String email, String password) {
        requestValidator.email(email);
        requestValidator.isPasswordNull(password);
    }

    public void validateChangePassword(String oldPassword, String newPassword) {
        requestValidator.isPasswordNull(oldPassword);
        requestValidator.isPasswordNull(newPassword);
        requestValidator.changePassword(oldPassword, newPassword);
    }

    public void validateUserRole(UserRole userRole) {
        requestValidator.isUserRoleNull(userRole);
        requestValidator.userRole(userRole);
    }
}

이제 validate로직이 추가되면 이곳만 건드리면 될 것이다. OCP를 준수하게 되었다!

그리고, dto클래스도 record로 바꿔주었다

public record SigninRequestDto(String email, String password) {
}

@JsonInclude(JsonInclude.Include.NON_NULL)
public record UserResponse(Long id, String email) {

}

이제 완전 값을 받아오는 일만 하는놈이 되었다. response또한 마찬가지로 레코드로 바꿨당

2. exception 세분화

현재의 exception은 세분화되어있지 않고 common에서 모든 걸 불러와서 사용중이다. enum이라던가.. enum이라던가를 사용해서 좀 더 세분화 시켜야겠다.

public abstract class BaseException extends RuntimeException {

    private final String message;
    private final HttpStatus httpStatus;

    protected BaseException(String message, HttpStatus httpStatus) {
        super(message);
        this.message = message;
        this.httpStatus = httpStatus;
    }

    public HttpStatus getStatus(){
        return httpStatus;
    }

    @Override
    public String getMessage(){
        return message;
    }
}

@Getter
@AllArgsConstructor
public enum ErrorCode {

    //400
    PAGING_ERROR(HttpStatus.BAD_REQUEST, "페이지 입력값이 잘못되었습니다."),
    ALREADY_USED_EMAIL(HttpStatus.BAD_REQUEST, "이미 사용중인 이메일입니다."),
    WRONG_PASSWORD(HttpStatus.BAD_REQUEST, "비밀번호를 잘못 입력하였습니다."),
    USER_NOT_FOUND(HttpStatus.BAD_REQUEST, "존재하지 않는 사용자입니다."),
    NICKNAME_REQUIRED(HttpStatus.BAD_REQUEST, "닉네임 입력값이 잘못되었습니다."),
    PASSWORD_REQUIRED(HttpStatus.BAD_REQUEST, "비밀번호 입력값이 잘못되었습니다."),

우선 baseException과 에러코드 enum을 만들어서 전체 exception의 틀을 정해줬다.

@RestControllerAdvice
public class GlobalExceptionHandler {

    @ExceptionHandler(InvalidRequestException.class)
    public ResponseEntity<Map<String, Object>> invalidRequestExceptionException(InvalidRequestException ex) {
        HttpStatus status = HttpStatus.BAD_REQUEST;
        return getErrorResponse(status, ex.getMessage());
    }

    @ExceptionHandler(AuthException.class)
    public ResponseEntity<Map<String, Object>> handleAuthException(AuthException ex) {
        HttpStatus status = HttpStatus.UNAUTHORIZED;
        return getErrorResponse(status, ex.getMessage());
    }

    @ExceptionHandler(ServerException.class)
    public ResponseEntity<Map<String, Object>> handleServerException(ServerException ex) {
        HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR;
        return getErrorResponse(status, ex.getMessage());
    }

    @ExceptionHandler(ForbiddenException.class)
    public ResponseEntity<Map<String, Object>> handleForbiddenException(ForbiddenException ex) {
        HttpStatus status = HttpStatus.FORBIDDEN;
        return getErrorResponse(status, ex.getMessage());
    }

    public ResponseEntity<Map<String, Object>> getErrorResponse(HttpStatus status, String message) {
        Map<String, Object> errorResponse = new HashMap<>();
        errorResponse.put("status", status.name());
        errorResponse.put("code", status.value());
        errorResponse.put("message", message);

        return new ResponseEntity<>(errorResponse, status);
    }
}

이처럼 현재의 exception은 하나를 추가할 때 마다 globalexceptionhandler에서 새로운 설정(status라던가)를 추가해줘야해서 매우 불편했다.

@RestControllerAdvice
public class GlobalExceptionHandler {

    @ExceptionHandler(BaseException.class)
    public ResponseEntity<Map<String, Object>> handleBaseException(BaseException e){
        return buildErrorResponse(e.getStatus(), e.getMessage(), null);
    }

    private ResponseEntity<Map<String, Object>> buildErrorResponse(HttpStatus status, String message, Object errors){
        Map<String, Object> response = new HashMap<>();
        response.put("status", status.value());
        response.put("error", status.getReasonPhrase());
        response.put("message", message);
        if(errors != null){
            response.put("errors", errors);
        }
        return ResponseEntity.status(status).body(response);
    }
}

BaseException만을 사용해 코드를 간소화시켰다.