From f6b441879c90825fde7f6b1f3ae11a0f24310c1d Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Fri, 27 Jan 2023 10:31:53 +0100 Subject: [PATCH] NEW: Support for lazy add on BeanList (#81) When just adding new entries to an existing BeanList, there is no need to load the underlying collection FIX: Issue when batch load occurs with lazily added elements --- .../java/io/ebean/bean/BeanCollection.java | 7 + .../main/java/io/ebean/common/BeanList.java | 26 +-- .../java/io/ebean/common/BeanListLazyAdd.java | 129 +++++++++++++ .../server/deploy/BaseCollectionHelp.java | 8 +- .../server/deploy/BeanCollectionUtil.java | 2 +- .../server/deploy/BeanListHelp.java | 22 ++- .../server/deploy/BeanMapHelp.java | 7 +- .../server/deploy/BeanSetHelp.java | 7 - .../tests/batchload/TestLazyAddBeanList.java | 180 ++++++++++++++++++ 9 files changed, 348 insertions(+), 40 deletions(-) create mode 100644 ebean-api/src/main/java/io/ebean/common/BeanListLazyAdd.java create mode 100644 ebean-test/src/test/java/org/tests/batchload/TestLazyAddBeanList.java diff --git a/ebean-api/src/main/java/io/ebean/bean/BeanCollection.java b/ebean-api/src/main/java/io/ebean/bean/BeanCollection.java index 1161476ddd..c25d0e9d87 100644 --- a/ebean-api/src/main/java/io/ebean/bean/BeanCollection.java +++ b/ebean-api/src/main/java/io/ebean/bean/BeanCollection.java @@ -164,6 +164,13 @@ enum ModifyListenMode { */ Collection actualEntries(); + /** + * Returns entries, that were lazily added at the end of the list. Might be null. + */ + default Collection getLazyAddedEntries() { + return null; + } + /** * return true if there are real rows held. Return false is this is using * Deferred fetch to lazy load the rows and the rows have not yet been diff --git a/ebean-api/src/main/java/io/ebean/common/BeanList.java b/ebean-api/src/main/java/io/ebean/common/BeanList.java index 12d0770d62..a1ecb1d4ec 100644 --- a/ebean-api/src/main/java/io/ebean/common/BeanList.java +++ b/ebean-api/src/main/java/io/ebean/common/BeanList.java @@ -8,14 +8,14 @@ /** * List capable of lazy loading and modification awareness. */ -public final class BeanList extends AbstractBeanCollection implements List, BeanCollectionAdd { +public class BeanList extends AbstractBeanCollection implements List, BeanCollectionAdd { private static final long serialVersionUID = 1L; /** * The underlying List implementation. */ - private List list; + List list; /** * Specify the underlying List implementation. @@ -111,30 +111,30 @@ public boolean checkEmptyLazyLoad() { } } + protected void initList(boolean skipLoad, boolean onlyIds) { + if (skipLoad) { + list = new ArrayList<>(); + } else { + lazyLoadCollection(onlyIds); + } + } + private void initClear() { lock.lock(); try { if (list == null) { - if (!disableLazyLoad && modifyListening) { - lazyLoadCollection(true); - } else { - list = new ArrayList<>(); - } + initList(disableLazyLoad || !modifyListening, true); } } finally { lock.unlock(); } } - private void init() { + void init() { lock.lock(); try { if (list == null) { - if (disableLazyLoad) { - list = new ArrayList<>(); - } else { - lazyLoadCollection(false); - } + initList(disableLazyLoad, false); } } finally { lock.unlock(); diff --git a/ebean-api/src/main/java/io/ebean/common/BeanListLazyAdd.java b/ebean-api/src/main/java/io/ebean/common/BeanListLazyAdd.java new file mode 100644 index 0000000000..6323a2c866 --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/common/BeanListLazyAdd.java @@ -0,0 +1,129 @@ +package io.ebean.common; + +import io.ebean.bean.BeanCollection; +import io.ebean.bean.BeanCollectionLoader; +import io.ebean.bean.EntityBean; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +/** + * This bean list can perform additions without populating the list. + * This might be useful, if you just want to add entries to an existing collection. + * Works only for lists and only if there is no order column + */ +public class BeanListLazyAdd extends BeanList { + + public BeanListLazyAdd(BeanCollectionLoader loader, EntityBean ownerBean, String propertyName) { + super(loader, ownerBean, propertyName); + } + + private List lazyAddedEntries; + + @Override + public boolean add(E bean) { + checkReadOnly(); + + lock.lock(); + try { + if (list == null) { + // list is not yet initialized, so we may add elements to a spare list + if (lazyAddedEntries == null) { + lazyAddedEntries = new ArrayList<>(); + } + lazyAddedEntries.add(bean); + } else { + list.add(bean); + } + } finally { + lock.unlock(); + } + + if (modifyListening) { + modifyAddition(bean); + } + return true; + } + + @Override + public boolean addAll(Collection beans) { + checkReadOnly(); + + lock.lock(); + try { + if (list == null) { + // list is not yet initialized, so we may add elements to a spare list + if (lazyAddedEntries == null) { + lazyAddedEntries = new ArrayList<>(); + } + lazyAddedEntries.addAll(beans); + } else { + list.addAll(beans); + } + } finally { + lock.unlock(); + } + + if (modifyListening) { + getModifyHolder().modifyAdditionAll(beans); + } + + return true; + } + + + @Override + public void loadFrom(BeanCollection other) { + super.loadFrom(other); + if (lazyAddedEntries != null) { + list.addAll(lazyAddedEntries); + lazyAddedEntries = null; + } + } + + /** + * on init, this happens on all accessor methods except on 'add' and addAll, + * we add the lazy added entries at the end of the list + */ + @Override + protected void initList(boolean skipLoad, boolean onlyIds) { + if (skipLoad) { + if (lazyAddedEntries != null) { + list = lazyAddedEntries; + lazyAddedEntries = null; + } else { + list = new ArrayList<>(); + } + } else { + lazyLoadCollection(onlyIds); + if (lazyAddedEntries != null) { + list.addAll(lazyAddedEntries); + lazyAddedEntries = null; + } + } + } + + @Override + public List getLazyAddedEntries() { + return lazyAddedEntries; + } + + @Override + public boolean isSkipSave() { + return lazyAddedEntries == null && super.isSkipSave(); + } + + public boolean checkEmptyLazyLoad() { + if (list != null) { + return false; + } else if (lazyAddedEntries == null) { + list = new ArrayList<>(); + return true; + } else { + list = lazyAddedEntries; + lazyAddedEntries = null; + return false; + } + } +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BaseCollectionHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BaseCollectionHelp.java index b6cb6314ba..f161f18230 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BaseCollectionHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BaseCollectionHelp.java @@ -11,7 +11,7 @@ abstract class BaseCollectionHelp implements BeanCollectionHelp { final BeanPropertyAssocMany many; - private final BeanDescriptor targetDescriptor; + final BeanDescriptor targetDescriptor; final String propertyName; BeanCollectionLoader loader; @@ -21,12 +21,6 @@ abstract class BaseCollectionHelp implements BeanCollectionHelp { this.propertyName = many.name(); } - BaseCollectionHelp() { - this.many = null; - this.targetDescriptor = null; - this.propertyName = null; - } - @Override public final void setLoader(BeanCollectionLoader loader) { this.loader = loader; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanCollectionUtil.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanCollectionUtil.java index 6532df7f15..65a8f1fb78 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanCollectionUtil.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanCollectionUtil.java @@ -32,7 +32,7 @@ public static Collection getActualEntries(Object o) { if (o instanceof BeanCollection) { BeanCollection bc = (BeanCollection) o; if (!bc.isPopulated()) { - return null; + return bc.getLazyAddedEntries(); } // For maps this is a collection of Map.Entry, otherwise it // returns a collection of beans diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanListHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanListHelp.java index c2ddfeca73..5e228bb9f9 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanListHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanListHelp.java @@ -6,6 +6,7 @@ import io.ebean.bean.BeanCollectionAdd; import io.ebean.bean.EntityBean; import io.ebean.common.BeanList; +import io.ebean.common.BeanListLazyAdd; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.api.json.SpiJsonWriter; @@ -19,12 +20,11 @@ */ public class BeanListHelp extends BaseCollectionHelp { + private final boolean hasOrderColumn; + BeanListHelp(BeanPropertyAssocMany many) { super(many); - } - - BeanListHelp() { - super(); + hasOrderColumn = many.hasOrderColumn(); } @Override @@ -53,7 +53,12 @@ public final BeanCollection createEmptyNoParent() { @Override public final BeanCollection createEmpty(EntityBean parentBean) { - BeanList beanList = new BeanList<>(loader, parentBean, propertyName); + BeanList beanList; + if (hasOrderColumn) { + beanList = new BeanList<>(loader, parentBean, propertyName); + } else { + beanList = new BeanListLazyAdd<>(loader, parentBean, propertyName); + } if (many != null) { beanList.setModifyListening(many.modifyListenMode()); } @@ -62,7 +67,12 @@ public final BeanCollection createEmpty(EntityBean parentBean) { @Override public final BeanCollection createReference(EntityBean parentBean) { - BeanList beanList = new BeanList<>(loader, parentBean, propertyName); + BeanList beanList; + if (hasOrderColumn) { + beanList = new BeanList<>(loader, parentBean, propertyName); + } else { + beanList = new BeanListLazyAdd<>(loader, parentBean, propertyName); + } beanList.setModifyListening(many.modifyListenMode()); return beanList; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanMapHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanMapHelp.java index 6ce968ca2b..7652a1b214 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanMapHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanMapHelp.java @@ -20,18 +20,13 @@ */ public class BeanMapHelp extends BaseCollectionHelp { - private final BeanPropertyAssocMany many; - private final BeanDescriptor targetDescriptor; - private final String propertyName; private final BeanProperty beanProperty; /** * When help is attached to a specific many property. */ BeanMapHelp(BeanPropertyAssocMany many) { - this.many = many; - this.targetDescriptor = many.targetDescriptor(); - this.propertyName = many.name(); + super(many); this.beanProperty = targetDescriptor.beanProperty(many.mapKey()); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanSetHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanSetHelp.java index ec2f6e31e1..4115b6ac4d 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanSetHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanSetHelp.java @@ -26,13 +26,6 @@ public class BeanSetHelp extends BaseCollectionHelp { super(many); } - /** - * For a query that returns a set. - */ - BeanSetHelp() { - super(); - } - @Override public final BeanCollectionAdd getBeanCollectionAdd(Object bc, String mapKey) { if (bc instanceof BeanSet) { diff --git a/ebean-test/src/test/java/org/tests/batchload/TestLazyAddBeanList.java b/ebean-test/src/test/java/org/tests/batchload/TestLazyAddBeanList.java new file mode 100644 index 0000000000..b4b4748732 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/batchload/TestLazyAddBeanList.java @@ -0,0 +1,180 @@ +package org.tests.batchload; + +import io.ebean.DB; +import io.ebean.ExpressionList; +import io.ebean.Transaction; +import io.ebean.common.BeanList; +import io.ebean.common.BeanListLazyAdd; +import io.ebean.test.LoggedSql; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.tests.model.basic.Contact; +import org.tests.model.basic.Customer; +import org.tests.order.OrderMaster; +import org.tests.order.OrderReferencedChild; + +import java.util.Arrays; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestLazyAddBeanList extends BaseTestCase { + + private Customer cust; + private OrderMaster orderMaster; + + @BeforeEach + void init() { + cust = new Customer(); + cust.setName("noFetch"); + DB.save(cust); + cust = DB.find(Customer.class, cust.getId()); // fetch fresh from DB + + orderMaster = new OrderMaster(); + DB.save(orderMaster); + orderMaster = DB.find(OrderMaster.class, orderMaster.getId()); + } + + @AfterEach + void tearDown() { + DB.delete(cust); + DB.delete(orderMaster); + } + + @Test + public void testCollectionType() { + assertThat(cust.getContacts()) + .isInstanceOf(BeanListLazyAdd.class); + + assertThat(orderMaster.getChildren()) + .isInstanceOf(BeanList.class) + .isNotInstanceOf(BeanListLazyAdd.class); + + assertThat(DB.reference(Customer.class, 1).getContacts()) + .isInstanceOf(BeanListLazyAdd.class); + + assertThat(DB.reference(OrderMaster.class, 1).getChildren()) + .isInstanceOf(BeanList.class) + .isNotInstanceOf(BeanListLazyAdd.class); + } + + @Test + public void testNoFetch() { + + LoggedSql.start(); + cust.addContact(new Contact("jim", "slim")); + cust.addContact(new Contact("joe", "big")); + DB.save(cust); + + List sql = LoggedSql.stop(); + assertThat(sql.get(0)).contains("insert into contact"); + assertThat(sql.get(1)).contains("bind(jim,slim,"); + assertThat(sql.get(2)).contains("bind(joe,big,"); + assertThat(sql).hasSize(3); + + List list = DB.find(Customer.class) + .fetch("contacts", "firstName") + .where() + .idEq(cust.getId()).findSingleAttributeList(); +// check if it is really saved + assertThat(list).containsExactlyInAnyOrder("jim", "joe"); + } + + @Test + public void testFetch() { + + LoggedSql.start(); + orderMaster.getChildren().add(new OrderReferencedChild("foo")); + orderMaster.getChildren().add(new OrderReferencedChild("bar")); + DB.save(orderMaster); + + List sql = LoggedSql.stop(); + + assertThat(sql.get(0)).contains("from order_referenced_parent"); // lazy load children + assertThat(sql.get(1)).contains("insert into order_referenced_parent"); + assertThat(sql.get(2)).contains("bind(D,foo,"); + assertThat(sql.get(3)).contains("bind(D,bar,"); + + List list = DB.find(OrderMaster.class) + .fetch("children", "name") + .where().idEq(orderMaster.getId()) + .findSingleAttributeList(); + // check if it is really saved + assertThat(list).containsExactlyInAnyOrder("foo", "bar"); + } + + @Test + public void testAddToExisting() { + + // add some existing entries + LoggedSql.start(); + cust.getContacts().addAll(Arrays.asList( + new Contact("jim", "slim"), + new Contact("joe", "big"))); + assertThat(LoggedSql.stop()).isEmpty(); + + LoggedSql.start(); + DB.save(cust); + assertThat(LoggedSql.stop()).hasSize(3); // insert + 2x bind + + cust = DB.find(Customer.class, cust.getId()); + + LoggedSql.start(); + List contacts = cust.getContacts(); + contacts.add(new Contact("charlie", "brown")); + assertThat(LoggedSql.stop()).isEmpty(); + + LoggedSql.start(); + assertThat(contacts). + extracting(Contact::getFirstName). + containsExactlyInAnyOrder("jim", "joe", "charlie"); + List sql = LoggedSql.stop(); + + assertThat(sql.get(0)).contains("from o_customer t0 left join contact t1 "); + assertThat(sql).hasSize(1); + } + + @Test + public void testBatch() { + for (int i = 0; i < 10; i++) { + Customer bcust = new Customer(); + bcust.setName("batch " + i); + //bcust.getContacts().add(new Contact("Noemi","Praml")); + DB.save(bcust); + } + + LoggedSql.start(); + List custs = DB.find(Customer.class).where().startsWith("name", "batch").findList(); + assertThat(custs).hasSize(10); + assertThat(LoggedSql.stop()).hasSize(1); + + + LoggedSql.start(); + for (Customer cust : custs) { + cust.getContacts().addAll(Arrays.asList( + new Contact(cust.getName() + " jim", "slim"), + new Contact(cust.getName() + " joe", "big"))); + } + assertThat(LoggedSql.stop()).isEmpty(); + + LoggedSql.start(); + custs.get(0).getContacts().size(); // trigger batch load + assertThat(LoggedSql.stop()).hasSize(1); + + LoggedSql.start(); + DB.saveAll(custs); + assertThat(LoggedSql.stop()).hasSize(21); + + LoggedSql.start(); + custs = DB.find(Customer.class).where().startsWith("name", "batch").findList(); + assertThat(custs).hasSize(10); + + for (Customer cust : custs) { + assertThat(cust.getContacts()).hasSize(2).extracting(Contact::getFirstName).containsExactlyInAnyOrder(cust.getName() + " jim", cust.getName() + " joe"); + } + assertThat(LoggedSql.stop()).hasSize(2); + } + +}