From 562d175de1f4435d623f2c4abeec82b38b609f41 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Tue, 23 Mar 2021 23:17:07 +1300 Subject: [PATCH] With FetchConfig.ofLazy() honor query.setLazyLoadBatchSize() Use 0 for default lazy batch such that it honors a value set via query.setLazyLoadBatchSize() --- .../src/main/java/io/ebean/FetchConfig.java | 21 ++++------ .../server/loadcontext/DLoadBaseContext.java | 8 +--- .../server/loadcontext/DLoadBeanContext.java | 4 +- .../server/loadcontext/DLoadContext.java | 42 ++++++++++--------- .../server/loadcontext/DLoadManyContext.java | 7 +--- .../test/java/io/ebean/FetchConfigTest.java | 3 +- .../server/grammer/ParseFetchConfigTest.java | 2 +- .../org/tests/query/TestQueryFindEach.java | 38 ++++++++++++----- 8 files changed, 68 insertions(+), 57 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/FetchConfig.java b/ebean-api/src/main/java/io/ebean/FetchConfig.java index 64f4c3daa..e2d4fbe35 100644 --- a/ebean-api/src/main/java/io/ebean/FetchConfig.java +++ b/ebean-api/src/main/java/io/ebean/FetchConfig.java @@ -46,7 +46,7 @@ public class FetchConfig implements Serializable { /** * Deprecated - migrate to one of the static factory methods like {@link FetchConfig#ofQuery()} - * + *

* Construct using default JOIN mode. */ @Deprecated @@ -89,7 +89,7 @@ public class FetchConfig implements Serializable { * Return FetchConfig to lazily load the relationship. */ public static FetchConfig ofLazy() { - return new FetchConfig(LAZY_MODE, 10); + return new FetchConfig(LAZY_MODE, 0); } /** @@ -110,8 +110,8 @@ public class FetchConfig implements Serializable { * We want to migrate away from mutating FetchConfig to a fully immutable FetchConfig. */ private FetchConfig mutate(int mode, int batchSize) { - if (batchSize < 1) { - throw new IllegalArgumentException("batch size "+batchSize+" must be > 0"); + if (batchSize < 0) { + throw new IllegalArgumentException("batch size " + batchSize + " must be > 0"); } this.mode = mode; this.batchSize = batchSize; @@ -124,7 +124,7 @@ public class FetchConfig implements Serializable { */ @Deprecated public FetchConfig lazy() { - return mutate(LAZY_MODE, 10); + return mutate(LAZY_MODE, 0); } /** @@ -137,7 +137,7 @@ public class FetchConfig implements Serializable { /** * Deprecated - migrate to FetchConfig.ofQuery(). - * + *

* Eagerly fetch the beans in this path as a separate query (rather than as * part of the main query). *

@@ -150,17 +150,15 @@ public class FetchConfig implements Serializable { /** * Deprecated - migrate to FetchConfig.ofQuery(batchSize). - * + *

* Eagerly fetch the beans in this path as a separate query (rather than as * part of the main query). *

* The queryBatchSize is the number of parent id's that this separate query * will load per batch. - *

*

* This will load all beans on this path eagerly unless a {@link #lazy(int)} * is also used. - *

* * @param batchSize the batch size used to load beans on this path */ @@ -171,13 +169,12 @@ public class FetchConfig implements Serializable { /** * Deprecated - migrate to FetchConfig.ofQuery(batchSize). - * + *

* Eagerly fetch the first batch of beans on this path. * This is similar to {@link #query(int)} but only fetches the first batch. *

* If there are more parent beans than the batch size then they will not be * loaded eagerly but instead use lazy loading. - *

* * @param batchSize the number of parent beans this path is populated for */ @@ -188,7 +185,7 @@ public class FetchConfig implements Serializable { /** * Deprecated - migrate to FetchConfig.ofCache(). - * + *

* Eagerly fetch the beans fetching the beans from the L2 bean cache * and using the DB for beans not in the cache. */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBaseContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBaseContext.java index 1be4717e9..24233cf87 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBaseContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBaseContext.java @@ -33,7 +33,7 @@ abstract class DLoadBaseContext { final boolean queryFetch; - DLoadBaseContext(DLoadContext parent, BeanDescriptor desc, String path, int defaultBatchSize, OrmQueryProperties queryProps) { + DLoadBaseContext(DLoadContext parent, BeanDescriptor desc, String path, OrmQueryProperties queryProps) { this.parent = parent; this.serverName = parent.getEbeanServer().getName(); this.desc = desc; @@ -42,11 +42,7 @@ abstract class DLoadBaseContext { this.hitCache = parent.isBeanCacheGet() && desc.isBeanCaching(); this.objectGraphNode = parent.getObjectGraphNode(path); this.queryFetch = queryProps != null && queryProps.isQueryFetch(); - this.batchSize = initBatchSize(defaultBatchSize, queryProps); - } - - private int initBatchSize(int batchSize, OrmQueryProperties queryProps) { - return queryProps == null ? batchSize : queryProps.getBatchSize(); + this.batchSize = parent.batchSize(queryProps); } /** diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java index d24a24532..25fffe5b7 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java @@ -31,8 +31,8 @@ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext { private LoadBuffer currentBuffer; - DLoadBeanContext(DLoadContext parent, BeanDescriptor desc, String path, int defaultBatchSize, OrmQueryProperties queryProps) { - super(parent, desc, path, defaultBatchSize, queryProps); + DLoadBeanContext(DLoadContext parent, BeanDescriptor desc, String path, OrmQueryProperties queryProps) { + super(parent, desc, path, queryProps); // bufferList only required when using query joins (queryFetch) this.bufferList = (!queryFetch) ? null : new ArrayList<>(); this.currentBuffer = createBuffer(batchSize); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadContext.java index 912b8fc0c..060633073 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadContext.java @@ -90,7 +90,7 @@ public class DLoadContext implements LoadContext { this.planLabel = null; this.profileLocation = null; this.profilingListener = null; - this.rootBeanContext = new DLoadBeanContext(this, rootDescriptor, null, defaultBatchSize, null); + this.rootBeanContext = new DLoadBeanContext(this, rootDescriptor, null, null); } private ObjectGraphOrigin initOrigin() { @@ -128,7 +128,7 @@ public class DLoadContext implements LoadContext { } // initialise rootBeanContext after origin and relativePath have been set - this.rootBeanContext = new DLoadBeanContext(this, rootDescriptor, null, defaultBatchSize, null); + this.rootBeanContext = new DLoadBeanContext(this, rootDescriptor, null, null); registerSecondaryQueries(secondaryQueries); } @@ -287,51 +287,55 @@ public class DLoadContext implements LoadContext { getManyContext(path, many).register(bc); } + int batchSize(OrmQueryProperties props) { + if (props == null) { + return defaultBatchSize; + } + int batchSize = props.getBatchSize(); + return batchSize == 0 ? defaultBatchSize : batchSize; + } + DLoadBeanContext getBeanContext(String path) { if (path == null) { return rootBeanContext; } - return beanMap.computeIfAbsent(path, p -> createBeanContext(p, defaultBatchSize, null)); + return beanMap.computeIfAbsent(path, p -> createBeanContext(p, null)); } DLoadBeanContext getBeanContextWithInherit(String path, BeanPropertyAssocOne property) { String key = path + ":" + property.getTargetDescriptor().getName(); - return beanMap.computeIfAbsent(key, p -> createBeanContext(property, path, defaultBatchSize, null)); + return beanMap.computeIfAbsent(key, p -> createBeanContext(property, path, null)); } private void registerSecondaryNode(boolean many, OrmQueryProperties props) { - int batchSize = props.getBatchSize(); - if (batchSize == 0) { - batchSize = defaultBatchSize; - } String path = props.getPath(); if (many) { - manyMap.put(path, createManyContext(path, batchSize, props)); + manyMap.put(path, createManyContext(path, props)); } else { - beanMap.put(path, createBeanContext(path, batchSize, props)); + beanMap.put(path, createBeanContext(path, props)); } } DLoadManyContext getManyContext(String path, BeanPropertyAssocMany many) { - return manyMap.computeIfAbsent(path, p -> createManyContext(p, many, defaultBatchSize)); + return manyMap.computeIfAbsent(path, p -> createManyContext(p, many)); } - private DLoadManyContext createManyContext(String path, BeanPropertyAssocMany many, int batchSize) { - return new DLoadManyContext(this, many, path, batchSize, null); + private DLoadManyContext createManyContext(String path, BeanPropertyAssocMany many) { + return new DLoadManyContext(this, many, path, null); } - private DLoadManyContext createManyContext(String path, int batchSize, OrmQueryProperties queryProps) { + private DLoadManyContext createManyContext(String path, OrmQueryProperties queryProps) { BeanPropertyAssocMany p = (BeanPropertyAssocMany) getBeanProperty(rootDescriptor, path); - return new DLoadManyContext(this, p, path, batchSize, queryProps); + return new DLoadManyContext(this, p, path, queryProps); } - private DLoadBeanContext createBeanContext(String path, int batchSize, OrmQueryProperties queryProps) { + private DLoadBeanContext createBeanContext(String path, OrmQueryProperties queryProps) { BeanPropertyAssoc p = (BeanPropertyAssoc) getBeanProperty(rootDescriptor, path); - return new DLoadBeanContext(this, p.getTargetDescriptor(), path, batchSize, queryProps); + return new DLoadBeanContext(this, p.getTargetDescriptor(), path, queryProps); } - private DLoadBeanContext createBeanContext(BeanPropertyAssoc property, String path, int batchSize, OrmQueryProperties queryProps) { - return new DLoadBeanContext(this, property.getTargetDescriptor(), path, batchSize, queryProps); + private DLoadBeanContext createBeanContext(BeanPropertyAssoc property, String path, OrmQueryProperties queryProps) { + return new DLoadBeanContext(this, property.getTargetDescriptor(), path, queryProps); } private BeanProperty getBeanProperty(BeanDescriptor desc, String path) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadManyContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadManyContext.java index 7a51127d8..5035f0802 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadManyContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadManyContext.java @@ -31,11 +31,8 @@ class DLoadManyContext extends DLoadBaseContext implements LoadManyContext { private LoadBuffer currentBuffer; - DLoadManyContext(DLoadContext parent, BeanPropertyAssocMany property, - String path, int defaultBatchSize, OrmQueryProperties queryProps) { - - super(parent, property.getBeanDescriptor(), path, defaultBatchSize, queryProps); - + DLoadManyContext(DLoadContext parent, BeanPropertyAssocMany property, String path, OrmQueryProperties queryProps) { + super(parent, property.getBeanDescriptor(), path, queryProps); this.property = property; this.docStoreMapped = property.isTargetDocStoreMapped(); // bufferList only required when using query joins (queryFetch) diff --git a/ebean-core/src/test/java/io/ebean/FetchConfigTest.java b/ebean-core/src/test/java/io/ebean/FetchConfigTest.java index abd09d692..f85808f6c 100644 --- a/ebean-core/src/test/java/io/ebean/FetchConfigTest.java +++ b/ebean-core/src/test/java/io/ebean/FetchConfigTest.java @@ -1,6 +1,5 @@ package io.ebean; -import io.ebean.FetchConfig; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -10,7 +9,7 @@ public class FetchConfigTest { @Test public void testLazy() { FetchConfig config = new FetchConfig().lazy(); - assertThat(config.getBatchSize()).isEqualTo(10); + assertThat(config.getBatchSize()).isEqualTo(0); } @Test diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/grammer/ParseFetchConfigTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/grammer/ParseFetchConfigTest.java index a940cd26b..c4976f56d 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/grammer/ParseFetchConfigTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/grammer/ParseFetchConfigTest.java @@ -19,7 +19,7 @@ public class ParseFetchConfigTest { @Test public void parseLazy() { FetchConfig lazy = ParseFetchConfig.parse("lazy"); - assertThat(lazy.getBatchSize()).isEqualTo(10); + assertThat(lazy.getBatchSize()).isEqualTo(0); } @Test diff --git a/ebean-core/src/test/java/org/tests/query/TestQueryFindEach.java b/ebean-core/src/test/java/org/tests/query/TestQueryFindEach.java index 1a2c16bab..b37f67279 100644 --- a/ebean-core/src/test/java/org/tests/query/TestQueryFindEach.java +++ b/ebean-core/src/test/java/org/tests/query/TestQueryFindEach.java @@ -169,22 +169,40 @@ public class TestQueryFindEach extends BaseTestCase { DB.find(OmBasicParent.class).delete(); insertData(); + test_setLazyLoadBatchSize_withFetchLazy(); + LoggedSqlCollector.start(); - try (final Transaction transaction = DB.beginTransaction()) { - // DB.find(OmBasicParent.class).findList(); - DB.find(OmBasicChild.class) - .setLazyLoadBatchSize(100) - //.fetchQuery("parent","name") - //.fetch("parent","name") - .findEach(child -> { - assertNotNull(child.getParent().getName()); - }); - } + + DB.find(OmBasicChild.class) + .setLazyLoadBatchSize(100) + .findEach(child -> { + assertNotNull(child.getParent().getName()); + }); final List sql = LoggedSqlCollector.stop(); assertThat(sql.size()).isLessThan(50); } + private void test_setLazyLoadBatchSize_withFetchLazy() { + + LoggedSqlCollector.start(); + + DB.find(OmBasicParent.class) + .setLazyLoadBatchSize(5) + .fetchLazy("children") + .setMaxRows(50) + .findEach(it -> { + it.getChildren().size(); + }); + + final List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(11); + assertThat(sql.get(0)).contains(" from om_basic_parent "); + for (int i = 1; i < 11; i++) { + assertThat(sql.get(i)).contains(" --bind(Array[5]"); + } + } + @Transactional(batchSize = 40) private void insertData() { for (int i = 0; i < 150; i++) {