bval-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Blevins <david.blev...@gmail.com>
Subject Re: BVAL-174 Return Parameter Validation Ignore void methods
Date Wed, 22 May 2019 03:16:53 GMT
> On May 21, 2019, at 2:20 PM, Romain Manni-Bucau <rmannibucau@gmail.com> wrote:
> 
> Probably a "too late to comment" thing but why is this true
> constraintsForMethod.hasConstrainedReturnValue())
> 
> Guess the code was right and either this method impl needs your diff or the
> use case is invalid.

Never too late :)  There is very likely a better fix and perhaps there is a bug here that
needs a much better resolution than the hack I threw in under time pressure.

What I noticed is that if the method return type is say `String`, all of the ConstraintValidators
on the method that do not apply to String are simply ignored.  I am not an Bean Validation
expert, so I convinced myself this was by design as you can compose constraints from other
constraints.  You could have an annotation called @GoodReturnData that validated all sorts
of return values using other annotated constraints and put it on all your methods.  I tested
this and BVal will happily ignore the ones that do not apply and enforce the ones that do.
 If none of the ConstraintValidators match, the return value is said to be valid.

Unless the return type is void, in which case an exception is thrown saying it cannot find
a ConstraintValidator for type Void.  I forget the exact stack trace, but I can get it easily
enough.  I took a quick look at what it would take to make constraintsForMethod.hasConstrainedReturnValue()
return false, but spun wheels a bit too long so put the hack in instead so we could have a
better discussion.

Indeed, the better fix would likely be returning false.  This seems consistent the "ignore
constraints that can't apply" philosophy and the TCK seems to be ok with that.

Where the use case would come in is my fix does not help people using the Validator API directly
to pass methods in and say "is this valid."  If they were doing that via reflection, which
we know they are because the API call takes a java.lang.reflect.Method instance, they'd need
to filter the void methods themselves like I did in the hack.  I.e. they'd need the "skip
these methods" hack too in their reflection code.

Side note, I'd have to double check, but I seem to recall it returning true even in the above
case where none of the ConstraintValidators could match the method.  I'm not sure if this
is because the matching is dynamic -- meaning a method could return java.lang.Object in the
declaration, but at runtime return a String value and trigger validation.  I don't know if
Bean Validation is specified to that level or not.


-David


Mime
View raw message