diff --git a/src/main/java/io/ebeaninternal/server/cache/CachedBeanDataFromBean.java b/src/main/java/io/ebeaninternal/server/cache/CachedBeanDataFromBean.java index c91fee62d..32ac2ce9a 100644 --- a/src/main/java/io/ebeaninternal/server/cache/CachedBeanDataFromBean.java +++ b/src/main/java/io/ebeaninternal/server/cache/CachedBeanDataFromBean.java @@ -11,11 +11,9 @@ import java.util.Map; public class CachedBeanDataFromBean { - public static CachedBeanData extract(BeanDescriptor desc, EntityBean bean) { EntityBeanIntercept ebi = bean._ebean_getIntercept(); - Map data = new LinkedHashMap<>(); BeanProperty idProperty = desc.getIdProperty(); @@ -25,11 +23,13 @@ public class CachedBeanDataFromBean { data.put(idProperty.getName(), idProperty.getCacheDataValue(bean)); } } - BeanProperty[] props = desc.propertiesNonMany(); // extract all the non-many properties - for (BeanProperty prop : props) { - if (ebi.isLoadedProperty(prop.getPropertyIndex())) { + final boolean dirty = ebi.isDirty(); + for (BeanProperty prop : desc.propertiesNonMany()) { + if (dirty && ebi.isDirtyProperty(prop.getPropertyIndex())) { + data.put(prop.getName(), prop.getCacheDataValueOrig(ebi)); + } else if (ebi.isLoadedProperty(prop.getPropertyIndex())) { data.put(prop.getName(), prop.getCacheDataValue(bean)); } } @@ -72,5 +72,4 @@ public class CachedBeanDataFromBean { return sharableBean; } - } diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 11d11d67e..31b3bc810 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -3,6 +3,7 @@ package io.ebeaninternal.server.deploy; import com.fasterxml.jackson.core.JsonToken; import io.ebean.ValuePair; import io.ebean.bean.EntityBean; +import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; import io.ebean.config.EncryptKey; import io.ebean.config.dbplatform.DbEncryptFunction; @@ -10,7 +11,6 @@ import io.ebean.config.dbplatform.DbPlatformType; import io.ebean.plugin.Property; import io.ebean.text.StringParser; import io.ebean.util.SplitName; -import io.ebean.util.StringHelper; import io.ebeaninternal.api.SpiExpressionRequest; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.api.json.SpiJsonReader; @@ -794,7 +794,17 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { *

*/ public Object getCacheDataValue(EntityBean bean) { - Object value = getValue(bean); + return cacheDataConvert(getValue(bean)); + } + + /** + * Return the bean cache value for this property using original values. + */ + public Object getCacheDataValueOrig(EntityBeanIntercept ebi) { + return cacheDataConvert(ebi.getOrigValue(propertyIndex)); + } + + private Object cacheDataConvert(Object value) { if (value == null || scalarType.isBinaryType()) { return value; } else { diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index d33dd95c2..59404d6b1 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -5,6 +5,7 @@ import io.ebean.SqlUpdate; import io.ebean.Transaction; import io.ebean.ValuePair; import io.ebean.bean.EntityBean; +import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; import io.ebean.util.SplitName; import io.ebeaninternal.api.SpiEbeanServer; @@ -426,9 +427,19 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr return getPropertyType(); } + /** + * Return the bean cache value for this property using original values. + */ + public Object getCacheDataValueOrig(EntityBeanIntercept ebi) { + return cacheDataConvert(ebi.getOrigValue(propertyIndex)); + } + @Override public Object getCacheDataValue(EntityBean bean) { - Object ap = getValue(bean); + return cacheDataConvert(getValue(bean)); + } + + private Object cacheDataConvert(Object ap) { if (ap == null) { return null; } diff --git a/src/test/java/io/ebeaninternal/server/cache/CacheBeanDataTest.java b/src/test/java/io/ebeaninternal/server/cache/CacheBeanDataTest.java index 2ba1b9747..957515f80 100644 --- a/src/test/java/io/ebeaninternal/server/cache/CacheBeanDataTest.java +++ b/src/test/java/io/ebeaninternal/server/cache/CacheBeanDataTest.java @@ -43,7 +43,7 @@ public class CacheBeanDataTest extends BaseTestCase { billingAddress.setLine1("92 Someplace Else"); c.setBillingAddress(billingAddress); - ((EntityBean) c)._ebean_getIntercept().setNewBeanForUpdate(); + ((EntityBean) c)._ebean_getIntercept().setLoaded(); CachedBeanData cacheData = CachedBeanDataFromBean.extract(desc, (EntityBean) c); diff --git a/src/test/java/io/ebeaninternal/server/cache/CachedBeanDataFromBeanTest.java b/src/test/java/io/ebeaninternal/server/cache/CachedBeanDataFromBeanTest.java index 1490bead6..323fcc2e0 100644 --- a/src/test/java/io/ebeaninternal/server/cache/CachedBeanDataFromBeanTest.java +++ b/src/test/java/io/ebeaninternal/server/cache/CachedBeanDataFromBeanTest.java @@ -5,21 +5,24 @@ import io.ebean.bean.EntityBean; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.transaction.DefaultPersistenceContext; +import org.junit.Test; import org.tests.model.basic.Address; import org.tests.model.basic.Car; +import org.tests.model.basic.Contact; import org.tests.model.basic.Customer; -import org.junit.Test; import java.sql.Date; +import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; public class CachedBeanDataFromBeanTest extends BaseTestCase { - SpiEbeanServer server = spiEbeanServer(); + private final SpiEbeanServer server = spiEbeanServer(); @Test - public void extract() throws Exception { + public void extract() { BeanDescriptor desc = server.getBeanDescriptor(Customer.class); @@ -64,4 +67,61 @@ public class CachedBeanDataFromBeanTest extends BaseTestCase { assertEquals(newCar.getDriver(), car.getDriver()); assertEquals(newCar.getNotes(), car.getNotes()); } + + @SuppressWarnings("unchecked") + @Test + public void dirtyScalar_expect_originalValueUsed() { + + Contact contact = new Contact(); + contact.setId(42); + contact.setLastName("Bygrave"); + contact.setFirstName("Foo"); + contact.setEmail("rob@email.com"); + + EntityBean entityBean = (EntityBean)contact; + entityBean._ebean_getIntercept().setLoaded(); + + // mutate, dirty + contact.setLastName("Banana"); + + final BeanDescriptor desc = getBeanDescriptor(Contact.class); + CachedBeanData cacheData = CachedBeanDataFromBean.extract(desc, entityBean); + + final Map data = cacheData.getData(); + assertThat(data.get("id")).isEqualTo("42"); + assertThat(data.get("lastName")).isEqualTo("Bygrave"); // ORIGINAL VALUE + assertThat(data.get("firstName")).isEqualTo(contact.getFirstName()); + assertThat(data.get("email")).isEqualTo(contact.getEmail()); + } + + @Test + public void dirtyManyToOne_expect_originalValueUsed() { + + Customer customer = new Customer(); + customer.setId(99); + + Contact contact = new Contact(); + contact.setFirstName("Foo"); + contact.setLastName("Bygrave"); + contact.setEmail("rob@email.com"); + contact.setCustomer(customer); + + EntityBean entityBean = (EntityBean)contact; + entityBean._ebean_getIntercept().setLoaded(); + + // mutate, dirty + Customer customer2 = new Customer(); + customer2.setId(108); + contact.setCustomer(customer2); + contact.setLastName("Banana"); + + final BeanDescriptor desc = getBeanDescriptor(Contact.class); + CachedBeanData cacheData = CachedBeanDataFromBean.extract(desc, entityBean); + + final Map data = cacheData.getData(); + assertThat(data.get("lastName")).isEqualTo("Bygrave"); // Original value + assertThat(data.get("customer")).isEqualTo("99"); // Original value + assertThat(data.get("firstName")).isEqualTo(contact.getFirstName()); + assertThat(data.get("email")).isEqualTo(contact.getEmail()); + } } diff --git a/src/test/java/org/tests/cache/TestBeanCacheContactLazyLoad.java b/src/test/java/org/tests/cache/TestBeanCacheContactLazyLoad.java new file mode 100644 index 000000000..9eb438032 --- /dev/null +++ b/src/test/java/org/tests/cache/TestBeanCacheContactLazyLoad.java @@ -0,0 +1,68 @@ +package org.tests.cache; + +import io.ebean.BaseTestCase; +import io.ebean.DB; +import io.ebeantest.LoggedSql; +import org.junit.Test; +import org.tests.model.basic.Contact; +import org.tests.model.basic.Customer; + +import java.sql.Date; +import java.time.LocalDate; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Test class testing a wrong behaviour of the bean cache. + */ +public class TestBeanCacheContactLazyLoad extends BaseTestCase { + + /** + * This test shows a wrong behaviour of the bean cache up to at least Ebean 12.4.*: + *
    + *
  • bean partially fetched via natural key, filling the cache
    • + *
  • bean modified using a setter
    • + *
  • getter called on non-loaded property to trigger lazy load
    • + *
  • bean fetched again using the same natural key, to hit cache
    • + *
  • it is expected, that the fetched bean does not contain the modification from before
    • + *
+ */ + @Test + public void testBeanCacheWithLazyLoading() { + final Customer customer = new Customer(); + customer.setName("Customer"); + customer.setAnniversary(Date.valueOf(LocalDate.of(2010, 1, 1))); + DB.save(customer); + + final Contact contact = new Contact(); + contact.setFirstName("Tim"); + contact.setLastName("Button"); + contact.setPhone("1234567890"); + contact.setMobile("4567890123"); + contact.setEmail("tim@button.com"); + contact.setCustomer(customer); + + DB.save(contact); + + // Only get two properties, so we have to lazy-load later + final Contact contactDb = DB.find(Contact.class).where().eq("email", "tim@button.com").select("email,lastName").findOne(); + assertThat(contactDb).isNotNull(); + LoggedSql.start(); + contactDb.setLastName("Buttonnnn"); + List sql = LoggedSql.collect(); + assertThat(sql).isEmpty(); // setter did not trigger lazy load + + // trigger lazy load + assertThat(contactDb.getPhone()).isEqualTo("1234567890"); + sql = LoggedSql.collect(); + assertThat(sql).isNotEmpty(); // Lazy-load took place + + final Contact contactDb2 = DB.find(Contact.class).where().eq("email", "tim@button.com").select("email,lastName").findOne(); + sql = LoggedSql.stop(); + assertThat(sql).isEmpty(); // We expect that the bean was loaded from cache + assertThat(contactDb2).isNotNull(); + assertThat(contactDb2.getLastName()).isEqualTo("Button"); + } + +}