Merge pull request #122 from robin-brabants/feature/redis-cluster-use…

…rname Add support for specific `username` setting in `RedisCluster`
laminas · Dec 9, 2024 · c2740a2 · c2740a2
2 parents b839667 + 71b69e1
commit c2740a2
Show file tree

Hide file tree

Showing 10 changed files with 218 additions and 55 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,7 +1,8 @@
-/.phpunit.result.cache
+/.phpunit.cache
 /.psalm-cache
 /docs/html/
 /laminas-mkdoc-theme.tgz
 /laminas-mkdoc-theme/
 /vendor/
 /.phpcs-cache
+/.idea
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,10 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <files psalm-version="5.24.0@462c80e31c34e58cc4f750c656be3927e80e550e">
-  <file src="src/Exception/InvalidRedisClusterConfigurationException.php">
-    <PossiblyUnusedMethod>
-      <code><![CDATA[fromInvalidSeedsConfiguration]]></code>
-    </PossiblyUnusedMethod>
-  </file>
   <file src="src/Redis.php">
     <InvalidOperand>
       <code><![CDATA[$pttl]]></code>
@@ -52,6 +47,7 @@
       <code><![CDATA[setSeeds]]></code>
       <code><![CDATA[setSslContext]]></code>
       <code><![CDATA[setTimeout]]></code>
+      <code><![CDATA[setUser]]></code>
     </PossiblyUnusedMethod>
   </file>
   <file src="src/RedisOptions.php">

diff --git a/src/RedisAuthenticationInfo.php b/src/RedisAuthenticationInfo.php
@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\Cache\Storage\Adapter;
+
+/**
+ * @internal
+ */
+final class RedisAuthenticationInfo
+{
+    /**
+     * @param non-empty-string|null $username
+     * @param non-empty-string $password
+     */
+    private function __construct(
+        private readonly string|null $username,
+        private readonly string $password,
+    ) {
+    }
+
+    public static function fromOptions(RedisClusterOptions|RedisOptions $options): self|null
+    {
+        $username = $options->getUser();
+        $password = $options->getPassword();
+
+        if ($password === null || $password === '') {
+            return null;
+        }
+
+        if ($username === null || $username === '') {
+            return new self(null, $password);
+        }
+
+        return new self($username, $password);
+    }
+
+    /**
+     * @see https://github.com/phpredis/phpredis/blob/4cd3f59356582a65aec1cceed44741bd5d161d9e/library.c#L4382
+     *
+     * @return array{0:non-empty-string,1?:non-empty-string}
+     */
+    public function toRedisAuthInfo(): array
+    {
+        if ($this->username === null) {
+            return [$this->password];
+        }
+
+        return [$this->username, $this->password];
+    }
+}
diff --git a/src/RedisClusterOptions.php b/src/RedisClusterOptions.php
@@ -64,6 +64,8 @@ final class RedisClusterOptions extends AdapterOptions
     /** @psalm-var array<positive-int,mixed> */
     private array $libOptions = [];
 
+    private string $user = '';
+
     private string $password = '';
 
     private ?SslContext $sslContext = null;
@@ -248,6 +250,19 @@ public function getLibOption(int $option, mixed $default = null): mixed
         return $this->libOptions[$option] ?? $default;
     }
 
