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..2ea4afbb5 100644 --- a/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java +++ b/ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java @@ -94,7 +94,7 @@ public final class EntityBeanIntercept implements Serializable { /** * Holds MD5 hash of json loaded jackson beans. */ - private String[] mutableHash; + private MutableJson[] mutableHash; /** * Holds json content determined at point of dirty check. @@ -381,7 +381,11 @@ public final class EntityBeanIntercept implements Serializable { this.origValues = 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)); + } } + this.dirty = false; } @@ -649,7 +653,7 @@ 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 ((flags[i] & FLAG_CHANGED_PROP) != 0 || isChangedByHash(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) { @@ -667,7 +671,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 ((flags[i] & FLAG_CHANGED_PROP) != 0 || isChangedByHash(i)) { if (propertyNames.contains(names[i])) { return true; } @@ -695,7 +699,12 @@ 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 (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) { // the property has been changed on this bean String propName = (prefix == null ? getProperty(i) : prefix + getProperty(i)); Object newVal = owner._ebean_getField(i); @@ -1150,13 +1159,19 @@ public final class EntityBeanIntercept implements Serializable { return ret; } - public String mutableHash(int propertyIndex) { + private boolean isChangedByHash(int propertyIndex) { + return mutableHash != null + && mutableHash[propertyIndex] != null + && !mutableHash[propertyIndex].isEqualToObject(owner._ebean_getField(propertyIndex)); + } + + public MutableJson mutableHash(int propertyIndex) { return mutableHash == null ? null : mutableHash[propertyIndex]; } - public void mutableHash(int propertyIndex, String content) { + public void mutableHash(int propertyIndex, MutableJson content) { if (mutableHash == null) { - mutableHash = new String[flags.length]; + mutableHash = new MutableJson[flags.length]; } mutableHash[propertyIndex] = content; } diff --git a/ebean-api/src/main/java/io/ebean/bean/MutableJson.java b/ebean-api/src/main/java/io/ebean/bean/MutableJson.java new file mode 100644 index 000000000..5989465e2 --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/bean/MutableJson.java @@ -0,0 +1,12 @@ +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 831664991..b9f7f168c 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.MutableJson; import io.ebean.text.StringFormatter; import io.ebean.text.StringParser; @@ -42,6 +44,9 @@ public interface ScalarType extends StringParser, StringFormatter, ScalarData throw new UnsupportedOperationException(); } + default MutableJson jsonMutable(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/BeanPropertyJsonMapper.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyJsonMapper.java index 2d24958b6..de5d148b5 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,6 +2,7 @@ package io.ebeaninternal.server.deploy; import io.ebean.bean.EntityBean; import io.ebean.bean.EntityBeanIntercept; +import io.ebean.bean.MutableJson; import io.ebean.core.type.DataReader; import io.ebean.text.TextException; import io.ebeaninternal.server.deploy.meta.DeployBeanProperty; @@ -25,11 +26,11 @@ 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 MutableJson 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 + //ebi.mutableHash(propertyIndex, scalarType.jsonMutable(json)); // for dirty detection next time + //must be done AFTER persistControllers are called. return true; } return false; @@ -43,8 +44,8 @@ public class BeanPropertyJsonMapper extends BeanProperty { setValue(bean, value); String json = reader.popJson(); if (json != null) { - final String hash = Md5.hash(json); - bean._ebean_getIntercept().mutableHash(propertyIndex, hash); + final String hash = scalarType.format(value); + bean._ebean_getIntercept().mutableHash(propertyIndex, scalarType.jsonMutable(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 0a93452f1..24cf050e3 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 @@ -32,8 +32,7 @@ class BindablePropertyJsonInsert extends BindableProperty { } else { // on insert store MD5 hash and push json final String json = prop.format(value); - final String hash = Md5.hash(json); - bean._ebean_getIntercept().mutableHash(propertyIndex, hash); + bean._ebean_getIntercept().mutableHash(propertyIndex, prop.getScalarType().jsonMutable(json)); request.pushJson(json); request.bind(value, prop); } 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..5b44b63e4 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.MutableJson; 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; /** @@ -66,6 +70,66 @@ class ScalarTypeJsonObjectMapper { public String jsonMapper(Object value) { 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); + } else { + return new PlainMutableJson(originalJson); + } + } @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..4d31f8a56 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() { @@ -69,12 +95,54 @@ 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"); + + + found.getPlainBean().setName("b"); + + // CHECKME: How do we get these checks to work? + // 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"); } }