From c583879d571be92dc5dec5e897db7bc07301e41f Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 18 Mar 2025 23:46:42 +1300 Subject: [PATCH] Modify disableLazyLoad to throw LazyInitialisationException instead of returning null Modify such that accessing an unloaded property throws a LazyInitialisationException. This includes ToMany collections that where previously initialised as empty collections (that didn't lazy load). This change brings the disableLazyLoad behaviour in line with the unmodifiable behaviour. It does mean that the use case of bulk mapping from entities to DTOs via something like MapStruct will no longer work nicely (mapping nulls and empty lists versus LazyInitialisationException). --- .../io/ebean/bean/EntityBeanIntercept.java | 5 --- .../java/io/ebean/bean/InterceptReadOnly.java | 5 --- .../io/ebean/bean/InterceptReadWrite.java | 11 +++--- .../server/loadcontext/DLoadContext.java | 2 +- .../server/query/SqlTreeLoadBean.java | 4 ++- .../config/PlatformNoGeneratedKeysTest.java | 8 ++--- .../org/tests/batchload/TestBeanState.java | 4 +-- .../batchload/TestQueryDisableLazyLoad.java | 35 +++++++++++++++---- .../batchload/TestQueryJoinToAssocOne.java | 24 ++++++------- .../aggregation/TestAggregationMany.java | 8 +++-- .../text/json/TestTextJsonReferenceBean.java | 6 ++-- 11 files changed, 64 insertions(+), 48 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java b/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java index cd6fa560df..11f2487832 100644 --- a/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java +++ b/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java @@ -381,11 +381,6 @@ public interface EntityBeanIntercept extends Serializable { */ void loadBean(int loadProperty); - /** - * Invoke the lazy loading. This method is synchronised externally. - */ - void loadBeanInternal(int loadProperty, BeanLoader loader); - /** * Called when a BeanCollection is initialised automatically. */ diff --git a/ebean-api/src/main/java/io/ebean/bean/InterceptReadOnly.java b/ebean-api/src/main/java/io/ebean/bean/InterceptReadOnly.java index 52a7de9089..47117c8820 100644 --- a/ebean-api/src/main/java/io/ebean/bean/InterceptReadOnly.java +++ b/ebean-api/src/main/java/io/ebean/bean/InterceptReadOnly.java @@ -393,11 +393,6 @@ public void loadBean(int loadProperty) { } - @Override - public void loadBeanInternal(int loadProperty, BeanLoader loader) { - - } - @Override public void initialisedMany(int propertyIndex) { loaded[propertyIndex] = true; diff --git a/ebean-api/src/main/java/io/ebean/bean/InterceptReadWrite.java b/ebean-api/src/main/java/io/ebean/bean/InterceptReadWrite.java index 56b2ab5493..da477e2867 100644 --- a/ebean-api/src/main/java/io/ebean/bean/InterceptReadWrite.java +++ b/ebean-api/src/main/java/io/ebean/bean/InterceptReadWrite.java @@ -2,6 +2,7 @@ import io.ebean.DB; import io.ebean.Database; +import io.ebean.LazyInitialisationException; import io.ebean.ValuePair; import jakarta.persistence.EntityNotFoundException; @@ -59,7 +60,7 @@ public final class InterceptReadWrite extends InterceptBase { private EntityBean embeddedOwner; private int embeddedOwnerIndex; /** - * One of NEW, REF, UPD. + * One of NEW, REFERENCE, LOADED. */ private int state; private boolean forceUpdate; @@ -645,6 +646,9 @@ public String lazyLoadProperty() { public void loadBean(int loadProperty) { lock.lock(); try { + if (disableLazyLoad) { + throw new LazyInitialisationException("Property not loaded: " + property(loadProperty)); + } if (beanLoader == null) { final Database database = DB.byName(ebeanServerName); if (database == null) { @@ -668,8 +672,7 @@ public void loadBean(int loadProperty) { } } - @Override - public void loadBeanInternal(int loadProperty, BeanLoader loader) { + private void loadBeanInternal(int loadProperty, BeanLoader loader) { if ((flags[loadProperty] & FLAG_LOADED_PROP) != 0) { // race condition where multiple threads calling preGetter concurrently return; @@ -771,7 +774,7 @@ public void preGetId() { @Override public void preGetter(int propertyIndex) { preGetterCallback(propertyIndex); - if (state == STATE_NEW || disableLazyLoad) { + if (state == STATE_NEW) { return; } if (!isLoadedProperty(propertyIndex)) { 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 feac1ab558..6a0cdbaabf 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 @@ -97,7 +97,7 @@ public DLoadContext(OrmQueryRequest request, SpiQuerySecondary secondaryQueri this.profilingListener = query.profilingListener(); this.planLabel = query.planLabel(); this.profileLocation = query.profileLocation(); - this.secondaryProperties = query.isUnmodifiable() ? new HashSet<>() : null; + this.secondaryProperties = query.isUnmodifiable() || query.isDisableLazyLoading() ? new HashSet<>() : null; ObjectGraphNode parentNode = query.parentNode(); if (parentNode != null) { 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 40d8317d42..0882b44146 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 @@ -32,6 +32,7 @@ class SqlTreeLoadBean implements SqlTreeLoad { private final boolean readIdNormal; private final boolean disableLazyLoad; private final boolean unmodifiable; + private final boolean loadListReferences; private final InheritInfo inheritInfo; final String prefix; private final Map pathMap; @@ -55,6 +56,7 @@ class SqlTreeLoadBean implements SqlTreeLoad { this.readIdNormal = readId && !temporalVersions; this.disableLazyLoad = node.disableLazyLoad; this.unmodifiable = node.unmodifiable; + this.loadListReferences = !unmodifiable && !disableLazyLoad; this.partialObject = node.partialObject; this.properties = node.properties; this.pathMap = node.pathMap; @@ -301,7 +303,7 @@ private void createListProxies() { boolean forceNewReference = queryMode == Mode.REFRESH_BEAN; for (STreePropertyAssocMany many : localDesc.propsMany()) { if (many != loadingChildProperty) { - if (!unmodifiable || ctx.includeSecondary(many.asMany())) { + if (loadListReferences || ctx.includeSecondary(many.asMany())) { // create a proxy for the many (deferred fetching) BeanCollection ref = many.createReference(localBean, forceNewReference); if (ref != null) { diff --git a/ebean-test/src/test/java/io/ebean/xtest/config/PlatformNoGeneratedKeysTest.java b/ebean-test/src/test/java/io/ebean/xtest/config/PlatformNoGeneratedKeysTest.java index db3c9622bd..c8c0486eaf 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/config/PlatformNoGeneratedKeysTest.java +++ b/ebean-test/src/test/java/io/ebean/xtest/config/PlatformNoGeneratedKeysTest.java @@ -1,10 +1,7 @@ package io.ebean.xtest.config; -import io.ebean.Database; -import io.ebean.DatabaseFactory; -import io.ebean.Transaction; +import io.ebean.*; import io.ebean.annotation.Platform; -import io.ebean.DatabaseBuilder; import io.ebean.config.DatabaseConfig; import io.ebean.config.dbplatform.DbIdentity; import io.ebean.config.dbplatform.IdType; @@ -15,6 +12,7 @@ import org.tests.model.draftable.BasicDraftableBean; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; public class PlatformNoGeneratedKeysTest { @@ -39,7 +37,7 @@ public void global_serverConfig_setDisableLazyLoading() { .findOne(); assertThat(found.getName()).isEqualTo("basic"); - assertThat(found.getDescription()).isNull(); + assertThrows(LazyInitialisationException.class, found::getDescription); } @Test diff --git a/ebean-test/src/test/java/org/tests/batchload/TestBeanState.java b/ebean-test/src/test/java/org/tests/batchload/TestBeanState.java index cc28e4807e..5213ec7198 100644 --- a/ebean-test/src/test/java/org/tests/batchload/TestBeanState.java +++ b/ebean-test/src/test/java/org/tests/batchload/TestBeanState.java @@ -2,7 +2,7 @@ import io.ebean.BeanState; import io.ebean.DB; -import io.ebean.UnmodifiableEntityException; +import io.ebean.LazyInitialisationException; import io.ebean.bean.EntityBean; import io.ebean.bean.EntityBeanIntercept; import io.ebean.xtest.BaseTestCase; @@ -83,7 +83,7 @@ void setDisableLazyLoad_expect_lazyLoadingDisabled() { BeanState beanState = DB.beanState(customer); beanState.setDisableLazyLoad(true); - assertNull(customer.getName()); + assertThrows(LazyInitialisationException.class, customer::getName); } @Test diff --git a/ebean-test/src/test/java/org/tests/batchload/TestQueryDisableLazyLoad.java b/ebean-test/src/test/java/org/tests/batchload/TestQueryDisableLazyLoad.java index 0ac8cc6be1..35efdd0b99 100644 --- a/ebean-test/src/test/java/org/tests/batchload/TestQueryDisableLazyLoad.java +++ b/ebean-test/src/test/java/org/tests/batchload/TestQueryDisableLazyLoad.java @@ -1,18 +1,18 @@ package org.tests.batchload; +import io.ebean.LazyInitialisationException; import io.ebean.xtest.BaseTestCase; import io.ebean.DB; import io.ebean.test.LoggedSql; import org.junit.jupiter.api.Test; import org.tests.model.basic.Order; -import org.tests.model.basic.OrderDetail; import org.tests.model.basic.ResetBasicData; +import java.sql.Date; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.*; public class TestQueryDisableLazyLoad extends BaseTestCase { @@ -32,8 +32,7 @@ public void onAssocMany() { Order order = l0.get(0); - List details = order.getDetails(); - assertEquals(details.size(), 0); + assertThrows(LazyInitialisationException.class, order::getDetails); List loggedSql = LoggedSql.stop(); assertThat(loggedSql).hasSize(1); @@ -59,7 +58,7 @@ public void onAssocOne() { Order order = l0.get(0); // normally invokes lazy loading - assertNull(order.getCustomer().getStatus()); + assertThrows(LazyInitialisationException.class, () -> order.getCustomer().getStatus()); List loggedSql = LoggedSql.stop(); assertThat(loggedSql).hasSize(1); @@ -83,7 +82,29 @@ public void onAssocOne_when_partial() { Order order = l0.get(0); // normally invokes lazy loading - assertNull(order.getCustomer().getStatus()); + assertThrows(LazyInitialisationException.class, () -> order.getCustomer().getStatus()); + + List loggedSql = LoggedSql.stop(); + assertThat(loggedSql).hasSize(1); + } + + @Test + public void onSetter_expect_LazyInitialisationException() { + ResetBasicData.reset(); + LoggedSql.start(); + + List l0 = DB.find(Order.class) + .setDisableLazyLoading(true) + .select("status, orderDate") + .orderBy().asc("id") + .findList(); + + assertThat(l0).isNotEmpty(); + + Order order = l0.get(0); + + // normally invokes lazy loading + assertThrows(LazyInitialisationException.class, () -> order.setShipDate(new Date(System.currentTimeMillis()))); List loggedSql = LoggedSql.stop(); assertThat(loggedSql).hasSize(1); diff --git a/ebean-test/src/test/java/org/tests/batchload/TestQueryJoinToAssocOne.java b/ebean-test/src/test/java/org/tests/batchload/TestQueryJoinToAssocOne.java index e04a18c59d..9b1a4a3ddf 100644 --- a/ebean-test/src/test/java/org/tests/batchload/TestQueryJoinToAssocOne.java +++ b/ebean-test/src/test/java/org/tests/batchload/TestQueryJoinToAssocOne.java @@ -1,5 +1,6 @@ package org.tests.batchload; +import io.ebean.LazyInitialisationException; import io.ebean.xtest.BaseTestCase; import io.ebean.DB; import io.ebean.test.LoggedSql; @@ -16,6 +17,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; public class TestQueryJoinToAssocOne extends BaseTestCase { @@ -91,15 +93,15 @@ public void testQueryJoinOnPartiallyPopulatedParent_withLazyLoadingDisabled() { Order order = l0.get(0); // normally invokes lazy loading - order.getOrderDate(); + assertThrows(LazyInitialisationException.class, order::getOrderDate); List details = order.getDetails(); OrderDetail orderDetail = details.get(0); // normally invokes lazy loading - orderDetail.getShipQty(); + assertThrows(LazyInitialisationException.class, orderDetail::getShipQty); // normally invokes lazy loading - order.getShipments().size(); + assertThrows(LazyInitialisationException.class, order::getShipments); List loggedSql = LoggedSql.stop(); assertThat(loggedSql).hasSize(2); @@ -129,13 +131,11 @@ public void disableLazyLoading_when_oneToManyList() { Order order = l0.get(0); // try to invoke lazy loading on the bean - assertThat(order.getCustomer()).isNull(); - assertThat(order.getCretime()).isNull(); + assertThrows(LazyInitialisationException.class, order::getCustomer); + assertThrows(LazyInitialisationException.class, order::getCretime); // try to invoke lazy loading on the OneToMany ... - List details = order.getDetails(); - assertThat(details).isEmpty(); - assertThat(details.size()).isEqualTo(0); + assertThrows(LazyInitialisationException.class, order::getDetails); List sql = LoggedSql.stop(); assertThat(sql).hasSize(1); @@ -163,7 +163,7 @@ public void disableLazyLoad_when_oneToManySet() { .setId(tenant.getId()) .findOne(); - assertThat(found.getRoles().size()).isEqualTo(0); + assertThrows(LazyInitialisationException.class, found::getRoles); List sql = LoggedSql.stop(); assertThat(sql).hasSize(1); @@ -189,7 +189,7 @@ public void disableLazyLoading_when_oneToManyMap() { .findOne(); // normally invokes lazy loading - assertThat(found.getRoles().size()).isEqualTo(0); + assertThrows(LazyInitialisationException.class, found::getRoles); // only 1 query ... no lazy loading query List sql = LoggedSql.stop(); @@ -217,12 +217,12 @@ public void testJoinOnPartiallyPopulatedParent_withLazyLoadingDisabled() { Order order = l0.get(0); // normally invokes lazy loading - order.getOrderDate(); + assertThrows(LazyInitialisationException.class, order::getOrderDate); List details = order.getDetails(); OrderDetail orderDetail = details.get(0); // normally invokes lazy loading - orderDetail.getShipQty(); + assertThrows(LazyInitialisationException.class, orderDetail::getShipQty); List loggedSql = LoggedSql.stop(); assertThat(loggedSql).hasSize(1); diff --git a/ebean-test/src/test/java/org/tests/model/aggregation/TestAggregationMany.java b/ebean-test/src/test/java/org/tests/model/aggregation/TestAggregationMany.java index dda574872a..4906ecc789 100644 --- a/ebean-test/src/test/java/org/tests/model/aggregation/TestAggregationMany.java +++ b/ebean-test/src/test/java/org/tests/model/aggregation/TestAggregationMany.java @@ -1,5 +1,6 @@ package org.tests.model.aggregation; +import io.ebean.LazyInitialisationException; import io.ebean.xtest.BaseTestCase; import io.ebean.DB; import io.ebean.test.LoggedSql; @@ -8,6 +9,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; public class TestAggregationMany extends BaseTestCase { @@ -26,7 +28,7 @@ public void fetchQuery_toAggregate() { for (DMachine machine : machines) { assertThat(machine.getAuxUseAggs()).isNotEmpty(); - assertThat(machine.getMachineStats()).isEmpty(); + assertThrows(LazyInitialisationException.class, machine::getMachineStats); } List sql = LoggedSql.stop(); @@ -57,7 +59,7 @@ public void fetch_toAggregate() { for (DMachine machine : machines) { assertThat(machine.getAuxUseAggs()).isNotEmpty(); - assertThat(machine.getMachineStats()).isEmpty(); + assertThrows(LazyInitialisationException.class, machine::getMachineStats); } List sql = LoggedSql.stop(); @@ -83,7 +85,7 @@ public void fetch_toAggregate_onlyAggColumnsInFetchProperties() { for (DMachine machine : machines) { assertThat(machine.getAuxUseAggs()).isNotEmpty(); System.out.println(machine); - assertThat(machine.getMachineStats()).isEmpty(); + assertThrows(LazyInitialisationException.class, machine::getMachineStats); } List sql = LoggedSql.stop(); diff --git a/ebean-test/src/test/java/org/tests/text/json/TestTextJsonReferenceBean.java b/ebean-test/src/test/java/org/tests/text/json/TestTextJsonReferenceBean.java index 1bf4e1f197..015282b02e 100644 --- a/ebean-test/src/test/java/org/tests/text/json/TestTextJsonReferenceBean.java +++ b/ebean-test/src/test/java/org/tests/text/json/TestTextJsonReferenceBean.java @@ -1,5 +1,6 @@ package org.tests.text.json; +import io.ebean.LazyInitialisationException; import io.ebean.text.json.JsonReadOptions; import io.ebean.xtest.BaseTestCase; import io.ebean.BeanState; @@ -19,8 +20,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; public class TestTextJsonReferenceBean extends BaseTestCase { @@ -33,7 +33,7 @@ void fromJson_refBean_shouldNotLazyLoadByDefault() { assertThat(DB.beanState(productRefBean).isReference()).isTrue(); // does not lazy load by default - assertThat(productRefBean.getName()).isNull(); + assertThrows(LazyInitialisationException.class, productRefBean::getName); } @Test