Skip to content

Commit b2dadc1

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.3' into 4.4
* 4.3: [DI] Fix dumping Doctrine-like service graphs (bis) Failing test case for complex near-circular situation + lazy
2 parents f453503 + 5196254 commit b2dadc1

File tree

7 files changed

+282
-43
lines changed

7 files changed

+282
-43
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ protected function processValue($value, $isRoot = false)
136136
$this->lazy = false;
137137

138138
$byConstructor = $this->byConstructor;
139-
$this->byConstructor = true;
139+
$this->byConstructor = $isRoot || $byConstructor;
140140
$this->processValue($value->getFactory());
141141
$this->processValue($value->getArguments());
142142

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php‎

Lines changed: 72 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ public function dump(array $options = [])
187187
}
188188
$this->container->getCompiler()->getServiceReferenceGraph()->clear();
189189
$checkedNodes = [];
190+
$this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
190191

191192
$this->docStar = $options['debug'] ? '*' : '';
192193

@@ -339,10 +340,10 @@ private function getProxyDumper(): ProxyDumper
339340
return $this->proxyDumper;
340341
}
341342

342-
private function analyzeCircularReferences($sourceId, array $edges, &$checkedNodes, &$currentPath = [])
343+
private function analyzeCircularReferences($sourceId, array $edges, &$checkedNodes, &$currentPath = [], $byConstructor = true)
343344
{
344345
$checkedNodes[$sourceId] = true;
345-
$currentPath[$sourceId] = $sourceId;
346+
$currentPath[$sourceId] = $byConstructor;
346347

347348
foreach ($edges as $edge) {
348349
$node = $edge->getDestNode();
@@ -351,44 +352,52 @@ private function analyzeCircularReferences($sourceId, array $edges, &$checkedNod
351352
if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
352353
// no-op
353354
} elseif (isset($currentPath[$id])) {
354-
$currentId = $id;
355-
foreach (array_reverse($currentPath) as $parentId) {
356-
$this->circularReferences[$parentId][$currentId] = $currentId;
357-
if ($parentId === $id) {
358-
break;
359-
}
360-
$currentId = $parentId;
361-
}
355+
$this->addCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
362356
} elseif (!isset($checkedNodes[$id])) {
363-
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath);
357+
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath, $edge->isReferencedByConstructor());
364358
} elseif (isset($this->circularReferences[$id])) {
365-
$this->connectCircularReferences($id, $currentPath);
359+
$this->connectCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
366360
}
367361
}
368362
unset($currentPath[$sourceId]);
369363
}
370364

