diff --git a/src/main/java/io/ebean/bean/EntityBeanIntercept.java b/src/main/java/io/ebean/bean/EntityBeanIntercept.java index 4a11be220..6ef1e64f0 100644 --- a/src/main/java/io/ebean/bean/EntityBeanIntercept.java +++ b/src/main/java/io/ebean/bean/EntityBeanIntercept.java @@ -40,6 +40,8 @@ public final class EntityBeanIntercept implements Serializable { private String ebeanServerName; + private boolean deletedFromCollection; + /** * The actual entity bean that 'owns' this intercept. */ @@ -1169,4 +1171,18 @@ public final class EntityBeanIntercept implements Serializable { } return ret; } + + /** + * Returns true if the entity was removed from a BeanCollection. This can be used to track movement from one collection to another. + */ + public boolean isDeletedFromCollection() { + return deletedFromCollection; + } + + /** + * Set if the entity was deleted from a BeanCollection. + */ + public void setDeletedFromCollection(final boolean deletedFromCollection) { + this.deletedFromCollection = deletedFromCollection; + } } diff --git a/src/main/java/io/ebean/common/ModifyHolder.java b/src/main/java/io/ebean/common/ModifyHolder.java index 247fb5cf2..6da14d3a5 100644 --- a/src/main/java/io/ebean/common/ModifyHolder.java +++ b/src/main/java/io/ebean/common/ModifyHolder.java @@ -1,5 +1,7 @@ package io.ebean.common; +import io.ebean.bean.EntityBean; + import java.io.Serializable; import java.util.Collection; import java.util.LinkedHashSet; @@ -54,6 +56,11 @@ class ModifyHolder implements Serializable { void modifyAddition(E bean) { if (bean != null) { touched = true; + + if (bean instanceof EntityBean) { + ((EntityBean) bean)._ebean_getIntercept().setDeletedFromCollection(false); + } + // If it is to delete then just remove the deletion if (!undoDeletion(bean)) { // Insert @@ -70,6 +77,11 @@ class ModifyHolder implements Serializable { void modifyRemoval(Object bean) { if (bean != null) { touched = true; + + if (bean instanceof EntityBean) { + ((EntityBean) bean)._ebean_getIntercept().setDeletedFromCollection(true); + } + // If it is to be added then just remove the addition if (!undoAddition(bean)) { modifyDeletions.add((E) bean); diff --git a/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java b/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java index 5f6a84ea3..138265d35 100644 --- a/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java +++ b/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java @@ -69,7 +69,7 @@ public class SaveManyBeans extends SaveManyBase { resetModifyState(); } } else { - if (isModifyListenMode()) { + if (isModifyListenMode() || many.hasOrderColumn()) { // delete any removed beans via private owned. Needs to occur before // a 'deleteMissingChildren' statement occurs removeAssocManyPrivateOwned(); @@ -348,14 +348,25 @@ public class SaveManyBeans extends SaveManyBase { BeanCollection c = (BeanCollection) value; Set modifyRemovals = c.getModifyRemovals(); - modifyListenReset(c); + + if (insertedParent) { + // after insert set the modify listening mode for private owned etc + c.setModifyListening(many.getModifyListenMode()); + } + + // We must not reset when we still have to update other entities in the collection and set their new orderColumn value + if (!many.hasOrderColumn()) { + c.modifyReset(); + } if (modifyRemovals != null && !modifyRemovals.isEmpty()) { for (Object removedBean : modifyRemovals) { if (removedBean instanceof EntityBean) { EntityBean eb = (EntityBean) removedBean; if (!eb._ebean_getIntercept().isNew()) { - // only delete if the bean was loaded meaning that it is known to exist in the DB - persister.deleteRequest(persister.createDeleteRemoved(removedBean, transaction, request.getFlags())); + if (eb._ebean_intercept().isDeletedFromCollection()) { + // only delete if the bean was loaded meaning that it is known to exist in the DB + persister.deleteRequest(persister.createDeleteRemoved(removedBean, transaction, request.getFlags())); + } } } } diff --git a/src/test/java/org/tests/model/version/TestVersionHierarchyModification.java b/src/test/java/org/tests/model/version/TestVersionHierarchyModification.java new file mode 100644 index 000000000..8c7608450 --- /dev/null +++ b/src/test/java/org/tests/model/version/TestVersionHierarchyModification.java @@ -0,0 +1,118 @@ +package org.tests.model.version; + +import io.ebean.BaseTestCase; +import io.ebean.Ebean; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestVersionHierarchyModification extends BaseTestCase { + + @Before + public void setup() { + final VersionParent parent = new VersionParent(); + parent.setName("vParent"); + + final VersionChild child1 = new VersionChild(); + child1.setName("vChild1"); + parent.getChildren().add(child1); + + final VersionToy toy11 = new VersionToy(); + toy11.setName("vToy1.1"); + child1.getToys().add(toy11); + + final VersionToy toy12 = new VersionToy(); + toy12.setName("vToy1.2"); + child1.getToys().add(toy12); + + final VersionChild child2 = new VersionChild(); + child2.setName("vChild2"); + parent.getChildren().add(child2); + + final VersionToy toy21 = new VersionToy(); + toy21.setName("vToy2.1"); + child2.getToys().add(toy21); + + final VersionToy toy22 = new VersionToy(); + toy22.setName("vToy2.2"); + child2.getToys().add(toy22); + + Ebean.save(parent); + } + + @After + public void cleanUp() { + Ebean.find(VersionParent.class).delete(); + } + + @Test + public void testMoveDown() { + VersionParent parent = Ebean.find(VersionParent.class).findOne(); + assertThat(parent).isNotNull(); + assertThat(parent.getChildren()).hasSize(2); + + VersionChild firstChild = parent.getChildren().get(0); + VersionChild secondChild = parent.getChildren().get(1); + + assertThat(firstChild.getToys()).hasSize(2); + assertThat(secondChild.getToys()).hasSize(2); + assertThat(firstChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy1.1", "vToy1.2"); + assertThat(secondChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy2.1", "vToy2.2"); + + final VersionToy toyToMove = firstChild.getToys().get(0); + firstChild.getToys().remove(toyToMove); + toyToMove.setChild(secondChild); + secondChild.getToys().add(1, toyToMove); + + Ebean.save(parent); + + parent = Ebean.find(VersionParent.class).findOne(); + assertThat(parent).isNotNull(); + assertThat(parent.getChildren()).hasSize(2); + + firstChild = parent.getChildren().get(0); + secondChild = parent.getChildren().get(1); + + assertThat(firstChild.getToys()).hasSize(1); + assertThat(secondChild.getToys()).hasSize(3); + assertThat(firstChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy1.2"); + assertThat(secondChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy2.1", "vToy1.1", "vToy2.2"); + } + + @Test + public void testMoveUp() { + VersionParent parent = Ebean.find(VersionParent.class).findOne(); + assertThat(parent).isNotNull(); + assertThat(parent.getChildren()).hasSize(2); + + VersionChild firstChild = parent.getChildren().get(0); + VersionChild secondChild = parent.getChildren().get(1); + + assertThat(firstChild.getToys()).hasSize(2); + assertThat(secondChild.getToys()).hasSize(2); + assertThat(firstChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy1.1", "vToy1.2"); + assertThat(secondChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy2.1", "vToy2.2"); + + final VersionToy toyToMove = secondChild.getToys().get(0); + secondChild.getToys().remove(toyToMove); + toyToMove.setChild(firstChild); + firstChild.getToys().add(1, toyToMove); + + Ebean.save(parent); + + parent = Ebean.find(VersionParent.class).findOne(); + assertThat(parent).isNotNull(); + assertThat(parent.getChildren()).hasSize(2); + + firstChild = parent.getChildren().get(0); + secondChild = parent.getChildren().get(1); + + assertThat(secondChild.getToys()).hasSize(1); + assertThat(firstChild.getToys()).hasSize(3); + assertThat(secondChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy2.2"); + assertThat(firstChild.getToys()).extracting(VersionToy::getName).containsExactly("vToy1.1", "vToy2.1", "vToy1.2"); + } + +} diff --git a/src/test/java/org/tests/model/version/VersionChild.java b/src/test/java/org/tests/model/version/VersionChild.java new file mode 100644 index 000000000..03e4725b1 --- /dev/null +++ b/src/test/java/org/tests/model/version/VersionChild.java @@ -0,0 +1,70 @@ +package org.tests.model.version; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.OrderColumn; +import javax.persistence.Version; +import java.util.ArrayList; +import java.util.List; + +@Entity +public class VersionChild { + + @Id + Integer id; + + String name; + + @Version + Integer version; + + @OneToMany(mappedBy = "child", cascade = CascadeType.ALL, orphanRemoval = true) + @OrderColumn(name = "position") + List toys = new ArrayList<>(); + + @ManyToOne + VersionParent parent; + + public Integer getId() { + return id; + } + + public void setId(final Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(final String name) { + this.name = name; + } + + public Integer getVersion() { + return version; + } + + public void setVersion(final Integer version) { + this.version = version; + } + + public List getToys() { + return toys; + } + + public void setToys(final List toys) { + this.toys = toys; + } + + public VersionParent getParent() { + return parent; + } + + public void setParent(final VersionParent parent) { + this.parent = parent; + } +} diff --git a/src/test/java/org/tests/model/version/VersionParent.java b/src/test/java/org/tests/model/version/VersionParent.java new file mode 100644 index 000000000..288992225 --- /dev/null +++ b/src/test/java/org/tests/model/version/VersionParent.java @@ -0,0 +1,58 @@ +package org.tests.model.version; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.OrderColumn; +import javax.persistence.Version; +import java.util.ArrayList; +import java.util.List; + +@Entity +public class VersionParent { + + @Id + Integer id; + + @Version + Integer version; + + String name; + + @OneToMany(mappedBy = "parent", cascade = CascadeType.ALL, orphanRemoval = true) + @OrderColumn(name = "position") + List children = new ArrayList<>(); + + public Integer getId() { + return id; + } + + public void setId(final Integer id) { + this.id = id; + } + + public Integer getVersion() { + return version; + } + + public void setVersion(final Integer version) { + this.version = version; + } + + public String getName() { + return name; + } + + public void setName(final String name) { + this.name = name; + } + + public List getChildren() { + return children; + } + + public void setChildren(final List children) { + this.children = children; + } +} diff --git a/src/test/java/org/tests/model/version/VersionToy.java b/src/test/java/org/tests/model/version/VersionToy.java new file mode 100644 index 000000000..b7491ecb3 --- /dev/null +++ b/src/test/java/org/tests/model/version/VersionToy.java @@ -0,0 +1,53 @@ +package org.tests.model.version; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Version; + +@Entity +public class VersionToy { + + @Id + Integer id; + + String name; + + @Version + Integer version; + + @ManyToOne + VersionChild child; + + public Integer getId() { + return id; + } + + public void setId(final Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(final String name) { + this.name = name; + } + + public Integer getVersion() { + return version; + } + + public void setVersion(final Integer version) { + this.version = version; + } + + public VersionChild getChild() { + return child; + } + + public void setChild(final VersionChild child) { + this.child = child; + } +} diff --git a/src/test/java/org/tests/order/TestOrderColumn.java b/src/test/java/org/tests/order/TestOrderColumn.java index 27bc69c35..a3722daad 100644 --- a/src/test/java/org/tests/order/TestOrderColumn.java +++ b/src/test/java/org/tests/order/TestOrderColumn.java @@ -107,4 +107,41 @@ public class TestOrderColumn extends TransactionalTestCase { assertThat(sql.get(2)).contains("bind(tt32"); } + @Test + public void testRemoveElement() { + final OrderMaster master = new OrderMaster(); + + for (int i = 0; i < 5; i++) { + final OrderReferencedChild child = new OrderReferencedChild("p" + i); + child.setChildName("c" + i); + + for (int j = 0; j < 3; j++) { + final OrderToy toy = new OrderToy("t" + i + j); + child.getToys().add(toy); + } + + master.getChildren().add(child); + } + + Ebean.save(master); + + final OrderMaster result = Ebean.find(OrderMaster.class).findOne(); + + final List children = result.getChildren(); + assertThat(children).hasSize(5); + + final OrderReferencedChild child = children.get(0); + + child.getToys().remove(1); + + LoggedSqlCollector.start(); + Ebean.save(result); + final List sql = LoggedSqlCollector.stop(); + + assertThat(sql).hasSize(4); + assertSql(sql.get(0)).contains("delete from order_toy where id=?"); + assertSql(sql.get(2)).contains("update order_toy set sort_order=? where id=?"); + assertSql(sql.get(3)).contains("bind(2,"); + } + }