2011-07-08 12 views
4

私は現在、社内での使用のために作られたインターンが、中小規模のJava Webアプリケーション(普通のJSP /サーブレットのみを使用しています)を維持しています。接続に問題があります。中規模のWebアプリケーションでデータベース接続を適切に処理する方法

「文が閉じている」、「接続が閉じている」などのエラーが表示され、アプリケーション全体が機能しなくなり、サーバーを再起動する必要があります。

私は多くの経験がなく、ベストプラクティスやデザインパターンなどについて教えたり教えたりする人がいませんが、これは正しい方法ではないと確信しています。私はDAL、DAO、DTOのよ​​うなものについて読んだことがあります。私たちのアプリはそれらのどれも持っていません。

全体のWebアプリケーション(。つまり、サーブレット)は、基本的に次のような呼び出しで満たされています。

Database db = Database.getInstance(); 
db.execute("INSERT INTO SomeTable VALUES (a, b, c)"); 
db.execute("UPDATE SomeTable SET Col = Val"); 

のSELECTは、そのように行われています。

モデルは、そのクラスで
ArrayList<Model> results = Model.fetch("SELECT * FROM SomeTable"); 

をHashMapを拡張し、テーブル内の単一の行を表します。

これはDatabase.javaのコードであり、間違っていることを誰かが指摘できるかどうか疑問に思っていました(私はかなりたくさんあると確信しています)、ベストプラクティスデータベース接続/接続処理に関して。

package classes; 

import java.sql.Connection; 
import java.sql.PreparedStatement; 
import java.sql.ResultSet; 
import java.sql.ResultSetMetaData; 
import java.sql.SQLException; 
import java.sql.Statement; 
import java.util.ArrayList; 
import java.util.HashMap; 

import javax.naming.InitialContext; 
import javax.naming.NamingException; 
import javax.sql.DataSource; 

public final class Database { 

    public static Database getInstance() { 
     if (Database.instance == null) { 
      Database.instance = new Database(); 
     } 
     return Database.instance; 
    } 

    // Returns the results for an SQL SELECT query. 
    public ArrayList<HashMap<String, Object>> fetch(String sql) { 

     ArrayList<HashMap<String, Object>> results = new ArrayList<HashMap<String, Object>>(); 

     try { 

      PreparedStatement stmt = this.connection.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT); 
      ResultSet rs = stmt.executeQuery(); 
      this.doFetch(rs, results); 
      stmt.close(); 

     } catch (SQLException e) { 
      this.handleException(e, sql); 
     } 

     return results; 
    } 

    public ArrayList<HashMap<String, Object>> fetch(String sql, ArrayList<Object> parameters) { 

     ArrayList<HashMap<String, Object>> results = new ArrayList<HashMap<String, Object>>(); 

     try { 

      // Bind parameters to statement. 
      PreparedStatement pstmt = this.connection.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT); 
      for (int i=0; i<parameters.size(); i++) { 
       pstmt.setObject(i+1, parameters.get(i)); 
      } 

      ResultSet rs = pstmt.executeQuery(); 
      this.doFetch(rs, results); 
      pstmt.close(); 

     } catch (SQLException e) { 
      this.handleException(e, sql, parameters); 
     } 

     return results; 
    } 

    public int execute(String sql) { 
     int result = 0; 
     try { 
      Statement stmt = this.connection.createStatement(); 
      result = stmt.executeUpdate(sql); 
      stmt.close(); 
     } catch (SQLException e) { 
      this.handleException(e, sql); 
     } 
     return result; 
    } 

    public int execute(String sql, ArrayList<Object> parameters) { 
     int result = 0; 
     try { 
      PreparedStatement pstmt = this.connection.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT); 
      for (int i=0; i<parameters.size(); i++) { 
       if (parameters.get(i) == null) { 
        pstmt.setNull(i+1, java.sql.Types.INTEGER); 
       } else { 
        pstmt.setObject(i+1, parameters.get(i)); 
       } 
      } 
      result = pstmt.executeUpdate(); 
      pstmt.close(); 
     } catch (SQLException e) { 
      this.handleException(e, sql, parameters); 
     } 
     return result; 
    } 

    public void commit() { 
     try { 
      this.connection.commit(); 
     } catch (SQLException e) { 
      System.out.println("Failed to commit transaction."); 
     } 
    } 

    public Connection getConnection() { 
     return this.connection; 
    } 


    private static Database instance; 
    private static DataSource dataSource = null; 
    private Connection connection; 

    private Database() { 
     this.connect(); 
     this.execute("SET SCHEMA " + Constant.DBSCHEMA); 
    } 

    private void connect() { 
     Connection connection = null; 
     if (dataSource == null) { 
      try { 
       InitialContext initialContext = new InitialContext(); 
       dataSource = (DataSource)initialContext.lookup(
         Constant.DEPLOYED ? Constant.PROD_JNDINAME : Constant.TEST_JNDINAME); 
      } catch (NamingException e) { 
       e.printStackTrace(); 
      } 
     } 
     try { 
      connection = dataSource.getConnection(); 
     } catch (SQLException e) { 
      e.printStackTrace(); 
     } 
     this.connection = connection; 
    } 

    // Fetches the results from the ResultSet into the given ArrayList. 

    private void doFetch(ResultSet rs, ArrayList<HashMap<String, Object>> results) throws SQLException { 
     ResultSetMetaData rsmd = rs.getMetaData(); 

     ArrayList<String> cols = new ArrayList<String>();   
     int numCols = rsmd.getColumnCount(); 

     for (int i=1; i<=numCols; i++) { 
      cols.add(rsmd.getColumnName(i)); 
     } 

     while (rs.next()) { 
      HashMap<String, Object> result = new HashMap<String, Object>(); 
      for (int i=1; i<=numCols; i++) { 
       result.put(cols.get(i-1), rs.getObject(i)); 
      } 
      results.add(result); 
     } 

     rs.close(); 
    } 

    private void handleException(SQLException e, String sql) { 
     System.out.println("SQLException " + e.getErrorCode() + ": " + e.getMessage()); 
     System.out.println("Statement: " + sql); 
     ExceptionAdapter ea = new ExceptionAdapter(e); 
     ea.setSQLInfo(e, sql); 
     throw ea; 
    } 

    private void handleException(SQLException e, String sql, ArrayList<Object> parameters) { 
     if (parameters.size() < 100) { 
      System.out.println("SQLException " + e.getErrorCode() + ": " + e.getMessage()); 
      System.out.println("PreparedStatement: " + sql.replace("?", "[?]")); 
      System.out.println("Parameters: " + parameters.toString()); 
     } 
     ExceptionAdapter ea = new ExceptionAdapter(e); 
     ea.setSQLInfo(e, sql, parameters); 
     throw ea; 
    } 
} 

