From f167c77eafc55dcce5da7bc7a34842b33328fa4e Mon Sep 17 00:00:00 2001 From: Mathias Arlaud Date: Fri, 4 Oct 2019 14:50:20 +0200 Subject: [PATCH] Handle non existent decorated services --- .../DependencyInjection/CHANGELOG.md | 1 + .../Compiler/DecoratorServicePass.php | 18 +++++- .../Compiler/ResolveChildDefinitionsPass.php | 3 +- .../Compiler/ResolveInvalidReferencesPass.php | 15 +++-- .../DependencyInjection/Definition.php | 22 ++++++-- .../DependencyInjection/Dumper/XmlDumper.php | 10 +++- .../DependencyInjection/Dumper/YamlDumper.php | 10 +++- .../Configurator/Traits/DecorateTrait.php | 10 +++- .../Loader/XmlFileLoader.php | 16 +++++- .../Loader/YamlFileLoader.php | 23 ++++++-- .../schema/dic/services/services-1.0.xsd | 9 +++ .../Compiler/DecoratorServicePassTest.php | 56 +++++++++++++++++++ .../ResolveInvalidReferencesPassTest.php | 38 +++++++++++++ .../Tests/DefinitionTest.php | 8 +++ .../Tests/Dumper/XmlDumperTest.php | 10 ++++ .../Tests/Dumper/YamlDumperTest.php | 7 +++ .../Tests/Fixtures/containers/container34.php | 12 ++++ .../Tests/Fixtures/php/services_rot13_env.php | 2 +- .../php/services_service_locator_argument.php | 2 +- .../Fixtures/php/services_subscriber.php | 4 +- .../Tests/Fixtures/xml/services6.xml | 1 + .../yaml/bad_decoration_on_invalid_null.yml | 7 +++ .../Tests/Fixtures/yaml/services34.yml | 17 ++++++ .../Tests/Fixtures/yaml/services6.yml | 5 ++ .../Tests/Loader/XmlFileLoaderTest.php | 2 + .../Tests/Loader/YamlFileLoaderTest.php | 10 ++++ 26 files changed, 290 insertions(+), 28 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container34.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_decoration_on_invalid_null.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services34.yml diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 461d52abd1..f2794db75d 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -15,6 +15,7 @@ CHANGELOG * added support for improved syntax to define method calls in Yaml * added `LazyString` for lazy computation of string values injected into services * made the `%env(base64:...)%` processor able to decode base64url + * added ability to choose behavior of decorations on non existent decorated services 4.3.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php b/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php index f49e8cf949..da909ae489 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php @@ -13,6 +13,9 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\Reference; /** * Overwrites a service but keeps the overridden one. @@ -37,7 +40,9 @@ class DecoratorServicePass implements CompilerPassInterface $decoratingDefinitions = []; foreach ($definitions as list($id, $definition)) { - list($inner, $renamedId) = $definition->getDecoratedService(); + $decoratedService = $definition->getDecoratedService(); + list($inner, $renamedId) = $decoratedService; + $invalidBehavior = $decoratedService[3] ?? ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; $definition->setDecoratedService(null); @@ -45,6 +50,7 @@ class DecoratorServicePass implements CompilerPassInterface $renamedId = $id.'.inner'; } $definition->innerServiceId = $renamedId; + $definition->decorationOnInvalid = $invalidBehavior; // we create a new alias/service for the service we are replacing // to be able to reference it in the new one @@ -53,13 +59,21 @@ class DecoratorServicePass implements CompilerPassInterface $public = $alias->isPublic(); $private = $alias->isPrivate(); $container->setAlias($renamedId, new Alias((string) $alias, false)); - } else { + } elseif ($container->hasDefinition($inner)) { $decoratedDefinition = $container->getDefinition($inner); $public = $decoratedDefinition->isPublic(); $private = $decoratedDefinition->isPrivate(); $decoratedDefinition->setPublic(false); $container->setDefinition($renamedId, $decoratedDefinition); $decoratingDefinitions[$inner] = $decoratedDefinition; + } elseif (ContainerInterface::IGNORE_ON_INVALID_REFERENCE === $invalidBehavior) { + $container->removeDefinition($id); + continue; + } elseif (ContainerInterface::NULL_ON_INVALID_REFERENCE === $invalidBehavior) { + $public = $definition->isPublic(); + $private = $definition->isPrivate(); + } else { + throw new ServiceNotFoundException($inner, $id); } if (isset($decoratingDefinitions[$inner])) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php index f176cb7957..453e3f63c3 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveChildDefinitionsPass.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\ChildDefinition; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\ExceptionInterface; use Symfony\Component\DependencyInjection\Exception\RuntimeException; @@ -149,7 +150,7 @@ class ResolveChildDefinitionsPass extends AbstractRecursivePass if (null === $decoratedService) { $def->setDecoratedService($decoratedService); } else { - $def->setDecoratedService($decoratedService[0], $decoratedService[1], $decoratedService[2]); + $def->setDecoratedService($decoratedService[0], $decoratedService[1], $decoratedService[2], $decoratedService[3] ?? ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE); } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php index a0686554e1..948de421f7 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php @@ -42,7 +42,9 @@ class ResolveInvalidReferencesPass implements CompilerPassInterface $this->signalingException = new RuntimeException('Invalid reference.'); try { - $this->processValue($container->getDefinitions(), 1); + foreach ($container->getDefinitions() as $this->currentId => $definition) { + $this->processValue($definition); + } } finally { $this->container = $this->signalingException = null; } @@ -72,9 +74,6 @@ class ResolveInvalidReferencesPass implements CompilerPassInterface $i = 0; foreach ($value as $k => $v) { - if (!$rootLevel) { - $this->currentId = $k; - } try { if (false !== $i && $k !== $i++) { $i = false; @@ -101,6 +100,14 @@ class ResolveInvalidReferencesPass implements CompilerPassInterface if ($this->container->has($id = (string) $value)) { return $value; } + + $currentDefinition = $this->container->getDefinition($this->currentId); + + // resolve decorated service behavior depending on decorator service + if ($currentDefinition->innerServiceId === $id && ContainerInterface::NULL_ON_INVALID_REFERENCE === $currentDefinition->decorationOnInvalid) { + return null; + } + $invalidBehavior = $value->getInvalidBehavior(); if (ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $invalidBehavior && $value instanceof TypedReference && !$this->container->has($id)) { diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 06636b2999..e2bc713a3a 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -56,6 +56,13 @@ class Definition */ public $innerServiceId; + /** + * @internal + * + * Used to store the behavior to follow when using service decoration and the decorated service is invalid + */ + public $decorationOnInvalid; + /** * @param string|null $class The service class * @param array $arguments An array of arguments to pass to the service constructor @@ -127,26 +134,33 @@ class Definition /** * Sets the service that this service is decorating. * - * @param string|null $id The decorated service id, use null to remove decoration - * @param string|null $renamedId The new decorated service id - * @param int $priority The priority of decoration + * @param string|null $id The decorated service id, use null to remove decoration + * @param string|null $renamedId The new decorated service id + * @param int $priority The priority of decoration + * @param int $invalidBehavior The behavior to adopt when decorated is invalid * * @return $this * * @throws InvalidArgumentException in case the decorated service id and the new decorated service id are equals */ - public function setDecoratedService($id, $renamedId = null, $priority = 0) + public function setDecoratedService($id, $renamedId = null, $priority = 0/*, int $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE*/) { if ($renamedId && $id === $renamedId) { throw new InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id)); } + $invalidBehavior = 3 < \func_num_args() ? (int) func_get_arg(3) : ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + $this->changes['decorated_service'] = true; if (null === $id) { $this->decoratedService = null; } else { $this->decoratedService = [$id, $renamedId, (int) $priority]; + + if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $invalidBehavior) { + $this->decoratedService[] = $invalidBehavior; + } } return $this; diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php index 8060170612..dd3a1edf01 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php @@ -116,9 +116,15 @@ class XmlDumper extends Dumper if ($definition->isLazy()) { $service->setAttribute('lazy', 'true'); } - if (null !== $decorated = $definition->getDecoratedService()) { - list($decorated, $renamedId, $priority) = $decorated; + if (null !== $decoratedService = $definition->getDecoratedService()) { + list($decorated, $renamedId, $priority) = $decoratedService; $service->setAttribute('decorates', $decorated); + + $decorationOnInvalid = $decoratedService[3] ?? ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + if (\in_array($decorationOnInvalid, [ContainerInterface::IGNORE_ON_INVALID_REFERENCE, ContainerInterface::NULL_ON_INVALID_REFERENCE], true)) { + $invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE === $decorationOnInvalid ? 'null' : 'ignore'; + $service->setAttribute('decoration-on-invalid', $invalidBehavior); + } if (null !== $renamedId) { $service->setAttribute('decoration-inner-name', $renamedId); } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index 4eb68677f6..ccb68ee8f1 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -131,8 +131,8 @@ class YamlDumper extends Dumper $code .= " shared: false\n"; } - if (null !== $decorated = $definition->getDecoratedService()) { - list($decorated, $renamedId, $priority) = $decorated; + if (null !== $decoratedService = $definition->getDecoratedService()) { + list($decorated, $renamedId, $priority) = $decoratedService; $code .= sprintf(" decorates: %s\n", $decorated); if (null !== $renamedId) { $code .= sprintf(" decoration_inner_name: %s\n", $renamedId); @@ -140,6 +140,12 @@ class YamlDumper extends Dumper if (0 !== $priority) { $code .= sprintf(" decoration_priority: %s\n", $priority); } + + $decorationOnInvalid = $decoratedService[3] ?? ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + if (\in_array($decorationOnInvalid, [ContainerInterface::IGNORE_ON_INVALID_REFERENCE, ContainerInterface::NULL_ON_INVALID_REFERENCE])) { + $invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE === $decorationOnInvalid ? 'null' : 'ignore'; + $code .= sprintf(" decoration_on_invalid: %s\n", $invalidBehavior); + } } if ($callable = $definition->getFactory()) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/DecorateTrait.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/DecorateTrait.php index 809838f9ba..222cf75813 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/DecorateTrait.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/Traits/DecorateTrait.php @@ -11,6 +11,7 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator\Traits; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; trait DecorateTrait @@ -18,15 +19,18 @@ trait DecorateTrait /** * Sets the service that this service is decorating. * - * @param string|null $id The decorated service id, use null to remove decoration + * @param string|null $id The decorated service id, use null to remove decoration + * @param string|null $renamedId The new decorated service id + * @param int $priority The priority of decoration + * @param int $invalidBehavior The behavior to adopt when decorated is invalid * * @return $this * * @throws InvalidArgumentException in case the decorated service id and the new decorated service id are equals */ - final public function decorate(?string $id, string $renamedId = null, int $priority = 0): self + final public function decorate(?string $id, string $renamedId = null, int $priority = 0, int $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE): self { - $this->definition->setDecoratedService($id, $renamedId, $priority); + $this->definition->setDecoratedService($id, $renamedId, $priority, $invalidBehavior); return $this; } diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 25886d2f50..1e6392b50a 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -362,10 +362,22 @@ class XmlFileLoader extends FileLoader $definition->setBindings($bindings); } - if ($value = $service->getAttribute('decorates')) { + if ($decorates = $service->getAttribute('decorates')) { + $decorationOnInvalid = $service->getAttribute('decoration-on-invalid') ?: 'exception'; + if ('exception' === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + } elseif ('ignore' === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE; + } elseif ('null' === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE; + } else { + throw new InvalidArgumentException(sprintf('Invalid value "%s" for attribute "decoration-on-invalid" on service "%s". Did you mean "exception", "ignore" or "null" in "%s"?', $decorationOnInvalid, (string) $service->getAttribute('id'), $file)); + } + $renameId = $service->hasAttribute('decoration-inner-name') ? $service->getAttribute('decoration-inner-name') : null; $priority = $service->hasAttribute('decoration-priority') ? $service->getAttribute('decoration-priority') : 0; - $definition->setDecoratedService($value, $renameId, $priority); + + $definition->setDecoratedService($decorates, $renameId, $priority, $invalidBehavior); } return $definition; diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index b219fcc801..279778cf6f 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -58,6 +58,7 @@ class YamlFileLoader extends FileLoader 'decorates' => 'decorates', 'decoration_inner_name' => 'decoration_inner_name', 'decoration_priority' => 'decoration_priority', + 'decoration_on_invalid' => 'decoration_on_invalid', 'autowire' => 'autowire', 'autoconfigure' => 'autoconfigure', 'bind' => 'bind', @@ -538,14 +539,28 @@ class YamlFileLoader extends FileLoader $definition->addTag($name, $tag); } - if (isset($service['decorates'])) { - if ('' !== $service['decorates'] && '@' === $service['decorates'][0]) { - throw new InvalidArgumentException(sprintf('The value of the "decorates" option for the "%s" service must be the id of the service without the "@" prefix (replace "%s" with "%s").', $id, $service['decorates'], substr($service['decorates'], 1))); + if (null !== $decorates = $service['decorates'] ?? null) { + if ('' !== $decorates && '@' === $decorates[0]) { + throw new InvalidArgumentException(sprintf('The value of the "decorates" option for the "%s" service must be the id of the service without the "@" prefix (replace "%s" with "%s").', $id, $service['decorates'], substr($decorates, 1))); + } + + $decorationOnInvalid = \array_key_exists('decoration_on_invalid', $service) ? $service['decoration_on_invalid'] : 'exception'; + if ('exception' === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; + } elseif ('ignore' === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE; + } elseif (null === $decorationOnInvalid) { + $invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE; + } elseif ('null' === $decorationOnInvalid) { + throw new InvalidArgumentException(sprintf('Invalid value "%s" for attribute "decoration_on_invalid" on service "%s". Did you mean null (without quotes) in "%s"?', $decorationOnInvalid, $id, $file)); + } else { + throw new InvalidArgumentException(sprintf('Invalid value "%s" for attribute "decoration_on_invalid" on service "%s". Did you mean "exception", "ignore" or null in "%s"?', $decorationOnInvalid, $id, $file)); } $renameId = isset($service['decoration_inner_name']) ? $service['decoration_inner_name'] : null; $priority = isset($service['decoration_priority']) ? $service['decoration_priority'] : 0; - $definition->setDecoratedService($service['decorates'], $renameId, $priority); + + $definition->setDecoratedService($decorates, $renameId, $priority, $invalidBehavior); } if (isset($service['autowire'])) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd index b491196af7..345c68a227 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd +++ b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd @@ -129,6 +129,7 @@ + @@ -281,6 +282,14 @@ + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/DecoratorServicePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/DecoratorServicePassTest.php index d8d7587c8d..191f7c8691 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/DecoratorServicePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/DecoratorServicePassTest.php @@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; class DecoratorServicePassTest extends TestCase { @@ -125,6 +126,61 @@ class DecoratorServicePassTest extends TestCase $this->assertNull($quxDefinition->getDecoratedService()); } + public function testProcessWithInvalidDecorated() + { + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::IGNORE_ON_INVALID_REFERENCE) + ; + + $this->process($container); + $this->assertFalse($container->has('decorator')); + + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::NULL_ON_INVALID_REFERENCE) + ; + + $this->process($container); + $this->assertTrue($container->has('decorator')); + $this->assertSame(ContainerInterface::NULL_ON_INVALID_REFERENCE, $decoratorDefinition->decorationOnInvalid); + + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setDecoratedService('unknown_service') + ; + + $this->expectException('Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException'); + $this->process($container); + } + + public function testProcessNoInnerAliasWithInvalidDecorated() + { + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::NULL_ON_INVALID_REFERENCE) + ; + + $this->process($container); + $this->assertFalse($container->hasAlias('decorator.inner')); + } + + public function testProcessWithInvalidDecoratedAndWrongBehavior() + { + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setDecoratedService('unknown_decorated', null, 0, 12) + ; + + $this->expectException('Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException'); + $this->process($container); + } + public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinition() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInvalidReferencesPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInvalidReferencesPassTest.php index 817cc64ce3..fb5c7d62d2 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInvalidReferencesPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInvalidReferencesPassTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; +use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass; use Symfony\Component\DependencyInjection\Compiler\ResolveInvalidReferencesPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -127,6 +128,43 @@ class ResolveInvalidReferencesPassTest extends TestCase $this->assertSame([[[]]], $def->getArguments()); } + public function testProcessSetDecoratedAsNullOnInvalid() + { + $container = new ContainerBuilder(); + $decoratorDefinition = $container + ->register('decorator') + ->setArguments([ + new Reference('decorator.inner'), + ]) + ->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::NULL_ON_INVALID_REFERENCE) + ; + + (new DecoratorServicePass())->process($container); + (new ResolveInvalidReferencesPass())->process($container); + + $this->assertSame([null], $decoratorDefinition->getArguments()); + } + + public function testProcessSetOnlyDecoratedAsNullOnInvalid() + { + $container = new ContainerBuilder(); + $unknownArgument = new Reference('unknown_argument'); + $decoratorDefinition = $container + ->register('decorator') + ->setArguments([ + new Reference('decorator.inner'), + $unknownArgument, + ]) + ->setDecoratedService('unknown_decorated', null, 0, ContainerInterface::NULL_ON_INVALID_REFERENCE) + ; + + (new DecoratorServicePass())->process($container); + (new ResolveInvalidReferencesPass())->process($container); + + $this->assertSame(null, $decoratorDefinition->getArguments()[0]); + $this->assertEquals($unknownArgument, $decoratorDefinition->getArguments()[1]); + } + protected function process(ContainerBuilder $container) { $pass = new ResolveInvalidReferencesPass(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index 242dbab342..f67cdd520e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\Reference; @@ -86,6 +87,13 @@ class DefinitionTest extends TestCase public function testSetGetDecoratedService() { + $def = new Definition('stdClass'); + $this->assertNull($def->getDecoratedService()); + $def->setDecoratedService('foo', 'foo.renamed', 5, ContainerInterface::NULL_ON_INVALID_REFERENCE); + $this->assertEquals(['foo', 'foo.renamed', 5, ContainerInterface::NULL_ON_INVALID_REFERENCE], $def->getDecoratedService()); + $def->setDecoratedService(null); + $this->assertNull($def->getDecoratedService()); + $def = new Definition('stdClass'); $this->assertNull($def->getDecoratedService()); $def->setDecoratedService('foo', 'foo.renamed', 5); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php index 17f839fa56..4472782820 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php @@ -146,6 +146,16 @@ class XmlDumperTest extends TestCase ", include $fixturesPath.'/containers/container16.php'], + [" + + + + + + + + +", include $fixturesPath.'/containers/container34.php'], ]; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php index 93b9e85783..7a83b2e6a1 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php @@ -71,6 +71,13 @@ class YamlDumperTest extends TestCase $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services24.yml', $dumper->dump()); } + public function testDumpDecoratedServices() + { + $container = include self::$fixturesPath.'/containers/container34.php'; + $dumper = new YamlDumper($container); + $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services34.yml', $dumper->dump()); + } + public function testDumpLoad() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container34.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container34.php new file mode 100644 index 0000000000..dc97e8586b --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container34.php @@ -0,0 +1,12 @@ +register('decorator') + ->setDecoratedService('decorated', 'decorated.inner', 1, ContainerInterface::NULL_ON_INVALID_REFERENCE) +; + +return $container; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php index 3234234641..5dcfa4159e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php @@ -47,7 +47,7 @@ class Symfony_DI_PhpDumper_Test_Rot13Parameters extends Container public function getRemovedIds(): array { return [ - '.service_locator.GU08LT9' => true, + '.service_locator.ZZqL6HL' => true, 'Psr\\Container\\ContainerInterface' => true, 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, ]; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php index 5615be83cf..427fad6c98 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php @@ -48,7 +48,7 @@ class Symfony_DI_PhpDumper_Service_Locator_Argument extends Container public function getRemovedIds(): array { return [ - '.service_locator.38dy3OH' => true, + '.service_locator.iSxuxv5' => true, 'Psr\\Container\\ContainerInterface' => true, 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, 'foo2' => true, diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php index 7b25658742..e184fad7bd 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php @@ -45,8 +45,8 @@ class ProjectServiceContainer extends Container public function getRemovedIds(): array { return [ - '.service_locator.nZQiwdg' => true, - '.service_locator.nZQiwdg.foo_service' => true, + '.service_locator.bPEFRiK' => true, + '.service_locator.bPEFRiK.foo_service' => true, 'Psr\\Container\\ContainerInterface' => true, 'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true, 'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true, diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml index b39c388ca5..8ee6228882 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml @@ -47,6 +47,7 @@ + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_decoration_on_invalid_null.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_decoration_on_invalid_null.yml new file mode 100644 index 0000000000..1c1169662b --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_decoration_on_invalid_null.yml @@ -0,0 +1,7 @@ +services: + foo: + class: stdClass + bar: + class: stdClass + decorates: "foo" + decoration_on_invalid: 'null' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services34.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services34.yml new file mode 100644 index 0000000000..d95e320ac3 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services34.yml @@ -0,0 +1,17 @@ + +services: + service_container: + class: Symfony\Component\DependencyInjection\ContainerInterface + public: true + synthetic: true + decorator: + decorates: decorated + decoration_inner_name: decorated.inner + decoration_priority: 1 + decoration_on_invalid: null + Psr\Container\ContainerInterface: + alias: service_container + public: false + Symfony\Component\DependencyInjection\ContainerInterface: + alias: service_container + public: false diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index 2a82b181ea..2a2b59b954 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -30,6 +30,11 @@ services: decorates: decorated decoration_inner_name: decorated.pif-pouf decoration_priority: 5 + decorator_service_with_name_and_priority_and_on_invalid: + decorates: decorated + decoration_inner_name: decorated.pif-pouf + decoration_priority: 5 + decoration_on_invalid: ignore new_factory1: { class: FooBarClass, factory: factory} new_factory2: { class: FooBarClass, factory: ['@baz', getClass]} new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index b5ca7c919c..4b9e244d01 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -22,6 +22,7 @@ use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument; use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument; use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Dumper\PhpDumper; use Symfony\Component\DependencyInjection\Loader\IniFileLoader; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; @@ -281,6 +282,7 @@ class XmlFileLoaderTest extends TestCase $this->assertEquals(['decorated', null, 0], $services['decorator_service']->getDecoratedService()); $this->assertEquals(['decorated', 'decorated.pif-pouf', 0], $services['decorator_service_with_name']->getDecoratedService()); $this->assertEquals(['decorated', 'decorated.pif-pouf', 5], $services['decorator_service_with_name_and_priority']->getDecoratedService()); + $this->assertEquals(['decorated', 'decorated.pif-pouf', 5, ContainerInterface::IGNORE_ON_INVALID_REFERENCE], $services['decorator_service_with_name_and_priority_and_on_invalid']->getDecoratedService()); } public function testParsesIteratorArgument() diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index e01f41d139..4e3a2bb38b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -22,6 +22,7 @@ use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument; use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument; use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Loader\IniFileLoader; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; @@ -174,6 +175,7 @@ class YamlFileLoaderTest extends TestCase $this->assertEquals(['decorated', null, 0], $services['decorator_service']->getDecoratedService()); $this->assertEquals(['decorated', 'decorated.pif-pouf', 0], $services['decorator_service_with_name']->getDecoratedService()); $this->assertEquals(['decorated', 'decorated.pif-pouf', 5], $services['decorator_service_with_name_and_priority']->getDecoratedService()); + $this->assertEquals(['decorated', 'decorated.pif-pouf', 5, ContainerInterface::IGNORE_ON_INVALID_REFERENCE], $services['decorator_service_with_name_and_priority_and_on_invalid']->getDecoratedService()); } public function testDeprecatedAliases() @@ -578,6 +580,14 @@ class YamlFileLoaderTest extends TestCase $loader->load('bad_decorates.yml'); } + public function testDecoratedServicesWithWrongOnInvalidSyntaxThrowsException() + { + $this->expectException('Symfony\Component\DependencyInjection\Exception\InvalidArgumentException'); + $this->expectExceptionMessage('Did you mean null (without quotes)'); + $loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('bad_decoration_on_invalid_null.yml'); + } + public function testInvalidTagsWithDefaults() { $this->expectException('Symfony\Component\DependencyInjection\Exception\InvalidArgumentException');