Ok, had a quick look, for executable validator types are not validated - see org.apache.bval.jsr.metadata.ReflectionBuilder.ForExecutable
I guess the ComputeConstraintValidatorClass logic should be imported in executable path, not fully sure what the spec says but we should clearly avoid the runtime solution since here we can avoid the interceptor completely IMHO. Here is a test to reproduce - simpler - if it helps: /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package org.apache.bval.jsr; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.junit.Assert.assertFalse; import java.lang.annotation.Retention; import javax.validation.Constraint; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; import javax.validation.Payload; import javax.validation.Validation; import javax.validation.ValidatorFactory; import javax.validation.metadata.MethodDescriptor; import javax.validation.metadata.ReturnValueDescriptor; import org.junit.Test; public class ReturnVoidConstraintTest { @Test public void checkIgnored() { final ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); final MethodDescriptor method = factory.getValidator() .getConstraintsForClass(Container.class) .getConstraintsForMethod("passthrough"); final ReturnValueDescriptor descriptor = method.getReturnValueDescriptor(); if (descriptor != null) { assertFalse(descriptor.getConstraintDescriptors().toString(), descriptor.hasConstraints()); } // else ok factory.close(); } public static class Container { @Password public void passthrough() { // no-op } } @Retention(RUNTIME) @Constraint(validatedBy = Password.Impl.class) public @interface Password { Class<?>[] groups() default {}; String message() default "failed"; Class<? extends Payload>[] payload() default {}; class Impl implements ConstraintValidator<Password, String> { @Override public boolean isValid(final String value, final ConstraintValidatorContext context) { return false; } } } } Unrelated notes: seems tomee lost the github links for examples + a requestscoped jaxrs endpoint with a local map as storage will likely always return an empty list in the @GET ;). Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> Le mer. 22 mai 2019 à 05:17, David Blevins <[email protected]> a écrit : > > 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 > >
