diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 641ddb2a..759e594f 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -12,6 +12,14 @@ jobs: SYMFONY_REQUIRE: "${{matrix.symfony-require}}" SYMFONY_DEPRECATIONS_HELPER: "max[self]=0" + services: + redis-sentinel: + image: bitnami/redis-sentinel + env: + REDIS_MASTER_HOST: localhost + ports: + - 26379:26379 + strategy: fail-fast: false matrix: diff --git a/docs/README.md b/docs/README.md index ab47e5d4..64781699 100644 --- a/docs/README.md +++ b/docs/README.md @@ -105,7 +105,7 @@ snc_redis: Please note that the master dsn connection needs to be tagged with the ```master``` alias. If not, `predis` will complain. -A setup using `predis` sentinel replication could look like this: +A setup using `predis` or `phpredis` sentinel replication could look like this: ``` yaml snc_redis: diff --git a/src/DependencyInjection/Configuration/Configuration.php b/src/DependencyInjection/Configuration/Configuration.php index 6ae423ba..d320d7a9 100644 --- a/src/DependencyInjection/Configuration/Configuration.php +++ b/src/DependencyInjection/Configuration/Configuration.php @@ -126,7 +126,7 @@ private function addClientsSection(ArrayNodeDefinition $rootNode): void ->scalarNode('profile')->defaultValue('default')->end() ->scalarNode('cluster')->defaultNull()->end() ->scalarNode('prefix')->defaultNull()->end() - ->enumNode('replication')->values([true, false, 'sentinel'])->end() + ->enumNode('replication')->values([true, false, 'sentinel'])->defaultValue(false)->end() ->scalarNode('service')->defaultNull()->end() ->enumNode('slave_failover')->values(['none', 'error', 'distribute', 'distribute_slaves'])->end() ->arrayNode('parameters') diff --git a/src/DependencyInjection/SncRedisExtension.php b/src/DependencyInjection/SncRedisExtension.php index 99de0366..7bf3b292 100644 --- a/src/DependencyInjection/SncRedisExtension.php +++ b/src/DependencyInjection/SncRedisExtension.php @@ -17,6 +17,7 @@ use LogicException; use Predis\Command\Processor\KeyPrefixProcessor; use Predis\Profile\Factory; +use RedisSentinel; use Snc\RedisBundle\DependencyInjection\Configuration\Configuration; use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn; use Snc\RedisBundle\DependencyInjection\Configuration\RedisEnvDsn; @@ -225,16 +226,22 @@ private function loadPredisConnectionParameters(string $clientAlias, array $opti /** @param mixed[] $options A client configuration */ private function loadPhpredisClient(array $options, ContainerBuilder $container): void { - $connectionCount = count($options['dsns']); - $hasClusterOption = $options['options']['cluster'] !== null; + $connectionCount = count($options['dsns']); + $hasClusterOption = $options['options']['cluster'] !== null; + $hasSentinelOption = (bool) $options['options']['replication']; - if ($connectionCount > 1 && !$hasClusterOption) { - throw new LogicException(sprintf('\RedisArray is not supported yet but \RedisCluster is: set option "cluster" to true to enable it.')); + if ($connectionCount > 1 && !$hasClusterOption && !$hasSentinelOption) { + throw new LogicException('Use options "cluster" or "sentinel" to enable support for multi DSN instances.'); + } + + if ($hasClusterOption && $hasSentinelOption) { + throw new LogicException('You cannot have both cluster and sentinel enabled for same redis connection'); } $phpredisClientClass = (string) $container->getParameter('snc_redis.phpredis_' . ($hasClusterOption ? 'cluster' : '') . 'client.class'); - $phpredisDef = new Definition($phpredisClientClass, [ - $phpredisClientClass, + + $phpredisDef = new Definition($phpredisClientClass, [ + $hasSentinelOption ? RedisSentinel::class : $phpredisClientClass, array_map('strval', $options['dsns']), $options['options'], $options['alias'], diff --git a/src/Factory/PhpredisClientFactory.php b/src/Factory/PhpredisClientFactory.php index 8efb75b7..f7e99169 100644 --- a/src/Factory/PhpredisClientFactory.php +++ b/src/Factory/PhpredisClientFactory.php @@ -4,12 +4,14 @@ namespace Snc\RedisBundle\Factory; +use InvalidArgumentException; use LogicException; use ProxyManager\Configuration; use ProxyManager\Factory\AccessInterceptorValueHolderFactory; use ProxyManager\Proxy\AccessInterceptorInterface; use Redis; use RedisCluster; +use RedisSentinel; use ReflectionClass; use ReflectionMethod; use Snc\RedisBundle\DependencyInjection\Configuration\RedisDsn; @@ -26,6 +28,7 @@ use function is_array; use function spl_autoload_register; use function sprintf; +use function var_export; /** @internal */ class PhpredisClientFactory @@ -67,8 +70,12 @@ public function __construct(callable $interceptor, ?Configuration $proxyConfigur */ public function create(string $class, array $dsns, array $options, string $alias, bool $loggingEnabled) { - if (!is_a($class, Redis::class, true) && !is_a($class, RedisCluster::class, true)) { - throw new LogicException(sprintf('The factory can only instantiate \Redis|\RedisCluster classes: "%s" asked', $class)); + $isRedis = is_a($class, Redis::class, true); + $isSentinel = is_a($class, RedisSentinel::class, true); + $isCluster = is_a($class, RedisCluster::class, true); + + if (!$isRedis && !$isSentinel && !$isCluster) { + throw new LogicException(sprintf('The factory can only instantiate Redis|RedisCluster|RedisSentinel classes: "%s" asked', $class)); } // Normalize the DSNs, because using processed environment variables could lead to nested values. @@ -76,7 +83,7 @@ public function create(string $class, array $dsns, array $options, string $alias $parsedDsns = array_map(static fn (string $dsn) => new RedisDsn($dsn), $dsns); - if (is_a($class, Redis::class, true)) { + if ($isRedis) { if (count($parsedDsns) > 1) { throw new LogicException('Cannot have more than 1 dsn with \Redis and \RedisArray is not supported yet.'); } @@ -84,9 +91,51 @@ public function create(string $class, array $dsns, array $options, string $alias return $this->createClient($parsedDsns[0], $class, $alias, $options, $loggingEnabled); } + if ($isSentinel) { + return $this->createClientFromSentinel($parsedDsns, $alias, $options, $loggingEnabled); + } + return $this->createClusterClient($parsedDsns, $class, $alias, $options, $loggingEnabled); } + /** + * @param list $dsns + * @param array{service: ?string} $options + */ + private function createClientFromSentinel(array $dsns, string $alias, array $options, bool $loggingEnabled): Redis + { + foreach ($dsns as $dsn) { + $address = (new RedisSentinel($dsn->getHost(), (int) $dsn->getPort()))->getMasterAddrByName($options['service']); + + if (!$address) { + continue; + } + + return $this->createClient( + new class ($dsn->__toString(), $address[0], (int) $address[1]) extends RedisDsn { + public function __construct(string $dsn, string $host, int $port) + { + parent::__construct($dsn); + $this->host = $host; + $this->port = $port; + } + }, + Redis::class, + $alias, + $options, + $loggingEnabled, + ); + } + + throw new InvalidArgumentException( + sprintf( + 'Failed to retrieve master information from sentinel %s and dsn %s.', + var_export($options['service'], true), + var_export($dsns, true), + ), + ); + } + /** * @param RedisDsn[] $dsns * @param mixed[] $options diff --git a/tests/DependencyInjection/Fixtures/config/yaml/env_phpredis_sentinel.yaml b/tests/DependencyInjection/Fixtures/config/yaml/env_phpredis_sentinel.yaml new file mode 100644 index 00000000..fd165b24 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/config/yaml/env_phpredis_sentinel.yaml @@ -0,0 +1,12 @@ +parameters: + env(REDIS_URL_1): redis://localhost:7000 + +snc_redis: + clients: + phpredissentinel: + type: phpredis + alias: phpredissentinel + dsn: "%env(REDIS_URL_1)%" + options: + replication: sentinel + service: mymaster diff --git a/tests/DependencyInjection/SncRedisExtensionEnvTest.php b/tests/DependencyInjection/SncRedisExtensionEnvTest.php index 4f813b69..c038f67f 100644 --- a/tests/DependencyInjection/SncRedisExtensionEnvTest.php +++ b/tests/DependencyInjection/SncRedisExtensionEnvTest.php @@ -9,6 +9,7 @@ use Predis\Profile\RedisVersion260; use Redis; use RedisCluster; +use RedisSentinel; use Snc\RedisBundle\DependencyInjection\SncRedisExtension; use Snc\RedisBundle\Factory\PredisParametersFactory; use Symfony\Component\Config\FileLocator; @@ -63,6 +64,7 @@ public function testPhpredisDefaultParameterConfig(): void 'profile' => 'default', 'cluster' => null, 'prefix' => null, + 'replication' => false, 'service' => null, ], $clientDefinition->getArgument(2), @@ -102,6 +104,7 @@ public function testPhpredisFullConfig(): void 'throw_errors' => true, 'profile' => 'default', 'cluster' => null, + 'replication' => false, 'service' => null, ], $clientDefinition->getArgument(2), @@ -140,6 +143,7 @@ public function testPhpredisWithAclConfig(): void 'serialization' => 'php', 'service' => null, 'throw_errors' => true, + 'replication' => null, ], $clientDefinition->getArgument(2), ); @@ -192,12 +196,43 @@ public function testPhpRedisClusterOption(): void 'serialization' => 'default', 'profile' => 'default', 'prefix' => null, + 'replication' => false, 'service' => null, ], $clientDefinition->getArgument(2), ); } + public function testPhpRedisSentinelOption(): void + { + $container = $this->getConfiguredContainer('env_phpredis_sentinel'); + $clientDefinition = $container->findDefinition('snc_redis.phpredissentinel'); + + $this->assertSame(Redis::class, $clientDefinition->getClass()); + $this->assertSame(RedisSentinel::class, $clientDefinition->getArgument(0)); + $this->assertStringContainsString('REDIS_URL_1', $clientDefinition->getArgument(1)[0]); + $this->assertSame('phpredissentinel', $clientDefinition->getArgument(3)); + $this->assertFalse($clientDefinition->getArgument(4)); + + $this->assertSame( + [ + 'replication' => 'sentinel', + 'service' => 'mymaster', + 'connection_async' => false, + 'connection_persistent' => false, + 'connection_timeout' => 5, + 'read_write_timeout' => null, + 'iterable_multibulk' => false, + 'throw_errors' => true, + 'serialization' => 'default', + 'profile' => 'default', + 'cluster' => null, + 'prefix' => null, + ], + $clientDefinition->getArgument(2), + ); + } + public function testPhpRedisClusterOptionMultipleDsn(): void { $container = $this->getConfiguredContainer('env_phpredis_cluster_multiple_dsn'); @@ -222,6 +257,7 @@ public function testPhpRedisClusterOptionMultipleDsn(): void 'serialization' => 'default', 'profile' => 'default', 'prefix' => null, + 'replication' => false, 'service' => null, ], $clientDefinition->getArgument(2), @@ -231,7 +267,7 @@ public function testPhpRedisClusterOptionMultipleDsn(): void public function testPhpRedisArrayIsNotSupported(): void { $this->expectException(LogicException::class); - $this->expectExceptionMessage('RedisArray is not supported yet'); + $this->expectExceptionMessage('Use options "cluster" or "sentinel" to enable support for multi DSN instances.'); $this->getConfiguredContainer('env_phpredis_array_not_supported'); } diff --git a/tests/Factory/PhpredisClientFactoryTest.php b/tests/Factory/PhpredisClientFactoryTest.php index e367908a..e6f4b1db 100644 --- a/tests/Factory/PhpredisClientFactoryTest.php +++ b/tests/Factory/PhpredisClientFactoryTest.php @@ -9,6 +9,7 @@ use Psr\Log\LoggerInterface; use Redis; use RedisCluster; +use RedisSentinel; use Snc\RedisBundle\Factory\PhpredisClientFactory; use Snc\RedisBundle\Logger\RedisCallInterceptor; use Snc\RedisBundle\Logger\RedisLogger; @@ -86,6 +87,28 @@ public function testCreateMinimalClusterConfig(): void $this->assertSame(0, $client->getOption(RedisCluster::OPT_SLAVE_FAILOVER)); } + public function testCreatSentinelConfig(): void + { + $this->logger->method('debug')->withConsecutive( + [$this->stringContains('Executing command "CONNECT 127.0.0.1 6379 5 ')], + ['Executing command "AUTH sncredis"'], + ); + $factory = new PhpredisClientFactory(new RedisCallInterceptor($this->redisLogger)); + + $client = $factory->create( + RedisSentinel::class, + ['redis://sncredis@localhost:26379'], + ['connection_timeout' => 5, 'connection_persistent' => false, 'service' => 'mymaster'], + 'phpredissentinel', + true, + ); + + $this->assertInstanceOf(Redis::class, $client); + $this->assertNull($client->getOption(Redis::OPT_PREFIX)); + $this->assertSame(0, $client->getOption(Redis::OPT_SERIALIZER)); + $this->assertSame('sncredis', $client->getAuth()); + } + public function testCreateFullConfig(): void { $factory = new PhpredisClientFactory(new RedisCallInterceptor($this->redisLogger)); diff --git a/tests/Functional/App/config.yaml b/tests/Functional/App/config.yaml index 5e82ff68..337bd8c6 100644 --- a/tests/Functional/App/config.yaml +++ b/tests/Functional/App/config.yaml @@ -41,6 +41,13 @@ snc_redis: read_write_timeout: 30 iterable_multibulk: false throw_errors: true + sentinel: + type: phpredis + alias: sentinel + dsn: redis://sncredis@localhost:26379 + options: + replication: true + service: mymaster with_acl: type: predis alias: with_acl