From 2a6713f24e8e5029cebdd5afcc58050405cb0744 Mon Sep 17 00:00:00 2001 From: Luke Kuzmish <42181698+cosmastech@users.noreply.github.com> Date: Tue, 18 Apr 2023 09:42:14 -0400 Subject: [PATCH] [9.x] Release lock for job implementing `ShouldBeUnique` that is dispatched `afterResponse()` (#46806) * failing test * rely on PendingDispatch@afterResponse so that ShouldBeUnique is checked * modifications to failing test * move lock/middleware handling to CallQueuedHandler@handle() * use CallQueuedHandler@handle if job employs InteractsWithQueue * style * revert changes to CallQueuedHandler & PendingDispatch * switch Bus\Dispatcher@dispatchAfterResponse to rely on Dispatcher@dispatchSync() * add `dispatchAfterResponse` test --- src/Illuminate/Bus/Dispatcher.php | 2 +- .../Foundation/Bus/Dispatchable.php | 2 +- .../Integration/Queue/JobDispatchingTest.php | 59 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Bus/Dispatcher.php b/src/Illuminate/Bus/Dispatcher.php index 4dc390e653fb..8ed3a21b7c4f 100644 --- a/src/Illuminate/Bus/Dispatcher.php +++ b/src/Illuminate/Bus/Dispatcher.php @@ -263,7 +263,7 @@ protected function pushCommandToQueue($queue, $command) public function dispatchAfterResponse($command, $handler = null) { $this->container->terminating(function () use ($command, $handler) { - $this->dispatchNow($command, $handler); + $this->dispatchSync($command, $handler); }); } diff --git a/src/Illuminate/Foundation/Bus/Dispatchable.php b/src/Illuminate/Foundation/Bus/Dispatchable.php index 83f19eea5764..ad471bf87fec 100644 --- a/src/Illuminate/Foundation/Bus/Dispatchable.php +++ b/src/Illuminate/Foundation/Bus/Dispatchable.php @@ -96,7 +96,7 @@ public static function dispatchNow(...$arguments) */ public static function dispatchAfterResponse(...$arguments) { - return app(Dispatcher::class)->dispatchAfterResponse(new static(...$arguments)); + return self::dispatch(...$arguments)->afterResponse(); } /** diff --git a/tests/Integration/Queue/JobDispatchingTest.php b/tests/Integration/Queue/JobDispatchingTest.php index 148246208def..f7fe354180a8 100644 --- a/tests/Integration/Queue/JobDispatchingTest.php +++ b/tests/Integration/Queue/JobDispatchingTest.php @@ -3,8 +3,11 @@ namespace Illuminate\Tests\Integration\Queue; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Cache\Repository; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; +use Illuminate\Queue\InteractsWithQueue; use Orchestra\Testbench\TestCase; class JobDispatchingTest extends TestCase @@ -70,6 +73,52 @@ public function testDoesNotDispatchConditionallyWithClosure() $this->assertTrue(Job::$ran); } + + public function testUniqueJobLockIsReleasedForJobDispatchedAfterResponse() + { + // get initial terminatingCallbacks + $terminatingCallbacksReflectionProperty = (new \ReflectionObject($this->app))->getProperty('terminatingCallbacks'); + $terminatingCallbacksReflectionProperty->setAccessible(true); + $startTerminatingCallbacks = $terminatingCallbacksReflectionProperty->getValue($this->app); + + UniqueJob::dispatchAfterResponse('test'); + $this->assertFalse( + $this->getJobLock(UniqueJob::class, 'test') + ); + + $this->app->terminate(); + $this->assertTrue(UniqueJob::$ran); + + $terminatingCallbacksReflectionProperty->setValue($this->app, $startTerminatingCallbacks); + + UniqueJob::$ran = false; + UniqueJob::dispatch('test')->afterResponse(); + $this->app->terminate(); + $this->assertTrue(UniqueJob::$ran); + + // acquire job lock and confirm that job is not dispatched after response + $this->assertTrue( + $this->getJobLock(UniqueJob::class, 'test') + ); + $terminatingCallbacksReflectionProperty->setValue($this->app, $startTerminatingCallbacks); + UniqueJob::$ran = false; + UniqueJob::dispatch('test')->afterResponse(); + $this->app->terminate(); + $this->assertFalse(UniqueJob::$ran); + + // confirm that dispatchAfterResponse also does not run + UniqueJob::dispatchAfterResponse('test'); + $this->app->terminate(); + $this->assertFalse(UniqueJob::$ran); + } + + /** + * Helpers. + */ + private function getJobLock($job, $value = null) + { + return $this->app->get(Repository::class)->lock('laravel_unique_job:'.$job.$value, 10)->get(); + } } class Job implements ShouldQueue @@ -96,3 +145,13 @@ public function replaceValue($value) static::$value = $value; } } + +class UniqueJob extends Job implements ShouldBeUnique +{ + use InteractsWithQueue; + + public function uniqueId() + { + return self::$value; + } +}