> On May 21, 2019, at 2:20 PM, Romain Manni-Bucau <[email protected]> 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
