Undo deprecation of AllowEmpty + ContinueIfEmpty

[carnage]

In the Input these comments: @deprecated 2.4.8 Add Zend\Validator\NotEmpty validator to the ValidatorChain. have been added to AllowEmpty and ContinueIfEmpty. These properties should not be deprecated. Their use differs subtly to the use case for a NotEmpty validator. If you add a not empty validator, you can no longer validate optional fields.

Consider a shipping notes field which is optional but if the user provides a value, the string should be between 5 and 200 characters long.

Continue if empty is also required as this allows validating optional fields against other field context eg if you have a field on the form 'Have you had any none driving convictions in the last 5 years?' as a yes/no radio and a 'If yes please provide details' For this, you would setup a validation chain which allows empty values, but continues if empty. You would then add a (custom) validator which ensures that a value is provided if the first question has a yes answer.

https://github.com/zendframework/zf2/issues/6668 Provides some more context on how these options work and interplay with each other.

[Maks3w]

Unfortunatelly those settings are incompatible with NotEmpty defaults.

This component try to guess if the value is empty regardless NotEmpty validator https://github.com/zendframework/zend-inputfilter/blob/master/src/Input.php#L391

So those options are clearly incompatible and the desired feature can be do just appending the NotEmpty validator to the validator chain.

You would then add a (custom) validator which ensures that a value is provided if the first question has a yes answer.

You will do it anyway. There is no setting which allow cross validation with other fields. If you can do it, please provide an snippet with the code present in 2.4 which do what you expect.

how these options work and interplay with each other.

I wrote a test suite testing the whole matrix of options https://github.com/zendframework/zend-inputfilter/blob/master/test/InputTest.php#L686-L714

And that test suite is usefull for to discover many values does not return the expected values https://github.com/zendframework/zend-inputfilter/blob/master/test/InputTest.php#L746-L749 https://github.com/zendframework/zend-inputfilter/blob/master/test/InputTest.php#L773-L777 https://github.com/zendframework/zend-inputfilter/blob/master/test/InputTest.php#L782-L786

In resume, the criteria for validate a given value must not be dependent if the input is required or not. What we do discover is commented values has different behavior when the input is optional.

Finally, this component forced us to deploy many LTS releases, aka critical bugs, and after audit the component we found those bugs are not completely fixed and are not fixable.

[carnage]

Currently the flags are supposed to work as follows:

required: When true validation will fail if the input is not set with a value. When false, input will be marked as valid if no value is set (eg setValue is never called)

When the component is used in conjunction with Zend form, all values in the form are provided to the input filter regardless of any data supplied by the user, so the input is ALWAYS set with a value - so this flag is fairly useless.

allow empty: When true, validation will pass if the value set is considered empty. The check here isn't ideal and is inconsistent with the notEmpty validator. When false, Allow empty will fail validation early if the value is set to an empty value; this behaviour isn't strictly required provided there is a NotEmpty validator first in the chain with break on failure set to true.

When the component is used in conjunction with Zend form, this is essentially the flag you use to say this field is optional. If the user provides a value which is considered to be empty and this flag is set no further validation is run. Without this flag EVERY validator would need to support a concept of 'return true if this value is empty' otherwise you would loose the ability to validate optional fields.

continue if empty: The behaviour of allow empty when false gives rise to the need for the continue if empty flag. This basically overrides the fail early behaviour to allow you to run a validation chain on empty values.

The only use case I've come upon for this is conditional validation. For example, a form in a project I worked on previously had two fields which were optional: start date and end date, however if you provided them they had to be a valid date and if you provided 1, you had to provide the other.

For something in 2.4 that uses this functionality consider the identical validator (https://github.com/zendframework/zend-validator/blob/master/src/Identical.php) which is commonly used for checking email + password fields match.

I'd welcome an example of how I can have an optional "email_address" field which validates that the email is correctly formatted and that it matches a "confirm_email_address" field on the same form without using the deprecated fields and using a Not Empty validator instead.

That aside, I do have an alternative implementation option which would allow deprecation, but it produces a large overhaul of input filter, filter and validator (and form to refactor to new implementation) which would be hard to do and maintain any backward compatibility.

[Maks3w]

In the changelog a snippet is provided

  $input = new Input();
  $input->setAllowEmpty(true); // Disable BC Break logic related to treat `null` values as valid empty value instead *not set*.
  $input->setContinueIfEmpty(true); // Disable BC Break logic related to treat `null` values as valid empty value instead *not set*.  
  $input->getValidatorChain()->attach(new Zend\Validator\NotEmpty(), /* break chain on failure */ true);

  $inputSpecification = [
    'allow_empty' => true,
    'continue_if_empty' => true,
    'validators' => [
      [
        'break_chain_on_failure' => true,
        'name' => 'Zend\\Validator\\NotEmpty',
      ],
    ],
  ];

[Maks3w]

I'll close this because is not longer related to this component. Its a question about how to apply the deprecations in zend\form

[carnage]

I think you are missing my point here. To my understanding deprecated = will be removed in the next major version eg Zend Input filter 3.0.0

If these two flags are removed, you will not only break a lot of user land code, but it will also break use cases for stock Zend components.

Breaking backward compatibility is perfectly acceptable in a major version if it is justified but you would usually provide some means to migrate existing code and get the same functionality. The migration you are proposing is woefully insufficient as it only covers the case of ensuring a value is not empty, yet doesn't cover any of the other use cases for these two flags.

This issue is related to this component: I am suggesting we undo the deprecation and subsequent removal of these flags due to the knock on effect it will have on user code and other Zend components.

[Ocramius]

@carnage that makes sense IMO (un-deprecating). In general, we should put in a blocker somewhere (or a note) that says that we cannot remove the feature unless we have a cleaner alternate approach.

[weierophinney]

There's a bigger problem that @maks3w was hinting at: the flags currently have already led to logic conflicts at this point, resulting in broken functionality in released versions. The reason for deprecating them is to prevent such conflicts in future revisions.

Yes, we will need a migration path. That said, any changes to this component that are non-BC-safe would be in a new major version, and would precede changes in dependent components such as zend-form, which helps us provide that path.

[Maks3w]

We should understand that allowEmpty should had been a sugar function for attach the NonEmpty validator and continueIfEmpty used for toggle the break on chain failure parameter.

But this can't not longer to be fixed without to break the BC.

So should we try to fix it on v3? The answer is no for the following reasons:

• Change the behavior without rename the methods will end to undesirable surprises for all those don't read the changelog, don't understand the impact of the changes and don't audit all his code and all the third party code will work with the new behavior (I'm one of them)
• Why to maintain duplicate behavior which can be achieved easily just declaring explicly the use of the NonEmpty validator.

said this. Do you want to preserve those functions? Ok, you can create your own Input class and preserve the actual behavior on future versions.