HHH-17729 move validation of constructors in HQL instantiations to SemanticQueryBuilder

also validate injection via fields/properties

hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java

@@ -1466,9 +1466,13 @@ public SqmDynamicInstantiation<?> visitInstantiation(HqlParser.InstantiationCont
 		}
 
 		if ( !dynamicInstantiation.checkInstantiation( creationContext.getTypeConfiguration() ) ) {
-			throw new SemanticException( "No matching constructor for type '"
+			final String typeName = dynamicInstantiation.getJavaType().getSimpleName();
-					+ dynamicInstantiation.getJavaType().getSimpleName() + "'",
+			if ( dynamicInstantiation.isFullyAliased() ) {
-					query );
+				throw new SemanticException( "Missing constructor or attributes for injection into type '" + typeName + "'", query );
+			}
+			else {
+				throw new SemanticException( "Missing constructor for type '" + typeName  + "'", query );
+			}
 		}
 
 		return dynamicInstantiation;

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmDynamicInstantiation.java

@@ -6,7 +6,6 @@
  */
 package org.hibernate.query.sqm.tree.select;
 
-import java.lang.reflect.Constructor;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -31,6 +30,7 @@
 import static org.hibernate.query.sqm.DynamicInstantiationNature.LIST;
 import static org.hibernate.query.sqm.DynamicInstantiationNature.MAP;
 import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.isConstructorCompatible;
+import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.isInjectionCompatible;
 
 /**
  * Represents a dynamic instantiation ({@code select new XYZ(...) ...}) as part of the SQM.
@@ -122,28 +122,35 @@ public boolean checkInstantiation(TypeConfiguration typeConfiguration) {
 				// where Class objects not available during build
 				return true;
 			}
-			if ( getArguments().stream().allMatch(arg -> arg.getAlias() != null ) ) {
+			final List<Class<?>> argTypes = argumentTypes();
-				// it's probably a bean injection-type instantiator, don't check it now
+			if ( isFullyAliased() ) {
-				return true;
+				final List<String> aliases =
+						getArguments().stream()
+								.map(SqmDynamicInstantiationArgument::getAlias)
+								.collect(toList());
+				return isInjectionCompatible( getJavaType(), aliases, argTypes )
+					|| isConstructorCompatible( getJavaType(), argTypes, typeConfiguration );
 			}
 			else {
-				final List<Class<?>> argTypes =
+				return isConstructorCompatible( getJavaType(), argTypes, typeConfiguration );
-						getArguments().stream()
-								.map(arg -> arg.getNodeJavaType().getJavaTypeClass())
-								.collect(toList());
-				for ( Constructor<?> constructor : getJavaType().getDeclaredConstructors() ) {
-					if ( isConstructorCompatible( constructor, argTypes, typeConfiguration ) ) {
-						return true;
-					}
-				}
-				return false;
 			}
 		}
 		else {
+			// TODO: is there anything we need to check for list/map instantiation?
 			return true;
 		}
 	}
 
+	private List<Class<?>> argumentTypes() {
+		return getArguments().stream()
+				.map(arg -> arg.getNodeJavaType().getJavaTypeClass())
+				.collect(toList());
+	}
+
+	public boolean isFullyAliased() {
+		return getArguments().stream().allMatch( arg -> arg.getAlias() != null );
+	}
+
 	@Override
 	public SqmDynamicInstantiation<T> copy(SqmCopyContext context) {
 		final SqmDynamicInstantiation<T> existing = context.getCopy( this );
@@ -323,9 +330,4 @@ public List<SqmSelectableNode<?>> getSelectionItems() {
 		visitSubSelectableNodes( list::add );
 		return list;
 	}
-
-	@Override
-	public boolean isCompoundSelection() {
-		return false;
-	}
 }

hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationAssemblerInjectionImpl.java

@@ -6,21 +6,25 @@
  */
 package org.hibernate.sql.results.graph.instantiation.internal;
 
+import java.beans.BeanInfo;
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.hibernate.internal.util.beans.BeanInfoHelper;
 import org.hibernate.query.sqm.sql.internal.InstantiationException;
