Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ ResponseEntity<OrderCancelResponse> getOrderCancel(
// 장바구니에서 주문서로 넘어가는 API
@Operation(summary = "장바구니에서 주문서로 넘어갈 때 사용하는 API")
ResponseEntity<CartResponse> getCheckout(User user, List<OrderDetailRequest> orderDetailRequest);

//바로 주문하기에서 주문서로 넘어가는 API
@Operation(summary = "바로 주문하기에서 주문서로 넘어갈 때 사용하는 API")
ResponseEntity<CartResponse> getCheckoutInstant(User user, Long saleProductId, int quantity);

//회원 주문 - 결제와 연동
// @Operation(
// summary = "회원 주문 및 결제 API",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import com.jishop.member.annotation.CurrentUser;
import com.jishop.member.domain.User;
import com.jishop.order.dto.*;
import com.jishop.order.service.OrderService;
import com.jishop.order.service.OrderCancelService;
import com.jishop.order.service.OrderCreationService;
import com.jishop.order.service.OrderGetService;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.Page;
Expand All @@ -18,14 +20,16 @@
@RequestMapping("/orders")
public class OrderControllerImpl implements OrderController {

private final OrderService orderService;
private final OrderCreationService orderCreationService;
private final OrderGetService orderGetService;
private final OrderCancelService orderCancelService;

// 주문 생성
@Override
@PostMapping
public ResponseEntity<OrderResponse> createOrder(@CurrentUser User user,
@Valid @RequestBody OrderRequest orderRequest) {
OrderResponse orderResponse = orderService.createOrder(user, orderRequest);
OrderResponse orderResponse = orderCreationService.createOrder(user, orderRequest);

return ResponseEntity.ok(orderResponse);
}
Expand All @@ -34,7 +38,7 @@ public ResponseEntity<OrderResponse> createOrder(@CurrentUser User user,
@Override
@GetMapping("/{orderId}")
public ResponseEntity<OrderDetailPageResponse> getOrder(@CurrentUser User user, @PathVariable Long orderId){
OrderDetailPageResponse orderDetailResponse = orderService.getOrder(user, orderId, null, null);
OrderDetailPageResponse orderDetailResponse = orderGetService.getOrder(user, orderId, null, null);
Copy link
Contributor

@jihaneol jihaneol Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orderGetService 이 무엇인가요.
주문을 얻어 오는 건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다! 하나의Service에 주문 관련 모든 걸 포함하기 싫어서 분리했습니다

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드를 작성하면 좋을 것 같아요!
주문과 같은 복잡한 서비스는 테스트 코드를 작성하면서 자연스럽게 구조를 개선하고 서비스 레이어를 분리하는 방식이 될 수 있습니다. 테스트 코드를 작성하는 과정에서 의존성 분리, 파라미터 정리, 공통 로직 추출 등이 자연스럽게 이루어져 유지보수성과 확장성에 강한 코드를 완성할 수 있다고 생각합니다.


return ResponseEntity.ok(orderDetailResponse);
}
Expand All @@ -48,7 +52,7 @@ public ResponseEntity<Page<OrderResponse>> getOrderList(
@RequestParam(value = "page", defaultValue = "0") int page,
@RequestParam(value = "size", defaultValue = "10") int size) {

Page<OrderResponse> responseList = orderService.getPaginatedOrders(user, period, page, size);
Page<OrderResponse> responseList = orderGetService.getPaginatedOrders(user, period, page, size);

return ResponseEntity.ok(responseList);
}
Expand All @@ -57,7 +61,7 @@ public ResponseEntity<Page<OrderResponse>> getOrderList(
@Override
@PatchMapping("/{orderId}")
public ResponseEntity<String> cancelOrder(@CurrentUser User user, @PathVariable Long orderId){
orderService.cancelOrder(user, orderId, null, null);
orderCancelService.cancelOrder(user, orderId, null, null);

return ResponseEntity.ok("주문이 취소되었습니다");
}
Expand All @@ -66,7 +70,7 @@ public ResponseEntity<String> cancelOrder(@CurrentUser User user, @PathVariable
@Override
@PostMapping("/instant")
public ResponseEntity<OrderResponse> createInstantOrder(@CurrentUser User user, @RequestBody @Valid OrderRequest orderRequest) {
OrderResponse orderResponse = orderService.createInstantOrder(user, orderRequest);
OrderResponse orderResponse = orderCreationService.createInstantOrder(user, orderRequest);

return ResponseEntity.ok(orderResponse);
}
Expand All @@ -75,26 +79,15 @@ public ResponseEntity<OrderResponse> createInstantOrder(@CurrentUser User user,
@Override
@GetMapping("/getCancel/{orderId}")
public ResponseEntity<OrderCancelResponse> getOrderCancel(@CurrentUser User user, @PathVariable Long orderId) {
OrderCancelResponse orderCancelResponse = orderService.getCancelPage(user, orderId, null, null);
OrderCancelResponse orderCancelResponse = orderGetService.getCancelPage(user, orderId, null, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orderGetService에서 getCancelPage가 있으니 주문을 가져오는데 null을 보내는 이유가 있을까요?
있다면 dto로 만들어서 처리하는 방법이 더 좋을 것 같아요

  1. 의미가 불명확하다.
  2. 파라미터 늘어날때 유지 보수 힘들다.
  3. 서비스 로직에 null 처리 코드와 같은 방어로직이 쌓인다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 회원은 orderId만 있으면 주문 취소 상세페이지를 가져올 수 있는데 비회원은 orderNumber와 phone이 있어야만 가져올 수 있게 설계되어있습니다! 같른 메서드를 사용하려다보니 null를 보내주게 되었습니다ㅜ


return ResponseEntity.ok(orderCancelResponse);
}

//장바구니에서 주문서로 넘어갈 때 사용하는 API
@PostMapping("/checkout")
public ResponseEntity<CartResponse> getCheckout(@CurrentUser User user, @RequestBody List<OrderDetailRequest> orderDetailRequest) {
CartResponse products = orderService.getCheckout(user, orderDetailRequest);

return ResponseEntity.ok(products);
}

//바로주문하기에서 주문서로 넘어갈 때 사용하는 API
@Override
@GetMapping("/checkoutInstant")
public ResponseEntity<CartResponse> getCheckoutInstant(@CurrentUser User user,
@RequestParam("saleProductId") Long saleProductId,
@RequestParam("quantity") int quantity) {
CartResponse products = orderService.getCheckoutInstant(user, saleProductId, quantity);
CartResponse products = orderGetService.getCheckout(user, orderDetailRequest);

return ResponseEntity.ok(products);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import com.jishop.order.dto.OrderDetailPageResponse;
import com.jishop.order.dto.OrderRequest;
import com.jishop.order.dto.OrderResponse;
import com.jishop.order.service.OrderService;
import com.jishop.order.service.OrderCancelService;
import com.jishop.order.service.OrderCreationService;
import com.jishop.order.service.OrderGetService;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
Expand All @@ -15,13 +17,15 @@
@RequestMapping("/ordersGuest")
public class OrderGuestControllerImpl implements OrderGuestController {

private final OrderService orderService;
private final OrderCreationService orderCreationService;
private final OrderGetService orderGetService;
private final OrderCancelService orderCancelService;

//비회원 주문 생성
@Override
@PostMapping
public ResponseEntity<OrderResponse> createGuestOrder(@RequestBody @Valid OrderRequest orderRequest) {
OrderResponse orderResponse = orderService.createGuestOrder(orderRequest);
OrderResponse orderResponse = orderCreationService.createOrder(null, orderRequest);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 위 리뷰와 동일합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 똑같이 메서드를 하나로 처리하려다보니 생긴 이슈입니다. 기능은 똑같은데 회원 비회원 메서드를 따로 만들면 중복코드이지 않나 싶습니다ㅜ 더 좋은 방법이 있다면 알려주시면 감사하갰습니다!🥹

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 오버로딩을 사용했습니다. 이렇게 하면 의도도 명확하고 실수도 줄일 수 있습니다


return ResponseEntity.ok(orderResponse);
}
Expand All @@ -30,7 +34,7 @@ public ResponseEntity<OrderResponse> createGuestOrder(@RequestBody @Valid OrderR
@Override
@PostMapping("/instant")
public ResponseEntity<OrderResponse> createGuestInstantOrder(@RequestBody @Valid OrderRequest orderRequest) {
OrderResponse orderResponse = orderService.createGuestInstantOrder(orderRequest);
OrderResponse orderResponse = orderCreationService.createInstantOrder(null, orderRequest);

return ResponseEntity.ok(orderResponse);
}
Expand All @@ -40,7 +44,7 @@ public ResponseEntity<OrderResponse> createGuestInstantOrder(@RequestBody @Valid
@GetMapping("/{orderNumber}")
public ResponseEntity<OrderDetailPageResponse> getGuestOrderDetail(@PathVariable String orderNumber,
@RequestParam String phone) {
OrderDetailPageResponse orderDetailList = orderService.getGuestOrder(orderNumber, phone);
OrderDetailPageResponse orderDetailList = orderGetService.getOrder(null, null, orderNumber, phone);

return ResponseEntity.ok(orderDetailList);
}
Expand All @@ -50,7 +54,7 @@ public ResponseEntity<OrderDetailPageResponse> getGuestOrderDetail(@PathVariable
@PatchMapping("/{orderNumber}")
public ResponseEntity<String> cancelGuestOrder(@PathVariable String orderNumber,
@RequestParam String phone) {
orderService.cancelGuestOrder(orderNumber, phone);
orderCancelService.cancelOrder(null, null, orderNumber, phone);

return ResponseEntity.ok("주문이 취소되었습니다.");
}
Expand All @@ -59,7 +63,7 @@ public ResponseEntity<String> cancelGuestOrder(@PathVariable String orderNumber,
@Override
@GetMapping("/getCancel/{orderNumber}")
public ResponseEntity<OrderCancelResponse> getGuestOrderCancel(@PathVariable String orderNumber, @RequestParam String phone){
OrderCancelResponse orderCancelResponse = orderService.getGuestCancelPage(orderNumber, phone);
OrderCancelResponse orderCancelResponse = orderGetService.getCancelPage(null,null, orderNumber, phone);

return ResponseEntity.ok(orderCancelResponse);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
package com.jishop.order.dto;

import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Positive;

public record OrderDetailRequest(
@NotNull(message = "상품 정보는 필수입니다.")
Long saleProductId,
@Positive(message = "수량은 1개 이상이어야 합니다.")
int quantity,
Long cartId
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ Page<Long> findOrderIdsByPeriod(@Param("userId") Long userId,

boolean existsByOrderNumber(String orderNumber);

//주문 취소를 위한 쿼리
@Query("SELECT o FROM Order o " +
"LEFT JOIN FETCH o.orderDetails od " +
"LEFT JOIN FETCH od.saleProduct sp " +
"LEFT JOIN FETCH sp.stock " +
"WHERE o.id = :orderId AND o.userId = :userId")
Optional<Order> findForCancellation(@Param("userId") Long userId, @Param("orderId") Long orderId);

Comment on lines -57 to -64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주문 취소 쿼리를 삭제하신 이유가 있을 까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하고있지 않아서 삭제했습니다!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주문 취소를 못하는 건가요?

Optional<Order> findById(Long orderId);

Optional<Order> findByOrderNumberAndPhone(String orderNumber, String phone);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,5 @@ public interface OrderGetService {
OrderCancelResponse getCancelPage(User user, Long orderId, String orderNumber, String phone);

//회원 비회원 장바구니에서 주문서로 넘어가는 API
CartResponse getCheckOut(User user, List<OrderDetailRequest> orderDetailRequest);

//회원 비회원 바로주문하기에서 주문서로 넘어가는 API
CartResponse getCheckoutInstant(User user, Long saleProductId, int quantity);
CartResponse getCheckout(User user, List<OrderDetailRequest> orderDetailRequest);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import com.jishop.order.service.DistributedLockService;
import com.jishop.order.service.OrderCreationService;
import com.jishop.order.service.OrderUtilService;
import com.jishop.saleproduct.repository.SaleProductRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -26,7 +25,6 @@
public class OrderCreationServiceImpl implements OrderCreationService {

private final OrderRepository orderRepository;
private final SaleProductRepository saleProductRepository;
private final AddressRepository addressRepository;
private final OrderUtilService orderUtilService;
private final DistributedLockService distributedLockService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public OrderCancelResponse getCancelPage(User user, Long orderId, String orderNu

//회원, 비회원 장바구니에서 주문서로 넘어가는 API
@Override
public CartResponse getCheckOut(User user, List<OrderDetailRequest> orderDetailRequest) {
public CartResponse getCheckout(User user, List<OrderDetailRequest> orderDetailRequest) {
List<Long> saleProductIds = orderDetailRequest.stream()
.map(OrderDetailRequest::saleProductId)
.toList();
Expand Down Expand Up @@ -122,32 +122,7 @@ public CartResponse getCheckOut(User user, List<OrderDetailRequest> orderDetailR
.toList();

// CartResponse 생성 및 반환
return CartResponse.of(cartDetails);
}

//바로 주문하기에서 주문서로 넘어갈 때 사용하는 API
@Override
public CartResponse getCheckoutInstant(User user, Long saleProductId, int quantity) {
SaleProduct saleProduct = saleProductRepository.findById(saleProductId)
.orElseThrow(() -> new DomainException(ErrorType.PRODUCT_NOT_FOUND));

int paymentPrice = saleProduct.getProduct().getDiscountPrice();
int orderPrice = saleProduct.getProduct().getOriginPrice();
int discountPrice = orderPrice - paymentPrice;

CartDetailResponse cartDetailResponse = CartDetailResponse.from(
null,
saleProduct,
quantity,
paymentPrice,
orderPrice,
discountPrice,
false
);

return CartResponse.of(List.of(cartDetailResponse));
}

return CartResponse.of(cartDetails); }

private OrderDetailPageResponse createOrderDetailPageResponse(Order order, User user) {
boolean isPurchasedConfirmed = order.getStatus() == OrderStatus.PURCHASED_CONFIRMED;
Expand Down
Loading