Skip to content

Commit

Permalink
invoice: CR for safety bounds
Browse files Browse the repository at this point in the history 
* Fix per-day check
* Add ability to disable check via -1 value

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Nov 7, 2016
1 parent 19c329e commit 432669d
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
27 changes: 21 additions & 6 deletions ...in/java/org/killbill/billing/invoice/generator/FixedAndRecurringInvoiceItemGenerator.java
View file
Expand Up @@ -19,6 +19,7 @@


import java.math.BigDecimal; import java.math.BigDecimal;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -399,25 +400,39 @@ private InvoiceItem generateFixedPriceItem(final UUID invoiceId, final UUID acco


// Trigger an exception if we create too many subscriptions for a subscription on a given day // 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 { private void safetyBound(final Iterable<InvoiceItem> resultingItems, final Multimap<UUID, LocalDate> createdItemsPerDayPerSubscription, final InternalTenantContext internalCallContext) throws InvoiceApiException {
if (config.getMaxDailyNumberOfItemsSafetyBound(internalCallContext) == -1) {
// Safety bound disabled
return;
}

for (final InvoiceItem invoiceItem : resultingItems) { for (final InvoiceItem invoiceItem : resultingItems) {
if (invoiceItem.getSubscriptionId() != null) { if (invoiceItem.getSubscriptionId() != null) {
trackInvoiceItemCreatedDay(invoiceItem, createdItemsPerDayPerSubscription, internalCallContext); final LocalDate resultingItemCreationDay = trackInvoiceItemCreatedDay(invoiceItem, createdItemsPerDayPerSubscription, internalCallContext);

final Collection<LocalDate> creationDaysForSubscription = createdItemsPerDayPerSubscription.get(invoiceItem.getSubscriptionId());
int i = 0;
for (final LocalDate creationDayForSubscription : creationDaysForSubscription) {
if (creationDayForSubscription.compareTo(resultingItemCreationDay) == 0) {
i++;
if (i > 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));
}


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) { private LocalDate trackInvoiceItemCreatedDay(final InvoiceItem invoiceItem, final Multimap<UUID, LocalDate> createdItemsPerDayPerSubscription, final InternalTenantContext internalCallContext) {
final UUID subscriptionId = invoiceItem.getSubscriptionId(); final UUID subscriptionId = invoiceItem.getSubscriptionId();
if (subscriptionId == null) { if (subscriptionId == null) {
return; return null;
} }


final LocalDate createdDay = internalCallContext.toLocalDate(invoiceItem.getCreatedDate()); final LocalDate createdDay = internalCallContext.toLocalDate(invoiceItem.getCreatedDate());
createdItemsPerDayPerSubscription.put(subscriptionId, createdDay); createdItemsPerDayPerSubscription.put(subscriptionId, createdDay);
return createdDay;
} }
} }
4 changes: 3 additions & 1 deletion invoice/src/test/java/org/killbill/billing/invoice/InvoiceTestSuiteWithEmbeddedDB.java
View file
Expand Up @@ -42,6 +42,7 @@
import org.killbill.billing.util.dao.NonEntityDao; import org.killbill.billing.util.dao.NonEntityDao;
import org.killbill.bus.api.PersistentBus; import org.killbill.bus.api.PersistentBus;
import org.killbill.clock.Clock; import org.killbill.clock.Clock;
import org.killbill.clock.ClockMock;
import org.killbill.commons.locker.GlobalLocker; import org.killbill.commons.locker.GlobalLocker;
import org.killbill.notificationq.api.NotificationQueueService; import org.killbill.notificationq.api.NotificationQueueService;
import org.slf4j.Logger; import org.slf4j.Logger;
Expand Down Expand Up @@ -91,7 +92,7 @@ public abstract class InvoiceTestSuiteWithEmbeddedDB extends GuicyKillbillTestSu
@Inject @Inject
protected GlobalLocker locker; protected GlobalLocker locker;
@Inject @Inject
protected Clock clock; protected ClockMock clock;
@Inject @Inject
protected InternalCallContextFactory internalCallContextFactory; protected InternalCallContextFactory internalCallContextFactory;
@Inject @Inject
Expand Down Expand Up @@ -127,6 +128,7 @@ public void beforeMethod() throws Exception {
controllerDispatcher.clearAll(); controllerDispatcher.clearAll();
bus.start(); bus.start();
restartInvoiceService(invoiceService); restartInvoiceService(invoiceService);
clock.resetDeltaFromReality();
} }


private void restartInvoiceService(final InvoiceService invoiceService) throws Exception { private void restartInvoiceService(final InvoiceService invoiceService) throws Exception {
Expand Down
32 changes: 30 additions & 2 deletions ...ava/org/killbill/billing/invoice/generator/TestFixedAndRecurringInvoiceItemGenerator.java
View file
Expand Up @@ -315,7 +315,35 @@ public void testSafetyBounds() throws InvoiceApiException {
for (int i = 0; i < threshold; i++) { for (int i = 0; i < threshold; i++) {
final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate.plusMonths(i), account.getCurrency()); final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate.plusMonths(i), account.getCurrency());
invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(), invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
clock.getUTCNow(), startDate.plusMonths(i).toDateTimeAtStartOfDay(), // Different days - should not trigger the safety bounds
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);
}

assertEquals(fixedAndRecurringInvoiceItemGenerator.generateItems(account,
UUID.randomUUID(),
events,
existingInvoices,
startDate.plusMonths(threshold),
account.getCurrency(),
new HashMap<UUID, SubscriptionFutureNotificationDates>(),
internalCallContext).size(), 1);

// Simulate a big catch-up on that day
for (int i = threshold; i < 2 * threshold; i++) {
final Invoice invoice = new DefaultInvoice(account.getId(), clock.getUTCToday(), startDate.plusMonths(i), account.getCurrency());
invoice.addInvoiceItem(new RecurringInvoiceItem(UUID.randomUUID(),
clock.getUTCNow(), // Same day
invoice.getId(), invoice.getId(),
account.getId(), account.getId(),
subscription.getBundleId(), subscription.getBundleId(),
Expand All @@ -335,7 +363,7 @@ public void testSafetyBounds() throws InvoiceApiException {
UUID.randomUUID(), UUID.randomUUID(),
events, events,
existingInvoices, existingInvoices,
startDate.plusMonths(threshold), startDate.plusMonths(2 * threshold),
account.getCurrency(), account.getCurrency(),
new HashMap<UUID, SubscriptionFutureNotificationDates>(), new HashMap<UUID, SubscriptionFutureNotificationDates>(),
internalCallContext); internalCallContext);
Expand Down

0 comments on commit 432669d

Please sign in to comment.