killbill-memoizeit

subscription: DB calls optimizations Signed-off-by: Pierre-Alexandre

4/18/2018 1:35:14 PM

Details

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
index 5a62193..c6f4c72 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
@@ -146,16 +146,6 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                                                                              final Catalog catalog,
                                                                              final CallContext callContext) throws SubscriptionBaseApiException, CatalogApiException {
         final List<SubscriptionSpecifier> subscriptions = new ArrayList<SubscriptionSpecifier>();
-        // TODO We should also look to the specifiers being created for validation
-        final List<SubscriptionBase> subscriptionsForBundle = getSubscriptionsForBundle(bundle.getId(), null, context);
-        SubscriptionBase baseSubscription = null;
-        for (final SubscriptionBase cur : subscriptionsForBundle) {
-            if (cur.getCategory() == ProductCategory.BASE) {
-                baseSubscription = cur;
-                break;
-            }
-        }
-
         for (final EntitlementSpecifier entitlement : entitlements) {
             final PlanPhaseSpecifier spec = entitlement.getPlanPhaseSpecifier();
             if (spec == null) {
@@ -175,6 +165,8 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
             // verify the number of subscriptions (of the same kind) allowed per bundle and the existing ones
             if (ProductCategory.ADD_ON.toString().equalsIgnoreCase(plan.getProduct().getCategory().toString())) {
                 if (plan.getPlansAllowedInBundle() != -1 && plan.getPlansAllowedInBundle() > 0) {
+                    // TODO We should also look to the specifiers being created for validation
+                    final List<SubscriptionBase> subscriptionsForBundle = getSubscriptionsForBundle(bundle.getId(), null, context);
                     final int existingAddOnsWithSamePlanName = addonUtils.countExistingAddOnsWithSamePlanName(subscriptionsForBundle, plan.getName());
                     final int currentAddOnsWithSamePlanName = countCurrentAddOnsWithSamePlanName(entitlements, catalog, plan.getName(), effectiveDate, callContext);
                     if ((existingAddOnsWithSamePlanName + currentAddOnsWithSamePlanName) > plan.getPlansAllowedInBundle()) {
@@ -184,7 +176,13 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                 }
             }
 
-            final DateTime bundleStartDate = baseOrStandalonePlanSpecifier != null ? effectiveDate : getBundleStartDateWithSanity(bundle.getId(), baseSubscription, plan, effectiveDate, catalog, context);
+            final DateTime bundleStartDate;
+            if (baseOrStandalonePlanSpecifier != null) {
+                bundleStartDate = effectiveDate;
+            } else {
+                final SubscriptionBase baseSubscription = dao.getBaseSubscription(bundle.getId(), catalog, context);
+                bundleStartDate = getBundleStartDateWithSanity(bundle.getId(), baseSubscription, plan, effectiveDate, catalog, context);
+            }
 
             final SubscriptionSpecifier subscription = new SubscriptionSpecifier();
             subscription.setRealPriceList(plan.getPriceListName());
@@ -273,13 +271,12 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                     if (bundle == null || (subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey() != null && subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey().equals(bundle.getExternalKey()))) {
                         throw new SubscriptionBaseApiException(ErrorCode.SUB_CREATE_INVALID_ENTITLEMENT_SPECIFIER);
                     }
-                } else if (subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey() != null) {
+                } else if (subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey() != null &&
+                           baseOrStandalonePlanSpecifier == null) { // Skip the expensive checks if we are about to create the bundle (validation will be done in SubscriptionDao#createSubscriptionBundle)
                     final List<SubscriptionBaseBundle> existingBundles = dao.getSubscriptionBundlesForKey(subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey(), context);
                     final SubscriptionBaseBundle tmp = getActiveBundleForKeyNotException(existingBundles, dao, clock, catalog, context);
                     if (tmp == null) {
-                        if (baseOrStandalonePlanSpecifier == null) {
-                            throw new SubscriptionBaseApiException(ErrorCode.SUB_CREATE_NO_BP, subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey());
-                        }
+                        throw new SubscriptionBaseApiException(ErrorCode.SUB_CREATE_NO_BP, subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey());
                     } else if (!tmp.getAccountId().equals(accountId)) {
                         throw new SubscriptionBaseApiException(ErrorCode.SUB_CREATE_ACTIVE_BUNDLE_KEY_EXISTS, subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey());
                     } else {
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
index 3d9a13b..0dd6967 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBaseApiService.java
@@ -76,8 +76,13 @@ import org.killbill.clock.Clock;
 import org.killbill.clock.DefaultClock;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Lists;
 import com.google.inject.Inject;
 
 public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiService {
@@ -121,11 +126,15 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
                 allSubscriptions.add(subscriptionBaseWithAddOns);
             }
 
-            dao.createSubscriptionsWithAddOns(allSubscriptions, eventsMap, fullCatalog, internalCallContext);
+            final List<SubscriptionBaseEvent> events = dao.createSubscriptionsWithAddOns(allSubscriptions, eventsMap, fullCatalog, internalCallContext);
+            final ListMultimap<UUID, SubscriptionBaseEvent> eventsBySubscription = ArrayListMultimap.<UUID, SubscriptionBaseEvent>create();
+            for (final SubscriptionBaseEvent event : events) {
+                eventsBySubscription.put(event.getSubscriptionId(), event);
+            }
 
             for (final List<SubscriptionBase> subscriptions : subscriptionBaseAndAddOnsList) {
                 for (final SubscriptionBase input : subscriptions) {
-                    ((DefaultSubscriptionBase) input).rebuildTransitions(dao.getEventsForSubscription(input.getId(), internalCallContext), fullCatalog);
+                    ((DefaultSubscriptionBase) input).rebuildTransitions(eventsBySubscription.get(input.getId()), fullCatalog);
                 }
             }
             return allSubscriptions;
diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiError.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiError.java
index 27a34bd..418d4f5 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiError.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiError.java
@@ -68,12 +68,6 @@ public class TestUserApiError extends SubscriptionTestSuiteNoDB {
     }
 
     @Test(groups = "fast")
-    public void testCreateSubscriptionBPExists() throws SubscriptionBaseApiException {
-        testUtil.createSubscription(bundle, "Shotgun", BillingPeriod.ANNUAL, PriceListSet.DEFAULT_PRICELIST_NAME);
-        tCreateSubscriptionInternal(bundle, "Shotgun", BillingPeriod.ANNUAL, PriceListSet.DEFAULT_PRICELIST_NAME, ErrorCode.SUB_CREATE_BP_EXISTS);
-    }
-
-    @Test(groups = "fast")
     public void testCreateSubscriptionAddOnNotAvailable() throws SubscriptionBaseApiException {
         testUtil.createSubscription(bundle, "Pistol", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME);
         tCreateSubscriptionInternal(bundle, "Telescopic-Scope", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, ErrorCode.SUB_CREATE_AO_NOT_AVAILABLE);