장바구니 추가 API 리팩토링

허석진·2023년 2월 20일
0

이전에 팀프로젝트를 하며 작성했던 코드를 리팩토링 "왜?" 라는 질문을 중심으로 블로깅을 하려고 한다.

이전코드

// CartPost는 추가할 장바구니의 정보(상품Id, 수량)을 리스트로 가지고 있는 requestBody
// CustomPrincipal은 API 요청 시에 클라이언트가 넘겨준 JWT를 Filter에서 decoding하여 Spring Security Context에 등록한 Principal
@Override
public void createCart(CartPost post, CustomPrincipal principal) {
    post.cartItems().forEach(cartInfo -> {
                Product product = productRepository.findById(cartInfo.productId())
                        .orElseThrow(() -> new BusinessLogicException(ExceptionCode.PRODUCT_NOT_FOUND));
                Profile profile = profileRepository.getReferenceById(principal.profileId());
                CartDto dto = cartInfo.toDto();

                cartRepository.findByProfile_IdAndProduct_Id(profile.getId(), product.getId())
                        .ifPresentOrElse(
                                cart -> {
                                    cart.updateQuantity(cart.getQuantity() + dto.quantity());
                                    cartRepository.save(cart);
                                }, () -> {
                                    cartRepository.save(dto.toEntity(profile, product));
                                }
                        );
            });
}

왜?

내가 이 코드를 리팩토링하려는 이유는 다음과 같다.

  • 존재 여부를 확인하는 코드를 여러 API에 공통적으로 추가하면 유지-보수가 어려워진다.
  • 존재하지 않는 상품 id가 100개 중에 99번째 존재하는 경우, 앞에 98개에 대한 필요없는 동작(트랜잭션으로 인해 롤백될 update와 insert)이 이뤄진다.

어떻게?

처음 생각한 방법은 존재하지 않는 상품 id가 포함되어 있는 경우 해당 id들만 무시하거나, 장바구니에 추가하지 못한 상품들에 대한 정보를 위에 코드에서 클라이언트에게 그대로 반환하려 했었다.
그러나 그렇게 할 경우 다음과 같은 문제가 발생한다.

  • 존재하지 않는 상품 id가 넘겨졌을 때 예외를 발생시키지 않게된다.
  • 여전히 존재 여부를 확인하는 코드가 분리되지 않는다.
  • createCart 메소드가 클라이언트에게 추가 정보를 반환함에 따라 복잡성이 높아진다.

그래서 나는 다음과 같은 방법으로 리팩토링을 진행하려한다.
  • 존재 여부를 확인하는 API를 새로 만들고 이후 이벤트, 품절 등에도 대응할 수 있도록 설계한다.
  • 클라이언트에서는 존재 여부를 확인하는 API가 반환하는 값을 통해 어떤 상품이 장바구니에 추가할 수 있고, 없는지 알 수 있게하고 추가할 수 있는 상품만을 장바구니 추가 API 하도록 설계한다.

리팩토링 이후 코드

checkProductsAvailable (상품 존재 여부, 장바구니 추가 가능 여부를 확인하는 API의 서비스 코드)

// createCart와는 다른 Controller로 url은 POST ~~/check-out
// DealProductCheckOutRequest에는 장바구니 추가하기 위해 선택한 상품 id와 수량의 리스트가 담겨있음
// TODO: Principal 은 이후 이벤트 등이 추가 되면 이벤트 대상 유저인지 판별하기 위함
@Override
public CheckOutResponse checkProductsAvailable(DealProductCheckOutRequest dealProductCheckOutRequest, CustomPrincipal principal) {
    CheckOutResponse response = CheckOutResponse.of();

    // 현재는 DB 에 해당 상품 id 들이 존재하는 지만 확인
    List<Long> productIds = dealProductCheckOutRequest.dealProductInfos().stream().map(DealProductInfo::productId).toList();
    if (!productRepository.existsAllById(productIds)) throw new BusinessLogicException(ExceptionCode.PRODUCT_NOT_FOUND);

    dealProductCheckOutRequest.dealProductInfos().stream()
            .forEach(info -> {
                // TODO: 이후 프로그램 확장 시에 판매 중지, 이벤트 만료 등을 이유로 장바구니에 추가 불가능한 상품을 거르는 로직 추가
                // if (~~)
                // response.unavailableProducts().add(info);
                // else
                // 장바구니에 추가 가능한 상품 정보를 사용가능한 리스트에 추가
                response.availableProducts().add(info);
            });

    return response;
}

위에 메소드가 기존 API에서 하던 기능 중 하나를 가지고 나왔으며 해당 메소드에서 이후 이벤트 상품, 판매 중지 상품 등을 판별하고 클라이언트로 반환 할 수 있게 한다.


추가적으로 선택한 상품의 숫자 만큼 Query를 실행하던 로직을 QueryDSL을 통해 1번의 Query로 모든 상품들이 DB에 존재하는지 확인 할 수 있게 변경했다.
QueryDSL을 사용한 코드는 다음과 같다.

@Override
public boolean existsAllById(List<Long> productIds) {
    QProduct product = QProduct.product;

    return from(product)
            .where(product.id.in(productIds))
            .fetchCount() == productIds.size();
}

createCart

@Override
public void createCart(CartPost post, CustomPrincipal principal) {
    Profile profile = profileRepository.getReferenceById(principal.profileId());

    post.dealProductInfos().forEach(info -> {
            CartDto dto = CartDto.of(info.quantity());

            cartRepository.findByProfile_IdAndProduct_Id(profile.getId(), info.productId())
                    .ifPresentOrElse(
                            cart -> {
                                cart.updateQuantity(cart.getQuantity() + dto.quantity());
                                cartRepository.save(cart);
                            }, () -> {
                                // Cart 엔티티가 직접 Product 를 직접 가지지 않고 productId 로 연관되면 추가 쿼리 필요 없음
                                // CartDto 에도 profile 만 넘겨주면 됨
                                Product product = productRepository.getReferenceById(info.productId());
                                cartRepository.save(dto.toEntity(profile, product));
                            }
                    );
        });
}

DB에 존재하는 상품 id인지 확인하는 로직이 분리되었고
추가로 변경된 사항은 이유 없이 여러번 호출되었던 profileRepository.getReferenceByIdproductRepository.getReferenceById가 필요한 때만 호출되게 수정했다.
(프로젝트 기간동안 정말 정신이 없었나보다...)

추가적으로 할 일

현재 모든 API가 N+1 문제를 가지고 있어 앞으로는 그걸 위주로 해결해나가면서 블로깅할 생각이다.
위에 코드에서도 profile, product에 대한 select문이 발생하지 않을 것을 기대하고 있었는데 update문을 사용할 경우 N+1 문제가 발생했다.
현 시점에서 알아봐야 할 것은 getReferenceById를 통해 연관관계를 채워주면 추가적인 select문이 발생하지 않는 것으로 알고 있었는데 내가 잘못 사용하고 있거나, 잘못 알고 있었던 것 같다.


수정 - 해당 코드의 N+1 문제 해결 (23.02.21)

해당코드에서 update문에 대해서만 N+1 문제가 발생하던 것은 findByProfile_IdAndProduct_Identity를 찾는데 성공했기 때문이었다.
찾아진 entity는 그것이 담고 있는 정보를 모두 불러왔고 아래의 캡처와 같이 장바구니가 가지고 있는 profileproduct 역시 모두 불러왔다.

나는 당연히 기본 값이 LAZY일 것이라고 생각하고 진행했기 때문에 벌어진 문제였고 현재는 fetch = FetchType.LAZY를 추가해 문제를 해결했다.

0개의 댓글