Tapestry 5
  1. Tapestry 5
  2. TAP5-2063

Add support for multivalued parameters in Link

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.3.7, 5.4
    • Component/s: tapestry-core
    • Labels:
      None

      Description


      Request.java has support for multivalued parameters but there is no way to add more than one parameter with the same name to the Link. Instead of throwing an IllegalArgumentException when the link already has a parameter with the same name Link.addParameter should add it to the URL as a multivalued parameter.

      1. TAP5-2063_1.patch
        55 kB
        Alejandro Scandroli

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          71d 15h 27m 1 Kalle Korhonen 14/Apr/13 08:08
          Howard M. Lewis Ship made changes -
          Link This issue is duplicated by TAP5-928 [ TAP5-928 ]
          Hide
          Andera A. added a comment - - edited

          Yep, I recently updated to version 5.3.7, and I noticed the same problem that Howard M. Lewis Ship describes in his comment from 04/Jun/13 19:26. And I find it a big problem.

          I have a page (actually, several already!) that takes an @ActivationRequestParameter-annotated parameter. If it is set, this parameter is used on that page to visually highlight the element represented by that parameter from a list of possible items, and to display further details of the selected element (i.e.: think of a master-detail page). I want the elements on the list to be clickable, so that if you click on a different element, the page will reload with that new parameter, effectively allowing you to "choose" which one you want to work with.

          Before 5.3.7, I used to create the links on the list by setting the parameter on the pageLink (i.e.: <a href="#" t:type="pageLink" t:page="this/page" t:parameters="prop:parameterMapForTheLink">). In that way, if any of the parameters on the map would have the same name as the one in the @ActivationRequestParameter, the link would effectively keep the one given in the map. And everything worked fine.

          Because the only way now to prevent the @ActivationRequestParameter value from appearing on the generated links is by setting the value of the field to null, I tried to do so (setting the value to null) after I had handled the parameter within onActivate(), and that way I can generate controlled links again. But this gives a problem with forms: on the same master-detail page from before I allow to edit the details of the selected element; when the @ActivationRequestParameter is set to null, the form's target URL has no parameter, so the page fails to identify which item to work with upon submitting it.

          I don't think it's a mistake to allow multi-valued @ActivationRequestParameter on the link, but the implementation needs a bit of re-thinking. I would expect that, for the explicit parameters passed to the link, the Tapestry framework should consider "ah! this is what the developer explicitly wants this link's parameter's value to be". Or put a different way, Tapestry would refrain from adding values out of the page's set of properties marked with @ActivationRequestParameter, for parameters explicitly assigned in the link's parameters' map.

          Show
          Andera A. added a comment - - edited Yep, I recently updated to version 5.3.7, and I noticed the same problem that Howard M. Lewis Ship describes in his comment from 04/Jun/13 19:26. And I find it a big problem. I have a page (actually, several already!) that takes an @ActivationRequestParameter-annotated parameter. If it is set, this parameter is used on that page to visually highlight the element represented by that parameter from a list of possible items, and to display further details of the selected element (i.e.: think of a master-detail page). I want the elements on the list to be clickable, so that if you click on a different element, the page will reload with that new parameter, effectively allowing you to "choose" which one you want to work with. Before 5.3.7, I used to create the links on the list by setting the parameter on the pageLink (i.e.: <a href="#" t:type="pageLink" t:page="this/page" t:parameters="prop:parameterMapForTheLink">). In that way, if any of the parameters on the map would have the same name as the one in the @ActivationRequestParameter, the link would effectively keep the one given in the map. And everything worked fine. Because the only way now to prevent the @ActivationRequestParameter value from appearing on the generated links is by setting the value of the field to null, I tried to do so (setting the value to null) after I had handled the parameter within onActivate(), and that way I can generate controlled links again. But this gives a problem with forms: on the same master-detail page from before I allow to edit the details of the selected element; when the @ActivationRequestParameter is set to null, the form's target URL has no parameter, so the page fails to identify which item to work with upon submitting it. I don't think it's a mistake to allow multi-valued @ActivationRequestParameter on the link, but the implementation needs a bit of re-thinking. I would expect that, for the explicit parameters passed to the link, the Tapestry framework should consider "ah! this is what the developer explicitly wants this link's parameter's value to be". Or put a different way, Tapestry would refrain from adding values out of the page's set of properties marked with @ActivationRequestParameter, for parameters explicitly assigned in the link's parameters' map.
          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #1076 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1076/)
          TAP5-2063: Repair conflict between multi-valued parameters in Links, and parameters via @ActivationRequestParameter (Revision 7f5767b774b0483206be8abc43a981bb44572c11)

          Result = SUCCESS
          hlship :
          Files :

          • tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/EventLink.java
          • tapestry-core/src/test/groovy/org/apache/tapestry5/corelib/components/PageLinkTest.groovy
          • tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/ActionLink.java
          • tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/PageLink.java
          • tapestry-core/src/main/java/org/apache/tapestry5/corelib/base/AbstractLink.java
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #1076 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1076/ ) TAP5-2063 : Repair conflict between multi-valued parameters in Links, and parameters via @ActivationRequestParameter (Revision 7f5767b774b0483206be8abc43a981bb44572c11) Result = SUCCESS hlship : Files : tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/EventLink.java tapestry-core/src/test/groovy/org/apache/tapestry5/corelib/components/PageLinkTest.groovy tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/ActionLink.java tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/PageLink.java tapestry-core/src/main/java/org/apache/tapestry5/corelib/base/AbstractLink.java
          Howard M. Lewis Ship made changes -
          Link This issue is related to TAP5-2126 [ TAP5-2126 ]
          Hide
          Howard M. Lewis Ship added a comment -

          There is a critical problem with this fix.

          I have an application that uses activation request parameters, and page links that work by setting the value of an ARP directly (it's for paginating a list, so there's an @ActivationRequestParameter on a page field, and PageLinks that set the page as a parameter.

          Because of the multi-valued support added with this bug, the final URL has two values for page: the current page, and the requested page. This makes it impossible to navigate to a different page.

          Show
          Howard M. Lewis Ship added a comment - There is a critical problem with this fix. I have an application that uses activation request parameters, and page links that work by setting the value of an ARP directly (it's for paginating a list, so there's an @ActivationRequestParameter on a page field, and PageLinks that set the page as a parameter. Because of the multi-valued support added with this bug, the final URL has two values for page: the current page, and the requested page. This makes it impossible to navigate to a different page.
          Hide
          Hudson added a comment -

          Integrated in tapestry-trunk-freestyle #1045 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1045/)
          FIXED - TAP5-2063: Add support for multivalued parameters in Link (Revision ac4dabeeae8e0db5425a08477a66a4131addc32b)

          Result = FAILURE
          kaosko :
          Files :

          • tapestry-core/src/main/java/org/apache/tapestry5/Link.java
          • tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LinkImplTest.java
          • tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LinkImpl.java
          • tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
          Show
          Hudson added a comment - Integrated in tapestry-trunk-freestyle #1045 (See https://builds.apache.org/job/tapestry-trunk-freestyle/1045/ ) FIXED - TAP5-2063 : Add support for multivalued parameters in Link (Revision ac4dabeeae8e0db5425a08477a66a4131addc32b) Result = FAILURE kaosko : Files : tapestry-core/src/main/java/org/apache/tapestry5/Link.java tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LinkImplTest.java tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LinkImpl.java tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
          Kalle Korhonen made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 5.3.7 [ 12323355 ]
          Fix Version/s 5.4 [ 12316401 ]
          Resolution Fixed [ 1 ]
          Hide
          Kalle Korhonen added a comment -

          Thanks Alejandro!

          Show
          Kalle Korhonen added a comment - Thanks Alejandro!
          Kalle Korhonen made changes -
          Assignee Kalle Korhonen [ kaosko ]
          Alejandro Scandroli made changes -
          Field Original Value New Value
          Attachment TAP5-2063_1.patch [ 12567590 ]
          Hide
          Alejandro Scandroli added a comment -

          patch attached

          Show
          Alejandro Scandroli added a comment - patch attached
          Alejandro Scandroli created issue -

            People

            • Assignee:
              Kalle Korhonen
              Reporter:
              Alejandro Scandroli
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development