[#35822] - ExtensionAdapter should expect a client_id as either integer or string
- Closed
- 14 Oct 2021
- Medium
- Build: 4.0-dev
- # 35822
- Diff
- nikosdion:fix/35376-update-adapter
User tests: Successful: Unsuccessful:
Pull Request for Issue #35376 and #29778.
Summary of Changes
Fix the \Joomla\CMS\Updater\Adapter\ExtensionAdapter to provide backwards compatibility with integer client_id values in update XML manifests (a.k.a. update sites).
Testing Instructions
See #35376 for installing the test “NULL PLUGIN” extension.
Set Error Reporting to Maximum and Debug Site to Yes.
Go to updates and try to check for updates.
Actual result BEFORE applying this Pull Request
You get a PHP Warning similar to this:
PHP Warning: Attempt to read property "id" on null in /application/libraries/src/Updater/Adapter/ExtensionAdapter.php on line 333
Expected result AFTER applying this Pull Request
There is no PHP warning.
Documentation Changes Required
Update [https://docs.joomla.org/Deploying_an_Update_Server](the official Update Server documentation), namely the paragraph for client. It should read the following:
client – Required for modules and templates as of 3.2.0. This is the Joomla application ID which corresponds to the extension is made for. For Joomla 1.6 up to and including 3.10 it's an integer: 0 for ‘site’ and 1 for ‘administrator’. For Joomla 4 you should use a string matching the application name, currently site or administrator.
For modules and templates this is pretty straightforward; a backend module or template is written for the application administrator whereas a frontend module or template is written for the application site. For other extensions please look in the #__extensions table for your extension and read the client_id column, keeping in mind that 0 means “site” and 1 means “administrator”.
Warning! You must always provide a client_id of 0 or site for plugins, frontend modules and frontend templates. If the client_id is missing Joomla will use the default value of administrator, therefore it will not show any updates for these extension types since the expected value (site) and the value it gets from the update site (administrator) do not match!
Historical note. Joomla 1.6 and 1.7 were using the field name client_id instead of client and expected an integer. Joomla 2.5 up to and including 3.10 used the field name client and expected an integer as well. Joomla 4 uses the field name client and expect an integer (deprecated) or string (preferred). Joomla 5 will still be using the field name client but will only support a string, e.g. site or administrator. Keep this in mind when supporting multiple Joomla versions with a single update site.
Notes
This is necessary for backwards compatibility during the transition phase of third party extensions from Joomla 3 to Joomla 4. During that time the developers need to provide a single package which supports both major versions of the CMS. Therefore the update site needs to be able to work correctly with both Joomla 3 (which expects client_id to be 0 or 1) and Joomla 4 (which expects client_id to be site or administrator).
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
For Joomla 1.6 up to and including 3.10 it's an integer: 0 for ‘site’ and 1 for ‘administrator’. For Joomla 4 you should use a string matching the application name, currently site or administrator.
I have not looked up when it got changed but atleast since 9 years (where the B/C layer got added) both where working fine. So saying "up to 3.10 it requires to be an integer" seems to be wrong to me as that was alteast since 9 years a b/c handling showing in the logs.
Thats also the reason it was finally removed and documented to be gone with J4.
When such warnings and notes are ignored for a that long time I guess there is no point in changing or removing this "b/c" handling ever again as it will come back to the table right after any major release where souch a change could get implemented. So I would say go restore it but make it permanent and remove the "deprecated" warning from J3. :(
Ah looks like @HLeithner took the decision about adding a b/c note again: #35376 (comment)
@HLeithner Can I please request you to decide to remove the "deprected" tag and just add a note to make sure it will stay forever for the reasons noted above.
This is really frustrating as I have been saying the same thing about isSite and isAdministrator but maintainers used the same argument (we made it deprecated x years ago)
Sorry I trusted @nikosdion on his words
DO NOT change the documentation to make a string again. This will break updates for extensions which are compatible with both Joomla 3 and 4.
Which seems to be wrong because since 2012 joomla logs a deprecation warning....
So now I'm the idiot because I didn't checked the source my self and already accepted this change... can only loose here...
I'll just put it this way.
OSM runs a directory, JED. This is integrated into OSM's product, Joomla. Not being listed there can be a serious problem for 3PDs.
JED requires 3PDs to use Joomla's extensions updater for their software to be listed. To do that, they need to create an update site for their extensions.
The official documentation of the update site XML structure even at the time of this writing very clearly states that the client/client_id key MUST be an integer.
So, this leaves us with two possibilities:
A. The documentation is right and the code in Joomla 4 is wrong. This is the stance I took because it's the stance that cannot get OSM sued.
B. The documentation is wrong and the code is right. This is the stance @zero-24 and @HLeithner are taking. In this case the official OSM documentation misleads 3PDs into being non–compliant to OSM's requirement for inclusion in the JED. This gets OSM sued and as we have seen from Epic vs Apple this can indeed get you in very hot water.
Stop thinking just like engineers and start thinking about the bigger picture. Do you REALLY want to take a stance that opens OSM to getting sued by anyone who got excluded from the JED because the Joomla extensions updates wouldn't work for them because the documentation was not in line with the code OR do you want to suck it up, pretend the code was wrong all along and fix it in a way that won't get OSM sued?
Anyway, my extensions are not affected because I am doing things by the code, not by the documentation. If you don't want to protect OSM against a very likely sueball I am not going to stand here and be told once again that I don't know what I am doing — I actually understand much more of the bigger picture than you do all together.
I am closing this PR and good luck to OSM.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-10-14 17:06:24 |
| Closed_By | ⇒ | nikosdion | |
| Labels |
Added:
?
|
||
B. The documentation is wrong and the code is right. This is the stance @zero-24 and @HLeithner are taking.
Just for the record as I have been blamed here:
- Yes the docs page is wrong and yes it should be corrected.
- I have not asked for this PR to be closed nor redjected in any terms.
- The decision to add this check back was based on a wrong claim "This will break updates for extensions which are compatible with both Joomla 3 and 4.".
So in conclusion yes there is an issue here and the options we have are:
- add the check here back and try it again with J5
- add the check back and keep it permanent
- dont add the check back
All of them have pro's and con's for sure :-)
I have tested this item✅ successfully on 66b3950
Fixed the issue as described.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35822.