From e80fdfbb474f35293973815a1e805979438a07a7 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 10 Jun 2021 18:55:07 +0500 Subject: [PATCH 01/59] added sdk and environment key --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index f204263a..798cf60b 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -28,6 +28,7 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.attributes = attributes or [] self.events = events or [] + def get_datafile(self): """ Get the datafile associated with OptimizelyConfig. @@ -51,7 +52,7 @@ def get_environment_key(self): A string containing environment key. """ return self.environment_key - + def get_attributes(self): """ Get the attributes associated with OptimizelyConfig @@ -68,7 +69,6 @@ def get_events(self): """ return self.events - class OptimizelyExperiment(object): def __init__(self, id, key, variations_map): self.id = id From d58b71a7e77d47b4d38ec3b27adf2fe226864ca4 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 3 Jun 2021 09:45:56 -0400 Subject: [PATCH 02/59] [MAINTENANCE] Remove Deprecated warnings during build - assertRaisesRegexp -> assertRaisesRegex - assertEquals -> assertEqual - isAlive() -> is_alive() - Check added to base.py to confirm attribute assertRaisesRegex for backwards compatibility to Python2.7 --- tests/test_user_context.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_user_context.py b/tests/test_user_context.py index fcffc415..9f4ab73a 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1296,3 +1296,4 @@ def test_decide_experiment(self): user_context = opt_obj.create_user_context('test_user') decision = user_context.decide('test_feature_in_experiment', [DecideOption.DISABLE_DECISION_EVENT]) self.assertTrue(decision.enabled, "decision should be enabled") + From a583ea5a39651e2c9f4f77a00de1733f176a1a49 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 15 Jun 2021 12:52:58 -0400 Subject: [PATCH 03/59] [OASIS-7757] Fix spelling of environment to fix testcases from failing --- optimizely/optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 798cf60b..f9649356 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -28,7 +28,6 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.attributes = attributes or [] self.events = events or [] - def get_datafile(self): """ Get the datafile associated with OptimizelyConfig. From 5b2196c2233fb879d77cf6d6d1cbe39c426dffbd Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 17 Jun 2021 14:26:53 -0400 Subject: [PATCH 04/59] [OASIS-7757] - Added additional test cases to test_optimizely and test_user_context. --- tests/test_optimizely.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 23454342..fca37d47 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -265,6 +265,19 @@ def test_init__sdk_key_and_datafile_access_token(self): self.assertIs(type(opt_obj.config_manager), config_manager.AuthDatafilePollingConfigManager) + def test_init__invalid_default_decide_options(self): + """ + Test to confirm that default decide options passed not as a list will trigger setting + self.deafulat_decide_options as an empty list. + """ + invalid_decide_options = {"testKey": "testOption"} + + mock_client_logger = mock.MagicMock() + with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): + opt_obj = optimizely.Optimizely(default_decide_options=invalid_decide_options) + + self.assertEqual(opt_obj.default_decide_options, []) + def test_invalid_json_raises_schema_validation_off(self): """ Test that invalid JSON logs error if schema validation is turned off. """ From f964f947262522f6706db0dcd7e6016bf3fde14f Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 22 Jun 2021 09:30:57 -0400 Subject: [PATCH 05/59] [OASIS-7800] Updated optimizely_config with attributes and events --- optimizely/optimizely_config.py | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index f9649356..2d7b74b6 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -13,6 +13,8 @@ import copy +from optimizely.helpers.validator import are_attributes_valid + from .project_config import ProjectConfig @@ -294,3 +296,37 @@ def _get_features_map(self, experiments_id_map): features_map[feature['key']] = optly_feature return features_map + + def get_attributes_map(self): + """ Gets attributes map for the project config. + + Returns: + dict -- Attribute key, OptimizelyAttribute map + """ + + attributes_map = {} + + for attribute in self.attributes: + optly_attribute = OptimizelyAttribute( + attribute['id'], attribute['key'] + ) + attributes_map[attribute['key']] = optly_attribute + + return attributes_map + + def get_events_map(self): + """ Gets attributes map for the project config. + + Returns: + dict -- Event key, OptimizelyEvent map + """ + + events_map = {} + + for event in self.events: + optly_event = OptimizelyEvent( + event['id'], event['key'], event.get('experimentIds', []) + ) + events_map[event['key']] = optly_event + + return events_map From f60ab0af5a8d71b082de44149953d835791e9509 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 23 Jun 2021 08:25:58 -0400 Subject: [PATCH 06/59] [OASIS-7757] - update copyright years and add more testcases for sdk_key and environment_key. Move decide tests to test_user_context from test_optimizely. --- tests/test_optimizely.py | 13 ------------- tests/test_user_context.py | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index fca37d47..23454342 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -265,19 +265,6 @@ def test_init__sdk_key_and_datafile_access_token(self): self.assertIs(type(opt_obj.config_manager), config_manager.AuthDatafilePollingConfigManager) - def test_init__invalid_default_decide_options(self): - """ - Test to confirm that default decide options passed not as a list will trigger setting - self.deafulat_decide_options as an empty list. - """ - invalid_decide_options = {"testKey": "testOption"} - - mock_client_logger = mock.MagicMock() - with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): - opt_obj = optimizely.Optimizely(default_decide_options=invalid_decide_options) - - self.assertEqual(opt_obj.default_decide_options, []) - def test_invalid_json_raises_schema_validation_off(self): """ Test that invalid JSON logs error if schema validation is turned off. """ diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 9f4ab73a..1620a76a 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1296,4 +1296,4 @@ def test_decide_experiment(self): user_context = opt_obj.create_user_context('test_user') decision = user_context.decide('test_feature_in_experiment', [DecideOption.DISABLE_DECISION_EVENT]) self.assertTrue(decision.enabled, "decision should be enabled") - + \ No newline at end of file From df1b0814174654f8742cb7aa8370c944f6d564a5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 23 Jun 2021 17:22:09 -0400 Subject: [PATCH 07/59] [OASIS-7800] Updated optimizely_config with attributes and events --- optimizely/optimizely_config.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 2d7b74b6..bfd1c1b0 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -70,6 +70,22 @@ def get_events(self): """ return self.events + def get_attributes(self): + """ Get the attributes associated with OptimizelyConfig + + returns: + A list of attributes. + """ + return self.attributes + + def get_events(self): + """ Get the events associated with OptimizelyConfig + + returns: + A list of attributes. + """ + return self.events + class OptimizelyExperiment(object): def __init__(self, id, key, variations_map): self.id = id From 9c9bcead09f243e80e07670d3369b701a7cfa611 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 24 Jun 2021 08:31:49 -0400 Subject: [PATCH 08/59] Run autopep8 to fix formatting issues after rebase --- optimizely/optimizely_config.py | 1 + tests/test_user_context.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index bfd1c1b0..66949df0 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -86,6 +86,7 @@ def get_events(self): """ return self.events + class OptimizelyExperiment(object): def __init__(self, id, key, variations_map): self.id = id diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 1620a76a..fcffc415 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -1296,4 +1296,3 @@ def test_decide_experiment(self): user_context = opt_obj.create_user_context('test_user') decision = user_context.decide('test_feature_in_experiment', [DecideOption.DISABLE_DECISION_EVENT]) self.assertTrue(decision.enabled, "decision should be enabled") - \ No newline at end of file From 61d5c24706a026e98ed1234b9b66b424ed17f64a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 24 Jun 2021 12:15:03 -0400 Subject: [PATCH 09/59] [OASIS-7800] - Added test cases for attribute map and events map --- tests/test_optimizely_config.py | 42 +++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 29bd2443..8ff8986b 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -863,3 +863,45 @@ def test__get_events(self): self.assertEqual(expected_value, config.get_events()) self.assertEqual(len(config.get_events()), 2) + + def test__get_attributes_map(self): + """ Test to check get_attributes_map returns the correct value """ + + actual_attributes_map = self.opt_config_service.get_attributes_map() + expected_attributes = self.expected_config['attributes'] + + expected_attributes_map = {} + + for expected_attribute in expected_attributes: + optly_attribute = optimizely_config.OptimizelyAttribute( + expected_attribute['id'], expected_attribute['key'] + ) + expected_attributes_map[expected_attribute['key']] = optly_attribute + + for attribute in actual_attributes_map.values(): + self.assertIsInstance(attribute, optimizely_config.OptimizelyAttribute) + + self.assertEqual(len(expected_attributes), len(actual_attributes_map)) + self.assertEqual(self.to_dict(actual_attributes_map), self.to_dict(expected_attributes_map)) + + def test__get_events_map(self): + """ Test to check that get_events_map returns the correct value """ + + actual_events_map = self.opt_config_service.get_events_map() + expected_events = self.expected_config['events'] + + expected_events_map = {} + + for expected_event in expected_events: + optly_event = optimizely_config.OptimizelyEvent( + expected_event['id'], + expected_event['key'], + expected_event['experimentIds'] + ) + expected_events_map[expected_event['key']] = optly_event + + for event in actual_events_map.values(): + self.assertIsInstance(event, optimizely_config.OptimizelyEvent) + + self.assertEqual(len(expected_events), len(actual_events_map)) + self.assertEqual(self.to_dict(actual_events_map), self.to_dict(expected_events_map)) From 13703ca44df0c9b5c3265b6ac7067fe1cc863904 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 24 Jun 2021 12:21:08 -0400 Subject: [PATCH 10/59] Remove unused import from optimizely config --- optimizely/optimizely_config.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 66949df0..eba380c5 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -13,8 +13,6 @@ import copy -from optimizely.helpers.validator import are_attributes_valid - from .project_config import ProjectConfig From ef95cd8dabd26df8d16196c23dac139cc2913c55 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 25 Jun 2021 15:59:14 -0400 Subject: [PATCH 11/59] Corrected comment wording in get functions for events. --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index eba380c5..f142f14d 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -80,7 +80,7 @@ def get_events(self): """ Get the events associated with OptimizelyConfig returns: - A list of attributes. + A list of events. """ return self.events @@ -330,7 +330,7 @@ def get_attributes_map(self): return attributes_map def get_events_map(self): - """ Gets attributes map for the project config. + """ Gets events map for the project config. Returns: dict -- Event key, OptimizelyEvent map From b8828b9ab54e09896a853e6152f38d9da38f69dc Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 2 Jul 2021 14:32:48 -0400 Subject: [PATCH 12/59] [OASIS-7812] - Add Audiences to OpotimizelyConfig to expose to users --- optimizely/entities.py | 2 + optimizely/helpers/condition.py | 1 + optimizely/optimizely_config.py | 151 ++++++++++++++++++------- tests/test_optimizely_config.py | 191 +++++++++++++++++++++++--------- 4 files changed, 258 insertions(+), 87 deletions(-) diff --git a/optimizely/entities.py b/optimizely/entities.py index 88cd49c4..f5d8a04e 100644 --- a/optimizely/entities.py +++ b/optimizely/entities.py @@ -53,6 +53,7 @@ def __init__( audienceConditions=None, groupId=None, groupPolicy=None, + audiences=None, **kwargs ): self.id = id @@ -66,6 +67,7 @@ def __init__( self.layerId = layerId self.groupId = groupId self.groupPolicy = groupPolicy + self.audiences = audiences def get_audience_conditions_or_ids(self): """ Returns audienceConditions if present, otherwise audienceIds. """ diff --git a/optimizely/helpers/condition.py b/optimizely/helpers/condition.py index 2cd80dde..57ec558c 100644 --- a/optimizely/helpers/condition.py +++ b/optimizely/helpers/condition.py @@ -26,6 +26,7 @@ class ConditionOperatorTypes(object): AND = 'and' OR = 'or' NOT = 'not' + operators = [AND, OR, NOT] class ConditionMatchTypes(object): diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index f142f14d..a113f6ae 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,13 +12,15 @@ # limitations under the License. import copy +from optimizely.helpers.condition import ConditionOperatorTypes +from optimizely.entities import Experiment from .project_config import ProjectConfig class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, - sdk_key=None, environment_key=None, attributes=None, events=None): + sdk_key=None, environment_key=None, attributes=None, events=None, audiences=None): self.revision = revision self.experiments_map = experiments_map self.features_map = features_map @@ -27,6 +29,7 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.environment_key = environment_key self.attributes = attributes or [] self.events = events or [] + self.audiences = audiences or [] def get_datafile(self): """ Get the datafile associated with OptimizelyConfig. @@ -51,7 +54,7 @@ def get_environment_key(self): A string containing environment key. """ return self.environment_key - + def get_attributes(self): """ Get the attributes associated with OptimizelyConfig @@ -84,12 +87,21 @@ def get_events(self): """ return self.events + def get_audiences(self): + """ Get the audiences associated with OptimizelyConfig + + returns: + A list of audiences. + """ + return self.audiences + class OptimizelyExperiment(object): def __init__(self, id, key, variations_map): self.id = id self.key = key self.variations_map = variations_map + self.audiences = "" class OptimizelyFeature(object): @@ -129,6 +141,13 @@ def __init__(self, id, key, experiment_ids): self.experiment_ids = experiment_ids +class OptimizelyAudience(object): + def __init__(self, id, name, conditions): + self.id = id + self.name = name + self.conditions = conditions + + class OptimizelyConfigService(object): """ Class encapsulating methods to be used in creating instance of OptimizelyConfig. """ @@ -155,6 +174,97 @@ def __init__(self, project_config): self._create_lookup_maps() + ''' + Merging typed_audiences with audiences from project_config. + The typed_audiences has higher presidence. + ''' + + typed_audiences = project_config.typed_audiences or [] + + for old_audience in project_config.audiences: + # check if old_audience.id exists in new_audiences.id from typed_audiences + if len([new_audience for new_audience in typed_audiences if new_audience.get('id') == old_audience.get('id')]) == 0: + if old_audience.get('id') == "$opt_dummy_audience": + continue + else: + typed_audiences.append(old_audience) + + self.audiences = typed_audiences + + audiences_map = {} + + for audience in self.audiences: + audiences_map[audience.get('id')] = audience.get('name') + + # Updating each entities.Experiment found in the experiment_key_map + for ent_exp in project_config.experiment_key_map.values(): + experiments_by_key, experiments_by_id = self._get_experiments_maps() + try: + optly_experiment = experiments_by_id[ent_exp.id] + self.update_experiment(optly_experiment, ent_exp.audienceConditions, audiences_map) + except KeyError: + # ID not in map + continue + + def update_experiment(self, experiment, conditions, audiences_map): + + audiences = self.replace_ids_with_names(conditions, audiences_map) + experiment.audiences = audiences + + def replace_ids_with_names(self, conditions, audiences_map): + # Confirm where conditions are coming from... + if conditions != None: + return self.stringify_conditions(conditions, audiences_map) + else: + return None + + def lookup_name_from_id(self, audience_id, audiences_map): + name = "" + try: + name = audiences_map[audience_id] + except KeyError: + name = audience_id + + return name + + def stringify_conditions(self, conditions, audiences_map): + ARGS = ConditionOperatorTypes.operators + condition = "" + ret = "(" + length = len(conditions) + + if length == 0: + return + if length == 1: + ''' + Lookup ID and replace with name + ''' + audience_name = self.lookup_name_from_id(conditions[0], audiences_map) + + return audience_name + + if length > 1: + for i in range(length): + if conditions[i] in ARGS: + condition = conditions[i] + else: + if type(conditions[i]) == list: + if i + 1 < length: + ret += self.stringify_conditions(conditions[i], + audiences_map) + ' ' + condition.upper() + ' ' + else: + ret += self.stringify_conditions(conditions[i], audiences_map) + else: + # Handle ID's here - Lookup required + audience_name = self.lookup_name_from_id(conditions[i], audiences_map) + if audience_name != None: + if i + 1 < length: + ret += '"' + audience_name + '" ' + condition.upper() + ' ' + else: + ret += '"' + audience_name + '"' + + return ret + ")" + def get_config(self): """ Gets instance of OptimizelyConfig @@ -176,7 +286,8 @@ def get_config(self): self.sdk_key, self.environment_key, self.attributes, - self.events) + self.events, + self.audiences) def _create_lookup_maps(self): """ Creates lookup maps to avoid redundant iteration of config objects. """ @@ -311,37 +422,3 @@ def _get_features_map(self, experiments_id_map): features_map[feature['key']] = optly_feature return features_map - - def get_attributes_map(self): - """ Gets attributes map for the project config. - - Returns: - dict -- Attribute key, OptimizelyAttribute map - """ - - attributes_map = {} - - for attribute in self.attributes: - optly_attribute = OptimizelyAttribute( - attribute['id'], attribute['key'] - ) - attributes_map[attribute['key']] = optly_attribute - - return attributes_map - - def get_events_map(self): - """ Gets events map for the project config. - - Returns: - dict -- Event key, OptimizelyEvent map - """ - - events_map = {} - - for event in self.events: - optly_event = OptimizelyEvent( - event['id'], event['key'], event.get('experimentIds', []) - ) - events_map[event['key']] = optly_event - - return events_map diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 8ff8986b..2a17aec6 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -13,7 +13,7 @@ import json -from optimizely import optimizely +from optimizely import entities, optimizely from optimizely import optimizely_config from . import base @@ -30,6 +30,7 @@ def setUp(self): 'environment_key': None, 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experimentIds': ['111127'], 'id': '111095'}], + 'audiences': [{'name': 'Test attribute users 1', 'conditions': '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]', 'id': '11154'}, {'name': 'Test attribute users 2', 'conditions': '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]', 'id': '11159'}, {'name': 'Test attribute users 3', 'conditions': '["and", ["or", ["or", {"match": "exact", "name": "experiment_attr", "type": "custom_attribute", "value": "group_experiment"}]]]', 'id': '11160'}], 'experiments_map': { 'test_experiment2': { 'variations_map': { @@ -51,7 +52,8 @@ def setUp(self): } }, 'id': '111133', - 'key': 'test_experiment2' + 'key': 'test_experiment2', + 'audiences': '' }, 'test_experiment': { 'variations_map': { @@ -155,7 +157,8 @@ def setUp(self): } }, 'id': '111127', - 'key': 'test_experiment' + 'key': 'test_experiment', + 'audiences': '' }, 'group_exp_1': { 'variations_map': { @@ -177,7 +180,8 @@ def setUp(self): } }, 'id': '32222', - 'key': 'group_exp_1' + 'key': 'group_exp_1', + 'audiences': '' }, 'group_exp_2': { 'variations_map': { @@ -199,7 +203,8 @@ def setUp(self): } }, 'id': '32223', - 'key': 'group_exp_2' + 'key': 'group_exp_2', + 'audiences': '' }, 'group_2_exp_1': { 'variations_map': { @@ -213,7 +218,8 @@ def setUp(self): }, }, 'id': '42222', - 'key': 'group_2_exp_1' + 'key': 'group_2_exp_1', + 'audiences': '' }, 'group_2_exp_2': { 'variations_map': { @@ -227,7 +233,8 @@ def setUp(self): }, }, 'id': '42223', - 'key': 'group_2_exp_2' + 'key': 'group_2_exp_2', + 'audiences': '' }, 'group_2_exp_3': { 'variations_map': { @@ -241,7 +248,8 @@ def setUp(self): }, }, 'id': '42224', - 'key': 'group_2_exp_3' + 'key': 'group_2_exp_3', + 'audiences': '' }, 'test_experiment3': { 'variations_map': { @@ -255,7 +263,8 @@ def setUp(self): }, }, 'id': '111134', - 'key': 'test_experiment3' + 'key': 'test_experiment3', + 'audiences': '' }, 'test_experiment4': { 'variations_map': { @@ -269,7 +278,8 @@ def setUp(self): }, }, 'id': '111135', - 'key': 'test_experiment4' + 'key': 'test_experiment4', + 'audiences': '' }, 'test_experiment5': { 'variations_map': { @@ -283,7 +293,8 @@ def setUp(self): }, }, 'id': '111136', - 'key': 'test_experiment5' + 'key': 'test_experiment5', + 'audiences': '' } }, 'features_map': { @@ -435,7 +446,8 @@ def setUp(self): } }, 'id': '111127', - 'key': 'test_experiment' + 'key': 'test_experiment', + 'audiences': '' } }, 'id': '91111', @@ -505,7 +517,8 @@ def setUp(self): } }, 'id': '32222', - 'key': 'group_exp_1' + 'key': 'group_exp_1', + 'audiences': '' } }, 'id': '91113', @@ -536,7 +549,8 @@ def setUp(self): } }, 'id': '32223', - 'key': 'group_exp_2' + 'key': 'group_exp_2', + 'audiences': '' } }, 'id': '91114', @@ -559,7 +573,8 @@ def setUp(self): }, }, 'id': '42222', - 'key': 'group_2_exp_1' + 'key': 'group_2_exp_1', + 'audiences': '' }, 'group_2_exp_2': { 'variations_map': { @@ -573,7 +588,8 @@ def setUp(self): }, }, 'id': '42223', - 'key': 'group_2_exp_2' + 'key': 'group_2_exp_2', + 'audiences': '' }, 'group_2_exp_3': { 'variations_map': { @@ -587,7 +603,8 @@ def setUp(self): }, }, 'id': '42224', - 'key': 'group_2_exp_3' + 'key': 'group_2_exp_3', + 'audiences': '' } }, 'id': '91115', @@ -610,7 +627,8 @@ def setUp(self): }, }, 'id': '111134', - 'key': 'test_experiment3' + 'key': 'test_experiment3', + 'audiences': '' }, 'test_experiment4': { 'variations_map': { @@ -624,7 +642,8 @@ def setUp(self): }, }, 'id': '111135', - 'key': 'test_experiment4' + 'key': 'test_experiment4', + 'audiences': '' }, 'test_experiment5': { 'variations_map': { @@ -638,7 +657,8 @@ def setUp(self): }, }, 'id': '111136', - 'key': 'test_experiment5' + 'key': 'test_experiment5', + 'audiences': '' } }, 'id': '91116', @@ -864,44 +884,115 @@ def test__get_events(self): self.assertEqual(expected_value, config.get_events()) self.assertEqual(len(config.get_events()), 2) - def test__get_attributes_map(self): - """ Test to check get_attributes_map returns the correct value """ + def test_get_audiences(self): + ''' Test to confirm get_audiences returns proper value ''' - actual_attributes_map = self.opt_config_service.get_attributes_map() - expected_attributes = self.expected_config['attributes'] + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=[ + { + 'name': 'Test_Audience', + 'id': '1234', + 'conditions': [ + '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]' + ] + } + ] + ) + expected_value = [ + { + 'name': 'Test_Audience', + 'id': '1234', + 'conditions': [ + '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]' + ] + } + ] + + self.assertEqual(expected_value, config.get_audiences()) - expected_attributes_map = {} + def test_stringify_conditions(self): + ''' + Test to confirm converting from audienceConditions list to String represented conditions + Note* this also test lookup_name_from_id in OptimizelyConfig + ''' - for expected_attribute in expected_attributes: - optly_attribute = optimizely_config.OptimizelyAttribute( - expected_attribute['id'], expected_attribute['key'] - ) - expected_attributes_map[expected_attribute['key']] = optly_attribute + experiment = {'audienceConditions': ['and', ['or', '3468206642', '3988293898'], [ + 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} - for attribute in actual_attributes_map.values(): - self.assertIsInstance(attribute, optimizely_config.OptimizelyAttribute) + audience_conditions = experiment.get('audienceConditions') - self.assertEqual(len(expected_attributes), len(actual_attributes_map)) - self.assertEqual(self.to_dict(actual_attributes_map), self.to_dict(expected_attributes_map)) + audiences_map = { + '3468206642': 'US', + '3988293898': 'FEMALE', + '3988293899': 'MALE', + '3468206646': 'THEM', + '3468206647': 'ANYONE' + } - def test__get_events_map(self): - """ Test to check that get_events_map returns the correct value """ + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) - actual_events_map = self.opt_config_service.get_events_map() - expected_events = self.expected_config['events'] + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '(("US" OR "FEMALE") AND ("MALE" OR "THEM" OR "ANYONE" OR "3468206644" OR "3468206643"))' + + self.assertEqual(result, expected_result) + + def test_update_experiment(self): + ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' + ent_experiment = entities.Experiment( + '12345', + 'test_update_experiment', + 'Running', + variations=None, + forcedVariations=None, + trafficAllocation=None, + layerId=None, + audienceIds=['432', '321', '543'], + audienceConditions=['and', ['or', '432', '321'], '543'] + ) + + audiences_map = { + '432': 'us', + '321': 'female', + '543': 'adult' + } + + update_experiment = optimizely_config.OptimizelyExperiment( + '12345', + 'test_update_experiment', + variations_map={} + ) + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) - expected_events_map = {} + config_service = optimizely_config.OptimizelyConfigService(config) - for expected_event in expected_events: - optly_event = optimizely_config.OptimizelyEvent( - expected_event['id'], - expected_event['key'], - expected_event['experimentIds'] - ) - expected_events_map[expected_event['key']] = optly_event + config_service.update_experiment(update_experiment, ent_experiment.audienceConditions, audiences_map) - for event in actual_events_map.values(): - self.assertIsInstance(event, optimizely_config.OptimizelyEvent) + expected_value = '(("us" OR "female") AND "adult")' - self.assertEqual(len(expected_events), len(actual_events_map)) - self.assertEqual(self.to_dict(actual_events_map), self.to_dict(expected_events_map)) + self.assertEqual(update_experiment.audiences, expected_value) From c3bc816023eb2657a4c84d447a0e473d6ba1772e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 2 Jul 2021 14:35:18 -0400 Subject: [PATCH 13/59] Remove unused import for Experiment in OptimizelyConfig --- optimizely/optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index a113f6ae..5f670278 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -13,7 +13,6 @@ import copy from optimizely.helpers.condition import ConditionOperatorTypes -from optimizely.entities import Experiment from .project_config import ProjectConfig From f1bd5337cfd8bbe42b4abc9472e27e2704d9683a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 2 Jul 2021 15:30:53 -0400 Subject: [PATCH 14/59] Update Formatting issues for lint --- optimizely/optimizely_config.py | 59 ++++++++++++++++++------------ tests/test_optimizely_config.py | 64 ++++++++++++++++++++++++++------- 2 files changed, 89 insertions(+), 34 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 5f670278..0c0e04c0 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -70,22 +70,6 @@ def get_events(self): """ return self.events - def get_attributes(self): - """ Get the attributes associated with OptimizelyConfig - - returns: - A list of attributes. - """ - return self.attributes - - def get_events(self): - """ Get the events associated with OptimizelyConfig - - returns: - A list of events. - """ - return self.events - def get_audiences(self): """ Get the audiences associated with OptimizelyConfig @@ -182,7 +166,8 @@ def __init__(self, project_config): for old_audience in project_config.audiences: # check if old_audience.id exists in new_audiences.id from typed_audiences - if len([new_audience for new_audience in typed_audiences if new_audience.get('id') == old_audience.get('id')]) == 0: + if len([new_audience for new_audience in typed_audiences + if new_audience.get('id') == old_audience.get('id')]) == 0: if old_audience.get('id') == "$opt_dummy_audience": continue else: @@ -195,7 +180,7 @@ def __init__(self, project_config): for audience in self.audiences: audiences_map[audience.get('id')] = audience.get('name') - # Updating each entities.Experiment found in the experiment_key_map + # Updating each OptimizelyExperiment based on the ID from entities.Experiment found in the experiment_key_map for ent_exp in project_config.experiment_key_map.values(): experiments_by_key, experiments_by_id = self._get_experiments_maps() try: @@ -206,18 +191,39 @@ def __init__(self, project_config): continue def update_experiment(self, experiment, conditions, audiences_map): + ''' + Gets an OptimizelyExperiment to update, conditions from + the corresponding entities.Experiment and an audiences_map + in the form of [id:name] + Updates the OptimizelyExperiment.audiences with a string + of conditions and audience names. + ''' audiences = self.replace_ids_with_names(conditions, audiences_map) experiment.audiences = audiences def replace_ids_with_names(self, conditions, audiences_map): - # Confirm where conditions are coming from... - if conditions != None: + ''' + Gets conditions and audiences_map [id:name] + + Returns: + a string of conditions with id's swapped with names + or None if no conditions found. + + ''' + if conditions is not None: return self.stringify_conditions(conditions, audiences_map) else: return None def lookup_name_from_id(self, audience_id, audiences_map): + ''' + Gets and audience ID and audiences map + + Returns: + The name corresponding to the ID + or None if not found + ''' name = "" try: name = audiences_map[audience_id] @@ -227,6 +233,14 @@ def lookup_name_from_id(self, audience_id, audiences_map): return name def stringify_conditions(self, conditions, audiences_map): + ''' + Gets a list of conditions from an entities.Experiment + and an audiences_map [id:name] + + Returns: + A string of conditions and names for the provided + list of conditions. + ''' ARGS = ConditionOperatorTypes.operators condition = "" ret = "(" @@ -248,15 +262,16 @@ def stringify_conditions(self, conditions, audiences_map): condition = conditions[i] else: if type(conditions[i]) == list: + # If the next item is a list, recursively call function on list if i + 1 < length: ret += self.stringify_conditions(conditions[i], audiences_map) + ' ' + condition.upper() + ' ' else: ret += self.stringify_conditions(conditions[i], audiences_map) else: - # Handle ID's here - Lookup required + # Handle ID's here - Lookup name based on ID audience_name = self.lookup_name_from_id(conditions[i], audiences_map) - if audience_name != None: + if audience_name is not None: if i + 1 < length: ret += '"' + audience_name + '" ' + condition.upper() + ' ' else: diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 2a17aec6..e03e2e75 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -30,7 +30,26 @@ def setUp(self): 'environment_key': None, 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experimentIds': ['111127'], 'id': '111095'}], - 'audiences': [{'name': 'Test attribute users 1', 'conditions': '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]', 'id': '11154'}, {'name': 'Test attribute users 2', 'conditions': '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]', 'id': '11159'}, {'name': 'Test attribute users 3', 'conditions': '["and", ["or", ["or", {"match": "exact", "name": "experiment_attr", "type": "custom_attribute", "value": "group_experiment"}]]]', 'id': '11160'}], + 'audiences': [ + { + 'name': 'Test attribute users 1', + 'conditions': '["and", ["or", ["or", ' + '{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]', + 'id': '11154' + }, + { + 'name': 'Test attribute users 2', + 'conditions': '["and", ["or", ["or", ' + '{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]', + 'id': '11159' + }, + { + 'name': 'Test attribute users 3', + 'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \ + \"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]", + 'id': '11160', + } + ], 'experiments_map': { 'test_experiment2': { 'variations_map': { @@ -899,7 +918,17 @@ def test_get_audiences(self): 'name': 'Test_Audience', 'id': '1234', 'conditions': [ - '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]' + ["and", + ["or", + ["or", + { + "name": "test_attribute", + "type": "custom_attribute", + "value": "test_value_1" + } + ] + ] + ] ] } ] @@ -909,7 +938,17 @@ def test_get_audiences(self): 'name': 'Test_Audience', 'id': '1234', 'conditions': [ - '["and", ["or", ["or", {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]' + ["and", + ["or", + ["or", + { + "name": "test_attribute", + "type": "custom_attribute", + "value": "test_value_1" + } + ] + ] + ] ] } ] @@ -917,22 +956,23 @@ def test_get_audiences(self): self.assertEqual(expected_value, config.get_audiences()) def test_stringify_conditions(self): - ''' + ''' Test to confirm converting from audienceConditions list to String represented conditions - Note* this also test lookup_name_from_id in OptimizelyConfig + Note* this also test lookup_name_from_id in OptimizelyConfig ''' experiment = {'audienceConditions': ['and', ['or', '3468206642', '3988293898'], [ - 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} audience_conditions = experiment.get('audienceConditions') audiences_map = { - '3468206642': 'US', - '3988293898': 'FEMALE', - '3988293899': 'MALE', - '3468206646': 'THEM', - '3468206647': 'ANYONE' + '3468206642': 'us', + '3988293898': 'female', + '3988293899': 'male', + '3468206646': 'them', + '3468206647': 'anyone' } config = optimizely_config.OptimizelyConfig( @@ -949,7 +989,7 @@ def test_stringify_conditions(self): result = config_service.stringify_conditions(audience_conditions, audiences_map) - expected_result = '(("US" OR "FEMALE") AND ("MALE" OR "THEM" OR "ANYONE" OR "3468206644" OR "3468206643"))' + expected_result = '(("us" OR "female") AND ("male" OR "them" OR "anyone" OR "3468206644" OR "3468206643"))' self.assertEqual(result, expected_result) From 0ad43d08dc6c9f9f79ee806f1d382ceea57e0f50 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 2 Jul 2021 15:56:49 -0400 Subject: [PATCH 15/59] Update Import for conditions in optimizely_config.py --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 0c0e04c0..9e91bc1f 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,7 +12,7 @@ # limitations under the License. import copy -from optimizely.helpers.condition import ConditionOperatorTypes +from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig From 942bfe9e04531e0bb05a059b801ae1c01ed0f588 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 15:19:08 -0400 Subject: [PATCH 16/59] Additional changes for audiences and added functions to create attributes list and events list. --- optimizely/entities.py | 2 - optimizely/optimizely_config.py | 114 ++++++--- tests/test_optimizely_config.py | 397 ++++++++++++++++++++++++++------ 3 files changed, 411 insertions(+), 102 deletions(-) diff --git a/optimizely/entities.py b/optimizely/entities.py index f5d8a04e..88cd49c4 100644 --- a/optimizely/entities.py +++ b/optimizely/entities.py @@ -53,7 +53,6 @@ def __init__( audienceConditions=None, groupId=None, groupPolicy=None, - audiences=None, **kwargs ): self.id = id @@ -67,7 +66,6 @@ def __init__( self.layerId = layerId self.groupId = groupId self.groupPolicy = groupPolicy - self.audiences = audiences def get_audience_conditions_or_ids(self): """ Returns audienceConditions if present, otherwise audienceIds. """ diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 9e91bc1f..f70499d8 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -162,33 +162,32 @@ def __init__(self, project_config): The typed_audiences has higher presidence. ''' - typed_audiences = project_config.typed_audiences or [] + typed_audiences = project_config.typed_audiences.copy() + optly_typed_audiences = [] + for typed_audience in typed_audiences: + optly_audience = OptimizelyAudience( + typed_audience.get('id'), + typed_audience.get('name'), + typed_audience.get('conditions') + ) + optly_typed_audiences.append(optly_audience) for old_audience in project_config.audiences: # check if old_audience.id exists in new_audiences.id from typed_audiences - if len([new_audience for new_audience in typed_audiences + if len([new_audience for new_audience in project_config.typed_audiences if new_audience.get('id') == old_audience.get('id')]) == 0: if old_audience.get('id') == "$opt_dummy_audience": continue else: - typed_audiences.append(old_audience) - - self.audiences = typed_audiences - - audiences_map = {} - - for audience in self.audiences: - audiences_map[audience.get('id')] = audience.get('name') + # Convert audiences array to OptimizelyAudience array + optly_audience = OptimizelyAudience( + old_audience.get('id'), + old_audience.get('name'), + old_audience.get('conditions') + ) + optly_typed_audiences.append(optly_audience) - # Updating each OptimizelyExperiment based on the ID from entities.Experiment found in the experiment_key_map - for ent_exp in project_config.experiment_key_map.values(): - experiments_by_key, experiments_by_id = self._get_experiments_maps() - try: - optly_experiment = experiments_by_id[ent_exp.id] - self.update_experiment(optly_experiment, ent_exp.audienceConditions, audiences_map) - except KeyError: - # ID not in map - continue + self.audiences = optly_typed_audiences def update_experiment(self, experiment, conditions, audiences_map): ''' @@ -243,7 +242,7 @@ def stringify_conditions(self, conditions, audiences_map): ''' ARGS = ConditionOperatorTypes.operators condition = "" - ret = "(" + ret = "" length = len(conditions) if length == 0: @@ -254,20 +253,33 @@ def stringify_conditions(self, conditions, audiences_map): ''' audience_name = self.lookup_name_from_id(conditions[0], audiences_map) - return audience_name + return '"' + audience_name + '"' + + if length == 2: + if conditions[0] in ARGS: + condition = conditions[0] + audience_name = self.lookup_name_from_id(conditions[1], audiences_map) + return condition.upper() + ' "' + audience_name + '"' + else: + condition = 'OR' + return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) + '" ' + + condition.upper() + ' "' + + self.lookup_name_from_id(conditions[1], audiences_map) + '"') - if length > 1: + if length > 2: for i in range(length): if conditions[i] in ARGS: condition = conditions[i] else: + if condition == "": + condition = 'OR' if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - ret += self.stringify_conditions(conditions[i], - audiences_map) + ' ' + condition.upper() + ' ' + ret += '(' + self.stringify_conditions(conditions[i], + audiences_map) + ') ' + condition.upper() + ' ' else: - ret += self.stringify_conditions(conditions[i], audiences_map) + ret += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: # Handle ID's here - Lookup name based on ID audience_name = self.lookup_name_from_id(conditions[i], audiences_map) @@ -277,7 +289,7 @@ def stringify_conditions(self, conditions, audiences_map): else: ret += '"' + audience_name + '"' - return ret + ")" + return ret + "" def get_config(self): """ Gets instance of OptimizelyConfig @@ -299,8 +311,8 @@ def get_config(self): self._datafile, self.sdk_key, self.environment_key, - self.attributes, - self.events, + self._get_attributes_list(self.attributes), + self._get_events_list(self.events), self.audiences) def _create_lookup_maps(self): @@ -389,7 +401,8 @@ def _get_all_experiments(self): return experiments def _get_experiments_maps(self): - """ Gets maps for all the experiments in the project config. + """ Gets maps for all the experiments in the project config and + updates the experiment with updated experiment audiences string. Returns: dict, dict -- experiment key/id to OptimizelyExperiment maps. @@ -398,12 +411,20 @@ def _get_experiments_maps(self): experiments_key_map = {} # Id map comes in handy to figure out feature experiment. experiments_id_map = {} + # Audiences map to use for updating experiments with new audience conditions string + audiences_map = {} + + # Build map from OptimizelyAudience array + for optly_audience in self.audiences: + audiences_map[optly_audience.id] = optly_audience.name all_experiments = self._get_all_experiments() for exp in all_experiments: optly_exp = OptimizelyExperiment( exp['id'], exp['key'], self._get_variations_map(exp) ) + # Updating each OptimizelyExperiment based on the ID from entities.Experiment found in the experiment_key_map + self.update_experiment(optly_exp, exp.get('audienceConditions', []), audiences_map) experiments_key_map[exp['key']] = optly_exp experiments_id_map[exp['id']] = optly_exp @@ -436,3 +457,38 @@ def _get_features_map(self, experiments_id_map): features_map[feature['key']] = optly_feature return features_map + + def _get_attributes_list(self, attributes): + """ Gets attributes list for the project config + + Returns: + List - OptimizelyAttributes + """ + attributes_list = [] + + for attribute in attributes: + optly_attribute = OptimizelyAttribute( + attribute['id'], + attribute['key'] + ) + attributes_list.append(optly_attribute) + + return attributes_list + + def _get_events_list(self, events): + """ Gets events list for the project_config + + Returns: + List - OptimizelyEvents + """ + events_list = [] + + for event in events: + optly_event = OptimizelyEvent( + event['id'], + event['key'], + event['experimentIds'] + ) + events_list.append(optly_event) + + return events_list diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index e03e2e75..8e2de228 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -13,7 +13,7 @@ import json -from optimizely import entities, optimizely +from optimizely import entities, optimizely, project_config from optimizely import optimizely_config from . import base @@ -29,7 +29,7 @@ def setUp(self): 'sdk_key': None, 'environment_key': None, 'attributes': [{'key': 'test_attribute', 'id': '111094'}], - 'events': [{'key': 'test_event', 'experimentIds': ['111127'], 'id': '111095'}], + 'events': [{'key': 'test_event', 'experiment_ids': ['111127'], 'id': '111095'}], 'audiences': [ { 'name': 'Test attribute users 1', @@ -72,7 +72,7 @@ def setUp(self): }, 'id': '111133', 'key': 'test_experiment2', - 'audiences': '' + 'audiences': None }, 'test_experiment': { 'variations_map': { @@ -177,7 +177,7 @@ def setUp(self): }, 'id': '111127', 'key': 'test_experiment', - 'audiences': '' + 'audiences': None }, 'group_exp_1': { 'variations_map': { @@ -200,7 +200,7 @@ def setUp(self): }, 'id': '32222', 'key': 'group_exp_1', - 'audiences': '' + 'audiences': None }, 'group_exp_2': { 'variations_map': { @@ -223,7 +223,7 @@ def setUp(self): }, 'id': '32223', 'key': 'group_exp_2', - 'audiences': '' + 'audiences': None }, 'group_2_exp_1': { 'variations_map': { @@ -238,7 +238,7 @@ def setUp(self): }, 'id': '42222', 'key': 'group_2_exp_1', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'group_2_exp_2': { 'variations_map': { @@ -253,7 +253,7 @@ def setUp(self): }, 'id': '42223', 'key': 'group_2_exp_2', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'group_2_exp_3': { 'variations_map': { @@ -268,7 +268,7 @@ def setUp(self): }, 'id': '42224', 'key': 'group_2_exp_3', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'test_experiment3': { 'variations_map': { @@ -283,7 +283,7 @@ def setUp(self): }, 'id': '111134', 'key': 'test_experiment3', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'test_experiment4': { 'variations_map': { @@ -298,7 +298,7 @@ def setUp(self): }, 'id': '111135', 'key': 'test_experiment4', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'test_experiment5': { 'variations_map': { @@ -313,7 +313,7 @@ def setUp(self): }, 'id': '111136', 'key': 'test_experiment5', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' } }, 'features_map': { @@ -466,7 +466,7 @@ def setUp(self): }, 'id': '111127', 'key': 'test_experiment', - 'audiences': '' + 'audiences': None } }, 'id': '91111', @@ -537,7 +537,7 @@ def setUp(self): }, 'id': '32222', 'key': 'group_exp_1', - 'audiences': '' + 'audiences': None } }, 'id': '91113', @@ -569,7 +569,7 @@ def setUp(self): }, 'id': '32223', 'key': 'group_exp_2', - 'audiences': '' + 'audiences': None } }, 'id': '91114', @@ -593,7 +593,7 @@ def setUp(self): }, 'id': '42222', 'key': 'group_2_exp_1', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'group_2_exp_2': { 'variations_map': { @@ -608,7 +608,7 @@ def setUp(self): }, 'id': '42223', 'key': 'group_2_exp_2', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'group_2_exp_3': { 'variations_map': { @@ -623,7 +623,7 @@ def setUp(self): }, 'id': '42224', 'key': 'group_2_exp_3', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' } }, 'id': '91115', @@ -647,7 +647,7 @@ def setUp(self): }, 'id': '111134', 'key': 'test_experiment3', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'test_experiment4': { 'variations_map': { @@ -662,7 +662,7 @@ def setUp(self): }, 'id': '111135', 'key': 'test_experiment4', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' }, 'test_experiment5': { 'variations_map': { @@ -677,7 +677,7 @@ def setUp(self): }, 'id': '111136', 'key': 'test_experiment5', - 'audiences': '' + 'audiences': 'OR "Test attribute users 3"' } }, 'id': '91116', @@ -905,6 +905,46 @@ def test__get_events(self): def test_get_audiences(self): ''' Test to confirm get_audiences returns proper value ''' + base_test = base.BaseTest() + base_test.setUp() + config_dict = base_test.config_dict_with_typed_audiences + + proj_conf = project_config.ProjectConfig( + json.dumps(config_dict), + logger=None, + error_handler=None + ) + + config_service = optimizely_config.OptimizelyConfigService(proj_conf) + + for audience in config_service.audiences: + self.assertIsInstance(audience, optimizely_config.OptimizelyAudience) + + config = config_service.get_config() + + for audience in config.get_audiences(): + self.assertIsInstance(audience, optimizely_config.OptimizelyAudience) + + self.assertEqual(len(config.get_audiences()), len(config_service.audiences)) + + def test_stringify_conditions_non_matching_ids(self): + ''' + Test to confirm Non-matching ID's are handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['and', ['or', '3468206642', '3988293898'], [ + 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + '3988293898': 'female', + '3988293899': 'male', + '3468206646': 'them', + '3468206647': 'anyone' + } config = optimizely_config.OptimizelyConfig( revision='101', @@ -913,66 +953,193 @@ def test_get_audiences(self): environment_key='TestEnvironmentKey', attributes={}, events={}, - audiences=[ - { - 'name': 'Test_Audience', - 'id': '1234', - 'conditions': [ - ["and", - ["or", - ["or", - { - "name": "test_attribute", - "type": "custom_attribute", - "value": "test_value_1" - } - ] - ] - ] - ] - } - ] + audiences=None ) - expected_value = [ - { - 'name': 'Test_Audience', - 'id': '1234', - 'conditions': [ - ["and", - ["or", - ["or", - { - "name": "test_attribute", - "type": "custom_attribute", - "value": "test_value_1" - } - ] - ] - ] - ] - } - ] - self.assertEqual(expected_value, config.get_audiences()) + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '("us" OR "female") AND ("male" OR "them" OR "anyone" OR "3468206644" OR "3468206643")' + + self.assertEqual(result, expected_result) - def test_stringify_conditions(self): + def test_stringify_conditions_or(self): ''' - Test to confirm converting from audienceConditions list to String represented conditions - Note* this also test lookup_name_from_id in OptimizelyConfig + Test to confirm OR is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['and', ['or', '3468206642', '3988293898'], [ - 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['or', '3468206642', '3988293898'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + '3988293898': 'female' + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '"us" OR "female"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_and(self): + ''' + Test to confirm AND is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['and', '3468206642', '3988293898', '3988346543'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} audience_conditions = experiment.get('audienceConditions') audiences_map = { '3468206642': 'us', '3988293898': 'female', - '3988293899': 'male', - '3468206646': 'them', - '3468206647': 'anyone' + '3988346543': 'adult' + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '"us" AND "female" AND "adult"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_not_one_id(self): + ''' + Test to confirm Not and one ID is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['not', '3468206642'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = 'NOT "us"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_and_one_id(self): + ''' + Test to confirm AND and one ID is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['and', '3468206642'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = 'AND "us"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_one_id(self): + ''' + TTest to confirm a single ID is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['3468206642'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '"us"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_no_condition_word(self): + ''' + Test to confirm array of ID's is handled as OR by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['3468206642', '3988293898'], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + '3988293898': 'female' } config = optimizely_config.OptimizelyConfig( @@ -989,7 +1156,74 @@ def test_stringify_conditions(self): result = config_service.stringify_conditions(audience_conditions, audiences_map) - expected_result = '(("us" OR "female") AND ("male" OR "them" OR "anyone" OR "3468206644" OR "3468206643"))' + expected_result = '"us" OR "female"' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_and_or_and(self): + ''' + Test to confirm [and[or][and]] is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['and', ['or', '1', ['and', '2', '3']], ['and', '11', ['or', '12', '13']]], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '1': 'us', + '2': 'female', + '3': 'adult', + '11': 'fr', + '12': 'male', + '13': 'kid' + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "kid"))' + + self.assertEqual(result, expected_result) + + def test_stringify_conditions_empty_string(self): + ''' + Test to confirm empty string is handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': '', + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = {} + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = None self.assertEqual(result, expected_result) @@ -1033,6 +1267,27 @@ def test_update_experiment(self): config_service.update_experiment(update_experiment, ent_experiment.audienceConditions, audiences_map) - expected_value = '(("us" OR "female") AND "adult")' + expected_value = '("us" OR "female") AND "adult"' self.assertEqual(update_experiment.audiences, expected_value) + + def test_optimizely_audience_conversion(self): + ''' Test to confirm that audience conversion works and has expected output ''' + base_test = base.BaseTest() + base_test.setUp() + config_dict = base_test.config_dict_with_typed_audiences + + TOTAL_AUDEINCES_ONCE_MERGED = 10 + + proj_conf = project_config.ProjectConfig( + json.dumps(config_dict), + logger=None, + error_handler=None + ) + + config_service = optimizely_config.OptimizelyConfigService(proj_conf) + + for audience in config_service.audiences: + self.assertIsInstance(audience, optimizely_config.OptimizelyAudience) + + self.assertEqual(len(config_service.audiences), TOTAL_AUDEINCES_ONCE_MERGED) From 528b8f2a2e5069c2fded5b38489c90f2fb200bb0 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 17:34:12 -0400 Subject: [PATCH 17/59] Add delivery_rules to OptimizelyConfig and testcases to support --- optimizely/optimizely_config.py | 45 +++++++++++++++++++++++++++++++-- tests/test_optimizely_config.py | 29 +++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index f70499d8..6ea5e7ca 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -15,11 +15,13 @@ from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig +from optimizely import project_config class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, - sdk_key=None, environment_key=None, attributes=None, events=None, audiences=None): + sdk_key=None, environment_key=None, attributes=None, events=None, + audiences=None, delivery_rules=None): self.revision = revision self.experiments_map = experiments_map self.features_map = features_map @@ -29,6 +31,7 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.attributes = attributes or [] self.events = events or [] self.audiences = audiences or [] + self.delivery_rules = delivery_rules or [] def get_datafile(self): """ Get the datafile associated with OptimizelyConfig. @@ -78,6 +81,14 @@ def get_audiences(self): """ return self.audiences + def get_delivery_rules(self): + """ Get the delivery rules list associated with OptimizelyConfig + + Returns: + List of OptimizelyExperiments as DeliveryRules from Rollouts + """ + return self.delivery_rules + class OptimizelyExperiment(object): def __init__(self, id, key, variations_map): @@ -154,6 +165,7 @@ def __init__(self, project_config): self.environment_key = project_config.environment_key self.attributes = project_config.attributes self.events = project_config.events + self.rollouts = project_config.rollouts self._create_lookup_maps() @@ -313,7 +325,9 @@ def get_config(self): self.environment_key, self._get_attributes_list(self.attributes), self._get_events_list(self.events), - self.audiences) + self.audiences, + self._get_delivery_rules(self.rollouts) + ) def _create_lookup_maps(self): """ Creates lookup maps to avoid redundant iteration of config objects. """ @@ -458,6 +472,33 @@ def _get_features_map(self, experiments_id_map): return features_map + def _get_delivery_rules(self, rollouts): + """ Gets an array of rollouts for the project config + + returns: + an array of OptimizelyExperiments as delivery rules + """ + # Return list for delivery rules + delivery_rules = [] + # Audiences map to use for updating experiments with new audience conditions string + audiences_map = {} + + # Build map from OptimizelyAudience array + for optly_audience in self.audiences: + audiences_map[optly_audience.id] = optly_audience.name + + for rollout in rollouts: + experiments = rollout.get('experiments_map') + if experiments: + for experiment in experiments: + optly_exp = OptimizelyExperiment( + experiment['id'], experiment['key'], self._get_variations_map(experiment) + ) + self.update_experiment(optly_exp, experiment.get('audienceConditions', []), audiences_map) + delivery_rules.append(optly_exp) + + return delivery_rules + def _get_attributes_list(self, attributes): """ Gets attributes list for the project config diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 8e2de228..d8f6a7f1 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -28,6 +28,7 @@ def setUp(self): self.expected_config = { 'sdk_key': None, 'environment_key': None, + 'delivery_rules': [], 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experiment_ids': ['111127'], 'id': '111095'}], 'audiences': [ @@ -1291,3 +1292,31 @@ def test_optimizely_audience_conversion(self): self.assertIsInstance(audience, optimizely_config.OptimizelyAudience) self.assertEqual(len(config_service.audiences), TOTAL_AUDEINCES_ONCE_MERGED) + + def test__get_delivery_rules(self): + base_test = base.BaseTest() + base_test.setUp() + config_dict = base_test.config_dict_with_features + + proj_conf = project_config.ProjectConfig( + json.dumps(config_dict), + logger=None, + error_handler=None + ) + + config_service = optimizely_config.OptimizelyConfigService(proj_conf) + + config = config_service.get_config() + + for rule in config.get_delivery_rules(): + self.assertIsInstance(rule, optimizely_config.OptimizelyAudience) + + experiments_list = [] + + for rollout in config_service.rollouts: + experiments = rollout.get('experiments_map') + if experiments: + for exp in experiments: + experiments_list.append(exp) + + self.assertEqual(len(config.get_delivery_rules()), len(experiments_list)) From c3c3d2c08efb36b2bfd3848ea7c760e9e57068de Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 17:53:25 -0400 Subject: [PATCH 18/59] Formatting issues --- optimizely/optimizely_config.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 6ea5e7ca..6a2db4b2 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -15,7 +15,6 @@ from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig -from optimizely import project_config class OptimizelyConfig(object): @@ -274,9 +273,8 @@ def stringify_conditions(self, conditions, audiences_map): return condition.upper() + ' "' + audience_name + '"' else: condition = 'OR' - return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) + '" ' - + condition.upper() + ' "' - + self.lookup_name_from_id(conditions[1], audiences_map) + '"') + return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) + + '" ' + condition.upper() + ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"') if length > 2: for i in range(length): @@ -415,7 +413,7 @@ def _get_all_experiments(self): return experiments def _get_experiments_maps(self): - """ Gets maps for all the experiments in the project config and + """ Gets maps for all the experiments in the project config and updates the experiment with updated experiment audiences string. Returns: From 2fef52a0bf606a6d4c07b9effd4f00b0a833b3ed Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 17:59:51 -0400 Subject: [PATCH 19/59] Fix line length in comments and statement --- optimizely/optimizely_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 6a2db4b2..db14bb79 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -273,8 +273,8 @@ def stringify_conditions(self, conditions, audiences_map): return condition.upper() + ' "' + audience_name + '"' else: condition = 'OR' - return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) - + '" ' + condition.upper() + ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"') + return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) + '" ' + + condition.upper() + ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"') if length > 2: for i in range(length): @@ -435,7 +435,7 @@ def _get_experiments_maps(self): optly_exp = OptimizelyExperiment( exp['id'], exp['key'], self._get_variations_map(exp) ) - # Updating each OptimizelyExperiment based on the ID from entities.Experiment found in the experiment_key_map + # Updating each OptimizelyExperiment self.update_experiment(optly_exp, exp.get('audienceConditions', []), audiences_map) experiments_key_map[exp['key']] = optly_exp From d3b2a461579ead59b3e5ffaa104db7da1d1c7a8c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 18:07:29 -0400 Subject: [PATCH 20/59] Shortened return statement --- optimizely/optimizely_config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index db14bb79..b6ec1318 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -273,8 +273,9 @@ def stringify_conditions(self, conditions, audiences_map): return condition.upper() + ' "' + audience_name + '"' else: condition = 'OR' - return ('"' + self.lookup_name_from_id(conditions[0], audiences_map) + '" ' + - condition.upper() + ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"') + name1 = self.lookup_name_from_id(conditions[0], audiences_map) + name2 = self.lookup_name_from_id(conditions[1], audiences_map) + return ('"' + name1 + '" ' + condition.upper() + ' "' + name2 + '"') if length > 2: for i in range(length): From dda6e760cd869fbe2305ef3669c00b232361b2e9 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 6 Jul 2021 18:21:23 -0400 Subject: [PATCH 21/59] Updated list copy to use slice for backwards compatibility --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index b6ec1318..09df937e 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -173,7 +173,7 @@ def __init__(self, project_config): The typed_audiences has higher presidence. ''' - typed_audiences = project_config.typed_audiences.copy() + typed_audiences = project_config.typed_audiences[:] optly_typed_audiences = [] for typed_audience in typed_audiences: optly_audience = OptimizelyAudience( From 4f2d7fbf27e0f7c4077580314f6c60082b0d677e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 09:05:25 -0400 Subject: [PATCH 22/59] Add in test datafile for typed audiences --- tests/test_optimizely_config.py | 216 ++++++++++++++++++++++++++++++-- 1 file changed, 207 insertions(+), 9 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index d8f6a7f1..f1336dd3 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -12,6 +12,7 @@ # limitations under the License. import json +import unittest from optimizely import entities, optimizely, project_config from optimizely import optimizely_config @@ -692,6 +693,209 @@ def setUp(self): self.actual_config = self.opt_config_service.get_config() self.actual_config_dict = self.to_dict(self.actual_config) + self.typed_audiences_config = { + 'version': '2', + 'rollouts': [], + 'projectId': '10431130345', + 'variables': [], + 'featureFlags': [], + 'experiments': [ + { + 'status': 'Running', + 'key': 'ab_running_exp_untargeted', + 'layerId': '10417730432', + 'trafficAllocation': [{'entityId': '10418551353', 'endOfRange': 10000}], + 'audienceIds': [], + 'variations': [ + {'variables': [], 'id': '10418551353', 'key': 'all_traffic_variation'}, + {'variables': [], 'id': '10418510624', 'key': 'no_traffic_variation'}, + ], + 'forcedVariations': {}, + 'id': '10420810910', + } + ], + 'audiences': [ + { + 'id': '3468206642', + 'name': 'exactString', + 'conditions': '["and", ["or", ["or", {"name": "house", ' + '"type": "custom_attribute", "value": "Gryffindor"}]]]', + }, + { + 'id': '3988293898', + 'name': '$$dummySubstringString', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3988293899', + 'name': '$$dummyExists', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3468206646', + 'name': '$$dummyExactNumber', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3468206647', + 'name': '$$dummyGtNumber', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3468206644', + 'name': '$$dummyLtNumber', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3468206643', + 'name': '$$dummyExactBoolean', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '3468206645', + 'name': '$$dummyMultipleCustomAttrs', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + { + 'id': '0', + 'name': '$$dummy', + 'conditions': '{ "type": "custom_attribute", ' + '"name": "$opt_dummy_attribute", "value": "impossible_value" }', + }, + ], + 'typedAudiences': [ + { + 'id': '3988293898', + 'name': 'substringString', + 'conditions': [ + 'and', + [ + 'or', + [ + 'or', + { + 'name': 'house', + 'type': 'custom_attribute', + 'match': 'substring', + 'value': 'Slytherin', + }, + ], + ], + ], + }, + { + 'id': '3988293899', + 'name': 'exists', + 'conditions': [ + 'and', + [ + 'or', + ['or', {'name': 'favorite_ice_cream', 'type': 'custom_attribute', 'match': 'exists'}], + ], + ], + }, + { + 'id': '3468206646', + 'name': 'exactNumber', + 'conditions': [ + 'and', + [ + 'or', + ['or', {'name': 'lasers', 'type': 'custom_attribute', 'match': 'exact', 'value': 45.5}], + ], + ], + }, + { + 'id': '3468206647', + 'name': 'gtNumber', + 'conditions': [ + 'and', + ['or', ['or', {'name': 'lasers', 'type': 'custom_attribute', 'match': 'gt', 'value': 70}]], + ], + }, + { + 'id': '3468206644', + 'name': 'ltNumber', + 'conditions': [ + 'and', + ['or', ['or', {'name': 'lasers', 'type': 'custom_attribute', 'match': 'lt', 'value': 1.0}]], + ], + }, + { + 'id': '3468206643', + 'name': 'exactBoolean', + 'conditions': [ + 'and', + [ + 'or', + [ + 'or', + {'name': 'should_do_it', 'type': 'custom_attribute', 'match': 'exact', 'value': True}, + ], + ], + ], + }, + { + 'id': '3468206645', + 'name': 'multiple_custom_attrs', + 'conditions': [ + "and", + [ + "or", + [ + "or", + {"type": "custom_attribute", "name": "browser", "value": "chrome"}, + {"type": "custom_attribute", "name": "browser", "value": "firefox"}, + ], + ], + ], + }, + { + "id": "18278344267", + "name": "semverReleaseLt1.2.3Gt1.0.0", + "conditions": [ + "and", + [ + "or", + [ + "or", + { + "value": "1.2.3", + "type": "custom_attribute", + "name": "android-release", + "match": "semver_lt" + } + ] + ], + [ + "or", + [ + "or", + { + "value": "1.0.0", + "type": "custom_attribute", + "name": "android-release", + "match": "semver_gt" + } + ] + ] + ] + } + ], + 'groups': [], + 'attributes': [], + 'accountId': '10367498574', + 'events': [{'experimentIds': ['10420810910'], 'id': '10404198134', 'key': 'winning'}], + 'revision': '1337', + } + def to_dict(self, obj): return json.loads(json.dumps(obj, default=lambda o: o.__dict__)) @@ -906,9 +1110,7 @@ def test__get_events(self): def test_get_audiences(self): ''' Test to confirm get_audiences returns proper value ''' - base_test = base.BaseTest() - base_test.setUp() - config_dict = base_test.config_dict_with_typed_audiences + config_dict = self.typed_audiences_config proj_conf = project_config.ProjectConfig( json.dumps(config_dict), @@ -1274,9 +1476,7 @@ def test_update_experiment(self): def test_optimizely_audience_conversion(self): ''' Test to confirm that audience conversion works and has expected output ''' - base_test = base.BaseTest() - base_test.setUp() - config_dict = base_test.config_dict_with_typed_audiences + config_dict = self.typed_audiences_config TOTAL_AUDEINCES_ONCE_MERGED = 10 @@ -1294,9 +1494,7 @@ def test_optimizely_audience_conversion(self): self.assertEqual(len(config_service.audiences), TOTAL_AUDEINCES_ONCE_MERGED) def test__get_delivery_rules(self): - base_test = base.BaseTest() - base_test.setUp() - config_dict = base_test.config_dict_with_features + config_dict = self.typed_audiences_config proj_conf = project_config.ProjectConfig( json.dumps(config_dict), From f90e3e3933a3a73772161d5efe3a5c1ff3839dc5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 09:10:03 -0400 Subject: [PATCH 23/59] remove unused import --- tests/test_optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index f1336dd3..9f8eac81 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -12,7 +12,6 @@ # limitations under the License. import json -import unittest from optimizely import entities, optimizely, project_config from optimizely import optimizely_config From bb98741c8270625f0dc53aefb70aceb9992ec4d8 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 10:12:05 -0400 Subject: [PATCH 24/59] Modify test_update_experiment to pass audiences as OptimizelyExperiment --- tests/test_optimizely_config.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 9f8eac81..944b0c7e 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1431,16 +1431,13 @@ def test_stringify_conditions_empty_string(self): def test_update_experiment(self): ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' - ent_experiment = entities.Experiment( + + audience_conditions=['and', ['or', '432', '321'], '543'] + + optly_experiment = optimizely_config.OptimizelyExperiment( '12345', 'test_update_experiment', - 'Running', - variations=None, - forcedVariations=None, - trafficAllocation=None, - layerId=None, - audienceIds=['432', '321', '543'], - audienceConditions=['and', ['or', '432', '321'], '543'] + variations_map={} ) audiences_map = { @@ -1449,29 +1446,22 @@ def test_update_experiment(self): '543': 'adult' } - update_experiment = optimizely_config.OptimizelyExperiment( - '12345', - 'test_update_experiment', - variations_map={} - ) - config = optimizely_config.OptimizelyConfig( revision='101', - experiments_map={}, + experiments_map={optly_experiment.id: optly_experiment}, features_map={}, environment_key='TestEnvironmentKey', attributes={}, events={}, - audiences=None ) config_service = optimizely_config.OptimizelyConfigService(config) - config_service.update_experiment(update_experiment, ent_experiment.audienceConditions, audiences_map) + config_service.update_experiment(optly_experiment, audience_conditions, audiences_map) expected_value = '("us" OR "female") AND "adult"' - self.assertEqual(update_experiment.audiences, expected_value) + self.assertEqual(optly_experiment.audiences, expected_value) def test_optimizely_audience_conversion(self): ''' Test to confirm that audience conversion works and has expected output ''' From 48f604b3a4a4ff5f3a8fd223291dad275a3f3956 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 10:15:40 -0400 Subject: [PATCH 25/59] Fix linting issues --- tests/test_optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 944b0c7e..b04765ac 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -13,7 +13,7 @@ import json -from optimizely import entities, optimizely, project_config +from optimizely import optimizely, project_config from optimizely import optimizely_config from . import base @@ -1432,7 +1432,7 @@ def test_stringify_conditions_empty_string(self): def test_update_experiment(self): ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' - audience_conditions=['and', ['or', '432', '321'], '543'] + audience_conditions = ['and', ['or', '432', '321'], '543'] optly_experiment = optimizely_config.OptimizelyExperiment( '12345', From 8179f2bf5065cb5716ccf9b508a8113e0561458c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 14:44:00 -0400 Subject: [PATCH 26/59] Style changes as requested to comments and variable naming. --- optimizely/optimizely_config.py | 20 +++++++++----------- tests/test_optimizely_config.py | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 09df937e..b222913b 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -190,7 +190,7 @@ def __init__(self, project_config): if old_audience.get('id') == "$opt_dummy_audience": continue else: - # Convert audiences array to OptimizelyAudience array + # Convert audiences lists to OptimizelyAudience array optly_audience = OptimizelyAudience( old_audience.get('id'), old_audience.get('name'), @@ -253,15 +253,13 @@ def stringify_conditions(self, conditions, audiences_map): ''' ARGS = ConditionOperatorTypes.operators condition = "" - ret = "" + conditions_str = "" length = len(conditions) if length == 0: return if length == 1: - ''' - Lookup ID and replace with name - ''' + #Lookup ID and replace with name audience_name = self.lookup_name_from_id(conditions[0], audiences_map) return '"' + audience_name + '"' @@ -287,20 +285,20 @@ def stringify_conditions(self, conditions, audiences_map): if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - ret += '(' + self.stringify_conditions(conditions[i], + conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' + condition.upper() + ' ' else: - ret += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' + conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: - # Handle ID's here - Lookup name based on ID + # Handle IDs here - Lookup name based on ID audience_name = self.lookup_name_from_id(conditions[i], audiences_map) if audience_name is not None: if i + 1 < length: - ret += '"' + audience_name + '" ' + condition.upper() + ' ' + conditions_str += '"' + audience_name + '" ' + condition.upper() + ' ' else: - ret += '"' + audience_name + '"' + conditions_str += '"' + audience_name + '"' - return ret + "" + return conditions_str + "" def get_config(self): """ Gets instance of OptimizelyConfig diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index b04765ac..2b7c947a 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1299,7 +1299,7 @@ def test_stringify_conditions_and_one_id(self): def test_stringify_conditions_one_id(self): ''' - TTest to confirm a single ID is handled properly by stringify_conditions functions + Test to confirm a single ID is handled properly by stringify_conditions functions ''' experiment = {'audienceConditions': ['3468206642'], From d694c7fa016a45ffb1659434c9a9c312da4050c5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 14:50:50 -0400 Subject: [PATCH 27/59] Lint Update --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index b222913b..b4527081 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -259,7 +259,7 @@ def stringify_conditions(self, conditions, audiences_map): if length == 0: return if length == 1: - #Lookup ID and replace with name + # Lookup ID and replace with name audience_name = self.lookup_name_from_id(conditions[0], audiences_map) return '"' + audience_name + '"' @@ -286,7 +286,7 @@ def stringify_conditions(self, conditions, audiences_map): # If the next item is a list, recursively call function on list if i + 1 < length: conditions_str += '(' + self.stringify_conditions(conditions[i], - audiences_map) + ') ' + condition.upper() + ' ' + audiences_map) + ') ' + condition.upper() + ' ' else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: From ec39096e9e8eb064596b58e645be2f9955539378 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 15:00:01 -0400 Subject: [PATCH 28/59] Fix line length --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index b4527081..40b9fddf 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -285,8 +285,8 @@ def stringify_conditions(self, conditions, audiences_map): if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += '(' + self.stringify_conditions(conditions[i], - audiences_map) + ') ' + condition.upper() + ' ' + conditions_str += ('(' + self.stringify_conditions(conditions[i], audiences_map) + + ') ' + condition.upper() + ' ') else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: From a687a9be0702d83531aa381b632763fcfbca3b8a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 15:05:15 -0400 Subject: [PATCH 29/59] Move upper() to central location to only call once --- optimizely/optimizely_config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 40b9fddf..f37417a8 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -278,15 +278,15 @@ def stringify_conditions(self, conditions, audiences_map): if length > 2: for i in range(length): if conditions[i] in ARGS: - condition = conditions[i] + condition = conditions[i].upper() else: if condition == "": condition = 'OR' if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += ('(' + self.stringify_conditions(conditions[i], audiences_map) + - ') ' + condition.upper() + ' ') + conditions_str += ('(' + self.stringify_conditions(conditions[i], + audiences_map) + ') ' + condition + ' ') else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: @@ -294,7 +294,7 @@ def stringify_conditions(self, conditions, audiences_map): audience_name = self.lookup_name_from_id(conditions[i], audiences_map) if audience_name is not None: if i + 1 < length: - conditions_str += '"' + audience_name + '" ' + condition.upper() + ' ' + conditions_str += '"' + audience_name + '" ' + condition + ' ' else: conditions_str += '"' + audience_name + '"' From 11be43040513a44071257b0daf96432bd46183f3 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 15:15:51 -0400 Subject: [PATCH 30/59] Indent fix --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index f37417a8..cc864764 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -286,7 +286,7 @@ def stringify_conditions(self, conditions, audiences_map): # If the next item is a list, recursively call function on list if i + 1 < length: conditions_str += ('(' + self.stringify_conditions(conditions[i], - audiences_map) + ') ' + condition + ' ') + audiences_map) + ') ' + condition + ' ') else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: From ecf2e45986ca200ea7dd80601d4eac90de077f1c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 7 Jul 2021 15:22:07 -0400 Subject: [PATCH 31/59] Broke statement into two for Lint issues --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index cc864764..0678eb29 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -285,8 +285,8 @@ def stringify_conditions(self, conditions, audiences_map): if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += ('(' + self.stringify_conditions(conditions[i], - audiences_map) + ') ' + condition + ' ') + conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' + conditions_str += condition + ' ' else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: From f568aad0d273ced70acba7ea92d57b97bfa6ea7b Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 08:01:20 -0400 Subject: [PATCH 32/59] Added test case for variation --- tests/test_optimizely_config.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 2b7c947a..417bd243 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1507,3 +1507,25 @@ def test__get_delivery_rules(self): experiments_list.append(exp) self.assertEqual(len(config.get_delivery_rules()), len(experiments_list)) + + def test__get_dvariations_from_experiment(self): + config_dict = self.typed_audiences_config + + proj_conf = project_config.ProjectConfig( + json.dumps(config_dict), + logger=None, + error_handler=None + ) + + config_service = optimizely_config.OptimizelyConfigService(proj_conf) + + experiments_key_map, experiments_id_map = config_service._get_experiments_maps() + + optly_experiment = experiments_id_map['10420810910'] + + for variation in optly_experiment.variations_map.values(): + self.assertIsInstance(variation, optimizely_config.OptimizelyVariation) + if variation.id == '10418551353': + self.assertEqual(variation.key, 'all_traffic_variation') + else: + self.assertEqual(variation.key, 'no_traffic_variation') \ No newline at end of file From 3b8706fa39afc42b4f2b416f8431b2c863df809c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 08:45:20 -0400 Subject: [PATCH 33/59] Add changes to decision service to us experiment ID instead of KEY. --- optimizely/decision_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 98060e8e..6bc92333 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -342,7 +342,7 @@ 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')) # Check if user meets audience conditions for targeting rule audience_conditions = rollout_rule.get_audience_conditions_or_ids() @@ -387,7 +387,7 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes break # Evaluate last rule i.e. "Everyone Else" rule - everyone_else_rule = project_config.get_experiment_from_key(rollout.experiments[-1].get('key')) + everyone_else_rule = project_config.get_experiment_from_id(rollout.experiments[-1].get('id')) audience_conditions = everyone_else_rule.get_audience_conditions_or_ids() audience_eval, audience_reasons = audience_helper.does_user_meet_audience_conditions( project_config, From 2504f98f89961d7e2b07e6cd1b8b1923b8c64fc0 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 08:50:26 -0400 Subject: [PATCH 34/59] Missing newline added --- tests/test_optimizely_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 417bd243..4a61bf27 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1507,8 +1507,8 @@ def test__get_delivery_rules(self): experiments_list.append(exp) self.assertEqual(len(config.get_delivery_rules()), len(experiments_list)) - - def test__get_dvariations_from_experiment(self): + + def test_get_variations_from_experiments_map(self): config_dict = self.typed_audiences_config proj_conf = project_config.ProjectConfig( @@ -1528,4 +1528,4 @@ def test__get_dvariations_from_experiment(self): if variation.id == '10418551353': self.assertEqual(variation.key, 'all_traffic_variation') else: - self.assertEqual(variation.key, 'no_traffic_variation') \ No newline at end of file + self.assertEqual(variation.key, 'no_traffic_variation') From 1cd6aabb1ff51c0d2fd566091e14ec3a03b7d72f Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 09:17:05 -0400 Subject: [PATCH 35/59] Remove redundant update function. Ad d period to comment --- optimizely/optimizely_config.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 0678eb29..6e55308f 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -200,18 +200,6 @@ def __init__(self, project_config): self.audiences = optly_typed_audiences - def update_experiment(self, experiment, conditions, audiences_map): - ''' - Gets an OptimizelyExperiment to update, conditions from - the corresponding entities.Experiment and an audiences_map - in the form of [id:name] - - Updates the OptimizelyExperiment.audiences with a string - of conditions and audience names. - ''' - audiences = self.replace_ids_with_names(conditions, audiences_map) - experiment.audiences = audiences - def replace_ids_with_names(self, conditions, audiences_map): ''' Gets conditions and audiences_map [id:name] @@ -232,7 +220,7 @@ def lookup_name_from_id(self, audience_id, audiences_map): Returns: The name corresponding to the ID - or None if not found + or None if not found. ''' name = "" try: @@ -435,7 +423,8 @@ def _get_experiments_maps(self): exp['id'], exp['key'], self._get_variations_map(exp) ) # Updating each OptimizelyExperiment - self.update_experiment(optly_exp, exp.get('audienceConditions', []), audiences_map) + audiences = self.replace_ids_with_names(exp.get('audienceConditions', []), audiences_map) + optly_exp.audiences = audiences experiments_key_map[exp['key']] = optly_exp experiments_id_map[exp['id']] = optly_exp @@ -473,7 +462,7 @@ def _get_delivery_rules(self, rollouts): """ Gets an array of rollouts for the project config returns: - an array of OptimizelyExperiments as delivery rules + an array of OptimizelyExperiments as delivery rules. """ # Return list for delivery rules delivery_rules = [] From bc98adcf584fc12eec4849bcffe33d6cbb5e5c3f Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 09:21:17 -0400 Subject: [PATCH 36/59] Rename testcase to remove replace_ids_with_names. --- tests/test_optimizely_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 4a61bf27..cf3640b2 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1429,7 +1429,7 @@ def test_stringify_conditions_empty_string(self): self.assertEqual(result, expected_result) - def test_update_experiment(self): + def test_replace_ids_with_names(self): ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' audience_conditions = ['and', ['or', '432', '321'], '543'] @@ -1457,8 +1457,8 @@ def test_update_experiment(self): config_service = optimizely_config.OptimizelyConfigService(config) - config_service.update_experiment(optly_experiment, audience_conditions, audiences_map) - + audiences = config_service.replace_ids_with_names(audience_conditions, audiences_map) + optly_experiment.audiences = audiences expected_value = '("us" OR "female") AND "adult"' self.assertEqual(optly_experiment.audiences, expected_value) From 98ff2b66fb1dcbf0aedbb50cd696ca72100d6208 Mon Sep 17 00:00:00 2001 From: Jake Brown Date: Fri, 9 Jul 2021 19:50:26 -0400 Subject: [PATCH 37/59] Duplicate experiment Key issue with multi feature flag (#347) * Changing lookup of experiment in decision service to use experiment ID instead of key to resolve bug * [BUGFIX] - Update to bucketer and added proper ID map to project_config. * Switch to generate experiments_key_map from experiment_id_map rather than the other way around. * Updated with unit tests. * Change line break before operator to avoid W504 Flake8 issue. * Flake8 changes to follow best practice and ignore the W504 --- .flake8 | 5 +++- optimizely/bucketer.py | 2 +- optimizely/project_config.py | 57 +++++++++++++++++++++++++++++++----- tests/test_config.py | 25 ++++++++++++++++ 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/.flake8 b/.flake8 index f31217bf..f5990a83 100644 --- a/.flake8 +++ b/.flake8 @@ -1,5 +1,8 @@ [flake8] # E722 - do not use bare 'except' -ignore = E722 +# W504 - Either W503 (Line break after Operand) or W503 ( +# Line break before operand needs to be ignored for line lengths +# greater than max-line-length. Best practice shows W504 +ignore = E722, W504 exclude = optimizely/lib/pymmh3.py,*virtualenv* max-line-length = 120 diff --git a/optimizely/bucketer.py b/optimizely/bucketer.py index ca5e0f28..24852100 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_by_experiment_id(experiment.id, variation_id) return variation, decide_reasons else: diff --git a/optimizely/project_config.py b/optimizely/project_config.py index ac97fac6..8a696599 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -68,7 +68,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.experiment_key_map = self._generate_key_map(self.experiments, 'key', entities.Experiment) + self.experiment_id_map = self._generate_key_map(self.experiments, 'id', 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) @@ -84,27 +84,36 @@ def __init__(self, datafile, logger, error_handler): self.rollout_id_map = self._generate_key_map(self.rollouts, 'id', entities.Layer) for layer in self.rollout_id_map.values(): for experiment in layer.experiments: - self.experiment_key_map[experiment['key']] = entities.Experiment(**experiment) + self.experiment_id_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(): - experiments_in_group_key_map = self._generate_key_map(group.experiments, 'key', entities.Experiment) - for experiment in experiments_in_group_key_map.values(): + 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_key_map) + self.experiment_id_map.update(experiments_in_group_id_map) - self.experiment_id_map = {} + self.experiment_key_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_id_map_by_experiment_id = {} + self.variation_key_map_by_experiment_id = {} + + for experiment in self.experiment_id_map.values(): + self.experiment_key_map[experiment.key] = experiment self.variation_key_map[experiment.key] = self._generate_key_map( experiment.variations, 'key', entities.Variation ) + self.variation_id_map[experiment.key] = {} + self.variation_id_map_by_experiment_id[experiment.id] = {} + self.variation_key_map_by_experiment_id[experiment.id] = {} + for variation in self.variation_key_map.get(experiment.key).values(): self.variation_id_map[experiment.key][variation.id] = variation + self.variation_id_map_by_experiment_id[experiment.id][variation.id] = variation + self.variation_key_map_by_experiment_id[experiment.id][variation.key] = variation self.variation_variable_usage_map[variation.id] = self._generate_key_map( variation.variables, 'id', entities.Variation.VariableUsage ) @@ -557,3 +566,35 @@ def is_feature_experiment(self, experiment_id): """ return experiment_id in self.experiment_feature_map + + def get_variation_from_id_by_experiment_id(self, experiment_id, variation_id): + """ Gets variation from variation id and specific experiment id + + Returns: + The variation for the experiment id and variation id + or empty dict if not found + """ + if (experiment_id in self.variation_id_map_by_experiment_id and + variation_id in self.variation_id_map_by_experiment_id[experiment_id]): + return self.variation_id_map_by_experiment_id[experiment_id][variation_id] + + self.logger.error('Variation with id "%s" not defined in the datafile for experiment "%s".', + variation_id, experiment_id) + + return {} + + def get_variation_from_key_by_experiment_id(self, experiment_id, variation_key): + """ Gets variation from variation key and specific experiment id + + Returns: + The variation for the experiment id and variation key + or empty dict if not found + """ + if (experiment_id in self.variation_key_map_by_experiment_id and + variation_key in self.variation_key_map_by_experiment_id[experiment_id]): + return self.variation_key_map_by_experiment_id[experiment_id][variation_key] + + self.logger.error('Variation with key "%s" not defined in the datafile for experiment "%s".', + variation_key, experiment_id) + + return {} diff --git a/tests/test_config.py b/tests/test_config.py index c9051054..fe0f8f38 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -41,6 +41,7 @@ def test_init(self): self.config_dict['groups'][0]['trafficAllocation'], ) } + expected_experiment_key_map = { 'test_experiment': entities.Experiment( '111127', @@ -1213,3 +1214,27 @@ def test_is_feature_experiment(self): self.assertStrictFalse(project_config.is_feature_experiment(experiment.id)) self.assertStrictTrue(project_config.is_feature_experiment(feature_experiment.id)) + + def test_get_variation_from_id_by_experiment_id(self): + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict)) + project_config = opt_obj.config_manager.get_config() + + experiment_id = '111127' + variation_id = '111128' + + variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) + + self.assertIsInstance(variation, entities.Variation) + + def test_get_variation_from_key_by_experiment_id(self): + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict)) + project_config = opt_obj.config_manager.get_config() + + experiment_id = '111127' + variation_key = 'control' + + variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key) + + self.assertIsInstance(variation, entities.Variation) From d6fa32d598bfb4b620841396387a82862937349e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 9 Jul 2021 10:08:27 -0400 Subject: [PATCH 38/59] Update delivery rules test. --- optimizely/optimizely_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 6e55308f..60304e59 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -84,7 +84,7 @@ def get_delivery_rules(self): """ Get the delivery rules list associated with OptimizelyConfig Returns: - List of OptimizelyExperiments as DeliveryRules from Rollouts + List of OptimizelyExperiments as DeliveryRules from Rollouts. """ return self.delivery_rules @@ -480,8 +480,8 @@ def _get_delivery_rules(self, rollouts): optly_exp = OptimizelyExperiment( experiment['id'], experiment['key'], self._get_variations_map(experiment) ) - self.update_experiment(optly_exp, experiment.get('audienceConditions', []), audiences_map) - delivery_rules.append(optly_exp) + audiences = self.replace_ids_with_names(experiment.get('audienceConditions', []), audiences_map) + optly_exp.audiences = audiences return delivery_rules From 16059d7c6cf31b16be51107d503974f6174965cd Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 07:54:55 -0400 Subject: [PATCH 39/59] Explenation added to flake8 for ignoring W504. --- optimizely/optimizely_config.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 60304e59..6c3adc08 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -248,20 +248,16 @@ def stringify_conditions(self, conditions, audiences_map): return if length == 1: # Lookup ID and replace with name - audience_name = self.lookup_name_from_id(conditions[0], audiences_map) - - return '"' + audience_name + '"' + return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' if length == 2: if conditions[0] in ARGS: - condition = conditions[0] audience_name = self.lookup_name_from_id(conditions[1], audiences_map) - return condition.upper() + ' "' + audience_name + '"' + return conditions[0].upper() + ' "' + audience_name + '"' else: - condition = 'OR' name1 = self.lookup_name_from_id(conditions[0], audiences_map) name2 = self.lookup_name_from_id(conditions[1], audiences_map) - return ('"' + name1 + '" ' + condition.upper() + ' "' + name2 + '"') + return ('"' + name1 + '" OR "' + name2 + '"') if length > 2: for i in range(length): @@ -273,8 +269,7 @@ def stringify_conditions(self, conditions, audiences_map): if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' - conditions_str += condition + ' ' + conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' + condition + ' ' else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: @@ -286,7 +281,7 @@ def stringify_conditions(self, conditions, audiences_map): else: conditions_str += '"' + audience_name + '"' - return conditions_str + "" + return conditions_str def get_config(self): """ Gets instance of OptimizelyConfig From 03206271b980892319322643181615cb5cdd948d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:00:28 -0400 Subject: [PATCH 40/59] Changes made to move delivery rules and experiment rules into OptimizelyFeature and update test cases to reflect the same. --- optimizely/optimizely_config.py | 48 ++++-- tests/base.py | 20 ++- tests/test_optimizely_config.py | 265 ++++++++++++++++++++++++++++---- 3 files changed, 296 insertions(+), 37 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 6c3adc08..39b6286a 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -15,12 +15,13 @@ from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig +from optimizely import project_config class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, sdk_key=None, environment_key=None, attributes=None, events=None, - audiences=None, delivery_rules=None): + audiences=None): self.revision = revision self.experiments_map = experiments_map self.features_map = features_map @@ -30,7 +31,6 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.attributes = attributes or [] self.events = events or [] self.audiences = audiences or [] - self.delivery_rules = delivery_rules or [] def get_datafile(self): """ Get the datafile associated with OptimizelyConfig. @@ -103,6 +103,8 @@ def __init__(self, id, key, experiments_map, variables_map): self.key = key self.experiments_map = experiments_map self.variables_map = variables_map + self.delivery_rules = [] + self.experiment_rules = [] class OptimizelyVariation(object): @@ -305,8 +307,7 @@ def get_config(self): self.environment_key, self._get_attributes_list(self.attributes), self._get_events_list(self.events), - self.audiences, - self._get_delivery_rules(self.rollouts) + self.audiences ) def _create_lookup_maps(self): @@ -436,24 +437,48 @@ def _get_features_map(self, experiments_id_map): dict -- feaure key to OptimizelyFeature map """ features_map = {} + experiment_rules = [] for feature in self.feature_flags: + + """ + TODO - + + delivery Rules : Filter rollouts based on the feature.rolloutID and take the + first item in the filtered list. the delivery rules are then that rollout + experiments. + + experiment Rules: feature.experimentIDs a list of experiments based on the feature.id matching + the experimentID. + + """ + + delivery_rules = self._get_delivery_rules(self.rollouts, feature.get('rolloutId')) + experiment_rules = [] + exp_map = {} for experiment_id in feature.get('experimentIds', []): optly_exp = experiments_id_map[experiment_id] exp_map[optly_exp.key] = optly_exp + # Append the optly experiment to experiment rules + # The optly experiment has already been updated + # during the experiment maps creation. + experiment_rules.append(optly_exp) + variables_map = self.feature_key_variable_key_to_variable_map[feature['key']] optly_feature = OptimizelyFeature( feature['id'], feature['key'], exp_map, variables_map ) + optly_feature.experiment_rules = experiment_rules + optly_feature.delivery_rules = delivery_rules features_map[feature['key']] = optly_feature return features_map - def _get_delivery_rules(self, rollouts): + def _get_delivery_rules(self, rollouts, rollout_id): """ Gets an array of rollouts for the project config returns: @@ -464,11 +489,16 @@ def _get_delivery_rules(self, rollouts): # Audiences map to use for updating experiments with new audience conditions string audiences_map = {} - # Build map from OptimizelyAudience array - for optly_audience in self.audiences: - audiences_map[optly_audience.id] = optly_audience.name + # Gets a rollout based on provided rollout_id + rollout = [rollout for rollout in rollouts if rollout.get('id') == rollout_id] + + if rollout: + rollout = rollout[0] + # Build map from OptimizelyAudience array + for optly_audience in self.audiences: + audiences_map[optly_audience.id] = optly_audience.name - for rollout in rollouts: + # Get the experiments_map for that rollout experiments = rollout.get('experiments_map') if experiments: for experiment in experiments: diff --git a/tests/base.py b/tests/base.py index 48b89106..d4c010d0 100644 --- a/tests/base.py +++ b/tests/base.py @@ -485,6 +485,8 @@ def setUp(self, config_dict='config_dict'): 'subType': 'json'}, {'id': '133', 'key': 'true_object', 'defaultValue': '{"true_test": 23.54}', 'type': 'json'}, ], + 'delivery_rules': [], + 'experiment_rules': [], }, { 'id': '91112', @@ -499,6 +501,8 @@ def setUp(self, config_dict='config_dict'): {'id': '136', 'key': 'object', 'defaultValue': '{"field": 1}', 'type': 'string', 'subType': 'json'}, ], + 'delivery_rules': [], + 'experiment_rules': [], }, { 'id': '91113', @@ -506,6 +510,8 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['32222'], 'rolloutId': '', 'variables': [], + 'delivery_rules': [], + 'experiment_rules': [], }, { 'id': '91114', @@ -513,6 +519,8 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['32223'], 'rolloutId': '211111', 'variables': [], + 'delivery_rules': [], + 'experiment_rules': [], }, { 'id': '91115', @@ -520,6 +528,8 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['42222', '42223', '42224'], 'rolloutId': '211111', 'variables': [], + 'delivery_rules': [], + 'experiment_rules': [], }, { 'id': '91116', @@ -527,6 +537,8 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['111134', '111135', '111136'], 'rolloutId': '211111', 'variables': [], + 'delivery_rules': [], + 'experiment_rules': [], }, ], } @@ -756,13 +768,15 @@ def setUp(self, config_dict='config_dict'): 'projectId': '11624721371', 'variables': [], 'featureFlags': [ - {'experimentIds': [], 'rolloutId': '11551226731', 'variables': [], 'id': '11477755619', 'key': 'feat'}, + {'experimentIds': [], 'rolloutId': '11551226731', 'variables': [], 'id': '11477755619', 'key': 'feat', 'delivery_rules': [], 'experiment_rules': [],}, { 'experimentIds': ['11564051718'], 'rolloutId': '11638870867', 'variables': [{'defaultValue': 'x', 'type': 'string', 'id': '11535264366', 'key': 'x'}], 'id': '11567102051', 'key': 'feat_with_var', + 'delivery_rules': [], + 'experiment_rules': [], }, { 'experimentIds': [], @@ -770,6 +784,8 @@ def setUp(self, config_dict='config_dict'): 'variables': [], 'id': '11567102052', 'key': 'feat2', + 'delivery_rules': [], + 'experiment_rules': [], }, { 'experimentIds': ['1323241599'], @@ -777,6 +793,8 @@ def setUp(self, config_dict='config_dict'): 'variables': [{'defaultValue': '10', 'type': 'integer', 'id': '11535264367', 'key': 'z'}], 'id': '11567102053', 'key': 'feat2_with_var', + 'delivery_rules': [], + 'experiment_rules': [], }, ], 'experiments': [ diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index cf3640b2..b89c3a26 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -28,7 +28,6 @@ def setUp(self): self.expected_config = { 'sdk_key': None, 'environment_key': None, - 'delivery_rules': [], 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experiment_ids': ['111127'], 'id': '111095'}], 'audiences': [ @@ -470,6 +469,114 @@ def setUp(self): 'audiences': None } }, + 'delivery_rules': [], + 'experiment_rules': [ + { + 'id': '111127', + 'key': 'test_experiment', + 'variations_map': { + 'control': { + 'id': '111128', + 'key': 'control', + 'feature_enabled': False, + 'variables_map': { + 'is_working': { + 'id': '127', + 'key': 'is_working', + 'type': 'boolean', + 'value': 'true' + }, + 'environment': { + 'id': '128', + 'key': 'environment', + 'type': 'string', + 'value': 'devel' + }, + 'cost': { + 'id': '129', + 'key': 'cost', + 'type': 'double', + 'value': '10.99' + }, + 'count': { + 'id': '130', + 'key': 'count', + 'type': 'integer', + 'value': '999' + }, + 'variable_without_usage': { + 'id': '131', + 'key': 'variable_without_usage', + 'type': 'integer', + 'value': '45' + }, + 'object': { + 'id': '132', + 'key': 'object', + 'type': 'json', + 'value': '{"test": 12}' + }, + 'true_object': { + 'id': '133', + 'key': 'true_object', + 'type': 'json', + 'value': '{"true_test": 23.54}' + } + } + }, + 'variation': { + 'id': '111129', + 'key': 'variation', + 'feature_enabled': True, + 'variables_map': { + 'is_working': { + 'id': '127', + 'key': 'is_working', + 'type': 'boolean', + 'value': 'true' + }, + 'environment': { + 'id': '128', + 'key': 'environment', + 'type': 'string', + 'value': 'staging' + }, + 'cost': { + 'id': '129', + 'key': 'cost', + 'type': 'double', + 'value': '10.02' + }, + 'count': { + 'id': '130', + 'key': 'count', + 'type': 'integer', + 'value': '4243' + }, + 'variable_without_usage': { + 'id': '131', + 'key': 'variable_without_usage', + 'type': 'integer', + 'value': '45' + }, + 'object': { + 'id': '132', + 'key': 'object', + 'type': 'json', + 'value': '{"test": 123}' + }, + 'true_object': { + 'id': '133', + 'key': 'true_object', + 'type': 'json', + 'value': '{"true_test": 1.4}' + } + } + } + }, + 'audiences': None + } + ], 'id': '91111', 'key': 'test_feature_in_experiment' }, @@ -509,6 +616,8 @@ def setUp(self): 'experiments_map': { }, + 'delivery_rules': [], + 'experiment_rules': [], 'id': '91112', 'key': 'test_feature_in_rollout' }, @@ -541,6 +650,28 @@ def setUp(self): 'audiences': None } }, + 'delivery_rules': [], + 'experiment_rules': [ + { + 'id': '32222', + 'key': 'group_exp_1', + 'variations_map': { + 'group_exp_1_control': { + 'id': '28901', + 'key': 'group_exp_1_control', + 'feature_enabled': None, + 'variables_map': {} + }, + 'group_exp_1_variation': { + 'id': '28902', + 'key': 'group_exp_1_variation', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': None + } + ], 'id': '91113', 'key': 'test_feature_in_group' }, @@ -573,6 +704,28 @@ def setUp(self): 'audiences': None } }, + 'delivery_rules': [], + 'experiment_rules': [ + { + 'id': '32223', + 'key': 'group_exp_2', + 'variations_map': { + 'group_exp_2_control': { + 'id': '28905', + 'key': 'group_exp_2_control', + 'feature_enabled': None, + 'variables_map': {} + }, + 'group_exp_2_variation': { + 'id': '28906', + 'key': 'group_exp_2_variation', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': None + } + ], 'id': '91114', 'key': 'test_feature_in_experiment_and_rollout' }, @@ -627,6 +780,48 @@ def setUp(self): 'audiences': 'OR "Test attribute users 3"' } }, + 'delivery_rules': [], + 'experiment_rules': [ + { + 'id': '42222', + 'key': 'group_2_exp_1', + 'variations_map': { + 'var_1': { + 'id': '38901', + 'key': 'var_1', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + }, + { + 'id': '42223', + 'key': 'group_2_exp_2', + 'variations_map': { + 'var_1': { + 'id': '38905', + 'key': 'var_1', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + }, + { + 'id': '42224', + 'key': 'group_2_exp_3', + 'variations_map': { + 'var_1': { + 'id': '38906', + 'key': 'var_1', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + } + ], 'id': '91115', 'key': 'test_feature_in_exclusion_group' }, @@ -681,6 +876,48 @@ def setUp(self): 'audiences': 'OR "Test attribute users 3"' } }, + 'delivery_rules': [], + 'experiment_rules': [ + { + 'id': '111134', + 'key': 'test_experiment3', + 'variations_map': { + 'control': { + 'id': '222239', + 'key': 'control', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + }, + { + 'id': '111135', + 'key': 'test_experiment4', + 'variations_map': { + 'control': { + 'id': '222240', + 'key': 'control', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + }, + { + 'id': '111136', + 'key': 'test_experiment5', + 'variations_map': { + 'control': { + 'id': '222241', + 'key': 'control', + 'feature_enabled': None, + 'variables_map': {} + } + }, + 'audiences': 'OR "Test attribute users 3"' + } + ], 'id': '91116', 'key': 'test_feature_in_multiple_experiments' } @@ -1482,32 +1719,6 @@ def test_optimizely_audience_conversion(self): self.assertEqual(len(config_service.audiences), TOTAL_AUDEINCES_ONCE_MERGED) - def test__get_delivery_rules(self): - config_dict = self.typed_audiences_config - - proj_conf = project_config.ProjectConfig( - json.dumps(config_dict), - logger=None, - error_handler=None - ) - - config_service = optimizely_config.OptimizelyConfigService(proj_conf) - - config = config_service.get_config() - - for rule in config.get_delivery_rules(): - self.assertIsInstance(rule, optimizely_config.OptimizelyAudience) - - experiments_list = [] - - for rollout in config_service.rollouts: - experiments = rollout.get('experiments_map') - if experiments: - for exp in experiments: - experiments_list.append(exp) - - self.assertEqual(len(config.get_delivery_rules()), len(experiments_list)) - def test_get_variations_from_experiments_map(self): config_dict = self.typed_audiences_config From 8187f6fed9a92e0a286881054b08152fbf7fd948 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:07:34 -0400 Subject: [PATCH 41/59] Formatter on new changes --- optimizely/lib/pymmh3.py | 64 ++++++++++++++++----------------- optimizely/optimizely_config.py | 7 ++-- tests/base.py | 3 +- tests/test_optimizely_config.py | 6 ++-- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/optimizely/lib/pymmh3.py b/optimizely/lib/pymmh3.py index 0f107709..4997de21 100755 --- a/optimizely/lib/pymmh3.py +++ b/optimizely/lib/pymmh3.py @@ -127,25 +127,25 @@ def fmix(k): for block_start in xrange(0, nblocks * 8, 8): # ??? big endian? k1 = ( - key[2 * block_start + 7] << 56 - | key[2 * block_start + 6] << 48 - | key[2 * block_start + 5] << 40 - | key[2 * block_start + 4] << 32 - | key[2 * block_start + 3] << 24 - | key[2 * block_start + 2] << 16 - | key[2 * block_start + 1] << 8 - | key[2 * block_start + 0] + key[2 * block_start + 7] << 56 | + key[2 * block_start + 6] << 48 | + key[2 * block_start + 5] << 40 | + key[2 * block_start + 4] << 32 | + key[2 * block_start + 3] << 24 | + key[2 * block_start + 2] << 16 | + key[2 * block_start + 1] << 8 | + key[2 * block_start + 0] ) k2 = ( - key[2 * block_start + 15] << 56 - | key[2 * block_start + 14] << 48 - | key[2 * block_start + 13] << 40 - | key[2 * block_start + 12] << 32 - | key[2 * block_start + 11] << 24 - | key[2 * block_start + 10] << 16 - | key[2 * block_start + 9] << 8 - | key[2 * block_start + 8] + key[2 * block_start + 15] << 56 | + key[2 * block_start + 14] << 48 | + key[2 * block_start + 13] << 40 | + key[2 * block_start + 12] << 32 | + key[2 * block_start + 11] << 24 | + key[2 * block_start + 10] << 16 | + key[2 * block_start + 9] << 8 | + key[2 * block_start + 8] ) k1 = (c1 * k1) & 0xFFFFFFFFFFFFFFFF @@ -258,31 +258,31 @@ def fmix(h): # body for block_start in xrange(0, nblocks * 16, 16): k1 = ( - key[block_start + 3] << 24 - | key[block_start + 2] << 16 - | key[block_start + 1] << 8 - | key[block_start + 0] + key[block_start + 3] << 24 | + key[block_start + 2] << 16 | + key[block_start + 1] << 8 | + key[block_start + 0] ) k2 = ( - key[block_start + 7] << 24 - | key[block_start + 6] << 16 - | key[block_start + 5] << 8 - | key[block_start + 4] + key[block_start + 7] << 24 | + key[block_start + 6] << 16 | + key[block_start + 5] << 8 | + key[block_start + 4] ) k3 = ( - key[block_start + 11] << 24 - | key[block_start + 10] << 16 - | key[block_start + 9] << 8 - | key[block_start + 8] + key[block_start + 11] << 24 | + key[block_start + 10] << 16 | + key[block_start + 9] << 8 | + key[block_start + 8] ) k4 = ( - key[block_start + 15] << 24 - | key[block_start + 14] << 16 - | key[block_start + 13] << 8 - | key[block_start + 12] + key[block_start + 15] << 24 | + key[block_start + 14] << 16 | + key[block_start + 13] << 8 | + key[block_start + 12] ) k1 = (c1 * k1) & 0xFFFFFFFF diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 39b6286a..d942491a 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -271,7 +271,8 @@ def stringify_conditions(self, conditions, audiences_map): if type(conditions[i]) == list: # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' + condition + ' ' + conditions_str += '(' + \ + self.stringify_conditions(conditions[i], audiences_map) + ') ' + condition + ' ' else: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: @@ -443,7 +444,7 @@ def _get_features_map(self, experiments_id_map): """ TODO - - + delivery Rules : Filter rollouts based on the feature.rolloutID and take the first item in the filtered list. the delivery rules are then that rollout experiments. @@ -462,7 +463,7 @@ def _get_features_map(self, experiments_id_map): exp_map[optly_exp.key] = optly_exp # Append the optly experiment to experiment rules - # The optly experiment has already been updated + # The optly experiment has already been updated # during the experiment maps creation. experiment_rules.append(optly_exp) diff --git a/tests/base.py b/tests/base.py index d4c010d0..5efe1dcb 100644 --- a/tests/base.py +++ b/tests/base.py @@ -768,7 +768,8 @@ def setUp(self, config_dict='config_dict'): 'projectId': '11624721371', 'variables': [], 'featureFlags': [ - {'experimentIds': [], 'rolloutId': '11551226731', 'variables': [], 'id': '11477755619', 'key': 'feat', 'delivery_rules': [], 'experiment_rules': [],}, + {'experimentIds': [], 'rolloutId': '11551226731', 'variables': [], 'id': '11477755619', + 'key': 'feat', 'delivery_rules': [], 'experiment_rules': [], }, { 'experimentIds': ['11564051718'], 'rolloutId': '11638870867', diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index b89c3a26..53248eb2 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -708,7 +708,7 @@ def setUp(self): 'experiment_rules': [ { 'id': '32223', - 'key': 'group_exp_2', + 'key': 'group_exp_2', 'variations_map': { 'group_exp_2_control': { 'id': '28905', @@ -723,7 +723,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': None + 'audiences': None } ], 'id': '91114', @@ -885,7 +885,7 @@ def setUp(self): 'control': { 'id': '222239', 'key': 'control', - 'feature_enabled': None, + 'feature_enabled': None, 'variables_map': {} } }, From ba3bbefdc340bc7810d8e32814731b14c1ec6473 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:13:46 -0400 Subject: [PATCH 42/59] Remove TODO and other dev comments --- optimizely/optimizely_config.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index d942491a..c88745da 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -442,18 +442,6 @@ def _get_features_map(self, experiments_id_map): for feature in self.feature_flags: - """ - TODO - - - delivery Rules : Filter rollouts based on the feature.rolloutID and take the - first item in the filtered list. the delivery rules are then that rollout - experiments. - - experiment Rules: feature.experimentIDs a list of experiments based on the feature.id matching - the experimentID. - - """ - delivery_rules = self._get_delivery_rules(self.rollouts, feature.get('rolloutId')) experiment_rules = [] @@ -461,10 +449,7 @@ def _get_features_map(self, experiments_id_map): for experiment_id in feature.get('experimentIds', []): optly_exp = experiments_id_map[experiment_id] exp_map[optly_exp.key] = optly_exp - - # Append the optly experiment to experiment rules - # The optly experiment has already been updated - # during the experiment maps creation. + experiment_rules.append(optly_exp) variables_map = self.feature_key_variable_key_to_variable_map[feature['key']] From 2567bac4ec103d11ef0694133eb4eb2e29641b0d Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:14:21 -0400 Subject: [PATCH 43/59] Remove TODO and other dev comments --- optimizely/optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index c88745da..2db02177 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -449,7 +449,6 @@ def _get_features_map(self, experiments_id_map): for experiment_id in feature.get('experimentIds', []): optly_exp = experiments_id_map[experiment_id] exp_map[optly_exp.key] = optly_exp - experiment_rules.append(optly_exp) variables_map = self.feature_key_variable_key_to_variable_map[feature['key']] From d401c7274c9ca280a309fef611fc2a37ab1f9fd5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:19:00 -0400 Subject: [PATCH 44/59] Remove unused imoprt --- optimizely/optimizely_config.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 2db02177..5af4a772 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -15,8 +15,6 @@ from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig -from optimizely import project_config - class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, From 6b4a00decc9505a33316d487413b9ec17caece94 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 11:24:03 -0400 Subject: [PATCH 45/59] Added nl after imports to follow Flake8 --- optimizely/optimizely_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 5af4a772..4fb6f37c 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -16,6 +16,7 @@ from .project_config import ProjectConfig + class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, sdk_key=None, environment_key=None, attributes=None, events=None, From a1ed815dc8cf936fc1bb4bd35c91e7f876475193 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 12:52:15 -0400 Subject: [PATCH 46/59] Add except for if incorrect type used in lookup_name_from_id() to prevent errors --- optimizely/optimizely_config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 4fb6f37c..77f1c75b 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -228,6 +228,8 @@ def lookup_name_from_id(self, audience_id, audiences_map): name = audiences_map[audience_id] except KeyError: name = audience_id + except TypeError: + name = "" return name From ec4218a3458b55a3564b26e203f586e09d7994c5 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 12:55:58 -0400 Subject: [PATCH 47/59] Correct except to assign audience_id instead of blank string --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 77f1c75b..6ee7e972 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -229,7 +229,7 @@ def lookup_name_from_id(self, audience_id, audiences_map): except KeyError: name = audience_id except TypeError: - name = "" + name = audience_id return name From 508f82f3f481823245c0f490be10bbd772de666e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 13:12:32 -0400 Subject: [PATCH 48/59] Casting name to string in case non string type for testing --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 6ee7e972..fe53098d 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -225,7 +225,7 @@ def lookup_name_from_id(self, audience_id, audiences_map): ''' name = "" try: - name = audiences_map[audience_id] + name = str(audiences_map[audience_id]) except KeyError: name = audience_id except TypeError: From 26ad430ec1d53a58ddbb289b7c00e77c0d9ec240 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 14:47:54 -0400 Subject: [PATCH 49/59] Added test and modification to account for scenario when NOT is followed by another list of ids and an operand resulting in a list of conditions containing one operand and 1 list. --- optimizely/optimizely_config.py | 9 +++++---- tests/test_optimizely_config.py | 36 ++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index fe53098d..10bfb3b0 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,6 +12,7 @@ # limitations under the License. import copy +from typing import List from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig @@ -225,11 +226,9 @@ def lookup_name_from_id(self, audience_id, audiences_map): ''' name = "" try: - name = str(audiences_map[audience_id]) + name = audiences_map[audience_id] except KeyError: name = audience_id - except TypeError: - name = audience_id return name @@ -254,9 +253,11 @@ def stringify_conditions(self, conditions, audiences_map): return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' if length == 2: - if conditions[0] in ARGS: + if conditions[0] in ARGS and not isinstance(conditions[1], List): audience_name = self.lookup_name_from_id(conditions[1], audiences_map) return conditions[0].upper() + ' "' + audience_name + '"' + elif conditions[0] == 'not' and isinstance(conditions[1], List): + return conditions[0].upper() + ' (' + self.stringify_conditions(conditions[1], audiences_map) + ')' else: name1 = self.lookup_name_from_id(conditions[0], audiences_map) name2 = self.lookup_name_from_id(conditions[1], audiences_map) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 53248eb2..ed0d1f5d 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1568,7 +1568,7 @@ def test_stringify_conditions_one_id(self): def test_stringify_conditions_no_condition_word(self): ''' - Test to confirm array of ID's is handled as OR by stringify_conditions functions + Test to confirm list of ID's is handled as OR by stringify_conditions functions ''' experiment = {'audienceConditions': ['3468206642', '3988293898'], @@ -1740,3 +1740,37 @@ def test_get_variations_from_experiments_map(self): self.assertEqual(variation.key, 'all_traffic_variation') else: self.assertEqual(variation.key, 'no_traffic_variation') + + def test_stringify_conditions_one_condition_and_list(self): + ''' + Test to confirm list of IDs and operand preceded by NOT + is handeld correctly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['not', ['and', '3468206642', '3988293898']], + 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map = { + '3468206642': 'us', + '3988293898': 'female' + } + + config = optimizely_config.OptimizelyConfig( + revision='101', + experiments_map={}, + features_map={}, + environment_key='TestEnvironmentKey', + attributes={}, + events={}, + audiences=None + ) + + config_service = optimizely_config.OptimizelyConfigService(config) + + result = config_service.stringify_conditions(audience_conditions, audiences_map) + + expected_result = 'NOT ("us" AND "female")' + + self.assertEqual(result, expected_result) From 1f35dda019ca9460479287b091aaf1f5af3faee2 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 12 Jul 2021 15:01:03 -0400 Subject: [PATCH 50/59] Update from isinstance to type and change List to list for python 3.4 --- optimizely/optimizely_config.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 10bfb3b0..cd6fef28 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,7 +12,6 @@ # limitations under the License. import copy -from typing import List from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig @@ -253,10 +252,10 @@ def stringify_conditions(self, conditions, audiences_map): return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' if length == 2: - if conditions[0] in ARGS and not isinstance(conditions[1], List): + if conditions[0] in ARGS and not type(conditions[1]) == list: audience_name = self.lookup_name_from_id(conditions[1], audiences_map) return conditions[0].upper() + ' "' + audience_name + '"' - elif conditions[0] == 'not' and isinstance(conditions[1], List): + elif conditions[0] == 'not' and type(conditions[1]) == list: return conditions[0].upper() + ' (' + self.stringify_conditions(conditions[1], audiences_map) + ')' else: name1 = self.lookup_name_from_id(conditions[0], audiences_map) From a4e497ff172f15ee5e754470f3fecd47125d2b87 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 13 Jul 2021 09:24:49 -0400 Subject: [PATCH 51/59] Simplified stringifyConditions algorithm to be more readable and less conditions. --- optimizely/optimizely_config.py | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index cd6fef28..4ec4ad78 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -241,49 +241,35 @@ def stringify_conditions(self, conditions, audiences_map): list of conditions. ''' ARGS = ConditionOperatorTypes.operators - condition = "" + condition = 'OR' conditions_str = "" length = len(conditions) if length == 0: return if length == 1: - # Lookup ID and replace with name return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' - if length == 2: - if conditions[0] in ARGS and not type(conditions[1]) == list: - audience_name = self.lookup_name_from_id(conditions[1], audiences_map) - return conditions[0].upper() + ' "' + audience_name + '"' - elif conditions[0] == 'not' and type(conditions[1]) == list: - return conditions[0].upper() + ' (' + self.stringify_conditions(conditions[1], audiences_map) + ')' - else: - name1 = self.lookup_name_from_id(conditions[0], audiences_map) - name2 = self.lookup_name_from_id(conditions[1], audiences_map) - return ('"' + name1 + '" OR "' + name2 + '"') - - if length > 2: + if length > 1: for i in range(length): if conditions[i] in ARGS: condition = conditions[i].upper() else: - if condition == "": - condition = 'OR' if type(conditions[i]) == list: - # If the next item is a list, recursively call function on list if i + 1 < length: - conditions_str += '(' + \ - self.stringify_conditions(conditions[i], audiences_map) + ') ' + condition + ' ' + conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' else: - conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ')' + conditions_str += condition + \ + ' (' + self.stringify_conditions(conditions[i], audiences_map) + ')' else: - # Handle IDs here - Lookup name based on ID audience_name = self.lookup_name_from_id(conditions[i], audiences_map) if audience_name is not None: - if i + 1 < length: + if i + 1 < length - 1: conditions_str += '"' + audience_name + '" ' + condition + ' ' + elif i + 1 == length: + conditions_str += condition + ' "' + audience_name + '"' else: - conditions_str += '"' + audience_name + '"' + conditions_str += '"' + audience_name + '" ' return conditions_str From bf92fd7e3a8c38f6a82bf8eb6989594fd0a7fdc9 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 13 Jul 2021 16:12:41 -0400 Subject: [PATCH 52/59] Updated changes - reuse test logic, modify stringify to proper handle one operand and one ID, remove unneed get for delivery_rules. --- optimizely/optimizely_config.py | 27 ++-- tests/test_optimizely_config.py | 276 +++++++++----------------------- 2 files changed, 82 insertions(+), 221 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 4ec4ad78..ca853b85 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -79,21 +79,13 @@ def get_audiences(self): """ return self.audiences - def get_delivery_rules(self): - """ Get the delivery rules list associated with OptimizelyConfig - - Returns: - List of OptimizelyExperiments as DeliveryRules from Rollouts. - """ - return self.delivery_rules - class OptimizelyExperiment(object): - def __init__(self, id, key, variations_map): + def __init__(self, id, key, variations_map, audiences=''): self.id = id self.key = key self.variations_map = variations_map - self.audiences = "" + self.audiences = audiences class OptimizelyFeature(object): @@ -207,13 +199,13 @@ def replace_ids_with_names(self, conditions, audiences_map): Returns: a string of conditions with id's swapped with names - or None if no conditions found. + or empty string if no conditions found. ''' if conditions is not None: return self.stringify_conditions(conditions, audiences_map) else: - return None + return '' def lookup_name_from_id(self, audience_id, audiences_map): ''' @@ -221,9 +213,9 @@ def lookup_name_from_id(self, audience_id, audiences_map): Returns: The name corresponding to the ID - or None if not found. + or '' if not found. ''' - name = "" + name = None try: name = audiences_map[audience_id] except KeyError: @@ -242,14 +234,15 @@ def stringify_conditions(self, conditions, audiences_map): ''' ARGS = ConditionOperatorTypes.operators condition = 'OR' - conditions_str = "" + conditions_str = '' length = len(conditions) if length == 0: return if length == 1: return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' - + if length == 2 and conditions[0] in ARGS and type(conditions[1]) is not list: + return '"' + self.lookup_name_from_id(conditions[1], audiences_map) + '"' if length > 1: for i in range(length): if conditions[i] in ARGS: @@ -408,7 +401,7 @@ def _get_experiments_maps(self): ) # Updating each OptimizelyExperiment audiences = self.replace_ids_with_names(exp.get('audienceConditions', []), audiences_map) - optly_exp.audiences = audiences + optly_exp.audiences = audiences or '' experiments_key_map[exp['key']] = optly_exp experiments_id_map[exp['id']] = optly_exp diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index ed0d1f5d..1e9e9210 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -72,7 +72,7 @@ def setUp(self): }, 'id': '111133', 'key': 'test_experiment2', - 'audiences': None + 'audiences': '' }, 'test_experiment': { 'variations_map': { @@ -177,7 +177,7 @@ def setUp(self): }, 'id': '111127', 'key': 'test_experiment', - 'audiences': None + 'audiences': '' }, 'group_exp_1': { 'variations_map': { @@ -200,7 +200,7 @@ def setUp(self): }, 'id': '32222', 'key': 'group_exp_1', - 'audiences': None + 'audiences': '' }, 'group_exp_2': { 'variations_map': { @@ -223,7 +223,7 @@ def setUp(self): }, 'id': '32223', 'key': 'group_exp_2', - 'audiences': None + 'audiences': '' }, 'group_2_exp_1': { 'variations_map': { @@ -238,7 +238,7 @@ def setUp(self): }, 'id': '42222', 'key': 'group_2_exp_1', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'group_2_exp_2': { 'variations_map': { @@ -253,7 +253,7 @@ def setUp(self): }, 'id': '42223', 'key': 'group_2_exp_2', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'group_2_exp_3': { 'variations_map': { @@ -268,7 +268,7 @@ def setUp(self): }, 'id': '42224', 'key': 'group_2_exp_3', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'test_experiment3': { 'variations_map': { @@ -283,7 +283,7 @@ def setUp(self): }, 'id': '111134', 'key': 'test_experiment3', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'test_experiment4': { 'variations_map': { @@ -298,7 +298,7 @@ def setUp(self): }, 'id': '111135', 'key': 'test_experiment4', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'test_experiment5': { 'variations_map': { @@ -313,7 +313,7 @@ def setUp(self): }, 'id': '111136', 'key': 'test_experiment5', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' } }, 'features_map': { @@ -466,7 +466,7 @@ def setUp(self): }, 'id': '111127', 'key': 'test_experiment', - 'audiences': None + 'audiences': '' } }, 'delivery_rules': [], @@ -574,7 +574,7 @@ def setUp(self): } } }, - 'audiences': None + 'audiences': '' } ], 'id': '91111', @@ -647,7 +647,7 @@ def setUp(self): }, 'id': '32222', 'key': 'group_exp_1', - 'audiences': None + 'audiences': '' } }, 'delivery_rules': [], @@ -669,7 +669,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': None + 'audiences': '' } ], 'id': '91113', @@ -701,7 +701,7 @@ def setUp(self): }, 'id': '32223', 'key': 'group_exp_2', - 'audiences': None + 'audiences': '' } }, 'delivery_rules': [], @@ -723,7 +723,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': None + 'audiences': '' } ], 'id': '91114', @@ -747,7 +747,7 @@ def setUp(self): }, 'id': '42222', 'key': 'group_2_exp_1', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'group_2_exp_2': { 'variations_map': { @@ -762,7 +762,7 @@ def setUp(self): }, 'id': '42223', 'key': 'group_2_exp_2', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'group_2_exp_3': { 'variations_map': { @@ -777,7 +777,7 @@ def setUp(self): }, 'id': '42224', 'key': 'group_2_exp_3', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' } }, 'delivery_rules': [], @@ -793,7 +793,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, { 'id': '42223', @@ -806,7 +806,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, { 'id': '42224', @@ -819,7 +819,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' } ], 'id': '91115', @@ -843,7 +843,7 @@ def setUp(self): }, 'id': '111134', 'key': 'test_experiment3', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'test_experiment4': { 'variations_map': { @@ -858,7 +858,7 @@ def setUp(self): }, 'id': '111135', 'key': 'test_experiment4', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, 'test_experiment5': { 'variations_map': { @@ -873,7 +873,7 @@ def setUp(self): }, 'id': '111136', 'key': 'test_experiment5', - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' } }, 'delivery_rules': [], @@ -889,7 +889,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, { 'id': '111135', @@ -902,7 +902,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' }, { 'id': '111136', @@ -915,7 +915,7 @@ def setUp(self): 'variables_map': {} } }, - 'audiences': 'OR "Test attribute users 3"' + 'audiences': '"Test attribute users 3"' } ], 'id': '91116', @@ -1366,23 +1366,16 @@ def test_get_audiences(self): self.assertEqual(len(config.get_audiences()), len(config_service.audiences)) - def test_stringify_conditions_non_matching_ids(self): - ''' - Test to confirm Non-matching ID's are handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['and', ['or', '3468206642', '3988293898'], [ - 'or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643']], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} - - audience_conditions = experiment.get('audienceConditions') - + def get_config_and_audiences_map_for_test(self): audiences_map = { - '3468206642': 'us', - '3988293898': 'female', - '3988293899': 'male', - '3468206646': 'them', - '3468206647': 'anyone' + '1': 'us', + '2': 'female', + '3': 'adult', + '4': 'them', + '5': 'anyone', + '11': 'fr', + '12': 'male', + '13': 'kid' } config = optimizely_config.OptimizelyConfig( @@ -1394,6 +1387,19 @@ def test_stringify_conditions_non_matching_ids(self): events={}, audiences=None ) + return audiences_map, config + + def test_stringify_conditions_non_matching_ids(self): + ''' + Test to confirm Non-matching ID's are handled properly by stringify_conditions functions + ''' + + experiment = {'audienceConditions': ['and', ['or', '1', '2'], [ + 'or', '12', '4', '5', '3468206644', '3468206643']]} + + audience_conditions = experiment.get('audienceConditions') + + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1408,25 +1414,11 @@ def test_stringify_conditions_or(self): Test to confirm OR is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['or', '3468206642', '3988293898'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['or', '1', '2']} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - '3988293898': 'female' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1441,26 +1433,11 @@ def test_stringify_conditions_and(self): Test to confirm AND is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['and', '3468206642', '3988293898', '3988346543'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['and', '1', '2', '3']} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - '3988293898': 'female', - '3988346543': 'adult' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1475,30 +1452,17 @@ def test_stringify_conditions_not_one_id(self): Test to confirm Not and one ID is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['not', '3468206642'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['not', '1']} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) result = config_service.stringify_conditions(audience_conditions, audiences_map) - expected_result = 'NOT "us"' + expected_result = '"us"' self.assertEqual(result, expected_result) @@ -1507,30 +1471,17 @@ def test_stringify_conditions_and_one_id(self): Test to confirm AND and one ID is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['and', '3468206642'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['and', '1']} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) result = config_service.stringify_conditions(audience_conditions, audiences_map) - expected_result = 'AND "us"' + expected_result = '"us"' self.assertEqual(result, expected_result) @@ -1539,24 +1490,11 @@ def test_stringify_conditions_one_id(self): Test to confirm a single ID is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['3468206642'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['1'], } audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1571,25 +1509,11 @@ def test_stringify_conditions_no_condition_word(self): Test to confirm list of ID's is handled as OR by stringify_conditions functions ''' - experiment = {'audienceConditions': ['3468206642', '3988293898'], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['1', '2']} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - '3988293898': 'female' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1604,29 +1528,11 @@ def test_stringify_conditions_and_or_and(self): Test to confirm [and[or][and]] is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['and', ['or', '1', ['and', '2', '3']], ['and', '11', ['or', '12', '13']]], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['and', ['or', '1', ['and', '2', '3']], ['and', '11', ['or', '12', '13']]]} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '1': 'us', - '2': 'female', - '3': 'adult', - '11': 'fr', - '12': 'male', - '13': 'kid' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1641,22 +1547,11 @@ def test_stringify_conditions_empty_string(self): Test to confirm empty string is handled properly by stringify_conditions functions ''' - experiment = {'audienceConditions': '', - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ''} audience_conditions = experiment.get('audienceConditions') - audiences_map = {} - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1669,7 +1564,7 @@ def test_stringify_conditions_empty_string(self): def test_replace_ids_with_names(self): ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' - audience_conditions = ['and', ['or', '432', '321'], '543'] + audience_conditions = ['and', ['or', '1', '2'], '3'] optly_experiment = optimizely_config.OptimizelyExperiment( '12345', @@ -1677,20 +1572,7 @@ def test_replace_ids_with_names(self): variations_map={} ) - audiences_map = { - '432': 'us', - '321': 'female', - '543': 'adult' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={optly_experiment.id: optly_experiment}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) @@ -1747,25 +1629,11 @@ def test_stringify_conditions_one_condition_and_list(self): is handeld correctly by stringify_conditions functions ''' - experiment = {'audienceConditions': ['not', ['and', '3468206642', '3988293898']], - 'audienceIds': ['0'], 'forcedVariations': {}, 'id': '1323241598'} + experiment = {'audienceConditions': ['not', ['and', '1', '2']]} audience_conditions = experiment.get('audienceConditions') - audiences_map = { - '3468206642': 'us', - '3988293898': 'female' - } - - config = optimizely_config.OptimizelyConfig( - revision='101', - experiments_map={}, - features_map={}, - environment_key='TestEnvironmentKey', - attributes={}, - events={}, - audiences=None - ) + audiences_map, config = self.get_config_and_audiences_map_for_test() config_service = optimizely_config.OptimizelyConfigService(config) From 2d6def34c61bd1a375d48e19d400c870c89d2551 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 08:58:56 -0400 Subject: [PATCH 53/59] Change to single test to run all cases for stringify. --- optimizely/optimizely_config.py | 14 +- tests/test_optimizely_config.py | 253 +++++--------------------------- 2 files changed, 47 insertions(+), 220 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index ca853b85..d8308626 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -238,11 +238,17 @@ def stringify_conditions(self, conditions, audiences_map): length = len(conditions) if length == 0: - return + return '' if length == 1: return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' - if length == 2 and conditions[0] in ARGS and type(conditions[1]) is not list: - return '"' + self.lookup_name_from_id(conditions[1], audiences_map) + '"' + if length == 2 and conditions[0] in ARGS and \ + type(conditions[1]) is not list and \ + conditions[1] not in ARGS: + if conditions[0] != "not": + return '"' + self.lookup_name_from_id(conditions[1], audiences_map) + '"' + else: + return conditions[0].upper() + \ + ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"' if length > 1: for i in range(length): if conditions[i] in ARGS: @@ -264,7 +270,7 @@ def stringify_conditions(self, conditions, audiences_map): else: conditions_str += '"' + audience_name + '" ' - return conditions_str + return conditions_str or '' def get_config(self): """ Gets instance of OptimizelyConfig diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 1e9e9210..37c039ae 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1366,13 +1366,11 @@ def test_get_audiences(self): self.assertEqual(len(config.get_audiences()), len(config_service.audiences)) - def get_config_and_audiences_map_for_test(self): + def test_stringify_audience_conditions_all_cases(self): audiences_map = { '1': 'us', '2': 'female', '3': 'adult', - '4': 'them', - '5': 'anyone', '11': 'fr', '12': 'male', '13': 'kid' @@ -1387,200 +1385,43 @@ def get_config_and_audiences_map_for_test(self): events={}, audiences=None ) - return audiences_map, config - def test_stringify_conditions_non_matching_ids(self): - ''' - Test to confirm Non-matching ID's are handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['and', ['or', '1', '2'], [ - 'or', '12', '4', '5', '3468206644', '3468206643']]} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '("us" OR "female") AND ("male" OR "them" OR "anyone" OR "3468206644" OR "3468206643")' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_or(self): - ''' - Test to confirm OR is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['or', '1', '2']} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us" OR "female"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_and(self): - ''' - Test to confirm AND is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['and', '1', '2', '3']} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us" AND "female" AND "adult"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_not_one_id(self): - ''' - Test to confirm Not and one ID is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['not', '1']} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_and_one_id(self): - ''' - Test to confirm AND and one ID is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['and', '1']} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_one_id(self): - ''' - Test to confirm a single ID is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['1'], } - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_no_condition_word(self): - ''' - Test to confirm list of ID's is handled as OR by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['1', '2']} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '"us" OR "female"' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_and_or_and(self): - ''' - Test to confirm [and[or][and]] is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['and', ['or', '1', ['and', '2', '3']], ['and', '11', ['or', '12', '13']]]} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "kid"))' - - self.assertEqual(result, expected_result) - - def test_stringify_conditions_empty_string(self): - ''' - Test to confirm empty string is handled properly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ''} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = None - - self.assertEqual(result, expected_result) - - def test_replace_ids_with_names(self): - ''' Test that OptimizelyExperiment updates with proper conditions for audiences ''' - - audience_conditions = ['and', ['or', '1', '2'], '3'] - - optly_experiment = optimizely_config.OptimizelyExperiment( - '12345', - 'test_update_experiment', - variations_map={} - ) - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - audiences = config_service.replace_ids_with_names(audience_conditions, audiences_map) - optly_experiment.audiences = audiences - expected_value = '("us" OR "female") AND "adult"' - - self.assertEqual(optly_experiment.audiences, expected_value) + audiences_input = [ + [], + ["or", "1", "2"], + ["and", "1", "2", "3"], + ["not", "1"], + ["or", "1"], + ["and", "1"], + ["1"], + ["1", "2"], + ["and", ["or", "1", "2"], "3"], + ["and", ["or", "1", ["and", "2", "3"]], ["and", "11", ["or", "12", "13"]]], + ["not", ["and", "1", "2"]], + ["or", "1", "100000"], + ["and", "and"] + ] + + audiences_output = [ + '', + '"us" OR "female"', + '"us" AND "female" AND "adult"', + 'NOT "us"', + '"us"', + '"us"', + '"us"', + '"us" OR "female"', + '("us" OR "female") AND "adult"', + '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "kid"))', + 'NOT ("us" AND "female")', + '"us" OR "100000"', + '' + ] + + for i in range(len(audiences_input)): + config_service = optimizely_config.OptimizelyConfigService(config) + result = config_service.stringify_conditions(audiences_input[i], audiences_map) + self.assertEqual(audiences_output[i], result) def test_optimizely_audience_conversion(self): ''' Test to confirm that audience conversion works and has expected output ''' @@ -1622,23 +1463,3 @@ def test_get_variations_from_experiments_map(self): self.assertEqual(variation.key, 'all_traffic_variation') else: self.assertEqual(variation.key, 'no_traffic_variation') - - def test_stringify_conditions_one_condition_and_list(self): - ''' - Test to confirm list of IDs and operand preceded by NOT - is handeld correctly by stringify_conditions functions - ''' - - experiment = {'audienceConditions': ['not', ['and', '1', '2']]} - - audience_conditions = experiment.get('audienceConditions') - - audiences_map, config = self.get_config_and_audiences_map_for_test() - - config_service = optimizely_config.OptimizelyConfigService(config) - - result = config_service.stringify_conditions(audience_conditions, audiences_map) - - expected_result = 'NOT ("us" AND "female")' - - self.assertEqual(result, expected_result) From bec791a0b80898f7544aa852b49718aaf2825fdb Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 09:03:03 -0400 Subject: [PATCH 54/59] Move config_service out of loop. --- tests/test_optimizely_config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 37c039ae..5167abd4 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1418,8 +1418,9 @@ def test_stringify_audience_conditions_all_cases(self): '' ] + config_service = optimizely_config.OptimizelyConfigService(config) + for i in range(len(audiences_input)): - config_service = optimizely_config.OptimizelyConfigService(config) result = config_service.stringify_conditions(audiences_input[i], audiences_map) self.assertEqual(audiences_output[i], result) From 286f178efd7fe40cd240053ca86229e3be3b7742 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 10:51:40 -0400 Subject: [PATCH 55/59] Added missed step in delivery rules. --- optimizely/optimizely_config.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index d8308626..c5a3fdf5 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -479,6 +479,8 @@ def _get_delivery_rules(self, rollouts, rollout_id): audiences = self.replace_ids_with_names(experiment.get('audienceConditions', []), audiences_map) optly_exp.audiences = audiences + delivery_rules.append(optly_exp) + return delivery_rules def _get_attributes_list(self, attributes): From c3faba391bc46fedb7b14cf1e1376e1e37050406 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 14:47:16 -0400 Subject: [PATCH 56/59] Update to properly refect datafile examples. --- optimizely/optimizely_config.py | 1 + tests/base.py | 20 +------------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index c5a3fdf5..ebbd7521 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,6 +12,7 @@ # limitations under the License. import copy +from os import pread from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig diff --git a/tests/base.py b/tests/base.py index 5efe1dcb..05127caf 100644 --- a/tests/base.py +++ b/tests/base.py @@ -485,8 +485,6 @@ def setUp(self, config_dict='config_dict'): 'subType': 'json'}, {'id': '133', 'key': 'true_object', 'defaultValue': '{"true_test": 23.54}', 'type': 'json'}, ], - 'delivery_rules': [], - 'experiment_rules': [], }, { 'id': '91112', @@ -501,8 +499,6 @@ def setUp(self, config_dict='config_dict'): {'id': '136', 'key': 'object', 'defaultValue': '{"field": 1}', 'type': 'string', 'subType': 'json'}, ], - 'delivery_rules': [], - 'experiment_rules': [], }, { 'id': '91113', @@ -510,8 +506,6 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['32222'], 'rolloutId': '', 'variables': [], - 'delivery_rules': [], - 'experiment_rules': [], }, { 'id': '91114', @@ -519,8 +513,6 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['32223'], 'rolloutId': '211111', 'variables': [], - 'delivery_rules': [], - 'experiment_rules': [], }, { 'id': '91115', @@ -528,8 +520,6 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['42222', '42223', '42224'], 'rolloutId': '211111', 'variables': [], - 'delivery_rules': [], - 'experiment_rules': [], }, { 'id': '91116', @@ -537,8 +527,6 @@ def setUp(self, config_dict='config_dict'): 'experimentIds': ['111134', '111135', '111136'], 'rolloutId': '211111', 'variables': [], - 'delivery_rules': [], - 'experiment_rules': [], }, ], } @@ -769,15 +757,13 @@ def setUp(self, config_dict='config_dict'): 'variables': [], 'featureFlags': [ {'experimentIds': [], 'rolloutId': '11551226731', 'variables': [], 'id': '11477755619', - 'key': 'feat', 'delivery_rules': [], 'experiment_rules': [], }, + 'key': 'feat'}, { 'experimentIds': ['11564051718'], 'rolloutId': '11638870867', 'variables': [{'defaultValue': 'x', 'type': 'string', 'id': '11535264366', 'key': 'x'}], 'id': '11567102051', 'key': 'feat_with_var', - 'delivery_rules': [], - 'experiment_rules': [], }, { 'experimentIds': [], @@ -785,8 +771,6 @@ def setUp(self, config_dict='config_dict'): 'variables': [], 'id': '11567102052', 'key': 'feat2', - 'delivery_rules': [], - 'experiment_rules': [], }, { 'experimentIds': ['1323241599'], @@ -794,8 +778,6 @@ def setUp(self, config_dict='config_dict'): 'variables': [{'defaultValue': '10', 'type': 'integer', 'id': '11535264367', 'key': 'z'}], 'id': '11567102053', 'key': 'feat2_with_var', - 'delivery_rules': [], - 'experiment_rules': [], }, ], 'experiments': [ From 199a35f3fd78530d5f311c5a7a970bb3a69b103a Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 14:50:04 -0400 Subject: [PATCH 57/59] Remove unused import --- optimizely/optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index ebbd7521..c5a3fdf5 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -12,7 +12,6 @@ # limitations under the License. import copy -from os import pread from .helpers.condition import ConditionOperatorTypes from .project_config import ProjectConfig From 4fa5d055d8159c1870a214839fe93c088eedb75e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 14 Jul 2021 16:50:15 -0400 Subject: [PATCH 58/59] Cleaned up get functions till overall cleanup of code. --- optimizely/optimizely_config.py | 40 --------------------------------- tests/test_optimizely_config.py | 20 ++++++++--------- 2 files changed, 10 insertions(+), 50 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index c5a3fdf5..11cc4b5c 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -39,46 +39,6 @@ def get_datafile(self): """ return self._datafile - def get_sdk_key(self): - """ Get the sdk key associated with OptimizelyConfig. - - Returns: - A string containing sdk key. - """ - return self.sdk_key - - def get_environment_key(self): - """ Get the environemnt key associated with OptimizelyConfig. - - Returns: - A string containing environment key. - """ - return self.environment_key - - def get_attributes(self): - """ Get the attributes associated with OptimizelyConfig - - returns: - A list of attributes. - """ - return self.attributes - - def get_events(self): - """ Get the events associated with OptimizelyConfig - - returns: - A list of events. - """ - return self.events - - def get_audiences(self): - """ Get the audiences associated with OptimizelyConfig - - returns: - A list of audiences. - """ - return self.audiences - class OptimizelyExperiment(object): def __init__(self, id, key, variations_map, audiences=''): diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 5167abd4..adbace59 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1229,7 +1229,7 @@ def test__get_sdk_key(self): expected_value = 'testSdkKey' - self.assertEqual(expected_value, config.get_sdk_key()) + self.assertEqual(expected_value, config.sdk_key) def test__get_sdk_key_invalid(self): """ Negative Test that tests get_sdk_key does not return the expected value. """ @@ -1243,7 +1243,7 @@ def test__get_sdk_key_invalid(self): invalid_value = 123 - self.assertNotEqual(invalid_value, config.get_sdk_key()) + self.assertNotEqual(invalid_value, config.sdk_key) def test__get_environment_key(self): """ Test that get_environment_key returns the expected value. """ @@ -1257,7 +1257,7 @@ def test__get_environment_key(self): expected_value = 'TestEnvironmentKey' - self.assertEqual(expected_value, config.get_environment_key()) + self.assertEqual(expected_value, config.environment_key) def test__get_environment_key_invalid(self): """ Negative Test that tests get_environment_key does not return the expected value. """ @@ -1271,7 +1271,7 @@ def test__get_environment_key_invalid(self): invalid_value = 321 - self.assertNotEqual(invalid_value, config.get_environment_key()) + self.assertNotEqual(invalid_value, config.environment_key) def test__get_attributes(self): """ Test that the get_attributes returns the expected value. """ @@ -1299,8 +1299,8 @@ def test__get_attributes(self): 'key': '234' }] - self.assertEqual(expected_value, config.get_attributes()) - self.assertEqual(len(config.get_attributes()), 2) + self.assertEqual(expected_value, config.attributes) + self.assertEqual(len(config.attributes), 2) def test__get_events(self): """ Test that the get_events returns the expected value. """ @@ -1341,8 +1341,8 @@ def test__get_events(self): } }] - self.assertEqual(expected_value, config.get_events()) - self.assertEqual(len(config.get_events()), 2) + self.assertEqual(expected_value, config.events) + self.assertEqual(len(config.events), 2) def test_get_audiences(self): ''' Test to confirm get_audiences returns proper value ''' @@ -1361,10 +1361,10 @@ def test_get_audiences(self): config = config_service.get_config() - for audience in config.get_audiences(): + for audience in config.audiences: self.assertIsInstance(audience, optimizely_config.OptimizelyAudience) - self.assertEqual(len(config.get_audiences()), len(config_service.audiences)) + self.assertEqual(len(config.audiences), len(config_service.audiences)) def test_stringify_audience_conditions_all_cases(self): audiences_map = { From c8d68f76e06b8c32979f7076298b5b953d3a615f Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 19 Jul 2021 08:44:24 -0400 Subject: [PATCH 59/59] Added comments to stringify_conditions, added testcase to test stringy for another edge case ['and'], changed method to combine typed_audiences and audiences from project_config to use lookup map rather than build a list and check length. Reduces time complexity. --- optimizely/optimizely_config.py | 44 +++++++++++++++++++-------------- tests/test_optimizely_config.py | 4 ++- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 11cc4b5c..23b5caa1 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -123,11 +123,12 @@ def __init__(self, project_config): ''' Merging typed_audiences with audiences from project_config. - The typed_audiences has higher presidence. + The typed_audiences has higher precidence. ''' typed_audiences = project_config.typed_audiences[:] optly_typed_audiences = [] + id_lookup_dict = {} for typed_audience in typed_audiences: optly_audience = OptimizelyAudience( typed_audience.get('id'), @@ -135,21 +136,18 @@ def __init__(self, project_config): typed_audience.get('conditions') ) optly_typed_audiences.append(optly_audience) + id_lookup_dict[typed_audience.get('id')] = typed_audience.get('id') for old_audience in project_config.audiences: # check if old_audience.id exists in new_audiences.id from typed_audiences - if len([new_audience for new_audience in project_config.typed_audiences - if new_audience.get('id') == old_audience.get('id')]) == 0: - if old_audience.get('id') == "$opt_dummy_audience": - continue - else: - # Convert audiences lists to OptimizelyAudience array - optly_audience = OptimizelyAudience( - old_audience.get('id'), - old_audience.get('name'), - old_audience.get('conditions') - ) - optly_typed_audiences.append(optly_audience) + if old_audience.get('id') not in id_lookup_dict and old_audience.get('id') != "$opt_dummy_audience": + # Convert audiences lists to OptimizelyAudience array + optly_audience = OptimizelyAudience( + old_audience.get('id'), + old_audience.get('name'), + old_audience.get('conditions') + ) + optly_typed_audiences.append(optly_audience) self.audiences = optly_typed_audiences @@ -193,13 +191,14 @@ def stringify_conditions(self, conditions, audiences_map): list of conditions. ''' ARGS = ConditionOperatorTypes.operators - condition = 'OR' + operand = 'OR' conditions_str = '' length = len(conditions) + # Edge cases for lengths 0, 1 or 2 if length == 0: return '' - if length == 1: + if length == 1 and conditions[0] not in ARGS: return '"' + self.lookup_name_from_id(conditions[0], audiences_map) + '"' if length == 2 and conditions[0] in ARGS and \ type(conditions[1]) is not list and \ @@ -209,24 +208,31 @@ def stringify_conditions(self, conditions, audiences_map): else: return conditions[0].upper() + \ ' "' + self.lookup_name_from_id(conditions[1], audiences_map) + '"' + # If length is 2 (where the one elemnt is a list) or greater if length > 1: for i in range(length): + # Operand is handled here and made Upper Case if conditions[i] in ARGS: - condition = conditions[i].upper() + operand = conditions[i].upper() else: + # Check if element is a list or not if type(conditions[i]) == list: + # Check if at the end or not to determine where to add the operand + # Recursive call to call stringify on embedded list if i + 1 < length: conditions_str += '(' + self.stringify_conditions(conditions[i], audiences_map) + ') ' else: - conditions_str += condition + \ + conditions_str += operand + \ ' (' + self.stringify_conditions(conditions[i], audiences_map) + ')' + # Not a list so we handle as and ID to lookup no recursion needed else: audience_name = self.lookup_name_from_id(conditions[i], audiences_map) if audience_name is not None: + # Below handles all cases for one ID or greater if i + 1 < length - 1: - conditions_str += '"' + audience_name + '" ' + condition + ' ' + conditions_str += '"' + audience_name + '" ' + operand + ' ' elif i + 1 == length: - conditions_str += condition + ' "' + audience_name + '"' + conditions_str += operand + ' "' + audience_name + '"' else: conditions_str += '"' + audience_name + '" ' diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index adbace59..b7cbbd7b 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1399,7 +1399,8 @@ def test_stringify_audience_conditions_all_cases(self): ["and", ["or", "1", ["and", "2", "3"]], ["and", "11", ["or", "12", "13"]]], ["not", ["and", "1", "2"]], ["or", "1", "100000"], - ["and", "and"] + ["and", "and"], + ["and"] ] audiences_output = [ @@ -1415,6 +1416,7 @@ def test_stringify_audience_conditions_all_cases(self): '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "kid"))', 'NOT ("us" AND "female")', '"us" OR "100000"', + '', '' ]