diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index ec90ced24b..772b1d7032 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/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> 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> 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> getArgumentTypes(Object[] ctorArgs) { + List> 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> 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); } /** diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java index e16ebaafe2..8682012fa4 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java +++ b/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 { @@ -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; + } + + } }