OPENJPA-2754 re-enable dbcp2 auto detection

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfiguration.java

@@ -486,7 +486,7 @@ public interface JDBCConfiguration
      * Create an instance of the {@link DriverDataSource} to use
      * for creating a {@link DataSource} from a JDBC {@link Driver}.
      */
-    DataSource newDriverDataSourceInstance();
+    DriverDataSource newDriverDataSourceInstance();
 
     /**
      * The plugin string for the {@link SchemaFactory} to use to provide

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfigurationImpl.java

@@ -36,6 +36,7 @@ import org.apache.openjpa.jdbc.kernel.UpdateManager;
 import org.apache.openjpa.jdbc.meta.MappingDefaults;
 import org.apache.openjpa.jdbc.meta.MappingRepository;
 import org.apache.openjpa.jdbc.schema.DataSourceFactory;
+import org.apache.openjpa.jdbc.schema.DriverDataSource;
 import org.apache.openjpa.jdbc.schema.SchemaFactory;
 import org.apache.openjpa.jdbc.sql.DBDictionary;
 import org.apache.openjpa.jdbc.sql.DBDictionaryFactory;
@@ -245,8 +246,9 @@ public class JDBCConfigurationImpl
 
         driverDataSourcePlugin = addPlugin("jdbc.DriverDataSource", false);
         aliases = new String[]{
+            "auto", "org.apache.openjpa.jdbc.schema.AutoDriverDataSource",
             "simple", "org.apache.openjpa.jdbc.schema.SimpleDriverDataSource",
-            "dbcp", "org.apache.commons.dbcp2.BasicDataSource"
+            "dbcp", "org.apache.openjpa.jdbc.schema.DBCPDriverDataSource"
         };
         driverDataSourcePlugin.setAliases(aliases);
         driverDataSourcePlugin.setDefault(aliases[0]);
@@ -686,9 +688,9 @@ public class JDBCConfigurationImpl
     }
 
     @Override
-    public DataSource newDriverDataSourceInstance() {
-        return (DataSource) driverDataSourcePlugin.
-            instantiate(DataSource.class, this);
+    public DriverDataSource newDriverDataSourceInstance() {
+        return (DriverDataSource) driverDataSourcePlugin.
+            instantiate(DriverDataSource.class, this);
     }
 
     @Override

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/AutoDriverDataSource.java

@@ -0,0 +1,45 @@
+/*
+ * 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.openjpa.jdbc.schema;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Properties;
+
+/**
+ * Automatic Commons DBCP pooling or Simple non-pooling driver data source.
+ * If the commons-dbcp packages are on the class path, then they will be used,
+ * else it will fall back to non-DBCP mode.
+ */
+public class AutoDriverDataSource
+    extends DBCPDriverDataSource {
+
+    @Override
+    public Connection getConnection(Properties props) throws SQLException {
+        // if we're using managed transactions, or user specified a DBCP driver
+        // or DBCP is not on the classpath, then use SimpleDriver
+        if (conf == null || conf.isTransactionModeManaged() || conf.isConnectionFactoryModeManaged() ||
+                !isDBCPLoaded(getClassLoader())) {
+            return getSimpleConnection(props);
+        } else {
+            // use DBCPDriverDataSource
+            return getDBCPConnection(props);
+        }
+    }
+}

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DBCPDriverDataSource.java

