From fb13db6c4c0bb3474e5e92d16acd901933989c00 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Mon, 19 Dec 2011 16:29:27 +0000 Subject: [PATCH] ALF-7260: fixed diffs to work against multiple matching db objects It is possible that multiple database objects, e.g. indexes, match a given object as described in the schema reference file. This fix means that diffs will be provided against all the matching items, where previously only the first matching item was reported against. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@32864 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../util/schemacomp/ComparisonUtils.java | 9 +- .../schemacomp/DefaultComparisonUtils.java | 70 +++++----- .../DefaultComparisonUtilsTest.java | 128 +++++++++++------- 3 files changed, 117 insertions(+), 90 deletions(-) diff --git a/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java b/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java index fa881e7101..22201cbeaa 100644 --- a/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java +++ b/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java @@ -33,14 +33,7 @@ import org.alfresco.util.schemacomp.model.Schema; */ public interface ComparisonUtils { - /** - * @deprecated This method ignores the fact that multiple objects may match. - * @param objToFind - * @return - */ - DbObject findSameObjectAs(Collection objects, final DbObject objToFind); - - List findEquivalentObjects(Schema schema, DbObject objToMatch); + List findEquivalentObjects(DbObject rootObject, DbObject objToMatch); void compareSimpleCollections(DbProperty leftProperty, DbProperty rightProperty, DiffContext ctx, Strength strength); diff --git a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java index 7822f34269..4a90421440 100644 --- a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java +++ b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java @@ -36,32 +36,12 @@ import org.apache.commons.collections.Predicate; * @author Matt Ward */ public class DefaultComparisonUtils implements ComparisonUtils -{ - /** - * @param objToFind - * @return - */ +{ @Override - public DbObject findSameObjectAs(Collection objects, final DbObject objToFind) - { - return (DbObject) CollectionUtils.find(objects, new Predicate() - { - @Override - public boolean evaluate(Object o) - { - DbObject object = (DbObject) o; - return object.sameAs(objToFind); - } - }); - } - - - - @Override - public List findEquivalentObjects(Schema schema, DbObject objToMatch) + public List findEquivalentObjects(DbObject rootObject, DbObject objToMatch) { EquivalentObjectSeeker objectSeeker = new EquivalentObjectSeeker(objToMatch); - schema.accept(objectSeeker); + rootObject.accept(objectSeeker); return objectSeeker.getMatches(); } @@ -147,17 +127,27 @@ public class DefaultComparisonUtils implements ComparisonUtils Results differences = ctx.getComparisonResults(); for (DbObject leftObj : leftCollection) { - DbObject rightObj = findSameObjectAs(rightCollection, leftObj); - - if (rightObj != null) - { - // There is an equivalent object in the right hand collection as - // in the left. - leftObj.diff(rightObj, ctx, strength); + boolean foundMatch = false; + + for (DbObject rootObject : rightCollection) + { + List matches = findEquivalentObjects(rootObject, leftObj); + + for (DbObject match : matches) + { + // There is an equivalent object in the right hand collection as in the left. + leftObj.diff(match, ctx, strength); + } + + if (matches.size() > 0) + { + foundMatch = true; + } } - else + + if (!foundMatch) { - // No equivalent object in the right hand collection. + // No equivalent object in the target collection. differences.add(Where.ONLY_IN_REFERENCE, new DbProperty(leftObj, null), null, strength); } } @@ -165,9 +155,19 @@ public class DefaultComparisonUtils implements ComparisonUtils // Identify objects in the right collection but not the left for (DbObject rightObj : rightCollection) { - DbObject leftObj = findSameObjectAs(leftCollection, rightObj); + boolean foundMatch = false; - if (leftObj == null) + for (DbObject rootObject : leftCollection) + { + List matches = findEquivalentObjects(rootObject, rightObj); + if (matches.size() > 0) + { + foundMatch = true; + break; + } + } + + if (!foundMatch) { // No equivalent object in the left hand collection. differences.add(Where.ONLY_IN_TARGET, null, new DbProperty(rightObj, null), strength); @@ -251,7 +251,7 @@ public class DefaultComparisonUtils implements ComparisonUtils } - private static class EquivalentObjectSeeker implements DbObjectVisitor + public static class EquivalentObjectSeeker implements DbObjectVisitor { private final List matches = new ArrayList(); private final DbObject objToMatch; diff --git a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java index a84065e856..ae2e18b55f 100644 --- a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java +++ b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java @@ -35,9 +35,11 @@ import org.alfresco.util.schemacomp.Result.Strength; import org.alfresco.util.schemacomp.model.AbstractDbObject; import org.alfresco.util.schemacomp.model.DbObject; import org.hibernate.dialect.Dialect; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; @@ -89,14 +91,11 @@ public class DefaultComparisonUtilsTest @Test public void compareCollections() { - DbObject db1 = mock(DbObject.class); - when(db1.sameAs(db1)).thenReturn(true); - DbObject db2 = mock(DbObject.class); // only in left - when(db2.sameAs(db2)).thenReturn(true); - DbObject db3 = mock(DbObject.class); // only in right - when(db3.sameAs(db3)).thenReturn(true); - DbObject db4 = mock(DbObject.class); - when(db4.sameAs(db4)).thenReturn(true); + DbObject db1 = new DatabaseObject("db1"); + DbObject db2 = new DatabaseObject("db2"); // only in left + DbObject db3 = new DatabaseObject("db3"); // only in right + DbObject db4 = new DatabaseObject("db4"); + Collection left = new ArrayList(); Collections.addAll(left, db1, db2, db4); @@ -106,13 +105,34 @@ public class DefaultComparisonUtilsTest comparisonUtils.compareCollections(left, right, ctx, Strength.ERROR); - // Objects in both are asked for their differences - verify(db1).diff(db1, ctx, Strength.ERROR); - verify(db4).diff(db4, ctx, Strength.ERROR); - - // Objects in only one collections are marked as such + // Differences and ommissions are noticed... + verify(differences).add(Where.IN_BOTH_BUT_DIFFERENCE, new DbProperty(db1), new DbProperty(db1)); verify(differences).add(Where.ONLY_IN_REFERENCE, new DbProperty(db2), null, Strength.ERROR); verify(differences).add(Where.ONLY_IN_TARGET, null, new DbProperty(db3), Strength.ERROR); + verify(differences).add(Where.IN_BOTH_BUT_DIFFERENCE, new DbProperty(db4), new DbProperty(db4)); + } + + @Test + public void compareCollectionsWithMultipleMatches() + { + DbObject db2 = new DatabaseObject("db2"); + DbObject db3 = new DatabaseObject("db3"); + DbObject db4 = new DatabaseObject("db4"); + DbObject db1 = new DatabaseObject("db1", db2, db3); + + Collection left = new ArrayList(); + Collections.addAll(left, db1, db4); + + Collection right = new ArrayList(); + Collections.addAll(right, db1, db2, db3); + + comparisonUtils.compareCollections(left, right, ctx, Strength.ERROR); + + // Differences and ommissions are noticed... + verify(differences).add(Where.ONLY_IN_REFERENCE, new DbProperty(db4), null, Strength.ERROR); + verify(differences).add(Where.IN_BOTH_BUT_DIFFERENCE, new DbProperty(db1), new DbProperty(db1)); + verify(differences).add(Where.IN_BOTH_BUT_DIFFERENCE, new DbProperty(db1), new DbProperty(db2)); + verify(differences).add(Where.IN_BOTH_BUT_DIFFERENCE, new DbProperty(db1), new DbProperty(db3)); } @@ -195,40 +215,6 @@ public class DefaultComparisonUtilsTest return new DbProperty(obj, propName, -1, true, propValue); } - @Test - public void findSameObjectAsSuccessfulFind() - { - // Make a list of mock DbOjbect objects to test against - int numObjects = 20; - List dbObjects = createMockDbObjects(numObjects); - - // The reference that will be used to look for one 'the same' in the collection. - DbObject toFind = mock(DbObject.class); - - // For all other objects sameAs() will return false - DbObject objShouldBeFound = dbObjects.get(12); - when(objShouldBeFound.sameAs(toFind)).thenReturn(true); - - DbObject found = comparisonUtils.findSameObjectAs(dbObjects, toFind); - - assertSame("Found the wrong DbObject", objShouldBeFound, found); - } - - - @Test - public void findSameObjectAsNotFound() - { - // Make a list of mock DbOjbect objects to test against - int numObjects = 20; - List dbObjects = createMockDbObjects(numObjects); - - // The reference that will be used to look for one 'the same' in the collection. - DbObject toFind = mock(DbObject.class); - - DbObject found = comparisonUtils.findSameObjectAs(dbObjects, toFind); - - assertNull("Should not have found a matching DbObject", found); - } private List createMockDbObjects(int size) { @@ -263,4 +249,52 @@ public class DefaultComparisonUtilsTest return this.collection; } } + + + public static class DatabaseObject extends AbstractDbObject + { + private DbObject[] equivalentObjects = new DbObject[] {}; + + public DatabaseObject(String name) + { + super(null, name); + } + + public DatabaseObject(String name, DbObject... equivalentObjects) + { + this(name); + this.equivalentObjects = equivalentObjects; + } + + @Override + public void accept(DbObjectVisitor visitor) + { + visitor.visit(this); + } + + @Override + protected void doDiff(DbObject right, DiffContext ctx, Strength strength) + { + DbProperty leftProp = new DbProperty(this); + DbProperty rightProp = new DbProperty(right); + ctx.getComparisonResults().add(Where.IN_BOTH_BUT_DIFFERENCE, leftProp, rightProp); + } + + @Override + public boolean sameAs(DbObject other) + { + // We can tell this stub to treat certain other objects as 'the same' as this object + // by supplying them in the constructor. If this object is invoked with t.sameAs(o) + // and o is in the list of equivalent objects supplied in the constructor, then + // sameAs() will return true. Otherwise the default sameAs() implementation is used. + for (DbObject o : equivalentObjects) + { + if (other.equals(o)) + { + return true; + } + } + return super.sameAs(other); + } + } }