Merge pull request #30225 from gsmet/resteasy-hv-produced-inheritance

Take into account @produces annotations from interfaces and superclasses in JaxrsEndPointValidationInterceptor

extensions/hibernate-validator/runtime/src/main/java/io/quarkus/hibernate/validator/runtime/jaxrs/JaxrsEndPointValidationInterceptor.java

@@ -1,9 +1,11 @@
 package io.quarkus.hibernate.validator.runtime.jaxrs;
 
 import java.lang.reflect.Method;
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 import javax.annotation.Priority;
 import javax.inject.Inject;
@@ -12,10 +14,9 @@
 import javax.interceptor.Interceptor;
 import javax.interceptor.InvocationContext;
 import javax.validation.ConstraintViolationException;
+import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
 
-import org.jboss.resteasy.util.MediaTypeHelper;
-
 import io.quarkus.hibernate.validator.runtime.interceptor.AbstractMethodValidationInterceptor;
 
 @JaxrsEndPointValidated
@@ -25,6 +26,8 @@ public class JaxrsEndPointValidationInterceptor extends AbstractMethodValidation
 
     private static final List<MediaType> JSON_MEDIA_TYPE_LIST = Collections.singletonList(MediaType.APPLICATION_JSON_TYPE);
 
+    private final ConcurrentHashMap<Method, List<MediaType>> producedMediaTypesCache = new ConcurrentHashMap<>();
+
     @Inject
     ResteasyConfigSupport resteasyConfigSupport;
 
@@ -34,7 +37,13 @@ public Object validateMethodInvocation(InvocationContext ctx) throws Exception {
         try {
             return super.validateMethodInvocation(ctx);
         } catch (ConstraintViolationException e) {
-            throw new ResteasyViolationExceptionImpl(e.getConstraintViolations(), getProduces(ctx.getMethod()));
+            List<MediaType> producedMediaTypes = getProduces(ctx.getMethod());
+
+            if (producedMediaTypes.isEmpty() && resteasyConfigSupport.isJsonDefault()) {
+                producedMediaTypes = JSON_MEDIA_TYPE_LIST;
+            }
+
+            throw new ResteasyViolationExceptionImpl(e.getConstraintViolations(), producedMediaTypes);
         }
     }
 
@@ -44,17 +53,91 @@ public void validateConstructorInvocation(InvocationContext ctx) throws Exceptio
         super.validateConstructorInvocation(ctx);
     }
 
