8000 merged branch aboks/doctrine_data_collector (PR #2733) · symfony/symfony@c22652f · GitHub
[go: up one dir, main page]
Skip to content

Commit c22652f

Browse files
fabpotfabpot
committed
merged branch aboks/doctrine_data_collector (PR #2733)
Commits ------- bb0d202 Switched sanitizeParameter() for existing varToString()-method; now always stores a string representation of each parameter 4fe4dfd Fixed vendor version mismatch in tests 28730e9 [DoctrineBridge] Added unit tests 4535abe [DoctrineBridge] Fixed attempt to serialize non-serializable values Discussion ---------- [DoctrineBridge] Fixed attempt to serialize non-serializable values Bug fix: yes Feature addition: no Backwards compatibility break: no (99%) Symfony2 tests pass: yes Fixes the following tickets: - Todo: - The Doctrine DBAL type system does not pose any restrictions on the php-types of parameters in queries. Hence one could write a doctrine-type that uses a resource or an `\SplFileInfo` as its corresponding php-type. Parameters of these types are logged in the `DoctrineDataCollector` however, which is then serialized in the profiler. Since resources or `\SplFileInfo` variables cannot be serialized this throws an exception. This PR fixes this problem (for known cases) by sanitizing the query parameters to only contain serializable types. The `isNotSerializable`-check surely is not complete yet, but more non-serializable classes can be added on a case-by-case basis. --------------------------------------------------------------------------- by fabpot at 2011/12/07 07:04:43 -0800 Tests do not pass for me. Furthermore, let's reuse what we already have in the framework (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpKernel.php#L187 -- yes you can just copy/paster the existing code). --------------------------------------------------------------------------- by aboks at 2011/12/09 01:41:14 -0800 @fabpot I fixed the tests (seems I had the wrong vendor versions in my copy) and reused the `varToString()`-code. This introduces a tiny BC break in the rare case that someone writes his own templates for the web profiler (the parameters returned by the data collector are now always a string; could be any type before). After merging this PR, merging 2.0 into master would give a merge conflict and failing tests (because of the changes related to the introduction of the `ManagerRegistry` interface). To prevent this, please merge #2820 into master directly after merging this PR (so before merging 2.0 into master). After that 2.0 can be cleanly merged into master. --------------------------------------------------------------------------- by stof at 2011/12/09 03:43:38 -0800 it is not a BC break. Using ``yaml_encode`` on a string will not break the template
2 parents c6db266 + bb0d202 commit c22652f

File tree

3 files changed

+177
-2
lines changed

3 files changed

+177
-2
lines changed

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

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function __construct(RegistryInterface $registry, DbalLogger $logger = nu
4141
public function collect(Request $request, Response $response, \Exception $exception = null)
4242
{
4343
$this->data = array(
44-
'queries' => null !== $this->logger ? $this->logger->queries : array(),
44+
'queries' => null !== $this->logger ? $this->sanitizeQueries($this->logger->queries) : array(),
4545
'connections' => $this->connections,
4646
'managers' => $this->managers,
4747
);
@@ -85,4 +85,48 @@ public function getName()
8585
return 'db';
8686
}
8787

88+
private function sanitizeQueries($queries)
89+
{
90+
foreach ($queries as $i => $query) {
91+
foreach ($query['params'] as $j => $param) {
92+
$queries[$i]['params'][$j] = $this->varToString($param);
93+
}
94+
}
95+
96+
return $queries;
97+
}
98+
99+
private function varToString($var)
100+
{
101+
if (is_object($var)) {
102+
return sprintf('Object(%s)', get_class($var));
103+
}
104+
105+
if (is_array($var)) {
106+
$a = array();
107+
foreach ($var as $k => $v) {
108+
$a[] = sprintf('%s => %s', $k, $this->varToString($v));
109+
}
110+
111+
return sprintf("Array(%s)", implode(', ', $a));
112+
}
113+
114+
if (is_resource($var)) {
115+
return sprintf('Resource(%s)', get_resource_type($var));
116+
}
117+
118+
if (null === $var) {
119+
return 'null';
120+
}
121+
122+
if (false === $var) {
123+
return 'false';
124+
}
125+
126+
if (true === $var) {
127+
return 'true';
128+
}
129+
130+
return (string) $var;
131+
}
88132
}

‎src/Symfony/Bundle/DoctrineBundle/Resources/views/Collector/db.html.twig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
<code>{{ query.sql }}</code>
3737
</div>
3838
<small>
39-
<strong>Parameters</strong>: {{ query.params|yaml_encode }}<br />
39+
<strong>Parameters</strong>: {{ query.params }}<br />
4040
<strong>Time</strong>: {{ '%0.2f'|format(query.executionMS * 1000) }} ms
4141
</small>
4242
</li>

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
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\Tests\Bridge\Doctrine\DataCollector;
13+
14+
use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\HttpFoundation\Response;
17+
18+
class DoctrineDataCollectorTest extends \PHPUnit_Framework_TestCase
19+
{
20+
public function testCollectConnections()
21+
{
22+
$c = $this->createCollector(array());
23+
$c->collect(new Request(), new Response());
24+
$this->assertEquals(array('default' => 'doctrine.dbal.default_connection'), $c->getConnections());
25+
}
26+
27+
public function testCollectManagers()
28+
{
29+
$c = $this->createCollector(array());
30+
$c->collect(new Request(), new Response());
31+
$this->assertEquals(array('default' => 'doctrine.orm.default_entity_manager'), $c->getManagers());
32+
}
33+
34+
public function testCollectQueryCount()
35+
{
36+
$c = $this->createCollector(array());
37+
$c->collect(new Request(), new Response());
38+
$this->assertEquals(0, $c->getQueryCount());
39+
40+
$queries = array(
41+
array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 0)
42+
);
43+
$c = $this->createCollector($queries);
44+
$c->collect(new Request(), new Response());
45+
$this->assertEquals(1, $c->getQueryCount());
46+
}
47+
48+
public function testCollectTime()
49+
{
50+
$c = $this->createCollector(array());
51+
$c->collect(new Request(), new Response());
52+
$this->assertEquals(0, $c->getTime());
53+
54+
$queries = array(
55+
array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 1)
56+
);
57+
$c = $this->createCollector($queries);
58+
$c->collect(new Request(), new Response());
59+
$this->assertEquals(1, $c->getTime());
60+
61+
$queries = array(
62+
array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 1),
63+
array('sql' => "SELECT * FROM table2", 'params' => array(), 'types' => array(), 'executionMS' => 2)
64+
);
65+
$c = $this->createCollector($queries);
66+
$c->collect(new Request(), new Response());
67+
$this->assertEquals(3, $c->getTime());
68+
}
69+
70+
/**
71+
* @dataProvider paramProvider
72+
*/
73+
public function testCollectQueries($param, $expected)
74+
{
75+
$queries = array(
76+
array('sql' => "SELECT * FROM table1 WHERE field1 = ?1", 'params' => array($param), 'types' => array(), 'executionMS' => 1)
77+
);
78+
$c = $this->createCollector($queries);
79+
$c->collect(new Request(), new Response());
80+
81+
$collected_queries = $c->getQueries();
82+
$this->assertEquals($expected, $collected_queries[0]['params'][0]);
83+
}
84+
85+
/**
86+
* @dataProvider paramProvider
87+
*/
88+
public function testSerialization($param, $expected)
89+
{
90+
$queries = array(
91+
array('sql' => "SELECT * FROM table1 WHERE field1 = ?1", 'params' => array($param), 'types' => array(), 'executionMS' => 1)
92+
);
93+
$c = $this->createCollector($queries);
94+
$c->collect(new Request(), new Response());
95+
$c = unserialize(serialize($c));
96+
97+
$collected_queries = $c->getQueries();
98+
$this->assertEquals($expected, $collected_queries[0]['params'][0]);
99+
}
100+
101+
public function paramProvider()
102+
{
103+
return array(
104+
array('some value', 'some value'),
105+
array(1, '1'),
106+
array(true, 'true'),
107+
array(null, 'null'),
108+
array(new \stdClass(), 'Object(stdClass)'),
109+
array(fopen(__FILE__, 'r'), 'Resource(stream)'),
110+
array(new \SplFileInfo(__FILE__), 'Object(SplFileInfo)'),
111+
);
112+
}
113+
114+
private function createCollector($queries)
115+
{
116+
$registry = $this->getMock('Symfony\Bridge\Doctrine\RegistryInterface');
117+
$registry
118+
->expects($this->any())
119+
->method('getConnectionNames')
120+
->will($this->returnValue(array('default' => 'doctrine.dbal.default_connection')));
121+
$registry
122+
->expects($this->any())
123+
->method('getEntityManagerNames')
124+
->will($this->returnValue(array('default' => 'doctrine.orm.default_entity_manager')));
125+
126+
$logger = $this->getMock('Symfony\Bridge\Doctrine\Logger\DbalLogger');
127+
$logger->queries = $queries;
128+
129+
return new DoctrineDataCollector($registry, $logger);
130+
}
131+
}

0 commit comments

Comments
 (0)
0