#816 - Error saving circular relationships (when generated Id values)

src/main/java/com/avaje/ebeaninternal/api/DerivedRelationshipData.java

@@ -1,27 +0,0 @@
-package com.avaje.ebeaninternal.api;
-
-public final class DerivedRelationshipData {
-
-	private final Object assocBean;
-	private final String logicalName;
-	private final Object bean;
-	
-	public DerivedRelationshipData(Object assocBean, String logicalName, Object bean) {
-	    this.assocBean = assocBean;
-	    this.logicalName = logicalName;
-	    this.bean = bean;
-    }
-	
-	public Object getAssocBean() {
-    	return assocBean;
-    }
-
-	public String getLogicalName() {
-    	return logicalName;
-	}
-
-	public Object getBean() {
-    	return bean;
-    }
-	
-}

src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java

@@ -8,13 +8,13 @@ import com.avaje.ebean.event.changelog.BeanChange;
 import com.avaje.ebean.event.changelog.ChangeSet;
 import com.avaje.ebeaninternal.server.core.PersistRequest;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
+import com.avaje.ebeaninternal.server.core.PersistDeferredRelationship;
 import com.avaje.ebeaninternal.server.persist.BatchControl;
 
 import javax.persistence.PersistenceException;
 import javax.persistence.RollbackException;
 import java.io.IOException;
 import java.sql.Connection;
-import java.util.List;
 
 /**
  * Wrapper of a ScopeTrans request and it's underlying transaction.
@@ -145,13 +145,8 @@ public class ScopedTransaction implements SpiTransaction {
   }
 
   @Override
-  public void registerDerivedRelationship(DerivedRelationshipData assocBean) {
-    transaction.registerDerivedRelationship(assocBean);
-  }
-
-  @Override
-  public List<DerivedRelationshipData> getDerivedRelationship(Object bean) {
-    return transaction.getDerivedRelationship(bean);
+  public void registerDeferred(PersistDeferredRelationship derived) {
+    transaction.registerDeferred(derived);
   }
 
   @Override

src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java

@@ -7,10 +7,10 @@ import com.avaje.ebean.event.changelog.BeanChange;
 import com.avaje.ebean.event.changelog.ChangeSet;
 import com.avaje.ebeaninternal.server.core.PersistRequest;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
+import com.avaje.ebeaninternal.server.core.PersistDeferredRelationship;
 import com.avaje.ebeaninternal.server.persist.BatchControl;
 
 import java.sql.Connection;
-import java.util.List;
 
 /**
  * Extends Transaction with additional API required on server.
@@ -48,15 +48,9 @@ public interface SpiTransaction extends Transaction {
   void logSummary(String msg);
 
   /**
-   * Register a "Derived Relationship" (that requires an additional update).
+   * Register a "Deferred Relationship" that requires an additional update later.
    */
-  void registerDerivedRelationship(DerivedRelationshipData assocBean);
-
-  /**
-   * Return the list of "Derived Relationships" that must be maintained after
-   * insert.
-   */
-  List<DerivedRelationshipData> getDerivedRelationship(Object bean);
+  void registerDeferred(PersistDeferredRelationship derived);
 
   /**
    * Add a deleting bean to the registered list.

src/main/java/com/avaje/ebeaninternal/server/core/PersistDeferredRelationship.java

@@ -0,0 +1,53 @@
+package com.avaje.ebeaninternal.server.core;
+
+import com.avaje.ebean.SqlUpdate;
+import com.avaje.ebean.bean.EntityBean;
+import com.avaje.ebeaninternal.api.SpiEbeanServer;
+import com.avaje.ebeaninternal.api.SpiTransaction;
+import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
+import com.avaje.ebeaninternal.server.deploy.id.ImportedId;
+
+/**
+ * Deferred update of a relationship where an Id value is not initially available
+ * so instead we execute this later as a SqlUpdate statement.
+ */
+public class PersistDeferredRelationship {
+
+  private final SpiEbeanServer ebeanServer;
+  private final BeanDescriptor<?> beanDescriptor;
+  private final EntityBean assocBean;
+  private final ImportedId importedId;
+  private final EntityBean bean;
+
+  public PersistDeferredRelationship(SpiEbeanServer ebeanServer, BeanDescriptor<?> beanDescriptor, EntityBean assocBean, ImportedId importedId, EntityBean bean) {
+
+    this.ebeanServer = ebeanServer;
+    this.beanDescriptor = beanDescriptor;
+    this.assocBean = assocBean;
+    this.importedId = importedId;
+    this.bean = bean;
+  }
+
+  /**
+   * Build and execute a SqlUpdate to set the importId value (as it will be available now).
+   * <p>
+   * This is executed later (deferred) until after JDBC batch flush or prior to commit.
+   * </p>
+   */
+  public void execute(SpiTransaction transaction) {
+
+    String sql = beanDescriptor.getUpdateImportedIdSql(importedId);
+    SqlUpdate sqlUpdate = ebeanServer.createSqlUpdate(sql);
+
+    // bind the set clause for the importedId
+    int pos = importedId.bind(1, sqlUpdate, assocBean);
+
+    // bind the where clause for the bean
+    Object[] idValues = beanDescriptor.getIdBinder().getIdValues(bean);
+    for (int j = 0; j < idValues.length; j++) {
+      sqlUpdate.setParameter(pos + j, idValues[j]);
+    }
+
+    ebeanServer.execute(sqlUpdate, transaction);
+  }
+}

