killbill-uncached

Merge pull request #985 from killbill/fix-for-784 Catalog

5/25/2018 2:12:47 PM

Details

diff --git a/catalog/src/main/java/org/killbill/billing/catalog/rules/DefaultPlanRules.java b/catalog/src/main/java/org/killbill/billing/catalog/rules/DefaultPlanRules.java
index 827aee9..6085476 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/rules/DefaultPlanRules.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/rules/DefaultPlanRules.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
- * Ning 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
  * License.  You may obtain a copy of the License at:
  *
@@ -146,14 +148,14 @@ public class DefaultPlanRules extends ValidatingConfig<StandaloneCatalog> implem
         return new PlanChangeResult(toPriceList, policy, alignment);
     }
 
-    public PlanAlignmentChange getPlanChangeAlignment(final PlanPhaseSpecifier from,
-                                                      final PlanSpecifier to, final StaticCatalog catalog) throws CatalogApiException {
+    private PlanAlignmentChange getPlanChangeAlignment(final PlanPhaseSpecifier from,
+                                                       final PlanSpecifier to, final StaticCatalog catalog) throws CatalogApiException {
         final PlanAlignmentChange result = DefaultCaseChange.getResult(changeAlignmentCase, from, to, catalog);
         return (result != null) ? result : PlanAlignmentChange.START_OF_BUNDLE;
     }
 
-    public BillingActionPolicy getPlanChangePolicy(final PlanPhaseSpecifier from,
-                                                   final PlanSpecifier to, final StaticCatalog catalog) throws CatalogApiException {
+    private BillingActionPolicy getPlanChangePolicy(final PlanPhaseSpecifier from,
+                                                    final PlanSpecifier to, final StaticCatalog catalog) throws CatalogApiException {
         final BillingActionPolicy result = DefaultCaseChange.getResult(changeCase, from, to, catalog);
         return (result != null) ? result : BillingActionPolicy.END_OF_TERM;
     }
@@ -167,7 +169,6 @@ public class DefaultPlanRules extends ValidatingConfig<StandaloneCatalog> implem
         return result;
     }
 
-
     @Override
     public ValidationErrors validate(final StandaloneCatalog catalog, final ValidationErrors errors) {
         //
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/StandaloneCatalog.java b/catalog/src/main/java/org/killbill/billing/catalog/StandaloneCatalog.java
index ed0e394..9d454b0 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/StandaloneCatalog.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/StandaloneCatalog.java
@@ -41,7 +41,6 @@ import org.killbill.billing.catalog.api.CatalogApiException;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.catalog.api.Listing;
 import org.killbill.billing.catalog.api.Plan;
-import org.killbill.billing.catalog.api.PlanAlignmentChange;
 import org.killbill.billing.catalog.api.PlanAlignmentCreate;
 import org.killbill.billing.catalog.api.PlanChangeResult;
 import org.killbill.billing.catalog.api.PlanPhase;
@@ -259,11 +258,6 @@ public class StandaloneCatalog extends ValidatingConfig<StandaloneCatalog> imple
     }
 
     @Override
-    public PlanAlignmentChange planChangeAlignment(final PlanPhaseSpecifier from, final PlanSpecifier to) throws CatalogApiException {
-        return planRules.getPlanChangeAlignment(from, to, this);
-    }
-
-    @Override
     public BillingActionPolicy planCancelPolicy(final PlanPhaseSpecifier planPhase) throws CatalogApiException {
         return planRules.getPlanCancelPolicy(planPhase, this);
     }
diff --git a/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java b/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
index 6e171ad..6402a57 100644
--- a/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
+++ b/catalog/src/main/java/org/killbill/billing/catalog/VersionedCatalog.java
@@ -319,15 +319,6 @@ public class VersionedCatalog extends ValidatingConfig<VersionedCatalog> impleme
     }
 
     @Override
