Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fix missing optional fields should not be validated #25

Conversation

Maks3w
Copy link
Member

@Maks3w Maks3w commented Aug 20, 2015

Prevent the assumption of null value to be a synonym of value not set. Null values are valid input values (like JSON object with null values)

@Maks3w Maks3w added the bug label Aug 20, 2015
@Maks3w Maks3w added this to the 2.5.5 milestone Aug 20, 2015
@Maks3w
Copy link
Member Author

Maks3w commented Aug 20, 2015

Most basic test does not work due zendframework/zendframework#5271 getRawValues() returns a list of pairs Input => null which make this logic to fail.

@Maks3w Maks3w force-pushed the hotfix/missing-optional-field-should-not-be-validated branch from 28ca59b to fc2e1e2 Compare August 20, 2015 11:54
@svycka
Copy link
Contributor

svycka commented Aug 20, 2015

I think this explains and fixes zendframework/zendframework#6638

@Maks3w
Copy link
Member Author

Maks3w commented Aug 20, 2015

It's not related. This does not change the meaning of any empty value. This only treat the case when the value is not set (not exists !== empty).

@svycka
Copy link
Contributor

svycka commented Aug 20, 2015

There was impossible to check if value exists or not, so I used notEmpty validator to cover this, but that was not ideal because there was some values treated as empty anyway. This fixes my issue because now I can check not set, is set or not empty, before this PR that was impossible. As I understand this PR adds this functionality.

@Maks3w Maks3w force-pushed the hotfix/missing-optional-field-should-not-be-validated branch from fc2e1e2 to 4946a05 Compare August 20, 2015 14:31
Maks3w added 2 commits August 21, 2015 00:09
Prevent the assumption of `null` value to be a synonym of value not set. Null values are valid input values (like JSON object with null values)
This logic added on zenframework/zf2#5271 makes impossible determinate on the loop when the data was set or if it using a fallback value.
@Maks3w Maks3w force-pushed the hotfix/missing-optional-field-should-not-be-validated branch from 99730a4 to 9c1213b Compare August 20, 2015 22:09
@weierophinney
Copy link
Member

I'm concerned that this changes test assumptions, as that generally indicates a BC break. Looking at the logic, however, it appears sound. I'll review more fully early next week.


$bar = new Input();
$bar->getValidatorChain()->attach(new Validator\Digits());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input::isValid() if ($empty && ! $required && ! $continueIfEmpty) { return true; }

Make this to pass. Test refactor generalize this and make the test resilient to InputInterface implementations with the use of the mock.

