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

[TuVanDev]

This pull request resolves the compatibility issues with colinmollenhour/php-redis-session-abstract v1.6.0.

Description (*)

The newest version of Cm RedisSession, version v1.6.0 has caused compatibility issues with Magento and Adobe Commerce projects, as they introduce a new method in their Interface which the Magento core class implemented.

<?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 magento/magento2#38728

Manual testing scenarios (*)

1. Install a new Magento project, or update the composer packages for the Magento project by running the command composer update
2. Run bin/magento setup:di:compile

• Expected result: Magento generated code and dependency injection configuration as expected.

• Actual result: An error is encountered:

Compilation was started.
Application code generator... 3/9 [=========>------------------]  33% 3 secs 362.0 MiB
Fatal error: Class Magento\Framework\Session\SaveHandler\Redis\Config contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Cm\RedisSession\Handler\ConfigInterface::getRetries) in /var/www/html/vendor/magento/framework/Session/SaveHandler/Redis/Config.php on line 16

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-v1.6.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 v1.6.0 and support connection retries for Redis session": "patches/magento-magento2-framework_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v1.6.0.patch"
        },
        "magento/magento2-base": {
            "Github-PR-38729 - Compatible with colinmollenhour/php-redis-session-abstract v1.6.0 and support connection retries for Redis session": "patches/magento-magento2-base_GitHub-PR-38729-compatible-with-collinmollenhour-redis-session-v1.6.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 (*)

• [x] Pull request has a meaningful description of its purpose
• [x] 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[bot]]

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

:exclamation: 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 :information_source: 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 https://github.com/colinmollenhour/php-redis-session-abstract/issues/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:

• https://github.com/magento/magento2/blob/c89312182ef2e747b47348e9cd1f1148ba7af078/composer.json#L41
• https://github.com/magento/magento2/blob/c89312182ef2e747b47348e9cd1f1148ba7af078/lib/internal/Magento/Framework/composer.json#L26

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

[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.