-
Notifications
You must be signed in to change notification settings - Fork 102
[feature] all() validator #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
e50acc6
7bbdbb5
389d85d
6cfa238
da27023
48b4865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| number_range: all(num(min=5), num(max=10)) | ||
| password: all(str(min=8), regex('^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).*$')) | ||
| email: all(str(), regex('^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$')) | ||
| mixed_list: list(all(num(min=0), num(max=100))) | ||
| nested: | ||
| value: all(int(min=10), int(max=20)) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| number_range: 12 # Over max limit | ||
| password: "pass" # Too short and missing required characters | ||
| email: "invalid-email" # Not a valid email format | ||
| mixed_list: | ||
| - 42 | ||
| - 150 # Over max limit | ||
| - 15 | ||
| nested: | ||
| value: 5 # Under min limit |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| number_range: 7 | ||
| password: "Password123" | ||
| email: "test@example.com" | ||
| mixed_list: | ||
| - 42 | ||
| - 78 | ||
| - 15 | ||
| nested: | ||
| value: 15 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,6 +157,48 @@ def _is_valid(self, value): | |
| return True | ||
|
|
||
|
|
||
| class All(Validator): | ||
| """All validators must succeed validator""" | ||
|
|
||
| tag = "all" | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| self.validators = [val for val in args if isinstance(val, Validator)] | ||
| super(All, self).__init__(*args, **kwargs) | ||
|
|
||
| def _is_valid(self, value): | ||
| return True | ||
|
|
||
| def validate(self, value): | ||
| """ | ||
| Override to validate against all validators. | ||
| Returns a list of all errors from all validators. | ||
| """ | ||
| # First check base validator conditions | ||
| errors = [] | ||
|
|
||
| # Make sure the type validates first | ||
| valid = self._is_valid(value) | ||
| if not valid: | ||
| errors.append(self.fail(value)) | ||
| return errors | ||
|
|
||
| # Then validate all the constraints | ||
| for constraint in self._constraints_inst: | ||
| error = constraint.is_valid(value) | ||
| if error: | ||
| if isinstance(error, list): | ||
| errors.extend(error) | ||
| else: | ||
| errors.append(error) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps better to delegate everything above to the base class, ie something like: errors = super().validate(value)
# Now validate against all child validators
for validator in self.validators:
errors.extend(validator.validate(value))
return errorsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the collection of error messages seems strange, especially when we look the NotAny() validator which should print out success messages. ("NotAny() failed because X matches Y") |
||
|
|
||
| # Now validate against all child validators | ||
| for validator in self.validators: | ||
| errors.extend(validator.validate(value)) | ||
|
|
||
| return errors | ||
|
|
||
|
|
||
| class Subset(Validator): | ||
| """Subset of several types validator""" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like each of these can be accomplished without
all(), eg:num(min=5, max=10)str(min=8, matches={regex})There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree the examples in the tests might give the impression that all() is redundant with existing validators. My goal with those cases was more to confirm that the implementation was working correctly and integrating cleanly with the existing system, rather than to showcase complex validation logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. To be clear, I don't think it's necessary to "showcase complexity", but rather to demonstrate utility. I agree there's a use case here involving composition of custom validators -- thanks for adding that to the README.