db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From arm...@apache.org
Subject cvs commit: db-ojb/src/java/org/apache/ojb/broker/accesslayer JdbcAccess.java JdbcAccessImpl.java ResultSetAndStatement.java
Date Sat, 05 Nov 2005 16:15:01 GMT
arminw      2005/11/05 08:15:01

  Modified:    src/java/org/apache/ojb/broker/accesslayer JdbcAccess.java
                        JdbcAccessImpl.java ResultSetAndStatement.java
  Log:
  fix potential resource leaks
  improve #doesExist method
  
  Revision  Changes    Path
  1.39      +10 -4     db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccess.java
  
  Index: JdbcAccess.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccess.java,v
  retrieving revision 1.38
  retrieving revision 1.39
  diff -u -r1.38 -r1.39
  --- JdbcAccess.java	1 Oct 2005 13:56:13 -0000	1.38
  +++ JdbcAccess.java	5 Nov 2005 16:15:01 -0000	1.39
  @@ -118,10 +118,16 @@
       public void executeUpdate(ClassDescriptor cld, FieldDescriptor[] fieldsToUpdate, Object
obj) throws PersistenceBrokerException;
   
       /**
  -     * This method only checks if the requested object can be
  -     * found in database (without full object materialization).
  +     * This method checks if the requested object can be
  +     * found in database (without object materialization).
  +     *
  +     * @param cld The {@link org.apache.ojb.broker.metadata.ClassDescriptor} of the
  +     * object/{@link org.apache.ojb.broker.Identity} to check.
  +     * @param objectOrIdentity The <em>object</em> or the associated
  +     * {@link org.apache.ojb.broker.Identity} of the object
  +     * @return Return <em>true</em> if the object is already persisted, <em>false</em>
if the object is transient.
        */
  -    public boolean doesExist(ClassDescriptor cld, Identity oid);
  +    public boolean doesExist(ClassDescriptor cld, Object objectOrIdentity);
   
       /**
        * performs a primary key lookup operation against RDBMS and materializes
  
  
  
  1.42      +43 -85    db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccessImpl.java
  
  Index: JdbcAccessImpl.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/JdbcAccessImpl.java,v
  retrieving revision 1.41
  retrieving revision 1.42
  diff -u -r1.41 -r1.42
  --- JdbcAccessImpl.java	22 Oct 2005 12:19:31 -0000	1.41
  +++ JdbcAccessImpl.java	5 Nov 2005 16:15:01 -0000	1.42
  @@ -413,20 +413,27 @@
       }
   
       /**
  -     * This method only checks if the requested object can be
  -     * found in DB (without full object materialization).
  +     * @see JdbcAccess#doesExist(org.apache.ojb.broker.metadata.ClassDescriptor, Object)
        */
  -    public boolean doesExist(ClassDescriptor cld, Identity oid)
  +    public boolean doesExist(ClassDescriptor cld, Object objectOrIdentity)
       {
           boolean result = false;
           String sql = broker.serviceSqlGenerator().getPreparedExistsStatement(cld).getStatement();
  +        ValueContainer[] values;
  +        if(objectOrIdentity instanceof Identity)
  +        {
  +            values = broker.serviceBrokerHelper().getKeyValues(cld, (Identity) objectOrIdentity);
  +        }
  +        else
  +        {
  +            values = broker.serviceBrokerHelper().getKeyValues(cld, objectOrIdentity);
  +        }
           StatementManager sm = broker.serviceStatementManager();
           PreparedStatement stmt = null;
           ResultSet rs = null;
           try
           {
               stmt = sm.getPreparedStatement(sql, false, StatementManager.FETCH_SIZE_NOT_APPLICABLE,
false);
  -            ValueContainer[] values = extractKeyValues(cld, oid);
               bindValues(stmt, values, 1, false);
               rs = stmt.executeQuery();
               result = rs.next();
  @@ -437,6 +444,7 @@
           }
           finally
           {
  +            // release resources
               sm.closeResources(stmt, rs);
           }
           return result;
  @@ -505,9 +513,9 @@
   
           final StatementManager stmtManager = broker.serviceStatementManager();
           final boolean scrollable = isScrollable(query);
  -        ResultSetAndStatement retval = null;
           SelectStatement sql = null;
  -
  +        PreparedStatement stmt = null;
  +        ResultSet rs = null;
           try
           {
               final int queryFetchSize = query.getFetchSize();
  @@ -520,10 +528,8 @@
                   sql = broker.serviceSqlGenerator().getPreparedSelectStatement(query, cld);
               }
               final boolean callableStmt = SqlHelper.isStoredProcedure(sql.getStatement());
  -            PreparedStatement stmt = stmtManager.getPreparedStatement(sql.getStatement(),
  +            stmt = stmtManager.getPreparedStatement(sql.getStatement(),
                       scrollable, queryFetchSize, callableStmt);
  -
  -            ResultSet rs;
               if(callableStmt)
               {
                   // Query implemented as a stored procedure, which must return a result
set.
  @@ -539,36 +545,19 @@
                   rs = stmt.executeQuery();
               }
   
  -            retval = new ResultSetAndStatement(stmtManager, stmt, rs, sql);
  -            return retval;
  +            return new ResultSetAndStatement(stmtManager, stmt, rs, sql);
           }
           catch(PersistenceBrokerException e)
           {
  +            // release resources on exception
  +            stmtManager.closeResources(stmt, rs);
               log.error("PersistenceBrokerException during the execution of the query: "
+ e.getMessage(), e);
  -            /*
  -             * MBAIRD: error condition could result in our
  -             * ResultSetAndStatement not being returned, and not being closed
  -             * since it is opened before the try loop, we should release it if
  -             * there is a problem.
  -             */
  -            if(retval != null)
  -            {
  -                retval.close();
  -            }
               throw e;
           }
           catch(SQLException e)
           {
  -            /*
  -             * MBAIRD: error condition could result in our
  -             * ResultSetAndStatement not being returned, and not being closed
  -             * since it is opened before the try loop, we should release it if
  -             * there is a problem.
  -             */
  -            if(retval != null)
  -            {
  -                retval.close();
  -            }
  +            // release resources on exception
  +            stmtManager.closeResources(stmt, rs);
               throw ExceptionHelper.generateException(e, sql.getStatement(), cld, null, null,
log);
           }
       }
  @@ -592,13 +581,12 @@
   
           final boolean isStoredprocedure = SqlHelper.isStoredProcedure(sql);
           StatementManager stmtMan = broker.serviceStatementManager();
  -        ResultSetAndStatement retval = null;
  +        ResultSet rs = null;
  +        PreparedStatement stmt = null;
           try
           {
  -            final PreparedStatement stmt = stmtMan.getPreparedStatement(sql,
  -                    scrollable, StatementManager.FETCH_SIZE_NOT_EXPLICITLY_SET, isStoredprocedure);
  -
  -            ResultSet rs;
  +            stmt = stmtMan.getPreparedStatement(sql, scrollable,
  +                    StatementManager.FETCH_SIZE_NOT_EXPLICITLY_SET, isStoredprocedure);
               if(isStoredprocedure)
               {
                   // Query implemented as a stored procedure, which must return a result
set.
  @@ -617,7 +605,7 @@
               // as we return the resultset for further operations, we cannot release the
statement yet.
               // that has to be done by the JdbcAccess-clients (i.e. RsIterator, ProxyRsIterator
and PkEnumeration.)
               // arminw: use anonymous inner class to satisfy constructor of
  -            retval = new ResultSetAndStatement(broker.serviceStatementManager(), stmt,
rs, new SelectStatement(){
  +            return new ResultSetAndStatement(broker.serviceStatementManager(), stmt, rs,
new SelectStatement(){
                   public Query getQueryInstance()
                   {
                       return null;
  @@ -633,32 +621,18 @@
                       return sql;
                   }
               });
  -            return retval;
           }
           catch(PersistenceBrokerException e)
           {
  +            // release resources on exception
  +            stmtMan.closeResources(stmt, rs);
               log.error("PersistenceBrokerException during the execution of the SQL query:
" + e.getMessage(), e);
  -
  -            /**
  -             * MBAIRD: error condition could result in our ResultSetAndStatement not being
returned, and not being closed
  -             * since it is opened before the try loop, we should release it if there is
a problem.
  -             */
  -            if(retval != null)
  -            {
  -                retval.close();
  -            }
               throw e;
           }
           catch(SQLException e)
           {
  -            /**
  -             * MBAIRD: error condition could result in our ResultSetAndStatement not being
returned, and not being closed
  -             * since it is opened before the try loop, we should release it if there is
a problem.
  -             */
  -            if(retval != null)
  -            {
  -                retval.close();
  -            }
  +            // release resources on exception
  +            stmtMan.closeResources(stmt, rs);
               throw ExceptionHelper.generateException(e, sql, null, null, null, log);
           }
       }
  @@ -684,16 +658,16 @@
           final StatementManager sm = broker.serviceStatementManager();
           SelectStatement sql = null;
           ValueContainer[] values = null;
  -        ResultSetAndStatement rs_stmt = null;
  +        ResultSet rs = null;
  +        PreparedStatement stmt = null;
  +        Object result = null;
           try
           {
  -            ResultSet rs;
  -            PreparedStatement stmt;
               sql = broker.serviceSqlGenerator().getPreparedSelectByPkStatement(cld);
               final boolean callableStmt = useCallableSelectByPKStatement(cld);
               stmt = sm.getPreparedStatement(sql.getStatement(), Query.NOT_SCROLLABLE,
                       StatementManager.FETCH_SIZE_NOT_APPLICABLE);
  -            values = extractKeyValues(cld, oid);
  +            values = broker.serviceBrokerHelper().getKeyValues(cld, oid);
               if(callableStmt)
               {
                   // First argument is the result set
  @@ -709,37 +683,33 @@
                   rs = stmt.executeQuery();
               }
               // data available read object, else return null
  +            ResultSetAndStatement rs_stmt = new ResultSetAndStatement(broker.serviceStatementManager(),
stmt, rs, sql);
               if (rs.next())
               {
                   Map row = new HashMap();
                   RowReader rowReader = broker.getRowReaderFor(cld);
  -
  -                rs_stmt = new ResultSetAndStatement(broker.serviceStatementManager(), stmt,
rs, sql);
                   rowReader.readObjectArrayFrom(rs_stmt, row);
  -
                   // read enclosing class reference first
                   Object enclosingObj = broker.materializeEnclosingClassReference(cld, row);
  -
  -                return rowReader.readObjectFrom(row, enclosingObj);
  -            }
  -            else
  -            {
  -                return null;
  +                result = rowReader.readObjectFrom(row, enclosingObj);
               }
  +            // close resources
  +            rs_stmt.close();
           }
           catch(PersistenceBrokerException e)
           {
  +            // release resources on exception
  +            sm.closeResources(stmt, rs);
               log.error("PersistenceBrokerException during the execution of materializeObject:
" + e.getMessage(), e);
               throw e;
           }
           catch(SQLException e)
           {
  +            // release resources on exception
  +            sm.closeResources(stmt, rs);
               throw ExceptionHelper.generateException(e, sql.getStatement(), cld, values,
null, log);
           }
  -        finally
  -        {
  -            if(rs_stmt != null) rs_stmt.close();
  -        }
  +        return result;
       }
   
       /**
  @@ -835,18 +805,6 @@
       }
   
       /**
  -     * Build an {@link org.apache.ojb.broker.core.ValueContainer} array of the PK field
values.
  -     *
  -     * @param cld The {@link org.apache.ojb.broker.metadata.ClassDescriptor} of the object.
  -     * @param oid The {@link org.apache.ojb.broker.Identity} of the object.
  -     * @return The PK field values.
  -     */
  -    protected ValueContainer[] extractKeyValues(ClassDescriptor cld, Identity oid)
  -    {
  -        return broker.serviceBrokerHelper().getKeyValues(cld, oid);
  -    }
  -
  -    /**
        * Returns all fields that represents a call to a procedure or
        * user-defined function.
        *
  
  
  
  1.18      +21 -21    db-ojb/src/java/org/apache/ojb/broker/accesslayer/ResultSetAndStatement.java
  
  Index: ResultSetAndStatement.java
  ===================================================================
  RCS file: /home/cvs/db-ojb/src/java/org/apache/ojb/broker/accesslayer/ResultSetAndStatement.java,v
  retrieving revision 1.17
  retrieving revision 1.18
  diff -u -r1.17 -r1.18
  --- ResultSetAndStatement.java	3 Oct 2005 17:35:25 -0000	1.17
  +++ ResultSetAndStatement.java	5 Nov 2005 16:15:01 -0000	1.18
  @@ -19,8 +19,6 @@
   import java.sql.Statement;
   
   import org.apache.ojb.broker.accesslayer.sql.SelectStatement;
  -import org.apache.ojb.broker.util.logging.Logger;
  -import org.apache.ojb.broker.util.logging.LoggerFactory;
   
   /**
    * Intern used wrapper for {@link Statement} and {@link ResultSet} instances.
  @@ -29,7 +27,7 @@
    */
   final public class ResultSetAndStatement
   {
  -	private static Logger log = LoggerFactory.getLogger(ResultSetAndStatement.class);
  +	// private static Logger log = LoggerFactory.getLogger(ResultSetAndStatement.class);
   
   	private final StatementManager manager;
       private boolean isClosed;
  @@ -65,21 +63,23 @@
           }
       }
   
  -    protected void finalize() throws Throwable
  -    {
  -        super.finalize();
  -        if(!isClosed && (m_stmt != null || m_rs != null))
  -        {
  -            log.warn("** Associated resources (Statement/ResultSet) not closed!" +
  -                    " Try automatic cleanup **");
  -            try
  -            {
  -                close();
  -            }
  -            catch (Exception ignore)
  -            {
  -                //ignore it                
  -            }
  -        }
  -    }
  +// arminw: This class is internaly used, thus we should take care to close all used
  +// resources without this check.
  +//    protected void finalize() throws Throwable
  +//    {
  +//        super.finalize();
  +//        if(!isClosed && (m_stmt != null || m_rs != null))
  +//        {
  +//            log.warn("** Associated resources (Statement/ResultSet) not closed!" +
  +//                    " Try automatic cleanup **");
  +//            try
  +//            {
  +//                close();
  +//            }
  +//            catch (Exception ignore)
  +//            {
  +//                //ignore it
  +//            }
  +//        }
  +//    }
   }
  
  
  

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


Mime
View raw message