Skip to content
Open
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 @@ -4,6 +4,8 @@
import java.util.ArrayList;
import java.util.List;

import org.hibernate.annotations.BatchSize;

import com.comatching.item.domain.order.enums.OrderStatus;

import jakarta.persistence.CascadeType;
Expand Down Expand Up @@ -68,6 +70,7 @@ public class Order {

private LocalDateTime decidedAt;

@BatchSize(size = 100)
@OneToMany(mappedBy = "order", cascade = CascadeType.ALL, orphanRemoval = true)
private List<OrderItem> orderItems = new ArrayList<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.util.List;
import java.util.Optional;

import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
Expand All @@ -25,7 +27,7 @@ SELECT CASE WHEN COUNT(o) > 0 THEN true ELSE false END

List<Order> findAllByStatusOrderByRequestedAtDesc(OrderStatus status);

List<Order> findAllByStatusAndExpiresAtAfterOrderByRequestedAtDesc(OrderStatus status, LocalDateTime now);
Page<Order> findAllByStatusAndExpiresAtAfter(OrderStatus status, LocalDateTime now, Pageable pageable);

List<Order> findTop100ByStatusAndExpiresAtBeforeOrderByExpiresAtAsc(OrderStatus status, LocalDateTime now);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package com.comatching.item.domain.product.service;

import java.util.List;
import org.springframework.data.domain.Pageable;

import com.comatching.common.dto.response.PagingResponse;
import com.comatching.item.domain.product.dto.PurchaseRequestDto;

public interface AdminPaymentService {

List<PurchaseRequestDto> getPendingRequests();
PagingResponse<PurchaseRequestDto> getPendingRequests(Pageable pageable);

void approvePurchase(Long requestId, Long adminId);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package com.comatching.item.domain.product.service;

import java.time.LocalDateTime;
import java.util.List;

import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import com.comatching.common.domain.enums.ItemRoute;
import com.comatching.common.dto.item.AddItemRequest;
import com.comatching.common.dto.response.PagingResponse;
import com.comatching.common.exception.BusinessException;
import com.comatching.item.domain.item.service.ItemService;
import com.comatching.item.domain.order.entity.Order;
Expand All @@ -29,6 +32,8 @@
public class AdminPaymentServiceImpl implements AdminPaymentService {

private static final String EXPIRE_REASON = "AUTO_EXPIRED_BEFORE_ADMIN_DECISION";
private static final int MAX_PENDING_REQUEST_PAGE_SIZE = 100;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

MAX_PENDING_REQUEST_PAGE_SIZE와 같은 설정 값은 코드 내 상수로 관리하기보다 @ConfigurationProperties를 통해 외부 설정 파일(yml/properties)에서 관리하는 것을 권장합니다. 이를 통해 운영 환경에서 코드 수정 없이 상한선을 조정할 수 있어 유연성이 높아집니다.

References
  1. 설정 값은 @value 남용 대신 @ConfigurationProperties 사용을 권장합니다. (link)

private static final Sort DEFAULT_PENDING_REQUEST_SORT = Sort.by(Sort.Direction.DESC, "requestedAt");

private final OrderRepository orderRepository;
private final OrderGrantLedgerRepository orderGrantLedgerRepository;
Expand All @@ -40,11 +45,25 @@ public class AdminPaymentServiceImpl implements AdminPaymentService {
private static final int PURCHASED_ITEM_EXPIRE_DAYS = 36500;

@Override
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

getPendingRequests 메서드는 단순 조회 로직이므로, 성능 최적화(스냅샷 저장 방지 등)를 위해 @Transactional(readOnly = true)를 명시하는 것이 좋습니다. 현재 클래스 레벨에 @Transactional이 선언되어 있어 기본적으로 쓰기 트랜잭션이 적용되고 있습니다.

	@Transactional(readOnly = true)
	@Override
References
  1. 조회 전용 로직에는 readOnly = true가 명시되어 있는지 검토합니다. (link)

public List<PurchaseRequestDto> getPendingRequests() {
return orderRepository.findAllByStatusAndExpiresAtAfterOrderByRequestedAtDesc(OrderStatus.PENDING, LocalDateTime.now())
.stream()
.map(PurchaseRequestDto::from)
.toList();
public PagingResponse<PurchaseRequestDto> getPendingRequests(Pageable pageable) {
Pageable boundedPageable = toBoundedPageable(pageable);
return PagingResponse.from(
orderRepository.findAllByStatusAndExpiresAtAfter(
OrderStatus.PENDING,
LocalDateTime.now(),
boundedPageable
).map(PurchaseRequestDto::from)
);
}

private Pageable toBoundedPageable(Pageable pageable) {
if (pageable == null || pageable.isUnpaged()) {
return PageRequest.of(0, MAX_PENDING_REQUEST_PAGE_SIZE, DEFAULT_PENDING_REQUEST_SORT);
}

int pageSize = Math.min(pageable.getPageSize(), MAX_PENDING_REQUEST_PAGE_SIZE);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

pageable.getPageSize()가 1보다 작은 경우(예: 0) PageRequest.of 호출 시 IllegalArgumentException이 발생할 수 있습니다. 최소 크기(1)를 보장하도록 Math.max(1, ...)를 추가하여 안정성을 높이는 것을 권장합니다.

Suggested change
int pageSize = Math.min(pageable.getPageSize(), MAX_PENDING_REQUEST_PAGE_SIZE);
int pageSize = Math.max(1, Math.min(pageable.getPageSize(), MAX_PENDING_REQUEST_PAGE_SIZE));

Sort sort = pageable.getSort().isSorted() ? pageable.getSort() : DEFAULT_PENDING_REQUEST_SORT;
return PageRequest.of(pageable.getPageNumber(), pageSize, sort);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.comatching.item.infra.controller;

import java.util.List;

import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.web.PageableDefault;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
Expand All @@ -14,6 +15,7 @@
import com.comatching.common.domain.enums.MemberRole;
import com.comatching.common.dto.member.MemberInfo;
import com.comatching.common.dto.response.ApiResponse;
import com.comatching.common.dto.response.PagingResponse;
import com.comatching.item.domain.product.dto.PurchaseRequestDto;
import com.comatching.item.domain.product.service.AdminPaymentService;

Expand All @@ -30,10 +32,13 @@ public class AdminPaymentController {
private final AdminPaymentService adminPaymentService;

@RequireRole(MemberRole.ROLE_ADMIN)
@Operation(summary = "승인 대기 목록 조회", description = "아직 처리되지 않은(PENDING) 구매 요청 목록을 최신순으로 조회합니다.")
@Operation(summary = "승인 대기 목록 조회", description = "아직 처리되지 않은(PENDING) 구매 요청 목록을 페이지 단위로 최신순 조회합니다.")
@GetMapping("/requests")
public ResponseEntity<ApiResponse<List<PurchaseRequestDto>>> getPendingRequests(@CurrentMember MemberInfo memberInfo) {
return ResponseEntity.ok(ApiResponse.ok(adminPaymentService.getPendingRequests()));
public ResponseEntity<ApiResponse<PagingResponse<PurchaseRequestDto>>> getPendingRequests(
@CurrentMember MemberInfo memberInfo,
@PageableDefault(size = 20, sort = "requestedAt", direction = Sort.Direction.DESC) Pageable pageable
) {
return ResponseEntity.ok(ApiResponse.ok(adminPaymentService.getPendingRequests(pageable)));
}

@RequireRole(MemberRole.ROLE_ADMIN)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.comatching.item.domain.product.service;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -16,11 +17,16 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.test.util.ReflectionTestUtils;

import com.comatching.common.domain.enums.ItemRoute;
import com.comatching.common.domain.enums.ItemType;
import com.comatching.common.dto.item.AddItemRequest;
import com.comatching.common.dto.response.PagingResponse;
import com.comatching.common.exception.BusinessException;
import com.comatching.item.domain.item.service.ItemService;
import com.comatching.item.domain.order.entity.Order;
Expand All @@ -31,6 +37,7 @@
import com.comatching.item.domain.order.repository.OrderRepository;
import com.comatching.item.domain.order.service.OrderDecisionLogService;
import com.comatching.item.domain.order.service.OrderOutboxService;
import com.comatching.item.domain.product.dto.PurchaseRequestDto;
import com.comatching.item.global.exception.PaymentErrorCode;

@ExtendWith(MockitoExtension.class)
Expand All @@ -55,6 +62,60 @@ class AdminPaymentServiceImplTest {
@Mock
private ItemService itemService;

@Test
@DisplayName("승인 대기 목록을 페이지 단위로 조회한다")
void shouldGetPendingRequestsWithPagination() {
// given
Pageable pageable = PageRequest.of(1, 2, Sort.by(Sort.Direction.DESC, "requestedAt"));
Order firstOrder = Order.builder()
.memberId(11L)
.requestedItemName("매칭 패키지")
.requesterRealName("홍길동")
.requesterUsername("길동")
.requestedPrice(9900)
.expectedPrice(9900)
.requestedAt(java.time.LocalDateTime.now())
.expiresAt(java.time.LocalDateTime.now().plusMinutes(10))
.build();
ReflectionTestUtils.setField(firstOrder, "id", 7L);
firstOrder.addOrderItem(OrderItem.builder().itemType(ItemType.MATCHING_TICKET).quantity(2).build());

given(orderRepository.findAllByStatusAndExpiresAtAfter(eq(OrderStatus.PENDING), any(), eq(pageable)))
.willReturn(new PageImpl<>(java.util.List.of(firstOrder), pageable, 5));

// when
PagingResponse<PurchaseRequestDto> response = adminPaymentService.getPendingRequests(pageable);

// then
assertThat(response.content()).hasSize(1);
assertThat(response.content().get(0).requestId()).isEqualTo(7L);
assertThat(response.content().get(0).matchingTicketQty()).isEqualTo(2);
assertThat(response.currentPage()).isEqualTo(1);
assertThat(response.size()).isEqualTo(2);
assertThat(response.totalElements()).isEqualTo(5);
assertThat(response.totalPages()).isEqualTo(3);
then(orderRepository).should()
.findAllByStatusAndExpiresAtAfter(eq(OrderStatus.PENDING), any(), eq(pageable));
}

@Test
@DisplayName("승인 대기 목록 페이지 크기는 최대 100개로 제한한다")
void shouldCapPendingRequestPageSize() {
// given
Pageable pageable = PageRequest.of(0, 500, Sort.by(Sort.Direction.DESC, "requestedAt"));
ArgumentCaptor<Pageable> pageableCaptor = ArgumentCaptor.forClass(Pageable.class);
given(orderRepository.findAllByStatusAndExpiresAtAfter(eq(OrderStatus.PENDING), any(), any(Pageable.class)))
.willReturn(new PageImpl<>(java.util.List.of(), PageRequest.of(0, 100), 0));

// when
adminPaymentService.getPendingRequests(pageable);

// then
then(orderRepository).should()
.findAllByStatusAndExpiresAtAfter(eq(OrderStatus.PENDING), any(), pageableCaptor.capture());
assertThat(pageableCaptor.getValue().getPageSize()).isEqualTo(100);
}

@Test
@DisplayName("관리자 승인 시 주문 상태를 승인으로 바꾸고 구성품을 지급한다")
void shouldApprovePendingOrderAndGrantRewards() {
Expand Down