[jira] [Comment Edited] (SLING-3407) ResourceBundleManager API

["Alexander Klimetschek (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-3407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13908725#comment-13908725 ] 

Alexander Klimetschek edited comment on SLING-3407 at 3/12/15 1:24 AM:
-----------------------------------------------------------------------

Here is a quick patch (compiles & tests success, but not fully tested in reality yet): [^SLING-3407.patch]

There are 2 open questions:

1) getDefaultLocale() method: do we need that anymore? See SLING-2577. I don't think a provider should be responsible for the default locale, and with multiple providers it doesn't really make sense anymore. Currently it is a fixed configuration on the JcrResourceBundleProvider and used as a fallback in case a "null" locale is passed to the provider. But IMO this needs to be handled at higher levels, such as the request object, which already does that - if you pass null as locale, it will use the request.getLocale() method which has its own resolution.

getDefaultLocale() is used in the I18nFilter though, as a default fallback for request.getLocale() in some cases (in case it's not a SlingHttpServletRequest afaics). But this is after the RequestLocaleResolver returned null - and I don't think this needs another fallback. One should have the default fallback over in the RequestLocaleResolver.

Long story short: don't have getDefaultLocale() on the new RBManager (note: it's currently in the patch), and deprecate it on the RBProvider interface.

2) BUNDLE_REQ_ATTR constant: I moved it over to the new RBManager interface and deprecated it on the RBProvider. But it seems it is not really used - the I18nFilter only reads it from the request attributes, but it never writes it. So application code would have to set it itself, not sure if that is a frequent thing to do.

So it might be ok to just keep it on the RBProvider for backwards compatibility and not bring it over and promote it further on RBManager.


was (Author: alexander.klimetschek):
Here is a quick patch (compiles & tests success, but not fully tested in reality yet): [^SLING-3407.patch]

There are 2 open questions:

1) getDefaultLocale() method: do we need that anymore? I don't think a provider should be responsible for the default locale, and with multiple providers it doesn't really make sense anymore. Currently it is a fixed configuration on the JcrResourceBundleProvider and used as a fallback in case a "null" locale is passed to the provider. But IMO this needs to be handled at higher levels, such as the request object, which already does that - if you pass null as locale, it will use the request.getLocale() method which has its own resolution.

getDefaultLocale() is used in the I18nFilter though, as a default fallback for request.getLocale() in some cases (in case it's not a SlingHttpServletRequest afaics). But this is after the RequestLocaleResolver returned null - and I don't think this needs another fallback. One should have the default fallback over in the RequestLocaleResolver.

Long story short: don't have getDefaultLocale() on the new RBManager (note: it's currently in the patch), and deprecate it on the RBProvider interface.

2) BUNDLE_REQ_ATTR constant: I moved it over to the new RBManager interface and deprecated it on the RBProvider. But it seems it is not really used - the I18nFilter only reads it from the request attributes, but it never writes it. So application code would have to set it itself, not sure if that is a frequent thing to do.

So it might be ok to just keep it on the RBProvider for backwards compatibility and not bring it over and promote it further on RBManager.

> ResourceBundleManager API
> -------------------------
>
>                 Key: SLING-3407
>                 URL: https://issues.apache.org/jira/browse/SLING-3407
>             Project: Sling
>          Issue Type: New Feature
>          Components: Extensions
>            Reporter: Alexander Klimetschek
>              Labels: i18n
>         Attachments: SLING-3407.patch
>
>
> Currently there is the ResourceBundleProvider service interface, which really is an SPI, now that you can have multiple of them. The right way of accessing them is to get all service references, sort by service ranking (highest first) and then take the result from the first one that doesn't give you a null result (null resource bundle from getResourceBundle()). This is done in the I18nFilter and is quite a bit of boilerplate that client code should not have to worry about.
> Thus there should be a new API, something like a ResourceBundleManager with the 2 methods for getResourceBundle() (with basename and without).
> [1] http://svn.apache.org/repos/asf/sling/trunk/contrib/extensions/i18n/src/main/java/org/apache/sling/i18n/impl/I18NFilter.java



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)