Skip to content
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

Explore FreeForm validation v2 #989

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.hibernate.validator.internal.metadata.provider.XmlMetaDataProvider;
import org.hibernate.validator.internal.properties.DefaultGetterPropertySelectionStrategy;
import org.hibernate.validator.internal.properties.javabean.JavaBeanHelper;
import org.hibernate.validator.internal.properties.propertyholder.PropertyAccessorCreatorProvider;
import org.hibernate.validator.internal.util.Contracts;
import org.hibernate.validator.internal.util.ExecutableHelper;
import org.hibernate.validator.internal.util.ExecutableParameterNameProvider;
Expand Down Expand Up @@ -136,6 +137,8 @@ public class ValidatorFactoryImpl implements HibernateValidatorFactory {

private final ValueExtractorManager valueExtractorManager;

private final PropertyAccessorCreatorProvider propertyAccessorCreatorProvider;

private final JavaBeanHelper javaBeanHelper;

private final ValidationOrderGenerator validationOrderGenerator;
Expand All @@ -144,6 +147,7 @@ public ValidatorFactoryImpl(ConfigurationState configurationState) {
ClassLoader externalClassLoader = getExternalClassLoader( configurationState );

this.valueExtractorManager = new ValueExtractorManager( configurationState.getValueExtractors() );
this.propertyAccessorCreatorProvider = new PropertyAccessorCreatorProvider();
this.beanMetaDataManagers = new ConcurrentHashMap<>();
this.constraintHelper = new ConstraintHelper();
this.typeResolutionHelper = new TypeResolutionHelper();
Expand Down Expand Up @@ -361,6 +365,7 @@ Validator createValidator(ConstraintValidatorFactory constraintValidatorFactory,
typeResolutionHelper,
validatorFactoryScopedContext.getParameterNameProvider(),
valueExtractorManager,
propertyAccessorCreatorProvider,
javaBeanHelper,
validationOrderGenerator,
methodValidationConfiguration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.core.ConstraintHelper;
import org.hibernate.validator.internal.metadata.raw.ConfigurationSource;
import org.hibernate.validator.internal.metadata.raw.ConstrainedElement;
import org.hibernate.validator.internal.metadata.raw.ConstrainedField;
import org.hibernate.validator.internal.metadata.raw.propertyholder.ConstrainedPropertyHolderElementBuilder;
import org.hibernate.validator.internal.metadata.raw.propertyholder.PropertyHolderConfiguration;
import org.hibernate.validator.internal.metadata.raw.propertyholder.PropertyHolderConfigurationSource;
import org.hibernate.validator.internal.properties.propertyholder.PropertyAccessorCreatorProvider;
import org.hibernate.validator.internal.util.StringHelper;
import org.hibernate.validator.internal.util.TypeResolutionHelper;

Expand All @@ -37,35 +39,40 @@ public class PropertyHolderBeanMetaDataBuilder<T> {
private final Set<BuilderDelegate> builders = newHashSet();
private final TypeResolutionHelper typeResolutionHelper;
private final ValueExtractorManager valueExtractorManager;
private final PropertyAccessorCreatorProvider propertyAccessorCreatorProvider;

private PropertyHolderConfigurationSource sequenceSource;
private PropertyHolderConfigurationSource providerSource;
private ConfigurationSource sequenceSource;
private ConfigurationSource providerSource;
private List<Class<?>> defaultGroupSequence;


private PropertyHolderBeanMetaDataBuilder(
ConstraintHelper constraintHelper,
TypeResolutionHelper typeResolutionHelper,
ValueExtractorManager valueExtractorManager,
PropertyAccessorCreatorProvider propertyAccessorCreatorProvider,
ValidationOrderGenerator validationOrderGenerator,
Class<T> propertyHolderClass) {
this.propertyHolderClass = propertyHolderClass;
this.constraintHelper = constraintHelper;
this.validationOrderGenerator = validationOrderGenerator;
this.typeResolutionHelper = typeResolutionHelper;
this.valueExtractorManager = valueExtractorManager;
this.propertyAccessorCreatorProvider = propertyAccessorCreatorProvider;
}

public static <T> PropertyHolderBeanMetaDataBuilder<T> getInstance(
ConstraintHelper constraintHelper,
TypeResolutionHelper typeResolutionHelper,
ValueExtractorManager valueExtractorManager,
PropertyAccessorCreatorProvider propertyAccessorCreatorProvider,
ValidationOrderGenerator validationOrderGenerator,
Class<T> propertyHolderClass) {
return new PropertyHolderBeanMetaDataBuilder<>(
constraintHelper,
typeResolutionHelper,
valueExtractorManager,
propertyAccessorCreatorProvider,
validationOrderGenerator,
propertyHolderClass );
}
Expand All @@ -79,8 +86,18 @@ public void add(PropertyHolderConfiguration configuration) {
defaultGroupSequence = configuration.getDefaultGroupSequence();
}

for ( ConstrainedElement constrainedElement : configuration.getConstrainedElements() ) {
addMetaDataToBuilder( constrainedElement, builders );
for ( ConstrainedPropertyHolderElementBuilder constrainedElement : configuration.getConstrainedElements() ) {
if ( constrainedElement.isConstrained() ) {
addMetaDataToBuilder(
constrainedElement.build(
typeResolutionHelper,
valueExtractorManager,
propertyAccessorCreatorProvider,
propertyHolderClass
),
builders
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Hibernate Validator, declare and validate application constraints
*
* License: Apache License, Version 2.0
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
*/
package org.hibernate.validator.internal.metadata.core;

import java.lang.annotation.Annotation;

import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.descriptor.ConstraintDescriptorImpl;
import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
import org.hibernate.validator.internal.properties.Constrainable;
import org.hibernate.validator.internal.util.TypeResolutionHelper;

/**
* @author Marko Bekhta
*/
public class MetaConstraintBuilder<A extends Annotation> {

private final ConstraintDescriptorImpl<A> constraintDescriptor;
private final ConstraintLocation.Builder locationBuilder;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The part about location builder in this commit can be ignored. It will be removed in later commits. And I think about removing this builder as well. I think that storing the ConstraintDefs will be enough and this logic will be moved elsewhere.


public MetaConstraintBuilder(ConstraintDescriptorImpl<A> constraintDescriptor, ConstraintLocation.Builder locationBuilder) {
this.constraintDescriptor = constraintDescriptor;
this.locationBuilder = locationBuilder;
}

public MetaConstraint<A> build(TypeResolutionHelper typeResolutionHelper, ValueExtractorManager valueExtractorManager, Constrainable constrainable) {
return MetaConstraints.create( typeResolutionHelper, valueExtractorManager, constraintDescriptor, locationBuilder.build( constrainable ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ static ConstraintLocation forParameter(Callable callable, int index) {
*/
ConstraintLocationKind getKind();

interface Builder {
ConstraintLocation build(Constrainable constrainable);

static Builder forPropertyHolderProperty() {
return new PropertyHolderPropertyConstraintLocationBuilder();
}
}

enum ConstraintLocationKind {
TYPE( ElementType.TYPE ),
CONSTRUCTOR( ElementType.CONSTRUCTOR ),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Hibernate Validator, declare and validate application constraints
*
* License: Apache License, Version 2.0
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
*/
package org.hibernate.validator.internal.metadata.location;

import org.hibernate.validator.internal.metadata.location.ConstraintLocation.Builder;
import org.hibernate.validator.internal.properties.Constrainable;
import org.hibernate.validator.internal.properties.Field;
import org.hibernate.validator.internal.util.Contracts;

/**
* @author Marko Bekhta
*/
public class PropertyHolderPropertyConstraintLocationBuilder implements Builder {

@Override
public ConstraintLocation build(Constrainable constrainable) {
Contracts.assertTrue( constrainable instanceof Field, "Only Field instances are accepted." );

return ConstraintLocation.forField( constrainable.as( Field.class ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.hibernate.validator.internal.metadata.provider.MetaDataProvider;
import org.hibernate.validator.internal.metadata.provider.PropertyHolderMetaDataProvider;
import org.hibernate.validator.internal.properties.javabean.JavaBeanHelper;
import org.hibernate.validator.internal.properties.propertyholder.PropertyAccessorCreatorProvider;
import org.hibernate.validator.internal.util.ExecutableHelper;
import org.hibernate.validator.internal.util.ExecutableParameterNameProvider;
import org.hibernate.validator.internal.util.TypeResolutionHelper;
Expand Down Expand Up @@ -51,6 +52,7 @@ public ConstraintMetaDataManager(ConstraintHelper constraintHelper,
TypeResolutionHelper typeResolutionHelper,
ExecutableParameterNameProvider parameterNameProvider,
ValueExtractorManager valueExtractorManager,
PropertyAccessorCreatorProvider propertyAccessorCreatorProvider,
JavaBeanHelper javaBeanHelper,
ValidationOrderGenerator validationOrderGenerator,
MethodValidationConfiguration methodValidationConfiguration,
Expand All @@ -72,6 +74,7 @@ public ConstraintMetaDataManager(ConstraintHelper constraintHelper,
constraintHelper,
typeResolutionHelper,
valueExtractorManager,
propertyAccessorCreatorProvider,
validationOrderGenerator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we try to be consistent in the order of the parameters? propertyHolderMetaDataProviders should be at the end.

);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.hibernate.validator.internal.metadata.core.ConstraintHelper;
import org.hibernate.validator.internal.metadata.provider.PropertyHolderMetaDataProvider;
import org.hibernate.validator.internal.metadata.raw.propertyholder.PropertyHolderConfiguration;
import org.hibernate.validator.internal.properties.propertyholder.PropertyAccessorCreatorProvider;
import org.hibernate.validator.internal.util.TypeResolutionHelper;
import org.hibernate.validator.internal.util.stereotypes.Immutable;

Expand All @@ -45,15 +46,18 @@ public class PropertyHolderBeanMetaDataProvider {
*/
private final ValueExtractorManager valueExtractorManager;

private final PropertyAccessorCreatorProvider propertyAccessorCreatorProvider;

private final ValidationOrderGenerator validationOrderGenerator;

private final MetaDataCache<PropertyHolderMetadataKey> metaDataCache;

public PropertyHolderBeanMetaDataProvider(List<PropertyHolderMetaDataProvider> propertyHolderMetaDataProviderList, ConstraintHelper constraintHelper, TypeResolutionHelper typeResolutionHelper, ValueExtractorManager valueExtractorManager, ValidationOrderGenerator validationOrderGenerator) {
public PropertyHolderBeanMetaDataProvider(List<PropertyHolderMetaDataProvider> propertyHolderMetaDataProviderList, ConstraintHelper constraintHelper, TypeResolutionHelper typeResolutionHelper, ValueExtractorManager valueExtractorManager, PropertyAccessorCreatorProvider propertyAccessorCreatorProvider, ValidationOrderGenerator validationOrderGenerator) {
this.propertyHolderMetaDataProviderList = propertyHolderMetaDataProviderList;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we make the list immutable? Usually we do that where it is declared as immutable.

this.constraintHelper = constraintHelper;
this.typeResolutionHelper = typeResolutionHelper;
this.valueExtractorManager = valueExtractorManager;
this.propertyAccessorCreatorProvider = propertyAccessorCreatorProvider;
this.validationOrderGenerator = validationOrderGenerator;

this.metaDataCache = new MetaDataCache<>();
Expand All @@ -69,7 +73,7 @@ public <T> BeanMetaData<T> getBeanMetaData(Class<T> propertyHolderClass, String

private <T> BeanMetaDataImpl<T> createBeanMetaData(PropertyHolderMetadataKey metadataKey) {
PropertyHolderBeanMetaDataBuilder builder = PropertyHolderBeanMetaDataBuilder.getInstance(
constraintHelper, typeResolutionHelper, valueExtractorManager, validationOrderGenerator, metadataKey.propertyHolderClass
constraintHelper, typeResolutionHelper, valueExtractorManager, propertyAccessorCreatorProvider, validationOrderGenerator, metadataKey.propertyHolderClass
);

for ( PropertyHolderMetaDataProvider metaDataProvider : propertyHolderMetaDataProviderList ) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Hibernate Validator, declare and validate application constraints
*
* License: Apache License, Version 2.0
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
*/
package org.hibernate.validator.internal.metadata.raw.propertyholder;

import java.util.Collections;
import java.util.Iterator;
import java.util.Set;

import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.metadata.core.MetaConstraint;
import org.hibernate.validator.internal.metadata.raw.ConfigurationSource;
import org.hibernate.validator.internal.metadata.raw.ConstrainedElement;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.stereotypes.Immutable;

/**
* Base implementation of with functionality common to all {@link ConstrainedElement} implementations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's at least a word missing here.

*
* @author Gunnar Morling
* @author Hardy Ferentschik
* @author Marko Bekhta
*/
public abstract class ConstrainedPropertyHolderElement implements ConstrainedElement {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything specific to property holders here. Is this expected?

private final ConstrainedElementKind kind;
protected final ConfigurationSource source;
@Immutable
protected final Set<MetaConstraint<?>> constraints;
protected final CascadingMetaDataBuilder cascadingMetaDataBuilder;
@Immutable
protected final Set<MetaConstraint<?>> typeArgumentConstraints;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's separate them with a new line, it will be more readable.


public ConstrainedPropertyHolderElement(ConfigurationSource source,
ConstrainedElementKind kind,
Set<MetaConstraint<?>> constraints,
Set<MetaConstraint<?>> typeArgumentConstraints,
CascadingMetaDataBuilder cascadingMetaDataBuilder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not propagate this way of indenting arguments, it's not very readable.

this.kind = kind;
this.source = source;
this.constraints = constraints != null ? CollectionHelper.toImmutableSet( constraints ) : Collections.<MetaConstraint<?>>emptySet();
this.typeArgumentConstraints = typeArgumentConstraints != null ? CollectionHelper.toImmutableSet( typeArgumentConstraints ) : Collections.<MetaConstraint<?>>emptySet();
this.cascadingMetaDataBuilder = cascadingMetaDataBuilder;
}

@Override
public ConstrainedElementKind getKind() {
return kind;
}

@Override
public Iterator<MetaConstraint<?>> iterator() {
return constraints.iterator();
}

@Override
public Set<MetaConstraint<?>> getConstraints() {
return constraints;
}

@Override
public Set<MetaConstraint<?>> getTypeArgumentConstraints() {
return typeArgumentConstraints;
}

@Override
public CascadingMetaDataBuilder getCascadingMetaDataBuilder() {
return cascadingMetaDataBuilder;
}

@Override
public boolean isConstrained() {
return cascadingMetaDataBuilder.isMarkedForCascadingOnAnnotatedObjectOrContainerElements()
|| cascadingMetaDataBuilder.hasGroupConversionsOnAnnotatedObjectOrContainerElements()
|| !constraints.isEmpty()
|| !typeArgumentConstraints.isEmpty();
}

@Override
public ConfigurationSource getSource() {
return source;
}

@Override
public String toString() {
return "AbstractConstrainedElement [kind=" + kind + ", source="
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name is incorrect.

+ source + ", constraints="
+ constraints + ", cascadingMetaDataBuilder="
+ cascadingMetaDataBuilder + "]";
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ( ( source == null ) ? 0 : source.hashCode() );
return result;
}

@Override
public boolean equals(Object obj) {
if ( this == obj ) {
return true;
}
if ( obj == null ) {
return false;
}
if ( getClass() != obj.getClass() ) {
return false;
}
ConstrainedPropertyHolderElement other = (ConstrainedPropertyHolderElement) obj;
if ( source != other.source ) {
return false;
}
return true;
}
}
Loading