8000 feature #20943 [DependencyInjection] Use current class as default cla… · symfony/symfony@37c5997 · GitHub
[go: up one dir, main page]
Skip to content

Commit 37c5997

Browse files
fabpotfabpot
committed
feature #20943 [DependencyInjection] Use current class as default class for factory declarations (ogizanagi)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DependencyInjection] Use current class as default class for factory declarations | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20888 | License | MIT | Doc PR | Should update the notice about the "class" attribute on http://symfony.com/doc/current/service_container/factories.html #20888 makes sense to me, considering the following sample extracted from the documentation: ```xml <service id="app.newsletter_manager" class="AppBundle\Email\NewsletterManager"> <factory class="AppBundle\Email\NewsletterManager" method="create" /> </service> ``` The class is used as a factory to create itself, thus it can be simplified to: ```xml <service id="app.newsletter_manager" class="AppBundle\Email\NewsletterManager"> <factory method="create" /> </service> ``` H 8000 owever, it's not possible to provide the same feature for the YAML format, because it doesn't allow to distinct a function from a method call if the class is not provided explicitly under the `factory` key, whereas the xml format use a dedicated `function` attribute. Would this inconsistency between those two formats be a no-go for this feature? The doc notices: > When using a factory to create services, the value chosen for the class option has no effect on the resulting service. The actual class name only depends on the object that is returned by the factory. However, the configured class name may be used by compiler passes and therefore should be set to a sensible value. If this is merged, it should be updated wisely in order to not confuse everyone with this feature when using the xml format. UPDATE: The yaml format is now supported when the class is not provided in the factory array: ```yml services: my_factory: class: Bar\Baz factory: [~, 'create'] ``` Commits ------- e6d8570 [DependencyInjection] Use current class as default class for factory declarations
2 parents c90fbb4 + e6d8570 commit 37c5997

File tree

11 files changed

+141
-2
lines changed

11 files changed

+141
-2
lines changed

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
3.3.0
55
-----
66

7+
* added support for omitting the factory class name in a service definition if the definition class is set
78
* deprecated case insensitivity of service identifiers
89
* added "iterator" argument type for lazy iteration over a set of values and services
910
* added "closure-proxy" argument type for turning services' methods into lazy callables

‎src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public function __construct()
5050
new ResolveDefinitionTemplatesPass(),
5151
new DecoratorServicePass(),
5252
new ResolveParameterPlaceHoldersPass(),
53+
new ResolveFactoryClassPass(),
5354
new FactoryReturnTypePass($resolveClassPass),
5455
new CheckDefinitionValidityPass(),
5556
new ResolveReferencesToAliasesPass(),

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveFactoryClassPass.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
< 6D40 code>9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Definition;
15+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
16+
17+
/**
18+
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
19+
*/
20+
class ResolveFactoryClassPass extends AbstractRecursivePass
21+
{
22+
/**
23+
* {@inheritdoc}
24+
*/
25+
protected function processValue($value, $isRoot = false)
26+
{
27+
if ($value instanceof Definition && is_array($factory = $value->getFactory()) && null === $factory[0]) {
28+
if (null === $class = $value->getClass()) {
29+
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));
30+
}
31+
32+
$factory[0] = $class;
33+
$value->setFactory($factory);
34+
}
35+
36+
return parent::processValue($value, $isRoot);
37+
}
38+
}

‎src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,9 @@ private function addService($definition, $id, \DOMElement $parent)
176176
$this->addService($callable[0], null, $factory);
177177
$factory->setAttribute('method', $callable[1]);
178178
} elseif (is_array($callable)) {
179-
$factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]);
179+
if (null !== $callable[0]) {
180+
$factory->setAttribute($callable[0] instanceof Reference ? 'service' : 'class', $callable[0]);
181+
}
180182
$factory->setAttribute('method', $callable[1]);
181183
} else {
182184
$factory->setAttribute('function', $callable);

‎src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
257257
} elseif ($childService = $factory->getAttribute('service')) {
258258
$class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false);
259259
} else {
260-
$class = $factory->getAttribute('class');
260+
$class = $factory->hasAttribute('class') ? $factory->getAttribute('class') : null;
261261
}
262262

