RFR: 8013839: Enhance Logger API for handling of resource bundles

[Mandy Chung]

Hi Daniel,

This is a good solution for Logger subclasses or existing apps that rely 
on the previous stack walk search to lookup a resource bundle.I have 
some comments on the specification:

Logger.setResourceBundle(ResourceBundle rb)
    Should this method simply throw NPE if rb == null since we can't 
reset a non-null rb/rbname to null?

Some suggestion to the javadoc:


   * Sets the resource bundle for this logger to use for localization.
   *
   * @param bundle  The resource bundle that this logger shall use.
   * @throws  IllegalArgumentException if the given bundle doesn't have a
   *         {@linkplain ResourceBundle#getBaseBundleName base name},
   *         or if this logger already has a resource bundle set but
   *         the given bundle has a different base name.


The getLogger(String name, String rbname) spec and the class specification
should be updated about different ways in setting a resource bundle for
a logger.

For example in the class spec, this paragraph and text following it talks
about resource bundle and name and also logrb method:

     Each Logger may have a ResourceBundle name associated with it.
     The named bundle will be used for localizing logging messages.
     If a Logger does not have its own ResourceBundle name, then
     it will inherit the ResourceBundle name from its parent,
     recursively up the tree.

Adding @see #setResourceBundle in the getLogger method would be helpful.

It'd be good to clarify the spec of getResourceBundle and getResourceBundleName
methods that they return the ResourceBundle and its base name if set via
setResourceBundle method.  Perhaps it might be good for the class spec
to capture different ways to set the resource bundle and and have the methods
to refer that specification.

The current implementation of the getLogger(String name, String rbname) method
uses the caller's class loader to look up the resource bundle.  Do you mind
taking this opportunity to update the spec to match the current implementation?
This fix is a follow-up on that change.

Maybe we should just use varargs (JDK-5001993) to replace the two
logrb methods one taking a single parameter and the other taking an
array of parameters:
     public void logrb(Level level, String sourceClass, String sourceMethod,
                       ResourceBundle bundle, String msg, Object... params)

I can live with the inconsistency.

ResourceBundle.getBaseBundleName - this is more a question to Naoto.

The implementation note about this method may return a pathname
but the spec states that it returns a fully qualified class name
is a bit strange.  Can you elaborate what problem we have here?
I am missing the context to connect how this is related to the
older versions of JRE

thanks
Mandy


On 9/20/2013 5:33 AM, Daniel Fuchs wrote:
> Hi,
>
> Please find below a patch for:
>
> 8013839: Enhance Logger API for handling of resource bundles, and
> 4814565: (rb) add method to get basename from a ResourceBundle
>
> <http://cr.openjdk.java.net/~dfuchs/webrev_8013839/webrev.06/>
>
>    1. It adds a ResourceBundle.getBaseBundleName() method,
>    2. It adds a Logger.setResourceBundle(ResourceBundle) method,
>    3. It adds a series of Logger.logrb(...) that take a ResourceBundle
>         object instead of a ResourceBundle name.
>         The previous versions of Logger.logrb(...) that took a
>         resource bundle name are therefore no longer needed,
>         they offer no added value above the new methods, and
>         are thus now deprecated.
>
> Logger.getLogger(name, rbname) will continue to work as before,
>      and will also throw an exception if a RB with a different name
>      has been set before, either through setResourceBundle or
>      Logger.getLogger(name, rbname).
>
> Logger.setResourceBundle works similarly to
>      Logger.getLogger(name, rbname) in the sense that it will not
>      allow to replace an existing bundle, unless both have the same
>      name. Like for Logger.getLogger(name, rbname) - it doesn't matter
>      by which method the previous bundle has been set.
>
>      This is mostly for consistency of the API - if thread A has
>      requested a logger with resource bundle name 'foo.Bundle'
>      and thread B attempts to set the resource bundle of that
>      logger to 'bar.Bundle' then thread B will get an IAE.
>
>      The resource bundle passed to Logger.setResourceBundle *must*
>      have a base name. Note that ResourceBundle objects will have
>      a base name by default if they have been loaded through
>      one of the ResourceBundle.getBundle(...) methods;
>      If ResourceBundle.getBaseBundleName() returns null an IAE
>      will be thrown.
>
>      Logger.setResourceBundle requires the "control" permission.
>
>      When a resource bundle is set with Logger.setResourceBundle,
>      then it's that resource bundle locale that will be used
>      when logging (and not the default locale).
>
> I have written a set of unit tests which should cover all the points
> listed above.
>
> best regards,
>
> -- daniel