[Spring 2기] CH3 심화 Spring_코드 개선 과제

오늘은 Spring 2기 CH3 심화 Spring_코드 개선 과제를 실시 해볼까 합니다.

 

우선 필수 과제부터 하나씩 살펴 볼까요? 이것을 하는 이유에 대해서 조금 설명 드리면 우리가 코드를 얼마나 알고 있냐? 이것이 아니라 우리가 코드 개선을 얼마나 할 수 있냐를 보는게 핵심 같았습니다. 꼭 필요한 과제인만큼 따라가고 싶은 마음은 굴뚝 같은데 아직 실력이 회사에 가서 뛸 만큼 능숙하지 않고 코드 해석도 그렇게 단단하지 않아서 마음이 좋지 않은것은 사실입니다. 

더 연습해서 확실하게 제것으로 만들어 보겠습니다. 시작하겠습니다.

 

Lv 0. 프로젝트 세팅 - 에러 분석

기존에 스프링을 실행 시키면 에러가 발생 했었습니다. 그 원인은 바로 resources 파일이 없었고 그것을 인코딩 할 파일이 없었기 떄문입니다. 가독성을 키우기 위해서는 application.yml을 사용하지만 저는 properties를 우선 사용 했습니다. 그래도 사용 가능하니 일단 패스 합시다.  

 

Lv 1. ArgumentResolver ( 살짝 헷갈릴수 있음 )

위 문제가 퀴즈 입니다. 일단 핵심만 먼저 체크 해보면 ArgumentResolver 클래스가 비활성화 된 상태여서 코드를 개선해야 됩니다.

그래서 저는 어노테이션을 사용하고 코드를 일정 개선 했습니다 

@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;
}

 

기존 코드는 원래 이렇게 사용 하도록 되었습니다. 지금 코드가 틀린것도 아니고 동작은 할수 있지만 올바른 ArgumentResolver 사용 방식은 아닙니다. supportsParmeter는 검증 메서드가 아닙니다. " 이 파라미터를 내가 처리할 수 있나? " 가 핵심인 메서드 입니다. 모든 ArgumentResolver에 대해 supportsParametet() 를 전부 호출 한다고 보면 됩니다. 

supportsParmeter 절대 원칙

예외 절대 금지

검증 로직 금지

return 내가 처리할 수 있냐? true : false; 이것만 사용하는게 핵심 입니다.

 

그래서 최종적으로 코드를 개선한 최종본 입니다.

@Component
public class AuthUserArgumentResolver
        implements HandlerMethodArgumentResolver {

    @Override
    public boolean supportsParameter(MethodParameter parameter) {
        return parameter.hasParameterAnnotation(Auth.class)
                && parameter.getParameterType().equals(AuthUser.class);
    }

    @Override
    public Object resolveArgument(
            @Nullable MethodParameter parameter,
            @Nullable ModelAndViewContainer mavContainer,
            NativeWebRequest webRequest,
            @Nullable WebDataBinderFactory binderFactory
    ) {

        HttpServletRequest request =
                (HttpServletRequest) webRequest.getNativeRequest();

        Long userId = (Long) request.getAttribute("userId");
        String email = (String) request.getAttribute("email");
        String role = (String) request.getAttribute("userRole");

        if (userId == null || email == null || role == null) {
            throw new AuthException("인증 정보가 존재하지 않습니다.");
        }

        return new AuthUser(
                userId,
                email,
                UserRole.of(role)
        );
    }
}

 

Lv 2. 코드 개선

  2-1 코드 개선 퀴즈 - Early Return 

- 해당 에러가 발생하는 상황일 때, passwordEncoder의 encode() 동작이 불필요하게 일어나지 않도록 코드를 해선하라.

 

기억하면 되는 흐름

검증(Validation)
   ↓
실패 조건 Early Return
   ↓
비싼 작업
   ↓
저장
   ↓
응답

기존 코드에서 흐름만 딱 바꿔 봣습니다.

