Fix #23 - Covers: unit tests for: stateless updates which fails when executing without...

pom.xml

@@ -94,7 +94,7 @@
     <dependency>
       <groupId>org.avaje.ebeanorm</groupId>
       <artifactId>avaje-ebeanorm-agent</artifactId>
-      <version>4.0.1-RC2-SNAPSHOT</version>
+      <version>4.0.1-RC3-SNAPSHOT</version>
       <scope>test</scope>
     </dependency>
         
@@ -169,7 +169,7 @@
       <plugin>
         <groupId>org.avaje.ebeanorm</groupId>
         <artifactId>avaje-ebeanorm-mavenenhancer</artifactId>
-        <version>4.0.1-RC2-SNAPSHOT</version>
+        <version>4.0.1-RC3-SNAPSHOT</version>
         <executions>
            <execution>
             <id>main</id>

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

@@ -31,6 +31,12 @@ public interface BeanCollection<E> extends Serializable {
     ALL
   }
 
+  /**
+   * Return true if the collection is empty and untouched. Used to detect if a
+   * collection was 'cleared' deliberately or just un-initialised.
+   */
+  public boolean isEmptyAndUntouched();
+
   /**
    * Return the bean that owns this collection.
    */

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

@@ -744,6 +744,13 @@ public final class EntityBeanIntercept implements Serializable {
     return obj1.equals(obj2);
   }
   
+  /**
+   * Called when a BeanCollection is initialised automatically.
+   */
+  public void initialisedMany(int propertyIndex) {
+    loadedProps[propertyIndex] = true;
+  }
+  
   /**
    * Method that is called prior to a getter method on the actual entity.
    */

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

@@ -58,6 +58,12 @@ 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.
    */
@@ -112,7 +118,14 @@ public abstract class AbstractBeanCollection<E> implements BeanCollection<E> {
     checkEmptyLazyLoad();
   }
 
