Introduce ClassUtils.getPubliclyAccessibleMethodIfPossible()

See spring-projectsgh-33216

spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java

@@ -176,9 +176,9 @@ private void addInitDestroyHint(Class<?> beanUserClass, String methodName) {
 		Method method = ReflectionUtils.findMethod(methodDeclaringClass, methodName);
 		if (method != null) {
 			this.hints.reflection().registerMethod(method, ExecutableMode.INVOKE);
-			Method interfaceMethod = ClassUtils.getInterfaceMethodIfPossible(method, beanUserClass);
-			if (!interfaceMethod.equals(method)) {
-				this.hints.reflection().registerMethod(interfaceMethod, ExecutableMode.INVOKE);
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, beanUserClass);
+			if (!publiclyAccessibleMethod.equals(method)) {
+				this.hints.reflection().registerMethod(publiclyAccessibleMethod, ExecutableMode.INVOKE);
 			}
 		}
 	}

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

@@ -1899,7 +1899,7 @@ protected void invokeCustomInitMethod(String beanName, Object bean, RootBeanDefi
 		if (logger.isTraceEnabled()) {
 			logger.trace("Invoking init method '" + methodName + "' on bean with name '" + beanName + "'");
 		}
-		Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(initMethod, beanClass);
+		Method methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(initMethod, beanClass);
 
 		try {
 			ReflectionUtils.makeAccessible(methodToInvoke);

spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java

@@ -147,7 +147,7 @@ else if (paramTypes.length == 1 && boolean.class != paramTypes[0]) {
 										beanName + "' has a non-boolean parameter - not supported as destroy method");
 							}
 						}
-						destroyMethod = ClassUtils.getInterfaceMethodIfPossible(destroyMethod, bean.getClass());
+						destroyMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(destroyMethod, bean.getClass());
 						destroyMethods.add(destroyMethod);
 					}
 				}
@@ -253,8 +253,8 @@ else if (this.destroyMethodNames != null) {
 			for (String destroyMethodName : this.destroyMethodNames) {
 				Method destroyMethod = determineDestroyMethod(destroyMethodName);
 				if (destroyMethod != null) {
-					invokeCustomDestroyMethod(
-							ClassUtils.getInterfaceMethodIfPossible(destroyMethod, this.bean.getClass()));
+					destroyMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(destroyMethod, this.bean.getClass());
+					invokeCustomDestroyMethod(destroyMethod);
 				}
 			}
 		}

spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java

@@ -631,7 +631,7 @@ private void privateDestroy() {
 
 	}
 
-	interface Initializable {
+	public interface Initializable {
 
 		void initialize();
 	}
@@ -643,7 +643,7 @@ public void initialize() {
 		}
 	}
 