+    public function getUser(): string
+    {
+        return $this->user;
+    }
+
+    /**
+     * @psalm-param non-empty-string $user
+     */
+    public function setUser(string $user): void
+    {
+        $this->user = $user;
+    }
+
     public function getPassword(): string
     {
         return $this->password;

diff --git a/src/RedisClusterOptionsFromIni.php b/src/RedisClusterOptionsFromIni.php
@@ -4,6 +4,7 @@
 
 namespace Laminas\Cache\Storage\Adapter;
 
+use InvalidArgumentException;
 use Laminas\Cache\Storage\Adapter\Exception\InvalidRedisClusterConfigurationException;
 use Webmozart\Assert\Assert;
 
@@ -28,7 +29,7 @@ final class RedisClusterOptionsFromIni
     private array $readTimeoutByName;
 
     /** @var array<non-empty-string,string> */
-    private array $authenticationByName;
+    private array $passwordByName;
 
     public function __construct()
     {
@@ -43,12 +44,17 @@ public function __construct()
 
         $seedsByName = [];
         parse_str($seedsConfiguration, $parsedSeedsByName);
-        foreach ($parsedSeedsByName as $name => $seeds) {
-            assert(is_string($name) && $name !== '');
-            Assert::isNonEmptyList($seeds);
-            Assert::allStringNotEmpty($seeds);
-            $seedsByName[$name] = $seeds;
+        try {
+            foreach ($parsedSeedsByName as $name => $seeds) {
+                assert(is_string($name) && $name !== '');
+                Assert::isNonEmptyList($seeds);
+                Assert::allStringNotEmpty($seeds);
+                $seedsByName[$name] = $seeds;
+            }
+        } catch (InvalidArgumentException) {
+            throw InvalidRedisClusterConfigurationException::fromInvalidSeedsConfiguration($seedsConfiguration);
         }
+
         $this->seedsByName = $seedsByName;
 
         $timeoutConfiguration = ini_get('redis.clusters.timeout');
@@ -88,16 +94,16 @@ public function __construct()
             $authenticationConfiguration = '';
         }
 
-        $authenticationByName = [];
+        $passwordByName = [];
         if ($authenticationConfiguration !== '') {
             parse_str($authenticationConfiguration, $parsedAuthenticationByName);
-            foreach ($parsedAuthenticationByName as $name => $authentication) {
-                assert(is_string($name) && $name !== '' && is_string($authentication));
-                $authenticationByName[$name] = $authentication;
+            foreach ($parsedAuthenticationByName as $name => $password) {
+                assert(is_string($name) && $name !== '' && is_string($password));
+                $passwordByName[$name] = $password;
             }
         }
 
-        $this->authenticationByName = $authenticationByName;
+        $this->passwordByName = $passwordByName;
     }
 
     /**
@@ -135,6 +141,6 @@ public function getReadTimeout(string $name, float $fallback): float
      */
     public function getPasswordByName(string $name, string $fallback): string
     {
-        return $this->authenticationByName[$name] ?? $fallback;
+        return $this->passwordByName[$name] ?? $fallback;
     }
 }
diff --git a/src/RedisClusterResourceManager.php b/src/RedisClusterResourceManager.php
@@ -45,35 +45,33 @@ public function getResource(): RedisClusterFromExtension
 
     private function createRedisResource(RedisClusterOptions $options): RedisClusterFromExtension
     {
+        $authenticationInfo = RedisAuthenticationInfo::fromOptions($options);
+
         if ($options->hasName()) {
             return $this->createRedisResourceFromName(
                 $options->getName(),
                 $options->getTimeout(),
                 $options->getReadTimeout(),
                 $options->isPersistent(),
-                $options->getPassword(),
+                $authenticationInfo,
                 $options->getSslContext()
             );
         }
 
-        $password = $options->getPassword();
-        if ($password === '') {
-            $password = null;
-        }
-
         /**
-         * Psalm currently (<= 5.23.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster
+         * Psalm currently (<= 5.26.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster
          * class in the phpredis extension.
          *
-         * @psalm-suppress TooManyArguments https://github.com/vimeo/psalm/pull/10862
+         * @psalm-suppress TooManyArguments
+         * @psalm-suppress InvalidArgument
          */
         return new RedisClusterFromExtension(
             null,
             $options->getSeeds(),
             $options->getTimeout(),
             $options->getReadTimeout(),
             $options->isPersistent(),
-            $password,
+            $authenticationInfo?->toRedisAuthInfo(),
             $options->getSslContext()?->toSslContextArray()
         );
     }
@@ -86,7 +84,7 @@ private function createRedisResourceFromName(
         float $fallbackTimeout,
         float $fallbackReadTimeout,
         bool $persistent,
-        string $fallbackPassword,
+        RedisAuthenticationInfo|null $fallbackAuthentication,
         ?SslContext $sslContext
     ): RedisClusterFromExtension {
         try {
@@ -97,24 +95,26 @@ private function createRedisResourceFromName(
             throw new InvalidRedisClusterConfigurationException($throwable->getMessage(), previous: $throwable);
         }
 
-        $seeds       = $options->getSeeds($name);
-        $timeout     = $options->getTimeout($name, $fallbackTimeout);
-        $readTimeout = $options->getReadTimeout($name, $fallbackReadTimeout);
-        $password    = $options->getPasswordByName($name, $fallbackPassword);
+        $seeds          = $options->getSeeds($name);
+        $timeout        = $options->getTimeout($name, $fallbackTimeout);
+        $readTimeout    = $options->getReadTimeout($name, $fallbackReadTimeout);
+        $password       = $options->getPasswordByName($name, '');
+        $authentication = $password === '' ? $fallbackAuthentication?->toRedisAuthInfo() : $password;
 
         /**
-         * Psalm currently (<= 5.23.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster
+         * Psalm currently (<= 5.26.1) uses an outdated (phpredis < 5.3.2) constructor signature for the RedisCluster
          * class in the phpredis extension.
          *
-         * @psalm-suppress TooManyArguments https://github.com/vimeo/psalm/pull/10862
+         * @psalm-suppress TooManyArguments
+         * @psalm-suppress PossiblyInvalidArgument
          */
         return new RedisClusterFromExtension(
             null,
             $seeds,
             $timeout,
             $readTimeout,
             $persistent,
-            $password,
+            $authentication,
             $sslContext?->toSslContextArray()
         );
     }

diff --git a/src/RedisOptions.php b/src/RedisOptions.php
@@ -228,6 +228,9 @@ public function setPassword(string $password): void
         $this->password = $password;
     }
 
+    /**
+     * @return non-empty-string|null
+     */
     public function getPassword(): string|null
     {
         return $this->password;
@@ -242,6 +245,9 @@ public function setUser(string $user): void
         $this->user = $user;
     }
 
+    /**
+     * @return non-empty-string|null
+     */
     public function getUser(): ?string
     {
         return $this->user;

diff --git a/src/RedisResourceManager.php b/src/RedisResourceManager.php
@@ -57,28 +57,14 @@ private function createRedisFromExtension(RedisOptions $options): RedisFromExten
             $port = $server['port'] ?? self::DEFAULT_REDIS_PORT;
         }
 
-        $authentication = [];
-        $user           = $options->getUser();
-        $password       = $options->getPassword();
-
-        if ($user !== null) {
-            $authentication[] = $user;
-        }
-
-        if ($password !== null) {
-            $authentication[] = $password;
-        }
-
-        if ($authentication === []) {
-            $authentication = null;
-        }
+        $authenticationInfo = RedisAuthenticationInfo::fromOptions($options);
 
         $resourceOptions = [
             'host'           => $host,
             'port'           => $port,
             'connectTimeout' => $server['timeout'] ?? null,
             'persistent'     => $options->getPersistentId() ?? $options->isPersistent(),
-            'auth'           => $authentication,
+            'auth'           => $authenticationInfo?->toRedisAuthInfo(),
         ];
 
         $resource = new RedisFromExtension(array_filter($resourceOptions, fn (mixed $value) => $value !== null));