added ability to define an priority method for a tagged collection

symfony · pcabreus · Jun 7, 2019 · Jun 10, 2019 · Jun 10, 2019 · Jun 10, 2019
commit 2df617e1e851fa0cacf1b0eec4aef8f010870983
diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php
@@ -22,14 +22,16 @@ class TaggedIteratorArgument extends IteratorArgument
     private $indexAttribute;
     private $defaultIndexMethod;
     private $needsIndexes = false;
+    private $defaultPriorityMethod;
 
     /**
-     * @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        $needsIndexes       Whether indexes are required and should be generated when computing the map
+     * @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        $needsIndexes          Whether indexes are required and should be generated when computing the map
+     * @param string|null $defaultPriorityMethod The static method that should be called to get the default priority of the service
      */
-    public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false)
+    public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null)
     {
         parent::__construct([]);
 
@@ -41,6 +43,7 @@ public function __construct(string $tag, string $indexAttribute = null, string $
         $this->indexAttribute = $indexAttribute;
         $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute ?? ''))).'Name');
         $this->needsIndexes = $needsIndexes;
+        $this->defaultPriorityMethod = $defaultPriorityMethod;
     }
 
     public function getTag()
@@ -62,4 +65,9 @@ public function needsIndexes(): bool
     {
         return $this->needsIndexes;
     }
+
+    public function getDefaultPriorityMethod(): ?string
+    {
+        return $this->defaultPriorityMethod;
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md
@@ -5,6 +5,7 @@ CHANGELOG
 -----
 
  * deprecated support for short factories and short configurators in Yaml
+ * added ability to define an priority method for a tagged collection
 
 4.3.0
 -----

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
@@ -41,39 +41,82 @@ trait PriorityTaggedServiceTrait
      */
     private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
     {
-        $indexAttribute = $defaultIndexMethod = $needsIndexes = null;
+        $indexAttribute = $defaultIndexMethod = $needsIndexes = $defaultPriorityMethod = null;
 
         if ($tagName instanceof TaggedIteratorArgument) {
             $indexAttribute = $tagName->getIndexAttribute();
             $defaultIndexMethod = $tagName->getDefaultIndexMethod();
             $needsIndexes = $tagName->needsIndexes();
+            $defaultPriorityMethod = $tagName->getDefaultPriorityMethod();
             $tagName = $tagName->getTag();
         }
 
         $services = [];
 
         foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $attributes) {
-            $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : (method_exists($serviceId, 'getDefaultPriority') ? $serviceId::getDefaultPriority() : 0);
+            $class = $container->getDefinition($serviceId)->getClass();
+            $class = $container->getParameterBag()->resolveValue($class) ?: null;
+
+            if (!$r = $container->getReflectionClass($class)) {
+                throw new InvalidArgumentException( sprintf('Class "%s" used for service "%s" cannot be found.', $class, $serviceId));
+            }
+
+            $priority = 0;
+            if ($defaultPriorityMethod && $r->hasMethod($defaultPriorityMethod)) {
+                if (!($rpm = $r->getMethod($defaultPriorityMethod))->isStatic()) {
+                    throw new InvalidArgumentException(
+                        sprintf(
+                            'Method "%s::%s()" should be static: tag "%s" on service "%s" is missing default priority method.',
+                            $class,
+                            $defaultPriorityMethod,
+                            $tagName,
+                            $serviceId
+                        )
+                    );
+                }
+
+                if (!$rpm->isPublic()) {
+                    throw new InvalidArgumentException(
+                        sprintf(
+                            'Method "%s::%s()" should be public: tag "%s" on service "%s" is missing default priority method.',
+                            $class,
+                            $defaultPriorityMethod,
+                            $tagName,
+                            $serviceId
+                        )
+                    );
+                }
+
+                $priority = $rpm->invoke(null);
+
+                if (!\is_integer($priority)) {
+                    throw new InvalidArgumentException(
+                        sprintf(
+                            'Method "%s::%s()" should return an integer, got %s: tag "%s" on service "%s" is missing default priority method.',
+                            $class,
+                            $defaultPriorityMethod,
+                            \gettype($priority),
+                            $tagName,
+                            $serviceId
+                        )
+                    );
+                }
+            }
+
+            $priority = $attributes[0]['priority'] ?? $priority;
 
             if (null === $indexAttribute && !$needsIndexes) {
                 $services[$priority][] = new Reference($serviceId);
 
                 continue;
             }
 
-            $class = $container->getDefinition($serviceId)->getClass();
-            $class = $container->getParameterBag()->resolveValue($class) ?: 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]);
 
                 continue;
             }
 
-            if (!$r = $container->getReflectionClass($class)) {
-                throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $serviceId));
-            }
-
             $class = $r->name;
 
             if (!$r->hasMethod($defaultIndexMethod)) {

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@@ -297,6 +297,10 @@ private function convertParameters(array $parameters, $type, \DOMElement $parent
                         $element->setAttribute('default-index-method', $tag->getDefaultIndexMethod());
                     }
                 }
