diff --git a/src/main/java/org/springframework/data/spel/EvaluationContextExtensionInformation.java b/src/main/java/org/springframework/data/spel/EvaluationContextExtensionInformation.java index 1153edd84..f9752fe25 100644 --- a/src/main/java/org/springframework/data/spel/EvaluationContextExtensionInformation.java +++ b/src/main/java/org/springframework/data/spel/EvaluationContextExtensionInformation.java @@ -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 functions; + private final MultiValueMap functions; /** * Creates a new {@link ExtensionTypeInformation} fir the given type. @@ -140,12 +139,12 @@ class EvaluationContextExtensionInformation { this.properties = discoverDeclaredProperties(type); } - private static MultiValueMap discoverDeclaredFunctions(Class type) { + private static MultiValueMap discoverDeclaredFunctions(Class type) { - MultiValueMap map = CollectionUtils.toMultiValueMap(new HashMap<>()); + MultiValueMap 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 getFunctions(Optional target) { + public MultiValueMap getFunctions(Optional target) { return target.map(this::getFunctions).orElseGet(() -> new LinkedMultiValueMap<>()); } - private MultiValueMap getFunctions(Object target) { - return methods.stream().collect(toMultiMap(NameAndArgumentCount::of, m -> new Function(m, target))); + private MultiValueMap getFunctions(Object target) { + return methods.stream().collect(toMultiMap(Method::getName, m -> new Function(m, target))); } /** diff --git a/src/main/java/org/springframework/data/spel/Functions.java b/src/main/java/org/springframework/data/spel/Functions.java index a9405917d..67e489721 100644 --- a/src/main/java/org/springframework/data/spel/Functions.java +++ b/src/main/java/org/springframework/data/spel/Functions.java @@ -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 functions = new LinkedMultiValueMap<>(); + private final MultiValueMap functions = new LinkedMultiValueMap<>(); void addAll(Map newFunctions) { newFunctions.forEach((n, f) -> { - NameAndArgumentCount k = NameAndArgumentCount.of(n, f.getParameterCount()); - List currentElements = get(k); + List currentElements = get(n); if (!contains(currentElements, f)) { - functions.add(k, f); + functions.add(n, f); } }); } - void addAll(MultiValueMap newFunctions) { + void addAll(MultiValueMap newFunctions) { newFunctions.forEach((k, list) -> { @@ -73,8 +67,8 @@ class Functions { }); } - List get(NameAndArgumentCount key) { - return functions.getOrDefault(key, Collections.emptyList()); + List get(String name) { + return functions.getOrDefault(name, Collections.emptyList()); } /** @@ -89,9 +83,12 @@ class Functions { */ Optional get(String name, List argumentTypes) { - Stream candidates = get(NameAndArgumentCount.of(name, argumentTypes.size())).stream() // + Stream candidates = get(name).stream() // .filter(f -> f.supports(argumentTypes)); - return bestMatch(candidates.collect(Collectors.toList()), argumentTypes); + + List collect = candidates.collect(Collectors.toList()); + + return bestMatch(collect, argumentTypes); } private static boolean contains(List 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()); - } - } } diff --git a/src/main/java/org/springframework/data/spel/spi/Function.java b/src/main/java/org/springframework/data/spel/spi/Function.java index b1aa0cc66..70be4a4ee 100644 --- a/src/main/java/org/springframework/data/spel/spi/Function.java +++ b/src/main/java/org/springframework/data/spel/spi/Function.java @@ -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 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 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 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); } /** diff --git a/src/main/java/org/springframework/data/util/ParameterTypes.java b/src/main/java/org/springframework/data/util/ParameterTypes.java new file mode 100644 index 000000000..1eb880979 --- /dev/null +++ b/src/main/java/org/springframework/data/util/ParameterTypes.java @@ -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, ParameterTypes> CACHE = new ConcurrentReferenceHashMap<>(); + + private final List types; + private final Lazy> alternatives; + + /** + * Creates a new {@link ParameterTypes} for the given types. + * + * @param types + */ + private ParameterTypes(List 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 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 getAllAlternatives() { + + List 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 getParent() { + return types.isEmpty() ? Optional.empty() : getParent(getTail()); + } + + protected final Optional getParent(TypeDescriptor tail) { + + return types.size() <= 1 // + ? Optional.empty() // + : Optional.of(ParentParameterTypes.of(types.subList(0, types.size() - 1), tail)); + } + + protected Optional 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 result = new ArrayList<>(types.subList(0, types.size() - 1)); + result.add(TypeDescriptor.array(descriptor)); + + return ParameterTypes.of(result); + } + + private Collection getAlternatives() { + + if (types.isEmpty()) { + return Collections.emptyList(); + } + + List 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 types, TypeDescriptor tail) { + + super(types); + this.tail = tail; + } + + public static ParentParameterTypes of(List types, TypeDescriptor tail) { + return new ParentParameterTypes(types, tail); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.ParameterTypes#getParent() + */ + @Override + protected Optional getParent() { + return super.getParent(tail); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.ParameterTypes#withLastVarArgs() + */ + @Override + protected Optional withLastVarArgs() { + + return !tail.isAssignableTo(super.getTail()) // + ? Optional.empty() // + : super.withLastVarArgs(); + } + } +} diff --git a/src/test/java/org/springframework/data/repository/query/ExtensionAwareEvaluationContextProviderUnitTests.java b/src/test/java/org/springframework/data/repository/query/ExtensionAwareEvaluationContextProviderUnitTests.java index a816cf7cf..13a25d587 100755 --- a/src/test/java/org/springframework/data/repository/query/ExtensionAwareEvaluationContextProviderUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/ExtensionAwareEvaluationContextProviderUnitTests.java @@ -271,6 +271,14 @@ public class ExtensionAwareEvaluationContextProviderUnitTests { .withMessageContaining("(java.lang.Integer)"); } + @Test // DATACMNS-1518 + public void invokesMethodWithVarArgs() { + + provider = createContextProviderWithOverloads(); + + assertThat(evaluateExpression("methodWithVarArgs('one', 'two')")).isEqualTo("varargs"); + } + private static ExtensionAwareQueryMethodEvaluationContextProvider createContextProviderWithOverloads() { return new ExtensionAwareQueryMethodEvaluationContextProvider(Collections.singletonList( // @@ -429,5 +437,9 @@ public class ExtensionAwareEvaluationContextProviderUnitTests { public String ambiguousOverloaded(Serializable o) { return "serializable"; } + + public String methodWithVarArgs(String... args) { + return "varargs"; + } } } diff --git a/src/test/java/org/springframework/data/spel/spi/FunctionUnitTests.java b/src/test/java/org/springframework/data/spel/spi/FunctionUnitTests.java new file mode 100644 index 000000000..d72450aaa --- /dev/null +++ b/src/test/java/org/springframework/data/spel/spi/FunctionUnitTests.java @@ -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; + } + } +} diff --git a/src/test/java/org/springframework/data/util/ParameterTypesUnitTests.java b/src/test/java/org/springframework/data/util/ParameterTypesUnitTests.java new file mode 100644 index 000000000..1bea7443e --- /dev/null +++ b/src/test/java/org/springframework/data/util/ParameterTypesUnitTests.java @@ -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 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); + } +}