[DBCP-547] Add a ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.

- Update version from 2.6.1-SNAPSHOT to 2.7.0 since this commits adds
new public APIs in BasicDataSource.
- Provide an alternative implementation from the PR
https://github.com/apache/commons-dbcp/pull/33 WRT to String value
handling. The handling of empty string for the new APIs is made
consistent with other String APIs instead of what is done in PR 33.
- Formatted new class TesterConnectionFactory differently from the PR
and sorted its methods.
- Closes #33.
diff --git a/pom.xml b/pom.xml
index 399774a..3d9b8f8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -26,7 +26,7 @@
   </parent>

   <modelVersion>4.0.0</modelVersion>

   <artifactId>commons-dbcp2</artifactId>

-  <version>2.6.1-SNAPSHOT</version>

+  <version>2.7.0-SNAPSHOT</version>

   <name>Apache Commons DBCP</name>

 

   <inceptionYear>2001</inceptionYear>

@@ -293,7 +293,7 @@
     <commons.rc.version>RC1</commons.rc.version>

     <commons.module.name>org.apache.commons.dbcp2</commons.module.name>

     

-    <commons.release.version>2.6.1</commons.release.version>

+    <commons.release.version>2.7.0</commons.release.version>

     <commons.release.desc>for JDBC 4.2 on Java 8</commons.release.desc>

     

     <commons.release.2.version>2.4.0</commons.release.2.version>

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6815b40..53e40a6 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -60,7 +60,7 @@
      -->

 

   <body>

-    <release version="2.6.1" date="2019-MM-DD" description="This is a minor release, including bug fixes and enhancements.">

+    <release version="2.7.0" date="2019-MM-DD" description="This is a minor release, including bug fixes and enhancements.">

       <action dev="jleroux" type="add" issue="DBCP-539" due-to="Jacques Le Roux">

         ManagedDataSource#close() should declare used exceptions.

       </action>

@@ -71,7 +71,7 @@
         Update tests from H2 1.4.198 to 1.4.199.

       </action>

       <action dev="ggregory" type="update" issue="DBCP-540" due-to="emopers">

-        Close ObjectOutputStream before calling toByteArray on underlying ByteArrayOutputStream #28.

+        Close ObjectOutputStream before calling toByteArray() on underlying ByteArrayOutputStream #28.

       </action>

       <action dev="ggregory" type="update" issue="DBCP-541" due-to="Allon Murienik">

         Upgrade to JUnit Jupiter #19.

@@ -85,6 +85,9 @@
       <action dev="ggregory" type="update" issue="DBCP-546" due-to="Sergey Chupov">

         Avoid NPE when calling DriverAdapterCPDS.toString().

       </action>

+      <action dev="ggregory" type="update" issue="DBCP-547" due-to="leechoongyon, Gary Gregory">

+        Add a ConnectionFactory class name setting for BasicDataSource.createConnectionFactory() #33.

+      </action>

     </release>

     <release version="2.6.0" date="2019-02-14" description="This is a minor release, including bug fixes and enhancements.">

       <action dev="chtompki" type="add" issue="DBCP-534" due-to="Peter Wicks">

diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
index 7c46359..3a3d065 100644
--- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
+++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java
@@ -109,7 +109,7 @@
     }

 

     protected static void validateConnectionFactory(final PoolableConnectionFactory connectionFactory)

-            throws Exception {

+        throws Exception {

         PoolableConnection conn = null;

         PooledObject<PoolableConnection> p = null;

         try {

@@ -315,6 +315,11 @@
     private volatile int validationQueryTimeoutSeconds = -1;

 

     /**

+     * The fully qualified Java class name of a {@link ConnectionFactory} implementation.

+     */

+    private String connectionFactoryClassName;

+

+    /**

      * These SQL statements run once after a Connection is created.

      * <p>

      * This property can be used for example to run ALTER SESSION SET NLS_SORT=XCYECH in an Oracle Database only once

@@ -364,7 +369,7 @@
      * The PrintWriter to which log messages should be directed.

      */

     private volatile PrintWriter logWriter = new PrintWriter(

-            new OutputStreamWriter(System.out, StandardCharsets.UTF_8));

+        new OutputStreamWriter(System.out, StandardCharsets.UTF_8));

 

     private AbandonedConfig abandonedConfig;

 

@@ -504,7 +509,7 @@
                 }

             } catch (final Exception t) {

                 final String message = "Cannot create JDBC driver of class '"

-                        + (driverClassName != null ? driverClassName : "") + "' for connect URL '" + url + "'";

+                    + (driverClassName != null ? driverClassName : "") + "' for connect URL '" + url + "'";

                 logWriter.println(message);

                 t.printStackTrace(logWriter);

                 throw new SQLException(message, t);

@@ -526,9 +531,7 @@
             log("DBCP DataSource configured without a 'password'");

         }

 

