이전에 팀프로젝트를 하며 작성했던 코드를 리팩토링 "왜?" 라는 질문을 중심으로 블로깅을 하려고 한다.
// 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));
}
);
});
}
내가 이 코드를 리팩토링하려는 이유는 다음과 같다.
처음 생각한 방법은 존재하지 않는 상품 id가 포함되어 있는 경우 해당 id들만 무시하거나, 장바구니에 추가하지 못한 상품들에 대한 정보를 위에 코드에서 클라이언트에게 그대로 반환하려 했었다.
그러나 그렇게 할 경우 다음과 같은 문제가 발생한다.
// 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();
}
@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.getReferenceById
와 productRepository.getReferenceById
가 필요한 때만 호출되게 수정했다.
(프로젝트 기간동안 정말 정신이 없었나보다...)
현재 모든 API가 N+1 문제를 가지고 있어 앞으로는 그걸 위주로 해결해나가면서 블로깅할 생각이다.
위에 코드에서도 profile
, product
에 대한 select문
이 발생하지 않을 것을 기대하고 있었는데 update문
을 사용할 경우 N+1 문제가 발생했다.
현 시점에서 알아봐야 할 것은 getReferenceById를 통해 연관관계를 채워주면 추가적인 select문
이 발생하지 않는 것으로 알고 있었는데 내가 잘못 사용하고 있거나, 잘못 알고 있었던 것 같다.
해당코드에서 update문
에 대해서만 N+1 문제가 발생하던 것은 findByProfile_IdAndProduct_Id
가 entity
를 찾는데 성공했기 때문이었다.
찾아진 entity
는 그것이 담고 있는 정보를 모두 불러왔고 아래의 캡처와 같이 장바구니
가 가지고 있는 profile
과 product
역시 모두 불러왔다.
나는 당연히 기본 값이 LAZY
일 것이라고 생각하고 진행했기 때문에 벌어진 문제였고 현재는 fetch = FetchType.LAZY
를 추가해 문제를 해결했다.