-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Break and deprecate internal dependency to JFactory::createSession() #11351
Conversation
|
Well either inline creating a null JSession object into |
Also, this is kind of a mid-point between what we have now and what's in your 4.0 branch, so it's kinda jumping the gun on moving toward a state that's being built from there. |
I'm just considering the number of people who in CLI apps that require some sort of session state to exist who just call |
Right now no. Nothing has changed at all in the existing workflow for how |
And what's the recommended flow for those building CLI apps that require/use sessions now that createSession is deprecated? |
Like I said, we can make JSession in 4.0 build a null session. For 3.x, On Saturday, July 30, 2016, George Wilson notifications@github.com wrote:
|
I have tested this item ✅ successfully on 38af19d This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11351. |
Nevermind. |
Summary of Changes
There is a rather difficult to deal with dependency to
JFactory::createSession()
to actually create aJSession
instance and have it function correctly. In a move that's somewhat similar to how the install application deals with this, I've refactored the CMS web application class to avoid usingJFactory::createSession()
to create the session and inlined all appropriate logic into the application'sloadSession()
method.I also propose deprecating
JFactory::createSession()
as this is now duplicated logic and we should begin working out how to transition the CMS to proper service injection and retrieval. The application classes are creating their own session objects and storing them internally at this point, we should pointJFactory::getSession()
to that reference when possible.JFactory::getSession()
should only return a null session instance if one cannot be created from the application object as of 4.0.Also fixed is checking in
JFactory::createSession()
if an expire time was injected via the options array and not re-calculating that if so; in custom applications this could cause unwanted behavior but for the default CMS behaviors it is calculating from the same data sources.Testing Instructions
Sessions still function correctly with this patch applied.