Skip to content

Commit

Permalink
Cleanup query logging code.
Browse files Browse the repository at this point in the history
Remove code duplication
  • Loading branch information
ADmad committed Jan 27, 2025
1 parent b88e5ac commit af71830
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 202 deletions.
29 changes: 3 additions & 26 deletions src/Error/ExceptionRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -20,6 +20,8 @@
*/
class ExceptionRenderer extends WebExceptionRenderer
{
use QueryLogTrait;

/**
* Renders validation errors and sends a 422 error code
*
Expand Down Expand Up @@ -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;
}
}
63 changes: 7 additions & 56 deletions src/Listener/ApiQueryLogListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,6 +23,8 @@
*/
class ApiQueryLogListener extends BaseListener
{
use QueryLogTrait;

/**
* {@inheritDoc}
*
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}

/**
Expand All @@ -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();
}
}
40 changes: 40 additions & 0 deletions src/Traits/QueryLogTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php
declare(strict_types=1);

namespace Crud\Traits;

use Cake\Datasource\ConnectionManager;

/**
* QueryLogTrait
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
*/
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;

Check warning on line 28 in src/Traits/QueryLogTrait.php

View check run for this annotation

Codecov / codecov/patch

src/Traits/QueryLogTrait.php#L28

Added line #L28 was not covered by tests
}

$logger = $driver->getLogger();
if ($logger && method_exists($logger, 'getLogs')) {
/** @var \Crud\Log\QueryLogger $logger */
$queryLog[$source] = $logger->getLogs();
}
}

return $queryLog;
}
}
126 changes: 6 additions & 120 deletions tests/TestCase/Listener/ApiQueryLogListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Cake\Core\Configure;
use Cake\Database\Connection;

Check failure on line 8 in tests/TestCase/Listener/ApiQueryLogListenerTest.php

View workflow job for this annotation

GitHub Actions / cs-stan / Coding Standard & Static Analysis

Type Cake\Database\Connection is not used in this file.
use Cake\Database\Driver;

Check failure on line 9 in tests/TestCase/Listener/ApiQueryLogListenerTest.php

View workflow job for this annotation

GitHub Actions / cs-stan / Coding Standard & Static Analysis

Type Cake\Database\Driver is not used in this file.
use Cake\Datasource\ConnectionManager;

Check failure on line 10 in tests/TestCase/Listener/ApiQueryLogListenerTest.php

View workflow job for this annotation

GitHub Actions / cs-stan / Coding Standard & Static Analysis

Type Cake\Datasource\ConnectionManager is not used in this file.
use Cake\Event\Event;
use Cake\Http\ServerRequest;
use Crud\Action\BaseAction;
Expand Down Expand Up @@ -99,11 +100,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'));
}
Expand Down Expand Up @@ -142,7 +143,7 @@ public function testBeforeRenderDebugTrue()
$Instance = $this
->getMockBuilder(ApiQueryLogListener::class)
->disableOriginalConstructor()
->onlyMethods(['_getQueryLogs', '_action', '_controller'])
->onlyMethods(['getQueryLogs', '_action', '_controller'])
->getMock();
$Instance
->expects($this->once())
Expand All @@ -154,128 +155,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'));
Expand Down

0 comments on commit af71830

Please sign in to comment.