From 8e8f9fc1cc8b9a113778027dfe9ac59c00d332d0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 12 May 2016 12:04:26 +0200 Subject: [PATCH] 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 --- .../org/sonar/process/ProcessProperties.java | 1 - .../org/sonar/application/JdbcSettings.java | 55 +++++- .../sonar/application/JdbcSettingsTest.java | 163 +++++++++++------- 3 files changed, 156 insertions(+), 63 deletions(-) diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java index b721895f88f..550a7014f0d 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java @@ -119,7 +119,6 @@ public class ProcessProperties { defaults.put(ProcessProperties.WEB_JAVA_ADDITIONAL_OPTS, ""); defaults.put(ProcessProperties.CE_JAVA_OPTS, "-Xmx512m -Xms128m -XX:MaxPermSize=160m -XX:+HeapDumpOnOutOfMemoryError -Djava.net.preferIPv4Stack=true"); defaults.put(ProcessProperties.CE_JAVA_ADDITIONAL_OPTS, ""); - defaults.put(ProcessProperties.JDBC_URL, "jdbc:h2:tcp://localhost:9092/sonar"); defaults.put(ProcessProperties.JDBC_MAX_ACTIVE, "60"); defaults.put(ProcessProperties.JDBC_MAX_IDLE, "5"); defaults.put(ProcessProperties.JDBC_MIN_IDLE, "2"); 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); } } -- 2.39.5