Quarkus: @InterceptorBinding based annotation disables ReST validation

[harthorst]

Describe the bug
1) I have a rest Interface "IGreetingResource" which defines a method annotated with @Valid and @Size
2) there is a rest controller "GreetingResource" which implements "TestApi" annotated with @Override

Validation works like expected unless I add a custom annotation to GreetingResource. That annotation itself is marked with @InterceptorBinding. Validation is not executed anymore unless I add the @Valid annotation explicitly to "GreetingResource". If I remove @InterceptorBinding validation works again so it seems to be connected to @InterceptorBinding.

It also seems to be a difference whether the @Valid annotation is placed in the interface or in the implementing class. In the interface it returns a 500 status code - in the class it returns a 400 which looks better to me. Maybe this issue is a little bit bigger than I describe here.

Expected behavior
Validation works.

Actual behavior
Validation is deactivated in combination with custom annotation.

To Reproduce
Steps to reproduce the behavior:

1. clone https://github.com/harthorst/quarkus-reproducer-interceptorbinding
2. run it and see the failing GreetingResourceTest

[famod]

FTR, the reproducer is using Quarkus 1.6.1.Final. Upgrade to 1.7.0.Final doesn't change anything.

@gsmet I debugged this and one problem seems to be located here: https://github.com/quarkusio/quarkus/blob/master/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java#L81

Since GreetingResource.greeting() _does have_ an annotation (but a custom one, not related to validation), IGreetingResource is _not_ checked and MethodValidatedAnnotationsTransformer.requiresValidation() returns false.

We could drop method.annotations().isEmpty() (and pull up the the check for direct annotations) but I don't know how "expensive" it is to check _every_ method for inherited annotations to be validated.

[famod]

And the problem with @Valid and 500 vs. 400 is caused by the fact that the interface method (IGreetingResource.greeting()) is considered a JaxRs method, but not the method in the actual resource class (GreetingResource.greeting()):

• https://github.com/quarkusio/quarkus/blob/master/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java#L72-L76
• https://github.com/quarkusio/quarkus/blob/master/extensions/hibernate-validator/runtime/src/main/java/io/quarkus/hibernate/validator/runtime/jaxrs/JaxrsEndPointValidationInterceptor.java#L34
• https://github.com/quarkusio/quarkus/blob/master/extensions/hibernate-validator/runtime/src/main/java/io/quarkus/hibernate/validator/runtime/jaxrs/ResteasyViolationExceptionMapper.java#L41

So I guess we also have to traverse the interfaces for this check (similar to requiresValidation()). Problem here: How to access MethodInfo instances from/for the interface(s)?

Not sure what role the @InterceptorBinding is playing here at all.

[gsmet]

@famod You're right there's something fishy in the AnnotationsTransformer: the logic is flawed.

The issue was introduced here: https://github.com/quarkusio/quarkus/commit/45f875bd1c4b651f7a43617012a4a2c2ae786895 . The empty test used to be a shortcut but the new logic is definitely incorrect.

[gsmet]

@famod given you debugged the thing, would you like to try fixing it? The idea is to get it fixed sometime next week to get it in 1.7.1.Final. If you don't have the time, I'll give it a shot.

[famod]

@gsmet Ok I'll try. I might need some guidance on how to get hold of the MethodInfo of the interface (to check for JaxRs annotations) but in that case I'll open a Zulip topic.

[gsmet]

Let's fix one issue at a time I would say. Open a PR for the easy one (with a test) and then we can discuss the second one. I'm around so you can just ping me anytime :).

[famod]

FTR: The disabled validation is now fixed in master.

The fix for the 400 vs. 500 problem is still being discussed, but I am optimistic we'll be able to fix this soon.