bug #35342 [DI] Fix support for multiple tags for locators and iterat…

…ors (Alexandre Parent) This PR was merged into the 4.4 branch. Discussion ---------- [DI] Fix support for multiple tags for locators and iterators | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Fix #34462, Fix #35326 | License | MIT | Doc PR | none Fix PriorityTaggedServiceTrait::findAndSortTaggedServices to work with multiple explicitely tagged services as would be expected by !tagged_locator. Also reorganize PriorityTaggedServiceTrait::findAndSortTaggedServices to be simpler and easier to understand. Commits ------- 6fc91eb [DI] Fix support for multiple tags for locators and iterators
symfony · Feb 4, 2020 · a59ce75 · a59ce75
2 parents 84d32ac + 6fc91eb
commit a59ce75
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 60 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
@@ -54,78 +54,81 @@ private function findAndSortTaggedServices($tagName, ContainerBuilder $container
 
         foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
             $class = $r = null;
-            $priority = 0;
-            if (isset($attributes[0]['priority'])) {
-                $priority = $attributes[0]['priority'];
-            } elseif ($defaultPriorityMethod) {
-                $class = $container->getDefinition($serviceId)->getClass();
-                $class = $container->getParameterBag()->resolveValue($class) ?: null;
-
-                if (($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));
-                    }
-
-                    $priority = $rm->invoke(null);
-
-                    if (!\is_int($priority)) {
-                        throw new InvalidArgumentException(sprintf('Method "%s::%s()" should return an integer, got %s: tag "%s" on service "%s".', $class, $defaultPriorityMethod, \gettype($priority), $tagName, $serviceId));
-                    }
-                }
-            }
-
-            if (null === $indexAttribute && !$needsIndexes) {
-                $services[$priority][] = new Reference($serviceId);
-
-                continue;
-            }
 
-            if (!$class) {
-                $class = $container->getDefinition($serviceId)->getClass();
-                $class = $container->getParameterBag()->resolveValue($class) ?: null;
-            }
+            $defaultPriority = null;
+            $defaultIndex = null;
 
-            if (null !== $indexAttribute && isset($attributes[0][$indexAttribute])) {
-                $services[$priority][$attributes[0][$indexAttribute]] = new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $attributes[0][$indexAttribute]);
+            foreach ($attributes as $attribute) {
+                $index = $priority = null;
 
-                continue;
-            }
+                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)) {
-                throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $serviceId));
-            }
+                    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));
+                        }
 
-            $class = $r->name;
+                        if (!$rm->isPublic()) {
+                            throw new InvalidArgumentException(sprintf('Method "%s::%s()" should be public: tag "%s" on service "%s".', $class, $defaultPriorityMethod, $tagName, $serviceId));
+                        }
 
-            if (!$r->hasMethod($defaultIndexMethod)) {
-                if ($needsIndexes) {
-                    $services[$priority][$serviceId] = new TypedReference($serviceId, $class);
+                        $defaultPriority = $rm->invoke(null);
 
-                    continue;
+                        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));
+                        }
+                    }
                 }
 
-                throw new InvalidArgumentException(sprintf('Method "%s::%s()" not found: tag "%s" on service "%s" is missing "%s" attribute.', $class, $defaultIndexMethod, $tagName, $serviceId, $indexAttribute));
-            }
+                $priority = $priority ?? $defaultPriority ?? 0;
+
+                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;
+
+                    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));
+                        }
+
+                        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));
+                        }
+
+                        $defaultIndex = $rm->invoke(null);
+
+                        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));
+                        }
+                    }
 
-            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));
-            }
+                    $defaultIndex = $defaultIndex ?? $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));
-            }
+                $index = $index ?? $defaultIndex;
 
-            $key = $rm->invoke(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 (!\is_string($key)) {
-                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($key), $tagName, $serviceId, $indexAttribute));
+                $services[$priority][$index] = $reference;
             }
-
-            $services[$priority][$key] = new TypedReference($serviceId, $class, ContainerBuilder::EXCEPTION_ON_INVALID_REFERENCE, $key);
         }
 
         if ($services) {

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php
@@ -314,6 +314,32 @@ public function testTaggedServiceWithIndexAttributeAndDefaultMethod()
         $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get(BarTagClass::class), 'foo' => $container->get(FooTagClass::class)], $param);
     }
 
+    public function testTaggedIteratorWithMultipleIndexAttribute()
+    {
+        $container = new ContainerBuilder();
+        $container->register(BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar', ['foo' => 'bar'])
+            ->addTag('foo_bar', ['foo' => 'bar_duplicate'])
+        ;
+        $container->register(FooTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar')
+            ->addTag('foo_bar')
+        ;
+        $container->register(FooBarTaggedClass::class)
+            ->addArgument(new TaggedIteratorArgument('foo_bar', 'foo'))
+            ->setPublic(true)
+        ;
+
+        $container->compile();
+
+        $s = $container->get(FooBarTaggedClass::class);
+
+        $param = iterator_to_array($s->getParam()->getIterator());
+        $this->assertSame(['bar' => $container->get(BarTagClass::class), 'bar_duplicate' => $container->get(BarTagClass::class), 'foo_tag_class' => $container->get(FooTagClass::class)], $param);
+    }
+
     public function testTaggedServiceWithDefaultPriorityMethod()
     {
         $container = new ContainerBuilder();
@@ -350,7 +376,7 @@ public function testTaggedServiceLocatorWithIndexAttribute()
             ->addTag('foo_bar')
         ;
         $container->register('foo_bar_tagged', FooBarTaggedClass::class)
-            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo')))
+            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', null, true)))
             ->setPublic(true)
         ;
 
@@ -369,6 +395,40 @@ public function testTaggedServiceLocatorWithIndexAttribute()
         $this->assertSame(['bar' => $container->get('bar_tag'), 'foo_tag_class' => $container->get('foo_tag')], $same);
     }
 
+    public function testTaggedServiceLocatorWithMultipleIndexAttribute()
+    {
+        $container = new ContainerBuilder();
+        $container->register('bar_tag', BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar', ['foo' => 'bar'])
+            ->addTag('foo_bar', ['foo' => 'bar_duplicate'])
+        ;
+        $container->register('foo_tag', FooTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar')
+            ->addTag('foo_bar')
+        ;
+        $container->register('foo_bar_tagged', FooBarTaggedClass::class)
+            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 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)));
+
+        $same = [
+            'bar' => $serviceLocator->get('bar'),
+            'bar_duplicate' => $serviceLocator->get('bar_duplicate'),
+            'foo_tag_class' => $serviceLocator->get('foo_tag_class'),
+        ];
+        $this->assertSame(['bar' => $container->get('bar_tag'), 'bar_duplicate' => $container->get('bar_tag'), 'foo_tag_class' => $container->get('foo_tag')], $same);
+    }
+
     public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
     {
         $container = new ContainerBuilder();
@@ -381,7 +441,7 @@ public function testTaggedServiceLocatorWithIndexAttributeAndDefaultMethod()
             ->addTag('foo_bar', ['foo' => 'foo'])
         ;
         $container->register('foo_bar_tagged', FooBarTaggedClass::class)
-            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar')))
+            ->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar', true)))
             ->setPublic(true)
         ;