commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pste...@apache.org
Subject svn commit: r907288 - in /commons/proper/dbcp/trunk/src: java/org/apache/commons/dbcp/datasources/ test/org/apache/commons/dbcp/ test/org/apache/commons/dbcp/datasources/
Date Sat, 06 Feb 2010 19:42:59 GMT
Author: psteitz
Date: Sat Feb  6 19:42:58 2010
New Revision: 907288

URL: http://svn.apache.org/viewvc?rev=907288&view=rev
Log:
Completed implementation of changed password support for SharedPoolDataSource and PerUserPoolDataSource. JIRA: DBCP-8.

Added:
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java   (with props)
Modified:
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/CPDSConnectionFactory.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/InstanceKeyDataSource.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/KeyedCPDSConnectionFactory.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PerUserPoolDataSource.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PoolKey.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionAndInfo.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/SharedPoolDataSource.java
    commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/UserPassKey.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterDriver.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
    commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/CPDSConnectionFactory.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/CPDSConnectionFactory.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/CPDSConnectionFactory.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/CPDSConnectionFactory.java Sat Feb  6 19:42:58 2010
@@ -30,7 +30,6 @@
 import javax.sql.ConnectionPoolDataSource;
 import javax.sql.PooledConnection;
 
-import org.apache.commons.dbcp.SQLNestedException;
 import org.apache.commons.pool.ObjectPool;
 import org.apache.commons.pool.PoolableObjectFactory;
 
@@ -42,18 +41,18 @@
  * @version $Revision$ $Date$
  */
 class CPDSConnectionFactory
-        implements PoolableObjectFactory, ConnectionEventListener {
+        implements PoolableObjectFactory, ConnectionEventListener, PooledConnectionManager {
 
     private static final String NO_KEY_MESSAGE
             = "close() was called on a Connection, but "
             + "I have no record of the underlying PooledConnection.";
 
-    protected ConnectionPoolDataSource _cpds = null;
-    protected volatile String _validationQuery = null;
-    protected volatile boolean _rollbackAfterValidation = false;
-    protected volatile ObjectPool _pool = null;
-    protected String _username = null;
-    protected String _password = null;
+    private final ConnectionPoolDataSource _cpds;
+    private final String _validationQuery;
+    private final boolean _rollbackAfterValidation;
+    private final ObjectPool _pool;
+    private String _username = null;
+    private String _password = null;
 
     /** 
      * Map of PooledConnections for which close events are ignored.
@@ -84,12 +83,7 @@
                                  String validationQuery,
                                  String username,
                                  String password) {
-        _cpds = cpds;
-        _pool = pool;
-        pool.setFactory(this);
-        _validationQuery = validationQuery;
-        _username = username;
-        _password = password;
+        this(cpds, pool, validationQuery, false, username, password);
     }
     
     /**
@@ -113,71 +107,23 @@
                                   boolean rollbackAfterValidation,
                                   String username,
                                   String password) {
-         this(cpds, pool, validationQuery, username, password);
+         _cpds = cpds;
+         _pool = pool;
+         pool.setFactory(this);
+         _validationQuery = validationQuery;
+         _username = username;
+         _password = password;
          _rollbackAfterValidation = rollbackAfterValidation;
      }
-
-
-    /**
-     * Sets the {@link ConnectionPoolDataSource} from which to obtain base
-     * {@link Connection}s.
-     * @param cpds the {@link ConnectionPoolDataSource} from which to obtain
-     *        base {@link Connection}s
-     */
-    public synchronized void setCPDS(ConnectionPoolDataSource cpds) {
-        _cpds = cpds;
-    }
-
-    /**
-     * Sets the query I use to {*link #validateObject validate}
-     * {@link Connection}s.
-     * Should return at least one row.
-     * May be <code>null</code>
-     * @param validationQuery a query to use to {@link #validateObject validate}
-     *        {@link Connection}s.
-     */
-    public void setValidationQuery(String validationQuery) {
-        _validationQuery = validationQuery;
-    }
-
-    /**
-     * Sets whether a rollback should be issued after 
-     * {@link #validateObject validating} 
-     * {@link Connection}s.
-     * @param rollbackAfterValidation whether a rollback should be issued after
-     *        {@link #validateObject validating} 
-     *        {@link Connection}s.
-     */
-    public void setRollbackAfterValidation(
-            boolean rollbackAfterValidation) {
-        _rollbackAfterValidation = rollbackAfterValidation;
-    }
-
-    /**
-     * Sets the {@link ObjectPool} in which to pool {*link Connection}s.
-     * @param pool the {*link ObjectPool} in which to pool those
-     *        {@link Connection}s
-     */
-    public synchronized void setPool(ObjectPool pool) throws SQLException {
-        if (null != _pool && pool != _pool) {
-            try {
-                _pool.close();
-            } catch (RuntimeException e) {
-                throw e;
-            } catch (Exception e) {
-                throw new SQLNestedException("Cannot set the pool on this factory", e);
-            }
-        }
-        _pool = pool;
-    }
-
-    /**
-     * Gets the {@link ObjectPool} for {@link Connection}s.
-     * @return connection pool
-     */
-    public synchronized ObjectPool getPool() {
-        return _pool;
-    }
+     
+     /**
+      * Returns the object pool used to pool connections created by this factory.
+      * 
+      * @return ObjectPool managing pooled connections
+      */
+     public ObjectPool getPool() {
+         return _pool;
+     }
 
     public synchronized Object makeObject() {
         Object obj;
@@ -345,5 +291,54 @@
             e.printStackTrace();
         }
     }
-
+    
+    // ***********************************************************************
+    // PooledConnectionManager implementation
+    // ***********************************************************************
+    
+    /**
+     * Invalidates the PooledConnection in the pool.  The CPDSConnectionFactory
+     * closes the connection and pool counters are updated appropriately.
+     * Also closes the pool.  This ensures that all idle connections are closed
+     * and connections that are checked out are closed on return.
+     */
+    public void invalidate(PooledConnection pc) throws SQLException {
+        Object info = pcMap.get(pc);
+        if (info == null) {
+            throw new IllegalStateException(NO_KEY_MESSAGE);
+        }
+        try {
+            _pool.invalidateObject(info);  // Destroy instance and update pool counters
+            _pool.close();  // Clear any other instances in this pool and kill others as they come back
+        } catch (Exception ex) {
+            throw (SQLException) new SQLException("Error invalidating connection").initCause(ex);
+        }   
+    }
+    
+    /**
+     * Sets the database password used when creating new connections.
+     * 
+     * @param password new password
+     */
+    public synchronized void setPassword(String password) {
+        _password = password;
+    }
+    
+    /**
+     * Verifies that the username matches the user whose connections are being managed by this
+     * factory and closes the pool if this is the case; otherwise does nothing.
+     */
+    public void closePool(String username) throws SQLException {
+        synchronized (this) {
+            if (username == null || !username.equals(_username)) {
+                return;
+            }
+        }
+        try {
+            _pool.close();
+        } catch (Exception ex) {
+            throw (SQLException) new SQLException("Error closing connection pool").initCause(ex);
+        } 
+    }
+    
 }

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/InstanceKeyDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/InstanceKeyDataSource.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/InstanceKeyDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/InstanceKeyDataSource.java Sat Feb  6 19:42:58 2010
@@ -151,9 +151,11 @@
     }
 
     /**
-     * Close pool being maintained by this datasource.
+     * Close the connection pool being maintained by this datasource.
      */
     public abstract void close() throws Exception;
