PR Review, Extra tests and translations

Queue/Backend/Redis.php

@@ -265,7 +265,7 @@ protected function connect()
         $success = $this->redis->connect($this->host, $this->port, $this->timeout, null, 100);
 
         if ($success && !empty($this->password)) {
-            $success = $this->redis->auth($this->password, $this->username);
+            $success = $this->redis->auth([$this->username, $this->password]);
         }
 
         if (!empty($this->database) || 0 === $this->database) {

Queue/Backend/RedisCluster.php

@@ -320,7 +320,8 @@ public function setConfig(
         $port,
         $timeout,
         #[\SensitiveParameter]
-        $password
+        $password,
+        $username = null,
     ) {
         $this->disconnect();
 
@@ -331,6 +332,10 @@ public function setConfig(
         if (!empty($password)) {
             $this->password = $password;
         }
+
+        if (!empty($username)) {
+            $this->username = $username;
+        }
     }
 
     private function disconnect()

SystemSettings.php

@@ -242,11 +242,11 @@ private function createNumberOfQueueWorkerSetting()
     private function createRedisUsernameSetting()
     {
         return $this->makeSetting('redisUsername', $default = '', FieldConfig::TYPE_STRING, function (FieldConfig $field) {
-            $field->title = 'Redis Username';
+            $field->title = Piwik::translate('QueuedTracking_RedisUsernameFieldTitle');
             $field->condition = 'backend=="redis"';
             $field->uiControl = FieldConfig::UI_CONTROL_TEXT;
             $field->uiControlAttributes = array('size' => 128);
-            $field->inlineHelp = 'Username for Redis ACL authentication. Leave empty if not used.';
+            $field->inlineHelp = Piwik::translate('QueuedTracking_RedisUsernameFieldHelp');
             $field->validators[] = new CharacterLength(null, 128);
         });
     }

lang/en.json

@@ -16,6 +16,8 @@
         "AvailableRedisBackendTypeStandAlone": "Stand-alone",
         "AvailableRedisBackendTypeSentinel": "Sentinel",
         "AvailableRedisBackendTypeCluster": "Cluster",
+        "RedisUsernameFieldTitle": "Redis Username",
+        "RedisUsernameFieldHelp": "Username for Redis ACL authentication. Leave empty if not used.",
         "RedisPasswordFieldTitle": "Redis password",
         "RedisPasswordFieldHelp": "Password set on the Redis server, if any. Redis can be instructed to require a password before allowing clients to execute commands.",
         "RedisDatabaseFieldTitle": "Redis database",

tests/Integration/Queue/Backend/RedisTest.php

@@ -10,6 +10,7 @@
 namespace Piwik\Plugins\QueuedTracking\tests\Integration\Queue\Backend;
 
 use Piwik\Plugins\QueuedTracking\Queue\Backend\Redis;
+use Piwik\Plugins\QueuedTracking\Queue\Factory;
 use Piwik\Plugins\QueuedTracking\tests\Framework\TestCase\IntegrationTestCase;
 
 /**
@@ -279,6 +280,104 @@ public function test_checkConnectionDetails_shouldFailIfPortIsWrong()
         $this->assertFalse($success);
     }
 
+    public function test_checkConnectionWithPasswordShouldFailIfPasswordIsWrong(): void
+    {
+        $password = 'correctPassword';
+
+        try {
+            $this->createRedisPassword($password);
+
+            $settings = Factory::getSettings();
+            $this->redis->setConfig($settings->redisHost->getValue(), (int) $settings->redisPort->getValue(), 0.2, 'wrongPassword');
+            $success = $this->redis->testConnection();
+
+            $this->assertFalse($success);
+        } finally {
+            $this->removeRedisPassword(null, $password);
+        }
+    }
+
+    public function test_checkConnectionWithCorrectPasswordShouldConnect(): void
+    {
+        $password = 'correctPassword';
+
+        try {
+            $this->createRedisPassword($password);
+
+            $settings = Factory::getSettings();
+            $this->redis->setConfig($settings->redisHost->getValue(), (int)  $settings->redisPort->getValue(), 0.2, $password);
+            $success = $this->redis->testConnection();
+
+            $this->assertTrue($success);
+        } finally {
+            $this->removeRedisPassword(null, $password);
+        }
+    }
+
+    public function test_checkConnectionWithUsernameShouldFailIfNotCorrect(): void
+    {
+        try {
+            $this->createRedisPassword($password = 'correctPassword', $username = 'correctUsername');
+
+            $settings = Factory::getSettings();
+            $this->redis->setConfig($settings->redisHost->getValue(), (int) $settings->redisPort->getValue(), 0.2, $password, 'wrongUsername');
+            $success = $this->redis->testConnection();
+
+            $this->assertFalse($success);
+        } finally {
+            $this->removeRedisPassword($username);
+        }
+    }
+
+    public function test_checkConnectionWithUsernameShouldConnectIfCorrect(): void
+    {
+        try {
+            $this->createRedisPassword($password = 'correctPassword', $username = 'correctUsername');
+
+            $settings = Factory::getSettings();
+            $this->redis->setConfig($settings->redisHost->getValue(), (int) $settings->redisPort->getValue(), 0.2, $password, $username);
+            $success = $this->redis->testConnection();
+
+            $this->assertTrue($success);
+        } finally {
+            $this->removeRedisPassword($username);
+        }
+    }
+
+    private function createRedisPassword($password, $username = null)
+    {
+        if (!empty($username)) {
+            $this->createAdminConnection()->rawcommand('ACL', 'SETUSER', $username, 'on', '>' . $password, '~*', '&*', '+@all');
+        } else {
+            $this->createAdminConnection()->rawcommand('CONFIG', 'SET', 'requirepass', $password);
+        }
+    }
+
+    private function removeRedisPassword($username = null, $requirepass = null): void
+    {
+        if (empty($username)) {
+            $this->createAdminConnection($requirepass)->rawcommand('CONFIG', 'SET', 'requirepass', '');
+        } else {
+            $this->createAdminConnection()->rawCommand('ACL', 'DELUSER', $username);
+        }
+    }
+
+    /**
+     * Admin connection used only to set up/tear down auth state for these tests. This must
+     * always target the real Redis master directly (127.0.0.1:6379) and not sentinel.
+     */
+    private function createAdminConnection($requirepass = null)
+    {
+        $connection = new \Redis();
+        $connection->connect('127.0.0.1', 6379, 0.2);
+
+        if (!empty($requirepass)) {
+            $connection->auth($requirepass);
+        }
+
+        return $connection;
+    }
+
     public function test_checkConnectionDetails_shouldNotFailIfConnectionDataIsCorrect()
     {
         $success = $this->createRedisBackend()->testConnection();