killbill-aplcache

fixes #170 Invoice fix to make sure that overlapping full repair

5/6/2014 7:15:24 PM

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestBundleTransfer.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestBundleTransfer.java
index ec7a677..499ed1b 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestBundleTransfer.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestBundleTransfer.java
@@ -99,6 +99,9 @@ public class TestBundleTransfer extends TestIntegrationBase {
         assertTrue(theItem.getStartDate().compareTo(new LocalDate(2012, 5, 11)) == 0);
         assertTrue(theItem.getEndDate().compareTo(new LocalDate(2013, 5, 11)) == 0);
         assertTrue(theItem.getAmount().compareTo(new BigDecimal("2399.9500")) == 0);
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -157,6 +160,9 @@ public class TestBundleTransfer extends TestIntegrationBase {
         assertTrue(theItem.getStartDate().compareTo(new LocalDate(2012, 5, 3)) == 0);
         assertTrue(theItem.getEndDate().compareTo(new LocalDate(2012, 6, 3)) == 0);
         assertTrue(theItem.getAmount().compareTo(new BigDecimal("249.95")) == 0);
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -219,6 +225,9 @@ public class TestBundleTransfer extends TestIntegrationBase {
         toBeChecked = ImmutableList.<ExpectedInvoiceItemCheck>of(
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 3), new LocalDate(2012, 5, 15), InvoiceItemType.RECURRING, new BigDecimal("99.98")));
         invoiceChecker.checkInvoice(invoices.get(0).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow", description = "Test entitlement-level transfer with add-on")
@@ -308,5 +317,8 @@ public class TestBundleTransfer extends TestIntegrationBase {
             Assert.assertEquals(subscription.getBillingStartDate(), transferDay);
             Assert.assertNull(subscription.getBillingEndDate());
         }
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 }
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
index d3e7445..2c08a30 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
@@ -23,6 +23,7 @@ import java.util.UUID;
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
 import org.joda.time.LocalDate;
+import org.killbill.billing.invoice.api.InvoiceApiException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -49,6 +50,7 @@ import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
 public class TestIntegration extends TestIntegrationBase {
 
@@ -88,6 +90,7 @@ public class TestIntegration extends TestIntegrationBase {
                                     new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), new LocalDate(2012, 5, 1), InvoiceItemType.REPAIR_ADJ, new BigDecimal("-399.95")),
                                     new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 1), new LocalDate(2012, 4, 1), InvoiceItemType.CBA_ADJ, new BigDecimal("399.95")));
 
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -169,6 +172,8 @@ public class TestIntegration extends TestIntegrationBase {
         addDaysAndCheckForCompletion(31, NextEvent.CANCEL);
         invoiceChecker.checkChargedThroughDate(subscription.getId(), new LocalDate(2012, 7, 31), callContext);
 
+        checkNoMoreInvoiceToGenerate(account);
+
         log.info("TEST PASSED !");
     }
 
