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/06 02:28:20 UTC

bval git commit: rejigger graph validation: cascades do not participate in Default group redefinition at the bean level

Repository: bval
Updated Branches:
  refs/heads/bv2 0dd0106b0 -> 5fd197b50


rejigger graph validation: cascades do not participate in Default group redefinition at the bean level


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

Branch: refs/heads/bv2
Commit: 5fd197b501c24ecb59af5cbbfcb3c2f5a5a6ae18
Parents: 0dd0106
Author: Matt Benson <mb...@apache.org>
Authored: Thu Apr 5 21:28:14 2018 -0500
Committer: Matt Benson <mb...@apache.org>
Committed: Thu Apr 5 21:28:14 2018 -0500

----------------------------------------------------------------------
 .../java/org/apache/bval/jsr/GraphContext.java  |  45 +++----
 .../apache/bval/jsr/job/ValidateParameters.java |  20 +++-
 .../org/apache/bval/jsr/job/ValidationJob.java  | 118 ++++++++++---------
 3 files changed, 104 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/5fd197b5/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java
index 3d40e75..6b93940 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java
@@ -102,26 +102,31 @@ public class GraphContext {
     }
 
     public ContainerElementKey runtimeKey(ContainerElementKey key) {
-        final Class<?> containerClass = key.getContainerClass();
-        final Class<? extends Object> runtimeType = value.getClass();
-        if (!runtimeType.equals(containerClass)) {
-            Exceptions.raiseUnless(containerClass.isAssignableFrom(runtimeType), ValidationException::new,
-                "Value %s is not assignment-compatible with %s", value, containerClass);
-
-            if (key.getTypeArgumentIndex() == null) {
-                return new ContainerElementKey(runtimeType, null);
-            }
-            final Map<TypeVariable<?>, Type> typeArguments = TypeUtils.getTypeArguments(runtimeType, containerClass);
-
-            Type type = typeArguments.get(containerClass.getTypeParameters()[key.getTypeArgumentIndex().intValue()]);
-
-            while (type instanceof TypeVariable<?>) {
-                final TypeVariable<?> var = (TypeVariable<?>) type;
-                final Type nextType = typeArguments.get(var);
-                if (nextType instanceof TypeVariable<?>) {
-                    type = nextType;
-                } else {
-                    return ContainerElementKey.forTypeVariable(var);
+        Validate.notNull(key);
+        if (value != null) {
+            final Class<?> containerClass = key.getContainerClass();
+            final Class<? extends Object> runtimeType = value.getClass();
+            if (!runtimeType.equals(containerClass)) {
+                Exceptions.raiseUnless(containerClass.isAssignableFrom(runtimeType), ValidationException::new,
+                    "Value %s is not assignment-compatible with %s", value, containerClass);
+
+                if (key.getTypeArgumentIndex() == null) {
+                    return new ContainerElementKey(runtimeType, null);
+                }
+                final Map<TypeVariable<?>, Type> typeArguments =
+                    TypeUtils.getTypeArguments(runtimeType, containerClass);
+
+                Type type =
+                    typeArguments.get(containerClass.getTypeParameters()[key.getTypeArgumentIndex().intValue()]);
+
+                while (type instanceof TypeVariable<?>) {
+                    final TypeVariable<?> var = (TypeVariable<?>) type;
+                    final Type nextType = typeArguments.get(var);
+                    if (nextType instanceof TypeVariable<?>) {
+                        type = nextType;
+                    } else {
+                        return ContainerElementKey.forTypeVariable(var);
+                    }
                 }
             }
         }

http://git-wip-us.apache.org/repos/asf/bval/blob/5fd197b5/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java
index 4adc4a9..af511f9 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java
@@ -23,7 +23,9 @@ import java.lang.reflect.Executable;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import javax.validation.ConstraintViolation;
@@ -125,16 +127,26 @@ public abstract class ValidateParameters<E extends Executable, T> extends Valida
         }
 
         @Override
-        void recurse(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
-            executableDescriptor.getParameterDescriptors().stream()
-                .map(pd -> new SproutFrame<ParameterD<?>>(this, (ParameterD<?>) pd, parameter(pd.getIndex())))
-                .forEach(f -> f.process(group, sink));
+        void process(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+            Validate.notNull(sink, "sink");
+            final Lazy<Set<Frame<?>>> parameterFrames = new Lazy<>(this::parameterFrames);
+            each(expand(group), (t, u) -> {
+                validateDescriptorConstraints(t, u);
+                parameterFrames.get().forEach(p -> p.validateDescriptorConstraints(t, u));
+            }, sink);
+            parameterFrames.get().forEach(p -> p.recurse(group, sink));
         }
 
         @Override
         Object getBean() {
             return object;
         }
+
+        private Set<Frame<?>> parameterFrames() {
+            return executableDescriptor.getParameterDescriptors().stream()
+                .map(pd -> new SproutFrame<ParameterD<?>>(this, (ParameterD<?>) pd, parameter(pd.getIndex())))
+                .collect(Collectors.toSet());
+        }
     }
 
     private static final String PARAMETERS_DO_NOT_MATCH = "Parameters do not match";

http://git-wip-us.apache.org/repos/asf/bval/blob/5fd197b5/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 d9a94a8..839c14c 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
@@ -97,21 +97,20 @@ public abstract class ValidationJob<T> {
             return ValidationJob.this;
         }
 
-        final void process(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+        void process(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
             Validate.notNull(sink, "sink");
-
-            each(expand(group), (g, s) -> {
-                validateDescriptorConstraints(g, s);
-                recurse(g, s);
-            }, sink);
+            each(expand(group), this::validateDescriptorConstraints, sink);
+            recurse(group, sink);
         }
 
-        abstract void recurse(Class<?> group, Consumer<ConstraintViolation<T>> sink);
+        void recurse(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+            throw new UnsupportedOperationException();
+        }
 
         abstract Object getBean();
 
         void validateDescriptorConstraints(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
-            constraintsFor(group)
+            constraintsFor(descriptor, group)
                 .forEach(c -> unwrap(c.getValueUnwrapping()).forEach(f -> f.validate(c, sink)));
         }
 
@@ -131,15 +130,6 @@ public abstract class ValidationJob<T> {
             return Stream.of(this);
         }
 
-        private Stream<ConstraintD<?>> constraintsFor(Class<?> group) {
-            return descriptor.getConstraintDescriptors().stream().<ConstraintD<?>> map(ConstraintD.class::cast)
-                .filter(c -> {
-                    final Set<Class<?>> constraintGroups = c.getGroups();
-                    return constraintGroups.contains(group)
-                        || constraintGroups.contains(Default.class) && c.getDeclaringClass().isAssignableFrom(group);
-                });
-        }
-
         @SuppressWarnings({ "rawtypes", "unchecked" })
         private boolean validate(ConstraintD<?> constraint, Consumer<ConstraintViolation<T>> sink) {
             if (!validatedPathsByConstraint
@@ -238,7 +228,7 @@ public abstract class ValidationJob<T> {
             return extractedType.orElse(elementClass);
         }
 
-        private Stream<Class<?>> expand(Class<?> group) {
+        Stream<Class<?>> expand(Class<?> group) {
             if (Default.class.equals(group)) {
                 final List<Class<?>> groupSequence = descriptor.getGroupSequence();
                 if (groupSequence != null) {
@@ -264,9 +254,14 @@ public abstract class ValidationJob<T> {
             this.realContext = context;
         }
 
-        @Override
-        void recurse(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
-            propertyFrames().forEach(f -> f.process(group, sink));
+        void process(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+            Validate.notNull(sink, "sink");
+            final Lazy<Set<Frame<?>>> propertyFrames = new Lazy<>(this::propertyFrames);
+            each(expand(group), (t, u) -> {
+                validateDescriptorConstraints(t, u);
+                propertyFrames.get().forEach(p -> p.validateDescriptorConstraints(t, u));
+            }, sink);
+            propertyFrames.get().forEach(p -> p.recurse(group, sink));
         }
 
         protected Frame<?> propertyFrame(PropertyD<?> d, GraphContext context) {
@@ -278,7 +273,7 @@ public abstract class ValidationJob<T> {
             return context.getValue();
         }
 
-        private Stream<Frame<?>> propertyFrames() {
+        private Set<Frame<?>> propertyFrames() {
             final Stream<PropertyD<?>> properties = descriptor.getConstrainedProperties().stream()
                 .flatMap(d -> ComposedD.unwrap(d, PropertyD.class)).map(d -> (PropertyD<?>) d);
 
@@ -296,8 +291,8 @@ public abstract class ValidationJob<T> {
                     throw new ValidationException(e);
                 }
             });
-            return reachableProperties.flatMap(
-                d -> d.read(realContext).filter(context -> !context.isRecursive()).map(child -> propertyFrame(d, child)));
+            return reachableProperties.flatMap(d -> d.read(realContext).filter(context -> !context.isRecursive())
+                .map(child -> propertyFrame(d, child))).collect(Collectors.toSet());
         }
     }
 
@@ -310,6 +305,25 @@ public abstract class ValidationJob<T> {
         public SproutFrame(Frame<?> parent, D descriptor, GraphContext context) {
             super(parent, descriptor, context);
         }
+        
+        @Override
+        void validateDescriptorConstraints(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+            super.validateDescriptorConstraints(group, sink);
+            if (context.getValue() != null) {
+                descriptor.getConstrainedContainerElementTypes().stream()
+                    .flatMap(d -> ComposedD.unwrap(d, ContainerElementTypeD.class)).forEach(d -> {
+                        if (constraintsFor(d, group).findFirst().isPresent()
+                            || !d.getConstrainedContainerElementTypes().isEmpty()) {
+                            final ValueExtractor<?> declaredTypeValueExtractor =
+                                context.getValidatorContext().getValueExtractors().find(d.getKey());
+                            ExtractValues.extract(context, d.getKey(), declaredTypeValueExtractor).stream()
+                                .filter(e -> !e.isRecursive())
+                                .map(e -> new ContainerElementConstraintsFrame(this, d, e))
+                                .forEach(f -> f.validateDescriptorConstraints(group, sink));
+                        }
+                    });
+            }
+        }
 
         @Override
         void recurse(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
@@ -317,19 +331,28 @@ public abstract class ValidationJob<T> {
                 validatorContext.getGroupsComputer().computeCascadingGroups(descriptor.getGroupConversions(),
                     descriptor.getDeclaringClass().isAssignableFrom(group) ? Default.class : group);
 
-            convertedGroups.getGroups().stream().map(Group::getGroup).forEach(g -> recurseSingleExpandedGroup(g, sink));
+            convertedGroups.getGroups().stream().map(Group::getGroup).forEach(g -> cascade(g, sink));
 
             sequences: for (List<Group> seq : convertedGroups.getSequences()) {
-                final boolean proceed = each(seq.stream().map(Group::getGroup), this::recurseSingleExpandedGroup, sink);
+                final boolean proceed = each(seq.stream().map(Group::getGroup), this::cascade, sink);
                 if (!proceed) {
                     break sequences;
                 }
             }
         }
 
-        protected void recurseSingleExpandedGroup(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
-            processContainerElements(group, sink);
-
+        private void cascade(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
+            if (context.getValue() != null) {
+                descriptor.getConstrainedContainerElementTypes().stream()
+                    .filter(d -> d.isCascaded() || !d.getConstrainedContainerElementTypes().isEmpty())
+                    .flatMap(d -> ComposedD.unwrap(d, ContainerElementTypeD.class)).forEach(d -> {
+                        final ValueExtractor<?> runtimeTypeValueExtractor =
+                            context.getValidatorContext().getValueExtractors().find(context.runtimeKey(d.getKey()));
+                        ExtractValues.extract(context, d.getKey(), runtimeTypeValueExtractor).stream()
+                            .filter(e -> !e.isRecursive()).map(e -> new ContainerElementCascadeFrame(this, d, e))
+                            .forEach(f -> f.recurse(group, sink));
+                    });
+            }
             if (!descriptor.isCascaded()) {
                 return;
             }
@@ -357,30 +380,6 @@ public abstract class ValidationJob<T> {
                 .map(context -> new BeanFrame<>(this, context)).forEach(b -> b.process(group, sink));
         }
 
-        private void processContainerElements(Class<?> group, Consumer<ConstraintViolation<T>> sink) {
-            if (context.getValue() == null) {
-                return;
-            }
-            // handle spec dichotomy: declared type for constraints; runtime type for cascades. Bypass #process()
-            descriptor.getConstrainedContainerElementTypes().stream()
-                .flatMap(d -> ComposedD.unwrap(d, ContainerElementTypeD.class)).forEach(d -> {
-                    if (!d.findConstraints().unorderedAndMatchingGroups(group).getConstraintDescriptors().isEmpty()) {
-                        final ValueExtractor<?> declaredTypeValueExtractor =
-                            context.getValidatorContext().getValueExtractors().find(d.getKey());
-                        ExtractValues.extract(context, d.getKey(), declaredTypeValueExtractor).stream()
-                            .filter(e -> !e.isRecursive()).map(e -> new ContainerElementConstraintsFrame(this, d, e))
-                            .forEach(f -> f.validateDescriptorConstraints(group, sink));
-                    }
-                    if (d.isCascaded() || !d.getConstrainedContainerElementTypes().isEmpty()) {
-                        final ValueExtractor<?> runtimeTypeValueExtractor =
-                            context.getValidatorContext().getValueExtractors().find(context.runtimeKey(d.getKey()));
-                        ExtractValues.extract(context, d.getKey(), runtimeTypeValueExtractor).stream()
-                            .filter(e -> !e.isRecursive()).map(e -> new ContainerElementCascadeFrame(this, d, e))
-                            .forEach(f -> f.recurse(group, sink));
-                    }
-                });
-        }
-
         protected GraphContext getMultiplexContext() {
             return context;
         }
@@ -527,6 +526,15 @@ public abstract class ValidationJob<T> {
     protected static final TypeVariable<?> MAP_VALUE = Map.class.getTypeParameters()[1];
     protected static final TypeVariable<?> ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0];
 
+    private static Stream<ConstraintD<?>> constraintsFor(ElementD<?, ?> descriptor, Class<?> group) {
+        return descriptor.getConstraintDescriptors().stream().<ConstraintD<?>> map(ConstraintD.class::cast)
+            .filter(c -> {
+                final Set<Class<?>> constraintGroups = c.getGroups();
+                return constraintGroups.contains(group)
+                    || constraintGroups.contains(Default.class) && c.getDeclaringClass().isAssignableFrom(group);
+            });
+    }
+
     protected final ApacheFactoryContext validatorContext;
 
     private final Groups groups;
@@ -571,8 +579,8 @@ public abstract class ValidationJob<T> {
         return results.reset(Collections::emptySet).get();
     }
 
-    private boolean each(Stream<Class<?>> groupSequence, BiConsumer<Class<?>, Consumer<ConstraintViolation<T>>> closure,
-        Consumer<ConstraintViolation<T>> sink) {
+    protected boolean each(Stream<Class<?>> groupSequence,
+        BiConsumer<Class<?>, Consumer<ConstraintViolation<T>>> closure, Consumer<ConstraintViolation<T>> sink) {
         final Lazy<Set<ConstraintViolation<T>>> sequenceViolations = new Lazy<>(LinkedHashSet::new);
         final Consumer<ConstraintViolation<T>> addSequenceViolation = sequenceViolations.consumer(Set::add);
         for (Class<?> g : (Iterable<Class<?>>) groupSequence::iterator) {