Feature No Code Attached Yet
avatar HermanPeeren
HermanPeeren
26 Dec 2023

A \Joomla\Database\DatabaseInterface is supposed to be injected into an \Joomla\CMS\MVC\Factory\MVCFactory. In case that would be forgotten, for instance when instantiating your own MVCFactory without making use of the \Joomla\CMS\Extension\Service\Provider\MVCFactory service provider, as a temprorary band-aid, the database is fetched from the container.

In https://github.com/joomla/joomla-cms/blob/5.0-dev/libraries/src/MVC/Factory/MVCFactory.php#L174-L175 a "deprecated" error-message is (suppressed) triggered and, as a temprorary band-aid, the database that is missing in the MVCFactory to inject in a DatabaseAwareInterface MVC object, is fetched from the container in a catch block:

    @trigger_error(sprintf('Database must be set, this will not be caught anymore in 5.0.'), E_USER_DEPRECATED);
    $model->setDatabase(Factory::getContainer()->get(DatabaseInterface::class));

The same for a MVC-Table object in lines 270-271.

But this code is still present in Joomla 5.

Should we now remove the band-aid in line 175 (and 271) and let it (unsuppressed) completely fail on line 174 ( and 270) in Joomla 5.0.2, or postpone that to 5.1.0 or even to 6.0.0? For there can be some 3PD code (I've lately seen one) that would fail on a Joomla-update if we correct this now.

avatar HermanPeeren HermanPeeren - open - 26 Dec 2023
avatar joomla-cms-bot joomla-cms-bot - change - 26 Dec 2023
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 26 Dec 2023
avatar HermanPeeren HermanPeeren - change - 26 Dec 2023
The description was changed
avatar HermanPeeren HermanPeeren - edited - 26 Dec 2023
avatar richard67
richard67 - comment - 26 Dec 2023

I would assume it will be removed in Joomla 6 due to our changed release strategy.

avatar HermanPeeren
HermanPeeren - comment - 27 Dec 2023

Reading the above I propose to change the deprecation message in both upcoming releases (5.0.2 and 4.4.2) to "Database must be set, this will not be caught anymore in 6.0." AND remove the band-aid (and let it fail) in the current 6.0-dev branch. If agreed upon I'll make a PR for those 3 little changes.

All other deprecations that are now announced for 6.0 (like setting the db in the workflow) should also be done in the current 6.0-dev branch. Will make a seperate PR for that. Actually removing deprecated code in an early stage in new banches can avoid mistakes like the one in this issue.

avatar richard67
richard67 - comment - 28 Dec 2023

@HermanPeeren I've asked in the CMS Maintainers channel on Mattermost for a confirmation of my assumption (6.0 for the band aid) and opinions on this issue. I don't have any opinions yet, maybe due to the holiday period. I'll let you know when I have some.

avatar rdeutz
rdeutz - comment - 2 Jan 2024

Looks good to me

avatar Hackwar Hackwar - change - 26 Mar 2024
Labels Added: Feature
avatar Hackwar Hackwar - labeled - 26 Mar 2024
avatar rdeutz
rdeutz - comment - 27 Apr 2024

@HermanPeeren would be great, if you can make the PR you said.

Add a Comment

Login with GitHub to post a comment