You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by dr...@apache.org on 2017/06/29 09:20:35 UTC

[2/3] brooklyn-server git commit: PR #731: address review comments for LoggingOutputStream

PR #731: address review comments for LoggingOutputStream

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8d7340c5
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8d7340c5
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8d7340c5

Branch: refs/heads/master
Commit: 8d7340c5d0d26b0ac69843d9e19f112a5f846845
Parents: e45a28f
Author: Aled Sage <al...@gmail.com>
Authored: Wed Jun 28 22:57:58 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Wed Jun 28 22:57:58 2017 +0100

----------------------------------------------------------------------
 .../util/stream/LoggingOutputStream.java        | 45 +++++++++++++++-----
 .../util/stream/LoggingOutputStreamTest.java    | 10 +++++
 2 files changed, 44 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8d7340c5/utils/common/src/main/java/org/apache/brooklyn/util/stream/LoggingOutputStream.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/stream/LoggingOutputStream.java b/utils/common/src/main/java/org/apache/brooklyn/util/stream/LoggingOutputStream.java
index 2f0e7bd..7c1322c 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/stream/LoggingOutputStream.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/stream/LoggingOutputStream.java
@@ -21,12 +21,17 @@ package org.apache.brooklyn.util.stream;
 import java.io.FilterOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.apache.http.util.ByteArrayBuffer;
 import org.slf4j.Logger;
 
 /**
  * Wraps another output stream, intercepting the writes to log it.
+ * 
+ * This is <em>not</em> thread safe. It assumes that calls like write, flush and close 
+ * will be done by the same thread.
  */
 public class LoggingOutputStream extends FilterOutputStream {
 
@@ -68,7 +73,9 @@ public class LoggingOutputStream extends FilterOutputStream {
     protected final Logger log;
     protected final String logPrefix;
     private final AtomicBoolean running = new AtomicBoolean(true);
-    private final StringBuilder lineSoFar = new StringBuilder(16);
+    
+    // Uses byte array, rather than StringBuilder, to handle Unicode chars longer than one byte
+    private ByteArrayBuffer lineSoFar = new ByteArrayBuffer(16);
 
     private LoggingOutputStream(Builder builder) {
         super(builder.out != null ? builder.out : NOOP_OUTPUT_STREAM);
@@ -78,7 +85,7 @@ public class LoggingOutputStream extends FilterOutputStream {
 
     @Override
     public void write(int b) throws IOException {
-        if (running.get() && b >= 0) onChar(b);
+        if (running.get()) onChar(b);
         out.write(b);
     }
 
@@ -86,8 +93,8 @@ public class LoggingOutputStream extends FilterOutputStream {
     public void flush() throws IOException {
         try {
             if (lineSoFar.length() > 0) {
-                onLine(lineSoFar.toString());
-                lineSoFar.setLength(0);
+                onLine(lineSoFar.buffer(), lineSoFar.length());
+                clearLineSoFar();
             }
         } finally {
             super.flush();
@@ -100,8 +107,8 @@ public class LoggingOutputStream extends FilterOutputStream {
     @Override
     public void close() throws IOException {
         try {
-            onLine(lineSoFar.toString());
-            lineSoFar.setLength(0);
+            onLine(lineSoFar.buffer(), lineSoFar.length());
+            clearLineSoFar();
         } finally {
             out.close();
             running.set(false);
@@ -110,21 +117,37 @@ public class LoggingOutputStream extends FilterOutputStream {
     
     public void onChar(int c) {
         if (c=='\n' || c=='\r') {
-            if (lineSoFar.length()>0)
+            if (lineSoFar.length() > 0) {
                 //suppress blank lines, so that we can treat either newline char as a line separator
                 //(eg to show curl updates frequently)
-                onLine(lineSoFar.toString());
-            lineSoFar.setLength(0);
+                onLine(lineSoFar.buffer(), lineSoFar.length());
+                clearLineSoFar();
+            }
+            
         } else {
-            lineSoFar.append((char)c);
+            lineSoFar.append(c);
+        }
+    }
+    
+    private void clearLineSoFar() {
+        lineSoFar.setLength(0);
+        
+        // Avoid keeping hold of a lot of memory for too long a time:
+        // if we'd constructed a huge buffer, then get rid of it.
+        if (lineSoFar.capacity() > 1024) {
+            lineSoFar = new ByteArrayBuffer(16);
         }
     }
+
+    public void onLine(byte[] line, int length) {
+        onLine(new String(line, 0, length, StandardCharsets.UTF_8));
+    }
     
     public void onLine(String line) {
         //right trim, in case there is \r or other funnies
         while (line.length()>0 && Character.isWhitespace(line.charAt(line.length()-1)))
             line = line.substring(0, line.length()-1);
-        //right trim, in case there is \r or other funnies
+        //left trim, in case there is \r or other funnies
         while (line.length()>0 && (line.charAt(0)=='\n' || line.charAt(0)=='\r'))
             line = line.substring(1);
         if (!line.isEmpty()) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8d7340c5/utils/common/src/test/java/org/apache/brooklyn/util/stream/LoggingOutputStreamTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/stream/LoggingOutputStreamTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/stream/LoggingOutputStreamTest.java
index 5163bb6..dc174c1 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/stream/LoggingOutputStreamTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/stream/LoggingOutputStreamTest.java
@@ -91,4 +91,14 @@ public class LoggingOutputStreamTest {
         
         assertEquals(logs, ImmutableList.of("myprefix:line1", "myprefix:line2"));
     }
+    
+    @Test
+    public void testLogsUnicode() throws Exception {
+        LoggingOutputStream out = LoggingOutputStream.builder().logger(mockLogger).build();
+        String test = "Лорем.";
+        out.write((test+"\n").getBytes(StandardCharsets.UTF_8));
+        out.flush();
+
+        assertEquals(logs, ImmutableList.of(test));
+    }
 }