fixed datasource autocommit handling without UserTransaction (#21)

src/main/java/com/zaxxer/sansorm/SqlClosure.java

@@ -188,12 +188,12 @@ public V execute(Connection connection, Object... params) throws SQLException
     */
    public final T execute()
    {
-      boolean hasTxManager = TransactionElf.hasTransactionManager();
-      boolean txOwner = !hasTxManager || TransactionElf.beginOrJoinTransaction();
+      boolean txOwner = !TransactionElf.hasTransactionManager() || TransactionElf.beginOrJoinTransaction();
       Connection connection = null;
       try {
          connection = ConnectionProxy.wrapConnection(dataSource.getConnection());
-         if (txOwner && hasTxManager) {
+         if (txOwner) {
+            // disable autoCommit mode as we are going to handle transaction by ourselves
             connection.setAutoCommit(false);
          }
          return (args == null)
@@ -210,13 +210,12 @@ public final T execute()
             rollback(connection);
          }
          throw new RuntimeException(e);
-      }
-      catch (RuntimeException e) {
+      } catch (Throwable e) {
          if (txOwner) {
             txOwner = false;
             rollback(connection);
          }
-         throw e; // no need to wrap
+         throw e;
       }
       finally {
          try {

src/test/java/com/zaxxer/sansorm/SqlClosureTest.java

@@ -0,0 +1,114 @@
+package com.zaxxer.sansorm;
+
+import org.h2.jdbcx.JdbcDataSource;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.sansorm.TestUtils;
+
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+@RunWith(Parameterized.class)
+public class SqlClosureTest {
+   @Parameterized.Parameters(name = "autocommit={0}, ut={1}")
+   public static Collection<Object[]> data() {
+      return Arrays.asList(new Object[][] {
+         { true, true }, { true, false }, { false, true }, { false, false }
+      });
+   }
+
+   @Parameterized.Parameter(0)
+   public boolean withAutoCommit;
+
+   @Parameterized.Parameter(1)
+   public boolean withUserTx;
+
+   @Before // not @BeforeClass to have fresh table in each test, also sde
+   public void setUp() throws IOException {
+      final JdbcDataSource dataSource = TestUtils.makeH2DataSource(/*autoCommit=*/withAutoCommit);
+      if (withUserTx) {
+         SansOrm.initializeTxSimple(dataSource);
+      } else {
+         SansOrm.initializeTxNone(dataSource);
+      }
+      SqlClosureElf.executeUpdate("CREATE TABLE tx_test (string VARCHAR(128))");
+   }
+
+   @After // not @AfterClass to have fresh table in each test
+   public void tearDown() {
+      SqlClosureElf.executeUpdate("DROP TABLE tx_test");
+      SansOrm.deinitialize();
+   }
+
+   @Test
+   public void shouldSupportNestedCalls() {
+      final Set<String> insertedValues = SqlClosure.sqlExecute(c -> {
+         SqlClosureElf.executeUpdate(c, "INSERT INTO tx_test VALUES (?)", "1");
+
+         // here goes nested SqlClosure
+         SqlClosure.sqlExecute(cNested -> SqlClosureElf.executeUpdate(cNested, "INSERT INTO tx_test VALUES (?)", "2"));
+
+         return getStrings(c);
+      });
+      assertThat(insertedValues).containsOnly("1", "2");
+   }
+
+   @Test
+   public void shouldRollbackHighestTx() {
+      assertThatThrownBy(() -> SqlClosure.sqlExecute(c -> {
+         SqlClosureElf.executeUpdate(c, "INSERT INTO tx_test VALUES (?)", "3");
+
+         // here goes nested SqlClosure
+         SqlClosure.sqlExecute(cNested -> {
+            SqlClosureElf.executeUpdate(cNested, "INSERT INTO tx_test VALUES (?)", "4");
+            throw new RuntimeException("boom!");
+         });
+
+         return SqlClosureElf.executeUpdate(c, "INSERT INTO tx_test VALUES (?)", "5");
+      })).isInstanceOf(RuntimeException.class).hasMessage("boom!");
+
+      final Set<String> insertedValues = SqlClosure.sqlExecute(SqlClosureTest::getStrings);
+      assertThat(insertedValues).isEmpty();
+   }
+
+   @Test
+   public void shouldRollbackNestedClosuresWithUserTransaction() {
+      assertThatThrownBy(() -> SqlClosure.sqlExecute(c -> {
+         SqlClosureElf.executeUpdate(c, "INSERT INTO tx_test VALUES (?)", "6");
+
+         // here goes nested SqlClosure
+         SqlClosure.sqlExecute(cNested -> SqlClosureElf.executeUpdate(cNested, "INSERT INTO tx_test VALUES (?)", "7"));
+
+         SqlClosureElf.executeUpdate(c, "INSERT INTO tx_test VALUES (?)", "8");
+         throw new Error("boom!"); // ie something not or type SQLException or RuntimeException
+      })).isInstanceOf(Error.class).hasMessage("boom!");
+
+      final Set<String> insertedValues = SqlClosure.sqlExecute(SqlClosureTest::getStrings);
+      if (withUserTx) {
+         assertThat(insertedValues).containsOnly().as("With UserTransaction nested closures share same tx scope");
+      } else {
+         assertThat(insertedValues).containsOnly("7").as("Without UserTransaction every closure defines it's own tx scope");
+      }
+   }
+
+   static Set<String> getStrings(Connection c) throws SQLException {
+      ResultSet rs = SqlClosureElf.executeQuery(c, "SELECT string FROM tx_test;");
+      Set<String> result = new HashSet<>();
+      while (rs.next()) {
+         result.add(rs.getString(1));
+      }
+      return result;
+   }
+}

src/test/java/org/sansorm/QueryTest.java

@@ -1,11 +1,9 @@
 package org.sansorm;
 
-import org.h2.jdbcx.JdbcDataSource;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.io.IOException;
 import java.sql.PreparedStatement;
 import java.sql.Timestamp;
 import java.util.Date;
@@ -26,16 +24,10 @@
 
 public class QueryTest
 {
-   public static void setUpDataSourceWithSimpleTx() throws IOException {
-      final JdbcDataSource dataSource = new JdbcDataSource();
-      dataSource.setUrl("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1");
-      SansOrm.initializeTxSimple(dataSource);
-   }
-
    @BeforeClass
-   public static void setup() throws Throwable
+   public static void setup()
    {
-      setUpDataSourceWithSimpleTx();
+      SansOrm.initializeTxNone(TestUtils.makeH2DataSource());
       SqlClosureElf.executeUpdate("CREATE TABLE target_class1 ("
          + "id INTEGER NOT NULL IDENTITY PRIMARY KEY, "
          + "timestamp TIMESTAMP, "

src/test/java/org/sansorm/QueryTest2.java

@@ -8,7 +8,6 @@
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sansorm.QueryTest.setUpDataSourceWithSimpleTx;
 
 import com.zaxxer.sansorm.SansOrm;
 import com.zaxxer.sansorm.SqlClosureElf;
@@ -19,7 +18,7 @@ public class QueryTest2
    @BeforeClass
    public static void setup() throws Throwable
    {
-      setUpDataSourceWithSimpleTx();
+      SansOrm.initializeTxNone(TestUtils.makeH2DataSource());
       SqlClosureElf.executeUpdate(
          "CREATE TABLE TargetClass2 ("
             + " id INTEGER NOT NULL IDENTITY PRIMARY KEY,"

src/test/java/org/sansorm/TestUtils.java

@@ -0,0 +1,53 @@
+package org.sansorm;
+
+import org.h2.jdbcx.JdbcDataSource;
+import org.sqlite.SQLiteConfig;
+import org.sqlite.SQLiteDataSource;
+
+import java.io.File;
+
+import com.zaxxer.hikari.HikariConfig;
+import com.zaxxer.hikari.HikariDataSource;
+
+public final class TestUtils {
+   private TestUtils() {
+   }
+
+   public static JdbcDataSource makeH2DataSource() {
+      return makeH2DataSource(true);
+   }
+
+   public static JdbcDataSource makeH2DataSource(boolean autoCommit) {
+      final JdbcDataSource dataSource = new JdbcDataSource();
+      dataSource.setUrl(String.format("jdbc:h2:mem:test;DB_CLOSE_DELAY=-1;autocommit=%s", autoCommit ? "on" : "off"));
+      return dataSource;
+   }
+
+   public static HikariDataSource makeSQLiteDataSource() {
+      return makeSQLiteDataSource(null, true);
+   }
+
+   public static HikariDataSource makeSQLiteDataSource(File db) {
+      return makeSQLiteDataSource(null, true);
+   }
+
+   public static HikariDataSource makeSQLiteDataSource(boolean autoCommit) {
+      return makeSQLiteDataSource(null, autoCommit);
+   }
+
+   public static HikariDataSource makeSQLiteDataSource(File db, boolean autoCommit) {
+      final SQLiteConfig sconfig = new SQLiteConfig();
+      sconfig.setJournalMode(SQLiteConfig.JournalMode.MEMORY);
+      SQLiteDataSource sds = new SQLiteDataSource(sconfig);
+      sds.setUrl(db == null
+         ? "jdbc:sqlite::memory:"
+         : "jdbc:sqlite:" + db.getAbsolutePath()
+      );
+
+      HikariConfig hconfig = new HikariConfig();
+      hconfig.setAutoCommit(autoCommit);
+      hconfig.setDataSource(sds);
+      hconfig.setMaximumPoolSize(1);
+      return new HikariDataSource(hconfig);
+   }
+}

src/test/java/org/sansorm/sqlite/QueryTestSQLite.java

@@ -2,8 +2,7 @@
 
 import org.junit.AfterClass;
 import org.junit.Test;
-import org.sqlite.SQLiteConfig;
-import org.sqlite.SQLiteDataSource;
+import org.sansorm.TestUtils;
 
 import java.io.Closeable;
 import java.io.File;
@@ -17,7 +16,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import com.zaxxer.hikari.HikariConfig;
 import com.zaxxer.hikari.HikariDataSource;
 import com.zaxxer.sansorm.OrmElf;
 import com.zaxxer.sansorm.SansOrm;
@@ -27,22 +25,9 @@
 import com.zaxxer.sansorm.internal.Introspector;
 
 public class QueryTestSQLite {
-   public static Closeable prepareSQLiteDatasource(File db) throws IOException {
-      final SQLiteConfig sconfig = new SQLiteConfig();
-      sconfig.setJournalMode(SQLiteConfig.JournalMode.MEMORY);
-      SQLiteDataSource sds = new SQLiteDataSource(sconfig);
-      sds.setUrl(db == null
-         ? "jdbc:sqlite::memory:"
-         : "jdbc:sqlite:" + db.getAbsolutePath()
-      );
-
-      HikariConfig hconfig = new HikariConfig();
-      hconfig.setAutoCommit(false);
-      hconfig.setDataSource(sds);
-      hconfig.setMaximumPoolSize(1);
-      HikariDataSource hds = new HikariDataSource(hconfig);
-
-      SansOrm.initializeTxSimple(hds);
+   public static Closeable prepareSQLiteDatasource(File db) {
+      HikariDataSource hds = TestUtils.makeSQLiteDataSource(db);
+      SansOrm.initializeTxNone(hds);
       SqlClosureElf.executeUpdate("CREATE TABLE IF NOT EXISTS TargetClassSQL ("
          + "id integer PRIMARY KEY AUTOINCREMENT,"
          + "string text NOT NULL,"