[#19594] - [4.0] Split the configuration of the session service on a per-application basis, restructure session GC command
- Fixed in Code Base
- 7 Aug 2018
- Medium
- Build: 4.0-dev
- # 19594
- Diff
- mbabker:4.0-split-session-service
User tests: Successful: Unsuccessful:
Summary of Changes
This is restructuring the definition of the core session service for 4.0.
Right now we have an issue where the session service is dependent on the application object for some checks and dependencies. We can loosen that dependency by splitting the definition up on a per-application basis, which as of this PR only leaves the internal dependency in place to get the Joomla\Input\Input objects used in the session store (this can be addressed in a later PR much more easily now).
Additionally, this gives a little more flexibility to our session:gc command. Right now there are technically different session stores/configurations for each of the core applications, primarily because of how the default session_name configuration value is set when shared sessions is not active. So now the command can run garbage collection factoring in each application's configuration appropriately, especially as a session_gc() call in PHP requires an active session.
Testing Instructions
This is predominantly code review, but all CMS operations should continue to function as normal too.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration CLI Installation Libraries |
| Labels |
Added:
?
|
||
Rebased after #19658
So the only app dependencies left to the session provider are to get the JInput object (which I usually just make a service in the container on its own) and to register the event listener for the SessionEvents::START event (this one's a little trickier in the scope of the CMS, we can't put this to a plugin because plugins aren't loaded until after the session is started since they rely on the active user and their access levels so we still need a separate non-plugin class to do this job and whatever is registering it has to be smart enough to deal with app differences (the install app just sets an internal data registry as a Registry object, the other web apps also set a default User object to the user key)).
Rebased. To be able to clean up service definition a little bit more and move the session event listener out of the application class to a dedicated event listener this PR really should be merged since it's better defining and splitting up the internal configuration of sessions making the other needed changes easier to abstract out.
Last attempt at rebasing this. Either merge it or close it please.
| Category | Administration CLI Installation Libraries | ⇒ | Administration CLI External Library Composer Change Installation Libraries |
| Labels |
Added:
?
|
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-08-07 05:40:16 |
| Closed_By | ⇒ | laoneo |
Thanks
@laoneo Just an FYI a combination of this and that
$container->get('config')thing I mentioned in #19531 should in effect let us fully decouple the app and session instantiation so that we shouldn't needFactory::$applicationto have anything set to it before$container->get('app');finishes building whatever application is being requested. So we would need your PR in first, then sort out that config service, then this be rebased on top of all of that.