Skip to content

[bundle] Tricky (unwanted?) configuration behaviour #634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Gnucki opened this issue Nov 14, 2018 · 2 comments
Closed

[bundle] Tricky (unwanted?) configuration behaviour #634

Gnucki opened this issue Nov 14, 2018 · 2 comments

Comments

@Gnucki
Copy link
Contributor

Gnucki commented Nov 14, 2018

I am trying to migrate from Sf4.1 to 4.2 so from enqueue 0.8 to 0.9.
I found a tricky behaviour with configuration that I am going to try to explain.

I have a bundle handling messaging for my apps where I define some enqueue config in the bundle:

enqueue:
    async_events:
        enabled: false
    transport:
        default:
            dsn: '%env(BROKER_DSN)%'
    client: ~

That I inject to enqueue extension with a prepend in my bundle extension:

class AsyncExtension extends Extension implements PrependExtensionInterface
{
    const CONFIG_PATH = __DIR__.'/../Resources/config';

    /**
     * {@inheritdoc}
     */
    public function prepend(ContainerBuilder $container)
    {
        $bundles = $container->getParameter('kernel.bundles');

        foreach ($container->getExtensions() as $name => $extension) {
            switch ($name) {
                case 'enqueue':
                    $config = Yaml::parseFile(self::CONFIG_PATH.'/enqueue.yaml');
                    $container->prependExtensionConfig($name, $config['enqueue']);
                    break;
                case 'framework':
                    $config = Yaml::parseFile(self::CONFIG_PATH.'/messenger.yaml');
                    $container->prependExtensionConfig($name, $config['framework']);
                    break;
            }
        }
    }

    // ...
}

But that config is never used! Why? Because of a conjunction of things!

First is that code in enqueue-bundle:

// Enqueue\Bundle\DependencyInjection/Configuration

$transportNode = $rootNode->children()->arrayNode('transport');
$transportNode
    ->beforeNormalization()
    ->always(function ($value) {
        if (empty($value)) {
            return ['default' => ['dsn' => 'null:']];
        }
        if (is_string($value)) {
            return ['default' => ['dsn' => $value]];
        }

        if (is_array($value) && array_key_exists('dsn', $value)) {
            return ['default' => $value];
        }

        return $value;
    });

This is defining a standard configuration in case of an empty configuration.

Second, is this code in Symfony kernel:

// Symfony\Component\HttpKernel\DependencyInjection\MergeExtensionConfigurationPass

class MergeExtensionConfigurationPass extends BaseMergeExtensionConfigurationPass
{
    private $extensions;

    public function __construct(array $extensions)
    {
        $this->extensions = $extensions;
    }

    public function process(ContainerBuilder $container)
    {
        foreach ($this->extensions as $extension) {
            if (!\count($container->getExtensionConfig($extension))) {
                $container->loadFromExtension($extension, array());
            }
        }

        parent::process($container);
    }
}

This is creating a default empty config for extension with no config. Remember that this is my bundle which is defining a default config with prepend mechanism. There is no config in my app.

However, prepend mechanism happens in parent::process($container); so after this code. This means that, at this point, my config is empty. Then, an empty config is added to the list of config to merge in enqueue-bundle extension:

// Enqueue\Bundle\DependencyInjection\EnqueueExtension

final class EnqueueExtension extends Extension implements PrependExtensionInterface
{
    public function load(array $configs, ContainerBuilder $container): void
    {
        $config = $this->processConfiguration($this->getConfiguration($configs, $container), $configs);
        // ...
    }
}      

The bad news? Prepend add config before others to make it overridable:

// Symfony\Component\DependencyInjection\ContainerBuilder

public function prependExtensionConfig($name, array $config)
{
    if (!isset($this->extensionConfigs[$name])) {
        $this->extensionConfigs[$name] = array();
    }

    array_unshift($this->extensionConfigs[$name], $config);
}

This means that default config created by MergeExtensionConfigurationPass override the one of my bundle at config merge step! Finally, I have:

enqueue:
    async_events:
        enabled: false
    transport:
        default:
            dsn: 'null:'
    client: ~

I hope I have been clear in my explanations. Any thoughts on that?

@makasim
Copy link
Member

makasim commented Nov 15, 2018

The magic will be removed in #628, one would have to explicitly configure everything.

@Gnucki
Copy link
Contributor Author

Gnucki commented Nov 15, 2018

Oh, nice! Waiting for it to be merged so!

@makasim makasim closed this as completed Nov 15, 2018
@makasim makasim added the 0.9 label Nov 27, 2018
@makasim makasim added this to the 0.9 milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants