From 4535abecee468a3b0a128b037c7bd4e0ef84b995 Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Sun, 27 Nov 2011 13:32:13 +0100 Subject: [PATCH 1/5] [DoctrineBridge] Fixed attempt to serialize non-serializable values --- .../DataCollector/DoctrineDataCollector.php | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php b/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php index ba8c3266379bf..bb31d66e512eb 100644 --- a/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php +++ b/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php @@ -41,7 +41,7 @@ public function __construct(RegistryInterface $registry, DbalLogger $logger = nu public function collect(Request $request, Response $response, \Exception $exception = null) { $this->data = array( - 'queries' => null !== $this->logger ? $this->logger->queries : array(), + 'queries' => null !== $this->logger ? $this->sanitizeQueries($this->logger->queries) : array(), 'connections' => $this->connections, 'managers' => $this->managers, ); @@ -85,4 +85,40 @@ public function getName() return 'db'; } + private function sanitizeQueries($queries) + { + foreach ($queries as $i => $query) { + foreach ($query['params'] as $j => $param) { + $queries[$i]['params'][$j] = $this->sanitizeParameter($param); + } + } + + return $queries; + } + + private function sanitizeParameter($param) + { + if (is_array($param)) { + foreach ($param as $key => $value) { + $param[$key] = $this->sanitizeParameter($value); + } + + return $param; + } + + if (is_resource($param)) { + return sprintf('Resource(%s)', get_resource_type($param)); + } + + if (is_object($param) && $this->isNotSerializable($param)) { + return sprintf('Object(%s)', get_class($param)); + } + + return $param; + } + + private function isNotSerializable($object) + { + return $object instanceof \SplFileInfo; + } } From 28730e91492a3d175574ebf8aee5a5e7536d1a3e Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Sun, 27 Nov 2011 13:43:02 +0100 Subject: [PATCH 2/5] [DoctrineBridge] Added unit tests --- .../DoctrineDataCollectorTest.php | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php diff --git a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php new file mode 100644 index 0000000000000..9038786ef2f56 --- /dev/null +++ b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php @@ -0,0 +1,131 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Tests\Bridge\Doctrine\DataCollector; + +use Symfony\Bridge\Doctrine\DataCollector\DoctrineDataCollector; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; + +class DoctrineDataCollectorTest extends \PHPUnit_Framework_TestCase +{ + public function testCollectConnections() + { + $c = $this->createCollector(array()); + $c->collect(new Request(), new Response()); + $this->assertEquals(array('default' => 'doctrine.dbal.default_connection'), $c->getConnections()); + } + + public function testCollectManagers() + { + $c = $this->createCollector(array()); + $c->collect(new Request(), new Response()); + $this->assertEquals(array('default' => 'doctrine.orm.default_entity_manager'), $c->getManagers()); + } + + public function testCollectQueryCount() + { + $c = $this->createCollector(array()); + $c->collect(new Request(), new Response()); + $this->assertEquals(0, $c->getQueryCount()); + + $queries = array( + array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 0) + ); + $c = $this->createCollector($queries); + $c->collect(new Request(), new Response()); + $this->assertEquals(1, $c->getQueryCount()); + } + + public function testCollectTime() + { + $c = $this->createCollector(array()); + $c->collect(new Request(), new Response()); + $this->assertEquals(0, $c->getTime()); + + $queries = array( + array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 1) + ); + $c = $this->createCollector($queries); + $c->collect(new Request(), new Response()); + $this->assertEquals(1, $c->getTime()); + + $queries = array( + array('sql' => "SELECT * FROM table1", 'params' => array(), 'types' => array(), 'executionMS' => 1), + array('sql' => "SELECT * FROM table2", 'params' => array(), 'types' => array(), 'executionMS' => 2) + ); + $c = $this->createCollector($queries); + $c->collect(new Request(), new Response()); + $this->assertEquals(3, $c->getTime()); + } + + /** + * @dataProvider paramProvider + */ + public function testCollectQueries($param, $expected) + { + $queries = array( + array('sql' => "SELECT * FROM table1 WHERE field1 = ?1", 'params' => array($param), 'types' => array(), 'executionMS' => 1) + ); + $c = $this->createCollector($queries); + $c->collect(new Request(), new Response()); + + $collected_queries = $c->getQueries(); + $this->assertEquals($expected, $collected_queries[0]['params'][0]); + } + + /** + * @dataProvider paramProvider + */ + public function testSerialization($param, $expected) + { + $queries = array( + array('sql' => "SELECT * FROM table1 WHERE field1 = ?1", 'params' => array($param), 'types' => array(), 'executionMS' => 1) + ); + $c = $this->createCollector($queries); + $c->collect(new Request(), new Response()); + $c = unserialize(serialize($c)); + + $collected_queries = $c->getQueries(); + $this->assertEquals($expected, $collected_queries[0]['params'][0]); + } + + public function paramProvider() + { + return array( + array('some value', 'some value'), + array(1, 1), + array(true, true), + array(null, null), + array(new \stdClass(), new \stdClass()), + array(fopen(__FILE__, 'r'), 'Resource(stream)'), + array(new \SplFileInfo(__FILE__), 'Object(SplFileInfo)'), + ); + } + + private function createCollector($queries) + { + $registry = $this->getMock('Doctrine\Common\Persistence\ManagerRegistry'); + $registry + ->expects($this->once()) + ->method('getConnectionNames') + ->will($this->returnValue(array('default' => 'doctrine.dbal.default_connection'))); + $registry + ->expects($this->once()) + ->method('getManagerNames') + ->will($this->returnValue(array('default' => 'doctrine.orm.default_entity_manager'))); + + $logger = $this->getMock('Symfony\Bridge\Doctrine\Logger\DbalLogger'); + $logger->queries = $queries; + + return new DoctrineDataCollector($registry, $logger); + } +} From 4fe4dfd116f2ff7e749eec2890403047d19d5d1c Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Thu, 8 Dec 2011 18:09:06 +0100 Subject: [PATCH 3/5] Fixed vendor version mismatch in tests --- .../Doctrine/DataCollector/DoctrineDataCollectorTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php index 9038786ef2f56..d6416f420804b 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php @@ -113,14 +113,14 @@ public function paramProvider() private function createCollector($queries) { - $registry = $this->getMock('Doctrine\Common\Persistence\ManagerRegistry'); + $registry = $this->getMock('Symfony\Bridge\Doctrine\RegistryInterface'); $registry - ->expects($this->once()) + ->expects($this->any()) ->method('getConnectionNames') ->will($this->returnValue(array('default' => 'doctrine.dbal.default_connection'))); $registry - ->expects($this->once()) - ->method('getManagerNames') + ->expects($this->any()) + ->method('getEntityManagerNames') ->will($this->returnValue(array('default' => 'doctrine.orm.default_entity_manager'))); $logger = $this->getMock('Symfony\Bridge\Doctrine\Logger\DbalLogger'); From bb0d2022505fccd8ddd08f641fec13490fab4eef Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Thu, 8 Dec 2011 18:14:27 +0100 Subject: [PATCH 4/5] Switched sanitizeParameter() for existing varToString()-method; now always stores a string representation of each parameter --- .../DataCollector/DoctrineDataCollector.php | 38 +++++++++++-------- .../Resources/views/Collector/db.html.twig | 2 +- .../DoctrineDataCollectorTest.php | 8 ++-- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php b/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php index bb31d66e512eb..edc5861d6f6d1 100644 --- a/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php +++ b/src/Symfony/Bridge/Doctrine/DataCollector/DoctrineDataCollector.php @@ -89,36 +89,44 @@ private function sanitizeQueries($queries) { foreach ($queries as $i => $query) { foreach ($query['params'] as $j => $param) { - $queries[$i]['params'][$j] = $this->sanitizeParameter($param); + $queries[$i]['params'][$j] = $this->varToString($param); } } return $queries; } - private function sanitizeParameter($param) + private function varToString($var) { - if (is_array($param)) { - foreach ($param as $key => $value) { - $param[$key] = $this->sanitizeParameter($value); + if (is_object($var)) { + return sprintf('Object(%s)', get_class($var)); + } + + if (is_array($var)) { + $a = array(); + foreach ($var as $k => $v) { + $a[] = sprintf('%s => %s', $k, $this->varToString($v)); } - return $param; + return sprintf("Array(%s)", implode(', ', $a)); } - if (is_resource($param)) { - return sprintf('Resource(%s)', get_resource_type($param)); + if (is_resource($var)) { + return sprintf('Resource(%s)', get_resource_type($var)); } - if (is_object($param) && $this->isNotSerializable($param)) { - return sprintf('Object(%s)', get_class($param)); + if (null === $var) { + return 'null'; } - return $param; - } + if (false === $var) { + return 'false'; + } - private function isNotSerializable($object) - { - return $object instanceof \SplFileInfo; + if (true === $var) { + return 'true'; + } + + return (string) $var; } } diff --git a/src/Symfony/Bundle/DoctrineBundle/Resources/views/Collector/db.html.twig b/src/Symfony/Bundle/DoctrineBundle/Resources/views/Collector/db.html.twig index f963d7957c6fa..94b534730515c 100644 --- a/src/Symfony/Bundle/DoctrineBundle/Resources/views/Collector/db.html.twig +++ b/src/Symfony/Bundle/DoctrineBundle/Resources/views/Collector/db.html.twig @@ -36,7 +36,7 @@ {{ query.sql }} - Parameters: {{ query.params|yaml_encode }}
+ Parameters: {{ query.params }}
Time: {{ '%0.2f'|format(query.executionMS * 1000) }} ms diff --git a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php index d6416f420804b..c82248d6c4c63 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php @@ -102,10 +102,10 @@ public function paramProvider() { return array( array('some value', 'some value'), - array(1, 1), - array(true, true), - array(null, null), - array(new \stdClass(), new \stdClass()), + array(1, '1'), + array(true, 'true'), + array(null, 'null'), + array(new \stdClass(), 'Object(stdClass)'), array(fopen(__FILE__, 'r'), 'Resource(stream)'), array(new \SplFileInfo(__FILE__), 'Object(SplFileInfo)'), ); From 01dbb0fc8c67f1fc87e3bae17357ee8d78c5e7c1 Mon Sep 17 00:00:00 2001 From: Arnout Boks Date: Fri, 9 Dec 2011 10:23:40 +0100 Subject: [PATCH 5/5] Adapted tests for master branch --- .../Doctrine/DataCollector/DoctrineDataCollectorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php index c82248d6c4c63..d8a2a9d246254 100644 --- a/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php +++ b/tests/Symfony/Tests/Bridge/Doctrine/DataCollector/DoctrineDataCollectorTest.php @@ -113,14 +113,14 @@ public function paramProvider() private function createCollector($queries) { - $registry = $this->getMock('Symfony\Bridge\Doctrine\RegistryInterface'); + $registry = $this->getMock('Doctrine\Common\Persistence\ManagerRegistry'); $registry ->expects($this->any()) ->method('getConnectionNames') ->will($this->returnValue(array('default' => 'doctrine.dbal.default_connection'))); $registry ->expects($this->any()) - ->method('getEntityManagerNames') + ->method('getManagerNames') ->will($this->returnValue(array('default' => 'doctrine.orm.default_entity_manager'))); $logger = $this->getMock('Symfony\Bridge\Doctrine\Logger\DbalLogger');