From 838502eb5e78f11fa1060996a02a95ec9f474368 Mon Sep 17 00:00:00 2001 From: Antoine Date: Thu, 13 Aug 2026 14:52:58 +0200 Subject: [PATCH 1/4] Add test for #1852 : cascaded delete order broken by a write from a persist callback A join entity owns the foreign key to the bean its delete cascades to, so the join row has to be deleted first. When a BeanPersistController writes to the database from preDelete, that write flushes the batch from inside the flush that is already running : the outer flush has taken the join rows out of their bean holder, so the inner flush finds only the assets and executes them first, which fails on the foreign key. BatchControl flush [DcoLink:0 d:2, DcoAsset:1 d:2] <- outer flush BatchControl flush [DcoLink:0 d:0, DcoAsset:1 d:2, DcoAudit:2 i:1] <- from preDelete The scenario is fixed as a side effect of a2f954a60 (#3830, released in 18.3.0) which moved controllerPreDelete() ahead of the cascade. The test pins that down : it fails with a DataIntegrityException on a2f954a60~1 and passes on master. The graph is fetched up front on purpose, a lazy load would flush the batch on its own and hide the ordering. --- .../tests/delete/TestDeleteCascadeOrder.java | 95 +++++++++++++++++++ .../org/tests/model/deleteorder/DcoAsset.java | 34 +++++++ .../org/tests/model/deleteorder/DcoAudit.java | 30 ++++++ .../org/tests/model/deleteorder/DcoLink.java | 46 +++++++++ .../model/deleteorder/DcoLinkAdapter.java | 54 +++++++++++ .../tests/model/deleteorder/DcoParent.java | 50 ++++++++++ 6 files changed, 309 insertions(+) create mode 100644 ebean-test/src/test/java/org/tests/delete/TestDeleteCascadeOrder.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoAsset.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoAudit.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoLink.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoLinkAdapter.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoParent.java diff --git a/ebean-test/src/test/java/org/tests/delete/TestDeleteCascadeOrder.java b/ebean-test/src/test/java/org/tests/delete/TestDeleteCascadeOrder.java new file mode 100644 index 0000000000..80f1bbe407 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/delete/TestDeleteCascadeOrder.java @@ -0,0 +1,95 @@ +package org.tests.delete; + +import io.ebean.DB; +import io.ebean.Transaction; +import io.ebean.test.LoggedSql; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.tests.model.deleteorder.DcoAsset; +import org.tests.model.deleteorder.DcoLinkAdapter; +import org.tests.model.deleteorder.DcoParent; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * A join entity owns the foreign key to the bean its delete cascades to, so the join row has to be + * deleted first. When a persist callback writes to the database it flushes the batch from inside the + * flush that is already running : the outer flush has taken the join rows out of their bean holder, + * so the inner flush only finds the assets and executes them first. + *

