killbill-uncached
Changes
invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java 46(+45 -1)
invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java 14(+12 -2)
Details
diff --git a/beatrix/src/test/resources/beatrix.properties b/beatrix/src/test/resources/beatrix.properties
index acc60b4..c0f1c1a 100644
--- a/beatrix/src/test/resources/beatrix.properties
+++ b/beatrix/src/test/resources/beatrix.properties
@@ -1,3 +1,4 @@
org.killbill.catalog.uri=catalogTest.xml
+org.killbill.invoice.maxDailyNumberOfItemsSafetyBound=30
org.killbill.payment.retry.days=8,8,8,8,8,8,8,8
org.killbill.osgi.bundle.install.dir=/var/tmp/beatrix-bundles
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/config/MultiTenantInvoiceConfig.java b/invoice/src/main/java/org/killbill/billing/invoice/config/MultiTenantInvoiceConfig.java
index 65f339f..9fad7c8 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/config/MultiTenantInvoiceConfig.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/config/MultiTenantInvoiceConfig.java
@@ -55,6 +55,20 @@ public class MultiTenantInvoiceConfig extends MultiTenantConfigBase implements I
}
@Override
+ public int getMaxDailyNumberOfItemsSafetyBound() {
+ return staticConfig.getMaxDailyNumberOfItemsSafetyBound();
+ }
+
+ @Override
+ public int getMaxDailyNumberOfItemsSafetyBound(final InternalTenantContext tenantContext) {
+ final String result = getStringTenantConfig("getMaxDailyNumberOfItemsSafetyBound", tenantContext);
+ if (result != null) {
+ return Integer.parseInt(result);
+ }
+ return getMaxDailyNumberOfItemsSafetyBound();
+ }
+
+ @Override
public TimeSpan getDryRunNotificationSchedule() {
return staticConfig.getDryRunNotificationSchedule();
}
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java b/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
index 55106c2..259c570 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
@@ -30,6 +30,7 @@ import org.joda.time.LocalDate;
import org.killbill.billing.ErrorCode;
import org.killbill.billing.account.api.ImmutableAccountData;
import org.killbill.billing.callcontext.InternalCallContext;
+import org.killbill.billing.callcontext.InternalTenantContext;
import org.killbill.billing.catalog.api.BillingMode;
import org.killbill.billing.catalog.api.BillingPeriod;
import org.killbill.billing.catalog.api.CatalogApiException;
@@ -47,11 +48,15 @@ import org.killbill.billing.invoice.model.RecurringInvoiceItemDataWithNextBillin
import org.killbill.billing.invoice.tree.AccountItemTree;
import org.killbill.billing.junction.BillingEvent;
import org.killbill.billing.junction.BillingEventSet;
+import org.killbill.billing.util.config.definition.InvoiceConfig;
import org.killbill.billing.util.currency.KillBillMoney;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.Multimap;
+import com.google.inject.Inject;
import static org.killbill.billing.invoice.generator.InvoiceDateUtils.calculateNumberOfWholeBillingPeriods;
import static org.killbill.billing.invoice.generator.InvoiceDateUtils.calculateProRationAfterLastBillingCycleDate;
@@ -61,10 +66,19 @@ public class FixedAndRecurringInvoiceItemGenerator extends InvoiceItemGenerator
private static final Logger log = LoggerFactory.getLogger(FixedAndRecurringInvoiceItemGenerator.class);
+ private final InvoiceConfig config;
+
+ @Inject
+ public FixedAndRecurringInvoiceItemGenerator(final InvoiceConfig config) {
+ this.config = config;
+ }
+
public List<InvoiceItem> generateItems(final ImmutableAccountData account, final UUID invoiceId, final BillingEventSet eventSet,
@Nullable final List<Invoice> existingInvoices, final LocalDate targetDate,
final Currency targetCurrency, final Map<UUID, SubscriptionFutureNotificationDates> perSubscriptionFutureNotificationDate,
final InternalCallContext internalCallContext) throws InvoiceApiException {
+ final Multimap<UUID, LocalDate> createdItemsPerDayPerSubscription = LinkedListMultimap.<UUID, LocalDate>create();
+
final AccountItemTree accountItemTree = new AccountItemTree(account.getId(), invoiceId);
if (existingInvoices != null) {
for (final Invoice invoice : existingInvoices) {
@@ -73,6 +87,8 @@ public class FixedAndRecurringInvoiceItemGenerator extends InvoiceItemGenerator
!eventSet.getSubscriptionIdsWithAutoInvoiceOff()
.contains(item.getSubscriptionId())) { //don't add items with auto_invoice_off tag
accountItemTree.addExistingItem(item);
+
+ trackInvoiceItemCreatedDay(item, createdItemsPerDayPerSubscription, internalCallContext);
}
}
}
@@ -83,7 +99,11 @@ public class FixedAndRecurringInvoiceItemGenerator extends InvoiceItemGenerator
processRecurringBillingEvents(invoiceId, account.getId(), eventSet, targetDate, targetCurrency, proposedItems, perSubscriptionFutureNotificationDate, existingInvoices, internalCallContext);
processFixedBillingEvents(invoiceId, account.getId(), eventSet, targetDate, targetCurrency, proposedItems, internalCallContext);
accountItemTree.mergeWithProposedItems(proposedItems);
- return accountItemTree.getResultingItemList();
+
+ final List<InvoiceItem> resultingItems = accountItemTree.getResultingItemList();
+ safetyBound(resultingItems, createdItemsPerDayPerSubscription, internalCallContext);
+
+ return resultingItems;
}
private void processRecurringBillingEvents(final UUID invoiceId, final UUID accountId, final BillingEventSet events,
@@ -376,4 +396,28 @@ public class FixedAndRecurringInvoiceItemGenerator extends InvoiceItemGenerator
}
}
}
+
+ // Trigger an exception if we create too many subscriptions for a subscription on a given day
+ private void safetyBound(final Iterable<InvoiceItem> resultingItems, final Multimap<UUID, LocalDate> createdItemsPerDayPerSubscription, final InternalTenantContext internalCallContext) throws InvoiceApiException {
+ for (final InvoiceItem invoiceItem : resultingItems) {
+ if (invoiceItem.getSubscriptionId() != null) {
+ trackInvoiceItemCreatedDay(invoiceItem, createdItemsPerDayPerSubscription, internalCallContext);
+
+ if (createdItemsPerDayPerSubscription.get(invoiceItem.getSubscriptionId()).size() > config.getMaxDailyNumberOfItemsSafetyBound(internalCallContext)) {
+ // Proposed items have already been logged
+ throw new InvoiceApiException(ErrorCode.UNEXPECTED_ERROR, String.format("SAFETY BOUND TRIGGERED subscriptionId='%s', resultingItem=%s", invoiceItem.getSubscriptionId(), invoiceItem));
+ }
+ }
+ }
+ }
+
+ private void trackInvoiceItemCreatedDay(final InvoiceItem invoiceItem, final Multimap<UUID, LocalDate> createdItemsPerDayPerSubscription, final InternalTenantContext internalCallContext) {
+ final UUID subscriptionId = invoiceItem.getSubscriptionId();
+ if (subscriptionId == null) {
+ return;
+ }
+
+ final LocalDate createdDay = internalCallContext.toLocalDate(invoiceItem.getCreatedDate());
+ createdItemsPerDayPerSubscription.put(subscriptionId, createdDay);
+ }
}
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
index 8c9c8c5..f28bca2 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/generator/TestDefaultInvoiceGenerator.java
@@ -1,7 +1,7 @@
/*
* Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2016 Groupon, Inc
+ * Copyright 2014-2016 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
@@ -122,6 +122,16 @@ public class TestDefaultInvoiceGenerator extends InvoiceTestSuiteNoDB {
}
@Override
+ public int getMaxDailyNumberOfItemsSafetyBound() {
+ return 10;
+ }
+
+ @Override
+ public int getMaxDailyNumberOfItemsSafetyBound(final InternalTenantContext tenantContext) {
+ return 10;
+ }
+
+ @Override
public boolean isEmailNotificationsEnabled() {
return false;
}
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 3cd5432..ca56b1f 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
@@ -19,11 +19,14 @@ package org.killbill.billing.invoice.generator;
import java.math.BigDecimal;
import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.UUID;
import org.joda.time.DateTime;
import org.joda.time.LocalDate;
+import org.killbill.billing.ErrorCode;
import org.killbill.billing.account.api.Account;
import org.killbill.billing.catalog.DefaultPrice;
import org.killbill.billing.catalog.MockInternationalPrice;
@@ -37,21 +40,25 @@ import org.killbill.billing.catalog.api.Plan;
import org.killbill.billing.catalog.api.PlanPhase;
import org.killbill.billing.invoice.InvoiceTestSuiteNoDB;
import org.killbill.billing.invoice.MockBillingEventSet;
+import org.killbill.billing.invoice.api.Invoice;
import org.killbill.billing.invoice.api.InvoiceApiException;
import org.killbill.billing.invoice.api.InvoiceItem;
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.RecurringInvoiceItem;
import org.killbill.billing.junction.BillingEvent;
import org.killbill.billing.junction.BillingEventSet;
import org.killbill.billing.subscription.api.SubscriptionBase;
import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
-import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteNoDB {
@@ -67,7 +74,7 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
account = invoiceUtil.createAccount(callContext);
subscription = invoiceUtil.createSubscription();
} catch (final Exception e) {
- Assert.fail(e.getMessage());
+ fail(e.getMessage());
}
}
@@ -180,7 +187,6 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
SubscriptionBaseTransitionType.CREATE);
events.add(event1);
-
final BillingEvent event2 = invoiceUtil.createMockBillingEvent(account, subscription, new DateTime("2016-01-08"),
plan, phase,
null, null, Currency.USD, BillingPeriod.NO_BILLING_PERIOD, 1,
@@ -213,7 +219,6 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
SubscriptionBaseTransitionType.CREATE);
events.add(event1);
-
final BillingEvent event2 = invoiceUtil.createMockBillingEvent(account, subscription, new DateTime("2016-01-09"),
plan, phase,
null, null, Currency.USD, BillingPeriod.NO_BILLING_PERIOD, 1,
@@ -228,7 +233,6 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
assertEquals(proposedItems.get(0).getAmount().compareTo(fixedPriceAmount), 0);
}
-
@Test(groups = "fast")
public void testProcessFixedBillingEventsWithMultipleChangeOnSameDay() throws InvoiceApiException {
@@ -249,8 +253,6 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
SubscriptionBaseTransitionType.CREATE);
events.add(event1);
-
-
final BigDecimal fixedPriceAmount2 = null;
final MockInternationalPrice fixedPrice2 = new MockInternationalPrice(new DefaultPrice(fixedPriceAmount2, Currency.USD));
final Plan plan2 = new MockPlan("my-plan2");
@@ -263,7 +265,6 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
SubscriptionBaseTransitionType.CHANGE);
events.add(event2);
-
final BigDecimal fixedPriceAmount3 = BigDecimal.ONE;
final MockInternationalPrice fixedPrice3 = new MockInternationalPrice(new DefaultPrice(fixedPriceAmount3, Currency.USD));
final Plan plan3 = new MockPlan("my-plan3");
@@ -283,4 +284,64 @@ public class TestFixedAndRecurringInvoiceItemGenerator extends InvoiceTestSuiteN
assertEquals(proposedItems.get(0).getAmount().compareTo(fixedPriceAmount3), 0);
}
+ @Test(groups = "fast")
+ public void testSafetyBounds() throws InvoiceApiException {
+ final int threshold = 15;
+ 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(),
+ planPhase.getRecurring().getBillingPeriod(),
+ 1,
+ BillingMode.IN_ADVANCE,
+ "Billing Event Desc",
+ 1L,
+ SubscriptionBaseTransitionType.CREATE);
+ events.add(event);
+
+ // Simulate a big catch-up
+ final List<Invoice> existingInvoices = new LinkedList<Invoice>();
+ for (int i = 0; i < threshold; i++) {
+ final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate.plusMonths(i), account.getCurrency());
+ invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
+ clock.getUTCNow(),
+ invoice.getId(),
+ account.getId(),
+ subscription.getBundleId(),
+ subscription.getId(),
+ event.getPlan().getName(),
+ event.getPlanPhase().getName(),
+ startDate.plusMonths(i),
+ startDate.plusMonths(1 + i),
+ amount,
+ amount,
+ account.getCurrency()));
+ existingInvoices.add(invoice);
+ }
+
+ try {
+ final List<InvoiceItem> generatedItems = fixedAndRecurringInvoiceItemGenerator.generateItems(account,
+ UUID.randomUUID(),
+ events,
+ existingInvoices,
+ startDate.plusMonths(threshold),
+ account.getCurrency(),
+ new HashMap<UUID, SubscriptionFutureNotificationDates>(),
+ internalCallContext);
+ fail();
+ } catch (final InvoiceApiException e) {
+ assertEquals(e.getCode(), ErrorCode.UNEXPECTED_ERROR.getCode());
+ }
+ }
}
diff --git a/util/src/main/java/org/killbill/billing/util/config/definition/InvoiceConfig.java b/util/src/main/java/org/killbill/billing/util/config/definition/InvoiceConfig.java
index 5e05cad..776621b 100644
--- a/util/src/main/java/org/killbill/billing/util/config/definition/InvoiceConfig.java
+++ b/util/src/main/java/org/killbill/billing/util/config/definition/InvoiceConfig.java
@@ -36,6 +36,16 @@ public interface InvoiceConfig extends KillbillConfig {
@Description("Maximum target date to consider when generating an invoice")
int getNumberOfMonthsInFuture(@Param("dummy") final InternalTenantContext tenantContext);
+ @Config("org.killbill.invoice.maxDailyNumberOfItemsSafetyBound")
+ @Default("15")
+ @Description("Maximum daily number of invoice items to generate for a subscription id")
+ int getMaxDailyNumberOfItemsSafetyBound();
+
+ @Config("org.killbill.invoice.maxDailyNumberOfItemsSafetyBound")
+ @Default("15")
+ @Description("Maximum daily number of invoice items to generate for a subscription id")
+ int getMaxDailyNumberOfItemsSafetyBound(@Param("dummy") final InternalTenantContext tenantContext);
+
@Config("org.killbill.invoice.dryRunNotificationSchedule")
@Default("0s")
@Description("DryRun invoice notification time before targetDate (ignored if set to 0s)")