@@ -231,11 +231,6 @@ public function isValid($context = null)
*/
protected function validateInputs(array $inputs, $data = [], $context = null)
{
// backwards compatibility
if (empty($data)) {
$data = $this->getRawValues();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removal of this make consistent the output of #14.

Basically this BC does not ensure the same data when the original is empty.

Due to the fact that default all fields will have a valid value (null) It's impossible detect when the field was really set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been removed in #7 due the same arguments described in this comment https://github.com/zendframework/zend-inputfilter/pull/7/files#r35374042

@Maks3w
Copy link
Member Author

Maks3w commented Aug 21, 2015

Note: It's impossible to undo all BC Breaks introduced in ZF 2.4. The split of null between not set and valid value can't be undo.

@weierophinney weierophinney merged commit 1560639 into zendframework:master Aug 26, 2015
weierophinney added a commit that referenced this pull request Aug 26, 2015
…ld-not-be-validated

Fix missing optional fields should not be validated
weierophinney added a commit that referenced this pull request Aug 26, 2015
weierophinney added a commit that referenced this pull request Aug 26, 2015
@Maks3w Maks3w deleted the hotfix/missing-optional-field-should-not-be-validated branch August 26, 2015 20:36
weierophinney added a commit to weierophinney/zend-inputfilter that referenced this pull request Sep 8, 2015
Includes these changes:

- [14: NotEmpty validator doesn't override the required attribute](zendframework#14)
- [16: BC in validators context value](zendframework#16)
- [22: Missing required field with fallback does not return input in getValidInput](zendframework#22)
- [24: Fix loop validation input context is mutable](zendframework#24)
- [25: Fix missing optional fields should not be validated](zendframework#25)
- [32: Promote HHVM](zendframework#32)
- [36: Fix docblocks declared as regular comment block](zendframework#36)
- [40: Remove test about Input setters permutation](zendframework#40)
- [41: Refactor tests for group 7448 as a data set matrix](zendframework#41)
- [42: Consolidate InputFilterPluginManager tests](zendframework#42)
- [43: Consolidate Factory tests + Fixes](zendframework#43)
- [44: Consolidate of InputInterface::merge and Fix unsafe merge](zendframework#44)
- [45: When merge Inputs don't merge values if source does not have one.](zendframework#45)
- [46: Expand test matrix with nonempty value scenarios](zendframework#46)
- [47: Feature/minor test improvements](zendframework#47)
- [48: Empty values + Allow Empty + Not continue if empty is always true](zendframework#48)
- [49: Add tests for exceptions and improve some messages](zendframework#49)
- [50: Optional fields without value should be valid ](zendframework#50)
- [51: Optional input without value are valid](zendframework#51)
- [52: Fix merge two inputs without value, result 'has value' flag should be false](zendframework#52)
- [53: Tune NonEmpty options for detect the expected empty values](zendframework#53)
- [55: Make symmetric the scenarios when required is true/false](zendframework#55)
- [56: Hotfix/minor changes](zendframework#56)
- [57: Consolidate tests for InputFilterInterface](zendframework#57)
- [61: Provide NotEmpty::IS&zendframework#95;EMPTY validation message for required input](zendframework#61)
- [63: Ensure custom error messages are used for required missing inputs](zendframework#63)
- [64: Feature/test cleanup](zendframework#64)
weierophinney added a commit to weierophinney/zend-inputfilter that referenced this pull request Sep 9, 2015
Includes these changes:

- [14: NotEmpty validator doesn't override the required attribute](zendframework#14)
- [16: BC in validators context value](zendframework#16)
- [24: Fix loop validation input context is mutable](zendframework#24)
- [25: Fix missing optional fields should not be validated](zendframework#25)
- [26: Deprecate magic logic for auto attaching a NonEmpty validator with breakChainOnFailure = true](zendframework#26)
- [31: Update the InputFilterInterface::add docblock to match existing, shipped implementations](zendframework#31)
- [32: Promote HHVM](zendframework#32)
- [36: Fix docblocks declared as regular comment block](zendframework#36)
- [40: Remove test about Input setters permutation](zendframework#40)
- [41: Refactor tests for group 7448 as a data set matrix](zendframework#41)
- [42: Consolidate InputFilterPluginManager tests](zendframework#42)
- [43: Consolidate Factory tests + Fixes](zendframework#43)
- [44: Consolidate of InputInterface::merge and Fix unsafe merge](zendframework#44)
- [45: When merge Inputs don't merge values if source does not have one.](zendframework#45)
- [46: Expand test matrix with nonempty value scenarios](zendframework#46)
- [47: Feature/minor test improvements](zendframework#47)
- [48: Empty values + Allow Empty + Not continue if empty is always true](zendframework#48)
- [49: Add tests for exceptions and improve some messages](zendframework#49)
- [50: Optional fields without value should be valid ](zendframework#50)
- [51: Optional input without value are valid](zendframework#51)
- [52: Fix merge two inputs without value, result 'has value' flag should be false](zendframework#52)
- [56: Hotfix/minor changes](zendframework#56)
- [57: Consolidate tests for InputFilterInterface](zendframework#57)
- [61: Provide NotEmpty::IS&zendframework#95;EMPTY validation message for required input](zendframework#61)
- [63: Ensure custom error messages are used for required missing inputs](zendframework#63)
- [64: Feature/test cleanup](zendframework#64)
weierophinney added a commit to weierophinney/zend-inputfilter that referenced this pull request Sep 9, 2015
Includes these changes:

- [14: NotEmpty validator doesn't override the required attribute](zendframework#14)
- [16: BC in validators context value](zendframework#16)
- [24: Fix loop validation input context is mutable](zendframework#24)
- [25: Fix missing optional fields should not be validated](zendframework#25)
- [26: Deprecate magic logic for auto attaching a NonEmpty validator with breakChainOnFailure = true](zendframework#26)
- [31: Update the InputFilterInterface::add docblock to match existing, shipped implementations](zendframework#31)
- [36: Fix docblocks declared as regular comment block](zendframework#36)
- [40: Remove test about Input setters permutation](zendframework#40)
- [41: Refactor tests for group 7448 as a data set matrix](zendframework#41)
- [42: Consolidate InputFilterPluginManager tests](zendframework#42)
- [43: Consolidate Factory tests + Fixes](zendframework#43)
- [44: Consolidate of InputInterface::merge and Fix unsafe merge](zendframework#44)
- [45: When merge Inputs don't merge values if source does not have one.](zendframework#45)
- [46: Expand test matrix with nonempty value scenarios](zendframework#46)
- [47: Feature/minor test improvements](zendframework#47)
- [48: Empty values + Allow Empty + Not continue if empty is always true](zendframework#48)
- [49: Add tests for exceptions and improve some messages](zendframework#49)
- [50: Optional fields without value should be valid ](zendframework#50)
- [51: Optional input without value are valid](zendframework#51)
- [52: Fix merge two inputs without value, result 'has value' flag should be false](zendframework#52)
- [56: Hotfix/minor changes](zendframework#56)
- [57: Consolidate tests for InputFilterInterface](zendframework#57)
- [61: Provide NotEmpty::IS&zendframework#95;EMPTY validation message for required input](zendframework#61)
- [63: Ensure custom error messages are used for required missing inputs](zendframework#63)
- [64: Feature/test cleanup](zendframework#64)
weierophinney added a commit to weierophinney/zend-inputfilter that referenced this pull request Sep 9, 2015
Includes these changes:

- [14: NotEmpty validator doesn't override the required attribute](zendframework#14)
- [16: BC in validators context value](zendframework#16)
- [24: Fix loop validation input context is mutable](zendframework#24)
- [25: Fix missing optional fields should not be validated](zendframework#25)
- [26: Deprecate magic logic for auto attaching a NonEmpty validator with breakChainOnFailure = true](zendframework#26)
- [31: Update the InputFilterInterface::add docblock to match existing, shipped implementations](zendframework#31)
- [36: Fix docblocks declared as regular comment block](zendframework#36)
- [40: Remove test about Input setters permutation](zendframework#40)
- [41: Refactor tests for group 7448 as a data set matrix](zendframework#41)
- [42: Consolidate InputFilterPluginManager tests](zendframework#42)
- [43: Consolidate Factory tests + Fixes](zendframework#43)
- [44: Consolidate of InputInterface::merge and Fix unsafe merge](zendframework#44)
- [45: When merge Inputs don't merge values if source does not have one.](zendframework#45)
- [46: Expand test matrix with nonempty value scenarios](zendframework#46)
- [47: Feature/minor test improvements](zendframework#47)
- [48: Empty values + Allow Empty + Not continue if empty is always true](zendframework#48)
- [49: Add tests for exceptions and improve some messages](zendframework#49)
- [50: Optional fields without value should be valid ](zendframework#50)
- [51: Optional input without value are valid](zendframework#51)
- [52: Fix merge two inputs without value, result 'has value' flag should be false](zendframework#52)
- [56: Hotfix/minor changes](zendframework#56)
- [57: Consolidate tests for InputFilterInterface](zendframework#57)
- [61: Provide NotEmpty::IS&zendframework#95;EMPTY validation message for required input](zendframework#61)
- [63: Ensure custom error messages are used for required missing inputs](zendframework#63)
- [64: Feature/test cleanup](zendframework#64)
weierophinney added a commit to zendframework/zendframework that referenced this pull request Dec 20, 2016
Includes these changes:

- [14: NotEmpty validator doesn't override the required attribute](zendframework/zend-inputfilter#14)
- [16: BC in validators context value](zendframework/zend-inputfilter#16)
- [24: Fix loop validation input context is mutable](zendframework/zend-inputfilter#24)
- [25: Fix missing optional fields should not be validated](zendframework/zend-inputfilter#25)
- [26: Deprecate magic logic for auto attaching a NonEmpty validator with breakChainOnFailure = true](zendframework/zend-inputfilter#26)
- [31: Update the InputFilterInterface::add docblock to match existing, shipped implementations](zendframework/zend-inputfilter#31)
- [36: Fix docblocks declared as regular comment block](zendframework/zend-inputfilter#36)
- [40: Remove test about Input setters permutation](zendframework/zend-inputfilter#40)
- [41: Refactor tests for group 7448 as a data set matrix](zendframework/zend-inputfilter#41)
- [42: Consolidate InputFilterPluginManager tests](zendframework/zend-inputfilter#42)
- [43: Consolidate Factory tests + Fixes](zendframework/zend-inputfilter#43)
- [44: Consolidate of InputInterface::merge and Fix unsafe merge](zendframework/zend-inputfilter#44)
- [45: When merge Inputs don't merge values if source does not have one.](zendframework/zend-inputfilter#45)
- [46: Expand test matrix with nonempty value scenarios](zendframework/zend-inputfilter#46)
- [47: Feature/minor test improvements](zendframework/zend-inputfilter#47)
- [48: Empty values + Allow Empty + Not continue if empty is always true](zendframework/zend-inputfilter#48)
- [49: Add tests for exceptions and improve some messages](zendframework/zend-inputfilter#49)
- [50: Optional fields without value should be valid ](zendframework/zend-inputfilter#50)
- [51: Optional input without value are valid](zendframework/zend-inputfilter#51)
- [52: Fix merge two inputs without value, result 'has value' flag should be false](zendframework/zend-inputfilter#52)
- [56: Hotfix/minor changes](zendframework/zend-inputfilter#56)
- [57: Consolidate tests for InputFilterInterface](zendframework/zend-inputfilter#57)
- [61: Provide NotEmpty::IS_EMPTY validation message for required input](zendframework/zend-inputfilter#61)
- [63: Ensure custom error messages are used for required missing inputs](zendframework/zend-inputfilter#63)
- [64: Feature/test cleanup](zendframework/zend-inputfilter#64)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants