[#13322] - [4.0] Optional session metadata, misc. session tweaks
- Fixed in Code Base
- 11 Apr 2018
- Medium
- Build: 4.0-dev
- # 13322
- Diff
- mbabker:4.0-no-session-metadata
- Success Hound No violations found. Woof! Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/drone/pr the build was successful Details
- Error continuous-integration/appveyor/pr AppVeyor build cancelled Details
User tests: Successful: Unsuccessful:
Summary of Changes
This should in full decouple the requirement for session metadata. It's done by adding a new global configuration switch (default on because B/C) that when switched off features dependent on this data will need to check. Plus, some other tweaks squeezed in while testing stuff.
Summary of change:
- New config field, if switching off metadata then the config model will also purge what data we have
- Features dependent on session metadata adjusted:
Joomla\CMS\Application\CMSApplication::afterSessionStart()does not run the code to write or cleanup metadata when disabled- Admin
mod_loggedmodule shows a message if metadata is disabled versus a list of users - Admin
mod_statusmodule hides site/admin user counts if metadata is disabled - Site
mod_whosonlinemodule shows a message if metadata is disabled versus a count of users PlgUserJoomlaonly interacts with the session data when the environment is set for itJoomla\CMS\Table\Table::isCheckedOut()modified to account for the metadata tracking
- Added fields to configure Redis session handler
- Removed fields to configure the now unsupported Memcache session handler
Joomla\CMS\Application\CMSApplication::afterSessionStart()will no longer cause an application fatal error if writing the session metadata fails; either it'll fail again when writing the at the end of the request (if using the database handler) and the session fails to persist or the user isn't using the database handler and it really doesn't matter anyway
Testing Instructions
With this patch applied over the 4.0 branch, it should be possible to turn off writing session related metadata to the database. When using a non-database handler, the session table should always stay empty. Switching this off when enabled should clear existing metadata.
Documentation Changes Required
Note that it'll be possible for data to not always be persisted into the database, namely the session ID, client, user ID, and username. When enabled, this will cause the checked out checks to not be able to determine if a user is still logged in.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_config Language & Strings Modules Installation Libraries Front End Plugins |
| Labels |
Added:
?
?
|
||
| Milestone |
Added: |
||
| Milestone |
Added: |
||
Sorry i didnt test this as i didnt understand it and others have repeatedly stated that only experienced developers should test library changes. if only those same people would actually do that :(
Well it is kind of technical in nature, and it also affects user facing features. So a mix of both would be helpful. But I'm really not interested in fighting with 9 months of merge conflicts if it's going to sit around.
Just want to add that if people agree with the missing features when session meta data is turned off, I can test and review this PR once conflict resolved
OK this should be good to go.
When using a non-database handler, the session table should always stay empty. Switching this off when enabled should clear existing metadata.
It clears the metadata field but leaves the record with the session_id - is that correct?
I have tested this item
Other than my question above everything still works as i would expect it to work and the module changes work as described etc
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13322.
It clears the metadata field but leaves the record with the session_id - is that correct?
Correct. The session related fields that are tracked are the session ID, time, and data.
All seems good to me then
It works for me, too. Just an issue with Session Save Path setting:
-
Might be a problem with path filter but when I enter a windows path like D:\www\sessiondata , it could not be saved
-
I guess you need to add some validation to the path to make sure it exists (like other paths). When a wrong path is entered, the application could not be started, an error like this displayed:
Warning: mkdir(): No such file or directory in D:\www\joomla4\libraries\vendor\joomla\session\src\Handler\FilesystemHandler.php on line 55
Error displaying the error page: A \Joomla\Session\SessionInterface object has not been set.: Could not create session directory "/home/abc"
@joomdonation - enter the wrong data and you get an error message telling you that - seems fine to me.
Pretty sure that this is nothing to do with this PR though and as such is out of scope here.
I'm not getting error. The path is saved to configuration.php and then the application could not be stared like I said
thats what I meant. You are getting the error (just not immediately).
BUT if this happened before this PR then it is out of scope and you should create a new issue if you feel it is wrong
Here is the screenshot after I entered a wrong path and save configuration, the site is completely broken:
I read description of the PR and thought this is included:
Added field to configure the file path if using the file session handler
But you are correct, look like that setting was introduced before this PR
Adding that setting was once in scope of this PR but at some point it got added to the repo separately, removed that note from the main description as the diff clearly shows that setting isn't being touched here now.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13322.
| Status | Pending | ⇒ | Ready to Commit |
Ready to Commit after two successful tests.
No longer interested in pursuing correct session architecture in Joomla. Bye.
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-14 14:06:36 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
| Status | Closed | ⇒ | New |
| Closed_Date | 2018-02-14 14:06:36 | ⇒ | |
| Closed_By | mbabker | ⇒ | |
| Labels |
Removed:
?
|
||
| Status | New | ⇒ | Pending |
Where are we with this? I see we got two successful tests but no merge and now conflicts?
Rebased. See y'all at the next merge conflict
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-04-11 06:35:16 |
| Closed_By | ⇒ | laoneo |
Is this worth my time to pursue rebasing?