HV-2127 Use nodes instead of paths to track processed beans

Signed-off-by: marko-bekhta <[email protected]>

Author: marko-bekhta

engine/src/main/java/org/hibernate/validator/internal/engine/path/NodeImpl.java

@@ -502,6 +502,16 @@ public boolean equals(Object obj) {
 			return false;
 		}
 		NodeImpl other = (NodeImpl) obj;
+		return samePath( other );
+	}
+
+	boolean sameNode(NodeImpl other) {
+		if ( this == other ) {
+			return true;
+		}
+		if ( other == null ) {
+			return false;
+		}
 		if ( hashCode != -1 && other.hashCode != -1 && hashCode != other.hashCode ) {
 			return false;
 		}
@@ -570,7 +580,24 @@ else if ( !Arrays.equals( parameterTypes, other.parameterTypes ) ) {
 		return true;
 	}
 
-	boolean isRootPath() {
+	boolean samePath(NodeImpl other) {
+		if ( this.size != other.size ) {
+			return false;
+		}
+		NodeImpl curr = this;
+		NodeImpl otherCurr = other;
+		while ( curr != null && otherCurr != null ) {
+			if ( !curr.sameNode( otherCurr ) ) {
+				return false;
+			}
+			otherCurr = otherCurr.parent;
+			curr = curr.parent;
+		}
+
+		return curr == null && otherCurr == null;
+	}
+
+	public boolean isRootPath() {
 		return parent == null && name == null;
 	}
 
@@ -615,7 +642,7 @@ boolean isSubPathOf(NodeImpl other) {
 		return curr.isRootPath();
 	}
 
-	boolean isSubPathOrContains(NodeImpl other) {
+	public boolean isSubPathOrContains(NodeImpl other) {
 		NodeImpl curr;
 		NodeImpl otherCurr;
 		if ( this.size > other.size ) {
@@ -644,23 +671,6 @@ boolean isSubPathOrContains(NodeImpl other) {
 		return curr.isRootPath() && otherCurr.isRootPath();
 	}
 
-	boolean samePath(NodeImpl other) {
-		if ( this.size != other.size ) {
-			return false;
-		}
-		NodeImpl curr = this;
-		NodeImpl otherCurr = other;
-		while ( curr != null && otherCurr != null ) {
-			if ( !curr.equals( otherCurr ) ) {
-				return false;
-			}
-			otherCurr = otherCurr.parent;
-			curr = curr.parent;
-		}
-
-		return curr == null && otherCurr == null;
-	}
-
 	protected static class NodeIterator implements Iterator<Path.Node> {
 		private final NodeImpl[] array;
 		private int index;

engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java

@@ -25,6 +25,7 @@
 import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager;
 import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintViolationCreationContext;
 import org.hibernate.validator.internal.engine.path.ModifiablePath;
+import org.hibernate.validator.internal.engine.path.NodeImpl;
 import org.hibernate.validator.internal.engine.valuecontext.ValueContext;
 import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
 import org.hibernate.validator.internal.metadata.core.MetaConstraint;
@@ -109,10 +110,10 @@ abstract class AbstractValidationContext<T> implements BaseBeanValidationContext
 	private Set<BeanGroupProcessedUnit> processedGroupUnits;
 
 	/**
-	 * Maps an object to a list of paths in which it has been validated. The objects are the bean instances.
+	 * Maps an object to a list of paths (represented by leaf nodes) in which it has been validated. The objects are the bean instances.
 	 */
 	@Lazy
-	private Map<Object, Set<ModifiablePath>> processedPathsPerBean;
+	private Map<Object, Set<NodeImpl>> processedPathsPerBean;
 
 	/**
 	 * Contains all failing constraints so far.
@@ -200,11 +201,10 @@ public boolean isBeanAlreadyValidated(Object value, Class<?> group, ModifiablePa
 			return false;
 		}
 
-		boolean alreadyValidated;
-		alreadyValidated = isAlreadyValidatedForCurrentGroup( value, group );
+		boolean alreadyValidated = isAlreadyValidatedForCurrentGroup( value, group );
 
 		if ( alreadyValidated ) {
-			alreadyValidated = isAlreadyValidatedForPath( value, path );
+			alreadyValidated = isAlreadyValidatedForPath( value, path.getLeafNode() );
 		}
 
 		return alreadyValidated;
@@ -270,7 +270,7 @@ protected abstract ConstraintViolation<T> createConstraintViolation(
 			ConstraintViolationCreationContext constraintViolationCreationContext);
 
 	@Override
-	public boolean hasMetaConstraintBeenProcessed(Object bean, Path path, MetaConstraint<?> metaConstraint) {
+	public boolean hasMetaConstraintBeenProcessed(Object bean, ModifiablePath path, MetaConstraint<?> metaConstraint) {
 		// this is only useful if the constraint is defined for more than 1 group as in the case it's only
 		// defined for one group, there is no chance it's going to be called twice.
 		if ( metaConstraint.isDefinedForOneGroupOnly() ) {
@@ -281,7 +281,7 @@ public boolean hasMetaConstraintBeenProcessed(Object bean, Path path, MetaConstr
 	}
 
 	@Override
-	public void markConstraintProcessed(Object bean, Path path, MetaConstraint<?> metaConstraint) {
+	public void markConstraintProcessed(Object bean, ModifiablePath path, MetaConstraint<?> metaConstraint) {
 		// this is only useful if the constraint is defined for more than 1 group as in the case it's only
 		// defined for one group, there is no chance it's going to be called twice.
 		if ( metaConstraint.isDefinedForOneGroupOnly() ) {
@@ -340,8 +340,8 @@ private String interpolate(
 		}
 	}
 
-	private boolean isAlreadyValidatedForPath(Object value, ModifiablePath path) {
-		Set<ModifiablePath> pathSet = getInitializedProcessedPathsPerBean().get( value );
+	private boolean isAlreadyValidatedForPath(Object value, NodeImpl path) {
+		Set<NodeImpl> pathSet = getInitializedProcessedPathsPerBean().get( value );
 		if ( pathSet == null ) {
 			return false;
 		}
@@ -354,7 +354,7 @@ private boolean isAlreadyValidatedForPath(Object value, ModifiablePath path) {
 		// it means that the new path we are testing cannot be a root path; also since we are cascading into inner
 		// objects, i.e. going further from the object tree root, it means that the new path cannot be shorter than
 		// the ones we've already encountered.
-		for ( ModifiablePath p : pathSet ) {
+		for ( NodeImpl p : pathSet ) {
 			if ( p.isSubPathOrContains( path ) ) {
 				return true;
 			}
@@ -367,16 +367,16 @@ private boolean isAlreadyValidatedForCurrentGroup(Object value, Class<?> group)
 	}
 
 	private void markCurrentBeanAsProcessedForCurrentPath(Object bean, ModifiablePath path) {
-		// HV-1031 The path object is mutated as we traverse the object tree, hence copy it before saving it
-		Map<Object, Set<ModifiablePath>> processedPathsPerBean = getInitializedProcessedPathsPerBean();
+		Map<Object, Set<NodeImpl>> processedPathsPerBean = getInitializedProcessedPathsPerBean();
 
-		Set<ModifiablePath> processedPaths = processedPathsPerBean.get( bean );
+		Set<NodeImpl> processedPaths = processedPathsPerBean.get( bean );
 		if ( processedPaths == null ) {
 			processedPaths = new HashSet<>();
 			processedPathsPerBean.put( bean, processedPaths );
 		}
 
-		processedPaths.add( ModifiablePath.createCopy( path ) );
+		// HV-1031 The path object is mutated as we traverse the object tree, hence we use the node which is not (for the most part):
+		processedPaths.add( path.getLeafNode() );
 	}
 
 	private void markCurrentBeanAsProcessedForCurrentGroup(Object bean, Class<?> group) {
@@ -397,7 +397,7 @@ private Set<BeanGroupProcessedUnit> getInitializedProcessedGroupUnits() {
 		return processedGroupUnits;
 	}
 
-	private Map<Object, Set<ModifiablePath>> getInitializedProcessedPathsPerBean() {
+	private Map<Object, Set<NodeImpl>> getInitializedProcessedPathsPerBean() {
 		if ( processedPathsPerBean == null ) {
 			processedPathsPerBean = new IdentityHashMap<>();
 		}
@@ -415,13 +415,13 @@ private static final class BeanPathMetaConstraintProcessedUnit {
 
 		// these fields are final but we don't mark them as final as an optimization
 		private Object bean;
-		private Path path;
+		private NodeImpl pathLeaf;
 		private MetaConstraint<?> metaConstraint;
 		private int hashCode;
 
-		BeanPathMetaConstraintProcessedUnit(Object bean, Path path, MetaConstraint<?> metaConstraint) {
+		BeanPathMetaConstraintProcessedUnit(Object bean, ModifiablePath path, MetaConstraint<?> metaConstraint) {
 			this.bean = bean;
-			this.path = path;
+			this.pathLeaf = path.getLeafNode(); // because the leaf represent the entire path.
 			this.metaConstraint = metaConstraint;
 			this.hashCode = createHashCode();
 		}
@@ -442,7 +442,7 @@ public boolean equals(Object o) {
 			if ( metaConstraint != that.metaConstraint ) {
 				return false;
 			}
-			if ( !path.equals( that.path ) ) {
+			if ( !pathLeaf.equals( that.pathLeaf ) ) {
 				return false;
 			}
 
@@ -456,7 +456,7 @@ public int hashCode() {
 
 		private int createHashCode() {
 			int result = System.identityHashCode( bean );
-			result = 31 * result + path.hashCode();
+			result = 31 * result + pathLeaf.hashCode();
 			result = 31 * result + System.identityHashCode( metaConstraint );
 			return result;
 		}

engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/BaseBeanValidationContext.java

@@ -4,7 +4,6 @@
  */
 package org.hibernate.validator.internal.engine.validationcontext;
 
-import jakarta.validation.Path;
 import jakarta.validation.TraversableResolver;
 import jakarta.validation.Validator;
 
@@ -39,9 +38,9 @@ public interface BaseBeanValidationContext<T> extends ValidationContext<T> {
 
 	void markCurrentBeanAsProcessed(ValueContext<?, ?> valueContext);
 
-	boolean hasMetaConstraintBeenProcessed(Object bean, Path path, MetaConstraint<?> metaConstraint);
+	boolean hasMetaConstraintBeenProcessed(Object bean, ModifiablePath path, MetaConstraint<?> metaConstraint);
 
-	void markConstraintProcessed(Object bean, Path path, MetaConstraint<?> metaConstraint);
+	void markConstraintProcessed(Object bean, ModifiablePath path, MetaConstraint<?> metaConstraint);
 
 	/**
 	 * @return {@code true} if current validation context can and should process passed meta constraint. Is used in