-	interface Disposable {
+	public interface Disposable {
 
 		void dispose();
 	}

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

@@ -135,10 +135,17 @@ public abstract class ClassUtils {
 	private static final Set<Class<?>> javaLanguageInterfaces;
 
 	/**
-	 * Cache for equivalent methods on an interface implemented by the declaring class.
+	 * Cache for equivalent methods on a public interface implemented by the declaring class.
 	 */
 	private static final Map<Method, Method> interfaceMethodCache = new ConcurrentReferenceHashMap<>(256);
 
+	/**
+	 * Cache for equivalent public methods in a public declaring type within the type hierarchy
+	 * of the method's declaring class.
+	 * @since 6.2
+	 */
+	private static final Map<Method, Method> publiclyAccessibleMethodCache = new ConcurrentReferenceHashMap<>(256);
+
 
 	static {
 		primitiveWrapperTypeMap.put(Boolean.class, boolean.class);
@@ -1388,8 +1395,9 @@ public static Method getMostSpecificMethod(Method method, @Nullable Class<?> tar
 
 	/**
 	 * 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.
+	 * <p>This is particularly useful for arriving at a public exported type on the Java
+	 * Module System which allows the method to be invoked via reflection 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
@@ -1402,12 +1410,14 @@ public static Method getInterfaceMethodIfPossible(Method 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.
+	 * <p>This is particularly useful for arriving at a public exported type on the Java
+	 * Module System which allows the method to be invoked via reflection without an illegal
+	 * access warning.
 	 * @param method the method to be invoked, potentially from an implementation class
 	 * @param targetClass the target class to check for declared interfaces
 	 * @return the corresponding interface method, or the original method if none found
 	 * @since 5.3.16
+	 * @see #getPubliclyAccessibleMethodIfPossible(Method, Class)
 	 * @see #getMostSpecificMethod
 	 */
 	public static Method getInterfaceMethodIfPossible(Method method, @Nullable Class<?> targetClass) {
@@ -1435,7 +1445,9 @@ private static Method findInterfaceMethodIfPossible(Method method, Class<?>[] pa
 		while (current != null && current != endClass) {
 			for (Class<?> ifc : current.getInterfaces()) {
 				try {
-					return ifc.getMethod(method.getName(), parameterTypes);
+					if (Modifier.isPublic(ifc.getModifiers())) {
+						return ifc.getMethod(method.getName(), parameterTypes);
+					}
 				}
 				catch (NoSuchMethodException ex) {
 					// ignore
@@ -1446,6 +1458,78 @@ private static Method findInterfaceMethodIfPossible(Method method, Class<?>[] pa
 		return method;
 	}
 
+	/**
+	 * Find the first publicly accessible method in the method's type hierarchy that has a
+	 * method signature equivalent to the supplied method, if possible.
+	 * <p>If the supplied method is {@code public} and declared in a {@code public} type,
+	 * the supplied method will be returned.
+	 * <p>Otherwise, this method recursively searches the class hierarchy and implemented
+	 * interfaces for an equivalent method that is {@code public} and declared in a
+	 * {@code public} type.
+	 * <p>If a publicly accessible equivalent method cannot be found, the supplied method
+	 * will be returned, indicating that no such equivalent method exists. Consequently,
+	 * callers of this method must manually validate the accessibility of the returned method
+	 * if public access is a requirement.
+	 * <p>This is particularly useful for arriving at a public exported type on the Java
+	 * Module System which allows the method to be invoked via reflection without an illegal
+	 * access warning. This is also useful for invoking methods via a public API in bytecode
+	 * &mdash; for example, for use with the Spring Expression Language (SpEL) compiler.
+	 * For example, if a non-public class overrides {@code toString()}, this method will
+	 * traverse up the type hierarchy to find the first public type that declares the method
+	 * (if there is one). For {@code toString()}, it may traverse as far as {@link Object}.
+	 * @param method the method to process
+	 * @param targetClass the target class to check for declared interfaces
+	 * @return the corresponding publicly accessible method, or the original method if none
+	 * found
+	 * @since 6.2
+	 * @see #getInterfaceMethodIfPossible(Method, Class)
+	 * @see #getMostSpecificMethod(Method, Class)
+	 */
+	public static Method getPubliclyAccessibleMethodIfPossible(Method method, @Nullable Class<?> targetClass) {
+		// If the method is not public, we can abort the search immediately; or if the method's
+		// declaring class is public, it's already publicly accessible.
+		if (!Modifier.isPublic(method.getModifiers()) || Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
+			return method;
+		}
+		Method interfaceMethod = getInterfaceMethodIfPossible(method, targetClass);
+		// If we found an interface method whose type is public, return the interface method.
+		if (!interfaceMethod.equals(method)) {
+			return interfaceMethod;
+		}
+
+		Method result = publiclyAccessibleMethodCache.computeIfAbsent(method, key -> {
+			// Attempt to search the type hierarchy.
+			Class<?> superclass = key.getDeclaringClass().getSuperclass();
+			if (superclass != null) {
+				return findPubliclyAccessibleMethod(superclass, key.getName(), key.getParameterTypes());
+			}
+			// Otherwise, no publicly accessible method was found.
+			return null;
+		});
+
+		return (result != null ? result : method);
+	}
+
+	@Nullable
+	private static Method findPubliclyAccessibleMethod(
+			Class<?> declaringClass, String methodName, Class<?>[] parameterTypes) {
+
+		if (Modifier.isPublic(declaringClass.getModifiers())) {
+			try {
+				return declaringClass.getDeclaredMethod(methodName, parameterTypes);
+			}
+			catch (NoSuchMethodException ex) {
+				// Continue below...
+			}
+		}
+
+		Class<?> superclass = declaringClass.getSuperclass();
+		if (superclass != null) {
+			return findPubliclyAccessibleMethod(superclass, methodName, parameterTypes);
+		}
+		return null;
+	}
+
 	/**
 	 * Determine whether the given method is declared by the user or at least pointing to
 	 * a user-declared method.

spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java

@@ -23,8 +23,11 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Member;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
+import java.time.ZoneId;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -37,6 +40,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
@@ -503,6 +507,174 @@ void overloadedStaticMethod() throws IllegalAccessException, InvocationTargetExc
 	}
 
 
+	@Nested  // gh-33216
+	class GetPubliclyAccessibleMethodTests {
+
+		@Test
+		void nonPublicMethod(TestInfo testInfo) {
+			Method originalMethod = testInfo.getTestMethod().get();
+
+			// Prerequisites for this use case:
+			assertNotPublic(originalMethod);
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertNotPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		// This method is intentionally public.
+		public void publicMethodInNonPublicType(TestInfo testInfo) {
+			Method originalMethod = testInfo.getTestMethod().get();
+
+			// Prerequisites for this use case:
+			assertPublic(originalMethod);
+			assertNotPublic(originalMethod.getDeclaringClass());
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertNotPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		void publicMethodInPublicType() throws Exception {
+			Class<?> originalType = String.class;
+			Method originalMethod = originalType.getDeclaredMethod("toString");
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(originalType);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		void publicInterfaceMethodInPublicType() throws Exception {
+			Class<?> originalType = ArrayList.class;
+			Method originalMethod = originalType.getDeclaredMethod("size");
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			// We do not expect the method to be found in List.class.
+			assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(originalType);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		void publicMethodInJavaLangObjectDeclaredInNonPublicType() throws Exception {
+			List<String> unmodifiableList = Collections.unmodifiableList(Arrays.asList("foo", "bar"));
+			Class<?> targetClass = unmodifiableList.getClass();
+
+			// Prerequisites for this use case:
+			assertNotPublic(targetClass);
+
+			Method originalMethod = targetClass.getMethod("toString");
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(Object.class);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		void publicMethodInJavaTimeZoneIdDeclaredInNonPublicSubclass() throws Exception {
+			// Returns a package-private java.time.ZoneRegion.
+			ZoneId zoneId = ZoneId.of("CET");
+			Class<?> targetClass = zoneId.getClass();
+
+			// Prerequisites for this use case:
+			assertNotPublic(targetClass);
+
+			Method originalMethod = targetClass.getDeclaredMethod("getId");
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(ZoneId.class);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test
+		void privateSubclassOverridesPropertyInPublicInterface() throws Exception {
+			Method originalMethod = PrivateSubclass.class.getDeclaredMethod("getText");
+
+			// Prerequisite: type must not be public for this use case.
+			assertNotPublic(originalMethod.getDeclaringClass());
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicInterface.class);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		private static void assertPubliclyAccessible(Method method) {
+			assertPublic(method);
+			assertPublic(method.getDeclaringClass());
+		}
+
+		private static void assertNotPubliclyAccessible(Method method) {
+			assertThat(!isPublic(method) || !isPublic(method.getDeclaringClass()))
+					.as("%s must not be publicly accessible", method)
+					.isTrue();
+		}
+
+		private static void assertPublic(Member member) {
+			assertThat(isPublic(member)).as("%s must be public", member).isTrue();
+		}
+
+		private static void assertPublic(Class<?> clazz) {
+			assertThat(isPublic(clazz)).as("%s must be public", clazz).isTrue();
+		}
+
+		private static void assertNotPublic(Member member) {
+			assertThat(!isPublic(member)).as("%s must be not be public", member).isTrue();
+		}
+
+		private static void assertNotPublic(Class<?> clazz) {
+			assertThat(!isPublic(clazz)).as("%s must be not be public", clazz).isTrue();
+		}
+
+		private static boolean isPublic(Class<?> clazz) {
+			return Modifier.isPublic(clazz.getModifiers());
+		}
+
+		private static boolean isPublic(Member member) {
+			return Modifier.isPublic(member.getModifiers());
+		}
+
+		private interface PrivateInterface {
+
+			String getMessage();
+
+			String getIndex(int index);
+		}
+
+		private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface {
+
+			@Override
+			public int getNumber() {
+				return 2;
+			}
+
+			@Override
+			public String getText() {
+				return "enigma";
+			}
+
+			@Override
+			public String getMessage() {
+				return "hello";
+			}
+
+			@Override
+			public String getIndex2(int index) {
+				return "sub-" + (2 * index);
+			}
+
+			@Override
+			public String getFruit(int index) {
+				return "fruit-" + index;
+			}
+		}
+
+	}
+
+
 	@Target(ElementType.METHOD)
 	@Retention(RetentionPolicy.RUNTIME)
 	@ValueSource(classes = { Boolean.class, Character.class, Byte.class, Short.class,