SONAR-7580 EmbeddedDatabase now assume jdbcurl is correctly configured

checking the URL and applying default values is not the responsibility of JdbcSettings class alone

2 files changed, 142 insertions, 47 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/db/EmbeddedDatabase.java b/server/sonar-server/src/main/java/org/sonar/server/db/EmbeddedDatabase.java
index 8867fe3f390..505d323367a 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/db/EmbeddedDatabase.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/db/EmbeddedDatabase.java
@@ -19,21 +19,28 @@
  */
 package org.sonar.server.db;
 
-import com.google.common.annotations.VisibleForTesting;
+import java.io.File;
+import java.sql.DriverManager;
+import java.sql.SQLException;
 import org.apache.commons.lang.StringUtils;
 import org.h2.Driver;
 import org.h2.tools.Server;
 import org.picocontainer.Startable;
 import org.sonar.api.config.Settings;
-import org.sonar.api.database.DatabaseProperties;
 import org.sonar.api.utils.SonarException;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
-import org.sonar.process.ProcessProperties;
 
-import java.io.File;
-import java.sql.DriverManager;
-import java.sql.SQLException;
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.String.format;
+import static org.apache.commons.lang.StringUtils.isNotEmpty;
+import static org.sonar.api.database.DatabaseProperties.PROP_EMBEDDED_PORT;
+import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD;
+import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD_DEFAULT_VALUE;
+import static org.sonar.api.database.DatabaseProperties.PROP_URL;
+import static org.sonar.api.database.DatabaseProperties.PROP_USER;
+import static org.sonar.api.database.DatabaseProperties.PROP_USER_DEFAULT_VALUE;
+import static org.sonar.process.ProcessProperties.PATH_DATA;
 
 public class EmbeddedDatabase implements Startable {
   private static final Logger LOG = Loggers.get(EmbeddedDatabase.class);
@@ -46,16 +53,19 @@ public class EmbeddedDatabase implements Startable {
 
   @Override
   public void start() {
-    File dbHome = getDataDirectory(settings);
+    File dbHome = new File(getRequiredSetting(PATH_DATA));
     if (!dbHome.exists()) {
       dbHome.mkdirs();
     }
 
-    String port = getSetting(DatabaseProperties.PROP_EMBEDDED_PORT, DatabaseProperties.PROP_EMBEDDED_PORT_DEFAULT_VALUE);
-    String user = getSetting(DatabaseProperties.PROP_USER, DatabaseProperties.PROP_USER_DEFAULT_VALUE);
-    String password = getSetting(DatabaseProperties.PROP_PASSWORD, DatabaseProperties.PROP_PASSWORD_DEFAULT_VALUE);
-    String url = getSetting(DatabaseProperties.PROP_URL, DatabaseProperties.PROP_USER_DEFAULT_VALUE);
+    startServer(dbHome);
+  }
 
+  private void startServer(File dbHome) {
+    String url = getRequiredSetting(PROP_URL);
+    String port = getRequiredSetting(PROP_EMBEDDED_PORT);
+    String user = getSetting(PROP_USER, PROP_USER_DEFAULT_VALUE);
+    String password = getSetting(PROP_PASSWORD, PROP_PASSWORD_DEFAULT_VALUE);
     try {
       if (url.contains("/mem:")) {
         server = Server.createTcpServer("-tcpPort", port, "-tcpAllowOthers", "-baseDir", dbHome.getAbsolutePath());
@@ -82,17 +92,18 @@ public class EmbeddedDatabase implements Startable {
     }
   }
 
-  @VisibleForTesting
-  File getDataDirectory(Settings settings) {
-    return new File(settings.getString(ProcessProperties.PATH_DATA));
+  private String getRequiredSetting(String property) {
+    String value = settings.getString(property);
+    checkArgument(isNotEmpty(value), "Missing property %s", property);
+    return value;
   }
 
   private String getSetting(String name, String defaultValue) {
     return StringUtils.defaultIfBlank(settings.getString(name), defaultValue);
   }
 
-  private void createDatabase(File dbHome, String user, String password) throws SQLException {
-    String url = String.format("jdbc:h2:%s/sonar;USER=%s;PASSWORD=%s", dbHome.getAbsolutePath(), user, password);
+  private static void createDatabase(File dbHome, String user, String password) throws SQLException {
+    String url = format("jdbc:h2:%s/sonar;USER=%s;PASSWORD=%s", dbHome.getAbsolutePath(), user, password);
 
     DriverManager.registerDriver(new Driver());
     DriverManager.getConnection(url).close();
diff --git a/server/sonar-server/src/test/java/org/sonar/server/db/EmbeddedDatabaseTest.java b/server/sonar-server/src/test/java/org/sonar/server/db/EmbeddedDatabaseTest.java
index adbfe3f672f..e88c71567ec 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/db/EmbeddedDatabaseTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/db/EmbeddedDatabaseTest.java
@@ -19,59 +19,143 @@
  */
 package org.sonar.server.db;
 
+import java.io.IOException;
+import java.sql.DriverManager;
 import org.h2.Driver;
+import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.Timeout;
 import org.sonar.api.config.Settings;
-import org.sonar.api.database.DatabaseProperties;
+import org.sonar.api.utils.log.LogTester;
 import org.sonar.process.NetworkUtils;
-import org.sonar.process.ProcessProperties;
-
-import java.io.File;
-import java.sql.DriverManager;
 
 import static junit.framework.Assert.fail;
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.api.database.DatabaseProperties.PROP_EMBEDDED_PORT;
+import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD;
+import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD_DEFAULT_VALUE;
+import static org.sonar.api.database.DatabaseProperties.PROP_URL;
+import static org.sonar.api.database.DatabaseProperties.PROP_USER;
+import static org.sonar.api.database.DatabaseProperties.PROP_USER_DEFAULT_VALUE;
+import static org.sonar.process.ProcessProperties.PATH_DATA;
 
 public class EmbeddedDatabaseTest {
 
   @Rule
-  public ExpectedException throwable = ExpectedException.none();
+  public ExpectedException expectedException = ExpectedException.none();
+  @Rule
+  public LogTester logTester = new LogTester();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+  @Rule
+  public Timeout timeout = Timeout.seconds(10);
+
+  private EmbeddedDatabase underTest;
+
+  @After
+  public void tearDown() throws Exception {
+    if (underTest != null) {
+      underTest.stop();
+    }
+  }
+
+  @Test
+  public void start_fails_with_IAE_if_property_Data_Path_is_not_set() {
+    EmbeddedDatabase underTest = new EmbeddedDatabase(new Settings());
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Missing property " + PATH_DATA);
 
-  @Test(timeout = 10000)
-  public void should_start_and_stop() {
+    underTest.start();
+  }
+
+  @Test
+  public void start_fails_with_IAE_if_property_Data_Path_is_empty() {
+    EmbeddedDatabase underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, ""));
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Missing property " + PATH_DATA);
+
+    underTest.start();
+  }
+
+  @Test
+  public void start_fails_with_IAE_if_JDBC_URL_settings_is_not_set() throws IOException {
+    EmbeddedDatabase underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()));
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Missing property " + PROP_URL);
+
+    underTest.start();
+  }
+
+  @Test
+  public void start_fails_with_IAE_if_embedded_port_settings_is_not_set() throws IOException {
+    EmbeddedDatabase underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath())
+      .setProperty(PROP_URL, "jdbc url"));
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Missing property " + PROP_EMBEDDED_PORT);
+
+    underTest.start();
+  }
+
+  @Test
+  public void start_ignores_URL_to_create_database_and_uses_default_username_and_password_when_then_are_not_set() throws IOException {
     int port = NetworkUtils.freePort();
+    underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath())
+      .setProperty(PROP_URL, "jdbc url")
+      .setProperty(PROP_EMBEDDED_PORT, "" + port));
 
-    EmbeddedDatabase database = new EmbeddedDatabase(testSettings(port));
-    database.start();
+    underTest.start();
 
-    try {
-      String driverUrl = String.format("jdbc:h2:tcp://localhost:%d/sonar;USER=login;PASSWORD=pwd", port);
-      DriverManager.registerDriver(new Driver());
-      DriverManager.getConnection(driverUrl).close();
-    } catch (Exception ex) {
-      fail("Unable to connect after start");
-    }
+    checkDbIsUp(port, PROP_USER_DEFAULT_VALUE, PROP_PASSWORD_DEFAULT_VALUE);
+  }
 
-    database.stop();
+  @Test
+  public void start_creates_db_with_specified_user_and_password() throws IOException {
+    int port = NetworkUtils.freePort();
+    underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath())
+      .setProperty(PROP_URL, "jdbc url")
+      .setProperty(PROP_EMBEDDED_PORT, "" + port)
+      .setProperty(PROP_USER, "foo")
+      .setProperty(PROP_PASSWORD, "bar"));
+
+    underTest.start();
+
+    checkDbIsUp(port, "foo", "bar");
   }
 
   @Test
