From 15e2d15eb60d3513173ffd03f8b2746e4f3d46b1 Mon Sep 17 00:00:00 2001 From: naquad Date: Fri, 23 Feb 2024 12:49:01 +0200 Subject: [PATCH] fix laravel/framework#49890: ShouldBeUnique behavior for missing models --- src/Illuminate/Bus/UniqueLock.php | 6 +- src/Illuminate/Queue/CallQueuedHandler.php | 66 +++++++++--- src/Illuminate/Queue/Queue.php | 18 +++- src/Illuminate/Queue/UniqueHandler.php | 116 +++++++++++++++++++++ tests/Integration/Queue/UniqueJobTest.php | 27 +++++ 5 files changed, 213 insertions(+), 20 deletions(-) create mode 100644 src/Illuminate/Queue/UniqueHandler.php diff --git a/src/Illuminate/Bus/UniqueLock.php b/src/Illuminate/Bus/UniqueLock.php index a4066b77c1c6..c70a7b252a71 100644 --- a/src/Illuminate/Bus/UniqueLock.php +++ b/src/Illuminate/Bus/UniqueLock.php @@ -70,6 +70,10 @@ protected function getKey($job) ? $job->uniqueId() : ($job->uniqueId ?? ''); - return 'laravel_unique_job:'.get_class($job).$uniqueId; + $jobName = property_exists($job, 'jobName') + ? $job->jobName + : get_class($job); + + return 'laravel_unique_job:'.$jobName.$uniqueId; } } diff --git a/src/Illuminate/Queue/CallQueuedHandler.php b/src/Illuminate/Queue/CallQueuedHandler.php index 5bee1d9ebb4c..a030956c6650 100644 --- a/src/Illuminate/Queue/CallQueuedHandler.php +++ b/src/Illuminate/Queue/CallQueuedHandler.php @@ -4,13 +4,10 @@ use Exception; use Illuminate\Bus\Batchable; -use Illuminate\Bus\UniqueLock; use Illuminate\Contracts\Bus\Dispatcher; -use Illuminate\Contracts\Cache\Repository as Cache; use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Encryption\Encrypter; use Illuminate\Contracts\Queue\Job; -use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Pipeline\Pipeline; @@ -60,17 +57,19 @@ public function call(Job $job, array $data) $job, $this->getCommand($data) ); } catch (ModelNotFoundException $e) { + $this->ensureUniqueJobLockIsReleased($data); + return $this->handleModelNotFound($job, $e); } if ($command instanceof ShouldBeUniqueUntilProcessing) { - $this->ensureUniqueJobLockIsReleased($command); + $this->ensureUniqueJobLockIsReleased($data); } $this->dispatchThroughMiddleware($job, $command); if (! $job->isReleased() && ! $command instanceof ShouldBeUniqueUntilProcessing) { - $this->ensureUniqueJobLockIsReleased($command); + $this->ensureUniqueJobLockIsReleased($data); } if (! $job->hasFailed() && ! $job->isReleased()) { @@ -83,6 +82,32 @@ public function call(Job $job, array $data) } } + /** + * Get the unserialized object from the given payload. + * + * @param string $key + * @param array $data + * @return mixed + */ + protected function getUnserializedItem(string $key, array $data) + { + if (isset($data[$key])) { + if ( + str_starts_with($data[$key], 'O:') || + $data[$key] == 'N;' + ) { + return unserialize($data[$key]); + } + + if ($this->container->bound(Encrypter::class)) { + return unserialize($this->container[Encrypter::class] + ->decrypt($data[$key])); + } + } + + return null; + } + /** * Get the command from the given payload. * @@ -93,17 +118,25 @@ public function call(Job $job, array $data) */ protected function getCommand(array $data) { - if (str_starts_with($data['command'], 'O:')) { - return unserialize($data['command']); - } - - if ($this->container->bound(Encrypter::class)) { - return unserialize($this->container[Encrypter::class]->decrypt($data['command'])); + $command = $this->getUnserializedItem('command', $data); + if ($command !== null) { + return $command; } throw new RuntimeException('Unable to extract job payload.'); } + /** + * Get the unique handler from the given payload. + * + * @param array $data + * @return \Illuminate\Queue\UniqueHandler|null + */ + protected function getUniqueHandler(array $data) + { + return $this->getUnserializedItem('uniqueHandler', $data); + } + /** * Dispatch the given job / command through its specified middleware. * @@ -196,13 +229,14 @@ protected function ensureSuccessfulBatchJobIsRecorded($command) /** * Ensure the lock for a unique job is released. * - * @param mixed $command + * @param array $data * @return void */ - protected function ensureUniqueJobLockIsReleased($command) + protected function ensureUniqueJobLockIsReleased($data) { - if ($command instanceof ShouldBeUnique) { - (new UniqueLock($this->container->make(Cache::class)))->release($command); + $handler = $this->getUniqueHandler($data); + if ($handler !== null) { + $handler->withContainer($this->container)->release(); } } @@ -246,7 +280,7 @@ public function failed(array $data, $e, string $uuid) $command = $this->getCommand($data); if (! $command instanceof ShouldBeUniqueUntilProcessing) { - $this->ensureUniqueJobLockIsReleased($command); + $this->ensureUniqueJobLockIsReleased($data); } if ($command instanceof \__PHP_Incomplete_Class) { diff --git a/src/Illuminate/Queue/Queue.php b/src/Illuminate/Queue/Queue.php index 09eb24526311..95cd2448ee65 100755 --- a/src/Illuminate/Queue/Queue.php +++ b/src/Illuminate/Queue/Queue.php @@ -139,6 +139,8 @@ protected function createPayloadArray($job, $queue, $data = '') */ protected function createObjectPayload($job, $queue) { + $handler = UniqueHandler::forJob($job); + $payload = $this->withCreatePayloadHooks($queue, [ 'uuid' => (string) Str::uuid(), 'displayName' => $this->getDisplayName($job), @@ -150,17 +152,27 @@ protected function createObjectPayload($job, $queue) 'timeout' => $job->timeout ?? null, 'retryUntil' => $this->getJobExpiration($job), 'data' => [ + 'uniqueHandler' => $handler, 'commandName' => $job, 'command' => $job, ], ]); - $command = $this->jobShouldBeEncrypted($job) && $this->container->bound(Encrypter::class) - ? $this->container[Encrypter::class]->encrypt(serialize(clone $job)) - : serialize(clone $job); + $handler = serialize($handler); + $command = serialize($job); + + if ( + $this->jobShouldBeEncrypted($job) && + $this->container->bound(Encrypter::class) + ) { + $encrypter = $this->container[Encrypter::class]; + $handler = $encrypter->encrypt($handler); + $command = $encrypter->encrypt($command); + } return array_merge($payload, [ 'data' => array_merge($payload['data'], [ + 'uniqueHandler' => $handler, 'commandName' => get_class($job), 'command' => $command, ]), diff --git a/src/Illuminate/Queue/UniqueHandler.php b/src/Illuminate/Queue/UniqueHandler.php new file mode 100644 index 000000000000..e70896a584a2 --- /dev/null +++ b/src/Illuminate/Queue/UniqueHandler.php @@ -0,0 +1,116 @@ +jobName = get_class($job); + + if (method_exists($job, 'uniqueId')) { + $this->uniqueId = $job->uniqueId(); + } elseif (isset($job->uniqueId)) { + $this->uniqueId = $job->uniqueId; + } + + if (method_exists($job, 'uniqueVia')) { + $this->uniqueVia = $job->uniqueVia()->getName(); + } + } + + /** + * Creates a new instance if the job should be unique. + * + * @param object $job + * @return \Illuminate\Queue\UniqueHandler|null + */ + public static function forJob(object $job) + { + if ( + $job instanceof ShouldBeUnique || + $job instanceof ShouldBeUniqueUntilProcessing + ) { + return new static($job); + } + + return null; + } + + /** + * Sets the container instance. + * + * @param \Illuminate\Contracts\Container\Container $container + * @return \Illuminate\Queue\UpdateHandler + */ + public function withContainer(Container $container) + { + $this->container = $container; + + return $this; + } + + /** + * Returns the cache instance for the job. + * + * @return \Illuminate\Contracts\Cache\Repository + */ + protected function getCacheStore() + { + return $this->container->make(CacheFactory::class) + ->store($this->uniqueVia); + } + + /** + * Releases the lock for the job. + * + * @return void + */ + public function release() + { + (new UniqueLock($this->getCacheStore()))->release($this); + } +} diff --git a/tests/Integration/Queue/UniqueJobTest.php b/tests/Integration/Queue/UniqueJobTest.php index 546e0cc8361a..c9836cda807b 100644 --- a/tests/Integration/Queue/UniqueJobTest.php +++ b/tests/Integration/Queue/UniqueJobTest.php @@ -8,6 +8,7 @@ use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Support\Facades\Bus; @@ -129,6 +130,22 @@ public function testLockCanBeReleasedBeforeProcessing() $this->assertTrue($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get()); } + public function testLockIsReleasedForJobsWithMissingModels() + { + $this->markTestSkippedWhenUsingSyncQueueDriver(); + + UniqueUntilStartTestJob::$handled = false; + + dispatch($job = new UniqueWithModelMissing); + + $this->assertFalse($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get()); + + $this->runQueueWorkerCommand(['--once' => true]); + + $this->assertFalse($job::$handled); + $this->assertTrue($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get()); + } + protected function getLockKey($job) { return 'laravel_unique_job:'.(is_string($job) ? $job : get_class($job)); @@ -184,3 +201,13 @@ class UniqueUntilStartTestJob extends UniqueTestJob implements ShouldBeUniqueUnt { public $tries = 2; } + +class UniqueWithModelMissing extends UniqueTestJob implements ShouldQueue, ShouldBeUnique +{ + public $deleteWhenMissingModels = true; + + public function __wakeup() + { + throw new ModelNotFoundException('test error'); + } +}