10000 bug #34384 [DoctrineBridge] Improve queries parameters display in Pro… · symfony/symfony@c1cab2b · GitHub
[go: up one dir, main page]
Skip to content

Commit c1cab2b

Browse files
fabpotfabpot
committed
bug #34384 [DoctrineBridge] Improve queries parameters display in Profiler (fancyweb)
This PR was merged into the 4.4 branch. Discussion ---------- [DoctrineBridge] Improve queries parameters display in Profiler | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #34234 | License | MIT | Doc PR | - ##  Stringable object: _The query is runnable._ ### Before <img width="773" alt="Screenshot 2019-11-14 at 21 05 53" src="https://user-images.githubusercontent.com/3658119/68892948-669cf180-0724-11ea-889b-8ce781956fc7.png"> ### After <img width="780" alt="Screenshot 2019-11-14 at 20 56 38" src="https://user-images.githubusercontent.com/3658119/68892978-787e9480-0724-11ea-843b-70a595633192.png"> ## Non stringable object: _Exception, the query is not runnable._ ### Before <img width="783" alt="Screenshot 2019-11-14 at 21 05 31" src="https://user-images.githubusercontent.com/3658119/68892993-80d6cf80-0724-11ea-9961-e32b65f81508.png"> ### After <img width="622" alt="Screenshot 2019-11-14 at 20 57 17" src="https://user-images.githubusercontent.com/3658119/68893007-87fddd80-0724-11ea-98cb-2ef695135673.png"> ## Error with an object: _`ConversionException` or a `\TypeError` when you specify the type when you set the parameter but you provide an invalid value and this value is an object (eg `->setParameter('foo', new \stdClass(), 'date')`), the query is not runnable._ ### Before <img width="775" alt="Screenshot 2019-11-14 at 21 04 45" src="https://user-images.githubusercontent.com/3658119/68893078-a9f76000-0724-11ea-9e9a-bb806ffa0a73.png"> ### After <img width="821" alt="Screenshot 2019-11-14 at 20 59 23" src="https://user-images.githubusercontent.com/3658119/68893098-b24f9b00-0724-11ea-8e9d-7179725b8bc1.png"> ## Error with anything else: _`ConversionException` or a `\TypeError` when you specify the type when you set the parameter but you provide an invalid value and this value is not an object (eg `->setParameter('foo', 'bar', 'date')`), the query is not runnable._ ### Before <img width="809" alt="Screenshot 2019-11-14 at 21 05 10" src="https://user-images.githubusercontent.com/3658119/68893031-93e99f80-0724-11ea-9c23-f8d05f2dbfbb.png"> ### After <img width="832" alt="Screenshot 2019-11-14 at 20 57 54" src="https://user-images.githubusercontent.com/3658119/68893055-9d730780-0724-11ea-8ce1-a7b8946aaf94.png"> The new `runnable` key will be used in `DoctrineBundle` profiler template to hide the `View runnable query` link when the query is not runnable. Commits ------- fe15f51 [DoctrineBridge] Improve queries parameters display in Profiler
2 parents 611f819 + fe15f51 commit c1cab2b

File tree

3 files changed

+168
-24
lines changed

3 files changed

+168
-24
lines changed

‎src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Symfony\Component\HttpFoundation\Request;
1919
use Symfony\Component\HttpFoundation\Response;
2020
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
21+
use Symfony\Component\VarDumper\Caster\Caster;
22+
use Symfony\Component\VarDumper\Cloner\Stub;
2123