+
+                if (null !== $tag->getDefaultPriorityMethod()) {
+                    $element->setAttribute('default-priority-method', $tag->getDefaultPriorityMethod());
+                }
             } elseif ($value instanceof IteratorArgument) {
                 $element->setAttribute('type', 'iterator');
                 $this->convertParameters($value->getValues(), $type, $element, 'key');

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
@@ -235,19 +235,24 @@ private function dumpValue($value)
             $tag = $value;
 
             if ($value instanceof TaggedIteratorArgument || ($value instanceof ServiceLocatorArgument && $tag = $value->getTaggedIteratorArgument())) {
-                if (null === $tag->getIndexAttribute()) {
-                    $content = $tag->getTag();
-                } else {
-                    $content = [
-                        'tag' => $tag->getTag(),
-                        'index_by' => $tag->getIndexAttribute(),
-                    ];
+                $content = ['tag' => $tag->getTag()];
+
+                if (null !== $tag->getIndexAttribute()) {
+                    $content['index_by'] = $tag->getIndexAttribute();
 
                     if (null !== $tag->getDefaultIndexMethod()) {
                         $content['default_index_method'] = $tag->getDefaultIndexMethod();
                     }
                 }
 
+                if (null !== $tag->getDefaultPriorityMethod()) {
+                    $content['default_priority_method'] = $tag->getDefaultPriorityMethod();
+                }
+
+                if (count($content) === 1) {
+                    $content = $tag->getTag();
+                }
+
                 return new TaggedValue($value instanceof TaggedIteratorArgument ? 'tagged' : 'tagged_locator', $content);
             }
 

diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php
@@ -115,22 +115,39 @@ function iterator(array $values): IteratorArgument
 
 /**
  * Creates a lazy iterator by tag name.
+ *
+ * @param string $tag
+ * @param string|null $indexAttribute
+ * @param string|null $defaultIndexMethod
+ * @param bool $needsIndexes
+ * @param string|null $defaultPriorityMethod
+ * @return TaggedIteratorArgument
  */
-function tagged(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null): TaggedIteratorArgument
+function tagged(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null, bool $needsIndexes = false, string $defaultPriorityMethod = null): TaggedIteratorArgument
 {
-    return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod);
+    return new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod,$needsIndexes, $defaultPriorityMethod);
 }
 
 /**
  * Creates a service locator by tag name.
+ *
+ * @param string $tag
+ * @param string $indexAttribute
+ * @param string|null $defaultIndexMethod
+ * @param bool $needsIndexes
+ * @param string|null $defaultPriorityMethod
+ * @return ServiceLocatorArgument
  */
-function tagged_locator(string $tag, string $indexAttribute, string $defaultIndexMethod = null): ServiceLocatorArgument
+function tagged_locator(string $tag, string $indexAttribute, string $defaultIndexMethod = null, bool $needsIndexes = true, string $defaultPriorityMethod = null): ServiceLocatorArgument
 {
-    return new ServiceLocatorArgument(new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, true));
+    return new ServiceLocatorArgument(new TaggedIteratorArgument($tag, $indexAttribute, $defaultIndexMethod, $needsIndexes, $defaultPriorityMethod));
 }
 
 /**
  * Creates an expression.
+ *
+ * @param string $expression
+ * @return Expression
  */
 function expr(string $expression): Expression
 {

diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -553,7 +553,7 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $file, $lowercase =
                         throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="%s" has no or empty "tag" attribute in "%s".', $name, $type, $file));
                     }
 
-                    $arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'), $arg->getAttribute('index-by') ?: null, $arg->getAttribute('default-index-method') ?: null, $forLocator);
+                    $arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'), $arg->getAttribute('index-by') ?: null, $arg->getAttribute('default-index-method') ?: null, $forLocator, $arg->getAttribute('default-priority-method') ?: null);
 
                     if ($forLocator) {
                         $arguments[$key] = new ServiceLocatorArgument($arguments[$key]);

diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
@@ -733,11 +733,11 @@ private function resolveServices($value, $file, $isParameter = false)
                 }
 
                 if (\is_array($argument) && isset($argument['tag']) && $argument['tag']) {
-                    if ($diff = array_diff(array_keys($argument), ['tag', 'index_by', 'default_index_method'])) {
-                        throw new InvalidArgumentException(sprintf('"!%s" tag contains unsupported key "%s"; supported ones are "tag", "index_by" and "default_index_method".', $value->getTag(), implode('"", "', $diff)));
+                    if ($diff = array_diff(array_keys($argument), ['tag', 'index_by', 'default_index_method', 'default_priority_method'])) {
+                        throw new InvalidArgumentException(sprintf('"!%s" tag contains unsupported key "%s"; supported ones are "tag", "index_by", "default_index_method" and "default_priority_method".', $value->getTag(), implode('"", "', $diff)));
                     }
 
-                    $argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null, $forLocator);
+                    $argument = new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null, $forLocator, $argument['default_priority_method'] ?? null);
 
                     if ($forLocator) {
                         $argument = new ServiceLocatorArgument($argument);

diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
@@ -236,6 +236,7 @@
     <xsd:attribute name="tag" type="xsd:string" />
     <xsd:attribute name="index-by" type="xsd:string" />
     <xsd:attribute name="default-index-method" type="xsd:string" />
+    <xsd:attribute name="default-priority-method" type="xsd:string" />
   </xsd:complexType>
 
   <xsd:complexType name="call">

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php
@@ -402,6 +402,30 @@ public function testTaggedServiceLocatorWithDefaultIndex()
         ];
         $this->assertSame($expected, ['baz' => $serviceLocator->get('baz')]);
     }
