Skip to content

Commit

Permalink
recursive equals improvements for unordered sets comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
@fiery-phoenix @joel-costigliola
fiery-phoenix authored and joel-costigliola committed Apr 2, 2017
1 parent 8d251c9 commit 21f1f9b
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 48 deletions.
151 changes: 105 additions & 46 deletions src/main/java/org/assertj/core/internal/DeepDifference.java
View file
Expand Up @@ -20,7 +20,6 @@
import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Deque;
Expand Down Expand Up @@ -60,10 +59,6 @@ private DualKey(List<String> path, Object key1, Object key2) {
this.key2 = key2;
}

private DualKey(Object key1, Object key2) {
this(new ArrayList<String>(), key1, key2);
}

@Override
public boolean equals(Object other) {
if (!(other instanceof DualKey)) {
Expand Down Expand Up @@ -140,7 +135,8 @@ public String toString() {
*
* @param a Object one to compare
* @param b Object two to compare
* @param comparatorByType
* @param comparatorByPropertyOrField comparators to compare properties or fields with the given names
* @param comparatorByType comparators to compare properties or fields with the given types
* @return the list of differences found or an empty list if objects are equivalent.
* Equivalent means that all field values of both subgraphs are the same,
* either at the field level or via the respectively encountered overridden
Expand All @@ -149,8 +145,14 @@ public String toString() {
public static List<Difference> determineDifferences(Object a, Object b,
Map<String, Comparator<?>> comparatorByPropertyOrField,
TypeComparators comparatorByType) {
return determineDifferences(a, b, null, comparatorByPropertyOrField, comparatorByType);
}

private static List<Difference> determineDifferences(Object a, Object b, List<String> parentPath,
Map<String, Comparator<?>> comparatorByPropertyOrField,
TypeComparators comparatorByType) {
final Set<DualKey> visited = new HashSet<>();
final Deque<DualKey> toCompare = initStack(a, b, visited);
final Deque<DualKey> toCompare = initStack(a, b, parentPath, comparatorByPropertyOrField, comparatorByType);
final List<Difference> differences = new ArrayList<>();

while (!toCompare.isEmpty()) {
Expand Down Expand Up @@ -237,21 +239,20 @@ public static List<Difference> determineDifferences(Object a, Object b,
continue;
}

// Handled unordered Sets. This is a slightly more expensive comparison because order cannot
// be assumed, a temporary Map must be created, however the comparison still runs in O(N) time.
if (key1 instanceof Set) {
if (!compareUnorderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare,
visited)) {
// Check any Collection that is List. In these cases, element
// order matters, therefore this comparison is faster than using unordered comparison.
if (key1 instanceof List) {
if (!compareOrderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare, visited)) {
differences.add(new Difference(currentPath, key1, key2));
continue;
}
continue;
}

// Check any Collection that is not a Set. In these cases, element
// order matters, therefore this comparison is faster than using unordered comparison.
// Handle unordered Collection.
if (key1 instanceof Collection) {
if (!compareOrderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare, visited)) {
if (!compareUnorderedCollection((Collection<?>) key1, (Collection<?>) key2, currentPath, toCompare,
visited, comparatorByPropertyOrField, comparatorByType)) {
differences.add(new Difference(currentPath, key1, key2));
continue;
}
Expand Down Expand Up @@ -287,15 +288,20 @@ public static List<Difference> determineDifferences(Object a, Object b,
continue;
}

for (Field field : getDeclaredFieldsIncludingInherited(key1.getClass())) {
List<String> path = new ArrayList<String>(currentPath);
String fieldName = field.getName();
path.add(fieldName);
DualKey dk = new DualKey(path,
COMPARISON.getSimpleValue(fieldName, key1),
COMPARISON.getSimpleValue(fieldName, key2));
if (!visited.contains(dk)) {
toCompare.addFirst(dk);
Set<String> key1FieldsNames = getFieldsNames(getDeclaredFieldsIncludingInherited(key1.getClass()));
Set<String> key2FieldsNames = getFieldsNames(getDeclaredFieldsIncludingInherited(key2.getClass()));
if (!key2FieldsNames.containsAll(key1FieldsNames)) {
differences.add(new Difference(currentPath, key1, key2));
} else {
for (String fieldName : key1FieldsNames) {
List<String> path = new ArrayList<>(currentPath);
path.add(fieldName);
DualKey dk = new DualKey(path,
COMPARISON.getSimpleValue(fieldName, key1),
COMPARISON.getSimpleValue(fieldName, key2));
if (!visited.contains(dk)) {
toCompare.addFirst(dk);
}
}
}
}
Expand All @@ -307,37 +313,58 @@ private static boolean hasCustomComparator(DualKey dualKey, Map<String, Comparat
TypeComparators comparatorByType) {
if (dualKey.key1.getClass() == dualKey.key2.getClass()) {
String fieldName = dualKey.getConcatenatedPath();
Comparator<?> fieldComparator = comparatorByPropertyOrField.containsKey(fieldName)
? comparatorByPropertyOrField.get(fieldName) : comparatorByType.get(dualKey.key1.getClass());
Comparator<?> fieldComparator = comparatorByPropertyOrField != null && comparatorByPropertyOrField.containsKey(fieldName)
? comparatorByPropertyOrField.get(fieldName) : comparatorByType == null
? null : comparatorByType.get(dualKey.key1.getClass());
return fieldComparator != null;
}
return false;
}

private static Deque<DualKey> initStack(Object a, Object b, Set<DualKey> visited) {
private static Deque<DualKey> initStack(Object a, Object b, List<String> parentPath,
Map<String, Comparator<?>> comparatorByPropertyOrField,
TypeComparators comparatorByType) {
Deque<DualKey> stack = new LinkedList<>();
if (a != null && !isContainerType(a)) {
List<String> currentPath = parentPath == null ? new ArrayList<String>() : parentPath;
DualKey basicDualKey = new DualKey(currentPath, a, b);
if (a != null && b != null && (parentPath == null || !hasCustomComparator(basicDualKey, comparatorByPropertyOrField, comparatorByType))
&& !isContainerType(a) && !isContainerType(b)) {
// disregard the equals method and start comparing fields
Collection<Field> fieldsOfRootObject = getDeclaredFieldsIncludingInherited(a.getClass());
if (!fieldsOfRootObject.isEmpty()) {
for (Field field : fieldsOfRootObject) {
String fieldName = field.getName();
DualKey dk = new DualKey(Arrays.asList(fieldName),
COMPARISON.getSimpleValue(fieldName, a),
COMPARISON.getSimpleValue(fieldName, b));
if (!visited.contains(dk)) {
Collection<Field> aFields = getDeclaredFieldsIncludingInherited(a.getClass());
if (!aFields.isEmpty()) {
Set<String> aFieldsNames = getFieldsNames(aFields);
Set<String> bFieldsNames = getFieldsNames(getDeclaredFieldsIncludingInherited(b.getClass()));
if (!bFieldsNames.containsAll(aFieldsNames)) {
stack.addFirst(basicDualKey);
} else {
for (Field field : aFields) {
String fieldName = field.getName();
List<String> fieldPath = new ArrayList<>(currentPath);
fieldPath.add(fieldName);
DualKey dk = new DualKey(fieldPath,
COMPARISON.getSimpleValue(fieldName, a),
COMPARISON.getSimpleValue(fieldName, b));
stack.addFirst(dk);
}
}
} else {
stack.addFirst(new DualKey(a, b));
stack.addFirst(basicDualKey);
}
} else {
stack.addFirst(new DualKey(a, b));
stack.addFirst(basicDualKey);
}
return stack;
}

private static Set<String> getFieldsNames(Collection<Field> fields) {
Set<String> result = new HashSet<>();
for (Field field : fields) {
result.add(field.getName());
}

return result;
}

private static boolean isContainerType(Object o) {
return o instanceof Collection || o instanceof Map;
}
Expand Down Expand Up @@ -398,9 +425,7 @@ private static <K, V> boolean compareOrderedCollection(Collection<K> col1, Colle
}

/**
* Deeply compare the two sets referenced by dualKey. This method attempts
* to quickly determine inequality by length, then if lengths match, it
* places one collection into a temporary Map by deepHashCode(), so that it
* It places one collection into a temporary Map by deepHashCode(), so that it
* can walk the other collection and look for each item in the map, which
* runs in O(N) time, rather than an O(N^2) lookup that would occur if each
* item from collection one was scanned for in collection two.
Expand All @@ -415,13 +440,9 @@ private static <K, V> boolean compareOrderedCollection(Collection<K> col1, Colle
* value of 'true' indicates that the Collections may be equal, and
* the sets items will be added to the Stack for further comparison.
*/
private static <K, V> boolean compareUnorderedCollection(Collection<K> col1, Collection<V> col2,
private static <K, V> boolean compareUnorderedCollectionByHashCodes(Collection<K> col1, Collection<V> col2,
List<String> path, Deque<DualKey> toCompare,
Set<DualKey> visited) {
if (col1.size() != col2.size()) {
return false;
}

Map<Integer, Object> fastLookup = new HashMap<>();
for (Object o : col2) {
fastLookup.put(deepHashCode(o), o);
Expand All @@ -442,6 +463,44 @@ private static <K, V> boolean compareUnorderedCollection(Collection<K> col1, Col
return true;
}

/**
* Deeply compares two collections referenced by dualKey. This method attempts
* to quickly determine inequality by length, then if lengths match, in case of
* collection type is Set and there are passed no custom comparators, there is used
* comparison on hashcodes basis, otherwise each element from one collection is checked
* for existence in another one using 'deep' comparison.
*/
private static <K, V> boolean compareUnorderedCollection(Collection<K> col1, Collection<V> col2,
List<String> path, Deque<DualKey> toCompare,
Set<DualKey> visited,
Map<String, Comparator<?>> comparatorByPropertyOrField,
TypeComparators comparatorByType) {
if (col1.size() != col2.size()) {
return false;
}

boolean hasNoCustomComparators = (comparatorByPropertyOrField == null || comparatorByPropertyOrField.isEmpty())
&& (comparatorByType == null || comparatorByType.isEmpty());
if (hasNoCustomComparators && col1 instanceof Set) {
// this comparison is used for performance optimization reasons
return compareUnorderedCollectionByHashCodes(col1, col2, path, toCompare, visited);
}

Collection<V> col2Copy = new LinkedList<>(col2);
for (Object o1 : col1) {
Iterator<V> iterator = col2Copy.iterator();
while (iterator.hasNext()) {
Object o2 = iterator.next();
if (determineDifferences(o1, o2, path, comparatorByPropertyOrField, comparatorByType).isEmpty()) {
iterator.remove();
break;
}
}
}

return col2Copy.isEmpty();
}

/**
* Deeply compare two SortedMap instances. This method walks the Maps in
* order, taking advantage of the fact that the Maps are SortedMaps.
Expand Down
67 changes: 65 additions & 2 deletions src/test/java/org/assertj/core/internal/DeepDifference_Test.java
View file
Expand Up @@ -23,6 +23,8 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.internal.ObjectsBaseTest.defaultTypeComparators;
import static org.assertj.core.internal.ObjectsBaseTest.noFieldComparators;
import static org.assertj.core.test.AlwaysEqualStringComparator.ALWAY_EQUALS;
import static org.assertj.core.util.BigDecimalComparator.BIG_DECIMAL_COMPARATOR;
import static org.assertj.core.util.Lists.newArrayList;
import static org.assertj.core.util.Sets.newLinkedHashSet;

Expand Down Expand Up @@ -78,6 +80,12 @@ public void testBigDecimals() {
assertHaveNoDifferences(bigDecimal1, bigDecimal2);
}

@Test
public void testWithDifferentFields() {
assertHaveDifferences("one", 1);
assertHaveDifferences(new Wrapper(new Wrapper("one")), new Wrapper("one"));
}

@Test
public void testPOJOequals() {
Class1 x = new Class1(true, tan(PI / 4), 1);
Expand Down Expand Up @@ -130,18 +138,45 @@ public void testUnorderedCollection() {
Set<String> a = newLinkedHashSet("one", "two", "three", "four", "five");
Set<String> b = newLinkedHashSet("three", "five", "one", "four", "two");
assertHaveNoDifferences(a, b);
assertHaveNoDifferences(a, b, noFieldComparators(), new TypeComparators());

Set<Integer> c = newLinkedHashSet(1, 2, 3, 4, 5);
assertHaveDifferences(a, c);
assertHaveDifferences(a, c, noFieldComparators(), null);

Set<Integer> d = newLinkedHashSet(4, 2, 6);
assertHaveDifferences(c, d);
assertHaveDifferences(c, d, noFieldComparators(), null);

Set<Class1> x1 = newLinkedHashSet(new Class1(true, log(pow(E, 2)), 6), new Class1(true, tan(PI / 4), 1));
Set<Class1> x2 = newLinkedHashSet(new Class1(true, 1, 1), new Class1(true, 2, 6));
assertHaveNoDifferences(x1, x2);
}

@Test
public void testUnorderedCollectionWithCustomComparatorsByType() {
TypeComparators comparatorsWithBigDecimalComparator = new TypeComparators();
comparatorsWithBigDecimalComparator.put(BigDecimal.class, BIG_DECIMAL_COMPARATOR);

Set<BigDecimal> a = newLinkedHashSet(new BigDecimal("1.0"), new BigDecimal("3"), new BigDecimal("2"), new BigDecimal("4"));
Set<BigDecimal> b = newLinkedHashSet(new BigDecimal("4"), new BigDecimal("1"), new BigDecimal("2.0"), new BigDecimal("3"));

assertHaveNoDifferences(a, b, noFieldComparators(), comparatorsWithBigDecimalComparator);

Set<BigDecimal> c = newLinkedHashSet(new BigDecimal("4"), new BigDecimal("1"), new BigDecimal("2.2"), new BigDecimal("3"));
assertHaveDifferences(a, c, noFieldComparators(), comparatorsWithBigDecimalComparator);
}

@Test
public void testUnorderedCollectionWithCustomComparatorsByFieldName() {
SetWrapper a = new SetWrapper(newLinkedHashSet(new Wrapper("one"), new Wrapper("two")));
SetWrapper b = new SetWrapper(newLinkedHashSet(new Wrapper("1"), new Wrapper("2")));

Map<String, Comparator<?>> fieldComparators = new HashMap<>();
fieldComparators.put("set.o", ALWAY_EQUALS);
assertHaveNoDifferences(a, b, fieldComparators, defaultTypeComparators());
}

@Test
public void testEquivalentMaps() {
Map<String, Integer> map1 = new LinkedHashMap<>();
Expand Down Expand Up @@ -288,11 +323,19 @@ public int compare(Map o1, Map o2) {
}

private void assertHaveNoDifferences(Object x, Object y) {
assertThat(DeepDifference.determineDifferences(x, y, noFieldComparators(), defaultTypeComparators())).isEmpty();
assertHaveNoDifferences(x, y, noFieldComparators(), defaultTypeComparators());
}

private void assertHaveNoDifferences(Object x, Object y, Map<String, Comparator<?>> fieldComparators, TypeComparators typeComparators) {
assertThat(DeepDifference.determineDifferences(x, y, fieldComparators, typeComparators)).isEmpty();
}

private void assertHaveDifferences(Object x, Object y) {
assertThat(DeepDifference.determineDifferences(x, y, noFieldComparators(), defaultTypeComparators())).isNotEmpty();
assertHaveDifferences(x, y, noFieldComparators(), defaultTypeComparators());
}

private void assertHaveDifferences(Object x, Object y, Map<String, Comparator<?>> fieldComparators, TypeComparators typeComparators) {
assertThat(DeepDifference.determineDifferences(x, y, fieldComparators, typeComparators)).isNotEmpty();
}

private static class EmptyClass {
Expand Down Expand Up @@ -352,6 +395,26 @@ public Class2() {
}
}

private static class Wrapper {

@SuppressWarnings("unused")
private Object o;

private Wrapper(Object o) {
this.o = o;
}
}

private static class SetWrapper {

@SuppressWarnings("unused")
private Set<?> set;

private SetWrapper(Set<?> set) {
this.set = set;
}
}

private void fillMap(Map<String, Integer> map) {
map.put("zulu", 26);
map.put("alpha", 1);
Expand Down

0 comments on commit 21f1f9b

Please sign in to comment.