From c5db0d38e757e1830e0e42ac5a061bc2dd2ccbb8 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Thu, 26 Sep 2024 11:58:34 +0200 Subject: [PATCH] HHH-18664 Consistent constructor matching logic for row-transformer --- .../internal/ConcreteSqmSelectQueryPlan.java | 12 +++- .../DynamicInstantiationResultImpl.java | 19 +++--- .../internal/InstantiationHelper.java | 16 +++-- .../RowTransformerConstructorImpl.java | 60 +++++-------------- 4 files changed, 48 insertions(+), 59 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java index e1eabc85bd..1a0b56b35c 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java @@ -287,7 +287,11 @@ public class ConcreteSqmSelectQueryPlan implements SelectQueryPlan { } else if ( isClass( resultType ) ) { try { - return new RowTransformerConstructorImpl<>( resultType, tupleMetadata ); + return new RowTransformerConstructorImpl<>( + resultType, + tupleMetadata, + sqm.nodeBuilder().getTypeConfiguration() + ); } catch (InstantiationException ie) { return new RowTransformerCheckingImpl<>( resultType ); @@ -310,7 +314,11 @@ public class ConcreteSqmSelectQueryPlan implements SelectQueryPlan { return (RowTransformer) new RowTransformerMapImpl( tupleMetadata ); } else if ( isClass( resultType ) ) { - return new RowTransformerConstructorImpl<>( resultType, tupleMetadata ); + return new RowTransformerConstructorImpl<>( + resultType, + tupleMetadata, + sqm.nodeBuilder().getTypeConfiguration() + ); } else { throw new QueryTypeMismatchException( "Result type '" + resultType.getSimpleName() diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationResultImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationResultImpl.java index e1b05c0c24..ece54851bc 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationResultImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationResultImpl.java @@ -19,12 +19,12 @@ import org.hibernate.sql.results.graph.DomainResultAssembler; import org.hibernate.sql.results.graph.InitializerParent; import org.hibernate.sql.results.graph.instantiation.DynamicInstantiationResult; import org.hibernate.type.descriptor.java.JavaType; - import org.hibernate.type.spi.TypeConfiguration; + import org.jboss.logging.Logger; import static java.util.stream.Collectors.toList; -import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.isConstructorCompatible; +import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.findMatchingConstructor; /** * @author Steve Ebersole @@ -164,13 +164,14 @@ public class DynamicInstantiationResultImpl implements DynamicInstantiationRe .getMappingMetamodel() .getTypeConfiguration(); // find a constructor matching argument types - for ( Constructor constructor : javaType.getJavaTypeClass().getDeclaredConstructors() ) { - if ( isConstructorCompatible( constructor, argumentTypes, typeConfiguration ) ) { - constructor.setAccessible( true ); - @SuppressWarnings("unchecked") - final Constructor construct = (Constructor) constructor; - return new DynamicInstantiationAssemblerConstructorImpl<>( construct, javaType, argumentReaders ); - } + final Constructor constructor = findMatchingConstructor( + javaType.getJavaTypeClass(), + argumentTypes, + typeConfiguration + ); + if ( constructor != null ) { + constructor.setAccessible( true ); + return new DynamicInstantiationAssemblerConstructorImpl<>( constructor, javaType, argumentReaders ); } if ( log.isDebugEnabled() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/InstantiationHelper.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/InstantiationHelper.java index 077e09aeff..cdf813dc04 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/InstantiationHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/InstantiationHelper.java @@ -55,12 +55,20 @@ public class InstantiationHelper { } public static boolean isConstructorCompatible(Class javaClass, List> argTypes, TypeConfiguration typeConfiguration) { - for ( Constructor constructor : javaClass.getDeclaredConstructors() ) { - if ( isConstructorCompatible( constructor, argTypes, typeConfiguration) ) { - return true; + return findMatchingConstructor( javaClass, argTypes, typeConfiguration ) != null; + } + + public static Constructor findMatchingConstructor( + Class type, + List> argumentTypes, + TypeConfiguration typeConfiguration) { + for ( final Constructor constructor : type.getDeclaredConstructors() ) { + if ( isConstructorCompatible( constructor, argumentTypes, typeConfiguration ) ) { + //noinspection unchecked + return (Constructor) constructor; } } - return false; + return null; } public static boolean isConstructorCompatible( diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java index c8b136ae40..e3c840acf4 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java @@ -13,8 +13,10 @@ import java.util.List; import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.tree.SqmExpressibleAccessor; +import org.hibernate.type.spi.TypeConfiguration; -import static org.hibernate.query.sqm.tree.expression.Compatibility.areAssignmentCompatible; +import static java.util.stream.Collectors.toList; +import static org.hibernate.sql.results.graph.instantiation.internal.InstantiationHelper.findMatchingConstructor; /** * {@link RowTransformer} instantiating an arbitrary class @@ -25,25 +27,25 @@ public class RowTransformerConstructorImpl implements RowTransformer { private final Class type; private final Constructor constructor; - public RowTransformerConstructorImpl(Class type, TupleMetadata tupleMetadata) { + public RowTransformerConstructorImpl( + Class type, + TupleMetadata tupleMetadata, + TypeConfiguration typeConfiguration) { this.type = type; final List> elements = tupleMetadata.getList(); - final Class[] sig = new Class[elements.size()]; - for (int i = 0; i < elements.size(); i++) { - sig[i] = resolveElementJavaType( elements.get( i ) ); - } - if ( sig.length == 1 && sig[0] == null ) { + final List> argumentTypes = elements.stream() + .map( RowTransformerConstructorImpl::resolveElementJavaType ) + .collect( toList() ); + if ( argumentTypes.size() == 1 && argumentTypes.get( 0 ) == null ) { // Can not (properly) resolve constructor for single null element throw new InstantiationException( "Cannot instantiate query result type, argument types are unknown ", type ); } - try { - constructor = findMatchingConstructor( type, sig ); - constructor.setAccessible( true ); - } - catch (Exception e) { - //TODO try again with primitive types - throw new InstantiationException( "Cannot instantiate query result type ", type, e ); + + constructor = findMatchingConstructor( type, argumentTypes, typeConfiguration ); + if ( constructor == null ) { + throw new InstantiationException( "Cannot instantiate query result type, found no matching constructor", type ); } + constructor.setAccessible( true ); } private static Class resolveElementJavaType(TupleElement element) { @@ -57,36 +59,6 @@ public class RowTransformerConstructorImpl implements RowTransformer { return element.getJavaType(); } - private Constructor findMatchingConstructor(Class type, Class[] sig) throws Exception { - try { - return type.getDeclaredConstructor( sig ); - } - catch (NoSuchMethodException | SecurityException e) { - constructor_loop: - for ( final Constructor constructor : type.getDeclaredConstructors() ) { - final Class[] parameterTypes = constructor.getParameterTypes(); - if ( parameterTypes.length == sig.length ) { - for ( int i = 0; i < sig.length; i++ ) { - final Class parameterType = parameterTypes[i]; - final Class argType = sig[i]; - final boolean assignmentCompatible; - assignmentCompatible = - argType == null && !parameterType.isPrimitive() - || areAssignmentCompatible( - parameterType, - argType - ); - if ( !assignmentCompatible ) { - continue constructor_loop; - } - } - return (Constructor) constructor; - } - } - throw e; - } - } - @Override public T transformRow(Object[] row) { try {