Merge pull request #393 from gsmet/method-validation-take-2

Some improvements to method validation

bean-validation/deployment/src/main/java/org/jboss/shamrock/beanvalidation/MethodValidatedAnnotationsTransformer.java

@@ -14,7 +14,15 @@
  */
 public class MethodValidatedAnnotationsTransformer implements AnnotationsTransformer {
 
-    private static final DotName JAXRS_PATH = DotName.createSimple("javax.ws.rs.Path");
+    private static final DotName[] JAXRS_METHOD_ANNOTATIONS = {
+            DotName.createSimple("javax.ws.rs.GET"),
+            DotName.createSimple("javax.ws.rs.HEAD"),
+            DotName.createSimple("javax.ws.rs.DELETE"),
+            DotName.createSimple("javax.ws.rs.OPTIONS"),
+            DotName.createSimple("javax.ws.rs.PATCH"),
+            DotName.createSimple("javax.ws.rs.POST"),
+            DotName.createSimple("javax.ws.rs.PUT"),
+    };
 
     private final Set<DotName> consideredAnnotations;
 
@@ -32,7 +40,7 @@ public void transform(TransformationContext transformationContext) {
         MethodInfo method = transformationContext.getTarget().asMethod();
 
         if (requiresValidation(method)) {
-            if (method.hasAnnotation(JAXRS_PATH) || method.declaringClass().annotations().containsKey(JAXRS_PATH)) {
+            if (isJaxrsMethod(method)) {
                 transformationContext.transform().add(DotName.createSimple(JaxrsEndPointValidated.class.getName())).done();
             } else {
                 transformationContext.transform().add(DotName.createSimple(MethodValidated.class.getName())).done();
@@ -53,4 +61,13 @@ private boolean requiresValidation(MethodInfo method) {
 
         return false;
     }
+
+    private boolean isJaxrsMethod(MethodInfo method) {
+        for (DotName jaxrsMethodAnnotation : JAXRS_METHOD_ANNOTATIONS) {
+            if (method.hasAnnotation(jaxrsMethodAnnotation)) {
+                return true;
+            }
+        }
+        return false;
+    }
 }

build-parent/pom.xml

@@ -100,6 +100,7 @@
       <postgresql-jdbc.version>42.2.5</postgresql-jdbc.version>
       <mariadb-jdbc.version>2.3.0</mariadb-jdbc.version>
       <shrinkwrap.version>1.2.6</shrinkwrap.version>
+      <rest-assured.version>3.2.0</rest-assured.version>
    </properties>
 
    <dependencyManagement>
@@ -525,6 +526,18 @@
             <artifactId>opentracing-tracerresolver</artifactId>
             <version>${opentracing-tracerresolver.version}</version>
          </dependency>
+         <dependency>
+            <groupId>io.rest-assured</groupId>
+            <artifactId>rest-assured</artifactId>
+            <version>${rest-assured.version}</version>
+            <scope>test</scope>
+            <exclusions>
+                <exclusion>
+                    <groupId>javax.activation</groupId>
+                    <artifactId>activation</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
          <dependency>
             <groupId>io.undertow</groupId>
             <artifactId>undertow-servlet</artifactId>

docs/src/main/asciidoc/json-and-validation-guide.adoc

@@ -115,15 +115,16 @@ Add the following method:
 
 [source, java]
 ----
+@Path("/manual-validation")
 @POST
 @Produces(MediaType.APPLICATION_JSON)
 @Consumes(MediaType.APPLICATION_JSON)
-public Result tryMe(Book book) {
+public Result tryMeManualValidation(Book book) {
     Set<ConstraintViolation<Book>> violations = validator.validate(book);
     Result res = new Result();
     if (violations.isEmpty()) {
         res.success = true;
-        res.message = "woohoo!";
+        res.message = "Book is valid! It was validated by manual validation.";
     } else {
         res.success = false;
         res.message = violations.stream()
@@ -174,6 +175,85 @@ private class Result {
 The class is very simple and only contains 2 fields and the associated getters and setters.
 Because we indicate that we produce JSON, the mapping to JSON is made automatically.
 
+== REST end point validation
+
+While using the `Validator` manually might be useful for some advanced usage,
+if you simply want to validate the parameters or the return value or your REST end point,
+you can annotate it directly, either with constraints (`@NotNull`, `@Digits`...)
+or with `@Valid` (which will cascade the validation to the bean).
+
+Let's create an end point validating the `Book` provided in the request:
+
+[source, java]
+----
+@Path("/end-point-method-validation")
+@POST
+@Produces(MediaType.APPLICATION_JSON)
+@Consumes(MediaType.APPLICATION_JSON)
+public Result tryMeEndPointMethodValidation(@Valid Book book) {
+    Result res = new Result();
+    res.success = true;
+    res.message = "Book is valid! It was validated by end point method validation.";
+    return res;
+}
+----
+
+As you can see, we don't have to manually validate the provided `Book` anymore as it is automatically validated.
+
+If a validation error is triggered, a violation report is generated and serialized as JSON as our end point produces a JSON output.
+It can be extracted and manipulated to display a proper error message.
+
+== Service method validation
+
+It might not always be handy to have the validation rules declared at the end point level as it could duplicate some business validation.
+
+The best option is then to annotate a method of your business service with your constraints (or in our particular case with `@Valid`):
+
+[source, java]
+----
+@ApplicationScoped
+public class BookService {
+
+    public void validateBook(@Valid Book book) {
+        // your business logic here
+    }
+}
+----
+
+Calling the service in your rest end point triggers the `Book` validation automatically:
+
+[source, java]
+----
+@Path("/service-method-validation")
+@POST
+@Produces(MediaType.APPLICATION_JSON)
+@Consumes(MediaType.APPLICATION_JSON)
+public Result tryMeServiceMethodValidation(Book book) {
+    Result res = new Result();
+
+    try {
+        bookService.validateBook(book);
+
+        res.success = true;
+        res.message = "Book is valid! It was validated by service method validation.";
+    }
+    catch (ConstraintViolationException e) {
+        res.success = false;
+        res.message = e.getConstraintViolations().stream()
+            .map(cv -> cv.getMessage())
+            .collect(Collectors.joining(", "));
+    }
+
+    return res;
+}
+----
+
+Note that, if you want to push the validation errors to the frontend, you have to catch the exception and push the information yourselves
+as they will not be automatically pushed to the JSON output.
+
+Keep in mind that you usually don't want to expose to the public the internals of your services
+- and especially not the validated value contained in the violation object.
+
 == A frontend
 
 Now let's add the simple web page to interact with our `BookResource`.
@@ -188,10 +268,10 @@ Now, let's see our application in action. Run it with:
 mvn compile shamrock:dev
 ```
 
-Then, open your browser to http://localhost:8080
+Then, open your browser to http://localhost:8080/:
 
 1. Enter the book details (valid or invalid)
-2. Click on the _try me_ button to check if your data is valid.
+2. Click on the _Try me..._ buttons to check if your data is valid using one of the methods we presented above.
 
 image:validation-guide-screenshot.png[alt=Application,width=800]
 

examples/bean-validation-strict/pom.xml

@@ -48,6 +48,11 @@
             <artifactId>javax.json</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.rest-assured</groupId>
+            <artifactId>rest-assured</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

examples/bean-validation-strict/src/test/java/org/jboss/shamrock/example/test/BeanValidationFunctionalityTest.java

@@ -1,12 +1,16 @@
 package org.jboss.shamrock.example.test;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
 
 import org.jboss.shamrock.test.ShamrockTest;
 import org.jboss.shamrock.test.URLTester;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import io.restassured.RestAssured;
+
 /**
  * Test various Bean Validation operations running in Shamrock
  */
@@ -23,7 +27,10 @@ public void testBasicFeatures() throws Exception {
                 .append("score (must be greater than or equal to 0)").append("\n");
         expected.append("passed");
 
-        assertEquals(expected.toString(), URLTester.relative("bean-validation/test/basic-features").invokeURL().asString());
+        RestAssured.when()
+                .get("/bean-validation/test/basic-features")
+                .then()
+                        .body(is(expected.toString()));
     }
 
     @Test
@@ -32,7 +39,10 @@ public void testCustomClassLevelConstraint() throws Exception {
         expected.append("failed:  (invalid MyOtherBean)").append("\n");
         expected.append("passed");
 
-        assertEquals(expected.toString(), URLTester.relative("bean-validation/test/custom-class-level-constraint").invokeURL().asString());
+        RestAssured.when()
+                .get("/bean-validation/test/custom-class-level-constraint")
+                .then()
+                        .body(is(expected.toString()));
     }
 
     @Test
@@ -41,14 +51,25 @@ public void testCDIBeanMethodValidation() {
         expected.append("passed").append("\n");
         expected.append("failed: greeting.arg0 (must not be null)");
 
-        assertEquals(expected.toString(), URLTester.relative("bean-validation/test/cdi-bean-method-validation").invokeURL().asString());
+        RestAssured.when()
+                .get("/bean-validation/test/cdi-bean-method-validation")
+                .then()
+                        .body(is(expected.toString()));
     }
 
     @Test
     public void testRestEndPointValidation() {
         // we can't test the content of the response as accessing the input stream throws an IOException
         assertEquals(400, URLTester.relative("bean-validation/test/rest-end-point-validation/plop/").invokeURL().statusCode());
+        RestAssured.when()
+                .get("/bean-validation/test/rest-end-point-validation/plop/")
+                .then()
+                        .statusCode(400)
+                        .body(containsString("numeric value out of bounds"));
 
-        assertEquals("42", URLTester.relative("bean-validation/test/rest-end-point-validation/42/").invokeURL().asString() );
+        RestAssured.when()
+                .get("/bean-validation/test/rest-end-point-validation/42/")
+                .then()
+                        .body(is("42"));
     }
 }

jaxrs/deployment/src/main/java/org/jboss/shamrock/jaxrs/JaxrsScanningProcessor.java

@@ -53,6 +53,8 @@
 import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
+import org.jboss.resteasy.api.validation.ResteasyConstraintViolation;
+import org.jboss.resteasy.api.validation.ViolationReport;
 import org.jboss.resteasy.core.MediaTypeMap;
 import org.jboss.resteasy.plugins.interceptors.AcceptEncodingGZIPFilter;
 import org.jboss.resteasy.plugins.interceptors.GZIPDecodingInterceptor;
@@ -139,7 +141,7 @@ public class JaxrsScanningProcessor {
 	 * If the resource class has an explicit CDI scope annotation then the value of
 	 * this annotation will always be used to control the lifecycle of the resource
 	 * class.
-	 * 
+	 *
 	 * IMPLEMENTATION NOTE: {@code javax.ws.rs.Path} turns into a CDI stereotype
 	 * with singleton scope. As a result, if a user annotates a JAX-RS resource with
 	 * a stereotype which has a different default scope the deployment fails with
@@ -302,6 +304,10 @@ public void build(BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
                 }
             }
         }
+
+        // In the case of a constraint violation, these elements might be returned as entities and will be serialized
+        reflectiveClass.produce(new ReflectiveClassBuildItem(true, true, ViolationReport.class.getName()));
+        reflectiveClass.produce(new ReflectiveClassBuildItem(true, true, ResteasyConstraintViolation.class.getName()));
     }
 
     @BuildStep