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;
