Skip to content

Commit 755a38e

Browse files
committed
Cleanup query logging code.
Remove code duplication
1 parent b88e5ac commit 755a38e

File tree

4 files changed

+49
-205
lines changed

4 files changed

+49
-205
lines changed

src/Error/ExceptionRenderer.php

+3-26
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
namespace Crud\Error;
55

66
use Cake\Core\Configure;
7-
use Cake\Datasource\ConnectionManager;
87
use Cake\Error\Debugger;
98
use Cake\Error\Renderer\WebExceptionRenderer;
109
use Cake\Http\Response;
1110
use Crud\Error\Exception\ValidationException;
11+
use Crud\Traits\QueryLogTrait;
1212
use Exception;
1313
use function Cake\Core\h;
1414

@@ -20,6 +20,8 @@
2020
*/
2121
class ExceptionRenderer extends WebExceptionRenderer
2222
{
23+
use QueryLogTrait;
24+
2325
/**
2426
* Renders validation errors and sends a 422 error code
2527
*
@@ -120,29 +122,4 @@ protected function _getErrorData(): array
120122

121123
return $data;
122124
}
123-
124-
/**
125-
* Helper method to get query log.
126-
*
127-
* @return array Query log.
128-
*/
129-
protected function _getQueryLog(): array
130-
{
131-
$queryLog = [];
132-
$sources = ConnectionManager::configured();
133-
foreach ($sources as $source) {
134-
$driver = ConnectionManager::get($source)->getDriver();
135-
if (!method_exists($driver, 'getLogger')) {
136-
continue;
137-
}
138-
139-
$logger = $driver->getLogger();
140-
if ($logger && method_exists($logger, 'getLogs')) {
141-
/** @var \Crud\Log\QueryLogger $logger */
142-
$queryLog[$source] = $logger->getLogs();
143-
}
144-
}
145-
146-
return $queryLog;
147-
}
148125
}

src/Listener/ApiQueryLogListener.php

+7-56
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
namespace Crud\Listener;
55

66
use Cake\Core\Configure;
7-
use Cake\Datasource\ConnectionInterface;
87
use Cake\Datasource\ConnectionManager;
98
use Cake\Datasource\Exception\MissingDatasourceConfigException;
109
use Cake\Event\EventInterface;
1110
use Crud\Log\QueryLogger;
11+
use Crud\Traits\QueryLogTrait;
1212

