To be honest, I'd be careful about adding new functionality to the database API now considering 4.0 migrated over to the Framework package and there are some API differences (such as there not being disconnect handlers but instead a post disconnect event dispatched to the event system (plugins)). So it wouldn't be good to add something into 3.9 just for it to go away in 4.0, and we aren't going to introduce the disconnect handler concept into the Framework code because the way we incorporated event dispatching is more appropriate as a general solution (not to mention it adds some more hook points that don't exist now).

Is the 4,0 functionality available in 3.9 as a forward compatibility feature?

No, and I don't think it's something we're going to do because even if we do there would still be another round of B/C breaks between introducing it into 3.9 and 4.0 (different event object construction, different internal event identifiers, the debug mode and arbitrary logging statements replaced with a query monitor concept).

So how do we get this functionality recreated in 4.0 then?

Do you want me to try to rework it on the 4.0 code base for both JDatabaseDriver (or equivalent) and the Debug plugin?

You wouldn't necessarily recreate it the same way.

Since the new version of the code dispatches events at connect/disconnect, you wouldn't need a method to explicitly register disconnect handlers; any event listeners (plugins) loaded into the dispatcher subscribed to that disconnect event would be triggered so things would be fine there.

Which leaves the query monitor part. I don't think I'd add an arbitrary behavior to push a default monitor into all instances created through the static factory. Since Joomla core isn't using multiple connections by default, presumably you've got code somewhere creating the secondary (or additional beyond that) connection object. So I'd suggest to go through the DatabaseFactory to create those objects (note it has the side effect of not triggering the static getInstance method or using the singleton store in the database driver itself) and add logic in there to push a query monitor into each created database driver (since the debug plugin in 4.0 is basically using those query monitors to catch logged queries).

That just leaves finding a way to distinguish between different database connections in the plugin in a human friendly way (because signature hashes aren't all that great). Not sure how cleanly you could accomplish that honestly.

This was developed in support of Fabrik which uses at least 1 additional connection even if there is only 1 database.

I will take a look at the 4.0 version of Debug and see if I can work out how to do the equivalent (assuming that Debug still has such a hook - which presumably is only there because there was not a direct way of collecting the information from the JDatabaseDriver).

A quick review suggests that a similar approach should work for v4.0. But instead of setting a disconnect handler, we need to instantiate a query monitor for every database connection.

Leave it with me and I will see what I can do about creating a 4.0 equivalent.

It surprises me that in framework 2 database you can only set one QueryMonitor for a databasedriver object. This does not seem sensible when multiple subsystems might want to monitor the same database queries e.g. Joomla debug and a query debugging tool etc.

However that aside, I see that Debug creates a QueryMonitor instance regardless of whether Debug is on or off (passing the debug boolean as a construct parameter). I am unclear why we would create a monitor if it is disabled and then does nothing, especially since there is no functionality to dynamically enable / disable it once it is created. And since the FrameWork Database doesn't know about CMS debugging, we cannot at the moment create a QueryMonitor inside the Framework because we don't know whether to enable it or not.

So a couple of questions to be answered:

a. Do we want to switch from supporting a single QueryMonitor to supporting multiple simultaneous monitors?

b. Do we really want to have QueryMonitors created disabled which then do nothing - or only create QueryMonitors when we want to monitor?

Once you answer these questions, I can almost certainly create 4.0 equivalent functionality in a nice way. And I will happily modify both Framework and CMS as needed to do this properly.

a. Do we want to switch from supporting a single QueryMonitor to supporting multiple simultaneous monitors?

No. It's not a pub/sub or event style system (though you could conceivably write a monitor to do events if you really wanted to). We have a ChainedMonitor which can push to multiple monitors, so really all you have to do is have a chain and add additional instances to it.

b. Do we really want to have QueryMonitors created disabled which then do nothing - or only create QueryMonitors when we want to monitor?

It doesn't really hurt anything. Arguably it's better to not do it to save a few CPU cycles, but it all boils down to preference I guess.

And since the FrameWork Database doesn't know about CMS debugging, we cannot at the moment create a QueryMonitor inside the Framework because we don't know whether to enable it or not.

The intent is for the monitor API to not know or care about someone's debug tooling being enabled. The intent is to not arbitrarily change internal behaviors because someone put a debug mode into their application, so if you have a monitor registered it is always called (and ultimately it becomes the downstream consumer's choice/responsibility to register services instead of us changing our internal behavior based on some boolean flag).

I am looking for a steer here. What would you think of the following approach:

Pass the DebugMonitor Class using a new static DatabaseDriver::setMonitorClass(DebugMonitor) which then adds a new instance of DebugMonitor to all existing DatabaseDriver instances, and does the same for any new DatabaseDriver instances as they are created in getInstance.

