merged branch Tobion/deprecate-debuggerinterface (PR #8929)

This PR was merged into the master branch.

Discussion
----------

[Templating] deprecate DebuggerInterface

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes: deprecated DebuggerInterface and Debugger
| Tests pass?   | yes
| Fixed tickets | #8926
| License       | MIT

- [x] add suggested optional dependency in templating component to Psr log
- [x] remove ProjectTemplateDebugger from tests

Commits
-------

b853438 [Templating] deprecate DebuggerInterface

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -12,6 +12,7 @@
 namespace Symfony\Bundle\FrameworkBundle\DependencyInjection;
 
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
@@ -372,6 +373,15 @@ class FrameworkExtension extends Extension
         if ($container->getParameter('kernel.debug')) {
             $loader->load('templating_debug.xml');
 
+            $logger = new Reference('logger', ContainerInterface::IGNORE_ON_INVALID_REFERENCE);
+
+            $container->getDefinition('templating.loader.cache')
+                ->addTag('monolog.logger', array('channel' => 'templating'))
+                ->addMethodCall('setLogger', array($logger));
+            $container->getDefinition('templating.loader.chain')
+                ->addTag('monolog.logger', array('channel' => 'templating'))
+                ->addMethodCall('setLogger', array($logger));
+
             $container->setDefinition('templating.engine.php', $container->findDefinition('debug.templating.engine.php'));
             $container->setAlias('debug.templating.engine.php', 'templating.engine.php');
         }

src/Symfony/Bundle/FrameworkBundle/Resources/config/templating.xml

@@ -52,11 +52,9 @@
         <service id="templating.loader.cache" class="%templating.loader.cache.class%" public="false">
             <argument type="service" id="templating.loader.wrapped" />
             <argument>%templating.loader.cache.path%</argument>
-            <call method="setDebugger"><argument type="service" id="templating.debugger" on-invalid="ignore" /></call>
         </service>
 
         <service id="templating.loader.chain" class="%templating.loader.chain.class%" public="false">
-            <call method="setDebugger"><argument type="service" id="templating.debugger" on-invalid="ignore" /></call>
         </service>
 
         <service id="templating.loader" alias="templating.loader.filesystem" />

src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_debug.xml

@@ -5,15 +5,10 @@
     xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <parameters>
-        <parameter key="templating.debugger.class">Symfony\Bundle\FrameworkBundle\Templating\Debugger</parameter>
         <parameter key="debug.templating.engine.php.class">Symfony\Bundle\FrameworkBundle\Templating\TimedPhpEngine</parameter>
     </parameters>
 
     <services>
-        <service id="templating.debugger" class="%templating.debugger.class%" public="false">
-            <tag name="monolog.logger" channel="templating" />
-            <argument type="service" id="logger" on-invalid="null" />
-        </service>
         <service id="debug.templating.engine.php" class="%debug.templating.engine.php.class%" public="false">
             <argument type="service" id="templating.name_parser" />
             <argument type="service" id="service_container" />

src/Symfony/Bundle/FrameworkBundle/Templating/Debugger.php

@@ -18,6 +18,8 @@ use Psr\Log\LoggerInterface;
  * Binds the Symfony templating loader debugger to the Symfony logger.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated Deprecated in 2.4, to be removed in 3.0. Use Psr\Log\LoggerInterface instead.
  */
 class Debugger implements DebuggerInterface
 {

src/Symfony/Component/Templating/DebuggerInterface.php

@@ -16,6 +16,8 @@ namespace Symfony\Component\Templating;
  * to debug template loader instances.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated Deprecated in 2.4, to be removed in 3.0. Use Psr\Log\LoggerInterface instead.
  */
 interface DebuggerInterface
 {

src/Symfony/Component/Templating/Loader/CacheLoader.php

@@ -56,7 +56,10 @@ class CacheLoader extends Loader
         $path = $dir.DIRECTORY_SEPARATOR.$file;
 
         if (is_file($path)) {
-            if (null !== $this->debugger) {
+            if (null !== $this->logger) {
+                $this->logger->debug(sprintf('Fetching template "%s" from cache', $template->get('name')));
+            } elseif (null !== $this->debugger) {
+                // just for BC, to be removed in 3.0
                 $this->debugger->log(sprintf('Fetching template "%s" from cache', $template->get('name')));
             }
 
@@ -75,7 +78,10 @@ class CacheLoader extends Loader
 
         file_put_contents($path, $content);
 
-        if (null !== $this->debugger) {
+        if (null !== $this->logger) {
+            $this->logger->debug(sprintf('Storing template "%s" in cache', $template->get('name')));
+        } elseif (null !== $this->debugger) {
+            // just for BC, to be removed in 3.0
             $this->debugger->log(sprintf('Storing template "%s" in cache', $template->get('name')));
         }
 

src/Symfony/Component/Templating/Loader/FilesystemLoader.php

@@ -63,20 +63,25 @@ class FilesystemLoader extends Loader
         $logs = array();
         foreach ($this->templatePathPatterns as $templatePathPattern) {
             if (is_file($file = strtr($templatePathPattern, $replacements)) && is_readable($file)) {
-                if (null !== $this->debugger) {
+                if (null !== $this->logger) {
+                    $this->logger->debug(sprintf('Loaded template file "%s"', $file));
+                } elseif (null !== $this->debugger) {
+                    // just for BC, to be removed in 3.0
                     $this->debugger->log(sprintf('Loaded template file "%s"', $file));
                 }
 
                 return new FileStorage($file);
             }
 
-            if (null !== $this->debugger) {
+            if (null !== $this->logger || null !== $this->debugger) {
                 $logs[] = sprintf('Failed loading template file "%s"', $file);
             }
         }
 
-        if (null !== $this->debugger) {
-            foreach ($logs as $log) {
+        foreach ($logs as $log) {
+            if (null !== $this->logger) {
+                $this->logger->debug($log);
+            } elseif (null !== $this->debugger) {
                 $this->debugger->log($log);
             }
         }

src/Symfony/Component/Templating/Loader/Loader.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Templating\Loader;
 
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Templating\DebuggerInterface;
 
 /**
@@ -20,12 +21,32 @@ use Symfony\Component\Templating\DebuggerInterface;
  */
 abstract class Loader implements LoaderInterface
 {
+    /**
+     * @var LoggerInterface|null
+     */
+    protected $logger;
+
+    /**
+     * @deprecated Deprecated in 2.4, to be removed in 3.0. Use $this->logger instead.
+     */
     protected $debugger;
 
+    /**
+     * Sets the debug logger to use for this loader.
+     *
+     * @param LoggerInterface $logger A logger instance
+     */
+    public function setLogger(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
     /**
      * Sets the debugger to use for this loader.
      *
      * @param DebuggerInterface $debugger A debugger instance
+     *
+     * @deprecated Deprecated in 2.4, to be removed in 3.0. Use $this->setLogger() instead.
      */
     public function setDebugger(DebuggerInterface $debugger)
     {

src/Symfony/Component/Templating/Tests/Fixtures/ProjectTemplateDebugger.php

@@ -1,40 +0,0 @@
-<?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\Component\Templating\Tests\Fixtures;
-
-use Symfony\Component\Templating\DebuggerInterface;
-
-class ProjectTemplateDebugger implements DebuggerInterface
-{
-    protected $messages = array();
-
-    public function log($message)
-    {
-        $this->messages[] = $message;
-    }
-
-    public function hasMessage($regex)
-    {
-        foreach ($this->messages as $message) {
-            if (preg_match('#'.preg_quote($regex, '#').'#', $message)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    public function getMessages()
-    {
-        return $this->messages;
-    }
-}

src/Symfony/Component/Templating/Tests/Loader/CacheLoaderTest.php

@@ -31,13 +31,19 @@ class CacheLoaderTest extends \PHPUnit_Framework_TestCase
     {
         $dir = sys_get_temp_dir().DIRECTORY_SEPARATOR.rand(111111, 999999);
         mkdir($dir, 0777, true);
+
         $loader = new ProjectTemplateLoader($varLoader = new ProjectTemplateLoaderVar(new TemplateNameParser()), $dir);
-        $loader->setDebugger($debugger = new \Symfony\Component\Templating\Tests\Fixtures\ProjectTemplateDebugger());
         $this->assertFalse($loader->load(new TemplateReference('foo', 'php')), '->load() returns false if the embed loader is not able to load the template');
+
+        $logger = $this->getMock('Psr\Log\LoggerInterface');
+        $logger->expects($this->once())->method('debug')->with('Storing template "index" in cache');
+        $loader->setLogger($logger);
         $loader->load(new TemplateReference('index'));
-        $this->assertTrue($debugger->hasMessage('Storing template'), '->load() logs a "Storing template" message if the template is found');
+
+        $logger = $this->getMock('Psr\Log\LoggerInterface');
+        $logger->expects($this->once())->method('debug')->with('Fetching template "index" from cache');
+        $loader->setLogger($logger);
         $loader->load(new TemplateReference('index'));
-        $this->assertTrue($debugger->hasMessage('Fetching template'), '->load() logs a "Storing template" message if the template is fetched from cache');
     }
 }
 

src/Symfony/Component/Templating/Tests/Loader/FilesystemLoaderTest.php

@@ -59,15 +59,16 @@ class FilesystemLoaderTest extends \PHPUnit_Framework_TestCase
         $this->assertInstanceOf('Symfony\Component\Templating\Storage\FileStorage', $storage, '->load() returns a FileStorage if you pass a relative template that exists');
         $this->assertEquals($path.'/foo.php', (string) $storage, '->load() returns a FileStorage pointing to the absolute path of the template');
 
+        $logger = $this->getMock('Psr\Log\LoggerInterface');
+        $logger->expects($this->exactly(2))->method('debug');
+
         $loader = new ProjectTemplateLoader2($pathPattern);
-        $loader->setDebugger($debugger = new \Symfony\Component\Templating\Tests\Fixtures\ProjectTemplateDebugger());
+        $loader->setLogger($logger);
         $this->assertFalse($loader->load(new TemplateReference('foo.xml', 'php')), '->load() returns false if the template does not exist for the given engine');
-        $this->assertTrue($debugger->hasMessage('Failed loading template'), '->load() logs a "Failed loading template" message if the template is not found');
 
         $loader = new ProjectTemplateLoader2(array(self::$fixturesPath.'/null/%name%', $pathPattern));
-        $loader->setDebugger($debugger = new \Symfony\Component\Templating\Tests\Fixtures\ProjectTemplateDebugger());
+        $loader->setLogger($logger);
         $loader->load(new TemplateReference('foo.php', 'php'));
-        $this->assertTrue($debugger->hasMessage('Loaded template file'), '->load() logs a "Loaded template file" message if the template is found');
     }
 }
 

src/Symfony/Component/Templating/Tests/Loader/LoaderTest.php

@@ -17,11 +17,20 @@ use Symfony\Component\Templating\TemplateReferenceInterface;
 
 class LoaderTest extends \PHPUnit_Framework_TestCase
 {
+    public function testGetSetLogger()
+    {
+        $loader = new ProjectTemplateLoader4(new TemplateNameParser());
+        $logger = $this->getMock('Psr\Log\LoggerInterface');
+        $loader->setLogger($logger);
+        $this->assertSame($logger, $loader->getLogger(), '->setLogger() sets the logger instance');
+    }
+
     public function testGetSetDebugger()
     {
         $loader = new ProjectTemplateLoader4(new TemplateNameParser());
-        $loader->setDebugger($debugger = new \Symfony\Component\Templating\Tests\Fixtures\ProjectTemplateDebugger());
-        $this->assertTrue($loader->getDebugger() === $debugger, '->setDebugger() sets the debugger instance');
+        $debugger = $this->getMock('Symfony\Component\Templating\DebuggerInterface');
+        $loader->setDebugger($debugger);
+        $this->assertSame($debugger, $loader->getDebugger(), '->setDebugger() sets the debugger instance');
     }
 }
 
@@ -31,6 +40,11 @@ class ProjectTemplateLoader4 extends Loader
     {
     }
 
+    public function getLogger()
+    {
+        return $this->logger;
+    }
+
     public function getDebugger()
     {
         return $this->debugger;

src/Symfony/Component/Templating/composer.json

@@ -18,6 +18,12 @@
     "require": {
         "php": ">=5.3.3"
     },
+    "require-dev": {
+        "psr/log": "~1.0"
+    },
+    "suggest": {
+        "psr/log": "For using debug logging in loaders"
+    },
     "autoload": {
         "psr-0": { "Symfony\\Component\\Templating\\": "" }
     },