[내일배움캠프 Spring 3기] CH 3 심화 Spring 코드 개선 과제 - 필수 기능

프로젝트 목표 요약

• Lv0: 애플리케이션 실행을 위한 JWT 키/DB 설정
• Lv1: @Auth AuthUser 파라미터 바인딩을 위한 ArgumentResolver 등록
• Lv2: 회원가입/로그인 및 비밀번호 검증 로직 리팩토링(가독성/책임 분리)
• Lv3: N+1 해결 (fetch join -> EntityGraph로 교체)
• Lv4: 테스트 코드 수정 + 서비스 로직 수정으로 테스트 실패 원인 제거

---

Lv0. 애플리케이션 실행 (JWT 키 / DB 설정)

문제 상황

처음 프로젝트 실행을 위해 application.yml에 JWT secret, datasource 설정이 필요했다.

예시:

jwt:
  secret:
    key: (base64 key)

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

jpa:
  hibernate:
    ddl-auto: create

실행 로그에서 Tomcat 8080, Hibernate, HikariCP 모두 정상 기동되면 Lv0은 통과로 볼 수 있다.

---

트러블슈팅 1)

처음에는 application-local.yml에 DB 비밀번호/JWT 키를 그대로 넣고 커밋해버렸다.
그 결과 GitHub 커밋 diff에서 그대로 노출되는 걸 확인했다.

내가 선택한 방식 (노출 방지)

• 민감 정보는 로컬 전용 파일(application-local.yml) 로 관리
• 깃에는 환경변수 placeholder만 남김
• 그리고 이미 올라간 히스토리는 git filter-repo로 정리

application.yml은 이렇게 바꾸었다:

jwt:
  secret:
    key: ${JWT_SECRET_KEY}

spring:
  datasource:
    url: ${DB_URL}
    username: ${DB_USERNAME}
    password: ${DB_PASSWORD}

그리고 application-local.yml은 .gitignore에 추가해서 추적 제외.

히스토리에서 파일 완전 제거

이미 커밋 히스토리에 비밀번호가 남으면 “파일을 삭제했더라도” 과거 커밋에서 볼 수 있다.
그래서 아래처럼 히스토리에서 제거했다.

git filter-repo --force --path src/main/resources/application-local.yml --invert-paths

이 과정에서 origin remote가 자동 제거되는 이슈가 있었고, remote를 다시 잡아야 push가 가능했다.

(이건 다음에 다시 같은 실수 안 하려고 꼭 기록해둔다.)

---

Lv1. ArgumentResolver 등록 (AuthUser 주입)

요구사항

컨트롤러에서 이렇게 받을 수 있어야 한다:

@PostMapping("/todos")
public ResponseEntity<TodoSaveResponse> saveTodo(
        @Auth AuthUser authUser,
        @Valid @RequestBody TodoSaveRequest todoSaveRequest
) { ... }

즉, @Auth + AuthUser 조합으로 들어오면
필터에서 넣어둔 request attribute를 꺼내서 AuthUser로 만들어 주입해야 한다.

---

트러블슈팅 2) Resolver 만들었는데 왜 동작을 안 하지?

원인

HandlerMethodArgumentResolver는 구현만 하면 자동으로 적용되지 않는다.
반드시 WebMvcConfigurer에서 등록해야 한다.

등록을 빼먹으면 컨트롤러에서 @Auth AuthUser를 받을 때 바인딩이 안 되거나,
예상치 못한 에러가 발생한다.

---

해결: WebConfig에 Resolver 등록

@Configuration
public class WebConfig implements WebMvcConfigurer {
    @Override
    public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers){
        resolvers.add(new AuthUserArgumentResolver());
    }
}

---

AuthUserArgumentResolver 구현 의도

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(...) {
        HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();

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

내가 이렇게 짠 이유

• supportsParameter에서 @Auth와 AuthUser를 강제 묶음 처리
  -> 컨트롤러 개발자가 실수로 @Auth String email 같은 형태로 쓰면 즉시 예외로 알려줌
• resolveArgument는 JwtFilter에서 저장한 attribute를 그대로 꺼내 인증 정보 전달 책임을 한 곳에 모음

---

Lv2. 코드 개선 (리팩토링)

1) Early Return 적용 (Signup 로직 개선)

문제

기존 코드에서는 조건문이 중첩되는 구조였다.

예를 들어 이메일 중복 체크 같은 로직에서 if 블록 안에 로직이 계속 들어가는 형태였다.

이 방식은 코드가 길어질수록 다음과 같은 문제가 발생한다.

• 조건문이 깊어짐 (Nested if)
• 핵심 로직 파악이 어려움
• 가독성이 떨어짐

---

해결: Early Return 적용

Early Return은 조건이 만족되지 않으면 바로 return 또는 예외를 던지는 방식이다.

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

그 다음에 정상 로직을 바로 진행한다.

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

---

Early Return을 적용한 이유

Early Return을 사용하면 다음 장점이 있다.

• 코드 깊이가 줄어든다
• 정상 흐름이 더 잘 보인다
• 예외 케이스를 먼저 처리할 수 있다

즉,

"예외 상황 먼저 처리 -> 정상 로직 실행"

이라는 구조가 만들어진다.

이 패턴은 Spring 서비스 로직에서 많이 사용하는 패턴이다.

---

2) WeatherClient 불필요한 if-else 제거

WeatherClient 코드에서 다음과 같은 구조가 있었다.

if (!HttpStatus.OK.equals(responseEntity.getStatusCode())) {
    throw new ServerException(...);
} else {
    if (weatherArray == null || weatherArray.length == 0) {
        throw new ServerException(...);
    }
}