I think it would also be necessary for the setMonitorClass/getInstance methods to implement ChainedMonitor because when you setMonitorClass existing instances may already have a different monitor, and new instances may be created by a call to getInstance which also includes a monitor in options. However I cannot find a reference to ChainedMonitor - is it an Interface or real code or just a concept? Please provide as much detail as you can and I will code anything necessary for e.g. a ChainedQueryMonitor class (and even a ChainedMonitorInterface if needed).

Since Framework is creating the DebugMonitor instance, we would not be able to easily tell it what options to pass to it e.g. to monitor or not. Nor could we use a method to enable / disable without a callback on creation. But the __Constructor can check for JDEBUG and enable / disable itself without needing it passed as a parameter.

The remainder of the v4 implementation would be similar to the v3.9 one i.e. adding a getInstances method to DatabaseDriver. Regarding signatures, I do not personally think there is a problem with using the getInstance signatures in Debug - the are not displayed to the user, but are simply a unique index for each DatabaseDriver instance used internally by Debug. However if you still object, please suggest an alternative (because I cannot use the object itself as a key).

Overall this would seem to provide a solution which is architecturally sound and which doesn't create any backward incompatibilities with existing Framework v2.

What do you think? If you are OK with this in principle, I will code and test and you can then make a final decision once you see the code.

I have installed J4.0 alpha 2 to use to dev and test the 4.0 version, but when I turn system debug on and configure the Debug plugin, I don't get any debug output in either front or back end.

I know it's an alpha, and I can diagnose this if needed, but was wondering if this was a known issue??

@Sophist-UK I don’t know the answer, but I want to make sure you are testing the latest version here github.com/joomla/joomla-cms/tree/4.0-dev

No. Downloaded the Alpha2 zip file from Github. I think I needed to install this to get the framework files.
But I can update from Git.

@Quy Do you have any views on my proposed v4 solution?

P.S. I have found ChainedMonitor.

Sorry I don’t. This is above my knowledge on this subject.

Nope - no change after I updated from Git.

This is a basic Joomla 4 install from scratch. Nothing special.

all i can say is that debug is working for me out of the box. Please make sure you are using the latest version and not the alpha 2

I have updated from the 4.0-dev branch of git. Is there a zip file I should use instead?

On my system, the Debug constructor is running, but the onAfterRespond event is not being called.

Ok - I have it working now - my mistake in configuration - and a bug in access level checking fixed in #19375.

Pass the DebugMonitor Class using a new static DatabaseDriver::setMonitorClass(DebugMonitor) which then adds a new instance of DebugMonitor to all existing DatabaseDriver instances, and does the same for any new DatabaseDriver instances as they are created in getInstance.

No. We are trying to move away from singleton storage in our APIs and this kind of change not only contradicts that but adds new magic behavior to the static factory. This type of decision is application specific logic and belongs in the application's DI/factory scope. To be honest, DatabaseDriver::getInstance() should be deprecated because in the context of the Database package on its own we are not concerned with whether there are multiple instances of the driver for the same connection, this is an application specific detail if it is warranted.

Since Framework is creating the DebugMonitor instance, we would not be able to easily tell it what options to pass to it e.g. to monitor or not. Nor could we use a method to enable / disable without a callback on creation. But the __Constructor can check for JDEBUG and enable / disable itself without needing it passed as a parameter.

Actually, the DebugMonitor is solely a CMS class only used by the debug plugin (so it's not even part of the CMS' general API, it could just as easily be an anonymous class inside the debug plugin). Change it however you feel is desired, the only catch is it must implement the interface when all is said and done.

I think it would also be necessary for the setMonitorClass/getInstance methods to implement ChainedMonitor

