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 9140329..e9c6342 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
@@ -21,8 +21,10 @@ package org.killbill.billing.invoice.tree;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
import java.util.UUID;
import org.joda.time.LocalDate;
@@ -42,14 +44,14 @@ import com.google.common.collect.Multimap;
public class ItemsInterval {
private final UUID targetInvoiceId;
- private final NodeInterval interval;
+ private final ItemsNodeInterval interval;
private LinkedList<Item> items;
- public ItemsInterval(final NodeInterval interval, final UUID targetInvoiceId) {
+ public ItemsInterval(final ItemsNodeInterval interval, final UUID targetInvoiceId) {
this(interval, targetInvoiceId, null);
}
- public ItemsInterval(final NodeInterval interval, final UUID targetInvoiceId, final Item initialItem) {
+ public ItemsInterval(final ItemsNodeInterval interval, final UUID targetInvoiceId, final Item initialItem) {
this.interval = interval;
this.targetInvoiceId = targetInvoiceId;
this.items = Lists.newLinkedList();
@@ -145,6 +147,10 @@ public class ItemsInterval {
private Item getResulting_CANCEL_Item() {
Preconditions.checkState(items.size() == 0 || items.size() == 1);
+ return getResulting_CANCEL_ItemNoChecks();
+ }
+
+ private Item getResulting_CANCEL_ItemNoChecks() {
return Iterables.tryFind(items, new Predicate<Item>() {
@Override
public boolean apply(final Item input) {
@@ -167,9 +173,40 @@ public class ItemsInterval {
Preconditions.checkState(items.size() <= 2, "Double billing detected: %s", items);
final Item item = items.size() > 0 && items.get(0).getAction() == ItemAction.ADD ? items.get(0) : null;
+
+ if (item != null) {
+ final Set<UUID> addItemsCancelled = new HashSet<UUID>();
+ if (items.size() > 1) {
+ addItemsCancelled.add(items.get(1).getLinkedId());
+ }
+ final Set<UUID> addItemsToBeCancelled = new HashSet<UUID>();
+ checkDoubleBilling(addItemsCancelled, addItemsToBeCancelled);
+ }
+
return item;
}
+ private void checkDoubleBilling(final Set<UUID> addItemsCancelled, final Set<UUID> addItemsToBeCancelled) {
+ final ItemsNodeInterval parentNodeInterval = (ItemsNodeInterval) interval.getParent();
+ if (parentNodeInterval == null) {
+ Preconditions.checkState(addItemsCancelled.equals(addItemsToBeCancelled), "Double billing detected: addItemsCancelled=%s, addItemsToBeCancelled=%s", addItemsCancelled, addItemsToBeCancelled);
+ return;
+ }
+ final ItemsInterval parentItemsInterval = parentNodeInterval.getItemsInterval();
+
+ final Item parentAddItem = parentItemsInterval.getResulting_ADD_Item();
+ if (parentAddItem != null) {
+ addItemsToBeCancelled.add(parentAddItem.getId());
+ }
+
+ final Item parentCancelItem = parentItemsInterval.getResulting_CANCEL_ItemNoChecks();
+ if (parentCancelItem != null) {
+ addItemsCancelled.add(parentCancelItem.getLinkedId());
+ }
+
+ parentItemsInterval.checkDoubleBilling(addItemsCancelled, addItemsToBeCancelled);
+ }
+
// Just ensure that ADD items precedes CANCEL items
public void insertSortedItem(final Item item) {
items.add(item);
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 26d30ff..16d7918 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
@@ -561,6 +561,87 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
}
@Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testOverlappingExistingItems() 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()));
+ // Correct one already generated
+ invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+ startDate.toDateTimeAtStartOfDay(),
+ invoice.getId(),
+ account.getId(),
+ subscription.getBundleId(),
+ subscription.getId(),
+ event.getPlan().getName(),
+ event.getPlanPhase().getName(),
+ startDate,
+ startDate.plusMonths(1),
+ amount,
+ amount,
+ account.getCurrency()));
+ existingInvoices.add(invoice);
+
+ try {
+ // There will be one proposed item but the tree will refuse the merge because of the bad state on disk
+ final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+ UUID.randomUUID(),
+ events,
+ existingInvoices,
+ startDate,
+ account.getCurrency(),
+ new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+ internalCallContext);
+
+ // Maybe we could auto-fix-it one day?
+ // assertEquals(generatedItems.size(), 1);
+ // assertTrue(generatedItems.get(0) instanceof RepairAdjInvoiceItem);
+ // assertEquals(generatedItems.get(0).getAmount().compareTo(amount.negate()), 0);
+ // assertEquals(generatedItems.get(0).getLinkedItemId(), invoice.getInvoiceItems().get(0).getId());
+
+ fail();
+ } 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 testOverlappingItemsWithRepair() throws InvoiceApiException {
final LocalDate startDate = new LocalDate("2016-01-01");
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 4c4688c..853b179 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
@@ -235,7 +235,7 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
final InvoiceItem repair1 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate1, endDate, amount1.negate(), currency, initial.getId());
final InvoiceItem newItem2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, repairDate2, endDate, amount3, rate3, currency);
- final InvoiceItem repair2 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate2, endDate, amount2.negate(), currency, initial.getId());
+ final InvoiceItem repair2 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate2, endDate, amount2.negate(), currency, newItem1.getId());
final List<InvoiceItem> expectedResult = Lists.newLinkedList();
final InvoiceItem expected1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, repairDate1, new BigDecimal("8.52"), rate1, currency);
@@ -465,6 +465,102 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
verifyResult(tree.getView(), expectedResult);
}
+ @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testOverlappingRecurring() {
+ final LocalDate startDate1 = new LocalDate(2012, 5, 1);
+ final LocalDate startDate2 = new LocalDate(2012, 5, 2);
+ final LocalDate endDate = new LocalDate(2012, 6, 1);
+
+ final BigDecimal rate = BigDecimal.TEN;
+ final BigDecimal amount = rate;
+
+ final InvoiceItem recurring1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate1, endDate, amount, rate, currency);
+ final InvoiceItem recurring2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate2, endDate, amount, rate, currency);
+
+ final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+ tree.addItem(recurring1);
+ tree.addItem(recurring2);
+
+ try {
+ tree.build();
+ fail();
+ } catch (final IllegalStateException e) {
+ }
+ }
+
+ @Test(groups = "fast")
+ public void testInvalidRepairCausingOverlappingRecurring() {
+ final LocalDate startDate = new LocalDate(2014, 1, 1);
+ final LocalDate endDate = new LocalDate(2014, 2, 1);
+
+ final LocalDate repairDate1 = new LocalDate(2014, 1, 23);
+
+ final LocalDate repairDate2 = new LocalDate(2014, 1, 26);
+
+ final BigDecimal rate1 = new BigDecimal("12.00");
+ final BigDecimal amount1 = rate1;
+
+ final BigDecimal rate2 = new BigDecimal("14.85");
+ final BigDecimal amount2 = rate2;
+
+ final BigDecimal rate3 = new BigDecimal("19.23");
+ final BigDecimal amount3 = rate3;
+
+ final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount1, rate1, currency);
+ final InvoiceItem newItem1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, repairDate1, endDate, amount2, rate2, currency);
+ final InvoiceItem repair1 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate1, endDate, amount1.negate(), currency, initial.getId());
+
+ final InvoiceItem newItem2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, repairDate2, endDate, amount3, rate3, currency);
+ // This repair should point to newItem1 instead
+ final InvoiceItem repair2 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate2, endDate, amount2.negate(), currency, initial.getId());
+
+ // Out-of-order insertion to show ordering doesn't matter
+ final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+ tree.addItem(repair1);
+ tree.addItem(repair2);
+ tree.addItem(initial);
+ tree.addItem(newItem1);
+ tree.addItem(newItem2);
+
+ try {
+ tree.build();
+ fail();
+ } catch (final IllegalStateException e) {
+ }
+ }
+
+ @Test(groups = "fast")
+ public void testInvalidRepairCausingOverlappingRecurringV2() {
+ final LocalDate startDate = new LocalDate(2014, 1, 1);
+ final LocalDate endDate = new LocalDate(2014, 2, 1);
+
+ final LocalDate repairDate1 = new LocalDate(2014, 1, 23);
+
+ final LocalDate repairDate2 = new LocalDate(2014, 1, 26);
+
+ final BigDecimal rate1 = new BigDecimal("12.00");
+ final BigDecimal amount1 = rate1;
+
+ final BigDecimal rate2 = new BigDecimal("14.85");
+ final BigDecimal amount2 = rate2;
+
+ final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount1, rate1, currency);
+ final InvoiceItem newItem1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, repairDate1, endDate, amount2, rate2, currency);
+ final InvoiceItem repair1 = new RepairAdjInvoiceItem(invoiceId, accountId, repairDate2, endDate, amount1.negate(), currency, initial.getId());
+
+ // Out-of-order insertion to show ordering doesn't matter
+ final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+ tree.addItem(repair1);
+ tree.addItem(initial);
+ tree.addItem(newItem1);
+
+ try {
+ tree.build();
+ fail();
+ } catch (final IllegalStateException e) {
+ }
+ }
+
// The test that first repair (repair1) and new Item (newItem1) end up being ignored.
@Test(groups = "fast")
public void testOverlappingRepair() {
