Skip to content

Commit 7eab6b9

Browse files
fabpotfabpot
committed
feature #17608 [DependencyInjection] Autowiring: add setter injection support (dunglas)
This PR was merged into the 3.2-dev branch. Discussion ---------- [DependencyInjection] Autowiring: add setter injection support | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | todo Add support for setter injection in the Dependency Injection Component. Setter injection should be avoided when possible. However, there is some legitimate use cases for it. This PR follows a proposal of @weaverryan to ease using [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle) with #16863. The basic idea is to include a setter for the required service in the trait and let the DependencyInjection Component autowire the dependency using the setter. This way, a newcomer can use the trait without having to create or modify the constructor of the action. /cc @derrabus Commits ------- a0d7cbe [DependencyInjection] Autowiring: add setter injection support
2 parents 35f201f + a0d7cbe commit 7eab6b9

File tree

3 files changed

+182
-9
lines changed

3 files changed

+182
-9
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.2.0
55
-----
66

7+
* added support for setter autowiring
78
* allowed to prioritize compiler passes by introducing a third argument to `PassConfig::addPass()`, to `Compiler::addPass` and to `ContainerBuilder::addCompilerPass()`
89
* added support for PHP constants in YAML configuration files
910
* deprecated the ability to set or unset a private service with the `Container::set()` method

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

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,45 @@ private function completeDefinition($id, Definition $definition)
110110
$this->container->addResource(static::createResourceForClass($reflectionClass));
111111
}
112112

113-
if (!$constructor = $reflectionClass->getConstructor()) {
114-
return;
113+
if ($constructor = $reflectionClass->getConstructor()) {
114+
$this->autowireMethod($id, $definition, $constructor, true);
115+
}
116+
117+
$methodsCalled = array();
118+
foreach ($definition->getMethodCalls() as $methodCall) {
119+
$methodsCalled[$methodCall[0]] = true;
120+
}
121+
122+
foreach ($reflectionClass->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
123+
$name = $reflectionMethod->getName();
124+
if (isset($methodsCalled[$name]) || $reflectionMethod->isStatic() || 1 !== $reflectionMethod->getNumberOfParameters() || 0 !== strpos($name, 'set')) {
125+
continue;
126+
}
127+
128+
$this->autowireMethod($id, $definition, $reflectionMethod, false);
115129
}
130+
}
116131

