-
Notifications
You must be signed in to change notification settings - Fork 28
Feature Notification Center #73
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
Merged
mikeproeng37
merged 10 commits into
optimizely:master
from
msohailhussain:Feature-NotificationCenter
Nov 29, 2017
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
42a285a
Feature Notification Center (#13)
oakbani bd218fc
:pen: Renamed notification type constants
oakbani 898242f
:pen: Renamed fireNotifications to sendNot..
oakbani ab37eb7
Merge remote-tracking branch 'remotes/optimizely/master' into Feature…
oakbani e8863d6
:pen: Code coverage nit
oakbani a18700d
:pencil2: feature_experiment/rollout notifications removed
oakbani 870cb5e
nit: Space
oakbani c0a1ada
Merge branch 'Feature-NotificationCenter' of https://github.com/msoha…
oakbani 146a886
Merge branch 'master' into Feature-NotificationCenter
thomaszurkan-optimizely a2e8bee
Improve Code Coverage
oakbani File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
23 changes: 23 additions & 0 deletions
23
src/Optimizely/Exceptions/InvalidCallbackArgumentCountException.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
| /** | ||
| * Copyright 2017, 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. | ||
| */ | ||
|
|
||
| namespace Optimizely\Exceptions; | ||
|
|
||
|
|
||
| class InvalidCallbackArgumentCountException extends OptimizelyException | ||
| { | ||
| } |
23 changes: 23 additions & 0 deletions
23
src/Optimizely/Exceptions/InvalidNotificationTypeException.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
| /** | ||
| * Copyright 2017, 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. | ||
| */ | ||
|
|
||
| namespace Optimizely\Exceptions; | ||
|
|
||
|
|
||
| class InvalidNotificationTypeException extends OptimizelyException | ||
| { | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| <?php | ||
| /** | ||
| * Copyright 2017, Optimizely Inc and Contributors | ||
| * | ||
| * 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. | ||
| */ | ||
| namespace Optimizely\Notification; | ||
|
|
||
| use ArgumentCountError; | ||
| use Exception; | ||
| use Throwable; | ||
|
|
||
| use Monolog\Logger; | ||
|
|
||
| use Optimizely\ErrorHandler\ErrorHandlerInterface; | ||
| use Optimizely\Exceptions\InvalidCallbackArgumentCountException; | ||
| use Optimizely\Exceptions\InvalidNotificationTypeException; | ||
| use Optimizely\Logger\LoggerInterface; | ||
| use Optimizely\Logger\NoOpLogger; | ||
|
|
||
| class NotificationCenter | ||
| { | ||
| private $_notificationId; | ||
|
|
||
| private $_notifications; | ||
|
|
||
| private $_logger; | ||
|
|
||
| private $_errorHandler; | ||
|
|
||
| public function __construct(LoggerInterface $logger, ErrorHandlerInterface $errorHandler) | ||
| { | ||
| $this->_notificationId = 1; | ||
| $this->_notifications = []; | ||
| foreach (array_values(NotificationType::getAll()) as $type) { | ||
| $this->_notifications[$type] = []; | ||
| } | ||
|
|
||
| $this->_logger = $logger; | ||
| $this->_errorHandler = $errorHandler; | ||
| } | ||
|
|
||
| public function getNotifications() | ||
| { | ||
| return $this->_notifications; | ||
| } | ||
|
|
||
| /** | ||
| * Adds a notification callback for a notification type to the notification center | ||
| * @param string $notification_type One of the constants defined in NotificationType | ||
| * @param string $notification_callback A valid PHP callback | ||
| * | ||
| * @return null Given invalid notification type/callback | ||
| * -1 Given callback has been already added | ||
| * int Notification ID for the added callback | ||
| */ | ||
| public function addNotificationListener($notification_type, $notification_callback) | ||
| { | ||
| if (!NotificationType::isNotificationTypeValid($notification_type)) { | ||
| $this->_logger->log(Logger::ERROR, "Invalid notification type."); | ||
| $this->_errorHandler->handleError(new InvalidNotificationTypeException('Invalid notification type.')); | ||
| return null; | ||
| } | ||
|
|
||
| if (!is_callable($notification_callback)) { | ||
| $this->_logger->log(Logger::ERROR, "Invalid notification callback."); | ||
| return null; | ||
| } | ||
|
|
||
| foreach (array_values($this->_notifications[$notification_type]) as $callback) { | ||
| if ($notification_callback == $callback) { | ||
| // Note: anonymous methods sent with the same body will be re-added. | ||
| // Only variable and object methods can be checked for duplication | ||
| $this->_logger->log(Logger::DEBUG, "Callback already added for notification type '{$notification_type}'."); | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| $this->_notifications[$notification_type][$this->_notificationId] = $notification_callback; | ||
| $this->_logger->log(Logger::INFO, "Callback added for notification type '{$notification_type}'."); | ||
| $returnVal = $this->_notificationId++; | ||
| return $returnVal; | ||
| } | ||
|
|
||
| /** | ||
| * Removes notification callback from the notification center | ||
| * @param int $notification_id notification IT | ||
| * | ||
| * @return true When callback removed | ||
| * false When no callback found for the given notification ID | ||
| */ | ||
| public function removeNotificationListener($notification_id) | ||
| { | ||
| foreach ($this->_notifications as $notification_type => $notifications) { | ||
| foreach (array_keys($notifications) as $id) { | ||
| if ($notification_id == $id) { | ||
| unset($this->_notifications[$notification_type][$id]); | ||
| $this->_logger->log(Logger::INFO, "Callback with notification ID '{$notification_id}' has been removed."); | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $this->_logger->log(Logger::DEBUG, "No Callback found with notification ID '{$notification_id}'."); | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * 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) | ||
| { | ||
| if (!NotificationType::isNotificationTypeValid($notification_type)) { | ||
| $this->_logger->log(Logger::ERROR, "Invalid notification type."); | ||
| $this->_errorHandler->handleError(new InvalidNotificationTypeException('Invalid notification type.')); | ||
| return; | ||
| } | ||
|
|
||
| $this->_notifications[$notification_type] = []; | ||
| $this->_logger->log(Logger::INFO, "All callbacks for notification type '{$notification_type}' have been removed."); | ||
| } | ||
|
|
||
| /** | ||
| * Removes all notifications for all notification types | ||
| * from the notification center | ||
| * | ||
| */ | ||
| public function cleanAllNotifications() | ||
| { | ||
| foreach (array_values(NotificationType::getAll()) as $type) { | ||
| $this->_notifications[$type] = []; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Executes all registered callbacks for the given notification type | ||
| * @param [type] $notification_type One of the constants defined in NotificationType | ||
| * @param array $args Array of items to pass as arguments to the callback | ||
| * | ||
| */ | ||
| public function sendNotifications($notification_type, array $args = []) | ||
| { | ||
| if (!isset($this->_notifications[$notification_type])) { | ||
| // No exception thrown and error logged since this method will be called from | ||
| // within the SDK | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * Note: Before PHP 7, if the callback in call_user_func is called with less number of arguments than expected, | ||
| * a warning is issued but the method is still executed with assigning null to the remaining | ||
| * arguments. From PHP 7, ArgumentCountError is thrown in such case and the method isn't executed. | ||
| * Therefore, we set error handler for warnings so that we raise an exception and notify the | ||
| * user that the registered callback has more number of arguments than | ||
| * expected. This should be done to keep a consistent behavior across all PHP versions. | ||
| */ | ||
|
|
||
| set_error_handler(array($this, 'reportArgumentCountError'), E_WARNING); | ||
|
|
||
| foreach (array_values($this->_notifications[$notification_type]) as $callback) { | ||
| try { | ||
| call_user_func_array($callback, $args); | ||
| } catch (ArgumentCountError $e) { | ||
| $this->reportArgumentCountError(); | ||
| } catch (Exception $e) { | ||
| $this->_logger->log(Logger::ERROR, "Problem calling notify callback."); | ||
| } | ||
| } | ||
|
|
||
| restore_error_handler(); | ||
| } | ||
|
|
||
| /** | ||
| * Logs and raises an exception when registered callback expects more number of arguments when executed | ||
| * | ||
| */ | ||
| public function reportArgumentCountError() | ||
| { | ||
| $this->_logger->log(Logger::ERROR, "Problem calling notify callback."); | ||
| $this->_errorHandler->handleError( | ||
| new InvalidCallbackArgumentCountException('Registered callback expects more number of arguments than the actual number') | ||
| ); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| <?php | ||
| /** | ||
| * Copyright 2017, Optimizely Inc and Contributors | ||
| * | ||
| * 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. | ||
| */ | ||
| namespace Optimizely\Notification; | ||
|
|
||
| class NotificationType | ||
| { | ||
| // format is EVENT: list of parameters to callback. | ||
| const ACTIVATE = "ACTIVATE:experiment, user_id, attributes, variation, event"; | ||
| const TRACK = "TRACK:event_key, user_id, attributes, event_tags, event"; | ||
|
|
||
| public static function isNotificationTypeValid($notification_type) | ||
| { | ||
| $oClass = new \ReflectionClass(__CLASS__); | ||
| $notificationTypeList = array_values($oClass->getConstants()); | ||
|
|
||
| return in_array($notification_type, $notificationTypeList); | ||
| } | ||
|
|
||
| public static function getAll() | ||
| { | ||
| $oClass = new \ReflectionClass(__CLASS__); | ||
| return $oClass->getConstants(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the sendImpression. Why not add a sendConversion as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sendImpression logic got separated from activate when we implemented isFeatureEnabled that had to send an impression event as well. Track is the only method right now that sends a conversion event. Separating out unit tests for sendConversion will require a lot of changes. Shall I do it in this PR or can we do it later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do it later if necessary.