Skip to content

Commit 85016e8

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

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
@@ -128,7 +128,7 @@ protected function processValue($value, bool $isRoot = false)
128128
$this->lazy = false;
129129

130130
$byConstructor = $this->byConstructor;
131-
$this->byConstructor = true;
131+
$this->byConstructor = $isRoot || $byConstructor;
132132
$this->processValue($value->getFactory());
133133
$this->processValue($value->getArguments());
134134

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

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

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

@@ -343,10 +344,10 @@ private function getProxyDumper(): ProxyDumper
343344
/**
344345
* @param ServiceReferenceGraphEdge[] $edges
345346
*/
346-
private function analyzeCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$currentPath = [])
347+
private function analyzeCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$currentPath = [], bool $byConstructor = true)
347348
{
348349
$checkedNodes[$sourceId] = true;
349-
$currentPath[$sourceId] = $sourceId;
350+
$currentPath[$sourceId] = $byConstructor;
350351

351352
foreach ($edges as $edge) {
352353
$node = $edge->getDestNode();
@@ -355,44 +356,52 @@ private function analyzeCircularReferences(string $sourceId, array $edges, array
355356
if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
356357
// no-op
357358
} elseif (isset($currentPath[$id])) {
358-
$currentId = $id;
359-
foreach (array_reverse($currentPath) as $parentId) {
360-
$this->circularReferences[$parentId][$currentId] = $currentId;
361-
if ($parentId === $id) {
362-
break;
363-
}
364-
$currentId = $parentId;
365-
}
359+
$this->addCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
366360
} elseif (!isset($checkedNodes[$id])) {
367-
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath);
361+
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath, $edge->isReferencedByConstructor());
368362
} elseif (isset($this->circularReferences[$id])) {
369-
$this->connectCircularReferences($id, $currentPath);
363+
$this->connectCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
370364
}
371365
}
372366
unset($currentPath[$sourceId]);
373367
}
374368

