Skip to content

[DoctrineBridge] Improve queries parameters display in Profiler #34384

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 17, 2019
Merged

[DoctrineBridge] Improve queries parameters display in Profiler #34384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 53 additions & 15 deletions src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
use Symfony\Component\VarDumper\Caster\Caster;
use Symfony\Component\VarDumper\Cloner\Stub;

/**
* DoctrineDataCollector.
Expand Down Expand Up @@ -121,6 +123,38 @@ public function getName()
return 'db';
}

/**
* {@inheritdoc}
*/
protected function getCasters()
{
return parent::getCasters() + [
ObjectParameter::class => static function (ObjectParameter $o, array $a, Stub $s): array {
$s->class = $o->getClass();
$s->value = $o->getObject();

$r = new \ReflectionClass($o->getClass());
if ($f = $r->getFileName()) {
$s->attr['file'] = $f;
$s->attr['line'] = $r->getStartLine();
} else {
unset($s->attr['file']);
unset($s->attr['line']);
}

if ($error = $o->getError()) {
return [Caster::PREFIX_VIRTUAL.'⚠' => $error->getMessage()];
}

if ($o->isStringable()) {
return [Caster::PREFIX_VIRTUAL.'__toString()' => (string) $o->getObject()];
}

return [Caster::PREFIX_VIRTUAL.'⚠' => sprintf('Object of class "%s" could not be converted to string.', $o->getClass())];
},
];
}

private function sanitizeQueries(string $connectionName, array $queries): array
{
foreach ($queries as $i => $query) {
Expand All @@ -133,6 +167,7 @@ private function sanitizeQueries(string $connectionName, array $queries): array
private function sanitizeQuery(string $connectionName, array $query): array
{
$query['explainable'] = true;
$query['runnable'] = true;
if (null === $query['params']) {
$query['params'] = [];
}
Expand All @@ -143,6 +178,7 @@ private function sanitizeQuery(string $connectionName, array $query): array
$query['types'] = [];
}
foreach ($query['params'] as $j => $param) {
$e = null;
if (isset($query['types'][$j])) {
// Transform the param according to the type
$type = $query['types'][$j];
Expand All @@ -154,18 +190,19 @@ private function sanitizeQuery(string $connectionName, array $query): array
try {
$param = $type->convertToDatabaseValue($param, $this->registry->getConnection($connectionName)->getDatabasePlatform());
} catch (\TypeError $e) {
// Error thrown while processing params, query is not explainable.
$query['explainable'] = false;
} catch (ConversionException $e) {
$query['explainable'] = false;
}
}
}

list($query['params'][$j], $explainable) = $this->sanitizeParam($param);
list($query['params'][$j], $explainable, $runnable) = $this->sanitizeParam($param, $e);
if (!$explainable) {
$query['explainable'] = false;
}

if (!$runnable) {
$query['runnable'] = false;
}
}

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

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

if (\is_array($var)) {
$a = [];
$original = true;
$explainable = $runnable = true;
foreach ($var as $k => $v) {
list($value, $orig) = $this->sanitizeParam($v);
$original = $original && $orig;
list($value, $e, $r) = $this->sanitizeParam($v, null);
$explainable = $explainable && $e;
$runnable = $runnable && $r;
$a[$k] = $value;
}

return [$a, $original];
return [$a, $explainable, $runnable];
}

if (\is_resource($var)) {
return [sprintf('/* Resource(%s) */', get_resource_type($var)), false];
return [sprintf('/* Resource(%s) */', get_resource_type($var)), false, false];
}

return [$var, true];
return [$var, true, true];
}
}
54 changes: 54 additions & 0 deletions src/Symfony/Bridge/Doctrine/DataCollector/ObjectParameter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\DataCollector;

final class ObjectParameter
{
private $object;
private $error;
private $stringable;
private $class;

/**
* @param object $object
*/
public function __construct($object, ?\Throwable $error)
{
$this->object = $object;
$this->error = $error;
$this->stringable = \is_callable([$object, '__toString']);
$this->class = \get_class($object);
}

/**
* @return object
*/
public function getObject()
{
return $this->object;
}

public function getError(): ?\Throwable
{
return $this->error;
}

public function isStringable(): bool
{
return $this->stringable;
}

public function getClass(): string
{
return $this->class;
}
}
70 changes: 61 additions & 9 deletions src/Symfony/Bridge/Doctrine/Tests/DataCollector/DoctrineDataCollectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\VarDumper\Cloner\Data;
use Symfony\Component\VarDumper\Dumper\CliDumper;

