HV-1363 Reworked the property filtering logic

Author: marko-bekhta

cdi/src/main/java/org/hibernate/validator/cdi/ValidationExtension.java

@@ -259,7 +259,8 @@ private <T> void determineConstrainedMethods(AnnotatedType<T> type, BeanDescript
 		for ( AnnotatedMethod<? super T> annotatedMethod : type.getMethods() ) {
 			Method method = annotatedMethod.getJavaMember();
 
-			boolean isGetter = ReflectionHelper.isGetterMethod( method );
+			//TODO: need to update the getter logic here:
+			boolean isGetter = false/*ReflectionHelper.isGetterMethod( method )*/;
 
 			// obtain @ValidateOnExecution from the top-most method in the hierarchy
 			Method methodForExecutableTypeRetrieval = replaceWithOverriddenOrInterfaceMethod( method, overriddenAndImplementedMethods );

engine/src/main/java/org/hibernate/validator/internal/cfg/context/TypeConstraintMappingContextImpl.java

@@ -11,13 +11,11 @@
 
 import java.lang.annotation.ElementType;
 import java.lang.invoke.MethodHandles;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 
 import org.hibernate.validator.cfg.ConstraintDef;
@@ -35,20 +33,15 @@
 import org.hibernate.validator.internal.properties.Callable;
 import org.hibernate.validator.internal.properties.Constrainable;
 import org.hibernate.validator.internal.properties.Property;
-import org.hibernate.validator.internal.properties.javabean.JavaBeanExecutable;
-import org.hibernate.validator.internal.properties.javabean.JavaBeanField;
+import org.hibernate.validator.internal.properties.javabean.JavaBean;
 import org.hibernate.validator.internal.util.Contracts;
 import org.hibernate.validator.internal.util.ExecutableHelper;
 import org.hibernate.validator.internal.util.TypeResolutionHelper;
 import org.hibernate.validator.internal.util.logging.Log;
 import org.hibernate.validator.internal.util.logging.LoggerFactory;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredConstructor;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredField;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredMethod;
-import org.hibernate.validator.internal.util.privilegedactions.GetMethodFromPropertyName;
 import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
-import org.hibernate.validator.properties.GetterPropertyMatcher;
 import org.hibernate.validator.spi.group.DefaultGroupSequenceProvider;
+import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
 
 /**
  * Constraint mapping creational context which allows to configure the class-level constraints for one bean.
@@ -58,13 +51,15 @@
  * @author Hardy Ferentschik
  * @author Gunnar Morling
  * @author Kevin Pollet <kevin.pollet@serli.com> (C) 2011 SERLI
+ * @author Marko Bekhta
  */
 public final class TypeConstraintMappingContextImpl<C> extends ConstraintMappingContextImplBase
 		implements TypeConstraintMappingContext<C> {
 
 	private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
 
 	private final Class<C> beanClass;
+	private final JavaBean javaBean;
 
 	private final Set<ExecutableConstraintMappingContextImpl> executableContexts = newHashSet();
 	private final Set<PropertyConstraintMappingContextImpl> propertyContexts = newHashSet();
@@ -73,12 +68,10 @@ public final class TypeConstraintMappingContextImpl<C> extends ConstraintMapping
 	private List<Class<?>> defaultGroupSequence;
 	private Class<? extends DefaultGroupSequenceProvider<? super C>> defaultGroupSequenceProviderClass;
 
-	private final GetterPropertyMatcher getterPropertyMatcher;
-
 	TypeConstraintMappingContextImpl(DefaultConstraintMapping mapping, Class<C> beanClass, GetterPropertyMatcher getterPropertyMatcher) {
 		super( mapping );
 		this.beanClass = beanClass;
-		this.getterPropertyMatcher = getterPropertyMatcher;
+		this.javaBean = new JavaBean( getterPropertyMatcher, beanClass );
 		mapping.getAnnotationProcessingOptions().ignoreAnnotationConstraintForClass( beanClass, Boolean.FALSE );
 	}
 
@@ -123,24 +116,24 @@ public PropertyConstraintMappingContext property(String property, ElementType el
 		Contracts.assertNotNull( elementType, "The element type must not be null." );
 		Contracts.assertNotEmpty( property, MESSAGES.propertyNameMustNotBeEmpty() );
 
-		Property member = getProperty(
+		Optional<Property> member = getProperty(
 				beanClass, property, elementType
 		);
 
-		if ( member == null || member.getDeclaringClass() != beanClass ) {
+		if ( !member.isPresent() || member.get().getDeclaringClass() != beanClass ) {
 			throw LOG.getUnableToFindPropertyWithAccessException( beanClass, property, elementType );
 		}
-
-		if ( configuredMembers.contains( member ) ) {
+		Property constrainable = member.get();
+		if ( configuredMembers.contains( constrainable ) ) {
 			throw LOG.getPropertyHasAlreadyBeConfiguredViaProgrammaticApiException( beanClass, property );
 		}
 
 		PropertyConstraintMappingContextImpl context = new PropertyConstraintMappingContextImpl(
 				this,
-				member
+				constrainable
 		);
 
-		configuredMembers.add( member );
+		configuredMembers.add( constrainable );
 		propertyContexts.add( context );
 		return context;
 	}
@@ -149,14 +142,13 @@ public PropertyConstraintMappingContext property(String property, ElementType el
 	public MethodConstraintMappingContext method(String name, Class<?>... parameterTypes) {
 		Contracts.assertNotNull( name, MESSAGES.methodNameMustNotBeNull() );
 
-		Method method = run( GetDeclaredMethod.action( beanClass, name, parameterTypes ) );
+		Optional<Callable> method = javaBean.getCallableByNameAndParameters( name, parameterTypes );
 
-		if ( method == null || method.getDeclaringClass() != beanClass ) {
-			throw LOG.getBeanDoesNotContainMethodException( beanClass, name, parameterTypes );
+		if ( !method.isPresent() || method.get().getDeclaringClass() != beanClass ) {
+			throw LOG.getBeanDoesNotContainMethodException( javaBean, name, parameterTypes );
 		}
 
-		Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, method );
-
+		Callable callable = method.get();
 		if ( configuredMembers.contains( callable ) ) {
 			throw LOG.getMethodHasAlreadyBeenConfiguredViaProgrammaticApiException(
 					beanClass,
@@ -173,16 +165,16 @@ public MethodConstraintMappingContext method(String name, Class<?>... parameterT
 
 	@Override
 	public ConstructorConstraintMappingContext constructor(Class<?>... parameterTypes) {
-		Constructor<C> constructor = run( GetDeclaredConstructor.action( beanClass, parameterTypes ) );
+		Optional<Callable> constructor = javaBean.getConstructorByParameters( parameterTypes );
 
-		if ( constructor == null || constructor.getDeclaringClass() != beanClass ) {
+		if ( !constructor.isPresent() || constructor.get().getDeclaringClass() != beanClass ) {
 			throw LOG.getBeanDoesNotContainConstructorException(
-					beanClass,
+					javaBean,
 					parameterTypes
 			);
 		}
 
-		Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, constructor );
+		Callable callable = constructor.get();
 
 		if ( configuredMembers.contains( callable ) ) {
 			throw LOG.getConstructorHasAlreadyBeConfiguredViaProgrammaticApiException(
@@ -263,9 +255,9 @@ protected ConstraintType getConstraintType() {
 	 * @param property The property name without "is", "get" or "has". Cannot be {@code null} or empty.
 	 * @param elementType The element type. Either {@code ElementType.FIELD} or {@code ElementType METHOD}.
 	 *
-	 * @return the member which matching the name and type or {@code null} if no such member exists.
+	 * @return the member which matching the name and type or {@link Optional#empty()} if no such member exists.
 	 */
-	private Property getProperty(Class<?> clazz, String property, ElementType elementType) {
+	private Optional<Property> getProperty(Class<?> clazz, String property, ElementType elementType) {
 		Contracts.assertNotNull( clazz, MESSAGES.classCannotBeNull() );
 
 		if ( property == null || property.length() == 0 ) {
@@ -277,13 +269,10 @@ private Property getProperty(Class<?> clazz, String property, ElementType elemen
 		}
 
 		if ( ElementType.FIELD.equals( elementType ) ) {
-			Field field = run( GetDeclaredField.action( clazz, property ) );
-			return field == null ? null : new JavaBeanField( field );
+			return javaBean.getFieldPropertyByName( property );
 		}
 		else {
-			Method method = GetMethodFromPropertyName.action( clazz, getterPropertyMatcher, property ).run();
-
-			return method == null ? null : JavaBeanExecutable.of( getterPropertyMatcher, method ).as( Property.class );
+			return javaBean.getCallablePropertyByName( property );
 		}
 	}
 

engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java

@@ -60,7 +60,7 @@
 import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
 import org.hibernate.validator.internal.util.stereotypes.Immutable;
 import org.hibernate.validator.internal.util.stereotypes.ThreadSafe;
-import org.hibernate.validator.properties.GetterPropertyMatcher;
+import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
 import org.hibernate.validator.spi.cfg.ConstraintMappingContributor;
 import org.hibernate.validator.spi.scripting.ScriptEvaluatorFactory;
 

engine/src/main/java/org/hibernate/validator/internal/metadata/BeanMetaDataManager.java

@@ -37,7 +37,7 @@
 import org.hibernate.validator.internal.util.TypeResolutionHelper;
 import org.hibernate.validator.internal.util.classhierarchy.ClassHierarchyHelper;
 import org.hibernate.validator.internal.util.stereotypes.Immutable;
-import org.hibernate.validator.properties.GetterPropertyMatcher;
+import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
 
 /**
  * This manager is in charge of providing all constraint related meta data

engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java

@@ -24,7 +24,6 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.lang.reflect.Parameter;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
@@ -37,8 +36,10 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.validation.GroupSequence;
 import javax.validation.Valid;
@@ -66,6 +67,7 @@
 import org.hibernate.validator.internal.properties.Callable;
 import org.hibernate.validator.internal.properties.Constrainable;
 import org.hibernate.validator.internal.properties.Property;
+import org.hibernate.validator.internal.properties.javabean.JavaBean;
 import org.hibernate.validator.internal.properties.javabean.JavaBeanExecutable;
 import org.hibernate.validator.internal.properties.javabean.JavaBeanField;
 import org.hibernate.validator.internal.util.CollectionHelper;
@@ -74,12 +76,11 @@
 import org.hibernate.validator.internal.util.annotation.ConstraintAnnotationDescriptor;
 import org.hibernate.validator.internal.util.logging.Log;
 import org.hibernate.validator.internal.util.logging.LoggerFactory;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredConstructors;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredFields;
-import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredMethods;
+import org.hibernate.validator.internal.util.privilegedactions.GetJavaBeanExecutableProperty;
+import org.hibernate.validator.internal.util.privilegedactions.GetJavaBeanFieldProperty;
 import org.hibernate.validator.internal.util.privilegedactions.GetMethods;
 import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
-import org.hibernate.validator.properties.GetterPropertyMatcher;
+import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
 import org.hibernate.validator.spi.group.DefaultGroupSequenceProvider;
 
 /**
@@ -138,23 +139,33 @@ public <T> BeanConfiguration<T> getBeanConfiguration(Class<T> beanClass) {
 	 * @return Retrieves constraint related meta data from the annotations of the given type.
 	 */
 	private <T> BeanConfiguration<T> retrieveBeanConfiguration(Class<T> beanClass) {
-		Set<ConstrainedElement> constrainedElements = getFieldMetaData( beanClass );
-		constrainedElements.addAll( getMethodMetaData( beanClass ) );
-		constrainedElements.addAll( getConstructorMetaData( beanClass ) );
+		JavaBean javaBean = new JavaBean( getterPropertyMatcher, beanClass );
 
+		Stream<ConstrainedElement> classLevelStream;
 		//TODO GM: currently class level constraints are represented by a PropertyMetaData. This
 		//works but seems somewhat unnatural
 		Set<MetaConstraint<?>> classLevelConstraints = getClassLevelConstraints( beanClass );
 		if ( !classLevelConstraints.isEmpty() ) {
-			ConstrainedType classLevelMetaData =
+			classLevelStream = Stream.of(
 					new ConstrainedType(
 							ConfigurationSource.ANNOTATION,
 							beanClass,
 							classLevelConstraints
-					);
-			constrainedElements.add( classLevelMetaData );
+					)
+			);
+		}
+		else {
+			classLevelStream = Stream.empty();
 		}
 
+		Set<ConstrainedElement> constrainedElements = Stream.concat(
+				classLevelStream,
+				javaBean.getAllConstrainables()
+						.map( this::toConstrainedElement )
+						.filter( Optional::isPresent )
+						.map( Optional::get )
+		).collect( Collectors.toSet() );
+
 		return new BeanConfiguration<>(
 				ConfigurationSource.ANNOTATION,
 				beanClass,
@@ -164,6 +175,19 @@ private <T> BeanConfiguration<T> retrieveBeanConfiguration(Class<T> beanClass) {
 		);
 	}
 
+	private Optional<ConstrainedElement> toConstrainedElement(Constrainable constrainable) {
+		if ( constrainable instanceof JavaBeanField ) {
+			if ( annotationProcessingOptions.areMemberConstraintsIgnoredFor( constrainable ) ) {
+				return Optional.empty();
+			}
+			return Optional.of( findPropertyMetaData( constrainable.as( Property.class ) ) );
+		}
+		else if ( constrainable instanceof JavaBeanExecutable ) {
+			return Optional.of( findExecutableMetaData( constrainable.as( Callable.class ) ) );
+		}
+		throw new IllegalStateException( String.format( "Received unknown constrainable '%s' of type %s", constrainable.getName(), constrainable.getClass() ) );
+	}
+
 	private List<Class<?>> getDefaultGroupSequence(Class<?> beanClass) {
 		GroupSequence groupSequenceAnnotation = beanClass.getAnnotation( GroupSequence.class );
 		return groupSequenceAnnotation != null ? Arrays.asList( groupSequenceAnnotation.value() ) : null;
@@ -218,25 +242,8 @@ private Set<MetaConstraint<?>> getClassLevelConstraints(Class<?> clazz) {
 		return classLevelConstraints;
 	}
 
-	private Set<ConstrainedElement> getFieldMetaData(Class<?> beanClass) {
-		Set<ConstrainedElement> propertyMetaData = newHashSet();
-
-		for ( Field field : run( GetDeclaredFields.action( beanClass ) ) ) {
-			Property property = new JavaBeanField( field );
-			// HV-172
-			if ( Modifier.isStatic( field.getModifiers() ) ||
-					annotationProcessingOptions.areMemberConstraintsIgnoredFor( property ) ||
-					field.isSynthetic() ) {
-
-				continue;
-			}
-
-			propertyMetaData.add( findPropertyMetaData( field, property ) );
-		}
-		return propertyMetaData;
-	}
-
-	private ConstrainedProperty findPropertyMetaData(Field field, Property property) {
+	private ConstrainedProperty findPropertyMetaData(Property property) {
+		Field field = run( GetJavaBeanFieldProperty.action( property.as( JavaBeanField.class ) ) );
 		Set<MetaConstraint<?>> constraints = convertToMetaConstraints(
 				findConstraints( field, ElementType.FIELD, property ),
 				property
@@ -269,44 +276,17 @@ private Set<MetaConstraint<?>> convertToMetaConstraints(List<ConstraintDescripto
 		return constraints;
 	}
 
-	private Set<ConstrainedExecutable> getConstructorMetaData(Class<?> clazz) {
-		Executable[] declaredConstructors = run( GetDeclaredConstructors.action( clazz ) );
-
-		return getMetaData( declaredConstructors );
-	}
-
-	private Set<ConstrainedExecutable> getMethodMetaData(Class<?> clazz) {
-		Executable[] declaredMethods = run( GetDeclaredMethods.action( clazz ) );
-
-		return getMetaData( declaredMethods );
-	}
-
-	private Set<ConstrainedExecutable> getMetaData(Executable[] executableElements) {
-		Set<ConstrainedExecutable> executableMetaData = newHashSet();
-
-		for ( Executable executable : executableElements ) {
-			// HV-172; ignoring synthetic methods (inserted by the compiler), as they can't have any constraints
-			// anyway and possibly hide the actual method with the same signature in the built meta model
-			if ( Modifier.isStatic( executable.getModifiers() ) || executable.isSynthetic() ) {
-				continue;
-			}
-
-			executableMetaData.add( findExecutableMetaData( executable ) );
-		}
-
-		return executableMetaData;
-	}
-
 	/**
 	 * Finds all constraint annotations defined for the given method or constructor.
 	 *
-	 * @param executable The executable element to check for constraints annotations.
+	 * @param callable The executable element to check for constraints annotations.
 	 *
 	 * @return A meta data object describing the constraints specified for the
-	 * given element.
+	 * 		given element.
 	 */
-	private ConstrainedExecutable findExecutableMetaData(Executable executable) {
-		Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, executable );
+	private ConstrainedExecutable findExecutableMetaData(Callable callable) {
+		Executable executable = run( GetJavaBeanExecutableProperty.action( callable.as( JavaBeanExecutable.class ) ) );
+
 		List<ConstrainedParameter> parameterConstraints = getParameterMetaData( executable, callable );
 
 		Map<ConstraintType, List<ConstraintDescriptorImpl<?>>> executableConstraints = findConstraints(
@@ -834,7 +814,7 @@ private TypeArgumentExecutableParameterLocation(Executable executable, int index
 
 		@Override
 		public ConstraintLocation toConstraintLocation(GetterPropertyMatcher getterPropertyMatcher) {
-			return ConstraintLocation.forParameter( JavaBeanExecutable.of( getterPropertyMatcher, executable ), index );
+			return ConstraintLocation.forParameter( JavaBean.toJavaBeanExecutable( getterPropertyMatcher, executable ), index );
 		}
 	}
 
@@ -860,7 +840,7 @@ private TypeArgumentReturnValueLocation(Executable executable) {
 
 		@Override
 		public ConstraintLocation toConstraintLocation(GetterPropertyMatcher getterPropertyMatcher) {
-			return ConstraintLocation.forReturnValue( JavaBeanExecutable.of( getterPropertyMatcher, executable ) );
+			return ConstraintLocation.forReturnValue( JavaBean.toJavaBeanExecutable( getterPropertyMatcher, executable ) );
 		}
 	}