From 755a38e514e32f9237103e72b313973a63e1b311 Mon Sep 17 00:00:00 2001
From: ADmad <admad.coder@gmail.com>
Date: Mon, 27 Jan 2025 21:48:26 +0530
Subject: [PATCH] Cleanup query logging code.
Remove code duplication
---
src/Error/ExceptionRenderer.php | 29 +---
src/Listener/ApiQueryLogListener.php | 63 +--------
src/Traits/QueryLogTrait.php | 34 +++++
.../Listener/ApiQueryLogListenerTest.php | 128 +-----------------
4 files changed, 49 insertions(+), 205 deletions(-)
create mode 100644 src/Traits/QueryLogTrait.php
diff --git a/src/Error/ExceptionRenderer.php b/src/Error/ExceptionRenderer.php
index 0cd1cece4..589cd25f4 100644
--- a/src/Error/ExceptionRenderer.php
+++ b/src/Error/ExceptionRenderer.php
@@ -4,11 +4,11 @@
namespace Crud\Error;
use Cake\Core\Configure;
-use Cake\Datasource\ConnectionManager;
use Cake\Error\Debugger;
use Cake\Error\Renderer\WebExceptionRenderer;
use Cake\Http\Response;
use Crud\Error\Exception\ValidationException;
+use Crud\Traits\QueryLogTrait;
use Exception;
use function Cake\Core\h;
@@ -20,6 +20,8 @@
*/
class ExceptionRenderer extends WebExceptionRenderer
{
+ use QueryLogTrait;
+
/**
* Renders validation errors and sends a 422 error code
*
@@ -120,29 +122,4 @@ protected function _getErrorData(): array
return $data;
}
-
- /**
- * Helper method to get query log.
- *
- * @return array Query log.
- */
- protected function _getQueryLog(): array
- {
- $queryLog = [];
- $sources = ConnectionManager::configured();
- foreach ($sources as $source) {
- $driver = ConnectionManager::get($source)->getDriver();
- if (!method_exists($driver, 'getLogger')) {
- continue;
- }
-
- $logger = $driver->getLogger();
- if ($logger && method_exists($logger, 'getLogs')) {
- /** @var \Crud\Log\QueryLogger $logger */
- $queryLog[$source] = $logger->getLogs();
- }
- }
-
- return $queryLog;
- }
}
diff --git a/src/Listener/ApiQueryLogListener.php b/src/Listener/ApiQueryLogListener.php
index 89eec7178..379384af3 100644
--- a/src/Listener/ApiQueryLogListener.php
+++ b/src/Listener/ApiQueryLogListener.php
@@ -4,11 +4,11 @@
namespace Crud\Listener;
use Cake\Core\Configure;
-use Cake\Datasource\ConnectionInterface;
use Cake\Datasource\ConnectionManager;
use Cake\Datasource\Exception\MissingDatasourceConfigException;
use Cake\Event\EventInterface;
use Crud\Log\QueryLogger;
+use Crud\Traits\QueryLogTrait;
/**
* When loaded Crud API will include query logs in the response
@@ -23,6 +23,8 @@
*/
class ApiQueryLogListener extends BaseListener
{
+ use QueryLogTrait;
+
/**
* {@inheritDoc}
*
@@ -60,11 +62,11 @@ public function implementedEvents(): array
*/
public function setupLogging(EventInterface $event): void
{
- $connections = $this->getConfig('connections') ?: $this->_getSources();
+ $connections = $this->getConfig('connections') ?: ConnectionManager::configured();
foreach ($connections as $connectionName) {
try {
- $driver = $this->_getSource($connectionName)->getDriver();
+ $driver = ConnectionManager::get($connectionName)->getDriver();
if (method_exists($driver, 'setLogger')) {
$driver->setLogger(new QueryLogger());
}
@@ -87,35 +89,7 @@ public function beforeRender(EventInterface $event): void
}
$this->_action()->setConfig('serialize.queryLog', 'queryLog');
- $this->_controller()->set('queryLog', $this->_getQueryLogs());
- }
-
- /**
- * Get the query logs for all sources
- *
- * @return array
- */
- protected function _getQueryLogs(): array
- {
- $sources = $this->_getSources();
-
- $queryLog = [];
- foreach ($sources as $source) {
- $driver = $this->_getSource($source)->getDriver();
- if (!method_exists($driver, 'getLogger')) {
- continue;
- }
-
- $logger = $driver->getLogger();
- if (method_exists($logger, 'getLogs')) {
- /**
- * @var \Crud\Log\QueryLogger $logger
- */
- $queryLog[$source] = $logger->getLogs();
- }
- }
-
- return $queryLog;
+ $this->_controller()->set('queryLog', $this->getQueryLogs());
}
/**
@@ -125,29 +99,6 @@ protected function _getQueryLogs(): array
*/
public function getQueryLogs(): array
{
- return $this->_getQueryLogs();
- }
-
- /**
- * Get a list of sources defined in database.php
- *
- * @return array
- * @codeCoverageIgnore
- */
- protected function _getSources(): array
- {
- return ConnectionManager::configured();
- }
-
- /**
- * Get a specific data source
- *
- * @param string $source Datasource name
- * @return \Cake\Datasource\ConnectionInterface
- * @codeCoverageIgnore
- */
- protected function _getSource(string $source): ConnectionInterface
- {
- return ConnectionManager::get($source);
+ return $this->_getQueryLog();
}
}
diff --git a/src/Traits/QueryLogTrait.php b/src/Traits/QueryLogTrait.php
new file mode 100644
index 000000000..0082953ef
--- /dev/null
+++ b/src/Traits/QueryLogTrait.php
@@ -0,0 +1,34 @@
+<?php
+declare(strict_types=1);
+
+namespace Crud\Traits;
+
+use Cake\Datasource\ConnectionManager;
+
+trait QueryLogTrait
+{
+ /**
+ * Get the query logs
+ *
+ * @return array
+ */
+ protected function _getQueryLog(): array
+ {
+ $queryLog = [];
+
+ foreach (ConnectionManager::configured() as $source) {
+ $driver = ConnectionManager::get($source)->getDriver();
+ if (!method_exists($driver, 'getLogger')) {
+ continue;
+ }
+
+ $logger = $driver->getLogger();
+ if ($logger && method_exists($logger, 'getLogs')) {
+ /** @var \Crud\Log\QueryLogger $logger */
+ $queryLog[$source] = $logger->getLogs();
+ }
+ }
+
+ return $queryLog;
+ }
+}
diff --git a/tests/TestCase/Listener/ApiQueryLogListenerTest.php b/tests/TestCase/Listener/ApiQueryLogListenerTest.php
index 0cdf343db..badf517ac 100644
--- a/tests/TestCase/Listener/ApiQueryLogListenerTest.php
+++ b/tests/TestCase/Listener/ApiQueryLogListenerTest.php
@@ -5,13 +5,10 @@
use Cake\Controller\Controller;
use Cake\Core\Configure;
-use Cake\Database\Connection;
-use Cake\Database\Driver;
use Cake\Event\Event;
use Cake\Http\ServerRequest;
use Crud\Action\BaseAction;
use Crud\Listener\ApiQueryLogListener;
-use Crud\Log\QueryLogger;
use Crud\TestSuite\TestCase;
/**
@@ -99,11 +96,11 @@ public function testBeforeRenderDebugFalse()
$Instance = $this
->getMockBuilder(ApiQueryLogListener::class)
->disableOriginalConstructor()
- ->onlyMethods(['_getQueryLogs'])
+ ->onlyMethods(['getQueryLogs'])
->getMock();
$Instance
->expects($this->never())
- ->method('_getQueryLogs');
+ ->method('getQueryLogs');
$Instance->beforeRender(new Event('something'));
}
@@ -142,7 +139,7 @@ public function testBeforeRenderDebugTrue()
$Instance = $this
->getMockBuilder(ApiQueryLogListener::class)
->disableOriginalConstructor()
- ->onlyMethods(['_getQueryLogs', '_action', '_controller'])
+ ->onlyMethods(['getQueryLogs', '_action', '_controller'])
->getMock();
$Instance
->expects($this->once())
@@ -154,128 +151,13 @@ public function testBeforeRenderDebugTrue()
->willReturn($Controller);
$Instance
->expects($this->once())
- ->method('_getQueryLogs')
+ ->method('getQueryLogs')
->willReturn([]);
$Instance->beforeRender(new Event('something'));
}
- /**
- * Test setting up the query loggers
- *
- * @return void
- */
- public function testSetupLogging()
- {
- $driver = $this
- ->getMockBuilder(Driver::class)
- ->getMock();
- $driver
- ->expects($this->once())
- ->method('setLogger')
- ->with($this->isInstanceOf(QueryLogger::class));
- $driver
- ->expects($this->any())
- ->method('enabled')
- ->willReturn(true);
-
- $DefaultSource = new Connection([
- 'name' => 'default',
- 'driver' => $driver,
- ]);
-
- $Instance = $this
- ->getMockBuilder(ApiQueryLogListener::class)
- ->disableOriginalConstructor()
- ->onlyMethods(['_getSources', '_getSource'])
- ->getMock();
- $Instance
- ->expects($this->once())
- ->method('_getSources')
- ->willReturn(['default']);
- $Instance
- ->expects($this->any())
- ->method('_getSource')
- ->with('default')
- ->willReturn($DefaultSource);
-
- $Instance->setupLogging(new Event('something'));
- }
-
- /**
- * Test setting up only specific query loggers
- *
- * @return void
- */
- public function testSetupLoggingConfiguredSources()
- {
- $driver = $this->getMockBuilder(Driver::class)
- ->disableOriginalConstructor()
- ->getMock();
- $driver
- ->expects($this->any())
- ->method('enabled')
- ->willReturn(true);
- $driver2 = $this->getMockBuilder(Driver::class)
- ->disableOriginalConstructor()
- ->getMock();
- $driver2
- ->expects($this->any())
- ->method('enabled')
- ->willReturn(true);
-
- $DefaultSource = new Connection([
- 'name' => 'default',
- 'driver' => $driver,
- ]);
- $TestSource = new Connection([
- 'name' => 'test',
- 'driver' => $driver2,
- ]);
-
- $Instance = $this
- ->getMockBuilder(ApiQueryLogListener::class)
- ->disableOriginalConstructor()
- ->onlyMethods(['_getSources', '_getSource'])
- ->getMock();
- $Instance
- ->expects($this->never())
- ->method('_getSources');
-
- $Instance
- ->expects($this->exactly(2))
- ->method('_getSource')
- ->with(...self::withConsecutive(['default'], ['test']))
- ->willReturnOnConsecutiveCalls($DefaultSource, $TestSource);
-
- $Instance->setConfig('connections', ['default', 'test']);
- $Instance->setupLogging(new Event('something'));
- }
-
- /**
- * Test getting query logs using protected method
- *
- * @return void
- */
- public function testProtectedGetQueryLogs()
- {
- $listener = new ApiQueryLogListener(new Controller(new ServerRequest()));
- $listener->setupLogging(new Event('something'));
- $this->setReflectionClassInstance($listener);
-
- $expected = [
- 'test' => [],
- ];
-
- $this->assertEquals($expected, $this->callProtectedMethod('_getQueryLogs', [], $listener));
- }
-
- /**
- * Test getting query logs using public getter.
- *
- * @return void
- */
- public function testPublicGetQueryLogs()
+ public function testQueryLogs()
{
$listener = new ApiQueryLogListener(new Controller(new ServerRequest()));
$listener->setupLogging(new Event('something'));