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.

Sign up for GitHub

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 orderBy for cached many requests - v2 #3486

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix orderBy for cached many requests - v2 #3486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e77f88f
add:failing test
nPraml Oct 1, 2024
2cee600
fix orderBy for cached many requests
nPraml Oct 1, 2024
7683f89
Use same code for non cached and cached loads
nPraml Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions ebean-core/src/main/java/io/ebeaninternal/api/LoadManyRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,8 @@ private BeanPropertyAssocMany<?> many() {

public SpiQuery<?> createQuery(SpiEbeanServer server) {
BeanPropertyAssocMany<?> many = many();
SpiQuery<?> query = many.newQuery(server);
SpiQuery<?> query = many.newLoadManyQuery(server, onlyIds);
query.usingTransaction(transaction);
String orderBy = many.lazyFetchOrderBy();
if (orderBy != null) {
query.orderBy(orderBy);
}
String extraWhere = many.extraWhere();
if (extraWhere != null) {
// replace special ${ta} placeholder with the base table alias
// which is always t0 and add the extra where clause
query.where().raw(extraWhere.replace("${ta}", "t0").replace("${mta}", "int_"));
}
query.setLazyLoadForParents(many);
final var pc = loadContext.persistenceContext();
many.addWhereParentIdIn(query, parentIdList(server, many, pc), loadContext.isUseDocStore());
query.setPersistenceContext(pc);
Expand All @@ -117,15 +106,7 @@ public SpiQuery<?> createQuery(SpiEbeanServer server) {
}
// potentially changes the joins, selected properties, cache mode
loadContext.configureQuery(query);
if (onlyIds) {
// lazy loading invoked via clear() and removeAll()
String mapKey = many.mapKey();
if (mapKey != null) {
query.select(mapKey);
} else {
query.select(many.targetIdProperty());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

CHECKME: Is ist ok, if the onlyIds code path is in newLoadManyQuery or may there be cases, where configureQuery overwrites the select?


return query;
}

Expand Down
23 changes: 7 additions & 16 deletions ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.ebeaninternal.server.transaction.DefaultPersistenceContext;

import jakarta.persistence.EntityNotFoundException;

import java.util.List;

import static java.lang.System.Logger.Level.DEBUG;
Expand Down Expand Up @@ -77,7 +78,11 @@ private void loadManyInternal(EntityBean parentBean, String propertyName, boolea
}
}

SpiQuery<?> query = server.createQuery(parentDesc.type());

SpiQuery<?> query = many.newLoadManyQuery(server, onlyIds);

many.addWhereParentIdIn(query, List.of(parentId), false);

if (refresh) {
// populate a new collection
BeanCollection<?> emptyCollection = many.createEmpty(parentBean);
Expand All @@ -87,26 +92,12 @@ private void loadManyInternal(EntityBean parentBean, String propertyName, boolea
query.setLoadDescription("lazy", null);
}

query.select(parentDesc.idBinder().idSelect());
if (onlyIds) {
query.fetch(many.name(), many.targetIdProperty());
} else {
query.fetch(many.name());
}
if (filterMany != null) {
query.setFilterMany(many.name(), filterMany);
}

query.where().idEq(parentId);
query.setBeanCacheMode(CacheMode.OFF);
query.setMode(Mode.LAZYLOAD_MANY);
query.setLazyLoadManyPath(many.name());
Copy link
Contributor

Choose a reason for hiding this comment

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

CHECKME: what do we need here?

query.setPersistenceContext(pc);
if (ebi.isReadOnly()) {
query.setReadOnly(true);
}

server.findOne(query);
server.findList(query);
if (beanCollection != null) {
if (beanCollection.checkEmptyLazyLoad()) {
if (log.isLoggable(DEBUG)) {
Expand Down
24 changes: 24 additions & 0 deletions ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,4 +1165,28 @@ public boolean createJoinTable() {
return false;
}
}

public SpiQuery<?> newLoadManyQuery(SpiEbeanServer server, boolean onlyIds) {
SpiQuery<?> query = newQuery(server);
String orderBy = lazyFetchOrderBy();
if (orderBy != null) {
query.orderBy(orderBy);
}
String extraWhere = extraWhere();
if (extraWhere != null) {
// replace special ${ta} placeholder with the base table alias
// which is always t0 and add the extra where clause
query.where().raw(extraWhere.replace("${ta}", "t0").replace("${mta}", "int_"));
}
query.setLazyLoadForParents(this);
if (onlyIds) {
// lazy loading invoked via clear() and removeAll()
if (mapKey() != null) {
query.select(mapKey());
} else {
query.select(targetIdProperty());
}
}
return query;
}
}
53 changes: 52 additions & 1 deletion ebean-test/src/test/java/org/tests/cascade/TestOrderedList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void test() {

List<OmOrderedDetail> details = master.getDetails();
for (int i = 0; i < 5; i++) {
details.add(new OmOrderedDetail("d"+i));
details.add(new OmOrderedDetail("d" + i));
}

LoggedSql.start();
Expand Down Expand Up @@ -188,4 +188,55 @@ public void testModifyListWithCache() {
assertThat(masterDb.getDetails()).containsExactlyInAnyOrder(detail3, detail1);

}

@Test
public void testModifyListWithCache2() {
final OmCacheOrderedMaster master = new OmCacheOrderedMaster("Master");
final OmCacheOrderedDetail detail1 = new OmCacheOrderedDetail("Detail1");
// detail1.setId(2L);
final OmCacheOrderedDetail detail2 = new OmCacheOrderedDetail("Detail2");
// detail2.setId(1L);
final OmCacheOrderedDetail detail3 = new OmCacheOrderedDetail("Detail3");
// detail3.setId(3L);
DB.save(detail1);
DB.save(detail2);
DB.save(detail3);
master.getDetails().add(detail1);
master.getDetails().add(detail2);
master.getDetails().add(detail3);

DB.save(master);

OmCacheOrderedMaster masterDb = DB.find(OmCacheOrderedMaster.class, master.getId()); // load cache
assertThat(masterDb.getDetails()).containsExactly(detail1, detail2, detail3);

masterDb = DB.find(OmCacheOrderedMaster.class, master.getId());
assertThat(masterDb.getDetails()).containsExactly(detail1, detail2, detail3); // hit cache

// 1 und 2 tauschen
masterDb.getDetails().add(0, masterDb.getDetails().remove(1));
DB.save(masterDb);

masterDb.getDetails().remove(2);
DB.save(masterDb);

LoggedSql.start();
OmCacheOrderedMaster masterDbNew = DB.find(OmCacheOrderedMaster.class, master.getId());
masterDbNew.getDetails().size();
assertThat(masterDbNew.getDetails()).containsExactly(detail2, detail1);
List<String> sql = LoggedSql.stop();
assertThat(sql).hasSize(1).first().asString()
.startsWith("select t0.master_id, t0.id, t0.name, t0.version, t0.sort_order, t0.master_id from om_cache_ordered_detail t0 where (t0.master_id) in (?) order by t0.master_id, t0.sort_order;");

DB.cacheManager().clearAll();
masterDbNew = DB.find(OmCacheOrderedMaster.class, master.getId());
LoggedSql.start();
masterDbNew.getDetails().size();
sql = LoggedSql.stop();
assertThat(sql).hasSize(1).first().asString()
.startsWith("select t0.master_id, t0.id, t0.name, t0.version, t0.sort_order, t0.master_id from om_cache_ordered_detail t0 where (t0.master_id) in (?) order by t0.master_id, t0.sort_order;");


assertThat(masterDbNew.getDetails()).containsExactly(detail2, detail1);
}
}