Added support for inheritance hierarchies validation#369
Added support for inheritance hierarchies validation#369ArielBerkovich wants to merge 2 commits intomaking:developfrom
Conversation
b7d42de to
15f8692
Compare
15f8692 to
b19615d
Compare
| public <C extends T> ValidatorBuilder<T> constraintOnClass(Class<C> clazz, | ||
| Validator<C> cValidator) { | ||
| Validator<T> TValidator = new ValidatorBuilder<T>() | ||
| .nest(clazz::cast, clazz.getName(), cValidator).build(); |
There was a problem hiding this comment.
target name cannot be determined automatically.
There was a problem hiding this comment.
I'm a bit puzzled, actually, since I cant think of a target name that makes sense other than the class name.
that is because unlike regular .nest() usage, I am not validating a nested field, but rather the whole child class.
perhaps using nest() is some sort of an abuse here, given that I use it as work around to cast my entity, not access a nested field.
I'll see if I can come up with a different approach.
There was a problem hiding this comment.
ended up creating a new Validatable class dedicated to this case called InheritanceValidator, would love to hear your thoughts
There was a problem hiding this comment.
I meant that the target name should be a parameter.
I don't think InheritanceValidator is needed.
There was a problem hiding this comment.
Understood, I appreciate your review by the way. I find Yavi's design and philosophy thoroughly engaging.
I agree that the target name could have been a parameter, my concern arises from utilizing nest() for non-nested fields, and determining the appropriate target name for a child class.
what do you think?
| Function<CharSequenceConstraint<Admin, String>, CharSequenceConstraint<Admin, String>> startsWithAdmin = ( | ||
| CharSequenceConstraint<Admin, String> c) -> c.startsWith("yavi"); | ||
|
|
||
| return Stream |
There was a problem hiding this comment.
The test case is difficult to understand.
Please add the simple example in the pull request to the test case.
There was a problem hiding this comment.
sure, no problem.
|
|
||
| /** | ||
| * @since 0.14.0 | ||
| */ |
There was a problem hiding this comment.
These methods are quite advanced. Can you please add to the JavaDoc what use cases it is useful for and some sample code?
| User validUser = new User("Rawad", "rawad@gmail", 25); | ||
| User invalidUser = new User("Almog", "almog@gmail", 19); | ||
|
|
||
| assertThat(validator.validate(validUser).isValid()).isTrue(); |
There was a problem hiding this comment.
It is not enough to simply verify the results of validation. Please also verify the contents of the constraints (name and messageKey).
1390541 to
63fc402
Compare
63fc402 to
4c5ccff
Compare
added constraintOnClass api to resolve issue: #278
this provides convenient API to create a parent class validator that can also validate different inherited classes extending it.
example:
given a class called Child that extends Parent, one could utilize constraintOnClass in the following way: