User tests: Successful: Unsuccessful:
Lightweight approach making the dispatcher namespace aware as an entry point for loading the controller injecting the app and the input..
RFC and WIP.. Test that namespaces in Controllers are now working and non-namespaced Controllers are still working
Namespaces working
Namespacing in Dispatcher not working
Yes
Status | New | ⇒ | Pending |
Category | ⇒ | SQL Administration com_admin Postgresql Installation Libraries |
Title |
|
Labels |
Added:
?
|
Labels |
Added:
?
|
Category | SQL Administration com_admin Postgresql Installation Libraries | ⇒ | SQL Administration com_admin Postgresql Installation Libraries Unit Tests |
Unless this namespace column is going to be used for all extensions, it should not be added as a separate column to the database.
A separate JComponentHelper::getNamespace()
method should not be added. Add the column to the query issued by JComponentHelper::load()
and the property to JComponentRecord
. The data will therefore be included in the cache and eliminate the need for extra queries just to get an extension namespace (JComponentHelper::getComponent('com_content')->namespace
).
JComponentHelper will not be the only thing calling the dispatcher. This is going to be the public 'interface' for extensions to call another extension (hence why the dispatcher itself isn't namespaced). It allows e.g. a module to call a component.
JComponentHelper::load()
is already generally preloading component records into memory anyway (and supports writing the result to cache so the database isn't being read for minimum 5 or 6 components on every request). Adding the property to JComponentRecord
and the loader will be perfectly acceptable. Otherwise a separate JComponentHelper::getNamespace()
with no caching WILL be a performance issue.
Labels |
Added:
?
|
+1 for using JComponentHelper::getComponent('com_content')->namespace to get namespace would be the right way. The reason Michael already mentioned, plus from my experience when I was working on Controllers, Models classes, look like we will have to get namespace of component from those classes ( to build Model, View, Table classes) as well, so using cache of already loading data would be better.
So we undeprecate passing in a parameter then? I think the dispatcher should be the only thing that should need to map a namespace to a path. Everything else inside a given component MVC layer is in the folder path and should be component and namespace aware.
Undeprecate passing a parameter where?
JComponentHelper::load
Well, it depends. Right now it can be pretty flaky and inconsistent (look at the code). The intent was to treat it similarly to JPluginHelper::load()
and have all the component records loaded into memory/cache instead of potentially having to do one query per component.
So it has an advantage of there really is only one read operation required to load component data (params, namespace, enabled flag, etc.) into memory on each request. The disadvantage though with Joomla's current architecture is that you're loading a lot of unneeded data into memory (a single request is probably easily loading data from 4-5 components, but we have 32 of them in core). In comparison, not all plugins are getting dispatched on every request (legitimately a good chunk of them only are used on write operations or authentication), but the same data is loading into the static data store in JPluginHelper
.
The big question here is are we happy requiring a JApplicationCms
object for the controller. I don't think there are many people creating new instances so the b/c break there is limited. But there are going to be a decent number overloading the constructor.
For 4.0 you probably have to do it as optional. And you definitely can't change the parameter order (so it has to be the options array then the new objects).
Either that or you're going to have a pretty tricky B/C layer going through the constructors.
Yep it's a B/C break, but are many people using new instead of JController::getInstance for controllers (couldn't come up with one in core)? So is it worth it? :-)
Many may not be direct instantiating themselves, but for controllers with extended constructors that call the parent constructor, it does become an issue (because ultimately JControllerLegacy::getInstance()
calls new ContentControllerArticles($options);
(https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/controllers/articles.php)).
Hmm okay, so we make it optional? (Again, had that in the beginning..) and keep the ordering $config = array(), $app = null, $input = null .. :-)
getInstance
isn't a problem because we are forceably injecting JFactory::getApplication()
there. So I don't think there is a problem instantiating a controller with this b/c break.
I think the b/c problem is the number of extensions calling parent::__construct
for example to register a custom task.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-03-20 01:16:37 |
Closed_By | ⇒ | wilsonge |
This will do for round 2 of refactoring
Added docs about the Controller constructor b/c break here https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4#Legacy_Controller Dispatcher docs still required