bug #29247 [DI] fix taking lazy services into account when dumping the container (nicolas-grekas)
This PR was merged into the 3.4 branch.
Discussion
----------
[DI] fix taking lazy services into account when dumping the container
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #29246
| License | MIT
| Doc PR | -
This PR fixes issues found while working on #29246.
It *does* fix the infinite loop, ~but replaces it by an exception (reopening #29078)~:
> ~It's a requirement to specify a Metadata Driver and pass it to Doctrine\ORM\Configuration::setMetadataDriverImpl()~
The full fix is not immediately accessible as it needs some core changes to the dumping logic. Requiring `symfony/proxy-manager-bridge` works around the issue properly.
See #29251 for 4.2
Commits
-------
67d7623e72
[DI] fix taking lazy services into account when dumping the container
This commit is contained in:
commit
73d98049fa
@ -37,6 +37,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
|
||||
private $hasProxyDumper;
|
||||
private $lazy;
|
||||
private $expressionLanguage;
|
||||
private $byConstructor;
|
||||
|
||||
/**
|
||||
* @param bool $onlyConstructorArguments Sets this Service Reference pass to ignore method calls
|
||||
@ -64,6 +65,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
|
||||
$this->graph = $container->getCompiler()->getServiceReferenceGraph();
|
||||
$this->graph->clear();
|
||||
$this->lazy = false;
|
||||
$this->byConstructor = false;
|
||||
|
||||
foreach ($container->getAliases() as $id => $alias) {
|
||||
$targetId = $this->getDefinitionId((string) $alias);
|
||||
@ -100,7 +102,8 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
|
||||
$targetDefinition,
|
||||
$value,
|
||||
$this->lazy || ($this->hasProxyDumper && $targetDefinition && $targetDefinition->isLazy()),
|
||||
ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior()
|
||||
ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior(),
|
||||
$this->byConstructor
|
||||
);
|
||||
|
||||
return $value;
|
||||
@ -118,8 +121,11 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe
|
||||
}
|
||||
$this->lazy = false;
|
||||
|
||||
$byConstructor = $this->byConstructor;
|
||||
$this->byConstructor = true;
|
||||
$this->processValue($value->getFactory());
|
||||
$this->processValue($value->getArguments());
|
||||
$this->byConstructor = $byConstructor;
|
||||
|
||||
if (!$this->onlyConstructorArguments) {
|
||||
$this->processValue($value->getProperties());
|
||||
|
@ -91,11 +91,13 @@ class ServiceReferenceGraph
|
||||
* @param string $reference
|
||||
* @param bool $lazy
|
||||
* @param bool $weak
|
||||
* @param bool $byConstructor
|
||||
*/
|
||||
public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false, bool $weak = false*/)
|
||||
public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false, bool $weak = false, bool $byConstructor = false*/)
|
||||
{
|
||||
$lazy = \func_num_args() >= 6 ? func_get_arg(5) : false;
|
||||
$weak = \func_num_args() >= 7 ? func_get_arg(6) : false;
|
||||
$byConstructor = \func_num_args() >= 8 ? func_get_arg(7) : false;
|
||||
|
||||
if (null === $sourceId || null === $destId) {
|
||||
return;
|
||||
@ -103,7 +105,7 @@ class ServiceReferenceGraph
|
||||
|
||||
$sourceNode = $this->createNode($sourceId, $sourceValue);
|
||||
$destNode = $this->createNode($destId, $destValue);
|
||||
$edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy, $weak);
|
||||
$edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy, $weak, $byConstructor);
|
||||
|
||||
$sourceNode->addOutEdge($edge);
|
||||
$destNode->addInEdge($edge);
|
||||
|
@ -25,6 +25,7 @@ class ServiceReferenceGraphEdge
|
||||
private $value;
|
||||
private $lazy;
|
||||
private $weak;
|
||||
private $byConstructor;
|
||||
|
||||
/**
|
||||
* @param ServiceReferenceGraphNode $sourceNode
|
||||
@ -32,14 +33,16 @@ class ServiceReferenceGraphEdge
|
||||
* @param mixed $value
|
||||
* @param bool $lazy
|
||||
* @param bool $weak
|
||||
* @param bool $byConstructor
|
||||
*/
|
||||
public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false, $weak = false)
|
||||
public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false, $weak = false, $byConstructor = false)
|
||||
{
|
||||
$this->sourceNode = $sourceNode;
|
||||
$this->destNode = $destNode;
|
||||
$this->value = $value;
|
||||
$this->lazy = $lazy;
|
||||
$this->weak = $weak;
|
||||
$this->byConstructor = $byConstructor;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -91,4 +94,14 @@ class ServiceReferenceGraphEdge
|
||||
{
|
||||
return $this->weak;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if the edge links with a constructor argument.
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
public function isReferencedByConstructor()
|
||||
{
|
||||
return $this->byConstructor;
|
||||
}
|
||||
}
|
||||
|
@ -154,12 +154,16 @@ class PhpDumper extends Dumper
|
||||
}
|
||||
}
|
||||
|
||||
(new AnalyzeServiceReferencesPass(false))->process($this->container);
|
||||
(new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
|
||||
$this->circularReferences = array();
|
||||
$checkedNodes = array();
|
||||
foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
|
||||
$currentPath = array($id => $id);
|
||||
$this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
|
||||
foreach (array(true, false) as $byConstructor) {
|
||||
foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
|
||||
if (!$node->getValue() instanceof Definition) {
|
||||
continue;
|
||||
}
|
||||
$currentPath = array($id => true);
|
||||
$this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor);
|
||||
}
|
||||
}
|
||||
$this->container->getCompiler()->getServiceReferenceGraph()->clear();
|
||||
|
||||
@ -297,27 +301,31 @@ EOF;
|
||||
return $this->proxyDumper;
|
||||
}
|
||||
|
||||
private function analyzeCircularReferences(array $edges, &$checkedNodes, &$currentPath)
|
||||
private function analyzeCircularReferences(array $edges, &$currentPath, $sourceId, $byConstructor)
|
||||
{
|
||||
foreach ($edges as $edge) {
|
||||
if ($byConstructor && !$edge->isReferencedByConstructor()) {
|
||||
continue;
|
||||
}
|
||||
$node = $edge->getDestNode();
|
||||
$id = $node->getId();
|
||||
|
||||
if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
|
||||
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 (!isset($this->circularReferences[$parentId][$currentId])) {
|
||||
$this->circularReferences[$parentId][$currentId] = $byConstructor;
|
||||
}
|
||||
if ($parentId === $id) {
|
||||
break;
|
||||
}
|
||||
$currentId = $parentId;
|
||||
}
|
||||
} elseif (!isset($checkedNodes[$id])) {
|
||||
$checkedNodes[$id] = true;
|
||||
} else {
|
||||
$currentPath[$id] = $id;
|
||||
$this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
|
||||
$this->analyzeCircularReferences($node->getOutEdges(), $currentPath, $id, $byConstructor);
|
||||
unset($currentPath[$id]);
|
||||
}
|
||||
}
|
||||
@ -700,8 +708,14 @@ EOF;
|
||||
|
||||
private function addInlineReference($id, Definition $definition, $targetId, $forConstructor)
|
||||
{
|
||||
list($callCount, $behavior) = $this->serviceCalls[$targetId];
|
||||
|
||||
while ($this->container->hasAlias($targetId)) {
|
||||
$targetId = (string) $this->container->getAlias($targetId);
|
||||
}
|
||||
|
||||
if ($id === $targetId) {
|
||||
return $this->addInlineService($id, $definition, $definition, $forConstructor);
|
||||
return $this->addInlineService($id, $definition, $definition);
|
||||
}
|
||||
|
||||
if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
|
||||
@ -710,9 +724,7 @@ EOF;
|
||||
|
||||
$hasSelfRef = isset($this->circularReferences[$id][$targetId]);
|
||||
$forConstructor = $forConstructor && !isset($this->definitionVariables[$definition]);
|
||||
list($callCount, $behavior) = $this->serviceCalls[$targetId];
|
||||
|
||||
$code = $hasSelfRef && !$forConstructor ? $this->addInlineService($id, $definition, $definition, $forConstructor) : '';
|
||||
$code = $hasSelfRef && $this->circularReferences[$id][$targetId] && !$forConstructor ? $this->addInlineService($id, $definition, $definition) : '';
|
||||
|
||||
if (isset($this->referenceVariables[$targetId]) || (2 > $callCount && (!$hasSelfRef || !$forConstructor))) {
|
||||
return $code;
|
||||
|
@ -133,7 +133,13 @@ class ProjectServiceContainer extends Container
|
||||
*/
|
||||
protected function getHandler2Service()
|
||||
{
|
||||
return $this->services['App\Handler2'] = new \App\Handler2(${($_ = isset($this->services['App\Db']) ? $this->services['App\Db'] : $this->getDbService()) && false ?: '_'}, ${($_ = isset($this->services['App\Schema']) ? $this->services['App\Schema'] : $this->getSchemaService()) && false ?: '_'}, ${($_ = isset($this->services['App\Processor']) ? $this->services['App\Processor'] : $this->getProcessorService()) && false ?: '_'});
|
||||
$a = ${($_ = isset($this->services['App\Processor']) ? $this->services['App\Processor'] : $this->getProcessorService()) && false ?: '_'};
|
||||
|
||||
if (isset($this->services['App\Handler2'])) {
|
||||
return $this->services['App\Handler2'];
|
||||
}
|
||||
|
||||
return $this->services['App\Handler2'] = new \App\Handler2(${($_ = isset($this->services['App\Db']) ? $this->services['App\Db'] : $this->getDbService()) && false ?: '_'}, ${($_ = isset($this->services['App\Schema']) ? $this->services['App\Schema'] : $this->getSchemaService()) && false ?: '_'}, $a);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -174,11 +174,16 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
|
||||
*/
|
||||
protected function getConnectionService()
|
||||
{
|
||||
$a = new \stdClass();
|
||||
$a = ${($_ = isset($this->services['dispatcher']) ? $this->services['dispatcher'] : $this->getDispatcherService()) && false ?: '_'};
|
||||
|
||||
$this->services['connection'] = $instance = new \stdClass(${($_ = isset($this->services['dispatcher']) ? $this->services['dispatcher'] : $this->getDispatcherService()) && false ?: '_'}, $a);
|
||||
if (isset($this->services['connection'])) {
|
||||
return $this->services['connection'];
|
||||
}
|
||||
$b = new \stdClass();
|
||||
|
||||
$a->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
|
||||
$this->services['connection'] = $instance = new \stdClass($a, $b);
|
||||
|
||||
$b->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
|
||||
|
||||
return $instance;
|
||||
}
|
||||
@ -190,14 +195,19 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
|
||||
*/
|
||||
protected function getConnection2Service()
|
||||
{
|
||||
$a = new \stdClass();
|
||||
$a = ${($_ = isset($this->services['dispatcher2']) ? $this->services['dispatcher2'] : $this->getDispatcher2Service()) && false ?: '_'};
|
||||
|
||||
$this->services['connection2'] = $instance = new \stdClass(${($_ = isset($this->services['dispatcher2']) ? $this->services['dispatcher2'] : $this->getDispatcher2Service()) && false ?: '_'}, $a);
|
||||
if (isset($this->services['connection2'])) {
|
||||
return $this->services['connection2'];
|
||||
}
|
||||
$b = new \stdClass();
|
||||
|
||||
$b = new \stdClass($instance);
|
||||
$b->handler2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
|
||||
$this->services['connection2'] = $instance = new \stdClass($a, $b);
|
||||
|
||||
$a->logger2 = $b;
|
||||
$c = new \stdClass($instance);
|
||||
$c->handler2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
|
||||
|
||||
$b->logger2 = $c;
|
||||
|
||||
return $instance;
|
||||
}
|
||||
@ -431,7 +441,13 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
|
||||
*/
|
||||
protected function getSubscriberService()
|
||||
{
|
||||
return $this->services['subscriber'] = new \stdClass(${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'});
|
||||
$a = ${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'};
|
||||
|
||||
if (isset($this->services['subscriber'])) {
|
||||
return $this->services['subscriber'];
|
||||
}
|
||||
|
||||
return $this->services['subscriber'] = new \stdClass($a);
|
||||
}
|
||||
|
||||
/**
|
||||
|
Reference in New Issue
Block a user