killbill-aplcache

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
index 6483fff..b21b278 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestSubscription.java
@@ -21,8 +21,10 @@ import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
+import org.killbill.billing.ErrorCode;
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.api.TestApiListener.NextEvent;
@@ -45,6 +47,7 @@ import org.killbill.billing.invoice.api.DryRunType;
 import org.killbill.billing.invoice.api.Invoice;
 import org.killbill.billing.invoice.api.InvoiceItemType;
 import org.killbill.billing.payment.api.PluginProperty;
+import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
@@ -246,7 +249,7 @@ public class TestSubscription extends TestIntegrationBase {
 
 
     @Test(groups = "slow")
-    public void testCancelFutureSubscription() throws Exception {
+    public void testCancelFutureSubscriptionWithPolicy() throws Exception {
 
         final LocalDate initialDate = new LocalDate(2015, 9, 1);
         clock.setDay(initialDate);
@@ -261,6 +264,7 @@ public class TestSubscription extends TestIntegrationBase {
         final Entitlement createdEntitlement = entitlementApi.createBaseEntitlement(account.getId(), spec, account.getExternalKey(), null, futureDate, ImmutableList.<PluginProperty>of(), callContext);
         assertEquals(createdEntitlement.getEffectiveStartDate().compareTo(futureDate), 0);
         assertEquals(createdEntitlement.getEffectiveEndDate(), null);
+        assertListenerStatus();
 
 
         final Entitlement cancelledEntitlement = createdEntitlement.cancelEntitlementWithPolicyOverrideBillingPolicy(EntitlementActionPolicy.IMMEDIATE, BillingActionPolicy.IMMEDIATE, null, callContext);
@@ -271,7 +275,48 @@ public class TestSubscription extends TestIntegrationBase {
         // NextEvent.INVOICE is required because of #467
         busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.INVOICE, NextEvent.BLOCK, NextEvent.CANCEL);
         clock.addDays(30);
+        assertListenerStatus();
+
+        // Just to make sure we really don't invoice for anything move to next month
+        clock.addMonths(1);
+        assertListenerStatus();
+    }
+
+    @Test(groups = "slow")
+    public void testCancelFutureSubscriptionWithRequestedDate() throws Exception {
 
+        final LocalDate initialDate = new LocalDate(2015, 9, 1);
+        clock.setDay(initialDate);
+
+        final Account account = createAccountWithNonOsgiPaymentMethod(getAccountData(1));
+
+        final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Shotgun", ProductCategory.BASE, BillingPeriod.ANNUAL, PriceListSet.DEFAULT_PRICELIST_NAME, null);
+
+        final LocalDate futureDate = new LocalDate(2015, 10, 1);
+
+        // No CREATE event as this is set in the future
+        final Entitlement createdEntitlement = entitlementApi.createBaseEntitlement(account.getId(), spec, account.getExternalKey(), null, futureDate, ImmutableList.<PluginProperty>of(), callContext);
+        assertEquals(createdEntitlement.getEffectiveStartDate().compareTo(futureDate), 0);
+        assertEquals(createdEntitlement.getEffectiveEndDate(), null);
+        assertListenerStatus();
+
+
+        final LocalDate invalidCancelDate = initialDate.plusDays(1);
+        try {
+            createdEntitlement.cancelEntitlementWithDate(invalidCancelDate, true, null, callContext);
+            Assert.fail("Should not succeed to cancel subscription prior startDate");
+        } catch (final EntitlementApiException e) {
+            assertEquals(e.getCode(), ErrorCode.SUB_INVALID_REQUESTED_DATE.getCode());
+        }
+
+        final Entitlement cancelledEntitlement = createdEntitlement.cancelEntitlementWithDate(futureDate, true, null, callContext);
+        assertEquals(cancelledEntitlement.getEffectiveEndDate().compareTo(futureDate), 0);
+        assertListenerStatus();
+
+        // Move off trial and reach start/cancellation date
+        // NextEvent.INVOICE is required because of #467
+        busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.INVOICE, NextEvent.BLOCK, NextEvent.CANCEL);
+        clock.addDays(30);
         assertListenerStatus();
 
         // Just to make sure we really don't invoice for anything move to next month

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
index db39491..64c4d39 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
@@ -510,7 +510,7 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                                                                                                          dryRunArguments.getPlanPhaseSpecifier().getPriceListName(), utcNow, tenantContext);
                                 policy = planChangeResult.getPolicy();
                             }
-                            changeEffectiveDate = subscriptionForChange.getPlanChangeEffectiveDate(subscriptionForChange.getStartDate(), policy);
+                            changeEffectiveDate = subscriptionForChange.getPlanChangeEffectiveDate(policy);
                         }
                         dryRunEvents = apiService.getEventsOnChangePlan(subscriptionForChange, plan, realPriceList, changeEffectiveDate, utcNow, true, context);
                         break;
