diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoice.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoice.java
index 1f481ea..1cbad77 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoice.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationInvoice.java
@@ -1,6 +1,6 @@
/*
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
*
* 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
@@ -30,6 +30,7 @@ import org.killbill.billing.ObjectType;
import org.killbill.billing.account.api.Account;
import org.killbill.billing.api.TestApiListener.NextEvent;
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.BillingPeriod;
import org.killbill.billing.catalog.api.PlanPhasePriceOverride;
@@ -38,7 +39,6 @@ import org.killbill.billing.catalog.api.PriceListSet;
import org.killbill.billing.catalog.api.ProductCategory;
import org.killbill.billing.entitlement.api.DefaultEntitlement;
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.invoice.api.DryRunArguments;
import org.killbill.billing.invoice.api.DryRunType;
@@ -50,14 +50,12 @@ import org.killbill.billing.invoice.model.ExternalChargeInvoiceItem;
import org.killbill.billing.payment.api.Payment;
import org.killbill.billing.payment.api.PluginProperty;
import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase;
-import org.testng.Assert;
import org.testng.annotations.Test;
import com.google.common.collect.ImmutableList;
import static com.tc.util.Assert.fail;
import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
public class TestIntegrationInvoice extends TestIntegrationBase {
@@ -506,4 +504,65 @@ public class TestIntegrationInvoice extends TestIntegrationBase {
}
+ @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")));
+ }
}
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 df593b8..d194140 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
@@ -1,7 +1,9 @@
/*
* 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
* License. You may obtain a copy of the License at:
*
@@ -16,11 +18,10 @@
package org.killbill.billing.invoice.tree;
+import java.math.BigDecimal;
import java.util.Comparator;
-import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
-import java.util.Map;
import java.util.UUID;
import javax.annotation.Nullable;
@@ -44,8 +45,8 @@ public class SubscriptionItemTree {
private final List<Item> items = new LinkedList<Item>();
private final List<Item> existingFullyAdjustedItems = new LinkedList<Item>();
- private final List<InvoiceItem> existingFixedItems = new LinkedList<InvoiceItem>();
- private final Map<LocalDate, InvoiceItem> remainingFixedItems = new HashMap<LocalDate, InvoiceItem>();
+ private final List<InvoiceItem> existingIgnoredItems = new LinkedList<InvoiceItem>();
+ private final List<InvoiceItem> remainingIgnoredItems = new LinkedList<InvoiceItem>();
private final List<InvoiceItem> pendingItemAdj = new LinkedList<InvoiceItem>();
private final UUID targetInvoiceId;
@@ -91,7 +92,12 @@ public class SubscriptionItemTree {
switch (invoiceItem.getInvoiceItemType()) {
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;
case REPAIR_ADJ:
@@ -99,7 +105,7 @@ public class SubscriptionItemTree {
break;
case FIXED:
- existingFixedItems.add(invoiceItem);
+ existingIgnoredItems.add(invoiceItem);
break;
case ITEM_ADJ:
@@ -158,6 +164,17 @@ public class SubscriptionItemTree {
public void mergeProposedItem(final InvoiceItem 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()) {
case RECURRING:
// merged means we've either matched the proposed to an existing, or triggered a repair
@@ -168,15 +185,7 @@ public class SubscriptionItemTree {
break;
case FIXED:
- final InvoiceItem existingItem = Iterables.tryFind(existingFixedItems, new Predicate<InvoiceItem>() {
- @Override
- public boolean apply(final InvoiceItem input) {
- return input.matches(invoiceItem);
- }
- }).orNull();
- if (existingItem == null) {
- remainingFixedItems.put(invoiceItem.getStartDate(), invoiceItem);
- }
+ remainingIgnoredItems.add(invoiceItem);
break;
default:
@@ -204,7 +213,7 @@ public class SubscriptionItemTree {
public List<InvoiceItem> getView() {
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>() {
@Override
public InvoiceItem apply(final Item input) {
@@ -278,8 +287,8 @@ public class SubscriptionItemTree {
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(", existingIgnoredItems=").append(existingIgnoredItems);
+ sb.append(", remainingIgnoredItems=").append(remainingIgnoredItems);
sb.append(", pendingItemAdj=").append(pendingItemAdj);
sb.append('}');
return sb.toString();
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 26f4170..49f3ce2 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
@@ -1493,6 +1493,32 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
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 {
final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
tree.getRoot().jsonSerializeTree(OBJECT_MAPPER, outputStream);
