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 @@ -54,7 +54,7 @@
import org.hibernate.validator.internal.metadata.manager.ConstraintMetaDataManager;
import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData;
import org.hibernate.validator.internal.metadata.aggregated.ParameterMetaData;
import org.hibernate.validator.internal.metadata.aggregated.PropertyMetaData;
Expand Down Expand Up @@ -603,7 +603,7 @@ private void validateCascadedAnnotatedObjectForCurrentGroup(Object value, BaseBe
}

private void validateCascadedContainerElementsForCurrentGroup(Object value, BaseBeanValidationContext<?> validationContext, ValueContext<?, ?> valueContext,
List<ContainerCascadingMetaData> containerElementTypesCascadingMetaData) {
List<? extends ContainerCascadingMetaData> containerElementTypesCascadingMetaData) {
for ( ContainerCascadingMetaData cascadingMetaData : containerElementTypesCascadingMetaData ) {
if ( !cascadingMetaData.isMarkedForCascadingOnAnnotatedObjectOrContainerElements() ) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import javax.validation.valueextraction.ValueExtractor;

import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.metadata.aggregated.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.PotentiallyContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.PotentiallyContainerCascadingMetaData;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.Contracts;
import org.hibernate.validator.internal.util.ReflectionHelper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import javax.validation.metadata.ContainerElementTypeDescriptor;
import javax.validation.metadata.GroupConversionDescriptor;

import org.hibernate.validator.internal.metadata.aggregated.cascading.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.core.MetaConstraint;
import org.hibernate.validator.internal.metadata.descriptor.ConstraintDescriptorImpl;
import org.hibernate.validator.internal.metadata.descriptor.ContainerElementTypeDescriptorImpl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject;
import org.hibernate.validator.internal.engine.valueextraction.ArrayElement;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.cascading.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.NonContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.manager.ConstraintMetaDataManager;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorHelper;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.cascading.ContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.NonContainerCascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.cascading.PotentiallyContainerCascadingMetaData;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.ReflectionHelper;
import org.hibernate.validator.internal.util.StringHelper;
Expand All @@ -47,6 +50,8 @@ public class CascadingMetaDataBuilder {
private static final CascadingMetaDataBuilder NON_CASCADING =
new CascadingMetaDataBuilder( null, null, null, null, false, Collections.emptyMap(), Collections.emptyMap() );

private final String mapping;
Copy link
Member Author

Choose a reason for hiding this comment

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

For now property holders and regular beans are using the same cascading metadata builder. When I've tried to break it into smaller reusable pieces, I've broke it so badly 😄 that I decided to revert the changes for now and just use the same builder. But this still is needed to be done.

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's try to have a todo list somewhere so that we don't forget about it.


/**
* The enclosing type that defines this type parameter.
*/
Expand Down Expand Up @@ -122,6 +127,8 @@ private CascadingMetaDataBuilder(Type enclosingType, TypeVariable<?> typeParamet
}
hasContainerElementsMarkedForCascading = tmpHasContainerElementsMarkedForCascading;
hasGroupConversionsOnAnnotatedObjectOrContainerElements = tmpHasGroupConversionsOnAnnotatedObjectOrContainerElements;

mapping = null;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you fix it in a later commit but if not, let's add a // FIXME or something so that we can track it.

}

public static CascadingMetaDataBuilder nonCascading() {
Expand All @@ -140,6 +147,10 @@ public Type getEnclosingType() {
return enclosingType;
}

public String getMappingName() {
return mapping;
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 be consistent between the method name and the property and call the property mappingName.

}
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it a bit more, I wonder if a string is such a good idea. I wonder if we shouldn't have a proper wrapper here which would contain the necessary information for beans or property holders.

Not sure only the mapping makes sense in this case, maybe the class should be part of it too so that we have something for the beans too.

Thinking out loud here, not sure if it totally makes sense.


public Class<?> getDeclaredContainerClass() {
return declaredContainerClass;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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.aggregated;
package org.hibernate.validator.internal.metadata.aggregated.cascading;

import java.lang.invoke.MethodHandles;
import java.lang.reflect.Type;
Expand All @@ -19,6 +19,8 @@
import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.StringHelper;
import org.hibernate.validator.internal.util.TypeVariables;
Expand Down Expand Up @@ -66,7 +68,7 @@ public class ContainerCascadingMetaData implements CascadingMetaData {
* Possibly the cascading type parameters corresponding to this type parameter if it is a parameterized type.
*/
@Immutable
private final List<ContainerCascadingMetaData> containerElementTypesCascadingMetaData;
private final List<? extends ContainerCascadingMetaData> containerElementTypesCascadingMetaData;

/**
* If this type parameter is marked for cascading.
Expand All @@ -92,10 +94,13 @@ public class ContainerCascadingMetaData implements CascadingMetaData {

public static ContainerCascadingMetaData of(ValueExtractorManager valueExtractorManager, CascadingMetaDataBuilder cascadingMetaDataBuilder,
Object context) {
if ( cascadingMetaDataBuilder.getMappingName() != null ) {
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, in the end, we really need this to be part of the builder once they are split.

return ContainerPropertyHolderCascadingMetaData.of( valueExtractorManager, cascadingMetaDataBuilder, context );
}
return new ContainerCascadingMetaData( valueExtractorManager, cascadingMetaDataBuilder );
}

private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager, CascadingMetaDataBuilder cascadingMetaDataBuilder) {
protected ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager, CascadingMetaDataBuilder cascadingMetaDataBuilder) {
this(
valueExtractorManager,
cascadingMetaDataBuilder.getEnclosingType(),
Expand Down Expand Up @@ -138,7 +143,7 @@ private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager,
}
}

ContainerCascadingMetaData(Type enclosingType, List<ContainerCascadingMetaData> containerElementTypesCascadingMetaData,
ContainerCascadingMetaData(Type enclosingType, List<? extends ContainerCascadingMetaData> containerElementTypesCascadingMetaData,
GroupConversionHelper groupConversionHelper, Set<ValueExtractorDescriptor> valueExtractorCandidates) {
this.enclosingType = enclosingType;
this.typeParameter = AnnotatedObject.INSTANCE;
Expand Down Expand Up @@ -206,7 +211,7 @@ public boolean isMarkedForCascadingOnAnnotatedObjectOrContainerElements() {
return cascading || hasContainerElementsMarkedForCascading;
}

public List<ContainerCascadingMetaData> getContainerElementTypesCascadingMetaData() {
public List<? extends ContainerCascadingMetaData> getContainerElementTypesCascadingMetaData() {
return containerElementTypesCascadingMetaData;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* 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.aggregated.cascading;

import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.List;
import java.util.Set;

import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.metadata.manager.ConstraintMetaDataManager;
import org.hibernate.validator.internal.util.Contracts;
import org.hibernate.validator.internal.util.StringHelper;

/**
* Extended view of container cascading metadta for property holders that in addition stores mapping name.
Copy link
Member

Choose a reason for hiding this comment

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

s/metadta/metadata/

*
* @author Marko Bekhta
*/
public class ContainerPropertyHolderCascadingMetaData extends ContainerCascadingMetaData {

/**
* Name of the constraint mappings to be applied.
*/
private final String mapping;
Copy link
Member

Choose a reason for hiding this comment

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

Same remark here about the name.


public static ContainerPropertyHolderCascadingMetaData of(ValueExtractorManager valueExtractorManager, CascadingMetaDataBuilder cascadingMetaDataBuilder,
Object context) {
Contracts.assertNotEmpty( cascadingMetaDataBuilder.getMappingName(), "Property holder mapping cannot be an empty string." );

return new ContainerPropertyHolderCascadingMetaData( valueExtractorManager, cascadingMetaDataBuilder );
}

private ContainerPropertyHolderCascadingMetaData(ValueExtractorManager valueExtractorManager, CascadingMetaDataBuilder cascadingMetaDataBuilder) {
super( valueExtractorManager, cascadingMetaDataBuilder );
this.mapping = cascadingMetaDataBuilder.getMappingName();
}

ContainerPropertyHolderCascadingMetaData(String mapping, Type enclosingType, List<ContainerPropertyHolderCascadingMetaData> containerElementTypesCascadingMetaData,
GroupConversionHelper groupConversionHelper, Set<ValueExtractorDescriptor> valueExtractorCandidates) {
super( enclosingType, containerElementTypesCascadingMetaData, groupConversionHelper, valueExtractorCandidates );
this.mapping = mapping;
}

ContainerPropertyHolderCascadingMetaData(String mapping, Type enclosingType, TypeVariable<?> typeParameter, Class<?> declaredContainerClass, TypeVariable<?> declaredTypeParameter,
GroupConversionHelper groupConversionHelper) {
super( enclosingType, typeParameter, declaredContainerClass, declaredTypeParameter, groupConversionHelper );
this.mapping = mapping;
}

@Override
public BeanMetaData<?> getBeanMetaDataForCascadable(ConstraintMetaDataManager constraintMetaDataManager, Object value) {
return constraintMetaDataManager.getPropertyHolderBeanMetaData( value.getClass(), mapping );
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append( getClass().getSimpleName() );
sb.append( " [" );
sb.append( "mapping=" ).append( mapping ).append( ", " );
sb.append( "enclosingType=" ).append( StringHelper.toShortString( getEnclosingType() ) ).append( ", " );
sb.append( "typeParameter=" ).append( getTypeParameter() ).append( ", " );
sb.append( "cascading=" ).append( isCascading() ).append( ", " );
sb.append( "containerElementTypesCascadingMetaData=" ).append( getContainerElementTypesCascadingMetaData() );
sb.append( "]" );
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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.aggregated;
package org.hibernate.validator.internal.metadata.aggregated.cascading;

import static org.hibernate.validator.internal.util.CollectionHelper.newHashSet;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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.aggregated;
package org.hibernate.validator.internal.metadata.aggregated.cascading;

import java.lang.invoke.MethodHandles;
import java.lang.reflect.TypeVariable;
Expand All @@ -14,6 +14,8 @@

import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;

Expand Down Expand Up @@ -47,7 +49,10 @@ public class NonContainerCascadingMetaData implements CascadingMetaData {
private GroupConversionHelper groupConversionHelper;

public static NonContainerCascadingMetaData of(CascadingMetaDataBuilder cascadingMetaDataBuilder, Object context) {
if ( !cascadingMetaDataBuilder.isCascading() ) {
if ( cascadingMetaDataBuilder.getMappingName() != null ) {
return NonContainerPropertyHolderCascadingMetaData.of( cascadingMetaDataBuilder, context );
}
else if ( !cascadingMetaDataBuilder.isCascading() ) {
return NON_CASCADING;
}
else if ( cascadingMetaDataBuilder.getGroupConversions().isEmpty() ) {
Expand All @@ -58,14 +63,14 @@ else if ( cascadingMetaDataBuilder.getGroupConversions().isEmpty() ) {
}
}

private NonContainerCascadingMetaData(CascadingMetaDataBuilder cascadingMetaDataBuilder) {
protected NonContainerCascadingMetaData(CascadingMetaDataBuilder cascadingMetaDataBuilder) {
this(
cascadingMetaDataBuilder.isCascading(),
GroupConversionHelper.of( cascadingMetaDataBuilder.getGroupConversions() )
);
}

private NonContainerCascadingMetaData(boolean cascading, GroupConversionHelper groupConversionHelper) {
protected NonContainerCascadingMetaData(boolean cascading, GroupConversionHelper groupConversionHelper) {
this.cascading = cascading;
this.groupConversionHelper = groupConversionHelper;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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.aggregated.cascading;

import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.metadata.manager.ConstraintMetaDataManager;
import org.hibernate.validator.internal.util.Contracts;

/**
* Extended view of non container cascading metadta for property holders that in addition stores mapping name.
*
* @author Marko Bekhta
*/
public class NonContainerPropertyHolderCascadingMetaData extends NonContainerCascadingMetaData {

/**
* Name of the constraint mappings to be applied.
*/
private final String mapping;

public static NonContainerPropertyHolderCascadingMetaData of(CascadingMetaDataBuilder cascadingMetaDataBuilder, Object context) {
Contracts.assertTrue( cascadingMetaDataBuilder.isCascading(), "Property holder cascading metadata should always be cascading." );
Contracts.assertNotEmpty( cascadingMetaDataBuilder.getMappingName(), "Property holder mapping cannot be an empty string." );

return new NonContainerPropertyHolderCascadingMetaData( cascadingMetaDataBuilder );
}

private NonContainerPropertyHolderCascadingMetaData(CascadingMetaDataBuilder cascadingMetaDataBuilder) {
super( cascadingMetaDataBuilder );
this.mapping = cascadingMetaDataBuilder.getMappingName();
}

@Override
public BeanMetaData<?> getBeanMetaDataForCascadable(ConstraintMetaDataManager constraintMetaDataManager, Object value) {
return constraintMetaDataManager.getPropertyHolderBeanMetaData( value.getClass(), mapping );
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append( getClass().getSimpleName() );
sb.append( " [" );
sb.append( "mapping=" ).append( mapping ).append( ", " );
sb.append( "cascading=" ).append( isCascading() ).append( ", " );
sb.append( "]" );
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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.aggregated;
package org.hibernate.validator.internal.metadata.aggregated.cascading;

import java.lang.invoke.MethodHandles;
import java.lang.reflect.TypeVariable;
Expand All @@ -16,6 +16,8 @@
import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor;
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorManager;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaDataBuilder;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;

Expand All @@ -38,10 +40,13 @@ public class PotentiallyContainerCascadingMetaData implements CascadingMetaData
private final Set<ValueExtractorDescriptor> potentialValueExtractorDescriptors;

public static PotentiallyContainerCascadingMetaData of(CascadingMetaDataBuilder cascadingMetaDataBuilder, Set<ValueExtractorDescriptor> potentialValueExtractorDescriptors, Object context) {
if ( cascadingMetaDataBuilder.getMappingName() != null ) {
return PotentiallyContainerPropertyHolderCascadingMetaData.of( cascadingMetaDataBuilder, potentialValueExtractorDescriptors, context );
}
return new PotentiallyContainerCascadingMetaData( cascadingMetaDataBuilder, potentialValueExtractorDescriptors );
}

private PotentiallyContainerCascadingMetaData(CascadingMetaDataBuilder cascadingMetaDataBuilder, Set<ValueExtractorDescriptor> potentialValueExtractorDescriptors) {
protected PotentiallyContainerCascadingMetaData(CascadingMetaDataBuilder cascadingMetaDataBuilder, Set<ValueExtractorDescriptor> potentialValueExtractorDescriptors) {
this( potentialValueExtractorDescriptors, GroupConversionHelper.of( cascadingMetaDataBuilder.getGroupConversions() ) );
}

Expand Down Expand Up @@ -91,19 +96,23 @@ public CascadingMetaData addRuntimeContainerSupport(ValueExtractorManager valueE
return new ContainerCascadingMetaData(
valueClass,
Collections.singletonList(
new ContainerCascadingMetaData(
compliantValueExtractor.getContainerType(),
compliantValueExtractor.getExtractedTypeParameter(),
compliantValueExtractor.getContainerType(),
compliantValueExtractor.getExtractedTypeParameter(),
groupConversionHelper.isEmpty() ? GroupConversionHelper.EMPTY : groupConversionHelper
)
createInnerMetadata( compliantValueExtractor, groupConversionHelper )
),
groupConversionHelper,
Collections.singleton( compliantValueExtractor )
);
}

protected ContainerCascadingMetaData createInnerMetadata(ValueExtractorDescriptor compliantValueExtractor, GroupConversionHelper groupConversionHelper) {
return new ContainerCascadingMetaData(
compliantValueExtractor.getContainerType(),
compliantValueExtractor.getExtractedTypeParameter(),
compliantValueExtractor.getContainerType(),
compliantValueExtractor.getExtractedTypeParameter(),
groupConversionHelper.isEmpty() ? GroupConversionHelper.EMPTY : groupConversionHelper
);
}

@Override
@SuppressWarnings("unchecked")
public <T extends CascadingMetaData> T as(Class<T> clazz) {
Expand Down
Loading