TCK GraphNavigationTest fixes: path comparisons, graph recursion
Project: http://git-wip-us.apache.org/repos/asf/bval/repo Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/ea55f78a Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/ea55f78a Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/ea55f78a Branch: refs/heads/bv2 Commit: ea55f78a0161b30c72293b80a00a46a760c23cd2 Parents: 31a63ef Author: Matt Benson <[email protected]> Authored: Thu Mar 8 16:05:00 2018 -0600 Committer: Matt Benson <[email protected]> Committed: Fri Mar 9 13:11:40 2018 -0600 ---------------------------------------------------------------------- .../org/apache/bval/jsr/job/ValidationJob.java | 9 +- .../java/org/apache/bval/jsr/util/NodeImpl.java | 130 +++++++++++++++++-- .../java/org/apache/bval/jsr/util/PathImpl.java | 4 + .../java/org/apache/bval/util/Comparators.java | 54 ++++++++ 4 files changed, 178 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/bval/blob/ea55f78a/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 87542f4..70905d0 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 @@ -22,7 +22,6 @@ import java.lang.reflect.Array; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -120,7 +119,7 @@ public abstract class ValidationJob<T> { @SuppressWarnings({ "rawtypes", "unchecked" }) private boolean validate(ConstraintD<?> constraint, Consumer<ConstraintViolation<T>> sink) { if (!validatedPathsByConstraint - .computeIfAbsent(constraint, k -> new ConcurrentSkipListSet<>(COMPARE_TO_STRING)) + .computeIfAbsent(constraint, k -> new ConcurrentSkipListSet<>(PathImpl.PATH_COMPARATOR)) .add(context.getPath())) { // seen, ignore: return true; @@ -293,8 +292,8 @@ public abstract class ValidationJob<T> { return; } } - multiplex().filter(context -> context.getValue() != null).map(context -> new BeanFrame<>(this, context)) - .forEach(b -> b.process(group, sink)); + multiplex().filter(context -> context.getValue() != null && !context.isRecursive()) + .map(context -> new BeanFrame<>(this, context)).forEach(b -> b.process(group, sink)); } private Stream<GraphContext> multiplex() { @@ -387,8 +386,6 @@ public abstract class ValidationJob<T> { } } - private static final Comparator<Path> COMPARE_TO_STRING = Comparator.comparing(Object::toString); - protected static final TypeVariable<?> MAP_VALUE = Map.class.getTypeParameters()[1]; protected static final TypeVariable<?> ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0]; http://git-wip-us.apache.org/repos/asf/bval/blob/ea55f78a/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java ---------------------------------------------------------------------- diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java index 4de46de..e8319f8 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java @@ -18,24 +18,62 @@ */ package org.apache.bval.jsr.util; -import javax.validation.ElementKind; -import javax.validation.Path; -import javax.validation.Path.Node; - -import org.apache.bval.util.Exceptions; +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; +import static java.util.Comparator.nullsFirst; import java.io.Serializable; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; +import java.util.function.Function; -public abstract class NodeImpl implements Path.Node, Serializable { +import javax.validation.ElementKind; +import javax.validation.Path; +import javax.validation.Path.BeanNode; +import javax.validation.Path.ConstructorNode; +import javax.validation.Path.ContainerElementNode; +import javax.validation.Path.MethodNode; +import javax.validation.Path.Node; +import javax.validation.Path.ParameterNode; +import javax.validation.Path.PropertyNode; + +import org.apache.bval.util.Comparators; +import org.apache.bval.util.Exceptions; +public abstract class NodeImpl implements Path.Node, Serializable { private static final long serialVersionUID = 1L; - private static final String INDEX_OPEN = "["; - private static final String INDEX_CLOSE = "]"; + + /** + * Comparator for any path {@link Node}. + */ + public static final Comparator<Path.Node> NODE_COMPARATOR = + nullsFirst(comparing(Node::getName, nullsFirst(naturalOrder())).thenComparing(NodeImpl::compareIterability) + .thenComparing(NodeImpl::compareSpecificNodeInfo)); + + private static final Comparator<Class<?>> CLASS_COMPARATOR = Comparator.nullsFirst( + Comparator.<Class<?>, Boolean> comparing(Class::isPrimitive).reversed().thenComparing(Class::getName)); + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static final Comparator<Object> KEY_COMPARATOR = nullsFirst(((Comparator<Object>) (quid, quo) -> { + if (quid instanceof Comparable<?> && quo instanceof Comparable<?>) { + try { + return Comparator.<Comparable> naturalOrder().compare((Comparable) quid, (Comparable) quo); + } catch (Exception e) { + // apparently not mutually comparable + } + } + if (quid instanceof Class<?> && quo instanceof Class<?>) { + return CLASS_COMPARATOR.compare((Class<?>) quid, (Class<?>) quo); + } + return 0; + }).thenComparing((Function<Object, String>) Objects::toString)); + + private static final char INDEX_OPEN = '['; + private static final char INDEX_CLOSE = ']'; private static <T extends Path.Node> Optional<T> optional(Class<T> type, Object o) { return Optional.ofNullable(o).filter(type::isInstance).map(type::cast); @@ -43,6 +81,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Append a Node to the specified StringBuilder. + * * @param node * @param to * @return to @@ -68,6 +107,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Get a NodeImpl indexed from the preceding node (or root). + * * @param index * @return NodeImpl */ @@ -79,6 +119,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Get a NodeImpl keyed from the preceding node (or root). + * * @param key * @return NodeImpl */ @@ -88,6 +129,66 @@ public abstract class NodeImpl implements Path.Node, Serializable { return result; } + private static int compareIterability(Node quid, Node quo) { + if (quid.isInIterable()) { + if (quo.isInIterable()) { + if (quid.getKey() != null) { + return Comparator.comparing(Node::getKey, KEY_COMPARATOR).compare(quid, quo); + } + if (quid.getIndex() == null) { + // this method cannot consistently order iterables without key or index; the first argument is + // always assumed to be less: + return -1; + } + return quo.getIndex() == null ? 1 : quid.getIndex().compareTo(quo.getIndex()); + } + return 1; + } + return quo.isInIterable() ? -1 : 0; + } + + private static int compareSpecificNodeInfo(Node quid, Node quo) { + final ElementKind kind = quid.getKind(); + final int k = kind.compareTo(quo.getKind()); + if (k != 0) { + return k; + } + final Comparator<Node> cmp; + switch (kind) { + case BEAN: + cmp = comparing(to(BeanNode.class), comparing(BeanNode::getContainerClass, CLASS_COMPARATOR) + .thenComparing(BeanNode::getTypeArgumentIndex, nullsFirst(naturalOrder()))); + break; + case PROPERTY: + cmp = comparing(to(PropertyNode.class), comparing(PropertyNode::getContainerClass, CLASS_COMPARATOR) + .thenComparing(PropertyNode::getTypeArgumentIndex, nullsFirst(naturalOrder()))); + break; + case CONTAINER_ELEMENT: + cmp = comparing(to(ContainerElementNode.class), + comparing(ContainerElementNode::getContainerClass, CLASS_COMPARATOR) + .thenComparing(ContainerElementNode::getTypeArgumentIndex, nullsFirst(naturalOrder()))); + break; + case CONSTRUCTOR: + cmp = comparing(to(ConstructorNode.class).andThen(ConstructorNode::getParameterTypes), + Comparators.comparingIterables(CLASS_COMPARATOR)); + break; + case METHOD: + cmp = comparing(to(MethodNode.class).andThen(MethodNode::getParameterTypes), + Comparators.comparingIterables(CLASS_COMPARATOR)); + break; + case PARAMETER: + cmp = comparing(to(ParameterNode.class).andThen(ParameterNode::getParameterIndex)); + break; + default: + return 0; + } + return cmp.compare(quid, quo); + } + + private static <T> Function<Object, T> to(Class<T> type) { + return type::cast; + } + private String name; private boolean inIterable; private Integer index; @@ -99,6 +200,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Create a new NodeImpl instance. + * * @param name */ private NodeImpl(String name) { @@ -107,6 +209,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Create a new NodeImpl instance. + * * @param node */ NodeImpl(Path.Node node) { @@ -141,7 +244,8 @@ public abstract class NodeImpl implements Path.Node, Serializable { } /** - * @param name the name to set + * @param name + * the name to set */ public void setName(String name) { this.name = name; @@ -157,6 +261,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Set whether this node represents a contained value of an {@link Iterable} or {@link Map}. + * * @param inIterable */ public void setInIterable(boolean inIterable) { @@ -173,6 +278,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Set the index of this node, implying <code>inIterable</code>. + * * @param index */ public void setIndex(Integer index) { @@ -195,6 +301,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { /** * Set the map key of this node, implying <code>inIterable</code>. + * * @param key */ public void setKey(Object key) { @@ -229,10 +336,7 @@ public abstract class NodeImpl implements Path.Node, Serializable { if (o == null || !getClass().equals(o.getClass())) { return false; } - final NodeImpl node = (NodeImpl) o; - - return inIterable == node.inIterable && Objects.equals(index, node.index) && Objects.equals(key, node.key) - && Objects.equals(name, node.name); + return NODE_COMPARATOR.compare(this, (NodeImpl) o) == 0; } /** http://git-wip-us.apache.org/repos/asf/bval/blob/ea55f78a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java ---------------------------------------------------------------------- diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java index e96a0de..3289a47 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java @@ -19,6 +19,7 @@ package org.apache.bval.jsr.util; import java.io.Serializable; +import java.util.Comparator; import java.util.Iterator; import java.util.LinkedList; import java.util.List; @@ -26,6 +27,7 @@ import java.util.Objects; import javax.validation.Path; +import org.apache.bval.util.Comparators; import org.apache.bval.util.Exceptions; /** @@ -39,6 +41,8 @@ public class PathImpl implements Path, Serializable { private static final long serialVersionUID = 1L; + public static final Comparator<Path> PATH_COMPARATOR = Comparators.comparingIterables(NodeImpl.NODE_COMPARATOR); + static final String PROPERTY_PATH_SEPARATOR = "."; /** http://git-wip-us.apache.org/repos/asf/bval/blob/ea55f78a/bval-jsr/src/main/java/org/apache/bval/util/Comparators.java ---------------------------------------------------------------------- diff --git a/bval-jsr/src/main/java/org/apache/bval/util/Comparators.java b/bval-jsr/src/main/java/org/apache/bval/util/Comparators.java new file mode 100644 index 0000000..940612c --- /dev/null +++ b/bval-jsr/src/main/java/org/apache/bval/util/Comparators.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.bval.util; + +import java.util.Comparator; +import java.util.Iterator; + +/** + * {@link Comparator} related utilities. + */ +public class Comparators { + + /** + * Get a {@link Comparator} capable of comparing {@link Iterable}s. + * + * @param each + * @return {@link Comparator} + */ + public static <T, I extends Iterable<T>> Comparator<I> comparingIterables(Comparator<? super T> each) { + return (quid, quo) -> { + final Iterator<T> quids = quid.iterator(); + final Iterator<T> quos = quo.iterator(); + + while (quids.hasNext()) { + if (quos.hasNext()) { + final int rz = each.compare(quids.next(), quos.next()); + if (rz != 0) { + return rz; + } + continue; + } + return 1; + } + return quos.hasNext() ? -1 : 0; + }; + } + + private Comparators() { + } +}
