2

I am using DAO pattern. In my case the DAO does not open or close connection; instead connection is passed to it through constructor; it uses that connection to perform its job. Can connection passing through constructor to DAO layer be unsafe in any case? If yes please tell me how.

This is how I am using it:

This is the parent of all DAO implementations:

public abstract class SuperDAO {
    private Connection connection;
    public SuperDAO(Connection connection) { this.connection = connection; }
    public Connection getConnection() { return this.connection; }
}

This is the User business object:

public class User {
    private String userId;
    private String password;
    public String getUserId() { return this.userId; }
    public void setUserId(String userId) { this.userId = userId; }
    public String getPassword() { return this.password; }
    public void setPassword(String password) { this.password = password; }
} 

This is the DAO of User business object:

public interface UserDAO {
    User getUser(String userId, String password);
}

This is the SQL Server implementation of DAO of User business object:

public class SQLServerUserDAO extends SuperDAO implements UserDAO {
    public SQLServerUserDAO(Connection connection) { super(connection); }
    public User getUser(String userId, String password) { 
        // perform task; use getConnection() for connection; dont close connection 
    }
}

This is the DAO Factory:

public abstract class DAOFactory {
    public static final int SQL_SERVER = 1;
    protected Connection connection;

    public static DAOFactory getDAOFactory(int factoryType, Connection connection) {
        DAOFactory daoFactory = null;

        switch (factoryType) {
        case SQL_SERVER:
            daoFactory = new SQLServerDAOFactory();
            daoFactory.connection = connection;
            break; 
        }

        return daoFactory;
    }

    public abstract UserDAO getUserDAO();
}

This is the SQL Server implementation of DAO Factory:

public class SQLServerDAOFactory extends DAOFactory {
    public UserDAO getUserDAO() {
        UserDAO userDAO = new SQLServerUserDAO(connection);
        return userDAO;
    }
}

And this is how I use it:

Connection connection = null;

try {
    connection = ... // obtain a connection;
    DAOFactory daoFactory = DAOFactory.getDAOFactory(DAOFactory.SQL_SERVER, connection);
    UserDAO userDAO = daoFactory.getUserDAO();
    ...
} finally {
    connection.close();
}

In which cases this code will be broken? Please advise.

Thanks

srh
  • 1,661
  • 4
  • 30
  • 57
  • What happens when the connection dies? Times out? How do you change the reference to a working connection? You also hog connections, making it impossible to pool them. Pass in a `DataSource` and close after each use. Use a connection pool. – Boris the Spider Jan 29 '15 at 21:15
  • Some times I need same connection in multiple DAOs for a transaction thats why I cannot pass DataSource. The service layer get connection using connection pool. Can you tell me how I am hogging connections? – srh Jan 29 '15 at 21:54
  • That's a very naive view. Sooner or later you reinvented wheel will turn out to be square. There are many ways to coordinate pools to return the same connection, from threadlocal connections to JTA. – Boris the Spider Jan 29 '15 at 21:56
  • ThreadLocal connections is a totally new concept for me. When I get connection from a DataSource am I not using JTA automatically? If not then JTA is a new concept too for me. – srh Jan 29 '15 at 22:20

1 Answers1

3

Can connection passing through constructor to DAO layer be unsafe in any case?

No, the way you're using it is safe.

However, you're reinventing the wheel. Rather use an existing ORM tool.

In which cases this code will be broken?

It will be broken if you're using container managed transactions, and you're obtaining a connection outside the transaction context while your DAO code is executing inside the transaction context.

For example, this would be bad:

@Stateless
public static class MyBean implements MyBusinessInterface {
    @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW)
    public void doSomething(Connection connection) {
        new SomeDAO().doSomething(connection);
    }
}

In other words, Your code is fine if you're not using container managed transactions at all, or if you're obtaining the connection from a managed connection pool within the same transactional context as your DAOs.

Note: in the finally block, you have a potential NullPointerException trying to call close() on a null connection variable.

If you do this instead, you will always have a connection to close:

Connection connection = //obtain a connection
try {
    DAOFactory daoFactory = DAOFactory.getDAOFactory(DAOFactory.SQL_SERVER, connection);
    UserDAO userDAO = daoFactory.getUserDAO();
    // etc
} finally {
    connection.close();
}
Community
  • 1
  • 1
gknicker
  • 5,509
  • 2
  • 25
  • 41
  • I also thought it is safe. But @tnurkiewicz said in a post: **Notice that passing connection as a constructor parameter is completely broken in multi-threaded environment.** in here http://stackoverflow.com/questions/14221989/dao-design-pattern-and-connection-handling – srh Jan 29 '15 at 21:42
  • What he's saying in that answer is, it's bad to obtain a connection outside a transactional context and pass it into a transactional context. Your code is fine if you're not using container managed transactions at all, or if you're obtaining the connection from a managed connection pool within the same transactional context as your DAOs. – gknicker Jan 29 '15 at 21:57
  • I am not clear about the transactional context. Can you give an example? Can you explain how to obtain a connection outside a transactional context and pass it into a transactional context? – srh Jan 29 '15 at 23:36
  • I added an example in my answer. If you're not using Java Enterprise beans, don't worry about it, because it doesn't apply to you. – gknicker Jan 29 '15 at 23:46
  • thanks for the example. The concept is clear now. I am using EJB but I have never seen a code like this where a Connection is passed to an EJB method. – srh Jan 30 '15 at 00:26