Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[Validator] Something wrong with Constraints\Collection #2615

Closed
Koc opened this Issue · 5 comments

3 participants

@Koc

The trivial (as I thinked before yesterday) problem: there is have some data in array $data = array('exist_field' => '') wich should be validated. If something wrong - return report like: array('field1' => 'error1', 'field2' => 'error2');

Variant one:

<?php
$constraintsCollection = new Collection(array(
    'fields' => array(
        'exist_field' => new NotBlank(),
        'not_exist_field' => new NotBlank(),
    ),
));

$result = $validator->validateValue($data, $constraintsCollection);

What I get

Array.[exist_field]:
    This value should not be blank
Array.:
    The fields "not_exist_field" are missing

so now I should by hand extract "not_exist_field" using $violation->getMessageParameters()['{{ field }}']. Not nice.

Variant two:

<?php

$constraintsCollection = new Collection(array(
    'fields' => array(
        'exist_field' => new NotBlank(),
        'not_exist_field' => new NotBlank(),
    ),
    'allowMissingFields' => true
));

$validator->validateValue($data, $constraintsCollection);

What I get

Array.[exist_field]: This value should not be blank 

Not enought.

Variant three:

<?php

$form = $this->createFormBuilder(null, array(
        'validation_constraint' => $constraintsCollection, // $constraintsCollection from variant one or two - no difference
        'error_bubbling' => true,
    ))
    ->add('exist_field', 'text')
    ->add('not_exist_field', 'text')
    ->getForm();

$form->bind($data);

vd($form->isValid(), $form->getData(), $form->getErrors()); // false, array('exist_field' => null, 'not_exist_field' => null), array()

So form is not valid, but no any message about incorrects fields.

Variant four:

<?php

$form = $this->createFormBuilder(null, array(
        'error_bubbling' => true,
    ))
    ->add('exist_field', 'text', array('validation_constraint' => new NotBlank()))
    ->add('not_exist_field', 'text', array('validation_constraint' => new NotBlank()))
    ->getForm();

$form->bind($data);

vd($form->isValid(), $form->getData(), $form->getErrors()); // true, array('exist_field' => null, 'not_exist_field' => null), array()

Form is valid? o_O

IMHO the easiest way to fix it - add to Collection constraint option replaceMissingFieldsWithNull.

@Koc
Koc commented

@bschussek , @fabpot what do you think about this problem?

@webmozart
Collaborator

@Koc I'm not sure I understand your problem. Can you check whether it is solved by the PR you referenced?

@fabpot fabpot referenced this issue from a commit
@fabpot fabpot merged branch bschussek/collection-validator (PR #3118)
Commits
-------

e6e3da5 [Validator] Improved test coverage of CollectionValidator and reduced test code duplication
509c7bf [Validator] Moved Optional and Required constraints to dedicated sub namespace.
bf59018 [Validator] Removed @api-tag from Optional and Required constraint, since these two are new.
6641f3e [Validator] Added constraints Optional and Required for the CollectionValidator

Discussion
----------

[Validator] Improve support for optional/required fields in Collection constraint

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: none

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=collection-validator)

Improves the `Collection` constraint to test on a more granular level if entries of the collection are optional or required. Before this could only be set using the "allowExtraFields" and "allowMissingFields" options, but these are very general and limited.

The former syntax - without Optional or Required - is still supported.