ありがとう!

答えて

13

クラスは決して接続を閉じません。this.connection.close()Databaseシングルトンであるため、アプリケーションは接続プール(データソース)を使用しません。すべての着信要求に対して1つの接続のみが使用されます。

法則:メソッドごとに1つの接続を取得します(多分SQL文ごとに)。 dataSource.getConnection()は高価ではありません。それはDatabaseクラス外で使用した場合、あなたが本当に設計上の問題

  • commitメソッドを削除する必要があり、公共getConnection方法を削除

    1. は、これは私がクラスをリファクタリングする方法をです。 、connection変数のインスタンスを削除する代わりに、コール

    2. あたりの接続を取得し、適切にそれぞれのfinallyブロックにこの接続を閉じて、私はconnection.setAutoCommit(false)が呼び出されないよう、それは意味がないと仮定し、私はrollback方法
    3. が表示されません

    免責事項:現時点での取引処理方法はわかりませんので、#2で間違っている可能性があります。接続を取得する方法について

    サンプルコード:

    Connection c = null; 
    try { 
        c = this.dataSource.getConnection(); 
        c.executeStatement("select * from dual"); 
    } catch (SQLException e) { 
        // handle... 
    } finally { 
        closeConnection(c); 
    } 
    

    このアプリは全く:-)仕事ができるか興味深い

  • 2

    単純なアプリごとにこのようにするのは問題ありません。しかし、アプリケーションが中程度に複雑であれば、iBatisなどの単純なフレームワークを調べることができます。

    私が間違いなく行うことのカップル。 1つは、例外がスローされたときにアプリケーションが接続をリークしている可能性があります。すべての終了ステートメントはfinallyブロック内に移動する必要があります。

    ので、代わりの:

    try { 
          Statement stmt = this.connection.createStatement(); 
          result = stmt.executeUpdate(sql); 
          stmt.close(); 
         } catch (SQLException e) { 
          this.handleException(e, sql); 
         } 
    

    ではなく、これを実行します。

    Statement stmt = null; 
    try { 
         stmt = this.connection.createStatement(); 
         result = stmt.executeUpdate(sql); 
        } catch (SQLException e) { 
         this.handleException(e, sql); 
        } finally { 
         if (stmt != null) stmt.close(); 
        } 
    

    他の事は、私はあなたがあなたのデータソースのデータベース接続プールを使用していることを確認しますです。これをTomcatで実行している場合は、tomcatのインストールで定義されている接続プールがあり、そのアプリケーションがその接続プールを使用していることを確認してください。

    EDIT:コードをもう一度見た後、データベース接続が実際に閉じられている場所も見えません。これはたぶんあなたが接続を使い果たした理由です。 Connection.close()を呼び出すDatabaseクラスにcloseメソッドを追加する必要があります。そして、あなたがあなたの質問を終えたときにそれを呼んでいることを確かめてください。 try/finallyブロックでも同様です。

    +0

    私はあなたのためのApacheコモンズのDbUtils.closeQuietly()を使用することを示唆していますfinallyブロック内のステートメント、結果セット、および接続オブジェクトを閉じます。これにより、close()を呼び出すときにNULLをチェックしSQLExceptionsをトラップしないようにします。 – svachon

    +0

    これはWebアプリケーションです。これは、任意の所与の時点において、処理される要求がいくつか存在する可能性があることを意味する。同期はありません。また、接続は、複数のスレッドから使​​用できるDatabaseクラスのメンバー変数です。 – Olaf

    1

    非常に安全でない方法でJDBC接続を使用しています。複数のスレッドからアクセスでき、スレッドセーフではありません。これはWebアプリケーションであり、同時に複数のリクエストが異なるユーザーから来ることがあります。アプリケーションがより頻繁にクラッシュしない小さな奇跡です。いくつかの戦略を使ってそれを修正することができます。接続は、ThreadLocalまたはスタックに格納できます。スタックに接続を維持する場合は、各メソッド呼び出し内で接続を開いて閉じなければなりません。これを実行するには、接続プールを使用する必要があります。接続プールはどんな場合でも傷つくことはありません。

    関連する問題