From 983beea417c9ffb0561857d0b7e69a97dfadc91d Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 11 May 2016 13:54:38 +0200 Subject: [PATCH] SONAR-7580 EmbeddedDatabase now assume jdbcurl is correctly configured checking the URL and applying default values is not the responsibility of JdbcSettings class alone --- .../org/sonar/server/db/EmbeddedDatabase.java | 43 ++++-- .../sonar/server/db/EmbeddedDatabaseTest.java | 146 ++++++++++++++---- 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"); + } } + } -- 2.39.5