feature #28858 [DI] Deprecated using env vars with cannotBeEmpty() (ro0NL)
This PR was squashed before being merged into the 4.3-dev branch (closes #28858).
Discussion
----------
[DI] Deprecated using env vars with cannotBeEmpty()
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes-ish
| New feature? | yes
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | yes
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | #28827
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
Continuation of #28838 for 4.2
Using environment variables for nodes marked `cannotBeEmpty()` is semantically not possible, we'll never know the value is empty yes/no during compile time. Neither we should assume one or another.
Commits
-------
397c19ee5f
[DI] Deprecated using env vars with cannotBeEmpty()
This commit is contained in:
commit
e6954494af
|
@ -29,6 +29,7 @@ Config
|
||||||
```
|
```
|
||||||
|
|
||||||
* Deprecated `FileLoaderLoadException`, use `LoaderLoadException` instead.
|
* Deprecated `FileLoaderLoadException`, use `LoaderLoadException` instead.
|
||||||
|
* Deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()`
|
||||||
|
|
||||||
Console
|
Console
|
||||||
-------
|
-------
|
||||||
|
|
|
@ -18,6 +18,7 @@ Config
|
||||||
* Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
|
* Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
|
||||||
* The `Processor` class has been made final
|
* The `Processor` class has been made final
|
||||||
* Removed `FileLoaderLoadException`, use `LoaderLoadException` instead.
|
* Removed `FileLoaderLoadException`, use `LoaderLoadException` instead.
|
||||||
|
* Using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` will throw an exception.
|
||||||
|
|
||||||
Console
|
Console
|
||||||
-------
|
-------
|
||||||
|
|
|
@ -6,6 +6,7 @@ CHANGELOG
|
||||||
|
|
||||||
* deprecated constructing a `TreeBuilder` without passing root node information
|
* deprecated constructing a `TreeBuilder` without passing root node information
|
||||||
* renamed `FileLoaderLoadException` to `LoaderLoadException`
|
* renamed `FileLoaderLoadException` to `LoaderLoadException`
|
||||||
|
* deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()`
|
||||||
|
|
||||||
4.1.0
|
4.1.0
|
||||||
-----
|
-----
|
||||||
|
|
|
@ -48,6 +48,8 @@ class ScalarNode extends VariableNode
|
||||||
*/
|
*/
|
||||||
protected function isValueEmpty($value)
|
protected function isValueEmpty($value)
|
||||||
{
|
{
|
||||||
|
// assume environment variables are never empty (which in practice is likely to be true during runtime)
|
||||||
|
// not doing so breaks many configs that are valid today
|
||||||
if ($this->isHandlingPlaceholder()) {
|
if ($this->isHandlingPlaceholder()) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
|
@ -81,6 +81,19 @@ class VariableNode extends BaseNode implements PrototypeNodeInterface
|
||||||
*/
|
*/
|
||||||
protected function finalizeValue($value)
|
protected function finalizeValue($value)
|
||||||
{
|
{
|
||||||
|
// deny environment variables only when using custom validators
|
||||||
|
// this avoids ever passing an empty value to final validation closures
|
||||||
|
if (!$this->allowEmptyValue && $this->isHandlingPlaceholder() && $this->finalValidationClosures) {
|
||||||
|
@trigger_error(sprintf('Setting path "%s" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead.', $this->getPath()), E_USER_DEPRECATED);
|
||||||
|
// $e = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an environment variable when empty values are not allowed by definition and are validated.', $this->getPath(), json_encode($value)));
|
||||||
|
// if ($hint = $this->getInfo()) {
|
||||||
|
// $e->addHint($hint);
|
||||||
|
// }
|
||||||
|
// $e->setPath($this->getPath());
|
||||||
|
//
|
||||||
|
// throw $e;
|
||||||
|
}
|
||||||
|
|
||||||
if (!$this->allowEmptyValue && $this->isValueEmpty($value)) {
|
if (!$this->allowEmptyValue && $this->isValueEmpty($value)) {
|
||||||
$ex = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an empty value, but got %s.', $this->getPath(), json_encode($value)));
|
$ex = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an empty value, but got %s.', $this->getPath(), json_encode($value)));
|
||||||
if ($hint = $this->getInfo()) {
|
if ($hint = $this->getInfo()) {
|
||||||
|
@ -120,6 +133,8 @@ class VariableNode extends BaseNode implements PrototypeNodeInterface
|
||||||
* @param mixed $value
|
* @param mixed $value
|
||||||
*
|
*
|
||||||
* @return bool
|
* @return bool
|
||||||
|
*
|
||||||
|
* @see finalizeValue()
|
||||||
*/
|
*/
|
||||||
protected function isValueEmpty($value)
|
protected function isValueEmpty($value)
|
||||||
{
|
{
|
||||||
|
|
|
@ -211,6 +211,38 @@ class ValidateEnvPlaceholdersPassTest extends TestCase
|
||||||
$this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
|
$this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* NOT LEGACY (test exception in 5.0).
|
||||||
|
*
|
||||||
|
* @group legacy
|
||||||
|
* @expectedDeprecation Setting path "env_extension.scalar_node_not_empty_validated" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead.
|
||||||
|
*/
|
||||||
|
public function testEmptyEnvWhichCannotBeEmptyForScalarNodeWithValidation(): void
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
$container->registerExtension($ext = new EnvExtension());
|
||||||
|
$container->prependExtensionConfig('env_extension', $expected = array(
|
||||||
|
'scalar_node_not_empty_validated' => '%env(SOME)%',
|
||||||
|
));
|
||||||
|
|
||||||
|
$this->doProcess($container);
|
||||||
|
|
||||||
|
$this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testPartialEnvWhichCannotBeEmptyForScalarNode(): void
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
$container->registerExtension($ext = new EnvExtension());
|
||||||
|
$container->prependExtensionConfig('env_extension', $expected = array(
|
||||||
|
'scalar_node_not_empty_validated' => 'foo %env(SOME)% bar',
|
||||||
|
));
|
||||||
|
|
||||||
|
$this->doProcess($container);
|
||||||
|
|
||||||
|
$this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
|
||||||
|
}
|
||||||
|
|
||||||
public function testEnvWithVariableNode(): void
|
public function testEnvWithVariableNode(): void
|
||||||
{
|
{
|
||||||
$container = new ContainerBuilder();
|
$container = new ContainerBuilder();
|
||||||
|
@ -282,6 +314,14 @@ class EnvConfiguration implements ConfigurationInterface
|
||||||
->children()
|
->children()
|
||||||
->scalarNode('scalar_node')->end()
|
->scalarNode('scalar_node')->end()
|
||||||
->scalarNode('scalar_node_not_empty')->cannotBeEmpty()->end()
|
->scalarNode('scalar_node_not_empty')->cannotBeEmpty()->end()
|
||||||
|
->scalarNode('scalar_node_not_empty_validated')
|
||||||
|
->cannotBeEmpty()
|
||||||
|
->validate()
|
||||||
|
->always(function ($value) {
|
||||||
|
return $value;
|
||||||
|
})
|
||||||
|
->end()
|
||||||
|
->end()
|
||||||
->integerNode('int_node')->end()
|
->integerNode('int_node')->end()
|
||||||
->floatNode('float_node')->end()
|
->floatNode('float_node')->end()
|
||||||
->booleanNode('bool_node')->end()
|
->booleanNode('bool_node')->end()
|
||||||
|
|
Reference in New Issue