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
>>
>>

Reply via email to