Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 18 additions & 11 deletions ebean-core/src/main/java/io/ebeaninternal/api/LoadManyRequest.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix here really - 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;
Expand All @@ -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());
Expand Down
Expand Up @@ -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;
Expand Down
53 changes: 53 additions & 0 deletions 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));
}
}
}
Expand Up @@ -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");
Expand Down
Expand Up @@ -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)
Expand Down
@@ -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 {
Expand All @@ -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);
}
}
}
6 changes: 3 additions & 3 deletions ebean-test/src/test/resources/logback-test.xml
Expand Up @@ -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"/>-->
Expand Down