Skip to content

Commit

Permalink
feature #52843 [DependencyInjection] Prepending extension configs wit…
Browse files Browse the repository at this point in the history
…h file loaders (yceruto)

This PR was merged into the 7.1 branch.

Discussion
----------

[DependencyInjection] Prepending extension configs with file loaders

| 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.

```php
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:
```php
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!

Commits
-------

6ffd706 prepend extension configs with file loaders
  • Loading branch information
nicolas-grekas committed Apr 3, 2024
2 parents 25c93ba + 6ffd706 commit 4ce4e5e
Show file tree
Hide file tree
Showing 17 changed files with 201 additions and 38 deletions.
2 changes: 2 additions & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Expand Up @@ -10,6 +10,8 @@ CHANGELOG
* Add `#[Lazy]` attribute as shortcut for `#[Autowire(lazy: [bool|string])]` and `#[Autoconfigure(lazy: [bool|string])]`
* Add `#[AutowireMethodOf]` attribute to autowire a method of a service as a callable
* Make `ContainerBuilder::registerAttributeForAutoconfiguration()` propagate to attribute classes that extend the registered class
* Add argument `$prepend` to `FileLoader::construct()` to prepend loaded configuration instead of appending it
* [BC BREAK] When used in the `prependExtension()` methods, the `ContainerConfigurator::import()` method now prepends the configuration instead of appending it

7.0
---
Expand Down
Expand Up @@ -49,7 +49,7 @@ final public function prepend(ContainerBuilder $container): void
$this->prependExtension($configurator, $container);
};

$this->executeConfiguratorCallback($container, $callback, $this);
$this->executeConfiguratorCallback($container, $callback, $this, true);
}

