RFR: 7159567 - inconsistent configuration of MemoryHandler

[Jim Gish]

Please review the updated changes at 
http://cr.openjdk.java.net/~jgish/Bug7159567-set-logging-MemoryHandler/

I've changed as you've requested, added some additional tests, did some 
better error handling in the case of a MemoryHandler not specifying a 
target (now throws RuntimeException with an appropriate message instead 
of attempting to load a null class and throwing NPE).  I also corrected 
the copyrights, tested with JCK, all jdk_lang tests and have submitted a 
JPRT job with core tests.

I've forwarded a CCC request (separately) and will await its approval 
and further review of this change.

Thanks,
     Jim

On 09/28/2012 05:32 PM, Mandy Chung wrote:
> On 9/28/2012 12:13 PM, Jim Gish wrote:
>> I've re-spun the change with additional usage notes in the spec to 
>> reflect the long-standing actual behavior.  Note that it doesn't 
>> change the spec per se, as it was already stated in LogManager. This 
>> change simply replicates that language with an example in each 
>> *Handler class to make it easier to find.
>>
>
> Thanks for looking into it.  This statement in LogManager does
> specify the properties for handlers:
>
>   The properties for loggers and Handlers will have names starting
>   with the dot-separated name for the handler or logger.
>
> Replicating that statement with an example is one way to do it.
> Would it be clearer if the prefix of the properties referenced
> in the bullet list is replaced from "java.util.logging" to
> some kind of prefix - something like this:
>
>  *<b>Configuration:</b>
>  * By default each<tt>ConsoleHandler</tt>  is initialized using the 
> following
>  *<tt>LogManager</tt>  configuration properties.  If properties are 
> not defined
>  * (or have invalid values) then the specified default values are used.
>  *<ul>
>  *<li>    <handler's classname>.level
>  *        specifies the default level for the<tt>Handler</tt>
>  *        (defaults to<tt>Level.INFO</tt>).
>  ...<snip>
>  *</ul>
>  *
>  * For example, the properties for {@code ConsoleHandler} would be:
>  *     java.util.logging.ConsoleHandler.level=INFO
>  * 
> java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter
>  *
>  * For a custom handler, e.g. com.foo.MyHandler, the properties would be:
>  *     com.foo.MyHandler.level=INFO
>  * com.foo.MyHandler.formatter=java.util.logging.SimpleFormatter
>
> This might add some clarity to the spec.
>
> This is a spec bug fix that I would suggest to file a CCC to
> track for compatibility.  I would also suggest running the JCK
> tests to find out if there is any regression due to this fix.
>
>
>> The webrev, as posted at 
>> http://cr.openjdk.java.net/~jgish/Bug7159567-set-logging-MemoryHandler/
>
> See my comment above w.r.t. the spec change.
>
> test/java/util/logging/MemoryHandler.java
>   L27: "via via" typo
>   L28: @run tag specifies the test name
>        So it should be @run main/othervm MemoryHandler
>
>   L43: jtreg runs the test in a different working directory
>   other than the test source.  So the test has to read
>   the system property ("test.src") to determine the location
>   of the properties file.  Typically, we will do this:
>     String src = System.getProperty("test.src", ".);
>     File   fname  = new File(src, LM_PROP_FNAME);
>
>   You don't need L44. You can reference LoggingDeadlock3.java test.
>
>   L51: this catch block to throw a RuntimeException doesn't seem
>   necessary.  If NPE is thrown, the test will fail anyway.
>
>   One suggestion to the test is to test both cases (one with
>   the specified target handler and one without).  You can
>   define a custom target handler so that the test can verify
>   if the expected one is used.  A simple handler to count
>   the number of log messages will do the work.
>
> test/java/util/logging/MemoryHandlerTest.props
>   I suggest to take out the comments and just keep the
>   properties the test needs to make it easier to tell
>   what's configured.
>   Perhaps you should also specify
>   java.util.logging.MemoryHandler.target to make sure
>   that the custom handler with no target handler specified
>   will not use j.u.l.MemoryHandler.target as the default.
>
> Mandy
>

-- 
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.gish at oracle.com