PR ebean-orm#3148 - FIX: Recursive batch flush can save beans in wrong order

Author: rPraml

ebean-core/src/main/java/io/ebeaninternal/server/persist/BatchControl.java

@@ -224,14 +224,21 @@ private void flushPstmtHolder(boolean reset) throws BatchedSqlException {
    * Execute all the requests contained in the list.
    */
   void executeNow(ArrayList<PersistRequest> list) throws BatchedSqlException {
-    for (int i = 0; i < list.size(); i++) {
-      if (i % batchSize == 0) {
-        // hit the batch size so flush
-        flushPstmtHolder();
+    boolean old = transaction.isFlushOnQuery();
+    transaction.setFlushOnQuery(false);
+    // disable flush on query due transsaction callbacks
+    try {
+      for (int i = 0; i < list.size(); i++) {
+        if (i % batchSize == 0) {
+          // hit the batch size so flush
+          flushPstmtHolder();
+        }
+        list.get(i).executeNow();
       }
-      list.get(i).executeNow();
+      flushPstmtHolder();
+    } finally {
+      transaction.setFlushOnQuery(old);
     }
-    flushPstmtHolder();
   }
 
   public void flushOnCommit() throws BatchedSqlException {

ebean-test/src/test/java/org/tests/batchinsert/TestBatchInsertFlush.java

@@ -1,30 +1,62 @@
 package org.tests.batchinsert;
 
-import io.ebean.*;
-import io.ebean.xtest.BaseTestCase;
-import io.ebean.xtest.IgnorePlatform;
+import io.ebean.DB;
+import io.ebean.Database;
+import io.ebean.Transaction;
 import io.ebean.annotation.PersistBatch;
 import io.ebean.annotation.Platform;
 import io.ebean.annotation.Transactional;
 import io.ebean.meta.MetaTimedMetric;
 import io.ebean.meta.ServerMetrics;
 import io.ebean.test.LoggedSql;
+import io.ebean.xtest.BaseTestCase;
+import io.ebean.xtest.IgnorePlatform;
 import io.ebean.xtest.base.DtoQuery2Test;
 import io.ebeaninternal.api.SpiTransaction;
 import org.junit.jupiter.api.Test;
-import org.tests.model.basic.Customer;
-import org.tests.model.basic.EBasicVer;
-import org.tests.model.basic.TSDetail;
-import org.tests.model.basic.TSMaster;
+import org.tests.model.basic.*;
 
 import java.sql.Timestamp;
+import java.util.ArrayList;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class TestBatchInsertFlush extends BaseTestCase {
 
+  @Test
+  public void batchFlush() {
+
+    TSMaster m = new TSMaster();
+    m.setName("master1");
+    DB.save(m);
+    DB.getDefault().cacheManager().clearAll();
+
+    try (Transaction txn = DB.beginTransaction()) {
+      txn.setBatchSize(2);
+      txn.setBatchMode(true);
+
+      List<Customer> customers = new ArrayList<>();
+
+      for (int i = 0; i < 3; i++) {
+        Customer customer = ResetBasicData.createCustomer("BatchFlushPreInsert " + i, null, null, 3);
+        customer.addContact(new Contact("Fred" + i, "Blue"));
+        customers.add(customer);
+      }
+
+      for (int i = 3; i < 6; i++) {
+        Customer customer = ResetBasicData.createCustomer("BatchFlushPostInsert " + i, null, null, 3);
+        customer.addContact(new Contact("Fred" + i, "Blue"));
+        customers.add(customer);
+      }
+
+      DB.saveAll(customers);
+
+      txn.commit();
+    }
+  }
+
   @Test
   public void no_cascade() {
 
@@ -70,7 +102,7 @@ public void no_cascade() {
       // detail
       assertThat(sql.get(3)).contains("insert into t_detail_with_other_namexxxyy");
 
-      assertThat(((SpiTransaction)transaction).label()).isEqualTo("TestBatchInsertFlush.no_cascade");
+      assertThat(((SpiTransaction) transaction).label()).isEqualTo("TestBatchInsertFlush.no_cascade");
 
     } finally {
       transaction.end();
@@ -133,7 +165,7 @@ public void transactional_flushOnDtoQuery() {
 
     // trigger JDBC batch by default
     DB.findDto(DtoQuery2Test.DCust.class, "select id, name from o_customer")
-    .findList();
+      .findList();
 
     List<String> sql = LoggedSql.stop();
     assertSql(sql.get(0)).contains("insert into e_basicver");

ebean-test/src/test/java/org/tests/model/basic/event/CustomerPersistAdapter.java

@@ -1,8 +1,10 @@
 package org.tests.model.basic.event;
 
+import io.ebean.DB;
 import io.ebean.event.BeanPersistAdapter;
 import io.ebean.event.BeanPersistRequest;
 import org.tests.model.basic.Customer;
+import org.tests.model.basic.TSMaster;
 
 public class CustomerPersistAdapter extends BeanPersistAdapter {
 
@@ -16,7 +18,9 @@ public boolean preInsert(BeanPersistRequest<?> request) {
 
 //		StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
 //		request.getTransaction().log("+++++ "+Arrays.toString(stackTrace));
-
+    if (((Customer) request.bean()).getName().startsWith("BatchFlushPreInsert")) {
+      DB.find(TSMaster.class).where().eq("name", "master1").exists();
+    }
     return true;
   }
 
@@ -28,4 +32,11 @@ public boolean preUpdate(BeanPersistRequest<?> request) {
     return true;
   }
 
+  @Override
+  public void postInsert(BeanPersistRequest<?> request) {
+    super.postInsert(request);
+    if (((Customer) request.bean()).getName().startsWith("BatchFlushPostInsert")) {
+      DB.find(TSMaster.class).where().eq("name", "master1").exists();
+    }
+  }
 }