Polishing.

Introduce constructor compatibility check by checking assignable types.

Original Pull Request: #3654

Author: mp911de

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java

@@ -24,6 +24,7 @@
 import jakarta.persistence.TypedQuery;
 
 import java.lang.reflect.Constructor;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
@@ -34,6 +35,7 @@
 import java.util.stream.Collectors;
 
 import org.springframework.beans.BeanUtils;
+import org.springframework.core.MethodParameter;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.jpa.provider.PersistenceProvider;
 import org.springframework.data.jpa.repository.EntityGraph;
@@ -56,7 +58,6 @@
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.ReflectionUtils;
 
 /**
  * Abstract base class to implement {@link RepositoryQuery}s.
@@ -347,7 +348,7 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) {
 					&& !type.getInputProperties().isEmpty();
 
 			if (this.dtoProjection) {
-				 this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType());
+				this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType());
 			} else {
 				this.preferredConstructor = null;
 			}
@@ -373,67 +374,97 @@ public Object convert(Object source) {
 
 			if (dtoProjection) {
 
-				Object[] ctorArgs = new Object[type.getInputProperties().size()];
+				Object[] ctorArgs = new Object[elements.size()];
+				for (int i = 0; i < ctorArgs.length; i++) {
+					ctorArgs[i] = tuple.get(i);
+				}
+
+				List<Class<?>> argTypes = getArgumentTypes(ctorArgs);
 
-				boolean containsNullValue = false;
-				for (int i = 0; i < type.getInputProperties().size(); i++) {
-					Object value = tuple.get(i);
-					ctorArgs[i] = value;
-					if (!containsNullValue && value == null) {
-						containsNullValue = true;
-					}
+				if (preferredConstructor != null && isConstructorCompatible(preferredConstructor.getConstructor(), argTypes)) {
+					return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs);
 				}
 
-				try {
+				return BeanUtils.instantiateClass(getFirstMatchingConstructor(ctorArgs, argTypes), ctorArgs);
+			}
 
-					if (preferredConstructor != null && preferredConstructor.getParameterCount() == ctorArgs.length) {
-						return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs);
-					}
+			return new TupleBackedMap(tupleWrapper.apply(tuple));
+		}
 
-					Constructor<?> ctor = null;
+		private Constructor<?> getFirstMatchingConstructor(Object[] ctorArgs, List<Class<?>> argTypes) {
 
-					if (!containsNullValue) { // let's seem if we have an argument type match
-						ctor = type.getReturnedType()
-								.getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class<?>[]::new));
-					}
+			for (Constructor<?> ctor : type.getReturnedType().getDeclaredConstructors()) {
 
-					if (ctor == null) { // let's see if there's more general constructor we could use that accepts our args
-						ctor = findFirstMatchingConstructor(ctorArgs);
-					}
+				if (ctor.getParameterCount() != ctorArgs.length) {
+					continue;
+				}
 
-					if (ctor != null) {
-						return BeanUtils.instantiateClass(ctor, ctorArgs);
-					}
-				} catch (ReflectiveOperationException e) {
-					ReflectionUtils.handleReflectionException(e);
+				if (isConstructorCompatible(ctor, argTypes)) {
+					return ctor;
 				}
 			}
 
-			return new TupleBackedMap(tupleWrapper.apply(tuple));
+			throw new IllegalStateException(String.format(
+					"Cannot find compatible constructor for DTO projection '%s' accepting '%s'", type.getReturnedType().getName(),
+					argTypes.stream().map(Class::getName).collect(Collectors.joining(", "))));
 		}
 
-		@Nullable
-		private Constructor<?> findFirstMatchingConstructor(Object[] ctorArgs) {
+		private static List<Class<?>> getArgumentTypes(Object[] ctorArgs) {
+			List<Class<?>> argTypes = new ArrayList<>(ctorArgs.length);
 
-			for (Constructor<?> ctor : type.getReturnedType().getDeclaredConstructors()) {
+			for (Object ctorArg : ctorArgs) {
+				argTypes.add(ctorArg == null ? Void.class : ctorArg.getClass());
+			}
+			return argTypes;
+		}
 
-				if (ctor.getParameterCount() == ctorArgs.length) {
-					boolean itsAMatch = true;
-					for (int i = 0; i < ctor.getParameterCount(); i++) {
-						if (ctorArgs[i] == null) {
-							continue;
-						}
-						if (!ClassUtils.isAssignable(ctor.getParameterTypes()[i], ctorArgs[i].getClass())) {
-							itsAMatch = false;
-							break;
-						}
-					}
-					if (itsAMatch) {
-						return ctor;
-					}
+		public static boolean isConstructorCompatible(Constructor<?> constructor, List<Class<?>> argumentTypes) {
+
+			if (constructor.getParameterCount() != argumentTypes.size()) {
+				return false;
+			}
+
+			for (int i = 0; i < argumentTypes.size(); i++) {
+
+				MethodParameter methodParameter = MethodParameter.forExecutable(constructor, i);
+				Class<?> argumentType = argumentTypes.get(i);
+
+				if (!areAssignmentCompatible(methodParameter.getParameterType(), argumentType)) {
+					return false;
 				}
 			}
-			return null;
+			return true;
+		}
+
+		private static boolean areAssignmentCompatible(Class<?> to, Class<?> from) {
+
+			if (from == Void.class && !to.isPrimitive()) {
+				// treat Void as the bottom type, the class of null
+				return true;
+			}
+
+			if (to.isPrimitive()) {
+
+				if (to == Short.TYPE) {
+					return from == Character.class || from == Byte.class;
+				}
+
+				if (to == Integer.TYPE) {
+					return from == Short.class || from == Character.class || from == Byte.class;
+				}
+
+				if (to == Long.TYPE) {
+					return from == Integer.class || from == Short.class || from == Character.class || from == Byte.class;
+				}
+
+				if (to == Double.TYPE) {
+					return from == Float.class;
+				}
+
+				return ClassUtils.isAssignable(to, from);
+			}
+
+			return ClassUtils.isAssignable(to, from);
 		}
 
 		/**

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java

@@ -18,14 +18,14 @@
 import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
+import jakarta.persistence.Tuple;
+import jakarta.persistence.TupleElement;
+
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
-import jakarta.persistence.Tuple;
-import jakarta.persistence.TupleElement;
-
 import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -49,6 +49,8 @@
  *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Christoph Strobl
+ * @author Mark Paluch
  * @soundtrack James Bay - Let it go (Chaos and the Calm)
  */
 @ExtendWith(MockitoExtension.class)
