-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 결제 승인 트랜잭션 분리 및 멱등성 보강 (#62) #103
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| -- Manual migration to enforce credit transaction idempotency at the database level. | ||
| -- Run after backing up the database. | ||
|
|
||
| -- Remove duplicate rows that violate the intended uniqueness rule and keep the earliest row. | ||
| WITH ranked_duplicates AS ( | ||
| SELECT | ||
| id, | ||
| ROW_NUMBER() OVER ( | ||
| PARTITION BY user_id, type, reference_id | ||
| ORDER BY id | ||
| ) AS duplicate_rank | ||
| FROM credit_transactions | ||
| WHERE reference_id IS NOT NULL | ||
| ) | ||
| DELETE FROM credit_transactions | ||
| WHERE id IN ( | ||
| SELECT id | ||
| FROM ranked_duplicates | ||
| WHERE duplicate_rank > 1 | ||
| ); | ||
|
|
||
| -- Abort before adding the constraint if duplicates still remain for any reason. | ||
| DO $$ | ||
| BEGIN | ||
| IF EXISTS ( | ||
| SELECT 1 | ||
| FROM credit_transactions | ||
| WHERE reference_id IS NOT NULL | ||
| GROUP BY user_id, type, reference_id | ||
| HAVING COUNT(*) > 1 | ||
| ) THEN | ||
| RAISE EXCEPTION | ||
| 'Duplicate credit_transactions remain for (user_id, type, reference_id); aborting unique constraint creation.'; | ||
| END IF; | ||
| END $$; | ||
|
|
||
| DO $$ | ||
| BEGIN | ||
| IF NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM information_schema.table_constraints | ||
| WHERE table_schema = current_schema() | ||
| AND table_name = 'credit_transactions' | ||
| AND constraint_name = 'uk_credit_transactions_user_type_reference' | ||
| ) THEN | ||
| ALTER TABLE credit_transactions | ||
| ADD CONSTRAINT uk_credit_transactions_user_type_reference | ||
| UNIQUE (user_id, type, reference_id); | ||
| END IF; | ||
| END $$; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| @Getter | ||
| public enum PaymentStatus { | ||
| PENDING, | ||
| PROCESSING, | ||
| FAILED, | ||
| COMPLETED | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import org.springframework.util.StringUtils; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
|
|
@@ -20,32 +21,33 @@ public class CreditService { | |
|
|
||
| @Transactional | ||
| public int charge(User user, int amount, String description, String referenceId) { | ||
| validatePositiveAmount(amount); | ||
| User managedUser = getManagedUser(user); | ||
| managedUser.increaseCredit(amount); | ||
| saveTransaction(managedUser, CreditTransactionType.CHARGE, amount, description, referenceId); | ||
| return managedUser.getCredit(); | ||
| return apply(user, CreditTransactionType.CHARGE, amount, description, referenceId); | ||
| } | ||
|
|
||
| @Transactional | ||
| public int use(User user, int amount, String description, String referenceId) { | ||
| validatePositiveAmount(amount); | ||
| User managedUser = getManagedUser(user); | ||
| try { | ||
| managedUser.decreaseCredit(amount); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new GeneralException(GeneralErrorCode.INSUFFICIENT_CREDIT, "크레딧이 부족합니다."); | ||
| } | ||
| saveTransaction(managedUser, CreditTransactionType.USE, -amount, description, referenceId); | ||
| return managedUser.getCredit(); | ||
| return apply(user, CreditTransactionType.USE, amount, description, referenceId); | ||
| } | ||
|
|
||
| @Transactional | ||
| public int refund(User user, int amount, String description, String referenceId) { | ||
| return apply(user, CreditTransactionType.REFUND, amount, description, referenceId); | ||
| } | ||
|
|
||
| private int apply(User user, CreditTransactionType type, int amount, String description, String referenceId) { | ||
| validatePositiveAmount(amount); | ||
| validateReferenceId(referenceId); | ||
| User managedUser = getManagedUser(user); | ||
| managedUser.increaseCredit(amount); | ||
| saveTransaction(managedUser, CreditTransactionType.REFUND, amount, description, referenceId); | ||
| CreditTransaction existingTransaction = creditTransactionRepository | ||
| .findByUserIdAndTypeAndReferenceId(managedUser.getId(), type, referenceId) | ||
| .orElse(null); | ||
| if (existingTransaction != null) { | ||
| return existingTransaction.getBalanceAfter(); | ||
| } | ||
|
|
||
| int transactionAmount = resolveTransactionAmount(type, amount); | ||
| applyCreditChange(managedUser, type, amount); | ||
| saveTransaction(managedUser, type, transactionAmount, description, referenceId); | ||
|
Comment on lines
+41
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reject same Line 44 returns the old balance for any existing Suggested fix+ int transactionAmount = resolveTransactionAmount(type, amount);
CreditTransaction existingTransaction = creditTransactionRepository
.findByUserIdAndTypeAndReferenceId(managedUser.getId(), type, referenceId)
.orElse(null);
if (existingTransaction != null) {
+ if (existingTransaction.getAmount() != transactionAmount) {
+ throw new GeneralException(
+ GeneralErrorCode.INVALID_PARAMETER,
+ "referenceId가 기존 거래 금액과 일치하지 않습니다."
+ );
+ }
return existingTransaction.getBalanceAfter();
}
- int transactionAmount = resolveTransactionAmount(type, amount);
applyCreditChange(managedUser, type, amount);
saveTransaction(managedUser, type, transactionAmount, description, referenceId);🤖 Prompt for AI Agents |
||
| return managedUser.getCredit(); | ||
| } | ||
|
|
||
|
|
@@ -55,6 +57,28 @@ private void validatePositiveAmount(int amount) { | |
| } | ||
| } | ||
|
|
||
| private void validateReferenceId(String referenceId) { | ||
| if (!StringUtils.hasText(referenceId)) { | ||
| throw new GeneralException(GeneralErrorCode.INVALID_PARAMETER, "referenceId는 필수입니다."); | ||
| } | ||
| } | ||
|
|
||
| private void applyCreditChange(User user, CreditTransactionType type, int amount) { | ||
| if (type == CreditTransactionType.USE) { | ||
| try { | ||
| user.decreaseCredit(amount); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new GeneralException(GeneralErrorCode.INSUFFICIENT_CREDIT, "크레딧이 부족합니다."); | ||
| } | ||
| return; | ||
| } | ||
| user.increaseCredit(amount); | ||
| } | ||
|
|
||
| private int resolveTransactionAmount(CreditTransactionType type, int amount) { | ||
| return type == CreditTransactionType.USE ? -amount : amount; | ||
| } | ||
|
|
||
| private void saveTransaction( | ||
| User user, | ||
| CreditTransactionType type, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ | |
| import com.jobdri.jobdri_api.domain.payment.entity.CreditPlan; | ||
| import com.jobdri.jobdri_api.domain.payment.entity.CreditTransactionType; | ||
| import com.jobdri.jobdri_api.domain.payment.entity.Payment; | ||
| import com.jobdri.jobdri_api.domain.payment.entity.PaymentStatus; | ||
| import com.jobdri.jobdri_api.domain.payment.repository.CreditTransactionRepository; | ||
| import com.jobdri.jobdri_api.domain.payment.repository.PaymentRepository; | ||
| import com.jobdri.jobdri_api.domain.user.entity.User; | ||
|
|
@@ -18,6 +17,7 @@ | |
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Propagation; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.util.Arrays; | ||
|
|
@@ -32,8 +32,8 @@ public class PaymentService { | |
| private final UserService userService; | ||
| private final PaymentRepository paymentRepository; | ||
| private final CreditTransactionRepository creditTransactionRepository; | ||
| private final PaymentTransactionService paymentTransactionService; | ||
| private final TossPaymentClient tossPaymentClient; | ||
| private final CreditService creditService; | ||
|
|
||
| @Value("${payment.toss.client-key:}") | ||
| private String tossClientKey; | ||
|
|
@@ -68,38 +68,24 @@ public PaymentPrepareResponse prepare(User user, PaymentPrepareRequest request) | |
| return PaymentPrepareResponse.of(payment, tossClientKey); | ||
| } | ||
|
|
||
| @Transactional | ||
| @Transactional(propagation = Propagation.NOT_SUPPORTED) | ||
| public PaymentConfirmResponse confirm(User user, PaymentConfirmRequest request) { | ||
| User validatedUser = userService.validateUser(user); | ||
| Payment payment = paymentRepository.findByOrderIdForUpdate(request.orderId()) | ||
| .orElseThrow(() -> new GeneralException( | ||
| GeneralErrorCode.PAYMENT_NOT_FOUND, | ||
| "결제 정보를 찾을 수 없습니다. orderId=" + request.orderId() | ||
| )); | ||
|
|
||
| if (!payment.getUser().getId().equals(validatedUser.getId())) { | ||
| throw new GeneralException(GeneralErrorCode.FORBIDDEN, "해당 결제에 접근할 수 없습니다."); | ||
| } | ||
| if (payment.getStatus() != PaymentStatus.PENDING) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다."); | ||
| } | ||
| if (payment.getPrice() != request.amount()) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_AMOUNT_MISMATCH, "결제 금액이 일치하지 않습니다."); | ||
| PaymentTransactionService.PaymentConfirmationStart start = | ||
| paymentTransactionService.startConfirmation(validatedUser.getId(), request); | ||
| if (start.alreadyCompleted()) { | ||
| return PaymentConfirmResponse.of(start.payment(), userService.getUser(validatedUser.getId()).getCredit()); | ||
| } | ||
|
|
||
| TossPaymentConfirmResponse tossResponse = | ||
| tossPaymentClient.confirm(request.paymentKey(), request.orderId(), request.amount()); | ||
| validateTossResponse(request, tossResponse); | ||
|
|
||
| payment.complete(request.paymentKey()); | ||
| int creditBalance = creditService.charge( | ||
| validatedUser, | ||
| payment.getCreditAmount(), | ||
| payment.getContent(), | ||
| payment.getOrderId() | ||
| ); | ||
|
|
||
| return PaymentConfirmResponse.of(payment, creditBalance); | ||
| try { | ||
| TossPaymentConfirmResponse tossResponse = | ||
| tossPaymentClient.confirm(request.paymentKey(), request.orderId(), request.amount()); | ||
| validateTossResponse(request, tossResponse); | ||
| } catch (RuntimeException e) { | ||
| paymentTransactionService.failConfirmation(validatedUser.getId(), request.orderId(), request.paymentKey()); | ||
| throw e; | ||
|
Comment on lines
+80
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Inspect Toss client failure taxonomy and confirm whether
# PaymentService can distinguish definitive payment rejection from unknown outcomes.
echo "Toss payment client definitions/usages:"
rg -n -C4 'class TossPaymentClient|interface TossPaymentClient|confirm\s*\('
echo
echo "Payment confirmation exception/status handling:"
rg -n -C4 'PAYMENT_CONFIRM_FAILED|PAYMENT_ALREADY_PROCESSED|RestClientException|WebClientResponseException|HttpStatus|RuntimeException'Repository: JobDri-Developer/BackEnd Length of output: 50380 Do not mark payments
This conflates definitive rejections with transient failures. If Toss approves but the response times out or the client fails to receive it, the payment becomes Distinguish transient/unknown outcomes from definitive rejections in 🤖 Prompt for AI Agents |
||
| } | ||
| return paymentTransactionService.completeConfirmation(validatedUser.getId(), request); | ||
| } | ||
|
|
||
| public CreditBalanceResponse getBalance(User user) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| package com.jobdri.jobdri_api.domain.payment.service; | ||
|
|
||
| import com.jobdri.jobdri_api.domain.payment.dto.request.PaymentConfirmRequest; | ||
| import com.jobdri.jobdri_api.domain.payment.dto.response.PaymentConfirmResponse; | ||
| import com.jobdri.jobdri_api.domain.payment.entity.Payment; | ||
| import com.jobdri.jobdri_api.domain.payment.entity.PaymentStatus; | ||
| import com.jobdri.jobdri_api.domain.payment.repository.PaymentRepository; | ||
| import com.jobdri.jobdri_api.domain.user.entity.User; | ||
| import com.jobdri.jobdri_api.domain.user.service.UserService; | ||
| import com.jobdri.jobdri_api.global.apiPayload.code.GeneralErrorCode; | ||
| import com.jobdri.jobdri_api.global.apiPayload.exception.GeneralException; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class PaymentTransactionService { | ||
|
|
||
| private final PaymentRepository paymentRepository; | ||
| private final UserService userService; | ||
| private final CreditService creditService; | ||
|
|
||
| @Transactional | ||
| public PaymentConfirmationStart startConfirmation(Long userId, PaymentConfirmRequest request) { | ||
| Payment payment = getOwnedPaymentForUpdate(userId, request.orderId()); | ||
| validateAmount(payment, request.amount()); | ||
|
|
||
| if (payment.getStatus() == PaymentStatus.COMPLETED) { | ||
| if (!payment.hasPaymentKey(request.paymentKey())) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다."); | ||
| } | ||
| return new PaymentConfirmationStart(payment, true); | ||
| } | ||
| if (payment.getStatus() == PaymentStatus.PROCESSING || payment.getStatus() == PaymentStatus.FAILED) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다."); | ||
| } | ||
|
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make same-key
One possible local recovery path- if (payment.getStatus() == PaymentStatus.PROCESSING || payment.getStatus() == PaymentStatus.FAILED) {
+ if (payment.getStatus() == PaymentStatus.PROCESSING) {
+ if (!payment.hasPaymentKey(request.paymentKey())) {
+ throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다.");
+ }
+ return new PaymentConfirmationStart(payment, false);
+ }
+ if (payment.getStatus() == PaymentStatus.FAILED) {
throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다.");
}🤖 Prompt for AI Agents |
||
|
|
||
| payment.markProcessing(request.paymentKey()); | ||
| return new PaymentConfirmationStart(payment, false); | ||
| } | ||
|
|
||
| @Transactional | ||
| public PaymentConfirmResponse completeConfirmation(Long userId, PaymentConfirmRequest request) { | ||
| Payment payment = getOwnedPaymentForUpdate(userId, request.orderId()); | ||
| validateAmount(payment, request.amount()); | ||
| if (!payment.hasPaymentKey(request.paymentKey())) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_CONFIRM_FAILED, "결제 승인 정보가 일치하지 않습니다."); | ||
| } | ||
| if (payment.getStatus() == PaymentStatus.COMPLETED) { | ||
| return PaymentConfirmResponse.of(payment, userService.getUser(userId).getCredit()); | ||
| } | ||
| if (payment.getStatus() != PaymentStatus.PROCESSING) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_ALREADY_PROCESSED, "이미 처리된 결제입니다."); | ||
| } | ||
|
|
||
| User user = userService.getUser(userId); | ||
| payment.complete(request.paymentKey()); | ||
| int creditBalance = creditService.charge( | ||
| user, | ||
| payment.getCreditAmount(), | ||
| payment.getContent(), | ||
| payment.getOrderId() | ||
| ); | ||
| return PaymentConfirmResponse.of(payment, creditBalance); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void failConfirmation(Long userId, String orderId, String paymentKey) { | ||
| Payment payment = getOwnedPaymentForUpdate(userId, orderId); | ||
| if (!payment.hasPaymentKey(paymentKey)) { | ||
| return; | ||
| } | ||
| if (payment.getStatus() == PaymentStatus.PROCESSING) { | ||
| payment.fail(); | ||
| } | ||
| } | ||
|
|
||
| private Payment getOwnedPaymentForUpdate(Long userId, String orderId) { | ||
| Payment payment = paymentRepository.findByOrderIdForUpdate(orderId) | ||
| .orElseThrow(() -> new GeneralException( | ||
| GeneralErrorCode.PAYMENT_NOT_FOUND, | ||
| "결제 정보를 찾을 수 없습니다. orderId=" + orderId | ||
| )); | ||
| if (!payment.belongsTo(userId)) { | ||
| throw new GeneralException(GeneralErrorCode.FORBIDDEN, "해당 결제에 접근할 수 없습니다."); | ||
| } | ||
| return payment; | ||
| } | ||
|
|
||
| private void validateAmount(Payment payment, int amount) { | ||
| if (payment.getPrice() != amount) { | ||
| throw new GeneralException(GeneralErrorCode.PAYMENT_AMOUNT_MISMATCH, "결제 금액이 일치하지 않습니다."); | ||
| } | ||
| } | ||
|
|
||
| public record PaymentConfirmationStart(Payment payment, boolean alreadyCompleted) { | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: JobDri-Developer/BackEnd
Length of output: 3867
Create an explicit migration to enforce the unique constraint and backfill existing duplicates.
The idempotency constraint
(user_id, type, reference_id)is defined only in JPA metadata at lines 13-21. No explicit Flyway/Liquibase migration exists inops/db/migrations/and the constraint is absent fromschema.sql. When deployed to production,ddl-auto: updatecannot reliably create or enforce complex constraints, and existing duplicate rows will cause the constraint application to fail.Create a migration file to:
credit_transactionsrows by(user_id, type, reference_id)ALTER TABLEAlso applies to: 45-46
🤖 Prompt for AI Agents