Skip to content

Commit

Permalink
subscription: Optimize code to only fetch catalog once per api call.
Browse files Browse the repository at this point in the history 
The rationale behind this change is that we want to optimize for catalog plugins (and avoid querying the plugin more than necessary).
This will also remove fetching catalog from SubscriptionDao, from within transactions (yuck...).
  • Loading branch information
@sbrossie
sbrossie committed May 3, 2017
1 parent 05d82e5 commit a361ac7
Show file tree
Hide file tree
Showing 15 changed files with 440 additions and 358 deletions.
23 changes: 14 additions & 9 deletions subscription/src/main/java/org/killbill/billing/subscription/alignment/PlanAligner.java
View file
Expand Up @@ -53,11 +53,9 @@
*/
public class PlanAligner extends BaseAligner {

private final CatalogInternalApi catalogInternalApi;

@Inject
public PlanAligner(final CatalogInternalApi catalogInternalApi) {
this.catalogInternalApi = catalogInternalApi;
public PlanAligner() {
}

private enum WhichPhase {
Expand All @@ -84,12 +82,14 @@ public TimedPhase[] getCurrentAndNextTimedPhaseOnCreate(final DateTime alignStar
@Nullable final PhaseType initialPhase,
final String priceList,
final DateTime effectiveDate,
final Catalog catalog,
final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
final List<TimedPhase> timedPhases = getTimedPhaseOnCreate(alignStartDate,
bundleStartDate,
plan,
initialPhase,
effectiveDate,
catalog,
context);
final TimedPhase[] result = new TimedPhase[2];
result[0] = getTimedPhase(timedPhases, effectiveDate, WhichPhase.CURRENT);
Expand All @@ -113,9 +113,10 @@ public TimedPhase getCurrentTimedPhaseOnChange(final DefaultSubscriptionBase sub
final Plan plan,
final DateTime effectiveDate,
final PhaseType newPlanInitialPhaseType,
final Catalog catalog,
final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {

return getTimedPhaseOnChange(subscription, plan, effectiveDate, newPlanInitialPhaseType, WhichPhase.CURRENT, context);
return getTimedPhaseOnChange(subscription, plan, effectiveDate, newPlanInitialPhaseType, WhichPhase.CURRENT, catalog, context);
}

/**
Expand All @@ -134,8 +135,9 @@ public TimedPhase getNextTimedPhaseOnChange(final DefaultSubscriptionBase subscr
final Plan plan,
final DateTime effectiveDate,
final PhaseType newPlanInitialPhaseType,
final Catalog catalog,
final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
return getTimedPhaseOnChange(subscription, plan, effectiveDate, newPlanInitialPhaseType, WhichPhase.NEXT, context);
return getTimedPhaseOnChange(subscription, plan, effectiveDate, newPlanInitialPhaseType, WhichPhase.NEXT, catalog, context);
}

/**
Expand All @@ -144,7 +146,7 @@ public TimedPhase getNextTimedPhaseOnChange(final DefaultSubscriptionBase subscr
* @param subscription the subscription for which we need to compute the next Phase event
* @return the next phase
*/
public TimedPhase getNextTimedPhase(final DefaultSubscriptionBase subscription, final DateTime effectiveDate, final InternalTenantContext context) {
public TimedPhase getNextTimedPhase(final DefaultSubscriptionBase subscription, final DateTime effectiveDate, final Catalog catalog, final InternalTenantContext context) {

try {
final SubscriptionBaseTransition pendingOrLastPlanTransition;
Expand All @@ -163,6 +165,7 @@ public TimedPhase getNextTimedPhase(final DefaultSubscriptionBase subscription,
pendingOrLastPlanTransition.getNextPlan(),
pendingOrLastPlanTransition.getNextPhase().getPhaseType(),
effectiveDate,
catalog,
context);
return getTimedPhase(timedPhases, effectiveDate, WhichPhase.NEXT);
case CHANGE:
Expand All @@ -176,6 +179,7 @@ public TimedPhase getNextTimedPhase(final DefaultSubscriptionBase subscription,
subscription.getAllTransitions().get(0).getNextPhase().getPhaseType(),
null,
WhichPhase.NEXT,
catalog,
context);
default:
throw new SubscriptionBaseError(String.format("Unexpected initial transition %s for current plan %s on subscription %s",
Expand All @@ -191,10 +195,9 @@ private List<TimedPhase> getTimedPhaseOnCreate(final DateTime subscriptionStartD
final Plan plan,
@Nullable final PhaseType initialPhase,
final DateTime effectiveDate,
final Catalog catalog,
final InternalTenantContext context)
throws CatalogApiException, SubscriptionBaseApiException {
final Catalog catalog = catalogInternalApi.getFullCatalog(true, true, context);

final PlanSpecifier planSpecifier = new PlanSpecifier(plan.getName());

final DateTime planStartDate;
Expand All @@ -218,6 +221,7 @@ private TimedPhase getTimedPhaseOnChange(final DefaultSubscriptionBase subscript
final DateTime effectiveDate,
final PhaseType newPlanInitialPhaseType,
final WhichPhase which,
final Catalog catalog,
final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
final SubscriptionBaseTransition pendingOrLastPlanTransition;
if (subscription.getState() == EntitlementState.PENDING) {
Expand All @@ -236,6 +240,7 @@ private TimedPhase getTimedPhaseOnChange(final DefaultSubscriptionBase subscript
subscription.getAllTransitions().get(0).getNextPhase().getPhaseType(),
newPlanInitialPhaseType,
which,
catalog,
context);
}

Expand All @@ -249,8 +254,8 @@ private TimedPhase getTimedPhaseOnChange(final DateTime subscriptionStartDate,
final PhaseType originalInitialPhase,
@Nullable final PhaseType newPlanInitialPhaseType,
final WhichPhase which,
final Catalog catalog,
final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
final Catalog catalog = catalogInternalApi.getFullCatalog(true, true, context);
final PlanPhaseSpecifier fromPlanPhaseSpecifier = new PlanPhaseSpecifier(currentPlan.getName(),
currentPhase.getPhaseType());

Expand Down
10 changes: 4 additions & 6 deletions subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionApiBase.java
View file
Expand Up @@ -20,6 +20,7 @@
import java.util.List;

import org.killbill.billing.callcontext.InternalTenantContext;
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.subscription.api.user.DefaultSubscriptionBase;
Expand All @@ -37,15 +38,12 @@ public class SubscriptionApiBase {

protected final SubscriptionBaseApiService apiService;
protected final Clock clock;
protected final CatalogInternalApi catalogInternalApi;

public SubscriptionApiBase(final SubscriptionDao dao, final SubscriptionBaseApiService apiService, final Clock clock, final CatalogInternalApi catalogInternalApi) {
public SubscriptionApiBase(final SubscriptionDao dao, final SubscriptionBaseApiService apiService, final Clock clock) {
this.dao = dao;
this.apiService = apiService;
this.clock = clock;
this.catalogInternalApi = catalogInternalApi;
}

protected List<SubscriptionBase> createSubscriptionsForApiUse(final List<SubscriptionBase> internalSubscriptions) {
return new ArrayList<SubscriptionBase>(Collections2.transform(internalSubscriptions, new Function<SubscriptionBase, SubscriptionBase>() {
@Override
Expand All @@ -59,10 +57,10 @@ protected DefaultSubscriptionBase createSubscriptionForApiUse(final Subscription
return new DefaultSubscriptionBase((DefaultSubscriptionBase) internalSubscription, apiService, clock);
}

protected DefaultSubscriptionBase createSubscriptionForApiUse(SubscriptionBuilder builder, List<SubscriptionBaseEvent> events, final InternalTenantContext context) throws CatalogApiException {
protected DefaultSubscriptionBase createSubscriptionForApiUse(SubscriptionBuilder builder, List<SubscriptionBaseEvent> events, final Catalog catalog, final InternalTenantContext context) throws CatalogApiException {
final DefaultSubscriptionBase subscription = new DefaultSubscriptionBase(builder, apiService, clock);
if (events.size() > 0) {
subscription.rebuildTransitions(events, catalogInternalApi.getFullCatalog(true, true, context));
subscription.rebuildTransitions(events, catalog);
}
return subscription;
}
Expand Down
36 changes: 25 additions & 11 deletions ...ption/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
View file
Expand Up @@ -25,6 +25,7 @@
import org.killbill.billing.callcontext.InternalCallContext;
import org.killbill.billing.callcontext.InternalTenantContext;
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.PhaseType;
import org.killbill.billing.catalog.api.Plan;
Expand All @@ -42,11 +43,11 @@
public interface SubscriptionBaseApiService {

public DefaultSubscriptionBase createPlan(SubscriptionBuilder builder, Plan plan, PhaseType initialPhase,
String realPriceList, DateTime effectiveDate, DateTime processedDate,
CallContext context)
String realPriceList, DateTime effectiveDate,
Catalog catalog, CallContext context)
throws SubscriptionBaseApiException;

public List<SubscriptionBaseWithAddOns> createPlansWithAddOns(UUID accountId, Iterable<SubscriptionAndAddOnsSpecifier> subscriptionsAndAddOns, CallContext context)
public List<SubscriptionBaseWithAddOns> createPlansWithAddOns(UUID accountId, Iterable<SubscriptionAndAddOnsSpecifier> subscriptionsAndAddOns, Catalog catalog, CallContext context)
throws SubscriptionBaseApiException;

public boolean cancel(DefaultSubscriptionBase subscription, CallContext context)
Expand All @@ -58,7 +59,7 @@ public boolean cancelWithRequestedDate(DefaultSubscriptionBase subscription, Dat
public boolean cancelWithPolicy(DefaultSubscriptionBase subscription, BillingActionPolicy policy, int accountBillCycleDayLocal, CallContext context)
throws SubscriptionBaseApiException;

public boolean cancelWithPolicyNoValidation(Iterable<DefaultSubscriptionBase> subscriptions, BillingActionPolicy policy, int accountBillCycleDayLocal, InternalCallContext context)
public boolean cancelWithPolicyNoValidationAndCatalog(Iterable<DefaultSubscriptionBase> subscriptions, BillingActionPolicy policy, int accountBillCycleDayLocal, Catalog catalog, InternalCallContext context)
throws SubscriptionBaseApiException;

public boolean uncancel(DefaultSubscriptionBase subscription, CallContext context)
Expand All @@ -81,25 +82,38 @@ public DateTime changePlanWithPolicy(DefaultSubscriptionBase subscription, PlanS
List<PlanPhasePriceOverride> overrides, BillingActionPolicy policy, CallContext context)
throws SubscriptionBaseApiException;

public int handleBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException;
public int handleBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, Catalog fullCatalog, final CallContext context) throws CatalogApiException;

public PlanChangeResult getPlanChangeResult(final DefaultSubscriptionBase subscription, PlanSpecifier spec, final DateTime effectiveDate, TenantContext context) throws SubscriptionBaseApiException;

//
// Lower level APIs for dryRun functionality
//
public List<SubscriptionBaseEvent> getEventsOnCreation(UUID bundleId, UUID subscriptionId, DateTime alignStartDate, DateTime bundleStartDate,

public List<SubscriptionBaseEvent> getEventsOnCreation(UUID subscriptionId, DateTime alignStartDate, DateTime bundleStartDate,
Plan plan, PhaseType initialPhase,
String realPriceList, DateTime effectiveDate, DateTime processedDate,
String realPriceList, DateTime effectiveDate,
Catalog fullCatalog,
InternalTenantContext context)
throws CatalogApiException, SubscriptionBaseApiException;

/*
public List<SubscriptionBaseEvent> getEventsOnChangePlan(final DefaultSubscriptionBase subscription, final Plan newPlan,
final String newPriceList, final DateTime effectiveDate,
final boolean addCancellationAddOnForEventsIfRequired, final InternalTenantContext internalTenantContext) throws CatalogApiException, SubscriptionBaseApiException {
*/

public List<SubscriptionBaseEvent> getEventsOnChangePlan(DefaultSubscriptionBase subscription, Plan newPlan,
String newPriceList, DateTime effectiveDate, DateTime processedDate,
boolean addCancellationAddOnForEventsIfRequired, InternalTenantContext context)
String newPriceList, DateTime effectiveDate,
boolean addCancellationAddOnForEventsIfRequired,
final Catalog fullCatalog,
InternalTenantContext context)
throws CatalogApiException, SubscriptionBaseApiException;

public List<SubscriptionBaseEvent> getEventsOnCancelPlan(final DefaultSubscriptionBase subscription,
final DateTime effectiveDate, final DateTime processedDate,
final boolean addCancellationAddOnForEventsIfRequired, final InternalTenantContext internalTenantContext) throws CatalogApiException;
final DateTime effectiveDate,
final boolean addCancellationAddOnForEventsIfRequired,
final Catalog fullCatalog,
final InternalTenantContext internalTenantContext) throws CatalogApiException;
}

0 comments on commit a361ac7

Please sign in to comment.