diff --git a/optimizely/notification_center.py b/optimizely/notification_center.py index 69ae8ce2..02eefd96 100644 --- a/optimizely/notification_center.py +++ b/optimizely/notification_center.py @@ -1,4 +1,4 @@ -# Copyright 2017, Optimizely +# Copyright 2017-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 @@ -11,46 +11,53 @@ # See the License for the specific language governing permissions and # limitations under the License. -from functools import reduce - from .helpers import enums +from . import logger as optimizely_logger + + +NOTIFICATION_TYPES = tuple(getattr(enums.NotificationTypes, attr) + for attr in dir(enums.NotificationTypes) + if not attr.startswith('__')) class NotificationCenter(object): - """ Class encapsulating Broadcast Notifications. The enums.NotifcationTypes includes predefined notifications.""" + """ Class encapsulating methods to manage notifications and their listeners. + The enums.NotificationTypes includes predefined notifications.""" - def __init__(self, logger): - self.notification_id = 1 - self.notifications = {} - for (attr, value) in enums.NotificationTypes.__dict__.items(): - self.notifications[value] = [] - self.logger = logger + def __init__(self, logger=None): + self.listener_id = 1 + self.notification_listeners = {} + for notification_type in NOTIFICATION_TYPES: + self.notification_listeners[notification_type] = [] + self.logger = optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) def add_notification_listener(self, notification_type, notification_callback): - """ Add a notification callback to the notification center. + """ Add a notification callback to the notification center for a given notification type. Args: - notification_type: A string representing the notification type from .helpers.enums.NotificationTypes - notification_callback: closure of function to call when event is triggered. + notification_type: A string representing the notification type from helpers.enums.NotificationTypes + notification_callback: Closure of function to call when event is triggered. Returns: - Integer notification id used to remove the notification or -1 if the notification has already been added. + Integer notification ID used to remove the notification or + -1 if the notification listener has already been added or + if the notification type is invalid. """ - if notification_type not in self.notifications: - self.notifications[notification_type] = [(self.notification_id, notification_callback)] - else: - if reduce(lambda a, b: a + 1, - filter(lambda tup: tup[1] == notification_callback, self.notifications[notification_type]), - 0) > 0: - return -1 - self.notifications[notification_type].append((self.notification_id, notification_callback)) + if notification_type not in NOTIFICATION_TYPES: + self.logger.error('Invalid notification_type: {} provided. Not adding listener.'.format(notification_type)) + return -1 - ret_val = self.notification_id + for _, listener in self.notification_listeners[notification_type]: + if listener == notification_callback: + self.logger.error('Listener has already been added. Not adding it again.') + return -1 - self.notification_id += 1 + self.notification_listeners[notification_type].append((self.listener_id, notification_callback)) + current_listener_id = self.listener_id + self.listener_id += 1 - return ret_val + return current_listener_id def remove_notification_listener(self, notification_id): """ Remove a previously added notification callback. @@ -62,27 +69,43 @@ def remove_notification_listener(self, notification_id): The function returns boolean true if found and removed, false otherwise. """ - for v in self.notifications.values(): - toRemove = list(filter(lambda tup: tup[0] == notification_id, v)) - if len(toRemove) > 0: - v.remove(toRemove[0]) + for listener in self.notification_listeners.values(): + listener_to_remove = list(filter(lambda tup: tup[0] == notification_id, listener)) + if len(listener_to_remove) > 0: + listener.remove(listener_to_remove[0]) return True return False - def clear_all_notifications(self): - """ Remove all notifications """ - for key in self.notifications.keys(): - self.notifications[key] = [] + def clear_notification_listeners(self, notification_type): + """ Remove notification listeners for a certain notification type. + + Args: + notification_type: String denoting notification type. + """ + + if notification_type not in NOTIFICATION_TYPES: + self.logger.error('Invalid notification_type: {} provided. Not removing any listener.'.format(notification_type)) + self.notification_listeners[notification_type] = [] def clear_notifications(self, notification_type): - """ Remove notifications for a certain notification type + """ (DEPRECATED since 3.2.0, use clear_notification_listeners) + Remove notification listeners for a certain notification type. Args: notification_type: key to the list of notifications .helpers.enums.NotificationTypes """ + self.clear_notification_listeners(notification_type) - self.notifications[notification_type] = [] + def clear_all_notification_listeners(self): + """ Remove all notification listeners. """ + for notification_type in self.notification_listeners.keys(): + self.clear_notification_listeners(notification_type) + + def clear_all_notifications(self): + """ (DEPRECATED since 3.2.0, use clear_all_notification_listeners) + Remove all notification listeners. """ + self.clear_all_notification_listeners() def send_notifications(self, notification_type, *args): """ Fires off the notification for the specific event. Uses var args to pass in a @@ -90,12 +113,17 @@ def send_notifications(self, notification_type, *args): Args: notification_type: Type of notification to fire (String from .helpers.enums.NotificationTypes) - args: variable list of arguments to the callback. + args: Variable list of arguments to the callback. """ - if notification_type in self.notifications: - for notification_id, callback in self.notifications[notification_type]: + if notification_type not in NOTIFICATION_TYPES: + self.logger.error('Invalid notification_type: {} provided. ' + 'Not triggering any notification.'.format(notification_type)) + return + + if notification_type in self.notification_listeners: + for notification_id, callback in self.notification_listeners[notification_type]: try: callback(*args) except: - self.logger.exception('Problem calling notify callback!') + self.logger.exception('Unknown problem when sending "{}" type notification.'.format(notification_type)) diff --git a/tests/test_notification_center.py b/tests/test_notification_center.py new file mode 100644 index 00000000..f07dc457 --- /dev/null +++ b/tests/test_notification_center.py @@ -0,0 +1,249 @@ +# 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 mock +import unittest + +from optimizely import notification_center +from optimizely.helpers import enums + + +def on_activate_listener(*args): + pass + + +def on_decision_listener(*args): + pass + + +def on_track_listener(*args): + pass + + +class NotificationCenterTest(unittest.TestCase): + + def test_add_notification_listener__valid_type(self): + """ Test successfully adding a notification listener. """ + + test_notification_center = notification_center.NotificationCenter() + + # Test by adding different supported notification listeners. + self.assertEqual( + 1, + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + ) + self.assertEqual( + 2, + 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) + ) + + def test_add_notification_listener__multiple_listeners(self): + """ Test that multiple listeners of the same type can be successfully added. """ + + def another_on_activate_listener(*args): + pass + + test_notification_center = notification_center.NotificationCenter() + + # Test by adding multiple listeners of same type. + self.assertEqual( + 1, + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + ) + self.assertEqual( + 2, test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, + another_on_activate_listener) + ) + + def test_add_notification_listener__invalid_type(self): + """ Test that adding an invalid notification listener fails and returns -1. """ + + mock_logger = mock.Mock() + test_notification_center = notification_center.NotificationCenter(logger=mock_logger) + + def notif_listener(*args): + pass + + self.assertEqual( + -1, + test_notification_center.add_notification_listener('invalid_notification_type', notif_listener) + ) + mock_logger.error.assert_called_once_with('Invalid notification_type: invalid_notification_type provided. ' + 'Not adding listener.') + + def test_add_notification_listener__same_listener(self): + """ Test that adding same listener again does nothing and returns -1. """ + + mock_logger = mock.Mock() + test_notification_center = notification_center.NotificationCenter(logger=mock_logger) + + self.assertEqual( + 1, + test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) + ) + self.assertEqual(1, len(test_notification_center.notification_listeners[enums.NotificationTypes.TRACK])) + + # Test that adding same listener again makes no difference. + self.assertEqual( + -1, + test_notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track_listener) + ) + self.assertEqual(1, len(test_notification_center.notification_listeners[enums.NotificationTypes.TRACK])) + mock_logger.error.assert_called_once_with('Listener has already been added. Not adding it again.') + + def test_remove_notification_listener__valid_listener(self): + """ Test that removing a valid notification listener returns True. """ + + def another_on_activate_listener(*args): + pass + + test_notification_center = notification_center.NotificationCenter() + + # Add multiple notification listeners. + self.assertEqual( + 1, + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + ) + self.assertEqual( + 2, + test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, on_decision_listener) + ) + self.assertEqual( + 3, test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, + another_on_activate_listener) + ) + + self.assertEqual(2, len(test_notification_center.notification_listeners[enums.NotificationTypes.ACTIVATE])) + self.assertEqual(1, len(test_notification_center.notification_listeners[enums.NotificationTypes.DECISION])) + self.assertEqual(0, len(test_notification_center.notification_listeners[enums.NotificationTypes.TRACK])) + + # Remove one of the activate listeners and assert. + self.assertTrue(test_notification_center.remove_notification_listener(3)) + self.assertEqual(1, len(test_notification_center.notification_listeners[enums.NotificationTypes.ACTIVATE])) + + def test_remove_notification_listener__invalid_listener(self): + """ Test that removing a invalid notification listener returns False. """ + + def another_on_activate_listener(*args): + pass + + test_notification_center = notification_center.NotificationCenter() + + # Add multiple notification listeners. + self.assertEqual( + 1, + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_listener) + ) + self.assertEqual( + 2, + test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, on_decision_listener) + ) + self.assertEqual( + 3, test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, + another_on_activate_listener) + ) + + # Try removing a listener which does not exist. + self.assertFalse(test_notification_center.remove_notification_listener(42)) + + def test_clear_notification_listeners(self): + """ Test that notification listeners of a certain type are cleared + up on using the clear_notification_listeners API. """ + + test_notification_center = notification_center.NotificationCenter() + + # Add listeners + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_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) + + # Assert all listeners are there: + for notification_type in notification_center.NOTIFICATION_TYPES: + self.assertEqual(1, len(test_notification_center.notification_listeners[notification_type])) + + # Clear all of type DECISION. + test_notification_center.clear_notification_listeners(enums.NotificationTypes.DECISION) + self.assertEqual(0, len(test_notification_center.notification_listeners[enums.NotificationTypes.DECISION])) + + def test_clear_notification_listeners__invalid_type(self): + """ Test that clear_notification_listener logs error if provided notification type is invalid. """ + + mock_logger = mock.Mock() + test_notification_center = notification_center.NotificationCenter(logger=mock_logger) + + test_notification_center.clear_notification_listeners('invalid_notification_type') + mock_logger.error.assert_called_once_with('Invalid notification_type: invalid_notification_type provided. ' + 'Not removing any listener.') + + def test_clear_all_notification_listeners(self): + """ Test that all notification listeners are cleared on using the clear all API. """ + + test_notification_center = notification_center.NotificationCenter() + + # Add listeners + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, on_activate_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) + + # Assert all listeners are there: + for notification_type in notification_center.NOTIFICATION_TYPES: + self.assertEqual(1, len(test_notification_center.notification_listeners[notification_type])) + + # Clear all and assert again. + test_notification_center.clear_all_notification_listeners() + + for notification_type in notification_center.NOTIFICATION_TYPES: + self.assertEqual(0, len(test_notification_center.notification_listeners[notification_type])) + + def set_listener_called_to_true(self): + """ Helper method which sets the value of listener_called to True. Used to test sending of notifications.""" + self.listener_called = True + + def test_send_notifications(self): + """ Test that send_notifications dispatches notification to the callback(s). """ + + test_notification_center = notification_center.NotificationCenter() + self.listener_called = False + test_notification_center.add_notification_listener(enums.NotificationTypes.DECISION, + self.set_listener_called_to_true) + test_notification_center.send_notifications(enums.NotificationTypes.DECISION) + self.assertTrue(self.listener_called) + + def test_send_notifications__invalid_notification_type(self): + """ Test that send_notifications logs exception when notification_type is invalid. """ + + mock_logger = mock.Mock() + test_notification_center = notification_center.NotificationCenter(logger=mock_logger) + test_notification_center.send_notifications('invalid_notification_type') + mock_logger.error.assert_called_once_with('Invalid notification_type: invalid_notification_type provided. ' + 'Not triggering any notification.') + + def test_send_notifications__fails(self): + """ Test that send_notifications logs exception when call back fails. """ + + # Defining a listener here which expects 2 arguments. + def some_listener(arg_1, arg_2): + pass + + mock_logger = mock.Mock() + test_notification_center = notification_center.NotificationCenter(logger=mock_logger) + test_notification_center.add_notification_listener(enums.NotificationTypes.ACTIVATE, + some_listener) + + # Not providing any of the 2 expected arguments during send. + test_notification_center.send_notifications(enums.NotificationTypes.ACTIVATE) + mock_logger.exception.assert_called_once_with( + 'Unknown problem when sending "{}" type notification.'.format(enums.NotificationTypes.ACTIVATE)) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index eacef745..cf9dc8ef 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -25,7 +25,6 @@ from optimizely import project_config from optimizely import version from optimizely.helpers import enums -from optimizely.notification_center import NotificationCenter from . import base @@ -281,9 +280,11 @@ def on_activate(experiment, user_id, attributes, variation, event): self.assertEqual(True, callbackhit[0]) self.optimizely.notification_center.remove_notification_listener(notification_id) - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.ACTIVATE])) + self.assertEqual(0, + len(self.optimizely.notification_center.notification_listeners[enums.NotificationTypes.ACTIVATE])) self.optimizely.notification_center.clear_all_notifications() - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.ACTIVATE])) + self.assertEqual(0, + len(self.optimizely.notification_center.notification_listeners[enums.NotificationTypes.ACTIVATE])) def test_add_track_remove_clear_listener(self): """ Test adding a listener tract passes correctly and gets called""" @@ -311,92 +312,11 @@ def on_track(event_key, user_id, attributes, event_tags, event): self.assertEqual(True, callback_hit[0]) - self.assertEqual(1, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) + self.assertEqual(1, len(self.optimizely.notification_center.notification_listeners[enums.NotificationTypes.TRACK])) self.optimizely.notification_center.remove_notification_listener(note_id) - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) + self.assertEqual(0, len(self.optimizely.notification_center.notification_listeners[enums.NotificationTypes.TRACK])) self.optimizely.notification_center.clear_all_notifications() - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - def test_add_same_listener(self): - """ Test adding a same listener """ - - def on_track(event_key, user_id, attributes, event_tags, event): - print('event_key={}', event_key) - - self.optimizely.notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track) - - self.assertEqual(1, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - self.optimizely.notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track) - - self.assertEqual(1, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - def test_add_listener_custom_type(self): - """ Test adding a same listener """ - custom_type = "custom_notification_type" - custom_called = [False] - - def on_custom_event(test_string): - custom_called[0] = True - print('Custom notification event tracked with parameter test_string={}', test_string) - - notification_id = self.optimizely.notification_center.add_notification_listener(custom_type, on_custom_event) - - self.assertEqual(1, len(self.optimizely.notification_center.notifications[custom_type])) - - self.optimizely.notification_center.send_notifications(custom_type, "test") - - self.assertTrue(custom_called[0]) - - self.optimizely.notification_center.remove_notification_listener(notification_id) - - self.assertEqual(0, len(self.optimizely.notification_center.notifications[custom_type])) - - self.optimizely.notification_center.clear_notifications(custom_type) - - self.assertEqual(0, len(self.optimizely.notification_center.notifications[custom_type])) - - def test_invalid_notification_send(self): - """ Test adding a same listener """ - custom_type = "custom_notification_type" - custom_called = [False] - - def on_custom_event(test_string): - custom_called[0] = True - print('Custom notification event tracked with parameter test_string={}', test_string) - mock_logger = mock.Mock() - notification_center = NotificationCenter(mock_logger) - notification_center.add_notification_listener(custom_type, on_custom_event) - notification_center.send_notifications(custom_type, 1, 2, "5", 6) - mock_logger.exception.assert_called_once_with('Problem calling notify callback!') - - def test_add_invalid_listener(self): - """ Test adding a invalid listener """ - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - def test_add_multi_listener(self): - """ Test adding a 2 listeners """ - def on_track(event_key, *args): - print("on track 1 called") - - def on_track2(event_key, *args): - print("on track 2 called") - - self.optimizely.notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track) - - self.assertEqual(1, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - self.optimizely.notification_center.add_notification_listener(enums.NotificationTypes.TRACK, on_track2) - - self.assertEqual(2, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - self.optimizely.notification_center.clear_all_notifications() - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - - def test_remove_listener(self): - """ Test remove listener that isn't added""" - self.optimizely.notification_center.remove_notification_listener(5) - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK])) - self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.ACTIVATE])) + self.assertEqual(0, len(self.optimizely.notification_center.notification_listeners[enums.NotificationTypes.TRACK])) def test_activate_and_decision_listener(self): """ Test that activate calls broadcast activate and decision with proper parameters. """