junction: Modify handling of BlockingState in junction to treat diffe…

…rent types (ACCOUNT,..) differently. Also remove the nesting logic, which means that adding same BlockingState event twice (same type, svc, block_billing) will be ignored the second time.
killbill · Apr 30, 2017 · 9d1f705 · 9d1f705
1 parent b968f80
commit 9d1f705
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 59 deletions.
diff --git a/...tion/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java b/...tion/src/main/java/org/killbill/billing/junction/plumbing/billing/BlockingCalculator.java
@@ -308,19 +308,18 @@ public String apply(final BlockingState input) {
             }
         }));
 
-        final Map<String, BlockingStateNesting> svcBlockedNesting = new HashMap<String, BlockingStateNesting>();
+        final Map<String, BlockingStateService> svcBlockedMap = new HashMap<String, BlockingStateService>();
         for (String svc : services) {
-            svcBlockedNesting.put(svc, new BlockingStateNesting());
+            svcBlockedMap.put(svc, new BlockingStateService());
         }
 
         for (final BlockingState e : inputBundleEvents) {
-            final BlockingStateNesting svcBlockingStateNesting = svcBlockedNesting.get(e.getService());
+            svcBlockedMap.get(e.getService()).addBlockingState(e);
-            svcBlockingStateNesting.addBlockingState(e);
         }
 
