From 6d67c40a7bf3d9afcb954dd1d5b4583111d75732 Mon Sep 17 00:00:00 2001
From: Koji Kawamura <ijokarumawak@apache.org>
Date: Mon, 17 Oct 2016 17:07:32 +0900
Subject: [PATCH] NIFI-2905: Log error stream of ExecuteProcess cmd

ExecuteProcess ignores error stream when Redirect Error Stream is
false, this commit let it to be logged instead so that user can see it on
bulletin.
---
 .../processors/standard/ExecuteProcess.java   |  3 +-
 .../standard/TestExecuteProcess.java          | 46 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
index 62860ed4a2a2..c0d3f2b4c543 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ExecuteProcess.java
@@ -316,8 +316,7 @@ protected Future<?> launchProcess(final ProcessContext context, final List<Strin
                 @Override
                 public void run() {
                     try (final BufferedReader reader = new BufferedReader(new InputStreamReader(externalProcess.getErrorStream()))) {
-                        while (reader.read() >= 0) {
-                        }
+                        reader.lines().filter(line -> line != null && line.length() > 0).forEach(getLogger()::warn);
                     } catch (final IOException ioe) {
                     }
                 }
diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestExecuteProcess.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestExecuteProcess.java
index 160bbdbe4297..8af82f11d168 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestExecuteProcess.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestExecuteProcess.java
@@ -30,6 +30,7 @@
 
 import org.apache.nifi.processor.ProcessContext;
 import org.apache.nifi.processors.standard.util.ArgumentUtils;
+import org.apache.nifi.util.LogMessage;
 import org.apache.nifi.util.MockFlowFile;
 import org.apache.nifi.util.TestRunner;
 import org.apache.nifi.util.TestRunners;
@@ -205,4 +206,49 @@ public void testBigInputSplit() {
 
         // assertEquals(inFile.length(), totalFlowFilesSize);
     }
+
+    @Test
+    public void testNotRedirectErrorStream() {
+        final TestRunner runner = TestRunners.newTestRunner(ExecuteProcess.class);
+        runner.setProperty(ExecuteProcess.COMMAND, "cd");
+        runner.setProperty(ExecuteProcess.COMMAND_ARGUMENTS, "does-not-exist");
+
+        ProcessContext processContext = runner.getProcessContext();
+
+        ExecuteProcess processor = (ExecuteProcess) runner.getProcessor();
+        processor.updateScheduledTrue();
+        processor.setupExecutor(processContext);
+
+        processor.onTrigger(processContext, runner.getProcessSessionFactory());
+
+        final List<LogMessage> warnMessages = runner.getLogger().getWarnMessages();
+        assertEquals("If redirect error stream is false, " +
+                "the output should be logged as a warning so that user can notice on bulletin.", 1, warnMessages.size());
+        final List<MockFlowFile> succeeded = runner.getFlowFilesForRelationship(ExecuteProcess.REL_SUCCESS);
+        assertEquals(0, succeeded.size());
+    }
+
+
+    @Test
+    public void testRedirectErrorStream() {
+        final TestRunner runner = TestRunners.newTestRunner(ExecuteProcess.class);
+        runner.setProperty(ExecuteProcess.COMMAND, "cd");
+        runner.setProperty(ExecuteProcess.COMMAND_ARGUMENTS, "does-not-exist");
+        runner.setProperty(ExecuteProcess.REDIRECT_ERROR_STREAM, "true");
+
+        ProcessContext processContext = runner.getProcessContext();
+
+        ExecuteProcess processor = (ExecuteProcess) runner.getProcessor();
+        processor.updateScheduledTrue();
+        processor.setupExecutor(processContext);
+
+        processor.onTrigger(processContext, runner.getProcessSessionFactory());
+
+        final List<LogMessage> warnMessages = runner.getLogger().getWarnMessages();
+        assertEquals("If redirect error stream is true " +
+                "the output should be sent as a content of flow-file.", 0, warnMessages.size());
+        final List<MockFlowFile> succeeded = runner.getFlowFilesForRelationship(ExecuteProcess.REL_SUCCESS);
+        assertEquals(1, succeeded.size());
+    }
+
 }