src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java

@@ -1,16 +1,15 @@
 package com.avaje.ebeaninternal.server.core;
 
 import com.avaje.ebean.ValuePair;
-import com.avaje.ebean.bean.PreGetterCallback;
-import com.avaje.ebeaninternal.api.ConcurrencyMode;
 import com.avaje.ebean.annotation.DocStoreMode;
 import com.avaje.ebean.bean.EntityBean;
 import com.avaje.ebean.bean.EntityBeanIntercept;
+import com.avaje.ebean.bean.PreGetterCallback;
 import com.avaje.ebean.event.BeanPersistController;
 import com.avaje.ebean.event.BeanPersistListener;
 import com.avaje.ebean.event.BeanPersistRequest;
 import com.avaje.ebean.event.changelog.BeanChange;
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
+import com.avaje.ebeaninternal.api.ConcurrencyMode;
 import com.avaje.ebeaninternal.api.SpiEbeanServer;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.api.TransactionEvent;
@@ -19,12 +18,13 @@ import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
 import com.avaje.ebeaninternal.server.deploy.BeanManager;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
 import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocMany;
+import com.avaje.ebeaninternal.server.deploy.id.ImportedId;
 import com.avaje.ebeaninternal.server.persist.BatchControl;
 import com.avaje.ebeaninternal.server.persist.PersistExecute;
 import com.avaje.ebeaninternal.server.transaction.BeanPersistIdMap;
+import com.avaje.ebeanservice.docstore.api.DocStoreUpdate;
 import com.avaje.ebeanservice.docstore.api.DocStoreUpdateContext;
 import com.avaje.ebeanservice.docstore.api.DocStoreUpdates;
-import com.avaje.ebeanservice.docstore.api.DocStoreUpdate;
 
 import javax.persistence.OptimisticLockException;
 import javax.persistence.PersistenceException;
@@ -180,7 +180,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
 
   /**
    * Return the document store event that should be used for this request.
-   *
+   * <p>
    * Used to check if the Transaction has set the mode to IGNORE when doing large batch inserts that we
    * don't want to send to the doc store.
    */
@@ -874,8 +874,11 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
     }
   }
 
