bug #28251 [HttpFoundation] Allow RedisCluster class for RedisSessionHandler (michaelperrin)

This PR was squashed before being merged into the 4.1 branch (closes #28251).

Discussion
----------

[HttpFoundation] Allow RedisCluster class for RedisSessionHandler

| Q             | A
| ------------- | ---
| Branch?       | 4.1 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |

The `RedisSessionHandler` added in Symfony 4.1 was supposed to accept `RedisCluster` instances but that possibility was not in the condition, leading to an `InvalidArgumentException`.

I formatted the object type condition to allow the `RedisCluster` instance value and also added a test for it.

A double space formatting issue has been fixed too (which is included in the same commit, I can change that if needed).

I could not add a test using RedisCluster, as the installed Redis instance on Travis doesn't use clusters.

Commits
-------

d2ecea0b6e [HttpFoundation] Allow RedisCluster class for RedisSessionHandler

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/RedisSessionHandler.php

@@ -33,14 +33,20 @@ class RedisSessionHandler extends AbstractSessionHandler
      * List of available options:
      *  * prefix: The prefix to use for the keys in order to avoid collision on the Redis server.
      *
-     * @param \Redis|\RedisArray|\RedisCluster|\Predis\Client $redis
-     * @param array                                           $options An associative array of options
+     * @param \Redis|\RedisArray|\RedisCluster|\Predis\Client|RedisProxy $redis
+     * @param array                                                      $options An associative array of options
      *
      * @throws \InvalidArgumentException When unsupported client or options are passed
      */
     public function __construct($redis, array $options = array())
     {
-        if (!$redis instanceof \Redis && !$redis instanceof \RedisArray && !$redis instanceof \Predis\Client && !$redis instanceof RedisProxy) {
+        if (
+            !$redis instanceof \Redis &&
+            !$redis instanceof \RedisArray &&
+            !$redis instanceof \RedisCluster &&
+            !$redis instanceof \Predis\Client &&
+            !$redis instanceof RedisProxy
+        ) {
             throw new \InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, \is_object($redis) ? \get_class($redis) : \gettype($redis)));
         }
 

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/AbstractRedisSessionHandlerTestCase.php

@@ -32,11 +32,6 @@ abstract class AbstractRedisSessionHandlerTestCase extends TestCase
      */
     protected $redisClient;
 
-    /**
-     * @var \Redis
-     */
-    protected $validator;
-
     /**
      * @return \Redis|\RedisArray|\RedisCluster|\Predis\Client
      */
@@ -52,9 +47,6 @@ abstract class AbstractRedisSessionHandlerTestCase extends TestCase
 
         $host = getenv('REDIS_HOST') ?: 'localhost';
 
-        $this->validator = new \Redis();
-        $this->validator->connect($host);
-
         $this->redisClient = $this->createRedisClient($host);
         $this->storage = new RedisSessionHandler(
             $this->redisClient,
@@ -154,24 +146,24 @@ abstract class AbstractRedisSessionHandlerTestCase extends TestCase
     protected function setFixture($key, $value, $ttl = null)
     {
         if (null !== $ttl) {
-            $this->validator->setex($key, $ttl, $value);
+            $this->redisClient->setex($key, $ttl, $value);
         } else {
-            $this->validator->set($key, $value);
+            $this->redisClient->set($key, $value);
         }
     }
 
     protected function getFixture($key)
     {
-        return $this->validator->get($key);
+        return $this->redisClient->get($key);
     }
 
     protected function hasFixture($key): bool
     {
-        return $this->validator->exists($key);
+        return $this->redisClient->exists($key);
     }
 
     protected function fixtureTtl($key): int
     {
-        return $this->validator->ttl($key);
+        return $this->redisClient->ttl($key);
     }
 }

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PredisClusterSessionHandlerTest.php

@@ -17,6 +17,6 @@ class PredisClusterSessionHandlerTest extends AbstractRedisSessionHandlerTestCas
 {
     protected function createRedisClient(string $host): Client
     {
-        return  new Client(array(array('host' => $host)));
+        return new Client(array(array('host' => $host)));
     }
 }

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/RedisClusterSessionHandlerTest.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpFoundation\Tests\Session\Storage\Handler;
+
+class RedisClusterSessionHandlerTest extends AbstractRedisSessionHandlerTestCase
+{
+    public static function setupBeforeClass()
+    {
+        if (!class_exists('RedisCluster')) {
+            self::markTestSkipped('The RedisCluster class is required.');
+        }
+
+        if (!$hosts = getenv('REDIS_CLUSTER_HOSTS')) {
+            self::markTestSkipped('REDIS_CLUSTER_HOSTS env var is not defined.');
+        }
+    }
+
+    protected function createRedisClient(string $host): \RedisCluster
+    {
+        return new \RedisCluster(null, explode(' ', getenv('REDIS_CLUSTER_HOSTS')));
+    }
+}