@@ -115,13 +117,88 @@ void findsValuesForAllVariantsSupportedByTheTuple() {
 	}
 
 	@Test // GH-3076
-	void dealsWithNullsInArgumetents() {
+	void dealsWithNullsInArguments() {
 
 		ReturnedType returnedType = ReturnedType.of(WithPC.class, DomainType.class, new SpelAwareProxyProjectionFactory());
 
+		doReturn(List.of(element, element, element)).when(tuple).getElements();
+		when(tuple.get(eq(0))).thenReturn("one");
+		when(tuple.get(eq(1))).thenReturn(null);
+		when(tuple.get(eq(2))).thenReturn(1);
+
+		Object result = new TupleConverter(returnedType).convert(tuple);
+		assertThat(result).isInstanceOf(WithPC.class);
+	}
+
+	@Test // GH-3076
+	void fallsBackToCompatibleConstructor() {
+
+		ReturnedType returnedType = spy(
+				ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory()));
+		when(returnedType.isProjecting()).thenReturn(true);
+		when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two", "three"));
+
+		doReturn(List.of(element, element, element)).when(tuple).getElements();
+		when(tuple.get(eq(0))).thenReturn("one");
+		when(tuple.get(eq(1))).thenReturn(null);
+		when(tuple.get(eq(2))).thenReturn(2);
+
+		MultipleConstructors result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple);
+
+		assertThat(result.one).isEqualTo("one");
+		assertThat(result.two).isNull();
+		assertThat(result.three).isEqualTo(2);
+
+		reset(tuple);
+
+		doReturn(List.of(element, element, element)).when(tuple).getElements();
+		when(tuple.get(eq(0))).thenReturn("one");
+		when(tuple.get(eq(1))).thenReturn(null);
+		when(tuple.get(eq(2))).thenReturn('a');
+
+		result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple);
+
+		assertThat(result.one).isEqualTo("one");
+		assertThat(result.two).isNull();
+		assertThat(result.three).isEqualTo(97);
+	}
+
+	@Test // GH-3076
+	void acceptsConstructorWithCastableType() {
+
+		ReturnedType returnedType = spy(
+				ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory()));
+		when(returnedType.isProjecting()).thenReturn(true);
+		when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two", "three", "four"));
+
+		doReturn(List.of(element, element, element, element)).when(tuple).getElements();
 		when(tuple.get(eq(0))).thenReturn("one");
 		when(tuple.get(eq(1))).thenReturn(null);
-		new TupleConverter(returnedType).convert(tuple);
+		when(tuple.get(eq(2))).thenReturn((byte) 2);
+		when(tuple.get(eq(3))).thenReturn(2.1f);
+
+		MultipleConstructors result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple);
+
+		assertThat(result.one).isEqualTo("one");
+		assertThat(result.two).isNull();
+		assertThat(result.three).isEqualTo(2);
+		assertThat(result.four).isEqualTo(2, offset(0.1d));
+	}
+
+	@Test // GH-3076
+	void failsForNonResolvableConstructor() {
+
+		ReturnedType returnedType = spy(
+				ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory()));
+		when(returnedType.isProjecting()).thenReturn(true);
+		when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two"));
+
+		doReturn(List.of(element, element)).when(tuple).getElements();
+		when(tuple.get(eq(0))).thenReturn(1);
+		when(tuple.get(eq(1))).thenReturn(null);
+
+		assertThatIllegalStateException().isThrownBy(() -> new TupleConverter(returnedType).convert(tuple))
+				.withMessageContaining("Cannot find compatible constructor for DTO projection");
 	}
 
 	interface SampleRepository extends CrudRepository<Object, Long> {
@@ -193,13 +270,40 @@ static class DomainType {
 		String one, two, three;
 	}
 
-	static class  WithPC {
+	static class WithPC {
 		String one;
 		String two;
+		long three;
 
-		public WithPC(String one, String two) {
+		public WithPC(String one, String two, long three) {
 			this.one = one;
 			this.two = two;
+			this.three = three;
 		}
 	}
+
+	static class MultipleConstructors {
+		String one;
+		String two;
+		long three;
+		double four;
+
+		public MultipleConstructors(String one) {
+			this.one = one;
+		}
+
+		public MultipleConstructors(String one, String two, long three) {
+			this.one = one;
+			this.two = two;
+			this.three = three;
+		}
+
+		public MultipleConstructors(String one, String two, short three, double four) {
+			this.one = one;
+			this.two = two;
+			this.three = three;
+			this.four = four;
+		}
+
+	}
 }