Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAPHY-140 refactor: 결제 기능 리펙토링 #89

Merged
merged 1 commit into from
Oct 2, 2024
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
17 changes: 12 additions & 5 deletions src/main/java/saphy/saphy/pay/domain/Pay.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import jakarta.persistence.*;
import jakarta.persistence.Id;
import java.math.BigDecimal;
import java.time.LocalDateTime;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand All @@ -27,6 +26,12 @@ public class Pay extends BaseEntity {
@JoinColumn(name = "item_id", nullable = false)
private Item item;

@Column(name = "merchant_id", unique = true)
private String merchantUid;

@Column(name = "imp_uid", unique = true)
private String impUid;

@Column(name = "amount", nullable = false)
private BigDecimal amount;

Expand All @@ -38,10 +43,12 @@ public class Pay extends BaseEntity {
@Column(name = "status", nullable = false)
private PayStatus status;

@Column(name = "transaction_id", unique = true)
private String transactionId;
public void setImpUid(String impUid) {
this.impUid = impUid;
}

@Column(name = "payment_date")
private LocalDateTime payDate;
public void setStatus(PayStatus status) {
this.status = status;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
import saphy.saphy.pay.domain.Pay;

public interface PayRepository extends JpaRepository<Pay, Long> {
Pay findByMerchantUid(String merchantUid);

}
12 changes: 12 additions & 0 deletions src/main/java/saphy/saphy/pay/dto/request/PayCompleteRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package saphy.saphy.pay.dto.request;

import java.math.BigDecimal;
import lombok.Getter;

@Getter
public class PayCompleteRequest {
private String merchantUid;
private Long itemId;
private String impUid;
private BigDecimal amount;
}
27 changes: 27 additions & 0 deletions src/main/java/saphy/saphy/pay/dto/request/PayPrepareRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package saphy.saphy.pay.dto.request;

import java.math.BigDecimal;
import lombok.Getter;
import saphy.saphy.item.domain.Item;
import saphy.saphy.pay.domain.Pay;
import saphy.saphy.pay.domain.PayMethod;
import saphy.saphy.pay.domain.PayStatus;

@Getter
public class PayPrepareRequest {
private Long itemId;
private int quantity;
private BigDecimal amount;
private PayMethod payMethod;

public Pay toEntity(Item item, String merchantUid, BigDecimal amount, PayMethod payMethod ){
return Pay.builder()
.item(item)
.merchantUid(merchantUid)
.amount(amount)
.payMethod(payMethod)
.status(PayStatus.PENDING)
.build();
}

}
40 changes: 0 additions & 40 deletions src/main/java/saphy/saphy/pay/dto/request/PayRequest.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package saphy.saphy.pay.dto.response;

import lombok.Getter;
import saphy.saphy.pay.domain.PayStatus;

@Getter
public class PayCompleteResponse {
private PayStatus status;


public PayCompleteResponse(PayStatus status) {
this.status = status;
}
}
15 changes: 15 additions & 0 deletions src/main/java/saphy/saphy/pay/dto/response/PayPrepareResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package saphy.saphy.pay.dto.response;

import java.math.BigDecimal;
import lombok.Getter;

@Getter
public class PayPrepareResponse {
private String merchantUid;
private BigDecimal amount;

public PayPrepareResponse(String merchantUid, BigDecimal amount) {
this.merchantUid = merchantUid;
this.amount = amount;
}
}
17 changes: 0 additions & 17 deletions src/main/java/saphy/saphy/pay/dto/response/PayResponse.java

This file was deleted.

26 changes: 13 additions & 13 deletions src/main/java/saphy/saphy/pay/presentation/PayController.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package saphy.saphy.pay.presentation;

import com.siot.IamportRestClient.exception.IamportResponseException;
import io.swagger.v3.oas.annotations.tags.Tag;
import java.io.IOException;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import saphy.saphy.global.response.ApiResponse;
import saphy.saphy.pay.dto.request.PayPrepareRequest;
import saphy.saphy.pay.dto.response.PayPrepareResponse;
import saphy.saphy.pay.service.PayService;
import saphy.saphy.pay.dto.request.PayRequest;
import saphy.saphy.pay.dto.response.PayResponse;
import saphy.saphy.pay.dto.request.PayCompleteRequest;
import saphy.saphy.pay.dto.response.PayCompleteResponse;

@RestController
@RequestMapping("/payments")
Expand All @@ -22,18 +24,16 @@
public class PayController {
private final PayService payService;

@PostMapping
public ApiResponse<PayResponse> processPayment(@RequestBody PayRequest request){
log.warn("impId >> " + request.getImpUid());
PayResponse response = payService.processPayment(request);

@PostMapping("/prepare")
public ApiResponse<PayPrepareResponse> preparePayment(@RequestBody PayPrepareRequest request) {
PayPrepareResponse response = payService.preparePay(request);
return new ApiResponse<>(response);
}

@GetMapping("/{paymentId}")
public ApiResponse<PayResponse> getPaymentDetails(@PathVariable Long paymentId) {
PayResponse response = payService.getPaymentDetails(paymentId);

@PostMapping("/complete")
public ApiResponse<PayCompleteResponse> completePayment(@RequestBody PayCompleteRequest request)
throws IamportResponseException, IOException {
PayCompleteResponse response = payService.completePay(request);
return new ApiResponse<>(response);
}
}
71 changes: 36 additions & 35 deletions src/main/java/saphy/saphy/pay/service/PayService.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,32 @@
package saphy.saphy.pay.service;

import static saphy.saphy.global.exception.ErrorCode.INVALID_REQUEST;
import static saphy.saphy.global.exception.ErrorCode.ITEM_OUT_OF_STOCK;
import static saphy.saphy.global.exception.ErrorCode.PAY_FAILURE;
import static saphy.saphy.global.exception.ErrorCode.PAY_INVALID;
import static saphy.saphy.global.exception.ErrorCode.PAY_NOT_FOUND;
import static saphy.saphy.global.exception.ErrorCode.PAY_PRICE_MISMATCH;

import com.siot.IamportRestClient.IamportClient;
import com.siot.IamportRestClient.exception.IamportResponseException;
import com.siot.IamportRestClient.request.CancelData;
import com.siot.IamportRestClient.response.IamportResponse;
import com.siot.IamportRestClient.response.Payment;
import jakarta.transaction.Transactional;
import java.io.IOException;
import java.math.BigDecimal;
import java.util.Optional;
import java.util.UUID;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Service;
import saphy.saphy.global.exception.SaphyException;
import saphy.saphy.item.domain.Item;
import saphy.saphy.item.domain.repository.ItemRepository;
import saphy.saphy.pay.domain.Pay;
import saphy.saphy.pay.domain.PayStatus;
import saphy.saphy.pay.domain.repository.PayRepository;
import saphy.saphy.pay.dto.request.PayRequest;
import saphy.saphy.pay.dto.response.PayResponse;
import saphy.saphy.pay.dto.request.PayCompleteRequest;
import saphy.saphy.pay.dto.request.PayPrepareRequest;
import saphy.saphy.pay.dto.response.PayCompleteResponse;
import saphy.saphy.pay.dto.response.PayPrepareResponse;

@Service
@RequiredArgsConstructor
Expand All @@ -34,56 +37,54 @@ public class PayService {
private final IamportClient iamportClient;
Copy link
Contributor

@holyPigeon holyPigeon Oct 2, 2024

Choose a reason for hiding this comment

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

여기서 ItemRepository -> ItemRepository<Item> 으로 바꾸면 밑에 로직에서 Optional을 안 써도 될 것 같습니다!



/**
* 결제를 진행합니다.
* */
@Transactional
public PayResponse processPayment(PayRequest request) {
public PayPrepareResponse preparePay(PayPrepareRequest request) {
// Item을 상속받는 엔티티들이 존재하기에, Optional로 받아옴
Optional<Item> optionalItem = itemRepository.findById(request.getItemId());

Item item = optionalItem.get();

Payment iamportPayment = getIamportPayment(request);
if (item.getStock() < request.getQuantity()) {
throw SaphyException.from(ITEM_OUT_OF_STOCK);
}
Comment on lines +47 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

어 이거 제가 1시간 전에 PR 올린 거에 들어있는 거긴 한데
item 엔티티 안에 재고에 따라 주문할 수 있는지 없는지 boolean canOrder() 메서드로 구현해놔서 나중에 바꿔도 될 것 같습니다!


// 물품 금액과 결제 금액이 같은지 검증
if (!iamportPayment.getAmount().equals(item.getPrice())) {
BigDecimal calculatedAmount = item.getPrice().multiply(BigDecimal.valueOf(request.getQuantity()));
Copy link
Member

Choose a reason for hiding this comment

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

오 BigDecimal이라 직접 안곱하고 multiply를 쓰는거군요
하나 배워갑니다 👏

if (!calculatedAmount.equals(request.getAmount())) {
throw SaphyException.from(PAY_PRICE_MISMATCH);
}
Comment on lines +47 to 54
Copy link
Member

Choose a reason for hiding this comment

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

나중에 이런 조건문들은 private 메서드로 따로 빼서 리팩토링 하면 더 코드가 깔끔해질 거 같아요!


Pay pay = request.toEntity(item, iamportPayment);

String merchantUid = generateMerchantUid();
Pay pay = request.toEntity(item, merchantUid, calculatedAmount, request.getPayMethod());
payRepository.save(pay);

// 재고 감소
item.decreaseStock(1);
item.decreaseStock(request.getQuantity());
itemRepository.save(item);

return new PayResponse(pay.getId(), pay.getTransactionId(), pay.getStatus());
return new PayPrepareResponse(merchantUid, calculatedAmount);
}

@NotNull
private Payment getIamportPayment(PayRequest request) {
IamportResponse<Payment> iamportResponse;
@Transactional
public PayCompleteResponse completePay(PayCompleteRequest request) throws IamportResponseException, IOException {
IamportResponse<Payment> payResponse = iamportClient.paymentByImpUid(request.getImpUid());

Pay pay = payRepository.findByMerchantUid(request.getMerchantUid());

try {
// PortOne 결제 확인
iamportResponse = iamportClient.paymentByImpUid(request.getImpUid());
if (payResponse.getResponse().getAmount().equals(pay.getAmount())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 if문 조건은 뭔가 private 메서드로 뽑을 수 있으면 훨씬 깔끔할 것 같습니다!

pay.setStatus(PayStatus.PAID);
pay.setImpUid(request.getImpUid());
payRepository.save(pay);

return new PayCompleteResponse(PayStatus.PAID);
} else {
iamportClient.cancelPaymentByImpUid(new CancelData(request.getImpUid(), true));
pay.setStatus(PayStatus.FAILED);
payRepository.save(pay);

} catch (IOException e) {
throw SaphyException.from(INVALID_REQUEST);
} catch (IamportResponseException | IllegalArgumentException e) {
throw SaphyException.from(PAY_FAILURE);
}

return Optional.ofNullable(iamportResponse.getResponse())
.orElseThrow(() -> SaphyException.from(PAY_INVALID));
}

@Transactional
public PayResponse getPaymentDetails(Long paymentId) {
Pay pay = payRepository.findById(paymentId)
.orElseThrow(() -> SaphyException.from(PAY_NOT_FOUND));
return new PayResponse(pay.getId(), pay.getTransactionId(), pay.getStatus());
private String generateMerchantUid() {
return "ORD-" + UUID.randomUUID();
Copy link
Member

Choose a reason for hiding this comment

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

UUID 앞에 ORD를 붙여서 직관적이라 좋은 거 같아요! 예전에 재우님 코드 리뷰에 있었던 내용을 반영한 건가요~?

}
}
2 changes: 1 addition & 1 deletion src/main/java/saphy/saphy/sales/domain/Sales.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ public class Sales extends BaseEntity {
private Member member;

@OneToOne(fetch = FetchType.LAZY) //여기 애매하긴 한데, 일단 1:1관계로 생각함.
@JoinColumn(name = "order_id")
@JoinColumn(name = "order_id") // 여기 이름 바꿔야 함
private Purchase purchase;
}
Loading