Merge pull request #3382 from ebean-orm/fix/3381-pc-clear

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

ebean-core/src/main/java/io/ebeaninternal/api/LoadManyRequest.java

@@ -3,6 +3,7 @@ package io.ebeaninternal.api;
 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,17 +58,22 @@ public final class LoadManyRequest extends LoadRequest {
     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) {
@@ -75,7 +81,7 @@ public final class LoadManyRequest extends LoadRequest {
       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 final class LoadManyRequest extends LoadRequest {
       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());

ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java

@@ -63,8 +63,8 @@ final class DefaultBeanLoader {
     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;

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));
+    }
+  }
+}

ebean-test/src/test/java/org/tests/model/elementcollection/TestElementCollectionCascade.java

@@ -46,7 +46,7 @@ public class TestElementCollectionCascade extends BaseTestCase {
     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");

ebean-test/src/test/java/org/tests/query/TestQueryFindIterate.java

@@ -172,9 +172,9 @@ public class TestQueryFindIterate extends BaseTestCase {
 
     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)

ebean-test/src/test/java/org/tests/query/TestRefToLazyLoadMany.java

@@ -1,7 +1,9 @@
 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;
@@ -9,6 +11,7 @@ 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 class TestRefToLazyLoadMany extends BaseTestCase {
     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);
+    }
   }
 }

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"/>-->