2022.05.24 「코드 리팩토링 Ver.2」

주마다 리팩터링 및 기능 추가하는 브랜치를 따 설계하며 프로젝트를 진행 중이다. 
이런 식으로 주마다 리팩터링을 하니까 확실히 코드가 깔끔해지는 걸 느낀다.
오늘 아침부터 진행한 코드 리팩토링은 유지보수성을 따지는 것은 물론이거니와 어려운 듯하면서도 새로운 개념을 도입해서 진행하니 나름 보람찼던 리팩터링이었다. 
직면했던 문제들을 나열하며 정리를 한번 해보겠다.

 

인증 & 인가 책임 분리

나는 인증, 인가를 인터셉터로 구현했다.

인증은 토큰 검증,

인가는 에너테이션 및 토큰 검증으로 구현했다.

 

기존 코드 : 인터셉터의 preHandle 메서드 안에 두 개의 로직이 동시에 들어있다.

토큰 검증 + 에너테이션 검증

=> 책임이 많다 -> 유지보수가 어렵다.

 

해결 : 인터셉터를 나누어 분리했다.

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
    log.info("인증 처리 인터셉터 실행");

    /* 토큰 추출 및 검증 */
    String requestToken = authorizationExtractor.extract(request, BEARER_TOKEN);
    jwtTokenProvider.validateToken(requestToken);

    /* 토큰 body에 존재하는 아이디와 등급 */
    final String tokenUserId = jwtTokenProvider.getUserId(requestToken);

    /* request에 토큰 유저 권한 추가 */
    request.setAttribute("tokenUserRole", jwtTokenProvider.getUserGrade(requestToken));

    /* Redis DB에 저장된 토큰 추출 */
    final ValueOperations<String, Object> valueOperations = redisTemplate.opsForValue();
    final String redisToken = (String) valueOperations.get(tokenUserId);

    /* DB에 토큰이 존재하지 않을 경우 */
    if (redisToken == null) {
        throw new RedisNullTokenException(AuthMessages.NULL_TOKEN.getMessage());
    }

    /* DB 토큰과 로그인 유저 토큰 정보가 일치하지 않을 경우 */
    if (!redisToken.equals(requestToken)) {
        throw new TokenMismatchException(AuthMessages.INVALID_TOKEN.getMessage());
    }

    return true;
}

=> 인증 처리 인터셉터

 

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
    log.info("권한 처리 인터셉터 실행");

    if (!(handler instanceof HandlerMethod)) {
        return true;
    }

    /* 핸들러메서드 에너테이션 값 추출 */
    HandlerMethod handlerMethod = (HandlerMethod) handler;
    Auth auth = handlerMethod.getMethodAnnotation(Auth.class);

    /* 권한이 필요 없는 접근 */
    if (auth == null) {
        return true;
    }

    /* 에너테이션 값 => 관리자일 경우 */
    if (auth.role() == ADMIN) {
        /* 로그인 유저 권한이 관리자가 아닐 경우 */
        if (!request.getAttribute("tokenUserRole").toString().equals(ADMIN.toString())) {
            throw new AuthenticationException(AuthMessages.NOT_ADMIN_AUTH.getMessage());
        }
    }

    return true;
}

=> 권한 처리 인터셉터

 

=> 인증이 이루어지고 인가가 이루어진다. 

 

 

중복된 코드 & 비즈니스 로직

장바구니 관련 컨트롤러에서 중복된 코드가 많이 발생했다.

작성을 먼저 하고 나서 리팩터링이 필수적으로 필요하다는 생각이 들었다.

또, Service 계층에 관해 궁금증이 있었다.

 

기존에 나는 Service 계층은 DB 로직을 가져오고 넘겨주는 역할을 한다고 알고 있었다.

그래서 CartController는 쿠키로 이루어지므로 Service를 이용하지 않았다. 

하지만 Service는 전반적인 비즈니스 로직을 처리하는 계층으로 컨트롤러에서 서비스로 로직을 이관해도 되었었다. 

 

그러나 이번 경우에는 로직 처리 보다는 중첩된 코드가 주된 문제 요소였기에, 컨트롤러에서 메서드를 분리하여 해결했다. 

/**
 * 장바구니 쿠키를 반환
 *
 * @param request servlet request 객체
 */
private void getCartCookie(HttpServletRequest request) {
    responseCartCookie = CookieUtil.getCartCookie(request.getCookies()).orElse(null);
}

/**
 * 장바구니 쿠키 값에서 map 객체 추출
 *
 * @param responseCookie 장바구니 쿠키
 * @throws UnsupportedEncodingException 인코딩 문제 시 예외 발생
 */
private void getCartDTOMap(Cookie responseCookie) throws UnsupportedEncodingException {
    cartDTOMap = CookieUtil.getCartItemDTOMap(responseCookie);
}

/**
 * 장바구니 map 객체에서 상품 삭제
 *
 * @param productNo 장바구니에 추가, 수정, 삭제할 상품 번호
 */
private void removeProductFromMap(int productNo) {
    cartDTOMap.remove(productNo);
}

/**
 * 전달할 장바구니 쿠키를 세팅
 *
 * @param response servlet response 객체
 * @throws UnsupportedEncodingException 인코딩 문제 시 예외 발생
 */
private void setCartCookie(HttpServletResponse response) throws UnsupportedEncodingException {
    responseCartCookie.setValue(URLEncoder.encode(JsonUtil.objectToString(cartDTOMap), ENC_TYPE));
    response.addCookie(responseCartCookie);
}

=> 컨트롤러에서 따로 분리한 메서드 

 

=> 장바구니 삭제 핸들러이다. 한눈에 봐도 메서드의 이름으로 유추가 가능하다. 

 

 

For문? Stream? 

배열이나 컬렉션을 가져와 반복을 통해 데이터를 꺼내서 변경 및 추출?

이젠 이렇게 할 필요가 없다. 

코드의 가독성도 떨어지고, 유지보수도 쉽지 않다.

 

=> 상수가 늘어나면? Switch 하나 더 추가할 거야? 줄어들면 제거할 거야?

 

public static Optional<UserGrade> of(int gradeNumber) {
    return Optional.of(Arrays.stream(UserGrade.values())
            .filter(userGrade1 -> userGrade1.getValue() == gradeNumber)
            .findFirst()
            .orElse(BASIC_USER));
}

=> 스트림을 사용하자 가독성이 매우 좋아지고, 변경 성도 현저히 줄어든다. 

 

 

=> 딱 봐도 필요 없는 for문이 돌아가지?

 

@Transactional(readOnly = true)
public List<ProductListDTO> getCategoryList(Map<String, Object> map) {
    return productMapper.selectCategoryList(map).stream()
            .map(productVO -> modelMapper.map(productVO, ProductListDTO.class))
            .collect(Collectors.toList());
}

=> 훨씬 간결해졌다. 

 

 

결론

이제 주문이 남았다. 주문도 restFul 하게 빠르게 작성해보고 부지런히 해봐야겠다.

리팩터링 하면서 코드가 깨끗해지는 걸 보니 할 땐 정말 머리에 쥐 나지만 끝나고 나면 진짜 뿌듯하다. 

더 노력해서 발전해보자~