Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup query logging code. #754

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
}
34 changes: 34 additions & 0 deletions src/Traits/QueryLogTrait.php
Original file line number Diff line number Diff line change
@@ -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;

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

View check run for this annotation

Codecov / codecov/patch

src/Traits/QueryLogTrait.php#L22

Added line #L22 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;
}
}
128 changes: 5 additions & 123 deletions tests/TestCase/Listener/ApiQueryLogListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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