Use attachToHandleForCleanup() everywhere

[jodastephen]

The method attachToHandleForCleanup() has been added recently, and looks to be a good solution to always ensuring things are closed. I could be wrong, but it seems like it might be desirable to set attachToHandleForCleanup() always, however I can see no easy way to do this. ie. I can see no easy way at the framework-level, obviously I could update all the use sites, but adding try-with-resources is a fair bit more verbose. I don't believe StatementCustomizer can do the job here.

Some possible approaches I came up with:

• could a new method be added to Handle - attachStatementsToHandleForCleanup() that is used to auto-set the flag whenever a BaseStatement subclass is created - feels quite nice, and can be configured in JdbiPlugin
• could there be a callback to JdbiPlugin whenever Handle creates a Query/Update/Call etc. - not sure what else the callbacks would be used for
• could there be a built-in subclass of Handle that calls attachToHandleForCleanup() after creating the statement - doesn't feel like JDBI design style
• could JDBI add methods like withQuery(String, QueryCallback) to Handle - feels like overkill

Background info:

• We use JDBI as a Java API, no annotations/Spring etc.
• We have our own JDBI plugin that is always called, so ideally we'd like to set attachToHandleForCleanup() there
• We use Java 11 at present

Thanks for JDBI!

[hgschmie]

Hi @jodastephen ,

Thank you for opening an issue with the Jdbi project!

This is an interesting proposal. We could add a flag to SqlStatements to set this as a default.

[hgschmie]

As a caveat: Adding every statement to a handle for cleanup means that resources may be held for a while (until the handle is actually closed). There is a chance that these resources "pile up", especially when many statements are executed.

[hgschmie]

adding the flag as proposed should do the trick. If you have a factory that creates the Jdbi objects, it should just call jdbi.getConfig(SqlStatements.class).setAttachAllStatementsForCleanup(true). Alternatively you can use a simple plugin:

public class AttachHandlesPlugin extends JdbiPlugin.Singleton {
    @Override
    public void customizeJdbi(Jdbi jdbi) {
        jdbi.getConfig(SqlStatements.class).setAttachAllStatementsForCleanup(true);
    }
}

and then call jdbi.installPlugin(new AttachHandlesPlugin());

[stevenschlansker]

If you want to avoid piling up resources, we might be able to use a PhantomReference to allow references to be cleaned up earlier than handle close.
This starts to get more and more complicated, though.

[jodastephen]

A flag on SqlStatements would be fine by me. We would call it from our existing plugin.

As I mentioned above, there may be a case for callback here, as the Java 8+ design alternative to try-with-resources:

// current explicit approach
jdbi.useHandle(handle -> {
    try (Update update = handle.createUpdate("INSERT INTO users (id, name) VALUES (:id, :name)")) {
        update.bind("id", id)
              .bind("name", "user_" + id)
              .execute();
    }
});
// potential new explicit approach
jdbi.useHandle(handle -> {
    handle.createUpdate(
        "INSERT INTO users (id, name) VALUES (:id, :name)",
        update -> {
            update.bind("id", id)
                .bind("name", "user_" + id)
                .execute();
    });
});

My thought process being that it is surprising to see try-with-resources in the middle of JDBI code, which is all lambda-based.

[stevenschlansker]

I agree, if we want users to close all statements, providing a callback-based alternative to t-w-r does look nice.

[hgschmie]

When I rewrote the code in 3.35.0 (which introduced the attachToHandleForCleanup(), I was careful to not change the behavior of the code before that rewrite. We don't know if there were peeps that rely on this behavior. I agree that the useHandle and withHandle code paths could just set this flag on the handle configuration now. That would allow anyone using the callbacks to not bother with t-w-r.

[hgschmie]

This will be addressed in the next release through #2294.

See https://jdbi.org/#_attaching_statements_to_the_handle_lifecycle for a preview of the release documentation. In a nutshell: If you use callbacks on the jdbi object, it will "just work" even if you don't attach the statement to the handle or do any t-w-r resource management. Outside these callbacks, there is a global flag that you can set to get this behavior everywhere.

[jodastephen]

Looks fabulous, thanks