Why are validation shown against value rather than field name?

[silentworks]

Currently If I try to validate $_POST['username'] I get the value in the validation message rather than the name username?

try {
    v::alnum()
        ->noWhitespace()
        ->length(4,22)
        ->check($_POST['username']);
} catch (\InvalidArgumentException $e) {
    var_dump($e->getMainMessage());
}

result

"" must not contain whitespace

what I was expecting

"username" must not contain whitespace

[augustohp]

Actually, what you see is the value that failed validation. You can use the method setName() in a Validator to set its name, it will be like that:

try {
    v::alnum()
        ->noWhitespace()
        ->length(4,22)
        ->setName('username')
        ->check($_POST['username']);
} catch (\InvalidArgumentException $e) {
    var_dump($e->getMainMessage());
}

[silentworks]

I thought that would have done the trick but it actually does nothing for me, message remains the same.

[drgomesp]

+1.

I do agree, though, that we shouldn't have to manually set the name for the specific validator.

[silentworks]

I have tried this on both the develop branch and the 0.4.* version.

[augustohp]

Sorry about that, now I made sure to check the code before posting for a solution.

try {
    $username = '';
    v::alnum()
     ->noWhitespace()
     ->length(4,22)
     ->setName('username')
     ->assert($username);
} catch (\InvalidArgumentException $e) {
    print_r($e->getFullMessage());
    echo PHP_EOL;
}

You have to use the method getFullMessage in the Exception. This method is only available when the validation is done using assert as it can be seen on the documentation.

Let me know if this works like you want.
We will try to make this more clear into the README.md.

@drgomesp How do you propose we get a name of a validator? I don't have many ideas on this. =(

[silentworks]

@augustohp This still yeild the same as before except that I now have the results in a list.

result

string '\-These rules must pass for username
  |-"" must not contain whitespace
  \-"" must have a length between 4 and 22' (length=116)

I was expecting this

string '\-These rules must pass for username
  |-"username" must not contain whitespace
  \-"username" must have a length between 4 and 22' (length=116)

[drgomesp]

@augustohp I would just dump the value being validated at the time.

For me, it makes a lot more sense than manually setting a name for it.

But I'm not sure this would be the best approach, though.

try {
    $username = '';
    v::alnum()
     ->noWhitespace()
     ->length(4,22)
     ->assert('Daniel Ribeiro');
} catch (\InvalidArgumentException $e) {
    print_r($e->getFullMessage());
    echo PHP_EOL;
}

The expected result would be:

"Daniel Ribeiro" must not contain whitespace

[silentworks]

Guys this make no sense to me at all, why would I want my message to say my value must not contain whitespace rather than the field that submitted the value.

How would you associate the validation to a field in a form then?

If I am validating the username, I want to tell the user that Username field much not contain whitespace not the value of the username field.

[drgomesp]

@silentworks there is really no magic on this, mate.

Take this as an example:

try {
    $username = "Daniel Ribeiro";
    v::alnum()
     ->noWhitespace()
     ->length(4,22)
     ->assert($username);
} catch (\InvalidArgumentException $e) {
    print_r($e->getFullMessage());
    echo PHP_EOL;
}

Unless you explicity tell the validator, like @augustohp said, there is no way it is going to know what is being treated.

That's why I suggested the value as the return that makes more sense.

[silentworks]

@drgomesp I think you are still missing what I am saying. I don't want the validation to return a value in the message string, I want it to return the field name. I am using setName but it still doesn't use the name in the validation message beside the line that say "This rule must pass for "

result

string '\-These rules must pass for username
  |-"" must not contain whitespace
  \-"" must have a length between 4 and 22' (length=116)

what I expect

string '\-These rules must pass for username
  |-"username" must not contain whitespace
  \-"username" must have a length between 4 and 22' (length=116)

[drgomesp]

@silentworks, ok, I did missunderstood.

Did you try using getFullMessage() instead of getMainMessage()?

[silentworks]

@drgomesp I tried both, all give same result, this seem to have not been considered when the library was written or if it was the docs doesn't say how to do it.

[alganet]

Looks broken indeed. Had similar problems with setName() and check() not reporting the proper identifiers for exceptions. The identifier is a value that the validator flow identifies as the best human representation for a given validation constrain (in the key() case, should be the key name as @silentworks expected).