+
+    public function testTaggedServiceWithPriorityMethod()
+    {
+        $container = new ContainerBuilder();
+        $container->register(BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar', ['priority' => 10])
+        ;
+        $container->register(FooTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar')
+        ;
+        $container->register(FooBarTaggedClass::class)
+            ->addArgument(new TaggedIteratorArgument('foo_bar', null, null, false, 'getDefaultPriority'))
+            ->setPublic(true)
+        ;
+
+        $container->compile();
+
+        $s = $container->get(FooBarTaggedClass::class);
+
+        $param = iterator_to_array($s->getParam()->getIterator());
+        $this->assertSame([0 => $container->get(FooTagClass::class), 1 => $container->get(BarTagClass::class)], $param);
+    }
 }
 
 class ServiceSubscriberStub implements ServiceSubscriberInterface

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php
@@ -220,6 +220,24 @@ public function testTaggedArguments()
         $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_arguments.xml', $dumper->dump());
     }
 
+    public function testTaggedWithDefaultPriorityMethod()
+    {
+        $taggedIterator = new TaggedIteratorArgument('foo_tag', null, null, false, 'foopriority');
+        $container = new ContainerBuilder();
+        $container->register('foo', 'Foo')->addTag('foo_tag');
+        $container->register('foo_tagged_iterator', 'Bar')
+            ->setPublic(true)
+            ->addArgument($taggedIterator)
+        ;
+        $container->register('foo_tagged_locator', 'Bar')
+            ->setPublic(true)
+            ->addArgument(new ServiceLocatorArgument($taggedIterator))
+        ;
+
+        $dumper = new XmlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_priority_method.xml', $dumper->dump());
+    }
+
     public function testDumpAbstractServices()
     {
         $container = include self::$fixturesPath.'/containers/container_abstract.php';

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php
@@ -109,6 +109,18 @@ public function testTaggedArguments()
         $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_argument.yml', $dumper->dump());
     }
 
+    public function testTaggedWithDefaultPriorityMethod()
+    {
+        $taggedIterator = new TaggedIteratorArgument('foo', null, null, false, 'foopriority');
+        $container = new ContainerBuilder();
+        $container->register('foo_service', 'Foo')->addTag('foo');
+        $container->register('foo_service_tagged_iterator', 'Bar')->addArgument($taggedIterator);
+        $container->register('foo_service_tagged_locator', 'Bar')->addArgument(new ServiceLocatorArgument($taggedIterator));
+
+        $dumper = new YamlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_priority_method.yml', $dumper->dump());
+    }
+
     private function assertEqualYamlStructure($expected, $yaml, $message = '')
     {
         $parser = new Parser();

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php
@@ -8,4 +8,9 @@ public static function getDefaultFooName()
     {
         return 'foo_tag_class';
     }
+
+    public static function getDefaultPriority()
+    {
+        return 20;
+    }
 }
diff --git a/...Component/DependencyInjection/Tests/Fixtures/xml/services_with_tagged_priority_method.xml b/...Component/DependencyInjection/Tests/Fixtures/xml/services_with_tagged_priority_method.xml
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
+    <service id="foo" class="Foo">
+      <tag name="foo_tag"/>
+    </service>
+    <service id="foo_tagged_iterator" class="Bar" public="true">
+      <argument type="tagged" tag="foo_tag" default-priority-method="foopriority"/>
+    </service>
+    <service id="foo_tagged_locator" class="Bar" public="true">
+      <argument type="tagged_locator" tag="foo_tag" default-priority-method="foopriority"/>
+    </service>
+    <service id="Psr\Container\ContainerInterface" alias="service_container" public="false"/>
+    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" public="false"/>
+  </services>
+</container>
diff --git a/...omponent/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_priority_method.yml b/...omponent/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_priority_method.yml
@@ -0,0 +1,22 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo_service:
+        class: Foo
+        tags:
+            - { name: foo }
+    foo_service_tagged_iterator:
+        class: Bar
+        arguments: [!tagged { tag: foo, default_priority_method: foopriority }]
+    foo_service_tagged_locator:
+        class: Bar
+        arguments: [!tagged_locator { tag: foo, default_priority_method: foopriority }]
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        public: false
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        public: false
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ CHANGELOG @@
     -----
      * deprecated support for short factories and short configurators in Yaml
+     * added ability to define an priority method for a tagged collection
 .3.0
     -----
@@ Expand Down @@