+    
+    protected abstract PooledConnectionManager getConnectionManager(UserPassKey upkey);
 
     /* JDBC_4_ANT_KEY_BEGIN */
     public boolean isWrapperFor(Class<?> iface) throws SQLException {
@@ -665,7 +667,16 @@
     }
 
     /**
-     * Attempt to establish a database connection.
+     * Attempt to retrieve a database connection using {@link #getPooledConnectionAndInfo(String, String)}
+     * with the provided username and password.  The password on the {@link PooledConnectionAndInfo}
+     * instance returned by <code>getPooledConnectionAndInfo</code> is compared to the <code>password</code>
+     * parameter.  If the comparison fails, a database connection using the supplied username and password
+     * is attempted.  If the connection attempt fails, an SQLException is thrown, indicating that the given password
+     * did not match the password used to create the pooled connection.  If the connection attempt succeeds, this
+     * means that the database password has been changed.  In this case, the <code>PooledConnectionAndInfo</code>
+     * instance retrieved with the old password is destroyed and the <code>getPooledConnectionAndInfo</code> is
+     * repeatedly invoked until a <code>PooledConnectionAndInfo</code> instance with the new password is returned. 
+     * 
      */
     public Connection getConnection(String username, String password)
             throws SQLException {        
@@ -693,10 +704,55 @@
         }
         
         if (!(null == password ? null == info.getPassword() 
-                : password.equals(info.getPassword()))) {
-            closeDueToException(info);
-            throw new SQLException("Given password did not match password used"
-                                   + " to create the PooledConnection.");
+                : password.equals(info.getPassword()))) {  // Password on PooledConnectionAndInfo does not match
+            try { // See if password has changed by attempting connection
+                testCPDS(username, password);
+            } catch (SQLException ex) {
+                // Password has not changed, so refuse client, but return connection to the pool
+                closeDueToException(info);
+                throw new SQLException("Given password did not match password used"
+                                       + " to create the PooledConnection.");
+            } catch (javax.naming.NamingException ne) {
+                throw (SQLException) new SQLException(
+                        "NamingException encountered connecting to database").initCause(ne);
+            }
+            /*
+             * Password must have changed -> destroy connection and keep retrying until we get a new, good one,
+             * destroying any idle connections with the old passowrd as we pull them from the pool.
+             */
+            final UserPassKey upkey = info.getUserPassKey();
+            final PooledConnectionManager manager = getConnectionManager(upkey);
+            manager.invalidate(info.getPooledConnection()); // Destroy and remove from pool
+            manager.setPassword(upkey.getPassword()); // Reset the password on the factory if using CPDSConnectionFactory
+            info = null;
+            for (int i = 0; i < 10; i++) { // Bound the number of retries - only needed if bad instances return 
+                try {
+                    info = getPooledConnectionAndInfo(username, password);
+                } catch (NoSuchElementException e) {
+                    closeDueToException(info);
+                    throw new SQLNestedException("Cannot borrow connection from pool", e);
+                } catch (RuntimeException e) {
+                    closeDueToException(info);
+                    throw e;
+                } catch (SQLException e) {            
+                    closeDueToException(info);
+                    throw e;
+                } catch (Exception e) {
+                    closeDueToException(info);
+                    throw new SQLNestedException("Cannot borrow connection from pool", e);
+                }
+                if (info != null && password.equals(info.getPassword())) {
+                    break;
+                } else {
+                    if (info != null) {
+                        manager.invalidate(info.getPooledConnection());
+                    }
+                    info = null;
+                }
+            }  
+            if (info == null) {
+                throw new SQLException("Cannot borrow connection from pool - password change failure.");
+            }
         }
 
         Connection con = info.getPooledConnection().getConnection();

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/KeyedCPDSConnectionFactory.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/KeyedCPDSConnectionFactory.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/KeyedCPDSConnectionFactory.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/KeyedCPDSConnectionFactory.java Sat Feb  6 19:42:58 2010
@@ -30,7 +30,6 @@
 import javax.sql.ConnectionPoolDataSource;
 import javax.sql.PooledConnection;
 
