Use QueryMonitorInterface instead of hard-coded DebugMonitor class

[AlterBrains]

Summary of Changes

Allow debug plugin to use instance of QueryMonitorInterface in query collector.

Currently, it's hard-coded to instance of DebugMonitor (which is a final class for some reason). Hence, we can't use custom monitors with debug plugin enabled.

Testing Instructions

Apply patch. Enable debug in global configuration and debug plugin.

Actual result BEFORE applying this Pull Request

See debug.

Expected result AFTER applying this Pull Request

See same debug, nothing is changed.

Link to documentations

Please select:

• No documentation changes for docs.joomla.org needed
• No documentation changes for manual.joomla.org needed

[ceford]

I have tested this item ✅ successfully on 1f1117f

No change in Database section of debug bar on applying this PR.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45579.}

[RickR2H]

I have tested this item ✅ successfully on 1f1117f

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45579.}

[RickR2H]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45579.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45579.}

[HLeithner]

Hi,

I'm sorry but looking at this pull request this change has a bigger issue.

You change the type hint from DebugMonitor to QueryMonitorInterface, which sounds theoretically ok. But if you look at the interface you will notices that only 2 methods are defined (start and stopQuery). Which are not even used in the QueryCollector. On the other hand the QueryCollector uses

• getLogs()
• getBoundParams()
• getTimings()
• getMemoryLogs()
• getCallStacks()

which all are not defined by this interface. So it would be wrong to use this interface as type hint.

Further more, debug functionality in joomla is not part of the b/c policy and not meant to be extended, which I would expect is the reason it's final.

Which limitation do you encounter, could it be solved in the core directly and would other people benefit?

[AlterBrains]

I created Sentry.io integration plugin (already in JED) and need to log SQL queries via custom monitor class. If debug is enabled, we have to apply a weird workaround to replace our custom debug monitor with native debug monitor, otherwise debug plugin fails.

I am happy to add missing methods to interface, it will help and won't break anything.

[laoneo]

Adding new methods to an interface breaks per se the classes implementing the interface ;-)

[AlterBrains]

I am afraid I was the first one who created non-core monitor :)

Maybe we can at least not make the current monitor class final?

[AlterBrains]

@laoneo , @HLeithner
So, can we just remove the final statement from Joomla\Database\Monitor\DebugMonitor? I will update the PR.
Otherwise, we have hard-coded debug plugin which limits 3rd-party debugging functionality. Sentry is life-saver and really helps the developers, but with enabled debug we have a headache to collect SQL data for Sentry.

[laoneo]

Can you prepare the pr in a way how it would work then for you, so we can discus this in maintainers meeting. There should be reason why the class is final.

[AlterBrains]

@laoneo I think it's easier to just remove final class limitation, but it requires a patch in Joomla database package https://github.com/joomla-framework/database

Btw it's strange that Joomla\Database\Monitor\DebugMonitor contains plugin-specific logic, framework package relies on QueryMonitorInterface but DebugMonitor should be in plugin src as plugin-specific class.

If I add plugin-specific Joomla\Plugin\System\Debug\Monitor\DebugMonitor class (copy from package) but without final limitations, there will be no need to patch framework package.

What do you think?

[HLeithner]

Btw it's strange that Joomla\Database\Monitor\DebugMonitor contains plugin-specific logic, framework package relies on QueryMonitorInterface but DebugMonitor should be in plugin src as plugin-specific class.

can you elaborate where you see plugin specific code in this class?

I see 2 ways to go, create an Interface or remove final... I personally would go the interface way.

[AlterBrains]

Joomla\Database\Monitor\DebugMonitor has methods only used in debug plugin, but not in database package - the methods you mentioned:

• getLogs()
• getBoundParams()
• getTimings()
• getMemoryLogs()
• getCallStacks()

This logic is out of database package, it's the logic from debug plugin itself.

So, you think to: either remove final or create a new debug plugin-specific interface and use it in the plugin.
But, with the new interface the final DebugMonitor still can't be extended and used in my code. Useless.

Final is evil here. We can't extend Joomla code.