Sonar: “Close this PreparedStatement”

Question

Why is SonarQube plugin for Jenkins complaining about the open statement if I close it in the finally block?

(I need to validate database connections in a separate function.)

final String PING = "SELECT 1 from dual";

public boolean validateConnection(Connection conn) { 

    PreparedStatement statement = null;
    try{
        if(conn == null){
            LOGGER.log( LogEntries.PingError, "Null connection on PING. Reached max # of connections or network issue. Stats: "+getCacheStatistics() );
            return false;
        }

        if(conn.isClosed()){
            // logger
            return false;   
        } 

        statement = conn.prepareStatement( PING ); //%%%%%% SONAR: Close this "PreparedStatement".
        statement.setQueryTimeout(QUERY_TIMEOUT);

        try( ResultSet rs = statement.executeQuery() ){
            if ( rs != null && rs.next() ) {
                return true;
            }
        }catch(Exception exRs){
            // logger
            throw exRs;
        }
    }catch(Exception ex){
        // logger
    }finally{
        try{
            statement.close();
        }catch(Exception excpt){
            // logger
        }
    }
    return false;
}

Answer 1

I've refactored my code in this way as suggested by @TT and sonar stopped complaining.

public boolean validateConnection(Connection conn) {

    LOGGER.log( LogEntries.PingConn );

    try{

        if(conn == null){
            LOGGER.log( LogEntries.PingError, "Null connection on PING. Reached max # of connections or network issue. Stats: "+getCacheStatistics() );
            return false;
        }

        if(conn.isClosed()){
            LOGGER.log( LogEntries.PingError, "Found closed connection during validation PING." );
            return false;   
        } 

        try( PreparedStatement statement = conn.prepareStatement( PING ) ){

             statement.setQueryTimeout(QUERY_TIMEOUT);

             try( ResultSet rs = statement.executeQuery() ){

                if ( rs != null && rs.next() ) {
                    return true;
                }
            }
        }

    }catch(Exception ex){
        LOGGER.log( LogEntries.PingError, ex );
    }

    return false;
}

Without "try-with-resource" the code could be refactored in the following way but in this case Sonar still complains:

public boolean validateConnection(Connection conn) {

    LOGGER.log( LogEntries.PingConn );

    PreparedStatement statement = null;
    ResultSet rs = null;
    try{

        if(conn == null){
            LOGGER.log( LogEntries.PingError, "Null connection on PING. Reached max # of connections or network issue. Stats: "+getCacheStatistics() );
            return false;
        }

        if(conn.isClosed()){
            LOGGER.log( LogEntries.PingError, "Found closed connection during validation PING." );
            return false;   
        } 

        statement = conn.prepareStatement( PING );
        statement.setQueryTimeout( QUERY_TIMEOUT );
        rs = statement.executeQuery();

        if ( rs != null && rs.next() ) {
            return true;
        }

    }catch(Exception ex){
        LOGGER.log( LogEntries.PingError, ex );
    }finally{
        try {
            if(rs!=null){
                rs.close();
            }
        } catch (SQLException eClosing1) {
            LOGGER.log( LogEntries.PingError, eClosing1 );
        }finally{
            try {
                if(statement!=null){
                    statement.close();
                }
            }catch (SQLException eClosing2) {
                LOGGER.log( LogEntries.PingError, eClosing2 );
            }   
        }
     }

    return false;
}

Answer 2

I donnot see the point of putting all this nested try blocks with no catch. You need only the first try with the suitable catch and finally where you close your statments and connection after checking them against null.