killbill-aplcache

invoice: add detection for invalid repairs CANCEL items

12/19/2016 10:13:08 AM

Details

diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
index eaa69f8..6a22b6a 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
@@ -107,7 +107,13 @@ public class FixedAndRecurringInvoiceItemGenerator extends InvoiceItemGenerator 
         final List<InvoiceItem> proposedItems = new ArrayList<InvoiceItem>();
         processRecurringBillingEvents(invoiceId, account.getId(), eventSet, targetDate, targetCurrency, proposedItems, perSubscriptionFutureNotificationDate, existingInvoices, internalCallContext);
         processFixedBillingEvents(invoiceId, account.getId(), eventSet, targetDate, targetCurrency, proposedItems, internalCallContext);
-        accountItemTree.mergeWithProposedItems(proposedItems);
+
+        try {
+            accountItemTree.mergeWithProposedItems(proposedItems);
+        } catch (final IllegalStateException e) {
+            // Proposed items have already been logged
+            throw new InvoiceApiException(e, ErrorCode.UNEXPECTED_ERROR, String.format("ILLEGAL INVOICING STATE accountItemTree=%s", accountItemTree.toString()));
+        }
 
         final List<InvoiceItem> resultingItems = accountItemTree.getResultingItemList();
         safetyBounds(resultingItems, createdItemsPerDayPerSubscription, internalCallContext);
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/AccountItemTree.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/AccountItemTree.java
index def85b1..e7ab1f8 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/AccountItemTree.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/AccountItemTree.java
@@ -209,4 +209,12 @@ public class AccountItemTree {
             }
         }).orNull();
     }
