From 52e827c9cf1a087bd12c94414ac4836278fa1a74 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 10 May 2019 12:19:34 +0200 Subject: [PATCH] [DI] default to service id - *not* FQCN - when building tagged locators --- .../Argument/TaggedIteratorArgument.php | 16 +++-- .../Compiler/PriorityTaggedServiceTrait.php | 10 ++-- .../Tests/Compiler/IntegrationTest.php | 60 +++++++++++++------ 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php index f3b323ce4c..c88b174169 100644 --- a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php +++ b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php @@ -21,22 +21,26 @@ class TaggedIteratorArgument extends IteratorArgument private $tag; private $indexAttribute; private $defaultIndexMethod; - private $useFqcnAsFallback = false; + private $needsIndexes = false; /** * @param string $tag The name of the tag identifying the target services * @param string|null $indexAttribute The name of the attribute that defines the key referencing each service in the tagged collection * @param string|null $defaultIndexMethod The static method that should be called to get each service's key when their tag doesn't define the previous attribute - * @param bool $useFqcnAsFallback Whether the FQCN of the service should be used as index when neither the attribute nor the method are defined + * @param bool $needsIndexes Whether indexes are required and should be generated when computing the map */ - public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $useFqcnAsFallback = false) + public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false) { parent::__construct([]); + if (null === $indexAttribute && $needsIndexes) { + $indexAttribute = preg_match('/[^.]++$/', $tag, $m) ? $m[0] : $tag; + } + $this->tag = $tag; $this->indexAttribute = $indexAttribute; $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name'); - $this->useFqcnAsFallback = $useFqcnAsFallback; + $this->needsIndexes = $needsIndexes; } public function getTag() @@ -54,8 +58,8 @@ class TaggedIteratorArgument extends IteratorArgument return $this->defaultIndexMethod; } - public function useFqcnAsFallback(): bool + public function needsIndexes(): bool { - return $this->useFqcnAsFallback; + return $this->needsIndexes; } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php index 3bf8e3be18..5f04eadef8 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php @@ -41,12 +41,12 @@ trait PriorityTaggedServiceTrait */ private function findAndSortTaggedServices($tagName, ContainerBuilder $container) { - $indexAttribute = $defaultIndexMethod = $useFqcnAsFallback = null; + $indexAttribute = $defaultIndexMethod = $needsIndexes = null; if ($tagName instanceof TaggedIteratorArgument) { $indexAttribute = $tagName->getIndexAttribute(); $defaultIndexMethod = $tagName->getDefaultIndexMethod(); - $useFqcnAsFallback = $tagName->useFqcnAsFallback(); + $needsIndexes = $tagName->needsIndexes(); $tagName = $tagName->getTag(); } @@ -55,7 +55,7 @@ trait PriorityTaggedServiceTrait foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) { $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0; - if (null === $indexAttribute && !$useFqcnAsFallback) { + if (null === $indexAttribute && !$needsIndexes) { $services[$priority][] = new Reference($serviceId); continue; @@ -77,8 +77,8 @@ trait PriorityTaggedServiceTrait $class = $r->name; if (!$r->hasMethod($defaultIndexMethod)) { - if ($useFqcnAsFallback) { - $services[$priority][$class] = new TypedReference($serviceId, $class); + if ($needsIndexes) { + $services[$priority][$serviceId] = new TypedReference($serviceId, $class); continue; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php index feeb163edb..83d3e23d3b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php @@ -292,22 +292,22 @@ class IntegrationTest extends TestCase public function testTaggedServiceLocatorWithIndexAttribute() { $container = new ContainerBuilder(); - $container->register(BarTagClass::class) + $container->register('bar_tag', BarTagClass::class) ->setPublic(true) ->addTag('foo_bar', ['foo' => 'bar']) ; - $container->register(FooTagClass::class) + $container->register('foo_tag', FooTagClass::class) ->setPublic(true) ->addTag('foo_bar') ; - $container->register(FooBarTaggedClass::class) + $container->register('foo_bar_tagged', FooBarTaggedClass::class) ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo'))) ->setPublic(true) ; $container->compile(); - $s = $container->get(FooBarTaggedClass::class); + $s = $container->get('foo_bar_tagged'); /** @var ServiceLocator $serviceLocator */ $serviceLocator = $s->getParam(); @@ -317,28 +317,28 @@ class IntegrationTest extends TestCase 'bar' => $serviceLocator->get('bar'), 'foo_tag_class' => $serviceLocator->get('foo_tag_class'), ]; - $this->assertSame(['bar' => $container->get(BarTagClass::class), 'foo_tag_class' => $container->get(FooTagClass::class)], $same); + $this->assertSame(['bar' => $container->get('bar_tag'), 'foo_tag_class' => $container->get('foo_tag')], $same); } public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod() { $container = new ContainerBuilder(); - $container->register(BarTagClass::class) + $container->register('bar_tag', BarTagClass::class) ->setPublic(true) ->addTag('foo_bar') ; - $container->register(FooTagClass::class) + $container->register('foo_tag', FooTagClass::class) ->setPublic(true) ->addTag('foo_bar', ['foo' => 'foo']) ; - $container->register(FooBarTaggedClass::class) + $container->register('foo_bar_tagged', FooBarTaggedClass::class) ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar'))) ->setPublic(true) ; $container->compile(); - $s = $container->get(FooBarTaggedClass::class); + $s = $container->get('foo_bar_tagged'); /** @var ServiceLocator $serviceLocator */ $serviceLocator = $s->getParam(); @@ -348,33 +348,59 @@ class IntegrationTest extends TestCase 'bar_tab_class_with_defaultmethod' => $serviceLocator->get('bar_tab_class_with_defaultmethod'), 'foo' => $serviceLocator->get('foo'), ]; - $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get(BarTagClass::class), 'foo' => $container->get(FooTagClass::class)], $same); + $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get('bar_tag'), 'foo' => $container->get('foo_tag')], $same); } - public function testTaggedServiceLocatorWithFqcnFallback() + public function testTaggedServiceLocatorWithFallback() { $container = new ContainerBuilder(); - $container->register(BarTagClass::class) + $container->register('bar_tag', BarTagClass::class) ->setPublic(true) ->addTag('foo_bar') ; - $container->register(FooBarTaggedClass::class) + $container->register('foo_bar_tagged', FooBarTaggedClass::class) ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', null, null, true))) ->setPublic(true) ; $container->compile(); - $s = $container->get(FooBarTaggedClass::class); + $s = $container->get('foo_bar_tagged'); /** @var ServiceLocator $serviceLocator */ $serviceLocator = $s->getParam(); $this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator))); - $same = [ - BarTagClass::class => $serviceLocator->get(BarTagClass::class), + $expected = [ + 'bar_tag' => $container->get('bar_tag'), ]; - $this->assertSame([BarTagClass::class => $container->get(BarTagClass::class)], $same); + $this->assertSame($expected, ['bar_tag' => $serviceLocator->get('bar_tag')]); + } + + public function testTaggedServiceLocatorWithDefaultIndex() + { + $container = new ContainerBuilder(); + $container->register('bar_tag', BarTagClass::class) + ->setPublic(true) + ->addTag('app.foo_bar', ['foo_bar' => 'baz']) + ; + $container->register('foo_bar_tagged', FooBarTaggedClass::class) + ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('app.foo_bar', null, null, true))) + ->setPublic(true) + ; + + $container->compile(); + + $s = $container->get('foo_bar_tagged'); + + /** @var ServiceLocator $serviceLocator */ + $serviceLocator = $s->getParam(); + $this->assertTrue($s->getParam() instanceof ServiceLocator, sprintf('Wrong instance, should be an instance of ServiceLocator, %s given', \is_object($serviceLocator) ? \get_class($serviceLocator) : \gettype($serviceLocator))); + + $expected = [ + 'baz' => $container->get('bar_tag'), + ]; + $this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]); } }