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/2fdff810 Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/2fdff810 Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/2fdff810 Branch: refs/heads/master Commit: 2fdff810aa4ab271131bea0fa4eef89bd5d21645 Parents: fe4faf0 Author: Matt Benson <[email protected]> Authored: Mon Apr 2 16:00:08 2018 -0500 Committer: Matt Benson <[email protected]> Committed: Tue Oct 16 12:28:20 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/2fdff810/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/2fdff810/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/2fdff810/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/2fdff810/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/2fdff810/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/2fdff810/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/2fdff810/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; }