371-
private function connectCircularReferences($sourceId, &$currentPath, &$subPath = [])
365+
private function connectCircularReferences($sourceId, &$currentPath, $byConstructor, &$subPath = [])
372366
{
373-
$subPath[$sourceId] = $sourceId;
374-
$currentPath[$sourceId] = $sourceId;
367+
$currentPath[$sourceId] = $subPath[$sourceId] = $byConstructor;
375368

376-
foreach ($this->circularReferences[$sourceId] as $id) {
369+
foreach ($this->circularReferences[$sourceId] as $id => $byConstructor) {
377370
if (isset($currentPath[$id])) {
378-
$currentId = $id;
379-
foreach (array_reverse($currentPath) as $parentId) {
380-
$this->circularReferences[$parentId][$currentId] = $currentId;
381-
if ($parentId === $id) {
382-
break;
383-
}
384-
$currentId = $parentId;
385-
}
371+
$this->addCircularReferences($id, $currentPath, $byConstructor);
386372
} elseif (!isset($subPath[$id]) && isset($this->circularReferences[$id])) {
387-
$this->connectCircularReferences($id, $currentPath, $subPath);
373+
$this->connectCircularReferences($id, $currentPath, $byConstructor, $subPath);
388374
}
389375
}
390-
unset($currentPath[$sourceId]);
391-
unset($subPath[$sourceId]);
376+
unset($currentPath[$sourceId], $subPath[$sourceId]);
377+
}
378+
379+
private function addCircularReferences($id, $currentPath, $byConstructor)
380+
{
381+
$currentPath[$id] = $byConstructor;
382+
$circularRefs = [];
383+
384+
foreach (array_reverse($currentPath) as $parentId => $v) {
385+
$byConstructor = $byConstructor && $v;
386+
$circularRefs[] = $parentId;
387+
388+
if ($parentId === $id) {
389+
break;
390+
}
391+
}
392+
393+
$currentId = $id;
394+
foreach ($circularRefs as $parentId) {
395+
if (empty($this->circularReferences[$parentId][$currentId])) {
396+
$this->circularReferences[$parentId][$currentId] = $byConstructor;
397+
}
398+
399+
$currentId = $parentId;
400+
}
392401
}
393402

394403
private function collectLineage($class, array &$lineage)
@@ -679,7 +688,6 @@ private function addService(string $id, Definition $definition): array
679688
$autowired = $definition->isAutowired() ? ' autowired' : '';
680689

681690
if ($definition->isLazy()) {
682-
unset($this->circularReferences[$id]);
683691
$lazyInitialization = '$lazyLoad = true';
684692
} else {
685693
$lazyInitialization = '';
@@ -755,12 +763,12 @@ private function addInlineVariables(string $id, Definition $definition, array $a
755763

756764
private function addInlineReference(string $id, Definition $definition, string $targetId, bool $forConstructor): string
757765
{
758-
list($callCount, $behavior) = $this->serviceCalls[$targetId];
759-
760766
while ($this->container->hasAlias($targetId)) {
761767
$targetId = (string) $this->container->getAlias($targetId);
762768
}
763769

770+
list($callCount, $behavior) = $this->serviceCalls[$targetId];
771+
764772
if ($id === $targetId) {
765773
return $this->addInlineService($id, $definition, $definition);
766774
}
@@ -769,9 +777,13 @@ private function addInlineReference(string $id, Definition $definition, string $
769777
return '';
770778
}
771779

772-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
773-
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
774-
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
780+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]);
781+
782+
if ($hasSelfRef && !$forConstructor && !$forConstructor = !$this->circularReferences[$id][$targetId]) {
783+
$code = $this->addInlineService($id, $definition, $definition);
784+
} else {
785+
$code = '';
786+
}
775787

776788
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
777789
return $code;
@@ -804,15 +816,23 @@ private function addInlineReference(string $id, Definition $definition, string $
804816

805817
private function addInlineService(string $id, Definition $definition, Definition $inlineDef = null, bool $forConstructor = true): string
806818
{
807-
$isSimpleInstance = $isRootInstance = null === $inlineDef;
819+
$code = '';
820+
821+
if ($isSimpleInstance = $isRootInstance = null === $inlineDef) {
822+
foreach ($this->serviceCalls as $targetId => list($callCount, $behavior, $byConstructor)) {
823+
if ($byConstructor && isset($this->circularReferences[$id][$targetId]) && !$this->circularReferences[$id][$targetId]) {
824+
$code .= $this->addInlineReference($id, $definition, $targetId, $forConstructor);
825+
}
826+
}
827+
}
808828

809829
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
810-
return '';
830+
return $code;
811831
}
812832

813833
$arguments = [$inlineDef->getArguments(), $inlineDef->getFactory()];
814834

815-
$code = $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
835+
$code .= $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
816836

817837
if ($arguments = array_filter([$inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()])) {
818838
$isSimpleInstance = false;
@@ -1473,20 +1493,24 @@ private function getServiceConditionals($value): string
14731493
return implode(' && ', $conditions);
14741494
}
14751495

1476-
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = []): \SplObjectStorage
1496+
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = [], bool $byConstructor = null): \SplObjectStorage
14771497
{
14781498
if (null === $definitions) {
14791499
$definitions = new \SplObjectStorage();
14801500
}
14811501

14821502
foreach ($arguments as $argument) {
14831503
if (\is_array($argument)) {
1484-
$this->getDefinitionsFromArguments($argument, $definitions, $calls);
1504+
$this->getDefinitionsFromArguments($argument, $definitions, $calls, $byConstructor);
14851505
} elseif ($argument instanceof Reference) {
14861506
$id = (string) $argument;
14871507

1508+
while ($this->container->hasAlias($id)) {
1509+
$id = (string) $this->container->getAlias($id);
1510+
}
1511+
14881512
if (!isset($calls[$id])) {
1489-
$calls[$id] = [0, $argument->getInvalidBehavior()];
1513+
$calls[$id] = [0, $argument->getInvalidBehavior(), $byConstructor];
14901514
} else {
14911515
$calls[$id][1] = min($calls[$id][1], $argument->getInvalidBehavior());
14921516
}
@@ -1498,8 +1522,10 @@ private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage
14981522
$definitions[$argument] = 1 + $definitions[$argument];
14991523
} else {
15001524
$definitions[$argument] = 1;
1501-
$arguments = [$argument->getArguments(), $argument->getFactory(), $argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
1502-
$this->getDefinitionsFromArguments($arguments, $definitions, $calls);
1525+
$arguments = [$argument->getArguments(), $argument->getFactory()];
1526+
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null === $byConstructor || $byConstructor);
1527+
$arguments = [$argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
1528+
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null !== $byConstructor && $byConstructor);
15031529
}
15041530
}
15051531

@@ -1620,6 +1646,11 @@ private function dumpValue($value, bool $interpolate = true): string
16201646
return '$'.$value;
16211647
} elseif ($value instanceof Reference) {
16221648
$id = (string) $value;
1649+
1650+
while ($this->container->hasAlias($id)) {
1651+
$id = (string) $this->container->getAlias($id);
1652+
}
1653+
16231654
if (null !== $this->referenceVariables && isset($this->referenceVariables[$id])) {
16241655
return $this->dumpValue($this->referenceVariables[$id], $interpolate);
16251656
}

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,13 @@ public function testAlmostCircular($visibility)
14301430
$this->assertEquals((object) ['bar6' => (object) []], $foo6);
14311431

14321432
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
1433+
1434+
$manager3 = $container->get('manager3');
1435+
$listener3 = $container->get('listener3');
1436+
$this->assertSame($manager3, $listener3->manager, 'Both should identically be the manager3 service');
1437+
1438+
$listener4 = $container->get('listener4');
1439+
$this->assertInstanceOf('stdClass', $listener4);
14331440
}
14341441

14351442
public function provideAlmostCircular()

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,13 @@ public function testAlmostCircular($visibility)
996996
$this->assertEquals((object) ['bar6' => (object) []], $foo6);
997997

998998
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
999+
1000+
$manager3 = $container->get('manager3');
1001+
$listener3 = $container->get('listener3');
1002+
$this->assertSame($manager3, $listener3->manager);
1003+
1004+
$listener4 = $container->get('listener4');
1005+
$this->assertInstanceOf('stdClass', $listener4);
9991006
}
10001007