-import org.apache.commons.dbcp.SQLNestedException;
 import org.apache.commons.pool.KeyedObjectPool;
 import org.apache.commons.pool.KeyedPoolableObjectFactory;
 
@@ -42,16 +41,16 @@
  * @version $Revision$ $Date$
  */
 class KeyedCPDSConnectionFactory
-    implements KeyedPoolableObjectFactory, ConnectionEventListener {
+    implements KeyedPoolableObjectFactory, ConnectionEventListener, PooledConnectionManager {
 
     private static final String NO_KEY_MESSAGE
             = "close() was called on a Connection, but "
             + "I have no record of the underlying PooledConnection.";
 
-    protected ConnectionPoolDataSource _cpds = null;
-    protected volatile String _validationQuery = null;
-    protected volatile boolean _rollbackAfterValidation = false;
-    protected volatile KeyedObjectPool _pool = null;
+    private final ConnectionPoolDataSource _cpds;
+    private final String _validationQuery;
+    private final boolean _rollbackAfterValidation;
+    private final KeyedObjectPool _pool;
     
     /** 
      * Map of PooledConnections for which close events are ignored.
@@ -74,10 +73,7 @@
     public KeyedCPDSConnectionFactory(ConnectionPoolDataSource cpds,
                                       KeyedObjectPool pool,
                                       String validationQuery) {
-        _cpds = cpds;
-        _pool = pool;
-        pool.setFactory(this);
-        _validationQuery = validationQuery;
+        this(cpds , pool, validationQuery, false);  
     }
 
     /**
@@ -95,60 +91,19 @@
                                       KeyedObjectPool pool, 
                                       String validationQuery,
                                       boolean rollbackAfterValidation) {
-        this(cpds , pool, validationQuery);
-        _rollbackAfterValidation = rollbackAfterValidation;
-    }
-
-    /**
-     * Sets the {@link ConnectionPoolDataSource} from which to obtain base {@link Connection}s.
-     * @param cpds the {@link ConnectionPoolDataSource} from which to obtain base {@link Connection}s
-     */
-    synchronized public void setCPDS(ConnectionPoolDataSource cpds) {
         _cpds = cpds;
-    }
-
-    /**
-     * Sets the query I use to {*link #validateObject validate} {*link Connection}s.
-     * Should return at least one row.
-     * May be <code>null</code>
-     * @param validationQuery a query to use to {*link #validateObject validate} {*link Connection}s.
-     */
-    public void setValidationQuery(String validationQuery) {
+        _pool = pool;
+        pool.setFactory(this);
         _validationQuery = validationQuery;
-    }
-
-    /**
-     * Sets whether a rollback should be issued after 
-     * {@link #validateObject validating} 
-     * {@link Connection}s.
-     * @param rollbackAfterValidation whether a rollback should be issued after
-     *        {@link #validateObject validating} 
-     *        {@link Connection}s.
-     */
-    public void setRollbackAfterValidation(
-            boolean rollbackAfterValidation) {
         _rollbackAfterValidation = rollbackAfterValidation;
     }
-
+    
     /**
-     * Sets the {*link ObjectPool} in which to pool {*link Connection}s.
-     * @param pool the {*link ObjectPool} in which to pool those {*link Connection}s
+     * Returns the keyed object pool used to pool connections created by this factory.
+     * 
+     * @return KeyedObjectPool managing pooled connections
      */
-    synchronized public void setPool(KeyedObjectPool pool)
-        throws SQLException {
-        if (null != _pool && pool != _pool) {
-            try {
-                _pool.close();
-            } catch (RuntimeException e) {
-                throw e;
-            } catch (Exception e) {
-                throw new SQLNestedException("Cannot set the pool on this factory", e);
-            }
-        }
-        _pool = pool;
-    }
-
-    public synchronized KeyedObjectPool getPool() {
+    public KeyedObjectPool getPool() {
         return _pool;
     }
 
@@ -334,4 +289,49 @@
             e.printStackTrace();
         }
     }
+    
+    // ***********************************************************************
+    // PooledConnectionManager implementation
+    // ***********************************************************************
+    
+    /**
+     * Invalidates the PooledConnection in the pool.  The KeyedCPDSConnectionFactory
+     * closes the connection and pool counters are updated appropriately.
+     * Also clears any idle instances associated with the username that was used
+     * to create the PooledConnection.  Connections associated with this user
+     * are not affected and they will not be automatically closed on return to the pool.
+     */
+    public void invalidate(PooledConnection pc) throws SQLException {
+        PooledConnectionAndInfo info = (PooledConnectionAndInfo) pcMap.get(pc);
+        if (info == null) {
+            throw new IllegalStateException(NO_KEY_MESSAGE);
+        }
+        UserPassKey key = info.getUserPassKey();
+        try {
+            _pool.invalidateObject(key, info);  // Destroy and update pool counters
+            _pool.clear(key); // Remove any idle instances with this key
+        } catch (Exception ex) {
+            throw (SQLException) new SQLException("Error invalidating connection").initCause(ex);
+        }
+    }
+    
+    /**
+     * Does nothing.  This factory does not cache user credentials.
+     */
+    public void setPassword(String password) {
+    }
+    
+    /**
+     * This implementation does not fully close the KeyedObjectPool, as
+     * this would affect all users.  Instead, it clears the pool associated
+     * with the given user.  This method is not currently used.
+     */
+    public void closePool(String username) throws SQLException {
+        try {
+            _pool.clear(new UserPassKey(username, null));
+        } catch (Exception ex) {
+            throw (SQLException) new SQLException("Error closing connection pool").initCause(ex);
+        } 
+    }
+    
 }

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PerUserPoolDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PerUserPoolDataSource.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PerUserPoolDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PerUserPoolDataSource.java Sat Feb  6 19:42:58 2010
@@ -24,27 +24,35 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.NoSuchElementException;
 
 import javax.naming.NamingException;
 import javax.naming.Reference;
 import javax.naming.StringRefAddr;
 import javax.sql.ConnectionPoolDataSource;
 
