[DependencyInjection] Create explicit factoryClass property for Definitions

Previously, the Definition class was used both for type inference and factory construction (if factoryService was absent). This is fine for cases where classes create instances of themselves (e.g. getInstance() or create()), but leads to ambiguity when we have a separate factory class.
This commit is contained in:
Jeremy Mikola 2011-02-02 12:22:27 -05:00 committed by Fabien Potencier
parent 575b75a9df
commit 743f25a287
19 changed files with 165 additions and 30 deletions

View File

@ -408,6 +408,7 @@ class DoctrineExtension extends AbstractDoctrineExtension
new Reference(sprintf('doctrine.orm.%s_configuration', $entityManager['name']))
);
$ormEmDef = new Definition('%doctrine.orm.entity_manager_class%', $ormEmArgs);
$ormEmDef->setFactoryClass('%doctrine.orm.entity_manager_class%');
$ormEmDef->setFactoryMethod('create');
$ormEmDef->addTag('doctrine.orm.entity_manager');
$container->setDefinition($entityManagerService, $ormEmDef);

View File

@ -157,6 +157,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.default_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());
@ -198,6 +199,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.default_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());
@ -239,6 +241,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.default_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());
@ -279,6 +282,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.default_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());
@ -313,6 +317,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.dm1_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());
@ -334,6 +339,7 @@ abstract class AbstractDoctrineExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.orm.dm2_entity_manager');
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.orm.entity_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.orm.entity_manager', $definition->getTags());

View File

@ -165,6 +165,7 @@ class DoctrineMongoDBExtension extends AbstractDoctrineExtension
new Reference($eventManagerId),
);
$odmDmDef = new Definition('%doctrine.odm.mongodb.document_manager_class%', $odmDmArgs);
$odmDmDef->setFactoryClass('%doctrine.odm.mongodb.document_manager_class%');
$odmDmDef->setFactoryMethod('create');
$odmDmDef->addTag('doctrine.odm.mongodb.document_manager');
$container->setDefinition(sprintf('doctrine.odm.mongodb.%s_document_manager', $documentManager['name']), $odmDmDef);

View File

@ -65,6 +65,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.default_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());
@ -92,6 +93,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.default_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());
@ -126,6 +128,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.default_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());
@ -154,6 +157,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.default_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());
@ -184,6 +188,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.dm1_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());
@ -199,6 +204,7 @@ abstract class AbstractMongoDBExtensionTest extends TestCase
$definition = $container->getDefinition('doctrine.odm.mongodb.dm2_document_manager');
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getClass());
$this->assertEquals('%doctrine.odm.mongodb.document_manager_class%', $definition->getFactoryClass());
$this->assertEquals('create', $definition->getFactoryMethod());
$this->assertArrayHasKey('doctrine.odm.mongodb.document_manager', $definition->getTags());

View File