-  public List<DerivedRelationshipData> getDerivedRelationships() {
-    return transaction.getDerivedRelationship(bean);
+  /**
+   * Register the derived relationships to get executed later (on JDBC batch flush or commit).
+   */
+  public void deferredRelationship(EntityBean assocBean, ImportedId importedId, EntityBean bean) {
+    transaction.registerDeferred(new PersistDeferredRelationship(ebeanServer, beanDescriptor, assocBean, importedId, bean));
   }
 
   private void postInsert() {

src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -51,6 +51,7 @@ import com.avaje.ebeaninternal.server.core.InternString;
 import com.avaje.ebeaninternal.server.core.PersistRequest;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
 import com.avaje.ebeaninternal.server.deploy.id.IdBinder;
+import com.avaje.ebeaninternal.server.deploy.id.ImportedId;
 import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanDescriptor;
 import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanPropertyLists;
 import com.avaje.ebeaninternal.server.el.ElComparator;
@@ -1408,6 +1409,13 @@ public class BeanDescriptor<T> implements MetaBeanInfo, BeanType<T> {
     updatePlanCache.put(key, plan);
   }
 
+  /**
+   * Return a Sql update statement to set the importedId value (deferred execution).
+   */
+  public String getUpdateImportedIdSql(ImportedId prop) {
+    return "update " + baseTable + " set " + prop.importedIdClause() + " where " + idBinder.getBindIdSql(null);
+  }
+
   /**
    * Return true if updates should only include changed properties. Otherwise
    * all loaded properties are included in the update.

src/main/java/com/avaje/ebeaninternal/server/deploy/id/ImportedId.java

@@ -2,6 +2,7 @@ package com.avaje.ebeaninternal.server.deploy.id;
 
 import java.sql.SQLException;
 
+import com.avaje.ebean.SqlUpdate;
 import com.avaje.ebean.bean.EntityBean;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
 import com.avaje.ebeaninternal.server.deploy.DbSqlContext;
@@ -46,6 +47,11 @@ public interface ImportedId {
    */
   Object bind(BindableRequest request, EntityBean bean) throws SQLException;
 
+  /**
+   * Bind the imported Id value to the SqlUpdate.
+   */
+  int bind(int position, SqlUpdate update, EntityBean bean);
+
   /**
    * For inserting into ManyToMany intersection.
    */
@@ -55,4 +61,9 @@ public interface ImportedId {
    * Used to derive a missing concatenated key from multiple imported keys.
    */
   BeanProperty findMatchImport(String matchDbColumn);
+
+  /**
+   * Return the set importedId clause.
+   */
+  String importedIdClause();
 }

src/main/java/com/avaje/ebeaninternal/server/deploy/id/ImportedIdEmbedded.java

@@ -4,6 +4,7 @@ import java.sql.SQLException;
 
 import javax.persistence.PersistenceException;
 
+import com.avaje.ebean.SqlUpdate;
 import com.avaje.ebean.bean.EntityBean;
 import com.avaje.ebeaninternal.server.deploy.BeanFkeyProperty;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
@@ -63,6 +64,34 @@ public class ImportedIdEmbedded implements ImportedId {
     }
   }
 
+  @Override
+  public String importedIdClause() {
+
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < imported.length; i++) {
+      if (i > 0) {
+        sb.append(", ");
+      }
+      sb.append(imported[i].localDbColumn).append(" = ?");
+    }
+    return sb.toString();
+  }
+
+  @Override
+  public int bind(int position, SqlUpdate update, EntityBean bean) {
+
+    int pos = position;
+
+    EntityBean embedded = (EntityBean) foreignAssocOne.getValue(bean);
+    for (int i = 0; i < imported.length; i++) {
+      if (imported[i].owner.isUpdateable()) {
+        Object scalarValue = imported[i].foreignProperty.getValue(embedded);
+        update.setParameter(pos++, scalarValue);
+      }
+    }
+    return pos;
+  }
+
   public Object bind(BindableRequest request, EntityBean bean) throws SQLException {
 
     Object embeddedId = null;

src/main/java/com/avaje/ebeaninternal/server/deploy/id/ImportedIdSimple.java

@@ -7,6 +7,7 @@ import java.util.List;
 
 import javax.persistence.PersistenceException;
 
+import com.avaje.ebean.SqlUpdate;
 import com.avaje.ebean.bean.EntityBean;
 import com.avaje.ebeaninternal.server.core.InternString;
 import com.avaje.ebeaninternal.server.deploy.BeanFkeyProperty;
@@ -110,6 +111,18 @@ public final class ImportedIdSimple implements ImportedId, Comparable<ImportedId
     request.appendColumn(localDbColumn);
   }
 
+  @Override
+  public String importedIdClause() {
+    return localDbColumn + " = ?";
+  }
+
+  @Override
+  public int bind(int position, SqlUpdate update, EntityBean bean) {
+    Object value = getIdValue(bean);
+    update.setParameter(position, value);
+    return ++position;
+  }
+
   public Object bind(BindableRequest request, EntityBean bean) throws SQLException {
 
     Object value = null;

src/main/java/com/avaje/ebeaninternal/server/persist/dml/DeleteHandler.java

@@ -1,6 +1,5 @@
 package com.avaje.ebeaninternal.server.persist.dml;
 
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
 
@@ -49,10 +48,5 @@ public class DeleteHandler extends DmlHandler {
 		checkRowCount(rowCount);
     return rowCount;
 	}
-
-  @Override
-  public void registerDerivedRelationship(DerivedRelationshipData assocBean) {
-    throw new RuntimeException("Never called on delete");
-  }
     
 }

