Struts 2
  1. Struts 2
  2. WW-4144

Have ObjectFactory buildResult obey ParameterNameAware restrictions for a Result

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.15.1
    • Fix Version/s: 2.3.16
    • Component/s: Core Actions
    • Labels:

      Description

      com.opensymphony.xwork2.ObjectFactory#buildResult(ResultConfig, Map), injects all of the resultConfig parameters into the result after it has been built.

      However, I'd like to be able to have my Result implement ParameterNameAware, and then have buildResult obey its acceptableParameterName() result so I can filter out what parameters can be injected.

      I'm sorry I don't have a proper patch, but it is a very small change. Only call setProperty on the result if it is not ParameterNameAware, or it is and the parameter name is acceptable.

       
      if ((!(result instanceof ParameterNameAware)) || (((ParameterNameAware) result).acceptableParameterName(paramEntry.getKey()))) {
          reflectionProvider.setProperty(paramEntry.getKey(), paramEntry.getValue(), result, extraContext, true);
      }
      

      I have been running a Struts 2 app for 6 years with this change in place. Just getting around to suggesting it as a patch

      It does also look like the documentation for ParameterNameAware would need to be updated to reflect that it can also be used for results, not just actions.

      1. WW-4144.patch
        13 kB
        Lukasz Lenart

        Activity

        Hide
        Lukasz Lenart added a comment - - edited

        Do you use exactly the same ParameterNameAware interface as for actions? What do you do if Result doesn't implement ParameterNameAware? And how do you use that? Wrap each predefined Result with your custom implementation?

        Show
        Lukasz Lenart added a comment - - edited Do you use exactly the same ParameterNameAware interface as for actions? What do you do if Result doesn't implement ParameterNameAware ? And how do you use that? Wrap each predefined Result with your custom implementation?
        Hide
        Jasper Rosenberg added a comment -

        Hi Lukasz!

        > Do you use exactly the same ParameterNameAware interface as for actions?

        As you can see in the code change I'm proposing above, yes, I think the same interface would work well, though the documentation would have to be updated to reflect this change.

        > What do you do if Result doesn't implement ParameterNameAware?
        Again, looking at the code above, it behaves exactly like the ParameterInterceptor in that if the Result doesn't implement ParameterNameAware, it just does the injection always (as it used to do).

        > And how do you use that? Wrap each predefined Result with your custom implementation?
        Yes, this came up because I needed to extend ServletActionRedirectResult to take a couple additional parameters (including "anchor" which I see is now supported since my last struts upgrade, sweet!).

        Show
        Jasper Rosenberg added a comment - Hi Lukasz! > Do you use exactly the same ParameterNameAware interface as for actions? As you can see in the code change I'm proposing above, yes, I think the same interface would work well, though the documentation would have to be updated to reflect this change. > What do you do if Result doesn't implement ParameterNameAware? Again, looking at the code above, it behaves exactly like the ParameterInterceptor in that if the Result doesn't implement ParameterNameAware, it just does the injection always (as it used to do). > And how do you use that? Wrap each predefined Result with your custom implementation? Yes, this came up because I needed to extend ServletActionRedirectResult to take a couple additional parameters (including "anchor" which I see is now supported since my last struts upgrade, sweet!).
        Hide
        Lukasz Lenart added a comment -

        I like the idea, but I don't like to use action related stuff to solve that. It'd be better to use something already built-in, without need to override Result - just define method/params and you are done.

        Show
        Lukasz Lenart added a comment - I like the idea, but I don't like to use action related stuff to solve that. It'd be better to use something already built-in, without need to override Result - just define method/params and you are done.
        Hide
        Lukasz Lenart added a comment -

        First attempt to restrict which parameters to set on Result. Also extended a bit ObjectFactory to use dedicated builder to build results - so you can define your own builder without messing with the whole ObjectFactory

        Show
        Lukasz Lenart added a comment - First attempt to restrict which parameters to set on Result. Also extended a bit ObjectFactory to use dedicated builder to build results - so you can define your own builder without messing with the whole ObjectFactory
        Hide
        Lukasz Lenart added a comment -

        I'd like change packages, ie. org.apache.struts.factory or something

        Show
        Lukasz Lenart added a comment - I'd like change packages, ie. org.apache.struts.factory or something
        Hide
        Jasper Rosenberg added a comment -

        That looks like a great approach, it definitely was a kludge to have to overwrite the whole buildResult() method before.

        I don't have a specific use case in mind, but I wonder if it would make sense to extend this concept to Interceptor parameter injection as well while you are thinking about it ObjectFactory.buildInterceptor().

        Show
        Jasper Rosenberg added a comment - That looks like a great approach, it definitely was a kludge to have to overwrite the whole buildResult() method before. I don't have a specific use case in mind, but I wonder if it would make sense to extend this concept to Interceptor parameter injection as well while you are thinking about it ObjectFactory.buildInterceptor().
        Hide
        Lukasz Lenart added a comment -

        Jasper Rosenberg ... it's just a small step, but ... If that will be ok, I will refactor the rest

        Show
        Lukasz Lenart added a comment - Jasper Rosenberg ... it's just a small step, but ... If that will be ok, I will refactor the rest
        Hide
        Jasper Rosenberg added a comment -

        Sounds great, thanks!

        Show
        Jasper Rosenberg added a comment - Sounds great, thanks!
        Show
        Lukasz Lenart added a comment - First draft of docs, feel free to comment https://cwiki.apache.org/confluence/display/WW/ObjectFactory#ObjectFactory-Definededicatedbuilder https://cwiki.apache.org/confluence/display/WW/Result+Types#ResultTypes-Extending
        Hide
        ASF subversion and git services added a comment -

        Commit 1506865 from Lukasz Lenart in branch 'struts2/trunk'
        [ https://svn.apache.org/r1506865 ]

        WW-4144 Defines new extension point to build Results and adjusts ObjectFactory to use it

        Show
        ASF subversion and git services added a comment - Commit 1506865 from Lukasz Lenart in branch 'struts2/trunk' [ https://svn.apache.org/r1506865 ] WW-4144 Defines new extension point to build Results and adjusts ObjectFactory to use it
        Hide
        ASF subversion and git services added a comment -

        Commit 1506872 from Lukasz Lenart in branch 'struts2/trunk'
        [ https://svn.apache.org/r1506872 ]

        WW-4144 Defines ParamNameAwareResult interfaces and ResultBuilder to be used with

        Show
        ASF subversion and git services added a comment - Commit 1506872 from Lukasz Lenart in branch 'struts2/trunk' [ https://svn.apache.org/r1506872 ] WW-4144 Defines ParamNameAwareResult interfaces and ResultBuilder to be used with
        Hide
        ASF subversion and git services added a comment -

        Commit 1507232 from Lukasz Lenart in branch 'struts2/trunk'
        [ https://svn.apache.org/r1507232 ]

        WW-4144 Renames ResultBuilder to ResultFactory to keep convention

        Show
        ASF subversion and git services added a comment - Commit 1507232 from Lukasz Lenart in branch 'struts2/trunk' [ https://svn.apache.org/r1507232 ] WW-4144 Renames ResultBuilder to ResultFactory to keep convention
        Hide
        Lukasz Lenart added a comment -

        Dedicated interface implemented, docs updated.

        Show
        Lukasz Lenart added a comment - Dedicated interface implemented, docs updated.
        Hide
        Jasper Rosenberg added a comment -

        Lukasz Lenart Sorry, I didn't get a chance to look at the docs last week, I was out of town. Looks good though!

        Show
        Jasper Rosenberg added a comment - Lukasz Lenart Sorry, I didn't get a chance to look at the docs last week, I was out of town. Looks good though!

          People

          • Assignee:
            Lukasz Lenart
            Reporter:
            Jasper Rosenberg
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development