From 088fcfb296ba91b3533c08120ef422be2736585f Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Thu, 3 Sep 2015 14:29:03 +1200 Subject: [PATCH] #406 - Remove diff "non flat mode" ... so diff going forward only supports flat mode --- .../com/avaje/ebean/config/ServerConfig.java | 19 +-- .../server/core/DefaultServer.java | 5 +- .../ebeaninternal/server/core/DiffHelp.java | 124 ++---------------- .../server/deploy/BeanDescriptor.java | 9 ++ .../server/query/CQueryEngine.java | 4 +- .../server/core/TestDiffHelpSimple.java | 19 ++- .../server/core/TestDiffHelpWithEmbedded.java | 30 ++--- 7 files changed, 42 insertions(+), 168 deletions(-) diff --git a/src/main/java/com/avaje/ebean/config/ServerConfig.java b/src/main/java/com/avaje/ebean/config/ServerConfig.java index 1054e2acd..7a89b00f2 100644 --- a/src/main/java/com/avaje/ebean/config/ServerConfig.java +++ b/src/main/java/com/avaje/ebean/config/ServerConfig.java @@ -359,7 +359,6 @@ public class ServerConfig { private int queryCacheMaxIdleTime = 600; private int queryCacheMaxTimeToLive = 60*60*6; private Object objectMapper; - private boolean diffFlatMode = true; /** * Set to true if you want eq("someProperty", null) to generate 1=1 rather than "is null" sql expression. @@ -2116,7 +2115,7 @@ public class ServerConfig { changeLogIncludeInserts = p.getBoolean("changeLogIncludeInserts", changeLogIncludeInserts); expressionEqualsWithNullAsNoop = p.getBoolean("expressionEqualsWithNullAsNoop", expressionEqualsWithNullAsNoop); - diffFlatMode = p.getBoolean("diffFlatMode", diffFlatMode); + asOfViewSuffix = p.get("asOfViewSuffix", asOfViewSuffix); asOfSysPeriod = p.get("asOfSysPeriod", asOfSysPeriod); historyTableSuffix = p.get("historyTableSuffix", historyTableSuffix); @@ -2229,22 +2228,6 @@ public class ServerConfig { 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; - } - /** * Return true if eq("someProperty", null) should to generate "1=1" rather than "is null" sql expression. */ 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 0aa536900..ab197fac3 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultServer.java @@ -132,8 +132,6 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { private final BeanDescriptorManager beanDescriptorManager; - private final DiffHelp diffHelp; - private final AutoFetchManager autoFetchManager; private final ReadAuditPrepare readAuditPrepare; @@ -217,7 +215,6 @@ public final class DefaultServer implements SpiServer, 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(); @@ -578,7 +575,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { } BeanDescriptor desc = getBeanDescriptor(a.getClass()); - return diffHelp.diff(a, b, desc); + return DiffHelp.diff(a, b, desc); } /** 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 85407c1cf..d542251ee 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DiffHelp.java @@ -1,14 +1,10 @@ package com.avaje.ebeaninternal.server.core; -import java.util.LinkedHashMap; -import java.util.Map; - import com.avaje.ebean.ValuePair; import com.avaje.ebean.bean.EntityBean; import com.avaje.ebeaninternal.server.deploy.BeanDescriptor; -import com.avaje.ebeaninternal.server.deploy.BeanProperty; -import com.avaje.ebeaninternal.server.deploy.BeanPropertyAssocOne; -import com.avaje.ebeaninternal.util.ValueUtil; + +import java.util.Map; /** * Helper to perform a diff given two beans of the same type. @@ -18,10 +14,7 @@ import com.avaje.ebeaninternal.util.ValueUtil; */ public class DiffHelp { - private final boolean flatMode; - - public DiffHelp(boolean flatMode) { - this.flatMode = flatMode; + private DiffHelp() { } /** @@ -34,123 +27,26 @@ public class DiffHelp { * This intentionally does not include as OneToMany or ManyToMany properties. *

