WW-4744: AnnotationUtils supports non-public methods

core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java

@@ -19,6 +19,7 @@
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.interceptor.annotations.InputConfig;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.reflect.MethodUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import com.opensymphony.xwork2.util.AnnotationUtils;
@@ -210,8 +211,7 @@ protected String processInputConfig(final Object action, final String method, fi
         InputConfig annotation = AnnotationUtils.findAnnotation(action.getClass().getMethod(method, EMPTY_CLASS_ARRAY), InputConfig.class);
         if (annotation != null) {
             if (StringUtils.isNotEmpty(annotation.methodName())) {
-                Method m = action.getClass().getMethod(annotation.methodName());
-                resultName = (String) m.invoke(action);
+                resultName = (String) MethodUtils.invokeMethod(action, true, annotation.methodName());
             } else {
                 resultName = annotation.resultName();
             }

core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java

@@ -20,6 +20,7 @@
 import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
 import com.opensymphony.xwork2.interceptor.PreResultListener;
 import com.opensymphony.xwork2.util.AnnotationUtils;
+import org.apache.commons.lang3.reflect.MethodUtils;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -117,12 +118,12 @@ public String intercept(ActionInvocation invocation) throws Exception {
             // methods are only sorted by priority
             Collections.sort(methods, new Comparator<Method>() {
                 public int compare(Method method1, Method method2) {
-                    return comparePriorities(method1.getAnnotation(Before.class).priority(),
-                                method2.getAnnotation(Before.class).priority());
+                    return comparePriorities(AnnotationUtils.findAnnotation(method1, Before.class).priority(),
+                            AnnotationUtils.findAnnotation(method2, Before.class).priority());
                 }
             });
             for (Method m : methods) {
-                final String resultCode = (String) m.invoke(action, (Object[]) null);
+                final String resultCode = (String) MethodUtils.invokeMethod(action, true, m.getName());
                 if (resultCode != null) {
                     // shortcircuit execution
                     return resultCode;
@@ -139,12 +140,12 @@ public int compare(Method method1, Method method2) {
             // methods are only sorted by priority
             Collections.sort(methods, new Comparator<Method>() {
                 public int compare(Method method1, Method method2) {
-                    return comparePriorities(method1.getAnnotation(After.class).priority(),
-                                method2.getAnnotation(After.class).priority());
+                    return comparePriorities(AnnotationUtils.findAnnotation(method1, After.class).priority(),
+                            AnnotationUtils.findAnnotation(method2, After.class).priority());
                 }
             });
             for (Method m : methods) {
-                m.invoke(action, (Object[]) null);
+                MethodUtils.invokeMethod(action, true, m.getName());
             }
         }
 
@@ -174,13 +175,13 @@ public void beforeResult(ActionInvocation invocation, String resultCode) {
             // methods are only sorted by priority
             Collections.sort(methods, new Comparator<Method>() {
                 public int compare(Method method1, Method method2) {
-                    return comparePriorities(method1.getAnnotation(BeforeResult.class).priority(),
-                                method2.getAnnotation(BeforeResult.class).priority());
+                    return comparePriorities(AnnotationUtils.findAnnotation(method1, BeforeResult.class).priority(),
+                            AnnotationUtils.findAnnotation(method2, BeforeResult.class).priority());
                 }
             });
             for (Method m : methods) {
                 try {
-                    m.invoke(action, (Object[]) null);
+                    MethodUtils.invokeMethod(action, true, m.getName());
                 } catch (Exception e) {
                     throw new XWorkException(e);
                 }

core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java

@@ -16,14 +16,15 @@
 package com.opensymphony.xwork2.util;
 
 import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.ClassUtils;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -116,123 +117,103 @@ public static void addAllInterfaces(Class clazz, List<Class> allInterfaces) {
      * @param annotation the {@link Annotation}s to find
      * @return A {@link Collection}&lt;{@link AnnotatedElement}&gt; containing all of the
      * method {@link AnnotatedElement}s matching the specified {@link Annotation}s
+     * @deprecated Will be removed after release of <a href="https://github.com/apache/commons-lang/pull/261">LANG-1317</a>
      */
+    @Deprecated
     public static Collection<Method> getAnnotatedMethods(Class clazz, Class<? extends Annotation>... annotation) {
-        Collection<Method> toReturn = new HashSet<>();
-
-        for (Method m : clazz.getMethods()) {
-            boolean found = false;
-            for (Class<? extends Annotation> c : annotation) {
-                if (null != findAnnotation(m, c)) {
-                    found = true;
-                    break;
-                }
+        List<Class<?>> allSuperclasses = ClassUtils.getAllSuperclasses(clazz);
+        allSuperclasses.add(0, clazz);
+        int sci = 0;
+        List<Class<?>> allInterfaces = ClassUtils.getAllInterfaces(clazz);
+        int ifi = 0;
+        final List<Method> annotatedMethods = new ArrayList<>();
+        while (ifi < allInterfaces.size() ||
+                sci < allSuperclasses.size()) {
+            Class<?> acls;
+            if (ifi >= allInterfaces.size()) {
+                acls = allSuperclasses.get(sci++);
             }
-            if (found) {
-                toReturn.add(m);
-            } else if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(m.getAnnotations())) {
-                toReturn.add(m);
+            else if (sci >= allSuperclasses.size()) {
+                acls = allInterfaces.get(ifi++);
             }
-        }
-
-        return toReturn;
-    }
-
-    /**
-     * Find a single {@link Annotation} of {@code annotationType} from the supplied
-     * {@link Method}, traversing its super methods (i.e., from superclasses and
-     * interfaces) if no annotation can be found on the given method itself.
-     * <p>Annotations on methods are not inherited by default, so we need to handle
-     * this explicitly.
-     *
-     * @param method         the method to look for annotations on
-     * @param annotationType the annotation type to look for
-     * @return the annotation found, or {@code null} if none
-     */
-    public static <A extends Annotation> A findAnnotation(Method method, Class<A> annotationType) {
-        A result = getAnnotation(method, annotationType);
-        Class<?> clazz = method.getDeclaringClass();
-        if (result == null) {
-            result = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
-        }
-        while (result == null) {
-            clazz = clazz.getSuperclass();
-            if (clazz == null || clazz.equals(Object.class)) {
-                break;
+            else if (sci <= ifi) {
+                acls = allSuperclasses.get(sci++);
             }
-            try {
-                Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
-                result = getAnnotation(equivalentMethod, annotationType);
-            } catch (NoSuchMethodException ex) {
-                // No equivalent method found
+            else {
+                acls = allInterfaces.get(ifi++);
             }
-            if (result == null) {
-                result = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
+            final Method[] allMethods = acls.getDeclaredMethods();
+            for (final Method method : allMethods) {
+                if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(method.getAnnotations())) {
+                    annotatedMethods.add(method);
+                    continue;
+                }
+                for (Class<? extends Annotation> c : annotation) {
+                    if (method.getAnnotation(c) != null) {
+                        annotatedMethods.add(method);
+                    }
+                }
             }
         }
-        return result;
+
+        return annotatedMethods;
     }
 
     /**
-     * Get a single {@link Annotation} of {@code annotationType} from the supplied
-     * Method, Constructor or Field. Meta-annotations will be searched if the annotation
-     * is not declared locally on the supplied element.
-     *
-     * @param annotatedElement the Method, Constructor or Field from which to get the annotation
-     * @param annotationType   the annotation type to look for, both locally and as a meta-annotation
-     * @return the matching annotation, or {@code null} if none found
+     * <p>BFS to find the annotation object that is present on the given method or any equivalent method in
+     * super classes and interfaces, with the given annotation type. Returns null if the annotation type was not present
+     * on any of them.</p>
+     * @param <A>
+     *            the annotation type
+     * @param method
+     *            the {@link Method} to query
+     * @param annotationCls
+     *            the {@link Annotation} to check if is present on the method
+     * @return an Annotation (possibly null).
+     * @deprecated Will be removed after release of <a href="https://github.com/apache/commons-lang/pull/261">LANG-1317</a>
      */
-    public static <T extends Annotation> T getAnnotation(AnnotatedElement annotatedElement, Class<T> annotationType) {
-        try {
-            T ann = annotatedElement.getAnnotation(annotationType);
-            if (ann == null) {
-                for (Annotation metaAnn : annotatedElement.getAnnotations()) {
-                    ann = metaAnn.annotationType().getAnnotation(annotationType);
-                    if (ann != null) {
-                        break;
-                    }
-                }
-            }
-            return ann;
-        } catch (Exception ex) {
-            // Assuming nested Class values not resolvable within annotation attributes...
-            return null;
-        }
-    }
+    @Deprecated
+    public static <A extends Annotation> A findAnnotation(final Method method, final Class<A> annotationCls) {
+        A annotation = method.getAnnotation(annotationCls);
 
-    private static <A extends Annotation> A searchOnInterfaces(Method method, Class<A> annotationType, Class<?>... ifcs) {
-        A annotation = null;
-        for (Class<?> iface : ifcs) {
-            if (isInterfaceWithAnnotatedMethods(iface)) {
+        if(annotation == null) {
+            Class<?> mcls = method.getDeclaringClass();
+            List<Class<?>> allSuperclasses = ClassUtils.getAllSuperclasses(mcls);
+            int sci = 0;
+            List<Class<?>> allInterfaces = ClassUtils.getAllInterfaces(mcls);
+            int ifi = 0;
+            while (ifi < allInterfaces.size() ||
+                    sci < allSuperclasses.size()) {
+                Class<?> acls;
+                if(ifi >= allInterfaces.size()) {
+                    acls = allSuperclasses.get(sci++);
+                }
+                else if(sci >= allSuperclasses.size()) {
+                    acls = allInterfaces.get(ifi++);
+                }
+                else if(ifi <= sci) {
+                    acls = allInterfaces.get(ifi++);
+                }
+                else {
+                    acls = allSuperclasses.get(sci++);
+                }
+                Method equivalentMethod = null;
                 try {
-                    Method equivalentMethod = iface.getMethod(method.getName(), method.getParameterTypes());
-                    annotation = getAnnotation(equivalentMethod, annotationType);
-                } catch (NoSuchMethodException ex) {
-                    // Skip this interface - it doesn't have the method...
+                    equivalentMethod = acls.getDeclaredMethod(method.getName(), method.getParameterTypes());
+                } catch (NoSuchMethodException e) {
+                    // If not found, just keep on breadth first search
                 }
-                if (annotation != null) {
-                    break;
+                if(equivalentMethod != null) {
+                    annotation = equivalentMethod.getAnnotation(annotationCls);
+                    if(annotation != null) {
+                        break;
+                    }
                 }
             }
         }
         return annotation;
     }
 
-    private static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
-        boolean found = false;
-        for (Method ifcMethod : iface.getMethods()) {
-            try {
-                if (ifcMethod.getAnnotations().length > 0) {
-                    found = true;
-                    break;
-                }
-            } catch (Exception ex) {
-                // Assuming nested Class values not resolvable within annotation attributes...
-            }
-        }
-        return found;
-    }
-
     /**
      * Returns the property name for a method.
      * This method is independent from property fields.

core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java

@@ -23,17 +23,12 @@
 
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.config.ConfigurationException;
-import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.util.AnnotationUtils;
 import com.opensymphony.xwork2.validator.ValidationInterceptor;
-import org.apache.commons.lang3.BooleanUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.struts2.StrutsConstants;
 
 import java.lang.reflect.Method;
-import java.util.Arrays;
-import java.util.Collection;
 
 /**
  * Extends the xwork validation interceptor to also check for a @SkipValidation
@@ -49,25 +44,9 @@ protected String doIntercept(ActionInvocation invocation) throws Exception {
         if (action != null) {
             Method method = getActionMethod(action.getClass(), invocation.getProxy().getMethod());
 
-            Collection<Method> annotatedMethods = AnnotationUtils.getAnnotatedMethods(action.getClass(), SkipValidation.class);
-            if (annotatedMethods.contains(method)) {
+            if (null != AnnotationUtils.findAnnotation(method, SkipValidation.class)) {
                 return invocation.invoke();
             }
-
-            LOG.debug("Check if method overrides an annotated method");
-            Class clazz = action.getClass().getSuperclass();
-            while (clazz != null) {
-                annotatedMethods = AnnotationUtils.getAnnotatedMethods(clazz, SkipValidation.class);
-                if (annotatedMethods != null) {
-                    for (Method annotatedMethod : annotatedMethods) {
-                        if (annotatedMethod.getName().equals(method.getName())
-                                && Arrays.equals(annotatedMethod.getParameterTypes(), method.getParameterTypes())
-                                && Arrays.equals(annotatedMethod.getExceptionTypes(), method.getExceptionTypes()))
-                            return invocation.invoke();
-                    }
-                }
-                clazz = clazz.getSuperclass();
-            }
         }
 
         return super.doIntercept(invocation);

core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java

@@ -51,14 +51,14 @@ public void testInterceptsBeforeAndAfter() throws Exception {
         ActionProxy proxy = actionProxyFactory.createActionProxy("", ANNOTATED_ACTION, null, null);
         assertEquals(Action.SUCCESS, proxy.execute());
         AnnotatedAction action = (AnnotatedAction)proxy.getInvocation().getAction();
-        assertEquals("baseBefore-before-execute-beforeResult-after", action.log);
+        assertEquals("interfaceBefore-baseBefore-basePrivateBefore-before-execute-beforeResult-basePrivateBeforeResult-interfaceBeforeResult-after-basePrivateAfter-interfaceAfter", action.log);
     }
 
     public void testInterceptsShortcircuitedAction() throws Exception {
         ActionProxy proxy = actionProxyFactory.createActionProxy("", SHORTCIRCUITED_ACTION, null, null);
         assertEquals("shortcircuit", proxy.execute());
         ShortcircuitedAction action = (ShortcircuitedAction)proxy.getInvocation().getAction();
-        assertEquals("baseBefore-before", action.log);
+        assertEquals("interfaceBefore-baseBefore-basePrivateBefore-before-basePrivateBeforeResult-interfaceBeforeResult", action.log);
     }
 
     private class MockConfigurationProvider implements ConfigurationProvider {

core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java

@@ -19,7 +19,7 @@
  * @author Zsolt Szasz, zsolt at lorecraft dot com
  * @author Rainer Hermanns
  */
-public class BaseAnnotatedAction {
+public class BaseAnnotatedAction implements InterfaceAnnotatedAction {
 
 	protected String log = "";
 
@@ -29,4 +29,36 @@ public String baseBefore() {
 		return null;
 	}
 
+    @Override
+    public String interfaceBefore() {
+        log = log + "interfaceBefore-";
+        return null;
+    }
+
+    @Override
+    public void interfaceBeforeResult() {
+        log = log + "-interfaceBeforeResult";
+    }
+
+    @Override
+    public void interfaceAfter() {
+        log = log + "-interfaceAfter";
+    }
+
+    @Before(priority=6)
+    private String basePrivateBefore() {
+        log = log + "basePrivateBefore-";
+        return null;
+    }
+
+    @BeforeResult(priority=4)
+    private void basePrivateBeforeResult() {
+        log = log + "-basePrivateBeforeResult";
+    }
+
+    @After(priority=4)
+    private void basePrivateAfter() {
+        log = log + "-basePrivateAfter";
+    }
+
 }