From 755a38e514e32f9237103e72b313973a63e1b311 Mon Sep 17 00:00:00 2001 From: ADmad 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 @@ +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'));