From c00484026c12d4a5cda51637326c311d871c5ac5 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Fri, 24 Jul 2015 20:12:26 +1200 Subject: [PATCH] #351 - ENH: Add diff to Version beans returned from findVersions() --- .../java/com/avaje/ebean/EbeanServer.java | 2 +- src/main/java/com/avaje/ebean/ValuePair.java | 27 ++++- src/main/java/com/avaje/ebean/Version.java | 26 ++++- .../com/avaje/ebean/config/ServerConfig.java | 30 +++++ .../server/core/DefaultServer.java | 3 +- .../ebeaninternal/server/core/DiffHelp.java | 110 ++++++++++-------- .../server/query/CQueryEngine.java | 28 +++++ .../server/core/TestDiffHelpSimple.java | 30 ++++- .../server/core/TestDiffHelpWithEmbedded.java | 19 ++- 9 files changed, 215 insertions(+), 60 deletions(-) diff --git a/src/main/java/com/avaje/ebean/EbeanServer.java b/src/main/java/com/avaje/ebean/EbeanServer.java index fb2a7354a..17d248f39 100644 --- a/src/main/java/com/avaje/ebean/EbeanServer.java +++ b/src/main/java/com/avaje/ebean/EbeanServer.java @@ -148,7 +148,7 @@ public interface EbeanServer { * difference comparison. *

