Skip to content

Commit b775b6d

Browse files
odrotbohmmp911de
authored andcommitted
Query method parameters are now aware of aggregate reference type.
To support the binding of Class parameters to queries for declared query methods, we have to be able to differentiate them from Class parameters that are supposed to represent projections. We can do that by relating the declared Class' element type to the aggregate root type as a Class typed to that or any subtype of it will never trigger a projection by definition. So far the Parameter(s) abstraction was solely created from a query method's Method. We now changed that for QueryMethod to forward the aggregate type detected on the RepositoryMetadata and consider it during the detection of dynamic projection parameters. As a mitigating measure, we now also support @param on Class-typed parameters to explicitly mark them for query binding. This is primarily to be able to add this support to the 2.7 The changes are built in a way that modules extending that mechanism will continue to work as is but with the intent to deprecate the previous API. Adapting extending code to the new APIs will automatically enable the support for bindable Class parameters on query methods. Fixes #2770. Original pull request: #2771
1 parent 6a036ad commit b775b6d

File tree

5 files changed

+135
-26
lines changed

5 files changed

+135
-26
lines changed

src/main/java/org/springframework/data/repository/query/DefaultParameters.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import java.lang.reflect.Method;
1919
import java.util.List;
2020

21-
import org.springframework.core.MethodParameter;
21+
import org.springframework.data.util.TypeInformation;
2222

2323
/**
2424
* Default implementation of {@link Parameters}.
@@ -31,18 +31,26 @@ public final class DefaultParameters extends Parameters<DefaultParameters, Param
3131
* Creates a new {@link DefaultParameters} instance from the given {@link Method}.
3232
*
3333
* @param method must not be {@literal null}.
34+
* @deprecated since 3.1, use {@link #DefaultParameters(Method, TypeInformation)} instead.
3435
*/
36+
@Deprecated(since = "3.1", forRemoval = true)
3537
public DefaultParameters(Method method) {
3638
super(method);
3739
}
3840

