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..0e260072e 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,8 +42,10 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; + import java.io.Closeable; import java.util.*; +import java.util.stream.Collectors; import static com.optimizely.ab.internal.SafetyUtils.tryClose; @@ -1194,42 +1196,26 @@ private OptimizelyUserContext createUserContextCopy(@Nonnull String userId, @Non 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); + allOptions.remove(OptimizelyDecideOption.ENABLED_FLAGS_ONLY); - 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()); - } + return decideForKeys(user, Arrays.asList(key), allOptions, true).get(key); + } + + private OptimizelyDecision createOptimizelyDecision( + OptimizelyUserContext user, + String flagKey, + FeatureDecision flagDecision, + DecisionReasons decisionReasons, + List allOptions, + ProjectConfig projectConfig + ) { + String userId = user.getUserId(); Boolean flagEnabled = false; if (flagDecision.variation != null) { @@ -1237,12 +1223,12 @@ OptimizelyDecision decide(@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(); @@ -1261,6 +1247,12 @@ OptimizelyDecision decide(@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, @@ -1268,7 +1260,7 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, userId, copiedAttributes, flagDecision.variation, - key, + flagKey, decisionSource.toString(), flagEnabled); } @@ -1276,7 +1268,7 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() .withUserId(userId) .withAttributes(copiedAttributes) - .withFlagKey(key) + .withFlagKey(flagKey) .withEnabled(flagEnabled) .withVariables(variableMap) .withVariationKey(variationKey) @@ -1291,30 +1283,84 @@ OptimizelyDecision decide(@Nonnull OptimizelyUserContext user, flagEnabled, optimizelyJSON, ruleKey, - key, + flagKey, user, reasonsToReport); } 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(); 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; - List allOptions = getAllOptions(options); + List allOptions = ignoreDefaultOptions ? options: getAllOptions(options); + + Map flagDecisions = new HashMap<>(); + Map decisionReasonsMap = new HashMap<>(); + + List flagsWithoutForcedDecision = new ArrayList<>(); + + List validKeys = new ArrayList<>(); for (String key : keys) { - OptimizelyDecision decision = decide(user, key, options); - if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.getEnabled()) { - decisionMap.put(key, decision); + FeatureFlag flag = projectConfig.getFeatureKeyMapping().get(key); + if (flag == null) { + decisionMap.put(key, OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.FLAG_KEY_INVALID.reason(key))); + continue; + } + + validKeys.add(key); + + DecisionReasons decisionReasons = DefaultDecisionReasons.newInstance(allOptions); + decisionReasonsMap.put(key, decisionReasons); + + 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); + } + } + + List> decisionList = + decisionService.getVariationsForFeatureList(flagsWithoutForcedDecision, user, projectConfig, allOptions); + + 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()); + } + + for (String key: validKeys) { + FeatureDecision flagDecision = flagDecisions.get(key); + 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); } } 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..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 @@ -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. * @@ -81,18 +81,24 @@ public DecisionService(@Nonnull Bucketer bucketer, /** * 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 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, @Nonnull ProjectConfig projectConfig, - @Nonnull List options) { - DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + @Nonnull List options, + @Nullable UserProfileTracker userProfileTracker, + @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 +122,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 (userProfileTracker != null) { + decisionVariation = getStoredVariation(experiment, userProfileTracker.getUserProfile(), 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 +142,8 @@ public DecisionResponse getVariation(@Nonnull Experiment experiment, variation = decisionVariation.getResult(); if (variation != null) { - if (userProfileService != null && !ignoreUPS) { - saveVariation(experiment, variation, userProfile); + if (userProfileTracker != null) { + userProfileTracker.updateUserProfile(experiment, variation); } else { logger.debug("This decision will not be saved since the UserProfileService is null."); } @@ -177,6 +157,39 @@ 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); + UserProfileTracker userProfileTracker = null; + + if (userProfileService != null && !ignoreUPS) { + 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(errorHandler); + } + return response; + } + @Nonnull public DecisionResponse getVariation(@Nonnull Experiment experiment, @Nonnull OptimizelyUserContext user, @@ -198,31 +211,70 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature @Nonnull OptimizelyUserContext user, @Nonnull ProjectConfig projectConfig, @Nonnull List options) { - DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + return getVariationsForFeatureList(Arrays.asList(featureFlag), user, projectConfig, options).get(0); + } + + /** + * 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 + * @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(); - DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options); - reasons.merge(decisionVariationResponse.getReasons()); + boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE); + UserProfileTracker userProfileTracker = null; - FeatureDecision decision = decisionVariationResponse.getResult(); - if (decision != null) { - return new DecisionResponse(decision, reasons); + if (userProfileService != null && !ignoreUPS) { + userProfileTracker = new UserProfileTracker(user.getUserId(), userProfileService, logger); + userProfileTracker.loadUserProfile(upsReasons, errorHandler); } - DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); - reasons.merge(decisionFeatureResponse.getReasons()); - decision = decisionFeatureResponse.getResult(); + List> decisions = new ArrayList<>(); - 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()); + for (FeatureFlag featureFlag: featureFlags) { + DecisionReasons reasons = DefaultDecisionReasons.newInstance(); + reasons.merge(upsReasons); + + DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker); + 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)); } - logger.info(message); - return new DecisionResponse(decision, reasons); + if (userProfileService != null && !ignoreUPS) { + userProfileTracker.saveUserProfile(errorHandler); + } + + return decisions; } @Nonnull @@ -244,13 +296,15 @@ public DecisionResponse getVariationForFeature(@Nonnull Feature DecisionResponse getVariationFromExperiment(@Nonnull ProjectConfig projectConfig, @Nonnull FeatureFlag featureFlag, @Nonnull OptimizelyUserContext user, - @Nonnull List options) { + @Nonnull List options, + @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); + DecisionResponse decisionVariation = + getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker); reasons.merge(decisionVariation.getReasons()); Variation variation = decisionVariation.getResult(); @@ -365,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. * @@ -615,11 +672,12 @@ 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, - @Nonnull List options) { + @Nonnull List options, + @Nullable UserProfileTracker userProfileTracker) { DecisionReasons reasons = DefaultDecisionReasons.newInstance(); String ruleKey = rule != null ? rule.getKey() : null; @@ -634,7 +692,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, userProfileTracker, null); reasons.merge(decisionResponse.getReasons()); variation = decisionResponse.getResult(); 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..2dee3d171 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/UserProfileTracker.java @@ -0,0 +1,109 @@ +/**************************************************************************** + * 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. * + * 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 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..34cf61543 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. @@ -20,10 +20,15 @@ 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; +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 +42,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 +353,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,8 +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), @@ -395,9 +404,84 @@ 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(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(2).getExperimentKey(), ""); + 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_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 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..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 @@ -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. * @@ -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( @@ -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( @@ -480,6 +491,33 @@ 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 ==========// /** @@ -743,27 +781,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"; @@ -961,9 +978,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())); }