From 46c964ed0f7c2fbd461f7939501370635cab4037 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Thu, 31 May 2018 14:45:28 +0500 Subject: [PATCH 01/10] :pen: Bot Filtering changes + Fix previous unit tests --- optimizely/decision_service.py | 3 +- optimizely/event_builder.py | 40 +++++++++++-- optimizely/helpers/enums.py | 6 ++ optimizely/project_config.py | 10 ++++ tests/base.py | 2 + tests/test_event_builder.py | 104 ++++++++++++++++++++------------- tests/test_optimizely.py | 57 ++++++++++++++++++ 7 files changed, 176 insertions(+), 46 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 80fe83100..4966e109c 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -25,7 +25,6 @@ Decision = namedtuple('Decision', 'experiment variation source') DECISION_SOURCE_EXPERIMENT = 'experiment' DECISION_SOURCE_ROLLOUT = 'rollout' -RESERVED_BUCKETING_ID_ATTRIBUTE = '$opt_bucketing_id' class DecisionService(object): @@ -50,7 +49,7 @@ def _get_bucketing_id(user_id, attributes): """ attributes = attributes or {} - return attributes.get(RESERVED_BUCKETING_ID_ATTRIBUTE, user_id) + return attributes.get(enums.ReservedAttributes.BUCKETING_ID, user_id) def get_forced_variation(self, experiment, user_id): """ Determine if a user is forced into a variation for the given experiment and return that variation. diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 18894a7f2..574bc0113 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -17,6 +17,7 @@ from abc import abstractproperty from . import version +from .helpers.enums import ReservedAttributes from .helpers import event_tag_utils @@ -85,6 +86,15 @@ def _get_anonymize_ip(self): return self.config.get_anonymize_ip_value() + def _get_bot_filtering(self): + """ Get IP anonymization bool + + Returns: + bool 'anonymizeIP' value in the datafile. + """ + + return self.config.get_bot_filtering_value() + @abstractmethod def _get_time(self): """ Get time in milliseconds to be added. @@ -172,19 +182,39 @@ def _get_attributes(self, attributes): if not attributes: return [] - for attribute_key in attributes.keys(): + for attribute_key in sorted(attributes.keys()): attribute_value = attributes.get(attribute_key) # Omit falsy attribute values if attribute_value: - attribute = self.config.get_attribute(attribute_key) - if attribute: + # Check for reserved attributes + reserved_attrs = [ReservedAttributes.BUCKETING_ID, ReservedAttributes.USER_AGENT] + if attribute_key in reserved_attrs: params.append({ - self.EventParams.EVENT_ID: attribute.id, + 'entity_id': attribute_key, 'key': attribute_key, 'type': self.EventParams.CUSTOM, - 'value': attribute_value, + 'value': attribute_value }) + else: + attribute = self.config.get_attribute(attribute_key) + if attribute: + params.append({ + 'entity_id': attribute.id, + 'key': attribute_key, + 'type': self.EventParams.CUSTOM, + 'value': attribute_value, + }) + + # Append Bot Filtering Attribute + attribute_key = ReservedAttributes.BOT_FILTERING + params.append({ + 'entity_id': attribute_key, + 'key': attribute_key, + 'type': self.EventParams.CUSTOM, + 'value': self._get_bot_filtering(), + }) + return params def _get_required_params_for_impression(self, experiment, variation_id): diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index ad09b0084..341155d1c 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -59,3 +59,9 @@ class NotificationTypes(object): """ ACTIVATE = "ACTIVATE:experiment, user_id, attributes, variation, event" TRACK = "TRACK:event_key, user_id, attributes, event_tags, event" + + +class ReservedAttributes(object): + BOT_FILTERING = '$opt_bot_filtering' + BUCKETING_ID = '$opt_bucketing_id' + USER_AGENT = '$opt_user_agent' diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 5f6865330..edcca3c49 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -56,6 +56,7 @@ def __init__(self, datafile, logger, error_handler): self.feature_flags = config.get('featureFlags', []) self.rollouts = config.get('rollouts', []) self.anonymize_ip = config.get('anonymizeIP', False) + self.bot_filtering = config.get('botFiltering', False) # Utility maps for quick lookup self.group_id_map = self._generate_key_map(self.groups, 'id', entities.Group) @@ -589,3 +590,12 @@ def get_anonymize_ip_value(self): """ return self.anonymize_ip + + def get_bot_filtering_value(self): + """ Gets the anonymize IP value. + + Returns: + A boolean value that indicates if the IP should be anonymized. + """ + + return self.bot_filtering diff --git a/tests/base.py b/tests/base.py index f2399fb40..12d27e863 100644 --- a/tests/base.py +++ b/tests/base.py @@ -150,6 +150,7 @@ def setUp(self): 'accountId': '12001', 'projectId': '111111', 'version': '4', + 'botFiltering': False, 'events': [{ 'key': 'test_event', 'experimentIds': ['111127'], @@ -381,3 +382,4 @@ def setUp(self): self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict)) self.project_config = self.optimizely.config + self.maxDiff = None diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index e842179a6..2e61d9a2f 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -45,6 +45,7 @@ class EventBuilderTest(base.BaseTest): def setUp(self): base.BaseTest.setUp(self) self.event_builder = self.optimizely.event_builder + self.maxDiff = None def _validate_event_object(self, event_obj, expected_url, expected_params, expected_verb, expected_headers): """ Helper method to validate properties of the event object. """ @@ -109,6 +110,11 @@ def test_create_impression_event__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -143,46 +149,51 @@ def test_create_impression_event__with_attributes(self): event_builder.EventBuilder.HTTP_HEADERS) def test_create_impression_event_when_attribute_is_not_in_datafile(self): - """ Test that create_impression_event creates Event object - with right params when attribute is not in the datafile. """ - - expected_params = { - 'account_id': '12001', - 'project_id': '111001', - 'visitors': [{ - 'visitor_id': 'test_user', - 'attributes': [], - 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], - 'events': [{ - 'timestamp': 42123, - 'entity_id': '111182', - 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', - 'key': 'campaign_activated' - }] - }] + """ Test that create_impression_event creates Event object + with right params when attribute is not in the datafile. """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], - 'client_name': 'python-sdk', - 'client_version': version.__version__, - 'anonymize_ip': False, - 'revision': '42' - } - - with mock.patch('time.time', return_value=42.123), \ - mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): - event_obj = self.event_builder.create_impression_event( - self.project_config.get_experiment_from_key('test_experiment'), - '111129', 'test_user', {'do_you_know_me': 'test_value'} - ) - self._validate_event_object(event_obj, - event_builder.EventBuilder.EVENTS_URL, - expected_params, - event_builder.EventBuilder.HTTP_VERB, - event_builder.EventBuilder.HTTP_HEADERS) + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + event_obj = self.event_builder.create_impression_event( + self.project_config.get_experiment_from_key('test_experiment'), + '111129', 'test_user', {'do_you_know_me': 'test_value'} + ) + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) def test_create_conversion_event(self): """ Test that create_conversion_event creates Event object @@ -239,6 +250,11 @@ def test_create_conversion_event__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -284,6 +300,11 @@ def test_create_conversion_event__with_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -341,6 +362,11 @@ def test_create_conversion_event__with_invalid_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index c223a73a0..85faaf22f 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -512,6 +512,11 @@ def test_activate__with_attributes__audience_match(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -560,6 +565,11 @@ def test_activate__with_attributes__audience_match__forced_bucketing(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -605,10 +615,20 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): 'visitors': [{ 'visitor_id': 'test_user', 'attributes': [{ + 'type': 'custom', + 'value': 'user_bucket_value', + 'entity_id': '$opt_bucketing_id', + 'key': '$opt_bucketing_id' + }, { 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -727,6 +747,11 @@ def test_track__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -773,10 +798,20 @@ def test_track__with_attributes__bucketing_id_provided(self): 'visitors': [{ 'visitor_id': 'test_user', 'attributes': [{ + 'type': 'custom', + 'value': 'user_bucket_value', + 'entity_id': '$opt_bucketing_id', + 'key': '$opt_bucketing_id' + }, { 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -851,6 +886,11 @@ def test_track__with_event_tags(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -905,6 +945,11 @@ def test_track__with_event_tags_revenue(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -991,6 +1036,11 @@ def test_track__with_event_tags__forced_bucketing(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -1044,6 +1094,11 @@ def test_track__with_invalid_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -1744,6 +1799,7 @@ def test_get_feature_variable__returns_none_if_unable_to_cast(self): class OptimizelyWithExceptionTest(base.BaseTest): + def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), @@ -1775,6 +1831,7 @@ def test_get_variation__with_attributes__invalid_attributes(self): class OptimizelyWithLoggingTest(base.BaseTest): + def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), logger=logger.SimpleLogger()) From 6b48a7b04f45120c290fd4b49838ad4e8d43dcf0 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Thu, 31 May 2018 16:26:04 +0500 Subject: [PATCH 02/10] :pen: Add unit test for bot filtering --- optimizely/decision_service.py | 2 +- optimizely/event_builder.py | 6 +- optimizely/helpers/enums.py | 2 +- optimizely/project_config.py | 4 +- tests/base.py | 1 - tests/test_config.py | 17 +++ tests/test_event_builder.py | 219 ++++++++++++++++++++++++++++++++- 7 files changed, 242 insertions(+), 9 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 4966e109c..fd02b5c04 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -1,4 +1,4 @@ -# Copyright 2017, Optimizely +# Copyright 2017-2018, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 574bc0113..4196fad75 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -17,8 +17,8 @@ from abc import abstractproperty from . import version -from .helpers.enums import ReservedAttributes from .helpers import event_tag_utils +from .helpers.enums import ReservedAttributes class Event(object): @@ -87,10 +87,10 @@ def _get_anonymize_ip(self): return self.config.get_anonymize_ip_value() def _get_bot_filtering(self): - """ Get IP anonymization bool + """ Get bot filtering bool Returns: - bool 'anonymizeIP' value in the datafile. + bool 'botFiltering' value in the datafile. """ return self.config.get_bot_filtering_value() diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 341155d1c..b2381300b 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -1,4 +1,4 @@ -# Copyright 2016-2017, Optimizely +# Copyright 2016-2018, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at diff --git a/optimizely/project_config.py b/optimizely/project_config.py index edcca3c49..dd2fb7068 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -592,10 +592,10 @@ def get_anonymize_ip_value(self): return self.anonymize_ip def get_bot_filtering_value(self): - """ Gets the anonymize IP value. + """ Gets the bot filtering value. Returns: - A boolean value that indicates if the IP should be anonymized. + A boolean value that indicates if bot filtering should be enabled. """ return self.bot_filtering diff --git a/tests/base.py b/tests/base.py index 12d27e863..276b87dbf 100644 --- a/tests/base.py +++ b/tests/base.py @@ -382,4 +382,3 @@ def setUp(self): self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict)) self.project_config = self.optimizely.config - self.maxDiff = None diff --git a/tests/test_config.py b/tests/test_config.py index c3dd35449..3159cb387 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -172,6 +172,7 @@ def test_init__with_v4_datafile(self): 'revision': '42', 'version': '4', 'anonymizeIP': False, + 'botFiltering': True, 'events': [{ 'key': 'test_event', 'experimentIds': ['111127'], @@ -387,6 +388,7 @@ def test_init__with_v4_datafile(self): self.assertEqual(config_dict['revision'], project_config.revision) self.assertEqual(config_dict['experiments'], project_config.experiments) self.assertEqual(config_dict['events'], project_config.events) + self.assertEqual(config_dict['botFiltering'], project_config.bot_filtering) expected_group_id_map = { '19228': entities.Group( @@ -679,6 +681,20 @@ def test_get_project_id(self): self.assertEqual(self.config_dict['projectId'], self.project_config.get_project_id()) + def test_get_bot_filtering(self): + """ Test that bot filtering is retrieved correctly when using get_bot_filtering_value. """ + + # Assert bot filtering is False when not provided in data file + self.assertTrue('botFiltering' not in self.config_dict) + self.assertEqual(False, self.project_config.get_bot_filtering_value()) + + # Assert bot filtering is retrieved as provided in the data file + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) + project_config = opt_obj.config + self.assertEqual(self.config_dict_with_features['botFiltering'], + self.project_config.get_bot_filtering_value() + ) + def test_get_experiment_from_key__valid_key(self): """ Test that experiment is retrieved correctly for valid experiment key. """ @@ -1073,6 +1089,7 @@ def test_set_forced_variation_when_called_to_remove_forced_variation(self): class ConfigLoggingTest(base.BaseTest): + def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 2e61d9a2f..357d64fc4 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -45,7 +45,6 @@ class EventBuilderTest(base.BaseTest): def setUp(self): base.BaseTest.setUp(self) self.event_builder = self.optimizely.event_builder - self.maxDiff = None def _validate_event_object(self, event_obj, expected_url, expected_params, expected_verb, expected_headers): """ Helper method to validate properties of the event object. """ @@ -195,6 +194,116 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) + def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled(self): + """ Test that create_impression_event creates Event object + with right params when user agent attribute is provided and + bot filtering is enabled """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': 'Edge', + 'entity_id': '$opt_user_agent', + 'key': '$opt_user_agent' + }, { + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'),\ + mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=True): + event_obj = self.event_builder.create_impression_event( + self.project_config.get_experiment_from_key('test_experiment'), + '111129', 'test_user', {'$opt_user_agent': 'Edge'} + ) + + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) + + def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled(self): + """ Test that create_impression_event creates Event object + with right params when user agent attribute is provided and + bot filtering is disabled """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': 'Chrome', + 'entity_id': '$opt_user_agent', + 'key': '$opt_user_agent' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + event_obj = self.event_builder.create_impression_event( + self.project_config.get_experiment_from_key('test_experiment'), + '111129', 'test_user', {'$opt_user_agent': 'Chrome'} + ) + + self.assertFalse(self.project_config.get_bot_filtering_value()) + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) + def test_create_conversion_event(self): """ Test that create_conversion_event creates Event object with right params when no attributes are provided. """ @@ -287,6 +396,114 @@ def test_create_conversion_event__with_attributes(self): event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) + def test_create_conversion_event__with_user_agent_when_bot_filtering_is_enabled(self): + """ Test that create_conversion_event creates Event object + with right params when user agent attribute is provided and + bot filtering is enabled """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': 'Edge', + 'entity_id': '$opt_user_agent', + 'key': '$opt_user_agent' + }, { + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111095', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'test_event' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ + mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=True): + event_obj = self.event_builder.create_conversion_event( + 'test_event', 'test_user', {'$opt_user_agent': 'Edge'}, None, [('111127', '111129')] + ) + + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) + + def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled(self): + """ Test that create_conversion_event creates Event object + with right params when user agent attribute is provided and + bot filtering is disabled """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': 'Chrome', + 'entity_id': '$opt_user_agent', + 'key': '$opt_user_agent' + }, { + 'type': 'custom', + 'value': False, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111095', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'test_event' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + event_obj = self.event_builder.create_conversion_event( + 'test_event', 'test_user', {'$opt_user_agent': 'Chrome'}, None, [('111127', '111129')] + ) + + self.assertFalse(self.project_config.get_bot_filtering_value()) + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) + def test_create_conversion_event__with_event_tags(self): """ Test that create_conversion_event creates Event object with right params when event tags are provided. """ From ca143317e35bf46d5d96a6fd3a56c9d0f469afcb Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 1 Jun 2018 17:03:47 +0500 Subject: [PATCH 03/10] :pen: JS Sync impl --- optimizely/decision_service.py | 2 +- optimizely/event_builder.py | 35 ++++------- optimizely/helpers/enums.py | 2 +- optimizely/project_config.py | 21 +++++-- tests/base.py | 2 +- tests/test_config.py | 54 ++++++++++++---- tests/test_event_builder.py | 112 +++++++++++++-------------------- tests/test_optimizely.py | 51 +-------------- 8 files changed, 118 insertions(+), 161 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index fd02b5c04..a3d8ee375 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -49,7 +49,7 @@ def _get_bucketing_id(user_id, attributes): """ attributes = attributes or {} - return attributes.get(enums.ReservedAttributes.BUCKETING_ID, user_id) + return attributes.get(enums.ControlAttributes.BUCKETING_ID, user_id) def get_forced_variation(self, experiment, user_id): """ Determine if a user is forced into a variation for the given experiment and return that variation. diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 4196fad75..7d7b8a45f 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -18,7 +18,7 @@ from . import version from .helpers import event_tag_utils -from .helpers.enums import ReservedAttributes +from .helpers.enums import ControlAttributes class Event(object): @@ -186,34 +186,23 @@ def _get_attributes(self, attributes): attribute_value = attributes.get(attribute_key) # Omit falsy attribute values if attribute_value: - # Check for reserved attributes - reserved_attrs = [ReservedAttributes.BUCKETING_ID, ReservedAttributes.USER_AGENT] - if attribute_key in reserved_attrs: + attribute_id = self.config.get_attribute_id(attribute_key) + if attribute_id: params.append({ - 'entity_id': attribute_key, + 'entity_id': attribute_id, 'key': attribute_key, 'type': self.EventParams.CUSTOM, - 'value': attribute_value + 'value': attribute_value, }) - else: - attribute = self.config.get_attribute(attribute_key) - if attribute: - params.append({ - 'entity_id': attribute.id, - 'key': attribute_key, - 'type': self.EventParams.CUSTOM, - 'value': attribute_value, - }) - # Append Bot Filtering Attribute - attribute_key = ReservedAttributes.BOT_FILTERING - params.append({ - 'entity_id': attribute_key, - 'key': attribute_key, - 'type': self.EventParams.CUSTOM, - 'value': self._get_bot_filtering(), - }) + if isinstance(self._get_bot_filtering(), bool): + params.append({ + 'entity_id': ControlAttributes.BOT_FILTERING, + 'key': ControlAttributes.BOT_FILTERING, + 'type': self.EventParams.CUSTOM, + 'value': self._get_bot_filtering(), + }) return params diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index b2381300b..35cf2ab10 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -61,7 +61,7 @@ class NotificationTypes(object): TRACK = "TRACK:event_key, user_id, attributes, event_tags, event" -class ReservedAttributes(object): +class ControlAttributes(object): BOT_FILTERING = '$opt_bot_filtering' BUCKETING_ID = '$opt_bucketing_id' USER_AGENT = '$opt_user_agent' diff --git a/optimizely/project_config.py b/optimizely/project_config.py index dd2fb7068..bf1c3c512 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -25,6 +25,8 @@ SUPPORTED_VERSIONS = [V2_CONFIG_VERSION] UNSUPPORTED_VERSIONS = [V1_CONFIG_VERSION] +RESERVED_ATTRIBUTE_PREFIX = '$opt_' + class ProjectConfig(object): """ Representation of the Optimizely project config. """ @@ -56,7 +58,7 @@ def __init__(self, datafile, logger, error_handler): self.feature_flags = config.get('featureFlags', []) self.rollouts = config.get('rollouts', []) self.anonymize_ip = config.get('anonymizeIP', False) - self.bot_filtering = config.get('botFiltering', False) + self.bot_filtering = config.get('botFiltering', None) # Utility maps for quick lookup self.group_id_map = self._generate_key_map(self.groups, 'id', entities.Group) @@ -364,20 +366,29 @@ def get_event(self, event_key): self.error_handler.handle_error(exceptions.InvalidEventException(enums.Errors.INVALID_EVENT_KEY_ERROR)) return None - def get_attribute(self, attribute_key): - """ Get attribute for the provided attribute key. + def get_attribute_id(self, attribute_key): + """ Get attribute ID for the provided attribute key. Args: attribute_key: Attribute key for which attribute is to be fetched. Returns: - Attribute corresponding to the provided attribute key. + Attribute ID corresponding to the provided attribute key. """ attribute = self.attribute_key_map.get(attribute_key) + has_reserved_prefix = attribute_key.startswith(RESERVED_ATTRIBUTE_PREFIX) if attribute: - return attribute + if has_reserved_prefix: + self.logger.log(enums.LogLevels.WARNING, + 'Attribute %s unexpectedly has reserved prefix %s; using attribute ID instead of reserved attribute name.' + % (attribute_key, RESERVED_ATTRIBUTE_PREFIX)) + + return attribute.id + + if has_reserved_prefix and attribute_key != enums.ControlAttributes.BOT_FILTERING: + return attribute_key self.logger.log(enums.LogLevels.ERROR, 'Attribute "%s" is not in datafile.' % attribute_key) self.error_handler.handle_error(exceptions.InvalidAttributeException(enums.Errors.INVALID_ATTRIBUTE_ERROR)) diff --git a/tests/base.py b/tests/base.py index 276b87dbf..9f3d285b0 100644 --- a/tests/base.py +++ b/tests/base.py @@ -150,7 +150,7 @@ def setUp(self): 'accountId': '12001', 'projectId': '111111', 'version': '4', - 'botFiltering': False, + 'botFiltering': True, 'events': [{ 'key': 'test_event', 'experimentIds': ['111127'], diff --git a/tests/test_config.py b/tests/test_config.py index 3159cb387..deb806f12 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -684,15 +684,15 @@ def test_get_project_id(self): def test_get_bot_filtering(self): """ Test that bot filtering is retrieved correctly when using get_bot_filtering_value. """ - # Assert bot filtering is False when not provided in data file + # Assert bot filtering is None when not provided in data file self.assertTrue('botFiltering' not in self.config_dict) - self.assertEqual(False, self.project_config.get_bot_filtering_value()) + self.assertIsNone(self.project_config.get_bot_filtering_value()) # Assert bot filtering is retrieved as provided in the data file opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config self.assertEqual(self.config_dict_with_features['botFiltering'], - self.project_config.get_bot_filtering_value() + project_config.get_bot_filtering_value() ) def test_get_experiment_from_key__valid_key(self): @@ -803,16 +803,32 @@ def test_get_event__invalid_key(self): self.assertIsNone(self.project_config.get_event('invalid_key')) - def test_get_attribute__valid_key(self): - """ Test that attribute is retrieved correctly for valid attribute key. """ + def test_get_attribute_id__valid_key(self): + """ Test that attribute ID is retrieved correctly for valid attribute key. """ - self.assertEqual(entities.Attribute('111094', 'test_attribute'), - self.project_config.get_attribute('test_attribute')) + self.assertEqual('111094', + self.project_config.get_attribute_id('test_attribute')) - def test_get_attribute__invalid_key(self): + def test_get_attribute_id__invalid_key(self): """ Test that None is returned when provided attribute key is invalid. """ - self.assertIsNone(self.project_config.get_attribute('invalid_key')) + self.assertIsNone(self.project_config.get_attribute_id('invalid_key')) + + def test_get_attribute_id__reserved_key(self): + """ Test that Attribute Key is returned as ID when provided attribute key is invalid. """ + self.assertEqual('$opt_user_agent', + self.project_config.get_attribute_id('$opt_user_agent')) + + def test_get_attribute_id__unknown_key_with_opt_prefix(self): + """ Test that Attribute Key is returned as ID when provided attribute key is not + present in the datafile but has $opt prefix. """ + self.assertEqual('$opt_interesting', + self.project_config.get_attribute_id('$opt_interesting')) + + def test_get_attribute_id__key_is_bot_filtering_enum(self): + """ Test that None is returned when provided attribute key is + equal to '$opt_bot_filtering'. """ + self.assertIsNone(self.project_config.get_attribute_id('$opt_bot_filtering')) def test_get_group__valid_id(self): """ Test that group is retrieved correctly for valid group ID. """ @@ -1152,14 +1168,26 @@ def test_get_event__invalid_key(self): mock_logging.assert_called_once_with(enums.LogLevels.ERROR, 'Event "invalid_key" is not in datafile.') - def test_get_attribute__invalid_key(self): + def test_get_attribute_id__invalid_key(self): """ Test that message is logged when provided attribute key is invalid. """ with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging: - self.project_config.get_attribute('invalid_key') + self.project_config.get_attribute_id('invalid_key') mock_logging.assert_called_once_with(enums.LogLevels.ERROR, 'Attribute "invalid_key" is not in datafile.') + def test_get_attribute_id__key_with_opt_prefix_but_not_a_control_attribute(self): + """ Test that message is logged when provided attribute key has $opt_ in prefix and + key is not one of the control attributes. """ + self.project_config.attribute_key_map['$opt_abc'] = entities.Attribute('007', '$opt_abc') + + with mock.patch('optimizely.logger.SimpleLogger.log') as mock_logging: + self.project_config.get_attribute_id('$opt_abc') + + mock_logging.assert_called_once_with(enums.LogLevels.WARNING, + ("Attribute $opt_abc unexpectedly has reserved prefix $opt_; " + "using attribute ID instead of reserved attribute name.")) + def test_get_group__invalid_id(self): """ Test that message is logged when provided group ID is invalid. """ @@ -1226,12 +1254,12 @@ def test_get_event__invalid_key(self): enums.Errors.INVALID_EVENT_KEY_ERROR, self.project_config.get_event, 'invalid_key') - def test_get_attribute__invalid_key(self): + def test_get_attribute_id__invalid_key(self): """ Test that exception is raised when provided attribute key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidAttributeException, enums.Errors.INVALID_ATTRIBUTE_ERROR, - self.project_config.get_attribute, 'invalid_key') + self.project_config.get_attribute_id, 'invalid_key') def test_get_group__invalid_id(self): """ Test that exception is raised when provided group ID is invalid. """ diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 357d64fc4..39af89f27 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -109,11 +109,6 @@ def test_create_impression_event__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -148,51 +143,46 @@ def test_create_impression_event__with_attributes(self): event_builder.EventBuilder.HTTP_HEADERS) def test_create_impression_event_when_attribute_is_not_in_datafile(self): - """ Test that create_impression_event creates Event object - with right params when attribute is not in the datafile. """ - - expected_params = { - 'account_id': '12001', - 'project_id': '111001', - 'visitors': [{ - 'visitor_id': 'test_user', - 'attributes': [{ - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' - }], - 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], - 'events': [{ - 'timestamp': 42123, - 'entity_id': '111182', - 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', - 'key': 'campaign_activated' + """ Test that create_impression_event creates Event object + with right params when attribute is not in the datafile. """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] }] - }] - }], - 'client_name': 'python-sdk', - 'client_version': version.__version__, - 'anonymize_ip': False, - 'revision': '42' - } - - with mock.patch('time.time', return_value=42.123), \ - mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): - event_obj = self.event_builder.create_impression_event( - self.project_config.get_experiment_from_key('test_experiment'), - '111129', 'test_user', {'do_you_know_me': 'test_value'} - ) - self._validate_event_object(event_obj, - event_builder.EventBuilder.EVENTS_URL, - expected_params, - event_builder.EventBuilder.HTTP_VERB, - event_builder.EventBuilder.HTTP_HEADERS) + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + event_obj = self.event_builder.create_impression_event( + self.project_config.get_experiment_from_key('test_experiment'), + '111129', 'test_user', {'do_you_know_me': 'test_value'} + ) + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled(self): """ Test that create_impression_event creates Event object @@ -291,19 +281,20 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled } with mock.patch('time.time', return_value=42.123), \ - mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'),\ + mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=False): event_obj = self.event_builder.create_impression_event( self.project_config.get_experiment_from_key('test_experiment'), '111129', 'test_user', {'$opt_user_agent': 'Chrome'} ) - self.assertFalse(self.project_config.get_bot_filtering_value()) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, expected_params, event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) + def test_create_conversion_event(self): """ Test that create_conversion_event creates Event object with right params when no attributes are provided. """ @@ -359,11 +350,6 @@ def test_create_conversion_event__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -492,12 +478,12 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled } with mock.patch('time.time', return_value=42.123), \ - mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ + mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=False): event_obj = self.event_builder.create_conversion_event( 'test_event', 'test_user', {'$opt_user_agent': 'Chrome'}, None, [('111127', '111129')] ) - self.assertFalse(self.project_config.get_bot_filtering_value()) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, expected_params, @@ -517,11 +503,6 @@ def test_create_conversion_event__with_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -579,11 +560,6 @@ def test_create_conversion_event__with_invalid_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 85faaf22f..3dd05e0c1 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -512,11 +512,6 @@ def test_activate__with_attributes__audience_match(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -565,11 +560,6 @@ def test_activate__with_attributes__audience_match__forced_bucketing(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -619,16 +609,11 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): 'value': 'user_bucket_value', 'entity_id': '$opt_bucketing_id', 'key': '$opt_bucketing_id' - }, { + },{ 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -747,11 +732,6 @@ def test_track__with_attributes(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -802,16 +782,11 @@ def test_track__with_attributes__bucketing_id_provided(self): 'value': 'user_bucket_value', 'entity_id': '$opt_bucketing_id', 'key': '$opt_bucketing_id' - }, { + },{ 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -886,11 +861,6 @@ def test_track__with_event_tags(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -945,11 +915,6 @@ def test_track__with_event_tags_revenue(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -1036,11 +1001,6 @@ def test_track__with_event_tags__forced_bucketing(self): 'value': 'test_value', 'entity_id': '111094', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'snapshots': [{ 'decisions': [{ @@ -1094,11 +1054,6 @@ def test_track__with_invalid_event_tags(self): 'type': 'custom', 'value': 'test_value', 'key': 'test_attribute' - }, { - 'type': 'custom', - 'value': False, - 'entity_id': '$opt_bot_filtering', - 'key': '$opt_bot_filtering' }], 'visitor_id': 'test_user', 'snapshots': [{ @@ -1799,7 +1754,6 @@ def test_get_feature_variable__returns_none_if_unable_to_cast(self): class OptimizelyWithExceptionTest(base.BaseTest): - def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), @@ -1831,7 +1785,6 @@ def test_get_variation__with_attributes__invalid_attributes(self): class OptimizelyWithLoggingTest(base.BaseTest): - def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), logger=logger.SimpleLogger()) From c458c08fbaa76160e91c7804bebb2093e0819e9c Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 1 Jun 2018 17:25:59 +0500 Subject: [PATCH 04/10] :pen: PEP8 fix --- optimizely/event_builder.py | 2 +- optimizely/project_config.py | 4 ++-- tests/test_config.py | 4 ++-- tests/test_event_builder.py | 1 - tests/test_optimizely.py | 6 ++++-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 7d7b8a45f..1b944fd58 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -90,7 +90,7 @@ def _get_bot_filtering(self): """ Get bot filtering bool Returns: - bool 'botFiltering' value in the datafile. + 'botFiltering' value in the datafile. """ return self.config.get_bot_filtering_value() diff --git a/optimizely/project_config.py b/optimizely/project_config.py index bf1c3c512..77da812cd 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -382,8 +382,8 @@ def get_attribute_id(self, attribute_key): if attribute: if has_reserved_prefix: self.logger.log(enums.LogLevels.WARNING, - 'Attribute %s unexpectedly has reserved prefix %s; using attribute ID instead of reserved attribute name.' - % (attribute_key, RESERVED_ATTRIBUTE_PREFIX)) + ('Attribute %s unexpectedly has reserved prefix %s; using attribute ID ' + 'instead of reserved attribute name.' % (attribute_key, RESERVED_ATTRIBUTE_PREFIX))) return attribute.id diff --git a/tests/test_config.py b/tests/test_config.py index deb806f12..cc5b9d00b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -815,7 +815,7 @@ def test_get_attribute_id__invalid_key(self): self.assertIsNone(self.project_config.get_attribute_id('invalid_key')) def test_get_attribute_id__reserved_key(self): - """ Test that Attribute Key is returned as ID when provided attribute key is invalid. """ + """ Test that Attribute Key is returned as ID when provided attribute key is reserved key. """ self.assertEqual('$opt_user_agent', self.project_config.get_attribute_id('$opt_user_agent')) @@ -826,7 +826,7 @@ def test_get_attribute_id__unknown_key_with_opt_prefix(self): self.project_config.get_attribute_id('$opt_interesting')) def test_get_attribute_id__key_is_bot_filtering_enum(self): - """ Test that None is returned when provided attribute key is + """ Test that None is returned when provided attribute key is equal to '$opt_bot_filtering'. """ self.assertIsNone(self.project_config.get_attribute_id('$opt_bot_filtering')) diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 39af89f27..bb848e85f 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -294,7 +294,6 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) - def test_create_conversion_event(self): """ Test that create_conversion_event creates Event object with right params when no attributes are provided. """ diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 3dd05e0c1..60b201f61 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -609,7 +609,7 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): 'value': 'user_bucket_value', 'entity_id': '$opt_bucketing_id', 'key': '$opt_bucketing_id' - },{ + }, { 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', @@ -782,7 +782,7 @@ def test_track__with_attributes__bucketing_id_provided(self): 'value': 'user_bucket_value', 'entity_id': '$opt_bucketing_id', 'key': '$opt_bucketing_id' - },{ + }, { 'type': 'custom', 'value': 'test_value', 'entity_id': '111094', @@ -1754,6 +1754,7 @@ def test_get_feature_variable__returns_none_if_unable_to_cast(self): class OptimizelyWithExceptionTest(base.BaseTest): + def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), @@ -1785,6 +1786,7 @@ def test_get_variation__with_attributes__invalid_attributes(self): class OptimizelyWithLoggingTest(base.BaseTest): + def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), logger=logger.SimpleLogger()) From 9e05758d164c907fba987ab831f8b9d71907cea4 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Thu, 7 Jun 2018 09:20:15 +0500 Subject: [PATCH 05/10] :pen: Changes --- optimizely/event_builder.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index ad2aed41b..95415a592 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -17,8 +17,8 @@ from abc import abstractproperty from . import version +from .helpers import enums from .helpers import event_tag_utils -from .helpers.enums import ControlAttributes class Event(object): @@ -81,7 +81,7 @@ def _get_anonymize_ip(self): """ Get IP anonymization bool Returns: - bool 'anonymizeIP' value in the datafile. + Boolean representing whether IP anonymization is enabled or not. """ return self.config.get_anonymize_ip_value() @@ -90,7 +90,7 @@ def _get_bot_filtering(self): """ Get bot filtering bool Returns: - 'botFiltering' value in the datafile. + Boolean representing whether bot filtering is enabled or not. """ return self.config.get_bot_filtering_value() @@ -182,7 +182,7 @@ def _get_attributes(self, attributes): if not attributes: return [] - for attribute_key in sorted(attributes.keys()): + for attribute_key in attributes.keys(): attribute_value = attributes.get(attribute_key) # Omit falsy attribute values if attribute_value: @@ -192,16 +192,17 @@ def _get_attributes(self, attributes): 'entity_id': attribute_id, 'key': attribute_key, 'type': self.EventParams.CUSTOM, - 'value': attribute_value, + 'value': attribute_value }) # Append Bot Filtering Attribute - if isinstance(self._get_bot_filtering(), bool): + bot_filtering_value = self._get_bot_filtering() + if isinstance(bot_filtering_value, bool): params.append({ - 'entity_id': ControlAttributes.BOT_FILTERING, - 'key': ControlAttributes.BOT_FILTERING, + 'entity_id': enums.ControlAttributes.BOT_FILTERING, + 'key': enums.ControlAttributes.BOT_FILTERING, 'type': self.EventParams.CUSTOM, - 'value': self._get_bot_filtering(), + 'value': bot_filtering_value }) return params From b938de352d3e10bf90b1a6caf309aa755dd92864 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Thu, 7 Jun 2018 09:28:27 +0500 Subject: [PATCH 06/10] :pen: sort attribute keys --- optimizely/event_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 95415a592..8d0697d6f 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -182,7 +182,7 @@ def _get_attributes(self, attributes): if not attributes: return [] - for attribute_key in attributes.keys(): + for attribute_key in sorted(attributes.keys()): attribute_value = attributes.get(attribute_key) # Omit falsy attribute values if attribute_value: From d94231a1dbaaa5f3bef78a292b04fb80705cba3f Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Thu, 7 Jun 2018 16:46:32 +0500 Subject: [PATCH 07/10] Add BotFiltering attr even when attributes are empty/None --- optimizely/event_builder.py | 28 ++++++++++----------- tests/test_event_builder.py | 50 +++++++++++++++++++++++++++++++++++++ tests/test_optimizely.py | 7 +++++- 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 8d0697d6f..2bab469c1 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -179,21 +179,19 @@ def _get_attributes(self, attributes): params = [] - if not attributes: - return [] - - for attribute_key in sorted(attributes.keys()): - attribute_value = attributes.get(attribute_key) - # Omit falsy attribute values - if attribute_value: - attribute_id = self.config.get_attribute_id(attribute_key) - if attribute_id: - params.append({ - 'entity_id': attribute_id, - 'key': attribute_key, - 'type': self.EventParams.CUSTOM, - 'value': attribute_value - }) + if isinstance(attributes, dict): + for attribute_key in sorted(attributes.keys()): + attribute_value = attributes.get(attribute_key) + # Omit falsy attribute values + if attribute_value: + attribute_id = self.config.get_attribute_id(attribute_key) + if attribute_id: + params.append({ + 'entity_id': attribute_id, + 'key': attribute_key, + 'type': self.EventParams.CUSTOM, + 'value': attribute_value + }) # Append Bot Filtering Attribute bot_filtering_value = self._get_bot_filtering() diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 7dd467291..1cc53f689 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -239,6 +239,56 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( event_builder.EventBuilder.HTTP_VERB, event_builder.EventBuilder.HTTP_HEADERS) + def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_enabled(self): + """ Test that create_impression_event creates Event object + with right params when empty attributes are provided and + bot filtering is enabled """ + + expected_params = { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'visitor_id': 'test_user', + 'attributes': [{ + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111129', + 'experiment_id': '111127', + 'campaign_id': '111182' + }], + 'events': [{ + 'timestamp': 42123, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'client_name': 'python-sdk', + 'client_version': version.__version__, + 'anonymize_ip': False, + 'revision': '42' + } + + with mock.patch('time.time', return_value=42.123), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'),\ + mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=True): + event_obj = self.event_builder.create_impression_event( + self.project_config.get_experiment_from_key('test_experiment'), + '111129', 'test_user', None + ) + + self._validate_event_object(event_obj, + event_builder.EventBuilder.EVENTS_URL, + expected_params, + event_builder.EventBuilder.HTTP_VERB, + event_builder.EventBuilder.HTTP_HEADERS) + def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled(self): """ Test that create_impression_event creates Event object with right params when user agent attribute is provided and diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index c9dae8816..6fce67b5a 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1224,7 +1224,12 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_property_fea 'project_id': '111111', 'visitors': [{ 'visitor_id': 'test_user', - 'attributes': [], + 'attributes': [{ + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], 'snapshots': [{ 'decisions': [{ 'variation_id': '111129', From cd7b44808a27e9efd83301735864b707b18d6c3e Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Tue, 12 Jun 2018 16:49:12 +0500 Subject: [PATCH 08/10] :pen: Changes --- optimizely/event_builder.py | 2 +- optimizely/project_config.py | 2 +- tests/test_config.py | 14 +++++--------- tests/test_event_builder.py | 4 ++++ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 2bab469c1..087dc1bf8 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -180,7 +180,7 @@ def _get_attributes(self, attributes): params = [] if isinstance(attributes, dict): - for attribute_key in sorted(attributes.keys()): + for attribute_key in attributes.keys(): attribute_value = attributes.get(attribute_key) # Omit falsy attribute values if attribute_value: diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 53c8a0d33..49ea28c18 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -386,7 +386,7 @@ def get_attribute_id(self, attribute_key): return attribute.id - if has_reserved_prefix and attribute_key != enums.ControlAttributes.BOT_FILTERING: + if has_reserved_prefix: return attribute_key self.logger.error('Attribute "%s" is not in datafile.' % attribute_key) diff --git a/tests/test_config.py b/tests/test_config.py index d1b5e0b05..8bc6ee370 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -691,9 +691,10 @@ def test_get_bot_filtering(self): # Assert bot filtering is retrieved as provided in the data file opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config - self.assertEqual(self.config_dict_with_features['botFiltering'], - project_config.get_bot_filtering_value() - ) + self.assertEqual( + self.config_dict_with_features['botFiltering'], + project_config.get_bot_filtering_value() + ) def test_get_experiment_from_key__valid_key(self): """ Test that experiment is retrieved correctly for valid experiment key. """ @@ -823,12 +824,7 @@ def test_get_attribute_id__unknown_key_with_opt_prefix(self): """ Test that Attribute Key is returned as ID when provided attribute key is not present in the datafile but has $opt prefix. """ self.assertEqual('$opt_interesting', - self.project_config.get_attribute_id('$opt_interesting')) - - def test_get_attribute_id__key_is_bot_filtering_enum(self): - """ Test that None is returned when provided attribute key is - equal to '$opt_bot_filtering'. """ - self.assertIsNone(self.project_config.get_attribute_id('$opt_bot_filtering')) + self.project_config.get_attribute_id('$opt_interesting')) def test_get_group__valid_id(self): """ Test that group is retrieved correctly for valid group ID. """ diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 1cc53f689..60ee7bd23 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -50,7 +50,11 @@ def _validate_event_object(self, event_obj, expected_url, expected_params, expec """ Helper method to validate properties of the event object. """ self.assertEqual(expected_url, event_obj.url) + + expected_params['visitors'][0]['attributes'].sort() + event_obj.params['visitors'][0]['attributes'].sort() self.assertEqual(expected_params, event_obj.params) + self.assertEqual(expected_verb, event_obj.http_verb) self.assertEqual(expected_headers, event_obj.headers) From c4359d12771105dfd2c39b890dc5c74076606d10 Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Tue, 12 Jun 2018 17:22:27 +0500 Subject: [PATCH 09/10] :pen: Sort in unit tests --- tests/test_event_builder.py | 7 +++++-- tests/test_optimizely.py | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index 60ee7bd23..82e1238d0 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -13,6 +13,7 @@ import mock import unittest +from operator import itemgetter from optimizely import event_builder from optimizely import exceptions @@ -51,8 +52,10 @@ def _validate_event_object(self, event_obj, expected_url, expected_params, expec self.assertEqual(expected_url, event_obj.url) - expected_params['visitors'][0]['attributes'].sort() - event_obj.params['visitors'][0]['attributes'].sort() + expected_params['visitors'][0]['attributes'] = \ + sorted(expected_params['visitors'][0]['attributes'], key=itemgetter('key')) + event_obj.params['visitors'][0]['attributes'] = \ + sorted(event_obj.params['visitors'][0]['attributes'], key=itemgetter('key')) self.assertEqual(expected_params, event_obj.params) self.assertEqual(expected_verb, event_obj.http_verb) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 6fce67b5a..e463264c1 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -13,6 +13,7 @@ import json import mock +from operator import itemgetter from optimizely import decision_service from optimizely import entities @@ -53,6 +54,11 @@ def _validate_event_object(self, event_obj, expected_url, expected_params, expec """ Helper method to validate properties of the event object. """ self.assertEqual(expected_url, event_obj.url) + + expected_params['visitors'][0]['attributes'] = \ + sorted(expected_params['visitors'][0]['attributes'], key=itemgetter('key')) + event_obj.params['visitors'][0]['attributes'] = \ + sorted(event_obj.params['visitors'][0]['attributes'], key=itemgetter('key')) self.assertEqual(expected_params, event_obj.params) self.assertEqual(expected_verb, event_obj.http_verb) self.assertEqual(expected_headers, event_obj.headers) From fe607b451cdd43acad445d2ebfaad9a809df001a Mon Sep 17 00:00:00 2001 From: Owais Akbani Date: Fri, 22 Jun 2018 12:44:54 +0500 Subject: [PATCH 10/10] Update testcase overwritten in merge --- tests/test_optimizely.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index be1d17711..f065b0514 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1292,7 +1292,12 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis 'project_id': '111111', 'visitors': [{ 'visitor_id': 'test_user', - 'attributes': [], + 'attributes': [{ + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering' + }], 'snapshots': [{ 'decisions': [{ 'variation_id': '111128',