From 1116adbc9490387783e538a5fd943c943b666598 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Thu, 2 Jul 2020 17:59:25 +0200 Subject: [PATCH] [6.x] Fix notifications database channel for anonymous notifiables (#33409) * Prevent usage of database channel with anonymous notifiable * Prevent sending for anonymous notifiables and database channel * Update AnonymousNotifiable.php * Update NotificationRoutesNotificationsTest.php Co-authored-by: Taylor Otwell --- .../Notifications/AnonymousNotifiable.php | 5 ++ .../Notifications/NotificationSender.php | 4 +- .../NotificationRoutesNotificationsTest.php | 11 ++++ .../Notifications/NotificationSenderTest.php | 59 +++++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Notifications/AnonymousNotifiable.php b/src/Illuminate/Notifications/AnonymousNotifiable.php index d820239f165c..eab959b7c564 100644 --- a/src/Illuminate/Notifications/AnonymousNotifiable.php +++ b/src/Illuminate/Notifications/AnonymousNotifiable.php @@ -3,6 +3,7 @@ namespace Illuminate\Notifications; use Illuminate\Contracts\Notifications\Dispatcher; +use InvalidArgumentException; class AnonymousNotifiable { @@ -22,6 +23,10 @@ class AnonymousNotifiable */ public function route($channel, $route) { + if ($channel === 'database') { + throw new InvalidArgumentException('The database channel does not support on-demand notifications.'); + } + $this->routes[$channel] = $route; return $this; diff --git a/src/Illuminate/Notifications/NotificationSender.php b/src/Illuminate/Notifications/NotificationSender.php index 688c1cd5d8f7..c65940aad3ea 100644 --- a/src/Illuminate/Notifications/NotificationSender.php +++ b/src/Illuminate/Notifications/NotificationSender.php @@ -102,7 +102,9 @@ public function sendNow($notifiables, $notification, array $channels = null) $notificationId = Str::uuid()->toString(); foreach ((array) $viaChannels as $channel) { - $this->sendToNotifiable($notifiable, $notificationId, clone $original, $channel); + if (! ($notifiable instanceof AnonymousNotifiable && $channel === 'database')) { + $this->sendToNotifiable($notifiable, $notificationId, clone $original, $channel); + } } }); } diff --git a/tests/Notifications/NotificationRoutesNotificationsTest.php b/tests/Notifications/NotificationRoutesNotificationsTest.php index f7d903ad34c6..af02994f64e1 100644 --- a/tests/Notifications/NotificationRoutesNotificationsTest.php +++ b/tests/Notifications/NotificationRoutesNotificationsTest.php @@ -5,6 +5,8 @@ use Illuminate\Container\Container; use Illuminate\Contracts\Notifications\Dispatcher; use Illuminate\Notifications\RoutesNotifications; +use Illuminate\Support\Facades\Notification; +use InvalidArgumentException; use Mockery as m; use PHPUnit\Framework\TestCase; use stdClass; @@ -50,6 +52,15 @@ public function testNotificationOptionRouting() $this->assertSame('bar', $instance->routeNotificationFor('foo')); $this->assertSame('taylor@laravel.com', $instance->routeNotificationFor('mail')); } + + public function testOnDemandNotificationsCannotUseDatabaseChannel() + { + $this->expectExceptionObject( + new InvalidArgumentException('The database channel does not support on-demand notifications.') + ); + + Notification::route('database', 'foo'); + } } class RoutesNotificationsTestInstance diff --git a/tests/Notifications/NotificationSenderTest.php b/tests/Notifications/NotificationSenderTest.php index 8dd398f47bf2..6c5a6aaf849d 100644 --- a/tests/Notifications/NotificationSenderTest.php +++ b/tests/Notifications/NotificationSenderTest.php @@ -6,6 +6,7 @@ use Illuminate\Contracts\Bus\Dispatcher as BusDispatcher; use Illuminate\Contracts\Events\Dispatcher as EventDispatcher; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Notifications\ChannelManager; use Illuminate\Notifications\Notifiable; use Illuminate\Notifications\Notification; @@ -34,6 +35,32 @@ public function testItCanSendQueuedNotificationsWithAStringVia() $sender->send($notifiable, new DummyQueuedNotificationWithStringVia()); } + + public function testItCanSendNotificationsWithAnEmptyStringVia() + { + $notifiable = new AnonymousNotifiable; + $manager = m::mock(ChannelManager::class); + $bus = m::mock(BusDispatcher::class); + $bus->shouldNotReceive('dispatch'); + $events = m::mock(EventDispatcher::class); + + $sender = new NotificationSender($manager, $bus, $events); + + $sender->sendNow($notifiable, new DummyNotificationWithEmptyStringVia()); + } + + public function testItCannotSendNotificationsViaDatabaseForAnonymousNotifiables() + { + $notifiable = new AnonymousNotifiable; + $manager = m::mock(ChannelManager::class); + $bus = m::mock(BusDispatcher::class); + $bus->shouldNotReceive('dispatch'); + $events = m::mock(EventDispatcher::class); + + $sender = new NotificationSender($manager, $bus, $events); + + $sender->sendNow($notifiable, new DummyNotificationWithDatabaseVia()); + } } class DummyQueuedNotificationWithStringVia extends Notification implements ShouldQueue @@ -51,3 +78,35 @@ public function via($notifiable) return 'mail'; } } + +class DummyNotificationWithEmptyStringVia extends Notification +{ + use Queueable; + + /** + * Get the notification channels. + * + * @param mixed $notifiable + * @return array|string + */ + public function via($notifiable) + { + return ''; + } +} + +class DummyNotificationWithDatabaseVia extends Notification +{ + use Queueable; + + /** + * Get the notification channels. + * + * @param mixed $notifiable + * @return array|string + */ + public function via($notifiable) + { + return 'database'; + } +}