src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java

@@ -1,13 +1,10 @@
 package com.avaje.ebeaninternal.server.persist.dml;
 
 import com.avaje.ebean.bean.EntityBean;
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
-import com.avaje.ebeaninternal.api.SpiEbeanServer;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.server.core.Message;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
 import com.avaje.ebeaninternal.server.deploy.BeanDescriptor;
-import com.avaje.ebeaninternal.server.deploy.BeanProperty;
 import com.avaje.ebeaninternal.server.persist.DmlUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -18,7 +15,6 @@ import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.List;
 
 /**
  * Insert bean handler.
@@ -134,30 +130,9 @@ public class InsertHandler extends DmlHandler {
     }
 
     checkRowCount(rowCount);
-    executeDerivedRelationships();
     return rowCount;
   }
 
-  protected void executeDerivedRelationships() {
-    List<DerivedRelationshipData> derivedRelationships = persistRequest.getDerivedRelationships();
-    if (derivedRelationships != null) {
-
-      SpiEbeanServer ebeanServer = (SpiEbeanServer) persistRequest.getEbeanServer();
-
-      for (int i = 0; i < derivedRelationships.size(); i++) {
-        DerivedRelationshipData derivedRelationshipData = derivedRelationships.get(i);
-
-        BeanDescriptor<?> beanDescriptor = ebeanServer.getBeanDescriptor(derivedRelationshipData.getBean().getClass());
-
-        BeanProperty prop = beanDescriptor.getBeanProperty(derivedRelationshipData.getLogicalName());
-        EntityBean entityBean = (EntityBean) derivedRelationshipData.getBean();
-        entityBean._ebean_getIntercept().markPropertyAsChanged(prop.getPropertyIndex());
-
-        ebeanServer.update(entityBean, transaction);
-      }
-    }
-  }
-
   /**
    * For non batch insert with generated keys.
    */
@@ -220,9 +195,4 @@ public class InsertHandler extends DmlHandler {
     }
   }
 
-  @Override
-  public void registerDerivedRelationship(DerivedRelationshipData derivedRelationship) {
-    persistRequest.getTransaction().registerDerivedRelationship(derivedRelationship);
-  }
-
 }

src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateHandler.java

@@ -1,6 +1,5 @@
 package com.avaje.ebeaninternal.server.persist.dml;
 
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.api.SpiUpdatePlan;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
@@ -74,9 +73,4 @@ public class UpdateHandler extends DmlHandler {
     return 0;
   }
 
-  @Override
-  public void registerDerivedRelationship(DerivedRelationshipData derivedRelationship) {
-    persistRequest.getTransaction().registerDerivedRelationship(derivedRelationship);
-  }
-
 }

src/main/java/com/avaje/ebeaninternal/server/persist/dmlbind/BindableAssocOne.java

@@ -1,15 +1,14 @@
 package com.avaje.ebeaninternal.server.persist.dmlbind;
 
-import java.sql.SQLException;
-import java.util.List;
-
 import com.avaje.ebean.bean.EntityBean;
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
 import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocOne;
 import com.avaje.ebeaninternal.server.deploy.id.ImportedId;
 import com.avaje.ebeaninternal.server.persist.dml.GenerateDmlRequest;
 
+import java.sql.SQLException;
+import java.util.List;
+
 /**
  * Bindable for an ManyToOne or OneToOne associated bean.
  */
@@ -51,10 +50,8 @@ public class BindableAssocOne implements Bindable {
       // this is the scenario for a derived foreign key
       // which will require an additional update
       // register for post insert of assocBean
-      // update of bean set ... importedId.getLogicalName();
-      // value of assocBean.getId
-      DerivedRelationshipData d = new DerivedRelationshipData(assocBean, assocOne.getName(), bean);
-      request.registerDerivedRelationship(d);
+      // update of bean set importedId
+      request.getPersistRequest().deferredRelationship(assocBean, importedId, bean);
     }
   }
 

src/main/java/com/avaje/ebeaninternal/server/persist/dmlbind/BindableRequest.java

