Skip to content
Permalink
Browse files
properly implement method inheritance rules wrt ValidateOnExecution
  • Loading branch information
mbenson committed Oct 16, 2018
1 parent 3b30b86 commit 99c0079e3fd24f3e255b1e8d6633f46ade4515ed
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 49 deletions.
@@ -24,6 +24,8 @@
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -55,9 +57,10 @@
import org.apache.bval.jsr.util.ExecutableTypes;
import org.apache.bval.jsr.util.Methods;
import org.apache.bval.jsr.util.Proxies;
import org.apache.bval.util.ObjectUtils;
import org.apache.bval.util.Validate;
import org.apache.bval.util.reflection.Reflection;
import org.apache.bval.util.reflection.Reflection.Interfaces;
import org.apache.bval.util.reflection.TypeUtils;

/**
* Interceptor class for the {@link BValBinding} {@link InterceptorBinding}.
@@ -69,6 +72,20 @@
// TODO: maybe add it through ASM to be compliant with CDI 1.0 containers using simply this class as a template to
// generate another one for CDI 1.1 impl
public class BValInterceptor implements Serializable {
private static Collection<ExecutableType> removeFrom(Collection<ExecutableType> coll,
ExecutableType... executableTypes) {
Validate.notNull(coll, "collection was null");
if (!(coll.isEmpty() || ObjectUtils.isEmptyArray(executableTypes))) {
final List<ExecutableType> toRemove = Arrays.asList(executableTypes);
if (!Collections.disjoint(coll, toRemove)) {
final Set<ExecutableType> result = EnumSet.copyOf(coll);
result.removeAll(toRemove);
return result;
}
}
return coll;
}

private transient volatile Set<ExecutableType> classConfiguration;
private transient volatile Map<Signature, Boolean> executableValidation;

@@ -174,13 +191,16 @@ private void initClassConfig(Class<?> targetClass) {
if (classConfiguration == null) {
synchronized (this) {
if (classConfiguration == null) {
final ValidateOnExecution annotation = CDI.current().getBeanManager()
.createAnnotatedType(targetClass).getAnnotation(ValidateOnExecution.class);

if (annotation == null) {
classConfiguration = globalConfiguration.getGlobalExecutableTypes();
final AnnotatedType<?> annotatedType = CDI.current().getBeanManager()
.createAnnotatedType(targetClass);

if (annotatedType.isAnnotationPresent(ValidateOnExecution.class)) {
// implicit does not apply at the class level:
classConfiguration = ExecutableTypes.interpret(
removeFrom(Arrays.asList(annotatedType.getAnnotation(ValidateOnExecution.class).type()),
ExecutableType.IMPLICIT));
} else {
classConfiguration = ExecutableTypes.interpret(annotation.type());
classConfiguration = globalConfiguration.getGlobalExecutableTypes();
}
}
}
@@ -203,41 +223,49 @@ private <T> boolean computeIsConstructorValidated(Class<T> targetClass, Construc
}

private <T> boolean computeIsMethodValidated(Class<T> targetClass, Method method) {
Collection<ExecutableType> declaredExecutableTypes = null;
final Signature signature = Signature.of(method);

AnnotatedMethod<?> declaringMethod = null;

for (final Class<?> c : Reflection.hierarchy(targetClass, Interfaces.INCLUDE)) {
final AnnotatedType<?> annotatedType = CDI.current().getBeanManager().createAnnotatedType(c);

final AnnotatedMethod<?> annotatedMethod = annotatedType.getMethods().stream()
.filter(am -> Signature.of(am.getJavaMember()).equals(Signature.of(method))).findFirst().orElse(null);
.filter(am -> Signature.of(am.getJavaMember()).equals(signature)).findFirst().orElse(null);

if (annotatedMethod == null) {
continue;
if (annotatedMethod != null) {
declaringMethod = annotatedMethod;
}
if (annotatedMethod.isAnnotationPresent(ValidateOnExecution.class)) {
final List<ExecutableType> validatedTypesOnMethod =
Arrays.asList(annotatedMethod.getAnnotation(ValidateOnExecution.class).type());
}
if (declaringMethod == null) {
return false;
}
final Collection<ExecutableType> declaredExecutableTypes;

// implicit directly on method -> early return:
if (validatedTypesOnMethod.contains(ExecutableType.IMPLICIT)) {
return true;
}
declaredExecutableTypes = validatedTypesOnMethod;
// ignore the hierarchy once the lowest method is found:
break;
if (declaringMethod.isAnnotationPresent(ValidateOnExecution.class)) {
final List<ExecutableType> validatedTypesOnMethod =
Arrays.asList(declaringMethod.getAnnotation(ValidateOnExecution.class).type());

// implicit directly on method -> early return:
if (validatedTypesOnMethod.contains(ExecutableType.IMPLICIT)) {
return true;
}
if (declaredExecutableTypes == null) {
if (annotatedType.isAnnotationPresent(ValidateOnExecution.class)) {
declaredExecutableTypes =
Arrays.asList(annotatedType.getAnnotation(ValidateOnExecution.class).type());
declaredExecutableTypes = validatedTypesOnMethod;
} else {
final AnnotatedType<?> declaringType = declaringMethod.getDeclaringType();
if (declaringType.isAnnotationPresent(ValidateOnExecution.class)) {
// IMPLICIT is meaningless at class level:
declaredExecutableTypes =
removeFrom(Arrays.asList(declaringType.getAnnotation(ValidateOnExecution.class).type()),
ExecutableType.IMPLICIT);
} else {
final Package pkg = declaringType.getJavaClass().getPackage();
if (pkg != null && pkg.isAnnotationPresent(ValidateOnExecution.class)) {
// presumably IMPLICIT is likewise meaningless at package level:
declaredExecutableTypes = removeFrom(
Arrays.asList(pkg.getAnnotation(ValidateOnExecution.class).type()), ExecutableType.IMPLICIT);
} else {
final Optional<Package> pkg = Optional.of(annotatedType).map(AnnotatedType::getBaseType)
.map(t -> TypeUtils.getRawType(t, null)).map(Class::getPackage)
.filter(p -> p.isAnnotationPresent(ValidateOnExecution.class));
if (pkg.isPresent()) {
declaredExecutableTypes =
Arrays.asList(pkg.get().getAnnotation(ValidateOnExecution.class).type());
}
declaredExecutableTypes = null;
}
}
}
@@ -129,15 +129,20 @@ public Map<String, MetadataBuilder.ForContainer<Method>> getGetters(Meta<Class<T
return getters;
}
final Map<String, MetadataBuilder.ForContainer<Method>> result = new LinkedHashMap<>();
final List<ContainerDelegate<Method>> delegates = new ArrayList<>();

getters.forEach((k, v) -> {
final Method getter = Methods.getter(hierarchyElement.getHost(), k);

Exceptions.raiseIf(getter == null, IllegalStateException::new,
"delegate builder specified unknown getter");

result.put(k, new ContainerDelegate<Method>(v, new Meta.ForMethod(getter)));
final ContainerDelegate<Method> d = new ContainerDelegate<>(v, new Meta.ForMethod(getter));
result.put(k, d);
delegates.add(d);
});
Liskov.validateValidateOnExecution(delegates);

return result;
}

@@ -148,11 +153,17 @@ public Map<Signature, MetadataBuilder.ForExecutable<Method>> getMethods(Meta<Cla
return methods;
}
final Map<Signature, MetadataBuilder.ForExecutable<Method>> result = new LinkedHashMap<>();
methods
.forEach((k, v) -> result.put(k,
new ExecutableDelegate<>(v, new Meta.ForMethod(
final List<ExecutableDelegate<Method>> delegates = new ArrayList<>();
methods.forEach((k, v) -> {
final ExecutableDelegate<Method> d = new ExecutableDelegate<>(v,
new Meta.ForMethod(
Reflection.getDeclaredMethod(hierarchyElement.getHost(), k.getName(), k.getParameterTypes())),
ParameterNameProvider::getParameterNames)));
ParameterNameProvider::getParameterNames);
result.put(k, d);
delegates.add(d);
});
Liskov.validateValidateOnExecution(delegates);

return result;
}
}
@@ -21,7 +21,6 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -34,6 +33,7 @@
import javax.validation.ConstraintDeclarationException;
import javax.validation.ElementKind;
import javax.validation.Valid;
import javax.validation.executable.ValidateOnExecution;

import org.apache.bval.jsr.metadata.HierarchyBuilder.ContainerDelegate;
import org.apache.bval.jsr.metadata.HierarchyBuilder.ElementDelegate;
@@ -44,7 +44,7 @@
class Liskov {
//@formatter:off
private enum ValidationElement {
constraints, cascades, groupConversions;
constraints, cascades, groupConversions, validateOnExecution;
}

private enum StrengtheningIssue implements Predicate<Map<Meta<?>, Set<ValidationElement>>> {
@@ -106,19 +106,19 @@ void check(Map<Meta<?>, Set<ValidationElement>> detectedValidationElements) {
}
}

static void validateContainerHierarchy(List<? extends ContainerDelegate<?>> delegates, ElementKind elementKind) {
static void validateContainerHierarchy(Collection<? extends ContainerDelegate<?>> delegates, ElementKind elementKind) {
if (Validate.notNull(delegates, "delegates").isEmpty()) {
return;
}
if (Validate.notNull(elementKind, "elementKind") == ElementKind.CONTAINER_ELEMENT) {
elementKind = getContainer(delegates.get(0).getHierarchyElement());
elementKind = getContainer(delegates.iterator().next().getHierarchyElement());
}
switch (elementKind) {
case RETURN_VALUE:
noRedeclarationOfReturnValueCascading(delegates);

final Map<Meta<?>, Set<ValidationElement>> detectedValidationElements =
detectValidationElements(delegates, detectGroupConversion());
detectValidationElements(delegates, ElementDelegate::getHierarchyElement, detectGroupConversion());

// pre-check return value overridden hierarchy:
Stream.of(StrengtheningIssue.values())
@@ -135,13 +135,17 @@ static void validateContainerHierarchy(List<? extends ContainerDelegate<?>> dele
}
}

static void validateCrossParameterHierarchy(List<? extends ElementDelegate<?, ?>> delegates) {
static void validateCrossParameterHierarchy(Collection<? extends ElementDelegate<?, ?>> delegates) {
if (Validate.notNull(delegates, "delegates").isEmpty()) {
return;
}
noStrengtheningOfPreconditions(delegates, detectConstraints());
}

static void validateValidateOnExecution(Collection<? extends HierarchyDelegate<?, ?>> delegates) {
noStrengtheningOfPreconditions(delegates, detectValidateOnExecution());
}

private static ElementKind getContainer(Meta<?> meta) {
Meta<?> m = meta;
while (m.getElementType() == ElementType.TYPE_USE) {
@@ -157,7 +161,7 @@ private static ElementKind getContainer(Meta<?> meta) {
}
}

private static void noRedeclarationOfReturnValueCascading(List<? extends ContainerDelegate<?>> delegates) {
private static void noRedeclarationOfReturnValueCascading(Collection<? extends ContainerDelegate<?>> delegates) {
final Map<Class<?>, Meta<?>> cascadedReturnValues =
delegates.stream().filter(ContainerDelegate::isCascade).map(HierarchyDelegate::getHierarchyElement)
.collect(Collectors.toMap(Meta::getDeclaringClass, Function.identity()));
@@ -171,11 +175,11 @@ private static void noRedeclarationOfReturnValueCascading(List<? extends Contain
}

@SafeVarargs
private static <D extends ElementDelegate<?, ?>> void noStrengtheningOfPreconditions(List<? extends D> delegates,
private static <D extends HierarchyDelegate<?, ?>> void noStrengtheningOfPreconditions(Collection<? extends D> delegates,
Function<? super D, ValidationElement>... detectors) {

final Map<Meta<?>, Set<ValidationElement>> detectedValidationElements =
detectValidationElements(delegates, detectors);
detectValidationElements(delegates, HierarchyDelegate::getHierarchyElement, detectors);

if (detectedValidationElements.isEmpty()) {
return;
@@ -186,11 +190,11 @@ private static void noRedeclarationOfReturnValueCascading(List<? extends Contain
}

@SafeVarargs
private static <D extends ElementDelegate<?, ?>> Map<Meta<?>, Set<ValidationElement>> detectValidationElements(
List<? extends D> delegates, Function<? super D, ValidationElement>... detectors) {
private static <T> Map<Meta<?>, Set<ValidationElement>> detectValidationElements(Collection<? extends T> delegates,
Function<? super T, Meta<?>> toMeta, Function<? super T, ValidationElement>... detectors) {
final Map<Meta<?>, Set<ValidationElement>> detectedValidationElements = new LinkedHashMap<>();
delegates.forEach(d -> {
detectedValidationElements.put(d.getHierarchyElement(),
detectedValidationElements.put(toMeta.apply(d),
Stream.of(detectors).map(dt -> dt.apply(d)).filter(Objects::nonNull)
.collect(Collectors.toCollection(() -> EnumSet.noneOf(ValidationElement.class))));
});
@@ -217,6 +221,11 @@ private static Function<ContainerDelegate<?>, ValidationElement> detectGroupConv
return d -> d.getGroupConversions().isEmpty() ? null : ValidationElement.groupConversions;
}

private static Function<HierarchyDelegate<?, ?>, ValidationElement> detectValidateOnExecution() {
return d -> d.getHierarchyElement().getHost().isAnnotationPresent(ValidateOnExecution.class)
? ValidationElement.validateOnExecution : null;
}

private Liskov() {
}
}

0 comments on commit 99c0079

Please sign in to comment.