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 e2add8b

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #25130 [DI] Fix handling of inlined definitions by ContainerBuilder (nicolas-grekas)
This PR was merged into the 2.7 branch. Discussion ---------- [DI] Fix handling of inlined definitions by ContainerBuilder | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Team work with @dunglas, debugging behat tests on Symfony 4: now that everything is private, inlining happens quite often on Symfony 4. This made us discover an old bug: inlining makes it possible to share the same definition instance to define a locally shared service (local to one service). This is handled properly in PhpDumper, but ContainerDumper is broken. Here is the fix. Commits ------- c9c18ac [DI] Fix handling of inlined definitions by ContainerBuilder
2 parents 9d68751 + c9c18ac commit e2add8b
Copy full SHA for e2add8b

File tree

3 files changed

+55
-19
lines changed
Filter options

3 files changed

+55
-19
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+32-18Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
441441
$this->loading[$id] = true;
442442

443443
try {
444-
$service = $this->createService($definition, $id);
444+
$service = $this->createService($definition, new \SplObjectStorage(), $id);
445445
} catch (\Exception $e) {
446446
unset($this->loading[$id]);
447447

@@ -827,8 +827,12 @@ public function findDefinition($id)
827827
*
828828
* @internal this method is public because of PHP 5.3 limitations, do not use it explicitly in your code
829829
*/
830-
public function createService(Definition $definition, $id, $tryProxy = true)
830+
public function createService(Definition $definition, \SplObjectStorage $inlinedDefinitions, $id = null, $tryProxy = true)
831831
{
832+
if (null === $id && isset($inlinedDefinitions[$definition])) {
833+
return $inlinedDefinitions[$definition];
834+
}
835+
832836
if ($definition instanceof DefinitionDecorator) {
833837
throw new RuntimeException(sprintf('Constructing service "%s" from a parent definition is not supported at build time.', $id));
834838
}
@@ -845,11 +849,11 @@ public function createService(Definition $definition, $id, $tryProxy = true)
845849
->instantiateProxy(
846850
$container,
847851
$definition,
848-
$id, function () use ($definition, $id, $container) {
849-
return $container->createService($definition, $id, false);
852+
$id, function () use ($definition, $inlinedDefinitions, $id, $container) {
853+
return $container->createService($definition, $inlinedDefinitions, $id, false);
850854
}
851855
);
852-
$this->shareService($definition, $proxy, $id);
856+
$this->shareService($definition, $proxy, $id, $inlinedDefinitions);
853857

854858
return $proxy;
855859
}
@@ -860,11 +864,11 @@ public function createService(Definition $definition, $id, $tryProxy = true)
860864
require_once $parameterBag->resolveValue($definition->getFile());
861865
}
862866

863-
$arguments = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())));
867+
$arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlinedDefinitions);
864868

865869
if (null !== $factory = $definition->getFactory()) {
866870
if (is_array($factory)) {
867-
$factory = array($this->resolveServices($parameterBag->resolveValue($factory[0])), $factory[1]);
871+
$factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlinedDefinitions), $factory[1]);
868872
} elseif (!is_string($factory)) {
869873
throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id));
870874
}
@@ -888,16 +892,16 @@ public function createService(Definition $definition, $id, $tryProxy = true)
888892

889893
if ($tryProxy || !$definition->isLazy()) {
890894
// share only if proxying failed, or if not a proxy
891-
$this->shareService($definition, $service, $id);
895+
$this->shareService($definition, $service, $id, $inlinedDefinitions);
892896
}
893897

894-
$properties = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())));
898+
$properties = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())), $inlinedDefinitions);
895899
foreach ($properties as $name => $value) {
896900
$service->$name = $value;
897901
}
898902

899903
foreach ($definition->getMethodCalls() as $call) {
900-
$this->callMethod($service, $call);
904+
$this->callMethod($service, $call, $inlinedDefinitions);
901905
}
902906

903907
if ($callable = $definition->getConfigurator()) {
@@ -907,7 +911,7 @@ public function createService(Definition $definition, $id, $tryProxy = true)
907911
if ($callable[0] instanceof Reference) {
908912
$callable[0] = $this->get((string) $callable[0], $callable[0]->getInvalidBehavior());
909913
} elseif ($callable[0] instanceof Definition) {
910-
$callable[0] = $this->createService($callable[0], null);
914+
$callable[0] = $this->createService($callable[0], $inlinedDefinitions);
911915
}
912916
}
913917