I'll take a look at this as soon as possible and report over here. If anyone's interested in fixing this probably the problem is in the ValidationException class or one of its derivates.

[wesleyvicthor]

Actually the problem is related with AbstractComposite.

The setName() method just sets the name of a group of rules, thus this group have a name:

v::allOf(v::alnum()->noWhitespace())->setName('POST data')->assert($_POST['username']);

When we try to set the name of a specific rule doesn't work because, the interface does not provide the access to the current rule.
That's occurs because the Validator doesn't return the added rule. It returns a self instance, so: Validator < AllOf < AbstractComposite < AbstractRule.

To fix this I propose to add the name parameter in the constructor of AbstractRule. The interface will looks like:

v::alnum()
    ->noWhitespace('username')
    ->length(4,22)
    ->check($_POST['username']);

@alganet @augustohp guys ?

[drgomesp]

It's a good proposal, @wesleyvicthor, but it doesn't look very semantic to me.

Still the problem should be resolved.

[silentworks]

@wesleyvicthor this would mean I would have to add a name to all my rules, which removes the whole DRYness of Respect\Validation. As @drgomesp said this doesn't fix the issue.

I am currently looking into the issue and will do a pull request once I nail it.

Can someone remove the FAQ label and label this as a Bug please.

[alganet]

@silentworks thank you very much!

I've also took a better look at the sample and thought on using something like this:

try {
    v::key(
        'username', 
        v::alnum()
         ->noWhitespace()
         ->length(4,22)
         ->check($_POST));
} catch (\InvalidArgumentException $e) {
    var_dump($e->getMainMessage());
}

Could you please test this?

@wesleyvicthor actually, the allOf validator chain can handle nested validators. That's exactly what getFullMessage() does. Check the AbstractNestedException and AbstractGroupedException, their interaction with the ExceptionIterator can ommit redundant and overnested validators (ex: v::allOf(v::allOf(v::allOf(v::bool())))) and don't report validation messages for irrelevant conteiners. Unfortunately, check() doesn't iterate over exceptions and some goodies added there aren't being applied outside this case.

[silentworks]

@augustohp I have tested and I now get

"Array" must contain only letters...

Have you tested it? Are there any particular requirements for Respect\Validation? Any PHP extension that should be on or any settings?

[silentworks]

Ok so it seems this work differently from how the documentation describes it and different to all the examples given. I have found a solution in doing this:

try {
    v::alnum()
        ->noWhitespace()
        ->length(4,22)
        ->check($_POST['username']);
} catch (\InvalidArgumentException $e) {
    var_dump($e->setName('Username')->getMainMessage());
}

However this becomes a problem when you want to validate 2 or more fields, since you can have only 1 catch. So if I wanted to validate a username and password.

try {
    v::alnum()
        ->noWhitespace()
        ->length(4,22)
        ->check($_POST['username']);

    v::alnum()
        ->noWhitespace()
        ->length(5,26)
        ->check($_POST['password']);
} catch (\InvalidArgumentException $e) {
    var_dump($e->setName('Username')->getMainMessage());
}

There is no way for me to setName for each separately.

[nickl-]

Wow busy busy busy =) awesomeness!

[silentworks]

I am close to a solution I think, the current issue is around each rule resetting $this->name in the AbstractRule class so by the time the getName_ method is called $this->name is blank. Its not persisting. I am thinking of calling the __ ValidationException__ setName method inside of the AbstractRule setName method, but I will have to modify the ValidationException class in order to call existing instance rather than a new one each time.

I hope all this make sense, if not I will make the change and send a pull request.

[wesleyvicthor]

@silentworks it is not directly related with setName or getName. I explained it above. You are not setting the name to the right object.

If you want to test groups, you can use AllOf. With your example, the AllOf method should work.

I know it is not the better approach but until we fix these bugs, the example below can fix your problem.

try {
    v::allOf(
         v::attribute('username', v::string()->length(4,22)->noWhitespace())->setName('username'),
         v::attribute('password', v::string()->length(5,26)->noWhitespace())->setName('password')
    )->setName('User Validation')->assert($_POST);
} catch (\InvalidArgumentException $e) {
    var_dump($e->getFullMessage());
}

I think we have some design issues. @alganet @augustohp

[silentworks]

