Skip to content

Commit 15e2d15

Browse files
committed
fix #49890: ShouldBeUnique behavior for missing models
1 parent e0be50a commit 15e2d15

File tree

5 files changed

+213
-20
lines changed

5 files changed

+213
-20
lines changed

src/Illuminate/Bus/UniqueLock.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ protected function getKey($job)
7070
? $job->uniqueId()
7171
: ($job->uniqueId ?? '');
7272

73-
return 'laravel_unique_job:'.get_class($job).$uniqueId;
73+
$jobName = property_exists($job, 'jobName')
74+
? $job->jobName
75+
: get_class($job);
76+
77+
return 'laravel_unique_job:'.$jobName.$uniqueId;
7478
}
7579
}

src/Illuminate/Queue/CallQueuedHandler.php

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,10 @@
44

55
use Exception;
66
use Illuminate\Bus\Batchable;
7-
use Illuminate\Bus\UniqueLock;
87
use Illuminate\Contracts\Bus\Dispatcher;
9-
use Illuminate\Contracts\Cache\Repository as Cache;
108
use Illuminate\Contracts\Container\Container;
119
use Illuminate\Contracts\Encryption\Encrypter;
1210
use Illuminate\Contracts\Queue\Job;
13-
use Illuminate\Contracts\Queue\ShouldBeUnique;
1411
use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
1512
use Illuminate\Database\Eloquent\ModelNotFoundException;
1613
use Illuminate\Pipeline\Pipeline;
@@ -60,17 +57,19 @@ public function call(Job $job, array $data)
6057
$job, $this->getCommand($data)
6158
);
6259
} catch (ModelNotFoundException $e) {
60+
$this->ensureUniqueJobLockIsReleased($data);
61+
6362
return $this->handleModelNotFound($job, $e);
6463
}
6564

6665
if ($command instanceof ShouldBeUniqueUntilProcessing) {
67-
$this->ensureUniqueJobLockIsReleased($command);
66+
$this->ensureUniqueJobLockIsReleased($data);
6867
}
6968

7069
$this->dispatchThroughMiddleware($job, $command);
7170

7271
if (! $job->isReleased() && ! $command instanceof ShouldBeUniqueUntilProcessing) {
73-
$this->ensureUniqueJobLockIsReleased($command);
72+
$this->ensureUniqueJobLockIsReleased($data);
7473
}
7574

7675
if (! $job->hasFailed() && ! $job->isReleased()) {
@@ -83,6 +82,32 @@ public function call(Job $job, array $data)
8382
}
8483
}
8584

