alexdebril/rss-atom-bundle

Issue With Logger

bshoemaker136 opened this issue ยท 8 comments

In your extension class you have this:

$this->setDefinition($container, 'logger', 'Psr\Log\NullLogger');

Isn't that overriding the typical Monolog logger that has been provided by Symfony? Is there a reason to override that?

Hi,

It's not overriding the default logger because setDefinition checks if the definition exists or has an alias before :

    protected function setDefinition(ContainerBuilder $container, $serviceName, $className)
    {
        if ( ! $container->hasDefinition($serviceName) && ! $container->hasAlias($serviceName)) {
            $container->setDefinition($serviceName, new Definition($className));
        }

        return $this;
    }

It creates a logger instance if your dependency injection doesn't have one, which is the case during the unit tests execution.

Do you experience an issue with that ?

After the upgrade form version 2.x to 3.0.4 the service logger was substituted from Symfony\Bridge\Monolog\Logger to Psr\Log\NullLogger. Strangely $container->getDefinitions() method gets an empty array and the setDefinition doesn't work properly. Maybe setDefinition needs some extra control because as the documentation say:

In the load() method, all services and parameters related to this extension will be loaded. This method doesn't get the actual container instance, but a copy. This container only has the parameters from the actual container. After loading the services and parameters, the copy will be merged into the actual container, to ensure all services and parameters are also added to the actual container.

Same here.
It seems that hasDefinition and hasAlias will return both false (maybe container isn't processed yet?) and, so, definition gets and override.

I've open a PR for this issue --> #128
Seems to work in my local application.

Thank you @DonCallisto , it works fine with the demo application. ๐Ÿ‘

@alexdebril you're welcome. Maybe that's perfect for a patch version like 3.0.5. What do you think?

Thank you!