@@ -530,7 +530,7 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                                                                                        subscriptionForCancellation.getCurrentPhase().getPhaseType());
                                 policy = catalogService.getFullCatalog(context).planCancelPolicy(spec, utcNow);
                             }
-                            cancelEffectiveDate = subscriptionForCancellation.getPlanChangeEffectiveDate(subscriptionForCancellation.getStartDate(), policy);
+                            cancelEffectiveDate = subscriptionForCancellation.getPlanChangeEffectiveDate(policy);
                         }
                         dryRunEvents = apiService.getEventsOnCancelPlan(subscriptionForCancellation, cancelEffectiveDate, utcNow, true, context);
                         break;

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 70e5911..6f04266 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
@@ -492,7 +492,7 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
         return getFutureEndDate() != null;
     }
 
-    public DateTime getPlanChangeEffectiveDate(final DateTime subscriptionStartDate, final BillingActionPolicy policy) {
+    public DateTime getPlanChangeEffectiveDate(final BillingActionPolicy policy) {
 
         final DateTime candidateResult;
         switch (policy) {
@@ -512,7 +512,7 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
                 throw new SubscriptionBaseError(String.format(
                         "Unexpected policy type %s", policy.toString()));
         }
-        return (candidateResult.compareTo(subscriptionStartDate) < 0) ? subscriptionStartDate : candidateResult;
+        return (candidateResult.compareTo(getStartDate()) < 0) ? getStartDate() : candidateResult;
     }
 
     public DateTime getCurrentPhaseStart() {

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 0afc9c7..0805f55 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
@@ -222,7 +222,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         try {
             final InternalTenantContext internalCallContext = createTenantContextFromBundleId(subscription.getBundleId(), context);
             final BillingActionPolicy policy = catalogService.getFullCatalog(internalCallContext).planCancelPolicy(planPhase, now);
-            final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(subscription.getStartDate(), policy);
+            final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(policy);
 
             return doCancelPlan(subscription, now, effectiveDate, context);
         } catch (final CatalogApiException e) {
@@ -248,7 +248,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
             throw new SubscriptionBaseApiException(ErrorCode.SUB_CANCEL_BAD_STATE, subscription.getId(), currentState);
         }
         final DateTime now = clock.getUTCNow();
-        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(subscription.getStartDate(), policy);
+        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(policy);
 
         return doCancelPlan(subscription, now, effectiveDate, context);
     }
@@ -319,7 +319,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
         validateEntitlementState(subscription);
 
         final PlanChangeResult planChangeResult = getPlanChangeResult(subscription, productName, term, priceList, now, context);
-        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(subscription.getStartDate(), planChangeResult.getPolicy());
+        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(planChangeResult.getPolicy());
         validateEffectiveDate(subscription, effectiveDate);
 
         try {
@@ -354,7 +354,7 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
 
         validateEntitlementState(subscription);
 
-        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(subscription.getStartDate(), policy);
+        final DateTime effectiveDate = subscription.getPlanChangeEffectiveDate(policy);
         try {
             return doChangePlan(subscription, productName, term, priceList, overrides, now, effectiveDate, context);
         } catch (final CatalogApiException e) {
@@ -548,10 +548,14 @@ public class DefaultSubscriptionBaseApiService implements SubscriptionBaseApiSer
     }
 
     private void validateEffectiveDate(final DefaultSubscriptionBase subscription, final DateTime effectiveDate) throws SubscriptionBaseApiException {
+
         final SubscriptionBaseTransition previousTransition = subscription.getPreviousTransition();
-        if (previousTransition != null && previousTransition.getEffectiveTransitionTime().isAfter(effectiveDate)) {
+
+        // Our effectiveDate must be after or equal the last transition that already occured (START, PHASE1, PHASE2,...) or the startDate for future started subscription
+        final DateTime earliestValidDate = previousTransition != null ? previousTransition.getEffectiveTransitionTime() : subscription.getStartDate();
+        if (effectiveDate.isBefore(earliestValidDate)) {
             throw new SubscriptionBaseApiException(ErrorCode.SUB_INVALID_REQUESTED_DATE,
-                                                   effectiveDate.toString(), previousTransition.getEffectiveTransitionTime());
+                                                   effectiveDate.toString(), previousTransition != null ? previousTransition.getEffectiveTransitionTime() : "null");
         }
     }