From 0411458e56523b7058fbaaaa877af3662866bc47 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 4 Nov 2024 22:45:57 +0600 Subject: [PATCH 01/29] update: UserProfile class created, changes in decision_service, decide_for_keys --- optimizely/decision_service.py | 8 +++- optimizely/optimizely.py | 67 +++++++++++++++++++++++++++++++++- optimizely/user_profile.py | 59 ++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 4 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 72254ce9..3e671245 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 @@ -247,6 +247,8 @@ def get_variation( project_config: ProjectConfig, experiment: entities.Experiment, user_context: OptimizelyUserContext, + user_profile_tracker: 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: diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index c50bfcb3..5b945095 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -42,6 +42,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 @@ -1215,6 +1216,27 @@ def _decide( rule_key=rule_key, flag_key=flag_key, user_context=user_context, reasons=reasons if should_include_reasons else [] ) + + def _create_optimizely_decision( + self, + user_context: Optional[OptimizelyUserContext], + flag_key: str, + flag_decision: Optional[Decision], + decision_reasons: Optional[list[str]], + decide_options: Optional[list[OptimizelyDecideOption]], + project_config: ProjectConfig + ) -> OptimizelyDecision: + user_id = user_context.user_id + flag_enabled = False + if flag_decision.variation is not None: + if flag_decision.variation.featureEnabled: + flag_enabled = True + + self.logger.info(f'Feature {flag_key} is enabled for user {user_id} {flag_enabled}"') + variable_dict = {} + # if OptimizelyDecideOption.EXCLUDE_VARIABLES not in decide_options: + # decision_variables = self. + return def _decide_all( self, @@ -1253,7 +1275,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,7 +1300,8 @@ 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 @@ -1285,11 +1309,50 @@ def _decide_for_keys( enabled_flags_only = OptimizelyDecideOption.ENABLED_FLAGS_ONLY in merged_decide_options decisions = {} + valid_keys = [] + reasons = [] + decision_reasons_dict = {} for key in keys: decision = self._decide(user_context, key, decide_options) if enabled_flags_only and not decision.enabled: continue decisions[key] = decision + + project_config = self.config_manager.get_config() + for key in keys: + 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 + valid_keys.append(key) + # decision_reasons = [] + # 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 + reasons += decision_reasons + + flags_without_forced_decision = [] + flag_decisions = {} + + if variation: + decision = Decision(None, variation, enums.DecisionSources.FEATURE_TEST) + flag_decisions[key] = decision + else: + # Regular decision + # decision, decision_reasons = self.decision_service.get_variation_for_feature(project_config, + # feature_flag, + # user_context, decide_options) + flags_without_forced_decision.append(feature_flag) + + #needs to be implemented + decisionList = self.decision_service.get_variation_for_feature_list(flags_without_forced_decision, user_context, project_config, merged_decide_options) + + + 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..3a311c49 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -14,6 +14,10 @@ from __future__ import annotations from typing import Any, Optional from sys import version_info +from .helpers.validator import is_user_profile_valid +from .entities import Experiment, Variation +from .decision_service import Decision +from optimizely.error_handler import BaseErrorHandler if version_info < (3, 8): from typing_extensions import Final @@ -90,3 +94,58 @@ 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: UserProfileService, logger=None): + self.user_id = user_id + self.user_profile_service = user_profile_service + # self.logger = logger or logging.getLogger(__name__) + self.profile_updated = False + self.user_profile = None + + def get_user_profile(self): + return self.user_profile + + def load_user_profile(self, reasons: Optional[list[str]], error_handler: Optional[BaseErrorHandler]): + try: + user_profile = self.user_profile_service.lookup(self.user_id) + if user_profile is None: + message = reasons.append("Unable to get a user profile from the UserProfileService.") + self.logger.info(message) + elif is_user_profile_valid(user_profile): + self.user_profile = user_profile + else: + message = reasons.append("The UserProfileService returned an invalid user_profile.") + self.logger.warning(message) + except Exception as exception: + message = reasons.append(str(exception)) + self.logger.error(message) + # Todo: add error handler + # error_handler.handle_error() + + if self.user_profile is None: + self.user_profile = UserProfile(self.user_id, {}) + + def update_user_profile(self, experiment: Experiment, variation: Variation): + decision:Decision = None + if experiment.id in self.user_profile.experiment_bucket_map: + decision = self.user_profile.experiment_bucket_map[experiment.id] + decision.variation = variation + else: + decision = Decision(experiment=None, variation=variation, source=None) + + self.user_profile.experiment_bucket_map[experiment.id] = decision + self.profile_updated = True + # self.logger.info(f'Updated variation "{variation.id}" of experiment "{experiment.id}" for user "{self.user_profile.user_id}".') + + + def save_user_profile(self, error_handler: Optional[BaseErrorHandler]): + if not self.profile_updated: + return + + try: + self.user_profile_service.save(self.user_profile) + 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}".') + # error_handler.handle_error(exception) From 43a0c74b23cff248f6d99f5932f4df74263bffc2 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 4 Nov 2024 23:03:44 +0600 Subject: [PATCH 02/29] update: get_variation function changed --- optimizely/decision_service.py | 33 +++++++++++++-------------------- optimizely/user_profile.py | 2 +- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 3e671245..620278b2 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -279,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.' @@ -300,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: + 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.') @@ -344,10 +337,10 @@ 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: 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) + self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) except: self.logger.exception(f'Unable to save user profile for user "{user_id}".') return variation, decide_reasons diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 3a311c49..a507d718 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -99,7 +99,7 @@ class UserProfileTracker: def __init__(self, user_id: str, user_profile_service: UserProfileService, logger=None): self.user_id = user_id self.user_profile_service = user_profile_service - # self.logger = logger or logging.getLogger(__name__) + self.logger = logger self.profile_updated = False self.user_profile = None From d4623395f1d1922b42b4892240aca4635c69dee4 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 6 Nov 2024 06:09:26 +0600 Subject: [PATCH 03/29] update: new function in decision_service --- optimizely/decision_service.py | 123 +++++++++++++++++++++++---------- optimizely/optimizely.py | 9 ++- optimizely/user_profile.py | 9 ++- 3 files changed, 99 insertions(+), 42 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 620278b2..c3df3185 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -476,44 +476,8 @@ 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, @@ -577,3 +541,86 @@ 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[list[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 = [] + + ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options + + 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: + 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, + user_profile_tracker, + decide_reasons, + 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 + decision = (variation, decide_reasons) + decisions.append(decision) + + if self.user_profile_service 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 5b945095..adbbdfba 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1348,8 +1348,13 @@ def _decide_for_keys( # user_context, decide_options) flags_without_forced_decision.append(feature_flag) - #needs to be implemented - decisionList = self.decision_service.get_variation_for_feature_list(flags_without_forced_decision, user_context, project_config, merged_decide_options) + + decisionList = self.decision_service.get_variations_for_feature_list( + project_config, + flags_without_forced_decision, + user_context, + merged_decide_options + ) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index a507d718..128f6389 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -22,7 +22,12 @@ 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 .project_config import ProjectConfig + from .logger import Logger class UserProfile: @@ -96,7 +101,7 @@ def save(self, user_profile: dict[str, Any]) -> None: pass class UserProfileTracker: - def __init__(self, user_id: str, user_profile_service: UserProfileService, logger=None): + def __init__(self, user_id: str, user_profile_service: UserProfileService, logger=Logger): self.user_id = user_id self.user_profile_service = user_profile_service self.logger = logger From fc976ff0abe6ef7b41d78910a8e037d59b12413f Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 6 Nov 2024 22:47:54 +0600 Subject: [PATCH 04/29] update: everything implemented from java. tests are failing --- optimizely/decision_service.py | 2 +- optimizely/optimizely.py | 164 +++++++++++++++++---------------- optimizely/user_profile.py | 3 - 3 files changed, 87 insertions(+), 82 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index c3df3185..d0f61740 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -248,7 +248,7 @@ def get_variation( experiment: entities.Experiment, user_context: OptimizelyUserContext, user_profile_tracker: UserProfileTracker, - reasons: list[str], + 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. diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index adbbdfba..6505c631 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -46,7 +46,7 @@ if TYPE_CHECKING: # prevent circular dependency by skipping import at runtime - from .user_profile import UserProfileService + from .user_profile import UserProfileService, UserProfileTracker from .helpers.event_tag_utils import EventTags @@ -169,6 +169,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. @@ -630,8 +631,8 @@ 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 = UserProfileTracker(user_context.user_id, self.user_profile_service, self.logger) + variation, _ = self.decision_service.get_variation(project_config, experiment, user_context, user_profile_tracker) if variation: variation_key = variation.key @@ -1119,55 +1120,62 @@ 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: Optional[OptimizelyUserContext], + flag_key: str, + flag_decision: Optional[Decision], + decision_reasons: Optional[list[str]], + decide_options: Optional[list[OptimizelyDecideOption]], + project_config: ProjectConfig + ) -> OptimizelyDecision: user_id = user_context.user_id - attributes = user_context.get_user_attributes() - variation_key = None - variation = None feature_enabled = False + 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 = None - flag_key = key all_variables = {} - experiment = None decision_source = DecisionSources.ROLLOUT - source_info: dict[str, Any] = {} 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 + + feature_flag = project_config.feature_key_map.get(flag_key) # 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 + # 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 # 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 '', + 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 '', decision_source, feature_enabled, user_id, attributes) @@ -1178,14 +1186,14 @@ def _decide( 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 @@ -1204,39 +1212,19 @@ def _decide( 'flag_key': flag_key, 'enabled': feature_enabled, 'variables': all_variables, - 'variation_key': variation_key, + 'variation_key': flag_decision.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 }, ) - return OptimizelyDecision(variation_key=variation_key, enabled=feature_enabled, variables=all_variables, + return OptimizelyDecision(variation_key=flag_decision.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 _create_optimizely_decision( - self, - user_context: Optional[OptimizelyUserContext], - flag_key: str, - flag_decision: Optional[Decision], - decision_reasons: Optional[list[str]], - decide_options: Optional[list[OptimizelyDecideOption]], - project_config: ProjectConfig - ) -> OptimizelyDecision: - user_id = user_context.user_id - flag_enabled = False - if flag_decision.variation is not None: - if flag_decision.variation.featureEnabled: - flag_enabled = True - - self.logger.info(f'Feature {flag_key} is enabled for user {user_id} {flag_enabled}"') - variable_dict = {} - # if OptimizelyDecideOption.EXCLUDE_VARIABLES not in decide_options: - # decision_variables = self. - return + def _decide_all( self, @@ -1310,15 +1298,17 @@ def _decide_for_keys( decisions = {} valid_keys = [] - reasons = [] decision_reasons_dict = {} - for key in keys: - decision = self._decide(user_context, key, decide_options) - if enabled_flags_only and not decision.enabled: - continue - decisions[key] = decision + # for key in keys: + # decision = self._decide(user_context, key, decide_options) + # if enabled_flags_only and not decision.enabled: + # continue + # decisions[key] = decision project_config = self.config_manager.get_config() + flags_without_forced_decision: list[entities.FeatureFlag] = [] + flag_decisions: dict[str, Decision] = {} + for key in keys: feature_flag = project_config.feature_key_map.get(key) if feature_flag is None: @@ -1333,10 +1323,7 @@ def _decide_for_keys( optimizely_decision_context, user_context) variation, decision_reasons = forced_decision_response - reasons += decision_reasons - - flags_without_forced_decision = [] - flag_decisions = {} + decision_reasons_dict[key] += decision_reasons if variation: decision = Decision(None, variation, enums.DecisionSources.FEATURE_TEST) @@ -1349,14 +1336,35 @@ def _decide_for_keys( flags_without_forced_decision.append(feature_flag) - decisionList = self.decision_service.get_variations_for_feature_list( + 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 + ) + if (OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options) or (optimizely_decision.enabled): + decisions[key] = optimizely_decision return decisions diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 128f6389..b3b066be 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -14,7 +14,6 @@ from __future__ import annotations from typing import Any, Optional from sys import version_info -from .helpers.validator import is_user_profile_valid from .entities import Experiment, Variation from .decision_service import Decision from optimizely.error_handler import BaseErrorHandler @@ -117,8 +116,6 @@ def load_user_profile(self, reasons: Optional[list[str]], error_handler: Optiona if user_profile is None: message = reasons.append("Unable to get a user profile from the UserProfileService.") self.logger.info(message) - elif is_user_profile_valid(user_profile): - self.user_profile = user_profile else: message = reasons.append("The UserProfileService returned an invalid user_profile.") self.logger.warning(message) From 4187366efd224775a26f9339c1a6976102b17bcd Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 6 Nov 2024 23:38:38 +0600 Subject: [PATCH 05/29] update: minor changes --- optimizely/optimizely.py | 4 ++-- optimizely/user_profile.py | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 6505c631..34ac5e5b 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1315,8 +1315,8 @@ def _decide_for_keys( decisions[key] = OptimizelyDecision(None, False, None, None, key, user_context, []) continue valid_keys.append(key) - # decision_reasons = [] - # decision_reasons_dict[key] = decision_reasons + decision_reasons = [] + 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, diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index b3b066be..574683d6 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -14,9 +14,7 @@ from __future__ import annotations from typing import Any, Optional from sys import version_info -from .entities import Experiment, Variation -from .decision_service import Decision -from optimizely.error_handler import BaseErrorHandler + if version_info < (3, 8): from typing_extensions import Final @@ -27,6 +25,9 @@ # prevent circular dependenacy by skipping import at runtime from .project_config import ProjectConfig from .logger import Logger + from .entities import Experiment, Variation + from .decision_service import Decision + from optimizely.error_handler import BaseErrorHandler class UserProfile: @@ -100,7 +101,7 @@ def save(self, user_profile: dict[str, Any]) -> None: pass class UserProfileTracker: - def __init__(self, user_id: str, user_profile_service: UserProfileService, logger=Logger): + def __init__(self, user_id: str, user_profile_service: UserProfileService, logger:Logger): self.user_id = user_id self.user_profile_service = user_profile_service self.logger = logger From 8d321804a87fbef295485179f26c8915325c4fc1 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 8 Nov 2024 22:08:21 +0600 Subject: [PATCH 06/29] update: user_profile_tracker added to tests --- optimizely/decision_service.py | 15 ++++++++---- optimizely/optimizely.py | 22 +++++++++-------- optimizely/user_profile.py | 13 +++++----- tests/test_decision_service.py | 43 +++++++++++++++++++++++++++++----- tests/test_optimizely.py | 12 ++++++---- 5 files changed, 75 insertions(+), 30 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index d0f61740..3a241976 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -303,6 +303,7 @@ def get_variation( # Check to see if user has a decision available for the given experiment if user_profile_tracker is not None: + user_profile_tracker.load_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 ' \ @@ -547,7 +548,7 @@ def get_variations_for_feature_list( project_config: ProjectConfig, features: list[entities.FeatureFlag], user_context: OptimizelyUserContext, - options: Optional[list[str]] = None + 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. @@ -562,8 +563,13 @@ def get_variations_for_feature_list( """ decide_reasons = [] - ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options + if options: + ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options + else: + ignore_ups = False + + user_profile_tracker: 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) @@ -605,8 +611,9 @@ def get_variations_for_feature_list( 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 + decision = [Decision(experiment, decision_variation, enums.DecisionSources.FEATURE_TEST), decide_reasons] + decisions.append(decision) + continue message = f'User "{user_context.user_id}" is not bucketed into any of the ' \ f'experiments on the feature "{feature.key}".' diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 34ac5e5b..1eefe618 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 @@ -46,7 +47,7 @@ if TYPE_CHECKING: # prevent circular dependency by skipping import at runtime - from .user_profile import UserProfileService, UserProfileTracker + from .user_profile import UserProfileService from .helpers.event_tag_utils import EventTags @@ -631,7 +632,7 @@ def get_variation( return None user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) - user_profile_tracker = UserProfileTracker(user_context.user_id, self.user_profile_service, self.logger) + user_profile_tracker = user_profile.UserProfileTracker(user_id, self.user_profile_service, self.logger) variation, _ = self.decision_service.get_variation(project_config, experiment, user_context, user_profile_tracker) if variation: variation_key = variation.key @@ -1152,7 +1153,7 @@ def _create_optimizely_decision( # Create Optimizely Decision Result. attributes = user_context.get_user_attributes() - rule_key = None + rule_key = flag_decision.experiment.key if flag_decision.experiment else None all_variables = {} decision_source = DecisionSources.ROLLOUT decision_event_dispatched = False @@ -1160,12 +1161,12 @@ def _create_optimizely_decision( feature_flag = project_config.feature_key_map.get(flag_key) # Fill in experiment and variation if returned (rollouts can have featureEnabled variables as well.) - # if decision.experiment is not None: - # experiment = decision.experiment + # if flag_decision.experiment is not None: + # experiment = flag_decision.experiment # source_info["experiment"] = experiment # rule_key = experiment.key if experiment else None - # if decision.variation is not None: - # variation = decision.variation + # if flag_decision.variation is not None: + # variation = flag_decision.variation # variation_key = variation.key # feature_enabled = variation.featureEnabled # decision_source = decision.source @@ -1201,7 +1202,7 @@ def _create_optimizely_decision( 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, @@ -1212,7 +1213,7 @@ def _create_optimizely_decision( 'flag_key': flag_key, 'enabled': feature_enabled, 'variables': all_variables, - 'variation_key': flag_decision.variation.key, + 'variation_key': variation_key, 'rule_key': rule_key, 'reasons': decision_reasons if should_include_reasons else [], 'decision_event_dispatched': decision_event_dispatched @@ -1220,7 +1221,7 @@ def _create_optimizely_decision( }, ) - return OptimizelyDecision(variation_key=flag_decision.variation.key, enabled=feature_enabled, variables=all_variables, + 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=decision_reasons if should_include_reasons else [] ) @@ -1308,6 +1309,7 @@ def _decide_for_keys( project_config = self.config_manager.get_config() flags_without_forced_decision: list[entities.FeatureFlag] = [] flag_decisions: dict[str, Decision] = {} + for key in keys: feature_flag = project_config.feature_key_map.get(key) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 574683d6..973d0f55 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -14,7 +14,8 @@ from __future__ import annotations from typing import Any, Optional from sys import version_info - +from . import logger as _logging +from . import decision_service if version_info < (3, 8): from typing_extensions import Final @@ -101,17 +102,17 @@ def save(self, user_profile: dict[str, Any]) -> None: pass class UserProfileTracker: - def __init__(self, user_id: str, user_profile_service: UserProfileService, logger:Logger): + def __init__(self, user_id: str, user_profile_service: UserProfileService, logger:Optional[_logging.Logger] = None): self.user_id = user_id self.user_profile_service = user_profile_service - self.logger = logger + self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) self.profile_updated = False self.user_profile = None def get_user_profile(self): return self.user_profile - def load_user_profile(self, reasons: Optional[list[str]], error_handler: Optional[BaseErrorHandler]): + def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Optional[BaseErrorHandler]=None): try: user_profile = self.user_profile_service.lookup(self.user_id) if user_profile is None: @@ -135,14 +136,14 @@ def update_user_profile(self, experiment: Experiment, variation: Variation): decision = self.user_profile.experiment_bucket_map[experiment.id] decision.variation = variation else: - decision = Decision(experiment=None, variation=variation, source=None) + decision = decision_service.Decision(experiment=None, variation=variation, source=None) self.user_profile.experiment_bucket_map[experiment.id] = decision self.profile_updated = True # self.logger.info(f'Updated variation "{variation.id}" of experiment "{experiment.id}" for user "{self.user_profile.user_id}".') - def save_user_profile(self, error_handler: Optional[BaseErrorHandler]): + def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None): if not self.profile_updated: return diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 4d755de5..aa9763fe 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", @@ -575,13 +582,14 @@ def test_get_variation__user_has_stored_decision(self): "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.assertEqual( entities.Variation("111128", "control"), variation, ) - + print("Actual UserProfile argument:", mock_get_stored_variation.call_args[0][2].__dict__) + print("Expected UserProfile argument:", user_profile.UserProfile("test_user", {"111127": {"variation_id": "111128"}}).__dict__) # 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" @@ -901,6 +909,8 @@ def test_get_variation__user_profile_save_fails(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" @@ -921,7 +931,7 @@ def test_get_variation__user_profile_save_fails(self): side_effect=Exception("major problem"), ) as mock_save: 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"), @@ -963,6 +973,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 +995,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 +1304,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 +1433,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 +1463,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 +1492,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 +1580,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 ) @@ -1777,6 +1798,8 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 user_id="test_user", user_attributes={ "experiment_attr": "group_experiment_invalid"}) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") expected_experiment = self.project_config.get_experiment_from_id("211147") expected_variation = self.project_config.get_variation_from_id( @@ -1789,6 +1812,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 +1827,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..f7307aca 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -369,9 +369,10 @@ 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 +767,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 +1123,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( From ba44a6c92a3958982c2811ffba8da0b9e82154b1 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 12:11:26 +0600 Subject: [PATCH 07/29] update: some tests fixed --- tests/test_user_context.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 48f08885..1ed50a14 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1053,21 +1053,25 @@ 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_once() def test_decide_for_all(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) @@ -1323,9 +1327,9 @@ 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]) From c78df2f501f76010927a3de1b13287925c318826 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 14:36:49 +0600 Subject: [PATCH 08/29] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20optimizely/decisi?= =?UTF-8?q?on=5Fservice.py=20->=20Added=20check=20for=20`ignore=5Fuser=5Fp?= =?UTF-8?q?rofile`=20in=20decision=20logic.=20=F0=9F=9B=A0=EF=B8=8F=20opti?= =?UTF-8?q?mizely/user=5Fprofile.py=20->=20Improved=20user=20profile=20loa?= =?UTF-8?q?ding=20with=20missing=20key=20checks.=20=F0=9F=9B=A0=EF=B8=8F?= =?UTF-8?q?=20tests/test=5Fdecision=5Fservice.py=20->=20Updated=20tests=20?= =?UTF-8?q?to=20include=20user=20profile=20tracker.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- optimizely/decision_service.py | 4 ++-- optimizely/user_profile.py | 12 ++++++++++-- tests/test_decision_service.py | 4 +++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 3a241976..0265c45d 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -302,7 +302,7 @@ def get_variation( return variation, decide_reasons # Check to see if user has a decision available for the given experiment - if user_profile_tracker is not None: + if user_profile_tracker is not None and not ignore_user_profile: user_profile_tracker.load_user_profile() variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile()) if variation: @@ -338,7 +338,7 @@ def get_variation( self.logger.info(message) decide_reasons.append(message) # Store this new decision and return the variation for the user - if user_profile_tracker is not None: + if user_profile_tracker is not None and not ignore_user_profile: try: user_profile_tracker.update_user_profile(experiment, variation) self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 973d0f55..f061459a 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -119,8 +119,16 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti message = reasons.append("Unable to get a user profile from the UserProfileService.") self.logger.info(message) else: - message = reasons.append("The UserProfileService returned an invalid user_profile.") - self.logger.warning(message) + 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 = reasons.append(str(exception)) self.logger.error(message) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index aa9763fe..54e81e5b 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -616,6 +616,8 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a 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" @@ -637,7 +639,7 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a "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.assertEqual( entities.Variation("111129", "variation"), From d02c3d8af936ee7e24087ab56d49ebd1d8101a60 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 17:21:03 +0600 Subject: [PATCH 09/29] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20tests/test=5Fdeci?= =?UTF-8?q?sion=5Fservice.py=20->=20Added=20expected=20decision=20object.?= =?UTF-8?q?=20=F0=9F=9B=A0=EF=B8=8F=20tests/test=5Fdecision=5Fservice.py?= =?UTF-8?q?=20->=20Updated=20experiment=20bucket=20map=20call.=20?= =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20tests/test=5Fdecision=5Fservice.py=20->?= =?UTF-8?q?=20Introduced=20user=5Fprofile=5Ftracker=20usage.=20?= =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20tests/test=5Fdecision=5Fservice.py=20->?= =?UTF-8?q?=20Modified=20method=20calls=20with=20user=5Fprofile=5Ftracker.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_decision_service.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 54e81e5b..389a9fb5 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -650,6 +650,11 @@ 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 ) + expected_decision = decision_service.Decision( + experiment=None, + variation=entities.Variation("111129", "variation"), + source=None + ) mock_lookup.assert_called_once_with("test_user") self.assertEqual(1, mock_get_stored_variation.call_count) mock_audience_check.assert_called_once_with( @@ -666,7 +671,7 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a mock_save.assert_called_once_with( { "user_id": "test_user", - "experiment_bucket_map": {"111127": {"variation_id": "111129"}}, + "experiment_bucket_map": {"111127": expected_decision}, } ) @@ -735,6 +740,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" @@ -755,7 +761,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): "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 @@ -787,6 +793,7 @@ def test_get_variation__user_profile_in_invalid_format(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" @@ -807,7 +814,7 @@ def test_get_variation__user_profile_in_invalid_format(self): "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.assertEqual( entities.Variation("111129", "variation"), From c67bcf389124b272ef6a8d1b468876c1cbce06b7 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 21:39:25 +0600 Subject: [PATCH 10/29] optimizely/decision_service.py -> fixed get_variations_for_feature_list --- optimizely/decision_service.py | 54 ++++++++++++++++------------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 0265c45d..502f8c21 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -577,53 +577,49 @@ def get_variations_for_feature_list( decisions = [] for feature in features: - decide_reasons = [] + 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 and the the user is bucketed into one of these experiments + # Check if the feature flag is under an experiment 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) - + 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 + 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, - decide_reasons, - options + project_config, experiment, user_context, user_profile_tracker, feature_reasons, options ) - decide_reasons += variation_reasons + feature_reasons.extend(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) - decision = [Decision(experiment, decision_variation, enums.DecisionSources.FEATURE_TEST), decide_reasons] - decisions.append(decision) - continue - - 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 - decision = (variation, decide_reasons) - decisions.append(decision) + self.logger.debug(f'User "{user_context.user_id}" 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) + feature_reasons.extend(rollout_reasons) + + if rollout_decision: + self.logger.debug(f'User "{user_context.user_id}" bucketed into rollout for feature "{feature.key}".') + else: + self.logger.debug(f'User "{user_context.user_id}" not bucketed into any rollout for feature "{feature.key}".') + + decisions.append((rollout_decision, feature_reasons)) + if self.user_profile_service is not None and ignore_ups is False: user_profile_tracker.save_user_profile() From 820ba3eb37f03d2bf932be4fe979a36e2959c8a3 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 22:10:00 +0600 Subject: [PATCH 11/29] optimizely/decision_service.py -> Fixed how rollout reasons are added tests/test_decision_service.py -> Added user profile tracker object --- optimizely/decision_service.py | 2 +- tests/test_decision_service.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 502f8c21..4a0d07e0 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -611,7 +611,7 @@ def get_variations_for_feature_list( # 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) - feature_reasons.extend(rollout_reasons) + feature_reasons.append(rollout_reasons) if rollout_decision: self.logger.debug(f'User "{user_context.user_id}" bucketed into rollout for feature "{feature.key}".') diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 389a9fb5..be1e6a2b 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -856,6 +856,7 @@ def test_get_variation__user_profile_lookup_fails(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" @@ -876,7 +877,7 @@ def test_get_variation__user_profile_lookup_fails(self): "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.assertEqual( entities.Variation("111129", "variation"), @@ -1473,7 +1474,7 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self self.project_config.get_experiment_from_key("test_experiment"), user, None, - [], + [[]], None ) @@ -1501,7 +1502,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, None, [], 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( From 309223ce9b4bcd3073b805a8c471b223426b35c5 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Nov 2024 23:50:15 +0600 Subject: [PATCH 12/29] tests/test_user_context.py -> fixed some tests --- tests/test_user_context.py | 53 ++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 1ed50a14..b63376f5 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -678,9 +678,9 @@ 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( @@ -968,14 +968,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 +987,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 +1006,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 @@ -1071,7 +1069,12 @@ def test_decide_for_keys__default_options__with__options(self): user_context.decide_for_keys(flags, options) - mock_get_variations.assert_called_once() + 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): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) From ca1b24831a04022b07035655b112ff8931c9a17b Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Nov 2024 06:00:47 +0600 Subject: [PATCH 13/29] optimizely/user_profile.py -> Updated type for `experiment_bucket_map`. tests/test_decision_service.py -> Fixed tests --- optimizely/user_profile.py | 4 +- tests/test_decision_service.py | 79 +++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index f061459a..f68140ed 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -46,7 +46,7 @@ class UserProfile: def __init__( self, user_id: str, - experiment_bucket_map: Optional[dict[str, dict[str, Optional[str]]]] = None, + experiment_bucket_map: Optional[dict[str, Decision]] = None, **kwargs: Any ): self.user_id = user_id @@ -131,7 +131,7 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti reasons.append(message) except Exception as exception: message = reasons.append(str(exception)) - self.logger.error(message) + self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}"as lookup failed.') # Todo: add error handler # error_handler.handle_error() diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index be1e6a2b..178e4453 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -788,7 +788,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): 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", @@ -801,7 +801,7 @@ def test_get_variation__user_profile_in_invalid_format(self): "optimizely.decision_service.DecisionService.get_whitelisted_variation", return_value=[None, []], ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" + "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None, ) as mock_get_stored_variation, mock.patch( "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] ) as mock_audience_check, mock.patch( @@ -814,20 +814,33 @@ def test_get_variation__user_profile_in_invalid_format(self): "optimizely.user_profile.UserProfileService.save" ) as mock_save: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, user_profile_tracker + self.project_config, + experiment, + user, + user_profile_tracker, # Pass the tracker + [], # Empty reasons list + None # No options ) 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_get_stored_variation.assert_called_once_with( + self.project_config, + experiment, + user_profile_tracker.get_user_profile() # Get the actual UserProfile object + ) + + # 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(), @@ -842,12 +855,13 @@ def test_get_variation__user_profile_in_invalid_format(self): 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"}}, + mock_save.assert_called_once_with({ + "user_id": "test_user", + "experiment_bucket_map": { + "111127": mock.ANY } - ) + }) + def test_get_variation__user_profile_lookup_fails(self): """ Test that get_variation acts gracefully when lookup fails. """ @@ -864,7 +878,8 @@ def test_get_variation__user_profile_lookup_fails(self): "optimizely.decision_service.DecisionService.get_whitelisted_variation", return_value=[None, []], ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" + "optimizely.decision_service.DecisionService.get_stored_variation", + return_value=None ) as mock_get_stored_variation, mock.patch( "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] ) as mock_audience_check, mock.patch( @@ -877,7 +892,7 @@ def test_get_variation__user_profile_lookup_fails(self): "optimizely.user_profile.UserProfileService.save" ) as mock_save: variation, _ = self.decision_service.get_variation( - self.project_config, experiment, user, user_profile_tracker + self.project_config, experiment, user, user_profile_tracker, [], None ) self.assertEqual( entities.Variation("111129", "variation"), @@ -889,8 +904,7 @@ def test_get_variation__user_profile_lookup_fails(self): 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(), @@ -899,18 +913,20 @@ def test_get_variation__user_profile_lookup_fails(self): 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"}}, + mock_save.assert_called_once_with({ + "user_id": "test_user", + "experiment_bucket_map": { + "111127": decision_service.Decision( + experiment=None, + variation=mock.ANY, # Don't care about the specific object instance + source=None + ) } - ) + }) def test_get_variation__user_profile_save_fails(self): """ Test that get_variation acts gracefully when save fails. """ @@ -928,7 +944,8 @@ def test_get_variation__user_profile_save_fails(self): "optimizely.decision_service.DecisionService.get_whitelisted_variation", return_value=[None, []], ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.decision_service.DecisionService.get_stored_variation" + "optimizely.decision_service.DecisionService.get_stored_variation", + return_value=None ) as mock_get_stored_variation, mock.patch( "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] ) as mock_audience_check, mock.patch( @@ -953,7 +970,7 @@ def test_get_variation__user_profile_save_fails(self): 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(), @@ -969,12 +986,16 @@ def test_get_variation__user_profile_save_fails(self): 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"}}, + mock_save.assert_called_once_with({ + "user_id": "test_user", + "experiment_bucket_map": { + "111127": decision_service.Decision( + experiment=None, + variation=mock.ANY, + source=None + ) } - ) + }) def test_get_variation__ignore_user_profile_when_specified(self): """ Test that we ignore the user profile service if specified. """ From d0bc3320b34bde57f66aaf70c35847c80f31fb39 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Nov 2024 07:21:53 +0600 Subject: [PATCH 14/29] all unit tests passing --- optimizely/optimizely.py | 11 ++++-- tests/test_user_context.py | 75 +++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 1eefe618..15ba2f3a 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1133,6 +1133,10 @@ def _decide( return decision + def _fix_nested_decision_reasons_list(self, decision_reasons)->list: + if len(decision_reasons)==1 and type(decision_reasons[0])==type(list()): + decision_reasons = decision_reasons[0] + return decision_reasons def _create_optimizely_decision( self, @@ -1155,7 +1159,7 @@ def _create_optimizely_decision( attributes = user_context.get_user_attributes() rule_key = flag_decision.experiment.key if flag_decision.experiment else None all_variables = {} - decision_source = DecisionSources.ROLLOUT + decision_source = flag_decision.source decision_event_dispatched = False feature_flag = project_config.feature_key_map.get(flag_key) @@ -1352,10 +1356,11 @@ def _decide_for_keys( flag_decisions[flag_key] = decision decision_reasons_dict[flag_key] += reasons - + print(decision_reasons_dict) for key in valid_keys: flag_decision = flag_decisions[key] decision_reasons = decision_reasons_dict[key] + decision_reasons = self._fix_nested_decision_reasons_list(decision_reasons) optimizely_decision = self._create_optimizely_decision( user_context, key, @@ -1367,7 +1372,7 @@ def _decide_for_keys( if (OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options) or (optimizely_decision.enabled): decisions[key] = optimizely_decision - + return decisions def _setup_odp(self, sdk_key: Optional[str]) -> None: diff --git a/tests/test_user_context.py b/tests/test_user_context.py index b63376f5..7ee3f1d4 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -228,9 +228,9 @@ 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 +303,9 @@ 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 +478,9 @@ 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 +553,9 @@ 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 +614,9 @@ 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( @@ -679,8 +679,8 @@ def test_decide__default_option__disable_decision_event(self): with mock.patch( 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', - return_value=([decision_service.Decision(mock_experiment, mock_variation, - enums.DecisionSources.FEATURE_TEST), []]), + 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 +739,9 @@ 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 +835,9 @@ 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 +914,9 @@ 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( @@ -1026,19 +1026,24 @@ 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 ) + # mock_decide.assert_any_call( + # user_context, + # 'test_feature_in_experiment', + # options + # ) + + # mock_decide.assert_any_call( + # user_context, + # 'test_feature_in_rollout', + # options + # ) self.assertEqual(mocked_decision_1, decisions['test_feature_in_experiment']) @@ -1638,6 +1643,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": [].', From e6e442abdec3eae971a73b6e8c98ec4e20bb2ecf Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Nov 2024 22:00:52 +0600 Subject: [PATCH 15/29] lint check --- optimizely/decision_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 4a0d07e0..6364903d 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -307,7 +307,7 @@ def get_variation( 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.' + f'"{experiment}" for user "{user_id}" from user profile.' self.logger.info(message) decide_reasons.append(message) return variation, decide_reasons From 0c9272272c31341e28c99b00f4e145500447c244 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 10:59:20 +0600 Subject: [PATCH 16/29] fix: typechecks added --- optimizely/decision_service.py | 17 +++++++++-------- optimizely/user_profile.py | 35 ++++++++++++++++++++++------------ tests/test_decision_service.py | 4 ++-- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 6364903d..4d0f787c 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -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,7 +247,7 @@ def get_variation( project_config: ProjectConfig, experiment: entities.Experiment, user_context: OptimizelyUserContext, - user_profile_tracker: UserProfileTracker, + user_profile_tracker: Optional[UserProfileTracker], reasons: list[str] = [], options: Optional[Sequence[str]] = None ) -> tuple[Optional[entities.Variation], list[str]]: @@ -341,7 +341,8 @@ def get_variation( if user_profile_tracker is not None and not ignore_user_profile: try: user_profile_tracker.update_user_profile(experiment, variation) - self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) + if self.user_profile_service is not None: + self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) except: self.logger.exception(f'Unable to save user profile for user "{user_id}".') return variation, decide_reasons @@ -561,7 +562,7 @@ def get_variations_for_feature_list( Returns: List of Decision namedtuple consisting of experiment and variation for the user. """ - decide_reasons = [] + decide_reasons: list[str] = [] if options: ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options @@ -569,7 +570,7 @@ def get_variations_for_feature_list( ignore_ups = False - user_profile_tracker: UserProfileTracker = None + 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) @@ -611,8 +612,8 @@ def get_variations_for_feature_list( # 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) - feature_reasons.append(rollout_reasons) - + if rollout_reasons: + feature_reasons.extend(rollout_reasons) if rollout_decision: self.logger.debug(f'User "{user_context.user_id}" bucketed into rollout for feature "{feature.key}".') else: @@ -620,7 +621,7 @@ def get_variations_for_feature_list( decisions.append((rollout_decision, feature_reasons)) - if self.user_profile_service is not None and ignore_ups is False: + 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/user_profile.py b/optimizely/user_profile.py index f68140ed..ef5c04cd 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -12,11 +12,11 @@ # limitations under the License. from __future__ import annotations -from typing import Any, Optional +from typing import Any, Optional, Union from sys import version_info from . import logger as _logging from . import decision_service - +from .helpers import enums if version_info < (3, 8): from typing_extensions import Final else: @@ -46,7 +46,7 @@ class UserProfile: def __init__( self, user_id: str, - experiment_bucket_map: Optional[dict[str, Decision]] = None, + experiment_bucket_map: Optional[dict[str, Union[Decision, dict[str, str]]]] = None, **kwargs: Any ): self.user_id = user_id @@ -64,8 +64,14 @@ def get_variation_for_experiment(self, experiment_id: str) -> Optional[str]: Returns: Variation ID corresponding to the experiment. None if no decision available. """ + experiment_data = self.experiment_bucket_map.get(experiment_id) + + if isinstance(experiment_data, decision_service.Decision): + return experiment_data.variation.id if experiment_data.variation is not None else None + elif isinstance(experiment_data, dict): + return experiment_data.get(self.VARIATION_ID_KEY) - return self.experiment_bucket_map.get(experiment_id, {self.VARIATION_ID_KEY: None}).get(self.VARIATION_ID_KEY) + return None def save_variation_for_experiment(self, experiment_id: str, variation_id: str) -> None: """ Helper method to save new experiment/variation as part of the user's profile. @@ -74,7 +80,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}}) @@ -107,16 +112,18 @@ def __init__(self, user_id: str, user_profile_service: UserProfileService, logge self.user_profile_service = user_profile_service self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) self.profile_updated = False - self.user_profile = None + self.user_profile = UserProfile(user_id, {}) def get_user_profile(self): return self.user_profile def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Optional[BaseErrorHandler]=None): + reasons = reasons if reasons else [] try: user_profile = self.user_profile_service.lookup(self.user_id) if user_profile is None: - message = reasons.append("Unable to get a user profile from the UserProfileService.") + message = "Unable to get a user profile from the UserProfileService." + reasons.append(message) self.logger.info(message) else: if 'user_id' in user_profile and 'experiment_bucket_map' in user_profile: @@ -130,7 +137,8 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti message = f"User profile is missing keys: {', '.join(missing_keys)}" reasons.append(message) except Exception as exception: - message = reasons.append(str(exception)) + message = str(exception) + reasons.append(message) self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}"as lookup failed.') # Todo: add error handler # error_handler.handle_error() @@ -139,10 +147,14 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti self.user_profile = UserProfile(self.user_id, {}) def update_user_profile(self, experiment: Experiment, variation: Variation): - decision:Decision = None if experiment.id in self.user_profile.experiment_bucket_map: decision = self.user_profile.experiment_bucket_map[experiment.id] - decision.variation = variation + if isinstance(decision, decision_service.Decision): + decision = decision_service.Decision( + experiment=decision.experiment, + variation=variation, + source=decision.source + ) else: decision = decision_service.Decision(experiment=None, variation=variation, source=None) @@ -154,9 +166,8 @@ def update_user_profile(self, experiment: Experiment, variation: Variation): def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None): if not self.profile_updated: return - try: - self.user_profile_service.save(self.user_profile) + 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}".') diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 178e4453..ef0a6d37 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1495,7 +1495,7 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self self.project_config.get_experiment_from_key("test_experiment"), user, None, - [[]], + [], None ) @@ -1523,7 +1523,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, None, [[]], 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( From af9715e13038adbb32194cb46fd817167535ddf9 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 11:08:03 +0600 Subject: [PATCH 17/29] more types updated --- optimizely/user_profile.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index ef5c04cd..2e34a414 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -107,7 +107,7 @@ def save(self, user_profile: dict[str, Any]) -> None: pass class UserProfileTracker: - def __init__(self, user_id: str, user_profile_service: UserProfileService, logger:Optional[_logging.Logger] = None): + 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()) @@ -120,11 +120,11 @@ def get_user_profile(self): def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Optional[BaseErrorHandler]=None): reasons = reasons if reasons else [] try: - user_profile = self.user_profile_service.lookup(self.user_id) + 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) - self.logger.info(message) + # self.logger.info(message) else: if 'user_id' in user_profile and 'experiment_bucket_map' in user_profile: self.user_profile = UserProfile( @@ -167,8 +167,9 @@ def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None): if not self.profile_updated: return try: - self.user_profile_service.save(self.user_profile.__dict__) - self.logger.info(f'Saved user profile of user "{self.user_profile.user_id}".') + 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}".') # error_handler.handle_error(exception) From e2d7c91708fc8f57fca87ade135954e0cc439938 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 11:29:29 +0600 Subject: [PATCH 18/29] all typechecks passing --- optimizely/optimizely.py | 39 +++++++++++---------------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 15ba2f3a..dee51c18 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -704,7 +704,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. @@ -715,7 +715,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: @@ -1133,18 +1133,13 @@ def _decide( return decision - def _fix_nested_decision_reasons_list(self, decision_reasons)->list: - if len(decision_reasons)==1 and type(decision_reasons[0])==type(list()): - decision_reasons = decision_reasons[0] - return decision_reasons - def _create_optimizely_decision( self, - user_context: Optional[OptimizelyUserContext], + user_context: OptimizelyUserContext, flag_key: str, - flag_decision: Optional[Decision], + flag_decision: Decision, decision_reasons: Optional[list[str]], - decide_options: Optional[list[OptimizelyDecideOption]], + decide_options: list[str], project_config: ProjectConfig ) -> OptimizelyDecision: user_id = user_context.user_id @@ -1164,30 +1159,18 @@ def _create_optimizely_decision( feature_flag = project_config.feature_key_map.get(flag_key) - # Fill in experiment and variation if returned (rollouts can have featureEnabled variables as well.) - # if flag_decision.experiment is not None: - # experiment = flag_decision.experiment - # source_info["experiment"] = experiment - # rule_key = experiment.key if experiment else None - # if flag_decision.variation is not None: - # variation = flag_decision.variation - # variation_key = variation.key - # feature_enabled = variation.featureEnabled - # decision_source = decision.source - # source_info["variation"] = variation - # 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 project_config.send_flag_decisions: self._send_impression_event(project_config, flag_decision.experiment, flag_decision.variation, flag_key, rule_key or '', - decision_source, feature_enabled, + 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: @@ -1301,7 +1284,7 @@ def _decide_for_keys( enabled_flags_only = OptimizelyDecideOption.ENABLED_FLAGS_ONLY in merged_decide_options - decisions = {} + decisions: dict[str, OptimizelyDecision] = {} valid_keys = [] decision_reasons_dict = {} # for key in keys: @@ -1314,14 +1297,15 @@ def _decide_for_keys( flags_without_forced_decision: list[entities.FeatureFlag] = [] flag_decisions: dict[str, Decision] = {} - + if project_config is None: + return decisions for key in keys: 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 valid_keys.append(key) - decision_reasons = [] + decision_reasons: list[str] = [] decision_reasons_dict[key] = decision_reasons optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(flag_key=key, rule_key=None) @@ -1360,7 +1344,6 @@ def _decide_for_keys( for key in valid_keys: flag_decision = flag_decisions[key] decision_reasons = decision_reasons_dict[key] - decision_reasons = self._fix_nested_decision_reasons_list(decision_reasons) optimizely_decision = self._create_optimizely_decision( user_context, key, From cc4da7ee0dc733332c9a866c4ea8dea9fce48f7c Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 11:33:22 +0600 Subject: [PATCH 19/29] gha typechecks fixed --- optimizely/user_profile.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 2e34a414..17aa8360 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -114,10 +114,10 @@ def __init__(self, user_id: str, user_profile_service: Optional[UserProfileServi self.profile_updated = False self.user_profile = UserProfile(user_id, {}) - def get_user_profile(self): + def get_user_profile(self) -> None: return self.user_profile - def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Optional[BaseErrorHandler]=None): + def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Optional[BaseErrorHandler]=None) -> None: reasons = reasons if reasons else [] try: user_profile = self.user_profile_service.lookup(self.user_id) if self.user_profile_service else None @@ -146,7 +146,7 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti if self.user_profile is None: self.user_profile = UserProfile(self.user_id, {}) - def update_user_profile(self, experiment: Experiment, variation: Variation): + def update_user_profile(self, experiment: Experiment, variation: Variation) -> None: if experiment.id in self.user_profile.experiment_bucket_map: decision = self.user_profile.experiment_bucket_map[experiment.id] if isinstance(decision, decision_service.Decision): @@ -163,7 +163,7 @@ def update_user_profile(self, experiment: Experiment, variation: Variation): # self.logger.info(f'Updated variation "{variation.id}" of experiment "{experiment.id}" for user "{self.user_profile.user_id}".') - def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None): + def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> None: if not self.profile_updated: return try: From dd62075347d461980aaa9f68a24c01132c25bc66 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 11:37:06 +0600 Subject: [PATCH 20/29] all typecheck should pass --- optimizely/user_profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 17aa8360..2f5d23f3 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -114,7 +114,7 @@ def __init__(self, user_id: str, user_profile_service: Optional[UserProfileServi self.profile_updated = False self.user_profile = UserProfile(user_id, {}) - def get_user_profile(self) -> None: + 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: From 2477d115ca1bb10c675b4b37baec6691b4486eff Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Nov 2024 13:30:28 +0600 Subject: [PATCH 21/29] lint check should pass --- optimizely/decision_service.py | 42 ++++----- optimizely/optimizely.py | 55 ++++++------ optimizely/user_profile.py | 31 +++---- tests/test_decision_service.py | 45 +++++----- tests/test_optimizely.py | 3 +- tests/test_user_context.py | 156 +++++++++++++++++++++++++-------- 6 files changed, 208 insertions(+), 124 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 4d0f787c..f5052b74 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -479,7 +479,6 @@ def get_variation_for_feature( Decision namedtuple consisting of experiment and variation for the user. """ return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0] - def validated_forced_decision( self, @@ -543,15 +542,15 @@ 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]]]: - """ + ) -> 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. @@ -563,24 +562,23 @@ def get_variations_for_feature_list( 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: @@ -603,28 +601,32 @@ def get_variations_for_feature_list( feature_reasons.extend(variation_reasons) if decision_variation: - self.logger.debug(f'User "{user_context.user_id}" bucketed into experiment "{experiment.key}" of feature "{feature.key}".') + self.logger.debug( + 'User "{}" bucketed into experiment "{}" of feature "{}".'.format( + user_context.user_id, experiment.key, 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) + 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}" bucketed into rollout for feature "{feature.key}".') + 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}" not bucketed into any rollout for feature "{feature.key}".') + 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 - - + return decisions diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index dee51c18..f785e524 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -633,7 +633,10 @@ def get_variation( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) user_profile_tracker = user_profile.UserProfileTracker(user_id, self.user_profile_service, self.logger) - variation, _ = self.decision_service.get_variation(project_config, experiment, user_context, user_profile_tracker) + variation, _ = self.decision_service.get_variation(project_config, + experiment, + user_context, + user_profile_tracker) if variation: variation_key = variation.key @@ -1123,16 +1126,16 @@ def _decide( 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, @@ -1147,23 +1150,26 @@ def _create_optimizely_decision( 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 = {} decision_source = flag_decision.source decision_event_dispatched = False - + 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 project_config.send_flag_decisions: - self._send_impression_event(project_config, flag_decision.experiment, flag_decision.variation, flag_key, rule_key or '', + 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) @@ -1189,7 +1195,11 @@ def _create_optimizely_decision( 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 + 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, @@ -1212,7 +1222,6 @@ def _create_optimizely_decision( rule_key=rule_key, flag_key=flag_key, user_context=user_context, reasons=decision_reasons if should_include_reasons else [] ) - def _decide_all( self, @@ -1282,7 +1291,7 @@ def _decide_for_keys( 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 + # enabled_flags_only = OptimizelyDecideOption.ENABLED_FLAGS_ONLY in merged_decide_options decisions: dict[str, OptimizelyDecision] = {} valid_keys = [] @@ -1292,11 +1301,11 @@ def _decide_for_keys( # if enabled_flags_only and not decision.enabled: # continue # decisions[key] = decision - + project_config = self.config_manager.get_config() flags_without_forced_decision: list[entities.FeatureFlag] = [] flag_decisions: dict[str, Decision] = {} - + if project_config is None: return decisions for key in keys: @@ -1307,39 +1316,34 @@ def _decide_for_keys( 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: - # Regular decision - # decision, decision_reasons = self.decision_service.get_variation_for_feature(project_config, - # feature_flag, - # user_context, decide_options) 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 - + print(decision_reasons_dict) for key in valid_keys: flag_decision = flag_decisions[key] @@ -1352,10 +1356,11 @@ def _decide_for_keys( merged_decide_options, project_config ) - - if (OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options) or (optimizely_decision.enabled): + 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 2f5d23f3..06930b24 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -16,16 +16,13 @@ from sys import version_info from . import logger as _logging from . import decision_service -from .helpers import enums if version_info < (3, 8): from typing_extensions import Final else: from typing import Final, TYPE_CHECKING # type: ignore - + if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime - from .project_config import ProjectConfig - from .logger import Logger from .entities import Experiment, Variation from .decision_service import Decision from optimizely.error_handler import BaseErrorHandler @@ -106,18 +103,23 @@ def save(self, user_profile: dict[str, Any]) -> None: """ pass + class UserProfileTracker: - def __init__(self, user_id: str, user_profile_service: Optional[UserProfileService], logger:Optional[_logging.Logger] = None): + 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: + def load_user_profile(self, reasons: Optional[list[str]] = [], + error_handler: Optional[BaseErrorHandler] = None) -> None: reasons = reasons if reasons else [] try: user_profile = self.user_profile_service.lookup(self.user_id) if self.user_profile_service else None @@ -128,7 +130,7 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti else: if 'user_id' in user_profile and 'experiment_bucket_map' in user_profile: self.user_profile = UserProfile( - user_profile['user_id'], + user_profile['user_id'], user_profile['experiment_bucket_map'] ) self.logger.info("User profile loaded successfully.") @@ -142,10 +144,10 @@ def load_user_profile(self, reasons: Optional[list[str]]=[], error_handler: Opti self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}"as lookup failed.') # Todo: add error handler # error_handler.handle_error() - + if self.user_profile is None: self.user_profile = UserProfile(self.user_id, {}) - + def update_user_profile(self, experiment: Experiment, variation: Variation) -> None: if experiment.id in self.user_profile.experiment_bucket_map: decision = self.user_profile.experiment_bucket_map[experiment.id] @@ -157,12 +159,10 @@ def update_user_profile(self, experiment: Experiment, variation: Variation) -> N ) else: decision = decision_service.Decision(experiment=None, variation=variation, source=None) - + self.user_profile.experiment_bucket_map[experiment.id] = decision self.profile_updated = True - # self.logger.info(f'Updated variation "{variation.id}" of experiment "{experiment.id}" for user "{self.user_profile.user_id}".') - - + def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> None: if not self.profile_updated: return @@ -171,5 +171,6 @@ def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> 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}".') + self.logger.warning(f'Failed to save user profile of user "{self.user_profile.user_id}" ' + f'for exception:{exception}".') # error_handler.handle_error(exception) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index ef0a6d37..0edcec30 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -588,8 +588,6 @@ def test_get_variation__user_has_stored_decision(self): entities.Variation("111128", "control"), variation, ) - print("Actual UserProfile argument:", mock_get_stored_variation.call_args[0][2].__dict__) - print("Expected UserProfile argument:", user_profile.UserProfile("test_user", {"111127": {"variation_id": "111128"}}).__dict__) # 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" @@ -651,8 +649,8 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a self.project_config, experiment, user.user_id ) expected_decision = decision_service.Decision( - experiment=None, - variation=entities.Variation("111129", "variation"), + experiment=None, + variation=entities.Variation("111129", "variation"), source=None ) mock_lookup.assert_called_once_with("test_user") @@ -788,7 +786,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): 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", @@ -825,22 +823,22 @@ def test_get_variation__user_profile_in_invalid_format(self): 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 mock_get_stored_variation.assert_called_once_with( self.project_config, experiment, user_profile_tracker.get_user_profile() # Get the actual UserProfile object ) - + # 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(), @@ -858,10 +856,9 @@ def test_get_variation__user_profile_in_invalid_format(self): mock_save.assert_called_once_with({ "user_id": "test_user", "experiment_bucket_map": { - "111127": mock.ANY + "111127": mock.ANY } }) - def test_get_variation__user_profile_lookup_fails(self): """ Test that get_variation acts gracefully when lookup fails. """ @@ -880,7 +877,7 @@ def test_get_variation__user_profile_lookup_fails(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None - ) as mock_get_stored_variation, mock.patch( + ), mock.patch( "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket", @@ -904,7 +901,7 @@ def test_get_variation__user_profile_lookup_fails(self): self.project_config, experiment, "test_user" ) mock_lookup.assert_called_once_with("test_user") - + mock_audience_check.assert_called_once_with( self.project_config, experiment.get_audience_conditions_or_ids(), @@ -913,7 +910,7 @@ def test_get_variation__user_profile_lookup_fails(self): user, mock_decision_service_logging ) - + mock_bucket.assert_called_once_with( self.project_config, experiment, "test_user", "test_user" ) @@ -946,7 +943,7 @@ def test_get_variation__user_profile_save_fails(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None - ) as mock_get_stored_variation, mock.patch( + ), mock.patch( "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=[True, []] ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket", @@ -970,7 +967,7 @@ def test_get_variation__user_profile_save_fails(self): self.project_config, experiment, "test_user" ) mock_lookup.assert_called_once_with("test_user") - + mock_audience_check.assert_called_once_with( self.project_config, experiment.get_audience_conditions_or_ids(), @@ -991,7 +988,7 @@ def test_get_variation__user_profile_save_fails(self): "experiment_bucket_map": { "111127": decision_service.Decision( experiment=None, - variation=mock.ANY, + variation=mock.ANY, source=None ) } @@ -1611,7 +1608,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 ) @@ -1829,8 +1826,6 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 user_id="test_user", user_attributes={ "experiment_attr": "group_experiment_invalid"}) - user_profile_service = user_profile.UserProfileService() - user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) feature = self.project_config.get_feature_from_key("test_feature_in_multiple_experiments") expected_experiment = self.project_config.get_experiment_from_id("211147") expected_variation = self.project_config.get_variation_from_id( @@ -1845,10 +1840,10 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 ) print(f"variation received is: {variation_received}") x = decision_service.Decision( - expected_experiment, - expected_variation, - enums.DecisionSources.ROLLOUT, - ) + expected_experiment, + expected_variation, + enums.DecisionSources.ROLLOUT, + ) print(f"need to be:{x}") self.assertEqual( decision_service.Decision( @@ -1858,7 +1853,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 f7307aca..79d655c5 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -372,7 +372,8 @@ def test_activate(self): 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, user_profile_tracker + self.project_config, self.project_config.get_experiment_from_key('test_experiment'), + user_context, user_profile_tracker ) self.assertEqual(1, mock_process.call_count) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 7ee3f1d4..6c4d7032 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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_variations_for_feature_list', - 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( @@ -972,7 +1044,7 @@ def side_effect(*args, **kwargs): res = {} for flag in flags: if flag == 'test_feature_in_experiment': - res[flag] = mocked_decision_1 + res[flag] = mocked_decision_1 else: res[flag] = mocked_decision_2 return res @@ -1010,7 +1082,7 @@ def side_effect(*args, **kwargs): res = {} for flag in flags: if flag == 'test_feature_in_experiment': - res[flag] = mocked_decision_1 + res[flag] = mocked_decision_1 else: res[flag] = mocked_decision_2 return res @@ -1064,18 +1136,18 @@ def test_decide_for_keys__default_options__with__options(self): 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.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,[])] - + + mock_get_variations.return_value = [(mock_decision, [])] + user_context.decide_for_keys(flags, options) mock_get_variations.assert_called_with( - mock.ANY, # ProjectConfig + mock.ANY, # ProjectConfig mock.ANY, # FeatureFlag list user_context, # UserContext object ['EXCLUDE_VARIABLES', 'ENABLED_FLAGS_ONLY'] @@ -1336,8 +1408,16 @@ def test_decide_experiment(self): mock_variation = project_config.get_variation_from_id('test_experiment', '111129') with mock.patch( 'optimizely.decision_service.DecisionService.get_variations_for_feature_list', - return_value=[(decision_service.Decision(mock_experiment, - mock_variation, enums.DecisionSources.FEATURE_TEST), []),] + 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]) From 27ef846e6e3e2215775393cc3e75c8a590f39c17 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 21 Nov 2024 01:11:02 +0600 Subject: [PATCH 22/29] removed unnecessary comments --- optimizely/decision_service.py | 4 ++-- optimizely/optimizely.py | 8 -------- optimizely/user_profile.py | 4 ---- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index f5052b74..49b0f574 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -602,8 +602,8 @@ def get_variations_for_feature_list( if decision_variation: self.logger.debug( - 'User "{}" bucketed into experiment "{}" of feature "{}".'.format( - user_context.user_id, experiment.key, feature.key) + 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)) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index f785e524..dbe4b3a0 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1291,16 +1291,9 @@ def _decide_for_keys( 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 = {} - # for key in keys: - # decision = self._decide(user_context, key, decide_options) - # if enabled_flags_only and not decision.enabled: - # continue - # decisions[key] = decision project_config = self.config_manager.get_config() flags_without_forced_decision: list[entities.FeatureFlag] = [] @@ -1344,7 +1337,6 @@ def _decide_for_keys( flag_decisions[flag_key] = decision decision_reasons_dict[flag_key] += reasons - print(decision_reasons_dict) for key in valid_keys: flag_decision = flag_decisions[key] decision_reasons = decision_reasons_dict[key] diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 06930b24..0b8b2dc0 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -126,7 +126,6 @@ def load_user_profile(self, reasons: Optional[list[str]] = [], if user_profile is None: message = "Unable to get a user profile from the UserProfileService." reasons.append(message) - # self.logger.info(message) else: if 'user_id' in user_profile and 'experiment_bucket_map' in user_profile: self.user_profile = UserProfile( @@ -142,8 +141,6 @@ def load_user_profile(self, reasons: Optional[list[str]] = [], message = str(exception) reasons.append(message) self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}"as lookup failed.') - # Todo: add error handler - # error_handler.handle_error() if self.user_profile is None: self.user_profile = UserProfile(self.user_id, {}) @@ -173,4 +170,3 @@ def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> except Exception as exception: self.logger.warning(f'Failed to save user profile of user "{self.user_profile.user_id}" ' f'for exception:{exception}".') - # error_handler.handle_error(exception) From e2810c8beb19debb672bc466f07282e6695473b7 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 21 Nov 2024 01:15:28 +0600 Subject: [PATCH 23/29] removed comments from test --- tests/test_user_context.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 6c4d7032..0c35e230 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1105,18 +1105,6 @@ def side_effect(*args, **kwargs): ['test_feature_in_rollout', 'test_feature_in_experiment'], options ) - # mock_decide.assert_any_call( - # user_context, - # 'test_feature_in_experiment', - # options - # ) - - # mock_decide.assert_any_call( - # user_context, - # 'test_feature_in_rollout', - # options - # ) - self.assertEqual(mocked_decision_1, decisions['test_feature_in_experiment']) def test_decide_for_keys__default_options__with__options(self): From 9228727efeddc3254d8c8b066959063501b4f4d6 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 21 Nov 2024 23:38:04 +0600 Subject: [PATCH 24/29] optimizely/decision_service.py -> Removed user profile save logic optimizely/optimizely.py -> Added loading and saving profile logic --- optimizely/decision_service.py | 2 -- optimizely/optimizely.py | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 49b0f574..be8cf678 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -341,8 +341,6 @@ def get_variation( if user_profile_tracker is not None and not ignore_user_profile: try: user_profile_tracker.update_user_profile(experiment, variation) - if self.user_profile_service is not None: - self.user_profile_service.save(user_profile_tracker.get_user_profile().__dict__) except: self.logger.exception(f'Unable to save user profile for user "{user_id}".') return variation, decide_reasons diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index dbe4b3a0..1b25bec6 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -633,10 +633,12 @@ def get_variation( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) 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 From b33a92bb30b29b0697f4af86bf2e825880aa206a Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 22 Nov 2024 10:27:01 +0600 Subject: [PATCH 25/29] optimizely/user_profile.py -> Updated experiment_bucket_map type optimizely/user_profile.py -> Testing user profile update logic --- optimizely/user_profile.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 0b8b2dc0..8ac58195 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -43,7 +43,7 @@ class UserProfile: def __init__( self, user_id: str, - experiment_bucket_map: Optional[dict[str, Union[Decision, dict[str, str]]]] = None, + experiment_bucket_map: Optional[dict[str, dict[str, Optional[str]]]] = None, **kwargs: Any ): self.user_id = user_id @@ -146,18 +146,21 @@ def load_user_profile(self, reasons: Optional[list[str]] = [], self.user_profile = UserProfile(self.user_id, {}) def update_user_profile(self, experiment: Experiment, variation: Variation) -> None: - if experiment.id in self.user_profile.experiment_bucket_map: - decision = self.user_profile.experiment_bucket_map[experiment.id] - if isinstance(decision, decision_service.Decision): - decision = decision_service.Decision( - experiment=decision.experiment, - variation=variation, - source=decision.source - ) - else: - decision = decision_service.Decision(experiment=None, variation=variation, source=None) - - self.user_profile.experiment_bucket_map[experiment.id] = decision + variation_id = variation.id + experiment_id = experiment.id + self.user_profile.save_variation_for_experiment(experiment_id, variation_id) + # if experiment.id in self.user_profile.experiment_bucket_map: + # decision = self.user_profile.experiment_bucket_map[experiment.id] + # if isinstance(decision, decision_service.Decision): + # decision = decision_service.Decision( + # experiment=decision.experiment, + # variation=variation, + # source=decision.source + # ) + # else: + # decision = decision_service.Decision(experiment=None, variation=variation, source=None) + + # self.user_profile.experiment_bucket_map[experiment.id] = decision self.profile_updated = True def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> None: From 1dd1eef2abd9a5a6bf65d937615a2c31c13c280f Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 22 Nov 2024 14:56:57 +0600 Subject: [PATCH 26/29] optimizely/decision_service.py -> Commented out profile loading optimizely/user_profile.py -> Removed unused import statement --- optimizely/decision_service.py | 2 +- optimizely/user_profile.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index be8cf678..ce44ed92 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -303,7 +303,7 @@ def get_variation( # Check to see if user has a decision available for the given experiment if user_profile_tracker is not None and not ignore_user_profile: - user_profile_tracker.load_user_profile() + # user_profile_tracker.load_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 ' \ diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 8ac58195..4baade33 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -12,7 +12,7 @@ # limitations under the License. from __future__ import annotations -from typing import Any, Optional, Union +from typing import Any, Optional from sys import version_info from . import logger as _logging from . import decision_service @@ -24,7 +24,6 @@ if TYPE_CHECKING: # prevent circular dependenacy by skipping import at runtime from .entities import Experiment, Variation - from .decision_service import Decision from optimizely.error_handler import BaseErrorHandler From 6387ca202a25fd507cc7814e93580e79eaf45bdd Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 25 Nov 2024 21:10:08 +0600 Subject: [PATCH 27/29] optimizely/decision_service.py -> Removed unused profile loading optimizely/user_profile.py -> Fixed handling of reasons list optimizely/user_profile.py -> Improved profile retrieval error logging tests/test_decision_service.py -> Updated mock checks to simplify tests tests/test_user_profile.py -> Added tests for user profile handling tests/test_optimizely.py -> New test for variation lookup and save --- optimizely/decision_service.py | 1 - optimizely/user_profile.py | 20 +-- tests/test_decision_service.py | 314 +-------------------------------- tests/test_optimizely.py | 29 +++ tests/test_user_profile.py | 74 ++++++++ 5 files changed, 113 insertions(+), 325 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index ce44ed92..df85464e 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -303,7 +303,6 @@ def get_variation( # Check to see if user has a decision available for the given experiment if user_profile_tracker is not None and not ignore_user_profile: - # user_profile_tracker.load_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 ' \ diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 4baade33..85897041 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -119,7 +119,8 @@ def get_user_profile(self) -> UserProfile: def load_user_profile(self, reasons: Optional[list[str]] = [], error_handler: Optional[BaseErrorHandler] = None) -> None: - reasons = reasons if reasons else [] + 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: @@ -139,27 +140,12 @@ def load_user_profile(self, reasons: Optional[list[str]] = [], 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 self.user_profile is None: - self.user_profile = UserProfile(self.user_id, {}) + self.logger.exception(f'Unable to retrieve user profile for user "{self.user_id}" as lookup failed.') 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) - # if experiment.id in self.user_profile.experiment_bucket_map: - # decision = self.user_profile.experiment_bucket_map[experiment.id] - # if isinstance(decision, decision_service.Decision): - # decision = decision_service.Decision( - # experiment=decision.experiment, - # variation=variation, - # source=decision.source - # ) - # else: - # decision = decision_service.Decision(experiment=None, variation=variation, source=None) - - # self.user_profile.experiment_bucket_map[experiment.id] = decision self.profile_updated = True def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> None: diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 0edcec30..6c5862a5 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -572,15 +572,7 @@ 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, user_profile_tracker ) @@ -592,23 +584,19 @@ def test_get_variation__user_has_stored_decision(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", {"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, @@ -630,12 +618,7 @@ 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, user_profile_tracker ) @@ -648,76 +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 ) - expected_decision = decision_service.Decision( - experiment=None, - variation=entities.Variation("111129", "variation"), - source=None - ) - 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": expected_decision}, - } - ) - - 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(), @@ -729,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. """ @@ -753,9 +667,6 @@ 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( @@ -769,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, @@ -784,216 +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={}) - 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" - ) 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", return_value=None, - ) 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, - user_profile_tracker, # Pass the tracker - [], # Empty reasons list - None # No options - ) - 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 - mock_get_stored_variation.assert_called_once_with( - self.project_config, - experiment, - user_profile_tracker.get_user_profile() # Get the actual UserProfile object - ) - - # 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": mock.ANY - } - }) - - 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={}) - 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" - ) 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", - return_value=None - ), 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, user_profile_tracker, [], 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") - - 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": decision_service.Decision( - experiment=None, - variation=mock.ANY, # Don't care about the specific object instance - source=None - ) - } - }) - - 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={}) - 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" - ) 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", - return_value=None - ), 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, user_profile_tracker - ) - 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") - - 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": decision_service.Decision( - experiment=None, - variation=mock.ANY, - source=None - ) - } - }) - def test_get_variation__ignore_user_profile_when_specified(self): """ Test that we ignore the user profile service if specified. """ diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 79d655c5..8d36b830 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1819,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_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".' + ) From 835949842a996ebf2dff65fc765e83582c8656de Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 26 Nov 2024 11:25:51 +0600 Subject: [PATCH 28/29] optimizely/user_profile.py -> Reverted back to variation ID retrieval logic. --- optimizely/user_profile.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index 85897041..c248b4f0 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -15,7 +15,7 @@ from typing import Any, Optional from sys import version_info from . import logger as _logging -from . import decision_service + if version_info < (3, 8): from typing_extensions import Final else: @@ -60,14 +60,7 @@ def get_variation_for_experiment(self, experiment_id: str) -> Optional[str]: Returns: Variation ID corresponding to the experiment. None if no decision available. """ - experiment_data = self.experiment_bucket_map.get(experiment_id) - - if isinstance(experiment_data, decision_service.Decision): - return experiment_data.variation.id if experiment_data.variation is not None else None - elif isinstance(experiment_data, dict): - return experiment_data.get(self.VARIATION_ID_KEY) - - return None + 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: """ Helper method to save new experiment/variation as part of the user's profile. From b825ab49905ac0b47513b06c96196c0640de0e24 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 27 Nov 2024 22:18:06 +0600 Subject: [PATCH 29/29] optimizely/user_profile.py -> Added error handling logic --- optimizely/user_profile.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/optimizely/user_profile.py b/optimizely/user_profile.py index c248b4f0..f5ded013 100644 --- a/optimizely/user_profile.py +++ b/optimizely/user_profile.py @@ -134,6 +134,8 @@ def load_user_profile(self, reasons: Optional[list[str]] = [], 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 @@ -151,3 +153,5 @@ def save_user_profile(self, error_handler: Optional[BaseErrorHandler] = None) -> 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)