fix: #3142 Cascading a OneToMany, set the relationship when the child…

… isn't new or dirty

That is, previously the relationship back from the child to the parent was being updated when the child bean was new or dirty BUT NOT in the case when the child bean was loaded and unchanged.

This fix includes that case updating the ManyToOne side of the relationship on the child when the save is cascaded.

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

@@ -588,10 +588,9 @@ public TableJoin intersectionTableJoin() {
   }
 
   /**
-   * Set the join properties from the parent bean to the child bean.
-   * This is only valid for OneToMany and NOT valid for ManyToMany.
+   * Set the parent bean to the child (update the relationship).
    */
-  public void setJoinValuesToChild(EntityBean parent, EntityBean child, Object mapKeyValue) {
+  public void setParentToChild(EntityBean parent, EntityBean child, Object mapKeyValue) {
     if (mapKeyProperty != null) {
       mapKeyProperty.setValue(child, mapKeyValue);
     }
@@ -602,6 +601,31 @@ public void setJoinValuesToChild(EntityBean parent, EntityBean child, Object map
     }
   }
 
+  /**
+   * Return true if the parent bean has been set to the child (updated the relationship).
+   */
+  public boolean setParentToChild(EntityBean parent, EntityBean child, Object mapKeyValue, BeanDescriptor<?> parentDesc) {
+    if (manyToMany
+      || childMasterProperty == null
+      || !child._ebean_getIntercept().isLoadedProperty(childMasterProperty.propertyIndex())) {
+      return false;
+    }
+
+    Object currentParent = childMasterProperty.getValue(child);
+    if (currentParent != null) {
+      Object newId = parentDesc.getId(parent);
+      Object oldId = parentDesc.id(currentParent);
+      if (Objects.equals(newId, oldId)) {
+        return false;
+      }
+    }
+    childMasterProperty.setValueIntercept(child, parent);
+    if (mapKeyProperty != null) {
+      mapKeyProperty.setValue(child, mapKeyValue);
+    }
+    return true;
+  }
+
   /**
    * Return the order by clause used to order the fetching of the data for
    * this list, set or map.

ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java

@@ -172,7 +172,9 @@ private void saveAllBeans(final BeanProperty orderColumn) {
           } else if (ebi.isNewOrDirty()) {
             skipSavingThisBean = false;
             // set the parent bean to detailBean
-            many.setJoinValuesToChild(parentBean, detail, mapKeyValue);
+            many.setParentToChild(parentBean, detail, mapKeyValue);
+          } else if (many.setParentToChild(parentBean, detail, mapKeyValue, request.descriptor())) {
+            skipSavingThisBean = false;
           } else {
             skipSavingThisBean = saveRecurseSkippable;
           }

ebean-test/src/test/java/org/tests/saveassociation/TestSaveAssociation.java

@@ -6,12 +6,12 @@
 import org.tests.model.basic.TSDetail;
 import org.tests.model.basic.TSMaster;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
 
-public class TestSaveAssociation extends BaseTestCase {
+class TestSaveAssociation extends BaseTestCase {
 
   @Test
-  public void test() {
+  void test() {
 
     TSMaster m0 = new TSMaster();
     m0.setName("master1");
@@ -26,7 +26,57 @@ public void test() {
     TSMaster m0Check = DB.find(TSMaster.class).fetch("details").where().idEq(m0.getId())
       .findOne();
 
-    assertEquals(2, m0Check.getDetails().size());
+    assertThat(m0Check.getDetails()).hasSize(2);
+    DB.delete(m0);
   }
 
+
+  @Test
+  void testCascadeSetParent() {
+    // setup
+    TSDetail detail = new TSDetail("master1 detail1");
+    DB.save(detail);
+
+    // act
+    TSMaster m0 = new TSMaster();
+    m0.setName("master2");
+    m0.addDetail(detail);
+    DB.save(m0);
+
+    // assert
+    TSMaster check = DB.find(TSMaster.class).fetch("details")
+      .where().idEq(m0.getId())
+      .findOne();
+
+    assertThat(check.getDetails()).hasSize(1);
+    assertThat(check.getDetails().get(0).getId()).isEqualTo(detail.getId());
+    DB.delete(m0);
+  }
+
+  @Test
+  void testCascadeChangeParent() {
+    // setup
+    TSDetail detail = new TSDetail("master3 detail1");
+    TSMaster m0 = new TSMaster();
+    m0.setName("master3");
+    m0.addDetail(detail);
+    DB.save(m0);
+
+    // act
+    TSMaster m1 = new TSMaster();
+    m1.setName("master4");
+    m1.addDetail(detail);
+    DB.save(m1);
+
+    // assert
+    TSMaster check = DB.find(TSMaster.class).fetch("details")
+      .where().idEq(m1.getId())
+      .findOne();
+
+    assertThat(check.getDetails()).hasSize(1);
+    assertThat(check.getDetails().get(0).getId()).isEqualTo(detail.getId());
+
+    DB.delete(m1);
+    DB.delete(TSMaster.class, m0.getId());
+  }
 }