From f4296770befd2da1a16ce01cf7334180c027174b Mon Sep 17 00:00:00 2001 From: "harish.h" Date: Sun, 2 Aug 2020 10:58:05 +0200 Subject: [PATCH 01/15] Issue #3161305 by harishh: composer validate errors and warnings --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ff18a35..3f468b7 100644 --- a/composer.json +++ b/composer.json @@ -1,10 +1,11 @@ { "name": "drupal/redis", "type": "drupal-module", + "description": "Integration of Drupal with the Redis key-value store.", "suggest": { "predis/predis": "^1.1.1" }, - "license": "GPL-2.0", + "license": "GPL-2.0-or-later", "autoload": { "psr-4": { "Drupal\\redis\\": "src" From c07fe76d645000585b4f57a2c7fe3e19b1045f35 Mon Sep 17 00:00:00 2001 From: jonhattan Date: Sun, 2 Aug 2020 11:32:08 +0200 Subject: [PATCH 02/15] Issue #3120363 by jonhattan: Unsupported operand in CacheTagsChecksumTrait --- src/Cache/RedisCacheTagsChecksum.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Cache/RedisCacheTagsChecksum.php b/src/Cache/RedisCacheTagsChecksum.php index cdbae31..a5f03dd 100644 --- a/src/Cache/RedisCacheTagsChecksum.php +++ b/src/Cache/RedisCacheTagsChecksum.php @@ -82,7 +82,8 @@ protected function getTagInvalidationCounts(array $tags) { // The mget command returns the values as an array with numeric keys, // combine it with the tags array to get the expected return value and run // it through intval() to convert to integers and FALSE to 0. - return array_map('intval', array_combine($tags, $this->client->mget($keys))); + $values = $this->client->mget($keys); + return $values ? array_map('intval', array_combine($tags, $values)) : []; } /** From 564f88eecb347248dd37c55a0206167ca07ca371 Mon Sep 17 00:00:00 2001 From: kanei Date: Mon, 28 Sep 2020 18:21:04 +0200 Subject: [PATCH 03/15] Issue #3166814 by mvantuch: [D9] RedisLockTest failing --- redis.info.yml | 1 + tests/src/Kernel/RedisLockTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/redis.info.yml b/redis.info.yml index b227ae2..fbeef12 100644 --- a/redis.info.yml +++ b/redis.info.yml @@ -4,3 +4,4 @@ package: Performance type: module core_version_requirement: ^8.8 || ^9 configure: redis.admin_display +php: 7.1.0 diff --git a/tests/src/Kernel/RedisLockTest.php b/tests/src/Kernel/RedisLockTest.php index f3a9a8e..7594fcf 100644 --- a/tests/src/Kernel/RedisLockTest.php +++ b/tests/src/Kernel/RedisLockTest.php @@ -41,7 +41,7 @@ public function register(ContainerBuilder $container) { /** * {@inheritdoc} */ - protected function setUp() { + protected function setUp(): void { parent::setUp(); $this->lock = $this->container->get('lock'); } From f5863f27c1883837cfe01835757dc18f614e7db4 Mon Sep 17 00:00:00 2001 From: bceyssens Date: Wed, 6 Jan 2021 23:11:01 +0100 Subject: [PATCH 04/15] Issue #3126631 by sharma.amitt16, bceyssens, shaktik: Redis report page is missing a template (D9) --- src/Controller/ReportController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controller/ReportController.php b/src/Controller/ReportController.php index 4a4415c..100beba 100755 --- a/src/Controller/ReportController.php +++ b/src/Controller/ReportController.php @@ -65,7 +65,7 @@ public static function create(ContainerInterface $container) { public function overview() { $build['report'] = [ - '#theme' => 'status_report', + '#type' => 'status_report', '#requirements' => [], ]; From 9a9a8f525c3cd644ed46f05e06750eda68ec25f8 Mon Sep 17 00:00:00 2001 From: berdir Date: Wed, 6 Jan 2021 23:13:07 +0100 Subject: [PATCH 05/15] Issue #3174685 by hussainweb, Berdir: Required parameter follows optional parameter deprecation in PHP 8 --- src/Cache/CacheBase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Cache/CacheBase.php b/src/Cache/CacheBase.php index fbe5089..61b2d74 100644 --- a/src/Cache/CacheBase.php +++ b/src/Cache/CacheBase.php @@ -357,7 +357,7 @@ protected function expandEntry(array $values, $allow_invalid) { * * @return array */ - protected function createEntryHash($cid, $data, $expire = Cache::PERMANENT, array $tags) { + protected function createEntryHash($cid, $data, $expire, array $tags) { // Always add a cache tag for the current bin, so that we can use that for // invalidateAll(). $tags[] = $this->getTagForBin(); From efe45dbc203859e5d4dca4b663f2fa0f3abc12f1 Mon Sep 17 00:00:00 2001 From: eelkeblok Date: Wed, 6 Jan 2021 23:16:59 +0100 Subject: [PATCH 06/15] Issue #2882796 by anish.a, eelkeblok, Dave Reid, jmullikin, carolpettirossi, Berdir: Add support for persistent connections to redis host --- README.md | 7 +++++++ src/Client/PhpRedis.php | 9 +++++++-- src/Client/Predis.php | 17 ++++++++++------- src/ClientFactory.php | 8 ++++++-- src/ClientInterface.php | 2 +- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index a3ba530..914b520 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,13 @@ If your Redis instance is remote, you can use this syntax: Port is optional, default is 6379 (default Redis port). +Use persistent connections +-------------------------- + +This mode needs the following setting: + + $settings['redis.connection']['persistent'] = TRUE; + Compression ------------------------- Compressing the data stored in redis can massively reduce the nedeed storage. diff --git a/src/Client/PhpRedis.php b/src/Client/PhpRedis.php index f9fc11c..7fe1141 100644 --- a/src/Client/PhpRedis.php +++ b/src/Client/PhpRedis.php @@ -14,7 +14,7 @@ class PhpRedis implements ClientInterface { /** * {@inheritdoc} */ - public function getClient($host = NULL, $port = NULL, $base = NULL, $password = NULL) { + public function getClient($host = NULL, $port = NULL, $base = NULL, $password = NULL, $replicationHosts = [], $persistent = FALSE) { $client = new \Redis(); // Sentinel mode, get the real master. @@ -25,7 +25,12 @@ public function getClient($host = NULL, $port = NULL, $base = NULL, $password = } } - $client->connect($host, $port); + if ($persistent) { + $client->pconnect($host, $port); + } + else { + $client->connect($host, $port); + } if (isset($password)) { $client->auth($password); diff --git a/src/Client/Predis.php b/src/Client/Predis.php index 747d73a..9d5d444 100644 --- a/src/Client/Predis.php +++ b/src/Client/Predis.php @@ -11,12 +11,13 @@ */ class Predis implements ClientInterface { - public function getClient($host = NULL, $port = NULL, $base = NULL, $password = NULL, $replicationHosts = NULL) { + public function getClient($host = NULL, $port = NULL, $base = NULL, $password = NULL, $replicationHosts = [], $persistent = FALSE) { $connectionInfo = [ 'password' => $password, 'host' => $host, 'port' => $port, - 'database' => $base + 'database' => $base, + 'persistent' => $persistent ]; foreach ($connectionInfo as $key => $value) { @@ -32,17 +33,19 @@ public function getClient($host = NULL, $port = NULL, $base = NULL, $password = date_default_timezone_set(@date_default_timezone_get()); // If we are passed in an array of $replicationHosts, we should attempt a clustered client connection. - if ($replicationHosts !== NULL) { + if (!empty($replicationHosts)) { $parameters = []; foreach ($replicationHosts as $replicationHost) { + $param = 'tcp://' . $replicationHost['host'] . ':' . $replicationHost['port'] + . '?persistent=' . (($persistent) ? 'true' : 'false'); + // Configure master. if ($replicationHost['role'] === 'primary') { - $parameters[] = 'tcp://' . $replicationHost['host'] . ':' . $replicationHost['port'] . '?alias=master'; - } - else { - $parameters[] = 'tcp://' . $replicationHost['host'] . ':' . $replicationHost['port']; + $param .= '&alias=master'; } + + $parameters[] = $param; } $options = ['replication' => true]; diff --git a/src/ClientFactory.php b/src/ClientFactory.php index e8df702..0120ebc 100644 --- a/src/ClientFactory.php +++ b/src/ClientFactory.php @@ -150,6 +150,7 @@ public static function getClient() { 'port' => self::REDIS_DEFAULT_PORT, 'base' => self::REDIS_DEFAULT_BASE, 'password' => self::REDIS_DEFAULT_PASSWORD, + 'persistent' => FALSE, ]; // If using replication, lets create the client appropriately. @@ -165,14 +166,17 @@ public static function getClient() { $settings['port'], $settings['base'], $settings['password'], - $settings['replication.host']); + $settings['replication.host'], + $settings['persistent']); } else { self::$_client = self::getClientInterface()->getClient( $settings['host'], $settings['port'], $settings['base'], - $settings['password']); + $settings['password'], + [], // There are no replication hosts. + $settings['persistent']); } } diff --git a/src/ClientInterface.php b/src/ClientInterface.php index 38b314f..2ecd895 100644 --- a/src/ClientInterface.php +++ b/src/ClientInterface.php @@ -12,7 +12,7 @@ interface ClientInterface { * @return mixed * Real client depends from the library behind. */ - public function getClient($host = NULL, $port = NULL, $base = NULL); + public function getClient($host = NULL, $port = NULL, $base = NULL, $password = NULL, $replicationHosts = [], $persistent = FALSE); /** * Get underlying library name used. From 943bf35cc79199c0d256093e1275894e935404c7 Mon Sep 17 00:00:00 2001 From: cafuego Date: Wed, 6 Jan 2021 23:25:28 +0100 Subject: [PATCH 07/15] Issue #3181377 by cafuego: Updated status message when not configured --- redis.install | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis.install b/redis.install index 3730606..173f6b6 100644 --- a/redis.install +++ b/redis.install @@ -32,7 +32,7 @@ function redis_requirements($phase) { 'title' => "Redis", 'value' => t("Not connected."), 'severity' => REQUIREMENT_WARNING, - 'description' => t("No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it."), + 'description' => t("No Redis client connected, this module is therefore not used. Ensure that Redis is configured correctly, or disable this module."), ]; } From bc2314e94b5ffecfe70d4957ef477bae8ae8df6d Mon Sep 17 00:00:00 2001 From: hugronaphor Date: Wed, 6 Jan 2021 23:28:16 +0100 Subject: [PATCH 08/15] Issue #3161250 by hugronaphor, mhavelant: Undefined index notices on the report page --- src/Controller/ReportController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controller/ReportController.php b/src/Controller/ReportController.php index 100beba..9ff362a 100755 --- a/src/Controller/ReportController.php +++ b/src/Controller/ReportController.php @@ -171,9 +171,9 @@ public function overview() { if ($memory_config['maxmemory']) { $memory_value = $this->t('@used_memory / @max_memory (@used_percentage%), maxmemory policy: @policy', [ - '@used_memory' => $info['used_memory_human'], + '@used_memory' => $info['used_memory_human'] ?? $info['Memory']['used_memory_human'], '@max_memory' => format_size($memory_config['maxmemory']), - '@used_percentage' => (int) ($info['used_memory'] / $memory_config['maxmemory'] * 100), + '@used_percentage' => (int) ($info['used_memory'] ?? $info['Memory']['used_memory'] / $memory_config['maxmemory'] * 100), '@policy' => $memory_config['maxmemory-policy'], ]); } From 28fa5429413058bdb857d84cb41d681814e9c340 Mon Sep 17 00:00:00 2001 From: Florent Torregrosa Date: Fri, 29 Oct 2021 15:14:51 +0200 Subject: [PATCH 09/15] Issue #2876099 by tobby, Dave Reid: Create submodule to store PHP sessions in Redis --- example.services.yml | 7 + modules/redis_sessions/README.md | 44 +++ .../redis_sessions/redis_sessions.info.yml | 7 + modules/redis_sessions/redis_sessions.install | 49 ++++ .../redis_sessions.services.yml | 8 + .../src/RedisSessionsSessionManager.php | 266 ++++++++++++++++++ src/Session/SessionManager.php | 262 +++++++++++++++++ 7 files changed, 643 insertions(+) create mode 100644 modules/redis_sessions/README.md create mode 100644 modules/redis_sessions/redis_sessions.info.yml create mode 100644 modules/redis_sessions/redis_sessions.install create mode 100644 modules/redis_sessions/redis_sessions.services.yml create mode 100644 modules/redis_sessions/src/RedisSessionsSessionManager.php create mode 100644 src/Session/SessionManager.php diff --git a/example.services.yml b/example.services.yml index 74a05f6..513e11e 100644 --- a/example.services.yml +++ b/example.services.yml @@ -31,3 +31,10 @@ services: flood: class: Drupal\Core\Flood\FloodInterface factory: ['@redis.flood.factory', get] + + # Replaces the session manager with a redis implementation. + session_manager: + class: Drupal\redis\Session\SessionManager + arguments: ['@request_stack', '@redis.factory', '@session_manager.metadata_bag', '@session_configuration', '@session_handler'] + calls: + - [setWriteSafeHandler, ['@session_handler.write_safe']] diff --git a/modules/redis_sessions/README.md b/modules/redis_sessions/README.md new file mode 100644 index 0000000..256df1a --- /dev/null +++ b/modules/redis_sessions/README.md @@ -0,0 +1,44 @@ +CONTENTS OF THIS FILE +--------------------- + + * Introduction + * Requirements + * Installation + * Configuration + + +INTRODUCTION +------------ +The Redis Sessions module creates an alternative to database storage for user +sessions. It uses native a PHP Redis sessions manager and custom settings to +use Redis for sessions handling and storage. + + +REQUIREMENTS +------------ + +This module requires the following modules: + + * Redis (https://drupal.org/project/redis) + + +INSTALLATION +------------ + + * Install as you would normally install a contributed Drupal module. See: + https://www.drupal.org/docs/8/extending-drupal-8/installing-modules + for further information. + + + +CONFIGURATION +------------- + + * By default, Redis Sessions will attempt to use the redis.connection host. + * OPTIONAL: You can add the save_path to your settings.php file, especially if + you want to use a different Redis service than what is used for cache. + ``` + $settings['redis_sessions'] = [ + 'save_path' => 'tcp://redis:6379', + ]; + ``` diff --git a/modules/redis_sessions/redis_sessions.info.yml b/modules/redis_sessions/redis_sessions.info.yml new file mode 100644 index 0000000..321e076 --- /dev/null +++ b/modules/redis_sessions/redis_sessions.info.yml @@ -0,0 +1,7 @@ +name: "Redis Sessions" +description: "A module to change PHP's session handling to use Redis" +package: "Performance and scalability" +type: module +core_version_requirement: ^8.8 || ^9 +dependencies: + - redis \ No newline at end of file diff --git a/modules/redis_sessions/redis_sessions.install b/modules/redis_sessions/redis_sessions.install new file mode 100644 index 0000000..862fb8e --- /dev/null +++ b/modules/redis_sessions/redis_sessions.install @@ -0,0 +1,49 @@ + "Redis Sessions", + 'value' => t("Connected, using the @name client.", array('@name' => ClientFactory::getClientName())), + 'severity' => REQUIREMENT_OK, + ); + } + else { + $requirements['redis_sessions_redis'] = array( + 'title' => "Redis Sessions", + 'value' => t("Not connected."), + 'severity' => REQUIREMENT_WARNING, + 'description' => t("No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it."), + ); + } + + $settings = \Drupal\Core\Site\Settings::get('redis_sessions'); + if (empty($settings['save_path'])) { + $requirements['redis_sessions_save_path'] = array( + 'title' => "Redis Sessions", + 'value' => t("Redis Sessions has not been configured with a save_path setting. See the CONFIGURATION section of this module's README.md."), + 'severity' => REQUIREMENT_OK, + ); + } + + return $requirements; +} diff --git a/modules/redis_sessions/redis_sessions.services.yml b/modules/redis_sessions/redis_sessions.services.yml new file mode 100644 index 0000000..4eb30a8 --- /dev/null +++ b/modules/redis_sessions/redis_sessions.services.yml @@ -0,0 +1,8 @@ +services: + + # Decorate the core session_manager service to use our extended class. + redis_sessions.session_manager: + class: Drupal\redis_sessions\RedisSessionsSessionManager + decorates: session_manager + decoration_priority: -10 + arguments: ['@redis_sessions.session_manager.inner', '@request_stack', '@database', '@session_manager.metadata_bag', '@session_configuration', '@session_handler'] diff --git a/modules/redis_sessions/src/RedisSessionsSessionManager.php b/modules/redis_sessions/src/RedisSessionsSessionManager.php new file mode 100644 index 0000000..bf3b8c5 --- /dev/null +++ b/modules/redis_sessions/src/RedisSessionsSessionManager.php @@ -0,0 +1,266 @@ +innerService = $session_manager; + parent::__construct($request_stack, $connection, $metadata_bag, $session_configuration, $handler); + + $save_path = $this->getSavePath(); + if (ClientFactory::hasClient()) { + if (!empty($save_path)) { + ini_set('session.save_path', $save_path); + ini_set('session.save_handler', 'redis'); + $this->redis = ClientFactory::getClient(); + } + else { + throw new \Exception("Redis Sessions has not been configured. See 'CONFIGURATION' in README.md in the redis_sessions module for instructions."); + } + } + else { + throw new \Exception("Redis client is not found. Is Redis module enabled and configured?"); + } + } + + /** + * Return the session.save_path string for PHP native session handling. + * + * Get save_path from site settings, since we can't inject it into the + * service directly. + * + * @return string + * A string of the full URL to the redis service. + */ + private function getSavePath() { + // Use the save_path value from settings.php first. + $settings = \Drupal\Core\Site\Settings::get('redis_sessions'); + if ($settings['save_path']) { + $save_path = $settings['save_path']; + } + else { + // If no save_path from settings.php, use Redis module's settings. + $settings = \Drupal\Core\Site\Settings::get('redis.connection'); + $save_path = "tcp://${settings['host']}:6379"; + } + + return $save_path; + } + + /** + * Return a key prefix to use in redis keys. + * + * @return string + * A string of the redis key prefix, with a trailing colon. + */ + private function getNativeSessionKey($suffix = '') { + // TODO: Get the string from a config option, or use the default string. + return 'PHPREDIS_SESSION:' . $suffix; + } + + /** + * Return the redis key for the current session ID. + * + * @return string + * A string of the redis key for the current session ID. + */ + private function getKey() { + return $this->getNativeSessionKey($this->getId()); + } + + /** + * Return a Drupal-specific key prefix to use in redis keys. + * + * @return string + * A string of the redis key prefix, with a trailing colon. + */ + private function getUidSessionKeyPrefix($suffix = '') { + // TODO: Get Redis module prefix value to add to the $sid Redis key prefix. + // TODO: Get the string from a config option, or use the default string. + return 'DRUPAL_REDIS_SESSION:' . $suffix; + } + + /** + * Return the redis key for the current session ID. + * + * @return string + * A string of the redis key for the current session ID. + */ + private function getUidSessionKey() { + $uid = $this->getSessionBagUid(); + return $this->getUidSessionKeyPrefix(Crypt::hashBase64($uid)); + } + + /** + * Get the User ID from the session metadata bags. + * + * Fetch the User ID from the metadata bags rather than a tradtional user + * lookup in case the UID is in the process of changing (logging in or out). + * + * @return int + * User id as passed to the constructor in a metadata bag. + */ + private function getSessionBagUid() { + foreach ($this->bags as $bag) { + if ($bag->getName() == 'attributes') { + $bag = $bag->getBag(); + $attributes = $bag->all(); + if (!empty($attributes['uid'])) { + return $attributes['uid']; + } + } + } + return 0; + } + + /** + * {@inheritdoc} + */ + public function isSessionObsolete() { + $bag_uid = $this->getSessionBagUid(); + $current_uid = \Drupal::currentUser()->id(); + + return ($bag_uid == 0 && $current_uid == 0); + } + + /** + * {@inheritdoc} + */ + public function save() { + $uid = $this->getSessionBagUid(); + + // Write the session data. + parent::save(); + + // Write a key:value pair to be able to find the UID by the SID later. + // NOTE: Checking for $uid here ensures that only sessions for logged-in + // users will have lookup keys. Anonymous sessions (if they exist at all) + // are transient and will be cleaned up via garbage collection. + // TODO: Add EX Seconds to the set() method for session life length. + // TODO: After adding EX and PX seconds, add 'NX'. + // See: https://redis.io/commands/set. + if ($uid) { + if (\Drupal::currentUser()->id()) { + $this->redis->set($this->getUidSessionKey(), $this->getKey()); + } + else { + $this->destroyObsolete($this->redis->get($this->getUidSessionKey())); + } + } + } + + /** + * {@inheritdoc} + */ + public function delete($uid) { + // Nothing to do if we are not allowed to change the session. + if ($this->isCli() || $this->innerService->isCli()) { + return; + } + + // Get the session key by $uid. + $sid = $this->redis->get($this->getUidSessionKey()); + + // Delete both key/value pairs associated with the session ID. + $this->redis->del($sid); + $this->redis->del($this->getKey()); + } + + /** + * {@inheritdoc} + */ + public function destroy() { + $uid = $this->getSessionBagUid(); + $this->redis->set("SESS_DESTROY:$uid:" . \Drupal::currentUser()->id()); + + if ($uid) { + if (\Drupal::currentUser()->id() == 0) { + $sid = $this->redis->get($this->getUidSessionKey()); + + $this->redis->del($sid); + $this->redis->del($this->getUidSessionKey()); + $this->redis->del($this->getKey()); + } + } + + $this->innerService->destroy(); + } + + /** + * Removes obsolete sessions. + * + * @param string $old_session_id + * The old session ID. + */ + public function destroyObsolete($old_session_id) { + $this->redis->del($old_session_id); + $this->redis->del($this->getUidSessionKey()); + } + + /** + * Migrates the current session to a new session id. + * + * @param string $old_session_id + * The old session ID. The new session ID is $this->getId(). + * + * @see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Session%21SessionManager.php/function/SessionManager%3A%3AmigrateStoredSession/8.2.x + */ + protected function migrateStoredSession($old_session_id) { + // The original session has been copied to a new session with a new key; + // remove the original session ID key. + // Test: redis-cli KEYS "*SESS*" | xargs redis-cli DEL && redis-cli. + $this->redis->del($this->getNativeSessionKey($old_session_id)); + } + +} diff --git a/src/Session/SessionManager.php b/src/Session/SessionManager.php new file mode 100644 index 0000000..89990bb --- /dev/null +++ b/src/Session/SessionManager.php @@ -0,0 +1,262 @@ +clientFactory = $clientFactory; + $this->redis = $this->clientFactory->getClient(); + if ($this->clientFactory->getClientName() == 'PhpRedis') { + ini_set('session.save_path', $this->getSavePath()); + ini_set('session.save_handler', 'redis'); + } + elseif ($this->clientFactory->getClientName() == 'Predis') { + $handler = new \Predis\Session\Handler($this->redis, array('gc_maxlifetime' => 5)); + $handler->register(); + } + } + + /** + * Return the session.save_path string for PHP native session handling. + * + * Get save_path from site settings, since we can't inject it into the + * service directly. + * + * @return string + * A string of the full URL to the redis service. + */ + private function getSavePath() { + // Use the save_path value from settings.php first. + $settings = \Drupal\Core\Site\Settings::get('redis_sessions'); + if ($settings['save_path']) { + $save_path = $settings['save_path']; + } + else { + // If no save_path from settings.php, use Redis module's settings. + $settings = \Drupal\Core\Site\Settings::get('redis.connection'); + $settings += [ + 'port' => '6379', + ]; + $save_path = "tcp://${settings['host']}:${settings['port']}"; + } + + return $save_path; + } + + /** + * Return a key prefix to use in redis keys. + * + * @return string + * A string of the redis key prefix, with a trailing colon. + */ + private function getNativeSessionKey($suffix = '') { + // TODO: Get the string from a config option, or use the default string. + return 'PHPREDIS_SESSION:' . $suffix; + } + + /** + * Return the redis key for the current session ID. + * + * @return string + * A string of the redis key for the current session ID. + */ + private function getKey() { + return $this->getNativeSessionKey($this->getId()); + } + + /** + * Return a Drupal-specific key prefix to use in redis keys. + * + * @return string + * A string of the redis key prefix, with a trailing colon. + */ + private function getUidSessionKeyPrefix($suffix = '') { + // TODO: Get Redis module prefix value to add to the $sid Redis key prefix. + // TODO: Get the string from a config option, or use the default string. + return 'DRUPAL_REDIS_SESSION:' . $suffix; + } + + /** + * Return the redis key for the current session ID. + * + * @return string + * A string of the redis key for the current session ID. + */ + private function getUidSessionKey() { + $uid = $this->getSessionBagUid(); + return $this->getUidSessionKeyPrefix(Crypt::hashBase64($uid)); + } + + /** + * Get the User ID from the session metadata bags. + * + * Fetch the User ID from the metadata bags rather than a traditional user + * lookup in case the UID is in the process of changing (logging in or out). + * + * @return int + * User id as passed to the constructor in a metadata bag. + */ + private function getSessionBagUid() { + foreach ($this->bags as $bag) { + // In Drupal 8.5 and above, the bag may be a proxy, in which case we need to get the actual bag. + if (method_exists($bag, 'getBag')) { + $bag = $bag->getBag(); + } + if ($bag instanceof AttributeBagInterface && $bag->has('uid')) { + return (int) $bag->get('uid'); + } + } + return 0; + } + + /** + * {@inheritdoc} + */ + public function isSessionObsolete() { + $bag_uid = $this->getSessionBagUid(); + $current_uid = \Drupal::currentUser()->id(); + return ($bag_uid == 0 && $current_uid == 0); + } + + /** + * {@inheritdoc} + */ + public function save() { + if ($this->isCli()) { + // We don't have anything to do if we are not allowed to save the session. + return; + } + + if ($this->isSessionObsolete()) { + // There is no session data to store, destroy the session if it was + // previously started. + if ($this->getSaveHandler()->isActive()) { + $this->destroy(); + } + } + else { + // There is session data to store. Start the session if it is not already + // started. + if (!$this->getSaveHandler()->isActive()) { + $this->startNow(); + } + // Write the session data. + parent::save(); + } + + $this->startedLazy = FALSE; + + // Write a key:value pair to be able to find the UID by the SID later. + // NOTE: Checking for $uid here ensures that only sessions for logged-in + // users will have lookup keys. Anonymous sessions (if they exist at all) + // are transient and will be cleaned up via garbage collection. + // TODO: Add EX Seconds to the set() method for session life length. + // TODO: After adding EX and PX seconds, add 'NX'. + // See: https://redis.io/commands/set. + if ($this->getSessionBagUid()) { + if (\Drupal::currentUser()->id()) { + $this->redis->set($this->getUidSessionKey(), $this->getKey()); + } + else { + $this->destroyObsolete($this->redis->get($this->getUidSessionKey())); + } + } + } + + /** + * {@inheritdoc} + */ + public function delete($uid) { + // Nothing to do if we are not allowed to change the session. + if ($this->isCli()) { + return; + } + + // Get the session key by $uid. + $sid = $this->redis->get($this->getUidSessionKey()); + + // Delete both key/value pairs associated with the session ID. + $this->redis->del($sid); + $this->redis->del($this->getKey()); + } + + /** + * {@inheritdoc} + */ + public function destroy() { + $uid = $this->getSessionBagUid(); + $this->redis->del("SESS_DESTROY:$uid:" . \Drupal::currentUser()->id()); + + if ($uid) { + if (\Drupal::currentUser()->id() == 0) { + $sid = $this->redis->get($this->getUidSessionKey()); + + $this->redis->del($sid); + $this->redis->del($this->getUidSessionKey()); + $this->redis->del($this->getKey()); + } + } + + parent::destroy(); + } + + /** + * Removes obsolete sessions. + * + * @param string $old_session_id + * The old session ID. + */ + public function destroyObsolete($old_session_id) { + $this->redis->del($old_session_id); + $this->redis->del($this->getUidSessionKey()); + } + + /** + * Migrates the current session to a new session id. + * + * @param string $old_session_id + * The old session ID. The new session ID is $this->getId(). + * + * @see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Session%21SessionManager.php/function/SessionManager%3A%3AmigrateStoredSession/8.2.x + */ + protected function migrateStoredSession($old_session_id) { + // The original session has been copied to a new session with a new key; + // remove the original session ID key. + // Test: redis-cli KEYS "*SESS*" | xargs redis-cli DEL && redis-cli. + $this->redis->del($this->getNativeSessionKey($old_session_id)); + } + +} From 0cd624d140cba79e73c1c045c05afe2b59a2e6b3 Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Fri, 29 Oct 2021 15:20:31 +0200 Subject: [PATCH 10/15] Issue #2876099 by Grimreaper: Some code cleanup. --- .../redis_sessions/redis_sessions.info.yml | 2 +- modules/redis_sessions/redis_sessions.install | 42 ++++++++++--------- .../redis_sessions.services.yml | 8 +++- .../src/RedisSessionsSessionManager.php | 20 ++++----- src/Session/SessionManager.php | 21 ++++++---- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/modules/redis_sessions/redis_sessions.info.yml b/modules/redis_sessions/redis_sessions.info.yml index 321e076..105a1cc 100644 --- a/modules/redis_sessions/redis_sessions.info.yml +++ b/modules/redis_sessions/redis_sessions.info.yml @@ -4,4 +4,4 @@ package: "Performance and scalability" type: module core_version_requirement: ^8.8 || ^9 dependencies: - - redis \ No newline at end of file + - redis:redis diff --git a/modules/redis_sessions/redis_sessions.install b/modules/redis_sessions/redis_sessions.install index 862fb8e..902d751 100644 --- a/modules/redis_sessions/redis_sessions.install +++ b/modules/redis_sessions/redis_sessions.install @@ -5,44 +5,46 @@ * Redis install related functions. */ -use \Drupal\redis\ClientFactory; +use Drupal\Core\Site\Settings; +use Drupal\redis\ClientFactory; /** * Implements hook_requirements(). */ function redis_sessions_requirements($phase) { + $requirements = []; // This module is configured via settings.php file. Using any other phase // than runtime to proceed to some consistency checks is useless. if ('runtime' !== $phase) { - return array(); + return $requirements; } - $requirements = array(); - if (ClientFactory::hasClient()) { - $requirements['redis_sessions_redis'] = array( - 'title' => "Redis Sessions", - 'value' => t("Connected, using the @name client.", array('@name' => ClientFactory::getClientName())), - 'severity' => REQUIREMENT_OK, - ); + $requirements['redis_sessions_redis'] = [ + 'title' => "Redis Sessions", + 'value' => t("Connected, using the @name client.", [ + '@name' => ClientFactory::getClientName(), + ]), + 'severity' => REQUIREMENT_OK, + ]; } else { - $requirements['redis_sessions_redis'] = array( - 'title' => "Redis Sessions", - 'value' => t("Not connected."), - 'severity' => REQUIREMENT_WARNING, + $requirements['redis_sessions_redis'] = [ + 'title' => "Redis Sessions", + 'value' => t("Not connected."), + 'severity' => REQUIREMENT_WARNING, 'description' => t("No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it."), - ); + ]; } - $settings = \Drupal\Core\Site\Settings::get('redis_sessions'); + $settings = Settings::get('redis_sessions'); if (empty($settings['save_path'])) { - $requirements['redis_sessions_save_path'] = array( - 'title' => "Redis Sessions", - 'value' => t("Redis Sessions has not been configured with a save_path setting. See the CONFIGURATION section of this module's README.md."), - 'severity' => REQUIREMENT_OK, - ); + $requirements['redis_sessions_save_path'] = [ + 'title' => "Redis Sessions", + 'value' => t("Redis Sessions has not been configured with a save_path setting. See the CONFIGURATION section of this module's README.md."), + 'severity' => REQUIREMENT_OK, + ]; } return $requirements; diff --git a/modules/redis_sessions/redis_sessions.services.yml b/modules/redis_sessions/redis_sessions.services.yml index 4eb30a8..799a49e 100644 --- a/modules/redis_sessions/redis_sessions.services.yml +++ b/modules/redis_sessions/redis_sessions.services.yml @@ -5,4 +5,10 @@ services: class: Drupal\redis_sessions\RedisSessionsSessionManager decorates: session_manager decoration_priority: -10 - arguments: ['@redis_sessions.session_manager.inner', '@request_stack', '@database', '@session_manager.metadata_bag', '@session_configuration', '@session_handler'] + arguments: + - '@redis_sessions.session_manager.inner' + - '@request_stack' + - '@database' + - '@session_manager.metadata_bag' + - '@session_configuration' + - '@session_handler' diff --git a/modules/redis_sessions/src/RedisSessionsSessionManager.php b/modules/redis_sessions/src/RedisSessionsSessionManager.php index bf3b8c5..e8ea4bb 100644 --- a/modules/redis_sessions/src/RedisSessionsSessionManager.php +++ b/modules/redis_sessions/src/RedisSessionsSessionManager.php @@ -1,14 +1,10 @@ id()) { diff --git a/src/Session/SessionManager.php b/src/Session/SessionManager.php index 89990bb..0fcf28e 100644 --- a/src/Session/SessionManager.php +++ b/src/Session/SessionManager.php @@ -6,7 +6,9 @@ use Drupal\Core\Session\MetadataBag; use Drupal\Core\Session\SessionConfigurationInterface; use Drupal\Core\Session\SessionManager as CoreSessionManager; +use Drupal\Core\Site\Settings; use Drupal\redis\ClientFactory; +use Predis\Session\Handler; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface; @@ -44,7 +46,7 @@ public function __construct(RequestStack $request_stack, ClientFactory $clientFa ini_set('session.save_handler', 'redis'); } elseif ($this->clientFactory->getClientName() == 'Predis') { - $handler = new \Predis\Session\Handler($this->redis, array('gc_maxlifetime' => 5)); + $handler = new Handler($this->redis, array('gc_maxlifetime' => 5)); $handler->register(); } } @@ -60,13 +62,13 @@ public function __construct(RequestStack $request_stack, ClientFactory $clientFa */ private function getSavePath() { // Use the save_path value from settings.php first. - $settings = \Drupal\Core\Site\Settings::get('redis_sessions'); + $settings = Settings::get('redis_sessions'); if ($settings['save_path']) { $save_path = $settings['save_path']; } else { // If no save_path from settings.php, use Redis module's settings. - $settings = \Drupal\Core\Site\Settings::get('redis.connection'); + $settings = Settings::get('redis.connection'); $settings += [ 'port' => '6379', ]; @@ -83,7 +85,7 @@ private function getSavePath() { * A string of the redis key prefix, with a trailing colon. */ private function getNativeSessionKey($suffix = '') { - // TODO: Get the string from a config option, or use the default string. + // @todo Get the string from a config option, or use the default string. return 'PHPREDIS_SESSION:' . $suffix; } @@ -104,8 +106,8 @@ private function getKey() { * A string of the redis key prefix, with a trailing colon. */ private function getUidSessionKeyPrefix($suffix = '') { - // TODO: Get Redis module prefix value to add to the $sid Redis key prefix. - // TODO: Get the string from a config option, or use the default string. + // @todo Get Redis module prefix value to add to the $sid Redis key prefix. + // @todo Get the string from a config option, or use the default string. return 'DRUPAL_REDIS_SESSION:' . $suffix; } @@ -131,7 +133,8 @@ private function getUidSessionKey() { */ private function getSessionBagUid() { foreach ($this->bags as $bag) { - // In Drupal 8.5 and above, the bag may be a proxy, in which case we need to get the actual bag. + // In Drupal 8.5 and above, the bag may be a proxy, in which case we need + // to get the actual bag. if (method_exists($bag, 'getBag')) { $bag = $bag->getBag(); } @@ -183,8 +186,8 @@ public function save() { // NOTE: Checking for $uid here ensures that only sessions for logged-in // users will have lookup keys. Anonymous sessions (if they exist at all) // are transient and will be cleaned up via garbage collection. - // TODO: Add EX Seconds to the set() method for session life length. - // TODO: After adding EX and PX seconds, add 'NX'. + // @todo Add EX Seconds to the set() method for session life length. + // @todo After adding EX and PX seconds, add 'NX'. // See: https://redis.io/commands/set. if ($this->getSessionBagUid()) { if (\Drupal::currentUser()->id()) { From 24b87475f9d99917161b37e41b0e0bd2ed4a3f29 Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Fri, 29 Oct 2021 15:23:56 +0200 Subject: [PATCH 11/15] Issue #2876099 by Grimreaper: Remove hardcoded port. --- modules/redis_sessions/src/RedisSessionsSessionManager.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/redis_sessions/src/RedisSessionsSessionManager.php b/modules/redis_sessions/src/RedisSessionsSessionManager.php index e8ea4bb..e934d8c 100644 --- a/modules/redis_sessions/src/RedisSessionsSessionManager.php +++ b/modules/redis_sessions/src/RedisSessionsSessionManager.php @@ -88,7 +88,10 @@ private function getSavePath() { else { // If no save_path from settings.php, use Redis module's settings. $settings = Settings::get('redis.connection'); - $save_path = "tcp://${settings['host']}:6379"; + $settings += [ + 'port' => '6379', + ]; + $save_path = "tcp://${settings['host']}:${settings['port']}"; } return $save_path; From b1da9efe7c3b93df99cd221044bce86ce6431a45 Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Thu, 4 Nov 2021 18:19:03 +0100 Subject: [PATCH 12/15] Issue #2876099 by Grimreaper: WIP Declare only one service in submodule which decorates. Clean dependency injection. --- example.services.yml | 7 - modules/redis_sessions/README.md | 4 +- .../redis_sessions/redis_sessions.info.yml | 4 +- modules/redis_sessions/redis_sessions.install | 17 +- .../redis_sessions.services.yml | 20 +- .../src/RedisSessionsSessionManager.php | 265 ------------------ .../Session/RedisSessionsSessionManager.php | 97 ++++--- redis.info.yml | 6 +- 8 files changed, 82 insertions(+), 338 deletions(-) delete mode 100644 modules/redis_sessions/src/RedisSessionsSessionManager.php rename src/Session/SessionManager.php => modules/redis_sessions/src/Session/RedisSessionsSessionManager.php (74%) diff --git a/example.services.yml b/example.services.yml index 513e11e..74a05f6 100644 --- a/example.services.yml +++ b/example.services.yml @@ -31,10 +31,3 @@ services: flood: class: Drupal\Core\Flood\FloodInterface factory: ['@redis.flood.factory', get] - - # Replaces the session manager with a redis implementation. - session_manager: - class: Drupal\redis\Session\SessionManager - arguments: ['@request_stack', '@redis.factory', '@session_manager.metadata_bag', '@session_configuration', '@session_handler'] - calls: - - [setWriteSafeHandler, ['@session_handler.write_safe']] diff --git a/modules/redis_sessions/README.md b/modules/redis_sessions/README.md index 256df1a..6fdbd0b 100644 --- a/modules/redis_sessions/README.md +++ b/modules/redis_sessions/README.md @@ -9,8 +9,9 @@ CONTENTS OF THIS FILE INTRODUCTION ------------ + The Redis Sessions module creates an alternative to database storage for user -sessions. It uses native a PHP Redis sessions manager and custom settings to +sessions. It uses a PHP Redis sessions manager and custom settings to use Redis for sessions handling and storage. @@ -30,7 +31,6 @@ INSTALLATION for further information. - CONFIGURATION ------------- diff --git a/modules/redis_sessions/redis_sessions.info.yml b/modules/redis_sessions/redis_sessions.info.yml index 105a1cc..e58b491 100644 --- a/modules/redis_sessions/redis_sessions.info.yml +++ b/modules/redis_sessions/redis_sessions.info.yml @@ -1,6 +1,6 @@ name: "Redis Sessions" -description: "A module to change PHP's session handling to use Redis" -package: "Performance and scalability" +description: "A module to change PHP's session handling to use Redis." +package: "Performance" type: module core_version_requirement: ^8.8 || ^9 dependencies: diff --git a/modules/redis_sessions/redis_sessions.install b/modules/redis_sessions/redis_sessions.install index 902d751..6129415 100644 --- a/modules/redis_sessions/redis_sessions.install +++ b/modules/redis_sessions/redis_sessions.install @@ -22,8 +22,8 @@ function redis_sessions_requirements($phase) { if (ClientFactory::hasClient()) { $requirements['redis_sessions_redis'] = [ - 'title' => "Redis Sessions", - 'value' => t("Connected, using the @name client.", [ + 'title' => 'Redis Sessions', + 'value' => t('Connected, using the @name client.', [ '@name' => ClientFactory::getClientName(), ]), 'severity' => REQUIREMENT_OK, @@ -31,17 +31,18 @@ function redis_sessions_requirements($phase) { } else { $requirements['redis_sessions_redis'] = [ - 'title' => "Redis Sessions", - 'value' => t("Not connected."), + 'title' => 'Redis Sessions', + 'value' => t('Not connected.'), 'severity' => REQUIREMENT_WARNING, - 'description' => t("No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it."), + 'description' => t('No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it.'), ]; } - $settings = Settings::get('redis_sessions'); - if (empty($settings['save_path'])) { + /** @var array $settings */ + $settings = Settings::get('redis_sessions', []); + if (!isset($settings['save_path']) || empty($settings['save_path'])) { $requirements['redis_sessions_save_path'] = [ - 'title' => "Redis Sessions", + 'title' => 'Redis Sessions', 'value' => t("Redis Sessions has not been configured with a save_path setting. See the CONFIGURATION section of this module's README.md."), 'severity' => REQUIREMENT_OK, ]; diff --git a/modules/redis_sessions/redis_sessions.services.yml b/modules/redis_sessions/redis_sessions.services.yml index 799a49e..c09ad37 100644 --- a/modules/redis_sessions/redis_sessions.services.yml +++ b/modules/redis_sessions/redis_sessions.services.yml @@ -1,14 +1,16 @@ services: - - # Decorate the core session_manager service to use our extended class. + # Replaces the session manager with a redis implementation. redis_sessions.session_manager: - class: Drupal\redis_sessions\RedisSessionsSessionManager + class: Drupal\redis_sessions\Session\SessionManager decorates: session_manager decoration_priority: -10 - arguments: - - '@redis_sessions.session_manager.inner' - - '@request_stack' - - '@database' - - '@session_manager.metadata_bag' - - '@session_configuration' + arguments: + - '@request_stack', + - '@database', + - '@session_manager.metadata_bag', + - '@session_configuration', + - '@redis.factory', + - '@current_user', - '@session_handler' + calls: + - [setWriteSafeHandler, ['@session_handler.write_safe']] diff --git a/modules/redis_sessions/src/RedisSessionsSessionManager.php b/modules/redis_sessions/src/RedisSessionsSessionManager.php deleted file mode 100644 index e934d8c..0000000 --- a/modules/redis_sessions/src/RedisSessionsSessionManager.php +++ /dev/null @@ -1,265 +0,0 @@ -innerService = $session_manager; - parent::__construct($request_stack, $connection, $metadata_bag, $session_configuration, $handler); - - $save_path = $this->getSavePath(); - if (ClientFactory::hasClient()) { - if (!empty($save_path)) { - ini_set('session.save_path', $save_path); - ini_set('session.save_handler', 'redis'); - $this->redis = ClientFactory::getClient(); - } - else { - throw new \Exception("Redis Sessions has not been configured. See 'CONFIGURATION' in README.md in the redis_sessions module for instructions."); - } - } - else { - throw new \Exception("Redis client is not found. Is Redis module enabled and configured?"); - } - } - - /** - * Return the session.save_path string for PHP native session handling. - * - * Get save_path from site settings, since we can't inject it into the - * service directly. - * - * @return string - * A string of the full URL to the redis service. - */ - private function getSavePath() { - // Use the save_path value from settings.php first. - $settings = Settings::get('redis_sessions'); - if ($settings['save_path']) { - $save_path = $settings['save_path']; - } - else { - // If no save_path from settings.php, use Redis module's settings. - $settings = Settings::get('redis.connection'); - $settings += [ - 'port' => '6379', - ]; - $save_path = "tcp://${settings['host']}:${settings['port']}"; - } - - return $save_path; - } - - /** - * Return a key prefix to use in redis keys. - * - * @return string - * A string of the redis key prefix, with a trailing colon. - */ - private function getNativeSessionKey($suffix = '') { - // @todo Get the string from a config option, or use the default string. - return 'PHPREDIS_SESSION:' . $suffix; - } - - /** - * Return the redis key for the current session ID. - * - * @return string - * A string of the redis key for the current session ID. - */ - private function getKey() { - return $this->getNativeSessionKey($this->getId()); - } - - /** - * Return a Drupal-specific key prefix to use in redis keys. - * - * @return string - * A string of the redis key prefix, with a trailing colon. - */ - private function getUidSessionKeyPrefix($suffix = '') { - // @todo Get Redis module prefix value to add to the $sid Redis key prefix. - // @todo Get the string from a config option, or use the default string. - return 'DRUPAL_REDIS_SESSION:' . $suffix; - } - - /** - * Return the redis key for the current session ID. - * - * @return string - * A string of the redis key for the current session ID. - */ - private function getUidSessionKey() { - $uid = $this->getSessionBagUid(); - return $this->getUidSessionKeyPrefix(Crypt::hashBase64($uid)); - } - - /** - * Get the User ID from the session metadata bags. - * - * Fetch the User ID from the metadata bags rather than a tradtional user - * lookup in case the UID is in the process of changing (logging in or out). - * - * @return int - * User id as passed to the constructor in a metadata bag. - */ - private function getSessionBagUid() { - foreach ($this->bags as $bag) { - if ($bag->getName() == 'attributes') { - $bag = $bag->getBag(); - $attributes = $bag->all(); - if (!empty($attributes['uid'])) { - return $attributes['uid']; - } - } - } - return 0; - } - - /** - * {@inheritdoc} - */ - public function isSessionObsolete() { - $bag_uid = $this->getSessionBagUid(); - $current_uid = \Drupal::currentUser()->id(); - - return ($bag_uid == 0 && $current_uid == 0); - } - - /** - * {@inheritdoc} - */ - public function save() { - $uid = $this->getSessionBagUid(); - - // Write the session data. - parent::save(); - - // Write a key:value pair to be able to find the UID by the SID later. - // NOTE: Checking for $uid here ensures that only sessions for logged-in - // users will have lookup keys. Anonymous sessions (if they exist at all) - // are transient and will be cleaned up via garbage collection. - // @todo Add EX Seconds to the set() method for session life length. - // @todo After adding EX and PX seconds, add 'NX'. - // See: https://redis.io/commands/set. - if ($uid) { - if (\Drupal::currentUser()->id()) { - $this->redis->set($this->getUidSessionKey(), $this->getKey()); - } - else { - $this->destroyObsolete($this->redis->get($this->getUidSessionKey())); - } - } - } - - /** - * {@inheritdoc} - */ - public function delete($uid) { - // Nothing to do if we are not allowed to change the session. - if ($this->isCli() || $this->innerService->isCli()) { - return; - } - - // Get the session key by $uid. - $sid = $this->redis->get($this->getUidSessionKey()); - - // Delete both key/value pairs associated with the session ID. - $this->redis->del($sid); - $this->redis->del($this->getKey()); - } - - /** - * {@inheritdoc} - */ - public function destroy() { - $uid = $this->getSessionBagUid(); - $this->redis->set("SESS_DESTROY:$uid:" . \Drupal::currentUser()->id()); - - if ($uid) { - if (\Drupal::currentUser()->id() == 0) { - $sid = $this->redis->get($this->getUidSessionKey()); - - $this->redis->del($sid); - $this->redis->del($this->getUidSessionKey()); - $this->redis->del($this->getKey()); - } - } - - $this->innerService->destroy(); - } - - /** - * Removes obsolete sessions. - * - * @param string $old_session_id - * The old session ID. - */ - public function destroyObsolete($old_session_id) { - $this->redis->del($old_session_id); - $this->redis->del($this->getUidSessionKey()); - } - - /** - * Migrates the current session to a new session id. - * - * @param string $old_session_id - * The old session ID. The new session ID is $this->getId(). - * - * @see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Session%21SessionManager.php/function/SessionManager%3A%3AmigrateStoredSession/8.2.x - */ - protected function migrateStoredSession($old_session_id) { - // The original session has been copied to a new session with a new key; - // remove the original session ID key. - // Test: redis-cli KEYS "*SESS*" | xargs redis-cli DEL && redis-cli. - $this->redis->del($this->getNativeSessionKey($old_session_id)); - } - -} diff --git a/src/Session/SessionManager.php b/modules/redis_sessions/src/Session/RedisSessionsSessionManager.php similarity index 74% rename from src/Session/SessionManager.php rename to modules/redis_sessions/src/Session/RedisSessionsSessionManager.php index 0fcf28e..606afab 100644 --- a/src/Session/SessionManager.php +++ b/modules/redis_sessions/src/Session/RedisSessionsSessionManager.php @@ -1,11 +1,13 @@ clientFactory = $clientFactory; + $this->clientFactory = $client_factory; + $this->currentUser = $current_user; $this->redis = $this->clientFactory->getClient(); if ($this->clientFactory->getClientName() == 'PhpRedis') { ini_set('session.save_path', $this->getSavePath()); @@ -148,9 +180,9 @@ private function getSessionBagUid() { /** * {@inheritdoc} */ - public function isSessionObsolete() { + protected function isSessionObsolete() { $bag_uid = $this->getSessionBagUid(); - $current_uid = \Drupal::currentUser()->id(); + $current_uid = $this->currentUser->id(); return ($bag_uid == 0 && $current_uid == 0); } @@ -158,29 +190,7 @@ public function isSessionObsolete() { * {@inheritdoc} */ public function save() { - if ($this->isCli()) { - // We don't have anything to do if we are not allowed to save the session. - return; - } - - if ($this->isSessionObsolete()) { - // There is no session data to store, destroy the session if it was - // previously started. - if ($this->getSaveHandler()->isActive()) { - $this->destroy(); - } - } - else { - // There is session data to store. Start the session if it is not already - // started. - if (!$this->getSaveHandler()->isActive()) { - $this->startNow(); - } - // Write the session data. - parent::save(); - } - - $this->startedLazy = FALSE; + parent::save(); // Write a key:value pair to be able to find the UID by the SID later. // NOTE: Checking for $uid here ensures that only sessions for logged-in @@ -190,7 +200,7 @@ public function save() { // @todo After adding EX and PX seconds, add 'NX'. // See: https://redis.io/commands/set. if ($this->getSessionBagUid()) { - if (\Drupal::currentUser()->id()) { + if ($this->currentUser->id()) { $this->redis->set($this->getUidSessionKey(), $this->getKey()); } else { @@ -203,7 +213,6 @@ public function save() { * {@inheritdoc} */ public function delete($uid) { - // Nothing to do if we are not allowed to change the session. if ($this->isCli()) { return; } @@ -220,11 +229,17 @@ public function delete($uid) { * {@inheritdoc} */ public function destroy() { + parent::destroy(); + + if ($this->isCli()) { + return; + } + $uid = $this->getSessionBagUid(); - $this->redis->del("SESS_DESTROY:$uid:" . \Drupal::currentUser()->id()); + $this->redis->del("SESS_DESTROY:$uid:" . $this->currentUser->id()); if ($uid) { - if (\Drupal::currentUser()->id() == 0) { + if ($this->currentUser->id() == 0) { $sid = $this->redis->get($this->getUidSessionKey()); $this->redis->del($sid); @@ -232,8 +247,6 @@ public function destroy() { $this->redis->del($this->getKey()); } } - - parent::destroy(); } /** @@ -242,7 +255,7 @@ public function destroy() { * @param string $old_session_id * The old session ID. */ - public function destroyObsolete($old_session_id) { + protected function destroyObsolete($old_session_id) { $this->redis->del($old_session_id); $this->redis->del($this->getUidSessionKey()); } diff --git a/redis.info.yml b/redis.info.yml index fbeef12..9348aa6 100644 --- a/redis.info.yml +++ b/redis.info.yml @@ -1,6 +1,6 @@ -name: Redis -description: Provide a module placeholder, for using as dependency for module that needs Redis. -package: Performance +name: "Redis" +description: "Provide a module placeholder, for using as dependency for module that needs Redis." +package: "Performance" type: module core_version_requirement: ^8.8 || ^9 configure: redis.admin_display From 81e2fedecd7ef6e0f3d4d060472b46fcd633d460 Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Fri, 5 Nov 2021 08:54:07 +0100 Subject: [PATCH 13/15] Issue #2876099 by Grimreaper: Fix services.yml --- modules/redis_sessions/redis_sessions.services.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/redis_sessions/redis_sessions.services.yml b/modules/redis_sessions/redis_sessions.services.yml index c09ad37..33a9bef 100644 --- a/modules/redis_sessions/redis_sessions.services.yml +++ b/modules/redis_sessions/redis_sessions.services.yml @@ -5,12 +5,12 @@ services: decorates: session_manager decoration_priority: -10 arguments: - - '@request_stack', - - '@database', - - '@session_manager.metadata_bag', - - '@session_configuration', - - '@redis.factory', - - '@current_user', + - '@request_stack' + - '@database' + - '@session_manager.metadata_bag' + - '@session_configuration' + - '@redis.factory' + - '@current_user' - '@session_handler' calls: - [setWriteSafeHandler, ['@session_handler.write_safe']] From 24a17d4fa41ad09a60bf71abacb2a9e2af58bf83 Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Wed, 10 Nov 2021 15:57:48 +0100 Subject: [PATCH 14/15] Issue #2876099 by Grimreaper: Use Symfony Redis Handler. --- modules/redis_sessions/README.md | 17 +- modules/redis_sessions/example.services.yml | 14 + modules/redis_sessions/redis_sessions.install | 52 ---- .../redis_sessions.services.yml | 18 +- .../Session/RedisSessionsSessionManager.php | 278 ------------------ .../src/Session/SessionHandlerFactory.php | 41 +++ 6 files changed, 65 insertions(+), 355 deletions(-) create mode 100644 modules/redis_sessions/example.services.yml delete mode 100644 modules/redis_sessions/redis_sessions.install delete mode 100644 modules/redis_sessions/src/Session/RedisSessionsSessionManager.php create mode 100644 modules/redis_sessions/src/Session/SessionHandlerFactory.php diff --git a/modules/redis_sessions/README.md b/modules/redis_sessions/README.md index 6fdbd0b..a796aa4 100644 --- a/modules/redis_sessions/README.md +++ b/modules/redis_sessions/README.md @@ -1,6 +1,6 @@ CONTENTS OF THIS FILE --------------------- - + * Introduction * Requirements * Installation @@ -25,7 +25,7 @@ This module requires the following modules: INSTALLATION ------------ - + * Install as you would normally install a contributed Drupal module. See: https://www.drupal.org/docs/8/extending-drupal-8/installing-modules for further information. @@ -34,11 +34,8 @@ INSTALLATION CONFIGURATION ------------- - * By default, Redis Sessions will attempt to use the redis.connection host. - * OPTIONAL: You can add the save_path to your settings.php file, especially if - you want to use a different Redis service than what is used for cache. - ``` - $settings['redis_sessions'] = [ - 'save_path' => 'tcp://redis:6379', - ]; - ``` +Either include the default example.services.yml from the module, which will +replace all supported backend services (check the file for the current list) +or copy the service definitions into a site specific services.yml. + + $settings['container_yamls'][] = 'modules/redis/modules/redis_sessions/example.services.yml'; diff --git a/modules/redis_sessions/example.services.yml b/modules/redis_sessions/example.services.yml new file mode 100644 index 0000000..0ced19e --- /dev/null +++ b/modules/redis_sessions/example.services.yml @@ -0,0 +1,14 @@ +# This file contains example services overrides. +# +# Enable with this line in settings.php +# $settings['container_yamls'][] = 'modules/redis/modules/redis_sessions/example.services.yml'; +# +# Or copy & paste the desired services into sites/default/services.yml. +# +# Note that the redis module must be enabled for this to work. + +services: + # Replaces the default session handler storage with a redis implementation. + session_handler.storage: + class: SessionHandlerInterface + factory: ['@redis_sessions.session_handler.factory', get] diff --git a/modules/redis_sessions/redis_sessions.install b/modules/redis_sessions/redis_sessions.install deleted file mode 100644 index 6129415..0000000 --- a/modules/redis_sessions/redis_sessions.install +++ /dev/null @@ -1,52 +0,0 @@ - 'Redis Sessions', - 'value' => t('Connected, using the @name client.', [ - '@name' => ClientFactory::getClientName(), - ]), - 'severity' => REQUIREMENT_OK, - ]; - } - else { - $requirements['redis_sessions_redis'] = [ - 'title' => 'Redis Sessions', - 'value' => t('Not connected.'), - 'severity' => REQUIREMENT_WARNING, - 'description' => t('No Redis client connected, this module is useless thereof. Ensure that you enabled module using it or disable it.'), - ]; - } - - /** @var array $settings */ - $settings = Settings::get('redis_sessions', []); - if (!isset($settings['save_path']) || empty($settings['save_path'])) { - $requirements['redis_sessions_save_path'] = [ - 'title' => 'Redis Sessions', - 'value' => t("Redis Sessions has not been configured with a save_path setting. See the CONFIGURATION section of this module's README.md."), - 'severity' => REQUIREMENT_OK, - ]; - } - - return $requirements; -} diff --git a/modules/redis_sessions/redis_sessions.services.yml b/modules/redis_sessions/redis_sessions.services.yml index 33a9bef..7ff05aa 100644 --- a/modules/redis_sessions/redis_sessions.services.yml +++ b/modules/redis_sessions/redis_sessions.services.yml @@ -1,16 +1,4 @@ services: - # Replaces the session manager with a redis implementation. - redis_sessions.session_manager: - class: Drupal\redis_sessions\Session\SessionManager - decorates: session_manager - decoration_priority: -10 - arguments: - - '@request_stack' - - '@database' - - '@session_manager.metadata_bag' - - '@session_configuration' - - '@redis.factory' - - '@current_user' - - '@session_handler' - calls: - - [setWriteSafeHandler, ['@session_handler.write_safe']] + redis_sessions.session_handler.factory: + class: Drupal\redis_sessions\Session\SessionHandlerFactory + arguments: ['@redis.factory'] diff --git a/modules/redis_sessions/src/Session/RedisSessionsSessionManager.php b/modules/redis_sessions/src/Session/RedisSessionsSessionManager.php deleted file mode 100644 index 606afab..0000000 --- a/modules/redis_sessions/src/Session/RedisSessionsSessionManager.php +++ /dev/null @@ -1,278 +0,0 @@ -clientFactory = $client_factory; - $this->currentUser = $current_user; - $this->redis = $this->clientFactory->getClient(); - if ($this->clientFactory->getClientName() == 'PhpRedis') { - ini_set('session.save_path', $this->getSavePath()); - ini_set('session.save_handler', 'redis'); - } - elseif ($this->clientFactory->getClientName() == 'Predis') { - $handler = new Handler($this->redis, array('gc_maxlifetime' => 5)); - $handler->register(); - } - } - - /** - * Return the session.save_path string for PHP native session handling. - * - * Get save_path from site settings, since we can't inject it into the - * service directly. - * - * @return string - * A string of the full URL to the redis service. - */ - private function getSavePath() { - // Use the save_path value from settings.php first. - $settings = Settings::get('redis_sessions'); - if ($settings['save_path']) { - $save_path = $settings['save_path']; - } - else { - // If no save_path from settings.php, use Redis module's settings. - $settings = Settings::get('redis.connection'); - $settings += [ - 'port' => '6379', - ]; - $save_path = "tcp://${settings['host']}:${settings['port']}"; - } - - return $save_path; - } - - /** - * Return a key prefix to use in redis keys. - * - * @return string - * A string of the redis key prefix, with a trailing colon. - */ - private function getNativeSessionKey($suffix = '') { - // @todo Get the string from a config option, or use the default string. - return 'PHPREDIS_SESSION:' . $suffix; - } - - /** - * Return the redis key for the current session ID. - * - * @return string - * A string of the redis key for the current session ID. - */ - private function getKey() { - return $this->getNativeSessionKey($this->getId()); - } - - /** - * Return a Drupal-specific key prefix to use in redis keys. - * - * @return string - * A string of the redis key prefix, with a trailing colon. - */ - private function getUidSessionKeyPrefix($suffix = '') { - // @todo Get Redis module prefix value to add to the $sid Redis key prefix. - // @todo Get the string from a config option, or use the default string. - return 'DRUPAL_REDIS_SESSION:' . $suffix; - } - - /** - * Return the redis key for the current session ID. - * - * @return string - * A string of the redis key for the current session ID. - */ - private function getUidSessionKey() { - $uid = $this->getSessionBagUid(); - return $this->getUidSessionKeyPrefix(Crypt::hashBase64($uid)); - } - - /** - * Get the User ID from the session metadata bags. - * - * Fetch the User ID from the metadata bags rather than a traditional user - * lookup in case the UID is in the process of changing (logging in or out). - * - * @return int - * User id as passed to the constructor in a metadata bag. - */ - private function getSessionBagUid() { - foreach ($this->bags as $bag) { - // In Drupal 8.5 and above, the bag may be a proxy, in which case we need - // to get the actual bag. - if (method_exists($bag, 'getBag')) { - $bag = $bag->getBag(); - } - if ($bag instanceof AttributeBagInterface && $bag->has('uid')) { - return (int) $bag->get('uid'); - } - } - return 0; - } - - /** - * {@inheritdoc} - */ - protected function isSessionObsolete() { - $bag_uid = $this->getSessionBagUid(); - $current_uid = $this->currentUser->id(); - return ($bag_uid == 0 && $current_uid == 0); - } - - /** - * {@inheritdoc} - */ - public function save() { - parent::save(); - - // Write a key:value pair to be able to find the UID by the SID later. - // NOTE: Checking for $uid here ensures that only sessions for logged-in - // users will have lookup keys. Anonymous sessions (if they exist at all) - // are transient and will be cleaned up via garbage collection. - // @todo Add EX Seconds to the set() method for session life length. - // @todo After adding EX and PX seconds, add 'NX'. - // See: https://redis.io/commands/set. - if ($this->getSessionBagUid()) { - if ($this->currentUser->id()) { - $this->redis->set($this->getUidSessionKey(), $this->getKey()); - } - else { - $this->destroyObsolete($this->redis->get($this->getUidSessionKey())); - } - } - } - - /** - * {@inheritdoc} - */ - public function delete($uid) { - if ($this->isCli()) { - return; - } - - // Get the session key by $uid. - $sid = $this->redis->get($this->getUidSessionKey()); - - // Delete both key/value pairs associated with the session ID. - $this->redis->del($sid); - $this->redis->del($this->getKey()); - } - - /** - * {@inheritdoc} - */ - public function destroy() { - parent::destroy(); - - if ($this->isCli()) { - return; - } - - $uid = $this->getSessionBagUid(); - $this->redis->del("SESS_DESTROY:$uid:" . $this->currentUser->id()); - - if ($uid) { - if ($this->currentUser->id() == 0) { - $sid = $this->redis->get($this->getUidSessionKey()); - - $this->redis->del($sid); - $this->redis->del($this->getUidSessionKey()); - $this->redis->del($this->getKey()); - } - } - } - - /** - * Removes obsolete sessions. - * - * @param string $old_session_id - * The old session ID. - */ - protected function destroyObsolete($old_session_id) { - $this->redis->del($old_session_id); - $this->redis->del($this->getUidSessionKey()); - } - - /** - * Migrates the current session to a new session id. - * - * @param string $old_session_id - * The old session ID. The new session ID is $this->getId(). - * - * @see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Session%21SessionManager.php/function/SessionManager%3A%3AmigrateStoredSession/8.2.x - */ - protected function migrateStoredSession($old_session_id) { - // The original session has been copied to a new session with a new key; - // remove the original session ID key. - // Test: redis-cli KEYS "*SESS*" | xargs redis-cli DEL && redis-cli. - $this->redis->del($this->getNativeSessionKey($old_session_id)); - } - -} diff --git a/modules/redis_sessions/src/Session/SessionHandlerFactory.php b/modules/redis_sessions/src/Session/SessionHandlerFactory.php new file mode 100644 index 0000000..6d01dd9 --- /dev/null +++ b/modules/redis_sessions/src/Session/SessionHandlerFactory.php @@ -0,0 +1,41 @@ +clientFactory = $client_factory; + } + + /** + * Get actual session handler. + * + * @return \SessionHandlerInterface + * Return the redis session handler. + */ + public function get() { + $client = $this->clientFactory->getClient(); + return new RedisSessionHandler($client); + } + +} From 7f864973bebf52b5a882bfa49b5d3313c03394bf Mon Sep 17 00:00:00 2001 From: florenttorregrosa Date: Wed, 10 Nov 2021 16:17:29 +0100 Subject: [PATCH 15/15] Issue #2876099 by Grimreaper: Add tests. --- .../src/Functional/RedisSessionWebTest.php | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 modules/redis_sessions/tests/src/Functional/RedisSessionWebTest.php diff --git a/modules/redis_sessions/tests/src/Functional/RedisSessionWebTest.php b/modules/redis_sessions/tests/src/Functional/RedisSessionWebTest.php new file mode 100644 index 0000000..340b9c6 --- /dev/null +++ b/modules/redis_sessions/tests/src/Functional/RedisSessionWebTest.php @@ -0,0 +1,49 @@ +siteDirectory . '/settings.php'; + chmod($filename, 0666); + $contents = file_get_contents($filename); + + // Add the container_yaml and cache definition. + $contents .= "\n\n" . '$settings["container_yamls"][] = "' . drupal_get_path('module', 'redis_sessions') . '/example.services.yml";'; + file_put_contents($filename, $contents); + OpCodeCache::invalidate(DRUPAL_ROOT . '/' . $filename); + + $this->rebuildContainer(); + } + +}