diff --git a/pom.xml b/pom.xml index 7ec0eb1ed..8c35d1482 100644 --- a/pom.xml +++ b/pom.xml @@ -94,7 +94,7 @@ org.avaje.ebeanorm avaje-ebeanorm-agent - 4.0.1-RC2-SNAPSHOT + 4.0.1-RC3-SNAPSHOT test @@ -169,7 +169,7 @@ org.avaje.ebeanorm avaje-ebeanorm-mavenenhancer - 4.0.1-RC2-SNAPSHOT + 4.0.1-RC3-SNAPSHOT main diff --git a/src/main/java/com/avaje/ebean/bean/BeanCollection.java b/src/main/java/com/avaje/ebean/bean/BeanCollection.java index a765d1111..ededeabc4 100644 --- a/src/main/java/com/avaje/ebean/bean/BeanCollection.java +++ b/src/main/java/com/avaje/ebean/bean/BeanCollection.java @@ -31,6 +31,12 @@ public interface BeanCollection 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. */ diff --git a/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java b/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java index b1e39d54e..a1bc4d89e 100644 --- a/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java +++ b/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. */ diff --git a/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java b/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java index f3a32087a..484909963 100644 --- a/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java +++ b/src/main/java/com/avaje/ebean/common/AbstractBeanCollection.java @@ -58,6 +58,12 @@ 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. */ @@ -112,7 +118,14 @@ public abstract class AbstractBeanCollection implements BeanCollection { 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); diff --git a/src/main/java/com/avaje/ebean/common/BeanList.java b/src/main/java/com/avaje/ebean/common/BeanList.java index 191026ad6..b70325e3e 100644 --- a/src/main/java/com/avaje/ebean/common/BeanList.java +++ b/src/main/java/com/avaje/ebean/common/BeanList.java @@ -46,6 +46,11 @@ public final class BeanList extends AbstractBeanCollection 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 extends AbstractBeanCollection implements List list = new ArrayList(); } } - 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 extends AbstractBeanCollection implements List } public boolean isEmpty() { - init(); + initAsUntouched(); 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 e92368c40..5c97d0991 100644 --- a/src/main/java/com/avaje/ebean/common/BeanMap.java +++ b/src/main/java/com/avaje/ebean/common/BeanMap.java @@ -39,6 +39,10 @@ public final class BeanMap extends AbstractBeanCollection 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 extends AbstractBeanCollection implements Ma map = new LinkedHashMap(); } } - 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 extends AbstractBeanCollection implements Ma } public boolean isEmpty() { - init(); + initAsUntouched(); 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 3d882ec85..e76b85b60 100644 --- a/src/main/java/com/avaje/ebean/common/BeanSet.java +++ b/src/main/java/com/avaje/ebean/common/BeanSet.java @@ -40,6 +40,10 @@ public final class BeanSet extends AbstractBeanCollection implements Set extends AbstractBeanCollection implements Set(); } } - 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 extends AbstractBeanCollection implements Set extends BeanPropertyAssoc { 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); 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 059fa25e7..67cc3a264 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/DefaultPersister.java +++ b/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); } diff --git a/src/test/java/com/avaje/ebeaninternal/server/deploy/TestCollectionLoadedStatus.java b/src/test/java/com/avaje/ebeaninternal/server/deploy/TestCollectionLoadedStatus.java new file mode 100644 index 000000000..dd2922574 --- /dev/null +++ b/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 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 contacts = customer.getContacts(); + Assert.assertNotNull(contacts); + Assert.assertTrue(contacts instanceof BeanCollection); + Assert.assertTrue(ebi.isLoadedProperty(contactsProperty.getPropertyIndex())); + } + +} diff --git a/src/test/java/com/avaje/tests/model/basic/event/CustomerPersistAdapter.java b/src/test/java/com/avaje/tests/model/basic/event/CustomerPersistAdapter.java index 5970ec6b1..0fa487234 100644 --- a/src/test/java/com/avaje/tests/model/basic/event/CustomerPersistAdapter.java +++ b/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; } diff --git a/src/test/java/com/avaje/tests/update/TestStatelessUpdate.java b/src/test/java/com/avaje/tests/update/TestStatelessUpdate.java index 9ddee8cce..12376dcd7 100644 --- a/src/test/java/com/avaje/tests/update/TestStatelessUpdate.java +++ b/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()); + 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()); + 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. 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"); + } + }