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 59a2450bd..e686464e2 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 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 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 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; } 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..aa5ea29df --- /dev/null +++ b/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; + } +} + \ No newline at end of file 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 831664991..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 @@ -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 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. 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 2d24958b6..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,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); } } 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 0a93452f1..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,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); 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 d71cf18d6..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 @@ -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 { 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 4bc0194d6..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 @@ -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 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)pair.getOldValue()).hasSize(1) + .extracting(PlainBean::getName).containsExactly("a"); + + LoggedSql.start(); DB.save(found); - final List sql = LoggedSql.stop(); + List 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" + } } 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; }