From 95768e678f29a77df2377e4091edd7e86574cd96 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Fri, 18 Oct 2024 03:55:12 +0600 Subject: [PATCH 01/24] 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 02/24] 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 03/24] 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 04/24] 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 05/24] 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 06/24] 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 8034de5bf89fdf3cd525b413930a507cb0542586 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Sat, 19 Oct 2024 04:26:55 +0600 Subject: [PATCH 07/24] update --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 8 +++++++- .../com/optimizely/ab/bucketing/DecisionServiceTest.java | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) 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..1bf26de2d 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -45,6 +45,7 @@ import java.io.Closeable; import java.util.*; +import java.util.stream.Collectors; import static com.optimizely.ab.internal.SafetyUtils.tryClose; @@ -1211,10 +1212,15 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { ProjectConfig projectConfig = getProjectConfig(); + List filteredOptions = options.stream() + .filter(opt -> opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY) + .collect(Collectors.toList()); + if (projectConfig == null) { return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); } - return decideForKeys(user, Arrays.asList(key), options).get(key); + + return decideForKeys(user, Arrays.asList(key), filteredOptions).get(key); } private OptimizelyDecision createOptimizelyDecision( 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 350e05b28..10e37b5f6 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 @@ -491,6 +491,9 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails ); } + //========== getVariationForFeatureList tests ==========// + + //========== getVariationForFeatureInRollout tests ==========// /** From 3087bad9e8a09bc8aedcef90d2f67212e1442b24 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Sat, 19 Oct 2024 04:56:41 +0600 Subject: [PATCH 08/24] fix --- .../java/com/optimizely/ab/Optimizely.java | 22 ++++++++++++++++--- .../ab/OptimizelyUserContextTest.java | 10 ++++----- 2 files changed, 24 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 1bf26de2d..03dd83e0d 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1324,6 +1324,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) { @@ -1331,6 +1333,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); @@ -1352,9 +1356,21 @@ 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 (Map.Entry entry: flagDecisions.entrySet()) { +// String key = entry.getKey(); +// FeatureDecision flagDecision = entry.getValue(); +// DecisionReasons decisionReasons = decisionReasonsMap.get((key)); +// +// OptimizelyDecision optimizelyDecision = createOptimizelyDecision( +// user, key, flagDecision, decisionReasons, allOptions, projectConfig +// ); +// +// if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || optimizelyDecision.getEnabled()) { +// decisionMap.put(key, optimizelyDecision); +// } +// } + 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 From fb5744b2440cbef06d241ecfa34609a8eaf4e59a Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Sat, 19 Oct 2024 07:19:39 +0600 Subject: [PATCH 09/24] add tests --- .../ab/OptimizelyUserContextTest.java | 37 ++++++++++++++++++- .../ab/bucketing/DecisionServiceTest.java | 24 ++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 54213dc6a..eaadea715 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -20,7 +20,9 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.bucketing.FeatureDecision; +import com.optimizely.ab.bucketing.UserProfile; import com.optimizely.ab.bucketing.UserProfileService; +import com.optimizely.ab.bucketing.UserProfileUtils; import com.optimizely.ab.config.*; import com.optimizely.ab.config.parser.ConfigParseException; import com.optimizely.ab.event.EventHandler; @@ -369,7 +371,7 @@ public void decideAll_allFlags() { OptimizelyUserContext user = optimizely.createUserContext(userId, attributes); Map decisions = user.decideAll(); - assertTrue(decisions.size() == 3); + assertEquals(decisions.size(), 3); assertEquals( decisions.get(flagKey1), @@ -421,6 +423,39 @@ public void decideAll_allFlags() { assertEquals(sentEvents.get(2).getUserContext().getUserId(), userId); } + @Test + public void decideForKeys_ups_batching() throws Exception { + UserProfileService ups = mock(UserProfileService.class); + + optimizely = new Optimizely.Builder() + .withDatafile(datafile) + .withUserProfileService(ups) + .build(); + + String flagKey1 = "feature_1"; + String flagKey2 = "feature_2"; + String flagKey3 = "feature_3"; + Map attributes = Collections.singletonMap("gender", "f"); + + OptimizelyUserContext user = optimizely.createUserContext(userId, attributes); + Map decisions = user.decideForKeys(Arrays.asList( + flagKey1, flagKey2, flagKey3 + )); + + assertEquals(decisions.size(), 3); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Map.class); + + + verify(ups, times(1)).lookup(userId); + verify(ups, times(1)).save(argumentCaptor.capture()); + + Map savedUps = argumentCaptor.getValue(); + UserProfile savedProfile = UserProfileUtils.convertMapToUserProfile(savedUps); + + assertEquals(savedProfile.userId, userId); + } + @Test public void decideAll_allFlags_enabledFlagsOnly() { String flagKey1 = "feature_1"; 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 10e37b5f6..d818826d4 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 @@ -493,6 +493,30 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails //========== getVariationForFeatureList tests ==========// + @Test + public void getVariationsForFeatureListBatchesUpsLoadAndSave() throws Exception { + Bucketer bucketer = new Bucketer(); + ErrorHandler mockErrorHandler = mock(ErrorHandler.class); + UserProfileService mockUserProfileService = mock(UserProfileService.class); + + DecisionService decisionService = new DecisionService(bucketer, mockErrorHandler, mockUserProfileService); + + FeatureFlag featureFlag1 = FEATURE_FLAG_MULTI_VARIATE_FEATURE; + FeatureFlag featureFlag2 = FEATURE_FLAG_MULTI_VARIATE_FUTURE_FEATURE; + FeatureFlag featureFlag3 = FEATURE_FLAG_MUTEX_GROUP_FEATURE; + + List> decisions = decisionService.getVariationsForFeatureList( + Arrays.asList(featureFlag1, featureFlag2, featureFlag3), + optimizely.createUserContext(genericUserId), + v4ProjectConfig, + new ArrayList<>() + ); + + assertEquals(decisions.size(), 3); + verify(mockUserProfileService, times(1)).lookup(genericUserId); + verify(mockUserProfileService, times(1)).save(anyObject()); + } + //========== getVariationForFeatureInRollout tests ==========// From fc31565dae5dc40bc188b3d831a98a55ba7fe743 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Sat, 19 Oct 2024 07:23:59 +0600 Subject: [PATCH 10/24] add test --- .../ab/OptimizelyUserContextTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index eaadea715..34cf61543 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -456,6 +456,34 @@ public void decideForKeys_ups_batching() throws Exception { assertEquals(savedProfile.userId, userId); } + @Test + public void decideAll_ups_batching() throws Exception { + UserProfileService ups = mock(UserProfileService.class); + + optimizely = new Optimizely.Builder() + .withDatafile(datafile) + .withUserProfileService(ups) + .build(); + + Map attributes = Collections.singletonMap("gender", "f"); + + OptimizelyUserContext user = optimizely.createUserContext(userId, attributes); + Map decisions = user.decideAll(); + + assertEquals(decisions.size(), 3); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Map.class); + + + verify(ups, times(1)).lookup(userId); + verify(ups, times(1)).save(argumentCaptor.capture()); + + Map savedUps = argumentCaptor.getValue(); + UserProfile savedProfile = UserProfileUtils.convertMapToUserProfile(savedUps); + + assertEquals(savedProfile.userId, userId); + } + @Test public void decideAll_allFlags_enabledFlagsOnly() { String flagKey1 = "feature_1"; From 56a041bf11e20b17d205c89e38e2755a6b9e071e Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Sat, 19 Oct 2024 23:56:02 +0600 Subject: [PATCH 11/24] up --- .../ab/bucketing/DecisionService.java | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 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 9a288130c..5a0c51f15 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 @@ -132,8 +132,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, if (variation != null) { if (userProfileTracker != null) { - updateUserProfile(experiment, variation, userProfileTracker.userProfile); - userProfileTracker.profileUpdated = true; + userProfileTracker.updateUserProfile(experiment, variation); } else { logger.debug("This decision will not be saved since the UserProfileService is null."); } @@ -233,7 +232,7 @@ private UserProfile getUserProfile(String userId, DecisionReasons reasons) { return userProfile; } - static class UserProfileTracker { + class UserProfileTracker { public UserProfile userProfile; public boolean profileUpdated; @@ -241,6 +240,24 @@ static class UserProfileTracker { this.userProfile = userProfile; this.profileUpdated = profileUpdated; } + + void updateUserProfile(@Nonnull Experiment experiment, + @Nonnull Variation variation) { + + 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); + profileUpdated = true; + logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", + variationId, experimentId, userProfile.userId); + } } /** @@ -495,31 +512,6 @@ 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); - logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", - variationId, experimentId, userProfile.userId); - } - /** * Save a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfileService}. * From 3562ae4f1cc679a5e54e21a40f0767946da71c2d Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Mon, 21 Oct 2024 17:51:56 +0600 Subject: [PATCH 12/24] fix fsc failures --- .../java/com/optimizely/ab/Optimizely.java | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 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 03dd83e0d..1c09b2b36 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1212,15 +1212,15 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { ProjectConfig projectConfig = getProjectConfig(); - List filteredOptions = options.stream() - .filter(opt -> opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY) - .collect(Collectors.toList()); - if (projectConfig == null) { return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); } - return decideForKeys(user, Arrays.asList(key), filteredOptions).get(key); + List filteredOptions = getAllOptions(options).stream() + .filter(opt -> opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY) + .collect(Collectors.toList()); + + return decideForKeys(user, Arrays.asList(key), filteredOptions, true).get(key); } private OptimizelyDecision createOptimizelyDecision( @@ -1305,8 +1305,15 @@ private OptimizelyDecision createOptimizelyDecision( } Map decideForKeys(@Nonnull OptimizelyUserContext user, + @Nonnull List keys, + @Nonnull List options) { + return decideForKeys(user, keys, options, false); + } + + private Map decideForKeys(@Nonnull OptimizelyUserContext user, @Nonnull List keys, - @Nonnull List options) { + @Nonnull List options, + boolean ignoreDefaultOptions) { Map decisionMap = new HashMap<>(); ProjectConfig projectConfig = getProjectConfig(); @@ -1317,7 +1324,7 @@ Map decideForKeys(@Nonnull OptimizelyUserContext use if (keys.isEmpty()) return decisionMap; - List allOptions = getAllOptions(options); + List allOptions = ignoreDefaultOptions ? options: getAllOptions(options); Map flagDecisions = new HashMap<>(); Map decisionReasonsMap = new HashMap<>(); @@ -1356,19 +1363,6 @@ 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(); -// DecisionReasons decisionReasons = decisionReasonsMap.get((key)); -// -// OptimizelyDecision optimizelyDecision = createOptimizelyDecision( -// user, key, flagDecision, decisionReasons, allOptions, projectConfig -// ); -// -// if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || optimizelyDecision.getEnabled()) { -// decisionMap.put(key, optimizelyDecision); -// } -// } for (String key: validKeys) { FeatureDecision flagDecision = flagDecisions.get(key); DecisionReasons decisionReasons = decisionReasonsMap.get((key)); From a1cc6085a8ca52566c5ec738a7bf20411a16108a Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Mon, 21 Oct 2024 18:29:33 +0600 Subject: [PATCH 13/24] static --- .../com/optimizely/ab/bucketing/DecisionService.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 5a0c51f15..fbe5aca7c 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 @@ -232,7 +232,7 @@ private UserProfile getUserProfile(String userId, DecisionReasons reasons) { return userProfile; } - class UserProfileTracker { + static class UserProfileTracker { public UserProfile userProfile; public boolean profileUpdated; @@ -261,12 +261,12 @@ void updateUserProfile(@Nonnull Experiment experiment, } /** - * Get the variation the user is bucketed into for the FeatureFlag + * Get the variations the user is bucketed into for the the list of feature flags * * @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 + * @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 From 08d66621b083672e78851425c59c50065ff9dec5 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 19:07:26 +0600 Subject: [PATCH 14/24] fix review comments --- .../java/com/optimizely/ab/Optimizely.java | 25 +--- .../ab/bucketing/DecisionService.java | 125 +++++++++--------- 2 files changed, 73 insertions(+), 77 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 1c09b2b36..69c4f6647 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1193,21 +1193,6 @@ private OptimizelyUserContext createUserContextCopy(@Nonnull String userId, @Non return new OptimizelyUserContext(this, userId, attributes, Collections.EMPTY_MAP, null, false); } - 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 decide(@Nonnull OptimizelyUserContext user, @Nonnull String key, @Nonnull List options) { @@ -1343,11 +1328,15 @@ private Map decideForKeys(@Nonnull OptimizelyUserCon validKeys.add(key); DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); - Optional forcedDecision = getForcedDecision(key, decisionReasons, projectConfig, user); decisionReasonsMap.put(key, decisionReasons); - if (forcedDecision.isPresent()) { - flagDecisions.put(key, forcedDecision.get()); + OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(key, null); + DecisionResponse forcedDecisionVariation = decisionService.validatedForcedDecision(optimizelyDecisionContext, projectConfig, user); + decisionReasons.merge(forcedDecisionVariation.getReasons()); + + if (forcedDecisionVariation.getResult() != null) { + flagDecisions.put(key, + new FeatureDecision(null, forcedDecisionVariation.getResult(), FeatureDecision.DecisionSource.FEATURE_TEST)); } else { flagsWithoutForcedDecision.add(flag); } 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 fbe5aca7c..51c3b385f 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,6 +78,17 @@ 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 + * @param userProfileTracker tracker for reading and updating user profile of the user + * @param reasons Decision reasons + * @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, @@ -164,19 +175,17 @@ 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; UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - UserProfile userProfile = getUserProfile(user.getUserId(), reasons); - userProfileTracker = new UserProfileTracker(userProfile, false); + userProfileTracker = new UserProfileTracker(user.getUserId()); + userProfileTracker.loadUserProfile(reasons); } - DecisionResponse response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons); - if(userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) { - saveUserProfile(userProfileTracker.userProfile); + if(userProfileService != null && !ignoreUPS) { + userProfileTracker.saveUserProfile(); } return response; } @@ -205,45 +214,42 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature return getVariationsForFeatureList(Arrays.asList(featureFlag), user, projectConfig, options).get(0); } - private UserProfile getUserProfile(String userId, DecisionReasons reasons) { - UserProfile userProfile = null; + class UserProfileTracker { + private UserProfile userProfile; + private boolean profileUpdated; + private String userId; - 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)); + UserProfileTracker(String userId) { + this.userId = userId; + this.profileUpdated = false; + this.userProfile = null; } - if (userProfile == null) { - userProfile = new UserProfile(userId, new HashMap()); - } - - return userProfile; - } - - static class UserProfileTracker { - public UserProfile userProfile; - public boolean profileUpdated; + public void loadUserProfile(DecisionReasons 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)); + } - UserProfileTracker(UserProfile userProfile, boolean profileUpdated) { - this.userProfile = userProfile; - this.profileUpdated = profileUpdated; + if (userProfile == null) { + userProfile = new UserProfile(userId, new HashMap()); + } } - void updateUserProfile(@Nonnull Experiment experiment, + public void updateUserProfile(@Nonnull Experiment experiment, @Nonnull Variation variation) { - String experimentId = experiment.getId(); String variationId = variation.getId(); Decision decision; @@ -258,10 +264,27 @@ void updateUserProfile(@Nonnull Experiment experiment, logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", variationId, experimentId, userProfile.userId); } + + public void saveUserProfile() { + // if there were no updates, no need to save + if (!this.profileUpdated) { + 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 variations the user is bucketed into for the the list of feature flags + * Get the variations the user is bucketed into for the list of feature flags * * @param featureFlags The feature flag list the user wants to access. * @param user The current OptimizelyuserContext @@ -280,8 +303,8 @@ public List> getVariationsForFeatureList(@Non UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - UserProfile userProfile = getUserProfile(user.getUserId(), upsReasons); - userProfileTracker = new UserProfileTracker(userProfile, false); + userProfileTracker = new UserProfileTracker(user.getUserId()); + userProfileTracker.loadUserProfile(upsReasons); } List> decisions = new ArrayList<>(); @@ -316,8 +339,8 @@ public List> getVariationsForFeatureList(@Non decisions.add(new DecisionResponse(decision, reasons)); } - if (userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) { - saveUserProfile(userProfileTracker.userProfile); + if (userProfileService != null && !ignoreUPS) { + userProfileTracker.saveUserProfile(); } return decisions; @@ -548,22 +571,6 @@ 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. * From 6c1d4ba2459e6a698065aade80c1e2fb84f27e54 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 19:29:13 +0600 Subject: [PATCH 15/24] try fixing ci --- build.gradle | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/build.gradle b/build.gradle index b8405e39b..35b8f3392 100644 --- a/build.gradle +++ b/build.gradle @@ -1,3 +1,12 @@ +buildscript { + repositories { + jcenter() + maven { + url 'https://plugins.gradle.org/m2/' + } + } +} + plugins { id 'com.github.kt3k.coveralls' version '2.8.2' id 'jacoco' From 667d55f1897e1e085572729da9c7acda00c067f9 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 19:38:24 +0600 Subject: [PATCH 16/24] upd --- build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/build.gradle b/build.gradle index 35b8f3392..1c7848941 100644 --- a/build.gradle +++ b/build.gradle @@ -1,6 +1,5 @@ buildscript { repositories { - jcenter() maven { url 'https://plugins.gradle.org/m2/' } From 899faa96d9b5131e6ff02b8da66c5d6a0639f1d1 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 20:02:33 +0600 Subject: [PATCH 17/24] edit --- build.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 1c7848941..1f03b51ff 100644 --- a/build.gradle +++ b/build.gradle @@ -1,8 +1,6 @@ buildscript { repositories { - maven { - url 'https://plugins.gradle.org/m2/' - } + mavenCentral() } } From 79ea882f61f680f59d83d3baf09755fea277ccc3 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 20:08:15 +0600 Subject: [PATCH 18/24] up --- build.gradle | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/build.gradle b/build.gradle index 1f03b51ff..b79880a54 100644 --- a/build.gradle +++ b/build.gradle @@ -1,14 +1,8 @@ -buildscript { - repositories { - mavenCentral() - } -} - plugins { id 'com.github.kt3k.coveralls' version '2.8.2' id 'jacoco' id 'me.champeau.gradle.jmh' version '0.4.5' - id 'nebula.optional-base' version '3.2.0' + id 'nebula.optional-base' version '7.0.0' id 'com.github.hierynomus.license' version '0.15.0' id 'com.github.spotbugs' version "4.5.0" } From 468168a566d5642d435955a38dc4ff0520889902 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 20:12:45 +0600 Subject: [PATCH 19/24] up --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index b79880a54..b8405e39b 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { id 'com.github.kt3k.coveralls' version '2.8.2' id 'jacoco' id 'me.champeau.gradle.jmh' version '0.4.5' - id 'nebula.optional-base' version '7.0.0' + id 'nebula.optional-base' version '3.2.0' id 'com.github.hierynomus.license' version '0.15.0' id 'com.github.spotbugs' version "4.5.0" } From 36f3d6febbf5a70a8c2f742e9e6cb4c2f49756bc Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 21:12:56 +0600 Subject: [PATCH 20/24] stream removal --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 7 +++---- 1 file changed, 3 insertions(+), 4 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 69c4f6647..0e260072e 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1201,11 +1201,10 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason()); } - List filteredOptions = getAllOptions(options).stream() - .filter(opt -> opt != OptimizelyDecideOption.ENABLED_FLAGS_ONLY) - .collect(Collectors.toList()); + List allOptions = getAllOptions(options); + allOptions.remove(OptimizelyDecideOption.ENABLED_FLAGS_ONLY); - return decideForKeys(user, Arrays.asList(key), filteredOptions, true).get(key); + return decideForKeys(user, Arrays.asList(key), allOptions, true).get(key); } private OptimizelyDecision createOptimizelyDecision( From 3181043614658f7536a9a3f3982f6e2daa63fec1 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 23:48:16 +0600 Subject: [PATCH 21/24] move class --- .../ab/bucketing/DecisionService.java | 83 ++----------- .../ab/bucketing/UserProfileTracker.java | 109 ++++++++++++++++++ 2 files changed, 116 insertions(+), 76 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java 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 51c3b385f..e882b8ca0 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 @@ -123,7 +123,7 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, } if (userProfileTracker != null) { - decisionVariation = getStoredVariation(experiment, userProfileTracker.userProfile, projectConfig); + decisionVariation = getStoredVariation(experiment, userProfileTracker.getUserProfile(), projectConfig); reasons.merge(decisionVariation.getReasons()); variation = decisionVariation.getResult(); // return the stored variation if it exists @@ -178,14 +178,14 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - userProfileTracker = new UserProfileTracker(user.getUserId()); - userProfileTracker.loadUserProfile(reasons); + userProfileTracker = new UserProfileTracker(user.getUserId(), userProfileService, logger); + userProfileTracker.loadUserProfile(reasons, errorHandler); } DecisionResponse response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons); if(userProfileService != null && !ignoreUPS) { - userProfileTracker.saveUserProfile(); + userProfileTracker.saveUserProfile(errorHandler); } return response; } @@ -214,75 +214,6 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature return getVariationsForFeatureList(Arrays.asList(featureFlag), user, projectConfig, options).get(0); } - class UserProfileTracker { - private UserProfile userProfile; - private boolean profileUpdated; - private String userId; - - UserProfileTracker(String userId) { - this.userId = userId; - this.profileUpdated = false; - this.userProfile = null; - } - - public void loadUserProfile(DecisionReasons 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)); - } - - if (userProfile == null) { - userProfile = new UserProfile(userId, new HashMap()); - } - } - - public void updateUserProfile(@Nonnull Experiment experiment, - @Nonnull Variation variation) { - 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); - profileUpdated = true; - logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", - variationId, experimentId, userProfile.userId); - } - - public void saveUserProfile() { - // if there were no updates, no need to save - if (!this.profileUpdated) { - 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 variations the user is bucketed into for the list of feature flags * @@ -303,8 +234,8 @@ public List> getVariationsForFeatureList(@Non UserProfileTracker userProfileTracker = null; if (userProfileService != null && !ignoreUPS) { - userProfileTracker = new UserProfileTracker(user.getUserId()); - userProfileTracker.loadUserProfile(upsReasons); + userProfileTracker = new UserProfileTracker(user.getUserId(), userProfileService, logger); + userProfileTracker.loadUserProfile(upsReasons, errorHandler); } List> decisions = new ArrayList<>(); @@ -340,7 +271,7 @@ public List> getVariationsForFeatureList(@Non } if (userProfileService != null && !ignoreUPS) { - userProfileTracker.saveUserProfile(); + userProfileTracker.saveUserProfile(errorHandler); } return decisions; diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java new file mode 100644 index 000000000..904ca86be --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java @@ -0,0 +1,109 @@ +/**************************************************************************** + * 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. * + * 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.bucketing; + +import com.optimizely.ab.OptimizelyRuntimeException; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Variation; +import com.optimizely.ab.error.ErrorHandler; +import com.optimizely.ab.optimizelydecision.DecisionReasons; + +import javax.annotation.Nonnull; +import java.util.HashMap; +import java.util.Map; + +import org.slf4j.Logger; + +class UserProfileTracker { + private UserProfileService userProfileService; + private Logger logger; + private UserProfile userProfile; + private boolean profileUpdated; + private String userId; + + UserProfileTracker( + @Nonnull String userId, + @Nonnull UserProfileService userProfileService, + @Nonnull Logger logger + ) { + this.userId = userId; + this.userProfileService = userProfileService; + this.logger = logger; + this.profileUpdated = false; + this.userProfile = null; + } + + public UserProfile getUserProfile() { + return userProfile; + } + + public void loadUserProfile(DecisionReasons reasons, ErrorHandler errorHandler) { + 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)); + } + + if (userProfile == null) { + userProfile = new UserProfile(userId, new HashMap()); + } + } + + public void updateUserProfile(@Nonnull Experiment experiment, + @Nonnull Variation variation) { + 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); + profileUpdated = true; + logger.info("Updated variation \"{}\" of experiment \"{}\" for user \"{}\".", + variationId, experimentId, userProfile.userId); + } + + public void saveUserProfile(ErrorHandler errorHandler) { + // if there were no updates, no need to save + if (!this.profileUpdated) { + 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)); + } + } +} \ No newline at end of file From abb10cd4e1c2dcbe9bb27ba3e09f4c13daa5bfc4 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 23:56:09 +0600 Subject: [PATCH 22/24] update --- .../main/java/com/optimizely/ab/bucketing/DecisionService.java | 3 +++ 1 file changed, 3 insertions(+) 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 e882b8ca0..ff48ffb99 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 @@ -419,6 +419,9 @@ DecisionResponse getWhitelistedVariation(@Nonnull Experiment experime return new DecisionResponse(null, reasons); } + + // TODO: Logically, it makes sense to move this method to UserProfileTracker. But some tests are also calling this + // method, requiring us to refactor those tests as well. We'll look to refactor this later. /** * Get the {@link Variation} that has been stored for the user in the {@link UserProfileService} implementation. * From 6893b6295e4e926c3fe49cbc2ca73465521ea22d Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 23:58:00 +0600 Subject: [PATCH 23/24] up --- .../java/com/optimizely/ab/bucketing/UserProfileTracker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java index 904ca86be..53554bd1a 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017-2022, 2024, Optimizely, Inc. and contributors * + * Copyright 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. * From 72b111f2e366bd8757490feb7cb29474a6166702 Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Tue, 22 Oct 2024 23:59:44 +0600 Subject: [PATCH 24/24] update --- .../java/com/optimizely/ab/bucketing/UserProfileTracker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java index 53554bd1a..2dee3d171 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2024, Optimizely, Inc. and contributors * + * Copyright 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. *