From f84b0d1a765f6a3dcb8950a8605f924e244e7899 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 26 Aug 2014 13:48:33 +0200 Subject: [PATCH] Add missing tests in sonar-process module --- .../java/org/sonar/process/AesCipher.java | 6 +- .../org/sonar/process/ConfigurationUtils.java | 11 +--- .../sonar/process/MinimumViableSystem.java | 8 ++- .../org/sonar/process/MonitoredProcess.java | 7 +-- .../java/org/sonar/process/ProcessUtils.java | 6 ++ .../org/sonar/process/ProcessWrapper.java | 7 +-- .../main/java/org/sonar/process/Props.java | 9 --- .../sonar/process/ConfigurationUtilsTest.java | 60 +++++++++++++++---- .../process/MinimumViableSystemTest.java | 23 +++++++ .../sonar/process/MonitoredProcessTest.java | 11 +--- .../org/sonar/process/ProcessUtilsTest.java | 8 ++- .../java/org/sonar/process/PropsTest.java | 39 ++++++++++++ .../main/java/org/sonar/application/App.java | 5 -- 13 files changed, 140 insertions(+), 60 deletions(-) diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/AesCipher.java b/server/process/sonar-process/src/main/java/org/sonar/process/AesCipher.java index 1ea932a3d9b..11d69a805c1 100644 --- a/server/process/sonar-process/src/main/java/org/sonar/process/AesCipher.java +++ b/server/process/sonar-process/src/main/java/org/sonar/process/AesCipher.java @@ -97,7 +97,7 @@ final class AesCipher implements Cipher { return loadSecretFileFromFile(path); } - Key loadSecretFileFromFile(@Nullable String path) throws IOException { + Key loadSecretFileFromFile(String path) throws IOException { if (StringUtils.isBlank(path)) { throw new IllegalStateException("Secret key not found. Please set the property " + ENCRYPTION_SECRET_KEY_PATH); } @@ -130,8 +130,4 @@ final class AesCipher implements Cipher { } return pathToSecretKey; } - - public void setPathToSecretKey(@Nullable String pathToSecretKey) { - this.pathToSecretKey = pathToSecretKey; - } } diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ConfigurationUtils.java b/server/process/sonar-process/src/main/java/org/sonar/process/ConfigurationUtils.java index 9d2cfd3fa74..355b6f885a2 100644 --- a/server/process/sonar-process/src/main/java/org/sonar/process/ConfigurationUtils.java +++ b/server/process/sonar-process/src/main/java/org/sonar/process/ConfigurationUtils.java @@ -25,7 +25,6 @@ import org.apache.commons.lang.text.StrSubstitutor; import java.io.File; import java.io.FileReader; -import java.io.IOException; import java.util.Enumeration; import java.util.Map; import java.util.Properties; @@ -50,22 +49,18 @@ public final class ConfigurationUtils { public static Props loadPropsFromCommandLineArgs(String[] args) { if (args.length != 1) { - throw new IllegalStateException("Only a single command-line argument is accepted " + + throw new IllegalArgumentException("Only a single command-line argument is accepted " + "(absolute path to configuration file)"); } File propertyFile = new File(args[0]); - if (!propertyFile.exists()) { - throw new IllegalStateException("Property file '" + args[0] + "' does not exist! "); - } - Properties properties = new Properties(); FileReader reader = null; try { reader = new FileReader(propertyFile); properties.load(reader); - } catch (IOException e) { - throw new IllegalStateException("Could not read properties from file '" + args[0] + "'", e); + } catch (Exception e) { + throw new IllegalStateException("Could not read properties from file: " + args[0], e); } finally { IOUtils.closeQuietly(reader); FileUtils.deleteQuietly(propertyFile); diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/MinimumViableSystem.java b/server/process/sonar-process/src/main/java/org/sonar/process/MinimumViableSystem.java index 591be4198fd..2389fa5aa18 100644 --- a/server/process/sonar-process/src/main/java/org/sonar/process/MinimumViableSystem.java +++ b/server/process/sonar-process/src/main/java/org/sonar/process/MinimumViableSystem.java @@ -49,13 +49,15 @@ public class MinimumViableSystem { * Verify that temp directory is writable */ private void checkWritableTempDir() { - String tempPath = System.getProperty("java.io.tmpdir"); + checkWritableDir(System.getProperty("java.io.tmpdir")); + } + + void checkWritableDir(String tempPath) { try { File tempFile = File.createTempFile("check", "tmp", new File(tempPath)); FileUtils.deleteQuietly(tempFile); } catch (IOException e) { - throw new MessageException(String.format( - "Temp directory is not writable: %s. Reason: %s", tempPath, e.getMessage())); + throw new IllegalStateException(String.format("Temp directory is not writable: %s", tempPath), e); } } 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 4a6c3ec5e51..0724ef42258 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 @@ -32,7 +32,6 @@ public abstract class MonitoredProcess implements ProcessMXBean { private final static Logger LOGGER = LoggerFactory.getLogger(MonitoredProcess.class); - public static final String DEBUG_AGENT = "-agentlib:jdwp"; private static final long AUTOKILL_TIMEOUT_MS = 30000L; private static final long AUTOKILL_CHECK_DELAY_MS = 2000L; public static final String NAME_PROPERTY = "pName"; @@ -51,7 +50,7 @@ public abstract class MonitoredProcess implements ProcessMXBean { private final boolean isMonitored; protected MonitoredProcess(Props props) { - this(props, !props.containsValue(DEBUG_AGENT)); + this(props, !ProcessUtils.isJvmDebugEnabled()); } protected MonitoredProcess(Props props, boolean monitor) { @@ -91,7 +90,7 @@ public abstract class MonitoredProcess implements ProcessMXBean { throw new IllegalStateException("Already started"); } LOGGER.debug("Process[{}] starting", name); - scheduleAutokill(this.isMonitored); + scheduleAutokill(isMonitored); try { doStart(); } catch (Exception e) { @@ -105,7 +104,7 @@ public abstract class MonitoredProcess implements ProcessMXBean { * If the process does not receive pings during the max allowed period, then * process auto-kills */ - private void scheduleAutokill(final Boolean isMonitored) { + private void scheduleAutokill(final boolean isMonitored) { final Runnable breakOnMissingPing = new Runnable() { @Override public void run() { diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java index a762446ec87..f399318aa1e 100644 --- a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java +++ b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java @@ -25,6 +25,8 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nullable; +import java.lang.management.ManagementFactory; + public class ProcessUtils { private static final Logger LOGGER = LoggerFactory.getLogger(ProcessUtils.class); @@ -73,4 +75,8 @@ public class ProcessUtils { IOUtils.closeQuietly(process.getErrorStream()); } } + + public static boolean isJvmDebugEnabled() { + return ManagementFactory.getRuntimeMXBean().getInputArguments().toString().indexOf("-agentlib:jdwp") > 0; + } } diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java index 12832151900..31ede432301 100644 --- a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java +++ b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java @@ -58,7 +58,7 @@ import java.util.concurrent.TimeUnit; */ public class ProcessWrapper extends Thread implements Terminable { - private final static Logger LOGGER = LoggerFactory.getLogger(ProcessWrapper.class); + private static final Logger LOGGER = LoggerFactory.getLogger(ProcessWrapper.class); public static final long READY_TIMEOUT_MS = 300000L; @@ -280,7 +280,7 @@ public class ProcessWrapper extends Thread implements Terminable { } private String localAddress() { - // TODO to be replaced by InetAddress.getLoopbackAddress() in Java 7 + // to be replaced by InetAddress.getLoopbackAddress() in Java 7 ? return "127.0.0.1"; } @@ -318,9 +318,6 @@ public class ProcessWrapper extends Thread implements Terminable { } public boolean waitForReady() throws InterruptedException { - if (processMXBean == null) { - return false; - } long now = 0; long wait = 500L; while (now < READY_TIMEOUT_MS) { 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 f8112ea6423..d6e2795a597 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 @@ -40,15 +40,6 @@ public class Props { return properties.containsKey(key); } - public boolean containsValue(String value) { - for (Object propertyValue : properties.values()) { - if (propertyValue.toString().contains(value)) { - return true; - } - } - return false; - } - @CheckForNull public String of(String key) { String value = properties.getProperty(key); 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 6191eb2ba1f..40585642954 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 @@ -20,15 +20,23 @@ package org.sonar.process; import com.google.common.collect.Maps; +import org.apache.commons.io.FileUtils; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.File; import java.util.Map; import java.util.Properties; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; public class ConfigurationUtilsTest { + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + @Test public void shouldInterpolateVariables() { Properties input = new Properties(); @@ -40,16 +48,48 @@ public class ConfigurationUtilsTest { Properties output = ConfigurationUtils.interpolateVariables(input, variables); - assertThat(output.size(), is(3)); - assertThat(output.getProperty("hello"), is("world")); - assertThat(output.getProperty("url"), is("jdbc:h2:mem")); - assertThat(output.getProperty("do_not_change"), is("${SONAR_JDBC_URL}")); + assertThat(output).hasSize(3); + assertThat(output.getProperty("hello")).isEqualTo("world"); + assertThat(output.getProperty("url")).isEqualTo("jdbc:h2:mem"); + assertThat(output.getProperty("do_not_change")).isEqualTo("${SONAR_JDBC_URL}"); // input is not changed - assertThat(input.size(), is(3)); - assertThat(input.getProperty("hello"), is("world")); - assertThat(input.getProperty("url"), is("${env:SONAR_JDBC_URL}")); - assertThat(input.getProperty("do_not_change"), is("${SONAR_JDBC_URL}")); + assertThat(input).hasSize(3); + assertThat(input.getProperty("hello")).isEqualTo("world"); + assertThat(input.getProperty("url")).isEqualTo("${env:SONAR_JDBC_URL}"); + assertThat(input.getProperty("do_not_change")).isEqualTo("${SONAR_JDBC_URL}"); + } + + @Test + public void loadPropsFromCommandLineArgs_missing_argument() throws Exception { + try { + ConfigurationUtils.loadPropsFromCommandLineArgs(new String[0]); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage()).startsWith("Only a single command-line argument is accepted"); + } + } + + @Test + public void loadPropsFromCommandLineArgs_load_properties_from_file() throws Exception { + File propsFile = temp.newFile(); + FileUtils.write(propsFile, "foo=bar"); + + Props result = ConfigurationUtils.loadPropsFromCommandLineArgs(new String[] {propsFile.getAbsolutePath()}); + assertThat(result.of("foo")).isEqualTo("bar"); + assertThat(result.rawProperties()).hasSize(1); } + @Test + public void loadPropsFromCommandLineArgs_file_does_not_exist() throws Exception { + File propsFile = temp.newFile(); + FileUtils.deleteQuietly(propsFile); + + try { + ConfigurationUtils.loadPropsFromCommandLineArgs(new String[]{propsFile.getAbsolutePath()}); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessage("Could not read properties from file: " + propsFile.getAbsolutePath()); + } + } } diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/MinimumViableSystemTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/MinimumViableSystemTest.java index a1bc83477bb..83841c79952 100644 --- a/server/process/sonar-process/src/test/java/org/sonar/process/MinimumViableSystemTest.java +++ b/server/process/sonar-process/src/test/java/org/sonar/process/MinimumViableSystemTest.java @@ -20,12 +20,19 @@ package org.sonar.process; import org.fest.assertions.Assertions; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; import static org.fest.assertions.Fail.fail; public class MinimumViableSystemTest { + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + /** * Verifies that all checks can be verified without error. * Test environment does not necessarily follows all checks. @@ -76,4 +83,20 @@ public class MinimumViableSystemTest { mve.checkJavaOptions(); // do not fail } + + @Test + public void checkWritableTempDir() throws Exception { + File dir = temp.newFolder(); + MinimumViableSystem mve = new MinimumViableSystem(); + + mve.checkWritableDir(dir.getAbsolutePath()); + + dir.delete(); + try { + mve.checkWritableDir(dir.getAbsolutePath()); + fail(); + } catch (IllegalStateException e) { + Assertions.assertThat(e).hasMessage("Temp directory is not writable: " + dir.getAbsolutePath()); + } + } } 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 e8f87147654..ef0379925c8 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 @@ -48,8 +48,7 @@ public class MonitoredProcessTest { public void should_not_monitor_debug() throws Exception { Properties properties = new Properties(); properties.setProperty(MonitoredProcess.NAME_PROPERTY, DummyProcess.NAME); - properties.setProperty("sonar.search.javaOpts", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005"); - DummyProcess dummyProcess = new DummyProcess(new Props(properties)); + DummyProcess dummyProcess = new DummyProcess(new Props(properties), false); assertThat(dummyProcess.isMonitored()).isFalse(); } @@ -64,7 +63,6 @@ public class MonitoredProcessTest { assertThat(dummyProcess.isMonitored()).isTrue(); } - @Test(timeout = 3000L) public void monitor_dies_when_no_pings() throws Exception { Properties properties = new Properties(); @@ -139,9 +137,8 @@ public class MonitoredProcessTest { public void process_does_not_die_when_debugged() throws Exception { Properties properties = new Properties(); properties.setProperty(MonitoredProcess.NAME_PROPERTY, DummyProcess.NAME); - properties.setProperty("sonar.search.javaOpts", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005"); - final DummyProcess dummyProcess = new DummyProcess(new Props(properties)); + final DummyProcess dummyProcess = new DummyProcess(new Props(properties), false); assertThat(dummyProcess.isMonitored()).isFalse(); dummyProcess.setTimeout(100L).setCheckDelay(100L); @@ -159,7 +156,6 @@ public class MonitoredProcessTest { assertProcessTerminated(dummyProcess); } - private void assertJoinAndTerminate(DummyProcess dummyProcess, Thread process) throws InterruptedException { process.join(); assertProcessTerminated(dummyProcess); @@ -181,10 +177,9 @@ public class MonitoredProcessTest { assertThat(dummyProcess.isTerminated()).isFalse(); } - private void assertProcessCreatedFile(DummyProcess dummyProcess) { assertThat(dummyProcess.getCheckFile()).isNotNull(); assertThat(dummyProcess.getCheckFile().getName()).isEqualTo(DummyProcess.CHECKFILE_NAME); } -} \ No newline at end of file +} diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java index 09ddf79ba5f..e508f5a57f9 100644 --- a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java +++ b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java @@ -21,10 +21,12 @@ package org.sonar.process; import org.junit.Test; +import static org.fest.assertions.Assertions.assertThat; + public class ProcessUtilsTest { @Test - public void check_process_alive() { - ProcessBuilder processBuilder = new ProcessBuilder(); + public void isJvmDebugEnabled() { + assertThat(ProcessUtils.isJvmDebugEnabled()).isFalse(); } -} \ No newline at end of file +} 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 775d24a2a64..2af69ef1ca0 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 @@ -93,4 +93,43 @@ public class PropsTest { assertThat(props.booleanOf("foo", false)).isTrue(); assertThat(props.booleanOf("bar", true)).isFalse(); } + + @Test + public void setDefault() throws Exception { + Properties p = new Properties(); + p.setProperty("foo", "foo_value"); + Props props = new Props(p); + 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(); + } + + @Test + public void set() throws Exception { + Properties p = new Properties(); + p.setProperty("foo", "old_foo"); + Props props = new Props(p); + props.set("foo", "new_foo"); + props.set("bar", "new_bar"); + + assertThat(props.of("foo")).isEqualTo("new_foo"); + assertThat(props.of("bar")).isEqualTo("new_bar"); + } + + @Test + public void raw_properties() throws Exception { + Properties p = new Properties(); + p.setProperty("encrypted_prop", "{aes}abcde"); + p.setProperty("clear_prop", "foo"); + Props props = new Props(p); + + assertThat(props.rawProperties()).hasSize(2); + // do not decrypt + assertThat(props.rawProperties().get("encrypted_prop")).isEqualTo("{aes}abcde"); + assertThat(props.rawProperties().get("clear_prop")).isEqualTo("foo"); + + } } diff --git a/sonar-application/src/main/java/org/sonar/application/App.java b/sonar-application/src/main/java/org/sonar/application/App.java index 67efece6734..43c192bf716 100644 --- a/sonar-application/src/main/java/org/sonar/application/App.java +++ b/sonar-application/src/main/java/org/sonar/application/App.java @@ -58,11 +58,6 @@ public class App implements ProcessMXBean { try { Logger logger = LoggerFactory.getLogger(getClass()); - if (props.containsValue(MonitoredProcess.DEBUG_AGENT)) { - logger.info("**********************************************************"); - logger.info("* sonarQube is running in debug mode. No monitoring *"); - logger.info("**********************************************************"); - } monitor.start(); File homeDir = props.fileOf("sonar.path.home"); -- 2.39.5