+ * See #1852. + */ +class TestDeleteCascadeOrder extends BaseTestCase { + + @AfterEach + void after() { + DcoLinkAdapter.reset(); + } + + @Test + void deleteLinkBeforeAsset() { + assertLinkDeletedBeforeAsset(deleteAllLinks(newParent(2))); + } + + @Test + void deleteLinkBeforeAsset_whenCallbackWritesOnPreDelete() { + DcoLinkAdapter.writeOnPreDelete(true); + + assertLinkDeletedBeforeAsset(deleteAllLinks(newParent(2))); + } + + @Test + void deleteLinkBeforeAsset_whenCallbackWritesOnPostDelete() { + DcoLinkAdapter.writeOnPostDelete(true); + + assertLinkDeletedBeforeAsset(deleteAllLinks(newParent(2))); + } + + private Long newParent(int assetCount) { + DcoParent parent = new DcoParent("parent-" + assetCount); + for (int i = 0; i < assetCount; i++) { + parent.addAsset(new DcoAsset("asset-" + i)); + } + DB.save(parent); + return parent.getId(); + } + + /** + * Remove every link of the parent, which cascades the delete to the assets behind them. The graph is + * fetched up front : a lazy load would flush the batch on its own and hide the ordering. + */ + private List deleteAllLinks(Long parentId) { + try (Transaction txn = DB.beginTransaction()) { + txn.setBatchMode(true); + DcoParent parent = DB.find(DcoParent.class) + .fetch("links") + .fetch("links.asset") + .where().idEq(parentId) + .findOne(); + parent.getLinks().clear(); + + LoggedSql.start(); + DB.save(parent); + txn.commit(); + return LoggedSql.stop(); + } + } + + private void assertLinkDeletedBeforeAsset(List sql) { + assertThat(firstIndexOf(sql, "delete from dco_link")) + .as("the join row must be deleted before the asset it references, statements were :%n%s", String.join("\n", sql)) + .isLessThan(firstIndexOf(sql, "delete from dco_asset")); + } + + private int firstIndexOf(List sql, String fragment) { + for (int i = 0; i < sql.size(); i++) { + if (sql.get(i).contains(fragment)) { + return i; + } + } + throw new AssertionError("no statement containing '" + fragment + "', statements were :\n" + String.join("\n", sql)); + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAsset.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAsset.java new file mode 100644 index 0000000000..69d9ea21fc --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAsset.java @@ -0,0 +1,34 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Version; + +/** + * Owned by a {@link DcoLink} through a cascading OneToOne, so deleting the link deletes the asset. + */ +@Entity +public class DcoAsset { + + @Id + @GeneratedValue + Long id; + + @Version + Long version; + + String name; + + public DcoAsset(String name) { + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAudit.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAudit.java new file mode 100644 index 0000000000..aae719d94a --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoAudit.java @@ -0,0 +1,30 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; + +/** + * Written from the delete callback of {@link DcoLink}, the way an audit or an outbox row is. + */ +@Entity +public class DcoAudit { + + @Id + @GeneratedValue + Long id; + + String message; + + public DcoAudit(String message) { + this.message = message; + } + + public Long getId() { + return id; + } + + public String getMessage() { + return message; + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLink.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLink.java new file mode 100644 index 0000000000..e4b8ac1a28 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLink.java @@ -0,0 +1,46 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToOne; + +/** + * Join entity between {@link DcoParent} and {@link DcoAsset}. It owns the foreign key to the asset, + * so the link row has to be deleted before the asset it points at. + *

