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
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
@@ -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;
}
}
63 changes: 7 additions & 56 deletions src/Listener/ApiQueryLogListener.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
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

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
@@ -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'));