From d8fa7da22d09e78c17c8453d9dc30a80dc5c2389 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Fri, 17 Nov 2023 14:46:53 +1300 Subject: [PATCH] Use Driver rather than DriverManager --- .../ebean/datasource/pool/ConnectionPool.java | 49 ++++--------------- .../ebean/datasource/pool/ObtainDriver.java | 44 +++++++++++++++++ .../datasource/pool/ObtainDriverTest.java | 34 +++++++++++++ 3 files changed, 88 insertions(+), 39 deletions(-) create mode 100644 ebean-datasource/src/main/java/io/ebean/datasource/pool/ObtainDriver.java create mode 100644 ebean-datasource/src/test/java/io/ebean/datasource/pool/ObtainDriverTest.java diff --git a/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java index 28ca0b5..4eddb95 100644 --- a/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java +++ b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ConnectionPool.java @@ -41,7 +41,6 @@ final class ConnectionPool implements DataSourcePool { private final DataSourcePoolListener poolListener; private final Properties connectionProps; private final List initSql; - private final String driver; private final String url; private final String user; private final String schema; @@ -63,6 +62,7 @@ final class ConnectionPool implements DataSourcePool { private final int maxStackTraceSize; private final Properties clientInfo; private final String applicationName; + private final Driver driver; private long lastTrimTime; /** * HeartBeat checking will discover when it goes down, and comes back up again. @@ -105,7 +105,6 @@ final class ConnectionPool implements DataSourcePool { this.captureStackTrace = params.isCaptureStackTrace(); this.maxStackTraceSize = params.getMaxStackTraceSize(); this.url = params.getUrl(); - this.driver = params.getDriver(); this.pstmtCacheSize = params.getPstmtCacheSize(); this.minConnections = params.getMinConnections(); this.maxConnections = params.getMaxConnections(); @@ -139,7 +138,7 @@ final class ConnectionPool implements DataSourcePool { this.connectionProps.setProperty(entry.getKey(), entry.getValue()); } } - checkDriver(); + this.driver = ObtainDriver.driver(params.getDriver(), url); if (!params.isOffline()) { init(); } @@ -156,31 +155,6 @@ private void init() { } } - /** - * Return true if driver has been explicitly configured. - */ - private boolean hasDriver() { - return driver != null && !driver.isEmpty(); - } - - /** - * Check driver exists when explicitly set. - */ - private void checkDriver() { - if (hasDriver()) { - try { - ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); - if (contextLoader != null) { - Class.forName(driver, true, contextLoader); - } else { - Class.forName(driver, true, this.getClass().getClassLoader()); - } - } catch (Throwable e) { - throw new IllegalStateException("Problem loading Database Driver [" + driver + "]: " + e.getMessage(), e); - } - } - } - /** * Accumulate the prepared statement cache metrics as connections are closed. */ @@ -420,7 +394,7 @@ private void checkDataSource() { /** * Initializes the connection we got from the driver. */ - private void initConnection(Connection conn) throws SQLException { + private Connection initConnection(Connection conn) throws SQLException { conn.setAutoCommit(autoCommit); // isolation level is set globally for all connections (at least for H2) and // you will need admin rights - so we do not change it, if it already matches. @@ -454,6 +428,7 @@ private void initConnection(Connection conn) throws SQLException { } } } + return conn; } /** @@ -472,9 +447,7 @@ private Connection createConnection() throws SQLException { private Connection createConnection(Properties properties, boolean notifyIsDown) throws SQLException { try { - final var conn = newConnection(properties); - initConnection(conn); - return conn; + return initConnection(newConnection(properties)); } catch (SQLException ex) { if (notifyIsDown) { notifyDataSourceIsDown(null); @@ -485,7 +458,7 @@ private Connection createConnection(Properties properties, boolean notifyIsDown) private Connection newConnection(Properties properties) throws SQLException { try { - return DriverManager.getConnection(url, properties); + return driver.connect(url, properties); } catch (SQLException e) { notifyLock.lock(); try { @@ -503,7 +476,7 @@ private Connection newConnection(Properties properties) throws SQLException { private Connection switchCredentials(Properties properties) throws SQLException { var copy = new Properties(properties); copy.setProperty("password", password2); - var connection = DriverManager.getConnection(url, copy); + var connection = driver.connect(url, copy); // success, permanently switch to use password2 from now on Log.info("DataSource [{0}] now using alternate credentials", name); fixedCredentials = true; @@ -817,13 +790,11 @@ int pstmtCacheSize() { */ @Override public Connection getConnection(String username, String password) throws SQLException { - Properties props = new Properties(); - props.putAll(connectionProps); + final var props = new Properties(connectionProps); props.setProperty("user", username); props.setProperty("password", password); - Connection conn = DriverManager.getConnection(url, props); - initConnection(conn); - return conn; + final var connection = driver.connect(url, props); + return initConnection(connection); } /** diff --git a/ebean-datasource/src/main/java/io/ebean/datasource/pool/ObtainDriver.java b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ObtainDriver.java new file mode 100644 index 0000000..b52bedc --- /dev/null +++ b/ebean-datasource/src/main/java/io/ebean/datasource/pool/ObtainDriver.java @@ -0,0 +1,44 @@ +package io.ebean.datasource.pool; + +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.SQLException; + +final class ObtainDriver { + + /** + * Return the jdbc Driver to use. + */ + static Driver driver(String driver, String url) { + if (driver != null && !driver.isEmpty()) { + return initDriver(driver); + } + try { + return DriverManager.getDriver(url); + } catch (SQLException e) { + throw new IllegalStateException("Unable to obtain Driver", e); + } + } + + private static Driver initDriver(String driver) { + Class driverClass = initDriverClass(driver); + try { + return Driver.class.cast(driverClass.getConstructor().newInstance()); + } catch (Throwable e) { + throw new IllegalStateException("Problem loading Database Driver: " + driver, e); + } + } + + private static Class initDriverClass(String driver) { + try { + ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); + if (contextLoader != null) { + return Class.forName(driver, true, contextLoader); + } else { + return Class.forName(driver, true, ObtainDriver.class.getClassLoader()); + } + } catch (Throwable e) { + throw new IllegalStateException("Problem loading Database Driver: " + driver, e); + } + } +} diff --git a/ebean-datasource/src/test/java/io/ebean/datasource/pool/ObtainDriverTest.java b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ObtainDriverTest.java new file mode 100644 index 0000000..ae2161e --- /dev/null +++ b/ebean-datasource/src/test/java/io/ebean/datasource/pool/ObtainDriverTest.java @@ -0,0 +1,34 @@ +package io.ebean.datasource.pool; + +import org.junit.jupiter.api.Test; + +import java.sql.Driver; + +import static org.assertj.core.api.Assertions.assertThat; + +class ObtainDriverTest { + + @Test + void h2() { + Driver h2Driver = ObtainDriver.driver(org.h2.Driver.class.getName(), "junk"); + assertThat(h2Driver).isInstanceOf(org.h2.Driver.class); + } + + @Test + void h2Url() { + assertThat(ObtainDriver.driver(null, "jdbc:h2:mem")).isInstanceOf(org.h2.Driver.class); + assertThat(ObtainDriver.driver("", "jdbc:h2:mem")).isInstanceOf(org.h2.Driver.class); + } + + @Test + void postgres() { + Driver h2Driver = ObtainDriver.driver(org.postgresql.Driver.class.getName(), "junk"); + assertThat(h2Driver).isInstanceOf(org.postgresql.Driver.class); + } + + @Test + void postgresUrl() { + Driver h2Driver = ObtainDriver.driver(null, "jdbc:postgresql://127.0.0.1:9999/app"); + assertThat(h2Driver).isInstanceOf(org.postgresql.Driver.class); + } +}