some refactor and tidying

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 MutableJson[] 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,13 +395,10 @@ 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);
-      if (mutableHash != null && mutableHash[i] != null) {
-        mutableHash[i].update(owner._ebean_getField(i));
-      }
+      flags[i] &= ~(FLAG_CHANGED_PROP | FLAG_ORIG_VALUE_SET);
     }
-    
     this.dirty = false;
   }
 
@@ -653,14 +666,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 || isChangedByHash(i)) {
+      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) + ".");
-      }
+      } 
     }
   }
 
@@ -671,7 +684,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 || isChangedByHash(i)) {
+      if (isChangedProp(i)) {
         if (propertyNames.contains(names[i])) {
           return true;
         }
@@ -699,16 +712,17 @@ 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 (isChangedByHash(i)) {
-        String propName = (prefix == null ? getProperty(i) : prefix + getProperty(i));
-        Object newVal = owner._ebean_getField(i);
-        Object oldVal = mutableHash[i].get();
-        dirtyValues.put(propName, new ValuePair(newVal, oldVal));
-      } else 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);
         Object oldVal = getOrigValue(i);
+        if ((flags[i] & (FLAG_ORIG_VALUE_SET | FLAG_MUTABLE_HASH_SET)) == FLAG_MUTABLE_HASH_SET) {
+          // mutable hash set, but not ORIG_VALUE
+          oldVal = mutableHash[i].get();
+          setOriginalValue(i, oldVal);
+        }
         if (notEqual(oldVal, newVal)) {
           dirtyValues.put(propName, new ValuePair(newVal, oldVal));
         }
@@ -726,7 +740,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);
@@ -761,7 +775,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
@@ -1159,20 +1173,29 @@ public final class EntityBeanIntercept implements Serializable {
     return ret;
   }
 
-  private boolean isChangedByHash(int propertyIndex) {
-    return mutableHash != null 
-        && mutableHash[propertyIndex] != null
-        && !mutableHash[propertyIndex].isEqualToObject(owner._ebean_getField(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 MutableJson mutableHash(int propertyIndex) {
+  public MutableHash mutableHash(int propertyIndex) {
     return mutableHash == null ? null : mutableHash[propertyIndex];
   }
 
-  public void mutableHash(int propertyIndex, MutableJson content) {
+  public void mutableHash(int propertyIndex, MutableHash content) {
     if (mutableHash == null) {
-      mutableHash = new MutableJson[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,16 @@
+package io.ebean.bean;
+
+
+public interface MutableHash {
+  
+  boolean isEqualToJson(String json);
+
+  default boolean isEqualToObject(Object obj) {
+    return true;
+  }
+  
+  default Object get() {
+    return null;
+  }
+}
+ 

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

@@ -1,12 +0,0 @@
-package io.ebean.bean;
-
-public interface MutableJson {
-  
-  boolean isEqualToObject(Object obj);
-  
-  boolean isEqualToJson(String json);
-  
-  Object get();
-
-  void update(Object obj);
-}

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

@@ -3,7 +3,7 @@ package io.ebean.core.type;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
 
-import io.ebean.bean.MutableJson;
+import io.ebean.bean.MutableHash;
 import io.ebean.text.StringFormatter;
 import io.ebean.text.StringParser;
 
@@ -44,9 +44,10 @@ public interface ScalarType<T> extends StringParser, StringFormatter, ScalarData
     throw new UnsupportedOperationException();
   }
 
-  default MutableJson jsonMutable(String json) {
+  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,15 +2,13 @@ package io.ebeaninternal.server.deploy;
 
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
-import io.ebean.bean.MutableJson;
+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 {
 
@@ -26,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 MutableJson oldHash = ebi.mutableHash(propertyIndex);
+    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, scalarType.jsonMutable(json)); // for dirty detection next time
-      //must be done AFTER persistControllers are called. 
       return true;
     }
     return false;
@@ -44,8 +40,8 @@ public class BeanPropertyJsonMapper extends BeanProperty {
         setValue(bean, value);
         String json = reader.popJson();
         if (json != null) {
-          final String hash = scalarType.format(value);
-          bean._ebean_getIntercept().mutableHash(propertyIndex, scalarType.jsonMutable(hash));
+          final MutableHash hash = scalarType.createMutableHash(json);
+          bean._ebean_getIntercept().mutableHash(propertyIndex, hash);
         }
       }
       return value;

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,10 @@ 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);
-        bean._ebean_getIntercept().mutableHash(propertyIndex, prop.getScalarType().jsonMutable(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

@@ -8,7 +8,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import com.fasterxml.jackson.databind.introspect.AnnotatedField;
 
-import io.ebean.bean.MutableJson;
+import io.ebean.bean.MutableHash;
 import io.ebean.core.type.DataBinder;
 import io.ebean.core.type.DataReader;
 import io.ebean.core.type.DocPropertyType;
@@ -51,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.
    */
@@ -71,63 +123,16 @@ class ScalarTypeJsonObjectMapper {
       return formatValue(value);
     }
     
-    private class Md5MutableJson implements MutableJson {
 
-      private String md5;
-      Md5MutableJson(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
-      }
-      @Override
-      public void update(Object obj) {
-        md5 = Md5.hash(format(obj));
-      }
-    }
-
-    private class PlainMutableJson implements MutableJson {
-
-      private String originalJson;
-      PlainMutableJson(String json) {
-        originalJson = json;
-      }
-      @Override
-      public boolean isEqualToObject(Object obj) {
-        return isEqualToJson(format(obj));
-      }
-
-      @Override
-      public boolean isEqualToJson(String json) {
-        return Objects.equals(originalJson, json);
-      }
-
-      @Override
-      public Object get() {
-        return parse(originalJson);
-      }
-      @Override
-      public void update(Object obj) {
-        originalJson = format(obj);
-      }
-    }
+    
     @Override
-    public MutableJson jsonMutable(String originalJson) {
-      if (false) {
-        return new Md5MutableJson(originalJson);
+    public MutableHash createMutableHash(String json) {
+      if (false) { // TODO should we make that configurable?
+        return new Md5MutableHash(json);
       } else {
-        return new PlainMutableJson(originalJson);
+        return new JsonMutableHash(this, json);
       }
     }
 

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

@@ -50,6 +50,7 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
     EBasicJsonJackson3 bean = new EBasicJsonJackson3();
     bean.setName("b1");
     bean.setPlainValue(contentBean);
+    bean.setPlainValue2(contentBean);
 
     bean.save();
 
@@ -121,10 +122,11 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
       .containsExactlyInAnyOrder("beanList=null,[name:a]","name=p1-mod,p1","version=2,1");
     
     
-    found.getPlainBean().setName("b");
+    assertThat(DB.getBeanState(found).isDirty()).isFalse();
     
-    // CHECKME: How do we get these checks to work?
-    // assertThat(DB.getBeanState(found).isDirty()).isTrue();
+    found.getPlainBean().setName("b");
+
+    assertThat(DB.getBeanState(found).isDirty()).isTrue();
     
     state = DB.getBeanState(found);
     assertThat(state.getChangedProps()).containsExactlyInAnyOrder("plainBean");
@@ -145,4 +147,24 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
       .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;
   }