@ -41,7 +41,7 @@ class CheckDefinitionValidityPass implements CompilerPassInterface
// non-synthetic, non-abstract service has class
if (!$definition->isAbstract() && !$definition->isSynthetic() && !$definition->getClass()) {
if ($definition->getFactoryService()) {
if ($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 '

View File

@ -49,8 +49,9 @@ class ResolveDefinitionTemplatesPass implements CompilerPassInterface
$def->setClass($parentDef->getClass());
$def->setArguments($parentDef->getArguments());
$def->setMethodCalls($parentDef->getMethodCalls());
$def->setFactoryService($parentDef->getFactoryService());
$def->setFactoryClass($parentDef->getFactoryClass());
$def->setFactoryMethod($parentDef->getFactoryMethod());
$def->setFactoryService($parentDef->getFactoryService());
$def->setConfigurator($parentDef->getConfigurator());
$def->setFile($parentDef->getFile());
$def->setPublic($parentDef->isPublic());
@ -60,6 +61,9 @@ class ResolveDefinitionTemplatesPass implements CompilerPassInterface
if (isset($changes['class'])) {
$def->setClass($definition->getClass());
}
if (isset($changes['factory_class'])) {
$def->setFactoryClass($definition->getFactoryClass());
}
if (isset($changes['factory_method'])) {
$def->setFactoryMethod($definition->getFactoryMethod());
}

View File

@ -32,7 +32,7 @@ class ResolveInterfaceInjectorsPass implements CompilerPassInterface
$loaded = false;
foreach ($container->getInterfaceInjectors() as $injector) {
if (null !== $definition->getFactoryService()) {
if (null !== $definition->getFactoryClass() || null !== $definition->getFactoryService()) {
continue;
}

View File

@ -695,10 +695,12 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
$arguments = $this->resolveServices($this->getParameterBag()->resolveValue($definition->getArguments()));
if (null !== $definition->getFactoryMethod()) {
if (null !== $definition->getFactoryService()) {
if (null !== $definition->getFactoryClass()) {
$factory = $this->getParameterBag()->resolveValue($definition->getFactoryClass());
} elseif (null !== $definition->getFactoryService()) {
$factory = $this->get($this->getParameterBag()->resolveValue($definition->getFactoryService()));
} else {
$factory = $this->getParameterBag()->resolveValue($definition->getClass());
throw new \RuntimeException('Cannot create service from factory method without a factory service or factory class.');
}
$service = call_user_func_array(array($factory, $definition->getFactoryMethod()), $arguments);

View File

@ -20,6 +20,7 @@ class Definition
{
protected $class;
protected $file;
protected $factoryClass;
protected $factoryMethod;
protected $factoryService;
protected $scope;
@ -50,15 +51,40 @@ class Definition
}
/**
* Sets the factory method able to create an instance of this class.
* Sets the name of the class that acts as a factory using the factory method,
* which will be invoked statically.
*
* @param string $method The method name
* @param string $factoryClass The factory class name
*
* @return Definition The current instance
*/
public function setFactoryMethod($method)
public function setFactoryClass($factoryClass)
{
$this->factoryMethod = $method;
$this->factoryClass = $factoryClass;
return $this;
}
/**
* Gets the factory class.
*
* @return string The factory class name
*/
public function getFactoryClass()
{
return $this->factoryClass;
}
/**
* Sets the factory method able to create an instance of this class.
*
* @param string $factoryMethod The factory method name
*
* @return Definition The current instance
*/
public function setFactoryMethod($factoryMethod)
{
$this->factoryMethod = $factoryMethod;
return $this;
}
@ -74,7 +100,7 @@ class Definition
}
/**
* Sets the name of the service that acts as a factory using the constructor method.
* Sets the name of the service that acts as a factory using the factory method.
*
* @param string $factoryService The factory service id
*

View File

@ -37,11 +37,11 @@ class DefinitionDecorator extends Definition
return parent::setClass($class);
}
public function setFactoryService($service)
public function setFactoryClass($class)
{
$this->changes['factory_service'] = true;
$this->changes['factory_class'] = true;
return parent::setFactoryService($service);
return parent::setFactoryClass($class);
}
public function setFactoryMethod($method)
@ -51,6 +51,13 @@ class DefinitionDecorator extends Definition
return parent::setFactoryMethod($method);
}
public function setFactoryService($service)
{
$this->changes['factory_service'] = true;
return parent::setFactoryService($service);
}
public function setConfigurator($callable)
{
$this->changes['configurator'] = true;

View File

@ -230,10 +230,12 @@ EOF;
}
if (null !== $sDefinition->getFactoryMethod()) {
if (null !== $sDefinition->getFactoryService()) {
if (null !== $sDefinition->getFactoryClass()) {
$code .= sprintf(" \$%s = call_user_func(array(%s, '%s')%s);\n", $name, $this->dumpValue($sDefinition->getFactoryClass()), $sDefinition->getFactoryMethod(), count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
} elseif (null !== $sDefinition->getFactoryService()) {
$code .= sprintf(" \$%s = %s->%s(%s);\n", $name, $this->getServiceCall($sDefinition->getFactoryService()), $sDefinition->getFactoryMethod(), implode(', ', $arguments));
} else {
$code .= sprintf(" \$%s = call_user_func(array(%s, '%s')%s);\n", $name, $class, $sDefinition->getFactoryMethod(), count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
throw new \RuntimeException('Factory service or factory class must be defined in service definition for '.$id);
}
} elseif (false !== strpos($class, '$')) {
$code .= sprintf(" \$class = %s;\n \$%s = new \$class(%s);\n", $class, $name, implode(', ', $arguments));
@ -294,10 +296,12 @@ EOF;
}
if (null !== $definition->getFactoryMethod()) {
if (null !== $definition->getFactoryService()) {
if (null !== $definition->getFactoryClass()) {
$code = sprintf(" $return{$instantiation}call_user_func(array(%s, '%s')%s);\n", $this->dumpValue($definition->getFactoryClass()), $definition->getFactoryMethod(), $arguments ? ', '.implode(', ', $arguments) : '');
} elseif (null !== $definition->getFactoryService()) {
$code = sprintf(" $return{$instantiation}%s->%s(%s);\n", $this->getServiceCall($definition->getFactoryService()), $definition->getFactoryMethod(), implode(', ', $arguments));
} else {
$code = sprintf(" $return{$instantiation}call_user_func(array(%s, '%s')%s);\n", $class, $definition->getFactoryMethod(), $arguments ? ', '.implode(', ', $arguments) : '');
throw new \RuntimeException('Factory method requires a factory service or factory class in service definition for '.$id);
}
} elseif (false !== strpos($class, '$')) {
$code = sprintf(" \$class = %s;\n $return{$instantiation}new \$class(%s);\n", $class, implode(', ', $arguments));
@ -404,8 +408,10 @@ EOF;
$return = '';
if ($definition->isSynthetic()) {
$return = sprintf('@throws \RuntimeException always since this service is expected to be injected dynamically');
} else if ($class = $definition->getClass()) {
} elseif ($class = $definition->getClass()) {
$return = sprintf("@return %s A %s instance.", 0 === strpos($class, '%') ? 'Object' : $class, $class);
} elseif ($definition->getFactoryClass()) {
$return = sprintf('@return Object An instance returned by %s::%s().', $definition->getFactoryClass(), $definition->getFactoryMethod());
} elseif ($definition->getFactoryService()) {
$return = sprintf('@return Object An instance returned by %s::%s().', $definition->getFactoryService(), $definition->getFactoryMethod());
}
@ -821,10 +827,12 @@ EOF;
}
if (null !== $value->getFactoryMethod()) {
if (null !== $value->getFactoryService()) {
if (null !== $value->getFactoryClass()) {
return sprintf("call_user_func(array(%s, '%s')%s)", $this->dumpValue($value->getFactoryClass()), $value->getFactoryMethod(), count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
} elseif (null !== $value->getFactoryService()) {
return sprintf("%s->%s(%s)", $this->getServiceCall($value->getFactoryService()), $value->getFactoryMethod(), implode(', ', $arguments));
} else {
return sprintf("call_user_func(array(%s, '%s')%s)", $class, $value->getFactoryMethod(), count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
throw new \RuntimeException('Cannot dump definitions which have factory method without factory service or factory class.');
}
}

View File

@ -143,7 +143,7 @@ class XmlFileLoader extends FileLoader
$definition = new Definition();
}
foreach (array('class', 'scope', 'public', 'factory-method', 'factory-service', 'synthetic', 'abstract') as $key) {
foreach (array('class', 'scope', 'public', 'factory-class', 'factory-method', 'factory-service', 'synthetic', 'abstract') as $key) {
if (isset($service[$key])) {
$method = 'set'.str_replace('-', '', $key);
$definition->$method((string) $service->getAttributeAsPhp($key));

View File

@ -165,6 +165,10 @@ class YamlFileLoader extends FileLoader
$definition->setAbstract($service['abstract']);
}
if (isset($service['factory_class'])) {
$definition->setFactoryClass($service['factory_class']);
}
if (isset($service['factory_method'])) {
$definition->setFactoryMethod($service['factory_method']);
}

View File

@ -105,6 +105,7 @@
<xsd:attribute name="public" type="boolean" />
<xsd:attribute name="synthetic" type="boolean" />
<xsd:attribute name="abstract" type="boolean" />
<xsd:attribute name="factory-class" type="xsd:string" />
<xsd:attribute name="factory-method" type="xsd:string" />
<xsd:attribute name="factory-service" type="xsd:string" />
<xsd:attribute name="alias" type="xsd:string" />

View File

@ -0,0 +1,61 @@
<?php
namespace Symfony\Tests\Component\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\Compiler\CheckDefinitionValidityPass;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;
class CheckDefinitionValidityPassTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \RuntimeException
*/
public function testProcessDetectsSyntheticNonPublicDefinitions()
{
$container = new ContainerBuilder();
$container->register('a')->setSynthetic(true)->setPublic(false);
$this->process($container);
}
/**
* @expectedException \RuntimeException
*/
public function testProcessDetectsSyntheticPrototypeDefinitions()
{
$container = new ContainerBuilder();
$container->register('a')->setSynthetic(true)->setScope(ContainerInterface::SCOPE_PROTOTYPE);
$this->process($container);
}
/**
* @expectedException \RuntimeException
*/
public function testProcessDetectsNonSyntheticNonAbstractDefinitionWithoutClass()
{
$container = new ContainerBuilder();
$container->register('a')->setSynthetic(false)->setAbstract(false);
$this->process($container);
}
public function testProcess()
{
$container = new ContainerBuilder();
$container->register('a', 'class');
$container->register('b', 'class')->setSynthetic(true)->setPublic(true);
$container->register('c', 'class')->setAbstract(true);
$container->register('d', 'class')->setSynthetic(true);
$this->process($container);
}
protected function process(ContainerBuilder $container)
{
$pass = new CheckDefinitionValidityPass();
$pass->process($container);
}
}

View File

@ -247,7 +247,7 @@ class ContainerBuilderTest extends \PHPUnit_Framework_TestCase
{
$builder = new ContainerBuilder();
$builder->register('bar', 'stdClass');
$builder->register('foo1', 'FooClass')->setFactoryMethod('getInstance')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar')));
$builder->register('foo1', 'FooClass')->setFactoryClass('FooClass')->setFactoryMethod('getInstance')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar')));
$builder->setParameter('value', 'bar');
$this->assertTrue($builder->get('foo1')->called, '->createService() calls the factory method to create the service instance');
$this->assertEquals(array('foo' => 'bar', 'bar' => 'foo', $builder->get('bar')), $builder->get('foo1')->arguments, '->createService() passes the arguments to the factory method');

View File

@ -34,8 +34,9 @@ class DefinitionDecoratorTest extends \PHPUnit_Framework_TestCase
{
return array(
array('class', 'class'),
array('factoryService', 'factory_service'),
array('factoryClass', 'factory_class'),
array('factoryMethod', 'factory_method'),
array('factoryService', 'factory_service'),
array('configurator', 'configurator'),
array('file', 'file'),
);

View File

@ -27,13 +27,18 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array('foo'), $def->getArguments(), '__construct() takes an optional array of arguments as its second argument');
}
/**
* @covers Symfony\Component\DependencyInjection\Definition::setFactoryMethod
* @covers Symfony\Component\DependencyInjection\Definition::getFactoryMethod
*/
public function testSetGetConstructor()
public function testSetGetFactoryClass()
{
$def = new Definition('stdClass');
$this->assertNull($def->getFactoryClass());
$this->assertSame($def, $def->setFactoryClass('stdClass2'), "->setFactoryClass() implements a fluent interface.");
$this->assertEquals('stdClass2', $def->getFactoryClass(), "->getFactoryClass() returns current class to construct this service.");
}
public function testSetGetFactoryMethod()
{
$def = new Definition('stdClass');
$this->assertNull($def->getFactoryMethod());
$this->assertSame($def, $def->setFactoryMethod('foo'), '->setFactoryMethod() implements a fluent interface');
$this->assertEquals('foo', $def->getFactoryMethod(), '->getFactoryMethod() returns the factory method name');
}
@ -42,8 +47,8 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase
{
$def = new Definition('stdClass');
$this->assertNull($def->getFactoryService());
$this->assertSame($def, $def->setFactoryService('stdClass2'), "->setFactoryService() implements a fluent interface.");
$this->assertEquals('stdClass2', $def->getFactoryService(), "->getFactoryService() returns current service to construct this service.");
$this->assertSame($def, $def->setFactoryService('foo.bar'), "->setFactoryService() implements a fluent interface.");
$this->assertEquals('foo.bar', $def->getFactoryService(), "->getFactoryService() returns current service to construct this service.");
}
/**

View File

@ -12,6 +12,7 @@ $container->
register('foo', 'FooClass')->
addTag('foo', array('foo' => 'foo'))->
addTag('foo', array('bar' => 'bar'))->
setFactoryClass('FooClass')->
setFactoryMethod('getInstance')->
setArguments(array('foo', new Reference('foo.baz'), array('%foo%' => 'foo is %foo%', 'bar' => '%foo%'), true, new Reference('service_container')))->
setScope('prototype')->
@ -27,6 +28,7 @@ $container->
;
$container->
register('foo.baz', '%baz_class%')->
setFactoryClass('%baz_class%')->
setFactoryMethod('getInstance')->
setConfigurator(array('%baz_class%', 'configureStatic1'))
;