Merge branch 'FOCONIS-feature/json-dirty-detection-md5' into feature/json-dirty-detection-md5

ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java

@@ -51,6 +51,11 @@ public final class EntityBeanIntercept implements Serializable {
    */
   private static final byte FLAG_ORIG_VALUE_SET = 8;
 
+  /**
+   * Flags indicating if the mutable hash is set.
+   */
+  private static final byte FLAG_MUTABLE_HASH_SET = 16;
+  
   private transient final ReentrantLock lock = new ReentrantLock();
   private transient NodeUsageCollector nodeUsageCollector;
   private transient PersistenceContext persistenceContext;
@@ -94,7 +99,7 @@ public final class EntityBeanIntercept implements Serializable {
   /**
    * Holds MD5 hash of json loaded jackson beans.
    */
-  private String[] mutableHash;
+  private MutableHash[] mutableHash;
 
   /**
    * Holds json content determined at point of dirty check.
@@ -239,6 +244,17 @@ public final class EntityBeanIntercept implements Serializable {
    * if any embedded beans are either new or dirty (and hence need saving).
    */
   public boolean isDirty() {
+    if (dirty) {
+      return true;
+    }
+    if (mutableHash != null) {
+      for (int i = 0; i < mutableHash.length; i++) {
+        if (mutableHash[i] != null && !mutableHash[i].isEqualToObject(owner._ebean_getField(i))) {
+          dirty = true;
+          break;
+        }
+      }
+    }
     return dirty;
   }
 
@@ -379,8 +395,9 @@ public final class EntityBeanIntercept implements Serializable {
     this.owner._ebean_setEmbeddedLoaded();
     this.lazyLoadProperty = -1;
     this.origValues = null;
+    this.mutableContent = null;
     for (int i = 0; i < flags.length; i++) {
-      flags[i] &= ~(FLAG_CHANGED_PROP + FLAG_ORIG_VALUE_SET);
+      flags[i] &= ~(FLAG_CHANGED_PROP | FLAG_ORIG_VALUE_SET);
     }
     this.dirty = false;
   }
@@ -453,6 +470,10 @@ public final class EntityBeanIntercept implements Serializable {
    * Return the original value that was changed via an update.
    */
   public Object getOrigValue(int propertyIndex) {
+    if ((flags[propertyIndex] & (FLAG_ORIG_VALUE_SET | FLAG_MUTABLE_HASH_SET)) == FLAG_MUTABLE_HASH_SET) {
+      // mutable hash set, but not ORIG_VALUE
+      setOriginalValue(propertyIndex, mutableHash[propertyIndex].get());
+    }
     if (origValues == null) {
       return null;
     }
@@ -649,14 +670,14 @@ public final class EntityBeanIntercept implements Serializable {
   public void addDirtyPropertyNames(Set<String> props, String prefix) {
     int len = getPropertyLength();
     for (int i = 0; i < len; i++) {
-      if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+      if (isChangedProp(i)) {
         // the property has been changed on this bean
         props.add((prefix == null ? getProperty(i) : prefix + getProperty(i)));
       } else if ((flags[i] & FLAG_EMBEDDED_DIRTY) != 0) {
         // an embedded property has been changed - recurse
         EntityBean embeddedBean = (EntityBean) owner._ebean_getField(i);
         embeddedBean._ebean_getIntercept().addDirtyPropertyNames(props, getProperty(i) + ".");
-      }
+      } 
     }
   }
 
@@ -667,7 +688,7 @@ public final class EntityBeanIntercept implements Serializable {
     String[] names = owner._ebean_getPropertyNames();
     int len = getPropertyLength();
     for (int i = 0; i < len; i++) {
-      if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+      if (isChangedProp(i)) {
         if (propertyNames.contains(names[i])) {
           return true;
         }
@@ -695,7 +716,8 @@ public final class EntityBeanIntercept implements Serializable {
   public void addDirtyPropertyValues(Map<String, ValuePair> dirtyValues, String prefix) {
     int len = getPropertyLength();
     for (int i = 0; i < len; i++) {
-      if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+   
+      if (isChangedProp(i)) {
         // the property has been changed on this bean
         String propName = (prefix == null ? getProperty(i) : prefix + getProperty(i));
         Object newVal = owner._ebean_getField(i);
@@ -717,7 +739,7 @@ public final class EntityBeanIntercept implements Serializable {
   public void addDirtyPropertyValues(BeanDiffVisitor visitor) {
     int len = getPropertyLength();
     for (int i = 0; i < len; i++) {
-      if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+      if (isChangedProp(i)) {
         // the property has been changed on this bean
         Object newVal = owner._ebean_getField(i);
         Object oldVal = getOrigValue(i);
@@ -752,7 +774,7 @@ public final class EntityBeanIntercept implements Serializable {
     }
     int len = getPropertyLength();
     for (int i = 0; i < len; i++) {
-      if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+      if ((flags[i] & FLAG_CHANGED_PROP) != 0) { // we do not check against mutablecontent here.
         sb.append(i).append(',');
       } else if ((flags[i] & FLAG_EMBEDDED_DIRTY) != 0) {
         // an embedded property has been changed - recurse
@@ -1150,14 +1172,29 @@ public final class EntityBeanIntercept implements Serializable {
     return ret;
   }
 
-  public String mutableHash(int propertyIndex) {
+  private boolean isChangedProp(int i) {
+    if ((flags[i] & FLAG_CHANGED_PROP) != 0) {
+      return true;
+    } else if (mutableHash == null || mutableHash[i] == null
+        || mutableHash[i].isEqualToObject(owner._ebean_getField(i))) {
+      return false;
+    } else {
+      // mark for change 
+      flags[i] |= FLAG_CHANGED_PROP;
+      dirty = true; // this makes the bean automatically dirty!
+      return true;
+    }
+  }
+  
+  public MutableHash mutableHash(int propertyIndex) {
     return mutableHash == null ? null : mutableHash[propertyIndex];
   }
 
-  public void mutableHash(int propertyIndex, String content) {
+  public void mutableHash(int propertyIndex, MutableHash content) {
     if (mutableHash == null) {
-      mutableHash = new String[flags.length];
+      mutableHash = new MutableHash[flags.length];
     }
+    flags[propertyIndex] |= FLAG_MUTABLE_HASH_SET;
     mutableHash[propertyIndex] = content;
   }
 

ebean-api/src/main/java/io/ebean/bean/MutableHash.java

@@ -0,0 +1,28 @@
+package io.ebean.bean;
+
+/**
+ * Interface to for mutable information in EntityBeanIntercept.
+ */
+public interface MutableHash {
+  
+  /**
+   * Compares the given json to an internal value. Can be a MD5 hash or a plain JSON string. 
+   * @return true if the value matches the hash.
+   */
+  boolean isEqualToJson(String json);
+
+  /**
+   * Compares the given object to an internal value. This is an optional method, but required for proper changelog/beanState support.
+   * The implementation can serialize the object and compare it against the original json.
+   */
+  default boolean isEqualToObject(Object obj) {
+    return true;
+  }
+  /**
+   * Creates a new instance from the internal json string. This is an optional method.
+   */
+  default Object get() {
+    return null;
+  }
+}
+ 

ebean-core-type/src/main/java/io/ebean/core/type/ScalarType.java

@@ -2,6 +2,8 @@ package io.ebean.core.type;
 
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
+
+import io.ebean.bean.MutableHash;
 import io.ebean.text.StringFormatter;
 import io.ebean.text.StringParser;
 
@@ -42,6 +44,10 @@ public interface ScalarType<T> extends StringParser, StringFormatter, ScalarData
     throw new UnsupportedOperationException();
   }
 
+  default MutableHash createMutableHash(String json) {
+    throw new UnsupportedOperationException();
+  }
+
   /**
    * Return true if this is a binary type and can not support parse() and format() from/to string.
    * This allows Ebean to optimise marshalling types to string.

ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java

@@ -4,6 +4,7 @@ import com.fasterxml.jackson.core.JsonToken;
 import io.ebean.ValuePair;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
+import io.ebean.bean.MutableHash;
 import io.ebean.bean.PersistenceContext;
 import io.ebean.config.EncryptKey;
 import io.ebean.config.dbplatform.DbEncryptFunction;
@@ -818,6 +819,13 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty {
   public Object parse(String value) {
     return scalarType.parse(value);
   }
+  
+  /**
+   * creates a mutableHash for the given JSON value.
+   */
+  public MutableHash createMutableHash(String json) {
+    return scalarType.createMutableHash(json);
+  }
 
   /**
    * Read the value for this property from L2 cache entry and set it to the bean.

ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyJsonMapper.java

@@ -2,14 +2,13 @@ package io.ebeaninternal.server.deploy;
 
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
+import io.ebean.bean.MutableHash;
 import io.ebean.core.type.DataReader;
 import io.ebean.text.TextException;
 import io.ebeaninternal.server.deploy.meta.DeployBeanProperty;
-import io.ebeaninternal.server.util.Md5;
 
 import javax.persistence.PersistenceException;
 import java.sql.SQLException;
-import java.util.Objects;
 
 public class BeanPropertyJsonMapper extends BeanProperty {
 
@@ -25,11 +24,9 @@ public class BeanPropertyJsonMapper extends BeanProperty {
   boolean isDirtyValue(Object value, EntityBeanIntercept ebi) {
     // dirty detection based on md5 hash of json content
     final String json = scalarType.jsonMapper(value);
-    final String newHash = Md5.hash(json);
-    final String oldHash = ebi.mutableHash(propertyIndex);
-    if (!Objects.equals(newHash, oldHash)) {
+    final MutableHash oldHash = ebi.mutableHash(propertyIndex);
+    if (oldHash == null || !oldHash.isEqualToJson(json)) {
       ebi.mutableContent(propertyIndex, json); // so we only convert to json once
-      ebi.mutableHash(propertyIndex, newHash); // for dirty detection next time
       return true;
     }
     return false;
@@ -43,7 +40,7 @@ public class BeanPropertyJsonMapper extends BeanProperty {
         setValue(bean, value);
         String json = reader.popJson();
         if (json != null) {
-          final String hash = Md5.hash(json);
+          final MutableHash hash = scalarType.createMutableHash(json);
           bean._ebean_getIntercept().mutableHash(propertyIndex, hash);
         }
       }

ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonInsert.java

@@ -1,8 +1,8 @@
 package io.ebeaninternal.server.persist.dmlbind;
 
 import io.ebean.bean.EntityBean;
+import io.ebean.bean.MutableHash;
 import io.ebeaninternal.server.deploy.BeanProperty;
-import io.ebeaninternal.server.util.Md5;
 
 import java.sql.SQLException;
 
@@ -30,9 +30,9 @@ class BindablePropertyJsonInsert extends BindableProperty {
       if (value == null) {
         request.bind(null, prop);
       } else {
-        // on insert store MD5 hash and push json
+        // on insert store hash and push json
         final String json = prop.format(value);
-        final String hash = Md5.hash(json);
+        final MutableHash hash = prop.createMutableHash(json);
         bean._ebean_getIntercept().mutableHash(propertyIndex, hash);
         request.pushJson(json);
         request.bind(value, prop);

ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonUpdate.java

@@ -1,6 +1,7 @@
 package io.ebeaninternal.server.persist.dmlbind;
 
 import io.ebean.bean.EntityBean;
+import io.ebean.bean.MutableHash;
 import io.ebeaninternal.server.deploy.BeanProperty;
 
 import java.sql.SQLException;
@@ -25,8 +26,10 @@ class BindablePropertyJsonUpdate extends BindableProperty {
     if (bean == null) {
       request.bind(null, prop);
     } else {
-      // on update push json
+      // on update store hash and push json
       final String json = bean._ebean_getIntercept().mutableContent(propertyIndex);
+      final MutableHash hash = prop.createMutableHash(json);
+      bean._ebean_getIntercept().mutableHash(propertyIndex, hash);
       request.pushJson(json);
       final Object value = prop.getValue(bean);
       request.bind(value, prop);

ebean-core/src/main/java/io/ebeaninternal/server/type/DataBind.java

@@ -46,12 +46,15 @@ public class DataBind implements DataBinder {
 
   @Override
   public void pushJson(String json) {
+    assert this.json == null; // we can only push one value
     this.json = json;
   }
 
   @Override
   public String popJson() {
-    return json;
+    String ret = json;
+    json = null;
+    return ret;
   }
 
   @Override

ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java

@@ -7,6 +7,8 @@ import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import com.fasterxml.jackson.databind.introspect.AnnotatedField;
+
+import io.ebean.bean.MutableHash;
 import io.ebean.core.type.DataBinder;
 import io.ebean.core.type.DataReader;
 import io.ebean.core.type.DocPropertyType;
@@ -15,6 +17,7 @@ import io.ebean.text.TextException;
 import io.ebeaninternal.json.ModifyAwareList;
 import io.ebeaninternal.json.ModifyAwareMap;
 import io.ebeaninternal.json.ModifyAwareSet;
+import io.ebeaninternal.server.util.Md5;
 
 import javax.persistence.PersistenceException;
 import java.io.DataInput;
@@ -24,6 +27,7 @@ import java.sql.SQLException;
 import java.sql.Types;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -47,7 +51,59 @@ class ScalarTypeJsonObjectMapper {
     }
     return new GenericObject(jsonManager, field, dbType, type);
   }
+  
+  private static class Md5MutableHash implements MutableHash {
 
+    private final String md5;
+    
+    Md5MutableHash(String json) {
+      md5 = Md5.hash(json);
+    }
+    
+    @Override
+    public boolean isEqualToObject(Object obj) {
+      return true; // we cannot determine differences...
+    }
+
+    @Override
+    public boolean isEqualToJson(String json) {
+      return Md5.hash(json).equals(md5);
+    }
+
+    @Override
+    public Object get() {
+      return null; // cannot create object from json
+    }
+
+  }
+  
+  private static class JsonMutableHash implements MutableHash {
+
+    private final String originalJson;
+    private ScalarType<?> parent;
+    
+    JsonMutableHash(ScalarType<?> parent, String json) {
+      this.parent = parent;
+      originalJson = json;
+    }
+    
+    @Override
+    public boolean isEqualToObject(Object obj) {
+      return isEqualToJson(parent.format(obj));
+    }
+
+    @Override
+    public boolean isEqualToJson(String json) {
+      return Objects.equals(originalJson, json);
+    }
+
+    @Override
+    public Object get() {
+      return parent.parse(originalJson);
+    }
+
+  }
+  
   /**
    * Maps any type (Object) using Jackson ObjectMapper.
    */
@@ -66,6 +122,19 @@ class ScalarTypeJsonObjectMapper {
     public String jsonMapper(Object value) {
       return formatValue(value);
     }
+    
+
+
+
+    
+    @Override
+    public MutableHash createMutableHash(String json) {
+      if (false) { // TODO should we make that configurable?
+        return new Md5MutableHash(json);
+      } else {
+        return new JsonMutableHash(this, json);
+      }
+    }
 
     @Override
     public Object read(DataReader reader) throws SQLException {

ebean-core/src/test/java/org/tests/json/TestDbJson_Jackson3.java

@@ -1,7 +1,11 @@
 package org.tests.json;
 
 import io.ebean.BaseTestCase;
+import io.ebean.BeanState;
 import io.ebean.DB;
+import io.ebean.ValuePair;
+import io.ebean.event.BeanPersistAdapter;
+import io.ebean.event.BeanPersistRequest;
 import io.ebeantest.LoggedSql;
 import org.junit.Test;
 import org.tests.model.json.EBasicJsonJackson3;
@@ -11,11 +15,33 @@ import org.tests.model.json.PlainBeanDirtyAware;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class TestDbJson_Jackson3 extends BaseTestCase {
 
+  public static class EBasicJsonListPersistController extends BeanPersistAdapter {
+
+    private static Map<String, ValuePair> updatedValues;
+
+    @Override
+    public boolean isRegisterFor(Class<?> cls) {
+      return EBasicJsonList.class.isAssignableFrom(cls);
+    }
+    
+    @Override
+    public boolean preInsert(BeanPersistRequest<?> request) {
+      updatedValues = request.getUpdatedValues();
+      return true;
+    }
+    
+    @Override
+    public boolean preUpdate(BeanPersistRequest<?> request) {
+      updatedValues = request.getUpdatedValues();
+      return true;
+    }
+  }
   @Test
   public void updateIncludesJsonColumn_when_explicit_isMarkedDirty() {
 
@@ -24,6 +50,7 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
     EBasicJsonJackson3 bean = new EBasicJsonJackson3();
     bean.setName("b1");
     bean.setPlainValue(contentBean);
+    bean.setPlainValue2(contentBean);
 
     bean.save();
 
@@ -69,12 +96,75 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
     found.setName("p1-mod");
     found.setBeanList(null);
 
+    BeanState state = DB.getBeanState(found);
+    assertThat(state.getChangedProps()).containsExactlyInAnyOrder("name", "beanList");
+    
+    ValuePair pair = state.getDirtyValues().get("name");
+    assertThat(pair.getNewValue()).isEqualTo("p1-mod");
+    assertThat(pair.getOldValue()).isEqualTo("p1");
+    
+    pair = state.getDirtyValues().get("beanList");
+    assertThat(pair.getNewValue()).isEqualTo(null);
+    assertThat((List<PlainBean>)pair.getOldValue()).hasSize(1)
+      .extracting(PlainBean::getName).containsExactly("a");
+    
+    
     LoggedSql.start();
     DB.save(found);
 
-    final List<String> sql = LoggedSql.stop();
+    List<String> sql = LoggedSql.stop();
     assertThat(sql).hasSize(1);
     // plain_bean=?, no longer included with MD5 dirty detection
     assertThat(sql.get(0)).contains("update ebasic_json_list set name=?, bean_list=?, version=? where id=?");
+    
+    assertThat(EBasicJsonListPersistController.updatedValues.entrySet())
+      .extracting(Map.Entry::toString)
+      .containsExactlyInAnyOrder("beanList=null,[name:a]","name=p1-mod,p1","version=2,1");
+    
+    
+    assertThat(DB.getBeanState(found).isDirty()).isFalse();
+    
+    found.getPlainBean().setName("b");
+
+    assertThat(DB.getBeanState(found).isDirty()).isTrue();
+    
+    state = DB.getBeanState(found);
+    assertThat(state.getChangedProps()).containsExactlyInAnyOrder("plainBean");
+    pair = state.getDirtyValues().get("plainBean");
+    assertThat(pair.getNewValue()).hasToString("name:b");
+    assertThat(pair.getOldValue()).hasToString("name:a");
+
+    
+    LoggedSql.start();
+    DB.save(found);
+
+    sql = LoggedSql.stop();
+    assertThat(sql).hasSize(1);
+    // plain_bean=?, no longer included with MD5 dirty detection
+    assertThat(sql.get(0)).contains("update ebasic_json_list set plain_bean=?, version=? where id=?");
+
+    assertThat(EBasicJsonListPersistController.updatedValues.entrySet())
+      .extracting(Map.Entry::toString)
+      .containsExactlyInAnyOrder("plainBean=name:b,name:a", "version=3,2");
+  }
+  
+  @Test
+  public void updateIncludesJsonColumn_when_list_loadedAndNotDirtyAware() {
+
+    PlainBean contentBean = new PlainBean("a", 42);
+    EBasicJsonList bean = new EBasicJsonList();
+    bean.setName("p1");
+    bean.setPlainBean(contentBean);
+    bean.setBeanList(Arrays.asList(contentBean));
+
+    DB.save(bean);
+    final EBasicJsonList found = DB.find(EBasicJsonList.class, bean.getId());
+    found.getBeanList().get(0).setName("p1-mod");
+
+    BeanState state = DB.getBeanState(found);
+    assertThat(state.getChangedProps()).containsExactlyInAnyOrder("beanList");
+    // this test fails, because we have a OmList instead of a GenericObject
+    // TODO: Can/Should we enhance the @DbJson/@DbJsonB annotations with a property "dirtyDetection" 
+  
   }
 }

ebean-core/src/test/java/org/tests/model/json/EBasicJsonJackson3.java

@@ -18,6 +18,9 @@ public class EBasicJsonJackson3 extends Model {
   @DbJson(length = 500)
   PlainBeanDirtyAware plainValue;
 
+  @DbJson(length = 500)
+  PlainBeanDirtyAware plainValue2;
+  
   @Version
   long version;
 
@@ -45,6 +48,14 @@ public class EBasicJsonJackson3 extends Model {
     this.plainValue = plainValue;
   }
 
+  public PlainBeanDirtyAware getPlainValue2() {
+    return plainValue2;
+  }
+
+  public void setPlainValue2(PlainBeanDirtyAware plainValue2) {
+    this.plainValue2 = plainValue2;
+  }
+  
   public long getVersion() {
     return version;
   }