[jcifs] Patch Suggestion

[Michael B Allen]

Hi Hardy,

Actually no. This is actually a flaw in the LogStream class. The code
I posted before will not work. The current LogStream implementation
looks like it might allow overriding PrintStream methods but the
stream parameter in the super(stream) call is actually being treated
as an OutputStream. So you can only override OutputStream methods and
not PrintStream methods. Because the PrintStream class could call
various write() methods multiple times for a single println call, that
is not very useful.

I have added this to the TODO list as something to fix.

The new implementation should probably look something like:

public class LogStream extends PrintStream
{

    private static String LINEBREAK = System.getProperty("line.separator");
    private static LogStream inst;

    public static int level = 1;

    public LogStream(OutputStream out)
    {
        super(out, true);
    }

    public static void setLevel(int level)
    {
        LogStream.level = level;
    }
    public static void setInstance(PrintStream stream)
    {
        if (stream == null) {
            inst = new LogStream(System.err);
        } else if (stream instanceof LogStream) {
            inst = (LogStream)stream;
        } else {
            inst = new LogStream(stream);
        }
    }
    public static LogStream getInstance()
    {
        if (inst == null)
            setInstance(null);
        return inst;
    }
    public void println(Object o)
    {
        byte[] msg = (o + LINEBREAK).getBytes();
        try {
            out.write(msg);
        } catch (IOException ioe) {
        }
    }
    public void println(String s)
    {
        byte[] msg = (s + LINEBREAK).getBytes();
        try {
            out.write(msg);
        } catch (IOException ioe) {
        }
    }
}

But I would have to study this for a while to make sure it's as simple
as it can be and backward compatible.

Mike

-- 
Michael B Allen
Java Active Directory Integration
http://www.ioplex.com/

On Wed, May 2, 2012 at 7:06 PM, Hardy Cherry <hcherry at xactware.com> wrote:
> After implementing Michael's solution I ran into some problems. Here is what I have and what I think is happening.
>
> Let's say I have this class
> public class TimeStampedJCIFSLogStream extends PrintStream
> {
>        private static final String newLine = System.getProperty("line.separator");
>
>        public TimeStampedJCIFSLogStream(PrintStream stream)
>        {
>                super(stream);
>        }
>
>        @Override
>        public void println(String x)
>        {
>                super.println(new java.util.Date().toString() + newLine + "  Source: jCIFS Error " + newLine + "  ERROR: " + x);
>        }
>
>        @Override
>        public void print(String s)
>        {
>                super.print(new java.util.Date().toString() + newLine + "  Source: jCIFS Error " + newLine + "  ERROR: " + s);
>        }
> }
>
> And here is the LogStream class in 1.3.17, I removed the log level variable because it's not important for this question.
>
> public class LogStream extends PrintStream {
>
>    private static LogStream inst;
>
>    public LogStream( PrintStream stream ) {
>        super( stream );
>    }
>    /**
>     * This must be called before <tt>getInstance</tt> is called or
>     * it will have no effect.
>     */
>    public static void setInstance( PrintStream stream ) {
>        inst = new LogStream( stream );
>    }
>    public static LogStream getInstance() {
>        if( inst == null ) {
>            setInstance( System.err );
>        }
>        return inst;
>    }
> }
>
> In my code I set up the LogStream like this.
> LogStream.setInstance(new TimeStampedJCIFSLogStream (System.err))
>
> Now at some jCIFS calls LogStream.println("Something to print");
>
> It appears that when it calls println, it calls the PrintStream.println() function and bypasses the TimeStampedJCIFSLogStream.println() function. I believe that happens because when I call setInstance it sets the instance to a new LogStream. I guess since it wraps the passed in print stream (which is an instance of TimeStampedJCIFSLogStream), it overrides the overridden methods in TimeStampedJCIFSLogStream with the println function that LogStream inherits. Without making changes to the LogStream class is there any way to get the code to call the TimeStampedJCIFSLogStream.println() function instead of the LogStream.println function?
>
>
> -Hardy
>
>
> -----Original Message-----
> From: Michael B Allen [mailto:ioplex at gmail.com]
> Sent: Tuesday, May 01, 2012 5:50 AM
> To: Hardy Cherry
> Cc: jcifs at lists.samba.org
> Subject: Re: [jcifs] Patch Suggestion
>
> On Mon, Apr 30, 2012 at 1:07 PM, Hardy Cherry <hcherry at xactware.com> wrote:
>> We are investigating the use of jCIFS in one of our products. To monitor our error logs we use splunk which depends on each log having a timestamp.
>> Would it be possible to format the logs jCIFS creates so that they have a time stamp?
>>
>> Here is my suggested change to the LogStream class:
>> Add an overrided version of println()
>>
>>    @Override
>>    public void println(String x)
>>    {
>>       String newLine = System.getProperty("line.separator");
>>       //Format the error before printing it. Will look like this
>>       //Mon Apr 30 04:06:05 MDT 2012
>>       //  Source: jCIFS Error Logging
>>       //  ERROR: x
>>       super.println(new java.util.Date().toString() + newLine + "
>> Source: jCIFS Error Logging " + newLine + "  ERROR: " + x);
>>    }
>
> Hi Hardy,
>
> A patch is not necessary. Just extend the log stream class, override both println methods as desired and then install it with LogStream.setInstance().
>
> But multiple lines for each entry is probably not what you want. A proper implementation would probably look something like:
>
> class TimestampedLogStream extends jcifs.util.LogStream {
>
>    SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
>
>    TimestampedLogStream(OutputStream out)
>    {
>        super(out);
>    }
>
>    public void println(Object o)
>    {
>        synchronized (sdf) {
>            super.println(sdf.format(new Date()) + ": " + o);
>        }
>    }
>    public void println(String s)
>    {
>        synchronized (sdf) {
>            super.println(sdf.format(new Date()) + ": " + s);
>        }
>
> And then install this early in your program somewhere with a statement like:
>
>  jcifs.util.LogStream.setInstance(new TimestampedLogStream(System.err));
>
> Mike