Skip to content

Commit af71830

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

File tree

4 files changed

+56
-202
lines changed

4 files changed

+56
-202
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

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

tests/TestCase/Listener/ApiQueryLogListenerTest.php

+6-120
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Cake\Core\Configure;
88
use Cake\Database\Connection;
99
use Cake\Database\Driver;
10+
use Cake\Datasource\ConnectionManager;
1011
use Cake\Event\Event;
1112
use Cake\Http\ServerRequest;
1213
use Crud\Action\BaseAction;
@@ -99,11 +100,11 @@ public function testBeforeRenderDebugFalse()
99100
$Instance = $this
100101
->getMockBuilder(ApiQueryLogListener::class)
101102
->disableOriginalConstructor()
102-
->onlyMethods(['_getQueryLogs'])
103+
->onlyMethods(['getQueryLogs'])
103104
->getMock();
104105
$Instance
105106
->expects($this->never())
106-
->method('_getQueryLogs');
107+
->method('getQueryLogs');
107108

108109
$Instance->beforeRender(new Event('something'));
109110
}
@@ -142,7 +143,7 @@ public function testBeforeRenderDebugTrue()
142143
$Instance = $this
143144
->getMockBuilder(ApiQueryLogListener::class)
144145
->disableOriginalConstructor()
145-
->onlyMethods(['_getQueryLogs', '_action', '_controller'])
146+
->onlyMethods(['getQueryLogs', '_action', '_controller'])
146147
->getMock();
147148
$Instance
148149
->expects($this->once())
@@ -154,128 +155,13 @@ public function testBeforeRenderDebugTrue()
154155
->willReturn($Controller);
155156
$Instance
156157
->expects($this->once())
157-
->method('_getQueryLogs')
158+
->method('getQueryLogs')
158159
->willReturn([]);
159160

160161
$Instance->beforeRender(new Event('something'));
161162
}
162163

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()
164+
public function testQueryLogs()
279165
{
280166
$listener = new ApiQueryLogListener(new Controller(new ServerRequest()));
281167
$listener->setupLogging(new Event('something'));

0 commit comments

Comments
 (0)