Usage:

    $array = array(
        'name' => 'Bernhard',
        'birthdate' => '1970-01-01',
    );
    $validator->validate($array, null, new Collection(array(
        'name' => new Required(),
        'birthdate' => new Optional(),
    ));

    // you can also pass additional constraints for the fields
    $validator->validate($array, null, new Collection(array(
        'name' => new Required(array(
            new Type('string'),
            new MinLength(3),
        )),
        'birthdate' => new Optional(new Date()),
    ));

---------------------------------------------------------------------------

by canni at 2012-01-15T20:22:17Z

@bschussek I've rewritten a lot of test code for Collection validator in 2.0 branch and also had modified validator itself, as it had a bug #3078, consider waiting with this PR till fabpot will merge 2.0 back into master, as there will be code conflicts :)

---------------------------------------------------------------------------

by Koc at 2012-01-15T23:13:04Z

Does it helps to #2615 ?

---------------------------------------------------------------------------

by fabpot at 2012-01-16T06:44:53Z

@canni: I've just merged 2.0 into master.

---------------------------------------------------------------------------

by bschussek at 2012-01-16T12:05:19Z

@fabpot: Rebased. I also fixed the CS issues mentioned by @stof.
e056480
@Koc

I've tryed on master brunch but PR does not solved this issue (.

The prolem: should be easy way validate data and report errors using custom format like array('field1' => 'this is required field', 'field2' => 'this value should be >= 10')

// what I do

  $data = array('exist_field' => '');

  $constraintsCollection = new Collection(array(
    'fields' => array(
       'exist_field' => new Required(new NotBlank()),
       'not_exist_field' => new Required(new NotBlank()),
    ),
  ));

  $result = $validator->validateValue($data, $constraintsCollection);

  var_dump($result);
  exit;

but property path for not exist field is empty (protected 'propertyPath' => string '')

@webmozart
Collaborator

I still don't understand. Can you write a failing test case please?

@Koc
  <?php
  $data = array('exist_field' => '');

  $constraintsCollection = new Collection(array(
    'fields' => array(
       'exist_field' => new Required(new NotBlank()),
       'not_exist_field' => new Required(new NotBlank()),
    ),
  ));

  $violations = $validator->validateValue($data, $constraintsCollection);

  assertEquals('[not_exist_field]', $violations[1]->getPropertyPath());
@fabpot fabpot referenced this issue from a commit
@fabpot fabpot merged branch bschussek/issue2615 (PR #3228)
Commits
-------

2e4ebe4 [Validator] Renamed methods addViolationAtRelativePath() and getAbsolutePropertyPath() in ExecutionContext
9153f0e [Validator] Deprecated ConstraintValidator methods setMessage(), getMessageTemplate() and getMessageParameters()
0417282 [Validator] Fixed typos
a30a679 [Validator] Made ExecutionContext immutable and introduced new class GlobalExecutionContext
fe85bbd [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt()
f77fd41 [Form] Fixed typos
1fc615c Fixed string access by curly brace to bracket
a103c28 [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now
f904a9e [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty
1dd302c [Validator] Fixed ConstraintViolationList::__toString() to not include dots in the output if the root is empty
1678a3d [Validator] Fixed: Validator::validateValue() propagates empty validation root instead of the provided value

Discussion
----------

[Validator] Improved "missing" and "extra" errors of Collection constraint

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2615
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2615)

Instead of a single violation

    Array:
        The fields "foo", "bar" are missing

various violations are now generated.

    Array[foo]:
        This field is missing
    Array[bar]:
        This field is missing

Apart from that, the PR contains various minor fixes to the Validator.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T09:14:52Z

@fabpot Ready for merge.
8245bf1
@fabpot fabpot closed this
@mmucklo mmucklo referenced this issue from a commit
@fabpot fabpot merged branch bschussek/collection-validator (PR #3118)
Commits
-------

e6e3da5 [Validator] Improved test coverage of CollectionValidator and reduced test code duplication
509c7bf [Validator] Moved Optional and Required constraints to dedicated sub namespace.
bf59018 [Validator] Removed @api-tag from Optional and Required constraint, since these two are new.
6641f3e [Validator] Added constraints Optional and Required for the CollectionValidator

Discussion
----------

[Validator] Improve support for optional/required fields in Collection constraint

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: none
Todo: none

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=collection-validator)

Improves the `Collection` constraint to test on a more granular level if entries of the collection are optional or required. Before this could only be set using the "allowExtraFields" and "allowMissingFields" options, but these are very general and limited.

The former syntax - without Optional or Required - is still supported.

Usage:

    $array = array(
        'name' => 'Bernhard',
        'birthdate' => '1970-01-01',
    );
    $validator->validate($array, null, new Collection(array(
        'name' => new Required(),
        'birthdate' => new Optional(),
    ));

    // you can also pass additional constraints for the fields
    $validator->validate($array, null, new Collection(array(
        'name' => new Required(array(
            new Type('string'),
            new MinLength(3),
        )),
        'birthdate' => new Optional(new Date()),
    ));

---------------------------------------------------------------------------

by canni at 2012-01-15T20:22:17Z

@bschussek I've rewritten a lot of test code for Collection validator in 2.0 branch and also had modified validator itself, as it had a bug #3078, consider waiting with this PR till fabpot will merge 2.0 back into master, as there will be code conflicts :)

---------------------------------------------------------------------------

by Koc at 2012-01-15T23:13:04Z

Does it helps to #2615 ?

---------------------------------------------------------------------------

by fabpot at 2012-01-16T06:44:53Z

@canni: I've just merged 2.0 into master.

---------------------------------------------------------------------------

by bschussek at 2012-01-16T12:05:19Z

@fabpot: Rebased. I also fixed the CS issues mentioned by @stof.
7a5e02f
@mmucklo mmucklo referenced this issue from a commit
@fabpot fabpot merged branch bschussek/issue2615 (PR #3228)
Commits
-------

2e4ebe4 [Validator] Renamed methods addViolationAtRelativePath() and getAbsolutePropertyPath() in ExecutionContext
9153f0e [Validator] Deprecated ConstraintValidator methods setMessage(), getMessageTemplate() and getMessageParameters()
0417282 [Validator] Fixed typos
a30a679 [Validator] Made ExecutionContext immutable and introduced new class GlobalExecutionContext
fe85bbd [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt()
f77fd41 [Form] Fixed typos
1fc615c Fixed string access by curly brace to bracket
a103c28 [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now
f904a9e [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty
1dd302c [Validator] Fixed ConstraintViolationList::__toString() to not include dots in the output if the root is empty
1678a3d [Validator] Fixed: Validator::validateValue() propagates empty validation root instead of the provided value

Discussion
----------

[Validator] Improved "missing" and "extra" errors of Collection constraint

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2615
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2615)

Instead of a single violation

    Array:
        The fields "foo", "bar" are missing

various violations are now generated.

    Array[foo]:
        This field is missing
    Array[bar]:
        This field is missing

Apart from that, the PR contains various minor fixes to the Validator.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T09:14:52Z

@fabpot Ready for merge.
be13be2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.