Problem/Motivation
In content_lock_timeout_user_logout() there is code part which checks if user sessions are stored in database.
/**
* Implements hook_user_logout().
*/
function content_lock_timeout_user_logout($account): void {
....
// Only do the database check if the original drupal session manager is used.
// Otherwise, it's not sure if sessions table has correct data. As it would be
// possible to extend the Class, instanceof is not used here!
if (get_class(Drupal::service('session_manager')) == SessionManager::class) {
$query = \Drupal::database()
->select('sessions');
$query->condition('uid', $account->id());
$query = $query->countQuery();
$session_count = (int) $query->execute()->fetchField();
}
....
}
https://git.drupalcode.org/project/content_lock/-/blob/3.x/modules/conte...
It is fine to do such check since 'sessions' table can be missing if other session storage (like Redis) is used - https://www.drupal.org/node/3431286.
But this checking isn't working properly if session storage is changed to some alternative. Because \Drupal::service('session_manager') is still \Drupal\Core\Session\SessionManager when session storage is (for example) \Symfony\Component\HttpFoundation\Session\Storage\Handler\RedisSessionHandler.
Steps to reproduce
I tried find example of the RedisSessionHandler usage. Here is related example - https://www.drupal.org/project/redis/issues/2876099#comment-15763319
Proposed resolution
Check \Drupal::service('session_handler.storage') instead of Drupal::service('session_manager').
Remaining tasks
None.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3514317-content_lock-content-lock-timeout-user-logout.patch | 1.34 KB | deimos |
Issue fork content_lock-3514317
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
deimos commentedI'm going to provide solution described in description.
Comment #4
deimos commentedCommitted fix into fork-branch. Attaching patch-file for safer applying in projects.
Ready for review/testing.
Comment #5
ciprian.stavovei commentedThe fix is working well.
Comment #6
alexpottThis seems correct. It's all a bit messy because of the way that StackedSessionHandlerPass works. It might be nice at some point to abstract the session counting logic so a user could supply an implementation for a different backend.