-import org.hibernate.query.sqm.tree.expression.Compatibility;
 import org.hibernate.sql.results.graph.DomainResultAssembler;
 import org.hibernate.sql.results.jdbc.spi.JdbcValuesSourceProcessingOptions;
 import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
 import org.hibernate.type.descriptor.java.JavaType;
 
+import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.findField;
+import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.propertyMatches;
+
 /**
  * @author Steve Ebersole
  */
@@ -38,52 +42,7 @@ public DynamicInstantiationAssemblerInjectionImpl(
 				targetJavaType,
 				beanInfo -> {
 					for ( ArgumentReader<?> argumentReader : argumentReaders ) {
-						boolean found = false;
+						beanInjections.add( injection( beanInfo, argumentReader, targetJavaType ) );
-						for ( PropertyDescriptor propertyDescriptor : beanInfo.getPropertyDescriptors() ) {
-							if ( argumentReader.getAlias().equals( propertyDescriptor.getName() ) ) {
-								if ( propertyDescriptor.getWriteMethod() != null ) {
-									final boolean assignmentCompatible = Compatibility.areAssignmentCompatible(
-											propertyDescriptor.getWriteMethod().getParameterTypes()[0],
-											argumentReader.getAssembledJavaType().getClass()
-									);
-									if ( assignmentCompatible ) {
-										propertyDescriptor.getWriteMethod().setAccessible( true );
-										beanInjections.add(
-												new BeanInjection(
-														new BeanInjectorSetter<>( propertyDescriptor.getWriteMethod() ),
-														argumentReader
-												)
-										);
-										found = true;
-										break;
-									}
-								}
-							}
-						}
-						if ( found ) {
-							continue;
-						}
-
-						// see if we can find a Field with the given name...
-						final Field field = findField(
-								targetJavaType,
-								argumentReader.getAlias(),
-								argumentReader.getAssembledJavaType().getJavaTypeClass()
-						);
-						if ( field != null ) {
-							beanInjections.add(
-									new BeanInjection(
-											new BeanInjectorField<>( field ),
-											argumentReader
-									)
-							);
-						}
-						else {
-							throw new InstantiationException(
-									"Cannot set field '" + argumentReader.getAlias()
-											+ "' to instantiate '" + targetJavaType.getName() + "'"
-							);
-						}
 					}
 				}
 		);
@@ -93,22 +52,29 @@ public DynamicInstantiationAssemblerInjectionImpl(
 		}
 	}
 
