From 95768e678f29a77df2377e4091edd7e86574cd96 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 03:55:12 +0600 Subject: [PATCH 1/7] update --- .vscode/settings.json | 3 + .../java/com/optimizely/ab/Optimizely.java | 170 ++++++++++ .../ab/bucketing/DecisionService.java | 292 ++++++++++++++---- 3 files changed, 397 insertions(+), 68 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..0ca4d0be2 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "java.compile.nullAnalysis.mode": "automatic" +} \ No newline at end of file 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 fede007d5..3f13d6f28 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -42,6 +42,8 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; +import javax.xml.catalog.CatalogFeatures.Feature; + import java.io.Closeable; import java.util.*; @@ -1296,6 +1298,174 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, reasonsToReport); } + Optional getForcedDecision(@Nonnull String flagKey, + @Nonnull DecisionReasons decisionReasons, + @Nonnull ProjectConfig projectConfig, + @Nonnull OptimizelyUserContext user) { + + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, null); + DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); + decisionReasons.merge(forcedDecisionVariation.getReasons()); + if (forcedDecisionVariation.getResult() != null) { + return Optional.of(new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST)); + } + + return Optional.empty(); + } + + OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, + @Nonnull String key, + @Nonnull List options) { + + ProjectConfig projectConfig = getProjectConfig(); + if (projectConfig == null) { + return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); + } + + FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); + if (flag == null) { + return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); + } + + String userId = user.getUserId(); + Map attributes = user.getAttributes(); + Boolean decisionEventDispatched = false; + List allOptions = getAllOptions(options); + DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); + + Map copiedAttributes = new HashMap<>(attributes); + FeatureDecision flagDecision; + + // Check Forced Decision + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); + DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); + decisionReasons.merge(forcedDecisionVariation.getReasons()); + if (forcedDecisionVariation.getResult() != null) { + flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); + } else { + // Regular decision + DecisionResponse decisionVariation = decisionService.getVariationForFeature( + flag, + user, + projectConfig, + allOptions); + flagDecision = decisionVariation.getResult(); + decisionReasons.merge(decisionVariation.getReasons()); + } + + Boolean flagEnabled = false; + if (flagDecision.variation != null) { + if (flagDecision.variation.getFeatureEnabled()) { + flagEnabled = true; + } + } + logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); + + Map variableMap = new HashMap<>(); + if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { + DecisionResponse> decisionVariables = getDecisionVariableMap( + flag, + flagDecision.variation, + flagEnabled); + variableMap = decisionVariables.getResult(); + decisionReasons.merge(decisionVariables.getReasons()); + } + OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); + + FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; + if (flagDecision.decisionSource != null) { + decisionSource = flagDecision.decisionSource; + } + + List reasonsToReport = decisionReasons.toReport(); + String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; + // TODO: add ruleKey values when available later. use a copy of experimentKey until then. + // add to event metadata as well (currently set to experimentKey) + String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; + + if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { + decisionEventDispatched = sendImpression( + projectConfig, + flagDecision.experiment, + userId, + copiedAttributes, + flagDecision.variation, + key, + decisionSource.toString(), + flagEnabled); + } + + DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() + .withUserId(userId) + .withAttributes(copiedAttributes) + .withFlagKey(key) + .withEnabled(flagEnabled) + .withVariables(variableMap) + .withVariationKey(variationKey) + .withRuleKey(ruleKey) + .withReasons(reasonsToReport) + .withDecisionEventDispatched(decisionEventDispatched) + .build(); + notificationCenter.send(decisionNotification); + + return new OptimizelyDecision( + variationKey, + flagEnabled, + optimizelyJSON, + ruleKey, + key, + user, + reasonsToReport); + } + + Map decideForKeysInternal(@Nonnull OptimizelyUserContext user, + @Nonnull List keys, + @Nonnull List options) { + Map decisionMap = new HashMap<>(); + + ProjectConfig projectConfig = getProjectConfig(); + if (projectConfig == null) { + logger.error("Optimizely instance is not valid, failing isFeatureEnabled call."); + return decisionMap; + } + + if (keys.isEmpty()) return decisionMap; + + String userId = user.getUserId(); + Map attributes = user.getAttributes(); + Boolean decisionEventDispatched = false; + List allOptions = getAllOptions(options); + + Map flagDecisions = new HashMap<>(); + Map decisionReasonsMap = new HashMap<>(); + + List keysWithoutForcedDecision = new ArrayList<>(); + + for (String key : keys) { + FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); + if (flag == null) { + decisionMap.put(key, OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key))); + continue; + } + + DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); + Optional forcedDecision = getForcedDecision(key, decisionReasons, projectConfig, user); + decisionReasonsMap.put(key, decisionReasons); + + if (forcedDecision.isPresent()) { + flagDecisions.put(key, forcedDecision.get()); + } else { + keysWithoutForcedDecision.add(key); + } + // OptimizelyDecision decision = decide(user, key, options); + // if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { + // decisionMap.put(key, decision); + // } + } + + return decisionMap; + } + Map decideForKeys(@Nonnull OptimizelyUserContext user, @Nonnull List keys, @Nonnull List options) { diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 84d47d03f..a958e1dda 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -78,21 +78,16 @@ public DecisionService(@Nonnull Bucketer bucketer, this.userProfileService = userProfileService; } - /** - * Get a {@link Variation} of an {@link Experiment} for a user to be allocated into. - * - * @param experiment The Experiment the user will be bucketed into. - * @param user The current OptimizelyUserContext - * @param projectConfig The current projectConfig - * @param options An array of decision options - * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons - */ @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, - @Nonnull List options) { - DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + @Nonnull List options, + @Nonnull UserProfile userProfile, + @Nullable DecisionReasons reasons) { + if (reasons == null) { + reasons = DefaultDecisionReasons.newInstance(); + } if (!ExperimentUtils.isExperimentActive(experiment)) { String message = reasons.addInfo("Experiment \"%s\" is not running.", experiment.getKey()); @@ -116,39 +111,13 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(variation, reasons); } - // fetch the user profile map from the user profile service - boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); - UserProfile userProfile = null; - - if (userProfileService != null && !ignoreUPS) { - try { - Map userProfileMap = userProfileService.lookup(user.getUserId()); - if (userProfileMap == null) { - String message = reasons.addInfo("We were unable to get a user profile map from the UserProfileService."); - logger.info(message); - } else if (UserProfileUtils.isValidUserProfileMap(userProfileMap)) { - userProfile = UserProfileUtils.convertMapToUserProfile(userProfileMap); - } else { - String message = reasons.addInfo("The UserProfileService returned an invalid map."); - logger.warn(message); - } - } catch (Exception exception) { - String message = reasons.addInfo(exception.getMessage()); - logger.error(message); - errorHandler.handleError(new OptimizelyRuntimeException(exception)); - } - - // check if user exists in user profile - if (userProfile != null) { - decisionVariation = getStoredVariation(experiment, userProfile, projectConfig); - reasons.merge(decisionVariation.getReasons()); - variation = decisionVariation.getResult(); - // return the stored variation if it exists - if (variation != null) { - return new DecisionResponse(variation, reasons); - } - } else { // if we could not find a user profile, make a new one - userProfile = new UserProfile(user.getUserId(), new HashMap()); + if (userProfile != null) { + decisionVariation = getStoredVariation(experiment, userProfile, projectConfig); + reasons.merge(decisionVariation.getReasons()); + variation = decisionVariation.getResult(); + // return the stored variation if it exists + if (variation != null) { + return new DecisionResponse(variation, reasons); } } @@ -162,8 +131,8 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, variation = decisionVariation.getResult(); if (variation != null) { - if (userProfileService != null && !ignoreUPS) { - saveVariation(experiment, variation, userProfile); + if (userProfile != null) { + updateUserProfile(experiment, variation, userProfile); } else { logger.debug("This decision will not be saved since the UserProfileService is null."); } @@ -177,6 +146,38 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(null, reasons); } + /** + * Get a {@link Variation} of an {@link Experiment} for a user to be allocated into. + * + * @param experiment The Experiment the user will be bucketed into. + * @param user The current OptimizelyUserContext + * @param projectConfig The current projectConfig + * @param options An array of decision options + * @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons + */ + @Nonnull + public DecisionResponse getVariation(@Nonnull Experiment experiment, + @Nonnull OptimizelyUserContext user, + @Nonnull ProjectConfig projectConfig, + @Nonnull List options) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + + // fetch the user profile map from the user profile service + boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + UserProfile userProfile = null; + + if (userProfileService != null && !ignoreUPS) { + userProfile = getUserProfile(user.getUserId(), reasons); + } + + DecisionResponse response = getVariation(experiment, user, projectConfig, options, userProfile, reasons); + + if(userProfileService != null && !ignoreUPS) { + saveUserProfile(userProfile); + } + return response; + } + @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull OptimizelyUserContext user, @@ -198,31 +199,145 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options) { - DecisionReasons reasons = DefaultDecisionReasons.newInstance(); +// DecisionReasons reasons = DefaultDecisionReasons.newInstance(); +// +// DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); +// reasons.merge(decisionVariationResponse.getReasons()); +// +// FeatureDecision decision = decisionVariationResponse.getResult(); +// if (decision != null) { +// return new DecisionResponse(decision, reasons); +// } +// +// DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); +// reasons.merge(decisionFeatureResponse.getReasons()); +// decision = decisionFeatureResponse.getResult(); +// +// String message; +// if (decision.variation == null) { +// message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", +// user.getUserId(), featureFlag.getKey()); +// } else { +// message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", +// user.getUserId(), featureFlag.getKey()); +// } +// logger.info(message); +// +// return new DecisionResponse(decision, reasons); + return getVariationsForFeatureList(Arrays.asList(featureFlag), user, projectConfig, options).get(0); + } - DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); - reasons.merge(decisionVariationResponse.getReasons()); + private UserProfile getUserProfile(String userId, DecisionReasons reasons) { + UserProfile userProfile = null; - FeatureDecision decision = decisionVariationResponse.getResult(); - if (decision != null) { - return new DecisionResponse(decision, reasons); + try { + Map userProfileMap = userProfileService.lookup(userId); + if (userProfileMap == null) { + String message = reasons.addInfo("We were unable to get a user profile map from the UserProfileService."); + logger.info(message); + } else if (UserProfileUtils.isValidUserProfileMap(userProfileMap)) { + userProfile = UserProfileUtils.convertMapToUserProfile(userProfileMap); + } else { + String message = reasons.addInfo("The UserProfileService returned an invalid map."); + logger.warn(message); + } + } catch (Exception exception) { + String message = reasons.addInfo(exception.getMessage()); + logger.error(message); + errorHandler.handleError(new OptimizelyRuntimeException(exception)); } - DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); - reasons.merge(decisionFeatureResponse.getReasons()); - decision = decisionFeatureResponse.getResult(); + if (userProfile == null) { + userProfile = new UserProfile(userId, new HashMap()); + } - String message; - if (decision.variation == null) { - message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", - user.getUserId(), featureFlag.getKey()); - } else { - message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", - user.getUserId(), featureFlag.getKey()); + return userProfile; + } + + /** + * Get the variation the user is bucketed into for the FeatureFlag + * + * @param featureFlags The feature flag list the user wants to access. + * @param user The current OptimizelyuserContext + * @param projectConfig The current projectConfig + * @param options An array of decision options + * @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons + */ + @Nonnull + public List> getVariationsForFeatureList(@Nonnull List featureFlags, + @Nonnull OptimizelyUserContext user, + @Nonnull ProjectConfig projectConfig, + @Nonnull List options) { + DecisionReasons upsReasons = DefaultDecisionReasons.newInstance(); + + boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + UserProfile userProfile = null; + + if (userProfileService != null && !ignoreUPS) { + userProfile = getUserProfile(user.getUserId(), upsReasons); } - logger.info(message); - return new DecisionResponse(decision, reasons); + List> decisions = new ArrayList<>(); + + for (FeatureFlag featureFlag: featureFlags) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + reasons.merge(upsReasons); + + DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfile); + reasons.merge(decisionVariationResponse.getReasons()); + + FeatureDecision decision = decisionVariationResponse.getResult(); + if (decision != null) { + decisions.add(new DecisionResponse(decision, reasons)); + continue; + } + + DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); + reasons.merge(decisionFeatureResponse.getReasons()); + decision = decisionFeatureResponse.getResult(); + + String message; + if (decision.variation == null) { + message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); + } else { + message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", + user.getUserId(), featureFlag.getKey()); + } + logger.info(message); + + decisions.add(new DecisionResponse(decision, reasons)); + } + + if (userProfileService != null && !ignoreUPS) { + saveUserProfile(userProfile); + } + + return decisions; + +// DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); +// reasons.merge(decisionVariationResponse.getReasons()); +// +// FeatureDecision decision = decisionVariationResponse.getResult(); +// if (decision != null) { +// return new DecisionResponse(decision, reasons); +// } + +// DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); +// reasons.merge(decisionFeatureResponse.getReasons()); +// decision = decisionFeatureResponse.getResult(); +// +// String message; +// if (decision.variation == null) { +// message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", +// user.getUserId(), featureFlag.getKey()); +// } else { +// message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", +// user.getUserId(), featureFlag.getKey()); +// } +// logger.info(message); +// +// return new DecisionResponse(decision, reasons); } @Nonnull @@ -244,13 +359,14 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature DecisionResponse getVariationFromExperiment(@Nonnull ProjectConfig projectConfig, @Nonnull FeatureFlag featureFlag, @Nonnull OptimizelyUserContext user, - @Nonnull List options) { + @Nonnull List options, + @Nullable UserProfile userProfile) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); - DecisionResponse decisionVariation = getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options); + DecisionResponse decisionVariation = getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfile); reasons.merge(decisionVariation.getReasons()); Variation variation = decisionVariation.getResult(); @@ -412,6 +528,29 @@ DecisionResponse getStoredVariation(@Nonnull Experiment experiment, } } + /** + * Save a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfileService}. + * + * @param experiment The experiment the user was buck + * @param variation The Variation to save. + * @param userProfile A {@link UserProfile} instance of the user information. + */ + void updateUserProfile(@Nonnull Experiment experiment, + @Nonnull Variation variation, + @Nonnull UserProfile userProfile) { + + String experimentId = experiment.getId(); + String variationId = variation.getId(); + Decision decision; + if (userProfile.experimentBucketMap.containsKey(experimentId)) { + decision = userProfile.experimentBucketMap.get(experimentId); + decision.variationId = variationId; + } else { + decision = new Decision(variationId); + } + userProfile.experimentBucketMap.put(experimentId, decision); + } + /** * Save a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfileService}. * @@ -448,6 +587,22 @@ void saveVariation(@Nonnull Experiment experiment, } } + void saveUserProfile(@Nonnull UserProfile userProfile) { + if (userProfileService == null) { + return; + } + + try { + userProfileService.save(userProfile.toMap()); + logger.info("Saved user profile of user \"{}\".", + userProfile.userId); + } catch (Exception exception) { + logger.warn("Failed to save user profile of user \"{}\".", + userProfile.userId); + errorHandler.handleError(new OptimizelyRuntimeException(exception)); + } + } + /** * Get the bucketingId of a user if a bucketingId exists in attributes, or else default to userId. * @@ -619,7 +774,8 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje @Nonnull String flagKey, @Nonnull Experiment rule, @Nonnull OptimizelyUserContext user, - @Nonnull List options) { + @Nonnull List options, + @Nullable UserProfile userProfile) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); String ruleKey = rule != null ? rule.getKey() : null; @@ -634,7 +790,7 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje return new DecisionResponse(variation, reasons); } //regular decision - DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options); + DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options, userProfile, null); reasons.merge(decisionResponse.getReasons()); variation = decisionResponse.getResult(); From 9361e97d09179017ca1283efb37d19d166c7025f Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 04:01:01 +0600 Subject: [PATCH 2/7] up --- .../ab/bucketing/DecisionServiceTest.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 6057b43cf..24898b34a 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -743,26 +743,26 @@ public void getVariationFromDeliveryRuleTest() { assertFalse(skipToEveryoneElse); } - @Test - public void getVariationFromExperimentRuleTest() { - int index = 3; - Experiment experiment = ROLLOUT_2.getExperiments().get(index); - Variation expectedVariation = null; - for (Variation variation : experiment.getVariations()) { - if (variation.getKey().equals("3137445031")) { - expectedVariation = variation; - } - } - DecisionResponse decisionResponse = decisionService.getVariationFromExperimentRule( - v4ProjectConfig, - FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), - experiment, - optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)), - Collections.emptyList() - ); - - assertEquals(expectedVariation, decisionResponse.getResult()); - } +// @Test +// public void getVariationFromExperimentRuleTest() { +// int index = 3; +// Experiment experiment = ROLLOUT_2.getExperiments().get(index); +// Variation expectedVariation = null; +// for (Variation variation : experiment.getVariations()) { +// if (variation.getKey().equals("3137445031")) { +// expectedVariation = variation; +// } +// } +// DecisionResponse decisionResponse = decisionService.getVariationFromExperimentRule( +// v4ProjectConfig, +// FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), +// experiment, +// optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)), +// Collections.emptyList() +// ); +// +// assertEquals(expectedVariation, decisionResponse.getResult()); +// } @Test public void validatedForcedDecisionWithRuleKey() { From e515a5bf36caada65c25cd37c536c1ed03d4e390 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 05:11:21 +0600 Subject: [PATCH 3/7] upd --- .../java/com/optimizely/ab/Optimizely.java | 449 +++++++++++------- 1 file changed, 275 insertions(+), 174 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 3f13d6f28..ab061bdc4 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1193,111 +1193,111 @@ private OptimizelyUserContext createUserContextCopy(@Nonnull String userId, @Non return new OptimizelyUserContext(this, userId, attributes, Collections.EMPTY_MAP, null, false); } - OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, - @Nonnull String key, - @Nonnull List options) { - - ProjectConfig projectConfig = getProjectConfig(); - if (projectConfig == null) { - return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); - } - - FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); - if (flag == null) { - return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); - } - - String userId = user.getUserId(); - Map attributes = user.getAttributes(); - Boolean decisionEventDispatched = false; - List allOptions = getAllOptions(options); - DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); - - Map copiedAttributes = new HashMap<>(attributes); - FeatureDecision flagDecision; - - // Check Forced Decision - OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); - DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); - decisionReasons.merge(forcedDecisionVariation.getReasons()); - if (forcedDecisionVariation.getResult() != null) { - flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); - } else { - // Regular decision - DecisionResponse decisionVariation = decisionService.getVariationForFeature( - flag, - user, - projectConfig, - allOptions); - flagDecision = decisionVariation.getResult(); - decisionReasons.merge(decisionVariation.getReasons()); - } - - Boolean flagEnabled = false; - if (flagDecision.variation != null) { - if (flagDecision.variation.getFeatureEnabled()) { - flagEnabled = true; - } - } - logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); - - Map variableMap = new HashMap<>(); - if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { - DecisionResponse> decisionVariables = getDecisionVariableMap( - flag, - flagDecision.variation, - flagEnabled); - variableMap = decisionVariables.getResult(); - decisionReasons.merge(decisionVariables.getReasons()); - } - OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); - - FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; - if (flagDecision.decisionSource != null) { - decisionSource = flagDecision.decisionSource; - } - - List reasonsToReport = decisionReasons.toReport(); - String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; - // TODO: add ruleKey values when available later. use a copy of experimentKey until then. - // add to event metadata as well (currently set to experimentKey) - String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; - - if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { - decisionEventDispatched = sendImpression( - projectConfig, - flagDecision.experiment, - userId, - copiedAttributes, - flagDecision.variation, - key, - decisionSource.toString(), - flagEnabled); - } - - DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() - .withUserId(userId) - .withAttributes(copiedAttributes) - .withFlagKey(key) - .withEnabled(flagEnabled) - .withVariables(variableMap) - .withVariationKey(variationKey) - .withRuleKey(ruleKey) - .withReasons(reasonsToReport) - .withDecisionEventDispatched(decisionEventDispatched) - .build(); - notificationCenter.send(decisionNotification); - - return new OptimizelyDecision( - variationKey, - flagEnabled, - optimizelyJSON, - ruleKey, - key, - user, - reasonsToReport); - } - +// OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, +// @Nonnull String key, +// @Nonnull List options) { +// +// ProjectConfig projectConfig = getProjectConfig(); +// if (projectConfig == null) { +// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); +// } +// +// FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); +// if (flag == null) { +// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); +// } +// +// String userId = user.getUserId(); +// Map attributes = user.getAttributes(); +// Boolean decisionEventDispatched = false; +// List allOptions = getAllOptions(options); +// DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); +// +// Map copiedAttributes = new HashMap<>(attributes); +// FeatureDecision flagDecision; +// +// // Check Forced Decision +// OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); +// DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); +// decisionReasons.merge(forcedDecisionVariation.getReasons()); +// if (forcedDecisionVariation.getResult() != null) { +// flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); +// } else { +// // Regular decision +// DecisionResponse decisionVariation = decisionService.getVariationForFeature( +// flag, +// user, +// projectConfig, +// allOptions); +// flagDecision = decisionVariation.getResult(); +// decisionReasons.merge(decisionVariation.getReasons()); +// } +// +// Boolean flagEnabled = false; +// if (flagDecision.variation != null) { +// if (flagDecision.variation.getFeatureEnabled()) { +// flagEnabled = true; +// } +// } +// logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); +// +// Map variableMap = new HashMap<>(); +// if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { +// DecisionResponse> decisionVariables = getDecisionVariableMap( +// flag, +// flagDecision.variation, +// flagEnabled); +// variableMap = decisionVariables.getResult(); +// decisionReasons.merge(decisionVariables.getReasons()); +// } +// OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); +// +// FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; +// if (flagDecision.decisionSource != null) { +// decisionSource = flagDecision.decisionSource; +// } +// +// List reasonsToReport = decisionReasons.toReport(); +// String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; +// // TODO: add ruleKey values when available later. use a copy of experimentKey until then. +// // add to event metadata as well (currently set to experimentKey) +// String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; +// +// if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { +// decisionEventDispatched = sendImpression( +// projectConfig, +// flagDecision.experiment, +// userId, +// copiedAttributes, +// flagDecision.variation, +// key, +// decisionSource.toString(), +// flagEnabled); +// } +// +// DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() +// .withUserId(userId) +// .withAttributes(copiedAttributes) +// .withFlagKey(key) +// .withEnabled(flagEnabled) +// .withVariables(variableMap) +// .withVariationKey(variationKey) +// .withRuleKey(ruleKey) +// .withReasons(reasonsToReport) +// .withDecisionEventDispatched(decisionEventDispatched) +// .build(); +// notificationCenter.send(decisionNotification); +// +// return new OptimizelyDecision( +// variationKey, +// flagEnabled, +// optimizelyJSON, +// ruleKey, +// key, +// user, +// reasonsToReport); +// } +// Optional getForcedDecision(@Nonnull String flagKey, @Nonnull DecisionReasons decisionReasons, @Nonnull ProjectConfig projectConfig, @@ -1312,46 +1312,123 @@ Optional getForcedDecision(@Nonnull String flagKey, return Optional.empty(); } - - OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, + + // TODO: UPS refactor cleanup + OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { - - ProjectConfig projectConfig = getProjectConfig(); - if (projectConfig == null) { - return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); - } - - FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); - if (flag == null) { - return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); - } - + return decideForKeys(user, Arrays.asList(key), options).get(key); + +// ProjectConfig projectConfig = getProjectConfig(); +// if (projectConfig == null) { +// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); +// } +// +// FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); +// if (flag == null) { +// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); +// } +// +// String userId = user.getUserId(); +// Map attributes = user.getAttributes(); +// Boolean decisionEventDispatched = false; +// List allOptions = getAllOptions(options); +// DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); +// +// Map copiedAttributes = new HashMap<>(attributes); +// FeatureDecision flagDecision; +// +// // Check Forced Decision +// OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); +// DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); +// decisionReasons.merge(forcedDecisionVariation.getReasons()); +// if (forcedDecisionVariation.getResult() != null) { +// flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); +// } else { +// // Regular decision +// DecisionResponse decisionVariation = decisionService.getVariationForFeature( +// flag, +// user, +// projectConfig, +// allOptions); +// flagDecision = decisionVariation.getResult(); +// decisionReasons.merge(decisionVariation.getReasons()); +// } +// +// Boolean flagEnabled = false; +// if (flagDecision.variation != null) { +// if (flagDecision.variation.getFeatureEnabled()) { +// flagEnabled = true; +// } +// } +// logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); +// +// Map variableMap = new HashMap<>(); +// if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { +// DecisionResponse> decisionVariables = getDecisionVariableMap( +// flag, +// flagDecision.variation, +// flagEnabled); +// variableMap = decisionVariables.getResult(); +// decisionReasons.merge(decisionVariables.getReasons()); +// } +// OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); +// +// FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; +// if (flagDecision.decisionSource != null) { +// decisionSource = flagDecision.decisionSource; +// } +// +// List reasonsToReport = decisionReasons.toReport(); +// String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; +// // TODO: add ruleKey values when available later. use a copy of experimentKey until then. +// // add to event metadata as well (currently set to experimentKey) +// String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; +// +// if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { +// decisionEventDispatched = sendImpression( +// projectConfig, +// flagDecision.experiment, +// userId, +// copiedAttributes, +// flagDecision.variation, +// key, +// decisionSource.toString(), +// flagEnabled); +// } +// +// DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() +// .withUserId(userId) +// .withAttributes(copiedAttributes) +// .withFlagKey(key) +// .withEnabled(flagEnabled) +// .withVariables(variableMap) +// .withVariationKey(variationKey) +// .withRuleKey(ruleKey) +// .withReasons(reasonsToReport) +// .withDecisionEventDispatched(decisionEventDispatched) +// .build(); +// notificationCenter.send(decisionNotification); +// +// return new OptimizelyDecision( +// variationKey, +// flagEnabled, +// optimizelyJSON, +// ruleKey, +// key, +// user, +// reasonsToReport); + } + + private OptimizelyDecision createOptimizelyDecision( + OptimizelyUserContext user, + String flagKey, + FeatureDecision flagDecision, + DecisionReasons decisionReasons, + List allOptions, + ProjectConfig projectConfig + ) { String userId = user.getUserId(); - Map attributes = user.getAttributes(); - Boolean decisionEventDispatched = false; - List allOptions = getAllOptions(options); - DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); - - Map copiedAttributes = new HashMap<>(attributes); - FeatureDecision flagDecision; - - // Check Forced Decision - OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); - DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); - decisionReasons.merge(forcedDecisionVariation.getReasons()); - if (forcedDecisionVariation.getResult() != null) { - flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); - } else { - // Regular decision - DecisionResponse decisionVariation = decisionService.getVariationForFeature( - flag, - user, - projectConfig, - allOptions); - flagDecision = decisionVariation.getResult(); - decisionReasons.merge(decisionVariation.getReasons()); - } Boolean flagEnabled = false; if (flagDecision.variation != null) { @@ -1359,12 +1436,12 @@ OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, flagEnabled = true; } } - logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); + logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", flagKey, userId, flagEnabled); Map variableMap = new HashMap<>(); if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { DecisionResponse> decisionVariables = getDecisionVariableMap( - flag, + projectConfig.getFeatureKeyMapping().get(flagKey), flagDecision.variation, flagEnabled); variableMap = decisionVariables.getResult(); @@ -1383,6 +1460,12 @@ OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, // add to event metadata as well (currently set to experimentKey) String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; + + Boolean decisionEventDispatched = false; + + Map attributes = user.getAttributes(); + Map copiedAttributes = new HashMap<>(attributes); + if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { decisionEventDispatched = sendImpression( projectConfig, @@ -1390,7 +1473,7 @@ OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, userId, copiedAttributes, flagDecision.variation, - key, + flagKey, decisionSource.toString(), flagEnabled); } @@ -1398,7 +1481,7 @@ OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() .withUserId(userId) .withAttributes(copiedAttributes) - .withFlagKey(key) + .withFlagKey(flagKey) .withEnabled(flagEnabled) .withVariables(variableMap) .withVariationKey(variationKey) @@ -1413,12 +1496,13 @@ OptimizelyDecision decideInternal(@Nonnull OptimizelyUserContext user, flagEnabled, optimizelyJSON, ruleKey, - key, + flagKey, user, reasonsToReport); } - - Map decideForKeysInternal(@Nonnull OptimizelyUserContext user, + + // TODO: UPS refactor cleanup + Map decideForKeys(@Nonnull OptimizelyUserContext user, @Nonnull List keys, @Nonnull List options) { Map decisionMap = new HashMap<>(); @@ -1439,7 +1523,7 @@ Map decideForKeysInternal(@Nonnull OptimizelyUserCon Map flagDecisions = new HashMap<>(); Map decisionReasonsMap = new HashMap<>(); - List keysWithoutForcedDecision = new ArrayList<>(); + List flagsWithoutForcedDecision = new ArrayList<>(); for (String key : keys) { FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); @@ -1455,41 +1539,58 @@ Map decideForKeysInternal(@Nonnull OptimizelyUserCon if (forcedDecision.isPresent()) { flagDecisions.put(key, forcedDecision.get()); } else { - keysWithoutForcedDecision.add(key); + flagsWithoutForcedDecision.add(flag); } - // OptimizelyDecision decision = decide(user, key, options); - // if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { - // decisionMap.put(key, decision); - // } } - return decisionMap; - } - - Map decideForKeys(@Nonnull OptimizelyUserContext user, - @Nonnull List keys, - @Nonnull List options) { - Map decisionMap = new HashMap<>(); + List> decisionList = + decisionService.getVariationsForFeatureList(flagsWithoutForcedDecision, user, projectConfig, allOptions); - ProjectConfig projectConfig = getProjectConfig(); - if (projectConfig == null) { - logger.error("Optimizely instance is not valid, failing isFeatureEnabled call."); - return decisionMap; + for (int i = 0; i < flagsWithoutForcedDecision.size(); i++) { + DecisionResponse decision = decisionList.get(i); + String flagKey = flagsWithoutForcedDecision.get(i).getKey(); + flagDecisions.put(flagKey, decision.getResult()); + decisionReasonsMap.get(flagKey).merge(decision.getReasons()); } - if (keys.isEmpty()) return decisionMap; - - List allOptions = getAllOptions(options); + for (Map.Entry entry: flagDecisions.entrySet()) { + String key = entry.getKey(); + FeatureDecision flagDecision = entry.getValue(); + DecisionReasons decisionReasons = decisionReasonsMap.get((key)); - for (String key : keys) { - OptimizelyDecision decision = decide(user, key, options); - if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { - decisionMap.put(key, decision); - } + OptimizelyDecision optimizelyDecision = createOptimizelyDecision( + user, key, flagDecision, decisionReasons, allOptions, projectConfig + ); + decisionMap.put(key, optimizelyDecision); } return decisionMap; } + +// Map decideForKeys(@Nonnull OptimizelyUserContext user, +// @Nonnull List keys, +// @Nonnull List options) { +// Map decisionMap = new HashMap<>(); +// +// ProjectConfig projectConfig = getProjectConfig(); +// if (projectConfig == null) { +// logger.error("Optimizely instance is not valid, failing isFeatureEnabled call."); +// return decisionMap; +// } +// +// if (keys.isEmpty()) return decisionMap; +// +// List allOptions = getAllOptions(options); +// +// for (String key : keys) { +// OptimizelyDecision decision = decide(user, key, options); +// if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { +// decisionMap.put(key, decision); +// } +// } +// +// return decisionMap; +// } Map decideAll(@Nonnull OptimizelyUserContext user, @Nonnull List options) { From 767f2ce93693ec288868c885bd0680394d6c9d95 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 05:31:06 +0600 Subject: [PATCH 4/7] fi --- .../com/optimizely/ab/bucketing/DecisionService.java | 2 ++ .../optimizely/ab/bucketing/DecisionServiceTest.java | 11 +++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index a958e1dda..2edb2b4ca 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -549,6 +549,8 @@ void updateUserProfile(@Nonnull Experiment experiment, decision = new Decision(variationId); } userProfile.experimentBucketMap.put(experimentId, decision); + logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", + variationId, experimentId, userProfile.userId); } /** diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 24898b34a..ddcfd93af 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -315,9 +315,9 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); - logbackVerifier.expectMessage(Level.INFO, - "The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" + - FEATURE_MULTI_VARIATE_FEATURE_KEY + "\"."); +// logbackVerifier.expectMessage(Level.INFO, +// "The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" + +// FEATURE_MULTI_VARIATE_FEATURE_KEY + "\"."); verify(spyFeatureFlag, times(2)).getExperimentIds(); verify(spyFeatureFlag, times(2)).getKey(); @@ -961,9 +961,12 @@ public void getVariationSavesBucketedVariationIntoUserProfile() throws Exception experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), noAudienceProjectConfig).getResult() ); logbackVerifier.expectMessage(Level.INFO, - String.format("Saved variation \"%s\" of experiment \"%s\" for user \"" + userProfileId + "\".", variation.getId(), + String.format("Updated variation \"%s\" of experiment \"%s\" for user \"" + userProfileId + "\".", variation.getId(), experiment.getId())); + logbackVerifier.expectMessage(Level.INFO, + String.format("Saved user profile of user \"%s\".", userProfileId)); + verify(userProfileService).save(eq(expectedUserProfile.toMap())); } From 4e2c4798ce287af2545cb584cd6c587b7743a018 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 20:25:43 +0600 Subject: [PATCH 5/7] update ups batching --- .../java/com/optimizely/ab/Optimizely.java | 14 +++-- .../ab/bucketing/DecisionService.java | 54 ++++++++++++------- .../ab/OptimizelyUserContextTest.java | 31 +++++++++-- .../ab/bucketing/DecisionServiceTest.java | 27 +++++++--- 4 files changed, 89 insertions(+), 37 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 ab061bdc4..fbbb1091c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1317,6 +1317,10 @@ Optional getForcedDecision(@Nonnull String flagKey, OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { + ProjectConfig projectConfig = getProjectConfig(); + if (projectConfig == null) { + return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); + } return decideForKeys(user, Arrays.asList(key), options).get(key); // ProjectConfig projectConfig = getProjectConfig(); @@ -1509,15 +1513,12 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use ProjectConfig projectConfig = getProjectConfig(); if (projectConfig == null) { - logger.error("Optimizely instance is not valid, failing isFeatureEnabled call."); + logger.error("Optimizely instance is not valid, failing decideForKeys call."); return decisionMap; } if (keys.isEmpty()) return decisionMap; - String userId = user.getUserId(); - Map attributes = user.getAttributes(); - Boolean decisionEventDispatched = false; List allOptions = getAllOptions(options); Map flagDecisions = new HashMap<>(); @@ -1561,7 +1562,10 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use OptimizelyDecision optimizelyDecision = createOptimizelyDecision( user, key, flagDecision, decisionReasons, allOptions, projectConfig ); - decisionMap.put(key, optimizelyDecision); + + if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || optimizelyDecision.getEnabled()) { + decisionMap.put(key, optimizelyDecision); + } } return decisionMap; diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 2edb2b4ca..083c7b120 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -83,7 +83,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options, - @Nonnull UserProfile userProfile, + @Nullable UserProfileTracker userProfileTracker, @Nullable DecisionReasons reasons) { if (reasons == null) { reasons = DefaultDecisionReasons.newInstance(); @@ -111,8 +111,8 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, return new DecisionResponse(variation, reasons); } - if (userProfile != null) { - decisionVariation = getStoredVariation(experiment, userProfile, projectConfig); + if (userProfileTracker != null) { + decisionVariation = getStoredVariation(experiment, userProfileTracker.userProfile, projectConfig); reasons.merge(decisionVariation.getReasons()); variation = decisionVariation.getResult(); // return the stored variation if it exists @@ -131,8 +131,9 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, variation = decisionVariation.getResult(); if (variation != null) { - if (userProfile != null) { - updateUserProfile(experiment, variation, userProfile); + if (userProfileTracker != null) { + updateUserProfile(experiment, variation, userProfileTracker.userProfile); + userProfileTracker.profileUpdated = true; } else { logger.debug("This decision will not be saved since the UserProfileService is null."); } @@ -164,16 +165,19 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, // fetch the user profile map from the user profile service boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); - UserProfile userProfile = null; +// UserProfile userProfile = null; + UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - userProfile = getUserProfile(user.getUserId(), reasons); + UserProfile userProfile = getUserProfile(user.getUserId(), reasons); + userProfileTracker = new UserProfileTracker(userProfile, false); } - DecisionResponse response = getVariation(experiment, user, projectConfig, options, userProfile, reasons); - if(userProfileService != null && !ignoreUPS) { - saveUserProfile(userProfile); + DecisionResponse response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons); + + if(userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) { + saveUserProfile(userProfileTracker.userProfile); } return response; } @@ -254,6 +258,16 @@ private UserProfile getUserProfile(String userId, DecisionReasons reasons) { return userProfile; } + static class UserProfileTracker { + public UserProfile userProfile; + public boolean profileUpdated; + + UserProfileTracker(UserProfile userProfile, boolean profileUpdated) { + this.userProfile = userProfile; + this.profileUpdated = profileUpdated; + } + } + /** * Get the variation the user is bucketed into for the FeatureFlag * @@ -271,10 +285,11 @@ public List> getVariationsForFeatureList(@Non DecisionReasons upsReasons = DefaultDecisionReasons.newInstance(); boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); - UserProfile userProfile = null; + UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - userProfile = getUserProfile(user.getUserId(), upsReasons); + UserProfile userProfile = getUserProfile(user.getUserId(), upsReasons); + userProfileTracker = new UserProfileTracker(userProfile, false); } List> decisions = new ArrayList<>(); @@ -283,7 +298,7 @@ public List> getVariationsForFeatureList(@Non DecisionReasons reasons = DefaultDecisionReasons.newInstance(); reasons.merge(upsReasons); - DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfile); + DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker); reasons.merge(decisionVariationResponse.getReasons()); FeatureDecision decision = decisionVariationResponse.getResult(); @@ -309,8 +324,8 @@ public List> getVariationsForFeatureList(@Non decisions.add(new DecisionResponse(decision, reasons)); } - if (userProfileService != null && !ignoreUPS) { - saveUserProfile(userProfile); + if (userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) { + saveUserProfile(userProfileTracker.userProfile); } return decisions; @@ -360,13 +375,14 @@ DecisionResponse getVariationFromExperiment(@Nonnull ProjectCon @Nonnull FeatureFlag featureFlag, @Nonnull OptimizelyUserContext user, @Nonnull List options, - @Nullable UserProfile userProfile) { + @Nullable UserProfileTracker userProfileTracker) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); - DecisionResponse decisionVariation = getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfile); + DecisionResponse decisionVariation = + getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker); reasons.merge(decisionVariation.getReasons()); Variation variation = decisionVariation.getResult(); @@ -777,7 +793,7 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje @Nonnull Experiment rule, @Nonnull OptimizelyUserContext user, @Nonnull List options, - @Nullable UserProfile userProfile) { + @Nullable UserProfileTracker userProfileTracker) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); String ruleKey = rule != null ? rule.getKey() : null; @@ -792,7 +808,7 @@ public DecisionResponse getVariationFromExperimentRule(@Nonnull Proje return new DecisionResponse(variation, reasons); } //regular decision - DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options, userProfile, null); + DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null); reasons.merge(decisionResponse.getReasons()); variation = decisionResponse.getResult(); 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 0c07ef56a..d5a91a9bb 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -23,7 +23,10 @@ import com.optimizely.ab.bucketing.UserProfileService; import com.optimizely.ab.config.*; import com.optimizely.ab.config.parser.ConfigParseException; +import com.optimizely.ab.event.EventHandler; +import com.optimizely.ab.event.EventProcessor; import com.optimizely.ab.event.ForwardingEventProcessor; +import com.optimizely.ab.event.internal.ImpressionEvent; import com.optimizely.ab.event.internal.payload.DecisionMetadata; import com.optimizely.ab.internal.LogbackVerifier; import com.optimizely.ab.notification.NotificationCenter; @@ -37,7 +40,10 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.*; import java.util.concurrent.CountDownLatch; @@ -345,9 +351,11 @@ public void decideAll_twoFlags() { @Test public void decideAll_allFlags() { + EventProcessor mockEventProcessor = mock(EventProcessor.class); + optimizely = new Optimizely.Builder() .withDatafile(datafile) - .withEventProcessor(new ForwardingEventProcessor(eventHandler, null)) + .withEventProcessor(mockEventProcessor) .build(); String flagKey1 = "feature_1"; @@ -361,7 +369,6 @@ public void decideAll_allFlags() { OptimizelyUserContext user = optimizely.createUserContext(userId, attributes); Map decisions = user.decideAll(); - assertTrue(decisions.size() == 3); assertEquals( @@ -395,9 +402,23 @@ public void decideAll_allFlags() { user, Collections.emptyList())); - eventHandler.expectImpression("10390977673", "10389729780", userId, attributes); - eventHandler.expectImpression("10420810910", "10418551353", userId, attributes); - eventHandler.expectImpression(null, "", userId, attributes); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ImpressionEvent.class); + verify(mockEventProcessor, times(3)).process(argumentCaptor.capture()); + + List sentEvents = argumentCaptor.getAllValues(); + assertEquals(sentEvents.size(), 3); + + assertEquals(sentEvents.get(2).getExperimentKey(), "exp_with_audience"); + assertEquals(sentEvents.get(2).getVariationKey(), "a"); + assertEquals(sentEvents.get(2).getUserContext().getUserId(), userId); + + + assertEquals(sentEvents.get(1).getExperimentKey(), "exp_no_audience"); + assertEquals(sentEvents.get(1).getVariationKey(), "variation_with_traffic"); + assertEquals(sentEvents.get(1).getUserContext().getUserId(), userId); + + assertEquals(sentEvents.get(0).getExperimentKey(), ""); + assertEquals(sentEvents.get(0).getUserContext().getUserId(), userId); } @Test diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index ddcfd93af..cafb3e048 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -24,6 +24,7 @@ import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.internal.ControlAttribute; import com.optimizely.ab.internal.LogbackVerifier; +import com.optimizely.ab.optimizelydecision.DecisionReasons; import com.optimizely.ab.optimizelydecision.DecisionResponse; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Before; @@ -297,7 +298,9 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment any(Experiment.class), any(OptimizelyUserContext.class), any(ProjectConfig.class), - anyObject() + anyObject(), + anyObject(), + any(DecisionReasons.class) ); // do not bucket to any rollouts doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(null, null, null))).when(decisionService).getVariationForFeatureInRollout( @@ -315,9 +318,9 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment assertNull(featureDecision.variation); assertNull(featureDecision.decisionSource); -// logbackVerifier.expectMessage(Level.INFO, -// "The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" + -// FEATURE_MULTI_VARIATE_FEATURE_KEY + "\"."); + logbackVerifier.expectMessage(Level.INFO, + "The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" + + FEATURE_MULTI_VARIATE_FEATURE_KEY + "\"."); verify(spyFeatureFlag, times(2)).getExperimentIds(); verify(spyFeatureFlag, times(2)).getKey(); @@ -381,7 +384,9 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() eq(featureExperiment), any(OptimizelyUserContext.class), any(ProjectConfig.class), - anyObject() + anyObject(), + anyObject(), + any(DecisionReasons.class) ); // return variation for rollout @@ -413,7 +418,9 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout() any(Experiment.class), any(OptimizelyUserContext.class), any(ProjectConfig.class), - anyObject() + anyObject(), + anyObject(), + any(DecisionReasons.class) ); } @@ -438,7 +445,9 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails eq(featureExperiment), any(OptimizelyUserContext.class), any(ProjectConfig.class), - anyObject() + anyObject(), + anyObject(), + any(DecisionReasons.class) ); // return variation for rollout @@ -470,7 +479,9 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails any(Experiment.class), any(OptimizelyUserContext.class), any(ProjectConfig.class), - anyObject() + anyObject(), + anyObject(), + any(DecisionReasons.class) ); logbackVerifier.expectMessage( From 437048c77f017f467ca04972179c885b8847be7b Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 20:38:30 +0600 Subject: [PATCH 6/7] update --- .vscode/settings.json | 3 - .../java/com/optimizely/ab/Optimizely.java | 235 +----------------- .../ab/bucketing/DecisionService.java | 53 +--- .../ab/OptimizelyUserContextTest.java | 2 +- .../ab/bucketing/DecisionServiceTest.java | 23 +- 5 files changed, 5 insertions(+), 311 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 0ca4d0be2..000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "java.compile.nullAnalysis.mode": "automatic" -} \ No newline at end of file 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 fbbb1091c..1e51ca743 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2016-2023, Optimizely, Inc. and contributors * + * Copyright 2016-2024, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -42,7 +42,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; -import javax.xml.catalog.CatalogFeatures.Feature; import java.io.Closeable; import java.util.*; @@ -1193,111 +1192,6 @@ private OptimizelyUserContext createUserContextCopy(@Nonnull String userId, @Non return new OptimizelyUserContext(this, userId, attributes, Collections.EMPTY_MAP, null, false); } -// OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, -// @Nonnull String key, -// @Nonnull List options) { -// -// ProjectConfig projectConfig = getProjectConfig(); -// if (projectConfig == null) { -// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); -// } -// -// FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); -// if (flag == null) { -// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); -// } -// -// String userId = user.getUserId(); -// Map attributes = user.getAttributes(); -// Boolean decisionEventDispatched = false; -// List allOptions = getAllOptions(options); -// DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); -// -// Map copiedAttributes = new HashMap<>(attributes); -// FeatureDecision flagDecision; -// -// // Check Forced Decision -// OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); -// DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); -// decisionReasons.merge(forcedDecisionVariation.getReasons()); -// if (forcedDecisionVariation.getResult() != null) { -// flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); -// } else { -// // Regular decision -// DecisionResponse decisionVariation = decisionService.getVariationForFeature( -// flag, -// user, -// projectConfig, -// allOptions); -// flagDecision = decisionVariation.getResult(); -// decisionReasons.merge(decisionVariation.getReasons()); -// } -// -// Boolean flagEnabled = false; -// if (flagDecision.variation != null) { -// if (flagDecision.variation.getFeatureEnabled()) { -// flagEnabled = true; -// } -// } -// logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); -// -// Map variableMap = new HashMap<>(); -// if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { -// DecisionResponse> decisionVariables = getDecisionVariableMap( -// flag, -// flagDecision.variation, -// flagEnabled); -// variableMap = decisionVariables.getResult(); -// decisionReasons.merge(decisionVariables.getReasons()); -// } -// OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); -// -// FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; -// if (flagDecision.decisionSource != null) { -// decisionSource = flagDecision.decisionSource; -// } -// -// List reasonsToReport = decisionReasons.toReport(); -// String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; -// // TODO: add ruleKey values when available later. use a copy of experimentKey until then. -// // add to event metadata as well (currently set to experimentKey) -// String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; -// -// if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { -// decisionEventDispatched = sendImpression( -// projectConfig, -// flagDecision.experiment, -// userId, -// copiedAttributes, -// flagDecision.variation, -// key, -// decisionSource.toString(), -// flagEnabled); -// } -// -// DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() -// .withUserId(userId) -// .withAttributes(copiedAttributes) -// .withFlagKey(key) -// .withEnabled(flagEnabled) -// .withVariables(variableMap) -// .withVariationKey(variationKey) -// .withRuleKey(ruleKey) -// .withReasons(reasonsToReport) -// .withDecisionEventDispatched(decisionEventDispatched) -// .build(); -// notificationCenter.send(decisionNotification); -// -// return new OptimizelyDecision( -// variationKey, -// flagEnabled, -// optimizelyJSON, -// ruleKey, -// key, -// user, -// reasonsToReport); -// } -// Optional getForcedDecision(@Nonnull String flagKey, @Nonnull DecisionReasons decisionReasons, @Nonnull ProjectConfig projectConfig, @@ -1313,7 +1207,6 @@ Optional getForcedDecision(@Nonnull String flagKey, return Optional.empty(); } - // TODO: UPS refactor cleanup OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { @@ -1322,106 +1215,6 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); } return decideForKeys(user, Arrays.asList(key), options).get(key); - -// ProjectConfig projectConfig = getProjectConfig(); -// if (projectConfig == null) { -// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); -// } -// -// FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); -// if (flag == null) { -// return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key)); -// } -// -// String userId = user.getUserId(); -// Map attributes = user.getAttributes(); -// Boolean decisionEventDispatched = false; -// List allOptions = getAllOptions(options); -// DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); -// -// Map copiedAttributes = new HashMap<>(attributes); -// FeatureDecision flagDecision; -// -// // Check Forced Decision -// OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flag.getKey(), null); -// DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); -// decisionReasons.merge(forcedDecisionVariation.getReasons()); -// if (forcedDecisionVariation.getResult() != null) { -// flagDecision = new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST); -// } else { -// // Regular decision -// DecisionResponse decisionVariation = decisionService.getVariationForFeature( -// flag, -// user, -// projectConfig, -// allOptions); -// flagDecision = decisionVariation.getResult(); -// decisionReasons.merge(decisionVariation.getReasons()); -// } -// -// Boolean flagEnabled = false; -// if (flagDecision.variation != null) { -// if (flagDecision.variation.getFeatureEnabled()) { -// flagEnabled = true; -// } -// } -// logger.info("Feature \"{}\" is enabled for user \"{}\"? {}", key, userId, flagEnabled); -// -// Map variableMap = new HashMap<>(); -// if (!allOptions.contains(OptimizelyDecideOption.EXCLUDE_VARIABLES)) { -// DecisionResponse> decisionVariables = getDecisionVariableMap( -// flag, -// flagDecision.variation, -// flagEnabled); -// variableMap = decisionVariables.getResult(); -// decisionReasons.merge(decisionVariables.getReasons()); -// } -// OptimizelyJSON optimizelyJSON = new OptimizelyJSON(variableMap); -// -// FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT; -// if (flagDecision.decisionSource != null) { -// decisionSource = flagDecision.decisionSource; -// } -// -// List reasonsToReport = decisionReasons.toReport(); -// String variationKey = flagDecision.variation != null ? flagDecision.variation.getKey() : null; -// // TODO: add ruleKey values when available later. use a copy of experimentKey until then. -// // add to event metadata as well (currently set to experimentKey) -// String ruleKey = flagDecision.experiment != null ? flagDecision.experiment.getKey() : null; -// -// if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { -// decisionEventDispatched = sendImpression( -// projectConfig, -// flagDecision.experiment, -// userId, -// copiedAttributes, -// flagDecision.variation, -// key, -// decisionSource.toString(), -// flagEnabled); -// } -// -// DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() -// .withUserId(userId) -// .withAttributes(copiedAttributes) -// .withFlagKey(key) -// .withEnabled(flagEnabled) -// .withVariables(variableMap) -// .withVariationKey(variationKey) -// .withRuleKey(ruleKey) -// .withReasons(reasonsToReport) -// .withDecisionEventDispatched(decisionEventDispatched) -// .build(); -// notificationCenter.send(decisionNotification); -// -// return new OptimizelyDecision( -// variationKey, -// flagEnabled, -// optimizelyJSON, -// ruleKey, -// key, -// user, -// reasonsToReport); } private OptimizelyDecision createOptimizelyDecision( @@ -1505,7 +1298,6 @@ private OptimizelyDecision createOptimizelyDecision( reasonsToReport); } - // TODO: UPS refactor cleanup Map decideForKeys(@Nonnull OptimizelyUserContext user, @Nonnull List keys, @Nonnull List options) { @@ -1570,31 +1362,6 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use return decisionMap; } - -// Map decideForKeys(@Nonnull OptimizelyUserContext user, -// @Nonnull List keys, -// @Nonnull List options) { -// Map decisionMap = new HashMap<>(); -// -// ProjectConfig projectConfig = getProjectConfig(); -// if (projectConfig == null) { -// logger.error("Optimizely instance is not valid, failing isFeatureEnabled call."); -// return decisionMap; -// } -// -// if (keys.isEmpty()) return decisionMap; -// -// List allOptions = getAllOptions(options); -// -// for (String key : keys) { -// OptimizelyDecision decision = decide(user, key, options); -// if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { -// decisionMap.put(key, decision); -// } -// } -// -// return decisionMap; -// } Map decideAll(@Nonnull OptimizelyUserContext user, @Nonnull List options) { diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 083c7b120..9a288130c 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017-2022, Optimizely, Inc. and contributors * + * Copyright 2017-2022, 2024, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -203,31 +203,6 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options) { -// DecisionReasons reasons = DefaultDecisionReasons.newInstance(); -// -// DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); -// reasons.merge(decisionVariationResponse.getReasons()); -// -// FeatureDecision decision = decisionVariationResponse.getResult(); -// if (decision != null) { -// return new DecisionResponse(decision, reasons); -// } -// -// DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); -// reasons.merge(decisionFeatureResponse.getReasons()); -// decision = decisionFeatureResponse.getResult(); -// -// String message; -// if (decision.variation == null) { -// message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", -// user.getUserId(), featureFlag.getKey()); -// } else { -// message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", -// user.getUserId(), featureFlag.getKey()); -// } -// logger.info(message); -// -// return new DecisionResponse(decision, reasons); return getVariationsForFeatureList(Arrays.asList(featureFlag), user, projectConfig, options).get(0); } @@ -329,30 +304,6 @@ public List> getVariationsForFeatureList(@Non } return decisions; - -// DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); -// reasons.merge(decisionVariationResponse.getReasons()); -// -// FeatureDecision decision = decisionVariationResponse.getResult(); -// if (decision != null) { -// return new DecisionResponse(decision, reasons); -// } - -// DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); -// reasons.merge(decisionFeatureResponse.getReasons()); -// decision = decisionFeatureResponse.getResult(); -// -// String message; -// if (decision.variation == null) { -// message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", -// user.getUserId(), featureFlag.getKey()); -// } else { -// message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", -// user.getUserId(), featureFlag.getKey()); -// } -// logger.info(message); -// -// return new DecisionResponse(decision, reasons); } @Nonnull @@ -788,7 +739,7 @@ public DecisionResponse getForcedVariation(@Nonnull Experiment experi } - public DecisionResponse getVariationFromExperimentRule(@Nonnull ProjectConfig projectConfig, + private DecisionResponse getVariationFromExperimentRule(@Nonnull ProjectConfig projectConfig, @Nonnull String flagKey, @Nonnull Experiment rule, @Nonnull OptimizelyUserContext user, 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 d5a91a9bb..e972b4de3 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 2021-2023, Optimizely and contributors + * Copyright 2021-2024, 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. diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index cafb3e048..350e05b28 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017-2022, Optimizely, Inc. and contributors * + * Copyright 2017-2022, 2024, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -754,27 +754,6 @@ public void getVariationFromDeliveryRuleTest() { assertFalse(skipToEveryoneElse); } -// @Test -// public void getVariationFromExperimentRuleTest() { -// int index = 3; -// Experiment experiment = ROLLOUT_2.getExperiments().get(index); -// Variation expectedVariation = null; -// for (Variation variation : experiment.getVariations()) { -// if (variation.getKey().equals("3137445031")) { -// expectedVariation = variation; -// } -// } -// DecisionResponse decisionResponse = decisionService.getVariationFromExperimentRule( -// v4ProjectConfig, -// FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(), -// experiment, -// optimizely.createUserContext(genericUserId, Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, AUDIENCE_ENGLISH_CITIZENS_VALUE)), -// Collections.emptyList() -// ); -// -// assertEquals(expectedVariation, decisionResponse.getResult()); -// } - @Test public void validatedForcedDecisionWithRuleKey() { String userId = "testUser1"; From 321dba53933931be91ddcbd6cea1673512dfc5a6 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Mon, 21 Oct 2024 17:23:43 +0600 Subject: [PATCH 7/7] fix event disptach order --- .../src/main/java/com/optimizely/ab/Optimizely.java | 9 ++++++--- .../com/optimizely/ab/OptimizelyUserContextTest.java | 10 +++++----- 2 files changed, 11 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 1e51ca743..0b3cb8d8c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1318,6 +1318,8 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use List flagsWithoutForcedDecision = new ArrayList<>(); + List validKeys = new ArrayList<>(); + for (String key : keys) { FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); if (flag == null) { @@ -1325,6 +1327,8 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use continue; } + validKeys.add(key); + DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); Optional forcedDecision = getForcedDecision(key, decisionReasons, projectConfig, user); decisionReasonsMap.put(key, decisionReasons); @@ -1346,9 +1350,8 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use decisionReasonsMap.get(flagKey).merge(decision.getReasons()); } - for (Map.Entry entry: flagDecisions.entrySet()) { - String key = entry.getKey(); - FeatureDecision flagDecision = entry.getValue(); + for (String key: validKeys) { + FeatureDecision flagDecision = flagDecisions.get(key); DecisionReasons decisionReasons = decisionReasonsMap.get((key)); OptimizelyDecision optimizelyDecision = createOptimizelyDecision( 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 e972b4de3..54213dc6a 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -408,17 +408,17 @@ public void decideAll_allFlags() { List sentEvents = argumentCaptor.getAllValues(); assertEquals(sentEvents.size(), 3); - assertEquals(sentEvents.get(2).getExperimentKey(), "exp_with_audience"); - assertEquals(sentEvents.get(2).getVariationKey(), "a"); - assertEquals(sentEvents.get(2).getUserContext().getUserId(), userId); + assertEquals(sentEvents.get(0).getExperimentKey(), "exp_with_audience"); + assertEquals(sentEvents.get(0).getVariationKey(), "a"); + assertEquals(sentEvents.get(0).getUserContext().getUserId(), userId); assertEquals(sentEvents.get(1).getExperimentKey(), "exp_no_audience"); assertEquals(sentEvents.get(1).getVariationKey(), "variation_with_traffic"); assertEquals(sentEvents.get(1).getUserContext().getUserId(), userId); - assertEquals(sentEvents.get(0).getExperimentKey(), ""); - assertEquals(sentEvents.get(0).getUserContext().getUserId(), userId); + assertEquals(sentEvents.get(2).getExperimentKey(), ""); + assertEquals(sentEvents.get(2).getUserContext().getUserId(), userId); } @Test