117-
$arguments = $definition->getArguments();
118-
foreach ($constructor->getParameters() as $index => $parameter) {
132+
/**
133+
* Autowires the constructor or a setter.
134+
*
135+
* @param string $id
136+
* @param Definition $definition
137+
* @param \ReflectionMethod $reflectionMethod
138+
* @param bool $isConstructor
139+
*
140+
* @throws RuntimeException
141+
*/
142+
private function autowireMethod($id, Definition $definition, \ReflectionMethod $reflectionMethod, $isConstructor)
143+
{
144+
if ($isConstructor) {
145+
$arguments = $definition->getArguments();
146+
} else {
147+
$arguments = array();
148+
}
149+
150+
$addMethodCall = false;
151+
foreach ($reflectionMethod->getParameters() as $index => $parameter) {
119152
if (array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
120153
continue;
121154
}
@@ -124,7 +157,11 @@ private function completeDefinition($id, Definition $definition)
124157
if (!$typeHint = $parameter->getClass()) {
125158
// no default value? Then fail
126159
if (!$parameter->isOptional()) {
127-
throw new RuntimeException(sprintf('Unable to autowire argument index %d ($%s) for the service "%s". If this is an object, give it a type-hint. Otherwise, specify this argument\'s value explicitly.', $index, $parameter->name, $id));
160+
if ($isConstructor) {
161+
throw new RuntimeException(sprintf('Unable to autowire argument index %d ($%s) for the service "%s". If this is an object, give it a type-hint. Otherwise, specify this argument\'s value explicitly.', $index, $parameter->name, $id));
162+
}
163+
164+
return;
128165
}
129166

130167
// specifically pass the default value
@@ -139,24 +176,35 @@ private function completeDefinition($id, Definition $definition)
139176

140177
if (isset($this->types[$typeHint->name])) {
141178
$value = new Reference($this->types[$typeHint->name]);
179+
$addMethodCall = true;
142180
} else {
143181
try {
144182
$value = $this->createAutowiredDefinition($typeHint, $id);
183+
$addMethodCall = true;
145184
} catch (RuntimeException $e) {
146185
if ($parameter->allowsNull()) {
147186
$value = null;
148187
} elseif ($parameter->isDefaultValueAvailable()) {
149188
$value = $parameter->getDefaultValue();
150189
} else {
151-
throw $e;
190+
// The exception code is set to 1 if the exception must be thrown even if it's a setter
191+
if (1 === $e->getCode() || $isConstructor) {
192+
throw $e;
193+
}
194+
195+
return;
152196
}
153197
}
154198
}
155199
} catch (\ReflectionException $e) {
156200
// Typehint against a non-existing class
157201

158202
if (!$parameter->isDefaultValueAvailable()) {
159-
throw new RuntimeException(sprintf('Cannot autowire argument %s for %s because the type-hinted class does not exist (%s).', $index + 1, $definition->getClass(), $e->getMessage()), 0, $e);
203+
if ($isConstructor) {
204+
throw new RuntimeException(sprintf('Cannot autowire argument %s for %s because the type-hinted class does not exist (%s).', $index + 1, $definition->getClass(), $e->getMessage()), 0, $e);
205+
}
206+
207+
return;
160208
}
161209

162210
$value = $parameter->getDefaultValue();
@@ -168,7 +216,12 @@ private function completeDefinition($id, Definition $definition)
168216
// it's possible index 1 was set, then index 0, then 2, etc
169217
// make sure that we re-order so they're injected as expected
170218
ksort($arguments);
171-
$definition->setArguments($arguments);
219+
220+
if ($isConstructor) {
221+
$definition->setArguments($arguments);
222+
} elseif ($addMethodCall) {
223+
$definition->addMethodCall($reflectionMethod->name, $arguments);
224+
}
172225
}
173226

174227
/**
@@ -266,7 +319,7 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint, $id)
266319
$classOrInterface = $typeHint->isInterface() ? 'interface' : 'class';
267320
$matchingServices = implode(', ', $this->ambiguousServiceTypes[$typeHint->name]);
268321

269-
throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $typeHint->name, $id, $classOrInterface, $matchingServices));
322+
throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s". Multiple services exist for this %s (%s).', $typeHint->name, $id, $classOrInterface, $matchingServices), 1);
270323
}
271324

272325
if (!$typeHint->isInstantiable()) {

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,47 @@ public function testOptionalScalarArgsNotPassedIfLast()
429429
);
430430
}
431431

432+
public function testSetterInjection()
433+
{
434+
$container = new ContainerBuilder();
435+
$container->register('app_foo', Foo::class);
436+
$container->register('app_a', A::class);
437+
$container->register('app_collision_a', CollisionA::class);
438+
$container->register('app_collision_b', CollisionB::class);
439+
440+
// manually configure *one* call, to override autowiring
441+
$container
442+
->register('setter_injection', SetterInjection::class)
443+
->setAutowired(true)
444+
->addMethodCall('setWithCallsConfigured', array('manual_arg1', 'manual_arg2'))
445+
;
446+
447+
$pass = new AutowirePass();
448+
$pass->process($container);
449+
450+
$methodCalls = $container->getDefinition('setter_injection')->getMethodCalls();
451+
452+
// grab the call method names
453+
$actualMethodNameCalls = array_map(function ($call) {
454+
return $call[0];
455+
}, $methodCalls);
456+
$this->assertEquals(
457+
array('setWithCallsConfigured', 'setFoo'),
458+
$actualMethodNameCalls
459+
);
460+
461+
// test setWithCallsConfigured args
462+
$this->assertEquals(
463+
array('manual_arg1', 'manual_arg2'),
464+
$methodCalls[0][1]
465+
);
466+
// test setFoo args
467+
$this->assertEquals(
468+
array(new Reference('app_foo')),
469+
$methodCalls[1][1]
470+
);
471+
}
472+
432473
/**
433474
* @dataProvider getCreateResourceTests
434475
*/
@@ -476,6 +517,24 @@ public function testIgnoreServiceWithClassNotExisting()
476517

477518
$this->assertTrue($container->hasDefinition('bar'));
478519
}
520+
521+
/**
522+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
523+
* @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "setter_injection_collision". Multiple services exist for this interface (c1, c2).
524+
* @expectedExceptionCode 1
525+
*/
526+
public function testSetterInjectionCollisionThrowsException()
527+
{
528+
$container = new ContainerBuilder();
529+
530+
$container->register('c1', CollisionA::class);
531+
$container->register('c2', CollisionB::class);
532+
$aDefinition = $container->register('setter_injection_collision', SetterInjectionCollision::class);
533+
$aDefinition->setAutowired(true);
534+
535+
$pass = new AutowirePass();
536+
$pass->process($container);
537+
}
479538
}
480539

481540
class Foo
@@ -648,9 +707,69 @@ public function setBar(Bar $bar)
648707
class IdenticalClassResource extends ClassForResource
649708
{
650709
}
710+
651711
class ClassChangedConstructorArgs extends ClassForResource
652712
{
653713
public function __construct($foo, Bar $bar, $baz)
654714
{
655715
}
656716
}
717+
718+
class SetterInjection
719+
{
720+
public function setFoo(Foo $foo)
721+
{
722+
// should be called
723+
}
724+
725+
public function setDependencies(Foo $foo, A $a)
726+
{
727+
// should be called
728+
}
729+
730+
public function setBar()
731+
{
732+
// should not be called
733+
}
734+
735+
public function setNotAutowireable(NotARealClass $n)
736+
{
737+
// should not be called
738+
}
739+
740+
public function setArgCannotAutowire($foo)
741+
{
742+
// should not be called
743+
}
744+
745+
public function setOptionalNotAutowireable(NotARealClass $n = null)
746+
{
747+
// should not be called
748+
}
749+
750+
public function setOptionalNoTypeHint($foo = null)
751+
{
752+
// should not be called
753+
}
754+
755+
public function setOptionalArgNoAutowireable($other = 'default_val')
756+
{
757+
// should not be called
758+
}
759+
760+
public function setWithCallsConfigured(A $a)
761+
{
762+
// this method has a calls configured on it
763+
// should not be called
764+
}
765+
}
766+
767+
class SetterInjectionCollision
768+
{
769+
public function setMultipleInstancesForOneArg(CollisionInterface $collision)
770+
{
771+
// The CollisionInterface cannot be autowired - there are multiple
772+
773+
// should throw an exception
774+
}
775+
}

0 commit comments

Comments
 (0)