Uploaded image for project: 'Velocity Tools'
  1. Velocity Tools
  2. VELTOOLS-58

ValidatorTool does not respect 'bundle' property for message elements when generating client-side validation code.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2, 1.3
    • Fix Version/s: 1.3
    • Component/s: VelocityStruts
    • Labels:
      None
    • Environment:
      JDK 1.4.2_09 / Tomcat 4.1.31 / Struts 1.2.7 / Commons-Validator 1.1.4

      Description

      I recently decided to split my application properties files into multiple bundles instead of one huge bundle in an effort to maintain some order. This required me to change my validation configuration to include the "bundle" property on my message elements:

      <field property="questionTypeId"
      depends="required,integer">
      <arg position="0" bundle="Clinician" key="prompt.responseOption.questionTypeId"/>
      </field>

      The ValidatorTool.getDynamicJavascript does not respect this setting and so I get messages like "The field ?en_US.responseOption.questionTypeId? is required.".

      ValidatorTool.getDynamicJavascript first gets the global message bundle and re-uses it for all of the error messages emitted by the javascript generator:

      String message = Resources.getMessage(messages, locale, va, field);

      In order to respect the <msg>'s bundle, the code should instead be this:

      String message = Resources.getMessage(app,
      request,
      messages,
      locale,
      va,
      field);

      It turns out that this doesn't fix the problem, although I'm sure that this change should be made. Perhaps someone with a better understanding of Struts/Validator/VelocityTools could shed some light on this situation.

      1. ValidatorTool.java
        28 kB
        Nathan Bubna
      2. VELTOOLS-58.patch
        0.8 kB
        Christopher Schultz

        Activity

        Hide
        chris@christopherschultz.net Christopher Schultz added a comment -

        Can anyone comment on this bug?

        Show
        chris@christopherschultz.net Christopher Schultz added a comment - Can anyone comment on this bug?
        Hide
        nbubna Nathan Bubna added a comment -

        Hmm. Here's the latest JavascriptValidatorTag.java source as a reference:

        http://svn.apache.org/viewvc/struts/struts1/trunk/taglib/src/main/java/org/apache/struts/taglib/html/JavascriptValidatorTag.java?view=markup
        and the javadoc for the Resources class
        http://struts.apache.org/1.x/struts-core/apidocs/org/apache/struts/validator/Resources.html

        i can't see the difference in Resources docs, nor did it make a difference for you, but since JavascriptValidatorTag makes the change you suggest, we probably should too.

        in general though, it looks like we need a way for you to request a specific bundle to be used at the start of the getDynamicJavascript(...) method where the MessageResources are obtained. This probably means matching versions of all the public methods but with the addition of a bundle parameter.

        Show
        nbubna Nathan Bubna added a comment - Hmm. Here's the latest JavascriptValidatorTag.java source as a reference: http://svn.apache.org/viewvc/struts/struts1/trunk/taglib/src/main/java/org/apache/struts/taglib/html/JavascriptValidatorTag.java?view=markup and the javadoc for the Resources class http://struts.apache.org/1.x/struts-core/apidocs/org/apache/struts/validator/Resources.html i can't see the difference in Resources docs, nor did it make a difference for you, but since JavascriptValidatorTag makes the change you suggest, we probably should too. in general though, it looks like we need a way for you to request a specific bundle to be used at the start of the getDynamicJavascript(...) method where the MessageResources are obtained. This probably means matching versions of all the public methods but with the addition of a bundle parameter.
        Hide
        chris@christopherschultz.net Christopher Schultz added a comment -

        It turns out that I was wrong... my propsed fix /does/ work: I just needed to have all my settings right.

        This method in ValidatorTool:

        protected String getDynamicJavascript(ValidatorResources resources,
        Locale locale,
        Form form)

        Has the following code in it:

        String message =
        Resources.getMessage(messages, locale, va, field);

        This should be changed to:

        String message =
        Resources.getMessage(app, request,
        messages, locale, va, field);

        ...and then the "bundle" attribute for argument elements in the validator config files will be respected.

        Show
        chris@christopherschultz.net Christopher Schultz added a comment - It turns out that I was wrong... my propsed fix /does/ work: I just needed to have all my settings right. This method in ValidatorTool: protected String getDynamicJavascript(ValidatorResources resources, Locale locale, Form form) Has the following code in it: String message = Resources.getMessage(messages, locale, va, field); This should be changed to: String message = Resources.getMessage(app, request, messages, locale, va, field); ...and then the "bundle" attribute for argument elements in the validator config files will be respected.
        Hide
        chris@christopherschultz.net Christopher Schultz added a comment -

        Patch to fix this issue.

        Show
        chris@christopherschultz.net Christopher Schultz added a comment - Patch to fix this issue.
        Hide
        nbubna Nathan Bubna added a comment -

        Here, try this out. I haven't tested it, but it compiles. I think it should be able to do what you want. If it works, let me know, and i'll commit it.

        Show
        nbubna Nathan Bubna added a comment - Here, try this out. I haven't tested it, but it compiles. I think it should be able to do what you want. If it works, let me know, and i'll commit it.
        Hide
        chris@christopherschultz.net Christopher Schultz added a comment -

        Nathan,

        I didn't see anything to try out, but my patch works; all of the messages that I have properly configured are now working.

        -chris

        Show
        chris@christopherschultz.net Christopher Schultz added a comment - Nathan, I didn't see anything to try out, but my patch works; all of the messages that I have properly configured are now working. -chris
        Hide
        nbubna Nathan Bubna added a comment -

        With that comment, i attached an updated version of ValidatorTool.java with the methods that take a bundle param.

        Show
        nbubna Nathan Bubna added a comment - With that comment, i attached an updated version of ValidatorTool.java with the methods that take a bundle param.
        Hide
        chris@christopherschultz.net Christopher Schultz added a comment -

        Nathan,

        Sorry, I'm not used to using JIRA, and I didn't notice the attachment.

        I'm not sure that your extensive changes are entirely necessary, since the desired bundle is read out of the validator configuration, and doesn't need to be passed-in to these javascript generating methods.

        The 'messages' passed into the Resources.getMessage method is just the "default", and that method will use the bundle declared in the validator config to choose the appropriate bundle. I think we just needed to be calling the appropriate method.

        I'm happy to test your fix, but I don't think it really adds anything to my 4-line patch.

        -chris

        Show
        chris@christopherschultz.net Christopher Schultz added a comment - Nathan, Sorry, I'm not used to using JIRA, and I didn't notice the attachment. I'm not sure that your extensive changes are entirely necessary, since the desired bundle is read out of the validator configuration, and doesn't need to be passed-in to these javascript generating methods. The 'messages' passed into the Resources.getMessage method is just the "default", and that method will use the bundle declared in the validator config to choose the appropriate bundle. I think we just needed to be calling the appropriate method. I'm happy to test your fix, but I don't think it really adds anything to my 4-line patch. -chris
        Hide
        nbubna Nathan Bubna added a comment -

        I added the bundle support both because you originally thought your fix did not work, and because JavascriptValidatorTag has functionality equivalent to it via a bundle attribute.

        Now, i'm curious why they have that... perhaps to override the bundle configured in your validator config??

        Anyway, i'd rather not add something i only half-understand, so i'll probably just commit your patch unless you or someone can vouch for the purpose of the rest.

        Show
        nbubna Nathan Bubna added a comment - I added the bundle support both because you originally thought your fix did not work, and because JavascriptValidatorTag has functionality equivalent to it via a bundle attribute. Now, i'm curious why they have that... perhaps to override the bundle configured in your validator config?? Anyway, i'd rather not add something i only half-understand, so i'll probably just commit your patch unless you or someone can vouch for the purpose of the rest.
        Hide
        niallp Niall Pemberton added a comment -

        I don't use velocity, but the way it works in the equivalent Struts <html:javascript> tag (and the <html:errors/> and <html:messages> tags) is that the bundle attribute on these tags sets the bundle for all the messages UNLESS they have been overriden by settings in the validation config.

        Also, as a "heads up" - Struts 1.3.5 which looks like its going to be voted GA has added resource support for validator variables - see the section "1.2 Variables in Resource Bundles" on the following page:
        http://wiki.apache.org/jakarta-commons/ValidatorVersion120

        Related Struts changes:
        http://svn.apache.org/viewvc?view=rev&revision=289694

        Show
        niallp Niall Pemberton added a comment - I don't use velocity, but the way it works in the equivalent Struts <html:javascript> tag (and the <html:errors/> and <html:messages> tags) is that the bundle attribute on these tags sets the bundle for all the messages UNLESS they have been overriden by settings in the validation config. Also, as a "heads up" - Struts 1.3.5 which looks like its going to be voted GA has added resource support for validator variables - see the section "1.2 Variables in Resource Bundles" on the following page: http://wiki.apache.org/jakarta-commons/ValidatorVersion120 Related Struts changes: http://svn.apache.org/viewvc?view=rev&revision=289694
        Hide
        nbubna Nathan Bubna added a comment -

        I want to make sure this is in line with the Struts tags before we release 1.3.

        Show
        nbubna Nathan Bubna added a comment - I want to make sure this is in line with the Struts tags before we release 1.3.
        Hide
        nbubna Nathan Bubna added a comment -

        Ok. At this point, i don't see much value in overloading most of the methods to take a bundle name as a parameter (as i did in the ValidatorTool.java i attached).

        So, i've committed Christopher's patch (not sure why i hadn't already done that), and i also made another change to support the new feature Niall pointed out. It basically mirrors what was done to the JavascriptValidatorTag in Struts revision 289694.

        If anyone really wants the functions to have an optional bundle param, let me know. Then i'll check it in, and you can test it for me. If this happens after 1.3 is out the door, we can just do a 1.3.1 release.

        This is the path of least resistance for me. Speak now or forever... speak later.

        Show
        nbubna Nathan Bubna added a comment - Ok. At this point, i don't see much value in overloading most of the methods to take a bundle name as a parameter (as i did in the ValidatorTool.java i attached). So, i've committed Christopher's patch (not sure why i hadn't already done that), and i also made another change to support the new feature Niall pointed out. It basically mirrors what was done to the JavascriptValidatorTag in Struts revision 289694. If anyone really wants the functions to have an optional bundle param, let me know. Then i'll check it in, and you can test it for me. If this happens after 1.3 is out the door, we can just do a 1.3.1 release. This is the path of least resistance for me. Speak now or forever... speak later.

          People

          • Assignee:
            Unassigned
            Reporter:
            chris@christopherschultz.net Christopher Schultz
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development