DATACMNS-1026 ExtensionAwareEvaluationContextProvider now returns all overloaded methods as functions

src/main/java/org/springframework/data/repository/query/EvaluationContextExtensionInformation.java

@@ -29,14 +29,17 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
-import java.util.stream.Collectors;
 
 import org.springframework.beans.BeanUtils;
 import org.springframework.data.repository.query.EvaluationContextExtensionInformation.ExtensionTypeInformation.PublicMethodAndFieldFilter;
+import org.springframework.data.repository.query.Functions.NameAndArgumentCount;
 import org.springframework.data.repository.query.spi.EvaluationContextExtension;
 import org.springframework.data.repository.query.spi.Function;
+import org.springframework.data.util.MultiValueMapCollector;
 import org.springframework.data.util.Streamable;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.MultiValueMap;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.ReflectionUtils.FieldFilter;
 import org.springframework.util.ReflectionUtils.MethodFilter;
@@ -126,7 +129,7 @@ public static class ExtensionTypeInformation {
 		 * 
 		 * @return the functions will never be {@literal null}.
 		 */
-		private final Map<String, Function> functions;
+		private final MultiValueMap<NameAndArgumentCount, Function> functions;
 
 		/**
 		 * Creates a new {@link ExtensionTypeInformation} fir the given type.
@@ -141,15 +144,15 @@ public ExtensionTypeInformation(Class<? extends EvaluationContextExtension> type
 			this.properties = discoverDeclaredProperties(type);
 		}
 
-		private static Map<String, Function> discoverDeclaredFunctions(Class<?> type) {
+		private static MultiValueMap<NameAndArgumentCount, Function> discoverDeclaredFunctions(Class<?> type) {
 
-			Map<String, Function> map = new HashMap<>();
+			MultiValueMap<NameAndArgumentCount, Function> map = CollectionUtils.toMultiValueMap(new HashMap<>());
 
 			ReflectionUtils.doWithMethods(type, //
-					method -> map.put(method.getName(), new Function(method, null)), //
+					method -> map.add(NameAndArgumentCount.of(method), new Function(method, null)), //
 					PublicMethodAndFieldFilter.STATIC);
 
-			return map.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(map);
+			return CollectionUtils.unmodifiableMultiValueMap(map);
 		}
 
 		@RequiredArgsConstructor
@@ -235,8 +238,7 @@ public RootObjectInformation(Class<?> type) {
 
 			}, PublicMethodAndFieldFilter.NON_STATIC);
 
-			ReflectionUtils.doWithFields(type, RootObjectInformation.this.fields::add,
-					PublicMethodAndFieldFilter.NON_STATIC);
+			ReflectionUtils.doWithFields(type, RootObjectInformation.this.fields::add, PublicMethodAndFieldFilter.NON_STATIC);
 		}
 
 		/**
@@ -245,14 +247,15 @@ public RootObjectInformation(Class<?> type) {
 		 * @param target can be {@literal null}.
 		 * @return the methods
 		 */
-		public Map<String, Function> getFunctions(Optional<Object> target) {
-
-			return target.map(it -> methods.stream()//
-					.collect(Collectors.toMap(//
-							Method::getName, //
-							method -> new Function(method, it), //
-							(left, right) -> right)))
-					.orElseGet(Collections::emptyMap);
+		public MultiValueMap<NameAndArgumentCount, Function> getFunctions(Optional<Object> target) {
+
+			return target.map( //
+					it -> methods.stream().collect( //
+							new MultiValueMapCollector<>( //
+									m -> NameAndArgumentCount.of(m), //
+									m -> new Function(m, it) //
+							))) //
+					.orElseGet(() -> CollectionUtils.toMultiValueMap(Collections.emptyMap()));
 		}
 
 		/**

src/main/java/org/springframework/data/repository/query/ExtensionAwareEvaluationContextProvider.java

@@ -23,7 +23,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -60,6 +59,7 @@
  * @author Thomas Darimont
  * @author Oliver Gierke
  * @author Christoph Strobl
+ * @author Jens Schauder
  * @since 1.9
  */
 public class ExtensionAwareEvaluationContextProvider implements EvaluationContextProvider, ApplicationContextAware {
@@ -80,7 +80,7 @@ public ExtensionAwareEvaluationContextProvider() {
 	/**
 	 * Creates a new {@link ExtensionAwareEvaluationContextProvider} for the given {@link EvaluationContextExtension}s.
 	 * 
-	 * @param adapters must not be {@literal null}.
+	 * @param extensions must not be {@literal null}.
 	 */
 	public ExtensionAwareEvaluationContextProvider(List<? extends EvaluationContextExtension> extensions) {
 
@@ -210,7 +210,7 @@ private class ExtensionAwarePropertyAccessor implements PropertyAccessor, Method
 		/**
 		 * Creates a new {@link ExtensionAwarePropertyAccessor} for the given {@link EvaluationContextExtension}s.
 		 * 
-		 * @param adapters must not be {@literal null}.
+		 * @param extensions must not be {@literal null}.
 		 */
 		public ExtensionAwarePropertyAccessor(List<? extends EvaluationContextExtension> extensions) {
 
@@ -307,19 +307,16 @@ public Class<?>[] getSpecificTargetClasses() {
 		}
 
 		/**
-		 * Returns a {@link MethodExecutor}
+		 * Returns a {@link MethodExecutor} wrapping a function from the adapter passed in as an argument.
 		 * 
-		 * @param adapter
-		 * @param name
-		 * @param argumentTypes
-		 * @return
+		 * @param adapter the source of functions to consider.
+		 * @param name the name of the function
+		 * @param argumentTypes the types of the arguments that the function must accept.
+		 * @return a matching {@link MethodExecutor}
 		 */
 		private Optional<MethodExecutor> getMethodExecutor(EvaluationContextExtensionAdapter adapter, String name,
 				List<TypeDescriptor> argumentTypes) {
-
-			return adapter.getFunctions().entrySet().stream()//
-					.filter(entry -> entry.getKey().equals(name))//
-					.findFirst().map(Entry::getValue).map(FunctionMethodExecutor::new);
+			return adapter.getFunctions().get(name, argumentTypes).map(FunctionMethodExecutor::new);
 		}
 
 		/**
@@ -328,7 +325,7 @@ private Optional<MethodExecutor> getMethodExecutor(EvaluationContextExtensionAda
 		 * 
 		 * @param extension must not be {@literal null}.
 		 * @param name must not be {@literal null} or empty.
-		 * @return
+		 * @return a {@link TypedValue} matching the given parameters.
 		 */
 		private TypedValue lookupPropertyFrom(EvaluationContextExtensionAdapter extension, String name) {
 
@@ -388,7 +385,7 @@ private static class EvaluationContextExtensionAdapter {
 
 		private final EvaluationContextExtension extension;
 
-		private final Map<String, Function> functions;
+		private final Functions functions = new Functions();
 		private final Map<String, Object> properties;
 
 		/**
@@ -401,17 +398,16 @@ private static class EvaluationContextExtensionAdapter {
 		public EvaluationContextExtensionAdapter(EvaluationContextExtension extension,
 				EvaluationContextExtensionInformation information) {
 
-			Assert.notNull(extension, "Extenstion must not be null!");
+			Assert.notNull(extension, "Extension must not be null!");
 			Assert.notNull(information, "Extension information must not be null!");
 
 			Optional<Object> target = Optional.ofNullable(extension.getRootObject());
 			ExtensionTypeInformation extensionTypeInformation = information.getExtensionTypeInformation();
 			RootObjectInformation rootObjectInformation = information.getRootObjectInformation(target);
 
-			this.functions = new HashMap<>();
-			this.functions.putAll(extensionTypeInformation.getFunctions());
-			this.functions.putAll(rootObjectInformation.getFunctions(target));
-			this.functions.putAll(extension.getFunctions());
+			functions.addAll(extension.getFunctions());
+			functions.addAll(rootObjectInformation.getFunctions(target));
+			functions.addAll(extensionTypeInformation.getFunctions());
 
 			this.properties = new HashMap<>();
 			this.properties.putAll(extensionTypeInformation.getProperties());
@@ -424,25 +420,25 @@ public EvaluationContextExtensionAdapter(EvaluationContextExtension extension,
 		/**
 		 * Returns the extension identifier.
 		 * 
-		 * @return
+		 * @return the id of the extension
 		 */
-		public String getExtensionId() {
+		String getExtensionId() {
 			return extension.getExtensionId();
 		}
 
 		/**
 		 * Returns all functions exposed.
 		 * 
-		 * @return
+		 * @return all exposed functions.
 		 */
-		public Map<String, Function> getFunctions() {
+		Functions getFunctions() {
 			return this.functions;
 		}
 
 		/**
 		 * Returns all properties exposed. Note, the value of a property can be a {@link Function} in turn
 		 * 
-		 * @return
+		 * @return a map from property name to property value.
 		 */
 		public Map<String, Object> getProperties() {
 			return this.properties;

src/main/java/org/springframework/data/repository/query/Functions.java

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2017 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.repository.query;
+
+import lombok.AllArgsConstructor;
+import lombok.Value;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.data.repository.query.spi.Function;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.MultiValueMap;
+
+/**
+ * {@link MultiValueMap} like data structure to keep lists of
+ * {@link org.springframework.data.repository.query.spi.Function}s indexed by name and argument list length, where the
+ * value lists are actually unique with respect to the signature.
+ *
+ * @author Jens Schauder
+ * @since 2.0
+ */
+class Functions {
+
+	private final MultiValueMap<NameAndArgumentCount, Function> functions = CollectionUtils
+			.toMultiValueMap(new HashMap<>());
+
+	void addAll(Map<String, Function> newFunctions) {
+
+		newFunctions.forEach((n, f) -> {
+			NameAndArgumentCount k = new NameAndArgumentCount(n, f.getParameterCount());
+			List<Function> currentElements = get(k);
+			if (!contains(currentElements, f)) {
+				functions.add(k, f);
+			}
+		});
+	}
+
+	void addAll(MultiValueMap<NameAndArgumentCount, Function> newFunctions) {
+
+		newFunctions.forEach((k, list) -> {
+			List<Function> currentElements = get(k);
+			list.stream() //
+					.filter(f -> !contains(currentElements, f)) //
+					.forEach(f -> functions.add(k, f));
+		});
+	}
+
+	List<Function> get(NameAndArgumentCount key) {
+		return functions.getOrDefault(key, Collections.emptyList());
+	}
+
+	/**
+	 * Gets the function that best matches the parameters given. The {@code name} must match, and the
+	 * {@code argumentTypes} must be compatible with parameter list of the function. In order to resolve ambiguity it
+	 * checks for a method with exactly matching parameter list.
+	 *
+	 * @param name the name of the method
+	 * @param argumentTypes types of arguments that the method must be able to accept
+	 * @return a {@code Function} if a unique on gets found. {@code Optional.empty} if none matches. Throws
+	 *         {@link IllegalStateException} if multiple functions match the parameters.
+	 */
+	Optional<Function> get(String name, List<TypeDescriptor> argumentTypes) {
+
+		Stream<Function> candidates = get(new NameAndArgumentCount(name, argumentTypes.size())).stream() //
+				.filter(f -> f.supports(argumentTypes));
+		return bestMatch(candidates.collect(Collectors.toList()), argumentTypes);
+	}
+
+	private static boolean contains(List<Function> elements, Function f) {
+		return elements.stream().anyMatch(f::isSignatureEqual);
+	}
+
+	private static Optional<Function> bestMatch(List<Function> candidates, List<TypeDescriptor> argumentTypes) {
+
+		if (candidates.isEmpty()) {
+			return Optional.empty();
+		}
+		if (candidates.size() == 1) {
+			return Optional.of(candidates.get(0));
+		}
+
+		Optional<Function> exactMatch = candidates.stream().filter(f -> f.supportsExact(argumentTypes)).findFirst();
+		if (!exactMatch.isPresent()) {
+			throw new IllegalStateException(createErrorMessage(candidates, argumentTypes));
+		}
+
+		return exactMatch;
+	}
+
+	private static String createErrorMessage(List<Function> candidates, List<TypeDescriptor> argumentTypes) {
+
+		String argumentTypeString = String.join( //
+				",", //
+				argumentTypes.stream().map(TypeDescriptor::getName).collect(Collectors.toList()));
+
+		String messageTemplate = "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.";
+
+		return String.format(messageTemplate, candidates.get(0).getName(), argumentTypeString);
+	}
+
+	@Value
+	@AllArgsConstructor
+	static class NameAndArgumentCount {
+		String name;
+		int count;
+
+		static NameAndArgumentCount of(Method m) {
+			return new NameAndArgumentCount(m.getName(), m.getParameterCount());
+		}
+	}
+}