Skip to content

Fix: SendImpressionEvent will return false when event is not sent #420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 27, 2021
Merged

Fix: SendImpressionEvent will return false when event is not sent #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa506dc
Fix: SendImpressionEvent will return false when event is not sent
mnoman09 Jan 27, 2021
0838a40
add more tests for decisionEventDispatched
jaeopt Jan 27, 2021
583b535
Update OptimizelyUserContextTest.java
jaeopt Jan 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,14 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
* @param flagKey It can either be empty if ruleType is experiment or it's feature key in case ruleType is feature-test or rollout
* @param ruleType It can either be experiment in case impression event is sent from activate or it's feature-test or rollout
*/
private void sendImpression(@Nonnull ProjectConfig projectConfig,
@Nullable Experiment experiment,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes,
@Nullable Variation variation,
@Nonnull String flagKey,
@Nonnull String ruleType,
@Nonnull boolean enabled) {
private boolean sendImpression(@Nonnull ProjectConfig projectConfig,
@Nullable Experiment experiment,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes,
@Nullable Variation variation,
@Nonnull String flagKey,
@Nonnull String ruleType,
@Nonnull boolean enabled) {

UserEvent userEvent = UserEventFactory.createImpressionEvent(
projectConfig,
Expand All @@ -275,7 +275,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
enabled);

if (userEvent == null) {
return;
return false;
}
eventProcessor.process(userEvent);
if (experiment != null) {
Expand All @@ -290,6 +290,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
experiment, userId, filteredAttributes, variation, impressionEvent);
notificationCenter.send(activateNotification);
}
return true;
}

//======== track calls ========//
Expand Down Expand Up @@ -1218,7 +1219,7 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user,
String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null;

if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) {
sendImpression(
decisionEventDispatched = sendImpression(
projectConfig,
flagDecision.experiment,
userId,
Expand All @@ -1227,7 +1228,6 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user,
key,
decisionSource.toString(),
flagEnabled);
decisionEventDispatched = true;
}

DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder()
Expand Down
108 changes: 107 additions & 1 deletion core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2020, Optimizely and contributors
* Copyright 2021, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -495,6 +495,112 @@ public void decide_doNotSendEvent_withOption() {
OptimizelyDecision decision = user.decide(flagKey, Arrays.asList(OptimizelyDecideOption.DISABLE_DECISION_EVENT));

assertEquals(decision.getVariationKey(), "variation_with_traffic");

Copy link
Contributor

@msohailhussain msohailhussain Jan 27, 2021

Choose a reason for hiding this comment

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

please update Copyright header.

// impression event not expected here
}

@Test
public void decide_sendEvent_featureTest_withSendFlagDecisionsOn() {
optimizely = new Optimizely.Builder()
.withDatafile(datafile)
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
.build();

Map<String, Object> attributes = Collections.singletonMap("gender", "f");
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);

optimizely.addDecisionNotificationHandler(
decisionNotification -> {
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
isListenerCalled = true;
});

String flagKey = "feature_2";
String experimentId = "10420810910";
String variationId = "10418551353";
isListenerCalled = false;
user.decide(flagKey);
assertTrue(isListenerCalled);

eventHandler.expectImpression(experimentId, variationId, userId, attributes);
}

@Test
public void decide_sendEvent_rollout_withSendFlagDecisionsOn() {
optimizely = new Optimizely.Builder()
.withDatafile(datafile)
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
.build();

Map<String, Object> attributes = Collections.singletonMap("gender", "f");
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);

optimizely.addDecisionNotificationHandler(
decisionNotification -> {
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
isListenerCalled = true;
});

String flagKey = "feature_3";
String experimentId = null;
String variationId = null;
isListenerCalled = false;
user.decide(flagKey);
assertTrue(isListenerCalled);

eventHandler.expectImpression(null, "", userId, attributes);
}

@Test
public void decide_sendEvent_featureTest_withSendFlagDecisionsOff() {
String datafileWithSendFlagDecisionsOff = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false");
optimizely = new Optimizely.Builder()
.withDatafile(datafileWithSendFlagDecisionsOff)
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
.build();

Map<String, Object> attributes = Collections.singletonMap("gender", "f");
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);

optimizely.addDecisionNotificationHandler(
decisionNotification -> {
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
isListenerCalled = true;
});

String flagKey = "feature_2";
String experimentId = "10420810910";
String variationId = "10418551353";
isListenerCalled = false;
user.decide(flagKey);
assertTrue(isListenerCalled);

eventHandler.expectImpression(experimentId, variationId, userId, attributes);
}

@Test
public void decide_sendEvent_rollout_withSendFlagDecisionsOff() {
String datafileWithSendFlagDecisionsOff = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false");
optimizely = new Optimizely.Builder()
.withDatafile(datafileWithSendFlagDecisionsOff)
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
.build();

Map<String, Object> attributes = Collections.singletonMap("gender", "f");
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);

optimizely.addDecisionNotificationHandler(
decisionNotification -> {
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), false);
isListenerCalled = true;
});

String flagKey = "feature_3";
isListenerCalled = false;
user.decide(flagKey);
assertTrue(isListenerCalled);

// impression event not expected here
}

// notifications
Expand Down