8000 [DependencyInjection] Introduce a new ParameterResolver by GuilhemN · Pull Request #17192 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[DependencyInjection] Introduce a new ParameterResolver #17192

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[DependencyInjection] Introduce a new ParameterResolver #17192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 16 additions & 68 deletions src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBag.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
*/
class ParameterBag implements ParameterBagInterface
{
use ParameterResolverTrait;

protected $parameters = array();
protected $resolved = false;

Expand Down Expand Up @@ -157,36 +159,6 @@ public function resolve()
$this->resolved = true;
}

/**
* Replaces parameter placeholders (%name%) by their values.
*
* @param mixed $value A value
* @param array $resolving An array of keys that are being resolved (used internally to detect circular references)
*
* @return mixed The resolved v 8000 alue
*
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist
* @throws ParameterCircularReferenceException if a circular reference if detected
* @throws RuntimeException when a given parameter has a type problem.
*/
public function resolveValue($value, array $resolving = array())
{
if (is_array($value)) {
$args = array();
foreach ($value as $k => $v) {
$args[$this->resolveValue($k, $resolving)] = $this->resolveValue($v, $resolving);
}

return $args;
}

if (!is_string($value)) {
return $value;
}

return $this->resolveString($value, $resolving);
}

/**
* Resolves parameters inside a string.
*
Expand All @@ -198,50 +170,21 @@ public function resolveValue($value, array $resolving = array())
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist
* @throws ParameterCircularReferenceException if a circular reference if detected
* @throws RuntimeException when a given parameter has a type problem.
*
* @deprecated since 3.2, to be removed in 4.0. Use {@link static::resolveValue()} instead.
*/
public function resolveString($value, array $resolving = array())
{
// we do this to deal with non string values (Boolean, integer, ...)
// as the preg_replace_callback throw an exception when trying
// a non-string in a parameter value
if (preg_match('/^%([^%\s]+)%$/', $value, $match)) {
$key = $match[1];
$lcKey = strtolower($key);

if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
}

$resolving[$lcKey] = true;

return $this->resolved ? $this->get($key) : $this->resolveValue($this->get($key), $resolving);
}

return preg_replace_callback('/%%|%([^%\s]+)%/', function ($match) use ($resolving, $value) {
// skip %%
if (!isset($match[1])) {
return '%%';
}

$key = $match[1];
$lcKey = strtolower($key);
if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
}

$resolved = $this->get($key);
@trigger_error(sprintf('The %s method is deprecated since version 3.2 and will be removed in 4.0. Use %s::resolveValue() instead.', __METHOD__, __CLASS__), E_USER_DEPRECATED);

if (!is_string($resolved) && !is_numeric($resolved)) {
throw new RuntimeException(sprintf('A string value must be composed of strings and/or numbers, but found parameter "%s" of type %s inside string value "%s".', $key, gettype($resolved), $value));
}

$resolved = (string) $resolved;
$resolving[$lcKey] = true;

return $this->isResolved() ? $resolved : $this->resolveString($resolved, $resolving);
}, $value);
return $this->resolveValue($value, $resolving);
}

/**
* Whether the parameters are resolved or not (%parameter% replaced).
*
* @return bool
*/
public function isResolved()
{
return $this->resolved;
Expand Down Expand Up @@ -288,4 +231,9 @@ public function unescapeValue($value)

return $value;
}

protected function getParameter($name, $resolving = array())
{
return $this->resolved ? $this->get($name) : $this->resolveValue($this->get($name), $resolving);
}
}
6 changes: 5 additions & 1 deletion src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBagInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ public function resolve();
*
* @param mixed $value A value
*
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist
* @return mixed The resolved value
*
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist
* @throws ParameterCircularReferenceException if a circular reference is detected
* @throws RuntimeException when a given parameter has a type problem.
*/
public function resolveValue($value);

Expand Down
102 changes: 102 additions & 0 deletions src/Symfony/Component/DependencyInjection/ParameterBag/ParameterResolverTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\DependencyInjection\ParameterBag;

use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
use Symfony\Component\DependencyInjection\Exception\ParameterCircularReferenceException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;

