killbill-memoizeit

subscription: Deactivate pending CREATE event upon CHANGE

4/21/2017 10:10:35 PM

Details

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
index fa7cc6c..89507b5 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegration.java
@@ -160,6 +160,7 @@ public class TestIntegration extends TestIntegrationBase {
         invoiceChecker.checkInvoiceNoAudits(dryRunInvoice, callContext, expectedInvoices);
 
 
+        clock.addDeltaFromReality(1000); // Make sure CHANGE does not collide with CREATE
         changeEntitlementAndCheckForCompletion(baseEntitlement, "Assault-Rifle", BillingPeriod.MONTHLY, null, NextEvent.CHANGE, NextEvent.INVOICE);
         invoiceChecker.checkInvoice(account.getId(), invoiceItemCount++, callContext, expectedInvoices);
         invoiceChecker.checkChargedThroughDate(subscription.getId(), clock.getUTCToday(), callContext);
@@ -263,6 +264,7 @@ public class TestIntegration extends TestIntegrationBase {
         //
         // CHANGE PLAN IMMEDIATELY AND EXPECT BOTH EVENTS: NextEvent.CHANGE NextEvent.INVOICE
         //
+        clock.addDeltaFromReality(1000); // Make sure CHANGE does not exactly align with CREATE
         changeEntitlementAndCheckForCompletion(baseEntitlement, "Assault-Rifle", BillingPeriod.MONTHLY, null, NextEvent.CHANGE, NextEvent.INVOICE);
         invoiceChecker.checkInvoice(account.getId(), invoiceItemCount++, callContext, new ExpectedInvoiceItemCheck(initialCreationDate.toLocalDate(), null, InvoiceItemType.FIXED, new BigDecimal("0")));
         invoiceChecker.checkChargedThroughDate(subscription.getId(), clock.getUTCToday(), callContext);
@@ -355,6 +357,7 @@ public class TestIntegration extends TestIntegrationBase {
         //
         // CHANGE PLAN IMMEDIATELY AND EXPECT BOTH EVENTS: NextEvent.CHANGE NextEvent.INVOICE
         //
+        clock.addDeltaFromReality(1000); // Ensure CHANGE does not collide with CREATE
         baseEntitlement = changeEntitlementAndCheckForCompletion(baseEntitlement, "Assault-Rifle", BillingPeriod.MONTHLY, null, NextEvent.CHANGE, NextEvent.INVOICE);
         invoiceChecker.checkInvoice(account.getId(), invoiceItemCount++, callContext, new ExpectedInvoiceItemCheck(initialCreationDate.toLocalDate(), null, InvoiceItemType.FIXED, new BigDecimal("0")));
         invoiceChecker.checkChargedThroughDate(subscription.getId(), clock.getUTCToday(), callContext);
diff --git a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
index 437a797..6023c86 100644
--- a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
+++ b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlement.java
@@ -263,7 +263,7 @@ public class TestDefaultEntitlement extends EntitlementTestSuiteWithEmbeddedDB {
         assertListenerStatus();
 
         // Immediate change during trial
-        testListener.pushExpectedEvent(NextEvent.CHANGE);
+        testListener.pushExpectedEvent(NextEvent.CREATE);
         entitlement.changePlan(new PlanSpecifier("Assault-Rifle", BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME), null, ImmutableList.<PluginProperty>of(), callContext);
         assertListenerStatus();
 
@@ -323,7 +323,7 @@ public class TestDefaultEntitlement extends EntitlementTestSuiteWithEmbeddedDB {
 
         entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), startDate, ImmutableList.<PluginProperty>of(), callContext);
 
-        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.CHANGE, NextEvent.BLOCK);
+        testListener.pushExpectedEvents(NextEvent.CREATE,  NextEvent.BLOCK);
         clock.addDays(10);
         assertListenerStatus();
 
diff --git a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
index 5e2f4f6..96cbcab 100644
--- a/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
+++ b/entitlement/src/test/java/org/killbill/billing/entitlement/api/TestDefaultEntitlementApi.java
@@ -647,6 +647,7 @@ public class TestDefaultEntitlementApi extends EntitlementTestSuiteWithEmbeddedD
         }
 
         // effectiveDate is null (same as first case above), but **did**  reach the billing startDate (and entitlement startDate) so will succeed
+        clock.addDeltaFromReality(1000); // Add one sec to make sure CHANGE event does not coincide with CREATE (realistic scenario), and therefore we do expect a CHANGE event
         testListener.pushExpectedEvents(NextEvent.CHANGE);
         final Entitlement result = entitlement.changePlanWithDate(spec2, ImmutableList.<PlanPhasePriceOverride>of(), null, ImmutableList.<PluginProperty>of(), callContext);
         assertListenerStatus();
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 bbfeef2..2f9eafe 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
@@ -627,7 +627,7 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
                 throw new SubscriptionBaseApiException(ErrorCode.SUB_CHANGE_AO_MAX_PLAN_ALLOWED_BY_BUNDLE, plan.getName());
             }
         }
