From aa506dcfbb6b2bf16e40ca2c2a7eec2aea58107f Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Wed, 27 Jan 2021 23:19:43 +0500 Subject: [PATCH 1/3] Fix: SendImpressionEvent will return false when event is not sent --- .../java/com/optimizely/ab/Optimizely.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 4440608b3..fb62ded7f 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -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 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 filteredAttributes, + @Nullable Variation variation, + @Nonnull String flagKey, + @Nonnull String ruleType, + @Nonnull boolean enabled) { UserEvent userEvent = UserEventFactory.createImpressionEvent( projectConfig, @@ -275,7 +275,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, enabled); if (userEvent == null) { - return; + return false; } eventProcessor.process(userEvent); if (experiment != null) { @@ -290,6 +290,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, experiment, userId, filteredAttributes, variation, impressionEvent); notificationCenter.send(activateNotification); } + return true; } //======== track calls ========// @@ -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, @@ -1227,7 +1228,6 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, key, decisionSource.toString(), flagEnabled); - decisionEventDispatched = true; } DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() From 0838a4080c46eb890deaf4efc7d59d79b4cfc87e Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Wed, 27 Jan 2021 13:35:00 -0800 Subject: [PATCH 2/3] add more tests for decisionEventDispatched --- .../ab/OptimizelyUserContextTest.java | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 5b4c29382..84a8ca5f0 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -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"); + + // 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 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 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 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 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 From 583b535fb87c3b6f035b7af538c5cf9602d910ba Mon Sep 17 00:00:00 2001 From: Jae Kim <45045038+jaeopt@users.noreply.github.com> Date: Wed, 27 Jan 2021 14:03:15 -0800 Subject: [PATCH 3/3] Update OptimizelyUserContextTest.java --- .../test/java/com/optimizely/ab/OptimizelyUserContextTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 84a8ca5f0..0ac8beedb 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -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.