This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch release in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
commit 183310f751a8fff3a2069abc5d92970be40bec46 Author: Gary Gregory <gardgreg...@gmail.com> AuthorDate: Tue Jul 30 09:41:53 2019 -0400 Revert "[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." This reverts commit 5b43bc7347ba68a4d5831060b8198266470c38ee. --- .../org/apache/commons/dbcp2/BasicDataSource.java | 81 +++++++++++++++++--- .../commons/dbcp2/DriverConnectionFactory.java | 88 ---------------------- 2 files changed, 69 insertions(+), 100 deletions(-) diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java index 6dd7320..c6a4c34 100644 --- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java +++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java @@ -467,7 +467,69 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean * If the connection factort cannot be created */ protected ConnectionFactory createConnectionFactory() throws SQLException { - return DriverConnectionFactory.createConnectionFactory(this); + // Load the JDBC driver class + Driver driverToUse = this.driver; + + 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 + "'"; + logWriter.println(message); + t.printStackTrace(logWriter); + 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 + "'"; + logWriter.println(message); + t.printStackTrace(logWriter); + throw new SQLException(message, t); + } + } + + // Set up the driver connection factory we will use + final String user = userName; + if (user != null) { + connectionProperties.put("user", user); + } else { + log("DBCP DataSource configured without a 'username'"); + } + + final String pwd = password; + if (pwd != null) { + connectionProperties.put("password", pwd); + } else { + log("DBCP DataSource configured without a 'password'"); + } + + final ConnectionFactory driverConnectionFactory = new DriverConnectionFactory(driverToUse, url, + connectionProperties); + return driverConnectionFactory; } /** @@ -806,6 +868,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean return connectionPool; } + // For unit testing Properties getConnectionProperties() { return connectionProperties; } @@ -1520,18 +1583,12 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } - void log(final String message) { + protected void log(final String message) { if (logWriter != null) { logWriter.println(message); } } - void log(Throwable throwable) { - if (logWriter != null) { - throwable.printStackTrace(logWriter); - } - } - @Override public void postDeregister() { // NO-OP @@ -1652,8 +1709,6 @@ 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> @@ -1682,6 +1737,8 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } + // ----------------------------------------------------- DataSource Methods + /** * Sets the connection properties passed to driver.connect(...). * <p> @@ -2171,8 +2228,6 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } } - // ------------------------------------------------------ Protected Methods - /** * Sets the value of the {@link #numTestsPerEvictionRun} property. * @@ -2187,6 +2242,8 @@ 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 a99ff9f..1b4f5dd 100644 --- a/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java +++ b/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java @@ -18,7 +18,6 @@ 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; @@ -34,93 +33,6 @@ 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