Skip to content

[Config] Renamings to allow all callback types #14364

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

[Config] Renamings to allow all callback types #14364

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
12 changes: 11 additions & 1 deletion UPGRADE-2.8.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
UPGRADE FROM 2.7 to 2.8
UPGRADE FROM 2.7 to 2.8
=======================

Form
Expand Down Expand Up @@ -136,3 +136,13 @@ DependencyInjection
<service id="foo" class="stdClass" shared="false" />
</services>
```

Config
------

* The methods `setNormalizationClosures()` and `setFinalValidationClosures()` in
`BaseNode` were deprecated, `setNormalizationCallbacks()` and
`setFinalValidationCallbacks()` should be used instead.

* The protected properties `normalizationClosures` and `finalValidationClosures` in
`BaseNode` were renamed to `normalizationCallbacks` and `finalValidationCallbacks`.
9 changes: 9 additions & 0 deletions src/Symfony/Component/Config/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
CHANGELOG
=========

2.8.0
-----

* [DEPRECATION] The protected properties `normalizationClosures` and `finalValidationClosures` in
`BaseNode` were renamed to `normalizationCallbacks` and `finalValidationCallbacks`.
* [DEPRECATION] The methods `setNormalizationClosures()` and `setFinalValidationClosures()` in
`BaseNode` were deprecated, `setNormalizationCallbacks()` and `setFinalValidationCallbacks()`
should be used instead.

2.7.0
-----

Expand Down
73 changes: 62 additions & 11 deletions src/Symfony/Component/Config/Definition/BaseNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,15 @@ abstract class BaseNode implements NodeInterface
{
protected $name;
protected $parent;
protected $normalizationCallbacks = array();
/**
* @deprecated since version 2.8, to be removed in 3.0. Use the normalizationCallbacks property instead.
*/
protected $normalizationClosures = array();
protected $finalValidationCallbacks = array();
Copy link
Member

Choose a reason for hiding this comment

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

Aliasing the old ones by reference would allow preserving BC.
you should do it in the constructor, and for completeness also add a magic __clone method:

public function __construct(...)
{
    $this->normalizationClosures = &$this->normalizationCallbacks;
    $this->finalValidationClosures = &$this->finalValidationCallbacks;

    // ...
}
// ...
public function __clone()
{
    // Break cross-clones references but preserve them inside each one.
    unset($this->normalizationCallbacks, $this->finalValidationCallbacks);
    $this->normalizationCallbacks = $this->normalizationClosures;
    $this->finalValidationCallbacks = $this->finalValidationClosures;
    $this->normalizationClosures = &$this->normalizationCallbacks;
    $this->finalValidationClosures = &$this->finalValidationCallbacks;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Referenced aliases added.

Copy link
Member

Choose a reason for hiding this comment

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

You forgot to keep the declaration above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works without, but of course it makes sense to keep the old property declarations as well. Restored now, with deprecation notes.

/**
* @deprecated since version 2.8, to be removed in 3.0. Use the finalValidationCallbacks property instead.
*/
protected $finalValidationClosures = array();
protected $allowOverwrite = true;
protected $required = false;
Expand All @@ -48,6 +56,10 @@ public function __construct($name, NodeInterface $parent = null)

$this->name = $name;
$this->parent = $parent;

// Backwards compatibility for old property names, to be removed in 3.0
$this->normalizationClosures = &$this->normalizationCallbacks;
$this->finalValidationClosures = &$this->finalValidationCallbacks;
Copy link
Member

Choose a reason for hiding this comment

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

Would it be an option to remove the normalizationCallbacks and finalValidationCallbacks properties and implementing __get() and __set() for them instead? This way we could also trigger deprecation notices when the deprecated properties are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, using magic methods would allow allow deprecation notices and also somewhat simpler implementation (no need for __clone() tricks). It would also make the properties effectively public, which could be a problem. In addition the new properties could also probably be private, unless there's some specific reason the old ones were protected.

All of these changes look like bikeshedding to me though. I checked with Packanalyst earlier and found just one class that would need to be changed for 3.0 compatibility -- and that one uses the setter methods. A quick search is not going to find every use case, but I think using the properties directly is likely rare even in custom subclasses.

Of course, I can still make the changes if there's agreement that using __get() and __set() would be preferred.

}

public function setAttribute($key, $value)
Expand Down Expand Up @@ -151,24 +163,52 @@ public function setAllowOverwrite($allow)
$this->allowOverwrite = (bool) $allow;
}

/**
* Sets the callbacks used for normalization.
*
* @param callable[] $callbacks An array of callbacks used for normalization
*/
public function setNormalizationCallbacks(array $callbacks)
{
$this->normalizationCallbacks = $callbacks;
}

/**
* Sets the closures used for normalization.
*
* @param \Closure[] $closures An array of Closures used for normalization
*
* @deprecated since version 2.8, to be removed in 3.0. Use setNormalizationCallbacks() instead
*/
public function setNormalizationClosures(array $closures)
{
$this->normalizationClosures = $closures;
@trigger_error('The '.__METHOD__.' method is deprecated since version 2.8 and will be removed in 3.0. Use the setNormalizationCallbacks() method instead.', E_USER_DEPRECATED);

$this->normalizationCallbacks = $closures;
}

/**
* Sets the callbacks used for final validation.
*
* @param callable[] $callbacks An array of callbacks used for final validation
*/
public function setFinalValidationCallbacks(array $callbacks)
{
$this->finalValidationCallbacks = $callbacks;
}

/**
* Sets the closures used for final validation.
*
* @param \Closure[] $closures An array of Closures used for final validation
*
* @deprecated since version 2.8, to be removed in 3.0. Use setFinalValidationCallbacks() instead
*/
public function setFinalValidationClosures(array $closures)
{
$this->finalValidationClosures = $closures;
@trigger_error('The '.__METHOD__.' method is deprecated since version 2.8 and will be removed in 3.0. Use the setFinalValidationCallbacks() method instead.', E_USER_DEPRECATED);

$this->finalValidationCallbacks = $closures;
}

/**
Expand Down Expand Up @@ -235,7 +275,7 @@ final public function merge($leftSide, $rightSide)
}

/**
* Normalizes a value, applying all normalization closures.
* Normalizes a value, applying all normalization callbacks.
*
* @param mixed $value Value to normalize.
*
Expand All @@ -245,9 +285,9 @@ final public function normalize($value)
{
$value = $this->preNormalize($value);

// run custom normalization closures
foreach ($this->normalizationClosures as $closure) {
$value = $closure($value);
// run custom normalization callbacks
foreach ($this->normalizationCallbacks as $callback) {
$value = call_user_func($callback, $value);
}

// replace value with their equivalent
Expand Down Expand Up @@ -287,7 +327,7 @@ public function getParent()
}

/**
* Finalizes a value, applying all finalization closures.
* Finalizes a value, applying all finalization callbacks.
*
* @param mixed $value The value to finalize
*
Expand All @@ -302,11 +342,11 @@ final public function finalize($value)

$value = $this->finalizeValue($value);

// Perform validation on the final value if a closure has been set.
// The closure is also allowed to return another value.
foreach ($this->finalValidationClosures as $closure) {
// Perform validation on the final value if a callback has been set.
// The callback is also allowed to return another value.
foreach ($this->finalValidationCallbacks as $callback) {
try {
$value = $closure($value);
$value = call_user_func($callback, $value);
} catch (Exception $e) {
throw $e;
} catch (\Exception $e) {
Expand All @@ -317,6 +357,17 @@ final public function finalize($value)
return $value;
}

public function __clone()
{
// Backwards compatibility for old property names, to be removed in 3.0
// Break cross-clones references but preserve them inside each one.
unset($this->normalizationCallbacks, $this->finalValidationCallbacks);
$this->normalizationCallbacks = $this->normalizationClosures;
$this->finalValidationCallbacks = $this->finalValidationClosures;
$this->normalizationClosures = &$this->normalizationCallbacks;
$this->finalValidationClosures = &$this->finalValidationCallbacks;
}

/**
* Validates the type of a Node.
*
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ protected function createNode()
$node->setNormalizeKeys($this->normalizeKeys);

if (null !== $this->normalization) {
$node->setNormalizationClosures($this->normalization->before);
$node->setNormalizationCallbacks($this->normalization->before);
$node->setXmlRemappings($this->normalization->remappings);
}

Expand All @@ -411,7 +411,7 @@ protected function createNode()
}

if (null !== $this->validation) {
$node->setFinalValidationClosures($this->validation->rules);
$node->setFinalValidationCallbacks($this->validation->rules);
}

return $node;
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Config/Definition/Builder/VariableNodeDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ protected function createNode()
$node = $this->instantiateNode();

if (null !== $this->normalization) {
$node->setNormalizationClosures($this->normalization->before);
$node->setNormalizationCallbacks($this->normalization->before);
}

if (null !== $this->merge) {
Expand All @@ -56,7 +56,7 @@ protected function createNode()
$node->setRequired($this->required);

if (null !== $this->validation) {
$node->setFinalValidationClosures($this->validation->rules);
$node->setFinalValidationCallbacks($this->validation->rules);
}

return $node;
Expand Down