Test and Fix for wrong values put into L2 cache when modifying a Json-Property before lazyLoading

Author: jonasPoehler

ebean-core/src/main/java/io/ebeaninternal/server/cache/CachedBeanDataFromBean.java

@@ -1,5 +1,6 @@
 package io.ebeaninternal.server.cache;
 
+import io.ebean.ModifyAwareType;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
@@ -26,16 +27,31 @@ public static CachedBeanData extract(BeanDescriptor<?> desc, EntityBean bean) {
     // extract all the non-many properties
     final boolean dirty = ebi.isDirty();
     for (BeanProperty prop : desc.propertiesNonMany()) {
-      if (dirty && ebi.isDirtyProperty(prop.propertyIndex())) {
+      if (!ebi.isLoadedProperty(prop.propertyIndex())) {
+        continue;
+      }
+      boolean useOriginalValue = false;
+      // For ModifyAwareType (especially JSON) there is a constellation where the entity might not yet be dirty, but the property
+      // has still changed. This would result in modified data being put into cache
+      if (ebi.value(prop.propertyIndex()) instanceof ModifyAwareType) {
+        useOriginalValue = ((ModifyAwareType) ebi.value(prop.propertyIndex())).isMarkedDirty();
+      } else if (dirty && ebi.isDirtyProperty(prop.propertyIndex())) {
+        useOriginalValue = true;
+      }
+      if (useOriginalValue) {
         data.put(prop.name(), prop.getCacheDataValueOrig(ebi));
-      } else if (ebi.isLoadedProperty(prop.propertyIndex())) {
+      } else {
         data.put(prop.name(), prop.getCacheDataValue(bean));
       }
     }
 
     for (BeanPropertyAssocMany<?> prop : desc.propertiesMany()) {
       if (prop.isElementCollection()) {
-        data.put(prop.name(), prop.getCacheDataValue(bean));
+        if (ebi.isChangedProperty(prop.propertyIndex())) {
+          data.put(prop.name(), prop.getCacheDataValueOrig(ebi));
+        } else {
+          data.put(prop.name(), prop.getCacheDataValue(bean));
+        }
       }
     }
 

ebean-test/src/test/java/org/tests/cache/TestQueryCache.java

@@ -10,9 +10,9 @@
 import io.ebean.cache.ServerCache;
 import io.ebean.test.LoggedSql;
 import io.ebean.xtest.BaseTestCase;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.tests.model.basic.Customer;
+import org.tests.model.basic.EBasicJsonCache;
 import org.tests.model.basic.ResetBasicData;
 import org.tests.model.cache.EColAB;
 
@@ -474,6 +474,56 @@ public void findIds() {
     assertThat(sql).hasSize(1);
   }
 
+  @Test
+  public void testDirtyUpCacheJsonMutationDetectionSource() {
+    EBasicJsonCache testDirty = new EBasicJsonCache();
+    testDirty.setName("testDirty");
+    testDirty.setDescription("desc");
+    testDirty.save();
+    DB.cacheManager().clearAll();
+
+    EBasicJsonCache fromDb = DB.find(EBasicJsonCache.class).select("name,tags").setId(testDirty.getId()).findOne();
+    assertThat(fromDb).isNotNull();
+    assertThat(fromDb.getName()).isEqualTo("testDirty");
+    assertThat(DB.beanState(fromDb).loadedProps()).containsExactlyInAnyOrder("id", "name", "tags");
+    fromDb.getTags().add("tagtag");
+
+    assertThat(DB.beanState(fromDb).loadedProps()).containsExactlyInAnyOrder("id", "name", "tags");
+
+    // trigger lazy load
+    assertThat(fromDb.getDescription()).isEqualTo("desc");
+
+    EBasicJsonCache fromDb2 = DB.find(EBasicJsonCache.class).setId(testDirty.getId()).findOne();
+    assertThat(fromDb2).isNotNull();
+    assertThat(fromDb2).isNotSameAs(fromDb);
+    assertThat(fromDb2.getTags()).isNullOrEmpty();
+  }
+
+  @Test
+  public void testDirtyUpCacheJsonMutationDetectionDefault() {
+    EBasicJsonCache testDirty = new EBasicJsonCache();
+    testDirty.setName("testDirty");
+    testDirty.setDescription("desc");
+    testDirty.save();
+    DB.cacheManager().clearAll();
+
+    EBasicJsonCache fromDb = DB.find(EBasicJsonCache.class).select("name,superTags").setId(testDirty.getId()).findOne();
+    assertThat(fromDb).isNotNull();
+    assertThat(fromDb.getName()).isEqualTo("testDirty");
+    assertThat(DB.beanState(fromDb).loadedProps()).containsExactlyInAnyOrder("id", "name", "superTags");
+    fromDb.getSuperTags().add("tagtag");
+
+    assertThat(DB.beanState(fromDb).loadedProps()).containsExactlyInAnyOrder("id", "name", "superTags");
+
+    // trigger lazy load
+    assertThat(fromDb.getDescription()).isEqualTo("desc");
+
+    EBasicJsonCache fromDb2 = DB.find(EBasicJsonCache.class).setId(testDirty.getId()).findOne();
+    assertThat(fromDb2).isNotNull();
+    assertThat(fromDb2).isNotSameAs(fromDb);
+    assertThat(fromDb2.getSuperTags()).isNullOrEmpty();
+  }
+
   @Test
   public void findCountDifferentQueriesBit() {
     DB.getDefault().pluginApi().cacheManager().clearAll();

ebean-test/src/test/java/org/tests/model/basic/EBasicJsonCache.java

@@ -0,0 +1,80 @@
+package org.tests.model.basic;
+
+import io.ebean.Model;
+import io.ebean.annotation.Cache;
+import io.ebean.annotation.DbJson;
+import io.ebean.annotation.Identity;
+import io.ebean.annotation.MutationDetection;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Table;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static io.ebean.annotation.IdentityGenerated.BY_DEFAULT;
+
+/**
+ * Test entity for making sure json mutation detection works with cache correctly.
+ *
+ * @author Jonas Pöhler, Foconis Analytics GmbH
+ */
+@Cache(enableQueryCache = true)
+@Entity
+@Table(name = "e_basic_json_cache")
+public class EBasicJsonCache extends Model {
+
+  @Id
+  @Identity(generated = BY_DEFAULT)
+  Integer id;
+
+  String name;
+
+  String description;
+
+  @DbJson(mutationDetection = MutationDetection.SOURCE)
+  List<String> tags = new ArrayList<>();
+
+  @DbJson(mutationDetection = MutationDetection.DEFAULT)
+  List<String> superTags = new ArrayList<>();
+
+  public Integer getId() {
+    return id;
+  }
+
+  public void setId(Integer id) {
+    this.id = id;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public String getDescription() {
+    return description;
+  }
+
+  public void setDescription(String description) {
+    this.description = description;
+  }
+
+  public List<String> getTags() {
+    return tags;
+  }
+
+  public void setTags(List<String> tags) {
+    this.tags = tags;
+  }
+
+  public List<String> getSuperTags() {
+    return superTags;
+  }
+
+  public void setSuperTags(List<String> superTags) {
+    this.superTags = superTags;
+  }
+}