This is an automated email from the ASF dual-hosted git repository. struberg pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit 6afab6486af7beda11e57696ec607a1c638e318e Author: Mark Struberg <strub...@apache.org> AuthorDate: Thu Jan 3 00:13:59 2019 +0100 OPENJPA-2754 re-enable dbcp2 auto detection --- .../openjpa/jdbc/conf/JDBCConfiguration.java | 2 +- .../openjpa/jdbc/conf/JDBCConfigurationImpl.java | 10 +- .../openjpa/jdbc/schema/AutoDriverDataSource.java | 45 +++ .../openjpa/jdbc/schema/DBCPDriverDataSource.java | 308 +++++++++++++++++++++ .../openjpa/jdbc/schema/DataSourceFactory.java | 80 ++---- .../jdbc/schema/SimpleDriverDataSource.java | 18 +- .../src/doc/manual/ref_guide_dbsetup.xml | 18 +- 7 files changed, 414 insertions(+), 67 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfiguration.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfiguration.java index 6808f46..34ee5eb 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfiguration.java +++ b/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 diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfigurationImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfigurationImpl.java index 9fdb6dd..abfea8a 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/conf/JDBCConfigurationImpl.java +++ b/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 diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/AutoDriverDataSource.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/AutoDriverDataSource.java new file mode 100644 index 0000000..90b828f --- /dev/null +++ b/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); + } + } +} diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DBCPDriverDataSource.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DBCPDriverDataSource.java new file mode 100644 index 0000000..c2d28e4 --- /dev/null +++ b/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 + } + +} + diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java index d41f154..8afd9df 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/DataSourceFactory.java +++ b/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)); - - 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; + 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; } // 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); } /** diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SimpleDriverDataSource.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SimpleDriverDataSource.java index 6b1a369..5273e2e 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SimpleDriverDataSource.java +++ b/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()); } diff --git a/openjpa-project/src/doc/manual/ref_guide_dbsetup.xml b/openjpa-project/src/doc/manual/ref_guide_dbsetup.xml index 5d5b8c1..a021dc0 100644 --- a/openjpa-project/src/doc/manual/ref_guide_dbsetup.xml +++ b/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