From c842af4fc16117661b999c0d9d8b7cf94d72530e Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Tue, 4 Jun 2019 16:52:05 -0700 Subject: [PATCH 1/8] feat(datafile-management): Introducing config manager for managing project config (#177) --- optimizely/config_manager.py | 239 +++++++++++++++++++++++++++++++++++ optimizely/helpers/enums.py | 15 +++ optimizely/logger.py | 6 +- optimizely/optimizely.py | 2 +- tests/test_config_manager.py | 185 +++++++++++++++++++++++++++ 5 files changed, 443 insertions(+), 4 deletions(-) create mode 100644 optimizely/config_manager.py create mode 100644 tests/test_config_manager.py diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py new file mode 100644 index 000000000..1eacd279a --- /dev/null +++ b/optimizely/config_manager.py @@ -0,0 +1,239 @@ +# 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 abc +import requests +import threading +import time +from requests import codes as http_status_codes +from requests import exceptions as requests_exceptions + +from . import exceptions as optimizely_exceptions +from . import logger as optimizely_logger +from . import project_config +from .error_handler import NoOpErrorHandler as noop_error_handler +from .helpers import enums + + +ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()}) + + +class BaseConfigManager(ABC): + """ Base class for Optimizely's config manager. """ + + def __init__(self, + logger=None, + error_handler=None): + """ Initialize config manager. + + Args: + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + """ + self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) + self.error_handler = error_handler or noop_error_handler + + @abc.abstractmethod + def get_config(self): + """ Get config for use by optimizely.Optimizely. + The config should be an instance of project_config.ProjectConfig.""" + pass + + +class StaticConfigManager(BaseConfigManager): + """ Config manager that returns ProjectConfig based on provided datafile. """ + + def __init__(self, + datafile=None, + logger=None, + error_handler=None): + """ 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. + """ + 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) + + def get_config(self): + """ Returns instance of ProjectConfig. + + Returns: + ProjectConfig. + """ + return self._config + + +class PollingConfigManager(BaseConfigManager): + """ Config manager that polls for the datafile and updated ProjectConfig based on an update interval. """ + + def __init__(self, + sdk_key=None, + datafile=None, + update_interval=None, + url=None, + url_template=None, + logger=None, + error_handler=None): + """ 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. + """ + 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._polling_thread = threading.Thread(target=self._run) + self._polling_thread.setDaemon(True) + if self._datafile: + self.set_config(self._datafile) + + @staticmethod + def get_datafile_url(sdk_key, url, url_template): + """ Helper method to determine URL from where to fetch the datafile. + + Args: + sdk_key: Key uniquely identifying the datafile. + url: String representing URL from which to fetch the datafile. + url_template: String representing template which is filled in with + SDK key to determine URL from which to fetch the datafile. + + Returns: + String representing URL to fetch datafile from. + + Raises: + optimizely.exceptions.InvalidInputException if: + - One of sdk_key or url is not provided. + - url_template is invalid. + """ + # Ensure that either is provided by the user. + if sdk_key is None and url is None: + raise optimizely_exceptions.InvalidInputException('Must provide at least one of sdk_key or url.') + + # Return URL if one is provided or use template and SDK key to get it. + if url is None: + try: + return url_template.format(sdk_key=sdk_key) + except (AttributeError, KeyError): + raise optimizely_exceptions.InvalidInputException( + 'Invalid url_template {} provided.'.format(url_template)) + + return url + + def set_update_interval(self, update_interval): + """ Helper method to set frequency at which datafile has to be polled and ProjectConfig updated. + + Args: + update_interval: Time in seconds after which to update datafile. + """ + self.update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + # If polling interval is less than minimum allowed interval then set it to default update interval. + if self.update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: + self.logger.debug('Invalid update_interval {} provided. Defaulting to {}'.format( + update_interval, + enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) + ) + self.update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + 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 + """ + 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. + + Args: + response: requests.Response + """ + try: + response.raise_for_status() + except requests_exceptions.HTTPError as err: + self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) + return + + # Leave datafile and config unchanged if it has not been modified. + if response.status_code == http_status_codes.not_modified: + self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) + return + + self.set_last_modified(response.headers) + self.set_config(response.content) + + def fetch_datafile(self): + """ Fetch datafile and set ProjectConfig. """ + + request_headers = {} + if self.last_modified: + request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified + + response = requests.get(self.datafile_url, + headers=request_headers, + timeout=enums.ConfigManager.REQUEST_TIMEOUT) + self._handle_response(response) + + @property + def is_running(self): + """ Check if polling thread is alive or not. """ + return self._polling_thread.is_alive() + + def _run(self): + """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ + while self.is_running: + self.fetch_datafile() + time.sleep(self.update_interval) + + def start(self): + """ Start the config manager and the thread to periodically fetch datafile. """ + if not self.is_running: + self._polling_thread.start() diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 25f6da591..f86f584e4 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -36,6 +36,16 @@ class AudienceEvaluationLogs(object): 'newer release of the Optimizely SDK.' +class ConfigManager(object): + DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' + # Default config update interval of 5 minutes + DEFAULT_UPDATE_INTERVAL = 5 * 60 + # Minimum config update interval of 1 second + MIN_UPDATE_INTERVAL = 1 + # Time in seconds before which request for datafile times out + REQUEST_TIMEOUT = 10 + + class ControlAttributes(object): BOT_FILTERING = '$opt_bot_filtering' BUCKETING_ID = '$opt_bucketing_id' @@ -79,6 +89,11 @@ class Errors(object): UNSUPPORTED_DATAFILE_VERSION = 'This version of the Python SDK does not support the given datafile version: "{}".' +class HTTPHeaders(object): + IF_MODIFIED_SINCE = 'If-Modified-Since' + LAST_MODIFIED = 'Last-Modified' + + class HTTPVerbs(object): GET = 'GET' POST = 'POST' diff --git a/optimizely/logger.py b/optimizely/logger.py index 293172074..9530b132e 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -1,4 +1,4 @@ -# Copyright 2016, 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 @@ -46,8 +46,8 @@ def reset_logger(name, level=None, handler=None): handler.setFormatter(logging.Formatter(_DEFAULT_LOG_FORMAT)) # We don't use ``.addHandler``, since this logger may have already been - # instantiated elsewhere with a different handler. It should only ever - # have one, not many. + # instantiated elsewhere with a different handler. It should only ever + # have one, not many. logger.handlers = [handler] return logger diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 7bd125e3f..559bb722d 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -58,7 +58,7 @@ def __init__(self, except exceptions.InvalidInputException as error: 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. + # has a handler capable of doing that. self.logger = _logging.reset_logger(self.logger_name) self.logger.exception(str(error)) return diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py new file mode 100644 index 000000000..0d86b055e --- /dev/null +++ b/tests/test_config_manager.py @@ -0,0 +1,185 @@ +# 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 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 + + +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] + }) + 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): + """ 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): + """ 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): + """ Test that get_datafile_url raises if url_template is invalid. """ + # No url_template provided + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid url_template None provided', + config_manager.PollingConfigManager.get_datafile_url, 'optly_datafile_key', None, None) + + # Incorrect url_template provided + test_url_template = 'invalid_url_template_without_sdk_key_field_{key}' + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid url_template {} provided'.format(test_url_template), + config_manager.PollingConfigManager.get_datafile_url, + 'optly_datafile_key', None, test_url_template) + + 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' + expected_url = test_url_template.format(sdk_key=test_sdk_key) + 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): + """ 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' + self.assertEqual(test_url, config_manager.PollingConfigManager.get_datafile_url(None, + test_url, + test_url_template)) + + 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' + test_url = 'www.myoptimizelydatafiles.com/my_key.json' + + # Assert that if url is provided, it is always returned + self.assertEqual(test_url, config_manager.PollingConfigManager.get_datafile_url(test_sdk_key, + test_url, + test_url_template)) + + def test_set_update_interval(self): + 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. + project_config_manager.set_update_interval(0.42) + self.assertEqual(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL, project_config_manager.update_interval) + + # Assert that if no update_interval is provided, it is set to default value. + project_config_manager.set_update_interval(None) + self.assertEqual(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL, project_config_manager.update_interval) + + # Assert that if valid update_interval is provided, it is set to that value. + project_config_manager.set_update_interval(42) + self.assertEqual(42, project_config_manager.update_interval) + + 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 = { + 'Last-Modified': last_modified_time, + 'Some-Other-Important-Header': 'some_value' + } + 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): + """ 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_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: + 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) + + # Call fetch_datafile again and assert that request to URL is with If-Modified-Since header. + with mock.patch('requests.get', return_value=test_response) as mock_requests: + project_config_manager.fetch_datafile() + + mock_requests.assert_called_once_with(expected_datafile_url, + headers={'If-Modified-Since': test_headers['Last-Modified']}, + 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) + + 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) + 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()) + + mock_fetch_datafile.assert_called_with() From 4af91c6416866ba8658362b8fbf2136c3cdf79c4 Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Wed, 12 Jun 2019 13:55:12 -0700 Subject: [PATCH 2/8] feat(datafile-management): Integrate config_manager with Optimizely (#179) --- optimizely/config_manager.py | 117 ++++++---- optimizely/decision_service.py | 2 +- optimizely/helpers/enums.py | 21 +- optimizely/helpers/validator.py | 39 ++-- optimizely/optimizely.py | 233 ++++++++++++------- optimizely/project_config.py | 20 +- tests/base.py | 2 +- tests/helpers_tests/test_audience.py | 6 +- tests/helpers_tests/test_validator.py | 16 ++ tests/test_config.py | 54 ++--- tests/test_config_manager.py | 162 +++++++++----- tests/test_decision_service.py | 4 +- tests/test_optimizely.py | 310 ++++++++++++++++++-------- 13 files changed, 648 insertions(+), 338 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 1eacd279a..ff54cf064 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,75 @@ 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 + + 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. ' + 'Old revision number: {}. New revision number: {}.'.format(previous_revision, config.get_revision()) + ) 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 +134,38 @@ 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) + 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._datafile = datafile self._config = None 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 +219,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. @@ -208,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/optimizely/decision_service.py b/optimizely/decision_service.py index ce09c4039..d8b08f9e3 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 f86f584e4..64cd05cb1 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_DATAFILE = 'Datafile has invalid format. Failing "{}".' + INVALID_AUDIENCE = 'Provided audience is not in datafile.' 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_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.' 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/helpers/validator.py b/optimizely/helpers/validator.py index 9f4bb9193..e08e1fd7e 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 559bb722d..925657d92 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,15 +49,19 @@ 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 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 @@ -63,51 +70,41 @@ 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 + 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_ERROR.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_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 +130,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 +142,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 +162,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 +186,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 +219,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 +248,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 @@ -279,15 +284,20 @@ 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): - 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('activate')) return None variation_key = self.get_variation(experiment_key, user_id, attributes) @@ -296,12 +306,12 @@ 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) + 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 @@ -316,27 +326,32 @@ 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): - 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, @@ -368,18 +383,23 @@ 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): - 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 +412,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 @@ -427,27 +447,32 @@ 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): - 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 +484,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) @@ -497,17 +523,22 @@ 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): - 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 +561,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 +587,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 +613,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 +639,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. @@ -603,18 +662,23 @@ 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): - 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. @@ -628,16 +692,21 @@ 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): - 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 0c29fb3c6..52e588376 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 07f025b84..57e31738b 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 e8174ee1b..4a586f4d4 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/helpers_tests/test_validator.py b/tests/helpers_tests/test_validator.py index a1daa2828..302a32ced 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 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 for invalid config manager implementation. """ + + 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. """ diff --git a/tests/test_config.py b/tests/test_config.py index fd971c679..305cf88ae 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_config_manager.py b/tests/test_config_manager.py index 0d86b055e..13e5f1708 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -14,40 +14,120 @@ 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. ' + '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. """ + + 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 +141,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 +149,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 +157,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 +168,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 +184,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 +196,22 @@ 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') + 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' } - 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 +225,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) - 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()) + 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._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_decision_service.py b/tests/test_decision_service.py index 3dab0131f..84a8fd692 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') @@ -855,7 +855,7 @@ 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): diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index cf9dc8ef1..edf590440 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 @@ -79,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. """ @@ -89,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. """ @@ -99,7 +100,20 @@ 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.') + 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): @@ -149,7 +163,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".' ) @@ -157,8 +171,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. """ @@ -172,13 +185,29 @@ def test_init_with_supported_datafile_version(self): mock_client_logger.exception.assert_not_called() self.assertTrue(opt_obj.is_valid) - def test_skip_json_validation_true(self): - """ Test that on setting skip_json_validation to true, JSON schema validation is not performed. """ + 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') - 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.assertIs(type(opt_obj.config_manager), config_manager.PollingConfigManager) - self.assertEqual(0, mock_datafile_validation.call_count) + 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_invalid_json_raises_schema_validation_off(self): """ Test that invalid JSON logs error if schema validation is turned off. """ @@ -189,12 +218,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() @@ -205,12 +234,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. """ @@ -456,7 +485,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] @@ -480,7 +509,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): @@ -488,7 +517,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] @@ -902,14 +931,28 @@ 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') 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".') + 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. """ @@ -1339,14 +1382,28 @@ 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') 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 \ @@ -1395,7 +1452,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', @@ -1439,14 +1496,28 @@ 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') 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. """ @@ -1548,7 +1619,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') @@ -1569,7 +1640,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, @@ -1629,7 +1700,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') @@ -1650,7 +1721,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, @@ -1711,7 +1782,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') @@ -1732,7 +1803,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, @@ -1756,7 +1827,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') @@ -1777,7 +1848,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, @@ -1801,7 +1872,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( @@ -1818,7 +1889,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, @@ -1837,7 +1908,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') @@ -1845,7 +1929,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) @@ -1878,9 +1963,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] @@ -1984,27 +2069,42 @@ 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') 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 \ 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')) @@ -2036,13 +2136,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')) @@ -2074,13 +2174,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')) @@ -2112,13 +2212,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', @@ -2153,15 +2253,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)) @@ -2191,15 +2291,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)) @@ -2229,15 +2329,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)) @@ -2267,15 +2367,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)) @@ -2304,17 +2404,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( @@ -2326,7 +2426,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')) @@ -2339,7 +2439,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')) @@ -2352,7 +2452,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')) @@ -2617,7 +2717,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')) @@ -2635,7 +2735,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')) @@ -2660,8 +2760,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', @@ -2719,8 +2819,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', @@ -2776,8 +2876,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, @@ -2795,8 +2895,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, @@ -2910,7 +3010,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. """ @@ -3206,14 +3306,29 @@ 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') 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 \ @@ -3236,14 +3351,29 @@ 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') 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 fd4ccd058bab3fcda2cefdee75e301f1fb3e900a Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Mon, 17 Jun 2019 15:45:37 -0700 Subject: [PATCH 3/8] Introduce update config notification (#181) --- optimizely/config_manager.py | 13 ++++++++++--- optimizely/helpers/enums.py | 3 +++ optimizely/optimizely.py | 7 +++++-- tests/test_config_manager.py | 26 +++++++++++++++++++++----- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index ff54cf064..7f658ba5d 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -21,7 +21,8 @@ from . import exceptions as optimizely_exceptions from . import logger as optimizely_logger from . import project_config -from .error_handler import NoOpErrorHandler as noop_error_handler +from .error_handler import NoOpErrorHandler +from .notification_center import NotificationCenter from .helpers import enums from .helpers import validator @@ -41,7 +42,7 @@ def __init__(self, error_handler: Provides a handle_error method to handle exceptions. """ self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) - self.error_handler = error_handler or noop_error_handler + self.error_handler = error_handler or NoOpErrorHandler() @abc.abstractmethod def get_config(self): @@ -57,6 +58,7 @@ def __init__(self, datafile=None, logger=None, error_handler=None, + notification_center=None, skip_json_validation=False): """ Initialize config manager. Datafile has to be provided to use. @@ -64,11 +66,13 @@ def __init__(self, datafile: JSON string representing the Optimizely project. logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. + notification_center: Notification center to generate config update notification. 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.notification_center = notification_center or NotificationCenter(self.logger) self._config = None self.validate_schema = not skip_json_validation self._set_config(datafile) @@ -108,8 +112,8 @@ def _set_config(self, datafile): if previous_revision == config.get_revision(): return - # TODO(ali): Add notification listener. self._config = config + self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) self.logger.debug( 'Received new datafile and updated config. ' 'Old revision number: {}. New revision number: {}.'.format(previous_revision, config.get_revision()) @@ -135,6 +139,7 @@ def __init__(self, url_template=None, logger=None, error_handler=None, + notification_center=None, skip_json_validation=False): """ Initialize config manager. One of sdk_key or url has to be set to be able to use. @@ -148,6 +153,7 @@ def __init__(self, determines URL from where to fetch the datafile. logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. + notification_center: Notification center to generate config update notification. skip_json_validation: Optional boolean param which allows skipping JSON schema validation upon object invocation. By default JSON schema validation will be performed. @@ -155,6 +161,7 @@ def __init__(self, """ super(PollingConfigManager, self).__init__(logger=logger, error_handler=error_handler, + notification_center=notification_center, skip_json_validation=skip_json_validation) self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 64cd05cb1..1e683fb3e 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -119,9 +119,12 @@ class NotificationTypes(object): DECISION notification listener has the following parameters: DecisionNotificationTypes type, str user_id, dict attributes, dict decision_info + OPTIMIZELY_CONFIG_UPDATE notification listener has no associated parameters. + TRACK notification listener has the following parameters: str event_key, str user_id, dict attributes (can be None), event_tags (can be None), Event event """ ACTIVATE = 'ACTIVATE:experiment, user_id, attributes, variation, event' DECISION = 'DECISION:type, user_id, attributes, decision_info' + OPTIMIZELY_CONFIG_UPDATE = 'OPTIMIZELY_CONFIG_UPDATE' TRACK = 'TRACK:event_key, user_id, attributes, event_tags, event' diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 925657d92..08b6be77c 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -23,7 +23,7 @@ from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums from .helpers import validator -from .notification_center import NotificationCenter as notification_center +from .notification_center import NotificationCenter class Optimizely(object): @@ -70,22 +70,25 @@ def __init__(self, self.logger.exception(str(error)) return + self.notification_center = NotificationCenter(self.logger) + 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, + notification_center=self.notification_center, skip_json_validation=skip_json_validation) else: self.config_manager = StaticConfigManager(datafile=datafile, logger=self.logger, error_handler=self.error_handler, + notification_center=self.notification_center, 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): """ Helper method to validate all instantiation parameters. diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 13e5f1708..5aafa3618 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -28,28 +28,38 @@ 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() + mock_notification_center = mock.Mock() project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger) + logger=mock_logger, + notification_center=mock_notification_center) 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.') + mock_notification_center.send_notifications.assert_called_once_with('OPTIMIZELY_CONFIG_UPDATE') 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() + mock_notification_center = mock.Mock() project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger) + logger=mock_logger, + notification_center=mock_notification_center) 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) + mock_notification_center.send_notifications.assert_called_once_with('OPTIMIZELY_CONFIG_UPDATE') + + mock_logger.reset_mock() + mock_notification_center.reset_mock() # 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) + self.assertEqual(0, mock_logger.debug.call_count) + self.assertEqual(0, mock_notification_center.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. """ @@ -78,9 +88,11 @@ def test_set_config__unsupported_datafile_version(self): test_datafile = json.dumps(self.config_dict_with_features) mock_logger = mock.Mock() + mock_notification_center = mock.Mock() project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger) + logger=mock_logger, + notification_center=mock_notification_center) invalid_version_datafile = self.config_dict_with_features.copy() invalid_version_datafile['version'] = 'invalid_version' @@ -90,19 +102,23 @@ def test_set_config__unsupported_datafile_version(self): 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".') + self.assertEqual(0, mock_notification_center.call_count) 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() + mock_notification_center = mock.Mock() project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger) + logger=mock_logger, + notification_center=mock_notification_center) # 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.') + self.assertEqual(0, mock_notification_center.call_count) def test_get_config(self): """ Test get_config. """ From 23b3a359a0d3cbf66a1a2a0d63aef5bfec7a508f Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Wed, 19 Jun 2019 15:12:12 -0700 Subject: [PATCH 4/8] Fixing notification center tests. --- tests/test_notification_center.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_notification_center.py b/tests/test_notification_center.py index f07dc4579..eec1abe6c 100644 --- a/tests/test_notification_center.py +++ b/tests/test_notification_center.py @@ -22,6 +22,10 @@ def on_activate_listener(*args): pass +def on_config_update_listener(*args): + pass + + def on_decision_listener(*args): pass @@ -44,10 +48,15 @@ def test_add_notification_listener__valid_type(self): ) self.assertEqual( 2, + test_notification_center.add_notification_listener(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE, + on_config_update_listener) + ) + self.assertEqual( + 3, test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, on_decision_listener) ) self.assertEqual( - 3, test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) + 4, test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) ) def test_add_notification_listener__multiple_listeners(self): @@ -167,6 +176,8 @@ def test_clear_notification_listeners(self): # Add listeners test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + test_notification_center.add_notification_listener(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE, + on_config_update_listener) test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, on_decision_listener) test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) @@ -195,6 +206,8 @@ def test_clear_all_notification_listeners(self): # Add listeners test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + test_notification_center.add_notification_listener(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE, + on_config_update_listener) test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, on_decision_listener) test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) From 40b70f37acf641bbcb499dc8e7f7c2270cb6cb37 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 18 Jun 2019 15:29:24 -0700 Subject: [PATCH 5/8] Fixing logger initialization in config_manager --- optimizely/config_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 7f658ba5d..35bdfa8e6 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -41,7 +41,7 @@ def __init__(self, logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. """ - self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) + self.logger = optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) self.error_handler = error_handler or NoOpErrorHandler() @abc.abstractmethod From dd0ff90850a18bd3d161d12db8dcc20eef91e669 Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Tue, 2 Jul 2019 14:41:25 -0700 Subject: [PATCH 6/8] Handling update_interval values. (#185) --- optimizely/config_manager.py | 28 +++++++++++++++++++++------- optimizely/helpers/validator.py | 4 ++-- tests/test_config_manager.py | 5 +++++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 35bdfa8e6..0c05d534a 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -212,15 +212,24 @@ def set_update_interval(self, update_interval): Args: update_interval: Time in seconds after which to update datafile. """ - self.update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + if not update_interval: + update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + self.logger.debug('Set config update interval to default value {}.'.format(update_interval)) + + if not isinstance(update_interval, (int, float)): + raise optimizely_exceptions.InvalidInputException( + 'Invalid update_interval "{}" provided.'.format(update_interval) + ) # If polling interval is less than minimum allowed interval then set it to default update interval. - if self.update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: - self.logger.debug('Invalid update_interval {} provided. Defaulting to {}'.format( + if update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: + self.logger.debug('update_interval value {} too small. Defaulting to {}'.format( update_interval, enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) ) - self.update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + self.update_interval = update_interval def set_last_modified(self, response_headers): """ Looks up and sets last modified time based on Last-Modified header in the response. @@ -269,9 +278,14 @@ def is_running(self): def _run(self): """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ - while self.is_running: - self.fetch_datafile() - time.sleep(self.update_interval) + try: + while self.is_running: + self.fetch_datafile() + time.sleep(self.update_interval) + except (OSError, OverflowError) as err: + self.logger.error('Error in time.sleep. ' + 'Provided update_interval value may be too big. Error: {}'.format(str(err))) + raise def start(self): """ Start the config manager and the thread to periodically fetch datafile. """ diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index e08e1fd7e..c9ad2a219 100644 --- a/optimizely/helpers/validator.py +++ b/optimizely/helpers/validator.py @@ -221,7 +221,7 @@ def is_finite_number(value): greater than absolute limit of 2^53 else False. """ if not isinstance(value, (numbers.Integral, float)): - # numbers.Integral instead of int to accomodate long integer in python 2 + # numbers.Integral instead of int to accommodate long integer in python 2 return False if isinstance(value, bool): @@ -244,7 +244,7 @@ def are_values_same_type(first_val, second_val): Args: first_val: Value to validate. - second_Val: Value to validate. + second_val: Value to validate. Returns: Boolean: True if both values belong to same type. Otherwise False. diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 5aafa3618..5ea73c263 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -188,6 +188,11 @@ 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 if invalid update_interval is set, then exception is raised. + with self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid update_interval "invalid interval" provided.'): + project_config_manager.set_update_interval('invalid interval') + # Assert that update_interval cannot be set to less than allowed minimum and instead is set to default value. project_config_manager.set_update_interval(0.42) self.assertEqual(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL, project_config_manager.update_interval) From e7c307619cda710349f962e8ef4b8c5066cbb700 Mon Sep 17 00:00:00 2001 From: Ali Abbas Rizvi Date: Wed, 3 Jul 2019 11:11:41 -0700 Subject: [PATCH 7/8] Making notification_center optional for optimizely.Optimizely. (#186) --- optimizely/config_manager.py | 32 +++++++++++---- optimizely/helpers/validator.py | 14 +++++++ optimizely/optimizely.py | 13 ++++-- tests/test_config_manager.py | 70 +++++++++++++++++++++++---------- tests/test_optimizely.py | 13 ++++++ 5 files changed, 111 insertions(+), 31 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 0c05d534a..d4fece65d 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -34,15 +34,34 @@ class BaseConfigManager(ABC): def __init__(self, logger=None, - error_handler=None): + error_handler=None, + notification_center=None): """ Initialize config manager. Args: logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. + notification_center: Provides instance of notification_center.NotificationCenter. """ self.logger = optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) self.error_handler = error_handler or NoOpErrorHandler() + self.notification_center = notification_center or NotificationCenter(self.logger) + self._validate_instantiation_options() + + def _validate_instantiation_options(self): + """ Helper method to validate all parameters. + + Raises: + Exception if provided options are invalid. + """ + if not validator.is_logger_valid(self.logger): + raise optimizely_exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('logger')) + + if not validator.is_error_handler_valid(self.error_handler): + raise optimizely_exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('error_handler')) + + if not validator.is_notification_center_valid(self.notification_center): + raise optimizely_exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('notification_center')) @abc.abstractmethod def get_config(self): @@ -71,8 +90,9 @@ def __init__(self, validation upon object invocation. By default JSON schema validation will be performed. """ - super(StaticConfigManager, self).__init__(logger=logger, error_handler=error_handler) - self.notification_center = notification_center or NotificationCenter(self.logger) + super(StaticConfigManager, self).__init__(logger=logger, + error_handler=error_handler, + notification_center=notification_center) self._config = None self.validate_schema = not skip_json_validation self._set_config(datafile) @@ -159,7 +179,8 @@ def __init__(self, JSON schema validation will be performed. """ - super(PollingConfigManager, self).__init__(logger=logger, + super(PollingConfigManager, self).__init__(datafile=datafile, + logger=logger, error_handler=error_handler, notification_center=notification_center, skip_json_validation=skip_json_validation) @@ -167,11 +188,8 @@ def __init__(self, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) self.set_update_interval(update_interval) self.last_modified = None - self._config = None self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) - if datafile: - self._set_config(datafile) self._polling_thread.start() @staticmethod diff --git a/optimizely/helpers/validator.py b/optimizely/helpers/validator.py index c9ad2a219..4c38735b7 100644 --- a/optimizely/helpers/validator.py +++ b/optimizely/helpers/validator.py @@ -17,6 +17,7 @@ import numbers from six import string_types +from optimizely.notification_center import NotificationCenter from optimizely.user_profile import UserProfile from . import constants @@ -110,6 +111,19 @@ def is_logger_valid(logger): return _has_method(logger, 'log') +def is_notification_center_valid(notification_center): + """ Given notification_center determine if it is valid or not. + + Args: + notification_center: Instance of notification_center.NotificationCenter + + Returns: + Boolean denoting instance is valid or not. + """ + + return isinstance(notification_center, NotificationCenter) + + def are_attributes_valid(attributes): """ Determine if attributes provided are dict or not. diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 08b6be77c..2f0c155cc 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -37,7 +37,8 @@ def __init__(self, skip_json_validation=False, user_profile_service=None, sdk_key=None, - config_manager=None): + config_manager=None, + notification_center=None): """ Optimizely init method for managing Custom projects. Args: @@ -52,6 +53,9 @@ def __init__(self, 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. + notification_center: Optional instance of notification_center.NotificationCenter. Useful when providing own + config_manager.BaseConfigManager implementation which can be using the + same NotificationCenter instance. """ self.logger_name = '.'.join([__name__, self.__class__.__name__]) self.is_valid = True @@ -59,6 +63,7 @@ def __init__(self, self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) self.error_handler = error_handler or noop_error_handler self.config_manager = config_manager + self.notification_center = notification_center or NotificationCenter(self.logger) try: self._validate_instantiation_options() @@ -70,8 +75,6 @@ def __init__(self, self.logger.exception(str(error)) return - self.notification_center = NotificationCenter(self.logger) - if not self.config_manager: if sdk_key: self.config_manager = PollingConfigManager(sdk_key=sdk_key, @@ -96,7 +99,6 @@ def _validate_instantiation_options(self): Raises: Exception if provided instantiation options are valid. """ - if self.config_manager and not validator.is_config_manager_valid(self.config_manager): raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('config_manager')) @@ -109,6 +111,9 @@ def _validate_instantiation_options(self): if not validator.is_error_handler_valid(self.error_handler): raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('error_handler')) + if not validator.is_notification_center_valid(self.notification_center): + raise exceptions.InvalidInputException(enums.Errors.INVALID_INPUT.format('notification_center')) + def _validate_user_inputs(self, attributes=None, event_tags=None): """ Helper method to validate user inputs. diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 5ea73c263..8950705fb 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -24,14 +24,40 @@ class StaticConfigManagerTest(base.BaseTest): + def test_init__invalid_logger_fails(self): + """ Test that initialization fails if logger is invalid. """ + class InvalidLogger(object): + pass + with self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Provided "logger" is in an invalid format.'): + config_manager.StaticConfigManager(logger=InvalidLogger()) + + def test_init__invalid_error_handler_fails(self): + """ Test that initialization fails if error_handler is invalid. """ + class InvalidErrorHandler(object): + pass + with self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Provided "error_handler" is in an invalid format.'): + config_manager.StaticConfigManager(error_handler=InvalidErrorHandler()) + + def test_init__invalid_notification_center_fails(self): + """ Test that initialization fails if notification_center is invalid. """ + class InvalidNotificationCenter(object): + pass + with self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Provided "notification_center" is in an invalid format.'): + config_manager.StaticConfigManager(notification_center=InvalidNotificationCenter()) + 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() mock_notification_center = mock.Mock() - project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger, - notification_center=mock_notification_center) + + with mock.patch('optimizely.config_manager.BaseConfigManager._validate_instantiation_options'): + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger, + notification_center=mock_notification_center) project_config_manager._set_config(test_datafile) mock_logger.debug.assert_called_with('Received new datafile and updated config. ' @@ -43,9 +69,11 @@ def test_set_config__twice(self): test_datafile = json.dumps(self.config_dict_with_features) mock_logger = mock.Mock() mock_notification_center = mock.Mock() - project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger, - notification_center=mock_notification_center) + + with mock.patch('optimizely.config_manager.BaseConfigManager._validate_instantiation_options'): + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger, + notification_center=mock_notification_center) project_config_manager._set_config(test_datafile) mock_logger.debug.assert_called_with('Received new datafile and updated config. ' @@ -71,16 +99,16 @@ def test_set_config__schema_validation(self): # 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) + 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) + 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): @@ -90,9 +118,10 @@ def test_set_config__unsupported_datafile_version(self): mock_logger = mock.Mock() mock_notification_center = mock.Mock() - project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger, - notification_center=mock_notification_center) + with mock.patch('optimizely.config_manager.BaseConfigManager._validate_instantiation_options'): + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger, + notification_center=mock_notification_center) invalid_version_datafile = self.config_dict_with_features.copy() invalid_version_datafile['version'] = 'invalid_version' @@ -111,9 +140,10 @@ def test_set_config__invalid_datafile(self): mock_logger = mock.Mock() mock_notification_center = mock.Mock() - project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, - logger=mock_logger, - notification_center=mock_notification_center) + with mock.patch('optimizely.config_manager.BaseConfigManager._validate_instantiation_options'): + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile, + logger=mock_logger, + notification_center=mock_notification_center) # Call set_config with invalid content project_config_manager._set_config('invalid_datafile') @@ -220,7 +250,7 @@ def test_set_last_modified(self, _): def test_fetch_datafile(self, _): """ Test that fetch_datafile sets config and last_modified based on response. """ with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile'): - project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + 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' @@ -249,6 +279,6 @@ def test_fetch_datafile(self, _): 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 = config_manager.PollingConfigManager(sdk_key='some_key') - self.assertTrue(project_config_manager.is_running) + 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 edf590440..baf606f55 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -155,6 +155,19 @@ class InvalidErrorHandler(object): mock_client_logger.exception.assert_called_once_with('Provided "error_handler" is in an invalid format.') self.assertFalse(opt_obj.is_valid) + def test_init__invalid_notification_center__logs_error(self): + """ Test that invalid notification_center logs error on init. """ + + class InvalidNotificationCenter(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), notification_center=InvalidNotificationCenter()) + + mock_client_logger.exception.assert_called_once_with('Provided "notification_center" is in an invalid format.') + self.assertFalse(opt_obj.is_valid) + def test_init__unsupported_datafile_version__logs_error(self): """ Test that datafile with unsupported version logs error on init. """ From 617478a76a7696c23f89c269268163669d7dbe1f Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 25 Jul 2019 09:49:00 -0700 Subject: [PATCH 8/8] Adjusting tests for get_feature_variable --- optimizely/optimizely.py | 6 +++++- tests/test_optimizely.py | 43 ++++++++++++++++++++-------------------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 94c8aa914..3e6569942 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -567,8 +567,12 @@ def get_feature_variable(self, feature_key, variable_key, user_id, attributes=No - Feature key is invalid. - Variable key is invalid. """ + project_config = self.config_manager.get_config() + if not project_config: + self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('get_feature_variable')) + return None - return self._get_feature_variable_for_type(feature_key, variable_key, None, user_id, attributes) + return self._get_feature_variable_for_type(project_config, feature_key, variable_key, None, user_id, attributes) def get_feature_variable_boolean(self, feature_key, variable_key, user_id, attributes=None): """ Returns value for a certain boolean variable attached to a feature flag. diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 37cf77d79..1a1f7689c 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -2266,14 +2266,14 @@ def test_get_feature_variable(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') # 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_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('test_feature_in_experiment', 'is_working', 'test_user')) @@ -2304,7 +2304,7 @@ def test_get_feature_variable(self): 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('test_feature_in_experiment', 'cost', 'test_user')) @@ -2335,7 +2335,7 @@ def test_get_feature_variable(self): 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('test_feature_in_experiment', 'count', 'test_user')) @@ -2366,7 +2366,7 @@ def test_get_feature_variable(self): 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', @@ -2511,8 +2511,8 @@ def test_get_feature_variable_integer_for_feature_in_rollout(self): ) def test_get_feature_variable_string_for_feature_in_rollout(self): - """ Test that get_feature_variable_double returns Double value as expected \ - and broadcasts decision with proper parameters. """ + """ Test that get_feature_variable_double returns Double value as expected + and broadcasts decision with proper parameters. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) mock_experiment = opt_obj.config_manager.get_config().get_experiment_from_key('211127') @@ -2549,12 +2549,11 @@ def test_get_feature_variable_string_for_feature_in_rollout(self): ) def test_get_feature_variable_for_feature_in_rollout(self): - """ Test that get_feature_variable returns value as expected \ - and broadcasts decision with proper parameters. """ + """ Test that get_feature_variable returns value as expected 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'} # Boolean @@ -2562,7 +2561,7 @@ def test_get_feature_variable_for_feature_in_rollout(self): 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('test_feature_in_rollout', 'is_running', 'test_user', attributes=user_attributes)) @@ -2591,7 +2590,7 @@ def test_get_feature_variable_for_feature_in_rollout(self): 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('test_feature_in_rollout', 'price', 'test_user', attributes=user_attributes)) @@ -2620,7 +2619,7 @@ def test_get_feature_variable_for_feature_in_rollout(self): 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('test_feature_in_rollout', 'count', 'test_user', attributes=user_attributes)) @@ -2649,7 +2648,7 @@ def test_get_feature_variable_for_feature_in_rollout(self): 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('test_feature_in_rollout', 'message', 'test_user', attributes=user_attributes)) @@ -2713,7 +2712,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')) @@ -2726,7 +2725,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')) @@ -2739,7 +2738,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.assertTrue(opt_obj.get_feature_variable('test_feature_in_experiment', 'is_working', 'test_user')) mock_config_logger.info.assert_called_once_with( @@ -2750,7 +2749,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('test_feature_in_experiment', 'cost', 'test_user')) @@ -2762,7 +2761,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('test_feature_in_experiment', 'count', 'test_user')) @@ -2774,7 +2773,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('test_feature_in_experiment', 'environment', 'test_user'))