Merge branch 'FOCONIS-test_beancache_wrong_result'

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<String, Object> 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;
   }
 
-
 }

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 {
    * </p>
    */
   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 {

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<T> extends BeanPropertyAssoc<T> 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;
     }

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);
 

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<Customer> 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<Contact> desc = getBeanDescriptor(Contact.class);
+    CachedBeanData cacheData = CachedBeanDataFromBean.extract(desc, entityBean);
+
+    final Map<String, Object> 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<Contact> desc = getBeanDescriptor(Contact.class);
+    CachedBeanData cacheData = CachedBeanDataFromBean.extract(desc, entityBean);
+
+    final Map<String, Object> 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());
+  }
 }

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.*:
+   * <ul>
+   *   <li>bean partially fetched via natural key, filling the cache</li>
+   *   <li>bean modified using a setter</li>
+   *   <li>getter called on non-loaded property to trigger lazy load</li>
+   *   <li>bean fetched again using the same natural key, to hit cache</li>
+   *   <li>it is expected, that the fetched bean does not contain the modification from before</li>
+   * </ul>
+   */
+  @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<String> 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");
+  }
+
+}