bug #390 Fix call to handler::pushProcessor (jderusse)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Fix call to handler::pushProcessor

This PR fixes call to undefined method `pushProcessor` on handler that does not implements the `ProcessableHandlerInterface`.

Commits
-------

ab41eea Fix call to pushProcessor on handlers V2

DependencyInjection/Compiler/AddProcessorsPass.php

@@ -11,8 +11,9 @@
 
 namespace Symfony\Bundle\MonologBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
 /**
@@ -36,6 +37,14 @@ public function process(ContainerBuilder $container)
 
                 if (!empty($tag['handler'])) {
                     $definition = $container->findDefinition(sprintf('monolog.handler.%s', $tag['handler']));
+                    $parentDef = $definition;
+                    while (!$parentDef->getClass() && $parentDef instanceof ChildDefinition) {
+                        $parentDef = $container->findDefinition($parentDef->getParent());
+                    }
+                    $class = $container->getParameterBag()->resolveValue($parentDef->getClass());
+                    if (!method_exists($class, 'pushProcessor')) {
+                        throw new \InvalidArgumentException(sprintf('The "%s" handler does not accept processors', $tag['handler']));
+                    }
                 } elseif (!empty($tag['channel'])) {
                     if ('app' === $tag['channel']) {
                         $definition = $container->getDefinition('monolog.logger');

DependencyInjection/MonologExtension.php

@@ -23,6 +23,7 @@
 use Symfony\Bundle\FullStack;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
+use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
@@ -191,14 +192,16 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
         }
 
         if ($handler['process_psr_3_messages']) {
-            $processorId = 'monolog.processor.psr_log_message';
-            if (!$container->hasDefinition($processorId)) {
-                $processor = new Definition('Monolog\\Processor\\PsrLogMessageProcessor');
-                $processor->setPublic(false);
-                $container->setDefinition($processorId, $processor);
-            }
+            if (method_exists($handlerClass, 'pushProcessor')) {
+                $processorId = 'monolog.processor.psr_log_message';
+                if (!$container->hasDefinition($processorId)) {
+                    $processor = new Definition('Monolog\\Processor\\PsrLogMessageProcessor');
+                    $processor->setPublic(false);
+                    $container->setDefinition($processorId, $processor);
+                }
 
-            $definition->addMethodCall('pushProcessor', [new Reference($processorId)]);
+                $definition->addMethodCall('pushProcessor', [new Reference($processorId)]);
+            }
         }
 
         switch ($handler['type']) {
@@ -892,6 +895,7 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
         case 'browser_console':
         case 'test':
         case 'null':
+        case 'noop':
         case 'debug':
             $definition->setArguments([
                 $handler['level'],

Tests/DependencyInjection/Compiler/AddProcessorsPassTest.php

@@ -11,13 +11,17 @@
 
 namespace Symfony\Bundle\MonologBundle\Tests\DependencyInjection\Compiler;
 
+use Monolog\Handler\NullHandler;
+use Monolog\Logger;
 use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\Monolog\Handler\ConsoleHandler;
 use Symfony\Bundle\MonologBundle\DependencyInjection\Compiler\AddProcessorsPass;
-use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\DependencyInjection\Definition;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Loader\FileLoader;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+use Symfony\Component\DependencyInjection\Reference;
 
 class AddProcessorsPassTest extends TestCase
 {
@@ -36,15 +40,42 @@ public function testHandlerProcessors()
         $this->assertEquals(['pushProcessor', [new Reference('test2')]], $calls[0]);
     }
 
+    public function testFailureOnHandlerWithoutPushProcessor()
+    {
+        $container = new ContainerBuilder();
+        $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../../../Resources/config'));
+        $loader->load('monolog.xml');
+
+        $service = new Definition(NullHandler::class);
+        $service->addTag('monolog.processor', ['handler' => 'test3']);
+        $container->setDefinition('monolog.handler.test3', $service);
+
+        $container->getCompilerPassConfig()->setOptimizationPasses([]);
+        $container->getCompilerPassConfig()->setRemovingPasses([]);
+        $container->addCompilerPass(new AddProcessorsPass());
+
+        if (Logger::API < 2) {
+            $container->compile();
+            $service = $container->getDefinition('monolog.handler.test3');
+            $calls = $service->getMethodCalls();
+            $this->assertCount(1, $calls);
+        } else {
+            $this->expectException(\InvalidArgumentException::class);
+            $this->expectExceptionMessage('The "test3" handler does not accept processors');
+            $container->compile();
+        }
+    }
+
     protected function getContainer()
     {
         $container = new ContainerBuilder();
         $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../../../Resources/config'));
         $loader->load('monolog.xml');
 
         $definition = $container->getDefinition('monolog.logger_prototype');
-        $container->setDefinition('monolog.handler.test', new Definition('%monolog.handler.null.class%', [100, false]));
-        $container->setDefinition('handler_test', new Definition('%monolog.handler.null.class%', [100, false]));
+        $container->setParameter('monolog.handler.console.class', ConsoleHandler::class);
+        $container->setDefinition('monolog.handler.test', new Definition('%monolog.handler.console.class%', [100, false]));
+        $container->setDefinition('handler_test', new Definition('%monolog.handler.console.class%', [100, false]));
         $container->setAlias('monolog.handler.test2', 'handler_test');
         $definition->addMethodCall('pushHandler', [new Reference('monolog.handler.test')]);
         $definition->addMethodCall('pushHandler', [new Reference('monolog.handler.test2')]);

Tests/DependencyInjection/FixtureMonologExtensionTest.php

@@ -245,6 +245,20 @@ public function testPsr3MessageProcessingEnabled()
         $this->assertContainsEquals(['pushProcessor', [new Reference('monolog.processor.psr_log_message')]], $methodCalls, 'The PSR-3 processor should be enabled');
     }
 
+    public function testPsr3MessageProcessingDisabledOnNullHandler()
+    {
+        if (\Monolog\Logger::API < 2) {
+            $this->markTestSkipped('This test requires Monolog v2');
+        }
+        $container = $this->getContainer('process_psr_3_messages_null');
+
+        $logger = $container->getDefinition('monolog.handler.custom');
+
+        $methodCalls = $logger->getMethodCalls();
+
+        $this->assertNotContainsEquals(['pushProcessor', [new Reference('monolog.processor.psr_log_message')]], $methodCalls, 'The PSR-3 processor should not be enabled');
+    }
+
     public function testPsr3MessageProcessingDisabled()
     {
         $container = $this->getContainer('process_psr_3_messages_disabled');

Tests/DependencyInjection/Fixtures/xml/process_psr_3_messages_null.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:monolog="http://symfony.com/schema/dic/monolog"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
+                        http://symfony.com/schema/dic/monolog http://symfony.com/schema/dic/monolog/monolog-1.0.xsd">
+    <monolog:config>
+        <monolog:handler name="custom" type="noop" process-psr-3-messages="true"/>
+    </monolog:config>
+</container>

Tests/DependencyInjection/Fixtures/yml/process_psr_3_messages_null.yml

@@ -0,0 +1,5 @@
+monolog:
+    handlers:
+        custom:
+            type: noop
+            process_psr_3_messages: true