@@ -250,6 +255,9 @@ public class TestIntegration extends TestIntegrationBase {
         // MOVE AFTER CANCEL DATE AND EXPECT EVENT : NextEvent.CANCEL
         addDaysAndCheckForCompletion(31, NextEvent.CANCEL);
         invoiceChecker.checkChargedThroughDate(subscription.getId(), new LocalDate(2012, 8, 2), callContext);
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -338,6 +346,8 @@ public class TestIntegration extends TestIntegrationBase {
         addDaysAndCheckForCompletion(31, NextEvent.CANCEL);
         invoiceChecker.checkChargedThroughDate(subscription.getId(), new LocalDate(2012, 8, 3), callContext);
 
+        checkNoMoreInvoiceToGenerate(account);
+
         log.info("TEST PASSED !");
 
     }
@@ -439,6 +449,9 @@ public class TestIntegration extends TestIntegrationBase {
         log.info("Moving clock from" + clock.getUTCNow() + " to " + clock.getUTCNow().plusDays(3));
         clock.addDays(3);
         assertListenerStatus();
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -473,6 +486,9 @@ public class TestIntegration extends TestIntegrationBase {
 
         assertEquals(refreshedBseEntitlement.getState(), EntitlementState.CANCELLED);
         assertEquals(newBaseEntitlement.getState(), EntitlementState.ACTIVE);
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -544,6 +560,9 @@ public class TestIntegration extends TestIntegrationBase {
                                     new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 5), new LocalDate(2012, 5, 2), InvoiceItemType.RECURRING, new BigDecimal("224.96")),
                                     new ExpectedInvoiceItemCheck(new LocalDate(2012, 4, 5), new LocalDate(2012, 4, 5), InvoiceItemType.CBA_ADJ, new BigDecimal("-224.96")));
 
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -606,5 +625,8 @@ public class TestIntegration extends TestIntegrationBase {
         assertEquals(invoices.size(), 14);
 
         assertListenerStatus();
+
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 }
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 f4311da..1cc1373 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
@@ -31,6 +31,7 @@ import javax.inject.Named;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
+import org.killbill.billing.ErrorCode;
 import org.killbill.billing.ObjectType;
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountData;
@@ -95,6 +96,7 @@ import org.killbill.bus.api.PersistentBus;
 import org.skife.jdbi.v2.IDBI;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -267,6 +269,15 @@ public class TestIntegrationBase extends BeatrixTestSuiteWithEmbeddedDB {
         log.debug("DONE WITH TEST");
     }
 
+    protected void checkNoMoreInvoiceToGenerate(final Account account) {
+        try {
+            invoiceUserApi.triggerInvoiceGeneration(account.getId(), clock.getUTCToday(), false, callContext);
+            fail("Should not have generated an extra invoice");
+        } catch (InvoiceApiException e) {
+            assertEquals(e.getCode(), ErrorCode.INVOICE_NOTHING_TO_DO.getCode());
+        }
+    }
+
     protected void verifyTestResult(final UUID accountId, final UUID subscriptionId,
                                     final DateTime startDate, @Nullable final DateTime endDate,
                                     final BigDecimal amount, final DateTime chargeThroughDate,
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoiceWithRepairLogic.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoiceWithRepairLogic.java
index 8b6fb85..17c9698 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoiceWithRepairLogic.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoiceWithRepairLogic.java
@@ -152,6 +152,8 @@ public class TestIntegrationInvoiceWithRepairLogic extends TestIntegrationBase {
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 2), new LocalDate(2012, 6, 1), InvoiceItemType.RECURRING, new BigDecimal("9.63")),
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 2), new LocalDate(2012, 5, 2), InvoiceItemType.CBA_ADJ, new BigDecimal("-9.63")));
         invoiceChecker.checkInvoice(invoices.get(2).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -375,6 +377,8 @@ public class TestIntegrationInvoiceWithRepairLogic extends TestIntegrationBase {
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 7, 1), new LocalDate(2012, 8, 1), InvoiceItemType.RECURRING, new BigDecimal("29.95")),
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 7, 8), new LocalDate(2012, 7, 8), InvoiceItemType.CBA_ADJ, new BigDecimal("-29.95")));
         invoiceChecker.checkInvoice(invoices.get(6).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -483,6 +487,8 @@ public class TestIntegrationInvoiceWithRepairLogic extends TestIntegrationBase {
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 7, 1), new LocalDate(2012, 8, 1), InvoiceItemType.RECURRING, new BigDecimal("249.95")),
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 7, 1), new LocalDate(2012, 7, 1), InvoiceItemType.CBA_ADJ, new BigDecimal("-249.95")));
         invoiceChecker.checkInvoice(invoices.get(4).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -546,12 +552,7 @@ public class TestIntegrationInvoiceWithRepairLogic extends TestIntegrationBase {
         paymentApi.createRefundWithItemsAdjustments(account, payment1.getId(), iias, PLUGIN_PROPERTIES, callContext);
         assertListenerStatus();
 
-        try {
-            invoiceUserApi.triggerInvoiceGeneration(account.getId(), new LocalDate(clock.getUTCToday()), false, callContext);
-            Assert.fail("Should not gnenerated an new invoice");
-        } catch (final InvoiceApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.INVOICE_NOTHING_TO_DO.getCode());
-        }
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     //
@@ -622,11 +623,6 @@ public class TestIntegrationInvoiceWithRepairLogic extends TestIntegrationBase {
         paymentApi.createRefundWithItemsAdjustments(account, payment1.getId(), iias, PLUGIN_PROPERTIES, callContext);
         assertListenerStatus();
 
-        try {
-            invoiceUserApi.triggerInvoiceGeneration(account.getId(), new LocalDate(clock.getUTCToday()), false, callContext);
-            Assert.fail("Should not have generated an new invoice");
-        } catch (final InvoiceApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.INVOICE_NOTHING_TO_DO.getCode());
-        }
+        checkNoMoreInvoiceToGenerate(account);
     }
 }
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationWithDifferentBillingPeriods.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationWithDifferentBillingPeriods.java
index 74bfe42..5334bfb 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationWithDifferentBillingPeriods.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationWithDifferentBillingPeriods.java
@@ -95,6 +95,8 @@ public class TestIntegrationWithDifferentBillingPeriods extends TestIntegrationB
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 12), new LocalDate(2013, 5, 1), InvoiceItemType.RECURRING, new BigDecimal("2327.62")),
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 12), new LocalDate(2012, 5, 12), InvoiceItemType.CBA_ADJ, new BigDecimal("-161.26")));
         invoiceChecker.checkInvoice(invoices.get(2).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -164,6 +166,8 @@ public class TestIntegrationWithDifferentBillingPeriods extends TestIntegrationB
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 8, 1), new LocalDate(2012, 11, 1), InvoiceItemType.RECURRING, new BigDecimal("69.95")));
         invoiceChecker.checkInvoice(invoices.get(3).getId(), callContext, toBeChecked);
 
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -234,6 +238,8 @@ public class TestIntegrationWithDifferentBillingPeriods extends TestIntegrationB
         toBeChecked = ImmutableList.<ExpectedInvoiceItemCheck>of(
                 new ExpectedInvoiceItemCheck(new LocalDate(2013, 6, 1), new LocalDate(2014, 6, 1), InvoiceItemType.RECURRING, new BigDecimal("2399.95")));
         invoiceChecker.checkInvoice(invoices.get(3).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     @Test(groups = "slow")
@@ -311,5 +317,7 @@ public class TestIntegrationWithDifferentBillingPeriods extends TestIntegrationB
         toBeChecked = ImmutableList.<ExpectedInvoiceItemCheck>of(
                 new ExpectedInvoiceItemCheck(new LocalDate(2013, 6, 1), new LocalDate(2014, 6, 1), InvoiceItemType.RECURRING, new BigDecimal("2399.95")));
         invoiceChecker.checkInvoice(invoices.get(3).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 }
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestRepairIntegration.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestRepairIntegration.java
index 2802896..3a681d2 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestRepairIntegration.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestRepairIntegration.java
@@ -148,6 +148,8 @@ public class TestRepairIntegration extends TestIntegrationBase {
 
             assertListenerStatus();
         }
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 
     protected SubscriptionBaseTimeline createSubscriptionReapir(final UUID id, final List<DeletedEvent> deletedEvents, final List<NewEvent> newEvents) {
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
index a44456c..c48e416 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
@@ -114,6 +114,8 @@ public class TestSubscription extends TestIntegrationBase {
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 11), new LocalDate(2012, 5, 11), InvoiceItemType.CBA_ADJ, new BigDecimal("-2334.20")));
         invoiceChecker.checkInvoice(invoices.get(3).getId(), callContext, toBeChecked);
 
