Skip to content

Commit

Permalink
invoice: ignore $0 recurring items for the purpose of the tree
Browse files Browse the repository at this point in the history 
This fixes #783.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Aug 2, 2017
1 parent 4320ce9 commit 03f276b
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 24 deletions.
69 changes: 64 additions & 5 deletions beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoice.java
View file
@@ -1,6 +1,6 @@
/* /*
* Copyright 2014-2016 Groupon, Inc * Copyright 2014-2017 Groupon, Inc
* Copyright 2014-2016 The Billing Project, LLC * Copyright 2014-2017 The Billing Project, LLC
* *
* The Billing Project 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 * (the "License"); you may not use this file except in compliance with the
Expand Down Expand Up @@ -30,6 +30,7 @@
import org.killbill.billing.account.api.Account; import org.killbill.billing.account.api.Account;
import org.killbill.billing.api.TestApiListener.NextEvent; import org.killbill.billing.api.TestApiListener.NextEvent;
import org.killbill.billing.beatrix.util.InvoiceChecker.ExpectedInvoiceItemCheck; import org.killbill.billing.beatrix.util.InvoiceChecker.ExpectedInvoiceItemCheck;
import org.killbill.billing.catalog.DefaultPlanPhasePriceOverride;
import org.killbill.billing.catalog.api.BillingActionPolicy; import org.killbill.billing.catalog.api.BillingActionPolicy;
import org.killbill.billing.catalog.api.BillingPeriod; import org.killbill.billing.catalog.api.BillingPeriod;
import org.killbill.billing.catalog.api.PlanPhasePriceOverride; import org.killbill.billing.catalog.api.PlanPhasePriceOverride;
Expand All @@ -38,7 +39,6 @@
import org.killbill.billing.catalog.api.ProductCategory; import org.killbill.billing.catalog.api.ProductCategory;
import org.killbill.billing.entitlement.api.DefaultEntitlement; import org.killbill.billing.entitlement.api.DefaultEntitlement;
import org.killbill.billing.entitlement.api.Entitlement; import org.killbill.billing.entitlement.api.Entitlement;
import org.killbill.billing.entitlement.api.EntitlementApiException;
import org.killbill.billing.entitlement.api.SubscriptionEventType; import org.killbill.billing.entitlement.api.SubscriptionEventType;
import org.killbill.billing.invoice.api.DryRunArguments; import org.killbill.billing.invoice.api.DryRunArguments;
import org.killbill.billing.invoice.api.DryRunType; import org.killbill.billing.invoice.api.DryRunType;
Expand All @@ -50,14 +50,12 @@
import org.killbill.billing.payment.api.Payment; import org.killbill.billing.payment.api.Payment;
import org.killbill.billing.payment.api.PluginProperty; import org.killbill.billing.payment.api.PluginProperty;
import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase; import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase;
import org.testng.Assert;
import org.testng.annotations.Test; import org.testng.annotations.Test;


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;


import static com.tc.util.Assert.fail; import static com.tc.util.Assert.fail;
import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue; import static org.testng.Assert.assertTrue;


public class TestIntegrationInvoice extends TestIntegrationBase { public class TestIntegrationInvoice extends TestIntegrationBase {
Expand Down Expand Up @@ -506,4 +504,65 @@ public void testDryRunWithPendingCancelledSubscription() throws Exception {


} }


@Test(groups = "slow", description = "https://github.com/killbill/killbill/issues/783")
public void testIntegrationWithRecurringFreePlan() throws Exception {
final DateTime initialCreationDate = new DateTime(2017, 1, 1, 0, 0, 0, 0, testTimeZone);
// set clock to the initial start date
clock.setTime(initialCreationDate);

final Account account = createAccountWithNonOsgiPaymentMethod(getAccountData(1));

final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Blowdart", BillingPeriod.MONTHLY, "notrial", null);

// Price override of $0
final List<PlanPhasePriceOverride> overrides = new ArrayList<PlanPhasePriceOverride>();
overrides.add(new DefaultPlanPhasePriceOverride("blowdart-monthly-notrial-evergreen", account.getCurrency(), null, BigDecimal.ZERO));
busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK, NextEvent.INVOICE);
final Entitlement entitlement = entitlementApi.createBaseEntitlement(account.getId(), spec, "bundleExternalKey", overrides, null, null, false, ImmutableList.<PluginProperty>of(), callContext);
assertListenerStatus();

invoiceChecker.checkInvoice(account.getId(), 1, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 1, 1), new LocalDate(2017, 2, 1), InvoiceItemType.RECURRING, BigDecimal.ZERO));

// 2017-02-01
busHandler.pushExpectedEvents(NextEvent.INVOICE);
clock.addMonths(1);
assertListenerStatus();

invoiceChecker.checkInvoice(account.getId(), 2, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 2, 1), new LocalDate(2017, 3, 1), InvoiceItemType.RECURRING, BigDecimal.ZERO));

// Do the change mid-month so the repair triggers the bug in https://github.com/killbill/killbill/issues/783
entitlement.changePlanWithDate(spec, ImmutableList.<PlanPhasePriceOverride>of(), new LocalDate("2017-02-15"), ImmutableList.<PluginProperty>of(), callContext);
assertListenerStatus();

// 2017-02-15
busHandler.pushExpectedEvents(NextEvent.CHANGE, NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
clock.addDays(15);
assertListenerStatus();

// Note: no repair
invoiceChecker.checkInvoice(account.getId(), 2, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 2, 1), new LocalDate(2017, 3, 1), InvoiceItemType.RECURRING, BigDecimal.ZERO));

invoiceChecker.checkInvoice(account.getId(), 3, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 2, 1), new LocalDate(2017, 2, 15), InvoiceItemType.RECURRING, BigDecimal.ZERO),
new ExpectedInvoiceItemCheck(new LocalDate(2017, 2, 15), new LocalDate(2017, 3, 1), InvoiceItemType.RECURRING, new BigDecimal("14.98")));

// 2017-03-01
busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
clock.addDays(15);
assertListenerStatus();

invoiceChecker.checkInvoice(account.getId(), 4, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 3, 1), new LocalDate(2017, 4, 1), InvoiceItemType.RECURRING, new BigDecimal("29.95")));

// 2017-04-01
busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
clock.addMonths(1);
assertListenerStatus();

invoiceChecker.checkInvoice(account.getId(), 5, callContext,
new ExpectedInvoiceItemCheck(new LocalDate(2017, 4, 1), new LocalDate(2017, 5, 1), InvoiceItemType.RECURRING, new BigDecimal("29.95")));
}
} }
3 changes: 3 additions & 0 deletions invoice/src/main/java/org/killbill/billing/invoice/model/InvoiceItemBase.java
View file
Expand Up @@ -229,6 +229,9 @@ public boolean matches(final Object o) {


final InvoiceItemBase that = (InvoiceItemBase) o; final InvoiceItemBase that = (InvoiceItemBase) o;


if (getInvoiceItemType() != null ? !getInvoiceItemType().equals(that.getInvoiceItemType()) : that.getInvoiceItemType() != null) {
return false;
}
if (accountId != null ? !accountId.equals(that.accountId) : that.accountId != null) { if (accountId != null ? !accountId.equals(that.accountId) : that.accountId != null) {
return false; return false;
} }
Expand Down
47 changes: 28 additions & 19 deletions invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
View file
@@ -1,7 +1,9 @@
/* /*
* Copyright 2010-2014 Ning, Inc. * Copyright 2010-2014 Ning, Inc.
* Copyright 2014-2017 Groupon, Inc
* Copyright 2014-2017 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 * (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at: * License. You may obtain a copy of the License at:
* *
Expand All @@ -16,11 +18,10 @@


package org.killbill.billing.invoice.tree; package org.killbill.billing.invoice.tree;


import java.math.BigDecimal;
import java.util.Comparator; import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.UUID; import java.util.UUID;


import javax.annotation.Nullable; import javax.annotation.Nullable;
Expand All @@ -44,8 +45,8 @@ public class SubscriptionItemTree {


private final List<Item> items = new LinkedList<Item>(); private final List<Item> items = new LinkedList<Item>();
private final List<Item> existingFullyAdjustedItems = new LinkedList<Item>(); private final List<Item> existingFullyAdjustedItems = new LinkedList<Item>();
private final List<InvoiceItem> existingFixedItems = new LinkedList<InvoiceItem>(); private final List<InvoiceItem> existingIgnoredItems = new LinkedList<InvoiceItem>();
private final Map<LocalDate, InvoiceItem> remainingFixedItems = new HashMap<LocalDate, InvoiceItem>(); private final List<InvoiceItem> remainingIgnoredItems = new LinkedList<InvoiceItem>();
private final List<InvoiceItem> pendingItemAdj = new LinkedList<InvoiceItem>(); private final List<InvoiceItem> pendingItemAdj = new LinkedList<InvoiceItem>();


private final UUID targetInvoiceId; private final UUID targetInvoiceId;
Expand Down Expand Up @@ -91,15 +92,20 @@ public void addItem(final InvoiceItem invoiceItem) {


switch (invoiceItem.getInvoiceItemType()) { switch (invoiceItem.getInvoiceItemType()) {
case RECURRING: case RECURRING:
root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD))); if (invoiceItem.getAmount().compareTo(BigDecimal.ZERO) == 0) {
// Nothing to repair -- https://github.com/killbill/killbill/issues/783
existingIgnoredItems.add(invoiceItem);
} else {
root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD)));
}
break; break;


case REPAIR_ADJ: case REPAIR_ADJ:
root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.CANCEL))); root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.CANCEL)));
break; break;


case FIXED: case FIXED:
existingFixedItems.add(invoiceItem); existingIgnoredItems.add(invoiceItem);
break; break;


case ITEM_ADJ: case ITEM_ADJ:
Expand Down Expand Up @@ -158,6 +164,17 @@ public void flatten(final boolean reverse) {
public void mergeProposedItem(final InvoiceItem invoiceItem) { public void mergeProposedItem(final InvoiceItem invoiceItem) {
Preconditions.checkState(!isBuilt, "Tree already built, unable to add new invoiceItem=%s", invoiceItem); Preconditions.checkState(!isBuilt, "Tree already built, unable to add new invoiceItem=%s", invoiceItem);


// Check if it was an existing item ignored for tree purposes (e.g. FIXED or $0 RECURRING, both of which aren't repaired)
final InvoiceItem existingItem = Iterables.tryFind(existingIgnoredItems, new Predicate<InvoiceItem>() {
@Override
public boolean apply(final InvoiceItem input) {
return input.matches(invoiceItem);
}
}).orNull();
if (existingItem != null) {
return;
}

switch (invoiceItem.getInvoiceItemType()) { switch (invoiceItem.getInvoiceItemType()) {
case RECURRING: case RECURRING:
// merged means we've either matched the proposed to an existing, or triggered a repair // merged means we've either matched the proposed to an existing, or triggered a repair
Expand All @@ -168,15 +185,7 @@ public void mergeProposedItem(final InvoiceItem invoiceItem) {
break; break;


case FIXED: case FIXED:
final InvoiceItem existingItem = Iterables.tryFind(existingFixedItems, new Predicate<InvoiceItem>() { remainingIgnoredItems.add(invoiceItem);
@Override
public boolean apply(final InvoiceItem input) {
return input.matches(invoiceItem);
}
}).orNull();
if (existingItem == null) {
remainingFixedItems.put(invoiceItem.getStartDate(), invoiceItem);
}
break; break;


default: default:
Expand Down Expand Up @@ -204,7 +213,7 @@ public void buildForMerge() {
public List<InvoiceItem> getView() { public List<InvoiceItem> getView() {


final List<InvoiceItem> tmp = new LinkedList<InvoiceItem>(); final List<InvoiceItem> tmp = new LinkedList<InvoiceItem>();
tmp.addAll(remainingFixedItems.values()); tmp.addAll(remainingIgnoredItems);
tmp.addAll(Collections2.filter(Collections2.transform(items, new Function<Item, InvoiceItem>() { tmp.addAll(Collections2.filter(Collections2.transform(items, new Function<Item, InvoiceItem>() {
@Override @Override
public InvoiceItem apply(final Item input) { public InvoiceItem apply(final Item input) {
Expand Down Expand Up @@ -278,8 +287,8 @@ public String toString() {
sb.append(", isMerged=").append(isMerged); sb.append(", isMerged=").append(isMerged);
sb.append(", items=").append(items); sb.append(", items=").append(items);
sb.append(", existingFullyAdjustedItems=").append(existingFullyAdjustedItems); sb.append(", existingFullyAdjustedItems=").append(existingFullyAdjustedItems);
sb.append(", existingFixedItems=").append(existingFixedItems); sb.append(", existingIgnoredItems=").append(existingIgnoredItems);
sb.append(", remainingFixedItems=").append(remainingFixedItems); sb.append(", remainingIgnoredItems=").append(remainingIgnoredItems);
sb.append(", pendingItemAdj=").append(pendingItemAdj); sb.append(", pendingItemAdj=").append(pendingItemAdj);
sb.append('}'); sb.append('}');
return sb.toString(); return sb.toString();
Expand Down
26 changes: 26 additions & 0 deletions invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
View file
Expand Up @@ -1493,6 +1493,32 @@ public void testWithWrongInitialItemInLoop() {
Assert.assertEquals(previousExistingSize, 3); Assert.assertEquals(previousExistingSize, 3);
} }


@Test(groups = "fast")
public void testWithFreeRecurring() {
final LocalDate startDate = new LocalDate(2012, 8, 1);
final LocalDate endDate = new LocalDate(2012, 9, 1);

final BigDecimal monthlyRate1 = new BigDecimal("12.00");
final BigDecimal monthlyRate2 = new BigDecimal("24.00");

final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
final InvoiceItem freeMonthly = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, BigDecimal.ZERO, BigDecimal.ZERO, currency);
tree.addItem(freeMonthly);
final InvoiceItem payingMonthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyRate1, monthlyRate1, currency);
tree.addItem(payingMonthly1);
tree.flatten(true);

final InvoiceItem proposedPayingMonthly2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyRate2, monthlyRate2, currency);
tree.mergeProposedItem(proposedPayingMonthly2);
tree.buildForMerge();

final List<InvoiceItem> expectedResult = Lists.newLinkedList();
expectedResult.add(proposedPayingMonthly2);
final InvoiceItem repair = new RepairAdjInvoiceItem(invoiceId, accountId, startDate, endDate, monthlyRate1.negate(), currency, payingMonthly1.getId());
expectedResult.add(repair);
verifyResult(tree.getView(), expectedResult);
}

private void printTreeJSON(final SubscriptionItemTree tree) throws IOException { private void printTreeJSON(final SubscriptionItemTree tree) throws IOException {
final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
tree.getRoot().jsonSerializeTree(OBJECT_MAPPER, outputStream); tree.getRoot().jsonSerializeTree(OBJECT_MAPPER, outputStream);
Expand Down

1 comment on commit 03f276b

@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.