From 2e4f867d64db9e93805379053809e3284ea15b50 Mon Sep 17 00:00:00 2001 From: shujat333 Date: Fri, 9 Jul 2021 19:13:09 +0500 Subject: [PATCH] wrong variation issue fix --- optimizely/bucketer.py | 2 +- optimizely/decision_service.py | 5 +- optimizely/project_config.py | 37 ++- tests/base.py | 411 +++++++++++++++++++++++++++++++- tests/test_sdks_updated_maps.py | 67 ++++++ 5 files changed, 512 insertions(+), 10 deletions(-) create mode 100644 tests/test_sdks_updated_maps.py diff --git a/optimizely/bucketer.py b/optimizely/bucketer.py index ca5e0f28..eb5d091e 100644 --- a/optimizely/bucketer.py +++ b/optimizely/bucketer.py @@ -142,7 +142,7 @@ 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) + variation = project_config.get_variation_from_id(experiment.id, variation_id) return variation, decide_reasons else: diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 98060e8e..1fd26037 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -21,7 +21,6 @@ from .helpers import validator from .user_profile import UserProfile - Decision = namedtuple('Decision', 'experiment variation source') @@ -342,8 +341,8 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes if rollout and len(rollout.experiments) > 0: for idx in range(len(rollout.experiments) - 1): logging_key = str(idx + 1) - rollout_rule = project_config.get_experiment_from_key(rollout.experiments[idx].get('key')) - + rollout_rule = project_config.get_experiment_from_id(rollout.experiments[idx].get('id')) + # rollout_rule = optimizely.entities.Experiment(**rollout.experiments[idx]) # Check if user meets audience conditions for targeting rule audience_conditions = rollout_rule.get_audience_conditions_or_ids() user_meets_audience_conditions, reasons_received = audience_helper.does_user_meet_audience_conditions( diff --git a/optimizely/project_config.py b/optimizely/project_config.py index ac97fac6..66323000 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -17,6 +17,7 @@ from .helpers import enums from . import entities from . import exceptions +import copy SUPPORTED_VERSIONS = [ enums.DatafileVersions.V2, @@ -56,6 +57,7 @@ def __init__(self, datafile, logger, error_handler): self.environment_key = config.get('environmentKey', None) self.groups = config.get('groups', []) self.experiments = config.get('experiments', []) + self.mapexperiment = copy.deepcopy(self.experiments) self.events = config.get('events', []) self.attributes = config.get('attributes', []) self.audiences = config.get('audiences', []) @@ -68,6 +70,7 @@ def __init__(self, datafile, logger, error_handler): # Utility maps for quick lookup self.group_id_map = self._generate_key_map(self.groups, 'id', entities.Group) + self.experimentID_map = self._generate_key_map(self.mapexperiment, 'id', entities.Experiment) self.experiment_key_map = self._generate_key_map(self.experiments, 'key', entities.Experiment) self.event_key_map = self._generate_key_map(self.events, 'key', entities.Event) self.attribute_key_map = self._generate_key_map(self.attributes, 'key', entities.Attribute) @@ -85,6 +88,7 @@ def __init__(self, datafile, logger, error_handler): for layer in self.rollout_id_map.values(): for experiment in layer.experiments: self.experiment_key_map[experiment['key']] = entities.Experiment(**experiment) + self.experimentID_map[experiment['id']] = entities.Experiment(**experiment) self.audience_id_map = self._deserialize_audience(self.audience_id_map) for group in self.group_id_map.values(): @@ -92,11 +96,16 @@ def __init__(self, datafile, logger, error_handler): for experiment in experiments_in_group_key_map.values(): experiment.__dict__.update({'groupId': group.id, 'groupPolicy': group.policy}) self.experiment_key_map.update(experiments_in_group_key_map) + experiments_in_group_id_map = self._generate_key_map(group.experiments, 'id', entities.Experiment) + for experiment in experiments_in_group_id_map.values(): + experiment.__dict__.update({'groupId': group.id, 'groupPolicy': group.policy}) + self.experiment_key_map.update(experiments_in_group_id_map) self.experiment_id_map = {} self.variation_key_map = {} self.variation_id_map = {} self.variation_variable_usage_map = {} + for experiment in self.experiment_key_map.values(): self.experiment_id_map[experiment.id] = experiment self.variation_key_map[experiment.key] = self._generate_key_map( @@ -108,6 +117,26 @@ def __init__(self, datafile, logger, error_handler): self.variation_variable_usage_map[variation.id] = self._generate_key_map( variation.variables, 'id', entities.Variation.VariableUsage ) + self.correct_variation_id_map = {} + self.temp_variation_id_map = {} + for experiment in self.experimentID_map.values(): + self.temp_variation_id_map[experiment.id] = self._generate_key_map( + experiment.variations, 'id', entities.Variation + ) + self.correct_variation_id_map[experiment.id] = {} + for variation in self.temp_variation_id_map.get(experiment.id).values(): + self.correct_variation_id_map[experiment.id][variation.id] = variation + self.variation_variable_usage_map[variation.id] = self._generate_key_map( + variation.variables, 'id', entities.Variation.VariableUsage + ) + + + + + + + + self.feature_key_map = self._generate_key_map(self.feature_flags, 'key', entities.FeatureFlag) @@ -280,8 +309,8 @@ def get_experiment_from_id(self, experiment_id): Experiment corresponding to the provided experiment ID. """ - experiment = self.experiment_id_map.get(experiment_id) - + # experiment = self.experiment_id_map.get(experiment_id) + experiment = self.experimentID_map.get(experiment_id) if experiment: return experiment @@ -362,8 +391,8 @@ def get_variation_from_id(self, experiment_key, variation_id): Object representing the variation. """ - variation_map = self.variation_id_map.get(experiment_key) - + #variation_map = self.variation_id_map.get(experiment_key) + variation_map = self.correct_variation_id_map.get(experiment_key) if variation_map: variation = variation_map.get(variation_id) if variation: diff --git a/tests/base.py b/tests/base.py index 48b89106..94a5f0d4 100644 --- a/tests/base.py +++ b/tests/base.py @@ -155,7 +155,7 @@ def setUp(self, config_dict='config_dict'): 'id': '111128', 'featureEnabled': False, 'variables': [ - {'id': '127', 'value': 'false'}, + {'id': '127', 'value': 'False'}, {'id': '128', 'value': 'prod'}, {'id': '129', 'value': '10.01'}, {'id': '130', 'value': '4242'}, @@ -408,6 +408,7 @@ def setUp(self, config_dict='config_dict'): ], 'rollouts': [ {'id': '201111', 'experiments': []}, + { 'id': '211111', 'experiments': [ @@ -492,7 +493,7 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': [], 'rolloutId': '211111', 'variables': [ - {'id': '132', 'key': 'is_running', 'defaultValue': 'false', 'type': 'boolean'}, + {'id': '132', 'key': 'is_running', 'defaultValue': 'False', 'type': 'boolean'}, {'id': '133', 'key': 'message', 'defaultValue': 'Hello', 'type': 'string'}, {'id': '134', 'key': 'price', 'defaultValue': '99.99', 'type': 'double'}, {'id': '135', 'key': 'count', 'defaultValue': '999', 'type': 'integer'}, @@ -1055,6 +1056,412 @@ def setUp(self, config_dict='config_dict'): 'revision': '3', } + self.config_same_exp_key = { + "version": "4", + "rollouts": [ + { + "experiments": [ + { + "status": "Running", + "audienceConditions": [ + "or", + "20406066925" + ], + "audienceIds": [ + "20406066925" + ], + "variations": [ + { + "variables": [], + "id": "8048", + "key": "variation2", + "featureEnabled": True + } + ], + "forcedVariations": {}, + "key": "targeted_delivery", + "layerId": "9300000007573", + "trafficAllocation": [ + { + "entityId": "8048", + "endOfRange": 10000 + } + ], + "id": "9300000007573" + }, + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "8046", + "key": "off", + "featureEnabled": False + } + ], + "forcedVariations": {}, + "key": "default-rollout-3046-20390585493", + "layerId": "default-layer-rollout-3046-20390585493", + "trafficAllocation": [ + { + "entityId": "8046", + "endOfRange": 10000 + } + ], + "id": "default-rollout-3046-20390585493" + } + ], + "id": "rollout-3046-20390585493" + }, + { + "experiments": [ + { + "status": "Running", + "audienceConditions": [ + "or", + "20415611520" + ], + "audienceIds": [ + "20415611520" + ], + "variations": [ + { + "variables": [], + "id": "8045", + "key": "variation1", + "featureEnabled": True + } + ], + "forcedVariations": {}, + "key": "targeted_delivery", + "layerId": "9300000007569", + "trafficAllocation": [ + { + "entityId": "8045", + "endOfRange": 10000 + } + ], + "id": "9300000007569" + }, + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "8043", + "key": "off", + "featureEnabled": False + } + ], + "forcedVariations": {}, + "key": "default-rollout-3045-20390585493", + "layerId": "default-layer-rollout-3045-20390585493", + "trafficAllocation": [ + { + "entityId": "8043", + "endOfRange": 10000 + } + ], + "id": "default-rollout-3045-20390585493" + } + ], + "id": "rollout-3045-20390585493" + } + ], + "typedAudiences": [ + { + "id": "20415611520", + "conditions": [ + "and", + [ + "or", + [ + "or", + { + "value": True, + "type": "custom_attribute", + "name": "hiddenLiveEnabled", + "match": "exact" + } + ] + ] + ], + "name": "polina-test1" + }, + { + "id": "20406066925", + "conditions": [ + "and", + [ + "or", + [ + "or", + { + "value": False, + "type": "custom_attribute", + "name": "hiddenLiveEnabled", + "match": "exact" + } + ] + ] + ], + "name": "polina-test2" + } + ], + "anonymizeIP": True, + "projectId": "20430981610", + "variables": [], + "featureFlags": [ + { + "experimentIds": [], + "rolloutId": "rollout-3046-20390585493", + "variables": [], + "id": "3046", + "key": "flag2" + }, + { + "experimentIds": [], + "rolloutId": "rollout-3045-20390585493", + "variables": [], + "id": "3045", + "key": "flag1" + } + ], + "experiments": [], + "audiences": [ + { + "id": "20415611520", + "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]", + "name": "polina-test1" + }, + { + "id": "20406066925", + "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]", + "name": "polina-test2" + }, + { + "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]", + "id": "$opt_dummy_audience", + "name": "Optimizely-Generated Audience for Backwards Compatibility" + } + ], + "groups": [], + "attributes": [ + { + "id": "20408641883", + "key": "hiddenLiveEnabled" + } + ], + "botFiltering": "False", + "accountId": "17882702980", + "events": [], + "revision": "25" + } + self.config_test_cases_key = { + "version": "4", + "rollouts": [ + { + "experiments": [ + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5452", + "key": "on", + "featureEnabled": True + } + ], + "forcedVariations": {}, + "key": "targeted_delivery", + "layerId": "9300000004981", + "trafficAllocation": [ + { + "entityId": "5452", + "endOfRange": 10000 + } + ], + "id": "9300000004981" + }, + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5451", + "key": "off", + "featureEnabled": False + } + ], + "forcedVariations": {}, + "key": "default-rollout-2029-20301771717", + "layerId": "default-layer-rollout-2029-20301771717", + "trafficAllocation": [ + { + "entityId": "5451", + "endOfRange": 10000 + } + ], + "id": "default-rollout-2029-20301771717" + } + ], + "id": "rollout-2029-20301771717" + }, + { + "experiments": [ + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5450", + "key": "on", + "featureEnabled": True + } + ], + "forcedVariations": {}, + "key": "targeted_delivery", + "layerId": "9300000004979", + "trafficAllocation": [ + { + "entityId": "5450", + "endOfRange": 10000 + } + ], + "id": "9300000004979" + }, + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5449", + "key": "off", + "featureEnabled": False + } + ], + "forcedVariations": {}, + "key": "default-rollout-2028-20301771717", + "layerId": "default-layer-rollout-2028-20301771717", + "trafficAllocation": [ + { + "entityId": "5449", + "endOfRange": 10000 + } + ], + "id": "default-rollout-2028-20301771717" + } + ], + "id": "rollout-2028-20301771717" + }, + { + "experiments": [ + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5448", + "key": "on", + "featureEnabled": True + } + ], + "forcedVariations": {}, + "key": "targeted_delivery", + "layerId": "9300000004977", + "trafficAllocation": [ + { + "entityId": "5448", + "endOfRange": 10000 + } + ], + "id": "9300000004977" + }, + { + "status": "Running", + "audienceConditions": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "id": "5447", + "key": "off", + "featureEnabled": False + } + ], + "forcedVariations": {}, + "key": "default-rollout-2027-20301771717", + "layerId": "default-layer-rollout-2027-20301771717", + "trafficAllocation": [ + { + "entityId": "5447", + "endOfRange": 10000 + } + ], + "id": "default-rollout-2027-20301771717" + } + ], + "id": "rollout-2027-20301771717" + } + ], + "typedAudiences": [], + "anonymizeIP": True, + "projectId": "20286295225", + "variables": [], + "featureFlags": [ + { + "experimentIds": [], + "rolloutId": "rollout-2029-20301771717", + "variables": [], + "id": "2029", + "key": "flag_3" + }, + { + "experimentIds": [], + "rolloutId": "rollout-2028-20301771717", + "variables": [], + "id": "2028", + "key": "flag_2" + }, + { + "experimentIds": [], + "rolloutId": "rollout-2027-20301771717", + "variables": [], + "id": "2027", + "key": "flag_1" + } + ], + "experiments": [], + "audiences": [ + { + "conditions": "[\"or\", {\"match\": \"exact\", \"name\": \"$opt_dummy_attribute\", \"type\": \"custom_attribute\", \"value\": \"$opt_dummy_value\"}]", + "id": "$opt_dummy_audience", + "name": "Optimizely-Generated Audience for Backwards Compatibility" + } + ], + "groups": [], + "attributes": [], + "botFiltering": False, + "accountId": "19947277778", + "events": [], + "revision": "11" + } + + config = getattr(self, config_dict) self.optimizely = optimizely.Optimizely(json.dumps(config)) self.project_config = self.optimizely.config_manager.get_config() diff --git a/tests/test_sdks_updated_maps.py b/tests/test_sdks_updated_maps.py new file mode 100644 index 00000000..ccabb873 --- /dev/null +++ b/tests/test_sdks_updated_maps.py @@ -0,0 +1,67 @@ +import json + +import mock + +from optimizely.decision.optimizely_decision import OptimizelyDecision +from optimizely.decision.optimizely_decide_option import OptimizelyDecideOption as DecideOption +from optimizely.helpers import enums +from . import base +from optimizely import optimizely, decision_service +from optimizely.optimizely_user_context import OptimizelyUserContext +from optimizely.user_profile import UserProfileService + + +class SampleSdkTests(base.BaseTest): + def setUp(self): + base.BaseTest.setUp(self, 'config_test_cases_key') + + def test_decide__flag1(self): + user_context = self.optimizely.create_user_context("test_user_1") + + expected = OptimizelyDecision( + variation_key='on', + rule_key='targeted_delivery', + enabled=True, + variables={}, + flag_key='flag_3', + user_context=user_context + ) + + actual = user_context.decide('flag_3') + print(expected.user_context.user_id, actual.user_context.user_id) + self.assertEqual(actual.variation_key, expected.variation_key) + self.assertEqual(actual.rule_key, expected.rule_key) + + def test_decide__flag2(self): + user_context = self.optimizely.create_user_context("test_user_1") + expected = OptimizelyDecision( + variation_key='on', + rule_key='targeted_delivery', + enabled=True, + variables={}, + flag_key='flag_2', + user_context=user_context + ) + + actual = user_context.decide('flag_2') + print(expected.user_context.user_id, actual.user_context.user_id) + self.assertEqual(actual.variation_key, expected.variation_key) + self.assertEqual(actual.rule_key, expected.rule_key) + + def test_decide__flag3(self): + user_context = self.optimizely.create_user_context("test_user_1") + expected = OptimizelyDecision( + variation_key='on', + rule_key='targeted_delivery', + enabled=True, + variables={}, + flag_key='flag_1', + user_context=user_context + ) + + actual = user_context.decide('flag_1') + print(expected.user_context.user_id, actual.user_context.user_id) + self.assertEqual(actual.variation_key, expected.variation_key) + self.assertEqual(actual.rule_key, expected.rule_key) + +