Merge pull request #62 from cakephp/fix-60

Fix queue parameter being required on non-default queues

Author: markstory

src/Command/WorkerCommand.php

@@ -48,8 +48,7 @@ public function getOptionParser(): ConsoleOptionParser
             'short' => 'c',
         ]);
         $parser->addOption('queue', [
-            'default' => 'default',
-            'help' => 'Name of queue to bind to',
+            'help' => 'Name of queue to bind to. Defaults to the queue config (--config).',
             'short' => 'Q',
         ]);
         $parser->addOption('processor', [
@@ -140,10 +139,14 @@ public function execute(Arguments $args, ConsoleIo $io)
             $processor->getEventManager()->on($listener);
             $extension->getEventManager()->on($listener);
         }
-        $url = Configure::read(sprintf('Queue.%s.url', $config));
+        $url = Configure::read("Queue.{$config}.url");
         $client = new SimpleClient($url, $logger);
-        /** @psalm-suppress InvalidArgument */
-        $client->bindTopic((string)$args->getOption('queue'), $processor, $args->getOption('processor'));
+        $queue = $args->getOption('queue')
+            ? (string)$args->getOption('queue')
+            : Configure::read("Queue.{$config}.queue", 'default');
+        $processorName = $args->getOption('processor') ? (string)$args->getOption('processor') : null;
+
+        $client->bindTopic($queue, $processor, $processorName);
         $client->consume($extension);
     }
 }

tests/TestCase/Command/WorkerCommandTest.php

@@ -146,10 +146,7 @@ public function testQueueProcessesWithLogger()
         ]);
 
         $this->exec('worker --max-runtime=1 --logger=debug --verbose');
-        $log = Log::engine('debug');
-        $this->assertIsArray($log->read());
-        $this->assertNotEmpty($log->read());
-        $this->assertEquals($log->read()[0], 'debug Max Iterations: 0');
+        $this->assertDebugLogContains('debug Max Iterations: 0');
     }
 
     /**
@@ -159,36 +156,30 @@ public function testQueueProcessesWithLogger()
      */
     public function testQueueProcessesJob()
     {
+        $config = [
+            'queue' => 'default',
+            'url' => 'file:///' . TMP . DS . 'queue',
+        ];
         Configure::write([
-            'Queue' => [
-                'default' => [
-                    'queue' => 'default',
-                    'url' => 'file:///' . TMP . DS . 'queue',
-                ],
-            ],
+            'Queue' => ['default' => $config],
         ]);
 
         Log::setConfig('debug', [
             'className' => 'Array',
             'levels' => ['notice', 'info', 'debug'],
         ]);
 
-        $this->exec('worker --max-runtime=3 --logger=debug --verbose');
-
         $callable = [WelcomeMailer::class, 'welcome'];
         $arguments = [];
         $options = ['config' => 'default'];
 
+        QueueManager::setConfig('default', $config);
         QueueManager::push($callable, $arguments, $options);
+        QueueManager::drop('default');
 
-        $log = Log::engine('debug');
-        $this->assertIsArray($log->read());
-        $this->assertNotEmpty($log->read());
-        foreach ($log->read() as $line) {
-            if (stripos($line, 'Welcome mail sent') !== false) {
-                $this->assertTrue(true);
-            }
-        }
+        $this->exec('worker --max-runtime=3 --logger=debug --verbose');
+
+        $this->assertDebugLogContains('Welcome mail sent');
     }
 
     /**
@@ -198,35 +189,74 @@ public function testQueueProcessesJob()
      */
     public function testQueueProcessesJobWithProcessor()
     {
+        $config = [
+            'queue' => 'default',
+            'url' => 'file:///' . TMP . DS . 'queue',
+        ];
         Configure::write([
-            'Queue' => [
-                'default' => [
-                    'queue' => 'default',
-                    'url' => 'file:///' . TMP . DS . 'queue',
-                ],
-            ],
+            'Queue' => ['default' => $config],
         ]);
-
         Log::setConfig('debug', [
             'className' => 'Array',
             'levels' => ['notice', 'info', 'debug'],
         ]);
 
+        $callable = [WelcomeMailer::class, 'welcome'];
+        $arguments = [];
+        $options = ['config' => 'default'];
+
+        QueueManager::setConfig('default', $config);
+        QueueManager::push($callable, $arguments, $options);
+        QueueManager::drop('default');
+
         $this->exec('worker --max-runtime=3 --processor=processor-name --logger=debug --verbose');
 
+        $this->assertDebugLogContains('Welcome mail sent');
+    }
+
+    /**
+     * Test non-default queue name
+     *
+     * @runInSeparateProcess
+     */
+    public function testQueueProcessesJobWithOtherQueue()
+    {
+        $config = [
+            'queue' => 'other',
+            'url' => 'file:///' . TMP . DS . 'queue',
+        ];
+        Configure::write([
+            'Queue' => ['other' => $config],
+        ]);
+
+        Log::setConfig('debug', [
+            'className' => 'Array',
+            'levels' => ['notice', 'info', 'debug'],
+        ]);
+
         $callable = [WelcomeMailer::class, 'welcome'];
         $arguments = [];
-        $options = ['config' => 'default'];
+        $options = ['config' => 'other'];
 
+        QueueManager::setConfig('other', $config);
         QueueManager::push($callable, $arguments, $options);
+        QueueManager::drop('other');
+
+        $this->exec('worker --config=other --max-runtime=3 --processor=processor-name --logger=debug --verbose');
 
+        $this->assertDebugLogContains('Welcome mail sent');
+    }
+
+    protected function assertDebugLogContains($expected): void
+    {
         $log = Log::engine('debug');
-        $this->assertIsArray($log->read());
-        $this->assertNotEmpty($log->read());
+        $found = false;
         foreach ($log->read() as $line) {
-            if (stripos($line, 'Welcome mail sent') !== false) {
-                $this->assertTrue(true);
+            if (strpos($line, $expected) !== false) {
+                $found = true;
+                break;
             }
         }
+        $this->assertTrue($found, "Did not find `{$expected}` in logs.");
     }
 }