diff --git a/src/main/java/org/springframework/data/repository/query/DefaultParameters.java b/src/main/java/org/springframework/data/repository/query/DefaultParameters.java index e67c8927e..1cadc973d 100644 --- a/src/main/java/org/springframework/data/repository/query/DefaultParameters.java +++ b/src/main/java/org/springframework/data/repository/query/DefaultParameters.java @@ -18,7 +18,7 @@ package org.springframework.data.repository.query; import java.lang.reflect.Method; import java.util.List; -import org.springframework.core.MethodParameter; +import org.springframework.data.util.TypeInformation; /** * Default implementation of {@link Parameters}. @@ -31,18 +31,26 @@ public final class DefaultParameters extends Parameters parameters) { - super(parameters); + /** + * Creates a new {@link DefaultParameters} instance from the given {@link Method} and aggregate + * {@link TypeInformation}. + * + * @param method must not be {@literal null}. + * @param aggregateType must not be {@literal null}. + */ + public DefaultParameters(Method method, TypeInformation aggregateType) { + super(method, param -> new Parameter(param, aggregateType)); } - @Override - protected Parameter createParameter(MethodParameter parameter) { - return new Parameter(parameter); + private DefaultParameters(List parameters) { + super(parameters); } @Override diff --git a/src/main/java/org/springframework/data/repository/query/Parameter.java b/src/main/java/org/springframework/data/repository/query/Parameter.java index 2bcfef10c..d1f792f9f 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameter.java +++ b/src/main/java/org/springframework/data/repository/query/Parameter.java @@ -17,7 +17,6 @@ package org.springframework.data.repository.query; import static java.lang.String.*; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -72,14 +71,27 @@ public class Parameter { * Creates a new {@link Parameter} for the given {@link MethodParameter}. * * @param parameter must not be {@literal null}. + * @deprecated since 3.1, use {@link #Parameter(MethodParameter, TypeInformation)} instead. */ + @Deprecated(since = "3.1", forRemoval = true) protected Parameter(MethodParameter parameter) { + this(parameter, TypeInformation.of(Parameter.class)); + } + + /** + * Creates a new {@link Parameter} for the given {@link MethodParameter} and aggregate {@link TypeInformation}. + * + * @param parameter must not be {@literal null}. + * @param aggregateType must not be {@literal null}. + */ + protected Parameter(MethodParameter parameter, TypeInformation aggregateType) { Assert.notNull(parameter, "MethodParameter must not be null"); + Assert.notNull(aggregateType, "TypeInformation must not be null!"); this.parameter = parameter; this.parameterType = potentiallyUnwrapParameterType(parameter); - this.isDynamicProjectionParameter = isDynamicProjectionParameter(parameter); + this.isDynamicProjectionParameter = isDynamicProjectionParameter(parameter, aggregateType); this.name = isSpecialParameterType(parameter.getParameterType()) ? Lazy.of(Optional.empty()) : Lazy.of(() -> { Param annotation = parameter.getParameterAnnotation(Param.class); return Optional.ofNullable(annotation == null ? parameter.getParameterName() : annotation.value()); @@ -206,23 +218,32 @@ public class Parameter { * * * @param parameter must not be {@literal null}. + * @param aggregateType the reference aggregate type, must not be {@literal null}. * @return */ - private static boolean isDynamicProjectionParameter(MethodParameter parameter) { + private static boolean isDynamicProjectionParameter(MethodParameter parameter, TypeInformation aggregateType) { if (!parameter.getParameterType().equals(Class.class)) { return false; } - Method method = parameter.getMethod(); + if (parameter.hasParameterAnnotation(Param.class)) { + return false; + } + + var method = parameter.getMethod(); if (method == null) { throw new IllegalArgumentException("Parameter is not associated with any method"); } - TypeInformation returnType = TypeInformation.fromReturnTypeOf(method); - TypeInformation unwrapped = QueryExecutionConverters.unwrapWrapperTypes(returnType); - TypeInformation reactiveUnwrapped = ReactiveWrapperConverters.unwrapWrapperTypes(unwrapped); + var returnType = TypeInformation.fromReturnTypeOf(method); + var unwrapped = QueryExecutionConverters.unwrapWrapperTypes(returnType); + var reactiveUnwrapped = ReactiveWrapperConverters.unwrapWrapperTypes(unwrapped); + + if (aggregateType.isAssignableFrom(reactiveUnwrapped)) { + return false; + } return reactiveUnwrapped.equals(TypeInformation.fromMethodParameter(parameter).getComponentType()); } diff --git a/src/main/java/org/springframework/data/repository/query/Parameters.java b/src/main/java/org/springframework/data/repository/query/Parameters.java index b5bb5ed04..7dfdf74c0 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameters.java +++ b/src/main/java/org/springframework/data/repository/query/Parameters.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.List; +import java.util.function.Function; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; @@ -62,11 +63,28 @@ public abstract class Parameters, T extends Parameter * Creates a new instance of {@link Parameters}. * * @param method must not be {@literal null}. + * @deprecated since 3.1, use {@link #Parameters(Method, Function)} instead. */ + @SuppressWarnings("null") + @Deprecated(since = "3.1", forRemoval = true) public Parameters(Method method) { + this(method, null); + } + + /** + * Creates a new {@link Parameters} instance for the given {@link Method} and {@link Function} to create a + * {@link Parameter} instance from a {@link MethodParameter}. + * + * @param method must not be {@literal null}. + * @param parameterFactory must not be {@literal null}. + */ + protected Parameters(Method method, Function parameterFactory) { Assert.notNull(method, "Method must not be null"); + // Factory nullability not enforced yet to support falling back to the deprecated + // createParameter(MethodParameter). Add assertion when the deprecation is removed. + int parameterCount = method.getParameterCount(); this.parameters = new ArrayList<>(parameterCount); @@ -80,7 +98,9 @@ public abstract class Parameters, T extends Parameter MethodParameter methodParameter = new MethodParameter(method, i); methodParameter.initParameterNameDiscovery(PARAMETER_NAME_DISCOVERER); - T parameter = createParameter(methodParameter); + T parameter = parameterFactory == null // + ? createParameter(methodParameter) // + : parameterFactory.apply(methodParameter); if (parameter.isSpecialParameter() && parameter.isNamedParameter()) { throw new IllegalArgumentException(PARAM_ON_SPECIAL); @@ -156,8 +176,13 @@ public abstract class Parameters, T extends Parameter * * @param parameter will never be {@literal null}. * @return + * @deprecated since 3.1, in your extension, call {@link #Parameters(Method, ParameterFactory)} instead. */ - protected abstract T createParameter(MethodParameter parameter); + @SuppressWarnings("unchecked") + @Deprecated(since = "3.1", forRemoval = true) + protected T createParameter(MethodParameter parameter) { + return (T) new Parameter(parameter); + } /** * Returns whether the method the {@link Parameters} was created for contains a {@link Pageable} argument. @@ -169,8 +194,8 @@ public abstract class Parameters, T extends Parameter } /** - * Returns the index of the {@link Pageable} {@link Method} parameter if available. Will return {@literal -1} if there - * is no {@link Pageable} argument in the {@link Method}'s parameter list. + * Returns the index of the {@link Pageable} {@link Method} parameter if available. Will return {@literal -1} if + * there is no {@link Pageable} argument in the {@link Method}'s parameter list. * * @return the pageableIndex */ @@ -179,8 +204,8 @@ public abstract class Parameters, T extends Parameter } /** - * Returns the index of the {@link Sort} {@link Method} parameter if available. Will return {@literal -1} if there is - * no {@link Sort} argument in the {@link Method}'s parameter list. + * Returns the index of the {@link Sort} {@link Method} parameter if available. Will return {@literal -1} if there + * is no {@link Sort} argument in the {@link Method}'s parameter list. * * @return */ @@ -289,8 +314,8 @@ public abstract class Parameters, T extends Parameter /** * Returns a bindable parameter with the given index. So for a method with a signature of - * {@code (Pageable pageable, String name)} a call to {@code #getBindableParameter(0)} will return the {@link String} - * parameter. + * {@code (Pageable pageable, String name)} a call to {@code #getBindableParameter(0)} will return the + * {@link String} parameter. * * @param bindableIndex * @return diff --git a/src/main/java/org/springframework/data/repository/query/QueryMethod.java b/src/main/java/org/springframework/data/repository/query/QueryMethod.java index 56d56e67c..76120505b 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -79,8 +79,8 @@ public class QueryMethod { this.method = method; this.unwrappedReturnType = potentiallyUnwrapReturnTypeFor(metadata, method); - this.parameters = createParameters(method); this.metadata = metadata; + this.parameters = createParameters(method); if (hasParameterOfType(method, Pageable.class)) { @@ -107,7 +107,7 @@ public class QueryMethod { Class repositoryDomainClass = metadata.getDomainType(); Class methodDomainClass = metadata.getReturnedDomainClass(method); - return (repositoryDomainClass == null) || repositoryDomainClass.isAssignableFrom(methodDomainClass) + return repositoryDomainClass == null || repositoryDomainClass.isAssignableFrom(methodDomainClass) ? methodDomainClass : repositoryDomainClass; }); @@ -119,11 +119,24 @@ public class QueryMethod { /** * Creates a {@link Parameters} instance. * - * @param method + * @param method must not be {@literal null}. * @return must not return {@literal null}. + * @deprecated since 3.1, call or override {@link #createParameters(Method, TypeInformation)} instead. */ + @Deprecated(since = "3.1", forRemoval = true) protected Parameters createParameters(Method method) { - return new DefaultParameters(method); + return createParameters(method, metadata.getDomainTypeInformation()); + } + + /** + * Creates a {@link Parameters} instance. + * + * @param method must not be {@literal null}. + * @param aggregateType must not be {@literal null}. + * @return must not return {@literal null}. + */ + protected Parameters createParameters(Method method, TypeInformation aggregateType) { + return new DefaultParameters(method, aggregateType); } /** diff --git a/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java index 516bac363..f0e08bba1 100644 --- a/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java @@ -23,15 +23,19 @@ import java.util.Optional; import java.util.stream.Stream; import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestFactory; import org.springframework.core.MethodParameter; +import org.springframework.data.repository.query.ParametersUnitTests.User; +import org.springframework.data.util.TypeInformation; /** * Unit tests for {@link Parameter}. * * @author Jens Schauder */ -class KParameterUnitTests { +class ParameterUnitTests { @Test // DATAJPA-1185 void classParameterWithSameTypeParameterAsReturnedListIsDynamicProjectionParameter() throws Exception { @@ -57,6 +61,28 @@ class KParameterUnitTests { assertThat(parameter.isDynamicProjectionParameter()).isTrue(); } + @TestFactory // #2770 + Stream doesNotConsiderClassParametersDynamicProjectionOnes() { + + var methods = Stream.of( // + "genericReturnNonDynamicBind", // + "staticReturnNonDynamicBindWildcard", // + "staticReturnNonDynamicBindWildcardExtends"); + + return DynamicTest.stream(methods, it -> it, it -> { + assertThat(new Parameter(getMethodParameter(it), TypeInformation.of(User.class)) + .isDynamicProjectionParameter()).isFalse(); + }); + } + + @Test // #2770 + void doesNotConsiderAtParamAnnotatedClassParameterDynamicProjectionOne() throws Exception { + + var parameter = new Parameter(getMethodParameter("atParamOnClass")); + + assertThat(parameter.isDynamicProjectionParameter()).isFalse(); + } + @NotNull private MethodParameter getMethodParameter(String methodName) throws NoSuchMethodException { return new MethodParameter(this.getClass().getDeclaredMethod(methodName, Class.class), 0); @@ -73,4 +99,20 @@ class KParameterUnitTests { Optional dynamicProjectionWithOptional(Class type) { return Optional.empty(); } + + T genericReturnNonDynamicBind(Class one) { + return null; + } + + User staticReturnNonDynamicBindWildcard(Class two) { + return null; + } + + User staticReturnNonDynamicBindWildcardExtends(Class one) { + return null; + } + + T atParamOnClass(@Param("type") Class type) { + return null; + } }