draft: fix: Don't release lock for ShouldBeUniqueUntilProcessing Job …

…that gets released (#54261) * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * Apply fixes from StyleCI * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * Apply fixes from StyleCI * fix: Don't release lock for ShouldBeUniqueUntilProcessing Job that gets released * Apply fixes from StyleCI --------- Co-authored-by: StyleCI Bot <[email protected]>
laravel · Jan 22, 2025 · c2c35e9 · c2c35e9
1 parent 787cf26
commit c2c35e9
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 4 deletions.
diff --git a/src/Illuminate/Queue/CallQueuedHandler.php b/src/Illuminate/Queue/CallQueuedHandler.php
@@ -66,10 +66,6 @@ public function call(Job $job, array $data)
             return $this->handleModelNotFound($job, $e);
         }
 
-        if ($command instanceof ShouldBeUniqueUntilProcessing) {
-            $this->ensureUniqueJobLockIsReleased($command);
-        }
-
         $this->dispatchThroughMiddleware($job, $command);
 
         if (! $job->isReleased() && ! $command instanceof ShouldBeUniqueUntilProcessing) {
@@ -123,6 +119,10 @@ protected function dispatchThroughMiddleware(Job $job, $command)
         return (new Pipeline($this->container))->send($command)
                 ->through(array_merge(method_exists($command, 'middleware') ? $command->middleware() : [], $command->middleware ?? []))
                 ->then(function ($command) use ($job) {
+                    if ($command instanceof ShouldBeUniqueUntilProcessing) {
+                        $this->ensureUniqueJobLockIsReleased($command);
+                    }
+
                     return $this->dispatcher->dispatchNow(
                         $command, $this->resolveHandler($job, $command)
                     );

diff --git a/tests/Integration/Queue/UniqueUntilProcessingJobTest.php b/tests/Integration/Queue/UniqueUntilProcessingJobTest.php
@@ -0,0 +1,89 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Queue;
+
+use Illuminate\Bus\Queueable;
+use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
+use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Foundation\Bus\Dispatchable;
+use Illuminate\Queue\InteractsWithQueue;
+use Illuminate\Support\Facades\DB;
+use Orchestra\Testbench\Attributes\WithMigration;
+
+#[WithMigration]
+#[WithMigration('cache')]
+#[WithMigration('queue')]
+class UniqueUntilProcessingJobTest extends QueueTestCase
+{
+    protected function defineEnvironment($app)
+    {
+        parent::defineEnvironment($app);
+        $app['config']->set('queue.default', 'database');
+        $app['config']->set('cache.default', 'database');
+        $this->driver = 'database';
+    }
+
+    public function testShouldBeUniqueUntilProcessingReleasesLockWhenJobIsReleasedByAMiddleware()
+    {
+        // Job that does not release and gets processed
+        UniqueTestJobThatDoesNotRelease::dispatch();
+        $lockKey = DB::table('cache_locks')->orderBy('id')->first()->key;
+        $this->assertNotNull($lockKey);
+        $this->runQueueWorkerCommand(['--once' => true]);
+        $this->assertFalse(UniqueTestJobThatDoesNotRelease::$released);
+        $lockKey = DB::table('cache_locks')->first()->key ?? null;
+        $this->assertNull($lockKey);
+        $this->assertDatabaseCount('jobs', 0);
+
+        // Job that releases and does not get processed
+        UniqueUntilProcessingJobThatReleases::dispatch();
+        $lockKey = DB::table('cache_locks')->first()->key;
+        $this->assertNotNull($lockKey);
+        $this->runQueueWorkerCommand(['--once' => true]);
+        $this->assertFalse(UniqueUntilProcessingJobThatReleases::$handled);
+        $this->assertTrue(UniqueUntilProcessingJobThatReleases::$released);
+        $lockKey = DB::table('cache_locks')->orderBy('id')->first()->key ?? null;
+        $this->assertNotNull($lockKey);
+
+        UniqueUntilProcessingJobThatReleases::dispatch();
+        $this->assertDatabaseCount('jobs', 1);
+    }
+}
+
+class UniqueTestJobThatDoesNotRelease implements ShouldQueue, ShouldBeUniqueUntilProcessing
+{
+    use InteractsWithQueue, Queueable, Dispatchable;
+
+    public static $handled = false;
+    public static $released = false;
+
+    public function __construct()
+    {
+        static::$handled = false;
+        static::$released = false;
+    }
+
+    public function handle()
+    {
+        static::$handled = true;
+    }
+}
+
+class UniqueUntilProcessingJobThatReleases extends UniqueTestJobThatDoesNotRelease
+{
+    public function middleware()
+    {
+        return [
+            function ($job) {
+                static::$released = true;
+
+                return $job->release(30);
+            },
+        ];
+    }
+
+    public function uniqueId()
+    {
+        return 100;
+    }
+}