-        return apiService.dryRunChangePlan((DefaultSubscriptionBase) subscription, spec, requestedDateWithMs, requestedPolicy, tenantContext);
+        return apiService.dryRunChangePlan((DefaultSubscriptionBase) subscription, spec, effectiveDate, requestedPolicy, tenantContext);
     }
 
     @Override
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 44d7fe9..54768ac 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
@@ -301,20 +301,7 @@ public class DefaultSubscriptionBase extends EntityBase implements SubscriptionB
                 clock, transitions, Order.ASC_FROM_PAST,
                 Visibility.ALL, TimeLimit.FUTURE_ONLY);
 
-        final SubscriptionBaseTransition initialPendingTransition = it.hasNext() ? it.next() : null;
-
-        // If we have multiple change aligning on the startDate we return the latest to ensure that we get access to right Plan
-        // TODO : However, this means this initial PENDING transition could be a CHANGE (which could confuse some clients, unclear)
-        SubscriptionBaseTransition result = initialPendingTransition;
-        while (it.hasNext()) {
-            final SubscriptionBaseTransition next = it.next();
-            if (next.getTransitionType() == SubscriptionBaseTransitionType.CHANGE && initialPendingTransition.getEffectiveTransitionTime().compareTo(next.getEffectiveTransitionTime()) == 0) {
-                result = next;
-            } else {
-                break;
-            }
-        }
-        return result;
+        return it.hasNext() ? it.next() : null;
     }
 
     @Override
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 332d497..0ee57df 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
@@ -33,6 +33,7 @@ import java.util.UUID;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
+import com.google.common.base.Preconditions;
 import org.joda.time.DateTime;
 import org.killbill.billing.ErrorCode;
 import org.killbill.billing.callcontext.InternalCallContext;
@@ -75,6 +76,7 @@ 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.exceptions.SubscriptionBaseError;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
@@ -454,7 +456,7 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
         return transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<List<SubscriptionBaseEvent>>() {
             @Override
             public List<SubscriptionBaseEvent> inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
-                final List<SubscriptionEventModelDao> models = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getEventsForSubscription(subscriptionId.toString(), context);
+                final List<SubscriptionEventModelDao> models = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getActiveEventsForSubscription(subscriptionId.toString(), context);
                 return filterSubscriptionBaseEvents(models);
             }
         });
@@ -641,17 +643,62 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
         });
     }
 
+
     @Override
