properly implement method inheritance rules wrt ValidateOnExecution
diff --git a/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java b/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java
index 4735369..042fa7e 100644
--- a/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java
+++ b/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java
@@ -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 @@
if (classConfiguration == null) {
synchronized (this) {
if (classConfiguration == null) {
- final ValidateOnExecution annotation = CDI.current().getBeanManager()
- .createAnnotatedType(targetClass).getAnnotation(ValidateOnExecution.class);
+ final AnnotatedType<?> annotatedType = CDI.current().getBeanManager()
+ .createAnnotatedType(targetClass);
- if (annotation == null) {
- classConfiguration = globalConfiguration.getGlobalExecutableTypes();
+ 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 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;
}
}
}
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
index b449a6b..da84490 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
@@ -129,6 +129,7 @@
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);
@@ -136,8 +137,12 @@
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 @@
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;
}
}
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
index 956f937..18dfadc 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
@@ -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 @@
}
}
- 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 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 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 @@
}
@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 @@
}
@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 @@
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() {
}
}
