killbill-memoizeit

subscription: Fix issues with changePlan operation on PENDING

4/14/2017 9:35:20 PM

Details

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 3b52dc5..6e66d0c 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
@@ -115,6 +115,7 @@ public class PlanAligner extends BaseAligner {
                                                    final DateTime effectiveDate,
                                                    final PhaseType newPlanInitialPhaseType,
                                                    final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
+
         return getTimedPhaseOnChange(subscription, plan, effectiveDate, newPlanInitialPhaseType, WhichPhase.CURRENT, context);
     }
 
@@ -221,10 +222,16 @@ public class PlanAligner extends BaseAligner {
                                              final PhaseType newPlanInitialPhaseType,
                                              final WhichPhase which,
                                              final InternalTenantContext context) throws CatalogApiException, SubscriptionBaseApiException {
+        final SubscriptionBaseTransition pendingOrLastPlanTransition;
+        if (subscription.getState() == EntitlementState.PENDING) {
+            pendingOrLastPlanTransition = subscription.getPendingTransition();
+        } else {
+            pendingOrLastPlanTransition = subscription.getLastTransitionForCurrentPlan();
+        }
         return getTimedPhaseOnChange(subscription.getAlignStartDate(),
                                      subscription.getBundleStartDate(),
-                                     subscription.getCurrentPhase(),
-                                     subscription.getCurrentPlan(),
+                                     pendingOrLastPlanTransition.getNextPhase(),
+                                     pendingOrLastPlanTransition.getNextPlan(),
                                      nextPlan,
                                      effectiveDate,
                                      // This method is only called while doing the change, hence we want to pass the change effective date
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 f6dac4c..d226c7a 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
@@ -341,7 +341,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
     public DateTime changePlan(final DefaultSubscriptionBase subscription, final PlanSpecifier spec, final List<PlanPhasePriceOverride> overrides, final CallContext context) throws SubscriptionBaseApiException {
         final DateTime now = clock.getUTCNow();
 
-        validateEntitlementState(subscription);
+        validateEntitlementState(subscription, null);
 
         final PlanChangeResult planChangeResult = getPlanChangeResult(subscription, spec, now, context);
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, null, planChangeResult.getPolicy(), context);
@@ -361,7 +361,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
                                                 final DateTime requestedDateWithMs, final CallContext context) throws SubscriptionBaseApiException {
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, requestedDateWithMs, null, context);
         validateEffectiveDate(subscription, effectiveDate);
-        validateEntitlementState(subscription);
+        validateEntitlementState(subscription, requestedDateWithMs);
 
         try {
             doChangePlan(subscription, spec, overrides, effectiveDate, context);
@@ -374,9 +374,10 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
 
     @Override
     public DateTime changePlanWithPolicy(final DefaultSubscriptionBase subscription, final PlanSpecifier spec, final List<PlanPhasePriceOverride> overrides, final BillingActionPolicy policy, final CallContext context) throws SubscriptionBaseApiException {
-        validateEntitlementState(subscription);
 
         final DateTime effectiveDate = dryRunChangePlan(subscription, spec, null, policy, context);
+
+        validateEntitlementState(subscription, effectiveDate);
         try {
             doChangePlan(subscription, spec, overrides, effectiveDate, context);
         } catch (final CatalogApiException e) {
@@ -519,7 +520,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         }
 
         if (subscription.getCategory() == ProductCategory.BASE && addCancellationAddOnForEventsIfRequired) {
-            final Product currentBaseProduct = changeEvent.getEffectiveDate().compareTo(clock.getUTCNow()) <= 0 ? newPlan.getProduct() : subscription.getCurrentPlan().getProduct();
+            final Product currentBaseProduct = changeEvent.getEffectiveDate().compareTo(clock.getUTCNow()) <= 0 ? newPlan.getProduct() : subscription.getCurrentOrPendingPlan().getProduct();
             addOnSubscriptionsToBeCancelled.addAll(addCancellationAddOnForEventsIfRequired(addOnCancelEvents, currentBaseProduct, subscription.getBundleId(), effectiveDate, internalTenantContext));
         }
         return changeEvents;
@@ -614,9 +615,12 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         }
     }
 
-    private void validateEntitlementState(final DefaultSubscriptionBase subscription) throws SubscriptionBaseApiException {
+    private void validateEntitlementState(final DefaultSubscriptionBase subscription, @Nullable final DateTime effectiveDate) throws SubscriptionBaseApiException {
         final EntitlementState currentState = subscription.getState();
-        if (currentState != EntitlementState.ACTIVE) {
+        if (effectiveDate == null && currentState != EntitlementState.ACTIVE) {
+            throw new SubscriptionBaseApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, subscription.getId(), currentState);
+        }
+        if (effectiveDate != null && effectiveDate.compareTo(subscription.getStartDate()) < 0) {
             throw new SubscriptionBaseApiException(ErrorCode.SUB_CHANGE_NON_ACTIVE, subscription.getId(), currentState);
         }
         if (subscription.isSubscriptionFutureCancelled()) {
diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestSubscriptionHelper.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestSubscriptionHelper.java
index 52ba58e..dcb1c5b 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestSubscriptionHelper.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestSubscriptionHelper.java
@@ -89,7 +89,10 @@ public class TestSubscriptionHelper {
 
     public DefaultSubscriptionBase createSubscriptionWithBundle(final UUID bundleId, final String productName, final BillingPeriod term, final String planSet, final DateTime requestedDate)
             throws SubscriptionBaseApiException {
-        testListener.pushExpectedEvent(NextEvent.CREATE);
+
+        if (requestedDate == null || requestedDate.compareTo(clock.getUTCNow()) <= 0) {
+            testListener.pushExpectedEvent(NextEvent.CREATE);
+        }
         final DefaultSubscriptionBase subscription = (DefaultSubscriptionBase) subscriptionApi.createSubscription(bundleId,
                                                                                                                   new PlanPhaseSpecifier(productName, term, planSet, null), null,
                                                                                                                   requestedDate == null ? clock.getUTCNow() : requestedDate, false, internalCallContext);
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 58fe498..1df37af 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
@@ -16,33 +16,34 @@
 
 package org.killbill.billing.subscription.api.user;
 
-import java.util.ArrayList;
-import java.util.List;
-
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
 import org.killbill.billing.ErrorCode;
-import org.killbill.billing.catalog.api.PlanAlignmentCreate;
-import org.killbill.billing.catalog.api.PlanSpecifier;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
 import org.killbill.billing.api.TestApiListener.NextEvent;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.Duration;
 import org.killbill.billing.catalog.api.PhaseType;
 import org.killbill.billing.catalog.api.Plan;
 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.catalog.api.PriceListSet;
 import org.killbill.billing.catalog.api.ProductCategory;
+import org.killbill.billing.entitlement.api.Entitlement;
 import org.killbill.billing.subscription.SubscriptionTestSuiteWithEmbeddedDB;
 import org.killbill.billing.subscription.api.SubscriptionBillingApiException;
 import org.killbill.billing.subscription.events.SubscriptionBaseEvent;
 import org.killbill.billing.subscription.events.user.ApiEvent;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.List;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
 public class TestUserApiChangePlan extends SubscriptionTestSuiteWithEmbeddedDB {
 
@@ -458,4 +459,51 @@ public class TestUserApiChangePlan extends SubscriptionTestSuiteWithEmbeddedDB {
         }
     }
 
+
+    @Test(groups = "slow")
+    public void testPendingSubscription() throws SubscriptionBaseApiException {
+
+        final String baseProduct = "Shotgun";
+        final BillingPeriod baseTerm = BillingPeriod.MONTHLY;
+        final String basePriceList = PriceListSet.DEFAULT_PRICELIST_NAME;
+
+        final DateTime startDate = clock.getUTCNow().plusDays(5);
+
+        final DefaultSubscriptionBase subscription = testUtil.createSubscription(bundle, baseProduct, baseTerm, basePriceList, startDate);
+        assertEquals(subscription.getState(), Entitlement.EntitlementState.PENDING);
+        assertEquals(subscription.getStartDate().compareTo(startDate), 0);
+
+        final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Pistol", baseTerm, basePriceList, null);
+
+        // First try with default api (no date -> IMM) => Call should fail because subscription is PENDING
+        try {
+            subscription.changePlan(spec, null, callContext);
+            fail("Change plan should have failed : subscription PENDING");
+        } catch (final SubscriptionBaseApiException e) {
+            assertEquals(e.getCode(), ErrorCode.SUB_CHANGE_NON_ACTIVE.getCode());
+        }
+
+        // Second try with date prior to startDate => Call should fail because subscription is PENDING
+        try {
+            subscription.changePlanWithDate(spec, null, startDate.minusDays(1), callContext);
+            fail("Change plan should have failed : subscription PENDING");
+        } catch (final SubscriptionBaseApiException e) {
+            assertEquals(e.getCode(), ErrorCode.SUB_INVALID_REQUESTED_DATE.getCode());
+        }
+
+        // Second try with date equals to startDate  Call should succeed, but no event because action in future
+        subscription.changePlanWithDate(spec, null, startDate, callContext);
+        assertListenerStatus();
+
+        // Move clock to startDate
+        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.CHANGE);
+        clock.addDays(5);
+        assertListenerStatus();
+
+        final DefaultSubscriptionBase subscription2 = (DefaultSubscriptionBase) subscriptionInternalApi.getSubscriptionFromId(subscription.getId(), internalCallContext);
+        assertEquals(subscription2.getStartDate().compareTo(startDate), 0);
+        assertEquals(subscription2.getState(), Entitlement.EntitlementState.ACTIVE);
+        assertEquals(subscription2.getCurrentPlan().getProduct().getName(), "Pistol");
+    }
+
 }