Skip to content

Commit

Permalink
entitlement: bug fixes in bulk create API
Browse files Browse the repository at this point in the history
* Block action if base subscription is block_entitlement
* Prevent add-on to be created if needed when
  bundle is specified by external key

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
pierre committed Apr 26, 2018
1 parent 3b42050 commit b41029f
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 43 deletions.
@@ -1,7 +1,7 @@
/* /*
* Copyright 2010-2013 Ning, Inc. * Copyright 2010-2013 Ning, Inc.
* Copyright 2014-2017 Groupon, Inc * Copyright 2014-2018 Groupon, Inc
* Copyright 2014-2017 The Billing Project, LLC * Copyright 2014-2018 The Billing Project, LLC
* *
* The Billing Project 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 * (the "License"); you may not use this file except in compliance with the
Expand Down Expand Up @@ -64,6 +64,8 @@ public interface EventsStream {


boolean isBlockChange(final DateTime effectiveDate); boolean isBlockChange(final DateTime effectiveDate);


boolean isBlockEntitlement(final DateTime effectiveDate);

int getDefaultBillCycleDayLocal(); int getDefaultBillCycleDayLocal();


Collection<BlockingState> getPendingEntitlementCancellationEvents(); Collection<BlockingState> getPendingEntitlementCancellationEvents();
Expand Down
Expand Up @@ -65,6 +65,8 @@ public List<SubscriptionBaseBundle> getBundlesForAccountAndKey(UUID accountId, S


public Iterable<UUID> getNonAOSubscriptionIdsForKey(String bundleKey, InternalTenantContext context); public Iterable<UUID> getNonAOSubscriptionIdsForKey(String bundleKey, InternalTenantContext context);


public SubscriptionBaseBundle getActiveBundleForKey(String bundleKey, Catalog catalog, InternalTenantContext context);

public List<SubscriptionBase> getSubscriptionsForBundle(UUID bundleId, DryRunArguments dryRunArguments, InternalTenantContext context) public List<SubscriptionBase> getSubscriptionsForBundle(UUID bundleId, DryRunArguments dryRunArguments, InternalTenantContext context)
throws SubscriptionBaseApiException; throws SubscriptionBaseApiException;


Expand Down
Expand Up @@ -38,6 +38,9 @@
import org.killbill.billing.callcontext.InternalCallContext; import org.killbill.billing.callcontext.InternalCallContext;
import org.killbill.billing.callcontext.InternalTenantContext; import org.killbill.billing.callcontext.InternalTenantContext;
import org.killbill.billing.catalog.api.BillingActionPolicy; import org.killbill.billing.catalog.api.BillingActionPolicy;
import org.killbill.billing.catalog.api.Catalog;
import org.killbill.billing.catalog.api.CatalogApiException;
import org.killbill.billing.catalog.api.CatalogInternalApi;
import org.killbill.billing.catalog.api.PlanPhasePriceOverride; import org.killbill.billing.catalog.api.PlanPhasePriceOverride;
import org.killbill.billing.catalog.api.PlanPhaseSpecifier; import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
import org.killbill.billing.catalog.api.ProductCategory; import org.killbill.billing.catalog.api.ProductCategory;
Expand Down Expand Up @@ -102,6 +105,7 @@ public class DefaultEntitlementApi extends DefaultEntitlementApiBase implements
private final NotificationQueueService notificationQueueService; private final NotificationQueueService notificationQueueService;
private final EntitlementPluginExecution pluginExecution; private final EntitlementPluginExecution pluginExecution;
private final SecurityApi securityApi; private final SecurityApi securityApi;
private final CatalogInternalApi catalogInternalApi;


@Inject @Inject
public DefaultEntitlementApi(final PersistentBus eventBus, final InternalCallContextFactory internalCallContextFactory, public DefaultEntitlementApi(final PersistentBus eventBus, final InternalCallContextFactory internalCallContextFactory,
Expand All @@ -110,6 +114,7 @@ public DefaultEntitlementApi(final PersistentBus eventBus, final InternalCallCon
final BlockingChecker checker, final NotificationQueueService notificationQueueService, final BlockingChecker checker, final NotificationQueueService notificationQueueService,
final EventsStreamBuilder eventsStreamBuilder, final EntitlementUtils entitlementUtils, final EventsStreamBuilder eventsStreamBuilder, final EntitlementUtils entitlementUtils,
final EntitlementPluginExecution pluginExecution, final EntitlementPluginExecution pluginExecution,
final CatalogInternalApi catalogInternalApi,
final SecurityApi securityApi) { final SecurityApi securityApi) {
super(eventBus, null, pluginExecution, internalCallContextFactory, subscriptionInternalApi, accountApi, blockingStateDao, clock, checker, notificationQueueService, eventsStreamBuilder, entitlementUtils, securityApi); super(eventBus, null, pluginExecution, internalCallContextFactory, subscriptionInternalApi, accountApi, blockingStateDao, clock, checker, notificationQueueService, eventsStreamBuilder, entitlementUtils, securityApi);
this.internalCallContextFactory = internalCallContextFactory; this.internalCallContextFactory = internalCallContextFactory;
Expand All @@ -123,6 +128,7 @@ public DefaultEntitlementApi(final PersistentBus eventBus, final InternalCallCon
this.entitlementUtils = entitlementUtils; this.entitlementUtils = entitlementUtils;
this.pluginExecution = pluginExecution; this.pluginExecution = pluginExecution;
this.securityApi = securityApi; this.securityApi = securityApi;
this.catalogInternalApi = catalogInternalApi;
this.dateHelper = new EntitlementDateHelper(); this.dateHelper = new EntitlementDateHelper();
} }


Expand Down Expand Up @@ -387,7 +393,15 @@ private List<UUID> createBaseEntitlementsWithAddOns(final OperationType operatio
public List<UUID> doCall(final EntitlementApi entitlementApi, final EntitlementContext updatedPluginContext) throws EntitlementApiException { public List<UUID> doCall(final EntitlementApi entitlementApi, final EntitlementContext updatedPluginContext) throws EntitlementApiException {
final InternalCallContext contextWithValidAccountRecordId = internalCallContextFactory.createInternalCallContext(accountId, callContext); final InternalCallContext contextWithValidAccountRecordId = internalCallContextFactory.createInternalCallContext(accountId, callContext);


final Catalog catalog;
try {
catalog = catalogInternalApi.getFullCatalog(true, true, contextWithValidAccountRecordId);
} catch (final CatalogApiException e) {
throw new EntitlementApiException(e);
}

final Map<UUID, Optional<EventsStream>> eventsStreamForBaseSubscriptionPerBundle = new HashMap<UUID, Optional<EventsStream>>(); final Map<UUID, Optional<EventsStream>> eventsStreamForBaseSubscriptionPerBundle = new HashMap<UUID, Optional<EventsStream>>();
final Map<String, Optional<UUID>> bundleKeyToIdMapping = new HashMap<String, Optional<UUID>>();
final Iterable<BaseEntitlementWithAddOnsSpecifier> baseEntitlementWithAddOnsSpecifiersAfterPlugins = updatedPluginContext.getBaseEntitlementWithAddOnsSpecifiers(); final Iterable<BaseEntitlementWithAddOnsSpecifier> baseEntitlementWithAddOnsSpecifiersAfterPlugins = updatedPluginContext.getBaseEntitlementWithAddOnsSpecifiers();
final Collection<SubscriptionBaseWithAddOnsSpecifier> subscriptionBaseWithAddOnsSpecifiers = new LinkedList<SubscriptionBaseWithAddOnsSpecifier>(); final Collection<SubscriptionBaseWithAddOnsSpecifier> subscriptionBaseWithAddOnsSpecifiers = new LinkedList<SubscriptionBaseWithAddOnsSpecifier>();
DateTime upTo = null; DateTime upTo = null;
Expand All @@ -399,7 +413,13 @@ public List<UUID> doCall(final EntitlementApi entitlementApi, final EntitlementC
upTo = upTo == null || upTo.compareTo(entitlementRequestedDate) < 0 ? entitlementRequestedDate : upTo; upTo = upTo == null || upTo.compareTo(entitlementRequestedDate) < 0 ? entitlementRequestedDate : upTo;


// Verify if the operation is valid for that bundle // Verify if the operation is valid for that bundle
preCheckAddEntitlement(baseEntitlementWithAddOnsSpecifier, entitlementRequestedDate, eventsStreamForBaseSubscriptionPerBundle, callContext, contextWithValidAccountRecordId); preCheckAddEntitlement(baseEntitlementWithAddOnsSpecifier,
entitlementRequestedDate,
eventsStreamForBaseSubscriptionPerBundle,
bundleKeyToIdMapping,
catalog,
callContext,
contextWithValidAccountRecordId);


final SubscriptionBaseWithAddOnsSpecifier subscriptionBaseWithAddOnsSpecifier = new SubscriptionBaseWithAddOnsSpecifier(baseEntitlementWithAddOnsSpecifier.getBundleId(), final SubscriptionBaseWithAddOnsSpecifier subscriptionBaseWithAddOnsSpecifier = new SubscriptionBaseWithAddOnsSpecifier(baseEntitlementWithAddOnsSpecifier.getBundleId(),
baseEntitlementWithAddOnsSpecifier.getExternalKey(), baseEntitlementWithAddOnsSpecifier.getExternalKey(),
Expand Down Expand Up @@ -444,43 +464,71 @@ private BaseEntitlementWithAddOnsSpecifier getFirstBaseEntitlementWithAddOnsSpec
private void preCheckAddEntitlement(final BaseEntitlementWithAddOnsSpecifier baseEntitlementWithAddOnsSpecifier, private void preCheckAddEntitlement(final BaseEntitlementWithAddOnsSpecifier baseEntitlementWithAddOnsSpecifier,
final DateTime entitlementRequestedDate, final DateTime entitlementRequestedDate,
final Map<UUID, Optional<EventsStream>> eventsStreamForBaseSubscriptionPerBundle, final Map<UUID, Optional<EventsStream>> eventsStreamForBaseSubscriptionPerBundle,
final Map<String, Optional<UUID>> bundleKeyToIdMapping,
final Catalog catalog,
final TenantContext callContext, final TenantContext callContext,
final InternalCallContext contextWithValidAccountRecordId) throws EntitlementApiException { final InternalCallContext contextWithValidAccountRecordId) throws EntitlementApiException {
// TODO In the addEntitlement codepath, bundleId is always set. But technically an existing bundle could be specified by externalKey in // In the addEntitlement codepath, bundleId is always set. But, technically, an existing bundle could be specified by externalKey in
// the createBaseEntitlementsWithAddOns codepath. In that case, we should also check if that bundle is blocked (expensive though, especially // the createBaseEntitlementsWithAddOns codepath. In that case, we should also check if that bundle is blocked.
// since bundles are pulled again below in subscriptions) UUID bundleId = baseEntitlementWithAddOnsSpecifier.getBundleId();
if (baseEntitlementWithAddOnsSpecifier.getBundleId() != null) { if (bundleId == null && baseEntitlementWithAddOnsSpecifier.getExternalKey() != null) {
if (eventsStreamForBaseSubscriptionPerBundle.get(baseEntitlementWithAddOnsSpecifier.getBundleId()) == null) { populateBundleKeyToIdMappingCache(baseEntitlementWithAddOnsSpecifier, bundleKeyToIdMapping, catalog, contextWithValidAccountRecordId);
final List<SubscriptionBase> subscriptionsByBundle;
try { final Optional<UUID> bundleIdForKey = bundleKeyToIdMapping.get(baseEntitlementWithAddOnsSpecifier.getExternalKey());
subscriptionsByBundle = subscriptionBaseInternalApi.getSubscriptionsForBundle(baseEntitlementWithAddOnsSpecifier.getBundleId(), null, contextWithValidAccountRecordId); if (bundleIdForKey.isPresent()) {
bundleId = bundleIdForKey.get();
}
}


if (subscriptionsByBundle == null || subscriptionsByBundle.isEmpty()) { if (bundleId == null) {
throw new EntitlementApiException(ErrorCode.SUB_NO_ACTIVE_SUBSCRIPTIONS, baseEntitlementWithAddOnsSpecifier.getBundleId()); return;
} }
} catch (final SubscriptionBaseApiException e) {
throw new EntitlementApiException(e); populateEventsStreamForBaseSubscriptionPerBundleCache(bundleId, eventsStreamForBaseSubscriptionPerBundle, callContext, contextWithValidAccountRecordId);
}
final Optional<EventsStream> eventsStreamForBaseSubscription = eventsStreamForBaseSubscriptionPerBundle.get(bundleId);
if (eventsStreamForBaseSubscription.isPresent()) {
preCheckAddEntitlement(bundleId, entitlementRequestedDate, baseEntitlementWithAddOnsSpecifier, eventsStreamForBaseSubscription.get());
}
}

private void populateBundleKeyToIdMappingCache(final BaseEntitlementWithAddOnsSpecifier baseEntitlementWithAddOnsSpecifier, final Map<String, Optional<UUID>> bundleKeyToIdMapping, final Catalog catalog, final InternalCallContext contextWithValidAccountRecordId) throws EntitlementApiException {
if (bundleKeyToIdMapping.get(baseEntitlementWithAddOnsSpecifier.getExternalKey()) == null) {
final SubscriptionBaseBundle bundle = subscriptionBaseInternalApi.getActiveBundleForKey(baseEntitlementWithAddOnsSpecifier.getExternalKey(), catalog, contextWithValidAccountRecordId);
if (bundle != null) {
bundleKeyToIdMapping.put(baseEntitlementWithAddOnsSpecifier.getExternalKey(), Optional.<UUID>of(bundle.getId()));
} else {
bundleKeyToIdMapping.put(baseEntitlementWithAddOnsSpecifier.getExternalKey(), Optional.<UUID>absent());
}
}
}

private void populateEventsStreamForBaseSubscriptionPerBundleCache(final UUID bundleId, final Map<UUID, Optional<EventsStream>> eventsStreamForBaseSubscriptionPerBundle, final TenantContext callContext, final InternalCallContext contextWithValidAccountRecordId) throws EntitlementApiException {
if (eventsStreamForBaseSubscriptionPerBundle.get(bundleId) == null) {
final List<SubscriptionBase> subscriptionsByBundle;
try {
subscriptionsByBundle = subscriptionBaseInternalApi.getSubscriptionsForBundle(bundleId, null, contextWithValidAccountRecordId);


final boolean isStandalone = Iterables.any(subscriptionsByBundle, if (subscriptionsByBundle == null || subscriptionsByBundle.isEmpty()) {
new Predicate<SubscriptionBase>() { throw new EntitlementApiException(ErrorCode.SUB_NO_ACTIVE_SUBSCRIPTIONS, bundleId);
@Override
public boolean apply(final SubscriptionBase input) {
return ProductCategory.STANDALONE.equals(input.getCategory());
}
});

if (!isStandalone) {
final EventsStream eventsStreamForBaseSubscription = eventsStreamBuilder.buildForBaseSubscription(baseEntitlementWithAddOnsSpecifier.getBundleId(), callContext);
eventsStreamForBaseSubscriptionPerBundle.put(baseEntitlementWithAddOnsSpecifier.getBundleId(), Optional.<EventsStream>of(eventsStreamForBaseSubscription));
} else {
eventsStreamForBaseSubscriptionPerBundle.put(baseEntitlementWithAddOnsSpecifier.getBundleId(), Optional.<EventsStream>absent());
} }
} catch (final SubscriptionBaseApiException e) {
throw new EntitlementApiException(e);
} }


final Optional<EventsStream> eventsStreamForBaseSubscription = eventsStreamForBaseSubscriptionPerBundle.get(baseEntitlementWithAddOnsSpecifier.getBundleId()); final boolean isStandalone = Iterables.any(subscriptionsByBundle,
if (eventsStreamForBaseSubscription.isPresent()) { new Predicate<SubscriptionBase>() {
preCheckAddEntitlement(baseEntitlementWithAddOnsSpecifier.getBundleId(), entitlementRequestedDate, baseEntitlementWithAddOnsSpecifier, eventsStreamForBaseSubscription.get()); @Override
public boolean apply(final SubscriptionBase input) {
return ProductCategory.STANDALONE.equals(input.getCategory());
}
});

if (!isStandalone) {
final EventsStream eventsStreamForBaseSubscription = eventsStreamBuilder.buildForBaseSubscription(bundleId, callContext);
eventsStreamForBaseSubscriptionPerBundle.put(bundleId, Optional.<EventsStream>of(eventsStreamForBaseSubscription));
} else {
eventsStreamForBaseSubscriptionPerBundle.put(bundleId, Optional.<EventsStream>absent());
} }
} }
} }
Expand All @@ -496,6 +544,8 @@ private void preCheckAddEntitlement(final UUID bundleId, final DateTime entitlem
// Check the base entitlement state is not blocked // Check the base entitlement state is not blocked
if (eventsStreamForBaseSubscription.isBlockChange(entitlementRequestedDate)) { if (eventsStreamForBaseSubscription.isBlockChange(entitlementRequestedDate)) {
throw new EntitlementApiException(new BlockingApiException(ErrorCode.BLOCK_BLOCKED_ACTION, BlockingChecker.ACTION_CHANGE, BlockingChecker.TYPE_SUBSCRIPTION, eventsStreamForBaseSubscription.getEntitlementId().toString())); throw new EntitlementApiException(new BlockingApiException(ErrorCode.BLOCK_BLOCKED_ACTION, BlockingChecker.ACTION_CHANGE, BlockingChecker.TYPE_SUBSCRIPTION, eventsStreamForBaseSubscription.getEntitlementId().toString()));
} else if (eventsStreamForBaseSubscription.isBlockEntitlement(entitlementRequestedDate)) {
throw new EntitlementApiException(new BlockingApiException(ErrorCode.BLOCK_BLOCKED_ACTION, BlockingChecker.ACTION_ENTITLEMENT, BlockingChecker.TYPE_SUBSCRIPTION, eventsStreamForBaseSubscription.getEntitlementId().toString()));
} }
} }


Expand Down
Expand Up @@ -225,6 +225,13 @@ public boolean isBlockChange(final DateTime effectiveDate) {
return aggregator.isBlockChange(); return aggregator.isBlockChange();
} }


@Override
public boolean isBlockEntitlement(final DateTime effectiveDate) {
Preconditions.checkState(effectiveDate != null);
final BlockingAggregator aggregator = getBlockingAggregator(effectiveDate);
return aggregator.isBlockEntitlement();
}

@Override @Override
public int getDefaultBillCycleDayLocal() { public int getDefaultBillCycleDayLocal() {
return defaultBillCycleDayLocal; return defaultBillCycleDayLocal;
Expand Down
Expand Up @@ -295,8 +295,7 @@ public List<SubscriptionBaseWithAddOns> createBaseSubscriptionsWithAddOns(final
} }
} else if (subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey() != null && } else if (subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey() != null &&
baseOrFirstStandalonePlanSpecifier == null) { // Skip the expensive checks if we are about to create the bundle (validation will be done in SubscriptionDao#createSubscriptionBundle) baseOrFirstStandalonePlanSpecifier == 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 = getActiveBundleForKey(subscriptionBaseWithAddOnsSpecifier.getBundleExternalKey(), catalog, context);
final SubscriptionBaseBundle tmp = getActiveBundleForKeyNotException(existingBundles, dao, clock, catalog, context);
if (tmp == null) { if (tmp == 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)) { } else if (!tmp.getAccountId().equals(accountId)) {
Expand Down Expand Up @@ -474,7 +473,9 @@ public Iterable<UUID> getNonAOSubscriptionIdsForKey(final String bundleKey, fina
return dao.getNonAOSubscriptionIdsForKey(bundleKey, context); return dao.getNonAOSubscriptionIdsForKey(bundleKey, context);
} }


public static SubscriptionBaseBundle getActiveBundleForKeyNotException(final Iterable<SubscriptionBaseBundle> existingBundles, final SubscriptionDao dao, final Clock clock, final Catalog catalog, final InternalTenantContext context) { @Override
public SubscriptionBaseBundle getActiveBundleForKey(final String bundleKey, final Catalog catalog, final InternalTenantContext context) {
final List<SubscriptionBaseBundle> existingBundles = dao.getSubscriptionBundlesForKey(bundleKey, context);
for (final SubscriptionBaseBundle cur : existingBundles) { for (final SubscriptionBaseBundle cur : existingBundles) {
final List<SubscriptionBase> subscriptions; final List<SubscriptionBase> subscriptions;
try { try {
Expand Down

0 comments on commit b41029f

Please sign in to comment.