bug #12393 [DependencyInjection] inlined factory not referenced (boekkooi)
This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #12393).
Discussion
----------
[DependencyInjection] inlined factory not referenced
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
While working with the DI I encountered a `You have requested a non-existent service "xxxxx"` exception.
Research it seems that a private reference was inlined but the private factory that was also referencing to the service was ignored.
A example of the problem:
```XML
<service id="manager"
class="\stdClass"
factory-method="getX"
factory-service="factory"
public="false">
<argument>X</argument>
</service>
<service id="repository"
class="\stdClass"
factory-method="getRepository"
factory-service="manager"
public="false">
<argument>X</argument>
</service>
<service id="storage" class="\stdClass" public="false">
<argument type="service" id="manager"/>
<argument type="service" id="repository"/>
</service>
```
What happens before the patch:
1. repository get's inlined
2. manager get's inlined for the first argument
3. manager get's removed since there was no reference.
After the first commit the following will happen:
1. repository get's inlined
2. manager get's inlined for the first argument
3. manager will not be removed since the inlined repository still references manager
This introduced a smell since InlineServiceDefinitionsPass was still inlining the manager for the first argument.
To fix this I have chosen that not inline factories if they are used more then once by the same definition.
So after the second commit the following will happen:
1. repository get's inlined
Personally I feel that the InlineServiceDefinitionsPass patch isn't the best possible one but that a different fix would probably mean breaking BC so it's probably a good idea to look at this for Symfony 3.0.
Commits
-------
7816a98
[DependencyInjection] inlined factory not referenced
This commit is contained in:
commit
0cbba7f317
@ -111,6 +111,10 @@ class AnalyzeServiceReferencesPass implements RepeatablePassInterface
|
|||||||
$this->processArguments($argument->getArguments());
|
$this->processArguments($argument->getArguments());
|
||||||
$this->processArguments($argument->getMethodCalls());
|
$this->processArguments($argument->getMethodCalls());
|
||||||
$this->processArguments($argument->getProperties());
|
$this->processArguments($argument->getProperties());
|
||||||
|
|
||||||
|
if ($argument->getFactoryService()) {
|
||||||
|
$this->processArguments(array(new Reference($argument->getFactoryService())));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -138,6 +138,10 @@ class InlineServiceDefinitionsPass implements RepeatablePassInterface
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (count($ids) > 1 && $definition->getFactoryService()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
return $container->getDefinition(reset($ids))->getScope() === $definition->getScope();
|
return $container->getDefinition(reset($ids))->getScope() === $definition->getScope();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -79,6 +79,28 @@ class AnalyzeServiceReferencesPassTest extends \PHPUnit_Framework_TestCase
|
|||||||
$this->assertSame($ref, $refs[0]->getValue());
|
$this->assertSame($ref, $refs[0]->getValue());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testProcessDetectsReferencesFromInlinedFactoryDefinitions()
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
|
||||||
|
$container
|
||||||
|
->register('a')
|
||||||
|
;
|
||||||
|
|
||||||
|
$factory = new Definition();
|
||||||
|
$factory->setFactoryService('a');
|
||||||
|
|
||||||
|
$container
|
||||||
|
->register('b')
|
||||||
|
->addArgument($factory)
|
||||||
|
;
|
||||||
|
|
||||||
|
$graph = $this->process($container);
|
||||||
|
|
||||||
|
$this->assertTrue($graph->hasNode('a'));
|
||||||
|
$this->assertCount(1, $refs = $graph->getNode('a')->getInEdges());
|
||||||
|
}
|
||||||
|
|
||||||
public function testProcessDoesNotSaveDuplicateReferences()
|
public function testProcessDoesNotSaveDuplicateReferences()
|
||||||
{
|
{
|
||||||
$container = new ContainerBuilder();
|
$container = new ContainerBuilder();
|
||||||
|
@ -110,6 +110,84 @@ class InlineServiceDefinitionsPassTest extends \PHPUnit_Framework_TestCase
|
|||||||
$this->assertSame($a, $inlinedArguments[0]);
|
$this->assertSame($a, $inlinedArguments[0]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testProcessInlinesPrivateFactoryReference()
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
|
||||||
|
$container->register('a')->setPublic(false);
|
||||||
|
$b = $container
|
||||||
|
->register('b')
|
||||||
|
->setPublic(false)
|
||||||
|
->setFactoryService('a')
|
||||||
|
;
|
||||||
|
|
||||||
|
$container
|
||||||
|
->register('foo')
|
||||||
|
->setArguments(array(
|
||||||
|
$ref = new Reference('b'),
|
||||||
|
));
|
||||||
|
|
||||||
|
$this->process($container);
|
||||||
|
|
||||||
|
$inlinedArguments = $container->getDefinition('foo')->getArguments();
|
||||||
|
$this->assertSame($b, $inlinedArguments[0]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testProcessDoesNotInlinePrivateFactoryIfReferencedMultipleTimesWithinTheSameDefinition()
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
$container
|
||||||
|
->register('a')
|
||||||
|
;
|
||||||
|
$container
|
||||||
|
->register('b')
|
||||||
|
->setPublic(false)
|
||||||
|
->setFactoryService('a')
|
||||||
|
;
|
||||||
|
|
||||||
|
$container
|
||||||
|
->register('foo')
|
||||||
|
->setArguments(array(
|
||||||
|
$ref1 = new Reference('b'),
|
||||||
|
$ref2 = new Reference('b'),
|
||||||
|
))
|
||||||
|
;
|
||||||
|
$this->process($container);
|
||||||
|
|
||||||
|
$args = $container->getDefinition('foo')->getArguments();
|
||||||
|
$this->assertSame($ref1, $args[0]);
|
||||||
|
$this->assertSame($ref2, $args[1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testProcessDoesNotInlineReferenceWhenUsedByInlineFactory()
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
$container
|
||||||
|
->register('a')
|
||||||
|
;
|
||||||
|
$container
|
||||||
|
->register('b')
|
||||||
|
->setPublic(false)
|
||||||
|
->setFactoryService('a')
|
||||||
|
;
|
||||||
|
|
||||||
|
$inlineFactory = new Definition();
|
||||||
|
$inlineFactory->setPublic(false);
|
||||||
|
$inlineFactory->setFactoryService('b');
|
||||||
|
|
||||||
|
$container
|
||||||
|
->register('foo')
|
||||||
|
->setArguments(array(
|
||||||
|
$ref = new Reference('b'),
|
||||||
|
$inlineFactory,
|
||||||
|
))
|
||||||
|
;
|
||||||
|
$this->process($container);
|
||||||
|
|
||||||
|
$args = $container->getDefinition('foo')->getArguments();
|
||||||
|
$this->assertSame($ref, $args[0]);
|
||||||
|
}
|
||||||
|
|
||||||
public function testProcessInlinesOnlyIfSameScope()
|
public function testProcessInlinesOnlyIfSameScope()
|
||||||
{
|
{
|
||||||
$container = new ContainerBuilder();
|
$container = new ContainerBuilder();
|
||||||
|
Reference in New Issue
Block a user