diff --git a/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java b/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java index 2ea4afbb5..37446a1b3 100644 --- a/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java +++ b/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 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 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; } diff --git a/ebean-api/src/main/java/io/ebean/bean/MutableHash.java b/ebean-api/src/main/java/io/ebean/bean/MutableHash.java new file mode 100644 index 000000000..09c8761af --- /dev/null +++ b/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; + } +} + \ No newline at end of file diff --git a/ebean-api/src/main/java/io/ebean/bean/MutableJson.java b/ebean-api/src/main/java/io/ebean/bean/MutableJson.java deleted file mode 100644 index 5989465e2..000000000 --- a/ebean-api/src/main/java/io/ebean/bean/MutableJson.java +++ /dev/null @@ -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); -} diff --git a/ebean-core-type/src/main/java/io/ebean/core/type/ScalarType.java b/ebean-core-type/src/main/java/io/ebean/core/type/ScalarType.java index b9f7f168c..55c8ad9bd 100644 --- a/ebean-core-type/src/main/java/io/ebean/core/type/ScalarType.java +++ b/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 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. diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index 4306ed97d..99ef307a7 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/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. diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyJsonMapper.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyJsonMapper.java index de5d148b5..204b9aac5 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyJsonMapper.java +++ b/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; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonInsert.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonInsert.java index 24cf050e3..e9de67a52 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonInsert.java +++ b/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); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonUpdate.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonUpdate.java index 3418369f5..ab281cf01 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/dmlbind/BindablePropertyJsonUpdate.java +++ b/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); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/DataBind.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/DataBind.java index 489046405..a1a9ec8cf 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/type/DataBind.java +++ b/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 diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java index 5b44b63e4..0012c4cd6 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java +++ b/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); } } diff --git a/ebean-core/src/test/java/org/tests/json/TestDbJson_Jackson3.java b/ebean-core/src/test/java/org/tests/json/TestDbJson_Jackson3.java index 4d31f8a56..8e399d3df 100644 --- a/ebean-core/src/test/java/org/tests/json/TestDbJson_Jackson3.java +++ b/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" + + } } diff --git a/ebean-core/src/test/java/org/tests/model/json/EBasicJsonJackson3.java b/ebean-core/src/test/java/org/tests/model/json/EBasicJsonJackson3.java index 3d942d58b..45375b516 100644 --- a/ebean-core/src/test/java/org/tests/model/json/EBasicJsonJackson3.java +++ b/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; }