diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java index 61d7366bc..7cdaf95df 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java @@ -294,8 +294,13 @@ final class BeanDescriptorCacheHelp { List idList = entry.getIdList(); bc.checkEmptyLazyLoad(); + int i = 0; for (Object id : idList) { - many.add(bc, targetDescriptor.createReference(readOnly, id, persistenceContext)); + final EntityBean ref = targetDescriptor.createReference(readOnly, id, persistenceContext); + if (many.hasOrderColumn()) { + ref._ebean_getIntercept().setSortOrder(++i); + } + many.add(bc, ref); } return true; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java index 33edf8dd8..f4a70f763 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java @@ -18,7 +18,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import static io.ebeaninternal.server.persist.DmlUtil.isNullOrZero; @@ -165,11 +164,15 @@ public final class SaveManyBeans extends SaveManyBase { if (many.hasJoinTable()) { skipSavingThisBean = targetDescriptor.isReference(ebi); } else { - if (orderColumn != null && !Objects.equals(sortOrder, orderColumn.getValue(detail))) { - orderColumn.setValue(detail, sortOrder); - ebi.setDirty(true); + int originalOrder = 0; + if (orderColumn != null) { + originalOrder = detail._ebean_getIntercept().getSortOrder(); + if (sortOrder != originalOrder) { + detail._ebean_intercept().setSortOrder(sortOrder); + ebi.setDirty(true); + } } - if (targetDescriptor.isReference(ebi)) { + if (targetDescriptor.isReference(ebi) && originalOrder == 0) { // we can skip this one skipSavingThisBean = true; } else if (ebi.isNewOrDirty()) { diff --git a/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedDetail.java b/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedDetail.java new file mode 100644 index 000000000..2febada00 --- /dev/null +++ b/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedDetail.java @@ -0,0 +1,60 @@ +package org.tests.cascade; + +import io.ebean.annotation.Cache; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Version; + +@Entity +@Cache +public class OmCacheOrderedDetail { + + @Id + Long id; + + String name; + + @ManyToOne + OmCacheOrderedMaster master; + + @Version + Long version; + + public OmCacheOrderedDetail(String name) { + this.name = name; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public OmCacheOrderedMaster getMaster() { + return master; + } + + public void setMaster(OmCacheOrderedMaster master) { + this.master = master; + } + + public Long getVersion() { + return version; + } + + public void setVersion(Long version) { + this.version = version; + } +} diff --git a/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedMaster.java b/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedMaster.java new file mode 100644 index 000000000..7e0dd4df3 --- /dev/null +++ b/ebean-core/src/test/java/org/tests/cascade/OmCacheOrderedMaster.java @@ -0,0 +1,66 @@ +package org.tests.cascade; + +import io.ebean.annotation.Cache; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.OrderColumn; +import javax.persistence.Version; +import java.util.List; + +@Entity +@Cache +public class OmCacheOrderedMaster { + + @Id + Long id; + + String name; + + /** + * Cascade ALL set automatically as we set order values when cascading. + */ + @OneToMany(mappedBy = "master") //, cascade = CascadeType.ALL) + @OrderColumn(name = "sort_order") + List details; + + @Version + Long version; + + public OmCacheOrderedMaster(String name) { + this.name = name; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getDetails() { + return details; + } + + public void setDetails(List details) { + this.details = details; + } + + public Long getVersion() { + return version; + } + + public void setVersion(Long version) { + this.version = version; + } +} diff --git a/ebean-core/src/test/java/org/tests/cascade/TestOrderedList.java b/ebean-core/src/test/java/org/tests/cascade/TestOrderedList.java index 0045a502b..a306cb105 100644 --- a/ebean-core/src/test/java/org/tests/cascade/TestOrderedList.java +++ b/ebean-core/src/test/java/org/tests/cascade/TestOrderedList.java @@ -5,6 +5,7 @@ import io.ebean.DB; import org.ebeantest.LoggedSqlCollector; import org.junit.Test; +import java.util.Collections; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -113,4 +114,78 @@ public class TestOrderedList extends BaseTestCase { final OmOrderedMaster masterDb = DB.find(OmOrderedMaster.class, master.getId()); assertThat(masterDb.getDetails()).hasSize(1); } + + @Test + public void testModifyList() { + final OmOrderedMaster master = new OmOrderedMaster("Master"); + final OmOrderedDetail detail1 = new OmOrderedDetail("Detail1"); + final OmOrderedDetail detail2 = new OmOrderedDetail("Detail2"); + final OmOrderedDetail detail3 = new OmOrderedDetail("Detail3"); + DB.save(detail1); + DB.save(detail2); + DB.save(detail3); + master.getDetails().add(detail1); + master.getDetails().add(detail2); + master.getDetails().add(detail3); + + DB.save(master); + + OmOrderedMaster masterDb = DB.find(OmOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail1, detail2, detail3); + + Collections.reverse(masterDb.getDetails()); + + DB.save(masterDb); + + masterDb = DB.find(OmOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail3, detail2, detail1); + + masterDb.getDetails().remove(1); + + DB.save(masterDb); + + masterDb = DB.find(OmOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail3, detail1); + + } + + @Test + public void testModifyListWithCache() { + final OmCacheOrderedMaster master = new OmCacheOrderedMaster("Master"); + final OmCacheOrderedDetail detail1 = new OmCacheOrderedDetail("Detail1"); + final OmCacheOrderedDetail detail2 = new OmCacheOrderedDetail("Detail2"); + final OmCacheOrderedDetail detail3 = new OmCacheOrderedDetail("Detail3"); + DB.save(detail1); + DB.save(detail2); + DB.save(detail3); + master.getDetails().add(detail1); + master.getDetails().add(detail2); + master.getDetails().add(detail3); + + DB.save(master); + + OmCacheOrderedMaster masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); // load cache + assertThat(masterDb.getDetails()).containsExactly(detail1, detail2, detail3); + + masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail1, detail2, detail3); // hit cache + + Collections.reverse(masterDb.getDetails()); + + DB.save(masterDb); + + masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail3, detail2, detail1); // load cache + + masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail3, detail2, detail1); // hit cache + + masterDb.getDetails().remove(1); + + DB.save(masterDb); + + masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); + assertThat(masterDb.getDetails()).containsExactly(detail3, detail1); + + } }