Skip to content

Commit

Permalink
Fix unique bug (#39302)
Browse files Browse the repository at this point in the history
* fix translation bug and add test

* refactor unique job queueing determination

* Apply fixes from StyleCI

Co-authored-by: Taylor Otwell <taylorotwell@users.noreply.github.com>
  • Loading branch information
taylorotwell and taylorotwell authored Oct 21, 2021
1 parent cca0c5f commit 4b785c6
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 12 deletions.
48 changes: 48 additions & 0 deletions src/Illuminate/Bus/UniqueLock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

namespace Illuminate\Bus;

use Illuminate\Contracts\Cache\Repository as Cache;

class UniqueLock
{
/**
* The cache repository implementation.
*
* @var \Illuminate\Contracts\Cache\Repository
*/
protected $cache;

/**
* Create a new unique lock manager instance.
*
* @param \Illuminate\Contracts\Cache\Repository $cache
* @return void
*/
public function __construct(Cache $cache)
{
$this->cache = $cache;
}

/**
* Attempt to acquire a lock for the given job.
*
* @param mixed $job
* @return bool
*/
public function acquire($job)
{
$uniqueId = method_exists($job, 'uniqueId')
? $job->uniqueId()
: ($job->uniqueId ?? '');

$cache = method_exists($job, 'uniqueVia')
? $job->uniqueVia()
: $this->cache;

return (bool) $cache->lock(
$key = 'laravel_unique_job:'.get_class($job).$uniqueId,
$job->uniqueFor ?? 0
)->get();
}
}
32 changes: 32 additions & 0 deletions src/Illuminate/Console/Scheduling/Schedule.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@

use Closure;
use DateTimeInterface;
use Illuminate\Bus\UniqueLock;
use Illuminate\Console\Application;
use Illuminate\Container\Container;
use Illuminate\Contracts\Bus\Dispatcher;
use Illuminate\Contracts\Cache\Repository as Cache;
use Illuminate\Contracts\Container\BindingResolutionException;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Queue\CallQueuedClosure;
use Illuminate\Support\ProcessUtils;
Expand Down Expand Up @@ -168,6 +171,35 @@ protected function dispatchToQueue($job, $queue, $connection)
$job = CallQueuedClosure::create($job);
}

if ($job instanceof ShouldBeUnique) {
return $this->dispatchUniqueJobToQueue($job, $queue, $connection);
}

$this->getDispatcher()->dispatch(
$job->onConnection($connection)->onQueue($queue)
);
}

/**
* Dispatch the given unique job to the queue.
*
* @param object $job
* @param string|null $queue
* @param string|null $connection
* @return void
*
* @throws \RuntimeException
*/
protected function dispatchUniqueJobToQueue($job, $queue, $connection)
{
if (! Container::getInstance()->bound(Cache::class)) {
throw new RuntimeException('Cache driver not available. Scheduling unique jobs not supported.');
}

if (! (new UniqueLock(Container::getInstance()->make(Cache::class)))->acquire($job)) {
return;
}

$this->getDispatcher()->dispatch(
$job->onConnection($connection)->onQueue($queue)
);
Expand Down
15 changes: 3 additions & 12 deletions src/Illuminate/Foundation/Bus/PendingDispatch.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Foundation\Bus;

use Illuminate\Bus\UniqueLock;
use Illuminate\Container\Container;
use Illuminate\Contracts\Bus\Dispatcher;
use Illuminate\Contracts\Cache\Repository as Cache;
Expand Down Expand Up @@ -159,18 +160,8 @@ protected function shouldDispatch()
return true;
}

$uniqueId = method_exists($this->job, 'uniqueId')
? $this->job->uniqueId()
: ($this->job->uniqueId ?? '');

$cache = method_exists($this->job, 'uniqueVia')
? $this->job->uniqueVia()
: Container::getInstance()->make(Cache::class);

return (bool) $cache->lock(
$key = 'laravel_unique_job:'.get_class($this->job).$uniqueId,
$this->job->uniqueFor ?? 0
)->get();
return (new UniqueLock(Container::getInstance()->make(Cache::class)))
->acquire($this->job);
}

/**
Expand Down
63 changes: 63 additions & 0 deletions tests/Integration/Console/UniqueJobSchedulingTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace Illuminate\Tests\Integration\Console;

use Illuminate\Bus\Queueable;
use Illuminate\Console\Scheduling\Schedule;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Support\Facades\Queue;
use Orchestra\Testbench\TestCase;

class UniqueJobSchedulingTest extends TestCase
{
public function testJobsPushedToQueue()
{
Queue::fake();
$this->dispatch(
TestJob::class,
TestJob::class,
TestJob::class,
TestJob::class
);

Queue::assertPushed(TestJob::class, 4);
}

public function testUniqueJobsPushedToQueue()
{
Queue::fake();
$this->dispatch(
UniqueTestJob::class,
UniqueTestJob::class,
UniqueTestJob::class,
UniqueTestJob::class
);

Queue::assertPushed(UniqueTestJob::class, 1);
}

private function dispatch(...$jobs)
{
/** @var \Illuminate\Console\Scheduling\Schedule $scheduler */
$scheduler = $this->app->make(Schedule::class);
foreach ($jobs as $job) {
$scheduler->job($job)->name('')->everyMinute();
}
$events = $scheduler->events();
foreach ($events as $event) {
$event->run($this->app);
}
}
}

class TestJob implements ShouldQueue
{
use InteractsWithQueue, Queueable, Dispatchable;
}

class UniqueTestJob extends TestJob implements ShouldBeUnique
{
}

0 comments on commit 4b785c6

Please sign in to comment.