From eff266fb1f22c450b070ef784326179b824975be Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 18 Apr 2017 22:45:36 -0500 Subject: [PATCH 1/5] option to add a queue 'prefix' to all tube names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit if you have multiple sites on a server that use the same queue driver, there is no way for your queue worker to distinguish which jobs belong to its application, and which jobs belong to other applications. one way around this is to use custom tube names. however this is difficult to ensure custom naming, and doesn’t offer a consistent way to set these. by adding a queue prefix config option, we have one location to set our value, which gets automatically applied when a job is added to the queue, and when it is pulled off the queue. --- src/Illuminate/Contracts/Queue/Queue.php | 15 ++++++++++ src/Illuminate/Queue/BeanstalkdQueue.php | 2 +- .../Queue/Console/ListenCommand.php | 4 ++- src/Illuminate/Queue/Console/WorkCommand.php | 4 ++- src/Illuminate/Queue/Queue.php | 30 +++++++++++++++++++ src/Illuminate/Queue/QueueManager.php | 13 +++++++- src/Illuminate/Queue/RedisQueue.php | 2 +- src/Illuminate/Queue/SqsQueue.php | 2 +- tests/Queue/QueueManagerTest.php | 3 ++ 9 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Contracts/Queue/Queue.php b/src/Illuminate/Contracts/Queue/Queue.php index 03c260003a3c..b29130564cbf 100644 --- a/src/Illuminate/Contracts/Queue/Queue.php +++ b/src/Illuminate/Contracts/Queue/Queue.php @@ -96,4 +96,19 @@ public function getConnectionName(); * @return $this */ public function setConnectionName($name); + + /** + * Set the queue prefix. + * + * @param string $prefix + * @return $this + */ + public function setQueuePrefix($prefix = null); + + /** + * Get the queue prefix. + * + * @return string + */ + public function getQueuePrefix(); } diff --git a/src/Illuminate/Queue/BeanstalkdQueue.php b/src/Illuminate/Queue/BeanstalkdQueue.php index 24dc9c0b4f8f..d6b0761d4c1e 100755 --- a/src/Illuminate/Queue/BeanstalkdQueue.php +++ b/src/Illuminate/Queue/BeanstalkdQueue.php @@ -148,7 +148,7 @@ public function deleteMessage($queue, $id) */ public function getQueue($queue) { - return $queue ?: $this->default; + return $this->getQueuePrefix().($queue ?: $this->default); } /** diff --git a/src/Illuminate/Queue/Console/ListenCommand.php b/src/Illuminate/Queue/Console/ListenCommand.php index 3d6d1b0edc73..70a20e5b020c 100755 --- a/src/Illuminate/Queue/Console/ListenCommand.php +++ b/src/Illuminate/Queue/Console/ListenCommand.php @@ -79,9 +79,11 @@ protected function getQueue($connection) { $connection = $connection ?: $this->laravel['config']['queue.default']; - return $this->input->getOption('queue') ?: $this->laravel['config']->get( + $queue = $this->input->getOption('queue') ?: $this->laravel['config']->get( "queue.connections.{$connection}.queue", 'default' ); + + return $this->laravel['config']->get('queue.prefix', null) . $queue; } /** diff --git a/src/Illuminate/Queue/Console/WorkCommand.php b/src/Illuminate/Queue/Console/WorkCommand.php index c430881d8477..9ba3199b1bad 100644 --- a/src/Illuminate/Queue/Console/WorkCommand.php +++ b/src/Illuminate/Queue/Console/WorkCommand.php @@ -196,9 +196,11 @@ protected function logFailedJob(JobFailed $event) */ protected function getQueue($connection) { - return $this->option('queue') ?: $this->laravel['config']->get( + $queue = $this->option('queue') ?: $this->laravel['config']->get( "queue.connections.{$connection}.queue", 'default' ); + + return $this->laravel['config']->get('queue.prefix', null) . $queue; } /** diff --git a/src/Illuminate/Queue/Queue.php b/src/Illuminate/Queue/Queue.php index c53dcf355392..1901be1c82e3 100755 --- a/src/Illuminate/Queue/Queue.php +++ b/src/Illuminate/Queue/Queue.php @@ -29,6 +29,13 @@ abstract class Queue */ protected $connectionName; + /** + * The queue prefix. + * + * @var string + */ + protected $queuePrefix; + /** * Push a new job onto the queue. * @@ -188,4 +195,27 @@ public function setContainer(Container $container) { $this->container = $container; } + + /** + * Set the queue prefix. + * + * @param string $prefix + * @return $this + */ + public function setQueuePrefix($prefix = null) + { + $this->queuePrefix = $prefix; + + return $this; + } + + /** + * Get the queue prefix. + * + * @return string + */ + public function getQueuePrefix() + { + return $this->queuePrefix; + } } diff --git a/src/Illuminate/Queue/QueueManager.php b/src/Illuminate/Queue/QueueManager.php index b8fcb3c91b0b..3601312ed1f5 100755 --- a/src/Illuminate/Queue/QueueManager.php +++ b/src/Illuminate/Queue/QueueManager.php @@ -152,7 +152,8 @@ protected function resolve($name) return $this->getConnector($config['driver']) ->connect($config) - ->setConnectionName($name); + ->setConnectionName($name) + ->setQueuePrefix($this->getQueuePrefix()); } /** @@ -232,6 +233,16 @@ public function setDefaultDriver($name) $this->app['config']['queue.default'] = $name; } + /** + * Get the name of the queue prefix. + * + * @return string + */ + public function getQueuePrefix() + { + return isset($this->app['config']['queue.prefix']) ? $this->app['config']['queue.prefix'] : null; + } + /** * Get the full name for the given connection. * diff --git a/src/Illuminate/Queue/RedisQueue.php b/src/Illuminate/Queue/RedisQueue.php index 9dad2419fdfd..6cb489b1e4d8 100644 --- a/src/Illuminate/Queue/RedisQueue.php +++ b/src/Illuminate/Queue/RedisQueue.php @@ -256,7 +256,7 @@ protected function getRandomId() */ protected function getQueue($queue) { - return 'queues:'.($queue ?: $this->default); + return 'queues:'.$this->getQueuePrefix().($queue ?: $this->default); } /** diff --git a/src/Illuminate/Queue/SqsQueue.php b/src/Illuminate/Queue/SqsQueue.php index dc85d9771a60..8388fd21606b 100755 --- a/src/Illuminate/Queue/SqsQueue.php +++ b/src/Illuminate/Queue/SqsQueue.php @@ -137,7 +137,7 @@ public function pop($queue = null) */ public function getQueue($queue) { - $queue = $queue ?: $this->default; + $queue = $this->getQueuePrefix().($queue ?: $this->default); return filter_var($queue, FILTER_VALIDATE_URL) === false ? rtrim($this->prefix, '/').'/'.$queue : $queue; diff --git a/tests/Queue/QueueManagerTest.php b/tests/Queue/QueueManagerTest.php index daaac410eccc..69237cd556d2 100755 --- a/tests/Queue/QueueManagerTest.php +++ b/tests/Queue/QueueManagerTest.php @@ -27,6 +27,7 @@ public function testDefaultConnectionCanBeResolved() $connector = m::mock('StdClass'); $queue = m::mock('StdClass'); $queue->shouldReceive('setConnectionName')->once()->with('sync')->andReturnSelf(); + $queue->shouldReceive('setQueuePrefix')->once()->with('')->andReturnSelf(); $connector->shouldReceive('connect')->once()->with(['driver' => 'sync'])->andReturn($queue); $manager->addConnector('sync', function () use ($connector) { return $connector; @@ -50,6 +51,7 @@ public function testOtherConnectionCanBeResolved() $connector = m::mock('StdClass'); $queue = m::mock('StdClass'); $queue->shouldReceive('setConnectionName')->once()->with('foo')->andReturnSelf(); + $queue->shouldReceive('setQueuePrefix')->once()->with('')->andReturnSelf(); $connector->shouldReceive('connect')->once()->with(['driver' => 'bar'])->andReturn($queue); $manager->addConnector('bar', function () use ($connector) { return $connector; @@ -72,6 +74,7 @@ public function testNullConnectionCanBeResolved() $connector = m::mock('StdClass'); $queue = m::mock('StdClass'); $queue->shouldReceive('setConnectionName')->once()->with('null')->andReturnSelf(); + $queue->shouldReceive('setQueuePrefix')->once()->with('')->andReturnSelf(); $connector->shouldReceive('connect')->once()->with(['driver' => 'null'])->andReturn($queue); $manager->addConnector('null', function () use ($connector) { return $connector; From 116cc9517585c0b5b0bc0bce36a9c57aa43d9e1d Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 18 Apr 2017 22:49:50 -0500 Subject: [PATCH 2/5] style fix --- src/Illuminate/Queue/Console/ListenCommand.php | 2 +- src/Illuminate/Queue/Console/WorkCommand.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Queue/Console/ListenCommand.php b/src/Illuminate/Queue/Console/ListenCommand.php index 70a20e5b020c..481a2403a49b 100755 --- a/src/Illuminate/Queue/Console/ListenCommand.php +++ b/src/Illuminate/Queue/Console/ListenCommand.php @@ -83,7 +83,7 @@ protected function getQueue($connection) "queue.connections.{$connection}.queue", 'default' ); - return $this->laravel['config']->get('queue.prefix', null) . $queue; + return $this->laravel['config']->get('queue.prefix', null).$queue; } /** diff --git a/src/Illuminate/Queue/Console/WorkCommand.php b/src/Illuminate/Queue/Console/WorkCommand.php index 9ba3199b1bad..a87744371248 100644 --- a/src/Illuminate/Queue/Console/WorkCommand.php +++ b/src/Illuminate/Queue/Console/WorkCommand.php @@ -200,7 +200,7 @@ protected function getQueue($connection) "queue.connections.{$connection}.queue", 'default' ); - return $this->laravel['config']->get('queue.prefix', null) . $queue; + return $this->laravel['config']->get('queue.prefix', null).$queue; } /** From 17327b7bbe461e7afa6a24436aeeb398cb9c7750 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 19 Apr 2017 10:15:50 -0500 Subject: [PATCH 3/5] add prefix to database queue driver --- src/Illuminate/Queue/DatabaseQueue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Queue/DatabaseQueue.php b/src/Illuminate/Queue/DatabaseQueue.php index 51a30a7df788..897d0bb8f4ea 100644 --- a/src/Illuminate/Queue/DatabaseQueue.php +++ b/src/Illuminate/Queue/DatabaseQueue.php @@ -308,7 +308,7 @@ public function deleteReserved($queue, $id) */ protected function getQueue($queue) { - return $queue ?: $this->default; + return $this->getQueuePrefix().($queue ?: $this->default); } /** From 1c40e8f5289ee46edb8ca1a1c6ff488d53e3d8ec Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 19 Apr 2017 19:44:49 -0500 Subject: [PATCH 4/5] revert new methods on contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it was pointed out to me that we can’t change contracts on 5.4, so we’ll set it back to the original here. these 2 methods can be targeted to 5.5, however. --- src/Illuminate/Contracts/Queue/Queue.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/Illuminate/Contracts/Queue/Queue.php b/src/Illuminate/Contracts/Queue/Queue.php index b29130564cbf..03c260003a3c 100644 --- a/src/Illuminate/Contracts/Queue/Queue.php +++ b/src/Illuminate/Contracts/Queue/Queue.php @@ -96,19 +96,4 @@ public function getConnectionName(); * @return $this */ public function setConnectionName($name); - - /** - * Set the queue prefix. - * - * @param string $prefix - * @return $this - */ - public function setQueuePrefix($prefix = null); - - /** - * Get the queue prefix. - * - * @return string - */ - public function getQueuePrefix(); } From 66401390182fb400caa7d461cd924ce362cbb6a9 Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 20 Apr 2017 10:25:23 -0500 Subject: [PATCH 5/5] use `get` method on config repository to access value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit also needed to update the tests to actually use a config repository rather than just an array, otherwise the tests will fail with a ‘call to member function get() on array’ message --- src/Illuminate/Queue/QueueManager.php | 2 +- tests/Queue/QueueManagerTest.php | 29 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/Illuminate/Queue/QueueManager.php b/src/Illuminate/Queue/QueueManager.php index 3601312ed1f5..2938a655d655 100755 --- a/src/Illuminate/Queue/QueueManager.php +++ b/src/Illuminate/Queue/QueueManager.php @@ -240,7 +240,7 @@ public function setDefaultDriver($name) */ public function getQueuePrefix() { - return isset($this->app['config']['queue.prefix']) ? $this->app['config']['queue.prefix'] : null; + return $this->app['config']->get('queue.prefix'); } /** diff --git a/tests/Queue/QueueManagerTest.php b/tests/Queue/QueueManagerTest.php index 69237cd556d2..6cd35ecb532b 100755 --- a/tests/Queue/QueueManagerTest.php +++ b/tests/Queue/QueueManagerTest.php @@ -4,6 +4,7 @@ use Mockery as m; use PHPUnit\Framework\TestCase; +use Illuminate\Config\Repository; use Illuminate\Queue\QueueManager; class QueueManagerTest extends TestCase @@ -15,11 +16,13 @@ public function tearDown() public function testDefaultConnectionCanBeResolved() { + $config = new Repository([ + 'queue.default' => 'sync', + 'queue.connections.sync' => ['driver' => 'sync'], + ]); + $app = [ - 'config' => [ - 'queue.default' => 'sync', - 'queue.connections.sync' => ['driver' => 'sync'], - ], + 'config' => $config, 'encrypter' => $encrypter = m::mock('Illuminate\Contracts\Encryption\Encrypter'), ]; @@ -39,11 +42,13 @@ public function testDefaultConnectionCanBeResolved() public function testOtherConnectionCanBeResolved() { + $config = new Repository([ + 'queue.default' => 'sync', + 'queue.connections.foo' => ['driver' => 'bar'], + ]); + $app = [ - 'config' => [ - 'queue.default' => 'sync', - 'queue.connections.foo' => ['driver' => 'bar'], - ], + 'config' => $config, 'encrypter' => $encrypter = m::mock('Illuminate\Contracts\Encryption\Encrypter'), ]; @@ -63,10 +68,12 @@ public function testOtherConnectionCanBeResolved() public function testNullConnectionCanBeResolved() { + $config = new Repository([ + 'queue.default' => 'null', + ]); + $app = [ - 'config' => [ - 'queue.default' => 'null', - ], + 'config' => $config, 'encrypter' => $encrypter = m::mock('Illuminate\Contracts\Encryption\Encrypter'), ];