RFR: 8033661: readConfiguration does not cleanly reinitialize the logging system

[Daniel Fuchs]

Hi Mandy,

On 13/09/15 00:44, Mandy Chung wrote:
> Have you considered keeping the same method name, readConfiguration
> with the new remapper parameter?  The downside is the difference that
> the reset is not invoked.  It might not matter because as you add in
> @apiNote that the existing readConfiguration method may be overridden
> for custom LogManager but discouraged to be used by applications.

I have reworked the API notes...

The class level documentation says:

  * <p>
  * If the "java.util.logging.config.class" property is set, then the
  * property value is treated as a class name.  The given class will be
  * loaded, an object will be instantiated, and that object's constructor
  * is responsible for reading in the initial configuration.  (That object
  * may use other system properties to control its configuration.)  The
  * alternate configuration class can use {@code 
readConfiguration(InputStream)}
  * to define properties in the LogManager.


So with that in mind, I have slightly altered the @apiNotes:

in readConfiguration():

      * @apiNote {@code readConfiguration} is principally useful for
      *    establishing the LogManager primordial configuration.
      *    <p>
      *    Calling this method directly from the application code after the
      *    LogManager has been initialized is discouraged.

           etc...

The rationale is that an application which needs to establish
a custom configuration should probably use the
{@code "java.util.logging.config.class"} property, and
call LogManager.readConfiguration(InputStream) from there,
as hinted in the class-level documentation.

in readConfiguration(InputStream):

      * @apiNote {@code readConfiguration} is principally useful for 
applications
      *    which use the {@code "java.util.logging.config.class"} property
      *    to establish their own custom configuration.
      *    <p>
      *    Calling this method directly from the application code after the
      *    LogManager has been initialized is discouraged.

Same rationale than above. After thinking it over I'm not
sure overriding readConfiguration is something we should
encourage. I would prefer some wording that does not imply
it :-)

>
> 1749      * Updates an existing configuration.
>
> We should specify what “existing configuration” is.
> If “java.util.logging.config.class” is set and it’s instantiated successfully,
> readConfiguration will not read from any configuration file.
> updateConfiguration only reads the config file regardless
> if “java.util.logging.config.class” is set.

I updated the doc for updateConfiguration(mapper):

      * Updates an existing configuration.
      * <p>
      * @implSpec
      * This is equivalent to calling:
      * <pre>
      *   try (final InputStream in = new 
FileInputStream(<logging.properties>)) {
      *       final BufferedInputStream bin = new BufferedInputStream(in);
      *       updateConfiguration(bin, mapper);
      *   }
      * </pre>
      * where {@code <logging.properties>} is the logging configuration 
file path.
      * <p>
      * Note that this method not take into account the value of the
      * {@code "java.util.logging.config.class"} property.
      * The {@code "java.util.logging.config.file"} system property is read
      * to find the logging properties file, whether the
      * {@code "java.util.logging.config.class"} property is set or not.
      * If the {@code "java.util.logging.config.file"} system property 
is not
      * defined then the default configuration is used.
      * The default configuration is typically loaded from the 
properties file
      * "{@code conf/logging.properties}" in the Java installation 
directory.
      *

in updateConfiguration(InputStream, mapper), after the table:

      * <p>
      * Note that this method has no special handling for the "config" 
property.
      * The new value provided by the mapper function will be stored in the
      * LogManager properties, but {@code updateConfiguration} will not 
parse its
      * value nor instantiate the classes it points to.


>
> 1761      * @param remapper
>
> “re” probably not needed here.   I think “mapper” should work.

OK. Should I still use the term of 'remapping function' then?
Or does that become 'mapping function' too?


Here is the new webrev - with the other comments from your previous
mail also integrated:

http://cr.openjdk.java.net/~dfuchs/webrev_8033661/webrev.01/

best regards,

-- daniel