From 2347a47cf4b541e488cd4d7164617f9124983a7e Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 19 Jan 2023 14:48:05 -0500 Subject: [PATCH 1/8] add notification center registry --- optimizely/config_manager.py | 13 ++++- optimizely/notification_center_registry.py | 66 ++++++++++++++++++++++ optimizely/optimizely.py | 17 +++--- tests/base.py | 5 +- 4 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 optimizely/notification_center_registry.py diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index c5cf8bca..2236e532 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -1,4 +1,4 @@ -# Copyright 2019-2020, 2022, Optimizely +# Copyright 2019-2020, 2022-2023, 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 @@ -25,6 +25,7 @@ from . import project_config from .error_handler import NoOpErrorHandler, BaseErrorHandler from .notification_center import NotificationCenter +from .notification_center_registry import _NotificationCenterRegistry from .helpers import enums from .helpers import validator from .optimizely_config import OptimizelyConfig, OptimizelyConfigService @@ -106,6 +107,7 @@ def __init__( ) self._config: project_config.ProjectConfig = None # type: ignore[assignment] self.optimizely_config: Optional[OptimizelyConfig] = None + self.sdk_key: Optional[str] = None self.validate_schema = not skip_json_validation self._set_config(datafile) @@ -146,8 +148,16 @@ def _set_config(self, datafile: Optional[str | bytes]) -> None: return self._config = config + self.sdk_key = self.sdk_key or config.sdk_key self.optimizely_config = OptimizelyConfigService(config).get_config() self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) + + internal_notification_center = _NotificationCenterRegistry.get_notification_center( + self.sdk_key, self.logger + ) + if internal_notification_center: + internal_notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) + self.logger.debug( 'Received new datafile and updated config. ' f'Old revision number: {previous_revision}. New revision number: {config.get_revision()}.' @@ -209,6 +219,7 @@ def __init__( notification_center=notification_center, skip_json_validation=skip_json_validation, ) + self.sdk_key = sdk_key or self.sdk_key self.datafile_url = self.get_datafile_url( sdk_key, url, url_template or self.DATAFILE_URL_TEMPLATE ) diff --git a/optimizely/notification_center_registry.py b/optimizely/notification_center_registry.py new file mode 100644 index 00000000..d72cab2b --- /dev/null +++ b/optimizely/notification_center_registry.py @@ -0,0 +1,66 @@ +# Copyright 2023, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations +from threading import Lock +from typing import Optional +from .logger import Logger as OptimizelyLogger +from .notification_center import NotificationCenter + + +class _NotificationCenterRegistry: + """ Class managing internal notification centers.""" + _notification_centers: dict[str, NotificationCenter] = {} + _lock = Lock() + + @classmethod + def get_notification_center( + cls, sdk_key: Optional[str], logger: Optional[OptimizelyLogger] = None + ) -> Optional[NotificationCenter]: + """Returns an internal notification center for the given sdk_key, creating one + if none exists yet. + + Args: + sdk_key: An optional string sdk key. If None, nothing is returned. + logger: Optional logger. + + Returns: + None or NotificationCenter + """ + + if not sdk_key: + if logger: + logger.info("No SDK key provided to get_notification_center") + return None + + with cls._lock: + if sdk_key in cls._notification_centers: + notification_center = cls._notification_centers[sdk_key] + else: + notification_center = NotificationCenter(logger) + cls._notification_centers[sdk_key] = notification_center + + return notification_center + + @classmethod + def remove_notification_center(cls, sdk_key: str) -> None: + """Remove a previously added notification center and clear all its listeners. + + Args: + sdk_key: The sdk_key of the notification center to remove. + """ + + with cls._lock: + notification_center = cls._notification_centers.pop(sdk_key, None) + if notification_center: + notification_center.clear_all_notification_listeners() diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 7a46f927..439533e6 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -1,4 +1,4 @@ -# Copyright 2016-2022, Optimizely +# Copyright 2016-2023, 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 @@ -37,6 +37,7 @@ from .helpers.sdk_settings import OptimizelySdkSettings from .helpers.enums import DecisionSources from .notification_center import NotificationCenter +from .notification_center_registry import _NotificationCenterRegistry from .odp.lru_cache import LRUCache from .odp.odp_manager import OdpManager from .optimizely_config import OptimizelyConfig, OptimizelyConfigService @@ -143,7 +144,7 @@ def __init__( self.logger.exception(str(error)) return - self.setup_odp() + self.setup_odp(sdk_key) self.odp_manager = OdpManager( self.sdk_settings.odp_disabled, @@ -1310,7 +1311,7 @@ def _decide_for_keys( decisions[key] = decision return decisions - def setup_odp(self) -> None: + def setup_odp(self, sdk_key: Optional[str]) -> None: """ - Make sure cache is instantiated with provided parameters or defaults. - Set up listener to update odp_config when datafile is updated. @@ -1318,10 +1319,12 @@ def setup_odp(self) -> None: if self.sdk_settings.odp_disabled: return - self.notification_center.add_notification_listener( - enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE, - self._update_odp_config_on_datafile_update - ) + internal_notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, self.logger) + if internal_notification_center: + internal_notification_center.add_notification_listener( + enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE, + self._update_odp_config_on_datafile_update + ) if self.sdk_settings.odp_segment_manager: return diff --git a/tests/base.py b/tests/base.py index 6e74e3aa..2445dcce 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1,4 +1,4 @@ -# Copyright 2016-2021, Optimizely +# Copyright 2016-2023 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 @@ -1262,7 +1262,8 @@ def setUp(self, config_dict='config_dict'): ], 'accountId': '10367498574', 'events': [], - 'revision': '101' + 'revision': '101', + 'sdkKey': 'segments-test' } config = getattr(self, config_dict) From 452d385b7a8bf5a87b187454f0198daa1ff3d3a4 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 26 Jan 2023 11:22:52 -0500 Subject: [PATCH 2/8] add abstractmethod get_sdk_key --- optimizely/config_manager.py | 18 ++++++++++++++---- optimizely/optimizely.py | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 2236e532..d421896c 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -79,6 +79,13 @@ def get_config(self) -> Optional[project_config.ProjectConfig]: The config should be an instance of project_config.ProjectConfig.""" pass + @abstractmethod + def get_sdk_key(self) -> Optional[str]: + """ Get sdk_key for use by optimizely.Optimizely. + The sdk_key should uniquely identify the datafile for a project and environment + combination.""" + pass + class StaticConfigManager(BaseConfigManager): """ Config manager that returns ProjectConfig based on provided datafile. """ @@ -107,10 +114,13 @@ def __init__( ) self._config: project_config.ProjectConfig = None # type: ignore[assignment] self.optimizely_config: Optional[OptimizelyConfig] = None - self.sdk_key: Optional[str] = None + self._sdk_key: Optional[str] = None self.validate_schema = not skip_json_validation self._set_config(datafile) + def get_sdk_key(self) -> Optional[str]: + return self._sdk_key + def _set_config(self, datafile: Optional[str | bytes]) -> None: """ Looks up and sets datafile and config based on response body. @@ -148,12 +158,12 @@ def _set_config(self, datafile: Optional[str | bytes]) -> None: return self._config = config - self.sdk_key = self.sdk_key or config.sdk_key + self._sdk_key = self._sdk_key or config.sdk_key self.optimizely_config = OptimizelyConfigService(config).get_config() self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) internal_notification_center = _NotificationCenterRegistry.get_notification_center( - self.sdk_key, self.logger + self._sdk_key, self.logger ) if internal_notification_center: internal_notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) @@ -219,7 +229,7 @@ def __init__( notification_center=notification_center, skip_json_validation=skip_json_validation, ) - self.sdk_key = sdk_key or self.sdk_key + self._sdk_key = sdk_key or self._sdk_key self.datafile_url = self.get_datafile_url( sdk_key, url, url_template or self.DATAFILE_URL_TEMPLATE ) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 439533e6..fec236b4 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -144,7 +144,7 @@ def __init__( self.logger.exception(str(error)) return - self.setup_odp(sdk_key) + self.setup_odp(sdk_key if sdk_key or not config_manager else config_manager.get_sdk_key()) self.odp_manager = OdpManager( self.sdk_settings.odp_disabled, From 05231d069b51dd9d4c808b00af47c84b944a4804 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 26 Jan 2023 18:51:54 -0500 Subject: [PATCH 3/8] ensure odp is init before listener is called --- optimizely/config_manager.py | 4 +- optimizely/notification_center_registry.py | 9 ++-- optimizely/optimizely.py | 50 ++++++++++++---------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index d21e77c3..8b981358 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -82,8 +82,8 @@ def get_config(self) -> Optional[project_config.ProjectConfig]: @abstractmethod def get_sdk_key(self) -> Optional[str]: """ Get sdk_key for use by optimizely.Optimizely. - The sdk_key should uniquely identify the datafile for a project and environment - combination.""" + The sdk_key should uniquely identify the datafile for a project and environment combination. + """ pass diff --git a/optimizely/notification_center_registry.py b/optimizely/notification_center_registry.py index d72cab2b..4b87b015 100644 --- a/optimizely/notification_center_registry.py +++ b/optimizely/notification_center_registry.py @@ -24,14 +24,12 @@ class _NotificationCenterRegistry: _lock = Lock() @classmethod - def get_notification_center( - cls, sdk_key: Optional[str], logger: Optional[OptimizelyLogger] = None - ) -> Optional[NotificationCenter]: + def get_notification_center(cls, sdk_key: Optional[str], logger: OptimizelyLogger) -> Optional[NotificationCenter]: """Returns an internal notification center for the given sdk_key, creating one if none exists yet. Args: - sdk_key: An optional string sdk key. If None, nothing is returned. + sdk_key: A string sdk key to uniquely identify the notification center. logger: Optional logger. Returns: @@ -39,8 +37,7 @@ def get_notification_center( """ if not sdk_key: - if logger: - logger.info("No SDK key provided to get_notification_center") + logger.error("No SDK key provided to get_notification_center") return None with cls._lock: diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index ba8d0c3a..e9c52966 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -144,18 +144,6 @@ def __init__( self.logger.exception(str(error)) return - self.setup_odp(sdk_key if sdk_key or not config_manager else config_manager.get_sdk_key()) - - self.odp_manager = OdpManager( - self.sdk_settings.odp_disabled, - self.sdk_settings.segments_cache, - self.sdk_settings.odp_segment_manager, - self.sdk_settings.odp_event_manager, - self.sdk_settings.fetch_segments_timeout, - self.sdk_settings.odp_event_timeout, - self.logger - ) - config_manager_options: dict[str, Any] = { 'datafile': datafile, 'logger': self.logger, @@ -175,8 +163,8 @@ def __init__( else: self.config_manager = StaticConfigManager(**config_manager_options) - if not self.sdk_settings.odp_disabled: - self._update_odp_config_on_datafile_update() + self.odp_manager: OdpManager + self.setup_odp(sdk_key or self.config_manager.get_sdk_key()) self.event_builder = event_builder.EventBuilder() self.decision_service = decision_service.DecisionService(self.logger, user_profile_service) @@ -1306,9 +1294,32 @@ def _decide_for_keys( def setup_odp(self, sdk_key: Optional[str]) -> None: """ - - Make sure cache is instantiated with provided parameters or defaults. + - Make sure odp manager is instantiated with provided parameters or defaults. - Set up listener to update odp_config when datafile is updated. + - Manually call callback in case datafile was received before the listener was registered. """ + + # no need to instantiate a cache if a custom cache or segment manager is provided. + if ( + not self.sdk_settings.odp_disabled and + not self.sdk_settings.odp_segment_manager and + not self.sdk_settings.segments_cache + ): + self.sdk_settings.segments_cache = LRUCache( + self.sdk_settings.segments_cache_size, + self.sdk_settings.segments_cache_timeout_in_secs + ) + + self.odp_manager = OdpManager( + self.sdk_settings.odp_disabled, + self.sdk_settings.segments_cache, + self.sdk_settings.odp_segment_manager, + self.sdk_settings.odp_event_manager, + self.sdk_settings.fetch_segments_timeout, + self.sdk_settings.odp_event_timeout, + self.logger + ) + if self.sdk_settings.odp_disabled: return @@ -1319,14 +1330,7 @@ def setup_odp(self, sdk_key: Optional[str]) -> None: self._update_odp_config_on_datafile_update ) - if self.sdk_settings.odp_segment_manager: - return - - if not self.sdk_settings.segments_cache: - self.sdk_settings.segments_cache = LRUCache( - self.sdk_settings.segments_cache_size, - self.sdk_settings.segments_cache_timeout_in_secs - ) + self._update_odp_config_on_datafile_update() def _update_odp_config_on_datafile_update(self) -> None: config = None From 7e34c9d673c0b15eb1b3263c0b17e3ad2ede8fe3 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 26 Jan 2023 18:53:20 -0500 Subject: [PATCH 4/8] add unit tests --- tests/base.py | 9 ++- tests/test_notification_center_registry.py | 84 +++++++++++++++++++++ tests/test_optimizely.py | 88 ++++++++++++++++++++-- tests/test_optimizely_config.py | 2 +- tests/test_optimizely_factory.py | 85 +++++++++++++++++++++ 5 files changed, 258 insertions(+), 10 deletions(-) create mode 100644 tests/test_notification_center_registry.py diff --git a/tests/base.py b/tests/base.py index 2445dcce..be9d7c3f 100644 --- a/tests/base.py +++ b/tests/base.py @@ -150,6 +150,7 @@ def setUp(self, config_dict='config_dict'): # datafile version 4 self.config_dict_with_features = { 'revision': '1', + 'sdkKey': 'features-test', 'accountId': '12001', 'projectId': '111111', 'version': '4', @@ -1261,7 +1262,13 @@ def setUp(self, config_dict='config_dict'): } ], 'accountId': '10367498574', - 'events': [], + 'events': [ + { + "experimentIds": ["10420810910"], + "id": "10404198134", + "key": "event1" + } + ], 'revision': '101', 'sdkKey': 'segments-test' } diff --git a/tests/test_notification_center_registry.py b/tests/test_notification_center_registry.py new file mode 100644 index 00000000..d30cc94f --- /dev/null +++ b/tests/test_notification_center_registry.py @@ -0,0 +1,84 @@ +# 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 +from unittest import mock +import copy + +from optimizely.notification_center_registry import _NotificationCenterRegistry +from optimizely.notification_center import NotificationCenter +from optimizely.optimizely import Optimizely +from optimizely.helpers.enums import NotificationTypes +from .base import BaseTest + + +class NotificationCenterRegistryTest(BaseTest): + def test_get_notification_center(self): + logger = mock.MagicMock() + sdk_key = 'test' + client = Optimizely(sdk_key=sdk_key, logger=logger) + notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, logger) + self.assertIsInstance(notification_center, NotificationCenter) + config_notifications = notification_center.notification_listeners[NotificationTypes.OPTIMIZELY_CONFIG_UPDATE] + + self.assertIn((mock.ANY, client._update_odp_config_on_datafile_update), config_notifications) + + logger.error.assert_not_called() + + _NotificationCenterRegistry.get_notification_center(None, logger) + + logger.error.assert_called_once_with('No SDK key provided to get_notification_center') + + client.close() + + def test_only_one_notification_center_created(self): + logger = mock.MagicMock() + sdk_key = 'single' + notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, logger) + client = Optimizely(sdk_key=sdk_key, logger=logger) + + self.assertIs(notification_center, _NotificationCenterRegistry.get_notification_center(sdk_key, logger)) + + logger.error.assert_not_called() + + client.close() + + def test_remove_notification_center(self): + logger = mock.MagicMock() + sdk_key = 'test' + test_datafile = json.dumps(self.config_dict_with_audience_segments) + test_response = self.fake_server_response(status_code=200, content=test_datafile) + notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, logger) + + with mock.patch('requests.get', return_value=test_response), \ + mock.patch.object(notification_center, 'send_notifications') as mock_send: + + client = Optimizely(sdk_key=sdk_key, logger=logger) + client.config_manager.get_config() + + mock_send.assert_called_once() + mock_send.reset_mock() + + _NotificationCenterRegistry.remove_notification_center(sdk_key) + self.assertNotIn(notification_center, _NotificationCenterRegistry._notification_centers) + + revised_datafile = copy.deepcopy(self.config_dict_with_audience_segments) + revised_datafile['revision'] = str(int(revised_datafile['revision']) + 1) + + # trigger notification + client.config_manager._set_config(json.dumps(revised_datafile)) + mock_send.assert_not_called() + + logger.error.assert_not_called() + + client.close() diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index c6132598..8ed2ca86 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -12,6 +12,7 @@ # limitations under the License. import json +import time from operator import itemgetter from unittest import mock @@ -25,6 +26,7 @@ from optimizely import logger from optimizely import optimizely from optimizely import optimizely_config +from optimizely.odp.odp_config import OdpConfigState from optimizely import project_config from optimizely import version from optimizely.event.event_factory import EventFactory @@ -92,7 +94,10 @@ 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.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_has_calls([ + mock.call('Provided "datafile" is in an invalid format.'), + mock.call('No SDK key provided to get_notification_center') + ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__null_datafile__logs_error(self): @@ -102,7 +107,10 @@ 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.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_has_calls([ + mock.call('Provided "datafile" is in an invalid format.'), + mock.call('No SDK key provided to get_notification_center') + ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__empty_datafile__logs_error(self): @@ -112,7 +120,10 @@ 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.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_has_calls([ + mock.call('Provided "datafile" is in an invalid format.'), + mock.call('No SDK key provided to get_notification_center') + ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) def test_init__invalid_config_manager__logs_error(self): @@ -204,9 +215,10 @@ def test_init__unsupported_datafile_version__logs_error(self): ) as mock_error_handler: opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_unsupported_version)) - mock_client_logger.error.assert_called_once_with( - 'This version of the Python SDK does not support the given datafile version: "5".' - ) + mock_client_logger.error.assert_has_calls([ + mock.call('No SDK key provided to get_notification_center'), + mock.call('This version of the Python SDK does not support the given datafile version: "5".') + ], any_order=True) args, kwargs = mock_error_handler.call_args self.assertIsInstance(args[0], exceptions.UnsupportedDatafileVersionException) @@ -276,7 +288,10 @@ def test_invalid_json_raises_schema_validation_off(self): ) as mock_error_handler: opt_obj = optimizely.Optimizely('invalid_json', skip_json_validation=True) - mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_has_calls([ + mock.call('Provided "datafile" is in an invalid format.'), + mock.call('No SDK key provided to get_notification_center') + ], any_order=True) 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.') @@ -293,7 +308,10 @@ def test_invalid_json_raises_schema_validation_off(self): {'version': '2', 'events': 'invalid_value', 'experiments': 'invalid_value'}, skip_json_validation=True, ) - mock_client_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.') + mock_client_logger.error.assert_has_calls([ + mock.call('Provided "datafile" is in an invalid format.'), + mock.call('No SDK key provided to get_notification_center') + ], any_order=True) 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.') @@ -4616,6 +4634,9 @@ def test_get_optimizely_config_with_custom_config_manager(self): return_config = some_obj.config_manager.get_config() class SomeConfigManager: + def get_sdk_key(self): + return return_config.sdk_key + def get_config(self): return return_config @@ -4631,6 +4652,57 @@ def get_config(self): self.assertEqual(1, mock_opt_service.call_count) + def test_odp_updated_with_custom_polling_config(self): + logger = mock.MagicMock() + + test_datafile = json.dumps(self.config_dict_with_audience_segments) + test_response = self.fake_server_response(status_code=200, content=test_datafile) + + def delay(*args, **kwargs): + time.sleep(.5) + return mock.DEFAULT + + with mock.patch('requests.get', return_value=test_response, side_effect=delay): + # initialize config_manager with delay, so it will receive the datafile after client initialization + custom_config_manager = config_manager.PollingConfigManager(sdk_key='sdk_key', logger=logger) + client = optimizely.Optimizely(config_manager=custom_config_manager) + odp_manager = client.odp_manager + + # confirm odp config has not yet been updated + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.UNDETERMINED) + + # wait for datafile + custom_config_manager.get_config() + + # wait for odp config to be updated + odp_manager.event_manager.event_queue.join() + + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.INTEGRATED) + + logger.error.assert_not_called() + + client.close() + + def test_odp_events_not_sent_with_legacy_apis(self): + logger = mock.MagicMock() + experiment_key = 'experiment-segment' + feature_key = 'flag-segment' + user_id = 'test_user' + + test_datafile = json.dumps(self.config_dict_with_audience_segments) + client = optimizely.Optimizely(test_datafile, logger=logger) + + with mock.patch.object(client.odp_manager.event_manager, 'send_event') as send_event_mock: + client.activate(experiment_key, user_id) + client.track('event1', user_id) + client.get_variation(experiment_key, user_id) + client.get_all_feature_variables(feature_key, user_id) + client.is_feature_enabled(feature_key, user_id) + + send_event_mock.assert_not_called() + + client.close() + class OptimizelyWithExceptionTest(base.BaseTest): def setUp(self): diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 640100d7..e33c1272 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -26,7 +26,7 @@ def setUp(self): self.opt_config_service = optimizely_config.OptimizelyConfigService(self.project_config) self.expected_config = { - 'sdk_key': '', + 'sdk_key': 'features-test', 'environment_key': '', 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experiment_ids': ['111127'], 'id': '111095'}], diff --git a/tests/test_optimizely_factory.py b/tests/test_optimizely_factory.py index 1792f80f..be41755a 100644 --- a/tests/test_optimizely_factory.py +++ b/tests/test_optimizely_factory.py @@ -12,9 +12,11 @@ # limitations under the License. import json +import time from unittest import mock from optimizely.config_manager import PollingConfigManager +from optimizely.odp.odp_config import OdpConfigState from optimizely.error_handler import NoOpErrorHandler from optimizely.event_dispatcher import EventDispatcher from optimizely.notification_center import NotificationCenter @@ -26,6 +28,10 @@ @mock.patch('requests.get') class OptimizelyFactoryTest(base.BaseTest): + def delay(*args, **kwargs): + time.sleep(.5) + return mock.DEFAULT + def setUp(self): super().setUp() self.datafile = '{ revision: "42" }' @@ -181,3 +187,82 @@ def test_update_odp_config_correctly(self, _): self.assertEqual(odp_config.get_api_host(), odp_settings['host']) client.close() + + def test_update_odp_config_correctly_with_custom_config_manager_and_delay(self, _): + logger = mock.MagicMock() + + test_datafile = json.dumps(self.config_dict_with_audience_segments) + test_response = self.fake_server_response(status_code=200, content=test_datafile) + + with mock.patch('requests.get', return_value=test_response, side_effect=self.delay): + # initialize config_manager with delay, so it will receive the datafile after client initialization + config_manager = PollingConfigManager(sdk_key='test', logger=logger) + client = OptimizelyFactory.default_instance_with_config_manager(config_manager=config_manager) + odp_manager = client.odp_manager + + # confirm odp config has not yet been updated + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.UNDETERMINED) + + # wait for datafile + client.config_manager.get_config() + + # wait for odp config to be updated + odp_manager.event_manager.event_queue.join() + + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.INTEGRATED) + + logger.error.assert_not_called() + + client.close() + + def test_update_odp_config_correctly_with_delay(self, _): + logger = mock.MagicMock() + + test_datafile = json.dumps(self.config_dict_with_audience_segments) + test_response = self.fake_server_response(status_code=200, content=test_datafile) + + with mock.patch('requests.get', return_value=test_response, side_effect=self.delay): + # initialize config_manager with delay, so it will receive the datafile after client initialization + client = OptimizelyFactory.default_instance(sdk_key='test') + odp_manager = client.odp_manager + + # confirm odp config has not yet been updated + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.UNDETERMINED) + + # wait for datafile + client.config_manager.get_config() + + # wait for odp config to be updated + odp_manager.event_manager.event_queue.join() + + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.INTEGRATED) + + logger.error.assert_not_called() + + client.close() + + def test_odp_updated_with_custom_instance(self, _): + logger = mock.MagicMock() + + test_datafile = json.dumps(self.config_dict_with_audience_segments) + test_response = self.fake_server_response(status_code=200, content=test_datafile) + + with mock.patch('requests.get', return_value=test_response, side_effect=self.delay): + # initialize config_manager with delay, so it will receive the datafile after client initialization + client = OptimizelyFactory.custom_instance(sdk_key='test') + odp_manager = client.odp_manager + + # confirm odp config has not yet been updated + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.UNDETERMINED) + + # wait for datafile + client.config_manager.get_config() + + # wait for odp config to be updated + odp_manager.event_manager.event_queue.join() + + self.assertEqual(odp_manager.odp_config.odp_state(), OdpConfigState.INTEGRATED) + + logger.error.assert_not_called() + + client.close() From 627756a3a536f659c4c12879bf6b04e96d493a18 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Tue, 31 Jan 2023 16:49:43 -0500 Subject: [PATCH 5/8] make sdk_key or datafile required in cfg_mgr --- optimizely/config_manager.py | 12 ++++++++---- optimizely/helpers/constants.py | 2 ++ optimizely/optimizely.py | 2 +- tests/base.py | 4 ++++ tests/test_config.py | 1 + tests/test_config_manager.py | 8 ++++---- tests/test_notification_center_registry.py | 2 +- tests/test_optimizely.py | 2 +- 8 files changed, 22 insertions(+), 11 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 8b981358..734c0f9b 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -201,11 +201,11 @@ def __init__( notification_center: Optional[NotificationCenter] = None, skip_json_validation: Optional[bool] = False, ): - """ Initialize config manager. One of sdk_key or url has to be set to be able to use. + """ Initialize config manager. One of sdk_key or datafile 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. + sdk_key: Optional string uniquely identifying the datafile. If not provided, datafile is required. + datafile: Optional JSON string representing the project. If not provided, sdk_key is required. update_interval: Optional floating point number representing time interval in seconds at which to request datafile and set ProjectConfig. blocking_timeout: Optional Time in seconds to block the get_config call until config object @@ -230,6 +230,10 @@ def __init__( skip_json_validation=skip_json_validation, ) self._sdk_key = sdk_key or self._sdk_key + + if sdk_key is None: + raise optimizely_exceptions.InvalidInputException('Must provide at least one of sdk_key or datafile.') + self.datafile_url = self.get_datafile_url( sdk_key, url, url_template or self.DATAFILE_URL_TEMPLATE ) @@ -436,7 +440,7 @@ def __init__( *args: Any, **kwargs: Any ): - """ Initialize config manager. One of sdk_key or url has to be set to be able to use. + """ Initialize config manager. One of sdk_key or datafile has to be set to be able to use. Args: datafile_access_token: String to be attached to the request header to fetch the authenticated datafile. diff --git a/optimizely/helpers/constants.py b/optimizely/helpers/constants.py index 06f2cb93..b84bee01 100644 --- a/optimizely/helpers/constants.py +++ b/optimizely/helpers/constants.py @@ -149,6 +149,7 @@ }, "version": {"type": "string"}, "revision": {"type": "string"}, + "sdkKey": {"type": "string"}, "integrations": { "type": "array", "items": { @@ -168,5 +169,6 @@ "attributes", "version", "revision", + "sdkKey", ], } diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index e9c52966..00451175 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -164,7 +164,7 @@ def __init__( self.config_manager = StaticConfigManager(**config_manager_options) self.odp_manager: OdpManager - self.setup_odp(sdk_key or self.config_manager.get_sdk_key()) + self.setup_odp(self.config_manager.get_sdk_key()) self.event_builder = event_builder.EventBuilder() self.decision_service = decision_service.DecisionService(self.logger, user_profile_service) diff --git a/tests/base.py b/tests/base.py index be9d7c3f..875a26e6 100644 --- a/tests/base.py +++ b/tests/base.py @@ -58,6 +58,7 @@ def fake_server_response(self, status_code: Optional[int] = None, def setUp(self, config_dict='config_dict'): self.config_dict = { 'revision': '42', + 'sdkKey': 'basic-test', 'version': '2', 'events': [ {'key': 'test_event', 'experimentIds': ['111127'], 'id': '111095'}, @@ -553,6 +554,7 @@ def setUp(self, config_dict='config_dict'): self.config_dict_with_multiple_experiments = { 'revision': '42', + 'sdkKey': 'multiple-experiments', 'version': '2', 'events': [ {'key': 'test_event', 'experimentIds': ['111127', '111130'], 'id': '111095'}, @@ -658,6 +660,7 @@ def setUp(self, config_dict='config_dict'): self.config_dict_with_unsupported_version = { 'version': '5', + 'sdkKey': 'unsupported-version', 'rollouts': [], 'projectId': '10431130345', 'variables': [], @@ -1074,6 +1077,7 @@ def setUp(self, config_dict='config_dict'): {'key': 'user_signed_up', 'id': '594090', 'experimentIds': ['1323241598', '1323241599']}, ], 'revision': '3', + 'sdkKey': 'typed-audiences', } self.config_dict_with_audience_segments = { diff --git a/tests/test_config.py b/tests/test_config.py index 3b95b02e..9a16035d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -160,6 +160,7 @@ def test_init__with_v4_datafile(self): # Adding some additional fields like live variables and IP anonymization config_dict = { 'revision': '42', + 'sdkKey': 'test', 'version': '4', 'anonymizeIP': False, 'botFiltering': True, diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 38dcfa33..0816b6f7 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -220,14 +220,14 @@ def test_get_config_blocks(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. """ + def test_init__no_sdk_key_no_datafile__fails(self, _): + """ Test that initialization fails if there is no sdk_key or datafile provided. """ self.assertRaisesRegex( optimizely_exceptions.InvalidInputException, - 'Must provide at least one of sdk_key or url.', + 'Must provide at least one of sdk_key or datafile.', config_manager.PollingConfigManager, sdk_key=None, - url=None, + datafile=None, ) def test_get_datafile_url__no_sdk_key_no_url_raises(self, _): diff --git a/tests/test_notification_center_registry.py b/tests/test_notification_center_registry.py index d30cc94f..e74a063c 100644 --- a/tests/test_notification_center_registry.py +++ b/tests/test_notification_center_registry.py @@ -55,7 +55,7 @@ def test_only_one_notification_center_created(self): def test_remove_notification_center(self): logger = mock.MagicMock() - sdk_key = 'test' + sdk_key = 'segments-test' test_datafile = json.dumps(self.config_dict_with_audience_segments) test_response = self.fake_server_response(status_code=200, content=test_datafile) notification_center = _NotificationCenterRegistry.get_notification_center(sdk_key, logger) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 8ed2ca86..2e7dd716 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -4664,7 +4664,7 @@ def delay(*args, **kwargs): with mock.patch('requests.get', return_value=test_response, side_effect=delay): # initialize config_manager with delay, so it will receive the datafile after client initialization - custom_config_manager = config_manager.PollingConfigManager(sdk_key='sdk_key', logger=logger) + custom_config_manager = config_manager.PollingConfigManager(sdk_key='segments-test', logger=logger) client = optimizely.Optimizely(config_manager=custom_config_manager) odp_manager = client.odp_manager From 665099ca8404a2d36d70b12c8b2b32c52db32801 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Tue, 31 Jan 2023 16:57:31 -0500 Subject: [PATCH 6/8] fix copyright date --- tests/test_notification_center_registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_notification_center_registry.py b/tests/test_notification_center_registry.py index e74a063c..a9f768ab 100644 --- a/tests/test_notification_center_registry.py +++ b/tests/test_notification_center_registry.py @@ -1,4 +1,4 @@ -# Copyright 2019, Optimizely +# Copyright 2023, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at From 298469c60fe460024c7dd01daa8e39ef409e8dd2 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 1 Feb 2023 11:59:20 -0500 Subject: [PATCH 7/8] revert datafile schema sdk_key required --- optimizely/helpers/constants.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/optimizely/helpers/constants.py b/optimizely/helpers/constants.py index b84bee01..06f2cb93 100644 --- a/optimizely/helpers/constants.py +++ b/optimizely/helpers/constants.py @@ -149,7 +149,6 @@ }, "version": {"type": "string"}, "revision": {"type": "string"}, - "sdkKey": {"type": "string"}, "integrations": { "type": "array", "items": { @@ -169,6 +168,5 @@ "attributes", "version", "revision", - "sdkKey", ], } From c0dc430f1a8e89f16b67cc3444b22704694de9ba Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 2 Feb 2023 09:44:43 -0500 Subject: [PATCH 8/8] extract error to enum --- optimizely/config_manager.py | 9 +++++---- optimizely/helpers/enums.py | 1 + optimizely/notification_center_registry.py | 3 ++- tests/test_config_manager.py | 2 +- tests/test_notification_center_registry.py | 4 ++-- tests/test_optimizely.py | 12 ++++++------ 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 734c0f9b..247f5ce5 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -204,7 +204,8 @@ def __init__( """ Initialize config manager. One of sdk_key or datafile has to be set to be able to use. Args: - sdk_key: Optional string uniquely identifying the datafile. If not provided, datafile is required. + sdk_key: Optional string uniquely identifying the datafile. If not provided, datafile must + contain a sdk_key. datafile: Optional JSON string representing the project. If not provided, sdk_key is required. update_interval: Optional floating point number representing time interval in seconds at which to request datafile and set ProjectConfig. @@ -231,11 +232,11 @@ def __init__( ) self._sdk_key = sdk_key or self._sdk_key - if sdk_key is None: - raise optimizely_exceptions.InvalidInputException('Must provide at least one of sdk_key or datafile.') + if self._sdk_key is None: + raise optimizely_exceptions.InvalidInputException(enums.Errors.MISSING_SDK_KEY) self.datafile_url = self.get_datafile_url( - sdk_key, url, url_template or self.DATAFILE_URL_TEMPLATE + self._sdk_key, url, url_template or self.DATAFILE_URL_TEMPLATE ) self.set_update_interval(update_interval) self.set_blocking_timeout(blocking_timeout) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 8ba311a1..56fb4946 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -126,6 +126,7 @@ class Errors: ODP_NOT_INTEGRATED: Final = 'ODP is not integrated.' ODP_NOT_ENABLED: Final = 'ODP is not enabled.' ODP_INVALID_DATA: Final = 'ODP data is not valid.' + MISSING_SDK_KEY: Final = 'SDK key not provided/cannot be found in the datafile.' class ForcedDecisionLogs: diff --git a/optimizely/notification_center_registry.py b/optimizely/notification_center_registry.py index 4b87b015..b07702ab 100644 --- a/optimizely/notification_center_registry.py +++ b/optimizely/notification_center_registry.py @@ -16,6 +16,7 @@ from typing import Optional from .logger import Logger as OptimizelyLogger from .notification_center import NotificationCenter +from .helpers.enums import Errors class _NotificationCenterRegistry: @@ -37,7 +38,7 @@ def get_notification_center(cls, sdk_key: Optional[str], logger: OptimizelyLogge """ if not sdk_key: - logger.error("No SDK key provided to get_notification_center") + logger.error(f'{Errors.MISSING_SDK_KEY} ODP may not work properly without it.') return None with cls._lock: diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 0816b6f7..6f4038cb 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -224,7 +224,7 @@ def test_init__no_sdk_key_no_datafile__fails(self, _): """ Test that initialization fails if there is no sdk_key or datafile provided. """ self.assertRaisesRegex( optimizely_exceptions.InvalidInputException, - 'Must provide at least one of sdk_key or datafile.', + enums.Errors.MISSING_SDK_KEY, config_manager.PollingConfigManager, sdk_key=None, datafile=None, diff --git a/tests/test_notification_center_registry.py b/tests/test_notification_center_registry.py index a9f768ab..9159d01a 100644 --- a/tests/test_notification_center_registry.py +++ b/tests/test_notification_center_registry.py @@ -18,7 +18,7 @@ from optimizely.notification_center_registry import _NotificationCenterRegistry from optimizely.notification_center import NotificationCenter from optimizely.optimizely import Optimizely -from optimizely.helpers.enums import NotificationTypes +from optimizely.helpers.enums import NotificationTypes, Errors from .base import BaseTest @@ -37,7 +37,7 @@ def test_get_notification_center(self): _NotificationCenterRegistry.get_notification_center(None, logger) - logger.error.assert_called_once_with('No SDK key provided to get_notification_center') + logger.error.assert_called_once_with(f'{Errors.MISSING_SDK_KEY} ODP may not work properly without it.') client.close() diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 2e7dd716..c0a69cf1 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -96,7 +96,7 @@ def test_init__invalid_datafile__logs_error(self): mock_client_logger.error.assert_has_calls([ mock.call('Provided "datafile" is in an invalid format.'), - mock.call('No SDK key provided to get_notification_center') + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.') ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) @@ -109,7 +109,7 @@ def test_init__null_datafile__logs_error(self): mock_client_logger.error.assert_has_calls([ mock.call('Provided "datafile" is in an invalid format.'), - mock.call('No SDK key provided to get_notification_center') + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.') ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) @@ -122,7 +122,7 @@ def test_init__empty_datafile__logs_error(self): mock_client_logger.error.assert_has_calls([ mock.call('Provided "datafile" is in an invalid format.'), - mock.call('No SDK key provided to get_notification_center') + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.') ], any_order=True) self.assertIsNone(opt_obj.config_manager.get_config()) @@ -216,7 +216,7 @@ def test_init__unsupported_datafile_version__logs_error(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_unsupported_version)) mock_client_logger.error.assert_has_calls([ - mock.call('No SDK key provided to get_notification_center'), + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.'), mock.call('This version of the Python SDK does not support the given datafile version: "5".') ], any_order=True) @@ -290,7 +290,7 @@ def test_invalid_json_raises_schema_validation_off(self): mock_client_logger.error.assert_has_calls([ mock.call('Provided "datafile" is in an invalid format.'), - mock.call('No SDK key provided to get_notification_center') + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.') ], any_order=True) args, kwargs = mock_error_handler.call_args self.assertIsInstance(args[0], exceptions.InvalidInputException) @@ -310,7 +310,7 @@ def test_invalid_json_raises_schema_validation_off(self): mock_client_logger.error.assert_has_calls([ mock.call('Provided "datafile" is in an invalid format.'), - mock.call('No SDK key provided to get_notification_center') + mock.call(f'{enums.Errors.MISSING_SDK_KEY} ODP may not work properly without it.') ], any_order=True) args, kwargs = mock_error_handler.call_args self.assertIsInstance(args[0], exceptions.InvalidInputException)