diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 72254ce9..df85464e 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -22,7 +22,7 @@ from .helpers import experiment as experiment_helper from .helpers import validator from .optimizely_user_context import OptimizelyUserContext, UserAttributes -from .user_profile import UserProfile, UserProfileService +from .user_profile import UserProfile, UserProfileService, UserProfileTracker if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime @@ -35,7 +35,7 @@ class Decision(NamedTuple): None if no experiment/variation was selected.""" experiment: Optional[entities.Experiment] variation: Optional[entities.Variation] - source: str + source: Optional[str] class DecisionService: @@ -247,6 +247,8 @@ def get_variation( project_config: ProjectConfig, experiment: entities.Experiment, user_context: OptimizelyUserContext, + user_profile_tracker: Optional[UserProfileTracker], + reasons: list[str] = [], options: Optional[Sequence[str]] = None ) -> tuple[Optional[entities.Variation], list[str]]: """ Top-level function to help determine variation user should be put in. @@ -260,7 +262,9 @@ def get_variation( Args: project_config: Instance of ProjectConfig. experiment: Experiment for which user variation needs to be determined. - user_context: contains user id and attributes + user_context: contains user id and attributes. + user_profile_tracker: tracker for reading and updating user profile of the user. + reasons: Decision reasons. options: Decide options. Returns: @@ -275,6 +279,8 @@ def get_variation( ignore_user_profile = False decide_reasons = [] + if reasons is not None: + decide_reasons += reasons # Check if experiment is running if not experiment_helper.is_experiment_running(experiment): message = f'Experiment "{experiment.key}" is not running.' @@ -296,23 +302,14 @@ def get_variation( return variation, decide_reasons # Check to see if user has a decision available for the given experiment - user_profile = UserProfile(user_id) - if not ignore_user_profile and self.user_profile_service: - try: - retrieved_profile = self.user_profile_service.lookup(user_id) - except: - self.logger.exception(f'Unable to retrieve user profile for user "{user_id}" as lookup failed.') - retrieved_profile = None - - if retrieved_profile and validator.is_user_profile_valid(retrieved_profile): - user_profile = UserProfile(**retrieved_profile) - variation = self.get_stored_variation(project_config, experiment, user_profile) - if variation: - message = f'Returning previously activated variation ID "{variation}" of experiment ' \ - f'"{experiment}" for user "{user_id}" from user profile.' - self.logger.info(message) - decide_reasons.append(message) - return variation, decide_reasons + if user_profile_tracker is not None and not ignore_user_profile: + variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile()) + if variation: + message = f'Returning previously activated variation ID "{variation}" of experiment ' \ + f'"{experiment}" for user "{user_id}" from user profile.' + self.logger.info(message) + decide_reasons.append(message) + return variation, decide_reasons else: self.logger.warning('User profile has invalid format.') @@ -340,10 +337,9 @@ def get_variation( self.logger.info(message) decide_reasons.append(message) # Store this new decision and return the variation for the user - if not ignore_user_profile and self.user_profile_service: + if user_profile_tracker is not None and not ignore_user_profile: try: - user_profile.save_variation_for_experiment(experiment.id, variation.id) - self.user_profile_service.save(user_profile.__dict__) + user_profile_tracker.update_user_profile(experiment, variation) except: self.logger.exception(f'Unable to save user profile for user "{user_id}".') return variation, decide_reasons @@ -479,44 +475,7 @@ def get_variation_for_feature( Returns: Decision namedtuple consisting of experiment and variation for the user. """ - decide_reasons = [] - - # Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments - if feature.experimentIds: - # Evaluate each experiment ID and return the first bucketed experiment variation - for experiment_id in feature.experimentIds: - experiment = project_config.get_experiment_from_id(experiment_id) - decision_variation = None - - if experiment: - optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(feature.key, - experiment.key) - - forced_decision_variation, reasons_received = self.validated_forced_decision( - project_config, optimizely_decision_context, user_context) - decide_reasons += reasons_received - - if forced_decision_variation: - decision_variation = forced_decision_variation - else: - decision_variation, variation_reasons = self.get_variation(project_config, - experiment, user_context, options) - decide_reasons += variation_reasons - - if decision_variation: - message = f'User "{user_context.user_id}" bucketed into a ' \ - f'experiment "{experiment.key}" of feature "{feature.key}".' - self.logger.debug(message) - return Decision(experiment, decision_variation, - enums.DecisionSources.FEATURE_TEST), decide_reasons - - message = f'User "{user_context.user_id}" is not bucketed into any of the ' \ - f'experiments on the feature "{feature.key}".' - self.logger.debug(message) - variation, rollout_variation_reasons = self.get_variation_for_rollout(project_config, feature, user_context) - if rollout_variation_reasons: - decide_reasons += rollout_variation_reasons - return variation, decide_reasons + return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0] def validated_forced_decision( self, @@ -580,3 +539,91 @@ def validated_forced_decision( user_context.logger.info(user_has_forced_decision_but_invalid) return None, reasons + + def get_variations_for_feature_list( + self, + project_config: ProjectConfig, + features: list[entities.FeatureFlag], + user_context: OptimizelyUserContext, + options: Optional[Sequence[str]] = None + ) -> list[tuple[Decision, list[str]]]: + """ + Returns the list of experiment/variation the user is bucketed in for the given list of features. + Args: + project_config: Instance of ProjectConfig. + features: List of features for which we are determining if it is enabled or not for the given user. + user_context: user context for user. + options: Decide options. + + Returns: + List of Decision namedtuple consisting of experiment and variation for the user. + """ + decide_reasons: list[str] = [] + + if options: + ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options + else: + ignore_ups = False + + user_profile_tracker: Optional[UserProfileTracker] = None + if self.user_profile_service is not None and not ignore_ups: + user_profile_tracker = UserProfileTracker(user_context.user_id, self.user_profile_service, self.logger) + user_profile_tracker.load_user_profile(decide_reasons, None) + + decisions = [] + + for feature in features: + feature_reasons = decide_reasons.copy() + experiment_decision_found = False # Track if an experiment decision was made for the feature + + # Check if the feature flag is under an experiment + if feature.experimentIds: + for experiment_id in feature.experimentIds: + experiment = project_config.get_experiment_from_id(experiment_id) + decision_variation = None + + if experiment: + optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext( + feature.key, experiment.key) + forced_decision_variation, reasons_received = self.validated_forced_decision( + project_config, optimizely_decision_context, user_context) + feature_reasons.extend(reasons_received) + + if forced_decision_variation: + decision_variation = forced_decision_variation + else: + decision_variation, variation_reasons = self.get_variation( + project_config, experiment, user_context, user_profile_tracker, feature_reasons, options + ) + feature_reasons.extend(variation_reasons) + + if decision_variation: + self.logger.debug( + f'User "{user_context.user_id}" ' + f'bucketed into experiment "{experiment.key}" of feature "{feature.key}".' + ) + decision = Decision(experiment, decision_variation, enums.DecisionSources.FEATURE_TEST) + decisions.append((decision, feature_reasons)) + experiment_decision_found = True # Mark that a decision was found + break # Stop after the first successful experiment decision + + # Only process rollout if no experiment decision was found + if not experiment_decision_found: + rollout_decision, rollout_reasons = self.get_variation_for_rollout(project_config, + feature, + user_context) + if rollout_reasons: + feature_reasons.extend(rollout_reasons) + if rollout_decision: + self.logger.debug(f'User "{user_context.user_id}" ' + f'bucketed into rollout for feature "{feature.key}".') + else: + self.logger.debug(f'User "{user_context.user_id}" ' + f'not bucketed into any rollout for feature "{feature.key}".') + + decisions.append((rollout_decision, feature_reasons)) + + if self.user_profile_service is not None and user_profile_tracker is not None and ignore_ups is False: + user_profile_tracker.save_user_profile() + + return decisions diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index c50bfcb3..1b25bec6 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -21,6 +21,7 @@ from . import exceptions from . import logger as _logging from . import project_config +from . import user_profile from .config_manager import AuthDatafilePollingConfigManager from .config_manager import BaseConfigManager from .config_manager import PollingConfigManager @@ -42,6 +43,7 @@ from .odp.odp_manager import OdpManager from .optimizely_config import OptimizelyConfig, OptimizelyConfigService from .optimizely_user_context import OptimizelyUserContext, UserAttributes +from .project_config import ProjectConfig if TYPE_CHECKING: # prevent circular dependency by skipping import at runtime @@ -168,6 +170,7 @@ def __init__( self.event_builder = event_builder.EventBuilder() self.decision_service = decision_service.DecisionService(self.logger, user_profile_service) + self.user_profile_service = user_profile_service def _validate_instantiation_options(self) -> None: """ Helper method to validate all instantiation parameters. @@ -629,8 +632,13 @@ def get_variation( return None user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - - variation, _ = self.decision_service.get_variation(project_config, experiment, user_context) + user_profile_tracker = user_profile.UserProfileTracker(user_id, self.user_profile_service, self.logger) + user_profile_tracker.load_user_profile() + variation, _ = self.decision_service.get_variation(project_config, + experiment, + user_context, + user_profile_tracker) + user_profile_tracker.save_user_profile() if variation: variation_key = variation.key @@ -701,7 +709,7 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona if (is_source_rollout or not decision.variation) and project_config.get_send_flag_decisions_value(): self._send_impression_event( project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key if - decision.experiment else '', decision.source, feature_enabled, user_id, attributes + decision.experiment else '', str(decision.source), feature_enabled, user_id, attributes ) # Send event if Decision came from an experiment. @@ -712,7 +720,7 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona } self._send_impression_event( project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key, - decision.source, feature_enabled, user_id, attributes + str(decision.source), feature_enabled, user_id, attributes ) if feature_enabled: @@ -1118,73 +1126,70 @@ def _decide( self.logger.debug('Provided decide options is not an array. Using default decide options.') decide_options = self.default_decide_options - # Create Optimizely Decision Result. + if OptimizelyDecideOption.ENABLED_FLAGS_ONLY in decide_options: + decide_options.remove(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) + + decision = self._decide_for_keys( + user_context, + [key], + decide_options, + True + )[key] + + return decision + + def _create_optimizely_decision( + self, + user_context: OptimizelyUserContext, + flag_key: str, + flag_decision: Decision, + decision_reasons: Optional[list[str]], + decide_options: list[str], + project_config: ProjectConfig + ) -> OptimizelyDecision: user_id = user_context.user_id - attributes = user_context.get_user_attributes() - variation_key = None - variation = None feature_enabled = False - rule_key = None - flag_key = key + if flag_decision.variation is not None: + if flag_decision.variation.featureEnabled: + feature_enabled = True + + self.logger.info(f'Feature {flag_key} is enabled for user {user_id} {feature_enabled}"') + + # Create Optimizely Decision Result. + attributes = user_context.get_user_attributes() + rule_key = flag_decision.experiment.key if flag_decision.experiment else None all_variables = {} - experiment = None - decision_source = DecisionSources.ROLLOUT - source_info: dict[str, Any] = {} + decision_source = flag_decision.source decision_event_dispatched = False - # Check forced decisions first - optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(flag_key=key, rule_key=rule_key) - forced_decision_response = self.decision_service.validated_forced_decision(config, - optimizely_decision_context, - user_context) - variation, decision_reasons = forced_decision_response - reasons += decision_reasons - - if variation: - decision = Decision(None, variation, enums.DecisionSources.FEATURE_TEST) - else: - # Regular decision - decision, decision_reasons = self.decision_service.get_variation_for_feature(config, - feature_flag, - user_context, decide_options) - - reasons += decision_reasons - - # Fill in experiment and variation if returned (rollouts can have featureEnabled variables as well.) - if decision.experiment is not None: - experiment = decision.experiment - source_info["experiment"] = experiment - rule_key = experiment.key if experiment else None - if decision.variation is not None: - variation = decision.variation - variation_key = variation.key - feature_enabled = variation.featureEnabled - decision_source = decision.source - source_info["variation"] = variation + feature_flag = project_config.feature_key_map.get(flag_key) # Send impression event if Decision came from a feature # test and decide options doesn't include disableDecisionEvent if OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options: - if decision_source == DecisionSources.FEATURE_TEST or config.send_flag_decisions: - self._send_impression_event(config, experiment, variation, flag_key, rule_key or '', - decision_source, feature_enabled, + if decision_source == DecisionSources.FEATURE_TEST or project_config.send_flag_decisions: + self._send_impression_event(project_config, + flag_decision.experiment, + flag_decision.variation, + flag_key, rule_key or '', + str(decision_source), feature_enabled, user_id, attributes) decision_event_dispatched = True # Generate all variables map if decide options doesn't include excludeVariables - if OptimizelyDecideOption.EXCLUDE_VARIABLES not in decide_options: + if OptimizelyDecideOption.EXCLUDE_VARIABLES not in decide_options and feature_flag: for variable_key, variable in feature_flag.variables.items(): variable_value = variable.defaultValue if feature_enabled: - variable_value = config.get_variable_value_for_variation(variable, decision.variation) + variable_value = project_config.get_variable_value_for_variation(variable, flag_decision.variation) self.logger.debug( f'Got variable value "{variable_value}" for ' f'variable "{variable_key}" of feature flag "{flag_key}".' ) try: - actual_value = config.get_typecast_value(variable_value, variable.type) + actual_value = project_config.get_typecast_value(variable_value, variable.type) except: self.logger.error('Unable to cast value. Returning None.') actual_value = None @@ -1192,7 +1197,11 @@ def _decide( all_variables[variable_key] = actual_value should_include_reasons = OptimizelyDecideOption.INCLUDE_REASONS in decide_options - + variation_key = ( + flag_decision.variation.key + if flag_decision is not None and flag_decision.variation is not None + else None + ) # Send notification self.notification_center.send_notifications( enums.NotificationTypes.DECISION, @@ -1205,7 +1214,7 @@ def _decide( 'variables': all_variables, 'variation_key': variation_key, 'rule_key': rule_key, - 'reasons': reasons if should_include_reasons else [], + 'reasons': decision_reasons if should_include_reasons else [], 'decision_event_dispatched': decision_event_dispatched }, @@ -1213,7 +1222,7 @@ def _decide( return OptimizelyDecision(variation_key=variation_key, enabled=feature_enabled, variables=all_variables, rule_key=rule_key, flag_key=flag_key, - user_context=user_context, reasons=reasons if should_include_reasons else [] + user_context=user_context, reasons=decision_reasons if should_include_reasons else [] ) def _decide_all( @@ -1253,7 +1262,8 @@ def _decide_for_keys( self, user_context: Optional[OptimizelyUserContext], keys: list[str], - decide_options: Optional[list[str]] = None + decide_options: Optional[list[str]] = None, + ignore_default_options: bool = False ) -> dict[str, OptimizelyDecision]: """ Args: @@ -1277,19 +1287,74 @@ def _decide_for_keys( merged_decide_options: list[str] = [] if isinstance(decide_options, list): merged_decide_options = decide_options[:] - merged_decide_options += self.default_decide_options + if not ignore_default_options: + merged_decide_options += self.default_decide_options else: self.logger.debug('Provided decide options is not an array. Using default decide options.') merged_decide_options = self.default_decide_options - enabled_flags_only = OptimizelyDecideOption.ENABLED_FLAGS_ONLY in merged_decide_options + decisions: dict[str, OptimizelyDecision] = {} + valid_keys = [] + decision_reasons_dict = {} + + project_config = self.config_manager.get_config() + flags_without_forced_decision: list[entities.FeatureFlag] = [] + flag_decisions: dict[str, Decision] = {} - decisions = {} + if project_config is None: + return decisions for key in keys: - decision = self._decide(user_context, key, decide_options) - if enabled_flags_only and not decision.enabled: + feature_flag = project_config.feature_key_map.get(key) + if feature_flag is None: + decisions[key] = OptimizelyDecision(None, False, None, None, key, user_context, []) continue - decisions[key] = decision + valid_keys.append(key) + decision_reasons: list[str] = [] + decision_reasons_dict[key] = decision_reasons + + optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(flag_key=key, rule_key=None) + forced_decision_response = self.decision_service.validated_forced_decision(project_config, + optimizely_decision_context, + user_context) + variation, decision_reasons = forced_decision_response + decision_reasons_dict[key] += decision_reasons + + if variation: + decision = Decision(None, variation, enums.DecisionSources.FEATURE_TEST) + flag_decisions[key] = decision + else: + flags_without_forced_decision.append(feature_flag) + + decision_list = self.decision_service.get_variations_for_feature_list( + project_config, + flags_without_forced_decision, + user_context, + merged_decide_options + ) + + for i in range(0, len(flags_without_forced_decision)): + decision = decision_list[i][0] + reasons = decision_list[i][1] + flag_key = flags_without_forced_decision[i].key + flag_decisions[flag_key] = decision + decision_reasons_dict[flag_key] += reasons + + for key in valid_keys: + flag_decision = flag_decisions[key] + decision_reasons = decision_reasons_dict[key] + optimizely_decision = self._create_optimizely_decision( + user_context, + key, + flag_decision, + decision_reasons, + merged_decide_options, + project_config + ) + enabled_flags_only_missing = OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options + is_enabled = optimizely_decision.enabled + if enabled_flags_only_missing or is_enabled: + decisions[key] = optimizely_decision + return decisions def _setup_odp(self, sdk_key: Optional[str]) -> None: diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 0410bcf7..f5ded013 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -14,11 +14,17 @@ from __future__ import annotations from typing import Any, Optional from sys import version_info +from . import logger as _logging if version_info < (3, 8): from typing_extensions import Final else: - from typing import Final # type: ignore + from typing import Final, TYPE_CHECKING # type: ignore + + if TYPE_CHECKING: + # prevent circular dependenacy by skipping import at runtime + from .entities import Experiment, Variation + from optimizely.error_handler import BaseErrorHandler class UserProfile: @@ -54,7 +60,6 @@ def get_variation_for_experiment(self, experiment_id: str) -> Optional[str]: Returns: Variation ID corresponding to the experiment. None if no decision available. """ - return self.experiment_bucket_map.get(experiment_id, {self.VARIATION_ID_KEY: None}).get(self.VARIATION_ID_KEY) def save_variation_for_experiment(self, experiment_id: str, variation_id: str) -> None: @@ -64,7 +69,6 @@ def save_variation_for_experiment(self, experiment_id: str, variation_id: str) - experiment_id: ID for experiment for which the decision is to be stored. variation_id: ID for variation that the user saw. """ - self.experiment_bucket_map.update({experiment_id: {self.VARIATION_ID_KEY: variation_id}}) @@ -90,3 +94,64 @@ def save(self, user_profile: dict[str, Any]) -> None: user_profile: Dict representing the user's profile. """ pass + + +class UserProfileTracker: + def __init__(self, + user_id: str, + user_profile_service: Optional[UserProfileService], + logger: Optional[_logging.Logger] = None): + self.user_id = user_id + self.user_profile_service = user_profile_service + self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) + self.profile_updated = False + self.user_profile = UserProfile(user_id, {}) + + def get_user_profile(self) -> UserProfile: + return self.user_profile + + def load_user_profile(self, reasons: Optional[list[str]] = [], + error_handler: Optional[BaseErrorHandler] = None) -> None: + if reasons is None: + reasons = [] + try: + user_profile = self.user_profile_service.lookup(self.user_id) if self.user_profile_service else None + if user_profile is None: + message = "Unable to get a user profile from the UserProfileService." + reasons.append(message) + else: + if 'user_id' in user_profile and 'experiment_bucket_map' in user_profile: + self.user_profile = UserProfile( + user_profile['user_id'], + user_profile['experiment_bucket_map'] + ) + self.logger.info("User profile loaded successfully.") + else: + missing_keys = [key for key in ['user_id', 'experiment_bucket_map'] if key not in user_profile] + message = f"User profile is missing keys: {', '.join(missing_keys)}" + reasons.append(message) + except Exception as exception: + message = str(exception) + reasons.append(message) + self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}" as lookup failed.') + if error_handler: + error_handler.handle_error(exception) + + def update_user_profile(self, experiment: Experiment, variation: Variation) -> None: + variation_id = variation.id + experiment_id = experiment.id + self.user_profile.save_variation_for_experiment(experiment_id, variation_id) + self.profile_updated = True + + def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> None: + if not self.profile_updated: + return + try: + if self.user_profile_service: + self.user_profile_service.save(self.user_profile.__dict__) + self.logger.info(f'Saved user profile of user "{self.user_profile.user_id}".') + except Exception as exception: + self.logger.warning(f'Failed to save user profile of user "{self.user_profile.user_id}" ' + f'for exception:{exception}".') + if error_handler: + error_handler.handle_error(exception) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 4d755de5..6c5862a5 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -485,6 +485,8 @@ def test_get_variation__bucketing_id_provided(self): "random_key": "random_value", "$opt_bucketing_id": "user_bucket_value", }) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch( "optimizely.decision_service.DecisionService.get_forced_variation", @@ -501,7 +503,8 @@ def test_get_variation__bucketing_id_provided(self): variation, _ = self.decision_service.get_variation( self.project_config, experiment, - user + user, + user_profile_tracker ) # Assert that bucket is called with appropriate bucketing ID @@ -515,6 +518,8 @@ def test_get_variation__user_whitelisted_for_variation(self): user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, logger=None, user_id="test_user", user_attributes={}) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch( "optimizely.decision_service.DecisionService.get_whitelisted_variation", @@ -531,7 +536,7 @@ def test_get_variation__user_whitelisted_for_variation(self): "optimizely.user_profile.UserProfileService.save" ) as mock_save: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user + self.project_config, experiment, user, user_profile_tracker ) self.assertEqual( entities.Variation("111128", "control"), @@ -554,6 +559,8 @@ def test_get_variation__user_has_stored_decision(self): user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, logger=None, user_id="test_user", user_attributes={}) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch( "optimizely.decision_service.DecisionService.get_whitelisted_variation", @@ -565,49 +572,38 @@ def test_get_variation__user_has_stored_decision(self): "optimizely.helpers.audience.does_user_meet_audience_conditions" ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", - return_value={ - "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111128"}}, - }, - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save" - ) as mock_save: + ) as mock_bucket: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None + self.project_config, experiment, user, user_profile_tracker ) self.assertEqual( entities.Variation("111128", "control"), variation, ) - # Assert that stored variation is returned and bucketing service is not involved mock_get_whitelisted_variation.assert_called_once_with( self.project_config, experiment, "test_user" ) - mock_lookup.assert_called_once_with("test_user") mock_get_stored_variation.assert_called_once_with( self.project_config, experiment, - user_profile.UserProfile( - "test_user", {"111127": {"variation_id": "111128"}} - ), + user_profile_tracker.user_profile ) self.assertEqual(0, mock_audience_check.call_count) self.assertEqual(0, mock_bucket.call_count) - self.assertEqual(0, mock_save.call_count) - def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_available( + def test_get_variation__user_bucketed_for_new_experiment__user_profile_tracker_available( self, ): """ Test that get_variation buckets and returns variation if no forced variation or decision available. - Also, stores decision if user profile service is available. """ + """ user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, logger=None, user_id="test_user", user_attributes={}) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch.object( self.decision_service, "logger" @@ -622,14 +618,9 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket", return_value=[entities.Variation("111129", "variation"), []], - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", - return_value={"user_id": "test_user", "experiment_bucket_map": {}}, - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save" - ) as mock_save: + ) as mock_bucket: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None + self.project_config, experiment, user, user_profile_tracker ) self.assertEqual( entities.Variation("111129", "variation"), @@ -640,71 +631,8 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a mock_get_whitelisted_variation.assert_called_once_with( self.project_config, experiment, user.user_id ) - mock_lookup.assert_called_once_with("test_user") - self.assertEqual(1, mock_get_stored_variation.call_count) - mock_audience_check.assert_called_once_with( - self.project_config, - experiment.get_audience_conditions_or_ids(), - enums.ExperimentAudienceEvaluationLogs, - "test_experiment", - user, - mock_decision_service_logging - ) - mock_bucket.assert_called_once_with( - self.project_config, experiment, "test_user", "test_user" - ) - mock_save.assert_called_once_with( - { - "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111129"}}, - } - ) - - def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_not_available( - self, - ): - """ Test that get_variation buckets and returns variation if - no forced variation and no user profile service available. """ - - # Unset user profile service - self.decision_service.user_profile_service = None - - user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, - logger=None, - user_id="test_user", - user_attributes={}) - experiment = self.project_config.get_experiment_from_key("test_experiment") - with mock.patch.object( - self.decision_service, "logger" - ) as mock_decision_service_logging, mock.patch( - "optimizely.decision_service.DecisionService.get_whitelisted_variation", - return_value=[None, []], - ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" - ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] - ) as mock_audience_check, mock.patch( - "optimizely.bucketer.Bucketer.bucket", - return_value=[entities.Variation("111129", "variation"), []], - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup" - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save" - ) as mock_save: - variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None - ) - self.assertEqual( - entities.Variation("111129", "variation"), - variation, - ) - # Assert that user is bucketed and new decision is not stored as user profile service is not available - mock_get_whitelisted_variation.assert_called_once_with( - self.project_config, experiment, "test_user" - ) - self.assertEqual(0, mock_lookup.call_count) - self.assertEqual(0, mock_get_stored_variation.call_count) + self.assertEqual(1, mock_get_stored_variation.call_count) mock_audience_check.assert_called_once_with( self.project_config, experiment.get_audience_conditions_or_ids(), @@ -716,7 +644,6 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_n mock_bucket.assert_called_once_with( self.project_config, experiment, "test_user", "test_user" ) - self.assertEqual(0, mock_save.call_count) def test_get_variation__user_does_not_meet_audience_conditions(self): """ Test that get_variation returns None if user is not in experiment. """ @@ -725,6 +652,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): logger=None, user_id="test_user", user_attributes={}) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, self.decision_service.user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch.object( self.decision_service, "logger" @@ -739,13 +667,10 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", - return_value={"user_id": "test_user", "experiment_bucket_map": {}}, - ) as mock_lookup, mock.patch( "optimizely.user_profile.UserProfileService.save" ) as mock_save: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None + self.project_config, experiment, user, user_profile_tracker ) self.assertIsNone( variation @@ -755,9 +680,8 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): mock_get_whitelisted_variation.assert_called_once_with( self.project_config, experiment, "test_user" ) - mock_lookup.assert_called_once_with("test_user") mock_get_stored_variation.assert_called_once_with( - self.project_config, experiment, user_profile.UserProfile("test_user") + self.project_config, experiment, user_profile_tracker.get_user_profile() ) mock_audience_check.assert_called_once_with( self.project_config, @@ -770,192 +694,6 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): self.assertEqual(0, mock_bucket.call_count) self.assertEqual(0, mock_save.call_count) - def test_get_variation__user_profile_in_invalid_format(self): - """ Test that get_variation handles invalid user profile gracefully. """ - - user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, - logger=None, - user_id="test_user", - user_attributes={}) - experiment = self.project_config.get_experiment_from_key("test_experiment") - with mock.patch.object( - self.decision_service, "logger" - ) as mock_decision_service_logging, mock.patch( - "optimizely.decision_service.DecisionService.get_whitelisted_variation", - return_value=[None, []], - ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" - ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] - ) as mock_audience_check, mock.patch( - "optimizely.bucketer.Bucketer.bucket", - return_value=[entities.Variation("111129", "variation"), []], - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", - return_value="invalid_profile", - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save" - ) as mock_save: - variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None - ) - self.assertEqual( - entities.Variation("111129", "variation"), - variation, - ) - - # Assert that user is bucketed and new decision is stored - mock_get_whitelisted_variation.assert_called_once_with( - self.project_config, experiment, "test_user" - ) - mock_lookup.assert_called_once_with("test_user") - # Stored decision is not consulted as user profile is invalid - self.assertEqual(0, mock_get_stored_variation.call_count) - mock_audience_check.assert_called_once_with( - self.project_config, - experiment.get_audience_conditions_or_ids(), - enums.ExperimentAudienceEvaluationLogs, - "test_experiment", - user, - mock_decision_service_logging - ) - mock_decision_service_logging.warning.assert_called_once_with( - "User profile has invalid format." - ) - mock_bucket.assert_called_once_with( - self.project_config, experiment, "test_user", "test_user" - ) - mock_save.assert_called_once_with( - { - "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111129"}}, - } - ) - - def test_get_variation__user_profile_lookup_fails(self): - """ Test that get_variation acts gracefully when lookup fails. """ - - user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, - logger=None, - user_id="test_user", - user_attributes={}) - experiment = self.project_config.get_experiment_from_key("test_experiment") - with mock.patch.object( - self.decision_service, "logger" - ) as mock_decision_service_logging, mock.patch( - "optimizely.decision_service.DecisionService.get_whitelisted_variation", - return_value=[None, []], - ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" - ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] - ) as mock_audience_check, mock.patch( - "optimizely.bucketer.Bucketer.bucket", - return_value=[entities.Variation("111129", "variation"), []], - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", - side_effect=Exception("major problem"), - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save" - ) as mock_save: - variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None - ) - self.assertEqual( - entities.Variation("111129", "variation"), - variation, - ) - - # Assert that user is bucketed and new decision is stored - mock_get_whitelisted_variation.assert_called_once_with( - self.project_config, experiment, "test_user" - ) - mock_lookup.assert_called_once_with("test_user") - # Stored decision is not consulted as lookup failed - self.assertEqual(0, mock_get_stored_variation.call_count) - mock_audience_check.assert_called_once_with( - self.project_config, - experiment.get_audience_conditions_or_ids(), - enums.ExperimentAudienceEvaluationLogs, - "test_experiment", - user, - mock_decision_service_logging - ) - mock_decision_service_logging.exception.assert_called_once_with( - 'Unable to retrieve user profile for user "test_user" as lookup failed.' - ) - mock_bucket.assert_called_once_with( - self.project_config, experiment, "test_user", "test_user" - ) - mock_save.assert_called_once_with( - { - "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111129"}}, - } - ) - - def test_get_variation__user_profile_save_fails(self): - """ Test that get_variation acts gracefully when save fails. """ - - user = optimizely_user_context.OptimizelyUserContext(optimizely_client=None, - logger=None, - user_id="test_user", - user_attributes={}) - experiment = self.project_config.get_experiment_from_key("test_experiment") - with mock.patch.object( - self.decision_service, "logger" - ) as mock_decision_service_logging, mock.patch( - "optimizely.decision_service.DecisionService.get_whitelisted_variation", - return_value=[None, []], - ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" - ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] - ) as mock_audience_check, mock.patch( - "optimizely.bucketer.Bucketer.bucket", - return_value=[entities.Variation("111129", "variation"), []], - ) as mock_bucket, mock.patch( - "optimizely.user_profile.UserProfileService.lookup", return_value=None - ) as mock_lookup, mock.patch( - "optimizely.user_profile.UserProfileService.save", - side_effect=Exception("major problem"), - ) as mock_save: - variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, None - ) - self.assertEqual( - entities.Variation("111129", "variation"), - variation, - ) - - # Assert that user is bucketed and new decision is stored - mock_get_whitelisted_variation.assert_called_once_with( - self.project_config, experiment, "test_user" - ) - mock_lookup.assert_called_once_with("test_user") - self.assertEqual(0, mock_get_stored_variation.call_count) - mock_audience_check.assert_called_once_with( - self.project_config, - experiment.get_audience_conditions_or_ids(), - enums.ExperimentAudienceEvaluationLogs, - "test_experiment", - user, - mock_decision_service_logging - ) - - mock_decision_service_logging.exception.assert_called_once_with( - 'Unable to save user profile for user "test_user".' - ) - mock_bucket.assert_called_once_with( - self.project_config, experiment, "test_user", "test_user" - ) - mock_save.assert_called_once_with( - { - "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111129"}}, - } - ) - def test_get_variation__ignore_user_profile_when_specified(self): """ Test that we ignore the user profile service if specified. """ @@ -963,6 +701,8 @@ def test_get_variation__ignore_user_profile_when_specified(self): logger=None, user_id="test_user", user_attributes={}) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) experiment = self.project_config.get_experiment_from_key("test_experiment") with mock.patch.object( self.decision_service, "logger" @@ -983,6 +723,8 @@ def test_get_variation__ignore_user_profile_when_specified(self): self.project_config, experiment, user, + user_profile_tracker, + [], options=['IGNORE_USER_PROFILE_SERVICE'], ) self.assertEqual( @@ -1290,6 +1032,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment( self.project_config, self.project_config.get_experiment_from_key("test_experiment"), user, + None, + [], None ) @@ -1417,6 +1161,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self) self.project_config, self.project_config.get_experiment_from_key("group_exp_1"), user, + None, + [], None ) @@ -1445,6 +1191,8 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self self.project_config, self.project_config.get_experiment_from_key("test_experiment"), user, + None, + [], None ) @@ -1472,7 +1220,7 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no ) mock_decision.assert_called_once_with( - self.project_config, self.project_config.get_experiment_from_id("32222"), user, False + self.project_config, self.project_config.get_experiment_from_id("32222"), user, None, [], False ) def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group_bucket_less_than_2500( @@ -1560,6 +1308,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_mutex_group with mock.patch( 'optimizely.bucketer.Bucketer._generate_bucket_value', return_value=6500) as mock_generate_bucket_value, \ mock.patch.object(self.project_config, 'logger') as mock_config_logging: + variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, user ) @@ -1789,6 +1538,13 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 variation_received, _ = self.decision_service.get_variation_for_feature( self.project_config, feature, user ) + print(f"variation received is: {variation_received}") + x = decision_service.Decision( + expected_experiment, + expected_variation, + enums.DecisionSources.ROLLOUT, + ) + print(f"need to be:{x}") self.assertEqual( decision_service.Decision( expected_experiment, @@ -1797,6 +1553,7 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 ), variation_received, ) + mock_config_logging.debug.assert_called_with( 'Assigned bucket 4000 to user with bucketing ID "test_user".') mock_generate_bucket_value.assert_called_with("test_user211147") diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f1d1db89..8d36b830 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -369,9 +369,11 @@ def test_activate(self): log_event = EventFactory.create_log_event(mock_process.call_args[0][0], self.optimizely.logger) user_context = mock_decision.call_args[0][2] + user_profile_tracker = mock_decision.call_args[0][3] mock_decision.assert_called_once_with( - self.project_config, self.project_config.get_experiment_from_key('test_experiment'), user_context + self.project_config, self.project_config.get_experiment_from_key('test_experiment'), + user_context, user_profile_tracker ) self.assertEqual(1, mock_process.call_count) @@ -766,11 +768,13 @@ def test_activate__with_attributes__audience_match(self): log_event = EventFactory.create_log_event(mock_process.call_args[0][0], self.optimizely.logger) user_context = mock_get_variation.call_args[0][2] + user_profile_tracker = mock_get_variation.call_args[0][3] mock_get_variation.assert_called_once_with( self.project_config, self.project_config.get_experiment_from_key('test_experiment'), - user_context + user_context, + user_profile_tracker ) self.assertEqual(1, mock_process.call_count) self._validate_event_object( @@ -1120,11 +1124,12 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): log_event = EventFactory.create_log_event(mock_process.call_args[0][0], self.optimizely.logger) user_context = mock_get_variation.call_args[0][2] - + user_profile_tracker = mock_get_variation.call_args[0][3] mock_get_variation.assert_called_once_with( self.project_config, self.project_config.get_experiment_from_key('test_experiment'), - user_context + user_context, + user_profile_tracker ) self.assertEqual(1, mock_process.call_count) self._validate_event_object( @@ -1814,6 +1819,35 @@ def test_get_variation(self): {'experiment_key': 'test_experiment', 'variation_key': variation}, ) + def test_get_variation_lookup_and_save_is_called(self): + """ Test that lookup is called, get_variation returns valid variation and then save is called""" + + with mock.patch( + 'optimizely.decision_service.DecisionService.get_variation', + return_value=(self.project_config.get_variation_from_id('test_experiment', '111129'), []), + ), mock.patch( + 'optimizely.notification_center.NotificationCenter.send_notifications' + ) as mock_broadcast, mock.patch( + 'optimizely.user_profile.UserProfileTracker.load_user_profile' + ) as mock_load_user_profile, mock.patch( + 'optimizely.user_profile.UserProfileTracker.save_user_profile' + ) as mock_save_user_profile: + variation = self.optimizely.get_variation('test_experiment', 'test_user') + self.assertEqual( + 'variation', variation, + ) + self.assertEqual(mock_load_user_profile.call_count, 1) + self.assertEqual(mock_save_user_profile.call_count, 1) + self.assertEqual(mock_broadcast.call_count, 1) + + mock_broadcast.assert_any_call( + enums.NotificationTypes.DECISION, + 'ab-test', + 'test_user', + {}, + {'experiment_key': 'test_experiment', 'variation_key': variation}, + ) + def test_get_variation_with_experiment_in_feature(self): """ Test that get_variation returns valid variation and broadcasts decision listener with type feature-test when get_variation returns feature experiment variation.""" diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 48f08885..0c35e230 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -228,9 +228,17 @@ def test_decide__feature_test(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -303,9 +311,17 @@ def test_decide__feature_test__send_flag_decision_false(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -478,9 +494,17 @@ def test_decide_feature_null_variation(self): mock_variation = None with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.ROLLOUT), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.ROLLOUT + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -553,9 +577,17 @@ def test_decide_feature_null_variation__send_flag_decision_false(self): mock_variation = None with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.ROLLOUT), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.ROLLOUT + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -614,9 +646,17 @@ def test_decide__option__disable_decision_event(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -678,9 +718,17 @@ def test_decide__default_option__disable_decision_event(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -739,9 +787,17 @@ def test_decide__option__exclude_variables(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -835,9 +891,17 @@ def test_decide__option__enabled_flags_only(self): expected_var = project_config.get_variation_from_key('211127', '211229') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(expected_experiment, expected_var, - enums.DecisionSources.ROLLOUT), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + expected_experiment, + expected_var, + enums.DecisionSources.ROLLOUT + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -914,9 +978,17 @@ def test_decide__default_options__with__options(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ) + ] ), mock.patch( 'optimizely.notification_center.NotificationCenter.send_notifications' ) as mock_broadcast_decision, mock.patch( @@ -968,14 +1040,17 @@ def test_decide_for_keys(self): mocked_decision_2 = OptimizelyDecision(flag_key='test_feature_in_rollout', enabled=False) def side_effect(*args, **kwargs): - flag = args[1] - if flag == 'test_feature_in_experiment': - return mocked_decision_1 - else: - return mocked_decision_2 + flags = args[1] + res = {} + for flag in flags: + if flag == 'test_feature_in_experiment': + res[flag] = mocked_decision_1 + else: + res[flag] = mocked_decision_2 + return res with mock.patch( - 'optimizely.optimizely.Optimizely._decide', side_effect=side_effect + 'optimizely.optimizely.Optimizely._decide_for_keys', side_effect=side_effect ) as mock_decide, mock.patch( 'optimizely.optimizely_user_context.OptimizelyUserContext._clone', return_value=user_context @@ -984,18 +1059,10 @@ def side_effect(*args, **kwargs): flags = ['test_feature_in_rollout', 'test_feature_in_experiment'] options = [] decisions = user_context.decide_for_keys(flags, options) - self.assertEqual(2, len(decisions)) - - mock_decide.assert_any_call( - user_context, - 'test_feature_in_experiment', - options - ) - mock_decide.assert_any_call( user_context, - 'test_feature_in_rollout', + ['test_feature_in_rollout', 'test_feature_in_experiment'], options ) @@ -1011,14 +1078,17 @@ def test_decide_for_keys__option__enabled_flags_only(self): mocked_decision_2 = OptimizelyDecision(flag_key='test_feature_in_rollout', enabled=False) def side_effect(*args, **kwargs): - flag = args[1] - if flag == 'test_feature_in_experiment': - return mocked_decision_1 - else: - return mocked_decision_2 + flags = args[1] + res = {} + for flag in flags: + if flag == 'test_feature_in_experiment': + res[flag] = mocked_decision_1 + else: + res[flag] = mocked_decision_2 + return res with mock.patch( - 'optimizely.optimizely.Optimizely._decide', side_effect=side_effect + 'optimizely.optimizely.Optimizely._decide_for_keys', side_effect=side_effect ) as mock_decide, mock.patch( 'optimizely.optimizely_user_context.OptimizelyUserContext._clone', return_value=user_context @@ -1028,20 +1098,13 @@ def side_effect(*args, **kwargs): options = ['ENABLED_FLAGS_ONLY'] decisions = user_context.decide_for_keys(flags, options) - self.assertEqual(1, len(decisions)) - - mock_decide.assert_any_call( - user_context, - 'test_feature_in_experiment', - options - ) + self.assertEqual(2, len(decisions)) mock_decide.assert_any_call( user_context, - 'test_feature_in_rollout', + ['test_feature_in_rollout', 'test_feature_in_experiment'], options ) - self.assertEqual(mocked_decision_1, decisions['test_feature_in_experiment']) def test_decide_for_keys__default_options__with__options(self): @@ -1053,20 +1116,29 @@ def test_decide_for_keys__default_options__with__options(self): user_context = opt_obj.create_user_context('test_user') with mock.patch( - 'optimizely.optimizely.Optimizely._decide' - ) as mock_decide, mock.patch( + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list' + ) as mock_get_variations, mock.patch( 'optimizely.optimizely_user_context.OptimizelyUserContext._clone', return_value=user_context ): flags = ['test_feature_in_experiment'] options = ['EXCLUDE_VARIABLES'] + + mock_decision = mock.MagicMock() + mock_decision.experiment = mock.MagicMock(key='test_experiment') + mock_decision.variation = mock.MagicMock(key='variation') + mock_decision.source = enums.DecisionSources.FEATURE_TEST + + mock_get_variations.return_value = [(mock_decision, [])] + user_context.decide_for_keys(flags, options) - mock_decide.assert_called_with( - user_context, - 'test_feature_in_experiment', - ['EXCLUDE_VARIABLES'] + mock_get_variations.assert_called_with( + mock.ANY, # ProjectConfig + mock.ANY, # FeatureFlag list + user_context, # UserContext object + ['EXCLUDE_VARIABLES', 'ENABLED_FLAGS_ONLY'] ) def test_decide_for_all(self): @@ -1323,9 +1395,17 @@ def test_decide_experiment(self): mock_experiment = project_config.get_experiment_from_key('test_experiment') mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( - 'optimizely.decision_service.DecisionService.get_variation_for_feature', - return_value=(decision_service.Decision(mock_experiment, - mock_variation, enums.DecisionSources.FEATURE_TEST), []), + 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', + return_value=[ + ( + decision_service.Decision( + mock_experiment, + mock_variation, + enums.DecisionSources.FEATURE_TEST + ), + [] + ), + ] ): user_context = opt_obj.create_user_context('test_user') decision = user_context.decide('test_feature_in_experiment', [DecideOption.DISABLE_DECISION_EVENT]) @@ -1631,6 +1711,8 @@ def test_should_return_valid_decision_after_setting_invalid_experiment_rule_vari self.assertEqual(decide_decision.user_context.get_user_attributes(), {}) expected_reasons = [ + 'Invalid variation is mapped to flag (test_feature_in_experiment), rule (test_experiment) ' + 'and user (test_user) in the forced decision map.', 'Invalid variation is mapped to flag (test_feature_in_experiment), rule (test_experiment) ' 'and user (test_user) in the forced decision map.', 'Evaluating audiences for experiment "test_experiment": [].', diff --git a/tests/test_user_profile.py b/tests/test_user_profile.py index ffeb3e34..84aacd05 100644 --- a/tests/test_user_profile.py +++ b/tests/test_user_profile.py @@ -14,6 +14,7 @@ import unittest from optimizely import user_profile +from unittest import mock class UserProfileTest(unittest.TestCase): @@ -63,3 +64,76 @@ def test_save(self): user_profile_service = user_profile.UserProfileService() self.assertIsNone(user_profile_service.save({'user_id': 'test_user', 'experiment_bucket_map': {}})) + + +class UserProfileTrackerTest(unittest.TestCase): + def test_load_user_profile_failure(self): + """Test that load_user_profile handles exceptions gracefully.""" + mock_user_profile_service = mock.MagicMock() + mock_logger = mock.MagicMock() + + user_profile_tracker = user_profile.UserProfileTracker( + user_id="test_user", + user_profile_service=mock_user_profile_service, + logger=mock_logger + ) + mock_user_profile_service.lookup.side_effect = Exception("Lookup failure") + + user_profile_tracker.load_user_profile() + + # Verify that the logger recorded the exception + mock_logger.exception.assert_called_once_with( + 'Unable to retrieve user profile for user "test_user" as lookup failed.' + ) + + # Verify that the user profile is reset to an empty profile + self.assertEqual(user_profile_tracker.user_profile.user_id, "test_user") + self.assertEqual(user_profile_tracker.user_profile.experiment_bucket_map, {}) + + def test_load_user_profile__user_profile_invalid(self): + """Test that load_user_profile handles an invalid user profile format.""" + mock_user_profile_service = mock.MagicMock() + mock_logger = mock.MagicMock() + + user_profile_tracker = user_profile.UserProfileTracker( + user_id="test_user", + user_profile_service=mock_user_profile_service, + logger=mock_logger + ) + + mock_user_profile_service.lookup.return_value = {"invalid_key": "value"} + + reasons = [] + user_profile_tracker.load_user_profile(reasons=reasons) + + # Verify that the logger recorded a warning for the missing keys + missing_keys_message = "User profile is missing keys: user_id, experiment_bucket_map" + self.assertIn(missing_keys_message, reasons) + + # Ensure the logger logs the invalid format + mock_logger.info.assert_not_called() + self.assertEqual(user_profile_tracker.user_profile.user_id, "test_user") + self.assertEqual(user_profile_tracker.user_profile.experiment_bucket_map, {}) + + # Verify the reasons list was updated + self.assertIn(missing_keys_message, reasons) + + def test_save_user_profile_failure(self): + """Test that save_user_profile handles exceptions gracefully.""" + mock_user_profile_service = mock.MagicMock() + mock_logger = mock.MagicMock() + + user_profile_tracker = user_profile.UserProfileTracker( + user_id="test_user", + user_profile_service=mock_user_profile_service, + logger=mock_logger + ) + + user_profile_tracker.profile_updated = True + mock_user_profile_service.save.side_effect = Exception("Save failure") + + user_profile_tracker.save_user_profile() + + mock_logger.warning.assert_called_once_with( + 'Failed to save user profile of user "test_user" for exception:Save failure".' + )