From 23cb83f726e5905b49c4901b149dfdd2d08375c1 Mon Sep 17 00:00:00 2001 From: Zan Baldwin Date: Mon, 18 Feb 2019 14:13:45 +0000 Subject: [PATCH] [DependencyInjection] Invokable Factory Services --- .../Component/DependencyInjection/Definition.php | 8 ++++++-- .../DependencyInjection/Loader/XmlFileLoader.php | 4 ++-- .../DependencyInjection/Loader/YamlFileLoader.php | 5 ++++- .../DependencyInjection/Tests/DefinitionTest.php | 4 ++++ .../Tests/Fixtures/xml/services6.xml | 3 +++ .../Tests/Fixtures/yaml/bad_factory_syntax.yml | 6 ++++++ .../Tests/Fixtures/yaml/services14.yml | 1 + .../Tests/Fixtures/yaml/services6.yml | 1 + .../Tests/Loader/XmlFileLoaderTest.php | 1 + .../Tests/Loader/YamlFileLoaderTest.php | 12 ++++++++++++ 10 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_factory_syntax.yml diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index b79a8c9a82..57b328d419 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -95,7 +95,7 @@ class Definition /** * Sets a factory. * - * @param string|array $factory A PHP function or an array containing a class/Reference and a method to call + * @param string|array|Reference $factory A PHP function, reference or an array containing a class/Reference and a method to call * * @return $this */ @@ -105,6 +105,8 @@ class Definition if (\is_string($factory) && false !== strpos($factory, '::')) { $factory = explode('::', $factory, 2); + } elseif ($factory instanceof Reference) { + $factory = [$factory, '__invoke']; } $this->factory = $factory; @@ -782,7 +784,7 @@ class Definition /** * Sets a configurator to call after the service is fully initialized. * - * @param string|array $configurator A PHP callable + * @param string|array|Reference $configurator A PHP function, reference or an array containing a class/Reference and a method to call * * @return $this */ @@ -792,6 +794,8 @@ class Definition if (\is_string($configurator) && false !== strpos($configurator, '::')) { $configurator = explode('::', $configurator, 2); + } elseif ($configurator instanceof Reference) { + $configurator = [$configurator, '__invoke']; } $this->configurator = $configurator; diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 6f672bdc7f..25777a62ac 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -317,7 +317,7 @@ class XmlFileLoader extends FileLoader $class = $factory->hasAttribute('class') ? $factory->getAttribute('class') : null; } - $definition->setFactory([$class, $factory->getAttribute('method')]); + $definition->setFactory([$class, $factory->getAttribute('method') ?: '__invoke']); } } @@ -332,7 +332,7 @@ class XmlFileLoader extends FileLoader $class = $configurator->getAttribute('class'); } - $definition->setConfigurator([$class, $configurator->getAttribute('method')]); + $definition->setConfigurator([$class, $configurator->getAttribute('method') ?: '__invoke']); } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 06fcbb4a91..93080a420a 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -571,12 +571,15 @@ class YamlFileLoader extends FileLoader * * @throws InvalidArgumentException When errors occur * - * @return string|array A parsed callable + * @return string|array|Reference A parsed callable */ private function parseCallable($callable, $parameter, $id, $file) { if (\is_string($callable)) { if ('' !== $callable && '@' === $callable[0]) { + if (false === strpos($callable, ':')) { + return [$this->resolveServices($callable, $file), '__invoke']; + } throw new InvalidArgumentException(sprintf('The value of the "%s" option for the "%s" service must be the id of the service without the "@" prefix (replace "%s" with "%s").', $parameter, $id, $callable, substr($callable, 1))); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index 3462726943..4308301823 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; class DefinitionTest extends TestCase { @@ -35,6 +36,9 @@ class DefinitionTest extends TestCase $def->setFactory('Foo::bar'); $this->assertEquals(['Foo', 'bar'], $def->getFactory(), '->setFactory() converts string static method call to the array'); + + $def->setFactory($ref = new Reference('baz')); + $this->assertSame([$ref, '__invoke'], $def->getFactory(), '->setFactory() converts service reference to class invoke call'); $this->assertSame(['factory' => true], $def->getChanges()); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml index c85b7a7c01..cab8939a50 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml @@ -59,6 +59,9 @@ + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_factory_syntax.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_factory_syntax.yml new file mode 100644 index 0000000000..aa728d3c59 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/bad_factory_syntax.yml @@ -0,0 +1,6 @@ +services: + factory: + class: Baz + invalid_factory: + class: FooBarClass + factory: '@factory:method' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml index 4c188c5fbc..5d64a6537b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml @@ -1,3 +1,4 @@ services: factory: { class: FooBarClass, factory: baz:getClass} factory_with_static_call: { class: FooBarClass, factory: FooBacFactory::createFooBar} + invokable_factory: { class: FooBarClass, factory: '@factory' } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index 1ee6c6ec74..2a82b181ea 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -34,6 +34,7 @@ services: new_factory2: { class: FooBarClass, factory: ['@baz', getClass]} new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]} new_factory4: { class: BazClass, factory: [~, getInstance]} + new_factory5: { class: FooBarClass, factory: '@baz' } Acme\WithShortCutArgs: [foo, '@baz'] alias_for_foo: '@foo' another_alias_for_foo: diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 20c80258e2..6f04a519a2 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -268,6 +268,7 @@ class XmlFileLoaderTest extends TestCase $this->assertEquals([new Reference('baz'), 'getClass'], $services['new_factory2']->getFactory(), '->load() parses the factory tag'); $this->assertEquals(['BazClass', 'getInstance'], $services['new_factory3']->getFactory(), '->load() parses the factory tag'); $this->assertSame([null, 'getInstance'], $services['new_factory4']->getFactory(), '->load() accepts factory tag without class'); + $this->assertEquals([new Reference('baz'), '__invoke'], $services['new_factory5']->getFactory(), '->load() accepts service reference as invokable factory'); $aliases = $container->getAliases(); $this->assertArrayHasKey('alias_for_foo', $aliases, '->load() parses elements'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 7870a521a9..afbb4485d0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -20,6 +20,7 @@ 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\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Loader\IniFileLoader; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; @@ -158,6 +159,7 @@ class YamlFileLoaderTest extends TestCase $this->assertEquals([new Reference('baz'), 'getClass'], $services['new_factory2']->getFactory(), '->load() parses the factory tag'); $this->assertEquals(['BazClass', 'getInstance'], $services['new_factory3']->getFactory(), '->load() parses the factory tag'); $this->assertSame([null, 'getInstance'], $services['new_factory4']->getFactory(), '->load() accepts factory tag without class'); + $this->assertEquals([new Reference('baz'), '__invoke'], $services['new_factory5']->getFactory(), '->load() accepts service reference as invokable factory'); $this->assertEquals(['foo', new Reference('baz')], $services['Acme\WithShortCutArgs']->getArguments(), '->load() parses short service definition'); $aliases = $container->getAliases(); @@ -196,6 +198,16 @@ class YamlFileLoaderTest extends TestCase $this->assertEquals([new Reference('baz'), 'getClass'], $services['factory']->getFactory(), '->load() parses the factory tag with service:method'); $this->assertEquals(['FooBacFactory', 'createFooBar'], $services['factory_with_static_call']->getFactory(), '->load() parses the factory tag with Class::method'); + $this->assertEquals([new Reference('factory'), '__invoke'], $services['invokable_factory']->getFactory(), '->load() parses string service reference'); + } + + public function testFactorySyntaxError() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The value of the "factory" option for the "invalid_factory" service must be the id of the service without the "@" prefix (replace "@factory:method" with "factory:method").'); + $loader->load('bad_factory_syntax.yml'); } public function testLoadConfiguratorShortSyntax()