class DoctrineDataCollectorTest extends TestCase
{
Expand Down Expand Up @@ -74,7 +75,7 @@ public function testCollectTime()
/**
* @dataProvider paramProvider
*/
public function testCollectQueries($param, $types, $expected, $explainable)
public function testCollectQueries($param, $types, $expected, $explainable, bool $runnable = true)
{
$queries = [
['sql' => 'SELECT * FROM table1 WHERE field1 = ?1', 'params' => [$param], 'types' => $types, 'executionMS' => 1],
Expand All @@ -83,8 +84,19 @@ public function testCollectQueries($param, $types, $expected, $explainable)
$c->collect(new Request(), new Response());

$collectedQueries = $c->getQueries();
$this->assertEquals($expected, $collectedQueries['default'][0]['params'][0]);

$collectedParam = $collectedQueries['default'][0]['params'][0];
if ($collectedParam instanceof Data) {
$dumper = new CliDumper($out = fopen('php://memory', 'r+b'));
$dumper->setColors(false);
$collectedParam->dump($dumper);
$this->assertStringMatchesFormat($expected, print_r(stream_get_contents($out, -1, 0), true));
} else {
$this->assertEquals($expected, $collectedParam);
}

$this->assertEquals($explainable, $collectedQueries['default'][0]['explainable']);
$this->assertSame($runnable, $collectedQueries['default'][0]['runnable']);
}

public function testCollectQueryWithNoParams()
Expand All @@ -100,9 +112,11 @@ public function testCollectQueryWithNoParams()
$this->assertInstanceOf(Data::class, $collectedQueries['default'][0]['params']);
$this->assertEquals([], $collectedQueries['default'][0]['params']->getValue());
$this->assertTrue($collectedQueries['default'][0]['explainable']);
$this->assertTrue($collectedQueries['default'][0]['runnable']);
$this->assertInstanceOf(Data::class, $collectedQueries['default'][1]['params']);
$this->assertEquals([], $collectedQueries['default'][1]['params']->getValue());
$this->assertTrue($collectedQueries['default'][1]['explainable']);
$this->assertTrue($collectedQueries['default'][1]['runnable']);
}

public function testCollectQueryWithNoTypes()
Expand Down Expand Up @@ -134,7 +148,7 @@ public function testReset()
/**
* @dataProvider paramProvider
*/
public function testSerialization($param, $types, $expected, $explainable)
public function testSerialization($param, $types, $expected, $explainable, bool $runnable = true)
{
$queries = [
['sql' => 'SELECT * FROM table1 WHERE field1 = ?1', 'params' => [$param], 'types' => $types, 'executionMS' => 1],
Expand All @@ -144,8 +158,19 @@ public function testSerialization($param, $types, $expected, $explainable)
$c = unserialize(serialize($c));

$collectedQueries = $c->getQueries();
$this->assertEquals($expected, $collectedQueries['default'][0]['params'][0]);

$collectedParam = $collectedQueries['default'][0]['params'][0];
if ($collectedParam instanceof Data) {
$dumper = new CliDumper($out = fopen('php://memory', 'r+b'));
$dumper->setColors(false);
$collectedParam->dump($dumper);
$this->assertStringMatchesFormat($expected, print_r(stream_get_contents($out, -1, 0), true));
} else {
$this->assertEquals($expected, $collectedParam);
}

$this->assertEquals($explainable, $collectedQueries['default'][0]['explainable']);
$this->assertSame($runnable, $collectedQueries['default'][0]['runnable']);
}

public function paramProvider()
Expand All @@ -156,19 +181,46 @@ public function paramProvider()
[true, [], true, true],
[null, [], null, true],
[new \DateTime('2011-09-11'), ['date'], '2011-09-11', true],
[fopen(__FILE__, 'r'), [], '/* Resource(stream) */', false],
[new \stdClass(), [], '/* Object(stdClass) */', false],
[fopen(__FILE__, 'r'), [], '/* Resource(stream) */', false, false],
[
new \stdClass(),
[],
<<<EOTXT
{#%d
⚠: "Object of class "stdClass" could not be converted to string."
}
EOTXT
,
false,
false,
],
[
new StringRepresentableClass(),
[],
'/* Object(Symfony\Bridge\Doctrine\Tests\DataCollector\StringRepresentableClass): */"string representation"',
<<<EOTXT
Symfony\Bridge\Doctrine\Tests\DataCollector\StringRepresentableClass {#%d
__toString(): "string representation"
}
EOTXT
,
false,
],
];

if (version_compare(Version::VERSION, '2.6', '>=')) {
$tests[] = ['this is not a date', ['date'], 'this is not a date', false];
$tests[] = [new \stdClass(), ['date'], '/* Object(stdClass) */', false];
$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];
$tests[] = [
new \stdClass(),
['date'],
<<<EOTXT
{#%d
⚠: "Could not convert PHP value of type 'stdClass' to type 'date'. Expected one of the following types: null, DateTime"
}
EOTXT
,
false,
false,
];
}

return $tests;
Expand Down