diff --git a/optimizely/bucketer.py b/optimizely/bucketer.py index 1cf71b85..940a9549 100644 --- a/optimizely/bucketer.py +++ b/optimizely/bucketer.py @@ -1,4 +1,4 @@ -# Copyright 2016-2017, 2019 Optimizely +# Copyright 2016-2017, 2019-2020 Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -38,12 +38,12 @@ def __init__(self): def _generate_unsigned_hash_code_32_bit(self, bucketing_id): """ Helper method to retrieve hash code. - Args: - bucketing_id: ID for bucketing. + Args: + bucketing_id: ID for bucketing. - Returns: - Hash code which is a 32 bit unsigned integer. - """ + Returns: + Hash code which is a 32 bit unsigned integer. + """ # Adjusting MurmurHash code to be unsigned return mmh3.hash(bucketing_id, self.bucket_seed) & UNSIGNED_MAX_32_BIT_VALUE @@ -51,12 +51,12 @@ def _generate_unsigned_hash_code_32_bit(self, bucketing_id): def _generate_bucket_value(self, bucketing_id): """ Helper function to generate bucket value in half-closed interval [0, MAX_TRAFFIC_VALUE). - Args: - bucketing_id: ID for bucketing. + Args: + bucketing_id: ID for bucketing. - Returns: - Bucket value corresponding to the provided bucketing ID. - """ + Returns: + Bucket value corresponding to the provided bucketing ID. + """ ratio = float(self._generate_unsigned_hash_code_32_bit(bucketing_id)) / MAX_HASH_VALUE return math.floor(ratio * MAX_TRAFFIC_VALUE) @@ -64,15 +64,15 @@ def _generate_bucket_value(self, bucketing_id): def find_bucket(self, project_config, bucketing_id, parent_id, traffic_allocations): """ Determine entity based on bucket value and traffic allocations. - Args: - project_config: Instance of ProjectConfig. - bucketing_id: ID to be used for bucketing the user. - parent_id: ID representing group or experiment. - traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations. + Args: + project_config: Instance of ProjectConfig. + bucketing_id: ID to be used for bucketing the user. + parent_id: ID representing group or experiment. + traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations. - Returns: - Entity ID which may represent experiment or variation. - """ + Returns: + Entity ID which may represent experiment or variation. + """ bucketing_key = BUCKETING_ID_TEMPLATE.format(bucketing_id=bucketing_id, parent_id=parent_id) bucketing_number = self._generate_bucket_value(bucketing_key) @@ -90,20 +90,21 @@ def find_bucket(self, project_config, bucketing_id, parent_id, traffic_allocatio def bucket(self, project_config, experiment, user_id, bucketing_id): """ For a given experiment and bucketing ID determines variation to be shown to user. - Args: - project_config: Instance of ProjectConfig. - experiment: Object representing the experiment for which user is to be bucketed. - user_id: ID for user. - bucketing_id: ID to be used for bucketing the user. + Args: + project_config: Instance of ProjectConfig. + experiment: Object representing the experiment or rollout rule in which user is to be bucketed. + user_id: ID for user. + bucketing_id: ID to be used for bucketing the user. - Returns: - Variation in which user with ID user_id will be put in. None if no variation. - """ + Returns: + Variation in which user with ID user_id will be put in. None if no variation. + """ if not experiment: return None - # Determine if experiment is in a mutually exclusive group + # Determine if experiment is in a mutually exclusive group. + # This will not affect evaluation of rollout rules. if experiment.groupPolicy in GROUP_POLICIES: group = project_config.get_group(experiment.groupId) @@ -131,10 +132,6 @@ def bucket(self, project_config, experiment, user_id, bucketing_id): variation_id = self.find_bucket(project_config, bucketing_id, experiment.id, experiment.trafficAllocation) if variation_id: variation = project_config.get_variation_from_id(experiment.key, variation_id) - project_config.logger.info( - 'User "%s" is in variation "%s" of experiment %s.' % (user_id, variation.key, experiment.key) - ) return variation - project_config.logger.info('User "%s" is in no variation.' % user_id) return None diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 2e813747..56764d7b 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -1,4 +1,4 @@ -# Copyright 2017-2019, Optimizely +# Copyright 2017-2020, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -21,6 +21,7 @@ from .helpers import validator from .user_profile import UserProfile + Decision = namedtuple('Decision', 'experiment variation source') @@ -250,7 +251,7 @@ def get_variation(self, project_config, experiment, user_id, attributes, ignore_ try: retrieved_profile = self.user_profile_service.lookup(user_id) except: - self.logger.exception('Unable to retrieve user profile for user "%s" as lookup failed.' % user_id) + self.logger.exception('Unable to retrieve user profile for user "{}" as lookup failed.'.format(user_id)) retrieved_profile = None if validator.is_user_profile_valid(retrieved_profile): @@ -262,8 +263,13 @@ def get_variation(self, project_config, experiment, user_id, attributes, ignore_ self.logger.warning('User profile has invalid format.') # Bucket user and store the new decision - if not audience_helper.is_user_in_experiment(project_config, experiment, attributes, self.logger): - self.logger.info('User "%s" does not meet conditions to be in experiment "%s".' % (user_id, experiment.key)) + audience_conditions = experiment.get_audience_conditions_or_ids() + if not audience_helper.does_user_meet_audience_conditions(project_config, audience_conditions, + enums.ExperimentAudienceEvaluationLogs, + experiment.key, + attributes, self.logger): + self.logger.info( + 'User "{}" does not meet conditions to be in experiment "{}".'.format(user_id, experiment.key)) return None # Determine bucketing ID to be used @@ -271,15 +277,19 @@ def get_variation(self, project_config, experiment, user_id, attributes, ignore_ variation = self.bucketer.bucket(project_config, experiment, user_id, bucketing_id) if variation: + self.logger.info( + 'User "%s" is in variation "%s" of experiment %s.' % (user_id, variation.key, experiment.key) + ) # Store this new decision and return the variation for the user if not ignore_user_profile and self.user_profile_service: try: user_profile.save_variation_for_experiment(experiment.id, variation.id) self.user_profile_service.save(user_profile.__dict__) except: - self.logger.exception('Unable to save user profile for user "%s".' % user_id) + self.logger.exception('Unable to save user profile for user "{}".'.format(user_id)) return variation + self.logger.info('User "%s" is in no variation.' % user_id) return None def get_variation_for_rollout(self, project_config, rollout, user_id, attributes=None): @@ -299,44 +309,56 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes # Go through each experiment in order and try to get the variation for the user if rollout and len(rollout.experiments) > 0: for idx in range(len(rollout.experiments) - 1): - experiment = project_config.get_experiment_from_key(rollout.experiments[idx].get('key')) + logging_key = str(idx + 1) + rollout_rule = project_config.get_experiment_from_key(rollout.experiments[idx].get('key')) # Check if user meets audience conditions for targeting rule - if not audience_helper.is_user_in_experiment(project_config, experiment, attributes, self.logger): - self.logger.debug('User "%s" does not meet conditions for targeting rule %s.' % (user_id, idx + 1)) + audience_conditions = rollout_rule.get_audience_conditions_or_ids() + if not audience_helper.does_user_meet_audience_conditions(project_config, + audience_conditions, + enums.RolloutRuleAudienceEvaluationLogs, + logging_key, + attributes, + self.logger): + self.logger.debug( + 'User "{}" does not meet conditions for targeting rule {}.'.format(user_id, logging_key)) continue - self.logger.debug('User "%s" meets conditions for targeting rule %s.' % (user_id, idx + 1)) + self.logger.debug( + 'User "{}" meets audience conditions for targeting rule {}.'.format(user_id, idx + 1)) # Determine bucketing ID to be used bucketing_id = self._get_bucketing_id(user_id, attributes) - variation = self.bucketer.bucket(project_config, experiment, user_id, bucketing_id) + variation = self.bucketer.bucket(project_config, rollout_rule, user_id, bucketing_id) if variation: self.logger.debug( - 'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key) + 'User "{}" is in the traffic group of targeting rule {}.'.format(user_id, logging_key) ) - return Decision(experiment, variation, enums.DecisionSources.ROLLOUT) + return Decision(rollout_rule, variation, enums.DecisionSources.ROLLOUT) else: # Evaluate no further rules self.logger.debug( - 'User "%s" is not in the traffic group for the targeting else. ' - 'Checking "Everyone Else" rule now.' % user_id + 'User "{}" is not in the traffic group for targeting rule {}. ' + 'Checking "Everyone Else" rule now.'.format(user_id, logging_key) ) break # Evaluate last rule i.e. "Everyone Else" rule - everyone_else_experiment = project_config.get_experiment_from_key(rollout.experiments[-1].get('key')) - if audience_helper.is_user_in_experiment( + everyone_else_rule = project_config.get_experiment_from_key(rollout.experiments[-1].get('key')) + audience_conditions = everyone_else_rule.get_audience_conditions_or_ids() + if audience_helper.does_user_meet_audience_conditions( project_config, - project_config.get_experiment_from_key(rollout.experiments[-1].get('key')), + audience_conditions, + enums.RolloutRuleAudienceEvaluationLogs, + 'Everyone Else', attributes, - self.logger, + self.logger ): # Determine bucketing ID to be used bucketing_id = self._get_bucketing_id(user_id, attributes) - variation = self.bucketer.bucket(project_config, everyone_else_experiment, user_id, bucketing_id) + variation = self.bucketer.bucket(project_config, everyone_else_rule, user_id, bucketing_id) if variation: - self.logger.debug('User "%s" meets conditions for targeting rule "Everyone Else".' % user_id) - return Decision(everyone_else_experiment, variation, enums.DecisionSources.ROLLOUT,) + self.logger.debug('User "{}" meets conditions for targeting rule "Everyone Else".'.format(user_id)) + return Decision(everyone_else_rule, variation, enums.DecisionSources.ROLLOUT,) return Decision(None, None, enums.DecisionSources.ROLLOUT) @@ -392,9 +414,6 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes variation = self.get_variation(project_config, experiment, user_id, attributes) if variation: - self.logger.debug( - 'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key) - ) return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST) else: self.logger.error(enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature')) @@ -407,9 +426,6 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes variation = self.get_variation(project_config, experiment, user_id, attributes) if variation: - self.logger.debug( - 'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key) - ) return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST) # Next check if user is part of a rollout diff --git a/optimizely/helpers/audience.py b/optimizely/helpers/audience.py index 7dd82526..857d20ef 100644 --- a/optimizely/helpers/audience.py +++ b/optimizely/helpers/audience.py @@ -15,29 +15,33 @@ from . import condition as condition_helper from . import condition_tree_evaluator -from .enums import AudienceEvaluationLogs as audience_logs -def is_user_in_experiment(config, experiment, attributes, logger): +def does_user_meet_audience_conditions(config, + audience_conditions, + audience_logs, + logging_key, + attributes, + logger): """ Determine for given experiment if user satisfies the audiences for the experiment. - Args: - config: project_config.ProjectConfig object representing the project. - experiment: Object representing the experiment. - attributes: Dict representing user attributes which will be used in determining - if the audience conditions are met. If not provided, default to an empty dict. - logger: Provides a logger to send log messages to. + Args: + config: project_config.ProjectConfig object representing the project. + audience_conditions: Audience conditions corresponding to the experiment or rollout rule. + audience_logs: Log class capturing the messages to be logged . + logging_key: String representing experiment key or rollout rule. To be used in log messages only. + attributes: Dict representing user attributes which will be used in determining + if the audience conditions are met. If not provided, default to an empty dict. + logger: Provides a logger to send log messages to. - Returns: - Boolean representing if user satisfies audience conditions for any of the audiences or not. - """ - - audience_conditions = experiment.get_audience_conditions_or_ids() - logger.debug(audience_logs.EVALUATING_AUDIENCES_COMBINED.format(experiment.key, json.dumps(audience_conditions))) + Returns: + Boolean representing if user satisfies audience conditions for any of the audiences or not. + """ + logger.debug(audience_logs.EVALUATING_AUDIENCES_COMBINED.format(logging_key, json.dumps(audience_conditions))) # Return True in case there are no audiences if audience_conditions is None or audience_conditions == []: - logger.info(audience_logs.AUDIENCE_EVALUATION_RESULT_COMBINED.format(experiment.key, 'TRUE')) + logger.info(audience_logs.AUDIENCE_EVALUATION_RESULT_COMBINED.format(logging_key, 'TRUE')) return True @@ -71,5 +75,5 @@ def evaluate_audience(audience_id): eval_result = condition_tree_evaluator.evaluate(audience_conditions, evaluate_audience) eval_result = eval_result or False - logger.info(audience_logs.AUDIENCE_EVALUATION_RESULT_COMBINED.format(experiment.key, str(eval_result).upper())) + logger.info(audience_logs.AUDIENCE_EVALUATION_RESULT_COMBINED.format(logging_key, str(eval_result).upper())) return eval_result diff --git a/optimizely/helpers/condition.py b/optimizely/helpers/condition.py index 0abafb01..0676aecb 100644 --- a/optimizely/helpers/condition.py +++ b/optimizely/helpers/condition.py @@ -1,4 +1,4 @@ -# Copyright 2016, 2018-2019, Optimizely +# Copyright 2016, 2018-2020, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -17,7 +17,7 @@ from six import string_types from . import validator -from .enums import AudienceEvaluationLogs as audience_logs +from .enums import CommonAudienceEvaluationLogs as audience_logs class ConditionOperatorTypes(object): diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index ecf038d7..944c7d3f 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -1,4 +1,4 @@ -# Copyright 2016-2019, Optimizely +# Copyright 2016-2020, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -14,11 +14,9 @@ import logging -class AudienceEvaluationLogs(object): +class CommonAudienceEvaluationLogs(object): AUDIENCE_EVALUATION_RESULT = 'Audience "{}" evaluated to {}.' - AUDIENCE_EVALUATION_RESULT_COMBINED = 'Audiences for experiment "{}" collectively evaluated to {}.' EVALUATING_AUDIENCE = 'Starting to evaluate audience "{}" with conditions: {}.' - EVALUATING_AUDIENCES_COMBINED = 'Evaluating audiences for experiment "{}": {}.' INFINITE_ATTRIBUTE_VALUE = ( 'Audience condition "{}" evaluated to UNKNOWN because the number value ' 'for user attribute "{}" is not in the range [-2^53, +2^53].' @@ -48,6 +46,16 @@ class AudienceEvaluationLogs(object): ) +class ExperimentAudienceEvaluationLogs(CommonAudienceEvaluationLogs): + AUDIENCE_EVALUATION_RESULT_COMBINED = 'Audiences for experiment "{}" collectively evaluated to {}.' + EVALUATING_AUDIENCES_COMBINED = 'Evaluating audiences for experiment "{}": {}.' + + +class RolloutRuleAudienceEvaluationLogs(CommonAudienceEvaluationLogs): + AUDIENCE_EVALUATION_RESULT_COMBINED = 'Audiences for rule {} collectively evaluated to {}.' + EVALUATING_AUDIENCES_COMBINED = 'Evaluating audiences for rule {}: {}.' + + class ConfigManager(object): AUTHENTICATED_DATAFILE_URL_TEMPLATE = 'https://config.optimizely.com/datafiles/auth/{sdk_key}.json' AUTHORIZATION_HEADER_DATA_TEMPLATE = 'Bearer {access_token}' diff --git a/tests/helpers_tests/test_audience.py b/tests/helpers_tests/test_audience.py index d0d4e3dd..95311887 100644 --- a/tests/helpers_tests/test_audience.py +++ b/tests/helpers_tests/test_audience.py @@ -16,6 +16,7 @@ from optimizely import optimizely from optimizely.helpers import audience +from optimizely.helpers import enums from tests import base @@ -24,8 +25,8 @@ def setUp(self): base.BaseTest.setUp(self) self.mock_client_logger = mock.MagicMock() - def test_is_user_in_experiment__no_audience(self): - """ Test that is_user_in_experiment returns True when experiment is using no audience. """ + def test_does_user_meet_audience_conditions__no_audience(self): + """ Test that does_user_meet_audience_conditions returns True when experiment is using no audience. """ user_attributes = {} @@ -34,7 +35,14 @@ def test_is_user_in_experiment__no_audience(self): experiment.audienceIds = [] experiment.audienceConditions = [] self.assertStrictTrue( - audience.is_user_in_experiment(self.project_config, experiment, user_attributes, self.mock_client_logger,) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger + ) ) # Audience Ids exist but Audience Conditions is Empty @@ -42,7 +50,15 @@ def test_is_user_in_experiment__no_audience(self): experiment.audienceIds = ['11154'] experiment.audienceConditions = [] self.assertStrictTrue( - audience.is_user_in_experiment(self.project_config, experiment, user_attributes, self.mock_client_logger,) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger + ) + ) # Audience Ids is Empty and Audience Conditions is None @@ -50,13 +66,21 @@ def test_is_user_in_experiment__no_audience(self): experiment.audienceIds = [] experiment.audienceConditions = None self.assertStrictTrue( - audience.is_user_in_experiment(self.project_config, experiment, user_attributes, self.mock_client_logger,) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger + ) + ) - def test_is_user_in_experiment__with_audience(self): - """ Test that is_user_in_experiment evaluates non-empty audience. - Test that is_user_in_experiment uses not None audienceConditions and ignores audienceIds. - Test that is_user_in_experiment uses audienceIds when audienceConditions is None. + def test_does_user_meet_audience_conditions__with_audience(self): + """ Test that does_user_meet_audience_conditions evaluates non-empty audience. + Test that does_user_meet_audience_conditions uses not None audienceConditions and ignores audienceIds. + Test that does_user_meet_audience_conditions uses audienceIds when audienceConditions is None. """ user_attributes = {'test_attribute': 'test_value_1'} @@ -71,8 +95,13 @@ def test_is_user_in_experiment__with_audience(self): ['or', '3468206642', '3988293898'], ['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643'], ] - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) self.assertEqual(experiment.audienceConditions, cond_tree_eval.call_args[0][0]) @@ -81,45 +110,70 @@ def test_is_user_in_experiment__with_audience(self): with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate') as cond_tree_eval: experiment.audienceConditions = None - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) self.assertEqual(experiment.audienceIds, cond_tree_eval.call_args[0][0]) - def test_is_user_in_experiment__no_attributes(self): - """ Test that is_user_in_experiment evaluates audience when attributes are empty. - Test that is_user_in_experiment defaults attributes to empty dict when attributes is None. + def test_does_user_meet_audience_conditions__no_attributes(self): + """ Test that does_user_meet_audience_conditions evaluates audience when attributes are empty. + Test that does_user_meet_audience_conditions defaults attributes to empty dict when attributes is None. """ experiment = self.project_config.get_experiment_from_key('test_experiment') # attributes set to empty dict with mock.patch('optimizely.helpers.condition.CustomAttributeConditionEvaluator') as custom_attr_eval: - audience.is_user_in_experiment(self.project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + {}, + self.mock_client_logger + ) self.assertEqual({}, custom_attr_eval.call_args[0][1]) # attributes set to None with mock.patch('optimizely.helpers.condition.CustomAttributeConditionEvaluator') as custom_attr_eval: - audience.is_user_in_experiment(self.project_config, experiment, None, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + None, + self.mock_client_logger + ) self.assertEqual({}, custom_attr_eval.call_args[0][1]) - def test_is_user_in_experiment__returns_True__when_condition_tree_evaluator_returns_True(self,): - """ Test that is_user_in_experiment returns True when call to condition_tree_evaluator returns True. """ + def test_does_user_meet_audience_conditions__returns_true__when_condition_tree_evaluator_returns_true(self): + """ Test that does_user_meet_audience_conditions returns True + when call to condition_tree_evaluator returns True. """ user_attributes = {'test_attribute': 'test_value_1'} experiment = self.project_config.get_experiment_from_key('test_experiment') with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=True): self.assertStrictTrue( - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) ) - def test_is_user_in_experiment__returns_False__when_condition_tree_evaluator_returns_None_or_False(self,): - """ Test that is_user_in_experiment returns False + def test_does_user_meet_audience_conditions_returns_false_when_condition_tree_evaluator_returns_none_or_false(self): + """ Test that does_user_meet_audience_conditions returns False when call to condition_tree_evaluator returns None or False. """ user_attributes = {'test_attribute': 'test_value_1'} @@ -127,21 +181,31 @@ def test_is_user_in_experiment__returns_False__when_condition_tree_evaluator_ret with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=None): self.assertStrictFalse( - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) ) with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=False): self.assertStrictFalse( - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) ) - def test_is_user_in_experiment__evaluates_audience_ids(self): - """ Test that is_user_in_experiment correctly evaluates audience Ids and + def test_does_user_meet_audience_conditions__evaluates_audience_ids(self): + """ Test that does_user_meet_audience_conditions correctly evaluates audience Ids and calls custom attribute evaluator for leaf nodes. """ experiment = self.project_config.get_experiment_from_key('test_experiment') @@ -149,7 +213,14 @@ def test_is_user_in_experiment__evaluates_audience_ids(self): experiment.audienceConditions = None with mock.patch('optimizely.helpers.condition.CustomAttributeConditionEvaluator') as custom_attr_eval: - audience.is_user_in_experiment(self.project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + {}, + self.mock_client_logger + ) audience_11154 = self.project_config.get_audience('11154') audience_11159 = self.project_config.get_audience('11159') @@ -163,8 +234,8 @@ def test_is_user_in_experiment__evaluates_audience_ids(self): any_order=True, ) - def test_is_user_in_experiment__evaluates_audience_conditions(self): - """ Test that is_user_in_experiment correctly evaluates audienceConditions and + def test_does_user_meet_audience_conditions__evaluates_audience_conditions(self): + """ Test that does_user_meet_audience_conditions correctly evaluates audienceConditions and calls custom attribute evaluator for leaf nodes. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) @@ -178,7 +249,14 @@ def test_is_user_in_experiment__evaluates_audience_conditions(self): ] with mock.patch('optimizely.helpers.condition.CustomAttributeConditionEvaluator') as custom_attr_eval: - audience.is_user_in_experiment(project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'audience_combinations_experiment', + {}, + self.mock_client_logger + ) audience_3468206642 = project_config.get_audience('3468206642') audience_3988293898 = project_config.get_audience('3988293898') @@ -199,8 +277,8 @@ def test_is_user_in_experiment__evaluates_audience_conditions(self): any_order=True, ) - def test_is_user_in_experiment__evaluates_audience_conditions_leaf_node(self): - """ Test that is_user_in_experiment correctly evaluates leaf node in audienceConditions. """ + def test_does_user_meet_audience_conditions__evaluates_audience_conditions_leaf_node(self): + """ Test that does_user_meet_audience_conditions correctly evaluates leaf node in audienceConditions. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) project_config = opt_obj.config_manager.get_config() @@ -208,7 +286,14 @@ def test_is_user_in_experiment__evaluates_audience_conditions_leaf_node(self): experiment.audienceConditions = '3468206645' with mock.patch('optimizely.helpers.condition.CustomAttributeConditionEvaluator') as custom_attr_eval: - audience.is_user_in_experiment(project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'audience_combinations_experiment', + {}, + self.mock_client_logger + ) audience_3468206645 = project_config.get_audience('3468206645') @@ -222,17 +307,24 @@ def test_is_user_in_experiment__evaluates_audience_conditions_leaf_node(self): ) -class AudienceLoggingTest(base.BaseTest): +class ExperimentAudienceLoggingTest(base.BaseTest): def setUp(self): base.BaseTest.setUp(self) self.mock_client_logger = mock.MagicMock() - def test_is_user_in_experiment__with_no_audience(self): + def test_does_user_meet_audience_conditions__with_no_audience(self): experiment = self.project_config.get_experiment_from_key('test_experiment') experiment.audienceIds = [] experiment.audienceConditions = [] - audience.is_user_in_experiment(self.project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + {}, + self.mock_client_logger + ) self.mock_client_logger.assert_has_calls( [ @@ -241,7 +333,7 @@ def test_is_user_in_experiment__with_no_audience(self): ] ) - def test_is_user_in_experiment__evaluates_audience_ids(self): + def test_does_user_meet_audience_conditions__evaluates_audience_ids(self): user_attributes = {'test_attribute': 'test_value_1'} experiment = self.project_config.get_experiment_from_key('test_experiment') experiment.audienceIds = ['11154', '11159'] @@ -252,8 +344,13 @@ def test_is_user_in_experiment__evaluates_audience_ids(self): with mock.patch( 'optimizely.helpers.condition.CustomAttributeConditionEvaluator.evaluate', side_effect=[None, None], ): - audience.is_user_in_experiment( - self.project_config, experiment, user_attributes, self.mock_client_logger, + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', + user_attributes, + self.mock_client_logger ) self.assertEqual(5, self.mock_client_logger.debug.call_count) @@ -274,7 +371,7 @@ def test_is_user_in_experiment__evaluates_audience_ids(self): ] ) - def test_is_user_in_experiment__evaluates_audience_conditions(self): + def test_does_user_meet_audience_conditions__evaluates_audience_conditions(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) project_config = opt_obj.config_manager.get_config() experiment = project_config.get_experiment_from_key('audience_combinations_experiment') @@ -290,7 +387,14 @@ def test_is_user_in_experiment__evaluates_audience_conditions(self): with mock.patch( 'optimizely.helpers.condition.CustomAttributeConditionEvaluator.evaluate', side_effect=[False, None, True], ): - audience.is_user_in_experiment(project_config, experiment, {}, self.mock_client_logger) + audience.does_user_meet_audience_conditions( + project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'audience_combinations_experiment', + {}, + self.mock_client_logger + ) self.assertEqual(7, self.mock_client_logger.debug.call_count) self.assertEqual(1, self.mock_client_logger.info.call_count) @@ -322,3 +426,127 @@ def test_is_user_in_experiment__evaluates_audience_conditions(self): ), ] ) + + +class RolloutRuleAudienceLoggingTest(base.BaseTest): + def setUp(self): + base.BaseTest.setUp(self) + self.mock_client_logger = mock.MagicMock() + + def test_does_user_meet_audience_conditions__with_no_audience(self): + # Using experiment as rule for testing log messages + experiment = self.project_config.get_experiment_from_key('test_experiment') + experiment.audienceIds = [] + experiment.audienceConditions = [] + + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + 'test_rule', + {}, + self.mock_client_logger + ) + + self.mock_client_logger.assert_has_calls( + [ + mock.call.debug('Evaluating audiences for rule test_rule: [].'), + mock.call.info('Audiences for rule test_rule collectively evaluated to TRUE.'), + ] + ) + + def test_does_user_meet_audience_conditions__evaluates_audience_ids(self): + # Using experiment as rule for testing log messages + user_attributes = {'test_attribute': 'test_value_1'} + experiment = self.project_config.get_experiment_from_key('test_experiment') + experiment.audienceIds = ['11154', '11159'] + experiment.audienceConditions = None + audience_11154 = self.project_config.get_audience('11154') + audience_11159 = self.project_config.get_audience('11159') + + with mock.patch( + 'optimizely.helpers.condition.CustomAttributeConditionEvaluator.evaluate', side_effect=[None, None], + ): + audience.does_user_meet_audience_conditions( + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + 'test_rule', + user_attributes, + self.mock_client_logger + ) + + self.assertEqual(5, self.mock_client_logger.debug.call_count) + self.assertEqual(1, self.mock_client_logger.info.call_count) + + self.mock_client_logger.assert_has_calls( + [ + mock.call.debug('Evaluating audiences for rule test_rule: ["11154", "11159"].'), + mock.call.debug( + 'Starting to evaluate audience "11154" with conditions: ' + audience_11154.conditions + '.' + ), + mock.call.debug('Audience "11154" evaluated to UNKNOWN.'), + mock.call.debug( + 'Starting to evaluate audience "11159" with conditions: ' + audience_11159.conditions + '.' + ), + mock.call.debug('Audience "11159" evaluated to UNKNOWN.'), + mock.call.info('Audiences for rule test_rule collectively evaluated to FALSE.'), + ] + ) + + def test_does_user_meet_audience_conditions__evaluates_audience_conditions(self): + # Using experiment as rule for testing log messages + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) + project_config = opt_obj.config_manager.get_config() + experiment = project_config.get_experiment_from_key('audience_combinations_experiment') + experiment.audienceIds = [] + experiment.audienceConditions = [ + 'or', + ['or', '3468206642', '3988293898', '3988293899'], + ] + audience_3468206642 = project_config.get_audience('3468206642') + audience_3988293898 = project_config.get_audience('3988293898') + audience_3988293899 = project_config.get_audience('3988293899') + + with mock.patch( + 'optimizely.helpers.condition.CustomAttributeConditionEvaluator.evaluate', side_effect=[False, None, True], + ): + audience.does_user_meet_audience_conditions( + project_config, + experiment.get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + 'test_rule', + {}, + self.mock_client_logger + ) + + self.assertEqual(7, self.mock_client_logger.debug.call_count) + self.assertEqual(1, self.mock_client_logger.info.call_count) + + self.mock_client_logger.assert_has_calls( + [ + mock.call.debug( + 'Evaluating audiences for rule ' + 'test_rule: ["or", ["or", "3468206642", ' + '"3988293898", "3988293899"]].' + ), + mock.call.debug( + 'Starting to evaluate audience "3468206642" with ' + 'conditions: ' + audience_3468206642.conditions + '.' + ), + mock.call.debug('Audience "3468206642" evaluated to FALSE.'), + mock.call.debug( + 'Starting to evaluate audience "3988293898" with ' + 'conditions: ' + audience_3988293898.conditions + '.' + ), + mock.call.debug('Audience "3988293898" evaluated to UNKNOWN.'), + mock.call.debug( + 'Starting to evaluate audience "3988293899" with ' + 'conditions: ' + audience_3988293899.conditions + '.' + ), + mock.call.debug('Audience "3988293899" evaluated to TRUE.'), + mock.call.info( + 'Audiences for rule test_rule collectively evaluated to TRUE.' + ), + ] + ) diff --git a/tests/test_bucketing.py b/tests/test_bucketing.py index 783c23e2..f0268b66 100644 --- a/tests/test_bucketing.py +++ b/tests/test_bucketing.py @@ -1,4 +1,4 @@ -# Copyright 2016-2019, Optimizely +# Copyright 2016-2020, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -234,9 +234,6 @@ def test_bucket(self): ) mock_config_logging.debug.assert_called_once_with('Assigned bucket 42 to user with bucketing ID "test_user".') - mock_config_logging.info.assert_called_once_with( - 'User "test_user" is in variation "control" of experiment test_experiment.' - ) # Empty entity ID with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4242), mock.patch.object( @@ -252,7 +249,6 @@ def test_bucket(self): ) mock_config_logging.debug.assert_called_once_with('Assigned bucket 4242 to user with bucketing ID "test_user".') - mock_config_logging.info.assert_called_once_with('User "test_user" is in no variation.') # Variation 2 with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=5042), mock.patch.object( @@ -269,9 +265,6 @@ def test_bucket(self): ) mock_config_logging.debug.assert_called_once_with('Assigned bucket 5042 to user with bucketing ID "test_user".') - mock_config_logging.info.assert_called_once_with( - 'User "test_user" is in variation "variation" of experiment test_experiment.' - ) # No matching variation with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=424242), mock.patch.object( @@ -289,7 +282,6 @@ def test_bucket(self): mock_config_logging.debug.assert_called_once_with( 'Assigned bucket 424242 to user with bucketing ID "test_user".' ) - mock_config_logging.info.assert_called_once_with('User "test_user" is in no variation.') def test_bucket__experiment_in_group(self): """ Test that for provided bucket values correct variation ID is returned. """ @@ -316,7 +308,6 @@ def test_bucket__experiment_in_group(self): mock_config_logging.info.assert_has_calls( [ mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'), - mock.call('User "test_user" is in variation "group_exp_1_variation" of experiment group_exp_1.'), ] ) @@ -356,7 +347,6 @@ def test_bucket__experiment_in_group(self): mock_config_logging.info.assert_has_calls( [ mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'), - mock.call('User "test_user" is in no variation.'), ] ) @@ -399,6 +389,5 @@ def test_bucket__experiment_in_group(self): mock_config_logging.info.assert_has_calls( [ mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'), - mock.call('User "test_user" is in no variation.'), ] ) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 0812368a..6875a1c0 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1,4 +1,4 @@ -# Copyright 2017-2019, Optimizely +# Copyright 2017-2020, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -420,7 +420,7 @@ def test_get_variation__experiment_not_running(self): ) as mock_decision_service_logging, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment" + "optimizely.helpers.audience.does_user_meet_audience_conditions" ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket, mock.patch( @@ -456,7 +456,7 @@ def test_get_variation__bucketing_id_provided(self): "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None, ), mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=True ), mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket: @@ -485,7 +485,7 @@ def test_get_variation__user_whitelisted_for_variation(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment" + "optimizely.helpers.audience.does_user_meet_audience_conditions" ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket, mock.patch( @@ -521,7 +521,7 @@ def test_get_variation__user_has_stored_decision(self): "optimizely.decision_service.DecisionService.get_stored_variation", return_value=entities.Variation("111128", "control"), ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment" + "optimizely.helpers.audience.does_user_meet_audience_conditions" ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket, mock.patch( @@ -572,7 +572,7 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None, ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -596,7 +596,12 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_a 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, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) mock_bucket.assert_called_once_with( self.project_config, experiment, "test_user", "test_user" @@ -626,7 +631,7 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_n ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -649,7 +654,12 @@ def test_get_variation__user_bucketed_for_new_experiment__user_profile_service_n self.assertEqual(0, mock_lookup.call_count) self.assertEqual(0, mock_get_stored_variation.call_count) mock_audience_check.assert_called_once_with( - self.project_config, experiment, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) mock_bucket.assert_called_once_with( self.project_config, experiment, "test_user", "test_user" @@ -669,7 +679,7 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): "optimizely.decision_service.DecisionService.get_stored_variation", return_value=None, ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=False + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=False ) as mock_audience_check, mock.patch( "optimizely.bucketer.Bucketer.bucket" ) as mock_bucket, mock.patch( @@ -693,7 +703,12 @@ def test_get_variation__user_does_not_meet_audience_conditions(self): self.project_config, experiment, user_profile.UserProfile("test_user") ) mock_audience_check.assert_called_once_with( - self.project_config, experiment, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) self.assertEqual(0, mock_bucket.call_count) self.assertEqual(0, mock_save.call_count) @@ -710,7 +725,7 @@ def test_get_variation__user_profile_in_invalid_format(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -735,7 +750,12 @@ def test_get_variation__user_profile_in_invalid_format(self): # Stored decision is not consulted as user profile is invalid self.assertEqual(0, mock_get_stored_variation.call_count) mock_audience_check.assert_called_once_with( - self.project_config, experiment, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) mock_decision_service_logging.warning.assert_called_once_with( "User profile has invalid format." @@ -762,7 +782,7 @@ def test_get_variation__user_profile_lookup_fails(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -787,7 +807,12 @@ def test_get_variation__user_profile_lookup_fails(self): # 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, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + 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.' @@ -814,7 +839,7 @@ def test_get_variation__user_profile_save_fails(self): ) as mock_get_whitelisted_variation, mock.patch( "optimizely.decision_service.DecisionService.get_stored_variation" ) as mock_get_stored_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -838,7 +863,12 @@ def test_get_variation__user_profile_save_fails(self): 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, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) mock_decision_service_logging.exception.assert_called_once_with( 'Unable to save user profile for user "test_user".' @@ -863,7 +893,7 @@ def test_get_variation__ignore_user_profile_when_specified(self): "optimizely.decision_service.DecisionService.get_whitelisted_variation", return_value=None, ) as mock_get_whitelisted_variation, mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "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"), @@ -888,7 +918,12 @@ def test_get_variation__ignore_user_profile_when_specified(self): self.project_config, experiment, "test_user" ) mock_audience_check.assert_called_once_with( - self.project_config, experiment, None, mock_decision_service_logging + self.project_config, + experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "test_experiment", + None, + mock_decision_service_logging ) mock_bucket.assert_called_once_with( self.project_config, experiment, "test_user", "test_user" @@ -928,7 +963,7 @@ def test_get_variation_for_rollout__returns_decision_if_user_in_rollout(self): rollout = self.project_config.get_rollout_from_id("211111") with mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=True ), self.mock_decision_logger as mock_decision_service_logging, mock.patch( "optimizely.bucketer.Bucketer.bucket", return_value=self.project_config.get_variation_from_id("211127", "211129"), @@ -945,13 +980,8 @@ def test_get_variation_for_rollout__returns_decision_if_user_in_rollout(self): ) # Check all log messages - mock_decision_service_logging.debug.assert_has_calls( - [ - mock.call('User "test_user" meets conditions for targeting rule 1.'), - mock.call( - 'User "test_user" is in variation 211129 of experiment 211127.' - ), - ] + mock_decision_service_logging.debug.assert_has_calls([ + mock.call('User "test_user" meets audience conditions for targeting rule 1.')] ) # Check that bucket is called with correct parameters @@ -968,7 +998,7 @@ def test_get_variation_for_rollout__calls_bucket_with_bucketing_id(self): rollout = self.project_config.get_rollout_from_id("211111") with mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=True ), self.mock_decision_logger as mock_decision_service_logging, mock.patch( "optimizely.bucketer.Bucketer.bucket", return_value=self.project_config.get_variation_from_id("211127", "211129"), @@ -989,12 +1019,7 @@ def test_get_variation_for_rollout__calls_bucket_with_bucketing_id(self): # Check all log messages mock_decision_service_logging.debug.assert_has_calls( - [ - mock.call('User "test_user" meets conditions for targeting rule 1.'), - mock.call( - 'User "test_user" is in variation 211129 of experiment 211127.' - ), - ] + [mock.call('User "test_user" meets audience conditions for targeting rule 1.')] ) # Check that bucket is called with correct parameters mock_bucket.assert_called_once_with( @@ -1015,7 +1040,7 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self): ) with mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=True + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=True ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch( "optimizely.bucketer.Bucketer.bucket", side_effect=[None, variation_to_mock] ): @@ -1033,13 +1058,17 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self): [ mock.call( self.project_config, - self.project_config.get_experiment_from_key("211127"), + self.project_config.get_experiment_from_key("211127").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + '1', None, mock_decision_service_logging, ), mock.call( self.project_config, - self.project_config.get_experiment_from_key("211147"), + self.project_config.get_experiment_from_key("211147").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + 'Everyone Else', None, mock_decision_service_logging, ), @@ -1050,9 +1079,9 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self): # Check all log messages mock_decision_service_logging.debug.assert_has_calls( [ - mock.call('User "test_user" meets conditions for targeting rule 1.'), + mock.call('User "test_user" meets audience conditions for targeting rule 1.'), mock.call( - 'User "test_user" is not in the traffic group for the targeting else. ' + 'User "test_user" is not in the traffic group for targeting rule 1. ' 'Checking "Everyone Else" rule now.' ), mock.call( @@ -1067,7 +1096,7 @@ def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self): rollout = self.project_config.get_rollout_from_id("211111") with mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", return_value=False + "optimizely.helpers.audience.does_user_meet_audience_conditions", return_value=False ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging: self.assertEqual( decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT), @@ -1081,19 +1110,25 @@ def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self): [ mock.call( self.project_config, - self.project_config.get_experiment_from_key("211127"), + self.project_config.get_experiment_from_key("211127").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + "1", None, mock_decision_service_logging, ), mock.call( self.project_config, - self.project_config.get_experiment_from_key("211137"), + self.project_config.get_experiment_from_key("211137").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + "2", None, mock_decision_service_logging, ), mock.call( self.project_config, - self.project_config.get_experiment_from_key("211147"), + self.project_config.get_experiment_from_key("211147").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + "Everyone Else", None, mock_decision_service_logging, ), @@ -1131,7 +1166,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment( "optimizely.decision_service.DecisionService.get_variation", return_value=expected_variation, ) - with decision_patch as mock_decision, self.mock_decision_logger as mock_decision_service_logging: + with decision_patch as mock_decision, self.mock_decision_logger: self.assertEqual( decision_service.Decision( expected_experiment, @@ -1150,11 +1185,6 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment( None, ) - # Check log message - mock_decision_service_logging.debug.assert_called_once_with( - 'User "test_user" is in variation variation of experiment test_experiment.' - ) - def test_get_variation_for_feature__returns_variation_for_feature_in_rollout(self): """ Test that get_variation_for_feature returns the variation of the experiment in the rollout that the user is bucketed into. """ @@ -1202,7 +1232,7 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ "211127", "211129" ) with mock.patch( - "optimizely.helpers.audience.is_user_in_experiment", + "optimizely.helpers.audience.does_user_meet_audience_conditions", side_effect=[False, True], ) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch( "optimizely.bucketer.Bucketer.bucket", return_value=expected_variation @@ -1221,13 +1251,17 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_ self.assertEqual(2, mock_audience_check.call_count) mock_audience_check.assert_any_call( self.project_config, - self.project_config.get_experiment_from_key("group_exp_2"), + self.project_config.get_experiment_from_key("group_exp_2").get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + "group_exp_2", None, mock_decision_service_logging, ) mock_audience_check.assert_any_call( self.project_config, - self.project_config.get_experiment_from_key("211127"), + self.project_config.get_experiment_from_key("211127").get_audience_conditions_or_ids(), + enums.RolloutRuleAudienceEvaluationLogs, + "1", None, mock_decision_service_logging, ) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f3f8863c..194ae77e 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1028,13 +1028,17 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): def test_activate__with_attributes__no_audience_match(self): """ Test that activate returns None when audience conditions do not match. """ - with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check: + with mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', + return_value=False) as mock_audience_check: self.assertIsNone( self.optimizely.activate('test_experiment', 'test_user', attributes={'test_attribute': 'test_value'},) ) + expected_experiment = self.project_config.get_experiment_from_key('test_experiment') mock_audience_check.assert_called_once_with( self.project_config, - self.project_config.get_experiment_from_key('test_experiment'), + expected_experiment.get_audience_conditions_or_ids(), + enums.ExperimentAudienceEvaluationLogs, + 'test_experiment', {'test_attribute': 'test_value'}, self.optimizely.logger, ) @@ -1054,7 +1058,7 @@ def test_activate__experiment_not_running(self): """ Test that activate returns None and does not process event when experiment is not Running. """ with mock.patch( - 'optimizely.helpers.audience.is_user_in_experiment', return_value=True + 'optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=True ) as mock_audience_check, mock.patch( 'optimizely.helpers.experiment.is_experiment_running', return_value=False ) as mock_is_experiment_running, mock.patch( @@ -1077,7 +1081,7 @@ def test_activate__whitelisting_overrides_audience_check(self): """ Test that during activate whitelist overrides audience check if user is in the whitelist. """ with mock.patch( - 'optimizely.helpers.audience.is_user_in_experiment', return_value=False + 'optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=False ) as mock_audience_check, mock.patch( 'optimizely.helpers.experiment.is_experiment_running', return_value=True ) as mock_is_experiment_running: @@ -1090,9 +1094,11 @@ def test_activate__whitelisting_overrides_audience_check(self): def test_activate__bucketer_returns_none(self): """ Test that activate returns None and does not process event when user is in no variation. """ - with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True), mock.patch( - 'optimizely.bucketer.Bucketer.bucket', return_value=None - ) as mock_bucket, mock.patch( + with mock.patch( + 'optimizely.helpers.audience.does_user_meet_audience_conditions', + return_value=True), mock.patch( + 'optimizely.bucketer.Bucketer.bucket', + return_value=None) as mock_bucket, mock.patch( 'optimizely.event.event_processor.ForwardingEventProcessor.process' ) as mock_process: self.assertIsNone(