Skip to content

Commit ff3fbd7

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Handle returning arrays and config-builders from config files
1 parent 9c413a3 commit ff3fbd7

File tree

8 files changed

+116
-14
lines changed

8 files changed

+116
-14
lines changed

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CHANGELOG
1010
* Add argument `$target` to `ContainerBuilder::registerAliasForArgument()`
1111
* Deprecate registering a service without a class when its id is a non-existing FQCN
1212
* Allow multiple `#[AsDecorator]` attributes
13+
* Handle returning arrays and config-builders from config files
1314

1415
7.3
1516
---

‎src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,42 @@ public function load(mixed $resource, ?string $type = null): mixed
5757
$this->setCurrentDir(\dirname($path));
5858
$this->container->fileExists($path);
5959

60+
// Force load ContainerConfigurator to make env(), param() etc available.
61+
class_exists(ContainerConfigurator::class);
62+
6063
// the closure forbids access to the private scope in the included file
6164
$load = \Closure::bind(function ($path, $env) use ($container, $loader, $resource, $type) {
6265
return include $path;
6366
}, $this, ProtectedPhpFileLoader::class);
6467

6568
try {
66-
$callback = $load($path, $this->env);
69+
if (1 === $result = $load($path, $this->env)) {
70+
$result = null;
71+
}
6772

68-
if (\is_object($callback) && \is_callable($callback)) {
69-
$this->executeCallback($callback, new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource, $this->env), $path);
73+
if (\is_object($result) && \is_callable($result)) {
74+
$result = $this->executeCallback($result, new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource, $this->env), $path);
7075
}
76+
if ($result instanceof ConfigBuilderInterface) {
77+
$this->loadExtensionConfig($result->getExtensionAlias(), ContainerConfigurator::processValue($result->toArray()));
78+
} elseif (is_iterable($result)) {
79+
foreach ($result as $key => $config) {
80+
if ($config instanceof ConfigBuilderInterface) {
81+
if (\is_string($key) && $config->getExtensionAlias() !== $key) {
82+
throw new InvalidArgumentException(\sprintf('The extension alias "%s" of the "%s" config builder does not match the key "%s" in file "%s".', $config->getExtensionAlias(), get_debug_type($config), $key, $path));
83+
}
84+
$this->loadExtensionConfig($config->getExtensionAlias(), ContainerConfigurator::processValue($config->toArray()));
85+
} elseif (!\is_string($key) || !\is_array($config)) {
86+
throw new InvalidArgumentException(\sprintf('The configuration returned in file "%s" must yield only string-keyed arrays or ConfigBuilderInterface values.', $path));
87+
} else {
88+
$this->loadExtensionConfig($key, ContainerConfigurator::processValue($config));
89+
}
90+
}
91+
} elseif (null !== $result) {
92+
throw new InvalidArgumentException(\sprintf('The return value in config file "%s" is invalid.', $path));
93+
}
94+
95+
$this->loadExtensionConfigs();
7196
} finally {
7297
$this->instanceof = [];
7398
$this->registerAliasesForSinglyImplementedInterfaces();
@@ -92,7 +117,7 @@ public function supports(mixed $resource, ?string $type = null): bool
92117
/**
93118
* Resolve the parameters to the $callback and execute it.
94119
*/
95-
private function executeCallback(callable $callback, ContainerConfigurator $containerConfigurator, string $path): void
120+
private function executeCallback(callable $callback, ContainerConfigurator $containerConfigurator, string $path): mixed
96121
{
97122
$callback = $callback(...);
98123
$arguments = [];
@@ -125,7 +150,7 @@ private function executeCallback(callable $callback, ContainerConfigurator $cont
125150
}
126151

127152
if ($excluded) {
128-
return;
153+
return null;
129154
}
130155

131156
foreach ($r->getParameters() as $parameter) {
@@ -163,21 +188,19 @@ private function executeCallback(callable $callback, ContainerConfigurator $cont
163188
}
164189
}
165190

166-
// Force load ContainerConfigurator to make env(), param() etc available.
167-
class_exists(ContainerConfigurator::class);
168-
169191
++$this->importing;
170192
try {
171-
$callback(...$arguments);
193+
return $callback(...$arguments);
194+
} catch (\Throwable $e) {
195+
$configBuilders = [];
196+
throw $e;
172197
} finally {
173198
--$this->importing;
174-
}
175199

176-
foreach ($configBuilders as $configBuilder) {
177-
$this->loadExtensionConfig($configBuilder->getExtensionAlias(), ContainerConfigurator::processValue($configBuilder->toArray()));
200+
foreach ($configBuilders as $configBuilder) {
201+
$this->loadExtensionConfig($configBuilder->getExtensionAlias(), ContainerConfigurator::processValue($configBuilder->toArray()));
202+
}
178203
}
179-
180-
$this->loadExtensionConfigs();
181204
}
182205

183206
/**

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/AcmeConfig.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ class AcmeConfig implements ConfigBuilderInterface
1010

1111
private $nested;
1212

13+
public function __construct(array $config = [])
14+
{
15+
$this->color = $config['color'] ?? null;
16+
$this->nested = $config['nested'] ?? null;
17+
}
18+
1319
public function color($value)
1420
{
1521
$this->color = $value;

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/return_config_builder.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
use Symfony\Component\DependencyInjection\Tests\Fixtures\AcmeConfig;
4+
5+
$config = new AcmeConfig();
6+
$config->color('red');
7+
8+
return $config;

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/return_generator.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
use Symfony\Component\DependencyInjection\Tests\Fixtures\AcmeConfig;
4+
5+
return function () {
6+
yield new AcmeConfig(['color' => 'red']);
7+
};

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/return_invalid_types.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
3+
return new \stdClass();

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/return_iterable_configs.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
use Symfony\Component\DependencyInjection\Tests\Fixtures\AcmeConfig;
4+
5+
return [
6+
'acme' => ['color' => 'red'],
7+
new AcmeConfig(),
8+
];

‎src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,50 @@ public function testNamedClosure()
319319
$dumper = new PhpDumper($container);
320320
$this->assertStringEqualsFile(\dirname(__DIR__).'/Fixtures/php/named_closure_compiled.php', $dumper->dump());
321321
}
322+
323+
public function testReturnsConfigBuilderObject()
324+
{
325+
$container = new ContainerBuilder();
326+
$container->registerExtension(new \AcmeExtension());
327+
$loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Fixtures/config'), 'prod', new ConfigBuilderGenerator(sys_get_temp_dir()));
328+
329+
$loader->load('return_config_builder.php');
330+
331+
$this->assertSame([['color' => 'red']], $container->getExtensionConfig('acme'));
332+
}
333+
334+
public function testReturnsIterableOfArraysAndBuilders()
335+
{
336+
$container = new ContainerBuilder();
337+
$container->registerExtension(new \AcmeExtension());
338+
$loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Fixtures/config'), 'prod', new ConfigBuilderGenerator(sys_get_temp_dir()));
339+
340+
$loader->load('return_iterable_configs.php');
341+
342+
$configs = $container->getExtensionConfig('acme');
343+
$this->assertCount(2, $configs);
344+
$this->assertSame('red', $configs[0]['color']);
345+
$this->assertArrayHasKey('color', $configs[1]);
346+
}
347+
348+
public function testThrowsOnInvalidReturnType()
349+
{
350+
$container = new ContainerBuilder();
351+
$loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Fixtures/config'), 'prod', new ConfigBuilderGenerator(sys_get_temp_dir()));
352+
353+
$this->expectException(InvalidArgumentException::class);
354+
$this->expectExceptionMessageMatches('/The return value in config file/');
355+
356+
$loader->load('return_invalid_types.php');
357+
}
358+
359+
public function testReturnsGenerator()
360+
{
361+
$container = new ContainerBuilder();
362+
$container->registerExtension(new \AcmeExtension());
363+
$loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Fixtures/config'), 'prod', new ConfigBuilderGenerator(sys_get_temp_dir()));
364+
365+
$loader->load('return_generator.php');
366+
$this->assertSame([['color' => 'red']], $container->getExtensionConfig('acme'));
367+
}
322368
}

0 commit comments

Comments
 (0)