final public function load(array $configs, ContainerBuilder $container): void
Expand Down
Expand Up @@ -30,10 +30,10 @@
*/
trait ExtensionTrait
{
private function executeConfiguratorCallback(ContainerBuilder $container, \Closure $callback, ConfigurableExtensionInterface $subject): void
private function executeConfiguratorCallback(ContainerBuilder $container, \Closure $callback, ConfigurableExtensionInterface $subject, bool $prepend = false): void
{
$env = $container->getParameter('kernel.environment');
$loader = $this->createContainerLoader($container, $env);
$loader = $this->createContainerLoader($container, $env, $prepend);
$file = (new \ReflectionObject($subject))->getFileName();
$bundleLoader = $loader->getResolver()->resolve($file);
if (!$bundleLoader instanceof PhpFileLoader) {
Expand All @@ -50,15 +50,15 @@ private function executeConfiguratorCallback(ContainerBuilder $container, \Closu
}
}

private function createContainerLoader(ContainerBuilder $container, string $env): DelegatingLoader
private function createContainerLoader(ContainerBuilder $container, string $env, bool $prepend): DelegatingLoader
{
$buildDir = $container->getParameter('kernel.build_dir');
$locator = new FileLocator();
$resolver = new LoaderResolver([
new XmlFileLoader($container, $locator, $env),
new YamlFileLoader($container, $locator, $env),
new XmlFileLoader($container, $locator, $env, $prepend),
new YamlFileLoader($container, $locator, $env, $prepend),
new IniFileLoader($container, $locator, $env),
new PhpFileLoader($container, $locator, $env, new ConfigBuilderGenerator($buildDir)),
new PhpFileLoader($container, $locator, $env, new ConfigBuilderGenerator($buildDir), $prepend),
new GlobFileLoader($container, $locator, $env),
new DirectoryLoader($container, $locator, $env),
new ClosureLoader($container, $env),
Expand Down
47 changes: 46 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
Expand Up @@ -45,10 +45,17 @@ abstract class FileLoader extends BaseFileLoader
/** @var array<string, Alias> */
protected array $aliases = [];
protected bool $autoRegisterAliasesForSinglyImplementedInterfaces = true;
protected bool $prepend = false;
protected array $extensionConfigs = [];
protected int $importing = 0;

public function __construct(ContainerBuilder $container, FileLocatorInterface $locator, ?string $env = null)
/**
* @param bool $prepend Whether to prepend extension config instead of appending them
*/
public function __construct(ContainerBuilder $container, FileLocatorInterface $locator, ?string $env = null, bool $prepend = false)
{
$this->container = $container;
$this->prepend = $prepend;

parent::__construct($locator, $env);
}
Expand All @@ -66,6 +73,7 @@ public function import(mixed $resource, ?string $type = null, bool|string $ignor
throw new \TypeError(sprintf('Invalid argument $ignoreErrors provided to "%s::import()": boolean or "not_found" expected, "%s" given.', static::class, get_debug_type($ignoreErrors)));
}

++$this->importing;
try {
return parent::import(...$args);
} catch (LoaderLoadException $e) {
Expand All @@ -82,6 +90,8 @@ public function import(mixed $resource, ?string $type = null, bool|string $ignor
if (__FILE__ !== $frame['file']) {
throw $e;
}
} finally {
--$this->importing;
}

return null;
Expand Down Expand Up @@ -217,6 +227,41 @@ public function registerAliasesForSinglyImplementedInterfaces(): void
$this->interfaces = $this->singlyImplemented = $this->aliases = [];
}

final protected function loadExtensionConfig(string $namespace, array $config): void
{
if (!$this->prepend) {
$this->container->loadFromExtension($namespace, $config);

return;
}

if ($this->importing) {
if (!isset($this->extensionConfigs[$namespace])) {
$this->extensionConfigs[$namespace] = [];
}
array_unshift($this->extensionConfigs[$namespace], $config);

return;
}

$this->container->prependExtensionConfig($namespace, $config);
}

final protected function loadExtensionConfigs(): void
{
if ($this->importing || !$this->extensionConfigs) {
return;
}

foreach ($this->extensionConfigs as $namespace => $configs) {
foreach ($configs as $config) {
$this->container->prependExtensionConfig($namespace, $config);
}
}

$this->extensionConfigs = [];
}

/**
* Registers a definition in the container with its instanceof-conditionals.
*/
Expand Down
17 changes: 13 additions & 4 deletions src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php
Expand Up @@ -36,9 +36,9 @@ class PhpFileLoader extends FileLoader
protected bool $autoRegisterAliasesForSinglyImplementedInterfaces = false;
private ?ConfigBuilderGeneratorInterface $generator;

public function __construct(ContainerBuilder $container, FileLocatorInterface $locator, ?string $env = null, ?ConfigBuilderGeneratorInterface $generator = null)
public function __construct(ContainerBuilder $container, FileLocatorInterface $locator, ?string $env = null, ?ConfigBuilderGeneratorInterface $generator = null, bool $prepend = false)
{
parent::__construct($container, $locator, $env);
parent::__construct($container, $locator, $env, $prepend);
$this->generator = $generator;
}

Expand Down Expand Up @@ -145,10 +145,19 @@ class_exists(ContainerConfigurator::class);

$callback(...$arguments);

/** @var ConfigBuilderInterface $configBuilder */
$this->loadFromExtensions($configBuilders);
}

/**
* @param iterable<ConfigBuilderInterface> $configBuilders
*/
private function loadFromExtensions(iterable $configBuilders): void
{
foreach ($configBuilders as $configBuilder) {
$containerConfigurator->extension($configBuilder->getExtensionAlias(), $configBuilder->toArray());
$this->loadExtensionConfig($configBuilder->getExtensionAlias(), $configBuilder->toArray());
}

$this->loadExtensionConfigs();
}

/**
Expand Down
Expand Up @@ -808,7 +808,7 @@ private function validateExtensions(\DOMDocument $dom, string $file): void
}

// can it be handled by an extension?
if (!$this->container->hasExtension($node->namespaceURI)) {
if (!$this->prepend && !$this->container->hasExtension($node->namespaceURI)) {
$extensionNamespaces = array_filter(array_map(fn (ExtensionInterface $ext) => $ext->getNamespace(), $this->container->getExtensions()));
throw new InvalidArgumentException(sprintf('There is no extension able to load the configuration for "%s" (in "%s"). Looked for namespace "%s", found "%s".', $node->tagName, $file, $node->namespaceURI, $extensionNamespaces ? implode('", "', $extensionNamespaces) : 'none'));
}
Expand All @@ -830,8 +830,10 @@ private function loadFromExtensions(\DOMDocument $xml): void
$values = [];
}

$this->container->loadFromExtension($node->namespaceURI, $values);
$this->loadExtensionConfig($node->namespaceURI, $values);
}

$this->loadExtensionConfigs();
}

/**
Expand Down
38 changes: 23 additions & 15 deletions src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
Expand Up @@ -129,22 +129,28 @@ public function load(mixed $resource, ?string $type = null): mixed
return null;
}

$this->loadContent($content, $path);
++$this->importing;
try {
$this->loadContent($content, $path);

// per-env configuration
if ($this->env && isset($content['when@'.$this->env])) {
if (!\is_array($content['when@'.$this->env])) {
throw new InvalidArgumentException(sprintf('The "when@%s" key should contain an array in "%s". Check your YAML syntax.', $this->env, $path));
}
// per-env configuration
if ($this->env && isset($content['when@'.$this->env])) {
if (!\is_array($content['when@'.$this->env])) {
throw new InvalidArgumentException(sprintf('The "when@%s" key should contain an array in "%s". Check your YAML syntax.', $this->env, $path));
}

$env = $this->env;
$this->env = null;
try {
$this->loadContent($content['when@'.$env], $path);
} finally {
$this->env = $env;
$env = $this->env;
$this->env = null;
try {
$this->loadContent($content['when@'.$env], $path);
} finally {
$this->env = $env;
}
}
} finally {
--$this->importing;
}
$this->loadExtensionConfigs();

return null;
}
Expand Down Expand Up @@ -802,7 +808,7 @@ private function validate(mixed $content, string $file): ?array
continue;
}

if (!$this->container->hasExtension($namespace)) {
if (!$this->prepend && !$this->container->hasExtension($namespace)) {
$extensionNamespaces = array_filter(array_map(fn (ExtensionInterface $ext) => $ext->getAlias(), $this->container->getExtensions()));
throw new InvalidArgumentException(sprintf('There is no extension able to load the configuration for "%s" (in "%s"). Looked for namespace "%s", found "%s".', $namespace, $file, $namespace, $extensionNamespaces ? sprintf('"%s"', implode('", "', $extensionNamespaces)) : 'none'));
}
Expand Down Expand Up @@ -941,12 +947,14 @@ private function loadFromExtensions(array $content): void
continue;
}

if (!\is_array($values) && null !== $values) {
if (!\is_array($values)) {
$values = [];
}

$this->container->loadFromExtension($namespace, $values);
$this->loadExtensionConfig($namespace, $values);
}

$this->loadExtensionConfigs();
}

private function checkDefinition(string $id, array $definition, string $file): void
Expand Down
Expand Up @@ -54,28 +54,55 @@ public function configure(DefinitionConfigurator $definition): void
self::assertSame($expected, $this->processConfiguration($extension));
}

public function testPrependAppendExtensionConfig()
public function testPrependExtensionConfig()
{
$extension = new class() extends AbstractExtension {
public function configure(DefinitionConfigurator $definition): void
{
$definition->rootNode()
->children()
->scalarNode('foo')->end()
->end();
}

public function prependExtension(ContainerConfigurator $container, ContainerBuilder $builder): void
{
// append config
$container->extension('third', ['foo' => 'append']);
// prepend config from plain array
$container->extension('third', ['foo' => 'pong'], true);

// prepend config from external file
$container->import('../Fixtures/config/packages/ping.yaml');
}

public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void
{
$container->parameters()->set('foo_param', $config['foo']);
}

// prepend config
$container->extension('third', ['foo' => 'prepend'], true);
public function getAlias(): string
{
return 'third';
}
};

$container = $this->processPrependExtension($extension);

$expected = [
['foo' => 'prepend'],
['foo' => 'a'],
['foo' => 'c1'],
['foo' => 'c2'],
['foo' => 'b'],
['foo' => 'ping'],
['foo' => 'zaa'],
['foo' => 'pong'],
['foo' => 'bar'],
['foo' => 'append'],
];

self::assertSame($expected, $container->getExtensionConfig('third'));

$container = $this->processLoadExtension($extension, $expected);

self::assertSame('bar', $container->getParameter('foo_param'));
}

public function testLoadExtension()
Expand Down
@@ -0,0 +1,10 @@
imports:
- { resource: './third_a.yaml' }
- { resource: './third_b.yaml' }

third:
foo: ping

when@test:
third:
foo: zaa
@@ -0,0 +1,2 @@
third:
foo: a
@@ -0,0 +1,5 @@
imports:
- { resource: './third_c.yaml' }

third:
foo: b
@@ -0,0 +1,6 @@
third:
foo: c1

when@test:
third:
foo: c2
Expand Up @@ -11,9 +11,13 @@
<service id="project.service.foo" class="BAR" public="true"/>
</services>

<project:bar babar="babar">
<project:bar foo="ping">
<another />
<another2>%project.parameter.foo%</another2>
</project:bar>

<when env="test">
<project:bar foo="zaa">
</project:bar>
</when>
</container>
Expand Up @@ -47,6 +47,21 @@ public function testLoad()
$this->assertEquals('foo', $container->getParameter('foo'), '->load() loads a PHP file resource');
}

public function testPrependExtensionConfig()
{
$container = new ContainerBuilder();
$container->registerExtension(new \AcmeExtension());
$container->prependExtensionConfig('acme', ['foo' => 'bar']);
$loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Fixtures'), 'prod', new ConfigBuilderGenerator(sys_get_temp_dir()), true);
$loader->load('config/config_builder.php');

$expected = [
['color' => 'blue'],
['foo' => 'bar'],
];
$this->assertSame($expected, $container->getExtensionConfig('acme'));
}

public function testConfigServices()
{
$fixtures = realpath(__DIR__.'/../Fixtures');
Expand Down
Expand Up @@ -604,6 +604,20 @@ public function testExtensions()
}
}

public function testPrependExtensionConfig()
{
$container = new ContainerBuilder();
$container->prependExtensionConfig('http://www.example.com/schema/project', ['foo' => 'bar']);
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'), prepend: true);
$loader->load('extensions/services1.xml');

$expected = [
['foo' => 'ping'],
['foo' => 'bar'],
];
$this->assertSame($expected, $container->getExtensionConfig('http://www.example.com/schema/project'));
}

public function testExtensionInPhar()
{
if (\extension_loaded('suhosin') && !str_contains(\ini_get('suhosin.executor.include.whitelist'), 'phar')) {
Expand Down

0 comments on commit 4ce4e5e

Please sign in to comment.