Skip to content

HV-2127 Make Path implementation not rely on Lists (or other collections) #1672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
package org.hibernate.validator.cdi.interceptor.spi;

import java.io.Serial;
import java.io.Serializable;
import java.lang.reflect.Member;
import java.util.Arrays;
Expand Down Expand Up @@ -37,6 +38,7 @@
@Priority(Interceptor.Priority.PLATFORM_AFTER + 800)
public class ValidationInterceptor implements Serializable {

@Serial
private static final long serialVersionUID = 604440259030722151L;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static java.util.stream.Collectors.toList;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -53,7 +54,10 @@ public boolean isValid(Collection collection, ConstraintValidatorContext constra
if ( constraintValidatorContext instanceof HibernateConstraintValidatorContext ) {
constraintValidatorContext.unwrap( HibernateConstraintValidatorContext.class )
.addMessageParameter( "duplicates", duplicates.stream().map( String::valueOf ).collect( Collectors.joining( ", " ) ) )
.withDynamicPayload( CollectionHelper.toImmutableList( duplicates ) );
// We cannot leverage the CollectionHelper.toImmutableList here as it does not allow `null` values.
// User collections may have `null`s in it and those could as well be duplicates
// so let's rely on the Collections.unmodifiableList here which accepts `null` values as long as the underlying collection allows it:
.withDynamicPayload( Collections.unmodifiableList( duplicates ) );
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
package org.hibernate.validator.internal.engine;

import java.io.Serial;
import java.io.Serializable;
import java.lang.invoke.MethodHandles;
import java.util.Map;
Expand All @@ -24,6 +25,7 @@
public class ConstraintViolationImpl<T> implements HibernateConstraintViolation<T>, Serializable {

private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
@Serial
private static final long serialVersionUID = -4970067626703103139L;

private final String interpolatedMessage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import org.hibernate.validator.internal.engine.groups.Sequence;
import org.hibernate.validator.internal.engine.groups.ValidationOrder;
import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator;
import org.hibernate.validator.internal.engine.path.ModifiablePath;
import org.hibernate.validator.internal.engine.path.NodeImpl;
import org.hibernate.validator.internal.engine.path.PathImpl;
import org.hibernate.validator.internal.engine.resolver.TraversableResolvers;
import org.hibernate.validator.internal.engine.validationcontext.BaseBeanValidationContext;
import org.hibernate.validator.internal.engine.validationcontext.ExecutableValidationContext;
Expand Down Expand Up @@ -164,7 +164,7 @@ public final <T> Set<ConstraintViolation<T>> validate(T object, Class<?>... grou
validatorScopedContext.getParameterNameProvider(),
object,
validationContext.getRootBeanMetaData(),
PathImpl.createRootPath()
ModifiablePath.createRootPath()
);

return validateInContext( validationContext, valueContext, validationOrder );
Expand All @@ -184,7 +184,7 @@ public final <T> Set<ConstraintViolation<T>> validateProperty(T object, String p
return Collections.emptySet();
}

PathImpl propertyPath = PathImpl.createPathFromString( propertyName );
ModifiablePath propertyPath = ModifiablePath.createPathFromString( propertyName );
BaseBeanValidationContext<T> validationContext = getValidationContextBuilder().forValidateProperty( rootBeanClass, rootBeanMetaData, object,
propertyPath );

Expand All @@ -211,7 +211,7 @@ public final <T> Set<ConstraintViolation<T>> validateValue(Class<T> beanType, St
return Collections.emptySet();
}

PathImpl propertyPath = PathImpl.createPathFromString( propertyName );
ModifiablePath propertyPath = ModifiablePath.createPathFromString( propertyName );
BaseBeanValidationContext<T> validationContext = getValidationContextBuilder().forValidateValue( beanType, rootBeanMetaData, propertyPath );

ValidationOrder validationOrder = determineGroupValidationOrder( groups );
Expand Down Expand Up @@ -813,7 +813,7 @@ private BeanValueContext<?, Object> buildNewLocalExecutionContext(ValueContext<?
return newValueContext;
}

private <T> Set<ConstraintViolation<T>> validateValueInContext(BaseBeanValidationContext<T> validationContext, Object value, PathImpl propertyPath,
private <T> Set<ConstraintViolation<T>> validateValueInContext(BaseBeanValidationContext<T> validationContext, Object value, ModifiablePath propertyPath,
ValidationOrder validationOrder) {
BeanValueContext<?, Object> valueContext = getValueContextForValueValidation( validationContext.getRootBeanClass(), propertyPath );
valueContext.setCurrentValidatedValue( value );
Expand Down Expand Up @@ -902,7 +902,7 @@ private <T> void validateParametersInContext(ExecutableValidationContext<T> vali
validatorScopedContext.getParameterNameProvider(),
parameterValues,
executableMetaData.getValidatableParametersMetaData(),
PathImpl.createPathForExecutable( executableMetaData )
ModifiablePath.createPathForExecutable( executableMetaData )
);

groupIterator = validationOrder.getGroupIterator();
Expand Down Expand Up @@ -1035,7 +1035,7 @@ private <T> ValueContext<T, Object> getExecutableValueContext(T object, Executab
validatorScopedContext.getParameterNameProvider(),
object,
validatable,
PathImpl.createPathForExecutable( executableMetaData )
ModifiablePath.createPathForExecutable( executableMetaData )
);

valueContext.setCurrentGroup( group );
Expand Down Expand Up @@ -1078,7 +1078,7 @@ private <V, T> void validateReturnValueInContext(ExecutableValidationContext<T>
validatorScopedContext.getParameterNameProvider(),
value,
executableMetaData.getReturnValueMetaData(),
PathImpl.createPathForExecutable( executableMetaData )
ModifiablePath.createPathForExecutable( executableMetaData )
);

groupIterator = validationOrder.getGroupIterator();
Expand Down Expand Up @@ -1184,7 +1184,7 @@ private <T> void validateReturnValueForSingleGroup(BaseBeanValidationContext<T>
* @return Returns an instance of {@code ValueContext} which describes the local validation context associated to
* the given property path.
*/
private <V> BeanValueContext<?, V> getValueContextForPropertyValidation(BaseBeanValidationContext<?> validationContext, PathImpl propertyPath) {
private <V> BeanValueContext<?, V> getValueContextForPropertyValidation(BaseBeanValidationContext<?> validationContext, ModifiablePath propertyPath) {
Class<?> clazz = validationContext.getRootBeanClass();
BeanMetaData<?> beanMetaData = validationContext.getRootBeanMetaData();
Object value = validationContext.getRootBean();
Expand Down Expand Up @@ -1262,7 +1262,7 @@ else if ( propertyPathNode.getKey() != null ) {
* the given property path.
*/
private <V> BeanValueContext<?, V> getValueContextForValueValidation(Class<?> rootBeanClass,
PathImpl propertyPath) {
ModifiablePath propertyPath) {
Class<?> clazz = rootBeanClass;
BeanMetaData<?> beanMetaData = null;
PropertyMetaData propertyMetaData = null;
Expand Down Expand Up @@ -1330,13 +1330,13 @@ private boolean isValidationRequired(BaseBeanValidationContext<?> validationCont
);
}

private boolean isReachable(BaseBeanValidationContext<?> validationContext, Object traversableObject, PathImpl path,
private boolean isReachable(BaseBeanValidationContext<?> validationContext, Object traversableObject, ModifiablePath path,
ConstraintLocationKind constraintLocationKind) {
if ( needToCallTraversableResolver( path, constraintLocationKind ) ) {
return true;
}

Path pathToObject = PathImpl.createCopyWithoutLeafNode( path );
Path pathToObject = ModifiablePath.createCopyWithoutLeafNode( path );
try {
return validationContext.getTraversableResolver().isReachable(
traversableObject,
Expand All @@ -1351,7 +1351,7 @@ private boolean isReachable(BaseBeanValidationContext<?> validationContext, Obje
}
}

private boolean needToCallTraversableResolver(PathImpl path, ConstraintLocationKind constraintLocationKind) {
private boolean needToCallTraversableResolver(ModifiablePath path, ConstraintLocationKind constraintLocationKind) {
// as the TraversableResolver interface is designed right now it does not make sense to call it when
// there is no traversable object hosting the property to be accessed. For this reason we don't call the resolver
// for class level constraints (ElementType.TYPE) or top level method parameters or return values.
Expand All @@ -1362,7 +1362,7 @@ private boolean needToCallTraversableResolver(PathImpl path, ConstraintLocationK
|| isReturnValueValidation( path );
}

private boolean isCascadeRequired(BaseBeanValidationContext<?> validationContext, Object traversableObject, PathImpl path,
private boolean isCascadeRequired(BaseBeanValidationContext<?> validationContext, Object traversableObject, ModifiablePath path,
ConstraintLocationKind constraintLocationKind) {
if ( needToCallTraversableResolver( path, constraintLocationKind ) ) {
return true;
Expand All @@ -1373,7 +1373,7 @@ private boolean isCascadeRequired(BaseBeanValidationContext<?> validationContext
return false;
}

Path pathToObject = PathImpl.createCopyWithoutLeafNode( path );
Path pathToObject = ModifiablePath.createCopyWithoutLeafNode( path );
try {
return validationContext.getTraversableResolver().isCascadable(
traversableObject,
Expand All @@ -1392,15 +1392,15 @@ private boolean isClassLevelConstraint(ConstraintLocationKind constraintLocation
return ConstraintLocationKind.TYPE.equals( constraintLocationKind );
}

private boolean isCrossParameterValidation(PathImpl path) {
private boolean isCrossParameterValidation(ModifiablePath path) {
return path.getLeafNode().getKind() == ElementKind.CROSS_PARAMETER;
}

private boolean isParameterValidation(PathImpl path) {
private boolean isParameterValidation(ModifiablePath path) {
return path.getLeafNode().getKind() == ElementKind.PARAMETER;
}

private boolean isReturnValueValidation(PathImpl path) {
private boolean isReturnValueValidation(ModifiablePath path) {
return path.getLeafNode().getKind() == ElementKind.RETURN_VALUE;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import jakarta.validation.ConstraintValidator;
Expand All @@ -38,7 +37,7 @@
* @author Guillaume Smet
* @author Marko Bekhta
*/
class ComposingConstraintTree<B extends Annotation> extends ConstraintTree<B> {
final class ComposingConstraintTree<B extends Annotation> extends ConstraintTree<B> {

private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );

Expand All @@ -61,6 +60,23 @@ private <U extends Annotation> ConstraintTree<U> createConstraintTree(Constraint
}
}

@Override
public boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext) {
List<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts = new ArrayList<>( 5 );
validateConstraints( validationContext, valueContext, violatedConstraintValidatorContexts );
if ( !violatedConstraintValidatorContexts.isEmpty() ) {
for ( ConstraintValidatorContextImpl constraintValidatorContext : violatedConstraintValidatorContexts ) {
for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
validationContext.addConstraintFailure(
valueContext, constraintViolationCreationContext, constraintValidatorContext.getConstraintDescriptor()
);
}
}
return false;
}
return true;
}

@Override
protected void validateConstraints(ValidationContext<?> validationContext,
ValueContext<?, ?> valueContext,
Expand All @@ -69,7 +85,7 @@ protected void validateConstraints(ValidationContext<?> validationContext,
validationContext, valueContext, violatedConstraintValidatorContexts
);

Optional<ConstraintValidatorContextImpl> violatedLocalConstraintValidatorContext;
ConstraintValidatorContextImpl violatedLocalConstraintValidatorContext;

// After all children are validated the actual ConstraintValidator of the constraint itself is executed
if ( mainConstraintNeedsEvaluation( validationContext, violatedConstraintValidatorContexts ) ) {
Expand Down Expand Up @@ -103,15 +119,15 @@ protected void validateConstraints(ValidationContext<?> validationContext,

// We re-evaluate the boolean composition by taking into consideration also the violations
// from the local constraintValidator
if ( !violatedLocalConstraintValidatorContext.isPresent() ) {
if ( violatedLocalConstraintValidatorContext == null ) {
compositionResult.setAtLeastOneTrue( true );
}
else {
compositionResult.setAllTrue( false );
}
}
else {
violatedLocalConstraintValidatorContext = Optional.empty();
violatedLocalConstraintValidatorContext = null;
}

if ( !passesCompositionTypeRequirement( violatedConstraintValidatorContexts, compositionResult ) ) {
Expand Down Expand Up @@ -157,7 +173,7 @@ private boolean mainConstraintNeedsEvaluation(ValidationContext<?> validationCon
private void prepareFinalConstraintViolations(ValidationContext<?> validationContext,
ValueContext<?, ?> valueContext,
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts,
Optional<ConstraintValidatorContextImpl> localConstraintValidatorContext) {
ConstraintValidatorContextImpl localConstraintValidatorContext) {
if ( reportAsSingleViolation() ) {
// We clear the current violations list anyway
violatedConstraintValidatorContexts.clear();
Expand All @@ -166,7 +182,7 @@ private void prepareFinalConstraintViolations(ValidationContext<?> validationCon
// violations or not (or if there is no local ConstraintValidator at all).
// If not we create a violation
// using the error message in the annotation declaration at top level.
if ( !localConstraintValidatorContext.isPresent() ) {
if ( localConstraintValidatorContext == null ) {
violatedConstraintValidatorContexts.add(
validationContext.createConstraintValidatorContextFor(
descriptor, valueContext.getPropertyPath()
Expand All @@ -183,8 +199,8 @@ private void prepareFinalConstraintViolations(ValidationContext<?> validationCon
// as checked in test CustomErrorMessage.java
// If no violations have been reported from the local ConstraintValidator, or no such validator exists,
// then we just add an empty list.
if ( localConstraintValidatorContext.isPresent() ) {
violatedConstraintValidatorContexts.add( localConstraintValidatorContext.get() );
if ( localConstraintValidatorContext != null ) {
violatedConstraintValidatorContexts.add( localConstraintValidatorContext );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
import java.lang.annotation.Annotation;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;

import jakarta.validation.ConstraintDeclarationException;
Expand All @@ -33,7 +31,7 @@
* @author Guillaume Smet
* @author Marko Bekhta
*/
public abstract class ConstraintTree<A extends Annotation> {
public abstract sealed class ConstraintTree<A extends Annotation> permits SimpleConstraintTree, ComposingConstraintTree {

private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );

Expand Down Expand Up @@ -68,21 +66,7 @@ public static <U extends Annotation> ConstraintTree<U> of(ConstraintValidatorMan
}
}

public final boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext) {
List<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts = new ArrayList<>( 5 );
validateConstraints( validationContext, valueContext, violatedConstraintValidatorContexts );
if ( !violatedConstraintValidatorContexts.isEmpty() ) {
for ( ConstraintValidatorContextImpl constraintValidatorContext : violatedConstraintValidatorContexts ) {
for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
validationContext.addConstraintFailure(
valueContext, constraintViolationCreationContext, constraintValidatorContext.getConstraintDescriptor()
);
}
}
return false;
}
return true;
}
public abstract boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext);

protected abstract void validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext,
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts);
Expand Down Expand Up @@ -168,7 +152,7 @@ private ValidationException getExceptionForNullValidator(Type validatedValueType
* @return an {@link Optional#empty()} if there is no violation or a corresponding {@link ConstraintValidatorContextImpl}
* otherwise.
*/
protected final <V> Optional<ConstraintValidatorContextImpl> validateSingleConstraint(
protected final <V> ConstraintValidatorContextImpl validateSingleConstraint(
ValueContext<?, ?> valueContext,
ConstraintValidatorContextImpl constraintValidatorContext,
ConstraintValidator<A, V> validator) {
Expand All @@ -187,9 +171,9 @@ protected final <V> Optional<ConstraintValidatorContextImpl> validateSingleConst
if ( !isValid ) {
//We do not add these violations yet, since we don't know how they are
//going to influence the final boolean evaluation
return Optional.of( constraintValidatorContext );
return constraintValidatorContext;
}
return Optional.empty();
return null;
}

@Override
Expand Down
Loading
Loading