+ * A real entity (rather than a plain join table) so that its delete fires a persistence callback, + * see {@link DcoLinkAdapter}. + */ +@Entity +public class DcoLink { + + @Id + @GeneratedValue + Long id; + + @ManyToOne(optional = false) + DcoParent parent; + + @OneToOne(optional = false, cascade = CascadeType.ALL, orphanRemoval = true) + DcoAsset asset; + + public DcoLink(DcoParent parent, DcoAsset asset) { + this.parent = parent; + this.asset = asset; + } + + public Long getId() { + return id; + } + + public DcoParent getParent() { + return parent; + } + + public DcoAsset getAsset() { + return asset; + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLinkAdapter.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLinkAdapter.java new file mode 100644 index 0000000000..495f9d6c22 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoLinkAdapter.java @@ -0,0 +1,54 @@ +package org.tests.model.deleteorder; + +import io.ebean.event.BeanPersistAdapter; +import io.ebean.event.BeanPersistRequest; + +/** + * Persists a bean from inside a delete callback of {@link DcoLink}, the way an audit or an outbox row + * is written. The BeanPersistController javadoc documents this as a supported use case. + *

+ * Off by default so that the callbacks only fire for the tests that ask for them. + */ +public class DcoLinkAdapter extends BeanPersistAdapter { + + private static boolean writeOnPreDelete; + private static boolean writeOnPostDelete; + + public static void writeOnPreDelete(boolean enabled) { + writeOnPreDelete = enabled; + } + + public static void writeOnPostDelete(boolean enabled) { + writeOnPostDelete = enabled; + } + + public static void reset() { + writeOnPreDelete = false; + writeOnPostDelete = false; + } + + @Override + public boolean isRegisterFor(Class cls) { + return DcoLink.class.equals(cls); + } + + @Override + public boolean preDelete(BeanPersistRequest request) { + if (writeOnPreDelete) { + audit(request, "pre"); + } + return true; + } + + @Override + public void postDelete(BeanPersistRequest request) { + if (writeOnPostDelete) { + audit(request, "post"); + } + } + + private void audit(BeanPersistRequest request, String phase) { + DcoLink link = (DcoLink) request.bean(); + request.database().save(new DcoAudit(phase + " delete of link " + link.getId()), request.transaction()); + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParent.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParent.java new file mode 100644 index 0000000000..bbb65f27c9 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParent.java @@ -0,0 +1,50 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.OneToMany; + +import java.util.ArrayList; +import java.util.List; + +/** + * Parent of a join entity, see {@link DcoLink}. + */ +@Entity +public class DcoParent { + + @Id + @GeneratedValue + Long id; + + String name; + + @OneToMany(cascade = CascadeType.ALL, mappedBy = "parent", orphanRemoval = true) + List links = new ArrayList<>(); + + public DcoParent(String name) { + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getLinks() { + return links; + } + + public void addAsset(DcoAsset asset) { + links.add(new DcoLink(this, asset)); + } +} From e7278e5cdfa79b43c8fc2437bd3b7b4015f2e3ca Mon Sep 17 00:00:00 2001 From: Antoine Date: Thu, 13 Aug 2026 14:57:17 +0200 Subject: [PATCH 2/4] Add failing test reproducing #1852 : out of order cascaded delete on a self referencing tree The shape reported on the issue in 2019 : a container cascades the delete down a tree of TreeBean, and the deletes are not issued deepest first. On 18.4.0 : delete from dco_tree where id in (?) -- the root, whose children are still there delete from dco_tree where id in (?,?,?) delete from dco_tree where id in (?,?) Referential integrity constraint violation: FK_DCO_TREE_PARENT_ID. This is a different defect from the batch reordering fixed by a2f954a60 : nothing is batched here, the recursion itself walks the tree in the wrong order. Disabled so it does not break the build, remove the annotation to see the failure. --- .../delete/TestDeleteTreeCascadeOrder.java | 50 ++++++++++++++++++ .../org/tests/model/deleteorder/DcoTree.java | 52 +++++++++++++++++++ .../model/deleteorder/DcoTreeContainer.java | 32 ++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 ebean-test/src/test/java/org/tests/delete/TestDeleteTreeCascadeOrder.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoTree.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoTreeContainer.java diff --git a/ebean-test/src/test/java/org/tests/delete/TestDeleteTreeCascadeOrder.java b/ebean-test/src/test/java/org/tests/delete/TestDeleteTreeCascadeOrder.java new file mode 100644 index 0000000000..d026917280 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/delete/TestDeleteTreeCascadeOrder.java @@ -0,0 +1,50 @@ +package org.tests.delete; + +import io.ebean.DB; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.tests.model.deleteorder.DcoTree; +import org.tests.model.deleteorder.DcoTreeContainer; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The tree shape reported on #1852 : + * deleting the container cascades down a self referencing tree, and the deletes have to reach the + * leaves before their parents. + */ +class TestDeleteTreeCascadeOrder extends BaseTestCase { + + /** + * Still reproduces on 18.4.0. The tree is deleted level by level but not deepest first : + * 

+   *   delete from dco_tree where id in (?)      -- the root, whose children are still there
+   *   delete from dco_tree where id in (?,?,?)
+   *   delete from dco_tree where id in (?,?)
+   *
+ * which fails with "Referential integrity constraint violation: FK_DCO_TREE_PARENT_ID". Disabled so + * that it does not break the build, remove the annotation to see the failure. + */ + @Disabled("reproduces #1852, not fixed yet") + @Test + void deleteContainerOfNestedTree() { + DcoTreeContainer container = new DcoTreeContainer(); + DcoTree root = new DcoTree("root"); + + DcoTree child1 = root.addChild("child 1"); + child1.addChild("child 1a").addChild("child 1a1"); + + DcoTree child2 = root.addChild("child 2"); + child2.addChild("child 2a"); + child2.addChild("child 2b"); + + container.getTrees().add(root); + DB.save(container); + + DB.delete(container); + + assertThat(DB.find(DcoTree.class).findCount()).isZero(); + assertThat(DB.find(DcoTreeContainer.class).where().idEq(container.getId()).findCount()).isZero(); + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTree.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTree.java new file mode 100644 index 0000000000..58b9e2be71 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTree.java @@ -0,0 +1,52 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; + +import java.util.ArrayList; +import java.util.List; + +/** + * Self referencing tree, as reported on #1852. + */ +@Entity +public class DcoTree { + + @Id + @GeneratedValue + Long id; + + String name; + + @ManyToOne + DcoTree parent; + + @ManyToOne + DcoTreeContainer container; + + @OneToMany(cascade = CascadeType.ALL, mappedBy = "parent") + List children = new ArrayList<>(); + + public DcoTree(String name) { + this.name = name; + } + + public Long getId() { + return id; + } + + public List getChildren() { + return children; + } + + public DcoTree addChild(String name) { + DcoTree child = new DcoTree(name); + child.parent = this; + children.add(child); + return child; + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTreeContainer.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTreeContainer.java new file mode 100644 index 0000000000..8a4cd988f3 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoTreeContainer.java @@ -0,0 +1,32 @@ +package org.tests.model.deleteorder; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.OneToMany; + +import java.util.ArrayList; +import java.util.List; + +/** + * Holds the roots of a {@link DcoTree}, as reported on #1852. + */ +@Entity +public class DcoTreeContainer { + + @Id + @GeneratedValue + Long id; + + @OneToMany(cascade = CascadeType.ALL) + List trees = new ArrayList<>(); + + public Long getId() { + return id; + } + + public List getTrees() { + return trees; + } +} From 1345c99e79816ed5a033d555460ae191ae3be740 Mon Sep 17 00:00:00 2001 From: Antoine Date: Thu, 13 Aug 2026 16:52:42 +0200 Subject: [PATCH 3/4] FIX: a persist done from a BeanPersistController callback flushes the batch mid-execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3148 stopped a query performed from a callback from flushing the batch that is already executing : BatchControl.executeNow disables flushOnQuery for the duration. A persist done from the same callback is not covered. It reaches BatchControl.executeOrQueue, which flushes, and the statements queued behind the one currently executing are issued early. Saving a parent/child graph in batch while an audit row is written from preInsert issues the children before their parent has an id : insert into dco_link (parent_id, asset_id) values (?,?) NULL not allowed for column "PARENT_ID" Same defect on the delete side, where the join rows are issued after the beans they reference (#1852, #3185) — that path no longer reproduces since a2f954a60 moved controllerPreDelete() ahead of the cascade, but only preDelete was moved, so preInsert and preUpdate still run inside the flush. Guard executeOrQueue with the same reasoning as the existing flushOnQuery guard : while the batch is executing, queue rather than flush. The statements added meanwhile are picked up by the do/while loop in executeAll(). --- .../server/persist/BatchControl.java | 17 ++++- .../tests/delete/TestInsertCascadeOrder.java | 75 +++++++++++++++++++ .../model/deleteorder/DcoParentAdapter.java | 33 ++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java create mode 100644 ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java index 24ed757e46..1633ff452f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java @@ -81,6 +81,13 @@ public final class BatchControl { */ private int bufferMax; + /** + * True while the batched requests are being executed. A persist performed from a + * BeanPersistController callback must not flush the batch that is executing it, the same way + * executeNow() stops a query from doing so. + */ + private boolean executing; + private final Queue[] queues = new Queue[2]; static final int DELETE_QUEUE = 0; @@ -163,8 +170,9 @@ public int executeStatementOrBatch(PersistRequest request, boolean batch, boolea * according to the depth (object graph depth). */ public int executeOrQueue(PersistRequestBean request, boolean batch) throws BatchedSqlException { - if (!batch || (batchFlushOnMixed && !pstmtHolder.isEmpty())) { - // flush when mixing beans and updateSql + if (!executing && (!batch || (batchFlushOnMixed && !pstmtHolder.isEmpty()))) { + // flush when mixing beans and updateSql, unless we are inside the execution of the batch + // itself : flushing then would issue the statements queued behind the current one early flush(); } if (!batch) { @@ -226,7 +234,9 @@ private void flushPstmtHolder(boolean reset) throws BatchedSqlException { void executeNow(ArrayList list) throws BatchedSqlException { boolean old = transaction.isFlushOnQuery(); transaction.setFlushOnQuery(false); - // disable flush on query due transaction callbacks + boolean oldExecuting = executing; + executing = true; + // disable flush on query and on persist due transaction callbacks try { for (int i = 0; i < list.size(); i++) { if (i % batchSize == 0) { @@ -237,6 +247,7 @@ void executeNow(ArrayList list) throws BatchedSqlException { } flushPstmtHolder(); } finally { + executing = oldExecuting; transaction.setFlushOnQuery(old); } } diff --git a/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java b/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java new file mode 100644 index 0000000000..cc88b73468 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java @@ -0,0 +1,75 @@ +package org.tests.delete; + +import io.ebean.DB; +import io.ebean.Transaction; +import io.ebean.test.LoggedSql; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.tests.model.deleteorder.DcoAsset; +import org.tests.model.deleteorder.DcoParent; +import org.tests.model.deleteorder.DcoParentAdapter; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Same defect as the cascaded delete one, on the insert side : a persist done from a BeanPersistController + * flushes the batch from inside the flush that is already running, and the statements queued behind the + * one being executed are issued out of order. + *

+ * #3148 fixed this for a flush triggered by a + * query (BatchControl.executeNow disables flushOnQuery), but a flush triggered by a write goes through + * BatchControl.executeOrQueue, which that guard does not cover. + */ +class TestInsertCascadeOrder extends BaseTestCase { + + @AfterEach + void after() { + DcoParentAdapter.writeOnPreInsert(false); + } + + @Test + void insertParentBeforeItsLinks_whenCallbackWritesDuringFlush() { + DcoParentAdapter.writeOnPreInsert(true); + + List sql = insertParents(3); + + // every parent has to be inserted before the link that points at it + assertThat(lastIndexOf(sql, "insert into dco_parent")) + .as("a parent must be inserted before the links referencing it, statements were :%n%s", String.join("\n", sql)) + .isLessThan(lastIndexOf(sql, "insert into dco_link")); + } + + private List insertParents(int count) { + List parents = new ArrayList<>(); + for (int i = 0; i < count; i++) { + DcoParent parent = new DcoParent("batch-parent-" + i); + parent.addAsset(new DcoAsset("batch-asset-" + i)); + parents.add(parent); + } + + try (Transaction txn = DB.beginTransaction()) { + txn.setBatchMode(true); + txn.setBatchSize(50); + LoggedSql.start(); + DB.saveAll(parents); + txn.commit(); + List sql = LoggedSql.stop(); + System.out.println("---- insert order ----"); + sql.stream().filter(s -> !s.contains("-- bind")).forEach(s -> System.out.println(" " + s)); + return sql; + } + } + + private int lastIndexOf(List sql, String fragment) { + for (int i = sql.size() - 1; i >= 0; i--) { + if (sql.get(i).contains(fragment)) { + return i; + } + } + throw new AssertionError("no statement containing '" + fragment + "', statements were :\n" + String.join("\n", sql)); + } +} diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java new file mode 100644 index 0000000000..d03b8333bb --- /dev/null +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java @@ -0,0 +1,33 @@ +package org.tests.model.deleteorder; + +import io.ebean.event.BeanPersistAdapter; +import io.ebean.event.BeanPersistRequest; + +/** + * Persists a bean from inside the insert callback of {@link DcoParent}, the way an audit or an outbox + * row is written. The BeanPersistController javadoc documents this as a supported use case. + *

+ * Off by default so that the callback only fires for the tests that ask for it. + */ +public class DcoParentAdapter extends BeanPersistAdapter { + + private static boolean writeOnPreInsert; + + public static void writeOnPreInsert(boolean enabled) { + writeOnPreInsert = enabled; + } + + @Override + public boolean isRegisterFor(Class cls) { + return DcoParent.class.equals(cls); + } + + @Override + public boolean preInsert(BeanPersistRequest request) { + if (writeOnPreInsert) { + DcoParent parent = (DcoParent) request.bean(); + request.database().save(new DcoAudit("inserting " + parent.getName()), request.transaction()); + } + return true; + } +} From d6d47449c7b32001945a4257a8a669638f1d4ebb Mon Sep 17 00:00:00 2001 From: Antoine Date: Fri, 14 Aug 2026 09:14:12 +0200 Subject: [PATCH 4/4] FIX: same guard on executeStatementOrBatch, a SqlUpdate from a callback also flushes mid-execution executeStatementOrBatch() flushes on (batchFlushOnMixed && !isBeansEmpty()), and persistedBeans is only cleared once executeAll() returns, so during the batch execution that condition holds and a SqlUpdate run from a BeanPersistController callback re-enters the flush the same way a save does. Reproduced with the same test, the callback running a SqlUpdate instead of a save : insert into dco_link (parent_id, asset_id) values (?,?) NULL not allowed for column "PARENT_ID" The second flush of that method, on pstmtHolder.maxSize() >= batchSize, is left untouched : it can only trigger when the pstmt holder fills up mid-execution and there is no test covering it. --- .../server/persist/BatchControl.java | 5 +++-- .../tests/delete/TestInsertCascadeOrder.java | 16 ++++++++++++++++ .../model/deleteorder/DcoParentAdapter.java | 18 +++++++++++++++++- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java index 1633ff452f..c59c20c087 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java @@ -146,8 +146,9 @@ public void setGetGeneratedKeys(Boolean getGeneratedKeys) { * to the depth. */ public int executeStatementOrBatch(PersistRequest request, boolean batch, boolean addBatch) throws BatchedSqlException { - if (!batch || (batchFlushOnMixed && !isBeansEmpty())) { - // flush when mixing beans and updateSql + if (!executing && (!batch || (batchFlushOnMixed && !isBeansEmpty()))) { + // flush when mixing beans and updateSql, unless we are inside the execution of the batch + // itself : flushing then would issue the statements queued behind the current one early flush(); } if (!batch) { diff --git a/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java b/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java index cc88b73468..204a73e032 100644 --- a/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java +++ b/ebean-test/src/test/java/org/tests/delete/TestInsertCascadeOrder.java @@ -29,6 +29,7 @@ class TestInsertCascadeOrder extends BaseTestCase { @AfterEach void after() { DcoParentAdapter.writeOnPreInsert(false); + DcoParentAdapter.sqlUpdateOnPreInsert(false); } @Test @@ -43,6 +44,21 @@ void insertParentBeforeItsLinks_whenCallbackWritesDuringFlush() { .isLessThan(lastIndexOf(sql, "insert into dco_link")); } + /** + * Same as above but the callback runs a SqlUpdate, which reaches BatchControl by + * executeStatementOrBatch rather than executeOrQueue. + */ + @Test + void insertParentBeforeItsLinks_whenCallbackRunsSqlUpdateDuringFlush() { + DcoParentAdapter.sqlUpdateOnPreInsert(true); + + List sql = insertParents(3); + + assertThat(lastIndexOf(sql, "insert into dco_parent")) + .as("a parent must be inserted before the links referencing it, statements were :%n%s", String.join("\n", sql)) + .isLessThan(lastIndexOf(sql, "insert into dco_link")); + } + private List insertParents(int count) { List parents = new ArrayList<>(); for (int i = 0; i < count; i++) { diff --git a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java index d03b8333bb..985ac1d48c 100644 --- a/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java +++ b/ebean-test/src/test/java/org/tests/model/deleteorder/DcoParentAdapter.java @@ -12,11 +12,20 @@ public class DcoParentAdapter extends BeanPersistAdapter { private static boolean writeOnPreInsert; + private static boolean sqlUpdateOnPreInsert; public static void writeOnPreInsert(boolean enabled) { writeOnPreInsert = enabled; } + /** + * Same as {@link #writeOnPreInsert(boolean)} but through SqlUpdate, which takes the + * BatchControl.executeStatementOrBatch path rather than executeOrQueue. + */ + public static void sqlUpdateOnPreInsert(boolean enabled) { + sqlUpdateOnPreInsert = enabled; + } + @Override public boolean isRegisterFor(Class cls) { return DcoParent.class.equals(cls); @@ -24,10 +33,17 @@ public boolean isRegisterFor(Class cls) { @Override public boolean preInsert(BeanPersistRequest request) { + DcoParent parent = (DcoParent) request.bean(); if (writeOnPreInsert) { - DcoParent parent = (DcoParent) request.bean(); request.database().save(new DcoAudit("inserting " + parent.getName()), request.transaction()); } + if (sqlUpdateOnPreInsert) { + request.database().sqlUpdate("update dco_audit set message = ? where id = ?") + .setParameter(1, "inserting " + parent.getName()) + .setParameter(2, -1L) + .usingTransaction(request.transaction()) + .execute(); + } return true; } }