From 09770aa93010ae2329e66b91a41f9705919575a0 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 3 Mar 2020 18:05:02 +0100 Subject: [PATCH] [DI] ignore extra tags added by autoconfiguration in PriorityTaggedServiceTrait --- .../Compiler/PriorityTaggedServiceTrait.php | 137 ++++++++++-------- .../PriorityTaggedServiceTraitTest.php | 46 ++++++ 2 files changed, 125 insertions(+), 58 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php index 1a2c829854dc..121b1dc71602 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php @@ -15,7 +15,6 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\DependencyInjection\TypedReference; /** * Trait that allows a generic method to find and sort service by priority option in the tag. @@ -50,11 +49,10 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container $tagName = $tagName->getTag(); } + $i = 0; $services = []; foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) { - $class = $r = null; - $defaultPriority = null; $defaultIndex = null; @@ -64,78 +62,101 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container if (isset($attribute['priority'])) { $priority = $attribute['priority']; } elseif (null === $defaultPriority && $defaultPriorityMethod) { - $class = $container->getDefinition($serviceId)->getClass(); - $class = $container->getParameterBag()->resolveValue($class) ?: null; - - if (($r = ($r ?? $container->getReflectionClass($class))) && $r->hasMethod($defaultPriorityMethod)) { - if (!($rm = $r->getMethod($defaultPriorityMethod))->isStatic()) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be static: tag "%s" on service "%s".', $class, $defaultPriorityMethod, $tagName, $serviceId)); - } - - if (!$rm->isPublic()) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be public: tag "%s" on service "%s".', $class, $defaultPriorityMethod, $tagName, $serviceId)); - } - - $defaultPriority = $rm->invoke(null); - - if (!\is_int($defaultPriority)) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should return an integer, got %s: tag "%s" on service "%s".', $class, $defaultPriorityMethod, \gettype($priority), $tagName, $serviceId)); - } - } + $defaultPriority = PriorityTaggedServiceUtil::getDefaultPriority($container, $serviceId, $defaultPriorityMethod, $tagName); } + $priority = $priority ?? $defaultPriority ?? $defaultPriority = 0; - $priority = $priority ?? $defaultPriority ?? 0; + if (null === $indexAttribute && !$needsIndexes) { + $services[] = [$priority, ++$i, null, $serviceId]; + continue 2; + } if (null !== $indexAttribute && isset($attribute[$indexAttribute])) { $index = $attribute[$indexAttribute]; - } elseif (null === $defaultIndex && null === $indexAttribute && !$needsIndexes) { - // With partially associative array, insertion to get next key is simpler. - $services[$priority][] = null; - end($services[$priority]); - $defaultIndex = key($services[$priority]); } elseif (null === $defaultIndex && $defaultIndexMethod) { - $class = $container->getDefinition($serviceId)->getClass(); - $class = $container->getParameterBag()->resolveValue($class) ?: null; + $defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $defaultIndexMethod, $tagName, $indexAttribute); + } + $index = $index ?? $defaultIndex ?? $defaultIndex = $serviceId; - if (($r = ($r ?? $container->getReflectionClass($class))) && $r->hasMethod($defaultIndexMethod)) { - if (!($rm = $r->getMethod($defaultIndexMethod))->isStatic()) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be static: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute)); - } + $services[] = [$priority, ++$i, $index, $serviceId]; + } + } - if (!$rm->isPublic()) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be public: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute)); - } + uasort($services, static function ($a, $b) { return $b[0] <=> $a[0] ?: $a[1] <=> $b[1]; }); - $defaultIndex = $rm->invoke(null); + $refs = []; + foreach ($services as [, , $index, $serviceId]) { + if (null === $index) { + $refs[] = new Reference($serviceId); + } else { + $refs[$index] = new Reference($serviceId); + } + } - if (!\is_string($defaultIndex)) { - throw new InvalidArgumentException(sprintf('Method "%s::%s()" should return a string, got %s: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, \gettype($defaultIndex), $tagName, $serviceId, $indexAttribute)); - } - } + return $refs; + } +} - $defaultIndex = $defaultIndex ?? $serviceId; - } +/** + * @internal + */ +class PriorityTaggedServiceUtil +{ + /** + * Gets the index defined by the default index method. + */ + public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $defaultIndexMethod, string $tagName, string $indexAttribute): ?string + { + $class = $container->getDefinition($serviceId)->getClass(); + $class = $container->getParameterBag()->resolveValue($class) ?: null; - $index = $index ?? $defaultIndex; + if (!($r = $container->getReflectionClass($class)) || !$r->hasMethod($defaultIndexMethod)) { + return null; + } - $reference = null; - if (!$class || 'stdClass' === $class) { - $reference = new Reference($serviceId); - } elseif ($index === $serviceId) { - $reference = new TypedReference($serviceId, $class); - } else { - $reference = new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, \is_string($index) ? $index : null); - } + if (!($rm = $r->getMethod($defaultIndexMethod))->isStatic()) { + throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be static or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute)); + } - $services[$priority][$index] = $reference; - } + if (!$rm->isPublic()) { + throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be public or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute)); + } + + $defaultIndex = $rm->invoke(null); + + if (!\is_string($defaultIndex)) { + throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should return a string (got %s) or tag "%s" on service "%s" is missing attribute "%s".', $class, $defaultIndexMethod, \gettype($defaultIndex), $tagName, $serviceId, $indexAttribute)); + } + + return $defaultIndex; + } + + /** + * Gets the priority defined by the default priority method. + */ + public static function getDefaultPriority(ContainerBuilder $container, string $serviceId, string $defaultPriorityMethod, string $tagName): ?int + { + $class = $container->getDefinition($serviceId)->getClass(); + $class = $container->getParameterBag()->resolveValue($class) ?: null; + + if (!($r = $container->getReflectionClass($class)) || !$r->hasMethod($defaultPriorityMethod)) { + return null; } - if ($services) { - krsort($services); - $services = array_merge(...$services); + if (!($rm = $r->getMethod($defaultPriorityMethod))->isStatic()) { + throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be static or tag "%s" on service "%s" is missing attribute "priority".', $class, $defaultPriorityMethod, $tagName, $serviceId)); + } + + if (!$rm->isPublic()) { + throw new InvalidArgumentException(sprintf('Either method "%s::%s()" should be public or tag "%s" on service "%s" is missing attribute "priority".', $class, $defaultPriorityMethod, $tagName, $serviceId)); + } + + $defaultPriority = $rm->invoke(null); + + if (!\is_int($defaultPriority)) { + throw new InvalidArgumentException(sprintf('Method "%s::%s()" should return an integer (got %s) or tag "%s" on service "%s" is missing attribute "priority".', $class, $defaultPriorityMethod, \gettype($defaultPriority), $tagName, $serviceId)); } - return $services; + return $defaultPriority; } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php index 57ca9af42712..25ac59784452 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php @@ -12,9 +12,11 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument; use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass; class PriorityTaggedServiceTraitTest extends TestCase { @@ -85,6 +87,50 @@ public function testWithEmptyArray() $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation(); $this->assertEquals([], $priorityTaggedServiceTraitImplementation->test('my_custom_tag', $container)); } + + public function testOnlyTheFirstNonIndexedTagIsListed() + { + $container = new ContainerBuilder(); + $container->register('service1')->addTag('my_custom_tag'); + + $definition = $container->register('service2', BarTagClass::class); + $definition->addTag('my_custom_tag', ['priority' => 100]); + $definition->addTag('my_custom_tag', []); + + $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation(); + + $expected = [ + new Reference('service2'), + new Reference('service1'), + ]; + $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test('my_custom_tag', $container)); + } + + public function testOnlyTheIndexedTagsAreListed() + { + $container = new ContainerBuilder(); + $container->register('service1')->addTag('my_custom_tag', ['foo' => 'bar']); + + $definition = $container->register('service2', BarTagClass::class); + $definition->addTag('my_custom_tag', ['priority' => 100]); + $definition->addTag('my_custom_tag', ['foo' => 'a']); + $definition->addTag('my_custom_tag', ['foo' => 'b', 'priority' => 100]); + $definition->addTag('my_custom_tag', ['foo' => 'b']); + $definition->addTag('my_custom_tag', []); + + $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation(); + + $tag = new TaggedIteratorArgument('my_custom_tag', 'foo'); + $expected = [ + 'bar_tag_class' => new Reference('service2'), + 'b' => new Reference('service2'), + 'bar' => new Reference('service1'), + 'a' => new Reference('service2'), + ]; + $services = $priorityTaggedServiceTraitImplementation->test($tag, $container); + $this->assertSame(array_keys($expected), array_keys($services)); + $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container)); + } } class PriorityTaggedServiceTraitImplementation