Skip to content

Commit 09ebbd8

Browse files
jderussejderusse
committed
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
2 parents 20611cc + ab41eea commit 09ebbd8

File tree

6 files changed

+87
-13
lines changed

6 files changed

+87
-13
lines changed

‎DependencyInjection/Compiler/AddProcessorsPass.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111

1212
namespace Symfony\Bundle\MonologBundle\DependencyInjection\Compiler;
1313

14-
use Symfony\Component\DependencyInjection\ContainerBuilder;
14+
use Symfony\Component\DependencyInjection\ChildDefinition;
1515
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
16+
use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Component\DependencyInjection\Reference;
1718

1819
/**
@@ -36,6 +37,14 @@ public function process(ContainerBuilder $container)
3637

3738
if (!empty($tag['handler'])) {
3839
$definition = $container->findDefinition(sprintf('monolog.handler.%s', $tag['handler']));
40+
$parentDef = $definition;
41+
while (!$parentDef->getClass() && $parentDef instanceof ChildDefinition) {
42+
$parentDef = $container->findDefinition($parentDef->getParent());
43+
}
44+
$class = $container->getParameterBag()->resolveValue($parentDef->getClass());
45+
if (!method_exists($class, 'pushProcessor')) {
46+
throw new \InvalidArgumentException(sprintf('The "%s" handler does not accept processors', $tag['handler']));
47+
}
3948
} elseif (!empty($tag['channel'])) {
4049
if ('app' === $tag['channel']) {
4150
$definition = $container->getDefinition('monolog.logger');

‎DependencyInjection/MonologExtension.php

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Symfony\Bundle\FullStack;
2424
use Symfony\Component\Config\FileLocator;
2525
use Symfony\Component\DependencyInjection\Argument\BoundArgument;
26+
use Symfony\Component\DependencyInjection\ChildDefinition;
2627
use Symfony\Component\DependencyInjection\ContainerBuilder;
2728
use Symfony\Component\DependencyInjection\Definition;
2829
use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
@@ -191,14 +192,16 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
191192
}
192193

193194
if ($handler['process_psr_3_messages']) {
194-
$processorId = 'monolog.processor.psr_log_message';
195-
if (!$container->hasDefinition($processorId)) {
196-
$processor = new Definition('Monolog\\Processor\\PsrLogMessageProcessor');
197-
$processor->setPublic(false);
198-
$container->setDefinition($processorId, $processor);
199-
}
195+
if (method_exists($handlerClass, 'pushProcessor')) {
196+
$processorId = 'monolog.processor.psr_log_message';
197+
if (!$container->hasDefinition($processorId)) {
198+
$processor = new Definition('Monolog\\Processor\\PsrLogMessageProcessor');
199+
$processor->setPublic(false);
200+
$container->setDefinition($processorId, $processor);
201+
}
200202

201-
$definition->addMethodCall('pushProcessor', [new Reference($processorId)]);
203+
$definition->addMethodCall('pushProcessor', [new Reference($processorId)]);
204+
}
202205
}
203206

204207
switch ($handler['type']) {
@@ -892,6 +895,7 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
892895
case 'browser_console':
893896
case 'test':
894897
case 'null':
898+
case 'noop':
895899
case 'debug':
896900
$definition->setArguments([
897901
$handler['level'],

‎Tests/DependencyInjection/Compiler/AddProcessorsPassTest.php

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@
1111

1212
namespace Symfony\Bundle\MonologBundle\Tests\DependencyInjection\Compiler;
1313

14+
use Monolog\Handler\NullHandler;
15+
use Monolog\Logger;
1416
use PHPUnit\Framework\TestCase;
17+
use Symfony\Bridge\Monolog\Handler\ConsoleHandler;
1518
use Symfony\Bundle\MonologBundle\DependencyInjection\Compiler\AddProcessorsPass;
16-
use Symfony\Component\DependencyInjection\Reference;
17-
use Symfony\Component\DependencyInjection\Definition;
18-
use Symfony\Component\DependencyInjection\ContainerBuilder;
1919
use Symfony\Component\Config\FileLocator;
20+
use Symfony\Component\DependencyInjection\ContainerBuilder;
21+
use Symfony\Component\DependencyInjection\Definition;
22+
use Symfony\Component\DependencyInjection\Loader\FileLoader;
2023
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
24+
use Symfony\Component\DependencyInjection\Reference;
2125

2226
class AddProcessorsPassTest extends TestCase
2327
{
@@ -36,15 +40,42 @@ public function testHandlerProcessors()
3640
$this->assertEquals(['pushProcessor', [new Reference('test2')]], $calls[0]);
3741
}
3842

43+
public function testFailureOnHandlerWithoutPushProcessor()
44+
{
45+
$container = new ContainerBuilder();
46+
$loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../../../Resources/config'));
47+
$loader->load('monolog.xml');
48+
49+
$service = new Definition(NullHandler::class);
50+
$service->addTag('monolog.processor', ['handler' => 'test3']);
51+
$container->setDefinition('monolog.handler.test3', $service);
52+
53+
$container->getCompilerPassConfig()->setOptimizationPasses([]);
54+
$container->getCompilerPassConfig()->setRemovingPasses([]);
55+
$container->addCompilerPass(new AddProcessorsPass());
56+
57+
if (Logger::API < 2) {
58+
$container->compile();
59+
$service = $container->getDefinition('monolog.handler.test3');
60+
$calls = $service->getMethodCalls();
61+
$this->assertCount(1, $calls);
62+
} else {
63+
$this->expectException(\InvalidArgumentException::class);
64+
$this->expectExceptionMessage('The "test3" handler does not accept processors');
65+
$container->compile();
66+
}
67+
}
68+
3969
protected function getContainer()
4070
{
4171
$container = new ContainerBuilder();
4272
$loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../../../Resources/config'));
4373
$loader->load('monolog.xml');
4474

4575
$definition = $container->getDefinition('monolog.logger_prototype');
46-
$container->setDefinition('monolog.handler.test', new Definition('%monolog.handler.null.class%', [100, false]));
47-
$container->setDefinition('handler_test', new Definition('%monolog.handler.null.class%', [100, false]));
76+
$container->setParameter('monolog.handler.console.class', ConsoleHandler::class);
77+
$container->setDefinition('monolog.handler.test', new Definition('%monolog.handler.console.class%', [100, false]));
78+
$container->setDefinition('handler_test', new Definition('%monolog.handler.console.class%', [100, false]));
4879
$container->setAlias('monolog.handler.test2', 'handler_test');
4980
$definition->addMethodCall('pushHandler', [new Reference('monolog.handler.test')]);
5081
$definition->addMethodCall('pushHandler', [new Reference('monolog.handler.test2')]);

‎Tests/DependencyInjection/FixtureMonologExtensionTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,20 @@ public function testPsr3MessageProcessingEnabled()
245245
$this->assertContainsEquals(['pushProcessor', [new Reference('monolog.processor.psr_log_message')]], $methodCalls, 'The PSR-3 processor should be enabled');
246246
}
247247

248+
public function testPsr3MessageProcessingDisabledOnNullHandler()
249+
{
250+
if (\Monolog\Logger::API < 2) {
251+
$this->markTestSkipped('This test requires Monolog v2');
252+
}
253+
$container = $this->getContainer('process_psr_3_messages_null');
254+
255+
$logger = $container->getDefinition('monolog.handler.custom');
256+
257+
$methodCalls = $logger->getMethodCalls();
258+
259+
$this->assertNotContainsEquals(['pushProcessor', [new Reference('monolog.processor.psr_log_message')]], $methodCalls, 'The PSR-3 processor should not be enabled');
260+
}
261+
248262
public function testPsr3MessageProcessingDisabled()
249263
{
250264
$container = $this->getContainer('process_psr_3_messages_disabled');

‎Tests/DependencyInjection/Fixtures/xml/process_psr_3_messages_null.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" ?>
2+
3+
<container xmlns="http://symfony.com/schema/dic/services"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xmlns:monolog="http://symfony.com/schema/dic/monolog"
6+
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
7+
http://symfony.com/schema/dic/monolog http://symfony.com/schema/dic/monolog/monolog-1.0.xsd">
8+
<monolog:config>
9+
<monolog:handler name="custom" type="noop" process-psr-3-messages="true"/>
10+
</monolog:config>
11+
</container>

‎Tests/DependencyInjection/Fixtures/yml/process_psr_3_messages_null.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
monolog:
2+
handlers:
3+
custom:
4+
type: noop
5+
process_psr_3_messages: true

0 commit comments

Comments
 (0)