jaxrs, payment: Fixes #821

Modify InvoicePaymentControlPluginApi and jaxrs layer to differenciate use cases for invalid (invoice) payments and already paid invoices.
Code will return a 400 when amount is invalid -- too big -- and a 204 invoice was already paid.

jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/JaxRsResourceBase.java

@@ -568,8 +568,11 @@ protected Payment createPurchaseForInvoice(final Account account, final UUID inv
             return paymentApi.createPurchaseWithPaymentControl(account, paymentMethodId, null, amountToPay, account.getCurrency(), paymentExternalKey, transactionExternalKey,
                                                                properties, createInvoicePaymentControlPluginApiPaymentOptions(externalPayment), callContext);
         } catch (final PaymentApiException e) {
-            if (e.getCode() == ErrorCode.PAYMENT_PLUGIN_EXCEPTION.getCode() &&
-                e.getMessage().contains("Aborted Payment for invoice")) {
+
+            if (e.getCode() == ErrorCode.PAYMENT_PLUGIN_EXCEPTION.getCode() /* &&
+                e.getMessage().contains("Invalid amount") */) { /* Plugin received bad input */
+                throw e;
+            } else if (e.getCode() == ErrorCode.PAYMENT_PLUGIN_API_ABORTED.getCode()) { /* Plugin aborted the call (e.g invoice already paid) */
                 return null;
             }
             throw e;

payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java

@@ -334,14 +334,6 @@ private PriorPaymentControlResult getPluginPurchaseResult(final PaymentControlCo
             // Is remaining amount > 0 ?
             final BigDecimal requestedAmount = validateAndComputePaymentAmount(invoice, paymentControlPluginContext.getAmount(), paymentControlPluginContext.isApiPayment());
             if (requestedAmount.compareTo(BigDecimal.ZERO) <= 0) {
-                if (paymentControlPluginContext.isApiPayment()) {
-                    throw new PaymentControlApiException("Abort purchase call: ", new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION,
-                                                                                                          String.format("Aborted Payment for invoice %s : invoice balance is = %s, requested payment amount is = %s",
-                                                                                                                        invoice.getId(),
-                                                                                                                        invoice.getBalance(),
-                                                                                                                        paymentControlPluginContext.getAmount())));
-
-                }
                 return new DefaultPriorPaymentControlResult(true);
             }
 
@@ -654,23 +646,25 @@ public boolean apply(final PaymentTransactionModelDao input) {
         return false;
     }
 
-    private BigDecimal validateAndComputePaymentAmount(final Invoice invoice, @Nullable final BigDecimal inputAmount, final boolean isApiPayment) {
+    private BigDecimal validateAndComputePaymentAmount(final Invoice invoice, @Nullable final BigDecimal inputAmount, final boolean isApiPayment) throws PaymentControlApiException {
 
         if (invoice.getBalance().compareTo(BigDecimal.ZERO) <= 0) {
             log.info("invoiceId='{}' has already been paid", invoice.getId());
             return BigDecimal.ZERO;
         }
+
         if (isApiPayment &&
             inputAmount != null &&
             invoice.getBalance().compareTo(inputAmount) < 0) {
-            log.info("invoiceId='{}' has a balance='{}' < retry paymentAmount='{}'", invoice.getId(), invoice.getBalance().floatValue(), inputAmount.floatValue());
-            return BigDecimal.ZERO;
-        }
-        if (inputAmount == null) {
-            return invoice.getBalance();
-        } else {
-            return invoice.getBalance().compareTo(inputAmount) < 0 ? invoice.getBalance() : inputAmount;
+            log.info("invoiceId='{}' has a balance='{}' < paymentAmount='{}'", invoice.getId(), invoice.getBalance().floatValue(), inputAmount.floatValue());
+            throw new PaymentControlApiException("Abort purchase call: ", new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION,
+                                                                                                  String.format("Invalid amount '%s' for invoice '%s': invoice balance is = '%s'",
+                                                                                                                inputAmount,
+                                                                                                                invoice.getId(),
+                                                                                                                invoice.getBalance())));
         }
+
+        return (inputAmount == null || invoice.getBalance().compareTo(inputAmount) < 0) ? invoice.getBalance() : inputAmount;
     }
 
     private boolean insert_AUTO_PAY_OFF_ifRequired(final PaymentControlContext paymentControlContext, final BigDecimal computedAmount) {

profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestInvoicePayment.java

@@ -24,6 +24,8 @@
 import java.util.UUID;
 
 import org.joda.time.DateTime;
+import org.killbill.billing.catalog.api.BillingPeriod;
+import org.killbill.billing.catalog.api.ProductCategory;
 import org.killbill.billing.client.KillBillClientException;
 import org.killbill.billing.client.model.Account;
 import org.killbill.billing.client.model.Invoice;
@@ -36,10 +38,12 @@
 import org.killbill.billing.client.model.PaymentMethod;
 import org.killbill.billing.client.model.PaymentTransaction;
 import org.killbill.billing.client.model.Payments;
+import org.killbill.billing.client.model.Subscription;
 import org.killbill.billing.osgi.api.OSGIServiceRegistration;
 import org.killbill.billing.payment.api.TransactionType;
 import org.killbill.billing.payment.plugin.api.PaymentPluginApi;
 import org.killbill.billing.payment.provider.MockPaymentProviderPlugin;
+import org.killbill.billing.util.tag.ControlTagType;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -50,6 +54,8 @@
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
 
 public class TestInvoicePayment extends TestJaxrsBase {
 
@@ -64,6 +70,9 @@ public void beforeMethod() throws Exception {
         mockPaymentProviderPlugin = (MockPaymentProviderPlugin) registry.getServiceForName(PLUGIN_NAME);
     }
 
+
+
+
     @Test(groups = "slow")
     public void testRetrievePayment() throws Exception {
         final InvoicePayment paymentJson = setupScenarioWithPayment();
@@ -252,7 +261,7 @@ public void testPaymentsAndRefundsPagination() throws Exception {
             Assert.assertTrue(paymentsPage.get(0).equals((Payment) allPayments.get(i)));
             paymentsPage = paymentsPage.getNext();
         }
-        Assert.assertNull(paymentsPage);
+        assertNull(paymentsPage);
     }
 
     @Test(groups = "slow")
@@ -287,6 +296,55 @@ public void testWithFailedInvoicePayment() throws Exception {
         }
     }
 
+
+    @Test(groups = "slow")
+    public void testManualInvoicePayment() throws Exception {
+
+        final Account accountJson = createAccountWithDefaultPaymentMethod();
+        assertNotNull(accountJson);
+
+        // Disable automatic payments
+        killBillClient.createAccountTag(accountJson.getAccountId(), ControlTagType.AUTO_PAY_OFF.getId(), requestOptions);
+
+        // Add a bundle, subscription and move the clock to get the first invoice
+        final Subscription subscriptionJson = createEntitlement(accountJson.getAccountId(), UUID.randomUUID().toString(), "Shotgun",
+                                                                ProductCategory.BASE, BillingPeriod.MONTHLY, true);
+        assertNotNull(subscriptionJson);
+        clock.addDays(32);
+        crappyWaitForLackOfProperSynchonization();
+
+        final List<Invoice> invoices = killBillClient.getInvoicesForAccount(accountJson.getAccountId(), requestOptions);
+        assertEquals(invoices.size(), 2);
+
+
+        final InvoicePayment invoicePayment1 = new InvoicePayment();
+        invoicePayment1.setPurchasedAmount(invoices.get(1).getBalance().add(BigDecimal.TEN));
+        invoicePayment1.setAccountId(accountJson.getAccountId());
+        invoicePayment1.setTargetInvoiceId(invoices.get(1).getInvoiceId());
+
+        // Pay too too much => 400
+        try {
+            killBillClient.createInvoicePayment(invoicePayment1, false, requestOptions);
+        } catch (final KillBillClientException e) {
+            assertTrue(true);
+        }
+
+
+        final InvoicePayment invoicePayment2 = new InvoicePayment();
+        invoicePayment2.setPurchasedAmount(invoices.get(1).getBalance());
+        invoicePayment2.setAccountId(accountJson.getAccountId());
+        invoicePayment2.setTargetInvoiceId(invoices.get(1).getInvoiceId());
+
+        // Just right, Yah! => 201
+        final InvoicePayment result2 = killBillClient.createInvoicePayment(invoicePayment2, false, requestOptions);
+        assertEquals(result2.getTransactions().size(), 1);
+        assertTrue(result2.getTransactions().get(0).getAmount().compareTo(invoices.get(1).getBalance()) == 0);
+
+        // Already paid -> 204
+        final InvoicePayment result3 = killBillClient.createInvoicePayment(invoicePayment2, false, requestOptions);
+        assertNull(result3);
+    }
+
     private BigDecimal getFractionOfAmount(final BigDecimal amount) {
         return amount.divide(BigDecimal.TEN).setScale(2, BigDecimal.ROUND_HALF_UP);
     }