Handle.afterCommit: if callback throws after we've already committed,…

… don't try to rollback

Fixes #2478

core/src/main/java/org/jdbi/v3/core/transaction/LocalTransactionHandler.java

@@ -101,6 +101,7 @@ static class BoundLocalTransactionHandler implements TransactionHandler {
         private final Map<String, Savepoint> savepoints = new HashMap<>();
         private boolean initialAutocommit;
         private boolean didBegin;
+        private boolean didTxnCommit;
         private boolean didTxnRollback;
 
         BoundLocalTransactionHandler(Handle handle) throws SQLException {
@@ -127,6 +128,7 @@ public void begin(Handle handle) {
         public void commit(Handle handle) {
             try {
                 handle.getConnection().commit();
+                didTxnCommit = true;
             } catch (SQLException e) {
                 throw new TransactionException("Failed to commit transaction", e);
             } finally {
@@ -215,13 +217,16 @@ public <R, X extends Exception> R inTransaction(Handle handle,
                 }
             } catch (Throwable e) {
                 try {
-                    handle.rollback();
+                    if (!didTxnCommit) {
+                        handle.rollback();
+                    }
                 } catch (Exception rollback) {
                     e.addSuppressed(rollback);
                 }
                 throw e;
             } finally {
                 didTxnRollback = false;
+                didTxnCommit = false;
             }
 
             return returnValue;
@@ -246,6 +251,8 @@ private void restoreAutoCommitState(Handle handle) {
                     handle.getConnection().setAutoCommit(initialAutocommit);
                     savepoints.clear();
                     didBegin = false;
+                    didTxnCommit = false;
+                    didTxnRollback = false;
                 }
             } catch (SQLException e) {
                 throw new UnableToRestoreAutoCommitStateException(e);

core/src/test/java/org/jdbi/v3/core/TestHandlePg.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jdbi.v3.core;
+
+import de.softwareforge.testing.postgres.junit5.EmbeddedPgExtension;
+import de.softwareforge.testing.postgres.junit5.MultiDatabaseBuilder;
+import org.assertj.core.api.Assertions;
+import org.jdbi.v3.core.junit5.PgDatabaseExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+public class TestHandlePg {
+
+    @RegisterExtension
+    public static EmbeddedPgExtension pg = MultiDatabaseBuilder.instanceWithDefaults().build();
+
+    @RegisterExtension
+    public PgDatabaseExtension pgExtension = PgDatabaseExtension.instance(pg);
+
+    private Handle h;
+
+    @BeforeEach
+    public void startUp() {
+        Assertions.setMaxStackTraceElementsDisplayed(100);
+        this.h = pgExtension.openHandle();
+    }
+
+    @AfterEach
+    public void tearDown() {
+        h.close();
+    }
+
+    @Test
+    public void testAfterCommitThrowsRollback() throws Exception {
+        assertThatThrownBy(() -> h.useTransaction(inner -> {
+            inner.execute("create table names(id int, name varchar)");
+            h.afterCommit(() ->
+                h.useTransaction(inner2 -> {
+                    assertThat(h.createQuery("select name from names where id = 1").mapTo(String.class).one())
+                            .isEqualTo("Kafka");
+                    throw new IllegalStateException("boom");
+                }));
+            inner.execute("insert into names (id, name) values (1, 'Kafka')");
+        }))
+                .isInstanceOf(IllegalStateException.class)
+                .hasMessage("boom")
+                .hasNoSuppressedExceptions();
+        assertThat(h.createQuery("select name from names where id = 1").mapTo(String.class).one())
+                .isEqualTo("Kafka");
+    }
+}