-  protected void touched() {
+  /**
+   * 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;
+    }
     if (beanCollectionTouched != null) {
       // only call this once
       beanCollectionTouched.notifyTouched(this);

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

@@ -46,6 +46,11 @@ public final class BeanList<E> extends AbstractBeanCollection<E> implements List
     super(loader, ownerBean, propertyName);
   }
 
+  @Override
+  public boolean isEmptyAndUntouched() {
+    return !touched && (list == null || list.isEmpty());
+  }
+
   @SuppressWarnings("unchecked")
   public void addBean(EntityBean bean) {
     list.add((E) bean);
@@ -77,16 +82,24 @@ public final class BeanList<E> extends AbstractBeanCollection<E> implements List
           list = new ArrayList<E>();
         }
       }
-      touched();
+      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();
+      touched(setTouched);
     }
   }
 
@@ -259,7 +272,7 @@ public final class BeanList<E> extends AbstractBeanCollection<E> implements List
   }
 
   public boolean isEmpty() {
-    init();
+    initAsUntouched();
     return list.isEmpty();
   }
 

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

@@ -39,6 +39,10 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> implements Ma
   public BeanMap(BeanCollectionLoader ebeanServer, EntityBean ownerBean, String propertyName) {
     super(ebeanServer, ownerBean, propertyName);
   }
+  
+  public boolean isEmptyAndUntouched() {
+    return !touched && (map == null || map.isEmpty());
+  }
 
   @SuppressWarnings("unchecked")
   public void internalPut(Object key, Object bean) {
@@ -86,16 +90,24 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> implements Ma
           map = new LinkedHashMap<K, E>();
         }
       }
-      touched();
+      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();
+      touched(setTouched);
     }
   }
 
@@ -212,7 +224,7 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> implements Ma
   }
 
   public boolean isEmpty() {
-    init();
+    initAsUntouched();
     return map.isEmpty();
   }
 

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

@@ -40,6 +40,10 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
     super(loader, ownerBean, propertyName);
   }
 
+  public boolean isEmptyAndUntouched() {
+    return !touched && (set == null || set.isEmpty());
+  }
+
   @SuppressWarnings("unchecked")
   public void addBean(EntityBean bean) {
     set.add((E) bean);
@@ -86,16 +90,24 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
           set = new LinkedHashSet<E>();
         }
       }
-      touched();
+      touched(true);
     }
   }
 
+  private void initWithoutTouchedFlag() {
+    init(false);
+  }
+  
   private void init() {
+    init(true);
+  }
+  
+  private void init(boolean setFlag) {
     synchronized (this) {
       if (set == null) {
         lazyLoadCollection(true);
       }
-      touched();
+      touched(setFlag);
     }
   }
 
@@ -220,7 +232,7 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
   }
 
   public boolean isEmpty() {
-    init();
+    initWithoutTouchedFlag();
     return set.isEmpty();
   }
 

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

@@ -158,6 +158,18 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> {
         help.add(bc, detailBean);
     }
 
+    public boolean isEmptyBeanCollection(EntityBean bean) {
+      Object val = getValue(bean);
+      if (val == null) {
+        return true;
+      }
+      if (val instanceof BeanCollection<?>) {
+        // if empty and not been cleared or elements removed
+        return ((BeanCollection<?>)val).isEmptyAndUntouched();
+      }
+      return false;
+    }
+    
     @Override
     public Object getValue(EntityBean bean) {
         return super.getValue(bean);

src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java

@@ -565,7 +565,10 @@ public final class DefaultPersister implements Persister {
 		// many's with cascade save
 		BeanPropertyAssocMany<?>[] manys = desc.propertiesManySave();
 		for (int i = 0; i < manys.length; i++) {
-			saveMany(new SaveManyPropRequest(insertedParent, manys[i], parentBean, request));
+		  // check that property is loaded and not empty uninitialised collection
+      if (request.isLoadedProperty(manys[i]) && !manys[i].isEmptyBeanCollection(parentBean)) {
+        saveMany(new SaveManyPropRequest(insertedParent, manys[i], parentBean, request));
+      }
 		}
 	}
 
@@ -605,7 +608,7 @@ public final class DefaultPersister implements Persister {
 		public boolean isSaveIntersection() {
 		  return t.isSaveAssocManyIntersection(many.getIntersectionTableJoin().getTable(), many.getBeanDescriptor().getName());
 		}
-
+		
 		private Object getValue() {
 			return many.getValue(parentBean);
 		}

src/test/java/com/avaje/ebeaninternal/server/deploy/TestCollectionLoadedStatus.java

@@ -0,0 +1,42 @@
+package com.avaje.ebeaninternal.server.deploy;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.avaje.ebean.BaseTestCase;
+import com.avaje.ebean.Ebean;
+import com.avaje.ebean.bean.BeanCollection;
+import com.avaje.ebean.bean.EntityBean;
+import com.avaje.ebean.bean.EntityBeanIntercept;
+import com.avaje.ebeaninternal.api.SpiEbeanServer;
+import com.avaje.tests.model.basic.Contact;
+import com.avaje.tests.model.basic.Customer;
+
+public class TestCollectionLoadedStatus extends BaseTestCase {
+
+  @Test
+  public void test() {
+    
+    SpiEbeanServer server = (SpiEbeanServer)Ebean.getServer(null);
+    BeanDescriptor<Customer> custDesc = server.getBeanDescriptor(Customer.class);
+    
+    Customer customer = new Customer();
+    EntityBean eb = (EntityBean)customer;
+    EntityBeanIntercept ebi = eb._ebean_getIntercept();
+    
+    BeanProperty contactsProperty = custDesc.getBeanProperty("contacts");
+    Assert.assertFalse(ebi.isLoadedProperty(contactsProperty.getPropertyIndex()));
+    
+    Object contactsViaInternal = contactsProperty.getValue(eb);
+    Assert.assertNull(contactsViaInternal);
+    Assert.assertFalse(ebi.isLoadedProperty(contactsProperty.getPropertyIndex()));
+    
+    List<Contact> contacts = customer.getContacts();
+    Assert.assertNotNull(contacts);
+    Assert.assertTrue(contacts instanceof BeanCollection);
+    Assert.assertTrue(ebi.isLoadedProperty(contactsProperty.getPropertyIndex()));
+  }
+  
+}

src/test/java/com/avaje/tests/model/basic/event/CustomerPersistAdapter.java

@@ -23,12 +23,8 @@ public class CustomerPersistAdapter extends BeanPersistAdapter {
   @Override
   public boolean preUpdate(BeanPersistRequest<?> request) {
 
-    Customer customer = (Customer) request.getBean();
-
-    if (customer.getContacts() != null) {
-      String test = "accessed";
-    }
-
+    // Do nothing intentionally. TestStatelessUpdate needs
+    // to control if customer contacts is 'touched'
     return true;
   }
 

src/test/java/com/avaje/tests/update/TestStatelessUpdate.java

@@ -1,7 +1,8 @@
 package com.avaje.tests.update;
 
-import com.avaje.tests.model.basic.Contact;
-import com.avaje.tests.model.basic.Customer;
+import java.util.ArrayList;
+import java.util.Collections;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -9,11 +10,11 @@ import org.junit.Test;
 import com.avaje.ebean.BaseTestCase;
 import com.avaje.ebean.Ebean;
 import com.avaje.ebean.EbeanServer;
+import com.avaje.tests.model.basic.Contact;
+import com.avaje.tests.model.basic.Customer;
 import com.avaje.tests.model.basic.EBasic;
 import com.avaje.tests.model.basic.EBasic.Status;
 
-import java.util.ArrayList;
-
 public class TestStatelessUpdate extends BaseTestCase {
 
   private EbeanServer server;
@@ -105,10 +106,10 @@ public class TestStatelessUpdate extends BaseTestCase {
   }
 
   /**
-   * Many relations mustn't be deleted when having a {@link com.avaje.ebean.event.BeanPersistAdapter} which is accessing this many field.
+   * Many relations mustn't be deleted when they are not loaded.
    */
   @Test
-  public void testStatelessUpdateWithPersistAdapterAndIgnoreNullValues() {
+  public void testStatelessUpdateIgnoreNullCollection() {
 
     // arrange
     Contact contact = new Contact();
@@ -126,12 +127,89 @@ public class TestStatelessUpdate extends BaseTestCase {
     customerWithChange.setId(customer.getId());
     customerWithChange.setName("new name");
 
-    server.update(customerWithChange, null, null, true, false);
+    // contacts is not loaded
+    Assert.assertFalse(containsContacts(customerWithChange));
+    server.update(customerWithChange);
 
     Customer result = Ebean.find(Customer.class, customer.getId());
 
-    // assert
+    // assert null list was ignored (missing children not deleted)
     Assert.assertNotNull(result.getContacts());
     Assert.assertFalse("the contacts mustn't be deleted", result.getContacts().isEmpty());
   }
+  
+  /**
+   * When BeanCollection is inadvertantly initialised and empty then ignore it
+   * Specifically a non-BeanCollection (like ArrayList) is not ignored in terms
+   * of deleting missing children.
+   */
+  @Test
+  public void testStatelessUpdateIgnoreEmptyBeanCollection() {
+
+    // arrange
+    Contact contact = new Contact();
+    contact.setFirstName("wobu :P");
+
+    Customer customer = new Customer();
+    customer.setName("something");
+    customer.setContacts(new ArrayList<Contact>());
+    customer.getContacts().add(contact);
+
+    server.save(customer);
+
+    // act
+    Customer customerWithChange = new Customer();
+    customerWithChange.setId(customer.getId());
+    customerWithChange.setName("new name");
+
+    // with Ebean enhancement this loads the an empty contacts BeanList
+    customerWithChange.getContacts();
+    
+    // contacts has been initialised to empty BeanList
+    Assert.assertTrue(containsContacts(customerWithChange));
+    server.update(customerWithChange);
+
+    Customer result = Ebean.find(Customer.class, customer.getId());
+
+    // assert empty bean list was ignore (missing children not deleted)
+    Assert.assertNotNull(result.getContacts());
+    Assert.assertFalse("the contacts mustn't be deleted", result.getContacts().isEmpty());
+  }
+  
+  @Test
+  public void testStatelessUpdateDeleteChildrenForNonBeanCollection() {
+
+    // arrange
+    Contact contact = new Contact();
+    contact.setFirstName("wobu :P");
+
+    Customer customer = new Customer();
+    customer.setName("something");
+    customer.setContacts(new ArrayList<Contact>());
+    customer.getContacts().add(contact);
+
+    server.save(customer);
+
+    // act
+    Customer customerWithChange = new Customer();
+    customerWithChange.setId(customer.getId());
+    customerWithChange.setName("new name");
+
+    // with Ebean enhancement this loads the an empty contacts BeanList
+    customerWithChange.setContacts(Collections.<Contact> emptyList());
+    
+    Assert.assertTrue(containsContacts(customerWithChange));
+    server.update(customerWithChange);
+
+    Customer result = Ebean.find(Customer.class, customer.getId());
+
+    // assert empty bean list was ignore (missing children not deleted)
+    Assert.assertNotNull(result.getContacts());
+    Assert.assertTrue("the contacts were deleted", result.getContacts().isEmpty());
+  }
+  
+  private boolean containsContacts(Customer cust) {
+    return server.getBeanState(cust).getLoadedProps().contains("contacts");
+  }
+  
 }