SONAR-4898 fix quality flaws

7 files changed, 83 insertions, 11 deletions

diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java
index 876ccc2b22f..68dfc34182b 100644
--- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/RmiJmxConnector.java
@@ -68,8 +68,6 @@ class RmiJmxConnector implements JmxConnector {
     ProcessMXBean mxBean = execute(callable, timeoutMs);
     if (mxBean != null) {
       register(processRef, mxBean);
-    } else if (!processRef.isTerminated()) {
-      throw new IllegalStateException("Fail to connect to JMX RMI server of " + processRef);
     }
   }
 
diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/StreamGobbler.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/StreamGobbler.java
index 55d95c8a467..cb412965f55 100644
--- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/StreamGobbler.java
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/StreamGobbler.java
@@ -38,9 +38,13 @@ class StreamGobbler extends Thread {
   private final Logger logger;
 
   StreamGobbler(InputStream is, String processKey) {
+    this(is, processKey, LoggerFactory.getLogger(processKey));
+  }
+
+  StreamGobbler(InputStream is, String processKey, Logger logger) {
     super(String.format("Gobbler[%s]", processKey));
     this.is = is;
-    this.logger = LoggerFactory.getLogger(processKey);
+    this.logger = logger;
   }
 
   @Override
@@ -51,8 +55,8 @@ class StreamGobbler extends Thread {
       while ((line = br.readLine()) != null) {
         logger.info(line);
       }
-    } catch (Exception ignored) {
-
+    } catch (Exception e) {
+      logger.error("Fail to read process logs", e);
     } finally {
       IOUtils.closeQuietly(br);
     }
@@ -63,6 +67,7 @@ class StreamGobbler extends Thread {
       try {
         gobbler.join();
       } catch (InterruptedException ignored) {
+        // consider as finished
       }
     }
   }
diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java
index 68934f21066..a3f7210f37f 100644
--- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Timeouts.java
@@ -25,7 +25,7 @@ package org.sonar.process.monitor;
 class Timeouts {
 
   private long terminationTimeout = 120000L;
-  private long jmxConnectionTimeout = 30000L;
+  private long jmxConnectionTimeout = 15000L;
   private long monitorPingInterval = 3000L;
   private long monitorIsReadyTimeout = 10000L;
   private long autokillPingTimeout = 60000L;
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java
index 766f24c86fc..8744fb73ff3 100644
--- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaCommandTest.java
@@ -76,4 +76,11 @@ public class JavaCommandTest {
     command.addJavaOption("-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005");
     assertThat(command.isDebugMode()).isTrue();
   }
+
+  @Test
+  public void split_java_options() throws Exception {
+    JavaCommand command = new JavaCommand("foo");
+    command.addJavaOptions("-Xmx512m -Xms256m -Dfoo");
+    assertThat(command.getJavaOptions()).containsOnly("-Xmx512m", "-Xms256m", "-Dfoo");
+  }
 }
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
index 6f0be0eb5be..043054c40ee 100644
--- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
@@ -33,7 +33,6 @@ import org.sonar.process.SystemExit;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.ConnectException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -359,10 +358,7 @@ public class MonitorTest {
           .readTimeout(500).connectTimeout(500);
         return httpRequest.ok() && httpRequest.body().equals("ping");
       } catch (HttpRequest.HttpRequestException e) {
-        if (e.getCause() instanceof ConnectException) {
-          return false;
-        }
-        throw new IllegalStateException("Fail to know the process status", e);
+        return false;
       }
     }
 
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java
index c4a0f735d60..2289c446566 100644
--- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/RmiJmxConnectorTest.java
@@ -22,6 +22,7 @@ package org.sonar.process.monitor;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.sonar.process.NetworkUtils;
 import org.sonar.process.ProcessMXBean;
 
 import static org.fest.assertions.Assertions.assertThat;
@@ -32,6 +33,20 @@ import static org.mockito.Mockito.when;
 public class RmiJmxConnectorTest {
 
   @Test
+  public void fail_to_connect_in_timely_fashion() throws Exception {
+    RmiJmxConnector connector = new RmiJmxConnector();
+    ProcessRef ref = mock(ProcessRef.class);
+
+    JavaCommand command = new JavaCommand("foo").setJmxPort(NetworkUtils.freePort());
+    try {
+      connector.connect(command, ref, 0L);
+      fail();
+    } catch (IllegalStateException e) {
+      // ok
+    }
+  }
+
+  @Test
   public void throw_exception_on_timeout() throws Exception {
     RmiJmxConnector connector = new RmiJmxConnector();
     ProcessRef ref = mock(ProcessRef.class);
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/StreamGobblerTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/StreamGobblerTest.java
new file mode 100644
index 00000000000..16ebb2837c5
--- /dev/null
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/StreamGobblerTest.java
@@ -0,0 +1,51 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube 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.
+ *
+ * SonarQube 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 this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.process.monitor;
+
+import org.apache.commons.io.IOUtils;
+import org.junit.Test;
+import org.slf4j.Logger;
+
+import java.io.InputStream;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+public class StreamGobblerTest {
+
+  @Test
+  public void forward_stream_to_log() throws Exception {
+    InputStream stream = IOUtils.toInputStream("one\nsecond log\nthird log\n");
+    Logger logger = mock(Logger.class);
+
+    StreamGobbler gobbler = new StreamGobbler(stream, "WEB", logger);
+    verifyZeroInteractions(logger);
+
+    gobbler.start();
+    StreamGobbler.waitUntilFinish(gobbler);
+
+    verify(logger).info("one");
+    verify(logger).info("second log");
+    verify(logger).info("third log");
+    verifyNoMoreInteractions(logger);
+  }
+}