/**
* Implementation of {@link ParameterResolverInterface}.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Guilhem N. <egetick@gmail.com>
*/
trait ParameterResolverTrait
{
/**
* Replaces parameter placeholders (%name%) by their values.
*
* @param mixed $value A value
*
* @return mixed The resolved value
*
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist
* @throws ParameterCircularReferenceException if a circular reference is detected
* @throws RuntimeException when a given parameter has a type problem.
*/
public function resolveValue($value, array $resolving = array())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo the public interface should not leak the $resolving argument (it's an implementation detail). We could add a private method that does the actual resolving an let resolveValue() call it with an empty array as the second argument.

Copy li 8000 nk
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And by the way, the ParameterResolverInterface even doesn't require this second argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine for me.

Besides that, I'm wondering if we shouldn't make this method private by default: in userland it is unlikely to be used outside the class using the trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: I agree, but we can't do that for bc reasons, see our previous conversation

{
if (is_array($value)) {
$args = array();
foreach ($value as $k => $v) {
$args[$this->resolveValue($k, $resolving)] = $this->resolveValue($v, $resolving);
}

return $args;
}

if (!is_string($value)) {
return $value;
}

// we do this to deal with non string values (Boolean, integer, ...)
// as the preg_replace_callback throw an exception when trying
// a non-string in a parameter value
if (preg_match('/^%([^%\s]+)%$/', $value, $match)) {
$key = $match[1];
$lcKey = strtolower($key);

if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
}

$resolving[$lcKey] = true;

return $this->getParameter($key, $resolving);
}

return preg_replace_callback('/%%|%([^%\s]+)%/', function ($match) use ($resolving, $value) {
// skip %%
if (!isset($match[1])) {
return '%%';
}

$key = $match[1];
$lcKey = strtolower($key);
if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
}

$resolving[$lcKey] = true;
$resolved = $this->getParameter($key, $resolving);

if (!is_string($resolved) && !is_numeric($resolved)) {
throw new RuntimeException(sprintf('A string value must be composed of strings and/or numbers, but found parameter "%s" of type %s inside string value "%s".', $key, gettype($resolved), $value));
}

return (string) $resolved;
}, $value);
}

/**
* Gets a parameter.
*
* @param string $name The parameter name
* @param array $resolving used internally to detect circular references
*
* @return mixed The parameter value
*
* @throws ParameterNotFoundException if the parameter is not defined
*/
abstract protected function getParameter($name, $resolving = array());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this function should be named getParameter or something else to avoid conflicts...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having an abstract method in the trait. What about adding a private setParameterBag() method instead that must be called by the consuming class? This method could populate an internal ParameterBag property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like it to work on top of a any container (and a container may not use ParameterBag). I didn't find anything better to solve this abstraction issue.

}
70 changes: 1 addition & 69 deletions src/Symfony/Component/DependencyInjection/Tests/ParameterBag/ParameterBagTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
use Symfony\Component\DependencyInjection\Exception\ParameterCircularReferenceException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;

class ParameterBagTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -107,73 +105,6 @@ public function testHas()
$this->assertFalse($bag->has('bar'), '->has() returns false if a parameter is not defined');
}