@@ -0,0 +1,308 @@
+/*
+ * 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.openjpa.jdbc.schema;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Iterator;
+import java.util.Locale;
+import java.util.Properties;
+
+import javax.sql.DataSource;
+
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+import org.apache.openjpa.lib.conf.Configurable;
+import org.apache.openjpa.lib.conf.Configuration;
+import org.apache.openjpa.lib.conf.Configurations;
+import org.apache.openjpa.lib.util.Closeable;
+
+/**
+ * Commons DBCP basic pooling driver data source.
+ * The commons-dbcp packages must be on the class path for this plugin to work,
+ * as it WILL NOT fall back to non-DBCP mode if they are missing. For automatic
+ * usage of Commons DBCP when available, use AutoDriverDataSource instead.
+ */
+public class DBCPDriverDataSource
+extends SimpleDriverDataSource implements Configurable, Closeable {
+
+    private static String DBCPPACKAGENAME = "org.apache.commons.dbcp2";
+    private static String DBCPBASICDATASOURCENAME = "org.apache.commons.dbcp2.BasicDataSource";
+    private static Class<?> _dbcpClass;
+    private static Boolean _dbcpAvail;
+    private static RuntimeException _dbcpEx;
+
+    protected JDBCConfiguration conf;
+    private DataSource _ds;
+
+    @Override
+    public Connection getConnection(Properties props) throws SQLException {
+        return getDBCPConnection(props);
+    }
+
+    @Override
+    public void close() throws SQLException {
+        try {
+            if (_ds != null) {
+                if (isDBCPLoaded(getClassLoader())) {
+                    ((org.apache.commons.dbcp2.BasicDataSource)_dbcpClass.cast(_ds)).close();
+                }
+            }
+        } catch (Exception e) {
+            // no-op
+        } catch (Throwable t) {
+            // no-op
+        } finally {
+            _ds = null;
+        }
+    }
+
+    protected Connection getDBCPConnection(Properties props) throws SQLException {
+        Connection con = getDBCPDataSource(props).getConnection();
+        if (con == null) {
+            throw new SQLException(_eloc.get("dbcp-ds-null",
+                DBCPBASICDATASOURCENAME, getConnectionDriverName(), getConnectionURL()).getMessage());
+        }
+        return con;
+    }
+
+    protected DataSource getDBCPDataSource(Properties props) {
+        if (isDBCPLoaded(getClassLoader())) {
+            if (_ds == null) {
+                try {
+                    Properties dbcpProps = updateDBCPProperties(props);
+                    _ds = (DataSource) Configurations.newInstance(DBCPBASICDATASOURCENAME, conf,
+                        dbcpProps, getClassLoader());
+                } catch (Exception e) {
+                    _dbcpEx = new RuntimeException(_eloc.get("driver-null", DBCPBASICDATASOURCENAME).getMessage(), e);
+                }
+                return _ds;
+            } else {
+                return _ds;
+            }
+        } else {
+            // user choose DBCP, so fail if it isn't on the classpath
+            if (_dbcpEx == null)
+                _dbcpEx = new RuntimeException(_eloc.get("driver-null", DBCPBASICDATASOURCENAME).getMessage());
+            throw _dbcpEx;
+        }
+    }
+
+    /**
+     * This method should not throw an exception, as it is called by
+     * AutoDriverDataSource to determine if user already specified
+     * to use Commons DBCP.
+     * @return true if ConnectionDriverName contains org.apache.commons.dbcp2,
+     *         otherwise false
+     */
+    protected boolean isDBCPDataSource() {
+        return (getConnectionDriverName() != null &&
+            getConnectionDriverName().toLowerCase(Locale.ENGLISH).indexOf(DBCPPACKAGENAME) >= 0);
+    }
+
+    /**
+     * This method should not throw an exception, as it is called by
+     * AutoDriverDataSource to determine if it should use DBCP or not
+     * based on if org.apache.commons.dbcp2.BasicDataSource can be loaded.
+     * @return true if Commons DBCP was found on the classpath, otherwise false
+     */
+    static protected boolean isDBCPLoaded(ClassLoader cl) {
+        if (Boolean.TRUE.equals(_dbcpAvail) && (_dbcpClass != null)) {
+            return true;
+        } else if (Boolean.FALSE.equals(_dbcpAvail)) {
+            return false;
+        } else {
+            // first time checking, so try to load it
+            try {
+                _dbcpClass = Class.forName(DBCPBASICDATASOURCENAME, true, cl);
+                _dbcpAvail = Boolean.TRUE;
+                return true;
+            } catch (Exception e) {
+                _dbcpAvail = Boolean.FALSE;
+                // save exception details for later instead of throwing here
+                _dbcpEx = new RuntimeException(_eloc.get("driver-null", DBCPBASICDATASOURCENAME).getMessage(), e);
+            }
+            return _dbcpAvail.booleanValue();
+        }
+    }
+
+    /**
+     * Normalize properties for Commons DBCP.  This should be done for every call from DataSourceFactory,
+     * as we do not have a pre-configured Driver to reuse.
+     * @param props
+     * @return updated properties
+     */
+    private Properties updateDBCPProperties(Properties props) {
+        Properties dbcpProps = mergeConnectionProperties(props);
+
+        // only perform the following check for the first connection attempt (_driverClassName == null),
+        // as multiple connections could be requested (like from SchemaTool) and isDBCPDriver() will be true
+        if (isDBCPDataSource()) {
+            String propDriver = hasProperty(dbcpProps, "driverClassName");
+            if (propDriver == null || propDriver.trim().isEmpty()) {
+                // if user specified DBCP for the connectionDriverName, then make sure they supplied a DriverClassName
+                throw new RuntimeException(_eloc.get("connection-property-invalid", "DriverClassName",
+                    propDriver).getMessage());
+            }
+            propDriver = hasProperty(dbcpProps, "url");
+            if (propDriver == null || propDriver.trim().isEmpty()) {
+                // if user specified DBCP for the connectionDriverName, then make sure they supplied a Url
+                throw new RuntimeException(_eloc.get("connection-property-invalid", "URL",
+                    propDriver).getMessage());
+            }
+        } else {
+            // set Commons DBCP expected DriverClassName to the original connection driver name
+            dbcpProps.setProperty(hasKey(dbcpProps, "driverClassName", "driverClassName"), getConnectionDriverName());
+            // set Commons DBCP expected URL property
+            dbcpProps.setProperty(hasKey(dbcpProps, "url", "url"), getConnectionURL());
+        }
+
+        // Commons DBCP requires non-Null username/password values in the connection properties
+        if (hasKey(dbcpProps, "username") == null) {
+            if (getConnectionUserName() != null)
+                dbcpProps.setProperty("username", getConnectionUserName());
+            else
+                dbcpProps.setProperty("username", "");
+        }
+        // Commons DBCP requires non-Null username/password values in the connection properties
+        if (hasKey(dbcpProps, "password") == null) {
+            if (getConnectionPassword() != null)
+                dbcpProps.setProperty("password", getConnectionPassword());
+            else
+                dbcpProps.setProperty("password", "");
+        }
+
+        // set some default properties for DBCP
+        if (hasKey(dbcpProps, "maxIdle") == null) {
+            dbcpProps.setProperty("maxIdle", "1");
+        }
+        if (hasKey(dbcpProps, "minIdle") == null) {
+            dbcpProps.setProperty("minIdle", "0");
+        }
+        if (hasKey(dbcpProps, "maxActive") == null) {
+            dbcpProps.setProperty("maxActive", "10");
+        }
+
+        return dbcpProps;
+    }
+
+    /**
+     * Merge the passed in properties with a copy of the existing _connectionProperties
+     * @param props
+     * @return Merged properties
+     */
+    private Properties mergeConnectionProperties(final Properties props) {
+        Properties mergedProps = new Properties();
+        mergedProps.putAll(getConnectionProperties());
+
+        // need to map "user" to "username" for Commons DBCP
+        String uid = removeProperty(mergedProps, "user");
+        if (uid != null) {
+            mergedProps.setProperty("username", uid);
+        }
+
+        // now, merge in any passed in properties
+        if (props != null && !props.isEmpty()) {
+            for (Iterator<Object> itr = props.keySet().iterator(); itr.hasNext();) {
+                String key = (String)itr.next();
+                String value = props.getProperty(key);
+                // need to map "user" to "username" for Commons DBCP
+                if ("user".equalsIgnoreCase(key)) {
+                    key = "username";
+                }
+                // case-insensitive search for existing key
+                String existingKey = hasKey(mergedProps, key);
+                if (existingKey != null) {
+                    // update existing entry
+                    mergedProps.setProperty(existingKey, value);
+                } else {
+                    // add property to the merged set
+                    mergedProps.setProperty(key, value);
+                }
+            }
+        }
+        return mergedProps;
+    }
+
+    /**
+     * Case-insensitive search of the given properties for the given key.
+     * @param props
+     * @param key
+     * @return Key name as found in properties or null if it was not found.
+     */
+    private String hasKey(Properties props, String key) {
+        return hasKey(props, key, null);
+    }
+
+    /**
+     * Case-insensitive search of the given properties for the given key.
+     * @param props
+     * @param key
+     * @param defaultKey
+     * @return Key name as found in properties or the given defaultKey if it was not found.
+     */
+    private String hasKey(Properties props, String key, String defaultKey)
+    {
+        if (props != null && key != null) {
+            for (Iterator<Object> itr = props.keySet().iterator(); itr.hasNext();) {
+                String entry = (String)itr.next();
+                if (key.equalsIgnoreCase(entry))
+                    return entry;
+            }
+        }
+        return defaultKey;
+    }
+
+    private String hasProperty(Properties props, String key) {
+        if (props != null && key != null) {
+            String entry = hasKey(props, key);
+            if (entry != null)
+                return props.getProperty(entry);
+        }
+        return null;
+
+    }
+
+    private String removeProperty(Properties props, String key) {
+        if (props != null && key != null) {
+            String entry = hasKey(props, key);
+            if (entry != null)
+                return (String)props.remove(entry);
+        }
+        return null;
+    }
+
+    // Configurable interface methods
+    @Override
+    public void setConfiguration(Configuration conf) {
+        if (conf instanceof JDBCConfiguration)
+            this.conf = (JDBCConfiguration)conf;
+    }
+
+    @Override
+    public void startConfiguration() {
+        // no-op
+    }
+
+    @Override
+    public void endConfiguration() {
+        // no-op
+    }
+
+}
+

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java

