Repository: bval
Updated Branches:
  refs/heads/bv2 9019464db -> 15b87ac22


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/15b87ac2
Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/15b87ac2
Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/15b87ac2

Branch: refs/heads/bv2
Commit: 15b87ac223c837f0fffbeb1da3a2fe46352d1e26
Parents: 9019464
Author: Matt Benson <mben...@apache.org>
Authored: Thu Apr 12 18:02:37 2018 -0500
Committer: Matt Benson <mben...@apache.org>
Committed: Thu Apr 12 18:02:37 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/15b87ac2/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/15b87ac2/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/15b87ac2/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() {
     }
 }

Reply via email to