코드 개선하기

오늘은 일부러 에러를 만들어놓은 코드를 리팩토링 및 개선하는 과제를 풀어보았다.
Git허브 주소

실행문제

첫번째로 실행부터 막혔었다.
그 이유는, 경로에 resource/application.yml이 존재하지 않아, jwt의 key를 가져올 수 없었기 때문이었다.

이를 해결한 후, 다시 실행을 눌러보았을때도 오류가 해결되지 않았다.
DB를 연결하는 코드도 작성하지 않았기 때문이었다.

그래서, 키는 https://randomkeygen.com/ 사이트에 접속하여 랜덤한 JWT key를 복사하여 가져왔고,
DB의 경우 원래 쓰던 MySQL계정과 연동시켜,

spring:
  datasource:
    url: jdbc:mysql://localhost:3306/mydb
    username: root
    password: 1234
    driver-class-name: com.mysql.cj.jdbc.Driver

jwt:
  secret:
    key: HLd0aVZ6e9YEHm33FcwBzuB4JY92T+aOpRH6KVSG/7k=

라는 application.yml 파일을 생성하였다.

코드 미사용 문제

public class AuthUserArgumentResolver implements HandlerMethodArgumentResolver {

    @Override
    public boolean supportsParameter(MethodParameter parameter) {
        boolean hasAuthAnnotation = parameter.getParameterAnnotation(Auth.class) != null;
        boolean isAuthUserType = parameter.getParameterType().equals(AuthUser.class);

        // @Auth 어노테이션과 AuthUser 타입이 함께 사용되지 않은 경우 예외 발생
        if (hasAuthAnnotation != isAuthUserType) {
            throw new AuthException("@Auth와 AuthUser 타입은 함께 사용되어야 합니다.");
        }

        return hasAuthAnnotation;
    }

    @Override
    public Object resolveArgument(
            @Nullable MethodParameter parameter,
            @Nullable ModelAndViewContainer mavContainer,
            NativeWebRequest webRequest,
            @Nullable WebDataBinderFactory binderFactory
    ) {
        HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();

        // JwtFilter 에서 set 한 userId, email, userRole 값을 가져옴
        Long userId = (Long) request.getAttribute("userId");
        String email = (String) request.getAttribute("email");
        UserRole userRole = UserRole.of((String) request.getAttribute("userRole"));

        return new AuthUser(userId, email, userRole);
    }
}

다음과 같은 코드에서 AuthUserArgumentResolver가 사용되지 않는다는 경고메시지가 출력되었다. 이는 Bean으로 등록하지 않아, 주입시에 Spring이 모르기 때문이라 판단하여 @Component 어노테이션을 활용해 Bean으로 등록해주었다.

