HV-1831 Initialize processed groups lazily

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

Author: marko-bekhta

engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java

@@ -160,7 +160,7 @@ public final <T> Set<ConstraintViolation<T>> validate(T object, Class<?>... grou
 		BaseBeanValidationContext<T> validationContext = getValidationContextBuilder().forValidate( rootBeanClass, rootBeanMetaData, object );
 
 		ValidationOrder validationOrder = determineGroupValidationOrder( groups );
-		BeanValueContext<?, Object> valueContext = ValueContexts.getLocalExecutionContextForBean(
+		BeanValueContext<?, Object> valueContext = ValueContexts.getLocalExecutionContextForRootBean(
 				validatorScopedContext.getParameterNameProvider(),
 				object,
 				validationContext.getRootBeanMetaData(),
@@ -648,7 +648,7 @@ private void validateCascadedAnnotatedObjectForCurrentGroup(Object value, BaseBe
 		Class<?> originalGroup = valueContext.getCurrentGroup();
 		Class<?> currentGroup = cascadingMetaData.convertGroup( originalGroup );
 
-		if ( validationContext.isBeanAlreadyValidated( value, currentGroup, valueContext.getPropertyPath() )
+		if ( validationContext.isBeanAlreadyValidated( value, currentGroup, valueContext )
 				|| shouldFailFast( validationContext ) ) {
 			return;
 		}
@@ -728,7 +728,7 @@ private void doValidate(Object value, String nodeName) {
 			Class<?> currentGroup = cascadingMetaData.convertGroup( originalGroup );
 
 			if ( value == null
-					|| validationContext.isBeanAlreadyValidated( value, currentGroup, valueContext.getPropertyPath() )
+					|| validationContext.isBeanAlreadyValidated( value, currentGroup, valueContext )
 					|| shouldFailFast( validationContext ) ) {
 				return;
 			}
@@ -803,6 +803,7 @@ private BeanValueContext<?, Object> buildNewLocalExecutionContext(ValueContext<?
 		Contracts.assertNotNull( value, "value cannot be null" );
 		BeanMetaData<?> beanMetaData = beanMetaDataManager.getBeanMetaData( value.getClass() );
 		newValueContext = ValueContexts.getLocalExecutionContextForBean(
+				valueContext,
 				validatorScopedContext.getParameterNameProvider(),
 				value,
 				beanMetaData,
@@ -1246,7 +1247,7 @@ else if ( propertyPathNode.getKey() != null ) {
 
 		propertyPath.removeLeafNode();
 
-		return ValueContexts.getLocalExecutionContextForBean( validatorScopedContext.getParameterNameProvider(), value, beanMetaData, propertyPath );
+		return ValueContexts.getLocalExecutionContextForRootBean( validatorScopedContext.getParameterNameProvider(), value, beanMetaData, propertyPath );
 	}
 
 	/**

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

@@ -7,7 +7,6 @@
 import java.lang.invoke.MethodHandles;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.Set;
 
@@ -103,18 +102,6 @@ abstract class AbstractValidationContext<T> implements BaseBeanValidationContext
 	@Lazy
 	private Set<BeanPathMetaConstraintProcessedUnit> processedPathUnits;
 
-	/**
-	 * The set of already processed groups per bean ({@link BeanGroupProcessedUnit}).
-	 */
-	@Lazy
-	private Set<BeanGroupProcessedUnit> processedGroupUnits;
-
-	/**
-	 * 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<NodeImpl>> processedPathsPerBean;
-
 	/**
 	 * Contains all failing constraints so far.
 	 */
@@ -196,28 +183,19 @@ public ConstraintValidatorFactory getConstraintValidatorFactory() {
 	}
 
 	@Override
-	public boolean isBeanAlreadyValidated(Object value, Class<?> group, ModifiablePath path) {
+	public boolean isBeanAlreadyValidated(Object value, Class<?> group, ValueContext<?, ?> valueContext) {
 		if ( !processedBeanTrackingEnabled ) {
 			return false;
 		}
-
-		boolean alreadyValidated = isAlreadyValidatedForCurrentGroup( value, group );
-
-		if ( alreadyValidated ) {
-			alreadyValidated = isAlreadyValidatedForPath( value, path.getLeafNode() );
-		}
-
-		return alreadyValidated;
+		return valueContext.isBeanAlreadyValidated( value, group );
 	}
 
 	@Override
 	public void markCurrentBeanAsProcessed(ValueContext<?, ?> valueContext) {
 		if ( !processedBeanTrackingEnabled ) {
 			return;
 		}
-
-		markCurrentBeanAsProcessedForCurrentGroup( valueContext.getCurrentBean(), valueContext.getCurrentGroup() );
-		markCurrentBeanAsProcessedForCurrentPath( valueContext.getCurrentBean(), valueContext.getPropertyPath() );
+		valueContext.markCurrentGroupAsProcessed();
 	}
 
 	@Override
@@ -340,70 +318,13 @@ private String interpolate(
 		}
 	}
 
-	private boolean isAlreadyValidatedForPath(Object value, NodeImpl path) {
-		Set<NodeImpl> pathSet = getInitializedProcessedPathsPerBean().get( value );
-		if ( pathSet == null ) {
-			return false;
-		}
-
-		if ( path.isRootPath() ) {
-			return true;
-		}
-
-		// Since this isAlreadyValidatedForPath(..) is only applicable for an object that is about to be cascaded into,
-		// 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 ( NodeImpl p : pathSet ) {
-			if ( p.isSubPathOrContains( path ) ) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-	private boolean isAlreadyValidatedForCurrentGroup(Object value, Class<?> group) {
-		return getInitializedProcessedGroupUnits().contains( new BeanGroupProcessedUnit( value, group ) );
-	}
-
-	private void markCurrentBeanAsProcessedForCurrentPath(Object bean, ModifiablePath path) {
-		Map<Object, Set<NodeImpl>> processedPathsPerBean = getInitializedProcessedPathsPerBean();
-
-		Set<NodeImpl> processedPaths = processedPathsPerBean.get( bean );
-		if ( processedPaths == null ) {
-			processedPaths = new HashSet<>();
-			processedPathsPerBean.put( bean, processedPaths );
-		}
-
-		// 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) {
-		getInitializedProcessedGroupUnits().add( new BeanGroupProcessedUnit( bean, group ) );
-	}
-
 	private Set<BeanPathMetaConstraintProcessedUnit> getInitializedProcessedPathUnits() {
 		if ( processedPathUnits == null ) {
 			processedPathUnits = new HashSet<>();
 		}
 		return processedPathUnits;
 	}
 
-	private Set<BeanGroupProcessedUnit> getInitializedProcessedGroupUnits() {
-		if ( processedGroupUnits == null ) {
-			processedGroupUnits = new HashSet<>();
-		}
-		return processedGroupUnits;
-	}
-
-	private Map<Object, Set<NodeImpl>> getInitializedProcessedPathsPerBean() {
-		if ( processedPathsPerBean == null ) {
-			processedPathsPerBean = new IdentityHashMap<>();
-		}
-		return processedPathsPerBean;
-	}
-
 	private Set<ConstraintViolation<T>> getInitializedFailingConstraintViolations() {
 		if ( failingConstraintViolations == null ) {
 			failingConstraintViolations = new HashSet<>();
@@ -462,48 +383,4 @@ private int createHashCode() {
 		}
 	}
 
-	private static final class BeanGroupProcessedUnit {
-
-		// these fields are final but we don't mark them as final as an optimization
-		private Object bean;
-		private Class<?> group;
-		private int hashCode;
-
-		BeanGroupProcessedUnit(Object bean, Class<?> group) {
-			this.bean = bean;
-			this.group = group;
-			this.hashCode = createHashCode();
-		}
-
-		@Override
-		public boolean equals(Object o) {
-			// null check intentionally left out
-			if ( this == o ) {
-				return true;
-			}
-
-			// No need to check if the class matches because of how this class is used in the set.
-			BeanGroupProcessedUnit that = (BeanGroupProcessedUnit) o;
-
-			if ( bean != that.bean ) { // instance equality
-				return false;
-			}
-			if ( !group.equals( that.group ) ) {
-				return false;
-			}
-
-			return true;
-		}
-
-		@Override
-		public int hashCode() {
-			return hashCode;
-		}
-
-		private int createHashCode() {
-			int result = System.identityHashCode( bean );
-			result = 31 * result + group.hashCode();
-			return result;
-		}
-	}
 }

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

@@ -34,7 +34,7 @@ public interface BaseBeanValidationContext<T> extends ValidationContext<T> {
 
 	TraversableResolver getTraversableResolver();
 
-	boolean isBeanAlreadyValidated(Object value, Class<?> group, ModifiablePath path);
+	boolean isBeanAlreadyValidated(Object value, Class<?> group, ValueContext<?, ?> valueContext);
 
 	void markCurrentBeanAsProcessed(ValueContext<?, ?> valueContext);
 

engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/BeanValueContext.java

@@ -4,26 +4,67 @@
  */
 package org.hibernate.validator.internal.engine.valuecontext;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.hibernate.validator.internal.engine.path.ModifiablePath;
 import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
 import org.hibernate.validator.internal.util.ExecutableParameterNameProvider;
+import org.hibernate.validator.internal.util.stereotypes.Lazy;
 
 /**
  * @author Marko Bekhta
  */
-public class BeanValueContext<T, V> extends ValueContext<T, V> {
+public final class BeanValueContext<T, V> extends ValueContext<T, V> {
 
 	/**
 	 * The metadata of the current bean.
 	 */
 	private final BeanMetaData<T> currentBeanMetaData;
 
-	BeanValueContext(ExecutableParameterNameProvider parameterNameProvider, T currentBean, BeanMetaData<T> currentBeanMetaData, ModifiablePath propertyPath) {
-		super( parameterNameProvider, currentBean, currentBeanMetaData, propertyPath );
+	/**
+	 * When we check whether the bean was validated we need to check that it was validated for the requested group.
+	 * This set tracks the groups we've already processed this bean for.
+	 */
+	@Lazy
+	private Set<Class<?>> alreadyProcessedGroups;
+
+	BeanValueContext(ValueContext<?, ?> parentContext, ExecutableParameterNameProvider parameterNameProvider, T currentBean, BeanMetaData<T> currentBeanMetaData, ModifiablePath propertyPath) {
+		super( parentContext, parameterNameProvider, currentBean, currentBeanMetaData, propertyPath );
 		this.currentBeanMetaData = currentBeanMetaData;
+		this.alreadyProcessedGroups = new HashSet<>();
 	}
 
-	public final BeanMetaData<T> getCurrentBeanMetaData() {
+	public BeanMetaData<T> getCurrentBeanMetaData() {
 		return currentBeanMetaData;
 	}
+
+	@Override
+	public boolean isBeanAlreadyValidated(Object value, Class<?> group) {
+		ValueContext<?, ?> curr = this;
+		while ( curr != null ) {
+			if ( curr.currentBean == value ) {
+				return curr.isProcessedForGroup( group );
+			}
+			curr = curr.parentContext;
+		}
+		return false;
+	}
+
+	@Override
+	public void markCurrentGroupAsProcessed() {
+		// if we just validate the default/single group it doesn't make sense to track it beyond the "current group" value
+		if ( this.previousGroup != null && this.previousGroup != this.currentGroup ) {
+			if ( this.alreadyProcessedGroups == null ) {
+				this.alreadyProcessedGroups = new HashSet<>();
+				this.alreadyProcessedGroups.add( this.previousGroup );
+			}
+			this.alreadyProcessedGroups.add( this.currentGroup );
+		}
+	}
+
+	@Override
+	protected boolean isProcessedForGroup(Class<?> group) {
+		return group == this.currentGroup || ( this.alreadyProcessedGroups != null && alreadyProcessedGroups.contains( group ) );
+	}
 }

engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ExecutableValueContext.java

@@ -0,0 +1,36 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.validator.internal.engine.valuecontext;
+
+import org.hibernate.validator.internal.engine.path.ModifiablePath;
+import org.hibernate.validator.internal.metadata.facets.Validatable;
+import org.hibernate.validator.internal.util.ExecutableParameterNameProvider;
+
+/**
+ * @author Marko Bekhta
+ */
+public final class ExecutableValueContext<T, V> extends ValueContext<T, V> {
+
+	ExecutableValueContext(ValueContext<?, ?> parentContext, ExecutableParameterNameProvider parameterNameProvider, T currentBean, Validatable validatable, ModifiablePath propertyPath) {
+		super( parentContext, parameterNameProvider, currentBean, validatable, propertyPath );
+	}
+
+	@Override
+	public boolean isBeanAlreadyValidated(Object value, Class<?> group) {
+		// executables start with the root bean as a value of an object that that executable is "called" on,
+		// but we haven't really validated that root, so we need to ignore it:
+		return false;
+	}
+
+	@Override
+	public void markCurrentGroupAsProcessed() {
+		// do nothing
+	}
+
+	@Override
+	protected boolean isProcessedForGroup(Class<?> group) {
+		return false;
+	}
+}