-    public void changePlan(final DefaultSubscriptionBase subscription, final List<SubscriptionBaseEvent> changeEvents, final List<DefaultSubscriptionBase> subscriptionsToBeCancelled, final List<SubscriptionBaseEvent> cancelEvents, final InternalCallContext context) {
+    public void changePlan(final DefaultSubscriptionBase subscription, final List<SubscriptionBaseEvent> originalInputChangeEvents, final List<DefaultSubscriptionBase> subscriptionsToBeCancelled, final List<SubscriptionBaseEvent> cancelEvents, final InternalCallContext context) {
+
+        // First event is expected to be the subscription CHANGE event
+        final SubscriptionBaseEvent inputChangeEvent =  originalInputChangeEvents.get(0);
+        Preconditions.checkState(inputChangeEvent.getType() == EventType.API_USER &&
+                ((ApiEvent) inputChangeEvent).getApiEventType() == ApiEventType.CHANGE);
+        Preconditions.checkState(inputChangeEvent.getSubscriptionId().equals(subscription.getId()));
+
         transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
             @Override
             public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+
                 final SubscriptionEventSqlDao transactional = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class);
-                final UUID subscriptionId = subscription.getId();
+                final List<SubscriptionEventModelDao> activeSubscriptionEvents = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getActiveEventsForSubscription(subscription.getId().toString(), context);
+
+                // First event is CREATE/TRANSFER event
+                final SubscriptionEventModelDao firstSubscriptionEvent = activeSubscriptionEvents.get(0);
+                final Iterable<SubscriptionEventModelDao> activePresentOrFutureSubscriptionEvents = Iterables.filter(activeSubscriptionEvents, new Predicate<SubscriptionEventModelDao>() {
+                    @Override
+                    public boolean apply(SubscriptionEventModelDao input) {
+                        return input.getEffectiveDate().compareTo(inputChangeEvent.getEffectiveDate()) >= 0;
+                    }
+                });
+
+                // We do a little magic here in case the CHANGE coincides exactly with the CREATE event to invalidate original CREATE event and
+                // change the input CHANGE event into a CREATE event.
+                final boolean isChangePlanOnStartDate = firstSubscriptionEvent.getEffectiveDate().compareTo(inputChangeEvent.getEffectiveDate()) == 0;
+
+                final List<SubscriptionBaseEvent> inputChangeEvents;
+                if (isChangePlanOnStartDate) {
+
+                    // Rebuild input event list with first the CREATE event and all original input events except for inputChangeEvent
+                    inputChangeEvents = new ArrayList<SubscriptionBaseEvent>();
+                    final SubscriptionBaseEvent newCreateEvent = new ApiEventBuilder((ApiEventChange) inputChangeEvent)
+                            .setApiEventType(ApiEventType.CREATE)
+                            .build();
+
+                    originalInputChangeEvents.remove(0);
+                    inputChangeEvents.add(newCreateEvent);
+                    inputChangeEvents.addAll(originalInputChangeEvents);
+
+                    // Deactivate original CREATE event
+                    unactivateEventFromTransaction(firstSubscriptionEvent, entitySqlDaoWrapperFactory, context);
+
+                } else {
+                    inputChangeEvents = originalInputChangeEvents;
+                }
+
+
+                cancelFutureEventsFromTransaction(activePresentOrFutureSubscriptionEvents, entitySqlDaoWrapperFactory, false, context);
+
 
-                cancelFutureEventsFromTransaction(subscriptionId, changeEvents.get(0).getEffectiveDate(), entitySqlDaoWrapperFactory, false, context);
 
-                for (final SubscriptionBaseEvent cur : changeEvents) {
+                for (final SubscriptionBaseEvent cur : inputChangeEvents) {
                     createAndRefresh(transactional, new SubscriptionEventModelDao(cur), context);
 
                     final boolean isBusEvent = cur.getEffectiveDate().compareTo(clock.getUTCNow()) <= 0 && (cur.getType() == EventType.API_USER);
@@ -659,7 +706,7 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
                 }
 
                 // Notify the Bus of the latest requested change
-                final SubscriptionBaseEvent finalEvent = changeEvents.get(changeEvents.size() - 1);
+                final SubscriptionBaseEvent finalEvent = inputChangeEvents.get(inputChangeEvents.size() - 1);
                 notifyBusOfRequestedChange(entitySqlDaoWrapperFactory, subscription, finalEvent, SubscriptionBaseTransitionType.CHANGE, context);
 
                 // Cancel associated add-ons
@@ -715,8 +762,12 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
 
     private void cancelFutureEventsFromTransaction(final UUID subscriptionId, final DateTime effectiveDate, final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory, final boolean includingBCDChange, final InternalCallContext context) {
         final List<SubscriptionEventModelDao> eventModels = entitySqlDaoWrapperFactory.become(SubscriptionEventSqlDao.class).getFutureOrPresentActiveEventForSubscription(subscriptionId.toString(), effectiveDate.toDate(), context);
-        for (final SubscriptionEventModelDao cur : eventModels) {
+        cancelFutureEventsFromTransaction(eventModels, entitySqlDaoWrapperFactory, includingBCDChange, context);
+    }
+
 
+    private void cancelFutureEventsFromTransaction(final Iterable<SubscriptionEventModelDao> eventModels, final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory, final boolean includingBCDChange, final InternalCallContext context) {
+        for (final SubscriptionEventModelDao cur : eventModels) {
             // Skip CREATE event (because of date equality in the query and we don't want to invalidate CREATE event that match a CANCEL event)
             if (cur.getEventType() == EventType.API_USER && (cur.getUserType()== ApiEventType.CREATE || cur.getUserType()== ApiEventType.TRANSFER)) {
                 continue;
@@ -879,40 +930,46 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
             return;
         }
         for (final SubscriptionBaseEvent curDryRun : dryRunEvents) {
+
+            boolean swapChangeEventWithCreate = false;
+
             if (curDryRun.getSubscriptionId() != null && curDryRun.getSubscriptionId().equals(subscriptionId)) {
 
-                //boolean inserted = false;
+                final boolean isApiChange = curDryRun.getType() == EventType.API_USER && ((ApiEvent) curDryRun).getApiEventType() == ApiEventType.CHANGE;
                 final Iterator<SubscriptionBaseEvent> it = events.iterator();
                 while (it.hasNext()) {
                     final SubscriptionBaseEvent event = it.next();
                     if (event.getEffectiveDate().isAfter(curDryRun.getEffectiveDate())) {
                         it.remove();
+                    } else if (event.getEffectiveDate().compareTo(curDryRun.getEffectiveDate()) == 0 &&
+                            isApiChange &&
+                            (event.getType() == EventType.API_USER && (((ApiEvent) event).getApiEventType() == ApiEventType.CREATE) || ((ApiEvent) event).getApiEventType() == ApiEventType.TRANSFER)) {
+                        it.remove();
+                        swapChangeEventWithCreate = true;
                     }
                 }
                 // Set total ordering value of the fake dryRun event to make sure billing events are correctly ordered
-                final SubscriptionBaseEvent curAdjustedDryRun;
+                // and also transform CHANGE event into CREATE in case of perfect effectiveDate match
+                final EventBaseBuilder eventBuilder;
+                switch (curDryRun.getType()) {
+                    case PHASE:
+                        eventBuilder = new PhaseEventBuilder((PhaseEvent) curDryRun);
+                        break;
+                    case BCD_UPDATE:
+                        eventBuilder = new BCDEventBuilder((BCDEvent) curDryRun);
+                        break;
+                    case API_USER:
+                    default:
+                        eventBuilder = new ApiEventBuilder((ApiEvent) curDryRun);
+                        if (swapChangeEventWithCreate) {
+                            ((ApiEventBuilder) eventBuilder).setApiEventType(ApiEventType.CREATE);
+                        }
+                        break;
+                }
                 if (!events.isEmpty()) {
-
-                    final EventBaseBuilder eventBuilder;
-                    switch (curDryRun.getType()) {
-                        case PHASE:
-                            eventBuilder = new PhaseEventBuilder((PhaseEvent) curDryRun);
-                            break;
-                        case BCD_UPDATE:
-                            eventBuilder = new BCDEventBuilder((BCDEvent) curDryRun);
-                            break;
-                        case API_USER:
-                        default:
-                            eventBuilder = new ApiEventBuilder((ApiEvent) curDryRun);
-                            break;
-                    }
                     eventBuilder.setTotalOrdering(events.get(events.size() - 1).getTotalOrdering() + 1);
-
-                    curAdjustedDryRun = eventBuilder.build();
-                } else {
-                    curAdjustedDryRun = curDryRun;
                 }
-                events.add(curAdjustedDryRun);
+                events.add(eventBuilder.build());
             }
         }
     }
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.java
index ab12d94..c47d482 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.java
@@ -51,8 +51,8 @@ public interface SubscriptionEventSqlDao extends EntitySqlDao<SubscriptionEventM
                                                                                         @BindBean final InternalTenantContext context);
 
     @SqlQuery
-    public List<SubscriptionEventModelDao> getEventsForSubscription(@Bind("subscriptionId") String subscriptionId,
-                                                                    @BindBean final InternalTenantContext context);
+    public List<SubscriptionEventModelDao> getActiveEventsForSubscription(@Bind("subscriptionId") String subscriptionId,
+                                                                          @BindBean final InternalTenantContext context);
 
     @SqlQuery
     public List<SubscriptionEventModelDao> getFutureActiveEventsForAccount(@Bind("now") Date now, @BindBean final InternalTenantContext context);
diff --git a/subscription/src/main/resources/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.sql.stg b/subscription/src/main/resources/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.sql.stg
index a924d70..cfb3ec4 100644
--- a/subscription/src/main/resources/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.sql.stg
+++ b/subscription/src/main/resources/org/killbill/billing/subscription/engine/dao/SubscriptionEventSqlDao.sql.stg
@@ -84,7 +84,7 @@ and effective_date >= :now
 ;
 >>
 
-getEventsForSubscription() ::= <<
+getActiveEventsForSubscription() ::= <<
 select <allTableFields()>
 , record_id as total_ordering
 from <tableName()>
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 62e0be1..33a1ef6 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
@@ -36,6 +36,7 @@ import org.killbill.billing.entitlement.api.SubscriptionEventType;
 import org.killbill.billing.invoice.api.DryRunArguments;
 import org.killbill.billing.subscription.SubscriptionTestSuiteWithEmbeddedDB;
 import org.killbill.billing.subscription.api.SubscriptionBase;
+import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
 import org.killbill.billing.subscription.api.SubscriptionBillingApiException;
 import org.killbill.billing.subscription.events.SubscriptionBaseEvent;
 import org.killbill.billing.subscription.events.user.ApiEvent;
@@ -488,6 +489,7 @@ 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) result1.get(0)).getCurrentOrPendingPlan().getName(), "pistol-monthly");
+        assertEquals(((DefaultSubscriptionBase) result1.get(0)).getPendingTransition().getTransitionType(), SubscriptionBaseTransitionType.CREATE);
         try {
             subscription.changePlan(spec, null, callContext);
             fail("Change plan should have failed : subscription PENDING");
@@ -521,7 +523,7 @@ public class TestUserApiChangePlan extends SubscriptionTestSuiteWithEmbeddedDB {
         assertListenerStatus();
 
         // Move clock to startDate
-        testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.CHANGE);
+        testListener.pushExpectedEvents(NextEvent.CREATE);
         clock.addDays(5);
         assertListenerStatus();