From 4f984ff2d4d82e5c1d29bcdf207b2a21546d076d Mon Sep 17 00:00:00 2001 From: Noemi Praml Date: Mon, 14 Oct 2024 16:40:53 +0200 Subject: [PATCH 1/5] add: failing test --- .../transaction/TestTransactionReadOnly.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java diff --git a/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java b/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java new file mode 100644 index 0000000000..a202bfa1eb --- /dev/null +++ b/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java @@ -0,0 +1,31 @@ +package org.tests.transaction; + +import io.ebean.DB; +import io.ebean.Transaction; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.Test; +import org.tests.model.basic.Customer; +import org.tests.model.basic.ResetBasicData; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class TestTransactionReadOnly extends BaseTestCase { + + @Test + public void testReadOnlyTransactionWithUpdateQuery() { + + ResetBasicData.reset(); + + try (Transaction txn = DB.beginTransaction()) { + txn.setReadOnly(true); + + assertThatThrownBy(() -> DB.update(Customer.class).set("name", "Rob2").where().eq("name", "Rob").update()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + txn.commit(); + } + + } + +} From e08985d7c5030afe54b80003ddb6719a55b3ffdb Mon Sep 17 00:00:00 2001 From: Noemi Praml Date: Mon, 14 Oct 2024 16:52:30 +0200 Subject: [PATCH 2/5] suggested solution --- .../io/ebeaninternal/server/transaction/JdbcTransaction.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java index e1b3b773ef..14987e8355 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java @@ -417,6 +417,10 @@ public final void depthReset() { @Override public final void markNotQueryOnly() { this.queryOnly = false; + // if the transaction is readonly, it should not allow updates/deletes + if (localReadOnly){ + throw new IllegalStateException("This transaction is read-only"); + } } @Override From 9ca6fa31e597e73d71de10be43c31948144a70ae Mon Sep 17 00:00:00 2001 From: Noemi Praml Date: Thu, 17 Oct 2024 16:19:03 +0200 Subject: [PATCH 3/5] more tests --- .../transaction/TestTransactionReadOnly.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java b/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java index a202bfa1eb..b644d8a592 100644 --- a/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java +++ b/ebean-test/src/test/java/org/tests/transaction/TestTransactionReadOnly.java @@ -4,9 +4,13 @@ import io.ebean.Transaction; import io.ebean.xtest.BaseTestCase; import org.junit.jupiter.api.Test; +import org.tests.model.basic.Country; import org.tests.model.basic.Customer; +import org.tests.model.basic.OrderDetail; import org.tests.model.basic.ResetBasicData; +import java.util.Arrays; + import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestTransactionReadOnly extends BaseTestCase { @@ -23,6 +27,93 @@ public void testReadOnlyTransactionWithUpdateQuery() { .isInstanceOf(IllegalStateException.class) .hasMessageContaining("This transaction is read-only"); + assertThatThrownBy(() -> DB.sqlUpdate("update o_customer set name = ? where name = ?") + .setParameter(1, "Rob2") + .setParameter(2, "Rob").execute()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.createUpdate(Customer.class, "update customer set name = ? where name = ?") + .setParameter(1, "Rob2") + .setParameter(2, "Rob").execute()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + Customer customer = DB.find(Customer.class).where().eq("name", "Rob").findOne(); + customer.setName("Rob2"); + assertThatThrownBy(() -> DB.save(customer)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + txn.commit(); + } + + } + + @Test + public void testReadOnlyTransactionWithDeleteQuery() { + + ResetBasicData.reset(); + + try (Transaction txn = DB.beginTransaction()) { + txn.setReadOnly(true); + + assertThatThrownBy(() -> DB.find(OrderDetail.class).where().eq("id", 1).delete()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.sqlUpdate("delete o_order_detail where id = ?") + .setParameter(1, 1).execute()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.createUpdate(OrderDetail.class, "delete o_order_detail where id = ?") + .setParameter(1, 1).execute()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + OrderDetail orderDetail = DB.find(OrderDetail.class).where().eq("id", 1).findOne(); + assertThatThrownBy(() -> DB.delete(orderDetail)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.deleteAll(Arrays.asList(orderDetail))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + txn.commit(); + } + + } + + @Test + public void testReadOnlyTransactionWithInsertQuery() { + + ResetBasicData.reset(); + + try (Transaction txn = DB.beginTransaction()) { + txn.setReadOnly(true); + + Country country = new Country(); + country.setName("Germany"); + country.setCode("DE"); + + assertThatThrownBy(() -> DB.save(country)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.saveAll(country)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.insert(country)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + + assertThatThrownBy(() -> DB.insertAll(Arrays.asList(country))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("This transaction is read-only"); + txn.commit(); } From 4438d5db7df570fda4737901a11fe9e735a8b1b2 Mon Sep 17 00:00:00 2001 From: Noemi Praml Date: Thu, 17 Oct 2024 16:19:19 +0200 Subject: [PATCH 4/5] Fixes in DefaultPersister --- .../io/ebeaninternal/server/persist/DefaultPersister.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index c87d9e65b3..2a57939246 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -87,6 +87,7 @@ public int executeOrmUpdate(Update update, Transaction t) { private int executeOrQueue(PersistRequest request) { try { request.initTransIfRequired(); + request.transaction().markNotQueryOnly(); int rc = request.executeOrQueue(); request.commitTransIfRequired(); return rc; @@ -383,6 +384,7 @@ public void update(EntityBean entityBean, Transaction t) { req.checkDraft(); try { req.initTransIfRequiredWithBatchCascade(); + req.transaction().markNotQueryOnly(); if (req.isReference()) { // its a reference so see if there are manys to save... if (req.isPersistCascade()) { @@ -430,6 +432,7 @@ public void insert(EntityBean bean, InsertOptions insertOptions, Transaction t) } try { req.initTransIfRequiredWithBatchCascade(); + req.transaction().markNotQueryOnly(); insert(req); req.resetDepth(); req.commitTransIfRequired(); @@ -562,6 +565,7 @@ private int deleteRequest(PersistRequestBean req, PersistRequestBean draft } try { req.initTransIfRequiredWithBatchCascade(); + req.transaction().markNotQueryOnly(); int rows = delete(req); if (draftReq != null) { // delete the 'draft' bean ('live' bean deleted first) From a452fc0e211ba8afeaa3a5ee33f082ffb7355ad3 Mon Sep 17 00:00:00 2001 From: Noemi Praml Date: Thu, 17 Oct 2024 16:30:42 +0200 Subject: [PATCH 5/5] call request.markNotQueryOnly --- .../java/io/ebeaninternal/server/core/PersistRequest.java | 7 +++++++ .../io/ebeaninternal/server/persist/DefaultPersister.java | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequest.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequest.java index afba654a39..8e5de354f4 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequest.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequest.java @@ -148,6 +148,13 @@ public void initTransIfRequired() { persistCascade = transaction.isPersistCascade(); } + /** + * Mark the underlying transaction as not being query only. + */ + public void markNotQueryOnly() { + transaction.markNotQueryOnly(); + } + /** * Return the type of this request. One of INSERT, UPDATE, DELETE, UPDATESQL or CALLABLESQL. */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index 2a57939246..1631e9d869 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -87,7 +87,7 @@ public int executeOrmUpdate(Update update, Transaction t) { private int executeOrQueue(PersistRequest request) { try { request.initTransIfRequired(); - request.transaction().markNotQueryOnly(); + request.markNotQueryOnly(); int rc = request.executeOrQueue(); request.commitTransIfRequired(); return rc; @@ -384,7 +384,7 @@ public void update(EntityBean entityBean, Transaction t) { req.checkDraft(); try { req.initTransIfRequiredWithBatchCascade(); - req.transaction().markNotQueryOnly(); + req.markNotQueryOnly(); if (req.isReference()) { // its a reference so see if there are manys to save... if (req.isPersistCascade()) { @@ -432,7 +432,7 @@ public void insert(EntityBean bean, InsertOptions insertOptions, Transaction t) } try { req.initTransIfRequiredWithBatchCascade(); - req.transaction().markNotQueryOnly(); + req.markNotQueryOnly(); insert(req); req.resetDepth(); req.commitTransIfRequired(); @@ -565,7 +565,7 @@ private int deleteRequest(PersistRequestBean req, PersistRequestBean draft } try { req.initTransIfRequiredWithBatchCascade(); - req.transaction().markNotQueryOnly(); + req.markNotQueryOnly(); int rows = delete(req); if (draftReq != null) { // delete the 'draft' bean ('live' bean deleted first)