-	private Field findField(Class<?> declaringClass, String name, Class<?> javaType) {
+	private static BeanInjection injection(BeanInfo beanInfo, ArgumentReader<?> argument, Class<?> targetJavaType) {
-		try {
+		final Class<?> argType = argument.getAssembledJavaType().getJavaTypeClass();
-			final Field field = declaringClass.getDeclaredField( name );
+		final String alias = argument.getAlias();
-			// field should never be null
+
-			if ( Compatibility.areAssignmentCompatible( field.getType(), javaType ) ) {
+		// see if we can find a property with the given name...
-				field.setAccessible( true );
+		for ( PropertyDescriptor propertyDescriptor : beanInfo.getPropertyDescriptors() ) {
-				return field;
+			if ( propertyMatches( alias, argType, propertyDescriptor ) ) {
-			}
+				final Method setter = propertyDescriptor.getWriteMethod();
-		}
+				setter.setAccessible(true);
-		catch (NoSuchFieldException ignore) {
+				return new BeanInjection( new BeanInjectorSetter<>( setter ), argument );
-			if ( declaringClass.getSuperclass() != null ) {
-				return findField( declaringClass.getSuperclass(), name, javaType );
 			}
 		}
 
-		return null;
+		// see if we can find a Field with the given name...
+		final Field field = findField( targetJavaType, alias, argType );
+		if ( field != null ) {
+			return new BeanInjection( new BeanInjectorField<>( field ), argument );
+		}
+		else {
+			throw new InstantiationException(
+					"Cannot set field '" + alias + "' to instantiate '" + targetJavaType.getName() + "'"
+			);
+		}
 	}
 
 	@Override
@@ -125,15 +91,14 @@ public T assemble(RowProcessingState rowProcessingState, JdbcValuesSourceProcess
 			constructor.setAccessible( true );
 			result = constructor.newInstance();
 		}
-		catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException | InstantiationException | java.lang.InstantiationException e) {
+		catch ( NoSuchMethodException | IllegalAccessException | InvocationTargetException | InstantiationException
+			   | java.lang.InstantiationException e ) {
 			throw new InstantiationException( "Error instantiating class '"
 					+ target.getJavaType().getTypeName() + "' using default constructor: " + e.getMessage(), e );
 		}
 		for ( BeanInjection beanInjection : beanInjections ) {
-			beanInjection.getBeanInjector().inject(
+			final Object assembled = beanInjection.getValueAssembler().assemble( rowProcessingState, options );
-					result,
+			beanInjection.getBeanInjector().inject( result, assembled );
-					beanInjection.getValueAssembler().assemble( rowProcessingState, options )
-			);
 		}
 		return result;
 	}

hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/InstantiationHelper.java

@@ -6,10 +6,14 @@
  */
 package org.hibernate.sql.results.graph.instantiation.internal;
 
+import org.hibernate.internal.util.beans.BeanInfoHelper;
 import org.hibernate.type.spi.TypeConfiguration;
 import org.jboss.logging.Logger;
 
+import java.beans.BeanInfo;
+import java.beans.PropertyDescriptor;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
 import java.lang.reflect.Type;
 import java.util.List;
 
@@ -27,6 +31,40 @@ private InstantiationHelper() {
 		// disallow direct instantiation
 	}
 
+	public static boolean isInjectionCompatible(Class<?> targetJavaType, List<String> aliases, List<Class<?>> argTypes) {
+		return BeanInfoHelper.visitBeanInfo(
+				targetJavaType,
+				beanInfo -> {
+                    for ( int i = 0; i < aliases.size(); i++ ) {
+                        final String alias = aliases.get(i);
+						final Class<?> argType = argTypes.get(i);
+						if ( !checkArgument( targetJavaType, beanInfo, alias, argType ) ) {
+							return false;
+						}
+					}
+					return true;
+				}
+		);
+	}
+
+	private static boolean checkArgument(Class<?> targetJavaType, BeanInfo beanInfo, String alias, Class<?> argType) {
+		for ( PropertyDescriptor propertyDescriptor : beanInfo.getPropertyDescriptors() ) {
+			if ( propertyMatches( alias, argType, propertyDescriptor ) ) {
+				return true;
+			}
+		}
+		return findField(targetJavaType, alias, argType) != null;
+	}
+
+	public static boolean isConstructorCompatible(Class<?> javaClass, List<Class<?>> argTypes, TypeConfiguration typeConfiguration) {
+		for ( Constructor<?> constructor : javaClass.getDeclaredConstructors() ) {
+			if ( isConstructorCompatible( constructor, argTypes, typeConfiguration) ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	public static boolean isConstructorCompatible(
 			Constructor<?> constructor,
 			List<Class<?>> argumentTypes,
@@ -58,4 +96,28 @@ public static boolean isConstructorCompatible(
 			return false;
 		}
 	}
+
+	static Field findField(Class<?> declaringClass, String name, Class<?> javaType) {
+		try {
+			final Field field = declaringClass.getDeclaredField( name );
+			// field should never be null
+			if ( areAssignmentCompatible( field.getType(), javaType ) ) {
+				field.setAccessible( true );
+				return field;
+			}
+		}
+		catch (NoSuchFieldException ignore) {
+			if ( declaringClass.getSuperclass() != null ) {
+				return findField( declaringClass.getSuperclass(), name, javaType );
+			}
+		}
+
+		return null;
+	}
+
+	static boolean propertyMatches(String alias, Class<?> argType, PropertyDescriptor propertyDescriptor) {
+		return alias.equals(propertyDescriptor.getName())
+			&& propertyDescriptor.getWriteMethod() != null
+			&& areAssignmentCompatible( propertyDescriptor.getWriteMethod().getParameterTypes()[0], argType );
+	}
 }