SONAR-7580 jdbc_url default value is computed or set by JdbcSettings

it used to be hardcoded to jdbc:h2:tcp://localhost:9092/sonar, which did not take into account the optionaly specified embedded database port property

2 files changed, 156 insertions, 62 deletions

diff --git a/sonar-application/src/main/java/org/sonar/application/JdbcSettings.java b/sonar-application/src/main/java/org/sonar/application/JdbcSettings.java
index 00a9a87353e..2f00b97ebc6 100644
--- a/sonar-application/src/main/java/org/sonar/application/JdbcSettings.java
+++ b/sonar-application/src/main/java/org/sonar/application/JdbcSettings.java
@@ -26,11 +26,19 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
+import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.process.MessageException;
 import org.sonar.process.ProcessProperties;
 import org.sonar.process.Props;
 
+import static org.apache.commons.lang.StringUtils.isEmpty;
+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_EMBEDDED_PORT_DEFAULT_VALUE;
+import static org.sonar.api.database.DatabaseProperties.PROP_URL;
+import static org.sonar.process.ProcessProperties.JDBC_URL;
+
 public class JdbcSettings {
 
   enum Provider {
@@ -45,8 +53,8 @@ public class JdbcSettings {
   }
 
   public void checkAndComplete(File homeDir, Props props) {
-    String url = props.nonNullValue(ProcessProperties.JDBC_URL);
-    Provider provider = driverProvider(url);
+    Provider provider = resolveProviderAndEnforceNonnullJdbcUrl(props);
+    String url = props.value(JDBC_URL);
     checkUrlParameters(provider, url);
     String driverPath = driverPath(homeDir, provider);
     props.set(ProcessProperties.JDBC_DRIVER_PATH, driverPath);
@@ -68,7 +76,23 @@ public class JdbcSettings {
     return files.get(0).getAbsolutePath();
   }
 
-  Provider driverProvider(String url) {
+  Provider resolveProviderAndEnforceNonnullJdbcUrl(Props props) {
+    String url = props.value(JDBC_URL);
+    String embeddedDatabasePort = props.value(PROP_EMBEDDED_PORT);
+
+    if (isNotEmpty(embeddedDatabasePort)) {
+      String correctUrl = buildH2JdbcUrl(embeddedDatabasePort);
+      warnIfUrlIsSet(embeddedDatabasePort, url, correctUrl);
+      props.set(PROP_URL, correctUrl);
+      return Provider.H2;
+    }
+
+    if (isEmpty(url)) {
+      props.set(PROP_URL, buildH2JdbcUrl(PROP_EMBEDDED_PORT_DEFAULT_VALUE));
+      props.set(PROP_EMBEDDED_PORT, PROP_EMBEDDED_PORT_DEFAULT_VALUE);
+      return Provider.H2;
+    }
+
     Pattern pattern = Pattern.compile("jdbc:(\\w+):.+");
     Matcher matcher = pattern.matcher(url);
     if (!matcher.find()) {
@@ -82,6 +106,10 @@ public class JdbcSettings {
     }
   }
 
+  private static String buildH2JdbcUrl(String embeddedDatabasePort) {
+    return "jdbc:h2:tcp://localhost:" + embeddedDatabasePort + "/sonar";
+  }
+
   void checkUrlParameters(Provider provider, String url) {
     if (Provider.MYSQL.equals(provider)) {
       checkRequiredParameter(url, "useUnicode=true");
@@ -91,6 +119,27 @@ public class JdbcSettings {
     }
   }
 
+  private static void warnIfUrlIsSet(String port, String existing, String expectedUrl) {
+    if (isNotEmpty(existing)) {
+      Logger logger = LoggerFactory.getLogger(JdbcSettings.class);
+      if (expectedUrl.equals(existing)) {
+        logger.warn("To change H2 database port, only property '{}' should be set (which current value is '{}'). " +
+          "Remove property '{}' from configuration to remove this warning.",
+          PROP_EMBEDDED_PORT, port,
+          PROP_URL);
+      } else {
+        logger.warn("Both '{}' and '{}' properties are set. " +
+          "The value of property '{}' ('{}') is not consistent with the value of property '{}' ('{}'). " +
+          "The value of property '{}' will be ignored and value '{}' will be used instead. " +
+          "To remove this warning, either remove property '{}' if your intent was to use the embedded H2 database, otherwise remove property '{}'.",
+          PROP_EMBEDDED_PORT, PROP_URL,
+          PROP_URL, existing, PROP_EMBEDDED_PORT, port,
+          PROP_URL, expectedUrl,
+          PROP_URL, PROP_EMBEDDED_PORT);
+      }
+    }
+  }
+
   private static void checkRequiredParameter(String url, String val) {
     if (!url.contains(val)) {
       throw new MessageException(String.format("JDBC URL must have the property '%s'", val));
diff --git a/sonar-application/src/test/java/org/sonar/application/JdbcSettingsTest.java b/sonar-application/src/test/java/org/sonar/application/JdbcSettingsTest.java
index 0aee3e83b0d..1d101c41ff0 100644
--- a/sonar-application/src/test/java/org/sonar/application/JdbcSettingsTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/JdbcSettingsTest.java
@@ -19,110 +19,150 @@
  */
 package org.sonar.application;
 
+import java.io.File;
+import java.util.Properties;
 import org.apache.commons.io.FileUtils;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.process.MessageException;
 import org.sonar.process.ProcessProperties;
 import org.sonar.process.Props;
 
-import java.io.File;
-import java.util.Properties;
-
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.fail;
+import static org.sonar.application.JdbcSettings.Provider;
+import static org.sonar.process.ProcessProperties.JDBC_URL;
 
 public class JdbcSettingsTest {
 
   @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+  @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
   JdbcSettings settings = new JdbcSettings();
 
   @Test
-  public void driver_provider() {
-    assertThat(settings.driverProvider("jdbc:oracle:thin:@localhost/XE")).isEqualTo(JdbcSettings.Provider.ORACLE);
-    assertThat(settings.driverProvider("jdbc:mysql://localhost:3306/sonar?useUnicode=true&characterEncoding=utf8&rewriteBatchedStatements=true&useConfigs=maxPerformance"))
-      .isEqualTo(JdbcSettings.Provider.MYSQL);
-    try {
-      settings.driverProvider("jdbc:microsoft:sqlserver://localhost");
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("Unsupported JDBC driver provider: microsoft");
-    }
-    try {
-      settings.driverProvider("oracle:thin:@localhost/XE");
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("Bad format of JDBC URL: oracle:thin:@localhost/XE");
-    }
+  public void resolve_H2_provider_when_props_is_empty_and_set_URL_to_default_H2() {
+    Props props = newProps();
+
+    assertThat(settings.resolveProviderAndEnforceNonnullJdbcUrl(props))
+      .isEqualTo(Provider.H2);
+    assertThat(props.nonNullValue(JDBC_URL)).isEqualTo("jdbc:h2:tcp://localhost:9092/sonar");
+  }
+
+  @Test
+  public void resolve_Oracle_when_jdbc_url_contains_oracle_in_any_case() {
+    checkProviderForUrlAndUnchangedUrl("jdbc:oracle:foo", Provider.ORACLE);
+    checkProviderForUrlAndUnchangedUrl("jdbc:OrAcLe:foo", Provider.ORACLE);
+  }
+
+  @Test
+  public void resolve_MySql_when_jdbc_url_contains_mysql_in_any_case() {
+    checkProviderForUrlAndUnchangedUrl("jdbc:mysql:foo", Provider.MYSQL);
+
+    checkProviderForUrlAndUnchangedUrl("jdbc:mYsQL:foo", Provider.MYSQL);
+  }
+
+  @Test
+  public void resolve_SqlServer_when_jdbc_url_contains_sqlserver_in_any_case() {
+    checkProviderForUrlAndUnchangedUrl("jdbc:sqlserver:foo", Provider.SQLSERVER);
+
+    checkProviderForUrlAndUnchangedUrl("jdbc:SQLSeRVeR:foo", Provider.SQLSERVER);
+  }
+
+  @Test
+  public void resolve_POSTGRESQL_when_jdbc_url_contains_POSTGRESQL_in_any_case() {
+    checkProviderForUrlAndUnchangedUrl("jdbc:postgresql:foo", Provider.POSTGRESQL);
+
+    checkProviderForUrlAndUnchangedUrl("jdbc:POSTGRESQL:foo", Provider.POSTGRESQL);
+  }
+
+  private void checkProviderForUrlAndUnchangedUrl(String url, Provider expected) {
+    Props props = newProps(JDBC_URL, url);
+
+    assertThat(settings.resolveProviderAndEnforceNonnullJdbcUrl(props)).isEqualTo(expected);
+    assertThat(props.nonNullValue(JDBC_URL)).isEqualTo(url);
+  }
+
+  @Test
+  public void fail_with_MessageException_when_provider_is_not_supported() {
+    Props props = newProps(JDBC_URL, "jdbc:microsoft:sqlserver://localhost");
+
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("Unsupported JDBC driver provider: microsoft");
+
+    settings.resolveProviderAndEnforceNonnullJdbcUrl(props);
+  }
+
+  @Test
+  public void fail_with_MessageException_when_url_does_not_have_jdbc_prefix() {
+    Props props = newProps(JDBC_URL, "oracle:thin:@localhost/XE");
+
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("Bad format of JDBC URL: oracle:thin:@localhost/XE");
+
+    settings.resolveProviderAndEnforceNonnullJdbcUrl(props);
   }
 
   @Test
   public void check_mysql_parameters() {
     // minimal -> ok
-    settings.checkUrlParameters(JdbcSettings.Provider.MYSQL,
+    settings.checkUrlParameters(Provider.MYSQL,
       "jdbc:mysql://localhost:3306/sonar?useUnicode=true&characterEncoding=utf8");
 
     // full -> ok
-    settings.checkUrlParameters(JdbcSettings.Provider.MYSQL,
+    settings.checkUrlParameters(Provider.MYSQL,
       "jdbc:mysql://localhost:3306/sonar?useUnicode=true&characterEncoding=utf8&rewriteBatchedStatements=true&useConfigs=maxPerformance");
 
     // missing required -> ko
-    try {
-      settings.checkUrlParameters(JdbcSettings.Provider.MYSQL,
-        "jdbc:mysql://localhost:3306/sonar?characterEncoding=utf8");
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("JDBC URL must have the property 'useUnicode=true'");
-    }
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("JDBC URL must have the property 'useUnicode=true'");
+
+    settings.checkUrlParameters(Provider.MYSQL, "jdbc:mysql://localhost:3306/sonar?characterEncoding=utf8");
   }
 
   @Test
-  public void check_oracle() throws Exception {
+  public void checkAndComplete_sets_driver_path_for_oracle() throws Exception {
     File home = temp.newFolder();
     File driverFile = new File(home, "extensions/jdbc-driver/oracle/ojdbc6.jar");
     FileUtils.touch(driverFile);
 
-    Props props = new Props(new Properties());
-    props.set("sonar.jdbc.url", "jdbc:oracle:thin:@localhost/XE");
+    Props props = newProps(JDBC_URL, "jdbc:oracle:thin:@localhost/XE");
     settings.checkAndComplete(home, props);
     assertThat(props.nonNullValueAsFile(ProcessProperties.JDBC_DRIVER_PATH)).isEqualTo(driverFile);
   }
 
   @Test
-  public void check_h2() throws Exception {
+  public void checkAndComplete_sets_driver_path_for_h2() throws Exception {
     File home = temp.newFolder();
     File driverFile = new File(home, "lib/jdbc/h2/h2.jar");
     FileUtils.touch(driverFile);
 
-    Props props = new Props(new Properties());
-    props.set("sonar.jdbc.url", "jdbc:h2:tcp://localhost:9092/sonar");
+    Props props = newProps(JDBC_URL, "jdbc:h2:tcp://localhost:9092/sonar");
     settings.checkAndComplete(home, props);
     assertThat(props.nonNullValueAsFile(ProcessProperties.JDBC_DRIVER_PATH)).isEqualTo(driverFile);
   }
 
   @Test
-  public void check_postgresql() throws Exception {
+  public void checkAndComplete_sets_driver_path_for_postgresql() throws Exception {
     File home = temp.newFolder();
     File driverFile = new File(home, "lib/jdbc/postgresql/pg.jar");
     FileUtils.touch(driverFile);
 
-    Props props = new Props(new Properties());
-    props.set("sonar.jdbc.url", "jdbc:postgresql://localhost/sonar");
+    Props props = newProps(JDBC_URL, "jdbc:postgresql://localhost/sonar");
     settings.checkAndComplete(home, props);
     assertThat(props.nonNullValueAsFile(ProcessProperties.JDBC_DRIVER_PATH)).isEqualTo(driverFile);
   }
 
   @Test
-  public void check_mssql() throws Exception {
+  public void checkAndComplete_sets_driver_path_for_mssql() throws Exception {
     File home = temp.newFolder();
     File driverFile = new File(home, "lib/jdbc/mssql/sqljdbc4.jar");
     FileUtils.touch(driverFile);
 
-    Props props = new Props(new Properties());
-    props.set("sonar.jdbc.url", "jdbc:sqlserver://localhost/sonar;SelectMethod=Cursor");
+    Props props = newProps(JDBC_URL, "jdbc:sqlserver://localhost/sonar;SelectMethod=Cursor");
     settings.checkAndComplete(home, props);
     assertThat(props.nonNullValueAsFile(ProcessProperties.JDBC_DRIVER_PATH)).isEqualTo(driverFile);
   }
@@ -133,31 +173,29 @@ public class JdbcSettingsTest {
     File driverFile = new File(home, "extensions/jdbc-driver/oracle/ojdbc6.jar");
     FileUtils.touch(driverFile);
 
-    String path = settings.driverPath(home, JdbcSettings.Provider.ORACLE);
+    String path = settings.driverPath(home, Provider.ORACLE);
     assertThat(path).isEqualTo(driverFile.getAbsolutePath());
   }
 
   @Test
   public void driver_dir_does_not_exist() throws Exception {
     File home = temp.newFolder();
-    try {
-      settings.driverPath(home, JdbcSettings.Provider.ORACLE);
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("Directory does not exist: extensions/jdbc-driver/oracle");
-    }
+
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("Directory does not exist: extensions/jdbc-driver/oracle");
+
+    settings.driverPath(home, Provider.ORACLE);
   }
 
   @Test
   public void no_files_in_driver_dir() throws Exception {
     File home = temp.newFolder();
     FileUtils.forceMkdir(new File(home, "extensions/jdbc-driver/oracle"));
-    try {
-      settings.driverPath(home, JdbcSettings.Provider.ORACLE);
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("Directory does not contain JDBC driver: extensions/jdbc-driver/oracle");
-    }
+
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("Directory does not contain JDBC driver: extensions/jdbc-driver/oracle");
+
+    settings.driverPath(home, Provider.ORACLE);
   }
 
   @Test
@@ -166,11 +204,18 @@ public class JdbcSettingsTest {
     FileUtils.touch(new File(home, "extensions/jdbc-driver/oracle/ojdbc5.jar"));
     FileUtils.touch(new File(home, "extensions/jdbc-driver/oracle/ojdbc6.jar"));
 
-    try {
-      settings.driverPath(home, JdbcSettings.Provider.ORACLE);
-      fail();
-    } catch (MessageException e) {
-      assertThat(e).hasMessage("Directory must contain only one JAR file: extensions/jdbc-driver/oracle");
+    expectedException.expect(MessageException.class);
+    expectedException.expectMessage("Directory must contain only one JAR file: extensions/jdbc-driver/oracle");
+
+    settings.driverPath(home, Provider.ORACLE);
+  }
+
+  private Props newProps(String... params) {
+    Properties properties = new Properties();
+    for (int i = 0; i < params.length; i++) {
+      properties.setProperty(params[i], params[i + 1]);
+      i++;
     }
+    return new Props(properties);
   }
 }