Click
  1. Click
  2. CLK-558

Binding multi request parameter to Page class field

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: core
    • Labels:
      None

      Description

      Click can bind request parameters to Page class public fields.
      However it can not bind multi parameter to an array or list field.

      I propose adding multi parameter binding support to
      ClickServlet#processPageRequestParams()

      For example, here is the HTML which send a multi parameter.

      <input type="checkbox" name="option" value="1"/>
      <input type="checkbox" name="option" value="2"/>
      ...

      Click would bind this parameter to array or list public field
      of the Page class such as...

      public String[] option;

      or

      public List<String> option;

      1. CLK-558.patch
        3 kB
        Md. Jahid Shohel
      2. CLK-558-test.patch
        11 kB
        Finn Bock

        Activity

        Hide
        Malcolm Edgar added a comment -

        Sounds good Naoki, please note this 2.1.0 build will be freezing pretty soon. So if you want to include this please do it soon, otherwise we should schedule it for the 2.2.0 release.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Sounds good Naoki, please note this 2.1.0 build will be freezing pretty soon. So if you want to include this please do it soon, otherwise we should schedule it for the 2.2.0 release. regards Malcolm Edgar
        Hide
        Naoki Takezoe added a comment -

        Hi Malcolm,
        I don't have enough time to do it soon, so I'll do it in 2.2.0.

        Show
        Naoki Takezoe added a comment - Hi Malcolm, I don't have enough time to do it soon, so I'll do it in 2.2.0.
        Hide
        Md. Jahid Shohel added a comment -

        Added patch to handle multi request parameter for page field

        Show
        Md. Jahid Shohel added a comment - Added patch to handle multi request parameter for page field
        Hide
        Finn Bock added a comment -

        Hi Jahid,

        Attached a patch to the test suite that tests several cases of multiple parameter. Some of them fails with your patch. I don't know how many of the cases we want to support but I think we should support arrays of all types that we already supports.

        Naoki also mention List<String>, which is possible, but I don't think List<Integer> is possible, so IMO it would be better to skip collections.

        Show
        Finn Bock added a comment - Hi Jahid, Attached a patch to the test suite that tests several cases of multiple parameter. Some of them fails with your patch. I don't know how many of the cases we want to support but I think we should support arrays of all types that we already supports. Naoki also mention List<String>, which is possible, but I don't think List<Integer> is possible, so IMO it would be better to skip collections.
        Hide
        Md. Jahid Shohel added a comment -

        Hi Finn,

        Then there will be many cases (at least all number primitives) that we have to cover. But what will happen if the values are mixed types? I mean on the source page-

        <input type="checkbox" name="option" value="1.0"/>
        <input type="checkbox" name="option" value="2"/>
        <input type="checkbox" name="option" value="$3"/>

        And on the destination page, should we expect a string[] Or double[]? Since its $3, it will not work for double[]. And there are many other cases like that. Considering these cases, do you think keeping them as string will be ok? Or we should add all those checkings for all primitives? Until now we support string, number(all 6 types of number) and boolean. Let me know, I will fix that then.

        Show
        Md. Jahid Shohel added a comment - Hi Finn, Then there will be many cases (at least all number primitives) that we have to cover. But what will happen if the values are mixed types? I mean on the source page- <input type="checkbox" name="option" value="1.0"/> <input type="checkbox" name="option" value="2"/> <input type="checkbox" name="option" value="$3"/> And on the destination page, should we expect a string[] Or double[]? Since its $3, it will not work for double[]. And there are many other cases like that. Considering these cases, do you think keeping them as string will be ok? Or we should add all those checkings for all primitives? Until now we support string, number(all 6 types of number) and boolean. Let me know, I will fix that then.
        Hide
        Finn Bock added a comment -

        I don't see a problem with mixed types in the array. If the destination page defines it as "public String[] option", then all is fine. If it defines it as double[] then we should get an exception and an error page like we do already for normal "public double option":

        ognl.OgnlException: id [java.lang.NumberFormatException: For input string: "$3"]
        at ognl.ObjectPropertyAccessor.setPossibleProperty(ObjectPropertyAccessor.java:83)
        ...

        But that's just IMO. What do others think?

        Show
        Finn Bock added a comment - I don't see a problem with mixed types in the array. If the destination page defines it as "public String[] option", then all is fine. If it defines it as double[] then we should get an exception and an error page like we do already for normal "public double option": ognl.OgnlException: id [java.lang.NumberFormatException: For input string: "$3"] at ognl.ObjectPropertyAccessor.setPossibleProperty(ObjectPropertyAccessor.java:83) ... But that's just IMO. What do others think?

          People

          • Assignee:
            Naoki Takezoe
            Reporter:
            Naoki Takezoe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development