From eb25d5c275a1e3f913587e91b349cfb8e04bf0b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malte=20Schlu=CC=88ter?= Date: Fri, 27 Nov 2020 13:55:13 +0100 Subject: [PATCH] [DI] The default index method wasn't used if the "index_by" attribute is missing --- .../Argument/TaggedIteratorArgument.php | 4 +- .../Compiler/PriorityTaggedServiceTrait.php | 23 +++++--- .../PriorityTaggedServiceTraitTest.php | 52 +++++++++++++++++++ .../FooTaggedForInvalidDefaultMethodClass.php | 21 ++++++++ 4 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php index d1d5f6d8a4..1ba8de790b 100644 --- a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php +++ b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php @@ -41,9 +41,9 @@ class TaggedIteratorArgument extends IteratorArgument $this->tag = $tag; $this->indexAttribute = $indexAttribute; - $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name'); + $this->defaultIndexMethod = $defaultIndexMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Name' : null); $this->needsIndexes = $needsIndexes; - $this->defaultPriorityMethod = $defaultPriorityMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Priority'); + $this->defaultPriorityMethod = $defaultPriorityMethod ?: ($indexAttribute ? 'getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Priority' : null); } public function getTag() diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php index caa2e03e71..f558927e48 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php @@ -46,7 +46,7 @@ trait PriorityTaggedServiceTrait $indexAttribute = $tagName->getIndexAttribute(); $defaultIndexMethod = $tagName->getDefaultIndexMethod(); $needsIndexes = $tagName->needsIndexes(); - $defaultPriorityMethod = $tagName->getDefaultPriorityMethod(); + $defaultPriorityMethod = $tagName->getDefaultPriorityMethod() ?? 'getDefaultPriority'; $tagName = $tagName->getTag(); } @@ -69,15 +69,15 @@ trait PriorityTaggedServiceTrait } $priority = $priority ?? $defaultPriority ?? $defaultPriority = 0; - if (null === $indexAttribute && !$needsIndexes) { + if (null === $indexAttribute && !$defaultIndexMethod && !$needsIndexes) { $services[] = [$priority, ++$i, null, $serviceId, null]; continue 2; } if (null !== $indexAttribute && isset($attribute[$indexAttribute])) { $index = $attribute[$indexAttribute]; - } elseif (null === $defaultIndex && $defaultIndexMethod && $class) { - $defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod, $tagName, $indexAttribute); + } elseif (null === $defaultIndex && $defaultPriorityMethod && $class) { + $defaultIndex = PriorityTaggedServiceUtil::getDefaultIndex($container, $serviceId, $class, $defaultIndexMethod ?? 'getDefaultName', $tagName, $indexAttribute); } $index = $index ?? $defaultIndex ?? $defaultIndex = $serviceId; @@ -116,24 +116,31 @@ class PriorityTaggedServiceUtil /** * Gets the index defined by the default index method. */ - public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, string $indexAttribute): ?string + public static function getDefaultIndex(ContainerBuilder $container, string $serviceId, string $class, string $defaultIndexMethod, string $tagName, ?string $indexAttribute): ?string { if (!($r = $container->getReflectionClass($class)) || !$r->hasMethod($defaultIndexMethod)) { return null; } + if (null !== $indexAttribute) { + $service = $class !== $serviceId ? sprintf('service "%s"', $serviceId) : 'on the corresponding service'; + $message = [sprintf('Either method "%s::%s()" should ', $class, $defaultIndexMethod), sprintf(' or tag "%s" on %s is missing attribute "%s".', $tagName, $service, $indexAttribute)]; + } else { + $message = [sprintf('Method "%s::%s()" should ', $class, $defaultIndexMethod), '.']; + } + 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)); + throw new InvalidArgumentException(implode('be static', $message)); } 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)); + throw new InvalidArgumentException(implode('be public', $message)); } $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)); + throw new InvalidArgumentException(implode(sprintf('return a string (got "%s")', \gettype($defaultIndex)), $message)); } return $defaultIndex; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php index 3feafac3d3..2674ea4dde 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/PriorityTaggedServiceTraitTest.php @@ -15,8 +15,11 @@ 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\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass; +use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTagClass; +use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTaggedForInvalidDefaultMethodClass; use Symfony\Component\DependencyInjection\TypedReference; class PriorityTaggedServiceTraitTest extends TestCase @@ -132,6 +135,55 @@ class PriorityTaggedServiceTraitTest extends TestCase $this->assertSame(array_keys($expected), array_keys($services)); $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container)); } + + public function testTheIndexedTagsByDefaultIndexMethod() + { + $container = new ContainerBuilder(); + $container->register('service1', FooTagClass::class)->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(); + + $tag = new TaggedIteratorArgument('my_custom_tag', 'foo', 'getFooBar'); + $expected = [ + 'bar_tab_class_with_defaultmethod' => new TypedReference('service2', BarTagClass::class), + 'service1' => new TypedReference('service1', FooTagClass::class), + ]; + $services = $priorityTaggedServiceTraitImplementation->test($tag, $container); + $this->assertSame(array_keys($expected), array_keys($services)); + $this->assertEquals($expected, $priorityTaggedServiceTraitImplementation->test($tag, $container)); + } + + /** + * @dataProvider provideInvalidDefaultMethods + */ + public function testTheIndexedTagsByDefaultIndexMethodFailure(string $defaultIndexMethod, ?string $indexAttribute, string $expectedExceptionMessage) + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + $container = new ContainerBuilder(); + + $container->register('service1', FooTaggedForInvalidDefaultMethodClass::class)->addTag('my_custom_tag'); + + $priorityTaggedServiceTraitImplementation = new PriorityTaggedServiceTraitImplementation(); + + $tag = new TaggedIteratorArgument('my_custom_tag', $indexAttribute, $defaultIndexMethod); + $priorityTaggedServiceTraitImplementation->test($tag, $container); + } + + public function provideInvalidDefaultMethods(): iterable + { + yield ['getMethodShouldBeStatic', null, sprintf('Method "%s::getMethodShouldBeStatic()" should be static.', FooTaggedForInvalidDefaultMethodClass::class)]; + yield ['getMethodShouldBeStatic', 'foo', sprintf('Either method "%s::getMethodShouldBeStatic()" should be static or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)]; + yield ['getMethodShouldBePublicInsteadProtected', null, sprintf('Method "%s::getMethodShouldBePublicInsteadProtected()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)]; + yield ['getMethodShouldBePublicInsteadProtected', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadProtected()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)]; + yield ['getMethodShouldBePublicInsteadPrivate', null, sprintf('Method "%s::getMethodShouldBePublicInsteadPrivate()" should be public.', FooTaggedForInvalidDefaultMethodClass::class)]; + yield ['getMethodShouldBePublicInsteadPrivate', 'foo', sprintf('Either method "%s::getMethodShouldBePublicInsteadPrivate()" should be public or tag "my_custom_tag" on service "service1" is missing attribute "foo".', FooTaggedForInvalidDefaultMethodClass::class)]; + } } class PriorityTaggedServiceTraitImplementation diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php new file mode 100644 index 0000000000..3f4ec4b297 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTaggedForInvalidDefaultMethodClass.php @@ -0,0 +1,21 @@ +