263263
$definition->setFactory(array($class, $factory->getAttribute('method')));

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,10 @@ private function parseCallable($callable, $parameter, $id, $file)
466466
return array($this->resolveServices($callable[0]), $callable[1]);
467467
}
468468

469+
if ('factory' === $parameter && isset($callable[1]) && null === $callable[0]) {
470+
return $callable;
471+
}
472+
469473
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));
470474
}
471475

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveFactoryClassPassTest.php

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Compiler\ResolveFactoryClassPass;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Definition;
17+
use Symfony\Component\DependencyInjection\Reference;
18+
19+
class ResolveFactoryClassPassTest extends \PHPUnit_Framework_TestCase
20+
{
21+
public function testProcess()
22+
{
23+
$container = new ContainerBuilder();
24+
25+
$factory = $container->register('factory', 'Foo\Bar');
26+
$factory->setFactory(array(null, 'create'));
27+
28+
$pass = new ResolveFactoryClassPass();
29+
$pass->process($container);
30+
31+
$this->assertSame(array('Foo\Bar', 'create'), $factory->getFactory());
32+
}
33+ F987
34+
public function testInlinedDefinitionFactoryIsProcessed()
35+
{
36+
$container = new ContainerBuilder();
37+
38+
$factory = $container->register('factory');
39+
$factory->setFactory(array((new Definition('Baz\Qux'))->setFactory(array(null, 'getInstance')), 'create'));
40+
41+
$pass = new ResolveFactoryClassPass();
42+
$pass->process($container);
43+
44+
$this->assertSame(array('Baz\Qux', 'getInstance'), $factory->getFactory()[0]->getFactory());
45+
}
46+
47+
public function provideFulfilledFactories()
48+
{
49+
return array(
50+
array(array('Foo\Bar', 'create')),
51+
array(array(new Reference('foo'), 'create')),
52+
array(array(new Definition('Baz'), 'create')),
53+
);
54+
}
55+
56+
/**
57+
* @dataProvider provideFulfilledFactories
58+
*/
59+
public function testIgnoresFulfilledFactories($factory)
60+
{
61+
$container = new ContainerBuilder();
62+
$definition = new Definition();
63+
$definition->setFactory($factory);
64+
65+
$container->setDefinition('factory', $definition);
66+
67+
$pass = new ResolveFactoryClassPass();
68+
$pass->process($container);
69+
70+
$this->assertSame($factory, $container->getDefinition('factory')->getFactory());
71+
}
72+
73+
/**
74+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
75+
* @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?
76+
*/
77+
public function testNotAnyClassThrowsException()
78+
{
79+
$container = new ContainerBuilder();
80+
81+
$factory = $container->register('factory');
82+
$factory->setFactory(array(null, 'create'));
83+
84+
$pass = new ResolveFactoryClassPass();
85+
$pass->process($container);
86+
}
87+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,8 @@
5858
<service id="new_factory3" class="FooBarClass">
5959
<factory class="BazClass" method="getInstance" />
6060
</service>
61+
<service id="new_factory4" class="BazClass">
62+
<factory method="getInstance" />
63+
</service>
6164
</services>
6265
</container>

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,5 @@ services:
3939
new_factory1: { class: FooBarClass, factory: factory}
4040
new_factory2: { class: FooBarClass, factory: ['@baz', getClass]}
4141
new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]}
42+
new_factory4: { class: BazClass, factory: [~, getInstance]}
4243
with_shortcut_args: [foo, '@baz']

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ public function testLoadServices()
246246
$this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag');
247247
$this->assertEquals(array(new Reference('baz', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag');
248248
$this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag');
249+
$this->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class');
249250

250251
$aliases = $container->getAliases();
251252
$this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses <service> elements');

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ public function testLoadServices()
153153
$this->assertEquals('factory', $services['new_factory1']->getFactory(), '->load() parses the factory tag');
154154
$this->assertEquals(array(new Reference('baz'), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag');
155155
$this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag');
156+
$this->assertSame(array(null, 'getInstance'), $services['new_factory4']->getFactory(), '->load() accepts factory tag without class');
156157
$this->assertEquals(array('foo', new Reference('baz')), $services['with_shortcut_args']->getArguments(), '->load() parses short service definition');
157158

158159
$aliases = $container->getAliases();

0 commit comments

Comments
 (0)
0