bug #34017 [Messenger] Fix ignored options in redis transport (chalasr)

This PR was merged into the 4.3 branch. Discussion ---------- [Messenger] Fix ignored options in redis transport | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #33425 | License | MIT | Doc PR | - Also fixes redis authentication failure handling (inline with invalid db index handling, borrowed from symfony/cache). /cc @alexander-schranz Commits ------- c83ff94 [Messenger] Fix ignored options in redis transport
symfony · Oct 22, 2019 · 13e15e2 · 13e15e2
2 parents afbbea3 + c83ff94
commit 13e15e2
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 13 deletions.
diff --git a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php
@@ -57,13 +57,8 @@ public function testFromDsn()
     public function testFromDsnWithOptions()
     {
         $this->assertEquals(
-            new Connection(['stream' => 'queue', 'group' => 'group1', 'consumer' => 'consumer1', 'auto_setup' => false], [
-                'host' => 'localhost',
-                'port' => 6379,
-            ], [
-                'serializer' => 2,
-            ]),
-            Connection::fromDsn('redis://localhost/queue/group1/consumer1', ['serializer' => 2, 'auto_setup' => false])
+            Connection::fromDsn('redis://localhost', ['stream' => 'queue', 'group' => 'group1', 'consumer' => 'consumer1', 'auto_setup' => false, 'serializer' => 2]),
+            Connection::fromDsn('redis://localhost/queue/group1/consumer1?serializer=2&auto_setup=0')
         );
     }
 
@@ -99,7 +94,21 @@ public function testAuth()
         $redis = $this->getMockBuilder(\Redis::class)->disableOriginalConstructor()->getMock();
 
         $redis->expects($this->exactly(1))->method('auth')
-            ->with('password');
+            ->with('password')
+            ->willReturn(true);
+
+        Connection::fromDsn('redis://password@localhost/queue', [], $redis);
+    }
+
+    public function testFailedAuth()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Redis connection failed');
+        $redis = $this->getMockBuilder(\Redis::class)->disableOriginalConstructor()->getMock();
+
+        $redis->expects($this->exactly(1))->method('auth')
+            ->with('password')
+            ->willReturn(false);
 
         Connection::fromDsn('redis://password@localhost/queue', [], $redis);
     }

diff --git a/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php b/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php
@@ -20,6 +20,7 @@
  *
  * @author Alexander Schranz <alexander@sulu.io>
  * @author Antoine Bluchet <soyuka@gmail.com>
+ * @author Robin Chalas <robin.chalas@gmail.com>
  *
  * @internal
  * @final
@@ -52,8 +53,8 @@ public function __construct(array $configuration, array $connectionCredentials =
         $this->connection->connect($connectionCredentials['host'] ?? '127.0.0.1', $connectionCredentials['port'] ?? 6379);
         $this->connection->setOption(\Redis::OPT_SERIALIZER, $redisOptions['serializer'] ?? \Redis::SERIALIZER_PHP);
 
-        if (isset($connectionCredentials['auth'])) {
-            $this->connection->auth($connectionCredentials['auth']);
+        if (isset($connectionCredentials['auth']) && !$this->connection->auth($connectionCredentials['auth'])) {
+            throw new InvalidArgumentException(sprintf('Redis connection failed: %s', $redis->getLastError()));
         }
 
         $this->stream = $configuration['stream'] ?? self::DEFAULT_OPTIONS['stream'];
@@ -70,9 +71,9 @@ public static function fromDsn(string $dsn, array $redisOptions = [], \Redis $re
 
         $pathParts = explode('/', $parsedUrl['path'] ?? '');
 
-        $stream = $pathParts[1] ?? null;
-        $group = $pathParts[2] ?? null;
-        $consumer = $pathParts[3] ?? null;
+        $stream = $pathParts[1] ?? $redisOptions['stream'] ?? null;
+        $group = $pathParts[2] ?? $redisOptions['group'] ?? null;
+        $consumer = $pathParts[3] ?? $redisOptions['consumer'] ?? null;
 
         $connectionCredentials = [
             'host' => $parsedUrl['host'] ?? '127.0.0.1',