From efa8849092651943cb290ec0f4ba8d6e4575b36e Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 7 May 2019 17:56:29 -0700 Subject: [PATCH 01/16] Introducing datafile manager package. WIP. --- optimizely/datafile_manager/__init__.py | 12 +++ optimizely/datafile_manager/base.py | 16 ++++ .../polling_datafile_manager.py | 80 +++++++++++++++++++ .../static_datafile_manager.py | 23 ++++++ 4 files changed, 131 insertions(+) create mode 100644 optimizely/datafile_manager/__init__.py create mode 100644 optimizely/datafile_manager/base.py create mode 100644 optimizely/datafile_manager/polling_datafile_manager.py create mode 100644 optimizely/datafile_manager/static_datafile_manager.py diff --git a/optimizely/datafile_manager/__init__.py b/optimizely/datafile_manager/__init__.py new file mode 100644 index 00000000..2d7c2f05 --- /dev/null +++ b/optimizely/datafile_manager/__init__.py @@ -0,0 +1,12 @@ +# Copyright 2019, 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 +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/optimizely/datafile_manager/base.py b/optimizely/datafile_manager/base.py new file mode 100644 index 00000000..b2bffea8 --- /dev/null +++ b/optimizely/datafile_manager/base.py @@ -0,0 +1,16 @@ +# Copyright 2019, 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 +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +class BaseDatafileManager(object): + def get_datafile(self): + raise NotImplementedError diff --git a/optimizely/datafile_manager/polling_datafile_manager.py b/optimizely/datafile_manager/polling_datafile_manager.py new file mode 100644 index 00000000..7d4d0a5e --- /dev/null +++ b/optimizely/datafile_manager/polling_datafile_manager.py @@ -0,0 +1,80 @@ +# Copyright 2019, 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 +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +import asyncio +import http +import requests + +from .base import BaseDatafileManager + + +DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' + + +class PollingDatafileManager(BaseDatafileManager): + + def __init__(self, + sdk_key=None, + url=None, + url_template=None): + self.is_started = False + assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' + datafile_url_template = url_template or DATAFILE_URL_TEMPLATE + if url is None: + self.datafile_url = datafile_url_template.format(sdk_key=sdk_key) + else: + self.datafile_url = url + self.datafile = None + self.last_modified = None + + def __del__(self): + self.stop() + + def set_last_modified(self, response): + self.last_modified = response.headers['Last-Modified'] + + def set_datafile(self, response): + if response.status_code == http.HTTPStatus.NOT_MODIFIED: + return + self.datafile = response.json() + + def fetch_datafile(self): + request_headers = { + 'If-Modified-Since': self.last_modified + } + response = requests.get(self.datafile_url, headers=request_headers) + + if response.status_code == http.HTTPStatus.OK: + self.set_datafile(response) + self.set_last_modified(response) + + async def _run(self): + while True: + self.fetch_datafile() + await asyncio.sleep(5) + + def start(self): + if not self.is_started: + self.is_started = True + event_loop = asyncio.get_event_loop() + event_loop.run_until_complete(self._run()) + + def stop(self): + if self.is_started: + self.is_started = False + event_loop = asyncio.get_event_loop() + event_loop.close() + + def get_datafile(self): + return self.datafile diff --git a/optimizely/datafile_manager/static_datafile_manager.py b/optimizely/datafile_manager/static_datafile_manager.py new file mode 100644 index 00000000..e2f7b2e3 --- /dev/null +++ b/optimizely/datafile_manager/static_datafile_manager.py @@ -0,0 +1,23 @@ +# Copyright 2019, 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 +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from .base import BaseDatafileManager + + +class StaticDatafileManager(BaseDatafileManager): + + def __init__(self, datafile): + self.datafile = datafile + + def get_datafile(self): + return self.datafile From 0cedd69806e4e8111c2d37df126c885bcf661876 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 10 May 2019 16:45:04 -0700 Subject: [PATCH 02/16] Moving to use threading --- optimizely/datafile_manager/__init__.py | 12 --- optimizely/datafile_manager/base.py | 16 ---- .../polling_datafile_manager.py | 80 ------------------- .../static_datafile_manager.py | 23 ------ optimizely/optimizely.py | 3 +- tests/test_optimizely.py | 4 +- 6 files changed, 4 insertions(+), 134 deletions(-) delete mode 100644 optimizely/datafile_manager/__init__.py delete mode 100644 optimizely/datafile_manager/base.py delete mode 100644 optimizely/datafile_manager/polling_datafile_manager.py delete mode 100644 optimizely/datafile_manager/static_datafile_manager.py diff --git a/optimizely/datafile_manager/__init__.py b/optimizely/datafile_manager/__init__.py deleted file mode 100644 index 2d7c2f05..00000000 --- a/optimizely/datafile_manager/__init__.py +++ /dev/null @@ -1,12 +0,0 @@ -# Copyright 2019, 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 -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/optimizely/datafile_manager/base.py b/optimizely/datafile_manager/base.py deleted file mode 100644 index b2bffea8..00000000 --- a/optimizely/datafile_manager/base.py +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2019, 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 -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -class BaseDatafileManager(object): - def get_datafile(self): - raise NotImplementedError diff --git a/optimizely/datafile_manager/polling_datafile_manager.py b/optimizely/datafile_manager/polling_datafile_manager.py deleted file mode 100644 index 7d4d0a5e..00000000 --- a/optimizely/datafile_manager/polling_datafile_manager.py +++ /dev/null @@ -1,80 +0,0 @@ -# Copyright 2019, 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 -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -import asyncio -import http -import requests - -from .base import BaseDatafileManager - - -DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' - - -class PollingDatafileManager(BaseDatafileManager): - - def __init__(self, - sdk_key=None, - url=None, - url_template=None): - self.is_started = False - assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' - datafile_url_template = url_template or DATAFILE_URL_TEMPLATE - if url is None: - self.datafile_url = datafile_url_template.format(sdk_key=sdk_key) - else: - self.datafile_url = url - self.datafile = None - self.last_modified = None - - def __del__(self): - self.stop() - - def set_last_modified(self, response): - self.last_modified = response.headers['Last-Modified'] - - def set_datafile(self, response): - if response.status_code == http.HTTPStatus.NOT_MODIFIED: - return - self.datafile = response.json() - - def fetch_datafile(self): - request_headers = { - 'If-Modified-Since': self.last_modified - } - response = requests.get(self.datafile_url, headers=request_headers) - - if response.status_code == http.HTTPStatus.OK: - self.set_datafile(response) - self.set_last_modified(response) - - async def _run(self): - while True: - self.fetch_datafile() - await asyncio.sleep(5) - - def start(self): - if not self.is_started: - self.is_started = True - event_loop = asyncio.get_event_loop() - event_loop.run_until_complete(self._run()) - - def stop(self): - if self.is_started: - self.is_started = False - event_loop = asyncio.get_event_loop() - event_loop.close() - - def get_datafile(self): - return self.datafile diff --git a/optimizely/datafile_manager/static_datafile_manager.py b/optimizely/datafile_manager/static_datafile_manager.py deleted file mode 100644 index e2f7b2e3..00000000 --- a/optimizely/datafile_manager/static_datafile_manager.py +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright 2019, 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 -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from .base import BaseDatafileManager - - -class StaticDatafileManager(BaseDatafileManager): - - def __init__(self, datafile): - self.datafile = datafile - - def get_datafile(self): - return self.datafile diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 559bb722..d57ef942 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -12,12 +12,13 @@ # limitations under the License. from six import string_types +from . import config_manager from . import decision_service from . import entities from . import event_builder from . import exceptions -from . import logger as _logging from . import project_config +from . import logger as _logging from .error_handler import NoOpErrorHandler as noop_error_handler from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index eacef745..8a3b92f6 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -22,7 +22,7 @@ from optimizely import exceptions from optimizely import logger from optimizely import optimizely -from optimizely import project_config +from optimizely import config_manager from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter @@ -164,7 +164,7 @@ def test_init__unsupported_datafile_version__logs_error(self): def test_init_with_supported_datafile_version(self): """ Test that datafile with supported version works as expected. """ - self.assertTrue(self.config_dict['version'] in project_config.SUPPORTED_VERSIONS) + self.assertTrue(self.config_dict['version'] in config_manager.SUPPORTED_VERSIONS) mock_client_logger = mock.MagicMock() with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): From 002f2b1b2ca8f2d0e0364398189beeb9b00dddf6 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:44:49 -0700 Subject: [PATCH 03/16] Introducing config manager with support for polling. --- optimizely/logger.py | 2 +- optimizely/optimizely.py | 1 - tests/test_optimizely.py | 7 ++++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/optimizely/logger.py b/optimizely/logger.py index 9530b132..80c1e547 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -1,4 +1,4 @@ -# Copyright 2016, 2018-2019, 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/optimizely.py b/optimizely/optimizely.py index d57ef942..efeabb7b 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -12,7 +12,6 @@ # limitations under the License. from six import string_types -from . import config_manager from . import decision_service from . import entities from . import event_builder diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 8a3b92f6..24c7e15e 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -12,17 +12,18 @@ # limitations under the License. import json -import mock from operator import itemgetter -from optimizely import decision_service +import mock + +import optimizely.config_manager +from optimizely import decision_service, config_manager from optimizely import entities from optimizely import error_handler from optimizely import event_builder from optimizely import exceptions from optimizely import logger from optimizely import optimizely -from optimizely import config_manager from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter From f7a646e66da1e26d6b49eca10160729868e6d2ab Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:51:03 -0700 Subject: [PATCH 04/16] undoing some changes --- optimizely/optimizely.py | 2 +- tests/test_optimizely.py | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index efeabb7b..559bb722 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -16,8 +16,8 @@ from . import entities from . import event_builder from . import exceptions -from . import project_config from . import logger as _logging +from . import project_config from .error_handler import NoOpErrorHandler as noop_error_handler from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 24c7e15e..eacef745 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -12,18 +12,17 @@ # limitations under the License. import json -from operator import itemgetter - import mock +from operator import itemgetter -import optimizely.config_manager -from optimizely import decision_service, config_manager +from optimizely import decision_service from optimizely import entities from optimizely import error_handler from optimizely import event_builder from optimizely import exceptions from optimizely import logger from optimizely import optimizely +from optimizely import project_config from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter @@ -165,7 +164,7 @@ def test_init__unsupported_datafile_version__logs_error(self): def test_init_with_supported_datafile_version(self): """ Test that datafile with supported version works as expected. """ - self.assertTrue(self.config_dict['version'] in config_manager.SUPPORTED_VERSIONS) + self.assertTrue(self.config_dict['version'] in project_config.SUPPORTED_VERSIONS) mock_client_logger = mock.MagicMock() with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): From 971f481b19abd9c62f61fa86867d033421124326 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:51:53 -0700 Subject: [PATCH 05/16] Fixing year --- optimizely/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/logger.py b/optimizely/logger.py index 80c1e547..9530b132 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -1,4 +1,4 @@ -# Copyright 2016, 2018, Optimizely +# Copyright 2016, 2018-2019, 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 From f810a040ae3f0e44f7237fd91431814fd01ed97d Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Mon, 3 Jun 2019 17:22:00 -0700 Subject: [PATCH 06/16] Getting ProjectConfig from optimizely.config_manager. WIP. --- optimizely/decision_service.py | 2 +- optimizely/helpers/enums.py | 19 +-- optimizely/optimizely.py | 209 ++++++++++++++++++--------- optimizely/project_config.py | 20 +-- tests/base.py | 2 +- tests/helpers_tests/test_audience.py | 6 +- tests/test_config.py | 54 +++---- tests/test_decision_service.py | 3 +- tests/test_optimizely.py | 118 +++++++-------- 9 files changed, 252 insertions(+), 181 deletions(-) diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index ce09c403..d8b08f9e 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -412,7 +412,7 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes )) return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST) else: - self.logger.error(enums.Errors.INVALID_GROUP_ID_ERROR.format('_get_variation_for_feature')) + self.logger.error(enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature')) # Next check if the feature is being experimented on elif feature.experimentIds: diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index f86f584e..6dbd2dd1 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -71,18 +71,19 @@ class DecisionSources(object): class Errors(object): - INVALID_ATTRIBUTE_ERROR = 'Provided attribute is not in datafile.' + INVALID_ATTRIBUTE = 'Provided attribute is not in datafile.' INVALID_ATTRIBUTE_FORMAT = 'Attributes provided are in an invalid format.' - INVALID_AUDIENCE_ERROR = 'Provided audience is not in datafile.' + INVALID_AUDIENCE = 'Provided audience is not in datafile.' INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".' INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.' - INVALID_EXPERIMENT_KEY_ERROR = 'Provided experiment is not in datafile.' - INVALID_EVENT_KEY_ERROR = 'Provided event is not in datafile.' - INVALID_FEATURE_KEY_ERROR = 'Provided feature key is not in the datafile.' - INVALID_GROUP_ID_ERROR = 'Provided group is not in datafile.' - INVALID_INPUT_ERROR = 'Provided "{}" is in an invalid format.' - INVALID_VARIATION_ERROR = 'Provided variation is not in datafile.' - INVALID_VARIABLE_KEY_ERROR = 'Provided variable key is not in the feature flag.' + INVALID_EXPERIMENT_KEY = 'Provided experiment is not in datafile.' + INVALID_EVENT_KEY = 'Provided event is not in datafile.' + INVALID_FEATURE_KEY = 'Provided feature key is not in the datafile.' + INVALID_GROUP_ID = 'Provided group is not in datafile.' + INVALID_INPUT = 'Provided "{}" is in an invalid format.' + INVALID_PROJECT_CONFIG = 'Invalid config. Optimizely instance is not valid. Failing "{}".' + INVALID_VARIATION = 'Provided variation is not in datafile.' + INVALID_VARIABLE_KEY = 'Provided variable key is not in the feature flag.' NONE_FEATURE_KEY_PARAMETER = '"None" is an invalid value for feature key.' NONE_USER_ID_PARAMETER = '"None" is an invalid value for user ID.' NONE_VARIABLE_KEY_PARAMETER = '"None" is an invalid value for variable key.' diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 559bb722..2a360578 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -17,7 +17,8 @@ from . import event_builder from . import exceptions from . import logger as _logging -from . import project_config +from .config_manager import StaticConfigManager +from .config_manager import PollingConfigManager from .error_handler import NoOpErrorHandler as noop_error_handler from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums @@ -29,16 +30,18 @@ class Optimizely(object): """ Class encapsulating all SDK functionality. """ def __init__(self, - datafile, + datafile=None, event_dispatcher=None, logger=None, error_handler=None, skip_json_validation=False, - user_profile_service=None): + user_profile_service=None, + sdk_key=None, + config_manager=None): """ Optimizely init method for managing Custom projects. Args: - datafile: JSON string representing the project. + datafile: Optional JSON string representing the project. Must provide at least one of datafile or sdk_key. event_dispatcher: Provides a dispatch_event method which if given a URL and params sends a request to it. logger: Optional component which provides a log method to log messages. By default nothing would be logged. error_handler: Optional component which provides a handle_error method to handle exceptions. @@ -46,12 +49,27 @@ def __init__(self, skip_json_validation: Optional boolean param which allows skipping JSON schema validation upon object invocation. By default JSON schema validation will be performed. user_profile_service: Optional component which provides methods to store and manage user profiles. + sdk_key: Optional string uniquely identifying the datafile corresponding to project and environment combination. + Must provide at least one of datafile or sdk_key. + config_manager: Optional component which implements optimizely.config_manager.BaseConfigManager. """ self.logger_name = '.'.join([__name__, self.__class__.__name__]) self.is_valid = True self.event_dispatcher = event_dispatcher or default_event_dispatcher self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) self.error_handler = error_handler or noop_error_handler + self.config_manager = config_manager + + if self.config_manager is None: + if sdk_key: + self.config_manager = PollingConfigManager(sdk_key=sdk_key, + datafile=datafile, + logger=self.logger, + error_handler=self.error_handler) + else: + self.config_manager = StaticConfigManager(datafile=datafile, + logger=self.logger, + error_handler=self.error_handler) try: self._validate_instantiation_options(datafile, skip_json_validation) @@ -63,25 +81,6 @@ def __init__(self, self.logger.exception(str(error)) return - error_msg = None - try: - self.config = project_config.ProjectConfig(datafile, self.logger, self.error_handler) - except exceptions.UnsupportedDatafileVersionException as error: - error_msg = error.args[0] - error_to_handle = error - except: - error_msg = enums.Errors.INVALID_INPUT_ERROR.format('datafile') - error_to_handle = exceptions.InvalidInputException(error_msg) - finally: - if error_msg: - self.is_valid = False - # We actually want to log this error to stderr, so make sure the logger - # has a handler capable of doing that. - self.logger = _logging.reset_logger(self.logger_name) - self.logger.exception(error_msg) - self.error_handler.handle_error(error_to_handle) - return - self.event_builder = event_builder.EventBuilder() self.decision_service = decision_service.DecisionService(self.logger, user_profile_service) self.notification_center = notification_center(self.logger) @@ -98,16 +97,16 @@ def _validate_instantiation_options(self, datafile, skip_json_validation): """ if not skip_json_validation and not validator.is_datafile_valid(datafile): - raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('datafile')) + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('datafile')) if not validator.is_event_dispatcher_valid(self.event_dispatcher): - raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('event_dispatcher')) + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('event_dispatcher')) if not validator.is_logger_valid(self.logger): - raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('logger')) + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('logger')) if not validator.is_error_handler_valid(self.error_handler): - raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT_ERROR.format('error_handler')) + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('error_handler')) def _validate_user_inputs(self, attributes=None, event_tags=None): """ Helper method to validate user inputs. @@ -133,10 +132,11 @@ def _validate_user_inputs(self, attributes=None, event_tags=None): return True - def _send_impression_event(self, experiment, variation, user_id, attributes): + def _send_impression_event(self, project_config, experiment, variation, user_id, attributes): """ Helper method to send impression event. Args: + project_config: Instance of ProjectConfig. experiment: Experiment for which impression event is being sent. variation: Variation picked for user for the given experiment. user_id: ID for user. @@ -144,7 +144,7 @@ def _send_impression_event(self, experiment, variation, user_id, attributes): """ impression_event = self.event_builder.create_impression_event( - self.config, + project_config, experiment, variation.id, user_id, @@ -164,10 +164,17 @@ def _send_impression_event(self, experiment, variation, user_id, attributes): self.notification_center.send_notifications(enums.NotificationTypes.ACTIVATE, experiment, user_id, attributes, variation, impression_event) - def _get_feature_variable_for_type(self, feature_key, variable_key, variable_type, user_id, attributes): + def _get_feature_variable_for_type(self, + project_config, + feature_key, + variable_key, + variable_type, + user_id, + attributes): """ Helper method to determine value for a certain variable attached to a feature flag based on type of variable. Args: + project_config: Instance of ProjectConfig. feature_key: Key of the feature whose variable's value is being accessed. variable_key: Key of the variable whose value is to be accessed. variable_type: Type of variable which could be one of boolean/double/integer/string. @@ -181,25 +188,25 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ - Mismatch with type of variable. """ if not validator.is_non_empty_string(feature_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('feature_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('feature_key')) return None if not validator.is_non_empty_string(variable_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('variable_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('variable_key')) return None if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return None if not self._validate_user_inputs(attributes): return None - feature_flag = self.config.get_feature_from_key(feature_key) + feature_flag = project_config.get_feature_from_key(feature_key) if not feature_flag: return None - variable = self.config.get_variable_for_feature(feature_key, variable_key) + variable = project_config.get_variable_for_feature(feature_key, variable_key) if not variable: return None @@ -214,12 +221,12 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ feature_enabled = False source_info = {} variable_value = variable.defaultValue - decision = self.decision_service.get_variation_for_feature(self.config, feature_flag, user_id, attributes) + decision = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_id, attributes) if decision.variation: feature_enabled = decision.variation.featureEnabled if feature_enabled: - variable_value = self.config.get_variable_value_for_variation(variable, decision.variation) + variable_value = project_config.get_variable_value_for_variation(variable, decision.variation) self.logger.info( 'Got variable value "%s" for variable "%s" of feature flag "%s".' % ( variable_value, variable_key, feature_key @@ -243,7 +250,7 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ } try: - actual_value = self.config.get_typecast_value(variable_value, variable_type) + actual_value = project_config.get_typecast_value(variable_value, variable_type) except: self.logger.error('Unable to cast value. Returning None.') actual_value = None @@ -283,11 +290,11 @@ def activate(self, experiment_key, user_id, attributes=None): return None if not validator.is_non_empty_string(experiment_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('experiment_key')) return None if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return None variation_key = self.get_variation(experiment_key, user_id, attributes) @@ -296,12 +303,17 @@ def activate(self, experiment_key, user_id, attributes=None): self.logger.info('Not activating user "%s".' % user_id) return None - experiment = self.config.get_experiment_from_key(experiment_key) - variation = self.config.get_variation_from_key(experiment_key, variation_key) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('activate')) + return None + + experiment = project_config.get_experiment_from_key(experiment_key) + variation = project_config.get_variation_from_key(experiment_key, variation_key) # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(experiment, variation, user_id, attributes) + self._send_impression_event(project_config, experiment, variation, user_id, attributes) return variation.key @@ -320,23 +332,28 @@ def track(self, event_key, user_id, attributes=None, event_tags=None): return if not validator.is_non_empty_string(event_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('event_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('event_key')) return if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return if not self._validate_user_inputs(attributes, event_tags): return - event = self.config.get_event(event_key) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('track')) + return + + event = project_config.get_event(event_key) if not event: self.logger.info('Not tracking user "%s" for event "%s".' % (user_id, event_key)) return conversion_event = self.event_builder.create_conversion_event( - self.config, + project_config, event_key, user_id, attributes, @@ -372,14 +389,19 @@ def get_variation(self, experiment_key, user_id, attributes=None): return None if not validator.is_non_empty_string(experiment_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('experiment_key')) return None if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) + return None + + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_variation')) return None - experiment = self.config.get_experiment_from_key(experiment_key) + experiment = project_config.get_experiment_from_key(experiment_key) variation_key = None if not experiment: @@ -392,11 +414,11 @@ def get_variation(self, experiment_key, user_id, attributes=None): if not self._validate_user_inputs(attributes): return None - variation = self.decision_service.get_variation(self.config, experiment, user_id, attributes) + variation = self.decision_service.get_variation(project_config, experiment, user_id, attributes) if variation: variation_key = variation.key - if self.config.is_feature_experiment(experiment.id): + if project_config.is_feature_experiment(experiment.id): decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST else: decision_notification_type = enums.DecisionNotificationTypes.AB_TEST @@ -431,23 +453,28 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): return False if not validator.is_non_empty_string(feature_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('feature_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('feature_key')) return False if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return False if not self._validate_user_inputs(attributes): return False - feature = self.config.get_feature_from_key(feature_key) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('is_feature_enabled')) + return False + + feature = project_config.get_feature_from_key(feature_key) if not feature: return False feature_enabled = False source_info = {} - decision = self.decision_service.get_variation_for_feature(self.config, feature, user_id, attributes) + decision = self.decision_service.get_variation_for_feature(project_config, feature, user_id, attributes) is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST if decision.variation: @@ -459,7 +486,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): 'experiment_key': decision.experiment.key, 'variation_key': decision.variation.key } - self._send_impression_event(decision.experiment, + self._send_impression_event(project_config, + decision.experiment, decision.variation, user_id, attributes) @@ -501,13 +529,18 @@ def get_enabled_features(self, user_id, attributes=None): return enabled_features if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return enabled_features if not self._validate_user_inputs(attributes): return enabled_features - for feature in self.config.feature_key_map.values(): + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_enabled_features')) + return enabled_features + + for feature in project_config.feature_key_map.values(): if self.is_feature_enabled(feature.key, user_id, attributes): enabled_features.append(feature.key) @@ -530,7 +563,14 @@ def get_feature_variable_boolean(self, feature_key, variable_key, user_id, attri """ variable_type = entities.Variable.Type.BOOLEAN - return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_feature_variable_boolean')) + return None + + return self._get_feature_variable_for_type( + project_config, feature_key, variable_key, variable_type, user_id, attributes + ) def get_feature_variable_double(self, feature_key, variable_key, user_id, attributes=None): """ Returns value for a certain double variable attached to a feature flag. @@ -549,7 +589,14 @@ def get_feature_variable_double(self, feature_key, variable_key, user_id, attrib """ variable_type = entities.Variable.Type.DOUBLE - return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_feature_variable_double')) + return None + + return self._get_feature_variable_for_type( + project_config, feature_key, variable_key, variable_type, user_id, attributes + ) def get_feature_variable_integer(self, feature_key, variable_key, user_id, attributes=None): """ Returns value for a certain integer variable attached to a feature flag. @@ -568,7 +615,14 @@ def get_feature_variable_integer(self, feature_key, variable_key, user_id, attri """ variable_type = entities.Variable.Type.INTEGER - return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_feature_variable_integer')) + return None + + return self._get_feature_variable_for_type( + project_config, feature_key, variable_key, variable_type, user_id, attributes + ) def get_feature_variable_string(self, feature_key, variable_key, user_id, attributes=None): """ Returns value for a certain string variable attached to a feature. @@ -587,7 +641,14 @@ def get_feature_variable_string(self, feature_key, variable_key, user_id, attrib """ variable_type = entities.Variable.Type.STRING - return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_feature_variable_string')) + return None + + return self._get_feature_variable_for_type( + project_config, feature_key, variable_key, variable_type, user_id, attributes + ) def set_forced_variation(self, experiment_key, user_id, variation_key): """ Force a user into a variation for a given experiment. @@ -607,14 +668,19 @@ def set_forced_variation(self, experiment_key, user_id, variation_key): return False if not validator.is_non_empty_string(experiment_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('experiment_key')) return False if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return False - return self.decision_service.set_forced_variation(self.config, experiment_key, user_id, variation_key) + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('set_forced_variation')) + return False + + return self.decision_service.set_forced_variation(project_config, experiment_key, user_id, variation_key) def get_forced_variation(self, experiment_key, user_id): """ Gets the forced variation for a given user and experiment. @@ -632,12 +698,17 @@ def get_forced_variation(self, experiment_key, user_id): return None if not validator.is_non_empty_string(experiment_key): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key')) + self.logger.error(enums.Errors.INVALID_INPUT.format('experiment_key')) return None if not isinstance(user_id, string_types): - self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id')) + self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) + return None + + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_forced_variation')) return None - forced_variation = self.decision_service.get_forced_variation(self.config, experiment_key, user_id) + forced_variation = self.decision_service.get_forced_variation(project_config, experiment_key, user_id) return forced_variation.key if forced_variation else None diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 0c29fb3c..52e58837 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -234,7 +234,7 @@ def get_experiment_from_key(self, experiment_key): return experiment self.logger.error('Experiment key "%s" is not in datafile.' % experiment_key) - self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY_ERROR)) + self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY)) return None def get_experiment_from_id(self, experiment_id): @@ -253,7 +253,7 @@ def get_experiment_from_id(self, experiment_id): return experiment self.logger.error('Experiment ID "%s" is not in datafile.' % experiment_id) - self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY_ERROR)) + self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY)) return None def get_group(self, group_id): @@ -272,7 +272,7 @@ def get_group(self, group_id): return group self.logger.error('Group ID "%s" is not in datafile.' % group_id) - self.error_handler.handle_error(exceptions.InvalidGroupException(enums.Errors.INVALID_GROUP_ID_ERROR)) + self.error_handler.handle_error(exceptions.InvalidGroupException(enums.Errors.INVALID_GROUP_ID)) return None def get_audience(self, audience_id): @@ -290,7 +290,7 @@ def get_audience(self, audience_id): return audience self.logger.error('Audience ID "%s" is not in datafile.' % audience_id) - self.error_handler.handle_error(exceptions.InvalidAudienceException((enums.Errors.INVALID_AUDIENCE_ERROR))) + self.error_handler.handle_error(exceptions.InvalidAudienceException((enums.Errors.INVALID_AUDIENCE))) def get_variation_from_key(self, experiment_key, variation_key): """ Get variation given experiment and variation key. @@ -311,11 +311,11 @@ def get_variation_from_key(self, experiment_key, variation_key): return variation else: self.logger.error('Variation key "%s" is not in datafile.' % variation_key) - self.error_handler.handle_error(exceptions.InvalidVariationException(enums.Errors.INVALID_VARIATION_ERROR)) + self.error_handler.handle_error(exceptions.InvalidVariationException(enums.Errors.INVALID_VARIATION)) return None self.logger.error('Experiment key "%s" is not in datafile.' % experiment_key) - self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY_ERROR)) + self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY)) return None def get_variation_from_id(self, experiment_key, variation_id): @@ -337,11 +337,11 @@ def get_variation_from_id(self, experiment_key, variation_id): return variation else: self.logger.error('Variation ID "%s" is not in datafile.' % variation_id) - self.error_handler.handle_error(exceptions.InvalidVariationException(enums.Errors.INVALID_VARIATION_ERROR)) + self.error_handler.handle_error(exceptions.InvalidVariationException(enums.Errors.INVALID_VARIATION)) return None self.logger.error('Experiment key "%s" is not in datafile.' % experiment_key) - self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY_ERROR)) + self.error_handler.handle_error(exceptions.InvalidExperimentException(enums.Errors.INVALID_EXPERIMENT_KEY)) return None def get_event(self, event_key): @@ -360,7 +360,7 @@ def get_event(self, event_key): return event self.logger.error('Event "%s" is not in datafile.' % event_key) - self.error_handler.handle_error(exceptions.InvalidEventException(enums.Errors.INVALID_EVENT_KEY_ERROR)) + self.error_handler.handle_error(exceptions.InvalidEventException(enums.Errors.INVALID_EVENT_KEY)) return None def get_attribute_id(self, attribute_key): @@ -387,7 +387,7 @@ def get_attribute_id(self, attribute_key): return attribute_key self.logger.error('Attribute "%s" is not in datafile.' % attribute_key) - self.error_handler.handle_error(exceptions.InvalidAttributeException(enums.Errors.INVALID_ATTRIBUTE_ERROR)) + self.error_handler.handle_error(exceptions.InvalidAttributeException(enums.Errors.INVALID_ATTRIBUTE)) return None def get_feature_from_key(self, feature_key): diff --git a/tests/base.py b/tests/base.py index 07f025b8..57e31738 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1078,4 +1078,4 @@ def setUp(self, config_dict='config_dict'): config = getattr(self, config_dict) self.optimizely = optimizely.Optimizely(json.dumps(config)) - self.project_config = self.optimizely.config + self.project_config = self.optimizely.config_manager.get_config() diff --git a/tests/helpers_tests/test_audience.py b/tests/helpers_tests/test_audience.py index e8174ee1..4a586f4d 100644 --- a/tests/helpers_tests/test_audience.py +++ b/tests/helpers_tests/test_audience.py @@ -148,7 +148,7 @@ def test_is_user_in_experiment__evaluates_audience_conditions(self): calls custom attribute evaluator for leaf nodes. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() experiment = project_config.get_experiment_from_key('audience_combinations_experiment') experiment.audienceIds = [] experiment.audienceConditions = ['or', ['or', '3468206642', '3988293898'], ['or', '3988293899', '3468206646', ]] @@ -176,7 +176,7 @@ def test_is_user_in_experiment__evaluates_audience_conditions_leaf_node(self): """ Test that is_user_in_experiment correctly evaluates leaf node in audienceConditions. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() experiment = project_config.get_experiment_from_key('audience_combinations_experiment') experiment.audienceConditions = '3468206645' @@ -236,7 +236,7 @@ def test_is_user_in_experiment__evaluates_audienceIds(self): def test_is_user_in_experiment__evaluates_audience_conditions(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() experiment = project_config.get_experiment_from_key('audience_combinations_experiment') experiment.audienceIds = [] experiment.audienceConditions = ['or', ['or', '3468206642', '3988293898', '3988293899']] diff --git a/tests/test_config.py b/tests/test_config.py index fd971c67..305cf88a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -385,7 +385,7 @@ def test_init__with_v4_datafile(self): } test_obj = optimizely.Optimizely(json.dumps(config_dict)) - project_config = test_obj.config + project_config = test_obj.config_manager.get_config() self.assertEqual(config_dict['accountId'], project_config.account_id) self.assertEqual(config_dict['projectId'], project_config.project_id) self.assertEqual(config_dict['revision'], project_config.revision) @@ -699,7 +699,7 @@ 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 + project_config = opt_obj.config_manager.get_config() self.assertEqual( self.config_dict_with_features['botFiltering'], project_config.get_bot_filtering_value() @@ -771,8 +771,8 @@ def test_get_audience__invalid_id(self): self.assertIsNone(self.project_config.get_audience('42')) def test_get_audience__prefers_typedAudiences_over_audiences(self): - opt = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) - config = opt.config + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) + config = opt_obj.config_manager.get_config() audiences = self.config_dict_with_typed_audiences['audiences'] typed_audiences = self.config_dict_with_typed_audiences['typedAudiences'] @@ -889,7 +889,7 @@ def test_get_group__invalid_id(self): def test_get_feature_from_key__valid_feature_key(self): """ Test that a valid feature is returned given a valid feature key. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() expected_feature = entities.FeatureFlag( '91112', @@ -910,7 +910,7 @@ def test_get_feature_from_key__invalid_feature_key(self): """ Test that None is returned given an invalid feature key. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() self.assertIsNone(project_config.get_feature_from_key('invalid_feature_key')) @@ -918,7 +918,7 @@ def test_get_rollout_from_id__valid_rollout_id(self): """ Test that a valid rollout is returned """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() expected_rollout = entities.Layer('211111', [{ 'id': '211127', @@ -998,7 +998,7 @@ def test_get_rollout_from_id__invalid_rollout_id(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features), logger=logger.NoOpLogger()) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() with mock.patch.object(project_config, 'logger') as mock_config_logging: self.assertIsNone(project_config.get_rollout_from_id('aabbccdd')) @@ -1007,7 +1007,7 @@ def test_get_rollout_from_id__invalid_rollout_id(self): def test_get_variable_value_for_variation__returns_valid_value(self): """ Test that the right value is returned. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() variation = project_config.get_variation_from_id('test_experiment', '111128') is_working_variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'is_working') @@ -1019,7 +1019,7 @@ def test_get_variable_value_for_variation__invalid_variable(self): """ Test that an invalid variable key will return None. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() variation = project_config.get_variation_from_id('test_experiment', '111128') self.assertIsNone(project_config.get_variable_value_for_variation(None, variation)) @@ -1028,7 +1028,7 @@ def test_get_variable_value_for_variation__no_variables_for_variation(self): """ Test that a variation with no variables will return None. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() variation = entities.Variation('1111281', 'invalid_variation', []) is_working_variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'is_working') @@ -1038,7 +1038,7 @@ def test_get_variable_value_for_variation__no_usage_of_variable(self): """ Test that a variable with no usage will return default value for variable. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() variation = project_config.get_variation_from_id('test_experiment', '111128') variable_without_usage_variable = project_config.get_variable_for_feature('test_feature_in_experiment', @@ -1049,7 +1049,7 @@ def test_get_variable_for_feature__returns_valid_variable(self): """ Test that the feature variable is returned. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'is_working') self.assertEqual(entities.Variable('127', 'is_working', 'boolean', 'true'), variable) @@ -1058,7 +1058,7 @@ def test_get_variable_for_feature__invalid_feature_key(self): """ Test that an invalid feature key will return None. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() self.assertIsNone(project_config.get_variable_for_feature('invalid_feature', 'is_working')) @@ -1066,7 +1066,7 @@ def test_get_variable_for_feature__invalid_variable_key(self): """ Test that an invalid variable key will return None. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() self.assertIsNone(project_config.get_variable_for_feature('test_feature_in_experiment', 'invalid_variable_key')) @@ -1077,7 +1077,7 @@ def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), logger=logger.SimpleLogger()) - self.project_config = self.optimizely.config + self.project_config = self.optimizely.config_manager.get_config() def test_get_experiment_from_key__invalid_key(self): """ Test that message is logged when provided experiment key is invalid. """ @@ -1169,76 +1169,76 @@ def setUp(self): base.BaseTest.setUp(self) self.optimizely = optimizely.Optimizely(json.dumps(self.config_dict), error_handler=error_handler.RaiseExceptionErrorHandler) - self.project_config = self.optimizely.config + self.project_config = self.optimizely.config_manager.get_config() def test_get_experiment_from_key__invalid_key(self): """ Test that exception is raised when provided experiment key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidExperimentException, - enums.Errors.INVALID_EXPERIMENT_KEY_ERROR, + enums.Errors.INVALID_EXPERIMENT_KEY, self.project_config.get_experiment_from_key, 'invalid_key') def test_get_audience__invalid_id(self): """ Test that message is logged when provided audience ID is invalid. """ self.assertRaisesRegexp(exceptions.InvalidAudienceException, - enums.Errors.INVALID_AUDIENCE_ERROR, + enums.Errors.INVALID_AUDIENCE, self.project_config.get_audience, '42') def test_get_variation_from_key__invalid_experiment_key(self): """ Test that exception is raised when provided experiment key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidExperimentException, - enums.Errors.INVALID_EXPERIMENT_KEY_ERROR, + enums.Errors.INVALID_EXPERIMENT_KEY, self.project_config.get_variation_from_key, 'invalid_key', 'control') def test_get_variation_from_key__invalid_variation_key(self): """ Test that exception is raised when provided variation key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidVariationException, - enums.Errors.INVALID_VARIATION_ERROR, + enums.Errors.INVALID_VARIATION, self.project_config.get_variation_from_key, 'test_experiment', 'invalid_key') def test_get_variation_from_id__invalid_experiment_key(self): """ Test that exception is raised when provided experiment key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidExperimentException, - enums.Errors.INVALID_EXPERIMENT_KEY_ERROR, + enums.Errors.INVALID_EXPERIMENT_KEY, self.project_config.get_variation_from_id, 'invalid_key', '111128') def test_get_variation_from_id__invalid_variation_id(self): """ Test that exception is raised when provided variation ID is invalid. """ self.assertRaisesRegexp(exceptions.InvalidVariationException, - enums.Errors.INVALID_VARIATION_ERROR, + enums.Errors.INVALID_VARIATION, self.project_config.get_variation_from_key, 'test_experiment', '42') def test_get_event__invalid_key(self): """ Test that exception is raised when provided event key is invalid. """ self.assertRaisesRegexp(exceptions.InvalidEventException, - enums.Errors.INVALID_EVENT_KEY_ERROR, + enums.Errors.INVALID_EVENT_KEY, self.project_config.get_event, 'invalid_key') 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, + enums.Errors.INVALID_ATTRIBUTE, 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. """ self.assertRaisesRegexp(exceptions.InvalidGroupException, - enums.Errors.INVALID_GROUP_ID_ERROR, + enums.Errors.INVALID_GROUP_ID, self.project_config.get_group, '42') def test_is_feature_experiment(self): """ Test that a true is returned if experiment is a feature test, false otherwise. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() experiment = project_config.get_experiment_from_key('test_experiment2') feature_experiment = project_config.get_experiment_from_key('test_experiment') diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 3dab0131..2d0df8c5 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -568,7 +568,7 @@ class FeatureFlagDecisionTests(base.BaseTest): def setUp(self): base.BaseTest.setUp(self) opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - self.project_config = opt_obj.config + self.project_config = opt_obj.config_manager.get_config() self.decision_service = opt_obj.decision_service self.mock_decision_logger = mock.patch.object(self.decision_service, 'logger') self.mock_config_logger = mock.patch.object(self.project_config, 'logger') @@ -856,7 +856,6 @@ def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): ) mock_decision_service_logging.error.assert_called_once_with( enums.Errors.INVALID_GROUP_ID_ERROR.format('_get_variation_for_feature') - ) def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature(self): """ Test that if a user is in the mutex group but the experiment is diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index eacef745..ae2d64dc 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -536,7 +536,7 @@ def test_is_feature_enabled__callback_listener(self): Also confirm that impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') access_callback = [False] @@ -560,7 +560,7 @@ def on_activate(experiment, user_id, attributes, variation, event): mock.patch('time.time', return_value=42): self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) self.assertTrue(access_callback[0]) def test_is_feature_enabled_rollout_callback_listener(self): @@ -568,7 +568,7 @@ def test_is_feature_enabled_rollout_callback_listener(self): Also confirm that no impression event is dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') access_callback = [False] @@ -1475,7 +1475,7 @@ def test_get_variation_with_experiment_in_feature(self): get_variation returns feature experiment variation.""" opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() with mock.patch( 'optimizely.decision_service.DecisionService.get_variation', @@ -1628,7 +1628,7 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab decision listener is called with proper parameters """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') mock_experiment = project_config.get_experiment_from_key('test_experiment') @@ -1649,7 +1649,7 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab mock.patch('time.time', return_value=42): self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, @@ -1709,7 +1709,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis decision is broadcasted with proper parameters """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') mock_experiment = project_config.get_experiment_from_key('test_experiment') @@ -1730,7 +1730,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis mock.patch('time.time', return_value=42): self.assertFalse(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, @@ -1791,7 +1791,7 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled decision is broadcasted with proper parameters """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') mock_experiment = project_config.get_experiment_from_key('test_experiment') @@ -1812,7 +1812,7 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled mock.patch('time.time', return_value=42): self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, @@ -1836,7 +1836,7 @@ def test_is_feature_enabled__returns_false_for_feature_rollout_if_feature_disabl decision is broadcasted with proper parameters """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') mock_experiment = project_config.get_experiment_from_key('test_experiment') @@ -1857,7 +1857,7 @@ def test_is_feature_enabled__returns_false_for_feature_rollout_if_feature_disabl mock.patch('time.time', return_value=42): self.assertFalse(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, @@ -1881,7 +1881,7 @@ def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_va Also confirm that impression event is not dispatched. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - project_config = opt_obj.config + project_config = opt_obj.config_manager.get_config() feature = project_config.get_feature_from_key('test_feature_in_experiment') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision( @@ -1898,7 +1898,7 @@ def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_va # Check that impression event is not sent self.assertEqual(0, mock_dispatch_event.call_count) - mock_decision.assert_called_once_with(opt_obj.config, feature, 'test_user', None) + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, @@ -1958,9 +1958,9 @@ def test_get_enabled_features__broadcasts_decision_for_each_feature(self): and broadcasts decision for each feature. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') - mock_variation_2 = opt_obj.config.get_variation_from_id('test_experiment', '111128') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') + mock_variation_2 = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111128') def side_effect(*args, **kwargs): feature = args[1] @@ -2078,13 +2078,13 @@ def test_get_feature_variable_boolean(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'is_working', 'test_user')) @@ -2116,13 +2116,13 @@ def test_get_feature_variable_double(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertEqual(10.02, opt_obj.get_feature_variable_double('test_feature_in_experiment', 'cost', 'test_user')) @@ -2154,13 +2154,13 @@ def test_get_feature_variable_integer(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertEqual(4243, opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user')) @@ -2192,13 +2192,13 @@ def test_get_feature_variable_string(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertEqual( 'staging', @@ -2233,15 +2233,15 @@ def test_get_feature_variable_boolean_for_feature_in_rollout(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('211127') - mock_variation = opt_obj.config.get_variation_from_id('211127', '211129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('211127', '211129') user_attributes = {'test_attribute': 'test_value'} with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.ROLLOUT)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_rollout', 'is_running', 'test_user', attributes=user_attributes)) @@ -2271,15 +2271,15 @@ def test_get_feature_variable_double_for_feature_in_rollout(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('211127') - mock_variation = opt_obj.config.get_variation_from_id('211127', '211129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('211127', '211129') user_attributes = {'test_attribute': 'test_value'} with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.ROLLOUT)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertTrue(opt_obj.get_feature_variable_double('test_feature_in_rollout', 'price', 'test_user', attributes=user_attributes)) @@ -2309,15 +2309,15 @@ def test_get_feature_variable_integer_for_feature_in_rollout(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('211127') - mock_variation = opt_obj.config.get_variation_from_id('211127', '211129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('211127', '211129') user_attributes = {'test_attribute': 'test_value'} with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.ROLLOUT)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertTrue(opt_obj.get_feature_variable_integer('test_feature_in_rollout', 'count', 'test_user', attributes=user_attributes)) @@ -2347,15 +2347,15 @@ def test_get_feature_variable_string_for_feature_in_rollout(self): and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('211127') - mock_variation = opt_obj.config.get_variation_from_id('211127', '211129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('211127', '211129') user_attributes = {'test_attribute': 'test_value'} with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.ROLLOUT)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logging, \ + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logging, \ mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_broadcast_decision: self.assertTrue(opt_obj.get_feature_variable_string('test_feature_in_rollout', 'message', 'test_user', attributes=user_attributes)) @@ -2384,17 +2384,17 @@ def test_get_feature_variable__returns_default_value_if_variable_usage_not_in_va """ Test that get_feature_variable_* returns default value if variable usage not present in variation. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') # Empty variable usage map for the mocked variation - opt_obj.config.variation_variable_usage_map['111129'] = None + opt_obj.config_manager.get_config().variation_variable_usage_map['111129'] = None # Boolean with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'is_working', 'test_user')) mock_config_logger.info.assert_called_once_with( @@ -2406,7 +2406,7 @@ def test_get_feature_variable__returns_default_value_if_variable_usage_not_in_va with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertEqual(10.99, opt_obj.get_feature_variable_double('test_feature_in_experiment', 'cost', 'test_user')) @@ -2419,7 +2419,7 @@ def test_get_feature_variable__returns_default_value_if_variable_usage_not_in_va with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertEqual(999, opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user')) @@ -2432,7 +2432,7 @@ def test_get_feature_variable__returns_default_value_if_variable_usage_not_in_va with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.FEATURE_TEST)), \ - mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertEqual('devel', opt_obj.get_feature_variable_string('test_feature_in_experiment', 'environment', 'test_user')) @@ -2697,7 +2697,7 @@ def test_get_feature_variable__returns_none_if_invalid_feature_key(self): """ Test that get_feature_variable_* returns None for invalid feature key. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - with mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + with mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertIsNone(opt_obj.get_feature_variable_boolean('invalid_feature', 'is_working', 'test_user')) self.assertIsNone(opt_obj.get_feature_variable_double('invalid_feature', 'cost', 'test_user')) self.assertIsNone(opt_obj.get_feature_variable_integer('invalid_feature', 'count', 'test_user')) @@ -2715,7 +2715,7 @@ def test_get_feature_variable__returns_none_if_invalid_variable_key(self): """ Test that get_feature_variable_* returns None for invalid variable key. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - with mock.patch.object(opt_obj.config, 'logger') as mock_config_logger: + with mock.patch.object(opt_obj.config_manager.get_config(), 'logger') as mock_config_logger: self.assertIsNone(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'invalid_variable', 'test_user')) @@ -2740,8 +2740,8 @@ def test_get_feature_variable__returns_default_value_if_feature_not_enabled(self """ Test that get_feature_variable_* returns default value if feature is not enabled for the user. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111128') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111128') # Boolean with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', @@ -2799,8 +2799,8 @@ def test_get_feature_variable__returns_default_value_if_feature_not_enabled_in_r """ Test that get_feature_variable_* returns default value if feature is not enabled for the user. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('211127') - mock_variation = opt_obj.config.get_variation_from_id('211127', '211229') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('211127', '211229') # Boolean with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', @@ -2856,8 +2856,8 @@ def test_get_feature_variable__returns_none_if_type_mismatch(self): """ Test that get_feature_variable_* returns None if type mismatch. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, @@ -2875,8 +2875,8 @@ def test_get_feature_variable__returns_none_if_unable_to_cast(self): """ Test that get_feature_variable_* returns None if unable_to_cast_value """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) - mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment') - mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111129') + mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('test_experiment') + mock_variation = opt_obj.config_manager.get_config().get_variation_from_id('test_experiment', '111129') with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature', return_value=decision_service.Decision(mock_experiment, mock_variation, @@ -2990,7 +2990,7 @@ def setUp(self): json.dumps(self.config_dict), logger=logger.SimpleLogger() ) - self.project_config = self.optimizely.config + self.project_config = self.optimizely.config_manager.get_config() def test_activate(self): """ Test that expected log messages are logged during activate. """ From 133d49cf0af4f87a3fd50569563171c6aac91846 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 4 Jun 2019 13:02:01 -0700 Subject: [PATCH 07/16] Adding tests --- optimizely/optimizely.py | 10 +++++----- tests/test_optimizely.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 2a360578..f759e557 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -297,17 +297,17 @@ def activate(self, experiment_key, user_id, attributes=None): self.logger.error(enums.Errors.INVALID_INPUT.format('user_id')) return None + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('activate')) + return None + variation_key = self.get_variation(experiment_key, user_id, attributes) if not variation_key: self.logger.info('Not activating user "%s".' % user_id) return None - project_config = self.config_manager.get_config() - if not project_config: - self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('activate')) - return None - experiment = project_config.get_experiment_from_key(experiment_key) variation = project_config.get_variation_from_key(experiment_key, variation_key) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index ae2d64dc..7dae6e1e 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -991,6 +991,17 @@ def test_activate__invalid_object(self): mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "activate".') + def test_activate__invalid_project_config(self): + """ Test that activate logs error and returns None if ProjectConfig is None. """ + + with mock.patch.object(self.optimizely.config_manager, 'get_config', return_value=None), \ + mock.patch.object(self.optimizely, 'logger') as mock_client_logging: + self.assertIsNone(self.optimizely.activate('test_experiment', 'test_user')) + + mock_client_logging.error.assert_called_once_with( + 'Invalid config. Optimizely instance is not valid. Failing "activate".' + ) + def test_track__with_attributes(self): """ Test that track calls dispatch_event with right params when attributes are provided. """ From c005ab953edb8e7fbdab8920daf97dc04de5ce77 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 6 Jun 2019 14:56:51 -0700 Subject: [PATCH 08/16] Making more changes. --- optimizely/config_manager.py | 104 ++++++++++++++++++++------------ optimizely/helpers/enums.py | 1 - optimizely/helpers/validator.py | 39 ++++++++---- optimizely/optimizely.py | 36 ++++++----- tests/test_optimizely.py | 32 +++++----- 5 files changed, 121 insertions(+), 91 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 1eacd279..8d951e5c 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -23,7 +23,7 @@ from . import project_config from .error_handler import NoOpErrorHandler as noop_error_handler from .helpers import enums - +from .helpers import validator ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()}) @@ -56,29 +56,67 @@ class StaticConfigManager(BaseConfigManager): def __init__(self, datafile=None, logger=None, - error_handler=None): + error_handler=None, + skip_json_validation=False): """ Initialize config manager. Datafile has to be provided to use. Args: datafile: JSON string representing the Optimizely project. logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. + skip_json_validation: Optional boolean param which allows skipping JSON schema + validation upon object invocation. By default + JSON schema validation will be performed. """ super(StaticConfigManager, self).__init__(logger=logger, error_handler=error_handler) self._config = None - if datafile: - self._config = project_config.ProjectConfig(datafile, self.logger, self.error_handler) + self.validate_schema = not skip_json_validation + self.set_config(datafile) + + def set_config(self, datafile): + """ Looks up and sets datafile and config based on response body. + + Args: + datafile: JSON string representing the Optimizely project. + """ + + if self.validate_schema: + if not validator.is_datafile_valid(datafile): + self.logger.error(enums.Errors.INVALID_INPUT.format('datafile')) + return + + error_msg = None + error_to_handle = None + config = None + + try: + config = project_config.ProjectConfig(datafile, self.logger, self.error_handler) + except optimizely_exceptions.UnsupportedDatafileVersionException as error: + error_msg = error.args[0] + error_to_handle = error + except: + error_msg = enums.Errors.INVALID_INPUT.format('datafile') + error_to_handle = optimizely_exceptions.InvalidInputException(error_msg) + finally: + if error_msg: + self.logger.error(error_msg) + self.error_handler.handle_error(error_to_handle) + return + + # TODO(ali): Add notification listener. + self._config = config + self.logger.debug('Received new datafile and updated config.') def get_config(self): """ Returns instance of ProjectConfig. Returns: - ProjectConfig. + ProjectConfig. None if not set. """ return self._config -class PollingConfigManager(BaseConfigManager): +class PollingConfigManager(StaticConfigManager): """ Config manager that polls for the datafile and updated ProjectConfig based on an update interval. """ def __init__(self, @@ -88,31 +126,37 @@ def __init__(self, url=None, url_template=None, logger=None, - error_handler=None): + error_handler=None, + skip_json_validation=False): """ Initialize config manager. One of sdk_key or url has to be set to be able to use. Args: - sdk_key: Optional string uniquely identifying the datafile. - datafile: Optional JSON string representing the project. - update_interval: Optional floating point number representing time interval in seconds - at which to request datafile and set ProjectConfig. - url: Optional string representing URL from where to fetch the datafile. If set it supersedes the sdk_key. - url_template: Optional string template which in conjunction with sdk_key - determines URL from where to fetch the datafile. - logger: Provides a logger instance. - error_handler: Provides a handle_error method to handle exceptions. + sdk_key: Optional string uniquely identifying the datafile. + datafile: Optional JSON string representing the project. + update_interval: Optional floating point number representing time interval in seconds + at which to request datafile and set ProjectConfig. + url: Optional string representing URL from where to fetch the datafile. If set it supersedes the sdk_key. + url_template: Optional string template which in conjunction with sdk_key + determines URL from where to fetch the datafile. + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + skip_json_validation: Optional boolean param which allows skipping JSON schema + validation upon object invocation. By default + JSON schema validation will be performed. + """ super(PollingConfigManager, self).__init__(logger=logger, error_handler=error_handler) self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) self.set_update_interval(update_interval) self.last_modified = None - self._datafile = datafile self._config = None + self.validate_schema = not skip_json_validation self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) - if self._datafile: - self.set_config(self._datafile) + if datafile: + self.set_config(datafile) + self._polling_thread.start() @staticmethod def get_datafile_url(sdk_key, url, url_template): @@ -166,30 +210,10 @@ def set_last_modified(self, response_headers): """ Looks up and sets last modified time based on Last-Modified header in the response. Args: - response_headers: requests.Response.headers + response_headers: requests.Response.headers """ self.last_modified = response_headers.get(enums.HTTPHeaders.LAST_MODIFIED) - def set_config(self, datafile): - """ Looks up and sets datafile and config based on response body. - - Args: - datafile: JSON string representing the Optimizely project. - """ - # TODO(ali): Add validation here to make sure that we do not update datafile and config if not a valid datafile. - self._datafile = datafile - # TODO(ali): Add notification listener. - self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) - self.logger.debug('Received new datafile and updated config.') - - def get_config(self): - """ Returns instance of ProjectConfig. - - Returns: - ProjectConfig. - """ - return self._config - def _handle_response(self, response): """ Helper method to handle response containing datafile. diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 6dbd2dd1..f4050324 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -74,7 +74,6 @@ class Errors(object): INVALID_ATTRIBUTE = 'Provided attribute is not in datafile.' INVALID_ATTRIBUTE_FORMAT = 'Attributes provided are in an invalid format.' INVALID_AUDIENCE = 'Provided audience is not in datafile.' - INVALID_DATAFILE = 'Datafile has invalid format. Failing "{}".' INVALID_EVENT_TAG_FORMAT = 'Event tags provided are in an invalid format.' INVALID_EXPERIMENT_KEY = 'Provided experiment is not in datafile.' INVALID_EVENT_KEY = 'Provided event is not in datafile.' diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index 9f4bb919..e08e1fd7 100644 --- a/optimizely/helpers/validator.py +++ b/optimizely/helpers/validator.py @@ -58,6 +58,32 @@ def _has_method(obj, method): return getattr(obj, method, None) is not None +def is_config_manager_valid(config_manager): + """ Given a config_manager determine if it is valid or not i.e. provides a get_config method. + + Args: + config_manager: Provides a get_config method to handle exceptions. + + Returns: + Boolean depending upon whether config_manager is valid or not. + """ + + return _has_method(config_manager, 'get_config') + + +def is_error_handler_valid(error_handler): + """ Given a error_handler determine if it is valid or not i.e. provides a handle_error method. + + Args: + error_handler: Provides a handle_error method to handle exceptions. + + Returns: + Boolean depending upon whether error_handler is valid or not. + """ + + return _has_method(error_handler, 'handle_error') + + def is_event_dispatcher_valid(event_dispatcher): """ Given a event_dispatcher determine if it is valid or not i.e. provides a dispatch_event method. @@ -84,19 +110,6 @@ def is_logger_valid(logger): return _has_method(logger, 'log') -def is_error_handler_valid(error_handler): - """ Given a error_handler determine if it is valid or not i.e. provides a handle_error method. - - Args: - error_handler: Provides a handle_error method to handle exceptions. - - Returns: - Boolean depending upon whether error_handler is valid or not. - """ - - return _has_method(error_handler, 'handle_error') - - def are_attributes_valid(attributes): """ Determine if attributes provided are dict or not. diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index f759e557..1cf69b96 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -60,19 +60,8 @@ def __init__(self, self.error_handler = error_handler or noop_error_handler self.config_manager = config_manager - if self.config_manager is None: - if sdk_key: - self.config_manager = PollingConfigManager(sdk_key=sdk_key, - datafile=datafile, - logger=self.logger, - error_handler=self.error_handler) - else: - self.config_manager = StaticConfigManager(datafile=datafile, - logger=self.logger, - error_handler=self.error_handler) - try: - self._validate_instantiation_options(datafile, skip_json_validation) + self._validate_instantiation_options() except exceptions.InvalidInputException as error: self.is_valid = False # We actually want to log this error to stderr, so make sure the logger @@ -81,23 +70,32 @@ def __init__(self, self.logger.exception(str(error)) return + if not self.config_manager: + if sdk_key: + self.config_manager = PollingConfigManager(sdk_key=sdk_key, + datafile=datafile, + logger=self.logger, + error_handler=self.error_handler, + skip_json_validation=skip_json_validation) + else: + self.config_manager = StaticConfigManager(datafile=datafile, + logger=self.logger, + error_handler=self.error_handler, + skip_json_validation=skip_json_validation) + self.event_builder = event_builder.EventBuilder() self.decision_service = decision_service.DecisionService(self.logger, user_profile_service) self.notification_center = notification_center(self.logger) - def _validate_instantiation_options(self, datafile, skip_json_validation): + def _validate_instantiation_options(self): """ Helper method to validate all instantiation parameters. - Args: - datafile: JSON string representing the project. - skip_json_validation: Boolean representing whether JSON schema validation needs to be skipped or not. - Raises: Exception if provided instantiation options are valid. """ - if not skip_json_validation and not validator.is_datafile_valid(datafile): - raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('datafile')) + if self.config_manager and not validator.is_config_manager_valid(self.config_manager): + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('config_manager')) if not validator.is_event_dispatcher_valid(self.event_dispatcher): raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('event_dispatcher')) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 7dae6e1e..2813c79e 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -989,18 +989,8 @@ def test_activate__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: self.assertIsNone(opt_obj.activate('test_experiment', 'test_user')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "activate".') - - def test_activate__invalid_project_config(self): - """ Test that activate logs error and returns None if ProjectConfig is None. """ - - with mock.patch.object(self.optimizely.config_manager, 'get_config', return_value=None), \ - mock.patch.object(self.optimizely, 'logger') as mock_client_logging: - self.assertIsNone(self.optimizely.activate('test_experiment', 'test_user')) - - mock_client_logging.error.assert_called_once_with( - 'Invalid config. Optimizely instance is not valid. Failing "activate".' - ) + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "activate".') def test_track__with_attributes(self): """ Test that track calls dispatch_event with right params when attributes are provided. """ @@ -1437,7 +1427,8 @@ def test_track__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: opt_obj.track('test_event', 'test_user') - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "track".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "track".') def test_track__invalid_experiment_key(self): """ Test that None is returned and expected log messages are logged during track \ @@ -1537,7 +1528,8 @@ def test_get_variation__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: self.assertIsNone(opt_obj.get_variation('test_experiment', 'test_user')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "get_variation".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "get_variation".') def test_get_variation_unknown_experiment_key(self): """ Test that get_variation retuns None when invalid experiment key is given. """ @@ -1936,7 +1928,8 @@ def test_is_feature_enabled__invalid_object(self): mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.assertFalse(opt_obj.is_feature_enabled('test_feature_in_experiment', 'user_1')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "is_feature_enabled".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "is_feature_enabled".') # Check that no event is sent self.assertEqual(0, mock_dispatch_event.call_count) @@ -2082,7 +2075,8 @@ def test_get_enabled_features__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: self.assertEqual([], opt_obj.get_enabled_features('user_1')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "get_enabled_features".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "get_enabled_features".') def test_get_feature_variable_boolean(self): """ Test that get_feature_variable_boolean returns Boolean value as expected \ @@ -3304,7 +3298,8 @@ def test_set_forced_variation__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: self.assertFalse(opt_obj.set_forced_variation('test_experiment', 'test_user', 'test_variation')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "set_forced_variation".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "set_forced_variation".') def test_set_forced_variation__invalid_experiment_key(self): """ Test that None is returned and expected log messages are logged during set_forced_variation \ @@ -3334,7 +3329,8 @@ def test_get_forced_variation__invalid_object(self): with mock.patch.object(opt_obj, 'logger') as mock_client_logging: self.assertIsNone(opt_obj.get_forced_variation('test_experiment', 'test_user')) - mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "get_forced_variation".') + mock_client_logging.error.assert_called_once_with('Invalid config. Optimizely instance is not valid. ' + 'Failing "get_forced_variation".') def test_get_forced_variation__invalid_experiment_key(self): """ Test that None is returned and expected log messages are logged during get_forced_variation \ From 66277c6cf76977443572eebfdd00543b5ca02a8a Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 6 Jun 2019 15:18:07 -0700 Subject: [PATCH 09/16] Updating tests. Still need to fix and add more tests. WIP. --- tests/test_optimizely.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 2813c79e..57ab7bf1 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -80,8 +80,8 @@ def test_init__invalid_datafile__logs_error(self): with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): opt_obj = optimizely.Optimizely('invalid_datafile') - mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.') - self.assertFalse(opt_obj.is_valid) + mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__null_datafile__logs_error(self): """ Test that null datafile logs error on init. """ @@ -90,8 +90,8 @@ def test_init__null_datafile__logs_error(self): with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): opt_obj = optimizely.Optimizely(None) - mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.') - self.assertFalse(opt_obj.is_valid) + mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__empty_datafile__logs_error(self): """ Test that empty datafile logs error on init. """ @@ -100,8 +100,8 @@ def test_init__empty_datafile__logs_error(self): with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): opt_obj = optimizely.Optimizely("") - mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.') - self.assertFalse(opt_obj.is_valid) + mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__invalid_event_dispatcher__logs_error(self): """ Test that invalid event_dispatcher logs error on init. """ @@ -150,7 +150,7 @@ def test_init__unsupported_datafile_version__logs_error(self): mock.patch('optimizely.error_handler.NoOpErrorHandler.handle_error') as mock_error_handler: opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_unsupported_version)) - mock_client_logger.exception.assert_called_once_with( + mock_client_logger.error.assert_called_once_with( 'This version of the Python SDK does not support the given datafile version: "5".' ) @@ -158,8 +158,7 @@ def test_init__unsupported_datafile_version__logs_error(self): self.assertIsInstance(args[0], exceptions.UnsupportedDatafileVersionException) self.assertEqual(args[0].args[0], 'This version of the Python SDK does not support the given datafile version: "5".') - - self.assertFalse(opt_obj.is_valid) + self.assertIsNone(opt_obj.config_manager.get_config()) def test_init_with_supported_datafile_version(self): """ Test that datafile with supported version works as expected. """ @@ -190,12 +189,12 @@ def test_invalid_json_raises_schema_validation_off(self): mock.patch('optimizely.error_handler.NoOpErrorHandler.handle_error') as mock_error_handler: opt_obj = optimizely.Optimizely('invalid_json', skip_json_validation=True) - mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') args, kwargs = mock_error_handler.call_args self.assertIsInstance(args[0], exceptions.InvalidInputException) self.assertEqual(args[0].args[0], 'Provided "datafile" is in an invalid format.') - self.assertFalse(opt_obj.is_valid) + self.assertIsNone(opt_obj.config_manager.get_config()) mock_client_logger.reset_mock() mock_error_handler.reset_mock() @@ -206,12 +205,12 @@ def test_invalid_json_raises_schema_validation_off(self): opt_obj = optimizely.Optimizely({'version': '2', 'events': 'invalid_value', 'experiments': 'invalid_value'}, skip_json_validation=True) - mock_client_logger.exception.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') args, kwargs = mock_error_handler.call_args self.assertIsInstance(args[0], exceptions.InvalidInputException) self.assertEqual(args[0].args[0], 'Provided "datafile" is in an invalid format.') - self.assertFalse(opt_obj.is_valid) + self.assertIsNone(opt_obj.config_manager.get_config()) def test_activate(self): """ Test that activate calls dispatch_event with right params and returns expected variation. """ From 01e9afe92895cb6ee83d808fd267850badc035e4 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 6 Jun 2019 22:57:06 -0700 Subject: [PATCH 10/16] Adding tests. --- tests/helpers_tests/test_validator.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/helpers_tests/test_validator.py b/tests/helpers_tests/test_validator.py index a1daa282..081b72a1 100644 --- a/tests/helpers_tests/test_validator.py +++ b/tests/helpers_tests/test_validator.py @@ -16,6 +16,7 @@ from six import PY2 +from optimizely import config_manager from optimizely import error_handler from optimizely import event_dispatcher from optimizely import logger @@ -26,6 +27,21 @@ class ValidatorTest(base.BaseTest): + def test_is_config_manager_valid__returns_true(self): + """ Test that valid config_manager returns True. """ + + self.assertTrue(validator.is_config_manager_valid(config_manager.StaticConfigManager)) + self.assertTrue(validator.is_config_manager_valid(config_manager.PollingConfigManager)) + + def test_is_config_manager_valid__returns_false(self): + """ Test that invalid config_manager returns False. """ + + class CustomConfigManager(object): + def some_other_method(self): + pass + + self.assertFalse(validator.is_config_manager_valid(CustomConfigManager())) + def test_is_datafile_valid__returns_true(self): """ Test that valid datafile returns True. """ From ee80334773d87ce99d3ff33242fa0ed61caa778e Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Mon, 10 Jun 2019 11:45:31 -0700 Subject: [PATCH 11/16] Adding tests. --- tests/test_optimizely.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 57ab7bf1..ed34e7c6 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -15,6 +15,7 @@ import mock from operator import itemgetter +from optimizely import config_manager from optimizely import decision_service from optimizely import entities from optimizely import error_handler @@ -103,6 +104,19 @@ def test_init__empty_datafile__logs_error(self): mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') self.assertIsNone(opt_obj.config_manager.get_config()) + def test_init__invalid_config_manager__logs_error(self): + """ Test that invalid config_manager logs error on init. """ + + class InvalidConfigManager(object): + pass + + mock_client_logger = mock.MagicMock() + with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + mock_client_logger.exception.assert_called_once_with('Provided "config_manager" is in an invalid format.') + self.assertFalse(opt_obj.is_valid) + def test_init__invalid_event_dispatcher__logs_error(self): """ Test that invalid event_dispatcher logs error on init. """ @@ -172,6 +186,30 @@ def test_init_with_supported_datafile_version(self): mock_client_logger.exception.assert_not_called() self.assertTrue(opt_obj.is_valid) + def test_init__datafile_only(self): + """ Test that if only datafile is provided then StaticConfigManager is used. """ + + opt_obj = optimizely.Optimizely(datafile=json.dumps(self.config_dict)) + self.assertIs(type(opt_obj.config_manager), config_manager.StaticConfigManager) + + def test_init__sdk_key_only(self): + """ Test that if only sdk_key is provided then PollingConfigManager is used. """ + + with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \ + mock.patch('threading.Thread.start'): + opt_obj = optimizely.Optimizely(sdk_key='test_sdk_key') + + self.assertIs(type(opt_obj.config_manager), config_manager.PollingConfigManager) + + def test_init__sdk_key_and_datafile(self): + """ Test that if both sdk_key and datafile is provided then PollingConfigManager is used. """ + + with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \ + mock.patch('threading.Thread.start'): + opt_obj = optimizely.Optimizely(datafile=json.dumps(self.config_dict), sdk_key='test_sdk_key') + + self.assertIs(type(opt_obj.config_manager), config_manager.PollingConfigManager) + def test_skip_json_validation_true(self): """ Test that on setting skip_json_validation to true, JSON schema validation is not performed. """ From 0899e756835b9ca23c73d7a33dfb6987fa4c8e42 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Mon, 10 Jun 2019 15:53:09 -0700 Subject: [PATCH 12/16] Finishing tests. --- optimizely/config_manager.py | 5 +- tests/test_config_manager.py | 142 +++++++++++++++++++++-------------- tests/test_optimizely.py | 8 -- 3 files changed, 89 insertions(+), 66 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 8d951e5c..0bb4cc42 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -145,13 +145,14 @@ def __init__(self, JSON schema validation will be performed. """ - super(PollingConfigManager, self).__init__(logger=logger, error_handler=error_handler) + super(PollingConfigManager, self).__init__(logger=logger, + error_handler=error_handler, + skip_json_validation=skip_json_validation) self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) self.set_update_interval(update_interval) self.last_modified = None self._config = None - self.validate_schema = not skip_json_validation self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) if datafile: diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 0d86b055..746a5e8c 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -14,40 +14,103 @@ import json import mock import requests -import unittest from optimizely import config_manager from optimizely import exceptions as optimizely_exceptions from optimizely import project_config from optimizely.helpers import enums +from . import base + + +class StaticConfigManagerTest(base.BaseTest): + def test_set_config__success(self): + """ Test set_config when datafile is valid. """ + test_datafile = json.dumps(self.config_dict_with_features) + mock_logger = mock.Mock() + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger) + + project_config_manager.set_config(test_datafile) + mock_logger.debug.assert_called_with('Received new datafile and updated config.') + + def test_set_config__schema_validation(self): + """ Test set_config calls or does not call schema validation based on skip_json_validation value. """ + + test_datafile = json.dumps(self.config_dict_with_features) + mock_logger = mock.Mock() + + # Test that schema is validated. + # Note: set_config is called in __init__ itself. + with mock.patch('optimizely.helpers.validator.is_datafile_valid', + return_value=True) as mock_validate_datafile: + config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger) + mock_validate_datafile.assert_called_once_with(test_datafile) + + # Test that schema is not validated if skip_json_validation option is set to True. + with mock.patch('optimizely.helpers.validator.is_datafile_valid', + return_value=True) as mock_validate_datafile: + config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger, + skip_json_validation=True) + mock_validate_datafile.assert_not_called() + + def test_set_config__unsupported_datafile_version(self): + """ Test set_config when datafile has unsupported version. """ + + test_datafile = json.dumps(self.config_dict_with_features) + mock_logger = mock.Mock() + + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger) + + invalid_version_datafile = self.config_dict_with_features.copy() + invalid_version_datafile['version'] = 'invalid_version' + test_datafile = json.dumps(invalid_version_datafile) + + # Call set_config with datafile having invalid version + project_config_manager.set_config(test_datafile) + mock_logger.error.assert_called_once_with('This version of the Python SDK does not support ' + 'the given datafile version: "invalid_version".') + + def test_set_config__invalid_datafile(self): + """ Test set_config when datafile is invalid. """ + + test_datafile = json.dumps(self.config_dict_with_features) + mock_logger = mock.Mock() + + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger) + + # Call set_config with invalid content + project_config_manager.set_config('invalid_datafile') + mock_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') -class StaticConfigManagerTest(unittest.TestCase): def test_get_config(self): - test_datafile = json.dumps({ - 'some_datafile_key': 'some_datafile_value', - 'version': project_config.SUPPORTED_VERSIONS[0] - }) + """ Test get_config. """ + test_datafile = json.dumps(self.config_dict_with_features) project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile) # Assert that config is set. self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) -class PollingConfigManagerTest(unittest.TestCase): - def test_init__no_sdk_key_no_url__fails(self): +@mock.patch('requests.get') +class PollingConfigManagerTest(base.BaseTest): + def test_init__no_sdk_key_no_url__fails(self, _): """ Test that initialization fails if there is no sdk_key or url provided. """ self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, 'Must provide at least one of sdk_key or url.', config_manager.PollingConfigManager, sdk_key=None, url=None) - def test_get_datafile_url__no_sdk_key_no_url_raises(self): + def test_get_datafile_url__no_sdk_key_no_url_raises(self, _): """ Test that get_datafile_url raises exception if no sdk_key or url is provided. """ self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, 'Must provide at least one of sdk_key or url.', config_manager.PollingConfigManager.get_datafile_url, None, None, 'url_template') - def test_get_datafile_url__invalid_url_template_raises(self): + def test_get_datafile_url__invalid_url_template_raises(self, _): """ Test that get_datafile_url raises if url_template is invalid. """ # No url_template provided self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, @@ -61,7 +124,7 @@ def test_get_datafile_url__invalid_url_template_raises(self): config_manager.PollingConfigManager.get_datafile_url, 'optly_datafile_key', None, test_url_template) - def test_get_datafile_url__sdk_key_and_template_provided(self): + def test_get_datafile_url__sdk_key_and_template_provided(self, _): """ Test get_datafile_url when sdk_key and template are provided. """ test_sdk_key = 'optly_key' test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' @@ -69,7 +132,7 @@ def test_get_datafile_url__sdk_key_and_template_provided(self): self.assertEqual(expected_url, config_manager.PollingConfigManager.get_datafile_url(test_sdk_key, None, test_url_template)) - def test_get_datafile_url__url_and_template_provided(self): + def test_get_datafile_url__url_and_template_provided(self, _): """ Test get_datafile_url when url and url_template are provided. """ test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' test_url = 'www.myoptimizelydatafiles.com/my_key.json' @@ -77,7 +140,7 @@ def test_get_datafile_url__url_and_template_provided(self): test_url, test_url_template)) - def test_get_datafile_url__sdk_key_and_url_and_template_provided(self): + def test_get_datafile_url__sdk_key_and_url_and_template_provided(self, _): """ Test get_datafile_url when sdk_key, url and url_template are provided. """ test_sdk_key = 'optly_key' test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' @@ -88,7 +151,8 @@ def test_get_datafile_url__sdk_key_and_url_and_template_provided(self): test_url, test_url_template)) - def test_set_update_interval(self): + def test_set_update_interval(self, _): + """ Test set_update_interval with different inputs. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') # Assert that update_interval cannot be set to less than allowed minimum and instead is set to default value. @@ -103,10 +167,9 @@ def test_set_update_interval(self): project_config_manager.set_update_interval(42) self.assertEqual(42, project_config_manager.update_interval) - def test_set_last_modified(self): + def test_set_last_modified(self, _): """ Test that set_last_modified sets last_modified field based on header. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') - self.assertIsNone(project_config_manager.last_modified) last_modified_time = 'Test Last Modified Time' test_response_headers = { @@ -116,41 +179,21 @@ def test_set_last_modified(self): project_config_manager.set_last_modified(test_response_headers) self.assertEqual(last_modified_time, project_config_manager.last_modified) - def test_set_and_get_config(self): - """ Test that set_last_modified sets config field based on datafile. """ - project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') - - # Assert that config is not set. - self.assertIsNone(project_config_manager.get_config()) - - # Set and check config. - project_config_manager.set_config(json.dumps({ - 'some_datafile_key': 'some_datafile_value', - 'version': project_config.SUPPORTED_VERSIONS[0] - })) - self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) - - def test_fetch_datafile(self): + def test_fetch_datafile(self, _): """ Test that fetch_datafile sets config and last_modified based on response. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') expected_datafile_url = 'https://cdn.optimizely.com/datafiles/some_key.json' test_headers = { 'Last-Modified': 'New Time' } - test_datafile = json.dumps({ - 'some_datafile_key': 'some_datafile_value', - 'version': project_config.SUPPORTED_VERSIONS[0] - }) + test_datafile = json.dumps(self.config_dict_with_features) test_response = requests.Response() test_response.status_code = 200 test_response.headers = test_headers test_response._content = test_datafile - with mock.patch('requests.get', return_value=test_response) as mock_requests: + with mock.patch('requests.get', return_value=test_response): project_config_manager.fetch_datafile() - mock_requests.assert_called_once_with(expected_datafile_url, - headers={}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT) self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) @@ -164,22 +207,9 @@ def test_fetch_datafile(self): self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) - def test_is_running(self): - """ Test is_running before and after starting thread. """ - project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') - self.assertFalse(project_config_manager.is_running) + def test_is_running(self, _): + """ Test that polling thread is running after instance of PollingConfigManager is created. """ with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile: - project_config_manager.start() - self.assertTrue(project_config_manager.is_running) - - mock_fetch_datafile.assert_called_with() - - def test_start(self): - """ Test that calling start starts the polling thread. """ - project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') - self.assertFalse(project_config_manager._polling_thread.is_alive()) - with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile: - project_config_manager.start() - self.assertTrue(project_config_manager._polling_thread.is_alive()) - + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + self.assertTrue(project_config_manager.is_running) mock_fetch_datafile.assert_called_with() diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index ed34e7c6..2488278c 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -210,14 +210,6 @@ def test_init__sdk_key_and_datafile(self): self.assertIs(type(opt_obj.config_manager), config_manager.PollingConfigManager) - def test_skip_json_validation_true(self): - """ Test that on setting skip_json_validation to true, JSON schema validation is not performed. """ - - with mock.patch('optimizely.helpers.validator.is_datafile_valid') as mock_datafile_validation: - optimizely.Optimizely(json.dumps(self.config_dict), skip_json_validation=True) - - self.assertEqual(0, mock_datafile_validation.call_count) - def test_invalid_json_raises_schema_validation_off(self): """ Test that invalid JSON logs error if schema validation is turned off. """ From ef9524755b70914002acb54c2a8cda832ab82881 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Mon, 10 Jun 2019 17:15:00 -0700 Subject: [PATCH 13/16] Improving coverage. --- optimizely/helpers/enums.py | 1 + optimizely/optimizely.py | 14 ++--- tests/test_optimizely.py | 108 +++++++++++++++++++++++++++++++++--- 3 files changed, 109 insertions(+), 14 deletions(-) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index f4050324..64cd05cb 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -80,6 +80,7 @@ class Errors(object): INVALID_FEATURE_KEY = 'Provided feature key is not in the datafile.' INVALID_GROUP_ID = 'Provided group is not in datafile.' INVALID_INPUT = 'Provided "{}" is in an invalid format.' + INVALID_OPTIMIZELY = 'Optimizely instance is not valid. Failing "{}".' INVALID_PROJECT_CONFIG = 'Invalid config. Optimizely instance is not valid. Failing "{}".' INVALID_VARIATION = 'Provided variation is not in datafile.' INVALID_VARIABLE_KEY = 'Provided variable key is not in the feature flag.' diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 1cf69b96..925657d9 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -284,7 +284,7 @@ def activate(self, experiment_key, user_id, attributes=None): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('activate')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('activate')) return None if not validator.is_non_empty_string(experiment_key): @@ -326,7 +326,7 @@ def track(self, event_key, user_id, attributes=None, event_tags=None): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('track')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('track')) return if not validator.is_non_empty_string(event_key): @@ -383,7 +383,7 @@ def get_variation(self, experiment_key, user_id, attributes=None): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_variation')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('get_variation')) return None if not validator.is_non_empty_string(experiment_key): @@ -447,7 +447,7 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('is_feature_enabled')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('is_feature_enabled')) return False if not validator.is_non_empty_string(feature_key): @@ -523,7 +523,7 @@ def get_enabled_features(self, user_id, attributes=None): enabled_features = [] if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_enabled_features')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('get_enabled_features')) return enabled_features if not isinstance(user_id, string_types): @@ -662,7 +662,7 @@ def set_forced_variation(self, experiment_key, user_id, variation_key): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('set_forced_variation')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('set_forced_variation')) return False if not validator.is_non_empty_string(experiment_key): @@ -692,7 +692,7 @@ def get_forced_variation(self, experiment_key, user_id): """ if not self.is_valid: - self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_forced_variation')) + self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('get_forced_variation')) return None if not validator.is_non_empty_string(experiment_key): diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 2488278c..635b4e02 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1011,7 +1011,20 @@ def test_activate__bucketer_returns_none(self): self.assertEqual(0, mock_dispatch_event.call_count) def test_activate__invalid_object(self): - """ Test that activate logs error if Optimizely object is not created correctly. """ + """ Test that activate logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertIsNone(opt_obj.activate('test_experiment', 'test_user')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. Failing "activate".') + + def test_activate__invalid_config(self): + """ Test that activate logs error if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_datafile') @@ -1449,7 +1462,20 @@ def test_track__whitelisted_user_overrides_audience_check(self): self.assertEqual(1, mock_dispatch_event.call_count) def test_track__invalid_object(self): - """ Test that track logs error if Optimizely object is not created correctly. """ + """ Test that track logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertIsNone(opt_obj.track('test_event', 'test_user')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. Failing "track".') + + def test_track__invalid_config(self): + """ Test that track logs error if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_datafile') @@ -1550,7 +1576,20 @@ def test_get_variation__returns_none(self): ) def test_get_variation__invalid_object(self): - """ Test that get_variation logs error if Optimizely object is not created correctly. """ + """ Test that get_variation logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertIsNone(opt_obj.get_variation('test_experiment', 'test_user')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. Failing "get_variation".') + + def test_get_variation__invalid_config(self): + """ Test that get_variation logs error if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_datafile') @@ -1949,7 +1988,20 @@ def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_va self.assertEqual(0, mock_dispatch_event.call_count) def test_is_feature_enabled__invalid_object(self): - """ Test that is_feature_enabled returns False if Optimizely object is not valid. """ + """ Test that is_feature_enabled returns False and logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertFalse(opt_obj.is_feature_enabled('test_feature_in_experiment', 'user_1')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. Failing "is_feature_enabled".') + + def test_is_feature_enabled__invalid_config(self): + """ Test that is_feature_enabled returns False if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_file') @@ -2097,7 +2149,21 @@ def test_get_enabled_features__invalid_attributes(self): mock_client_logging.error.assert_called_once_with('Provided attributes are in an invalid format.') def test_get_enabled_features__invalid_object(self): - """ Test that get_enabled_features returns empty list if Optimizely object is not valid. """ + """ Test that get_enabled_features returns empty list if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertEqual([], opt_obj.get_enabled_features('test_user')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. ' + 'Failing "get_enabled_features".') + + def test_get_enabled_features__invalid_config(self): + """ Test that get_enabled_features returns empty list if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_file') @@ -3320,7 +3386,21 @@ def test_get_variation__invalid_attributes__forced_bucketing(self): self.assertEqual('variation', variation_key) def test_set_forced_variation__invalid_object(self): - """ Test that set_forced_variation logs error if Optimizely object is not created correctly. """ + """ Test that set_forced_variation logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertFalse(opt_obj.set_forced_variation('test_experiment', 'test_user', 'test_variation')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. ' + 'Failing "set_forced_variation".') + + def test_set_forced_variation__invalid_config(self): + """ Test that set_forced_variation logs error if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_datafile') @@ -3351,7 +3431,21 @@ def test_set_forced_variation__invalid_user_id(self): mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.') def test_get_forced_variation__invalid_object(self): - """ Test that get_forced_variation logs error if Optimizely object is not created correctly. """ + """ Test that get_forced_variation logs error if Optimizely instance is invalid. """ + + class InvalidConfigManager(object): + pass + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) + + with mock.patch.object(opt_obj, 'logger') as mock_client_logging: + self.assertIsNone(opt_obj.get_forced_variation('test_experiment', 'test_user')) + + mock_client_logging.error.assert_called_once_with('Optimizely instance is not valid. ' + 'Failing "get_forced_variation".') + + def test_get_forced_variation__invalid_config(self): + """ Test that get_forced_variation logs error if config is invalid. """ opt_obj = optimizely.Optimizely('invalid_datafile') From 1e56ddd473243a4eccfcc7435b6c57921ac904f4 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 11 Jun 2019 09:58:00 -0700 Subject: [PATCH 14/16] Addressing feedback from Mike Ng --- tests/helpers_tests/test_validator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/helpers_tests/test_validator.py b/tests/helpers_tests/test_validator.py index 081b72a1..302a32ce 100644 --- a/tests/helpers_tests/test_validator.py +++ b/tests/helpers_tests/test_validator.py @@ -28,13 +28,13 @@ class ValidatorTest(base.BaseTest): def test_is_config_manager_valid__returns_true(self): - """ Test that valid config_manager returns True. """ + """ Test that valid config_manager returns True for valid config manager implementation. """ self.assertTrue(validator.is_config_manager_valid(config_manager.StaticConfigManager)) self.assertTrue(validator.is_config_manager_valid(config_manager.PollingConfigManager)) def test_is_config_manager_valid__returns_false(self): - """ Test that invalid config_manager returns False. """ + """ Test that invalid config_manager returns False for invalid config manager implementation. """ class CustomConfigManager(object): def some_other_method(self): From 15a432e11a48675506a3de3432d754c26c95d4d5 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 11 Jun 2019 23:30:45 -0700 Subject: [PATCH 15/16] Responding to Matt's feedback. --- optimizely/config_manager.py | 18 +++++++++++++----- tests/test_config_manager.py | 25 +++++++++++++++++++++---- tests/test_decision_service.py | 3 ++- tests/test_optimizely.py | 4 ++-- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 0bb4cc42..ff54cf06 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -71,9 +71,9 @@ def __init__(self, super(StaticConfigManager, self).__init__(logger=logger, error_handler=error_handler) self._config = None self.validate_schema = not skip_json_validation - self.set_config(datafile) + self._set_config(datafile) - def set_config(self, datafile): + def _set_config(self, datafile): """ Looks up and sets datafile and config based on response body. Args: @@ -103,9 +103,17 @@ def set_config(self, datafile): self.error_handler.handle_error(error_to_handle) return + previous_revision = self._config.get_revision() if self._config else None + + if previous_revision == config.get_revision(): + return + # TODO(ali): Add notification listener. self._config = config - self.logger.debug('Received new datafile and updated config.') + self.logger.debug( + 'Received new datafile and updated config. ' + 'Old revision number: {}. New revision number: {}.'.format(previous_revision, config.get_revision()) + ) def get_config(self): """ Returns instance of ProjectConfig. @@ -156,7 +164,7 @@ def __init__(self, self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) if datafile: - self.set_config(datafile) + self._set_config(datafile) self._polling_thread.start() @staticmethod @@ -233,7 +241,7 @@ def _handle_response(self, response): return self.set_last_modified(response.headers) - self.set_config(response.content) + self._set_config(response.content) def fetch_datafile(self): """ Fetch datafile and set ProjectConfig. """ diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 746a5e8c..b9fe7574 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -31,8 +31,25 @@ def test_set_config__success(self): project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, logger=mock_logger) - project_config_manager.set_config(test_datafile) - mock_logger.debug.assert_called_with('Received new datafile and updated config.') + project_config_manager._set_config(test_datafile) + mock_logger.debug.assert_called_with('Received new datafile and updated config. ' + 'Old revision number: None. New revision number: 1.') + + def test_set_config__twice(self): + """ Test calling set_config twice with same content to ensure config is not updated. """ + test_datafile = json.dumps(self.config_dict_with_features) + mock_logger = mock.Mock() + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger) + + project_config_manager._set_config(test_datafile) + mock_logger.debug.assert_called_with('Received new datafile and updated config. ' + 'Old revision number: None. New revision number: 1.') + self.assertEqual(1, mock_logger.debug.call_count) + + # Call set config again and confirm that no new log message denoting config update is there + project_config_manager._set_config(test_datafile) + self.assertEqual(1, mock_logger.debug.call_count) def test_set_config__schema_validation(self): """ Test set_config calls or does not call schema validation based on skip_json_validation value. """ @@ -70,7 +87,7 @@ def test_set_config__unsupported_datafile_version(self): test_datafile = json.dumps(invalid_version_datafile) # Call set_config with datafile having invalid version - project_config_manager.set_config(test_datafile) + project_config_manager._set_config(test_datafile) mock_logger.error.assert_called_once_with('This version of the Python SDK does not support ' 'the given datafile version: "invalid_version".') @@ -84,7 +101,7 @@ def test_set_config__invalid_datafile(self): logger=mock_logger) # Call set_config with invalid content - project_config_manager.set_config('invalid_datafile') + project_config_manager._set_config('invalid_datafile') mock_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') def test_get_config(self): diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 2d0df8c5..84a8fd69 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -855,7 +855,8 @@ def test_get_variation_for_feature__returns_none_for_invalid_group_id(self): self.decision_service.get_variation_for_feature(self.project_config, feature, 'test_user') ) mock_decision_service_logging.error.assert_called_once_with( - enums.Errors.INVALID_GROUP_ID_ERROR.format('_get_variation_for_feature') + enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature') + ) def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature(self): """ Test that if a user is in the mutex group but the experiment is diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 635b4e02..95201ef9 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -195,7 +195,7 @@ def test_init__datafile_only(self): def test_init__sdk_key_only(self): """ Test that if only sdk_key is provided then PollingConfigManager is used. """ - with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \ + with mock.patch('optimizely.config_manager.PollingConfigManager._set_config'), \ mock.patch('threading.Thread.start'): opt_obj = optimizely.Optimizely(sdk_key='test_sdk_key') @@ -204,7 +204,7 @@ def test_init__sdk_key_only(self): def test_init__sdk_key_and_datafile(self): """ Test that if both sdk_key and datafile is provided then PollingConfigManager is used. """ - with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \ + with mock.patch('optimizely.config_manager.PollingConfigManager._set_config'), \ mock.patch('threading.Thread.start'): opt_obj = optimizely.Optimizely(datafile=json.dumps(self.config_dict), sdk_key='test_sdk_key') From ebb0ae3904e21230fec1cce15d9b4d207a27ec86 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Wed, 12 Jun 2019 13:43:55 -0700 Subject: [PATCH 16/16] Fixing test. --- tests/test_config_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index b9fe7574..13e5f170 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -198,7 +198,8 @@ def test_set_last_modified(self, _): def test_fetch_datafile(self, _): """ Test that fetch_datafile sets config and last_modified based on response. """ - project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile'): + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') expected_datafile_url = 'https://cdn.optimizely.com/datafiles/some_key.json' test_headers = { 'Last-Modified': 'New Time'