2224
/**
2325
* DoctrineDataCollector.
@@ -121,6 +123,38 @@ public function getName()
121123
return 'db';
122124
}
123125

126+
/**
127+
* {@inheritdoc}
128+
*/
129+
protected function getCasters()
130+
{
131+
return parent::getCasters() + [
132+
ObjectParameter::class => static function (ObjectParameter $o, array $a, Stub $s): array {
133+
$s->class = $o->getClass();
134+
$s->value = $o->getObject();
135+
136+
$r = new \ReflectionClass($o->getClass());
137+
if ($f = $r->getFileName()) {
138+
$s->attr['file'] = $f;
139+
$s->attr['line'] = $r->getStartLine();
140+
} else {
141+
unset($s->attr['file']);
142+
unset($s->attr['line']);
143+
}
144+
145+
if ($error = $o->getError()) {
146+
return [Caster::PREFIX_VIRTUAL.'⚠' => $error->getMessage()];
147+
}
148+
149+
if ($o->isStringable()) {
150+
return [Caster::PREFIX_VIRTUAL.'__toString()' => (string) $o->getObject()];
151+
}
152+
153+
return [Caster::PREFIX_VIRTUAL.'⚠' => sprintf('Object of class "%s" could not be converted to string.', $o->getClass())];
154+
},
155+
];
156+
}
157+
124158
private function sanitizeQueries(string $connectionName, array $queries): array
125159
{
126160
foreach ($queries as $i => $query) {
@@ -133,6 +167,7 @@ private function sanitizeQueries(string $connectionName, array $queries): array
133167
private function sanitizeQuery(string $connectionName, array $query): array
134168
{
135169
$query['explainable'] = true;
170+
$query['runnable'] = true;
136171
if (null === $query['params']) {
137172
$query['params'] = [];
138173
}
@@ -143,6 +178,7 @@ private function sanitizeQuery(string $connectionName, array $query): array
143178
$query['types'] = [];
144179
}
145180
foreach ($query['params'] as $j => $param) {
181+
$e = null;
146182
if (isset($query['types'][$j])) {
147183
// Transform the param according to the type
148184
$type = $query['types'][$j];
@@ -154,18 +190,19 @@ private function sanitizeQuery(string $connectionName, array $query): array
154190
try {
155191
$param = $type->convertToDatabaseValue($param, $this->registry->getConnection($connectionName)->getDatabasePlatform());
156192
} catch (\TypeError $e) {
157-
// Error thrown while processing params, query is not explainable.
158-
$query['explainable'] = false;
159193
} catch (ConversionException $e) {
160-
$query['explainable'] = false;
161194
}
162195
}
163196
}
164197

165-
list($query['params'][$j], $explainable) = $this->sanitizeParam($param);
198+
list($query['params'][$j], $explainable, $runnable) = $this->sanitizeParam($param, $e);
166199
if (!$explainable) {
167200
$query['explainable'] = false;
168201
}
202+
203+
if (!$runnable) {
204+
$query['runnable'] = false;
205+
}
169206
}
170207

171208
$query['params'] = $this->cloneVar($query['params']);
@@ -180,32 +217,33 @@ private function sanitizeQuery(string $connectionName, array $query): array
180217
* indicating if the original value was kept (allowing to use the sanitized
181218
* value to explain the query).
182219
*/
183-
private function sanitizeParam($var): array
220+
private function sanitizeParam($var, ?\Throwable $error): array
184221
{
185222
if (\is_object($var)) {
186-
$className = \get_class($var);
223+
return [$o = new ObjectParameter($var, $error), false, $o->isStringable() && !$error];
224+
}
187225

188-
return method_exists($var, '__toString') ?
189-
[sprintf('/* Object(%s): */"%s"', $className, $var->__toString()), false] :
190-
[sprintf('/* Object(%s) */', $className), false];
226+
if ($error) {
227+
return ['⚠ '.$error->getMessage(), false, false];
191228
}
192229

193230
if (\is_array($var)) {
194231
$a = [];
195-
$original = true;
232+
$explainable = $runnable = true;
196233
foreach ($var as $k => $v) {
197-
list($value, $orig) = $this->sanitizeParam($v);
198-
$original = $original && $orig;
234+
list($value, $e, $r) = $this->sanitizeParam($v, null);
235+
$explainable = $explainable && $e;
236+
$runnable = $runnable && $r;
199237
$a[$k] = $value;
200238
}
201239

202-
return [$a, $original];
240+
return [$a, $explainable, $runnable];
203241
}
204242

205243
if (\is_resource($var)) {
206-
return [sprintf('/* Resource(%s) */', get_resource_type($var)), false];
244+
return [sprintf('/* Resource(%s) */', get_resource_type($var)), false, false];
207245
}
208246

209-
return [$var, true];
247+
return [$var, true, true];
210248
}
211249
}

