8000 [Validator] Fixed group handling in composite constraints by webmozart · Pull Request #11505 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] Fixed group handling in composite constraints #11505

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

Merged
merged 1 commit into from
Aug 5, 2014
Merged

[Validator] Fixed group handling in composite constraints #11505

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
[Validator] Fixed group handling in composite constraints
  • Loading branch information
@webmozart
webmozart committed Aug 5, 2014
commit f6c070d073ab1a76435a0595a0749bc3fa48e925
83 changes: 72 additions & 11 deletions src/Symfony/Component/Validator/Constraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
*
* Constraint instances are immutable and serializable.
*
* @property array $groups The groups that the constraint belongs to
*
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @api
Expand All @@ -48,11 +50,6 @@ abstract class Constraint
*/
const PROPERTY_CONSTRAINT = 'property';

/**
* @var array
*/
public $groups = array(self::DEFAULT_GROUP);

/**
* Initializes the constraint with options.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this was changed. Makes it a little inconsistent between groups option and all the rest which are real properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we need a way to determine whether the $groups property was set (in that case it's not overridden by Composite constraints) or not (in that case the Composite constraint sets it). The alternative would be to store something like $providedOptions, but that was even uglier:

  • If we store $providedOptions in the serialized form of the constraints, the size of the serialized data grows unnecessarily.
  • If we don't, then the condition $constraint == unserialize(serialize($constraint)) doesn't hold anymore, which breaks many tests.

So I went with this solution.

*
Expand Down Expand Up @@ -86,6 +83,10 @@ public function __construct($options = null)
{
$invalidOptions = array();
$missingOptions = array_flip((array) $this->getRequiredOptions());
$knownOptions = get_object_vars($this);

// The "groups" option is added to the object lazily
$knownOptions['groups'] = true;

if (is_array($options) && count($options) >= 1 && isset($options['value']) && !property_exists($this, 'value')) {
$options[$this->getDefaultOption()] = $options['value'];
Expand All @@ -94,14 +95,14 @@ public function __construct($options = null)

if (is_array($options) && count($options) > 0 && is_string(key($options))) {
foreach ($options as $option => $value) {
if (property_exists($this, $option)) {
if (array_key_exists($option, $knownOptions)) {
$this->$option = $value;
unset($missingOptions[$option]);
} else {
$invalidOptions[] = $option;
}
}
} elseif (null !== $options && ! (is_array($options) && count($options) === 0)) {
} elseif (null !== $options && !(is_array($options) && count($options) === 0)) {
$option = $this->getDefaultOption();

if (null === $option) {
Expand All @@ -110,7 +111,7 @@ public function __construct($options = null)
);
}

if (property_exists($this, $option)) {
if (array_key_exists($option, $knownOptions)) {
$this->$option = $options;
unset($missingOptions[$option]);
} else {
Expand All @@ -131,15 +132,56 @@ public function __construct($options = null)
array_keys($missingOptions)
);
}

$this->groups = (array) $this->groups;
}

/**
* Unsupported operation.
* Sets the value of a lazily initialized option.
*
* Corresponding properties are added to the object on first access. Hence
* this method will be called at most once per constraint instance and
* option name.
*
* @param string $option The option name
* @param mixed $value The value to set
*
* @throws InvalidOptionsException If an invalid option name is given
*/
public function __set($option, $value)
{
if ('groups' === $option) {
$this->groups = (array) $value;

return;
}

throw new InvalidOptionsException(sprintf('The option "%s" does not exist in constraint %s', $option, get_class($this)), array($option));
}

/**
* Returns the value of a lazily initialized option.
*
* Corresponding properties are added to the object on first access. Hence
* this method will be called at most once per constraint instance and
* option name.
*
* @param string $option The option name
*
* @return mixed The value of the option
*
* @throws InvalidOptionsException If an invalid option name is given
*
* @internal This method should not be used or overwritten in userland code.
*
* @since 2.6
*/
public function __get($option)
{
if ('groups' === $option) {
$this->groups = array(self::DEFAULT_GROUP);

return $this->groups;
}

throw new InvalidOptionsException(sprintf('The option "%s" does not exist in constraint %s', $option, get_class($this)), array($option));
}

Expand Down Expand Up @@ -217,4 +259,23 @@ public function getTargets()
{
return self::PROPERTY_CONSTRAINT;
}

