From e80e8e027a6f704e66a9aeb6b1b14998c958488a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kli=C4=8Dka?= Date: Thu, 18 Jan 2024 16:49:04 +0100 Subject: [PATCH 1/6] One to many replacing collection failing test cases --- .../o2m/OneToManyListMarkAsDirtyTest.java | 149 ++++++++++++++++++ .../java/org/tests/o2m/dm/GoodsCapacity.java | 41 +++++ .../java/org/tests/o2m/dm/StrategicPlan.java | 34 ++++ 3 files changed, 224 insertions(+) create mode 100644 ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java create mode 100644 ebean-test/src/test/java/org/tests/o2m/dm/GoodsCapacity.java create mode 100644 ebean-test/src/test/java/org/tests/o2m/dm/StrategicPlan.java diff --git a/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java new file mode 100644 index 000000000..e5210e2e8 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java @@ -0,0 +1,149 @@ +package org.tests.o2m; + +import io.ebean.DB; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.Test; +import org.tests.o2m.dm.*; + +import java.sql.Timestamp; +import java.util.ArrayList; + +import static org.assertj.core.api.Assertions.assertThat; + +public class OneToManyListMarkAsDirtyTest extends BaseTestCase { + @Test + public void workingTestCase() { + var g = new GoodsEntity(); + DB.save(g); + + var planStart = new StrategicPlan(); + planStart.setName("aaa"); + planStart.setGoodsCapacities(new ArrayList<>()); + DB.save(planStart); + + var planFirstCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planFirstCapacity.setGoodsCapacities(new ArrayList<>()); + var a_b = new GoodsCapacity(); + a_b.setGoods(g); + planFirstCapacity.getGoodsCapacities().add(a_b); + DB.save(planFirstCapacity); + + var planSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planSecondCapacity.setGoodsCapacities(new ArrayList<>()); + var a_c = new GoodsCapacity(); + a_c.setGoods(g); + planSecondCapacity.getGoodsCapacities().add(a_c); + // without this the goods capacity is saved correctly, version remains same + // (not sure if version should stay the same if OneToMany relationship is changed (through cascade) + // DB.markAsDirty(planSecondCapacity); + DB.save(planSecondCapacity); + + var refreshedPlanSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().size()).isEqualTo(1); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().get(0).getId()).isEqualTo(a_c.getId()); + assertThat(refreshedPlanSecondCapacity.getVersion()).isEqualTo(planFirstCapacity.getVersion()); + assertThat(refreshedPlanSecondCapacity.getWhenModified()).isEqualTo(planFirstCapacity.getWhenModified()); + } + + @Test + public void oneToManyShouldBeDeletedWithParentMarkAsDirtyTest() { + var g = new GoodsEntity(); + DB.save(g); + + var planStart = new StrategicPlan(); + planStart.setName("aaa"); + planStart.setGoodsCapacities(new ArrayList<>()); + DB.save(planStart); + + var planFirstCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planFirstCapacity.setGoodsCapacities(new ArrayList<>()); + var a_b = new GoodsCapacity(); + a_b.setGoods(g); + planFirstCapacity.getGoodsCapacities().add(a_b); + DB.save(planFirstCapacity); + + var planSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planSecondCapacity.setGoodsCapacities(new ArrayList<>()); + var a_c = new GoodsCapacity(); + a_c.setGoods(g); + planSecondCapacity.getGoodsCapacities().add(a_c); + // force version increase + // without this the goods capacity is saved correctly, but version isn't + DB.markAsDirty(planSecondCapacity); + DB.save(planSecondCapacity); + + var refreshedPlanSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().size()).isEqualTo(1); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().get(0).getId()).isEqualTo(a_c.getId()); + assertThat(refreshedPlanSecondCapacity.getVersion()).isGreaterThan(planFirstCapacity.getVersion()); + assertThat(refreshedPlanSecondCapacity.getWhenModified()).isAfter(planFirstCapacity.getWhenModified()); + } + + @Test + public void oneToManyShouldBeDeletedWithParentMarkAsDirtyTestWorkAround() { + var g = new GoodsEntity(); + DB.save(g); + + var planStart = new StrategicPlan(); + planStart.setName("aaa"); + planStart.setGoodsCapacities(new ArrayList<>()); + DB.save(planStart); + + var planFirstCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planFirstCapacity.setGoodsCapacities(new ArrayList<>()); + var a_b = new GoodsCapacity(); + a_b.setGoods(g); + planFirstCapacity.getGoodsCapacities().add(a_b); + DB.save(planFirstCapacity); + + var planSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); +// planSecondCapacity.setGoodsCapacities(new ArrayList<>()); + planSecondCapacity.getGoodsCapacities().clear(); + var a_c = new GoodsCapacity(); + a_c.setGoods(g); + planSecondCapacity.getGoodsCapacities().add(a_c); + // force version increase + // without this the goods capacity is saved correctly, but version isn't + DB.markAsDirty(planSecondCapacity); + DB.save(planSecondCapacity); + + var refreshedPlanSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().size()).isEqualTo(1); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().get(0).getId()).isEqualTo(a_c.getId()); + assertThat(refreshedPlanSecondCapacity.getVersion()).isGreaterThan(planFirstCapacity.getVersion()); + assertThat(refreshedPlanSecondCapacity.getWhenModified()).isAfter(planFirstCapacity.getWhenModified()); + } + + @Test + public void oneToManyShouldBeDeletedWithParentManualModifiedWhenTest() { + var g = new GoodsEntity(); + DB.save(g); + + var planStart = new StrategicPlan(); + planStart.setName("aaa"); + planStart.setGoodsCapacities(new ArrayList<>()); + DB.save(planStart); + + var planFirstCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planFirstCapacity.setGoodsCapacities(new ArrayList<>()); + var a_b = new GoodsCapacity(); + a_b.setGoods(g); + planFirstCapacity.getGoodsCapacities().add(a_b); + DB.save(planFirstCapacity); + + var planSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + planSecondCapacity.setGoodsCapacities(new ArrayList<>()); + var a_c = new GoodsCapacity(); + a_c.setGoods(g); + planSecondCapacity.getGoodsCapacities().add(a_c); + planSecondCapacity.setWhenCreated(new Timestamp(System.currentTimeMillis())); + + DB.save(planSecondCapacity); + + var refreshedPlanSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().size()).isEqualTo(1); + assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().get(0).getId()).isEqualTo(a_c.getId()); + assertThat(refreshedPlanSecondCapacity.getVersion()).isGreaterThan(planFirstCapacity.getVersion()); + assertThat(refreshedPlanSecondCapacity.getWhenModified()).isAfter(planFirstCapacity.getWhenModified()); + } +} diff --git a/ebean-test/src/test/java/org/tests/o2m/dm/GoodsCapacity.java b/ebean-test/src/test/java/org/tests/o2m/dm/GoodsCapacity.java new file mode 100644 index 000000000..697804f01 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/dm/GoodsCapacity.java @@ -0,0 +1,41 @@ +package org.tests.o2m.dm; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; + +@Entity +//@Table(name = "strategic_plan_goods_capacities") +public class GoodsCapacity { + @Id + private Long id; + + @ManyToOne + private StrategicPlan strategicPlan; + @ManyToOne + private GoodsEntity goods; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public StrategicPlan getStrategicPlan() { + return strategicPlan; + } + + public void setStrategicPlan(StrategicPlan strategicPlan) { + this.strategicPlan = strategicPlan; + } + + public GoodsEntity getGoods() { + return goods; + } + + public void setGoods(GoodsEntity goods) { + this.goods = goods; + } +} diff --git a/ebean-test/src/test/java/org/tests/o2m/dm/StrategicPlan.java b/ebean-test/src/test/java/org/tests/o2m/dm/StrategicPlan.java new file mode 100644 index 000000000..c55e8d810 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/dm/StrategicPlan.java @@ -0,0 +1,34 @@ +package org.tests.o2m.dm; + + +import io.ebean.annotation.DbJsonB; +import jakarta.persistence.*; + +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; + +@Entity +//@Table(name = "strategic_plans") +public class StrategicPlan extends HistoryColumns { + private String name; + + @OneToMany(mappedBy = "strategicPlan", cascade = CascadeType.ALL, orphanRemoval = true) + private List goodsCapacities = new ArrayList<>(); + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getGoodsCapacities() { + return goodsCapacities; + } + + public void setGoodsCapacities(List goodsCapacities) { + this.goodsCapacities = goodsCapacities; + } +} From 006e33cb3e60e41f519a515945172352d9f73b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kli=C4=8Dka?= Date: Thu, 18 Jan 2024 17:29:24 +0100 Subject: [PATCH 2/6] One to many replacing collection failing test cases --- .../test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java index e5210e2e8..aa35a1036 100644 --- a/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java +++ b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java @@ -41,6 +41,7 @@ public class OneToManyListMarkAsDirtyTest extends BaseTestCase { var refreshedPlanSecondCapacity = DB.find(StrategicPlan.class, planStart.getId()); assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().size()).isEqualTo(1); assertThat(refreshedPlanSecondCapacity.getGoodsCapacities().get(0).getId()).isEqualTo(a_c.getId()); + // todo is this expected? assertThat(refreshedPlanSecondCapacity.getVersion()).isEqualTo(planFirstCapacity.getVersion()); assertThat(refreshedPlanSecondCapacity.getWhenModified()).isEqualTo(planFirstCapacity.getWhenModified()); } @@ -103,7 +104,6 @@ public class OneToManyListMarkAsDirtyTest extends BaseTestCase { a_c.setGoods(g); planSecondCapacity.getGoodsCapacities().add(a_c); // force version increase - // without this the goods capacity is saved correctly, but version isn't DB.markAsDirty(planSecondCapacity); DB.save(planSecondCapacity); From 04ba81d12f7294f7d15f22c2da0e452639843800 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Fri, 9 Feb 2024 08:30:42 +1300 Subject: [PATCH 3/6] #3310 Update test only, reuse existing entity beans - Reuse the existing OmBeanListParent and child - Simplify the test setup code - Rename test methods to maybe better reflect what I think is failing Noting that markAsDirty doesn't specifically have anything to do with this bug but it's more on whether the parent bean is dirty or dirty (markAsDirty is just a way to make the parent bean dirty). --- .../model/orphanremoval/OmBeanListParent.java | 47 +++++ .../o2m/OneToManyListMarkAsDirtyTest.java | 195 +++++++----------- .../java/org/tests/o2m/dm/GoodsCapacity.java | 41 ---- .../java/org/tests/o2m/dm/StrategicPlan.java | 34 --- 4 files changed, 124 insertions(+), 193 deletions(-) delete mode 100644 ebean-test/src/test/java/org/tests/o2m/dm/GoodsCapacity.java delete mode 100644 ebean-test/src/test/java/org/tests/o2m/dm/StrategicPlan.java diff --git a/ebean-test/src/test/java/org/tests/model/orphanremoval/OmBeanListParent.java b/ebean-test/src/test/java/org/tests/model/orphanremoval/OmBeanListParent.java index 1aacb79d6..026199f70 100644 --- a/ebean-test/src/test/java/org/tests/model/orphanremoval/OmBeanListParent.java +++ b/ebean-test/src/test/java/org/tests/model/orphanremoval/OmBeanListParent.java @@ -2,10 +2,14 @@ package org.tests.model.orphanremoval; import io.ebean.Model; +import io.ebean.annotation.WhenCreated; +import io.ebean.annotation.WhenModified; import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.OneToMany; import jakarta.persistence.Version; + +import java.time.Instant; import java.util.List; import static jakarta.persistence.CascadeType.ALL; @@ -19,6 +23,13 @@ public class OmBeanListParent extends Model { @Version private long version; + private String name; + + @WhenCreated + private Instant whenCreated; + @WhenModified + private Instant whenModified; + @OneToMany(cascade = ALL, mappedBy = "parent", orphanRemoval = true) private List children; @@ -35,6 +46,42 @@ public class OmBeanListParent extends Model { this.children.clear(); this.children.addAll(children); } + + public void setChildren2(List children) { + this.children = children; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } + + public Instant getWhenModified() { + return whenModified; + } + + public void setWhenModified(Instant whenModified) { + this.whenModified = whenModified; + } + + public Instant getWhenCreated() { + return whenCreated; + } + + public void setWhenCreated(Instant whenCreated) { + this.whenCreated = whenCreated; + } } diff --git a/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java index aa35a1036..ed618e821 100644 --- a/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java +++ b/ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java @@ -3,147 +3,106 @@ package org.tests.o2m; import io.ebean.DB; import io.ebean.xtest.BaseTestCase; import org.junit.jupiter.api.Test; -import org.tests.o2m.dm.*; - -import java.sql.Timestamp; +import org.tests.model.orphanremoval.OmBeanListChild; +import org.tests.model.orphanremoval.OmBeanListParent; +import java.time.Instant; import java.util.ArrayList; import static org.assertj.core.api.Assertions.assertThat; -public class OneToManyListMarkAsDirtyTest extends BaseTestCase { +class OneToManyListMarkAsDirtyTest extends BaseTestCase { @Test - public void workingTestCase() { - var g = new GoodsEntity(); - DB.save(g); + void usingNewListWithNonDirtyParent_expect_orphanDeleted_works() { + // setup + var parent = new OmBeanListParent(); + var a_b = new OmBeanListChild("b"); + parent.getChildren().add(a_b); + DB.save(parent); - var planStart = new StrategicPlan(); - planStart.setName("aaa"); - planStart.setGoodsCapacities(new ArrayList<>()); - DB.save(planStart); + // act + var secondParent = DB.find(OmBeanListParent.class, parent.getId()); + secondParent.setChildren2(new ArrayList<>()); //