-        final ConnectionFactory driverConnectionFactory = new DriverConnectionFactory(driverToUse, url,

-                connectionProperties);

-        return driverConnectionFactory;

+        return createConnectionFactory(driverToUse);

     }

 

     /**

@@ -683,10 +686,10 @@
      * @return a non-null instance

      */

     protected GenericObjectPool<PoolableConnection> createObjectPool(final PoolableConnectionFactory factory,

-            final GenericObjectPoolConfig<PoolableConnection> poolConfig, final AbandonedConfig abandonedConfig) {

+        final GenericObjectPoolConfig<PoolableConnection> poolConfig, final AbandonedConfig abandonedConfig) {

         GenericObjectPool<PoolableConnection> gop;

-        if (abandonedConfig != null && (abandonedConfig.getRemoveAbandonedOnBorrow()

-                || abandonedConfig.getRemoveAbandonedOnMaintenance())) {

+        if (abandonedConfig != null

+            && (abandonedConfig.getRemoveAbandonedOnBorrow() || abandonedConfig.getRemoveAbandonedOnMaintenance())) {

             gop = new GenericObjectPool<>(factory, poolConfig, abandonedConfig);

         } else {

             gop = new GenericObjectPool<>(factory, poolConfig);

@@ -706,11 +709,11 @@
      * @return A new PoolableConnectionFactory configured with the current configuration of this BasicDataSource

      */

     protected PoolableConnectionFactory createPoolableConnectionFactory(final ConnectionFactory driverConnectionFactory)

-            throws SQLException {

+        throws SQLException {

         PoolableConnectionFactory connectionFactory = null;

         try {

             connectionFactory = new PoolableConnectionFactory(driverConnectionFactory,

-                    ObjectNameWrapper.unwrap(registeredJmxObjectName));

+                ObjectNameWrapper.unwrap(registeredJmxObjectName));

             connectionFactory.setValidationQuery(validationQuery);

             connectionFactory.setValidationQueryTimeout(validationQueryTimeoutSeconds);

             connectionFactory.setConnectionInitSql(connectionInitSqls);

@@ -991,6 +994,20 @@
     }

 

     /**

+     * Returns the ConnectionFactoryClassName that has been configured for use by this pool.

+     * <p>

+     * Note: This getter only returns the last value set by a call to

+     * {@link #setConnectionFactoryClassName(String)}.

+     * </p>

+     *

+     * @return the ConnectionFactoryClassName that has been configured for use by this pool.

+     * @since 2.7.0

+     */

+    public String getConnectionFactoryClassName() {

+        return this.connectionFactoryClassName;

+    }

+

+    /**

      * Returns the value of the flag that controls whether or not connections being returned to the pool will be checked

      * and configured with {@link Connection#setAutoCommit(boolean) Connection.setAutoCommit(true)} if the auto commit

      * setting is {@code false} when the connection is returned. It is <code>true</code> by default.

@@ -1505,7 +1522,7 @@
             poolableConnection = connection.unwrap(PoolableConnection.class);

             if (poolableConnection == null) {

                 throw new IllegalStateException(

-                        "Cannot invalidate connection: Connection is not a poolable connection.");

+                    "Cannot invalidate connection: Connection is not a poolable connection.");

             }

         } catch (final SQLException e) {

             throw new IllegalStateException("Cannot invalidate connection: Unwrapping poolable connection failed.", e);

@@ -1551,6 +1568,16 @@
     }

 

     /**

+     * Delegates in a null-safe manner to {@link String#isEmpty()}.

+     *

+     * @param value the string to test, may be null.

+     * @return boolean false if value is null, otherwise {@link String#isEmpty()}.

+     */

+    private boolean isEmpty(String value) {

+        return value == null ? true : value.trim().isEmpty();

+    }

+

+    /**

      * Returns true if we are pooling statements.

      *

      * @return true if prepared and callable statements are pooled

@@ -1723,7 +1750,7 @@
         if (connectionInitSqls != null && connectionInitSqls.size() > 0) {

             ArrayList<String> newVal = null;

             for (final String s : connectionInitSqls) {

-                if (s != null && s.trim().length() > 0) {

+                if (!isEmpty(s)) {

                     if (newVal == null) {

                         newVal = new ArrayList<>();

                     }

@@ -1801,10 +1828,10 @@
      *            the default catalog

      */

     public void setDefaultCatalog(final String defaultCatalog) {

-        if (defaultCatalog != null && defaultCatalog.trim().length() > 0) {

-            this.defaultCatalog = defaultCatalog;

-        } else {

+        if (isEmpty(defaultCatalog)) {

             this.defaultCatalog = null;

+        } else {

+            this.defaultCatalog = defaultCatalog;

         }

     }

 

@@ -1851,10 +1878,10 @@
      * @since 2.5.0

      */

     public void setDefaultSchema(final String defaultSchema) {

-        if (defaultSchema != null && defaultSchema.trim().length() > 0) {

-            this.defaultSchema = defaultSchema;

-        } else {

+        if (isEmpty(defaultSchema)) {

             this.defaultSchema = null;

+        } else {

+            this.defaultSchema = defaultSchema;

         }

     }

 

@@ -1902,7 +1929,7 @@
         if (disconnectionSqlCodes != null && disconnectionSqlCodes.size() > 0) {

             HashSet<String> newVal = null;

             for (final String s : disconnectionSqlCodes) {

-                if (s != null && s.trim().length() > 0) {

+                if (!isEmpty(s)) {

                     if (newVal == null) {

                         newVal = new HashSet<>();

                     }

@@ -1961,10 +1988,25 @@
      *            the class name of the JDBC driver

      */

     public synchronized void setDriverClassName(final String driverClassName) {

-        if (driverClassName != null && driverClassName.trim().length() > 0) {

-            this.driverClassName = driverClassName;

-        } else {

+        if (isEmpty(driverClassName)) {

             this.driverClassName = null;

+        } else {

+            this.driverClassName = driverClassName;

+        }

+    }

+

+    /**

+     * Sets the ConnectionFactory class name.

+     *

+     * @param connectionFactoryClassName

+     * @since 2.7.0

+     */

+

+    public void setConnectionFactoryClassName(final String connectionFactoryClassName) {

+        if (isEmpty(connectionFactoryClassName)) {

+            this.connectionFactoryClassName = null;

+        } else {

+            this.connectionFactoryClassName = connectionFactoryClassName;

         }

     }

 

@@ -2481,10 +2523,10 @@
      *            the new value for the validation query

      */

     public void setValidationQuery(final String validationQuery) {

-        if (validationQuery != null && validationQuery.trim().length() > 0) {

-            this.validationQuery = validationQuery;

-        } else {

+        if (isEmpty(validationQuery)) {

             this.validationQuery = null;

+        } else {

+            this.validationQuery = validationQuery;

         }

     }

 

@@ -2527,4 +2569,22 @@
         config.setJmxNameBase(base.toString());

         config.setJmxNamePrefix(Constants.JMX_CONNECTION_POOL_PREFIX);

     }

+

+    private ConnectionFactory createConnectionFactory(final Driver driver) throws SQLException {

+        if (connectionFactoryClassName != null) {

+            try {

+                Class<?> connectionFactoryFromCCL = Class.forName(connectionFactoryClassName);

+                return (ConnectionFactory) connectionFactoryFromCCL

+                    .getConstructor(Driver.class, String.class, Properties.class)

+                    .newInstance(driver, url, connectionProperties);

+            } catch (final Exception t) {

+                final String message = "Cannot load ConnectionFactory implementation '" + connectionFactoryClassName + "'";

+                logWriter.println(message);

+                t.printStackTrace(logWriter);

+                throw new SQLException(message, t);

+            }

+        }

+        return new DriverConnectionFactory(driver, url, connectionProperties);

+    }

+

 }

diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
index dfaab5e..e826cff 100644
--- a/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
+++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
@@ -87,6 +87,7 @@
     private static final String PROP_VALIDATION_QUERY = "validationQuery";
     private static final String PROP_VALIDATION_QUERY_TIMEOUT = "validationQueryTimeout";
     private static final String PROP_JMX_NAME = "jmxName";
+    private static final String PROP_CONNECTION_FACTORY_CLASS_NAME = "connectionFactoryClassName";
 
     /**
      * The property name for connectionInitSqls. The associated value String must be of the form [query;]*
@@ -141,7 +142,8 @@
             PROP_REMOVE_ABANDONED_TIMEOUT, PROP_LOG_ABANDONED, PROP_ABANDONED_USAGE_TRACKING, PROP_POOL_PREPARED_STATEMENTS,
             PROP_MAX_OPEN_PREPARED_STATEMENTS, PROP_CONNECTION_PROPERTIES, PROP_MAX_CONN_LIFETIME_MILLIS,
             PROP_LOG_EXPIRED_CONNECTIONS, PROP_ROLLBACK_ON_RETURN, PROP_ENABLE_AUTO_COMMIT_ON_RETURN,
-            PROP_DEFAULT_QUERY_TIMEOUT, PROP_FAST_FAIL_VALIDATION, PROP_DISCONNECTION_SQL_CODES, PROP_JMX_NAME };
+            PROP_DEFAULT_QUERY_TIMEOUT, PROP_FAST_FAIL_VALIDATION, PROP_DISCONNECTION_SQL_CODES, PROP_JMX_NAME,
+            PROP_CONNECTION_FACTORY_CLASS_NAME };
 
     /**
      * Obsolete properties from DBCP 1.x. with warning strings suggesting new properties. LinkedHashMap will guarantee
@@ -548,6 +550,13 @@
             dataSource.setDisconnectionSqlCodes(parseList(value, ','));
         }
 
+        value = properties.getProperty(PROP_CONNECTION_FACTORY_CLASS_NAME);
+        if (value != null) {
+        	dataSource.setConnectionFactoryClassName(value);
+        }
+
+
+
         // DBCP-215
         // Trick to make sure that initialSize connections are created
         if (dataSource.getInitialSize() > 0) {
diff --git a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java
index aee9a65..732bdf0 100644
--- a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java
+++ b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java
@@ -17,8 +17,13 @@
 
 package org.apache.commons.dbcp2;
 
-import static org.junit.jupiter.api.Assertions.*;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
@@ -38,8 +43,8 @@
 import org.hamcrest.CoreMatchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -875,6 +880,42 @@
         Assertions.assertNotEquals(Boolean.valueOf(original),
                 Boolean.valueOf(ds.getConnectionPool().getLogAbandoned()));
     }
+
+    /**
+     * JIRA: DBCP-547
+     * Verify that ConnectionFactory interface in BasicDataSource.createConnectionFactory().
+     */
+    @Test
+    public void testCreateConnectionFactory() throws Exception {
+
+    	/** not set ConnectionFactoryClassName */
+    	Properties properties = new Properties();
+        properties.put("initialSize", "1");
+        properties.put("driverClassName", "org.apache.commons.dbcp2.TesterDriver");
+        properties.put("url", "jdbc:apache:commons:testdriver");
+        properties.put("username", "foo");
+        properties.put("password", "bar");
+        BasicDataSource ds = BasicDataSourceFactory.createDataSource(properties);
+        Connection conn = ds.getConnection();
+        assertNotNull(conn);
+        conn.close();
+        ds.close();
+
+        /** set ConnectionFactoryClassName */
+        properties = new Properties();
+        properties.put("initialSize", "1");
+        properties.put("driverClassName", "org.apache.commons.dbcp2.TesterDriver");
+        properties.put("url", "jdbc:apache:commons:testdriver");
+        properties.put("username", "foo");
+        properties.put("password", "bar");
+        properties.put("connectionFactoryClassName", "org.apache.commons.dbcp2.TesterConnectionFactory");
+        ds = BasicDataSourceFactory.createDataSource(properties);
+        conn = ds.getConnection();
+        assertNotNull(conn);
+
+        conn.close();
+        ds.close();
+    }
 }
 
 /**
diff --git a/src/test/java/org/apache/commons/dbcp2/TesterConnectionFactory.java b/src/test/java/org/apache/commons/dbcp2/TesterConnectionFactory.java
new file mode 100644
index 0000000..7718e02
--- /dev/null
+++ b/src/test/java/org/apache/commons/dbcp2/TesterConnectionFactory.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.dbcp2;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.SQLException;
+import java.util.Properties;
+
+/**
+ * Dummy {@link ConnectionFactory} for testing purpose.
+ */
+public class TesterConnectionFactory implements ConnectionFactory {
+    
+    private final String connectionString;
+    private final Driver driver;
+    private final Properties properties;
+
+    /**
+     * Constructs a connection factory for a given Driver.
+     *
+     * @param driver        The Driver.
+     * @param connectString The connection string.
+     * @param properties    The connection properties.
+     */
+    public TesterConnectionFactory(final Driver driver, final String connectString, final Properties properties) {
+        this.driver = driver;
+        this.connectionString = connectString;
+        this.properties = properties;
+    }
+
+    @Override
+    public Connection createConnection() throws SQLException {
+        Connection conn = driver.connect(connectionString, properties);
+        doSomething(conn);
+        return conn;
+    }
+
+    private void doSomething(Connection conn) {
+        // do something
+    }
+
+    /**
+     * @return The connection String.
+     */
+    public String getConnectionString() {
+        return connectionString;
+    }
+
+    /**
+     * @return The Driver.
+     */
+    public Driver getDriver() {
+        return driver;
+    }
+
+    /**
+     * @return The Properties.
+     */
+    public Properties getProperties() {
+        return properties;
+    }
+
+    @Override
+    public String toString() {
+        return this.getClass().getName() + " [" + String.valueOf(driver) + ";" + String.valueOf(connectionString) + ";"
+            + String.valueOf(properties) + "]";
+    }
+}