Rollback JDBC transaction after caught mapper failures

Author: Philip Z

teaql-provider-jdbc/src/main/java/io/teaql/provider/jdbc/JdbcSqlExecutor.java

@@ -186,6 +186,9 @@ public List<Map<String, Object>> queryForList(String sql, Object[] params) {
         } catch (SQLException e) {
             markTransactionRollbackOnly();
             throw new RuntimeException("JDBC query failed for parameterized SQL: " + sql, e);
+        } catch (RuntimeException | Error failure) {
+            markTransactionRollbackOnly();
+            throw failure;
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
         }
@@ -213,6 +216,9 @@ public <T extends io.teaql.core.Entity> List<T> query(
         } catch (SQLException exception) {
             markTransactionRollbackOnly();
             throw new RuntimeException("JDBC typed query failed for parameterized SQL: " + sql, exception);
+        } catch (RuntimeException | Error failure) {
+            markTransactionRollbackOnly();
+            throw failure;
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
         }
@@ -343,6 +349,9 @@ public void execute(String sql) {
         } catch (SQLException e) {
             markTransactionRollbackOnly();
             throw new RuntimeException(e);
+        } catch (RuntimeException | Error failure) {
+            markTransactionRollbackOnly();
+            throw failure;
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
         }
@@ -369,6 +378,9 @@ public int update(String sql, Object[] params) {
         } catch (SQLException e) {
             markTransactionRollbackOnly();
             throw new RuntimeException(e);
+        } catch (RuntimeException | Error failure) {
+            markTransactionRollbackOnly();
+            throw failure;
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
         }
@@ -393,6 +405,9 @@ public int[] batchUpdate(String sql, List<Object[]> paramsList) {
         } catch (SQLException e) {
             markTransactionRollbackOnly();
             throw new RuntimeException(e);
+        } catch (RuntimeException | Error failure) {
+            markTransactionRollbackOnly();
+            throw failure;
         } finally {
             if (owned && connection != null) try { connection.close(); } catch (SQLException ignored) { }
         }
@@ -421,7 +436,7 @@ public void executeInTransaction(Runnable action) {
                 action.run();
                 if (Boolean.TRUE.equals(transactionRollbackOnly.get())) {
                     throw new IllegalStateException(
-                            "JDBC transaction is rollback-only after a nested failure");
+                            "JDBC transaction is rollback-only after a failed operation");
                 }
                 connection.commit();
             } catch (RuntimeException | Error failure) {
@@ -438,9 +453,9 @@ public void executeInTransaction(Runnable action) {
     }
 
     private void markTransactionRollbackOnly() {
-        // A failed statement may have left earlier writes in the shared
-        // transaction. Without a savepoint, catching that failure cannot
-        // make the transaction safe to commit.
+        // A failed statement, mapper or adapter operation may have left earlier
+        // writes in the shared transaction. Without a savepoint, catching the
+        // failure cannot make the transaction safe to commit.
         if (transactionConnection.get() != null) transactionRollbackOnly.set(true);
     }
 

teaql-provider-jdbc/src/test/java/io/teaql/provider/jdbc/JdbcSqlExecutorTest.java

@@ -135,6 +135,24 @@ public void caughtStatementOrReadFailureCannotCommitEarlierWrites() {
         }
     }
 
+    @Test
+    public void caughtTypedRowMapperFailureCannotCommitEarlierWrites() {
+        assertThrows(IllegalStateException.class, () -> sqlExecutor.executeInTransaction(() -> {
+            sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                    new Object[] {95, "must roll back", 20});
+            try {
+                sqlExecutor.query("SELECT id FROM test_user WHERE id = ?", new Object[] {95},
+                        row -> { throw new IllegalArgumentException("bad typed row"); });
+            } catch (IllegalArgumentException expected) {
+                // The mapper failure escaped a query using the same transaction.
+            }
+        }));
+
+        assertEquals(0, ((Number) sqlExecutor.queryForList(
+                "SELECT count(*) AS total FROM test_user", new Object[0])
+                .get(0).get("total")).intValue());
+    }
+
     @Test
     public void testQueryForListNormalizesAliasesForPortableMapping() {
         List<Map<String, Object>> rows = sqlExecutor.queryForList(