From 3fa8f04a4c4e14d825fce301696ce3c2d6f84694 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Mon, 27 Jul 2015 22:59:18 +1200 Subject: [PATCH] #358 - ENH: Add ServerConfig updateAllPropertiesInBatch with a default of true --- .../com/avaje/ebean/config/ServerConfig.java | 29 +++++- .../ebeaninternal/api/ScopedTransaction.java | 2 +- .../ebeaninternal/api/SpiEbeanServer.java | 7 +- .../ebeaninternal/api/SpiTransaction.java | 3 +- .../server/core/DefaultServer.java | 8 ++ .../server/core/PersistRequestBean.java | 32 ++++++- .../server/persist/dml/InsertHandler.java | 1 - .../server/persist/dml/UpdateMeta.java | 13 +-- .../server/transaction/JdbcTransaction.java | 4 +- .../ebeaninternal/api/TDSpiEbeanServer.java | 5 + .../batchinsert/TestBatchInsertSimple.java | 10 ++ .../com/avaje/tests/model/basic/UTMaster.java | 92 ++++++++++--------- 12 files changed, 149 insertions(+), 57 deletions(-) diff --git a/src/main/java/com/avaje/ebean/config/ServerConfig.java b/src/main/java/com/avaje/ebean/config/ServerConfig.java index 3c18c67b4..0e7b7f51a 100644 --- a/src/main/java/com/avaje/ebean/config/ServerConfig.java +++ b/src/main/java/com/avaje/ebean/config/ServerConfig.java @@ -243,7 +243,12 @@ public class ServerConfig { * Behaviour of update to include on the change properties. */ private boolean updateChangesOnly = true; - + + /** + * Behaviour of updates in JDBC batch to by default include all properties. + */ + private boolean updateAllPropertiesInBatch = true; + /** * Default behaviour for updates when cascade save on a O2M or M2M to delete any missing children. */ @@ -1516,7 +1521,26 @@ public class ServerConfig { public void setUpdateChangesOnly(boolean updateChangesOnly) { this.updateChangesOnly = updateChangesOnly; } - + + /** + * Returns true if updates in JDBC batch default to include all properties by default. + */ + public boolean isUpdateAllPropertiesInBatch() { + return updateAllPropertiesInBatch; + } + + /** + * Set to false if by default updates in JDBC batch should not include all properties. + *

+ * This mode can be explicitly set per transaction. + *

