[9.x] Fix unique locking on broadcast events (laravel#43516)

* Fix broadcasting locks * Update BroadcastManager.php * Respect event uniqueVia * Remove UniqueBroadcastEvent, use event instead * clean up logic * formatting * formatting Co-authored-by: Taylor Otwell <taylor@laravel.com>
Ken-vdE · Aug 9, 2022 · b76b904 · b76b904
1 parent 08c955d
commit b76b904
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 71 deletions.
diff --git a/src/Illuminate/Broadcasting/BroadcastManager.php b/src/Illuminate/Broadcasting/BroadcastManager.php
@@ -10,10 +10,12 @@
 use Illuminate\Broadcasting\Broadcasters\NullBroadcaster;
 use Illuminate\Broadcasting\Broadcasters\PusherBroadcaster;
 use Illuminate\Broadcasting\Broadcasters\RedisBroadcaster;
+use Illuminate\Bus\UniqueLock;
 use Illuminate\Contracts\Broadcasting\Factory as FactoryContract;
 use Illuminate\Contracts\Broadcasting\ShouldBeUnique;
 use Illuminate\Contracts\Broadcasting\ShouldBroadcastNow;
 use Illuminate\Contracts\Bus\Dispatcher as BusDispatcherContract;
+use Illuminate\Contracts\Cache\Repository as Cache;
 use Illuminate\Contracts\Foundation\CachesRoutes;
 use InvalidArgumentException;
 use Psr\Log\LoggerInterface;
@@ -166,12 +168,32 @@ public function queue($event)
             $queue = $event->queue;
         }
 
-        $this->app->make('queue')->connection($event->connection ?? null)->pushOn(
-            $queue,
-            $event instanceof ShouldBeUnique
-                    ? new UniqueBroadcastEvent(clone $event)
-                    : new BroadcastEvent(clone $event)
-        );
+        if ($this->mustBeUniqueAndCannotAcquireLock($event)) {
+            return;
+        }
+
+        $this->app->make('queue')
+                ->connection($event->connection ?? null)
+                ->pushOn($queue, new BroadcastEvent(clone $event));
+    }
+
+    /**
+     * Determine if the broadcastable event must be unique and determine if we can acquire the necessary lock.
+     *
+     * @param  mixed  $event
+     * @return bool
+     */
+    protected function mustBeUniqueAndCannotAcquireLock($event)
+    {
+        if (! $event instanceof ShouldBeUnique) {
+            return false;
+        }
+
+        return ! (new UniqueLock(
+            method_exists($event, 'uniqueVia')
+                ? $event->uniqueVia()
+                : $this->app->make(Cache::class)
+        ))->acquire($event);
     }
 
     /**

diff --git a/src/Illuminate/Broadcasting/UniqueBroadcastEvent.php b/src/Illuminate/Broadcasting/UniqueBroadcastEvent.php
diff --git a/tests/Integration/Broadcasting/BroadcastManagerTest.php b/tests/Integration/Broadcasting/BroadcastManagerTest.php
@@ -3,7 +3,6 @@
 namespace Illuminate\Tests\Integration\Broadcasting;
 
 use Illuminate\Broadcasting\BroadcastEvent;
-use Illuminate\Broadcasting\UniqueBroadcastEvent;
 use Illuminate\Contracts\Broadcasting\ShouldBeUnique;
 use Illuminate\Contracts\Broadcasting\ShouldBroadcast;
 use Illuminate\Contracts\Broadcasting\ShouldBroadcastNow;
@@ -44,11 +43,11 @@ public function testUniqueEventsCanBeBroadcast()
 
         Broadcast::queue(new TestEventUnique);
 
-        Bus::assertNotDispatched(UniqueBroadcastEvent::class);
-        Queue::assertPushed(UniqueBroadcastEvent::class);
+        Bus::assertNotDispatched(BroadcastEvent::class);
+        Queue::assertPushed(BroadcastEvent::class);
 
-        $lockKey = 'laravel_unique_job:'.UniqueBroadcastEvent::class.TestEventUnique::class;
-        $this->assertTrue($this->app->get(Cache::class)->lock($lockKey, 10)->get());
+        $lockKey = 'laravel_unique_job:'.TestEventUnique::class;
+        $this->assertFalse($this->app->get(Cache::class)->lock($lockKey, 10)->get());
     }
 }