This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
commit 8431c21d408f67e0f95f8bf0d9ce5a17eaa7ebf9 Author: Gary Gregory <gardgreg...@gmail.com> AuthorDate: Tue Jul 30 09:56:01 2019 -0400 [Checkstyle] Move DriverConnectionFactory creation from BasicDataSource to a factory method _in_ DriverConnectionFactory. This moves the factory code closer to home and fixes a checkstyle violation that had BasicDataSource over 2,500 lines of source code. --- .../org/apache/commons/dbcp2/BasicDataSource.java | 56 ++++++++------ .../commons/dbcp2/DriverConnectionFactory.java | 88 ++++++++++++++++++++++ 2 files changed, 121 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java index 6bf8b34..ad879b8 100644 --- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java +++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java @@ -76,8 +76,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean private static final Log log = LogFactory.getLog(BasicDataSource.class); - // ------------------------------------------------------------- Properties - static { // Attempt to prevent deadlocks - see DBCP - 272 DriverManager.getDrivers(); @@ -108,6 +106,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } + @SuppressWarnings("resource") protected static void validateConnectionFactory(final PoolableConnectionFactory connectionFactory) throws Exception { PoolableConnection conn = null; @@ -450,7 +449,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } /** - * Creates a JDBC connection factory for this datasource. The JDBC driver is loaded using the following algorithm: + * Creates a JDBC connection factory for this data source. The JDBC driver is loaded using the following algorithm: * <ol> * <li>If a Driver instance has been specified via {@link #setDriver(Driver)} use it</li> * <li>If no Driver instance was specified and {@link #driverClassName} is specified that class is loaded using the @@ -460,7 +459,9 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean * context class loader of the current thread.</li> * <li>If a driver still isn't loaded one is loaded via the {@link DriverManager} using the specified {@link #url}. * </ol> + * <p> * This method exists so subclasses can replace the implementation class. + * </p> * * @return A new connection factory. * @@ -731,6 +732,17 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } /** + * Manually evicts idle connections + * + * @throws Exception when there is a problem evicting idle objects. + */ + public void evict() throws Exception { + if (connectionPool != null) { + connectionPool.evict(); + } + } + + /** * Gets the print writer used by this configuration to log information on abandoned objects. * * @return The print writer used by this configuration to log information on abandoned objects. @@ -845,7 +857,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean return connectionPool; } - // For unit testing Properties getConnectionProperties() { return connectionProperties; } @@ -1518,18 +1529,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } /** - * Manually evicts idle connections. - * - * @throws Exception Thrown by {@link GenericObjectPool#evict()}. - * @see GenericObjectPool#evict() - */ - public void evict() throws Exception { - if (connectionPool != null) { - connectionPool.evict(); - } - } - - /** * Returns the value of the accessToUnderlyingConnectionAllowed property. * * @return true if access to the underlying connection is allowed, false otherwise. @@ -1540,7 +1539,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } /** - * If true, this data source is closed and no more connections can be retrieved from this datasource. + * If true, this data source is closed and no more connections can be retrieved from this data source. * * @return true, if the data source is closed; false otherwise */ @@ -1597,6 +1596,18 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } + /** + * Logs the given throwable. + * + * @param throwable the throwable. + * @since 2.7.0 + */ + protected void log(Throwable throwable) { + if (logWriter != null) { + throwable.printStackTrace(logWriter); + } + } + @Override public void postDeregister() { // NO-OP @@ -1712,6 +1723,8 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean this.cacheState = cacheState; } + // ----------------------------------------------------- DataSource Methods + /** * Sets the list of SQL statements to be executed when a physical connection is first created. * <p> @@ -1739,8 +1752,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } - // ----------------------------------------------------- DataSource Methods - /** * Sets the connection properties passed to driver.connect(...). * <p> @@ -1964,10 +1975,9 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean /** * Sets the ConnectionFactory class name. * - * @param connectionFactoryClassName + * @param connectionFactoryClassName A class name. * @since 2.7.0 */ - public void setConnectionFactoryClassName(final String connectionFactoryClassName) { if (isEmpty(connectionFactoryClassName)) { this.connectionFactoryClassName = null; @@ -2218,6 +2228,8 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } + // ------------------------------------------------------ Protected Methods + /** * Sets the value of the {@link #numTestsPerEvictionRun} property. * @@ -2231,8 +2243,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } - // ------------------------------------------------------ Protected Methods - /** * <p> * Sets the {@link #password}. diff --git a/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java b/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java index 1b4f5dd..a99ff9f 100644 --- a/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java +++ b/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java @@ -18,6 +18,7 @@ package org.apache.commons.dbcp2; import java.sql.Connection; import java.sql.Driver; +import java.sql.DriverManager; import java.sql.SQLException; import java.util.Properties; @@ -33,6 +34,93 @@ public class DriverConnectionFactory implements ConnectionFactory { private final Properties properties; /** + * Creates a JDBC connection factory for the given data source. The JDBC driver is loaded using the following + * algorithm: + * <ol> + * <li>If a Driver instance has been specified via {@link #setDriver(Driver)} use it</li> + * <li>If no Driver instance was specified and {@link #driverClassName} is specified that class is loaded using the + * {@link ClassLoader} of this class or, if {@link #driverClassLoader} is set, {@link #driverClassName} is loaded + * with the specified {@link ClassLoader}.</li> + * <li>If {@link #driverClassName} is specified and the previous attempt fails, the class is loaded using the + * context class loader of the current thread.</li> + * <li>If a driver still isn't loaded one is loaded via the {@link DriverManager} using the specified {@link #url}. + * </ol> + * <p> + * This method exists so subclasses can replace the implementation class. + * </p> + * + * @return A new connection factory. + * + * @throws SQLException If the connection factory cannot be created + */ + static DriverConnectionFactory createConnectionFactory(final BasicDataSource basicDataSource) throws SQLException { + // Load the JDBC driver class + Driver driverToUse = basicDataSource.getDriver(); + final String driverClassName = basicDataSource.getDriverClassName(); + final ClassLoader driverClassLoader = basicDataSource.getDriverClassLoader(); + final String url = basicDataSource.getUrl(); + + if (driverToUse == null) { + Class<?> driverFromCCL = null; + if (driverClassName != null) { + try { + try { + if (driverClassLoader == null) { + driverFromCCL = Class.forName(driverClassName); + } else { + driverFromCCL = Class.forName(driverClassName, true, driverClassLoader); + } + } catch (final ClassNotFoundException cnfe) { + driverFromCCL = Thread.currentThread().getContextClassLoader().loadClass(driverClassName); + } + } catch (final Exception t) { + final String message = "Cannot load JDBC driver class '" + driverClassName + "'"; + basicDataSource.log(message); + basicDataSource.log(t); + throw new SQLException(message, t); + } + } + + try { + if (driverFromCCL == null) { + driverToUse = DriverManager.getDriver(url); + } else { + // Usage of DriverManager is not possible, as it does not + // respect the ContextClassLoader + // N.B. This cast may cause ClassCastException which is handled below + driverToUse = (Driver) driverFromCCL.getConstructor().newInstance(); + if (!driverToUse.acceptsURL(url)) { + throw new SQLException("No suitable driver", "08001"); + } + } + } catch (final Exception t) { + final String message = "Cannot create JDBC driver of class '" + + (driverClassName != null ? driverClassName : "") + "' for connect URL '" + url + "'"; + basicDataSource.log(message); + basicDataSource.log(t); + throw new SQLException(message, t); + } + } + + // Set up the driver connection factory we will use + final String user = basicDataSource.getUsername(); + if (user != null) { + basicDataSource.addConnectionProperty("user", user); + } else { + basicDataSource.log("DBCP DataSource configured without a 'username'"); + } + + final String pwd = basicDataSource.getPassword(); + if (pwd != null) { + basicDataSource.addConnectionProperty("password", pwd); + } else { + basicDataSource.log("DBCP DataSource configured without a 'password'"); + } + + return new DriverConnectionFactory(driverToUse, url, basicDataSource.getConnectionProperties()); + } + + /** * Constructs a connection factory for a given Driver. * * @param driver