Skip to content
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.

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

[Validator] Incorrect documentation #6159

Closed
aybbou opened this issue Jan 18, 2016 · 6 comments
Closed

[Validator] Incorrect documentation #6159

aybbou opened this issue Jan 18, 2016 · 6 comments

Comments

@aybbou
Copy link
Contributor

aybbou commented Jan 18, 2016

Hi,

So I was looking at the documentation for validating objects, and I read here that :

If you have inheritance (e.g. User extends BaseUser) and you validate with the class name of the subclass (i.e. User), then all constraints in the User and BaseUser will be validated. However, if you validate using the base class (i.e. BaseUser), then only the default constraints in the BaseUser class will be validated.

But when I tested this, it wasn't working :

class BaseUser
{
    /**
     * @Assert\NotBlank()
     */
    public $name;
}

and

class User extends BaseUser
{
    /**
     * @Assert\NotBlank()
     */
    public $email;
}

and then in the controller I do something like :

    $user = new User();
    $errors = $this->get('validator')->validate($user, null, array('User'));
    dump($errors);

I have a ConstraintViolationList object containing only the $email violation. So I was wondering if this is a bug in the Validator component or the documentation is not up to date.

I am using Symfony 3.0.1 & PHP 5.6

Thanks.

@wouterj
Copy link
Member

wouterj commented Jan 20, 2016

I can confirm the behaviour you described.

The JSR 303 specs (which the Validator component implements) describes the documented behaviour: http://beanvalidation.org/1.0/spec/#d0e1425 So it seems like the code should be changed to reflect this.

@wouterj
Copy link
Member

wouterj commented Feb 6, 2016

Actually, I misread the specs and the docs should be fixed instead:

For every class X:

A. For each superclass Y of X, the group Y contains all constraints of the group Y of Y
this rule prepares formal concepts for recursive discovery

So we should fix the docs instead. Can you maybe do that by clicking on the "edit" button of this particular page? (if you aren't sure or don't have time, feel free to say so and someone else will have a go on this).

@wouterj wouterj added bug actionable Clear and specific issues ready for anyone to take them. Validator labels Feb 6, 2016
@aybbou
Copy link
Contributor Author

aybbou commented Feb 9, 2016

Thank you for response @wouterj, but I'm not sure that I fully understand what to do here.

In my previous example, should the dump($errors) only show the $email error ?

Also, I think that the B.III should be applied here:

B. The group X contains the following constraints:
III. if X has a direct superclass Y, every constraint in the group Y
all Default constraints hosted on the superclasses of X: constraints are inherited by the class hierarchy

So I do think that the documentation is correct and the code should be changed.

/cc @webmozart

@wouterj
Copy link
Member

wouterj commented May 21, 2016

Hmm, I think you're correct @aybbou. I think I confused "superclass" with "subclass".

Can you please open an issue in the code repository? (or even fix it in the code?)

@wouterj wouterj removed the actionable Clear and specific issues ready for anyone to take them. label May 21, 2016
@mickaelandrieu
Copy link
Contributor

@wouterj to me sounds good regarding the explanation about Sequence group in docs http://symfony.com/doc/current/book/validation.html#group-sequence

nldr: if you set User as third argument, only validations from this class will be used. The best here is to not fill the third argument, isn't it?

Mickaël

@javiereguiluz
Copy link
Member

I'm closing this because the conclusion was that docs are correct and this is a bug in the code. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants