Skip to content

Commit 7fff820

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.2'
* 3.2: (51 commits) [FrameworkBundle] [Workflow] Fix service marking store configuration Fix merge [Validator] add class name to the cache key [Serializer] Remove AbstractObjectNormalizer::isAttributeToNormalize Throw less misleading exception when property access not found [Twig] Fix deprecations with Twig 1.29 [FrameworkBundle] Fix validation cache warmer with failing or missing classes Fixed typo [FrameworkBundle] Removed the kernel.debug parameter from the cache pool namespace seed Fix email address fix the docblock in regard to the role argument [Bridge\Twig] Trigger deprecation when using FormExtension::$renderer Don't use the "app" global variable in the profiler [VarDumper] fix tests when xdebug is enabled Fix merge FIXED NON EXISTING TYPE DECLARATION [Form] Add failing test for data collector bug [Cache] Fix dumping SplDoublyLinkedList iter mode [Form] Fix FormDataCollector Ignore missing 'debug.file_link_formatter' service in Debug and Twig bundles ...
2 parents 3e61303 + 0370545 commit 7fff820

File tree

9 files changed

+142
-27
lines changed

9 files changed

+142
-27
lines changed

‎Compiler/CheckCircularReferencesPass.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,18 @@ private function checkOutEdges(array $edges)
6060
$id = $node->getId();
6161

6262
if (empty($this->checkedNodes[$id])) {
63-
$searchKey = array_search($id, $this->currentPath);
64-
$this->currentPath[] = $id;
6563

66-
if (false !== $searchKey) {
67-
throw new ServiceCircularReferenceException($id, array_slice($this->currentPath, $searchKey));
68-
}
64+
// don't check circular dependencies for lazy services
65+
if (!$node->getValue() || !$node->getValue()->isLazy()) {
66+
$searchKey = array_search($id, $this->currentPath);
67+
$this->currentPath[] = $id;
68+
69+
if (false !== $searchKey) {
70+
throw new ServiceCircularReferenceException($id, array_slice($this->currentPath, $searchKey));
71+
}
6972

70-
$this->checkOutEdges($node->getOutEdges());
73+
$this->checkOutEdges($node->getOutEdges());
74+
}
7175

7276
$this->checkedNodes[$id] = true;
7377
array_pop($this->currentPath);

‎ContainerBuilder.php

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,34 +1026,47 @@ public function getExpressionLanguageProviders()
10261026
}
10271027

