diff --git a/documentation/pom.xml b/documentation/pom.xml index acfc746309..474208e766 100644 --- a/documentation/pom.xml +++ b/documentation/pom.xml @@ -39,7 +39,7 @@ true true - -Duser.language=en + -Duser.language=en -Duser.country=EN forbidden-allow-junit.txt .. 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 59463b202b..3385f0d753 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 @@ -62,6 +62,8 @@ import java.lang.annotation.Annotation; import java.lang.invoke.MethodHandles; import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.ArrayList; @@ -78,6 +80,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import jakarta.validation.Valid; import org.hibernate.validator.constraints.CodePointLength; import org.hibernate.validator.constraints.ConstraintComposition; import org.hibernate.validator.constraints.CreditCardNumber; @@ -324,6 +327,7 @@ import org.hibernate.validator.internal.constraintvalidators.hv.time.DurationMaxValidator; import org.hibernate.validator.internal.constraintvalidators.hv.time.DurationMinValidator; import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorDescriptor; +import org.hibernate.validator.internal.properties.Constrainable; import org.hibernate.validator.internal.util.CollectionHelper; import org.hibernate.validator.internal.util.Contracts; import org.hibernate.validator.internal.util.logging.Log; @@ -381,6 +385,14 @@ public class ConstraintHelper { private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() ); private static final String JODA_TIME_CLASS_NAME = "org.joda.time.ReadableInstant"; private static final String JAVA_MONEY_CLASS_NAME = "javax.money.MonetaryAmount"; + private static final java.util.regex.Pattern BUILTIN_TYPE_NAMES = java.util.regex.Pattern.compile( "" + + // primitives + "(boolean|byte|char|int|short|float|double|long" + + // boxed primitives + "|java.lang.Boolean|java.lang.Byte|java.lang.Character|java.lang.Integer|java.lang.Short|java.lang.Float|java.lang.Double|java.lang.Long" + + // selected final types from java.* hierarchy + "|java.lang.String" + + ")" ); @Immutable private final Map, List>> enabledBuiltinConstraints; @@ -757,7 +769,8 @@ private ConstraintHelper(Set enabledBuiltinConstraints) { putBuiltinConstraint( tmpConstraints, EAN.class, EANValidator.class ); } if ( enabledBuiltinConstraints.contains( ORG_HIBERNATE_VALIDATOR_CONSTRAINTS_EMAIL ) ) { - putBuiltinConstraint( tmpConstraints, org.hibernate.validator.constraints.Email.class, org.hibernate.validator.internal.constraintvalidators.hv.EmailValidator.class ); + putBuiltinConstraint( tmpConstraints, org.hibernate.validator.constraints.Email.class, + org.hibernate.validator.internal.constraintvalidators.hv.EmailValidator.class ); } if ( enabledBuiltinConstraints.contains( ORG_HIBERNATE_VALIDATOR_CONSTRAINTS_ISBN ) ) { putBuiltinConstraint( tmpConstraints, ISBN.class, ISBNValidator.class ); @@ -787,7 +800,8 @@ private ConstraintHelper(Set enabledBuiltinConstraints) { putBuiltinConstraint( tmpConstraints, NIP.class, NIPValidator.class ); } if ( enabledBuiltinConstraints.contains( ORG_HIBERNATE_VALIDATOR_CONSTRAINTS_NOT_BLANK ) ) { - putBuiltinConstraint( tmpConstraints, org.hibernate.validator.constraints.NotBlank.class, org.hibernate.validator.internal.constraintvalidators.hv.NotBlankValidator.class ); + putBuiltinConstraint( tmpConstraints, org.hibernate.validator.constraints.NotBlank.class, + org.hibernate.validator.internal.constraintvalidators.hv.NotBlankValidator.class ); } if ( enabledBuiltinConstraints.contains( ORG_HIBERNATE_VALIDATOR_CONSTRAINTS_NOT_EMPTY ) ) { putBuiltinConstraint( tmpConstraints, org.hibernate.validator.constraints.NotEmpty.class ); @@ -856,9 +870,7 @@ public static Set getBuiltinConstraints() { } /** - * Returns the constraint validator classes for the given constraint - * annotation type, as retrieved from - * + * Returns the constraint validator classes for the given constraint annotation type, as retrieved from *
    *
  • {@link Constraint#validatedBy()}, *
  • internally registered validators for built-in constraints
    • @@ -866,12 +878,10 @@ public static Set getBuiltinConstraints() { *
  • programmatically registered validators (see * {@link org.hibernate.validator.cfg.ConstraintMapping#constraintDefinition(Class)}).
    • *
- * * The result is cached internally. * * @param annotationType The constraint annotation type. - * @param the type of the annotation - * + * @param the type of the annotation * @return The validator classes for the given type. */ public List> getAllValidatorDescriptors(Class annotationType) { @@ -880,19 +890,17 @@ public List> getAllValid } /** - * Returns those validator descriptors for the given constraint annotation - * matching the given target. + * Returns those validator descriptors for the given constraint annotation matching the given target. * - * @param annotationType The annotation of interest. + * @param annotationType The annotation of interest. * @param validationTarget The target, either annotated element or parameters. - * @param the type of the annotation - * + * @param the type of the annotation * @return A list with matching validator descriptors. */ public List> findValidatorDescriptors(Class annotationType, ValidationTarget validationTarget) { return getAllValidatorDescriptors( annotationType ).stream() - .filter( d -> supportsValidationTarget( d, validationTarget ) ) - .collect( Collectors.toList() ); + .filter( d -> supportsValidationTarget( d, validationTarget ) ) + .collect( Collectors.toList() ); } private boolean supportsValidationTarget(ConstraintValidatorDescriptor validatorDescriptor, ValidationTarget target) { @@ -900,17 +908,16 @@ private boolean supportsValidationTarget(ConstraintValidatorDescriptor valida } /** - * Registers the given validator descriptors with the given constraint - * annotation type. + * Registers the given validator descriptors with the given constraint annotation type. * - * @param annotationType The constraint annotation type + * @param annotationType The constraint annotation type * @param validatorDescriptors The validator descriptors to register - * @param keepExistingClasses Whether already-registered validators should be kept or not - * @param the type of the annotation + * @param keepExistingClasses Whether already-registered validators should be kept or not + * @param the type of the annotation */ public void putValidatorDescriptors(Class annotationType, - List> validatorDescriptors, - boolean keepExistingClasses) { + List> validatorDescriptors, + boolean keepExistingClasses) { List> validatorDescriptorsToAdd = new ArrayList<>(); @@ -930,9 +937,7 @@ 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. + * @return {@code true} if the specified annotation is a multi value constraints, {@code false} otherwise. */ public boolean isMultiValueConstraint(Class annotationType) { if ( isJdkAnnotation( annotationType ) ) { @@ -962,12 +967,10 @@ public boolean isMultiValueConstraint(Class annotationType /** * Returns the constraints which are part of the given multi-value constraint. *

- * Invoke {@link #isMultiValueConstraint(Class)} prior to calling this method to check whether a given constraint - * actually is a multi-value constraint. + * Invoke {@link #isMultiValueConstraint(Class)} prior to calling this method to check whether a given constraint actually is a multi-value constraint. * * @param multiValueConstraint the multi-value constraint annotation from which to retrieve the contained constraints - * @param the type of the annotation - * + * @param the type of the annotation * @return A list of constraint annotations, may be empty but never {@code null}. */ public List getConstraintsFromMultiValueConstraint(A multiValueConstraint) { @@ -982,8 +985,7 @@ public List getConstraintsFromMultiValueConst } /** - * Checks whether the specified annotation is a valid constraint annotation. A constraint annotation has to - * fulfill the following conditions: + * Checks whether the specified annotation is a valid constraint annotation. A constraint annotation has to fulfill the following conditions: *

* * @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) { @@ -1133,10 +1134,7 @@ 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. + * @return A list with the default validators as retrieved from {@link Constraint#validatedBy()} or the list of validators for built-in constraints. */ @SuppressWarnings("unchecked") private List> getDefaultValidatorDescriptors(Class annotationType) { @@ -1173,9 +1171,39 @@ private static T run(PrivilegedAction action) { } /** - * A type-safe wrapper around a concurrent map from constraint types to - * associated validator classes. The casts are safe as data is added trough - * the typed API only. + * this method inspects the type of the @Valid annotation and decides, if the annotation is useful. + *

+ * This method returns false, if the {@link jakarta.validation.Valid} annotation is applied to types matching {@link #BUILTIN_TYPE_NAMES}: + *

    + *
  • a native type (int, boolean, etc
    • + *
  • a boxed native type (Integer, Boolean, etc
    • + *
  • some types in java.lang.* package, eg String
    • + *
+ *

+ * + * @param annotation the Valid annotation + * @param constrainable the constraint element + * @param type of annotation + * @return true, if the Valid annotation is not applicable + */ + public boolean isNonApplicableValidAnnotation(A annotation, Constrainable constrainable) { + if ( !( annotation instanceof Valid ) ) { + return false; + } + String typeName = constrainable.getType().getTypeName(); + Type typeForResolution = constrainable.getTypeForValidatorResolution(); + if ( typeForResolution instanceof ParameterizedType ) { + return Arrays.stream( ( (ParameterizedType) typeForResolution ).getActualTypeArguments() ) + .allMatch( pType -> BUILTIN_TYPE_NAMES.matcher( pType.getTypeName() ).matches() ); + } + else { + return BUILTIN_TYPE_NAMES.matcher( typeName ).matches(); + } + } + + /** + * A type-safe wrapper around a concurrent map from constraint types to associated validator classes. The casts are safe as data is added trough the typed + * API only. * * @author Gunnar Morling */ diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java index b2f8a9c778..ee7e77318d 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java @@ -529,6 +529,11 @@ protected List> findConstrain return Collections.emptyList(); } + // address HV-1831: do not create an Annotation object for unwanted Valid annotations + if ( constraintCreationContext.getConstraintHelper().isNonApplicableValidAnnotation( annotation, constrainable ) ) { + return Collections.emptyList(); + } + List constraints = newArrayList(); Class annotationType = annotation.annotationType(); if ( constraintCreationContext.getConstraintHelper().isConstraintAnnotation( annotationType ) ) { diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/traversableresolver/Suit.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/traversableresolver/Suit.java index 4c59bdaea7..d0ab1432a5 100644 --- a/engine/src/test/java/org/hibernate/validator/test/internal/engine/traversableresolver/Suit.java +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/traversableresolver/Suit.java @@ -12,18 +12,30 @@ import jakarta.validation.GroupSequence; import jakarta.validation.groups.Default; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + /** * @author Emmanuel Bernard */ -@GroupSequence( {Suit.class, Cloth.class }) +@GroupSequence({ Suit.class, Cloth.class }) public class Suit { @Max(value = 50, groups = { Default.class, Cloth.class }) @Min(1) + @Valid // this should be ignored private Integer size; - @Valid private Trousers trousers; + @Valid + private Trousers trousers; private Jacket jacket; + @Valid + private boolean awesomeDesign; + + @Valid Set someSet = new HashSet<>( Arrays.asList( 1, 2, 3, 4, 5, 6 ) ); + Set<@Valid Integer> someOtherSet = new HashSet<>( Arrays.asList( 1, 2, 3, 4, 5, 6 ) ); + public Trousers getTrousers() { return trousers; } @@ -48,4 +60,12 @@ public Integer getSize() { public void setSize(Integer size) { this.size = size; } + + public boolean isAwesomeDesign() { + return awesomeDesign; + } + + public void setAwesomeDesign(boolean awesomeDesign) { + this.awesomeDesign = awesomeDesign; + } } diff --git a/performance/pom.xml b/performance/pom.xml index f2f4559e01..beadb7ae9e 100644 --- a/performance/pom.xml +++ b/performance/pom.xml @@ -190,9 +190,10 @@ log4j log4j + 1.2.17 - + hv-6.1 @@ -234,6 +236,7 @@ log4j log4j + 1.2.17 diff --git a/performance/src/main/java/org/hibernate/validator/performance/BenchmarkRunner.java b/performance/src/main/java/org/hibernate/validator/performance/BenchmarkRunner.java index 728e66cc81..43103c252d 100644 --- a/performance/src/main/java/org/hibernate/validator/performance/BenchmarkRunner.java +++ b/performance/src/main/java/org/hibernate/validator/performance/BenchmarkRunner.java @@ -10,6 +10,8 @@ import java.util.stream.Stream; import org.hibernate.validator.performance.cascaded.CascadedValidation; +import org.hibernate.validator.performance.cascaded.CascadedValidationWithManyPrimitiveValids; +import org.hibernate.validator.performance.cascaded.CascadedValidationWithoutPrimitiveValids; import org.hibernate.validator.performance.cascaded.CascadedWithLotsOfItemsValidation; import org.hibernate.validator.performance.simple.SimpleValidation; import org.hibernate.validator.performance.statistical.StatisticalValidation; @@ -32,13 +34,15 @@ public final class BenchmarkRunner { private static final Stream> DEFAULT_TEST_CLASSES = Stream.of( - SimpleValidation.class.getName(), - CascadedValidation.class.getName(), - CascadedWithLotsOfItemsValidation.class.getName(), - StatisticalValidation.class.getName(), + CascadedValidationWithManyPrimitiveValids.class.getName(), + CascadedValidationWithoutPrimitiveValids.class.getName() +// SimpleValidation.class.getName(), +// CascadedValidation.class.getName(), +// CascadedWithLotsOfItemsValidation.class.getName(), +// StatisticalValidation.class.getName()//, // Benchmarks specific to Bean Validation 2.0 // Tests are located in a separate source folder only added for implementations compatible with BV 2.0 - "org.hibernate.validator.performance.multilevel.MultiLevelContainerValidation" + //"org.hibernate.validator.performance.multilevel.MultiLevelContainerValidation" ).map( BenchmarkRunner::classForName ).filter( Objects::nonNull ); private BenchmarkRunner() { diff --git a/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithManyPrimitiveValids.java b/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithManyPrimitiveValids.java new file mode 100644 index 0000000000..431a6474db --- /dev/null +++ b/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithManyPrimitiveValids.java @@ -0,0 +1,139 @@ +/* + * 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.performance.cascaded; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Valid; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import jakarta.validation.constraints.NotNull; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Hardy Ferentschik + */ +public class CascadedValidationWithManyPrimitiveValids { + + @State(Scope.Benchmark) + public static class CascadedValidationState { + + public volatile Validator validator; + public volatile Person person; + + public CascadedValidationState() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + validator = factory.getValidator(); + + // TODO graphs needs to be generated and deeper + Person kermit = new Person( "kermit", 55, 0, 1_000_000_000L, 25.6f, true ); + Person piggy = new Person( "miss piggy", 19, 0, 10_000_000L, 55.6f, true ); + Person gonzo = new Person( "gonzo", 55, 1_000_000, 100_000_000L, 35.1f, true ); + + for ( var i = 0; i < 10; i++ ) { + kermit.addBloodPressureReading( i ); + piggy.addBloodPressureReading( 1000 + i ); + gonzo.addBloodPressureReading( 10000 + i ); + kermit.addBrainCellCount( (long) i ); + piggy.addBrainCellCount( (long) ( 1000 + i ) ); + gonzo.addBrainCellCount( (long) ( 10000 + i ) ); + } + + kermit.addFriend( piggy ).addFriend( gonzo ); + piggy.addFriend( kermit ).addFriend( gonzo ); + gonzo.addFriend( kermit ).addFriend( piggy ); + + person = kermit; + } + } + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Fork(value = 1) + @Threads(50) + @Warmup(iterations = 10) + @Measurement(iterations = 20) + public void testCascadedValidation(CascadedValidationState state, Blackhole bh) { + Set> violations = state.validator.validate( state.person ); + assertThat( violations ).hasSize( 0 ); + + bh.consume( violations ); + } + + public static class Person { + + @NotNull + @Valid + String name; + + @Valid + int age; + + @Valid + long hairCount; + + @Valid + double balance; + + @Valid + float size; + + @Valid + boolean alive; + + @Valid + List bloodPressureReadings = new ArrayList<>(); + + List<@Valid Long> brainCellCount = new ArrayList<>(); + + @Valid + Set friends = new HashSet<>(); + + public Person(String name, int age, long hairCount, double balance, float size, boolean alive) { + this.name = name; + this.age = age; + this.hairCount = hairCount; + this.balance = balance; + this.size = size; + this.alive = alive; + } + + public Person addFriend(Person friend) { + friends.add( friend ); + return this; + } + + public Person addBrainCellCount(Long bcCount) { + brainCellCount.add( bcCount ); + return this; + } + + public Person addBloodPressureReading(Integer bpReading) { + bloodPressureReadings.add( bpReading ); + return this; + } + } +} diff --git a/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithoutPrimitiveValids.java b/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithoutPrimitiveValids.java new file mode 100644 index 0000000000..06b4048f42 --- /dev/null +++ b/performance/src/main/java/org/hibernate/validator/performance/cascaded/CascadedValidationWithoutPrimitiveValids.java @@ -0,0 +1,132 @@ +/* + * 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.performance.cascaded; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Valid; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import jakarta.validation.constraints.NotNull; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Hardy Ferentschik + */ +public class CascadedValidationWithoutPrimitiveValids { + + @State(Scope.Benchmark) + public static class CascadedValidationState { + + public volatile Validator validator; + public volatile Person person; + + public CascadedValidationState() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + validator = factory.getValidator(); + + // TODO graphs needs to be generated and deeper + Person kermit = new Person( "kermit", 55, 0, 1_000_000_000L, 25.6f, true ); + Person piggy = new Person( "miss piggy", 19, 0, 10_000_000L, 55.6f, true ); + Person gonzo = new Person( "gonzo", 55, 1_000_000, 100_000_000L, 35.1f, true ); + + for ( var i = 0; i < 10; i++ ) { + kermit.addBloodPressureReading( i ); + piggy.addBloodPressureReading( 1000 + i ); + gonzo.addBloodPressureReading( 10000 + i ); + kermit.addBrainCellCount( (long) i ); + piggy.addBrainCellCount( (long) ( 1000 + i ) ); + gonzo.addBrainCellCount( (long) ( 10000 + i ) ); + } + + kermit.addFriend( piggy ).addFriend( gonzo ); + piggy.addFriend( kermit ).addFriend( gonzo ); + gonzo.addFriend( kermit ).addFriend( piggy ); + + person = kermit; + } + } + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + @Fork(value = 1) + @Threads(50) + @Warmup(iterations = 10) + @Measurement(iterations = 20) + public void testCascadedValidation(CascadedValidationState state, Blackhole bh) { + Set> violations = state.validator.validate( state.person ); + assertThat( violations ).hasSize( 0 ); + + bh.consume( violations ); + } + + public static class Person { + + @NotNull + String name; + + int age; + + long hairCount; + + double balance; + + float size; + + boolean alive; + + List bloodPressureReadings = new ArrayList<>(); + + List brainCellCount = new ArrayList<>(); + + @Valid + Set friends = new HashSet<>(); + + public Person(String name, int age, long hairCount, double balance, float size, boolean alive) { + this.name = name; + this.age = age; + this.hairCount = hairCount; + this.balance = balance; + this.size = size; + this.alive = alive; + } + + public Person addFriend(Person friend) { + friends.add( friend ); + return this; + } + + public Person addBrainCellCount(Long bcCount) { + brainCellCount.add( bcCount ); + return this; + } + + public Person addBloodPressureReading(Integer bpReading) { + bloodPressureReadings.add( bpReading ); + return this; + } + } +}