+ * + * @see com.avaje.ebean.Transaction#setUpdateAllLoadedProperties(boolean) + */ + public void setUpdateAllPropertiesInBatch(boolean updateAllPropertiesInBatch) { + this.updateAllPropertiesInBatch = updateAllPropertiesInBatch; + } + /** * Return true if updates by default delete missing children when cascading save to a OneToMany or * ManyToMany. When not set this defaults to true. @@ -1860,6 +1884,7 @@ public class ServerConfig { collectQueryStatsByNode = p.getBoolean("collectQueryStatsByNode", collectQueryStatsByNode); collectQueryOrigins = p.getBoolean("collectQueryOrigins", collectQueryOrigins); + updateAllPropertiesInBatch = p.getBoolean("updateAllPropertiesInBatch", updateAllPropertiesInBatch); updateChangesOnly = p.getBoolean("updateChangesOnly", updateChangesOnly); boolean defaultDeleteMissingChildren = p.getBoolean("defaultDeleteMissingChildren", updatesDeleteMissingChildren); diff --git a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java index 79f59fc7e..d8346960d 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java @@ -158,7 +158,7 @@ public class ScopedTransaction implements SpiTransaction { } @Override - public boolean isUpdateAllLoadedProperties() { + public Boolean isUpdateAllLoadedProperties() { return transaction.isUpdateAllLoadedProperties(); } diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiEbeanServer.java b/src/main/java/com/avaje/ebeaninternal/api/SpiEbeanServer.java index 39d51e7cb..5cbea7a2e 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiEbeanServer.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiEbeanServer.java @@ -32,7 +32,12 @@ public interface SpiEbeanServer extends EbeanServer, BeanLoader, BeanCollectionL * Return true if query origins should be collected. */ boolean isCollectQueryOrigins(); - + + /** + * Return true if updates in JDBC batch should include all columns if unspecified on the transaction. + */ + boolean isUpdateAllPropertiesInBatch(); + /** * Return the server configuration. */ diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java index 3f3dc9e93..670d53a8e 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java @@ -101,8 +101,9 @@ public interface SpiTransaction extends Transaction { /** * Return true if this transaction has updateAllLoadedProperties set. + * If null is returned the server default is used (set on ServerConfig). */ - boolean isUpdateAllLoadedProperties(); + Boolean isUpdateAllLoadedProperties(); /** * Return the batchSize specifically set for this transaction or 0. diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java index b6057aa02..c4bdbb550 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java @@ -148,6 +148,8 @@ public final class DefaultServer implements SpiEbeanServer { */ private List ebeanPlugins; + private final boolean updateAllPropertiesInBatch; + private final boolean collectQueryOrigins; private final boolean collectQueryStatsByNode; @@ -182,6 +184,7 @@ public final class DefaultServer implements SpiEbeanServer { this.beanDescriptorManager = config.getBeanDescriptorManager(); beanDescriptorManager.setEbeanServer(this); + this.updateAllPropertiesInBatch = serverConfig.isUpdateAllPropertiesInBatch(); this.collectQueryOrigins = serverConfig.isCollectQueryOrigins(); this.collectQueryStatsByNode = serverConfig.isCollectQueryStatsByNode(); this.maxCallStack = serverConfig.getMaxCallStack(); @@ -252,6 +255,11 @@ public final class DefaultServer implements SpiEbeanServer { return collectQueryOrigins; } + @Override + public boolean isUpdateAllPropertiesInBatch() { + return updateAllPropertiesInBatch; + } + public int getLazyLoadBatchSize() { return lazyLoadBatchSize; } diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java index 55f6ad25c..22772b056 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java @@ -117,6 +117,11 @@ public final class PersistRequestBean extends PersistRequest implements BeanP */ private boolean batchOnCascadeSet; + /** + * Set for updates to determine if all loaded properties are included in the update. + */ + private boolean requestUpdateAllLoadedProps; + public PersistRequestBean(SpiEbeanServer server, T bean, Object parentBean, BeanManager mgr, SpiTransaction t, PersistExecute persistExecute, PersistRequest.Type type, boolean saveRecurse) { @@ -544,6 +549,9 @@ public final class PersistRequestBean extends PersistRequest implements BeanP // if bean persisted again then should result in an update intercept.setLoaded(); + if (isInsert()) { + postInsert(); + } addEvent(); @@ -644,7 +652,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP * Return true if the property should be included in the update. */ public boolean isAddToUpdate(BeanProperty prop) { - if (transaction.isUpdateAllLoadedProperties()) { + if (requestUpdateAllLoadedProps) { return intercept.isLoadedProperty(prop.getPropertyIndex()); } else { return intercept.isDirtyProperty(prop.getPropertyIndex()); @@ -655,7 +663,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP return transaction.getDerivedRelationship(bean); } - public void postInsert() { + private void postInsert() { // mark all properties as loaded after an insert to support immediate update int len = intercept.getPropertyLength(); for (int i = 0; i < len; i++) { @@ -710,4 +718,24 @@ public final class PersistRequestBean extends PersistRequest implements BeanP return updatedManysOnly; } + + /** + * Determine if all loaded properties should be used for an update. + *

+ * Takes into account transaction setting and JDBC batch. + *

