Details
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 67c1da9..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,7 +75,6 @@ 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.InvoiceItemCatalogBase;
import org.killbill.billing.invoice.model.InvoiceItemFactory;
import org.killbill.billing.invoice.model.ItemAdjInvoiceItem;
import org.killbill.billing.invoice.model.ParentInvoiceItem;
@@ -558,35 +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 existingItem = Iterables.tryFind(tmpInvoiceForInvoicePlugins.getInvoiceItems(), new Predicate<InvoiceItem>() {
- @Override
- public boolean apply(final InvoiceItem input) {
- return input.getId().equals(cur.getId());
- }
- }).orNull();
- if (existingItem != null) {
- invoice.removeInvoiceItem(existingItem);
- }
- invoice.addInvoiceItem(cur);
+ invoice.removeInvoiceItem(cbaItemPreInvoicePlugins);
}
// Use credit after we call the plugin (https://github.com/killbill/killbill/issues/637)
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 f75462e..59f8db4 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoicePluginDispatcher.java
@@ -155,32 +155,54 @@ 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) {
+ // 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) {
+
+ 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 {
