Repository: struts Updated Branches: refs/heads/master 7627b8355 -> ed20ccd3f
WW-4744: AnnotationUtils supports non-public methods Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/e1bb1a70 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/e1bb1a70 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/e1bb1a70 Branch: refs/heads/master Commit: e1bb1a70c089b80b3924391da9d0268b5bcc29f3 Parents: 6dcff10 Author: Yasser Zamani <[email protected]> Authored: Sun Mar 19 20:02:45 2017 +0430 Committer: Yasser Zamani <[email protected]> Committed: Sun Mar 19 20:02:45 2017 +0430 ---------------------------------------------------------------------- .../interceptor/DefaultWorkflowInterceptor.java | 6 +- .../AnnotationWorkflowInterceptor.java | 22 +- .../xwork2/util/AnnotationUtils.java | 147 +++++++--- .../xwork2/util/ReflectionUtils.java | 274 +++++++++++++++++++ .../AnnotationValidationInterceptor.java | 18 +- .../AnnotationWorkflowInterceptorTest.java | 4 +- .../annotations/BaseAnnotatedAction.java | 34 ++- .../annotations/InterfaceAnnotatedAction.java | 12 + .../xwork2/util/AnnotationUtilsTest.java | 13 +- .../xwork2/util/annotation/DummyClass.java | 4 + .../xwork2/util/annotation/DummyInterface.java | 2 +- .../AnnotationValidationInterceptorTest.java | 33 ++- .../interceptor/BeanValidationInterceptor.java | 4 +- .../BeanValidationInterceptorTest.java | 14 + .../actions/ModelDrivenAction.java | 6 +- .../actions/ModelDrivenActionInterface.java | 8 + .../src/test/resources/bean-validation-test.xml | 5 + 17 files changed, 524 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java index a9f2565..38fa810 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java @@ -18,6 +18,7 @@ package com.opensymphony.xwork2.interceptor; import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.interceptor.annotations.InputConfig; +import com.opensymphony.xwork2.util.ReflectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -210,8 +211,9 @@ public class DefaultWorkflowInterceptor extends MethodFilterInterceptor { 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); + Method m = ReflectionUtils.findMethod(action.getClass(), annotation.methodName()); + ReflectionUtils.makeAccessible(m); + resultName = (String) ReflectionUtils.invokeMethod(m, action); } else { resultName = annotation.resultName(); } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java index dbe4315..122d245 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java @@ -20,6 +20,7 @@ import com.opensymphony.xwork2.XWorkException; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; import com.opensymphony.xwork2.interceptor.PreResultListener; import com.opensymphony.xwork2.util.AnnotationUtils; +import com.opensymphony.xwork2.util.ReflectionUtils; import java.lang.reflect.Method; import java.util.ArrayList; @@ -117,12 +118,13 @@ public class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // 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); + ReflectionUtils.makeAccessible(m); + final String resultCode = (String) ReflectionUtils.invokeMethod(m, action); if (resultCode != null) { // shortcircuit execution return resultCode; @@ -139,12 +141,13 @@ public class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // 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); + ReflectionUtils.makeAccessible(m); + ReflectionUtils.invokeMethod(m, action); } } @@ -174,13 +177,14 @@ public class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // 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); + ReflectionUtils.makeAccessible(m); + ReflectionUtils.invokeMethod(m, action); } catch (Exception e) { throw new XWorkException(e); } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java b/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java index 36166dd..87442c4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java @@ -1,6 +1,7 @@ /* * Copyright 2002-2006,2009 The Apache Software Foundation. - * + * Copyright 2002-2014 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 @@ -25,6 +26,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -37,6 +40,12 @@ import java.util.regex.Pattern; * @author Rainer Hermanns * @author Zsolt Szasz, zsolt at lorecraft dot com * @author Dan Oxlade, dan d0t oxlade at gmail d0t c0m + * @author Rob Harrop + * @author Juergen Hoeller + * @author Sam Brannen + * @author Mark Fisher + * @author Chris Beams + * @author Phillip Webb * @version $Id$ */ public class AnnotationUtils { @@ -44,6 +53,12 @@ public class AnnotationUtils { private static final Pattern SETTER_PATTERN = Pattern.compile("set([A-Z][A-Za-z0-9]*)$"); private static final Pattern GETTER_PATTERN = Pattern.compile("(get|is|has)([A-Z][A-Za-z0-9]*)$"); + private static final Map<AnnotationCacheKey, Annotation> findAnnotationCache = + new ConcurrentHashMap<AnnotationCacheKey, Annotation>(256); + + private static final Map<Class<?>, Boolean> annotatedInterfaceCache = + new ConcurrentHashMap<Class<?>, Boolean>(256); + /** * Adds all fields with the specified Annotation of class clazz and its superclasses to allFields * @@ -117,23 +132,24 @@ public class AnnotationUtils { * @return A {@link Collection}<{@link AnnotatedElement}> containing all of the * method {@link AnnotatedElement}s matching the specified {@link Annotation}s */ - public static Collection<Method> getAnnotatedMethods(Class clazz, Class<? extends Annotation>... annotation) { - Collection<Method> toReturn = new HashSet<>(); + public static Collection<Method> getAnnotatedMethods(Class clazz, final Class<? extends Annotation>... annotation) { + final 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; + ReflectionUtils.doWithMethods(clazz, new ReflectionUtils.MethodCallback() { + @Override + public void doWith(Method method) throws IllegalArgumentException { + if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(method.getAnnotations())) { + toReturn.add(method); + return; + } + for (Class<? extends Annotation> c : annotation) { + if (null != findAnnotation(method, c)) { + toReturn.add(method); + break; + } } } - if (found) { - toReturn.add(m); - } else if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(m.getAnnotations())) { - toReturn.add(m); - } - } + }); return toReturn; } @@ -150,25 +166,32 @@ public class AnnotationUtils { * @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(); + AnnotationCacheKey cacheKey = new AnnotationCacheKey(method, annotationType); + A result = (A) findAnnotationCache.get(cacheKey); if (result == null) { - result = searchOnInterfaces(method, annotationType, clazz.getInterfaces()); - } - while (result == null) { - clazz = clazz.getSuperclass(); - if (clazz == null || clazz.equals(Object.class)) { - break; - } - try { - Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); - result = getAnnotation(equivalentMethod, annotationType); - } catch (NoSuchMethodException ex) { - // No equivalent method found - } + 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; + } + try { + Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); + result = getAnnotation(equivalentMethod, annotationType); + } catch (NoSuchMethodException ex) { + // No equivalent method found + } + if (result == null) { + result = searchOnInterfaces(method, annotationType, clazz.getInterfaces()); + } + } + if (result != null) { + findAnnotationCache.put(cacheKey, result); + } } return result; } @@ -219,6 +242,10 @@ public class AnnotationUtils { } private static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) { + Boolean flag = annotatedInterfaceCache.get(iface); + if (flag != null) { + return flag; + } boolean found = false; for (Method ifcMethod : iface.getMethods()) { try { @@ -230,6 +257,7 @@ public class AnnotationUtils { // Assuming nested Class values not resolvable within annotation attributes... } } + annotatedInterfaceCache.put(iface, found); return found; } @@ -267,20 +295,61 @@ public class AnnotationUtils { * @return The annotation or null. */ public static <T extends Annotation> T findAnnotation(Class<?> clazz, Class<T> annotationClass) { - T ann = clazz.getAnnotation(annotationClass); - while (ann == null && clazz != null) { + AnnotationCacheKey cacheKey = new AnnotationCacheKey(clazz, annotationClass); + T ann = (T) findAnnotationCache.get(cacheKey); + if (ann == null) { ann = clazz.getAnnotation(annotationClass); - if (ann == null) { - ann = clazz.getPackage().getAnnotation(annotationClass); - } - if (ann == null) { - clazz = clazz.getSuperclass(); - if (clazz != null) { - ann = clazz.getAnnotation(annotationClass); + while (ann == null && clazz != null) { + ann = clazz.getAnnotation(annotationClass); + if (ann == null) { + ann = clazz.getPackage().getAnnotation(annotationClass); + } + if (ann == null) { + clazz = clazz.getSuperclass(); + if (clazz != null) { + ann = clazz.getAnnotation(annotationClass); + } } } + if (ann != null) { + findAnnotationCache.put(cacheKey, ann); + } } return ann; } + + + /** + * Cache key for the AnnotatedElement cache. + */ + private static class AnnotationCacheKey { + + private final AnnotatedElement element; + + private final Class<? extends Annotation> annotationType; + + public AnnotationCacheKey(AnnotatedElement element, Class<? extends Annotation> annotationType) { + this.element = element; + this.annotationType = annotationType; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AnnotationCacheKey)) { + return false; + } + AnnotationCacheKey otherKey = (AnnotationCacheKey) other; + return (this.element.equals(otherKey.element) && + this.annotationType.equals(otherKey.annotationType)); + } + + @Override + public int hashCode() { + return (this.element.hashCode() * 29 + this.annotationType.hashCode()); + } + } } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java b/core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java new file mode 100644 index 0000000..08bd933 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java @@ -0,0 +1,274 @@ +/* + * Copyright 2002-2015 The Apache Software Foundation. + * Copyright 2002-2015 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 com.opensymphony.xwork2.util; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.UndeclaredThrowableException; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Simple utility class for working with the reflection API and handling + * reflection exceptions. + * + * <p>Only intended for internal use. + * + * @author Juergen Hoeller + * @author Rob Harrop + * @author Rod Johnson + * @author Costin Leau + * @author Sam Brannen + * @author Chris Beams + */ +public abstract class ReflectionUtils { + + /** + * Cache for {@link Class#getDeclaredMethods()}, allowing for fast iteration. + */ + private static final Map<Class<?>, Method[]> declaredMethodsCache = + new ConcurrentHashMap<Class<?>, Method[]>(256); + + /** + * Perform the given callback operation on all matching methods of the given + * class and superclasses. + * <p>The same named method occurring on subclass and superclass will appear + * twice, unless excluded by a {@link MethodFilter}. + * @param clazz the class to introspect + * @param mc the callback to invoke for each method + * @see #doWithMethods(Class, MethodCallback, MethodFilter) + */ + public static void doWithMethods(Class<?> clazz, MethodCallback mc) { + doWithMethods(clazz, mc, null); + } + + /** + * Perform the given callback operation on all matching methods of the given + * class and superclasses (or given interface and super-interfaces). + * <p>The same named method occurring on subclass and superclass will appear + * twice, unless excluded by the specified {@link MethodFilter}. + * @param clazz the class to introspect + * @param mc the callback to invoke for each method + * @param mf the filter that determines the methods to apply the callback to + */ + public static void doWithMethods(Class<?> clazz, MethodCallback mc, MethodFilter mf) { + // Keep backing up the inheritance hierarchy. + Method[] methods = getDeclaredMethods(clazz); + for (Method method : methods) { + if (mf != null && !mf.matches(method)) { + continue; + } + try { + mc.doWith(method); + } + catch (IllegalAccessException ex) { + throw new IllegalStateException("Not allowed to access method '" + method.getName() + "': " + ex); + } + } + if (clazz.getSuperclass() != null) { + doWithMethods(clazz.getSuperclass(), mc, mf); + } + else if (clazz.isInterface()) { + for (Class<?> superIfc : clazz.getInterfaces()) { + doWithMethods(superIfc, mc, mf); + } + } + } + + /** + * Make the given method accessible, explicitly setting it accessible if + * necessary. The {@code setAccessible(true)} method is only called + * when actually necessary, to avoid unnecessary conflicts with a JVM + * SecurityManager (if active). + * @param method the method to make accessible + * @see java.lang.reflect.Method#setAccessible + */ + public static void makeAccessible(Method method) { + if ((!Modifier.isPublic(method.getModifiers()) || + !Modifier.isPublic(method.getDeclaringClass().getModifiers())) && !method.isAccessible()) { + method.setAccessible(true); + } + } + + /** + * Attempt to find a {@link Method} on the supplied class with the supplied name + * and no parameters. Searches all superclasses up to {@code Object}. + * <p>Returns {@code null} if no {@link Method} can be found. + * @param clazz the class to introspect + * @param name the name of the method + * @return the Method object, or {@code null} if none found + */ + public static Method findMethod(Class<?> clazz, String name) { + return findMethod(clazz, name, new Class<?>[0]); + } + + /** + * Attempt to find a {@link Method} on the supplied class with the supplied name + * and parameter types. Searches all superclasses up to {@code Object}. + * <p>Returns {@code null} if no {@link Method} can be found. + * @param clazz the class to introspect + * @param name the name of the method + * @param paramTypes the parameter types of the method + * (may be {@code null} to indicate any signature) + * @return the Method object, or {@code null} if none found + */ + public static Method findMethod(Class<?> clazz, String name, Class<?>... paramTypes) { + Class<?> searchType = clazz; + while (searchType != null) { + Method[] methods = (searchType.isInterface() ? searchType.getMethods() : getDeclaredMethods(searchType)); + for (Method method : methods) { + if (name.equals(method.getName()) && + (paramTypes == null || Arrays.equals(paramTypes, method.getParameterTypes()))) { + return method; + } + } + searchType = searchType.getSuperclass(); + } + return null; + } + + /** + * Invoke the specified {@link Method} against the supplied target object with no arguments. + * The target object can be {@code null} when invoking a static {@link Method}. + * <p>Thrown exceptions are handled via a call to {@link #handleReflectionException}. + * @param method the method to invoke + * @param target the target object to invoke the method on + * @return the invocation result, if any + * @see #invokeMethod(java.lang.reflect.Method, Object, Object[]) + */ + public static Object invokeMethod(Method method, Object target) { + return invokeMethod(method, target, new Object[0]); + } + + /** + * Invoke the specified {@link Method} against the supplied target object with the + * supplied arguments. The target object can be {@code null} when invoking a + * static {@link Method}. + * <p>Thrown exceptions are handled via a call to {@link #handleReflectionException}. + * @param method the method to invoke + * @param target the target object to invoke the method on + * @param args the invocation arguments (may be {@code null}) + * @return the invocation result, if any + */ + public static Object invokeMethod(Method method, Object target, Object... args) { + try { + return method.invoke(target, args); + } + catch (Exception ex) { + handleReflectionException(ex); + } + throw new IllegalStateException("Should never get here"); + } + + /** + * Handle the given reflection exception. Should only be called if no + * checked exception is expected to be thrown by the target method. + * <p>Throws the underlying RuntimeException or Error in case of an + * InvocationTargetException with such a root cause. Throws an + * IllegalStateException with an appropriate message else. + * @param ex the reflection exception to handle + */ + public static void handleReflectionException(Exception ex) { + if (ex instanceof NoSuchMethodException) { + throw new IllegalStateException("Method not found: " + ex.getMessage()); + } + if (ex instanceof IllegalAccessException) { + throw new IllegalStateException("Could not access method: " + ex.getMessage()); + } + if (ex instanceof InvocationTargetException) { + handleInvocationTargetException((InvocationTargetException) ex); + } + if (ex instanceof RuntimeException) { + throw (RuntimeException) ex; + } + throw new UndeclaredThrowableException(ex); + } + + /** + * Handle the given invocation target exception. Should only be called if no + * checked exception is expected to be thrown by the target method. + * <p>Throws the underlying RuntimeException or Error in case of such a root + * cause. Throws an IllegalStateException else. + * @param ex the invocation target exception to handle + */ + public static void handleInvocationTargetException(InvocationTargetException ex) { + rethrowRuntimeException(ex.getTargetException()); + } + + /** + * Rethrow the given {@link Throwable exception}, which is presumably the + * <em>target exception</em> of an {@link InvocationTargetException}. Should + * only be called if no checked exception is expected to be thrown by the + * target method. + * <p>Rethrows the underlying exception cast to an {@link RuntimeException} or + * {@link Error} if appropriate; otherwise, throws an + * {@link IllegalStateException}. + * @param ex the exception to rethrow + * @throws RuntimeException the rethrown exception + */ + public static void rethrowRuntimeException(Throwable ex) { + if (ex instanceof RuntimeException) { + throw (RuntimeException) ex; + } + if (ex instanceof Error) { + throw (Error) ex; + } + throw new UndeclaredThrowableException(ex); + } + + /** + * This variant retrieves {@link Class#getDeclaredMethods()} from a local cache + * in order to avoid the JVM's SecurityManager check and defensive array copying. + */ + private static Method[] getDeclaredMethods(Class<?> clazz) { + Method[] result = declaredMethodsCache.get(clazz); + if (result == null) { + result = clazz.getDeclaredMethods(); + declaredMethodsCache.put(clazz, result); + } + return result; + } + + + /** + * Action to take on each method. + */ + public interface MethodCallback { + + /** + * Perform an operation using the given method. + * @param method the method to operate on + */ + void doWith(Method method) throws IllegalArgumentException, IllegalAccessException; + } + + + /** + * Callback optionally used to filter methods to be operated on by a method callback. + */ + public interface MethodFilter { + + /** + * Determine whether the given method matches. + * @param method the method to check + */ + boolean matches(Method method); + } +} http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java index 572572e..26e1545 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java @@ -49,25 +49,9 @@ public class AnnotationValidationInterceptor extends ValidationInterceptor { 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); http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java index 7b39267..a500571 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java @@ -51,14 +51,14 @@ public class AnnotationWorkflowInterceptorTest extends XWorkTestCase { 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 { http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java index ed088f1..7fd294a 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java @@ -19,7 +19,7 @@ package com.opensymphony.xwork2.interceptor.annotations; * @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 class BaseAnnotatedAction { 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"; + } + } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java new file mode 100644 index 0000000..2761b23 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java @@ -0,0 +1,12 @@ +package com.opensymphony.xwork2.interceptor.annotations; + +public interface InterfaceAnnotatedAction { + @Before + String interfaceBefore(); + + @BeforeResult(priority=3) + void interfaceBeforeResult(); + + @After(priority=3) + void interfaceAfter(); +} http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java b/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java index 8ad4f35..7412116 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java @@ -43,14 +43,15 @@ public class AnnotationUtilsTest extends TestCase { public void testGetAnnotatedMethodsIncludingSuperclassAndInterface() throws Exception { Collection<? extends AnnotatedElement> ans = AnnotationUtils.getAnnotatedMethods(DummyClassExt.class, Deprecated.class, MyAnnotation.class, MyAnnotation2.class, MyAnnotationI.class); - assertEquals(3, ans.size()); + assertEquals(5, ans.size()); } @SuppressWarnings("unchecked") - public void testGetAnnotedMethodsWithoutAnnotationArgs() throws Exception { + public void testGetAnnotatedMethodsWithoutAnnotationArgs() throws Exception { Collection<? extends AnnotatedElement> ans = AnnotationUtils.getAnnotatedMethods(DummyClass.class); - assertTrue(ans.size() == 1); - assertEquals(ans.iterator().next(), DummyClass.class.getMethod("methodWithAnnotation")); + assertTrue(ans.size() == 2); + assertTrue(ans.contains(DummyClass.class.getMethod("methodWithAnnotation"))); + assertTrue(ans.contains(DummyClass.class.getDeclaredMethod("privateMethodWithAnnotation"))); } @SuppressWarnings("unchecked") @@ -65,10 +66,10 @@ public class AnnotationUtilsTest extends TestCase { assertEquals(1, ans.size()); ans = AnnotationUtils.getAnnotatedMethods(DummyClass.class, MyAnnotation.class, MyAnnotation2.class); - assertEquals(1, ans.size()); + assertEquals(2, ans.size()); ans = AnnotationUtils.getAnnotatedMethods(DummyClassExt.class, MyAnnotation.class, MyAnnotation2.class); - assertEquals(2, ans.size()); + assertEquals(4, ans.size()); } public void testFindAnnotationOnClass() { http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java b/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java index da2f80e..f6a4be1 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java @@ -13,4 +13,8 @@ public class DummyClass implements DummyInterface { @Override public void interfaceMethodWithAnnotation() { } + + @MyAnnotation2 + private void privateMethodWithAnnotation() { + } } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java b/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java index 6faa3e4..7713d3f 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java @@ -3,5 +3,5 @@ package com.opensymphony.xwork2.util.annotation; public interface DummyInterface { @MyAnnotationI - public void interfaceMethodWithAnnotation(); + void interfaceMethodWithAnnotation(); } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java index 85bec79..0abe57d 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java @@ -78,6 +78,18 @@ public class AnnotationValidationInterceptorTest extends StrutsInternalTestCase mockActionProxy.verify(); } + public void testShouldSkipProtected() throws Exception { + mockActionProxy.expectAndReturn("getMethod", "skipMeProtected"); + interceptor.doIntercept((ActionInvocation)mockActionInvocation.proxy()); + mockActionProxy.verify(); + } + + public void testShouldSkipByInterface() throws Exception { + mockActionProxy.expectAndReturn("getMethod", "skipMeByInterface"); + interceptor.doIntercept((ActionInvocation)mockActionInvocation.proxy()); + mockActionProxy.verify(); + } + public void testShouldSkip2() throws Exception { mockActionProxy.expectAndReturn("getMethod", "skipMe2"); interceptor.doIntercept((ActionInvocation)mockActionInvocation.proxy()); @@ -112,9 +124,14 @@ public class AnnotationValidationInterceptorTest extends StrutsInternalTestCase public String skipMeBase() { return "skipme"; } + + @Override + public String skipMeProtected() { + return super.skipMeProtected(); + } } - public static class TestActionBase { + public static class TestActionBase implements TestActionInterface { @SkipValidation public String skipMeBase() { @@ -133,6 +150,20 @@ public class AnnotationValidationInterceptorTest extends StrutsInternalTestCase public String skipMe2() { return "skipme2"; } + + @SkipValidation + protected String skipMeProtected() { + return "skipMeProtected"; + } + + @Override + public String skipMeByInterface() { + return "skipMeByInterface"; + } } + public static interface TestActionInterface { + @SkipValidation + String skipMeByInterface(); + } } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java ---------------------------------------------------------------------- diff --git a/plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java b/plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java index 3123a4b..6059070 100644 --- a/plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java +++ b/plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java @@ -99,9 +99,7 @@ public class BeanValidationInterceptor extends MethodFilterInterceptor { LOG.debug("Validating [{}/{}] with method [{}]", invocation.getProxy().getNamespace(), invocation.getProxy().getActionName(), methodName); } - Collection<Method> annotatedMethods = AnnotationUtils.getAnnotatedMethods(action.getClass(), SkipValidation.class); - - if (!annotatedMethods.contains(getActionMethod(action.getClass(), methodName))) { + if (null == AnnotationUtils.findAnnotation(getActionMethod(action.getClass(), methodName), SkipValidation.class)) { // performing bean validation on action performBeanValidation(action, validator); } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java ---------------------------------------------------------------------- diff --git a/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java index 5db0b36..2d04ba2 100644 --- a/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java +++ b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java @@ -106,6 +106,20 @@ public class BeanValidationInterceptorTest extends XWorkTestCase { assertEquals(0, fieldErrors.size()); } + public void testModelDrivenActionSkipValidationByInterface() throws Exception { + ActionProxy baseActionProxy = actionProxyFactory.createActionProxy("bean-validation", "modelDrivenActionSkipValidationByInterface", null, null); + ModelDrivenAction action = (ModelDrivenAction) baseActionProxy.getAction(); + action.getModel().setName(null); + action.getModel().setEmail(null); + action.getModel().getAddress().setStreet(null); + baseActionProxy.execute(); + + Map<String, List<String>> fieldErrors = ((ValidationAware) baseActionProxy.getAction()).getFieldErrors(); + + assertNotNull(fieldErrors); + assertEquals(0, fieldErrors.size()); + } + public void testFieldAction() throws Exception { ActionProxy baseActionProxy = actionProxyFactory.createActionProxy("bean-validation", "fieldAction", null, null); FieldAction action = (FieldAction) baseActionProxy.getAction(); http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java ---------------------------------------------------------------------- diff --git a/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java index a51c6b5..2de4764 100644 --- a/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java +++ b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java @@ -26,7 +26,7 @@ import org.apache.struts.beanvalidation.models.Person; import javax.validation.Valid; -public class ModelDrivenAction extends ActionSupport implements ModelDriven<Person> { +public class ModelDrivenAction extends ActionSupport implements ModelDriven<Person>, ModelDrivenActionInterface { @Valid private Person model = new Person(); @@ -35,4 +35,8 @@ public class ModelDrivenAction extends ActionSupport implements ModelDriven<Pers return model; } + @Override + public String skipMeByInterface() { + return SUCCESS; + } } http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java ---------------------------------------------------------------------- diff --git a/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java new file mode 100644 index 0000000..9eba4bf --- /dev/null +++ b/plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java @@ -0,0 +1,8 @@ +package org.apache.struts.beanvalidation.actions; + +import org.apache.struts2.interceptor.validation.SkipValidation; + +public interface ModelDrivenActionInterface { + @SkipValidation + String skipMeByInterface(); +} http://git-wip-us.apache.org/repos/asf/struts/blob/e1bb1a70/plugins/bean-validation/src/test/resources/bean-validation-test.xml ---------------------------------------------------------------------- diff --git a/plugins/bean-validation/src/test/resources/bean-validation-test.xml b/plugins/bean-validation/src/test/resources/bean-validation-test.xml index cb44a74..fe649ab 100644 --- a/plugins/bean-validation/src/test/resources/bean-validation-test.xml +++ b/plugins/bean-validation/src/test/resources/bean-validation-test.xml @@ -26,6 +26,11 @@ <interceptor-ref name="beanValidation"/> <result type="void"/> </action> + <action name="modelDrivenActionSkipValidationByInterface" class="org.apache.struts.beanvalidation.actions.ModelDrivenAction" + method="skipMeByInterface"> + <interceptor-ref name="beanValidation"/> + <result type="void"/> + </action> <action name="fieldAction" class="org.apache.struts.beanvalidation.actions.FieldAction"> <interceptor-ref name="beanValidation"/> <result type="void"/>
