? ? Success

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
19 Nov 2017

Summary of Changes

Moves namespaces to be explicit rather than magic

@laoneo this is your chance to have the argument about this finally ;)

Testing Instructions

Install Joomla. ENSURE YOU HAVE DELETED libraries/autoload_psr4 before testing if installing it into an existing 4.x install

Expected result

Everything still works

TODO: Add support for plugins - currently this only handles components and modules

Documentation Changes Required

Document that extensions to do autoloading must declare their base namespace in their XML files

avatar wilsonge wilsonge - open - 19 Nov 2017
avatar wilsonge wilsonge - change - 19 Nov 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Nov 2017
Category SQL Installation Postgresql Libraries Front End Plugins Unit Tests
avatar wilsonge wilsonge - change - 19 Nov 2017
The description was changed
avatar wilsonge wilsonge - edited - 19 Nov 2017
avatar C-Lodder
C-Lodder - comment - 19 Nov 2017

Document that extensions to do autoloading must declare their base namespace in their XML files

example?

avatar izharaazmi
izharaazmi - comment - 19 Nov 2017

Can you please also provide examples of non core extensions.

I think that omitting namespace in manifest should also be supported with some kind of deprecated or similar warning for devs. Or that's intentional to enforce?

avatar mbabker
mbabker - comment - 19 Nov 2017

To get support for the newer component structure (namespaced classes and autoloading) you have to make the code changes and add the line to the XML manifest. For now no other 3.x based behaviors are deprecated so you can keep doing things the way you have before (but I imagine by 5.0 we'll be dropping support for the current structure).

avatar izharaazmi
izharaazmi - comment - 19 Nov 2017

In any 3.x do we have support for namespace extensions and still not requiring this manifest entry? If no, I guess this is perfect then.

avatar wilsonge
wilsonge - comment - 19 Nov 2017

We don't have it yet - we will probably be adding it into 3.9 (and when we do we'll need to create that DB column)

avatar wilsonge wilsonge - change - 19 Nov 2017
Title
Move namespacing be explicit
[4.0] Move namespacing be explicit
avatar wilsonge wilsonge - edited - 19 Nov 2017
avatar Fedik
Fedik - comment - 19 Nov 2017

maybe a bit stupid question, but how the update will work for the extensions?

Example on j3 I have a plugin "beer"
I do joomla update to 4.0, and then update an existing plugin,
whether Joomla will pick up the namespace from my "beer" plugin?
or I need to remove, and then install again the plugin?

avatar wilsonge
wilsonge - comment - 19 Nov 2017

You either have namespaces or not.

If you are using a plugin with namespaces then we should be able to make this work in Joomla 3.9. When you upgrade no issues. If you try and use namespaced code in <3.8 - Joomla will not work because it won't be able to build the class name up correctly here: https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Plugin/PluginHelper.php#L260

avatar laoneo
laoneo - comment - 19 Nov 2017

I still think the namespace information is not something which should be put in the database, because it is runtime information. As long as it works and we can backport it to 3.9, go for it.

avatar mbabker
mbabker - comment - 19 Nov 2017

Well as long as we don't have the luxury of having something like Symfony's cache:warmup command to compile all the things so this doesn't have to be handled during runtime or be able to just put namespace mapping direct into Composer's autoloader we have to have a way to get the namespaces to build the mapping array. The database is going to be cheaper than trying to parse the manifest of every extension (one query versus reading over 100 files from the filesystem).

We just need to be completely sure that this info gets updated in the database correctly at install/update.

avatar laoneo
laoneo - comment - 20 Nov 2017

What happens when the user disables the classmap plugin? Will then all namespaced extensions not work anymore?

avatar wilsonge
wilsonge - comment - 20 Nov 2017

All the plugin does is force regenerate the mapping file after an extension is installed/uninstalled/updated. So if the plugin was disabled then all existing extensions would work (even if someone manually deleted the file), it's just that new extensions installed wouldn't be autoloaded until the libraries/autoload_psr4.php file was manually deleted (the application will always create it if it doesn't exist for whatever reason)

avatar laoneo
laoneo - comment - 20 Nov 2017

For me it sounds scary to put that responsibility into a plugin which can be deactivated. Is there a reason to have it deactivated?

avatar Fedik
Fedik - comment - 20 Nov 2017

just an idea,
as the file will generated automatically, can we place it under /cache/_system/ ?
to allow easily remove that file to force regenerate the new mapping

it can be useful in case when something went wrong: just tell "clean that f*n cache" instead of try to explain how to remove/update libraries/autoload_psr4.php

avatar joomdonation
joomdonation - comment - 21 Nov 2017

I installed a copy of https://github.com/wilsonge/joomla-cms/tree/namespacing-completed, tested it and it is working OK

  1. Install component registers correct namespace to autoload_psr4

  2. Uninstall component remove the registered namespace of that component to autoload_psr4.php

  3. Update namespace of component in the xml file, update component and the updated namespace is correctly registered to autoload_psr4.php

  4. Uninstall module removes the registered namespace of that module from autoload_psr4.php

  5. Navigate to random pages and see no errors

One thing doesn't work is that installing new module doesn't register the namespace of that module to autoload_psr4.php, guess it is the same for plugin

No testing to plugins yet.

avatar wilsonge
wilsonge - comment - 21 Nov 2017

Namespaced plugins will not work at the moment. Because their class creation in \Joomla\CMS\Plugin CMSPlugin still only creates a non-namespaced class instance. I'll look into the module namespaces not being stored

avatar laoneo
laoneo - comment - 21 Nov 2017

I really don't understand why we need to make that so complicated over a plugin and have redundant data in a file. If you guys want to go that way, then why not use the database as only point of truth? For now if somebody disables the plugin, new extensions after installation will not work.

avatar joomdonation
joomdonation - comment - 21 Nov 2017

If you guys want to go that way, then why not use the database as only point of truth

I think if this method is accepted, then we should not have to define/set namespace in component dispatcher anymore like on this line https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_content/dispatcher.php#L28 ?

For now if somebody disables the plugin, new extensions after installation will not work.

I think we should make it as a protected plugin so that it could not be disabled/uninstalled.

avatar Fedik
Fedik - comment - 21 Nov 2017

If you guys want to go that way, then why not use the database as only point of truth

I think because the data always the same, it juts add 1 db request, which always return same result. So it is good to have a cached result, stored somewhere.
Changes happens only on install/uninstall, which is not an "everyday case".

avatar joomdonation
joomdonation - comment - 7 Jan 2018

I'll look into the module namespaces not being stored

@wilsonge I created PR #19325 which store namespace for modules during installation/upgrade. If we decide to go with this direction, then you can merge both the PRs.

avatar wilsonge wilsonge - change - 9 Apr 2018
Labels Added: ? ? ?
avatar joomla-cms-bot joomla-cms-bot - change - 16 Apr 2018
Category SQL Installation Postgresql Libraries Front End Plugins Unit Tests SQL Installation Postgresql Libraries
avatar wilsonge
wilsonge - comment - 2 May 2018

@mbabker can you review again please - tests should be fixed with joomla/test-unit#3 (maybe - fixing unit tests in a different repo is hard....)

avatar wilsonge wilsonge - change - 19 Jun 2018
Labels Removed: ?
avatar wilsonge wilsonge - change - 19 Jun 2018
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-06-19 19:00:04
Closed_By wilsonge
avatar wilsonge wilsonge - close - 19 Jun 2018
avatar wilsonge wilsonge - merge - 19 Jun 2018

Add a Comment

Login with GitHub to post a comment