killbill-aplcache

invoice: CR for 4805b33 and a7ff76e6

10/28/2017 12:38:48 AM

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithTaxItems.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithTaxItems.java
index c3dd36c..4c88a98 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithTaxItems.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithTaxItems.java
@@ -143,7 +143,7 @@ public class TestWithTaxItems extends TestIntegrationBase {
         assertListenerStatus();
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
-        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), new TaxInvoiceItem(UUID.randomUUID(), null, account.getId(), null, "Tax Item", new LocalDate(2012, 5, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Remove AUTO_INVOICING_OFF => Invoice + Payment
         remove_AUTO_INVOICING_OFF_Tag(account.getId(), NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
@@ -159,7 +159,7 @@ public class TestWithTaxItems extends TestIntegrationBase {
         assertListenerStatus();
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
-        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), new TaxInvoiceItem(UUID.randomUUID(), null, account.getId(), null, "Tax Item", new LocalDate(2012, 5, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Remove AUTO_INVOICING_OFF => Invoice + Payment
         remove_AUTO_INVOICING_OFF_Tag(account.getId(), NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
@@ -181,7 +181,7 @@ public class TestWithTaxItems extends TestIntegrationBase {
         assertListenerStatus();
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
-        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), new TaxInvoiceItem(UUID.randomUUID(), null, account.getId(), null, "Tax Item", new LocalDate(2012, 5, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Remove AUTO_INVOICING_OFF => Invoice + Payment
         remove_AUTO_INVOICING_OFF_Tag(account.getId(), NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
@@ -224,7 +224,7 @@ public class TestWithTaxItems extends TestIntegrationBase {
                                     new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), new LocalDate(2012, 4, 1), InvoiceItemType.CREDIT_ADJ, new BigDecimal("-100")));
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
-        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), new TaxInvoiceItem(UUID.randomUUID(), null, account.getId(), null, "Tax Item", new LocalDate(2012, 4, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Verify dry-run scenario
         final Invoice dryRunInvoice = invoiceUserApi.triggerInvoiceGeneration(account.getId(), new LocalDate(2012, 5, 1), new TestDryRunArguments(DryRunType.TARGET_DATE), callContext);
@@ -234,7 +234,7 @@ public class TestWithTaxItems extends TestIntegrationBase {
                                                                                        new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), new LocalDate(2012, 4, 1), InvoiceItemType.CBA_ADJ, new BigDecimal("-30.95"))));
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
-        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(UUID.randomUUID(), new TaxInvoiceItem(UUID.randomUUID(), null, account.getId(), null, "Tax Item", new LocalDate(2012, 5, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Move to Evergreen PHASE to verify non-dry-run scenario
         busHandler.pushExpectedEvents(NextEvent.PHASE, NextEvent.INVOICE);
@@ -266,13 +266,13 @@ public class TestWithTaxItems extends TestIntegrationBase {
 
         // Make sure TestInvoicePluginApi will return an additional TAX item
         final UUID invoiceTaxItemId = UUID.randomUUID();
-        testInvoicePluginApi.addTaxItem(invoiceTaxItemId, BigDecimal.ONE);
+        testInvoicePluginApi.addTaxItem(invoiceTaxItemId, new TaxInvoiceItem(invoiceTaxItemId, null, account.getId(), null, "Tax Item", new LocalDate(2012, 4, 1), BigDecimal.ONE, account.getCurrency()));
 
         // Insert external charge autoCommit = false => Invoice will be in DRAFT
-        final List<InvoiceItem> items = invoiceUserApi.insertExternalCharges(account.getId(), clock.getUTCNow().toLocalDate(), ImmutableList.<InvoiceItem>of(new ExternalChargeInvoiceItem(null, account.getId(), null, "foo", new LocalDate(2012, 4, 1), null, new BigDecimal("33.80"), account.getCurrency())), false, callContext);
+        invoiceUserApi.insertExternalCharges(account.getId(), clock.getUTCNow().toLocalDate(), ImmutableList.<InvoiceItem>of(new ExternalChargeInvoiceItem(null, account.getId(), null, "foo", new LocalDate(2012, 4, 1), null, new BigDecimal("33.80"), account.getCurrency())), false, callContext);
 
         // Make sure TestInvoicePluginApi **update** the original TAX item
-        testInvoicePluginApi.addTaxItem(invoiceTaxItemId, new BigDecimal("12.45"));
+        testInvoicePluginApi.addTaxItem(invoiceTaxItemId, new TaxInvoiceItem(invoiceTaxItemId, null, account.getId(), null, "Tax Item", new LocalDate(2012, 4, 1), new BigDecimal("12.45"), account.getCurrency()));
 
         // Move to Evergreen PHASE, but invoice remains in DRAFT mode
         busHandler.pushExpectedEvents(NextEvent.PHASE /*, NextEvent.INVOICE */);
@@ -296,17 +296,18 @@ public class TestWithTaxItems extends TestIntegrationBase {
 
     public class TestInvoicePluginApi implements InvoicePluginApi {
 
-        private final Map<UUID, BigDecimal> taxItems;
+        private final Map<UUID, TaxInvoiceItem> taxItems;
 
         public TestInvoicePluginApi() {
-            taxItems = new HashMap<UUID, BigDecimal>();
+            taxItems = new HashMap<UUID, TaxInvoiceItem>();
         }
 
         @Override
         public List<InvoiceItem> getAdditionalInvoiceItems(final Invoice invoice, final boolean isDryRun, final Iterable<PluginProperty> pluginProperties, final CallContext callContext) {
             final List<InvoiceItem> result = new ArrayList<InvoiceItem>();
             for (UUID itemId : taxItems.keySet()) {
-                result.add(new TaxInvoiceItem(itemId, invoice.getId(), invoice.getAccountId(), null, "Tax Item", clock.getUTCNow().toLocalDate(), taxItems.get(itemId), invoice.getCurrency()));
+                final TaxInvoiceItem item = taxItems.get(itemId);
+                result.add(new TaxInvoiceItem(item.getId(), invoice.getId(), invoice.getAccountId(), item.getBundleId(), "Tax Item", item.getStartDate(), item.getAmount(), invoice.getCurrency()));
             }
             taxItems.clear();
             return result;
@@ -316,8 +317,10 @@ public class TestWithTaxItems extends TestIntegrationBase {
             taxItems.clear();
         }
 
-        public void addTaxItem(final UUID invoiceItemId, final BigDecimal amount) {
-            taxItems.put(invoiceItemId, amount);
+        public void addTaxItem(final UUID invoiceItemId, final TaxInvoiceItem item) {
+            taxItems.put(invoiceItemId, item);
         }
+
+
     }
 }
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/util/AuditChecker.java b/beatrix/src/test/java/org/killbill/billing/beatrix/util/AuditChecker.java
index 987e05e..db44b58 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/util/AuditChecker.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/util/AuditChecker.java
@@ -157,7 +157,7 @@ public class AuditChecker {
 
         for (InvoiceItem cur : invoice.getInvoiceItems()) {
             final List<AuditLog> auditLogs = getAuditLogForInvoiceItem(cur, context);
-            //Assert.assertEquals(auditLogs.size(), 1);
+            Assert.assertTrue(auditLogs.size() >= 1);
             checkAuditLog(ChangeType.INSERT, context, auditLogs.get(0), cur.getId(), InvoiceItemSqlDao.class, false, false);
         }
     }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
index 06182fe..75d4550 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
@@ -39,7 +39,6 @@ import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.entity.EntityPersistenceException;
-import org.killbill.billing.invoice.InvoiceDispatcher;
 import org.killbill.billing.invoice.InvoiceDispatcher.FutureAccountNotifications;
 import org.killbill.billing.invoice.InvoiceDispatcher.FutureAccountNotifications.SubscriptionNotification;
 import org.killbill.billing.invoice.InvoicePluginDispatcher;
@@ -348,7 +347,8 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
                             allInvoiceIds.add(invoiceItemModelDao.getInvoiceId());
                         } else if (InvoicePluginDispatcher.ALLOWED_INVOICE_ITEM_TYPES.contains(invoiceItemModelDao.getType()) &&
                                    (invoiceItemModelDao.getAmount().compareTo(existingInvoiceItem.getAmount()) != 0)) {
-                            Preconditions.checkState(existingInvoiceItem.getCurrency() == invoiceItemModelDao.getCurrency());
+                            checkAgainstExistingInvoiceItemState(existingInvoiceItem, invoiceItemModelDao);
+
                             transInvoiceItemSqlDao.updateAmount(invoiceItemModelDao.getId().toString(), invoiceItemModelDao.getAmount(), context);
                         }
                     }
@@ -1303,4 +1303,49 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
     private List<Tag> getInvoicesTags(final InternalTenantContext context) {
         return tagInternalApi.getTagsForAccountType(ObjectType.INVOICE, false, context);
     }
+
+    private static void checkAgainstExistingInvoiceItemState(final InvoiceItemModelDao existingInvoiceItem, final InvoiceItemModelDao inputInvoiceItem) {
+        Preconditions.checkState(existingInvoiceItem.getAccountId().equals(inputInvoiceItem.getAccountId()), String.format("Unexpected account ID '%s' for invoice item '%s'",
+                                                                                                                           inputInvoiceItem.getAccountId(), existingInvoiceItem.getId()));
+        if (existingInvoiceItem.getChildAccountId() != null) {
+            Preconditions.checkState(existingInvoiceItem.getChildAccountId().equals(inputInvoiceItem.getChildAccountId()), String.format("Unexpected child account ID '%s' for invoice item '%s'",
+                                                                                                                                         inputInvoiceItem.getChildAccountId(), existingInvoiceItem.getId()));
+        }
+        Preconditions.checkState(existingInvoiceItem.getInvoiceId().equals(inputInvoiceItem.getInvoiceId()), String.format("Unexpected invoice ID '%s' for invoice item '%s'",
+                                                                                                                           inputInvoiceItem.getInvoiceId(), existingInvoiceItem.getId()));
+        if (existingInvoiceItem.getBundleId() != null) {
+            Preconditions.checkState(existingInvoiceItem.getBundleId().equals(inputInvoiceItem.getBundleId()), String.format("Unexpected bundle ID '%s' for invoice item '%s'",
+                                                                                                                             inputInvoiceItem.getBundleId(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getSubscriptionId() != null) {
+            Preconditions.checkState(existingInvoiceItem.getSubscriptionId().equals(inputInvoiceItem.getSubscriptionId()), String.format("Unexpected subscription ID '%s' for invoice item '%s'",
+                                                                                                                                         inputInvoiceItem.getSubscriptionId(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getPlanName() != null) {
+            Preconditions.checkState(existingInvoiceItem.getPlanName().equals(inputInvoiceItem.getPlanName()), String.format("Unexpected plan name '%s' for invoice item '%s'",
+                                                                                                                             inputInvoiceItem.getPlanName(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getPhaseName() != null) {
+            Preconditions.checkState(existingInvoiceItem.getPhaseName().equals(inputInvoiceItem.getPhaseName()), String.format("Unexpected phase name '%s' for invoice item '%s'",
+                                                                                                                               inputInvoiceItem.getPhaseName(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getUsageName() != null) {
+            Preconditions.checkState(existingInvoiceItem.getUsageName().equals(inputInvoiceItem.getUsageName()), String.format("Unexpected usage name '%s' for invoice item '%s'",
+                                                                                                                               inputInvoiceItem.getUsageName(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getStartDate() != null) {
+            Preconditions.checkState(existingInvoiceItem.getStartDate().equals(inputInvoiceItem.getStartDate()), String.format("Unexpected startDate '%s' for invoice item '%s'",
+                                                                                                                               inputInvoiceItem.getStartDate(), existingInvoiceItem.getId()));
+        }
+        if (existingInvoiceItem.getEndDate() != null) {
+            Preconditions.checkState(existingInvoiceItem.getEndDate().equals(inputInvoiceItem.getEndDate()), String.format("Unexpected endDate '%s' for invoice item '%s'",
+                                                                                                                           inputInvoiceItem.getEndDate(), existingInvoiceItem.getId()));
+        }
+
+        Preconditions.checkState(existingInvoiceItem.getCurrency() == inputInvoiceItem.getCurrency(), String.format("Unexpected currency '%s' for invoice item '%s'",
+                                                                                                                    inputInvoiceItem.getCurrency(), existingInvoiceItem.getId()));
+        Preconditions.checkState(existingInvoiceItem.getType() == inputInvoiceItem.getType(), String.format("Unexpected item type '%s' for invoice item '%s'",
+                                                                                                            inputInvoiceItem.getType(), existingInvoiceItem.getId()));
+    }
+
 }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/DefaultInvoiceGenerator.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/DefaultInvoiceGenerator.java
index 5f710aa..2e46d79 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/generator/DefaultInvoiceGenerator.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/generator/DefaultInvoiceGenerator.java
@@ -104,7 +104,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
                     return input.getId().equals(targetInvoiceId);
                 }
             }).orNull();
-            Preconditions.checkNotNull(originalInvoice);
+            Preconditions.checkNotNull(originalInvoice, "Expecting to find an existing invoice matching the targetInvoiceId");
             invoice.addInvoiceItems(originalInvoice.getInvoiceItems());
         }
 
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/UsageInvoiceItemGenerator.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/UsageInvoiceItemGenerator.java
index be097d9..64e6c4c 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/generator/UsageInvoiceItemGenerator.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/generator/UsageInvoiceItemGenerator.java
@@ -171,7 +171,7 @@ public class UsageInvoiceItemGenerator extends InvoiceItemGenerator {
     }
 
     private Map<UUID, List<InvoiceItem>> extractPerSubscriptionExistingInArrearUsageItems(final Map<String, Usage> knownUsage, @Nullable final Iterable<Invoice> existingInvoices) {
-        if (existingInvoices == null || !existingInvoices.iterator().hasNext()) {
+        if (existingInvoices == null || Iterables.isEmpty(existingInvoices)) {
             return ImmutableMap.of();
         }
 
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
index 8b22693..46d742e 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
@@ -420,7 +420,7 @@ public class InvoiceDispatcher {
                     final InvoiceItem exitingItem = Iterables.tryFind(tmpInvoiceForInvoicePlugins.getInvoiceItems(), new Predicate<InvoiceItem>() {
                         @Override
                         public boolean apply(final InvoiceItem input) {
-                            return input.getInvoiceItemType().equals(cur.getInvoiceItemType());
+                            return input.getId().equals(cur.getId());
                         }
                     }).orNull();
                     if (exitingItem != null) {
diff --git a/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java b/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java
index a2804e0..5e89611 100644
--- a/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java
+++ b/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java
@@ -19,8 +19,6 @@ package org.killbill.billing.util.security.shiro.realm;
 
 import java.util.List;
 
-import javax.validation.constraints.AssertTrue;
-
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.authc.AuthenticationException;
 import org.apache.shiro.authc.AuthenticationToken;