@laoneo one for you

Why do you set the Factory::$database property and not use the one from the container? In the model you should not rely on the global database object, only on the one which was injected in the model. Perhaps you can elaborate your use case a bit more, so we can provide a fix for it.

Why do you set the Factory::$database property and not use the one from the container? In the model you should not rely on the global database object, only on the one which was injected in the model. Perhaps you can elaborate your use case a bit more, so we can provide a fix for it.

@laoneo Why aren't you wondering why Joomla! provided Factory::$database and possible to override it? so many methods which can be overridden with the same way like that, by override Factory::$database we can make more feature for the database as well as we can make a global translation system from the external system plugin.

Anyway it worked before 4.2 and then the shortsighted changing code make it broken.

@laoneo We use that way because of very obvious reasons:

1. It is a common practice in the Joomal core code base itself to set the global object in the Factory, it is not something we invented:

Factory::$application: https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/includes/app.php#L58

Factory::$language: https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/components/com_languages/src/Controller/InstalledController.php#L49

Factory::$document: https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Application/SiteApplication.php#L153

2. The global database object is deprecated in Joomla 5, not Joomla 4.2, so it should not break at this moment

https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Factory.php#L117

Our usage is a little bit similar to #38505, that why we referenced it.

For us it is a clear bug, we did not know why the change is needed in Joomla 4.2, but it should be done in a way that should not break what Joomla is doing fine for years.

Thanks

I guess you did understand the deprecation message wrong. This was deprecated in 4.0 with the aim to get removed in 5.0 (with the latest policy change it will be in 6.0).

Nevertheless, it should be the same behavior. That's why I try to understand your use case, because the db object from the factory should basically be the same as in the constructor. So I need a way to reproduce the issue, otherwise I can't provide a fix. Just shouting around "this is wrong" helps nobody.

And #38505 made assumptions clearly in a wrong way, but I was able to restore the old behavior thanks to the reproducible code.

Anyway it worked before 4.2 and then the shortsighted changing code make it broken.

What is here shortsighted?

xyz.zip

Above is a simple system plugin to override the global $database object. For simplicity, it just switch the dbType (from mysql <> mysqli).

Install and enable it, then in the ArticleModel here:

https://github.com/joomla/joomla-cms/blob/4.2-dev/components/com_content/src/Model/ArticleModel.php#L97

Actual: $db is still the default Joomla dabase object

Expected: $db should be the overridden database object as set in the system plugin

Hope it is clearer now.

Thanks for the plugin. The problem is within Joomla 4, Factory::$database can diverge from the one in the container when it is set like you did in the plugin constructor. So all calls to Factory::getContainer()->get('DatabaseDriver') will also point to the old one. I mean, we can add a check in the MVCFactory, to set the database only when no one is set in the model, but this is from architecture point of view wrong as when you load a model through the factory, then it should point to the one from within the factory.

For example this call here uses also the wrong db then

All of the open source CMS which strong care about backward compatibility and you don't care about that.

Before your PR anything works OK but so many extensions broken after your PR, isn't still enough to call be shortsight?

How do you care about backward compatibility? also, did you read anouncement about J5 yet? Ok it here: https://www.joomla.org/announcements/release-news/5868-joomla-5-panta-rhei-the-follow-up.html

Joomla 5 will not include breaking changes for templates and third-party extensions.
Will not remove any code that was marked as deprecated in Joomla 4.

As you said:

In Joomla 4 you should really set the database in the container and not on the Factory alone.

But it maybe fit within J!5 not J!4 for now

OK, It doesn't matter if we know what's wrong and correct it. Not just being conservative looking for every reason to make others believe you're right.

All of the open source CMS which strong care about backward compatibility and you don't care about that.

You have no clue how much I care about backwards compatibility. So calm down and don't talk in that way with me.

Before your PR anything works OK but so many extensions broken after your PR, isn't still enough to call be shortsight?

This pr just revealed an issue which exists since Joomla 4.0.

OK, It doesn't matter if we know what's wrong and correct it. Not just being conservative looking for every reason to make others believe you're right.

Did you actually read my answers? I'm here to help, even when you use stuff in a way you shouldn't anymore in Joomla 4. All I want to do is to fix the issue correctly and not just hacking it. Instead of yelling around, it would be better you stay constructive and don't do any personal attacks. If you want to help, then make a pull request and fix the issue, point at others helps nobody.

@laoneo

It is a very specific issue related to Joomla 4.2.0+

The code that causes this issue was added in Joomla 4.2.0, there is absolute no issues with previous versions.

I did provide the necessary code to replicate it, just comment out the buggy code that was added in Joomla 4.2.0 and you will see the different.

The buggy code that was added in Joomla 4.2.0 completely ignores the database that the model set during the initiation.

https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/MVC/Model/BaseDatabaseModel.php#L108-L114

So will this major B/C break be mitigated or not?