killbill-memoizeit
Changes
entitlement/src/main/java/com/ning/billing/entitlement/engine/core/EventsStreamBuilder.java 7(+3 -4)
Details
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/dao/ProxyBlockingStateDao.java b/entitlement/src/main/java/com/ning/billing/entitlement/dao/ProxyBlockingStateDao.java
index 87ffd36..bef2260 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/dao/ProxyBlockingStateDao.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/dao/ProxyBlockingStateDao.java
@@ -18,6 +18,7 @@ package com.ning.billing.entitlement.dao;
import java.util.Collection;
import java.util.Comparator;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -41,12 +42,12 @@ import com.ning.billing.entitlement.EventsStream;
import com.ning.billing.entitlement.api.BlockingState;
import com.ning.billing.entitlement.api.BlockingStateType;
import com.ning.billing.entitlement.api.DefaultEntitlementApi;
-import com.ning.billing.entitlement.api.Entitlement.EntitlementState;
import com.ning.billing.entitlement.api.EntitlementApiException;
import com.ning.billing.entitlement.engine.core.EventsStreamBuilder;
import com.ning.billing.subscription.api.SubscriptionBase;
import com.ning.billing.subscription.api.SubscriptionBaseInternalApi;
import com.ning.billing.util.cache.CacheControllerDispatcher;
+import com.ning.billing.util.customfield.ShouldntHappenException;
import com.ning.billing.util.dao.NonEntityDao;
import com.ning.billing.util.entity.Pagination;
@@ -60,7 +61,101 @@ public class ProxyBlockingStateDao implements BlockingStateDao {
private static final Logger log = LoggerFactory.getLogger(ProxyBlockingStateDao.class);
// Ordering is critical here, especially for Junction
- public static final Ordering<BlockingState> BLOCKING_STATE_ORDERING = Ordering.<BlockingState>from(new Comparator<BlockingState>() {
+ public static List<BlockingState> sortedCopy(final Iterable<BlockingState> blockingStates) {
+ final List<BlockingState> blockingStatesSomewhatSorted = BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED.immutableSortedCopy(blockingStates);
+
+ final List<BlockingState> result = new LinkedList<BlockingState>();
+
+ // Take care of the ties
+ final Iterator<BlockingState> iterator = blockingStatesSomewhatSorted.iterator();
+ BlockingState prev = null;
+ while (iterator.hasNext()) {
+ final BlockingState current = iterator.next();
+ if (iterator.hasNext()) {
+ final BlockingState next = iterator.next();
+ if (prev != null && current.getEffectiveDate().equals(next.getEffectiveDate()) && current.getBlockedId().equals(next.getBlockedId())) {
+ // Same date, same blockable id
+
+ // Make sure block billing transitions are respected first
+ BlockingState prevCandidate = insertTiedBlockingStatesInTheRightOrder(result, current, next, prev.isBlockBilling(), current.isBlockBilling(), next.isBlockBilling());
+ if (prevCandidate == null) {
+ // Then respect block entitlement transitions
+ prevCandidate = insertTiedBlockingStatesInTheRightOrder(result, current, next, prev.isBlockEntitlement(), current.isBlockEntitlement(), next.isBlockEntitlement());
+ if (prevCandidate == null) {
+ // And finally block changes transitions
+ prevCandidate = insertTiedBlockingStatesInTheRightOrder(result, current, next, prev.isBlockChange(), current.isBlockChange(), next.isBlockChange());
+ if (prevCandidate == null) {
+ // Trust the creation date (see BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED below)
+ result.add(current);
+ result.add(next);
+ prev = next;
+ } else {
+ prev = prevCandidate;
+ }
+ } else {
+ prev = prevCandidate;
+ }
+ } else {
+ prev = prevCandidate;
+ }
+ } else {
+ result.add(current);
+ result.add(next);
+ prev = next;
+ }
+ } else {
+ // End of the list
+ result.add(current);
+ }
+ }
+
+ return result;
+ }
+
+ private static BlockingState insertTiedBlockingStatesInTheRightOrder(final Collection<BlockingState> result,
+ final BlockingState current,
+ final BlockingState next,
+ final boolean prevBlocked,
+ final boolean currentBlocked,
+ final boolean nextBlocked) {
+ final BlockingState prev;
+
+ if (prevBlocked && currentBlocked && nextBlocked) {
+ // Tricky use case, bail
+ return null;
+ } else if (prevBlocked && currentBlocked && !nextBlocked) {
+ result.add(next);
+ result.add(current);
+ prev = current;
+ } else if (prevBlocked && !currentBlocked && nextBlocked) {
+ result.add(current);
+ result.add(next);
+ prev = next;
+ } else if (prevBlocked && !currentBlocked && !nextBlocked) {
+ // Tricky use case, bail
+ return null;
+ } else if (!prevBlocked && currentBlocked && nextBlocked) {
+ // Tricky use case, bail
+ return null;
+ } else if (!prevBlocked && currentBlocked && !nextBlocked) {
+ result.add(current);
+ result.add(next);
+ prev = next;
+ } else if (!prevBlocked && !currentBlocked && nextBlocked) {
+ result.add(next);
+ result.add(current);
+ prev = current;
+ } else if (!prevBlocked && !currentBlocked && !nextBlocked) {
+ // Tricky use case, bail
+ return null;
+ } else {
+ throw new ShouldntHappenException("Marker exception for code clarity");
+ }
+
+ return prev;
+ }
+
+ private static final Ordering<BlockingState> BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED = Ordering.<BlockingState>from(new Comparator<BlockingState>() {
@Override
public int compare(final BlockingState o1, final BlockingState o2) {
// effective_date column NOT NULL
@@ -72,23 +167,7 @@ public class ProxyBlockingStateDao implements BlockingStateDao {
if (blockableIdComparison != 0) {
return blockableIdComparison;
} else {
- // Same date, same blockable id - make sure billing transitions are respected first (assume block -> clear transitions)
- if (!o1.isBlockBilling() && o2.isBlockBilling()) {
- return 1;
- } else if (o1.isBlockBilling() && !o2.isBlockBilling()) {
- return -1;
- }
-
- // Then respect other blocking states
- if ((!o1.isBlockChange() && o2.isBlockChange()) ||
- (!o1.isBlockEntitlement() && o2.isBlockEntitlement())) {
- return 1;
- } else if ((o1.isBlockChange() && !o2.isBlockChange()) ||
- (o1.isBlockEntitlement() && !o2.isBlockEntitlement())) {
- return -1;
- }
-
- // Otherwise, just respect the created date
+ // Same date, same blockable id, just respect the created date for now (see sortedCopyOf method above)
return o1.getCreatedDate().compareTo(o2.getCreatedDate());
}
}
@@ -254,7 +333,7 @@ public class ProxyBlockingStateDao implements BlockingStateDao {
}
// Return the sorted list
- return BLOCKING_STATE_ORDERING.immutableSortedCopy(blockingStatesOnDiskCopy);
+ return sortedCopy(blockingStatesOnDiskCopy);
}
private BlockingState findEntitlementCancellationBlockingState(@Nullable final UUID blockedId, final Iterable<BlockingState> blockingStatesOnDisk) {
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/engine/core/EventsStreamBuilder.java b/entitlement/src/main/java/com/ning/billing/entitlement/engine/core/EventsStreamBuilder.java
index a051f1a..1dcf932 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/engine/core/EventsStreamBuilder.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/engine/core/EventsStreamBuilder.java
@@ -46,6 +46,7 @@ import com.ning.billing.entitlement.api.svcs.DefaultAccountEventsStreams;
import com.ning.billing.entitlement.block.BlockingChecker;
import com.ning.billing.entitlement.dao.DefaultBlockingStateDao;
import com.ning.billing.entitlement.dao.OptimizedProxyBlockingStateDao;
+import com.ning.billing.entitlement.dao.ProxyBlockingStateDao;
import com.ning.billing.subscription.api.SubscriptionBase;
import com.ning.billing.subscription.api.SubscriptionBaseInternalApi;
import com.ning.billing.subscription.api.user.SubscriptionBaseApiException;
@@ -60,8 +61,6 @@ import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
-import static com.ning.billing.entitlement.dao.ProxyBlockingStateDao.BLOCKING_STATE_ORDERING;
-
@Singleton
public class EventsStreamBuilder {
@@ -207,7 +206,7 @@ public class EventsStreamBuilder {
final Collection<BlockingState> blockingStateSet = new LinkedHashSet<BlockingState>(accountBlockingStates);
blockingStateSet.addAll(bundleBlockingStates);
blockingStateSet.addAll(subscriptionBlockingStates);
- final List<BlockingState> blockingStates = BLOCKING_STATE_ORDERING.immutableSortedCopy(blockingStateSet);
+ final List<BlockingState> blockingStates = ProxyBlockingStateDao.sortedCopy(blockingStateSet);
final EventsStream eventStream = buildForEntitlement(account, bundle, baseSubscription, subscription, allSubscriptionsForBundle, blockingStates, internalTenantContext);
entitlementsPerBundle.get(bundleId).add(eventStream);
@@ -315,7 +314,7 @@ public class EventsStreamBuilder {
final Collection<BlockingState> blockingStateSet = new LinkedHashSet<BlockingState>(accountBlockingStates);
blockingStateSet.addAll(bundleBlockingStates);
blockingStateSet.addAll(subscriptionBlockingStates);
- final List<BlockingState> blockingStates = BLOCKING_STATE_ORDERING.immutableSortedCopy(blockingStateSet);
+ final List<BlockingState> blockingStates = ProxyBlockingStateDao.sortedCopy(blockingStateSet);
return buildForEntitlement(account, bundle, baseSubscription, subscription, allSubscriptionsForBundle, blockingStates, internalTenantContext);
}
diff --git a/entitlement/src/test/java/com/ning/billing/entitlement/api/TestDefaultSubscriptionApi.java b/entitlement/src/test/java/com/ning/billing/entitlement/api/TestDefaultSubscriptionApi.java
index 0aed9fc..2117962 100644
--- a/entitlement/src/test/java/com/ning/billing/entitlement/api/TestDefaultSubscriptionApi.java
+++ b/entitlement/src/test/java/com/ning/billing/entitlement/api/TestDefaultSubscriptionApi.java
@@ -46,14 +46,16 @@ public class TestDefaultSubscriptionApi extends EntitlementTestSuiteWithEmbedded
final Account account = accountApi.createAccount(getAccountData(7), callContext);
final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Shotgun", ProductCategory.BASE, BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null);
testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.CREATE, NextEvent.BLOCK);
- final Entitlement entitlement1 = entitlementApi.createBaseEntitlement(account.getId(), spec, UUID.fromString("d87c78b4-c6de-4387-8a3b-4a5850dc29fc").toString(), initialDate, callContext);
- final Entitlement entitlement2 = entitlementApi.createBaseEntitlement(account.getId(), spec, UUID.fromString("c56245e7-11a5-4a41-8854-3d31e24bcdcc").toString(), initialDate, callContext);
+ // Hardcode the UUIDs to have a predictable ordering
+ final Entitlement entitlement1 = entitlementApi.createBaseEntitlement(account.getId(), spec, UUID.fromString("a87c78b4-c6de-4387-8a3b-4a5850dc29fc").toString(), initialDate, callContext);
+ final Entitlement entitlement2 = entitlementApi.createBaseEntitlement(account.getId(), spec, UUID.fromString("b56245e7-11a5-4a41-8854-3d31e24bcdcc").toString(), initialDate, callContext);
entitlementUtils.setBlockingStateAndPostBlockingTransitionEvent(new DefaultBlockingState(account.getId(), BlockingStateType.ACCOUNT, "stateName", "service", false, false, false, clock.getUTCNow()),
internalCallContextFactory.createInternalCallContext(account.getId(), callContext));
assertListenerStatus();
final List<SubscriptionBundle> bundles = subscriptionApi.getSubscriptionBundlesForAccountId(account.getId(), callContext);
Assert.assertEquals(bundles.size(), 2);
+
// This will test the ordering as well
subscriptionBundleChecker(bundles, initialDate, entitlement1, 0);
subscriptionBundleChecker(bundles, initialDate, entitlement2, 1);
diff --git a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBlockingCalculator.java b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBlockingCalculator.java
index dbf0294..71a5b22 100644
--- a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBlockingCalculator.java
+++ b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestBlockingCalculator.java
@@ -711,7 +711,7 @@ public class TestBlockingCalculator extends JunctionTestSuiteNoDB {
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. See also TestDefaultInternalBillingApi.
+ // behavior in ProxyBlockingStateDao#BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED. See also TestDefaultInternalBillingApi.
blockingEvents = new ArrayList<BlockingState>();
blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, DISABLED_BUNDLE, "test", true, true, true, now.plusDays(1)));
blockingEvents.add(new DefaultBlockingState(ovdId, BlockingStateType.SUBSCRIPTION_BUNDLE, CLEAR_BUNDLE, "test", false, false, false, now.plusDays(2)));
diff --git a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
index 6aac83a..e99269a 100644
--- a/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
+++ b/junction/src/test/java/com/ning/billing/junction/plumbing/billing/TestDefaultInternalBillingApi.java
@@ -48,7 +48,7 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
//
// Pierre, why do we have invocationCount > 0 here?
//
- // This test will exercise ProxyBlockingStateDao#BLOCKING_STATE_ORDERING - unfortunately, for some reason,
+ // This test will exercise ProxyBlockingStateDao#BLOCKING_STATE_ORDERING_WITH_TIES_UNHANDLED - unfortunately, for some reason,
// the ordering doesn't seem deterministic. In some scenarii,
// BlockingState(idA, effectiveDate1, BLOCK), BlockingState(idA, effectiveDate2, CLEAR), BlockingState(idB, effectiveDate2, BLOCK), BlockingState(idB, effectiveDate3, CLEAR)
// is ordered
@@ -200,4 +200,70 @@ public class TestDefaultInternalBillingApi extends JunctionTestSuiteWithEmbedded
Assert.assertEquals(events.get(6).getTransitionType(), SubscriptionBaseTransitionType.END_BILLING_DISABLED);
Assert.assertEquals(events.get(6).getEffectiveDate(), block5Date);
}
+
+ // 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 {
+ final LocalDate initialDate = new LocalDate(2013, 8, 7);
+ clock.setDay(initialDate);
+
+ final Account account = accountApi.createAccount(getAccountData(7), callContext);
+ final InternalCallContext internalCallContext = internalCallContextFactory.createInternalCallContext(account.getId(), callContext);
+
+ testListener.pushExpectedEvent(NextEvent.CREATE);
+ final PlanPhaseSpecifier spec = new PlanPhaseSpecifier("Shotgun", ProductCategory.BASE, BillingPeriod.MONTHLY, PriceListSet.DEFAULT_PRICELIST_NAME, null);
+ final Entitlement entitlement = entitlementApi.createBaseEntitlement(account.getId(), spec, account.getExternalKey(), initialDate, callContext);
+ final SubscriptionBase subscription = subscriptionInternalApi.getSubscriptionFromId(entitlement.getId(), internalCallContext);
+ assertListenerStatus();
+
+ final DateTime block1Date = clock.getUTCNow();
+ testListener.pushExpectedEvents(NextEvent.BLOCK);
+ final DefaultBlockingState state1 = new DefaultBlockingState(account.getId(),
+ BlockingStateType.ACCOUNT,
+ DefaultEntitlementApi.ENT_STATE_BLOCKED,
+ EntitlementService.ENTITLEMENT_SERVICE_NAME,
+ true,
+ true,
+ true,
+ block1Date);
+ blockingInternalApi.setBlockingState(state1, internalCallContext);
+
+ clock.addDays(1);
+
+ final DateTime block2Date = clock.getUTCNow();
+ testListener.pushExpectedEvents(NextEvent.BLOCK, NextEvent.BLOCK);
+ final DefaultBlockingState state2 = new DefaultBlockingState(account.getId(),
+ BlockingStateType.ACCOUNT,
+ DefaultEntitlementApi.ENT_STATE_CLEAR,
+ EntitlementService.ENTITLEMENT_SERVICE_NAME,
+ false,
+ false,
+ false,
+ block2Date);
+ blockingInternalApi.setBlockingState(state2, internalCallContext);
+ // Same date
+ final DefaultBlockingState state3 = new DefaultBlockingState(account.getId(),
+ BlockingStateType.ACCOUNT,
+ DefaultEntitlementApi.ENT_STATE_BLOCKED,
+ EntitlementService.ENTITLEMENT_SERVICE_NAME,
+ true,
+ true,
+ true,
+ block2Date);
+ blockingInternalApi.setBlockingState(state3, internalCallContext);
+ assertListenerStatus();
+
+ // Nothing should happen
+ 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(), 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);
+ }
}