diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 58dcbf7920854..ad32230ff59fe 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 3.3.0 ----- + * added support for omitting the factory class name in a service definition if the definition class is set * deprecated case insensitivity of service identifiers * added "iterator" argument type for lazy iteration over a set of values and services * added "closure-proxy" argument type for turning services' methods into lazy callables diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index a7c2fab884f67..eddecfeb4c365 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -50,6 +50,7 @@ public function __construct() new ResolveDefinitionTemplatesPass(), new DecoratorServicePass(), new ResolveParameterPlaceHoldersPass(), + new ResolveFactoryClassPass(), new FactoryReturnTypePass($resolveClassPass), new CheckDefinitionValidityPass(), new ResolveReferencesToAliasesPass(), diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveFactoryClassPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveFactoryClassPass.php new file mode 100644 index 0000000000000..c41cf973fe620 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveFactoryClassPass.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; + +/** + * @author Maxime Steinhausser + */ +class ResolveFactoryClassPass extends AbstractRecursivePass +{ + /** + * {@inheritdoc} + */ + protected function processValue($value, $isRoot = false) + { + if ($value instanceof Definition && is_array($factory = $value->getFactory()) && null === $factory[0]) { + if (null === $class = $value->getClass()) { + throw new RuntimeException(sprintf('The "%s" service is defined to be created by a factory, but is missing the factory class. Did you forget to define the factory or service class?', $this->currentId)); + } + + $factory[0] = $class; + $value->setFactory($factory); + } + + return parent::processValue($value, $isRoot); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php index e9060930b1f1a..e067fb931d4b9 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php @@ -176,7 +176,9 @@ private function addService($definition, $id, \DOMElement $parent) $this->addService($callable[0], null, $factory); $factory->setAttribute('method', $callable[1]); } elseif (is_array($callable)) { - $factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]); + if (null !== $callable[0]) { + $factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]); + } $factory->setAttribute('method', $callable[1]); } else { $factory->setAttribute('function', $callable); diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 46b1fea1c47da..d428c1399b871 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -257,7 +257,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = } elseif ($childService = $factory->getAttribute('service')) { $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false); } else { - $class = $factory->getAttribute('class'); + $class = $factory->hasAttribute('class') ? $factory->getAttribute('class') : null; } $definition->setFactory(array($class, $factory->getAttribute('method'))); diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 9cdc9587c97af..df7d959cbef55 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -466,6 +466,10 @@ private function parseCallable($callable, $parameter, $id, $file) return array($this->resolveServices($callable[0]), $callable[1]); } + if ('factory' === $parameter && isset($callable[1]) && null === $callable[0]) { + return $callable; + } + 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)); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveFactoryClassPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveFactoryClassPassTest.php new file mode 100644 index 0000000000000..e3ce57e248f4f --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveFactoryClassPassTest.php @@ -0,0 +1,87 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Tests\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\ResolveFactoryClassPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; + +class ResolveFactoryClassPassTest extends \PHPUnit_Framework_TestCase +{ + public function testProcess() + { + $container = new ContainerBuilder(); + + $factory = $container->register('factory', 'Foo\Bar'); + $factory->setFactory(array(null, 'create')); + + $pass = new ResolveFactoryClassPass(); + $pass->process($container); + + $this->assertSame(array('Foo\Bar', 'create'), $factory->getFactory()); + } + + public function testInlinedDefinitionFactoryIsProcessed() + { + $container = new ContainerBuilder(); + + $factory = $container->register('factory'); + $factory->setFactory(array((new Definition('Baz\Qux'))->setFactory(array(null, 'getInstance')), 'create')); + + $pass = new ResolveFactoryClassPass(); + $pass->process($container); + + $this->assertSame(array('Baz\Qux', 'getInstance'), $factory->getFactory()[0]->getFactory()); + } + + public function provideFulfilledFactories() + { + return array( + array(array('Foo\Bar', 'create')), + array(array(new Reference('foo'), 'create')), + array(array(new Definition('Baz'), 'create')), + ); + } + + /** + * @dataProvider provideFulfilledFactories + */ + public function testIgnoresFulfilledFactories($factory) + { + $container = new ContainerBuilder(); + $definition = new Definition(); + $definition->setFactory($factory); + + $container->setDefinition('factory', $definition); + + $pass = new ResolveFactoryClassPass(); + $pass->process($container); + + $this->assertSame($factory, $container->getDefinition('factory')->getFactory()); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage The "factory" service is defined to be created by a factory, but is missing the factory class. Did you forget to define the factory or service class? + */ + public function testNotAnyClassThrowsException() + { + $container = new ContainerBuilder(); + + $factory = $container->register('factory'); + $factory->setFactory(array(null, 'create')); + + $pass = new ResolveFactoryClassPass(); + $pass->process($container); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml index 70981ff5450b2..5134ff7671f11 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml @@ -58,5 +58,8 @@ + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index 79810050ca311..1b8dc7d81d7c5 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -39,4 +39,5 @@ services: new_factory1: { class: FooBarClass, factory: factory} new_factory2: { class: FooBarClass, factory: ['@baz', getClass]} new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]} + new_factory4: { class: BazClass, factory: [~, getInstance]} with_shortcut_args: [foo, '@baz'] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 7ff6086a432db..a1b95616805e4 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -246,6 +246,7 @@ public function testLoadServices() $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->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class'); $aliases = $container->getAliases(); $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses elements'); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 1cf02f7d405c7..26937cd7168ee 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -153,6 +153,7 @@ public function testLoadServices() $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->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class'); $this->assertEquals(array('foo', new Reference('baz')), $services['with_shortcut_args']->getArguments(), '->load() parses short service definition'); $aliases = $container->getAliases();