‎src/Symfony/Bridge/Doctrine/DataCollector/ObjectParameter.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bridge\Doctrine\DataCollector;
13+
14+
final class ObjectParameter
15+
{
16+
private $object;
17+
private $error;
18+
private $stringable;
19+
private $class;
20+
21+
/**
22+
* @param object $object
23+
*/
24+
public function __construct($object, ?\Throwable $error)
25+
{
26+
$this->object = $object;
27+
$this->error = $error;
28+
$this->stringable = \is_callable([$object, '__toString']);
29+
$this->class = \get_class($object);
30+
}
31+
32+
/**
33+
* @return object
34+
*/
35+
public function getObject()
36+
{
37+
return $this->object;
38+
}
39+
40+
public function getError(): ?\Throwable
41+
{
42+
return $this->error;
43+
}
44+
45+
public function isStringable(): bool
46+
{
47+
return $this->stringable;
48+
}
49+
50+
public function getClass(): string
51+
{
52+
return $this->class;
53+
}
54+
}

‎src/Symfony/Bridge/Doctrine/Tests/DataCollector/DoctrineDataCollectorTest.php

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\HttpFoundation\Request;
1919
use Symfony\Component\HttpFoundation\Response;
2020
use Symfony\Component\VarDumper\Cloner\Data;
21+
use Symfony\Component\VarDumper\Dumper\CliDumper;
2122

