MethodUtils cannot find or invoke a public method on a public class

implemented in its package-private superclass

- Add org.apache.commons.lang3.reflect.MethodUtils.getAccessibleMethod(Class<?>,
Method)

Author: garydgregory

src/changes/changes.xml

@@ -49,6 +49,7 @@ The <action> type attribute can be add,update,fix,remove.
     <action issue="LANG-1778" type="fix" dev="ggregory" due-to="wuwu2000">MethodUtils.getMatchingMethod() doesn't respect the hierarchy of methods #1414.</action>
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">org.apache.commons.lang3.reflect.MethodUtils.getMethodObject(Class&lt;?&gt;, String, Class&lt;?&gt;...) now returns null instead of throwing a NullPointerException, as it does for other exception types.</action>
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">Reduce spurious failures in org.apache.commons.lang3.ArrayUtilsTest methods that test ArrayUtils.shuffle() methods.</action>
+    <action                   type="fix" dev="ggregory" due-to="Gary Gregory">MethodUtils cannot find or invoke a public method on a public class implemented in its package-private superclass.</action>
     <!-- FIX Javadoc -->
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">[javadoc] General improvements.</action>
     <action                   type="fix" dev="ggregory" due-to="Gary Gregory">[javadoc] Fix thrown exception documentation for org.apache.commons.lang3.reflect.MethodUtils.getMethodObject(Class&lt;?&gt;, String, Class&lt;?&gt;...).</action>
@@ -61,6 +62,7 @@ The <action> type attribute can be add,update,fix,remove.
     <!-- ADD -->
     <action                   type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.lang3.ArrayUtils.SOFT_MAX_ARRAY_LENGTH.</action>
     <action                   type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.lang3.SystemUtils.IS_OS_NETWARE.</action>
+    <action                   type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.lang3.reflect.MethodUtils.getAccessibleMethod(Class, Method).</action>
     <!-- UPDATE -->
     <action type="update" dev="ggregory" due-to="Gary Gregory">[test] Bump org.apache.commons:commons-text from 1.13.1 to 1.14.0.</action>
   </release>

src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java

@@ -109,31 +109,39 @@ public static Method getAccessibleMethod(final Class<?> cls, final String method
     }
 
     /**
-     * Gets an accessible method (that is, one that can be invoked via
-     * reflection) that implements the specified Method. If no such method
-     * can be found, return {@code null}.
+     * Gets an accessible method (that is, one that can be invoked via reflection) that implements the specified Method. If no such method can be found, return
+     * {@code null}.
      *
      * @param method The method that we wish to call, may be null.
      * @return The accessible method
      */
