8000 [DI] Fix dumping some complex service graphs by nicolas-grekas · Pull Request #28366 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI] Fix dumping some complex service graphs #28366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
6D40
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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']

Original file line number Diff line number Diff line change
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