killbill-memoizeit

diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestMigrationSubscriptions.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestMigrationSubscriptions.java
index b109e27..cb1574c 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestMigrationSubscriptions.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestMigrationSubscriptions.java
@@ -1,6 +1,6 @@
 /*
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
  *
  * 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
@@ -330,12 +330,9 @@ public class TestMigrationSubscriptions extends TestIntegrationBase {
         assertListenerStatus();
     }
 
-
     // Not exactly a migration test, but verifies correct behavior when using BlockingState (see https://github.com/killbill/killbill/issues/744)
-    @Test(groups = "slow", invocationCount = 1)
-    public void testBlockingStates() throws Exception {
-
-
+    @Test(groups = "slow")
+    public void testBlockingStatesV1() throws Exception {
         final DateTime initialDate = new DateTime(2017, 3, 1, 0, 1, 35, 0, DateTimeZone.UTC);
         clock.setDeltaFromReality(initialDate.getMillis() - clock.getUTCNow().getMillis());
 
@@ -350,7 +347,7 @@ public class TestMigrationSubscriptions extends TestIntegrationBase {
         clock.addDays(1);
 
         final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("pistol-monthly-notrial", null);
-        busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK, NextEvent.NULL_INVOICE);
+        busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK);
         entitlementApi.createBaseEntitlement(account.getId(), spec, "bundleExternalKey", ImmutableList.<PlanPhasePriceOverride>of(), null, null, false, ImmutableList.<PluginProperty>of(), callContext);
         assertListenerStatus();
 
@@ -366,7 +363,34 @@ public class TestMigrationSubscriptions extends TestIntegrationBase {
         assertListenerStatus();
     }
 
+    @Test(groups = "slow")
+    public void testBlockingStatesV2() throws Exception {
+        final DateTime initialDate = new DateTime(2017, 3, 1, 0, 1, 35, 0, DateTimeZone.UTC);
+        clock.setDeltaFromReality(initialDate.getMillis() - clock.getUTCNow().getMillis());
+
+        final Account account = createAccountWithNonOsgiPaymentMethod(getAccountData(0));
+        assertNotNull(account);
+
+        final BlockingState blockingState1 = new DefaultBlockingState(account.getId(), BlockingStateType.ACCOUNT, "state1", "Service", false, false, true, null);
+        final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("pistol-monthly-notrial", null);
+
+        // Unlike the previous scenario, we create the subscription and set the blocking state at the same time
+        busHandler.pushExpectedEvents(NextEvent.BLOCK, NextEvent.CREATE, NextEvent.BLOCK);
+        subscriptionApi.addBlockingState(blockingState1, null, ImmutableList.<PluginProperty>of(), callContext);
+        entitlementApi.createBaseEntitlement(account.getId(), spec, "bundleExternalKey", ImmutableList.<PlanPhasePriceOverride>of(), null, null, false, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
+
+        clock.addMonths(1);
+
+        busHandler.pushExpectedEvents(NextEvent.BLOCK, NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
+        final BlockingState blockingState2 = new DefaultBlockingState(account.getId(), BlockingStateType.ACCOUNT, "state2", "Service", false, false, false, null);
+        subscriptionApi.addBlockingState(blockingState2, null, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
 
+        busHandler.pushExpectedEvents(NextEvent.INVOICE, NextEvent.INVOICE_PAYMENT, NextEvent.PAYMENT);
+        clock.addMonths(1);
+        assertListenerStatus();
+    }
 
     private BaseEntitlementWithAddOnsSpecifier buildBaseEntitlementWithAddOnsSpecifier(final LocalDate entitlementMigrationDate, final LocalDate billingMigrationDate, final String externalKey, final List<EntitlementSpecifier> specifierList) {
         return new BaseEntitlementWithAddOnsSpecifier() {

diff --git a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
index 00169c3..ab0c806 100644
--- a/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
+++ b/junction/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Hashtable;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -238,7 +237,7 @@ public class BlockingCalculator {
         for (final DisabledDuration duration : disabledDuration) {
             for (final BillingEvent event : filteredBillingEvents) {
                 if (duration.getEnd() == null || event.getEffectiveDate().isBefore(duration.getEnd())) {
-                    if (event.getEffectiveDate().isAfter(duration.getStart())) { //between the pair
+                    if (!event.getEffectiveDate().isBefore(duration.getStart())) { //between the pair
                         result.add(event);
                     }
                 } else { //after the last event of the pair no need to keep checking
@@ -281,9 +280,9 @@ public class BlockingCalculator {
         final SortedSet<BillingEvent> filteredBillingEvents = filter(billingEvents, subscription);
         BillingEvent result = filteredBillingEvents.first();
 
-        if (disabledDurationStart.isBefore(result.getEffectiveDate())) {
+        if (!disabledDurationStart.isAfter(result.getEffectiveDate())) {
             //This case can happen, for example, if we have an add on and the bundle goes into disabled before the add on is created
-            return result;
+            return null;
         }
 
         for (final BillingEvent event : filteredBillingEvents) {

diff --git a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
index 61b9a97..32a7bb7 100644
--- a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
+++ b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
  *
  * 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
@@ -203,6 +203,24 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
         assertEquals(results.first(), e1);
     }
 
+    // Open with no previous event (only at the same time)
+    // -----[X-----------------------------
+    @Test(groups = "fast")
+    public void testEventsToRemoveOpenSameTime() {
+        final DateTime now = clock.getUTCNow();
+        final List<DisabledDuration> disabledDuration = new ArrayList<BlockingCalculator.DisabledDuration>();
+        final SortedSet<BillingEvent> billingEvents = new TreeSet<BillingEvent>();
+
+        disabledDuration.add(new DisabledDuration(now, null));
+        final BillingEvent e1 = createRealEvent(now, subscription1);
+        billingEvents.add(e1);
+
+        final SortedSet<BillingEvent> results = blockingCalculator.eventsToRemove(disabledDuration, billingEvents, subscription1);
+
+        assertEquals(results.size(), 1);
+        assertEquals(results.first(), e1);
+    }
+
     // Closed duration with a single previous event
     // --X--[------------]---------------------
     @Test(groups = "fast")
@@ -379,6 +397,22 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
         assertEquals(results.size(), 0);
     }
 
+    // Open with no previous event (only at the same time)
+    // -----[X-----------------------------
+    @Test(groups = "fast")
+    public void testCreateNewEventsOpenSameTime() throws CatalogApiException {
+        final DateTime now = clock.getUTCNow();
+        final List<DisabledDuration> disabledDuration = new ArrayList<BlockingCalculator.DisabledDuration>();
+        final SortedSet<BillingEvent> billingEvents = new TreeSet<BillingEvent>();
+
+        disabledDuration.add(new DisabledDuration(now, null));
+        billingEvents.add(createRealEvent(now, subscription1));
+
+        final SortedSet<BillingEvent> results = blockingCalculator.createNewEvents(disabledDuration, billingEvents, subscription1, internalCallContext);
+
+        assertEquals(results.size(), 0);
+    }
+
     // Closed duration with a single previous event
     // --X--[------------]---------------------
     @Test(groups = "fast")
@@ -759,6 +793,41 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
     }
 
     @Test(groups = "fast")
+    public void testCreateAndMergeDisablePairs() {
+        final List<BlockingState> blockingEvents = new ArrayList<BlockingState>();
+        final UUID ovdId = UUID.randomUUID();
+        final DateTime entitlementStartDate = clock.getUTCNow();
+        final DateTime blockEffectiveDate = entitlementStartDate.plusSeconds(1);
+        final DateTime unblockEffectiveDate = blockEffectiveDate.plusDays(2);
+
+        // Similar to an entitlement start event
+        blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, CLEAR_BUNDLE, "test", false, false, false, entitlementStartDate));
+        List<DisabledDuration> pairs = blockingCalculator.createBlockingDurations(blockingEvents);
+        assertEquals(pairs.size(), 0);
+
+        blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, DISABLED_BUNDLE, "test", true, true, true, blockEffectiveDate));
+
+        pairs = blockingCalculator.createBlockingDurations(blockingEvents);
+        assertEquals(pairs.size(), 1);
+        assertEquals(pairs.get(0).getStart().compareTo(blockEffectiveDate), 0);
+        assertNull(pairs.get(0).getEnd());
+
+        blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, CLEAR_BUNDLE, "test", false, false, false, unblockEffectiveDate));
+
+        pairs = blockingCalculator.createBlockingDurations(blockingEvents);
+        assertEquals(pairs.size(), 1);
+        assertEquals(pairs.get(0).getStart().compareTo(blockEffectiveDate), 0);
+        assertEquals(pairs.get(0).getEnd().compareTo(unblockEffectiveDate), 0);
+
+        blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, DISABLED_BUNDLE, "test", true, true, true, unblockEffectiveDate));
+
+        pairs = blockingCalculator.createBlockingDurations(blockingEvents);
+        assertEquals(pairs.size(), 1);
+        assertEquals(pairs.get(0).getStart().compareTo(blockEffectiveDate), 0);
+        assertNull(pairs.get(0).getEnd());
+    }
+
+    @Test(groups = "fast")
     public void testSimpleWithClearBlockingDuration() throws Exception {
 
         final BillingEvent trial = createRealEvent(new LocalDate(2012, 5, 1).toDateTimeAtStartOfDay(DateTimeZone.UTC), subscription1, SubscriptionBaseTransitionType.CREATE);

diff --git a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
index f72fb2f..c8c99f1 100644
--- a/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
+++ b/junction/src/test/java/org/killbill/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 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:
  *
@@ -20,17 +22,14 @@ import java.util.List;
 
 import org.joda.time.DateTime;
 import org.joda.time.LocalDate;
-import org.killbill.billing.payment.api.PluginProperty;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
+import org.joda.time.Period;
+import org.joda.time.ReadablePeriod;
+import org.joda.time.Seconds;
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.api.TestApiListener.NextEvent;
-import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.catalog.api.PriceListSet;
-import org.killbill.billing.catalog.api.ProductCategory;
 import org.killbill.billing.entitlement.EntitlementService;
 import org.killbill.billing.entitlement.api.BlockingStateType;
 import org.killbill.billing.entitlement.api.DefaultEntitlementApi;
@@ -38,8 +37,11 @@ import org.killbill.billing.entitlement.api.Entitlement;
 import org.killbill.billing.junction.BillingEvent;
 import org.killbill.billing.junction.DefaultBlockingState;
 import org.killbill.billing.junction.JunctionTestSuiteWithEmbeddedDB;
+import org.killbill.billing.payment.api.PluginProperty;
 import org.killbill.billing.subscription.api.SubscriptionBase;
 import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
+import org.testng.Assert;
+import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 
@@ -49,7 +51,7 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
     // The invocationCount > 0 was to trigger an issue where events would come out-of-order randomly.
     // While the bug shouldn't occur anymore, we're keeping it just in case (the test will also try to insert the events out-of-order manually).
     // This test also checks we don't generate billing events for blocking durations less than a day (https://github.com/killbill/killbill/issues/267).
-    @Test(groups = "slow", description = "Check blocking states with same effective date are correctly handled", invocationCount = 10, enabled = false)
+    @Test(groups = "slow", description = "Check blocking states with same effective date are correctly handled", invocationCount = 10)
     public void testBlockingStatesWithSameEffectiveDate() throws Exception {
         final LocalDate initialDate = new LocalDate(2013, 8, 7);
         clock.setDay(initialDate);
@@ -187,6 +189,15 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
     // See https://github.com/killbill/killbill/commit/92042843e38a67f75495b207385e4c1f9ca60990#commitcomment-4749967
     @Test(groups = "slow", description = "Check unblock then block states with same effective date are correctly handled", invocationCount = 10)
     public void testUnblockThenBlockBlockingStatesWithSameEffectiveDate() throws Exception {
+        testUnblockThenBlockBlockingStatesWithSimilarEffectiveDate(Seconds.ZERO);
+    }
+
+    @Test(groups = "slow", description = "Check unblock then block states with almost the same effective date are correctly handled", invocationCount = 10)
+    public void testUnblockThenBlockBlockingStatesWithAlmostSameEffectiveDate() throws Exception {
+        testUnblockThenBlockBlockingStatesWithSimilarEffectiveDate(Seconds.ONE);
+    }
+
+    private void testUnblockThenBlockBlockingStatesWithSimilarEffectiveDate(final ReadablePeriod delay) throws Exception {
         final LocalDate initialDate = new LocalDate(2013, 8, 7);
         clock.setDay(initialDate);
 
@@ -198,7 +209,10 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
         final SubscriptionBase subscription = subscriptionInternalApi.getSubscriptionFromId(entitlement.getId(), internalCallContext);
         assertListenerStatus();
 
-        final DateTime block1Date = clock.getUTCNow();
+        final DateTime block1Date = subscription.getStartDate().plus(delay);
+        // Make sure to update the clock here, because we don't disable for periods less than a day
+        clock.setTime(block1Date);
+
         testListener.pushExpectedEvents(NextEvent.BLOCK);
         final DefaultBlockingState state1 = new DefaultBlockingState(account.getId(),
                                                                      BlockingStateType.ACCOUNT,
@@ -239,13 +253,17 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
         clock.addDays(3);
         assertListenerStatus();
 
-        // Expected blocking duration:
-        // * 2013-08-07 to now [2013-08-07 to 2013-08-08 then 2013-08-08 to now]
         final List<BillingEvent> events = ImmutableList.<BillingEvent>copyOf(billingInternalApi.getBillingEventsForAccountAndUpdateAccountBCD(account.getId(), null, internalCallContext));
-        Assert.assertEquals(events.size(), 2);
-        Assert.assertEquals(events.get(0).getTransitionType(), SubscriptionBaseTransitionType.CREATE);
-        Assert.assertEquals(events.get(0).getEffectiveDate(), subscription.getStartDate());
-        Assert.assertEquals(events.get(1).getTransitionType(), SubscriptionBaseTransitionType.START_BILLING_DISABLED);
-        Assert.assertEquals(events.get(1).getEffectiveDate(), block1Date);
+        if (delay.toPeriod().toStandardDuration().compareTo(Period.ZERO.toStandardDuration()) == 0) {
+            Assert.assertEquals(events.size(), 0);
+        } else {
+            // Expected blocking duration:
+            // * 2013-08-07 to now [2013-08-07 to 2013-08-08 then 2013-08-08 to now]
+            Assert.assertEquals(events.size(), 2);
+            Assert.assertEquals(events.get(0).getTransitionType(), SubscriptionBaseTransitionType.CREATE);
+            Assert.assertEquals(events.get(0).getEffectiveDate(), subscription.getStartDate());
+            Assert.assertEquals(events.get(1).getTransitionType(), SubscriptionBaseTransitionType.START_BILLING_DISABLED);
+            Assert.assertEquals(events.get(1).getEffectiveDate(), block1Date);
+        }
     }
 }