Bug? Wrong order in batch save

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

@@ -29,8 +29,6 @@
  */
 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 int executeOrQueue(PersistRequestBean<?> request, boolean batch) throws B
  * 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 @@ private boolean isBeanHoldersEmpty() {
  * 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;
+ }
+
+}