#709 - Refactor: BeanCollection internals, rename method isEmptyAndUntouched() to isSkipSave(), remove 'touched' flag

src/main/java/com/avaje/ebean/bean/BeanCollection.java

@@ -54,10 +54,12 @@ public interface BeanCollection<E> 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.
+   * <p>
+   * Returning true means can safely skip cascade save for this bean collection.
+   * </p>
    */
-  boolean isEmptyAndUntouched();
+  boolean isSkipSave();
 
   /**
    * Return the bean that owns this collection.

src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java

@@ -51,12 +51,6 @@ public abstract class AbstractBeanCollection<E> implements BeanCollection<E> {
   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<E> implements BeanCollection<E> {
     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<E> implements BeanCollection<E> {
       return modifyHolder.getModifyRemovals();
     }
   }
+
+  /**
+   * Return true if there are underlying additions or removals.
+   */
+  public boolean holdsModifications() {
+    return modifyHolder != null && modifyHolder.hasModifications();
+  }
 }

src/main/java/com/avaje/ebean/common/BeanList.java

@@ -52,12 +52,11 @@ public final class BeanList<E> extends AbstractBeanCollection<E> 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<E> extends AbstractBeanCollection<E> implements List
           list = new ArrayList<E>();
         }
       }
-      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<E> extends AbstractBeanCollection<E> implements List
   }
 
   public boolean isEmpty() {
-    initAsUntouched();
+    init();
     return list.isEmpty();
   }
 

src/main/java/com/avaje/ebean/common/BeanMap.java

@@ -45,11 +45,10 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> 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<K, E> extends AbstractBeanCollection<E> implements Ma
           map = new LinkedHashMap<K, E>();
         }
       }
-      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<K, E> extends AbstractBeanCollection<E> implements Ma
   }
 
   public boolean isEmpty() {
-    initAsUntouched();
+    init();
     return map.isEmpty();
   }
 

src/main/java/com/avaje/ebean/common/BeanSet.java

@@ -46,11 +46,10 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
     this.ownerBean = ownerBean;
     this.propertyName = propertyName;
     this.set = null;
-    this.touched = false;
   }
 
-  public boolean isEmptyAndUntouched() {
-    return !touched && (set == null || set.isEmpty());
+  public boolean isSkipSave() {
+    return set == null || (set.isEmpty() && !holdsModifications());
   }
 
   @SuppressWarnings("unchecked")
@@ -117,24 +116,14 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
           set = new LinkedHashSet<E>();
         }
       }
-      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<E> extends AbstractBeanCollection<E> implements Set<E
   }
 
   public boolean isEmpty() {
-    initAsUntouched();
+    init();
     return set.isEmpty();
   }
 

src/main/java/com/avaje/ebean/common/ModifyHolder.java

@@ -71,4 +71,11 @@ class ModifyHolder<E> implements Serializable {
   Set<E> getModifyRemovals() {
     return modifyDeletions;
   }
+
+  /**
+   * Return true if there additions or removals.
+   */
+  boolean hasModifications() {
+    return !modifyDeletions.isEmpty() || !modifyAdditions.isEmpty();
+  }
 }

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

@@ -238,13 +238,13 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> {
   /**
    * 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

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]);