killbill-memoizeit

explicitly reject the duplicated txn on sucessful external

11/4/2016 7:20:13 PM

Details

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 a4f2337..05a5829 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
@@ -405,12 +405,8 @@ public class PaymentProcessor extends ProcessorBase {
                     // We can't tell where we should be in the state machine - bail (cannot be enforced by the state machine unfortunately because UNKNOWN and PLUGIN_FAILURE are both treated as EXCEPTION)
                     if (transactionToComplete.getTransactionStatus() == TransactionStatus.UNKNOWN) {
                         throw new PaymentApiException(ErrorCode.PAYMENT_INVALID_OPERATION, paymentStateContext.getTransactionType(), transactionToComplete.getTransactionStatus());
-                    }
-
-                    // If Janitor confirmed the status, just return the status to the client.
-                    if (transactionType != TransactionType.AUTHORIZE &&
-                        (transactionToComplete.getTransactionStatus() == TransactionStatus.SUCCESS || transactionToComplete.getTransactionStatus() == TransactionStatus.PAYMENT_FAILURE)) {
-                        return getPayment(paymentId, true, properties, callContext, internalCallContext);
+                    } else if (transactionToComplete.getTransactionStatus() == TransactionStatus.SUCCESS ){
+                        throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
                     }
 
                     paymentStateContext.setPaymentTransactionModelDao(transactionToComplete);
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 8e5421f..d840924 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
@@ -1446,7 +1446,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             final Payment completedPayment = createPayment(TransactionType.AUTHORIZE, initialPayment.getId(), initialPayment.getExternalKey(), transactionExternalKey, authAmount, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_INVALID_OPERATION.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
         }
     }
 
@@ -1464,7 +1464,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             final Payment completedPayment = createPayment(TransactionType.AUTHORIZE, initialPayment.getId(), initialPayment.getExternalKey(), transactionExternalKey, authAmount, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_INVALID_OPERATION.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
         }
     }
 
@@ -1741,10 +1741,16 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
         mockPaymentProviderPlugin.overridePaymentPluginStatus(pendingPayment.getId(), pendingPayment.getTransactions().get(1).getId(), PaymentPluginStatus.PROCESSED);
 
         // 2nd capture request with identical transaction external key
-        pendingPayment = createPayment(TransactionType.CAPTURE, authorization.getId(), paymentExternalKey, paymentTransactionExternalKey, requestedAmount, PaymentPluginStatus.PROCESSED);
-        Assert.assertEquals(pendingPayment.getTransactions().size(), 2);
-        Assert.assertEquals(pendingPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
-        Assert.assertEquals(pendingPayment.getTransactions().get(1).getTransactionStatus(), TransactionStatus.SUCCESS);
+        try {
+            createPayment(TransactionType.CAPTURE, authorization.getId(), paymentExternalKey, paymentTransactionExternalKey, requestedAmount, PaymentPluginStatus.PROCESSED);
+            Assert.fail();
+        } catch (final PaymentApiException e) {
+            final Payment refreshedPayment = paymentApi.getPayment(pendingPayment.getId(), true, ImmutableList.<PluginProperty>of(), callContext);
+            Assert.assertEquals(refreshedPayment.getTransactions().size(), 2);
+            Assert.assertEquals(refreshedPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+            Assert.assertEquals(refreshedPayment.getTransactions().get(1).getTransactionStatus(), TransactionStatus.SUCCESS);
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+        }
     }
 
     @Test(groups = "slow")
@@ -1800,9 +1806,10 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
 
         // 2nd capture request with identical transaction external key
         pendingPayment = createPayment(TransactionType.CAPTURE, authorization.getId(), paymentExternalKey, paymentTransactionExternalKey, requestedAmount, PaymentPluginStatus.PROCESSED);
-        Assert.assertEquals(pendingPayment.getTransactions().size(), 2);
+        Assert.assertEquals(pendingPayment.getTransactions().size(), 3);
         Assert.assertEquals(pendingPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
         Assert.assertEquals(pendingPayment.getTransactions().get(1).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);
+        Assert.assertEquals(pendingPayment.getTransactions().get(2).getTransactionStatus(), TransactionStatus.SUCCESS);
     }
 
     @Test(groups = "slow")