From bf62efd220d0bd554db2a68e678d5aad816cd861 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 16 Sep 2021 16:52:20 +0200 Subject: [PATCH] Add failing testcases for wrong order in batch save and possible fix for one of them --- .../server/persist/BatchControl.java | 22 +++--- .../server/transaction/TransactionTest.java | 70 +++++++++++++++++++ .../tests/model/basic/relates/Relation1.java | 64 +++++++++++++++++ .../tests/model/basic/relates/Relation2.java | 64 +++++++++++++++++ .../tests/model/basic/relates/Relation3.java | 42 +++++++++++ 5 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 ebean-test/src/test/java/io/ebeaninternal/server/transaction/TransactionTest.java create mode 100644 ebean-test/src/test/java/org/tests/model/basic/relates/Relation1.java create mode 100644 ebean-test/src/test/java/org/tests/model/basic/relates/Relation2.java create mode 100644 ebean-test/src/test/java/org/tests/model/basic/relates/Relation3.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 c2a2c834c..4e13035ee 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 @@ -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 persistedBeans = new IdentityHashMap<>(); + private final IdentityHashMap 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); diff --git a/ebean-test/src/test/java/io/ebeaninternal/server/transaction/TransactionTest.java b/ebean-test/src/test/java/io/ebeaninternal/server/transaction/TransactionTest.java new file mode 100644 index 000000000..374e4519e --- /dev/null +++ b/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); + } +} \ No newline at end of file diff --git a/ebean-test/src/test/java/org/tests/model/basic/relates/Relation1.java b/ebean-test/src/test/java/org/tests/model/basic/relates/Relation1.java new file mode 100644 index 000000000..ba3d3cbf2 --- /dev/null +++ b/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; + } + +} diff --git a/ebean-test/src/test/java/org/tests/model/basic/relates/Relation2.java b/ebean-test/src/test/java/org/tests/model/basic/relates/Relation2.java new file mode 100644 index 000000000..2af3bc4cb --- /dev/null +++ b/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; + } + +} diff --git a/ebean-test/src/test/java/org/tests/model/basic/relates/Relation3.java b/ebean-test/src/test/java/org/tests/model/basic/relates/Relation3.java new file mode 100644 index 000000000..826e3564e --- /dev/null +++ b/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; + } + +}