From 101bfd79bfc9853e7935f2c87411516eda0a8b2f Mon Sep 17 00:00:00 2001 From: Anthony MARTIN Date: Wed, 13 Feb 2019 17:39:28 +0100 Subject: [PATCH] [DI] change name to tag + add XMl support + adding yaml/xml tests --- .../Argument/TaggedIteratorArgument.php | 9 +++- .../DependencyInjection/CHANGELOG.md | 1 + .../DependencyInjection/Dumper/XmlDumper.php | 8 +++ .../DependencyInjection/Dumper/YamlDumper.php | 13 +++++ .../Loader/XmlFileLoader.php | 5 +- .../Loader/YamlFileLoader.php | 24 +++++---- .../schema/dic/services/services-1.0.xsd | 2 + .../Tests/Compiler/IntegrationTest.php | 52 +++++++++++++++++++ .../Tests/Dumper/XmlDumperTest.php | 14 +++++ .../Tests/Dumper/YamlDumperTest.php | 11 ++++ .../Tests/Fixtures/BarTagClass.php | 16 ++++++ .../Tests/Fixtures/FooBarTaggedClass.php | 18 +++++++ .../Tests/Fixtures/FooTagClass.php | 11 ++++ .../xml/services_with_tagged_arguments.xml | 14 +++++ .../yaml/services_with_tagged_argument.yml | 19 +++++++ .../Tests/Loader/XmlFileLoaderTest.php | 12 +++++ .../Tests/Loader/YamlFileLoaderTest.php | 12 +++++ 17 files changed, 227 insertions(+), 14 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/BarTagClass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooBarTaggedClass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_with_tagged_arguments.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml diff --git a/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/TaggedIteratorArgument.php index b6a06f7de7..fabfb00451 100644 --- 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 index 87739fc975..e5fcd4f768 100644 --- 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 index 704df2af39..4b01bd30d3 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php @@ -286,6 +286,14 @@ class XmlDumper extends Dumper } 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 index 5b9e747315..875ebbc0ab 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -233,6 +233,19 @@ class YamlDumper extends Dumper } 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 index 16b9b48c20..6f672bdc7f 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -353,7 +353,7 @@ class XmlFileLoader extends FileLoader 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 @@ class XmlFileLoader extends FileLoader 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 index 700cd79be5..06fcbb4a91 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -203,7 +203,7 @@ class YamlFileLoader extends FileLoader 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 @@ class YamlFileLoader extends FileLoader */ 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 @@ class YamlFileLoader extends FileLoader 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 @@ class YamlFileLoader extends FileLoader $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 @@ class YamlFileLoader extends FileLoader } } - 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 @@ class YamlFileLoader extends FileLoader 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 index 25ef73a14e..b14ffc8dcd 100644 --- 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 @@ + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php index b9f9d7bf36..1ba0a53eaa 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php @@ -14,10 +14,14 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; 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 @@ class IntegrationTest extends TestCase $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 index ac274c6f26..b110cdc5e8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/XmlDumperTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Dumper; 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 @@ class XmlDumperTest extends TestCase $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 index 49ee8e6f30..61a1aec510 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/YamlDumperTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Dumper; 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 @@ class YamlDumperTest extends TestCase $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 new file mode 100644 index 0000000000..9e065f6b10 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/BarTagClass.php @@ -0,0 +1,16 @@ +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 new file mode 100644 index 0000000000..c1279b9a9f --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FooTagClass.php @@ -0,0 +1,11 @@ + + + + + + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_with_tagged_argument.yml new file mode 100644 index 0000000000..bf7cf06930 --- /dev/null +++ b/src/Symfony/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 index 9cb64f39da..20c80258e2 100644 --- 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\FileResource; 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 @@ class XmlFileLoaderTest extends TestCase } } + 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 */ diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 8c9ccaf06f..7870a521a9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -18,6 +18,7 @@ use Symfony\Component\Config\Resource\FileResource; 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\Loader\IniFileLoader; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; @@ -279,6 +280,17 @@ class YamlFileLoaderTest extends TestCase } } + public function testTaggedArgumentsWithIndex() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('services_with_tagged_argument.yml'); + + $this->assertCount(1, $container->getDefinition('foo_service')->getTag('foo')); + $this->assertCount(1, $container->getDefinition('foo_service_tagged')->getArguments()); + $this->assertEquals(new TaggedIteratorArgument('foo', 'barfoo', 'foobar'), $container->getDefinition('foo_service_tagged')->getArgument(0)); + } + public function testNameOnlyTagsAreAllowedAsString() { $container = new ContainerBuilder();