From bd8531d2d893d154f2f639657afa828254c52d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rouven=20We=C3=9Fling?= Date: Sun, 13 Apr 2014 17:18:39 +0200 Subject: [PATCH 1/2] added a new Syntax to define factories as callables. --- UPGRADE-3.0.md | 5 +++ .../DependencyInjection/CHANGELOG.md | 5 +++ .../Compiler/AnalyzeServiceReferencesPass.php | 3 ++ .../Compiler/CheckDefinitionValidityPass.php | 9 ++++- .../Compiler/InlineServiceDefinitionsPass.php | 5 +++ .../ResolveDefinitionTemplatesPass.php | 3 ++ .../DependencyInjection/Definition.php | 35 ++++++++++++++++ .../DefinitionDecorator.php | 12 ++++++ .../DependencyInjection/Dumper/PhpDumper.php | 36 ++++++++++++++++- .../DependencyInjection/Dumper/XmlDumper.php | 11 +++++ .../DependencyInjection/Dumper/YamlDumper.php | 34 +++++++++++----- .../Loader/XmlFileLoader.php | 15 +++++++ .../Loader/YamlFileLoader.php | 13 ++++++ .../schema/dic/services/services-1.0.xsd | 5 ++- .../Component/DependencyInjection/README.md | 3 +- .../CheckDefinitionValidityPassTest.php | 11 +++++ .../Tests/DefinitionDecoratorTest.php | 1 + .../Tests/DefinitionTest.php | 11 +++++ .../Tests/Fixtures/containers/container9.php | 11 +++++ .../Tests/Fixtures/graphviz/services9.dot | 2 + .../Tests/Fixtures/php/services9.php | 40 +++++++++++++++++++ .../Tests/Fixtures/php/services9_compiled.php | 21 ++++++++++ .../Tests/Fixtures/xml/services6.xml | 9 +++++ .../Tests/Fixtures/xml/services9.xml | 7 ++++ .../Tests/Fixtures/yaml/services14.yml | 2 + .../Tests/Fixtures/yaml/services6.yml | 3 ++ .../Tests/Fixtures/yaml/services9.yml | 8 ++++ .../Tests/Loader/XmlFileLoaderTest.php | 3 ++ .../Tests/Loader/YamlFileLoaderTest.php | 13 ++++++ 29 files changed, 320 insertions(+), 16 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 38156e47f675a..e108ceeed9d0e 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -86,6 +86,11 @@ UPGRADE FROM 2.x to 3.0 $table->render(); ``` +### DependencyInjection + + * The methods `setFactoryClass()`, `setFactoryMethod()` and `setFactoryService()` have been removed in favor of `setFactory()`. + Services defined using YAML or XML use the same syntax as configurators. + ### EventDispatcher * The interface `Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcherInterface` diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 5a88e34e47d50..b78e440974b2c 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +2.6.0 +----- + + * added new factory syntax and deprecated the old one + 2.5.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php index f488052596a90..2cc7aad5b2c2e 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php @@ -81,6 +81,9 @@ public function process(ContainerBuilder $container) if ($definition->getConfigurator()) { $this->processArguments(array($definition->getConfigurator())); } + if ($definition->getFactory()) { + $this->processArguments(array($definition->getFactory())); + } } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php index c8978f377f44c..fd14dabb919bb 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php @@ -56,9 +56,16 @@ public function process(ContainerBuilder $container) )); } + if ($definition->getFactory() && ($definition->getFactoryClass() || $definition->getFactoryService() || $definition->getFactoryMethod())) { + throw new RuntimeException(sprintf( + 'A service ("%s") can use either the old or the new factory syntax, not both.', + $id + )); + } + // non-synthetic, non-abstract service has class if (!$definition->isAbstract() && !$definition->isSynthetic() && !$definition->getClass()) { - if ($definition->getFactoryClass() || $definition->getFactoryService()) { + if ($definition->getFactory() || $definition->getFactoryClass() || $definition->getFactoryService()) { throw new RuntimeException(sprintf( 'Please add the class to service "%s" even if it is constructed by a factory ' .'since we might need to add method calls based on compile-time checks.', diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index d2d3599046456..32b506f3eeda0 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -67,6 +67,11 @@ public function process(ContainerBuilder $container) $definition->setConfigurator( $configurator[0] ); + + $factory = $this->inlineArguments($container, array($definition->getFactory())); + $definition->setFactory( + $factory[0] + ); } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php index 4527840e73f0b..abda1874c9b60 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php @@ -103,6 +103,9 @@ private function resolveDefinition($id, DefinitionDecorator $definition) if (isset($changes['factory_service'])) { $def->setFactoryService($definition->getFactoryService()); } + if (isset($changes['factory'])) { + $def->setFactory($definition->getFactory()); + } if (isset($changes['configurator'])) { $def->setConfigurator($definition->getConfigurator()); } diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index f83c069c63906..85408764fd909 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -25,6 +25,7 @@ class Definition { private $class; private $file; + private $factory; private $factoryClass; private $factoryMethod; private $factoryService; @@ -56,6 +57,34 @@ public function __construct($class = null, array $arguments = array()) $this->arguments = $arguments; } + /** + * Sets a factory + * + * @param callable $factory The PHP callable to call or an array containing a Reference and a method to call + * + * @return Definition The current instance + * + * @api + */ + public function setFactory($factory) + { + $this->factory = $factory; + + return $this; + } + + /** + * Gets the factory . + * + * @return callable|array The PHP callable to call or an array containing a Reference and a method to call + * + * @api + */ + public function getFactory() + { + return $this->factory; + } + /** * Sets the name of the class that acts as a factory using the factory method, * which will be invoked statically. @@ -65,6 +94,7 @@ public function __construct($class = null, array $arguments = array()) * @return Definition The current instance * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function setFactoryClass($factoryClass) { @@ -79,6 +109,7 @@ public function setFactoryClass($factoryClass) * @return string|null The factory class name * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function getFactoryClass() { @@ -93,6 +124,7 @@ public function getFactoryClass() * @return Definition The current instance * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function setFactoryMethod($factoryMethod) { @@ -142,6 +174,7 @@ public function getDecoratedService() * @return string|null The factory method name * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function getFactoryMethod() { @@ -156,6 +189,7 @@ public function getFactoryMethod() * @return Definition The current instance * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function setFactoryService($factoryService) { @@ -170,6 +204,7 @@ public function setFactoryService($factoryService) * @return string|null The factory service id * * @api + * @deprecated Deprecated since version 2.5, to be removed in 3.0. */ public function getFactoryService() { diff --git a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php index 497c394dffedf..60e11a37bf17a 100644 --- a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php +++ b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php @@ -81,6 +81,18 @@ public function setClass($class) * * @api */ + public function setFactory($callable) + { + $this->changes['factory'] = true; + + return parent::setFactory($callable); + } + + /** + * {@inheritDoc} + * + * @api + */ public function setFactoryClass($class) { $this->changes['factory_class'] = true; diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 8fb3791627930..fbd284d4b70c6 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -161,6 +161,7 @@ private function addServiceLocalTempVariables($cId, $definition) $this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior); $this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior); $this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior); + $this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior); } $code = ''; @@ -523,6 +524,17 @@ private function addService($id, $definition) $return[] = '@throws RuntimeException always since this service is expected to be injected dynamically'; } elseif ($class = $definition->getClass()) { $return[] = sprintf("@return %s A %s instance.", 0 === strpos($class, '%') ? 'object' : "\\".$class, $class); + } elseif ($definition->getFactory()) { + $factory = $definition->getFactory(); + if (is_string($factory)) { + $return[] = sprintf('@return object An instance returned by %s().', $factory); + } elseif (is_array($factory) && (is_string($factory[0]) || $factory[0] instanceof Definition || $factory[0] instanceof Reference)) { + if (is_string($factory[0] || $factory[0] instanceof Reference)) { + $return[] = sprintf('@return object An instance returned by %s::%s().', (string) $factory[0], $factory[1]); + } elseif ($factory[0] instanceof Definition) { + $return[] = sprintf('@return object An instance returned by %s::%s().', $factory[0]->getClass(), $factory[1]); + } + } } elseif ($definition->getFactoryClass()) { $return[] = sprintf('@return object An instance returned by %s::%s().', $definition->getFactoryClass(), $definition->getFactoryMethod()); } elseif ($definition->getFactoryService()) { @@ -701,7 +713,26 @@ private function addNewInstance($id, Definition $definition, $return, $instantia $arguments[] = $this->dumpValue($value); } - if (null !== $definition->getFactoryMethod()) { + if (null !== $definition->getFactory()) { + $callable = $definition->getFactory(); + if (is_array($callable)) { + if ($callable[0] instanceof Reference + || ($callable[0] instanceof Definition && $this->definitionVariables->contains($callable[0]))) { + return sprintf(" $return{$instantiation}%s->%s(%s);\n", $this->dumpValue($callable[0]), $callable[1], $arguments ? implode(', ', $arguments) : ''); + } + + $class = $this->dumpValue($callable[0]); + // If the class is a string we can optimize call_user_func away + if (strpos($class, "'") === 0) { + return sprintf(" $return{$instantiation}\%s::%s(%s);\n", substr($class, 1, -1), $callable[1], $arguments ? implode(', ', $arguments) : ''); + } + + return sprintf(" $return{$instantiation}call_user_func(array(%s, '%s'), %s);\n", $this->dumpValue($callable[0]), $callable[1], $arguments ? ', '.implode(', ', $arguments) : ''); + } + + return sprintf(" $return{$instantiation}%s(%s);\n", $callable, $arguments ? implode(', ', $arguments) : ''); + + } elseif (null !== $definition->getFactoryMethod()) { if (null !== $definition->getFactoryClass()) { $class = $this->dumpValue($definition->getFactoryClass()); @@ -1116,7 +1147,8 @@ private function getInlinedDefinitions(Definition $definition) $this->getDefinitionsFromArguments($definition->getArguments()), $this->getDefinitionsFromArguments($definition->getMethodCalls()), $this->getDefinitionsFromArguments($definition->getProperties()), - $this->getDefinitionsFromArguments(array($definition->getConfigurator())) + $this->getDefinitionsFromArguments(array($definition->getConfigurator())), + $this->getDefinitionsFromArguments(array($definition->getFactory())) ); $this->inlinedDefinitions->offsetSet($definition, $definitions); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php index e3a533eed2783..2f120c18b7476 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php @@ -176,6 +176,17 @@ private function addService($definition, $id, \DOMElement $parent) $this->addMethodCalls($definition->getMethodCalls(), $service); + if ($callable = $definition->getFactory()) { + $factory = $this->document->createElement('factory'); + if (is_array($callable)) { + $factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]); + $factory->setAttribute('method', $callable[1]); + } else { + $factory->setAttribute('function', $callable); + } + $service->appendChild($factory); + } + if ($callable = $definition->getConfigurator()) { $configurator = $this->document->createElement('configurator'); if (is_array($callable)) { diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index 613620c2333ab..f96416e1cbaaa 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -147,16 +147,12 @@ private function addService($id, $definition) } } - if ($callable = $definition->getConfigurator()) { - if (is_array($callable)) { - if ($callable[0] instanceof Reference) { - $callable = array($this->getServiceCall((string) $callable[0], $callable[0]), $callable[1]); - } else { - $callable = array($callable[0], $callable[1]); - } - } + if ($callable = $definition->getFactory()) { + $code .= sprintf(" factory: %s\n", $this->dumper->dump($this->dumpCallable($callable), 0)); + } - $code .= sprintf(" configurator: %s\n", $this->dumper->dump($callable, 0)); + if ($callable = $definition->getConfigurator()) { + $code .= sprintf(" configurator: %s\n", $this->dumper->dump($this->dumpCallable($callable), 0)); } return $code; @@ -222,6 +218,26 @@ private function addParameters() return $this->dumper->dump(array('parameters' => $parameters), 2); } + /** + * Dumps callable to YAML format + * + * @param callable $callable + * + * @return callable + */ + private function dumpCallable($callable) + { + if (is_array($callable)) { + if ($callable[0] instanceof Reference) { + $callable = array($this->getServiceCall((string) $callable[0], $callable[0]), $callable[1]); + } else { + $callable = array($callable[0], $callable[1]); + } + } + + return $callable; + } + /** * Dumps the value to YAML format * diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 32d2bb19ba40e..658b364084a04 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -167,6 +167,21 @@ private function parseDefinition($id, \DOMElement $service, $file) $definition->setArguments($this->getArgumentsAsPhp($service, 'argument')); $definition->setProperties($this->getArgumentsAsPhp($service, 'property')); + if ($factories = $this->getChildren($service, 'factory')) { + $factory = $factories[0]; + if ($function = $factory->getAttribute('function')) { + $definition->setFactory($function); + } else { + if ($childService = $factory->getAttribute('service')) { + $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false); + } else { + $class = $factory->getAttribute('class'); + } + + $definition->setFactory(array($class, $factory->getAttribute('method'))); + } + } + if ($configurators = $this->getChildren($service, 'configurator')) { $configurator = $configurators[0]; if ($function = $configurator->getAttribute('function')) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 39bdf94887243..a1f940200bc86 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -194,6 +194,19 @@ private function parseDefinition($id, $service, $file) $definition->setAbstract($service['abstract']); } + if (isset($service['factory'])) { + if (is_string($service['factory'])) { + if (strpos($service['factory'], ':')) { + $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])); + } + } + if (isset($service['factory_class'])) { $definition->setFactoryClass($service['factory_class']); } 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 54a4544ad50d3..966f4de07fe43 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 @@ -64,7 +64,7 @@ - + @@ -76,7 +76,8 @@ - + + diff --git a/src/Symfony/Component/DependencyInjection/README.md b/src/Symfony/Component/DependencyInjection/README.md index 29d1cd0aa0c5d..e4332613d3375 100644 --- a/src/Symfony/Component/DependencyInjection/README.md +++ b/src/Symfony/Component/DependencyInjection/README.md @@ -38,8 +38,7 @@ If your service is retrieved by calling a static method: $sc ->register('bar', '%bar.class%') - ->setFactoryClass('%bar.class%') - ->setFactoryMethod('getInstance') + ->setFactory(array('%bar.class%', 'getInstance')) ->addArgument('Aarrg!!!') ; $sc->setParameter('bar.class', 'Bar'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckDefinitionValidityPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckDefinitionValidityPassTest.php index b5e49842e7af4..ed04a8b18ce5a 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckDefinitionValidityPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckDefinitionValidityPassTest.php @@ -50,6 +50,17 @@ public function testProcessDetectsNonSyntheticNonAbstractDefinitionWithoutClass( $this->process($container); } + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + */ + public function testProcessDetectsBothFactorySyntaxesUsed() + { + $container = new ContainerBuilder(); + $container->register('a')->setFactory(array('a', 'b'))->setFactoryClass('a'); + + $this->process($container); + } + public function testProcess() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionDecoratorTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionDecoratorTest.php index a266e9b31a4e0..cea67408f0ddb 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionDecoratorTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionDecoratorTest.php @@ -43,6 +43,7 @@ public function getPropertyTests() { return array( array('class', 'class'), + array('factory', 'factory'), array('factoryClass', 'factory_class'), array('factoryMethod', 'factory_method'), array('factoryService', 'factory_service'), diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index 9b4cf21ff90c7..dfe8a03bf6aac 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -27,6 +27,17 @@ public function testConstructor() $this->assertEquals(array('foo'), $def->getArguments(), '__construct() takes an optional array of arguments as its second argument'); } + /** + * @covers Symfony\Component\DependencyInjection\Definition::setFactory + * @covers Symfony\Component\DependencyInjection\Definition::getFactory + */ + public function testSetGetFactory() + { + $def = new Definition('stdClass'); + $this->assertSame($def, $def->setFactory('foo'), '->setFactory() implements a fluent interface'); + $this->assertEquals('foo', $def->getFactory(), '->getFactory() returns the factory'); + } + public function testSetGetFactoryClass() { $def = new Definition('stdClass'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php index 15ccc03d0c44f..a34a8b580d414 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php @@ -103,5 +103,16 @@ ->register('decorator_service_with_name', 'stdClass') ->setDecoratedService('decorated', 'decorated.pif-pouf') ; +$container + ->register('new_factory', 'FactoryClass') + ->setProperty('foo', 'bar') + ->setScope('container') + ->setPublic(false) +; +$container + ->register('new_factory_service', 'FooBarBaz') + ->setProperty('foo', 'bar') + ->setFactory(array(new Reference('new_factory'), 'getInstance')) +; return $container; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot index 6bf53e56b65d5..f4f498df2adae 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot @@ -19,6 +19,8 @@ digraph sc { node_decorated [label="decorated\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"]; node_decorator_service [label="decorator_service\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"]; node_decorator_service_with_name [label="decorator_service_with_name\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"]; + node_new_factory [label="new_factory\nFactoryClass\n", shape=record, fillcolor="#eeeeee", style="filled"]; + node_new_factory_service [label="new_factory_service\nFooBarBaz\n", shape=record, fillcolor="#eeeeee", style="filled"]; node_service_container [label="service_container\nSymfony\\Component\\DependencyInjection\\ContainerBuilder\n", shape=record, fillcolor="#9999ff", style="filled"]; node_foo2 [label="foo2\n\n", shape=record, fillcolor="#ff9999", style="filled"]; node_foo3 [label="foo3\n\n", shape=record, fillcolor="#ff9999", style="filled"]; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php index c7f4ddc967b0e..4ca34ef9ae612 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php @@ -38,6 +38,8 @@ public function __construct() 'foo_with_inline' => 'getFooWithInlineService', 'inlined' => 'getInlinedService', 'method_call1' => 'getMethodCall1Service', + 'new_factory' => 'getNewFactoryService', + 'new_factory_service' => 'getNewFactoryServiceService', 'request' => 'getRequestService', ); $this->aliases = array( @@ -265,6 +267,23 @@ protected function getMethodCall1Service() return $instance; } + /** + * Gets the 'new_factory_service' service. + * + * This service is shared. + * This method always returns the same instance of the service. + * + * @return \FooBarBaz A FooBarBaz instance. + */ + protected function getNewFactoryServiceService() + { + $this->services['new_factory_service'] = $instance = $this->get('new_factory')->getInstance(); + + $instance->foo = 'bar'; + + return $instance; + } + /** * Gets the 'request' service. * @@ -331,6 +350,27 @@ protected function getInlinedService() return $instance; } + /** + * Gets the 'new_factory' service. + * + * This service is shared. + * This method always returns the same instance of the service. + * + * This service is private. + * If you want to be able to request this service from the container directly, + * make it public, otherwise you might end up with broken code. + * + * @return \FactoryClass A FactoryClass instance. + */ + protected function getNewFactoryService() + { + $this->services['new_factory'] = $instance = new \FactoryClass(); + + $instance->foo = 'bar'; + + return $instance; + } + /** * Gets the default parameters. * diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index f0d54b6ecc3d1..bf47e8216ce84 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -46,6 +46,7 @@ public function __construct() 'foo_bar' => 'getFooBarService', 'foo_with_inline' => 'getFooWithInlineService', 'method_call1' => 'getMethodCall1Service', + 'new_factory_service' => 'getNewFactoryServiceService', 'request' => 'getRequestService', ); $this->aliases = array( @@ -269,6 +270,26 @@ protected function getMethodCall1Service() return $instance; } + /** + * Gets the 'new_factory_service' service. + * + * This service is shared. + * This method always returns the same instance of the service. + * + * @return \FooBarBaz A FooBarBaz instance. + */ + protected function getNewFactoryServiceService() + { + $a = new \FactoryClass(); + $a->foo = 'bar'; + + $this->services['new_factory_service'] = $instance = $a->getInstance(); + + $instance->foo = 'bar'; + + return $instance; + } + /** * Gets the 'request' service. * diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml index 7e2753eb7c73f..b896512fbf0e0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml @@ -52,5 +52,14 @@ + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml index d817334ca347c..2a9abde984516 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml @@ -91,6 +91,13 @@ + + bar + + + bar + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml new file mode 100644 index 0000000000000..a6f825e42e318 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services14.yml @@ -0,0 +1,2 @@ +services: + factory: { class: FooBarClass, factory: baz:getClass} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index 885b6c327a6fd..04828646cb988 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -35,3 +35,6 @@ services: decorator_service_with_name: decorates: decorated decoration_inner_name: decorated.pif-pouf + new_factory1: { class: FooBarClass, factory: factory} + new_factory2: { class: FooBarClass, factory: [@baz, getClass]} + new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml index 0422be8dd7886..178c1c92d855a 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml @@ -88,5 +88,13 @@ services: class: stdClass decorates: decorated decoration_inner_name: decorated.pif-pouf + new_factory: + class: FactoryClass + public: false + properties: { foo: bar } + new_factory_service: + class: FooBarBaz + properties: { foo: bar } + factory: ['@new_factory', getInstance] alias_for_foo: @foo alias_for_alias: @foo diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 5a9a905cd8e55..002f87ea7b297 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -216,6 +216,9 @@ public function testLoadServices() $this->assertNull($services['factory_service']->getClass()); $this->assertEquals('getInstance', $services['factory_service']->getFactoryMethod()); $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService()); + $this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag'); + $this->assertEquals(array(new Reference('baz', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag'); + $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag'); $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); $this->assertTrue($services['request']->isSynchronized(), '->load() parses the synchronized flag'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 6c893db936a20..40942f4dc7abb 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -141,6 +141,9 @@ public function testLoadServices() $this->assertEquals(array(array('setBar', array()), array('setBar', array()), array('setBar', array(new Expression('service("foo").foo() ~ parameter("foo")')))), $services['method_call1']->getMethodCalls(), '->load() parses the method_call tag'); $this->assertEquals(array(array('setBar', array('foo', new Reference('foo'), array(true, false)))), $services['method_call2']->getMethodCalls(), '->load() parses the method_call tag'); $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService()); + $this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag'); + $this->assertEquals(array(new Reference('baz'), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag'); + $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag'); $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); $this->assertTrue($services['request']->isSynchronized(), '->load() parses the synchronized flag'); @@ -159,6 +162,16 @@ public function testLoadServices() $this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService()); } + public function testLoadFactoryShortSyntax() + { + $container = new ContainerBuilder(); + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('services14.yml'); + $services = $container->getDefinitions(); + + $this->assertEquals(array(new Reference('baz'), 'getClass'), $services['factory']->getFactory(), '->load() parses the factory tag'); + } + public function testExtensions() { $container = new ContainerBuilder(); From 187aeeeaf790f16ec0344acf7d432f781994e58d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Wed, 24 Sep 2014 08:35:53 +0200 Subject: [PATCH 2/2] fixed CS --- UPGRADE-3.0.md | 6 +++-- .../Compiler/CheckDefinitionValidityPass.php | 21 ++++------------ .../Compiler/InlineServiceDefinitionsPass.php | 8 ++----- .../DependencyInjection/Definition.php | 24 ++++++++----------- .../DefinitionDecorator.php | 4 +--- .../DependencyInjection/Dumper/PhpDumper.php | 3 +-- .../Loader/YamlFileLoader.php | 2 +- 7 files changed, 23 insertions(+), 45 deletions(-) diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index e108ceeed9d0e..17417689ab0fe 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -88,8 +88,10 @@ UPGRADE FROM 2.x to 3.0 ### DependencyInjection - * The methods `setFactoryClass()`, `setFactoryMethod()` and `setFactoryService()` have been removed in favor of `setFactory()`. - Services defined using YAML or XML use the same syntax as configurators. + * The methods `Definition::setFactoryClass()`, + `Definition::setFactoryMethod()`, and `Definition::setFactoryService()` have + been removed in favor of `Definition::setFactory()`. Services defined using + YAML or XML use the same syntax as configurators. ### EventDispatcher diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php index fd14dabb919bb..ce89f24e183e3 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php @@ -42,35 +42,22 @@ public function process(ContainerBuilder $container) foreach ($container->getDefinitions() as $id => $definition) { // synthetic service is public if ($definition->isSynthetic() && !$definition->isPublic()) { - throw new RuntimeException(sprintf( - 'A synthetic service ("%s") must be public.', - $id - )); + throw new RuntimeException(sprintf('A synthetic service ("%s") must be public.', $id)); } // synthetic service has non-prototype scope if ($definition->isSynthetic() && ContainerInterface::SCOPE_PROTOTYPE === $definition->getScope()) { - throw new RuntimeException(sprintf( - 'A synthetic service ("%s") cannot be of scope "prototype".', - $id - )); + throw new RuntimeException(sprintf('A synthetic service ("%s") cannot be of scope "prototype".', $id)); } if ($definition->getFactory() && ($definition->getFactoryClass() || $definition->getFactoryService() || $definition->getFactoryMethod())) { - throw new RuntimeException(sprintf( - 'A service ("%s") can use either the old or the new factory syntax, not both.', - $id - )); + throw new RuntimeException(sprintf('A service ("%s") can use either the old or the new factory syntax, not both.', $id)); } // non-synthetic, non-abstract service has class if (!$definition->isAbstract() && !$definition->isSynthetic() && !$definition->getClass()) { if ($definition->getFactory() || $definition->getFactoryClass() || $definition->getFactoryService()) { - throw new RuntimeException(sprintf( - 'Please add the class to service "%s" even if it is constructed by a factory ' - .'since we might need to add method calls based on compile-time checks.', - $id - )); + throw new RuntimeException(sprintf('Please add the class to service "%s" even if it is constructed by a factory since we might need to add method calls based on compile-time checks.', $id)); } throw new RuntimeException(sprintf( diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index 32b506f3eeda0..6c529ee8bd04a 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -64,14 +64,10 @@ public function process(ContainerBuilder $container) ); $configurator = $this->inlineArguments($container, array($definition->getConfigurator())); - $definition->setConfigurator( - $configurator[0] - ); + $definition->setConfigurator($configurator[0]); $factory = $this->inlineArguments($container, array($definition->getFactory())); - $definition->setFactory( - $factory[0] - ); + $definition->setFactory($factory[0]); } } diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 85408764fd909..920fd3bd5766e 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -58,13 +58,11 @@ public function __construct($class = null, array $arguments = array()) } /** - * Sets a factory + * Sets a factory. * - * @param callable $factory The PHP callable to call or an array containing a Reference and a method to call + * @param string|array $factory A PHP function or an array containing a class/Reference and a method to call * * @return Definition The current instance - * - * @api */ public function setFactory($factory) { @@ -74,11 +72,9 @@ public function setFactory($factory) } /** - * Gets the factory . + * Gets the factory. * - * @return callable|array The PHP callable to call or an array containing a Reference and a method to call - * - * @api + * @return string|array The PHP function or an array containing a class/Reference and a method to call */ public function getFactory() { @@ -94,7 +90,7 @@ public function getFactory() * @return Definition The current instance * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function setFactoryClass($factoryClass) { @@ -109,7 +105,7 @@ public function setFactoryClass($factoryClass) * @return string|null The factory class name * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function getFactoryClass() { @@ -124,7 +120,7 @@ public function getFactoryClass() * @return Definition The current instance * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function setFactoryMethod($factoryMethod) { @@ -174,7 +170,7 @@ public function getDecoratedService() * @return string|null The factory method name * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function getFactoryMethod() { @@ -189,7 +185,7 @@ public function getFactoryMethod() * @return Definition The current instance * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function setFactoryService($factoryService) { @@ -204,7 +200,7 @@ public function setFactoryService($factoryService) * @return string|null The factory service id * * @api - * @deprecated Deprecated since version 2.5, to be removed in 3.0. + * @deprecated Deprecated since version 2.6, to be removed in 3.0. */ public function getFactoryService() { diff --git a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php index 60e11a37bf17a..390147ea06645 100644 --- a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php +++ b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php @@ -78,8 +78,6 @@ public function setClass($class) /** * {@inheritdoc} - * - * @api */ public function setFactory($callable) { @@ -89,7 +87,7 @@ public function setFactory($callable) } /** - * {@inheritDoc} + * {@inheritdoc} * * @api */ diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index fbd284d4b70c6..3e5de6cb8252c 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -529,7 +529,7 @@ private function addService($id, $definition) if (is_string($factory)) { $return[] = sprintf('@return object An instance returned by %s().', $factory); } elseif (is_array($factory) && (is_string($factory[0]) || $factory[0] instanceof Definition || $factory[0] instanceof Reference)) { - if (is_string($factory[0] || $factory[0] instanceof Reference)) { + if (is_string($factory[0]) || $factory[0] instanceof Reference) { $return[] = sprintf('@return object An instance returned by %s::%s().', (string) $factory[0], $factory[1]); } elseif ($factory[0] instanceof Definition) { $return[] = sprintf('@return object An instance returned by %s::%s().', $factory[0]->getClass(), $factory[1]); @@ -731,7 +731,6 @@ private function addNewInstance($id, Definition $definition, $return, $instantia } return sprintf(" $return{$instantiation}%s(%s);\n", $callable, $arguments ? implode(', ', $arguments) : ''); - } elseif (null !== $definition->getFactoryMethod()) { if (null !== $definition->getFactoryClass()) { $class = $this->dumpValue($definition->getFactoryClass()); diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index a1f940200bc86..202ac138edf36 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -196,7 +196,7 @@ private function parseDefinition($id, $service, $file) if (isset($service['factory'])) { if (is_string($service['factory'])) { - if (strpos($service['factory'], ':')) { + if (strpos($service['factory'], ':')) { $parts = explode(':', $service['factory']); $definition->setFactory(array($this->resolveServices('@'.$parts[0]), $parts[1])); } else {