@Transactional
public SignupResponse signup(SignupRequest signupRequest) {
    // 기존 흐름  >>  encode 먼저 활성화 >> 예외 처리 실행
    // 바꾼 흐름  >>  예최 처리 우선 실행 >> encode 활성화 리팩토링
    if (userRepository.existsByEmail(signupRequest.getEmail())) {
        throw new InvalidRequestException("이미 존재하는 이메일입니다.");
    }

    String encodedPassword = passwordEncoder.encode(signupRequest.getPassword());

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

    User newUser = new User(
            signupRequest.getEmail(),
            encodedPassword,
            userRole
    );
    User savedUser = userRepository.save(newUser);

    String bearerToken = jwtUtil.createToken(savedUser.getId(), savedUser.getEmail(), userRole);

    return new SignupResponse(bearerToken);
}

 

2-2 코드 개선 퀴즈 - 불필요한 if - else 피하기

실패 조건은 위에서 바로 끝내고 정상 흐름만 아래에 남긴다. 이것이 리팩토링 핵심 원칙 입니다.

❌ 불필요한 else 블록 제거하기

@Component
public class WeatherClient {

    private final RestTemplate restTemplate;

    public WeatherClient(RestTemplateBuilder builder) {
        this.restTemplate = builder.build();
    }

    public String getTodayWeather() {
        ResponseEntity<WeatherDto[]> responseEntity =
                restTemplate.getForEntity(buildWeatherApiUri(), WeatherDto[].class);

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

        WeatherDto[] weatherArray = responseEntity.getBody();

        if (weatherArray == null || weatherArray.length == 0) {
                throw new ServerException("날씨 데이터가 없습니다.");
        }

        String today = getCurrentDate();

        for (WeatherDto weatherDto : weatherArray) {
            if (today.equals(weatherDto.getDate())) {
                return weatherDto.getWeather();
            }
        }

        throw new ServerException("오늘에 해당하는 날씨 데이터를 찾을 수 없습니다.");
    }

 

이렇게 코드를 개선 했습니다. 

왜 개선해야 될까요 ? 

 

나쁜 코드  ❌

if (실패) {
    throw
} else {
    if (또 실패) {
        throw
    } else {
        정상 로직
    }
}

 

개선 코드 ✅

if (실패) throw;
if (실패) throw;

정상 로직만 아래에 둔다

 

Lv 3. 코드 개선 퀴즈 - Validation 

changePassword() 중 아래 코드 부분을 해당 API 요청 DTO에서 처리할 수 있게 개선하세요

우선 의존성을 확인하고 UserChangePasswordRequest Validation 어노에티션을 추가 합니다.

package org.example.expert.domain.user.dto.request;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Pattern;
import jakarta.validation.constraints.Size;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
@AllArgsConstructor
public class UserChangePasswordRequest {

    @NotBlank(message = "기존 비밀번호는 필수입니다.")
    private String oldPassword;

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

 

그리고 Sevice 코드를 개선하겠습니다.

package org.example.expert.domain.user.service;

import lombok.RequiredArgsConstructor;
import org.example.expert.config.PasswordEncoder;
import org.example.expert.domain.common.exception.InvalidRequestException;
import org.example.expert.domain.user.dto.request.UserChangePasswordRequest;
import org.example.expert.domain.user.dto.response.UserResponse;
import org.example.expert.domain.user.entity.User;
import org.example.expert.domain.user.repository.UserRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class UserService {

    private final UserRepository userRepository;
    private final PasswordEncoder passwordEncoder;

    @Transactional(readOnly = true)
    public UserResponse getUser(long userId) {
        User user = userRepository.findById(userId).orElseThrow(() -> new InvalidRequestException("User not found"));
        return new UserResponse(user.getId(), user.getEmail());
    }

    @Transactional
    public void changePassword(long userId, UserChangePasswordRequest userChangePasswordRequest) {

        User user = userRepository.findById(userId)
                .orElseThrow(() -> new InvalidRequestException("User not found"));

        if (passwordEncoder.matches(userChangePasswordRequest.getNewPassword(), user.getPassword())) {
            throw new InvalidRequestException("새 비밀번호는 기존 비밀번호와 같을 수 없습니다.");
        }

        if (!passwordEncoder.matches(userChangePasswordRequest.getOldPassword(), user.getPassword())) {
            throw new InvalidRequestException("잘못된 비밀번호입니다.");
        }

        user.changePassword(passwordEncoder.encode(userChangePasswordRequest.getNewPassword()));
    }
}

 

이러면 완료입니다. 

 

Lv 3-2. N+1 문제  

이 문제는 ai를 좀 사용했습니다. 

 

@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(Long todoId);

 

기존에 있던 코드를 개선 했습니다.

package org.example.expert.domain.todo.repository;

import org.example.expert.domain.todo.entity.Todo;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

import java.util.Optional;

public interface TodoRepository extends JpaRepository<Todo, Long> {

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

