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
@@ -0,0 +1,113 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Commit message: simple a implementation for Map. I think the a is misplaced.

* 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.properties.propertyholder;

import java.lang.invoke.MethodHandles;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import org.hibernate.validator.internal.properties.propertyholder.map.MapPropertyAccessorCreator;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.TypeHelper;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;
import org.hibernate.validator.internal.util.privilegedactions.GetClassLoader;
import org.hibernate.validator.internal.util.privilegedactions.GetInstancesFromServiceLoader;
import org.hibernate.validator.spi.propertyholder.PropertyAccessorCreator;

/**
* @author Marko Bekhta
*/
public class PropertyAccessorCreatorProvider {

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

private final Set<PropertyAccessorCreator<?>> configuredPropertyCreators = new HashSet<>();

public PropertyAccessorCreatorProvider() {
//add default property creator for a Map
configuredPropertyCreators.add( new MapPropertyAccessorCreator() );

List<PropertyAccessorCreator> propertyAccessorCreators = run( GetInstancesFromServiceLoader.action(
run( GetClassLoader.fromContext() ),
PropertyAccessorCreator.class
) );
for ( PropertyAccessorCreator propertyAccessorCreator : propertyAccessorCreators ) {
configuredPropertyCreators.add( propertyAccessorCreator );
}
Copy link
Member

Choose a reason for hiding this comment

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

We should make the set Immutable if we expect it to be.

}

@SuppressWarnings("unchecked")
public <T> PropertyAccessorCreator<T> getPropertyAccessorCreatorFor(Class<T> propertyHolderType) {

Set<PropertyAccessorCreator> possibleCreators = configuredPropertyCreators
.stream()
.filter( el -> TypeHelper.isAssignable( el.getPropertyHolderType(), propertyHolderType ) )
.collect( Collectors.toSet() );

Set<PropertyAccessorCreator> creators = getMaximallySpecificPropertyAccessorCreators( possibleCreators );

if ( creators.isEmpty() ) {
throw LOG.getUnableToFindPropertyCreatorException( propertyHolderType );
}
else if ( creators.size() > 1 ) {
throw LOG.getUnableToFinUniquedPropertyCreatorException( propertyHolderType );
Copy link
Member

Choose a reason for hiding this comment

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

ToFinUniquedProperty -> ToFindUniquedProperty

}
else {
return creators.iterator().next();
}
}

private Set<PropertyAccessorCreator> getMaximallySpecificPropertyAccessorCreators(Set<PropertyAccessorCreator> possiblePropertyAccessorCreators) {
Set<PropertyAccessorCreator> propertyAccessorCreators = CollectionHelper.newHashSet( possiblePropertyAccessorCreators.size() );
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 have a few short circuit condition if we have 0 or 1 as input?


for ( PropertyAccessorCreator creator : possiblePropertyAccessorCreators ) {
if ( propertyAccessorCreators.isEmpty() ) {
propertyAccessorCreators.add( creator );
continue;
}
Iterator<PropertyAccessorCreator> candidatesIterator = propertyAccessorCreators.iterator();
boolean isNewRoot = true;
while ( candidatesIterator.hasNext() ) {
PropertyAccessorCreator candidate = candidatesIterator.next();

// we consider the strictly more specific value extractor so 2 value extractors for the same container
// type should throw an error in the end if no other more specific value extractor is found.
if ( candidate.getPropertyHolderType().equals( creator.getPropertyHolderType() ) ) {
continue;
}

if ( TypeHelper.isAssignable( candidate.getPropertyHolderType(), creator.getPropertyHolderType() ) ) {
candidatesIterator.remove();
}
else if ( TypeHelper.isAssignable( creator.getPropertyHolderType(), candidate.getPropertyHolderType() ) ) {
isNewRoot = false;
}
}
if ( isNewRoot ) {
propertyAccessorCreators.add( creator );
}
}
return propertyAccessorCreators;
}

/**
* Runs the given privileged action, using a privileged block if required.
*
* <b>NOTE:</b> This must never be changed into a publicly available method to avoid execution of arbitrary
* privileged actions within HV's protection domain.
*/
private <T> T run(PrivilegedAction<T> action) {
return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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.properties.propertyholder;

import java.lang.reflect.Type;

import org.hibernate.validator.internal.metadata.raw.ConstrainedElement.ConstrainedElementKind;
import org.hibernate.validator.internal.properties.Property;
import org.hibernate.validator.internal.properties.PropertyAccessor;

/**
* @author Marko Bekhta
*/
public class PropertyHolderProperty implements Property {
Copy link
Member Author

Choose a reason for hiding this comment

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

This property holder property can be used for any kind property holder. It should work just fine with Maps JsonObjects and anything else. All the accessing logic is provided by a property accessor defined by PropertyAccessorCreator interface. PropertyAccessorCreatorProvider gives a creator based on a property holder type. This allows to store the user entered metadata once and then create "real" validation metadata based on a property holder type.


private final Class<?> propertyHolderType;
private final PropertyAccessor propertyAccessor;

private final String name;
private final Class<?> type;

public PropertyHolderProperty(Class<?> propertyHolderType, PropertyAccessor propertyAccessor, String name, Class<?> type) {
this.propertyHolderType = propertyHolderType;
this.propertyAccessor = propertyAccessor;
this.name = name;
this.type = type;
}

@Override
public String getPropertyName() {
return getName();
}

@Override
public PropertyAccessor createAccessor() {
return propertyAccessor;
}

@Override
public String getName() {
return name;
}

@Override
public Class<?> getDeclaringClass() {
return propertyHolderType;
}

@Override
public Type getTypeForValidatorResolution() {
return getType();
}

@Override
public Type getType() {
return type;
}

@Override
public ConstrainedElementKind getConstrainedElementKind() {
return ConstrainedElementKind.FIELD;
}

@Override
public boolean equals(Object o) {
if ( this == o ) {
return true;
}
if ( o == null || getClass() != o.getClass() ) {
return false;
}

PropertyHolderProperty that = (PropertyHolderProperty) o;

if ( !propertyHolderType.equals( that.propertyHolderType ) ) {
return false;
}
if ( !name.equals( that.name ) ) {
return false;
}
if ( !type.equals( that.type ) ) {
return false;
}

return true;
}

@Override
public int hashCode() {
int result = propertyHolderType.hashCode();
result = 31 * result + name.hashCode();
result = 31 * result + type.hashCode();
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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.properties.propertyholder.map;

import java.lang.invoke.MethodHandles;
import java.lang.reflect.Type;
import java.util.Map;

import org.hibernate.validator.internal.properties.PropertyAccessor;
import org.hibernate.validator.internal.util.TypeHelper;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;
import org.hibernate.validator.spi.propertyholder.PropertyAccessorCreator;

/**
* @author Marko Bekhta
*/
public class MapPropertyAccessorCreator implements PropertyAccessorCreator<Map> {
Copy link
Member

Choose a reason for hiding this comment

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

If we're gonna use the raw type, let's add the proper @SuppressWarnings


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

@Override
public Class<Map> getPropertyHolderType() {
return Map.class;
}

@Override
public PropertyAccessor create(String propertyName, Type propertyType) {
return new MapPropertyAccessor( propertyName, propertyType );
}

private static class MapPropertyAccessor implements PropertyAccessor {

private final String name;
private final Type type;

private MapPropertyAccessor(String name, Type type) {
this.name = name;
this.type = type;
}

@Override
public Object getValueFrom(Object bean) {
if ( !( bean instanceof Map ) ) {
throw LOG.getUnexpextedPropertyHolderTypeException( Map.class, bean.getClass() );
}
Object value = ( (Map) bean ).get( name );
if ( value != null && !TypeHelper.isAssignable( type, value.getClass() ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we need this complicated thing TypeHelper.isAssignable? Or if we could simply check the class?

Do we expect the type to not be a class?

throw LOG.getUnexpextedPropertyTypeInPropertyHolderException( type, value.getClass(), name );
}
return value;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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>.
*/
/**
* Contains default {@link org.hibernate.validator.spi.propertyholder.PropertyAccessorCreator} implementaion for {@link java.util.Map},
* and related classes.
*/
package org.hibernate.validator.internal.properties.propertyholder.map;
Original file line number Diff line number Diff line change
@@ -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 <http://www.apache.org/licenses/LICENSE-2.0>.
*/
/**
* Contains {@link org.hibernate.validator.internal.properties.propertyholder.PropertyAccessorCreatorProvider}
* and all related classes, as well as default {@link org.hibernate.validator.spi.propertyholder.PropertyAccessorCreator}
* implementaion for {@link java.util.Map}.
*/
package org.hibernate.validator.internal.properties.propertyholder;
Original file line number Diff line number Diff line change
Expand Up @@ -875,4 +875,16 @@ ConstraintDefinitionException getConstraintValidatorDefinitionConstraintMismatch

@Message(id = 248, value = "Unable to get an XML schema named %s.")
ValidationException unableToGetXmlSchema(String schemaResourceName);

@Message(id = 249, value = "Unable to find property creator for property holder of %1$s type.")
Copy link
Member

Choose a reason for hiding this comment

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

of type X

ValidationException getUnableToFindPropertyCreatorException(@FormatWith(ClassObjectFormatter.class) Class<?> clazz);

@Message(id = 250, value = "Unable to find single unique property creator for property holder of %1$s type.")
Copy link
Member

Choose a reason for hiding this comment

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

to find a single
of type X

ValidationException getUnableToFinUniquedPropertyCreatorException(@FormatWith(ClassObjectFormatter.class) Class<?> clazz);

@Message(id = 251, value = "Unexpected property holder type received. Expected %1$s type, but instead got %2$s.")
Copy link
Member

Choose a reason for hiding this comment

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

Expected type X

ValidationException getUnexpextedPropertyHolderTypeException(@FormatWith(ClassObjectFormatter.class) Class<?> expecetedHolderType, @FormatWith(ClassObjectFormatter.class) Class<?> realHolderType);

@Message(id = 252, value = "Unexpected property type in a given property holder. Expected that '%3$s' will be of %1$s type, but instead it is %2$s.")
Copy link
Member

Choose a reason for hiding this comment

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

of type X but instead it is of type Y

ValidationException getUnexpextedPropertyTypeInPropertyHolderException(Type expecetedType, @FormatWith(ClassObjectFormatter.class) Class<?> realType, String propertyName);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.spi.propertyholder;

import java.lang.reflect.Type;

import org.hibernate.validator.internal.properties.PropertyAccessor;

/**
* This interface expose the functionality of creating the property accessor based on
Copy link
Member

Choose a reason for hiding this comment

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

exposes

* a string representation of a property.
*
* @author Marko Bekhta
*/
public interface PropertyAccessorCreator<T> {

/**
* @return the type of the property holder this creator can be applied to.
*/
Class<T> getPropertyHolderType();

/**
* Creates property accessor for a given name and type.
*
* @param propertyName property name
* @param propertyType property type
*
* @return created property
*/
PropertyAccessor create(String propertyName, Type propertyType);
Copy link
Member

Choose a reason for hiding this comment

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

Will we really support the full range of Type? Maybe we should make it a Class and accept the limitations of it? Or does it cause an issue with nested structures?

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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>.
*/

/**
* <p>
Copy link
Member

Choose a reason for hiding this comment

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

No <p> at the beginning.

* This package provides support for customization of the property holder validation by
* exposing a set of interfaces that helps creating properties for custom property holders.
* </p>
Copy link
Member

Choose a reason for hiding this comment

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

Here you just need another <p>. The javadoc tool complains when encountering </p> (I know there are a few in our doc but let's not add more).

* <p>This package is part of the public Hibernate Validator API.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Do we usually say our SPIs are part of our public APIs? Honest question :).

*/
package org.hibernate.validator.spi.propertyholder;