/**
* Optimizes the serialized value to minimize storage space.
*
* @return array The properties to serialize
*
* @internal This method may be replaced by an implementation of
* {@link \Serializable} in the future. Please don't use or
* overwrite it.
*
* @since 2.6
*/
public function __sleep()
{
// Initialize "groups" option if it is not set
$this->__get('groups');

return array_keys(get_object_vars($this));
}
}
32 changes: 6 additions & 26 deletions src/Symfony/Component/Validator/Constraints/All.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@

namespace Symfony\Component\Validator\Constraints;

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Exception\ConstraintDefinitionException;

/**
* @Annotation
* @Target({"PROPERTY", "METHOD", "ANNOTATION"})
Expand All @@ -22,32 +19,10 @@
*
* @api
*/
class All extends Constraint
class All extends Composite
{
public $constraints = array();

/**
* {@inheritdoc}
*/
public function __construct($options = null)
{
parent::__construct($options);

if (!is_array($this->constraints)) {
$this->constraints = array($this->constraints);
}

foreach ($this->constraints as $constraint) {
if (!$constraint instanceof Constraint) {
throw new ConstraintDefinitionException(sprintf('The value %s is not an instance of Constraint in constraint %s', $constraint, __CLASS__));
}

if ($constraint instanceof Valid) {
throw new ConstraintDefinitionException(sprintf('The constraint Valid cannot be nested inside constraint %s. You can only declare the Valid constraint directly on a field or method.', __CLASS__));
}
}
}

public function getDefaultOption()
{
return 'constraints';
Expand All @@ -57,4 +32,9 @@ public function getRequiredOptions()
{
return array('constraints');
}

protected function getCompositeOption()
{
return 'constraints';
}
}
3 changes: 1 addition & 2 deletions src/Symfony/Component/Validator/Constraints/AllValidator.php
8000 View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,10 @@ public function validate($value, Constraint $constraint)
}

$context = $this->context;
$group = $context->getGroup();
$validator = $context->getValidator()->inContext($context);

foreach ($value as $key => $element) {
$validator->atPath('['.$key.']')->validate($element, $constraint->constraints, $group);
$validator->atPath('['.$key.']')->validate($element, $constraint->constraints);
}
}
}
29 changes: 14 additions & 15 deletions src/Symfony/Component/Validator/Constraints/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
* @api
*/
class Collection extends Constraint
class Collection extends Composite
{
public $fields = array();
public $allowExtraFields = false;
Expand All @@ -42,6 +42,14 @@ public function __construct($options = null)
}

parent::__construct($options);
}

/**
* {@inheritdoc}
*/
protected function initializeNestedConstraints()
{
parent::initializeNestedConstraints();

if (!is_array($this->fields)) {
throw new ConstraintDefinitionException(sprintf('The option "fields" is expected to be an array in constraint %s', __CLASS__));
Expand All @@ -57,25 +65,16 @@ public function __construct($options = null)
if (!$field instanceof Optional && !$field instanceof Required) {
$this->fields[$fieldName] = $field = new Required($field);
}

if (!is_array($field->constraints)) {
$field->constraints = array($field->constraints);
}

foreach ($field->constraints as $constraint) {
if (!$constraint instanceof Constraint) {
throw new ConstraintDefinitionException(sprintf('The value %s of the field %s is not an instance of Constraint in constraint %s', $constraint, $fieldName, __CLASS__));
}

if ($constraint instanceof Valid) {
throw new ConstraintDefinitionException(sprintf('The constraint Valid cannot be nested inside constraint %s. You can only declare the Valid constraint directly on a field or method.', __CLASS__));
}
}
}
}

public function getRequiredOptions()
{
return array('fields');
}

protected function getCompositeOption()
{
return 'fields';
}
}
3 changes: 1 addition & 2 deletions src/Symfony/Component/Validator/Constraints/CollectionValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public function validate($value, Constraint $constraint)
// remove the initialize() method and pass the context as last argument
// to validate() instead.
$context = $this->context;
$group = $context->getGroup();
$validator = $context->getValidator()->inContext($context);

foreach ($constraint->fields as $field => $fieldConstraint) {
Expand All @@ -60,7 +59,7 @@ public function validate($value, Constraint $constraint)
if ($existsInArray || $existsInArrayAccess) {
if (count($fieldConstraint->constraints) > 0) {
$validator->atPath('['.$field.']')
->validate($value[$field], $fieldConstraint->constraints, $group);
->validate($value[$field], $fieldConstraint->constraints);
}
} elseif (!$fieldConstraint instanceof Optional && !$constraint->allowMissingFields) {
$context->buildViolation($constraint->missingFieldsMessage)
Expand Down
Loading
0