@@ -930,15 +934,20 @@ public function createService(Definition $definition, $id, $tryProxy = true)
930934
* the real service instances and all expressions evaluated
931935
*/
932936
public function resolveServices($value)
937+
{
938+
return $this->doResolveServices($value, new \SplObjectStorage());
939+
}
940+
941+
private function doResolveServices($value, \SplObjectStorage $inlinedDefinitions)
933942
{
934943
if (is_array($value)) {
935944
foreach ($value as $k => $v) {
936-
$value[$k] = $this->resolveServices($v);
945+
$value[$k] = $this->doResolveServices($v, $inlinedDefinitions);
937946
}
938947
} elseif ($value instanceof Reference) {
939948
$value = $this->get((string) $value, $value->getInvalidBehavior());
940949
} elseif ($value instanceof Definition) {
941-
$value = $this->createService($value, null);
950+
$value = $this->createService($value, $inlinedDefinitions);
942951
} elseif ($value instanceof Expression) {
943952
$value = $this->getExpressionLanguage()->evaluate($value, array('container' => $this));
944953
}
@@ -1065,14 +1074,14 @@ private function synchronize($id)
10651074
foreach ($definition->getMethodCalls() as $call) {
10661075
foreach ($call[1] as $argument) {
10671076
if ($argument instanceof Reference && $id == (string) $argument) {
1068-
$this->callMethod($this->get($definitionId), $call);
1077+
$this->callMethod($this->get($definitionId), $call, new \SplObjectStorage());
10691078
}
10701079
}
10711080
}
10721081
}
10731082
}
10741083

1075-
private function callMethod($service, $call)
1084+
private function callMethod($service, $call, \SplObjectStorage $inlinedDefinitions)
10761085
{
10771086
$services = self::getServiceConditionals($call[1]);
10781087

@@ -1082,7 +1091,7 @@ private function callMethod($service, $call)
10821091
}
10831092
}
10841093

1085-
call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1]))));
1094+
call_user_func_array(array($service, $call[0]), $this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlinedDefinitions));
10861095
}
10871096

10881097
/**
@@ -1094,9 +1103,14 @@ private function callMethod($service, $call)
10941103
*
10951104
* @throws InactiveScopeException
10961105
*/
1097-
private function shareService(Definition $definition, $service, $id)
1106+
private function shareService(Definition $definition, $service, $id, \SplObjectStorage $inlinedDefinitions)
10981107
{
1099-
if (null !== $id && self::SCOPE_PROTOTYPE !== $scope = $definition->getScope()) {
1108+
if (self::SCOPE_PROTOTYPE === $scope = $definition->getScope()) {
1109+
return;
1110+
}
1111+
if (null === $id) {
1112+
$inlinedDefinitions[$definition] = $service;
1113+
} else {
11001114
if (self::SCOPE_CONTAINER !== $scope && !isset($this->scopedServices[$scope])) {
11011115
throw new InactiveScopeException($id, $scope);
11021116
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,28 @@ public function testLazyLoadedService()
827827
$this->assertTrue($classInList);
828828
}
829829

830+
public function testInlinedDefinitions()
831+
{
832+
$container = new ContainerBuilder();
833+
834+
$definition = new Definition('BarClass');
835+
836+
$container->register('bar_user', 'BarUserClass')
837+
->addArgument($definition)
838+
->setProperty('foo', $definition);
839+
840+
$container->register('bar', 'BarClass')
841+
->setProperty('foo', $definition)
842+
->addMethodCall('setBaz', array($definition));
843+
844+
$barUser = $container->get('bar_user');
845+
$bar = $container->get('bar');
846+
847+
$this->assertSame($barUser->foo, $barUser->bar);
848+
$this->assertSame($bar->foo, $bar->getBaz());
849+
$this->assertNotSame($bar->foo, $barUser->foo);
850+
}
851+
830852
public function testInitializePropertiesBeforeMethodCalls()
831853
{
832854
$container = new ContainerBuilder();

‎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
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ function sc_configure($instance)
88
$instance->configure();
99
}
1010

11-
class BarClass
11+
class BarClass extends BazClass
1212
{
1313
protected $baz;
1414
public $foo = 'foo';

0 commit comments

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