10000 [DependencyInjection] Allow array attributes for service tags by aschempp · Pull Request #38540 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] Allow array attributes for service tags #38540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[DependencyInjection] Allow array attributes for service tags #38540

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8494c7c
Allow array attributes for service tags
aschempp Oct 13, 2020
c9ee2dd
Rename methods and arguments
aschempp Jul 14, 2021
3d7d4f8
Fixed recursion and added tests for YamlFileLoader
aschempp Jul 14, 2021
aeb2ce1
Fixed XMLFileLoader and XSD and added test
aschempp Jul 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fixed XMLFileLoader and XSD and added test
  • Loading branch information
@aschempp
aschempp committed Jul 15, 2021
commit aeb2ce1a29fa438ac4af93e8c635705d61826214
8 changes: 4 additions & 4 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,14 @@ private function parseDefinition(\DOMElement $service, string $file, Definition
$tags = $this->getChildren($service, 'tag');

foreach ($tags as $tag) {
$tagName = $tag->nodeValue;
if ('' === $tagName && '' === $tagName = $tag->getAttribute('name')) {
$tagName = $tag->hasChildNodes() || '' === $tag->nodeValue ? $tag->getAttribute('name') : $tag->nodeValue;
if ('' === $tagName) {
throw new InvalidArgumentException(sprintf('The tag name for service "%s" in "%s" must be a non-empty string.', (string) $service->getAttribute('id'), $file));
}

$parameters = $this->getTagAttributes($tag, sprintf('The attribute name of tag "%s" for service "%s" in %s must be a non-empty string.', $tagName, (string) $service->getAttribute('id'), $file));
foreach ($tag->attributes as $name => $node) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something is missing here, that would allow removing this foreach. Right now, this looks like duplication with the new method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The foreach loop is to handle attributes on the tag itself, the method handles nested arguments.

Example in the initial comment:

<?xml version="1.0" ?>
<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="foo" class="BarClass" public="true">
        <tag name="foo_tag" some-option="cat">
            <attribute name="array_option">
                <attribute name="key1">lorem</attribute>
                <attribute name="key2">ipsum</attribute>
            </attribute>
        </tag>
    </service>
  </services>
</container>

The foreach would read some-option while the method recursively reads <atttribute …>

if ('name' === $name && '' === $tagName) {
if ('name' === $name) {
continue;
}

Expand Down Expand Up @@ -597,7 +597,7 @@ private function getTagAttributes(\DOMNode $node, string $missingName): array
}

if ($this->getChildren($childNode, 'attribute')) {
$parameters[$childNode->getAttribute('name')] = $this->getTagAttributes($childNode, $missingName);
$parameters[$name] = $this->getTagAttributes($childNode, $missingName);
} else {
if (false !== strpos($name, '-') && false === strpos($name, '_') && !\array_key_exists($normalizedName = str_replace('-', '_', $name), $parameters)) {
$parameters[$normalizedName] = XmlUtils::phpize($childNode->nodeValue);
Expand Down
12 changes: 4 additions & 8 deletions src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,11 @@
<xsd:attribute name="public" type="boolean" />
</xsd:complexType>

<xsd:complexType name="tag">
<xsd:complexType name="tag" mixed="true">
<xsd:choice minOccurs="0">
<xsd:element name="attribute" type="tag_attribute" maxOccurs="unbounded" />
<xsd:element name="attribute" type="tag_attribute" maxOccurs="unbounded"/>
</xsd:choice>
<xsd:simpleContent>
<xsd:extension base="xsd:string">
<xsd:anyAttribute namespace="##any" processContents="lax" />
</xsd:extension>
</xsd:simpleContent>
<xsd:anyAttribute namespace="##any" processContents="lax" />
</xsd:complexType>

<xsd:complexType name="deprecated">
Expand All @@ -229,7 +225,7 @@
</xsd:simpleContent>
</xsd:complexType>

<xsd:complexType name="tag_attribute">
<xsd:complexType name="tag_attribute" mixed="true">
<xsd:choice minOccurs="0">
<xsd:element name="attribute" type="tag_attribute" maxOccurs="unbounded" />
</xsd:choice>
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_array_tags.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 https://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="foo" class="Foo">
<tag name="foo_tag">
<attribute name="foo">bar</attribute>
<attribute name="bar">
<attribute name="foo">bar</attribute>
<attribute name="bar">foo</attribute>
</attribute>
</tag>
</service>
</services>
</container>
9 changes: 9 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,15 @@ public function testParseServiceClosure()
$this->assertEquals(new ServiceClosureArgument(new Reference('bar', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)), $container->getDefinition('foo')->getArgument(0));
}

public function testParseServiceTagsWithArrayAttributes()
{
$container = new ContainerBuilder();
$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());
}

public function testParseTagsWithoutNameThrowsException()
{
$this->expectException(InvalidArgumentException::class);
Expand Down
0