@wesleyvicthor your solution is not a fix as this still leaves the user in the blind as to which rules are failing.

The result I get is this:

These rules must pass for User Validation |
- These rules must pass for user name
- These rules must pass for password

There are no indication as to what these rules are.

[silentworks]

No update on this one, its would seem even though it is a critical bug, its not being addressed.

[alganet]

Hey @silentworks, sorry for the delay! Have you tried my solution?

Also, @wesleyvicthor solution has a little problem, it should be v::key (for array keys) and not v::attribute (for object attributes). Maybe changing that could make it run properly and report nested exceptions better.

[silentworks]

I have tried all solutions and none work. If possible can someone provide me with a working sample code that will output messages based upon field name.

I have decided in the short term to next two try and catch statements to accomplish what I am trying to do at the moment.

[wesleyvicthor]

try {
        v::allOf(
            v::key('email', v::email()),
            v::key('passw', v::notEmpty()->noWhitespace()->length(4, 8))
        )->assert($_POST);

} catch (\InvalidArgumentException $e) {
    $errors = $e->findMessages(
        array(
            'email' => 'email error',
            'passw' => 'password error',
        )
    );
}

have you tried this ?

[alganet]

I can confirm this is working:

            v::key('username', v::length(1,3))
                     ->key('birthdate', v::date())
                     ->setName("User Subscription Form")
                     ->assert(array('username' => 'xxxxx', 'birthdate' => 'x'));

The message output with the appropriate array keys is is this:

\-These rules must pass for User Subscription Form
  |-Key username must be valid
  | \-These rules must pass for "xxxxx"
  |   \-"xxxxx" must have a length between 1 and 3
  \-Key birthdate must be valid
    \-These rules must pass for "x"
      \-"x" must be a valid date

I'm already fixed it to this:

-These rules must pass for User Subscription Form
  |-Key username must be valid
  | \-"xxxxx" must have a length between 1 and 3
  \-Key birthdate must be valid
    \-"x" must be a valid date

This fix will be pushed today as soon as I exaustively test it!

[alganet]

This Push should fix the messages to display like the latest in the comment above! There is even a test there with that code. We'll release this in a package soon, but feel free to test the develop branch and tell if it worked!

[nickl-]

Yeah!

[FoxxMD]

👍

[asperon]

Anybody looking into this?

[henriquemoody]

Looks like we still haven't changed it yet, even with the Key rule.

This code:

try {
    v::key('username', v::alnum()->noWhitespace()->length(4,22))->check($_POST);
} catch (ValidationExceptionInterface $exception) {
    echo $exception->getMainMessage().PHP_EOL;
}

Displays this message:

"a b" must not contain whitespace

But what you was expecting was:

"username" must not contain whitespace

I'm not sure if we can even fix this on the current version because even if it's not the expected behaviour by many of you I don't see this as a Bug, it's just a different behaviour and there are test counting with this behaviour.

However, this is going to change on our first major version, as you can see on this test: simple-full-message-with-key-4. So, you may consider this as covered.

[henriquemoody]

Any way, this looks easy to change, see #365.

I'm just not sure if we should release this on 0.9<, 0.10 or just on 1.0. Any thoughts?

[augustohp]

I'm just not sure if we should release this on 0.9<, 0.10 or just on 1.0. Any thoughts?

0.10, I would not change this on 0.9 which is the current version, right?

[henriquemoody]

Yeah, I just kept it on master only.

[chozilla]

Hello, as fare as I can tell, the described problem is not fixed.
The ->setName() function still does not behave as expected or described in the use cases in this discussion.

// example
$testValue = array();
$testValue['nameFirst'] = 'Jane DOOO0E FILLTEXT! MUCH LONG';
$testValue['nameLast'] = 'Doe';

$nameValidator = \Respect\Validation\Validator::stringType()->setName('First Name')
  ->alpha()->setName('First Name')
  ->noWhitespace()->setName('First Name')
  ->length(2, 15)->setName('First Name');

try {
    \Respect\Validation\Validator::key('nameFirst', $nameValidator)->setName('First Name')->assert($testValue);
} catch(\Respect\Validation\Exceptions\NestedValidationException $nestedException) {

  $nestedException->setName('First Name');

  foreach($nestedException as $exception) {
    $exception->setName('First Name');
  }
  print_r($nestedException->getMessages())
}

