From b55891853c9386d47aac1c01f03964b143a0f8c9 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Wed, 30 Sep 2020 23:16:05 +0500 Subject: [PATCH 1/7] Added Flag decision support - Added unit tests --- .../java/com/optimizely/ab/Optimizely.java | 42 +++++---- .../ab/config/DatafileProjectConfig.java | 7 ++ .../optimizely/ab/config/ProjectConfig.java | 2 + .../parser/DatafileGsonDeserializer.java | 7 +- .../parser/DatafileJacksonDeserializer.java | 7 +- .../ab/config/parser/JsonConfigParser.java | 4 + .../config/parser/JsonSimpleConfigParser.java | 4 + .../ab/event/internal/EventFactory.java | 3 +- .../ab/event/internal/ImpressionEvent.java | 19 +++- .../ab/event/internal/UserEventFactory.java | 46 +++++++-- .../ab/event/internal/payload/Decision.java | 18 +++- .../internal/payload/DecisionMetadata.java | 93 +++++++++++++++++++ .../com/optimizely/ab/OptimizelyTest.java | 60 ++++++++++-- .../ab/config/ValidProjectConfigV4.java | 4 +- .../ab/event/internal/EventFactoryTest.java | 18 +++- .../event/internal/UserEventFactoryTest.java | 25 ++++- .../OptimizelyConfigServiceTest.java | 1 + .../config/valid-project-config-v4.json | 1 + 18 files changed, 317 insertions(+), 44 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java 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 e32a39cb5..d39cfafe4 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -216,7 +216,7 @@ private Variation activate(@Nullable ProjectConfig projectConfig, return null; } - sendImpression(projectConfig, experiment, userId, copiedAttributes, variation); + sendImpression(projectConfig, experiment, userId, copiedAttributes, variation, experiment.getKey(), "experiment"); return variation; } @@ -225,22 +225,26 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map filteredAttributes, - @Nonnull Variation variation) { - if (!experiment.isRunning()) { - logger.info("Experiment has \"Launched\" status so not dispatching event during activation."); - return; - } + @Nonnull Variation variation, + @Nonnull String flagKey, + @Nonnull String flagType) { UserEvent userEvent = UserEventFactory.createImpressionEvent( projectConfig, experiment, variation, userId, - filteredAttributes); + filteredAttributes, + flagKey, + flagType); + if (userEvent == null) { + return; + } eventProcessor.process(userEvent); - logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); - + if (experiment != null) { + logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); + } // Kept For backwards compatibility. // This notification is deprecated and the new DecisionNotifications // are sent via their respective method calls. @@ -386,16 +390,22 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig); Boolean featureEnabled = false; SourceInfo sourceInfo = new RolloutSourceInfo(); + if (featureDecision.decisionSource != null) { + decisionSource = featureDecision.decisionSource; + } + sendImpression( + projectConfig, + featureDecision.experiment, + userId, + copiedAttributes, + featureDecision.variation, + featureKey, + decisionSource.toString()); if (featureDecision.variation != null) { + // This information is only necessary for feature tests. + // For rollouts experiments and variations are an implementation detail only. if (featureDecision.decisionSource.equals(FeatureDecision.DecisionSource.FEATURE_TEST)) { - sendImpression( - projectConfig, - featureDecision.experiment, - userId, - copiedAttributes, - featureDecision.variation); - decisionSource = featureDecision.decisionSource; sourceInfo = new FeatureTestSourceInfo(featureDecision.experiment.getKey(), featureDecision.variation.getKey()); } else { logger.info("The user \"{}\" is not included in an experiment for feature \"{}\".", diff --git a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java index 0b188f064..786d13a2c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java @@ -61,6 +61,7 @@ public class DatafileProjectConfig implements ProjectConfig { private final String revision; private final String version; private final boolean anonymizeIP; + private final boolean sendFlagDecisions; private final Boolean botFiltering; private final List attributes; private final List audiences; @@ -103,6 +104,7 @@ public DatafileProjectConfig(String accountId, String projectId, String version, this( accountId, anonymizeIP, + false, null, projectId, revision, @@ -121,6 +123,7 @@ public DatafileProjectConfig(String accountId, String projectId, String version, // v4 constructor public DatafileProjectConfig(String accountId, boolean anonymizeIP, + boolean sendFlagDecisions, Boolean botFiltering, String projectId, String revision, @@ -139,6 +142,7 @@ public DatafileProjectConfig(String accountId, this.version = version; this.revision = revision; this.anonymizeIP = anonymizeIP; + this.sendFlagDecisions = sendFlagDecisions; this.botFiltering = botFiltering; this.attributes = Collections.unmodifiableList(attributes); @@ -322,6 +326,9 @@ public String getRevision() { return revision; } + @Override + public boolean getSendFlagDecisions() { return sendFlagDecisions; } + @Override public boolean getAnonymizeIP() { return anonymizeIP; diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java index 7f83e30b8..5a85fbd4e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java @@ -55,6 +55,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey, String getRevision(); + boolean getSendFlagDecisions(); + boolean getAnonymizeIP(); Boolean getBotFiltering(); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java index f0b0f50bd..99ab71b78 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, Optimizely and contributors + * Copyright 2016-2020, 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. @@ -83,9 +83,11 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa anonymizeIP = jsonObject.get("anonymizeIP").getAsBoolean(); } + List featureFlags = null; List rollouts = null; Boolean botFiltering = null; + boolean sendFlagDecisions = false; if (datafileVersion >= Integer.parseInt(DatafileProjectConfig.Version.V4.toString())) { Type featureFlagsType = new TypeToken>() { }.getType(); @@ -95,11 +97,14 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa rollouts = context.deserialize(jsonObject.get("rollouts").getAsJsonArray(), rolloutsType); if (jsonObject.has("botFiltering")) botFiltering = jsonObject.get("botFiltering").getAsBoolean(); + if (jsonObject.has("sendFlagDecisions")) + sendFlagDecisions = jsonObject.get("sendFlagDecisions").getAsBoolean(); } return new DatafileProjectConfig( accountId, anonymizeIP, + sendFlagDecisions, botFiltering, projectId, revision, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java index ab0455d9c..06ae5b1a9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, Optimizely and contributors + * Copyright 2016-2020, 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. @@ -64,17 +64,22 @@ public DatafileProjectConfig deserialize(JsonParser parser, DeserializationConte List featureFlags = null; List rollouts = null; Boolean botFiltering = null; + boolean sendFlagDecisions = false; if (datafileVersion >= Integer.parseInt(DatafileProjectConfig.Version.V4.toString())) { featureFlags = JacksonHelpers.arrayNodeToList(node.get("featureFlags"), FeatureFlag.class, codec); rollouts = JacksonHelpers.arrayNodeToList(node.get("rollouts"), Rollout.class, codec); if (node.hasNonNull("botFiltering")) { botFiltering = node.get("botFiltering").asBoolean(); } + if (node.hasNonNull("sendFlagDecisions")) { + sendFlagDecisions = node.get("sendFlagDecisions").asBoolean(); + } } return new DatafileProjectConfig( accountId, anonymizeIP, + sendFlagDecisions, botFiltering, projectId, revision, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index ad0d971bd..5f707cb69 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -73,16 +73,20 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse List featureFlags = null; List rollouts = null; Boolean botFiltering = null; + boolean sendFlagDecisions = false; if (datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString())) { featureFlags = parseFeatureFlags(rootObject.getJSONArray("featureFlags")); rollouts = parseRollouts(rootObject.getJSONArray("rollouts")); if (rootObject.has("botFiltering")) botFiltering = rootObject.getBoolean("botFiltering"); + if (rootObject.has("sendFlagDecisions")) + sendFlagDecisions = rootObject.getBoolean("sendFlagDecisions"); } return new DatafileProjectConfig( accountId, anonymizeIP, + sendFlagDecisions, botFiltering, projectId, revision, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index b6236ffa7..b5238a356 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -80,16 +80,20 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse List featureFlags = null; List rollouts = null; Boolean botFiltering = null; + boolean sendFlagDecisions = false; if (datafileVersion >= Integer.parseInt(DatafileProjectConfig.Version.V4.toString())) { featureFlags = parseFeatureFlags((JSONArray) rootObject.get("featureFlags")); rollouts = parseRollouts((JSONArray) rootObject.get("rollouts")); if (rootObject.containsKey("botFiltering")) botFiltering = (Boolean) rootObject.get("botFiltering"); + if (rootObject.containsKey("sendFlagDecisions")) + sendFlagDecisions = (Boolean) rootObject.get("sendFlagDecisions"); } return new DatafileProjectConfig( accountId, anonymizeIP, + sendFlagDecisions, botFiltering, projectId, revision, diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java index 0aee045c5..5a881128d 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, Optimizely and contributors + * Copyright 2016-2020, 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. @@ -99,6 +99,7 @@ private static Visitor createVisitor(ImpressionEvent impressionEvent) { .setCampaignId(impressionEvent.getLayerId()) .setExperimentId(impressionEvent.getExperimentId()) .setVariationId(impressionEvent.getVariationId()) + .setMetadata(impressionEvent.getMetadata()) .setIsCampaignHoldback(false) .build(); diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/ImpressionEvent.java b/core-api/src/main/java/com/optimizely/ab/event/internal/ImpressionEvent.java index 510069fa2..38f9dc905 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/ImpressionEvent.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/ImpressionEvent.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019, Optimizely and contributors + * Copyright 2019-2020, 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. @@ -16,6 +16,8 @@ */ package com.optimizely.ab.event.internal; +import com.optimizely.ab.event.internal.payload.DecisionMetadata; + import java.util.StringJoiner; /** @@ -28,19 +30,22 @@ public class ImpressionEvent extends BaseEvent implements UserEvent { private final String experimentKey; private final String variationKey; private final String variationId; + private final DecisionMetadata metadata; private ImpressionEvent(UserContext userContext, String layerId, String experimentId, String experimentKey, String variationKey, - String variationId) { + String variationId, + DecisionMetadata metadata) { this.userContext = userContext; this.layerId = layerId; this.experimentId = experimentId; this.experimentKey = experimentKey; this.variationKey = variationKey; this.variationId = variationId; + this.metadata = metadata; } @Override @@ -68,6 +73,8 @@ public String getVariationId() { return variationId; } + public DecisionMetadata getMetadata() { return metadata; } + public static class Builder { private UserContext userContext; @@ -76,6 +83,7 @@ public static class Builder { private String experimentKey; private String variationKey; private String variationId; + private DecisionMetadata metadata; public Builder withUserContext(UserContext userContext) { this.userContext = userContext; @@ -107,8 +115,13 @@ public Builder withVariationId(String variationId) { return this; } + public Builder withMetadata(DecisionMetadata metadata) { + this.metadata = metadata; + return this; + } + public ImpressionEvent build() { - return new ImpressionEvent(userContext, layerId, experimentId, experimentKey, variationKey, variationId); + return new ImpressionEvent(userContext, layerId, experimentId, experimentKey, variationKey, variationId, metadata); } } diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index da741979c..3bf78c0b0 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, Optimizely and contributors + * Copyright 2016-2020, 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. @@ -16,9 +16,11 @@ */ package com.optimizely.ab.event.internal; +import com.optimizely.ab.bucketing.FeatureDecision; import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.internal.EventTagUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,7 +35,30 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje @Nonnull Experiment activatedExperiment, @Nonnull Variation variation, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes, + @Nonnull String flagKey, + @Nonnull String flagType) { + + if ((FeatureDecision.DecisionSource.ROLLOUT.equals(flagType) || variation == null) && !projectConfig.getSendFlagDecisions()) + { + return null; + } + + String variationKey = null; + String variationID = null; + if (variation != null) { + variationKey = variation.getKey(); + variationID = variation.getId(); + } + + String layerID = null; + String experimentId = null; + String experimentKey = null; + if (activatedExperiment != null) { + layerID = activatedExperiment.getLayerId(); + experimentId = activatedExperiment.getId(); + experimentKey = activatedExperiment.getKey(); + } UserContext userContext = new UserContext.Builder() .withUserId(userId) @@ -41,13 +66,20 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje .withProjectConfig(projectConfig) .build(); + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setFlagType(flagType) + .setVariationKey(variationKey) + .build(); + return new ImpressionEvent.Builder() .withUserContext(userContext) - .withLayerId(activatedExperiment.getLayerId()) - .withExperimentId(activatedExperiment.getId()) - .withExperimentKey(activatedExperiment.getKey()) - .withVariationId(variation.getId()) - .withVariationKey(variation.getKey()) + .withLayerId(layerID) + .withExperimentId(experimentId) + .withExperimentKey(experimentKey) + .withVariationId(variationID) + .withVariationKey(variationKey) + .withMetadata(metadata) .build(); } diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/Decision.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/Decision.java index 47eb3a790..a9e571dd1 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/Decision.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/Decision.java @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. @@ -28,16 +28,19 @@ public class Decision { String variationId; @JsonProperty("is_campaign_holdback") boolean isCampaignHoldback; + @JsonProperty("metadata") + DecisionMetadata metadata; @VisibleForTesting public Decision() { } - public Decision(String campaignId, String experimentId, String variationId, boolean isCampaignHoldback) { + public Decision(String campaignId, String experimentId, String variationId, boolean isCampaignHoldback, DecisionMetadata metadata) { this.campaignId = campaignId; this.experimentId = experimentId; this.variationId = variationId; this.isCampaignHoldback = isCampaignHoldback; + this.metadata = metadata; } public String getCampaignId() { @@ -56,6 +59,8 @@ public boolean getIsCampaignHoldback() { return isCampaignHoldback; } + public DecisionMetadata getMetadata() { return metadata; } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -74,6 +79,7 @@ public int hashCode() { int result = campaignId.hashCode(); result = 31 * result + experimentId.hashCode(); result = 31 * result + variationId.hashCode(); + result = 31 * result + metadata.hashCode(); result = 31 * result + (isCampaignHoldback ? 1 : 0); return result; } @@ -84,6 +90,7 @@ public static class Builder { private String experimentId; private String variationId; private boolean isCampaignHoldback; + private DecisionMetadata metadata; public Builder setCampaignId(String campaignId) { this.campaignId = campaignId; @@ -95,6 +102,11 @@ public Builder setExperimentId(String experimentId) { return this; } + public Builder setMetadata(DecisionMetadata metadata) { + this.metadata = metadata; + return this; + } + public Builder setVariationId(String variationId) { this.variationId = variationId; return this; @@ -106,7 +118,7 @@ public Builder setIsCampaignHoldback(boolean isCampaignHoldback) { } public Decision build() { - return new Decision(campaignId, experimentId, variationId, isCampaignHoldback); + return new Decision(campaignId, experimentId, variationId, isCampaignHoldback, metadata); } } } diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java new file mode 100644 index 000000000..27363aed3 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java @@ -0,0 +1,93 @@ +/** + * + * Copyright 2020, 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.event.internal.payload; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.optimizely.ab.annotations.VisibleForTesting; + +public class DecisionMetadata { + @JsonProperty("flag_type") + String flagType; + @JsonProperty("flag_key") + String flagKey; + @JsonProperty("variation_key") + String variationKey; + + @VisibleForTesting + public DecisionMetadata() { + } + + public DecisionMetadata(String flagType, String flagKey, String variationKey) { + this.flagType = flagType; + this.flagKey = flagKey; + this.variationKey = variationKey; + } + + public String getFlagType() { + return flagType; + } + + public String getFlagKey() { return flagKey; } + + public String getVariationKey() { return variationKey; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + DecisionMetadata that = (DecisionMetadata) o; + + if (!flagType.equals(that.flagType)) return false; + if (!flagKey.equals(that.flagKey)) return false; + return variationKey.equals(that.variationKey); + } + + @Override + public int hashCode() { + int result = flagType.hashCode(); + result = 31 * result + flagKey.hashCode(); + result = 31 * result + variationKey.hashCode(); + return result; + } + + public static class Builder { + + private String flagType; + private String flagKey; + private String variationKey; + + public Builder setFlagType(String flagType) { + this.flagType = flagType; + return this; + } + + public Builder setFlagKey(String flagKey) { + this.flagKey = flagKey; + return this; + } + + public Builder setVariationKey(String variationKey) { + this.variationKey = variationKey; + return this; + } + + public DecisionMetadata build() { + return new DecisionMetadata(flagType, flagKey, variationKey); + } + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 21dcd017e..c73be3fa3 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -491,6 +491,7 @@ public void isFeatureEnabledWithExperimentKeyForced() throws Exception { assertTrue(optimizely.setForcedVariation(activatedExperiment.getKey(), testUserId, null)); assertNull(optimizely.getForcedVariation(activatedExperiment.getKey(), testUserId)); assertFalse(optimizely.isFeatureEnabled(FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), testUserId)); + eventHandler.expectImpression(null, null, testUserId); } /** @@ -899,11 +900,11 @@ public void activateExperimentStatusPrecedesForcedVariation() throws Exception { } /** - * Verify that {@link Optimizely#activate(String, String)} doesn't dispatch an event for an experiment with a - * "Launched" status. + * Verify that {@link Optimizely#activate(String, String)} dispatches an event for an experiment with a + * "Launched" status when SendFlagDecisions is true. */ @Test - public void activateLaunchedExperimentDoesNotDispatchEvent() throws Exception { + public void activateLaunchedExperimentDispatchesEvent() throws Exception { Experiment launchedExperiment = datafileVersion == 4 ? noAudienceProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_LAUNCHED_EXPERIMENT_KEY) : noAudienceProjectConfig.getExperiments().get(2); @@ -914,11 +915,10 @@ public void activateLaunchedExperimentDoesNotDispatchEvent() throws Exception { // Force variation to launched experiment. optimizely.setForcedVariation(launchedExperiment.getKey(), testUserId, expectedVariation.getKey()); - logbackVerifier.expectMessage(Level.INFO, - "Experiment has \"Launched\" status so not dispatching event during activation."); Variation variation = optimizely.activate(launchedExperiment.getKey(), testUserId); assertNotNull(variation); assertThat(variation.getKey(), is(expectedVariation.getKey())); + eventHandler.expectImpression(launchedExperiment.getId(), expectedVariation.getId(), testUserId); } /** @@ -1684,7 +1684,13 @@ public void getEnabledFeaturesWithListenerMultipleFeatureEnabled() throws Except List featureFlags = optimizely.getEnabledFeatures(testUserId, Collections.emptyMap()); assertEquals(2, featureFlags.size()); - // Why is there only a single impression when there are 2 enabled features? + eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression("3794675122", "589640735", testUserId); + eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, null, testUserId); eventHandler.expectImpression("1786133852", "1619235542", testUserId); // Verify that listener being called @@ -1720,6 +1726,16 @@ public void getEnabledFeaturesWithNoFeatureEnabled() throws Exception { // Verify that listener not being called assertFalse(isListenerCalled); + + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); } @@ -1838,6 +1854,7 @@ public void isFeatureEnabledWithListenerUserInExperimentFeatureOff() throws Exce /** * Verify that the {@link Optimizely#isFeatureEnabled(String, String, Map)} * notification listener of isFeatureEnabled is called when feature is not in experiment and not in rollout + * and it dispatch event * returns false */ @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") @@ -1871,6 +1888,7 @@ public void isFeatureEnabledWithListenerUserNotInExperimentAndNotInRollOut() thr "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); + eventHandler.expectImpression(null, null, genericUserId); // Verify that listener being called assertTrue(isListenerCalled); @@ -1918,6 +1936,9 @@ public void isFeatureEnabledWithListenerUserInRollOut() throws Exception { // Verify that listener being called assertTrue(isListenerCalled); assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); + + eventHandler.expectImpression("3794675122", "589640735", genericUserId, Collections.singletonMap("house", "Gryffindor")); + } //======GetFeatureVariable Notification TESTS======// @@ -3169,6 +3190,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); + eventHandler.expectImpression(null, null, genericUserId); verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), @@ -3215,6 +3237,7 @@ public void isFeatureEnabledReturnsTrueButDoesNotSendWhenUserIsBucketedIntoVaria "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? true" ); + eventHandler.expectImpression("3421010877", "variationId", genericUserId); verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), @@ -3288,6 +3311,7 @@ public void isFeatureEnabledTrueWhenFeatureEnabledOfVariationIsTrue() throws Exc ); assertTrue(optimizely.isFeatureEnabled(validFeatureKey, genericUserId)); + eventHandler.expectImpression("3421010877", "variationId", genericUserId); } @@ -3317,6 +3341,7 @@ public void isFeatureEnabledFalseWhenFeatureEnabledOfVariationIsFalse() throws E ); assertFalse(spyOptimizely.isFeatureEnabled(FEATURE_MULTI_VARIATE_FEATURE_KEY, genericUserId)); + eventHandler.expectImpression("3421010877", "variationId", genericUserId); } @@ -3415,6 +3440,13 @@ public void getEnabledFeatureWithValidUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression("3794675122", "589640735", genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression("1785077004", "1566407342", genericUserId); + eventHandler.expectImpression("828245624", "3137445031", genericUserId); + eventHandler.expectImpression("828245624", "3137445031", genericUserId); eventHandler.expectImpression("1786133852", "1619235542", genericUserId); } @@ -3432,6 +3464,13 @@ public void getEnabledFeatureWithEmptyUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures("", Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); + eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression("3794675122", "589640735", ""); + eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression("1785077004", "1566407342", ""); + eventHandler.expectImpression("828245624", "3137445031", ""); + eventHandler.expectImpression("828245624", "3137445031", ""); eventHandler.expectImpression("4138322202", "1394671166", ""); } @@ -3480,6 +3519,15 @@ public void getEnabledFeatureWithMockDecisionService() throws Exception { List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertTrue(featureFlags.isEmpty()); + + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, null, genericUserId); } /** diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index f50a2780e..5a922452f 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017-2019, Optimizely and contributors + * Copyright 2017-2020, 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. @@ -37,6 +37,7 @@ public class ValidProjectConfigV4 { private static final String PROJECT_ID = "3918735994"; private static final String REVISION = "1480511547"; private static final String VERSION = "4"; + private static final Boolean SEND_FLAG_DECISIONS = true; // attributes private static final String ATTRIBUTE_HOUSE_ID = "553339214"; @@ -1429,6 +1430,7 @@ public static ProjectConfig generateValidProjectConfigV4() { return new DatafileProjectConfig( ACCOUNT_ID, ANONYMIZE_IP, + SEND_FLAG_DECISIONS, BOT_FILTERING, PROJECT_ID, REVISION, diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java index e823d13e5..6ce5007d6 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, Optimizely and contributors + * Copyright 2016-2020, 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. @@ -23,6 +23,7 @@ import com.optimizely.ab.config.*; import com.optimizely.ab.event.LogEvent; import com.optimizely.ab.event.internal.payload.Decision; +import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.ReservedEventKey; @@ -98,14 +99,16 @@ public void createImpressionEventPassingUserAgentAttribute() throws Exception { Variation bucketedVariation = activatedExperiment.getVariations().get(0); Attribute attribute = validProjectConfig.getAttributes().get(0); String userId = "userId"; + String flagType = "experiment"; Map attributeMap = new HashMap(); attributeMap.put(attribute.getKey(), "value"); attributeMap.put(ControlAttribute.USER_AGENT_ATTRIBUTE.toString(), "Chrome"); - + DecisionMetadata metadata = new DecisionMetadata(flagType, activatedExperiment.getKey(), "variationKey"); Decision expectedDecision = new Decision.Builder() .setCampaignId(activatedExperiment.getLayerId()) .setExperimentId(activatedExperiment.getId()) .setVariationId(bucketedVariation.getId()) + .setMetadata(metadata) .setIsCampaignHoldback(false) .build(); @@ -169,10 +172,17 @@ public void createImpressionEvent() throws Exception { String userId = "userId"; Map attributeMap = Collections.singletonMap(attribute.getKey(), "value"); + DecisionMetadata decisionMetadata = new DecisionMetadata.Builder() + .setFlagKey(activatedExperiment.getKey()) + .setFlagType("experiment") + .setVariationKey(bucketedVariation.getKey()) + .build(); + Decision expectedDecision = new Decision.Builder() .setCampaignId(activatedExperiment.getLayerId()) .setExperimentId(activatedExperiment.getId()) .setVariationId(bucketedVariation.getId()) + .setMetadata(decisionMetadata) .setIsCampaignHoldback(false) .build(); @@ -1050,7 +1060,9 @@ public static LogEvent createImpressionEvent(ProjectConfig projectConfig, activatedExperiment, variation, userId, - attributes); + attributes, + activatedExperiment.getKey(), + "experiment"); return EventFactory.createLogEvent(userEvent); diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java index 83c3c79d0..8b9b42f89 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019, Optimizely and contributors + * Copyright 2019-2020, 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. @@ -20,6 +20,7 @@ import com.optimizely.ab.config.Experiment; import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.Variation; +import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.internal.ReservedEventKey; import org.junit.Before; import org.junit.Test; @@ -60,11 +61,28 @@ public class UserEventFactoryTest { private Experiment experiment; private Variation variation; + private DecisionMetadata decisionMetadata; @Before public void setUp() { experiment = new Experiment(EXPERIMENT_ID, EXPERIMENT_KEY, LAYER_ID); variation = new Variation(VARIATION_ID, VARIATION_KEY); + decisionMetadata = new DecisionMetadata("experiment", EXPERIMENT_KEY, VARIATION_KEY); + } + + @Test + public void createImpressionEventNull() { + + ImpressionEvent actual = UserEventFactory.createImpressionEvent( + projectConfig, + experiment, + null, + USER_ID, + ATTRIBUTES, + EXPERIMENT_KEY, + "rollout" + ); + assertNull(actual); } @Test @@ -74,7 +92,9 @@ public void createImpressionEvent() { experiment, variation, USER_ID, - ATTRIBUTES + ATTRIBUTES, + EXPERIMENT_KEY, + "experiment" ); assertTrue(actual.getTimestamp() > 0); @@ -90,6 +110,7 @@ public void createImpressionEvent() { assertEquals(EXPERIMENT_KEY, actual.getExperimentKey()); assertEquals(VARIATION_ID, actual.getVariationId()); assertEquals(VARIATION_KEY, actual.getVariationKey()); + assertEquals(decisionMetadata, actual.getMetadata()); } @Test diff --git a/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java b/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java index 94058776b..4ceb0a67c 100644 --- a/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java @@ -149,6 +149,7 @@ private ProjectConfig generateOptimizelyConfig() { "2360254204", true, true, + true, "3918735994", "1480511547", "4", diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index 42e965967..88b5f815b 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -2,6 +2,7 @@ "accountId": "2360254204", "anonymizeIP": true, "botFiltering": true, + "sendFlagDecisions": true, "projectId": "3918735994", "revision": "1480511547", "version": "4", From ca32f1f930dd8c470ae498512c00a58ec229ec35 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Thu, 1 Oct 2020 14:55:56 +0500 Subject: [PATCH 2/7] Rollout to string fix --- .../java/com/optimizely/ab/event/internal/UserEventFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 3bf78c0b0..900eb718f 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -39,7 +39,7 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje @Nonnull String flagKey, @Nonnull String flagType) { - if ((FeatureDecision.DecisionSource.ROLLOUT.equals(flagType) || variation == null) && !projectConfig.getSendFlagDecisions()) + if ((FeatureDecision.DecisionSource.ROLLOUT.toString().equals(flagType) || variation == null) && !projectConfig.getSendFlagDecisions()) { return null; } From fdc1afbd04623dbd0626a755d11c0cfe678b5a91 Mon Sep 17 00:00:00 2001 From: Muhammad Noman Date: Fri, 2 Oct 2020 04:00:43 +0500 Subject: [PATCH 3/7] Made experiment and variation nullable in createImpression added documentation of sendImpression --- .../src/main/java/com/optimizely/ab/Optimizely.java | 11 +++++++++++ .../ab/event/internal/UserEventFactory.java | 5 +++-- 2 files changed, 14 insertions(+), 2 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 d39cfafe4..36d5a8cb4 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -221,6 +221,17 @@ private Variation activate(@Nullable ProjectConfig projectConfig, return variation; } + /** + * Creates and sends impression event. + * + * @param projectConfig the current projectConfig + * @param experiment the experiment user bucketed into and dispatch an impression event + * @param userId the ID of the user + * @param filteredAttributes the attributes of the user + * @param variation the variation that was returned from activate. + * @param flagKey It can either be experiment key in case if flagType is experiment or it's feature key in case flagType is feature-test or rollout + * @param flagType 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, @Nonnull Experiment experiment, @Nonnull String userId, diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 900eb718f..2f4315912 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -25,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.Map; import java.util.UUID; @@ -32,8 +33,8 @@ public class UserEventFactory { private static final Logger logger = LoggerFactory.getLogger(UserEventFactory.class); public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig projectConfig, - @Nonnull Experiment activatedExperiment, - @Nonnull Variation variation, + @Nullable Experiment activatedExperiment, + @Nullable Variation variation, @Nonnull String userId, @Nonnull Map attributes, @Nonnull String flagKey, From 95409f049b1cfeaaacbfb2854a1bafa42c2f6307 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Mon, 12 Oct 2020 20:19:15 +0500 Subject: [PATCH 4/7] Renamed flag_type to rule_type and added rule_key --- .../ab/event/internal/UserEventFactory.java | 24 +++---- .../internal/payload/DecisionMetadata.java | 40 ++++++++---- .../com/optimizely/ab/OptimizelyTest.java | 62 +++++++++---------- .../ab/event/internal/EventFactoryTest.java | 6 +- .../event/internal/UserEventFactoryTest.java | 2 +- 5 files changed, 75 insertions(+), 59 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 2f4315912..809d03b4c 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -27,7 +27,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Map; -import java.util.UUID; public class UserEventFactory { private static final Logger logger = LoggerFactory.getLogger(UserEventFactory.class); @@ -38,24 +37,24 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje @Nonnull String userId, @Nonnull Map attributes, @Nonnull String flagKey, - @Nonnull String flagType) { + @Nonnull String ruleType) { - if ((FeatureDecision.DecisionSource.ROLLOUT.toString().equals(flagType) || variation == null) && !projectConfig.getSendFlagDecisions()) + if ((FeatureDecision.DecisionSource.ROLLOUT.toString().equals(ruleType) || variation == null) && !projectConfig.getSendFlagDecisions()) { return null; } - String variationKey = null; - String variationID = null; + String variationKey = ""; + String variationID = ""; + String finalRuleType = ""; + String layerID = null; + String experimentId = null; + String experimentKey = ""; + if (variation != null) { variationKey = variation.getKey(); variationID = variation.getId(); - } - - String layerID = null; - String experimentId = null; - String experimentKey = null; - if (activatedExperiment != null) { + finalRuleType = ruleType; layerID = activatedExperiment.getLayerId(); experimentId = activatedExperiment.getId(); experimentKey = activatedExperiment.getKey(); @@ -69,7 +68,8 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje DecisionMetadata metadata = new DecisionMetadata.Builder() .setFlagKey(flagKey) - .setFlagType(flagType) + .setRuleKey(experimentKey) + .setRuleType(finalRuleType) .setVariationKey(variationKey) .build(); diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java index 27363aed3..cfe02e3cb 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java @@ -20,10 +20,13 @@ import com.optimizely.ab.annotations.VisibleForTesting; public class DecisionMetadata { - @JsonProperty("flag_type") - String flagType; + @JsonProperty("flag_key") String flagKey; + @JsonProperty("rule_key") + String ruleKey; + @JsonProperty("rule_type") + String ruleType; @JsonProperty("variation_key") String variationKey; @@ -31,14 +34,19 @@ public class DecisionMetadata { public DecisionMetadata() { } - public DecisionMetadata(String flagType, String flagKey, String variationKey) { - this.flagType = flagType; + public DecisionMetadata(String flagKey, String ruleKey, String ruleType, String variationKey) { this.flagKey = flagKey; + this.ruleKey = ruleKey; + this.ruleType = ruleType; this.variationKey = variationKey; } - public String getFlagType() { - return flagType; + public String getRuleType() { + return ruleType; + } + + public String getRuleKey() { + return ruleKey; } public String getFlagKey() { return flagKey; } @@ -52,27 +60,35 @@ public boolean equals(Object o) { DecisionMetadata that = (DecisionMetadata) o; - if (!flagType.equals(that.flagType)) return false; + if (!ruleType.equals(that.ruleType)) return false; + if (!ruleKey.equals(that.ruleKey)) return false; if (!flagKey.equals(that.flagKey)) return false; return variationKey.equals(that.variationKey); } @Override public int hashCode() { - int result = flagType.hashCode(); + int result = ruleType.hashCode(); result = 31 * result + flagKey.hashCode(); + result = 31 * result + ruleKey.hashCode(); result = 31 * result + variationKey.hashCode(); return result; } public static class Builder { - private String flagType; + private String ruleType; + private String ruleKey; private String flagKey; private String variationKey; - public Builder setFlagType(String flagType) { - this.flagType = flagType; + public Builder setRuleType(String ruleType) { + this.ruleType = ruleType; + return this; + } + + public Builder setRuleKey(String ruleKey) { + this.ruleKey = ruleKey; return this; } @@ -87,7 +103,7 @@ public Builder setVariationKey(String variationKey) { } public DecisionMetadata build() { - return new DecisionMetadata(flagType, flagKey, variationKey); + return new DecisionMetadata(flagKey, ruleKey, ruleType, variationKey); } } } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index c73be3fa3..cd4c926c8 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -491,7 +491,7 @@ public void isFeatureEnabledWithExperimentKeyForced() throws Exception { assertTrue(optimizely.setForcedVariation(activatedExperiment.getKey(), testUserId, null)); assertNull(optimizely.getForcedVariation(activatedExperiment.getKey(), testUserId)); assertFalse(optimizely.isFeatureEnabled(FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), testUserId)); - eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, "", testUserId); } /** @@ -1684,13 +1684,13 @@ public void getEnabledFeaturesWithListenerMultipleFeatureEnabled() throws Except List featureFlags = optimizely.getEnabledFeatures(testUserId, Collections.emptyMap()); assertEquals(2, featureFlags.size()); - eventHandler.expectImpression(null, null, testUserId); - eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, "", testUserId); + eventHandler.expectImpression(null, "", testUserId); eventHandler.expectImpression("3794675122", "589640735", testUserId); - eventHandler.expectImpression(null, null, testUserId); - eventHandler.expectImpression(null, null, testUserId); - eventHandler.expectImpression(null, null, testUserId); - eventHandler.expectImpression(null, null, testUserId); + eventHandler.expectImpression(null, "", testUserId); + eventHandler.expectImpression(null, "", testUserId); + eventHandler.expectImpression(null, "", testUserId); + eventHandler.expectImpression(null, "", testUserId); eventHandler.expectImpression("1786133852", "1619235542", testUserId); // Verify that listener being called @@ -1727,14 +1727,14 @@ public void getEnabledFeaturesWithNoFeatureEnabled() throws Exception { // Verify that listener not being called assertFalse(isListenerCalled); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); assertTrue(optimizely.notificationCenter.removeNotificationListener(notificationId)); } @@ -1888,7 +1888,7 @@ public void isFeatureEnabledWithListenerUserNotInExperimentAndNotInRollOut() thr "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); // Verify that listener being called assertTrue(isListenerCalled); @@ -3190,7 +3190,7 @@ public void isFeatureEnabledReturnsFalseWhenUserIsNotBucketedIntoAnyVariation() "Feature \"" + validFeatureKey + "\" is enabled for user \"" + genericUserId + "\"? false" ); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); verify(mockDecisionService).getVariationForFeature( eq(FEATURE_FLAG_MULTI_VARIATE_FEATURE), @@ -3440,10 +3440,10 @@ public void getEnabledFeatureWithValidUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures(genericUserId, Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); eventHandler.expectImpression("3794675122", "589640735", genericUserId); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); eventHandler.expectImpression("1785077004", "1566407342", genericUserId); eventHandler.expectImpression("828245624", "3137445031", genericUserId); eventHandler.expectImpression("828245624", "3137445031", genericUserId); @@ -3464,10 +3464,10 @@ public void getEnabledFeatureWithEmptyUserId() throws Exception { List featureFlags = optimizely.getEnabledFeatures("", Collections.emptyMap()); assertFalse(featureFlags.isEmpty()); - eventHandler.expectImpression(null, null, ""); - eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression(null, "", ""); + eventHandler.expectImpression(null, "", ""); eventHandler.expectImpression("3794675122", "589640735", ""); - eventHandler.expectImpression(null, null, ""); + eventHandler.expectImpression(null, "", ""); eventHandler.expectImpression("1785077004", "1566407342", ""); eventHandler.expectImpression("828245624", "3137445031", ""); eventHandler.expectImpression("828245624", "3137445031", ""); @@ -3520,14 +3520,14 @@ public void getEnabledFeatureWithMockDecisionService() throws Exception { Collections.emptyMap()); assertTrue(featureFlags.isEmpty()); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); - eventHandler.expectImpression(null, null, genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); + eventHandler.expectImpression(null, "", genericUserId); } /** diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java index 6ce5007d6..f4be1b965 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java @@ -99,11 +99,11 @@ public void createImpressionEventPassingUserAgentAttribute() throws Exception { Variation bucketedVariation = activatedExperiment.getVariations().get(0); Attribute attribute = validProjectConfig.getAttributes().get(0); String userId = "userId"; - String flagType = "experiment"; + String ruleType = "experiment"; Map attributeMap = new HashMap(); attributeMap.put(attribute.getKey(), "value"); attributeMap.put(ControlAttribute.USER_AGENT_ATTRIBUTE.toString(), "Chrome"); - DecisionMetadata metadata = new DecisionMetadata(flagType, activatedExperiment.getKey(), "variationKey"); + DecisionMetadata metadata = new DecisionMetadata(activatedExperiment.getKey(), activatedExperiment.getKey(), ruleType, "variationKey"); Decision expectedDecision = new Decision.Builder() .setCampaignId(activatedExperiment.getLayerId()) .setExperimentId(activatedExperiment.getId()) @@ -174,7 +174,7 @@ public void createImpressionEvent() throws Exception { DecisionMetadata decisionMetadata = new DecisionMetadata.Builder() .setFlagKey(activatedExperiment.getKey()) - .setFlagType("experiment") + .setRuleType("experiment") .setVariationKey(bucketedVariation.getKey()) .build(); diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java index 8b9b42f89..ee62d8029 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java @@ -67,7 +67,7 @@ public class UserEventFactoryTest { public void setUp() { experiment = new Experiment(EXPERIMENT_ID, EXPERIMENT_KEY, LAYER_ID); variation = new Variation(VARIATION_ID, VARIATION_KEY); - decisionMetadata = new DecisionMetadata("experiment", EXPERIMENT_KEY, VARIATION_KEY); + decisionMetadata = new DecisionMetadata(EXPERIMENT_KEY, EXPERIMENT_KEY, "experiment", VARIATION_KEY); } @Test From 029cfd5abaea16184529cddd88cb9a3a3779bfc0 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Mon, 12 Oct 2020 20:33:33 +0500 Subject: [PATCH 5/7] nit fix --- .../ab/event/internal/payload/DecisionMetadata.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java index cfe02e3cb..f5120b230 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/DecisionMetadata.java @@ -49,9 +49,13 @@ public String getRuleKey() { return ruleKey; } - public String getFlagKey() { return flagKey; } + public String getFlagKey() { + return flagKey; + } - public String getVariationKey() { return variationKey; } + public String getVariationKey() { + return variationKey; + } @Override public boolean equals(Object o) { From 42d7165073e87a1fdb52e029885121358b90c5e7 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Tue, 13 Oct 2020 19:42:47 +0500 Subject: [PATCH 6/7] set ruletype to rollout when decision source is null set flagKey to empty in activate case --- .../java/com/optimizely/ab/Optimizely.java | 29 +++++++++++++++---- .../ab/event/internal/UserEventFactory.java | 4 +-- 2 files changed, 25 insertions(+), 8 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 36d5a8cb4..754a4ec09 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -216,7 +216,7 @@ private Variation activate(@Nullable ProjectConfig projectConfig, return null; } - sendImpression(projectConfig, experiment, userId, copiedAttributes, variation, experiment.getKey(), "experiment"); + sendImpression(projectConfig, experiment, userId, copiedAttributes, variation, "experiment"); return variation; } @@ -229,8 +229,27 @@ private Variation activate(@Nullable ProjectConfig projectConfig, * @param userId the ID of the user * @param filteredAttributes the attributes of the user * @param variation the variation that was returned from activate. - * @param flagKey It can either be experiment key in case if flagType is experiment or it's feature key in case flagType is feature-test or rollout - * @param flagType It can either be experiment in case impression event is sent from activate or it's 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, + @Nonnull Experiment experiment, + @Nonnull String userId, + @Nonnull Map filteredAttributes, + @Nonnull Variation variation, + @Nonnull String ruleType) { + sendImpression(projectConfig, experiment, userId, filteredAttributes, variation, "", ruleType); + } + + /** + * Creates and sends impression event. + * + * @param projectConfig the current projectConfig + * @param experiment the experiment user bucketed into and dispatch an impression event + * @param userId the ID of the user + * @param filteredAttributes the attributes of the user + * @param variation the variation that was returned from activate. + * @param flagKey It can either be experiment key in case 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, @Nonnull Experiment experiment, @@ -238,7 +257,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, @Nonnull Map filteredAttributes, @Nonnull Variation variation, @Nonnull String flagKey, - @Nonnull String flagType) { + @Nonnull String ruleType) { UserEvent userEvent = UserEventFactory.createImpressionEvent( projectConfig, @@ -247,7 +266,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, userId, filteredAttributes, flagKey, - flagType); + ruleType); if (userEvent == null) { return; diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java index 809d03b4c..74457922e 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/UserEventFactory.java @@ -46,7 +46,6 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje String variationKey = ""; String variationID = ""; - String finalRuleType = ""; String layerID = null; String experimentId = null; String experimentKey = ""; @@ -54,7 +53,6 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje if (variation != null) { variationKey = variation.getKey(); variationID = variation.getId(); - finalRuleType = ruleType; layerID = activatedExperiment.getLayerId(); experimentId = activatedExperiment.getId(); experimentKey = activatedExperiment.getKey(); @@ -69,7 +67,7 @@ public static ImpressionEvent createImpressionEvent(@Nonnull ProjectConfig proje DecisionMetadata metadata = new DecisionMetadata.Builder() .setFlagKey(flagKey) .setRuleKey(experimentKey) - .setRuleType(finalRuleType) + .setRuleType(ruleType) .setVariationKey(variationKey) .build(); From 799e65dcc270093c1ce153349a77dcb6e2137675 Mon Sep 17 00:00:00 2001 From: "FOLIO3PK\\muhammadnoman" Date: Fri, 23 Oct 2020 18:00:11 +0500 Subject: [PATCH 7/7] Comments resolved Updated userEventFactory create impression test to avoid sending experiment key as flag key in case of ruletype experiment --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- .../optimizely/ab/event/internal/UserEventFactoryTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 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 754a4ec09..c3e035e2c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -248,7 +248,7 @@ private void sendImpression(@Nonnull ProjectConfig projectConfig, * @param userId the ID of the user * @param filteredAttributes the attributes of the user * @param variation the variation that was returned from activate. - * @param flagKey It can either be experiment key in case if ruleType is experiment or it's feature key in case ruleType is feature-test or rollout + * @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, diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java index ee62d8029..87b667658 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/UserEventFactoryTest.java @@ -67,7 +67,7 @@ public class UserEventFactoryTest { public void setUp() { experiment = new Experiment(EXPERIMENT_ID, EXPERIMENT_KEY, LAYER_ID); variation = new Variation(VARIATION_ID, VARIATION_KEY); - decisionMetadata = new DecisionMetadata(EXPERIMENT_KEY, EXPERIMENT_KEY, "experiment", VARIATION_KEY); + decisionMetadata = new DecisionMetadata("", EXPERIMENT_KEY, "experiment", VARIATION_KEY); } @Test @@ -93,7 +93,7 @@ public void createImpressionEvent() { variation, USER_ID, ATTRIBUTES, - EXPERIMENT_KEY, + "", "experiment" );