39-
private DefaultParameters(List<Parameter> parameters) {
40-
super(parameters);
41+
/**
42+
* Creates a new {@link DefaultParameters} instance from the given {@link Method} and aggregate
43+
* {@link TypeInformation}.
44+
*
45+
* @param method must not be {@literal null}.
46+
* @param aggregateType must not be {@literal null}.
47+
*/
48+
public DefaultParameters(Method method, TypeInformation<?> aggregateType) {
49+
super(method, param -> new Parameter(param, aggregateType));
4150
}
4251

43-
@Override
44-
protected Parameter createParameter(MethodParameter parameter) {
45-
return new Parameter(parameter);
52+
private DefaultParameters(List<Parameter> parameters) {
53+
super(parameters);
4654
}
4755

4856
@Override

src/main/java/org/springframework/data/repository/query/Parameter.java

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import static java.lang.String.*;
1919

20-
import java.lang.reflect.Method;
2120
import java.util.ArrayList;
2221
import java.util.Arrays;
2322
import java.util.Collections;
@@ -72,14 +71,27 @@ public class Parameter {
7271
* Creates a new {@link Parameter} for the given {@link MethodParameter}.
7372
*
7473
* @param parameter must not be {@literal null}.
74+
* @deprecated since 3.1, use {@link #Parameter(MethodParameter, TypeInformation)} instead.
7575
*/
76+
@Deprecated(since = "3.1", forRemoval = true)
7677
protected Parameter(MethodParameter parameter) {
78+
this(parameter, TypeInformation.of(Parameter.class));
79+
}
80+
81+
/**
82+
* Creates a new {@link Parameter} for the given {@link MethodParameter} and aggregate {@link TypeInformation}.
83+
*
84+
* @param parameter must not be {@literal null}.
85+
* @param aggregateType must not be {@literal null}.
86+
*/
87+
protected Parameter(MethodParameter parameter, TypeInformation<?> aggregateType) {
7788

7889
Assert.notNull(parameter, "MethodParameter must not be null");
90+
Assert.notNull(aggregateType, "TypeInformation must not be null!");
7991

8092
this.parameter = parameter;
8193
this.parameterType = potentiallyUnwrapParameterType(parameter);
82-
this.isDynamicProjectionParameter = isDynamicProjectionParameter(parameter);
94+
this.isDynamicProjectionParameter = isDynamicProjectionParameter(parameter, aggregateType);
8395
this.name = isSpecialParameterType(parameter.getParameterType()) ? Lazy.of(Optional.empty()) : Lazy.of(() -> {
8496
Param annotation = parameter.getParameterAnnotation(Param.class);
8597
return Optional.ofNullable(annotation == null ? parameter.getParameterName() : annotation.value());
@@ -206,23 +218,32 @@ boolean isSort() {
206218
* </code>
207219
*
208220
* @param parameter must not be {@literal null}.
221+
* @param aggregateType the reference aggregate type, must not be {@literal null}.
209222
* @return
210223
*/
211-
private static boolean isDynamicProjectionParameter(MethodParameter parameter) {
224+
private static boolean isDynamicProjectionParameter(MethodParameter parameter, TypeInformation<?> aggregateType) {
212225

213226
if (!parameter.getParameterType().equals(Class.class)) {
214227
return false;
215228
}
216229

217-
Method method = parameter.getMethod();
230+
if (parameter.hasParameterAnnotation(Param.class)) {
231+
return false;
232+
}
233+
234+
var method = parameter.getMethod();
218235

219236
if (method == null) {
220237
throw new IllegalArgumentException("Parameter is not associated with any method");
221238
}
222239

223-
TypeInformation<?> returnType = TypeInformation.fromReturnTypeOf(method);
224-
TypeInformation<?> unwrapped = QueryExecutionConverters.unwrapWrapperTypes(returnType);
225-
TypeInformation<?> reactiveUnwrapped = ReactiveWrapperConverters.unwrapWrapperTypes(unwrapped);
240+
var returnType = TypeInformation.fromReturnTypeOf(method);
241+
var unwrapped = QueryExecutionConverters.unwrapWrapperTypes(returnType);
242+
var reactiveUnwrapped = ReactiveWrapperConverters.unwrapWrapperTypes(unwrapped);
243+
244+
if (aggregateType.isAssignableFrom(reactiveUnwrapped)) {
245+
return false;
246+
}
226247

227248
return reactiveUnwrapped.equals(TypeInformation.fromMethodParameter(parameter).getComponentType());
228249
}

src/main/java/org/springframework/data/repository/query/Parameters.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Arrays;
2323
import java.util.Iterator;
2424
import java.util.List;
25+
import java.util.function.Function;
2526

2627
import org.springframework.core.DefaultParameterNameDiscoverer;
2728
import org.springframework.core.MethodParameter;
@@ -62,11 +63,28 @@ public abstract class Parameters<S extends Parameters<S, T>, T extends Parameter
6263
* Creates a new instance of {@link Parameters}.
6364
*
6465
* @param method must not be {@literal null}.
66+
* @deprecated since 3.1, use {@link #Parameters(Method, Function)} instead.
6567
*/
68+
@SuppressWarnings("null")
69+
@Deprecated(since = "3.1", forRemoval = true)
6670
public Parameters(Method method) {
71+
this(method, null);
72+
}
73+
74+
/**
75+
* Creates a new {@link Parameters} instance for the given {@link Method} and {@link Function} to create a
76+
* {@link Parameter} instance from a {@link MethodParameter}.
77+
*
78+
* @param method must not be {@literal null}.
79+
* @param parameterFactory must not be {@literal null}.
80+
*/
81+
protected Parameters(Method method, Function<MethodParameter, T> parameterFactory) {
6782

6883
Assert.notNull(method, "Method must not be null");
6984

85+
// Factory nullability not enforced yet to support falling back to the deprecated
86+
// createParameter(MethodParameter). Add assertion when the deprecation is removed.
87+
7088
int parameterCount = method.getParameterCount();
7189

7290
this.parameters = new ArrayList<>(parameterCount);
@@ -80,7 +98,9 @@ public Parameters(Method method) {
8098
MethodParameter methodParameter = new MethodParameter(method, i);
8199
methodParameter.initParameterNameDiscovery(PARAMETER_NAME_DISCOVERER);
82100

83-
T parameter = createParameter(methodParameter);
101+
T parameter = parameterFactory == null //
102+
? createParameter(methodParameter) //
103+
: parameterFactory.apply(methodParameter);
84104

85105
if (parameter.isSpecialParameter() && parameter.isNamedParameter()) {
86106
throw new IllegalArgumentException(PARAM_ON_SPECIAL);
@@ -156,8 +176,13 @@ private S getBindable() {
156176
*
157177
* @param parameter will never be {@literal null}.
158178
* @return
179+
* @deprecated since 3.1, in your extension, call {@link #Parameters(Method, ParameterFactory)} instead.
159180
*/
160-
protected abstract T createParameter(MethodParameter parameter);
181+
@SuppressWarnings("unchecked")
182+
@Deprecated(since = "3.1", forRemoval = true)
183+
protected T createParameter(MethodParameter parameter) {
184+
return (T) new Parameter(parameter);
185+
}
161186

162187
/**
163188
* Returns whether the method the {@link Parameters} was created for contains a {@link Pageable} argument.
@@ -169,8 +194,8 @@ public boolean hasPageableParameter() {
169194
}
170195

171196
/**
172-
* Returns the index of the {@link Pageable} {@link Method} parameter if available. Will return {@literal -1} if there
173-
* is no {@link Pageable} argument in the {@link Method}'s parameter list.
197+
* Returns the index of the {@link Pageable} {@link Method} parameter if available. Will return {@literal -1} if
198+
* there is no {@link Pageable} argument in the {@link Method}'s parameter list.
174199
*
175200
* @return the pageableIndex
176201
*/
@@ -179,8 +204,8 @@ public int getPageableIndex() {
179204
}
180205

181206
/**
182-
* Returns the index of the {@link Sort} {@link Method} parameter if available. Will return {@literal -1} if there is
183-
* no {@link Sort} argument in the {@link Method}'s parameter list.
207+
* Returns the index of the {@link Sort} {@link Method} parameter if available. Will return {@literal -1} if there
208+
* is no {@link Sort} argument in the {@link Method}'s parameter list.
184209
*
185210
* @return
186211
*/
@@ -289,8 +314,8 @@ public S getBindableParameters() {
289314

290315
/**
291316
* Returns a bindable parameter with the given index. So for a method with a signature of
292-
* {@code (Pageable pageable, String name)} a call to {@code #getBindableParameter(0)} will return the {@link String}
293-
* parameter.
317+
* {@code (Pageable pageable, String name)} a call to {@code #getBindableParameter(0)} will return the
318+
* {@link String} parameter.
294319
*
295320
* @param bindableIndex
296321
* @return

src/main/java/org/springframework/data/repository/query/QueryMethod.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
7979

8080
this.method = method;
8181
this.unwrappedReturnType = potentiallyUnwrapReturnTypeFor(metadata, method);
82-
this.parameters = createParameters(method);
8382
this.metadata = metadata;
83+
this.parameters = createParameters(method);
8484

8585
if (hasParameterOfType(method, Pageable.class)) {
8686

@@ -107,7 +107,7 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
107107
Class<?> repositoryDomainClass = metadata.getDomainType();
108108
Class<?> methodDomainClass = metadata.getReturnedDomainClass(method);
109109

110-
return (repositoryDomainClass == null) || repositoryDomainClass.isAssignableFrom(methodDomainClass)
110+
return repositoryDomainClass == null || repositoryDomainClass.isAssignableFrom(methodDomainClass)
111111
? methodDomainClass
112112
: repositoryDomainClass;
113113
});
@@ -119,11 +119,24 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
119119
/**
120120
* Creates a {@link Parameters} instance.
121121
*
122-
* @param method
122+
* @param method must not be {@literal null}.
123123
* @return must not return {@literal null}.
124+
* @deprecated since 3.1, call or override {@link #createParameters(Method, TypeInformation)} instead.
124125
*/
126+
@Deprecated(since = "3.1", forRemoval = true)
125127
protected Parameters<?, ?> createParameters(Method method) {
126-
return new DefaultParameters(method);
128+
return createParameters(method, metadata.getDomainTypeInformation());
129+
}
130+
131+
/**
132+
* Creates a {@link Parameters} instance.
133+
*
134+
* @param method must not be {@literal null}.
135+
* @param aggregateType must not be {@literal null}.
136+
* @return must not return {@literal null}.
137+
*/
138+
protected Parameters<?, ?> createParameters(Method method, TypeInformation<?> aggregateType) {
139+
return new DefaultParameters(method, aggregateType);
127140
}
128141

129142
/**

src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,19 @@
2323
import java.util.stream.Stream;
2424

2525
import org.jetbrains.annotations.NotNull;
26+
import org.junit.jupiter.api.DynamicTest;
2627
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.api.TestFactory;
2729
import org.springframework.core.MethodParameter;
30+
import org.springframework.data.repository.query.ParametersUnitTests.User;
31+
import org.springframework.data.util.TypeInformation;
2832

2933
/**
3034
* Unit tests for {@link Parameter}.
3135
*
3236
* @author Jens Schauder
3337
*/
34-
class KParameterUnitTests {
38+
class ParameterUnitTests {
3539

3640
@Test // DATAJPA-1185
3741
void classParameterWithSameTypeParameterAsReturnedListIsDynamicProjectionParameter() throws Exception {
@@ -57,6 +61,28 @@ void classParameterWithSameTypeParameterAsReturnedOptionalIsDynamicProjectionPar
5761
assertThat(parameter.isDynamicProjectionParameter()).isTrue();
5862
}
5963

64+
@TestFactory // #2770
65+
Stream<DynamicTest> doesNotConsiderClassParametersDynamicProjectionOnes() {
66+
67+
var methods = Stream.of( //
68+
"genericReturnNonDynamicBind", //
69+
"staticReturnNonDynamicBindWildcard", //
70+
"staticReturnNonDynamicBindWildcardExtends");
71+
72+
return DynamicTest.stream(methods, it -> it, it -> {
73+
assertThat(new Parameter(getMethodParameter(it), TypeInformation.of(User.class))
74+
.isDynamicProjectionParameter()).isFalse();
75+
});
76+
}
77+
78+
@Test // #2770
79+
void doesNotConsiderAtParamAnnotatedClassParameterDynamicProjectionOne() throws Exception {
80+
81+
var parameter = new Parameter(getMethodParameter("atParamOnClass"));
82+
83+
assertThat(parameter.isDynamicProjectionParameter()).isFalse();
84+
}
85+
6086
@NotNull
6187
private MethodParameter getMethodParameter(String methodName) throws NoSuchMethodException {
6288
return new MethodParameter(this.getClass().getDeclaredMethod(methodName, Class.class), 0);
@@ -73,4 +99,20 @@ <T> Stream<T> dynamicProjectionWithStream(Class<T> type) {
7399
<T> Optional<T> dynamicProjectionWithOptional(Class<T> type) {
74100
return Optional.empty();
75101
}
102+
103+
<T> T genericReturnNonDynamicBind(Class<? extends User> one) {
104+
return null;
105+
}
106+
107+
User staticReturnNonDynamicBindWildcard(Class<?> two) {
108+
return null;
109+
}
110+
111+
User staticReturnNonDynamicBindWildcardExtends(Class<? extends User> one) {
112+
return null;
113+
}
114+
115+
<T> T atParamOnClass(@Param("type") Class<T> type) {
116+
return null;
117+
}
76118
}

0 commit comments

Comments
 (0)