[DI] default to service id - *not* FQCN - when building tagged locators

symfony · fabpot · May 13, 2019 · May 10, 2019 · May 11, 2019 · 52e827c9cf1a087bd12c94414ac4836278fa1a74
commit 52e827c9cf1a087bd12c94414ac4836278fa1a74
diff --git 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 @@ public function getDefaultIndexMethod(): ?string
        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
@@ -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 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
        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 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
            $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
@@ -292,22 +292,22 @@ public function testTaggedServiceWithIndexAttributeAndDefaultMethod()
    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 @@ public function testTaggedServiceLocatorWithIndexAttribute()
            '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 @@ public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
            '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($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([BarTagClass::class => $container->get(BarTagClass::class)], $same);
+        $this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]);
    }
 }