8000 feature #22665 [DI] Do not throw autowiring exceptions for a service … · symfony/symfony@a3f7860 · GitHub
[go: up one dir, main page]
Skip to content

Commit a3f7860

Browse files
fabpotfabpot
committed
feature #22665 [DI] Do not throw autowiring exceptions for a service that will be removed (weaverryan)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Do not throw autowiring exceptions for a service that will be removed | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no (arguable) | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Hi guys! tl;dr Do no throw a "Cannot autowire service id foo_bar" if that service (`foo_bar`) is private and is ultimately removed from the container. I ran into a problem with the new PSR-4 service loader: our existing projects often contains directories with a mixture of services and model classes. In reality, that's not a problem: since the services are private, if any "extra" classes are registered as service, they're removed from the container because they're not referenced. In other words, the system is great: model classes do *not* become services naturally... because nobody tries to inject them as services. However, if your model classes have constructor args... then things blow up on compilation. This fixes that: it delays autowiring errors until after `RemoveUnusedDefinitionsPass` runs and then does *not* throw those exceptions if the service is gone. Cheers! Commits ------- f4913fe Fixing a bug where services that were eventually removed could cause autowire errors
2 parents 428b9bf + f4913fe commit a3f7860

File tree

6 files changed

+194
-5
lines changed

6 files changed

+194
-5
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/AutowireExceptionPass.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
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\ContainerBuilder;
15+
16+
/**
17+
* Throws autowire exceptions from AutowirePass for definitions that still exist.
18+
*
19+
* @author Ryan Weaver <ryan@knpuniversity.com>
20+
*/
21+
class AutowireExceptionPass implements CompilerPassInterface
22+
{
23+
private $autowirePass;
24+
25+
public function __construct(AutowirePass $autowirePass)
26+
{
8000
27+
$this->autowirePass = $autowirePass;
28+
}
29+
30+
public function process(ContainerBuilder $container)
31+
{
32+
foreach ($this->autowirePass->getAutowiringExceptions() as $exception) {
33+
if ($container->hasDefinition($exception->getServiceId())) {
34+
throw $exception;
35+
}
36+
}
37+
}
38+
}

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\DependencyInjection\Config\AutowireServiceResource;
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
1616
use Symfony\Component\DependencyInjection\Definition;
17+
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
1718
use Symfony\Component\DependencyInjection\Exce 6D40 ption\RuntimeException;
1819
use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
1920
use Symfony\Component\DependencyInjection\TypedReference;
@@ -31,12 +32,33 @@ class AutowirePass extends AbstractRecursivePass
3132
private $ambiguousServiceTypes = array();
3233
private $autowired = array();
3334
private $lastFailure;
35+
private $throwOnAutowiringException;
36+
private $autowiringExceptions = array();
37+
38+
/**
39+
* @param bool $throwOnAutowireException If false, retrieved errors via getAutowiringExceptions
40+
*/
41+
public function __construct($throwOnAutowireException = true)
42+
{
43+
$this->throwOnAutowiringException = $throwOnAutowireException;
44+
}
45+
46+
/**
47+
* @return AutowiringFailedException[]
48+
*/
49+
public function getAutowiringExceptions()
50+
{
51+
return $this->autowiringExceptions;
52+
}
3453

3554
/**
3655
* {@inheritdoc}
3756
*/
3857
public function process(ContainerBuilder $container)
3958
{
59+
// clear out any possibly stored exceptions from before
60+
$this->autowiringExceptions = array();
61+
4062
try {
4163
parent::process($container);
4264
} finally {
@@ -75,6 +97,21 @@ public static function createResourceForClass(\ReflectionClass $reflectionClass)
7597
* {@inheritdoc}
7698
*/
7799
protected function processValue($value, $isRoot = false)
100+
{
101+
try {
102+
return $this->doProcessValue($value, $isRoot);
103+
} catch (AutowiringFailedException $e) {
104+
if ($this->throwOnAutowiringException) {
105+
throw $e;
106+
}
107+
108+
$this->autowiringExceptions[] = $e;
109+
110+
return parent::processValue($value, $isRoot);
111+
}
112+
}
113+
114+
private function doProcessValue($value, $isRoot = false)
78115
{
79116
if ($value instanceof TypedReference) {
80117
if ($ref = $this->getAutowiredReference($value)) {
@@ -190,7 +227,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
190227

191228
if (!$reflectionMethod->isPublic()) {
192229
$class = $reflectionClass->name;
193-
throw new RuntimeException(sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
230+
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
194231
}
195232
$methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
196233
}
@@ -231,7 +268,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
231268

232269
// no default value? Then fail
233270
if (!$parameter->isDefaultValueAvailable()) {
234-
throw new RuntimeException(sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
271+
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": argument "$%s" of method "%s()" must have a type-hint or be given a value explicitly.', $this->currentId, $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
235272
}
236273

237274
// specifically pass the default value
@@ -246,7 +283,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
246283
if ($parameter->isDefaultValueAvailable()) {
247284
$value = $parameter->getDefaultValue();
248285
} elseif (!$parameter->allowsNull()) {
249-
throw new RuntimeException($failureMessage);
286+
throw new AutowiringFailedException($this->currentId, $failureMessage);
250287
}
251288
$this->container->log($this, $failureMessage);
252289
}
@@ -407,15 +444,18 @@ private function createAutowiredDefinition($type)
407444
$argumentDefinition->setAutowired(true);
408445

409446
try {
447+
$originalThrowSetting = $this->throwOnAutowiringException;
448+
$this->throwOnAutowiringException = true;
410449
$this->processValue($argumentDefinition, true);
411450
$this->container->setDefinition($argumentId, $argumentDefinition);
412-
} catch (RuntimeException $e) {
451+
} catch (AutowiringFailedException $e) {
413452
$this->autowired[$type] = false;
414453
$this->lastFailure = $e->getMessage();
415454
$this->container->log($this, $this->lastFailure);
416455

417456
return;
418457
} finally {
458+
$this->throwOnAutowiringException = $originalThrowSetting;
419459
$this->currentId = $currentId;
420460
}
421461

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public function __construct()
5757
new CheckDefinitionValidityPass(),
5858
new RegisterServiceSubscribersPass(),
5959
new ResolveNamedArgumentsPass(),
60-
new AutowirePass(),
60+
$autowirePass = new AutowirePass(false),
6161
new ResolveServiceSubscribersPass(),
6262
new ResolveReferencesToAliasesPass(),
6363
new ResolveInvalidReferencesPass(),
@@ -77,6 +77,7 @@ public function __construct()
7777
new AnalyzeServiceReferencesPass(),
7878
new RemoveUnusedDefinitionsPass(),
7979
)),
80+
new AutowireExceptionPass($autowirePass),
8081
new CheckExceptionOnInvalidReferenceBehaviorPass(),
8182
));
8283
}