+
+    @Override
+    public String toString() {
+        final StringBuilder sb = new StringBuilder("AccountItemTree{");
+        sb.append("subscriptionItemTree=").append(subscriptionItemTree);
+        sb.append('}');
+        return sb.toString();
+    }
 }
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 c217011..9140329 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
@@ -106,7 +106,7 @@ public class ItemsInterval {
 
         for (final UUID invoiceItemId : cancellingPairPerInvoiceItemId.keySet()) {
             final Collection<Item> itemsToRemove = cancellingPairPerInvoiceItemId.get(invoiceItemId);
-            Preconditions.checkArgument(itemsToRemove.size() <= 2, "Too many repairs for invoiceItemId='%s': %s", invoiceItemId, itemsToRemove);
+            Preconditions.checkState(itemsToRemove.size() <= 2, "Too many repairs for invoiceItemId='%s': %s", invoiceItemId, itemsToRemove);
             if (itemsToRemove.size() == 2) {
                 for (final Item itemToRemove : itemsToRemove) {
                     items.remove(itemToRemove);
@@ -126,6 +126,15 @@ public class ItemsInterval {
         });
     }
 
+    public Iterable<Item> get_CANCEL_items() {
+        return Iterables.filter(items, new Predicate<Item>() {
+            @Override
+            public boolean apply(final Item input) {
+                return input.getAction() == ItemAction.CANCEL;
+            }
+        });
+    }
+
     public NodeInterval getNodeInterval() {
         return interval;
     }
@@ -189,14 +198,24 @@ public class ItemsInterval {
         items.remove(item);
     }
 
-    public Item getCancelledItemIfExists(final UUID targetId) {
-        final Item item = Iterables.tryFind(items, new Predicate<Item>() {
-            @Override
-            public boolean apply(final Item input) {
-                return input.getAction() == ItemAction.CANCEL && input.getLinkedId().equals(targetId);
-            }
-        }).orNull();
-        return item;
+    public Item getCancellingItemIfExists(final UUID targetId) {
+        return Iterables.tryFind(items,
+                                 new Predicate<Item>() {
+                                     @Override
+                                     public boolean apply(final Item input) {
+                                         return input.getAction() == ItemAction.CANCEL && input.getLinkedId().equals(targetId);
+                                     }
+                                 }).orNull();
+    }
+
+    public Item getCancelledItemIfExists(final UUID linkedId) {
+        return Iterables.tryFind(items,
+                                 new Predicate<Item>() {
+                                     @Override
+                                     public boolean apply(final Item input) {
+                                         return input.getAction() == ItemAction.ADD && input.getId().equals(linkedId);
+                                     }
+                                 }).orNull();
     }
 
     public int size() {
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 4106f87..e8416ba 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
@@ -24,7 +24,6 @@ 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;
@@ -314,6 +313,23 @@ public class ItemsNodeInterval extends NodeInterval {
                     curNode.getParent().removeChild(curNode);
                 }
 
+                // Sanity: cancelled items should only be in the same node or parents
+                if (curNode.getLeftChild() != null) {
+                    for (final Item curCancelItem : curNodeItems.get_CANCEL_items()) {
+                        curNode.getLeftChild()
+                               .walkTree(new WalkCallback() {
+                                   @Override
+                                   public void onCurrentNode(final int depth, final NodeInterval curNode, final NodeInterval parent) {
+                                       final ItemsInterval curChildItems = ((ItemsNodeInterval) curNode).getItemsInterval();
+                                       final Item cancelledItem = curChildItems.getCancelledItemIfExists(curCancelItem.getLinkedId());
+                                       if (cancelledItem != null) {
+                                           throw new IllegalStateException(String.format("Invalid cancelledItem=%s for cancelItem=%s", cancelledItem, curCancelItem));
+                                       }
+                                   }
+                               });
+                    }
+                }
+
                 if (!curNode.isPartitionedByChildren()) {
                     return;
                 }
@@ -337,7 +353,7 @@ public class ItemsNodeInterval extends NodeInterval {
                     boolean foundFullRepairByParts = curChild != null;
                     while (curChild != null) {
                         final ItemsInterval curChildItems = ((ItemsNodeInterval) curChild).getItemsInterval();
-                        Item cancellingItem = curChildItems.getCancelledItemIfExists(curAddItem.getId());
+                        Item cancellingItem = curChildItems.getCancellingItemIfExists(curAddItem.getId());
                         if (cancellingItem == null) {
                             foundFullRepairByParts = false;
                             break;
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
index de29b6e..70effcf 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
@@ -274,6 +274,23 @@ public class SubscriptionItemTree {
     }
 
     @Override
+    public String toString() {
+        final StringBuilder sb = new StringBuilder("SubscriptionItemTree{");
+        sb.append("targetInvoiceId=").append(targetInvoiceId);
+        sb.append(", subscriptionId=").append(subscriptionId);
+        sb.append(", root=").append(root);
+        sb.append(", isBuilt=").append(isBuilt);
+        sb.append(", isMerged=").append(isMerged);
+        sb.append(", items=").append(items);
+        sb.append(", existingFullyAdjustedItems=").append(existingFullyAdjustedItems);
+        sb.append(", existingFixedItems=").append(existingFixedItems);
+        sb.append(", remainingFixedItems=").append(remainingFixedItems);
+        sb.append(", pendingItemAdj=").append(pendingItemAdj);
+        sb.append('}');
+        return sb.toString();
+    }
+
+    @Override
     public boolean equals(final Object o) {
         if (this == o) {
             return true;
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestFixedAndRecurringInvoiceItemGenerator.java b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestFixedAndRecurringInvoiceItemGenerator.java
index 7f19ff7..9e166a1 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestFixedAndRecurringInvoiceItemGenerator.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestFixedAndRecurringInvoiceItemGenerator.java
@@ -49,6 +49,7 @@ import org.killbill.billing.invoice.generator.InvoiceWithMetadata.SubscriptionFu
 import org.killbill.billing.invoice.model.DefaultInvoice;
 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;
@@ -490,8 +491,288 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
                                                                                                          new HashMap<UUID, SubscriptionFutureNotificationDates>(),
                                                                                                          internalCallContext);
             fail();
-        } catch (final IllegalStateException e) {
-            assertTrue(e.getMessage().startsWith("Double billing detected"));
+        } catch (final InvoiceApiException e) {
+            assertEquals(e.getCode(), ErrorCode.UNEXPECTED_ERROR.getCode());
+            assertTrue(e.getCause().getMessage().startsWith("Double billing detected"));
+        }
+    }
+
+    @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+    public void testOverlappingItems() throws InvoiceApiException {
+        final LocalDate startDate = new LocalDate("2016-01-01");
+
+        final BillingEventSet events = new MockBillingEventSet();
+        final BigDecimal amount = BigDecimal.TEN;
+        final MockInternationalPrice price = new MockInternationalPrice(new DefaultPrice(amount, account.getCurrency()));
+        final Plan plan = new MockPlan("my-plan");
+        final PlanPhase planPhase = new MockPlanPhase(price, null, BillingPeriod.MONTHLY, PhaseType.EVERGREEN);
+        final BillingEvent event = invoiceUtil.createMockBillingEvent(account,
+                                                                      subscription,
+                                                                      startDate.toDateTimeAtStartOfDay(),
+                                                                      plan,
+                                                                      planPhase,
+                                                                      null,
+                                                                      amount,
+                                                                      account.getCurrency(),
+                                                                      BillingPeriod.MONTHLY,
+                                                                      1,
+                                                                      BillingMode.IN_ADVANCE,
+                                                                      "Billing Event Desc",
+                                                                      1L,
+                                                                      SubscriptionBaseTransitionType.CREATE);
+        events.add(event);
+
+        // Simulate a previous mis-bill: existing item is for [2016-01-01,2016-01-30], proposed will be for [2016-01-01,2016-02-01]
+        final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+        final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+        invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        subscription.getBundleId(),
+                                                        subscription.getId(),
+                                                        event.getPlan().getName(),
+                                                        event.getPlanPhase().getName(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        amount,
+                                                        amount,
+                                                        account.getCurrency()));
+        existingInvoices.add(invoice);
+
+        // We will repair the wrong item and generate the correct recurring item
+        final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+                                                                                                     UUID.randomUUID(),
+                                                                                                     events,
+                                                                                                     existingInvoices,
+                                                                                                     startDate,
+                                                                                                     account.getCurrency(),
+                                                                                                     new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+                                                                                                     internalCallContext);
+        assertEquals(generatedItems.size(), 2);
+        assertTrue(generatedItems.get(0) instanceof RecurringInvoiceItem);
+        assertEquals(generatedItems.get(0).getStartDate(), new LocalDate("2016-01-01"));
+        assertEquals(generatedItems.get(0).getEndDate(), new LocalDate("2016-02-01"));
+        assertEquals(generatedItems.get(0).getAmount().compareTo(amount), 0);
+        assertTrue(generatedItems.get(1) instanceof RepairAdjInvoiceItem);
+        assertEquals(generatedItems.get(1).getAmount().compareTo(amount.negate()), 0);
+        assertEquals(generatedItems.get(1).getLinkedItemId(), invoice.getInvoiceItems().get(0).getId());
+    }
+
+    @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+    public void testOverlappingItemsWithRepair() throws InvoiceApiException {
+        final LocalDate startDate = new LocalDate("2016-01-01");
+
+        final BillingEventSet events = new MockBillingEventSet();
+        final BigDecimal amount = BigDecimal.TEN;
+        final MockInternationalPrice price = new MockInternationalPrice(new DefaultPrice(amount, account.getCurrency()));
+        final Plan plan = new MockPlan("my-plan");
+        final PlanPhase planPhase = new MockPlanPhase(price, null, BillingPeriod.MONTHLY, PhaseType.EVERGREEN);
+        final BillingEvent event = invoiceUtil.createMockBillingEvent(account,
+                                                                      subscription,
+                                                                      startDate.toDateTimeAtStartOfDay(),
+                                                                      plan,
+                                                                      planPhase,
+                                                                      null,
+                                                                      amount,
+                                                                      account.getCurrency(),
+                                                                      BillingPeriod.MONTHLY,
+                                                                      1,
+                                                                      BillingMode.IN_ADVANCE,
+                                                                      "Billing Event Desc",
+                                                                      1L,
+                                                                      SubscriptionBaseTransitionType.CREATE);
+        events.add(event);
+
+        // Simulate a previous mis-bill: existing item is for [2016-01-01,2016-01-30], proposed will be for [2016-01-01,2016-02-01]
+        final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+        final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+        invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        subscription.getBundleId(),
+                                                        subscription.getId(),
+                                                        event.getPlan().getName(),
+                                                        event.getPlanPhase().getName(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        amount,
+                                                        amount,
+                                                        account.getCurrency()));
+        // But the system has already repaired it
+        invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        BigDecimal.ONE.negate(), // Note! The amount will not matter
+                                                        account.getCurrency(),
+                                                        invoice.getInvoiceItems().get(0).getId()));
+        existingInvoices.add(invoice);
+
+        // We will generate the correct recurring item
+        final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+                                                                                                     UUID.randomUUID(),
+                                                                                                     events,
+                                                                                                     existingInvoices,
+                                                                                                     startDate,
+                                                                                                     account.getCurrency(),
+                                                                                                     new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+                                                                                                     internalCallContext);
+        assertEquals(generatedItems.size(), 1);
+        assertTrue(generatedItems.get(0) instanceof RecurringInvoiceItem);
+        assertEquals(generatedItems.get(0).getStartDate(), new LocalDate("2016-01-01"));
+        assertEquals(generatedItems.get(0).getEndDate(), new LocalDate("2016-02-01"));
+        assertEquals(generatedItems.get(0).getAmount().compareTo(amount), 0);
+    }
+
+    @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+    public void testOverlappingItemsWithTooManyRepairs() throws InvoiceApiException {
+        final LocalDate startDate = new LocalDate("2016-01-01");
+
+        final BillingEventSet events = new MockBillingEventSet();
+        final BigDecimal amount = BigDecimal.TEN;
+        final MockInternationalPrice price = new MockInternationalPrice(new DefaultPrice(amount, account.getCurrency()));
+        final Plan plan = new MockPlan("my-plan");
+        final PlanPhase planPhase = new MockPlanPhase(price, null, BillingPeriod.MONTHLY, PhaseType.EVERGREEN);
+        final BillingEvent event = invoiceUtil.createMockBillingEvent(account,
+                                                                      subscription,
+                                                                      startDate.toDateTimeAtStartOfDay(),
+                                                                      plan,
+                                                                      planPhase,
+                                                                      null,
+                                                                      amount,
+                                                                      account.getCurrency(),
+                                                                      BillingPeriod.MONTHLY,
+                                                                      1,
+                                                                      BillingMode.IN_ADVANCE,
+                                                                      "Billing Event Desc",
+                                                                      1L,
+                                                                      SubscriptionBaseTransitionType.CREATE);
+        events.add(event);
+
+        // Simulate a previous mis-bill: existing item is for [2016-01-01,2016-01-30], proposed will be for [2016-01-01,2016-02-01]
+        final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+        final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+        invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        subscription.getBundleId(),
+                                                        subscription.getId(),
+                                                        event.getPlan().getName(),
+                                                        event.getPlanPhase().getName(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        amount,
+                                                        amount,
+                                                        account.getCurrency()));
+        // But the system has already repaired it
+        invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        BigDecimal.ONE.negate(), // Note! The amount will not matter
+                                                        account.getCurrency(),
+                                                        invoice.getInvoiceItems().get(0).getId()));
+        // Twice!
+        invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        BigDecimal.ONE.negate(), // Note! The amount will not matter
+                                                        account.getCurrency(),
+                                                        invoice.getInvoiceItems().get(0).getId()));
+        existingInvoices.add(invoice);
+
+        try {
+            final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+                                                                                                         UUID.randomUUID(),
+                                                                                                         events,
+                                                                                                         existingInvoices,
+                                                                                                         startDate,
+                                                                                                         account.getCurrency(),
+                                                                                                         new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+                                                                                                         internalCallContext);
+            fail();
+        } catch (final InvoiceApiException e) {
+            assertEquals(e.getCode(), ErrorCode.UNEXPECTED_ERROR.getCode());
+            assertTrue(e.getCause().getMessage().startsWith("Too many repairs"));
+        }
+    }
+
+    @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+    public void testOverlappingItemsWithInvalidRepair() throws InvoiceApiException {
+        final LocalDate startDate = new LocalDate("2016-01-01");
+
+        final BillingEventSet events = new MockBillingEventSet();
+        final BigDecimal amount = BigDecimal.TEN;
+        final MockInternationalPrice price = new MockInternationalPrice(new DefaultPrice(amount, account.getCurrency()));
+        final Plan plan = new MockPlan("my-plan");
+        final PlanPhase planPhase = new MockPlanPhase(price, null, BillingPeriod.MONTHLY, PhaseType.EVERGREEN);
+        final BillingEvent event = invoiceUtil.createMockBillingEvent(account,
+                                                                      subscription,
+                                                                      startDate.toDateTimeAtStartOfDay(),
+                                                                      plan,
+                                                                      planPhase,
+                                                                      null,
+                                                                      amount,
+                                                                      account.getCurrency(),
+                                                                      BillingPeriod.MONTHLY,
+                                                                      1,
+                                                                      BillingMode.IN_ADVANCE,
+                                                                      "Billing Event Desc",
+                                                                      1L,
+                                                                      SubscriptionBaseTransitionType.CREATE);
+        events.add(event);
+
+        // Simulate a previous mis-bill: existing item is for [2016-01-01,2016-01-30], proposed will be for [2016-01-01,2016-02-01]
+        final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+        final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+        invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        subscription.getBundleId(),
+                                                        subscription.getId(),
+                                                        event.getPlan().getName(),
+                                                        event.getPlanPhase().getName(),
+                                                        startDate,
+                                                        startDate.plusDays(29),
+                                                        amount,
+                                                        amount,
+                                                        account.getCurrency()));
+        // Also, the system has repaired a bigger period
+        invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+                                                        startDate.toDateTimeAtStartOfDay(),
+                                                        invoice.getId(),
+                                                        account.getId(),
+                                                        startDate,
+                                                        startDate.plusDays(30),
+                                                        BigDecimal.ONE.negate(), // Amount does not matter
+                                                        account.getCurrency(),
+                                                        invoice.getInvoiceItems().get(0).getId()));
+        existingInvoices.add(invoice);
+
+        try {
+            final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+                                                                                                         UUID.randomUUID(),
+                                                                                                         events,
+                                                                                                         existingInvoices,
+                                                                                                         startDate,
+                                                                                                         account.getCurrency(),
+                                                                                                         new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+                                                                                                         internalCallContext);
+            fail();
+        } catch (final InvoiceApiException e) {
+            assertEquals(e.getCode(), ErrorCode.UNEXPECTED_ERROR.getCode());
+            assertTrue(e.getCause().getMessage().startsWith("Invalid cancelledItem"));
         }
     }
 
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 b8467f4..f1920e8 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
@@ -45,6 +45,7 @@ import com.google.common.collect.Lists;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
 public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
 
