User tests: Successful: Unsuccessful:
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.
Apply patch. Enable debug in global configuration and debug plugin.
See debug.
See same debug, nothing is changed.
Please select:
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
I have tested this item ✅ successfully on 1f1117f
RTC
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
PR-5.3-dev
|
RTC
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.
Adding new methods to an interface breaks per se the classes implementing the interface ;-)
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?
@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.
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.
@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?
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.
Joomla\Database\Monitor\DebugMonitor
has methods only used in debug plugin, but not in database package - the methods you mentioned:
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.
Joomla\Database\Monitor\DebugMonitor
has methods only used in debug plugin, but not in database package - the methods you mentioned:
This logic is out of database package, it's the logic from debug plugin itself.
So: 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.
For me the best and simplest solution is to have non-final DebugMonitor class in debug plugin src.
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.