8000 bug #28366 [DI] Fix dumping some complex service graphs (nicolas-grekas) · symfony/symfony@8851c14 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8851c14

Browse files
nicolas-grekasnicolas-grekas
committed
bug #28366 [DI] Fix dumping some complex service graphs (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- [DI] Fix dumping some complex service graphs | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28296 (and its duplicates #28355 & #28362 ~+ possibly #28304~) | License | MIT | Doc PR | - Commits ------- 769fd4b [DI] Fix dumping some complex service graphs
2 parents edbd869 + 769fd4b commit 8851c14

File tree

7 files changed

+152
-10
lines changed

7 files changed

+152
-10
lines changed

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ private function addServiceInlinedDefinitions($id, Definition $definition, \SplO
516516

517517
$code .= $this->addNewInstance($def, '$'.$name, ' = ', $id);
518518

519-
if (!$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) {
519+
if (!$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true, $inlinedDefinitions)) {
520520
$code .= $this->addServiceProperties($def, $name);
521521
$code .= $this->addServiceMethodCalls($def, $name);
522522
$code .= $this->addServiceConfigurator($def, $name);
@@ -669,7 +669,7 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition,
669669
{
670670
$code = '';
671671
foreach ($inlinedDefinitions as $def) {
672-
if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) {
672+
if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true, $inlinedDefinitions)) {
673673
continue;
674674
}
675675

@@ -812,6 +812,7 @@ protected function {$methodName}($lazyInitialization)
812812

813813
$inlinedDefinitions = $this->getDefinitionsFromArguments(array($definition));
814814
$constructorDefinitions = $this->getDefinitionsFromArguments(array($definition->getArguments(), $definition->getFactory()));
815+
unset($constructorDefinitions[$definition]); // ensures $definition will be last
815816
$otherDefinitions = new \SplObjectStorage();
816817
$serviceCalls = array();
817818

@@ -1152,6 +1153,9 @@ private function addRemovedIds()
11521153
$ids = array_keys($ids);
11531154
sort($ids);
11541155
foreach ($ids as $id) {
1156+
if (preg_match('/^\d+_[^~]++~[._a-zA-Z\d]{7}$/', $id)) {
1157+
continue;
1158+
}
11551159
$code .= ' '.$this->doExport($id)." => true,\n";
11561160
}
11571161

@@ -1635,15 +1639,15 @@ private function getDefinitionsFromArguments(array $arguments, $isConstructorArg
16351639
*
16361640
* @return bool
16371641
*/
1638-
private function hasReference($id, array $arguments, $deep = false, array &$visited = array())
1642+
private function hasReference($id, array $arguments, $deep = false, \SplObjectStorage $inlinedDefinitions = null, array &$visited = array())
16391643
{
16401644
if (!isset($this->circularReferences[$id])) {
16411645
return false;
16421646
}
16431647

16441648
foreach ($arguments as $argument) {
16451649
if (\is_array($argument)) {
1646-
if ($this->hasReference($id, $argument, $deep, $visited)) {
1650+
if ($this->hasReference($id, $argument, $deep, $inlinedDefinitions, $visited)) {
16471651
return true;
16481652
}
16491653

@@ -1662,6 +1666,9 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi
16621666

16631667
$service = $this->container->getDefinition($argumentId);
16641668
} elseif ($argument instanceof Definition) {
1669+
if (isset($inlinedDefinitions[$argument])) {
1670+
return true;
1671+
}
16651672
$service = $argument;
16661673
} else {
16671674
continue;
@@ -1673,7 +1680,7 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi
16731680
continue;
16741681
}
16751682

1676-
if ($this->hasReference($id, array($service->getArguments(), $service->getFactory(), $service->getProperties(), $service->getMethodCalls(), $service->getConfigurator()), $deep, $visited)) {
1683+
if ($this->hasReference($id, array($service->getArguments(), $service->getFactory(), $service->getProperties(), $service->getMethodCalls(), $service->getConfigurator()), $deep, $inlinedDefinitions, $visited)) {
16771684
return true;
16781685
}
16791686
}

‎src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file, $defaults)
402402
{
403403
$definitions = array();
404404
$count = 0;
405-
$suffix = ContainerBuilder::hash($file);
405+
$suffix = '~'.ContainerBuilder::hash($file);
406406

407407
$xpath = new \DOMXPath($xml);
408408
$xpath->registerNamespace('container', self::NS);
@@ -412,7 +412,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file, $defaults)
412412
foreach ($nodes as $node) {
413413
if ($services = $this->getChildren($node, 'service')) {
414414
// give it a unique name
415-
$id = sprintf('%d_%s', ++$count, preg_replace('/^.*\\\\/', '', $services[0]->getAttribute('class')).'~'.$suffix);
415+
$id = sprintf('%d_%s', ++$count, preg_replace('/^.*\\\\/', '', $services[0]->getAttribute('class')).$suffix);
416416
$node->setAttribute('id', $id);
417417
$node->setAttribute('service', $id);
418418

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function load($resource, $type = null)
142142

143143
// services
144144
$this->anonymousServicesCount = 0;
145-
$this->anonymousServicesSuffix = ContainerBuilder::hash($path);
145+
$this->anonymousServicesSuffix = '~'.ContainerBuilder::hash($path);
146146
$this->setCurrentDir(\dirname($path));
147147
try {
148148
$this->parseDefinitions($content, $path);

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,21 @@ public function provideAlmostCircular()
838838
yield array('private');
839839
}
840840

841+
public function testDeepServiceGraph()
842+
{
843+
$container = new ContainerBuilder();
844+
845+
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
846+
$loader->load('services_deep_graph.yml');
847+
848+
$container->compile();
849+
850+
$dumper = new PhpDumper($container);
851+
$dumper->dump();
852+
853+
$this->assertStringEqualsFile(self::$fixturesPath.'/php/services_deep_graph.php', $dumper->dump());
854+
}
855+
841856
public function testHotPathOptimizations()
842857
{
843858
$container = include self::$fixturesPath.'/containers/container_inline_requires.php';

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_deep_graph.php

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
3+
use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
4+
use Symfony\Component\DependencyInjection\ContainerInterface;
5+
use Symfony\Component\DependencyInjection\Container;
6+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
7+
use Symfony\Component\DependencyInjection\Exception\LogicException;
8+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
9+
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
10+
11+
/**
12+
* This class has been auto-generated
13+
* by the Symfony Dependency Injection Component.
14+
*
15+
* @final since Symfony 3.3
16+
*/
17+
class ProjectServiceContainer extends Container
18+
{
19+
private $parameters;
20+
private $targetDirs = array();
21+
22+
public function __construct()
23+
{
24+
$this->services = array();
25+
$this->methodMap = array(
26+
'bar' => 'getBarService',
27+
'foo' => 'getFooService',
28+
);
29+
30+
$this->aliases = array();
31+
}
32+
33+
public function getRemovedIds()
34+
{
35+
return array(
36+
'Psr\\Container\\ContainerInterface' => true,
37+
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
38+
);
39+
}
40+
41+
public function compile()
42+
{
43+
throw new LogicException('You cannot compile a dumped container that was already compiled.');
44+
}
45+
46+
public function isCompiled()
47+
{
48+
return true;
49+
}
50+
51+
public function isFrozen()
52+
{
53+
@trigger_error(sprintf('The %s() method is deprecated since Symfony 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);
54+
55+
return true;
56+
}
57+
58+
/**
59+
* Gets the public 'bar' shared service.
60+
*
61+
* @return \c5
62+
*/
63+
protected function getBarService()
64+
{
65+
$this->services['bar'] = $instance = new \c5();
66+
67+
$instance->p5 = new \c6(${($_ = isset($this->services['foo']) ? $this->services['foo'] : $this->getFooService()) && false ?: '_'});
68+
69+
return $instance;
70+
}
71+
72+
/**
73+
* Gets the public 'foo' shared service.
74+
*
75+
* @return \c1
76+
*/
77+
protected function getFooService()
78+
{
79+
$a = ${($_ = isset($this->services['bar']) ? $this->services['bar'] : $this->getBarService()) && false ?: '_'};
80+
81+
if (isset($this->services['foo'])) {
82+
return $this->services['foo'];
83+
}
84+
85+
$b = new \c2();
86+
87+
$this->services['foo'] = $instance = new \c1($a, $b);
88+
89+
$c = new \c3();
90+
91+
$c->p3 = new \c4();
92+
$b->p2 = $c;
93+
94+
return $instance;
95+
}
96+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_deep_graph.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
2+
services:
3+
foo:
4+
class: c1
5+
public: true
6+
arguments:
7+
- '@bar'
8+
- !service
9+
class: c2
10+
properties:
11+
p2: !service
12+
class: c3
13+
properties:
14+
p3: !service
15+
class: c4
16+
17+
bar:
18+
class: c5
19+
public: true
20+
properties:
21+
p5: !service
22+
class: c6
23+
arguments: ['@foo']
24+

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ public function testAnonymousServices()
586586
$this->assertCount(1, $args);
587587
$this->assertInstanceOf(Reference::class, $args[0]);
588588
$this->assertTrue($container->has((string) $args[0]));
589-
$this->assertRegExp('/^\d+_Bar[._A-Za-z0-9]{7}$/', (string) $args[0]);
589+
$this->assertRegExp('/^\d+_Bar~[._A-Za-z0-9]{7}$/', (string) $args[0]);
590590

591591
$anonymous = $container->getDefinition((string) $args[0]);
592592
$this->assertEquals('Bar', $anonymous->getClass());
@@ -598,7 +598,7 @@ public function testAnonymousServices()
598598
$this->assertInternalType('array', $factory);
599599
$this->assertInstanceOf(Reference::class, $factory[0]);
600600
$this->assertTrue($container->has((string) $factory[0]));
601-
$this->assertRegExp('/^\d+_Quz[._A-Za-z0-9]{7}$/', (string) $factory[0]);
601+
$this->assertRegExp('/^\d+_Quz~[._A-Za-z0-9]{7}$/', (string) $factory[0]);
602602
$this->assertEquals('constructFoo', $factory[1]);
603603

604604
$anonymous = $container->getDefinition((string) $factory[0]);

0 commit comments

Comments
 (0)
0