Keep nested JDBC save failures rollback-only

Author: Philip Z

examples/committed-events/src/test/java/io/teaql/examples/committed/CommittedEventsExampleTest.java

@@ -203,6 +203,21 @@ record -> {
                 new Object[] {42L}).get(0).get("name"));
         assertEquals(2L, ((Number) sql.queryForList("SELECT version FROM order_data WHERE id = ?",
                 new Object[] {42L}).get(0).get("version")).longValue());
+
+        // Even if application code catches the inner save failure, the shared
+        // JDBC transaction must not commit a business row without its outbox.
+        order.updateProperty("name", "caught failure must roll back");
+        assertThrows(IllegalStateException.class, () -> sql.executeInTransaction(() -> {
+            try {
+                order.auditAs("reject caught outbox failure").save(context);
+            } catch (RuntimeException expected) {
+                // Simulate a caller that handles the failed save and returns.
+            }
+        }));
+        assertEquals("submitted", sql.queryForList("SELECT name FROM order_data WHERE id = ?",
+                new Object[] {42L}).get(0).get("name"));
+        assertEquals(2L, ((Number) sql.queryForList("SELECT version FROM order_data WHERE id = ?",
+                new Object[] {42L}).get(0).get("version")).longValue());
         runtime.close();
     }
 }

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

@@ -21,6 +21,7 @@ public class JdbcSqlExecutor implements SqlExecutionAdapter {
     // A held view retains only this token, not the executor, connection or DataSource.
     private final Object storageIdentity = new Object();
     private final ThreadLocal<Connection> transactionConnection = new ThreadLocal<>();
+    private final ThreadLocal<Boolean> transactionRollbackOnly = new ThreadLocal<>();
     private final List<Consumer<Connection>> connectionInitializers = new CopyOnWriteArrayList<>();
 
     public JdbcSqlExecutor(DataSource dataSource) {
@@ -395,21 +396,35 @@ public int[] batchUpdate(String sql, List<Object[]> paramsList) {
     @Override
     public void executeInTransaction(Runnable action) {
         if (transactionConnection.get() != null) {
-            action.run();
+            try {
+                action.run();
+            } catch (RuntimeException | Error failure) {
+                // The nested action shares the outer JDBC transaction. Without a
+                // savepoint, its partial writes cannot be committed safely even if
+                // application code catches the exception before the outer returns.
+                transactionRollbackOnly.set(true);
+                throw failure;
+            }
             return;
         }
         try (Connection connection = openConnection()) {
             boolean autoCommit = connection.getAutoCommit();
             connection.setAutoCommit(false);
             transactionConnection.set(connection);
+            transactionRollbackOnly.set(false);
             try {
                 action.run();
+                if (Boolean.TRUE.equals(transactionRollbackOnly.get())) {
+                    throw new IllegalStateException(
+                            "JDBC transaction is rollback-only after a nested failure");
+                }
                 connection.commit();
             } catch (RuntimeException | Error failure) {
                 connection.rollback();
                 throw failure;
             } finally {
                 transactionConnection.remove();
+                transactionRollbackOnly.remove();
                 connection.setAutoCommit(autoCommit);
             }
         } catch (SQLException e) {

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

@@ -18,6 +18,7 @@
 import java.util.logging.Logger;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 public class JdbcSqlExecutorTest {
@@ -62,6 +63,34 @@ public void testExecuteAndInsert() throws Exception {
         }
     }
 
+    @Test
+    public void nestedFailureCaughtByCallerStillRollsBackOuterTransaction() {
+        assertThrows(IllegalStateException.class, () -> sqlExecutor.executeInTransaction(() -> {
+            sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                    new Object[] {91, "before failure", 20});
+            try {
+                sqlExecutor.executeInTransaction(() -> {
+                    sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                            new Object[] {92, "failed save", 20});
+                    throw new IllegalStateException("simulated outbox append failure");
+                });
+            } catch (IllegalStateException expected) {
+                // Application code may log a failed save and continue its outer callback.
+            }
+        }));
+
+        assertEquals(0, ((Number) sqlExecutor.queryForList(
+                "SELECT count(*) AS total FROM test_user", new Object[0])
+                .get(0).get("total")).intValue());
+
+        sqlExecutor.executeInTransaction(() -> sqlExecutor.executeInTransaction(() ->
+                sqlExecutor.update("INSERT INTO test_user (id, name, age) VALUES (?, ?, ?)",
+                        new Object[] {93, "next transaction", 20})));
+        assertEquals(1, ((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(