Fix for #3381 - Empty OneToMany when overflowing LazyLoadBatchSize an…

…d mixed queries 14.0.0 via #3295 introduced this bug. #3295 introduced a behaviour where bulk updates clear the persistence context. Now the lazy loading of a BeanCollection works with an assumption that the "parent bean" is in the persistence context (and this assumption is broken with that change for this test case). That is, the bulk update is clearing the persistence context - removing the parent bean BEFORE the lazy loading is invoked ... and that doesn't then work because the parent bean isn't in the persistence context. This fix means that when lazy loading many's, the parent beans are putIfAbsent into the persistence context.
ebean-orm · Apr 4, 2024 · 1d413b0 · 1d413b0
1 parent ab45ad3
commit 1d413b0
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 27 deletions.
diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/LoadManyRequest.java b/ebean-core/src/main/java/io/ebeaninternal/api/LoadManyRequest.java
@@ -3,6 +3,7 @@
 import io.ebean.CacheMode;
 import io.ebean.bean.BeanCollection;
 import io.ebean.bean.EntityBean;
+import io.ebean.bean.PersistenceContext;
 import io.ebeaninternal.server.core.BindPadding;
 import io.ebeaninternal.server.core.OrmQueryRequest;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
@@ -57,25 +58,30 @@ public String description() {
     return loadContext.fullPath();
   }
 
