Merge pull request #1039 from nickbabcock/constraint-messages

Descriptive constraint violation messages

dropwizard-benchmarks/src/main/java/io/dropwizard/benchmarks/jersey/ConstraintViolationBenchmark.java

@@ -0,0 +1,84 @@
+package io.dropwizard.benchmarks.jersey;
+
+import io.dropwizard.jersey.validation.ConstraintMessage;
+import org.hibernate.validator.constraints.NotEmpty;
+import org.openjdk.jmh.annotations.*;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Valid;
+import javax.validation.Validation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.ws.rs.HeaderParam;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.commons.lang3.reflect.MethodUtils.getAccessibleMethod;
+
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Benchmark)
+public class ConstraintViolationBenchmark {
+
+    public static class Resource {
+        public String paramFunc(@HeaderParam("cheese") @NotEmpty String secretSauce) {
+            return secretSauce;
+        }
+
+        public String objectFunc(@Valid Foo foo) {
+            return foo.toString();
+        }
+    }
+
+    public static class Foo {
+        @NotEmpty
+        private String bar;
+    }
+
+    private ConstraintViolation<ConstraintViolationBenchmark.Resource> paramViolation;
+    private ConstraintViolation<ConstraintViolationBenchmark.Resource> objViolation;
+
+    @Setup
+    public void prepare() {
+        final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
+        final ExecutableValidator execValidator = validator.forExecutables();
+
+        Set<ConstraintViolation<ConstraintViolationBenchmark.Resource>> paramViolations =
+            execValidator.validateParameters(
+                new Resource(),
+                getAccessibleMethod(ConstraintViolationBenchmark.Resource.class, "paramFunc", String.class),
+                new Object[]{""} // the parameter value
+            );
+        paramViolation = paramViolations.iterator().next();
+
+        Set<ConstraintViolation<ConstraintViolationBenchmark.Resource>> objViolations =
+            execValidator.validateParameters(
+                new Resource(),
+                getAccessibleMethod(ConstraintViolationBenchmark.Resource.class, "objectFunc", Foo.class),
+                new Object[]{new Foo()} // the parameter value
+            );
+        objViolation = objViolations.iterator().next();
+    }
+
+    @Benchmark
+    public String paramViolation() {
+        return ConstraintMessage.getMessage(paramViolation);
+    }
+
+    @Benchmark
+    public String objViolation() {
+        return ConstraintMessage.getMessage(objViolation);
+    }
+
+    public static void main(String[] args) throws Exception {
+        new Runner(new OptionsBuilder()
+                .include(ConstraintViolationBenchmark.class.getSimpleName())
+                .forks(1)
+                .warmupIterations(5)
+                .measurementIterations(5)
+                .build())
+                .run();
+    }
+}

dropwizard-jersey/pom.xml