@@ -1,6 +1,5 @@
 package com.avaje.ebeaninternal.server.persist.dmlbind;
 
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
 import com.avaje.ebeaninternal.server.deploy.BeanProperty;
 
@@ -54,8 +53,6 @@ public interface BindableRequest {
    */
   PersistRequestBean<?> getPersistRequest();
 
-  void registerDerivedRelationship(DerivedRelationshipData assocBean);
-
   /**
    * Return the system current time in millis. This is expected to the same time used
    * by multiple generated properties for a single request.

src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java

@@ -8,11 +8,11 @@ import com.avaje.ebean.config.ServerConfig;
 import com.avaje.ebean.config.dbplatform.DatabasePlatform.OnQueryOnly;
 import com.avaje.ebean.event.changelog.BeanChange;
 import com.avaje.ebean.event.changelog.ChangeSet;
-import com.avaje.ebeaninternal.api.DerivedRelationshipData;
 import com.avaje.ebeaninternal.api.SpiTransaction;
 import com.avaje.ebeaninternal.api.TransactionEvent;
 import com.avaje.ebeaninternal.server.core.PersistRequest;
 import com.avaje.ebeaninternal.server.core.PersistRequestBean;
+import com.avaje.ebeaninternal.server.core.PersistDeferredRelationship;
 import com.avaje.ebeaninternal.server.lib.util.Str;
 import com.avaje.ebeaninternal.server.persist.BatchControl;
 import org.slf4j.Logger;
@@ -136,8 +136,6 @@ public class JdbcTransaction implements SpiTransaction {
 
   protected HashMap<String, String> m2mIntersectionSave;
 
-  protected HashMap<Integer, List<DerivedRelationshipData>> derivedRelMap;
-
   protected Map<String, Object> userObjects;
 
   protected List<TransactionCallback> callbackList;
@@ -146,6 +144,8 @@ public class JdbcTransaction implements SpiTransaction {
 
   protected TChangeLogHolder changeLogHolder;
 
+  protected List<PersistDeferredRelationship> deferredList;
+
   /**
    * The mode for updating doc store indexes for this transaction.
    * Only set when you want to override the default behavior.
@@ -334,26 +334,11 @@ public class JdbcTransaction implements SpiTransaction {
   }
 
   @Override
-  public List<DerivedRelationshipData> getDerivedRelationship(Object bean) {
-    if (derivedRelMap == null) {
-      return null;
+  public void registerDeferred(PersistDeferredRelationship derived) {
+    if (deferredList == null) {
+      deferredList = new ArrayList<PersistDeferredRelationship>();
     }
-    return derivedRelMap.get(System.identityHashCode(bean));
-  }
-
-  @Override
-  public void registerDerivedRelationship(DerivedRelationshipData derivedRelationship) {
-    if (derivedRelMap == null) {
-      derivedRelMap = new HashMap<Integer, List<DerivedRelationshipData>>();
-    }
-    Integer key = System.identityHashCode(derivedRelationship.getAssocBean());
-
-    List<DerivedRelationshipData> list = derivedRelMap.get(key);
-    if (list == null) {
-      list = new ArrayList<DerivedRelationshipData>();
-      derivedRelMap.put(key, list);
-    }
-    list.add(derivedRelationship);
+    deferredList.add(derived);
   }
 
   /**
@@ -715,9 +700,22 @@ public class JdbcTransaction implements SpiTransaction {
     if (!isActive()) {
       throw new IllegalStateException(illegalStateMessage);
     }
+    internalBatchFlush();
+  }
+
+  /**
+   * Flush the JDBC batch and execute derived relationship statements if necessary.
+   */
+  private void internalBatchFlush() {
     if (batchControl != null) {
       batchControl.flush();
     }
+    if (deferredList != null) {
+      for (PersistDeferredRelationship deferred : deferredList) {
+        deferred.execute(this);
+      }
+      deferredList.clear();
+    }
   }
 
   /**
@@ -886,9 +884,7 @@ public class JdbcTransaction implements SpiTransaction {
    * Batch flush, jdbc commit, trigger registered TransactionCallbacks, notify l2 cache etc.
    */
   private void flushCommitAndNotify() throws SQLException {
-    if (batchControl != null && !batchControl.isEmpty()) {
-      batchControl.flush();
-    }
+    internalBatchFlush();
     firePreCommit();
     // only performCommit can throw an exception
     performCommit();