You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bval.apache.org by mb...@apache.org on 2018/04/02 21:05:23 UTC

bval git commit: constraint definition validation

Repository: bval
Updated Branches:
  refs/heads/bv2 005b7d83e -> 26f8d1403


constraint definition validation


Project: http://git-wip-us.apache.org/repos/asf/bval/repo
Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/26f8d140
Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/26f8d140
Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/26f8d140

Branch: refs/heads/bv2
Commit: 26f8d1403151a457a94563ebe8041960a4b0dcd6
Parents: 005b7d8
Author: Matt Benson <mb...@apache.org>
Authored: Mon Apr 2 16:00:08 2018 -0500
Committer: Matt Benson <mb...@apache.org>
Committed: Mon Apr 2 16:00:08 2018 -0500

----------------------------------------------------------------------
 .../jsr/ConstraintAnnotationAttributes.java     | 63 ++++++++++++++------
 .../apache/bval/jsr/descriptor/ConstraintD.java |  1 +
 .../bval/jsr/metadata/ReflectionBuilder.java    |  2 +-
 .../bval/jsr/util/AnnotationsManager.java       | 50 ++++++++++++++++
 .../bval/jsr/xml/AnnotationProxyBuilder.java    | 29 +++------
 .../java/org/apache/bval/util/ObjectUtils.java  |  8 +--
 .../apache/bval/util/reflection/TypeUtils.java  |  2 +-
 7 files changed, 108 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintAnnotationAttributes.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintAnnotationAttributes.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintAnnotationAttributes.java
index d9a1c69..6c285d5 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintAnnotationAttributes.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintAnnotationAttributes.java
@@ -16,22 +16,28 @@
  */
 package org.apache.bval.jsr;
 
-import org.apache.bval.util.Exceptions;
-import org.apache.bval.util.reflection.Reflection;
-import org.apache.bval.util.reflection.TypeUtils;
-import org.apache.commons.weaver.privilizer.Privilizing;
-import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
-
-import javax.validation.Constraint;
-import javax.validation.ConstraintTarget;
-import javax.validation.Payload;
-
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
+import java.util.Collections;
+import java.util.EnumSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.function.Predicate;
+
+import javax.validation.Constraint;
+import javax.validation.ConstraintTarget;
+import javax.validation.Payload;
+
+import org.apache.bval.util.Exceptions;
+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.TypeUtils;
+import org.apache.commons.weaver.privilizer.Privilizing;
+import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
 
 /**
  * Defines the well-known attributes of {@link Constraint} annotations.
@@ -43,27 +49,27 @@ public enum ConstraintAnnotationAttributes {
     /**
      * "message"
      */
-    MESSAGE("message"),
+    MESSAGE("message", m -> true),
 
     /**
      * "groups"
      */
-    GROUPS("groups"),
+    GROUPS("groups", ObjectUtils::isEmptyArray),
 
     /**
      * "payload"
      */
-    PAYLOAD("payload"),
+    PAYLOAD("payload", ObjectUtils::isEmptyArray),
 
     /**
      * "validationAppliesTo"
      */
-    VALIDATION_APPLIES_TO("validationAppliesTo"),
+    VALIDATION_APPLIES_TO("validationAppliesTo", Predicate.isEqual(ConstraintTarget.IMPLICIT)),
 
     /**
      * "value" for multi-valued constraints
      */