375-
private function connectCircularReferences(string $sourceId, array &$currentPath, array &$subPath = [])
369+
private function connectCircularReferences(string $sourceId, array &$currentPath, bool $byConstructor, array &$subPath = [])
376370
{
377-
$subPath[$sourceId] = $sourceId;
378-
$currentPath[$sourceId] = $sourceId;
371+
$currentPath[$sourceId] = $subPath[$sourceId] = $byConstructor;
379372

380-
foreach ($this->circularReferences[$sourceId] as $id) {
373+
foreach ($this->circularReferences[$sourceId] as $id => $byConstructor) {
381374
if (isset($currentPath[$id])) {
382-
$currentId = $id;
383-
foreach (array_reverse($currentPath) as $parentId) {
384-
$this->circularReferences[$parentId][$currentId] = $currentId;
385-
if ($parentId === $id) {
386-
break;
387-
}
388-
$currentId = $parentId;
389-
}
375+
$this->addCircularReferences($id, $currentPath, $byConstructor);
390376
} elseif (!isset($subPath[$id]) && isset($this->circularReferences[$id])) {
391-
$this->connectCircularReferences($id, $currentPath, $subPath);
377+
$this->connectCircularReferences($id, $currentPath, $byConstructor, $subPath);
392378
}
393379
}
394-
unset($currentPath[$sourceId]);
395-
unset($subPath[$sourceId]);
380+
unset($currentPath[$sourceId], $subPath[$sourceId]);
381+
}
382+
383+
private function addCircularReferences(string $id, array $currentPath, bool $byConstructor)
384+
{
385+
$currentPath[$id] = $byConstructor;
386+
$circularRefs = [];
387+
388+
foreach (array_reverse($currentPath) as $parentId => $v) {
389+
$byConstructor = $byConstructor && $v;
390+
$circularRefs[] = $parentId;
391+
392+
if ($parentId === $id) {
393+
break;
394+
}
395+
}
396+
397+
$currentId = $id;
398+
foreach ($circularRefs as $parentId) {
399+
if (empty($this->circularReferences[$parentId][$currentId])) {
400+
$this->circularReferences[$parentId][$currentId] = $byConstructor;
401+
}
402+
403+
$currentId = $parentId;
404+
}
396405
}
397406

398407
private function collectLineage(string $class, array &$lineage)
@@ -683,7 +692,6 @@ private function addService(string $id, Definition $definition): array
683692
$autowired = $definition->isAutowired() ? ' autowired' : '';
684693

685694
if ($definition->isLazy()) {
686-
unset($this->circularReferences[$id]);
687695
$lazyInitialization = '$lazyLoad = true';
688696
} else {
689697
$lazyInitialization = '';
@@ -759,12 +767,12 @@ private function addInlineVariables(string $id, Definition $definition, array $a
759767

760768
private function addInlineReference(string $id, Definition $definition, string $targetId, bool $forConstructor): string
761769
{
762-
list($callCount, $behavior) = $this->serviceCalls[$targetId];
763-
764770
while ($this->container->hasAlias($targetId)) {
765771
$targetId = (string) $this->container->getAlias($targetId);
766772
}
767773

774+
list($callCount, $behavior) = $this->serviceCalls[$targetId];
775+
768776
if ($id === $targetId) {
769777
return $this->addInlineService($id, $definition, $definition);
770778
}
@@ -773,9 +781,13 @@ private function addInlineReference(string $id, Definition $definition, string $
773781
return '';
774782
}
775783

776-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
777-
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
778-
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
784+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]);
785+
786+
if ($hasSelfRef && !$forConstructor && !$forConstructor = !$this->circularReferences[$id][$targetId]) {
787+
$code = $this->addInlineService($id, $definition, $definition);
788+
} else {
789+
$code = '';
790+
}
779791

780792
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
781793
return $code;
@@ -808,15 +820,23 @@ private function addInlineReference(string $id, Definition $definition, string $
808820

809821
private function addInlineService(string $id, Definition $definition, Definition $inlineDef = null, bool $forConstructor = true): string
810822
{
811-
$isSimpleInstance = $isRootInstance = null === $inlineDef;
823+
$code = '';
824+
825+
if ($isSimpleInstance = $isRootInstance = null === $inlineDef) {
826+
foreach ($this->serviceCalls as $targetId => list($callCount, $behavior, $byConstructor)) {
827+
if ($byConstructor && isset($this->circularReferences[$id][$targetId]) && !$this->circularReferences[$id][$targetId]) {
828+
$code .= $this->addInlineReference($id, $definition, $targetId, $forConstructor);
829+
}
830+
}
831+
}
812832

813833
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
814-
return '';
834+
return $code;
815835
}
816836

817837
$arguments = [$inlineDef->getArguments(), $inlineDef->getFactory()];
818838

819-
$code = $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
839+
$code .= $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
820840

821841
if ($arguments = array_filter([$inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()])) {
822842
$isSimpleInstance = false;
@@ -1475,20 +1495,24 @@ private function getServiceConditionals($value): string
14751495
return implode(' && ', $conditions);
14761496
}
14771497

1478-
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = []): \SplObjectStorage
1498+
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = [], bool $byConstructor = null): \SplObjectStorage
14791499
{
14801500
if (null === $definitions) {
14811501
$definitions = new \SplObjectStorage();
14821502
}
14831503

14841504
foreach ($arguments as $argument) {
14851505
if (\is_array($argument)) {
1486-
$this->getDefinitionsFromArguments($argument, $definitions, $calls);
1506+
$this->getDefinitionsFromArguments($argument, $definitions, $calls, $byConstructor);
14871507
} elseif ($argument instanceof Reference) {
14881508
$id = (string) $argument;
14891509

1510+
while ($this->container->hasAlias($id)) {
1511+
$id = (string) $this->container->getAlias($id);
1512+
}
1513+
14901514
if (!isset($calls[$id])) {
1491-
$calls[$id] = [0, $argument->getInvalidBehavior()];
1515+
$calls[$id] = [0, $argument->getInvalidBehavior(), $byConstructor];
14921516
} else {
14931517
$calls[$id][1] = min($calls[$id][1], $argument->getInvalidBehavior());
14941518
}
@@ -1500,8 +1524,10 @@ private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage
15001524
$definitions[$argument] = 1 + $definitions[$argument];
15011525
} else {
15021526
$definitions[$argument] = 1;
1503-
$arguments = [$argument->getArguments(), $argument->getFactory(), $argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
1504-
$this->getDefinitionsFromArguments($arguments, $definitions, $calls);
1527+
$arguments = [$argument->getArguments(), $argument->getFactory()];
1528+
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null === $byConstructor || $byConstructor);
1529+
$arguments = [$argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
1530+
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null !== $byConstructor && $byConstructor);
15051531
}
15061532
}
15071533

@@ -1622,6 +1648,11 @@ private function dumpValue($value, bool $interpolate = true): string
16221648
return '$'.$value;
16231649
} elseif ($value instanceof Reference) {
16241650
$id = (string) $value;
1651+
1652+
while ($this->container->hasAlias($id)) {
1653+
$id = (string) $this->container->getAlias($id);
1654+
}
1655+
16251656
if (null !== $this->referenceVariables && isset($this->referenceVariables[$id])) {
16261657
return $this->dumpValue($this->referenceVariables[$id], $interpolate);
16271658
}

‎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)