feature #24887 [Cache][Lock] Add RedisProxy for lazy Redis connection…

…s (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [Cache][Lock] Add RedisProxy for lazy Redis connections | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24865 | License | MIT | Doc PR | - That's the only provider that is not lazy by default, leading to bad DX (see linked issue.) Best reviewed [ignoring whitespaces](https://github.com/symfony/symfony/pull/24887/files?w=1). Commits ------- 1f5e353 [Cache][Lock] Add RedisProxy for lazy Redis connections
symfony · Nov 10, 2017 · bf52031 · bf52031
2 parents b7928c3 + 1f5e353
commit bf52031
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 26 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php
@@ -134,7 +134,7 @@ public static function getServiceProvider(ContainerBuilder $container, $name)
                 $definition = new Definition(AbstractAdapter::class);
                 $definition->setPublic(false);
                 $definition->setFactory(array(AbstractAdapter::class, 'createConnection'));
-                $definition->setArguments(array($dsn));
+                $definition->setArguments(array($dsn, array('lazy' => true)));
                 $container->setDefinition($name, $definition);
             }
         }

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -1547,7 +1547,7 @@ private function registerLockConfiguration(array $config, ContainerBuilder $cont
                             $connectionDefinition = new Definition(\stdClass::class);
                             $connectionDefinition->setPublic(false);
                             $connectionDefinition->setFactory(array(AbstractAdapter::class, 'createConnection'));
-                            $connectionDefinition->setArguments(array($storeDsn));
+                            $connectionDefinition->setArguments(array($storeDsn, array('lazy' => true)));
                             $container->setDefinition($connectionDefinitionId, $connectionDefinition);
                         }
 

diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PredisAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PredisAdapterTest.php
@@ -44,6 +44,7 @@ public function testCreateConnection()
             'persistent_id' => null,
             'read_timeout' => 0,
             'retry_interval' => 0,
+            'lazy' => false,
             'database' => '1',
             'password' => null,
         );

diff --git a/src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterTest.php
@@ -13,13 +13,22 @@
 
 use Symfony\Component\Cache\Adapter\AbstractAdapter;
 use Symfony\Component\Cache\Adapter\RedisAdapter;
+use Symfony\Component\Cache\Traits\RedisProxy;
 
 class RedisAdapterTest extends AbstractRedisAdapterTest
 {
     public static function setupBeforeClass()
     {
         parent::setupBeforeClass();
-        self::$redis = AbstractAdapter::createConnection('redis://'.getenv('REDIS_HOST'));
+        self::$redis = AbstractAdapter::createConnection('redis://'.getenv('REDIS_HOST'), array('lazy' => true));
+    }
+
+    public function createCachePool($defaultLifetime = 0)
+    {
+        $adapter = parent::createCachePool($defaultLifetime);
+        $this->assertInstanceOf(RedisProxy::class, self::$redis);
+
+        return $adapter;
     }
 
     public function testCreateConnection()

diff --git a/src/Symfony/Component/Cache/Traits/RedisProxy.php b/src/Symfony/Component/Cache/Traits/RedisProxy.php
@@ -0,0 +1,65 @@
+<?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\Cache\Traits;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ *
+ * @internal
+ */
+class RedisProxy
+{
+    private $redis;
+    private $initializer;
+    private $ready = false;
+
+    public function __construct(\Redis $redis, \Closure $initializer)
+    {
+        $this->redis = $redis;
+        $this->initializer = $initializer;
+    }
+
+    public function __call($method, array $args)
+    {
+        $this->ready ?: $this->ready = $this->initializer->__invoke($this->redis);
+
+        return \call_user_func_array(array($this->redis, $method), $args);
+    }
+
+    public function hscan($strKey, &$iIterator, $strPattern = null, $iCount = null)
+    {
+        $this->ready ?: $this->ready = $this->initializer->__invoke($this->redis);
+
+        return $this->redis->hscan($strKey, $iIterator, $strPattern, $iCount);
+    }
+
+    public function scan(&$iIterator, $strPattern = null, $iCount = null)
+    {
+        $this->ready ?: $this->ready = $this->initializer->__invoke($this->redis);
+
+        return $this->redis->scan($iIterator, $strPattern, $iCount);
+    }
+
+    public function sscan($strKey, &$iIterator, $strPattern = null, $iCount = null)
+    {
+        $this->ready ?: $this->ready = $this->initializer->__invoke($this->redis);
+
+        return $this->redis->sscan($strKey, $iIterator, $strPattern, $iCount);
+    }
+
+    public function zscan($strKey, &$iIterator, $strPattern = null, $iCount = null)
+    {
+        $this->ready ?: $this->ready = $this->initializer->__invoke($this->redis);
+
+        return $this->redis->zscan($strKey, $iIterator, $strPattern, $iCount);
+    }
+}
diff --git a/src/Symfony/Component/Cache/Traits/RedisTrait.php b/src/Symfony/Component/Cache/Traits/RedisTrait.php
@@ -34,6 +34,7 @@ trait RedisTrait
         'timeout' => 30,
         'read_timeout' => 0,
         'retry_interval' => 0,
+        'lazy' => false,
     );
     private $redis;
 
@@ -49,7 +50,7 @@ public function init($redisClient, $namespace = '', $defaultLifetime = 0)
         }
         if ($redisClient instanceof \RedisCluster) {
             $this->enableVersioning();
-        } elseif (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \Predis\Client) {
+        } elseif (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) {
             throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, is_object($redisClient) ? get_class($redisClient) : gettype($redisClient)));
         }
         $this->redis = $redisClient;
