Add failing testcases for wrong order in batch save and possible fix for one of them

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

@@ -29,8 +29,6 @@ import java.util.List;
  */
 public final class BatchControl {
 
-  private static final Object DUMMY = new Object();
-
   /**
    * Used to sort queue entries by depth.
    */
@@ -52,7 +50,7 @@ public final class BatchControl {
    * Set of beans in this batch. This is used to ensure that a single bean instance is not included
    * in the batch twice (two separate insert requests etc).
    */
-  private final IdentityHashMap<Object, Object> persistedBeans = new IdentityHashMap<>();
+  private final IdentityHashMap<Object, String> persistedBeans = new IdentityHashMap<>();
 
   /**
    * Helper to determine statement ordering based on depth (and type).
@@ -181,15 +179,22 @@ public final class BatchControl {
    * Add the request to the batch and return true if we should flush.
    */
   private boolean addToBatch(PersistRequestBean<?> request) {
-    Object alreadyInBatch = persistedBeans.put(request.entityBean(), DUMMY);
+    int depth = transaction.depth();
+    BeanDescriptor<?> desc = request.descriptor();
+    // batching by bean type AND depth
+    String key = desc.rootName() + ":" + depth;
+    
+    String alreadyInBatch = persistedBeans.put(request.entityBean(), key);
     if (alreadyInBatch != null) {
       // special case where the same bean instance has already been
       // added to the batch (doesn't really occur with non-batching
       // as the bean gets changed from dirty to loaded earlier)
-      return false;
+      BatchedBeanHolder beanHolder = getBeanHolder(request, alreadyInBatch);
+      int ordering = depthOrder.orderingFor(depth);
+      return beanHolder.getOrder() > ordering;
     }
 
-    BatchedBeanHolder beanHolder = getBeanHolder(request);
+    BatchedBeanHolder beanHolder = getBeanHolder(request, key);
     int bufferSize = beanHolder.append(request);
 
     bufferMax = Math.max(bufferMax, bufferSize);
@@ -342,14 +347,11 @@ public final class BatchControl {
    * Return an entry for the given type description. The type description is
    * typically the bean class name (or table name for MapBeans).
    */
-  private BatchedBeanHolder getBeanHolder(PersistRequestBean<?> request) {
+  private BatchedBeanHolder getBeanHolder(PersistRequestBean<?> request, String key) {
 
     int depth = transaction.depth();
     BeanDescriptor<?> desc = request.descriptor();
 
-    // batching by bean type AND depth
-    String key = desc.rootName() + ":" + depth;
-
     BatchedBeanHolder batchBeanHolder = beanHoldMap.get(key);
     if (batchBeanHolder == null) {
       int ordering = depthOrder.orderingFor(depth);

ebean-test/src/test/java/io/ebeaninternal/server/transaction/TransactionTest.java

@@ -0,0 +1,70 @@
+package io.ebeaninternal.server.transaction;
+
+import io.ebean.*;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.tests.model.basic.relates.*;
+
+public class TransactionTest extends BaseTestCase {
+  private Relation1 r1 = new Relation1("R1");
+  private Relation2 r2 = new Relation2("R2");
+  private Relation3 r3 = new Relation3("R3");
+
+  private Transaction txn;
+
+  @BeforeEach
+  void beginTransaction() {
+    txn = DB.beginTransaction();
+    txn.setBatchMode(true);
+  }
+
+  @AfterEach
+  void commitTransaction() {
+    txn.commit();
+    txn.close();
+  }
+
+  @Test
+  public void testMultiSave1() {
+    r2.setWithCascade(r3);
+    r1.setWithCascade(r2);
+    DB.save(r1);
+  }
+
+  @Test
+  public void testMultiSave2() {
+    r2.setWithCascade(r3);
+    r1.setWithCascade(r2);
+    DB.save(r3);
+    DB.save(r1);
+  }
+
+  @Test
+  public void testMultiSave3() {
+    r2.setWithCascade(r3);
+    r1.setWithCascade(r2);
+    DB.save(r3);
+    DB.save(r2);
+    DB.save(r1);
+  }
+
+  @Test
+  public void testMultiSave4() {
+    r2.setNoCascade(r3);
+    r1.setWithCascade(r2);
+    DB.save(r3);
+    // Workaround: txn.flush();
+    DB.save(r1);
+  }
+
+  @Test
+  public void testMultiSave5() {
+    r2.setNoCascade(r3);
+    r1.setNoCascade(r2);
+    DB.save(r3);
+    DB.save(r2);
+    DB.save(r1);
+  }
+}

ebean-test/src/test/java/org/tests/model/basic/relates/Relation1.java

@@ -0,0 +1,64 @@
+package org.tests.model.basic.relates;
+
+
+import java.util.UUID;
+
+import javax.persistence.*;
+
+import io.ebean.annotation.ChangeLog;
+
+/**
+ * Relation entity
+ */
+@Entity
+@ChangeLog
+public class Relation1 {
+
+  @Id
+  private UUID id = UUID.randomUUID();
+
+  private String name;
+
+  public Relation1(String name) {
+    this.name = name;
+  }
+
+  @ManyToOne
+  private Relation2 noCascade;
+
+  @ManyToOne(cascade = CascadeType.ALL)
+  private Relation2 withCascade;
+
+  public UUID getId() {
+    return id;
+  }
+
+  public void setId(UUID id) {
+    this.id = id;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public Relation2 getNoCascade() {
+    return noCascade;
+  }
+
+  public void setNoCascade(Relation2 noCascade) {
+    this.noCascade = noCascade;
+  }
+
+  public Relation2 getWithCascade() {
+    return withCascade;
+  }
+
+  public void setWithCascade(Relation2 withCascade) {
+    this.withCascade = withCascade;
+  }
+  
+}

ebean-test/src/test/java/org/tests/model/basic/relates/Relation2.java

@@ -0,0 +1,64 @@
+package org.tests.model.basic.relates;
+
+
+import java.util.UUID;
+
+import javax.persistence.*;
+
+import io.ebean.annotation.ChangeLog;
+
+/**
+ * Relation entity
+ */
+@Entity
+@ChangeLog
+public class Relation2 {
+
+  @Id
+  private UUID id = UUID.randomUUID();
+
+  private String name;
+
+  public Relation2(String name) {
+    this.name = name;
+  }
+
+  @ManyToOne
+  private Relation3 noCascade;
+
+  @ManyToOne(cascade = CascadeType.ALL)
+  private Relation3 withCascade;
+
+  public UUID getId() {
+    return id;
+  }
+
+  public void setId(UUID id) {
+    this.id = id;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public Relation3 getNoCascade() {
+    return noCascade;
+  }
+
+  public void setNoCascade(Relation3 noCascade) {
+    this.noCascade = noCascade;
+  }
+
+  public Relation3 getWithCascade() {
+    return withCascade;
+  }
+
+  public void setWithCascade(Relation3 withCascade) {
+    this.withCascade = withCascade;
+  }
+  
+}

ebean-test/src/test/java/org/tests/model/basic/relates/Relation3.java

@@ -0,0 +1,42 @@
+package org.tests.model.basic.relates;
+
+
+import java.util.UUID;
+
+import javax.persistence.*;
+
+import io.ebean.annotation.ChangeLog;
+
+/**
+ * Relation entity
+ */
+@Entity
+@ChangeLog
+public class Relation3 {
+
+  @Id
+  private UUID id = UUID.randomUUID();
+
+  private String name;
+
+  public Relation3(String name) {
+    this.name = name;
+  }
+
+  public UUID getId() {
+    return id;
+  }
+
+  public void setId(UUID id) {
+    this.id = id;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+}