85+
/**
86+
* Get the unserialized object from the given payload.
87+
*
88+
* @param string $key
89+
* @param array $data
90+
* @return mixed
91+
*/
92+
protected function getUnserializedItem(string $key, array $data)
93+
{
94+
if (isset($data[$key])) {
95+
if (
96+
str_starts_with($data[$key], 'O:') ||
97+
$data[$key] == 'N;'
98+
) {
99+
return unserialize($data[$key]);
100+
}
101+
102+
if ($this->container->bound(Encrypter::class)) {
103+
return unserialize($this->container[Encrypter::class]
104+
->decrypt($data[$key]));
105+
}
106+
}
107+
108+
return null;
109+
}
110+
86111
/**
87112
* Get the command from the given payload.
88113
*
@@ -93,17 +118,25 @@ public function call(Job $job, array $data)
93118
*/
94119
protected function getCommand(array $data)
95120
{
96-
if (str_starts_with($data['command'], 'O:')) {
97-
return unserialize($data['command']);
98-
}
99-
100-
if ($this->container->bound(Encrypter::class)) {
101-
return unserialize($this->container[Encrypter::class]->decrypt($data['command']));
121+
$command = $this->getUnserializedItem('command', $data);
122+
if ($command !== null) {
123+
return $command;
102124
}
103125

104126
throw new RuntimeException('Unable to extract job payload.');
105127
}
106128

129+
/**
130+
* Get the unique handler from the given payload.
131+
*
132+
* @param array $data
133+
* @return \Illuminate\Queue\UniqueHandler|null
134+
*/
135+
protected function getUniqueHandler(array $data)
136+
{
137+
return $this->getUnserializedItem('uniqueHandler', $data);
138+
}
139+
107140
/**
108141
* Dispatch the given job / command through its specified middleware.
109142
*
@@ -196,13 +229,14 @@ protected function ensureSuccessfulBatchJobIsRecorded($command)
196229
/**
197230
* Ensure the lock for a unique job is released.
198231
*
199-
* @param mixed $command
232+
* @param array $data
200233
* @return void
201234
*/
202-
protected function ensureUniqueJobLockIsReleased($command)
235+
protected function ensureUniqueJobLockIsReleased($data)
203236
{
204-
if ($command instanceof ShouldBeUnique) {
205-
(new UniqueLock($this->container->make(Cache::class)))->release($command);
237+
$handler = $this->getUniqueHandler($data);
238+
if ($handler !== null) {
239+
$handler->withContainer($this->container)->release();
206240
}
207241
}
208242

@@ -246,7 +280,7 @@ public function failed(array $data, $e, string $uuid)
246280
$command = $this->getCommand($data);
247281

248282
if (! $command instanceof ShouldBeUniqueUntilProcessing) {
249-
$this->ensureUniqueJobLockIsReleased($command);
283+
$this->ensureUniqueJobLockIsReleased($data);
250284
}
251285

252286
if ($command instanceof \__PHP_Incomplete_Class) {

src/Illuminate/Queue/Queue.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ protected function createPayloadArray($job, $queue, $data = '')
139139
*/
140140
protected function createObjectPayload($job, $queue)
141141
{
142+
$handler = UniqueHandler::forJob($job);
143+
142144
$payload = $this->withCreatePayloadHooks($queue, [
143145
'uuid' => (string) Str::uuid(),
144146
'displayName' => $this->getDisplayName($job),
@@ -150,17 +152,27 @@ protected function createObjectPayload($job, $queue)
150152
'timeout' => $job->timeout ?? null,
151153
'retryUntil' => $this->getJobExpiration($job),
152154
'data' => [
155+
'uniqueHandler' => $handler,
153156
'commandName' => $job,
154157
'command' => $job,
155158
],
156159
]);
157160

158-
$command = $this->jobShouldBeEncrypted($job) && $this->container->bound(Encrypter::class)
159-
? $this->container[Encrypter::class]->encrypt(serialize(clone $job))
160-
: serialize(clone $job);
161+
$handler = serialize($handler);
162+
$command = serialize($job);
163+
164+
if (
165+
$this->jobShouldBeEncrypted($job) &&
166+
$this->container->bound(Encrypter::class)
167+
) {
168+
$encrypter = $this->container[Encrypter::class];
169+
$handler = $encrypter->encrypt($handler);
170+
$command = $encrypter->encrypt($command);
171+
}
161172

162173
return array_merge($payload, [
163174
'data' => array_merge($payload['data'], [
175+
'uniqueHandler' => $handler,
164176
'commandName' => get_class($job),
165177
'command' => $command,
166178
]),
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
3+
namespace Illuminate\Queue;
4+
5+
use Illuminate\Bus\UniqueLock;
6+
use Illuminate\Contracts\Cache\Factory as CacheFactory;
7+
use Illuminate\Contracts\Container\Container;
8+
use Illuminate\Contracts\Queue\ShouldBeUnique;
9+
use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
10+
11+
/**
12+
* A helper class to manage the unique ID and cache instance for a job
13+
* base on the data of the job itself.
14+
*/
15+
class UniqueHandler
16+
{
17+
/**
18+
* Original job name.
19+
*
20+
* @var string
21+
*/
22+
public $jobName;
23+
24+
/**
25+
* The unique ID for the job.
26+
*
27+
* @var string|null
28+
*/
29+
public $uniqueId = null;
30+
31+
/**
32+
* Cache connection name for the job.
33+
*
34+
* @var string|null
35+
*/
36+
protected $uniqueVia = null;
37+
38+
/**
39+
* The container instance.
40+
*
41+
* @var \Illuminate\Contracts\Container\Container
42+
*/
43+
protected $container;
44+
45+
/**
46+
* Create a new handler instance.
47+
*
48+
* @param object $job
49+
*/
50+
public function __construct(object $job)
51+
{
52+
$this->jobName = get_class($job);
53+
54+
if (method_exists($job, 'uniqueId')) {
55+
$this->uniqueId = $job->uniqueId();
56+
} elseif (isset($job->uniqueId)) {
57+
$this->uniqueId = $job->uniqueId;
58+
}
59+
60+
if (method_exists($job, 'uniqueVia')) {
61+
$this->uniqueVia = $job->uniqueVia()->getName();
62+
}
63+
}
64+
65+
/**
66+
* Creates a new instance if the job should be unique.
67+
*
68+
* @param object $job
69+
* @return \Illuminate\Queue\UniqueHandler|null
70+
*/
71+
public static function forJob(object $job)
72+
{
73+
if (
74+
$job instanceof ShouldBeUnique ||
75+
$job instanceof ShouldBeUniqueUntilProcessing
76+
) {
77+
return new static($job);
78+
}
79+
80+
return null;
81+
}
82+
83+
/**
84+
* Sets the container instance.
85+
*
86+
* @param \Illuminate\Contracts\Container\Container $container
87+
* @return \Illuminate\Queue\UpdateHandler
88+
*/
89+
public function withContainer(Container $container)
90+
{
91+
$this->container = $container;
92+
93+
return $this;
94+
}
95+
96+
/**
97+
* Returns the cache instance for the job.
98+
*
99+
* @return \Illuminate\Contracts\Cache\Repository
100+
*/
101+
protected function getCacheStore()
102+
{
103+
return $this->container->make(CacheFactory::class)
104+
->store($this->uniqueVia);
105+
}
106+
107+
/**
108+
* Releases the lock for the job.
109+
*
110+
* @return void
111+
*/
112+
public function release()
113+
{
114+
(new UniqueLock($this->getCacheStore()))->release($this);
115+
}
116+
}

tests/Integration/Queue/UniqueJobTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Illuminate\Contracts\Queue\ShouldBeUnique;
99
use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
1010
use Illuminate\Contracts\Queue\ShouldQueue;
11+
use Illuminate\Database\Eloquent\ModelNotFoundException;
1112
use Illuminate\Foundation\Bus\Dispatchable;
1213
use Illuminate\Queue\InteractsWithQueue;
1314
use Illuminate\Support\Facades\Bus;
@@ -129,6 +130,22 @@ public function testLockCanBeReleasedBeforeProcessing()
129130
$this->assertTrue($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get());
130131
}
131132

133+
public function testLockIsReleasedForJobsWithMissingModels()
134+
{
135+
$this->markTestSkippedWhenUsingSyncQueueDriver();
136+
137+
UniqueUntilStartTestJob::$handled = false;
138+
139+
dispatch($job = new UniqueWithModelMissing);
140+
141+
$this->assertFalse($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get());
142+
143+
$this->runQueueWorkerCommand(['--once' => true]);
144+
145+
$this->assertFalse($job::$handled);
146+
$this->assertTrue($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get());
147+
}
148+
132149
protected function getLockKey($job)
133150
{
134151
return 'laravel_unique_job:'.(is_string($job) ? $job : get_class($job));
@@ -184,3 +201,13 @@ class UniqueUntilStartTestJob extends UniqueTestJob implements ShouldBeUniqueUnt
184201
{
185202
public $tries = 2;
186203
}
204+
205+
class UniqueWithModelMissing extends UniqueTestJob implements ShouldQueue, ShouldBeUnique
206+
{
207+
public $deleteWhenMissingModels = true;
208+
209+
public function __wakeup()
210+
{
211+
throw new ModelNotFoundException('test error');
212+
}
213+
}

0 commit comments

Comments
 (0)