killbill-memoizeit

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
index 91ee466..4b040d1 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
@@ -102,6 +102,7 @@ import org.killbill.billing.payment.api.TransactionType;
 import org.killbill.billing.payment.invoice.InvoicePaymentControlPluginApi;
 import org.killbill.billing.payment.provider.MockPaymentProviderPlugin;
 import org.killbill.billing.subscription.api.SubscriptionBase;
+import org.killbill.billing.subscription.api.SubscriptionBaseInternalApi;
 import org.killbill.billing.subscription.api.SubscriptionBaseService;
 import org.killbill.billing.subscription.api.timeline.SubscriptionBaseTimelineApi;
 import org.killbill.billing.subscription.api.transfer.SubscriptionBaseTransferApi;
@@ -222,6 +223,8 @@ public class TestIntegrationBase extends BeatrixTestSuiteWithEmbeddedDB implemen
     @Inject
     protected SubscriptionApi subscriptionApi;
 
+    @Inject
+    protected SubscriptionBaseInternalApi subscriptionBaseInternalApiApi;
 
     @Named(BeatrixIntegrationModule.NON_OSGI_PLUGIN_NAME)
     @Inject
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithInvoicePlugin.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithInvoicePlugin.java
index a80c2fa..f770663 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithInvoicePlugin.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestWithInvoicePlugin.java
@@ -18,6 +18,7 @@
 package org.killbill.billing.beatrix.integration;
 
 import java.math.BigDecimal;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.Callable;
@@ -34,10 +35,13 @@ import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountData;
 import org.killbill.billing.api.TestApiListener.NextEvent;
 import org.killbill.billing.beatrix.util.InvoiceChecker.ExpectedInvoiceItemCheck;
+import org.killbill.billing.catalog.api.BillingActionPolicy;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.catalog.api.ProductCategory;
 import org.killbill.billing.entitlement.api.DefaultEntitlement;
+import org.killbill.billing.entitlement.api.Entitlement;
+import org.killbill.billing.entitlement.api.Entitlement.EntitlementActionPolicy;
 import org.killbill.billing.invoice.api.DefaultInvoiceService;
 import org.killbill.billing.invoice.api.Invoice;
 import org.killbill.billing.invoice.api.InvoiceApiException;
@@ -63,6 +67,7 @@ import org.killbill.notificationq.api.NotificationQueueService;
 import org.killbill.notificationq.api.NotificationQueueService.NoSuchNotificationQueue;
 import org.killbill.queue.retry.RetryNotificationEvent;
 import org.killbill.queue.retry.RetryableService;
