You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Brad Cupit (JIRA)" <ji...@apache.org> on 2008/04/10 22:05:10 UTC

[jira] Created: (WW-2587) @SkipValidation not found on superclass method

@SkipValidation not found on superclass method
----------------------------------------------

                 Key: WW-2587
                 URL: https://issues.apache.org/struts/browse/WW-2587
             Project: Struts 2
          Issue Type: Bug
          Components: Core Interceptors
            Reporter: Brad Cupit


The SkipValidation annotation is a great addition, and it worked well for me, until I used Spring to create CGLIB proxies for my Actions. CGLIB creates a class which extends my Action class, and the SkipValidation annotation isn't found on the CGLIB-generated proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, so validation is occurring on GETs instead of just the POSTs (note: I have an Action with two entry points: a show() method and a submit() method and show() has @SkipValidation).

Workaround:
use one action per url and then only always validate

Ideally, the AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Jeromy Evans (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43609#action_43609 ] 

Jeromy Evans commented on WW-2587:
----------------------------------

I think the ideal is that the same algorithm used by the AnnovationActionValidationManager to recursively check classes and interfaces for annotations is also used to find @SkipValidation.  I wouldn't want to create another recursive search that does the same thing but slightly differently.  I've already added a utility class to the JSON plugin to do this to find @SMDMethod (based on the ValidationActionValidationManager) so maybe some refactoring is in order.  
This crosses over with xwork though.

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43615#action_43615 ] 

Brad Cupit commented on WW-2587:
--------------------------------

Sounds great to me. By having both implemented in the same place in code, we don't have to worry about two different implementations getting out of sync. Does this bug need to be filed in the xwork JIRA?

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brad Cupit updated WW-2587:
---------------------------

    Description: 
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

Workaround:
use one action per url and then only always validate

Fix:
Ideally, the AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

  was:
The SkipValidation annotation is a great addition, and it worked well for me, until I used Spring to create CGLIB proxies for my Actions. CGLIB creates a class which extends my Action class, and the SkipValidation annotation isn't found on the CGLIB-generated proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, so validation is occurring on GETs instead of just the POSTs (note: I have an Action with two entry points: a show() method and a submit() method and show() has @SkipValidation).

Workaround:
use one action per url and then only always validate

Ideally, the AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.


> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> Workaround:
> use one action per url and then only always validate
> Fix:
> Ideally, the AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Jeromy Evans (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43625#action_43625 ] 

Jeromy Evans commented on WW-2587:
----------------------------------

Yeah, I agree with you.  Do you have time to prepare a patch? 

AnnotationUtils.getAnnotatedMethods() returns an unordered collection.  The interceptor will need to iterate through every matched method, check if the name matches (using the current method) and skip the invocation if *any* matches.  Unfortunately that means you can't override a method to remove the @SkipValidation annotation.

(findAnnotatedMethods is deprecated in 2.1 but getAnnotatedMethods is equivalent)

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brad Cupit updated WW-2587:
---------------------------

    Description: 
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

potential workarounds:
a) use one action per url so validation can always be done for that Action, or
b) use excludeMethods param when configuring the validation interceptor in struts.xml

Ideal fix:
The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

  was:
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

potential workarounds:
a) use one action per url and then only always validate, or
b) use excludeMethods while configuring the validation interceptor in struts.xml

Ideal fix:
The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.


> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action per url so validation can always be done for that Action, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (WW-2587) @SkipValidation not found on superclass method

Posted by "Musachy Barroso (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Musachy Barroso resolved WW-2587.
---------------------------------

       Resolution: Fixed
    Fix Version/s: 2.1.3

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>             Fix For: 2.1.3
>
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brad Cupit updated WW-2587:
---------------------------

    Description: 
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

potential workarounds:
a) use one action per url and then only always validate, or
b) use excludeMethods while configuring the validation interceptor in struts.xml

Ideal fix:
The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

  was:
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

Workaround:
use one action per url and then only always validate

Fix:
Ideally, the AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.


> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action per url and then only always validate, or
> b) use excludeMethods while configuring the validation interceptor in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43618#action_43618 ] 

Brad Cupit commented on WW-2587:
--------------------------------

xwork has a utility method: AnnotationUtils.findAnnotatedMethods() which appears to check superclass methods for annotations. There would still be more work to determine that the 'current' action method being called overrides the superclass method found by AnnotationUtils.findAnnotatedMethods(), but it shouldn't be hard.

I do agree that the appropriate place to put the @SkipValidation support is with the rest of the annotation validation. Having said that, I'm very interested in getting this issue addressed, and if a major refactoring of annotation validation, or, if having to take the Struts 2 feature of @SkipValidation and port it to xwork poses a problem, I'd favor getting it fixed quickly. Of course, I'm not the one who has to maintain the S2 codebase, so I'm very biased.  :-)

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=44017#action_44017 ] 

