From 35a40ace6f5a90d79e7dd707bf156c16da9801a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Bogusz?= Date: Mon, 21 Jan 2019 01:28:28 +0100 Subject: [PATCH] [DI] Fixes: #28326 - Overriding services autowired by name under _defaults bind not working --- .../Compiler/ResolveBindingsPass.php | 2 ++ .../DependencyInjection/ContainerBuilder.php | 31 +++++++++++++++++++ .../Configurator/ServiceConfigurator.php | 2 ++ .../DependencyInjection/Loader/FileLoader.php | 2 ++ .../Tests/Fixtures/yaml/defaults_bindings.yml | 11 +++++++ .../Fixtures/yaml/defaults_bindings2.yml | 7 +++++ .../Tests/Loader/YamlFileLoaderTest.php | 17 ++++++++++ 7 files changed, 72 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php index 0fad285e5a..a82d4163c6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php @@ -34,6 +34,8 @@ class ResolveBindingsPass extends AbstractRecursivePass */ public function process(ContainerBuilder $container) { + $this->usedBindings = $container->getRemovedBindingIds(); + try { parent::process($container); diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 0c1966a42e..72307464a7 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -123,6 +123,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface private $removedIds = []; + private $removedBindingIds = []; + private static $internalTypes = [ 'int' => true, 'float' => true, @@ -1504,6 +1506,35 @@ class ContainerBuilder extends Container implements TaggedContainerInterface return isset($this->definitions[$id]) || isset($this->aliasDefinitions[$id]) || isset($this->removedIds[$id]) ? $id : parent::normalizeId($id); } + /** + * Gets removed binding ids. + * + * @return array + * + * @internal + */ + public function getRemovedBindingIds() + { + return $this->removedBindingIds; + } + + /** + * Adds a removed binding id. + * + * @param int $id + * + * @internal + */ + public function addRemovedBindingIds($id) + { + if ($this->hasDefinition($id)) { + foreach ($this->getDefinition($id)->getBindings() as $key => $binding) { + list(, $bindingId) = $binding->getValues(); + $this->removedBindingIds[(int) $bindingId] = true; + } + } + } + /** * Returns the Service Conditionals. * diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php index 12054cad0c..21b1669d09 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php @@ -59,6 +59,8 @@ class ServiceConfigurator extends AbstractServiceConfigurator { parent::__destruct(); + $this->container->addRemovedBindingIds($this->id); + if (!$this->definition instanceof ChildDefinition) { $this->container->setDefinition($this->id, $this->definition->setInstanceofConditionals($this->instanceof)); } else { diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php index a428c6e35f..0dd1a3d8bd 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php @@ -91,6 +91,8 @@ abstract class FileLoader extends BaseFileLoader */ protected function setDefinition($id, Definition $definition) { + $this->container->addRemovedBindingIds($id); + if ($this->isLoadingInstanceof) { if (!$definition instanceof ChildDefinition) { throw new InvalidArgumentException(sprintf('Invalid type definition "%s": ChildDefinition expected, "%s" given.', $id, \get_class($definition))); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml new file mode 100644 index 0000000000..2e054f563d --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings.yml @@ -0,0 +1,11 @@ +services: + _defaults: + bind: + $quz: value + $foo: [value] + + bar: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\Bar + + foo: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml new file mode 100644 index 0000000000..75e9834526 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/defaults_bindings2.yml @@ -0,0 +1,7 @@ +services: + _defaults: + bind: + $quz: ~ + + bar: + class: Symfony\Component\DependencyInjection\Tests\Fixtures\Bar diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 7bcf0ec057..001fa33e59 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -17,6 +17,7 @@ use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\Config\Resource\GlobResource; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; +use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Loader\IniFileLoader; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; @@ -732,4 +733,20 @@ class YamlFileLoaderTest extends TestCase '$factory' => 'factory', ], array_map(function ($v) { return $v->getValues()[0]; }, $definition->getBindings())); } + + /** + * The pass may throw an exception, which will cause the test to fail. + */ + public function testOverriddenDefaultsBindings() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('defaults_bindings.yml'); + $loader->load('defaults_bindings2.yml'); + + (new ResolveBindingsPass())->process($container); + + $this->assertTrue(true); + } }