@@ -126,5 +126,9 @@
             <artifactId>jetty-continuation</artifactId>
             <version>${jetty.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
     </dependencies>
 </project>

dropwizard-jersey/src/main/java/io/dropwizard/jersey/validation/ConstraintMessage.java

@@ -0,0 +1,127 @@
+package io.dropwizard.jersey.validation;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
+import io.dropwizard.validation.ConstraintViolations;
+import io.dropwizard.validation.ValidationMethod;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.commons.lang3.reflect.MethodUtils;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.ElementKind;
+import javax.validation.Path;
+import javax.ws.rs.*;
+import javax.ws.rs.core.Context;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.concurrent.ConcurrentMap;
+
+public class ConstraintMessage {
+    private static final ConcurrentMap<Path, String> reflectCache = Maps.newConcurrentMap();
+
+    /**
+     * Gets the human friendly location of where the violation was raised.
+     */
+    public static String getMessage(ConstraintViolation<?> v) {
+        if (reflectCache.containsKey(v.getPropertyPath())) {
+            return reflectCache.get(v.getPropertyPath());
+        }
+
+        String message = calculateMessage(v);
+        reflectCache.put(v.getPropertyPath(), message);
+        return message;
+    }
+
+    private static String calculateMessage(ConstraintViolation<?> v) {
+        final Optional<String> returnValueName = getMethodReturnValueName(v);
+        if (returnValueName.isPresent()) {
+            final String name = isValidationMethod(v) ?
+                    StringUtils.substringBeforeLast(returnValueName.get(), ".") : returnValueName.get();
+            return name + " " + v.getMessage();
+        } else if (isValidationMethod(v)) {
+            return ConstraintViolations.validationMethodFormatted(v);
+        } else {
+            final String name = getMemberName(v).or(v.getPropertyPath().toString());
+            return name + " " + v.getMessage();
+        }
+    }
+
+    /**
+     * Gets a method parameter (or a parameter field) name, if the violation raised in it.
+     */
+    private static Optional<String> getMemberName(ConstraintViolation<?> violation) {
+        final int size = Iterables.size(violation.getPropertyPath());
+        if (size < 2) {
+            return Optional.absent();
+        }
+
+        final Path.Node parent = Iterables.get(violation.getPropertyPath(), size - 2);
+        final Path.Node member = Iterables.getLast(violation.getPropertyPath());
+        final Class<?> resourceClass = violation.getLeafBean().getClass();
+        switch (parent.getKind()) {
+            case PARAMETER:
+                Field field = FieldUtils.getDeclaredField(resourceClass, member.getName(), true);
+                return getMemberName(field.getDeclaredAnnotations());
+            case METHOD:
+                List<Class<?>> params = parent.as(Path.MethodNode.class).getParameterTypes();
+                Class<?>[] parcs = params.toArray(new Class<?>[params.size()]);
+                Method method = MethodUtils.getAccessibleMethod(resourceClass, parent.getName(), parcs);
+
+                int paramIndex = member.as(Path.ParameterNode.class).getParameterIndex();
+                return getMemberName(method.getParameterAnnotations()[paramIndex]);
+            default:
+                return Optional.absent();
+        }
+    }
+
+    /**
+     * Gets the method return value name, if the violation is raised in it
+     */
+    private static Optional<String> getMethodReturnValueName(ConstraintViolation<?> violation) {
+        int returnValueNames = -1;
+
+        final StringBuilder result = new StringBuilder("server response");
+        for (Path.Node node : violation.getPropertyPath()) {
+            if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
+                returnValueNames = 0;
+            } else if (returnValueNames >= 0) {
+                result.append(returnValueNames++ == 0 ? " " : ".").append(node);
+            }
+        }
+
+        return returnValueNames >= 0 ? Optional.of(result.toString()) : Optional.<String>absent();
+    }
+
+    /**
+     * Derives member's name and type from it's annotations
+     */
+    private static Optional<String> getMemberName(Annotation[] memberAnnotations) {
+        for (Annotation a : memberAnnotations) {
+            if (a instanceof QueryParam) {
+                return Optional.of("query param " + ((QueryParam) a).value());
+            } else if (a instanceof PathParam) {
+                return Optional.of("path param " + ((PathParam) a).value());
+            } else if (a instanceof HeaderParam) {
+                return Optional.of("header " + ((HeaderParam) a).value());
+            } else if (a instanceof CookieParam) {
+                return Optional.of("cookie " + ((CookieParam) a).value());
+            } else if (a instanceof FormParam) {
+                return Optional.of("form field " + ((FormParam) a).value());
+            } else if (a instanceof Context) {
+                return Optional.of("context");
+            } else if (a instanceof MatrixParam) {
+                return Optional.of("matrix param " + ((MatrixParam) a).value());
+            }
+        }
+
+        return Optional.absent();
+    }
+
+    private static boolean isValidationMethod(ConstraintViolation<?> v) {
+        return v.getConstraintDescriptor().getAnnotation() instanceof ValidationMethod;
+    }
+}

dropwizard-jersey/src/main/java/io/dropwizard/jersey/validation/ConstraintViolationExceptionMapper.java

@@ -1,21 +1,31 @@
 package io.dropwizard.jersey.validation;
 
+import com.google.common.base.Function;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
 import io.dropwizard.validation.ConstraintViolations;
 
 import javax.validation.ConstraintViolation;
 import javax.validation.ConstraintViolationException;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.ExceptionMapper;
 import javax.ws.rs.ext.Provider;
-import java.util.Set;
 
 @Provider
 public class ConstraintViolationExceptionMapper implements ExceptionMapper<ConstraintViolationException> {
+
     @Override
     public Response toResponse(ConstraintViolationException exception) {
-        final ValidationErrorMessage message = new ValidationErrorMessage(exception.getConstraintViolations());
+        final ImmutableList<String> errors = FluentIterable.from(exception.getConstraintViolations())
+                .transform(new Function<ConstraintViolation<?>, String>() {
+                    @Override
+                    public String apply(ConstraintViolation<?> v) {
+                        return ConstraintMessage.getMessage(v);
+                    }
+                }).toList();
+
         return Response.status(ConstraintViolations.determineStatus(exception.getConstraintViolations()))
-                       .entity(message)
-                       .build();
+                .entity(new ValidationErrorMessage(errors))
+                .build();
     }
 }

dropwizard-jersey/src/main/java/io/dropwizard/jersey/validation/ValidationErrorMessage.java

@@ -2,16 +2,12 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.collect.ImmutableList;
-import io.dropwizard.validation.ConstraintViolations;
-
-import javax.validation.ConstraintViolation;
-import java.util.Set;
 
 public class ValidationErrorMessage {
     private final ImmutableList<String> errors;
 
-    public ValidationErrorMessage(Set<ConstraintViolation<?>> errors) {
-        this.errors = ConstraintViolations.formatUntyped(errors);
+    public ValidationErrorMessage(ImmutableList<String> errors) {
+        this.errors = errors;
     }
 
     @JsonProperty