diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/Item.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/Item.java
index 1b4ff98..8c31b4b 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/Item.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/Item.java
@@ -143,12 +143,12 @@ public class Item {
}
public void incrementAdjustedAmount(final BigDecimal increment) {
- Preconditions.checkState(increment.compareTo(BigDecimal.ZERO) > 0);
+ Preconditions.checkState(increment.compareTo(BigDecimal.ZERO) > 0, "Invalid adjustment increment='%s', item=%s", increment, this);
adjustedAmount = adjustedAmount.add(increment);
}
public void incrementCurrentRepairedAmount(final BigDecimal increment) {
- Preconditions.checkState(increment.compareTo(BigDecimal.ZERO) > 0);
+ Preconditions.checkState(increment.compareTo(BigDecimal.ZERO) > 0, "Invalid repair increment='%s', item=%s", increment, this);
currentRepairedAmount = currentRepairedAmount.add(increment);
}
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 e8416ba..51b2b26 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
@@ -297,6 +297,7 @@ public class ItemsNodeInterval extends NodeInterval {
// When we detect such nodes, we delete both the ADD in the parent interval and the CANCEL in the children (and cleanup the interval if it does not have items)
//
private void pruneTree() {
+ final NodeInterval root = this;
walkTree(new WalkCallback() {
@Override
public void onCurrentNode(final int depth, final NodeInterval curNode, final NodeInterval parent) {
@@ -313,9 +314,9 @@ 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()) {
+ for (final Item curCancelItem : curNodeItems.get_CANCEL_items()) {
+ // Sanity: cancelled items should only be in the same node or parents
+ if (curNode.getLeftChild() != null) {
curNode.getLeftChild()
.walkTree(new WalkCallback() {
@Override
@@ -328,6 +329,19 @@ public class ItemsNodeInterval extends NodeInterval {
}
});
}
+
+ // Sanity: make sure the CANCEL item points to an ADD item
+ final NodeInterval nodeIntervalForCancelledItem = root.findNode(new SearchCallback() {
+ @Override
+ public boolean isMatch(final NodeInterval curNode) {
+ final ItemsInterval curChildItems = ((ItemsNodeInterval) curNode).getItemsInterval();
+ final Item cancelledItem = curChildItems.getCancelledItemIfExists(curCancelItem.getLinkedId());
+ return cancelledItem != null;
+ }
+ });
+ if (nodeIntervalForCancelledItem == null) {
+ throw new IllegalStateException(String.format("Missing cancelledItem for cancelItem=%s", curCancelItem));
+ }
}
if (!curNode.isPartitionedByChildren()) {
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 9e166a1..26d30ff 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
@@ -48,6 +48,7 @@ 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.invoice.model.FixedPriceInvoiceItem;
+import org.killbill.billing.invoice.model.ItemAdjInvoiceItem;
import org.killbill.billing.invoice.model.RecurringInvoiceItem;
import org.killbill.billing.invoice.model.RepairAdjInvoiceItem;
import org.killbill.billing.junction.BillingEvent;
@@ -776,6 +777,133 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
}
}
+ @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testInvalidRepair() throws InvoiceApiException {
+ final LocalDate startDate = new LocalDate("2016-01-01");
+
+ final BillingEventSet events = new MockBillingEventSet();
+
+ final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+ final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+ // Dangling repair
+ invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+ startDate.toDateTimeAtStartOfDay(),
+ invoice.getId(),
+ account.getId(),
+ startDate,
+ startDate.plusMonths(1),
+ BigDecimal.ONE.negate(),
+ account.getCurrency(),
+ UUID.randomUUID()));
+ 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("Missing cancelledItem"));
+ }
+ }
+
+ @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testInvalidAdjustment() throws InvoiceApiException {
+ final LocalDate startDate = new LocalDate("2016-01-01");
+
+ final BillingEventSet events = new MockBillingEventSet();
+
+ final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+ final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate, account.getCurrency());
+ // Dangling adjustment
+ invoice.addInvoiceItem(new ItemAdjInvoiceItem(UUID.randomUUID(),
+ startDate.toDateTimeAtStartOfDay(),
+ invoice.getId(),
+ account.getId(),
+ startDate,
+ "Dangling adjustment",
+ BigDecimal.ONE.negate(),
+ account.getCurrency(),
+ UUID.randomUUID()));
+ 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("Missing subscription id"));
+ }
+ }
+
+ @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testItemFullyRepairedAndFullyAdjusted() throws InvoiceApiException {
+ final LocalDate startDate = new LocalDate("2016-01-01");
+
+ final BillingEventSet events = new MockBillingEventSet();
+ final BigDecimal amount = BigDecimal.TEN;
+
+ // Subscription incorrectly invoiced
+ 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(),
+ "my-plan",
+ "my-plan-monthly",
+ startDate,
+ startDate.plusMonths(1),
+ amount,
+ amount,
+ account.getCurrency()));
+ // Repaired by the system
+ invoice.addInvoiceItem(new RepairAdjInvoiceItem(UUID.randomUUID(),
+ startDate.toDateTimeAtStartOfDay(),
+ invoice.getId(),
+ account.getId(),
+ startDate,
+ startDate.plusMonths(1),
+ BigDecimal.ONE.negate(),
+ account.getCurrency(),
+ invoice.getInvoiceItems().get(0).getId()));
+ invoice.addInvoiceItem(new ItemAdjInvoiceItem(invoice.getInvoiceItems().get(0),
+ startDate,
+ amount.negate(), // Note! The amount will matter
+ account.getCurrency()));
+ 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"));
+ }
+ }
+
// Simulate a bug in the generator where two fixed items for the same day and subscription end up in the resulting items
@Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
public void testTooManyFixedInvoiceItemsForGivenSubscriptionAndStartDatePostMerge() throws InvoiceApiException {
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 f1920e8..4c4688c 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
@@ -194,6 +194,24 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
}
@Test(groups = "fast")
+ public void testDanglingRepair() {
+ 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 repair = new RepairAdjInvoiceItem(invoiceId, accountId, startDate.minusDays(1), endDate, rate.negate(), currency, UUID.randomUUID());
+
+ final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+ tree.addItem(repair);
+ try {
+ tree.build();
+ fail();
+ } catch (final IllegalStateException e) {
+ }
+ }
+
+ @Test(groups = "fast")
public void testMultipleRepair() {
final LocalDate startDate = new LocalDate(2014, 1, 1);
