[DependencyInjection] Prepending extension configs with file loaders

[yceruto]

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	Fix #52789
License	MIT

#52636 continuation.

This is another try to simplify even more the prepending extension config strategy for Bundles and Extensions.

Feature: "Import and prepend an extension configuration from an external YAML, XML or PHP file"

Useful when you need to pre-configure some functionalities in your app, such as mapping some DBAL types provided by your bundle, or simply pre-configuring some default values automatically when your bundle is installed, without losing the ability to override them if desired in userland.

class AcmeFooBundle extends AbstractBundle
{
    public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void
    {
        // Before
        $config = Yaml::parse(file_get_contents(__DIR__.'/../config/doctrine.yaml'));
        $builder->prependExtensionConfig('doctrine', $config['doctrine']);

        // After
        $container->import('../config/doctrine.yaml');
    }
}

The "Before" section is limited to importing/parsing this kind of configuration by hand; it doesn't support features like when@env, recursive importing, or defining parameters/services in the same file. Further, you can't simply use ContainerConfigurator::import() or any *FileLoader::load() here, as they will append the extension config instead of prepending it, defeating the prepend method purpose and breaking your app's config strategy. Thus, you are forced to use $builder->prependExtensionConfig() as the only way.

This is because if you append any extension config using $container->import(), then you won't be able to change that config in your app as it's merged with priority. If anyone is doing that currently, it shouldn't be intentional.

Therefore, the "After" proposal changes that behavior for $container->import(). Now, all extension configurations encountered in your external file will be prepended instead. BUT, this will only happen here, at this moment, during the prependExtension() method.

Of course, this little behavior change is a "BC break". However, it is a behavior that makes more sense than the previous one, enabling the usage of ContainerConfigurator::import() here, which was previously ineffective.

This capability is also available for YamlFileLoader, XmlFileLoader and PhpFileLoader through a new boolean constructor argument:

class AcmeFooBundle extends AbstractBundle
{
    public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void
    {
        // Before
        // - It can't be used for prepending config purposes

        // After
        $loader = new YamlFileLoader($builder, new FileLocator(__DIR__.'/../config/'), prepend: true);
        $loader->load('doctrine.yaml'); // now it prepends extension configs as default behavior
        // or
        $loader->import('prepend/*.yaml');
    }
}

These changes only affect the loading strategy for extension configs; everything else keeps working as before.

Cheers!

[nicolas-grekas]

I'm just wondering about recursive imports: are they also reversed?

[yceruto]

I'm just wondering about recursive imports: are they also reversed?

The behavior is set up for the whole process, so yes, all extension configs imported recursively will be prepended too. However, unless there are split configs of the same extension distributed into several files matching the same config key, it doesn't matter.

[yceruto]

The behavior is set up for the whole process, so yes, all extension configs imported recursively will be prepended too.

Which is expected if you're importing from the prepend method.

[nicolas-grekas]

Maybe you already answered but just to be sure: if I have a yaml file that imports two other files, I'd expect them to be loaded in top-down order, is that going to be the case?

[yceruto]

Maybe you already answered but just to be sure: if I have a yaml file that imports two other files, I'd expect them to be loaded in top-down order, is that going to be the case?

I will confirm with a test case to ensure we know what the behavior will be.

[nicolas-grekas]

LGTM (small rebase needed)

[yceruto]

Thanks Nicolas! rebased done.

[yceruto]

Friendly ping @symfony/mergers, there is a BC behavior break on the table here; it would be great to have a double check in case we miss something.

[yceruto]

Hey @nicolas-grekas, I rebased, rechecked, and updated another part where when@env wasn't working as expected. I added another test case to cover it. So, after a while, this looks ready to me. Wdyt?

[nicolas-grekas]

Just some minor CS things and 🚀

[nicolas-grekas]

Anyone @symfony/mergers ? (after a small rebase)

[nicolas-grekas]

Thank you @yceruto.