[DI] change name to tag + add XMl support + adding yaml/xml tests

symfony · nicolas-grekas · Feb 22, 2019 · Dec 13, 2018 · Feb 13, 2019 · Feb 22, 2019
commit 101bfd79bfc9853e7935f2c87411516eda0a8b2f
diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php
@@ -22,14 +22,21 @@ class TaggedIteratorArgument extends IteratorArgument
     private $indexAttribute;
     private $defaultIndexMethod;
 
+    /**
+     * TaggedIteratorArgument constructor.
+     *
+     * @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
+     */
     public function __construct(string $tag, string $indexAttribute = null, string $defaultIndexMethod = null)
     {
         parent::__construct([]);
 
         $this->tag = $tag;
-        $this->indexAttribute = $indexAttribute ?: null;
 
         if ($indexAttribute) {
+            $this->indexAttribute = $indexAttribute;
             $this->defaultIndexMethod = $defaultIndexMethod ?: ('getDefault'.str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $indexAttribute))).'Name');
         }
     }

diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md
@@ -9,6 +9,7 @@ CHANGELOG
  * added `%env(nullable:...)%` processor to allow empty variables to be processed as null values
  * added support for deprecating aliases
  * made `ContainerParametersResource` final and not implement `Serializable` anymore
+ * added ability to define an index for a tagged collection
 
 4.2.0
 -----

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@@ -286,6 +286,14 @@ private function convertParameters(array $parameters, $type, \DOMElement $parent
             } elseif ($value instanceof TaggedIteratorArgument) {
                 $element->setAttribute('type', 'tagged');
                 $element->setAttribute('tag', $value->getTag());
+
+                if (null !== $value->getIndexAttribute()) {
+                    $element->setAttribute('index-by', $value->getIndexAttribute());
+                }
+
+                if (null !== $value->getDefaultIndexMethod()) {
+                    $element->setAttribute('default-index-method', $value->getDefaultIndexMethod());
+                }
             } 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
