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/ecb4f746 Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/ecb4f746 Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/ecb4f746 Branch: refs/heads/bv2 Commit: ecb4f746e48d99e8928d61289dc81291400b1e70 Parents: f5ad044 Author: Matt Benson <[email protected]> Authored: Thu Apr 5 21:28:14 2018 -0500 Committer: Matt Benson <[email protected]> Committed: Tue Oct 16 12:28:20 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/ecb4f746/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/ecb4f746/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/ecb4f746/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) {
