So will you take care of it, Romain? You also know the codebase much better :D
Am Mi., 22. Mai 2019 um 07:30 Uhr schrieb Romain Manni-Bucau < [email protected]>: > PS: quick workaround is to add the supported target explicitly to avoid > RETURN_TYPE I guess, not sure how elegant it is though > > 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 à 07:18, Romain Manni-Bucau <[email protected]> a > écrit : > > > 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 > >> > >> >
