Will wait for Matt feedback and we should recheck the spec but can help mid june yes. Have to admit i also see it as a good opportunity to enter the codebase to maybe a good call for contribution ;)
We should probably try to release the 2.0.3 which fix some thread safety issues Le mer. 22 mai 2019 à 09:31, Thomas Andraschko <[email protected]> a écrit : > 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 > > >> > > >> > > >