+        checkNoMoreInvoiceToGenerate(account);
+
     }
 
     @Test(groups = "slow")
@@ -167,5 +169,7 @@ public class TestSubscription extends TestIntegrationBase {
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 11), new LocalDate(2013, 5, 1), InvoiceItemType.RECURRING, new BigDecimal("5835.57")),
                 new ExpectedInvoiceItemCheck(new LocalDate(2012, 5, 11), new LocalDate(2012, 5, 11), InvoiceItemType.CBA_ADJ, new BigDecimal("-2334.20")));
         invoiceChecker.checkInvoice(invoices.get(2).getId(), callContext, toBeChecked);
+
+        checkNoMoreInvoiceToGenerate(account);
     }
 }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
index e4137cd..672045b 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
@@ -90,43 +90,56 @@ public class ItemsInterval {
      *
      * @param output
      * @param mergeMode
+     * @return whether or not the parent should ignore the interval covered by the child interval
      */
-    public void buildFromItems(final List<Item> output, final boolean mergeMode) {
-        final Item item  = getResultingItem(mergeMode);
-        if (item != null) {
-            output.add(item);
+    public boolean buildFromItems(final List<Item> output, final boolean mergeMode) {
+        final ItemWithResult itemWithResult  = getResultingItem(mergeMode);
+        if (itemWithResult.getItem() != null) {
+            output.add(itemWithResult.getItem());
         }
+        return itemWithResult.isIgnorePeriod();
     }
 
-    private Item getResultingItem(final boolean mergeMode) {
+    private ItemWithResult getResultingItem(final boolean mergeMode) {
         return mergeMode ? getResulting_CANCEL_Item() : getResulting_ADD_Item();
     }
 
-    private Item getResulting_CANCEL_Item() {
+    private ItemWithResult getResulting_CANCEL_Item() {
         Preconditions.checkState(items.size() == 0 || items.size() == 1);
-        return Iterables.tryFind(items, new Predicate<Item>() {
+        return new ItemWithResult(Iterables.tryFind(items, new Predicate<Item>() {
             @Override
             public boolean apply(final Item input) {
                 return input.getAction() == ItemAction.CANCEL;
             }
-        }).orNull();
+        }).orNull(), true);
     }
 
 
-    private Item getResulting_ADD_Item() {
+    private ItemWithResult getResulting_ADD_Item() {
 
         final Set<UUID> repairedIds = new HashSet<UUID>();
         final ListIterator<Item> it = items.listIterator(items.size());
 
+        //
+        // We can have an {0,n} pairs of ADD/CANCEL (cancelling each other), and in addition to that we could have:
+        // a - One ADD that has not been cancelled => We want to return that item and let the parent (NodeInterval) know that it should ignore
+        //   the period as this is accounted for with the item returned
+        // b - One CANCEL => We return NO item but we also want the parent to know that it should ignore
+        //   the period as this is accounted -- period should remain unbilled.
+        // c - nothing => The parent should NOT ignore the period as there is no child element that can account for it.
+        //
         while (it.hasPrevious()) {
             final Item cur = it.previous();
             switch (cur.getAction()) {
                 case ADD:
                     // If we found a CANCEL item pointing to that item then don't return it as it was repair (full repair scenario)
                     if (!repairedIds.contains(cur.getId())) {
-                        return cur;
+                        // Case a
+                        return new ItemWithResult(cur, true);
+                    } else {
+                        // Remove from the list so we know if there is anything else (case b or c)
+                        repairedIds.remove(cur.getId());
                     }
-                    break;
 
                 case CANCEL:
                     // In all cases populate the set with the id of target item being repaired
@@ -136,7 +149,9 @@ public class ItemsInterval {
                     break;
             }
         }
-        return null;
+        return repairedIds.size() > 0 ?
+               new ItemWithResult(null, true) :  /* case b */
+               new ItemWithResult(null, false); /* case c */
     }
 
 
@@ -180,7 +195,8 @@ public class ItemsInterval {
      */
     private Item createNewItem(LocalDate startDate, LocalDate endDate, final boolean mergeMode) {
 
-        final Item item  = getResultingItem(mergeMode);
+        final ItemWithResult itemWithResult  = getResultingItem(mergeMode);
+        final Item item = itemWithResult.getItem();
         if (item == null) {
             return null;
         }
@@ -191,4 +207,20 @@ public class ItemsInterval {
         }
         return result;
     }
+
+    private final class ItemWithResult {
+        private final Item item;
+        private final boolean ignorePeriod;
+
+        private ItemWithResult(final Item item, final boolean ignorePeriod) {
+            this.item = item;
+            this.ignorePeriod = ignorePeriod;
+        }
+        public Item getItem() {
+            return item;
+        }
+        public boolean isIgnorePeriod() {
+            return ignorePeriod;
+        }
+    }
 }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
index 81b17ea..a43de4b 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
@@ -83,9 +83,9 @@ public class ItemsNodeInterval extends NodeInterval {
             }
 
             @Override
-            public void onLastNode(final NodeInterval curNode) {
+            public boolean onLastNode(final NodeInterval curNode) {
                 final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
-                items.buildFromItems(output, false);
+                return items.buildFromItems(output, false);
             }
         });
     }
@@ -121,9 +121,9 @@ public class ItemsNodeInterval extends NodeInterval {
             }
 
             @Override
-            public void onLastNode(final NodeInterval curNode) {
+            public boolean onLastNode(final NodeInterval curNode) {
                 final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
-                items.buildFromItems(output, true);
+                return items.buildFromItems(output, true);
             }
         });
     }
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/NodeInterval.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/NodeInterval.java
index 05ff456..f96b256 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/NodeInterval.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/NodeInterval.java
@@ -49,14 +49,14 @@ public class NodeInterval {
      * Build the tree by calling the callback on the last node in the tree or remaining part with no children.
      *
      * @param callback the callback which perform the build logic.
+     * @return whether or not the parent NodeInterval should ignore the period covered by the child (NodeInterval)
      */
-    public void build(final BuildNodeCallback callback) {
+    public boolean build(final BuildNodeCallback callback) {
 
         Preconditions.checkNotNull(callback);
 
         if (leftChild == null) {
-            callback.onLastNode(this);
-            return;
+            return callback.onLastNode(this);
         }
 
         LocalDate curDate = start;
@@ -65,15 +65,18 @@ public class NodeInterval {
             if (curChild.getStart().compareTo(curDate) > 0) {
                 callback.onMissingInterval(this, curDate, curChild.getStart());
             }
-            curChild.build(callback);
-            curDate = curChild.getEnd();
+            boolean ignorePeriod = curChild.build(callback);
+            if (ignorePeriod) {
+                curDate = curChild.getEnd();
+            }
             curChild = curChild.getRightSibling();
         }
 
-        // Finally if there is a hole at the end, we build the missing piece from ourself
+        // Finally if there is a hole at the end, we build the missing piece from ourselves
         if (curDate.compareTo(end) < 0) {
             callback.onMissingInterval(this, curDate, end);
         }
+        return true;
     }
 
     /**
@@ -369,8 +372,10 @@ public class NodeInterval {
          * Called when we hit a node with no children
          *
          * @param curNode current node
+         * @return true if the curNode's parent should ignore that interval -- accounted for by curChild.
+         *
          */
-        public void onLastNode(NodeInterval curNode);
+        public boolean onLastNode(NodeInterval curNode);
     }
 
     /**
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
index faa4eb2..9b35daf 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
@@ -20,6 +20,7 @@ import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -27,40 +28,43 @@ import java.util.UUID;
 import javax.annotation.Nullable;
 
 import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
-import org.killbill.billing.catalog.api.BillingMode;
-import org.mockito.Mockito;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.annotations.Test;
-
 import org.killbill.billing.catalog.DefaultPrice;
 import org.killbill.billing.catalog.MockInternationalPrice;
 import org.killbill.billing.catalog.MockPlan;
 import org.killbill.billing.catalog.MockPlanPhase;
+import org.killbill.billing.catalog.api.BillingMode;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.CatalogApiException;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.catalog.api.PhaseType;
 import org.killbill.billing.catalog.api.Plan;
 import org.killbill.billing.catalog.api.PlanPhase;
-import org.killbill.clock.Clock;
-import org.killbill.clock.DefaultClock;
+import org.killbill.billing.entity.EntityPersistenceException;
 import org.killbill.billing.invoice.InvoiceTestSuiteNoDB;
 import org.killbill.billing.invoice.MockBillingEventSet;
 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.InvoicePaymentType;
+import org.killbill.billing.invoice.model.DefaultInvoice;
 import org.killbill.billing.invoice.model.DefaultInvoicePayment;
 import org.killbill.billing.invoice.model.FixedPriceInvoiceItem;
 import org.killbill.billing.invoice.model.RecurringInvoiceItem;
+import org.killbill.billing.invoice.model.RepairAdjInvoiceItem;
 import org.killbill.billing.junction.BillingEvent;
 import org.killbill.billing.junction.BillingEventSet;
 import org.killbill.billing.subscription.api.SubscriptionBase;
 import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
 import org.killbill.billing.util.config.InvoiceConfig;
 import org.killbill.billing.util.currency.KillBillMoney;
+import org.killbill.clock.Clock;
+import org.killbill.clock.DefaultClock;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
 
 import static org.killbill.billing.invoice.TestInvoiceHelper.EIGHT;
 import static org.killbill.billing.invoice.TestInvoiceHelper.FIFTEEN;
@@ -899,6 +903,75 @@ public class TestDefaultInvoiceGenerator extends InvoiceTestSuiteNoDB {
         assertEquals(invoice2.getBalance().compareTo(FIVE), 0);
     }
 
+    // Regression test for #170 (see https://github.com/killbill/killbill/pull/173)
+    @Test(groups = "fast")
+    public void testRegressionFor170() throws EntityPersistenceException, InvoiceApiException, CatalogApiException {
+        final UUID accountId = UUID.randomUUID();
+        final Currency currency = Currency.USD;
+        final SubscriptionBase subscription = createSubscription();
+        final MockInternationalPrice recurringPrice = new MockInternationalPrice(new DefaultPrice(new BigDecimal("2.9500"), Currency.USD));
+        final MockPlanPhase phase = new MockPlanPhase(recurringPrice, null);
+        final Plan plan = new MockPlan(phase);
+
+        final Invoice existingInvoice = new DefaultInvoice(UUID.randomUUID(), accountId, null, clock.getUTCToday(), new LocalDate(2013, 7, 15), currency, false);
+
+        // Set the existing recurring invoice item 2013/06/15 - 2013/07/15
+        final LocalDate startDate = new LocalDate(2013, 06, 15);
+        final LocalDate endDate = new LocalDate(2013, 07, 15);
+        final InvoiceItem recurringInvoiceItem = new RecurringInvoiceItem(existingInvoice.getId(), accountId, subscription.getBundleId(),
+                                                                          subscription.getId(), plan.getName(), phase.getName(),
+                                                                          startDate, endDate, recurringPrice.getPrice(currency),
+                                                                          recurringPrice.getPrice(currency), Currency.USD);
+        existingInvoice.addInvoiceItem(recurringInvoiceItem);
+
+        // Set an existing repair item
+        final LocalDate repairStartDate = new LocalDate(2013, 06, 21);
+        final LocalDate repairEndDate = new LocalDate(2013, 06, 26);
+        final BigDecimal repairAmount = new BigDecimal("0.4900").negate();
+        final InvoiceItem repairItem = new RepairAdjInvoiceItem(existingInvoice.getId(), accountId, repairStartDate, repairEndDate,
+                                                                repairAmount, currency, recurringInvoiceItem.getId());
+        existingInvoice.addInvoiceItem(repairItem);
+
+        // Create the billing event associated with the subscription creation
+        final BillingEventSet events = new MockBillingEventSet();
+        final BillingEvent event = invoiceUtil.createMockBillingEvent(null, subscription, new DateTime("2013-06-15", DateTimeZone.UTC),
+                                                                      plan, phase,
+                                                                      null, recurringPrice.getPrice(currency), currency,
+                                                                      BillingPeriod.MONTHLY, 15, BillingMode.IN_ADVANCE, "testEvent", 1L,
+                                                                      SubscriptionBaseTransitionType.CREATE);
+        events.add(event);
+
+        final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+        existingInvoices.add(existingInvoice);
+
+        // Generate a new invoice
+        final Invoice invoice = generator.generateInvoice(accountId, events, existingInvoices, new LocalDate(2013, 10, 30), currency, internalCallContext);
+        assertEquals(invoice.getNumberOfItems(), 7);
+        assertEquals(invoice.getInvoiceItems().get(0).getStartDate(), new LocalDate(2013, 6, 15));
+        assertEquals(invoice.getInvoiceItems().get(0).getEndDate(), new LocalDate(2013, 7, 15));
+        assertEquals(invoice.getInvoiceItems().get(1).getStartDate(), new LocalDate(2013, 6, 15));
+        assertEquals(invoice.getInvoiceItems().get(1).getEndDate(), new LocalDate(2013, 6, 21));
+        assertEquals(invoice.getInvoiceItems().get(2).getStartDate(), new LocalDate(2013, 6, 26));
+        assertEquals(invoice.getInvoiceItems().get(2).getEndDate(), new LocalDate(2013, 7, 15));
+        assertEquals(invoice.getInvoiceItems().get(3).getStartDate(), new LocalDate(2013, 7, 15));
+        assertEquals(invoice.getInvoiceItems().get(3).getEndDate(), new LocalDate(2013, 8, 15));
+        assertEquals(invoice.getInvoiceItems().get(4).getStartDate(), new LocalDate(2013, 8, 15));
+        assertEquals(invoice.getInvoiceItems().get(4).getEndDate(), new LocalDate(2013, 9, 15));
+        assertEquals(invoice.getInvoiceItems().get(5).getStartDate(), new LocalDate(2013, 9, 15));
+        assertEquals(invoice.getInvoiceItems().get(5).getEndDate(), new LocalDate(2013, 10, 15));
+        assertEquals(invoice.getInvoiceItems().get(6).getStartDate(), new LocalDate(2013, 10, 15));
+        assertEquals(invoice.getInvoiceItems().get(6).getEndDate(), new LocalDate(2013, 11, 15));
+
+        // Add newly generated invoice to existing invoices
+        existingInvoices.add(invoice);
+
+        // Generate next invoice (no-op)
+        final Invoice newInvoice = generator.generateInvoice(accountId, events, existingInvoices, new LocalDate(2013, 10, 30), currency, internalCallContext);
+        assertEquals(newInvoice.getNumberOfItems(), 1);
+        assertEquals(invoice.getInvoiceItems().get(0).getStartDate(), new LocalDate(2013, 6, 15));
+        assertEquals(invoice.getInvoiceItems().get(0).getEndDate(), new LocalDate(2013, 7, 15));
+    }
+
     private void distributeItems(final List<Invoice> invoices) {
         final Map<UUID, Invoice> invoiceMap = new HashMap<UUID, Invoice>();
 
@@ -1006,4 +1079,4 @@ public class TestDefaultInvoiceGenerator extends InvoiceTestSuiteNoDB {
         }
         log.info("--------------------  END DETAIL ----------------------");
     }
-}
+}
\ No newline at end of file
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestNodeInterval.java b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestNodeInterval.java
index f75aad3..bbfeafd 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestNodeInterval.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestNodeInterval.java
@@ -160,8 +160,9 @@ public class TestNodeInterval /* extends InvoiceTestSuiteNoDB  */ {
             }
 
             @Override
-            public void onLastNode(final NodeInterval curNode) {
+            public boolean onLastNode(final NodeInterval curNode) {
                 // Nothing
+                return true;
             }
         });
 
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
index 317fb9e..b02c83b 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
@@ -351,6 +351,51 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
+    // The test that first repair (repair1) and new Item (newItem1) end up being ignored.
+    @Test(groups = "fast")
+    public void testOverlappingRepair() {
+
+        final LocalDate startDate = new LocalDate(2012, 5, 1);
+        final LocalDate endDate = new LocalDate(2013, 5, 1);
+
+        final LocalDate changeDate = new LocalDate(2012, 5, 11);
+        final LocalDate monthlyAlignmentDate = new LocalDate(2012, 6, 1);
+
+        final BigDecimal rate1 = new BigDecimal("2400.00");
+        final BigDecimal amount1 = rate1;
+
+        final BigDecimal rate2 = new BigDecimal("300.00");
+        final BigDecimal amount2 = rate2;
+
+        // Start with a ANNUAL plan (high rate, rate1)
+        final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount1, rate1, currency);
+
+        // Change to MONTHLY plan 10 days later (low rate, rate2)
+        final InvoiceItem repair1 = new RepairAdjInvoiceItem(invoiceId, accountId, changeDate, endDate, amount1.negate(), currency, initial.getId());
+        final InvoiceItem newItem1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, "someelse", "someelse", changeDate, monthlyAlignmentDate, amount2, rate2, currency);
+
+        // On the same day, now revert back to ANNUAL
+        final InvoiceItem repair2 = new RepairAdjInvoiceItem(invoiceId, accountId, changeDate, monthlyAlignmentDate, amount2.negate(), currency, newItem1.getId());
+        final InvoiceItem newItem2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, changeDate, endDate, amount1, rate1, currency);
+
+
+        final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
+        tree.addItem(initial);
+        tree.addItem(newItem1);
+        tree.addItem(repair1);
+        tree.addItem(newItem2);
+        tree.addItem(repair2);
+
+        final List<InvoiceItem> expectedResult = Lists.newLinkedList();
+        final InvoiceItem expected1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, changeDate, new BigDecimal("65.75"), rate1, currency);
+        expectedResult.add(expected1);
+        expectedResult.add(newItem2);
+
+        tree.build();
+        verifyResult(tree.getView(), expectedResult);
+    }
+
+
     @Test(groups = "fast")
     public void testMergeWithNoExisting() {