killbill-memoizeit

payment: set processed amount to 0 in case of failure This

2/3/2016 11:17:45 PM

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
index d768769..11b050c 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
@@ -223,6 +223,8 @@ public class TestInvoicePayment extends TestIntegrationBase {
         assertTrue(invoice1.getPaidAmount().compareTo(BigDecimal.ZERO) == 0);
         assertTrue(invoice1.getChargedAmount().compareTo(new BigDecimal("249.95")) == 0);
         assertEquals(invoice1.getPayments().size(), 1);
+        assertEquals(invoice1.getPayments().get(0).getAmount().compareTo(BigDecimal.ZERO), 0);
+        assertEquals(invoice1.getPayments().get(0).getCurrency(), Currency.USD);
         assertFalse(invoice1.getPayments().get(0).isSuccess());
 
         final BigDecimal accountBalance1 = invoiceUserApi.getAccountBalance(account.getId(), callContext);
@@ -230,6 +232,11 @@ public class TestInvoicePayment extends TestIntegrationBase {
 
         final List<Payment> payments = paymentApi.getAccountPayments(account.getId(), false, ImmutableList.<PluginProperty>of(), callContext);
         assertEquals(payments.size(), 1);
+        assertEquals(payments.get(0).getTransactions().size(), 1);
+        assertEquals(payments.get(0).getTransactions().get(0).getAmount().compareTo(new BigDecimal("249.95")), 0);
+        assertEquals(payments.get(0).getTransactions().get(0).getCurrency(), Currency.USD);
+        assertEquals(payments.get(0).getTransactions().get(0).getProcessedAmount().compareTo(BigDecimal.ZERO), 0);
+        assertEquals(payments.get(0).getTransactions().get(0).getProcessedCurrency(), Currency.USD);
 
         // Trigger the payment retry
         busHandler.pushExpectedEvents(NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
@@ -245,5 +252,13 @@ public class TestInvoicePayment extends TestIntegrationBase {
 
         final BigDecimal accountBalance2 = invoiceUserApi.getAccountBalance(account.getId(), callContext);
         assertTrue(accountBalance2.compareTo(BigDecimal.ZERO) == 0);
+
+        final List<Payment> payments2 = paymentApi.getAccountPayments(account.getId(), false, ImmutableList.<PluginProperty>of(), callContext);
+        assertEquals(payments2.size(), 1);
+        assertEquals(payments2.get(0).getTransactions().size(), 2);
+        assertEquals(payments2.get(0).getTransactions().get(1).getAmount().compareTo(new BigDecimal("249.95")), 0);
+        assertEquals(payments2.get(0).getTransactions().get(1).getCurrency(), Currency.USD);
+        assertEquals(payments2.get(0).getTransactions().get(1).getProcessedAmount().compareTo(new BigDecimal("249.95")), 0);
+        assertEquals(payments2.get(0).getTransactions().get(1).getProcessedCurrency(), Currency.USD);
     }
 }
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
index 15dc191..c71407d 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
@@ -211,11 +211,23 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
         // Recompute new lastSuccessPaymentState. This is important to be able to allow new operations on the state machine (for e.g an AUTH_SUCCESS would now allow a CAPTURE operation)
         final String lastSuccessPaymentState = paymentStateMachineHelper.isSuccessState(newPaymentState) ? newPaymentState : null;
 
-        // Update the processedAmount, processedCurrency if we got a paymentTransactionInfoPlugin from the plugin and if this is a non error state
-        final BigDecimal processedAmount = (paymentTransactionInfoPlugin != null && isPendingOrFinalTransactionStatus(transactionStatus)) ?
-                                           paymentTransactionInfoPlugin.getAmount() : paymentTransaction.getProcessedAmount();
-        final Currency processedCurrency = (paymentTransactionInfoPlugin != null && isPendingOrFinalTransactionStatus(transactionStatus)) ?
-                                           paymentTransactionInfoPlugin.getCurrency() : paymentTransaction.getProcessedCurrency();
+        // Update processedAmount and processedCurrency
+        final BigDecimal processedAmount;
+        if (TransactionStatus.SUCCESS.equals(transactionStatus) || TransactionStatus.PENDING.equals(transactionStatus)) {
+            if (paymentTransactionInfoPlugin == null || paymentTransactionInfoPlugin.getAmount() == null) {
+                processedAmount = paymentTransaction.getProcessedAmount();
+            } else {
+                processedAmount = paymentTransactionInfoPlugin.getAmount();
+            }
+        } else {
+            processedAmount = BigDecimal.ZERO;
+        }
+        final Currency processedCurrency;
+        if (paymentTransactionInfoPlugin == null || paymentTransactionInfoPlugin.getCurrency() == null) {
+            processedCurrency = paymentTransaction.getProcessedCurrency();
+        } else {
+            processedCurrency = paymentTransactionInfoPlugin.getCurrency();
+        }
 
         // Update the gatewayErrorCode, gatewayError if we got a paymentTransactionInfoPlugin
         final String gatewayErrorCode = paymentTransactionInfoPlugin != null ? paymentTransactionInfoPlugin.getGatewayErrorCode() : paymentTransaction.getGatewayErrorCode();
@@ -239,12 +251,6 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
         return (newTransactionStatus != TransactionStatus.UNKNOWN) ? newTransactionStatus : currentTransactionStatus;
     }
 
-    private boolean isPendingOrFinalTransactionStatus(final TransactionStatus transactionStatus) {
-        return (transactionStatus == TransactionStatus.PENDING ||
-                transactionStatus == TransactionStatus.SUCCESS ||
-                transactionStatus == TransactionStatus.PAYMENT_FAILURE);
-    }
-
     private PaymentPluginApi getPaymentPluginApi(final PaymentModelDao item, final String pluginName) {
         final PaymentPluginApi pluginApi = pluginRegistry.getServiceForName(pluginName);
         Preconditions.checkState(pluginApi != null, "Janitor IncompletePaymentTransactionTask cannot retrieve PaymentPluginApi for plugin %s (payment id %s), skipping", pluginName, item.getId());
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentAutomatonDAOHelper.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentAutomatonDAOHelper.java
index 2b3a215..5b273be 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentAutomatonDAOHelper.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentAutomatonDAOHelper.java
@@ -103,8 +103,22 @@ public class PaymentAutomatonDAOHelper {
 
     public void processPaymentInfoPlugin(final TransactionStatus transactionStatus, @Nullable final PaymentTransactionInfoPlugin paymentInfoPlugin,
                                          final String currentPaymentStateName) {
-        final BigDecimal processedAmount = paymentInfoPlugin == null ? null : paymentInfoPlugin.getAmount();
-        final Currency processedCurrency = paymentInfoPlugin == null ? null : paymentInfoPlugin.getCurrency();
+        final BigDecimal processedAmount;
+        if (TransactionStatus.SUCCESS.equals(transactionStatus) || TransactionStatus.PENDING.equals(transactionStatus)) {
+            if (paymentInfoPlugin == null || paymentInfoPlugin.getAmount() == null) {
+                processedAmount = paymentStateContext.getAmount();
+            } else {
+                processedAmount = paymentInfoPlugin.getAmount();
+            }
+        } else {
+            processedAmount = BigDecimal.ZERO;
+        }
+        final Currency processedCurrency;
+        if (paymentInfoPlugin == null || paymentInfoPlugin.getCurrency() == null) {
+            processedCurrency = paymentStateContext.getCurrency();
+        } else {
+            processedCurrency = paymentInfoPlugin.getCurrency();
+        }
         final String gatewayErrorCode = paymentInfoPlugin == null ? null : paymentInfoPlugin.getGatewayErrorCode();
         final String gatewayErrorMsg = paymentInfoPlugin == null ? null : paymentInfoPlugin.getGatewayError();
 
diff --git a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java
index 0cf263e..1a1bd0d 100644
--- a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java
@@ -216,7 +216,7 @@ public final class InvoicePaymentControlPluginApi implements PaymentControlPlugi
                 try {
                     log.debug("Notifying invoice of failed payment: id={}, amount={}, currency={}, invoiceId={}", paymentControlContext.getPaymentId(), paymentControlContext.getAmount(), paymentControlContext.getCurrency(), invoiceId);
                     invoiceApi.notifyOfPayment(invoiceId,
-                                               paymentControlContext.getAmount(),
+                                               BigDecimal.ZERO,
                                                paymentControlContext.getCurrency(),
                                                // processed currency may be null so we use currency; processed currency will be updated if/when payment succeeds
                                                paymentControlContext.getCurrency(),
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 d5c1aa5..d6e293e 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
@@ -182,7 +182,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
         assertEquals(payment.getTransactions().get(0).getPaymentId(), payment.getId());
         assertEquals(payment.getTransactions().get(0).getAmount().compareTo(requestedAmount), 0);
         assertEquals(payment.getTransactions().get(0).getCurrency(), Currency.AED);
-        assertEquals(payment.getTransactions().get(0).getProcessedAmount().compareTo(requestedAmount), 0);
+        assertEquals(payment.getTransactions().get(0).getProcessedAmount().compareTo(BigDecimal.ZERO), 0);
         assertEquals(payment.getTransactions().get(0).getProcessedCurrency(), Currency.AED);
 
         assertEquals(payment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);
@@ -405,7 +405,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
         assertEquals(payment.getTransactions().get(0).getPaymentId(), payment.getId());
         assertEquals(payment.getTransactions().get(0).getAmount().compareTo(requestedAmount), 0);
         assertEquals(payment.getTransactions().get(0).getCurrency(), Currency.USD);
-        assertEquals(payment.getTransactions().get(0).getProcessedAmount().compareTo(requestedAmount), 0); // This is weird...
+        assertEquals(payment.getTransactions().get(0).getProcessedAmount().compareTo(BigDecimal.ZERO), 0);
         assertEquals(payment.getTransactions().get(0).getProcessedCurrency(), Currency.USD);
 
         assertEquals(payment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);