From de734665cd4e979050b6776d7241792b7367c8fe Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Thu, 24 Sep 2020 22:14:48 +1200 Subject: [PATCH] #2061 - Fix for L2 bean cache loaded using lazy loading on mutated partially loaded bean The fix is when the property is dirty (mutated) put the original value into the cache entry. --- .../server/cache/CachedBeanDataFromBean.java | 11 ++-- .../server/deploy/BeanProperty.java | 14 +++- .../server/deploy/BeanPropertyAssocOne.java | 13 +++- .../server/cache/CacheBeanDataTest.java | 2 +- .../cache/CachedBeanDataFromBeanTest.java | 66 ++++++++++++++++++- 5 files changed, 93 insertions(+), 13 deletions(-) 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()); + } }