Skip to content
Permalink
Browse files

bug #22857 [DI] Fix autowire error for inlined services (weaverryan)

This PR was squashed before being merged into the 3.3 branch (closes #22857).

Discussion
----------

[DI] Fix autowire error for inlined services

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22848
| License       | MIT
| Doc PR        | n/a

The `AutowirePass` defers autowiring exceptions until later so that we don't throw autowiring exceptions for services that are ultimately removed. But, if a service is *inlined*, then it appears to be removed, and so we don't throw the exception. This fixes that.

It's an easy fix - but it's a bit ugly. We're adding a bit more "state" to the passes... simply because there is some information that needs to be shared through the compiler process. There might be a better way of doing this in the future (e.g. storing some metadata on the `Compiler`), but this *does* work well.

Commits
-------

4bcef3d [DI] Fix autowire error for inlined services
  • Loading branch information...
nicolas-grekas committed May 23, 2017
2 parents bca7b41 + 4bcef3d commit 2f4dea56b4134ddc3505ab2d33bc8014d73934c4
@@ -21,16 +21,30 @@
class AutowireExceptionPass implements CompilerPassInterface
{
private $autowirePass;
private $inlineServicePass;
public function __construct(AutowirePass $autowirePass)
public function __construct(AutowirePass $autowirePass, InlineServiceDefinitionsPass $inlineServicePass)
{
$this->autowirePass = $autowirePass;
$this->inlineServicePass = $inlineServicePass;
}
public function process(ContainerBuilder $container)
{
foreach ($this->autowirePass->getAutowiringExceptions() as $exception) {
if ($container->hasDefinition($exception->getServiceId())) {
// the pass should only be run once
if (null === $this->autowirePass || null === $this->inlineServicePass) {
return;
}
$inlinedIds = $this->inlineServicePass->getInlinedServiceIds();
$exceptions = $this->autowirePass->getAutowiringExceptions();
// free up references
$this->autowirePass = null;
$this->inlineServicePass = null;
foreach ($exceptions as $exception) {
if ($container->hasDefinition($exception->getServiceId()) || in_array($exception->getServiceId(), $inlinedIds)) {
throw $exception;
}
}
@@ -23,6 +23,7 @@
class InlineServiceDefinitionsPass extends AbstractRecursivePass implements RepeatablePassInterface
{
private $repeatedPass;
private $inlinedServiceIds = array();
/**
* {@inheritdoc}
@@ -32,6 +33,16 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
$this->repeatedPass = $repeatedPass;
}
/**
* Returns an array of all services inlined by this pass.
*
* @return array Service id strings
*/
public function getInlinedServiceIds()
{
return $this->inlinedServiceIds;
}
/**
* {@inheritdoc}
*/
@@ -46,6 +57,7 @@ protected function processValue($value, $isRoot = false)
if ($this->isInlineableDefinition($id, $definition, $this->container->getCompiler()->getServiceReferenceGraph())) {
$this->container->log($this, sprintf('Inlined service "%s" to "%s".', $id, $this->currentId));
$this->inlinedServiceIds[] = $id;
if ($definition->isShared()) {
return $definition;
@@ -73,11 +73,11 @@ public function __construct()
new RemoveAbstractDefinitionsPass(),
new RepeatedPass(array(
new AnalyzeServiceReferencesPass(),
new InlineServiceDefinitionsPass(),
$inlinedServicePass = new InlineServiceDefinitionsPass(),
new AnalyzeServiceReferencesPass(),
new RemoveUnusedDefinitionsPass(),
)),
new AutowireExceptionPass($autowirePass),
new AutowireExceptionPass($autowirePass, $inlinedServicePass),
new CheckExceptionOnInvalidReferenceBehaviorPass(),
));
}
@@ -14,6 +14,7 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Compiler\AutowireExceptionPass;
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
use Symfony\Component\DependencyInjection\Compiler\InlineServiceDefinitionsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
@@ -29,10 +30,45 @@ public function testThrowsException()
->method('getAutowiringExceptions')
->will($this->returnValue(array($autowireException)));
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
->getMock();
$inlinePass->expects($this->any())
->method('getInlinedServiceIds')
->will($this->returnValue(array()));
$container = new ContainerBuilder();
$container->register('foo_service_id');
$pass = new AutowireExceptionPass($autowirePass);
$pass = new AutowireExceptionPass($autowirePass, $inlinePass);
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 testThrowExceptionIfServiceInlined()
{
$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)));
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
->getMock();
$inlinePass->expects($this->any())
->method('getInlinedServiceIds')
->will($this->returnValue(array('foo_service_id')));
// don't register the foo_service_id service
$container = new ContainerBuilder();
$pass = new AutowireExceptionPass($autowirePass, $inlinePass);
try {
$pass->process($container);
@@ -52,9 +88,15 @@ public function testNoExceptionIfServiceRemoved()
->method('getAutowiringExceptions')
->will($this->returnValue(array($autowireException)));
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
->getMock();
$inlinePass->expects($this->any())
->method('getInlinedServiceIds')
->will($this->returnValue(array()));
$container = new ContainerBuilder();
$pass = new AutowireExceptionPass($autowirePass);
$pass = new AutowireExceptionPass($autowirePass, $inlinePass);
$pass->process($container);
// mark the test as passed
@@ -252,6 +252,30 @@ public function testProcessDoesNotSetLazyArgumentValuesAfterInlining()
$this->assertSame('inline', (string) $values[0]);
}
public function testGetInlinedServiceIds()
{
$container = new ContainerBuilder();
$container
->register('inlinable.service')
->setPublic(false)
;
$container
->register('non_inlinable.service')
->setPublic(true)
;
$container
->register('service')
->setArguments(array(new Reference('inlinable.service')))
;
$inlinePass = new InlineServiceDefinitionsPass();
$repeatedPass = new RepeatedPass(array(new AnalyzeServiceReferencesPass(), $inlinePass));
$repeatedPass->process($container);
$this->assertEquals(array('inlinable.service'), $inlinePass->getInlinedServiceIds());
}
protected function process(ContainerBuilder $container)
{
$repeatedPass = new RepeatedPass(array(new AnalyzeServiceReferencesPass(), new InlineServiceDefinitionsPass()));

0 comments on commit 2f4dea5

Please sign in to comment.
You can’t perform that action at this time.