killbill-aplcache
Merge remote-tracking branch 'origin/master' into invoice-date-work Conflicts: invoice/src/main/java/com/ning/billing/invoice/api/user/DefaultInvoiceUserApi.java invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java jaxrs/src/main/java/com/ning/billing/jaxrs/resources/CreditResource.java Signed-off-by: …
7/14/2012 10:27:05 PM
Changes
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 90db0b1..5ee9262 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;
@@ -124,18 +125,25 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
@Override
public InvoiceItem getCreditById(final UUID creditId) throws InvoiceApiException {
- return dao.getCreditById(creditId);
+ final 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 LocalDate 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(final UUID accountId, final UUID invoiceId, final BigDecimal amount,
final LocalDate effectiveDate, final Currency currency, final 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 73d6cba..6ffb3d1 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
@@ -307,11 +307,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;
@@ -340,7 +344,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
BigDecimal cbaAdjAmount = BigDecimal.ZERO;
if (accountCbaAvailable.compareTo(BigDecimal.ZERO) > 0) {
cbaAdjAmount = (requestedPositiveAmount.compareTo(accountCbaAvailable) > 0) ? accountCbaAvailable.negate() : requestedPositiveAmount.negate();
- final InvoiceItem cbaAdjItem = new CreditBalanceAdjInvoiceItem(invoice.getId(), invoice.getAccountId(), new LocalDate(context.getCreatedDate()), cbaAdjAmount, invoice.getCurrency());
+ final InvoiceItem cbaAdjItem = new CreditBalanceAdjInvoiceItem(invoice.getId(), invoice.getAccountId(), context.getCreatedDate().toLocalDate(), cbaAdjAmount, invoice.getCurrency());
transInvoiceItemDao.create(cbaAdjItem, context);
}
final BigDecimal requestedPositiveAmountAfterCbaAdj = requestedPositiveAmount.add(cbaAdjAmount);
@@ -349,7 +353,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
final BigDecimal maxBalanceToAdjust = (invoiceBalanceAfterRefund.compareTo(BigDecimal.ZERO) <= 0) ? BigDecimal.ZERO : invoiceBalanceAfterRefund;
final BigDecimal requestedPositiveAmountToAdjust = requestedPositiveAmountAfterCbaAdj.compareTo(maxBalanceToAdjust) > 0 ? maxBalanceToAdjust : requestedPositiveAmountAfterCbaAdj;
if (requestedPositiveAmountToAdjust.compareTo(BigDecimal.ZERO) > 0) {
- final InvoiceItem adjItem = new RefundAdjInvoiceItem(invoice.getId(), invoice.getAccountId(), new LocalDate(context.getCreatedDate()), requestedPositiveAmountToAdjust.negate(), invoice.getCurrency());
+ final InvoiceItem adjItem = new RefundAdjInvoiceItem(invoice.getId(), invoice.getAccountId(), context.getCreatedDate().toLocalDate(), requestedPositiveAmountToAdjust.negate(), invoice.getCurrency());
transInvoiceItemDao.create(adjItem, context);
}
}
@@ -427,10 +431,9 @@ 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 LocalDate effectiveDate, final Currency currency,
final CallContext context) {
-
return invoiceSqlDao.inTransaction(new Transaction<InvoiceItem, InvoiceSqlDao>() {
@Override
public InvoiceItem inTransaction(final InvoiceSqlDao transactional, final TransactionStatus status) throws Exception {
@@ -441,9 +444,24 @@ 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().toLocalDate(),
+ 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 dcaa40b..083a3c8 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
@@ -27,6 +27,7 @@ import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.LocalDate;
import org.mockito.Mockito;
+import org.testng.Assert;
import org.testng.annotations.Test;
import com.ning.billing.catalog.DefaultPrice;
@@ -47,6 +48,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;
@@ -739,6 +741,111 @@ public class TestInvoiceDao extends InvoiceDaoTestBase {
}
+
+ @Test(groups = "slow")
+ public void testAccountCredit() {
+
+ final UUID accountId = UUID.randomUUID();
+
+ final LocalDate effectiveDate = new LocalDate(2011, 3, 1);
+
+ 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 LocalDate targetDate = new LocalDate(2011, 2, 15);
+ final Invoice invoice1 = new DefaultInvoice(accountId, clock.getUTCToday(), targetDate, Currency.USD);
+ invoiceDao.create(invoice1, invoice1.getTargetDate().getDayOfMonth(), context);
+
+ final LocalDate startDate = new LocalDate(2011, 3, 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,
+ amount1, Currency.USD);
+ invoiceItemSqlDao.create(item1, context);
+
+ // Create the credit item
+ final LocalDate effectiveDate = new LocalDate(2011, 3, 1);
+
+ 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 adad09b..f5711d7 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;
@@ -32,6 +33,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;
@@ -68,19 +70,18 @@ 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 Account account = accountUserApi.getAccountById(credit.getAccountId());
+ final CreditJson creditJson = new CreditJson(credit, account.getTimeZone());
+ 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 Account account = accountUserApi.getAccountById(credit.getAccountId());
- final CreditJson creditJson = new CreditJson(credit, account.getTimeZone());
- 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) {
- log.warn(String.format("Failed to locate credit for id %s", creditId), e);
- return Response.status(Response.Status.NO_CONTENT).build();
} catch (AccountApiException e) {
log.warn(String.format("Failed to locate account for credit id %s", creditId), e);
- return Response.status(Response.Status.NO_CONTENT).build();
+ return Response.status(Response.Status.NOT_FOUND).entity(e.getMessage()).type(MediaType.TEXT_PLAIN_TYPE).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();
}
}