Skip to content

Introduce MethodParameterFactory to Parameters #2996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>3.3.0-SNAPSHOT</version>
<version>3.3.0-GH-2995-SNAPSHOT</version>

<name>Spring Data Core</name>
<description>Core Spring concepts underpinning every Spring Data module.</description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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 //
Expand Down Expand Up @@ -421,7 +440,9 @@ public static boolean isBindable(Class<?> type) {
return !TYPES.contains(type);
}

@Override
public Iterator<T> iterator() {
return parameters.iterator();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superfluos?

}
Original file line number Diff line number Diff line change
@@ -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();

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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" });

Expand All @@ -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" });

Expand All @@ -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" });

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand Down
Loading