-  private List<Object> parentIdList(SpiEbeanServer server) {
-    List<Object> idList = new ArrayList<>();
-    BeanPropertyAssocMany<?> many = many();
+  private List<Object> parentIdList(SpiEbeanServer server, BeanPropertyAssocMany<?> many, PersistenceContext pc) {
+    final var idList = new ArrayList<>(loadContext.size());
+    final var descriptor = many.descriptor();
     for (int i = 0; i < loadContext.size(); i++) {
-      BeanCollection<?> bc = loadContext.get(i);
+      final BeanCollection<?> bc = loadContext.get(i);
       if (bc != null) {
-        if (lazy && !originIncluded && bc == originCollection) {
-          originIncluded = true;
-        }
-        idList.add(many.parentId(bc.owner()));
+        final var parent = bc.owner();
+        final var parentId = descriptor.getId(parent);
+        idList.add(parentId);
         bc.setLoader(server); // don't use the load buffer again
+        if (lazy) {
+          descriptor.contextPutIfAbsent(pc, parentId, parent);
+          if (!originIncluded && bc == originCollection) {
+            originIncluded = true;
+          }
+        }
       }
     }
     if (originCollection != null && !originIncluded) {
       CoreLog.log.log(INFO, "Batch lazy loading including origin collection - size:{0}", idList.size());
       idList.add(many.parentId(originCollection.owner()));
       originCollection.setLoader(server); // don't use the load buffer again
     }
-    if (many.targetDescriptor().isPadInExpression()) {
+    if (descriptor.isPadInExpression()) {
       BindPadding.padIds(idList);
     }
     return idList;
@@ -99,8 +105,9 @@ public SpiQuery<?> createQuery(SpiEbeanServer server) {
       query.where().raw(extraWhere.replace("${ta}", "t0").replace("${mta}", "int_"));
     }
     query.setLazyLoadForParents(many);
-    many.addWhereParentIdIn(query, parentIdList(server), loadContext.isUseDocStore());
-    query.setPersistenceContext(loadContext.persistenceContext());
+    final var pc = loadContext.persistenceContext();
+    many.addWhereParentIdIn(query, parentIdList(server, many, pc), loadContext.isUseDocStore());
+    query.setPersistenceContext(pc);
     query.setLoadDescription(lazy ? "lazy" : "query", description());
     if (lazy) {
       query.setLazyLoadBatchSize(loadContext.batchSize());

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java
@@ -63,8 +63,8 @@ private void loadManyInternal(EntityBean parentBean, String propertyName, boolea
     Object parentId = parentDesc.getId(parentBean);
     if (pc == null) {
       pc = new DefaultPersistenceContext();
-      parentDesc.contextPut(pc, parentId, parentBean);
     }
+    parentDesc.contextPutIfAbsent(pc, parentId, parentBean);
     boolean useManyIdCache = beanCollection != null && parentDesc.isManyPropCaching() && many.isUseCache();
     if (useManyIdCache) {
       Boolean readOnly = null;

diff --git a/ebean-test/src/test/java/org/tests/cascade/TestLazyLoadMany.java b/ebean-test/src/test/java/org/tests/cascade/TestLazyLoadMany.java
@@ -0,0 +1,53 @@
+package org.tests.cascade;
+
+import io.ebean.DB;
+import io.ebean.Transaction;
+import io.ebean.xtest.BaseTestCase;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class TestLazyLoadMany extends BaseTestCase {
+
+  COOne create(String parentName, int childCount) {
+    COOne m0 = new COOne(parentName);
+    for (int i = 0; i < childCount; i++) {
+      m0.getChildren().add(new COOneMany(parentName+"_"+i));
+    }
+    return m0;
+  }
+  @Test
+  void loadAfterPCCleared() {
+
+    var m0 = create("tll-m0", 2);
+    var m1 = create("tll-m1", 4);
+    var m2 = create("tll-m2", 5);
+
+    DB.saveAll(m0, m1, m2);
+
+    try (Transaction transaction = DB.beginTransaction()) {
+      List<COOne> children = DB.find(COOne.class)
+        .setLazyLoadBatchSize(2)
+        .where().startsWith("name", "tll-m")
+        .findList();
+
+      assertThat(children).hasSize(3);
+      assertThat(children.get(0).getChildren()).describedAs("invoke lazy loading on children").hasSize(2);
+
+      DB.sqlUpdate("update coone set name = ? where id = ?")
+        .setParameters("new name", children.get(0).getId())
+        .executeNow(); // clears the persistence context in 14.0.0+ due to #3295 #3301
+
+      // the children here were already lazy loaded
+      assertThat(children.get(1).getChildren()).hasSize(4);
+      // this invokes the lazy loading of children AFTER the persistence context was cleared
+      assertThat(children.get(2).getChildren()).hasSize(5);
+
+      transaction.rollback();
+    } finally {
+      DB.deleteAll(List.of(m0, m1, m2));
+    }
+  }
+}
diff --git a/ebean-test/src/test/java/org/tests/model/elementcollection/TestElementCollectionCascade.java b/ebean-test/src/test/java/org/tests/model/elementcollection/TestElementCollectionCascade.java
@@ -46,7 +46,7 @@ public void test() {
     assertThat(sql.get(0)).contains("select t0.id, t0.name, t0.version from ecsm_parent t0 where t0.id = ?");
     if (!isPostgresCompatible()) {
       assertThat(sql.get(1)).contains("select t0.ecsm_parent_id, t0.one_id, t0.name, t0.version from ecsm_child t0 where (t0.ecsm_parent_id) in (?)");
-      assertThat(sql.get(2)).contains("select t0.host_id, t0.value from ecsm_values t0 where (t0.host_id) in (?,?)");
+      assertThat(sql.get(2)).contains("select t0.host_id, t0.value from ecsm_values t0 where (t0.host_id) in (?,?,?,?,?)");
     }
 
     Set<String> vals0 = childVals.get("c0");

diff --git a/ebean-test/src/test/java/org/tests/query/TestQueryFindIterate.java b/ebean-test/src/test/java/org/tests/query/TestQueryFindIterate.java
@@ -172,9 +172,9 @@ public void testWithTwoJoins() {
 
     List<String> loggedSql = LoggedSql.stop();
 
-    assertEquals(2, loggedSql.size());
-    assertThat(trimSql(loggedSql.get(0), 7).contains("select t0.id, t0.status, t0.order_date, t1.id, t1.name, t2.id, t2.order_qty, t2.ship_qty"));
-    assertThat(trimSql(loggedSql.get(1), 7).contains("select t0.order_id, t0.id, t0.ship_time, t0.cretime, t0.updtime, t0.version, t0.order_id from or_order_ship"));
+    assertThat(loggedSql).hasSize(2);
+    assertThat(trimSql(loggedSql.get(0), 7)).contains("select t0.id, t0.status, t0.order_date, t1.id, t1.name, t2.id, t2.order_qty, t2.ship_qty");
+    assertThat(trimSql(loggedSql.get(1), 7)).contains("select t0.order_id, t0.id, t0.ship_time, t0.cretime, t0.updtime, t0.version, t0.order_id from or_order_ship");
   }
 
   @ForPlatform(Platform.H2)

diff --git a/ebean-test/src/test/java/org/tests/query/TestRefToLazyLoadMany.java b/ebean-test/src/test/java/org/tests/query/TestRefToLazyLoadMany.java
@@ -1,14 +1,17 @@
 package org.tests.query;
 
+import io.ebean.Transaction;
 import io.ebean.xtest.BaseTestCase;
 import io.ebean.DB;
+import io.ebeaninternal.api.SpiTransaction;
 import org.junit.jupiter.api.Test;
 import org.tests.model.basic.Contact;
 import org.tests.model.basic.Customer;
 import org.tests.model.basic.ResetBasicData;
 
 import java.util.List;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class TestRefToLazyLoadMany extends BaseTestCase {
@@ -26,17 +29,23 @@ public void test() {
     assertEquals(3, DB.beanState(c).loadedProps().size());
 
     // now lazy load the contacts
-    contacts2.size();
+    int expectedSize = contacts2.size();
 
-    Customer c2 = DB.reference(Customer.class, c.getId());
+    try (Transaction transaction = DB.beginTransaction()) {
+      Customer c2 = DB.reference(Customer.class, c.getId());
 
-    // we only "loaded" the contacts BeanList and not all of c2
-    List<Contact> contacts = c2.getContacts();
-    // Set<String> loadedProps = DB.beanState(c2).getLoadedProps();
-    // assertEquals(1, loadedProps.size());
+      SpiTransaction spiTxn = (SpiTransaction)transaction;
+      spiTxn.persistenceContext().clear();
 
-    // now lazy load the contacts
-    contacts.size();
+      // we only "loaded" the contacts BeanList and not all of c2
+      List<Contact> contacts = c2.getContacts();
+      // Set<String> loadedProps = DB.beanState(c2).getLoadedProps();
+      // assertEquals(1, loadedProps.size());
+
+      // now lazy load the contacts
+      int lazyLoadedSize = contacts.size();
 
+      assertThat(lazyLoadedSize).isEqualTo(expectedSize);
+    }
   }
 }
diff --git a/ebean-test/src/test/resources/logback-test.xml b/ebean-test/src/test/resources/logback-test.xml
@@ -82,9 +82,9 @@
 
 <!--  <logger name="io.ebean.DDL" level="DEBUG"/>-->
 
-<!--  <logger name="io.ebean.SQL" level="TRACE"/>-->
-<!--  <logger name="io.ebean.TXN" level="TRACE"/>-->
-<!--  <logger name="io.ebean.SUM" level="TRACE"/>-->
+  <logger name="io.ebean.SQL" level="TRACE"/>
+  <logger name="io.ebean.TXN" level="TRACE"/>
+  <logger name="io.ebean.SUM" level="TRACE"/>
 
 <!--  <logger name="io.ebean.cache" level="TRACE"/>-->
 <!--  <logger name="io.ebean.cache.REGION" level="TRACE"/>-->