Merge pull request #13529 from gsmet/rework-inheritance

Rework constrained method inheritance for Hibernate Validator extension
quarkusio · Nov 28, 2020 · 0ebf0ff · 0ebf0ff
2 parents b7d41ce + ddea520
commit 0ebf0ff
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 21 deletions.
diff --git a/.../src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java b/.../src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java
@@ -232,7 +232,7 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC
                 allConsideredAnnotations));
 
         Set<DotName> classNamesToBeValidated = new HashSet<>();
-        Map<DotName, Set<SimpleMethodSignatureKey>> inheritedAnnotationsToBeValidated = new HashMap<>();
+        Map<DotName, Set<SimpleMethodSignatureKey>> methodsWithInheritedValidation = new HashMap<>();
         Set<String> detectedBuiltinConstraints = new HashSet<>();
 
         for (DotName consideredAnnotation : allConsideredAnnotations) {
@@ -261,7 +261,7 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC
                     reflectiveMethods.produce(new ReflectiveMethodBuildItem(annotation.target().asMethod()));
                     contributeClassMarkedForCascadingValidation(classNamesToBeValidated, indexView, consideredAnnotation,
                             annotation.target().asMethod().returnType());
-                    contributeMethodsWithInheritedValidation(inheritedAnnotationsToBeValidated, indexView,
+                    contributeMethodsWithInheritedValidation(methodsWithInheritedValidation, indexView,
                             annotation.target().asMethod());
                 } else if (annotation.target().kind() == AnnotationTarget.Kind.METHOD_PARAMETER) {
                     contributeClass(classNamesToBeValidated, indexView,
@@ -271,7 +271,7 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC
                             // FIXME this won't work in the case of synthetic parameters
                             annotation.target().asMethodParameter().method().parameters()
                                     .get(annotation.target().asMethodParameter().position()));
-                    contributeMethodsWithInheritedValidation(inheritedAnnotationsToBeValidated, indexView,
+                    contributeMethodsWithInheritedValidation(methodsWithInheritedValidation, indexView,
                             annotation.target().asMethodParameter().method());
                 } else if (annotation.target().kind() == AnnotationTarget.Kind.CLASS) {
                     contributeClass(classNamesToBeValidated, indexView, annotation.target().asClass().name());
@@ -292,7 +292,7 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC
                 .produce(new AnnotationsTransformerBuildItem(
                         new MethodValidatedAnnotationsTransformer(allConsideredAnnotations,
                                 jaxRsMethods,
-                                inheritedAnnotationsToBeValidated)));
+                                methodsWithInheritedValidation)));
 
         Set<Class<?>> classesToBeValidated = new HashSet<>();
         for (DotName className : classNamesToBeValidated) {
@@ -376,13 +376,23 @@ private static void contributeClassMarkedForCascadingValidation(Set<DotName> cla
     }
 
     private static void contributeMethodsWithInheritedValidation(
-            Map<DotName, Set<SimpleMethodSignatureKey>> inheritedAnnotationsToBeValidated,
+            Map<DotName, Set<SimpleMethodSignatureKey>> methodsWithInheritedValidation,
             IndexView indexView, MethodInfo method) {
         ClassInfo clazz = method.declaringClass();
+
+        methodsWithInheritedValidation.computeIfAbsent(clazz.name(), k -> new HashSet<>())
+                .add(new SimpleMethodSignatureKey(method));
+
         if (Modifier.isInterface(clazz.flags())) {
-            // Remember annotated interface methods that must be validated
-            inheritedAnnotationsToBeValidated.computeIfAbsent(clazz.name(), k -> new HashSet<>())
-                    .add(new SimpleMethodSignatureKey(method));
+            for (ClassInfo implementor : indexView.getAllKnownImplementors(clazz.name())) {
+                methodsWithInheritedValidation.computeIfAbsent(implementor.name(), k -> new HashSet<>())
+                        .add(new SimpleMethodSignatureKey(method));
+            }
+        } else {
+            for (ClassInfo subclass : indexView.getAllKnownSubclasses(clazz.name())) {
+                methodsWithInheritedValidation.computeIfAbsent(subclass.name(), k -> new HashSet<>())
+                        .add(new SimpleMethodSignatureKey(method));
+            }
         }
     }
 
@@ -410,6 +420,18 @@ private static Map<DotName, Set<SimpleMethodSignatureKey>> gatherJaxRsMethods(
                     MethodInfo method = annotation.target().asMethod();
                     jaxRsMethods.computeIfAbsent(method.declaringClass().name(), k -> new HashSet<>())
                             .add(new SimpleMethodSignatureKey(method));
+
+                    if (Modifier.isInterface(method.declaringClass().flags())) {
+                        for (ClassInfo implementor : indexView.getAllKnownImplementors(method.declaringClass().name())) {
+                            jaxRsMethods.computeIfAbsent(implementor.name(), k -> new HashSet<>())
+                                    .add(new SimpleMethodSignatureKey(method));
+                        }
+                    } else {
+                        for (ClassInfo subclass : indexView.getAllKnownSubclasses(method.declaringClass().name())) {
+                            jaxRsMethods.computeIfAbsent(subclass.name(), k -> new HashSet<>())
+                                    .add(new SimpleMethodSignatureKey(method));
+                        }
+                    }
                 }
             }
         }

