Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add notification center registry #413

Merged
merged 9 commits into from
Feb 3, 2023
23 changes: 22 additions & 1 deletion optimizely/config_manager.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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. """
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
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()}.'
Expand Down Expand Up @@ -209,6 +229,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
)
Expand Down
63 changes: 63 additions & 0 deletions optimizely/notification_center_registry.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# 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: 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("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()
61 changes: 34 additions & 27 deletions optimizely/optimizely.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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(sdk_key or self.config_manager.get_sdk_key())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see 2 changes related to "sdk_key" in here and in config_manager, both optional.
If none of them required, then we may have a case sdk_key is null.
As noted in my comment in config_manager, we may not need get_sdk_key in config_manager.
We can consider "sdk_key" in optimizely from optional to required while removing the changes in config_manager.


self.event_builder = event_builder.EventBuilder()
self.decision_service = decision_service.DecisionService(self.logger, user_profile_service)
Expand Down Expand Up @@ -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

Expand Down
14 changes: 11 additions & 3 deletions tests/base.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -1261,8 +1262,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)
Expand Down
84 changes: 84 additions & 0 deletions tests/test_notification_center_registry.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# Copyright 2019, Optimizely
andrewleap-optimizely marked this conversation as resolved.
Show resolved Hide resolved
# 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()
Loading