Commons Validator
  1. Commons Validator
  2. VALIDATOR-149

required validator doesn't work with String[] (multi-selects)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1.0 (alpha)
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      The "required" validator doesn't work for String[] i.e. using multi-select input
      types.

      I fixed it locally on my machine, though, so here is what I did if you are
      interested:

      I changed the org.apache.commons.validator.ValidatorUtil.getValueAsString()
      method from:

      public static String getValueAsString(Object bean, String property) {
      Object value = null;

      try

      { value = PropertyUtils.getProperty(bean, property); } catch (Exception e) { log.error(e.getMessage(), e); }
      return (value != null ? value.toString() : null);
      }

      to:

      public static String getValueAsString(Object bean, String property) {
      Object value = null;

      try { value = PropertyUtils.getProperty(bean, property); }

      catch (Exception e)

      { log.error(e.getMessage(), e); }

      //Special case, check if String[]
      try
      {
      String[] valueArray = (String[])value;
      if(valueArray==null || valueArray.length==0)

      { value = null; }

      }
      catch(ClassCastException cce)

      { //Then it wasn't a String[] }

      return (value != null ? value.toString() : null);
      }

      I guess, just to be safe the second catch could catch type Exception instead but
      that is up to you.

      Anyway, it fixed my validation problem for String[] (multi-selects) and doesn't
      appear to have broken and validations which worked before.

      To sum up the problem and requested enhancement, I am using Struts (but it would
      happen the same way with HTML only) to submit an input of type select and
      multiple="true" and the validator framework does not detect if nothing is
      selected in that case. I traced down to the getValueAsString() method as the
      easiest place to add a fix / change so that the least impact could be made.
      Above is the code change I made on my local machine to fix "required" type
      validation for a multi-select input.

        Activity

        shane created issue -
        Hide
        Robert Leland added a comment -

        This looks pretty good, though the code will be altered
        a little since for regular fields a class cast exception would be thrown.
        It should be possible to only try doing the cast when all else has failed.

        Also then that would mean the client side Javascript is also
        not handling String[] either, correct ?

        Generally,
        Normally you do a 'diff -u file.before.java file.after.java > file.diff'
        or do it directly from CVS. If you are using windows you
        can use the cygwin package to get all the unix type commands.
        Then that patch added as an attachment.

        Since this is a simple patch then 'inline' is ok.

        For a more involved case you would create a simple test case that
        shows the problem, that could then be used as a unit test.

        Again, what you gave looks good.

        Show
        Robert Leland added a comment - This looks pretty good, though the code will be altered a little since for regular fields a class cast exception would be thrown. It should be possible to only try doing the cast when all else has failed. Also then that would mean the client side Javascript is also not handling String[] either, correct ? Generally, Normally you do a 'diff -u file.before.java file.after.java > file.diff' or do it directly from CVS. If you are using windows you can use the cygwin package to get all the unix type commands. Then that patch added as an attachment. Since this is a simple patch then 'inline' is ok. For a more involved case you would create a simple test case that shows the problem, that could then be used as a unit test. Again, what you gave looks good.
        Hide
        David Graham added a comment -

        I just ran into this problem as well and will commit a variation on the
        suggested approach soon. I think we should check for Collection and String[]
        (let's refer to these collectively as the "list"). If the list is empty we
        return "", else we return list.toString(). This way we'll only be returning
        null when there's an error retrieving the property.

        Show
        David Graham added a comment - I just ran into this problem as well and will commit a variation on the suggested approach soon. I think we should check for Collection and String[] (let's refer to these collectively as the "list"). If the list is empty we return "", else we return list.toString(). This way we'll only be returning null when there's an error retrieving the property.
        Hide
        David Graham added a comment -

        Fixed.

        Show
        David Graham added a comment - Fixed.
        Henri Yandell made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 22121 12340887
        Henri Yandell made changes -
        Affects Version/s unspecified [ 12311647 ]
        Component/s Validator [ 12311135 ]
        Key COM-736 VALIDATOR-149
        Project Commons [ 12310458 ] Commons Validator [ 12310494 ]
        Assignee David Graham [ dgraham@apache.org ]
        Niall Pemberton made changes -
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Niall Pemberton added a comment -

        Re-openned and the set to "Resolved Fixed" again to correct "resolution" which was lost in Bugzilla --> JIRA conversion

        Show
        Niall Pemberton added a comment - Re-openned and the set to "Resolved Fixed" again to correct "resolution" which was lost in Bugzilla --> JIRA conversion
        Niall Pemberton made changes -
        Fix Version/s 1.1.0 (alpha) [ 12311786 ]
        Resolution Fixed [ 1 ]
        Status Reopened [ 4 ] Resolved [ 5 ]
        Henri Yandell made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

        Error rendering 'com.atlassian.jirafisheyeplugin:fisheye-issuepanel'. Please contact your JIRA administrators.

          People

          • Assignee:
            Unassigned
            Reporter:
            shane
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development