diff --git a/src/main/java/com/avaje/ebean/bean/BeanCollection.java b/src/main/java/com/avaje/ebean/bean/BeanCollection.java index 4b144b7eb..17cbf6ab3 100644 --- a/src/main/java/com/avaje/ebean/bean/BeanCollection.java +++ b/src/main/java/com/avaje/ebean/bean/BeanCollection.java @@ -54,10 +54,12 @@ public interface BeanCollection extends Serializable { void reset(EntityBean ownerBean, String propertyName); /** - * Return true if the collection is empty and untouched. Used to detect if a - * collection was 'cleared' deliberately or just un-initialised. + * Return true if the collection is uninitialised or is empty without any held modifications. + *

+ * Returning true means can safely skip cascade save for this bean collection. + *

*/ - boolean isEmptyAndUntouched(); + boolean isSkipSave(); /** * Return the bean that owns this collection. diff --git a/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java b/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java index 9d9653e31..7e1e21171 100644 --- a/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java +++ b/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java @@ -51,12 +51,6 @@ public abstract class AbstractBeanCollection implements BeanCollection { protected boolean modifyRemoveListening; protected boolean modifyListening; - /** - * Flag used to tell if empty collections have been cleared etc or just - * uninitialised. - */ - protected boolean touched; - /** * Constructor not non-lazy loading collection. */ @@ -105,16 +99,6 @@ public abstract class AbstractBeanCollection implements BeanCollection { checkEmptyLazyLoad(); } - /** - * Set touched. If setFlag is false then typically an isEmpty() call and still - * considering that to be untouched. - */ - protected void touched(boolean setFlag) { - if (setFlag) { - touched = true; - } - } - public boolean isRegisteredWithLoadContext() { return registeredWithLoadContext; } @@ -206,4 +190,11 @@ public abstract class AbstractBeanCollection implements BeanCollection { return modifyHolder.getModifyRemovals(); } } + + /** + * Return true if there are underlying additions or removals. + */ + public boolean holdsModifications() { + return modifyHolder != null && modifyHolder.hasModifications(); + } } diff --git a/src/main/java/com/avaje/ebean/common/BeanList.java b/src/main/java/com/avaje/ebean/common/BeanList.java index 2ac4c523d..fbfb9bfd5 100644 --- a/src/main/java/com/avaje/ebean/common/BeanList.java +++ b/src/main/java/com/avaje/ebean/common/BeanList.java @@ -52,12 +52,11 @@ public final class BeanList extends AbstractBeanCollection implements List this.ownerBean = ownerBean; this.propertyName = propertyName; this.list = null; - this.touched = false; } @Override - public boolean isEmptyAndUntouched() { - return !touched && (list == null || list.isEmpty()); + public boolean isSkipSave() { + return list == null || (list.isEmpty() && !holdsModifications()); } @SuppressWarnings("unchecked") @@ -109,24 +108,14 @@ public final class BeanList extends AbstractBeanCollection implements List list = new ArrayList(); } } - touched(true); } } - private void initAsUntouched() { - init(false); - } - private void init() { - init(true); - } - - private void init(boolean setTouched) { synchronized (this) { if (list == null) { lazyLoadCollection(false); } - touched(setTouched); } } @@ -296,7 +285,7 @@ public final class BeanList extends AbstractBeanCollection implements List } public boolean isEmpty() { - initAsUntouched(); + init(); return list.isEmpty(); } diff --git a/src/main/java/com/avaje/ebean/common/BeanMap.java b/src/main/java/com/avaje/ebean/common/BeanMap.java index d31e01968..f046c83f9 100644 --- a/src/main/java/com/avaje/ebean/common/BeanMap.java +++ b/src/main/java/com/avaje/ebean/common/BeanMap.java @@ -45,11 +45,10 @@ public final class BeanMap extends AbstractBeanCollection implements Ma this.ownerBean = ownerBean; this.propertyName = propertyName; this.map = null; - this.touched = false; } - public boolean isEmptyAndUntouched() { - return !touched && (map == null || map.isEmpty()); + public boolean isSkipSave() { + return map == null || (map.isEmpty() && !holdsModifications()); } @Override @@ -125,24 +124,14 @@ public final class BeanMap extends AbstractBeanCollection implements Ma map = new LinkedHashMap(); } } - touched(true); } } - private void initAsUntouched() { - init(false); - } - private void init() { - init(true); - } - - private void init(boolean setTouched) { synchronized (this) { if (map == null) { lazyLoadCollection(false); } - touched(setTouched); } } @@ -249,7 +238,7 @@ public final class BeanMap extends AbstractBeanCollection implements Ma } public boolean isEmpty() { - initAsUntouched(); + init(); return map.isEmpty(); } diff --git a/src/main/java/com/avaje/ebean/common/BeanSet.java b/src/main/java/com/avaje/ebean/common/BeanSet.java index 8e1ed5b2f..67df7ddb2 100644 --- a/src/main/java/com/avaje/ebean/common/BeanSet.java +++ b/src/main/java/com/avaje/ebean/common/BeanSet.java @@ -46,11 +46,10 @@ public final class BeanSet extends AbstractBeanCollection implements Set extends AbstractBeanCollection implements Set(); } } - touched(true); } } - private void initAsUntouched() { - init(false); - } - private void init() { - init(true); - } - - private void init(boolean setTouched) { synchronized (this) { if (set == null) { lazyLoadCollection(true); } - touched(setTouched); } } @@ -260,7 +249,7 @@ public final class BeanSet extends AbstractBeanCollection implements Set implements Serializable { Set getModifyRemovals() { return modifyDeletions; } + + /** + * Return true if there additions or removals. + */ + boolean hasModifications() { + return !modifyDeletions.isEmpty() || !modifyAdditions.isEmpty(); + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanPropertyAssocMany.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanPropertyAssocMany.java index 9fd52d7c3..97af7847e 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanPropertyAssocMany.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanPropertyAssocMany.java @@ -238,13 +238,13 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc { /** * Return true if this is considered 'empty' from a save perspective. */ - public boolean isEmptyBeanCollection(EntityBean bean, boolean insertedParent) { + public boolean isSkipSaveBeanCollection(EntityBean bean, boolean insertedParent) { Object val = getValue(bean); if (val == null) { return true; } if ((val instanceof BeanCollection)) { - return ((BeanCollection) val).isEmptyAndUntouched(); + return ((BeanCollection) val).isSkipSave(); } if (insertedParent) { // check 'vanilla' collection types diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java index 9bf0a2793..f7f1dd151 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java @@ -806,8 +806,8 @@ public final class DefaultPersister implements Persister { // many's with cascade save BeanPropertyAssocMany[] manys = desc.propertiesManySave(); for (int i = 0; i < manys.length; i++) { - // check that property is loaded and not empty uninitialised collection - if (request.isLoadedProperty(manys[i]) && !manys[i].isEmptyBeanCollection(parentBean, insertedParent)) { + // check that property is loaded and collection should be cascaded to + if (request.isLoadedProperty(manys[i]) && !manys[i].isSkipSaveBeanCollection(parentBean, insertedParent)) { saveMany(new SaveManyPropRequest(insertedParent, manys[i], parentBean, request), insertMode); if (!insertedParent) { request.addUpdatedManyProperty(manys[i]);