@@ -62,8 +63,6 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
     private final String phaseName = "my-phase";
     private final Currency currency = Currency.USD;
 
-
-
     @Test(groups = "fast")
     public void testWithExistingSplitRecurring() {
 
@@ -118,7 +117,6 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
         Assert.assertTrue(tree.getView().isEmpty());
     }
 
-
     @Test(groups = "fast")
     public void testSimpleRepair() {
 
@@ -166,6 +164,36 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
     }
 
     @Test(groups = "fast")
+    public void testInvalidRepair() {
+        final LocalDate startDate = new LocalDate(2014, 1, 1);
+        final LocalDate endDate = new LocalDate(2014, 2, 1);
+
+        final BigDecimal rate = new BigDecimal("12.00");
+
+        final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, rate, rate, currency);
+        final InvoiceItem tooEarlyRepair = new RepairAdjInvoiceItem(invoiceId, accountId, startDate.minusDays(1), endDate, rate.negate(), currency, initial.getId());
+        final InvoiceItem tooLateRepair = new RepairAdjInvoiceItem(invoiceId, accountId, startDate, endDate.plusDays(1), rate.negate(), currency, initial.getId());
+
+        SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+        tree.addItem(initial);
+        tree.addItem(tooEarlyRepair);
+        try {
+            tree.build();
+            fail();
+        } catch (final IllegalStateException e) {
+        }
+
+        tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+        tree.addItem(initial);
+        tree.addItem(tooLateRepair);
+        try {
+            tree.build();
+            fail();
+        } catch (final IllegalStateException e) {
+        }
+    }
+
+    @Test(groups = "fast")
     public void testMultipleRepair() {
 
         final LocalDate startDate = new LocalDate(2014, 1, 1);
@@ -498,7 +526,6 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     // Will test the case A from ItemsNodeInterval#prune logic (an item is left on the interval)
     @Test(groups = "fast")
     public void testFullRepairPruneLogic2() {
@@ -544,7 +571,6 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
 
     }
 
-
     // Will test the case B from ItemsNodeInterval#prune logic
     @Test(groups = "fast")
     public void testFullRepairByPartsPruneLogic1() {
@@ -643,8 +669,6 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
-
     @Test(groups = "fast")
     public void testMergeWithNoExisting() {