incubator-jspwiki-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Jaquith (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JSPWIKI-351) Incorrect bundles specified in JSPs
Date Wed, 20 Aug 2008 19:04:44 GMT

    [ https://issues.apache.org/jira/browse/JSPWIKI-351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624097#action_12624097
] 

Andrew Jaquith commented on JSPWIKI-351:
----------------------------------------

Indeed, when we get to Stripes, developers (and Stripes) should not guess where the strings
are located. I filed this bug precisely because of that fact. TODAY, developers need to guess.
And tomorrow, Stripes will have to.

In theory, we distinguish between top-level JSPs (which provide business logic and field validation)
and template *Content JSPs (which present the UI). Whether something is in CoreResources versus
default.props seems to depend on whether we've shipped it something as opposed to a third
party customization. 

But this has led to some slightly weird conventions. For example, the method that validates
the input for new user profiles (in UserManager.validateProfile) uses the CoreResources bundle
for generating errors -- but the presentation markup on ProfileContent.jsp uses the "default"
bundle. Why? Is there something intrinsic about validation that makes it a "core" feature
rather than a "custom template" feature? Why look up field names in two places? 

The key problem, in my view, is that in "core" we have mixed presentation-tier messages (for
example, JSP form field validation) in with core class messages that have nothing to do with
the presentation tier. I would suggest to you that this is the wrong distinction. The key
point of separation (assuming we need one at all!) is not who (us v. someone els) WROTE the
message strings, but what TIER the messages apply to. 

For example, I've mixed together the various "login" properties from CoreResources and the
default bundle. Can you tell which belongs to which? I sure can't. 

login.tab=Login 
login.title=Login 
login.heading.login=Sign in to {0} 
login.help=Please sign in with your login name and password. 
login.errorprefix=Error:&nbsp; 
login.error.capslock=Invalid login (please check your Caps Lock key) #obsolete 
login.error.password=Not a valid login. 
login.error.noaccess=It seems you don't have access to that. Sorry. 
login.login=Login 
login.password=Password 
login.lostpw=Lost your password? 
login.lostpw.reset.login={0} to log in once you retrieve your new password. 
login.register.tab=Register New User 

Having a single set of bundles for the JSP tier is critical for Stripes. Stripes relies on
being able to locate a standard resource bundle for looking up validation error strings, field
labels and the like. In the servlet configuration, you can tell Stripes where to find these
strings by setting the "StripesResources" servlet init param. (You can actually configure
two different bundles, one for errors and one for field names, but that's a minor point.)
The point is, Stripes has no knowledge of what a JSPWiki "default template" is versus a "core
resource"; it just wants a predictable place to find message strings. 

For example, let's look at what happens in a typical Stripes ActionBean custom validation
method. 

    @ValidationMethod( on = "save", when = ValidationState.ALWAYS ) 
    public void validatePasswords( ValidationErrors errors ) 
    { 
        // If a password was supplied, the same value must also have been passed 
        // to passwordAgain 
        if( m_profile.getPassword() != null ) 
        { 
            if( !m_profile.getPassword().equals( m_passwordAgain ) ) 
            { 
                errors.add( "profile.password", new LocalizableError( "security.error.passwordnomatch"
) ); 
            } 
        } 
    } 

This method fires before the save() event handler executes, and it makes sure the two password
fields match. The ValidationErrors object contains the set of errors for this particular submitted
form. If we add a LocalizableError (constructed with a message key), it will look in the bundle
we've configured Stripes to use. You could argue, in this case, that Stripes should obviously
use CoreResources, and in this case I would agree. In case we wanted to use the template's
bundle, we could supply a "SimpleError" type instead, and look up and supply as hard-coded
text the localized string we want, from the  bundle we want. Ok, fine. 

But, of course, we won't always have as much control over which message bundle we use. Consider
a setFullname() setter method on the ActionBean with a @Validate annotation: 

@Validate( field = "fullname", maxlength = 100, required = true, on = "save", label="security.user.fullname"
) 

The "label" portion of the annotation tells Stripes to use the "security.user.fullname" key
for reporting errors. Stripes will look in the bundle we told it to use, presumably CoreResources.
But if we wanted to use something in a template bundle, we'd be hosed. 

Given the constraint that Stripes requires a consistent place for looking up message keys,
we have exactly two choices here: 
* Move ALL presentation-tier messages to default.properties and have Stripes use that as its
default bundle. We could do this in 2.8
* Consolidate the properties in CoreResources and default.props and have Stripes use that
bundle. It would be hard to do this in 2.8.

I favor the latter -- collapsing ALL i18n properties (plugins too) into a single set of files.
In addition, I think it would also make sense to externalize the properties into a directory
in WEB-INF, rather than keep them in JSPWiki.jar. If, indeed, we expect developers to customize
things, it seems burdensome to force them to rebuild the JSPWiki.jar every time they change
a message string.

> Incorrect bundles specified in JSPs
> -----------------------------------
>
>                 Key: JSPWIKI-351
>                 URL: https://issues.apache.org/jira/browse/JSPWIKI-351
>             Project: JSPWiki
>          Issue Type: Bug
>          Components: Default template
>    Affects Versions: 2.8
>         Environment: All
>            Reporter: Andrew Jaquith
>             Fix For: 2.8
>
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> i18n strings are improperly stored in CoreResources_*.properties, when they should have
been specified in templates/default_*.properties. The comments at the top of CoreResources
specifies that messages are for "JSPWiki internal code, the so-called core code." But these
JSPs all look up and use message strings from CoreResources:
> * Comment.jsp
> * Install.jsp
> * LostPassword.jsp
> * NewGroup.jsp
> * Rename.jsp
> Example: 
>         // Weepy tears and hankies all 'round.
>         if( wikiSession.isAuthenticated() )
>         {
>             response.sendError( HttpServletResponse.SC_FORBIDDEN, rb.getString("login.error.noaccess")
);
>             return;
>         }
> This is clearly a template/JSP-level error message, NOT an internal error. And similar
kinds of code are sprinked all over the other JSPs.
> I recommend we consolidate default.properties and CoreResources.properties. The easiest
way would simply be to concatenate the files. Then, in WikiContext.getBundle(), any requests
for "CoreResources" could be simply diverted to default.properties.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message