fix: add notification center registry (#413)

* add notification center registry

* add abstractmethod get_sdk_key to BaseConfigManager

* make sdk_key or datafile required in PollingConfigManager

Author: andrewleap-optimizely

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
@@ -78,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. """
@@ -106,9 +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.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.
 
@@ -146,8 +158,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()}.'
@@ -181,11 +201,12 @@ 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 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.
             blocking_timeout: Optional Time in seconds to block the get_config call until config object
@@ -209,8 +230,13 @@ def __init__(
             notification_center=notification_center,
             skip_json_validation=skip_json_validation,
         )
+        self._sdk_key = sdk_key or self._sdk_key
+
+        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)
@@ -415,7 +441,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.

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:

optimizely/notification_center_registry.py

@@ -0,0 +1,64 @@
+# 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
+from .helpers.enums import Errors
+
+
+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: OptimizelyLogger) -> Optional[NotificationCenter]:
+        """Returns an internal notification center for the given sdk_key, creating one
+        if none exists yet.
+
+        Args:
+        sdk_key: A string sdk key to uniquely identify the notification center.
+        logger: Optional logger.
+
+        Returns:
+        None or NotificationCenter
+        """
+
+        if not sdk_key:
+            logger.error(f'{Errors.MISSING_SDK_KEY} ODP may not work properly without it.')
+            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()

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,18 +144,6 @@ def __init__(
             self.logger.exception(str(error))
             return
 
-        self.setup_odp()
-
-        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,
@@ -174,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(self.config_manager.get_sdk_key())
 
         self.event_builder = event_builder.EventBuilder()
         self.decision_service = decision_service.DecisionService(self.logger, user_profile_service)
@@ -1303,28 +1292,46 @@ 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.
+        - 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.
         """
-        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
+        # 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_segment_manager:
+        if self.sdk_settings.odp_disabled:
             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
+        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
             )
 
+        self._update_odp_config_on_datafile_update()
+
     def _update_odp_config_on_datafile_update(self) -> None:
         config = None
 

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
@@ -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'},
@@ -150,6 +151,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',
@@ -552,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'},
@@ -657,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': [],
@@ -1073,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 = {
@@ -1261,8 +1266,15 @@ def setUp(self, config_dict='config_dict'):
                 }
             ],
             'accountId': '10367498574',
-            'events': [],
-            'revision': '101'
+            'events': [
+                {
+                    "experimentIds": ["10420810910"],
+                    "id": "10404198134",
+                    "key": "event1"
+                }
+            ],
+            'revision': '101',
+            'sdkKey': 'segments-test'
         }
 
         config = getattr(self, config_dict)

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,

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.',
+            enums.Errors.MISSING_SDK_KEY,
             config_manager.PollingConfigManager,
             sdk_key=None,
-            url=None,
+            datafile=None,
         )
 
     def test_get_datafile_url__no_sdk_key_no_url_raises(self, _):