Repository: bval
Updated Branches:
  refs/heads/bv2 3409b647b -> 0dd0106b0


no implicit groups on interfaces; retain elsewhere


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

Branch: refs/heads/bv2
Commit: 7fab00d5e2b6b9aa012df0a8574d3f0355a25403
Parents: 3409b64
Author: Matt Benson <mben...@apache.org>
Authored: Wed Apr 4 16:45:57 2018 -0500
Committer: Matt Benson <mben...@apache.org>
Committed: Wed Apr 4 16:45:57 2018 -0500

----------------------------------------------------------------------
 .../apache/bval/jsr/descriptor/ConstraintD.java |   1 -
 .../bval/jsr/descriptor/DescriptorManager.java  |   6 +-
 .../bval/jsr/descriptor/MetadataReader.java     |  48 ++++++--
 .../org/apache/bval/jsr/job/ValidationJob.java  |   8 +-
 .../bval/jsr/metadata/CompositeBuilder.java     |  12 +-
 .../bval/jsr/metadata/HierarchyBuilder.java     |  21 ++--
 .../bval/jsr/metadata/MetadataBuilder.java      |   6 +-
 .../bval/jsr/util/AnnotationsManager.java       | 119 ++++++++-----------
 .../org/apache/bval/jsr/BeanDescriptorTest.java |   3 -
 9 files changed, 114 insertions(+), 110 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/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 494002b..a5e528b 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,7 +87,6 @@ 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/7fab00d5/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java
