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 cd6fa560d..11f248783 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 52a7de908..47117c882 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 final class InterceptReadOnly extends InterceptBase { } - @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 56b2ab549..da477e286 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 @@ package io.ebean.bean; 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 final class InterceptReadWrite extends InterceptBase { 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 final class InterceptReadWrite extends InterceptBase { } } - @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 final class InterceptReadWrite extends InterceptBase { @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 feac1ab55..6a0cdbaab 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 final class DLoadContext implements LoadContext { 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 40d8317d4..0882b4414 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 @@ class SqlTreeLoadBean implements SqlTreeLoad { 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 db3c9622b..c8c0486ea 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.basic.EBasicVer; 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 class PlatformNoGeneratedKeysTest { .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 cc28e4807..5213ec719 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 @@ package org.tests.batchload; 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 @@ class TestBeanState extends BaseTestCase { 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 0ac8cc6be..35efdd0b9 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 class TestQueryDisableLazyLoad extends BaseTestCase { 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 class TestQueryDisableLazyLoad extends BaseTestCase { 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 class TestQueryDisableLazyLoad extends BaseTestCase { 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 e04a18c59..9b1a4a3dd 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.Arrays; 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 class TestQueryJoinToAssocOne extends BaseTestCase { 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 class TestQueryJoinToAssocOne extends BaseTestCase { 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 class TestQueryJoinToAssocOne extends BaseTestCase { .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 class TestQueryJoinToAssocOne extends BaseTestCase { .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 class TestQueryJoinToAssocOne extends BaseTestCase { 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 dda574872..4906ecc78 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 org.junit.jupiter.api.Test; 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 class TestAggregationMany extends BaseTestCase { 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 class TestAggregationMany extends BaseTestCase { 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 class TestAggregationMany extends BaseTestCase { 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 1bf4e1f19..015282b02 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.io.IOException; 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 @@ public class TestTextJsonReferenceBean extends BaseTestCase { assertThat(DB.beanState(productRefBean).isReference()).isTrue(); // does not lazy load by default - assertThat(productRefBean.getName()).isNull(); + assertThrows(LazyInitialisationException.class, productRefBean::getName); } @Test