Skip to content

Commit

Permalink
improved self validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@manuel-hegner
manuel-hegner committed May 3, 2018
1 parent ac6d3fd commit 1bb0cfa
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 38 deletions.
12 changes: 6 additions & 6 deletions dropwizard-validation/pom.xml
View file
Expand Up @@ -46,14 +46,14 @@
<artifactId>javassist</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
<version>1.7.25</version>
<scope>test</scope>
<groupId>uk.org.lidalia</groupId>
<artifactId>slf4j-test</artifactId>
<version>1.2.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
40 changes: 29 additions & 11 deletions ...dation/src/main/java/io/dropwizard/validation/selfvalidating/SelfValidatingValidator.java
View file
@@ -1,7 +1,16 @@
package io.dropwizard.validation.selfvalidating;

import com.fasterxml.classmate.AnnotationConfiguration;
import com.fasterxml.classmate.AnnotationInclusion;
import com.fasterxml.classmate.MemberResolver;
import com.fasterxml.classmate.ResolvedType;
import com.fasterxml.classmate.ResolvedTypeWithMembers;
import com.fasterxml.classmate.TypeResolver;
import com.fasterxml.classmate.members.RawMethod;
import com.fasterxml.classmate.members.ResolvedMethod;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Maps;

import javassist.ClassPool;
import javassist.CtClass;
import javassist.CtMethod;
Expand All @@ -17,6 +26,7 @@
import java.util.List;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.IntPredicate;
import java.util.stream.Collectors;