----------------------------------------------------------------------
diff --git 
a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java 
b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java
index 8f1e22f..dff4f77 100644
--- 
a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java
+++ 
b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java
@@ -50,13 +50,11 @@ public class DescriptorManager {
     private final ApacheValidatorFactory validatorFactory;
     private final ConcurrentMap<Class<?>, BeanD<?>> beanDescriptors = new 
ConcurrentHashMap<>();
     private final ReflectionBuilder reflectionBuilder;
-    private final MetadataReader metadataReader;
 
     public DescriptorManager(ApacheValidatorFactory validatorFactory) {
         super();
         this.validatorFactory = Validate.notNull(validatorFactory, 
"validatorFactory");
         this.reflectionBuilder = new ReflectionBuilder(validatorFactory);
-        this.metadataReader = new MetadataReader(validatorFactory);
     }
 
     public <T> BeanDescriptor getBeanDescriptor(Class<T> beanClass) {
@@ -66,7 +64,9 @@ public class DescriptorManager {
         if (beanDescriptors.containsKey(beanClass)) {
             return beanDescriptors.get(beanClass);
         }
-        final BeanD<T> beanD = new BeanD<>(metadataReader.forBean(beanClass, 
builder(beanClass)));
+        final MetadataReader.ForBean<T> reader =
+            new MetadataReader(validatorFactory, 
beanClass).forBean(builder(beanClass));
+        final BeanD<T> beanD = new BeanD<>(reader);
         @SuppressWarnings("unchecked")
         final BeanD<T> result =
             Optional.ofNullable((BeanD<T>) 
beanDescriptors.putIfAbsent(beanClass, beanD)).orElse(beanD);

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java
----------------------------------------------------------------------
diff --git 
a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java 
b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java
index e5e4db6..1673107 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java
@@ -50,7 +50,9 @@ import javax.validation.metadata.PropertyDescriptor;
 import javax.validation.metadata.Scope;
 
 import org.apache.bval.jsr.ApacheValidatorFactory;
+import org.apache.bval.jsr.ConstraintAnnotationAttributes;
 import org.apache.bval.jsr.groups.GroupConversion;
+import org.apache.bval.jsr.groups.GroupsComputer;
 import org.apache.bval.jsr.metadata.ContainerElementKey;
 import org.apache.bval.jsr.metadata.EmptyBuilder;
 import org.apache.bval.jsr.metadata.Meta;
@@ -58,7 +60,9 @@ import org.apache.bval.jsr.metadata.MetadataBuilder;
 import org.apache.bval.jsr.metadata.Signature;
 import org.apache.bval.jsr.util.Methods;
 import org.apache.bval.jsr.util.ToUnmodifiable;
+import org.apache.bval.jsr.xml.AnnotationProxyBuilder;
 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;
 
@@ -75,16 +79,43 @@ class MetadataReader {
         }
 
         Set<ConstraintD<?>> getConstraints() {
-            return builder.getConstraintsByScope(meta).entrySet().stream()
-                .flatMap(e -> describe(e.getValue(), e.getKey(), 
meta)).collect(ToUnmodifiable.set());
+            return 
builder.getConstraintDeclarationMap(meta).entrySet().stream().flatMap(e -> {
+                final Meta<E> m = e.getKey();
+                final Class<?> declaredBy = m.getDeclaringClass();
+                final Scope scope = declaredBy.equals(beanClass) ? 
Scope.LOCAL_ELEMENT : Scope.HIERARCHY;
+                return Stream.of(e.getValue())
+                    .peek(
+                        c -> 
validatorFactory.getAnnotationsManager().validateConstraintDefinition(c.annotationType()))
+                    .map(c -> rewriteConstraint(c, declaredBy))
+                    .map(c -> new ConstraintD<>(c, scope, m, 
validatorFactory));
+            }).collect(ToUnmodifiable.set());
         }
 
         ApacheValidatorFactory getValidatorFactory() {
             return validatorFactory;
         }
 
-        private Stream<ConstraintD<?>> describe(Annotation[] constraints, 
Scope scope, Meta<?> meta) {
-            return Stream.of(constraints).map(c -> new ConstraintD<>(c, scope, 
meta, validatorFactory));
+        private <A extends Annotation> A rewriteConstraint(A constraint, 
Class<?> declaredBy) {
+            boolean mustRewrite = false;
+            Class<?>[] groups =
+                
ConstraintAnnotationAttributes.GROUPS.analyze(constraint.annotationType()).read(constraint);
+            if (groups.length == 0) {
+                mustRewrite = true;
+                groups = GroupsComputer.DEFAULT_GROUP;
+            }
+            if (!(declaredBy.equals(beanClass) || beanClass.isInterface())) {
+                if (ObjectUtils.arrayContains(groups, Default.class)
+                    && !ObjectUtils.arrayContains(groups, declaredBy)) {
+                    mustRewrite = true;
+                    groups = ObjectUtils.arrayAdd(groups, declaredBy);
+                }
+            }
+            if (mustRewrite) {
+                final AnnotationProxyBuilder<A> builder = new 
AnnotationProxyBuilder<A>(constraint);
+                builder.setGroups(groups);
+                return builder.createAnnotation();
+            }
+            return constraint;
         }
     }
 
@@ -316,13 +347,16 @@ class MetadataReader {
     }
 
     private final ApacheValidatorFactory validatorFactory;
+    private final Class<?> beanClass;
 
-    MetadataReader(ApacheValidatorFactory validatorFactory) {
+    MetadataReader(ApacheValidatorFactory validatorFactory, Class<?> 
beanClass) {
         super();
         this.validatorFactory = Validate.notNull(validatorFactory, 
"validatorFactory");
+        this.beanClass = Validate.notNull(beanClass, "beanClass");
     }
 
-    <T> MetadataReader.ForBean<T> forBean(Class<T> beanClass, 
MetadataBuilder.ForBean<T> builder) {
-        return new MetadataReader.ForBean<>(new Meta.ForClass<>(beanClass), 
builder);
+    @SuppressWarnings("unchecked")
+    <T> MetadataReader.ForBean<T> forBean(MetadataBuilder.ForBean<T> builder) {
+        return new MetadataReader.ForBean<>(new Meta.ForClass<>((Class<T>) 
beanClass), builder);
     }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java 
b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java
index 0bbf17d..d9a94a8 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java
@@ -133,11 +133,11 @@ public abstract class ValidationJob<T> {
 
         private Stream<ConstraintD<?>> constraintsFor(Class<?> group) {
             return 
descriptor.getConstraintDescriptors().stream().<ConstraintD<?>> 
map(ConstraintD.class::cast)
-                .filter(c -> Stream.of(group).anyMatch(t -> {
+                .filter(c -> {
                     final Set<Class<?>> constraintGroups = c.getGroups();
-                    return constraintGroups.contains(t)
-                        || constraintGroups.contains(Default.class) && 
c.getDeclaringClass().isAssignableFrom(t);
-                }));
+                    return constraintGroups.contains(group)
+                        || constraintGroups.contains(Default.class) && 
c.getDeclaringClass().isAssignableFrom(group);
+                });
         }
 
         @SuppressWarnings({ "rawtypes", "unchecked" })

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/CompositeBuilder.java
----------------------------------------------------------------------
diff --git 
a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/CompositeBuilder.java 
b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/CompositeBuilder.java
index 77f3b60..60419fb 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/CompositeBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/CompositeBuilder.java
@@ -39,7 +39,6 @@ import java.util.stream.Stream;
 
 import javax.validation.ElementKind;
 import javax.validation.ParameterNameProvider;
-import javax.validation.metadata.Scope;
 
 import org.apache.bval.jsr.ApacheValidatorFactory;
 import org.apache.bval.jsr.groups.GroupConversion;
@@ -71,7 +70,8 @@ public class CompositeBuilder {
         }
 
         <K, D> Map<K, D> merge(Function<DELEGATE, Map<K, D>> toMap, 
BiFunction<K, List<D>, D> merge) {
-            final List<Map<K, D>> maps = 
delegates.stream().map(toMap).collect(Collectors.toList());
+            final List<Map<K, D>> maps =
+                delegates.stream().map(toMap).filter(m -> 
!m.isEmpty()).collect(Collectors.toList());
 
             final Function<? super K, ? extends D> valueMapper = k -> {
                 final List<D> mappedDelegates =
@@ -140,8 +140,8 @@ public class CompositeBuilder {
         }
 
         @Override
-        public Map<Scope, Annotation[]> getConstraintsByScope(Meta<E> meta) {
-            return CompositeBuilder.this.getConstraintsByScope(this, meta);
+        public Map<Meta<E>, Annotation[]> getConstraintDeclarationMap(Meta<E> 
meta) {
+            return CompositeBuilder.this.getConstraintDeclarationMap(this, 
meta);
         }
 
         @Override
@@ -263,9 +263,9 @@ public class CompositeBuilder {
             .mapToObj(n -> new Meta.ForParameter(parameters[n], 
parameterNames.get(n))).collect(Collectors.toList());
     }
 
-    protected <E extends AnnotatedElement> Map<Scope, Annotation[]> 
getConstraintsByScope(
+    protected <E extends AnnotatedElement> Map<Meta<E>, Annotation[]> 
getConstraintDeclarationMap(
         CompositeBuilder.ForElement<? extends MetadataBuilder.ForElement<E>, 
E> composite, Meta<E> meta) {
-        return Collections.singletonMap(Scope.LOCAL_ELEMENT, 
composite.getDeclaredConstraints(meta));
+        return Collections.singletonMap(meta, 
composite.getDeclaredConstraints(meta));
     }
 
     protected <T> List<Class<?>> getGroupSequence(CompositeBuilder.ForClass<T> 
composite, Meta<Class<T>> meta) {

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/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 4861c1a..5fe228c 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
@@ -26,7 +26,6 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.EnumMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -37,10 +36,10 @@ import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
+import java.util.stream.Stream;
 
 import javax.validation.ElementKind;
 import javax.validation.ParameterNameProvider;
-import javax.validation.metadata.Scope;
 
 import org.apache.bval.jsr.ApacheValidatorFactory;
 import org.apache.bval.jsr.groups.GroupConversion;
@@ -314,20 +313,16 @@ public class HierarchyBuilder extends CompositeBuilder {
     }
 
     @Override
-    protected <E extends AnnotatedElement> Map<Scope, Annotation[]> 
getConstraintsByScope(
+    protected <E extends AnnotatedElement> Map<Meta<E>, Annotation[]> 
getConstraintDeclarationMap(
         CompositeBuilder.ForElement<? extends MetadataBuilder.ForElement<E>, 
E> composite, Meta<E> meta) {
 
-        final Iterator<? extends MetadataBuilder.ForElement<E>> iter = 
composite.delegates.iterator();
-
-        final Map<Scope, Annotation[]> result = new EnumMap<>(Scope.class);
-        result.put(Scope.LOCAL_ELEMENT, 
iter.next().getDeclaredConstraints(meta));
+        @SuppressWarnings("unchecked")
+        final Function<MetadataBuilder.ForElement<E>, Meta<E>> keyMapper =
+            d -> 
Optional.of(d).filter(HierarchyDelegate.class::isInstance).map(HierarchyDelegate.class::cast)
+                .map(HierarchyDelegate::getHierarchyElement).orElse(meta);
 
-        if (iter.hasNext()) {
-            final List<Annotation> hierarchyConstraints = new ArrayList<>();
-            iter.forEachRemaining(d -> 
Collections.addAll(hierarchyConstraints, d.getDeclaredConstraints(meta)));
-            result.put(Scope.HIERARCHY, hierarchyConstraints.toArray(new 
Annotation[hierarchyConstraints.size()]));
-        }
-        return result;
+        return 
composite.delegates.stream().collect(Collectors.toMap(keyMapper, d -> 
d.getDeclaredConstraints(meta),
+            (u, v) -> Stream.of(u, 
v).flatMap(Stream::of).toArray(Annotation[]::new), LinkedHashMap::new));
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataBuilder.java
----------------------------------------------------------------------
diff --git 
a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataBuilder.java 
b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataBuilder.java
index ff60df0..c66a80a 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataBuilder.java
@@ -29,8 +29,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import javax.validation.metadata.Scope;
-
 import org.apache.bval.jsr.groups.GroupConversion;
 
 /**
@@ -65,8 +63,8 @@ public final class MetadataBuilder {
 
         Annotation[] getDeclaredConstraints(Meta<E> meta);
 
-        default Map<Scope, Annotation[]> getConstraintsByScope(Meta<E> meta) {
-            return Collections.singletonMap(Scope.LOCAL_ELEMENT, 
getDeclaredConstraints(meta));
+        default Map<Meta<E>, Annotation[]> getConstraintDeclarationMap(Meta<E> 
meta) {
+            return Collections.singletonMap(meta, 
getDeclaredConstraints(meta));
         }
     }
 

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/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 1995c52..dac1294 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
@@ -29,14 +29,12 @@ import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
-import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -54,9 +52,6 @@ import org.apache.bval.jsr.ConfigurationImpl;
 import org.apache.bval.jsr.ConstraintAnnotationAttributes;
 import org.apache.bval.jsr.ConstraintAnnotationAttributes.Worker;
 import org.apache.bval.jsr.ConstraintCached.ConstraintValidatorInfo;
-import org.apache.bval.jsr.groups.Group;
-import org.apache.bval.jsr.groups.Groups;
-import org.apache.bval.jsr.groups.GroupsComputer;
 import org.apache.bval.jsr.metadata.Meta;
 import org.apache.bval.jsr.xml.AnnotationProxyBuilder;
 import org.apache.bval.util.Exceptions;
@@ -259,32 +254,14 @@ public class AnnotationsManager {
      * @return Annotation[]
      */
     public static Annotation[] getDeclaredConstraints(Meta<?> meta) {
-        final Annotation[] result = getDeclaredConstraints(meta.getHost());
-        final Class<?> dc = meta.getDeclaringClass();
-        if (dc.isInterface()) {
-            final GroupsComputer groupsComputer = new GroupsComputer();
-            // ensure interface group is implied by Default group:
-            Stream.of(result).map(c -> {
-                final Groups groups = groupsComputer.computeGroups(
-                    
ConstraintAnnotationAttributes.GROUPS.analyze(c.annotationType()).<Class<?>[]> 
read(c));
-                if (groups.getGroups().stream().anyMatch(Group::isDefault)) {
-                    final Set<Class<?>> groupClasses = 
groups.getGroups().stream().map(Group::getGroup)
-                        .collect(Collectors.toCollection(LinkedHashSet::new));
-                    if (groupClasses.add(dc)) {
-                        final AnnotationProxyBuilder<?> proxyBuilder = new 
AnnotationProxyBuilder<>(c);
-                        proxyBuilder.setGroups(groupClasses.toArray(new 
Class[groupClasses.size()]));
-                        return proxyBuilder.createAnnotation();
-                    }
-                }
-                return c;
-            }).toArray(n -> result);
-        }
-        return result;
+        return getDeclaredConstraints(meta.getHost());
     }
 
     private static Annotation[] getDeclaredConstraints(AnnotatedElement e) {
         final Annotation[] declaredAnnotations = e.getDeclaredAnnotations();
-
+        if (declaredAnnotations.length == 0) {
+            return declaredAnnotations;
+        }
         // collect constraint explicitly nested into repeatable containers:
         final Map<Class<? extends Annotation>, Annotation[]> repeated = new 
HashMap<>();
 
@@ -296,18 +273,19 @@ public class AnnotationsManager {
                 repeated.put(annotationType, w.read(a));
             }
         }
-        final Set<Annotation> constraints = Stream.of(declaredAnnotations)
-            .filter((Predicate<Annotation>) a -> 
a.annotationType().isAnnotationPresent(Constraint.class))
-            .collect(Collectors.toSet());
+        Stream<Annotation> constraints = Stream.of(declaredAnnotations)
+                .filter(a -> 
a.annotationType().isAnnotationPresent(Constraint.class));
 
-        Exceptions.raiseIf(
-            constraints.stream().map(Annotation::annotationType).filter(t -> 
t.isAnnotationPresent(Repeatable.class))
-                .map(rct -> 
rct.getAnnotation(Repeatable.class).value()).anyMatch(repeated::containsKey),
-            ConstraintDeclarationException::new,
-            "Simultaneous declaration of repeatable constraint and associated 
container on %s", e);
+        if (!repeated.isEmpty()) {
+            constraints = constraints.peek(c -> Exceptions.raiseIf(
+                Optional.of(c.annotationType()).map(t -> 
t.getAnnotation(Repeatable.class)).map(Repeatable::value)
+                    .filter(repeated::containsKey).isPresent(),
+                ConstraintDeclarationException::new,
+                "Simultaneous declaration of repeatable constraint and 
associated container on %s", e));
 
-        return Stream.concat(constraints.stream(), 
repeated.values().stream().flatMap(Stream::of))
-            .toArray(Annotation[]::new);
+            constraints = Stream.concat(constraints, 
repeated.values().stream().flatMap(Stream::of));
+        }
+        return constraints.toArray(Annotation[]::new);
     }
 
     private final ApacheValidatorFactory validatorFactory;
@@ -328,47 +306,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);
+        if (VALIDATED_CONSTRAINT_TYPES.contains(type)) {
+            return;
+        }
+        Exceptions.raiseUnless(type.isAnnotationPresent(Constraint.class), 
ConstraintDefinitionException::new,
+            "%s is not a validation constraint", type);
 
-            final Set<ValidationTarget> supportedTargets = 
supportedTargets(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()));
+        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 (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()))
 {
+                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,
-                            "%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);
-                        }
+                            "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());
+            } else if (attr.isMandatory()) {
+                Exceptions.raise(ConstraintDefinitionException::new, "%s does 
not declare mandatory attribute %s",
+                    type, attr);
             }
-            attributes.keySet().forEach(k -> 
Exceptions.raiseIf(k.startsWith("valid"),
-                ConstraintDefinitionException::new, "Invalid constraint 
attribute %s", k));
+            attributes.remove(attr.getAttributeName());
         }
+        attributes.keySet().forEach(k -> 
Exceptions.raiseIf(k.startsWith("valid"),
+            ConstraintDefinitionException::new, "Invalid constraint attribute 
%s", k));
+
+        VALIDATED_CONSTRAINT_TYPES.add(type);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/bval/blob/7fab00d5/bval-jsr/src/test/java/org/apache/bval/jsr/BeanDescriptorTest.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/BeanDescriptorTest.java 
b/bval-jsr/src/test/java/org/apache/bval/jsr/BeanDescriptorTest.java
index 5c949c1..d81f90a 100644
--- a/bval-jsr/src/test/java/org/apache/bval/jsr/BeanDescriptorTest.java
+++ b/bval-jsr/src/test/java/org/apache/bval/jsr/BeanDescriptorTest.java
@@ -42,7 +42,6 @@ import javax.validation.metadata.PropertyDescriptor;
 import javax.validation.metadata.Scope;
 
 import org.apache.bval.jsr.util.TestUtils;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -121,8 +120,6 @@ public class BeanDescriptorTest extends ValidationTestBase {
      * interface group when querying the interface directly.
      */
     @Test
-    // spec does not dictate this
-    @Ignore
     public void testNoImplicitGroupWhenQueryingInterfaceDirectly() {
         Set<ConstraintDescriptor<?>> nameDescriptors =
             
validator.getConstraintsForClass(Person.class).getConstraintsForProperty("name").getConstraintDescriptors();

Reply via email to