Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 71209de

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[DI] Fix "almost-circular" dependencies handling
1 parent e973c24 commit 71209de
Copy full SHA for 71209de

12 files changed

+225
-31
lines changed

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+10-3Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
122122
private $autoconfiguredInstanceof = array();
123123

124124
private $removedIds = array();
125+
private $alreadyLoading = array();
125126

126127
public function __construct(ParameterBagInterface $parameterBag = null)
127128
{
@@ -594,12 +595,13 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
594595
throw $e;
595596
}
596597

597-
$this->loading[$id] = true;
598+
$loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading';
599+
$this->{$loading}[$id] = true;
598600

599601
try {
600602
$service = $this->createService($definition, $id);
601603
} finally {
602-
unset($this->loading[$id]);
604+
unset($this->{$loading}[$id]);
603605
}
604606

605607
return $service;
@@ -1089,6 +1091,10 @@ private function createService(Definition $definition, $id, $tryProxy = true)
10891091

10901092
$arguments = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())));
10911093

1094+
if (null !== $id && $definition->isShared() && isset($this->services[$id])) {
1095+
return $this->services[$id];
1096+
}
1097+
10921098
if (null !== $factory = $definition->getFactory()) {
10931099
if (is_array($factory)) {
10941100
$factory = array($this->resolveServices($parameterBag->resolveValue($factory[0])), $factory[1]);
@@ -1551,7 +1557,8 @@ private function callMethod($service, $call)
15511557
private function shareService(Definition $definition, $service, $id)
15521558
{
15531559
if (null !== $id && $definition->isShared()) {
1554-
$this->services[$this->normalizeId($id)] = $service;
1560+
$this->services[$id] = $service;
1561+
unset($this->loading[$id], $this->alreadyLoading[$id]);
15551562
}
15561563
}
15571564

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+25-10Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -261,13 +261,15 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra
261261

262262
array_unshift($inlinedDefinitions, $definition);
263263

264+
$isNonLazyShared = !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
264265
$calls = $behavior = array();
265266
foreach ($inlinedDefinitions as $iDefinition) {
266-
$this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior);
267-
$this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior);
268-
$this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior);
269-
$this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior);
270-
$this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior);
267+
$this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, (int) $isNonLazyShared);
268+
$callsInit = $isNonLazyShared && $iDefinition !== $definition && !$this->hasReference($cId, $iDefinition->getMethodCalls(), true) && !$this->hasReference($cId, $iDefinition->getProperties(), true);
269+
$this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior, (int) $callsInit);
270+
$this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior, (int) $callsInit);
271+
$this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior, (int) $callsInit);
272+
$this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior, (int) $isNonLazyShared);
271273
}
272274

273275
$code = '';
@@ -289,6 +291,16 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra
289291
}
290292

291293
if ('' !== $code) {
294+
if ($isNonLazyShared) {
295+
$code .= <<<EOTXT
296+
297+
if (isset(\$this->services['$cId'])) {
298+
return \$this->services['$cId'];
299+
}
300+
301+
EOTXT;
302+
}
303+
292304
$code .= "\n";
293305
}
294306

@@ -495,15 +507,15 @@ private function isTrivialInstance(Definition $definition)
495507
}
496508

