8000 [Bugfix] Handle model not found exception in queue listener · BrianLangevin/laravel-json-api@8eb4321 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8eb4321

Browse files
lindyhopchrislindyhopchris
committed
[Bugfix] Handle model not found exception in queue listener
Closes cloudcreativity#358
1 parent f39dfc1 commit 8eb4321

File tree

7 files changed

+115
-5
lines changed

7 files changed

+115
-5
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file lin 10000 e numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ Update `zend-diactoros` dependency.
1818
Fix using an alternative decoding type for update (`PATCH`) requests.
1919
- [#370](https://github.com/cloudcreativity/laravel-json-api/pull/370)
2020
Fix wrong validation error title when creating a custom validator.
21+
- [#358](https://github.com/cloudcreativity/laravel-json-api/issues/358)
22+
Queue listener could trigger a `ModelNotFoundException` when deserializing a job that had deleted a
23+
model during its `handle()` method.
2124

2225
## [1.1.0] - 2019-04-12
2326

‎docs/features/async.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,43 @@ class ProcessPodcast implements ShouldQueue
217217
}
218218
```
219219

220+
## Manually Marking Client Jobs as Complete
221+
222+
This package will, in most cases, automatically mark the stored representation of the job as complete.
223+
We do this by listening the Laravel's queue events.
224+
225+
There is one scenario where we cannot do this: if your job deletes a model during its `handle` method.
226+
This is because we cannot deserialize the job in our listener without causing a `ModelNotFoundException`.
227+
228+
In these scenarios, you will need to manually mark the stored representation of the job as complete.
229+
Use the `didComplete()` method, which accepts one argument: a boolean indicating success (will be
230+
`true` if not provided).
231+
232+
For example:
233+
234+
```php
235+
namespace App\Jobs;
236+
237+
use CloudCreativity\LaravelJsonApi\Queue\ClientDispatchable;
238+
use Illuminate\Contracts\Queue\ShouldQueue;
239+
240+
class RemovePodcast implements ShouldQueue
241+
{
242+
243+
use ClientDispatchable;
244+
245+
// ...
246+
247+
public function handle()
248+
{
249+
// ...logic to remove a podcast.
250+
251+
$this->podcast->delete();
252+
$this->didComplete();
253+
}
254+
}
255+
```
256+
220257
## Routing
221258

222259
The final step of setup is to enable asynchronous process routes on a resource. These

‎src/Queue/ClientDispatchable.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,25 @@ public function didCreate($resource): void
9494
$this->clientJob->setResource($resource)->save();
9595
}
9696
}
97+
98+
/**
99+
* Mark the client job as completed.
100+
*
101+
* Although our queue listeners handle this for you in most cases, there
102+
* are some scenarios where it is not possible to do this. E.g. if your
103+
* job deletes a model that is one of its properties, a `ModelNotFoundException`
104+
* will be triggered when our listener deserializes the job.
105+
*
106+
* Therefore this method is provided so that you can manually mark the
107+
* client job as completed, if needed.
108+
*
109+
* @param bool $success
110+
* @return void
111+
*/
112+
public function didComplete(bool $success = true): void
113+
{
114+
if ($this->wasClientDispatched()) {
115+
$this->clientJob->completed($success);
116+
}
117+
}
97118
}

‎src/Queue/ClientJob.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,19 @@ public function processed($job): void
168168
]);
169169
}
170170

171+
/**
172+
* @param bool $success
173+
* @return void
174+
*/
175+
public function completed(bool $success = true): void
176+
{
177+
$this->update([
178+
'attempts' => $this->attempts + 1,
179+
'completed_at' => Carbon::now(),
180+
'failed' => !$success,
181+
]);
182+
}
183+
171184
/**
172185
* @return Api
173186
*/

‎src/Queue/UpdateClientProcess.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
use CloudCreativity\LaravelJsonApi\Contracts\Queue\AsynchronousProcess;
2121
use Illuminate\Contracts\Queue\Job;
22+
use Illuminate\Database\Eloquent\ModelNotFoundException;
2223
use Illuminate\Queue\Events\JobFailed;
2324
use Illuminate\Queue\Events\JobProcessed;
2425

@@ -55,7 +56,15 @@ private function deserialize(Job $job)
5556
$data = $this->payload($job)['data'] ?? [];
5657
$command = $data['command'] ?? null;
5758

58-
return is_string($command) ? unserialize($command) : null;
59+
if (!is_string($command)) {
60+
return null;
61+
}
62+
63+
try {
64< F438 /td>+
return unserialize($command) ?: null;
65+
} catch (ModelNotFoundException $ex) {
66+
return null;
67+
}
5968
}
6069

6170
/**

‎tests/lib/Integration/Queue/QueueEventsTest.php

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ protected function setUp(): void
3434
Carbon::setTestNow('2018-10-23 12:00:00.123456');
3535
}
3636

37-
public function testCompletes()
37+
public function testCompletes(): void
3838
{
3939
$job = new TestJob();
4040
$job->clientJob = factory(ClientJob::class)->create();
@@ -53,7 +53,7 @@ public function testCompletes()
5353
$this->assertInstanceOf(Download::class, $clientJob->getResource());
5454
}
5555

56-
public function testFails()
56+
public function testFails(): void
5757
{
5858
$job = new TestJob();
5959
$job->ex = true;
@@ -73,4 +73,20 @@ public function testFails()
7373
'failed' => true,
7474
]);
7575
}
76+
77+
public function testDoesNotCauseException(): void
78+
{
79+
$job = new TestJob();
80+
$job->model = factory(Download::class)->create();
81+
$job->clientJob = factory(ClientJob::class)->create();
82+
83+
dispatch($job);
84+
85+
$this->assertDatabaseHas('json_api_client_jobs', [
8 EF5E 6+
'uuid' => $job->clientJob->getKey(),
87+
'attempts' => 1,
88+
'completed_at' => Carbon::now()->format('Y-m-d H:i:s'),
89+
'failed' => false,
90+
]);
91+
}
7692
}

‎tests/lib/Integration/Queue/TestJob.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,29 @@ class TestJob implements ShouldQueue
4444
*/
4545
public $tries = 2;
4646

47+
/**
48+
* @var Download|null
49+
*/
50+
public $model;
51+
4752
/**
4853
* Execute the job.
4954
*
50-
* @return Download
55+
* @return Download|null
5156
* @throws \Exception
5257
*/
53-
public function handle(): Download
58+
public function handle(): ?Download
5459
{
5560
if ($this->ex) {
5661
throw new \LogicException('Boom.');
5762
}
5863

64+
if ($this->model) {
65+
$this->model->delete();
66+
$this->didComplete();
67+
return null;
68+
}
69+
5970
$download = factory(Download::class)->create();
6071
$this->didCreate($download);
6172

0 commit comments

Comments
 (0)
0