Skip to content

Commit

Permalink
[Bugfix] Handle model not found exception in queue listener
Browse files Browse the repository at this point in the history
Closes #358
  • Loading branch information
lindyhopchris committed Jun 18, 2019
1 parent f39dfc1 commit 8eb4321
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ Update `zend-diactoros` dependency.
Fix using an alternative decoding type for update (`PATCH`) requests.
- [#370](https://github.com/cloudcreativity/laravel-json-api/pull/370)
Fix wrong validation error title when creating a custom validator.
- [#358](https://github.com/cloudcreativity/laravel-json-api/issues/358)
Queue listener could trigger a `ModelNotFoundException` when deserializing a job that had deleted a
model during its `handle()` method.

## [1.1.0] - 2019-04-12

Expand Down
37 changes: 37 additions & 0 deletions docs/features/async.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,43 @@ class ProcessPodcast implements ShouldQueue
}
```

## Manually Marking Client Jobs as Complete

This package will, in most cases, automatically mark the stored representation of the job as complete.
We do this by listening the Laravel's queue events.

There is one scenario where we cannot do this: if your job deletes a model during its `handle` method.
This is because we cannot deserialize the job in our listener without causing a `ModelNotFoundException`.

In these scenarios, you will need to manually mark the stored representation of the job as complete.
Use the `didComplete()` method, which accepts one argument: a boolean indicating success (will be
`true` if not provided).

For example:

```php
namespace App\Jobs;

use CloudCreativity\LaravelJsonApi\Queue\ClientDispatchable;
use Illuminate\Contracts\Queue\ShouldQueue;

class RemovePodcast implements ShouldQueue
{

use ClientDispatchable;

// ...

public function handle()
{
// ...logic to remove a podcast.

$this->podcast->delete();
$this->didComplete();
}
}
```

## Routing

The final step of setup is to enable asynchronous process routes on a resource. These
Expand Down
21 changes: 21 additions & 0 deletions src/Queue/ClientDispatchable.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,25 @@ public function didCreate($resource): void
$this->clientJob->setResource($resource)->save();
}
}

/**
* Mark the client job as completed.
*
* Although our queue listeners handle this for you in most cases, there
* are some scenarios where it is not possible to do this. E.g. if your
* job deletes a model that is one of its properties, a `ModelNotFoundException`
* will be triggered when our listener deserializes the job.
*
* Therefore this method is provided so that you can manually mark the
* client job as completed, if needed.
*
* @param bool $success
* @return void
*/
public function didComplete(bool $success = true): void
{
if ($this->wasClientDispatched()) {
$this->clientJob->completed($success);
}
}
}
13 changes: 13 additions & 0 deletions src/Queue/ClientJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,19 @@ public function processed($job): void
]);
}

/**
* @param bool $success
* @return void
*/
public function completed(bool $success = true): void
{
$this->update([
'attempts' => $this->attempts + 1,
'completed_at' => Carbon::now(),
'failed' => !$success,
]);
}

/**
* @return Api
*/
Expand Down
11 changes: 10 additions & 1 deletion src/Queue/UpdateClientProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use CloudCreativity\LaravelJsonApi\Contracts\Queue\AsynchronousProcess;
use Illuminate\Contracts\Queue\Job;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Queue\Events\JobFailed;
use Illuminate\Queue\Events\JobProcessed;

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

return is_string($command) ? unserialize($command) : null;
if (!is_string($command)) {
return null;
}

try {
return unserialize($command) ?: null;
} catch (ModelNotFoundException $ex) {
return null;
}
}

/**
Expand Down
20 changes: 18 additions & 2 deletions tests/lib/Integration/Queue/QueueEventsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected function setUp(): void
Carbon::setTestNow('2018-10-23 12:00:00.123456');
}

public function testCompletes()
public function testCompletes(): void
{
$job = new TestJob();
$job->clientJob = factory(ClientJob::class)->create();
Expand All @@ -53,7 +53,7 @@ public function testCompletes()
$this->assertInstanceOf(Download::class, $clientJob->getResource());
}

public function testFails()
public function testFails(): void
{
$job = new TestJob();
$job->ex = true;
Expand All @@ -73,4 +73,20 @@ public function testFails()
'failed' => true,
]);
}

public function testDoesNotCauseException(): void
{
$job = new TestJob();
$job->model = factory(Download::class)->create();
$job->clientJob = factory(ClientJob::class)->create();

dispatch($job);

$this->assertDatabaseHas('json_api_client_jobs', [
'uuid' => $job->clientJob->getKey(),
'attempts' => 1,
'completed_at' => Carbon::now()->format('Y-m-d H:i:s'),
'failed' => false,
]);
}
}
15 changes: 13 additions & 2 deletions tests/lib/Integration/Queue/TestJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,29 @@ class TestJob implements ShouldQueue
*/
public $tries = 2;

/**
* @var Download|null
*/
public $model;

/**
* Execute the job.
*
* @return Download
* @return Download|null
* @throws \Exception
*/
public function handle(): Download
public function handle(): ?Download
{
if ($this->ex) {
throw new \LogicException('Boom.');
}

if ($this->model) {
$this->model->delete();
$this->didComplete();
return null;
}

$download = factory(Download::class)->create();
$this->didCreate($download);

Expand Down

0 comments on commit 8eb4321

Please sign in to comment.