merged branch vicb/doctrine_logger (PR #5139)

Commits
-------

a0709fc [DoctrineBridge] Fix log of non utf8 data

Discussion
----------

Doctrine logger - fix logging of binary data

fix #5115

---------------------------------------------------------------------------

by travisbot at 2012-08-01T11:21:07Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2008862) (merged a0709fc3 into 1da896dc).

---------------------------------------------------------------------------

by stof at 2012-08-01T11:54:45Z

I see a way to fix it in a far better way: instead of json_encoding the parameters and appending them to the SQL, we could pass them as context to the logger (the optional second argument) as Monolog already handles normalizing the context (and in a better way silencing the error).
Btw, this would also make the log message better for rich logger as they would receive the array (for instance, the FirephpHandler is able to send the context as an array instead of a string as firebug is able to dump arrays)

---------------------------------------------------------------------------

by vicb at 2012-08-01T12:19:06Z

@stof you're right that would be much better. What about keeping this fix for 2.0 and use your suggestion for 2.1 (as the public `log` method prototype would change (to take the context as argument) ?

---------------------------------------------------------------------------

by stof at 2012-08-01T12:21:09Z

@vicb as the method is public, it need to be done in 2.1 only indeed. The next question being "why is it public ?" :)

---------------------------------------------------------------------------

by fabpot at 2012-08-03T07:47:39Z

So, what's the next step?

---------------------------------------------------------------------------

by vicb at 2012-08-03T07:48:38Z

I think this should be merged in 2.0 and then 2.1 should be updated with the suggestion from @stof

---------------------------------------------------------------------------

by fabpot at 2012-08-03T07:54:16Z

Can you provide a PR for 2.1 so that I merge both at the same time?

src/Symfony/Bridge/Doctrine/Logger/DbalLogger.php

@@ -41,7 +41,7 @@ class DbalLogger extends DebugStack
         parent::startQuery($sql, $params, $types);
 
         if (null !== $this->logger) {
-            $this->log($sql.' ('.json_encode($params).')');
+            $this->log($sql.' ('.@json_encode($params).')');
         }
     }
 

tests/Symfony/Tests/Bridge/Doctrine/Logger/DbalLoggerTest.php

@@ -0,0 +1,39 @@
+<?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\Tests\Bridge\Doctrine\Logger;
+
+
+class DbalLoggerTest extends \PHPUnit_Framework_TestCase
+{
+    public function testLogNonUtf8()
+    {
+        $logger = $this->getMock('Symfony\\Component\\HttpKernel\\Log\\LoggerInterface');
+
+        $dbalLogger = $this
+            ->getMockBuilder('Symfony\\Bridge\\Doctrine\\Logger\\DbalLogger')
+            ->setConstructorArgs(array($logger, null))
+            ->setMethods(array('log'))
+            ->getMock()
+        ;
+
+        $dbalLogger
+            ->expects($this->once())
+            ->method('log')
+            ->with('SQL ({"utf8":"foo","nonutf8":null})')
+        ;
+
+        $dbalLogger->startQuery('SQL', array(
+            'utf8'    => 'foo',
+            'nonutf8' => "\x7F\xFF"
+        ));
+    }
+}