Skip to content

Commit

Permalink
Make IterableDiff to always compare actual elements to expected eleme…
Browse files Browse the repository at this point in the history
…nts and not the the other way around in case the comparison is not symmetrical.

This occurs with the recursive comparison when the types being compared are not the same and the actual type has less fields than the expected type (the recursive comparison ignores fields non defined in actual type)
Fixes assertj#2147
  • Loading branch information
joel-costigliola committed Mar 21, 2021
1 parent 7b5d1d3 commit 06d3bfc
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 13 deletions.
48 changes: 36 additions & 12 deletions src/main/java/org/assertj/core/internal/IterableDiff.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ class IterableDiff {
<T> IterableDiff(Iterable<T> actual, Iterable<T> expected, ComparisonStrategy comparisonStrategy) {
this.comparisonStrategy = comparisonStrategy;
// return the elements in actual that are not in expected: actual - expected
this.unexpected = subtract(actual, expected);
this.unexpected = unexpectedActualElements(actual, expected);
// return the elements in expected that are not in actual: expected - actual
this.missing = subtract(expected, actual);
this.missing = missingActualElements(actual, expected);
}

static <T> IterableDiff diff(Iterable<T> actual, Iterable<T> expected, ComparisonStrategy comparisonStrategy) {
Expand All @@ -46,27 +46,51 @@ boolean differencesFound() {
* Returns the list of elements in the first iterable that are not in the second, i.e. first - second
*
* @param <T> the element type
* @param first the list we want to subtract from
* @param second the list to subtract
* @param actual the list we want to subtract from
* @param expected the list to subtract
* @return the list of elements in the first iterable that are not in the second, i.e. first - second
*/
private <T> List<Object> subtract(Iterable<T> first, Iterable<T> second) {
private <T> List<Object> unexpectedActualElements(Iterable<T> actual, Iterable<T> expected) {
List<Object> missingInFirst = new ArrayList<>();
// use a copy to deal correctly with potential duplicates
List<T> copyOfSecond = newArrayList(second);
for (Object elementInFirst : first) {
if (iterableContains(copyOfSecond, elementInFirst)) {
List<T> copyOfExpected = newArrayList(expected);
for (Object elementInActual : actual) {
if (isActualElementInExpected(elementInActual, copyOfExpected)) {
// remove the element otherwise a duplicate would be found in the case if there is one in actual
iterablesRemoveFirst(copyOfSecond, elementInFirst);
iterablesRemoveFirst(copyOfExpected, elementInActual);
} else {
missingInFirst.add(elementInFirst);
missingInFirst.add(elementInActual);
}
}
return unmodifiableList(missingInFirst);
}

private boolean iterableContains(Iterable<?> actual, Object value) {
return comparisonStrategy.iterableContains(actual, value);
private <T> boolean isActualElementInExpected(Object elementInActual, List<T> copyOfExpected) {
// the order of comparisonStrategy.areEqual is important if element comparison is not symmetrical, we must compare actual to
// expected but not expected to actual, for ex recursive comparison where:
// - actual element is PersonDto, expected a list of Person
// - Person has more fields than PersonDto => comparing PersonDto to Person is ok as it looks at PersonDto fields only,
// --- the opposite always fails as the reference fields are Person fields and PersonDto does not have all of them.
return copyOfExpected.stream().anyMatch(expectedElement -> comparisonStrategy.areEqual(elementInActual, expectedElement));
}

private <T> List<Object> missingActualElements(Iterable<T> actual, Iterable<T> expected) {
List<Object> missingInExpected = new ArrayList<>();
// use a copy to deal correctly with potential duplicates
List<T> copyOfActual = newArrayList(actual);
for (Object expectedElement : expected) {
if (iterableContains(copyOfActual, expectedElement)) {
// remove the element otherwise a duplicate would be found in the case if there is one in actual
iterablesRemoveFirst(copyOfActual, expectedElement);
} else {
missingInExpected.add(expectedElement);
}
}
return unmodifiableList(missingInExpected);
}

private boolean iterableContains(Iterable<?> actual, Object expectedElement) {
return comparisonStrategy.iterableContains(actual, expectedElement);
}

private void iterablesRemoveFirst(Iterable<?> actual, Object value) {
Expand Down
59 changes: 58 additions & 1 deletion src/test/java/org/assertj/core/internal/IterableDiff_Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.internal.IterableDiff.diff;
import static org.assertj.core.util.Lists.list;
import static org.assertj.core.util.Lists.newArrayList;

import java.util.List;
Expand Down Expand Up @@ -105,7 +106,7 @@ void should_not_report_any_differences_between_two_case_sensitive_iterables_acco
expected = newArrayList("A", "B", "C", "D");
// WHEN
IterableDiff diff = diff(actual, expected, comparisonStrategy);
// THEN
// THEN6
assertThatNoDiff(diff);
}

Expand Down Expand Up @@ -147,4 +148,60 @@ private static void assertThatNoDiff(IterableDiff diff) {
assertThat(diff.unexpected).isEmpty();
}

// issue #2147
@Test
void should_work_when_comparison_strategy_is_not_symmetrical() {
// GIVEN
Address address1 = new Address(12, "xyz", "abc", "432432", "asdsa");
Address address2 = new Address(13, "xyzx", "abcds", "32432432", "asdsdfsa");
Address address3 = new Address(14, "xyzsa", "axbc", "4sd32432", "asdsfsda");
List<AddressDto> addressDtoList = list(AddressDto.from(address1), AddressDto.from(address2), AddressDto.from(address3));
// WHEN/THEN
assertThat(addressDtoList).usingRecursiveComparison()
.isEqualTo(list(address1, address2, address3));
assertThat(addressDtoList).asList()
.usingRecursiveFieldByFieldElementComparator()
.containsExactly(address1, address2, address3);
}

static class Address {
int id;
String randomProperty;
String name;
String secondName;
String street;
String postCode;

public Address(int id, String name, String secondName, String street, String postCode) {
this.id = id;
this.name = name;
this.secondName = secondName;
this.street = street;
this.postCode = postCode;
}

}

@SuppressWarnings("unused")
static class AddressDto {
private final int id;
private final String name;
private final String secondName;
private final String street;
private final String postCode;

public AddressDto(int id, String name, String secondName, String street, String postCode) {
this.id = id;
this.name = name;
this.secondName = secondName;
this.street = street;
this.postCode = postCode;
}

static AddressDto from(Address le) {
return new AddressDto(le.id, le.name, le.secondName, le.street, le.postCode);
}

}

}

0 comments on commit 06d3bfc

Please sign in to comment.