Added some tests

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

@@ -318,7 +318,7 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> implements ST
   /**
    * Find the Id's of detail beans given a parent Id and optionally exclude detail IDs
    */
-  public List<Object> findIdsByParentId(Object parentId, Transaction t, boolean hard, List<Object> excludeDetailIds) {
+  public List<Object> findIdsByParentId(Object parentId, Transaction t, boolean hard, Set<Object> excludeDetailIds) {
     return sqlHelp.findIdsByParentId(parentId, t, hard, excludeDetailIds);
   }
 
@@ -755,7 +755,7 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> implements ST
     throw new PersistenceException(from + ": Could not find mapKey property " + mapKey + " on " + to);
   }
 
-  public IntersectionRow buildManyDeleteChildren(EntityBean parentBean, List<Object> excludeDetailIds) {
+  public IntersectionRow buildManyDeleteChildren(EntityBean parentBean, Set<Object> excludeDetailIds) {
     IntersectionRow row = new IntersectionRow(tableJoin.getTable(), targetDescriptor);
     if (excludeDetailIds != null && !excludeDetailIds.isEmpty()) {
       row.setExcludeIds(excludeDetailIds, targetDescriptor());

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

@@ -10,6 +10,7 @@ import io.ebeaninternal.server.deploy.visitor.VisitProperties;
 import io.ebeaninternal.server.util.Str;
 
 import java.util.List;
+import java.util.Set;
 
 class BeanPropertyAssocManySqlHelp<T> {
 
@@ -123,14 +124,23 @@ class BeanPropertyAssocManySqlHelp<T> {
     many.bindParentIdsIn(rawWhere, parentIds, query);
   }
 
-  List<Object> findIdsByParentId(Object parentId, Transaction t, boolean hard, List<Object> excludeDetailIds) {
+  List<Object> findIdsByParentId(Object parentId, Transaction t, boolean hard, Set<Object> excludeDetailIds) {
     final SpiEbeanServer server = descriptor.ebeanServer();
     final SpiQuery<?> query = many.newQuery(server);
     many.bindParentIdEq(rawParentIdEQ(""), parentId, query);
     if (hard) {
       query.setIncludeSoftDeletes();
     }
+
     if (excludeDetailIds != null && !excludeDetailIds.isEmpty()) {
+      if (excludeDetailIds.size() > 1000) { // TODO: Wait for #3176
+        // if we hit the parameter limit, we must filter that on the java side.
+        // There is no easy way to batch "not in" queries.
+        // checkme: We could pass the first 1000-2000 params to the DB and filter the rest
+        List<Object> ret = server.findIds(query, t);
+        ret.removeIf(id -> excludeDetailIds.contains(id));
+        return ret;
+      }
       query.where().not(query.getExpressionFactory().idIn(excludeDetailIds));
     }
     return server.findIds(query, t);

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

@@ -11,13 +11,14 @@ import io.ebeaninternal.server.persist.DeleteMode;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 public final class IntersectionRow {
 
   private final String tableName;
   private final BeanDescriptor<?> targetDescriptor;
   private final LinkedHashMap<String, Object> values = new LinkedHashMap<>();
-  private List<Object> excludeIds;
+  private Set<Object> excludeIds;
   private BeanDescriptor<?> excludeDescriptor;
 
   IntersectionRow(String tableName, BeanDescriptor<?> targetDescriptor) {
@@ -33,7 +34,7 @@ public final class IntersectionRow {
   /**
    * Set Id's to exclude. This is for deleting non-attached detail Id's.
    */
-  void setExcludeIds(List<Object> excludeIds, BeanDescriptor<?> excludeDescriptor) {
+  void setExcludeIds(Set<Object> excludeIds, BeanDescriptor<?> excludeDescriptor) {
     this.excludeIds = excludeIds;
     this.excludeDescriptor = excludeDescriptor;
   }

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

@@ -1082,12 +1082,13 @@ public final class DefaultPersister implements Persister {
    * </p>
    */
   void deleteManyDetails(SpiTransaction t, BeanDescriptor<?> desc, EntityBean parentBean,
-                         BeanPropertyAssocMany<?> many, List<Object> excludeDetailIds, DeleteMode deleteMode) {
+                         BeanPropertyAssocMany<?> many, Set<Object> excludeDetailIds, DeleteMode deleteMode) {
     if (many.cascadeInfo().isDelete()) {
       // cascade delete the beans in the collection
       BeanDescriptor<?> targetDesc = many.targetDescriptor();
       if (deleteMode.isHard() || targetDesc.isSoftDelete()) {
-        if (targetDesc.isDeleteByStatement()) {
+        if (targetDesc.isDeleteByStatement()
+          && (excludeDetailIds == null || excludeDetailIds.size() <= 1000)) { // TODO wait for #3176
           // Just delete all the children with one statement
           IntersectionRow intRow = many.buildManyDeleteChildren(parentBean, excludeDetailIds);
           SqlUpdate sqlDelete = intRow.createDelete(server, deleteMode);

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

@@ -9,7 +9,10 @@ import io.ebeaninternal.server.core.PersistRequestBean;
 import io.ebeaninternal.server.deploy.*;
 
 import javax.persistence.PersistenceException;
-import java.util.*;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
 import static io.ebeaninternal.server.persist.DmlUtil.isNullOrZero;
 import static java.lang.System.Logger.Level.WARNING;
@@ -205,9 +208,10 @@ final class SaveManyBeans extends SaveManyBase {
 
   /**
    * Return the Id values of beans we know are being updated (any others are orphans)
+   * If there are no IDs, null is returned.
    */
-  private List<Object> detailIds() {
-    final var detailIds = new ArrayList<>();
+  private Set<Object> detailIds() {
+    final var detailIds = new HashSet<>();
     for (Object detailBean : collection) {
       if (isMap) {
         detailBean = ((Map.Entry<?, ?>) detailBean).getValue();
@@ -222,7 +226,7 @@ final class SaveManyBeans extends SaveManyBase {
         }
       }
     }
-    return detailIds;
+    return detailIds.isEmpty() ? null : detailIds;
   }
 
   /**

ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java

@@ -7,56 +7,141 @@ import org.junit.jupiter.api.Test;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 
 class TestOrphanCollectionReplacement extends BaseTestCase {
 
   @Test
-  void replaceCollection_whenOrphan_expect_forcedInsert() {
-    long parentId;
-    { // setup
-      List<COOneMany> children = new ArrayList<>();
-      children.add(new COOneMany("c0"));
-      children.add(new COOneMany("c1"));
-
-      COOne parent = new COOne("p0");
-      parent.setChildren(children);
-
-      DB.save(parent);
-      parentId = parent.getId();
+  void replaceCollection_whenOrphan_expect_forcedInsertWithStatement() {
+    long parentId = setup(1000); // can be handled by statement for SqlServer
+    List<String> sql = doUpdate(parentId, name -> !"c0".equals(name));
+    if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+      assertThat(sql).hasSize(4);
+      assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
+      assertThat(sql.get(1)).contains(" -- bind(");
+      assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+      assertThat(sql.get(3)).contains(" -- bind(");
     }
 
-    { // act
-      COOne fetchedParent = DB.find(COOne.class, parentId);
-      assert fetchedParent != null;
+    if (isSqlServer()) {
+      // statement mode
+      assertThat(sql).hasSize(4);
+      assertThat(sql.get(0)).contains("update coone_many set deleted=1 where coone_id = ? and not ( id ");
+      assertThat(sql.get(1)).contains(" -- bind(");
+      assertThat(sql.get(2)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+      assertThat(sql.get(3)).contains(" -- bind(");
+    }
+    COOne fetchedUser2 = DB.find(COOne.class, parentId);
+    requireNonNull(fetchedUser2);
+    assertThat(fetchedUser2.getChildren())
+      .hasSize(1000)
+      .extracting(COOneMany::getName)
+      .doesNotContain("c0")// filtered
+      .contains("c1")
+      .contains("cTest"); // added
+  }
 
-      COOneMany role = new COOneMany("c2");
+  @Test
+  void replaceCollection_whenOrphan_expect_forcedInsertWithFilter() {
+    long parentId = setup(2500); // we cannot make a "not in" query for so many params
+    List<String> sql = doUpdate(parentId, name -> !"c0".equals(name));
+    if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+      assertThat(sql).hasSize(4);
+      assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
+      assertThat(sql.get(1)).contains(" -- bind(");
+      assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+      assertThat(sql.get(3)).contains(" -- bind(");
+    }
 
-      List<COOneMany> filtered = fetchedParent.getChildren().stream().filter(r -> "c0".equals(r.getName())).collect(Collectors.toList());
+    if (isSqlServer()) {
+      // filter mode
+      assertThat(sql).hasSize(5);
+      assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = 0 and t0.deleted = 0; --bind");
+      assertThat(sql.get(1)).contains("update coone_many set deleted=1 where id  in (?)");
+      assertThat(sql.get(2)).contains(" -- bind(");
+      assertThat(sql.get(3)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+      assertThat(sql.get(4)).contains(" -- bind(");
+    }
+    COOne fetchedUser2 = DB.find(COOne.class, parentId);
+    requireNonNull(fetchedUser2);
+    assertThat(fetchedUser2.getChildren())
+      .hasSize(2500)
+      .extracting(COOneMany::getName)
+      .doesNotContain("c0")// filtered
+      .contains("c1")
+      .contains("cTest"); // added
+  }
 
-      List<COOneMany> updatedRoles = new ArrayList<>();
-      updatedRoles.addAll(filtered);
-      updatedRoles.addAll(List.of(role));
-      fetchedParent.setChildren(updatedRoles);
+  @Test
+  void replaceCollection_whenOrphan_expect_forcedInsertWithManyReplacement() {
+    long parentId = setup(5000); //  we will replace 2500 beans in this step
+    List<String> sql = doUpdate(parentId, name -> Integer.parseInt(name.substring(1)) >= 2500);
+    if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+      assertThat(sql).hasSize(4);
+      assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
+      assertThat(sql.get(1)).contains(" -- bind(");
+      assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+      assertThat(sql.get(3)).contains(" -- bind(");
+    }
 
-      LoggedSql.start();
-      DB.save(fetchedParent);
-      var sql = LoggedSql.stop();
-      if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
-        assertThat(sql).hasSize(4);
-        assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
-        assertThat(sql.get(1)).contains(" -- bind(");
-        assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
-        assertThat(sql.get(3)).contains(" -- bind(");
-      }
+    if (isSqlServer()) {
+      // filter mode
+      assertThat(sql).hasSize(7);
+      assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = 0 and t0.deleted = 0; --bind"); // find all Ids
+      assertThat(sql.get(1)).contains("update coone_many set deleted=1 where id  in (?,?,?");
+      assertThat(sql.get(2)).contains(" -- bind(Array[2000]="); // update first 2000
+      assertThat(sql.get(3)).contains("update coone_many set deleted=1 where id  in (?,?,?");
+      assertThat(sql.get(4)).contains(" -- bind(Array[500]="); // update next 500
+      assertThat(sql.get(5)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+      assertThat(sql.get(6)).contains(" -- bind(");
     }
 
     COOne fetchedUser2 = DB.find(COOne.class, parentId);
     requireNonNull(fetchedUser2);
-    assertEquals(2, fetchedUser2.getChildren().size());
+    assertThat(fetchedUser2.getChildren())
+      .hasSize(2501)
+      .extracting(COOneMany::getName)
+      .doesNotContain("c0")// filtered
+      .contains("c2500")
+      .contains("cTest"); // added
+  }
+
+
+  private static List<String> doUpdate(long parentId, Predicate<String> filter) {
+    COOne fetchedParent = DB.find(COOne.class, parentId);
+    assert fetchedParent != null;
+
+
+    List<COOneMany> filtered = fetchedParent.getChildren().stream().filter(r -> filter.test(r.getName())).collect(Collectors.toList());
+
+    List<COOneMany> updatedRoles = new ArrayList<>();
+    updatedRoles.addAll(filtered);
+    updatedRoles.add(new COOneMany("cTest"));
+    fetchedParent.setChildren(updatedRoles);
+
+    LoggedSql.start();
+    DB.save(fetchedParent);
+    return LoggedSql.stop();
+  }
+
+  private static long setup(int count) {
+    long parentId;
+    // setup
+    List<COOneMany> children = new ArrayList<>();
+    for (int i = 0; i < count; i++) {
+      children.add(new COOneMany("c" + i));
+
+    }
+
+    COOne parent = new COOne("p0");
+    parent.setChildren(children);
+
+    DB.save(parent);
+    parentId = parent.getId();
+    return parentId;
   }
 }