diff --git a/payment/src/main/java/org/killbill/billing/payment/api/DefaultPaymentApi.java b/payment/src/main/java/org/killbill/billing/payment/api/DefaultPaymentApi.java
index d73faad..ac0702a 100644
--- a/payment/src/main/java/org/killbill/billing/payment/api/DefaultPaymentApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/api/DefaultPaymentApi.java
@@ -462,7 +462,6 @@ public class DefaultPaymentApi extends DefaultApiBase implements PaymentApi {
checkNotNullParameter(currency, "currency");
}
checkNotNullParameter(paymentId, "paymentId");
- checkNotNullParameter(paymentTransactionExternalKey, "paymentTransactionExternalKey");
checkNotNullParameter(properties, "plugin properties");
final String transactionType = TransactionType.REFUND.name();
@@ -717,7 +716,6 @@ public class DefaultPaymentApi extends DefaultApiBase implements PaymentApi {
public Payment createChargebackReversal(final Account account, final UUID paymentId, final String paymentTransactionExternalKey, final CallContext callContext) throws PaymentApiException {
checkNotNullParameter(account, "account");
checkNotNullParameter(paymentId, "paymentId");
- checkNotNullParameter(paymentTransactionExternalKey, "paymentTransactionExternalKey");
final String transactionType = TransactionType.CHARGEBACK.name();
Payment payment = null;
@@ -756,7 +754,6 @@ public class DefaultPaymentApi extends DefaultApiBase implements PaymentApi {
checkNotNullParameter(account, "account");
checkNotNullParameter(paymentId, "paymentId");
- checkNotNullParameter(paymentTransactionExternalKey, "paymentTransactionExternalKey");
final String transactionType = TransactionType.CHARGEBACK.name();
Payment payment = null;
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
index e8981f9..ab61ca5 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
@@ -55,8 +55,8 @@ import org.killbill.billing.payment.api.TransactionType;
import org.killbill.billing.payment.core.janitor.IncompletePaymentTransactionTask;
import org.killbill.billing.payment.core.sm.PaymentAutomatonDAOHelper;
import org.killbill.billing.payment.core.sm.PaymentAutomatonRunner;
-import org.killbill.billing.payment.dao.PaymentAttemptModelDao;
import org.killbill.billing.payment.core.sm.PaymentStateContext;
+import org.killbill.billing.payment.dao.PaymentAttemptModelDao;
import org.killbill.billing.payment.dao.PaymentDao;
import org.killbill.billing.payment.dao.PaymentModelDao;
import org.killbill.billing.payment.dao.PaymentTransactionModelDao;
@@ -456,6 +456,11 @@ public class PaymentProcessor extends ProcessorBase {
throw new PaymentApiException(ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID, paymentStateContext.getPaymentId());
}
+ if (paymentStateContext.getPaymentTransactionExternalKey() != null) {
+ final List<PaymentTransactionModelDao> allPaymentTransactionsForKey = daoHelper.getPaymentDao().getPaymentTransactionsByExternalKey(paymentStateContext.getPaymentTransactionExternalKey(), internalCallContext);
+ runSanityOnTransactionExternalKey(allPaymentTransactionsForKey, paymentStateContext, internalCallContext);
+ }
+
if (paymentStateContext.getTransactionId() != null || paymentStateContext.getPaymentTransactionExternalKey() != null) {
// If a transaction id or key is passed, we are maybe completing an existing transaction (unless a new key was provided)
final List<PaymentTransactionModelDao> paymentTransactionsForCurrentPayment = daoHelper.getPaymentDao().getTransactionsForPayment(paymentStateContext.getPaymentId(), paymentStateContext.getInternalCallContext());
@@ -499,6 +504,31 @@ public class PaymentProcessor extends ProcessorBase {
return getPayment(nonNullPaymentId, true, false, properties, callContext, internalCallContext);
}
+ private void runSanityOnTransactionExternalKey(final Iterable<PaymentTransactionModelDao> allPaymentTransactionsForKey,
+ final PaymentStateContext paymentStateContext,
+ final InternalCallContext internalCallContext) throws PaymentApiException {
+ for (final PaymentTransactionModelDao paymentTransactionModelDao : allPaymentTransactionsForKey) {
+ // Sanity: verify we don't already have a successful transaction for that key (chargeback reversals are a bit special, it's the only transaction type we can revert)
+ if (paymentTransactionModelDao.getTransactionExternalKey().equals(paymentStateContext.getPaymentTransactionExternalKey()) &&
+ paymentTransactionModelDao.getTransactionStatus() == TransactionStatus.SUCCESS &&
+ paymentTransactionModelDao.getTransactionType() != TransactionType.CHARGEBACK) {
+ throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
+ }
+
+ // Sanity: don't share keys across accounts
+ if (!paymentTransactionModelDao.getAccountRecordId().equals(internalCallContext.getAccountRecordId())) {
+ UUID accountId;
+ try {
+ accountId = accountInternalApi.getAccountByRecordId(paymentTransactionModelDao.getAccountRecordId(), internalCallContext).getId();
+ } catch (final AccountApiException e) {
+ log.warn("Unable to retrieve account", e);
+ accountId = null;
+ }
+ throw new PaymentApiException(ErrorCode.PAYMENT_TRANSACTION_DIFFERENT_ACCOUNT_ID, accountId);
+ }
+ }
+ }
+
private PaymentTransactionModelDao findTransactionToCompleteAndRunSanityChecks(final PaymentModelDao paymentModelDao,
final Iterable<PaymentTransactionModelDao> paymentTransactionsForCurrentPayment,
final PaymentStateContext paymentStateContext,
@@ -524,26 +554,6 @@ public class PaymentProcessor extends ProcessorBase {
throw new PaymentApiException(ErrorCode.PAYMENT_INVALID_PARAMETER, "transactionType", String.format("%s doesn't match existing transaction type %s", paymentStateContext.getTransactionType(), paymentTransactionModelDao.getTransactionType()));
}
- // Sanity: verify we don't already have a successful transaction for that key (chargeback reversals are a bit special, it's the only transaction type we can revert)
- if (paymentTransactionModelDao.getTransactionExternalKey().equals(paymentStateContext.getPaymentTransactionExternalKey()) &&
- paymentTransactionModelDao.getTransactionStatus() == TransactionStatus.SUCCESS &&
- paymentTransactionModelDao.getTransactionType() != TransactionType.CHARGEBACK) {
- throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
- }
-
- // Sanity: don't share keys across accounts
- if (paymentTransactionModelDao.getTransactionExternalKey().equals(paymentStateContext.getPaymentTransactionExternalKey()) &&
- !paymentTransactionModelDao.getAccountRecordId().equals(internalCallContext.getAccountRecordId())) {
- UUID accountId;
- try {
- accountId = accountInternalApi.getAccountByRecordId(paymentModelDao.getAccountRecordId(), internalCallContext).getId();
- } catch (final AccountApiException e) {
- log.warn("Unable to retrieve account", e);
- accountId = null;
- }
- throw new PaymentApiException(ErrorCode.PAYMENT_TRANSACTION_DIFFERENT_ACCOUNT_ID, accountId);
- }
-
// UNKNOWN transactions are potential candidates, we'll invoke the Janitor first though
if (paymentTransactionModelDao.getTransactionStatus() == TransactionStatus.PENDING || paymentTransactionModelDao.getTransactionStatus() == TransactionStatus.UNKNOWN) {
completionCandidates.add(paymentTransactionModelDao);
diff --git a/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java b/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
index 16a43e7..e141506 100644
--- a/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
+++ b/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
@@ -2008,7 +2008,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
createPayment(TransactionType.CAPTURE, authorization.getId(), null, authKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
Assert.fail();
} catch (final PaymentApiException e) {
- Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_INVALID_PARAMETER.getCode());
+ Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
}
try {
@@ -2080,7 +2080,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
createPayment(TransactionType.AUTHORIZE, null, null, captureKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
Assert.fail();
} catch (final PaymentApiException e) {
- Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_INVALID_PARAMETER.getCode());
+ Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
}
try {
@@ -2098,6 +2098,44 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
Assert.assertEquals(capturedPayment2.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
Assert.assertEquals(capturedPayment2.getTransactions().get(1).getTransactionStatus(), TransactionStatus.SUCCESS);
Assert.assertEquals(capturedPayment2.getTransactions().get(2).getTransactionStatus(), TransactionStatus.SUCCESS);
+
+ final String refundKey = UUID.randomUUID().toString();
+ final Payment refundedPayment = createPayment(TransactionType.REFUND, authorization.getId(), null, refundKey, BigDecimal.ONE, PaymentPluginStatus.PROCESSED);
+ Assert.assertEquals(refundedPayment.getTransactions().size(), 4);
+ Assert.assertEquals(refundedPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+ Assert.assertEquals(refundedPayment.getTransactions().get(1).getTransactionStatus(), TransactionStatus.SUCCESS);
+ Assert.assertEquals(refundedPayment.getTransactions().get(2).getTransactionStatus(), TransactionStatus.SUCCESS);
+ Assert.assertEquals(refundedPayment.getTransactions().get(3).getTransactionStatus(), TransactionStatus.SUCCESS);
+
+ // Second payment
+
+ final String auth2Key = UUID.randomUUID().toString();
+ final Payment authorization2 = createPayment(TransactionType.AUTHORIZE, null, null, auth2Key, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
+ assertNotNull(authorization2);
+ Assert.assertEquals(authorization2.getTransactions().size(), 1);
+ Assert.assertEquals(authorization2.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+
+ try {
+ // Capture with an existing successful transaction external key should fail
+ createPayment(TransactionType.CAPTURE, authorization2.getId(), null, captureKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
+ Assert.fail();
+ } catch (final PaymentApiException e) {
+ Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+ }
+
+ final String capture2Key = UUID.randomUUID().toString();
+ final Payment capture2 = createPayment(TransactionType.CAPTURE, authorization2.getId(), null, capture2Key, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
+ Assert.assertEquals(capture2.getTransactions().size(), 2);
+ Assert.assertEquals(capture2.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+ Assert.assertEquals(capture2.getTransactions().get(1).getTransactionStatus(), TransactionStatus.SUCCESS);
+
+ try {
+ // Refund with an existing successful transaction external key should fail
+ createPayment(TransactionType.REFUND, authorization2.getId(), null, refundKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
+ Assert.fail();
+ } catch (final PaymentApiException e) {
+ Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+ }
}
@Test(groups = "slow")
@@ -2301,6 +2339,14 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
paymentTransactionExternalKey,
pluginProperties,
callContext);
+ case REFUND:
+ return paymentApi.createRefund(account,
+ paymentId,
+ amount,
+ amount == null ? null : account.getCurrency(),
+ paymentTransactionExternalKey,
+ pluginProperties,
+ callContext);
default:
Assert.fail();
return null;
