Issue Details (XML | Word | Printable)

Key: STR-2971
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Niall Pemberton
Reporter: Christopher Schultz
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Struts 1

one of validator.Resources.getActionMessage method does not respect message's "bundle" attribute

Created: 08/Nov/06 01:44 PM   Updated: 04/Jul/07 03:36 AM
Component/s: Core
Affects Version/s: 1.2.9
Fix Version/s: 1.3.6

Environment: Struts 1.2.9, commons-validator 1.1.4


 Description  « Hide
I have a custom validator that uses the currently-loged-in user's date format preferences to validate date input. I use the following code to return error messages when there is a problem:

    if(error)
    {
errors.add(field.getKey(),
Resources.getActionMessage(request, va, field));
return Boolean.FALSE;
    }

The variables 'field' (commons.validator.Field), 'request', (HttpServletRequest) and 'va' (commons.validator.ValidatorAction) are all passed-in as parameters to this method, similar to other validateFoo(...) methods/classes.

I recently separated my application resources (strings) into separate files based upon user role, to help manage the avalance of strings I had to deal with. I added a "bundle" attribute to my <msg> elements in my validation configuration file. This works for all of the stock validators, but not mine. I checked to see what the difference was between my code and the validators defined in struts.validator.FieldChecks, and they use this:

            errors.add(field.getKey(),
                       Resources.getActionMessage(validator, request, va, field));

Note the addition of that first parameter. I checked the code for these two different methods in the Resources class, and it appears that the latter will use the bundle attribute and get the appropriate message resource.

I'm happy to change my code to use this other method -- it's no sweat to make that change. But I was suprised to see that this particular variety of the getActionMessage method did not respect the bundle setting, and also did not document that behavior.

The "working" method uses the ValidatorAction parameter (missing in the method in question) to get the ServletContext where the MessageResources objects can be located. Since the HttpServletRequest is available in the "non-working" method, I believe this can still be achieved.

Perhaps this "non-working" method should be deprecated in favor of the second method? Or, is there some historical reason why it needs to exist, and exhibit this behavior?


 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Niall Pemberton added a comment - 09/Nov/06 01:52 AM
History of this was that the flavour that works with bundles was added when bundle support was added. Can't actually remember why I introduced a new method, except that getting the ServletContext via the Session causes a session to be created if it doesn't already exist - the ServletContext in the Validator object is retrieved from the Servlet which is available in the ActionForm when the Validator object is initialized.

I agree that the old method should have been deprecated, which I have now done

   http://svn.apache.org/viewvc?view=rev&revision=472736

Niall

Christopher Schultz added a comment - 09/Nov/06 02:29 PM
Niall,
I would highly recommend adding documentation that mentions the fact that the now-deprecated method does not support bundles specified in <msg> elements. That gives the reader some indication of why it was deprecated, and why it is more appropriate to use the alternate method.

-chris

Niall Pemberton added a comment - 09/Nov/06 06:58 PM
OK, any chance of a patch with what you would like to see?

Christopher Schultz added a comment - 16/Nov/06 01:51 PM
I don't have the struts source code, and I would just add method javadoc, so I'll just drop in this comment if you don't mind. How about something like this, added to the method javadoc:

     *
     * Note that this method does not respect any bundle information stored with the
     * field's &lt;msg&lt; elements, and so localization does not work properly. This method is
     * deprecated for this reason, and you should use
     * {@link #getActionMessage(Validator,HttpServletRequest,ValidatorAction,Field)}
     * instead.

Had I seen this comment in the Javadoc, I would have been able to solve this problem myself without playing around too much.

If anyone objects to the deprecation of this method, we could always re-write it to properly work with the bundle. In the "preferred" method, the Validator argument is used for no other reason than to get the ServletContext. Since /this/ method has access to the request, the ServletContext can be obtained from there just as easily. Maybe there's a reason to prefer the one available from the Validator object, but it seems like it's the same thing to me.

-chris

Henri Yandell added a comment - 16/Nov/06 10:32 PM
While looking at the source I noticed that there is no getActionError(Validator, HttpServletRequest, ValidatorAction, Field) method; and that the getActionError method that is there does not support bundles. Should it?

Niall Pemberton added a comment - 18/Nov/06 02:32 AM
Chris,
 I've added a warning based on your suggestion - thanks:
        http://svn.apache.org/viewvc?view=rev&revision=476419

Henri,
There is no getActionError() method at all in Struts 1.3.x - it was deprecated in Struts 1.2.x and remove in Struts 1.3.x. Support for alternatvie resource bundles was only added in Struts 1.3 and hasn't been back ported to 1.2 (which is why the Validator dependency in Struts 1.2.x was left at Validator 1.1.4)