Fix for #3381 - Empty OneToMany when overflowing LazyLoadBatchSize and 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.
This commit is contained in:
Rob Bygrave
2024-04-04 20:13:20 +13:00
parent f556e04f58
commit 7ea4311662
7 changed files with 96 additions and 27 deletions
@@ -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());
@@ -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;
@@ -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));
}
}
}
@@ -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");
@@ -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)
@@ -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);
}
}
}
@@ -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"/>-->