diff --git a/...java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java b/...java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java
@@ -23,15 +23,15 @@ public class MethodValidatedAnnotationsTransformer implements AnnotationsTransfo
     private static final Logger LOGGER = Logger.getLogger(MethodValidatedAnnotationsTransformer.class.getPackage().getName());
 
     private final Set<DotName> consideredAnnotations;
-    private final Map<DotName, Set<SimpleMethodSignatureKey>> inheritedAnnotationsToBeValidated;
+    private final Map<DotName, Set<SimpleMethodSignatureKey>> methodsWithInheritedValidation;
     private final Map<DotName, Set<SimpleMethodSignatureKey>> jaxRsMethods;
 
     MethodValidatedAnnotationsTransformer(Set<DotName> consideredAnnotations,
             Map<DotName, Set<SimpleMethodSignatureKey>> jaxRsMethods,
-            Map<DotName, Set<SimpleMethodSignatureKey>> inheritedAnnotationsToBeValidated) {
+            Map<DotName, Set<SimpleMethodSignatureKey>> methodsWithInheritedValidation) {
         this.consideredAnnotations = consideredAnnotations;
         this.jaxRsMethods = jaxRsMethods;
-        this.inheritedAnnotationsToBeValidated = inheritedAnnotationsToBeValidated;
+        this.methodsWithInheritedValidation = methodsWithInheritedValidation;
     }
 
     @Override
@@ -67,18 +67,15 @@ private boolean requiresValidation(MethodInfo method) {
                 return true;
             }
         }
+
         // This method has no annotations of its own: look for inherited annotations
-        ClassInfo clazz = method.declaringClass();
-        SimpleMethodSignatureKey signatureKey = new SimpleMethodSignatureKey(method);
-        for (Map.Entry<DotName, Set<SimpleMethodSignatureKey>> validatedMethod : inheritedAnnotationsToBeValidated.entrySet()) {
-            DotName ifaceOrSuperClass = validatedMethod.getKey();
-            // note: only check the direct superclass since we do not (yet) have the entire ClassInfo hierarchy here
-            if ((clazz.interfaceNames().contains(ifaceOrSuperClass) || ifaceOrSuperClass.equals(clazz.superName()))
-                    && validatedMethod.getValue().contains(signatureKey)) {
-                return true;
-            }
+
+        Set<SimpleMethodSignatureKey> validatedMethods = methodsWithInheritedValidation.get(method.declaringClass().name());
+        if (validatedMethods == null || validatedMethods.isEmpty()) {
+            return false;
         }
-        return false;
+
+        return validatedMethods.contains(new SimpleMethodSignatureKey(method));
     }
 
     private boolean isJaxrsMethod(MethodInfo method) {

diff --git a/...e-validator/src/main/java/io/quarkus/it/hibernate/validator/inheritance/BookResource.java b/...e-validator/src/main/java/io/quarkus/it/hibernate/validator/inheritance/BookResource.java
@@ -0,0 +1,16 @@
+package io.quarkus.it.hibernate.validator.inheritance;
+
+import javax.validation.constraints.NotNull;
+import javax.ws.rs.GET;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
+
+public interface BookResource {
+
+    String PATH = "/books";
+
+    @GET
+    @Produces(MediaType.TEXT_PLAIN)
+    String hello(@NotNull @QueryParam("name") String name);
+}
diff --git a/...ator/src/main/java/io/quarkus/it/hibernate/validator/inheritance/ScienceBookResource.java b/...ator/src/main/java/io/quarkus/it/hibernate/validator/inheritance/ScienceBookResource.java
@@ -0,0 +1,10 @@
+package io.quarkus.it.hibernate.validator.inheritance;
+
+import javax.ws.rs.Path;
+
+@Path(ScienceBookResource.PATH)
+public interface ScienceBookResource extends BookResource {
+
+    String PATH = BookResource.PATH + "/science";
+
+}
diff --git a/.../src/main/java/io/quarkus/it/hibernate/validator/inheritance/ScienceBookResourceImpl.java b/.../src/main/java/io/quarkus/it/hibernate/validator/inheritance/ScienceBookResourceImpl.java
@@ -0,0 +1,9 @@
+package io.quarkus.it.hibernate.validator.inheritance;
+
+public class ScienceBookResourceImpl implements ScienceBookResource {
+
+    @Override
+    public String hello(String name) {
+        return "Hello " + name;
+    }
+}
diff --git a/.../src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java b/.../src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java
@@ -196,4 +196,13 @@ public void testHibernateOrmIntegration() {
                 .then()
                 .statusCode(500);
     }
+
+    @Test
+    public void testInheritance() {
+        RestAssured.given()
+                .get("/books/science")
+                .then()
+                .statusCode(400)
+                .body(containsString("must not be null"));
+    }
 }