ReflectiveMethodExecutor invokes interface method if possible

Issue: SPR-16845

spring-core/src/main/java/org/springframework/util/ClassUtils.java

@@ -885,7 +885,7 @@ public abstract class ClassUtils {
 	public static Class<?> getUserClass(Class<?> clazz) {
 		if (clazz.getName().contains(CGLIB_CLASS_SEPARATOR)) {
 			Class<?> superclass = clazz.getSuperclass();
-			if (superclass != null && Object.class != superclass) {
+			if (superclass != null && superclass != Object.class) {
 				return superclass;
 			}
 		}
@@ -1244,10 +1244,11 @@ public abstract class ClassUtils {
 	 * access (e.g. calls to {@code Class#getDeclaredMethods} etc, this implementation
 	 * will fall back to returning the originally provided method.
 	 * @param method the method to be invoked, which may come from an interface
-	 * @param targetClass the target class for the current invocation.
-	 * May be {@code null} or may not even implement the method.
+	 * @param targetClass the target class for the current invocation
+	 * (may be {@code null} or may not even implement the method)
 	 * @return the specific target method, or the original method if the
-	 * {@code targetClass} doesn't implement it or is {@code null}
+	 * {@code targetClass} does not implement it
+	 * @see #getInterfaceMethodIfPossible
 	 */
 	public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
 		if (targetClass != null && targetClass != method.getDeclaringClass() && isOverridable(method, targetClass)) {
@@ -1273,6 +1274,34 @@ public abstract class ClassUtils {
 		return method;
 	}
 
+	/**
+	 * Determine a corresponding interface method for the given method handle, if possible.
+	 * <p>This is particularly useful for arriving at a public exported type on Jigsaw
+	 * which can be reflectively invoked without an illegal access warning.
+	 * @param method the method to be invoked, potentially from an implementation class
+	 * @return the corresponding interface method, or the original method if none found
+	 * @since 5.1
+	 * @see #getMostSpecificMethod
+	 */
+	public static Method getInterfaceMethodIfPossible(Method method) {
+		if (Modifier.isPublic(method.getModifiers()) && !method.getDeclaringClass().isInterface()) {
+			Class<?> current = method.getDeclaringClass();
+			while (current != null && current != Object.class) {
+				Class<?>[] ifcs = current.getInterfaces();
+				for (Class<?> ifc : ifcs) {
+					try {
+						return ifc.getMethod(method.getName(), method.getParameterTypes());
+					}
+					catch (NoSuchMethodException ex) {
+						// ignore
+					}
+				}
+				current = current.getSuperclass();
+			}
+		}
+		return method;
+	}
+
 	/**
 	 * Determine whether the given method is declared by the user or at least pointing to
 	 * a user-declared method.

spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

@@ -182,8 +182,7 @@ public class MethodReference extends SpelNodeImpl {
 			@Nullable TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
 
 		List<MethodResolver> methodResolvers = evaluationContext.getMethodResolvers();
-		if (methodResolvers.size() != 1 ||
-				!(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) {
+		if (methodResolvers.size() != 1 || !(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) {
 			// Not a default ReflectiveMethodResolver - don't know whether caching is valid
 			return null;
 		}

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java

@@ -26,6 +26,7 @@ import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.MethodExecutor;
 import org.springframework.expression.TypedValue;
 import org.springframework.lang.Nullable;
+import org.springframework.util.ClassUtils;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -37,7 +38,9 @@ import org.springframework.util.ReflectionUtils;
  */
 public class ReflectiveMethodExecutor implements MethodExecutor {
 
-	private final Method method;
+	private final Method originalMethod;
+
+	private final Method methodToInvoke;
 
 	@Nullable
 	private final Integer varargsPosition;
@@ -50,8 +53,13 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	private boolean argumentConversionOccurred = false;
 
 
+	/**
+	 * Create a new executor for the given method.
+	 * @param method the method to invoke
+	 */
 	public ReflectiveMethodExecutor(Method method) {
-		this.method = method;
+		this.originalMethod = method;
+		this.methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method);
 		if (method.isVarArgs()) {
 			Class<?>[] paramTypes = method.getParameterTypes();
 			this.varargsPosition = paramTypes.length - 1;
@@ -62,29 +70,33 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	}
 
 
-	public Method getMethod() {
-		return this.method;
+	/**
+	 * Return the original method that this executor has been configured for.
+	 */
+	public final Method getMethod() {
+		return this.originalMethod;
 	}
 
 	/**
 	 * Find the first public class in the methods declaring class hierarchy that declares this method.
 	 * Sometimes the reflective method discovery logic finds a suitable method that can easily be
 	 * called via reflection but cannot be called from generated code when compiling the expression
-	 * because of visibility restrictions. For example if a non public class overrides toString(), this
-	 * helper method will walk up the type hierarchy to find the first public type that declares the
-	 * method (if there is one!). For toString() it may walk as far as Object.
+	 * because of visibility restrictions. For example if a non-public class overrides toString(),
+	 * this helper method will walk up the type hierarchy to find the first public type that declares
+	 * the method (if there is one!). For toString() it may walk as far as Object.
 	 */
 	@Nullable
 	public Class<?> getPublicDeclaringClass() {
 		if (!this.computedPublicDeclaringClass) {
-			this.publicDeclaringClass = discoverPublicClass(this.method, this.method.getDeclaringClass());
+			this.publicDeclaringClass =
+					discoverPublicDeclaringClass(this.originalMethod, this.originalMethod.getDeclaringClass());
 			this.computedPublicDeclaringClass = true;
 		}
 		return this.publicDeclaringClass;
 	}
 
 	@Nullable
-	private Class<?> discoverPublicClass(Method method, Class<?> clazz) {
+	private Class<?> discoverPublicDeclaringClass(Method method, Class<?> clazz) {
 		if (Modifier.isPublic(clazz.getModifiers())) {
 			try {
 				clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
@@ -94,12 +106,8 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 				// Continue below...
 			}
 		}
-		Class<?>[] ifcs = clazz.getInterfaces();
-		for (Class<?> ifc: ifcs) {
-			discoverPublicClass(method, ifc);
-		}
 		if (clazz.getSuperclass() != null) {
-			return discoverPublicClass(method, clazz.getSuperclass());
+			return discoverPublicDeclaringClass(method, clazz.getSuperclass());
 		}
 		return null;
 	}
@@ -113,17 +121,17 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
 	public TypedValue execute(EvaluationContext context, Object target, Object... arguments) throws AccessException {
 		try {
 			this.argumentConversionOccurred = ReflectionHelper.convertArguments(
-					context.getTypeConverter(), arguments, this.method, this.varargsPosition);
-			if (this.method.isVarArgs()) {
+					context.getTypeConverter(), arguments, this.originalMethod, this.varargsPosition);
+			if (this.originalMethod.isVarArgs()) {
 				arguments = ReflectionHelper.setupArgumentsForVarargsInvocation(
-						this.method.getParameterTypes(), arguments);
+						this.originalMethod.getParameterTypes(), arguments);
 			}
-			ReflectionUtils.makeAccessible(this.method);
-			Object value = this.method.invoke(target, arguments);
-			return new TypedValue(value, new TypeDescriptor(new MethodParameter(this.method, -1)).narrow(value));
+			ReflectionUtils.makeAccessible(this.methodToInvoke);
+			Object value = this.methodToInvoke.invoke(target, arguments);
+			return new TypedValue(value, new TypeDescriptor(new MethodParameter(this.originalMethod, -1)).narrow(value));
 		}
 		catch (Exception ex) {
-			throw new AccessException("Problem invoking method: " + this.method, ex);
+			throw new AccessException("Problem invoking method: " + this.methodToInvoke, ex);
 		}
 	}
 

spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.expression.spel;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.expression.Expression;
@@ -36,17 +35,11 @@ public class CachedMethodExecutorTests {
 
 	private final ExpressionParser parser = new SpelExpressionParser();
 
-	private StandardEvaluationContext context;
-
-
-	@Before
-	public void setUp() throws Exception {
-		this.context = new StandardEvaluationContext(new RootObject());
-	}
+	private final StandardEvaluationContext context = new StandardEvaluationContext(new RootObject());
 
 
 	@Test
-	public void testCachedExecutionForParameters() throws Exception {
+	public void testCachedExecutionForParameters() {
 		Expression expression = this.parser.parseExpression("echo(#var)");
 
 		assertMethodExecution(expression, 42, "int: 42");
@@ -56,7 +49,7 @@ public class CachedMethodExecutorTests {
 	}
 
 	@Test
-	public void testCachedExecutionForTarget() throws Exception {
+	public void testCachedExecutionForTarget() {
 		Expression expression = this.parser.parseExpression("#var.echo(42)");
 
 		assertMethodExecution(expression, new RootObject(), "int: 42");
@@ -76,7 +69,6 @@ public class CachedMethodExecutorTests {
 		public String echo(String value) {
 			return "String: " + value;
 		}
-
 	}
 
 	public static class RootObject extends BaseObject {
@@ -84,7 +76,6 @@ public class CachedMethodExecutorTests {
 		public String echo(int value) {
 			return "int: " + value;
 		}
-
 	}
 
 }