Feature/required validator

[Maks3w]

These allows manage the required message like any other ValidatorInterface element.

Note: While you can add the validator manually the flag setRequired must be set too

  $input = new Input();
  $input->setRequired(true);empty value instead *not set*.
  $input->getValidatorChain()->attach(
      new Zend\InputFilter\Validator\Required(),
      true                             // break chain on failure

  );

  $inputSpecification = [
    'required'   => true,
    'validators' => [
      [
        'break_chain_on_failure' => true,
        'name'                   => 'Zend\\InputFilter\\Validator\\Required',
      ],
    ],
  ];

Note: setRequired(false) may not be enough and you will need remove the validator from the chain.

[Maks3w]

Inject a validator in the chain make difficult reset the input state for to validate different values or transititions set <=> not set

[larsnystrom]

I haven't tried running this yet, but I like the idea.

Why must "the flag setRequired" be set when adding the validator manually? Wouldn't it be better to just deprecate the flag?

[Maks3w]

Because the validator does not validate the input value (https://github.com/zendframework/zend-inputfilter/pull/62/files#diff-5e41c64803af5c0ad8ad84c3e25c834cR412)

I didn't resolve how to make to replace the whole required funcionallity with the validator.

This is needed too https://github.com/zendframework/zend-inputfilter/blob/master/src/BaseInputFilter.php#L253

[larsnystrom]

Hmm, yeah that's true. Do you think it would be possible to use the $data array as $context instead of the raw values? That would enable the Required-validator to check whether a key exists without having a reference to the input object.

You would have to inject the input's name in the Required-validator's constructor as well, so the validator knew which key to look for.

What do you think?

[Maks3w]

IIRC every input is present in $context with the default value of null.

[Maks3w]

Also rely on $context makes the input dependant of InputFilter implementation and can't be used standalone.

[larsnystrom]

The problem is that they're initialized to null. Null is still a value, and would pass the required-validator. So we need the original input data, where the the input is not present.

I don't think relying on $context makes the Input dependent on the InputFilter implementation. The Input::isValid() method is already accepting a $context parameter. We can control the default value of that parameter inside the Input class.

1. If no $context parameter is provided, and if there is no value, then set the $context to [].
2. If no $context parameter is provided, and if there is a value, then set the $context to ['inputName' => $val].
3. If a $context parameter is provided, then use that $context.

[Maks3w]

Even if looking in $context is needed to know what input name to search.

[larsnystrom]

Yes, that's why the input's name must be injected into the validator. If you add a RequiredValidator::setName() method, it will be compatible with the plugin manager.

[Maks3w]

I start to think the best option could be to have 2 validator chains. One for validate the data and another one for validate the input state.

Something like this

• Is required or not (RequiredValidator::isValid(input))
• Data is valid or not (DataValidator::isValid(Input))
  • Data is valid because has X characters (FooValidator::isValid(data))
  • Data is valid because exists on DB

[larsnystrom]

I just realized that my idea doesn't work. (The absence of a Required-validator wouldn't be equivalent to setRequired(false), since setRequired(false) implies not running any validators, but an absent Required-validator wouldn't give any information concerning whether to run the other validators.)

Two validator chains might be a good idea.

[akrabat]

IMO, if we are going to have a Required validator, then we really can't also have a required flag without causing confusion.

[stefanotorresi]

I agree with @akrabat

[larsnystrom]

I also agree with that. Right now it seems like the only way to implement a Required validator and not keep the required flag is to implement two validator chains, like Maks3w suggested.

I'm not sure if that's a good idea or not.

[Maks3w]

For to make this works seems It's needed a flag for "break the chain on success" for the ValidatorChain and create an OptionalValidator.

Thoughts? I think its reasonable to have break_chain_on_success and break_chain_on_failure

[larsnystrom]

I think we should merge #67 and #73 now since those won't cause any BC breaks.

Then I think we should do a more thorough refactorization for version 3.0.

In 3.0 I hope we can remove the required attribute altogether, make null equivalent to a missing value (thus leaving all validation to the normal validator chain), and have an optional attribute which defaults to false (optional can never fail so no error message required, just skip filters and validation if it's optional and the value is missing). I'd also like to remove fallback, continue_if_empty and allow_empty and instead have a default value, which would be used instead of null when a value is missing, but still be put through the whole filter and validation process.

But that's just a dream right now.

[Maks3w]

I don't see this as a BC Break because the translation feature seems never has work. Anyway the purpose of this PR is discuss about the required / optional field concept

[larsnystrom]

I was serious in my last post where I suggested we completely get rid of the required flag and add an optional flag. That would be a BC break, but a lot simpler to maintain than two different validator chains.

[stefanotorresi]

I don't see this as a BC Break because the translation feature seems never has work.

that's not correct, it worked prior to 2.4.

[Maks3w]

@stefanotorresi Prior to 2.5.2/2.4.5 there was not a required message.

[stefanotorresi]

it was handled by the notEmpty validator, which was always injected for required inputs before continue_if_empty and allow_empty flags were deprecated.

[weierophinney]

I really like this approach, but I share the same concern voiced by @akrabat — unless the required flag is tied to the Required validator, we're going to have a lot of confusion.

It seems like this is mostly the case right now, though; isValid() auto-prepends the validator if the $required flag is true. It's the other side of the equation we need, however: isRequired() should return true if the Required validator is present in the chain.

If you can implement that, I can merge this.

[weierophinney]

Moved to 2.7.0 milestone; we're ready for 2.6.0 (zend-servicemanager forwards-compatibility).

[Maks3w]

I keep thinking in new ways for make this better. Probably with something like #87

[weierophinney]

This repository has been closed and moved to laminas/laminas-inputfilter; a new issue has been opened at laminas/laminas-inputfilter#10.

[weierophinney]

This repository has been moved to laminas/laminas-inputfilter. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

• Squash all commits in your branch (git rebase -i origin/{branch})
• Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
• Run the laminas/laminas-migration tool on the code.
• Clone laminas/laminas-inputfilter to another directory.
• Copy the files from the second bullet point to the clone of laminas/laminas-inputfilter.
• In your clone of laminas/laminas-inputfilter, commit the files, push to your fork, and open the new PR.
  We will be providing tooling via laminas/laminas-migration soon to help automate the process.