-    private List<MediaType> getProduces(Method method) {
-        MediaType[] producedMediaTypes = MediaTypeHelper.getProduces(method.getDeclaringClass(), method);
+    private List<MediaType> getProduces(Method originalMethod) {
+        List<MediaType> cachedMediaTypes = producedMediaTypesCache.get(originalMethod);
+
+        if (cachedMediaTypes != null) {
+            return cachedMediaTypes;
+        }
+
+        return producedMediaTypesCache.computeIfAbsent(originalMethod, new Function<Method, List<MediaType>>() {
+
+            @Override
+            public List<MediaType> apply(Method method) {
+                return doGetProduces(originalMethod);
+            }
+        });
+    }
+
+    /**
+     * Ideally, we would be able to get the information from RESTEasy so that we can follow strictly the inheritance rules.
+     * But, given RESTEasy Reactive is our new default REST layer, I think we can live with this limitation.
+     * <p>
+     * Superclass method annotations have precedence, then interface methods and finally class annotations.
+     */
+    private List<MediaType> doGetProduces(Method originalMethod) {
+        Class<?> currentClass = originalMethod.getDeclaringClass();
+        List<Class<?>> interfaces = new ArrayList<>();
+
+        do {
+            List<MediaType> classMethodProducedMediaTypes = getProducesFromMethod(currentClass, originalMethod);
+            if (!classMethodProducedMediaTypes.isEmpty()) {
+                return classMethodProducedMediaTypes;
+            }
+
+            for (Class<?> interfaze : currentClass.getInterfaces()) {
+                interfaces.add(interfaze);
+            }
+
+            currentClass = currentClass.getSuperclass();
+        } while (!Object.class.equals(currentClass));
 
-        if (producedMediaTypes == null) {
-            if (resteasyConfigSupport.isJsonDefault()) {
-                return JSON_MEDIA_TYPE_LIST;
+        for (Class<?> interfaze : interfaces) {
+            List<MediaType> interfaceMethodProducedMediaTypes = getProducesFromMethod(interfaze, originalMethod);
+            if (!interfaceMethodProducedMediaTypes.isEmpty()) {
+                return interfaceMethodProducedMediaTypes;
             }
+        }
+
+        List<MediaType> classProducedMediaTypes = getProduces(originalMethod.getDeclaringClass().getAnnotation(Produces.class));
+        if (!classProducedMediaTypes.isEmpty()) {
+            return classProducedMediaTypes;
+        }
 
+        for (Class<?> interfaze : interfaces) {
+            List<MediaType> interfaceProducedMediaTypes = getProduces(interfaze.getAnnotation(Produces.class));
+            if (!interfaceProducedMediaTypes.isEmpty()) {
+                return interfaceProducedMediaTypes;
+            }
+        }
+
+        return Collections.emptyList();
+    }
+
+    private List<MediaType> getProducesFromMethod(Class<?> currentClass, Method originalMethod) {
+        if (currentClass.equals(originalMethod.getDeclaringClass())) {
+            return getProduces(originalMethod.getAnnotation(Produces.class));
+        }
+
+        try {
+            return getProduces(currentClass
+                    .getMethod(originalMethod.getName(), originalMethod.getParameterTypes()).getAnnotation(Produces.class));
+        } catch (NoSuchMethodException | SecurityException e) {
+            // we don't have a visible method around, let's ignore this class
+            return Collections.emptyList();
+        }
+    }
+
+    public static List<MediaType> getProduces(Produces produces) {
+        if (produces == null) {
             return Collections.emptyList();
         }
 
-        return Arrays.asList(producedMediaTypes);
+        MediaType[] mediaTypes = new MediaType[produces.value().length];
+        for (int i = 0; i < produces.value().length; i++) {
+            mediaTypes[i] = MediaType.valueOf(produces.value()[i]);
+        }
+
+        return mediaTypes.length != 0 ? List.of(mediaTypes) : Collections.emptyList();
     }
 }

integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java

@@ -49,6 +49,7 @@
 
 @Path("/hibernate-validator/test")
 public class HibernateValidatorTestResource
+        extends HibernateValidatorTestResourceSuperclass
         implements HibernateValidatorTestResourceGenericInterface<Integer>, HibernateValidatorTestResourceInterface {
 
     @Inject
@@ -165,6 +166,13 @@ public String testRestEndPointInterfaceValidationWithAnnotationOnImplMethod(Stri
         return id;
     }
 
+    // all JAX-RS annotations are defined in the superclass
+    @Override
+    @SomeInterceptorBindingAnnotation
+    public String testRestEndPointInterfaceValidationWithAnnotationOnOverriddenMethod(String id) {
+        return id;
+    }
+
     @GET
     @Path("/rest-end-point-generic-method-validation/{id}")
     @Produces(MediaType.TEXT_PLAIN)

integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResourceSuperclass.java

@@ -0,0 +1,19 @@
+package io.quarkus.it.hibernate.validator;
+
+import javax.validation.constraints.Digits;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+
+public class HibernateValidatorTestResourceSuperclass {
+
+    @GET
+    @Path("/rest-end-point-interface-validation-annotation-on-overridden-method/{id}/")
+    @Produces(MediaType.TEXT_PLAIN)
+    public String testRestEndPointInterfaceValidationWithAnnotationOnOverriddenMethod(
+            @Digits(integer = 5, fraction = 0) @PathParam("id") String id) {
+        return id;
+    }
+}

integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java

@@ -278,11 +278,29 @@ public void testRestEndPointInterfaceValidationWithAnnotationOnImplMethod() {
                 .get("/hibernate-validator/test/rest-end-point-interface-validation-annotation-on-impl-method/plop/")
                 .then()
                 .statusCode(400)
+                .contentType(ContentType.TEXT)
                 .body(containsString("numeric value out of bounds"));
 
         RestAssured.when()
                 .get("/hibernate-validator/test/rest-end-point-interface-validation-annotation-on-impl-method/42/")
                 .then()
+                .contentType(ContentType.TEXT)
+                .body(is("42"));
+    }
+
+    @Test
+    public void testRestEndPointInterfaceValidationWithAnnotationOnOverriddenMethod() {
+        RestAssured.when()
+                .get("/hibernate-validator/test/rest-end-point-interface-validation-annotation-on-overridden-method/plop/")
+                .then()
+                .statusCode(400)
+                .contentType(ContentType.TEXT)
+                .body(containsString("numeric value out of bounds"));
+
+        RestAssured.when()
+                .get("/hibernate-validator/test/rest-end-point-interface-validation-annotation-on-overridden-method/42/")
+                .then()
+                .contentType(ContentType.TEXT)
                 .body(is("42"));
     }