    @EntityGraph(attributePaths = {"user"})
    Optional<Todo> findByIdWithUser(@Param("todoId") Long todoId);

    int countById(Long todoId);
}

 

LV 4. 테스트코드 연습

4-1

 

package org.example.expert.config;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import static org.junit.jupiter.api.Assertions.assertTrue;

@ExtendWith(SpringExtension.class)
class PasswordEncoderTest {

    @InjectMocks
    private PasswordEncoder passwordEncoder;

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

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

        // then
        assertTrue(matches);
    }
}

 

 

4-2

 

Test ServicecommentTest를 인스턴스 테스트 해본 결과, 동작하지 않다는 것을 확인 했고 그 부분에 대해서 코드 수정이 필요하다고 판단 했습니다. 예외 과정에서 발생한 실제는 ServerException이 발생할 것이라고 기대했지만 실제로는 InvalidRequestException이 발생했습니다. 이건 테스트가 틀린게 아니라 기대한 예외 타입이 실제 코드와 다르다는 것을 알게 되었습니다. 이 코드를 실행하면 ServerException을 사용하겠다. 라는 코드를 InvalidRequestException으로 응답을 바꿧습니다.

 

    @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
        InvalidRequestException exception = assertThrows( InvalidRequestException.class,
                        () -> commentService.saveComment(authUser, todoId, request)
                );

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

잘 실행 됐음을 확인 했습니다.

 

4-3

 

기존 ManagerService 클래스를 확인 해보니까 이렇게 나와 있었습니다.

확인해보니까 InvalidRequestException을 기대하지만 NullPointerException이 응답하고 있는 상황이라 이제부터 이 문제를 해결해 볼까 합니다. 근데.. 아무리 봐도 .. 테스트 코드가 문제였나.. 의문이 들었습니다.  방법을 찾지 못하다가 키워드 하나를 발견하게 됐는데

 

→ todo.getUser() 가 null 이면
→ 예외를 던지기도 전에 NPE가 터짐

 

데이터를 조회 하다가 예외를 던지기도 전에 NPE가 터진다고 확인할 수 있었습니다. NPE가 안터지도록 방어 코드가 하나 있어야 된다는 것을 결국 알게 되었습니다. 그렇게 todo.getUser() 가 null일 경우 우선 예외를 하나 만들어 NPE가 터지지 않도록 코드를 개선 했습니다.

서비스는 비정상 입력이나 데이터 불일치 상황에서도 NPE 같은 런타임 오류로 죽지 말고, 의미 있는 도메인 예외를 던져야 하는것이 이번 과제 핵심이이였습니다. 기존 제 생각에서는 user는 nullable=false 인데.. 왜 체크를 했어야 하는지 의문이였습니다.하지만 방어 코드로 todo.getuser() == null 방어 코드를 만들어 null.getId()가 되어 NullPointException이 터지지 않도록 바꿔봤습니다.