killbill-memoizeit

junction: Modify junction code to take into account the service

4/29/2017 5:50:01 PM

Details

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 16eadea..b109e27 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
@@ -22,6 +22,8 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
 
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 import org.joda.time.LocalDate;
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountData;
@@ -29,20 +31,27 @@ import org.killbill.billing.api.TestApiListener.NextEvent;
 import org.killbill.billing.beatrix.util.InvoiceChecker.ExpectedInvoiceItemCheck;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.PhaseType;
+import org.killbill.billing.catalog.api.PlanPhasePriceOverride;
 import org.killbill.billing.catalog.api.PlanPhaseSpecifier;
 import org.killbill.billing.catalog.api.PriceListSet;
 import org.killbill.billing.entitlement.api.BaseEntitlementWithAddOnsSpecifier;
+import org.killbill.billing.entitlement.api.BlockingState;
+import org.killbill.billing.entitlement.api.BlockingStateType;
 import org.killbill.billing.entitlement.api.DefaultEntitlementSpecifier;
 import org.killbill.billing.entitlement.api.Entitlement;
 import org.killbill.billing.entitlement.api.Entitlement.EntitlementState;
 import org.killbill.billing.entitlement.api.EntitlementSpecifier;
+import org.killbill.billing.invoice.api.Invoice;
 import org.killbill.billing.invoice.api.InvoiceItemType;
+import org.killbill.billing.junction.DefaultBlockingState;
 import org.killbill.billing.payment.api.PluginProperty;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 
+import static org.testng.Assert.assertNotNull;
+
 //
 // These scenarios emulate commons migrations problems (they go on verifying proper entitlement startDate, and proper billing startDate along with invoices, ..)
 //
