Replace XRLogger with some logging framework API

[aleksandr-m]

Log4j2 perhaps? And get rid of openhtmltopdf-log4j and openhtmltopdf-slf4j.
Wdyt?

[danfickle]

G'day @aleksandr-m
I was thinking of the following use case. Imagine a CMS to output PDF documents. There are many templates and an on-line template editor to create more. Currently, the only way for the template author to see her errors is to have access to the server logs. Even if she has access, the output for the template she is editing is mixed in with output for all the other templates that may be output at the same time. Wouldn't it be better if she could see the errors for the template she is working on in the online editor?

To support this use case I propose to have a log interceptor for each run configurable in the builder. Then the log output could be shown in the online editor. While doing this I suggest we also move to logging a enum constant (with associated message format) rather than a string directly for anyone that wants to translate or filter log messages. So the log message class might look like following:

public class LogMessage {
   private final LogMessageId id;
   private final Object[] args;
   private final Level level;

   // constructor...
   // getters...
}

and the LogMessageId enum like following:

public enum LogMessageId {
  UNRECOGNIZED_CSS_PROPERTY("The CSS property {} is not recognized"),
  UNIMPLEMENTED_CSS_VALUE("The CSS value {} is not currently implemented");

  private final String msgFormat;
  private LogMessageId(String format) {
    this.msgFormat = format;
  }

  public String getMessageFormat() {
    return msgFormat;
  }
}

Then we would provide a DefaultStringLogInterceptor something like:

public class DefaultStringLogInterceptor implements LogReceiver {
  private final StringBuilder sb = new StringBuilder();

  @Override
  public void receive(LogMessage msg) {
    sb.append(MessageFormat.format(msg.getId().getMessageFormat(), msg.getArguments()));
  }
  
  @Override
  public String toString() {
    return sb.toString();
  }
}

As for Log4j2, I would be wary of including it directly as it may cause jar version hell for many users of the library. I agree we should get rid of openhtmltopdf-log4j and openhtmltopdf-slf4j and replace with examples in the documentation (they're both single files) because I doubt anyone is using them because of aforementioned version hell.

OK, that's a long post, anyway, what do you think?

[aleksandr-m]

You don't have to include whole Log4j2 just log4j-api.

Then the ugly string concatenations

XRLog.cssParse("Removing stylesheet '" + uri + "' from cache by request.");

can be replaced with

LOG.info("Removing stylesheet '{}' from cache by request.", uri);

etc.

And the end users can use different logging implementations.

[aarowman]

+1

[stechio]

@aleksandr-m said:

Log4j2 perhaps? And get rid of openhtmltopdf-log4j and openhtmltopdf-slf4j.
[cut]
You don't have to include whole Log4j2 just log4j-api.
[cut]
And the end users can use different logging implementations.

I fully support this request: it would be highly beneficial (at both implementation and integration levels) to this project to get rid of its legacy logging system in favor of a modern, convenient, widely available framework which neatly integrates to runtime environments without the burden of custom dependencies.