From 8dd438be61da8e415c1b27686fe4af477f7d7d9d Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 18 Feb 2025 07:33:48 +1300 Subject: [PATCH] #3564 Fix for nested transaction with batch-mode does not flush (#3565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- README.md | 5 -- .../java/io/ebeaninternal/api/ScopeTrans.java | 2 + .../ebeaninternal/api/ScopedTransaction.java | 2 +- .../server/core/DefaultServer.java | 4 ++ .../transaction/TestExecuteComplete.java | 2 +- .../org/tests/transaction/TestNested.java | 8 ++-- .../transaction/TestNestedTransaction.java | 46 ++++++++++++++++++- 7 files changed, 55 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 732345e16..785b48c0e 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,6 @@ Work at the highest level of abstraction and drop down levels as needed. - - - - - diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/ScopeTrans.java b/ebean-core/src/main/java/io/ebeaninternal/api/ScopeTrans.java index 348f1e4a4..00f54d987 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/ScopeTrans.java +++ b/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); diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/ScopedTransaction.java b/ebean-core/src/main/java/io/ebeaninternal/api/ScopedTransaction.java index 9f46c97a7..c5762c0d0 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/ScopedTransaction.java +++ b/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 caughtThrowable(T e) { return current.caughtThrowable(e); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 330302787..d3a376e70 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/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 { diff --git a/ebean-test/src/test/java/org/tests/transaction/TestExecuteComplete.java b/ebean-test/src/test/java/org/tests/transaction/TestExecuteComplete.java index db31d630f..297525732 100644 --- a/ebean-test/src/test/java/org/tests/transaction/TestExecuteComplete.java +++ b/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(); diff --git a/ebean-test/src/test/java/org/tests/transaction/TestNested.java b/ebean-test/src/test/java/org/tests/transaction/TestNested.java index 798769831..3389f27cb 100644 --- a/ebean-test/src/test/java/org/tests/transaction/TestNested.java +++ b/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) { diff --git a/ebean-test/src/test/java/org/tests/transaction/TestNestedTransaction.java b/ebean-test/src/test/java/org/tests/transaction/TestNestedTransaction.java index 4e4887ac3..737ebf889 100644 --- a/ebean-test/src/test/java/org/tests/transaction/TestNestedTransaction.java +++ b/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 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"); + } + } + }