diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java index 75a99ece96..3515a393ae 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java @@ -83,8 +83,17 @@ public ConstraintViolationImpl(String messageTemplate, String message, T rootBea this.elementType = elementType; this.returnValue = returnValue; this.parameters = parameters; - this.hashCode = Arrays.deepHashCode(new Object[] { messageTemplate, message, rootBean, rootBeanClass, leafBean, - value, propertyPath, elementType, constraintDescriptor, returnValue, parameters }); + int h = Objects.hashCode(messageTemplate); + h = 31 * h + Objects.hashCode(message); + h = 31 * h + Objects.hashCode(rootBean); + h = 31 * h + Objects.hashCode(rootBeanClass); + h = 31 * h + Objects.hashCode(leafBean); + h = 31 * h + Objects.hashCode(value); + h = 31 * h + Objects.hashCode(propertyPath); + h = 31 * h + Objects.hashCode(elementType); + h = 31 * h + Objects.hashCode(constraintDescriptor); + h = 31 * h + Objects.hashCode(returnValue); + this.hashCode = 31 * h + Arrays.deepHashCode(parameters); } /** 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 e65f3be4e2..ae4e0ad1cb 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 @@ -65,6 +65,12 @@ public ApacheFactoryContext getValidatorContext() { } public PathImpl getPath() { + final PathImpl p = path; + if (p == null && pendingNode == null) { + // derive a private copy straight from the parent rather than materializing this context's path only to + // copy it again; the mutation appends fresh nodes, so nothing is shared with other copies + return derivePath(); + } return PathImpl.copy(pathReference()); } @@ -77,17 +83,22 @@ public PathImpl getPath() { public PathImpl pathReference() { PathImpl p = path; if (p == null) { - p = PathImpl.copy(parent.pathReference()); - if (pendingNode != null) { - p.addNode(pendingNode); - } else { - pendingMutation.accept(p); - } + p = derivePath(); path = p; } return p; } + private PathImpl derivePath() { + final PathImpl p = PathImpl.copy(parent.pathReference()); + if (pendingNode != null) { + p.addNode(pendingNode); + } else { + pendingMutation.accept(p); + } + return p; + } + public Object getValue() { return value; } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java index 39bd290f2a..40ac708019 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java @@ -19,6 +19,7 @@ package org.apache.bval.jsr.job; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import jakarta.validation.ClockProvider; @@ -137,12 +138,20 @@ private PathImpl extensiblePath() { private final ConstraintD constraint; // typically holds a single violation; a list with a containment check keeps set semantics cheaply private List> violations; + /** Whether violations are only counted, not built, because the caller discards them. */ + private final boolean discardViolations; private boolean defaultConstraintViolationDisabled; ConstraintValidatorContextImpl(ValidationJob.Frame frame, ConstraintD constraint) { + this(frame, constraint, false); + } + + ConstraintValidatorContextImpl(ValidationJob.Frame frame, ConstraintD constraint, + boolean discardViolations) { super(); this.frame = Validate.notNull(frame, "frame"); this.constraint = Validate.notNull(constraint, "constraint"); + this.discardViolations = discardViolations; } @Override @@ -183,6 +192,10 @@ List> getRequiredViolations() { if (defaultConstraintViolationDisabled) { Exceptions.raise(ValidationException::new, "Expected custom constraint violation(s)"); } + if (discardViolations) { + violations = Collections.emptyList(); + return violations; + } addError(getDefaultConstraintMessageTemplate(), frame.context.getPath()); } return violations; @@ -200,6 +213,10 @@ public Object getValidatedValue() { @SuppressWarnings({ "unchecked", "rawtypes" }) private void addError(String messageTemplate, PathImpl propertyPath) { + if (discardViolations) { + violations = Collections.emptyList(); + return; + } if (violations == null) { violations = new ArrayList<>(2); } 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 6f469f4584..c58f02c2e0 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 @@ -32,7 +32,6 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; -import java.util.stream.Collectors; import java.util.stream.Stream; import jakarta.validation.ConstraintValidator; @@ -46,6 +45,7 @@ import jakarta.validation.constraintvalidation.ValidationTarget; import jakarta.validation.groups.Default; import jakarta.validation.metadata.CascadableDescriptor; +import jakarta.validation.metadata.ConstraintDescriptor; import jakarta.validation.metadata.ContainerDescriptor; import jakarta.validation.metadata.ContainerElementTypeDescriptor; import jakarta.validation.metadata.GroupConversionDescriptor; @@ -67,6 +67,7 @@ import org.apache.bval.jsr.groups.GroupStrategy; import org.apache.bval.jsr.groups.Groups; import org.apache.bval.jsr.metadata.ContainerElementKey; +import org.apache.bval.jsr.resolver.DefaultTraversableResolver; import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.jsr.valueextraction.ExtractValues; @@ -149,7 +150,7 @@ private boolean validate(ConstraintD constraint, Consumer constraintValidatorContext = - new ConstraintValidatorContextImpl<>(this, constraint); + new ConstraintValidatorContextImpl<>(this, constraint, sink == DISCARD); final boolean valid; if (constraintValidator == null) { @@ -184,14 +185,16 @@ private boolean validateComposed(ConstraintD constraint, Consumer> effectiveSink = constraint.isReportAsSingleViolation() ? cv -> { - } : sink; - - // collect validation results to set of Boolean, ensuring all are evaluated: - final Set validationResults = constraint.getComposingConstraints().stream().map(ConstraintD.class::cast) - .map(c -> validate(c, effectiveSink)).collect(Collectors.toSet()); - - return Collections.singleton(Boolean.TRUE).equals(validationResults); + @SuppressWarnings("unchecked") + final Consumer> effectiveSink = + constraint.isReportAsSingleViolation() ? (Consumer>) DISCARD : sink; + + // evaluate all composing constraints, even after one has failed: + boolean allValid = true; + for (final ConstraintDescriptor c : constraint.getComposingConstraints()) { + allValid &= validate((ConstraintD) c, effectiveSink); + } + return allValid; } @SuppressWarnings({ "rawtypes" }) @@ -304,17 +307,19 @@ private Set> propertyFrames() { final Set> frames = new HashSet<>(); for (final PropertyDescriptor pd : descriptor.getConstrainedProperties()) { ComposedD.forEachUnwrapped(pd, PropertyD.class, d -> { - final PathImpl p = realContext.getPath(); - p.addProperty(d.getPropertyName()); - try { - if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), getRootBeanClass(), - p, d.getElementType())) { - return; + if (!traversesAll(traversableResolver)) { + final PathImpl p = realContext.getPath(); + p.addProperty(d.getPropertyName()); + try { + if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), + getRootBeanClass(), p, d.getElementType())) { + return; + } + } catch (ValidationException ve) { + throw ve; + } catch (Exception e) { + throw new ValidationException(e); } - } catch (ValidationException ve) { - throw ve; - } catch (Exception e) { - throw new ValidationException(e); } for (final Iterator it = d.read(realContext).iterator(); it.hasNext();) { final GraphContext child = it.next(); @@ -393,8 +398,8 @@ private void cascade(GroupStrategy groups, Consumer> sink if (!descriptor.isCascaded()) { return; } - if (descriptor instanceof PropertyDescriptor) { - final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + if (descriptor instanceof PropertyDescriptor && !traversesAll(traversableResolver)) { final Object traversableObject = Optional.ofNullable(context.getParent()).map(GraphContext::getValue).orElse(null); @@ -570,6 +575,13 @@ Object getBean() { } } + /** + * Sink for the violations of constraints composing one reported as a single violation. Recognized by identity so + * that such violations are not built at all. + */ + private static final Consumer DISCARD = cv -> { + }; + protected static final TypeVariable MAP_VALUE = Map.class.getTypeParameters()[1]; protected static final TypeVariable ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0]; @@ -595,6 +607,15 @@ private static boolean matchesGroups(ConstraintD constraint, Set targe return false; } + /** + * Learn whether {@code traversableResolver} is known to report every property as reachable and cascadable, so + * that consulting it (and building the paths it takes) can be skipped. + */ + private static boolean traversesAll(TraversableResolver traversableResolver) { + return traversableResolver.getClass() == DefaultTraversableResolver.class + && ((DefaultTraversableResolver) traversableResolver).isTraverseAll(); + } + protected final ApacheFactoryContext validatorContext; protected final Groups groups; diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java b/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java index af88caf8f3..ff514e27f9 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java @@ -49,6 +49,16 @@ public DefaultTraversableResolver() { initJpa(); } + /** + * Learn whether this resolver reports every property as both reachable and cascadable, i.e. whether calls to it + * can be skipped altogether. + * + * @return {@code true} if no JPA-aware resolver is in use + */ + public boolean isTraverseAll() { + return jpaTR == null; + } + /** * {@inheritDoc} */ 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 14eaab025e..754a30e92a 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 @@ -383,7 +383,11 @@ public boolean equals(Object o) { */ @Override public int hashCode() { - return Objects.hash(name, Boolean.valueOf(inIterable), index, key, getKind()); + int h = Objects.hashCode(name); + h = 31 * h + Boolean.hashCode(inIterable); + h = 31 * h + Objects.hashCode(index); + h = 31 * h + Objects.hashCode(key); + return 31 * h + Objects.hashCode(getKind()); } public int getParameterIndex() { 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 5cd389df33..5e87ac85e2 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,9 +19,9 @@ package org.apache.bval.jsr.util; import java.io.Serializable; +import java.util.ArrayList; import java.util.Comparator; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -164,7 +164,7 @@ private static boolean isAwaitingPropertyName(NodeImpl n) { return n != null && n.getName() == null && (n.isInIterable() || n.getContainerClass() != null); } - private final LinkedList nodeList = new LinkedList<>(); + private final ArrayList nodeList; // Copy-on-write for the leaf node. copy() of another PathImpl shares node references instead of deep-copying // (nodes are effectively immutable once they are interior; only the current leaf is ever mutated in place). @@ -172,19 +172,25 @@ private static boolean isAwaitingPropertyName(NodeImpl n) { private boolean sharedLeaf; private PathImpl() { + nodeList = new ArrayList<>(4); } private PathImpl(Path path) { if (path instanceof PathImpl) { final PathImpl source = (PathImpl) path; // share node references; the leaf will be copied on first mutation by whichever path mutates it - nodeList.addAll(source.nodeList); + final int size = source.nodeList.size(); + nodeList = new ArrayList<>(size + 2); + for (int i = 0; i < size; i++) { + nodeList.add(source.nodeList.get(i)); + } if (!nodeList.isEmpty()) { sharedLeaf = true; // the source now shares its leaf with this copy, so it must copy-on-write too source.sharedLeaf = true; } } else { + nodeList = new ArrayList<>(); path.forEach(n -> nodeList.add(newNode(n))); } } @@ -229,7 +235,7 @@ public boolean isRootPath() { if (nodeList.size() != 1) { return false; } - final Path.Node first = nodeList.peekFirst(); + final Path.Node first = nodeList.get(0); return !first.isInIterable() && first.getName() == null; } @@ -243,7 +249,7 @@ public boolean isRootPath() { public PathImpl addNode(Node node) { final NodeImpl impl = node instanceof NodeImpl ? (NodeImpl) node : newNode(node); if (isRootPath()) { - nodeList.pop(); + nodeList.remove(0); } nodeList.add(impl); // the appended node is caller-owned/fresh, so the new leaf is exclusively owned @@ -298,7 +304,7 @@ public NodeImpl removeLeafNode() { Exceptions.raiseIf(isRootPath() || nodeList.isEmpty(), IllegalStateException::new, "No nodes in path!"); try { - return nodeList.removeLast(); + return nodeList.remove(nodeList.size() - 1); } finally { if (nodeList.isEmpty()) { nodeList.add(new NodeImpl.BeanNodeImpl()); @@ -336,7 +342,7 @@ public NodeImpl getNode(int index) { public NodeImpl getRootNode() { Exceptions.raiseIf(nodeList.isEmpty(), IndexOutOfBoundsException::new, "Path is empty"); - return nodeList.peekFirst(); + return nodeList.get(0); } /** @@ -346,7 +352,7 @@ public NodeImpl getRootNode() { public NodeImpl getLeafNode() { Exceptions.raiseIf(nodeList.isEmpty(), IndexOutOfBoundsException::new, "Path is empty"); - return nodeList.peekLast(); + return nodeList.get(nodeList.size() - 1); } /**