+ */ + public boolean determineUpdateAllLoadedProperties() { + + Boolean txnUpdateAll = transaction.isUpdateAllLoadedProperties(); + if (txnUpdateAll != null) { + // use the setting explicitly set on the transaction + requestUpdateAllLoadedProps = txnUpdateAll; + } else { + // if using batch use the server default setting + requestUpdateAllLoadedProps = isBatchThisRequest() && ebeanServer.isUpdateAllPropertiesInBatch(); + } + + return requestUpdateAllLoadedProps; + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java index 0a1783640..b17b32578 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/InsertHandler.java @@ -137,7 +137,6 @@ public class InsertHandler extends DmlHandler { checkRowCount(rc); executeDerivedRelationships(); - persistRequest.postInsert(); } protected void executeDerivedRelationships() { diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateMeta.java b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateMeta.java index 7a1d9dec0..7656211a8 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateMeta.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/dml/UpdateMeta.java @@ -90,12 +90,12 @@ public final class UpdateMeta { */ public SpiUpdatePlan getUpdatePlan(PersistRequestBean request) { - ConcurrencyMode mode = request.determineConcurrencyMode(); if (request.isDynamicUpdateSql()) { - return getDynamicUpdatePlan(mode, request); + return getDynamicUpdatePlan(request); } // 'full bean' update... + ConcurrencyMode mode = request.determineConcurrencyMode(); switch (mode) { case NONE: return modeNoneUpdatePlan; @@ -108,14 +108,12 @@ public final class UpdateMeta { } } - private SpiUpdatePlan getDynamicUpdatePlan(ConcurrencyMode mode, PersistRequestBean persistRequest) { - - // we can use a cached UpdatePlan for the changed properties + private SpiUpdatePlan getDynamicUpdatePlan(PersistRequestBean persistRequest) { EntityBeanIntercept ebi = persistRequest.getEntityBeanIntercept(); int hash; - if (persistRequest.getTransaction().isUpdateAllLoadedProperties()) { + if (persistRequest.determineUpdateAllLoadedProperties()) { hash = ebi.getLoadedPropertyHash(); } else { hash = ebi.getDirtyPropertyHash(); @@ -132,6 +130,7 @@ public final class UpdateMeta { Integer key = Integer.valueOf(hash); + // check if we can use a cached UpdatePlan SpiUpdatePlan updatePlan = beanDescriptor.getUpdatePlan(key); if (updatePlan != null) { return updatePlan; @@ -144,6 +143,8 @@ public final class UpdateMeta { set.addToUpdate(persistRequest, list); BindableList bindableList = new BindableList(list); + ConcurrencyMode mode = persistRequest.determineConcurrencyMode(); + // build the SQL for this update statement String sql = genSql(mode, persistRequest, bindableList); diff --git a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java index ce3b61646..1f452ab94 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java @@ -91,7 +91,7 @@ public class JdbcTransaction implements SpiTransaction { protected boolean localReadOnly; - protected boolean updateAllLoadedProperties; + protected Boolean updateAllLoadedProperties; protected PersistBatch oldBatchMode; @@ -400,7 +400,7 @@ public class JdbcTransaction implements SpiTransaction { this.updateAllLoadedProperties = updateAllLoadedProperties; } - public boolean isUpdateAllLoadedProperties() { + public Boolean isUpdateAllLoadedProperties() { return updateAllLoadedProperties; } diff --git a/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java b/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java index 6030c0120..4ceb6869b 100644 --- a/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java +++ b/src/test/java/com/avaje/ebeaninternal/api/TDSpiEbeanServer.java @@ -48,6 +48,11 @@ public class TDSpiEbeanServer implements SpiEbeanServer { return false; } + @Override + public boolean isUpdateAllPropertiesInBatch() { + return false; + } + @Override public ServerConfig getServerConfig() { return null; diff --git a/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java b/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java index 3d3a6a3ed..93dd03e8e 100644 --- a/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java +++ b/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java @@ -121,6 +121,16 @@ public class TestBatchInsertSimple extends BaseTestCase { // escalate based on batchOnCascade value Ebean.saveAll(masters); + for (int i = 0; i < masters.size() ; i++) { + UTMaster utMaster = masters.get(i); + utMaster.setName(utMaster.getName()+"-Mod"); + if (i % 2 == 0) { + // make the updates a little bit different + utMaster.setDescription("Blah"); + } + } + + Ebean.saveAll(masters); } private UTMaster createMasterAndDetails(int masterPos, int size) { diff --git a/src/test/java/com/avaje/tests/model/basic/UTMaster.java b/src/test/java/com/avaje/tests/model/basic/UTMaster.java index b8854d529..b25922b31 100644 --- a/src/test/java/com/avaje/tests/model/basic/UTMaster.java +++ b/src/test/java/com/avaje/tests/model/basic/UTMaster.java @@ -13,56 +13,66 @@ import javax.persistence.Table; import javax.persistence.Version; @Entity -@Table(name="ut_master") +@Table(name = "ut_master") public class UTMaster extends Model { - @Id - Integer id; - - String name; - - @Version - Integer version; - - @OneToMany(cascade=CascadeType.ALL) - List details; + @Id + Integer id; - public Integer getId() { - return id; - } + String name; - public void setId(Integer id) { - this.id = id; - } + String description; - public String getName() { - return name; - } + @Version + Integer version; - public void setName(String name) { - this.name = name; - } + @OneToMany(cascade = CascadeType.ALL) + List details; - public Integer getVersion() { - return version; - } + public Integer getId() { + return id; + } - public void setVersion(Integer version) { - this.version = version; - } + public void setId(Integer id) { + this.id = id; + } - public List getDetails() { - return details; - } + public String getName() { + return name; + } - public void setDetails(List details) { - this.details = details; - } - - public void addDetail(UTDetail detail) { - if (details == null){ - details = new ArrayList(); - } - details.add(detail); + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public Integer getVersion() { + return version; + } + + public void setVersion(Integer version) { + this.version = version; + } + + public List getDetails() { + return details; + } + + public void setDetails(List details) { + this.details = details; + } + + public void addDetail(UTDetail detail) { + if (details == null) { + details = new ArrayList(); } + details.add(detail); + } }