killbill-memoizeit

junction: Harden code for subscription that miss START event

1/7/2016 10:19:03 PM

Details

diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
index 6806e9c..3e36378 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
@@ -23,6 +23,7 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.UUID;
@@ -92,7 +93,7 @@ public class BlockingCalculator {
      *
      * @param billingEvents the original list of billing events to update (without overdue events)
      */
-    public void insertBlockingEvents(final SortedSet<BillingEvent> billingEvents, final InternalTenantContext context) {
+    public void insertBlockingEvents(final SortedSet<BillingEvent> billingEvents, final Set<UUID> skippedSubscriptions, final InternalTenantContext context) {
         if (billingEvents.size() <= 0) {
             return;
         }
@@ -116,10 +117,16 @@ public class BlockingCalculator {
         final Map<UUID, List<BlockingState>> perSubscriptionBlockingEvents = getPerTypeBlockingEvents(BlockingStateType.SUBSCRIPTION, blockingEvents);
 
         for (final UUID bundleId : bundleMap.keySet()) {
+
+            final List<BlockingState> bundleBlockingEvents = perBundleBlockingEvents.get(bundleId)  != null ? perBundleBlockingEvents.get(bundleId) : ImmutableList.<BlockingState>of();
+
             for (final SubscriptionBase subscription : bundleMap.get(bundleId)) {
+                // Avoid inserting additional events for subscriptions that don't even have a START event
+                if (skippedSubscriptions.contains(subscription.getId())) {
+                    continue;
+                }
 
                 final List<BlockingState> subscriptionBlockingEvents = perSubscriptionBlockingEvents.get(subscription.getId()) != null ? perSubscriptionBlockingEvents.get(subscription.getId()) : ImmutableList.<BlockingState>of();
-                final List<BlockingState> bundleBlockingEvents = perBundleBlockingEvents.get(bundleId)  != null ? perBundleBlockingEvents.get(bundleId) : ImmutableList.<BlockingState>of();
                 final List<BlockingState> aggregateSubscriptionBlockingEvents = getAggregateBlockingEventsPerSubscription(subscriptionBlockingEvents, bundleBlockingEvents, accountBlockingEvents);
                 final List<DisabledDuration> accountBlockingDurations = createBlockingDurations(aggregateSubscriptionBlockingEvents);
 
diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
index ed51da8..71bf65d 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultInternalBillingApi.java
@@ -16,12 +16,17 @@
 
 package org.killbill.billing.junction.plumbing.billing;
 
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.UUID;
 
 import javax.annotation.Nullable;
 
+import org.joda.time.DateTimeZone;
+import org.joda.time.LocalDate;
 import org.killbill.billing.ObjectType;
 import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.account.api.AccountInternalApi;
@@ -38,6 +43,7 @@ import org.killbill.billing.junction.BillingEventSet;
 import org.killbill.billing.junction.BillingInternalApi;
 import org.killbill.billing.subscription.api.SubscriptionBase;
 import org.killbill.billing.subscription.api.SubscriptionBaseInternalApi;
+import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseBundle;
 import org.killbill.billing.tag.TagInternalApi;
@@ -90,9 +96,8 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
 
 
 
+        final Set<UUID> skippedSubscriptions = new HashSet<UUID>();
         try {
-
-
             // Check to see if billing is off for the account
             final List<Tag> accountTags = tagApi.getTags(accountId, ObjectType.ACCOUNT, context);
             final boolean found_AUTO_INVOICING_OFF = is_AUTO_INVOICING_OFF(accountTags);
@@ -100,7 +105,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
                 return new DefaultBillingEventSet(true, currentCatalog.getRecurringBillingMode(), account.getTimeZone()); // billing is off, we are done
             }
 
-            addBillingEventsForBundles(bundles, account, dryRunArguments, context, result);
+            addBillingEventsForBundles(bundles, account, dryRunArguments, context, result, skippedSubscriptions);
         } catch (SubscriptionBaseApiException e) {
             log.warn("Failed while getting BillingEvent", e);
         }
@@ -108,7 +113,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
         // Pretty-print the events, before and after the blocking calculator does its magic
         final StringBuilder logStringBuilder = new StringBuilder("Computed billing events for accountId ").append(accountId);
         eventsToString(logStringBuilder, result, "\nBilling Events Raw");
-        blockCalculator.insertBlockingEvents(result, context);
+        blockCalculator.insertBlockingEvents(result, skippedSubscriptions, context);
         eventsToString(logStringBuilder, result, "\nBilling Events After Blocking");
         log.info(logStringBuilder.toString());
 
@@ -123,7 +128,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
     }
 
     private void addBillingEventsForBundles(final List<SubscriptionBaseBundle> bundles, final ImmutableAccountData account, final DryRunArguments dryRunArguments, final InternalCallContext context,
-                                            final DefaultBillingEventSet result) throws SubscriptionBaseApiException, AccountApiException {
+                                            final DefaultBillingEventSet result, final Set<UUID> skipSubscriptionsSet) throws SubscriptionBaseApiException, AccountApiException {
 
         final boolean dryRunMode = dryRunArguments != null;
 
@@ -135,7 +140,7 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
             final UUID fakeBundleId = UUIDs.randomUUID();
             final List<SubscriptionBase> subscriptions = subscriptionApi.getSubscriptionsForBundle(fakeBundleId, dryRunArguments, context);
 
-            addBillingEventsForSubscription(account, subscriptions, fakeBundleId, dryRunMode, context, result);
+            addBillingEventsForSubscription(account, subscriptions, fakeBundleId, dryRunMode, context, result, skipSubscriptionsSet);
 
         }
 
@@ -154,21 +159,24 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
                     result.getSubscriptionIdsWithAutoInvoiceOff().add(subscription.getId());
                 }
             } else { // billing is not off
-                addBillingEventsForSubscription(account, subscriptions, bundle.getId(), dryRunMode, context, result);
+                addBillingEventsForSubscription(account, subscriptions, bundle.getId(), dryRunMode, context, result, skipSubscriptionsSet);
             }
         }
     }
 
