Skip to content

Commit

Permalink
Simplify concurrency handling in SelfValidatingValidator (#2201)
Browse files Browse the repository at this point in the history 
Instead of synchronizing on "methodMap", we could use a `ConcurrentMap`
and the `putIfAbsent` method, because a cashable value depends only
from the key.
  • Loading branch information
@arteam
arteam committed Nov 10, 2017
1 parent a1f4b33 commit 5e11718
Showing 1 changed file with 51 additions and 64 deletions.
115 changes: 51 additions & 64 deletions ...dation/src/main/java/io/dropwizard/validation/selfvalidating/SelfValidatingValidator.java
View file
@@ -1,10 +1,11 @@
package io.dropwizard.validation.selfvalidating; package io.dropwizard.validation.selfvalidating;


import com.google.common.collect.Maps;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;


import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidator;
Expand All @@ -27,88 +28,74 @@ public class SelfValidatingValidator implements ConstraintValidator<SelfValidati


private static final Logger LOG = LoggerFactory.getLogger(SelfValidatingValidator.class); private static final Logger LOG = LoggerFactory.getLogger(SelfValidatingValidator.class);
private static final AtomicInteger COUNTER = new AtomicInteger(); private static final AtomicInteger COUNTER = new AtomicInteger();

private final HashMap<Class<?>, List<ValidationCaller<?>>> methodMap = new HashMap<>(); private final ConcurrentMap<Class<?>, List<ValidationCaller<?>>> methodMap = Maps.newConcurrentMap();

@Override @Override
public void initialize(SelfValidating constraintAnnotation) {} public void initialize(SelfValidating constraintAnnotation) {
}


@SuppressWarnings({ "rawtypes", "unchecked" }) @SuppressWarnings({"rawtypes", "unchecked"})
@Override @Override
public boolean isValid(Object value, ConstraintValidatorContext context) { public boolean isValid(Object value, ConstraintValidatorContext context) {
List<ValidationCaller<?>> callers = findMethods(value.getClass()); List<ValidationCaller<?>> callers = methodMap.computeIfAbsent(value.getClass(), this::findMethods);


ViolationCollector collector = new ViolationCollector(context); ViolationCollector collector = new ViolationCollector(context);
context.disableDefaultConstraintViolation(); context.disableDefaultConstraintViolation();
for (ValidationCaller caller:callers) { for (ValidationCaller caller : callers) {
caller.setValidationObject(value); caller.setValidationObject(value);
caller.call(collector); caller.call(collector);
} }
return !collector.hasViolationOccurred(); return !collector.hasViolationOccurred();
} }


/** /**
* This method generates and caches <code>ValidationCaller</code>s for each method annotated * This method generates <code>ValidationCaller</code>s for each method annotated
* with <code>@SelfValidation</code> that adheres to required signature. * with <code>@SelfValidation</code> that adheres to required signature.
* @param annotated
* @return
*/ */
private synchronized List<ValidationCaller<?>> findMethods(Class<? extends Object> annotated) { private List<ValidationCaller<?>> findMethods(Class<?> annotated) {
if (methodMap.containsKey(annotated)) List<ValidationCaller<?>> l = new ArrayList<>();
return methodMap.get(annotated);

ClassPool cp;
synchronized (methodMap) { CtClass callerSuperclass;
if (methodMap.containsKey(annotated)) CtClass[] callingParameters;
return methodMap.get(annotated); try {

cp = ClassPool.getDefault();
List<ValidationCaller<?>> l = new ArrayList<>(); callerSuperclass = cp.get(ValidationCaller.class.getName());

callingParameters = new CtClass[]{cp.get(ViolationCollector.class.getName())};
ClassPool cp; } catch (NotFoundException e) {
CtClass callerSuperclass; throw new IllegalStateException("Failed to load included class", e);
CtClass[] callingParameters; }
try {
cp = ClassPool.getDefault(); for (Method m : annotated.getMethods()) {
callerSuperclass = cp.get(ValidationCaller.class.getName()); if (m.isAnnotationPresent(SelfValidation.class)) {
callingParameters = new CtClass[] {cp.get(ViolationCollector.class.getName())}; if (!void.class.equals(m.getReturnType()))
} catch (NotFoundException e) { LOG.error("The method {} is annotated with SelfValidation but does not return void. It is ignored.", m);
throw new IllegalStateException("Failed to load included class", e); else if (m.getParameterTypes().length != 1 || !m.getParameterTypes()[0].equals(ViolationCollector.class))
} LOG.error("The method {} is annotated with SelfValidation but does not have a single parameter of type {}", m, ViolationCollector.class);

else if ((m.getModifiers() & Modifier.PUBLIC) == 0)
for (Method m:annotated.getMethods()) { LOG.error("The method {} is annotated with SelfValidation but is not public", m);
if (m.isAnnotationPresent(SelfValidation.class)) { else {
if (!void.class.equals(m.getReturnType())) try {
LOG.error("The method {} is annotated with SelfValidation but does not return void. It is ignored.", m); CtClass cc = cp.makeClass("ValidationCallerGeneratedImpl" + COUNTER.getAndIncrement());
else if (m.getParameterTypes().length != 1 || !m.getParameterTypes()[0].equals(ViolationCollector.class)) cc.setSuperclass(callerSuperclass);
LOG.error("The method {} is annotated with SelfValidation but does not have a single parameter of type {}", m, ViolationCollector.class);
else if ((m.getModifiers() & Modifier.PUBLIC) == 0) CtMethod method = new CtMethod(CtClass.voidType, "call", callingParameters, cc);
LOG.error("The method {} is annotated with SelfValidation but is not public", m); cc.addMethod(method);
else { method.setBody("{ return ((" + annotated.getName() + ")getValidationObject())." + m.getName() + "($1); }");
try {
CtClass cc = cp.makeClass("ValidationCallerGeneratedImpl" + COUNTER.getAndIncrement()); cc.setModifiers(Modifier.PUBLIC);
cc.setSuperclass(callerSuperclass); @SuppressWarnings("unchecked")

ValidationCaller<?> caller = (ValidationCaller<?>) cc.toClass().getConstructor().newInstance();
CtMethod method = new CtMethod(CtClass.voidType, "call", callingParameters, cc); l.add(caller);
cc.addMethod(method); } catch (Exception e) {
method.setBody("{ return ((" + annotated.getName() + ")getValidationObject())." + m.getName() + "($1); }"); LOG.error("Failed to generate ValidationCaller for method " + m.toString(), e);

cc.setModifiers(Modifier.PUBLIC);
@SuppressWarnings("unchecked")
ValidationCaller<?> caller = (ValidationCaller<?>) cc.toClass().getConstructor().newInstance();
l.add(caller);
} catch (Exception e) {
LOG.error("Failed to generate ValidationCaller for method " + m.toString(), e);
}
} }
} }
} }
if (l.isEmpty())
LOG.error("The class {} is annotated with SelfValidating but contains no valid methods that are annotated with SelfValidation", annotated);

methodMap.put(annotated, l);
return l;
} }

if (l.isEmpty())

LOG.error("The class {} is annotated with SelfValidating but contains no valid methods that are annotated with SelfValidation", annotated);
return l;
} }

} }

0 comments on commit 5e11718

Please sign in to comment.