Fixing a bug where services that were eventually removed could cause autowire errors
This commit is contained in:
parent
f56245594c
commit
f4913feaa8
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
@ -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;
|
||||
}
|
||||
|
||||
|
@ -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(),
|
||||
));
|
||||
}
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
@ -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);
|
||||
}
|
||||
}
|
@ -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".
|
||||
|
Reference in New Issue
Block a user