From 45f875bd1c4b651f7a43617012a4a2c2ae786895 Mon Sep 17 00:00:00 2001 From: Erin Schnabel Date: Thu, 7 Nov 2019 16:44:08 -0500 Subject: [PATCH 1/2] hibernate validator: Remember annotated interface methods resolves #1888 --- .../HibernateValidatorProcessor.java | 21 +++++++++++++++++- ...MethodValidatedAnnotationsTransformer.java | 17 +++++++++++++- .../HibernateValidatorTestResource.java | 22 +++++++++++++++++++ .../hibernate/validator/ZipCodeService.java | 10 +++++++++ .../validator/ZipCodeServiceImpl.java | 12 ++++++++++ .../HibernateValidatorFunctionalityTest.java | 12 ++++++++++ 6 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeService.java create mode 100644 integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeServiceImpl.java diff --git a/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java b/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java index ee36facc0b8e3..9e94cbc02c547 100644 --- a/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java +++ b/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/HibernateValidatorProcessor.java @@ -5,8 +5,10 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Modifier; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Predicate; @@ -30,6 +32,7 @@ import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; +import org.jboss.jandex.MethodInfo; import org.jboss.jandex.Type; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; @@ -144,6 +147,7 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC consideredAnnotations.add(VALIDATE_ON_EXECUTION); Set classNamesToBeValidated = new HashSet<>(); + Map> inheritedAnnotationsToBeValidated = new HashMap<>(); for (DotName consideredAnnotation : consideredAnnotations) { Collection annotationInstances = indexView.getAnnotations(consideredAnnotation); @@ -160,6 +164,8 @@ 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, + annotation.target().asMethod()); } else if (annotation.target().kind() == AnnotationTarget.Kind.METHOD_PARAMETER) { contributeClass(classNamesToBeValidated, indexView, annotation.target().asMethodParameter().method().declaringClass().name()); @@ -168,6 +174,8 @@ 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, + annotation.target().asMethodParameter().method()); } else if (annotation.target().kind() == AnnotationTarget.Kind.CLASS) { contributeClass(classNamesToBeValidated, indexView, annotation.target().asClass().name()); // no need for reflection in the case of a class level constraint @@ -183,7 +191,8 @@ public void build(HibernateValidatorRecorder recorder, RecorderContext recorderC annotationsTransformers .produce(new AnnotationsTransformerBuildItem( new MethodValidatedAnnotationsTransformer(consideredAnnotations, - additionalJaxRsMethodAnnotationsDotNames))); + additionalJaxRsMethodAnnotationsDotNames, + inheritedAnnotationsToBeValidated))); Set> classesToBeValidated = new HashSet<>(); for (DotName className : classNamesToBeValidated) { @@ -245,6 +254,16 @@ private static void contributeClassMarkedForCascadingValidation(Set cla } } + private static void contributeMethodsWithInheritedValidation(Map> inheritedAnnotationsToBeValidated, + IndexView indexView, MethodInfo method) { + ClassInfo clazz = method.declaringClass(); + if (Modifier.isInterface(clazz.flags())) { + // Remember annotated interface methods that must be validated + inheritedAnnotationsToBeValidated.computeIfAbsent(clazz.name(), k -> new HashSet()) + .add(method.name().toString()); + } + } + private static DotName getClassName(Type type) { switch (type.kind()) { case CLASS: diff --git a/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java b/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java index 00e7fe1b9ebce..98dd70635b545 100644 --- a/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java +++ b/extensions/hibernate-validator/deployment/src/main/java/io/quarkus/hibernate/validator/deployment/MethodValidatedAnnotationsTransformer.java @@ -3,9 +3,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Map; import java.util.Set; import org.jboss.jandex.AnnotationTarget.Kind; +import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.MethodInfo; @@ -30,10 +32,14 @@ public class MethodValidatedAnnotationsTransformer implements AnnotationsTransfo private final Set consideredAnnotations; private final Collection effectiveJaxRsMethodDefiningAnnotations; + private final Map> inheritedAnnotationsToBeValidated; MethodValidatedAnnotationsTransformer(Set consideredAnnotations, - Collection additionalJaxRsMethodAnnotationsDotNames) { + Collection additionalJaxRsMethodAnnotationsDotNames, + Map> inheritedAnnotationsToBeValidated) { this.consideredAnnotations = consideredAnnotations; + this.inheritedAnnotationsToBeValidated = inheritedAnnotationsToBeValidated; + this.effectiveJaxRsMethodDefiningAnnotations = new ArrayList<>( JAXRS_METHOD_ANNOTATIONS.length + additionalJaxRsMethodAnnotationsDotNames.size()); effectiveJaxRsMethodDefiningAnnotations.addAll(Arrays.asList(JAXRS_METHOD_ANNOTATIONS)); @@ -60,6 +66,15 @@ public void transform(TransformationContext transformationContext) { private boolean requiresValidation(MethodInfo method) { if (method.annotations().isEmpty()) { + // This method has no annotations of its own: look for inherited annotations + ClassInfo clazz = method.declaringClass(); + String methodName = method.name().toString(); + for (Map.Entry> validatedMethod : inheritedAnnotationsToBeValidated.entrySet()) { + if (clazz.interfaceNames().contains(validatedMethod.getKey()) + && validatedMethod.getValue().contains(methodName)) { + return true; + } + } return false; } diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java index 5687450405037..20b6e60e93620 100644 --- a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java @@ -38,6 +38,9 @@ public class HibernateValidatorTestResource @Inject GreetingService greetingService; + @Inject + ZipCodeService zipCodeResource; + @GET @Path("/basic-features") @Produces(MediaType.TEXT_PLAIN) @@ -132,6 +135,25 @@ public String testInjection() { return result.build(); } + @GET + @Path("/test-inherited-constraints") + @Produces(MediaType.TEXT_PLAIN) + public String testInheritedConstraints() { + ResultBuilder result = new ResultBuilder(); + + zipCodeResource.echoZipCode("12345"); + + result.append(formatViolations(Collections.emptySet())); + + try { + zipCodeResource.echoZipCode("1234"); + } catch (ConstraintViolationException e) { + result.append(formatViolations(e.getConstraintViolations())); + } + + return result.build(); + } + private String formatViolations(Set> violations) { if (violations.isEmpty()) { return "passed"; diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeService.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeService.java new file mode 100644 index 0000000000000..6e48a72dab3af --- /dev/null +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeService.java @@ -0,0 +1,10 @@ +package io.quarkus.it.hibernate.validator; + +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Size; + +public interface ZipCodeService { + + public String echoZipCode(@NotNull @Size(min = 5, max = 5) String zipCode); + +} diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeServiceImpl.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeServiceImpl.java new file mode 100644 index 0000000000000..75f93ba0e6176 --- /dev/null +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/ZipCodeServiceImpl.java @@ -0,0 +1,12 @@ +package io.quarkus.it.hibernate.validator; + +import javax.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class ZipCodeServiceImpl implements ZipCodeService { + + @Override + public String echoZipCode(String zipCode) { + return zipCode; + } +} diff --git a/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java b/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java index 2c388e95a4bd8..0a279834e9a91 100644 --- a/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java +++ b/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java @@ -103,4 +103,16 @@ public void testInjection() throws Exception { .then() .body(is(expected.toString())); } + + @Test + public void testInheritedConstraints() { + StringBuilder expected = new StringBuilder(); + expected.append("passed").append("\n") + .append("failed: echoZipCode.arg0 (size must be between 5 and 5)"); + + RestAssured.when() + .get("/hibernate-validator/test/test-inherited-constraints") + .then() + .body(is(expected.toString())); + } } From 063a0cf5a0eb212005be531d87817fe80d6effeb Mon Sep 17 00:00:00 2001 From: Erin Schnabel Date: Mon, 11 Nov 2019 10:41:13 -0500 Subject: [PATCH 2/2] Add a test for a subclass method --- .../validator/EnhancedGreetingService.java | 16 ++++++++++++ .../hibernate/validator/GreetingService.java | 4 ++- .../HibernateValidatorTestResource.java | 26 +++++++++++++++++-- .../HibernateValidatorFunctionalityTest.java | 16 ++++++++++-- 4 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/EnhancedGreetingService.java diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/EnhancedGreetingService.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/EnhancedGreetingService.java new file mode 100644 index 0000000000000..a63703882965e --- /dev/null +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/EnhancedGreetingService.java @@ -0,0 +1,16 @@ +package io.quarkus.it.hibernate.validator; + +import javax.annotation.Priority; +import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.inject.Alternative; + +@Alternative +@Priority(1) +@ApplicationScoped +public class EnhancedGreetingService extends GreetingService { + + @Override + public String greeting(String name) { + return super.greeting("Enhanced " + name); + } +} diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/GreetingService.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/GreetingService.java index 485f79f90cbda..99ab24f1d4980 100644 --- a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/GreetingService.java +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/GreetingService.java @@ -1,13 +1,15 @@ package io.quarkus.it.hibernate.validator; +import javax.annotation.Priority; import javax.enterprise.context.ApplicationScoped; import javax.validation.constraints.NotNull; @ApplicationScoped +@Priority(2) public class GreetingService { public String greeting(@NotNull String name) { return "hello " + name; } -} \ No newline at end of file +} diff --git a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java index 20b6e60e93620..e452b17c0b3aa 100644 --- a/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java +++ b/integration-tests/hibernate-validator/src/main/java/io/quarkus/it/hibernate/validator/HibernateValidatorTestResource.java @@ -38,6 +38,9 @@ public class HibernateValidatorTestResource @Inject GreetingService greetingService; + @Inject + EnhancedGreetingService enhancedGreetingService; + @Inject ZipCodeService zipCodeResource; @@ -136,9 +139,9 @@ public String testInjection() { } @GET - @Path("/test-inherited-constraints") + @Path("/test-inherited-implements-constraints") @Produces(MediaType.TEXT_PLAIN) - public String testInheritedConstraints() { + public String testInheritedImplementsConstraints() { ResultBuilder result = new ResultBuilder(); zipCodeResource.echoZipCode("12345"); @@ -154,6 +157,25 @@ public String testInheritedConstraints() { return result.build(); } + @GET + @Path("/test-inherited-extends-constraints") + @Produces(MediaType.TEXT_PLAIN) + public String testInheritedExtendsConstraints() { + ResultBuilder result = new ResultBuilder(); + + enhancedGreetingService.greeting("test"); + + result.append(formatViolations(Collections.emptySet())); + + try { + enhancedGreetingService.greeting(null); + } catch (ConstraintViolationException e) { + result.append(formatViolations(e.getConstraintViolations())); + } + + return result.build(); + } + private String formatViolations(Set> violations) { if (violations.isEmpty()) { return "passed"; diff --git a/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java b/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java index 0a279834e9a91..5a239f730f2a2 100644 --- a/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java +++ b/integration-tests/hibernate-validator/src/test/java/io/quarkus/it/hibernate/validator/HibernateValidatorFunctionalityTest.java @@ -105,13 +105,25 @@ public void testInjection() throws Exception { } @Test - public void testInheritedConstraints() { + public void testInheritedImplementsConstraints() { StringBuilder expected = new StringBuilder(); expected.append("passed").append("\n") .append("failed: echoZipCode.arg0 (size must be between 5 and 5)"); RestAssured.when() - .get("/hibernate-validator/test/test-inherited-constraints") + .get("/hibernate-validator/test/test-inherited-implements-constraints") + .then() + .body(is(expected.toString())); + } + + @Test + public void testInheritedExtendsConstraints() { + StringBuilder expected = new StringBuilder(); + expected.append("passed").append("\n"); + expected.append("failed: greeting.arg0 (must not be null)"); + + RestAssured.when() + .get("/hibernate-validator/test/test-inherited-extends-constraints") .then() .body(is(expected.toString())); }