From 3408228766fbef786388ea68993d699707c214cd Mon Sep 17 00:00:00 2001 From: Kim Heebin Date: Thu, 30 Nov 2023 01:05:13 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20=ED=86=A0=EC=8A=A4=20=EA=B2=B0=EC=A0=9C?= =?UTF-8?q?=20=EC=8A=B9=EC=9D=B8=20=EC=8B=A4=ED=8C=A8=20=EC=8B=9C=20?= =?UTF-8?q?=EC=98=88=EC=99=B8=20=EC=B2=98=EB=A6=AC=20(#188)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 토스 결제 승인 실패 시 예외 throw * test: 결제 승인 로직 변경에 따른 테스트 수정 --- .../application/payment/PaymentService.java | 30 ++--- .../moabam/api/domain/payment/Payment.java | 4 +- .../payment/ConfirmTossPaymentRequest.java | 12 -- .../payment/TossPaymentMapper.java | 18 --- .../payment/TossPaymentService.java | 11 +- .../api/presentation/PaymentController.java | 15 ++- .../error/exception/TossPaymentException.java | 8 ++ .../error/handler/GlobalExceptionHandler.java | 3 +- .../payment/PaymentServiceTest.java | 123 +++++++----------- .../payment/TossPaymentServiceTest.java | 6 +- .../presentation/PaymentControllerTest.java | 27 +++- .../support/fixture/PaymentFixture.java | 9 -- 12 files changed, 121 insertions(+), 145 deletions(-) delete mode 100644 src/main/java/com/moabam/api/dto/payment/ConfirmTossPaymentRequest.java delete mode 100644 src/main/java/com/moabam/api/infrastructure/payment/TossPaymentMapper.java create mode 100644 src/main/java/com/moabam/global/error/exception/TossPaymentException.java diff --git a/src/main/java/com/moabam/api/application/payment/PaymentService.java b/src/main/java/com/moabam/api/application/payment/PaymentService.java index 6ace0d48..710e37b0 100644 --- a/src/main/java/com/moabam/api/application/payment/PaymentService.java +++ b/src/main/java/com/moabam/api/application/payment/PaymentService.java @@ -13,9 +13,7 @@ import com.moabam.api.dto.payment.ConfirmPaymentRequest; import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; import com.moabam.api.dto.payment.PaymentRequest; -import com.moabam.api.infrastructure.payment.TossPaymentMapper; import com.moabam.api.infrastructure.payment.TossPaymentService; -import com.moabam.global.error.exception.MoabamException; import com.moabam.global.error.exception.NotFoundException; import lombok.RequiredArgsConstructor; @@ -38,24 +36,26 @@ public void request(Long memberId, Long paymentId, PaymentRequest request) { payment.request(request.orderId()); } - @Transactional - public void confirm(Long memberId, ConfirmPaymentRequest request) { + public Payment validateInfo(Long memberId, ConfirmPaymentRequest request) { Payment payment = getByOrderId(request.orderId()); payment.validateInfo(memberId, request.amount()); - try { - ConfirmTossPaymentResponse response = tossPaymentService.confirm( - TossPaymentMapper.toConfirmRequest(request.paymentKey(), request.orderId(), request.amount()) - ); - payment.confirm(response.paymentKey(), response.approvedAt()); + return payment; + } + + @Transactional + public void confirm(Long memberId, Payment payment, ConfirmTossPaymentResponse response) { + payment.confirm(response.paymentKey()); - if (payment.isCouponApplied()) { - couponService.discount(payment.getCouponWalletId(), memberId); - } - bugService.charge(memberId, payment.getProduct()); - } catch (MoabamException exception) { - payment.fail(request.paymentKey()); + if (payment.isCouponApplied()) { + couponService.discount(payment.getCouponWalletId(), memberId); } + bugService.charge(memberId, payment.getProduct()); + } + + @Transactional + public void fail(Payment payment, String paymentKey) { + payment.fail(paymentKey); } private Payment getById(Long paymentId) { diff --git a/src/main/java/com/moabam/api/domain/payment/Payment.java b/src/main/java/com/moabam/api/domain/payment/Payment.java index f2056499..db81105b 100644 --- a/src/main/java/com/moabam/api/domain/payment/Payment.java +++ b/src/main/java/com/moabam/api/domain/payment/Payment.java @@ -133,9 +133,9 @@ public void request(String orderId) { this.requestedAt = LocalDateTime.now(); } - public void confirm(String paymentKey, LocalDateTime approvedAt) { + public void confirm(String paymentKey) { this.paymentKey = paymentKey; - this.approvedAt = approvedAt; + this.approvedAt = LocalDateTime.now(); this.status = PaymentStatus.DONE; } diff --git a/src/main/java/com/moabam/api/dto/payment/ConfirmTossPaymentRequest.java b/src/main/java/com/moabam/api/dto/payment/ConfirmTossPaymentRequest.java deleted file mode 100644 index 3527f148..00000000 --- a/src/main/java/com/moabam/api/dto/payment/ConfirmTossPaymentRequest.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.moabam.api.dto.payment; - -import lombok.Builder; - -@Builder -public record ConfirmTossPaymentRequest( - String paymentKey, - String orderId, - int amount -) { - -} diff --git a/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentMapper.java b/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentMapper.java deleted file mode 100644 index 146036eb..00000000 --- a/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentMapper.java +++ /dev/null @@ -1,18 +0,0 @@ -package com.moabam.api.infrastructure.payment; - -import com.moabam.api.dto.payment.ConfirmTossPaymentRequest; - -import lombok.AccessLevel; -import lombok.NoArgsConstructor; - -@NoArgsConstructor(access = AccessLevel.PRIVATE) -public final class TossPaymentMapper { - - public static ConfirmTossPaymentRequest toConfirmRequest(String paymentKey, String orderId, int amount) { - return ConfirmTossPaymentRequest.builder() - .paymentKey(paymentKey) - .orderId(orderId) - .amount(amount) - .build(); - } -} diff --git a/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentService.java b/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentService.java index 2389746e..6c10bd5a 100644 --- a/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentService.java +++ b/src/main/java/com/moabam/api/infrastructure/payment/TossPaymentService.java @@ -6,13 +6,14 @@ import org.springframework.http.HttpStatusCode; import org.springframework.http.MediaType; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.client.WebClient; -import com.moabam.api.dto.payment.ConfirmTossPaymentRequest; +import com.moabam.api.dto.payment.ConfirmPaymentRequest; import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; import com.moabam.global.config.TossPaymentConfig; -import com.moabam.global.error.exception.MoabamException; +import com.moabam.global.error.exception.TossPaymentException; import com.moabam.global.error.model.ErrorResponse; import jakarta.annotation.PostConstruct; @@ -20,6 +21,7 @@ import reactor.core.publisher.Mono; @Service +@Transactional(readOnly = true) @RequiredArgsConstructor public class TossPaymentService { @@ -37,13 +39,14 @@ public void init() { .build(); } - public ConfirmTossPaymentResponse confirm(ConfirmTossPaymentRequest request) { + @Transactional + public ConfirmTossPaymentResponse confirm(ConfirmPaymentRequest request) { return webClient.post() .uri("/v1/payments/confirm") .body(BodyInserters.fromValue(request)) .retrieve() .onStatus(HttpStatusCode::isError, response -> response.bodyToMono(ErrorResponse.class) - .flatMap(error -> Mono.error(new MoabamException(error.message())))) + .flatMap(error -> Mono.error(new TossPaymentException(error.message())))) .bodyToMono(ConfirmTossPaymentResponse.class) .block(); } diff --git a/src/main/java/com/moabam/api/presentation/PaymentController.java b/src/main/java/com/moabam/api/presentation/PaymentController.java index e294f6fc..33ea55e4 100644 --- a/src/main/java/com/moabam/api/presentation/PaymentController.java +++ b/src/main/java/com/moabam/api/presentation/PaymentController.java @@ -9,10 +9,14 @@ import org.springframework.web.bind.annotation.RestController; import com.moabam.api.application.payment.PaymentService; +import com.moabam.api.domain.payment.Payment; import com.moabam.api.dto.payment.ConfirmPaymentRequest; +import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; import com.moabam.api.dto.payment.PaymentRequest; +import com.moabam.api.infrastructure.payment.TossPaymentService; import com.moabam.global.auth.annotation.Auth; import com.moabam.global.auth.model.AuthMember; +import com.moabam.global.error.exception.TossPaymentException; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; @@ -23,6 +27,7 @@ public class PaymentController { private final PaymentService paymentService; + private final TossPaymentService tossPaymentService; @PostMapping("/{paymentId}") @ResponseStatus(HttpStatus.OK) @@ -34,6 +39,14 @@ public void request(@Auth AuthMember member, @PathVariable Long paymentId, @PostMapping("/confirm") @ResponseStatus(HttpStatus.OK) public void confirm(@Auth AuthMember member, @Valid @RequestBody ConfirmPaymentRequest request) { - paymentService.confirm(member.id(), request); + Payment payment = paymentService.validateInfo(member.id(), request); + + try { + ConfirmTossPaymentResponse response = tossPaymentService.confirm(request); + paymentService.confirm(member.id(), payment, response); + } catch (TossPaymentException exception) { + paymentService.fail(payment, request.paymentKey()); + throw exception; + } } } diff --git a/src/main/java/com/moabam/global/error/exception/TossPaymentException.java b/src/main/java/com/moabam/global/error/exception/TossPaymentException.java new file mode 100644 index 00000000..2b86b1d4 --- /dev/null +++ b/src/main/java/com/moabam/global/error/exception/TossPaymentException.java @@ -0,0 +1,8 @@ +package com.moabam.global.error.exception; + +public class TossPaymentException extends MoabamException { + + public TossPaymentException(String errorMessage) { + super(errorMessage); + } +} diff --git a/src/main/java/com/moabam/global/error/handler/GlobalExceptionHandler.java b/src/main/java/com/moabam/global/error/handler/GlobalExceptionHandler.java index 7218b312..d45037c4 100644 --- a/src/main/java/com/moabam/global/error/handler/GlobalExceptionHandler.java +++ b/src/main/java/com/moabam/global/error/handler/GlobalExceptionHandler.java @@ -21,6 +21,7 @@ import com.moabam.global.error.exception.ForbiddenException; import com.moabam.global.error.exception.MoabamException; import com.moabam.global.error.exception.NotFoundException; +import com.moabam.global.error.exception.TossPaymentException; import com.moabam.global.error.exception.UnauthorizedException; import com.moabam.global.error.model.ErrorResponse; @@ -58,7 +59,7 @@ protected ErrorResponse handleBadRequestException(MoabamException moabamExceptio } @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) - @ExceptionHandler(FcmException.class) + @ExceptionHandler({FcmException.class, TossPaymentException.class}) protected ErrorResponse handleFcmException(MoabamException moabamException) { return new ErrorResponse(moabamException.getMessage(), null); } diff --git a/src/test/java/com/moabam/api/application/payment/PaymentServiceTest.java b/src/test/java/com/moabam/api/application/payment/PaymentServiceTest.java index 039563e2..3aa0e491 100644 --- a/src/test/java/com/moabam/api/application/payment/PaymentServiceTest.java +++ b/src/test/java/com/moabam/api/application/payment/PaymentServiceTest.java @@ -9,7 +9,6 @@ import java.util.Optional; import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -19,13 +18,11 @@ import com.moabam.api.application.bug.BugService; import com.moabam.api.application.coupon.CouponService; import com.moabam.api.domain.payment.Payment; -import com.moabam.api.domain.payment.PaymentStatus; import com.moabam.api.domain.payment.repository.PaymentRepository; import com.moabam.api.domain.payment.repository.PaymentSearchRepository; import com.moabam.api.dto.payment.ConfirmPaymentRequest; +import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; import com.moabam.api.dto.payment.PaymentRequest; -import com.moabam.api.infrastructure.payment.TossPaymentService; -import com.moabam.global.error.exception.MoabamException; import com.moabam.global.error.exception.NotFoundException; @ExtendWith(MockitoExtension.class) @@ -40,88 +37,56 @@ class PaymentServiceTest { @Mock CouponService couponService; - @Mock - TossPaymentService tossPaymentService; - @Mock PaymentRepository paymentRepository; @Mock PaymentSearchRepository paymentSearchRepository; - @DisplayName("결제를 요청한다.") - @Nested - class Request { - - @DisplayName("해당 결제 정보가 존재하지 않으면 예외가 발생한다.") - @Test - void not_found_exception() { - // given - Long memberId = 1L; - Long paymentId = 1L; - PaymentRequest request = new PaymentRequest(ORDER_ID); - given(paymentRepository.findById(paymentId)).willReturn(Optional.empty()); - - // when, then - assertThatThrownBy(() -> paymentService.request(memberId, paymentId, request)) - .isInstanceOf(NotFoundException.class) - .hasMessage("존재하지 않는 결제 정보입니다."); - } + @DisplayName("결제 요청 시 해당 결제 정보가 존재하지 않으면 예외가 발생한다.") + @Test + void request_not_found_exception() { + // given + Long memberId = 1L; + Long paymentId = 1L; + PaymentRequest request = new PaymentRequest(ORDER_ID); + given(paymentRepository.findById(paymentId)).willReturn(Optional.empty()); + + // when, then + assertThatThrownBy(() -> paymentService.request(memberId, paymentId, request)) + .isInstanceOf(NotFoundException.class) + .hasMessage("존재하지 않는 결제 정보입니다."); + } + + @DisplayName("결제 정보 검증 시 해당 결제 정보가 존재하지 않으면 예외가 발생한다.") + @Test + void validate_info_not_found_exception() { + // given + Long memberId = 1L; + Payment payment = payment(bugProduct()); + ConfirmPaymentRequest request = confirmPaymentRequest(); + given(paymentSearchRepository.findByOrderId(request.orderId())).willReturn(Optional.empty()); + + // when, then + assertThatThrownBy(() -> paymentService.validateInfo(memberId, request)) + .isInstanceOf(NotFoundException.class) + .hasMessage("존재하지 않는 결제 정보입니다."); } - @DisplayName("결제를 승인한다.") - @Nested - class Confirm { - - @DisplayName("해당 결제 정보가 존재하지 않으면 예외가 발생한다.") - @Test - void not_found_exception() { - // given - Long memberId = 1L; - ConfirmPaymentRequest request = confirmPaymentRequest(); - given(paymentSearchRepository.findByOrderId(request.orderId())).willReturn(Optional.empty()); - - // when, then - assertThatThrownBy(() -> paymentService.confirm(memberId, request)) - .isInstanceOf(NotFoundException.class) - .hasMessage("존재하지 않는 결제 정보입니다."); - } - - @DisplayName("쿠폰을 적용한 경우 쿠폰을 차감한 후 벌레를 충전한다.") - @Test - void use_coupon_success() { - // given - Long memberId = 1L; - Long couponWalletId = 1L; - Payment payment = paymentWithCoupon(bugProduct(), discount1000Coupon(), couponWalletId); - ConfirmPaymentRequest request = confirmPaymentRequest(); - given(paymentSearchRepository.findByOrderId(request.orderId())).willReturn(Optional.of(payment)); - given(tossPaymentService.confirm(confirmTossPaymentRequest())).willReturn(confirmTossPaymentResponse()); - - // when - paymentService.confirm(memberId, request); - - // then - verify(couponService, times(1)).discount(couponWalletId, memberId); - verify(bugService, times(1)).charge(memberId, payment.getProduct()); - } - - @DisplayName("실패한다.") - @Test - void fail() { - // given - Long memberId = 1L; - Long couponWalletId = 1L; - Payment payment = paymentWithCoupon(bugProduct(), discount1000Coupon(), couponWalletId); - ConfirmPaymentRequest request = confirmPaymentRequest(); - given(paymentSearchRepository.findByOrderId(request.orderId())).willReturn(Optional.of(payment)); - given(tossPaymentService.confirm(any())).willThrow(MoabamException.class); - - // when - paymentService.confirm(memberId, request); - - // then - assertThat(payment.getStatus()).isEqualTo(PaymentStatus.ABORTED); - } + @DisplayName("결제 승인 시 쿠폰을 적용한 경우 쿠폰을 차감한 후 벌레를 충전한다.") + @Test + void confirm_with_coupon_success() { + // given + Long memberId = 1L; + Long couponWalletId = 1L; + Payment payment = paymentWithCoupon(bugProduct(), discount1000Coupon(), couponWalletId); + ConfirmTossPaymentResponse response = confirmTossPaymentResponse(); + + // when + paymentService.confirm(memberId, payment, response); + + // then + verify(couponService, times(1)).discount(couponWalletId, memberId); + verify(bugService, times(1)).charge(memberId, payment.getProduct()); } } diff --git a/src/test/java/com/moabam/api/infrastructure/payment/TossPaymentServiceTest.java b/src/test/java/com/moabam/api/infrastructure/payment/TossPaymentServiceTest.java index e9a57d0e..10617019 100644 --- a/src/test/java/com/moabam/api/infrastructure/payment/TossPaymentServiceTest.java +++ b/src/test/java/com/moabam/api/infrastructure/payment/TossPaymentServiceTest.java @@ -15,7 +15,7 @@ import org.springframework.test.context.ActiveProfiles; import com.fasterxml.jackson.databind.ObjectMapper; -import com.moabam.api.dto.payment.ConfirmTossPaymentRequest; +import com.moabam.api.dto.payment.ConfirmPaymentRequest; import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; import com.moabam.global.config.TossPaymentConfig; import com.moabam.global.error.exception.MoabamException; @@ -58,7 +58,7 @@ class Confirm { @Test void success() throws Exception { // given - ConfirmTossPaymentRequest request = confirmTossPaymentRequest(); + ConfirmPaymentRequest request = confirmPaymentRequest(); ConfirmTossPaymentResponse expected = confirmTossPaymentResponse(); mockWebServer.enqueue(new MockResponse() .setResponseCode(200) @@ -76,7 +76,7 @@ void success() throws Exception { @Test void exception() { // given - ConfirmTossPaymentRequest request = confirmTossPaymentRequest(); + ConfirmPaymentRequest request = confirmPaymentRequest(); String jsonString = "{\"code\":\"NOT_FOUND_PAYMENT\",\"message\":\"존재하지 않는 결제 입니다.\"}"; mockWebServer.enqueue(new MockResponse() .setResponseCode(404) diff --git a/src/test/java/com/moabam/api/presentation/PaymentControllerTest.java b/src/test/java/com/moabam/api/presentation/PaymentControllerTest.java index 1fba2dca..cca7b738 100644 --- a/src/test/java/com/moabam/api/presentation/PaymentControllerTest.java +++ b/src/test/java/com/moabam/api/presentation/PaymentControllerTest.java @@ -34,6 +34,7 @@ import com.moabam.api.dto.payment.ConfirmPaymentRequest; import com.moabam.api.dto.payment.PaymentRequest; import com.moabam.api.infrastructure.payment.TossPaymentService; +import com.moabam.global.error.exception.TossPaymentException; import com.moabam.support.annotation.WithMember; import com.moabam.support.common.WithoutFilterSupporter; @@ -116,7 +117,7 @@ void success() throws Exception { Payment payment = paymentRepository.save(payment(product)); payment.request(ORDER_ID); ConfirmPaymentRequest request = confirmPaymentRequest(); - given(tossPaymentService.confirm(confirmTossPaymentRequest())).willReturn(confirmTossPaymentResponse()); + given(tossPaymentService.confirm(request)).willReturn(confirmTossPaymentResponse()); given(memberService.findMember(memberId)).willReturn(member()); // expected @@ -149,5 +150,29 @@ void bad_request_body_exception(String paymentKey, String orderId, int amount) t .andExpect(jsonPath("$.message").value("올바른 요청 정보가 아닙니다.")) .andDo(print()); } + + @DisplayName("토스 결제 승인 요청이 실패하면 예외가 발생한다.") + @WithMember + @Test + void confirm_toss_exception() throws Exception { + // given + Long memberId = getAuthMember().id(); + Product product = productRepository.save(bugProduct()); + Payment payment = paymentRepository.save(payment(product)); + payment.request(ORDER_ID); + ConfirmPaymentRequest request = confirmPaymentRequest(); + given(memberService.findMember(memberId)).willReturn(member()); + given(tossPaymentService.confirm(request)).willThrow(TossPaymentException.class); + + // expected + mockMvc.perform(post("/payments/confirm") + .contentType(APPLICATION_JSON) + .content(objectMapper.writeValueAsString(request))) + .andExpect(status().isInternalServerError()) + .andDo(print()); + System.out.println(payment.getStatus()); + assertThat(payment.getPaymentKey()).isEqualTo(PAYMENT_KEY); + assertThat(payment.getStatus()).isEqualTo(PaymentStatus.ABORTED); + } } } diff --git a/src/test/java/com/moabam/support/fixture/PaymentFixture.java b/src/test/java/com/moabam/support/fixture/PaymentFixture.java index a46e4b7b..019b72ab 100644 --- a/src/test/java/com/moabam/support/fixture/PaymentFixture.java +++ b/src/test/java/com/moabam/support/fixture/PaymentFixture.java @@ -10,7 +10,6 @@ import com.moabam.api.domain.payment.PaymentStatus; import com.moabam.api.domain.product.Product; import com.moabam.api.dto.payment.ConfirmPaymentRequest; -import com.moabam.api.dto.payment.ConfirmTossPaymentRequest; import com.moabam.api.dto.payment.ConfirmTossPaymentResponse; public final class PaymentFixture { @@ -53,14 +52,6 @@ public static ConfirmPaymentRequest confirmPaymentRequest() { .build(); } - public static ConfirmTossPaymentRequest confirmTossPaymentRequest() { - return ConfirmTossPaymentRequest.builder() - .paymentKey(PAYMENT_KEY) - .orderId(ORDER_ID) - .amount(AMOUNT) - .build(); - } - public static ConfirmTossPaymentResponse confirmTossPaymentResponse() { return ConfirmTossPaymentResponse.builder() .paymentKey(PAYMENT_KEY)