Support connection retries for Redis session and compatible with colinmollenhour/php-redis-session-abstract v2.0.0

[TuVanDev]

This pull request is intended to resolve compatibility issues with colinmollenhour/php-redis-session-abstract v1.6.0 and to support connection retries for Redis sessions, which were introduced in v1.6.0 of colinmollenhour/php-redis-session-abstract. However, since colinmollenhour/php-redis-session-abstract reverted the changes in v1.7.0, the compatibility issue no longer exists. Furthermore, colinmollenhour/php-redis-session-abstract has also released version v2.0.0, which is identical to 1.6.0. Therefore, the changes in this pull request now remain to support connection retries for Redis sessions, as introduced in v2.0.0 of colinmollenhour/php-redis-session-abstract.

Description (*)

The newest version of Cm RedisSession, version v2.0.0 has introduced support connection retries. This pull request intends to implement it in the Magento codebase.

<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
namespace Magento\Framework\Session\SaveHandler\Redis;

class Config implements \Cm\RedisSession\Handler\ConfigInterface
{
}

The changes made in this pull request include:

• Added support retries connection for Redis sessions
• Added unit tests

Related Pull Requests

• N/A

Fixed Issues (if relevant)

1. Fixes Credis update to 1.6 breaks Magento 2.4.7 #38728

Manual testing scenarios (*)

1. ...
2. ...

Questions or comments

Patches

Until the issue is resolved in the next Magento version, you can apply the two patches I have prepared:

1. The patch for the magento/framework package:
   File name: magento-magento2-base_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v1.6.0.patch

diff --git a/vendor/magento/framework/Session/SaveHandler/Redis/Config.php b/vendor/magento/framework/Session/SaveHandler/Redis/Config.php
index 70b666f8..ac6e6227 100644
--- a/vendor/magento/framework/Session/SaveHandler/Redis/Config.php
+++ b/vendor/magento/framework/Session/SaveHandler/Redis/Config.php
@@ -45,6 +45,11 @@ class Config implements \Cm\RedisSession\Handler\ConfigInterface
      */
     const PARAM_TIMEOUT                 = 'session/redis/timeout';
 
+    /**
+     * Configuration path for number of connection retries
+     */
+    const PARAM_RETRIES = 'session/redis/retries';
+
     /**
      * Configuration path for persistent identifier
      */
@@ -220,6 +225,14 @@ class Config implements \Cm\RedisSession\Handler\ConfigInterface
         return $this->deploymentConfig->get(self::PARAM_TIMEOUT);
     }
 
+    /**
+     * @inheritdoc
+     */
+    public function getRetries()
+    {
+        return $this->deploymentConfig->get(self::PARAM_RETRIES);
+    }
+
     /**
      * @inheritdoc
      */
diff --git a/vendor/magento/framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php b/vendor/magento/framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php
index 75944922..75f8d5ba 100644
--- a/vendor/magento/framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php
+++ b/vendor/magento/framework/Session/Test/Unit/SaveHandler/Redis/ConfigTest.php
@@ -114,6 +114,16 @@ class ConfigTest extends TestCase
         $this->assertEquals($this->config->getTimeout(), $expected);
     }
 
