Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4744

AnnotationWorkflowInterceptor should supports non-public annotated methods

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5.12
    • Component/s: Core Interceptors
    • Labels:
      None

      Description

      @Before
      protected String prepare(){
          //TODO
          return null;
      }
      

      https://github.com/apache/struts/blob/master/core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java#L115

      List<Method> methods = new ArrayList<>(AnnotationUtils.getAnnotatedMethods(action.getClass(), Before.class));
      

      https://github.com/apache/struts/blob/master/core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java#L123

      for (Method m : clazz.getMethods()) 
      

      clazz.getMethods() only return public methods, so method "prepare" will be excluded, and protected modifier is a good practice for intercept method.We should improve AnnotationUtils.getAnnotatedMethods() to return all methods. Perhaps use an ConcurrentHashMap as cache is much better.

        Issue Links

          Activity

          Hide
          yasser.zamani Yasser Zamani added a comment -

          Thanks! why does it should? here, I myself prefer to respect user defined access modifiers instead of make an off-topic complicated processing which AnnotationWorkflowInterceptor was not made for that.
          Anyway we may update docs to warn user about accessibility of annotated methods.
          wdyt?

          Show
          yasser.zamani Yasser Zamani added a comment - Thanks! why does it should? here, I myself prefer to respect user defined access modifiers instead of make an off-topic complicated processing which AnnotationWorkflowInterceptor was not made for that. Anyway we may update docs to warn user about accessibility of annotated methods. wdyt?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I'm not sure if this is a good solution, why not just make this method public? Also why to use ConcurrentHashMap as cache?

          Show
          lukaszlenart Lukasz Lenart added a comment - I'm not sure if this is a good solution, why not just make this method public? Also why to use ConcurrentHashMap as cache ?
          Hide
          quaff zhouyanming added a comment -

          Make it non-public to prevent it called by others but only called by interceptor, Most of spring annotation supports non-public methods such as lifecycle annotations like @PostConstruct @PreDestroy. Cache is not required, but I have tested it can improve performance, because search annotation on every method and every class hierarchy is a little heavy.

          Show
          quaff zhouyanming added a comment - Make it non-public to prevent it called by others but only called by interceptor, Most of spring annotation supports non-public methods such as lifecycle annotations like @PostConstruct @PreDestroy. Cache is not required, but I have tested it can improve performance, because search annotation on every method and every class hierarchy is a little heavy.
          Hide
          yasser.zamani Yasser Zamani added a comment -

          I am working on this

          Show
          yasser.zamani Yasser Zamani added a comment - I am working on this
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user yasserzamani opened a pull request:

          https://github.com/apache/struts/pull/124

          WW-4744: AnnotationUtils supports non-public methods

          With these changes, AnnotationUtils also has equipped with an internal cache. Mainly, getAnnotatedMethods and all it's usages have improved and equipped with enough unit tests (also all findAnnotation usages have equipped).

          NOTES:
          To avoid reinventing the wheel, I copied from Spring framework, So, I copied their license and authors as well where needed

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/yasserzamani/struts WW-4744

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/struts/pull/124.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #124


          commit e1bb1a70c089b80b3924391da9d0268b5bcc29f3
          Author: Yasser Zamani <yasser.zamani@live.com>
          Date: 2017-03-19T15:32:45Z

          WW-4744: AnnotationUtils supports non-public methods


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user yasserzamani opened a pull request: https://github.com/apache/struts/pull/124 WW-4744 : AnnotationUtils supports non-public methods With these changes, AnnotationUtils also has equipped with an internal cache. Mainly, getAnnotatedMethods and all it's usages have improved and equipped with enough unit tests (also all findAnnotation usages have equipped). NOTES: To avoid reinventing the wheel, I copied from Spring framework, So, I copied their license and authors as well where needed You can merge this pull request into a Git repository by running: $ git pull https://github.com/yasserzamani/struts WW-4744 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/124.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #124 commit e1bb1a70c089b80b3924391da9d0268b5bcc29f3 Author: Yasser Zamani <yasser.zamani@live.com> Date: 2017-03-19T15:32:45Z WW-4744 : AnnotationUtils supports non-public methods
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/124#discussion_r107079345

          — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java —
          @@ -37,13 +40,25 @@

          • @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
              • End diff –

          I'm not sure if adding those authors here makes any sense, in most cases we (at ASF) trying to remove the `@author` tag also asking those users about this class can confuse them.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/124#discussion_r107079345 — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java — @@ -37,13 +40,25 @@ @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 End diff – I'm not sure if adding those authors here makes any sense, in most cases we (at ASF) trying to remove the `@author` tag also asking those users about this class can confuse them.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on a diff in the pull request:

          https://github.com/apache/struts/pull/124#discussion_r107079461

          — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java —
          @@ -150,25 +166,32 @@ public static void addAllInterfaces(Class clazz, List<Class> allInterfaces) {

          • @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

              • End diff –

          no need to LOG this exception?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/124#discussion_r107079461 — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java — @@ -150,25 +166,32 @@ public static void addAllInterfaces(Class clazz, List<Class> allInterfaces) { @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 End diff – no need to LOG this exception?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          Looks good, just have some doubts about putting `@author` tag

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 Looks good, just have some doubts about putting `@author` tag
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cnenning commented on the issue:

          https://github.com/apache/struts/pull/124

          > Should I ask legal [1] about that? What do you think?

          That's the class that has been copied from spring. Yes, I think it's better to ask how to handle this.

          Show
          githubbot ASF GitHub Bot added a comment - Github user cnenning commented on the issue: https://github.com/apache/struts/pull/124 > Should I ask legal [1] about that? What do you think? That's the class that has been copied from spring. Yes, I think it's better to ask how to handle this.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          > That's the class that has been copied from spring

          I meant, coping classes is ok if the license allows that (Spring uses AL 2.0) but my questions was about adding authors to some other class (not copied from Spring), maybe let's allow Yasser explain why he did that.

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 > That's the class that has been copied from spring I meant, coping classes is ok if the license allows that (Spring uses AL 2.0) but my questions was about adding authors to some other class (not copied from Spring), maybe let's allow Yasser explain why he did that.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aleksandr-m commented on the issue:

          https://github.com/apache/struts/pull/124

          Do we need custom annotation utility at all? Can't we just use existing libraries e.g. Apache Commons [MethodUtils](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/reflect/MethodUtils.html)?

          Show
          githubbot ASF GitHub Bot added a comment - Github user aleksandr-m commented on the issue: https://github.com/apache/struts/pull/124 Do we need custom annotation utility at all? Can't we just use existing libraries e.g. Apache Commons [MethodUtils] ( https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/reflect/MethodUtils.html)?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          > Yasser explain why he did that

          Thank you for your review.

          `AnnotationUtils` is not a new class but to avoid reinventing the wheel, I copied and merged some codes from Spring into that. While Spring has `Copyright 2002-2014 the original author or authors.` in his License header, I was not sure what to do with `@author`s

          Currently, I am searching Internet for same to see what people do. Finally, I also may ask Spring.

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 > Yasser explain why he did that Thank you for your review. `AnnotationUtils` is not a new class but to avoid reinventing the wheel, I copied and merged some codes from Spring into that. While Spring has `Copyright 2002-2014 the original author or authors.` in his License header, I was not sure what to do with `@author`s Currently, I am searching Internet for same to see what people do. Finally, I also may ask Spring.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          We must rather ask ASF legal body, you can post an issue in JIRA (see above)

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 We must rather ask ASF legal body, you can post an issue in JIRA (see above)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          > We must rather ask ASF legal body, you can post an issue in JIRA (see above)

          Thank you. As I could not find similar issue, I created [Merge others codes having same license but different copyrights](https://issues.apache.org/jira/browse/LEGAL-295).

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 > We must rather ask ASF legal body, you can post an issue in JIRA (see above) Thank you. As I could not find similar issue, I created [Merge others codes having same license but different copyrights] ( https://issues.apache.org/jira/browse/LEGAL-295 ).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on a diff in the pull request:

          https://github.com/apache/struts/pull/124#discussion_r107110525

          — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java —
          @@ -150,25 +166,32 @@ public static void addAllInterfaces(Class clazz, List<Class> allInterfaces) {

          • @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

              • End diff –

          No, it is common to not found the method in one of supercleasses during our hierarchically search.

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on a diff in the pull request: https://github.com/apache/struts/pull/124#discussion_r107110525 — Diff: core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java — @@ -150,25 +166,32 @@ public static void addAllInterfaces(Class clazz, List<Class> allInterfaces) { @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 End diff – No, it is common to not found the method in one of supercleasses during our hierarchically search.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          I also wait for ASF LEGAL part's answer but after some study, it seems I was not allowed to do such work 😞 If so, I think I have to write my own utils at where @aleksandr-m mentioned; re-inventing the wheel

          Below are references; what do you think, please?

          References:
          [1] https://www.apache.org/licenses/LICENSE-2.0(https://www.apache.org/licenses/LICENSE-2.0)

          > 4. Redistribution.
          > c. You must retain, in the Source form of any Derivative Works that You distribute, all copyright, patent, trademark, and attribution notices from the Source form of the Work, excluding those notices that do not pertain to any part of the Derivative Works; and

          [2] http://www.apache.org/legal/src-headers.html(http://www.apache.org/legal/src-headers.html)

          > TREATMENT OF THIRD-PARTY WORKS
          > 1. Do not modify or remove any copyright notices or licenses within third-party works.

          [3] http://www.apache.org/dev/apply-license.html(http://www.apache.org/dev/apply-license.html)

          > CAN/SHOULD INDIVIDUAL COMMITTERS ADDED COPYRIGHT STATEMENTS TO THE NOTICE OR SOURCE CODE FILES?
          >
          > No.
          >
          > Though committers retain copyright, Apache asks that they do not add copyright statements. See the policy for more details.

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 I also wait for ASF LEGAL part's answer but after some study, it seems I was not allowed to do such work 😞 If so, I think I have to write my own utils at where @aleksandr-m mentioned; re-inventing the wheel Below are references; what do you think, please? References: [1] https://www.apache.org/licenses/LICENSE-2.0 ( https://www.apache.org/licenses/LICENSE-2.0 ) > 4. Redistribution. > c. You must retain, in the Source form of any Derivative Works that You distribute, all copyright, patent, trademark, and attribution notices from the Source form of the Work, excluding those notices that do not pertain to any part of the Derivative Works; and [2] http://www.apache.org/legal/src-headers.html ( http://www.apache.org/legal/src-headers.html ) > TREATMENT OF THIRD-PARTY WORKS > 1. Do not modify or remove any copyright notices or licenses within third-party works. [3] http://www.apache.org/dev/apply-license.html ( http://www.apache.org/dev/apply-license.html ) > CAN/SHOULD INDIVIDUAL COMMITTERS ADDED COPYRIGHT STATEMENTS TO THE NOTICE OR SOURCE CODE FILES? > > No. > > Though committers retain copyright, Apache asks that they do not add copyright statements. See the policy for more details.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          > Do we need custom annotation utility at all? Can't we just use existing libraries e.g. Apache Commons MethodUtils?

          Thank you @aleksandr-m !

          I am working on it. I started by creating LANG-1317(https://issues.apache.org/jira/browse/LANG-1317)

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 > Do we need custom annotation utility at all? Can't we just use existing libraries e.g. Apache Commons MethodUtils? Thank you @aleksandr-m ! I am working on it. I started by creating LANG-1317 ( https://issues.apache.org/jira/browse/LANG-1317 )
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          ✅ All copied Spring's code including what I was imported in previously merged PR#117(https://github.com/apache/struts/pull/117) removed 👌

          After merge of [LANG-1317 PR](https://github.com/apache/commons-lang/pull/261) and S2's upgrade to use commons lang 3.6, I'll remove deprecated `getAnnotatedMethods` and `findAnnotation` to Apache Commons Lang 3.6 👌

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 ✅ All copied Spring's code including what I was imported in previously merged PR#117 ( https://github.com/apache/struts/pull/117 ) removed 👌 After merge of [LANG-1317 PR] ( https://github.com/apache/commons-lang/pull/261 ) and S2's upgrade to use commons lang 3.6, I'll remove deprecated `getAnnotatedMethods` and `findAnnotation` to Apache Commons Lang 3.6 👌
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          So as I understand this is ready to be merged with a future notice to use Commons Lang 3.6 when available?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 So as I understand this is ready to be merged with a future notice to use Commons Lang 3.6 when available?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

          https://github.com/apache/struts/pull/124

          @lukaszlenart , Yes in my opinion, Thank you!

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/124 @lukaszlenart , Yes in my opinion, Thank you!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          Cool, I will review this tomorrow just to double check and LGTM! Great work!

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 Cool, I will review this tomorrow just to double check and LGTM! Great work!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/124

          Great work, LGTM! 👍

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/124 Great work, LGTM! 👍
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit e1bb1a70c089b80b3924391da9d0268b5bcc29f3 in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=e1bb1a7 ]

          WW-4744: AnnotationUtils supports non-public methods

          Show
          jira-bot ASF subversion and git services added a comment - Commit e1bb1a70c089b80b3924391da9d0268b5bcc29f3 in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=e1bb1a7 ] WW-4744 : AnnotationUtils supports non-public methods
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 2d5f0aaa667dca310007f224efbbb622bad5a472 in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=2d5f0aa ]

          WW-4744: Remove copied Spring's code (also from PR#117)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 2d5f0aaa667dca310007f224efbbb622bad5a472 in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=2d5f0aa ] WW-4744 : Remove copied Spring's code (also from PR#117)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 67069a97243489b338b88786ec1e0ace03a14bba in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=67069a9 ]

          WW-4744: Add @Deprecated tags

          Show
          jira-bot ASF subversion and git services added a comment - Commit 67069a97243489b338b88786ec1e0ace03a14bba in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=67069a9 ] WW-4744 : Add @Deprecated tags
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f0a9226ffbed12477c346ce70cab40804398f4a7 in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=f0a9226 ]

          WW-4744: Correct copyright to exactly as previous with no change

          Show
          jira-bot ASF subversion and git services added a comment - Commit f0a9226ffbed12477c346ce70cab40804398f4a7 in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=f0a9226 ] WW-4744 : Correct copyright to exactly as previous with no change
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3cee495a1214bde01a7b0f061040ff60ece105ab in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=3cee495 ]

          WW-4744: Add new line at end of DummyInterface file

          Show
          jira-bot ASF subversion and git services added a comment - Commit 3cee495a1214bde01a7b0f061040ff60ece105ab in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=3cee495 ] WW-4744 : Add new line at end of DummyInterface file
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/struts/pull/124

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/124
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ed20ccd3f33143d675f9c1fbb07cfa750c7de357 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=ed20ccd ]

          WW-4744 Solves problem with supporting non public annotated methods in AnnotationWorkflowInterceptor

          Show
          jira-bot ASF subversion and git services added a comment - Commit ed20ccd3f33143d675f9c1fbb07cfa750c7de357 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=ed20ccd ] WW-4744 Solves problem with supporting non public annotated methods in AnnotationWorkflowInterceptor
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Thanks Yasser Zamani, PR got merged!

          Show
          lukaszlenart Lukasz Lenart added a comment - Thanks Yasser Zamani , PR got merged!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Struts-JDK7-master #613 (See https://builds.apache.org/job/Struts-JDK7-master/613/)
          WW-4744: AnnotationUtils supports non-public methods (yasser.zamani: rev e1bb1a70c089b80b3924391da9d0268b5bcc29f3)

          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java
          • (edit) plugins/bean-validation/src/test/resources/bean-validation-test.xml
          • (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java
          • (edit) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java
          • (add) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java
          • (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java
          • (add) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java
          • (edit) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java
          • (add) core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java
          • (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java
          • (edit) core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java
            WW-4744: Remove copied Spring's code (also from PR#117) (yasser.zamani: rev 2d5f0aaa667dca310007f224efbbb622bad5a472)
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java
          • (delete) core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java
          • (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java
          • (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java
            WW-4744: Add @Deprecated tags (yasser.zamani: rev 67069a97243489b338b88786ec1e0ace03a14bba)
          • (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java
            WW-4744: Correct copyright to exactly as previous with no change (yasser.zamani: rev f0a9226ffbed12477c346ce70cab40804398f4a7)
          • (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java
            WW-4744: Add new line at end of DummyInterface file (yasser.zamani: rev 3cee495a1214bde01a7b0f061040ff60ece105ab)
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Struts-JDK7-master #613 (See https://builds.apache.org/job/Struts-JDK7-master/613/ ) WW-4744 : AnnotationUtils supports non-public methods (yasser.zamani: rev e1bb1a70c089b80b3924391da9d0268b5bcc29f3) (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java (edit) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/BaseAnnotatedAction.java (edit) plugins/bean-validation/src/test/resources/bean-validation-test.xml (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java (edit) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/BeanValidationInterceptorTest.java (add) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenActionInterface.java (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java (add) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/InterfaceAnnotatedAction.java (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java (edit) core/src/test/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptorTest.java (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java (edit) plugins/bean-validation/src/test/java/org/apache/struts/beanvalidation/actions/ModelDrivenAction.java (add) core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java (edit) core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java WW-4744 : Remove copied Spring's code (also from PR#117) (yasser.zamani: rev 2d5f0aaa667dca310007f224efbbb622bad5a472) (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java (delete) core/src/main/java/com/opensymphony/xwork2/util/ReflectionUtils.java (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java WW-4744 : Add @Deprecated tags (yasser.zamani: rev 67069a97243489b338b88786ec1e0ace03a14bba) (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java WW-4744 : Correct copyright to exactly as previous with no change (yasser.zamani: rev f0a9226ffbed12477c346ce70cab40804398f4a7) (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java WW-4744 : Add new line at end of DummyInterface file (yasser.zamani: rev 3cee495a1214bde01a7b0f061040ff60ece105ab) (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user yasserzamani opened a pull request:

          https://github.com/apache/struts/pull/144

          WW-4744 WW-4694 Removes annotation search to commons lang 3.6

          During resolution of WW-4744 and WW-4694, I duplicated some works with annotations into Apache commons lang. Now, they just released them 😃 So this PR removes them from S2 which make it more light and decreases it's tests execution time 👌

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/yasserzamani/struts WW-4744

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/struts/pull/144.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #144


          commit 33e1eeb386c98beeaeff51504a2ff156098f2c01
          Author: Yasser Zamani <yasser.zamani@live.com>
          Date: 2017-06-21T08:58:45Z

          WW-4744 WW-4694 Removes annotation search to commons lang 3.6


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user yasserzamani opened a pull request: https://github.com/apache/struts/pull/144 WW-4744 WW-4694 Removes annotation search to commons lang 3.6 During resolution of WW-4744 and WW-4694 , I duplicated some works with annotations into Apache commons lang. Now, they just released them 😃 So this PR removes them from S2 which make it more light and decreases it's tests execution time 👌 You can merge this pull request into a Git repository by running: $ git pull https://github.com/yasserzamani/struts WW-4744 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/144.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #144 commit 33e1eeb386c98beeaeff51504a2ff156098f2c01 Author: Yasser Zamani <yasser.zamani@live.com> Date: 2017-06-21T08:58:45Z WW-4744 WW-4694 Removes annotation search to commons lang 3.6
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 33e1eeb386c98beeaeff51504a2ff156098f2c01 in struts's branch refs/heads/master from Yasser Zamani
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=33e1eeb ]

          WW-4744 WW-4694 Removes annotation search to commons lang 3.6

          Show
          jira-bot ASF subversion and git services added a comment - Commit 33e1eeb386c98beeaeff51504a2ff156098f2c01 in struts's branch refs/heads/master from Yasser Zamani [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=33e1eeb ] WW-4744 WW-4694 Removes annotation search to commons lang 3.6
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 8f53b6f59efe0f713e4689ef10fe6bae5e4c0cb7 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=8f53b6f ]

          WW-4744 WW-4694 Removes annotation search to commons lang 3.6

          Show
          jira-bot ASF subversion and git services added a comment - Commit 8f53b6f59efe0f713e4689ef10fe6bae5e4c0cb7 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=8f53b6f ] WW-4744 WW-4694 Removes annotation search to commons lang 3.6
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/struts/pull/144

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/144
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Struts-JDK7-master #640 (See https://builds.apache.org/job/Struts-JDK7-master/640/)
          WW-4744 WW-4694 Removes annotation search to commons lang 3.6 (yasser.zamani: rev 33e1eeb386c98beeaeff51504a2ff156098f2c01)

          • (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java
          • (edit) core/src/main/java/org/apache/struts2/components/Component.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClassExt.java
          • (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java
          • (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java
          • (delete) core/src/test/java/com/opensymphony/xwork2/util/annotation/MyAnnotationI.java
          • (delete) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java
          • (edit) pom.xml
          • (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Struts-JDK7-master #640 (See https://builds.apache.org/job/Struts-JDK7-master/640/ ) WW-4744 WW-4694 Removes annotation search to commons lang 3.6 (yasser.zamani: rev 33e1eeb386c98beeaeff51504a2ff156098f2c01) (edit) core/src/main/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptor.java (edit) core/src/main/java/org/apache/struts2/components/Component.java (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClassExt.java (edit) plugins/bean-validation/src/main/java/org/apache/struts/beanvalidation/validation/interceptor/BeanValidationInterceptor.java (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/annotations/AnnotationWorkflowInterceptor.java (edit) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyClass.java (edit) core/src/main/java/com/opensymphony/xwork2/interceptor/DefaultWorkflowInterceptor.java (edit) core/src/main/java/com/opensymphony/xwork2/util/AnnotationUtils.java (delete) core/src/test/java/com/opensymphony/xwork2/util/annotation/MyAnnotationI.java (delete) core/src/test/java/com/opensymphony/xwork2/util/annotation/DummyInterface.java (edit) pom.xml (edit) core/src/test/java/com/opensymphony/xwork2/util/AnnotationUtilsTest.java

            People

            • Assignee:
              Unassigned
              Reporter:
              quaff zhouyanming
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development