From 88b6a60feb005effd7a925f351ef0090b05ae1ec Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 6 Jul 2022 20:19:41 +1200 Subject: [PATCH] #2730 - Secondary @OneToMany fetch query is not happening if the same Entity type is fetched --- .../server/deploy/DbReadContext.java | 6 - .../io/ebeaninternal/server/query/CQuery.java | 8 -- .../server/query/SqlTreeLoadBean.java | 14 +- .../server/query/SqlTreeLoadManyRoot.java | 4 + .../java/org/tests/o2m/recurse/RMItem.java | 51 +++++++ .../org/tests/o2m/recurse/RMItemHolder.java | 80 +++++++++++ .../TestFetchOneToManySameTypeTwoPaths.java | 133 ++++++++++++++++++ 7 files changed, 280 insertions(+), 16 deletions(-) create mode 100644 ebean-test/src/test/java/org/tests/o2m/recurse/RMItem.java create mode 100644 ebean-test/src/test/java/org/tests/o2m/recurse/RMItemHolder.java create mode 100644 ebean-test/src/test/java/org/tests/o2m/recurse/TestFetchOneToManySameTypeTwoPaths.java diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbReadContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbReadContext.java index 0ed8284d8..e79e7e0df 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbReadContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbReadContext.java @@ -6,7 +6,6 @@ import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; import io.ebean.core.type.DataReader; import io.ebeaninternal.api.SpiQuery; -import io.ebeaninternal.server.query.STreePropertyAssocMany; import java.util.Map; @@ -70,11 +69,6 @@ public interface DbReadContext { */ void register(BeanPropertyAssocMany many, BeanCollection bc); - /** - * Return the property that is associated with the many. There can only be - * one. This can be null. - */ - STreePropertyAssocMany getManyProperty(); /** * Set back the bean that has just been loaded with its id. diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQuery.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQuery.java index 3566ee08a..d153c359a 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQuery.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQuery.java @@ -605,14 +605,6 @@ public final class CQuery implements DbReadContext, CancelableQuery, SpiProfi return logWhereSql; } - /** - * Return the property that is associated with the many. There can only be one - * per SqlSelect. This can be null. - */ - @Override - public STreePropertyAssocMany getManyProperty() { - return manyProperty; - } public String getBindLog() { return bindLog; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java index 664c5e24f..312fb359a 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java @@ -36,6 +36,7 @@ class SqlTreeLoadBean implements SqlTreeLoad { private final SpiQuery.TemporalMode temporalMode; private final boolean temporalVersions; final IdBinder lazyLoadParentIdBinder; + private final STreePropertyAssocMany loadingChildProperty; SqlTreeLoadBean(SqlTreeNodeBean node) { this.lazyLoadParent = node.lazyLoadParent; @@ -55,6 +56,16 @@ class SqlTreeLoadBean implements SqlTreeLoad { this.properties = node.properties; this.pathMap = node.pathMap; this.children = node.createLoadChildren(); + this.loadingChildProperty = loadingChildProperty(); + } + + private STreePropertyAssocMany loadingChildProperty() { + for (SqlTreeLoad child : children) { + if (child instanceof SqlTreeLoadManyRoot) { + return ((SqlTreeLoadManyRoot) child).manyProp(); + } + } + return null; } boolean isRoot() { @@ -280,10 +291,9 @@ class SqlTreeLoadBean implements SqlTreeLoad { * included in the actual query. */ private void createListProxies() { - STreePropertyAssocMany fetchedMany = ctx.getManyProperty(); boolean forceNewReference = queryMode == Mode.REFRESH_BEAN; for (STreePropertyAssocMany many : localDesc.propsMany()) { - if (many != fetchedMany) { + if (many != loadingChildProperty) { if (readOnlyNoIntercept) { many.createEmptyReference(localBean); } else { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadManyRoot.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadManyRoot.java index 3622e770e..560a231e9 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadManyRoot.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadManyRoot.java @@ -14,6 +14,10 @@ final class SqlTreeLoadManyRoot extends SqlTreeLoadBean { this.manyProp = node.manyProp; } + STreePropertyAssocMany manyProp() { + return manyProp; + } + @Override public EntityBean load(DbReadContext cquery, EntityBean parentBean, EntityBean contextParent) throws SQLException { // pass in null for parentBean because added to a collection rather than set to the parentBean diff --git a/ebean-test/src/test/java/org/tests/o2m/recurse/RMItem.java b/ebean-test/src/test/java/org/tests/o2m/recurse/RMItem.java new file mode 100644 index 000000000..a08a0a58f --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/recurse/RMItem.java @@ -0,0 +1,51 @@ +package org.tests.o2m.recurse; + +import javax.persistence.*; +import java.util.List; + +@Entity +public class RMItem { + + @Id + private long itemId; + + @ManyToOne + @JoinColumn(name = "item_group_id") + private RMItem itemGroup; + + private String name; + + @OneToMany(mappedBy = "itemGroup") + private List subItems; + + public RMItem() { + } + + public RMItem(String name) { + this.name = name; + } + + public Long getItemId() { + return itemId; + } + + public void setItemId(Long itemId) { + this.itemId = itemId; + } + + public RMItem getItemGroup() { + return itemGroup; + } + + public void setItemGroup(RMItem itemGroup) { + this.itemGroup = itemGroup; + } + + public List getSubItems() { + return subItems; + } + + public void setSubItems(List subItems) { + this.subItems = subItems; + } +} diff --git a/ebean-test/src/test/java/org/tests/o2m/recurse/RMItemHolder.java b/ebean-test/src/test/java/org/tests/o2m/recurse/RMItemHolder.java new file mode 100644 index 000000000..aa77c3e73 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/recurse/RMItemHolder.java @@ -0,0 +1,80 @@ +package org.tests.o2m.recurse; + +import io.ebean.Model; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Version; + +@Entity +public class RMItemHolder extends Model { + + @Id + long id; + String name; + String notes; + @ManyToOne + //@JoinColumn(name = "item_a_id") + private RMItem itemA; + @ManyToOne + //@JoinColumn(name = "item_b_id") + private RMItem itemB; + @Version + long version; + + public RMItemHolder(String name) { + this.name = name; + } + + public RMItemHolder() { + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getNotes() { + return notes; + } + + public void setNotes(String notes) { + this.notes = notes; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } + + public RMItem getItemA() { + return itemA; + } + + public void setItemA(RMItem itemA) { + this.itemA = itemA; + } + + public RMItem getItemB() { + return itemB; + } + + public void setItemB(RMItem itemB) { + this.itemB = itemB; + } +} diff --git a/ebean-test/src/test/java/org/tests/o2m/recurse/TestFetchOneToManySameTypeTwoPaths.java b/ebean-test/src/test/java/org/tests/o2m/recurse/TestFetchOneToManySameTypeTwoPaths.java new file mode 100644 index 000000000..34df0ac88 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/o2m/recurse/TestFetchOneToManySameTypeTwoPaths.java @@ -0,0 +1,133 @@ +package org.tests.o2m.recurse; + +import io.ebean.DB; +import io.ebean.Database; +import io.ebean.FetchConfig; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TestFetchOneToManySameTypeTwoPaths { + + @Test + void testSubItemListFetch_ofQuery() { + Database server = DB.getDefault(); + + RMItem itemA = new RMItem("a"); + server.save(itemA); + RMItem itemB = new RMItem("b"); + server.save(itemB); + + for (int i=0; i<2; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemA); + server.save(subItem); + } + + for (int i=0; i<3; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemB); + server.save(subItem); + } + + RMItemHolder customer = new RMItemHolder(); + customer.setItemA(itemA); + customer.setItemB(itemB); + server.save(customer); + + // This is OK + { + RMItemHolder requestedCustomer = server.find(RMItemHolder.class) + .setDisableLazyLoading(true) + .fetch("itemA.subItems", FetchConfig.ofQuery()) + .fetch("itemB.subItems", FetchConfig.ofQuery()) + .where() + .eq("id", customer.getId()) + .findOne(); + assertEquals(2, requestedCustomer.getItemA().getSubItems().size()); + assertEquals(3, requestedCustomer.getItemB().getSubItems().size()); + } + } + + @Test + void testSubItemListFetch_itemAFirst() { + Database server = DB.getDefault(); + + RMItem itemA = new RMItem("aa"); + server.save(itemA); + RMItem itemB = new RMItem("bb"); + server.save(itemB); + + for (int i=0; i<2; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemA); + server.save(subItem); + } + + for (int i=0; i<3; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemB); + server.save(subItem); + } + + RMItemHolder customer = new RMItemHolder(); + customer.setItemA(itemA); + customer.setItemB(itemB); + server.save(customer); + + // This fails because requestedCustomer.getItemB().getSubItems() is not loaded + { + RMItemHolder requestedCustomer = server.find(RMItemHolder.class) + .setDisableLazyLoading(true) + .fetch("itemA.subItems") + .fetch("itemB.subItems") + .where() + .eq("id", customer.getId()) + .findOne(); + assertEquals(2, requestedCustomer.getItemA().getSubItems().size()); + assertEquals(3, requestedCustomer.getItemB().getSubItems().size()); + } + } + + @Test + void testSubItemListFetch_itemBFirst() { + Database server = DB.getDefault(); + + RMItem itemA = new RMItem("a"); + server.save(itemA); + RMItem itemB = new RMItem("b"); + server.save(itemB); + + for (int i=0; i<2; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemA); + server.save(subItem); + } + + for (int i=0; i<5; i++) { + RMItem subItem = new RMItem(); + subItem.setItemGroup(itemB); + server.save(subItem); + } + + RMItemHolder customer = new RMItemHolder(); + customer.setItemA(itemA); + customer.setItemB(itemB); + server.save(customer); + + // This fails because requestedCustomer.getItemA().getSubItems() is not loaded + { + RMItemHolder requestedCustomer = server.find(RMItemHolder.class) + .setDisableLazyLoading(true) + .fetch("itemB.subItems") + .fetch("itemA.subItems") + .where() + .eq("id", customer.getId()) + .findOne(); + assertEquals(2, requestedCustomer.getItemA().getSubItems().size()); + assertEquals(5, requestedCustomer.getItemB().getSubItems().size()); + System.out.println("here"); + } + } + +}