497509
foreach ($definition->getArguments() as $arg) {
498-
if (!$arg || ($arg instanceof Reference && 'service_container' !== (string) $arg)) {
510+
if (!$arg || ($arg instanceof Reference && 'service_container' === (string) $arg)) {
499511
continue;
500512
}
501513
if (is_array($arg) && 3 >= count($arg)) {
502514
foreach ($arg as $k => $v) {
503515
if ($this->dumpValue($k) !== $this->dumpValue($k, false)) {
504516
return false;
505517
}
506-
if (!$v || ($v instanceof Reference && 'service_container' !== (string) $v)) {
518+
if (!$v || ($v instanceof Reference && 'service_container' === (string) $v)) {
507519
continue;
508520
}
509521
if (!is_scalar($v) || $this->dumpValue($v) !== $this->dumpValue($v, false)) {
@@ -1396,16 +1408,16 @@ private function getServiceConditionals($value)
13961408
/**
13971409
* Builds service calls from arguments.
13981410
*/
1399-
private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior)
1411+
private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior, $callsInit = 0)
14001412
{
14011413
foreach ($arguments as $argument) {
14021414
if (is_array($argument)) {
1403-
$this->getServiceCallsFromArguments($argument, $calls, $behavior);
1415+
$this->getServiceCallsFromArguments($argument, $calls, $behavior, $callsInit);
14041416
} elseif ($argument instanceof Reference) {
14051417
$id = (string) $argument;
14061418

14071419
if (!isset($calls[$id])) {
1408-
$calls[$id] = 0;
1420+
$calls[$id] = $callsInit;
14091421
}
14101422
if (!isset($behavior[$id])) {
14111423
$behavior[$id] = $argument->getInvalidBehavior();
@@ -1491,6 +1503,9 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi
14911503
if ($deep && !isset($visited[$argumentId]) && 'service_container' !== $argumentId) {
14921504
$visited[$argumentId] = true;
14931505

1506+
if (!$this->container->hasDefinition($argumentId)) {
1507+
continue;
1508+
}
14941509
$service = $this->container->getDefinition($argumentId);
14951510

14961511
// if the proxy manager is enabled, disable searching for references in lazy services,

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,15 @@ public function testUninitializedReference()
12031203
$this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
12041204
}
12051205

1206+
public function testAlmostCircular()
1207+
{
1208+
$container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
1209+
1210+
$foo = $container->get('foo');
1211+
1212+
$this->assertSame($foo, $foo->bar->foobar->foo);
1213+
}
1214+
12061215
public function testRegisterForAutoconfiguration()
12071216
{
12081217
$container = new ContainerBuilder();

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,19 @@ public function testUninitializedReference()
764764
$this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
765765
}
766766

767+
public function testAlmostCircular()
768+
{
769+
$container = include self::$fixturesPath.'/containers/container_almost_circular.php';
770+
$container->compile();
771+
$dumper = new PhpDumper($container);
772+
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Almost_Circular')));
773+
774+
$container = new \Symfony_DI_PhpDumper_Test_Almost_Circular();
775+
$foo = $container->get('foo');
776+
777+
$this->assertSame($foo, $foo->bar->foobar->foo);
778+
}
779+
767780
public function testDumpHandlesLiteralClassWithRootNamespace()
768781
{
769782
$container = new ContainerBuilder();

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

Copy file name to clipboard
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
use Symfony\Component\DependencyInjection\ContainerBuilder;
4+
use Symfony\Component\DependencyInjection\Definition;
5+
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
6+
use Symfony\Component\DependencyInjection\Reference;
7+
8+
$container = new ContainerBuilder();
9+
10+
$definition = new Definition(FoobarCircular::class, array(new Reference('foo')));
11+
$container->setDefinition('foobar', $definition);
12+
13+
$definition = new Definition(FooCircular::class, array(new Reference('bar')));
14+
$container->setDefinition('foo', $definition);
15+
16+
$definition = new Definition(BarCircular::class);
17+
$definition->addMethodCall('addFoobar', array(new Reference('foobar')));
18+
$container->setDefinition('bar', $definition);
19+
20+
return $container;

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,27 @@ public function __construct($lazyValues, $lazyEmptyValues)
110110
$this->lazyEmptyValues = $lazyEmptyValues;
111111
}
112112
}
113+
114+
class FoobarCircular
115+
{
116+
public function __construct($foo)
117+
{
118+
$this->foo = $foo;
119+
}
120+
}
121+
122+
class FooCircular
123+
{
124+
public function __construct($bar)
125+
{
126+
$this->bar = $bar;
127+
}
128+
}
129+
130+
class BarCircular
131+
{
132+
public function addFoobar($foobar)
133+
{
134+
$this->foobar = $foobar;
135+
}
136+
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
+29-3Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ protected function getBarService()
8282
{
8383
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
8484

85+
if (isset($this->services['bar'])) {
86+
return $this->services['bar'];
87+
}
88+
8589
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
8690

8791
$a->configure($instance);
@@ -182,7 +186,13 @@ protected function getDeprecatedServiceService()
182186
*/
183187
protected function getFactoryServiceService()
184188
{
185-
return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'}->getInstance();
189+
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
190+
191+
if (isset($this->services['factory_service'])) {
192+
return $this->services['factory_service'];
193+
}
194+
195+
return $this->services['factory_service'] = $a->getInstance();
186196
}
187197

188198
/**
@@ -192,7 +202,13 @@ protected function getFactoryServiceService()
192202
*/
193203
protected function getFactoryServiceSimpleService()
194204
{
195-
return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'}->getInstance();
205+
$a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'};
206+
207+
if (isset($this->services['factory_service_simple'])) {
208+
return $this->services['factory_service_simple'];
209+
}
210+
211+
return $this->services['factory_service_simple'] = $a->getInstance();
196212
}
197213

198214
/**
@@ -204,6 +220,10 @@ protected function getFooService()
204220
{
205221
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
206222

223+
if (isset($this->services['foo'])) {
224+
return $this->services['foo'];
225+
}
226+
207227
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
208228

209229
$instance->foo = 'bar';
@@ -321,7 +341,13 @@ protected function getMethodCall1Service()
321341
*/
322342
protected function getNewFactoryServiceService()
323343
{
324-
$this->services['new_factory_service'] = $instance = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'}->getInstance();
344+
$a = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'};
345+
346+
if (isset($this->services['new_factory_service'])) {
347+
return $this->services['new_factory_service'];
348+
}
349+
350+
$this->services['new_factory_service'] = $instance = $a->getInstance();
325351

326352
$instance->foo = 'bar';
327353

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt
+31-5Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
2222

2323
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
2424

25+
if (isset($this->services['bar'])) {
26+
return $this->services['bar'];
27+
}
28+
2529
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
2630

2731
$a->configure($instance);
@@ -48,12 +52,18 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
4852
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
4953
// Returns the public 'configured_service' shared service.
5054

51-
$a = new \ConfClass();
52-
$a->setFoo(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'});
55+
$a = ${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'};
56+
57+
if (isset($this->services['configured_service'])) {
58+
return $this->services['configured_service'];
59+
}
60+
61+
$b = new \ConfClass();
62+
$b->setFoo($a);
5363

5464
$this->services['configured_service'] = $instance = new \stdClass();
5565

56-
$a->configureStdClass($instance);
66+
$b->configureStdClass($instance);
5767

5868
return $instance;
5969

@@ -106,7 +116,13 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
106116
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
107117
// Returns the public 'factory_service' shared service.
108118

109-
return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'}->getInstance();
119+
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
120+
121+
if (isset($this->services['factory_service'])) {
122+
return $this->services['factory_service'];
123+
}
124+
125+
return $this->services['factory_service'] = $a->getInstance();
110126

111127
[Container%s/getFactoryServiceSimpleService.php] => <?php
112128

@@ -115,7 +131,13 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
115131
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
116132
// Returns the public 'factory_service_simple' shared service.
117133

118-
return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->load(__DIR__.'/getFactorySimpleService.php')) && false ?: '_'}->getInstance();
134+
$a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->load(__DIR__.'/getFactorySimpleService.php')) && false ?: '_'};
135+
136+
if (isset($this->services['factory_service_simple'])) {
137+
return $this->services['factory_service_simple'];
138+
}
139+
140+
return $this->services['factory_service_simple'] = $a->getInstance();
119141

120142
[Container%s/getFactorySimpleService.php] => <?php
121143

@@ -137,6 +159,10 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
137159

138160
$a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->load(__DIR__.'/getFoo_BazService.php')) && false ?: '_'};
139161

162+
if (isset($this->services['foo'])) {
163+
return $this->services['foo'];
164+
}
165+
140166
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);
141167

142168
$instance->foo = 'bar';

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.