2223
class DoctrineDataCollectorTest extends TestCase
2324
{
@@ -74,7 +75,7 @@ public function testCollectTime()
7475
/**
7576
* @dataProvider paramProvider
7677
*/
77-
public function testCollectQueries($param, $types, $expected, $explainable)
78+
public function testCollectQueries($param, $types, $expected, $explainable, bool $runnable = true)
7879
{
7980
$queries = [
8081
['sql' => 'SELECT * FROM table1 WHERE field1 = ?1', 'params' => [$param], 'types' => $types, 'executionMS' => 1],
@@ -83,8 +84,19 @@ public function testCollectQueries($param, $types, $expected, $explainable)
8384
$c->collect(new Request(), new Response());
8485

8586
$collectedQueries = $c->getQueries();
86-
$this->assertEquals($expected, $collectedQueries['default'][0]['params'][0]);
87+
88+
$collectedParam = $collectedQueries['default'][0]['params'][0];
89+
if ($collectedParam instanceof Data) {
90+
$dumper = new CliDumper($out = fopen('php://memory', 'r+b'));
91+
$dumper->setColors(false);
92+
$collectedParam->dump($dumper);
93+
$this->assertStringMatchesFormat($expected, print_r(stream_get_contents($out, -1, 0), true));
94+
} else {
95+
$this->assertEquals($expected, $collectedParam);
96+
}
97+
8798
$this->assertEquals($explainable, $collectedQueries['default'][0]['explainable']);
99+
$this->assertSame($runnable, $collectedQueries['default'][0]['runnable']);
88100
}
89101

90102
public function testCollectQueryWithNoParams()
@@ -100,9 +112,11 @@ public function testCollectQueryWithNoParams()
100112
$this->assertInstanceOf(Data::class, $collectedQueries['default'][0]['params']);
101113
$this->assertEquals([], $collectedQueries['default'][0]['params']->getValue());
102114
$this->assertTrue($collectedQueries['default'][0]['explainable']);
115+
$this->assertTrue($collectedQueries['default'][0]['runnable']);
103116
$this->assertInstanceOf(Data::class, $collectedQueries['default'][1]['params']);
104117
$this->assertEquals([], $collectedQueries['default'][1]['params']->getValue());
105118
$this->assertTrue($collectedQueries['default'][1]['explainable']);
119+
$this->assertTrue($collectedQueries['default'][1]['runnable']);
106120
}
107121

108122
public function testCollectQueryWithNoTypes()
@@ -134,7 +148,7 @@ public function testReset()
134148
/**
135149
* @dataProvider paramProvider
136150
*/
137-
public function testSerialization($param, $types, $expected, $explainable)
151+
public function testSerialization($param, $types, $expected, $explainable, bool $runnable = true)
138152
{
139153
$queries = [
140154
['sql' => 'SELECT * FROM table1 WHERE field1 = ?1', 'params' => [$param], 'types' => $types, 'executionMS' => 1],
@@ -144,8 +158,19 @@ public function testSerialization($param, $types, $expected, $explainable)
144158
$c = unserialize(serialize($c));
145159

146160
$collectedQueries = $c->getQueries();
147-
$this->assertEquals($expected, $collectedQueries['default'][0]['params'][0]);
161+
162+
$collectedParam = $collectedQueries['default'][0]['params'][0];
163+
if ($collectedParam instanceof Data) {
164+
$dumper = new CliDumper($out = fopen('php://memory', 'r+b'));
165+
$dumper->setColors(false);
166+
$collectedParam->dump($dumper);
167+
$this->assertStringMatchesFormat($expected, print_r(stream_get_contents($out, -1, 0), true));
168+
} else {
169+
$this->assertEquals($expected, $collectedParam);
170+
}
171+
148172
$this->assertEquals($explainable, $collectedQueries['default'][0]['explainable']);
173+
$this->assertSame($runnable, $collectedQueries['default'][0]['runnable']);
149174
}
150175

151176
public function paramProvider()
@@ -156,19 +181,46 @@ public function paramProvider()
156181
[true, [], true, true],
157182
[null, [], null, true],
158183
[new \DateTime('2011-09-11'), ['date'], '2011-09-11', true],
159-
[fopen(__FILE__, 'r'), [], '/* Resource(stream) */', false],
160-
[new \stdClass(), [], '/* Object(stdClass) */', false],
184+
[fopen(__FILE__, 'r'), [], '/* Resource(stream) */', false, false],
185+
[
186+
new \stdClass(),
187+
[],
188+
<<<EOTXT
189+
{#%d
190+
⚠: "Object of class "stdClass" could not be converted to string."
191+
}
192+
EOTXT
193+
,
194+
false,
195+
false,
196+
],
161197
[
162198
new StringRepresentableClass(),
163199
[],
164-
'/* Object(Symfony\Bridge\Doctrine\Tests\DataCollector\StringRepresentableClass): */"string representation"',
200+
<<<EOTXT
201+
Symfony\Bridge\Doctrine\Tests\DataCollector\StringRepresentableClass {#%d
202+
__toString(): "string representation"
203+
}
204+
EOTXT
205+
,
165206
false,
166207
],
167208
];
168209

169210
if (version_compare(Version::VERSION, '2.6', '>=')) {
170-
$tests[] = ['this is not a date', ['date'], 'this is not a date', false];
171-
$tests[] = [new \stdClass(), ['date'], '/* Object(stdClass) */', false];
211+
$tests[] = ['this is not a date', ['date'], "⚠ Could not convert PHP value 'this is not a date' of type 'string' to type 'date'. Expected one of the following types: null, DateTime", false, false];
212+
$tests[] = [
213+
new \stdClass(),
214+
['date'],
215+
<<<EOTXT
216+
{#%d
217+
⚠: "Could not convert PHP value of type 'stdClass' to type 'date'. Expected one of the following types: null, DateTime"
218+
}
219+
EOTXT
220+
,
221+
false,
222+
false,
223+
];
172224
}
173225

174226
return $tests;

0 commit comments

Comments
 (0)
0