Skip to content

Commit 06b128e

Browse files
authored
Fix: SendImpressionEvent will return false when event is not sent (#420)
1 parent c17ff5e commit 06b128e

File tree

2 files changed

+118
-12
lines changed

2 files changed

+118
-12
lines changed

core-api/src/main/java/com/optimizely/ab/Optimizely.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -255,14 +255,14 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
255255
* @param flagKey It can either be empty if ruleType is experiment or it's feature key in case ruleType is feature-test or rollout
256256
* @param ruleType It can either be experiment in case impression event is sent from activate or it's feature-test or rollout
257257
*/
258-
private void sendImpression(@Nonnull ProjectConfig projectConfig,
259-
@Nullable Experiment experiment,
260-
@Nonnull String userId,
261-
@Nonnull Map<String, ?> filteredAttributes,
262-
@Nullable Variation variation,
263-
@Nonnull String flagKey,
264-
@Nonnull String ruleType,
265-
@Nonnull boolean enabled) {
258+
private boolean sendImpression(@Nonnull ProjectConfig projectConfig,
259+
@Nullable Experiment experiment,
260+
@Nonnull String userId,
261+
@Nonnull Map<String, ?> filteredAttributes,
262+
@Nullable Variation variation,
263+
@Nonnull String flagKey,
264+
@Nonnull String ruleType,
265+
@Nonnull boolean enabled) {
266266

267267
UserEvent userEvent = UserEventFactory.createImpressionEvent(
268268
projectConfig,
@@ -275,7 +275,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
275275
enabled);
276276

277277
if (userEvent == null) {
278-
return;
278+
return false;
279279
}
280280
eventProcessor.process(userEvent);
281281
if (experiment != null) {
@@ -290,6 +290,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig,
290290
experiment, userId, filteredAttributes, variation, impressionEvent);
291291
notificationCenter.send(activateNotification);
292292
}
293+
return true;
293294
}
294295

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

12201221
if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) {
1221-
sendImpression(
1222+
decisionEventDispatched = sendImpression(
12221223
projectConfig,
12231224
flagDecision.experiment,
12241225
userId,
@@ -1227,7 +1228,6 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user,
12271228
key,
12281229
decisionSource.toString(),
12291230
flagEnabled);
1230-
decisionEventDispatched = true;
12311231
}
12321232

12331233
DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder()

core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2020, Optimizely and contributors
3+
* Copyright 2021, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -495,6 +495,112 @@ public void decide_doNotSendEvent_withOption() {
495495
OptimizelyDecision decision = user.decide(flagKey, Arrays.asList(OptimizelyDecideOption.DISABLE_DECISION_EVENT));
496496

497497
assertEquals(decision.getVariationKey(), "variation_with_traffic");
498+
499+
// impression event not expected here
500+
}
501+
502+
@Test
503+
public void decide_sendEvent_featureTest_withSendFlagDecisionsOn() {
504+
optimizely = new Optimizely.Builder()
505+
.withDatafile(datafile)
506+
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
507+
.build();
508+
509+
Map<String, Object> attributes = Collections.singletonMap("gender", "f");
510+
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
511+
512+
optimizely.addDecisionNotificationHandler(
513+
decisionNotification -> {
514+
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
515+
isListenerCalled = true;
516+
});
517+
518+
String flagKey = "feature_2";
519+
String experimentId = "10420810910";
520+
String variationId = "10418551353";
521+
isListenerCalled = false;
522+
user.decide(flagKey);
523+
assertTrue(isListenerCalled);
524+
525+
eventHandler.expectImpression(experimentId, variationId, userId, attributes);
526+
}
527+
528+
@Test
529+
public void decide_sendEvent_rollout_withSendFlagDecisionsOn() {
530+
optimizely = new Optimizely.Builder()
531+
.withDatafile(datafile)
532+
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
533+
.build();
534+
535+
Map<String, Object> attributes = Collections.singletonMap("gender", "f");
536+
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
537+
538+
optimizely.addDecisionNotificationHandler(
539+
decisionNotification -> {
540+
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
541+
isListenerCalled = true;
542+
});
543+
544+
String flagKey = "feature_3";
545+
String experimentId = null;
546+
String variationId = null;
547+
isListenerCalled = false;
548+
user.decide(flagKey);
549+
assertTrue(isListenerCalled);
550+
551+
eventHandler.expectImpression(null, "", userId, attributes);
552+
}
553+
554+
@Test
555+
public void decide_sendEvent_featureTest_withSendFlagDecisionsOff() {
556+
String datafileWithSendFlagDecisionsOff = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false");
557+
optimizely = new Optimizely.Builder()
558+
.withDatafile(datafileWithSendFlagDecisionsOff)
559+
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
560+
.build();
561+
562+
Map<String, Object> attributes = Collections.singletonMap("gender", "f");
563+
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
564+
565+
optimizely.addDecisionNotificationHandler(
566+
decisionNotification -> {
567+
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), true);
568+
isListenerCalled = true;
569+
});
570+
571+
String flagKey = "feature_2";
572+
String experimentId = "10420810910";
573+
String variationId = "10418551353";
574+
isListenerCalled = false;
575+
user.decide(flagKey);
576+
assertTrue(isListenerCalled);
577+
578+
eventHandler.expectImpression(experimentId, variationId, userId, attributes);
579+
}
580+
581+
@Test
582+
public void decide_sendEvent_rollout_withSendFlagDecisionsOff() {
583+
String datafileWithSendFlagDecisionsOff = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false");
584+
optimizely = new Optimizely.Builder()
585+
.withDatafile(datafileWithSendFlagDecisionsOff)
586+
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
587+
.build();
588+
589+
Map<String, Object> attributes = Collections.singletonMap("gender", "f");
590+
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
591+
592+
optimizely.addDecisionNotificationHandler(
593+
decisionNotification -> {
594+
Assert.assertEquals(decisionNotification.getDecisionInfo().get(DECISION_EVENT_DISPATCHED), false);
595+
isListenerCalled = true;
596+
});
597+
598+
String flagKey = "feature_3";
599+
isListenerCalled = false;
600+
user.decide(flagKey);
601+
assertTrue(isListenerCalled);
602+
603+
// impression event not expected here
498604
}
499605

500606
// notifications

0 commit comments

Comments
 (0)