Make connection_persistent parameter accept a connection id (#739)

Seldaek · web-flow · commit 05bfb149e57d · 2025-06-29T14:30:01.000+02:00
diff --git a/composer.json b/composer.json
@@ -37,7 +37,7 @@
         "monolog/monolog": "*",
         "phpunit/phpunit": "^9.5.28 || ^10",
         "predis/predis": "^2.0 || ^3.0 ",
-        "seec/phpunit-consecutive-params": "dev-master",
+        "seec/phpunit-consecutive-params": "^1.1.4",
         "symfony/browser-kit": "^5.4.20 ||^6.0 || ^7.0",
         "symfony/cache": "^5.4.20 ||^6.0 || ^7.0",
         "symfony/config": "^5.4.20 ||^6.0 || ^7.0",
diff --git a/docs/README.md b/docs/README.md
@@ -171,6 +171,44 @@ snc_redis:
 
 It also works for the Phpredis Cluster mode.
 
+### Persistent Connections ###
+
+When using persistent connections, you can provide a unique identifier to ensure that different clients don't share the same socket connection. This is especially important when operating multiple clients within the same application that communicate with the same Redis server.
+
+The `connection_persistent` option accepts either a boolean or string value:
+- `true`: Enable persistent connections using the client alias as the connection ID
+- `false`: Disable persistent connections (default)
+- `string`: Enable persistent connections using the provided string as the connection ID
+
+``` yaml
+snc_redis:
+    clients:
+        app_cache:
+            type: predis  # or phpredis
+            alias: app_cache
+            dsn: redis://localhost/0
+            options:
+                connection_persistent: "app_cache_connection"  # Custom persistent ID
+        session_store:
+            type: predis  # or phpredis
+            alias: session_store
+            dsn: redis://localhost/1
+            options:
+                connection_persistent: true  # Uses alias as persistent ID
+        simple_cache:
+            type: phpredis
+            alias: simple_cache
+            dsn: redis://localhost/2
+            options:
+                connection_persistent: false  # No persistent connection
+```
+
+**Why this matters:** Without a unique persistent connection ID, two clients connecting to different databases on the same Redis server would share the same persistent socket, leading to unexpected behavior where commands from one client could affect the database context of another.
+
+**For phpredis clients:** The persistent ID is passed directly to the `pconnect()` function as the persistent ID parameter.
+
+**For predis clients:** The persistent ID is mapped to the `conn_uid` connection option (requires predis/predis version 2.4.0 or higher). This ensures each client creates its own socket so the connection context won't be shared across clients.
+
 ### Sessions ###
 
 Use Redis sessions by utilizing Symfony built-in Redis session handler like so:
diff --git a/src/DependencyInjection/Configuration/Configuration.php b/src/DependencyInjection/Configuration/Configuration.php
@@ -29,7 +29,9 @@
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 
 use function class_exists;
+use function is_bool;
 use function is_iterable;
+use function is_string;
 use function trigger_deprecation;
 
 class Configuration implements ConfigurationInterface
@@ -124,7 +126,13 @@ private function addClientsSection(ArrayNodeDefinition $rootNode): void
                                         ->scalarPrototype()->end()
                                     ->end()
                                     ->booleanNode('connection_async')->defaultFalse()->end()
-                                    ->booleanNode('connection_persistent')->defaultFalse()->end()
+                                    ->variableNode('connection_persistent')
+                                        ->defaultFalse()
+                                        ->validate()
+                                            ->ifTrue(static fn ($v) => !(is_bool($v) || (is_string($v) && $v !== '')))
+                                            ->thenInvalid('connection_persistent must be a boolean or string')
+                                        ->end()
+                                    ->end()
                                     ->scalarNode('connection_timeout')->cannotBeEmpty()->defaultValue(5)->end()
                                     ->scalarNode('read_write_timeout')->defaultNull()->end()
                                     ->booleanNode('iterable_multibulk')->defaultFalse()->end()
diff --git a/src/DependencyInjection/SncRedisExtension.php b/src/DependencyInjection/SncRedisExtension.php
@@ -15,6 +15,7 @@
 
 use InvalidArgumentException;
 use LogicException;
+use Predis\Client;
 use RedisSentinel;
 use Relay\Sentinel;
 use Snc\RedisBundle\DependencyInjection\Configuration\Configuration;
@@ -35,7 +36,9 @@
 use function assert;
 use function class_exists;
 use function count;
+use function is_string;
 use function sprintf;
+use function version_compare;
 
 class SncRedisExtension extends Extension
 {
@@ -135,10 +138,28 @@ private function loadPredisClient(array $client, ContainerBuilder $container): v
         // predis connection parameters have been renamed in v0.8
         $client['options']['async_connect'] = $client['options']['connection_async'];
         $client['options']['timeout']       = $client['options']['connection_timeout'];
-        $client['options']['persistent']    = $client['options']['connection_persistent'];
-        $client['options']['exceptions']    = $client['options']['throw_errors'];
+        // Handle connection_persistent as bool|string
+        if (is_string($client['options']['connection_persistent'])) {
+            $client['options']['persistent'] = true;
+            // For predis, use the string value as conn_uid (requires predis >= 2.4.0)
+            if (class_exists('Predis\Client')) {
+                if (!version_compare(Client::VERSION, '2.4.0', '>=')) {
+                    throw new InvalidConfigurationException(
+                        'Using connection_persistent as string for Predis requires predis/predis version 2.4.0 or higher. ' .
+                        sprintf('Current version: %s', Client::VERSION),
+                    );
+                }
+
+                $client['options']['conn_uid'] = $client['options']['connection_persistent'];
+            }
+        } else {
+            $client['options']['persistent'] = $client['options']['connection_persistent'];
+        }
+
+        $client['options']['exceptions'] = $client['options']['throw_errors'];
         // fix ssl configuration key name
         $client['options']['ssl'] = $client['options']['parameters']['ssl_context'] ?? [];
+
         unset($client['options']['connection_async']);
         unset($client['options']['connection_timeout']);
         unset($client['options']['connection_persistent']);
diff --git a/src/Factory/PhpredisClientFactory.php b/src/Factory/PhpredisClientFactory.php
@@ -32,6 +32,7 @@
 use function in_array;
 use function is_a;
 use function is_array;
+use function is_string;
 use function phpversion;
 use function spl_autoload_register;
 use function sprintf;
@@ -111,9 +112,9 @@ public function create(string $class, array $dsns, array $options, string $alias
     }
 
     /**
-     * @param class-string                                                                                                                                                                             $class
-     * @param list<RedisDsn>                                                                                                                                                                           $dsns
-     * @param array{service: ?string, connection_persistent: ?bool, connection_timeout: ?string, read_write_timeout: ?string, parameters: array{sentinel_username: string, sentinel_password: string}} $options
+     * @param class-string                                                                                                                                                                                                  $class
+     * @param list<RedisDsn>                                                                                                                                                                                                $dsns
+     * @param array{service: ?string, connection_persistent: bool|non-empty-string|null, connection_timeout: ?string, read_write_timeout: ?string, parameters: array{sentinel_username: string, sentinel_password: string}} $options
      *
      * @return Redis|Relay
      */
@@ -123,9 +124,15 @@ private function createClientFromSentinel(string $class, array $dsns, string $al
         $sentinelClass        = $isRelay ? Sentinel::class : RedisSentinel::class;
         $masterName           = $options['service'];
         $connectionTimeout    = $options['connection_timeout'] ?? 0;
-        $connectionPersistent = $options['connection_persistent'] ? $masterName : null;
-        $readTimeout          = $options['read_write_timeout'] ?? 0;
-        $parameters           = $options['parameters'];
+        $connectionPersistent = null;
+        if ($options['connection_persistent']) {
+            $connectionPersistent = is_string($options['connection_persistent'])
+                ? $options['connection_persistent']
+                : $masterName;
+        }
+
+        $readTimeout = $options['read_write_timeout'] ?? 0;
+        $parameters  = $options['parameters'];
 
         foreach ($dsns as $dsn) {
             $args = [
@@ -254,11 +261,18 @@ private function createClient(RedisDsn $dsn, string $class, string $alias, array
             $context['stream'] = $options['parameters']['ssl_context'];
         }
 
+        $persistentId = null;
+        if (!empty($options['connection_persistent'])) {
+            $persistentId = is_string($options['connection_persistent'])
+                ? $options['connection_persistent']
+                : $alias;
+        }
+
         $connectParameters = [
             $socket ?? ($dsn->getTls() ? 'tls://' : '') . $dsn->getHost(),
             $dsn->getPort(),
             $options['connection_timeout'],
-            empty($options['connection_persistent']) ? null : $dsn->getPersistentId(),
+            $persistentId,
             5, // retry interval
             5, // read timeout
             $context,
diff --git a/tests/DependencyInjection/SncRedisExtensionTest.php b/tests/DependencyInjection/SncRedisExtensionTest.php
@@ -15,6 +15,7 @@
 
 use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
+use Predis\Client;
 use Redis;
 use RedisException;
 use Relay\Relay;
@@ -30,9 +31,11 @@
 use Symfony\Component\Yaml\Parser;
 
 use function array_key_exists;
+use function class_exists;
 use function count;
 use function current;
 use function sys_get_temp_dir;
+use function version_compare;
 
 /**
  * SncRedisExtensionTest
@@ -755,6 +758,174 @@ private function getPhpRedisWithInvalidACLYamlMinimalConfig(): string
                 username: user
                 password: password
             
+YAML;
+    }
+
+    public function testPredisWithConnectionPersistentBool(): void
+    {
+        if (!class_exists('Predis\Client')) {
+            $this->markTestSkipped('Predis not available');
+        }
+
+        if (version_compare(Client::VERSION, '2.4.0', '<')) {
+            $this->markTestSkipped('Predis version 2.4.0 or higher required for connection_persistent');
+        }
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getPredisWithConnectionPersistentBoolYamlConfig());
+        $extension->load([$config], $container = $this->getContainer());
+
+        $this->assertTrue($container->hasDefinition('snc_redis.default'));
+        $definition = $container->getDefinition('snc_redis.connection.default_parameters.default');
+        $this->assertTrue($definition->getArgument(0)['persistent']);
+        $this->assertNull($definition->getArgument(0)['conn_uid']);
+    }
+
+    public function testPredisWithConnectionPersistentString(): void
+    {
+        if (!class_exists('Predis\Client')) {
+            $this->markTestSkipped('Predis not available');
+        }
+
+        if (version_compare(Client::VERSION, '2.4.0', '<')) {
+            $this->markTestSkipped('Predis version 2.4.0 or higher required for connection_persistent');
+        }
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getPredisWithConnectionPersistentStringYamlConfig());
+        $extension->load([$config], $container = $this->getContainer());
+
+        $this->assertTrue($container->hasDefinition('snc_redis.default'));
+        $definition = $container->getDefinition('snc_redis.connection.default_parameters.default');
+        $this->assertTrue($definition->getArgument(0)['persistent']);
+        $this->assertSame('my_custom_conn_uid', $definition->getArgument(0)['conn_uid']);
+    }
+
+    public function testPredisWithConnectionPersistentVersionTooOld(): void
+    {
+        if (!class_exists('Predis\Client')) {
+            $this->markTestSkipped('Predis not available');
+        }
+
+        if (version_compare(Client::VERSION, '2.4.0', '>=')) {
+            $this->markTestSkipped('This test requires Predis version < 2.4.0');
+        }
+
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('Using connection_persistent as string for Predis requires predis/predis version 2.4.0 or higher');
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getPredisWithConnectionPersistentStringYamlConfig());
+        $extension->load([$config], $container = $this->getContainer());
+    }
+
+    private function getPredisWithConnectionPersistentBoolYamlConfig(): string
+    {
+        return <<<'YAML'
+clients:
+    default:
+        type: predis
+        alias: default
+        dsn: redis://localhost:6379/0
+        options:
+            connection_persistent: true
+
+YAML;
+    }
+
+    private function getPredisWithConnectionPersistentStringYamlConfig(): string
+    {
+        return <<<'YAML'
+clients:
+    default:
+        type: predis
+        alias: default
+        dsn: redis://localhost:6379/0
+        options:
+            connection_persistent: my_custom_conn_uid
+
+YAML;
+    }
+
+    public function testPredisWithConnectionPersistentFalse(): void
+    {
+        if (!class_exists('Predis\Client')) {
+            $this->markTestSkipped('Predis not available');
+        }
+
+        if (version_compare(Client::VERSION, '2.4.0', '<')) {
+            $this->markTestSkipped('Predis version 2.4.0 or higher required for connection_persistent');
+        }
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getPredisWithConnectionPersistentFalseYamlConfig());
+        $extension->load([$config], $container = $this->getContainer());
+
+        $this->assertTrue($container->hasDefinition('snc_redis.default'));
+        $definition = $container->getDefinition('snc_redis.connection.default_parameters.default');
+        $this->assertFalse($definition->getArgument(0)['persistent']);
+        $this->assertNull($definition->getArgument(0)['conn_uid']);
+    }
+
+    private function getPredisWithConnectionPersistentFalseYamlConfig(): string
+    {
+        return <<<'YAML'
+clients:
+    default:
+        type: predis
+        alias: default
+        dsn: redis://localhost:6379/0
+        options:
+            connection_persistent: false
+
+YAML;
+    }
+
+    public function testInvalidConnectionPersistentValue(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('connection_persistent must be a boolean or string');
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getInvalidConnectionPersistentYamlConfig());
+        $extension->load([$config], $this->getContainer());
+    }
+
+    private function getInvalidConnectionPersistentYamlConfig(): string
+    {
+        return <<<'YAML'
+clients:
+    default:
+        type: predis
+        alias: default
+        dsn: redis://localhost:6379/0
+        options:
+            connection_persistent: []
+
+YAML;
+    }
+
+    public function testEmptyStringConnectionPersistentValue(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->expectExceptionMessage('connection_persistent must be a boolean or string');
+
+        $extension = new SncRedisExtension();
+        $config    = $this->parseYaml($this->getEmptyStringConnectionPersistentYamlConfig());
+        $extension->load([$config], $this->getContainer());
+    }
+
+    private function getEmptyStringConnectionPersistentYamlConfig(): string
+    {
+        return <<<'YAML'
+clients:
+    default:
+        type: predis
+        alias: default
+        dsn: redis://localhost:6379/0
+        options:
+            connection_persistent: ""
+            
 YAML;
     }
 
diff --git a/tests/Factory/PhpredisClientFactoryTest.php b/tests/Factory/PhpredisClientFactoryTest.php
