Introduce ParametersSource to Parameters.

mp911de · mp911de · commit 39697c7f93e0 · 2023-12-07T16:15:56.000+01:00
We now provide a ParametersSource object to create MethodParameter objects associated with the enclosing class so parameters can resolve generics properly.
diff --git a/src/main/java/org/springframework/data/repository/query/DefaultParameters.java b/src/main/java/org/springframework/data/repository/query/DefaultParameters.java
@@ -39,14 +39,13 @@ public DefaultParameters(Method method) {
 	}
 
 	/**
-	 * Creates a new {@link DefaultParameters} instance from the given {@link Method} and aggregate
-	 * {@link TypeInformation}.
+	 * Creates a new {@link DefaultParameters} instance from the given {@link ParametersSource}.
 	 *
-	 * @param method must not be {@literal null}.
-	 * @param aggregateType must not be {@literal null}.
+	 * @param parametersSource must not be {@literal null}.
+	 * @since 3.2.1
 	 */
-	public DefaultParameters(Method method, TypeInformation<?> aggregateType) {
-		super(method, param -> new Parameter(param, aggregateType));
+	public DefaultParameters(ParametersSource parametersSource) {
+		super(parametersSource, param -> new Parameter(param, parametersSource.getDomainType()));
 	}
 
 	private DefaultParameters(List<Parameter> parameters) {
diff --git a/src/main/java/org/springframework/data/repository/query/Parameters.java b/src/main/java/org/springframework/data/repository/query/Parameters.java
@@ -44,7 +44,8 @@
  */
 public abstract class Parameters<S extends Parameters<S, T>, T extends Parameter> implements Streamable<T> {
 
-	public static final List<Class<?>> TYPES = Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class, Limit.class);
+	public static final List<Class<?>> TYPES = Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class,
+			Limit.class);
 
 	private static final String PARAM_ON_SPECIAL = format("You must not use @%s on a parameter typed %s or %s",
 			Param.class.getSimpleName(), Pageable.class.getSimpleName(), Sort.class.getSimpleName());
@@ -82,14 +83,30 @@ public Parameters(Method method) {
 	 * @param method must not be {@literal null}.
 	 * @param parameterFactory must not be {@literal null}.
 	 * @since 3.0.2
+	 * @deprecated since 3.2.1, use {@link Parameters(ParametersSource, Function)} instead.
 	 */
+	@Deprecated(since = "3.2.1", forRemoval = true)
 	protected Parameters(Method method, Function<MethodParameter, T> parameterFactory) {
+		this(ParametersSource.of(method), parameterFactory);
+	}
+
+	/**
+	 * 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 parametersSource must not be {@literal null}.
+	 * @param parameterFactory must not be {@literal null}.
+	 * @since 3.2.1
+	 */
+	protected Parameters(ParametersSource parametersSource,
+			Function<MethodParameter, T> parameterFactory) {
 
-		Assert.notNull(method, "Method must not be null");
+		Assert.notNull(parametersSource, "ParametersSource 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.
 
+		Method method = parametersSource.getMethod();
 		int parameterCount = method.getParameterCount();
 
 		this.parameters = new ArrayList<>(parameterCount);
@@ -102,7 +119,9 @@ protected Parameters(Method method, Function<MethodParameter, T> parameterFactor
 
 		for (int i = 0; i < parameterCount; i++) {
 
-			MethodParameter methodParameter = new MethodParameter(method, i);
+			MethodParameter methodParameter = new MethodParameter(method, i)
+					.withContainingClass(parametersSource.getContainingClass());
+
 			methodParameter.initParameterNameDiscovery(PARAMETER_NAME_DISCOVERER);
 
 			T parameter = parameterFactory == null //
@@ -421,7 +440,9 @@ public static boolean isBindable(Class<?> type) {
 		return !TYPES.contains(type);
 	}
 
+	@Override
 	public Iterator<T> iterator() {
 		return parameters.iterator();
 	}
+
 }
diff --git a/src/main/java/org/springframework/data/repository/query/ParametersSource.java b/src/main/java/org/springframework/data/repository/query/ParametersSource.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright 2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.repository.query;
+
+import java.lang.reflect.Method;
+
+import org.springframework.data.repository.core.RepositoryMetadata;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.util.Assert;
+
+/**
+ * Interface providing access to the method, containing class and domain type as source object for parameter
+ * descriptors.
+ *
+ * @author Mark Paluch
+ * @since 3.2.1
+ */
+public interface ParametersSource {
+
+	/**
+	 * Create a new parameter source for the given {@link Method}.
+	 *
+	 * @param method the method to use.
+	 * @return a new parameter source for the given {@link Method}.
+	 */
+	static ParametersSource of(Method method) {
+
+		Assert.notNull(method, "Method must not be null");
+
+		return new ParametersSource() {
+
+			@Override
+			public Class<?> getContainingClass() {
+				return method.getDeclaringClass();
+			}
+
+			@Override
+			public Method getMethod() {
+				return method;
+			}
+
+			@Override
+			public TypeInformation<?> getDomainType() {
+				return TypeInformation.OBJECT;
+			}
+		};
+	}
+
+	/**
+	 * Create a new parameter source for the given {@link Method} in the context of {@link RepositoryMetadata}.
+	 *
+	 * @param method the method to use.
+	 * @return a new parameter source for the given {@link Method}.
+	 */
+	static ParametersSource of(RepositoryMetadata metadata, Method method) {
+
+		Assert.notNull(metadata, "RepositoryMetadata must not be null");
+		Assert.notNull(method, "Method must not be null");
+
+		if (!method.getDeclaringClass().isAssignableFrom(metadata.getRepositoryInterface())) {
+			throw new IllegalArgumentException(
+					"Method " + method + " is not defined in the type hierarchy of " + metadata.getRepositoryInterface());
+		}
+
+		return new ParametersSource() {
+
+			@Override
+			public Class<?> getContainingClass() {
+				return metadata.getRepositoryInterface();
+			}
+
+			@Override
+			public Method getMethod() {
+				return method;
+			}
+
+			@Override
+			public TypeInformation<?> getDomainType() {
+				return metadata.getDomainTypeInformation();
+			}
+		};
+	}
+
+	/**
+	 * @return the class that contains the {@link #getMethod()}.
+	 */
+	Class<?> getContainingClass();
+
+	/**
+	 * @return the method for which the parameter source is defined.
+	 */
+	Method getMethod();
+
+	/**
+	 * @return the domain type associated with the parameter source.
+	 */
+	TypeInformation<?> getDomainType();
+
+}
diff --git a/src/main/java/org/springframework/data/repository/query/QueryMethod.java b/src/main/java/org/springframework/data/repository/query/QueryMethod.java
@@ -183,16 +183,29 @@ private boolean calculateIsCollectionQuery() {
 	 * @param method must not be {@literal null}.
 	 * @param domainType must not be {@literal null}.
 	 * @return must not return {@literal null}.
+	 * @deprecated since 3.2.1
 	 * @since 3.0.2
 	 */
+	@Deprecated(since = "3.2.1", forRemoval = true)
 	protected Parameters<?, ?> createParameters(Method method, TypeInformation<?> domainType) {
-		return new DefaultParameters(method, domainType);
+		return createParameters(ParametersSource.of(getMetadata(), method));
+	}
+
+	/**
+	 * Creates a {@link Parameters} instance.
+	 *
+	 * @param parametersSource must not be {@literal null}.
+	 * @return must not return {@literal null}.
+	 * @since 3.2.1
+	 */
+	protected Parameters<?, ?> createParameters(ParametersSource parametersSource) {
+		return new DefaultParameters(parametersSource);
 	}
 
 	/**
 	 * Returns the method's name.
 	 *
-	 * @return
+	 * @return the method's name.
 	 */
 	public String getName() {
 		return method.getName();
diff --git a/src/test/java/org/springframework/data/repository/core/support/RepositoryMethodInvokerUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/RepositoryMethodInvokerUnitTests.java
@@ -114,7 +114,8 @@ void capturesImperativeDurationCorrectly() throws Exception {
 
 		repositoryMethodInvoker("findAll").invoke();
 
-		assertThat(multicaster.first().getDuration(TimeUnit.MILLISECONDS)).isCloseTo(250, Percentage.withPercentage(10));
+		assertThat(multicaster.first()
+				.getDuration(TimeUnit.MILLISECONDS)).isCloseTo(500, Percentage.withPercentage(50));
 	}
 
 	@Test // DATACMNS-1764
@@ -124,7 +125,8 @@ void capturesReactiveDurationCorrectly() throws Exception {
 
 		repositoryMethodInvokerForReactive("findAll").<Flux<TestDummy>> invoke().subscribe();
 
-		assertThat(multicaster.first().getDuration(TimeUnit.MILLISECONDS)).isCloseTo(250, Percentage.withPercentage(10));
+		assertThat(multicaster.first()
+				.getDuration(TimeUnit.MILLISECONDS)).isCloseTo(500, Percentage.withPercentage(50));
 	}
 
 	@Test // DATACMNS-1764
diff --git a/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java
@@ -25,6 +25,7 @@
 import org.springframework.data.domain.ScrollPosition;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.repository.core.RepositoryMetadata;
 
 /**
  * Unit tests for {@link ParametersParameterAccessor}.
@@ -36,10 +37,11 @@
 class ParametersParameterAccessorUnitTests {
 
 	Parameters<?, ?> parameters;
+	RepositoryMetadata metadata;
 
 	@BeforeEach
 	void setUp() throws Exception {
-		parameters = new DefaultParameters(Sample.class.getMethod("method", String.class, int.class));
+		parameters = new DefaultParameters(ParametersSource.of(Sample.class.getMethod("method", String.class, int.class)));
 	}
 
 	@Test
@@ -62,7 +64,7 @@ void detectsNullValue() throws Exception {
 		assertThat(accessor.hasBindableNullValue()).isTrue();
 
 		var method = Sample.class.getMethod("method", Pageable.class, String.class);
-		var parameters = new DefaultParameters(method);
+		var parameters = new DefaultParameters(ParametersSource.of(method));
 
 		accessor = new ParametersParameterAccessor(parameters, new Object[] { null, "Foo" });
 		assertThat(accessor.hasBindableNullValue()).isFalse();
@@ -72,7 +74,7 @@ void detectsNullValue() throws Exception {
 	void iteratesonlyOverBindableValues() throws Exception {
 
 		var method = Sample.class.getMethod("method", Pageable.class, String.class);
-		var parameters = new DefaultParameters(method);
+		var parameters = new DefaultParameters(ParametersSource.of(method));
 
 		var accessor = new ParametersParameterAccessor(parameters, new Object[] { PageRequest.of(0, 10), "Foo" });
 
@@ -84,7 +86,7 @@ void iteratesonlyOverBindableValues() throws Exception {
 	void handlesScrollPositionAsAParameterType() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", ScrollPosition.class, String.class);
-		var parameters = new DefaultParameters(method);
+		var parameters = new DefaultParameters(ParametersSource.of(method));
 
 		var accessor = new ParametersParameterAccessor(parameters, new Object[] { ScrollPosition.offset(1), "Foo" });
 
@@ -96,7 +98,7 @@ void handlesScrollPositionAsAParameterType() throws NoSuchMethodException {
 	void handlesPageRequestAsAParameterType() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("methodWithPageRequest", PageRequest.class, String.class);
-		var parameters = new DefaultParameters(method);
+		var parameters = new DefaultParameters(ParametersSource.of(method));
 
 		var accessor = new ParametersParameterAccessor(parameters, new Object[] { PageRequest.of(0, 10), "Foo" });
 
@@ -108,7 +110,7 @@ void handlesPageRequestAsAParameterType() throws NoSuchMethodException {
 	void handlesLimitAsAParameterType() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Limit.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Limit.of(100), "spring" });
 
 		assertThat(accessor).hasSize(1);
@@ -119,7 +121,7 @@ void handlesLimitAsAParameterType() throws NoSuchMethodException {
 	void returnsLimitIfAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Limit.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Limit.of(100), "spring" });
 
 		assertThat(accessor.getLimit()).extracting(Limit::max).isEqualTo(100);
@@ -129,7 +131,7 @@ void returnsLimitIfAvailable() throws NoSuchMethodException {
 	void readsLimitFromPageableIfAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Pageable.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Pageable.ofSize(100), "spring" });
 
 		assertThat(accessor.getLimit()).extracting(Limit::max).isEqualTo(100);
@@ -139,7 +141,7 @@ void readsLimitFromPageableIfAvailable() throws NoSuchMethodException {
 	void returnsUnlimitedIfNoLimitingAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Sort.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Pageable.ofSize(100), "spring" });
 
 		assertThat(accessor.getLimit().isUnlimited()).isTrue();
@@ -149,7 +151,7 @@ void returnsUnlimitedIfNoLimitingAvailable() throws NoSuchMethodException {
 	void appliesLimitToPageableIfAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Limit.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Limit.of(100), "spring" });
 
 		Pageable pageable = accessor.getPageable();
@@ -161,7 +163,7 @@ void appliesLimitToPageableIfAvailable() throws NoSuchMethodException {
 	void appliesLimitToPageableIfRequested() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Limit.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Limit.of(100), "spring" });
 
 		assertThat(accessor).hasSize(1);
@@ -172,7 +174,7 @@ void appliesLimitToPageableIfRequested() throws NoSuchMethodException {
 	void appliesSortToPageableIfAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Sort.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Sort.by("one", "two"), "spring" });
 
 		Pageable pageable = accessor.getPageable();
@@ -184,7 +186,7 @@ void appliesSortToPageableIfAvailable() throws NoSuchMethodException {
 	void appliesSortAndLimitToPageableIfAvailable() throws NoSuchMethodException {
 
 		var method = Sample.class.getMethod("method", Sort.class, Limit.class, String.class);
-		var accessor = new ParametersParameterAccessor(new DefaultParameters(method),
+		var accessor = new ParametersParameterAccessor(new DefaultParameters(ParametersSource.of(method)),
 				new Object[] { Sort.by("one", "two"), Limit.of(42), "spring" });
 
 		Pageable pageable = accessor.getPageable();
diff --git a/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java

Original file line number	Diff line number	Diff line change
`@@ -39,14 +39,13 @@ public DefaultParameters(Method method) {`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`/**`
`42`		`- * Creates a new {@link DefaultParameters} instance from the given {@link Method} and aggregate`
`43`		`- * {@link TypeInformation}.`
	`42`	`+ * Creates a new {@link DefaultParameters} instance from the given {@link ParametersSource}.`
`44`	`43`	`*`
`45`		`- * @param method must not be {@literal null}.`
`46`		`- * @param aggregateType must not be {@literal null}.`
	`44`	`+ * @param parametersSource must not be {@literal null}.`
	`45`	`+ * @since 3.2.1`
`47`	`46`	`*/`
`48`		`- public DefaultParameters(Method method, TypeInformation<?> aggregateType) {`
`49`		`- super(method, param -> new Parameter(param, aggregateType));`
	`47`	`+ public DefaultParameters(ParametersSource parametersSource) {`
	`48`	`+ super(parametersSource, param -> new Parameter(param, parametersSource.getDomainType()));`
`50`	`49`	`}`
`51`	`50`
`52`	`51`	`private DefaultParameters(List<Parameter> parameters) {`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,8 @@ void capturesImperativeDurationCorrectly() throws Exception {`
`114`	`114`
`115`	`115`	`repositoryMethodInvoker("findAll").invoke();`
`116`	`116`
`117`		`- assertThat(multicaster.first().getDuration(TimeUnit.MILLISECONDS)).isCloseTo(250, Percentage.withPercentage(10));`
	`117`	`+ assertThat(multicaster.first()`
	`118`	`+ .getDuration(TimeUnit.MILLISECONDS)).isCloseTo(500, Percentage.withPercentage(50));`
`118`	`119`	`}`
`119`	`120`
`120`	`121`	`@Test // DATACMNS-1764`
`@@ -124,7 +125,8 @@ void capturesReactiveDurationCorrectly() throws Exception {`
`124`	`125`
`125`	`126`	`repositoryMethodInvokerForReactive("findAll").<Flux<TestDummy>> invoke().subscribe();`
`126`	`127`
`127`		`- assertThat(multicaster.first().getDuration(TimeUnit.MILLISECONDS)).isCloseTo(250, Percentage.withPercentage(10));`
	`128`	`+ assertThat(multicaster.first()`
	`129`	`+ .getDuration(TimeUnit.MILLISECONDS)).isCloseTo(500, Percentage.withPercentage(50));`
`128`	`130`	`}`
`129`	`131`
`130`	`132`	`@Test // DATACMNS-1764`