Brad Cupit commented on WW-2587:
--------------------------------

I think this is going to fix the bug just fine, but I have a concern about the test.

The original bug relates to annotated super class methods that are overridden by the subclass.
The current test seems to test
a) normal (un-annotated) super class methods overridden by a subclass method, and the subclass method has the annotation
b) annotated super class methods not overridden in the subclass

Both of the above conditions are good, and should be tested. I wonder if another test should be added though, to catch methods in the superclass which are annotated and overridden in the subclass (but the subclass method is not annotated). This test is necessary to prove that @SkipValidation works the same way as the other validation annotations.

How about this, add the below method to TestActionBase:
@SkipValidation
public String skipMeOverride() {
    throw new IllegalArgumentException("i should have been overridden by a subclass");
}

Add this to TestAction:
public String skipMeOverride() {
    return "override an annotated method, but don't add any new annotations (superclass annotations should still apply)";
}

And finally, add the following test to AnnotationValidationInterceptorTest:
public void testShouldSkipOverride() throws Exception {
    mockActionProxy.expectAndReturn("getMethod", "skipMeOverride");
    interceptor.doIntercept((ActionInvocation)mockActionInvocation.proxy());
    mockActionProxy.verify();
}

Note: i don't really understand the tests 100%. It seems that if the mock ActionProxy only has getMethod() called once, and nothing else happens, then the method was skipped. If getActionName() is called on the mock ActionProxy though, then the method is not skipped. If that's a true assumption, then the above test should work and prove that the bug is fixed for good.

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>             Fix For: 2.1.3
>
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=44021#action_44021 ] 

Brad Cupit commented on WW-2587:
--------------------------------

thank you for fixing the bug (and for the compliment)  :-)

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>             Fix For: 2.1.3
>
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WW-2587) @SkipValidation not found on superclass method

Posted by "Musachy Barroso (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=44020#action_44020 ] 

Musachy Barroso commented on WW-2587:
-------------------------------------

That was an excellent catch Brad, I modified the code as it was missing that test case (when the method is not overwritten). Thanks.

> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>             Fix For: 2.1.3
>
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43615#action_43615 ] 

bradcupit edited comment on WW-2587 at 4/14/08 6:07 AM:
---------------------------------------------------------

Sounds good. By having both implemented in the same place in code, we don't have to worry about two different implementations getting out of sync. Does this bug need to be filed in the xwork JIRA?

      was (Author: bradcupit):
    Sounds great to me. By having both implemented in the same place in code, we don't have to worry about two different implementations getting out of sync. Does this bug need to be filed in the xwork JIRA?
  
> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WW-2587) @SkipValidation not found on superclass method

Posted by "Brad Cupit (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brad Cupit updated WW-2587:
---------------------------

    Description: 
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

potential workarounds:
a) use one action method per Action so validation can always be done for that entire class, or
b) use excludeMethods param when configuring the validation interceptor in struts.xml

Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml

Ideal fix:
The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

  was:
The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.

I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.

Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.

Unfortunately, the validation annotations (like @RequiredStringValidator), _are_ found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.

potential workarounds:
a) use one action per url so validation can always be done for that Action, or
b) use excludeMethods param when configuring the validation interceptor in struts.xml

Ideal fix:
The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.


> @SkipValidation not found on superclass method
> ----------------------------------------------
>
>                 Key: WW-2587
>                 URL: https://issues.apache.org/struts/browse/WW-2587
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Brad Cupit
>
> The SkipValidation annotation is a great addition, but it would be nice if it was found on super class methods (i.e. the method being overridden) and/or interfaces.
> I have a CGLIB-generated proxy for my Action (created by Spring) so the SkipValidation annotation is never found, since CGLIB-proxies extend the class they proxy.
> Admittedly, this is more of a problem with CGLIB, than with Struts2, however if @SkipValidation were allowed on methods in interfaces or superclasses, my problem would be solved.
> Unfortunately, the validation annotations (like @RequiredStringValidator), are found on super classes. So the net effect is that my SkipValidation annotation is not found, but the validation annotations are, which means validation is occurring when it shouldn't.
> potential workarounds:
> a) use one action method per Action so validation can always be done for that entire class, or
> b) use excludeMethods param when configuring the validation interceptor in struts.xml
> Note that for option b) to work with zero-configuration, the @ParentPackage annotation must be applied to the Action class that will use the package defined in struts.xml
> Ideal fix:
> The AnnotationValidationInterceptor#doIntercept(ActionInvocation) method would check for the SkipValidation annotation on the method being overridden. This would not only fix the CGLIB-proxy issue, but would also allow SkipValidation to be used on interfaces and superclasses.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.