Simplify SelfValidatingValidator (#2413)

Remove use of Javassist from SelfValidatingValidator in favor of
plain old Java.

dropwizard-validation/src/main/java/io/dropwizard/validation/selfvalidating/SelfValidatingValidator.java

@@ -6,21 +6,16 @@
 import com.fasterxml.classmate.ResolvedTypeWithMembers;
 import com.fasterxml.classmate.TypeResolver;
 import com.fasterxml.classmate.members.ResolvedMethod;
-import javassist.ClassPool;
-import javassist.CtClass;
-import javassist.CtMethod;
-import javassist.NotFoundException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.validation.ConstraintValidator;
 import javax.validation.ConstraintValidatorContext;
-import java.lang.reflect.Modifier;
+import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 /**
@@ -29,11 +24,9 @@
  * the validation methods efficiently and then calls them.
  */
 public class SelfValidatingValidator implements ConstraintValidator<SelfValidating, Object> {
-
-    private static final AtomicInteger COUNTER = new AtomicInteger();
     private static final Logger log = LoggerFactory.getLogger(SelfValidatingValidator.class);
 
-    private final ConcurrentMap<Class<?>, List<ValidationCaller<?>>> methodMap = new ConcurrentHashMap<>();
+    private final ConcurrentMap<Class<?>, List<ValidationCaller>> methodMap = new ConcurrentHashMap<>();
     private final AnnotationConfiguration annotationConfiguration = new AnnotationConfiguration.StdConfiguration(AnnotationInclusion.INCLUDE_AND_INHERIT_IF_INHERITED);
     private final TypeResolver typeResolver = new TypeResolver();
     private final MemberResolver memberResolver = new MemberResolver(typeResolver);
@@ -59,37 +52,13 @@ public boolean isValid(Object value, ConstraintValidatorContext context) {
      * with <code>@SelfValidation</code> that adheres to required signature.
      */
     @SuppressWarnings("unchecked")
-    private List<ValidationCaller<?>> findMethods(Class<?> annotated) {
-        final ClassPool cp;
-        final CtClass callerSuperclass;
-        final CtClass[] callingParameters;
-        try {
-            cp = ClassPool.getDefault();
-            callerSuperclass = cp.get(ValidationCaller.class.getName());
-            callingParameters = new CtClass[]{cp.get(ViolationCollector.class.getName())};
-        } catch (NotFoundException e) {
-            throw new IllegalStateException("Failed to load included class", e);
-        }
-
+    private <T> List<ValidationCaller> findMethods(Class<T> annotated) {
         ResolvedTypeWithMembers annotatedType = memberResolver.resolve(typeResolver.resolve(annotated), annotationConfiguration, null);
-        final List<ValidationCaller<?>> callers = Arrays.stream(annotatedType.getMemberMethods())
+        final List<ValidationCaller> callers = Arrays.stream(annotatedType.getMemberMethods())
             .filter(this::isValidationMethod)
             .filter(this::isMethodCorrect)
-            .map(m -> {
-                try {
-                    CtClass cc = cp.makeClass("ValidationCallerGeneratedImpl" + COUNTER.getAndIncrement());
-                    cc.setSuperclass(callerSuperclass);
-
-                    CtMethod cm = new CtMethod(CtClass.voidType, "call", callingParameters, cc);
-                    cc.addMethod(cm);
-                    cm.setBody("{ return ((" + annotated.getName() + ")getValidationObject())." + m.getName() + "($1); }");
-
-                    cc.setModifiers(Modifier.PUBLIC);
-                    return (ValidationCaller<?>) cc.toClass().getConstructor().newInstance();
-                } catch (Exception e) {
-                    throw new IllegalStateException("Failed to generate ValidationCaller for method " + m, e);
-                }
-            }).collect(Collectors.toList());
+            .map(m -> new ProxyValidationCaller(annotated, m))
+            .collect(Collectors.toList());
         if (callers.isEmpty()) {
             log.warn("The class {} is annotated with @SelfValidating but contains no valid methods that are annotated " +
                 "with @SelfValidation", annotated);
@@ -100,7 +69,7 @@ private List<ValidationCaller<?>> findMethods(Class<?> annotated) {
     private boolean isValidationMethod(ResolvedMethod m) {
         return m.get(SelfValidation.class) != null;
     }
-    
+
     boolean isMethodCorrect(ResolvedMethod m) {
         if (m.getReturnType()!=null) {
             log.error("The method {} is annotated with @SelfValidation but does not return void. It is ignored", m.getRawMember());
@@ -115,4 +84,25 @@ boolean isMethodCorrect(ResolvedMethod m) {
         }
         return true;
     }
+
+    final class ProxyValidationCaller<T> extends ValidationCaller<T> {
+        private final Class<T> cls;
+        private final ResolvedMethod resolvedMethod;
+
+        ProxyValidationCaller(Class<T> cls, ResolvedMethod resolvedMethod) {
+            this.cls = cls;
+            this.resolvedMethod = resolvedMethod;
+        }
+
+        @Override
+        public void call(ViolationCollector vc) {
+            final Method method = resolvedMethod.getRawMember();
+            final T obj = cls.cast(getValidationObject());
+            try {
+                method.invoke(obj, vc);
+            } catch (ReflectiveOperationException e) {
+                throw new IllegalStateException("Couldn't call " + resolvedMethod + " on " + getValidationObject(), e);
+            }
+        }
+    }
 }

dropwizard-validation/src/test/java/io/dropwizard/validation/selfvalidating/SelfValidatingValidatorTest.java

@@ -1,93 +1,126 @@
 package io.dropwizard.validation.selfvalidating;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
-import java.util.Arrays;
-
-import org.junit.Test;
-
 import com.fasterxml.classmate.AnnotationConfiguration;
 import com.fasterxml.classmate.AnnotationInclusion;
 import com.fasterxml.classmate.MemberResolver;
 import com.fasterxml.classmate.ResolvedTypeWithMembers;
 import com.fasterxml.classmate.TypeResolver;
 import com.fasterxml.classmate.members.ResolvedMethod;
+import io.dropwizard.validation.BaseValidator;
+import org.junit.Test;
 
-public class SelfValidatingValidatorTest {
-
-    @SelfValidating
-    public static class InvalidExample {
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import java.util.Arrays;
+import java.util.Set;
 
-        @SelfValidation
-        public void validateCorrect(ViolationCollector col) {
-        }
+import static org.assertj.core.api.Assertions.assertThat;
 
-        @SelfValidation
-        public void validateFailAdditionalParameters(ViolationCollector col, int a) {
-        }
+public class SelfValidatingValidatorTest {
+    private SelfValidatingValidator selfValidatingValidator = new SelfValidatingValidator();
 
-        @SelfValidation
-        public boolean validateFailReturn(ViolationCollector col) {
-            return true;
-        }
+    @Test
+    public void validObjectHasNoViolations() throws Exception {
+        final Validator validator = BaseValidator.newValidator();
+        final Set<ConstraintViolation<ValidExample>> violations = validator.validate(new ValidExample(1));
+        assertThat(violations).isEmpty();
+    }
 
-        @SelfValidation
-        private void validateFailPrivate(ViolationCollector col) {
-        }
+    @Test
+    public void invalidObjectHasViolations() throws Exception {
+        final Validator validator = BaseValidator.newValidator();
+        final Set<ConstraintViolation<ValidExample>> violations = validator.validate(new ValidExample(-1));
+        assertThat(violations)
+                .isNotEmpty()
+                .allSatisfy(violation -> assertThat(violation.getMessage()).isEqualTo("n must be positive!"));
     }
-
-    private SelfValidatingValidator selfValidatingValidator = new SelfValidatingValidator();
 
     @Test
     public void correctMethod() throws Exception {
         assertThat(selfValidatingValidator.isMethodCorrect(
-            getMethod("validateCorrect", ViolationCollector.class)))
-            .isTrue();
+                getMethod("validateCorrect", ViolationCollector.class)))
+                .isTrue();
     }
 
     @Test
     public void voidIsNotAccepted() throws Exception {
         assertThat(selfValidatingValidator.isMethodCorrect(
-            getMethod("validateFailReturn", ViolationCollector.class)))
-            .isFalse();
+                getMethod("validateFailReturn", ViolationCollector.class)))
+                .isFalse();
     }
 
     @Test
     public void privateIsNotAccepted() throws Exception {
         assertThat(selfValidatingValidator.isMethodCorrect(
-            getMethod("validateFailPrivate", ViolationCollector.class)))
-            .isFalse();
+                getMethod("validateFailPrivate", ViolationCollector.class)))
+                .isFalse();
     }
 
     @Test
     public void additionalParametersAreNotAccepted() throws Exception {
         assertThat(selfValidatingValidator.isMethodCorrect(
-            getMethod("validateFailAdditionalParameters", ViolationCollector.class, int.class)))
-            .isFalse();
+                getMethod("validateFailAdditionalParameters", ViolationCollector.class, int.class)))
+                .isFalse();
     }
-    
+
     private ResolvedMethod getMethod(String name, Class<?>... params) {
         AnnotationConfiguration annotationConfiguration = new AnnotationConfiguration.StdConfiguration(AnnotationInclusion.INCLUDE_AND_INHERIT_IF_INHERITED);
         TypeResolver typeResolver = new TypeResolver();
         MemberResolver memberResolver = new MemberResolver(typeResolver);
         ResolvedTypeWithMembers annotatedType = memberResolver.resolve(typeResolver.resolve(InvalidExample.class), annotationConfiguration, null);
-        for(ResolvedMethod m : annotatedType.getMemberMethods()) {
-            if(hasSignature(m, name, params)) {
+        for (ResolvedMethod m : annotatedType.getMemberMethods()) {
+            if (hasSignature(m, name, params)) {
                 return m;
             }
         }
-        throw new IllegalStateException("Could not resolve method "+name+Arrays.toString(params)+" in "+InvalidExample.class);
+        throw new IllegalStateException("Could not resolve method " + name + Arrays.toString(params) + " in " + InvalidExample.class);
     }
 
     private boolean hasSignature(ResolvedMethod m, String name, Class<?>[] params) {
-        if(!m.getName().equals(name) || m.getArgumentCount() != params.length) {
+        if (!m.getName().equals(name) || m.getArgumentCount() != params.length) {
             return false;
         }
-        for(int i=0 ; i < params.length ; i++) {
-            if(!m.getArgumentType(i).getErasedType().equals(params[i]))
+        for (int i = 0; i < params.length; i++) {
+            if (!m.getArgumentType(i).getErasedType().equals(params[i]))
                 return false;
         }
         return true;
     }
 
+
+    @SelfValidating
+    static class InvalidExample {
+        @SelfValidation
+        public void validateCorrect(ViolationCollector col) {
+        }
+
+        @SelfValidation
+        public void validateFailAdditionalParameters(ViolationCollector col, int a) {
+        }
+
+        @SelfValidation
+        public boolean validateFailReturn(ViolationCollector col) {
+            return true;
+        }
+
+        @SelfValidation
+        private void validateFailPrivate(ViolationCollector col) {
+        }
+    }
+
+    @SelfValidating
+    static class ValidExample {
+        final int n;
+
+        ValidExample(int n) {
+            this.n = n;
+        }
+
+        @SelfValidation
+        public void validate(ViolationCollector col) {
+            if (n < 0) {
+                col.addViolation("n must be positive!");
+            }
+        }
+    }
 }