joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.78k stars 3.65k forks source link

[RFC] It needs a safe way to determine if an extension is core or not #16579

Closed richard67 closed 7 years ago

richard67 commented 7 years ago

Steps to reproduce the issue

It needs a safe way to determine if an extension is core or not.

Using the ID in the extensions table is not a safe way.

See discussion following this comment: https://github.com/joomla/joomla-cms/pull/16494#issuecomment-306917787

Expected result

It can be determined for diverse purpose, e.g. like proposed with PRs #16474 or #16494 , if an extension is core or not, also after core extensions have been installed using the extensions discovery function.

Actual result

If core extensions have been installed using the extensions discovery function, e.g. after some update problems, the IDs of these extensions in the extensions table are not within the expected range below 10000, and so this way to determine if core or not core does not work.

Additional comments

A safe way could either be a hard-coded array of IDs or names in a globally used class in PHP, or to set a value in a new column "is_core" (or similar) to true (1) in the installation scripts of the core (where the extensions table is filled for core extensions) and the extension (where it is used by the extensions discovery function).

The advantage of doing it in the SQL scripts could be that it is easier to maintain because there is no extra file to be remembered for editing, too, when adding new core extensions.

The disadvantage of doing it in the SQL scripts is that a bad or silly 3rd party extensions developer could set it in his SQL scripts, too.

Because I prefer idiot-safe solutions I would vote for the hard-coded array, but that's only my opinion.

I think this should be discussed by experienced maintainers and maybe even by the PLT.

richard67 commented 7 years ago

Could someone add a label "Discussion" to this issue?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

richard67 commented 7 years ago

Hmm, I corrected something in the description and now status has changed back from "discussion" to "new".


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

mbabker commented 7 years ago

There are a couple of PRs from Andre that I've tagged for 3.8 that help with this. --

richard67 commented 7 years ago

@mbabker Well, I remember there was something with protected extensions. Questions to @andrepereiradasilva : Can we rely on "protected extension" is equal to "core extension"? Or is it a feature also being usable for 3rd party extensions?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

richard67 commented 7 years ago

As far as I understand, the reason why the 2 PRs I mentioned in the description need to know if core or not core extension is that they are dealing with update sites, which in case of 3rd party extensions can be found in the extension's XML file but in case of the core it depends on the update component's options (update channel, custom URL) and so has to be handled differently.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

andrepereiradasilva commented 7 years ago

A time ago made some PR to allow to lock extensions to not being able to be uninstalled. All core extensions would be locked But it doesnt mean all locked extension would be core extensions. See https://github.com/joomla/joomla-cms/pull/13037/files

You maybe could have a extension helper ir something with a list of core extensions to help since this seems a frequent issue that is raised.

Bakual commented 7 years ago

Maybe it's possible to just link each core extension to the "Joomla" updateserver? Not sure about the implications.

piotrmocko commented 7 years ago

@Bakual when you will assign each extension to an updateserver then Joomla will try to find update and there will be so many external request that looking for updates will end with a timeout.

Some time ago I have installed locally Joomla 3.6.0 and my custom extensions IDs are starting from 803, just after GB language pack with ID 802. So I guess that you can not relay on that.

In my opinion the best way is to add a new column #__extensions.core = 0 / 1

@richard67 it can be also a PHP helper with an array as it is the safest way

Bakual commented 7 years ago

In my opinion the best way is to add a new column #__extensions.core = 0 / 1

I don't like that much. Imho it's a static list of elements which could as well live as a property of a PHP class. I would prefer a helper or property somewhere in com_admin or com_installer.

Storing such static information into the database doesn't make much sense to me. Also updating the database during an CMS update to populate the column could fail as well when updated without using Joomla Update. So the same people who don't have the right IDs for their extensions may miss that update SQL :smile:

But that's my personal opinion.

andrepereiradasilva commented 7 years ago

A JExtensionHelper::getCoreExtensionIds() that returns all core extension ids in the #__extensions table?

That means that all updates that need core extension ids must be done trough com_admin script.php (as IMHO all non db schema updates should be ... ).

andrepereiradasilva commented 7 years ago

first let me say i don't have much time to mantain PRs.

So i did the code to have a JExtensionHelper::getCoreExtensionIds() that returns all core extension ids. See https://github.com/joomla/joomla-cms/compare/staging...andrepereiradasilva:exte3nsion-helper-core

If anyone want to use that code (or parts of it) to make a PR, be my guest.

wojsmol commented 7 years ago

@andrepereiradasilva It would be good to have one list of core extensions instead of two.

richard67 commented 7 years ago

@andrepereiradasilva I maybe can create a PR on weekend when I have more time than during the week. I'll check your branch mentioned above of course, thanks for that.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

andrepereiradasilva commented 7 years ago

you are right @wojsmol let me check

richard67 commented 7 years ago

Well, maybe it needs 2 lists - softcore and hardcore extensions ... :-) (joking)

andrepereiradasilva commented 7 years ago

now it as only one list :) https://github.com/joomla/joomla-cms/compare/staging...andrepereiradasilva:exte3nsion-helper-core

BTW there are several extensions missing from that update manifest list... Just noticed there are 160 core extensions!

andrepereiradasilva commented 7 years ago

hardcore extensions are at least com_tags and the plugin extension joomla :wink:

richard67 commented 7 years ago

@andrepereiradasilva You wrote: "BTW there are several extensions missing from that update manifest list...". What do you mean with that? I am not sure if I fully understand which list you mean, yours or the manifest XMLs, and what meands missing. Is your list complete now, and the XMLs are not, or vice versa?

@Bakual @mbabker @piotrmocko @wojsmol What do you think about andre's solution? To me it seems to be OK. If it is OK for you, too, and nobody else wants to do it or has time, I can make a PR of course, but all the credits should go to andre then.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16579.

Bakual commented 7 years ago

I'm fine with his proposal.

andrepereiradasilva commented 7 years ago

@richard67 don't need credits. Just don't have time to mantain pr yes my list as all extensions but please double check with instalattion sql file

wojsmol commented 7 years ago

@richard67 It looks ok to me.

richard67 commented 7 years ago

Guys, I have created a PR: See #16613 . Please review and test.

richard67 commented 7 years ago

Closing as there is a PR #16613 . Discussion can be continued there, testers and reviewers are welcome.