@@ -18,7 +18,6 @@
  */
 package org.apache.openjpa.jdbc.schema;
 
-import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.sql.Connection;
 import java.sql.Driver;
@@ -57,7 +56,7 @@ import org.apache.openjpa.util.UserException;
  * @author Abe White
  */
 public class DataSourceFactory {
-    private static final String DBCPBASICDATASOURCENAME = "org.apache.commons.dbcp2.BasicDataSource";
+
     private static final Localizer _loc = Localizer.forPackage(DataSourceFactory.class);
     protected static Localizer _eloc = Localizer.forPackage(DelegatingDataSource.class);
 
@@ -86,42 +85,27 @@ public class DataSourceFactory {
             }
 
             if (Driver.class.isAssignableFrom(driverClass)) {
-                DataSource rawDs = conf.newDriverDataSourceInstance();
-                if (rawDs instanceof DriverDataSource) {
-                    DriverDataSource ds = (DriverDataSource)rawDs;
-                    ds.setClassLoader(loader);
-                    ds.setConnectionDriverName(driver);
-                    ds.setConnectionProperties(Configurations.
-                        parseProperties(props));
+                DriverDataSource ds = conf.newDriverDataSourceInstance();
+                ds.setClassLoader(loader);
+                ds.setConnectionDriverName(driver);
+                ds.setConnectionProperties(Configurations.
+                    parseProperties(props));
 
-                    if (!factory2) {
-                        ds.setConnectionFactoryProperties(Configurations.
-                            parseProperties(conf.getConnectionFactoryProperties()));
-                        ds.setConnectionURL(conf.getConnectionURL());
-                        ds.setConnectionUserName(conf.getConnectionUserName());
-                        ds.setConnectionPassword(conf.getConnectionPassword());
-                    } else {
-                        ds.setConnectionFactoryProperties
-                            (Configurations.parseProperties(conf.
-                            getConnectionFactory2Properties()));
-                        ds.setConnectionURL(conf.getConnection2URL());
-                        ds.setConnectionUserName(conf.getConnection2UserName());
-                        ds.setConnectionPassword(conf.getConnection2Password());
-                    }
-                    return ds;
-                } else if (DBCPBASICDATASOURCENAME.equals(rawDs.getClass().getName())) {
-                    reflectiveCall(rawDs, "setDriverClassLoader", ClassLoader.class, loader);
-                    reflectiveCall(rawDs, "setDriverClassName", String.class, driver);
-                    reflectiveCall(rawDs, "setConnectionProperties", String.class, props);
-
-                    reflectiveCall(rawDs, "setUrl", String.class, factory2
-                            ? conf.getConnection2URL() : conf.getConnectionURL());
-                    reflectiveCall(rawDs, "setUsername", String.class, factory2
-                            ? conf.getConnection2UserName() : conf.getConnectionUserName());
-                    reflectiveCall(rawDs, "setPassword", String.class, factory2
-                            ? conf.getConnection2Password() : conf.getConnectionPassword());
-                    return rawDs;
+                if (!factory2) {
+                    ds.setConnectionFactoryProperties(Configurations.
+                        parseProperties(conf.getConnectionFactoryProperties()));
+                    ds.setConnectionURL(conf.getConnectionURL());
+                    ds.setConnectionUserName(conf.getConnectionUserName());
+                    ds.setConnectionPassword(conf.getConnectionPassword());
+                } else {
+                    ds.setConnectionFactoryProperties
+                        (Configurations.parseProperties(conf.
+                        getConnectionFactory2Properties()));
+                    ds.setConnectionURL(conf.getConnection2URL());
+                    ds.setConnectionUserName(conf.getConnection2UserName());
+                    ds.setConnectionPassword(conf.getConnection2Password());
                 }
+                return ds;
             }
 
             // see if their driver name is actually a data source
@@ -142,12 +126,6 @@ public class DataSourceFactory {
         throw new UserException(_loc.get("bad-driver", driver)).setFatal(true);
     }
 
-    private static void reflectiveCall(Object obj, String meth, Class<?> valClass, Object value) throws Exception {
-        Class<?> clazz = obj.getClass();
-        Method method = clazz.getMethod(meth, valClass);
-        method.invoke(obj, value);
-    }
-
     /**
      * Install listeners and base decorators.
      */
@@ -277,7 +255,7 @@ public class DataSourceFactory {
 
             return ds;
         } catch (Exception e) {
-        	throw newConnectException(conf, factory2, e);
+            throw newConnectException(conf, factory2, e);
         } finally {
             if (conn != null)
                 try {
@@ -290,13 +268,13 @@ public class DataSourceFactory {
     }
 
     static OpenJPAException newConnectException(JDBCConfiguration conf,
-    		boolean factory2, Exception cause) {
-    	return new UserException(_eloc.get("poolds-null", factory2
-          	  ? new Object[]{conf.getConnection2DriverName(),
-          			         conf.getConnection2URL()}
-          	  : new Object[]{conf.getConnectionDriverName(),
-          		             conf.getConnectionURL()}),
-          		             cause).setFatal(true);
+                                                boolean factory2, Exception cause) {
+        return new UserException(_eloc.get("poolds-null", factory2
+                ? new Object[]{conf.getConnection2DriverName(),
+                conf.getConnection2URL()}
+                : new Object[]{conf.getConnectionDriverName(),
+                conf.getConnectionURL()}),
+                cause).setFatal(true);
     }
 
     /**

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SimpleDriverDataSource.java

@@ -83,15 +83,15 @@ public class SimpleDriverDataSource
     }
 
     protected Connection getSimpleConnection(Properties props) throws SQLException {
-		Properties conProps = new Properties();
-		if (props != null) {
-			conProps.putAll(props);
-		}
-		if (_connectionProperties != null) {
-			conProps.putAll(_connectionProperties);
-		}
-    	Connection con = getSimpleDriver().connect(_connectionURL, conProps);
-    	if (con == null) {
+        Properties conProps = new Properties();
+        if (props != null) {
+            conProps.putAll(props);
+        }
+        if (_connectionProperties != null) {
+            conProps.putAll(_connectionProperties);
+        }
+        Connection con = getSimpleDriver().connect(_connectionURL, conProps);
+        if (con == null) {
             throw new SQLException(_eloc.get("poolds-null",
                     _connectionDriverName, _connectionURL).getMessage());
         }

openjpa-project/src/doc/manual/ref_guide_dbsetup.xml

@@ -79,8 +79,22 @@ provided implementations.
                 </seealso>
             </indexterm>
             <para>
-<literal>openjpa.jdbc.DriverDataSource=simple</literal>, will make
-OpenJPA to use <classname>org.apache.openjpa.jdbc.schema.SimpleDriverDataSource</classname>
+                Starting with OpenJPA 2.1, a new <classname>org.apache.openjpa.jdbc.schema.AutoDriverDataSource</classname>
+                is provided as the default, which will automatically
+                select between the old <classname>SimpleDriverDataSource</classname> and a new
+                <classname>DBCPDriverDataSource</classname> implementation based on if
+                <ulink url="https://commons.apache.org/dbcp/">Apache Commons DBCP2</ulink>
+                has been provided on the classpath and OpenJPA is not running in a container
+                managed mode or with managed transactions.  Note, that only the
+                <literal>openjpa-all.jar</literal> includes Commons DBCP2, so you will need to
+                include the <literal>commons-dbcp2.jar</literal> from the OpenJPA binary
+                distribution if you are using the normal <literal>openjpa.jar</literal>.
+            </para>
+            <para>
+                To disable the automatic usage of Apache Commons DBCP when it is discovered
+                on the classpath, set
+                <literal>openjpa.jdbc.DriverDataSource=simple</literal>, which will revert
+                OpenJPA to the prior behavior of using <classname>org.apache.openjpa.jdbc.schema.SimpleDriverDataSource</classname>
             </para>
             <para>
 To force usage of Apache Commons DBCP2, which will cause a fatal exception to