10011008
public function provideAlmostCircular()

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
use Symfony\Component\DependencyInjection\ContainerBuilder;
44
use Symfony\Component\DependencyInjection\Definition;
5-
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
65
use Symfony\Component\DependencyInjection\Reference;
76
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooForCircularWithAddCalls;
87

@@ -102,6 +101,35 @@
102101
$container->register('subscriber2', 'stdClass')->setPublic(false)
103102
->addArgument(new Reference('manager2'));
104103

104+
// doctrine-like event system with listener
105+
106+
$container->register('manager3', 'stdClass')
107+
->setLazy(true)
108+
->setPublic(true)
109+
->addArgument(new Reference('connection3'));
110+
111+
$container->register('connection3', 'stdClass')
112+
->setPublic($public)
113+
->setProperty('listener', [new Reference('listener3')]);
114+
115+
$container->register('listener3', 'stdClass')
116+
->setPublic(true)
117+
->setProperty('manager', new Reference('manager3'));
118+
119+
// doctrine-like event system with small differences
120+
121+
$container->register('manager4', 'stdClass')
122+
->setLazy(true)
123+
->addArgument(new Reference('connection4'));
124+
125+
$container->register('connection4', 'stdClass')
126+
->setPublic($public)
127+
->setProperty('listener', [new Reference('listener4')]);
128+
129+
$container->register('listener4', 'stdClass')
130+
->setPublic(true)
131+
->addArgument(new Reference('manager4'));
132+
105133
// private service involved in a loop
106134

107135
$container->register('foo6', 'stdClass')

0 commit comments

Comments
 (0)