invoice: @see #361. Previous fix 862d141 (and initial fix b1790a8) wa…

…s incorrect Move the removal of the $0 recurring and usage items as late as possible but inside InvoiceWithMetadata to potentially set resulting invoice to null if nothing remains
killbill · Sep 13, 2015 · 86ae0d2 · 86ae0d2
1 parent 7d43124
commit 86ae0d2
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 15 deletions.
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
@@ -18,7 +18,9 @@
 
 package org.killbill.billing.invoice.generator;
 
+import java.math.BigDecimal;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -34,6 +36,7 @@
 import org.killbill.billing.invoice.api.Invoice;
 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.generator.InvoiceWithMetadata.SubscriptionFutureNotificationDates;
 import org.killbill.billing.invoice.model.DefaultInvoice;
 import org.killbill.billing.junction.BillingEventSet;
@@ -84,6 +87,7 @@ public InvoiceWithMetadata generateInvoice(final Account account, @Nullable fina
         final List<InvoiceItem> usageItems = usageInvoiceItemGenerator.generateItems(account, invoiceId, events, existingInvoices, adjustedTargetDate, targetCurrency, perSubscriptionFutureNotificationDates, context);
         invoice.addInvoiceItems(usageItems);
 
+
         return new InvoiceWithMetadata(invoice.getInvoiceItems().isEmpty() ? null : invoice, perSubscriptionFutureNotificationDates);
     }
 
@@ -109,4 +113,5 @@ private LocalDate adjustTargetDate(final List<Invoice> existingInvoices, final L
         }
         return maxDate;
     }
+
 }
diff --git a/...in/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java b/...in/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
@@ -177,8 +177,6 @@ private List<InvoiceItem> processRecurringEvent(final UUID invoiceId, final UUID
                     }
                 }
                 updatePerSubscriptionNextNotificationDate(thisEvent.getSubscription().getId(), itemDataWithNextBillingCycleDate.getNextBillingCycleDate(), items, billingMode, perSubscriptionFutureNotificationDate);
-                // Filtering $0 items needs to occur after we compute nextNotificationDate, because these items contain important date info for the case of IN_ADVANCE billing
-                remove0$RecurringItems(items);
             }
         }
 
@@ -192,15 +190,6 @@ private List<InvoiceItem> processRecurringEvent(final UUID invoiceId, final UUID
         return items;
     }
 
-    private void remove0$RecurringItems(final List<InvoiceItem> items) {
-        final Iterator<InvoiceItem> it = items.iterator();
-        while (it.hasNext()) {
-            final InvoiceItem item = it.next();
-            if (item.getAmount().compareTo(BigDecimal.ZERO) == 0) {
-                it.remove();
-            }
-        }
-    }
 
     private void updatePerSubscriptionNextNotificationDate(final UUID subscriptionId, final LocalDate nextBillingCycleDate, final List<InvoiceItem> newProposedItems, final BillingMode billingMode, final Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDates) {
 

diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/InvoiceItemGenerator.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/InvoiceItemGenerator.java
@@ -40,5 +40,4 @@ public abstract List<InvoiceItem> generateItems(final Account account, final UUI
                                                     final Currency targetCurrency, Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDate,
                                                     final InternalCallContext context) throws InvoiceApiException;
 
-
 }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/InvoiceWithMetadata.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/InvoiceWithMetadata.java
@@ -17,7 +17,10 @@
 
 package org.killbill.billing.invoice.generator;
 
+import java.math.BigDecimal;
 import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
@@ -34,13 +37,15 @@
 
 public class InvoiceWithMetadata {
 
-    private final Invoice invoice;
     private final Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDates;
 
-    public InvoiceWithMetadata(final Invoice invoice, final Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDates) {
-        this.invoice = invoice;
+    private Invoice invoice;
+
+    public InvoiceWithMetadata(final Invoice originalInvoice, final Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDates) {
+        this.invoice = originalInvoice;
         this.perSubscriptionFutureNotificationDates = perSubscriptionFutureNotificationDates;
         build();
+        remove$0RecurringAndUsageItems();
     }
 
     public Invoice getInvoice() {
@@ -74,6 +79,23 @@ public boolean apply(final InvoiceItem input) {
         });
     }
 
+    protected void remove$0RecurringAndUsageItems() {
+        if (invoice != null) {
+            final Iterator<InvoiceItem> it = invoice.getInvoiceItems().iterator();
+            while (it.hasNext()) {
+                final InvoiceItem item = it.next();
+                if ((item.getInvoiceItemType() == InvoiceItemType.RECURRING ||  item.getInvoiceItemType() == InvoiceItemType.USAGE) &&
+                    item.getAmount().compareTo(BigDecimal.ZERO) == 0) {
+                    it.remove();
+                }
+            }
+            if (invoice.getInvoiceItems().isEmpty()) {
+                invoice = null;
+            }
+        }
+    }
+
+
     public static class SubscriptionFutureNotificationDates {
 
         private final BillingMode recurringBillingMode;