From c65b611995393573fb12ebbc8f0bc7c31ad67a64 Mon Sep 17 00:00:00 2001 From: HypeMC Date: Wed, 7 Jun 2023 22:02:05 +0200 Subject: [PATCH 1/2] [DependencyInjection] Don't ignore attributes on the actual decorator --- Compiler/ResolveInstanceofConditionalsPass.php | 7 ++++--- .../Compiler/ResolveInstanceofConditionalsPassTest.php | 10 +++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Compiler/ResolveInstanceofConditionalsPass.php b/Compiler/ResolveInstanceofConditionalsPass.php index b211b84e1..0a55d15d5 100644 --- a/Compiler/ResolveInstanceofConditionalsPass.php +++ b/Compiler/ResolveInstanceofConditionalsPass.php @@ -87,7 +87,7 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi $instanceofDef->setAbstract(true)->setParent($parent ?: '.abstract.instanceof.'.$id); $parent = '.instanceof.'.$interface.'.'.$key.'.'.$id; $container->setDefinition($parent, $instanceofDef); - $instanceofTags[] = $instanceofDef->getTags(); + $instanceofTags[] = [$interface, $instanceofDef->getTags()]; $instanceofBindings = $instanceofDef->getBindings() + $instanceofBindings; foreach ($instanceofDef->getMethodCalls() as $methodCall) { @@ -126,8 +126,9 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi // Don't add tags to service decorators $i = \count($instanceofTags); while (0 <= --$i) { - foreach ($instanceofTags[$i] as $k => $v) { - if (null === $definition->getDecoratedService() || \in_array($k, $tagsToKeep, true)) { + [$interface, $tags] = $instanceofTags[$i]; + foreach ($tags as $k => $v) { + if (null === $definition->getDecoratedService() || $interface === $definition->getClass() || \in_array($k, $tagsToKeep, true)) { foreach ($v as $v) { if ($definition->hasTag($k) && \in_array($v, $definition->getTag($k))) { continue; diff --git a/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index b7ea0f4ac..cdbf7a318 100644 --- a/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -318,6 +318,7 @@ public function testDecoratorsAreNotAutomaticallyTagged() $decorator->setDecoratedService('decorated'); $decorator->setInstanceofConditionals([ parent::class => (new ChildDefinition(''))->addTag('tag'), + self::class => (new ChildDefinition(''))->addTag('other-tag'), ]); $decorator->setAutoconfigured(true); $decorator->addTag('manual'); @@ -325,11 +326,18 @@ public function testDecoratorsAreNotAutomaticallyTagged() $container->registerForAutoconfiguration(parent::class) ->addTag('tag') ; + $container->registerForAutoconfiguration(self::class) + ->addTag('last-tag') + ; (new ResolveInstanceofConditionalsPass())->process($container); (new ResolveChildDefinitionsPass())->process($container); - $this->assertSame(['manual' => [[]]], $container->getDefinition('decorator')->getTags()); + $this->assertSame([ + 'manual' => [[]], + 'other-tag' => [[]], + 'last-tag' => [[]], + ], $container->getDefinition('decorator')->getTags()); } public function testDecoratorsKeepBehaviorDescribingTags() From 814914327d8baa9df6deed290069139bcbf8b9e2 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 6 Jul 2023 17:21:26 +0200 Subject: [PATCH 2/2] [DepdencyInjection] Fix costly logic when checking errored definitions --- Compiler/DefinitionErrorExceptionPass.php | 68 +++++++++-------------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/Compiler/DefinitionErrorExceptionPass.php b/Compiler/DefinitionErrorExceptionPass.php index aabc5b71d..cb0c58894 100644 --- a/Compiler/DefinitionErrorExceptionPass.php +++ b/Compiler/DefinitionErrorExceptionPass.php @@ -26,7 +26,6 @@ class DefinitionErrorExceptionPass extends AbstractRecursivePass { private $erroredDefinitions = []; - private $targetReferences = []; private $sourceReferences = []; /** @@ -37,45 +36,10 @@ public function process(ContainerBuilder $container) try { parent::process($container); - if (!$this->erroredDefinitions) { - return; - } - - $runtimeIds = []; - - foreach ($this->sourceReferences as $id => $sourceIds) { - foreach ($sourceIds as $sourceId => $isRuntime) { - if (!$isRuntime) { - continue 2; - } - } - - unset($this->erroredDefinitions[$id]); - $runtimeIds[$id] = $id; - } - - if (!$this->erroredDefinitions) { - return; - } - - foreach ($this->targetReferences as $id => $targetIds) { - if (!isset($this->sourceReferences[$id]) || isset($runtimeIds[$id]) || isset($this->erroredDefinitions[$id])) { - continue; - } - foreach ($this->targetReferences[$id] as $targetId => $isRuntime) { - foreach ($this->sourceReferences[$id] as $sourceId => $isRuntime) { - if ($sourceId !== $targetId) { - $this->sourceReferences[$targetId][$sourceId] = false; - $this->targetReferences[$sourceId][$targetId] = false; - } - } - } - - unset($this->sourceReferences[$id]); - } + $visitedIds = []; foreach ($this->erroredDefinitions as $id => $definition) { - if (isset($this->sourceReferences[$id])) { + if ($this->isErrorForRuntime($id, $visitedIds)) { continue; } @@ -86,7 +50,6 @@ public function process(ContainerBuilder $container) } } finally { $this->erroredDefinitions = []; - $this->targetReferences = []; $this->sourceReferences = []; } } @@ -105,10 +68,8 @@ protected function processValue($value, bool $isRoot = false) if ($value instanceof Reference && $this->currentId !== $targetId = (string) $value) { if (ContainerInterface::RUNTIME_EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) { $this->sourceReferences[$targetId][$this->currentId] ?? $this->sourceReferences[$targetId][$this->currentId] = true; - $this->targetReferences[$this->currentId][$targetId] ?? $this->targetReferences[$this->currentId][$targetId] = true; } else { $this->sourceReferences[$targetId][$this->currentId] = false; - $this->targetReferences[$this->currentId][$targetId] = false; } return $value; @@ -122,4 +83,29 @@ protected function processValue($value, bool $isRoot = false) return parent::processValue($value); } + + private function isErrorForRuntime(string $id, array &$visitedIds): bool + { + if (!isset($this->sourceReferences[$id])) { + return false; + } + + if (isset($visitedIds[$id])) { + return $visitedIds[$id]; + } + + $visitedIds[$id] = true; + + foreach ($this->sourceReferences[$id] as $sourceId => $isRuntime) { + if ($visitedIds[$sourceId] ?? $visitedIds[$sourceId] = $this->isErrorForRuntime($sourceId, $visitedIds)) { + continue; + } + + if (!$isRuntime) { + return false; + } + } + + return true; + } }