+import org.mockito.Mockito;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -113,6 +118,7 @@ public class TestWithInvoicePlugin extends TestIntegrationBase {
         testInvoicePluginApi.additionalInvoiceItem = null;
         testInvoicePluginApi.shouldAddTaxItem = true;
         testInvoicePluginApi.isAborted = false;
+        testInvoicePluginApi.shouldUpdateDescription = false;
         testInvoicePluginApi.rescheduleDate = null;
         testInvoicePluginApi.wasRescheduled = false;
         testInvoicePluginApi.invocationCount = 0;
@@ -338,6 +344,53 @@ public class TestWithInvoicePlugin extends TestIntegrationBase {
     }
 
     @Test(groups = "slow")
+    public void testUpdateDescription() throws Exception {
+        testInvoicePluginApi.shouldAddTaxItem = false;
+        testInvoicePluginApi.shouldUpdateDescription = true;
+
+        // We take april as it has 30 days (easier to play with BCD)
+        // Set clock to the initial start date - we implicitly assume here that the account timezone is UTC
+        clock.setDay(new LocalDate(2012, 4, 1));
+
+        final AccountData accountData = getAccountData(1);
+        final Account account = createAccountWithNonOsgiPaymentMethod(accountData);
+        accountChecker.checkAccount(account.getId(), accountData, callContext);
+
+        // Create original subscription (Trial PHASE) -> $0 invoice but plugin added one item
+        final Entitlement bpSubscription = createBaseEntitlementAndCheckForCompletion(account.getId(), "bundleKey", "Pistol", ProductCategory.BASE, BillingPeriod.MONTHLY, NextEvent.CREATE, NextEvent.BLOCK, NextEvent.INVOICE);
+        final Invoice firstInvoice = invoiceChecker.checkInvoice(account.getId(), 1, callContext,
+                                                                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), null, InvoiceItemType.FIXED, new BigDecimal("0")));
+        subscriptionChecker.checkSubscriptionCreated(bpSubscription.getId(), internalCallContext);
+        checkInvoiceDescriptions(firstInvoice);
+
+        // Move to Evergreen PHASE
+        busHandler.pushExpectedEvents(NextEvent.PHASE, NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
+        clock.addDays(30);
+        assertListenerStatus();
+        final Invoice secondInvoice = invoiceChecker.checkInvoice(account.getId(), 2, callContext,
+                                                                  new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 1), new LocalDate(2012, 6, 1), InvoiceItemType.RECURRING, new BigDecimal("29.95")));
+        checkInvoiceDescriptions(secondInvoice);
+
+        // Cancel START_OF_TERM to make sure odd items like CBA are updated too
+        busHandler.pushExpectedEvents(NextEvent.BLOCK, NextEvent.CANCEL, NextEvent.INVOICE);
+        bpSubscription.cancelEntitlementWithPolicyOverrideBillingPolicy(EntitlementActionPolicy.IMMEDIATE,
+                                                                        BillingActionPolicy.START_OF_TERM,
+                                                                        ImmutableList.<PluginProperty>of(),
+                                                                        callContext);
+        assertListenerStatus();
+        final Invoice thirdInvoice = invoiceChecker.checkInvoice(account.getId(), 3, callContext,
+                                                                         new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 1), new LocalDate(2012, 6, 1), InvoiceItemType.REPAIR_ADJ, new BigDecimal("29.95").negate()),
+                                                                         new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 1), new LocalDate(2012, 5, 1), InvoiceItemType.CBA_ADJ, new BigDecimal("29.95")));
+        checkInvoiceDescriptions(thirdInvoice);
+    }
+
+    private void checkInvoiceDescriptions(final Invoice invoice) {
+        for (final InvoiceItem invoiceItem : invoice.getInvoiceItems()) {
+            assertEquals(invoiceItem.getDescription(), String.format("[plugin] %s", invoiceItem.getId()));
+        }
+    }
+
+    @Test(groups = "slow")
     public void testRescheduledViaNotification() throws Exception {
         testInvoicePluginApi.shouldAddTaxItem = false;
 
@@ -649,6 +702,7 @@ public class TestWithInvoicePlugin extends TestIntegrationBase {
         InvoiceItem additionalInvoiceItem;
         boolean shouldAddTaxItem = true;
         boolean isAborted = false;
+        boolean shouldUpdateDescription = false;
         DateTime rescheduleDate;
         boolean wasRescheduled = false;
         int invocationCount = 0;
@@ -679,6 +733,14 @@ public class TestWithInvoicePlugin extends TestIntegrationBase {
                 return ImmutableList.<InvoiceItem>of(additionalInvoiceItem);
             } else if (shouldAddTaxItem) {
                 return ImmutableList.<InvoiceItem>of(createTaxInvoiceItem(invoice));
+            } else if (shouldUpdateDescription) {
+                final List<InvoiceItem> updatedInvoiceItems = new LinkedList<InvoiceItem>();
+                for (final InvoiceItem invoiceItem : invoice.getInvoiceItems()) {
+                    final InvoiceItem updatedInvoiceItem = Mockito.spy(invoiceItem);
+                    Mockito.when(updatedInvoiceItem.getDescription()).thenReturn(String.format("[plugin] %s", invoiceItem.getId()));
+                    updatedInvoiceItems.add(updatedInvoiceItem);
+                }
+                return updatedInvoiceItems;
             } else {
                 return ImmutableList.<InvoiceItem>of();
             }
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/usage/TestConsumableInArrear.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/usage/TestConsumableInArrear.java
index 607a2b6..d3ffe9c 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/usage/TestConsumableInArrear.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/usage/TestConsumableInArrear.java
@@ -194,6 +194,59 @@ public class TestConsumableInArrear extends TestIntegrationBase {
         assertListenerStatus();
     }
 
+    @Test(groups = "slow")
+    public void testWithNoRecurringPlan() throws Exception {
+        // We take april as it has 30 days (easier to play with BCD)
+        // Set clock to the initial start date - we implicitly assume here that the account timezone is UTC
+        clock.setDay(new LocalDate(2012, 4, 1));
+
+        final AccountData accountData = getAccountData(1);
+        final Account account = createAccountWithNonOsgiPaymentMethod(accountData);
+        accountChecker.checkAccount(account.getId(), accountData, callContext);
+
+        // Create subscription
+        final DefaultEntitlement bpSubscription = createBaseEntitlementAndCheckForCompletion(account.getId(), "bundleKey", "Trebuchet", ProductCategory.BASE, BillingPeriod.NO_BILLING_PERIOD, NextEvent.CREATE, NextEvent.BLOCK, NextEvent.NULL_INVOICE);
+        // Check bundle after BP got created otherwise we get an error from auditApi.
+        subscriptionChecker.checkSubscriptionCreated(bpSubscription.getId(), internalCallContext);
+
+        Assert.assertNull(bpSubscription.getSubscriptionBase().getChargedThroughDate());
+
+        // Record usage for first month
+        setUsage(bpSubscription.getId(), "stones", new LocalDate(2012, 4, 5), 85L, callContext);
+        setUsage(bpSubscription.getId(), "stones", new LocalDate(2012, 4, 15), 150L, callContext);
+
+        busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
+        clock.addMonths(1);
+        assertListenerStatus();
+
+        invoiceChecker.checkInvoice(account.getId(), 1, callContext,
+                                    new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), new LocalDate(2012, 5, 1), InvoiceItemType.USAGE, new BigDecimal("1000")));
+
+        final DateTime firstExpectedCTD = account.getReferenceTime().withMonthOfYear(5).withDayOfMonth(1);
+        Assert.assertEquals(subscriptionBaseInternalApiApi.getSubscriptionFromId(bpSubscription.getId(), internalCallContext).getChargedThroughDate().compareTo(firstExpectedCTD), 0);
+
+        // No usage in second month
+        busHandler.pushExpectedEvents(NextEvent.NULL_INVOICE);
+        clock.addMonths(1);
+        assertListenerStatus();
+
+        Assert.assertEquals(subscriptionBaseInternalApiApi.getSubscriptionFromId(bpSubscription.getId(), internalCallContext).getChargedThroughDate().compareTo(firstExpectedCTD), 0);
+
+        // Record usage for third month (verify invoicing resumes)
+        setUsage(bpSubscription.getId(), "stones", new LocalDate(2012, 6, 5), 25L, callContext);
+        setUsage(bpSubscription.getId(), "stones", new LocalDate(2012, 6, 15), 50L, callContext);
+
+        busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
+        clock.addMonths(1);
+        assertListenerStatus();
+
+        invoiceChecker.checkInvoice(account.getId(), 2, callContext,
+                                    new ExpectedInvoiceItemCheck(new LocalDate(2012, 6, 1), new LocalDate(2012, 7, 1), InvoiceItemType.USAGE, new BigDecimal("100")));
+
+        final DateTime secondExpectedCTD = account.getReferenceTime().withMonthOfYear(7).withDayOfMonth(1);
+        Assert.assertEquals(subscriptionBaseInternalApiApi.getSubscriptionFromId(bpSubscription.getId(), internalCallContext).getChargedThroughDate().compareTo(secondExpectedCTD), 0);
+    }
+
     private void setUsage(final UUID subscriptionId, final String unitType, final LocalDate startDate, final Long amount, final CallContext context) throws UsageApiException {
         final List<UsageRecord> usageRecords = new ArrayList<UsageRecord>();
         usageRecords.add(new UsageRecord(startDate, amount));
diff --git a/catalog/src/test/resources/catalogTest.xml b/catalog/src/test/resources/catalogTest.xml
index e4969b1..77a6048 100644
--- a/catalog/src/test/resources/catalogTest.xml
+++ b/catalog/src/test/resources/catalogTest.xml
@@ -41,6 +41,7 @@
 
     <units>
         <unit name="bullets"/>
+        <unit name="stones"/>
     </units>
 
     <products>
@@ -81,6 +82,9 @@
                 <addonProduct>Bullets</addonProduct>
             </available>
         </product>
+        <product name="Trebuchet">
+            <category>BASE</category>
+        </product>
         <product name="Cleaning">
             <category>ADD_ON</category>
         </product>
@@ -1354,6 +1358,49 @@
             <plansAllowedInBundle>-1</plansAllowedInBundle>
             <!-- arbitrary number of these (multipack) -->
         </plan>
+        <plan name="trebuchet-usage-in-arrear" prettyName="Trebuchet Monthly Plan">
+            <product>Trebuchet</product>
+            <finalPhase type="EVERGREEN">
+                <duration>
+                    <unit>UNLIMITED</unit>
+                </duration>
+                <usages>
+                    <usage name="trebuchet-in-arrear-usage" billingMode="IN_ARREAR" usageType="CAPACITY">
+                        <billingPeriod>MONTHLY</billingPeriod>
+                        <tiers>
+                            <tier>
+                                <limits>
+                                    <limit>
+                                        <unit>stones</unit>
+                                        <max>100</max>
+                                    </limit>
+                                </limits>
+                                <recurringPrice>
+                                    <price>
+                                        <currency>USD</currency>
+                                        <value>100</value>
+                                    </price>
+                                </recurringPrice>
+                            </tier>
+                            <tier>
+                                <limits>
+                                    <limit>
+                                        <unit>stones</unit>
+                                        <max>-1</max>
+                                    </limit>
+                                </limits>
+                                <recurringPrice>
+                                    <price>
+                                        <currency>USD</currency>
+                                        <value>1000</value>
+                                    </price>
+                                </recurringPrice>
+                            </tier>
+                        </tiers>
+                    </usage>
+                </usages>
+            </finalPhase>
+        </plan>
     </plans>
     <priceLists>
         <defaultPriceList name="DEFAULT">
@@ -1368,6 +1415,7 @@
                 <plan>pistol-quarterly</plan>
                 <plan>shotgun-annual</plan>
                 <plan>assault-rifle-annual</plan>
+                <plan>trebuchet-usage-in-arrear</plan>
                 <plan>laser-scope-monthly</plan>
                 <plan>telescopic-scope-monthly</plan>
                 <plan>cleaning-monthly</plan>
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/api/InvoiceApiHelper.java b/invoice/src/main/java/org/killbill/billing/invoice/api/InvoiceApiHelper.java
index 0f6c011..7558dd5 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/api/InvoiceApiHelper.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/api/InvoiceApiHelper.java
@@ -92,7 +92,7 @@ public class InvoiceApiHelper {
 
         boolean success = false;
         GlobalLock lock = null;
-        Iterable<Invoice> invoicesForPlugins = null;
+        Iterable<DefaultInvoice> invoicesForPlugins = null;
         try {
             lock = locker.lockWithNumberOfTries(LockerType.ACCNT_INV_PAY.toString(), accountId.toString(), invoiceConfig.getMaxGlobalLockRetries());
 
@@ -100,10 +100,9 @@ public class InvoiceApiHelper {
 
             final InternalCallContext internalCallContext = internalCallContextFactory.createInternalCallContext(accountId, context);
             final List<InvoiceModelDao> invoiceModelDaos = new LinkedList<InvoiceModelDao>();
-            for (final Invoice invoiceForPlugin : invoicesForPlugins) {
-                // Call plugin
-                final List<InvoiceItem> additionalInvoiceItems = invoicePluginDispatcher.getAdditionalInvoiceItems(invoiceForPlugin, isDryRun, context, internalCallContext);
-                invoiceForPlugin.addInvoiceItems(additionalInvoiceItems);
+            for (final DefaultInvoice invoiceForPlugin : invoicesForPlugins) {
+                // Call plugin(s)
+                invoicePluginDispatcher.updateOriginalInvoiceWithPluginInvoiceItems(invoiceForPlugin, isDryRun, context, internalCallContext);
 
                 // Transformation to InvoiceModelDao
                 final InvoiceModelDao invoiceModelDao = new InvoiceModelDao(invoiceForPlugin);
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java b/invoice/src/main/java/org/killbill/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
index ca4e3c1..fddc528 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/api/svcs/DefaultInvoiceInternalApi.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -36,7 +36,6 @@ import org.killbill.billing.invoice.api.Invoice;
 import org.killbill.billing.invoice.api.InvoiceApiException;
 import org.killbill.billing.invoice.api.InvoiceApiHelper;
 import org.killbill.billing.invoice.api.InvoiceInternalApi;
-import org.killbill.billing.invoice.api.InvoiceItem;
 import org.killbill.billing.invoice.api.InvoicePayment;
 import org.killbill.billing.invoice.api.InvoicePaymentType;
 import org.killbill.billing.invoice.api.InvoiceStatus;
@@ -76,6 +75,10 @@ public class DefaultInvoiceInternalApi implements InvoiceInternalApi {
 
     @Override
     public Invoice getInvoiceById(final UUID invoiceId, final InternalTenantContext context) throws InvoiceApiException {
+        return getInvoiceByIdInternal(invoiceId, context);
+    }
+
+    private DefaultInvoice getInvoiceByIdInternal(final UUID invoiceId, final InternalTenantContext context) {
         return new DefaultInvoice(dao.getById(invoiceId, context));
     }
 
@@ -132,12 +135,12 @@ public class DefaultInvoiceInternalApi implements InvoiceInternalApi {
 
         // See https://github.com/killbill/killbill/issues/265
         final CallContext callContext = internalCallContextFactory.createCallContext(context);
-        final Invoice invoice = getInvoiceById(refund.getInvoiceId(), context);
+        final DefaultInvoice invoice = getInvoiceByIdInternal(refund.getInvoiceId(), context);
         final UUID accountId = invoice.getAccountId();
         final WithAccountLock withAccountLock = new WithAccountLock() {
             @Override
-            public Iterable<Invoice> prepareInvoices() throws InvoiceApiException {
-                return ImmutableList.<Invoice>of(invoice);
+            public Iterable<DefaultInvoice> prepareInvoices() throws InvoiceApiException {
+                return ImmutableList.<DefaultInvoice>of(invoice);
             }
         };
         invoiceApiHelper.dispatchToInvoicePluginsAndInsertItems(accountId, false, withAccountLock, callContext);
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/api/user/DefaultInvoiceUserApi.java b/invoice/src/main/java/org/killbill/billing/invoice/api/user/DefaultInvoiceUserApi.java
index 29668fb..fc0dbb9 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/api/user/DefaultInvoiceUserApi.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/api/user/DefaultInvoiceUserApi.java
@@ -204,6 +204,10 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
 
     @Override
     public Invoice getInvoice(final UUID invoiceId, final TenantContext context) throws InvoiceApiException {
+        return getInvoiceInternal(invoiceId, context);
+    }
+
+    private DefaultInvoice getInvoiceInternal(final UUID invoiceId, final TenantContext context) {
         final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(invoiceId, ObjectType.INVOICE, context);
         return new DefaultInvoice(dao.getById(invoiceId, internalTenantContext), getCatalogSafelyForPrettyNames(internalTenantContext));
     }
@@ -285,13 +289,13 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
         final WithAccountLock withAccountLock = new WithAccountLock() {
 
             @Override
-            public Iterable<Invoice> prepareInvoices() throws InvoiceApiException {
+            public Iterable<DefaultInvoice> prepareInvoices() throws InvoiceApiException {
                 final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(accountId, context);
                 final LocalDate invoiceDate = internalTenantContext.toLocalDate(context.getCreatedDate());
 
                 // Group all new external charges on the same invoice (per currency)
-                final Map<Currency, Invoice> newInvoicesForExternalCharges = new HashMap<Currency, Invoice>();
-                final Map<UUID, Invoice> existingInvoicesForExternalCharges = new HashMap<UUID, Invoice>();
+                final Map<Currency, DefaultInvoice> newInvoicesForExternalCharges = new HashMap<Currency, DefaultInvoice>();
+                final Map<UUID, DefaultInvoice> existingInvoicesForExternalCharges = new HashMap<UUID, DefaultInvoice>();
 
                 for (final InvoiceItem charge : charges) {
                     final Invoice invoiceForExternalCharge;
@@ -301,13 +305,13 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
                         final Currency currency = charge.getCurrency();
                         if (newInvoicesForExternalCharges.get(currency) == null) {
                             final InvoiceStatus status = autoCommit ? InvoiceStatus.COMMITTED : InvoiceStatus.DRAFT;
-                            final Invoice newInvoiceForExternalCharge = new DefaultInvoice(accountId, invoiceDate, effectiveDate, currency, status);
+                            final DefaultInvoice newInvoiceForExternalCharge = new DefaultInvoice(accountId, invoiceDate, effectiveDate, currency, status);
                             newInvoicesForExternalCharges.put(currency, newInvoiceForExternalCharge);
                         }
                         invoiceForExternalCharge = newInvoicesForExternalCharges.get(currency);
                     } else {
                         if (existingInvoicesForExternalCharges.get(invoiceIdForExternalCharge) == null) {
-                            final Invoice existingInvoiceForExternalCharge = getInvoice(invoiceIdForExternalCharge, context);
+                            final DefaultInvoice existingInvoiceForExternalCharge = getInvoiceInternal(invoiceIdForExternalCharge, context);
                             if (InvoiceStatus.COMMITTED.equals(existingInvoiceForExternalCharge.getStatus())) {
                                 throw new InvoiceApiException(ErrorCode.INVOICE_ALREADY_COMMITTED, existingInvoiceForExternalCharge.getId());
                             }
@@ -341,7 +345,7 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
                     invoiceForExternalCharge.addInvoiceItem(externalCharge);
                 }
 
-                return Iterables.<Invoice>concat(newInvoicesForExternalCharges.values(), existingInvoicesForExternalCharges.values());
+                return Iterables.<DefaultInvoice>concat(newInvoicesForExternalCharges.values(), existingInvoicesForExternalCharges.values());
             }
         };
 
@@ -382,12 +386,12 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
             private InvoiceItem creditItem;
 
             @Override
-            public List<Invoice> prepareInvoices() throws InvoiceApiException {
+            public List<DefaultInvoice> prepareInvoices() throws InvoiceApiException {
                 final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(accountId, context);
                 final LocalDate invoiceDate = internalTenantContext.toLocalDate(context.getCreatedDate());
 
                 // Create an invoice for that credit if it doesn't exist
-                final Invoice invoiceForCredit;
+                final DefaultInvoice invoiceForCredit;
                 if (invoiceId == null) {
                     final InvoiceStatus status = autoCommit ? InvoiceStatus.COMMITTED : InvoiceStatus.DRAFT;
                     invoiceForCredit = new DefaultInvoice(accountId, invoiceDate, effectiveDate, currency, status);
@@ -411,7 +415,7 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
                                                       itemDetails);
                 invoiceForCredit.addInvoiceItem(creditItem);
 
-                return ImmutableList.<Invoice>of(invoiceForCredit);
+                return ImmutableList.<DefaultInvoice>of(invoiceForCredit);
             }
         };
 
@@ -443,8 +447,8 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
 
         final WithAccountLock withAccountLock = new WithAccountLock() {
             @Override
-            public Iterable<Invoice> prepareInvoices() throws InvoiceApiException {
-                final Invoice invoice = getInvoiceAndCheckCurrency(invoiceId, currency, context);
+            public Iterable<DefaultInvoice> prepareInvoices() throws InvoiceApiException {
+                final DefaultInvoice invoice = getInvoiceAndCheckCurrency(invoiceId, currency, context);
                 final InvoiceItem adjustmentItem = invoiceApiHelper.createAdjustmentItem(invoice,
                                                                                          invoiceItemId,
                                                                                          amount,
@@ -457,7 +461,7 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
                     invoice.addInvoiceItem(adjustmentItem);
                 }
 
-                return ImmutableList.<Invoice>of(invoice);
+                return ImmutableList.<DefaultInvoice>of(invoice);
             }
         };
 
@@ -560,8 +564,8 @@ public class DefaultInvoiceUserApi implements InvoiceUserApi {
                                                                     }));
     }
 
-    private Invoice getInvoiceAndCheckCurrency(final UUID invoiceId, @Nullable final Currency currency, final TenantContext context) throws InvoiceApiException {
-        final Invoice invoice = getInvoice(invoiceId, context);
+    private DefaultInvoice getInvoiceAndCheckCurrency(final UUID invoiceId, @Nullable final Currency currency, final TenantContext context) throws InvoiceApiException {
+        final DefaultInvoice invoice = getInvoiceInternal(invoiceId, context);
         // Check the specified currency matches the one of the existing invoice
         if (currency != null && invoice.getCurrency() != currency) {
             throw new InvoiceApiException(ErrorCode.CURRENCY_INVALID, currency, invoice.getCurrency());
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/api/WithAccountLock.java b/invoice/src/main/java/org/killbill/billing/invoice/api/WithAccountLock.java
index 165c100..2e13120 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/api/WithAccountLock.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/api/WithAccountLock.java
@@ -1,6 +1,6 @@
 /*
- * Copyright 2015 Groupon, Inc
- * Copyright 2015 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -17,7 +17,9 @@
 
 package org.killbill.billing.invoice.api;
 
+import org.killbill.billing.invoice.model.DefaultInvoice;
+
 public interface WithAccountLock {
 
-    public Iterable<Invoice> prepareInvoices() throws InvoiceApiException;
+    public Iterable<DefaultInvoice> prepareInvoices() throws InvoiceApiException;
 }
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 dcd487e..00af476 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
@@ -75,12 +75,9 @@ import org.killbill.billing.invoice.generator.InvoiceWithMetadata;
 import org.killbill.billing.invoice.generator.InvoiceWithMetadata.SubscriptionFutureNotificationDates;
 import org.killbill.billing.invoice.generator.InvoiceWithMetadata.SubscriptionFutureNotificationDates.UsageDef;
 import org.killbill.billing.invoice.model.DefaultInvoice;
-import org.killbill.billing.invoice.model.FixedPriceInvoiceItem;
-import org.killbill.billing.invoice.model.InvoiceItemCatalogBase;
 import org.killbill.billing.invoice.model.InvoiceItemFactory;
 import org.killbill.billing.invoice.model.ItemAdjInvoiceItem;
 import org.killbill.billing.invoice.model.ParentInvoiceItem;
-import org.killbill.billing.invoice.model.RecurringInvoiceItem;
 import org.killbill.billing.invoice.notification.DefaultNextBillingDateNotifier;
 import org.killbill.billing.invoice.notification.NextBillingDateNotificationKey;
 import org.killbill.billing.junction.BillingEvent;
@@ -560,58 +557,20 @@ public class InvoiceDispatcher {
 
         boolean success = false;
         try {
-            // Generate missing credit (> 0 for generation and < 0 for use) prior we call the plugin
+            // Generate missing credit (> 0 for generation and < 0 for use) prior we call the plugin(s)
             final InvoiceItem cbaItemPreInvoicePlugins = computeCBAOnExistingInvoice(invoice, internalCallContext);
-            DefaultInvoice tmpInvoiceForInvoicePlugins = invoice;
             if (cbaItemPreInvoicePlugins != null) {
-                tmpInvoiceForInvoicePlugins = (DefaultInvoice) tmpInvoiceForInvoicePlugins.clone();
-                tmpInvoiceForInvoicePlugins.addInvoiceItem(cbaItemPreInvoicePlugins);
+                invoice.addInvoiceItem(cbaItemPreInvoicePlugins);
             }
+
             //
             // Ask external invoice plugins if additional items (tax, etc) shall be added to the invoice
             //
-            final List<InvoiceItem> additionalInvoiceItemsFromPlugins = invoicePluginDispatcher.getAdditionalInvoiceItems(tmpInvoiceForInvoicePlugins, isDryRun, callContext, internalCallContext);
-            if (additionalInvoiceItemsFromPlugins.isEmpty()) {
-                // PERF: avoid re-computing the CBA if no change was made
+            final boolean invoiceUpdated = invoicePluginDispatcher.updateOriginalInvoiceWithPluginInvoiceItems(invoice, isDryRun, callContext, internalCallContext);
+            if (invoiceUpdated) {
+                // Remove the temporary CBA item as we need to re-compute CBA
                 if (cbaItemPreInvoicePlugins != null) {
-                    invoice.addInvoiceItem(cbaItemPreInvoicePlugins);
-                }
-            } else {
-                // Add or update items from generated invoice
-                for (final InvoiceItem cur : additionalInvoiceItemsFromPlugins) {
-                    final InvoiceItem exitingItem = Iterables.tryFind(tmpInvoiceForInvoicePlugins.getInvoiceItems(), new Predicate<InvoiceItem>() {
-                        @Override
-                        public boolean apply(final InvoiceItem input) {
-                            return input.getId().equals(cur.getId());
-                        }
-                    }).orNull();
-                    if (exitingItem != null) {
-                        invoice.removeInvoiceItem(exitingItem);
-                    }
-
-                    final InvoiceItem sanitizedInvoiceItemFromPlugin = new InvoiceItemCatalogBase(cur.getId(),
-                                                                                                  cur.getCreatedDate(),
-                                                                                                  MoreObjects.firstNonNull(cur.getInvoiceId(), invoice.getId()),
-                                                                                                  cur.getAccountId(),
-                                                                                                  cur.getBundleId(),
-                                                                                                  cur.getSubscriptionId(),
-                                                                                                  cur.getDescription(),
-                                                                                                  cur.getPlanName(),
-                                                                                                  cur.getPhaseName(),
-                                                                                                  cur.getUsageName(),
-                                                                                                  cur.getPrettyPlanName(),
-                                                                                                  cur.getPrettyPhaseName(),
-                                                                                                  cur.getPrettyUsageName(),
-                                                                                                  cur.getStartDate(),
-                                                                                                  cur.getEndDate(),
-                                                                                                  cur.getAmount(),
-                                                                                                  cur.getRate(),
-                                                                                                  cur.getCurrency(),
-                                                                                                  cur.getLinkedItemId(),
-                                                                                                  cur.getQuantity(),
-                                                                                                  cur.getItemDetails(),
-                                                                                                  cur.getInvoiceItemType());
-                    invoice.addInvoiceItem(sanitizedInvoiceItemFromPlugin);
+                    invoice.removeInvoiceItem(cbaItemPreInvoicePlugins);
                 }
 
                 // Use credit after we call the plugin (https://github.com/killbill/killbill/issues/637)
@@ -639,7 +598,7 @@ public class InvoiceDispatcher {
                 success = true;
 
                 try {
-                    setChargedThroughDates(invoice.getInvoiceItems(FixedPriceInvoiceItem.class), invoice.getInvoiceItems(RecurringInvoiceItem.class), internalCallContext);
+                    setChargedThroughDates(invoice, internalCallContext);
                 } catch (final SubscriptionBaseApiException e) {
                     log.error("Failed handling SubscriptionBase change.", e);
                     return null;
@@ -854,12 +813,23 @@ public class InvoiceDispatcher {
         return internalCallContextFactory.createCallContext(context);
     }
 
-    private void setChargedThroughDates(final Collection<InvoiceItem> fixedPriceItems,
-                                        final Collection<InvoiceItem> recurringItems,
-                                        final InternalCallContext context) throws SubscriptionBaseApiException {
+    private void setChargedThroughDates(final Invoice invoice, final InternalCallContext context) throws SubscriptionBaseApiException {
+        // Don't use invoice.getInvoiceItems(final Class<T> clazz) as some items can come from plugins
+        final Collection<InvoiceItem> invoiceItemsToConsider = new LinkedList<InvoiceItem>();
+        for (final InvoiceItem invoiceItem : invoice.getInvoiceItems()) {
+            switch (invoiceItem.getInvoiceItemType()) {
+                case FIXED:
+                case RECURRING:
+                case USAGE:
+                    invoiceItemsToConsider.add(invoiceItem);
+                    break;
+                default:
+                    break;
+            }
+        }
+
         final Map<UUID, DateTime> chargeThroughDates = new HashMap<UUID, DateTime>();
-        addInvoiceItemsToChargeThroughDates(chargeThroughDates, fixedPriceItems, context);
-        addInvoiceItemsToChargeThroughDates(chargeThroughDates, recurringItems, context);
+        addInvoiceItemsToChargeThroughDates(chargeThroughDates, invoiceItemsToConsider, context);
 
         for (final UUID subscriptionId : chargeThroughDates.keySet()) {
             if (subscriptionId != null) {
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java b/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java
index 6bbdaf1..59f8db4 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java
@@ -24,6 +24,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.UUID;
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -38,6 +39,7 @@ import org.killbill.billing.invoice.api.InvoiceApiException;
 import org.killbill.billing.invoice.api.InvoiceItem;
 import org.killbill.billing.invoice.api.InvoiceItemType;
 import org.killbill.billing.invoice.model.DefaultInvoice;
+import org.killbill.billing.invoice.model.InvoiceItemCatalogBase;
 import org.killbill.billing.invoice.plugin.api.InvoiceContext;
 import org.killbill.billing.invoice.plugin.api.InvoicePluginApi;
 import org.killbill.billing.invoice.plugin.api.PriorInvoiceResult;
@@ -49,7 +51,10 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 
 public class InvoicePluginDispatcher {
 
@@ -150,39 +155,116 @@ public class InvoicePluginDispatcher {
         }
     }
 
-    //
-    // If we have multiple plugins there is a question of plugin ordering and also a 'product' questions to decide whether
-    // subsequent plugins should have access to items added by previous plugins
-    //
-    public List<InvoiceItem> getAdditionalInvoiceItems(final Invoice originalInvoice, final boolean isDryRun, final CallContext callContext, final InternalTenantContext tenantContext) throws InvoiceApiException {
+    public boolean updateOriginalInvoiceWithPluginInvoiceItems(final DefaultInvoice originalInvoice, final boolean isDryRun, final CallContext callContext, final InternalTenantContext tenantContext) throws InvoiceApiException {
         log.debug("Invoking invoice plugins getAdditionalInvoiceItems: isDryRun='{}', originalInvoice='{}'", isDryRun, originalInvoice);
 
-        final List<InvoiceItem> additionalInvoiceItems = new LinkedList<InvoiceItem>();
-
         final Collection<InvoicePluginApi> invoicePlugins = getInvoicePlugins(tenantContext).values();
         if (invoicePlugins.isEmpty()) {
-            return additionalInvoiceItems;
+            return false;
         }
 
-        // We clone the original invoice so plugins don't remove/add items
-        final Invoice clonedInvoice = (Invoice) ((DefaultInvoice) originalInvoice).clone();
+        boolean invoiceUpdated = false;
         for (final InvoicePluginApi invoicePlugin : invoicePlugins) {
-            final List<InvoiceItem> items = invoicePlugin.getAdditionalInvoiceItems(clonedInvoice, isDryRun, ImmutableList.<PluginProperty>of(), callContext);
-            if (items != null) {
-                for (final InvoiceItem item : items) {
-                    validateInvoiceItemFromPlugin(item, invoicePlugin);
-                    additionalInvoiceItems.add(item);
+            // We clone the original invoice so plugins don't remove/add items
+            final Invoice clonedInvoice = (Invoice) originalInvoice.clone();
+            final List<InvoiceItem> additionalInvoiceItemsForPlugin = invoicePlugin.getAdditionalInvoiceItems(clonedInvoice, isDryRun, ImmutableList.<PluginProperty>of(), callContext);
+
+            if (additionalInvoiceItemsForPlugin != null && !additionalInvoiceItemsForPlugin.isEmpty()) {
+                final Collection<InvoiceItem> additionalInvoiceItems = new LinkedList<InvoiceItem>();
+                for (final InvoiceItem additionalInvoiceItem : additionalInvoiceItemsForPlugin) {
+                    final InvoiceItem sanitizedInvoiceItem = validateAndSanitizeInvoiceItemFromPlugin(originalInvoice, additionalInvoiceItem, invoicePlugin);
+                    additionalInvoiceItems.add(sanitizedInvoiceItem);
                 }
+                invoiceUpdated = invoiceUpdated || updateOriginalInvoiceWithPluginInvoiceItems(originalInvoice, additionalInvoiceItems);
             }
         }
-        return additionalInvoiceItems;
+
+        return invoiceUpdated;
+    }
+
+    private boolean updateOriginalInvoiceWithPluginInvoiceItems(final DefaultInvoice originalInvoice, final Collection<InvoiceItem> additionalInvoiceItems) {
+        if (additionalInvoiceItems.isEmpty()) {
+            return false;
+        }
+
+        // Add or update items from generated invoice
+        for (final InvoiceItem additionalInvoiceItem : additionalInvoiceItems) {
+            final InvoiceItem existingItem = Iterables.tryFind(originalInvoice.getInvoiceItems(),
+                                                               new Predicate<InvoiceItem>() {
+                                                                   @Override
+                                                                   public boolean apply(final InvoiceItem originalInvoiceItem) {
+                                                                       return originalInvoiceItem.getId().equals(additionalInvoiceItem.getId());
+                                                                   }
+                                                               }).orNull();
+            if (existingItem != null) {
+                originalInvoice.removeInvoiceItem(existingItem);
+            }
+            originalInvoice.addInvoiceItem(additionalInvoiceItem);
+        }
+
+        return true;
+    }
+
+    private InvoiceItem validateAndSanitizeInvoiceItemFromPlugin(final Invoice originalInvoice, final InvoiceItem additionalInvoiceItem, final InvoicePluginApi invoicePlugin) throws InvoiceApiException {
+        final InvoiceItem existingItem = Iterables.<InvoiceItem>tryFind(originalInvoice.getInvoiceItems(),
+                                                                      new Predicate<InvoiceItem>() {
+                                                                          @Override
+                                                                          public boolean apply(final InvoiceItem originalInvoiceItem) {
+                                                                              return originalInvoiceItem.getId().equals(additionalInvoiceItem.getId());
+                                                                          }
+                                                                      }).orNull();
+
+        if (!ALLOWED_INVOICE_ITEM_TYPES.contains(additionalInvoiceItem.getInvoiceItemType()) && existingItem == null) {
+            log.warn("Ignoring invoice item of type {} from InvoicePlugin {}: {}", additionalInvoiceItem.getInvoiceItemType(), invoicePlugin, additionalInvoiceItem);
+            throw new InvoiceApiException(ErrorCode.INVOICE_ITEM_TYPE_INVALID, additionalInvoiceItem.getInvoiceItemType());
+        }
+
+        final UUID invoiceId = MoreObjects.firstNonNull(mutableField("invoiceId", existingItem != null ? existingItem.getInvoiceId() : null, additionalInvoiceItem.getInvoiceId(), invoicePlugin),
+                                                        originalInvoice.getId());
+        return new InvoiceItemCatalogBase(additionalInvoiceItem.getId(),
+                                          mutableField("createdDate", existingItem != null ? existingItem.getCreatedDate() : null, additionalInvoiceItem.getCreatedDate(), invoicePlugin),
+                                          invoiceId,
+                                          immutableField("accountId", existingItem, existingItem != null ? existingItem.getAccountId() : null, additionalInvoiceItem.getAccountId(), invoicePlugin),
+                                          immutableField("bundleId", existingItem, existingItem != null ? existingItem.getBundleId() : null, additionalInvoiceItem.getBundleId(), invoicePlugin),
+                                          immutableField("subscriptionId", existingItem, existingItem != null ? existingItem.getSubscriptionId() : null, additionalInvoiceItem.getSubscriptionId(), invoicePlugin),
+                                          mutableField("description", existingItem != null ? existingItem.getDescription() : null, additionalInvoiceItem.getDescription(), invoicePlugin),
+                                          immutableField("planName", existingItem, existingItem != null ? existingItem.getPlanName() : null, additionalInvoiceItem.getPlanName(), invoicePlugin),
+                                          immutableField("phaseName", existingItem, existingItem != null ? existingItem.getPhaseName() : null, additionalInvoiceItem.getPhaseName(), invoicePlugin),
+                                          immutableField("usageName", existingItem, existingItem != null ? existingItem.getUsageName() : null, additionalInvoiceItem.getUsageName(), invoicePlugin),
+                                          mutableField("prettyPlanName", existingItem != null ? existingItem.getPrettyPlanName() : null, additionalInvoiceItem.getPrettyPlanName(), invoicePlugin),
+                                          mutableField("prettyPhaseName", existingItem != null ? existingItem.getPrettyPhaseName() : null, additionalInvoiceItem.getPrettyPhaseName(), invoicePlugin),
+                                          mutableField("prettyUsageName", existingItem != null ? existingItem.getPrettyUsageName() : null, additionalInvoiceItem.getPrettyUsageName(), invoicePlugin),
+                                          immutableField("startDate", existingItem, existingItem != null ? existingItem.getStartDate() : null, additionalInvoiceItem.getStartDate(), invoicePlugin),
+                                          immutableField("endDate", existingItem, existingItem != null ? existingItem.getEndDate() : null, additionalInvoiceItem.getEndDate(), invoicePlugin),
+                                          mutableField("amount", existingItem != null ? existingItem.getAmount() : null, additionalInvoiceItem.getAmount(), invoicePlugin),
+                                          immutableField("rate", existingItem, existingItem != null ? existingItem.getRate() : null, additionalInvoiceItem.getRate(), invoicePlugin),
+                                          immutableField("currency", existingItem, existingItem != null ? existingItem.getCurrency() : null, additionalInvoiceItem.getCurrency(), invoicePlugin),
+                                          immutableField("linkedItemId", existingItem, existingItem != null ? existingItem.getLinkedItemId() : null, additionalInvoiceItem.getLinkedItemId(), invoicePlugin),
+                                          immutableField("quantity", existingItem, existingItem != null ? existingItem.getQuantity() : null, additionalInvoiceItem.getQuantity(), invoicePlugin),
+                                          mutableField("itemDetails", existingItem != null ? existingItem.getItemDetails() : null, additionalInvoiceItem.getItemDetails(), invoicePlugin),
+                                          immutableField("invoiceItemType", existingItem, existingItem != null ? existingItem.getInvoiceItemType() : null, additionalInvoiceItem.getInvoiceItemType(), invoicePlugin));
     }
 
-    private void validateInvoiceItemFromPlugin(final InvoiceItem invoiceItem, final InvoicePluginApi invoicePlugin) throws InvoiceApiException {
-        if (!ALLOWED_INVOICE_ITEM_TYPES.contains(invoiceItem.getInvoiceItemType())) {
-            log.warn("Ignoring invoice item of type {} from InvoicePlugin {}: {}", invoiceItem.getInvoiceItemType(), invoicePlugin, invoiceItem);
-            throw new InvoiceApiException(ErrorCode.INVOICE_ITEM_TYPE_INVALID, invoiceItem.getInvoiceItemType());
+    private <T> T mutableField(final String fieldName, @Nullable final T existingValue, @Nullable final T updatedValue, final InvoicePluginApi invoicePlugin) {
+        if (updatedValue != null) {
+            log.debug("Overriding mutable invoice item value from InvoicePlugin {} for fieldName='{}': existingValue='{}', updatedValue='{}'",
+                      invoicePlugin, fieldName, existingValue, updatedValue);
+            return updatedValue;
+        } else {
+            return existingValue;
+        }
+    }
+
+    private <T> T immutableField(final String fieldName, @Nullable final InvoiceItem existingItem, @Nullable final T existingValue, @Nullable final T updatedValue, final InvoicePluginApi invoicePlugin) {
+        if (existingItem == null) {
+            return updatedValue;
+        }
+
+        if (updatedValue != null && !updatedValue.equals(existingValue)) {
+            log.warn("Ignoring immutable invoice item value from InvoicePlugin {} for fieldName='{}': existingValue='{}', updatedValue='{}'",
+                     invoicePlugin, fieldName, existingValue, updatedValue);
         }
+        return existingValue;
     }
 
     private Map<String, InvoicePluginApi> getInvoicePlugins(final InternalTenantContext tenantContext) {
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
index cfe419e..14563ed 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestCatalog.java
@@ -106,7 +106,7 @@ public class TestCatalog extends TestJaxrsBase {
         Assert.assertEquals(catalogsJson.get(0).getName(), "Firearms");
         Assert.assertEquals(catalogsJson.get(0).getEffectiveDate(), Date.valueOf("2011-01-01"));
         Assert.assertEquals(catalogsJson.get(0).getCurrencies().size(), 3);
-        Assert.assertEquals(catalogsJson.get(0).getProducts().size(), 12);
+        Assert.assertEquals(catalogsJson.get(0).getProducts().size(), 13);
         Assert.assertEquals(catalogsJson.get(0).getPriceLists().size(), 7);
 
         for (final Product productJson : catalogsJson.get(0).getProducts()) {