10281028
/**
1029-
* Resolves env parameter placeholders in a string.
1029+
* Resolves env parameter placeholders in a string or an array.
10301030
*
1031-
* @param string $string The string to resolve
1031+
* @param mixed $value The value to resolve
10321032
* @param string|null $format A sprintf() format to use as replacement for env placeholders or null to use the default parameter format
10331033
* @param array &$usedEnvs Env vars found while resolving are added to this array
10341034
*
10351035
* @return string The string with env parameters resolved
10361036
*/
1037-
public function resolveEnvPlaceholders($string, $format = null, array &$usedEnvs = null)
1037+
public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs = null)
10381038
{
1039-
$bag = $this->getParameterBag();
1040-
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;
1041-
10421039
if (null === $format) {
10431040
$format = '%%env(%s)%%';
10441041
}
10451042

1043+
if (is_array($value)) {
1044+
$result = array();
1045+
foreach ($value as $k => $v) {
1046+
$result[$this->resolveEnvPlaceholders($k, $format, $usedEnvs)] = $this->resolveEnvPlaceholders($v, $format, $usedEnvs);
1047+
}
1048+
1049+
return $result;
1050+
}
1051+
1052+
if (!is_string($value)) {
1053+
return $value;
1054+
}
1055+
1056+
$bag = $this->getParameterBag();
1057+
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;
1058+
10461059
foreach ($envPlaceholders as $env => $placeholders) {
10471060
foreach ($placeholders as $placeholder) {
1048-
if (false !== stripos($string, $placeholder)) {
1049-
$string = str_ireplace($placeholder, sprintf($format, $env), $string);
1061+
if (false !== stripos($value, $placeholder)) {
1062+
$value = str_ireplace($placeholder, sprintf($format, $env), $value);
10501063
$usedEnvs[$env] = $env;
10511064
$this->envCounters[$env] = isset($this->envCounters[$env]) ? 1 + $this->envCounters[$env] : 1;
10521065
}
10531066
}
10541067
}
10551068

1056-
return $string;
1069+
return $value;
10571070
}
10581071

10591072
/**

‎Dumper/PhpDumper.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,13 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi
13101310
$visited[$argumentId] = true;
13111311

13121312
$service = $this->container->getDefinition($argumentId);
1313+
1314+
// if the proxy manager is enabled, disable searching for references in lazy services,
1315+
// as these services will be instantiated lazily and don't have direct related references.
1316+
if ($service->isLazy() && !$this->getProxyDumper() instanceof NullDumper) {
1317+
continue;
1318+
}
1319+
13131320
$arguments = array_merge($service->getMethodCalls(), $service->getArguments(), $service->getProperties());
13141321

13151322
if ($this->hasReference($id, $arguments, $deep, $visited)) {

‎Loader/XmlFileLoader.php

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -346,21 +346,22 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $lowercase = true)
346346
$arg->setAttribute('key', $arg->getAttribute('name'));
347347
}
348348

349-
if (!$arg->hasAttribute('key')) {
350-
$key = !$arguments ? 0 : max(array_keys($arguments)) + 1;
351-
} else {
352-
$key = $arg->getAttribute('key');
353-
}
354-
355-
// parameter keys are case insensitive
356-
if ('parameter' == $name && $lowercase) {
357-
$key = strtolower($key);
358-
}
359-
360349
// this is used by DefinitionDecorator to overwrite a specific
361350
// argument of the parent definition
362351
if ($arg->hasAttribute('index')) {
363352
$key = 'index_'.$arg->getAttribute('index');
353+
} elseif (!$arg->hasAttribute('key')) {
354+
// Append an empty argument, then fetch its key to overwrite it later
355+
$arguments[] = null;
356+
$keys = array_keys($arguments);
357+
$key = array_pop($keys);
358+
} else {
359+
$key = $arg->getAttribute('key');
360+
361+
// parameter keys are case insensitive
362+
if ('parameter' == $name && $lowercase) {
363+
$key = strtolower($key);
364+
}
364365
}
365366

366367
switch ($arg->getAttribute('type')) {

‎Tests/ContainerBuilderTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ public function testMerge()
495495
$bag = new EnvPlaceholderParameterBag();
496496
$bag->get('env(Foo)');
497497
$config = new ContainerBuilder($bag);
498-
$config->resolveEnvPlaceholders($bag->get('env(Bar)'));
498+
$this->assertSame(array('%env(Bar)%'), $config->resolveEnvPlaceholders(array($bag->get('env(Bar)'))));
499499
$container->merge($config);
500500
$this->assertEquals(array('Foo' => 0, 'Bar' => 1), $container->getEnvCounters());
501501
}

‎Tests/Dumper/PhpDumperTest.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Tests\Dumper;
1313

14+
use DummyProxyDumper;
1415
use Symfony\Component\Config\FileLocator;
1516
use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
@@ -21,6 +22,8 @@
2122
use Symfony\Component\DependencyInjection\Variable;
2223
use Symfony\Component\ExpressionLanguage\Expression;
2324

25+
require_once __DIR__.'/../Fixtures/includes/classes.php';
26+
2427
class PhpDumperTest extends \PHPUnit_Framework_TestCase
2528
{
2629
protected static $fixturesPath;
@@ -340,4 +343,52 @@ public function testInitializePropertiesBeforeMethodCalls()
340343
$container = new \Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls();
341344
$this->assertTrue($container->get('bar')->callPassed(), '->dump() initializes properties before method calls');
342345
}
346+
347+
public function testCircularReferenceAllowanceForLazyServices()
348+
{
349+
$container = new ContainerBuilder();
350+
$container->register('foo', 'stdClass')->addArgument(new Reference('bar'));
351+
$container->register('bar', 'stdClass')->setLazy(true)->addArgument(new Reference('foo'));
352+
$container->compile();
353+
354+
$dumper = new PhpDumper($container);
355+
$dumper->dump();
356+
}
357+
358+
public function testCircularReferenceAllowanceForInlinedDefinitionsForLazyServices()
359+
{
360+
/*
361+
* test graph:
362+
* [connection] -> [event_manager] --> [entity_manager](lazy)
363+
* |
364+
* --(call)- addEventListener ("@lazy_service")
365+
*
366+
* [lazy_service](lazy) -> [entity_manager](lazy)
367+
*
368+
*/
369+
370+
$container = new ContainerBuilder();
371+
372+
$eventManagerDefinition = new Definition('stdClass');
373+
374+
$connectionDefinition = $container->register('connection', 'stdClass');
375+
$connectionDefinition->addArgument($eventManagerDefinition);
376+
377+
$container->register('entity_manager', 'stdClass')
378+
->setLazy(true)
379+
->addArgument(new Reference('connection'));
380+
381+
$lazyServiceDefinition = $container->register('lazy_service', 'stdClass');
382+
$lazyServiceDefinition->setLazy(true);
383+
$lazyServiceDefinition->addArgument(new Reference('entity_manager'));
384+
385+
$eventManagerDefinition->addMethodCall('addEventListener', array(new Reference('lazy_service')));
386+
387+
$container->compile();
388+
389+
$dumper = new PhpDumper($container);
390+
391+
$dumper->setProxyDumper(new DummyProxyDumper());
392+
$dumper->dump();
393+
}
343394
}

‎Tests/Fixtures/includes/classes.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<?php
22

3+
use Symfony\Component\DependencyInjection\Definition;
4+
use Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface as ProxyDumper;
5+
36
function sc_configure($instance)
47
{
58
$instance->configure();
@@ -76,3 +79,21 @@ public function callPassed()
7679
return $this->callPassed;
7780
}
7881
}
82+
83+
class DummyProxyDumper implements ProxyDumper
84+
{
85+
public function isProxyCandidate(Definition $definition)
86+
{
87+
return false;
88+
}
89+
90+
public function getProxyFactoryCode(Definition $definition, $id)
91+
{
92+
return '';
93+
}
94+
95+
public function getProxyCode(Definition $definition)
96+
{
97+
return '';
98+
}
99+
}

‎Tests/Fixtures/xml/with_key_outside_collection.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
3+
<services>
4+
<service id="foo" class="Foo">
5+
<argument key="type">foo</argument>
6+
<argument>bar</argument>
7+
</service>
8+
</services>
9+
</container>

‎Tests/Loader/XmlFileLoaderTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,4 +571,13 @@ public function testAliasDefinitionContainsUnsupportedElements()
571571

572572
$this->assertTrue($container->has('bar'));
573573
}
574+
575+
public function testArgumentWithKeyOutsideCollection()
576+
{
577+
$container = new ContainerBuilder();
578+
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
579+
$loader->load('with_key_outside_collection.xml');
580+
581+
$this->assertSame(array('type' => 'foo', 'bar'), $container->getDefinition('foo')->getArguments());
582+
}
574583
}

0 commit comments

Comments
 (0)