java 在中型 Web 应用程序中处理数据库连接的正确方法

Proper way of handling database connections in a medium sized web application

提问by Connection

I'm currently maintaining a small-medium sized java web application (using only plain JSPs/Servlets) that an intern made for internal company use and I'm having some trouble with connections.

我目前正在维护一个中小型 Java Web 应用程序（仅使用普通的 JSP/Servlets），这是一个实习生为公司内部使用而制作的，但我在连接方面遇到了一些问题。

Sometimes just out of nowhere, we get errors like "Statement is closed" or "Connection is closed" and then the whole application would just stop working and the server has to be restarted.

有时，我们会突然收到诸如“语句已关闭”或“连接已关闭”之类的错误，然后整个应用程序就会停止工作，并且必须重新启动服务器。

I don't have a lot of experience and I don't have anyone to mentor or teach me regarding best practices, design patterns, etc. but I'm pretty sure this is not the right way to do this. I've read about stuff like DALs, DAOs, and DTOs. Our app has none of those.

我没有很多经验，也没有人指导或教我有关最佳实践、设计模式等的知识。但我很确定这不是正确的方法。我读过关于 DAL、DAO 和 DTO 之类的东西。我们的应用程序没有这些。

The whole web application (ie. the servlets) are basically filled with calls similar to the following:

整个 Web 应用程序（即 servlet）基本上充满了类似于以下的调用：

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

SELECTs are done like so:

SELECT 是这样完成的：

ArrayList<Model> results = Model.fetch("SELECT * FROM SomeTable");

Where Model is a class that extends HashMap and represents a single Row in a Table.

其中 Model 是一个类，它扩展了 HashMap 并表示表中的单个行。

This is the code for Database.java and was wondering if someone can point out obvious things that are wrong (I'm pretty sure there are a lot), any quick fixes that can be done and some resources on best practices with regards to database connections / connection handling.

这是 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;
    }
}

Thanks!

谢谢！

回答by home

The class does never, ever close a connection: this.connection.close(). As Databaseis a Singletonthe application does not make use of the connection pool (the datasource). Only one connection is used for all incoming requests.

该类永远不会关闭连接：this.connection.close()。作为Database一个单身的应用程序不使用连接池（数据源）的。所有传入请求仅使用一个连接。

Rule of thumb: get one connection per method (maybe per SQL statement). dataSource.getConnection()is not expensive.

经验法则：每个方法（可能每个 SQL 语句）获得一个连接。dataSource.getConnection()不贵。

This is how I would refactor the class:

这就是我重构类的方式：

1. remove the public getConnectionmethod, if it used outside the Databaseclass you really have a design problem
2. remove the commitmethod. I suppose it does not make sense as connection.setAutoCommit(false)is never called and I don't see a rollbackmethod
3. remove the instance variable connection, instead obtain a connection per call
4. and properly close this connection in the finallyblock of each call

1. 删除公共getConnection方法，如果它在Database类之外使用，你真的有设计问题
2. 删除commit方法。我想它没有意义，因为connection.setAutoCommit(false)从来没有被调用过，而且我没有看到任何rollback方法
3. 删除实例变量connection，而是在每次调用时获取一个连接
4. 并在finally每次调用的块中正确关闭此连接

Disclaimer: No idea how your transaction handling works at the moment, so I may be wrong with #2.

免责声明：目前不知道您的事务处理是如何工作的，所以我对#2 的看法可能是错误的。

Sample code for a method to obtain a connection:

获取连接方法的示例代码：

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

Interesting how this app can work at all :-)

有趣的是这个应用程序如何工作:-)

回答by Nobody

There is nothing wrong with doing it this way for simple apps per-say. But if your application is even moderately complex, you may want to look into a simple framework such as iBatis.

对于简单的应用程序来说，这样做并没有错。但是，如果您的应用程序稍微复杂一些，您可能需要研究一个简单的框架，例如 iBatis。

A couple of things I would definitely do though. For one, the application could be leaking connections when an exception is thrown do to the way the statements are being closed. All the close statements should be moved inside finally blocks.

有几件事我肯定会做。一方面，当抛出异常时，应用程序可能会泄漏连接，这与关闭语句的方式有关。所有的 close 语句都应该移动到 finally 块中。

So instead of:

所以而不是：

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

Do this instead:

改为这样做：

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();
    }

The other thing is I would make sure you are using a database connection pool for your datasource. If you are running this in Tomcat, hopefully there is a connection pool defined in the tomcat installation, and your application is using that.

另一件事是我会确保您为数据源使用数据库连接池。如果您在 Tomcat 中运行它，希望在 tomcat 安装中定义了一个连接池，并且您的应用程序正在使用它。

EDIT: After looking at the code again, I'm also not seeing where the database connection is actually being closed. This is probably why you run out of connections. YOu need to add a close method to the Database class, that calls connection.close(). And make sure you are calling it when you are finished with your queries. Again, in a try/finally block.

编辑：再次查看代码后，我也没有看到数据库连接实际关闭的位置。这可能就是您用尽连接的原因。您需要向 Database 类添加一个 close 方法，该方法调用 connection.close()。并确保在完成查询后调用它。同样，在 try/finally 块中。

回答by Olaf

You are using JDBC connection in a very unsafe way. It can be accessed from multiple threads and it is not thread safe. This is a web application and multiple requests can come from different users at the same time. It's a small miracle your application is not crashing more frequently. You can use several strategies to fix that. You can store connections in a ThreadLocal or on the stack. If you are going to keep connections on the stack, you will have to open and close them within each method call. For this to have some performance, you will have to use the connection pool. Connection pool wouldn't hurt in any case.

您正在以一种非常不安全的方式使用 JDBC 连接。它可以从多个线程访问，它不是线程安全的。这是一个 Web 应用程序，多个请求可以同时来自不同的用户。您的应用程序没有更频繁地崩溃，这是一个小奇迹。您可以使用多种策略来解决这个问题。您可以将连接存储在 ThreadLocal 或堆栈中。如果要在堆栈上保持连接，则必须在每个方法调用中打开和关闭它们。为了获得一些性能，您将不得不使用连接池。连接池在任何情况下都不会受到伤害。

回答by Jamy Spencer

To answer your question about design principles, this object is essentially a DAO object, it just doesn't use the naming convention, also a large application would have several of these objects for different types of data (maybe using a base DAO object that they all inherit from).

为了回答你关于设计原则的问题，这个对象本质上是一个 DAO 对象，它只是不使用命名约定，对于不同类型的数据，一个大型应用程序也会有几个这样的对象（也许使用它们的基本 DAO 对象）都继承自）。

The broad idea is that A DAO is a central place where database connections are handled so that you don't have all that code in a Controller object.

广泛的想法是，DAO 是处理数据库连接的中心位置，因此您不会在 Controller 对象中拥有所有这些代码。

Aside from the shortcomings already pointed out by others, this is some solid code written by someone who understands object oriented programming very well. My recommendation is to change the object from a singleton and to manage the database connection with a connection pool(as others have already mentioned).

除了其他人已经指出的缺点之外，这是一些非常了解面向对象编程的人编写的一些可靠的代码。我的建议是从单例更改对象并使用连接池管理数据库连接（正如其他人已经提到的）。

This seems to be a highly abstracted object that returns an arraylist of maps(key value pairs), that can be used for different datatypes which is then used in the Model objects or the datatypes to build java objects with the information returned.

这似乎是一个高度抽象的对象，它返回映射（键值对）的数组列表，可用于不同的数据类型，然后在模型对象或数据类型中使用返回的信息构建 java 对象。