#3564 Fix for nested transaction with batch-mode does not flush (#3565)

* ADD: Testcase for showing nested transaction with batch-mode not flushing batch on commit

* Update test only - TestNestedTransaction.test_txn_nestedWithInnerBatch()

Update the test to show that we desire the nested transaction changes to be "visible" after a commit regardless of flushOnQuery etc

* #3564 Fix for nested transaction with batch-mode does not flush

- Adds flush() into ScopeTrans.commitTransaction()
- Related fixes in DefaultServer execute() and executeCall() where it was performing extra unnecessary nesting on PersistenceException

* Touch

---------

Co-authored-by: Jonas Pöhler <jonas.poehler@foconis.de>

README.md

@@ -59,11 +59,6 @@ Work at the highest level of abstraction and drop down levels as needed.
           <img width="222px" src="https://group.foconis.com/download/ci/logo/png-72dpi/logo-quer/foconis-analytics-quer.png">
         </a>
       </td>
-      <td align="center" valign="middle">
-        <a href="https://www.payintech.com/" target="_blank">
-          <img width="222px" src="https://ebean.io/images/sponsor_PayinTech-logo-noir.png">
-        </a>
-      </td>
       <td align="center" valign="middle">
         <a href="https://www.premium-minds.com" target="_blank">
           <img width="222px" src="https://ebean.io/images/logo-med-principal.png">

ebean-core/src/main/java/io/ebeaninternal/api/ScopeTrans.java

@@ -142,6 +142,8 @@ public final class ScopeTrans {
       transaction.commit();
     } else {
       nestedCommit = true;
+      transaction.flush();
+      // restore the batch settings
       transaction.setFlushOnQuery(restoreBatchFlushOnQuery);
       transaction.setBatchMode(restoreBatch);
       transaction.setBatchOnCascade(restoreBatchOnCascade);

ebean-core/src/main/java/io/ebeaninternal/api/ScopedTransaction.java

@@ -168,7 +168,7 @@ public final class ScopedTransaction extends SpiTransactionProxy {
   /**
    * Maybe rollback based on TxScope rollback on settings.
    */
-  public Exception caughtThrowable(Exception e) {
+  public <T extends Exception> T caughtThrowable(T e) {
     return current.caughtThrowable(e);
   }
 

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -680,6 +680,8 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
       return callable.call();
     } catch (Error e) {
       throw scopeTrans.caughtError(e);
+    } catch (PersistenceException e) {
+      throw scopeTrans.caughtThrowable(e);
     } catch (Exception e) {
       throw new PersistenceException(scopeTrans.caughtThrowable(e));
     } finally {
@@ -699,6 +701,8 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
       runnable.run();
     } catch (Error e) {
       throw t.caughtError(e);
+    } catch (PersistenceException e) {
+      throw t.caughtThrowable(e);
     } catch (Exception e) {
       throw new PersistenceException(t.caughtThrowable(e));
     } finally {

ebean-test/src/test/java/org/tests/transaction/TestExecuteComplete.java

@@ -67,7 +67,7 @@ public class TestExecuteComplete extends BaseTestCase {
 
   @ForPlatform(Platform.H2)
   @Test
-  public void transactional_errorOnCommit_expect_threadScopeCleanup() {
+  void transactional_errorOnCommit_expect_threadScopeCleanup() {
     try {
       errorOnCommit();
       fail();

ebean-test/src/test/java/org/tests/transaction/TestNested.java

@@ -9,11 +9,10 @@ import java.util.concurrent.Callable;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class TestNested extends BaseTestCase {
+class TestNested extends BaseTestCase {
 
   @Test
-  public void testRunnableFail() {
-
+  void testRunnableFail() {
     try {
       DB.execute(this::willFail);
     } catch (PersistenceException e) {
@@ -22,8 +21,7 @@ public class TestNested extends BaseTestCase {
   }
 
   @Test
-  public void testCallableFail() {
-
+  void testCallableFail() {
     try {
       DB.execute(this::willFailCallable);
     } catch (PersistenceException e) {

ebean-test/src/test/java/org/tests/transaction/TestNestedTransaction.java

@@ -1,14 +1,17 @@
 package org.tests.transaction;
 
-import io.ebean.TxScope;
-import io.ebean.xtest.BaseTestCase;
 import io.ebean.DB;
 import io.ebean.Transaction;
+import io.ebean.TxScope;
+import io.ebean.test.LoggedSql;
+import io.ebean.xtest.BaseTestCase;
+import org.assertj.core.api.ListAssert;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.tests.model.basic.EBasic;
+import org.tests.model.basic.EBasicVer;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -251,4 +254,43 @@ public class TestNestedTransaction extends BaseTestCase {
     }
   }
 
+  @Test
+  public void test_txn_nestedWithInnerBatch() {
+    try (Transaction txn1 = DB.beginTransaction()) {
+      txn1.setFlushOnQuery(false);
+      try (Transaction txn2 = DB.beginTransaction()) {
+        LoggedSql.start();
+        txn2.setBatchMode(true);
+
+        EBasicVer basic = new EBasicVer("New name");
+        basic.setId(1000);
+        basic.setDescription("New description");
+        DB.save(basic);
+
+        txn2.flush();
+
+        basic.setName("OtherName");
+        DB.save(basic);
+
+        // should perform a flush
+        txn2.commit();
+
+        ListAssert<String> sqlAssert = assertThat(LoggedSql.stop()).hasSize(6);
+        sqlAssert.element(0).asString().contains("insert into e_basicver");
+        sqlAssert.element(1).asString().contains("-- bind(1000,New name");
+        sqlAssert.element(2).asString().contains("-- executeBatch() size:1");
+        sqlAssert.element(3).asString().contains("update e_basicver");
+        sqlAssert.element(4).asString().contains("-- bind(OtherName,");
+        sqlAssert.element(5).asString().contains("-- executeBatch() size:1");
+      }
+
+      // we expect the changes in the nested transaction to made visible
+      // regardless of the other transactions flushOnQuery mode
+      EBasicVer eBasicVer = DB.find(EBasicVer.class, 1000);
+      assertThat(eBasicVer.getName())
+        .describedAs("changes in nested transaction were not flushed")
+        .isEqualTo("OtherName");
+    }
+  }
+
 }