Skip to content

Commit

Permalink
bug #20199 [DependencyInjection] Fix duplication of placeholders (mic…
Browse files Browse the repository at this point in the history
…kadoo)

This PR was squashed before being merged into the 3.2-dev branch (closes #20199).

Discussion
----------

[DependencyInjection] Fix duplication of placeholders

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  |no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20198
| License       | MIT
| Doc PR        | reference to the documentation PR, if any

Fixes performance issues related to merging parameter bags when using the `env()` parameters introduced in #19681

Commits
-------

124f30d [DependencyInjection] Fix duplication of placeholders
  • Loading branch information
fabpot committed Oct 11, 2016
2 parents a5d134b + 124f30d commit 406462f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
Expand Up @@ -30,7 +30,9 @@ public function get($name)
$env = substr($name, 4, -1);

if (isset($this->envPlaceholders[$env])) {
return $this->envPlaceholders[$env][0];
foreach ($this->envPlaceholders[$env] as $placeholder) {
return $placeholder; // return first result
}
}
if (preg_match('/\W/', $env)) {
throw new InvalidArgumentException(sprintf('Invalid %s name: only "word" characters are allowed.', $name));
Expand All @@ -44,7 +46,11 @@ public function get($name)
}
}

return $this->envPlaceholders[$env][] = sprintf('env_%s_%s', $env, md5($name.uniqid(mt_rand(), true)));
$uniqueName = md5($name.uniqid(mt_rand(), true));
$placeholder = sprintf('env_%s_%s', $env, $uniqueName);
$this->envPlaceholders[$env][$placeholder] = $placeholder;

return $placeholder;
}

return parent::get($name);
Expand Down
Expand Up @@ -23,4 +23,19 @@ public function testGetThrowsInvalidArgumentExceptionIfEnvNameContainsNonWordCha
$bag = new EnvPlaceholderParameterBag();
$bag->get('env(%foo%)');
}

public function testMergeWillNotDuplicateIdenticalParameters()
{
$originalPlaceholders = array('database_host' => array('localhost'));
$firstBag = new EnvPlaceholderParameterBag($originalPlaceholders);

// initialize placeholders
$firstBag->get('env(database_host)');
$secondBag = clone $firstBag;

$firstBag->mergeEnvPlaceholders($secondBag);
$mergedPlaceholders = $firstBag->getEnvPlaceholders();

$this->assertCount(1, $mergedPlaceholders['database_host']);
}
}

0 comments on commit 406462f

Please sign in to comment.