From 0eb0e178f2e689ecc7b5d37c636054f7a658576f Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Wed, 24 Feb 2021 19:10:09 +0100 Subject: [PATCH 1/5] HV-1831 Unfinished experiments --- .../BaseHibernateValidatorConfiguration.java | 4 + .../engine/AbstractConfigurationImpl.java | 18 +++++ .../PredefinedScopeValidatorFactoryImpl.java | 46 +++++++---- .../internal/engine/ValidatorFactoryImpl.java | 7 +- .../engine/ValidatorFactoryScopedContext.java | 28 ++++++- ...adablesProcessedBeansTrackingStrategy.java | 31 +++++++ ...edScopeProcessedBeansTrackingStrategy.java | 65 +++++++++++++++ .../ProcessedBeansTrackingStrategy.java | 20 +++++ .../AbstractValidationContext.java | 63 +++++++++++---- .../BeanValidationContext.java | 9 ++- .../ParameterExecutableValidationContext.java | 11 ++- ...eturnValueExecutableValidationContext.java | 10 ++- .../ValidatorScopedContext.java | 11 +++ .../PredefinedScopeBeanMetaDataManager.java | 5 ++ performance/pom.xml | 80 ++++++++++++++----- 15 files changed, 342 insertions(+), 66 deletions(-) create mode 100644 engine/src/main/java/org/hibernate/validator/internal/engine/tracking/HasCascadablesProcessedBeansTrackingStrategy.java create mode 100644 engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java create mode 100644 engine/src/main/java/org/hibernate/validator/internal/engine/tracking/ProcessedBeansTrackingStrategy.java diff --git a/engine/src/main/java/org/hibernate/validator/BaseHibernateValidatorConfiguration.java b/engine/src/main/java/org/hibernate/validator/BaseHibernateValidatorConfiguration.java index 7ce04ef270..97e2f5fe15 100644 --- a/engine/src/main/java/org/hibernate/validator/BaseHibernateValidatorConfiguration.java +++ b/engine/src/main/java/org/hibernate/validator/BaseHibernateValidatorConfiguration.java @@ -25,6 +25,7 @@ import org.hibernate.validator.cfg.ConstraintMapping; import org.hibernate.validator.constraints.ParameterScriptAssert; import org.hibernate.validator.constraints.ScriptAssert; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel; import org.hibernate.validator.spi.messageinterpolation.LocaleResolver; import org.hibernate.validator.metadata.BeanMetaDataClassNormalizer; @@ -480,4 +481,7 @@ default S locales(Locale... locales) { */ @Incubating S customViolationExpressionLanguageFeatureLevel(ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel); + + @Incubating + S processedBeansTrackingStrategy(ProcessedBeansTrackingStrategy processedBeanTrackingStrategy); } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java index f858c61d9e..088805e0d7 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java @@ -42,6 +42,7 @@ import org.hibernate.validator.internal.cfg.context.DefaultConstraintMapping; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorFactoryImpl; import org.hibernate.validator.internal.engine.resolver.TraversableResolvers; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor; import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager; import org.hibernate.validator.internal.properties.DefaultGetterPropertySelectionStrategy; @@ -132,6 +133,7 @@ public abstract class AbstractConfigurationImpl getProgrammaticMappings() { return programmaticMappings; } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeValidatorFactoryImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeValidatorFactoryImpl.java index ca59ece651..5b96537c63 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeValidatorFactoryImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeValidatorFactoryImpl.java @@ -9,11 +9,11 @@ import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineAllowMultipleCascadedValidationOnReturnValues; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineAllowOverridingMethodAlterParameterConstraint; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineAllowParallelMethodsDefineParameterConstraints; -import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineConstraintExpressionLanguageFeatureLevel; -import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineCustomViolationExpressionLanguageFeatureLevel; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineBeanMetaDataClassNormalizer; +import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineConstraintExpressionLanguageFeatureLevel; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineConstraintMappings; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineConstraintValidatorPayload; +import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineCustomViolationExpressionLanguageFeatureLevel; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineExternalClassLoader; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineFailFast; import static org.hibernate.validator.internal.engine.ValidatorFactoryConfigurationHelper.determineScriptEvaluatorFactory; @@ -44,8 +44,10 @@ import org.hibernate.validator.PredefinedScopeHibernateValidatorFactory; import org.hibernate.validator.internal.cfg.context.DefaultConstraintMapping; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager; +import org.hibernate.validator.internal.engine.constraintvalidation.HibernateConstraintValidatorInitializationContextImpl; import org.hibernate.validator.internal.engine.constraintvalidation.PredefinedScopeConstraintValidatorManagerImpl; import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator; +import org.hibernate.validator.internal.engine.tracking.PredefinedScopeProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager; import org.hibernate.validator.internal.metadata.PredefinedScopeBeanMetaDataManager; import org.hibernate.validator.internal.metadata.core.ConstraintHelper; @@ -112,23 +114,16 @@ public PredefinedScopeValidatorFactoryImpl(ConfigurationState configurationState determineAllowParallelMethodsDefineParameterConstraints( hibernateSpecificConfig, properties ) ).build(); - this.validatorFactoryScopedContext = new ValidatorFactoryScopedContext( - configurationState.getMessageInterpolator(), - configurationState.getTraversableResolver(), - new ExecutableParameterNameProvider( configurationState.getParameterNameProvider() ), - configurationState.getClockProvider(), - determineTemporalValidationTolerance( configurationState, properties ), - determineScriptEvaluatorFactory( configurationState, properties, externalClassLoader ), - determineFailFast( hibernateSpecificConfig, properties ), - determineTraversableResolverResultCacheEnabled( hibernateSpecificConfig, properties ), - determineConstraintValidatorPayload( hibernateSpecificConfig ), - determineConstraintExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ), - determineCustomViolationExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ) - ); + ExecutableParameterNameProvider parameterNameProvider = new ExecutableParameterNameProvider( configurationState.getParameterNameProvider() ); + ScriptEvaluatorFactory scriptEvaluatorFactory = determineScriptEvaluatorFactory( configurationState, properties, externalClassLoader ); + Duration temporalValidationTolerance = determineTemporalValidationTolerance( configurationState, properties ); + + HibernateConstraintValidatorInitializationContextImpl constraintValidatorInitializationContext = new HibernateConstraintValidatorInitializationContextImpl( + scriptEvaluatorFactory, configurationState.getClockProvider(), temporalValidationTolerance ); this.constraintValidatorManager = new PredefinedScopeConstraintValidatorManagerImpl( configurationState.getConstraintValidatorFactory(), - this.validatorFactoryScopedContext.getConstraintValidatorInitializationContext() + constraintValidatorInitializationContext ); this.validationOrderGenerator = new ValidationOrderGenerator(); @@ -171,7 +166,7 @@ public PredefinedScopeValidatorFactoryImpl(ConfigurationState configurationState this.beanMetaDataManager = new PredefinedScopeBeanMetaDataManager( constraintCreationContext, executableHelper, - validatorFactoryScopedContext.getParameterNameProvider(), + parameterNameProvider, javaBeanHelper, validationOrderGenerator, buildMetaDataProviders( constraintCreationContext, xmlMetaDataProvider, constraintMappings ), @@ -180,6 +175,23 @@ public PredefinedScopeValidatorFactoryImpl(ConfigurationState configurationState hibernateSpecificConfig.getBeanClassesToInitialize() ); + this.validatorFactoryScopedContext = new ValidatorFactoryScopedContext( + configurationState.getMessageInterpolator(), + configurationState.getTraversableResolver(), + parameterNameProvider, + configurationState.getClockProvider(), + temporalValidationTolerance, + scriptEvaluatorFactory, + determineFailFast( hibernateSpecificConfig, properties ), + determineTraversableResolverResultCacheEnabled( hibernateSpecificConfig, properties ), + determineConstraintValidatorPayload( hibernateSpecificConfig ), + determineConstraintExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ), + determineCustomViolationExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ), + ( hibernateSpecificConfig != null && hibernateSpecificConfig.getProcessedBeansTrackingStrategy() != null ) + ? hibernateSpecificConfig.getProcessedBeansTrackingStrategy() + : new PredefinedScopeProcessedBeansTrackingStrategy( beanMetaDataManager ), + constraintValidatorInitializationContext ); + if ( LOG.isDebugEnabled() ) { logValidatorFactoryScopedConfiguration( validatorFactoryScopedContext ); } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java index e7ae23617d..95a86a6d03 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java @@ -47,6 +47,7 @@ import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManagerImpl; import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator; +import org.hibernate.validator.internal.engine.tracking.HasCascadablesProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager; import org.hibernate.validator.internal.metadata.BeanMetaDataManager; import org.hibernate.validator.internal.metadata.BeanMetaDataManagerImpl; @@ -160,8 +161,10 @@ public ValidatorFactoryImpl(ConfigurationState configurationState) { determineTraversableResolverResultCacheEnabled( hibernateSpecificConfig, properties ), determineConstraintValidatorPayload( hibernateSpecificConfig ), determineConstraintExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ), - determineCustomViolationExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ) - ); + determineCustomViolationExpressionLanguageFeatureLevel( hibernateSpecificConfig, properties ), + ( hibernateSpecificConfig != null && hibernateSpecificConfig.getProcessedBeansTrackingStrategy() != null ) + ? hibernateSpecificConfig.getProcessedBeansTrackingStrategy() + : new HasCascadablesProcessedBeansTrackingStrategy() ); ConstraintValidatorManager constraintValidatorManager = new ConstraintValidatorManagerImpl( configurationState.getConstraintValidatorFactory(), diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryScopedContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryScopedContext.java index 85b9599471..dc53b4a35e 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryScopedContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryScopedContext.java @@ -15,6 +15,7 @@ import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext; import org.hibernate.validator.internal.engine.constraintvalidation.HibernateConstraintValidatorInitializationContextImpl; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.util.Contracts; import org.hibernate.validator.internal.util.ExecutableParameterNameProvider; import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel; @@ -78,6 +79,11 @@ public class ValidatorFactoryScopedContext { */ private final ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel; + /** + * Strategy used to enable or not processed beans tracking. + */ + private final ProcessedBeansTrackingStrategy processedBeansTrackingStrategy; + /** * The constraint validator initialization context. */ @@ -93,15 +99,16 @@ public class ValidatorFactoryScopedContext { boolean traversableResolverResultCacheEnabled, Object constraintValidatorPayload, ExpressionLanguageFeatureLevel constraintExpressionLanguageFeatureLevel, - ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel) { + ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel, + ProcessedBeansTrackingStrategy processedBeansTrackingStrategy) { this( messageInterpolator, traversableResolver, parameterNameProvider, clockProvider, temporalValidationTolerance, scriptEvaluatorFactory, failFast, traversableResolverResultCacheEnabled, constraintValidatorPayload, constraintExpressionLanguageFeatureLevel, - customViolationExpressionLanguageFeatureLevel, + customViolationExpressionLanguageFeatureLevel, processedBeansTrackingStrategy, new HibernateConstraintValidatorInitializationContextImpl( scriptEvaluatorFactory, clockProvider, temporalValidationTolerance ) ); } - private ValidatorFactoryScopedContext(MessageInterpolator messageInterpolator, + ValidatorFactoryScopedContext(MessageInterpolator messageInterpolator, TraversableResolver traversableResolver, ExecutableParameterNameProvider parameterNameProvider, ClockProvider clockProvider, @@ -112,6 +119,7 @@ private ValidatorFactoryScopedContext(MessageInterpolator messageInterpolator, Object constraintValidatorPayload, ExpressionLanguageFeatureLevel constraintExpressionLanguageFeatureLevel, ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel, + ProcessedBeansTrackingStrategy processedBeanTrackingStrategy, HibernateConstraintValidatorInitializationContextImpl constraintValidatorInitializationContext) { this.messageInterpolator = messageInterpolator; this.traversableResolver = traversableResolver; @@ -124,6 +132,7 @@ private ValidatorFactoryScopedContext(MessageInterpolator messageInterpolator, this.constraintValidatorPayload = constraintValidatorPayload; this.constraintExpressionLanguageFeatureLevel = constraintExpressionLanguageFeatureLevel; this.customViolationExpressionLanguageFeatureLevel = customViolationExpressionLanguageFeatureLevel; + this.processedBeansTrackingStrategy = processedBeanTrackingStrategy; this.constraintValidatorInitializationContext = constraintValidatorInitializationContext; } @@ -175,6 +184,10 @@ public ExpressionLanguageFeatureLevel getCustomViolationExpressionLanguageFeatur return this.customViolationExpressionLanguageFeatureLevel; } + public ProcessedBeansTrackingStrategy getProcessedBeansTrackingStrategy() { + return processedBeansTrackingStrategy; + } + static class Builder { private final ValidatorFactoryScopedContext defaultContext; @@ -189,6 +202,7 @@ static class Builder { private Object constraintValidatorPayload; private ExpressionLanguageFeatureLevel constraintExpressionLanguageFeatureLevel; private ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel; + private ProcessedBeansTrackingStrategy processedBeansTrackingStrategy; private HibernateConstraintValidatorInitializationContextImpl constraintValidatorInitializationContext; Builder(ValidatorFactoryScopedContext defaultContext) { @@ -206,6 +220,7 @@ static class Builder { this.constraintValidatorPayload = defaultContext.constraintValidatorPayload; this.constraintExpressionLanguageFeatureLevel = defaultContext.constraintExpressionLanguageFeatureLevel; this.customViolationExpressionLanguageFeatureLevel = defaultContext.customViolationExpressionLanguageFeatureLevel; + this.processedBeansTrackingStrategy = defaultContext.processedBeansTrackingStrategy; this.constraintValidatorInitializationContext = defaultContext.constraintValidatorInitializationContext; } @@ -292,6 +307,12 @@ public ValidatorFactoryScopedContext.Builder setCustomViolationExpressionLanguag return this; } + public ValidatorFactoryScopedContext.Builder setProcessedBeansTrackingStrategy( + ProcessedBeansTrackingStrategy processedBeansTrackingStrategy) { + this.processedBeansTrackingStrategy = processedBeansTrackingStrategy; + return this; + } + public ValidatorFactoryScopedContext build() { return new ValidatorFactoryScopedContext( messageInterpolator, @@ -305,6 +326,7 @@ public ValidatorFactoryScopedContext build() { constraintValidatorPayload, constraintExpressionLanguageFeatureLevel, customViolationExpressionLanguageFeatureLevel, + processedBeansTrackingStrategy, HibernateConstraintValidatorInitializationContextImpl.of( constraintValidatorInitializationContext, scriptEvaluatorFactory, diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/HasCascadablesProcessedBeansTrackingStrategy.java b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/HasCascadablesProcessedBeansTrackingStrategy.java new file mode 100644 index 0000000000..7a7a33083f --- /dev/null +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/HasCascadablesProcessedBeansTrackingStrategy.java @@ -0,0 +1,31 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.internal.engine.tracking; + +import java.lang.reflect.Executable; + +public class HasCascadablesProcessedBeansTrackingStrategy implements ProcessedBeansTrackingStrategy { + + @Override + public boolean isEnabledForBean(Class beanClass, boolean hasCascadables) { + return hasCascadables; + } + + @Override + public boolean isEnabledForReturnValue(Executable executable, boolean hasCascadables) { + return hasCascadables; + } + + @Override + public boolean isEnabledForParameters(Executable executable, boolean hasCascadables) { + return hasCascadables; + } + + @Override + public void clear() { + } +} diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java new file mode 100644 index 0000000000..9e3a155b25 --- /dev/null +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java @@ -0,0 +1,65 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.internal.engine.tracking; + +import java.lang.reflect.Executable; +import java.util.HashMap; +import java.util.Map; + +import org.hibernate.validator.internal.metadata.PredefinedScopeBeanMetaDataManager; +import org.hibernate.validator.internal.util.CollectionHelper; + +public class PredefinedScopeProcessedBeansTrackingStrategy implements ProcessedBeansTrackingStrategy { + + private final Map, Boolean> trackingEnabledForBeans; + + private final Map trackingEnabledForReturnValues; + + private final Map trackingEnabledForParameters; + + public PredefinedScopeProcessedBeansTrackingStrategy(PredefinedScopeBeanMetaDataManager beanMetaDataManager) { + // TODO: build the maps from the information inside the beanMetaDataManager + + this.trackingEnabledForBeans = CollectionHelper.toImmutableMap( new HashMap<>() ); + this.trackingEnabledForReturnValues = CollectionHelper.toImmutableMap( new HashMap<>() ); + this.trackingEnabledForParameters = CollectionHelper.toImmutableMap( new HashMap<>() ); + } + + @Override + public boolean isEnabledForBean(Class rootBeanClass, boolean hasCascadables) { + if ( !hasCascadables ) { + return false; + } + + return trackingEnabledForBeans.getOrDefault( rootBeanClass, true ); + } + + @Override + public boolean isEnabledForReturnValue(Executable executable, boolean hasCascadables) { + if ( !hasCascadables ) { + return false; + } + + return trackingEnabledForReturnValues.getOrDefault( executable, true ); + } + + @Override + public boolean isEnabledForParameters(Executable executable, boolean hasCascadables) { + if ( !hasCascadables ) { + return false; + } + + return trackingEnabledForParameters.getOrDefault( executable, true ); + } + + @Override + public void clear() { + trackingEnabledForBeans.clear(); + trackingEnabledForReturnValues.clear(); + trackingEnabledForParameters.clear(); + } +} diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/ProcessedBeansTrackingStrategy.java b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/ProcessedBeansTrackingStrategy.java new file mode 100644 index 0000000000..b5db188e3b --- /dev/null +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/ProcessedBeansTrackingStrategy.java @@ -0,0 +1,20 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.internal.engine.tracking; + +import java.lang.reflect.Executable; + +public interface ProcessedBeansTrackingStrategy { + + boolean isEnabledForBean(Class beanClass, boolean hasCascadables); + + boolean isEnabledForReturnValue(Executable executable, boolean hasCascadables); + + boolean isEnabledForParameters(Executable executable, boolean hasCascadables); + + void clear(); +} diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java index 361d2bf65f..d8d0b1e390 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java @@ -7,9 +7,10 @@ package org.hibernate.validator.internal.engine.validationcontext; import java.lang.invoke.MethodHandles; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.Iterator; import java.util.Map; import java.util.Set; @@ -96,25 +97,25 @@ abstract class AbstractValidationContext implements BaseBeanValidationContext /** * Indicates if the tracking of already validated bean should be disabled. */ - private final boolean disableAlreadyValidatedBeanTracking; + private final boolean processedBeanTrackingEnabled; /** * The set of already processed meta constraints per bean - path ({@link BeanPathMetaConstraintProcessedUnit}). */ @Lazy - private Set processedPathUnits; + private HashSet processedPathUnits; /** * The set of already processed groups per bean ({@link BeanGroupProcessedUnit}). */ @Lazy - private Set processedGroupUnits; + private HashSet processedGroupUnits; /** * Maps an object to a list of paths in which it has been validated. The objects are the bean instances. */ @Lazy - private Map> processedPathsPerBean; + private HashMap> processedPathsPerBean; /** * Contains all failing constraints so far. @@ -131,7 +132,7 @@ protected AbstractValidationContext( T rootBean, Class rootBeanClass, BeanMetaData rootBeanMetaData, - boolean disableAlreadyValidatedBeanTracking + boolean processedBeanTrackingEnabled ) { this.constraintValidatorManager = constraintValidatorManager; this.validatorScopedContext = validatorScopedContext; @@ -143,7 +144,7 @@ protected AbstractValidationContext( this.rootBeanClass = rootBeanClass; this.rootBeanMetaData = rootBeanMetaData; - this.disableAlreadyValidatedBeanTracking = disableAlreadyValidatedBeanTracking; + this.processedBeanTrackingEnabled = processedBeanTrackingEnabled; } @Override @@ -188,7 +189,7 @@ public ConstraintValidatorFactory getConstraintValidatorFactory() { @Override public boolean isBeanAlreadyValidated(Object value, Class group, PathImpl path) { - if ( disableAlreadyValidatedBeanTracking ) { + if ( !processedBeanTrackingEnabled ) { return false; } @@ -204,7 +205,7 @@ public boolean isBeanAlreadyValidated(Object value, Class group, PathImpl pat @Override public void markCurrentBeanAsProcessed(ValueContext valueContext) { - if ( disableAlreadyValidatedBeanTracking ) { + if ( !processedBeanTrackingEnabled ) { return; } @@ -333,7 +334,7 @@ private String interpolate( } private boolean isAlreadyValidatedForPath(Object value, PathImpl path) { - Set pathSet = getInitializedProcessedPathsPerBean().get( value ); + ArrayList pathSet = getInitializedProcessedPathsPerBean().get( new ProcessedBean( value ) ); if ( pathSet == null ) { return false; } @@ -369,12 +370,13 @@ private boolean isAlreadyValidatedForCurrentGroup(Object value, Class group) private void markCurrentBeanAsProcessedForCurrentPath(Object bean, PathImpl path) { // HV-1031 The path object is mutated as we traverse the object tree, hence copy it before saving it - Map> processedPathsPerBean = getInitializedProcessedPathsPerBean(); + HashMap> processedPathsPerBean = getInitializedProcessedPathsPerBean(); - Set processedPaths = processedPathsPerBean.get( bean ); + ProcessedBean processedBean = new ProcessedBean( bean ); + ArrayList processedPaths = processedPathsPerBean.get( processedBean ); if ( processedPaths == null ) { - processedPaths = new HashSet<>(); - processedPathsPerBean.put( bean, processedPaths ); + processedPaths = new ArrayList<>(); + processedPathsPerBean.put( processedBean, processedPaths ); } processedPaths.add( PathImpl.createCopy( path ) ); @@ -391,16 +393,16 @@ private Set getInitializedProcessedPathUnit return processedPathUnits; } - private Set getInitializedProcessedGroupUnits() { + private HashSet getInitializedProcessedGroupUnits() { if ( processedGroupUnits == null ) { processedGroupUnits = new HashSet<>(); } return processedGroupUnits; } - private Map> getInitializedProcessedPathsPerBean() { + private HashMap> getInitializedProcessedPathsPerBean() { if ( processedPathsPerBean == null ) { - processedPathsPerBean = new IdentityHashMap<>(); + processedPathsPerBean = new HashMap<>(); } return processedPathsPerBean; } @@ -505,4 +507,31 @@ private int createHashCode() { return result; } } + + private static final class ProcessedBean { + + private Object bean; + private int hashCode = -1; + + ProcessedBean(Object bean) { + this.bean = bean; + } + + @Override + public boolean equals(Object o) { + return this.bean == ((ProcessedBean) o).bean; + } + + @Override + public int hashCode() { + if ( hashCode == -1 ) { + hashCode = createHashCode(); + } + return hashCode; + } + + private int createHashCode() { + return System.identityHashCode( bean ); + } + } } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/BeanValidationContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/BeanValidationContext.java index ab7aa153d8..de2789ac4b 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/BeanValidationContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/BeanValidationContext.java @@ -16,6 +16,7 @@ import org.hibernate.validator.internal.engine.ConstraintViolationImpl; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintViolationCreationContext; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valuecontext.ValueContext; import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData; @@ -37,12 +38,14 @@ class BeanValidationContext extends AbstractValidationContext { BeanMetaData rootBeanMetaData ) { super( constraintValidatorManager, constraintValidatorFactory, validatorScopedContext, traversableResolver, constraintValidatorInitializationContext, - rootBean, rootBeanClass, rootBeanMetaData, buildDisableAlreadyValidatedBeanTracking( rootBeanMetaData ) + rootBean, rootBeanClass, rootBeanMetaData, buildProcessedBeansTrackingEnabled( validatorScopedContext.getProcessedBeansTrackingStrategy(), + rootBeanClass, rootBeanMetaData ) ); } - private static boolean buildDisableAlreadyValidatedBeanTracking(BeanMetaData rootBeanMetaData) { - return !rootBeanMetaData.hasCascadables(); + private static boolean buildProcessedBeansTrackingEnabled(ProcessedBeansTrackingStrategy processedBeansTrackingStrategy, Class rootBeanClass, + BeanMetaData rootBeanMetaData) { + return processedBeansTrackingStrategy.isEnabledForBean( rootBeanClass, rootBeanMetaData.hasCascadables() ); } @Override diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ParameterExecutableValidationContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ParameterExecutableValidationContext.java index fc08e920b4..a41641859f 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ParameterExecutableValidationContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ParameterExecutableValidationContext.java @@ -24,6 +24,7 @@ import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintViolationCreationContext; import org.hibernate.validator.internal.engine.constraintvalidation.CrossParameterConstraintValidatorContextImpl; import org.hibernate.validator.internal.engine.path.PathImpl; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valuecontext.ValueContext; import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData; import org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData; @@ -68,7 +69,8 @@ public class ParameterExecutableValidationContext extends AbstractValidationC ) { super( constraintValidatorManager, constraintValidatorFactory, validatorScopedContext, traversableResolver, constraintValidatorInitializationContext, rootBean, rootBeanClass, rootBeanMetaData, - buildDisableAlreadyValidatedBeanTracking( executableMetaData ) + buildProcessedBeansTrackingEnabled( validatorScopedContext.getProcessedBeansTrackingStrategy(), executable, + executableMetaData ) ); this.executable = executable; this.executableMetaData = executableMetaData; @@ -85,13 +87,16 @@ public Optional getExecutableMetaData() { return executableMetaData; } - private static boolean buildDisableAlreadyValidatedBeanTracking(Optional executableMetaData) { + private static boolean buildProcessedBeansTrackingEnabled(ProcessedBeansTrackingStrategy processedBeansTrackingStrategy, + Executable executable, + Optional executableMetaData) { if ( !executableMetaData.isPresent() ) { // the method is unconstrained so there's no need to worry about the tracking return false; } - return !executableMetaData.get().getValidatableParametersMetaData().hasCascadables(); + return processedBeansTrackingStrategy.isEnabledForReturnValue( executable, + executableMetaData.get().getValidatableParametersMetaData().hasCascadables() ); } @Override diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ReturnValueExecutableValidationContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ReturnValueExecutableValidationContext.java index 0c7715db79..af975c9679 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ReturnValueExecutableValidationContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ReturnValueExecutableValidationContext.java @@ -19,6 +19,7 @@ import org.hibernate.validator.internal.engine.ConstraintViolationImpl; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintViolationCreationContext; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.engine.valuecontext.ValueContext; import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData; import org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData; @@ -61,7 +62,8 @@ public class ReturnValueExecutableValidationContext extends AbstractValidatio ) { super( constraintValidatorManager, constraintValidatorFactory, validatorScopedContext, traversableResolver, constraintValidatorInitializationContext, rootBean, rootBeanClass, rootBeanMetaData, - buildDisableAlreadyValidatedBeanTracking( executableMetaData ) + buildProcessedBeansTrackingEnabled( validatorScopedContext.getProcessedBeansTrackingStrategy(), executable, + executableMetaData ) ); this.executable = executable; this.executableMetaData = executableMetaData; @@ -78,13 +80,15 @@ public Optional getExecutableMetaData() { return executableMetaData; } - private static boolean buildDisableAlreadyValidatedBeanTracking(Optional executableMetaData) { + private static boolean buildProcessedBeansTrackingEnabled(ProcessedBeansTrackingStrategy processedBeansTrackingStrategy, + Executable executable, + Optional executableMetaData) { if ( !executableMetaData.isPresent() ) { // the method is unconstrained so there's no need to worry about the tracking return false; } - return !executableMetaData.get().getReturnValueMetaData().hasCascadables(); + return processedBeansTrackingStrategy.isEnabledForReturnValue( executable, executableMetaData.get().getReturnValueMetaData().hasCascadables() ); } @Override diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ValidatorScopedContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ValidatorScopedContext.java index 0e78c05025..5b35742e85 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ValidatorScopedContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/ValidatorScopedContext.java @@ -13,6 +13,7 @@ import javax.validation.Validator; import org.hibernate.validator.internal.engine.ValidatorFactoryScopedContext; +import org.hibernate.validator.internal.engine.tracking.ProcessedBeansTrackingStrategy; import org.hibernate.validator.internal.util.ExecutableParameterNameProvider; import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel; import org.hibernate.validator.spi.scripting.ScriptEvaluatorFactory; @@ -76,6 +77,11 @@ public class ValidatorScopedContext { */ private final ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel; + /** + * Strategy used to enable or not processed beans tracking. + */ + private final ProcessedBeansTrackingStrategy processedBeansTrackingStrategy; + public ValidatorScopedContext(ValidatorFactoryScopedContext validatorFactoryScopedContext) { this.messageInterpolator = validatorFactoryScopedContext.getMessageInterpolator(); this.parameterNameProvider = validatorFactoryScopedContext.getParameterNameProvider(); @@ -87,6 +93,7 @@ public ValidatorScopedContext(ValidatorFactoryScopedContext validatorFactoryScop this.constraintValidatorPayload = validatorFactoryScopedContext.getConstraintValidatorPayload(); this.constraintExpressionLanguageFeatureLevel = validatorFactoryScopedContext.getConstraintExpressionLanguageFeatureLevel(); this.customViolationExpressionLanguageFeatureLevel = validatorFactoryScopedContext.getCustomViolationExpressionLanguageFeatureLevel(); + this.processedBeansTrackingStrategy = validatorFactoryScopedContext.getProcessedBeansTrackingStrategy(); } public MessageInterpolator getMessageInterpolator() { @@ -128,4 +135,8 @@ public ExpressionLanguageFeatureLevel getConstraintExpressionLanguageFeatureLeve public ExpressionLanguageFeatureLevel getCustomViolationExpressionLanguageFeatureLevel() { return customViolationExpressionLanguageFeatureLevel; } + + public ProcessedBeansTrackingStrategy getProcessedBeansTrackingStrategy() { + return processedBeansTrackingStrategy; + } } diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/PredefinedScopeBeanMetaDataManager.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/PredefinedScopeBeanMetaDataManager.java index b7215b0637..0da27bf01c 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/PredefinedScopeBeanMetaDataManager.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/PredefinedScopeBeanMetaDataManager.java @@ -11,6 +11,7 @@ import java.lang.annotation.ElementType; import java.lang.reflect.Executable; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -119,6 +120,10 @@ public BeanMetaData getBeanMetaData(Class beanClass) { return beanMetaData; } + public Collection> getBeanMetaData() { + return beanMetaDataMap.values(); + } + @Override public void clear() { beanMetaDataMap.clear(); diff --git a/performance/pom.xml b/performance/pom.xml index 393d7c8068..e54f562629 100644 --- a/performance/pom.xml +++ b/performance/pom.xml @@ -182,8 +182,52 @@ jakarta.el - log4j - log4j + org.apache.logging.log4j + log4j-core + + + + + + + org.codehaus.mojo + build-helper-maven-plugin + + + + + + hv-6.2 + + + validator + hv-6.2 + + + + 2.0.1.Final + Hibernate Validator + 6.2.0.Final + + + + javax.validation + validation-api + ${validation-api.version} + + + ${project.groupId} + hibernate-validator + ${beanvalidation-impl.version} + + + org.glassfish + javax.el + 3.0.1-b11 + + + org.apache.logging.log4j + log4j-core @@ -223,8 +267,8 @@ jakarta.el - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -267,8 +311,8 @@ 3.0.1-b11 - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -311,8 +355,8 @@ 3.0.1-b11 - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -352,8 +396,8 @@ ${javax-el.version} - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -393,8 +437,8 @@ ${javax-el.version} - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -434,8 +478,8 @@ ${javax-el.version} - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -475,8 +519,8 @@ ${javax-el.version} - log4j - log4j + org.apache.logging.log4j + log4j-core @@ -505,8 +549,8 @@ ${beanvalidation-impl.version} - log4j - log4j + org.apache.logging.log4j + log4j-core From d4acd34c2b8d0c1440d2c776fe316cd639f6a0e3 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 25 Feb 2021 11:25:48 +0100 Subject: [PATCH 2/5] HV-1831 Add a couple of examples illustrating various cases --- ...edScopeProcessedBeansTrackingStrategy.java | 5 ++ .../ProcessedBeansTrackingCycles1Test.java | 49 ++++++++++++++++ .../ProcessedBeansTrackingCycles2Test.java | 50 ++++++++++++++++ .../ProcessedBeansTrackingCycles3Test.java | 51 ++++++++++++++++ .../ProcessedBeansTrackingCycles4Test.java | 51 ++++++++++++++++ .../ProcessedBeansTrackingCycles5Test.java | 58 +++++++++++++++++++ .../ProcessedBeansTrackingNoCycles1Test.java | 44 ++++++++++++++ .../ProcessedBeansTrackingNoCycles2Test.java | 45 ++++++++++++++ .../ProcessedBeansTrackingNoCycles3Test.java | 48 +++++++++++++++ 9 files changed, 401 insertions(+) create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles1Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles2Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles3Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles4Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles5Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles1Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java create mode 100644 engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles3Test.java diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java index 9e3a155b25..a78c7ee43c 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java @@ -23,6 +23,11 @@ public class PredefinedScopeProcessedBeansTrackingStrategy implements ProcessedB public PredefinedScopeProcessedBeansTrackingStrategy(PredefinedScopeBeanMetaDataManager beanMetaDataManager) { // TODO: build the maps from the information inside the beanMetaDataManager + // There is a good chance we will need a structure with the whole hierarchy of constraint classes. + // That's something we could add to PredefinedScopeBeanMetaDataManager, as we are already doing similar things + // there (see the ClassHierarchyHelper.getHierarchy call). + // In the predefined scope case, we will have the whole hierarchy of constrained classes passed to + // PredefinedScopeBeanMetaDataManager. this.trackingEnabledForBeans = CollectionHelper.toImmutableMap( new HashMap<>() ); this.trackingEnabledForReturnValues = CollectionHelper.toImmutableMap( new HashMap<>() ); diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles1Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles1Test.java new file mode 100644 index 0000000000..9da4aba204 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles1Test.java @@ -0,0 +1,49 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * This is the most simple example. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingCycles1Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + @Valid + private Child child; + } + + private static class Child { + + @NotNull + private String property; + + @Valid + private Parent parent; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles2Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles2Test.java new file mode 100644 index 0000000000..5fa8226919 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles2Test.java @@ -0,0 +1,50 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * Simple enough but this time the cascading annotation is in the container element. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingCycles2Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private List<@Valid Child> children; + } + + private static class Child { + + @NotNull + private String property; + + @Valid + private Parent parent; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles3Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles3Test.java new file mode 100644 index 0000000000..2d136405e5 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles3Test.java @@ -0,0 +1,51 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; +import java.util.Map; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * Simple enough but this time the cascading annotation is deep in a container element. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingCycles3Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private Map> children; + } + + private static class Child { + + @NotNull + private String property; + + @Valid + private Parent parent; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles4Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles4Test.java new file mode 100644 index 0000000000..75cfbf50b6 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles4Test.java @@ -0,0 +1,51 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; +import java.util.Map; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * Simple enough but this time the cascading annotation is deep in a container element with a bound. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingCycles4Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private Map> children; + } + + private static class Child { + + @NotNull + private String property; + + @Valid + private Parent parent; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles5Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles5Test.java new file mode 100644 index 0000000000..9b5457638a --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles5Test.java @@ -0,0 +1,58 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * This one is a bit more tricky: during the validation, when cascading, we take into account the runtime type to get + * the metadata, not the declared type. + *

+ * So even if you couldn't have a cycle with the declared type, when trying to find the cycles, we need to take into + * consideration all the subclasses too. The good news is that we are in a closed world so we have them all passed + * to our PredefinedScopedValidatorFactoryImpl! + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingCycles5Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private List<@Valid ChildWithNoCycles> children; + } + + private static class ChildWithNoCycles { + + @NotNull + private String property; + } + + private static class Child extends ChildWithNoCycles { + + @Valid + private Parent parent; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles1Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles1Test.java new file mode 100644 index 0000000000..8bb1d30b74 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles1Test.java @@ -0,0 +1,44 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingNoCycles1Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + @Valid + private Child child; + } + + private static class Child { + + @NotNull + private String property; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java new file mode 100644 index 0000000000..b80a032b35 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java @@ -0,0 +1,45 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingNoCycles2Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private List<@Valid Child> children; + } + + private static class Child { + + @NotNull + private String property; + } +} diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles3Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles3Test.java new file mode 100644 index 0000000000..10d4c6b239 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles3Test.java @@ -0,0 +1,48 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.test.internal.engine.tracking; + +import java.util.List; + +import javax.validation.Valid; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.testutils.ValidatorUtil; +import org.testng.annotations.Test; + +/** + * This is not a real test, just an illustration. + *

+ * In this case, given we will have all the subclasses of Child in the metadata, we should be able to know there are no + * cycles. + * + * @author Guillaume Smet + */ +public class ProcessedBeansTrackingNoCycles3Test { + + @Test + public void testSerializeHibernateEmail() throws Exception { + Validator validator = ValidatorUtil.getValidator(); + + validator.validate( new Parent() ); + } + + private static class Parent { + + @NotNull + private String property; + + private List<@Valid ? extends Child> children; + } + + private static class Child { + + @NotNull + private String property; + } +} From aea064236988170511c109fd5464021385550bf8 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Mon, 8 Mar 2021 19:41:22 +0100 Subject: [PATCH 3/5] HV-1831 Clean up another experiment that shouldn't have been committed --- .../AbstractValidationContext.java | 53 +++++-------------- 1 file changed, 12 insertions(+), 41 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java index d8d0b1e390..235cac7af4 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java @@ -7,10 +7,9 @@ package org.hibernate.validator.internal.engine.validationcontext; import java.lang.invoke.MethodHandles; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.Iterator; import java.util.Map; import java.util.Set; @@ -103,19 +102,19 @@ abstract class AbstractValidationContext implements BaseBeanValidationContext * The set of already processed meta constraints per bean - path ({@link BeanPathMetaConstraintProcessedUnit}). */ @Lazy - private HashSet processedPathUnits; + private Set processedPathUnits; /** * The set of already processed groups per bean ({@link BeanGroupProcessedUnit}). */ @Lazy - private HashSet processedGroupUnits; + private Set processedGroupUnits; /** * Maps an object to a list of paths in which it has been validated. The objects are the bean instances. */ @Lazy - private HashMap> processedPathsPerBean; + private Map> processedPathsPerBean; /** * Contains all failing constraints so far. @@ -334,7 +333,7 @@ private String interpolate( } private boolean isAlreadyValidatedForPath(Object value, PathImpl path) { - ArrayList pathSet = getInitializedProcessedPathsPerBean().get( new ProcessedBean( value ) ); + Set pathSet = getInitializedProcessedPathsPerBean().get( value ); if ( pathSet == null ) { return false; } @@ -370,13 +369,12 @@ private boolean isAlreadyValidatedForCurrentGroup(Object value, Class group) private void markCurrentBeanAsProcessedForCurrentPath(Object bean, PathImpl path) { // HV-1031 The path object is mutated as we traverse the object tree, hence copy it before saving it - HashMap> processedPathsPerBean = getInitializedProcessedPathsPerBean(); + Map> processedPathsPerBean = getInitializedProcessedPathsPerBean(); - ProcessedBean processedBean = new ProcessedBean( bean ); - ArrayList processedPaths = processedPathsPerBean.get( processedBean ); + Set processedPaths = processedPathsPerBean.get( bean ); if ( processedPaths == null ) { - processedPaths = new ArrayList<>(); - processedPathsPerBean.put( processedBean, processedPaths ); + processedPaths = new HashSet<>(); + processedPathsPerBean.put( bean, processedPaths ); } processedPaths.add( PathImpl.createCopy( path ) ); @@ -393,16 +391,16 @@ private Set getInitializedProcessedPathUnit return processedPathUnits; } - private HashSet getInitializedProcessedGroupUnits() { + private Set getInitializedProcessedGroupUnits() { if ( processedGroupUnits == null ) { processedGroupUnits = new HashSet<>(); } return processedGroupUnits; } - private HashMap> getInitializedProcessedPathsPerBean() { + private Map> getInitializedProcessedPathsPerBean() { if ( processedPathsPerBean == null ) { - processedPathsPerBean = new HashMap<>(); + processedPathsPerBean = new IdentityHashMap<>(); } return processedPathsPerBean; } @@ -507,31 +505,4 @@ private int createHashCode() { return result; } } - - private static final class ProcessedBean { - - private Object bean; - private int hashCode = -1; - - ProcessedBean(Object bean) { - this.bean = bean; - } - - @Override - public boolean equals(Object o) { - return this.bean == ((ProcessedBean) o).bean; - } - - @Override - public int hashCode() { - if ( hashCode == -1 ) { - hashCode = createHashCode(); - } - return hashCode; - } - - private int createHashCode() { - return System.identityHashCode( bean ); - } - } } From cea9b8cb40609394cba9731809ab72021e31b6fd Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 10 Mar 2021 10:02:52 -0800 Subject: [PATCH 4/5] Add the same bean to List twice --- .../tracking/ProcessedBeansTrackingNoCycles2Test.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java index b80a032b35..90a3e0e7f6 100644 --- a/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingNoCycles2Test.java @@ -6,6 +6,7 @@ */ package org.hibernate.validator.test.internal.engine.tracking; +import java.util.ArrayList; import java.util.List; import javax.validation.Valid; @@ -26,7 +27,14 @@ public class ProcessedBeansTrackingNoCycles2Test { public void testSerializeHibernateEmail() throws Exception { Validator validator = ValidatorUtil.getValidator(); - validator.validate( new Parent() ); + final Parent parent = new Parent(); + parent.property = "parent property"; + final Child child = new Child(); + child.property = "child property"; + parent.children = new ArrayList<>(); + parent.children.add( child ); + parent.children.add( child ); + validator.validate( parent ); } private static class Parent { From f4d765617643dc751e1b9a75700b1457d9d41c21 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 11 Mar 2021 14:00:12 +0100 Subject: [PATCH 5/5] Copy nodes when changing the nature of the leaf --- .../validator/internal/engine/path/PathImpl.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java index d83a9e8f7e..017615db26 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java @@ -195,7 +195,7 @@ private NodeImpl addMethodNode(String name, Class[] parameterTypes) { } public NodeImpl makeLeafNodeIterable() { - requiresWriteableNodeList(); + copyNodeList(); currentLeafNode = NodeImpl.makeIterable( currentLeafNode ); @@ -205,7 +205,7 @@ public NodeImpl makeLeafNodeIterable() { } public NodeImpl makeLeafNodeIterableAndSetIndex(Integer index) { - requiresWriteableNodeList(); + copyNodeList(); currentLeafNode = NodeImpl.makeIterableAndSetIndex( currentLeafNode, index ); @@ -215,7 +215,7 @@ public NodeImpl makeLeafNodeIterableAndSetIndex(Integer index) { } public NodeImpl makeLeafNodeIterableAndSetMapKey(Object key) { - requiresWriteableNodeList(); + copyNodeList(); currentLeafNode = NodeImpl.makeIterableAndSetMapKey( currentLeafNode, key ); @@ -300,6 +300,10 @@ private void requiresWriteableNodeList() { return; } + copyNodeList(); + } + + private void copyNodeList() { // Usually, the write operation is about adding one more node, so let's make the list one element larger. List newNodeList = new ArrayList<>( nodeList.size() + 1 ); newNodeList.addAll( nodeList );