FIX: OrderdColums did not work properly with cache

ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java

@@ -294,8 +294,13 @@ final class BeanDescriptorCacheHelp<T> {
 
     List<Object> 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;
   }

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))) {
+          int originalOrder = 0;
-            orderColumn.setValue(detail, sortOrder);
+          if (orderColumn != null) {
-            ebi.setDirty(true);
+            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()) {

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;
+  }
+}

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<OmCacheOrderedDetail> 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<OmCacheOrderedDetail> getDetails() {
+    return details;
+  }
+
+  public void setDetails(List<OmCacheOrderedDetail> details) {
+    this.details = details;
+  }
+
+  public Long getVersion() {
+    return version;
+  }
+
+  public void setVersion(Long version) {
+    this.version = version;
+  }
+}

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);
+
+  }
 }