여기서 문제는

• 불필요한 else 블록
• 조건문 중첩
• 가독성 저하

였다.

---

개선 방법

불필요한 else를 제거하고 조건을 분리했다.

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

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

---

이렇게 변경한 이유

불필요한 else는 코드 가독성을 떨어뜨린다.

조건이 실패하면 바로 예외를 던지고
정상 흐름을 이어가는 방식이 더 자연스럽다.

이 역시 Early Fail / Guard Clause 패턴이라고 한다.

---

3) 비밀번호 검증 로직을 DTO Validation으로 이동

기존 코드에서는 비밀번호 검증을 서비스 레이어에서 직접 수행하고 있었다.

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

이 방식은 다음 문제가 있다.

• 서비스 로직이 길어짐
• 검증 로직이 여러 서비스에 흩어질 수 있음
• 재사용성이 떨어짐

---

해결: DTO Validation 사용

DTO에 Validation을 적용했다.

@NotBlank
private String newPassword;

그리고 Controller에서

@Valid

를 사용하여 자동 검증하도록 했다.

---

Validation을 DTO로 옮긴 이유

Spring에서는 입력값 검증을 DTO에서 처리하는 것이 일반적인 패턴이다.

이렇게 하면

• 서비스 로직이 단순해짐
• 책임 분리 가능
• 코드 재사용 가능
• 유지보수성 증가

---

Lv2 코드 개선을 통해 얻은 것

이번 리팩토링을 통해 단순히 기능 구현을 넘어서

• 가독성 있는 코드 작성 방법
• 서비스 로직을 단순하게 유지하는 방법
• Spring Validation의 활용 방식

을 이해할 수 있었다.

특히,

"비즈니스 로직과 검증 로직을 분리하는 것"

이 서비스 레이어 설계에서 매우 중요하다는 것을 느꼈다.

---

Lv3. N+1 해결 (fetch join -> EntityGraph로 변경)

문제 시나리오

getTodos()에서 Todo 목록 조회 후 todo.getUser()를 접근한다.

Todo.user는 LAZY이기 때문에,
Todo N개 조회 후 user를 접근하는 순간 N번 추가 쿼리가 발생할 수 있다 -> N+1.

기존 코드는 JPQL fetch join을 사용하고 있었다:

public interface TodoRepository extends JpaRepository<Todo, Long> {

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

    int countById(Long todoId);
}

---

요구사항: EntityGraph로 동일 동작 구현

EntityGraph 방식 예시 (최종 목표 형태)

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

그리고 단건 조회도 동일하게:

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

주의: @EntityGraph는 “연관 엔티티를 함께 로딩”하도록 힌트를 주는 방식이고,
fetch join처럼 직접 JPQL을 쓰지 않아도 N+1을 방지할 수 있다.

---

Lv4. 테스트 코드 수정 + 서비스 로직 보완

Lv4는 “코드가 맞는데 테스트가 틀림 / 테스트는 맞는데 로직이 바뀌어서 깨짐” 같은 상황을 직접 다루게 했다.

---

(1) PasswordEncoderTest 수정

문제

matches() 메서드 파라미터 순서를 잘못 넣으면 테스트가 실패한다.

정상은:

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

encoded를 raw처럼 넣으면 당연히 비교가 안 맞는다.

---

(2) ManagerServiceTest: Todo 없을 때 NPE가 아니라 InvalidRequestException

내가 고친 방향

• 테스트 메서드명부터 컨텍스트에 맞게 수정
  (NPE가 아니라 InvalidRequestException을 기대해야 함)
• 예외 타입 + 메시지를 동시에 검증하도록 assertThatThrownBy를 사용

assertThatThrownBy(() -> managerService.getManagers(todoId))
    .isInstanceOf(InvalidRequestException.class)
    .hasMessage("Todo not found");

왜 assertThatThrownBy를 썼나?

• 예외 검증을 체이닝으로 한 번에 표현 가능 -> 가독성 올라감
• assertThrows는 메시지 검증까지 하려면 예외를 변수로 받아서 추가 검증해야 함 -> 의도가 길어짐

---

(3) CommentServiceTest: Todo 없을 때 던지는 예외가 다르다

테스트는 ServerException을 기대했지만, 실제 컨텍스트(서비스 구현)는 InvalidRequestException을 던질 가능성이 높다.

즉, 테스트의 기대값(예외 타입/메시지)을 서비스 컨벤션과 맞춰야 테스트가 의미를 가진다.

---

(4) 핵심 트러블슈팅: todo.user가 null이면 NPE -> 서비스 로직 수정

증상

테스트에서 일부러 Todo.user = null로 만들었더니, 서비스 로직에서 아래가 터졌다:

todo.getUser().getId()

해결

권한 체크 전에 null 방어 로직을 넣었다.

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

내가 이렇게 처리한 이유

• user가 null이면 권한 검증 자체가 불가능함 (비교할 대상이 없음)
• “NPE(런타임 오류)” 대신 “의도한 비즈니스 예외”로 바꾸면
  • 테스트도 안정화되고
  • API 사용자 입장에서도 에러 의미가 명확해진다

---

회고: 이번 과제로 얻은 것

• ArgumentResolver는 만들기보다 등록 누락이 더 흔한 실수라는 점
• 테스트는 “성공시키는 것”보다 왜 실패했는지 원인을 분해하는 과정이 훨씬 학습이 된다는 점
• 예외 처리의 목적은 “에러를 막는 것”이 아니라 의미 있는 실패를 만드는 것이라는 점
  (NPE -> InvalidRequestException으로 변경한 케이스)

---

프로젝트 github 링크

CH3 프로젝트 github 링크