@@ -321,6 +330,44 @@ 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 {
+
+
+        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);
+
+        busHandler.pushExpectedEvents(NextEvent.BLOCK);
+        final BlockingState blockingState1 = new DefaultBlockingState(account.getId(), BlockingStateType.ACCOUNT, "state1", "Service", false, false, true, null);
+        subscriptionApi.addBlockingState(blockingState1, null, ImmutableList.<PluginProperty>of(), callContext);
+        assertListenerStatus();
+
+        clock.addDays(1);
+
+        final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("pistol-monthly-notrial", null);
+        busHandler.pushExpectedEvents(NextEvent.CREATE, NextEvent.BLOCK, NextEvent.NULL_INVOICE);
+        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() {
                 @Override
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 724e307..00169c3 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,6 +22,7 @@ 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;
@@ -50,10 +51,13 @@ import org.killbill.billing.subscription.api.SubscriptionBase;
 import org.killbill.billing.subscription.api.SubscriptionBaseTransitionType;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
 import com.google.inject.Inject;
 
 public class BlockingCalculator {
@@ -63,7 +67,7 @@ public class BlockingCalculator {
     private final BlockingInternalApi blockingApi;
     private final CatalogService catalogService;
 
-    protected static class DisabledDuration {
+    protected static class DisabledDuration implements Comparable<DisabledDuration> {
 
         private final DateTime start;
         private DateTime end;
@@ -84,6 +88,51 @@ public class BlockingCalculator {
         public void setEnd(final DateTime end) {
             this.end = end;
         }
+
+
+        // Order by start date first and then end date
+        @Override
+        public int compareTo(final DisabledDuration o) {
+            int result = start.compareTo(o.getStart());
+            if (result == 0) {
+                result = end.compareTo(o.getEnd());
+            }
+            return result;
+        }
+
+        //
+        //
+        //  Assumptions (based on ordering):
+        //   * this.start <= o.start
+        //   * this.end <= o.end when this.start == o.start
+        //
+        // Case 1: this contained into o => false
+        // |---------|       this
+        // |--------------|  o
+        //
+        // Case 2: this overlaps with o => false
+        // |---------|            this
+        //      |--------------|  o
+        //
+        // Case 3: o contains into this => false
+        // |---------| this
+        //      |---|  o
+        //
+        // Case 4: this and o are adjacent => false
+        // |---------| this
+        //           |---|  o
+        // Case 5: this and o are disjoint => true
+        // |---------| this
+        //             |---|  o
+        public boolean isDisjoint(final DisabledDuration o) {
+            return end.compareTo(o.getStart()) < 0;
+        }
+
+        public static DisabledDuration mergeDuration(DisabledDuration d1, DisabledDuration d2) {
+            final DateTime endDate = d1.getEnd().compareTo(d2.getEnd()) < 0 ? d2.getEnd() : d1.getEnd();
+            return new DisabledDuration(d1.getStart(), endDate);
+        }
+
     }
 
     @Inject
@@ -212,7 +261,7 @@ public class BlockingCalculator {
             final BillingEvent precedingFinalEvent = precedingBillingEventForSubscription(duration.getEnd(), billingEvents, subscription);
 
             if (precedingInitialEvent != null) { // there is a preceding billing event
-                result.add(createNewDisableEvent(duration.getStart(), precedingInitialEvent, catalog, context));
+                result.add(createNewDisableEvent(duration.getStart(), precedingInitialEvent, catalog));
                 if (duration.getEnd() != null) { // no second event in the pair means they are still disabled (no re-enable)
                     result.add(createNewReenableEvent(duration.getEnd(), precedingFinalEvent, catalog, context));
                 }
@@ -224,21 +273,21 @@ public class BlockingCalculator {
         return result;
     }
 
-    protected BillingEvent precedingBillingEventForSubscription(final DateTime datetime, final SortedSet<BillingEvent> billingEvents, final SubscriptionBase subscription) {
-        if (datetime == null) { //second of a pair can be null if there's no re-enabling
+    protected BillingEvent precedingBillingEventForSubscription(final DateTime disabledDurationStart, final SortedSet<BillingEvent> billingEvents, final SubscriptionBase subscription) {
+        if (disabledDurationStart == null) { //second of a pair can be null if there's no re-enabling
             return null;
         }
 
         final SortedSet<BillingEvent> filteredBillingEvents = filter(billingEvents, subscription);
         BillingEvent result = filteredBillingEvents.first();
 
-        if (datetime.isBefore(result.getEffectiveDate())) {
+        if (disabledDurationStart.isBefore(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 null;
+            return result;
         }
 
         for (final BillingEvent event : filteredBillingEvents) {
-            if (!event.getEffectiveDate().isBefore(datetime)) { // found it its the previous event
+            if (!event.getEffectiveDate().isBefore(disabledDurationStart)) { // found it its the previous event
                 return result;
             } else { // still looking
                 result = event;
@@ -250,17 +299,18 @@ public class BlockingCalculator {
     protected SortedSet<BillingEvent> filter(final SortedSet<BillingEvent> billingEvents, final SubscriptionBase subscription) {
         final SortedSet<BillingEvent> result = new TreeSet<BillingEvent>();
         for (final BillingEvent event : billingEvents) {
-            if (event.getSubscription() == subscription) {
+            if (event.getSubscription().getId().equals(subscription.getId())) {
                 result.add(event);
             }
         }
         return result;
     }
 
-    protected BillingEvent createNewDisableEvent(final DateTime odEventTime, final BillingEvent previousEvent, final Catalog catalog, final InternalTenantContext context) throws CatalogApiException {
+    protected BillingEvent createNewDisableEvent(final DateTime disabledDurationStart, final BillingEvent previousEvent, final Catalog catalog) throws CatalogApiException {
+
         final int billCycleDay = previousEvent.getBillCycleDayLocal();
         final SubscriptionBase subscription = previousEvent.getSubscription();
-        final DateTime effectiveDate = odEventTime;
+        final DateTime effectiveDate = disabledDurationStart;
         final PlanPhase planPhase = previousEvent.getPlanPhase();
         final Plan plan = previousEvent.getPlan();
 
@@ -317,15 +367,28 @@ public class BlockingCalculator {
     }
 
     // In ascending order
-    protected List<DisabledDuration> createBlockingDurations(final Iterable<BlockingState> overdueBundleEvents) {
+    protected List<DisabledDuration> createBlockingDurations(final Iterable<BlockingState> inputBundleEvents) {
+
         final List<DisabledDuration> result = new ArrayList<BlockingCalculator.DisabledDuration>();
-        // Earliest blocking event
-        BlockingState first = null;
 
-        int blockedNesting = 0;
-        BlockingState lastOne = null;
-        for (final BlockingState e : overdueBundleEvents) {
-            lastOne = e;
+        final Set<String> services = ImmutableSet.copyOf(Iterables.transform(inputBundleEvents, new Function<BlockingState, String>() {
+            @Override
+            public String apply(final BlockingState input) {
+                return input.getService();
+            }
+        }));
+
+        final Map<String, BlockingStateNesting> svcBlockedNesting = new HashMap<String, BlockingStateNesting>();
+        for (String svc : services) {
+            svcBlockedNesting.put(svc, new BlockingStateNesting());
+        }
+
+        for (final BlockingState e : inputBundleEvents) {
+
+            final BlockingStateNesting svcBlockingStateNesting = svcBlockedNesting.get(e.getService());
+            int blockedNesting = svcBlockingStateNesting.getBlockedNesting();
+            BlockingState first = svcBlockingStateNesting.getFirst();
+
             if (e.isBlockBilling() && blockedNesting == 0) {
                 // First blocking event of contiguous series of blocking events
                 first = e;
@@ -337,37 +400,116 @@ public class BlockingCalculator {
                 blockedNesting--;
                 if (blockedNesting == 0) {
                     // End of the interval
-                    addDisabledDuration(result, first, e);
+                    svcBlockingStateNesting.addDisabledDuration(first, e);
                     first = null;
                 }
             }
+
+            svcBlockingStateNesting.setFirst(first);
+            svcBlockingStateNesting.setBlockedNesting(blockedNesting);
+            svcBlockingStateNesting.setLastOne(e);
         }
 
-        if (first != null) { // found a transition to disabled with no terminating event
-            addDisabledDuration(result, first, lastOne.isBlockBilling() ? null : lastOne);
+
+        for (final BlockingStateNesting svc : svcBlockedNesting.values()) {
+            if (svc.getFirst() != null) {
+                svc.addDisabledDuration(svc.getFirst(), svc.getLastOne().isBlockBilling() ? null : svc.getLastOne());
+            }
+        }
+
+        Iterable<DisabledDuration> unorderedDisabledDuration = Iterables.concat(Iterables.transform(svcBlockedNesting.values(), new Function<BlockingStateNesting, List<DisabledDuration>>() {
+            @Override
+            public List<DisabledDuration> apply(final BlockingStateNesting input) {
+                return input.getResult();
+            }
+        }));
+
+        final List<DisabledDuration> sortedDisabledDuration = Ordering.natural().sortedCopy(unorderedDisabledDuration);
+
+        DisabledDuration prevDuration = null;
+        for (DisabledDuration d : sortedDisabledDuration) {
+            // isDisjoint
+            if (prevDuration == null) {
+                prevDuration = d;
+            } else {
+                if (prevDuration.isDisjoint(d)) {
+                    result.add(prevDuration);
+                    prevDuration = d;
+                } else {
+                    prevDuration = DisabledDuration.mergeDuration(prevDuration, d);
+                }
+            }
+        }
+        if (prevDuration != null) {
+            result.add(prevDuration);
         }
 
         return result;
     }
 
-    private void addDisabledDuration(final List<DisabledDuration> result, final BlockingState firstBlocking, @Nullable final BlockingState firstNonBlocking) {
-        final DisabledDuration lastOne;
-        if (!result.isEmpty()) {
-            lastOne = result.get(result.size() - 1);
-        } else {
-            lastOne = null;
+
+    private static class BlockingStateNesting {
+
+        final List<DisabledDuration> result;
+
+        private int blockedNesting;
+        private BlockingState first;
+
+        private BlockingState lastOne;
+
+        public BlockingStateNesting() {
+            this.blockedNesting = 0;
+            this.first = null;
+            this.lastOne = null;
+            this.result = new ArrayList<DisabledDuration>();
         }
 
-        final DateTime startDate = firstBlocking.getEffectiveDate();
-        final DateTime endDate = firstNonBlocking == null ? null : firstNonBlocking.getEffectiveDate();
-        if (lastOne != null && lastOne.getEnd().compareTo(startDate) == 0) {
-            lastOne.setEnd(endDate);
-        } else if (endDate == null || Days.daysBetween(startDate, endDate).getDays() >= 1) {
-            // Don't disable for periods less than a day (see https://github.com/killbill/killbill/issues/267)
-            result.add(new DisabledDuration(startDate, endDate));
+        public int getBlockedNesting() {
+            return blockedNesting;
+        }
+
+        public BlockingState getFirst() {
+            return first;
+        }
+
+        public List<DisabledDuration> getResult() {
+            return result;
+        }
+
+        public BlockingState getLastOne() {
+            return lastOne;
+        }
+
+        public void setLastOne(final BlockingState lastOne) {
+            this.lastOne = lastOne;
+        }
+
+        public void setBlockedNesting(final int blockedNesting) {
+            this.blockedNesting = blockedNesting;
+        }
+
+        public void setFirst(final BlockingState input) {
+            first = input;
+        }
+
+        private void addDisabledDuration(final BlockingState firstBlocking, @Nullable final BlockingState firstNonBlocking) {
+
+            final DisabledDuration lastOne = result.isEmpty() ? null : result.get(result.size() - 1);
+
+            final DateTime startDate = firstBlocking.getEffectiveDate();
+            final DateTime endDate = firstNonBlocking == null ? null : firstNonBlocking.getEffectiveDate();
+
+
+            if (lastOne != null && lastOne.getEnd().compareTo(startDate) >= 0) {
+                lastOne.setEnd(endDate);
+            } else if (endDate == null || Days.daysBetween(startDate, endDate).getDays() >= 1) {
+                // Don't disable for periods less than a day (see https://github.com/killbill/killbill/issues/267)
+                result.add(new DisabledDuration(startDate, endDate));
+            }
         }
     }
 
+
     @VisibleForTesting
     static AtomicLong getGlobalTotalOrder() {
         return globaltotalOrder;
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 2d17bdc..61b9a97 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
@@ -594,7 +594,7 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
         final DateTime now = clock.getUTCNow();
         final BillingEvent event = new MockBillingEvent();
 
-        final BillingEvent result = blockingCalculator.createNewDisableEvent(now, event, null, internalCallContext);
+        final BillingEvent result = blockingCalculator.createNewDisableEvent(now, event, null);
         assertEquals(result.getBillCycleDayLocal(), event.getBillCycleDayLocal());
         assertEquals(result.getEffectiveDate(), now);
         assertEquals(result.getPlanPhase(), event.getPlanPhase());