Skip to content

Commit 47fd5e8

Browse files
fabpotfabpot
committed
[DependencyInjection] fixed placeholder management in parameter values
1 parent 6bad580 commit 47fd5e8

File tree

14 files changed

+99
-59
lines changed

14 files changed

+99
-59
lines changed

‎src/Symfony/Components/DependencyInjection/Container.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,17 @@ public function __construct(ParameterBagInterface $parameterBag = null)
7272
}
7373

7474
/**
75-
* Freezes the container parameter bag.
75+
* Freezes the container.
76+
*
77+
* This method does two things:
78+
*
79+
* * Parameter values are resolved;
80+
* * The parameter bag is freezed.
7681
*/
7782
public function freeze()
7883
{
84+
$this->parameterBag->resolve();
85+
7986
$this->parameterBag = new FrozenParameterBag($this->parameterBag->all());
8087
}
8188

‎src/Symfony/Components/DependencyInjection/ContainerBuilder.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ public function addObjectResource($object)
7979
*/
8080
public function loadFromExtension(LoaderExtensionInterface $extension, $tag, array $values = array())
8181
{
82+
if (true === $this->isFrozen()) {
83+
throw new \LogicException('Cannot load from an extension on a frozen container.');
84+
}
85+
8286
$namespace = $extension->getAlias();
8387

8488
$this->addObjectResource($extension);
@@ -187,6 +191,10 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
187191
*/
188192
public function merge(ContainerBuilder $container)
189193
{
194+
if (true === $this->isFrozen()) {
195+
throw new \LogicException('Cannot merge on a frozen container.');
196+
}
197+
190198
$this->addDefinitions($container->getDefinitions());
191199
$this->addAliases($container->getAliases());
192200
$this->parameterBag->add($container->getParameterBag()->all());
@@ -215,10 +223,16 @@ public function getExtensionContainers()
215223
}
216224

217225
/**
218-
* Commits the extension configuration into the main configuration
219-
* and resolves parameter values.
226+
* Freezes the container.
227+
*
228+
* This method does four things:
229+
*
230+
* * The extension configurations are merged;
231+
* * Parameter values are resolved;
232+
* * The parameter bag is freezed;
233+
* * Extension loading is disabled.
220234
*/
221-
public function commit()
235+
public function freeze()
222236
{
223237
$parameters = $this->parameterBag->all();
224238
$definitions = $this->definitions;
@@ -233,9 +247,7 @@ public function commit()
233247
$this->addAliases($aliases);
234248
$this->parameterBag->add($parameters);
235249

236-
foreach ($this->parameterBag->all() as $key => $value) {
237-
$this->parameterBag->set($key, $this->getParameterBag()->resolveValue($value));
238-
}
250+
parent::freeze();
239251
}
240252

241253
/**

‎src/Symfony/Components/DependencyInjection/Dumper/XmlDumper.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,13 @@ protected function addParameters()
4242
return '';
4343
}
4444

45-
return sprintf(" <parameters>\n%s </parameters>\n", $this->convertParameters($this->escape($this->container->getParameterBag()->all()), 'parameter', 4));
45+
if ($this->container->isFrozen()) {
46+
$parameters = $this->escape($this->container->getParameterBag()->all());
47+
} else {
48+
$parameters = $this->container->getParameterBag()->all();
49+
}
50+
51+
return sprintf(" <parameters>\n%s </parameters>\n", $this->convertParameters($parameters, 'parameter', 4));
4652
}
4753

4854
protected function addService($id, $definition)

‎src/Symfony/Components/DependencyInjection/Dumper/YamlDumper.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,13 @@ protected function addParameters()
128128
return '';
129129
}
130130

131-
return Yaml::dump(array('parameters' => $this->prepareParameters($this->container->getParameterBag()->all())), 2);
131+
if ($this->container->isFrozen()) {
132+
$parameters = $this->prepareParameters($this->container->getParameterBag()->all());
133+
} else {
134+
$parameters = $this->container->getParameterBag()->all();
135+
}
136+
137+
return Yaml::dump(array('parameters' => $parameters), 2);
132138
}
133139

134140
/**

‎src/Symfony/Components/DependencyInjection/ParameterBag/ParameterBag.php

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ public function has($name)
105105
return array_key_exists(strtolower($name), $this->parameters);
106106
}
107107

108+
/**
109+
* Replaces parameter placeholders (%name%) by their values for all parameters.
110+
*/
111+
public function resolve()
112+
{
113+
foreach ($this->parameters as $key => $value) {
114+
$this->parameters[$key] = $this->resolveValue($value);
115+
}
116+
}
117+
108118
/**
109119
* Replaces parameter placeholders (%name%) by their values.
110120
*
@@ -120,31 +130,24 @@ public function resolveValue($value)
120130
$args[$this->resolveValue($k)] = $this->resolveValue($v);
121131
}
122132

123-
$value = $args;
124-
} else if (is_string($value)) {
125-
if (preg_match('/^%([^%]+)%$/', $value, $match)) {
126-
// we do this to deal with non string values (boolean, integer, ...)
127-
// the preg_replace_callback converts them to strings
128-
if (!array_key_exists($name = strtolower($match[1]), $this->parameters)) {
129-
throw new \RuntimeException(sprintf('The parameter "%s" must be defined.', $name));
130-
}
131-
132-
$value = $this->parameters[$name];
133-
} else {
134-
$parameters = $this->parameters;
135-
$replaceParameter = function ($match) use ($parameters, $value)
136-
{
137-
if (!array_key_exists($name = strtolower($match[2]), $parameters)) {
138-
throw new \RuntimeException(sprintf('The parameter "%s" must be defined (used in the following expression: "%s").', $name, $value));
139-
}
140-
141-
return $parameters[$name];
142-
};
143-
144-
$value = str_replace('%%', '%', preg_replace_callback('/(?<!%)(%)([^%]+)\1/', $replaceParameter, $value));
145-
}
133+
return $args;
146134
}
147135

148-
return $value;
136+
if (!is_string($value)) {
137+
return $value;
138+
}
139+
140+
if (preg_match('/^%([^%]+)%$/', $value, $match)) {
141+
// we do this to deal with non string values (boolean, integer, ...)
142+
// the preg_replace_callback converts them to strings
143+
return $this->get(strtolower($match[1]));
144+
}
145+
146+
return str_replace('%%', '%', preg_replace_callback(array('/(?<!%)%([^%]+)%/'), array($this, 'resolveValueCallback'), $value));
147+
}
148+
149+
protected function resolveValueCallback($match)
150+
{
151+
return $this->get(strtolower($match[1]));
149152
}
150153
}

‎tests/Symfony/Tests/Components/DependencyInjection/ContainerBuilderTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,14 +313,14 @@ public function testMerge()
313313
$config = new ContainerBuilder(new ParameterBag(array('foo' => '%bar%')));
314314
$container->merge($config);
315315
////// FIXME
316-
$container->commit();
316+
$container->freeze();
317317
$this->assertEquals(array('bar' => 'foo', 'foo' => 'foo'), $container->getParameterBag()->all(), '->merge() evaluates the values of the parameters towards already defined ones');
318318

319319
$container = new ContainerBuilder(new ParameterBag(array('bar' => 'foo')));
320320
$config = new ContainerBuilder(new ParameterBag(array('foo' => '%bar%', 'baz' => '%foo%')));
321321
$container->merge($config);
322322
////// FIXME
323-
$container->commit();
323+
$container->freeze();
324324
$this->assertEquals(array('bar' => 'foo', 'foo' => 'foo', 'baz' => 'foo'), $container->getParameterBag()->all(), '->merge() evaluates the values of the parameters towards already defined ones');
325325

326326
$container = new ContainerBuilder();

‎tests/Symfony/Tests/Components/DependencyInjection/CrossCheckTest.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,24 @@ public function testCrossCheck($fixture, $type)
3232
$loaderClass = 'Symfony\\Components\\DependencyInjection\\Loader\\'.ucfirst($type).'FileLoader';
3333
$dumperClass = 'Symfony\\Components\\DependencyInjection\\Dumper\\'.ucfirst($type).'Dumper';
3434

35-
$container1 = new ContainerBuilder();
36-
$loader1 = new $loaderClass($container1);
37-
$loader1->load(self::$fixturesPath.'/'.$type.'/'.$fixture);
38-
$container1->setParameter('path', self::$fixturesPath.'/includes');
35+
$tmp = tempnam('sf_service_container', 'sf');
36+
37+
$loader1 = new $loaderClass();
38+
file_put_contents($tmp, file_get_contents(self::$fixturesPath.'/'.$type.'/'.$fixture));
39+
$container1 = $loader1->load($tmp);
3940

4041
$dumper = new $dumperClass($container1);
41-
$tmp = tempnam('sf_service_container', 'sf');
4242
file_put_contents($tmp, $dumper->dump());
4343

44-
$container2 = new ContainerBuilder();
45-
$loader2 = new $loaderClass($container2);
46-
$loader2->load($tmp);
47-
$container2->setParameter('path', self::$fixturesPath.'/includes');
44+
$loader2 = new $loaderClass();
45+
$container2 = $loader2->load($tmp);
4846

4947
unlink($tmp);
5048

49+
$this->assertEquals($container2->getAliases(), $container1->getAliases(), 'loading a dump from a previously loaded container returns the same container');
50+
$this->assertEquals($container2->getDefinitions(), $container1->getDefinitions(), 'loading a dump from a previously loaded container returns the same container');
51+
$this->assertEquals($container2->getParameterBag()->all(), $container1->getParameterBag()->all(), '->getParameterBag() returns the same value for both containers');
52+
5153
$this->assertEquals(serialize($container2), serialize($container1), 'loading a dump from a previously loaded container returns the same container');
5254

5355
$this->assertEquals($container2->getParameterBag()->all(), $container1->getParameterBag()->all(), '->getParameterBag() returns the same value for both containers');

‎tests/Symfony/Tests/Components/DependencyInjection/Fixtures/containers/container8.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
use Symfony\Components\DependencyInjection\ParameterBag\ParameterBag;
55

66
$container = new ContainerBuilder(new ParameterBag(array(
7-
'FOO' => 'bar',
8-
'bar' => 'foo is %foo bar',
7+
'FOO' => '%baz%',
8+
'baz' => 'bar',
9+
'bar' => 'foo is %%foo bar',
910
'values' => array(true, false, null, 0, 1000.3, 'true', 'false', 'null'),
1011
)));
1112

‎tests/Symfony/Tests/Components/DependencyInjection/Fixtures/php/services8.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ public function findAnnotatedServiceIds($name)
4747
protected function getDefaultParameters()
4848
{
4949
return array(
50-
'foo' => 'bar',
51-
'bar' => 'foo is %foo bar',
50+
'foo' => '%baz%',
51+
'baz' => 'bar',
52+
'bar' => 'foo is %%foo bar',
5253
'values' => array(
5354
0 => true,
5455
1 => false,

‎tests/Symfony/Tests/Components/DependencyInjection/Fixtures/xml/services8.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
55
xsi:schemaLocation="http://www.symfony-project.org/schema/dic/services http://www.symfony-project.org/schema/dic/services/services-1.0.xsd">
66
<parameters>
7-
<parameter key="foo">bar</parameter>
7+
<parameter key="foo">%baz%</parameter>
8+
<parameter key="baz">bar</parameter>
89
<parameter key="bar">foo is %%foo bar</parameter>
910
<parameter key="values" type="collection">
1011
<parameter>true</parameter>

‎tests/Symfony/Tests/Components/DependencyInjection/Fixtures/yaml/services8.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
parameters:
2-
foo: bar
2+
foo: '%baz%'
3+
baz: bar
34
bar: 'foo is %%foo bar'
45
values: [true, false, null, 0, 1000.3, 'true', 'false', 'null']
56

‎tests/Symfony/Tests/Components/DependencyInjection/Loader/XmlFileLoaderTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public function testExtensions()
173173

174174
// extension without an XSD
175175
$config = $loader->load('extensions/services1.xml');
176-
$config->commit();
176+
$config->freeze();
177177
$services = $config->getDefinitions();
178178
$parameters = $config->getParameterBag()->all();
179179

@@ -185,7 +185,7 @@ public function testExtensions()
185185

186186
// extension with an XSD
187187
$config = $loader->load('extensions/services2.xml');
188-
$config->commit();
188+
$config->freeze();
189189
$services = $config->getDefinitions();
190190
$parameters = $config->getParameterBag()->all();
191191

‎tests/Symfony/Tests/Components/DependencyInjection/Loader/YamlFileLoaderTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function testExtensions()
107107
$loader = new ProjectLoader3(self::$fixturesPath.'/yaml');
108108

109109
$config = $loader->load('services10.yml');
110-
$config->commit();
110+
$config->freeze();
111111
$services = $config->getDefinitions();
112112
$parameters = $config->getParameterBag()->all();
113113

‎tests/Symfony/Tests/Components/DependencyInjection/ParameterBag/ParameterBagTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,18 @@ public function testResolveValue()
9797
$bag = new ParameterBag(array());
9898
try {
9999
$bag->resolveValue('%foobar%', array());
100-
$this->fail('->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
100+
$this->fail('->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
101101
} catch (\Exception $e) {
102-
$this->assertInstanceOf('\RuntimeException', $e, '->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
103-
$this->assertEquals('The parameter "foobar" must be defined.', $e->getMessage(), '->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
102+
$this->assertInstanceOf('\InvalidArgumentException', $e, '->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
103+
$this->assertEquals('The parameter "foobar" must be defined.', $e->getMessage(), '->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
104104
}
105105

106106
try {
107107
$bag->resolveValue('foo %foobar% bar', array());
108-
$this->fail('->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
108+
$this->fail('->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
109109
} catch (\Exception $e) {
110-
$this->assertInstanceOf('\RuntimeException', $e, '->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
111-
$this->assertEquals('The parameter "foobar" must be defined (used in the following expression: "foo %foobar% bar").', $e->getMessage(), '->resolveValue() throws a RuntimeException if a placeholder references a non-existant parameter');
110+
$this->assertInstanceOf('\InvalidArgumentException', $e, '->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
111+
$this->assertEquals('The parameter "foobar" must be defined.', $e->getMessage(), '->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existant parameter');
112112
}
113113
}
114114
}

0 commit comments

Comments
 (0)