Struts 2
  1. Struts 2
  2. WW-3753

The AnnotationActionValidatorManager does not adhere to the ActionValidatorManager interface's contract

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.3.3
    • Component/s: None
    • Labels:
      None

      Description

      An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

      I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

      1. ww3753_patch
        5 kB
        Rees Byars

        Issue Links

          Activity

          Rees Byars created issue -
          Hide
          Lukasz Lenart added a comment -

          Could you prepare a patch with solution that works for you ?

          Show
          Lukasz Lenart added a comment - Could you prepare a patch with solution that works for you ?
          Hide
          Rees Byars added a comment -

          Yes, I will do it this weekend.

          Show
          Rees Byars added a comment - Yes, I will do it this weekend.
          Rees Byars made changes -
          Field Original Value New Value
          Description An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "proxy.getMethod()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the AnnotationValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from proxy.getMethod(), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

          Either the ActionValidatorManager should be changed to no longer accept a context parameter and handle the context itself in every case (which seems to not be the best solution), or the AnnotationActionValidatorManager should be changed to use the given context in every case.
          An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

          Either the ActionValidatorManager should be changed to no longer accept a context parameter and handle the context itself in every case (which seems to not be the best solution), or the AnnotationActionValidatorManager should be changed to use the given context in every case. The ValidationInterceptor will need a small change as well in this case to maintain the functionality of WW-3194.
          Rees Byars made changes -
          Link This issue is related to WW-3194 [ WW-3194 ]
          Rees Byars made changes -
          Link This issue is related to WW-2996 [ WW-2996 ]
          Rees Byars made changes -
          Description An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

          Either the ActionValidatorManager should be changed to no longer accept a context parameter and handle the context itself in every case (which seems to not be the best solution), or the AnnotationActionValidatorManager should be changed to use the given context in every case. The ValidationInterceptor will need a small change as well in this case to maintain the functionality of WW-3194.
          An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

          Either the ActionValidatorManager should be changed to no longer accept a context parameter and handle the context itself in every case (which seems to not be the best solution), or the AnnotationActionValidatorManager should be changed to use the given context in every case. The ValidationInterceptor will need a small change as well in this case to maintain the functionality of WW-3194 without reintroducing the memory leaks from WW-2996.
          Rees Byars made changes -
          Attachment ww3753_patch [ 12514188 ]
          Hide
          Rees Byars added a comment -

          Ok, so the patch consists of two parts:

          1) The buildValidatorKey() has been changed to accept the context as a parameter again. Now, instead of always using config.getName() in place of the context name (as implemented per WW-2996 to correct memory leaks for wil card actions), the config name is used in place of the context only if it contains the wild card character. In this way, the flexibility of the original design is maintained while also accounting for the memory leaks.

          2) A getValidationContext() method has been added to the ValidationInterceptor. This method as it is implemented performs the same proxy.getActionName() call to create the context as is currently used. Extracting this function into a separate method allows for a useful bit of flexibility in that the method can be overridden to provide custom contexts.

          As well, the buildValidaterKey() test was changed to pass for the standard use case, but it still needs to have an assertion added for the wild card case.

          Show
          Rees Byars added a comment - Ok, so the patch consists of two parts: 1) The buildValidatorKey() has been changed to accept the context as a parameter again. Now, instead of always using config.getName() in place of the context name (as implemented per WW-2996 to correct memory leaks for wil card actions), the config name is used in place of the context only if it contains the wild card character. In this way, the flexibility of the original design is maintained while also accounting for the memory leaks. 2) A getValidationContext() method has been added to the ValidationInterceptor. This method as it is implemented performs the same proxy.getActionName() call to create the context as is currently used. Extracting this function into a separate method allows for a useful bit of flexibility in that the method can be overridden to provide custom contexts. As well, the buildValidaterKey() test was changed to pass for the standard use case, but it still needs to have an assertion added for the wild card case.
          Rees Byars made changes -
          Description An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.

          Either the ActionValidatorManager should be changed to no longer accept a context parameter and handle the context itself in every case (which seems to not be the best solution), or the AnnotationActionValidatorManager should be changed to use the given context in every case. The ValidationInterceptor will need a small change as well in this case to maintain the functionality of WW-3194 without reintroducing the memory leaks from WW-2996.
          An ActionValidatorManager accepts a java.util.String "context" parameter for identifying the appropriate configurations. In the AnnotationActionValidatorManager's buildValidatorKey() method, however, "config.getName()" is used instead of the passed-in context. This violates the contract of the interface and tightly couples the AnnotationActionValidatorManager to the ValidationInterceptor.

          I have a situation whereby I have created my own validation interceptor for a special case that passes in a context not derived from "proxy.getActionName()" (equivalent to config.getName() except for in the case of wildcards), only to find that this context isn't used properly by the manager. I then created my own manager, changing only the buildValidatorKey() to use the given context, and it works well.
          Hide
          Rees Byars added a comment -

          Hey, I just wanted to follow-up with this, and see if I could maybe get some feedback. I'm not familiar with the turnaround on these kinds of issues on this project. Is there any likelihood that my problem and patch will get analyzed, or do I just need to move on?

          Show
          Rees Byars added a comment - Hey, I just wanted to follow-up with this, and see if I could maybe get some feedback. I'm not familiar with the turnaround on these kinds of issues on this project. Is there any likelihood that my problem and patch will get analyzed, or do I just need to move on?
          Hide
          Lukasz Lenart added a comment -

          Patch applied, thanks!

          Show
          Lukasz Lenart added a comment - Patch applied, thanks!
          Lukasz Lenart made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Assignee Lukasz Lenart [ lukaszlenart ]
          Fix Version/s 2.3.2 [ 12319199 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Struts2 #421 (See https://builds.apache.org/job/Struts2/421/)
          WW-3753 - adheres AnnotationActionValidatorManager to ActionValidatorManager interface's contract (Revision 1293791)

          Result = UNSTABLE
          lukaszlenart :
          Files :

          • /struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManager.java
          • /struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/validator/ValidationInterceptor.java
          • /struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManagerTest.java
          Show
          Hudson added a comment - Integrated in Struts2 #421 (See https://builds.apache.org/job/Struts2/421/ ) WW-3753 - adheres AnnotationActionValidatorManager to ActionValidatorManager interface's contract (Revision 1293791) Result = UNSTABLE lukaszlenart : Files : /struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManager.java /struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/validator/ValidationInterceptor.java /struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManagerTest.java
          Hide
          Hudson added a comment -

          Integrated in Struts2 #422 (See https://builds.apache.org/job/Struts2/422/)
          WW-3753 - adheres AnnotationActionValidatorManager to ActionValidatorManager interface's contract (Revision 1293795)

          Result = SUCCESS
          lukaszlenart :
          Files :

          • /struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java
          Show
          Hudson added a comment - Integrated in Struts2 #422 (See https://builds.apache.org/job/Struts2/422/ ) WW-3753 - adheres AnnotationActionValidatorManager to ActionValidatorManager interface's contract (Revision 1293795) Result = SUCCESS lukaszlenart : Files : /struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/validation/AnnotationValidationInterceptorTest.java
          Hide
          Rees Byars added a comment -

          Thanks for the quick action! And I apologize for overlooking the AnnotationValidationInterceptorTest in my patch.

          Show
          Rees Byars added a comment - Thanks for the quick action! And I apologize for overlooking the AnnotationValidationInterceptorTest in my patch.
          Lukasz Lenart made changes -
          Fix Version/s 2.3.3 [ 12320642 ]
          Fix Version/s 2.3.2 [ 12319199 ]

            People

            • Assignee:
              Lukasz Lenart
              Reporter:
              Rees Byars
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development