@@ -233,6 +233,19 @@ private function dumpValue($value)
         }
         if ($value instanceof ArgumentInterface) {
             if ($value instanceof TaggedIteratorArgument) {
+                if (null !== $value->getIndexAttribute()) {
+                    $taggedValueContent = [
+                        'tag' => $value->getTag(),
+                        'index_by' => $value->getIndexAttribute(),
+                    ];
+
+                    if (null !== $value->getDefaultIndexMethod()) {
+                        $taggedValueContent['default_index_method'] = $value->getDefaultIndexMethod();
+                    }
+
+                    return new TaggedValue('tagged', $taggedValueContent);
+                }
+
                 return new TaggedValue('tagged', $value->getTag());
             }
             if ($value instanceof IteratorArgument) {

diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@@ -353,7 +353,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
                     continue;
                 }
 
-                if (false !== strpos($name, '-') && false === strpos($name, '_') && !array_key_exists($normalizedName = str_replace('-', '_', $name), $parameters)) {
+                if (false !== strpos($name, '-') && false === strpos($name, '_') && !\array_key_exists($normalizedName = str_replace('-', '_', $name), $parameters)) {
                     $parameters[$normalizedName] = XmlUtils::phpize($node->nodeValue);
                 }
                 // keep not normalized key
@@ -537,7 +537,8 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $file, $lowercase =
                     if (!$arg->getAttribute('tag')) {
                         throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="tagged" has no or empty "tag" attribute in "%s".', $name, $file));
                     }
-                    $arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'));
+
+                    $arguments[$key] = new TaggedIteratorArgument($arg->getAttribute('tag'), $arg->getAttribute('index-by') ?: null, $arg->getAttribute('default-index-method') ?: null);
                     break;
                 case 'binary':
                     if (false === $value = base64_decode($arg->nodeValue)) {

diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
@@ -203,7 +203,7 @@ private function parseDefinitions(array $content, string $file)
             throw new InvalidArgumentException(sprintf('The "services" key should contain an array in %s. Check your YAML syntax.', $file));
         }
 
-        if (array_key_exists('_instanceof', $content['services'])) {
+        if (\array_key_exists('_instanceof', $content['services'])) {
             $instanceof = $content['services']['_instanceof'];
             unset($content['services']['_instanceof']);
 
@@ -235,7 +235,7 @@ private function parseDefinitions(array $content, string $file)
      */
     private function parseDefaults(array &$content, string $file): array
     {
-        if (!array_key_exists('_defaults', $content['services'])) {
+        if (!\array_key_exists('_defaults', $content['services'])) {
             return [];
         }
         $defaults = $content['services']['_defaults'];
@@ -342,7 +342,7 @@ private function parseDefinition($id, $service, $file, array $defaults)
 
         if (isset($service['alias'])) {
             $this->container->setAlias($id, $alias = new Alias($service['alias']));
-            if (array_key_exists('public', $service)) {
+            if (\array_key_exists('public', $service)) {
                 $alias->setPublic($service['public']);
             } elseif (isset($defaults['public'])) {
                 $alias->setPublic($defaults['public']);
@@ -430,7 +430,7 @@ private function parseDefinition($id, $service, $file, array $defaults)
             $definition->setAbstract($service['abstract']);
         }
 
-        if (array_key_exists('deprecated', $service)) {
+        if (\array_key_exists('deprecated', $service)) {
             $definition->setDeprecated(true, $service['deprecated']);
         }
 
@@ -545,11 +545,11 @@ private function parseDefinition($id, $service, $file, array $defaults)
             }
         }
 
-        if (array_key_exists('namespace', $service) && !array_key_exists('resource', $service)) {
+        if (\array_key_exists('namespace', $service) && !\array_key_exists('resource', $service)) {
             throw new InvalidArgumentException(sprintf('A "resource" attribute must be set when the "namespace" attribute is set for service "%s" in %s. Check your YAML syntax.', $id, $file));
         }
 
-        if (array_key_exists('resource', $service)) {
+        if (\array_key_exists('resource', $service)) {
             if (!\is_string($service['resource'])) {
                 throw new InvalidArgumentException(sprintf('A "resource" attribute must be of type string for service "%s" in %s. Check your YAML syntax.', $id, $file));
             }
@@ -713,14 +713,16 @@ private function resolveServices($value, $file, $isParameter = false)
                 if (\is_string($argument) && $argument) {
                     return new TaggedIteratorArgument($argument);
                 }
-                if (\is_array($argument) && isset($argument['name']) && $argument['name']) {
-                    if (array_diff(array_keys($argument), ['name', 'index_by', 'default_index_method'])) {
-                        throw new InvalidArgumentException('"!tagged" tag contains unsupported keys. Supported are: "name, index_by, default_index_method".');
+
+                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('"!tagged" tag contains unsupported key "%s"; supported ones are "tag", "index_by" and "default_index_method".', implode('"", "', $diff)));
                     }
 
-                    return new TaggedIteratorArgument($argument['name'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null);
+                    return new TaggedIteratorArgument($argument['tag'], $argument['index_by'] ?? null, $argument['default_index_method'] ?? null);
                 }
-                throw new InvalidArgumentException(sprintf('"!tagged" tag only accepts a non empty string or an array with a key "name" in "%s".', $file));
+
+                throw new InvalidArgumentException(sprintf('"!tagged" tags only accept a non empty string or an array with a key "tag" in "%s".', $file));
             }
             if ('service' === $value->getTag()) {
                 if ($isParameter) {

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
@@ -234,6 +234,8 @@
     <xsd:attribute name="index" type="xsd:integer" />
     <xsd:attribute name="on-invalid" type="invalid_sequence" />
     <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: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
@@ -14,10 +14,14 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Alias;
+use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\BarTagClass;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooBarTaggedClass;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\FooTagClass;
 
 /**
  * This class tests the integration of the different compiler passes.
@@ -234,6 +238,54 @@ public function getYamlCompileTests()
             $container,
         ];
     }
+
+    public function testTaggedServiceWithIndexAttribute()
+    {
+        $container = new ContainerBuilder();
+        $container->register(BarTagClass::class, BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar', ['foo' => 'bar'])
+        ;
+        $container->register(FooTagClass::class, FooTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar')
+        ;
+        $container->register(FooBarTaggedClass::class, 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), 'foo_tag_class' => $container->get(FooTagClass::class)], $param);
+    }
+
+    public function testTaggedServiceWithIndexAttributeAndDefaultMethod()
+    {
+        $container = new ContainerBuilder();
+        $container->register(BarTagClass::class, BarTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar')
+        ;
+        $container->register(FooTagClass::class, FooTagClass::class)
+            ->setPublic(true)
+            ->addTag('foo_bar', ['foo' => 'foo'])
+        ;
+        $container->register(FooBarTaggedClass::class, FooBarTaggedClass::class)
+            ->addArgument(new TaggedIteratorArgument('foo_bar', 'foo', 'getFooBar'))
+            ->setPublic(true)
+        ;
+
+        $container->compile();
+
+        $s = $container->get(FooBarTaggedClass::class);
+
+        $param = iterator_to_array($s->getParam()->getIterator());
+        $this->assertSame(['bar_tab_class_with_defaultmethod' => $container->get(BarTagClass::class), 'foo' => $container->get(FooTagClass::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
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Dumper\XmlDumper;
@@ -200,6 +201,19 @@ public function testDumpLoad()
         $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_dump_load.xml', $dumper->dump());
     }
 
+    public function testTaggedArgument()
+    {
+        $container = new ContainerBuilder();
+        $container->register('foo', 'Foo')->addTag('foo_tag');
+        $container->register('foo_tagged_iterator', 'Bar')
+            ->setPublic(true)
+            ->addArgument(new TaggedIteratorArgument('foo_tag', 'barfoo', 'foobar'))
+        ;
+
+        $dumper = new XmlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/xml/services_with_tagged_arguments.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
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
@@ -95,6 +96,16 @@ public function testInlineServices()
         $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_inline.yml', $dumper->dump());
     }
 
+    public function testTaggedArgument()
+    {
+        $container = new ContainerBuilder();
+        $container->register('foo_service', 'Foo')->addTag('foo');
+        $container->register('foo_service_tagged', 'Bar')->addArgument(new TaggedIteratorArgument('foo', 'barfoo', 'foobar'));
+
+        $dumper = new YamlDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/yaml/services_with_tagged_argument.yml', $dumper->dump());
+    }
+
     private function assertEqualYamlStructure($expected, $yaml, $message = '')
     {
         $parser = new Parser();

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/BarTagClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/BarTagClass.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class BarTagClass
+{
+    public static function getDefaultFooName()
+    {
+        return 'bar_tag_class';
+    }
+
+    public static function getFooBar()
+    {
+        return 'bar_tab_class_with_defaultmethod';
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooBarTaggedClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooBarTaggedClass.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooBarTaggedClass
+{
+    private $param;
+
+    public function __construct($param = [])
+    {
+        $this->param = $param;
+    }
+
+    public function getParam()
+    {
+        return $this->param;
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class FooTagClass
+{
+    public static function getDefaultFooName()
+    {
+        return 'foo_tag_class';
+    }
+}
diff --git a/...mfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_tagged_arguments.xml b/...mfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_tagged_arguments.xml
@@ -0,0 +1,14 @@
+<?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 http://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" index-by="barfoo" default-index-method="foobar"/>
+    </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/...mfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml b/...mfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml
@@ -0,0 +1,19 @@
+
+services:
+    service_container:
+        class: Symfony\Component\DependencyInjection\ContainerInterface
+        public: true
+        synthetic: true
+    foo_service:
+        class: Foo
+        tags:
+            - { name: foo }
+    foo_service_tagged:
+        class: Bar
+        arguments: [!tagged { tag: foo, index_by: barfoo, default_index_method: foobar }]
+    Psr\Container\ContainerInterface:
+        alias: service_container
+        public: false
+    Symfony\Component\DependencyInjection\ContainerInterface:
+        alias: service_container
+        public: false
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
@@ -18,6 +18,7 @@
 use Symfony\Component\Config\Resource\GlobResource;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
+use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
 use Symfony\Component\DependencyInjection\Loader\IniFileLoader;
@@ -315,6 +316,17 @@ public function testParsesTags()
         }
     }
 
+    public function testParseTaggedArgumentsWithIndexBy()
+    {
+        $container = new ContainerBuilder();
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
+        $loader->load('services_with_tagged_arguments.xml');
+
+        $this->assertCount(1, $container->getDefinition('foo')->getTag('foo_tag'));
+        $this->assertCount(1, $container->getDefinition('foo_tagged_iterator')->getArguments());
+        $this->assertEquals(new TaggedIteratorArgument('foo_tag', 'barfoo', 'foobar'), $container->getDefinition('foo_tagged_iterator')->getArgument(0));
+    }
+
     /**
      * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
      */