+import org.apache.commons.dbcp.SQLNestedException;
+
 import org.apache.commons.pool.ObjectPool;
 import org.apache.commons.pool.impl.GenericObjectPool;
-import org.apache.commons.dbcp.SQLNestedException;
 
 /**
- * <p>
- * A pooling <code>DataSource</code> appropriate for deployment within
+ * <p>A pooling <code>DataSource</code> appropriate for deployment within
  * J2EE environment.  There are many configuration options, most of which are
  * defined in the parent class.  This datasource uses individual pools per 
  * user, and some properties can be set specifically for a given user, if the 
  * deployment environment can support initialization of mapped properties.
  * So for example, a pool of admin or write-access Connections can be
  * guaranteed a certain number of connections, separate from a maximum
- * set for users with read-only connections. 
- * </p>
+ * set for users with read-only connections.</p>
+ * 
+ * <p>User passwords can be changed without re-initializing the datasource.
+ * When a <code>getConnection(username, password)</code> request is processed 
+ * with a password that is different from those used to create connections in the
+ * pool associated with <code>username</code>, an attempt is made to create a
+ * new connection using the supplied password and if this succeeds, the existing
+ * pool is cleared and a new pool is created for connections using the new password.</p>
+ * 
  *
  * @author John D. McNally
  * @version $Revision$ $Date$
@@ -68,7 +76,7 @@
     /**
      * Map to keep track of Pools for a given user
      */
