Fix some quality flaws

9 files changed, 63 insertions, 58 deletions

diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java b/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
index d9187c3e6d4..5f0c03bea7a 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.process;
 
-import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -35,7 +34,6 @@ public abstract class MonitoredProcess implements ProcessMXBean {
   private static final long AUTOKILL_TIMEOUT_MS = 30000L;
   private static final long AUTOKILL_CHECK_DELAY_MS = 2000L;
   public static final String NAME_PROPERTY = "pName";
-  public static final String MISSING_NAME_ARGUMENT = "Missing Name argument";
 
   private Long lastPing;
   private final String name;
@@ -56,12 +54,7 @@ public abstract class MonitoredProcess implements ProcessMXBean {
   protected MonitoredProcess(Props props, boolean monitor) {
     this.isMonitored = monitor;
     this.props = props;
-    this.name = props.of(NAME_PROPERTY);
-
-    // Testing required properties
-    if (StringUtils.isEmpty(name)) {
-      throw new IllegalStateException(MISSING_NAME_ARGUMENT);
-    }
+    this.name = props.nonNullValue(NAME_PROPERTY);
 
     JmxUtils.registerMBean(this, name);
     ProcessUtils.addSelfShutdownHook(this);
diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessLogging.java b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessLogging.java
index 6b2e49e55fd..dacddd91847 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessLogging.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessLogging.java
@@ -35,7 +35,7 @@ public class ProcessLogging {
       JoranConfigurator configurator = new JoranConfigurator();
       configurator.setContext(context);
       context.reset();
-      context.putProperty(PATH_LOGS_PROPERTY, props.of(PATH_LOGS_PROPERTY));
+      context.putProperty(PATH_LOGS_PROPERTY, props.nonNullValue(PATH_LOGS_PROPERTY));
       doConfigure(configurator, logbackXmlResource);
     } catch (JoranException ignored) {
       // StatusPrinter will handle this
diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/Props.java b/server/process/sonar-process/src/main/java/org/sonar/process/Props.java
index d6e2795a597..b868702eafc 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/Props.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/Props.java
@@ -23,6 +23,7 @@ import org.apache.commons.lang.StringUtils;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.io.File;
 import java.util.Properties;
 
@@ -41,7 +42,7 @@ public class Props {
   }
 
   @CheckForNull
-  public String of(String key) {
+  public String value(String key) {
     String value = properties.getProperty(key);
     if (value != null && encryption.isEncrypted(value)) {
       value = encryption.decrypt(value);
@@ -49,29 +50,41 @@ public class Props {
     return value;
   }
 
-  public String of(String key, @Nullable String defaultValue) {
-    String s = of(key);
+  public String nonNullValue(String key) {
+    String value = value(key);
+    if (value == null) {
+      throw new IllegalArgumentException("Missing property: " + key);
+    }
+    return value;
+  }
+
+  @CheckForNull
+  public String value(String key, @Nullable String defaultValue) {
+    String s = value(key);
     return s == null ? defaultValue : s;
   }
 
-  public boolean booleanOf(String key) {
-    String s = of(key);
+  public boolean valueAsBoolean(String key) {
+    String s = value(key);
     return s != null && Boolean.parseBoolean(s);
   }
 
-  public boolean booleanOf(String key, boolean defaultValue) {
-    String s = of(key);
+  public boolean valueAsBoolean(String key, boolean defaultValue) {
+    String s = value(key);
     return s != null ? Boolean.parseBoolean(s) : defaultValue;
   }
 
-  @CheckForNull
-  public File fileOf(String key) {
-    String s = of(key);
-    return s != null ? new File(s) : null;
+  public File nonNullValueAsFile(String key) {
+    String s = value(key);
+    if (s == null) {
+      throw new IllegalArgumentException("Property " + key + " is missing");
+    }
+    return new File(s);
   }
 
-  public Integer intOf(String key) {
-    String s = of(key);
+  @CheckForNull
+  public Integer valueAsInt(String key) {
+    String s = value(key);
     if (s != null && !"".equals(s)) {
       try {
         return Integer.parseInt(s);
@@ -82,8 +95,8 @@ public class Props {
     return null;
   }
 
-  public int intOf(String key, int defaultValue) {
-    Integer i = intOf(key);
+  public int valueAsInt(String key, int defaultValue) {
+    Integer i = valueAsInt(key);
     return i == null ? defaultValue : i;
   }
 
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/BaseProcessTest.java
index f8b9430298f..2045cd4516d 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/BaseProcessTest.java
@@ -29,7 +29,7 @@ import java.io.File;
 import java.io.IOException;
 import java.net.ServerSocket;
 
-public abstract class ProcessTest {
+public abstract class BaseProcessTest {
 
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
@@ -49,11 +49,10 @@ public abstract class ProcessTest {
     dummyAppJar = FileUtils.toFile(getClass().getResource("/sonar-dummy-app.jar"));
   }
 
-
   @After
   public void tearDown() {
     if (proc != null) {
-      proc.destroy();
+      ProcessUtils.destroyQuietly(proc);
     }
   }
 
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/BaseProcessWrapperTest.java
index 2a7f10ce3b7..b217edb2584 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/BaseProcessWrapperTest.java
@@ -30,7 +30,7 @@ import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 
 
-public class ProcessWrapperTest extends ProcessTest {
+public class BaseProcessWrapperTest extends BaseProcessTest {
 
   @Test
   public void has_dummy_app() {
@@ -100,7 +100,7 @@ public class ProcessWrapperTest extends ProcessTest {
     props.set("spaceHome", home.getAbsolutePath());
 
     // 3 start dummy app
-    File effectiveHome = props.fileOf("spaceHome");
+    File effectiveHome = props.nonNullValueAsFile("spaceHome");
 
     String cp = FilenameUtils.concat(new File(effectiveHome, "lib").getAbsolutePath(), "*");
     System.out.println("cp = " + cp);
@@ -115,4 +115,4 @@ public class ProcessWrapperTest extends ProcessTest {
     assertThat(process.isAlive()).isFalse();
     assertCanStart(process);
   }
-}
\ No newline at end of file
+}
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ConfigurationUtilsTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/ConfigurationUtilsTest.java
index 40585642954..de928b93850 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/ConfigurationUtilsTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/ConfigurationUtilsTest.java
@@ -76,7 +76,7 @@ public class ConfigurationUtilsTest {
     FileUtils.write(propsFile, "foo=bar");
 
     Props result = ConfigurationUtils.loadPropsFromCommandLineArgs(new String[] {propsFile.getAbsolutePath()});
-    assertThat(result.of("foo")).isEqualTo("bar");
+    assertThat(result.value("foo")).isEqualTo("bar");
     assertThat(result.rawProperties()).hasSize(1);
   }
 
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTestBase.java
index e10a158ed87..0980251cdc7 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTestBase.java
@@ -25,7 +25,7 @@ import org.junit.Test;
 
 import static org.fest.assertions.Assertions.assertThat;
 
-public class MonitorTest extends ProcessTest {
+public class MonitorTestBase extends BaseProcessTest {
 
 
   Monitor monitor;
@@ -75,4 +75,4 @@ public class MonitorTest extends ProcessTest {
     assertThat(monitor.isAlive()).isFalse();
     assertThat(process.isAlive()).isFalse();
   }
-}
\ No newline at end of file
+}
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/MonitoredProcessTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/MonitoredProcessTest.java
index ef0379925c8..c753bd00422 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/MonitoredProcessTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/MonitoredProcessTest.java
@@ -35,8 +35,8 @@ public class MonitoredProcessTest {
     try {
       new DummyProcess(new Props(properties), true);
       fail();
-    } catch (Exception e) {
-      assertThat(e.getMessage()).isEqualTo("Missing Name argument");
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage()).isEqualTo("Missing property: pName");
     }
 
     properties.setProperty(MonitoredProcess.NAME_PROPERTY, DummyProcess.NAME);
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/PropsTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/PropsTest.java
index 2af69ef1ca0..5d283b44f8f 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/PropsTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/PropsTest.java
@@ -34,10 +34,10 @@ public class PropsTest {
     p.setProperty("foo", "bar");
     Props props = new Props(p);
 
-    assertThat(props.of("foo")).isEqualTo("bar");
-    assertThat(props.of("foo", "default value")).isEqualTo("bar");
-    assertThat(props.of("unknown")).isNull();
-    assertThat(props.of("unknown", "default value")).isEqualTo("default value");
+    assertThat(props.value("foo")).isEqualTo("bar");
+    assertThat(props.value("foo", "default value")).isEqualTo("bar");
+    assertThat(props.value("unknown")).isNull();
+    assertThat(props.value("unknown", "default value")).isEqualTo("default value");
   }
 
   @Test
@@ -47,12 +47,12 @@ public class PropsTest {
     p.setProperty("blank", "");
     Props props = new Props(p);
 
-    assertThat(props.intOf("foo")).isEqualTo(33);
-    assertThat(props.intOf("foo", 44)).isEqualTo(33);
-    assertThat(props.intOf("blank")).isNull();
-    assertThat(props.intOf("blank", 55)).isEqualTo(55);
-    assertThat(props.intOf("unknown")).isNull();
-    assertThat(props.intOf("unknown", 44)).isEqualTo(44);
+    assertThat(props.valueAsInt("foo")).isEqualTo(33);
+    assertThat(props.valueAsInt("foo", 44)).isEqualTo(33);
+    assertThat(props.valueAsInt("blank")).isNull();
+    assertThat(props.valueAsInt("blank", 55)).isEqualTo(55);
+    assertThat(props.valueAsInt("unknown")).isNull();
+    assertThat(props.valueAsInt("unknown", 44)).isEqualTo(44);
   }
 
   @Test
@@ -62,7 +62,7 @@ public class PropsTest {
     Props props = new Props(p);
 
     try {
-      props.intOf("foo");
+      props.valueAsInt("foo");
       fail();
     } catch (IllegalStateException e) {
       assertThat(e).hasMessage("Value of property foo is not an integer: bar");
@@ -76,9 +76,9 @@ public class PropsTest {
     p.setProperty("bar", "false");
     Props props = new Props(p);
 
-    assertThat(props.booleanOf("foo")).isTrue();
-    assertThat(props.booleanOf("bar")).isFalse();
-    assertThat(props.booleanOf("unknown")).isFalse();
+    assertThat(props.valueAsBoolean("foo")).isTrue();
+    assertThat(props.valueAsBoolean("bar")).isFalse();
+    assertThat(props.valueAsBoolean("unknown")).isFalse();
   }
 
   @Test
@@ -88,10 +88,10 @@ public class PropsTest {
     p.setProperty("bar", "false");
     Props props = new Props(p);
 
-    assertThat(props.booleanOf("unset", false)).isFalse();
-    assertThat(props.booleanOf("unset", true)).isTrue();
-    assertThat(props.booleanOf("foo", false)).isTrue();
-    assertThat(props.booleanOf("bar", true)).isFalse();
+    assertThat(props.valueAsBoolean("unset", false)).isFalse();
+    assertThat(props.valueAsBoolean("unset", true)).isTrue();
+    assertThat(props.valueAsBoolean("foo", false)).isTrue();
+    assertThat(props.valueAsBoolean("bar", true)).isFalse();
   }
 
   @Test
@@ -102,9 +102,9 @@ public class PropsTest {
     props.setDefault("foo", "foo_def");
     props.setDefault("bar", "bar_def");
 
-    assertThat(props.of("foo")).isEqualTo("foo_value");
-    assertThat(props.of("bar")).isEqualTo("bar_def");
-    assertThat(props.of("other")).isNull();
+    assertThat(props.value("foo")).isEqualTo("foo_value");
+    assertThat(props.value("bar")).isEqualTo("bar_def");
+    assertThat(props.value("other")).isNull();
   }
 
   @Test
@@ -115,8 +115,8 @@ public class PropsTest {
     props.set("foo", "new_foo");
     props.set("bar", "new_bar");
 
-    assertThat(props.of("foo")).isEqualTo("new_foo");
-    assertThat(props.of("bar")).isEqualTo("new_bar");
+    assertThat(props.value("foo")).isEqualTo("new_foo");
+    assertThat(props.value("bar")).isEqualTo("new_bar");
   }
 
   @Test