Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Move ShouldBeUnique locking from PendingDispatch to Dispatcher #50381

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions src/Illuminate/Bus/Dispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

use Closure;
use Illuminate\Contracts\Bus\QueueingDispatcher;
use Illuminate\Contracts\Cache\Repository as Cache;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Queue\Queue;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\PendingChain;
use Illuminate\Pipeline\Pipeline;
Expand Down Expand Up @@ -85,14 +87,19 @@ public function dispatch($command)
*
* @param mixed $command
* @param mixed $handler
* @param bool $checkLocks
* @return mixed
*/
public function dispatchSync($command, $handler = null)
public function dispatchSync($command, $handler = null, $checkLocks = true)
{
if ($this->queueResolver &&
$this->commandShouldBeQueued($command) &&
method_exists($command, 'onConnection')) {
return $this->dispatchToQueue($command->onConnection('sync'));
return $this->dispatchToQueue($command->onConnection('sync'), $checkLocks);
}

if($checkLocks && ! $this->shouldDispatch($command)) {
return null;
}

return $this->dispatchNow($command, $handler);
Expand Down Expand Up @@ -194,6 +201,27 @@ public function getCommandHandler($command)
return false;
}

/**
* Determine if the job should be dispatched.
*
* @param mixed $command
* @param bool $shouldLock
* @return bool
*/
protected function shouldDispatch($command, $shouldLock = true)
{
if (! $command instanceof ShouldBeUnique) {
return true;
}

$lock = (new UniqueLock(\Illuminate\Container\Container::getInstance()->make(Cache::class)));
if ($shouldLock) {
return $lock->acquire($command);
} else {
return ! $lock->isAcquired($command);
}
}

/**
* Determine if the given command should be queued.
*
Expand All @@ -209,12 +237,17 @@ protected function commandShouldBeQueued($command)
* Dispatch a command to its appropriate handler behind a queue.
*
* @param mixed $command
* @param bool $checkLocks
* @return mixed
*
* @throws \RuntimeException
*/
public function dispatchToQueue($command)
public function dispatchToQueue($command, $checkLocks = true)
{
if($checkLocks && ! $this->shouldDispatch($command)) {
return null;
}

$connection = $command->connection ?? null;

$queue = call_user_func($this->queueResolver, $connection);
Expand Down Expand Up @@ -263,8 +296,12 @@ protected function pushCommandToQueue($queue, $command)
*/
public function dispatchAfterResponse($command, $handler = null)
{
if(! $this->shouldDispatch($command)) {
return;
}

$this->container->terminating(function () use ($command, $handler) {
$this->dispatchSync($command, $handler);
$this->dispatchSync($command, $handler, false);
});
}

Expand Down
15 changes: 15 additions & 0 deletions src/Illuminate/Bus/UniqueLock.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ public function acquire($job)
return (bool) $cache->lock($this->getKey($job), $uniqueFor)->get();
}

/**
* Check if the lock for the given job is already acquired.
*
* @param mixed $job
* @return bool
*/
public function isAcquired($job)
{
$cache = method_exists($job, 'uniqueVia')
? $job->uniqueVia()
: $this->cache;

return (bool) $cache->lock($this->getKey($job))->get();
}

/**
* Release the lock for the given job.
*
Expand Down
4 changes: 0 additions & 4 deletions src/Illuminate/Console/Scheduling/Schedule.php
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,6 @@ protected function dispatchUniqueJobToQueue($job, $queue, $connection)
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
23 changes: 1 addition & 22 deletions src/Illuminate/Foundation/Bus/PendingDispatch.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +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;
use Illuminate\Contracts\Queue\ShouldBeUnique;

class PendingDispatch
{
Expand Down Expand Up @@ -149,21 +145,6 @@ public function afterResponse()
return $this;
}

/**
* Determine if the job should be dispatched.
*
* @return bool
*/
protected function shouldDispatch()
{
if (! $this->job instanceof ShouldBeUnique) {
return true;
}

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

/**
* Dynamically proxy methods to the underlying job.
*
Expand All @@ -185,9 +166,7 @@ public function __call($method, $parameters)
*/
public function __destruct()
{
if (! $this->shouldDispatch()) {
return;
} elseif ($this->afterResponse) {
if ($this->afterResponse) {
app(Dispatcher::class)->dispatchAfterResponse($this->job);
} else {
app(Dispatcher::class)->dispatch($this->job);
Expand Down
Loading