-  public void should_return_embedded_data_directory() {
-    Settings settings = testSettings(0);
-    EmbeddedDatabase database = new EmbeddedDatabase(settings);
+  public void start_supports_in_memory_H2_JDBC_URL() throws IOException {
+    int port = NetworkUtils.freePort();
+    underTest = new EmbeddedDatabase(new Settings()
+      .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath())
+      .setProperty(PROP_URL, "jdbc:h2:mem:sonar")
+      .setProperty(PROP_EMBEDDED_PORT, "" + port)
+      .setProperty(PROP_USER, "foo")
+      .setProperty(PROP_PASSWORD, "bar"));
+
+    underTest.start();
 
-    File dataDirectory = database.getDataDirectory(settings);
-    assertThat(dataDirectory).isNotNull();
-    assertThat(dataDirectory.getPath()).endsWith("testDB");
+    checkDbIsUp(port, "foo", "bar");
   }
 
-  static Settings testSettings(int port) {
-    return new Settings()
-      .setProperty(DatabaseProperties.PROP_USER, "login")
-      .setProperty(DatabaseProperties.PROP_PASSWORD, "pwd")
-      .setProperty(DatabaseProperties.PROP_EMBEDDED_PORT, "" + port)
-      .setProperty(ProcessProperties.PATH_DATA, "./target/testDB");
+  private void checkDbIsUp(int port, String user, String password) {
+    try {
+      String driverUrl = String.format("jdbc:h2:tcp://localhost:%d/sonar;USER=%s;PASSWORD=%s", port, user, password);
+      DriverManager.registerDriver(new Driver());
+      DriverManager.getConnection(driverUrl).close();
+    } catch (Exception ex) {
+      fail("Unable to connect after start");
+    }
   }
+
 }