[logback-dev] [JIRA] Issue Comment Edited: (LBCORE-128) Please support implementation of binary log files in RollingFileAppender/FileAppender

Tue Feb 16 23:20:33 CET 2010

    [ http://jira.qos.ch/browse/LBCORE-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11533#action_11533 ] 

Joern Huxhorn edited comment on LBCORE-128 at 2/16/10 11:19 PM:
----------------------------------------------------------------

My branch was meant to implement what I need in a manner of least possible impact.
I detached the implementation of FileAppender from WriterAppender, extending UnsynchronizedAppenderBase instead.

writerWrite(String s, boolean flush) (which I didn't rename even though there's no writer anymore but a stream instead - I didn't want to create problems in extending classes outside of logback) is now using convertToBytes(String s) and writeBytes(byte[] bytes, boolean flush), i.e. the appender writes bytes into a stream instead of writing a String into a Writer. Encoding of the String isn't handled by the Writer anymore. Instead, String.getBytes(charset) is used to transform into bytes. The charset is initialized once during startup.

I've also taken a look at Maartens Encoder but I fear I don't really get the reason for either the decorate method or the use of generics in that case.

In the meantime, I've also implemented streaming encoder/decoder interfaces for sulky/Lilith.
http://github.com/huxi/sulky/tree/master/sulky-codec/src/main/java/de/huxhorn/sulky/codec/streaming/
Those are a supplement of my previous byte[]-based encoders/decoders and allow chaining. I use generics to define the type that is encoded/decoded. The decoration of the stream is meant to be encapsulated in the actual implementation.

I realize that this does not allow reusing of the same encapsulated stream, though. This means that encoding using Java serialization is less efficient since duplicate strings, for example, are serialized over and over instead of only once, referencing the already serialized string.
It has the advantage that written byte[] are not dependent on previously written ones, on the other hand. This is crucial for random event access, in my case.

I don't say Maartens design is bad and mine is better. Neither you nor (especially!) Maarten should get this impression.

If you are actually aiming for a bigger change than my small patch then it might be a good idea to keep the layout concept, even for binary files, and add the ability to use binary layouts that would also contain the encoder for the event.
I've enabled binary headers and footers by declaring writeHeader() & writeFooter() protected instead of private in FileAppender - so I could (re)implement them as needed in my extending class.

If Layout would/could handle binary data for header, footer and actual events instead, extending RollingFileAppender wouldn't be necessary anymore. One could simply declare a LilithLayout (with parameters?) instead.
This would be a much cleaner design but I feared that such a fundamental change wouldn't have any chance of being accepted for inclusion. 

Thanks for taking a look at this issue. I really appreciate it!

      was (Author: jhuxhorn):
    My branch was meant to implement what I need in a manner of least possible impact.
I detached the implementation of FileAppender from WriterAppender, extending UnsynchronizedAppenderBase instead.

writerWrite(String s, boolean flush) (which I didn't rename even though there's no writer anymore but a stream instead) is now using convertToBytes(String s) and writeBytes(byte[] bytes, boolean flush), i.e. the appender writes bytes into a stream instead of writing a String into a Writer. Encoding of the String isn't handled by the Writer anymore. Instead, String.getBytes(charset) is used to transform into bytes. The charset is initialized once during startup.