Skip to content

Commit

Permalink
invoice: fix NPE in ItemsInterval.createNewItem
Browse files Browse the repository at this point in the history
Handle the case where there is nothing to pro-rate, because the original
item has already been fully repaired or adjusted.

This fixes #286.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
pierre committed Mar 3, 2015
1 parent 3186df5 commit ebbc0fe
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 6 deletions.
@@ -1,7 +1,9 @@
/*
* Copyright 2010-2014 Ning, Inc.
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
*
* Ning licenses this file to you under the Apache License, version 2.0
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
Expand Down Expand Up @@ -184,7 +186,9 @@ public boolean isSameKind(final Item other) {

final InvoiceItem otherItem = other.toInvoiceItem();

return !id.equals(otherItem.getId()) &&
// See https://github.com/killbill/killbill/issues/286
return otherItem != null &&
!id.equals(otherItem.getId()) &&
// Finally, for the tricky part... In case of complete repairs, the new invoiceItem will always meet all of the
// following conditions: same type, subscription, start date. Depending on the catalog configuration, the end
// date check could also match (e.g. repair from annual to monthly). For that scenario, we need to default
Expand Down
@@ -1,7 +1,9 @@
/*
* Copyright 2010-2014 Ning, Inc.
* Copyright 2014-2015 Groupon, Inc
* Copyright 2014-2015 The Billing Project, LLC
*
* Ning licenses this file to you under the Apache License, version 2.0
* The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
Expand All @@ -27,6 +29,7 @@
import java.util.UUID;

import org.joda.time.LocalDate;
import org.killbill.billing.invoice.api.InvoiceItem;
import org.killbill.billing.invoice.tree.Item.ItemAction;

import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -227,15 +230,20 @@ public int size() {
* @param mergeMode mode to consider.
* @return
*/
private Item createNewItem(LocalDate startDate, LocalDate endDate, final boolean mergeMode) {
private Item createNewItem(final LocalDate startDate, final LocalDate endDate, final boolean mergeMode) {

final Item item = getResultingItem(mergeMode);
if (item == null) {
return null;
}

final Item result = new Item(item.toProratedInvoiceItem(startDate, endDate), targetInvoiceId, item.getAction());
if (item.getAction() == ItemAction.CANCEL && result != null) {
final InvoiceItem proratedInvoiceItem = item.toProratedInvoiceItem(startDate, endDate);
if (proratedInvoiceItem == null) {
return null;
}

final Item result = new Item(proratedInvoiceItem, targetInvoiceId, item.getAction());
if (item.getAction() == ItemAction.CANCEL) {
item.incrementCurrentRepairedAmount(result.getAmount());
}
return result;
Expand Down
Expand Up @@ -855,6 +855,32 @@ public void verifyJson() {
}
}

@Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/286")
public void testMaxedOutProRation() {
final LocalDate startDate = new LocalDate(2014, 1, 1);
final LocalDate cancelDate = new LocalDate(2014, 1, 25);
final LocalDate endDate = new LocalDate(2014, 2, 1);

final BigDecimal monthlyRate1 = new BigDecimal("12.00");
final BigDecimal monthlyAmount1 = monthlyRate1;

final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);

final InvoiceItem existing1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount1, monthlyRate1, currency);
tree.addItem(existing1);
// Fully item adjust the recurring item
final InvoiceItem existingItemAdj1 = new ItemAdjInvoiceItem(existing1, startDate, monthlyRate1.negate(), currency);
tree.addItem(existingItemAdj1);
tree.flatten(true);

final InvoiceItem proposed1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, cancelDate, monthlyAmount1, monthlyRate1, currency);
tree.mergeProposedItem(proposed1);
tree.buildForMerge();

final List<InvoiceItem> expectedResult = Lists.newLinkedList();
verifyResult(tree.getView(), expectedResult);
}

private void verifyResult(final List<InvoiceItem> result, final List<InvoiceItem> expectedResult) {
assertEquals(result.size(), expectedResult.size());
for (int i = 0; i < expectedResult.size(); i++) {
Expand Down

1 comment on commit ebbc0fe

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.