killbill-aplcache

Fix issue with credit-- to also include CBA when needed-- Add

7/14/2012 9:55:46 PM

Details

diff --git a/api/src/main/java/com/ning/billing/ErrorCode.java b/api/src/main/java/com/ning/billing/ErrorCode.java
index 8ea1ee2..5f94c15 100644
--- a/api/src/main/java/com/ning/billing/ErrorCode.java
+++ b/api/src/main/java/com/ning/billing/ErrorCode.java
@@ -185,6 +185,7 @@ public enum ErrorCode {
     INVOICE_TARGET_DATE_TOO_FAR_IN_THE_FUTURE(4005, "The target date was too far in the future. Target Date: %s"),
     INVOICE_NOT_FOUND(4006, "No invoice could be found for id %s."),
     INVOICE_NOTHING_TO_DO(4007, "No invoice to generate for account %s and date %s"),
+    INVOICE_NO_SUCH_CREDIT(4008, "Credit Item for id %s does not exist"),
 
     /*
      *
@@ -198,7 +199,7 @@ public enum ErrorCode {
     CHARGE_BACK_DOES_NOT_EXIST(4004, "Could not find chargeback for id %s."),
     INVOICE_PAYMENT_BY_ATTEMPT_NOT_FOUND(4905, "No invoice payment could be found for paymentAttempt id %s."),
     REFUND_AMOUNT_TOO_HIGH(4906, "Tried to refund %s of a %s payment."),
-    REFUND_AMOUNT_IS_POSITIVE(4907, "Refund for positve amounts are not permitted"),
+    CREDIT_AMOUNT_INVALID(4907, "Credit amount %s should be striclty positive"),
 
     /*
      *
diff --git a/invoice/src/main/java/com/ning/billing/invoice/api/user/DefaultInvoiceUserApi.java b/invoice/src/main/java/com/ning/billing/invoice/api/user/DefaultInvoiceUserApi.java
index 19b5523..3228d1d 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/api/user/DefaultInvoiceUserApi.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/api/user/DefaultInvoiceUserApi.java
@@ -36,6 +36,7 @@ import com.ning.billing.invoice.api.InvoiceItem;
 import com.ning.billing.invoice.api.InvoicePayment;
 import com.ning.billing.invoice.api.InvoiceUserApi;
 import com.ning.billing.invoice.dao.InvoiceDao;
+import com.ning.billing.invoice.model.CreditAdjInvoiceItem;
 import com.ning.billing.invoice.template.HtmlInvoiceGenerator;
 import com.ning.billing.util.api.TagApiException;
 import com.ning.billing.util.callcontext.CallContext;
@@ -114,19 +115,26 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
 
     @Override
     public InvoiceItem getCreditById(final UUID creditId) throws InvoiceApiException {
-        return dao.getCreditById(creditId);
+        InvoiceItem creditItem = dao.getCreditById(creditId);
+        if (creditItem == null) {
+            throw new InvoiceApiException(ErrorCode.INVOICE_NO_SUCH_CREDIT, creditId);
+        }
+        return new CreditAdjInvoiceItem(creditItem.getId(), creditItem.getInvoiceId(), creditItem.getAccountId(), creditItem.getStartDate(), creditItem.getAmount().negate(), creditItem.getCurrency());
     }
 
     @Override
     public InvoiceItem insertCredit(final UUID accountId, final BigDecimal amount, final DateTime effectiveDate,
                                     final Currency currency, final CallContext context) throws InvoiceApiException {
-        return dao.insertCredit(accountId, null, amount, effectiveDate, currency, context);
+        return insertCreditForInvoice(accountId, null, amount, effectiveDate, currency, context);
     }
 
     @Override
     public InvoiceItem insertCreditForInvoice(UUID accountId, UUID invoiceId,
             BigDecimal amount, DateTime effectiveDate, Currency currency,
             CallContext context) throws InvoiceApiException {
+        if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
+            throw new InvoiceApiException(ErrorCode.CREDIT_AMOUNT_INVALID, amount);
+        }
         return dao.insertCredit(accountId, invoiceId, amount, effectiveDate, currency, context);
     }
 
diff --git a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
index 2dec344..255a683 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
@@ -306,11 +306,15 @@ public class DefaultInvoiceDao implements InvoiceDao {
                 }
                 final BigDecimal maxRefundAmount = payment.getAmount() == null ? BigDecimal.ZERO : payment.getAmount();
                 final BigDecimal requestedPositiveAmount = amount == null ? maxRefundAmount : amount;
+                // This check is good but not enough, we need to also take into account previous refunds
+                // (But that should have been checked in the payment call already)
                 if (requestedPositiveAmount.compareTo(maxRefundAmount) > 0) {
                     throw new InvoiceApiException(ErrorCode.REFUND_AMOUNT_TOO_HIGH, requestedPositiveAmount, maxRefundAmount);
                 }
 
-                // Before we go further, check if that refund already got inserted
+                // Before we go further, check if that refund already got inserted -- the payment system keps a state machine
+                // and so this call may be called several time for the same  paymentCookieId (which is really the refundId)
+                //
                 final InvoicePayment existingRefund = transactional.getPaymentsForCookieId(paymentCookieId.toString());
                 if (existingRefund != null) {
                     return existingRefund;
@@ -426,7 +430,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
     }
 
     @Override
-    public InvoiceItem insertCredit(final UUID accountId, final UUID invoiceId, final BigDecimal amount,
+    public InvoiceItem insertCredit(final UUID accountId, final UUID invoiceId, final BigDecimal positiveCreditAmount,
                                     final DateTime effectiveDate, final Currency currency,
                                     final CallContext context) {
 
@@ -440,9 +444,23 @@ public class DefaultInvoiceDao implements InvoiceDao {
                     invoiceIdForRefund = invoiceForRefund.getId();
                 }
 
-                final InvoiceItem credit = new CreditAdjInvoiceItem(invoiceIdForRefund, accountId, effectiveDate, amount, currency);
+                final InvoiceItem credit = new CreditAdjInvoiceItem(invoiceIdForRefund, accountId, effectiveDate, positiveCreditAmount.negate(), currency);
                 final InvoiceItemSqlDao transInvoiceItemDao = transactional.become(InvoiceItemSqlDao.class);
                 transInvoiceItemDao.create(credit, context);
+
+
+                final Invoice invoice = transactional.getById(invoiceIdForRefund.toString());
+                if (invoice != null) {
+                    populateChildren(invoice, transactional);
+                } else {
+                    throw new IllegalStateException("Invoice shouldn't be null for credit at this stage " + invoiceIdForRefund);
+                }
+                // If invoice balance becomes negative we add some CBA item
+                if (invoice.getBalance().compareTo(BigDecimal.ZERO) < 0) {
+                    final InvoiceItem cbaAdjItem = new CreditBalanceAdjInvoiceItem(invoice.getId(), invoice.getAccountId(), context.getCreatedDate(), invoice.getBalance().negate(), invoice.getCurrency());
+                    transInvoiceItemDao.create(cbaAdjItem, context);
+
+                }
                 return credit;
             }
         });
diff --git a/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java b/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
index 6f3594c..70501c0 100644
--- a/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
+++ b/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
@@ -25,6 +25,7 @@ import java.util.UUID;
 
 import org.joda.time.DateTime;
 import org.mockito.Mockito;
+import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.ning.billing.catalog.DefaultPrice;
@@ -45,6 +46,7 @@ import com.ning.billing.invoice.MockBillingEventSet;
 import com.ning.billing.invoice.api.Invoice;
 import com.ning.billing.invoice.api.InvoiceApiException;
 import com.ning.billing.invoice.api.InvoiceItem;
+import com.ning.billing.invoice.api.InvoiceItemType;
 import com.ning.billing.invoice.api.InvoicePayment;
 import com.ning.billing.invoice.api.InvoicePayment.InvoicePaymentType;
 import com.ning.billing.invoice.model.CreditAdjInvoiceItem;
@@ -770,6 +772,112 @@ public class TestInvoiceDao extends InvoiceDaoTestBase {
 
     }
 
+
+    @Test(groups = "slow")
+    public void testAccountCredit() {
+
+        final UUID accountId = UUID.randomUUID();
+
+        final DateTime effectiveDate = new DateTime(2011, 3, 1, 0, 0, 0, 0);
+
+        final BigDecimal creditAmount = new BigDecimal("5.0");
+
+        invoiceDao.insertCredit(accountId, null, creditAmount, effectiveDate, Currency.USD, context);
+
+        final List<Invoice> invoices = invoiceDao.getAllInvoicesByAccount(accountId);
+        assertEquals(invoices.size(), 1);
+
+        Invoice invoice = invoices.get(0);
+        assertTrue(invoice.getBalance().compareTo(BigDecimal.ZERO) == 0);
+        List<InvoiceItem> invoiceItems = invoice.getInvoiceItems();
+        assertEquals(invoiceItems.size(), 2);
+        boolean foundCredit = false;
+        boolean foundCBA = false;
+        for (InvoiceItem cur : invoiceItems) {
+            if (cur.getInvoiceItemType() == InvoiceItemType.CREDIT_ADJ) {
+                foundCredit = true;
+                assertTrue(cur.getAmount().compareTo(creditAmount.negate()) == 0);
+            } else if (cur.getInvoiceItemType() == InvoiceItemType.CBA_ADJ) {
+                foundCBA = true;
+                assertTrue(cur.getAmount().compareTo(creditAmount) == 0);
+            }
+        }
+        assertTrue(foundCredit);
+        assertTrue(foundCBA);
+    }
+
+
+    @Test(groups = "slow")
+    public void testInvoiceCreditWithBalancePositive() {
+        final BigDecimal creditAmount = new BigDecimal("2.0");
+        final BigDecimal expectedBalance = new BigDecimal("3.0");
+        final boolean expectCBA = false;
+        testInvoiceCreditInternal(creditAmount, expectedBalance, expectCBA);
+    }
+
+    @Test(groups = "slow")
+    public void testInvoiceCreditWithBalanceNegative() {
+        final BigDecimal creditAmount = new BigDecimal("7.0");
+        final BigDecimal expectedBalance = new BigDecimal("0.0");
+        final boolean expectCBA = true;
+        testInvoiceCreditInternal(creditAmount, expectedBalance, expectCBA);
+    }
+
+    @Test(groups = "slow")
+    public void testInvoiceCreditWithBalanceZero() {
+        final BigDecimal creditAmount = new BigDecimal("5.0");
+        final BigDecimal expectedBalance = new BigDecimal("0.0");
+        final boolean expectCBA = false;
+        testInvoiceCreditInternal(creditAmount, expectedBalance, expectCBA);
+    }
+
+    private void testInvoiceCreditInternal(BigDecimal creditAmount, BigDecimal expectedBalance, boolean expectCBA) {
+
+        final UUID accountId = UUID.randomUUID();
+        final UUID bundleId = UUID.randomUUID();
+
+
+        // Crete one invoice with a fixed invoice item
+        final DateTime targetDate = new DateTime(2011, 2, 15, 0, 0, 0, 0);
+        final Invoice invoice1 = new DefaultInvoice(accountId, clock.getUTCNow(), targetDate, Currency.USD);
+        invoiceDao.create(invoice1, invoice1.getTargetDate().getDayOfMonth(), context);
+
+        final DateTime startDate = new DateTime(2011, 3, 1, 0, 0, 0, 0);
+        final DateTime endDate = startDate.plusMonths(1);
+
+        final BigDecimal amount1 = new BigDecimal("5.0");
+
+        // Fixed Item
+        final FixedPriceInvoiceItem item1 = new FixedPriceInvoiceItem(invoice1.getId(), accountId, bundleId, UUID.randomUUID(), "test plan", "test phase A", startDate,
+                                                                      endDate, amount1, Currency.USD);
+        invoiceItemSqlDao.create(item1, context);
+
+        // Create the credit item
+        final DateTime effectiveDate = new DateTime(2011, 3, 1, 0, 0, 0, 0);
+
+        invoiceDao.insertCredit(accountId, invoice1.getId(), creditAmount, effectiveDate, Currency.USD, context);
+
+        final List<Invoice> invoices = invoiceDao.getAllInvoicesByAccount(accountId);
+        assertEquals(invoices.size(), 1);
+
+        Invoice invoice = invoices.get(0);
+        assertTrue(invoice.getBalance().compareTo(expectedBalance) == 0);
+        List<InvoiceItem> invoiceItems = invoice.getInvoiceItems();
+        assertEquals(invoiceItems.size(), expectCBA ? 3 : 2);
+        boolean foundCredit = false;
+        boolean foundCBA = false;
+        for (InvoiceItem cur : invoiceItems) {
+            if (cur.getInvoiceItemType() == InvoiceItemType.CREDIT_ADJ) {
+                foundCredit = true;
+                assertTrue(cur.getAmount().compareTo(creditAmount.negate()) == 0);
+            } else if (cur.getInvoiceItemType() == InvoiceItemType.CBA_ADJ) {
+                foundCBA = true;
+            }
+        }
+        assertEquals(foundCBA, expectCBA);
+        assertTrue(foundCredit);
+    }
+
     @Test(groups = "slow")
     public void testGetUnpaidInvoicesByAccountId() {
         final UUID accountId = UUID.randomUUID();
diff --git a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/CreditResource.java b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/CreditResource.java
index 74ea6a3..4d016a8 100644
--- a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/CreditResource.java
+++ b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/CreditResource.java
@@ -23,6 +23,7 @@ import javax.ws.rs.POST;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import java.util.UUID;
 
@@ -31,6 +32,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import com.ning.billing.ErrorCode;
 import com.ning.billing.account.api.Account;
 import com.ning.billing.account.api.AccountApiException;
 import com.ning.billing.account.api.AccountUserApi;
@@ -67,16 +69,14 @@ public class CreditResource implements JaxrsResource {
     public Response getCredit(@PathParam("creditId") final String creditId) {
         try {
             final InvoiceItem credit = invoiceUserApi.getCreditById(UUID.fromString(creditId));
-            if (credit == null) {
-                return Response.status(Response.Status.NOT_FOUND).build();
+            final CreditJson creditJson = new CreditJson(credit);
+            return Response.status(Response.Status.OK).entity(creditJson).build();
+        } catch (InvoiceApiException e) {
+            if (e.getCode() == ErrorCode.INVOICE_NO_SUCH_CREDIT.getCode()) {
+                return Response.status(Response.Status.NOT_FOUND).entity(e.getMessage()).type(MediaType.TEXT_PLAIN_TYPE).build();
             } else {
-                final CreditJson creditJson = new CreditJson(credit);
-                return Response.status(Response.Status.OK).entity(creditJson).build();
+                return Response.status(Response.Status.BAD_REQUEST).entity(e.getMessage()).type(MediaType.TEXT_PLAIN_TYPE).build();
             }
-        } catch (InvoiceApiException e) {
-            final String error = String.format("Failed to locate credit for id %s", creditId);
-            log.info(error, e);
-            return Response.status(Response.Status.NO_CONTENT).build();
         }
     }
 
diff --git a/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java b/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
index 1bcedf9..9e2f0af 100644
--- a/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
+++ b/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
@@ -17,10 +17,13 @@
 package com.ning.billing.payment.api;
 
 import java.math.BigDecimal;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
 import com.google.inject.Inject;
 import com.ning.billing.ErrorCode;
 import com.ning.billing.account.api.Account;
diff --git a/payment/src/test/java/com/ning/billing/payment/dao/MockPaymentDao.java b/payment/src/test/java/com/ning/billing/payment/dao/MockPaymentDao.java
index c1ff2b7..9f4775c 100644
--- a/payment/src/test/java/com/ning/billing/payment/dao/MockPaymentDao.java
+++ b/payment/src/test/java/com/ning/billing/payment/dao/MockPaymentDao.java
@@ -16,6 +16,7 @@
 package com.ning.billing.payment.dao;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -160,32 +161,26 @@ public class MockPaymentDao implements PaymentDao {
     @Override
     public RefundModelDao insertRefund(RefundModelDao refundInfo,
             CallContext context) {
-        // TODO Auto-generated method stub
         return null;
     }
 
     @Override
     public void updateRefundStatus(UUID refundId, RefundStatus status,
             CallContext context) {
-        // TODO Auto-generated method stub
-
     }
 
     @Override
     public RefundModelDao getRefund(UUID refundId) {
-        // TODO Auto-generated method stub
         return null;
     }
 
     @Override
     public List<RefundModelDao> getRefundsForPayment(UUID paymentId) {
-        // TODO Auto-generated method stub
-        return null;
+        return Collections.emptyList();
     }
 
     @Override
     public List<RefundModelDao> getRefundsForAccount(UUID accountId) {
-        // TODO Auto-generated method stub
-        return null;
+        return Collections.emptyList();
     }
 }