HHH-19383 - validation of NativeQuery result mappings

Signed-off-by: Jan Schatteman <[email protected]>

Author: jrenaat

hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java

@@ -5,6 +5,7 @@
 package org.hibernate.query.sql.internal;
 
 import java.io.Serializable;
+import java.lang.reflect.Constructor;
 import java.time.Instant;
 import java.util.Calendar;
 import java.util.Collection;
@@ -125,6 +126,7 @@
 import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty;
 import static org.hibernate.internal.util.collections.CollectionHelper.isNotEmpty;
 import static org.hibernate.internal.util.collections.CollectionHelper.makeCopy;
+import static org.hibernate.internal.util.type.PrimitiveWrapperHelper.getDescriptorByPrimitiveType;
 import static org.hibernate.jpa.HibernateHints.HINT_NATIVE_LOCK_MODE;
 import static org.hibernate.query.results.internal.Builders.resultClassBuilder;
 import static org.hibernate.query.results.ResultSetMapping.resolveResultSetMapping;
@@ -330,25 +332,54 @@ private void handleExplicitResultSetMapping() {
 				setTupleTransformerForResultType( resultType );
 			}
 			else {
-				checkResultType( resultType );
+				checkResultType( resultType, resultSetMapping );
 			}
 		}
 	}
 
-	private void checkResultType(Class<R> resultType) {
-		switch ( resultSetMapping.getNumberOfResultBuilders() ) {
-			case 0:
-				throw new IllegalArgumentException( "Named query exists, but did not specify a resultClass" );
-			case 1:
-				final Class<?> actualResultJavaType =
-						resultSetMapping.getResultBuilders().get( 0 ).getJavaType();
-				if ( actualResultJavaType != null && !resultType.isAssignableFrom( actualResultJavaType ) ) {
-					throw buildIncompatibleException( resultType, actualResultJavaType );
-				}
-				break;
-			default:
-				throw new IllegalArgumentException( "Cannot create TypedQuery for query with more than one return" );
+	private void checkResultType(Class<R> resultType, ResultSetMapping resultSetMapping) {
+		// resultType can be null if any of the deprecated methods were used to create the query
+		if ( resultType != null && !isResultTypeAlwaysAllowed( resultType )) {
+			switch ( resultSetMapping.getNumberOfResultBuilders() ) {
+				case 0:
+					if ( !resultSetMapping.isDynamic() ) {
+						throw new IllegalArgumentException( "Named query exists, but did not specify a resultClass" );
+					}
+					break;
+				case 1:
+					final Class<?> actualResultJavaType =
+							resultSetMapping.getResultBuilders().get( 0 ).getJavaType();
+					if ( actualResultJavaType != null && !resultType.isAssignableFrom( actualResultJavaType ) ) {
+						throw buildIncompatibleException( resultType, actualResultJavaType );
+					}
+					break;
+				default:
+					// The return type has to be a class with an appropriate constructor, i.e. one whose parameter types match
+					// the types of the result builders. If none such constructor is found, throw an IAE
+					if ( !validConstructorFoundForResultType( resultType, resultSetMapping ) ) {
+						throw new IllegalArgumentException( "The declared return type for a multi-valued result set mapping should be Object[], Map, List, or Tuple" );
+					}
+			}
+		}
+	}
+
+	private boolean validConstructorFoundForResultType(Class<R> resultType, ResultSetMapping resultSetMapping) {
+		// Only 1 constructor with the right number of parameters is allowed (see NativeQueryConstructorTransformer)
+		Constructor<?> constructor = resultType.getConstructors()[0];
+		if ( constructor.getParameterCount() != resultSetMapping.getNumberOfResultBuilders() ) {
+			return false;
+		}
+		final List<ResultBuilder> resultBuilders = resultSetMapping.getResultBuilders();
+		Class<?>[] paramTypes = constructor.getParameterTypes();
+		for ( int i = 0; i < resultBuilders.size(); i++ ) {
+			if (
+					resultBuilders.get( i ).getJavaType() != ( paramTypes[i].isPrimitive() ?
+							getDescriptorByPrimitiveType(paramTypes[i] ).getWrapperClass() :
+							paramTypes[i]) ) {
+				return false;
+			}
 		}
+		return true;
 	}
 
 	protected <T> void setTupleTransformerForResultType(Class<T> resultClass) {
@@ -740,6 +771,7 @@ else if ( !isResultTypeAlwaysAllowed( resultType )
 		else {
 			mapping = resultSetMapping;
 		}
+		checkResultType( resultType, mapping );
 		return isCacheableQuery()
 				? getInterpretationCache()
 						.resolveSelectQueryPlan( selectInterpretationsKey( mapping ), () -> createQueryPlan( mapping ) )

hibernate-core/src/test/java/org/hibernate/orm/test/sql/hand/query/NativeSQLQueriesTest.java

@@ -38,6 +38,8 @@
 import org.hibernate.query.NativeQuery;
 import org.hibernate.query.Query;
 import org.hibernate.query.ResultListTransformer;
+import org.hibernate.testing.orm.junit.Jira;
+import org.hibernate.testing.orm.junit.JiraGroup;
 import org.hibernate.transform.ResultTransformer;
 import org.hibernate.transform.Transformers;
 import org.hibernate.type.StandardBasicTypes;
@@ -59,6 +61,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -938,6 +941,32 @@ public void testAliasToBeanMap(SessionFactoryScope scope) {
 		);
 	}
 
+	@Test
+	@JiraGroup(
+			{
+					@Jira("https://hibernate.atlassian.net/browse/HHH-19376"),
+					@Jira("https://hibernate.atlassian.net/browse/HHH-19383")
+			}
+	)
+	public void testMutateResultSetMapping(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					String sql = "SELECT p.*, COUNT(*) OVER() AS total_count "
+								+ "FROM Person p "
+								+ "WHERE p.name ILIKE :name "
+								+ "ORDER BY p.id";
+					// Declare Person as result type
+					NativeQuery<Person> query = session.createNativeQuery(sql, Person.class);
+					query.setParameter("name", "Ja%");
+					query.setMaxResults(2);
+					query.setFirstResult(0);
+					// Now mutate the result set mapping and verify an Exception is thrown
+					assertThrows( IllegalArgumentException.class,
+							() -> query.addScalar( "total_count", StandardBasicTypes.LONG).list() );
+				}
+		);
+	}
+
 	private String buildLongString(int size, char baseChar) {
 		StringBuilder buff = new StringBuilder();
 		for( int i = 0; i < size; i++ ) {