diff --git a/src/main/java/io/ebean/bean/EntityBeanIntercept.java b/src/main/java/io/ebean/bean/EntityBeanIntercept.java index 8d23fdbcc..1f0656eec 100644 --- a/src/main/java/io/ebean/bean/EntityBeanIntercept.java +++ b/src/main/java/io/ebean/bean/EntityBeanIntercept.java @@ -1,6 +1,7 @@ package io.ebean.bean; import io.ebean.DB; +import io.ebean.Database; import io.ebean.ValuePair; import javax.persistence.EntityNotFoundException; @@ -104,9 +105,6 @@ public final class EntityBeanIntercept implements Serializable { /** * Create a intercept with a given entity. - *

- * Refer to agent ProxyConstructor. - *

*/ public EntityBeanIntercept(Object ownerBean) { this.owner = (EntityBean) ownerBean; @@ -821,14 +819,14 @@ public final class EntityBeanIntercept implements Serializable { synchronized (this) { if (beanLoader == null) { - BeanLoader serverLoader = (BeanLoader) DB.byName(ebeanServerName); - if (serverLoader == null) { - throw new PersistenceException("Server [" + ebeanServerName + "] was not found?"); + final Database database = DB.byName(ebeanServerName); + if (database == null) { + throw new PersistenceException("Database [" + ebeanServerName + "] was not found?"); } // For stand alone reference bean or after deserialisation lazy load // using the ebeanServer. Synchronise only on the bean. - loadBeanInternal(loadProperty, serverLoader); + loadBeanInternal(loadProperty, database.getPluginApi()); return; } } diff --git a/src/main/java/io/ebean/bean/SingleBeanLoader.java b/src/main/java/io/ebean/bean/SingleBeanLoader.java new file mode 100644 index 000000000..fd4ce6651 --- /dev/null +++ b/src/main/java/io/ebean/bean/SingleBeanLoader.java @@ -0,0 +1,48 @@ +package io.ebean.bean; + +import io.ebean.Database; + +/** + * BeanLoader used when single beans are loaded (which is usually not ideal / N+1). + */ +public abstract class SingleBeanLoader implements BeanLoader { + + protected final Database database; + + SingleBeanLoader(Database database) { + this.database = database; + } + + @Override + public String getName() { + return database.getName(); + } + + /** + * Single bean lazy loaded when bean from L2 cache. + */ + public static class L2 extends SingleBeanLoader { + public L2(Database database) { + super(database); + } + + @Override + public void loadBean(EntityBeanIntercept ebi) { + database.getPluginApi().loadBeanL2(ebi); + } + } + + /** + * Single bean lazy loaded when a reference bean. + */ + public static class Ref extends SingleBeanLoader { + public Ref(Database database) { + super(database); + } + + @Override + public void loadBean(EntityBeanIntercept ebi) { + database.getPluginApi().loadBeanRef(ebi); + } + } +} diff --git a/src/main/java/io/ebean/plugin/SpiServer.java b/src/main/java/io/ebean/plugin/SpiServer.java index d7bf69b62..e672158d8 100644 --- a/src/main/java/io/ebean/plugin/SpiServer.java +++ b/src/main/java/io/ebean/plugin/SpiServer.java @@ -1,6 +1,8 @@ package io.ebean.plugin; import io.ebean.EbeanServer; +import io.ebean.bean.BeanLoader; +import io.ebean.bean.EntityBeanIntercept; import io.ebean.config.ServerConfig; import io.ebean.config.dbplatform.DatabasePlatform; @@ -10,7 +12,7 @@ import java.util.List; /** * Extensions to Database API made available to plugins. */ -public interface SpiServer extends EbeanServer { +public interface SpiServer extends EbeanServer, BeanLoader { /** * Return the serverConfig. @@ -52,4 +54,19 @@ public interface SpiServer extends EbeanServer { */ DataSource getReadOnlyDataSource(); + /** + * Invoke lazy loading on this single bean (reference bean). + */ + void loadBeanRef(EntityBeanIntercept ebi); + + /** + * Invoke lazy loading on this single bean (L2 cache bean). + */ + void loadBeanL2(EntityBeanIntercept ebi); + + /** + * Invoke lazy loading on this single bean when no BeanLoader is set. + * Typically due to serialisation or multiple stateless updates. + */ + void loadBean(EntityBeanIntercept ebi); } diff --git a/src/main/java/io/ebeaninternal/api/ExtraMetrics.java b/src/main/java/io/ebeaninternal/api/ExtraMetrics.java index dac31b2a9..2d950c40a 100644 --- a/src/main/java/io/ebeaninternal/api/ExtraMetrics.java +++ b/src/main/java/io/ebeaninternal/api/ExtraMetrics.java @@ -2,6 +2,7 @@ package io.ebeaninternal.api; import io.ebean.meta.MetricType; import io.ebean.meta.MetricVisitor; +import io.ebean.metric.CountMetric; import io.ebean.metric.MetricFactory; import io.ebean.metric.TimedMetric; @@ -12,6 +13,9 @@ public class ExtraMetrics { private final TimedMetric bindCapture; private final TimedMetric planCollect; + private final CountMetric loadOneL2; + private final CountMetric loadOneRef; + private final CountMetric loadOneNoLoader; /** * Create the extra metrics. @@ -20,6 +24,9 @@ public class ExtraMetrics { final MetricFactory factory = MetricFactory.get(); this.bindCapture = factory.createTimedMetric(MetricType.ORM, "ebean.queryplan.bindcapture"); this.planCollect = factory.createTimedMetric(MetricType.ORM, "ebean.queryplan.collect"); + this.loadOneL2 = factory.createCountMetric(MetricType.ORM, "loadone.l2"); + this.loadOneRef = factory.createCountMetric(MetricType.ORM, "loadone.ref"); + this.loadOneNoLoader = factory.createCountMetric(MetricType.ORM, "loadone.noloader"); } /** @@ -36,11 +43,38 @@ public class ExtraMetrics { return planCollect; } + /** + * Increment counter for lazy loading one bean from L2 cache. + * All good when lazy loading also hits L2 cache. + */ + public void incrementLoadOneL2() { + loadOneL2.increment(); + } + + /** + * Increment counter for lazy loading on reference bean. + * We ought to be able to avoid this by changing to a tuned query. + */ + public void incrementLoadOneRef() { + loadOneRef.increment(); + } + + /** + * Increment counter for lazy loading one bean due to no loader. + * Likely due to multiple stateless updates or serialisation. + */ + public void incrementLoadOneNoLoader() { + loadOneNoLoader.increment(); + } + /** * Collect the metrics. */ public void visitMetrics(MetricVisitor visitor) { bindCapture.visit(visitor); planCollect.visit(visitor); + loadOneL2.visit(visitor); + loadOneRef.visit(visitor); + loadOneNoLoader.visit(visitor); } } diff --git a/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java b/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java index 36b1f3901..43424c0c5 100644 --- a/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java +++ b/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java @@ -31,7 +31,7 @@ import java.util.function.Predicate; /** * Service Provider extension to EbeanServer. */ -public interface SpiEbeanServer extends ExtendedServer, EbeanServer, BeanLoader, BeanCollectionLoader { +public interface SpiEbeanServer extends ExtendedServer, EbeanServer, BeanCollectionLoader { /** * Return the log manager. diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 402d0eb61..0df57d6d7 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -580,6 +580,19 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { @Override public void loadBean(EntityBeanIntercept ebi) { beanLoader.loadBean(ebi); + extraMetrics.incrementLoadOneNoLoader(); + } + + @Override + public void loadBeanRef(EntityBeanIntercept ebi) { + beanLoader.loadBean(ebi); + extraMetrics.incrementLoadOneRef(); + } + + @Override + public void loadBeanL2(EntityBeanIntercept ebi) { + beanLoader.loadBean(ebi); + extraMetrics.incrementLoadOneL2(); } @Override diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index 3cdffe723..190e29e77 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -10,6 +10,7 @@ import io.ebean.bean.BeanCollection; import io.ebean.bean.EntityBean; import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; +import io.ebean.bean.SingleBeanLoader; import io.ebean.cache.QueryCacheEntry; import io.ebean.config.EncryptKey; import io.ebean.config.ServerConfig; @@ -2040,7 +2041,7 @@ public class BeanDescriptor implements BeanType, STreeType { if (disableLazyLoad) { ebi.setDisableLazyLoad(true); } else { - ebi.setBeanLoader(ebeanServer); + ebi.setBeanLoader(refBeanLoader()); } ebi.setReference(idPropertyIndex); if (Boolean.TRUE == readOnly) { @@ -2058,6 +2059,14 @@ public class BeanDescriptor implements BeanType, STreeType { } } + SingleBeanLoader refBeanLoader() { + return new SingleBeanLoader.Ref(ebeanServer); + } + + SingleBeanLoader l2BeanLoader() { + return new SingleBeanLoader.L2(ebeanServer); + } + /** * Create a non read only reference bean without checking cacheSharableBeans. */ @@ -2072,7 +2081,7 @@ public class BeanDescriptor implements BeanType, STreeType { EntityBean eb = createEntityBean(); id = convertSetId(id, eb); EntityBeanIntercept ebi = eb._ebean_getIntercept(); - ebi.setBeanLoader(ebeanServer); + ebi.setBeanLoader(refBeanLoader()); ebi.setReference(idPropertyIndex); if (pc != null) { contextPut(pc, id, eb); diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java index fc5de7082..c59d00063 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java @@ -690,7 +690,7 @@ final class BeanDescriptorCacheHelp { EntityBeanIntercept ebi = bean._ebean_getIntercept(); // Not using loadContext here so no batch lazy loading for these beans - ebi.setBeanLoader(desc.getEbeanServer()); + ebi.setBeanLoader(desc.l2BeanLoader()); if (Boolean.TRUE.equals(readOnly)) { ebi.setReadOnly(true); } diff --git a/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java b/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java index 1b7f209e5..3b07c3165 100644 --- a/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java +++ b/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java @@ -323,10 +323,6 @@ public class TDSpiEbeanServer implements SpiEbeanServer { public void loadMany(BeanCollection collection, boolean onlyIds) { } - @Override - public void loadBean(EntityBeanIntercept ebi) { - } - @Override public void shutdown(boolean shutdownDataSource, boolean deregisterDriver) { } diff --git a/src/test/java/io/ebeaninternal/server/deploy/TestReferenceWithConstructorProperties.java b/src/test/java/io/ebeaninternal/server/deploy/TestReferenceWithConstructorProperties.java index 6493f1abd..6c1626ce5 100644 --- a/src/test/java/io/ebeaninternal/server/deploy/TestReferenceWithConstructorProperties.java +++ b/src/test/java/io/ebeaninternal/server/deploy/TestReferenceWithConstructorProperties.java @@ -3,13 +3,17 @@ package io.ebeaninternal.server.deploy; import io.ebean.BaseTestCase; import io.ebean.BeanState; import io.ebean.Ebean; +import org.junit.Test; import org.tests.model.basic.Order; import org.tests.model.basic.ResetBasicData; -import org.junit.Assert; -import org.junit.Test; import java.util.Set; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + public class TestReferenceWithConstructorProperties extends BaseTestCase { /** @@ -24,14 +28,14 @@ public class TestReferenceWithConstructorProperties extends BaseTestCase { BeanState beanState = Ebean.getBeanState(order); Set loadedProps = beanState.getLoadedProps(); - Assert.assertEquals(1, loadedProps.size()); - Assert.assertTrue(beanState.isReference()); + assertEquals(1, loadedProps.size()); + assertTrue(beanState.isReference()); // read the status invokes lazy loading order.getStatus(); - Assert.assertFalse(beanState.isReference()); - + assertFalse(beanState.isReference()); + assertNotNull(order.getCustomer()); } } diff --git a/src/test/java/org/tests/cache/TestCacheCollectionIds.java b/src/test/java/org/tests/cache/TestCacheCollectionIds.java index c57811889..5cff419c4 100644 --- a/src/test/java/org/tests/cache/TestCacheCollectionIds.java +++ b/src/test/java/org/tests/cache/TestCacheCollectionIds.java @@ -9,16 +9,24 @@ import io.ebean.Update; import io.ebean.cache.ServerCache; import io.ebean.cache.ServerCacheManager; import io.ebeaninternal.server.cache.CachedManyIds; -import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; -import org.tests.model.basic.*; +import org.tests.model.basic.Contact; +import org.tests.model.basic.Country; +import org.tests.model.basic.Customer; +import org.tests.model.basic.OBeanChild; +import org.tests.model.basic.OCachedBean; +import org.tests.model.basic.OCachedBeanChild; +import org.tests.model.basic.Order; +import org.tests.model.basic.OrderDetail; +import org.tests.model.basic.ResetBasicData; import java.util.ArrayList; import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public class TestCacheCollectionIds extends BaseTestCase { @@ -88,7 +96,7 @@ public class TestCacheCollectionIds extends BaseTestCase { List contacts2 = customer2.getContacts(); for (Contact contact : contacts2) { - contact.getFirstName(); + assertNotNull(contact.getFirstName()); contact.getEmail(); } return contacts2.size(); @@ -297,7 +305,7 @@ public class TestCacheCollectionIds extends BaseTestCase { // assert that the cache contains the expected entry assertEquals("countries cache now loaded with 1 entry", 1, cachedBeanCountriesCache.size()); CachedManyIds dummyEntry = (CachedManyIds) cachedBeanCountriesCache.get(dummyLoad.getId()); - Assert.assertNotNull(dummyEntry); + assertNotNull(dummyEntry); assertEquals("2 ids in the entry", 2, dummyEntry.getIdList().size()); assertTrue(dummyEntry.getIdList().contains("NZ")); assertTrue(dummyEntry.getIdList().contains("AU")); diff --git a/src/test/java/org/tests/cache/TestL2CacheWithSharedBean.java b/src/test/java/org/tests/cache/TestL2CacheWithSharedBean.java index 3e00c2d74..647fbf742 100644 --- a/src/test/java/org/tests/cache/TestL2CacheWithSharedBean.java +++ b/src/test/java/org/tests/cache/TestL2CacheWithSharedBean.java @@ -12,6 +12,8 @@ import org.tests.model.basic.FeatureDescription; import org.junit.Assert; import org.junit.Test; +import static org.junit.Assert.assertEquals; + public class TestL2CacheWithSharedBean extends BaseTestCase { private TunedQueryInfo createTunedQueryInfo(OrmQueryDetail tunedDetail) { @@ -25,7 +27,7 @@ public class TestL2CacheWithSharedBean extends BaseTestCase { FeatureDescription f1 = new FeatureDescription(); f1.setName("one"); - f1.setDescription(null); + f1.setDescription("helloOne"); Ebean.save(f1); @@ -44,19 +46,20 @@ public class TestL2CacheWithSharedBean extends BaseTestCase { FeatureDescription fd2 = query.findOne(); // LOAD cache - fd2.getDescription(); // invoke lazy load (this fails) + String description0 = fd2.getDescription(); // invoke lazy load + assertEquals("helloOne", description0); // load the cache FeatureDescription fetchOne = Ebean.find(FeatureDescription.class, f1.getId()); Assert.assertNotNull(fetchOne); - Assert.assertEquals(1, beanCache.getStatistics(false).getSize()); + assertEquals(1, beanCache.getStatistics(false).getSize()); FeatureDescription fetchTwo = Ebean.find(FeatureDescription.class, f1.getId()); FeatureDescription fetchThree = Ebean.find(FeatureDescription.class, f1.getId()); Assert.assertSame(fetchTwo, fetchThree); - String description = fetchThree.getDescription(); - Assert.assertNull(description); + String description1 = fetchThree.getDescription(); + assertEquals("helloOne", description1); } }