killbill-memoizeit

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 0d9d275..cff670f 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
@@ -38,8 +38,9 @@ import org.killbill.billing.catalog.api.PlanAlignmentCreate;
 import org.killbill.billing.catalog.api.PlanPhase;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.catalog.api.PlanSpecifier;
+import org.killbill.billing.entitlement.api.Entitlement.EntitlementState;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
-import org.killbill.billing.subscription.api.user.SubscriptionBaseTransitionData;
+import org.killbill.billing.subscription.api.user.SubscriptionBaseTransition;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase;
 import org.killbill.billing.subscription.exceptions.SubscriptionBaseError;
 
@@ -89,7 +90,6 @@ public class PlanAligner extends BaseAligner {
                                                                    bundleStartDate,
                                                                    plan,
                                                                    initialPhase,
-                                                                   priceList,
                                                                    effectiveDate,
                                                                    context);
         final TimedPhase[] result = new TimedPhase[2];
@@ -145,39 +145,42 @@ public class PlanAligner extends BaseAligner {
      * @return the next phase
      */
     public TimedPhase getNextTimedPhase(final DefaultSubscriptionBase subscription, final DateTime effectiveDate, final InternalTenantContext context) {
+
+
         try {
-            final SubscriptionBaseTransitionData lastPlanTransition = subscription.getLastTransitionForCurrentPlan();
-            if (effectiveDate.isBefore(lastPlanTransition.getEffectiveTransitionTime())) {
-                throw new SubscriptionBaseError(String.format("Cannot specify an effectiveDate prior to last Plan Change, subscription = %s, effectiveDate = %s",
-                                                         subscription.getId(), effectiveDate));
+            final SubscriptionBaseTransition pendingOrLastPlanTransition;
+            if (subscription.getState() == EntitlementState.PENDING) {
+                pendingOrLastPlanTransition = subscription.getPendingTransition();
+            } else {
+                pendingOrLastPlanTransition = subscription.getLastTransitionForCurrentPlan();
             }
 
-            switch (lastPlanTransition.getTransitionType()) {
+
+            switch (pendingOrLastPlanTransition.getTransitionType()) {
                 // If we never had any Plan change, borrow the logic for createPlan alignment
                 case CREATE:
                 case TRANSFER:
                     final List<TimedPhase> timedPhases = getTimedPhaseOnCreate(subscription.getAlignStartDate(),
                                                                                subscription.getBundleStartDate(),
-                                                                               lastPlanTransition.getNextPlan(),
-                                                                               lastPlanTransition.getNextPhase().getPhaseType(),
-                                                                               lastPlanTransition.getNextPriceList().getName(),
+                                                                               pendingOrLastPlanTransition.getNextPlan(),
+                                                                               pendingOrLastPlanTransition.getNextPhase().getPhaseType(),
                                                                                effectiveDate,
                                                                                context);
                     return getTimedPhase(timedPhases, effectiveDate, WhichPhase.NEXT);
                 case CHANGE:
                     return getTimedPhaseOnChange(subscription.getAlignStartDate(),
                                                  subscription.getBundleStartDate(),
-                                                 lastPlanTransition.getPreviousPhase(),
-                                                 lastPlanTransition.getPreviousPlan(),
-                                                 lastPlanTransition.getNextPlan(),
+                                                 pendingOrLastPlanTransition.getPreviousPhase(),
+                                                 pendingOrLastPlanTransition.getPreviousPlan(),
+                                                 pendingOrLastPlanTransition.getNextPlan(),
                                                  effectiveDate,
-                                                 lastPlanTransition.getEffectiveTransitionTime(),
+                                                 pendingOrLastPlanTransition.getEffectiveTransitionTime(),
                                                  subscription.getAllTransitions().get(0).getNextPhase().getPhaseType(),
                                                  WhichPhase.NEXT,
                                                  context);
                 default:
                     throw new SubscriptionBaseError(String.format("Unexpected initial transition %s for current plan %s on subscription %s",
-                                                             lastPlanTransition.getTransitionType(), subscription.getCurrentPlan(), subscription.getId()));
+                                                             pendingOrLastPlanTransition.getTransitionType(), subscription.getCurrentPlan(), subscription.getId()));
             }
         } catch (Exception /* SubscriptionBaseApiException, CatalogApiException */ e) {
             throw new SubscriptionBaseError(String.format("Could not compute next phase change for subscription %s", subscription.getId()), e);
@@ -188,7 +191,6 @@ public class PlanAligner extends BaseAligner {
                                                    final DateTime bundleStartDate,
                                                    final Plan plan,
                                                    @Nullable final PhaseType initialPhase,
-                                                   final String priceList,
                                                    final DateTime effectiveDate,
                                                    final InternalTenantContext context)
             throws CatalogApiException, SubscriptionBaseApiException {

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java b/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
index 83c8117..bcba397 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/SubscriptionBaseApiService.java
@@ -83,7 +83,7 @@ public interface SubscriptionBaseApiService {
                                          List<PlanPhasePriceOverride> overrides, BillingActionPolicy policy, CallContext context)
             throws SubscriptionBaseApiException;
 
-    public int cancelAddOnsIfRequiredOnBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException;
+    public int handleBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException;
 
     public PlanChangeResult getPlanChangeResult(final DefaultSubscriptionBase subscription, PlanSpecifier spec, final DateTime effectiveDate, TenantContext context) throws SubscriptionBaseApiException;
 

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 19731b9..554c3c9 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
@@ -65,10 +65,12 @@ import org.killbill.billing.subscription.engine.dao.SubscriptionDao;
 import org.killbill.billing.subscription.events.SubscriptionBaseEvent;
 import org.killbill.billing.subscription.events.phase.PhaseEvent;
 import org.killbill.billing.subscription.events.phase.PhaseEventData;
+import org.killbill.billing.subscription.events.user.ApiEvent;
 import org.killbill.billing.subscription.events.user.ApiEventBuilder;
 import org.killbill.billing.subscription.events.user.ApiEventCancel;
 import org.killbill.billing.subscription.events.user.ApiEventChange;
 import org.killbill.billing.subscription.events.user.ApiEventCreate;
+import org.killbill.billing.subscription.events.user.ApiEventType;
 import org.killbill.billing.subscription.events.user.ApiEventUncancel;
 import org.killbill.billing.util.callcontext.CallContext;
 import org.killbill.billing.util.callcontext.InternalCallContextFactory;
@@ -312,7 +314,14 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         uncancelEvents.add(uncancelEvent);
 
         final InternalCallContext internalCallContext = createCallContextFromBundleId(subscription.getBundleId(), context);
-        final TimedPhase nextTimedPhase = planAligner.getNextTimedPhase(subscription, now, internalCallContext);
+        //
+        // Used to compute effective for next phase (which was set unactive during cancellation).
+        // In case of a pending subscription we don't want to pass an effective date prior the CREATE event as we would end up with the wrong
+        // transition in PlanAligner (next transition would be CREATE instead of potential next PHASE)
+        //
+        final DateTime planAlignerEffectiveDate = subscription.getState() == EntitlementState.PENDING ? subscription.getStartDate() : now;
+
+        final TimedPhase nextTimedPhase = planAligner.getNextTimedPhase(subscription, planAlignerEffectiveDate, internalCallContext);
         final PhaseEvent nextPhaseEvent = (nextTimedPhase != null) ?
                                           PhaseEventData.createNextPhaseEvent(subscription.getId(), nextTimedPhase.getPhase().getName(), nextTimedPhase.getStartPhase()) :
                                           null;
@@ -547,15 +556,21 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
     }
 
     @Override
-    public int cancelAddOnsIfRequiredOnBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException {
-        final Product baseProduct = (subscription.getState() == EntitlementState.CANCELLED) ? null : subscription.getCurrentPlan().getProduct();
+    public int handleBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException {
 
-        final List<SubscriptionBaseEvent> cancelEvents = new LinkedList<SubscriptionBaseEvent>();
         final InternalCallContext internalCallContext = createCallContextFromBundleId(subscription.getBundleId(), context);
-        final List<DefaultSubscriptionBase> subscriptionsToBeCancelled = computeAddOnsToCancel(cancelEvents, baseProduct, subscription.getBundleId(), event.getEffectiveDate(), internalCallContext);
-        dao.cancelSubscriptionsOnBasePlanEvent(subscription, event, subscriptionsToBeCancelled, cancelEvents, internalCallContext);
+        if (((ApiEvent) event).getApiEventType() == ApiEventType.CANCEL || ((ApiEvent) event).getApiEventType() == ApiEventType.CHANGE) {
+            final Product baseProduct = (subscription.getState() == EntitlementState.CANCELLED) ? null : subscription.getCurrentPlan().getProduct();
+
+            final List<SubscriptionBaseEvent> cancelEvents = new LinkedList<SubscriptionBaseEvent>();
+            final List<DefaultSubscriptionBase> subscriptionsToBeCancelled = computeAddOnsToCancel(cancelEvents, baseProduct, subscription.getBundleId(), event.getEffectiveDate(), internalCallContext);
+            dao.cancelSubscriptionsOnBasePlanEvent(subscription, event, subscriptionsToBeCancelled, cancelEvents, internalCallContext);
 
-        return subscriptionsToBeCancelled.size();
+            return subscriptionsToBeCancelled.size();
+        } else {
+            dao.notifyOnBasePlanEvent(subscription, event, internalCallContext);
+            return 0;
+        }
     }
 
     private List<DefaultSubscriptionBase> computeAddOnsToCancel(final Collection<SubscriptionBaseEvent> cancelEvents, final CatalogEntity baseProduct, final UUID bundleId, final DateTime effectiveDate, final InternalCallContext internalCallContext) throws CatalogApiException {

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/core/DefaultSubscriptionBaseService.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/core/DefaultSubscriptionBaseService.java
index fd0e273..183814e 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/core/DefaultSubscriptionBaseService.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/core/DefaultSubscriptionBaseService.java
@@ -204,7 +204,7 @@ public class DefaultSubscriptionBaseService implements EventListener, Subscripti
     }
 
     private boolean onBasePlanEvent(final DefaultSubscriptionBase baseSubscription, final SubscriptionBaseEvent event, final CallContext context) throws CatalogApiException {
-        apiService.cancelAddOnsIfRequiredOnBasePlanEvent(baseSubscription, event, context);
+        apiService.handleBasePlanEvent(baseSubscription, event, context);
         return true;
     }
 }

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 cff861a..9bbc75d 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
@@ -97,6 +97,7 @@ import org.killbill.notificationq.api.NotificationQueue;
 import org.killbill.notificationq.api.NotificationQueueService;
 import org.killbill.notificationq.api.NotificationQueueService.NoSuchNotificationQueue;
 import org.skife.jdbi.v2.IDBI;
+import org.skife.jdbi.v2.sqlobject.customizers.OverrideStatementLocatorWith;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -569,6 +570,18 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
     }
 
     @Override
+    public void notifyOnBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final InternalCallContext context) {
+        transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
+            @Override
+            public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+                notifyBusOfEffectiveImmediateChange(entitySqlDaoWrapperFactory, subscription, event, 0, context);
+                return null;
+            }
+        });
+
+    }
+
+    @Override
     public void cancelSubscriptions(final List<DefaultSubscriptionBase> subscriptions, final List<SubscriptionBaseEvent> cancelEvents, final InternalCallContext context) {
         transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
             @Override

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
index 3430d2e..516d80c 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
@@ -87,6 +87,8 @@ public interface SubscriptionDao extends EntityDao<SubscriptionBundleModelDao, S
 
     public void cancelSubscriptionsOnBasePlanEvent(DefaultSubscriptionBase subscription, SubscriptionBaseEvent event, List<DefaultSubscriptionBase> subscriptions, List<SubscriptionBaseEvent> cancelEvents, InternalCallContext context);
 
+    public void notifyOnBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final InternalCallContext context);
+
     public void cancelSubscriptions(List<DefaultSubscriptionBase> subscriptions, List<SubscriptionBaseEvent> cancelEvents, InternalCallContext context);
 
     public void uncancelSubscription(DefaultSubscriptionBase subscription, List<SubscriptionBaseEvent> uncancelEvents, InternalCallContext context);

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 869e886..3dd19b6 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
@@ -80,14 +80,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertNull(phasesInThePast[0]);
         Assert.assertEquals(phasesInThePast[1].getStartPhase(), defaultSubscriptionBase.getBundleStartDate());
 
