You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/07/08 15:14:02 UTC
[commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.
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
The following commit(s) were added to refs/heads/master by this push:
new 8a579d3 [DBCP-547] Add a ConnectionFactory class name setting for BasicDataSource.createConnectionFactory() #33.
8a579d3 is described below
commit 8a579d304595853012ccf8c2bc93022c383a35bb
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Jul 8 11:13:58 2019 -0400
[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.
---
pom.xml | 4 +-
src/changes/changes.xml | 7 +-
.../org/apache/commons/dbcp2/BasicDataSource.java | 110 ++++++++++++++++-----
.../commons/dbcp2/BasicDataSourceFactory.java | 11 ++-
.../apache/commons/dbcp2/TestBasicDataSource.java | 45 ++++++++-
.../commons/dbcp2/TesterConnectionFactory.java | 84 ++++++++++++++++
6 files changed, 229 insertions(+), 32 deletions(-)
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 @@ The <action> type attribute can be add,update,fix,remove.
-->
<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 @@ The <action> type attribute can be add,update,fix,remove.
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 @@ The <action> type attribute can be add,update,fix,remove.
<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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
}
protected static void validateConnectionFactory(final PoolableConnectionFactory connectionFactory)
- throws Exception {
+ throws Exception {
PoolableConnection conn = null;
PooledObject<PoolableConnection> p = null;
try {
@@ -315,6 +315,11 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
}
} 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
log("DBCP DataSource configured without a 'password'");
}
- final ConnectionFactory driverConnectionFactory = new DriverConnectionFactory(driverToUse, url,
- connectionProperties);
- return driverConnectionFactory;
+ return createConnectionFactory(driverToUse);
}
/**
@@ -683,10 +686,10 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
}
/**
+ * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
}
/**
+ * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* the class name of the JDBC driver
*/
public synchronized void setDriverClassName(final String driverClassName) {
- if (driverClassName != null && driverClassName.trim().length() > 0) {
+ 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.driverClassName = null;
+ this.connectionFactoryClassName = connectionFactoryClassName;
}
}
@@ -2481,10 +2523,10 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
* 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
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 @@ public class BasicDataSourceFactory implements ObjectFactory {
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 @@ public class BasicDataSourceFactory implements ObjectFactory {
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 @@ public class BasicDataSourceFactory implements ObjectFactory {
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.apache.commons.logging.LogFactory;
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 @@ public class TestBasicDataSource extends TestConnectionPool {
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) + "]";
+ }
+}
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for BasicDataSource.createConnectionFactory()
#33.
Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Jul 11, 2019 at 9:44 AM Matt Sicker <bo...@gmail.com> wrote:
> Thanks, Mark! I’m in complete agreement on your formatting philosophy here
> (consistency over bikeshedding). And also kinda like the new line after
> operator, too, though no strong opinion there since I seem to forget which
> style I prefer sometimes.
>
+1 to consistency that can be applied simply via an IDE's formatter.
Gary
>
> On Thu, Jul 11, 2019 at 04:21, Mark Thomas <ma...@apache.org> wrote:
>
> > On 10/07/2019 22:47, Gary Gregory wrote:
> > > On Wed, Jul 10, 2019 at 11:52 AM Mark Thomas <ma...@apache.org> wrote:
> > >
> > >> On 10/07/2019 15:49, Gary Gregory wrote:
> > >>
> > >>> Without arguing about the merits of one kind of formatting vs.
> > another...
> > >>> If you can configure the Eclipse formatter to do that, I'd consider
> it,
> > >>> otherwise, I'm not into what I'd call "artisanal formatting" ;-)
> > >>
> > >>
> > >> The Eclipse setting you want is:
> > >>
> > >> Formatter > Line Wrapping > Default indentation for wrapped lines
> > >>
> > >> and set it to 2 (which should be the default).
> > >>
> > >>
> > >> That seems to do the trick when I run it locally. I'd commit the
> result
> > >> but the default settings change nearly every line in the file.
> > >>
> > >> Looking more closely, that appears to be a line ending issue. I
> thought
> > >> the accepted practice was to use unix line endings in the repo and
> > >> native line endings locally. It looks like there are some Windows line
> > >> endings in the repo.
> > >>
> > >> It would be worth saving your Eclipse formatter settings in the source
> > >> tree somewhere so everybody can work from the same set.
> > >>
> > >
> > > I set the setting you mentioned to 2 and saved my config
> > > here: src/conf/eclipse/formatter.xml
> > > I did not reformat anything.
> >
> > Thanks. I applied that to BasicDataSource.
> >
> > I haven't applied that formatting to all files although it probably
> > makes sense to do so.
> >
> > I've looked through the formatting and my personal preference would be
> > to change one more thing (actually a handful of settings for different
> > operators):
> > - wrap before operator -> wrap after operator
> >
> > but that is a low priority for me. If the consensus is against that I'm
> > fine with that. I'd rather spend time convincing people it is helpful
> > for the project to use a consistent formatting throughout than endlessly
> > debate the merits of each individual option.
> >
> > I also fixed the line endings of the 10 or so files that were using \r\n
> > line endings rather than the recommended \n.
> >
> > Mark
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> > --
> Matt Sicker <bo...@gmail.com>
>
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for BasicDataSource.createConnectionFactory()
#33.
Posted by Matt Sicker <bo...@gmail.com>.
Thanks, Mark! I’m in complete agreement on your formatting philosophy here
(consistency over bikeshedding). And also kinda like the new line after
operator, too, though no strong opinion there since I seem to forget which
style I prefer sometimes.
On Thu, Jul 11, 2019 at 04:21, Mark Thomas <ma...@apache.org> wrote:
> On 10/07/2019 22:47, Gary Gregory wrote:
> > On Wed, Jul 10, 2019 at 11:52 AM Mark Thomas <ma...@apache.org> wrote:
> >
> >> On 10/07/2019 15:49, Gary Gregory wrote:
> >>
> >>> Without arguing about the merits of one kind of formatting vs.
> another...
> >>> If you can configure the Eclipse formatter to do that, I'd consider it,
> >>> otherwise, I'm not into what I'd call "artisanal formatting" ;-)
> >>
> >>
> >> The Eclipse setting you want is:
> >>
> >> Formatter > Line Wrapping > Default indentation for wrapped lines
> >>
> >> and set it to 2 (which should be the default).
> >>
> >>
> >> That seems to do the trick when I run it locally. I'd commit the result
> >> but the default settings change nearly every line in the file.
> >>
> >> Looking more closely, that appears to be a line ending issue. I thought
> >> the accepted practice was to use unix line endings in the repo and
> >> native line endings locally. It looks like there are some Windows line
> >> endings in the repo.
> >>
> >> It would be worth saving your Eclipse formatter settings in the source
> >> tree somewhere so everybody can work from the same set.
> >>
> >
> > I set the setting you mentioned to 2 and saved my config
> > here: src/conf/eclipse/formatter.xml
> > I did not reformat anything.
>
> Thanks. I applied that to BasicDataSource.
>
> I haven't applied that formatting to all files although it probably
> makes sense to do so.
>
> I've looked through the formatting and my personal preference would be
> to change one more thing (actually a handful of settings for different
> operators):
> - wrap before operator -> wrap after operator
>
> but that is a low priority for me. If the consensus is against that I'm
> fine with that. I'd rather spend time convincing people it is helpful
> for the project to use a consistent formatting throughout than endlessly
> debate the merits of each individual option.
>
> I also fixed the line endings of the 10 or so files that were using \r\n
> line endings rather than the recommended \n.
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
> --
Matt Sicker <bo...@gmail.com>
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.
Posted by Mark Thomas <ma...@apache.org>.
On 10/07/2019 22:47, Gary Gregory wrote:
> On Wed, Jul 10, 2019 at 11:52 AM Mark Thomas <ma...@apache.org> wrote:
>
>> On 10/07/2019 15:49, Gary Gregory wrote:
>>
>>> Without arguing about the merits of one kind of formatting vs. another...
>>> If you can configure the Eclipse formatter to do that, I'd consider it,
>>> otherwise, I'm not into what I'd call "artisanal formatting" ;-)
>>
>>
>> The Eclipse setting you want is:
>>
>> Formatter > Line Wrapping > Default indentation for wrapped lines
>>
>> and set it to 2 (which should be the default).
>>
>>
>> That seems to do the trick when I run it locally. I'd commit the result
>> but the default settings change nearly every line in the file.
>>
>> Looking more closely, that appears to be a line ending issue. I thought
>> the accepted practice was to use unix line endings in the repo and
>> native line endings locally. It looks like there are some Windows line
>> endings in the repo.
>>
>> It would be worth saving your Eclipse formatter settings in the source
>> tree somewhere so everybody can work from the same set.
>>
>
> I set the setting you mentioned to 2 and saved my config
> here: src/conf/eclipse/formatter.xml
> I did not reformat anything.
Thanks. I applied that to BasicDataSource.
I haven't applied that formatting to all files although it probably
makes sense to do so.
I've looked through the formatting and my personal preference would be
to change one more thing (actually a handful of settings for different
operators):
- wrap before operator -> wrap after operator
but that is a low priority for me. If the consensus is against that I'm
fine with that. I'd rather spend time convincing people it is helpful
for the project to use a consistent formatting throughout than endlessly
debate the merits of each individual option.
I also fixed the line endings of the 10 or so files that were using \r\n
line endings rather than the recommended \n.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for BasicDataSource.createConnectionFactory()
#33.
Posted by Gary Gregory <ga...@gmail.com>.
On Wed, Jul 10, 2019 at 11:52 AM Mark Thomas <ma...@apache.org> wrote:
> On 10/07/2019 15:49, Gary Gregory wrote:
>
> > Without arguing about the merits of one kind of formatting vs. another...
> > If you can configure the Eclipse formatter to do that, I'd consider it,
> > otherwise, I'm not into what I'd call "artisanal formatting" ;-)
>
>
> The Eclipse setting you want is:
>
> Formatter > Line Wrapping > Default indentation for wrapped lines
>
> and set it to 2 (which should be the default).
>
>
> That seems to do the trick when I run it locally. I'd commit the result
> but the default settings change nearly every line in the file.
>
> Looking more closely, that appears to be a line ending issue. I thought
> the accepted practice was to use unix line endings in the repo and
> native line endings locally. It looks like there are some Windows line
> endings in the repo.
>
> It would be worth saving your Eclipse formatter settings in the source
> tree somewhere so everybody can work from the same set.
>
I set the setting you mentioned to 2 and saved my config
here: src/conf/eclipse/formatter.xml
I did not reformat anything.
Gary
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.
Posted by Mark Thomas <ma...@apache.org>.
On 10/07/2019 15:49, Gary Gregory wrote:
> Without arguing about the merits of one kind of formatting vs. another...
> If you can configure the Eclipse formatter to do that, I'd consider it,
> otherwise, I'm not into what I'd call "artisanal formatting" ;-)
The Eclipse setting you want is:
Formatter > Line Wrapping > Default indentation for wrapped lines
and set it to 2 (which should be the default).
That seems to do the trick when I run it locally. I'd commit the result
but the default settings change nearly every line in the file.
Looking more closely, that appears to be a line ending issue. I thought
the accepted practice was to use unix line endings in the repo and
native line endings locally. It looks like there are some Windows line
endings in the repo.
It would be worth saving your Eclipse formatter settings in the source
tree somewhere so everybody can work from the same set.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for BasicDataSource.createConnectionFactory()
#33.
Posted by Gary Gregory <ga...@gmail.com>.
On Tue, Jul 9, 2019 at 4:59 AM Mark Thomas <ma...@apache.org> wrote:
> On 08/07/2019 21:42, Gary Gregory wrote:
> > On Mon, Jul 8, 2019 at 11:23 AM Mark Thomas <ma...@apache.org> wrote:
> >
> >> On 08/07/2019 16:14, ggregory@apache.org wrote:
> >>> 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
> >>>
> >>>
> >>> The following commit(s) were added to refs/heads/master by this push:
> >>> new 8a579d3 [DBCP-547] Add a ConnectionFactory class name setting
> >> for BasicDataSource.createConnectionFactory() #33.
> >>> 8a579d3 is described below
> >>>
> >>> commit 8a579d304595853012ccf8c2bc93022c383a35bb
> >>> Author: Gary Gregory <ga...@gmail.com>
> >>> AuthorDate: Mon Jul 8 11:13:58 2019 -0400
> >>>
> >>> [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.
> >>
> >> This appears to have used an indent of 4 spaces for continuation lines
> >> rather than 8, making the code harder to read.
> >>
> >
> > I'm pretty sure most of the code base does not use super-wide 8 spaces
> for
> > indentation (for example between an if-statement and its body.
>
> I am not talking about standard indentation of code blocks, for which |I
> agree 4 spaces is thr standard. I am talking about indentation *for
> continuation lines*. Take the following example:
>
> >>> 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 @@ public class BasicDataSource implements DataSource,
> >> BasicDataSourceMXBean, MBean
> >>> }
> >>>
> >>> protected static void validateConnectionFactory(final
> >> PoolableConnectionFactory connectionFactory)
> >>> - throws Exception {
> >>> + throws Exception {
> >>> PoolableConnection conn = null;
> >>> PooledObject<PoolableConnection> p = null;
> >>> try {
>
> I've edited the code to avoid line wrapping in email but the original was:
>
> protected static void validateConnectionFactory(final PCF factory)
> throws Exception {
> PoolableConnection conn = null;
> PooledObject<PoolableConnection> p = null;
> try {
>
> Note how the "throws Exception {" line is indented 8 spaces rather than
> 4 because it is a continuation of the line above.
>
> After the reformatting this became:
>
> protected static void validateConnectionFactory(final PCF factory)
> throws Exception {
> PoolableConnection conn = null;
> PooledObject<PoolableConnection> p = null;
> try {
>
> The continuation line is now only indented 4 spaces. This aligns it with
> the contained code block making it visually harder to separate the two.
>
> I quickly skimmed through the DBCP code base. I didn't find that many
> continuation lines but those that I did all used an 8 space indent.
Without arguing about the merits of one kind of formatting vs. another...
If you can configure the Eclipse formatter to do that, I'd consider it,
otherwise, I'm not into what I'd call "artisanal formatting" ;-)
Gary
> mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.
Posted by Mark Thomas <ma...@apache.org>.
On 08/07/2019 21:42, Gary Gregory wrote:
> On Mon, Jul 8, 2019 at 11:23 AM Mark Thomas <ma...@apache.org> wrote:
>
>> On 08/07/2019 16:14, ggregory@apache.org wrote:
>>> 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
>>>
>>>
>>> The following commit(s) were added to refs/heads/master by this push:
>>> new 8a579d3 [DBCP-547] Add a ConnectionFactory class name setting
>> for BasicDataSource.createConnectionFactory() #33.
>>> 8a579d3 is described below
>>>
>>> commit 8a579d304595853012ccf8c2bc93022c383a35bb
>>> Author: Gary Gregory <ga...@gmail.com>
>>> AuthorDate: Mon Jul 8 11:13:58 2019 -0400
>>>
>>> [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.
>>
>> This appears to have used an indent of 4 spaces for continuation lines
>> rather than 8, making the code harder to read.
>>
>
> I'm pretty sure most of the code base does not use super-wide 8 spaces for
> indentation (for example between an if-statement and its body.
I am not talking about standard indentation of code blocks, for which |I
agree 4 spaces is thr standard. I am talking about indentation *for
continuation lines*. Take the following example:
>>> 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 @@ public class BasicDataSource implements DataSource,
>> BasicDataSourceMXBean, MBean
>>> }
>>>
>>> protected static void validateConnectionFactory(final
>> PoolableConnectionFactory connectionFactory)
>>> - throws Exception {
>>> + throws Exception {
>>> PoolableConnection conn = null;
>>> PooledObject<PoolableConnection> p = null;
>>> try {
I've edited the code to avoid line wrapping in email but the original was:
protected static void validateConnectionFactory(final PCF factory)
throws Exception {
PoolableConnection conn = null;
PooledObject<PoolableConnection> p = null;
try {
Note how the "throws Exception {" line is indented 8 spaces rather than
4 because it is a continuation of the line above.
After the reformatting this became:
protected static void validateConnectionFactory(final PCF factory)
throws Exception {
PoolableConnection conn = null;
PooledObject<PoolableConnection> p = null;
try {
The continuation line is now only indented 4 spaces. This aligns it with
the contained code block making it visually harder to separate the two.
I quickly skimmed through the DBCP code base. I didn't find that many
continuation lines but those that I did all used an 8 space indent.
mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for BasicDataSource.createConnectionFactory()
#33.
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Jul 8, 2019 at 11:23 AM Mark Thomas <ma...@apache.org> wrote:
> On 08/07/2019 16:14, ggregory@apache.org wrote:
> > 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
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 8a579d3 [DBCP-547] Add a ConnectionFactory class name setting
> for BasicDataSource.createConnectionFactory() #33.
> > 8a579d3 is described below
> >
> > commit 8a579d304595853012ccf8c2bc93022c383a35bb
> > Author: Gary Gregory <ga...@gmail.com>
> > AuthorDate: Mon Jul 8 11:13:58 2019 -0400
> >
> > [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.
>
> This appears to have used an indent of 4 spaces for continuation lines
> rather than 8, making the code harder to read.
>
I'm pretty sure most of the code base does not use super-wide 8 spaces for
indentation (for example between an if-statement and its body.
Gary
>
> Mark
>
>
> > - Closes #33.
> > ---
> > pom.xml | 4 +-
> > src/changes/changes.xml | 7 +-
> > .../org/apache/commons/dbcp2/BasicDataSource.java | 110
> ++++++++++++++++-----
> > .../commons/dbcp2/BasicDataSourceFactory.java | 11 ++-
> > .../apache/commons/dbcp2/TestBasicDataSource.java | 45 ++++++++-
> > .../commons/dbcp2/TesterConnectionFactory.java | 84
> ++++++++++++++++
> > 6 files changed, 229 insertions(+), 32 deletions(-)
> >
> > 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 @@ The <action> type attribute can be
> add,update,fix,remove.
> > -->
> >
> > <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 @@ The <action> type attribute can be
> add,update,fix,remove.
> > 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 @@ The <action> type attribute can be
> add,update,fix,remove.
> > <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 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > }
> >
> > protected static void validateConnectionFactory(final
> PoolableConnectionFactory connectionFactory)
> > - throws Exception {
> > + throws Exception {
> > PoolableConnection conn = null;
> > PooledObject<PoolableConnection> p = null;
> > try {
> > @@ -315,6 +315,11 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > 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 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > * 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 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > }
> > } 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 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > log("DBCP DataSource configured without a 'password'");
> > }
> >
> > - final ConnectionFactory driverConnectionFactory = new
> DriverConnectionFactory(driverToUse, url,
> > - connectionProperties);
> > - return driverConnectionFactory;
> > + return createConnectionFactory(driverToUse);
> > }
> >
> > /**
> > @@ -683,10 +686,10 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * @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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * @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 @@ public class BasicDataSource implements DataSource,
> BasicDataSourceMXBean, MBean
> > }
> >
> > /**
> > + * 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > }
> >
> > /**
> > + * 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * @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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * the class name of the JDBC driver
> > */
> > public synchronized void setDriverClassName(final String
> driverClassName) {
> > - if (driverClassName != null && driverClassName.trim().length()
> > 0) {
> > + 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.driverClassName = null;
> > + this.connectionFactoryClassName =
> connectionFactoryClassName;
> > }
> > }
> >
> > @@ -2481,10 +2523,10 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > * 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 @@ public class BasicDataSource implements
> DataSource, BasicDataSourceMXBean, MBean
> > 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 @@ public class BasicDataSourceFactory implements
> ObjectFactory {
> > 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 @@ public class BasicDataSourceFactory implements
> ObjectFactory {
> > 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 @@ public class BasicDataSourceFactory implements
> ObjectFactory {
> > 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.apache.commons.logging.LogFactory;
> > 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 @@ public class TestBasicDataSource extends
> TestConnectionPool {
> > 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) + "]";
> > + }
> > +}
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [commons-dbcp] branch master updated: [DBCP-547] Add a
ConnectionFactory class name setting for
BasicDataSource.createConnectionFactory() #33.
Posted by Mark Thomas <ma...@apache.org>.
On 08/07/2019 16:14, ggregory@apache.org wrote:
> 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
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 8a579d3 [DBCP-547] Add a ConnectionFactory class name setting for BasicDataSource.createConnectionFactory() #33.
> 8a579d3 is described below
>
> commit 8a579d304595853012ccf8c2bc93022c383a35bb
> Author: Gary Gregory <ga...@gmail.com>
> AuthorDate: Mon Jul 8 11:13:58 2019 -0400
>
> [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.
This appears to have used an indent of 4 spaces for continuation lines
rather than 8, making the code harder to read.
Mark
> - Closes #33.
> ---
> pom.xml | 4 +-
> src/changes/changes.xml | 7 +-
> .../org/apache/commons/dbcp2/BasicDataSource.java | 110 ++++++++++++++++-----
> .../commons/dbcp2/BasicDataSourceFactory.java | 11 ++-
> .../apache/commons/dbcp2/TestBasicDataSource.java | 45 ++++++++-
> .../commons/dbcp2/TesterConnectionFactory.java | 84 ++++++++++++++++
> 6 files changed, 229 insertions(+), 32 deletions(-)
>
> 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 @@ The <action> type attribute can be add,update,fix,remove.
> -->
>
> <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 @@ The <action> type attribute can be add,update,fix,remove.
> 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 @@ The <action> type attribute can be add,update,fix,remove.
> <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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> }
>
> protected static void validateConnectionFactory(final PoolableConnectionFactory connectionFactory)
> - throws Exception {
> + throws Exception {
> PoolableConnection conn = null;
> PooledObject<PoolableConnection> p = null;
> try {
> @@ -315,6 +315,11 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> }
> } 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> log("DBCP DataSource configured without a 'password'");
> }
>
> - final ConnectionFactory driverConnectionFactory = new DriverConnectionFactory(driverToUse, url,
> - connectionProperties);
> - return driverConnectionFactory;
> + return createConnectionFactory(driverToUse);
> }
>
> /**
> @@ -683,10 +686,10 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> }
>
> /**
> + * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> }
>
> /**
> + * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * @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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * the class name of the JDBC driver
> */
> public synchronized void setDriverClassName(final String driverClassName) {
> - if (driverClassName != null && driverClassName.trim().length() > 0) {
> + 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.driverClassName = null;
> + this.connectionFactoryClassName = connectionFactoryClassName;
> }
> }
>
> @@ -2481,10 +2523,10 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> * 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 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean
> 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 @@ public class BasicDataSourceFactory implements ObjectFactory {
> 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 @@ public class BasicDataSourceFactory implements ObjectFactory {
> 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 @@ public class BasicDataSourceFactory implements ObjectFactory {
> 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.apache.commons.logging.LogFactory;
> 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 @@ public class TestBasicDataSource extends TestConnectionPool {
> 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) + "]";
> + }
> +}
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org