Market 프로젝트 리팩토링 - 1 // 발견된 사항들

1. Controller 레이어에서 참고하는 Authentication 객체의 사용

vs  SecurityUti과 같은 공통 메서드를 사용하는 방법

 

   @GetMapping("/my-cart")
    public ResponseEntity<ApiResponseDto<CartDto>> getCartItems(Authentication authentication) {
        Long customerId = securityUtil.getCurrentCustomerId(authentication);
        CartDto cartDto = cartService.getCartItems(customerId);
        String message = MessageUtil.getMessage(CartApiMessage.CART_LIST_SUCCESS);
        return ApiResponseUtil.success(message, cartDto, 200);
    }

 

이러한 코드에서는 로그인한 유저의 정보를 얻어올 필요가 있었다.

-> customerId 값이 있어야 Id에 해당하는 CartDto같은 객체들을 또 알아낼 수 있기때문

 

SpringSecurity를 통한 인증 과정(로그인 - username, password의 자격 증명)을 거치고 JWT를 생성받아 토큰에서 사용자 정보를 가져와 사용하고 있는 시점에서 Authentication 객체는 SpringSecurityContextHolder에 저장되어 있다.

(서버측 요청에 따라 JWT가 유효한지 확인하고, JWT 에서 사용자 정보를 저장한다.)

 

사용자 정보를 여기서 가져올 수 있는 것이다.

 

ContextHolder에서 직접 가져올 수도 있고, 이 코드처럼 Authentication 객체에서 추출 할 수도 있다.

어플리케이션 전역적으로는 ContextHolder에서 정보를 가져올 수 있지만 특정 상황에서 명시적인 용도로 사용할때에는 Authentication객체를 직접 참조하여 가져오는것이 나을 수 있다. 

 

-> Controller에서 customerId를 참조하기 위해 Authentication 객체를 이용하기로 한다.

---

 

Cart, Order 2개의 도메인에서 비슷하게 사용될 것이기때문에, 매번 customerId를 추출해내는 로직을 처음에는 CustomerService에 따로 만들었고, 각 컨트롤러에 이를 주입받아 사용하였다.

 

하지만 각 컨트롤러 (CartController, OrderController)에 CustomerService를 주입받아 사용 하는 것은 가능은 하지만, Cart, Order 두 도메인에 CustomerService가 관여를 하고있기 때문에 최대한 관심사를 분리하고, 중앙화를 하여 관리하는 것이 좋다고 생각했다. 또한 두 도메인의 컨트롤러에서 중복된 코드를 줄일 수 있으므로 다른 메서드를 생성하기로 하였다.

 

SecurityUtil.java

// ...상위 내용 생략

public Long getCurrentCustomerId(Authentication authentication) {
        String customerName = authentication.getName();
        return customerService.findCustomerIdByCustomerName(customerName);
    }

 

SecurityUtil 이라는 보안 관련 클래스에서 이미 SecurityContextHolder 내의 정보를 참조하여 customerName을 얻어오는 로직이 있었기에, 여기에 따로 customerId를 가져오는 로직을 추가하여 각 컨트롤러에 사용하기로 한다.

 

리팩토링을 진행하였고, 코드가 좀 더 깔끔해진 것을 볼 수 있다.

(깔끔해졌다기 보다는, 좀 더 이해하기 쉽고 가독성이 높아진 것을 느낀다)

 

 @GetMapping("/my-order")
    public ResponseEntity<ApiResponseDto<List<OrderDto>>> getOrderList(Authentication authentication) {
        String customerName = authentication.getName();
        Long customerId = customerService.findCustomerIdByAuthentication(authentication);
        List<OrderDto> orderDto = orderService.getOrderList(customerId);

        return ApiResponseUtil.success(customerName + " 님의 주문 목록 입니다.", orderDto,200);

    }
// 리팩토링 전
// -------------------------- 
// 리팩토링 후
    
 @GetMapping("/my-order")
    public ResponseEntity<ApiResponseDto<List<OrderDto>>> getOrderList(Authentication authentication) {
        String customerName = authentication.getName();
        Long customerId = securityUtil.getCurrentCustomerId(authentication);
        List<OrderDto> orderDto = orderService.getOrderList(customerId);
        String message = MessageUtil.getFormattedMessage(OrderApiMessage.ORDER_LIST_SUCCESS, customerName);
        return ApiResponseUtil.success(message, orderDto, 200);
    }

 

추가로 ApiResponse에 사용된 Message들을 변수로 따로 저장하여 관리하였다.

MessageUtil 클래스를 이용하여 사용자의 정보가 필요한 fomattedMessage나 그냥 String만 존재하는 값들을 한곳에 모아서 관리한다.

public class MessageUtil {

    public static String getMessage(String key) {
        return key;
    }
    public static String getFormattedMessage(String key, Object... arg) {
        return MessageFormat.format(key, arg);
    }
}

