From 58f65a222b825c2da31bab43ddbfdcc7b7a850af Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 2 Jul 2019 18:51:36 -0700 Subject: [PATCH] Making notification_center optional for optimizely.Optimizely. --- 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. """