diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feature #22665 [DI] Do not throw autowiring exceptions for a service that will be removed (weaverryan)

Browse Source 
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
-------

f4913feaa8 Fixing a bug where services that were eventually removed could cause autowire errors
This commit is contained in:
Fabien Potencier 2017-05-11 10:07:41 -07:00
commit a3f78608e8
6 changed files with 194 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/Symfony/Component/DependencyInjection/Compiler/AutowireExceptionPass.php Normal file
View File

@ -0,0 +1,38 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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\ContainerBuilder;
/**
* Throws autowire exceptions from AutowirePass for definitions that still exist.
*
* @author Ryan Weaver <ryan@knpuniversity.com>
*/
class AutowireExceptionPass implements CompilerPassInterface
{
private $autowirePass;
public function __construct(AutowirePass $autowirePass)
{
$this->autowirePass = $autowirePass;
}
public function process(ContainerBuilder $container)
{
foreach ($this->autowirePass->getAutowiringExceptions() as $exception) {
if ($container->hasDefinition($exception->getServiceId())) {
throw $exception;
}
}
}
}

48
src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
View File

@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Compiler;
use Symfony\Component\DependencyInjection\Config\AutowireServiceResource;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
use Symfony\Component\DependencyInjection\TypedReference;
@ -31,12 +32,33 @@ class AutowirePass extends AbstractRecursivePass
private $ambiguousServiceTypes = array();
private $autowired = array();
private $lastFailure;
private $throwOnAutowiringException;
private $autowiringExceptions = array();
/**
* @param bool $throwOnAutowireException If false, retrieved errors via getAutowiringExceptions
*/
public function __construct($throwOnAutowireException = true)
{
$this->throwOnAutowiringException = $throwOnAutowireException;
}
/**
* @return AutowiringFailedException[]
*/
public function getAutowiringExceptions()
{
return $this->autowiringExceptions;
}
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
// clear out any possibly stored exceptions from before
$this->autowiringExceptions = array();
try {
parent::process($container);
} finally {
@ -75,6 +97,21 @@ class AutowirePass extends AbstractRecursivePass
* {@inheritdoc}
*/
protected function processValue($value, $isRoot = false)
{
try {
return $this->doProcessValue($value, $isRoot);
} catch (AutowiringFailedException $e) {
if ($this->throwOnAutowiringException) {
throw $e;
}
$this->autowiringExceptions[] = $e;
return parent::processValue($value, $isRoot);
}
}
private function doProcessValue($value, $isRoot = false)
{
if ($value instanceof TypedReference) {
if ($ref = $this->getAutowiredReference($value)) {
@ -190,7 +227,7 @@ class AutowirePass extends AbstractRecursivePass
if (!$reflectionMethod->isPublic()) {
$class = $reflectionClass->name;
throw new RuntimeException(sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
}
$methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
}
@ -231,7 +268,7 @@ class AutowirePass extends AbstractRecursivePass
// no default value? Then fail
if (!$parameter->isDefaultValueAvailable()) {
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));
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));
}
// specifically pass the default value
@ -246,7 +283,7 @@ class AutowirePass extends AbstractRecursivePass
if ($parameter->isDefaultValueAvailable()) {
$value = $parameter->getDefaultValue();
} elseif (!$parameter->allowsNull()) {
throw new RuntimeException($failureMessage);
throw new AutowiringFailedException($this->currentId, $failureMessage);
}
$this->container->log($this, $failureMessage);
}
@ -407,15 +444,18 @@ class AutowirePass extends AbstractRecursivePass
$argumentDefinition->setAutowired(true);
try {
$originalThrowSetting = $this->throwOnAutowiringException;
$this->throwOnAutowiringException = true;
$this->processValue($argumentDefinition, true);
$this->container->setDefinition($argumentId, $argumentDefinition);
} catch (RuntimeException $e) {
} catch (AutowiringFailedException $e) {
$this->autowired[$type] = false;
$this->lastFailure = $e->getMessage();
$this->container->log($this, $this->lastFailure);
return;
} finally {
$this->throwOnAutowiringException = $originalThrowSetting;
$this->currentId = $currentId;
}

3
src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
View File

@ -57,7 +57,7 @@ class PassConfig
new CheckDefinitionValidityPass(),
new RegisterServiceSubscribersPass(),
new ResolveNamedArgumentsPass(),
new AutowirePass(),
$autowirePass = new AutowirePass(false),
new ResolveServiceSubscribersPass(),
new ResolveReferencesToAliasesPass(),
new ResolveInvalidReferencesPass(),
@ -77,6 +77,7 @@ class PassConfig
new AnalyzeServiceReferencesPass(),
new RemoveUnusedDefinitionsPass(),
)),
new AutowireExceptionPass($autowirePass),
new CheckExceptionOnInvalidReferenceBehaviorPass(),
));
}

32
src/Symfony/Component/DependencyInjection/Exception/AutowiringFailedException.php Normal file
View File

@ -0,0 +1,32 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\DependencyInjection\Exception;
/**
* Thrown when a definition cannot be autowired.
*/
class AutowiringFailedException extends RuntimeException
{
private $serviceId;
public function __construct($serviceId, $message = '', $code = 0, Exception $previous = null)
{
$this->serviceId = $serviceId;
parent::__construct($message, $code, $previous);
}
public function getServiceId()
{
return $this->serviceId;
}
}

63
src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowireExceptionPassTest.php Normal file
View File

@ -0,0 +1,63 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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 PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Compiler\AutowireExceptionPass;
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
class AutowireExceptionPassTest extends TestCase
{
public function testThrowsException()
{
$autowirePass = $this->getMockBuilder(AutowirePass::class)
->getMock();
$autowireException = new AutowiringFailedException('foo_service_id', 'An autowiring exception message');
$autowirePass->expects($this->any())
->method('getAutowiringExceptions')
->will($this->returnValue(array($autowireException)));
$container = new ContainerBuilder();
$container->register('foo_service_id');
$pass = new AutowireExceptionPass($autowirePass);
try {
$pass->process($container);
$this->fail('->process() should throw the exception if the service id exists');
} catch (\Exception $e) {
$this->assertSame($autowireException, $e);
}
}
public function testNoExceptionIfServiceRemoved()
{
$autowirePass = $this->getMockBuilder(AutowirePass::class)
->getMock();
$autowireException = new AutowiringFailedException('non_existent_service');
$autowirePass->expects($this->any())
->method('getAutowiringExceptions')
->will($this->returnValue(array($autowireException)));
$container = new ContainerBuilder();
$pass = new AutowireExceptionPass($autowirePass);
$pass->process($container);
// mark the test as passed
$this->assertTrue(true);
}
}

15
src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
View File

@ -135,6 +135,21 @@ class AutowirePassTest extends TestCase
$this->assertEquals(DInterface::class, (string) $container->getDefinition('h')->getArgument(1));
}
public function testExceptionsAreStored()
{
$container = new ContainerBuilder();
$container->register('c1', __NAMESPACE__.'\CollisionA');
$container->register('c2', __NAMESPACE__.'\CollisionB');
$container->register('c3', __NAMESPACE__.'\CollisionB');
$aDefinition = $container->register('a', __NAMESPACE__.'\CannotBeAutowired');
$aDefinition->setAutowired(true);
$pass = new AutowirePass(false);
$pass->process($container);
$this->assertCount(1, $pass->getAutowiringExceptions());
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @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".