‎src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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\Exception;
13+
14+
/**
15+
* Thrown when a definition cannot be autowired.
16+
*/
17+
class AutowiringFailedException extends RuntimeException
18+
{
19+
private $serviceId;
20+
21+
public function __construct($serviceId, $message = '', $code = 0, Exception $previous = null)
22+
{
23+
$this->serviceId = $serviceId;
24+
25+
parent::__construct($message, $code, $previous);
26+
}
27+
28+
public function getServiceId()
29+
{
30+
return $this->serviceId;
31+
}
32+
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
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 PHPUnit\Framework\TestCase;
15+
use Symfony\Component\DependencyInjection\Compiler\AutowireExceptionPass;
16+
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
17+
use Symfony\Component\DependencyInjection\ContainerBuilder;
18+
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
19+
20+
class AutowireExceptionPassTest extends TestCase
21+
{
22+
public function testThrowsException()
23+
{
24+
$autowirePass = $this->getMockBuilder(AutowirePass::class)
25+
->getMock();
26+
27+
$autowireException = new AutowiringFailedException('foo_service_id', 'An autowiring exception message');
28+
$autowirePass->expects($this->any())
29+
->method('getAutowiringExceptions')
30+
->will($this->returnValue(array($autowireException)));
31+
32+
$container = new ContainerBuilder();
33+
$container->register('foo_service_id');
34+
35+
$pass = new AutowireExceptionPass($autowirePass);
36+
37+
try {
38+
$pass->process($container);
39+
$this->fail('->process() should throw the exception if the service id exists');
40+
} catch (\Exception $e) {
41+
$this->assertSame($autowireException, $e);
42+
}
43+
}
44+
45+
public function testNoExceptionIfServiceRemoved()
46+
{
47+
$autowirePass = $this->getMockBuilder(AutowirePass::class)
48+
->getMock();
49+
50+
$autowireException = new AutowiringFailedException('non_existent_service');
51+
$autowirePass->expects($this->any())
52+
->method('getAutowiringExceptions')
53+
->will($this->returnValue(array($autowireException)));
54+
55+
$container = new ContainerBuilder();
56+
57+
$pass = new AutowireExceptionPass($autowirePass);
58+
59+
$pass->process($container);
60+
// mark the test as passed
61+
$this->assertTrue(true);
62+
}
63+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,21 @@ public function testCompleteExistingDefinitionWithNotDefinedArguments()
135135
$this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1));
136136
}
137137

138+
public function testExceptionsAreStored()
139+
{
140+
$container = new ContainerBuilder();
141+
142+
$container->register('c1', __NAMESPACE__.'\CollisionA');
143+
$container->register('c2', __NAMESPACE__.'\CollisionB');
144+
$container->register('c3', __NAMESPACE__.'\CollisionB');
145+
$aDefinition = $container->register('a', __NAMESPACE__.'\CannotBeAutowired');
146+
$aDefinition->setAutowired(true);
147+
148+
$pass = new AutowirePass(false);
149+
$pass->process($container);
150+
$this->assertCount(1, $pass->getAutowiringExceptions());
151+
}
152+
138153
/**
139154
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
140155
* @expectedExceptionMessage Cannot autowire service "a": argument "$collision" of method "Symfony\Component\DependencyInjection\Tests\Compiler\CannotBeAutowired::__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "c1", "c2", "c3".

0 commit comments

Comments
 (0)
0