DATACMNS-1518 - Fixed detection of varargs overloads for SpEL expressions.
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.
This commit is contained in:
@@ -34,7 +34,6 @@ import java.util.Optional;
|
||||
|
||||
import org.springframework.beans.BeanUtils;
|
||||
import org.springframework.data.spel.EvaluationContextExtensionInformation.ExtensionTypeInformation.PublicMethodAndFieldFilter;
|
||||
import org.springframework.data.spel.Functions.NameAndArgumentCount;
|
||||
import org.springframework.data.spel.spi.EvaluationContextExtension;
|
||||
import org.springframework.data.spel.spi.Function;
|
||||
import org.springframework.data.util.Streamable;
|
||||
@@ -125,7 +124,7 @@ class EvaluationContextExtensionInformation {
|
||||
*
|
||||
* @return the functions will never be {@literal null}.
|
||||
*/
|
||||
private final MultiValueMap<NameAndArgumentCount, Function> functions;
|
||||
private final MultiValueMap<String, Function> functions;
|
||||
|
||||
/**
|
||||
* Creates a new {@link ExtensionTypeInformation} fir the given type.
|
||||
@@ -140,12 +139,12 @@ class EvaluationContextExtensionInformation {
|
||||
this.properties = discoverDeclaredProperties(type);
|
||||
}
|
||||
|
||||
private static MultiValueMap<NameAndArgumentCount, Function> discoverDeclaredFunctions(Class<?> type) {
|
||||
private static MultiValueMap<String, Function> discoverDeclaredFunctions(Class<?> type) {
|
||||
|
||||
MultiValueMap<NameAndArgumentCount, Function> map = CollectionUtils.toMultiValueMap(new HashMap<>());
|
||||
MultiValueMap<String, Function> map = CollectionUtils.toMultiValueMap(new HashMap<>());
|
||||
|
||||
ReflectionUtils.doWithMethods(type, //
|
||||
method -> map.add(NameAndArgumentCount.of(method), new Function(method, null)), //
|
||||
method -> map.add(method.getName(), new Function(method, null)), //
|
||||
PublicMethodAndFieldFilter.STATIC);
|
||||
|
||||
return CollectionUtils.unmodifiableMultiValueMap(map);
|
||||
@@ -243,12 +242,12 @@ class EvaluationContextExtensionInformation {
|
||||
* @param target can be {@literal null}.
|
||||
* @return the methods
|
||||
*/
|
||||
public MultiValueMap<NameAndArgumentCount, Function> getFunctions(Optional<Object> target) {
|
||||
public MultiValueMap<String, Function> getFunctions(Optional<Object> target) {
|
||||
return target.map(this::getFunctions).orElseGet(() -> new LinkedMultiValueMap<>());
|
||||
}
|
||||
|
||||
private MultiValueMap<NameAndArgumentCount, Function> getFunctions(Object target) {
|
||||
return methods.stream().collect(toMultiMap(NameAndArgumentCount::of, m -> new Function(m, target)));
|
||||
private MultiValueMap<String, Function> getFunctions(Object target) {
|
||||
return methods.stream().collect(toMultiMap(Method::getName, m -> new Function(m, target)));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -15,11 +15,6 @@
|
||||
*/
|
||||
package org.springframework.data.spel;
|
||||
|
||||
import lombok.AccessLevel;
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.Value;
|
||||
|
||||
import java.lang.reflect.Method;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -46,22 +41,21 @@ class Functions {
|
||||
private static final String MESSAGE_TEMPLATE = "There are multiple matching methods of name '%s' for parameter types (%s), but no "
|
||||
+ "exact match. Make sure to provide only one matching overload or one with exactly those types.";
|
||||
|
||||
private final MultiValueMap<NameAndArgumentCount, Function> functions = new LinkedMultiValueMap<>();
|
||||
private final MultiValueMap<String, Function> functions = new LinkedMultiValueMap<>();
|
||||
|
||||
void addAll(Map<String, Function> newFunctions) {
|
||||
|
||||
newFunctions.forEach((n, f) -> {
|
||||
|
||||
NameAndArgumentCount k = NameAndArgumentCount.of(n, f.getParameterCount());
|
||||
List<Function> currentElements = get(k);
|
||||
List<Function> currentElements = get(n);
|
||||
|
||||
if (!contains(currentElements, f)) {
|
||||
functions.add(k, f);
|
||||
functions.add(n, f);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
void addAll(MultiValueMap<NameAndArgumentCount, Function> newFunctions) {
|
||||
void addAll(MultiValueMap<String, Function> newFunctions) {
|
||||
|
||||
newFunctions.forEach((k, list) -> {
|
||||
|
||||
@@ -73,8 +67,8 @@ class Functions {
|
||||
});
|
||||
}
|
||||
|
||||
List<Function> get(NameAndArgumentCount key) {
|
||||
return functions.getOrDefault(key, Collections.emptyList());
|
||||
List<Function> get(String name) {
|
||||
return functions.getOrDefault(name, Collections.emptyList());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -89,9 +83,12 @@ class Functions {
|
||||
*/
|
||||
Optional<Function> get(String name, List<TypeDescriptor> argumentTypes) {
|
||||
|
||||
Stream<Function> candidates = get(NameAndArgumentCount.of(name, argumentTypes.size())).stream() //
|
||||
Stream<Function> candidates = get(name).stream() //
|
||||
.filter(f -> f.supports(argumentTypes));
|
||||
return bestMatch(candidates.collect(Collectors.toList()), argumentTypes);
|
||||
|
||||
List<Function> collect = candidates.collect(Collectors.toList());
|
||||
|
||||
return bestMatch(collect, argumentTypes);
|
||||
}
|
||||
|
||||
private static boolean contains(List<Function> elements, Function f) {
|
||||
@@ -125,16 +122,4 @@ class Functions {
|
||||
|
||||
return String.format(MESSAGE_TEMPLATE, candidates.get(0).getName(), argumentTypeString);
|
||||
}
|
||||
|
||||
@Value
|
||||
@AllArgsConstructor(access = AccessLevel.PRIVATE, staticName = "of")
|
||||
static class NameAndArgumentCount {
|
||||
|
||||
String name;
|
||||
int count;
|
||||
|
||||
static NameAndArgumentCount of(Method m) {
|
||||
return NameAndArgumentCount.of(m.getName(), m.getParameterCount());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,15 +15,17 @@
|
||||
*/
|
||||
package org.springframework.data.spel.spi;
|
||||
|
||||
import java.lang.reflect.Array;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
import org.springframework.core.convert.TypeDescriptor;
|
||||
import org.springframework.data.util.ParameterTypes;
|
||||
import org.springframework.lang.Nullable;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.TypeUtils;
|
||||
|
||||
/**
|
||||
* Value object to represent a function. Can either be backed by a static {@link Method} invocation (see
|
||||
@@ -75,7 +77,37 @@ public class Function {
|
||||
* @throws Exception
|
||||
*/
|
||||
public Object invoke(Object[] arguments) throws Exception {
|
||||
return method.invoke(target, arguments);
|
||||
|
||||
if (method.getParameterCount() == arguments.length) {
|
||||
return method.invoke(target, arguments);
|
||||
}
|
||||
|
||||
Class<?>[] types = method.getParameterTypes();
|
||||
Class<?> tailType = types[types.length - 1];
|
||||
|
||||
if (tailType.isArray()) {
|
||||
|
||||
List<Object> argumentsToUse = new ArrayList<>(types.length);
|
||||
|
||||
// Add all arguments up until the last one
|
||||
for (int i = 0; i < types.length - 1; i++) {
|
||||
argumentsToUse.add(arguments[i]);
|
||||
}
|
||||
|
||||
// Gather all other arguments into an array of the tail type
|
||||
Object[] varargs = (Object[]) Array.newInstance(tailType.getComponentType(), arguments.length - types.length + 1);
|
||||
int count = 0;
|
||||
|
||||
for (int i = types.length - 1; i < arguments.length; i++) {
|
||||
varargs[count++] = arguments[i];
|
||||
}
|
||||
|
||||
argumentsToUse.add(varargs);
|
||||
|
||||
return method.invoke(target, argumentsToUse.size() == 1 ? argumentsToUse.get(0) : argumentsToUse.toArray());
|
||||
}
|
||||
|
||||
throw new IllegalStateException(String.format("Could not invoke method %s for arguments %s!", method, arguments));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -103,20 +135,7 @@ public class Function {
|
||||
* @return
|
||||
*/
|
||||
public boolean supports(List<TypeDescriptor> argumentTypes) {
|
||||
|
||||
if (method.getParameterCount() != argumentTypes.size()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
Class<?>[] parameterTypes = method.getParameterTypes();
|
||||
|
||||
for (int i = 0; i < parameterTypes.length; i++) {
|
||||
if (!TypeUtils.isAssignable(parameterTypes[i], argumentTypes.get(i).getType())) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
return ParameterTypes.of(argumentTypes).areValidFor(method);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -135,20 +154,7 @@ public class Function {
|
||||
* @return {@code true} if the types are equal, {@code false} otherwise.
|
||||
*/
|
||||
public boolean supportsExact(List<TypeDescriptor> argumentTypes) {
|
||||
|
||||
if (method.getParameterCount() != argumentTypes.size()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
Class<?>[] parameterTypes = method.getParameterTypes();
|
||||
|
||||
for (int i = 0; i < parameterTypes.length; i++) {
|
||||
if (parameterTypes[i] != argumentTypes.get(i).getType()) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
return ParameterTypes.of(argumentTypes).exactlyMatchParametersOf(method);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user