Skip to content

Commit

Permalink
entitlement: compute blocking states baed on all services and not onl…
Browse files Browse the repository at this point in the history 
…y ENTITLEMENT_SERVICE_NAME. That is required for block() / unblock() apis to behave correctly
  • Loading branch information
@sbrossie
sbrossie committed Oct 31, 2015
1 parent ca75439 commit fd7c0c1
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions ...ement/src/main/java/org/killbill/billing/entitlement/engine/core/DefaultEventsStream.java
View file
Expand Up @@ -446,18 +446,17 @@ private void computeStateForEntitlement() {
} }


private void computeEntitlementBlockingStates() { private void computeEntitlementBlockingStates() {
subscriptionEntitlementStates = filterBlockingStatesForEntitlementService(BlockingStateType.SUBSCRIPTION, subscription.getId()); subscriptionEntitlementStates = filterBlockingStatesForTypeAndId(BlockingStateType.SUBSCRIPTION, subscription.getId());
bundleEntitlementStates = filterBlockingStatesForEntitlementService(BlockingStateType.SUBSCRIPTION_BUNDLE, subscription.getBundleId()); bundleEntitlementStates = filterBlockingStatesForTypeAndId(BlockingStateType.SUBSCRIPTION_BUNDLE, subscription.getBundleId());
accountEntitlementStates = filterBlockingStatesForEntitlementService(BlockingStateType.ACCOUNT, account.getId()); accountEntitlementStates = filterBlockingStatesForTypeAndId(BlockingStateType.ACCOUNT, account.getId());
} }


private List<BlockingState> filterBlockingStatesForEntitlementService(final BlockingStateType blockingStateType, @Nullable final UUID blockableId) { private List<BlockingState> filterBlockingStatesForTypeAndId(final BlockingStateType blockingStateType, @Nullable final UUID blockableId) {
return ImmutableList.<BlockingState>copyOf(Iterables.<BlockingState>filter(blockingStates, return ImmutableList.<BlockingState>copyOf(Iterables.<BlockingState>filter(blockingStates,
new Predicate<BlockingState>() { new Predicate<BlockingState>() {
@Override @Override
public boolean apply(final BlockingState input) { public boolean apply(final BlockingState input) {
return blockingStateType.equals(input.getType()) && return blockingStateType.equals(input.getType()) &&
EntitlementService.ENTITLEMENT_SERVICE_NAME.equals(input.getService()) &&
input.getBlockedId().equals(blockableId); input.getBlockedId().equals(blockableId);
} }
})); }));
Expand Down

2 comments on commit fd7c0c1

@pierre
Copy link
Member

@pierre pierre commented on fd7c0c1 Nov 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this change because now any service can impact the entitlement status as seen by the entitlement system? For example, any service can insert an event with state name ENT_CANCELLED and mark the entitlement as cancelled (or change the entitlementEffectiveEndDate, etc.).

@sbrossie
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue comes from the method computeBlockingAggregator which is trying to compute the blockingAggregator (that is across all services). If we filter only per-service level, the computed blockingAggregator ends up being wrong (because it only accounts for entitlement service).

That being i think you are correct that something is off (at the very least the naming of variables and method should be changed). I'll take a second pass.

Please sign in to comment.