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
This commit is contained in:
Nicolas Grekas 2019-07-29 19:31:03 +02:00
commit b2dadc110d
7 changed files with 282 additions and 43 deletions

View File

@ -136,7 +136,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
$this->lazy = false;
$byConstructor = $this->byConstructor;
$this->byConstructor = true;
$this->byConstructor = $isRoot || $byConstructor;
$this->processValue($value->getFactory());
$this->processValue($value->getArguments());

View File

@ -187,6 +187,7 @@ class PhpDumper extends Dumper
}
$this->container->getCompiler()->getServiceReferenceGraph()->clear();
$checkedNodes = [];
$this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
$this->docStar = $options['debug'] ? '*' : '';
@ -339,10 +340,10 @@ EOF;
return $this->proxyDumper;
}
private function analyzeCircularReferences($sourceId, array $edges, &$checkedNodes, &$currentPath = [])
private function analyzeCircularReferences($sourceId, array $edges, &$checkedNodes, &$currentPath = [], $byConstructor = true)
{
$checkedNodes[$sourceId] = true;
$currentPath[$sourceId] = $sourceId;
$currentPath[$sourceId] = $byConstructor;
foreach ($edges as $edge) {
$node = $edge->getDestNode();
@ -351,44 +352,52 @@ EOF;
if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
// no-op
} elseif (isset($currentPath[$id])) {
$currentId = $id;
foreach (array_reverse($currentPath) as $parentId) {
$this->circularReferences[$parentId][$currentId] = $currentId;
if ($parentId === $id) {
break;
}
$currentId = $parentId;
}
$this->addCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
} elseif (!isset($checkedNodes[$id])) {
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath);
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath, $edge->isReferencedByConstructor());
} elseif (isset($this->circularReferences[$id])) {
$this->connectCircularReferences($id, $currentPath);
$this->connectCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
}
}
unset($currentPath[$sourceId]);
}
private function connectCircularReferences($sourceId, &$currentPath, &$subPath = [])
private function connectCircularReferences($sourceId, &$currentPath, $byConstructor, &$subPath = [])
{
$subPath[$sourceId] = $sourceId;
$currentPath[$sourceId] = $sourceId;
$currentPath[$sourceId] = $subPath[$sourceId] = $byConstructor;
foreach ($this->circularReferences[$sourceId] as $id) {
foreach ($this->circularReferences[$sourceId] as $id => $byConstructor) {
if (isset($currentPath[$id])) {
$currentId = $id;
foreach (array_reverse($currentPath) as $parentId) {
$this->circularReferences[$parentId][$currentId] = $currentId;
if ($parentId === $id) {
break;
}
$currentId = $parentId;
}
$this->addCircularReferences($id, $currentPath, $byConstructor);
} elseif (!isset($subPath[$id]) && isset($this->circularReferences[$id])) {
$this->connectCircularReferences($id, $currentPath, $subPath);
$this->connectCircularReferences($id, $currentPath, $byConstructor, $subPath);
}
}
unset($currentPath[$sourceId]);
unset($subPath[$sourceId]);
unset($currentPath[$sourceId], $subPath[$sourceId]);
}
private function addCircularReferences($id, $currentPath, $byConstructor)
{
$currentPath[$id] = $byConstructor;
$circularRefs = [];
foreach (array_reverse($currentPath) as $parentId => $v) {
$byConstructor = $byConstructor && $v;
$circularRefs[] = $parentId;
if ($parentId === $id) {
break;
}
}
$currentId = $id;
foreach ($circularRefs as $parentId) {
if (empty($this->circularReferences[$parentId][$currentId])) {
$this->circularReferences[$parentId][$currentId] = $byConstructor;
}
$currentId = $parentId;
}
}
private function collectLineage($class, array &$lineage)
@ -679,7 +688,6 @@ EOF;
$autowired = $definition->isAutowired() ? ' autowired' : '';
if ($definition->isLazy()) {
unset($this->circularReferences[$id]);
$lazyInitialization = '$lazyLoad = true';
} else {
$lazyInitialization = '';
@ -755,12 +763,12 @@ EOF;
private function addInlineReference(string $id, Definition $definition, string $targetId, bool $forConstructor): string
{
list($callCount, $behavior) = $this->serviceCalls[$targetId];
while ($this->container->hasAlias($targetId)) {
$targetId = (string) $this->container->getAlias($targetId);
}
list($callCount, $behavior) = $this->serviceCalls[$targetId];
if ($id === $targetId) {
return $this->addInlineService($id, $definition, $definition);
}
@ -769,9 +777,13 @@ EOF;
return '';
}
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
$hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]);
if ($hasSelfRef && !$forConstructor && !$forConstructor = !$this->circularReferences[$id][$targetId]) {
$code = $this->addInlineService($id, $definition, $definition);
} else {
$code = '';
}
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
return $code;
@ -804,15 +816,23 @@ EOTXT
private function addInlineService(string $id, Definition $definition, Definition $inlineDef = null, bool $forConstructor = true): string
{
$isSimpleInstance = $isRootInstance = null === $inlineDef;
$code = '';
if ($isSimpleInstance = $isRootInstance = null === $inlineDef) {
foreach ($this->serviceCalls as $targetId => list($callCount, $behavior, $byConstructor)) {
if ($byConstructor && isset($this->circularReferences[$id][$targetId]) && !$this->circularReferences[$id][$targetId]) {
$code .= $this->addInlineReference($id, $definition, $targetId, $forConstructor);
}
}
}
if (isset($this->definitionVariables[$inlineDef = $inlineDef ?: $definition])) {
return '';
return $code;
}
$arguments = [$inlineDef->getArguments(), $inlineDef->getFactory()];
$code = $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
$code .= $this->addInlineVariables($id, $definition, $arguments, $forConstructor);
if ($arguments = array_filter([$inlineDef->getProperties(), $inlineDef->getMethodCalls(), $inlineDef->getConfigurator()])) {
$isSimpleInstance = false;
@ -1473,7 +1493,7 @@ EOF;
return implode(' && ', $conditions);
}
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = []): \SplObjectStorage
private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null, array &$calls = [], bool $byConstructor = null): \SplObjectStorage
{
if (null === $definitions) {
$definitions = new \SplObjectStorage();
@ -1481,12 +1501,16 @@ EOF;
foreach ($arguments as $argument) {
if (\is_array($argument)) {
$this->getDefinitionsFromArguments($argument, $definitions, $calls);
$this->getDefinitionsFromArguments($argument, $definitions, $calls, $byConstructor);
} elseif ($argument instanceof Reference) {
$id = (string) $argument;
while ($this->container->hasAlias($id)) {
$id = (string) $this->container->getAlias($id);
}
if (!isset($calls[$id])) {
$calls[$id] = [0, $argument->getInvalidBehavior()];
$calls[$id] = [0, $argument->getInvalidBehavior(), $byConstructor];
} else {
$calls[$id][1] = min($calls[$id][1], $argument->getInvalidBehavior());
}
@ -1498,8 +1522,10 @@ EOF;
$definitions[$argument] = 1 + $definitions[$argument];
} else {
$definitions[$argument] = 1;
$arguments = [$argument->getArguments(), $argument->getFactory(), $argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
$this->getDefinitionsFromArguments($arguments, $definitions, $calls);
$arguments = [$argument->getArguments(), $argument->getFactory()];
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null === $byConstructor || $byConstructor);
$arguments = [$argument->getProperties(), $argument->getMethodCalls(), $argument->getConfigurator()];
$this->getDefinitionsFromArguments($arguments, $definitions, $calls, null !== $byConstructor && $byConstructor);
}
}
@ -1620,6 +1646,11 @@ EOF;
return '$'.$value;
} elseif ($value instanceof Reference) {
$id = (string) $value;
while ($this->container->hasAlias($id)) {
$id = (string) $this->container->getAlias($id);
}
if (null !== $this->referenceVariables && isset($this->referenceVariables[$id])) {
return $this->dumpValue($this->referenceVariables[$id], $interpolate);
}

View File

@ -1430,6 +1430,13 @@ class ContainerBuilderTest extends TestCase
$this->assertEquals((object) ['bar6' => (object) []], $foo6);
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
$manager3 = $container->get('manager3');
$listener3 = $container->get('listener3');
$this->assertSame($manager3, $listener3->manager, 'Both should identically be the manager3 service');
$listener4 = $container->get('listener4');
$this->assertInstanceOf('stdClass', $listener4);
}
public function provideAlmostCircular()

View File

@ -996,6 +996,13 @@ class PhpDumperTest extends TestCase
$this->assertEquals((object) ['bar6' => (object) []], $foo6);
$this->assertInstanceOf(\stdClass::class, $container->get('root'));
$manager3 = $container->get('manager3');
$listener3 = $container->get('listener3');
$this->assertSame($manager3, $listener3->manager);
$listener4 = $container->get('listener4');
$this->assertInstanceOf('stdClass', $listener4);
}
public function provideAlmostCircular()