+    public function testGetRetries()
+    {
+        $expected = 10;
+        $this->deploymentConfigMock->expects($this->once())
+            ->method('get')
+            ->willReturn(Config::PARAM_RETRIES)
+            ->willReturn($expected);
+        $this->assertEquals($this->config->getRetries(), $expected);
+    }
+
     public function testGetPersistentIdentifier()
     {
         $expected = 'sess01';

2. The patch for the magento/magento2-base package:
   File name: magento-magento2-framework_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v2.0.0.patch

diff --git a/vendor/magento/magento2-base/setup/src/Magento/Setup/Model/ConfigOptionsList/Session.php b/vendor/magento/magento2-base/setup/src/Magento/Setup/Model/ConfigOptionsList/Session.php
index 4a3a02b3..eeaa6174 100644
--- a/vendor/magento/magento2-base/setup/src/Magento/Setup/Model/ConfigOptionsList/Session.php
+++ b/vendor/magento/magento2-base/setup/src/Magento/Setup/Model/ConfigOptionsList/Session.php
@@ -23,6 +23,7 @@ class Session implements ConfigOptionsListInterface
     const INPUT_KEY_SESSION_REDIS_PORT = 'session-save-redis-port';
     const INPUT_KEY_SESSION_REDIS_PASSWORD = 'session-save-redis-password';
     const INPUT_KEY_SESSION_REDIS_TIMEOUT = 'session-save-redis-timeout';
+    const INPUT_KEY_SESSION_REDIS_RETRIES = 'session-save-redis-retries';
     const INPUT_KEY_SESSION_REDIS_PERSISTENT_IDENTIFIER = 'session-save-redis-persistent-id';
     const INPUT_KEY_SESSION_REDIS_DATABASE = 'session-save-redis-db';
     const INPUT_KEY_SESSION_REDIS_COMPRESSION_THRESHOLD = 'session-save-redis-compression-threshold';
@@ -47,6 +48,7 @@ class Session implements ConfigOptionsListInterface
     const CONFIG_PATH_SESSION_REDIS_PORT = 'session/redis/port';
     const CONFIG_PATH_SESSION_REDIS_PASSWORD = 'session/redis/password';
     const CONFIG_PATH_SESSION_REDIS_TIMEOUT = 'session/redis/timeout';
+    const CONFIG_PATH_SESSION_REDIS_RETRIES = 'session/redis/retries';
     const CONFIG_PATH_SESSION_REDIS_PERSISTENT_IDENTIFIER = 'session/redis/persistent_identifier';
     const CONFIG_PATH_SESSION_REDIS_DATABASE = 'session/redis/database';
     const CONFIG_PATH_SESSION_REDIS_COMPRESSION_THRESHOLD = 'session/redis/compression_threshold';
@@ -75,6 +77,7 @@ class Session implements ConfigOptionsListInterface
         self::INPUT_KEY_SESSION_REDIS_PORT => '6379',
         self::INPUT_KEY_SESSION_REDIS_PASSWORD => '',
         self::INPUT_KEY_SESSION_REDIS_TIMEOUT => '2.5',
+        self::INPUT_KEY_SESSION_REDIS_RETRIES => '0',
         self::INPUT_KEY_SESSION_REDIS_PERSISTENT_IDENTIFIER => '',
         self::INPUT_KEY_SESSION_REDIS_DATABASE => '2',
         self::INPUT_KEY_SESSION_REDIS_COMPRESSION_THRESHOLD => '2048',
@@ -117,6 +120,7 @@ class Session implements ConfigOptionsListInterface
         self::INPUT_KEY_SESSION_REDIS_PORT => self::CONFIG_PATH_SESSION_REDIS_PORT,
         self::INPUT_KEY_SESSION_REDIS_PASSWORD => self::CONFIG_PATH_SESSION_REDIS_PASSWORD,
         self::INPUT_KEY_SESSION_REDIS_TIMEOUT => self::CONFIG_PATH_SESSION_REDIS_TIMEOUT,
+        self::INPUT_KEY_SESSION_REDIS_RETRIES => self::CONFIG_PATH_SESSION_REDIS_RETRIES,
         self::INPUT_KEY_SESSION_REDIS_PERSISTENT_IDENTIFIER => self::CONFIG_PATH_SESSION_REDIS_PERSISTENT_IDENTIFIER,
         self::INPUT_KEY_SESSION_REDIS_DATABASE => self::CONFIG_PATH_SESSION_REDIS_DATABASE,
         self::INPUT_KEY_SESSION_REDIS_COMPRESSION_THRESHOLD => self::CONFIG_PATH_SESSION_REDIS_COMPRESSION_THRESHOLD,
@@ -177,6 +181,12 @@ class Session implements ConfigOptionsListInterface
                 self::CONFIG_PATH_SESSION_REDIS_TIMEOUT,
                 'Connection timeout, in seconds'
             ),