*/ - public Map diff(Object newBean, Object oldBean, BeanDescriptor desc) { + public static Map diff(Object newBean, Object oldBean, BeanDescriptor desc) { if (!(newBean instanceof EntityBean)) { - throw new IllegalArgumentException("First bean expected to be an enhanced EntityBean? bean:"+newBean); + throw new IllegalArgumentException("First bean expected to be an enhanced EntityBean? bean:" + newBean); } if (oldBean != null) { if (!(oldBean instanceof EntityBean)) { - throw new IllegalArgumentException("Second bean expected to be an enhanced EntityBean? bean:"+oldBean); + throw new IllegalArgumentException("Second bean expected to be an enhanced EntityBean? bean:" + oldBean); } if (!newBean.getClass().isAssignableFrom(oldBean.getClass())) { throw new IllegalArgumentException("Second bean not assignable to the first bean?"); } } - - if (oldBean == null) { - return ((EntityBean) newBean)._ebean_getIntercept().getDirtyValues(); - } - Map map = new LinkedHashMap(); - diff(null, map, (EntityBean) newBean, (EntityBean) oldBean, desc); - return map; - } - - public void diff(String prefix, Map map, EntityBean newBean, EntityBean oldBean, BeanDescriptor desc) { - - if (flatMode) { - desc.diff(prefix, map, newBean, oldBean); - } else { - - // check the simple properties - BeanProperty[] base = desc.propertiesBaseScalar(); - for (int i = 0; i < base.length; i++) { - base[i].diff(prefix, map, newBean, oldBean); - } - - diffAssocOne(prefix, newBean, oldBean, desc, map); - diffEmbedded(prefix, newBean, oldBean, desc, map); + if (oldBean == null) { + return ((EntityBean) newBean)._ebean_getIntercept().getDirtyValues(); } - } - /** - * Check the Embedded bean properties for differences. - *

- * If ANY of the properties are different then the whole Embedded bean is - * determined to be different as is added to the map. - *

- */ - private void diffEmbedded(String prefix, EntityBean newBean, EntityBean oldBean, BeanDescriptor desc, Map map) { + return desc.diff((EntityBean) newBean, (EntityBean) oldBean); + } - BeanPropertyAssocOne[] emb = desc.propertiesEmbedded(); - - for (int i = 0; i < emb.length; i++) { - EntityBean newVal = (EntityBean)emb[i].getValue(newBean); - EntityBean oldVal = (EntityBean)emb[i].getValue(oldBean); - - if (!isBothNull(newVal, oldVal)) { - String propName = (prefix == null) ? emb[i].getName() : prefix + emb[i].getName(); - if (isDiffNull(newVal, oldVal)) { - // one of the embedded beans is null - if (flatMode) { - BeanDescriptor embDesc = emb[i].getTargetDescriptor(); - diff(propName, 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(propName, map, newVal, oldVal, embDesc); - } - } - } - } - - /** - * 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 newBean, EntityBean oldBean, BeanDescriptor desc, Map map) { - - BeanPropertyAssocOne[] ones = desc.propertiesOne(); - - for (int i = 0; i < ones.length; i++) { - Object newVal = ones[i].getValue(newBean); - Object oldVal = ones[i].getValue(oldBean); - - 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); - - 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)); - - } else { - map.put(propName, new ValuePair(newVal, oldVal)); - } - } - } - } - } - - private boolean isBothNull(Object newVal, Object oldVal) { - return newVal == null && oldVal == null; - } - - private boolean isDiffNull(Object newVal, Object oldVal) { - if (newVal == null) { - return oldVal != null; - } else { - return oldVal == null; - } - } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java index 8455aaf96..130ac332a 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptor.java @@ -2073,6 +2073,15 @@ public class BeanDescriptor implements MetaBeanInfo, SpiBeanType { } } + /** + * Return the diff comparing the bean values. + */ + public Map diff(EntityBean newBean, EntityBean oldBean) { + Map map = new LinkedHashMap(); + diff(null, map, newBean, oldBean); + return map; + } + /** * Populate the diff for updates with flattened non-null property values. */ 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 8288ef9f4..831d7442b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java @@ -35,8 +35,6 @@ public class CQueryEngine { private static final String T0 = "t0"; - private final DiffHelp diffHelp = new DiffHelp(true); - private final boolean forwardOnlyHintOnFindIterate; private final CQueryBuilder queryBuilder; @@ -259,7 +257,7 @@ public class CQueryEngine { private void deriveVersionDiff(Version current, Version prior, BeanDescriptor descriptor) { - Map diff = diffHelp.diff(current.getBean(), prior.getBean(), descriptor); + Map diff = DiffHelp.diff(current.getBean(), prior.getBean(), descriptor); current.setDiff(diff); } 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 118129115..f39f3b86f 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java +++ b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpSimple.java @@ -20,8 +20,6 @@ import com.avaje.tests.model.basic.Order.Status; public class TestDiffHelpSimple extends BaseTestCase { - DiffHelp diffHelp = new DiffHelp(false); - long firstTime = System.currentTimeMillis()-10000; long secondTime = System.currentTimeMillis(); @@ -59,7 +57,7 @@ public class TestDiffHelpSimple extends BaseTestCase { order2.setShipDate(new Date(secondTime)); order2.setOrderDate(new Date(secondTime)); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(5, diff.size()); @@ -68,7 +66,7 @@ public class TestDiffHelpSimple extends BaseTestCase { Assert.assertTrue(keySet.contains("status")); Assert.assertTrue(keySet.contains("shipDate")); Assert.assertTrue(keySet.contains("orderDate")); - Assert.assertTrue(keySet.contains("customer")); + Assert.assertTrue(keySet.contains("customer.id")); } @Test @@ -85,8 +83,7 @@ public class TestDiffHelpSimple extends BaseTestCase { order2.setShipDate(new Date(secondTime)); order2.setOrderDate(new Date(secondTime)); - DiffHelp diffHelp = new DiffHelp(true); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(5, diff.size()); @@ -107,7 +104,7 @@ public class TestDiffHelpSimple extends BaseTestCase { Order order2 = createBaseOrder(server); order2.setId(14); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(0, diff.size()); } @@ -122,13 +119,13 @@ public class TestDiffHelpSimple extends BaseTestCase { order2.setStatus(Status.COMPLETE); order2.setShipDate(null); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(3, diff.size()); Set keySet = diff.keySet(); Assert.assertTrue(keySet.contains("status")); - Assert.assertTrue(keySet.contains("customer")); + Assert.assertTrue(keySet.contains("customer.id")); Assert.assertTrue(keySet.contains("shipDate")); ValuePair shipDatePair = diff.get("shipDate"); @@ -147,7 +144,7 @@ public class TestDiffHelpSimple extends BaseTestCase { Order order2 = createBaseOrder(server); order2.setShipDate(new Date(secondTime)); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(1, diff.size()); Set keySet = diff.keySet(); @@ -168,7 +165,7 @@ public class TestDiffHelpSimple extends BaseTestCase { Order order2 = createBaseOrder(server); order2.setShipDate(null); - Map diff = diffHelp.diff(order1, order2, orderDesc); + Map diff = DiffHelp.diff(order1, order2, orderDesc); Assert.assertEquals(0, diff.size()); } 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 366ed9977..328d0b482 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java +++ b/src/test/java/com/avaje/ebeaninternal/server/core/TestDiffHelpWithEmbedded.java @@ -16,9 +16,8 @@ import com.avaje.tests.model.embedded.Eembeddable; public class TestDiffHelpWithEmbedded extends BaseTestCase { - DiffHelp diffHelp = new DiffHelp(false); - EbeanServer server; + BeanDescriptor emainDesc; public TestDiffHelpWithEmbedded() { @@ -35,7 +34,7 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { emain2.getEmbeddable().setDescription("baz"); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); ValuePair valuePair = diff.get("embeddable.description"); @@ -58,7 +57,7 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { emain2.setEmbeddable(embeddable); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); ValuePair valuePair = diff.get("embeddable.description"); @@ -74,9 +73,7 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { EMain emain2 = createEMain(); emain2.getEmbeddable().setDescription("baz"); - DiffHelp diffHelp = new DiffHelp(true); - - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); ValuePair valuePair = diff.get("embeddable.description"); @@ -92,14 +89,13 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { emain1.setEmbeddable(null); EMain emain2 = createEMain(); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); - ValuePair valuePair = diff.get("embeddable"); + ValuePair valuePair = diff.get("embeddable.description"); Assert.assertNotNull(valuePair); Assert.assertNull(valuePair.getNewValue()); - Assert.assertTrue(valuePair.getOldValue() instanceof Eembeddable); - Assert.assertEquals("bar",((Eembeddable)valuePair.getOldValue()).getDescription()); + Assert.assertEquals("bar", valuePair.getOldValue()); } @Test @@ -109,14 +105,13 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { EMain emain2 = createEMain(); emain2.setEmbeddable(null); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); - ValuePair valuePair = diff.get("embeddable"); + ValuePair valuePair = diff.get("embeddable.description"); Assert.assertNotNull(valuePair); Assert.assertNull(valuePair.getOldValue()); - Assert.assertTrue(valuePair.getNewValue() instanceof Eembeddable); - Assert.assertEquals("bar",((Eembeddable)valuePair.getNewValue()).getDescription()); + Assert.assertEquals("bar", valuePair.getNewValue()); } @Test @@ -126,8 +121,7 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { EMain emain2 = createEMain(); emain2.setEmbeddable(null); - DiffHelp diffHelp = new DiffHelp(true); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(1, diff.size()); ValuePair valuePair = diff.get("embeddable.description"); @@ -144,7 +138,7 @@ public class TestDiffHelpWithEmbedded extends BaseTestCase { EMain emain2 = createEMain(); emain2.setEmbeddable(null); - Map diff = diffHelp.diff(emain1, emain2, emainDesc); + Map diff = DiffHelp.diff(emain1, emain2, emainDesc); Assert.assertEquals(0, diff.size()); }