From de92ec677ef9540433235d09d0473ce46441ffc0 Mon Sep 17 00:00:00 2001 From: rashidsp Date: Fri, 29 Jun 2018 12:00:56 +0500 Subject: [PATCH 1/2] Renames notification-center methods --- .../Notification/NotificationCenter.php | 32 ++++++- .../NotificationCenterTest.php | 83 +++++++++++++++---- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/src/Optimizely/Notification/NotificationCenter.php b/src/Optimizely/Notification/NotificationCenter.php index 38840b4b..d327bf8f 100644 --- a/src/Optimizely/Notification/NotificationCenter.php +++ b/src/Optimizely/Notification/NotificationCenter.php @@ -1,6 +1,6 @@ _logger->log( + Logger::WARNING, + "'clearNotifications' is deprecated. Call 'clearNotificationListeners' instead." + ); + $this->clearNotificationListeners($notification_type); + } + /** * Removes all notification callbacks for the given notification type * * @param string $notification_type One of the constants defined in NotificationType */ - public function clearNotifications($notification_type) + public function clearNotificationListeners($notification_type) { if (!NotificationType::isNotificationTypeValid($notification_type)) { $this->_logger->log(Logger::ERROR, "Invalid notification type."); @@ -134,11 +147,24 @@ public function clearNotifications($notification_type) $this->_logger->log(Logger::INFO, "All callbacks for notification type '{$notification_type}' have been removed."); } + /** + * Logs deprecation message + * + * @deprecated Use 'clearAllNotificationListeners' instead. + */ + public function cleanAllNotifications(){ + $this->_logger->log( + Logger::WARNING, + "'cleanAllNotifications' is deprecated. Call 'clearAllNotificationListeners' instead." + ); + $this->clearAllNotificationListeners(); + } + /** * Removes all notifications for all notification types * from the notification center */ - public function cleanAllNotifications() + public function clearAllNotificationListeners() { foreach (array_values(NotificationType::getAll()) as $type) { $this->_notifications[$type] = []; diff --git a/tests/NotificationTests/NotificationCenterTest.php b/tests/NotificationTests/NotificationCenterTest.php index ead54064..a79259b1 100644 --- a/tests/NotificationTests/NotificationCenterTest.php +++ b/tests/NotificationTests/NotificationCenterTest.php @@ -73,7 +73,7 @@ public function testAddNotificationWithInvalidParams() public function testAddNotificationWithValidTypeAndCallback() { $notificationType = NotificationType::ACTIVATE; - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); //////////////////////////////////////////////////////////////////////////////////////////////////////////// // === should add, log and return notification ID when a plain function is passed as an argument === // @@ -135,7 +135,7 @@ function () { public function testAddNotificationForMultipleNotificationTypes() { - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // === should add, log and return notification ID when a valid callback is added for each notification type === // @@ -179,7 +179,7 @@ function () { public function testAddNotificationForMultipleCallbacksForASingleNotificationType() { - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); /////////////////////////////////////////////////////////////////////////////////////// // === should add, log and return notification ID when multiple valid callbacks @@ -243,7 +243,7 @@ public function testAddNotificationThatAlreadyAddedCallbackIsNotReAdded() $functionToSend = function () { }; - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); /////////////////////////////////////////////////////////////////////////// // ===== verify that a variable method with same body isn't re-added ===== // @@ -313,7 +313,7 @@ public function testAddNotificationThatAlreadyAddedCallbackIsNotReAdded() public function testRemoveNotification() { - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); // add a callback for multiple notification types $this->assertSame( @@ -407,10 +407,35 @@ function () { ); } - public function testClearNotifications() + public function testclearNotificationsAndVerifyThatclearNotificationListenersWithArgsIsCalled() + { + # Mock NotificationCenter + $this->notificationCenterMock = $this->getMockBuilder(NotificationCenter::class) + ->setConstructorArgs(array($this->loggerMock, $this->errorHandlerMock)) + ->setMethods(array('clearNotificationListeners')) + ->getMock(); + + # Log deprecation message + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with( + Logger::WARNING, + sprintf("'clearNotifications' is deprecated. Call 'clearNotificationListeners' instead.") + ); + + $this->notificationCenterMock->expects($this->once()) + ->method('clearNotificationListeners') + ->with( + NotificationType::ACTIVATE + ); + + $this->notificationCenterMock->clearNotifications(NotificationType::ACTIVATE); + } + + public function testclearNotificationListeners() { // ensure that notifications length is zero for each notification type - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); // add a callback for multiple notification types $this->notificationCenterObj->addNotificationListener( @@ -457,7 +482,7 @@ function () { ->method('handleError') ->with(new InvalidNotificationTypeException('Invalid notification type.')); - $this->assertNull($this->notificationCenterObj->clearNotifications($invalid_type)); + $this->assertNull($this->notificationCenterObj->clearNotificationListeners($invalid_type)); // Verify that notifications length for NotificationType::ACTIVATE is still 2 $this->assertSame( @@ -482,7 +507,7 @@ function () { sprintf("All callbacks for notification type '%s' have been removed.", NotificationType::ACTIVATE) ); - $this->notificationCenterObj->clearNotifications(NotificationType::ACTIVATE); + $this->notificationCenterObj->clearNotificationListeners(NotificationType::ACTIVATE); // Verify that notifications length for NotificationType::ACTIVATE is now 0 $this->assertSame( @@ -499,11 +524,11 @@ function () { /////////////////////////////////////////////////////////////////////////////////////////////////////////// // == Verify that no error is thrown when clearNotification is called again for the same notification type === // /////////////////////////////////////////////////////////////////////////////////////////////////////////// - $this->notificationCenterObj->clearNotifications(NotificationType::ACTIVATE); + $this->notificationCenterObj->clearNotificationListeners(NotificationType::ACTIVATE); } - public function testCleanAllNotifications() + public function testclearAllNotificationListeners() { // using a new notification center object to avoid using the method being tested, // to reset notifications list @@ -558,10 +583,10 @@ function () { ); //////////////////////////////////////////////////////////////////////////////////////////////////// - // === verify that cleanAllNotifications removes all notifications for each notification type === // + // === verify that clearAllNotificationListeners removes all notifications for each notification type === // //////////////////////////////////////////////////////////////////////////////////////////////////// - $notificationCenterA->cleanAllNotifications(); + $notificationCenterA->clearAllNotificationListeners(); // verify that notifications length for each type is now set to 0 $this->assertSame( @@ -574,15 +599,37 @@ function () { ); /////////////////////////////////////////////////////////////////////////////////////// - //=== verify that cleanAllNotifications doesn't throw an error when called again === // + //=== verify that clearAllNotificationListeners doesn't throw an error when called again === // /////////////////////////////////////////////////////////////////////////////////////// - $notificationCenterA->cleanAllNotifications(); + $notificationCenterA->clearAllNotificationListeners(); + } + + public function testcleanAllNotificationsAndVerifyThatclearAllNotificationListenersIsCalled() + { + # Mock NotificationCenter + $this->notificationCenterMock = $this->getMockBuilder(NotificationCenter::class) + ->setConstructorArgs(array($this->loggerMock, $this->errorHandlerMock)) + ->setMethods(array('clearAllNotificationListeners')) + ->getMock(); + + # Log deprecation message + $this->loggerMock->expects($this->at(0)) + ->method('log') + ->with( + Logger::WARNING, + sprintf("'cleanAllNotifications' is deprecated. Call 'clearAllNotificationListeners' instead.") + ); + + $this->notificationCenterMock->expects($this->once()) + ->method('clearAllNotificationListeners'); + + $this->notificationCenterMock->cleanAllNotifications(); } public function testsendNotificationsGivenLessThanExpectedNumberOfArguments() { $clientObj = new FireNotificationTester; - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); // add a notification callback with arguments $this->notificationCenterObj->addNotificationListener( @@ -610,7 +657,7 @@ public function testsendNotificationsAndVerifyThatAllCallbacksWithoutArgsAreCall ->setMethods(array('decision_callback_no_args', 'decision_callback_no_args_2', 'track_callback_no_args')) ->getMock(); - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); //add notification callbacks $this->notificationCenterObj->addNotificationListener( @@ -661,7 +708,7 @@ public function testsendNotificationsAndVerifyThatAllCallbacksWithArgsAreCalled( ->setMethods(array('decision_callback_with_args', 'decision_callback_with_args_2', 'track_callback_no_args')) ->getMock(); - $this->notificationCenterObj->cleanAllNotifications(); + $this->notificationCenterObj->clearAllNotificationListeners(); //add notification callbacks with args $this->notificationCenterObj->addNotificationListener( From cf765adcad01aabeb2ac34275499572e8e5a4c7a Mon Sep 17 00:00:00 2001 From: rashidsp Date: Mon, 2 Jul 2018 13:19:53 +0500 Subject: [PATCH 2/2] review changes --- tests/NotificationTests/NotificationCenterTest.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/NotificationTests/NotificationCenterTest.php b/tests/NotificationTests/NotificationCenterTest.php index a79259b1..b5774bde 100644 --- a/tests/NotificationTests/NotificationCenterTest.php +++ b/tests/NotificationTests/NotificationCenterTest.php @@ -407,7 +407,7 @@ function () { ); } - public function testclearNotificationsAndVerifyThatclearNotificationListenersWithArgsIsCalled() + public function testClearNotificationsAndVerifyThatClearNotificationListenersWithArgsIsCalled() { # Mock NotificationCenter $this->notificationCenterMock = $this->getMockBuilder(NotificationCenter::class) @@ -432,7 +432,7 @@ public function testclearNotificationsAndVerifyThatclearNotificationListenersWit $this->notificationCenterMock->clearNotifications(NotificationType::ACTIVATE); } - public function testclearNotificationListeners() + public function testClearNotificationListeners() { // ensure that notifications length is zero for each notification type $this->notificationCenterObj->clearAllNotificationListeners(); @@ -528,7 +528,7 @@ function () { } - public function testclearAllNotificationListeners() + public function testClearAllNotificationListeners() { // using a new notification center object to avoid using the method being tested, // to reset notifications list @@ -604,7 +604,7 @@ function () { $notificationCenterA->clearAllNotificationListeners(); } - public function testcleanAllNotificationsAndVerifyThatclearAllNotificationListenersIsCalled() + public function testCleanAllNotificationsAndVerifyThatClearAllNotificationListenersIsCalled() { # Mock NotificationCenter $this->notificationCenterMock = $this->getMockBuilder(NotificationCenter::class) @@ -626,7 +626,7 @@ public function testcleanAllNotificationsAndVerifyThatclearAllNotificationListen $this->notificationCenterMock->cleanAllNotifications(); } - public function testsendNotificationsGivenLessThanExpectedNumberOfArguments() + public function testSendNotificationsGivenLessThanExpectedNumberOfArguments() { $clientObj = new FireNotificationTester; $this->notificationCenterObj->clearAllNotificationListeners(); @@ -651,7 +651,7 @@ public function testsendNotificationsGivenLessThanExpectedNumberOfArguments() $this->notificationCenterObj->sendNotifications(NotificationType::ACTIVATE, array("HelloWorld")); } - public function testsendNotificationsAndVerifyThatAllCallbacksWithoutArgsAreCalled() + public function testSendNotificationsAndVerifyThatAllCallbacksWithoutArgsAreCalled() { $clientMock = $this->getMockBuilder(FireNotificationTester::class) ->setMethods(array('decision_callback_no_args', 'decision_callback_no_args_2', 'track_callback_no_args')) @@ -702,7 +702,7 @@ public function testsendNotificationsAndVerifyThatAllCallbacksWithoutArgsAreCall $this->notificationCenterObj->sendNotifications("abacada"); } - public function testsendNotificationsAndVerifyThatAllCallbacksWithArgsAreCalled() + public function testSendNotificationsAndVerifyThatAllCallbacksWithArgsAreCalled() { $clientMock = $this->getMockBuilder(FireNotificationTester::class) ->setMethods(array('decision_callback_with_args', 'decision_callback_with_args_2', 'track_callback_no_args'))