Merge pull request #3310 from Ichtil/one_to_many_list_clear_test

Previous entities are not deleted when replacing OneToMany collection with a new one

ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java

@@ -380,6 +380,14 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     return intercept.dirtyPropertyNames();
   }
 
+  public boolean isChangedProperty(int propertyIndex) {
+    if (dirtyProperties == null) {
+      return intercept.isChangedProperty(propertyIndex);
+    } else {
+      return dirtyProperties[propertyIndex];
+    }
+  }
+
   /**
    * Return the dirty properties on this request.
    */
@@ -865,8 +873,8 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     }
     setNotifyCache();
     boolean isChangeLog = beanDescriptor.isChangeLog();
-    if (type == Type.UPDATE && (isChangeLog || notifyCache || docStoreMode == DocStoreMode.UPDATE)) {
-      // get the dirty properties for update notification to the doc store
+    if (type == Type.UPDATE) {
+      // get the dirty properties for notify cache & orphanRemoval of vanilla collection detection
       dirtyProperties = intercept.dirtyProperties();
     }
     if (isChangeLog) {

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

@@ -328,7 +328,7 @@ final class SaveManyBeans extends SaveManyBase {
   }
 
   private boolean isChangedProperty() {
-    return parentBean._ebean_getIntercept().isChangedProperty(many.propertyIndex());
+    return request.isChangedProperty(many.propertyIndex());
   }
 
   private void removeAssocManyOrphans() {

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<OmBeanListChild> children;
 
@@ -35,6 +46,42 @@ public class OmBeanListParent extends Model {
     this.children.clear();
     this.children.addAll(children);
   }
+
+  public void setChildren2(List<OmBeanListChild> 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;
+  }
 }
 
 

ebean-test/src/test/java/org/tests/o2m/OneToManyListMarkAsDirtyTest.java

@@ -0,0 +1,114 @@
+package org.tests.o2m;
+
+import io.ebean.DB;
+import io.ebean.xtest.BaseTestCase;
+import org.junit.jupiter.api.Test;
+import org.tests.model.orphanremoval.OmBeanListChild;
+import org.tests.model.orphanremoval.OmBeanListParent;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.LockSupport;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class OneToManyListMarkAsDirtyTest extends BaseTestCase {
+  @Test
+  void usingNewListWithNonDirtyParent_expect_orphanDeleted_works() {
+    // setup
+    var parent = new OmBeanListParent();
+    var a_b = new OmBeanListChild("b");
+    parent.getChildren().add(a_b);
+    DB.save(parent);
+    LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
+
+    // act
+    var secondParent = DB.find(OmBeanListParent.class, parent.getId());
+    secondParent.setChildren2(new ArrayList<>()); // <!-- HERE - setting a new ArrayList rather than using clear()
+    var a_c = new OmBeanListChild("b");
+    secondParent.getChildren().add(a_c);
+    // working here as secondParent itself is not dirty
+    DB.save(secondParent);
+
+    var refreshedParent = DB.find(OmBeanListParent.class, parent.getId());
+    assertThat(refreshedParent.getChildren().size()).isEqualTo(1);
+    assertThat(refreshedParent.getChildren().get(0).getId()).isEqualTo(a_c.getId());
+    // Q: is this expected? A: Rob Bygrave - Yes it is expected
+    assertThat(refreshedParent.getVersion()).isEqualTo(parent.getVersion());
+    assertThat(refreshedParent.getWhenModified()).isEqualTo(parent.getWhenModified());
+  }
+
+  @Test
+  void usingNewListWithParentDirty_expect_orphanDeleted_fails2() {
+    // setup
+    var parent = new OmBeanListParent();
+    var a_b = new OmBeanListChild("b");
+    parent.getChildren().add(a_b);
+    DB.save(parent);
+    LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
+
+    // act
+    var secondParent = DB.find(OmBeanListParent.class, parent.getId());
+    secondParent.setChildren2(new ArrayList<>()); // <!-- HERE: using new ArrayList with orphanRemoval
+    var a_c = new OmBeanListChild("c");
+    secondParent.getChildren().add(a_c);
+    // force version increase
+    DB.markAsDirty(secondParent);
+    DB.save(secondParent);
+
+    var refreshedParent = DB.find(OmBeanListParent.class, parent.getId());
+    assertThat(refreshedParent.getChildren().size()).isEqualTo(1);
+    assertThat(refreshedParent.getChildren().get(0).getId()).isEqualTo(a_c.getId());
+    assertThat(refreshedParent.getVersion()).isGreaterThan(parent.getVersion());
+    assertThat(refreshedParent.getWhenModified()).isAfter(parent.getWhenModified());
+  }
+
+  @Test
+  void usingClearWithParentDirty_expect_orphanDeleted_works() {
+    // setup
+    var parent = new OmBeanListParent();
+    var a_b = new OmBeanListChild("b");
+    parent.getChildren().add(a_b);
+    DB.save(parent);
+    LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
+
+    // act
+    var secondParent = DB.find(OmBeanListParent.class, parent.getId());
+    secondParent.getChildren().clear(); // <!-- HERE: Using clear() with orphanRemoval
+    var a_c = new OmBeanListChild("c");
+    secondParent.getChildren().add(a_c);
+    // force version increase
+    DB.markAsDirty(secondParent);
+    DB.save(secondParent);
+
+    var refreshedParent = DB.find(OmBeanListParent.class, parent.getId());
+    assertThat(refreshedParent.getChildren().size()).isEqualTo(1);
+    assertThat(refreshedParent.getChildren().get(0).getId()).isEqualTo(a_c.getId());
+    assertThat(refreshedParent.getVersion()).isGreaterThan(parent.getVersion());
+    assertThat(refreshedParent.getWhenModified()).isAfter(parent.getWhenModified());
+  }
+
+  @Test
+  void usingNewListWithParentDirty_expect_orphanDeleted_fails() {
+    // setup
+    var parent = new OmBeanListParent();
+    var a_b = new OmBeanListChild("b");
+    parent.getChildren().add(a_b);
+    DB.save(parent);
+    LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1));
+
+    // act
+    var secondParent = DB.find(OmBeanListParent.class, parent.getId());
+    secondParent.setChildren2(new ArrayList<>()); // <!-- HERE: new ArrayList with orphanRemoval
+    var a_c = new OmBeanListChild("c");
+    secondParent.getChildren().add(a_c);
+    secondParent.setWhenCreated(Instant.now());
+    DB.save(secondParent);
+
+    var refreshedParent = DB.find(OmBeanListParent.class, parent.getId());
+    assertThat(refreshedParent.getChildren().size()).isEqualTo(1);
+    assertThat(refreshedParent.getChildren().get(0).getId()).isEqualTo(a_c.getId());
+    assertThat(refreshedParent.getVersion()).isGreaterThan(parent.getVersion());
+    assertThat(refreshedParent.getWhenModified()).isAfter(parent.getWhenModified());
+  }
+}