public function testResolveValue()
{
$bag = new ParameterBag(array());
$this->assertEquals('foo', $bag->resolveValue('foo'), '->resolveValue() returns its argument unmodified if no placeholders are found');

$bag = new ParameterBag(array('foo' => 'bar'));
$this->assertEquals('I\'m a bar', $bag->resolveValue('I\'m a %foo%'), '->resolveValue() replaces placeholders by their values');
$this->assertEquals(array('bar' => 'bar'), $bag->resolveValue(array('%foo%' => '%foo%')), '->resolveValue() replaces placeholders in keys and values of arrays');
$this->assertEquals(array('bar' => array('bar' => array('bar' => 'bar'))), $bag->resolveValue(array('%foo%' => array('%foo%' => array('%foo%' => '%foo%')))), '->resolveValue() replaces placeholders in nested arrays');
$this->assertEquals('I\'m a %%foo%%', $bag->resolveValue('I\'m a %%foo%%'), '->resolveValue() supports % escaping by doubling it');
$this->assertEquals('I\'m a bar %%foo bar', $bag->resolveValue('I\'m a %foo% %%foo %foo%'), '->resolveValue() supports % escaping by doubling it');
$this->assertEquals(array('foo' => array('bar' => array('ding' => 'I\'m a bar %%foo %%bar'))), $bag->resolveValue(array('foo' => array('bar' => array('ding' => 'I\'m a bar %%foo %%bar')))), '->resolveValue() supports % escaping by doubling it');

$bag = new ParameterBag(array('foo' => true));
$this->assertTrue($bag->resolveValue('%foo%'), '->resolveValue() replaces arguments that are just a placeholder by their value without casting them to strings');
$bag = new ParameterBag(array('foo' => null));
$this->assertNull($bag->resolveValue('%foo%'), '->resolveValue() replaces arguments that are just a placeholder by their value without casting them to strings');

$bag = new ParameterBag(array('foo' => 'bar', 'baz' => '%%%foo% %foo%%% %%foo%% %%%foo%%%'));
$this->assertEquals('%%bar bar%% %%foo%% %%bar%%', $bag->resolveValue('%baz%'), '->resolveValue() replaces params placed besides escaped %');

$bag = new ParameterBag(array('baz' => '%%s?%%s'));
$this->assertEquals('%%s?%%s', $bag->resolveValue('%baz%'), '->resolveValue() is not replacing greedily');

$bag = new ParameterBag(array());
try {
$bag->resolveValue('%foobar%');
$this->fail('->resolveValue() throws an InvalidArgumentException if a placeholder references a non-existent parameter');
} catch (ParameterNotFoundException $e) {
$this->assertEquals('You have requested a non-existent parameter "foobar".', $e->getMessage(), '->resolveValue() throws a ParameterNotFoundException if a placeholder references a non-existent parameter');
}

try {
$bag->resolveValue('foo %foobar% bar');
$this->fail('->resolveValue() throws a ParameterNotFoundException if a placeholder references a non-existent parameter');
} catch (ParameterNotFoundException $e) {
$this->assertEquals('You have requested a non-existent parameter "foobar".', $e->getMessage(), '->resolveValue() throws a ParameterNotFoundException if a placeholder references a non-existent parameter');
}

$bag = new ParameterBag(array('foo' => 'a %bar%', 'bar' => array()));
try {
$bag->resolveValue('%foo%');
$this->fail('->resolveValue() throws a RuntimeException when a parameter embeds another non-string parameter');
} catch (RuntimeException $e) {
$this->assertEquals('A string value must be composed of strings and/or numbers, but found parameter "bar" of type array inside string value "a %bar%".', $e->getMessage(), '->resolveValue() throws a RuntimeException when a parameter embeds another non-string parameter');
}

$bag = new ParameterBag(array('foo' => '%bar%', 'bar' => '%foobar%', 'foobar' => '%foo%'));
try {
$bag->resolveValue('%foo%');
$this->fail('->resolveValue() throws a ParameterCircularReferenceException when a parameter has a circular reference');
} catch (ParameterCircularReferenceException $e) {
$this->assertEquals('Circular reference detected for parameter "foo" ("foo" > "bar" > "foobar" > "foo").', $e->getMessage(), '->resolveValue() throws a ParameterCircularReferenceException when a parameter has a circular reference');
}

$bag = new ParameterBag(array('foo' => 'a %bar%', 'bar' => 'a %foobar%', 'foobar' => 'a %foo%'));
try {
$bag->resolveValue('%foo%');
$this->fail('->resolveValue() throws a ParameterCircularReferenceException when a parameter has a circular reference');
} catch (ParameterCircularReferenceException $e) {
$this->assertEquals('Circular reference detected for parameter "foo" ("foo" > "bar" > "foobar" > "foo").', $e->getMessage(), '->resolveValue() throws a ParameterCircularReferenceException when a parameter has a circular reference');
}

$bag = new ParameterBag(array('host' => 'foo.bar', 'port' => 1337));
$this->assertEquals('foo.bar:1337', $bag->resolveValue('%host%:%port%'));
}

public function testResolveIndicatesWhyAParameterIsNeeded()
{
$bag = new ParameterBag(array('foo' => '%bar%'));
Expand Down Expand Up @@ -221,6 +152,7 @@ public function testEscapeValue()

/**
* @dataProvider stringsWithSpacesProvider
* @group legacy
*/
public function testResolveStringWithSpacesReturnsString($expected, $test, $description)
{
Expand Down
Loading
0