-    VALUE("value");
+    VALUE("value", ObjectUtils::isEmptyArray);
 
     @SuppressWarnings("unused")
     private static class Types {
@@ -74,23 +80,29 @@ public enum ConstraintAnnotationAttributes {
         ConstraintTarget validationAppliesTo;
     }
 
-    private final Type type;
+    private static final Set<ConstraintAnnotationAttributes> MANDATORY =
+            Collections.unmodifiableSet(EnumSet.of(ConstraintAnnotationAttributes.MESSAGE,
+                ConstraintAnnotationAttributes.GROUPS, ConstraintAnnotationAttributes.PAYLOAD));
+
+    private final Class<?> type;
     private final String attributeName;
+    private final Predicate<Object> validateDefaultValue;
 
-    private ConstraintAnnotationAttributes(final String name) {
+    private ConstraintAnnotationAttributes(final String name, Predicate<Object> validateDefaultValue) {
         this.attributeName = name;
         try {
-            this.type = Types.class.getDeclaredField(getAttributeName()).getGenericType();
+            this.type = Types.class.getDeclaredField(getAttributeName()).getType();
         } catch (Exception e) {
             // should never happen
             throw new RuntimeException(e);
         }
+        this.validateDefaultValue = Validate.notNull(validateDefaultValue, "validateDefaultValue");
     }
 
     /**
      * Get the expected type of the represented attribute.
      */
-    public Type getType() {
+    public Class<?> getType() {
         return type;
     }
 
@@ -103,6 +115,11 @@ public enum ConstraintAnnotationAttributes {
         return attributeName;
     }
 
+    @Override
+    public String toString() {
+        return attributeName;
+    }
+
     /**
      * Put <code>value</code> into a map with <code>this.attributeName</code> as
      * key.
@@ -141,6 +158,14 @@ public enum ConstraintAnnotationAttributes {
         return new Worker<C>(clazz);
     }
 
+    public boolean isMandatory() {
+        return MANDATORY.contains(this);
+    }
+
+    public boolean isValidDefaultValue(Object o) {
+        return validateDefaultValue.test(o);
+    }
+
     // this is static but related to Worker
     private static final ConcurrentMap<Class<?>, Worker<?>> WORKER_CACHE = new ConcurrentHashMap<>();
     private static final ConcurrentMap<Class<?>, ConcurrentMap<String, Method>> METHOD_BY_NAME_AND_CLASS =

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java
index caa8d57..a8f95ed 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java
@@ -87,6 +87,7 @@ public class ConstraintD<A extends Annotation> implements ConstraintDescriptor<A
 
     public ConstraintD(A annotation, Scope scope, Meta<?> meta, ApacheValidatorFactory validatorFactory) {
         this.annotation = Validate.notNull(annotation, "annotation");
+        validatorFactory.getAnnotationsManager().validateConstraintDefinition(annotation.annotationType());
         this.scope = Validate.notNull(scope, "scope");
         this.meta = Validate.notNull(meta, "meta");
 

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/ReflectionBuilder.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/ReflectionBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/ReflectionBuilder.java
index df8319b..0a70b7c 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/ReflectionBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/ReflectionBuilder.java
@@ -300,7 +300,7 @@ public class ReflectionBuilder {
 
         private Map<ConstraintTarget, List<Annotation>> getConstraintsByTarget() {
             final Annotation[] declaredConstraints = AnnotationsManager.getDeclaredConstraints(meta);
-            if (ObjectUtils.isEmpty(declaredConstraints)) {
+            if (ObjectUtils.isEmptyArray(declaredConstraints)) {
                 return Collections.emptyMap();
             }
             final Map<ConstraintTarget, List<Annotation>> result = new EnumMap<>(ConstraintTarget.class);

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
index 3b41ec9..38d73e3 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
@@ -27,6 +27,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -181,6 +182,11 @@ public class AnnotationsManager {
         }
     }
 
+    private static final Set<ConstraintAnnotationAttributes> CONSTRAINT_ATTRIBUTES =
+        Collections.unmodifiableSet(EnumSet.complementOf(EnumSet.of(ConstraintAnnotationAttributes.VALUE)));
+
+    private static final Set<Class<? extends Annotation>> VALIDATED_CONSTRAINT_TYPES = new HashSet<>();
+
     public static Map<String, Object> readAttributes(Annotation a) {
         final Lazy<Map<String, Object>> result = new Lazy<>(LinkedHashMap::new);
 
@@ -295,6 +301,50 @@ public class AnnotationsManager {
         }
     }
 
+    public void validateConstraintDefinition(Class<? extends Annotation> type) {
+        if (VALIDATED_CONSTRAINT_TYPES.add(type)) {
+            Exceptions.raiseUnless(type.isAnnotationPresent(Constraint.class), ConstraintDefinitionException::new,
+                "%s is not a validation constraint", type);
+
+            final Set<ValidationTarget> supportedTargets = supportedTargets(type);
+
+            final Map<String, Method> attributes =
+                Stream.of(Reflection.getDeclaredMethods(type)).filter(m -> m.getParameterCount() == 0)
+                    .collect(Collectors.toMap(Method::getName, Function.identity()));
+
+            if (supportedTargets.size() > 1
+                && !attributes.containsKey(ConstraintAnnotationAttributes.VALIDATION_APPLIES_TO.getAttributeName())) {
+                Exceptions.raise(ConstraintDefinitionException::new,
+                    "Constraint %s is both generic and cross-parameter but lacks %s attribute", type.getName(),
+                    ConstraintAnnotationAttributes.VALIDATION_APPLIES_TO);
+            }
+            for (ConstraintAnnotationAttributes attr : CONSTRAINT_ATTRIBUTES) {
+                if (attributes.containsKey(attr.getAttributeName())) {
+                    Exceptions.raiseUnless(attr.analyze(type).isValid(), ConstraintDefinitionException::new,
+                        "%s declared invalid type for attribute %s", type, attr);
+
+                    if (!attr.isValidDefaultValue(attributes.get(attr.getAttributeName()).getDefaultValue())) {
+                        Exceptions.raise(ConstraintDefinitionException::new,
+                            "%s declares invalid default value for attribute %s", type, attr);
+                    }
+                    if (attr == ConstraintAnnotationAttributes.VALIDATION_APPLIES_TO) {
+                        if (supportedTargets.size() == 1) {
+                            Exceptions.raise(ConstraintDefinitionException::new,
+                                "Pure %s constraint %s should not declare attribute %s",
+                                supportedTargets.iterator().next(), type, attr);
+                        }
+                    }
+                } else if (attr.isMandatory()) {
+                    Exceptions.raise(ConstraintDefinitionException::new, "%s does not declare mandatory attribute %s",
+                        type, attr);
+                }
+                attributes.remove(attr.getAttributeName());
+            }
+            attributes.keySet().forEach(k -> Exceptions.raiseIf(k.startsWith("valid"),
+                ConstraintDefinitionException::new, "Invalid constraint attribute %s", k));
+        }
+    }
+
     /**
      * Retrieve the composing constraints for the specified constraint
      * {@link Annotation}.

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java
index fa6dab6..de8dd9a 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java
@@ -16,13 +16,6 @@
  */
 package org.apache.bval.jsr.xml;
 
-import org.apache.bval.cdi.EmptyAnnotationLiteral;
-import org.apache.bval.jsr.ConstraintAnnotationAttributes;
-import org.apache.bval.util.reflection.Reflection;
-import org.apache.commons.weaver.privilizer.Privileged;
-import org.apache.commons.weaver.privilizer.Privilizing;
-import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
-
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationHandler;
@@ -41,6 +34,14 @@ import javax.validation.Valid;
 import javax.validation.ValidationException;
 import javax.validation.groups.ConvertGroup;
 
+import org.apache.bval.cdi.EmptyAnnotationLiteral;
+import org.apache.bval.jsr.ConstraintAnnotationAttributes;
+import org.apache.bval.jsr.util.AnnotationsManager;
+import org.apache.bval.util.reflection.Reflection;
+import org.apache.commons.weaver.privilizer.Privileged;
+import org.apache.commons.weaver.privilizer.Privilizing;
+import org.apache.commons.weaver.privilizer.Privilizing.CallTo;
+
 /**
  * Description: Holds the information and creates an annotation proxy during xml
  * parsing of validation mapping constraints. <br/>
@@ -94,19 +95,7 @@ public final class AnnotationProxyBuilder<A extends Annotation> {
     @SuppressWarnings("unchecked")
     public AnnotationProxyBuilder(A annot) {
         this((Class<A>) annot.annotationType());
-        // Obtain the "elements" of the annotation
-        for (Method m : methods) {
-            final boolean mustUnset = Reflection.setAccessible(m, true);
-            try {
-                this.elements.put(m.getName(), m.invoke(annot));
-            } catch (Exception e) {
-                throw new ValidationException("Cannot access annotation " + annot + " element: " + m.getName(), e);
-            } finally {
-                if (mustUnset) {
-                    Reflection.setAccessible(m, false);
-                }
-            }
-        }
+        elements.putAll(AnnotationsManager.readAttributes(annot));
     }
 
     public Method[] getMethods() {

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/util/ObjectUtils.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/util/ObjectUtils.java b/bval-jsr/src/main/java/org/apache/bval/util/ObjectUtils.java
index 2ccf90d..0f5b2f7 100644
--- a/bval-jsr/src/main/java/org/apache/bval/util/ObjectUtils.java
+++ b/bval-jsr/src/main/java/org/apache/bval/util/ObjectUtils.java
@@ -51,12 +51,8 @@ public final class ObjectUtils {
         return object == null ? defaultValue : object;
     }
 
-    public static <T> boolean isNotEmpty(final T[] array) {
-        return !isEmpty(array);
-    }
-
-    public static boolean isEmpty(final Object[] array) {
-        return array == null || array.length == 0;
+    public static boolean isEmptyArray(final Object array) {
+        return array == null || array.getClass().isArray() && Array.getLength(array) == 0;
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/bval/blob/26f8d140/bval-jsr/src/main/java/org/apache/bval/util/reflection/TypeUtils.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/util/reflection/TypeUtils.java b/bval-jsr/src/main/java/org/apache/bval/util/reflection/TypeUtils.java
index a75e1ad..3f8f9d2 100644
--- a/bval-jsr/src/main/java/org/apache/bval/util/reflection/TypeUtils.java
+++ b/bval-jsr/src/main/java/org/apache/bval/util/reflection/TypeUtils.java
@@ -171,7 +171,7 @@ public class TypeUtils {
          * @param lowerBounds of this type
          */
         private WildcardTypeImpl(final Type[] upperBounds, final Type[] lowerBounds) {
-            this.upperBounds = ObjectUtils.isEmpty(upperBounds) ? EMPTY_UPPER_BOUNDS : upperBounds;
+            this.upperBounds = ObjectUtils.isEmptyArray(upperBounds) ? EMPTY_UPPER_BOUNDS : upperBounds;
             this.lowerBounds = lowerBounds == null ? EMPTY_LOWER_BOUNDS : lowerBounds;
         }