-        final Iterable<DisabledDuration> unorderedDisabledDuration = Iterables.concat(Iterables.transform(svcBlockedNesting.values(), new Function<BlockingStateNesting, List<DisabledDuration>>() {
+        final Iterable<DisabledDuration> unorderedDisabledDuration = Iterables.concat(Iterables.transform(svcBlockedMap.values(), new Function<BlockingStateService, List<DisabledDuration>>() {
             @Override
-            public List<DisabledDuration> apply(final BlockingStateNesting input) {
+            public List<DisabledDuration> apply(final BlockingStateService input) {
                 return input.build();
             }
         }));

diff --git a/...lumbing/billing/BlockingStateNesting.java → ...lumbing/billing/BlockingStateService.java b/...lumbing/billing/BlockingStateNesting.java → ...lumbing/billing/BlockingStateService.java
@@ -18,58 +18,52 @@
 package org.killbill.billing.junction.plumbing.billing;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.UUID;
 
 import javax.annotation.Nullable;
 
 import org.joda.time.DateTime;
 import org.joda.time.Days;
 import org.killbill.billing.entitlement.api.BlockingState;
 
-public class BlockingStateNesting {
+public class BlockingStateService {
 
-    private int nestingLevel;
+    final Map<UUID, BlockingState> perObjectTypeFirstBlockingState = new HashMap<UUID, BlockingState>();
-    private BlockingState first;
 
-    final List<DisabledDuration> result;
+    private final List<DisabledDuration> result;
 
-    public BlockingStateNesting() {
+    public BlockingStateService() {
-        this.nestingLevel = 0;
-        this.first = null;
         this.result = new ArrayList<DisabledDuration>();
     }
 
-
     public List<DisabledDuration> build() {
-        if (first != null) {
+        for (BlockingState cur : perObjectTypeFirstBlockingState.values()) {
-            addDisabledDuration(null);
+            if (cur != null) {
+                addDisabledDuration(cur, null);
+            }
         }
         return result;
     }
 
     public void addBlockingState(final BlockingState currentBlockingState) {
 
-        if (currentBlockingState.isBlockBilling()) {
+        final BlockingState firstBlockingState = perObjectTypeFirstBlockingState.get(currentBlockingState.getBlockedId());
-            if (nestingLevel == 0) {
+        if (currentBlockingState.isBlockBilling() && firstBlockingState == null) {
-                first = currentBlockingState;
+            perObjectTypeFirstBlockingState.put(currentBlockingState.getBlockedId(), currentBlockingState);
-            }
+        } else if (!currentBlockingState.isBlockBilling() && firstBlockingState != null) {
-            nestingLevel++;
+            addDisabledDuration(firstBlockingState, currentBlockingState.getEffectiveDate());
-        }
+            perObjectTypeFirstBlockingState.put(currentBlockingState.getBlockedId(), null);
-
-        if (!currentBlockingState.isBlockBilling() && nestingLevel > 0) {
-            nestingLevel--;
-            if (nestingLevel == 0) {
-                addDisabledDuration(currentBlockingState.getEffectiveDate());
-                first = null;
-            }
         }
     }
 
-    private void addDisabledDuration(@Nullable final DateTime disableDurationEndDate) {
+    private void addDisabledDuration(final BlockingState firstBlockingState, @Nullable final DateTime disableDurationEndDate) {
 
-        if (disableDurationEndDate == null || Days.daysBetween(first.getEffectiveDate(), disableDurationEndDate).getDays() >= 1) {
+        if (disableDurationEndDate == null || Days.daysBetween(firstBlockingState.getEffectiveDate(), disableDurationEndDate).getDays() >= 1) {
             // Don't disable for periods less than a day (see https://github.com/killbill/killbill/issues/267)
-            result.add(new DisabledDuration(first.getEffectiveDate(), disableDurationEndDate));
+            result.add(new DisabledDuration(firstBlockingState.getEffectiveDate(), disableDurationEndDate));
         }
     }
 }
diff --git a/.../src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java b/.../src/test/java/org/killbill/billing/junction/plumbing/billing/TestBlockingCalculator.java
@@ -749,7 +749,8 @@ public void testCreateDisablePairs() {
         assertEquals(pairs.size(), 1);
         assertNotNull(pairs.get(0).getStart());
         assertEquals(pairs.get(0).getStart(), now.plusDays(1));
-        assertNull(pairs.get(0).getEnd());
+        assertNotNull(pairs.get(0).getEnd());
+        assertEquals(pairs.get(0).getEnd(), now.plusDays(3));
 
         blockingEvents = new ArrayList<BlockingState>();
         blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, CLEAR_BUNDLE, "test", false, false, false, now));
@@ -762,7 +763,8 @@ public void testCreateDisablePairs() {
         assertEquals(pairs.size(), 1);
         assertNotNull(pairs.get(0).getStart());
         assertEquals(pairs.get(0).getStart(), now.plusDays(1));
-        assertNull(pairs.get(0).getEnd());
+        assertNotNull(pairs.get(0).getEnd());
+        assertEquals(pairs.get(0).getEnd(), now.plusDays(4));
 
         // Verify ordering at the same effective date doesn't matter. This is to work around nondeterministic ordering
         // behavior in ProxyBlockingStateDao#BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED. See also TestDefaultInternalBillingApi.
@@ -848,13 +850,16 @@ public void testSimpleWithClearBlockingDuration() throws Exception {
 
         blockingCalculator.insertBlockingEvents(billingEvents, new HashSet<UUID>(), internalCallContext);
 
-        assertEquals(billingEvents.size(), 3);
+        assertEquals(billingEvents.size(), 5);
         final List<BillingEvent> events = new ArrayList<BillingEvent>(billingEvents);
         assertEquals(events.get(0).getEffectiveDate(), new LocalDate(2012, 5, 1).toDateTimeAtStartOfDay(DateTimeZone.UTC));
         assertEquals(events.get(0).getTransitionType(), SubscriptionBaseTransitionType.CREATE);
         assertEquals(events.get(1).getEffectiveDate(), new LocalDate(2012, 5, 31).toDateTimeAtStartOfDay(DateTimeZone.UTC));
         assertEquals(events.get(1).getTransitionType(), SubscriptionBaseTransitionType.PHASE);
         assertEquals(events.get(2).getEffectiveDate(), new LocalDate(2012, 7, 15).toDateTimeAtStartOfDay(DateTimeZone.UTC));
         assertEquals(events.get(2).getTransitionType(), SubscriptionBaseTransitionType.START_BILLING_DISABLED);
-    }
+        assertEquals(events.get(3).getEffectiveDate(), new LocalDate(2012, 7, 25).toDateTimeAtStartOfDay(DateTimeZone.UTC));
+        assertEquals(events.get(3).getTransitionType(), SubscriptionBaseTransitionType.END_BILLING_DISABLED);
+        assertEquals(events.get(4).getEffectiveDate(), new LocalDate(2012, 7, 25).toDateTimeAtStartOfDay(DateTimeZone.UTC));
+        assertEquals(events.get(4).getTransitionType(), SubscriptionBaseTransitionType.CHANGE);    }
 }
diff --git a/...ing/billing/TestBlockingStateNesting.java → ...ing/billing/TestBlockingStateService.java b/...ing/billing/TestBlockingStateNesting.java → ...ing/billing/TestBlockingStateService.java
@@ -27,19 +27,33 @@
 import org.killbill.billing.entitlement.api.BlockingStateType;
 import org.killbill.billing.junction.DefaultBlockingState;
 import org.killbill.billing.junction.JunctionTestSuiteNoDB;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
 
-public class TestBlockingStateNesting extends JunctionTestSuiteNoDB {
+public class TestBlockingStateService extends JunctionTestSuiteNoDB {
 
 
+    private UUID accountId;
+    private UUID bundleId;
+    private UUID subscriptionId;
+
+    @BeforeMethod(groups = "fast")
+    public void beforeMethod() throws Exception {
+       super.beforeMethod();
+        this.accountId = UUID.randomUUID();
+        this.bundleId = UUID.randomUUID();
+        this.subscriptionId = UUID.randomUUID();
+    }
+
     //
     // In all tests:
-    // * events are B(locked) or U(nblocked)
+    // * Events are B(locked) or U(nblocked)
-    // * Types does not matter as we only care about nesting level (A(ccount), B(undle), S(ubscription))
+    // * Types are (A(ccount), B(undle), S(ubscription))
 
     //             B    B     U     U
     //             |----|-----|-----|
@@ -48,7 +62,7 @@ public class TestBlockingStateNesting extends JunctionTestSuiteNoDB {
     //  Expected:  B----------------U
     //
     @Test(groups = "fast")
-    public void testZeroNestingLevel() throws Exception {
+    public void testInterlaceTypes() throws Exception {
 
         final List<BlockingState> input = new ArrayList<BlockingState>();
 
@@ -60,13 +74,14 @@ public void testZeroNestingLevel() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(2)));
         input.add(createBillingBlockingState(BlockingStateType.SUBSCRIPTION_BUNDLE, false, testInit.plusDays(3)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
         final List<DisabledDuration> result = test.build();
 
-        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(3)));
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(2)),
+                                                                 new DisabledDuration(testInit.plusDays(1), testInit.plusDays(3)));
 
         verify(result, expected);
     }
@@ -79,7 +94,7 @@ public void testZeroNestingLevel() throws Exception {
     // Expected:     B-------------------
     //
     @Test(groups = "fast")
-    public void testPositiveNestingLevel() throws Exception {
+    public void testInterlaceTypesWithNoEnd() throws Exception {
 
         final List<BlockingState> input = new ArrayList<BlockingState>();
 
@@ -90,13 +105,14 @@ public void testPositiveNestingLevel() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.SUBSCRIPTION_BUNDLE, true, testInit.plusDays(1)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(2)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
         final List<DisabledDuration> result = test.build();
 
-        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, null));
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(2)),
+                                                                 new DisabledDuration(testInit.plusDays(1), null));
 
         verify(result, expected);
     }
@@ -120,7 +136,7 @@ public void testMultipleDisabledDurations() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(2)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(3)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
@@ -152,13 +168,14 @@ public void testSameBlockingDates() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.SUBSCRIPTION_BUNDLE, false, testInit.plusDays(1)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(2)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
         final List<DisabledDuration> result = test.build();
 
-        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(2)));
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(1)),
+                                                                 new DisabledDuration(testInit, testInit.plusDays(2)));
 
         verify(result, expected);
     }
@@ -181,7 +198,7 @@ public void testSameBlockingUnblockingDates() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
@@ -210,7 +227,7 @@ public void testBlockingUnblockingDatesLessThanADay1() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusHours(10)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
@@ -240,7 +257,7 @@ public void testBlockingUnblockingDatesLessThanADay2() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(1)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(1)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
@@ -271,13 +288,44 @@ public void testBlockingUnblockingDatesLessThanADay3() throws Exception {
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(1)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(1)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
         final List<DisabledDuration> result = test.build();
 
-        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, null));
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(1)));
+
+        verify(result, expected);
+    }
+
+
+    //              B       UB
+    //              |-------|
+    //              A       AA
+    //
+    //  Expected:   B--------
+    //
+    @Test(groups = "fast")
+    public void testBlockingUnblockingDatesLessThanADay4() throws Exception {
+
+        final List<BlockingState> input = new ArrayList<BlockingState>();
+
+        final DateTimeZone tz = DateTimeZone.forID("America/Los_Angeles");
+        final DateTime testInit = new DateTime(2017, 04, 29, 14, 15, 53, tz);
+        clock.setTime(testInit);
+        input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit));
+        input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit.plusDays(1)));
+        input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(1)));
+
+        final BlockingStateService test = new BlockingStateService();
+        for (BlockingState cur : input) {
+            test.addBlockingState(cur);
+        }
+        final List<DisabledDuration> result = test.build();
+
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit, testInit.plusDays(1)),
+                                                                 new DisabledDuration(testInit.plusDays(1), null));
 
         verify(result, expected);
     }
