User tests: Successful: Unsuccessful:
Use the config variables in the session service provider from the application. Question here, should the app not being fetched from the container instead of the Factory?
I'm splitting #16918 into different pr's to be easier to review.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Made a quick test and I could move setting the session from the app service provider here to the execute function. So it should be possible to load the session after the app is created in the application service provider. As said just a quick test didn't test other stuff.
Well, you can do that, but then you've got the application arbitrarily injecting its own dependencies (either by creating them inline or accessing the container). So we're losing a huge piece of testability here if $app->execute()
is doing $this->setSession($container->get('session'));
without mocking the container, not much better than what we have now with putting stuff into JFactory's public statics.
This is just one of those spots that aren't pretty because configuring the session service is reliant on the active application. So either the session config keeps a dependency to JFactory or you break dependency injection on the application.
You got me wrong. I didn't say I want to do it that way. Just wanted to test if the session instance is really needed when creating the application in the container.
To create the application, the session isn't needed. But to create the session, the application is needed. That's where we are running into problems.
Just to be clear here, it's not an architectural dependency; it's all configuration based (the force SSL option being on a per-client basis as an example, or the default value of the session_name
config option being the class name of the primary application for the request, the one which should be in the factory).
I guess we shifted here. Lets get the session issue sorted in another pr then.
Labels |
Added:
?
|
Conflicts fixed.
Sooner or later we're really going to need a $container->get('config');
that gets injected into apps versus relying on the apps using the Autoconfigurable behavior (Factory::getConfig()
can proxy to that for the time being so the same root config registry is still accessible).
For now this PR seems fine, but that really needs to be one of the next architecture steps we take.
I will have a look into the configuration service provider.
Closing this one then.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-13 07:27:55 |
Closed_By | ⇒ | laoneo |
Well, here's the problem. The session service is instantiated by the application service, so
$container->get('application');
would end up in a recursive loop between that and the session. We also are not yet injecting aRegistry
instance into the application representing the parsedJConfig
object, so we're still relying on things to inherently resolve themselves.Eventually, we will need a
config
service in the container similar to the one defined in https://github.com/joomla/framework.joomla.org/blob/master/src/Service/ConfigurationProvider.php (the part about how the service is shared, not necessarily having the provider parse the config itself). This way when there are services which require the configuration (i.e. https://github.com/joomla/framework.joomla.org/blob/master/src/Service/DatabaseProvider.php) then that can be accessed without being dependent on a complex service like the application, especially when the dependency exists while the application dependency is being resolved.