zendframework / zend-inputfilter

InputFilter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
64 stars 50 forks source link

Input fallback_value option limitation/feature suggestion #70

Closed svycka closed 8 years ago

svycka commented 8 years ago

Would be nice if possible to use fallback value on optional fields only when input not set. Current behavior is when input is not set or validation fails, but I would like to fail validation if input is not valid and use fallback only when input is not set at all. With current behavior is impossible to check if data is invalid.

example:

// create $inputFilter with 'optional_field' input and 'fallback' as fallback value

// when input is not set
$inputFilter->setData([]);
$inputFilter->isValid(); // return true
$inputFilter->getValue('optional_field'); // return 'fallback'

// when input is set but not valid
$inputFilter->setData(['optional_field' => 'invalid']);
$inputFilter->isValid(); // return false

This is not possible at the moment, but very important scenario for me. I think after https://github.com/zendframework/zend-inputfilter/pull/25 when required option works as 'is set' and not as 'not empty' this would make more sense. no?

I think having fallback on invalid data is bad idea. Also if data should be modified somehow to be valid there is filters for this.

larsnystrom commented 8 years ago

A fallback_if_invalid setting would fix this. If it defaulted to true there would be no BC break. It would also be pretty easy to implement.

weierophinney commented 8 years ago

It would also be pretty easy to implement.

Considering the dozens of hours spent with the most recent releases to build a test matrix for the current combination of options, I beg to differ. Any new options introduced at this point need to be very carefully evaluated, as they add more permutations to the test matrix: we already are testing something like 32 combinations of options; adding another would double that to 64.

larsnystrom commented 8 years ago

Considering the dozens of hours spent with the most recent releases to build a test matrix for the current combination of options, I beg to differ. Any new options introduced at this point need to be very carefully evaluated, as they add more permutations to the test matrix: we already are testing something like 32 combinations of options; adding another would double that to 64.

In that case, maybe we could remove the "fallback if invalid" behaviour in 3.0?

svycka commented 8 years ago

I think the name is not correct for this behavior I am thinking about default_value or something this should be used only if value not set, otherwise should be used fallback if set. this also would solve BC problem but also have big test matrix problem. But well if this is good feature and we want it in 3.0 we can do it now and remove whole test matrix in 3.0 when other options will be removed.

Maks3w commented 8 years ago

The behavior can be do with very little effort by extending Input with your custom class.

Personally I prefer don't add implementations are not required by the Input interface and just provide stable and tested implementations.

Anyway, feel free to send a PR with the desired implementation and I'll reconsider my opinion if the implementation is simple and bugfree

Hint: https://github.com/zendframework/zend-inputfilter/blob/a05fa0bfca3a82ef119e1af2e970a48d787913c7/test/InputTest.php#L581-L587 https://github.com/zendframework/zend-inputfilter/blob/a05fa0bfca3a82ef119e1af2e970a48d787913c7/test/BaseInputFilterTest.php#L473-L506

svycka commented 8 years ago

@Maks3w what do you think if we make this default behavior in ZF3? I think it is more logical to add fallback(default value) if value isn't set, but not when value is invalid(I would want error here). Or support both, but maybe that would be to complicated.

This would be BC but for 3.0 this is acceptable.

vaclavvanik commented 8 years ago

@svycka +1 I use same behavior especially in grid controllers

Maks3w commented 8 years ago

@svycka I don't consider replace a behavior an option. Instead I suggest send a PR where two behaviors could be used (one at time) probably setting a second parameter to setFallbackValue with an optional second argument where the default value is the current behavior.

Maks3w commented 8 years ago

Closed due inactivity

Erikvv commented 6 years ago

Ran into this limitation today made me pull my hair out. Fallback values which just work in the empty case would be very much appreciated.

W.r.t. complexity in the combination of flags: I think the solution is to split the Input class to multiple classes. Some combinations of options don't make sense or are very niche and you avoid that entirely that way.