-        // Verify the next phase via the other API
-        try {
-            planAligner.getNextTimedPhase(defaultSubscriptionBase, effectiveDateInThePast, internalCallContext);
-            Assert.fail("Can't use getNextTimedPhase(): the effective date is before the initial plan");
-        } catch (SubscriptionBaseError e) {
-            Assert.assertTrue(true);
-        }
-
         // Try a change plan now (simulate an IMMEDIATE policy)
         final String newProductName = "shotgun-monthly";
         final DateTime effectiveChangeDate = clock.getUTCNow();
@@ -125,14 +117,6 @@ public class TestPlanAligner extends SubscriptionTestSuiteNoDB {
         Assert.assertNull(phasesInThePast[0]);
         Assert.assertEquals(phasesInThePast[1].getStartPhase(), defaultSubscriptionBase.getStartDate());
 
-        // Verify the next phase via the other API
-        try {
-            planAligner.getNextTimedPhase(defaultSubscriptionBase, effectiveDateInThePast, internalCallContext);
-            Assert.fail("Can't use getNextTimedPhase(): the effective date is before the initial plan");
-        } catch (SubscriptionBaseError e) {
-            Assert.assertTrue(true);
-        }
-
         // Try a change plan (simulate END_OF_TERM policy)
         final String newProductName = "telescopic-scope-monthly";
         final DateTime effectiveChangeDate = defaultSubscriptionBase.getStartDate().plusMonths(1);

diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCancel.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCancel.java
index af4ddbb..be86546 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCancel.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCancel.java
@@ -43,6 +43,7 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 
@@ -384,6 +385,48 @@ public class TestUserApiCancel extends SubscriptionTestSuiteWithEmbeddedDB {
         subscription = (DefaultSubscriptionBase) subscriptionInternalApi.getSubscriptionFromId(subscription.getId(), internalCallContext);
         Assert.assertEquals(subscription.getAllTransitions().get(subscription.getAllTransitions().size() - 1).getTransitionType(), SubscriptionBaseTransitionType.CANCEL);
         Assert.assertEquals(new LocalDate(subscription.getAllTransitions().get(subscription.getAllTransitions().size() - 1).getEffectiveTransitionTime(), accountData.getTimeZone()), new LocalDate(2016, 12, 1));
+    }
+
+    @Test(groups = "slow")
+    public void testCancelUncancelFutureSubscription() throws SubscriptionBaseApiException {
+        final DateTime init = clock.getUTCNow();
+
+        final String productName = "Shotgun";
+        final BillingPeriod term = BillingPeriod.MONTHLY;
+        final String planSetName = PriceListSet.DEFAULT_PRICELIST_NAME;
+
+
+        final DateTime futureCreationDate = init.plusDays(10);
+
+        DefaultSubscriptionBase subscription = (DefaultSubscriptionBase) subscriptionInternalApi.createSubscription(bundle.getId(),
+                                                                                                                    testUtil.getProductSpecifier(productName, planSetName, term, null), null, futureCreationDate, false, internalCallContext);
+        assertListenerStatus();
+        assertNotNull(subscription);
+        assertEquals(subscription.getState(), EntitlementState.PENDING);
+
+        // Cancel / Uncancel a few times to make sure this works and we end up on a stable state
+        for (int i = 0; i < 3; i++) {
+            subscription.cancelWithPolicy(BillingActionPolicy.IMMEDIATE, null, -1, callContext);
+
+            subscription = (DefaultSubscriptionBase) subscriptionInternalApi.getSubscriptionFromId(subscription.getId(), internalCallContext);
+            assertEquals(subscription.getState(), EntitlementState.PENDING);
+
+            subscription.uncancel(callContext);
+        }
+
+        // Now check we are on the right state (as if nothing had happened)
+        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.UNCANCEL, NextEvent.UNCANCEL, NextEvent.UNCANCEL);
+        clock.addDays(10);
+        assertListenerStatus();
+
+        subscription = (DefaultSubscriptionBase) subscriptionInternalApi.getSubscriptionFromId(subscription.getId(), internalCallContext);
+        assertEquals(subscription.getState(), EntitlementState.ACTIVE);
+
+        testListener.pushExpectedEvent(NextEvent.PHASE);
+        clock.addMonths(1);
+        assertListenerStatus();
+
+
 
     }
 

diff --git a/subscription/src/test/java/org/killbill/billing/subscription/engine/dao/MockSubscriptionDaoMemory.java b/subscription/src/test/java/org/killbill/billing/subscription/engine/dao/MockSubscriptionDaoMemory.java
index 8fc4b4b..271bcad 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/engine/dao/MockSubscriptionDaoMemory.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/engine/dao/MockSubscriptionDaoMemory.java
@@ -351,6 +351,11 @@ public class MockSubscriptionDaoMemory extends MockEntityDaoBase<SubscriptionBun
     }
 
     @Override
+    public void notifyOnBasePlanEvent(final DefaultSubscriptionBase subscription, final SubscriptionBaseEvent event, final InternalCallContext context) {
+        notifyBusOfEffectiveImmediateChange(subscription, event, subscriptions.size(), context);
+    }
+
+    @Override
     public void cancelSubscriptions(final List<DefaultSubscriptionBase> subscriptions, final List<SubscriptionBaseEvent> cancelEvents, final InternalCallContext context) {
         synchronized (events) {
             for (int i = 0; i < subscriptions.size(); i++) {