Author: psteitz
Date: Mon Feb 16 20:18:38 2015
New Revision: 1660195
URL: http://svn.apache.org/r1660195
Log:
Added property name verification to BasicDataSourceFactory. References including
obsolete or unrecognized properties now generate log messages.
JIRA: DBCP-435
Thanks to Denixx Baykin.
Modified:
commons/proper/dbcp/trunk/src/changes/changes.xml
commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/StackMessageLog.java
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestBasicDataSourceFactory.java
Modified: commons/proper/dbcp/trunk/src/changes/changes.xml
URL:
http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/changes/changes.xml?rev=1660195&r1=1660194&r2=1660195&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/changes/changes.xml (original)
+++ commons/proper/dbcp/trunk/src/changes/changes.xml Mon Feb 16 20:18:38 2015
@@ -114,6 +114,10 @@ The <action> type attribute can be add,u
<action dev="psteitz" type="update">
Eliminated synchronization in BasicDataSource getNumActive, getNumIdle
methods.
</action>
+ <action issue="DBCP-435" type="update" due-to="Denixx Baykin">
+ Added property name verification to BasicDataSourceFactory. References
including
+ obsolete or unrecognized properties now generate log messages.
+ </action>
</release>
<release version="2.0.1" date="24 May 2014" description="This is a bug fix
release.">
<action dev="markt" type="fix">
Modified:
commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
URL:
http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java?rev=1660195&r1=1660194&r2=1660195&view=diff
==============================================================================
---
commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
(original)
+++
commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java
Mon Feb 16 20:18:38 2015
@@ -17,13 +17,22 @@
package org.apache.commons.dbcp2;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.pool2.impl.BaseObjectPoolConfig;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;
import java.sql.Connection;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Enumeration;
import java.util.Hashtable;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
import java.util.Properties;
import java.util.StringTokenizer;
@@ -52,6 +61,8 @@ import javax.naming.spi.ObjectFactory;
*/
public class BasicDataSourceFactory implements ObjectFactory {
+ private static final Log log =
LogFactory.getLog(BasicDataSourceFactory.class);
+
private static final String PROP_DEFAULTAUTOCOMMIT = "defaultAutoCommit";
private static final String PROP_DEFAULTREADONLY = "defaultReadOnly";
private static final String PROP_DEFAULTTRANSACTIONISOLATION =
"defaultTransactionIsolation";
@@ -105,6 +116,24 @@ public class BasicDataSourceFactory impl
*/
private static final String PROP_DISCONNECTION_SQL_CODES =
"disconnectionSqlCodes";
+ /*
+ * Block with obsolete properties from DBCP 1.x.
+ * Warn users that these are ignored and they should use the 2.x
properties.
+ */
+ private static final String NUPROP_MAXACTIVE = "maxActive";
+ private static final String NUPROP_REMOVEABANDONED = "removeAbandoned";
+ private static final String NUPROP_MAXWAIT = "maxWait";
+
+ /*
+ * Block with properties expected in a DataSource
+ * This props will not be listed as ignored - we know that they may appear
in Resource,
+ * and not listing them as ignored.
+ */
+ private static final String SILENTPROP_FACTORY = "factory";
+ private static final String SILENTPROP_SCOPE = "scope";
+ private static final String SILENTPROP_SINGLETON = "singleton";
+ private static final String SILENTPROP_AUTH = "auth";
+
private static final String[] ALL_PROPERTIES = {
PROP_DEFAULTAUTOCOMMIT,
PROP_DEFAULTREADONLY,
@@ -150,6 +179,46 @@ public class BasicDataSourceFactory impl
PROP_DISCONNECTION_SQL_CODES
};
+ /**
+ * Obsolete properties from DBCP 1.x. with warning strings suggesting
+ * new properties. LinkedHashMap will guarantee that properties will be
listed
+ * to output in order of insertion into map.
+ */
+ private static final Map<String, String> NUPROP_WARNTEXT = new
LinkedHashMap<>();
+
+ static {
+ NUPROP_WARNTEXT.put(
+ NUPROP_MAXACTIVE,
+ "Property " + NUPROP_MAXACTIVE + " is not used in DBCP2, use "
+ PROP_MAXTOTAL + " instead. "
+ + PROP_MAXTOTAL + " default value is " +
GenericObjectPoolConfig.DEFAULT_MAX_TOTAL+".");
+ NUPROP_WARNTEXT.put(
+ NUPROP_REMOVEABANDONED,
+ "Property " + NUPROP_REMOVEABANDONED + " is not used in DBCP2,"
+ + " use one or both of "
+ + PROP_REMOVEABANDONEDONBORROW + " or " +
PROP_REMOVEABANDONEDONMAINTENANCE + " instead. "
+ + "Both have default value set to false.");
+ NUPROP_WARNTEXT.put(
+ NUPROP_MAXWAIT,
+ "Property " + NUPROP_MAXWAIT + " is not used in DBCP2"
+ + " , use " + PROP_MAXWAITMILLIS + " instead. "
+ + PROP_MAXWAITMILLIS + " default value is " +
BaseObjectPoolConfig.DEFAULT_MAX_WAIT_MILLIS+".");
+ }
+
+ /**
+ * Silent Properties.
+ * These properties will not be listed as ignored - we know that they may
appear in JDBC Resource references,
+ * and we will not list them as ignored.
+ */
+ private static final List<String> SILENT_PROPERTIES = new ArrayList<>();
+
+ static {
+ SILENT_PROPERTIES.add(SILENTPROP_FACTORY);
+ SILENT_PROPERTIES.add(SILENTPROP_SCOPE);
+ SILENT_PROPERTIES.add(SILENTPROP_SINGLETON);
+ SILENT_PROPERTIES.add(SILENTPROP_AUTH);
+
+ }
+
// -------------------------------------------------- ObjectFactory Methods
/**
@@ -181,6 +250,17 @@ public class BasicDataSourceFactory impl
return null;
}
+ // Check property names and log warnings about obsolete and / or
unknown properties
+ final List<String> warnings = new ArrayList<String>();
+ final List<String> infoMessages = new ArrayList<String>();
+ validatePropertyNames(ref, name, warnings, infoMessages);
+ for (String warning : warnings) {
+ log.warn(warning);
+ }
+ for (String infoMessage : infoMessages) {
+ log.info(infoMessage);
+ }
+
Properties properties = new Properties();
for (String propertyName : ALL_PROPERTIES) {
RefAddr ra = ref.get(propertyName);
@@ -194,6 +274,58 @@ public class BasicDataSourceFactory impl
}
/**
+ * Collects warnings and info messages. Warnings are generated when an
obsolete
+ * property is set. Unknown properties generate info messages.
+ *
+ * @param ref Reference to check properties of
+ * @param name Name provided to getObject
+ * @param warnings container for warning messages
+ * @param infoMessasges container for info messages
+ */
+ private void validatePropertyNames(Reference ref, Name name, List<String>
warnings,
+ List<String> infoMessages) {
+ final List<String> allPropsAsList = Arrays.asList(ALL_PROPERTIES);
+ final String nameString = name != null ? "Name = " + name.toString() +
" " : "";
+ if (NUPROP_WARNTEXT!=null && !NUPROP_WARNTEXT.keySet().isEmpty()) {
+ for (String propertyName : NUPROP_WARNTEXT.keySet()) {
+ final RefAddr ra = ref.get(propertyName);
+ if (ra != null && !allPropsAsList.contains(ra.getType())) {
+ final StringBuilder stringBuilder = new
StringBuilder(nameString);
+ final String propertyValue = ra.getContent().toString();
+ stringBuilder.append(NUPROP_WARNTEXT.get(propertyName))
+ .append(" You have set value of \"")
+ .append(propertyValue)
+ .append("\" for \"")
+ .append(propertyName)
+ .append("\" property, which is being ignored.");
+ warnings.add(stringBuilder.toString());
+ }
+ }
+ }
+
+ final Enumeration<RefAddr> allRefAddrs = ref.getAll();
+ while (allRefAddrs.hasMoreElements()) {
+ final RefAddr ra = allRefAddrs.nextElement();
+ final String propertyName = ra.getType();
+ // If property name is not in the properties list, we haven't
warned on it
+ // and it is not in the "silent" list, tell user we are ignoring
it.
+ if (!(allPropsAsList.contains(propertyName)
+ || NUPROP_WARNTEXT.keySet().contains(propertyName)
+ || SILENT_PROPERTIES.contains(propertyName))) {
+ final String propertyValue = ra.getContent().toString();
+ final StringBuilder stringBuilder = new
StringBuilder(nameString);
+ stringBuilder.append("Ignoring unknown property: ")
+ .append("value of \"")
+ .append(propertyValue)
+ .append("\" for \"")
+ .append(propertyName)
+ .append("\" property");
+ infoMessages.add(stringBuilder.toString());
+ }
+ }
+ }
+
+ /**
* Creates and configures a {@link BasicDataSource} instance based on the
* given properties.
*
Modified:
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/StackMessageLog.java
URL:
http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/StackMessageLog.java?rev=1660195&r1=1660194&r2=1660195&view=diff
==============================================================================
---
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/StackMessageLog.java
(original)
+++
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/StackMessageLog.java
Mon Feb 16 20:18:38 2015
@@ -17,7 +17,10 @@
package org.apache.commons.dbcp2;
+import java.util.ArrayList;
import java.util.EmptyStackException;
+import java.util.Iterator;
+import java.util.List;
import java.util.Stack;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -30,7 +33,7 @@ import org.apache.commons.logging.impl.S
*
* @version $Id$
*/
-public class StackMessageLog extends SimpleLog {
+public class StackMessageLog extends SimpleLog {
private static final long serialVersionUID = 1L;
private static Stack<String> messageStack = new Stack<String>();
@@ -80,6 +83,18 @@ public class StackMessageLog extends Sim
}
return ret;
}
+
+ /**
+ * Note: iterator is fail-fast, lock the stack first.
+ */
+ public static List<String> getAll() {
+ final Iterator<String> iterator = messageStack.iterator();
+ final List<String> messages = new ArrayList<String>();
+ while (iterator.hasNext()) {
+ messages.add(iterator.next());
+ }
+ return messages;
+ }
public static void clear() {
lock.lock();
Modified:
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestBasicDataSourceFactory.java
URL:
http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestBasicDataSourceFactory.java?rev=1660195&r1=1660194&r2=1660195&view=diff
==============================================================================
---
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestBasicDataSourceFactory.java
(original)
+++
commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestBasicDataSourceFactory.java
Mon Feb 16 20:18:38 2015
@@ -22,7 +22,10 @@ import static org.junit.Assert.assertNot
import static org.junit.Assert.assertTrue;
import java.sql.Connection;
+import java.util.List;
import java.util.Properties;
+import javax.naming.Reference;
+import javax.naming.StringRefAddr;
import org.junit.Test;
@@ -118,4 +121,31 @@ public class TestBasicDataSourceFactory
assertTrue(ds.getDisconnectionSqlCodes().contains("XXX"));
assertTrue(ds.getDisconnectionSqlCodes().contains("YYY"));
}
+
+ @Test
+ public void testValidateProperties() throws Exception {
+ try {
+ StackMessageLog.lock();
+ final Reference ref = new Reference("javax.sql.DataSource",
+
BasicDataSourceFactory.class.getName(), null);
+ ref.add(new StringRefAddr("foo", "bar")); // Unknown
+ ref.add(new StringRefAddr("maxWait", "100")); // Changed
+ ref.add(new StringRefAddr("driverClassName",
"org.apache.commons.dbcp2.TesterDriver")); //OK
+ final BasicDataSourceFactory basicDataSourceFactory = new
BasicDataSourceFactory();
+ basicDataSourceFactory.getObjectInstance(ref, null, null, null);
+ final List<String> messages = StackMessageLog.getAll();
+ assertEquals(2,messages.size());
+ for (String message : messages) {
+ if (message.contains("maxWait")) {
+ assertTrue(message.contains("use maxWaitMillis"));
+ } else {
+ assertTrue(message.contains("foo"));
+ assertTrue(message.contains("Ignoring unknown property"));
+ }
+ }
+ } finally {
+ StackMessageLog.clear();
+ StackMessageLog.unLock();
+ }
+ }
}
