From e0d4b9d76d3a001708667a1682d5a469e44d686b Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Wed, 21 Aug 2024 18:37:44 +0200 Subject: [PATCH 1/2] HV-2031 Make PredefinedScopeHibernateValidatorFactory aware of constraint mappings defined purely in XML --- .../PredefinedScopeValidatorFactoryImpl.java | 6 +- .../metadata/core/ConstraintHelper.java | 138 ++++++++++++------ .../provider/XmlMetaDataProvider.java | 6 + .../PredefinedScopeValidatorFactoryTest.java | 24 +++ .../test/predefinedscope/SimpleXmlBean.java | 12 ++ .../constraints-simplexmlbean.xml | 21 +++ 6 files changed, 163 insertions(+), 44 deletions(-) create mode 100644 engine/src/test/java/org/hibernate/validator/test/predefinedscope/SimpleXmlBean.java create mode 100644 engine/src/test/resources/org/hibernate/validator/test/predefinedscope/constraints-simplexmlbean.xml 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 8e98fd7c85..83976e0a2b 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 @@ -29,6 +29,7 @@ import java.lang.invoke.MethodHandles; import java.time.Duration; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -188,9 +189,12 @@ public PredefinedScopeValidatorFactoryImpl(ConfigurationState configurationState // or from programmatic mappings registerCustomConstraintValidators( constraintMappings, constraintHelper ); + Set> beanClassesToInitialize = new HashSet<>( hibernateSpecificConfig.getBeanClassesToInitialize() ); + XmlMetaDataProvider xmlMetaDataProvider; if ( mappingParser != null && mappingParser.createConstrainedElements() ) { xmlMetaDataProvider = new XmlMetaDataProvider( mappingParser ); + beanClassesToInitialize.addAll( xmlMetaDataProvider.configuredBeanClasses() ); } else { xmlMetaDataProvider = null; @@ -205,7 +209,7 @@ public PredefinedScopeValidatorFactoryImpl(ConfigurationState configurationState buildMetaDataProviders( constraintCreationContext, xmlMetaDataProvider, constraintMappings ), methodValidationConfiguration, determineBeanMetaDataClassNormalizer( hibernateSpecificConfig ), - hibernateSpecificConfig.getBeanClassesToInitialize() + beanClassesToInitialize ); if ( LOG.isDebugEnabled() ) { diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/ConstraintHelper.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/ConstraintHelper.java index a88fba4895..f685c26bc2 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/ConstraintHelper.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/ConstraintHelper.java @@ -105,6 +105,7 @@ import jakarta.validation.constraints.Size; import jakarta.validation.constraintvalidation.ValidationTarget; +import org.hibernate.validator.cfg.ConstraintMapping; import org.hibernate.validator.constraints.BitcoinAddress; import org.hibernate.validator.constraints.CodePointLength; import org.hibernate.validator.constraints.ConstraintComposition; @@ -372,7 +373,7 @@ * @author Gunnar Morling * @author Guillaume Smet */ -public class ConstraintHelper { +public abstract class ConstraintHelper { public static final String GROUPS = "groups"; public static final String PAYLOAD = "payload"; @@ -385,9 +386,6 @@ public class ConstraintHelper { private static final String JODA_TIME_CLASS_NAME = "org.joda.time.ReadableInstant"; private static final String JAVA_MONEY_CLASS_NAME = "javax.money.MonetaryAmount"; - @Immutable - private final Map, List>> enabledBuiltinConstraints; - private final ConcurrentMap, Boolean> externalConstraints = new ConcurrentHashMap<>(); private final ConcurrentMap, Boolean> multiValueConstraints = new ConcurrentHashMap<>(); @@ -399,21 +397,20 @@ public class ConstraintHelper { private Boolean jodaTimeInClassPath; public static ConstraintHelper forAllBuiltinConstraints() { - return new ConstraintHelper( new HashSet<>( Arrays.asList( BuiltinConstraint.values() ) ) ); + return new StaticConstraintHelper(); } public static ConstraintHelper forBuiltinConstraints(Set enabledConstraints) { - return new ConstraintHelper( BuiltinConstraint.resolve( enabledConstraints ) ); + return new DynamicConstraintHelper( BuiltinConstraint.resolve( enabledConstraints ) ); } @SuppressWarnings("deprecation") - private ConstraintHelper(Set enabledBuiltinConstraints) { + protected Map, List>> resolve(Set enabledBuiltinConstraints) { if ( enabledBuiltinConstraints.isEmpty() ) { - this.enabledBuiltinConstraints = Collections.emptyMap(); - return; + return Collections.emptyMap(); } - Map, List>> tmpConstraints = new HashMap<>(); + Map, List>> tmpConstraints = new HashMap<>(); // Bean Validation constraints @@ -832,21 +829,27 @@ private ConstraintHelper(Set enabledBuiltinConstraints) { putBuiltinConstraint( tmpConstraints, BitcoinAddress.class, BitcoinAddressValidator.class ); } - this.enabledBuiltinConstraints = Collections.unmodifiableMap( tmpConstraints ); + return tmpConstraints; } - private static void putBuiltinConstraint(Map, List>> validators, - Class constraintType) { + private static void putBuiltinConstraint( + Map, List>> validators, + Class constraintType + ) { validators.put( constraintType, Collections.emptyList() ); } - private static void putBuiltinConstraint(Map, List>> validators, - Class constraintType, Class> validatorType) { + private static void putBuiltinConstraint( + Map, List>> validators, + Class constraintType, Class> validatorType + ) { validators.put( constraintType, Collections.singletonList( ConstraintValidatorDescriptor.forBuiltinClass( validatorType, constraintType ) ) ); } - private static void putBuiltinConstraints(Map, List>> validators, - Class constraintType, List>> validatorTypes) { + private static void putBuiltinConstraints( + Map, List>> validators, + Class constraintType, List>> validatorTypes + ) { List> descriptors = new ArrayList<>( validatorTypes.size() ); for ( Class> validatorType : validatorTypes ) { @@ -873,14 +876,13 @@ public static Set getBuiltinConstraints() { *
  • internally registered validators for built-in constraints
    • *
  • XML configuration and
    • *
  • programmatically registered validators (see - * {@link org.hibernate.validator.cfg.ConstraintMapping#constraintDefinition(Class)}).
    • + * {@link ConstraintMapping#constraintDefinition(Class)}). * * * The result is cached internally. * * @param annotationType The constraint annotation type. * @param     the type of the annotation - * * @return The validator classes for the given type. */ public List> getAllValidatorDescriptors(Class annotationType) { @@ -895,7 +897,6 @@ public List> getAllValid * @param annotationType The annotation of interest. * @param validationTarget The target, either annotated element or parameters. * @param the type of the annotation - * * @return A list with matching validator descriptors. */ public List> findValidatorDescriptors(Class annotationType, ValidationTarget validationTarget) { @@ -937,9 +938,8 @@ public void putValidatorDescriptors(Class annotationTy * Checks whether a given annotation is a multi value constraint or not. * * @param annotationType the annotation type to check. - * * @return {@code true} if the specified annotation is a multi value constraints, {@code false} - * otherwise. + * otherwise. */ public boolean isMultiValueConstraint(Class annotationType) { if ( isJdkAnnotation( annotationType ) ) { @@ -974,7 +974,6 @@ public boolean isMultiValueConstraint(Class annotationType * * @param multiValueConstraint the multi-value constraint annotation from which to retrieve the contained constraints * @param the type of the annotation - * * @return A list of constraint annotations, may be empty but never {@code null}. */ public List getConstraintsFromMultiValueConstraint(A multiValueConstraint) { @@ -997,7 +996,6 @@ public List getConstraintsFromMultiValueConst * * * @param annotationType The annotation type to test. - * * @return {@code true} if the annotation fulfills the above conditions, {@code false} otherwise. */ public boolean isConstraintAnnotation(Class annotationType) { @@ -1139,32 +1137,84 @@ private boolean isJavaMoneyInClasspath() { * Returns the default validators for the given constraint type. * * @param annotationType The constraint annotation type. - * * @return A list with the default validators as retrieved from - * {@link Constraint#validatedBy()} or the list of validators for - * built-in constraints. + * {@link Constraint#validatedBy()} or the list of validators for + * built-in constraints. */ @SuppressWarnings("unchecked") - private List> getDefaultValidatorDescriptors(Class annotationType) { - //safe cause all CV for a given annotation A are CV - final List> builtInValidators = (List>) enabledBuiltinConstraints - .get( annotationType ); + protected abstract List> getDefaultValidatorDescriptors(Class annotationType); - if ( builtInValidators != null ) { - return builtInValidators; + private static boolean isClassPresent(String className) { + return IsClassPresent.action( className, ConstraintHelper.class.getClassLoader() ); + } + + private static class StaticConstraintHelper extends ConstraintHelper { + + @Immutable + private final Map, List>> enabledBuiltinConstraints; + + private StaticConstraintHelper() { + this.enabledBuiltinConstraints = Collections.unmodifiableMap( resolve( new HashSet<>( Arrays.asList( BuiltinConstraint.values() ) ) ) ); } - Class>[] validatedBy = (Class>[]) annotationType - .getAnnotation( Constraint.class ) - .validatedBy(); + @SuppressWarnings("unchecked") + @Override + protected List> getDefaultValidatorDescriptors(Class annotationType) { + //safe cause all CV for a given annotation A are CV + final List> builtInValidators = (List>) enabledBuiltinConstraints + .get( annotationType ); + + if ( builtInValidators != null ) { + return builtInValidators; + } + + Class>[] validatedBy = (Class>[]) annotationType + .getAnnotation( Constraint.class ) + .validatedBy(); - return Stream.of( validatedBy ) - .map( c -> ConstraintValidatorDescriptor.forClass( c, annotationType ) ) - .collect( Collectors.collectingAndThen( Collectors.toList(), CollectionHelper::toImmutableList ) ); + return Stream.of( validatedBy ) + .map( c -> ConstraintValidatorDescriptor.forClass( c, annotationType ) ) + .collect( Collectors.collectingAndThen( Collectors.toList(), CollectionHelper::toImmutableList ) ); + } } - private static boolean isClassPresent(String className) { - return IsClassPresent.action( className, ConstraintHelper.class.getClassLoader() ); + private static class DynamicConstraintHelper extends ConstraintHelper { + + @Immutable + private final Map, List>> enabledBuiltinConstraints; + + private DynamicConstraintHelper(Set initialConstraints) { + this.enabledBuiltinConstraints = new HashMap<>( resolve( initialConstraints ) ); + } + + @SuppressWarnings("unchecked") + @Override + protected List> getDefaultValidatorDescriptors(Class annotationType) { + //safe cause all CV for a given annotation A are CV + final List> builtInValidators = (List>) enabledBuiltinConstraints + .get( annotationType ); + + if ( builtInValidators != null ) { + return builtInValidators; + } + else { + // let's give it a try and see if we can find this constraint among the built-in ones: + Set builtinConstraints = BuiltinConstraint.resolve( Collections.singleton( annotationType.getName() ) ); + if ( !builtinConstraints.isEmpty() ) { + Map, List>> additionalDescriptors = resolve( builtinConstraints ); + enabledBuiltinConstraints.putAll( additionalDescriptors ); + return (List>) additionalDescriptors.get( annotationType ); + } + } + + Class>[] validatedBy = (Class>[]) annotationType + .getAnnotation( Constraint.class ) + .validatedBy(); + + return Stream.of( validatedBy ) + .map( c -> ConstraintValidatorDescriptor.forClass( c, annotationType ) ) + .collect( Collectors.collectingAndThen( Collectors.toList(), CollectionHelper::toImmutableList ) ); + } } /** @@ -1183,8 +1233,10 @@ private void put(Class annotationType, List List> computeIfAbsent(Class annotationType, - Function, List>> mappingFunction) { + private List> computeIfAbsent( + Class annotationType, + Function, List>> mappingFunction + ) { return (List>) constraintValidatorDescriptors.computeIfAbsent( annotationType, (Function, ? extends List>>) mappingFunction diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/XmlMetaDataProvider.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/XmlMetaDataProvider.java index 7c2e57f94e..be0f9060c9 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/XmlMetaDataProvider.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/XmlMetaDataProvider.java @@ -9,6 +9,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import org.hibernate.validator.internal.metadata.core.AnnotationProcessingOptions; import org.hibernate.validator.internal.metadata.raw.BeanConfiguration; @@ -66,4 +67,9 @@ public BeanConfiguration getBeanConfiguration(Class beanClass) { public AnnotationProcessingOptions getAnnotationProcessingOptions() { return annotationProcessingOptions; } + + public Set> configuredBeanClasses() { + return configuredBeans.values().stream().map( BeanConfiguration::getBeanClass ) + .collect( Collectors.toSet() ); + } } diff --git a/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java index be2aaffbd8..7de772698e 100644 --- a/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java +++ b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java @@ -34,6 +34,7 @@ import jakarta.validation.ValidatorFactory; import jakarta.validation.constraints.Email; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; import org.hibernate.validator.PredefinedScopeHibernateValidator; import org.hibernate.validator.metadata.BeanMetaDataClassNormalizer; @@ -328,6 +329,29 @@ public String interpolate(String messageTemplate, Context context) { assertThat( violations ).containsOnlyViolations( violationOf( NotNull.class ).withMessage( "another string" ) ); } + @Test + public void testXmlDefinedConstraints() { + // we assume that all the metadata is defined in the xml, + // hence there is no built-in constraints nor beans to init: + try ( + ValidatorFactory factory = Validation.byProvider( PredefinedScopeHibernateValidator.class ) + .configure() + .builtinConstraints( Collections.emptySet() ) + .initializeBeanMetaData( Collections.emptySet() ) + .addMapping( PredefinedScopeValidatorFactoryTest.class.getResourceAsStream( "constraints-simplexmlbean.xml" ) ) + .buildValidatorFactory() + ) { + Validator validator = factory.getValidator(); + + assertThat( validator.validate( new SimpleXmlBean() ) ) + .containsOnlyViolations( + violationOf( Positive.class ).withMessage( "must be greater than 0" ), + violationOf( NotNull.class ).withMessage( "must not be null" ) + ); + } + + } + private static ValidatorFactory getValidatorFactory() { Set> beanMetaDataToInitialize = new HashSet<>(); beanMetaDataToInitialize.add( Bean.class ); diff --git a/engine/src/test/java/org/hibernate/validator/test/predefinedscope/SimpleXmlBean.java b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/SimpleXmlBean.java new file mode 100644 index 0000000000..67413babe0 --- /dev/null +++ b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/SimpleXmlBean.java @@ -0,0 +1,12 @@ +/* + * 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.predefinedscope; + +public class SimpleXmlBean { + public int id; + public String name; +} diff --git a/engine/src/test/resources/org/hibernate/validator/test/predefinedscope/constraints-simplexmlbean.xml b/engine/src/test/resources/org/hibernate/validator/test/predefinedscope/constraints-simplexmlbean.xml new file mode 100644 index 0000000000..7ea5c179e3 --- /dev/null +++ b/engine/src/test/resources/org/hibernate/validator/test/predefinedscope/constraints-simplexmlbean.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + From 95adbe651c5adc9e8cef5fecc7ecbf014c0445ae Mon Sep 17 00:00:00 2001 From: marko-bekhta Date: Fri, 30 Aug 2024 11:25:07 +0200 Subject: [PATCH 2/2] HV-2031 Add a config knob to predefined configuration so that we can get the "old-default" behavior by-default and enable the additional collection of information from the XML metadata --- ...dScopeHibernateValidatorConfiguration.java | 13 +++++++++++ .../PredefinedScopeConfigurationImpl.java | 12 ++++++++++ .../PredefinedScopeValidatorFactoryImpl.java | 8 +++++-- .../metadata/core/ConstraintHelper.java | 16 ++++++++++--- .../PredefinedScopeValidatorFactoryTest.java | 23 +++++++++++++++++-- 5 files changed, 65 insertions(+), 7 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/PredefinedScopeHibernateValidatorConfiguration.java b/engine/src/main/java/org/hibernate/validator/PredefinedScopeHibernateValidatorConfiguration.java index a295e88430..5426f6c2e0 100644 --- a/engine/src/main/java/org/hibernate/validator/PredefinedScopeHibernateValidatorConfiguration.java +++ b/engine/src/main/java/org/hibernate/validator/PredefinedScopeHibernateValidatorConfiguration.java @@ -32,4 +32,17 @@ public interface PredefinedScopeHibernateValidatorConfiguration extends BaseHibe @Incubating @Deprecated PredefinedScopeHibernateValidatorConfiguration initializeLocales(Set locales); + + /** + * Specify whether to append the {@link #builtinConstraints(Set) built-in constraints} and {@link #initializeBeanMetaData(Set) beans to initialize} + * with constraints and beans provided only through XML mapping. + *

    + * This option is enabled by default. + * + * @param include Whether to include the beans defined only in xml as part of the {@link #initializeBeanMetaData(Set) set of beans to initialize} + * and also add built-in constraints used only in xml definitions as part of the {@link #builtinConstraints(Set) set of built-in constraints}. + * @return {@code this} for chaining configuration method calls. + */ + @Incubating + PredefinedScopeHibernateValidatorConfiguration includeBeansAndConstraintsDefinedOnlyInXml(boolean include); } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeConfigurationImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeConfigurationImpl.java index 5a0d92383d..bfeae2c8a2 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeConfigurationImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/PredefinedScopeConfigurationImpl.java @@ -30,6 +30,8 @@ public class PredefinedScopeConfigurationImpl extends AbstractConfigurationImpl< private Set> beanClassesToInitialize; + private boolean includeBeansAndConstraintsDefinedOnlyInXml = true; + public PredefinedScopeConfigurationImpl(BootstrapState state) { super( state ); } @@ -58,6 +60,10 @@ public Set> getBeanClassesToInitialize() { return beanClassesToInitialize; } + public boolean isIncludeBeansAndConstraintsDefinedOnlyInXml() { + return includeBeansAndConstraintsDefinedOnlyInXml; + } + @Override @Deprecated public PredefinedScopeHibernateValidatorConfiguration initializeLocales(Set localesToInitialize) { @@ -66,6 +72,12 @@ public PredefinedScopeHibernateValidatorConfiguration initializeLocales(Set enabledConstraints) { - return new DynamicConstraintHelper( BuiltinConstraint.resolve( enabledConstraints ) ); + public static ConstraintHelper forBuiltinConstraints(Set enabledConstraints, boolean attemptToResolveMissingBuiltInConstraintsOnTheFly) { + Set builtinConstraints = BuiltinConstraint.resolve( enabledConstraints ); + if ( attemptToResolveMissingBuiltInConstraintsOnTheFly ) { + return new DynamicConstraintHelper( builtinConstraints ); + } + else { + return new StaticConstraintHelper( builtinConstraints ); + } } @SuppressWarnings("deprecation") @@ -1154,7 +1160,11 @@ private static class StaticConstraintHelper extends ConstraintHelper { private final Map, List>> enabledBuiltinConstraints; private StaticConstraintHelper() { - this.enabledBuiltinConstraints = Collections.unmodifiableMap( resolve( new HashSet<>( Arrays.asList( BuiltinConstraint.values() ) ) ) ); + this( new HashSet<>( Arrays.asList( BuiltinConstraint.values() ) ) ); + } + + private StaticConstraintHelper(Set builtinConstraints) { + this.enabledBuiltinConstraints = Collections.unmodifiableMap( resolve( builtinConstraints ) ); } @SuppressWarnings("unchecked") diff --git a/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java index 7de772698e..636de71cd8 100644 --- a/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java +++ b/engine/src/test/java/org/hibernate/validator/test/predefinedscope/PredefinedScopeValidatorFactoryTest.java @@ -330,7 +330,27 @@ public String interpolate(String messageTemplate, Context context) { } @Test - public void testXmlDefinedConstraints() { + public void testXmlDefinedConstraintsDiscoveryDisabled() { + // we assume that all the metadata is defined in the xml, + // hence there is no built-in constraints nor beans to init, + // But we also do not ask HV to append the sets with the beans from XMLs or built-in constraints: + try ( + ValidatorFactory factory = Validation.byProvider( PredefinedScopeHibernateValidator.class ) + .configure() + .builtinConstraints( Collections.emptySet() ) + .initializeBeanMetaData( Collections.emptySet() ) + .includeBeansAndConstraintsDefinedOnlyInXml( false ) + .addMapping( PredefinedScopeValidatorFactoryTest.class.getResourceAsStream( "constraints-simplexmlbean.xml" ) ) + .buildValidatorFactory() + ) { + Validator validator = factory.getValidator(); + // Because all the metadata for this bean was in the XML, and we ignore it: + assertNoViolations( validator.validate( new SimpleXmlBean() ) ); + } + } + + @Test + public void testXmlDefinedConstraintsDiscoveryEnabled() { // we assume that all the metadata is defined in the xml, // hence there is no built-in constraints nor beans to init: try ( @@ -349,7 +369,6 @@ public void testXmlDefinedConstraints() { violationOf( NotNull.class ).withMessage( "must not be null" ) ); } - } private static ValidatorFactory getValidatorFactory() {