Browse Source
Before this commit we haven't properly resolved methods on a root object provided by an EvaluationContextExtension that was using varargs. With a vararg method, the number of parameters handed into the method is not necessary equal to the number of parameters. We previously simply skipped methods with a different number of arguments. We now try direct matches first but calculate valid varargs alternatives in case that initial lookup fails and try to match those alternatives. This lookup is implemented in ….util.ParameterTypes now and used by ….spel.spi.Function. The latter now also handles the actual invocation of those methods properly by collecting the trailing arguments into an array.pull/420/head
7 changed files with 560 additions and 64 deletions
@ -0,0 +1,321 @@
@@ -0,0 +1,321 @@
|
||||
/* |
||||
* Copyright 2019 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 |
||||
* |
||||
* http://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.util; |
||||
|
||||
import lombok.EqualsAndHashCode; |
||||
import lombok.RequiredArgsConstructor; |
||||
|
||||
import java.lang.reflect.Method; |
||||
import java.util.ArrayList; |
||||
import java.util.Arrays; |
||||
import java.util.Collection; |
||||
import java.util.Collections; |
||||
import java.util.List; |
||||
import java.util.Optional; |
||||
import java.util.concurrent.ConcurrentMap; |
||||
import java.util.stream.Collectors; |
||||
|
||||
import org.springframework.core.convert.TypeDescriptor; |
||||
import org.springframework.util.Assert; |
||||
import org.springframework.util.ConcurrentReferenceHashMap; |
||||
import org.springframework.util.TypeUtils; |
||||
|
||||
/** |
||||
* Abstraction over a list of parameter value types. Allows to check whether a list of parameter values with the given |
||||
* type setup is a candidate for the invocation of a given {@link Method} (see {@link #areValidFor(Method)}). This is |
||||
* necessary to properly match parameter values against methods declaring varargs arguments. The implementation favors |
||||
* direct matches and only computes the alternative sets of types to be considered if the primary one doesn't match. |
||||
* |
||||
* @author Oliver Drotbohm |
||||
* @since 2.1.7 |
||||
* @soundtrack Signs, High Times - Tedeschi Trucks Band (Signs) |
||||
*/ |
||||
@EqualsAndHashCode(of = "types") |
||||
@RequiredArgsConstructor |
||||
public class ParameterTypes { |
||||
|
||||
private static final TypeDescriptor OBJECT_DESCRIPTOR = TypeDescriptor.valueOf(Object.class); |
||||
private static final ConcurrentMap<List<TypeDescriptor>, ParameterTypes> CACHE = new ConcurrentReferenceHashMap<>(); |
||||
|
||||
private final List<TypeDescriptor> types; |
||||
private final Lazy<Collection<ParameterTypes>> alternatives; |
||||
|
||||
/** |
||||
* Creates a new {@link ParameterTypes} for the given types. |
||||
* |
||||
* @param types |
||||
*/ |
||||
private ParameterTypes(List<TypeDescriptor> types) { |
||||
|
||||
this.types = types; |
||||
this.alternatives = Lazy.of(() -> getAlternatives()); |
||||
} |
||||
|
||||
/** |
||||
* Returns the {@link ParameterTypes} for the given list of {@link TypeDescriptor}s. |
||||
* |
||||
* @param types must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
public static ParameterTypes of(List<TypeDescriptor> types) { |
||||
|
||||
Assert.notNull(types, "Types must not be null!"); |
||||
|
||||
return CACHE.computeIfAbsent(types, ParameterTypes::new); |
||||
} |
||||
|
||||
/** |
||||
* Returns the {@link ParameterTypes} for the given {@link Class}es. |
||||
* |
||||
* @param types must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
static ParameterTypes of(Class<?>... types) { |
||||
|
||||
Assert.notNull(types, "Types must not be null!"); |
||||
Assert.noNullElements(types, "Types must not have null elements!"); |
||||
|
||||
return of(Arrays.stream(types) //
|
||||
.map(TypeDescriptor::valueOf) //
|
||||
.collect(Collectors.toList())); |
||||
} |
||||
|
||||
/** |
||||
* Returns whether the parameter types are valid for the given {@link Method}. That means, a parameter value list with |
||||
* the given type arrangement is a valid list to invoke the given method. |
||||
* |
||||
* @param method must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
public boolean areValidFor(Method method) { |
||||
|
||||
Assert.notNull(method, "Method must not be null!"); |
||||
|
||||
// Direct matches
|
||||
if (areValidTypes(method)) { |
||||
return true; |
||||
} |
||||
|
||||
return hasValidAlternativeFor(method); |
||||
} |
||||
|
||||
/** |
||||
* Returns whether we have a valid alternative variant (making use of varargs) that will match the given method's |
||||
* signature. |
||||
* |
||||
* @param method |
||||
* @return |
||||
*/ |
||||
private boolean hasValidAlternativeFor(Method method) { |
||||
|
||||
return alternatives.get().stream().anyMatch(it -> it.areValidTypes(method)) //
|
||||
|| getParent().map(parent -> parent.hasValidAlternativeFor(method)).orElse(false); |
||||
} |
||||
|
||||
/** |
||||
* Returns all suitable alternatives to the current {@link ParameterTypes}. |
||||
* |
||||
* @return will never be {@literal null}. |
||||
*/ |
||||
List<ParameterTypes> getAllAlternatives() { |
||||
|
||||
List<ParameterTypes> result = new ArrayList<>(); |
||||
result.addAll(alternatives.get()); |
||||
|
||||
getParent().ifPresent(it -> result.addAll(it.getAllAlternatives())); |
||||
|
||||
return result; |
||||
} |
||||
|
||||
/** |
||||
* Returns whether the {@link ParameterTypes} consists of the given types. |
||||
* |
||||
* @param types must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
boolean hasTypes(Class<?>... types) { |
||||
|
||||
Assert.notNull(types, "Types must not be null!"); |
||||
|
||||
return Arrays.stream(types) //
|
||||
.map(TypeDescriptor::valueOf) //
|
||||
.collect(Collectors.toList())//
|
||||
.equals(this.types); |
||||
} |
||||
|
||||
/** |
||||
* Returns whether the current parameter types match the given {@link Method}'s parameters exactly, i.e. they're |
||||
* equal, not only assignable. |
||||
* |
||||
* @param method must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
public boolean exactlyMatchParametersOf(Method method) { |
||||
|
||||
if (method.getParameterCount() != types.size()) { |
||||
return false; |
||||
} |
||||
|
||||
Class<?>[] parameterTypes = method.getParameterTypes(); |
||||
|
||||
for (int i = 0; i < parameterTypes.length; i++) { |
||||
if (parameterTypes[i] != types.get(i).getType()) { |
||||
return false; |
||||
} |
||||
} |
||||
|
||||
return true; |
||||
} |
||||
|
||||
/* |
||||
* (non-Javadoc) |
||||
* @see java.lang.Object#toString() |
||||
*/ |
||||
@Override |
||||
public String toString() { |
||||
|
||||
return types.stream() //
|
||||
.map(TypeDescriptor::getType) //
|
||||
.map(Class::getSimpleName) //
|
||||
.collect(Collectors.joining(", ", "(", ")")); |
||||
} |
||||
|
||||
protected Optional<ParameterTypes> getParent() { |
||||
return types.isEmpty() ? Optional.empty() : getParent(getTail()); |
||||
} |
||||
|
||||
protected final Optional<ParameterTypes> getParent(TypeDescriptor tail) { |
||||
|
||||
return types.size() <= 1 //
|
||||
? Optional.empty() //
|
||||
: Optional.of(ParentParameterTypes.of(types.subList(0, types.size() - 1), tail)); |
||||
} |
||||
|
||||
protected Optional<ParameterTypes> withLastVarArgs() { |
||||
|
||||
TypeDescriptor lastDescriptor = types.get(types.size() - 1); |
||||
|
||||
return lastDescriptor.isArray() //
|
||||
? Optional.empty() //
|
||||
: Optional.ofNullable(withVarArgs(lastDescriptor)); |
||||
} |
||||
|
||||
@SuppressWarnings("null") |
||||
private ParameterTypes withVarArgs(TypeDescriptor descriptor) { |
||||
|
||||
TypeDescriptor lastDescriptor = types.get(types.size() - 1); |
||||
|
||||
if (lastDescriptor.isArray() && lastDescriptor.getElementTypeDescriptor().equals(descriptor)) { |
||||
return this; |
||||
} |
||||
|
||||
List<TypeDescriptor> result = new ArrayList<>(types.subList(0, types.size() - 1)); |
||||
result.add(TypeDescriptor.array(descriptor)); |
||||
|
||||
return ParameterTypes.of(result); |
||||
} |
||||
|
||||
private Collection<ParameterTypes> getAlternatives() { |
||||
|
||||
if (types.isEmpty()) { |
||||
return Collections.emptyList(); |
||||
} |
||||
|
||||
List<ParameterTypes> alternatives = new ArrayList<>(); |
||||
|
||||
withLastVarArgs().ifPresent(alternatives::add); |
||||
|
||||
ParameterTypes objectVarArgs = withVarArgs(OBJECT_DESCRIPTOR); |
||||
|
||||
if (!alternatives.contains(objectVarArgs)) { |
||||
alternatives.add(objectVarArgs); |
||||
} |
||||
|
||||
return alternatives; |
||||
} |
||||
|
||||
/** |
||||
* Returns whether the current type list makes up valid arguments for the given method. |
||||
* |
||||
* @param method must not be {@literal null}. |
||||
* @return |
||||
*/ |
||||
private boolean areValidTypes(Method method) { |
||||
|
||||
Assert.notNull(method, "Method must not be null!"); |
||||
|
||||
if (method.getParameterCount() != types.size()) { |
||||
return false; |
||||
} |
||||
|
||||
Class<?>[] parameterTypes = method.getParameterTypes(); |
||||
|
||||
for (int i = 0; i < parameterTypes.length; i++) { |
||||
if (!TypeUtils.isAssignable(parameterTypes[i], types.get(i).getType())) { |
||||
return false; |
||||
} |
||||
} |
||||
|
||||
return true; |
||||
} |
||||
|
||||
private TypeDescriptor getTail() { |
||||
return types.get(types.size() - 1); |
||||
} |
||||
|
||||
/** |
||||
* Extension of {@link ParameterTypes} that remembers the seed tail and only adds typed varargs if the current tail is |
||||
* assignable to the seed one. |
||||
* |
||||
* @author Oliver Drotbohm |
||||
*/ |
||||
@EqualsAndHashCode(callSuper = true) |
||||
static class ParentParameterTypes extends ParameterTypes { |
||||
|
||||
private final TypeDescriptor tail; |
||||
|
||||
private ParentParameterTypes(List<TypeDescriptor> types, TypeDescriptor tail) { |
||||
|
||||
super(types); |
||||
this.tail = tail; |
||||
} |
||||
|
||||
public static ParentParameterTypes of(List<TypeDescriptor> types, TypeDescriptor tail) { |
||||
return new ParentParameterTypes(types, tail); |
||||
} |
||||
|
||||
/* |
||||
* (non-Javadoc) |
||||
* @see org.springframework.data.util.ParameterTypes#getParent() |
||||
*/ |
||||
@Override |
||||
protected Optional<ParameterTypes> getParent() { |
||||
return super.getParent(tail); |
||||
} |
||||
|
||||
/* |
||||
* (non-Javadoc) |
||||
* @see org.springframework.data.util.ParameterTypes#withLastVarArgs() |
||||
*/ |
||||
@Override |
||||
protected Optional<ParameterTypes> withLastVarArgs() { |
||||
|
||||
return !tail.isAssignableTo(super.getTail()) //
|
||||
? Optional.empty() //
|
||||
: super.withLastVarArgs(); |
||||
} |
||||
} |
||||
} |
||||
@ -0,0 +1,68 @@
@@ -0,0 +1,68 @@
|
||||
/* |
||||
* Copyright 2019 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 |
||||
* |
||||
* http://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.spel.spi; |
||||
|
||||
import static org.assertj.core.api.Assertions.*; |
||||
|
||||
import java.lang.reflect.Method; |
||||
import java.util.Arrays; |
||||
|
||||
import org.junit.Test; |
||||
import org.springframework.core.convert.TypeDescriptor; |
||||
import org.springframework.util.ReflectionUtils; |
||||
|
||||
/** |
||||
* Unit tests for {@link Function}. |
||||
* |
||||
* @author Oliver Drotbohm |
||||
*/ |
||||
public class FunctionUnitTests { |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void detectsVarArgsOverload() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "someMethod", String[].class); |
||||
|
||||
Function function = new Function(method, new Sample()); |
||||
|
||||
TypeDescriptor stringDescriptor = TypeDescriptor.valueOf(String.class); |
||||
|
||||
assertThat(function.supports(Arrays.asList(stringDescriptor, stringDescriptor))).isTrue(); |
||||
} |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void detectsObjectVarArgsOverload() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "onePlusObjectVarargs", String.class, Object[].class); |
||||
|
||||
Function function = new Function(method, new Sample()); |
||||
|
||||
TypeDescriptor stringDescriptor = TypeDescriptor.valueOf(String.class); |
||||
|
||||
assertThat(function.supports(Arrays.asList(stringDescriptor, stringDescriptor))).isTrue(); |
||||
} |
||||
|
||||
class Sample { |
||||
|
||||
String someMethod(String... args) { |
||||
return "result"; |
||||
} |
||||
|
||||
String onePlusObjectVarargs(String string, Object... args) { |
||||
return null; |
||||
} |
||||
} |
||||
} |
||||
@ -0,0 +1,105 @@
@@ -0,0 +1,105 @@
|
||||
/* |
||||
* Copyright 2019 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 |
||||
* |
||||
* http://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.util; |
||||
|
||||
import static org.assertj.core.api.Assertions.*; |
||||
|
||||
import java.lang.reflect.Method; |
||||
import java.util.List; |
||||
|
||||
import org.junit.Test; |
||||
import org.springframework.util.ReflectionUtils; |
||||
|
||||
/** |
||||
* Unit test for {@link ParameterTypes}. |
||||
* |
||||
* @author Oliver Drotbohm |
||||
*/ |
||||
public class ParameterTypesUnitTests { |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void detectsDirectMatch() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "twoStrings", String.class, String.class); |
||||
|
||||
ParameterTypes types = ParameterTypes.of(String.class, String.class); |
||||
|
||||
assertThat(types.areValidFor(method)).isTrue(); |
||||
assertThat(types.exactlyMatchParametersOf(method)).isTrue(); |
||||
} |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void supportsSimpleVarArg() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "stringPlusStringVarArg", String.class, String[].class); |
||||
|
||||
ParameterTypes types = ParameterTypes.of(String.class, String.class); |
||||
|
||||
assertThat(types.areValidFor(method)).isTrue(); |
||||
assertThat(types.exactlyMatchParametersOf(method)).isFalse(); |
||||
} |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void supportsTrailingObjectVarArg() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "stringPlusObjectVarArg", String.class, Object[].class); |
||||
|
||||
ParameterTypes types = ParameterTypes.of(String.class, String.class); |
||||
|
||||
assertThat(types.areValidFor(method)).isTrue(); |
||||
assertThat(types.exactlyMatchParametersOf(method)).isFalse(); |
||||
} |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void supportsObjectVarArg() { |
||||
|
||||
Method method = ReflectionUtils.findMethod(Sample.class, "objectVarArg", Object[].class); |
||||
|
||||
ParameterTypes types = ParameterTypes.of(String.class, String.class); |
||||
|
||||
assertThat(types.areValidFor(method)).isTrue(); |
||||
assertThat(types.exactlyMatchParametersOf(method)).isFalse(); |
||||
|
||||
} |
||||
|
||||
@Test // DATACMNS-1518
|
||||
public void doesNotAddNonObjectVarArgsForParents() { |
||||
|
||||
ParameterTypes types = ParameterTypes.of(String.class, String.class, Integer.class, Integer.class); |
||||
|
||||
List<ParameterTypes> alternatives = types.getAllAlternatives(); |
||||
|
||||
assertThat(alternatives).hasSize(6); |
||||
|
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(String.class, String.class, Integer.class, Integer[].class)); |
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(String.class, String.class, Integer.class, Object[].class)); |
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(String.class, String.class, Integer[].class)); |
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(String.class, String.class, Object[].class)); |
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(String.class, Object[].class)); |
||||
assertThat(alternatives).anyMatch(it -> it.hasTypes(Object[].class)); |
||||
} |
||||
|
||||
interface Sample { |
||||
|
||||
void twoStrings(String first, String second); |
||||
|
||||
void stringPlusStringVarArg(String first, String... second); |
||||
|
||||
void stringPlusObjectVarArg(String first, Object... second); |
||||
|
||||
void objectVarArg(Object... args); |
||||
} |
||||
} |
||||
Loading…
Reference in new issue