No. The ChainedMonitor is a convenience class to help consumers of our API plug in multiple monitors if desired, but the active database driver is only ever aware of one monitor and that is the one passed into the getInstance method options (since right now we don't have a setter for it in the API). If we're going to force that to support chains then honestly I'd suggest replacing it with immutable events otherwise the monitor very quickly becomes a lightweight version of a pub/sub or event system.

Ok - I get what you are saying about having clear water between CMS and Framework, and I am happy to rethink on how to do this entirely within the CMS layer. But now I am unclear how an extension like Fabrik is supposed to create a databasedriver instance using the CMS factory.

I guess for v4 we can use createDbo - so I can hook in there instead. But this is apparently deprecated for v5. I guess I could do with an example on how a component would create a DatabaseDriver instance to a different database.

For now keep using the existing static getInstance method or the DatabaseFactory.

Joomla\CMS\Factory is still geared toward the CMS' primary database connection and will continue to be, this hasn't ever really been hookable by extensions except for the fact all of its properties are public and can be overwritten.

Now we have a chicken and egg - I need to have all DatabaseDriver instances created through a central point so I can attach the DebugMonitor, but you are telling me that additional databases should be defined by going directly to the Framework.

So the choices seem to be to either do a hack to extend existing Factory createDbo to be able to create non-default DatabaseDriver instances or create a proper CMS factory layer between Joomla CMS extensions and the Framework. Or is that exactly what you are trying to move away from?

With all of the work happening in the 4.0 branch, the aim is to move toward the use of service factories and rely less on static factory methods and singleton object storage. Presumably we'll wire the database service provider to use the DatabaseFactory class to build drivers and have that be a service in the DI container that can be extended as is the intent with our other service factories.

Just remember though that the database is one of the services that without core hacking you can't really touch the initial connection's configuration as the database is needed to get the available plugins at an absolute minimum.

Very high level, you have a database service provider registering default services for the application's main database connection and the database factory, like https://github.com/joomla-framework/database/blob/2.0-dev/src/Service/DatabaseProvider.php

In the application (in this case the CMS), an extension wanting to add some custom logic would extend the factory service to introduce their own factory with their extra custom logic, something like this:

use Joomla\Database\DatabaseFactory;
use Joomla\DI\Container;

$container->extend(
    DatabaseFactory::class,
    function (DatabaseFactory $factory, Container $container)
    {
        return new class extends DatabaseFactory
        {
            public function getDriver($name = 'mysqli', array $options = [])
            {
                $driver = parent::getDriver($name, $options);

                // Custom logic for each created driver here

                return $driver;
            }
        }
    }
);

And where do I put that code? A new file I assume, but called what, located where and linked in how?

I assume that I would redirect legacy JDatabaseDriver through that instead of direct to the Framework DatabaseDriver - or is that handled somehow automatically.

This all seems complicated to the novice, though I can already see why it makes things simpler in the end by making everything more loosely coupled.

Right now you wouldn't put that code anywhere because the CMS isn't directing calls to create drivers through the service factory, it's all still going through the static getInstance method for the moment. But when we get to a state where it goes through the factory, you'd use a system plugin and modify the container with the onAfterInitialise event.

Also I think we will still have a problem because we won't know whether the DatabaseDriver will return an existing instance or a new instance, and we cannot tell at the moment whether the monitor has been set, and setting it again to a new Monitor instance would lose any data already collected by the existing monitor instance. At a minimum we need to be able either to tell that it is new or to be able to get the existing monitor value to check if has already been set.

No, you don't need to know if something is new or not. When using that service factory, any time your code gets called it is always going to create a new driver instance (there is no singleton storage involved there). Which means at the absolute worst right now there isn't a way to pull a monitor out of a driver except through Reflection, and that is only going to affect the database connection created by core to get to the point where plugins can start adding their custom logic (and since core itself isn't creating a monitor under any case right now there is no conflict to worry about until you get to the point the debug plugin is constructed).

It seems like I need to get us to the state where we direct calls through the factory. So all legacy routes using getDbo need to be funnelled through it and components like Fabrik will need to switch to using it. (Fabrik already has a Worker function for creating database instances, though it is not yet universally used, so for V4 support we need to move all Fabrik connections to use the Fabrik worker function and make the worker function go through the service.)

I am getting more confused rather than less confused, and this is now getting to the point that I am taking more of your time talking about this interface that it would take you to write the service layer yourself without any additional logic and for me then to extend it to make Debug support multiple drivers.

I do, however, believe that we need to provide two additional framework functions to avoid going through hoops:

1. To have a DatabaseDriver->getMonitor function so that we don't have to store the monitor instance separately and then try to connect the monitor instance with the drive instance.

2. To have a DatabaseDriver::getInstances function so that we can list the instances.

Without these we would have to keep another layer of caching in the CMS DI factory layer to provide this functionality. Have I got this wrong? Would this break the architectural model?

(To be honest, the framework supports multiple database drivers, Joomla recognises that extensions might create new drivers - so shouldn't Joomla facilitate the management of these as standard (including handling debug) in order to be architecturally sound? I appreciate that making Joomla more architecturally sound is a WIP, but any help you can give me here is definitely appreciated.)

It is two years since the last significant comment. Is anything happening with this?

This was (or is) a J3 enhancement to support tools like Fabrik which can use multiple database connections - and debug doesn't currently display the SQL calls for these,.

I asked the Fabrik team to help write the J4 shim to support multiple database connections (which they will need to support J4 anyway), but that help was not forthcoming. So I have not been able to write a J4 version of this myself.

Well as J3 is not accepting anything other than bug fixes and there is nothing coming for J4 I guess this should be closed

Due to lack of support by Fabrik's new owners, I have stopped Fabrik-based web sites and cannot support this PR. Closing.