@@ -117,19 +118,30 @@ public static function createConnection($dsn, array $options = array())
         if (is_a($class, \Redis::class, true)) {
             $connect = $params['persistent'] || $params['persistent_id'] ? 'pconnect' : 'connect';
             $redis = new $class();
-            @$redis->{$connect}($params['host'], $params['port'], $params['timeout'], $params['persistent_id'], $params['retry_interval']);
 
-            if (@!$redis->isConnected()) {
-                $e = ($e = error_get_last()) && preg_match('/^Redis::p?connect\(\): (.*)/', $e['message'], $e) ? sprintf(' (%s)', $e[1]) : '';
-                throw new InvalidArgumentException(sprintf('Redis connection failed%s: %s', $e, $dsn));
-            }
+            $initializer = function ($redis) use ($connect, $params, $dsn, $auth) {
+                @$redis->{$connect}($params['host'], $params['port'], $params['timeout'], $params['persistent_id'], $params['retry_interval']);
+
+                if (@!$redis->isConnected()) {
+                    $e = ($e = error_get_last()) && preg_match('/^Redis::p?connect\(\): (.*)/', $e['message'], $e) ? sprintf(' (%s)', $e[1]) : '';
+                    throw new InvalidArgumentException(sprintf('Redis connection failed%s: %s', $e, $dsn));
+                }
+
+                if ((null !== $auth && !$redis->auth($auth))
+                    || ($params['dbindex'] && !$redis->select($params['dbindex']))
+                    || ($params['read_timeout'] && !$redis->setOption(\Redis::OPT_READ_TIMEOUT, $params['read_timeout']))
+                ) {
+                    $e = preg_replace('/^ERR /', '', $redis->getLastError());
+                    throw new InvalidArgumentException(sprintf('Redis connection failed (%s): %s', $e, $dsn));
+                }
+
+                return true;
+            };
 
-            if ((null !== $auth && !$redis->auth($auth))
-                || ($params['dbindex'] && !$redis->select($params['dbindex']))
-                || ($params['read_timeout'] && !$redis->setOption(\Redis::OPT_READ_TIMEOUT, $params['read_timeout']))
-            ) {
-                $e = preg_replace('/^ERR /', '', $redis->getLastError());
-                throw new InvalidArgumentException(sprintf('Redis connection failed (%s): %s', $e, $dsn));
+            if ($params['lazy']) {
+                $redis = new RedisProxy($redis, $initializer);
+            } else {
+                $initializer($redis);
             }
         } elseif (is_a($class, \Predis\Client::class, true)) {
             $params['scheme'] = $scheme;

diff --git a/src/Symfony/Component/Lock/Store/RedisStore.php b/src/Symfony/Component/Lock/Store/RedisStore.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Lock\Store;
 
+use Symfony\Component\Cache\Traits\RedisProxy;
 use Symfony\Component\Lock\Exception\InvalidArgumentException;
 use Symfony\Component\Lock\Exception\LockConflictedException;
 use Symfony\Component\Lock\Exception\LockExpiredException;
@@ -24,14 +25,6 @@
  */
 class RedisStore implements StoreInterface
 {
-    private static $defaultConnectionOptions = array(
-        'class' => null,
-        'persistent' => 0,
-        'persistent_id' => null,
-        'timeout' => 30,
-        'read_timeout' => 0,
-        'retry_interval' => 0,
-    );
     private $redis;
     private $initialTtl;
 
@@ -41,7 +34,7 @@ class RedisStore implements StoreInterface
      */
     public function __construct($redisClient, $initialTtl = 300.0)
     {
-        if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\Client) {
+        if (!$redisClient instanceof \Redis && !$redisClient instanceof \RedisArray && !$redisClient instanceof \RedisCluster && !$redisClient instanceof \Predis\Client && !$redisClient instanceof RedisProxy) {
             throw new InvalidArgumentException(sprintf('%s() expects parameter 1 to be Redis, RedisArray, RedisCluster or Predis\Client, %s given', __METHOD__, is_object($redisClient) ? get_class($redisClient) : gettype($redisClient)));
         }
 
@@ -139,7 +132,7 @@ public function exists(Key $key)
      */
     private function evaluate($script, $resource, array $args)
     {
-        if ($this->redis instanceof \Redis || $this->redis instanceof \RedisCluster) {
+        if ($this->redis instanceof \Redis || $this->redis instanceof \RedisCluster || $this->redis instanceof RedisProxy) {
             return $this->redis->eval($script, array_merge(array($resource), $args), 1);
         }
 

diff --git a/src/Symfony/Component/Lock/Store/StoreFactory.php b/src/Symfony/Component/Lock/Store/StoreFactory.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Lock\Store;
 
+use Symfony\Component\Cache\Traits\RedisProxy;
 use Symfony\Component\Lock\Exception\InvalidArgumentException;
 
 /**
@@ -27,7 +28,7 @@ class StoreFactory
      */
     public static function createStore($connection)
     {
-        if ($connection instanceof \Redis || $connection instanceof \RedisArray || $connection instanceof \RedisCluster || $connection instanceof \Predis\Client) {
+        if ($connection instanceof \Redis || $connection instanceof \RedisArray || $connection instanceof \RedisCluster || $connection instanceof \Predis\Client || $connection instanceof RedisProxy) {
             return new RedisStore($connection);
         }
         if ($connection instanceof \Memcached) {