SONAR-3318 Allow to capture stdout and stderr using CommandExecutor

Also improve handling of exceptions.

9 files changed, 219 insertions, 35 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/CommandExecutor.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/CommandExecutor.java
index 55d52f6063b..8ec3bf2cb66 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/CommandExecutor.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/CommandExecutor.java
@@ -19,25 +19,26 @@
  */
 package org.sonar.api.utils.command;
 
+import com.google.common.io.Closeables;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.concurrent.*;
 
-import org.apache.commons.io.IOUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * Synchronously execute a native command line. It's much more limited than the Apache Commons Exec library.
- * For example it does not allow to get process output, to run asynchronously or to automatically quote
- * command-line arguments.
+ * For example it does not allow to run asynchronously or to automatically quote command-line arguments.
  *
  * @since 2.7
  */
 public final class CommandExecutor {
 
+  private static final Logger LOG = LoggerFactory.getLogger(CommandExecutor.class);
+
   private static final CommandExecutor INSTANCE = new CommandExecutor();
 
   private CommandExecutor() {
@@ -48,22 +49,24 @@ public final class CommandExecutor {
     return INSTANCE;
   }
 
-  public int execute(Command command, long timeoutMilliseconds) {
+  /**
+   * @throws CommandException
+   * @since 2.15
+   */
+  public int execute(Command command, StreamConsumer stdOut, StreamConsumer stdErr, long timeoutMilliseconds) {
     ExecutorService executorService = null;
     Process process = null;
     StreamGobbler outputGobbler = null;
     StreamGobbler errorGobbler = null;
     try {
-      LoggerFactory.getLogger(getClass()).debug("Executing command: " + command);
       ProcessBuilder builder = new ProcessBuilder(command.toStrings());
       if (command.getDirectory() != null) {
         builder.directory(command.getDirectory());
       }
       process = builder.start();
 
-      // consume and display the error and output streams
-      outputGobbler = new StreamGobbler(process.getInputStream());
-      errorGobbler = new StreamGobbler(process.getErrorStream());
+      outputGobbler = new StreamGobbler(process.getInputStream(), stdOut);
+      errorGobbler = new StreamGobbler(process.getErrorStream(), stdErr);
       outputGobbler.start();
       errorGobbler.start();
 
@@ -76,12 +79,22 @@ public final class CommandExecutor {
 
       executorService = Executors.newSingleThreadExecutor();
       Future<Integer> ft = executorService.submit(call);
-      return ft.get(timeoutMilliseconds, TimeUnit.MILLISECONDS);
+      int exitCode = ft.get(timeoutMilliseconds, TimeUnit.MILLISECONDS);
+      if (outputGobbler.getException() != null) {
+        throw new CommandException(command, "Error inside stdOut parser", outputGobbler.getException());
+      }
+      if (errorGobbler.getException() != null) {
+        throw new CommandException(command, "Error inside stdErr parser", errorGobbler.getException());
+      }
+      return exitCode;
 
     } catch (TimeoutException te) {
       process.destroy();
       throw new CommandException(command, "Timeout exceeded: " + timeoutMilliseconds + " ms", te);
 
+    } catch (CommandException e) {
+      throw e;
+
     } catch (Exception e) {
       throw new CommandException(command, e);
 
@@ -96,11 +109,24 @@ public final class CommandExecutor {
     }
   }
 
+  /**
+   * Execute command and display error and output streams in log.
+   * Method {@link #execute(Command, StreamConsumer, StreamConsumer, long)} is preferable,
+   * when fine-grained control of output of command required.
+   *
+   * @throws CommandException
+   */
+  public int execute(Command command, long timeoutMilliseconds) {
+    LOG.info("Executing command: " + command);
+    return execute(command, new DefaultConsumer(), new DefaultConsumer(), timeoutMilliseconds);
+  }
+
   private void closeStreams(Process process) {
     if (process != null) {
-      IOUtils.closeQuietly(process.getInputStream());
-      IOUtils.closeQuietly(process.getOutputStream());
-      IOUtils.closeQuietly(process.getErrorStream());
+      Closeables.closeQuietly(process.getInputStream());
+      Closeables.closeQuietly(process.getInputStream());
+      Closeables.closeQuietly(process.getOutputStream());
+      Closeables.closeQuietly(process.getErrorStream());
     }
   }
 
@@ -109,36 +135,58 @@ public final class CommandExecutor {
       try {
         thread.join();
       } catch (InterruptedException e) {
-        // ignore
+        LOG.error("InterruptedException while waiting finish of " + thread.toString(), e);
       }
     }
   }
 
   private static class StreamGobbler extends Thread {
-    InputStream is;
+    private final InputStream is;
+    private final StreamConsumer consumer;
+    private volatile Exception exception;
 
-    StreamGobbler(InputStream is) {
+    StreamGobbler(InputStream is, StreamConsumer consumer) {
       super("ProcessStreamGobbler");
       this.is = is;
+      this.consumer = consumer;
     }
 
     @Override
     public void run() {
-      Logger logger = LoggerFactory.getLogger(CommandExecutor.class);
       InputStreamReader isr = new InputStreamReader(is);
       BufferedReader br = new BufferedReader(isr);
       try {
         String line;
         while ((line = br.readLine()) != null) {
-          logger.info(line);
+          consumeLine(line);
         }
       } catch (IOException ioe) {
-        logger.error("Error while reading stream", ioe);
+        exception = ioe;
 
       } finally {
-        IOUtils.closeQuietly(br);
-        IOUtils.closeQuietly(isr);
+        Closeables.closeQuietly(br);
+        Closeables.closeQuietly(isr);
+      }
+    }
+
+    private void consumeLine(String line) {
+      if (exception == null) {
+        try {
+          consumer.consumeLine(line);
+        } catch (Exception e) {
+          exception = e;
+        }
       }
     }
+
+    public Exception getException() {
+      return exception;
+    }
+  }
+
+  private static class DefaultConsumer implements StreamConsumer {
+    public void consumeLine(String line) {
+      LOG.info(line);
+    }
   }
 }
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/StreamConsumer.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/StreamConsumer.java
new file mode 100644
index 00000000000..2e818247014
--- /dev/null
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/StreamConsumer.java
@@ -0,0 +1,26 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.api.utils.command;
+
+public interface StreamConsumer {
+
+  void consumeLine(String line);
+
+}
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/package-info.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/package-info.java
new file mode 100644
index 00000000000..d91fb84cf43
--- /dev/null
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/command/package-info.java
@@ -0,0 +1,24 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+@ParametersAreNonnullByDefault
+package org.sonar.api.utils.command;
+
+import javax.annotation.ParametersAreNonnullByDefault;
+
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandExecutorTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandExecutorTest.java
index 10dc22b0f35..6bcb1307e51 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandExecutorTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/command/CommandExecutorTest.java
@@ -21,7 +21,12 @@ package org.sonar.api.utils.command;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.SystemUtils;
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
 
 import java.io.File;
 import java.io.IOException;
@@ -34,6 +39,76 @@ import static org.junit.matchers.JUnitMatchers.containsString;
 
 public class CommandExecutorTest {
 
+  @Rule
+  public TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @Rule
+  public TestName testName = new TestName();
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  private File workDir;
+
+  @Before
+  public void setUp() {
+    workDir = tempFolder.newFolder(testName.getMethodName());
+  }
+
+  @Test
+  public void shouldConsumeStdOutAndStdErr() throws Exception {
+    final StringBuilder stdOutBuilder = new StringBuilder();
+    StreamConsumer stdOutConsumer = new StreamConsumer() {
+      public void consumeLine(String line) {
+        stdOutBuilder.append(line).append(SystemUtils.LINE_SEPARATOR);
+      }
+    };
+    final StringBuilder stdErrBuilder = new StringBuilder();
+    StreamConsumer stdErrConsumer = new StreamConsumer() {
+      public void consumeLine(String line) {
+        stdErrBuilder.append(line).append(SystemUtils.LINE_SEPARATOR);
+      }
+    };
+    Command command = Command.create(getScript("output")).setDirectory(workDir);
+    int exitCode = CommandExecutor.create().execute(command, stdOutConsumer, stdErrConsumer, 1000L);
+    assertThat(exitCode, is(0));
+
+    String stdOut = stdOutBuilder.toString();
+    String stdErr = stdErrBuilder.toString();
+    assertThat(stdOut, containsString("stdOut: first line"));
+    assertThat(stdOut, containsString("stdOut: second line"));
+    assertThat(stdErr, containsString("stdErr: first line"));
+    assertThat(stdErr, containsString("stdErr: second line"));
+  }
+
+  @Test
+  public void stdOutConsumerCanThrowException() throws Exception {
+    Command command = Command.create(getScript("output")).setDirectory(workDir);
+    thrown.expect(CommandException.class);
+    thrown.expectMessage("Error inside stdOut parser");
+    CommandExecutor.create().execute(command, BAD_CONSUMER, NOP_CONSUMER, 1000L);
+  }
+
+  @Test
+  public void stdErrConsumerCanThrowException() throws Exception {
+    Command command = Command.create(getScript("output")).setDirectory(workDir);
+    thrown.expect(CommandException.class);
+    thrown.expectMessage("Error inside stdErr parser");
+    CommandExecutor.create().execute(command, NOP_CONSUMER, BAD_CONSUMER, 1000L);
+  }
+
+  private static final StreamConsumer NOP_CONSUMER = new StreamConsumer() {
+    public void consumeLine(String line) {
+      // nop
+    }
+  };
+
+  private static final StreamConsumer BAD_CONSUMER = new StreamConsumer() {
+    public void consumeLine(String line) {
+      throw new RuntimeException();
+    }
+  };
+
   @Test
   public void shouldEchoArguments() throws IOException {
     String executable = getScript("echo");
@@ -47,15 +122,12 @@ public class CommandExecutorTest {
   @Test
   public void shouldConfigureWorkingDirectory() throws IOException {
     String executable = getScript("echo");
-    File dir = new File("target/tmp/CommandExecutorTest/shouldConfigureWorkingDirectory");
-    FileUtils.forceMkdir(dir);
-    FileUtils.cleanDirectory(dir);
 
-    int exitCode = CommandExecutor.create().execute(Command.create(executable).setDirectory(dir), 1000L);
+    int exitCode = CommandExecutor.create().execute(Command.create(executable).setDirectory(workDir), 1000L);
     assertThat(exitCode, is(0));
 
-    File log = new File(dir, "echo.log");
-    assertThat(FileUtils.readFileToString(log), containsString(dir.getCanonicalPath()));
+    File log = new File(workDir, "echo.log");
+    assertThat(FileUtils.readFileToString(log), containsString(workDir.getCanonicalPath()));
   }
 
   @Test
@@ -63,7 +135,7 @@ public class CommandExecutorTest {
     String executable = getScript("forever");
     long start = System.currentTimeMillis();
     try {
-      CommandExecutor.create().execute(Command.create(executable), 300L);
+      CommandExecutor.create().execute(Command.create(executable).setDirectory(workDir), 300L);
       fail();
     } catch (CommandException e) {
       long duration = System.currentTimeMillis() - start;
@@ -73,12 +145,13 @@ public class CommandExecutorTest {
     }
   }
 
-  @Test(expected = CommandException.class)
+  @Test
   public void shouldFailIfScriptNotFound() {
-    CommandExecutor.create().execute(Command.create("notfound"), 1000L);
+    thrown.expect(CommandException.class);
+    CommandExecutor.create().execute(Command.create("notfound").setDirectory(workDir), 1000L);
   }
 
-  private String getScript(String name) throws IOException {
+  private static String getScript(String name) throws IOException {
     String filename;
     if (SystemUtils.IS_OS_WINDOWS) {
       filename = name + ".bat";
@@ -87,4 +160,5 @@ public class CommandExecutorTest {
     }
     return new File("src/test/scripts/" + filename).getCanonicalPath();
   }
+
 }
diff --git a/sonar-plugin-api/src/test/scripts/echo.bat b/sonar-plugin-api/src/test/scripts/echo.bat
index 0f2436ceb71..0d2e92c40b6 100755
--- a/sonar-plugin-api/src/test/scripts/echo.bat
+++ b/sonar-plugin-api/src/test/scripts/echo.bat
@@ -1,3 +1,3 @@
 @ECHO OFF
 @ECHO %CD% > echo.log
-@ECHO "Parameter: " + %1
+@ECHO Parameter: %1
diff --git a/sonar-plugin-api/src/test/scripts/echo.sh b/sonar-plugin-api/src/test/scripts/echo.sh
index 5e2504df664..487e35e0257 100755
--- a/sonar-plugin-api/src/test/scripts/echo.sh
+++ b/sonar-plugin-api/src/test/scripts/echo.sh
@@ -2,4 +2,4 @@
 
 WORKING_DIR=`pwd`
 echo $WORKING_DIR > echo.log
-echo "Parameter: " + $1
+echo "Parameter: $1"
diff --git a/sonar-plugin-api/src/test/scripts/forever.sh b/sonar-plugin-api/src/test/scripts/forever.sh
index 67289db934e..d7b6a9b38fd 100755
--- a/sonar-plugin-api/src/test/scripts/forever.sh
+++ b/sonar-plugin-api/src/test/scripts/forever.sh
@@ -1,4 +1,5 @@
 #!/bin/sh
+
 while test "notempty"
 do
   sleep 1
diff --git a/sonar-plugin-api/src/test/scripts/output.bat b/sonar-plugin-api/src/test/scripts/output.bat
new file mode 100755
index 00000000000..967bc6092fb
--- /dev/null
+++ b/sonar-plugin-api/src/test/scripts/output.bat
@@ -0,0 +1,5 @@
+@ECHO OFF
+@ECHO stdOut: first line
+@ECHO stdOut: second line
+@ECHO stdErr: first line 1>&2
+@ECHO stdErr: second line 1>&2
diff --git a/sonar-plugin-api/src/test/scripts/output.sh b/sonar-plugin-api/src/test/scripts/output.sh
new file mode 100755
index 00000000000..cbb0fea9e0f
--- /dev/null
+++ b/sonar-plugin-api/src/test/scripts/output.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+echo stdOut: first line
+echo stdOut: second line
+echo stdErr: first line 1>&2
+echo stdErr: second line 1>&2