View File

@ -2,7 +2,6 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooForCircularWithAddCalls;
@ -102,6 +101,35 @@ $container->register('dispatcher2', 'stdClass')->setPublic($public)
$container->register('subscriber2', 'stdClass')->setPublic(false)
->addArgument(new Reference('manager2'));
// doctrine-like event system with listener
$container->register('manager3', 'stdClass')
->setLazy(true)
->setPublic(true)
->addArgument(new Reference('connection3'));
$container->register('connection3', 'stdClass')
->setPublic($public)
->setProperty('listener', [new Reference('listener3')]);
$container->register('listener3', 'stdClass')
->setPublic(true)
->setProperty('manager', new Reference('manager3'));
// doctrine-like event system with small differences
$container->register('manager4', 'stdClass')
->setLazy(true)
->addArgument(new Reference('connection4'));
$container->register('connection4', 'stdClass')
->setPublic($public)
->setProperty('listener', [new Reference('listener4')]);
$container->register('listener4', 'stdClass')
->setPublic(true)
->addArgument(new Reference('manager4'));
// private service involved in a loop
$container->register('foo6', 'stdClass')

View File

@ -33,9 +33,12 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
'foo5' => 'getFoo5Service',
'foo6' => 'getFoo6Service',
'foobar4' => 'getFoobar4Service',
'listener3' => 'getListener3Service',
'listener4' => 'getListener4Service',
'logger' => 'getLoggerService',
'manager' => 'getManagerService',
'manager2' => 'getManager2Service',
'manager3' => 'getManager3Service',
'root' => 'getRootService',
'subscriber' => 'getSubscriberService',
];
@ -63,6 +66,8 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
'bar6' => true,
'config' => true,
'config2' => true,
'connection3' => true,
'connection4' => true,
'dispatcher' => true,
'dispatcher2' => true,
'foo4' => true,
@ -75,6 +80,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
'level5' => true,
'level6' => true,
'logger2' => true,
'manager4' => true,
'multiuse1' => true,
'subscriber2' => true,
];
@ -249,6 +255,36 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
return $instance;
}
/**
* Gets the public 'listener3' shared service.
*
* @return \stdClass
*/
protected function getListener3Service()
{
$this->services['listener3'] = $instance = new \stdClass();
$instance->manager = ($this->services['manager3'] ?? $this->getManager3Service());
return $instance;
}
/**
* Gets the public 'listener4' shared service.
*
* @return \stdClass
*/
protected function getListener4Service()
{
$a = ($this->privates['manager4'] ?? $this->getManager4Service());
if (isset($this->services['listener4'])) {
return $this->services['listener4'];
}
return $this->services['listener4'] = new \stdClass($a);
}
/**
* Gets the public 'logger' shared service.
*
@ -301,6 +337,24 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
return $this->services['manager2'] = new \stdClass($a);
}
/**
* Gets the public 'manager3' shared service.
*
* @return \stdClass
*/
protected function getManager3Service($lazyLoad = true)
{
$a = ($this->services['listener3'] ?? $this->getListener3Service());
if (isset($this->services['manager3'])) {
return $this->services['manager3'];
}
$b = new \stdClass();
$b->listener = [0 => $a];
return $this->services['manager3'] = new \stdClass($b);
}
/**
* Gets the public 'root' shared service.
*
@ -364,4 +418,20 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
return $instance;
}
/**
* Gets the private 'manager4' shared service.
*
* @return \stdClass
*/
protected function getManager4Service($lazyLoad = true)
{
$a = new \stdClass();
$this->privates['manager4'] = $instance = new \stdClass($a);
$a->listener = [0 => ($this->services['listener4'] ?? $this->getListener4Service())];
return $instance;
}
}