-    public PlanAlignmentChange planChangeAlignment(final PlanPhaseSpecifier from,
-                                                   final PlanSpecifier to,
-                                                   final DateTime requestedDate,
-                                                   final DateTime subscriptionStartDate) throws CatalogApiException {
-        final StaticCatalog staticCatalog = getStaticCatalog(from, requestedDate, subscriptionStartDate);
-        return staticCatalog.planChangeAlignment(from, to);
-    }
-
-    @Override
     public PlanAlignmentCreate planCreateAlignment(final PlanSpecifier specifier,
                                                    final DateTime requestedDate,
                                                    final DateTime subscriptionStartDate) throws CatalogApiException {
@@ -349,7 +340,8 @@ public class VersionedCatalog extends ValidatingConfig<VersionedCatalog> impleme
                                        final DateTime requestedDate,
                                        final DateTime subscriptionStartDate)
             throws CatalogApiException {
-        final StaticCatalog staticCatalog = getStaticCatalog(from, requestedDate, subscriptionStartDate);
+        // Use the "to" specifier, to make sure the new plan always exists
+        final StaticCatalog staticCatalog = getStaticCatalog(to, requestedDate, subscriptionStartDate);
         return staticCatalog.planChange(from, to);
     }
 
@@ -474,12 +466,6 @@ public class VersionedCatalog extends ValidatingConfig<VersionedCatalog> impleme
     }
 
     @Override
-    public PlanAlignmentChange planChangeAlignment(final PlanPhaseSpecifier from,
-                                                   final PlanSpecifier to) throws CatalogApiException {
-        return versionForDate(clock.getUTCNow()).planChangeAlignment(from, to);
-    }
-
-    @Override
     public List<Listing> getAvailableAddOnListings(final String baseProductName, @Nullable final String priceListName) throws CatalogApiException {
         return versionForDate(clock.getUTCNow()).getAvailableAddOnListings(baseProductName, priceListName);
     }
diff --git a/catalog/src/test/java/org/killbill/billing/catalog/MockCatalog.java b/catalog/src/test/java/org/killbill/billing/catalog/MockCatalog.java
index 5b3f1b2..dbdfce6 100644
--- a/catalog/src/test/java/org/killbill/billing/catalog/MockCatalog.java
+++ b/catalog/src/test/java/org/killbill/billing/catalog/MockCatalog.java
@@ -29,7 +29,6 @@ import org.killbill.billing.catalog.api.Catalog;
 import org.killbill.billing.catalog.api.CatalogApiException;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.catalog.api.Plan;
-import org.killbill.billing.catalog.api.PlanAlignmentChange;
 import org.killbill.billing.catalog.api.PlanAlignmentCreate;
 import org.killbill.billing.catalog.api.PlanChangeResult;
 import org.killbill.billing.catalog.api.PlanPhase;
@@ -168,18 +167,6 @@ public class MockCatalog extends StandaloneCatalog implements Catalog {
     }
 
     @Override
-    public PlanAlignmentChange planChangeAlignment(final PlanPhaseSpecifier from, final PlanSpecifier to, final DateTime requestedDate, final DateTime subscriptionStartDate)
-            throws CatalogApiException {
-        return planChangeAlignment(from, to);
-    }
-
-    @Override
-    public PlanAlignmentChange planChangeAlignment(final PlanPhaseSpecifier from, final PlanSpecifier to)
-            throws CatalogApiException {
-        return super.planChangeAlignment(from, to);
-    }
-
-    @Override
     public BillingActionPolicy planCancelPolicy(final PlanPhaseSpecifier planPhase) throws CatalogApiException {
         return super.planCancelPolicy(planPhase);
     }
diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultBillingEvent.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultBillingEvent.java
index 5c66e3c..c08a7be 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultBillingEvent.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/DefaultBillingEvent.java
@@ -31,7 +31,7 @@ import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.catalog.api.Plan;
 import org.killbill.billing.catalog.api.PlanPhase;
 import org.killbill.billing.catalog.api.Usage;
-import org.killbill.billing.events.EffectiveSubscriptionInternalEvent;
+import org.killbill.billing.events.SubscriptionInternalEvent;
 import org.killbill.billing.junction.BillingEvent;
 import org.killbill.billing.subscription.api.SubscriptionBase;
 import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
