From 185a737273eef7625ecd081cfbdd119a1e16af0b Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Thu, 5 Nov 2020 18:09:06 +0100 Subject: [PATCH] Add Notification Channel Extender (#2432) --- framework/core/src/Extend/Notification.php | 46 ++++++- .../Driver/AlertNotificationDriver.php | 50 ++++++++ .../Driver/EmailNotificationDriver.php | 69 ++++++++++ .../Driver/NotificationDriverInterface.php | 34 +++++ .../core/src/Notification/Event/Sending.php | 3 + .../Notification/Job/SendNotificationsJob.php | 3 - .../NotificationServiceProvider.php | 41 +++--- .../src/Notification/NotificationSyncer.php | 59 ++++----- .../extenders/NotificationTest.php | 118 +++++++++++++++++- 9 files changed, 371 insertions(+), 52 deletions(-) create mode 100644 framework/core/src/Notification/Driver/AlertNotificationDriver.php create mode 100644 framework/core/src/Notification/Driver/EmailNotificationDriver.php create mode 100644 framework/core/src/Notification/Driver/NotificationDriverInterface.php diff --git a/framework/core/src/Extend/Notification.php b/framework/core/src/Extend/Notification.php index ad91e18bb..fd0f59a1b 100644 --- a/framework/core/src/Extend/Notification.php +++ b/framework/core/src/Extend/Notification.php @@ -16,23 +16,63 @@ class Notification implements ExtenderInterface { private $blueprints = []; private $serializers = []; + private $drivers = []; + private $typesEnabledByDefault = []; - public function type(string $blueprint, string $serializer, array $channelsEnabledByDefault = []) + /** + * @param string $blueprint The ::class attribute of the blueprint class. + * This blueprint should implement \Flarum\Notification\Blueprint\BlueprintInterface. + * @param string $serializer The ::class attribute of the serializer class. + * This serializer should extend from \Flarum\Api\Serializer\AbstractSerializer. + * @param string[] $driversEnabledByDefault The names of the drivers enabled by default for this notification type. + * (example: alert, email). + * @return self + */ + public function type(string $blueprint, string $serializer, array $driversEnabledByDefault = []) { - $this->blueprints[$blueprint] = $channelsEnabledByDefault; + $this->blueprints[$blueprint] = $driversEnabledByDefault; $this->serializers[$blueprint::getType()] = $serializer; return $this; } + /** + * @param string $driverName The name of the notification driver. + * @param string $driver The ::class attribute of the driver class. + * This driver should implement \Flarum\Notification\Driver\NotificationDriverInterface. + * @param string[] $typesEnabledByDefault The names of blueprint classes of types enabled by default for this driver. + * @return self + */ + public function driver(string $driverName, string $driver, array $typesEnabledByDefault = []) + { + $this->drivers[$driverName] = $driver; + $this->typesEnabledByDefault[$driverName] = $typesEnabledByDefault; + + return $this; + } + public function extend(Container $container, Extension $extension = null) { $container->extend('flarum.notification.blueprints', function ($existingBlueprints) { - return array_merge($existingBlueprints, $this->blueprints); + $existingBlueprints = array_merge($existingBlueprints, $this->blueprints); + + foreach ($this->typesEnabledByDefault as $driverName => $typesEnabledByDefault) { + foreach ($typesEnabledByDefault as $blueprintClass) { + if (isset($existingBlueprints[$blueprintClass]) && (! in_array($driverName, $existingBlueprints[$blueprintClass]))) { + $existingBlueprints[$blueprintClass][] = $driverName; + } + } + } + + return $existingBlueprints; }); $container->extend('flarum.api.notification_serializers', function ($existingSerializers) { return array_merge($existingSerializers, $this->serializers); }); + + $container->extend('flarum.notification.drivers', function ($existingDrivers) { + return array_merge($existingDrivers, $this->drivers); + }); } } diff --git a/framework/core/src/Notification/Driver/AlertNotificationDriver.php b/framework/core/src/Notification/Driver/AlertNotificationDriver.php new file mode 100644 index 000000000..ccaba0169 --- /dev/null +++ b/framework/core/src/Notification/Driver/AlertNotificationDriver.php @@ -0,0 +1,50 @@ +queue = $queue; + } + + /** + * {@inheritDoc} + */ + public function send(BlueprintInterface $blueprint, array $users): void + { + if (count($users)) { + $this->queue->push(new SendNotificationsJob($blueprint, $users)); + } + } + + /** + * {@inheritdoc} + */ + public function registerType(string $blueprintClass, array $driversEnabledByDefault): void + { + User::addPreference( + User::getNotificationPreferenceKey($blueprintClass::getType(), 'alert'), + 'boolval', + in_array('alert', $driversEnabledByDefault) + ); + } +} diff --git a/framework/core/src/Notification/Driver/EmailNotificationDriver.php b/framework/core/src/Notification/Driver/EmailNotificationDriver.php new file mode 100644 index 000000000..36cdb8b9d --- /dev/null +++ b/framework/core/src/Notification/Driver/EmailNotificationDriver.php @@ -0,0 +1,69 @@ +queue = $queue; + } + + /** + * {@inheritDoc} + */ + public function send(BlueprintInterface $blueprint, array $users): void + { + if ($blueprint instanceof MailableInterface) { + $this->mailNotifications($blueprint, $users); + } + } + + /** + * Mail a notification to a list of users. + * + * @param MailableInterface $blueprint + * @param User[] $recipients + */ + protected function mailNotifications(MailableInterface $blueprint, array $recipients) + { + foreach ($recipients as $user) { + if ($user->shouldEmail($blueprint::getType())) { + $this->queue->push(new SendEmailNotificationJob($blueprint, $user)); + } + } + } + + /** + * {@inheritdoc} + */ + public function registerType(string $blueprintClass, array $driversEnabledByDefault): void + { + if ((new ReflectionClass($blueprintClass))->implementsInterface(MailableInterface::class)) { + User::addPreference( + User::getNotificationPreferenceKey($blueprintClass::getType(), 'email'), + 'boolval', + in_array('email', $driversEnabledByDefault) + ); + } + } +} diff --git a/framework/core/src/Notification/Driver/NotificationDriverInterface.php b/framework/core/src/Notification/Driver/NotificationDriverInterface.php new file mode 100644 index 000000000..b2fb24bda --- /dev/null +++ b/framework/core/src/Notification/Driver/NotificationDriverInterface.php @@ -0,0 +1,34 @@ +blueprint, $this->recipients)); - Notification::notify($this->recipients, $this->blueprint); } } diff --git a/framework/core/src/Notification/NotificationServiceProvider.php b/framework/core/src/Notification/NotificationServiceProvider.php index c7cca3217..7a169e1f6 100644 --- a/framework/core/src/Notification/NotificationServiceProvider.php +++ b/framework/core/src/Notification/NotificationServiceProvider.php @@ -12,8 +12,6 @@ namespace Flarum\Notification; use Flarum\Event\ConfigureNotificationTypes; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Notification\Blueprint\DiscussionRenamedBlueprint; -use Flarum\User\User; -use ReflectionClass; class NotificationServiceProvider extends AbstractServiceProvider { @@ -22,6 +20,13 @@ class NotificationServiceProvider extends AbstractServiceProvider */ public function register() { + $this->app->singleton('flarum.notification.drivers', function () { + return [ + 'alert' => Driver\AlertNotificationDriver::class, + 'email' => Driver\EmailNotificationDriver::class, + ]; + }); + $this->app->singleton('flarum.notification.blueprints', function () { return [ DiscussionRenamedBlueprint::class => ['alert'] @@ -34,9 +39,20 @@ class NotificationServiceProvider extends AbstractServiceProvider */ public function boot() { + $this->setNotificationDrivers(); $this->setNotificationTypes(); } + /** + * Register notification drivers. + */ + protected function setNotificationDrivers() + { + foreach ($this->app->make('flarum.notification.drivers') as $driverName => $driver) { + NotificationSyncer::addNotificationDriver($driverName, $this->app->make($driver)); + } + } + /** * Register notification types. */ @@ -49,29 +65,22 @@ class NotificationServiceProvider extends AbstractServiceProvider new ConfigureNotificationTypes($blueprints) ); - foreach ($blueprints as $blueprint => $channelsEnabledByDefault) { - $this->addType($blueprint, $channelsEnabledByDefault); + foreach ($blueprints as $blueprint => $driversEnabledByDefault) { + $this->addType($blueprint, $driversEnabledByDefault); } } - protected function addType(string $blueprint, array $channelsEnabledByDefault) + protected function addType(string $blueprint, array $driversEnabledByDefault) { Notification::setSubjectModel( $type = $blueprint::getType(), $blueprint::getSubjectModel() ); - User::addPreference( - User::getNotificationPreferenceKey($type, 'alert'), - 'boolval', - in_array('alert', $channelsEnabledByDefault) - ); - - if ((new ReflectionClass($blueprint))->implementsInterface(MailableInterface::class)) { - User::addPreference( - User::getNotificationPreferenceKey($type, 'email'), - 'boolval', - in_array('email', $channelsEnabledByDefault) + foreach (NotificationSyncer::getNotificationDrivers() as $driverName => $driver) { + $driver->registerType( + $blueprint, + $driversEnabledByDefault ); } } diff --git a/framework/core/src/Notification/NotificationSyncer.php b/framework/core/src/Notification/NotificationSyncer.php index 1bcc6ebb7..b6c40ead2 100644 --- a/framework/core/src/Notification/NotificationSyncer.php +++ b/framework/core/src/Notification/NotificationSyncer.php @@ -10,10 +10,9 @@ namespace Flarum\Notification; use Flarum\Notification\Blueprint\BlueprintInterface; -use Flarum\Notification\Job\SendEmailNotificationJob; -use Flarum\Notification\Job\SendNotificationsJob; +use Flarum\Notification\Driver\NotificationDriverInterface; +use Flarum\Notification\Event\Sending; use Flarum\User\User; -use Illuminate\Contracts\Queue\Queue; /** * The Notification Syncer commits notification blueprints to the database, and @@ -38,14 +37,11 @@ class NotificationSyncer protected static $sentTo = []; /** - * @var Queue + * A map of notification drivers. + * + * @var NotificationDriverInterface[] */ - protected $queue; - - public function __construct(Queue $queue) - { - $this->queue = $queue; - } + protected static $notificationDrivers = []; /** * Sync a notification so that it is visible to the specified users, and not @@ -102,12 +98,13 @@ class NotificationSyncer // receiving this notification for the first time (we know because they // didn't have a record in the database). As both operations can be // intensive on resources (database and mail server), we queue them. - if (count($newRecipients)) { - $this->queue->push(new SendNotificationsJob($blueprint, $newRecipients)); + foreach (static::getNotificationDrivers() as $driverName => $driver) { + $driver->send($blueprint, $newRecipients); } - if ($blueprint instanceof MailableInterface) { - $this->mailNotifications($blueprint, $newRecipients); + if (count($newRecipients)) { + // Deprecated in beta 15, removed in beta 16 + event(new Sending($blueprint, $newRecipients)); } } @@ -150,21 +147,6 @@ class NotificationSyncer static::$onePerUser = false; } - /** - * Mail a notification to a list of users. - * - * @param MailableInterface $blueprint - * @param User[] $recipients - */ - protected function mailNotifications(MailableInterface $blueprint, array $recipients) - { - foreach ($recipients as $user) { - if ($user->shouldEmail($blueprint::getType())) { - $this->queue->push(new SendEmailNotificationJob($blueprint, $user)); - } - } - } - /** * Set the deleted status of a list of notification records. * @@ -175,4 +157,23 @@ class NotificationSyncer { Notification::whereIn('id', $ids)->update(['is_deleted' => $isDeleted]); } + + /** + * Adds a notification driver to the list. + * + * @param string $driverName + * @param NotificationDriverInterface $driver + */ + public static function addNotificationDriver(string $driverName, NotificationDriverInterface $driver) + { + static::$notificationDrivers[$driverName] = $driver; + } + + /** + * @return NotificationDriverInterface[] + */ + public static function getNotificationDrivers(): array + { + return static::$notificationDrivers; + } } diff --git a/framework/core/tests/integration/extenders/NotificationTest.php b/framework/core/tests/integration/extenders/NotificationTest.php index 03388e529..2fd8ad46f 100644 --- a/framework/core/tests/integration/extenders/NotificationTest.php +++ b/framework/core/tests/integration/extenders/NotificationTest.php @@ -11,9 +11,12 @@ namespace Flarum\Tests\integration\extenders; use Flarum\Extend; use Flarum\Notification\Blueprint\BlueprintInterface; +use Flarum\Notification\Driver\NotificationDriverInterface; use Flarum\Notification\Notification; +use Flarum\Notification\NotificationSyncer; +use Flarum\Tests\integration\TestCase; -class NotificationTest extends \Flarum\Tests\integration\TestCase +class NotificationTest extends TestCase { /** * @test @@ -23,6 +26,27 @@ class NotificationTest extends \Flarum\Tests\integration\TestCase $this->assertArrayNotHasKey('customNotificationType', Notification::getSubjectModels()); } + /** + * @test + */ + public function notification_serializer_doesnt_exist_by_default() + { + $this->app(); + + $this->assertNotContains( + 'customNotificationTypeSerializer', + $this->app->getContainer()->make('flarum.api.notification_serializers') + ); + } + + /** + * @test + */ + public function notification_driver_doesnt_exist_by_default() + { + $this->assertArrayNotHasKey('customNotificationDriver', NotificationSyncer::getNotificationDrivers()); + } + /** * @test */ @@ -37,6 +61,64 @@ class NotificationTest extends \Flarum\Tests\integration\TestCase $this->assertArrayHasKey('customNotificationType', Notification::getSubjectModels()); } + + /** + * @test + */ + public function notification_serializer_exists_if_added() + { + $this->extend((new Extend\Notification)->type( + CustomNotificationType::class, + 'customNotificationTypeSerializer' + )); + + $this->app(); + + $this->assertContains( + 'customNotificationTypeSerializer', + $this->app->getContainer()->make('flarum.api.notification_serializers') + ); + } + + /** + * @test + */ + public function notification_driver_exists_if_added() + { + $this->extend((new Extend\Notification())->driver( + 'customNotificationDriver', + CustomNotificationDriver::class + )); + + $this->app(); + + $this->assertArrayHasKey('customNotificationDriver', NotificationSyncer::getNotificationDrivers()); + } + + /** + * @test + */ + public function notification_driver_enabled_types_exist_if_added() + { + $this->extend( + (new Extend\Notification()) + ->type(CustomNotificationType::class, 'customSerializer') + ->type(SecondCustomNotificationType::class, 'secondCustomSerializer', ['customDriver']) + ->type(ThirdCustomNotificationType::class, 'thirdCustomSerializer') + ->driver('customDriver', CustomNotificationDriver::class, [CustomNotificationType::class]) + ->driver('secondCustomDriver', SecondCustomNotificationDriver::class, [SecondCustomNotificationType::class]) + ); + + $this->app(); + + $blueprints = $this->app->getContainer()->make('flarum.notification.blueprints'); + + $this->assertContains('customDriver', $blueprints[CustomNotificationType::class]); + $this->assertCount(1, $blueprints[CustomNotificationType::class]); + $this->assertContains('customDriver', $blueprints[SecondCustomNotificationType::class]); + $this->assertContains('secondCustomDriver', $blueprints[SecondCustomNotificationType::class]); + $this->assertEmpty($blueprints[ThirdCustomNotificationType::class]); + } } class CustomNotificationType implements BlueprintInterface @@ -66,3 +148,37 @@ class CustomNotificationType implements BlueprintInterface return 'customNotificationTypeSubjectModel'; } } + +class SecondCustomNotificationType extends CustomNotificationType +{ + public static function getType() + { + return 'secondCustomNotificationType'; + } +} + +class ThirdCustomNotificationType extends CustomNotificationType +{ + public static function getType() + { + return 'thirdCustomNotificationType'; + } +} + +class CustomNotificationDriver implements NotificationDriverInterface +{ + public function send(BlueprintInterface $blueprint, array $users): void + { + // ... + } + + public function registerType(string $blueprintClass, array $driversEnabledByDefault): void + { + // ... + } +} + +class SecondCustomNotificationDriver extends CustomNotificationDriver +{ + // ... +}