1313
/**
1414
* When loaded Crud API will include query logs in the response
@@ -23,6 +23,8 @@
2323
*/
2424
class ApiQueryLogListener extends BaseListener
2525
{
26+
use QueryLogTrait;
27+
2628
/**
2729
* {@inheritDoc}
2830
*
@@ -60,11 +62,11 @@ public function implementedEvents(): array
6062
*/
6163
public function setupLogging(EventInterface $event): void
6264
{
63-
$connections = $this->getConfig('connections') ?: $this->_getSources();
65+
$connections = $this->getConfig('connections') ?: ConnectionManager::configured();
6466

6567
foreach ($connections as $connectionName) {
6668
try {
67-
$driver = $this->_getSource($connectionName)->getDriver();
69+
$driver = ConnectionManager::get($connectionName)->getDriver();
6870
if (method_exists($driver, 'setLogger')) {
6971
$driver->setLogger(new QueryLogger());
7072
}
@@ -87,35 +89,7 @@ public function beforeRender(EventInterface $event): void
8789
}
8890

8991
$this->_action()->setConfig('serialize.queryLog', 'queryLog');
90-
$this->_controller()->set('queryLog', $this->_getQueryLogs());
91-
}
92-
93-
/**
94-
* Get the query logs for all sources
95-
*
96-
* @return array
97-
*/
98-
protected function _getQueryLogs(): array
99-
{
100-
$sources = $this->_getSources();
101-
102-
$queryLog = [];
103-
foreach ($sources as $source) {
104-
$driver = $this->_getSource($source)->getDriver();
105-
if (!method_exists($driver, 'getLogger')) {
106-
continue;
107-
}
108-
109-
$logger = $driver->getLogger();
110-
if (method_exists($logger, 'getLogs')) {
111-
/**
112-
* @var \Crud\Log\QueryLogger $logger
113-
*/
114-
$queryLog[$source] = $logger->getLogs();
115-
}
116-
}
117-
118-
return $queryLog;
92+
$this->_controller()->set('queryLog', $this->getQueryLogs());
11993
}
12094

12195
/**
@@ -125,29 +99,6 @@ protected function _getQueryLogs(): array
12599
*/
126100
public function getQueryLogs(): array
127101
{
128-
return $this->_getQueryLogs();
129-
}
130-
131-
/**
132-
* Get a list of sources defined in database.php
133-
*
134-
* @return array
135-
* @codeCoverageIgnore
136-
*/
137-
protected function _getSources(): array
138-
{
139-
return ConnectionManager::configured();
140-
}
141-
142-
/**
143-
* Get a specific data source
144-
*
145-
* @param string $source Datasource name
146-
* @return \Cake\Datasource\ConnectionInterface
147-
* @codeCoverageIgnore
148-
*/
149-
protected function _getSource(string $source): ConnectionInterface
150-
{
151-
return ConnectionManager::get($source);
102+
return $this->_getQueryLog();
152103
}
153104
}

src/Traits/QueryLogTrait.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Crud\Traits;
5+
6+
use Cake\Datasource\ConnectionManager;
7+
8+
trait QueryLogTrait
9+
{
10+
/**
11+
* Get the query logs
12+
*
13+
* @return array
14+
*/
15+
protected function _getQueryLog(): array
16+
{
17+
$queryLog = [];
18+
19+
foreach (ConnectionManager::configured() as $source) {
20+
$driver = ConnectionManager::get($source)->getDriver();
21+
if (!method_exists($driver, 'getLogger')) {
22+
continue;
23+
}
24+
25+
$logger = $driver->getLogger();
26+
if ($logger && method_exists($logger, 'getLogs')) {
27+
/** @var \Crud\Log\QueryLogger $logger */
28+
$queryLog[$source] = $logger->getLogs();
29+
}
30+
}
31+
32+
return $queryLog;
33+
}
34+
}

tests/TestCase/Listener/ApiQueryLogListenerTest.php

+5-123
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,10 @@
55

66
use Cake\Controller\Controller;
77
use Cake\Core\Configure;
8-
use Cake\Database\Connection;
9-
use Cake\Database\Driver;
108
use Cake\Event\Event;
119
use Cake\Http\ServerRequest;
1210
use Crud\Action\BaseAction;
1311
use Crud\Listener\ApiQueryLogListener;
14-
use Crud\Log\QueryLogger;
1512
use Crud\TestSuite\TestCase;
1613

1714
/**
@@ -99,11 +96,11 @@ public function testBeforeRenderDebugFalse()
9996
$Instance = $this
10097
->getMockBuilder(ApiQueryLogListener::class)
10198
->disableOriginalConstructor()
102-
->onlyMethods(['_getQueryLogs'])
99+
->onlyMethods(['getQueryLogs'])
103100
->getMock();
104101
$Instance
105102
->expects($this->never())
106-
->method('_getQueryLogs');
103+
->method('getQueryLogs');
107104

108105
$Instance->beforeRender(new Event('something'));
109106
}
@@ -142,7 +139,7 @@ public function testBeforeRenderDebugTrue()
142139
$Instance = $this
143140
->getMockBuilder(ApiQueryLogListener::class)
144141
->disableOriginalConstructor()
145-
->onlyMethods(['_getQueryLogs', '_action', '_controller'])
142+
->onlyMethods(['getQueryLogs', '_action', '_controller'])
146143
->getMock();
147144
$Instance
148145
->expects($this->once())
@@ -154,128 +151,13 @@ public function testBeforeRenderDebugTrue()
154151
->willReturn($Controller);
155152
$Instance
156153
->expects($this->once())
157-
->method('_getQueryLogs')
154+
->method('getQueryLogs')
158155
->willReturn([]);
159156

160157
$Instance->beforeRender(new Event('something'));
161158
}
162159

163-
/**
164-
* Test setting up the query loggers
165-
*
166-
* @return void
167-
*/
168-
public function testSetupLogging()
169-
{
170-
$driver = $this
171-
->getMockBuilder(Driver::class)
172-
->getMock();
173-
$driver
174-
->expects($this->once())
175-
->method('setLogger')
176-
->with($this->isInstanceOf(QueryLogger::class));
177-
$driver
178-
->expects($this->any())
179-
->method('enabled')
180-
->willReturn(true);
181-
182-
$DefaultSource = new Connection([
183-
'name' => 'default',
184-
'driver' => $driver,
185-
]);
186-
187-
$Instance = $this
188-
->getMockBuilder(ApiQueryLogListener::class)
189-
->disableOriginalConstructor()
190-
->onlyMethods(['_getSources', '_getSource'])
191-
->getMock();
192-
$Instance
193-
->expects($this->once())
194-
->method('_getSources')
195-
->willReturn(['default']);
196-
$Instance
197-
->expects($this->any())
198-
->method('_getSource')
199-
->with('default')
200-
->willReturn($DefaultSource);
201-
202-
$Instance->setupLogging(new Event('something'));
203-
}
204-
205-
/**
206-
* Test setting up only specific query loggers
207-
*
208-
* @return void
209-
*/
210-
public function testSetupLoggingConfiguredSources()
211-
{
212-
$driver = $this->getMockBuilder(Driver::class)
213-
->disableOriginalConstructor()
214-
->getMock();
215-
$driver
216-
->expects($this->any())
217-
->method('enabled')
218-
->willReturn(true);
219-
$driver2 = $this->getMockBuilder(Driver::class)
220-
->disableOriginalConstructor()
221-
->getMock();
222-
$driver2
223-
->expects($this->any())
224-
->method('enabled')
225-
->willReturn(true);
226-
227-
$DefaultSource = new Connection([
228-
'name' => 'default',
229-
'driver' => $driver,
230-
]);
231-
$TestSource = new Connection([
232-
'name' => 'test',
233-
'driver' => $driver2,
234-
]);
235-
236-
$Instance = $this
237-
->getMockBuilder(ApiQueryLogListener::class)
238-
->disableOriginalConstructor()
239-
->onlyMethods(['_getSources', '_getSource'])
240-
->getMock();
241-
$Instance
242-
->expects($this->never())
243-
->method('_getSources');
244-
245-
$Instance
246-
->expects($this->exactly(2))
247-
->method('_getSource')
248-
->with(...self::withConsecutive(['default'], ['test']))
249-
->willReturnOnConsecutiveCalls($DefaultSource, $TestSource);
250-
251-
$Instance->setConfig('connections', ['default', 'test']);
252-
$Instance->setupLogging(new Event('something'));
253-
}
254-
255-
/**
256-
* Test getting query logs using protected method
257-
*
258-
* @return void
259-
*/
260-
public function testProtectedGetQueryLogs()
261-
{
262-
$listener = new ApiQueryLogListener(new Controller(new ServerRequest()));
263-
$listener->setupLogging(new Event('something'));
264-
$this->setReflectionClassInstance($listener);
265-
266-
$expected = [
267-
'test' => [],
268-
];
269-
270-
$this->assertEquals($expected, $this->callProtectedMethod('_getQueryLogs', [], $listener));
271-
}
272-
273-
/**
274-
* Test getting query logs using public getter.
275-
*
276-
* @return void
277-
*/
278-
public function testPublicGetQueryLogs()
160+
public function testQueryLogs()
279161
{
280162
$listener = new ApiQueryLogListener(new Controller(new ServerRequest()));
281163
$listener->setupLogging(new Event('something'));

0 commit comments

Comments
 (0)