불필요한 encode문제 해결

    @Transactional
    public SignupResponse signup(SignupRequest signupRequest) {
        
        String encodedPassword = passwordEncoder.encode(signupRequest.getPassword());

        UserRole userRole = UserRole.of(signupRequest.getUserRole());
        
        if (userRepository.existsByEmail(signupRequest.getEmail())) {
            throw new InvalidRequestException("이미 존재하는 이메일입니다.");
        }

위는 AuthService의 코드이다.
만약 Email이 존재하는 오류가 발생한다면, 비밀번호를 encode하는 동작이 불필요하다 판단하였다.

    @Transactional
    public SignupResponse signup(SignupRequest signupRequest) {

        if (userRepository.existsByEmail(signupRequest.getEmail())) {
            throw new InvalidRequestException("이미 존재하는 이메일입니다.");
        }
        String encodedPassword = passwordEncoder.encode(signupRequest.getPassword());

        UserRole userRole = UserRole.of(signupRequest.getUserRole());

이를 해결하기 위해, email을 먼저 검증하여 encode를 나중에 실행하게 하였다.

불필요한 if else문

WeatherDto[] weatherArray = responseEntity.getBody();
if (!HttpStatus.OK.equals(responseEntity.getStatusCode())) {
    throw new ServerException("날씨 데이터를 가져오는데 실패했습니다. 상태 코드: " + responseEntity.getStatusCode());
} else {
    if (weatherArray == null || weatherArray.length == 0) {
        throw new ServerException("날씨 데이터가 없습니다.");
    }
}

다음은 client.WeatherClient 코드의 일부분이다.
여기서 HTTP상태코드가 일치하지 않을경우에 weatherArray의 null여부, 길이를 다시한번 확인하는데, 이는 불필요한 if else문이라 판단하여,

        if (!HttpStatus.OK.equals(responseEntity.getStatusCode())) {
            throw new ServerException("날씨 데이터를 가져오는데 실패했습니다. 상태 코드: " + responseEntity.getStatusCode());
        }
        if (weatherArray == null || weatherArray.length == 0) throw new ServerException("날씨 데이터가 없습니다.");

이와같이 분리해주었다.

Dto에서 검증 수행하기

if (userChangePasswordRequest.getNewPassword().length() < 8 ||
        !userChangePasswordRequest.getNewPassword().matches("ㅅ") ||
        !userChangePasswordRequest.getNewPassword().matches(".*[A-Z].*")) {
    throw new InvalidRequestException("새 비밀번호는 8자 이상이어야 하고, 숫자와 대문자를 포함해야 합니다.");
}

주어진 코드는 Service내에서 비밀번호의 검증을 수행하였다.
비밀번호 형식을 검증하는 경우 Dto에서 수행하는것이 더 좋다 판단하여 해당 코드를 삭제하고
UserChangePasswordRequest에

public class UserChangePasswordRequest {

    @NotBlank
    private String oldPassword;

    @NotBlank
    @Size(min = 8, message = "새 비밀번호는 8자 이상이어야 합니다")
    @Pattern(regexp = "^(?=.*\\d)(?=.*[A-Z]).+$", message = "새 비밀번호는 숫자와 대문자를 포함해야 합니다")
    private String newPassword;
}

위와 같이 Validation을 사용하여 바꿔주었다.

N+1 다른방식으로 해결하기

    @Query("SELECT t FROM Todo t LEFT JOIN FETCH t.user u ORDER BY t.modifiedAt DESC")
    Page<Todo> findAllByOrderByModifiedAtDesc(Pageable pageable);

    @Query("SELECT t FROM Todo t " +
            "LEFT JOIN FETCH t.user " +
            "WHERE t.id = :todoId")
    Optional<Todo> findByIdWithUser(@Param("todoId") Long todoId);

기존의 코드는 @Query를 통해 fetch join으로 N+1문제를 해결하였다.

    @EntityGraph(attributePaths = "user")
    Page<Todo> findAllByOrderByModifiedAtDesc(Pageable pageable);

    @EntityGraph(attributePaths = "user")
    Optional<Todo> findById(Long todoId);

이를 다른방식으로도 해결해보고자 @EntityGraph 어노테이션을 활용, Service부분의 코드도 findByIdWithUser에서 findById로 수정하여 해결하였다.

테스트 수행하기

비밀번호 인코딩 테스트

    @InjectMocks
    private PasswordEncoder passwordEncoder;

    @Test
    void matches_메서드가_정상적으로_동작한다() {
        // given
        String rawPassword = "testPassword";
        String encodedPassword = passwordEncoder.encode(rawPassword);

        // when
        boolean matches = passwordEncoder.matches(encodedPassword, rawPassword);

        // then
        assertTrue(matches);
    }

비밀번호 인코드 과정을 테스트하는 코드이다.
matches의 인수를 반대로 적어놓아 테스트에 오류가 발생하였고, 둘의 순서를 rawPassword, encodedPassword로 바꿈으로서 해결하였다.

댓글 등록 테스트

@Test
public void comment_등록_중_할일을_찾지_못해_에러가_발생한다() {
    // given
    long todoId = 1;
    CommentSaveRequest request = new CommentSaveRequest("contents");
    AuthUser authUser = new AuthUser(1L, "email", UserRole.USER);

    given(todoRepository.findById(anyLong())).willReturn(Optional.empty());

    // when
    ServerException exception = assertThrows(ServerException.class, () -> {
        commentService.saveComment(authUser, todoId, request);
    });

    // then
    assertEquals("Todo not found", exception.getMessage());
}

다음의 테스트 코드이다. 댓글 생성 요청에서 발생하는 오류를 테스트 하는 코드이다.
하지만, `ServerException'은 클라이언트의 유효하지 않은 요청 상대로 사용하기에는 부적절하기 떄문에, ServerException 대신 InvalidRequestException을 사용하여 해결하였다.

테스트를 통한 로직 수정하기

@Test
    void todo의_user가_null인_경우_예외가_발생한다() {
        // given
        AuthUser authUser = new AuthUser(1L, "a@a.com", UserRole.USER);
        long todoId = 1L;
        long managerUserId = 2L;

        Todo todo = new Todo();
        ReflectionTestUtils.setField(todo, "user", null);

        ManagerSaveRequest managerSaveRequest = new ManagerSaveRequest(managerUserId);

        given(todoRepository.findById(todoId)).willReturn(Optional.of(todo));

        // when & then
        InvalidRequestException exception = assertThrows(InvalidRequestException.class, () ->
            managerService.saveManager(authUser, todoId, managerSaveRequest)
        );

        assertEquals("일정을 생성한 유저만 담당자를 지정할 수 있습니다.", exception.getMessage());
    }

해당 테스트 로직은 문제 없이 잘 동작해야 하지만, 막상 테스트를 실행시켜보면 그렇지 않다.
이는 팀원이 로직을 수정했을때 발생할수 있는 상황이다.

//        if (!ObjectUtils.nullSafeEquals(user.getId(), todo.getUser().getId())) {
//            throw new InvalidRequestException("일정을 생성한 유저만 담당자를 지정할 수 있습니다.");
//        }
        if (todo.getUser() == null || !ObjectUtils.nullSafeEquals(user.getId(), todo.getUser().getId())) {
            throw new InvalidRequestException("일정을 생성한 유저만 담당자를 지정할 수 있습니다.");
        }

이를 다음과 같이 수정하여, todo의 사용자가 null인 상황도 추가하여 해결하였다.

트러블 슈팅

오늘의 과제는 전체적으로 간단하여 어려운 부분이 딱히 존재하지 않았다.
제일 어려웠던 부분을 골라보자면 과제를 처음 시작했을때, 초기 세팅 자체가 안되어있어, 실행이 아예 안됐었다. 이를 해결하는 과정에서, InteliJ의 오류 로그를 살펴 보았고, JWT Key 및 DB세팅 문제라는것을 알게되어, application.yml을 추가함으로서 해결하였다.