public class CartApiMessage {
        public static final String CART_LIST_SUCCESS = "회원님의 장바구니 정보 입니다.";
        public static final String CART_ITEM_ADD_SUCCESS = "장바구니에 상품이 담겼습니다.";
        public static final String CART_ITEM_UPDATE_SUCCESS = "수량이 변경 되었습니다.";
        public static final String CART_ITEM_DELETE_SUCCESS = "상품이 장바구니에서 제거되었습니다.";
        public static final String CART_CLEAR_SUCCESS = "장바구니를 비웠습니다.";
    }

 

단순히  String 메시지만 보여준다면, key값만 설정하고

동적인 사용자의 정보를 함께 표현한다면 key값과 arg값을 같이 사용해준다.

 

전체적인 컨트롤러의 코드가 일관성 있게 변화하였다.

---

2. CustomUserDetails 클래스의 이해

 

기껏 CustomerUserDetails를 만들어 사용중인데, 어디서 사용되는지 잘 모르겠거니와 no usage라는 메시지도 떠있다.

 

하지만 오버라이딩 된 다른 메서드들은 그 사용처를 정확히 알 수 없으므로 어떤 과정에서 이러한 UserDetails 객체가 사용되는지 정확히 알아야겠다.

 

인증과정에서,

loaduserByUsername 메서드를 통해 UserDetails 객체를 조회하는데 이때 CustomUserDetails 클래스의 인스턴스가 생성되어 여기서 반환된 사용자 정보가 입력된 로그인 정보와 비교된다.

 

결국 LoginProvider 에서 로그인을 시도할 때, UserDetails 객체가 필요한 것이었다.

패키지 구조를 좀 더 명확하게 분리해야겠다.

 

3. 패키지 구조 변경

 

 

 

한눈에 알아볼 수는 있었지만, 굳이 코드 자체를 읽어보고 판단하지 않아도 어떤 역할인지 알 수 있도록 Security관련 클래스들을 재구성해보았다. misc는 아직 좋지않은 패키지 명이므로 또 수정이 필요하겠다. 

 

우선 각 역할에 맞게 패키지를 구성하고, Security관련된 핸들러들을 함께 집어넣었다. 패키지 구조는 언제봐도 쉽지않다.

팀에서 이러한 것들도 전부 고려를 해야할 듯하다. 이건 개인 프로젝트이니 내맘대로해야지

 

---

4. 각 도메인 코드 리팩토링

* 이하 도메인 리팩토링 부분은 공통된 부분이 많음

* builder 패턴을 간소화하여 from 메서드를 사용하거나 하는 부분은, 딱히 정해진 해답이 없다. 어떤 방법이 더 유용하고 간소화되고, 가독성이 좋아지는지는 순전히 개인과 팀의 선택에 달렸다고 한다. 

 

1. Product

1) builder 패턴 숨기기 -> 정적메서드 사용

 

2) Controller에 String으로 하드 코딩되어있는 부분 변수로 변경하기

-> messageUtil을 사용하면 다른 컨트롤러에서도 사용이 가능하다. 이 클래스 하나로 메시지 포맷팅을 전부 진행할 수 있다. (위에서 언급)

 

3) SecurityUtil 사용으로 Authetication 객체를 참조하는 로직을 중앙화 하기

-> 위에서 언급한 그 내용이다.

2. Customer

1) 1.Product와 같음.

 

2) register 메서드의 builder 패턴을 숨기기에는 다른 연관된 데이터가 많아서 모델을 전체적으로바꾸어야 한다.

-> 잠시 보류하기로 함.

3. Cart

1) 1, 2 와 같음

4. Order

1) service 코드의 전체적인 리팩토링 (작업중)

-> 아직 마무리는 못했지만 다양한 관점에서 리팩토링 할 부분이 나타났다.

 

각 메서드에서 중복된 코드를 제거하고 , 분리가 필요한 메서스들은 분리하여 사용한다.

사용자 정의 예외들을 만들었으므로 Runtime예외가 있는 부분을 수정해준다.

도메인 모델에서 entity와 Dto의 사용이 혼재되어있다.

의존성 주입에서 생성자vs필드 주입을 일관성있게 변경하여야한다. 등등

 

이후 마무리 하고 추가할 예정

 

 

5. 마무리

리팩토링이라는 부분도 무척이나 파고 파면 많은 것들이 나왔다.

 

java (소프트웨어 엔지니어링의 범용 원칙) 설계에 해당하는 다양한 내용들을 맞추어가면서 어떻게 하면 더 좋은 코드를 작성할 수 있는지 계속해서 고민해보아야 한다.

 

가능한한 단순하게, SOLID원칙 등 다양한 관점에서 코드를 더 좋게 만들어보자