Skip to content

Commit

Permalink
bug #28366 [DI] Fix dumping some complex service graphs (nicolas-grekas)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nicolas-grekas committed Sep 5, 2018
2 parents edbd869 + 769fd4b commit 8851c14
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 10 deletions.
17 changes: 12 additions & 5 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -516,7 +516,7 @@ private function addServiceInlinedDefinitions($id, Definition $definition, \SplO

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

if (!$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) {
if (!$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true, $inlinedDefinitions)) {
$code .= $this->addServiceProperties($def, $name);
$code .= $this->addServiceMethodCalls($def, $name);
$code .= $this->addServiceConfigurator($def, $name);
Expand Down Expand Up @@ -669,7 +669,7 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition,
{
$code = '';
foreach ($inlinedDefinitions as $def) {
if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) {
if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true, $inlinedDefinitions)) {
continue;
}

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

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

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

Expand Down Expand Up @@ -1635,15 +1639,15 @@ private function getDefinitionsFromArguments(array $arguments, $isConstructorArg
*
* @return bool
*/
private function hasReference($id, array $arguments, $deep = false, array &$visited = array())
private function hasReference($id, array $arguments, $deep = false, \SplObjectStorage $inlinedDefinitions = null, array &$visited = array())
{
if (!isset($this->circularReferences[$id])) {
return false;
}

foreach ($arguments as $argument) {
if (\is_array($argument)) {
if ($this->hasReference($id, $argument, $deep, $visited)) {
if ($this->hasReference($id, $argument, $deep, $inlinedDefinitions, $visited)) {
return true;
}

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

$service = $this->container->getDefinition($argumentId);
} elseif ($argument instanceof Definition) {
if (isset($inlinedDefinitions[$argument])) {
return true;
}
$service = $argument;
} else {
continue;
Expand All @@ -1673,7 +1680,7 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi
continue;
}

if ($this->hasReference($id, array($service->getArguments(), $service->getFactory(), $service->getProperties(), $service->getMethodCalls(), $service->getConfigurator()), $deep, $visited)) {
if ($this->hasReference($id, array($service->getArguments(), $service->getFactory(), $service->getProperties(), $service->getMethodCalls(), $service->getConfigurator()), $deep, $inlinedDefinitions, $visited)) {
return true;
}
}
Expand Down
Expand Up @@ -402,7 +402,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file, $defaults)
{
$definitions = array();
$count = 0;
$suffix = ContainerBuilder::hash($file);
$suffix = '~'.ContainerBuilder::hash($file);

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

Expand Down
Expand Up @@ -142,7 +142,7 @@ public function load($resource, $type = null)

// services
$this->anonymousServicesCount = 0;
$this->anonymousServicesSuffix = ContainerBuilder::hash($path);
$this->anonymousServicesSuffix = '~'.ContainerBuilder::hash($path);
$this->setCurrentDir(\dirname($path));
try {
$this->parseDefinitions($content, $path);
Expand Down
Expand Up @@ -838,6 +838,21 @@ public function provideAlmostCircular()
yield array('private');
}

public function testDeepServiceGraph()
{
$container = new ContainerBuilder();

$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('services_deep_graph.yml');

$container->compile();

$dumper = new PhpDumper($container);
$dumper->dump();

$this->assertStringEqualsFile(self::$fixturesPath.'/php/services_deep_graph.php', $dumper->dump());
}

public function testHotPathOptimizations()
{
$container = include self::$fixturesPath.'/containers/container_inline_requires.php';
Expand Down
@@ -0,0 +1,96 @@
<?php

use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;

/**
* This class has been auto-generated
* by the Symfony Dependency Injection Component.
*
* @final since Symfony 3.3
*/
class ProjectServiceContainer extends Container
{
private $parameters;
private $targetDirs = array();

public function __construct()
{
$this->services = array();
$this->methodMap = array(
'bar' => 'getBarService',
'foo' => 'getFooService',
);

$this->aliases = array();
}

public function getRemovedIds()
{
return array(
'Psr\\Container\\ContainerInterface' => true,
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
);
}

public function compile()
{
throw new LogicException('You cannot compile a dumped container that was already compiled.');
}

public function isCompiled()
{
return true;
}

public function isFrozen()
{
@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);

return true;
}

/**
* Gets the public 'bar' shared service.
*
* @return \c5
*/
protected function getBarService()
{
$this->services['bar'] = $instance = new \c5();

$instance->p5 = new \c6(${($_ = isset($this->services['foo']) ? $this->services['foo'] : $this->getFooService()) && false ?: '_'});

return $instance;
}

/**
* Gets the public 'foo' shared service.
*
* @return \c1
*/
protected function getFooService()
{
$a = ${($_ = isset($this->services['bar']) ? $this->services['bar'] : $this->getBarService()) && false ?: '_'};

if (isset($this->services['foo'])) {
return $this->services['foo'];
}

$b = new \c2();

$this->services['foo'] = $instance = new \c1($a, $b);

$c = new \c3();

$c->p3 = new \c4();
$b->p2 = $c;

return $instance;
}
}
@@ -0,0 +1,24 @@

services:
foo:
class: c1
public: true
arguments:
- '@bar'
- !service
class: c2
properties:
p2: !service
class: c3
properties:
p3: !service
class: c4

bar:
class: c5
public: true
properties:
p5: !service
class: c6
arguments: ['@foo']

Expand Up @@ -586,7 +586,7 @@ public function testAnonymousServices()
$this->assertCount(1, $args);
$this->assertInstanceOf(Reference::class, $args[0]);
$this->assertTrue($container->has((string) $args[0]));
$this->assertRegExp('/^\d+_Bar[._A-Za-z0-9]{7}$/', (string) $args[0]);
$this->assertRegExp('/^\d+_Bar~[._A-Za-z0-9]{7}$/', (string) $args[0]);

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

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

0 comments on commit 8851c14

Please sign in to comment.