+            new TextConfigOption(
+                self::INPUT_KEY_SESSION_REDIS_RETRIES,
+                TextConfigOption::FRONTEND_WIZARD_TEXT,
+                self::CONFIG_PATH_SESSION_REDIS_RETRIES,
+                'Redis connection retries.'
+            ),
             new TextConfigOption(
                 self::INPUT_KEY_SESSION_REDIS_PERSISTENT_IDENTIFIER,
                 TextConfigOption::FRONTEND_WIZARD_TEXT,
diff --git a/vendor/magento/magento2-base/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsList/SessionTest.php b/vendor/magento/magento2-base/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsList/SessionTest.php
index 98ef1d60..cbd5b1de 100644
--- a/vendor/magento/magento2-base/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsList/SessionTest.php
+++ b/vendor/magento/magento2-base/setup/src/Magento/Setup/Test/Unit/Model/ConfigOptionsList/SessionTest.php
@@ -147,6 +147,7 @@ class SessionTest extends TestCase
                     'port' => '',
                     'password' => '',
                     'timeout' => '',
+                    'retries' => '',
                     'persistent_identifier' => '',
                     'database' => '',
                     'compression_threshold' => '',
@@ -204,6 +205,7 @@ class SessionTest extends TestCase
                     'port' => '',
                     'password' => '',
                     'timeout' => '',
+                    'retries' => '',
                     'persistent_identifier' => '',
                     'database' => '',
                     'compression_threshold' => '',

To apply patches for Magento Open Source and Adobe Commerce, please refer to the following documentation:

• Document on how to apply a composer patch for Magento open source project: https://experienceleague.adobe.com/docs/commerce-operations/upgrade-guide/patches/apply.html
  The configuration for applying the two patches using the cweagans/composer-patches composer patch plugin to address this issue should look like the following:

"extra": {
    "magento-force": "override",
    "patches": {
        "magento/framework": {
            "Github-PR-38729 - Compatible with colinmollenhour/php-redis-session-abstract v2.0.0 and support connection retries for Redis session": "patches/magento-magento2-framework_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v2.0.0.patch"
        },
        "magento/magento2-base": {
            "Github-PR-38729 - Compatible with colinmollenhour/php-redis-session-abstract v2.0.0 and support connection retries for Redis session": "patches/magento-magento2-base_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v2.0.0.patch"
        }
    }
}

• Document on how to apply a composer patch for Adobe Commerce on Cloud infrastructure https://experienceleague.adobe.com/docs/commerce-knowledge-base/kb/how-to/how-to-apply-a-composer-patch-provided-by-magento.html?lang=en, and in more details here https://experienceleague.adobe.com/docs/commerce-cloud-service/user-guide/develop/upgrade/apply-patches.html#:~:text=Copy-,Apply%20a%20custom%20patch,-When%20you%20deploy

Feature information

If this pull request is accepted, you will be able to configure the number of retries for the Redis connection in the app/etc/env.php file, under the session section:

'session' => [
    'save' => 'redis',
    'redis' => [
        // additional configurations
        'retries' => 10,
        // other configurations
    ]
]

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @TuVanDev. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[TuVanDev]

@magento run all tests

[hostep]

Hmm, let's hold on a bit and see what the response is from colinmollenhour/php-redis-session-abstract#58, I'm pretty sure the change will be reverted, maybe they'll release a 2.0.0 with this change, and then we can implement this fix ...

[haroonsulahri]

I have been facing the same issue recently on my site when I updated her composer then I started getting this error while running the compilation command.

[hostep]

So, since version 1.7.0 of colinmollenhour/php-redis-session-abstract has been released (which has the same code as 1.5.5) and version 2.0.0 now is the same as 1.6.0, this PR should be updated and the constraints in the composer.json files should be changed to ^2.0 here:

• magento2/composer.json

  Line 41 in c893121

  "colinmollenhour/php-redis-session-abstract": "^1.5",

• magento2/lib/internal/Magento/Framework/composer.json

  Line 26 in c893121

  "colinmollenhour/php-redis-session-abstract": "^1.5",

Update: maybe you can also change the description of this PR?

@TuVanDev: could you do this? Thanks!

[TuVanDev]

Thanks @hostep

I've updated the colinmollenhour/php-redis-session-abstract dependency package to the latest version, 2.0.

To the maintainers: Since the Magento\Framework\Session\SaveHandler\Redis\Config and Magento\Setup\Model\ConfigOptionsList\Session classes have not been updated for years, this PR may encounter failed tests due to the existing code. Unfortunately, I don't currently have the time to address this issue, so I would greatly appreciate it if someone could assist me with resolving it. Thank you!

[hostep]

Thanks @TuVanDev, can you also update the title of the PR with the correct version? And maybe remove the patch instructions from the description, it's not longer relevant as a quick fix now that v1.7 is out. Thank you!

[engcom-Hotel]

@magento run all tests

[engcom-Hotel]

Hello @TuVanDev,

Thanks for the collaboration!

Please check the below review comment and also the automated tests failures. It seems the failures are due to code changes, mainly static test failures.

Thanks

[engcom-Hotel]

Please add the return type for this method.

[TuVanDev]

Hi @engcom-Alfa , this method is similar to other methods in the Config class. If we add the return type to this method, we should also add the return types to other methods for code consistency.

    /**
     * @inheritdoc
     */
    public function getPassword()
    {
        return $this->deploymentConfig->get(self::PARAM_PASSWORD);
    }
    /**
     * @inheritdoc
     */
    public function getTimeout()
    {
        return $this->deploymentConfig->get(self::PARAM_TIMEOUT);
    }

    /**
     * @inheritdoc
     */
    public function getRetries()
        {
        return $this->deploymentConfig->get(self::PARAM_RETRIES);
    }

I understand that the changes in this pull request may result in failed tests, as the classes Magento\Framework\Session\SaveHandler\Redis\Config and Magento\Setup\Model\ConfigOptionsList\Session have not been updated for years. Unfortunately, I don't currently have the time to address this issue, so I would greatly appreciate it if someone could assist me with resolving it.

[TuVanDev]

@hostep I've updated the title to match the new version of colinmollenhour/php-redis-session-abstract. Regarding the patches, I believe it is definitely helpful for someone who would like to apply the connection retries before it becomes available in the Magento codebase, so I'll keep it as it is.

Thanks for suggestions.

[engcom-Hotel]

Hello @TuVanDev,

Please resolve the conflicts of this PR, so that we can move further with this.

Meanwhile marking this PR as draft.

Thanks

[engcom-Bravo]

Hello,

Internal Team will pick this PR sooner and will work on the failures and ensure the compatibility of library with latest version.

Thanks.