#1844 - Persisting an update of ElementCollection with JDBC batch, ensure delete, insert batch order

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

@@ -27,7 +27,7 @@ import io.ebeaninternal.server.persist.BatchedSqlException;
 import io.ebeaninternal.server.persist.DeleteMode;
 import io.ebeaninternal.server.persist.Flags;
 import io.ebeaninternal.server.persist.PersistExecute;
-import io.ebeaninternal.server.persist.SaveManyBeans;
+import io.ebeaninternal.server.persist.SaveMany;
 import io.ebeaninternal.server.transaction.BeanPersistIdMap;
 import io.ebeanservice.docstore.api.DocStoreUpdate;
 import io.ebeanservice.docstore.api.DocStoreUpdateContext;
@@ -169,10 +169,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
    */
   private boolean getterCallback;
 
-  /**
-   * postUpdate notifications. Used to combine bean and element update updates into single postUpdate event.
-   */
-  private int pendingPostUpdateNotify;
+  private boolean pendingPostUpdateNotify;
 
   /**
    * Set to true when post execute has occurred (so includes batch flush).
@@ -187,7 +184,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
   /**
    * Many to many intersection table changes that are held for later batch processing.
    */
-  private List<SaveManyBeans> saveManyIntersections;
+  private List<SaveMany> saveMany;
 
   public PersistRequestBean(SpiEbeanServer server, T bean, Object parentBean, BeanManager<T> mgr, SpiTransaction t,
                             PersistExecute persistExecute, PersistRequest.Type type, int flags) {
@@ -889,12 +886,8 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
   }
 
   private void postUpdateNotify() {
-    if (pendingPostUpdateNotify > 0) {
-      // invoke the delayed postUpdate notification (combined with element collection update)
+    if (pendingPostUpdateNotify) {
       controller.postUpdate(this);
-    } else {
-      // batched update with no element collection, send postUpdate notification once it executes
-      pendingPostUpdateNotify = -1;
     }
   }
 
@@ -917,6 +910,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
    */
   @Override
   public void postExecute() {
+    saveQueuedMany();
     postExecute = true;
     if (controller != null) {
       controllerPost();
@@ -944,12 +938,11 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     if (isLogSummary()) {
       logSummary();
     }
-    saveQueuedManyIntersection();
   }
 
-  private void saveQueuedManyIntersection() {
-    if (saveManyIntersections != null) {
-      saveManyIntersections.forEach(SaveManyBeans::saveIntersectionBatch);
+  private void saveQueuedMany() {
+    if (saveMany != null) {
+      saveMany.forEach(SaveMany::saveBatch);
     }
   }
 
@@ -960,19 +953,14 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     if (controller != null && !dirty) {
       // fire preUpdate notification when only element collection updated
       controller.preUpdate(this);
-    }
-  }
-
-  /**
-   * Combine with the beans postUpdate event notification.
-   */
-  public boolean postElementCollectionUpdate() {
-    if (controller != null) {
-      pendingPostUpdateNotify += 2;
+      pendingPostUpdateNotify = true;
     }
     if (!dirty) {
       setNotifyCache();
     }
+  }
+
+  public boolean isNotifyCache() {
     return notifyCache;
   }
 
@@ -982,13 +970,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
         controller.postInsert(this);
         break;
       case UPDATE:
-        if (pendingPostUpdateNotify == -1) {
-          // notify now - batched bean update with no element collection
-          controller.postUpdate(this);
-        } else {
-          // delay notify to combine with element collection update
-          pendingPostUpdateNotify++;
-        }
+        controller.postUpdate(this);
         break;
       case DELETE_SOFT:
         controller.postSoftDelete(this);
@@ -1105,7 +1087,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
       updatedManysOnly = true;
       setNotifyCache();
       addPostCommitListeners();
-      saveQueuedManyIntersection();
+      saveQueuedMany();
     }
     notifyCacheOnComplete();
     postUpdateNotify();
@@ -1444,20 +1426,20 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
   }
 
   /**
-   * Return true if the intersection table updates should be queued and batched.
+   * Return true if the intersection table updates or element collection updates should be queued.
    */
-  public boolean isQueueManyIntersection() {
+  public boolean isQueueSaveMany() {
     return !postExecute;
   }
 
   /**
-   * The intersection table updates to the batch executed later on postExecute.
+   * The intersection table updates or element collection to the batch executed later on postExecute.
    */
-  public void addManyIntersection(SaveManyBeans saveManyIntersection) {
-    if (this.saveManyIntersections == null) {
-      this.saveManyIntersections = new ArrayList<>();
+  public void addSaveMany(SaveMany saveManyRequest) {
+    if (this.saveMany == null) {
+      this.saveMany = new ArrayList<>();
     }
-    this.saveManyIntersections.add(saveManyIntersection);
+    this.saveMany.add(saveManyRequest);
   }
 
   public boolean isForcedUpdate() {
@@ -1471,7 +1453,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     saveRecurse = true;
   }
 
-  public void setGeneratedId() {
+  private void setGeneratedId() {
     beanDescriptor.setGeneratedId(entityBean, transaction);
   }
 }

src/main/java/io/ebeaninternal/server/core/Persister.java

@@ -104,8 +104,18 @@ public interface Persister {
   void executeOrQueue(SpiSqlUpdate update, SpiTransaction t, boolean queue);
 
   /**
-   * Add the statement to JDBC batch for later execution via executeBatch.
+   * Queue the SqlUpdate for early execution (with JDBC batch).
    */
+  void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t);
+
+  /**
+   * Queue the SqlUpdate for late execution (with JDBC batch).
+   */
+  void addToFlushQueueLast(SpiSqlUpdate update, SpiTransaction t);
+
+    /**
+     * Add the statement to JDBC batch for later execution via executeBatch.
+     */
   void addBatch(SpiSqlUpdate sqlUpdate, SpiTransaction transaction);
 
   /**

src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java

@@ -963,7 +963,7 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> implements ST
     return !elementCollection && cascadeInfo.isDelete();
   }
 
-  public String insertElementCollection() {
+  public SpiSqlUpdate insertElementCollection() {
     return sqlHelp.insertElementCollection();
   }
 

src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManySqlHelp.java

@@ -59,8 +59,8 @@ class BeanPropertyAssocManySqlHelp<T> {
     return sb.toString();
   }
 
-  String insertElementCollection() {
-    return elementCollectionInsertSql;
+  SpiSqlUpdate insertElementCollection() {
+    return new DefaultSqlUpdate(elementCollectionInsertSql);
   }
 
   private static class Cols extends BaseTablePropertyVisitor {

src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java

@@ -151,6 +151,16 @@ public final class DefaultPersister implements Persister {
     }
   }
 
+  @Override
+  public void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t) {
+    addToFlushQueue(update, t, true);
+  }
+
+  @Override
+  public void addToFlushQueueLast(SpiSqlUpdate update, SpiTransaction t) {
+    addToFlushQueue(update, t, false);
+  }
+
   private void addToFlushQueue(SpiSqlUpdate update, SpiTransaction t, boolean early) {
     new PersistRequestUpdateSql(server, update, t, persistExecute).addToFlushQueue(early);
   }
@@ -949,18 +959,15 @@ public final class DefaultPersister implements Persister {
 
   private SaveManyBase saveManyRequest(boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
     if (!many.isElementCollection()) {
-      return new SaveManyBeans(insertedParent, many, parentBean, request, this);
-
+      return new SaveManyBeans(this, insertedParent, many, parentBean, request);
     } else if (many.getManyType().isMap()) {
-      return new SaveManyElementCollectionMap(insertedParent, many, parentBean, request);
-
+      return new SaveManyElementCollectionMap(this, insertedParent, many, parentBean, request);
     } else {
-      return new SaveManyElementCollection(insertedParent, many, parentBean, request);
+      return new SaveManyElementCollection(this, insertedParent, many, parentBean, request);
     }
   }
 
   void deleteManyIntersection(EntityBean bean, BeanPropertyAssocMany<?> many, SpiTransaction t, boolean publish, boolean queue) {
-
     SpiSqlUpdate sqlDelete = deleteAllIntersection(bean, many, publish);
     if (queue) {
       addToFlushQueue(sqlDelete, t, true);

src/main/java/io/ebeaninternal/server/persist/SaveMany.java

@@ -0,0 +1,13 @@
+package io.ebeaninternal.server.persist;
+
+/**
+ * Save many that can be queued up to execute after the associated
+ * bean has been actually been persisted.
+ */
+public interface SaveMany {
+
+  /**
+   * Save the many property (after the associated bean persist).
+   */
+  void saveBatch();
+}

src/main/java/io/ebeaninternal/server/persist/SaveManyBase.java

@@ -14,10 +14,11 @@ import java.io.IOException;
 /**
  * Base for saving entity bean collections and element collections.
  */
-abstract class SaveManyBase {
+abstract class SaveManyBase implements SaveMany {
 
   private static final Logger log = LoggerFactory.getLogger(SaveManyBase.class);
 
+  final DefaultPersister persister;
   final PersistRequestBean<?> request;
   final SpiEbeanServer server;
   final boolean insertedParent;
@@ -26,7 +27,8 @@ abstract class SaveManyBase {
   final EntityBean parentBean;
   final Object value;
 
-  SaveManyBase(boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
+  SaveManyBase(DefaultPersister persister, boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
+    this.persister = persister;
     this.request = request;
     this.server = request.getServer();
     this.insertedParent = insertedParent;
@@ -41,10 +43,10 @@ abstract class SaveManyBase {
    */
   abstract void save();
 
-  void preElementCollectionUpdate(Object parentId) {
+  void preElementCollectionUpdate() {
     if (!insertedParent) {
       request.preElementCollectionUpdate();
-      server.execute(many.deleteByParentId(parentId, null), transaction);
+      persister.addToFlushQueue(many.deleteByParentId(request.getBeanId(), null), transaction);
     }
   }
 
@@ -64,7 +66,7 @@ abstract class SaveManyBase {
 
   void postElementCollectionUpdate() {
     if (!insertedParent) {
-      if (request.postElementCollectionUpdate()) {
+      if (request.isNotifyCache()) {
         try {
           String asJson = many.jsonWriteCollection(value);
           request.addCollectionChange(many.getName(), asJson);

src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java

@@ -37,12 +37,10 @@ public class SaveManyBeans extends SaveManyBase {
   private final DeleteMode deleteMode;
 
   private Collection<?> collection;
-  private final DefaultPersister persister;
   private int sortOrder;
 
-  SaveManyBeans(boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request, DefaultPersister persister) {
-    super(insertedParent, many, parentBean, request);
-    this.persister = persister;
+  SaveManyBeans(DefaultPersister persister, boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
+    super(persister, insertedParent, many, parentBean, request);
     this.cascade = many.getCascadeInfo().isSave();
     this.publish = request.isPublish();
     this.targetDescriptor = many.getTargetDescriptor();
@@ -238,9 +236,9 @@ public class SaveManyBeans extends SaveManyBase {
     if (value == null) {
       return;
     }
-    if (request.isQueueManyIntersection()) {
+    if (request.isQueueSaveMany()) {
       // queue/delay until bean persist request is flushed
-      request.addManyIntersection(this);
+      request.addSaveMany(this);
     } else {
       saveAssocManyIntersection(false);
     }
@@ -249,7 +247,8 @@ public class SaveManyBeans extends SaveManyBase {
   /**
    * Push intersection table changes onto batch flush queue.
    */
-  public void saveIntersectionBatch() {
+  @Override
+  public void saveBatch() {
     saveAssocManyIntersection(true);
   }
 

src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollection.java

@@ -1,7 +1,7 @@
 package io.ebeaninternal.server.persist;
 
-import io.ebean.SqlUpdate;
 import io.ebean.bean.EntityBean;
+import io.ebeaninternal.api.SpiSqlUpdate;
 import io.ebeaninternal.server.core.PersistRequestBean;
 import io.ebeaninternal.server.deploy.BeanCollectionUtil;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
@@ -13,28 +13,44 @@ import java.util.Collection;
  */
 class SaveManyElementCollection extends SaveManyBase {
 
-  SaveManyElementCollection(boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
-    super(insertedParent, many, parentBean, request);
+  private Collection<?> collection;
+
+  SaveManyElementCollection(DefaultPersister persister, boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
+    super(persister, insertedParent, many, parentBean, request);
+  }
+
+  private boolean modifiedCollection() {
+    return collection != null && (insertedParent || BeanCollectionUtil.isModified(value));
   }
 
   @Override
   void save() {
-
-    Collection<?> collection = BeanCollectionUtil.getActualEntries(value);
-    if (collection != null && (insertedParent || BeanCollectionUtil.isModified(value))) {
-      Object parentId = request.getBeanId();
-      preElementCollectionUpdate(parentId);
-
-      transaction.depth(+1);
-      SqlUpdate sqlInsert = server.createSqlUpdate(many.insertElementCollection());
-      for (Object value : collection) {
-        sqlInsert.setNextParameter(parentId);
-        many.bindElementValue(sqlInsert, value);
-        server.execute(sqlInsert, transaction);
+    collection = BeanCollectionUtil.getActualEntries(value);
+    if (modifiedCollection()) {
+      preElementCollectionUpdate();
+      if (insertedParent && request.isQueueSaveMany()) {
+        request.addSaveMany(this);
+      } else {
+        saveCollection();
       }
-      transaction.depth(-1);
-      resetModifyState();
-      postElementCollectionUpdate();
     }
   }
+
+  @Override
+  public void saveBatch() {
+    saveCollection();
+  }
+
+  private void saveCollection() {
+    SpiSqlUpdate proto = many.insertElementCollection();
+    Object parentId = request.getBeanId();
+    for (Object value : collection) {
+      final SpiSqlUpdate sqlInsert = proto.copy();
+      sqlInsert.setNextParameter(parentId);
+      many.bindElementValue(sqlInsert, value);
+      persister.addToFlushQueueLast(sqlInsert, transaction);
+    }
+    resetModifyState();
+    postElementCollectionUpdate();
+  }
 }

src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollectionMap.java

@@ -1,7 +1,7 @@
 package io.ebeaninternal.server.persist;
 
-import io.ebean.SqlUpdate;
 import io.ebean.bean.EntityBean;
+import io.ebeaninternal.api.SpiSqlUpdate;
 import io.ebeaninternal.server.core.PersistRequestBean;
 import io.ebeaninternal.server.deploy.BeanCollectionUtil;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
@@ -14,31 +14,46 @@ import java.util.Set;
  */
 class SaveManyElementCollectionMap extends SaveManyBase {
 
-  SaveManyElementCollectionMap(boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
-    super(insertedParent, many, parentBean, request);
+  private Set<Map.Entry<?, ?>> entries;
+
+  SaveManyElementCollectionMap(DefaultPersister persister, boolean insertedParent, BeanPropertyAssocMany<?> many, EntityBean parentBean, PersistRequestBean<?> request) {
+    super(persister, insertedParent, many, parentBean, request);
+  }
+
+  private boolean modifiedCollection() {
+    return entries != null && (insertedParent || BeanCollectionUtil.isModified(value));
   }
 
   @SuppressWarnings("unchecked")
   @Override
   void save() {
-
-    Set<Map.Entry<?, ?>> entries = (Set<Map.Entry<?, ?>>) BeanCollectionUtil.getActualEntries(value);
-    if (entries != null && (insertedParent || BeanCollectionUtil.isModified(value))) {
-      Object parentId = request.getBeanId();
-      preElementCollectionUpdate(parentId);
-
-      transaction.depth(+1);
-      SqlUpdate sqlInsert = server.createSqlUpdate(many.insertElementCollection());
-      for (Map.Entry<?, ?> entry : entries) {
-        sqlInsert.setNextParameter(parentId);
-        sqlInsert.setNextParameter(entry.getKey());
-        many.bindElementValue(sqlInsert, entry.getValue());
-        server.execute(sqlInsert, transaction);
+    entries = (Set<Map.Entry<?, ?>>) BeanCollectionUtil.getActualEntries(value);
+    if (modifiedCollection()) {
+      preElementCollectionUpdate();
+      if (insertedParent && request.isQueueSaveMany()) {
+        request.addSaveMany(this);
+      } else {
+        saveCollection();
       }
-
-      transaction.depth(-1);
-      resetModifyState();
-      postElementCollectionUpdate();
     }
   }
+
+  @Override
+  public void saveBatch() {
+    saveCollection();
+  }
+
+  private void saveCollection() {
+    SpiSqlUpdate proto = many.insertElementCollection();
+    Object parentId = request.getBeanId();
+    for (Map.Entry<?, ?> entry : entries) {
+      final SpiSqlUpdate sqlInsert = proto.copy();
+      sqlInsert.setNextParameter(parentId);
+      sqlInsert.setNextParameter(entry.getKey());
+      many.bindElementValue(sqlInsert, entry.getValue());
+      persister.addToFlushQueueLast(sqlInsert, transaction);
+    }
+    resetModifyState();
+    postElementCollectionUpdate();
+  }
 }