-    private transient Map /* <PoolKey, ObjectPool> */ pools = new HashMap();
+    private transient Map /* <PoolKey, PooledConnectionManager> */ managers = new HashMap();
 
     /**
      * Default no-arg constructor for Serialization
@@ -80,10 +88,10 @@
      * Close pool(s) being maintained by this datasource.
      */
     public void close() {
-        for (Iterator poolIter = pools.values().iterator();
+        for (Iterator poolIter = managers.values().iterator();
              poolIter.hasNext();) {    
             try {
-                ((ObjectPool) poolIter.next()).close();
+              ((CPDSConnectionFactory) poolIter.next()).getPool().close();
             } catch (Exception closePoolException) {
                     //ignore and try to close others.
             }
@@ -337,7 +345,7 @@
      * Get the number of active connections in the pool for a given user.
      */
     public int getNumActive(String username, String password) {
-        ObjectPool pool = (ObjectPool)pools.get(getPoolKey(username,password));
+        ObjectPool pool = getPool(getPoolKey(username,password));
         return (pool == null) ? 0 : pool.getNumActive();
     }
 
@@ -352,7 +360,7 @@
      * Get the number of idle connections in the pool for a given user.
      */
     public int getNumIdle(String username, String password) {
-        ObjectPool pool = (ObjectPool)pools.get(getPoolKey(username,password));
+        ObjectPool pool = getPool(getPoolKey(username,password));
         return (pool == null) ? 0 : pool.getNumIdle();
     }
 
@@ -364,29 +372,56 @@
         getPooledConnectionAndInfo(String username, String password)
         throws SQLException {
 
-        PoolKey key = getPoolKey(username,password);
-        Object pool;
+        final PoolKey key = getPoolKey(username,password);
+        ObjectPool pool;
+        PooledConnectionManager manager;
         synchronized(this) {
-            pool = pools.get(key);
-            if (pool == null) {
+            manager = (PooledConnectionManager) managers.get(key);
+            if (manager == null) {
                 try {
                     registerPool(username, password);
-                    pool = pools.get(key);
+                    manager = (PooledConnectionManager) managers.get(key);
                 } catch (NamingException e) {
                     throw new SQLNestedException("RegisterPool failed", e);
                 }
             }
+            pool = ((CPDSConnectionFactory) manager).getPool();
         }
 
         PooledConnectionAndInfo info = null;
         try {
-            info = (PooledConnectionAndInfo)((ObjectPool) pool).borrowObject();
+            info = (PooledConnectionAndInfo) pool.borrowObject();
         }
-        catch (Exception e) {
+        catch (NoSuchElementException ex) {
             throw new SQLNestedException(
-                "Could not retrieve connection info from pool", e);
+                    "Could not retrieve connection info from pool", ex);
+        }
+        catch (Exception e) {
+            // See if failure is due to CPDSConnectionFactory authentication failure
+            try {
+                testCPDS(username, password);
+            } catch (Exception ex) {
+                throw (SQLException) new SQLException(
+                        "Could not retrieve connection info from pool").initCause(ex);
+            }
+            // New password works, so kill the old pool, create a new one, and borrow
+            manager.closePool(username);
+            synchronized (this) {
+                managers.remove(key);
+            }
+            try {
+                registerPool(username, password);
+                pool = getPool(key);
+            } catch (NamingException ne) {
+                throw new SQLNestedException("RegisterPool failed", ne);
+            }
+            try {
+                info = (PooledConnectionAndInfo)((ObjectPool) pool).borrowObject();
+            } catch (Exception ex) {
+                throw (SQLException) new SQLException(
+                "Could not retrieve connection info from pool").initCause(ex);
+            }
         }
-        
         return info;
     }
 
@@ -428,6 +463,11 @@
             con.setReadOnly(defaultReadOnly);
         }
     }
+    
+    protected PooledConnectionManager getConnectionManager(UserPassKey upkey) {
+        return (PooledConnectionManager) managers.get(getPoolKey(
+                upkey.getUsername(), upkey.getPassword()));
+    }
 
     /**
      * Returns a <code>PerUserPoolDataSource</code> {@link Reference}.
@@ -442,7 +482,7 @@
     }
     
     private PoolKey getPoolKey(String username, String password) { 
-        return new PoolKey(getDataSourceName(), username+password); 
+        return new PoolKey(getDataSourceName(), username); 
     }
 
     private synchronized void registerPool(
@@ -478,11 +518,10 @@
         // Set up the factory we will use (passing the pool associates
         // the factory with the pool, so we do not have to do so
         // explicitly)
-        new CPDSConnectionFactory(cpds, pool, getValidationQuery(),
-                                  isRollbackAfterValidation(), 
-                                  username, password);
+        CPDSConnectionFactory factory = new CPDSConnectionFactory(cpds, pool, getValidationQuery(),
+                isRollbackAfterValidation(), username, password);
            
-        Object old = pools.put(getPoolKey(username,password), pool);
+        Object old = managers.put(getPoolKey(username,password), factory);
         if (old != null) {
             throw new IllegalStateException("Pool already contains an entry for this user/password: "+username);
         }
@@ -503,11 +542,23 @@
             PerUserPoolDataSource oldDS = (PerUserPoolDataSource)
                 new PerUserPoolDataSourceFactory()
                     .getObjectInstance(getReference(), null, null, null);
-            this.pools = oldDS.pools;
+            this.managers = oldDS.managers;
         }
         catch (NamingException e)
         {
             throw new IOException("NamingException: " + e);
         }
     }
+    
+    /**
+     * Returns the object pool associated with the given PoolKey.
+     * 
+     * @param key PoolKey identifying the pool 
+     * @return the GenericObjectPool pooling connections for the username and datasource
+     * specified by the PoolKey
+     */
+    private GenericObjectPool getPool(PoolKey key) {
+        CPDSConnectionFactory mgr = (CPDSConnectionFactory) managers.get(key);
+        return mgr == null ? null : (GenericObjectPool) mgr.getPool();
+    }
 }

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PoolKey.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PoolKey.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PoolKey.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PoolKey.java Sat Feb  6 19:42:58 2010
@@ -26,18 +26,18 @@
     private static final long serialVersionUID = 2252771047542484533L;
 
     private final String datasourceName;
-    private final String userPwd;
+    private final String username;
     
-    PoolKey(String datasourceName, String userPwd) {
+    PoolKey(String datasourceName, String username) {
         this.datasourceName = datasourceName;
-        this.userPwd = userPwd;
+        this.username = username;
     }
     
     public boolean equals(Object obj) {
         if (obj instanceof PoolKey) {
             PoolKey pk = (PoolKey)obj;
             return (null == datasourceName ? null == pk.datasourceName : datasourceName.equals(pk.datasourceName)) &&
-                (null == userPwd ? null == pk.userPwd : userPwd.equals(pk.userPwd));
+                (null == username ? null == pk.username : username.equals(pk.username));
         } else {
             return false;   
         }
@@ -48,8 +48,8 @@
         if (datasourceName != null) {
             h += datasourceName.hashCode();
         }
-        if (userPwd != null) {
-            h = 29 * h + userPwd.hashCode();
+        if (username != null) {
+            h = 29 * h + username.hashCode();
         }
         return h;
     }
@@ -57,7 +57,7 @@
     public String toString() {
         StringBuffer sb = new StringBuffer(50);
         sb.append("PoolKey(");
-        sb.append(userPwd).append(", ").append(datasourceName);
+        sb.append(username).append(", ").append(datasourceName);
         sb.append(')');
         return sb.toString();
     }

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionAndInfo.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionAndInfo.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionAndInfo.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionAndInfo.java Sat Feb  6 19:42:58 2010
@@ -20,6 +20,9 @@
 import javax.sql.PooledConnection;
 
 /**
+ * Immutable poolable object holding a PooledConnection along with the username and password 
+ * used to create the connection.
+ * 
  * @version $Revision$ $Date$
  */
 final class PooledConnectionAndInfo {
@@ -27,9 +30,8 @@
     private final String password;
     private final String username;
     private final UserPassKey upkey;
-
-    PooledConnectionAndInfo(PooledConnection pc, 
-                            String username, String password) {
+    
+    PooledConnectionAndInfo(PooledConnection pc, String username, String password) {
         this.pooledConnection = pc;
         this.username = username;
         this.password = password;
@@ -59,4 +61,5 @@
     final String getUsername() {
         return username;
     }
+    
 }

Added: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java?rev=907288&view=auto
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java (added)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java Sat Feb  6 19:42:58 2010
@@ -0,0 +1,55 @@
+/*
+ * 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.dbcp.datasources;
+
+import java.sql.SQLException;
+import javax.sql.PooledConnection;
+
+/**
+ * Methods to manage PoolableConnections and the connection pools that source them.
+ * 
+ * @since 1.3
+ * @version $Revision$ $Date$
+ */
+interface PooledConnectionManager {
+    /**
+     * Close the PooledConnection and remove it from the connection pool
+     * to which it belongs, adjusting pool counters.
+     * 
+     * @param pc PooledConnection to be invalidated
+     * @throws SQLException if an SQL error occurs closing the connection
+     */
+    void invalidate(PooledConnection pc) throws SQLException;
+    
+    /**
+     * Sets the databsase password used when creating connections.
+     * 
+     * @param password password used when authenticating to the database
+     */
+    void setPassword(String password);
+    
+    
+    /**
+     * Closes the connection pool associated with the given user.
+     * 
+     * @param username user name
+     * @throws SQLException if an error occurs closing idle connections in the pool
+     */
+    void closePool(String username) throws SQLException;
+    
+}

Propchange: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/PooledConnectionManager.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/SharedPoolDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/SharedPoolDataSource.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/SharedPoolDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/SharedPoolDataSource.java Sat Feb  6 19:42:58 2010
@@ -33,10 +33,18 @@
 import org.apache.commons.dbcp.SQLNestedException;
 
 /**
- * A pooling <code>DataSource</code> appropriate for deployment within
+ * <p>A pooling <code>DataSource</code> appropriate for deployment within
  * J2EE environment.  There are many configuration options, most of which are
  * defined in the parent class. All users (based on username) share a single 
- * maximum number of Connections in this datasource.
+ * maximum number of Connections in this datasource.</p>
+ * 
+ * <p>User passwords can be changed without re-initializing the datasource.
+ * When a <code>getConnection(username, password)</code> request is processed 
+ * with a password that is different from those used to create connections in the
+ * pool associated with <code>username</code>, an attempt is made to create a
+ * new connection using the supplied password and if this succeeds, idle connections
+ * created using the old password are destroyed and new connections are created
+ * using the new password.</p>
  *
  * @author John D. McNally
  * @version $Revision$ $Date$
@@ -50,7 +58,8 @@
     private int maxIdle = GenericObjectPool.DEFAULT_MAX_IDLE;
     private int maxWait = (int)Math.min(Integer.MAX_VALUE,
         GenericObjectPool.DEFAULT_MAX_WAIT);
-    private KeyedObjectPool pool = null;
+    private transient KeyedObjectPool pool = null;
+    private transient KeyedCPDSConnectionFactory factory = null;
 
     /**
      * Default no-arg constructor for Serialization
@@ -177,6 +186,10 @@
         }
         return info;
     }
+    
+    protected PooledConnectionManager getConnectionManager(UserPassKey upkey)  {
+        return factory;
+    }
 
     /**
      * Returns a <code>SharedPoolDataSource</code> {@link Reference}.
@@ -213,7 +226,7 @@
         // Set up the factory we will use (passing the pool associates
         // the factory with the pool, so we do not have to do so
         // explicitly)
-        new KeyedCPDSConnectionFactory(cpds, pool, getValidationQuery(),
+        factory = new KeyedCPDSConnectionFactory(cpds, pool, getValidationQuery(),
                                        isRollbackAfterValidation());
     }
 

Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/UserPassKey.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/UserPassKey.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/UserPassKey.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/datasources/UserPassKey.java Sat Feb  6 19:42:58 2010
@@ -20,7 +20,14 @@
 import java.io.Serializable;
 
 /**
- * Holds a username, password pair.
+ * <p>Holds a username, password pair.  Serves as a poolable object key for the KeyedObjectPool
+ * backing a SharedPoolDataSource.  Two instances with the same username are considered equal.
+ * This ensures that there will be only one keyed pool for each user in the pool.  The password
+ * is used (along with the username) by the KeyedCPDSConnectionFactory when creating new connections.</p>
+ * 
+ * <p>{@link InstanceKeyDataSource#getConnection(String, String)} validates that the password used to create
+ * a connection matches the password provided by the client.</p>
+ * 
  * @version $Revision$ $Date$
  */
 class UserPassKey implements Serializable {
@@ -50,8 +57,10 @@
     }
     
     /**
-     * @return <code>true</code> if the username and password fields for both 
-     * objects are equal.
+     * @return <code>true</code> if the username fields for both 
+     * objects are equal.  Two instances with the same username
+     * but different passwords are considered equal.
+     * 
      * @see java.lang.Object#equals(java.lang.Object)
      */
     public boolean equals(Object obj) {
@@ -69,22 +78,17 @@
         
         UserPassKey key = (UserPassKey) obj;
         
-        boolean usersEqual =
-            (this.username == null
-                ? key.username == null
-                : this.username.equals(key.username));
-                
-        boolean passwordsEqual =
-            (this.password == null
-                ? key.password == null
-                : this.password.equals(key.password));
-
-        return (usersEqual && passwordsEqual);
+        return this.username == null ?
+                key.username == null :
+                this.username.equals(key.username);       
     }
 
+    /**
+     * Returns the hash of the username. 
+     */
     public int hashCode() {
         return (this.username != null ?
-                (this.username + this.password).hashCode() : 0);
+                (this.username).hashCode() : 0);
     }
 
     public String toString() {

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterDriver.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterDriver.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterDriver.java (original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterDriver.java Sat Feb  6 19:42:58 2010
@@ -58,7 +58,9 @@
      * TesterDriver specific method to add users to the list of valid users 
      */
     public static void addUser(String username, String password) {
-        validUserPasswords.put(username, password);
+        synchronized (validUserPasswords) {
+            validUserPasswords.put(username, password);
+        }
     }
 
     public boolean acceptsURL(String url) throws SQLException {
@@ -67,17 +69,15 @@
 
     private void assertValidUserPassword(String user, String password) 
         throws SQLException {
-        String realPassword = validUserPasswords.getProperty(user);
-        if (realPassword == null) 
-        {
-            throw new SQLException(user + " is not a valid username.");
-        }
-        if (!realPassword.equals(password)) 
-        {
-            throw new SQLException(password + 
-                                   " is not the correct password for " +
-                                   user + ".  The correct password is " +
-                                   realPassword);
+        synchronized (validUserPasswords) {
+            String realPassword = validUserPasswords.getProperty(user);
+            if (realPassword == null) {
+                throw new SQLException(user + " is not a valid username.");
+            }
+            if (!realPassword.equals(password)) {
+                throw new SQLException(password + " is not the correct password for " + user
+                        + ".  The correct password is " + realPassword);
+            }
         }
     }
 

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestPerUserPoolDataSource.java Sat Feb  6 19:42:58 2010
@@ -105,7 +105,7 @@
         }
         catch (SQLException e)
         {
-            if (!e.getMessage().startsWith("x is not the correct password")) 
+            if (!e.getMessage().startsWith("Given password did not match")) 
             {
                 throw e;
             }
@@ -518,16 +518,32 @@
             fail("Should have generated SQLException");
         } catch (SQLException expected) {
         }
-        ds.getConnection("foo", "bar").close();
+        Connection con1 = ds.getConnection("foo", "bar");
+        Connection con2 = ds.getConnection("foo", "bar");
+        Connection con3 = ds.getConnection("foo", "bar");
+        con1.close();
+        con2.close();
         TesterDriver.addUser("foo","bay"); // change the user/password setting
         try {
-            ds.getConnection("foo", "bay").close(); // new password
+            Connection con4 = ds.getConnection("foo", "bay"); // new password
+            // Idle instances with old password should have been cleared
+            assertEquals("Should be no idle connections in the pool", 
+                    0, ((PerUserPoolDataSource) ds).getNumIdle("foo", "bar"));
+            con4.close();
+            // Should be one idle instance with new pwd
+            assertEquals("Should be one idle connection in the pool", 
+                    1, ((PerUserPoolDataSource) ds).getNumIdle("foo", "bay"));
             try {
                 ds.getConnection("foo", "bar").close(); // old password
-                System.out.println("Should have generated SQLException"); // TODO should be fail()
+                fail("Should have generated SQLException"); 
             } catch (SQLException expected) {
             }
-            ds.getConnection("foo", "bay").close();
+            Connection con5 = ds.getConnection("foo", "bay"); // take the idle one
+            con3.close(); // Return a connection with the old password
+            ds.getConnection("foo", "bay").close();  // will try bad returned connection and destroy it
+            assertEquals("Should be one idle connection in the pool", 
+                    1, ((PerUserPoolDataSource) ds).getNumIdle("foo", "bar"));
+            con5.close();
         } finally {
             TesterDriver.addUser("foo","bar");
         }

Modified: commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java?rev=907288&r1=907287&r2=907288&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/datasources/TestSharedPoolDataSource.java Sat Feb  6 19:42:58 2010
@@ -107,8 +107,7 @@
         }
         catch (SQLException e)
         {
-            Throwable t = e.getCause();
-            if (!t.getMessage().startsWith("x is not the correct password"))
+            if (!e.getMessage().startsWith("Given password did not match")) 
             {
                 throw e;
             }
@@ -559,16 +558,32 @@
             fail("Should have generated SQLException");
         } catch (SQLException expected) {
         }
-        ds.getConnection("foo", "bar").close();
+        Connection con1 = ds.getConnection("foo", "bar");
+        Connection con2 = ds.getConnection("foo", "bar");
+        Connection con3 = ds.getConnection("foo", "bar");
+        con1.close();
+        con2.close();
         TesterDriver.addUser("foo","bay"); // change the user/password setting
         try {
-            ds.getConnection("foo", "bay").close(); // new password
+            Connection con4 = ds.getConnection("foo", "bay"); // new password
+            // Idle instances with old password should have been cleared
+            assertEquals("Should be no idle connections in the pool", 
+                   0, ((SharedPoolDataSource) ds).getNumIdle());
+            con4.close();
+            // Should be one idle instance with new pwd
+            assertEquals("Should be one idle connection in the pool", 
+                    1, ((SharedPoolDataSource) ds).getNumIdle());
             try {
                 ds.getConnection("foo", "bar").close(); // old password
-                System.out.println("Should have generated SQLException"); // TODO should be fail()
+                fail("Should have generated SQLException"); 
             } catch (SQLException expected) {
             }
-            ds.getConnection("foo", "bay").close();
+            Connection con5 = ds.getConnection("foo", "bay"); // take the idle one
+            con3.close(); // Return a connection with the old password
+            ds.getConnection("foo", "bay").close();  // will try bad returned connection and destroy it
+            assertEquals("Should be one idle connection in the pool", 
+                    1, ((SharedPoolDataSource) ds).getNumIdle());
+            con5.close();
         } finally {
             TesterDriver.addUser("foo","bar");
         }



Mime
View raw message