Merge pull request #114 from dwnusbaum/log-tweaks

Fix getLogInputStream and doConsoleText for large log files
jenkinsci · Nov 6, 2018 · 72cc8ac · 72cc8ac
2 parents 1fa3953 + 133573a
commit 72cc8ac
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java b/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
@@ -1017,14 +1017,14 @@ private final class NodePrintListener implements GraphListener.Synchronous {
     @Override public InputStream getLogInputStream() throws IOException {
         // Inefficient but probably rarely used anyway.
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        getLogText().writeRawLogTo(0, baos);
+        writeLogTo(getLogText()::writeLogTo, baos);
         return new ByteArrayInputStream(baos.toByteArray());
     }
 
     @Override public void doConsoleText(StaplerRequest req, StaplerResponse rsp) throws IOException {
         rsp.setContentType("text/plain;charset=UTF-8");
         try (OutputStream os = rsp.getCompressedOutputStream(req)) {
-            getLogText().writeLogTo(0, os);
+            writeLogTo(getLogText()::writeLogTo, os);
         }
     }
 
@@ -1035,10 +1035,26 @@ private final class NodePrintListener implements GraphListener.Synchronous {
     @SuppressWarnings("deprecation")
     @Override public String getLog() throws IOException {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        getLogText().writeRawLogTo(0, baos);
+        writeLogTo(getLogText()::writeRawLogTo, baos);
         return baos.toString("UTF-8");
     }
 
+    @FunctionalInterface private interface WriteMethod {
+        public long writeLogTo(long start, OutputStream os) throws IOException;
+    }
+
+    private void writeLogTo(WriteMethod method, OutputStream os) throws IOException {
+        // Similar to Run#writeWholeLogTo but terminates even if !logText.complete:
+        long pos = 0;
+        while (true) {
+            long pos2 = method.writeLogTo(pos, os);
+            if (pos2 <= pos) {
+                break;
+            }
+            pos = pos2;
+        }
+    }
+
     @Override public List<String> getLog(int maxLines) throws IOException {
         int lineCount = 0;
         List<String> logLines = new LinkedList<>();

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/job/console/DefaultLogStorageTest.java b/src/test/java/org/jenkinsci/plugins/workflow/job/console/DefaultLogStorageTest.java
@@ -40,7 +40,9 @@
 import hudson.slaves.SlaveComputer;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.InputStream;
 import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Set;
 import java.util.logging.Level;
@@ -58,6 +60,7 @@
 import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
 import org.jenkinsci.plugins.workflow.steps.StepExecution;
 import org.jenkinsci.plugins.workflow.steps.SynchronousStepExecution;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
 import org.junit.Ignore;
@@ -254,4 +257,36 @@ static class Execution extends SynchronousStepExecution<Void> {
         b.writeWholeLogTo(System.out);
     }
 
+    @Test public void doConsoleText() throws Exception {
+        WorkflowJob p = r.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition("@NonCPS def giant() {(0..19999).join('\\n')}; echo giant(); semaphore 'wait'", true));
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        SemaphoreStep.waitForStart("wait/1", b);
+        assertThat(r.createWebClient().goTo(b.getUrl() + "consoleText", "text/plain").getWebResponse().getContentAsString(), containsString("\n12345\n"));
+        SemaphoreStep.success("wait/1", null);
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+    }
+
+    @Test public void getLogInputStream() throws Exception {
+        WorkflowJob p = r.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition("@NonCPS def giant() {(0..19999).join('\\n')}; echo giant(); semaphore 'wait'", true));
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        SemaphoreStep.waitForStart("wait/1", b);
+        try (InputStream logStream = b.getLogInputStream()) {
+            assertThat(IOUtils.toString(logStream, StandardCharsets.UTF_8), containsString("\n12345\n"));
+        }
+        SemaphoreStep.success("wait/1", null);
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+    }
+
+    @Test public void getLog() throws Exception {
+        WorkflowJob p = r.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition("@NonCPS def giant() {(0..19999).join('\\n')}; echo giant(); semaphore 'wait'", true));
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        SemaphoreStep.waitForStart("wait/1", b);
+        assertThat(b.getLog(), containsString("\n12345\n"));
+        SemaphoreStep.success("wait/1", null);
+        r.assertBuildStatusSuccess(r.waitForCompletion(b));
+    }
+
 }