@@ -59,35 +59,47 @@ public class DefaultBillingEvent implements BillingEvent {
     private final boolean isDisableEvent;
     private final PlanPhase nextPlanPhase;
 
-    public DefaultBillingEvent(final EffectiveSubscriptionInternalEvent transition, final SubscriptionBase subscription, final int billCycleDayLocal, final Currency currency, final Catalog catalog) throws CatalogApiException {
-
-        this.catalog = catalog;
-
+    public DefaultBillingEvent(final SubscriptionInternalEvent transition,
+                               final SubscriptionBase subscription,
+                               final int billCycleDayLocal,
+                               final Currency currency,
+                               final Catalog catalog) throws CatalogApiException {
         final boolean isActive = transition.getTransitionType() != SubscriptionBaseTransitionType.CANCEL;
 
-        this.billCycleDayLocal = billCycleDayLocal;
-        this.subscription = subscription;
-        this.effectiveDate = transition.getEffectiveTransitionTime();
-        final String planPhaseName = isActive ? transition.getNextPhase() : transition.getPreviousPhase();
-        this.planPhase = (planPhaseName != null) ? catalog.findPhase(planPhaseName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+        if (isActive) {
+            final String planName = transition.getNextPlan();
+            this.plan = (planName != null) ? catalog.findPlan(planName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
 
-        final String planName = isActive ? transition.getNextPlan() : transition.getPreviousPlan();
-        this.plan = (planName != null) ? catalog.findPlan(planName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+            final String planPhaseName = transition.getNextPhase();
+            this.planPhase = (planPhaseName != null && this.plan != null) ? this.plan.findPhase(planPhaseName) : null;
+            this.nextPlanPhase = this.planPhase;
 
-        final String nextPhaseName = transition.getNextPhase();
-        this.nextPlanPhase = (nextPhaseName != null) ? catalog.findPhase(nextPhaseName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+            this.billingPeriod = getRecurringBillingPeriod(nextPlanPhase);
+        } else {
+            final String planName = transition.getPreviousPlan();
+            this.plan = (planName != null) ? catalog.findPlan(planName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+            final Plan prevPlan = this.plan;
 
-        final String prevPhaseName = transition.getPreviousPhase();
-        final PlanPhase prevPlanPhase = (prevPhaseName != null) ? catalog.findPhase(prevPhaseName, transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+            final String planPhaseName = transition.getPreviousPhase();
+            this.planPhase = (planPhaseName != null && this.plan != null) ? this.plan.findPhase(planPhaseName) : null;
+            this.nextPlanPhase = null;
 
-        this.fixedPrice = transition.getTransitionType() != SubscriptionBaseTransitionType.BCD_CHANGE ? getFixedPrice(nextPlanPhase, currency) : null;
+            final String prevPhaseName = transition.getPreviousPhase();
+            final PlanPhase prevPlanPhase = (prevPhaseName != null && prevPlan != null) ? prevPlan.findPhase(prevPhaseName) : null;
+            this.billingPeriod = getRecurringBillingPeriod(prevPlanPhase);
+        }
+
+        this.billCycleDayLocal = billCycleDayLocal;
+        this.catalog = catalog;
         this.currency = currency;
         this.description = transition.getTransitionType().toString();
-        this.billingPeriod = getRecurringBillingPeriod(isActive ? nextPlanPhase : prevPlanPhase);
-        this.type = transition.getTransitionType();
+        this.effectiveDate = transition.getEffectiveTransitionTime();
+        this.fixedPrice = transition.getTransitionType() != SubscriptionBaseTransitionType.BCD_CHANGE ? getFixedPrice(nextPlanPhase, currency) : null;
+        this.isDisableEvent = false;
+        this.subscription = subscription;
         this.totalOrdering = transition.getTotalOrdering();
+        this.type = transition.getTransitionType();
         this.usages = initializeUsage(isActive);
-        this.isDisableEvent = false;
     }
 
     public DefaultBillingEvent(final SubscriptionBase subscription, final DateTime effectiveDate, final boolean isActive,
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 73dc2cb..e033ad6 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
@@ -43,6 +43,7 @@ import org.killbill.billing.catalog.api.PlanPhase;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.entitlement.api.SubscriptionEventType;
 import org.killbill.billing.events.EffectiveSubscriptionInternalEvent;
+import org.killbill.billing.events.SubscriptionInternalEvent;
 import org.killbill.billing.invoice.api.DryRunArguments;
 import org.killbill.billing.junction.BillingEvent;
 import org.killbill.billing.junction.BillingEventSet;
@@ -239,18 +240,18 @@ public class DefaultInternalBillingApi implements BillingInternalApi {
         return BillCycleDayCalculator.calculateBcdForAlignment(bcdCache, subscription, baseSubscription, alignment, internalTenantContext, accountBillCycleDayLocal);
     }
 
-    private PlanPhaseSpecifier getPlanPhaseSpecifierFromTransition(final Catalog catalog, final EffectiveSubscriptionInternalEvent transition) throws CatalogApiException {
+    private PlanPhaseSpecifier getPlanPhaseSpecifierFromTransition(final Catalog catalog, final SubscriptionInternalEvent transition) throws CatalogApiException {
         final Plan prevPlan = (transition.getPreviousPlan() != null) ? catalog.findPlan(transition.getPreviousPlan(), transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
         final Plan nextPlan = (transition.getNextPlan() != null) ? catalog.findPlan(transition.getNextPlan(), transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
 
         final Plan plan = (transition.getTransitionType() != SubscriptionBaseTransitionType.CANCEL) ? nextPlan : prevPlan;
 
-        final PlanPhase prevPhase = (transition.getPreviousPhase() != null) ? catalog.findPhase(transition.getPreviousPhase(), transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
-        final PlanPhase nextPhase = (transition.getNextPhase() != null) ? catalog.findPhase(transition.getNextPhase(), transition.getEffectiveTransitionTime(), transition.getSubscriptionStartDate()) : null;
+        final PlanPhase prevPhase = prevPlan != null && transition.getPreviousPhase() != null ? prevPlan.findPhase(transition.getPreviousPhase()) : null;
+        final PlanPhase nextPhase = nextPlan != null && transition.getNextPhase() != null ? nextPlan.findPhase(transition.getNextPhase()) : null;
+
         final PlanPhase phase = (transition.getTransitionType() != SubscriptionBaseTransitionType.CANCEL) ? nextPhase : prevPhase;
 
         return new PlanPhaseSpecifier(plan.getName(), phase.getPhaseType());
-
     }
 
     private boolean is_AUTO_INVOICING_OFF(final List<Tag> tags) {

pom.xml 2(+1 -1)

diff --git a/pom.xml b/pom.xml
index fd1dd5a..4a54af7 100644
--- a/pom.xml
+++ b/pom.xml
@@ -21,7 +21,7 @@
     <parent>
         <artifactId>killbill-oss-parent</artifactId>
         <groupId>org.kill-bill.billing</groupId>
-        <version>0.141.69</version>
+        <version>0.141.70</version>
     </parent>
     <artifactId>killbill</artifactId>
     <version>0.19.16-SNAPSHOT</version>
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/alignment/PlanAligner.java b/subscription/src/main/java/org/killbill/billing/subscription/alignment/PlanAligner.java
index 35bd5e1..3eede41 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/alignment/PlanAligner.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/alignment/PlanAligner.java
@@ -271,7 +271,7 @@ public class PlanAligner extends BaseAligner {
         final PlanSpecifier toPlanSpecifier = new PlanSpecifier(nextPlan.getName());
         final PhaseType initialPhase;
         final DateTime planStartDate;
-        final PlanAlignmentChange alignment = catalog.planChangeAlignment(fromPlanPhaseSpecifier, toPlanSpecifier, catalogEffectiveDate, subscriptionStartDate);
+        final PlanAlignmentChange alignment = catalog.planChange(fromPlanPhaseSpecifier, toPlanSpecifier, catalogEffectiveDate, subscriptionStartDate).getAlignment();
         switch (alignment) {
             case START_OF_SUBSCRIPTION:
                 planStartDate = subscriptionStartDate;
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/timeline/DefaultSubscriptionBaseTimeline.java b/subscription/src/main/java/org/killbill/billing/subscription/api/timeline/DefaultSubscriptionBaseTimeline.java
index 9c677fd..8bb206e 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/timeline/DefaultSubscriptionBaseTimeline.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/timeline/DefaultSubscriptionBaseTimeline.java
@@ -65,7 +65,7 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
 
         final List<ExistingEvent> result = new LinkedList<SubscriptionBaseTimeline.ExistingEvent>();
 
-        String prevPlanName = null;
+        Plan prevPlan = null;
         String prevProductName = null;
         BillingPeriod prevBillingPeriod = null;
         String prevPriceListName = null;
@@ -84,7 +84,7 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
             BillingPeriod billingPeriod = null;
             String priceListName = null;
             PhaseType phaseType = null;
-            String planName = null;
+            Plan plan = null;
             String planPhaseName = null;
             Integer billCycleDayLocal = null;
 
@@ -93,9 +93,9 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
                 case PHASE:
                     final PhaseEvent phaseEV = (PhaseEvent) cur;
                     planPhaseName = phaseEV.getPhase();
-                    phaseType = catalog.findPhase(phaseEV.getPhase(), cur.getEffectiveDate(), startDate).getPhaseType();
                     // A PHASE event always occurs within the same plan (and is never the first event)
-                    planName = prevPlanName;
+                    phaseType = prevPlan != null ? prevPlan.findPhase(phaseEV.getPhase()).getPhaseType() : null;
+                    plan = prevPlan;
                     productName = prevProductName;
                     billingPeriod = getBillingPeriod(catalog, phaseEV.getPhase(), cur.getEffectiveDate(), startDate);
                     priceListName = prevPriceListName;
@@ -109,10 +109,9 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
                 case API_USER:
                     final ApiEvent userEV = (ApiEvent) cur;
                     apiType = userEV.getApiEventType();
-                    planName = userEV.getEventPlan();
                     planPhaseName = userEV.getEventPlanPhase();
-                    final Plan plan = (userEV.getEventPlan() != null) ? catalog.findPlan(userEV.getEventPlan(), cur.getEffectiveDate(), startDate) : null;
-                    phaseType = (userEV.getEventPlanPhase() != null) ? catalog.findPhase(userEV.getEventPlanPhase(), cur.getEffectiveDate(), startDate).getPhaseType() : prevPhaseType;
+                    plan = (userEV.getEventPlan() != null) ? catalog.findPlan(userEV.getEventPlan(), cur.getEffectiveDate(), startDate) : null;
+                    phaseType = (plan != null && userEV.getEventPlanPhase() != null) ? plan.findPhase(userEV.getEventPlanPhase()).getPhaseType() : prevPhaseType;
                     productName = (plan != null) ? plan.getProduct().getName() : prevProductName;
                     billingPeriod = (userEV.getEventPlanPhase() != null) ? getBillingPeriod(catalog, userEV.getEventPlanPhase(), cur.getEffectiveDate(), startDate) : prevBillingPeriod;
                     priceListName = (userEV.getPriceList() != null) ? userEV.getPriceList() : prevPriceListName;
@@ -121,10 +120,10 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
 
             final SubscriptionBaseTransitionType transitionType = SubscriptionBaseTransitionData.toSubscriptionTransitionType(cur.getType(), apiType);
 
-            final String planNameWithClosure = planName;
+            final String planNameWithClosure = plan != null ? plan.getName() : null;
             final String planPhaseNameWithClosure = planPhaseName;
             final Integer billCycleDayLocalWithClosure = billCycleDayLocal;
-            final PlanPhaseSpecifier spec = new PlanPhaseSpecifier(planName, phaseType);
+            final PlanPhaseSpecifier spec = new PlanPhaseSpecifier(planNameWithClosure, phaseType);
             result.add(new ExistingEvent() {
                 @Override
                 public SubscriptionBaseTransitionType getSubscriptionTransitionType() {
@@ -167,7 +166,7 @@ public class DefaultSubscriptionBaseTimeline implements SubscriptionBaseTimeline
                 }
             });
 
-            prevPlanName = planName;
+            prevPlan = plan;
             prevProductName = productName;
             prevBillingPeriod = billingPeriod;
             prevPriceListName = priceListName;
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBase.java b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBase.java
index 0fe6c5a..62bb358 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBase.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/user/DefaultSubscriptionBase.java
@@ -263,7 +263,7 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
     }
 
     @Override
-    public boolean cancelWithPolicy(final BillingActionPolicy policy, int accountBillCycleDayLocal, final CallContext context) throws SubscriptionBaseApiException {
+    public boolean cancelWithPolicy(final BillingActionPolicy policy, final int accountBillCycleDayLocal, final CallContext context) throws SubscriptionBaseApiException {
         return apiService.cancelWithPolicy(this, policy, accountBillCycleDayLocal, context);
     }
 
@@ -695,10 +695,10 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
 
         removeEverythingPastCancelEvent(events);
 
-        UUID nextUserToken = null;
+        final UUID nextUserToken = null;
 
-        UUID nextEventId = null;
-        DateTime nextCreatedDate = null;
+        UUID nextEventId;
+        DateTime nextCreatedDate;
         EntitlementState nextState = null;
         String nextPlanName = null;
         String nextPhaseName = null;
@@ -777,13 +777,9 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
                             "Unexpected Event type = %s", cur.getType()));
             }
 
-            Plan nextPlan = null;
-            PlanPhase nextPhase = null;
-            PriceList nextPriceList = null;
-
-            nextPlan = (nextPlanName != null) ? catalog.findPlan(nextPlanName, cur.getEffectiveDate(), getAlignStartDate()) : null;
-            nextPhase = (nextPhaseName != null) ? catalog.findPhase(nextPhaseName, cur.getEffectiveDate(), getAlignStartDate()) : null;
-            nextPriceList = (nextPlan != null) ? catalog.findPriceListForPlan(nextPlanName, cur.getEffectiveDate(), getAlignStartDate()) : null;
+            final Plan nextPlan = (nextPlanName != null) ? catalog.findPlan(nextPlanName, cur.getEffectiveDate(), getAlignStartDate()) : null;
+            final PlanPhase nextPhase = (nextPlan != null && nextPhaseName != null) ? nextPlan.findPhase(nextPhaseName) : null;
+            final PriceList nextPriceList = (nextPlan != null) ? catalog.findPriceListForPlan(nextPlanName, cur.getEffectiveDate(), getAlignStartDate()) : null;
 
             final SubscriptionBaseTransitionData transition = new SubscriptionBaseTransitionData(
                     cur.getId(), id, bundleId, bundleExternalKey, cur.getType(), apiEventType,
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
index 2277355..43a51bc 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
@@ -516,8 +516,7 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
         return transactionalSqlDao.execute(true, new EntitySqlDaoTransactionWrapper<List<SubscriptionBaseEvent>>() {
             @Override
             public List<SubscriptionBaseEvent> inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
-                final List<SubscriptionEventModelDao> models = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getActiveEventsForSubscription(subscriptionId.toString(), context);
-                return filterSubscriptionBaseEvents(models);
+                return getEventsForSubscriptionInTransaction(entitySqlDaoWrapperFactory, subscriptionId, context);
             }
         });
     }
@@ -1087,12 +1086,19 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
         }
     }
 
+    private List<SubscriptionBaseEvent> getEventsForSubscriptionInTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory, final UUID subscriptionId, final InternalTenantContext context) {
+        final List<SubscriptionEventModelDao> models = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getActiveEventsForSubscription(subscriptionId.toString(), context);
+        return filterSubscriptionBaseEvents(models);
+    }
+
     // Sends bus notification for event on effective date -- only used for operation that happen immediately
     private void rebuildSubscriptionAndNotifyBusOfEffectiveImmediateChange(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory, final DefaultSubscriptionBase subscription,
                                                                            final SubscriptionBaseEvent immediateEvent, final int seqId, final Catalog catalog, final InternalCallContext context) {
         try {
-            final DefaultSubscriptionBase upToDateSubscription = createSubscriptionWithNewEvent(subscription, immediateEvent, catalog, context);
-            notifyBusOfEffectiveImmediateChange(entitySqlDaoWrapperFactory, upToDateSubscription, immediateEvent, seqId, context);
+            // We need to rehydrate the subscription, as some events might have been canceled on disk (e.g. future PHASE after while doing a change plan)
+            final List<SubscriptionBaseEvent> activeSubscriptionEvents = getEventsForSubscriptionInTransaction(entitySqlDaoWrapperFactory, subscription.getId(), context);
+            subscription.rebuildTransitions(activeSubscriptionEvents, catalog);
+            notifyBusOfEffectiveImmediateChange(entitySqlDaoWrapperFactory, subscription, immediateEvent, seqId, context);
         } catch (final CatalogApiException e) {
             log.warn("Failed to post effective event for subscriptionId='{}'", subscription.getId(), e);
         }
@@ -1171,21 +1177,6 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
         createAndRefresh(transBundleDao, new SubscriptionBundleModelDao(bundleData), context);
     }
 
-    //
-    // Creates a copy of the existing subscriptions whose 'transitions' will reflect the new event
-    //
-    private DefaultSubscriptionBase createSubscriptionWithNewEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent newEvent, final Catalog catalog, final InternalTenantContext context) throws CatalogApiException {
-
-        final DefaultSubscriptionBase subscriptionWithNewEvent = new DefaultSubscriptionBase(subscription, null, clock);
-        final List<SubscriptionBaseEvent> allEvents = new LinkedList<SubscriptionBaseEvent>();
-        if (subscriptionWithNewEvent.getEvents() != null) {
-            allEvents.addAll(subscriptionWithNewEvent.getEvents());
-        }
-        allEvents.add(newEvent);
-        subscriptionWithNewEvent.rebuildTransitions(allEvents, catalog);
-        return subscriptionWithNewEvent;
-    }
-
     private InternalCallContext contextWithUpdatedDate(final InternalCallContext input) {
         return new InternalCallContext(input, input.getCreatedDate());
     }
diff --git a/subscription/src/test/java/org/killbill/billing/subscription/alignment/TestPlanAligner.java b/subscription/src/test/java/org/killbill/billing/subscription/alignment/TestPlanAligner.java
index 6fe78b1..ab1a3ea 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/alignment/TestPlanAligner.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/alignment/TestPlanAligner.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
- * Ning 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
  * License.  You may obtain a copy of the License at:
  *
@@ -143,7 +145,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertNull(newPhase);
     }
 
-
     //
     // Scenario : change Plan with START_OF_SUBSCRIPTION after skipping TRIAL on Create to a new Plan that only has EVERGREEN
     //
@@ -173,7 +174,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertEquals(currentPhase.getPhase().getPhaseType(), PhaseType.EVERGREEN);
     }
 
-
     //
     // Scenario : change Plan with START_OF_SUBSCRIPTION after skipping TRIAL on Create to a new Plan that has {TRIAL, EVERGREEN}
     //
@@ -203,7 +203,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertEquals(currentPhase.getPhase().getPhaseType(), PhaseType.EVERGREEN);
     }
 
-
     //
     // Scenario : change Plan with START_OF_SUBSCRIPTION after skipping TRIAL on Create to a new Plan that has {TRIAL, DISCOUNT, EVERGREEN}
     //
@@ -300,7 +299,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertEquals(nextPhase.getPhase().getPhaseType(), PhaseType.FIXEDTERM);
     }
 
-
     //
     // Scenario : change Plan with START_OF_SUBSCRIPTION to a new Plan that has {TRIAL, DISCOUNT, EVERGREEN} and specifying a target PhaseType = DISCOUNT
     //
@@ -372,7 +370,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertNull(nextPhase);
     }
 
-
     private DefaultSubscriptionBase createSubscription(final DateTime bundleStartDate, final DateTime alignStartDate, final String productName, final PhaseType phaseType) throws CatalogApiException {
         final SubscriptionBuilder builder = new SubscriptionBuilder();
         builder.setBundleStartDate(bundleStartDate);
diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
index 04e31de..31a211e 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiChangePlan.java
@@ -521,25 +521,6 @@ public class TestUserApiChangePlan extends SubscriptionTestSuiteWithEmbeddedDB {
         // Check we are seeing the right PENDING transition (pistol-monthly), not the START but the CHANGE on the same date
         assertEquals(((DefaultSubscriptionBase) result2.get(0)).getCurrentOrPendingPlan().getName(), "pistol-monthly");
 
-        // To spice up the test, we insert manually an additional CHANGE event on the exact same dateas the CREATE to verify that code will also invalidate such event when doing the changePlanXX
-
-        final SubscriptionEventModelDao event = new SubscriptionEventModelDao(subscription.getEvents().get(0));
-        event.setId(UUID.randomUUID());
-        event.setUserType(ApiEventType.CHANGE);
-        event.setPlanName("blowdart-monthly");
-        event.setPhaseName("blowdart-monthly-trial");
-        dbi.withHandle(new HandleCallback<Void>() {
-            @Override
-            public Void withHandle(Handle handle) throws Exception {
-                final SubscriptionEventSqlDao sqlDao = handle.attach(SubscriptionEventSqlDao.class);
-                sqlDao.create(event, internalCallContext);
-                return null;
-            }
-        });
-
-        final DefaultSubscriptionBase refreshed1 = (DefaultSubscriptionBase) subscriptionInternalApi.getSubscriptionFromId(subscription.getId(), internalCallContext);
-        assertEquals(refreshed1.getEvents().size(), subscription.getEvents().size() + 1);
-
         subscription.changePlanWithDate(spec, null, subscription.getStartDate(), callContext);
         assertListenerStatus();