|
Description:
|
- We configure logback through logback.xml
- Initial, pre-configuration state of logback has an "OnConsoleStatusListener" set up.
In effect, if I understand what I see in the debugger, once the "configuration" tag is encountered, the following call stack happens:
Interpreter.startElement() calls
Interpreter.callBeginAction() calls
ConfigurationAction.begin() calls
OnConsoleStatusListener.addNewInstanceToContext(context);
which creates a new OnConsoleStatusListener, starts it, then adds it to the StatusManager reachable via the context.
At the moment, the first messages appear on the Console as the "retrospective" is printed out if the messages in there are not too old.
Now:
If logback.xml contains
<statusListener class="ch.qos.logback.core.status.OnConsoleStatusListener" />
then a second OnConsoleStatusListener instance will be added to the StatusManager reachable via the context.
This leads to a confusing duplication of messages. First all the retrospective, which may have grown since the first printout, is printed. All subsequent status messages are then printed, too.
Would it not be better to check in
OnConsoleStatusListener.addNewInstanceToContext(Context context)
whether a OnConsoleStatusListener already exists in the StatusManager (in this case, the ch.qos.logback.core.BasicStatusManager) and do nothing if that is so?
This is a special handling of OnConsoleStatusListener, of course, barring some more general idea (like a flag on the listener saying "I am responsible for the console and there can be only one").
There is the nasty problem that the new OnConsoleStatusListener must not be started before we are sure it could be added, because starting makes it print out the retrospective. But start() is not in the interface of StatusListener. :-(
This thus seems to demand that BasicStatusManager.add() is specially tuned to OnConsoleStatusListener:
-------------
public void add(StatusListener listener) {
if (listener != null) {
synchronized (statusListenerListLock) {
boolean addIt = true;
// Check that there is no OnConsoleStatusListener already
// If not, start it before adding it
if (listener instanceof OnConsoleStatusListener) {
for (StatusListener cur : statusListenerList) {
if (cur instanceof OnConsoleStatusListener) {
addIt = false;
break;
}
}
if (addIt) {
((OnConsoleStatusListener) listener).start();
}
}
if (addIt) {
statusListenerList.add(listener);
}
}
}
}
--------
And that OnConsoleStatusListener leave "start()" to the "add()" method. This is not fully nice, because the hidden assumption is that the StatusManager returned by the Context is indeed a BasicStatusManager. The correct solution would be to have start() defined on StatusListener and call the add() method addAndStart(), but that would change the interface:
--------
static public void addNewInstanceToContext(Context context) {
OnConsoleStatusListener onConsoleStatusListener = new OnConsoleStatusListener();
onConsoleStatusListener.setContext(context);
// onConsoleStatusListener.start();
context.getStatusManager().add(onConsoleStatusListener);
}
--------
|
Major