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 a9f2565c9..f9bcbd8f9 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java @@ -19,6 +19,7 @@ import com.opensymphony.xwork2.Action; 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 @@ 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); + resultName = (String) MethodUtils.invokeMethod(action, true, annotation.methodName()); } else { resultName = annotation.resultName(); } 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 dbe4315ae..38e350338 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 org.apache.commons.lang3.reflect.MethodUtils; import java.lang.reflect.Method; import java.util.ArrayList; @@ -117,12 +118,12 @@ public class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // methods are only sorted by priority Collections.sort(methods, new Comparator() { 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 class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // methods are only sorted by priority Collections.sort(methods, new Comparator() { 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 class AnnotationWorkflowInterceptor extends AbstractInterceptor implement // methods are only sorted by priority Collections.sort(methods, new Comparator() { 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); } 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 36166ddb2..e0af7a5d4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java +++ b/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 class AnnotationUtils { * @param annotation the {@link Annotation}s to find * @return A {@link Collection}<{@link AnnotatedElement}> containing all of the * method {@link AnnotatedElement}s matching the specified {@link Annotation}s + * @deprecated Will be removed after release of LANG-1317 */ + @Deprecated public static Collection getAnnotatedMethods(Class clazz, Class... annotation) { - Collection toReturn = new HashSet<>(); - - for (Method m : clazz.getMethods()) { - boolean found = false; - for (Class c : annotation) { - if (null != findAnnotation(m, c)) { - found = true; - break; + List> allSuperclasses = ClassUtils.getAllSuperclasses(clazz); + allSuperclasses.add(0, clazz); + int sci = 0; + List> allInterfaces = ClassUtils.getAllInterfaces(clazz); + int ifi = 0; + final List annotatedMethods = new ArrayList<>(); + 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 (sci <= ifi) { + acls = allSuperclasses.get(sci++); + } + else { + acls = allInterfaces.get(ifi++); + } + final Method[] allMethods = acls.getDeclaredMethods(); + for (final Method method : allMethods) { + if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(method.getAnnotations())) { + annotatedMethods.add(method); + continue; + } + for (Class c : annotation) { + if (method.getAnnotation(c) != null) { + annotatedMethods.add(method); + } } } - if (found) { - toReturn.add(m); - } else if (ArrayUtils.isEmpty(annotation) && ArrayUtils.isNotEmpty(m.getAnnotations())) { - toReturn.add(m); - } } - return toReturn; + return annotatedMethods; } /** - * 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. - *

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 + *

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.

