Mailing-List: contact ojb-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "OJB Developers List" <ojb-dev@db.apache.org>
Date: 7 Apr 2005 13:08:57 -0000
Message-ID: <20050407130857.44596.qmail@minotaur.apache.org>
From: mkalen@apache.org
To: db-ojb-cvs@apache.org
Subject: cvs commit: db-ojb/src/java/org/apache/ojb/broker/accesslayer
 ConnectionFactoryAbstractImpl.java ConnectionFactoryDBCPImpl.java
 ConnectionFactoryNotPooledImpl.java ConnectionFactoryPooledImpl.java

mkalen      2005/04/07 06:08:57

  Modified:    src/java/org/apache/ojb/broker/accesslayer Tag:
                        OJB_1_0_RELEASE ConnectionFactoryAbstractImpl.java
                        ConnectionFactoryDBCPImpl.java
                        ConnectionFactoryNotPooledImpl.java
                        ConnectionFactoryPooledImpl.java
  Log:
  http://issues.apache.org/scarab/issues/id/OJB301. Rename checkout/release methods and improve JavaDoc for better overview of ConnectionFactory operations. Add Connection checks in NotPooled factory. Add TODO why I think parts of Ilkka's patch should be rolled out.
  
  Revision  Changes    Path
  No                   revision
  No                   revision
  1.10.2.3  +46 -36    db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryAbstractImpl.java
  
  Index: ConnectionFactoryAbstractImpl.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryAbstractImpl.java,v
  retrieving revision 1.10.2.2
  retrieving revision 1.10.2.3
  diff -u -r1.10.2.2 -r1.10.2.3
  --- ConnectionFactoryAbstractImpl.java	16 Mar 2005 17:47:19 -0000	1.10.2.2
  +++ ConnectionFactoryAbstractImpl.java	7 Apr 2005 13:08:57 -0000	1.10.2.3
  @@ -48,25 +48,44 @@
       private Map dataSourceCache = new HashMap();
   
       /**
  -     * Implement this method. This method was called to obtain
  -     * a jdbc-connection from the pool.
  -     * <br/>
  -     * Note: This method was
  -     * not called, if jdbc-connection-descriptor use datasources - OJB
  -     * only pool connections from DriverManager.
  +     * Returns a valid JDBC Connection. Implement this method in concrete subclasses.
  +     * Concrete implementations using Connection pooling are responsible for any validation
  +     * and pool removal management.
  +     * <p>
  +     * Note: This method is never called for a jdbc-connection-descriptor that uses datasources,
  +     * OJB only manages connections from DriverManager.
  +     * <p>
  +     * Note: If the concrete implementation does not callback to
  +     * {@link #newConnectionFromDriverManager(org.apache.ojb.broker.metadata.JdbcConnectionDescriptor)}
  +     * when creating a new Connection, it <em>must</em> call
  +     * {@link #initializeJdbcConnection(java.sql.Connection, org.apache.ojb.broker.metadata.JdbcConnectionDescriptor)}
  +     * so that the platform implementation can peform any RDBMS-specific init tasks for newly
  +     * created Connection objetcs.
  +     *
  +     * @param jcd the connection descriptor for which to return a validated Connection
  +     * @return a valid Connection, never null.
  +     * Specific implementations <em>must</em> guarantee that the connection is not null and
  +     * that it is valid.
  +     * @throws LookupException if a valid Connection could not be obtained
        */
  -    public abstract Connection getConnectionFromPool(JdbcConnectionDescriptor jcd)
  +    public abstract Connection checkOutJdbcConnection(JdbcConnectionDescriptor jcd)
               throws LookupException;
   
       /**
  -     * Implement this method. Was called to return a
  -     * connection to pool.
  -     * <br/>
  -     * Note: This method was
  -     * not called, if the jdbc-connection-descriptor uses datasources - OJB
  -     * only pool connections from DriverManager.
  +     * Releases a Connection after use. Implement this method in concrete subclasses.
  +     * Concrete implementations using Connection pooling are responsible for any validation
  +     * and pool removal management.
  +     * <p>
  +     * Note: This method is never called for a jdbc-connection-descriptor that uses datasources,
  +     * OJB only manages connections from DriverManager.
  +     *
  +     * @param jcd the connection descriptor for which the connection was created
  +     * @param con the connection to release.
  +     * Callers <em>must</em> guarantee that the passed connection was obtained by calling
  +     * {@link #checkOutJdbcConnection(org.apache.ojb.broker.metadata.JdbcConnectionDescriptor)}.
  +     * @throws LookupException //TODO: mkalen: document when. Rename from "LookupException"?
        */
  -    public abstract void returnConnectionToPool(JdbcConnectionDescriptor jcd, Connection con)
  +    public abstract void releaseJdbcConnection(JdbcConnectionDescriptor jcd, Connection con)
               throws LookupException;
   
       public void releaseConnection(JdbcConnectionDescriptor jcd, Connection con)
  @@ -87,7 +106,7 @@
           {
               try
               {
  -                returnConnectionToPool(jcd, con);
  +                releaseJdbcConnection(jcd, con);
               }
               catch (LookupException e)
               {
  @@ -115,25 +134,8 @@
           }
           else
           {
  -            conn = getConnectionFromPool(jcd);
  -            // check connection
  -            try
  -            {
  -                if (conn == null || conn.isClosed())
  -                {
  -                    log.error("Connection for JdbcConnectionDescriptor (" +
  -                            (jcd.getDatasourceName() != null ? "datasource: " + jcd.getDatasourceName() :
  -                            "db-url: " + getDbURL(jcd) + ", user: " + jcd.getUserName()) +
  -                            ") is invalid: " +
  -                            (conn != null ? "*closed*" : "*null*"));
  -                    throw new LookupException("Could not lookup valid connection from pool/DB, connection was "+conn);
  -                }
  -            }
  -            catch (SQLException e)
  -            {
  -                log.error("Error during sanity check of new DB Connection", e);
  -                throw new LookupException("Connection check failed", e);
  -            }
  +            conn = checkOutJdbcConnection(jcd);
  +            // connection is now guaranteed to be valid by API contract (else exception is thrown)
           }
           return conn;
       }
  @@ -141,7 +143,7 @@
       /**
        * Initialize the connection with the specified properties in OJB
        * configuration files and platform depended properties.
  -     * Invoke this method after a NEW connection was created.
  +     * Invoke this method after a NEW connection is created, not if re-using from pool.
        *
        * @see org.apache.ojb.broker.platforms.PlatformFactory
        * @see org.apache.ojb.broker.platforms.Platform
  @@ -294,4 +296,12 @@
                   jcd.getProtocol() + ":" + jcd.getSubProtocol() + ":" + jcd.getDbAlias();
       }
   
  +    protected String getJcdDescription(JdbcConnectionDescriptor jcd)
  +    {
  +        return "Connection for JdbcConnectionDescriptor (" +
  +               (jcd.getDatasourceName() != null ? "datasource: " + jcd.getDatasourceName() :
  +                "db-url: " + getDbURL(jcd) + ", user: " + jcd.getUserName()) +
  +               ")";
  +    }
  +
   }
  
  
  
  1.10.2.2  +3 -3      db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryDBCPImpl.java
  
  Index: ConnectionFactoryDBCPImpl.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryDBCPImpl.java,v
  retrieving revision 1.10.2.1
  retrieving revision 1.10.2.2
  diff -u -r1.10.2.1 -r1.10.2.2
  --- ConnectionFactoryDBCPImpl.java	16 Mar 2005 17:51:18 -0000	1.10.2.1
  +++ ConnectionFactoryDBCPImpl.java	7 Apr 2005 13:08:57 -0000	1.10.2.2
  @@ -61,7 +61,7 @@
       /** Synchronize object for operations not synchronized on Map only. */
       private Object poolSynch = new Object();
   
  -    public Connection getConnectionFromPool(JdbcConnectionDescriptor jcd) throws LookupException
  +    public Connection checkOutJdbcConnection(JdbcConnectionDescriptor jcd) throws LookupException
       {
           final DataSource ds = getDataSource(jcd);
   
  @@ -80,7 +80,7 @@
           return conn;
       }
   
  -    public void returnConnectionToPool(JdbcConnectionDescriptor jcd, Connection con)
  +    public void releaseJdbcConnection(JdbcConnectionDescriptor jcd, Connection con)
               throws LookupException
       {
           try
  
  
  
  1.5.2.1   +36 -7     db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryNotPooledImpl.java
  
  Index: ConnectionFactoryNotPooledImpl.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryNotPooledImpl.java,v
  retrieving revision 1.5
  retrieving revision 1.5.2.1
  diff -u -r1.5 -r1.5.2.1
  --- ConnectionFactoryNotPooledImpl.java	4 Apr 2004 23:53:30 -0000	1.5
  +++ ConnectionFactoryNotPooledImpl.java	7 Apr 2005 13:08:57 -0000	1.5.2.1
  @@ -1,6 +1,6 @@
   package org.apache.ojb.broker.accesslayer;
   
  -/* Copyright 2002-2004 The Apache Software Foundation
  +/* Copyright 2002-2005 The Apache Software Foundation
    *
    * Licensed under the Apache License, Version 2.0 (the "License");
    * you may not use this file except in compliance with the License.
  @@ -32,15 +32,43 @@
   {
       private Logger log = LoggerFactory.getLogger(ConnectionFactoryNotPooledImpl.class);
   
  -    public Connection getConnectionFromPool(JdbcConnectionDescriptor jcd) throws LookupException
  +    public Connection checkOutJdbcConnection(JdbcConnectionDescriptor jcd) throws LookupException
       {
           if (log.isDebugEnabled())
  -            log.debug(
  -                    "getConnectionFromPool was called, this implementation ever return a new connection");
  -        return this.newConnectionFromDriverManager(jcd);
  +        {
  +            log.debug("checkOutJdbcConnection: this implementation always return a new Connection");
  +        }
  +        final Connection conn = newConnectionFromDriverManager(jcd);
  +        validateConnection(conn, jcd);
  +        // Connection is now guaranteed to be valid (else validateConnection must throw exception)
  +        return conn;
       }
   
  -    public void returnConnectionToPool(JdbcConnectionDescriptor jcd, Connection con)
  +    protected void validateConnection(Connection conn, JdbcConnectionDescriptor jcd)
  +            throws LookupException
  +    {
  +        if (conn == null)
  +        {
  +            log.error(getJcdDescription(jcd) + " failed, DriverManager returned null");
  +            throw new LookupException("No Connection returned from DriverManager");
  +        }
  +        try
  +        {
  +            if (conn.isClosed())
  +            {
  +                log.error(getJcdDescription(jcd) + " is invalid (closed)");
  +                throw new LookupException("Could not create valid connection, connection was " +
  +                                          conn);
  +            }
  +        }
  +        catch (SQLException e)
  +        {
  +            log.error(getJcdDescription(jcd) + " failed validation with exception");
  +            throw new LookupException("Connection validation failed", e);
  +        }
  +    }
  +
  +    public void releaseJdbcConnection(JdbcConnectionDescriptor jcd, Connection con)
               throws LookupException
       {
           try
  @@ -52,4 +80,5 @@
               log.warn("Connection.close() failed, message was " + e.getMessage());
           }
       }
  +
   }
  
  
  
  1.15.2.4  +32 -21    db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryPooledImpl.java
  
  Index: ConnectionFactoryPooledImpl.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/ConnectionFactoryPooledImpl.java,v
  retrieving revision 1.15.2.3
  retrieving revision 1.15.2.4
  diff -u -r1.15.2.3 -r1.15.2.4
  --- ConnectionFactoryPooledImpl.java	16 Mar 2005 18:04:52 -0000	1.15.2.3
  +++ ConnectionFactoryPooledImpl.java	7 Apr 2005 13:08:57 -0000	1.15.2.4
  @@ -43,15 +43,16 @@
    */
   public class ConnectionFactoryPooledImpl extends ConnectionFactoryAbstractImpl
   {
  +
       private Logger log = LoggerFactory.getLogger(ConnectionFactoryPooledImpl.class);
       private Map connectionPools = new HashMap();
   
       public ConnectionFactoryPooledImpl()
       {
  -
  +        // default no-arg constructor
       }
   
  -    public void returnConnectionToPool(JdbcConnectionDescriptor jcd, Connection con)
  +    public void releaseJdbcConnection(JdbcConnectionDescriptor jcd, Connection con)
               throws LookupException
       {
           try
  @@ -62,8 +63,10 @@
                * connections to pool. We do this test independent from the from the
                * commons-pool settings, which also supports validation on return of
                * a connecion.
  +             * TODO: mkalen: this breaks the ObjectPool API contract and should only be
  +             * TODO: mkalen: handled when using testOnReturn=true, awaiting user feedback to remove
                */
  -            if(!con.isClosed())
  +            if (!con.isClosed())
               {
                   ((ObjectPool) this.connectionPools.get(jcd.getPBKey())).returnObject(con);
               }
  @@ -74,7 +77,7 @@
           }
       }
   
  -    public Connection getConnectionFromPool(JdbcConnectionDescriptor jcd) throws LookupException
  +    public Connection checkOutJdbcConnection(JdbcConnectionDescriptor jcd) throws LookupException
       {
           ObjectPool op = (ObjectPool) connectionPools.get(jcd.getPBKey());
           if (op == null)
  @@ -86,15 +89,17 @@
                   connectionPools.put(jcd.getPBKey(), op);
               }
           }
  +        final Connection conn;
           try
           {
  -            return (Connection) op.borrowObject();
  +            conn = (Connection) op.borrowObject();
           }
           catch (Exception e)
           {
               throw new LookupException("Could not borrow connection from pool - " +
                       JdbcConnectionDescriptor.class.getName() + ":  " + jcd, e);
           }
  +        return conn;
       }
   
       /**
  @@ -158,23 +163,29 @@
   
           public boolean validateObject(Object obj)
           {
  -            Connection con = (Connection) obj;
               boolean isValid = false;
  -            try
  -            {
  -                isValid = !con.isClosed();
  -            }
  -            catch (SQLException e)
  -            {
  -                log.warn("Connection validation failed: " + e.getMessage());
  -                if (log.isDebugEnabled()) log.debug(e);
  -                isValid = false;
  -            }
  -            final String validationQuery;
  -            validationQuery = jcd.getConnectionPoolDescriptor().getValidationQuery();
  -            if (isValid && validationQuery != null)
  +            if (obj != null)
               {
  -                isValid = validateConnection(con, validationQuery);
  +                final Connection con = (Connection) obj;
  +                try
  +                {
  +                    isValid = !con.isClosed();
  +                }
  +                catch (SQLException e)
  +                {
  +                    log.warn("Connection validation failed: " + e.getMessage());
  +                    if (log.isDebugEnabled()) log.debug(e);
  +                    isValid = false;
  +                }
  +                if (isValid)
  +                {
  +                    final String validationQuery;
  +                    validationQuery = jcd.getConnectionPoolDescriptor().getValidationQuery();
  +                    if (validationQuery != null)
  +                    {
  +                        isValid = validateConnection(con, validationQuery);
  +                    }
  +                }
               }
               return isValid;
           }
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org