*/ - Map diff(Object a, Object b); + Map diff(Object newBean, Object oldBean); /** * Create a new instance of T that is an EntityBean. diff --git a/src/main/java/com/avaje/ebean/ValuePair.java b/src/main/java/com/avaje/ebean/ValuePair.java index ec2b6f112..8cb03ee3a 100644 --- a/src/main/java/com/avaje/ebean/ValuePair.java +++ b/src/main/java/com/avaje/ebean/ValuePair.java @@ -5,10 +5,19 @@ package com.avaje.ebean; */ public class ValuePair { - private final Object newValue; + protected Object newValue; - private final Object oldValue; + protected Object oldValue; + /** + * Default constructor for JSON tools. + */ + public ValuePair() { + } + + /** + * Construct with the pair of new and old values. + */ public ValuePair(Object newValue, Object oldValue) { this.newValue = newValue; this.oldValue = oldValue; @@ -44,6 +53,20 @@ public class ValuePair { return oldValue; } + /** + * Set the new value. + */ + public void setNewValue(Object newValue) { + this.newValue = newValue; + } + + /** + * Set the old value. + */ + public void setOldValue(Object oldValue) { + this.oldValue = oldValue; + } + public String toString() { return newValue + "," + oldValue; } diff --git a/src/main/java/com/avaje/ebean/Version.java b/src/main/java/com/avaje/ebean/Version.java index cca628ad5..525dfb148 100644 --- a/src/main/java/com/avaje/ebean/Version.java +++ b/src/main/java/com/avaje/ebean/Version.java @@ -1,6 +1,7 @@ package com.avaje.ebean; import java.sql.Timestamp; +import java.util.Map; /** * Wraps a version of a @History bean. @@ -10,17 +11,22 @@ public class Version { /** * The version of the bean. */ - T bean; + protected T bean; /** * The effective start date time of this version. */ - Timestamp start; + protected Timestamp start; /** * The effective end date time of this version. */ - Timestamp end; + protected Timestamp end; + + /** + * The map of changed properties. + */ + protected Map diff; /** * Construct with bean and an effective date time range. @@ -78,4 +84,18 @@ public class Version { public void setEnd(Timestamp end) { this.end = end; } + + /** + * Set the map of differences from this bean to the prior version. + */ + public void setDiff(Map diff) { + this.diff = diff; + } + + /** + * Return the map of differences from this bean to the prior version. + */ + public Map getDiff() { + return diff; + } } diff --git a/src/main/java/com/avaje/ebean/config/ServerConfig.java b/src/main/java/com/avaje/ebean/config/ServerConfig.java index 594fd8009..3c18c67b4 100644 --- a/src/main/java/com/avaje/ebean/config/ServerConfig.java +++ b/src/main/java/com/avaje/ebean/config/ServerConfig.java @@ -317,6 +317,7 @@ public class ServerConfig { private int queryCacheMaxIdleTime = 600; private int queryCacheMaxTimeToLive = 60*60*6; private Object objectMapper; + private boolean diffFlatMode; /** * Construct a Server Configuration for programmatically creating an EbeanServer. @@ -1876,6 +1877,7 @@ public class ServerConfig { persistenceContextScope = PersistenceContextScope.valueOf(p.get("persistenceContextScope", "TRANSACTION")); + diffFlatMode = p.getBoolean("diffFlatMode", diffFlatMode); asOfViewSuffix = p.get("asOfViewSuffix", asOfViewSuffix); asOfSysPeriod = p.get("asOfSysPeriod", asOfSysPeriod); dataSourceJndiName = p.get("dataSourceJndiName", dataSourceJndiName); @@ -1967,11 +1969,39 @@ public class ServerConfig { return databasePlatform.isDisallowBatchOnCascade() ? PersistBatch.NONE : persistBatchOnCascade; } + /** + * Return the Jackson ObjectMapper. + *

+ * Note that this is not strongly typed as Jackson ObjectMapper is an optional dependency. + *

+ */ public Object getObjectMapper() { return objectMapper; } + /** + * Set the Jackson ObjectMapper. + *

+ * Note that this is not strongly typed as Jackson ObjectMapper is an optional dependency. + *

+ */ public void setObjectMapper(Object objectMapper) { this.objectMapper = objectMapper; } + + /** + * Return true if diff should return flat properties with dot notation rather than + * embedded beans or reference beans (when the associated bean id is different). + */ + public boolean isDiffFlatMode() { + return diffFlatMode; + } + + /** + * Set to true if diff should return flat properties with dot notation rather than + * embedded beans or reference beans (when the associated bean id is different). + */ + public void setDiffFlatMode(boolean diffFlatMode) { + this.diffFlatMode = diffFlatMode; + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java index 3bdf44108..b6057aa02 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java @@ -88,7 +88,7 @@ public final class DefaultServer implements SpiEbeanServer { private final BeanDescriptorManager beanDescriptorManager; - private final DiffHelp diffHelp = new DiffHelp(); + private final DiffHelp diffHelp; private final AutoFetchManager autoFetchManager; @@ -171,6 +171,7 @@ public final class DefaultServer implements SpiEbeanServer { this.backgroundExecutor = config.getBackgroundExecutor(); this.serverName = serverConfig.getName(); + this.diffHelp = new DiffHelp(serverConfig.isDiffFlatMode()); this.lazyLoadBatchSize = serverConfig.getLazyLoadBatchSize(); this.queryBatchSize = serverConfig.getQueryBatchSize(); this.cqueryEngine = config.getCQueryEngine(); diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java b/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java index 58c2884ad..6e24fc251 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java @@ -18,7 +18,12 @@ import com.avaje.ebeaninternal.util.ValueUtil; */ public class DiffHelp { - + private final boolean flatMode; + + public DiffHelp(boolean flatMode) { + this.flatMode = flatMode; + } + /** * Return a map of the differences between a and b. *

@@ -29,45 +34,45 @@ public class DiffHelp { * This intentionally does not include as OneToMany or ManyToMany properties. *

*/ - public Map diff(Object a, Object b, BeanDescriptor desc) { + public Map diff(Object newBean, Object oldBean, BeanDescriptor desc) { - if (a instanceof EntityBean == false) { - throw new IllegalArgumentException("First bean expected to be an enhanced EntityBean? bean:"+a); + if (!(newBean instanceof EntityBean)) { + throw new IllegalArgumentException("First bean expected to be an enhanced EntityBean? bean:"+newBean); } - if (b != null) { - if (b instanceof EntityBean == false) { - throw new IllegalArgumentException("Second bean expected to be an enhanced EntityBean? bean:"+b); + if (oldBean != null) { + if (!(oldBean instanceof EntityBean)) { + throw new IllegalArgumentException("Second bean expected to be an enhanced EntityBean? bean:"+oldBean); } - if (!a.getClass().isAssignableFrom(b.getClass())) { + if (!newBean.getClass().isAssignableFrom(oldBean.getClass())) { throw new IllegalArgumentException("Second bean not assignable to the first bean?"); } } - if (b == null) { - return ((EntityBean) a)._ebean_getIntercept().getDirtyValues(); + if (oldBean == null) { + return ((EntityBean) newBean)._ebean_getIntercept().getDirtyValues(); } Map map = new LinkedHashMap(); - diff(null, map, (EntityBean)a, (EntityBean)b, desc); + diff(null, map, (EntityBean) newBean, (EntityBean) oldBean, desc); return map; } - public void diff(String prefix, Map map, EntityBean first, EntityBean sec, BeanDescriptor desc) { + public void diff(String prefix, Map map, EntityBean newBean, EntityBean oldBean, BeanDescriptor desc) { // check the simple properties BeanProperty[] base = desc.propertiesBaseScalar(); for (int i = 0; i < base.length; i++) { - Object aval = base[i].getValue(first); - Object bval = base[i].getValue(sec); - if (!ValueUtil.areEqual(aval, bval)) { + Object newVal = (newBean == null) ? null : base[i].getValue(newBean); + Object oldVal = (oldBean == null) ? null : base[i].getValue(oldBean); + if (!ValueUtil.areEqual(newVal, oldVal)) { String propName = (prefix == null) ? base[i].getName() : prefix + base[i].getName(); - map.put(propName, new ValuePair(aval, bval)); + map.put(propName, new ValuePair(newVal, oldVal)); } } - diffAssocOne(prefix, first, sec, desc, map); - diffEmbedded(prefix, first, sec, desc, map); + diffAssocOne(prefix, newBean, oldBean, desc, map); + diffEmbedded(prefix, newBean, oldBean, desc, map); } /** @@ -77,24 +82,29 @@ public class DiffHelp { * determined to be different as is added to the map. *

*/ - private void diffEmbedded(String prefix, EntityBean a, EntityBean b, BeanDescriptor desc, Map map) { + private void diffEmbedded(String prefix, EntityBean newBean, EntityBean oldBean, BeanDescriptor desc, Map map) { BeanPropertyAssocOne[] emb = desc.propertiesEmbedded(); for (int i = 0; i < emb.length; i++) { - EntityBean aval = (EntityBean)emb[i].getValue(a); - EntityBean bval = (EntityBean)emb[i].getValue(b); + EntityBean newVal = (EntityBean)emb[i].getValue(newBean); + EntityBean oldVal = (EntityBean)emb[i].getValue(oldBean); - if (!isBothNull(aval, bval)) { + if (!isBothNull(newVal, oldVal)) { String propName = (prefix == null) ? emb[i].getName() : prefix + emb[i].getName(); - if (isDiffNull(aval, bval)) { + if (isDiffNull(newVal, oldVal)) { // one of the embedded beans is null - map.put(propName, new ValuePair(aval, bval)); + if (flatMode) { + BeanDescriptor embDesc = emb[i].getTargetDescriptor(); + diff(emb[i].getName()+".", map, newVal, oldVal, embDesc); + } else { + map.put(propName, new ValuePair(newVal, oldVal)); + } } else { // recursively diff into the embedded bean BeanDescriptor embDesc = emb[i].getTargetDescriptor(); - diff(emb[i].getName()+".", map, aval, bval, embDesc); + diff(emb[i].getName()+".", map, newVal, oldVal, embDesc); } } } @@ -104,45 +114,43 @@ public class DiffHelp { * If the properties are different by null OR if the id value is different, * then add the Assoc One bean to the map. */ - private void diffAssocOne(String prefix, EntityBean a, EntityBean b, BeanDescriptor desc, Map map) { + private void diffAssocOne(String prefix, EntityBean newBean, EntityBean oldBean, BeanDescriptor desc, Map map) { BeanPropertyAssocOne[] ones = desc.propertiesOne(); for (int i = 0; i < ones.length; i++) { - Object aval = ones[i].getValue(a); - Object bval = ones[i].getValue(b); + Object newVal = ones[i].getValue(newBean); + Object oldVal = ones[i].getValue(oldBean); - if (!isBothNull(aval, bval)) { - String propName = (prefix == null) ? ones[i].getName() : prefix + ones[i].getName(); - if (isDiffNull(aval, bval)) { - // one of them is/was null - map.put(propName, new ValuePair(aval, bval)); + if (!isBothNull(newVal, oldVal)) { + BeanDescriptor oneDesc = ones[i].getTargetDescriptor(); + Object newId = (newVal == null) ? null : oneDesc.getId((EntityBean)newVal); + Object oldId = (oldVal == null) ? null : oneDesc.getId((EntityBean)oldVal); - } else { - // check to see if the Id properties - // are different - BeanDescriptor oneDesc = ones[i].getTargetDescriptor(); - Object aOneId = oneDesc.getId((EntityBean)aval); - Object bOneId = oneDesc.getId((EntityBean)bval); + if (!ValueUtil.areEqual(newId, oldId)) { + String propName = (prefix == null) ? ones[i].getName() : prefix + ones[i].getName(); + // the ids are different + if (flatMode) { + String idName = oneDesc.getIdProperty().getName(); + map.put(propName + "." + idName, new ValuePair(newId, oldId)); - if (!ValueUtil.areEqual(aOneId, bOneId)) { - // the ids are different - map.put(propName, new ValuePair(aval, bval)); - } - } - } + } else { + map.put(propName, new ValuePair(newVal, oldVal)); + } + } + } } } - private boolean isBothNull(Object aval, Object bval) { - return aval == null && bval == null; + private boolean isBothNull(Object newVal, Object oldVal) { + return newVal == null && oldVal == null; } - private boolean isDiffNull(Object aval, Object bval) { - if (aval == null) { - return bval != null; + private boolean isDiffNull(Object newVal, Object oldVal) { + if (newVal == null) { + return oldVal != null; } else { - return bval == null; + return oldVal == null; } } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java index bed71fc76..90e819b6a 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java @@ -1,10 +1,14 @@ package com.avaje.ebeaninternal.server.query; import java.sql.SQLException; +import java.util.Collections; import java.util.List; import java.util.Map; +import com.avaje.ebean.ValuePair; import com.avaje.ebean.Version; +import com.avaje.ebeaninternal.server.core.DiffHelp; +import com.avaje.ebeaninternal.server.deploy.BeanDescriptor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,6 +36,8 @@ public class CQueryEngine { private static final String T0 = "t0"; + private final DiffHelp diffHelp = new DiffHelp(true); + private final boolean forwardOnlyHintOnFindIterate; private final CQueryBuilder queryBuilder; @@ -180,6 +186,8 @@ public class CQueryEngine { } List> versions = cquery.readVersions(); + deriveVersionDiffs(versions, request); + if (request.isLogSummary()) { logFindManySummary(cquery); } @@ -196,6 +204,26 @@ public class CQueryEngine { } } + private void deriveVersionDiffs(List> versions, OrmQueryRequest request) { + + BeanDescriptor descriptor = request.getBeanDescriptor(); + + Version current = versions.get(0); + for (int i = 1; i < versions.size(); i++) { + Version next = versions.get(i); + deriveVersionDiff(current, next, descriptor); + current = next; + } + // put an empty map into the last one + current.setDiff(Collections.EMPTY_MAP); + } + + private void deriveVersionDiff(Version current, Version prior, BeanDescriptor descriptor) { + + Map diff = diffHelp.diff(current.getBean(), prior.getBean(), descriptor); + current.setDiff(diff); + } + /** * Return the lower sys_period given the table alias of the query or default. */ diff --git a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java index 07ffbe588..118129115 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java +++ b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java @@ -20,7 +20,7 @@ import com.avaje.tests.model.basic.Order.Status; public class TestDiffHelpSimple extends BaseTestCase { - DiffHelp diffHelp = new DiffHelp(); + DiffHelp diffHelp = new DiffHelp(false); long firstTime = System.currentTimeMillis()-10000; long secondTime = System.currentTimeMillis(); @@ -71,6 +71,34 @@ public class TestDiffHelpSimple extends BaseTestCase { Assert.assertTrue(keySet.contains("customer")); } + @Test + public void testBasicChanges_given_flatMode() { + + + Order order1 = createBaseOrder(server); + + Order order2 = new Order(); + order2.setId(14); + order2.setCretime(new Timestamp(secondTime)); + order2.setCustomer(server.getReference(Customer.class, 2133)); + order2.setStatus(Status.COMPLETE); + order2.setShipDate(new Date(secondTime)); + order2.setOrderDate(new Date(secondTime)); + + DiffHelp diffHelp = new DiffHelp(true); + Map diff = diffHelp.diff(order1, order2, orderDesc); + + Assert.assertEquals(5, diff.size()); + + Set keySet = diff.keySet(); + Assert.assertTrue(keySet.contains("cretime")); + Assert.assertTrue(keySet.contains("status")); + Assert.assertTrue(keySet.contains("shipDate")); + Assert.assertTrue(keySet.contains("orderDate")); + Assert.assertTrue(keySet.contains("customer.id")); + Assert.assertEquals(2133, diff.get("customer.id").getOldValue()); + Assert.assertEquals(1234, diff.get("customer.id").getNewValue()); + } @Test public void testIdIgnored() { diff --git a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java index d66e806d8..f15686b47 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java +++ b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java @@ -16,7 +16,7 @@ import com.avaje.tests.model.embedded.Eembeddable; public class TestDiffHelpWithEmbedded extends BaseTestCase { - DiffHelp diffHelp = new DiffHelp(); + DiffHelp diffHelp = new DiffHelp(false); EbeanServer server; BeanDescriptor emainDesc; @@ -101,6 +101,23 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { Assert.assertEquals("bar",((Eembeddable)valuePair.getNewValue()).getDescription()); } + @Test + public void testSecondEmbeddedIsNull_given_flatMode() { + + EMain emain1 = createEMain(); + EMain emain2 = createEMain(); + emain2.setEmbeddable(null); + + DiffHelp diffHelp = new DiffHelp(true); + Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Assert.assertEquals(1, diff.size()); + ValuePair valuePair = diff.get("embeddable.description"); + + Assert.assertNotNull(valuePair); + Assert.assertNull(valuePair.getOldValue()); + Assert.assertEquals("bar", valuePair.getNewValue()); + } + @Test public void testBothEmbeddedIsNull() {