View File

@ -29,6 +29,8 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
'baz6' => 'getBaz6Service',
'connection' => 'getConnectionService',
'connection2' => 'getConnection2Service',
'connection3' => 'getConnection3Service',
'connection4' => 'getConnection4Service',
'dispatcher' => 'getDispatcherService',
'dispatcher2' => 'getDispatcher2Service',
'foo' => 'getFooService',
@ -40,9 +42,12 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
'foobar2' => 'getFoobar2Service',
'foobar3' => 'getFoobar3Service',
'foobar4' => 'getFoobar4Service',
'listener3' => 'getListener3Service',
'listener4' => 'getListener4Service',
'logger' => 'getLoggerService',
'manager' => 'getManagerService',
'manager2' => 'getManager2Service',
'manager3' => 'getManager3Service',
'root' => 'getRootService',
'subscriber' => 'getSubscriberService',
];
@ -75,6 +80,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
'level5' => true,
'level6' => true,
'logger2' => true,
'manager4' => true,
'multiuse1' => true,
'subscriber2' => true,
];
@ -189,6 +195,34 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
return $instance;
}
/**
* Gets the public 'connection3' shared service.
*
* @return \stdClass
*/
protected function getConnection3Service()
{
$this->services['connection3'] = $instance = new \stdClass();
$instance->listener = [0 => ($this->services['listener3'] ?? $this->getListener3Service())];
return $instance;
}
/**
* Gets the public 'connection4' shared service.
*
* @return \stdClass
*/
protected function getConnection4Service()
{
$this->services['connection4'] = $instance = new \stdClass();
$instance->listener = [0 => ($this->services['listener4'] ?? $this->getListener4Service())];
return $instance;
}
/**
* Gets the public 'dispatcher' shared service.
*
@ -349,6 +383,36 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
return $instance;
}
/**
* Gets the public 'listener3' shared service.
*
* @return \stdClass
*/
protected function getListener3Service()
{
$this->services['listener3'] = $instance = new \stdClass();
$instance->manager = ($this->services['manager3'] ?? $this->getManager3Service());
return $instance;
}
/**
* Gets the public 'listener4' shared service.
*
* @return \stdClass
*/
protected function getListener4Service()
{
$a = ($this->privates['manager4'] ?? $this->getManager4Service());
if (isset($this->services['listener4'])) {
return $this->services['listener4'];
}
return $this->services['listener4'] = new \stdClass($a);
}
/**
* Gets the public 'logger' shared service.
*
@ -401,6 +465,22 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
return $this->services['manager2'] = new \stdClass($a);
}
/**
* Gets the public 'manager3' shared service.
*
* @return \stdClass
*/
protected function getManager3Service($lazyLoad = true)
{
$a = ($this->services['connection3'] ?? $this->getConnection3Service());
if (isset($this->services['manager3'])) {
return $this->services['manager3'];
}
return $this->services['manager3'] = new \stdClass($a);
}
/**
* Gets the public 'root' shared service.
*
@ -464,4 +544,20 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
return $instance;
}
/**
* Gets the private 'manager4' shared service.
*
* @return \stdClass
*/
protected function getManager4Service($lazyLoad = true)
{
$a = ($this->services['connection4'] ?? $this->getConnection4Service());
if (isset($this->privates['manager4'])) {
return $this->privates['manager4'];
}
return $this->privates['manager4'] = new \stdClass($a);
}
}