properly implement method inheritance rules wrt ValidateOnExecution
Project: http://git-wip-us.apache.org/repos/asf/bval/repo Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/99c0079e Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/99c0079e Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/99c0079e Branch: refs/heads/bv2 Commit: 99c0079e3fd24f3e255b1e8d6633f46ade4515ed Parents: 3b30b86 Author: Matt Benson <[email protected]> Authored: Thu Apr 12 18:02:37 2018 -0500 Committer: Matt Benson <[email protected]> Committed: Tue Oct 16 12:28:20 2018 -0500 ---------------------------------------------------------------------- .../org/apache/bval/cdi/BValInterceptor.java | 92 +++++++++++++------- .../bval/jsr/metadata/HierarchyBuilder.java | 21 +++-- .../org/apache/bval/jsr/metadata/Liskov.java | 33 ++++--- 3 files changed, 97 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/bval/blob/99c0079e/bval-jsr/src/main/java/org/apache/bval/cdi/BValInterceptor.java ---------------------------------------------------------------------- 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.Executable; 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.metadata.Signature; 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 @@ import org.apache.bval.util.reflection.TypeUtils; // 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 @@ public class BValInterceptor implements Serializable { 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 @@ public class BValInterceptor implements Serializable { } 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; } } } http://git-wip-us.apache.org/repos/asf/bval/blob/99c0079e/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java ---------------------------------------------------------------------- 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 @@ public class HierarchyBuilder extends CompositeBuilder { 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 @@ public class HierarchyBuilder extends CompositeBuilder { 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 class HierarchyBuilder extends CompositeBuilder { 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; } } http://git-wip-us.apache.org/repos/asf/bval/blob/99c0079e/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java ---------------------------------------------------------------------- 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.Collection; 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 java.util.stream.Stream; 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 @@ import org.apache.bval.util.Validate; 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 @@ class Liskov { } } - 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 @@ class Liskov { } } - 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 @@ class Liskov { } } - 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 @@ class Liskov { } @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 @@ class Liskov { } @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 @@ class Liskov { 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() { } }
