Fix JDBC connection Avoid passing the name of the table in the JDBC URL while making sure connection options are honored. Change-Id: I05f43d06131068c5c82db0adf1ef8b94d52ddcac
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfig.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfig.java index 744d477..c60f4cc 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfig.java +++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfig.java
@@ -14,6 +14,7 @@ package com.ericsson.gerrit.plugins.eventslog; +import com.google.common.base.Joiner; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.PluginConfigFactory; @@ -81,12 +82,16 @@ storeUrl = cfg.getString(CONFIG_URL, DEFAULT_URL); localStorePath = Paths.get(cfg.getString(CONFIG_LOCAL_PATH, defaultLocalPath)); - urlOptions = cfg.getString(CONFIG_URL_OPTIONS, ""); + urlOptions = concatenate(cfg.getStringList(CONFIG_URL_OPTIONS)); storeUsername = cfg.getString(CONFIG_USERNAME); storePassword = cfg.getString(CONFIG_PASSWORD); evictIdleTime = cfg.getInt(CONFIG_EVICT_IDLE_TIME, DEFAULT_EVICT_IDLE_TIME); } + private String concatenate(String[] stringList) { + return Joiner.on(";").join(stringList); + } + public int getMaxAge() { return maxAge; }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java index 3aa3bc2..3f31a22 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java +++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java
@@ -53,7 +53,8 @@ SQLClient(String storeDriver, String storeUrl, String urlOptions) { ds = new BasicDataSource(); ds.setDriverClassName(storeDriver); - ds.setUrl(storeUrl + TABLE_NAME + ";" + urlOptions); + ds.setUrl(storeUrl); + ds.setConnectionProperties(urlOptions); } /** @@ -104,8 +105,7 @@ boolean dbExists() throws SQLException { try (Connection conn = ds.getConnection(); ResultSet tables = - conn.getMetaData().getTables(null, null, TABLE_NAME.toUpperCase(), - null)) { + conn.getMetaData().getTables(null, null, TABLE_NAME, null)) { return tables.next(); } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLModule.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLModule.java index 9020339..e8f0559 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLModule.java +++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLModule.java
@@ -57,7 +57,7 @@ String path = cfg.getLocalStorePath().toString(); path = path.endsWith("/") ? path : path + "/"; SQLClient sqlClient = new SQLClient(cfg.getLocalStoreDriver(), - H2_DB_PREFIX + path, cfg.getUrlOptions()); + H2_DB_PREFIX + path + SQLTable.TABLE_NAME, cfg.getUrlOptions()); sqlClient.setEvictIdleTime(cfg.getEvictIdleTime()); return sqlClient; }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 19299d3..749c600 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -9,7 +9,8 @@ maxAge = 20 returnLimit = 10000 storeUrl = jdbc:h2:~/gerrit/db/ - urlOptions = DB_CLOSE_DELAY=10 + urlOptions = loglevel=INFO + urlOptions = logUnclosedConnections=true copyLocal = true ``` @@ -43,7 +44,10 @@ also be defined in secure.config. plugin.@PLUGIN@.urlOptions -: Options to append to the database url. +: Options to append to the database url. Each option should be specified in a + separate line using the option=value format. For example: + urlOptions = loglevel=INFO + urlOptions = logUnclosedConnections=true plugin.@PLUGIN@.maxTries : Maximum number of times the plugin should attempt to store the event if a
diff --git a/src/test/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfigTest.java b/src/test/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfigTest.java index 461f197..d6ae725 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfigTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/eventslog/EventsLogConfigTest.java
@@ -38,6 +38,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.easymock.EasyMock.expect; +import com.google.common.base.Joiner; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.PluginConfigFactory; import com.google.gerrit.server.config.SitePaths; @@ -60,6 +61,7 @@ private EasyMockSupport easyMock; private String defaultLocalStorePath; private String localStorePath; + private String [] urlOptions = new String[]{"a=b", "c=d"}; private static final int CUSTOM_EVICT_IDLE_TIME = 10000; @@ -88,7 +90,7 @@ expect(configMock.getString(CONFIG_DRIVER, DEFAULT_DRIVER)).andReturn(DEFAULT_DRIVER); expect(configMock.getString(CONFIG_URL, DEFAULT_URL)).andReturn(DEFAULT_URL); expect(configMock.getString(CONFIG_LOCAL_PATH, defaultLocalStorePath)).andReturn(defaultLocalStorePath); - expect(configMock.getString(CONFIG_URL_OPTIONS, "")).andReturn(""); + expect(configMock.getStringList(CONFIG_URL_OPTIONS)).andReturn(new String[0]); expect(configMock.getString(CONFIG_USERNAME)).andReturn(null); expect(configMock.getString(CONFIG_PASSWORD)).andReturn(null); expect(configMock.getInt(CONFIG_EVICT_IDLE_TIME, DEFAULT_EVICT_IDLE_TIME)).andReturn(DEFAULT_EVICT_IDLE_TIME); @@ -107,7 +109,7 @@ expect(configMock.getString(CONFIG_DRIVER, DEFAULT_DRIVER)).andReturn("org.h2.Driver2"); expect(configMock.getString(CONFIG_URL, DEFAULT_URL)).andReturn("jdbc:h2:~/gerrit/db"); expect(configMock.getString(CONFIG_LOCAL_PATH, defaultLocalStorePath)).andReturn(localStorePath); - expect(configMock.getString(CONFIG_URL_OPTIONS, "")).andReturn("DB_CLOSE_DELAY=10"); + expect(configMock.getStringList(CONFIG_URL_OPTIONS)).andReturn(urlOptions); expect(configMock.getString(CONFIG_USERNAME)).andReturn("testUsername"); expect(configMock.getString(CONFIG_PASSWORD)).andReturn("testPassword"); expect(configMock.getInt(CONFIG_EVICT_IDLE_TIME, DEFAULT_EVICT_IDLE_TIME)).andReturn(CUSTOM_EVICT_IDLE_TIME); @@ -149,7 +151,7 @@ assertThat(config.getLocalStorePath().toString() + "/").isEqualTo(localStorePath); assertThat(config.getStoreDriver()).isEqualTo("org.h2.Driver2"); assertThat(config.getStoreUrl()).isEqualTo("jdbc:h2:~/gerrit/db"); - assertThat(config.getUrlOptions()).isEqualTo("DB_CLOSE_DELAY=10"); + assertThat(config.getUrlOptions()).isEqualTo(Joiner.on(";").join(urlOptions)); assertThat(config.getStoreUsername()).isEqualTo("testUsername"); assertThat(config.getStorePassword()).isEqualTo("testPassword"); assertThat(config.getEvictIdleTime()).isEqualTo(CUSTOM_EVICT_IDLE_TIME);
diff --git a/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLStoreTest.java b/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLStoreTest.java index 95e7692..dcfec06 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLStoreTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLStoreTest.java
@@ -59,10 +59,10 @@ public class SQLStoreTest { private static final Logger log = LoggerFactory.getLogger(SQLStoreTest.class); - private static final String TEST_URL = "jdbc:h2:mem:"; + private static final String TEST_URL = "jdbc:h2:mem:" + TABLE_NAME; private static final String TEST_LOCAL_URL = "jdbc:h2:mem:test:"; private static final String TEST_DRIVER = "org.h2.Driver"; - private static final String TEST_OPTIONS = "DB_CLOSE_DELAY=-1"; + private static final String TEST_OPTIONS = "DB_CLOSE_DELAY=-1;DATABASE_TO_UPPER=false"; private static final String TERM_CONN_MSG = "terminating connection"; private static final String MSG = "message"; private static final String GENERIC_QUERY = "SELECT * FROM " + TABLE_NAME; @@ -76,7 +76,7 @@ private SQLStore store; private ScheduledThreadPoolExecutor poolMock; - private String path = TEST_URL + TABLE_NAME + ";" + TEST_OPTIONS; + private String path = TEST_URL + ";" + TEST_OPTIONS; private Connection conn; private Statement stat; private List<SQLEntry> results;