-    public static Method getAccessibleMethod(Method method) {
-        if (!MemberUtils.isAccessible(method)) {
+    public static Method getAccessibleMethod(final Method method) {
+        return method != null ? getAccessibleMethod(method.getDeclaringClass(), method) : null;
+    }
+
+    /**
+     * Gets an accessible method (that is, one that can be invoked via reflection) that implements the specified Method. If no such method can be found, return
+     * {@code null}.
+     *
+     * @param cls The implementing class, may be null.
+     * @param method The method that we wish to call, may be null.
+     * @return The accessible method or null.
+     * @since 3.19.0
+     */
+    public static Method getAccessibleMethod(final Class<?> cls, final Method method) {
+        if (!MemberUtils.isPublic(method)) {
             return null;
         }
         // If the declaring class is public, we are done
-        final Class<?> cls = method.getDeclaringClass();
         if (ClassUtils.isPublic(cls)) {
             return method;
         }
         final String methodName = method.getName();
         final Class<?>[] parameterTypes = method.getParameterTypes();
         // Check the implemented interfaces and subinterfaces
-        method = getAccessibleMethodFromInterfaceNest(cls, methodName, parameterTypes);
+        final Method method2 = getAccessibleMethodFromInterfaceNest(cls, methodName, parameterTypes);
         // Check the superclass chain
-        if (method == null) {
-            method = getAccessibleMethodFromSuperclass(cls, methodName, parameterTypes);
-        }
-        return method;
+        return method2 != null ? method2 : getAccessibleMethodFromSuperclass(cls, methodName, parameterTypes);
     }
 
     /**

src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java

@@ -29,6 +29,7 @@
 
 import java.awt.Color;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
@@ -55,19 +56,22 @@
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * {@link Tests MethodUtils}.
  */
 class MethodUtilsTest extends AbstractLangTest {
+
     protected abstract static class AbstractGetMatchingMethod implements InterfaceGetMatchingMethod {
         public abstract void testMethod5(Exception exception);
     }
+
     protected abstract static class AbstractGetMatchingMethod2 implements InterfaceGetMatchingMethod {
         @Override
         public void testMethod6() { }
     }
-
     interface ChildInterface {
     }
 
@@ -116,14 +120,15 @@ public void testMethod4(final Color aColor1, final Color aColor2) {
         public void testMethod4(final Long aLong, final Long anotherLong) {
         }
     }
+
     private static final class GetMatchingMethodImpl extends AbstractGetMatchingMethod {
         @Override
         public void testMethod5(final Exception exception) {
         }
     }
-
     public static class GrandParentObject {
     }
+
     public static class InheritanceBean {
         public void testOne(final GrandParentObject obj) {
         }
@@ -143,7 +148,6 @@ public void testTwo(final GrandParentObject obj) {
         public void testTwo(final Object obj) {
         }
     }
-
     interface InterfaceGetMatchingMethod {
         default void testMethod6() {
         }
@@ -403,13 +407,16 @@ public ImmutablePair<String, Object[]> varOverloadEcho(final String... args) {
 
     }
 
+    static class TestBeanSubclass extends TestBean {
+    }
+
     static class TestBeanWithInterfaces implements PrivateInterface {
         public String foo() {
             return "foo()";
         }
     }
 
-    private static final class TestMutable implements Mutable<Object> {
+    private static class TestMutable implements Mutable<Object> {
         @Override
         public Object getValue() {
             return null;
@@ -420,6 +427,10 @@ public void setValue(final Object value) {
         }
     }
 
+    private static final class TestMutableSubclass extends TestMutable {
+
+    }
+
     private TestBean testBean;
 
     private final Map<Class<?>, Class<?>[]> classCache = new HashMap<>();
@@ -466,32 +477,36 @@ void testDistance() throws Exception {
         distanceMethod.setAccessible(false);
     }
 
-    @Test
-    void testGetAccessibleInterfaceMethod() throws Exception {
+    @ParameterizedTest
+    @ValueSource(classes = {TestMutable.class, TestMutableSubclass.class})
+    void testGetAccessibleInterfaceMethod(final Class<?> clazz) throws Exception {
         final Class<?>[][] p = {ArrayUtils.EMPTY_CLASS_ARRAY, null};
         for (final Class<?>[] element : p) {
-            final Method method = TestMutable.class.getMethod("getValue", element);
+            final Method method = clazz.getMethod("getValue", element);
             final Method accessibleMethod = MethodUtils.getAccessibleMethod(method);
             assertNotSame(accessibleMethod, method);
             assertSame(Mutable.class, accessibleMethod.getDeclaringClass());
+            final Method accessibleMethod2 = MethodUtils.getAccessibleMethod(clazz, method);
+            assertNotSame(accessibleMethod2, method);
+            assertSame(Mutable.class, accessibleMethod2.getDeclaringClass());
         }
     }
 
-    @Test
-    void testGetAccessibleInterfaceMethodFromDescription() {
-        final Class<?>[][] p = {ArrayUtils.EMPTY_CLASS_ARRAY, null};
+    @ParameterizedTest
+    @ValueSource(classes = {TestMutable.class, TestMutableSubclass.class})
+    void testGetAccessibleInterfaceMethodFromDescription(final Class<?> clazz) {
+        final Class<?>[][] p = { ArrayUtils.EMPTY_CLASS_ARRAY, null };
         for (final Class<?>[] element : p) {
-            final Method accessibleMethod = MethodUtils.getAccessibleMethod(
-                    TestMutable.class, "getValue", element);
+            final Method accessibleMethod = MethodUtils.getAccessibleMethod(clazz, "getValue", element);
             assertSame(Mutable.class, accessibleMethod.getDeclaringClass());
         }
     }
 
     @Test
     void testGetAccessibleMethodInaccessible() throws Exception {
-        final Method expected = TestBean.class.getDeclaredMethod("privateStuff");
-        final Method actual = MethodUtils.getAccessibleMethod(expected);
-        assertNull(actual);
+        assertNull(MethodUtils.getAccessibleMethod(TestBean.class.getDeclaredMethod("privateStuff")));
+        assertNull(MethodUtils.getAccessibleMethod(TestBean.class, TestBean.class.getDeclaredMethod("privateStuff")));
+        assertNull(MethodUtils.getAccessibleMethod(TestBeanSubclass.class, TestBean.class.getDeclaredMethod("privateStuff")));
     }
 
     @Test
@@ -502,18 +517,52 @@ void testGetAccessibleMethodPrivateInterface() throws Exception {
         assertNull(actual);
     }
 
+    @Test
+    void testGetAccessibleMethodPublicSub() throws Exception {
+        // PackageBean class is package-private
+        final int modifiers = PackageBean.class.getModifiers();
+        assertFalse(Modifier.isPrivate(modifiers));
+        assertFalse(Modifier.isProtected(modifiers));
+        assertFalse(Modifier.isPublic(modifiers));
+        // make sure that bean does what it should: compile
+        new PublicSubBean().setBar("");
+        // make sure that bean does what it should
+        final PublicSubBean bean = new PublicSubBean();
+        assertEquals(bean.getFoo(), "This is foo", "Start value (foo)");
+        assertEquals(bean.getBar(), "This is bar", "Start value (bar)");
+        bean.setFoo("new foo");
+        bean.setBar("new bar");
+        assertEquals(bean.getFoo(), "new foo", "Set value (foo)");
+        assertEquals(bean.getBar(), "new bar", "Set value (bar)");
+        // see if we can access public methods in a default access superclass
+        // from a public access subclass instance
+        MethodUtils.invokeExactMethod(bean, "setFoo", "alpha");
+        assertEquals(bean.getFoo(), "alpha", "Set value (foo:2)");
+        MethodUtils.invokeExactMethod(bean, "setBar", "beta");
+        assertEquals(bean.getBar(), "beta", "Set value (bar:2)");
+        // PublicSubBean.setFoo(String)
+        Method method = MethodUtils.getAccessibleMethod(PublicSubBean.class, "setFoo", String.class);
+        assertNotNull(method, "getAccessibleMethod() setFoo is Null");
+        method.invoke(bean, "1111");
+        assertEquals("1111", bean.getFoo(), "Set value (foo:3)");
+        // PublicSubBean.setBar(String)
+        method = MethodUtils.getAccessibleMethod(PublicSubBean.class, "setBar", String.class);
+        assertNotNull(method, "getAccessibleMethod() setBar is Null");
+        method.invoke(bean, "2222");
+        assertEquals("2222", bean.getBar(), "Set value (bar:3)");
+    }
+
     @Test
     void testGetAccessiblePublicMethod() throws Exception {
-        assertSame(MutableObject.class, MethodUtils.getAccessibleMethod(
-                MutableObject.class.getMethod("getValue",
-                        ArrayUtils.EMPTY_CLASS_ARRAY)).getDeclaringClass());
+        assertSame(MutableObject.class,
+                MethodUtils.getAccessibleMethod(MutableObject.class.getMethod("getValue", ArrayUtils.EMPTY_CLASS_ARRAY)).getDeclaringClass());
+        assertSame(MutableObject.class, MethodUtils
+                .getAccessibleMethod(MutableObject.class, MutableObject.class.getMethod("getValue", ArrayUtils.EMPTY_CLASS_ARRAY)).getDeclaringClass());
     }
 
     @Test
     void testGetAccessiblePublicMethodFromDescription() {
-        assertSame(MutableObject.class, MethodUtils.getAccessibleMethod(
-                MutableObject.class, "getValue", ArrayUtils.EMPTY_CLASS_ARRAY)
-                .getDeclaringClass());
+        assertSame(MutableObject.class, MethodUtils.getAccessibleMethod(MutableObject.class, "getValue", ArrayUtils.EMPTY_CLASS_ARRAY).getDeclaringClass());
     }
 
     @Test

src/test/java/org/apache/commons/lang3/reflect/PackageBean.java

@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *      https://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.apache.commons.lang3.reflect;
+
+/**
+ * This class is designed to test the default access JVM problem workaround. The issue is that public methods of a public subclass contained in a default access
+ * superclass are returned by reflection but an IllegalAccessException is thrown when they are invoked.
+ * <p>
+ * This is the default access superclass
+ * </p>
+ */
+class PackageBean {
+
+    private String bar = "This is bar";
+
+    /**
+     * Package private constructor, can only use factory method to create beans.
+     */
+    PackageBean() {
+    }
+
+    public String getBar() {
+        return this.bar;
+    }
+
+    public void setBar(final String bar) {
+        this.bar = bar;
+    }
+}

src/test/java/org/apache/commons/lang3/reflect/PublicSubBean.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *      https://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.apache.commons.lang3.reflect;
+
+/**
+ * This class is designed to test the default access JVM problem workaround. The issue is that public methods of a public subclass contained in a default access
+ * superclass are returned by reflection but an IllegalAccessException is thrown when they are invoked.
+ * <p>
+ * This is the default access superclass
+ * </p>
+ */
+public class PublicSubBean extends PackageBean {
+
+    /**
+     * A directly implemented property.
+     */
+    private String foo = "This is foo";
+
+    /**
+     * Package private constructor, can only use factory method to create beans.
+     */
+    public PublicSubBean() {
+    }
+
+    public String getFoo() {
+        return this.foo;
+    }
+
+    public void setFoo(final String foo) {
+        this.foo = foo;
+    }
+}