/**
Expand All @@ -30,6 +40,9 @@ public class SelfValidatingValidator implements ConstraintValidator<SelfValidati
private static final Logger log = LoggerFactory.getLogger(SelfValidatingValidator.class);

private final ConcurrentMap<Class<?>, List<ValidationCaller<?>>> methodMap = Maps.newConcurrentMap();
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);

@Override
public void initialize(SelfValidating constraintAnnotation) {
Expand Down Expand Up @@ -64,9 +77,10 @@ private List<ValidationCaller<?>> findMethods(Class<?> annotated) {
throw new IllegalStateException("Failed to load included class", e);
}

final List<ValidationCaller<?>> callers = Arrays.stream(annotated.getDeclaredMethods())
.filter(m -> m.isAnnotationPresent(SelfValidation.class))
.filter(this::isCorrectMethod)
ResolvedTypeWithMembers annotatedType = memberResolver.resolve(typeResolver.resolve(annotated), annotationConfiguration, null);
final List<ValidationCaller<?>> callers = Arrays.stream(annotatedType.getMemberMethods())
.filter(this::isValidationMethod)
.filter(this::isMethodCorrect)
.map(m -> {
try {
CtClass cc = cp.makeClass("ValidationCallerGeneratedImpl" + COUNTER.getAndIncrement());
Expand All @@ -83,23 +97,27 @@ private List<ValidationCaller<?>> findMethods(Class<?> annotated) {
}
}).collect(Collectors.toList());
if (callers.isEmpty()) {
log.error("The class {} is annotated with @SelfValidating but contains no valid methods that are annotated " +
log.warn("The class {} is annotated with @SelfValidating but contains no valid methods that are annotated " +
"with @SelfValidation", annotated);
}
return callers;
}

private boolean isValidationMethod(ResolvedMethod m) {
return m.get(SelfValidation.class) != null;
}

@VisibleForTesting
boolean isCorrectMethod(Method m) {
if (!void.class.equals(m.getReturnType())) {
log.error("The method {} is annotated with @SelfValidation but does not return void. It is ignored", m);
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());
return false;
} else if (m.getParameterTypes().length != 1 || !m.getParameterTypes()[0].equals(ViolationCollector.class)) {
} else if (m.getArgumentCount() != 1 || !m.getArgumentType(0).getErasedType().equals(ViolationCollector.class)) {
log.error("The method {} is annotated with @SelfValidation but does not have a single parameter of type {}",
m, ViolationCollector.class);
m.getRawMember(), ViolationCollector.class);
return false;
} else if (!Modifier.isPublic(m.getModifiers())) {
log.error("The method {} is annotated with @SelfValidation but is not public", m);
} else if (!m.isPublic()) {
log.error("The method {} is annotated with @SelfValidation but is not public", m.getRawMember());
return false;
}
return true;
Expand Down
2 changes: 2 additions & 0 deletions ...zard-validation/src/main/java/io/dropwizard/validation/selfvalidating/SelfValidation.java
View file
Expand Up @@ -2,6 +2,7 @@

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
Expand All @@ -15,5 +16,6 @@
@Documented
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@Inherited //used by classmate reflection
public @interface SelfValidation {
}
126 changes: 115 additions & 11 deletions dropwizard-validation/src/test/java/io/dropwizard/validation/SelfValidationTest.java
View file
@@ -1,17 +1,32 @@
package io.dropwizard.validation;

import io.dropwizard.validation.selfvalidating.SelfValidating;
import io.dropwizard.validation.selfvalidating.SelfValidation;
import io.dropwizard.validation.selfvalidating.ViolationCollector;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThat;

import javax.annotation.concurrent.NotThreadSafe;
import javax.validation.Validator;

import static org.assertj.core.api.Assertions.assertThat;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import io.dropwizard.validation.selfvalidating.SelfValidating;
import io.dropwizard.validation.selfvalidating.SelfValidation;
import io.dropwizard.validation.selfvalidating.ViolationCollector;
import uk.org.lidalia.slf4jext.Level;
import uk.org.lidalia.slf4jtest.LoggingEvent;
import uk.org.lidalia.slf4jtest.TestLoggerFactory;

@NotThreadSafe
public class SelfValidationTest {

private static final String FAILED = "failed";
private static final String FAILED_RESULT = " " + FAILED;

@Before @After
public void clearAllLoggers() {
//this must be a clear all because the validation runs in other threads
TestLoggerFactory.clearAll();
}

@SelfValidating
public static class FailingExample {
Expand All @@ -20,6 +35,27 @@ public void validateFail(ViolationCollector col) {
col.addViolation(FAILED);
}
}

public static class SubclassExample extends FailingExample {
@SelfValidation
public void subValidateFail(ViolationCollector col) {
col.addViolation(FAILED+"subclass");
}
}

@SelfValidating
public static class AnnotatedSubclassExample extends FailingExample {
@SelfValidation
public void subValidateFail(ViolationCollector col) {
col.addViolation(FAILED+"subclass");
}
}

public static class OverridingExample extends FailingExample {
@Override
public void validateFail(ViolationCollector col) {
}
}

@SelfValidating
public static class DirectContextExample {
Expand Down Expand Up @@ -96,13 +132,47 @@ public static class NoValidations {
@Test
public void failingExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new FailingExample())))
.containsOnly(" " + FAILED);
.containsExactlyInAnyOrder(FAILED_RESULT);
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void subClassExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new SubclassExample())))
.containsExactlyInAnyOrder(
FAILED_RESULT,
FAILED_RESULT+"subclass"
);
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void annotatedSubClassExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new AnnotatedSubclassExample())))
.containsExactlyInAnyOrder(
FAILED_RESULT,
FAILED_RESULT+"subclass"
);
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void overridingSubClassExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new OverridingExample())))
.isEmpty();
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void correctExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new CorrectExample())))
.isEmpty();
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
Expand All @@ -111,33 +181,67 @@ public void multipleTestingOfSameClass() throws Exception {
.isEmpty();
assertThat(ConstraintViolations.format(validator.validate(new CorrectExample())))
.isEmpty();
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void testDirectContextUsage() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new DirectContextExample())))
.containsOnly(" " + FAILED);
.containsExactlyInAnyOrder(FAILED_RESULT);
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void complexExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new ComplexExample())))
.containsOnly(
" " + FAILED + "1",
" " + FAILED + "2",
" " + FAILED + "3"
.containsExactlyInAnyOrder(
FAILED_RESULT + "1",
FAILED_RESULT + "2",
FAILED_RESULT + "3"
);
assertThat(TestLoggerFactory.getAllLoggingEvents())
.isEmpty();
}

@Test
public void invalidExample() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new InvalidExample())))
.isEmpty();
assertThat(TestLoggerFactory.getAllLoggingEvents())
.containsExactlyInAnyOrder(
new LoggingEvent(
Level.ERROR,
"The method {} is annotated with @SelfValidation but does not have a single parameter of type {}",
InvalidExample.class.getMethod("validateFailAdditionalParameters", ViolationCollector.class, int.class),
ViolationCollector.class
),
new LoggingEvent(
Level.ERROR,
"The method {} is annotated with @SelfValidation but does not return void. It is ignored",
InvalidExample.class.getMethod("validateFailReturn", ViolationCollector.class)
),
new LoggingEvent(
Level.ERROR,
"The method {} is annotated with @SelfValidation but is not public",
InvalidExample.class.getDeclaredMethod("validateFailPrivate", ViolationCollector.class)
)
);
}

@Test
public void giveWarningIfNoValidationMethods() throws Exception {
assertThat(ConstraintViolations.format(validator.validate(new NoValidations())))
.isEmpty();
assertThat(TestLoggerFactory.getAllLoggingEvents())
.containsExactlyInAnyOrder(
new LoggingEvent(
Level.WARN,
"The class {} is annotated with @SelfValidating but contains no valid methods that are annotated with @SelfValidation",
NoValidations.class
)

);
}
}
53 changes: 43 additions & 10 deletions ...on/src/test/java/io/dropwizard/validation/selfvalidating/SelfValidatingValidatorTest.java
View file
@@ -1,8 +1,17 @@
package io.dropwizard.validation.selfvalidating;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.Arrays;

import org.junit.Test;

import static org.assertj.core.api.Assertions.assertThat;
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;

public class SelfValidatingValidatorTest {

Expand All @@ -26,35 +35,59 @@ public boolean validateFailReturn(ViolationCollector col) {
private void validateFailPrivate(ViolationCollector col) {
}
}

private SelfValidatingValidator selfValidatingValidator = new SelfValidatingValidator();

@Test
public void correctMethod() throws Exception {
assertThat(selfValidatingValidator.isCorrectMethod(InvalidExample.class
.getDeclaredMethod("validateCorrect", ViolationCollector.class)))
assertThat(selfValidatingValidator.isMethodCorrect(
getMethod("validateCorrect", ViolationCollector.class)))
.isTrue();
}

@Test
public void voidIsNotAccepted() throws Exception {
assertThat(selfValidatingValidator.isCorrectMethod(InvalidExample.class
.getDeclaredMethod("validateFailReturn", ViolationCollector.class)))
assertThat(selfValidatingValidator.isMethodCorrect(
getMethod("validateFailReturn", ViolationCollector.class)))
.isFalse();
}

@Test
public void privateIsNotAccepted() throws Exception {
assertThat(selfValidatingValidator.isCorrectMethod(InvalidExample.class
.getDeclaredMethod("validateFailPrivate", ViolationCollector.class)))
assertThat(selfValidatingValidator.isMethodCorrect(
getMethod("validateFailPrivate", ViolationCollector.class)))
.isFalse();
}

@Test
public void additionalParametersAreNotAccepted() throws Exception {
assertThat(selfValidatingValidator.isCorrectMethod(InvalidExample.class
.getDeclaredMethod("validateFailAdditionalParameters", ViolationCollector.class, int.class)))
assertThat(selfValidatingValidator.isMethodCorrect(
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)) {
return m;
}
}
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) {
return false;
}
for(int i=0 ; i < params.length ; i++) {
if(!m.getArgumentType(i).getErasedType().equals(params[i]))
return false;
}
return true;
}

}
1 change: 1 addition & 0 deletions dropwizard-validation/src/test/resources/slf4jtest.properties
View file
@@ -0,0 +1 @@
print.level=INFO

0 comments on commit 1bb0cfa

Please sign in to comment.