On 9/10/14 1:25 PM, Alan Bateman wrote:
On 10/09/2014 10:49, Daniel Fuchs wrote:
Hi,

Please find below a proposed patch for

8043306 - Provide a replacement for the API that allowed to listen
          for LogManager configuration changes
https://bugs.openjdk.java.net/browse/JDK-8043306

Proposed Patch:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.00/

It would be nice if ConfigurationListener could be an interface. I
realize this might mean looking again at the security concerns in this
area again to see if we could get away without requiring a construction
time permission check. Clearly if it could be an interface then it begs
the question as to why it's just not a Runnable but that probably means
yet more effort to figure out the right security and whether the access
control context should be recorded when registering.

One nice thing about ConfigurationListener being an abstract class is
that I don't need to invent an 'IdentityCopyOnWriteArrayList' which
is what I would prefer to use if equals() could be overriden by
implementations.

Also I like it that you need to have the LoggingPermission("Control")
to subclass ConfigurationListener.

In practice - I believe that there aren't that many application
which need this functionality - and I also believe that for those
that need it then there will be a single ConfigurationListener
that will be registered only once, and therefore having an
abstract class for ConfigurationListener shouldn't be that much of
a hassle for them.

Assuming ConfigurationListener remains as an abstract class then the
no-arg constructor will need a one-line javadoc comment.

Oh - right - thanks for catching that.


A small suggestion for addConfigurationListener to return LogManager to
facilitate method invocation chaining when configuring the LogManager.
It could be done for remove too but that is probably less interesting.

Good suggestion.

Another comment on addConfigurationListener is that about the wording
"re-read and the configuration is changed". I think the configuration
listeners are invoked when ever the configuration is read even if there
aren't any changes.

Right. Although I believe it would be difficult (though maybe not
impossible) to manage to register a configuration listener before
the configuration is read for the first time ;-)

The javadoc links in the readConfiguration methods make the line length
a bit inconsistent with the existing javadoc, maybe the package name
could be just dropped from the links or the link moved to the next line.

OK - I'll see what I can do.

Thanks Alan!

-- daniel


-Alan.



Reply via email to