BaseLogEventWriter setPrintStream modified to cater for different sce…

…narios, including being called twice.

MappingLogEventWriter setPrintStream modified to cater for different scenarios, including being called twice.

MappingLogEventWriter stream handling simplified by keeping p as the original set PrintStream and treating the CaptureStream as separate.

jpos/src/main/java/org/jpos/util/BaseLogEventWriter.java

@@ -37,6 +37,12 @@ public abstract class BaseLogEventWriter implements LogEventWriter {
 
     @Override
     public void setPrintStream(PrintStream p) {
+        if (p == null) {
+            close();
+            return;
+        }
+        if (this.p == p) return;
+        if (this.p != null) close();
         this.p = p;
     }
 

jpos/src/main/java/org/jpos/util/MappingLogEventWriter.java

@@ -57,46 +57,43 @@
 public class MappingLogEventWriter extends BaseLogEventWriter implements XmlConfigurable {
     List<LogEventMapper> eventMappers;
     List<ByteArrayMapper> outputMappers;
-    ByteArrayOutputStream captureStream;
-    PrintStream originalPrintStream;
+    ByteArrayOutputStream captureOutputStream;
+    PrintStream capturePrintStream;
 
     @Override
     public void setPrintStream(PrintStream p) {
         super.setPrintStream(p);
-        configurePrintStreams();
+        if (p != null && capturePrintStream == null) {
+            configureCaptureStreams();
+        }
     }
 
     @Override
     public synchronized void close() {
-        if (captureStream != null) {
-            try {
-                captureStream.close();
-            } catch (IOException ignored) {
-            } finally {
-                captureStream = null;
-            }
-        }
-        if (originalPrintStream != null) {
-            originalPrintStream.close();
-            originalPrintStream = null;
+        if (capturePrintStream != null) {
+            capturePrintStream.close();
+            capturePrintStream = null;
+            captureOutputStream = null;
         }
         super.close();
     }
 
     @Override
     public void write(LogEvent ev) {
         ev = mapEvents(ev);
-        delegateWriteToSuper(ev);
-        if (outputMappers != null && captureStream != null) {
+        if (capturePrintStream != null) {
+            writeToCaptureStream(ev);
             try {
-                byte[] output = mapOutput(captureStream.toByteArray());
-                originalPrintStream.write(output);
+                byte[] output = mapOutput(captureOutputStream.toByteArray());
+                p.write(output);
             } catch (IOException e) {
-                e.printStackTrace(originalPrintStream);
+                e.printStackTrace(p);
             } finally {
-                originalPrintStream.flush();
-                captureStream.reset();
+                p.flush();
+                captureOutputStream.reset();
             }
+        } else {
+            delegateWriteToSuper(ev);
         }
     }
 
@@ -106,11 +103,10 @@ public void setConfiguration(Element e) throws ConfigurationException {
         configureOutputMappers(e);
     }
 
-    protected void configurePrintStreams() {
+    protected void configureCaptureStreams() {
         if (outputMappers != null && !outputMappers.isEmpty()) {
-            originalPrintStream = p;
-            captureStream = new ByteArrayOutputStream();
-            this.p = new PrintStream(captureStream);
+            captureOutputStream = new ByteArrayOutputStream();
+            capturePrintStream = new PrintStream(captureOutputStream);
         }
     }
 
@@ -192,9 +188,20 @@ protected byte[] mapOutput(byte[] output) {
      * This method exists and is used so that we can verify the order of instructions
      * during a call to write in unit tests.
      *
-     * @param ev LogEvent
+     * @param ev LogEvent to write.
      */
     protected void delegateWriteToSuper(LogEvent ev) {
         super.write(ev);
     }
+
+    /**
+     * Write to capture print stream when defined.
+     * @param ev LogEvent to write.
+     */
+    protected void writeToCaptureStream(LogEvent ev) {
+        if (capturePrintStream != null && ev != null) {
+            ev.dump(capturePrintStream, "");
+            capturePrintStream.flush();
+        }
+    }
 }

jpos/src/test/java/org/jpos/util/BaseLogEventWriterTest.java

@@ -4,12 +4,38 @@
 
 import java.io.PrintStream;
 
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
 public class BaseLogEventWriterTest {
 
+    @Test
+    void testSetPrintStreamShouldCallCloseWhenNullArgument() {
+        TestLogEventWriter writer = spy(new TestLogEventWriter());
+        writer.setPrintStream(null);
+        verify(writer).close();
+        assertNull(writer.p);
+    }
+
+    @Test
+    void testSetPrintStreamShouldCallCloseOnDifferentArgument() {
+        TestLogEventWriter writer = spy(new TestLogEventWriter());
+        writer.p = new PrintStream(System.out);
+        PrintStream newP = new PrintStream(System.out);
+        writer.setPrintStream(newP);
+        verify(writer).close();
+        assertEquals(newP, writer.p);
+    }
+
+    @Test
+    void testSetPrintStreamShouldNOTCallCloseWhenSetSameOrFirstTime() {
+        TestLogEventWriter writer = spy(new TestLogEventWriter());
+        PrintStream printStream = new PrintStream(System.out);
+        writer.setPrintStream(printStream);
+        writer.setPrintStream(printStream);
+        verify(writer, never()).close();
+    }
+
     @Test
     void testShouldClosePrintStreamOnClose() {
         LogEventWriter writer = new TestLogEventWriter();

jpos/src/test/java/org/jpos/util/MappingLogEventWriterTest.java

@@ -41,6 +41,21 @@
 
 public class MappingLogEventWriterTest {
 
+    @Test
+    void testSetPrintStreamShouldConfigurePrintStreamsWhenCapturePrintStreamNull() {
+        MappingLogEventWriter writer = spy(new MappingLogEventWriter());
+        writer.setPrintStream(new PrintStream(System.out));
+        verify(writer).configureCaptureStreams();
+    }
+
+    @Test
+    void testSetPrintStreamShouldNOTConfigurePrintStreamsWhenCapturePrintStreamNOTNull() {
+        MappingLogEventWriter writer = spy(new MappingLogEventWriter());
+        writer.capturePrintStream = new PrintStream(System.out);
+        writer.setPrintStream(new PrintStream(System.out));
+        verify(writer, never()).configureCaptureStreams();
+    }
+
     @Test
     void testMapEventsShouldCallEventMappersInOrderAndReturnFinalResult() {
         MappingLogEventWriter writer = new MappingLogEventWriter();
@@ -101,9 +116,10 @@ void testShouldSetupAdditionalStreamsWhenOutputMappersNotEmpty() throws Configur
         mappers.add(b -> b);
         PrintStream printStream = new PrintStream(System.out);
         writer.setPrintStream(printStream);
-        assertEquals(writer.originalPrintStream, printStream);
-        assertNotNull(writer.captureStream);
-        assertNotEquals(writer.originalPrintStream, writer.p);
+        assertEquals(writer.p, printStream);
+        assertNotNull(writer.captureOutputStream);
+        assertNotNull(writer.capturePrintStream);
+        assertNotEquals(writer.capturePrintStream, writer.p);
         printStream.close();
     }
 
@@ -115,16 +131,16 @@ void testShouldNOTSetupAdditionalStreamsWhenOutputMappersEmptyOrNull() throws Co
         writer.setPrintStream(printStream);
         writer.setConfiguration(root);
         assertEquals(printStream, writer.p);
-        assertNull(writer.captureStream);
-        assertNull(writer.originalPrintStream);
+        assertNull(writer.captureOutputStream);
+        assertNull(writer.capturePrintStream);
 
         writer = new MappingLogEventWriter();
         writer.outputMappers = new ArrayList<>();
         writer.setPrintStream(printStream);
         writer.setConfiguration(root);
         assertEquals(printStream, writer.p);
-        assertNull(writer.captureStream);
-        assertNull(writer.originalPrintStream);
+        assertNull(writer.captureOutputStream);
+        assertNull(writer.capturePrintStream);
         printStream.close();
     }
 
@@ -160,27 +176,24 @@ void testShouldClosePrintStreamsProperly() throws ConfigurationException, IOExce
         writer.setConfiguration(root);
         writer.close();
         assertNull(writer.p);
-        assertNull(writer.originalPrintStream);
-        assertNull(writer.captureStream);
+        assertNull(writer.capturePrintStream);
+        assertNull(writer.captureOutputStream);
 
         PrintStream p1 = mock(PrintStream.class);
         PrintStream p2 = mock(PrintStream.class);
-        ByteArrayOutputStream baos = mock(ByteArrayOutputStream.class);
         writer.p = p1;
-        writer.originalPrintStream = p2;
-        writer.captureStream = baos;
+        writer.capturePrintStream = p2;
         writer.close();
         verify(p1).close();
         verify(p2).close();
-        verify(baos).close();
     }
 
     @Test
     void testShouldNotTryToCloseAnyStreamsThatAreNull() {
         MappingLogEventWriter writer = new MappingLogEventWriter();
         writer.p = null;
-        writer.captureStream = null;
-        writer.originalPrintStream = null;
+        writer.captureOutputStream = null;
+        writer.capturePrintStream = null;
         assertDoesNotThrow(writer::close);
     }
 
@@ -213,17 +226,35 @@ void testShouldAddOutputMapperWhenConfigured() throws ConfigurationException {
     }
 
     @Test
-    void testWriteShouldMapEventsThenWriteOnSuperThenMapOutput() {
+    void testWriteShouldMapEventsThenWriteOnSuperWhenNoOutputMappers() {
         MappingLogEventWriter spy = spy(new MappingLogEventWriter());
         spy.outputMappers = new ArrayList<>();
-        spy.captureStream = new ByteArrayOutputStream();
-        spy.originalPrintStream = mock(PrintStream.class);
+        spy.captureOutputStream = new ByteArrayOutputStream();
+        spy.p = mock(PrintStream.class);
         LogEvent ev = new LogEvent();
         spy.write(ev);
         InOrder inOrder = inOrder(spy);
         inOrder.verify(spy).mapEvents(ev);
         inOrder.verify(spy).delegateWriteToSuper(ev);
-        inOrder.verify(spy).mapOutput(spy.captureStream.toByteArray());
+        verify(spy, never()).writeToCaptureStream(ev);
+        verify(spy, never()).mapOutput(any());
+    }
+
+    @Test
+    void testWriteShouldMapEventsThenMapOutputThenWriteToMainPrintStream() throws IOException {
+        MappingLogEventWriter spy = spy(new MappingLogEventWriter());
+        spy.outputMappers = new ArrayList<>();
+        spy.outputMappers.add(b -> b);
+        PrintStream printStream = mock(PrintStream.class);
+        spy.setPrintStream(printStream);
+        LogEvent ev = new LogEvent();
+        spy.write(ev);
+        InOrder inOrder = inOrder(spy, printStream);
+        inOrder.verify(spy).mapEvents(ev);
+        inOrder.verify(spy).writeToCaptureStream(ev);
+        inOrder.verify(spy).mapOutput(any());
+        inOrder.verify(printStream).write(any());
+        verify(spy, never()).delegateWriteToSuper(ev);
     }
 
     @Test
@@ -243,7 +274,7 @@ void testMakeSureOutputIsOnlyThatOfCurrentEvent() throws IOException {
         List<ByteArrayMapper> outputMappers = new ArrayList<>();
         outputMappers.add(b -> b);
         writer.outputMappers = outputMappers;
-        writer.configurePrintStreams();
+        writer.configureCaptureStreams();
         LogEvent e1 = new LogEvent("1");
         e1.setNoArmor(true);
         e1.addMessage("1");