bug #54273 [DependencyInjection] fix XmlDumper when a tag contains also a 'name' property (lyrixx)

nicolas-grekas · nicolas-grekas · commit bf1ffd3f029e · 2024-03-14T09:39:46.000+01:00
This PR was merged into the 6.4 branch. Discussion ---------- [DependencyInjection] fix XmlDumper when a tag contains also a 'name' property | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT Since #47364, the tag can contain an **array** of attributes. And since #47364, SF uses it to store workflow metadata and also a **name** property. It likely broke the XML generation, and so it brokes `debug:router` command for instance. Sorry, I didn't notice it before! Before the patch, I got that in my container var/cache/dev/App_KernelDevDebugContainer.xml ```xml <tag>workflow<attribute name="name">article</attribute><attribute name="metadata"><attribute name="title">Manage article</attribute></attribute></tag> ``` After, I got ```xml <tag name="workflow"> <attribute name="name">article</attribute> <attribute name="metadata"> <attribute name="title">Manage article</attribute> </attribute> </tag> ``` Commits ------- 4a4b011 [DependencyInjection] fix XmlDumper when a tag contains also a 'name' property
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@@ -134,16 +134,17 @@ private function addService(Definition $definition, ?string $id, \DOMElement $pa
         foreach ($tags as $name => $tags) {
             foreach ($tags as $attributes) {
                 $tag = $this->document->createElement('tag');
-                if (!\array_key_exists('name', $attributes)) {
-                    $tag->setAttribute('name', $name);
-                } else {
-                    $tag->appendChild($this->document->createTextNode($name));
-                }
 
                 // Check if we have recursive attributes
                 if (array_filter($attributes, \is_array(...))) {
+                    $tag->setAttribute('name', $name);
                     $this->addTagRecursiveAttributes($tag, $attributes);
                 } else {
+                    if (!\array_key_exists('name', $attributes)) {
+                        $tag->setAttribute('name', $name);
+                    } else {
+                        $tag->appendChild($this->document->createTextNode($name));
+                    }
                     foreach ($attributes as $key => $value) {
                         $tag->setAttribute($key, $value ?? '');
                     }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_non_scalar_tags.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_non_scalar_tags.php
@@ -10,6 +10,7 @@
 $container
     ->register('foo', FooClass::class)
     ->addTag('foo_tag', [
+        'name' => 'attributeName',
         'foo' => 'bar',
         'bar' => [
             'foo' => 'bar',
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_array_tags.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_array_tags.xml
@@ -4,6 +4,7 @@
     <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" public="true" synthetic="true"/>
     <service id="foo" class="Bar\FooClass">
       <tag name="foo_tag">
+        <attribute name="name">attributeName</attribute>
         <attribute name="foo">bar</attribute>
         <attribute name="bar">
           <attribute name="foo">bar</attribute>
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_array_tags.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_array_tags.yml
@@ -7,4 +7,4 @@ services:
     foo:
         class: Bar\FooClass
         tags:
-            - foo_tag: { foo: bar, bar: { foo: bar, bar: foo } }
+            - foo_tag: { name: attributeName, foo: bar, bar: { foo: bar, bar: foo } }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
@@ -468,7 +468,7 @@ public function testParseServiceTagsWithArrayAttributes()
         $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
         $loader->load('services_with_array_tags.xml');
 
-        $this->assertEquals(['foo_tag' => [['foo' => 'bar', 'bar' => ['foo' => 'bar', 'bar' => 'foo']]]], $container->getDefinition('foo')->getTags());
+        $this->assertEquals(['foo_tag' => [['name' => 'attributeName', 'foo' => 'bar', 'bar' => ['foo' => 'bar', 'bar' => 'foo']]]], $container->getDefinition('foo')->getTags());
     }
 
     public function testParseTagsWithoutNameThrowsException()

Original file line number	Diff line number	Diff line change
`@@ -468,7 +468,7 @@ public function testParseServiceTagsWithArrayAttributes()`
`468`	`468`	`$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));`
`469`	`469`	`$loader->load('services_with_array_tags.xml');`
`470`	`470`
`471`		`- $this->assertEquals(['foo_tag' => [['foo' => 'bar', 'bar' => ['foo' => 'bar', 'bar' => 'foo']]]], $container->getDefinition('foo')->getTags());`
	`471`	`+ $this->assertEquals(['foo_tag' => [['name' => 'attributeName', 'foo' => 'bar', 'bar' => ['foo' => 'bar', 'bar' => 'foo']]]], $container->getDefinition('foo')->getTags());`
`472`	`472`	`}`
`473`	`473`
`474`	`474`	`public function testParseTagsWithoutNameThrowsException()`