Outputs:

Array ( 
  [0] => These rules must pass for nameFirst 
  [1] => nameFirst must contain only letters (a-z) 
  [2] => nameFirst must not contain whitespace 
  [3] => nameFirst must have a length between 2 and 15 
) 

Whatever I try... I am not able to replace 'nameFirst' with 'First Name' with any means provided by the basic Validator API.

EDIT
Ok, I found one way to make the message display correctly, but this is for sure not what was intended.

try {
    \Respect\Validation\Validator::key('nameFirst', $nameValidator)->assert($testValue);
} catch(\Respect\Validation\Exceptions\NestedValidationException $nestedException) {

  foreach($nestedException as $exception) {
    //If you set the name on the child exceptions...
    $exception->setName('First Name'); 
    //...and trigger the setting of the 'name' param...
    $exception->setParam('name', 'WHATEVER'); 
    //...you get the expected output. (it does not matter what value you set)
  }
  print_r($nestedException->getMessages())
}

Outputs:

Array (
  [0] => These rules must pass for First Name 
  [1] => First Name must contain only letters (a-z) 
  [2] => First Name must not contain whitespace 
  [3] => First Name must have a length between 2 and 15 
) 

[FoxxMD]

For anyone else coming across this who wants to use Respect with models and also like to have this sensible default (getting error messages with attribute name rather than value)

abstract class Base {
    protected abstract function getValidationRules();

    //http://respect.li/Validation/docs/validators.html
    public function validate($rules = null, $catchAndReturn = false) {

        $errors = [];
        $rulesToRun = null === $rules ? $this->getValidationRules() : $rules;
        foreach ($rulesToRun as $validator) {
            try {
                $validator->check($this);
            } catch (ExceptionInterface $e) {
                $varName = current($validator->getRules())->getName();
                $errors[$varName] = $e->getMainMessage();
            }
        }
        if ($catchAndReturn) {
            return $errors;
        } else {
            if (!empty($errors)) {
                throw new ValidationFailureException('Validation failed for this object.', $errors);
            }
        }
    }
}

and then in your model:

    protected function getValidationRules()
    {
        return [
            v::attribute('name', v::notEmpty()),
            v::attribute('type', v::notEmpty()),
            v::attribute('userId', v::numeric()),
            v::attribute('createDate', v::notEmpty()->date())
        ];
    }

[JCMais]

After some tries, for people having issues while trying to validate keys, make sure you are calling setName to the validators for the key:

v::key( 'pass', v::stringType()->length( 6 )->setName( 'Password' ) )

# Password must have a length greater than 6

v::key( 'pass', v::stringType()->length( 6 ) )->setName( 'Password' )

# pass must have a length greater than 6

[chozilla]

I thought my example where i call setName() in like 7 different places showed that this is not the case...

[JCMais]

@chozilla you probably are using an outdated version. Because I get the correct output with the code I posted.

[shorif2000]

i have the following which is not working

...
$this->demand_ref = v::optional(v::stringType()->length(0,15)->regex('/^(INC|CRQ|TAS|RLM)*?\d{12}/i'))->setName('Demand ref');
...

public function validateForm(array $params) : string {
        foreach ($params as $key => $value){
            if(isset($this->{$key})){
                //$rule = v::attribute(strtolower($key), $this->{$key});
                $rule = $this->{$key};
                try {
                    if(is_array($value)){
                        foreach ($value as $k => $v){
                            $valid = $rule->assert($v);
                        }
                    }else{
                        $valid = $rule->assert($value);
                        //$this->log->debug("$key : $valid");
                    }


                } catch(NestedValidationException $exception) {
                    $this->log->error("$key : " . $exception->getFullMessage());
                    //$exception->setParam('translator', $this->translateMessage($key));
                    //echo $exception->getFullMessage();
                    $errors = $exception->findMessages([
                        'regex' => "$key is Invalid"
                    ]);
                    
                    $this->log->error($errors);
                    //return $errors;
                    return $exception->getFullMessage();
                }
            }
        }

        return '';
    }

what i see is - demand_ref Invalid

[wesleyvicthor]

@shorif2000 what were you expecting to see ?

[shorif2000]

i used set name so i should see something like Demand ref - is Invalid . it should match the messages 'regex' => "$key is Invalid"