@@ -297,17 +345,18 @@ public void testStartingWithUnblock() throws Exception {
         final DateTimeZone tz = DateTimeZone.forID("America/Los_Angeles");
         final DateTime testInit = new DateTime(2017, 04, 29, 14, 15, 53, tz);
         clock.setTime(testInit);
-        input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, false, testInit));
+        input.add(createBillingBlockingState(BlockingStateType.SUBSCRIPTION_BUNDLE, false, testInit));
-        input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(1)));
         input.add(createBillingBlockingState(BlockingStateType.ACCOUNT, true, testInit.plusDays(1)));
+        input.add(createBillingBlockingState(BlockingStateType.SUBSCRIPTION_BUNDLE, true, testInit.plusDays(2)));
 
-        final BlockingStateNesting test = new BlockingStateNesting();
+        final BlockingStateService test = new BlockingStateService();
         for (BlockingState cur : input) {
             test.addBlockingState(cur);
         }
         final List<DisabledDuration> result = test.build();
 
-        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit.plusDays(1), null));
+        final List<DisabledDuration> expected = ImmutableList.of(new DisabledDuration(testInit.plusDays(2), null),
+                                                                 new DisabledDuration(testInit.plusDays(1), null));
 
         verify(result, expected);
     }
@@ -318,12 +367,33 @@ public void testStartingWithUnblock() throws Exception {
     private void verify(final List<DisabledDuration> actual, final List<DisabledDuration> expected) {
         assertEquals(expected.size(), actual.size());
         for (int i = 0; i < actual.size(); i++) {
-            assertEquals(actual.get(i), expected.get(i));
+            boolean found = false;
+            for (int j = 0; j < expected.size(); j++) {
+                if (actual.get(i).equals(expected.get(j))) {
+                    found = true;
+                    break;
+                }
+            }
+            assertTrue(found);
         }
     }
 
     private BlockingState createBillingBlockingState(final BlockingStateType type, final boolean blockBilling, final DateTime effectiveDate) {
-        return new DefaultBlockingState(UUID.randomUUID(), type, UUID.randomUUID().toString(), "SVC", false, false, blockBilling, effectiveDate);
+        final UUID blockedId;
+        switch(type) {
+            case ACCOUNT:
+                blockedId = accountId;
+                break;
+            case SUBSCRIPTION_BUNDLE:
+                blockedId = bundleId;
+                break;
+            case SUBSCRIPTION:
+                blockedId = subscriptionId;
+                break;
+            default:
+                throw new IllegalStateException("Unexpexted type");
+        }
+        return new DefaultBlockingState(blockedId, type, UUID.randomUUID().toString(), "SVC", false, false, blockBilling, effectiveDate);
     }
 
 }