Vincent Siveton
added a comment - Basically, we need to improve logging in Sink, Parser and Macro.
Using Plexus AbstractLogEnabled seems the better way i.e.
public class SinkAdapter
extends AbstractLogEnabled
implements Sink
{}
The actual problem is that Sinks are not used as plexus components. Instantiation is direct, i.e.:
MySink sink = new Sink( aWriter );
due NPE when calling getLogger()
A solution could be to refactor all sinks, parser and macro to use them as components.
We could also need to specify a logger, i.e.
public class SinkAdapter
extends AbstractLogEnabled
implements Sink
{
private Logger logger;
/** {@inheritDoc} */
public final Logger getLogger()
{
if ( logger == null )
{
if ( super.getLogger() != null )
{
logger = super.getLogger();
}
else
{
logger = new Slf4jLogger( Logger.LEVEL_DEBUG, LoggerFactory.getLogger( getClass() ));
}
}
return logger;
}
public final void setLogger( Logger logger )
{
this.logger = logger;
}
}
And specify the current logger during the instantiation, i.e.
MySink sink = new Sink( aWriter );
sink.setLogger( getLogger );
After some more discussion, here is my proposition: only DefaultDoxia extends AbstractLogEnabled. Sink, parser and macro have independent instances of a logger that they inherit from a super interface (note: api changes!) so in principle they can be set independently. In practice it's the parser that should propagate its logger to the sink and macros that it's using. I have put the logging package into a separate module to avoid a circular dependency, since both sink and parser should be LogEnabled.
I have tested by adding a simple xdoc to the site-renderer test that contains an invalid element, one can see that the plexus logger injected in DefaultDoxia is used indeed.
Lukas Theussl
added a comment - After some more discussion, here is my proposition: only DefaultDoxia extends AbstractLogEnabled. Sink, parser and macro have independent instances of a logger that they inherit from a super interface (note: api changes!) so in principle they can be set independently. In practice it's the parser that should propagate its logger to the sink and macros that it's using. I have put the logging package into a separate module to avoid a circular dependency, since both sink and parser should be LogEnabled.
I have tested by adding a simple xdoc to the site-renderer test that contains an invalid element, one can see that the plexus logger injected in DefaultDoxia is used indeed.
Remark: I don't think adding a enableLoggin() interface to the Sink API is a good solution but I understand it may be an interim one. From an external users of the Doxia API having to implement an enableLogging() method when all I do is provide an implementation for my Sink is strange. I don't want to log anything in my Sink (or if I do it's with my own logging subsystem).
Vincent Massol
added a comment - Remark: I don't think adding a enableLoggin() interface to the Sink API is a good solution but I understand it may be an interim one. From an external users of the Doxia API having to implement an enableLogging() method when all I do is provide an implementation for my Sink is strange. I don't want to log anything in my Sink (or if I do it's with my own logging subsystem).
New Feature
Closed
Major
Basically, we need to improve logging in Sink, Parser and Macro.
Using Plexus AbstractLogEnabled seems the better way i.e.
public class SinkAdapter extends AbstractLogEnabled implements Sink {}The actual problem is that Sinks are not used as plexus components. Instantiation is direct, i.e.:
due NPE when calling getLogger()
A solution could be to refactor all sinks, parser and macro to use them as components.
We could also need to specify a logger, i.e.
public class SinkAdapter extends AbstractLogEnabled implements Sink { private Logger logger; /** {@inheritDoc} */ public final Logger getLogger() { if ( logger == null ) { if ( super.getLogger() != null ) { logger = super.getLogger(); } else { logger = new Slf4jLogger( Logger.LEVEL_DEBUG, LoggerFactory.getLogger( getClass() )); } } return logger; } public final void setLogger( Logger logger ) { this.logger = logger; } }And specify the current logger during the instantiation, i.e.
Other ideas?
public class SinkAdapter extends AbstractLogEnabled implements Sink {}public class SinkAdapter extends AbstractLogEnabled implements Sink { private Logger logger; /** {@inheritDoc} */ public final Logger getLogger() { if ( logger == null ) { if ( super.getLogger() != null ) { logger = super.getLogger(); } else { logger = new Slf4jLogger( Logger.LEVEL_DEBUG, LoggerFactory.getLogger( getClass() )); } } return logger; } public final void setLogger( Logger logger ) { this.logger = logger; } }