+ * @param + * 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 LANG-1317 */ - public static A findAnnotation(Method method, Class 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; - } - 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()); - } - } - return result; - } + @Deprecated + public static A findAnnotation(final Method method, final Class annotationCls) { + A annotation = method.getAnnotation(annotationCls); - /** - * 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 - */ - public static T getAnnotation(AnnotatedElement annotatedElement, Class annotationType) { - try { - T ann = annotatedElement.getAnnotation(annotationType); - if (ann == null) { - for (Annotation metaAnn : annotatedElement.getAnnotations()) { - ann = metaAnn.annotationType().getAnnotation(annotationType); - if (ann != null) { + if(annotation == null) { + Class mcls = method.getDeclaringClass(); + List> allSuperclasses = ClassUtils.getAllSuperclasses(mcls); + int sci = 0; + List> 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 { + equivalentMethod = acls.getDeclaredMethod(method.getName(), method.getParameterTypes()); + } catch (NoSuchMethodException e) { + // If not found, just keep on breadth first search + } + if(equivalentMethod != null) { + annotation = equivalentMethod.getAnnotation(annotationCls); + if(annotation != null) { break; } } } - return ann; - } catch (Exception ex) { - // Assuming nested Class values not resolvable within annotation attributes... - return null; - } - } - - private static A searchOnInterfaces(Method method, Class annotationType, Class... ifcs) { - A annotation = null; - for (Class iface : ifcs) { - if (isInterfaceWithAnnotatedMethods(iface)) { - 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... - } - 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. 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 572572eb0..b9ba26c35 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 @@ -23,17 +23,12 @@ package org.apache.struts2.interceptor.validation; 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 @@ public class AnnotationValidationInterceptor extends ValidationInterceptor { if (action != null) { Method method = getActionMethod(action.getClass(), invocation.getProxy().getMethod()); - Collection 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); 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 7b39267f6..a500571da 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 { 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 ed088f16e..7fd294a75 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"; + } + } 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 000000000..2761b233c --- /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(); +} 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 8ad4f3585..1b6e0d5c7 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java @@ -3,13 +3,13 @@ package com.opensymphony.xwork2.util; import com.opensymphony.xwork2.util.annotation.Dummy2Class; import com.opensymphony.xwork2.util.annotation.DummyClass; import com.opensymphony.xwork2.util.annotation.DummyClassExt; +import com.opensymphony.xwork2.util.annotation.DummyInterface; import com.opensymphony.xwork2.util.annotation.MyAnnotation; import com.opensymphony.xwork2.util.annotation.MyAnnotation2; import com.opensymphony.xwork2.util.annotation.MyAnnotationI; import junit.framework.TestCase; -import java.lang.annotation.Retention; import java.lang.reflect.AnnotatedElement; import java.util.Collection; @@ -18,15 +18,6 @@ import java.util.Collection; */ public class AnnotationUtilsTest extends TestCase { - public void testGetAnnotationMeta() throws Exception { - assertNotNull(AnnotationUtils.getAnnotation(DummyClass.class.getMethod("methodWithAnnotation"), Retention.class)); - } - - public void testGetAnnotation() throws Exception { - assertNull(AnnotationUtils.getAnnotation(DummyClass.class.getMethod("methodWithAnnotation"), Deprecated.class)); - assertNotNull(AnnotationUtils.getAnnotation(DummyClass.class.getMethod("methodWithAnnotation"), MyAnnotation.class)); - } - public void testFindAnnotationFromSuperclass() throws Exception { assertNotNull(AnnotationUtils.findAnnotation(DummyClassExt.class.getMethod("methodWithAnnotation"), MyAnnotation.class)); } @@ -43,14 +34,16 @@ public class AnnotationUtilsTest extends TestCase { public void testGetAnnotatedMethodsIncludingSuperclassAndInterface() throws Exception { Collection ans = AnnotationUtils.getAnnotatedMethods(DummyClassExt.class, Deprecated.class, MyAnnotation.class, MyAnnotation2.class, MyAnnotationI.class); - assertEquals(3, ans.size()); + assertEquals(4, ans.size()); } @SuppressWarnings("unchecked") - public void testGetAnnotedMethodsWithoutAnnotationArgs() throws Exception { + public void testGetAnnotatedMethodsWithoutAnnotationArgs() throws Exception { Collection ans = AnnotationUtils.getAnnotatedMethods(DummyClass.class); - assertTrue(ans.size() == 1); - assertEquals(ans.iterator().next(), DummyClass.class.getMethod("methodWithAnnotation")); + assertEquals(3, ans.size()); + assertTrue(ans.contains(DummyClass.class.getMethod("methodWithAnnotation"))); + assertTrue(ans.contains(DummyClass.class.getDeclaredMethod("privateMethodWithAnnotation"))); + assertTrue(ans.contains(DummyInterface.class.getDeclaredMethod("interfaceMethodWithAnnotation"))); } @SuppressWarnings("unchecked") @@ -65,10 +58,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(3, ans.size()); } public void testFindAnnotationOnClass() { 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 da2f80e41..f6a4be11a 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() { + } } 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 6faa3e46e..4d5f908df 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 @@ -1,7 +1,6 @@ package com.opensymphony.xwork2.util.annotation; public interface DummyInterface { - - @MyAnnotationI - public void interfaceMethodWithAnnotation(); -} \ No newline at end of file + @MyAnnotationI + void interfaceMethodWithAnnotation(); +} 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 85bec790f..0abe57dbe 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(); + } } 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 3123a4b50..a20d134fb 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 @@ -39,7 +39,6 @@ import org.apache.struts2.interceptor.validation.SkipValidation; import javax.validation.ConstraintViolation; import javax.validation.Validator; import java.lang.reflect.Method; -import java.util.Collection; import java.util.Set; /** @@ -99,9 +98,7 @@ public class BeanValidationInterceptor extends MethodFilterInterceptor { LOG.debug("Validating [{}/{}] with method [{}]", invocation.getProxy().getNamespace(), invocation.getProxy().getActionName(), methodName); } - Collection 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); } 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 5db0b3616..2d04ba28b 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> 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(); 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 a51c6b5f1..2de4764a6 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 { +public class ModelDrivenAction extends ActionSupport implements ModelDriven, ModelDrivenActionInterface { @Valid private Person model = new Person(); @@ -35,4 +35,8 @@ public class ModelDrivenAction extends ActionSupport implements ModelDriven + + + +