+
     private void addBillingEventsForSubscription(final ImmutableAccountData account,
                                                  final List<SubscriptionBase> subscriptions,
                                                  final UUID bundleId,
                                                  final boolean dryRunMode,
                                                  final InternalCallContext context,
-                                                 final DefaultBillingEventSet result) throws AccountApiException {
+                                                 final DefaultBillingEventSet result,
+                                                 final Set<UUID> skipSubscriptionsSet) throws AccountApiException {
 
         // If dryRun is specified, we don't want to to update the account BCD value, so we initialize the flag updatedAccountBCD to true
         boolean updatedAccountBCD = dryRunMode;
 
+
         int currentAccountBCD = accountApi.getBCD(account.getId(), context);
         for (final SubscriptionBase subscription : subscriptions) {
 
@@ -177,7 +185,18 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
                 continue;
             }
 
-            for (final EffectiveSubscriptionInternalEvent transition : subscriptionApi.getBillingTransitions(subscription, context)) {
+            final List<EffectiveSubscriptionInternalEvent> billingTransitions = subscriptionApi.getBillingTransitions(subscription, context);
+            if (billingTransitions.isEmpty() ||
+                (billingTransitions.get(0).getTransitionType() != SubscriptionBaseTransitionType.CREATE &&
+                 billingTransitions.get(0).getTransitionType() != SubscriptionBaseTransitionType.MIGRATE_BILLING &&
+                 billingTransitions.get(0).getTransitionType() != SubscriptionBaseTransitionType.TRANSFER)) {
+                log.warn("Skipping billing events for subscription " + subscription.getId() + ": Does not start with a valid CREATE transition");
+                skipSubscriptionsSet.add(subscription.getId());
+                return;
+            }
+
+
+            for (final EffectiveSubscriptionInternalEvent transition : billingTransitions) {
                 try {
                     final int bcdLocal = bcdCalculator.calculateBcd(account, currentAccountBCD, bundleId, subscription, transition, context);
 
diff --git a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
index 15b967f..c0abd20 100644
--- a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
+++ b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
@@ -18,6 +18,7 @@ package org.killbill.billing.junction.plumbing.billing;
 
 import java.math.BigDecimal;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
@@ -118,7 +119,7 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
 
         setBlockingStates(blockingStates);
 
-        blockingCalculator.insertBlockingEvents(billingEvents, internalCallContext);
+        blockingCalculator.insertBlockingEvents(billingEvents, new HashSet<UUID>(), internalCallContext);
 
         assertEquals(billingEvents.size(), 7);
 
@@ -750,7 +751,7 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
 
         setBlockingStates(blockingEvents);
 
-        blockingCalculator.insertBlockingEvents(billingEvents, internalCallContext);
+        blockingCalculator.insertBlockingEvents(billingEvents, new HashSet<UUID>(), internalCallContext);
 
         assertEquals(billingEvents.size(), 5);
         final List<BillingEvent> events = new ArrayList<BillingEvent>(billingEvents);