From 84025d188cb67966fa4b71b39410b3ec113d18e2 Mon Sep 17 00:00:00 2001 From: Oleg Voronkovich Date: Sun, 26 Jun 2016 22:09:06 +0300 Subject: [PATCH 1/4] [DependencyInjection] Add support for short services configurators syntax --- .../Loader/YamlFileLoader.php | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 8d60d83216083..35aaab2eb5b88 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -223,16 +223,7 @@ private function parseDefinition($id, $service, $file) } if (isset($service['factory'])) { - if (is_string($service['factory'])) { - if (strpos($service['factory'], ':') !== false && strpos($service['factory'], '::') === false) { - $parts = explode(':', $service['factory']); - $definition->setFactory(array($this->resolveServices('@'.$parts[0]), $parts[1])); - } else { - $definition->setFactory($service['factory']); - } - } else { - $definition->setFactory(array($this->resolveServices($service['factory'][0]), $service['factory'][1])); - } + $definition->setFactory($this->parseCallable($service['factory'], 'factory', $id, $file)); } if (isset($service['file'])) { @@ -248,11 +239,7 @@ private function parseDefinition($id, $service, $file) } if (isset($service['configurator'])) { - if (is_string($service['configurator'])) { - $definition->setConfigurator($service['configurator']); - } else { - $definition->setConfigurator(array($this->resolveServices($service['configurator'][0]), $service['configurator'][1])); - } + $definition->setConfigurator($this->parseCallable($service['configurator'], 'configurator', $id, $file)); } if (isset($service['calls'])) { @@ -339,6 +326,39 @@ private function parseDefinition($id, $service, $file) $this->container->setDefinition($id, $definition); } + /** + * Parses a callable. + * + * @param string|array $callable A callable + * @param string $parameter A parameter (e.g. 'factory' or 'configurator') + * @param string $id A service identifier + * @param string $file A parsed file + * + * @throws InvalidArgumentException When errors are occuried + * + * @return string|array A parsed callable + */ + private function parseCallable($callable, $parameter, $id, $file) + { + if (is_string($callable)) { + if (false !== strpos($callable, ':') && false === strpos($callable, '::')) { + $parts = explode(':', $callable); + + return array($this->resolveServices('@'.$parts[0]), $parts[1]); + } + + return $callable; + } elseif (is_array($callable)) { + if (isset($callable[0]) && isset($callable[1])) { + return array($this->resolveServices($callable[0]), $callable[1]); + } + + throw new InvalidArgumentException(sprintf('Parameter "%s" must contain an array with two elements for service "%s" in %s. Check your YAML syntax.', $parameter, $id, $file)); + } + + throw new InvalidArgumentException(sprintf('Parameter "%s" must be a string or an array for service "%s" in %s. Check your YAML syntax.', $parameter, $id, $file)); + } + /** * Loads a YAML file. * From 23472ae5397e57aa2b711fbc848f6c933c8fa2ac Mon Sep 17 00:00:00 2001 From: Oleg Voronkovich Date: Mon, 27 Jun 2016 15:39:29 +0300 Subject: [PATCH 2/4] Use 'if' instead 'elseif' --- .../Component/DependencyInjection/Loader/YamlFileLoader.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 35aaab2eb5b88..3859b87b8b2da 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -348,7 +348,9 @@ private function parseCallable($callable, $parameter, $id, $file) } return $callable; - } elseif (is_array($callable)) { + } + + if (is_array($callable)) { if (isset($callable[0]) && isset($callable[1])) { return array($this->resolveServices($callable[0]), $callable[1]); } From 09848dd8a76480d13cee03e9794c26257c798ffe Mon Sep 17 00:00:00 2001 From: Oleg Voronkovich Date: Mon, 27 Jun 2016 16:10:14 +0300 Subject: [PATCH 3/4] Add a check for leading '@' --- .../Component/DependencyInjection/Loader/YamlFileLoader.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 3859b87b8b2da..98e78efce8a06 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -341,6 +341,10 @@ private function parseDefinition($id, $service, $file) private function parseCallable($callable, $parameter, $id, $file) { if (is_string($callable)) { + if ('' !== $callable && '@' === $callable[0]) { + 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))); + } + if (false !== strpos($callable, ':') && false === strpos($callable, '::')) { $parts = explode(':', $callable); From cb397cbf8efdb3f244f9cfa5a42c22daf7294dc3 Mon Sep 17 00:00:00 2001 From: Oleg Voronkovich Date: Mon, 27 Jun 2016 19:16:58 +0300 Subject: [PATCH 4/4] Add tests. Update Definition::setConfigurator method --- .../Component/DependencyInjection/Definition.php | 10 +++++++--- .../yaml/services_configurator_short_syntax.yml | 9 +++++++++ .../Tests/Loader/YamlFileLoaderTest.php | 11 +++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_configurator_short_syntax.yml diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 608e5825658d4..b697f02708e1f 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -612,13 +612,17 @@ public function getDeprecationMessage($id) /** * Sets a configurator to call after the service is fully initialized. * - * @param callable $callable A PHP callable + * @param string|array $configurator A PHP callable * * @return Definition The current instance */ - public function setConfigurator($callable) + public function setConfigurator($configurator) { - $this->configurator = $callable; + if (is_string($configurator) && strpos($configurator, '::') !== false) { + $configurator = explode('::', $configurator, 2); + } + + $this->configurator = $configurator; return $this; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_configurator_short_syntax.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_configurator_short_syntax.yml new file mode 100644 index 0000000000000..68e8137ec2cc7 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_configurator_short_syntax.yml @@ -0,0 +1,9 @@ +services: + + foo_bar: + class: FooBarClass + configurator: foo_bar_configurator:configure + + foo_bar_with_static_call: + class: FooBarClass + configurator: FooBarConfigurator::configureFooBar diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 16185a7216c31..a5a498f47eb7e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -165,6 +165,17 @@ public function testLoadFactoryShortSyntax() $this->assertEquals(array('FooBacFactory', 'createFooBar'), $services['factory_with_static_call']->getFactory(), '->load() parses the factory tag with Class::method'); } + public function testLoadConfiguratorShortSyntax() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('services_configurator_short_syntax.yml'); + $services = $container->getDefinitions(); + + $this->assertEquals(array(new Reference('foo_bar_configurator'), 'configure'), $services['foo_bar']->getConfigurator(), '->load() parses the configurator tag with service:method'); + $this->assertEquals(array('FooBarConfigurator', 'configureFooBar'), $services['foo_bar_with_static_call']->getConfigurator(), '->load() parses the configurator tag with Class::method'); + } + public function testExtensions() { $container = new ContainerBuilder();