Skip to content

Commit

Permalink
Ignore containers when checking compared fields existence in the recu…
Browse files Browse the repository at this point in the history
…rsive comparison

Fix #3349

Containers support is addressed by #3354
  • Loading branch information
Joel Costigliola committed Feb 3, 2024
1 parent bad16ef commit ae62aca
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 20 deletions.
Expand Up @@ -16,6 +16,7 @@
import static java.lang.System.identityHashCode;
import static java.util.Collections.unmodifiableList;
import static java.util.Objects.requireNonNull;
import static org.assertj.core.internal.RecursiveHelper.isContainer;
import static org.assertj.core.api.recursive.comparison.FieldLocation.rootFieldLocation;
import static org.assertj.core.util.Arrays.array;
import static org.assertj.core.util.Arrays.isArray;
Expand Down Expand Up @@ -327,20 +328,6 @@ public boolean hasNoNullValues() {
return actual != null && expected != null;
}

private static boolean isContainer(Object o) {
return o instanceof Iterable ||
o instanceof Map ||
o instanceof Optional ||
o instanceof AtomicReference ||
o instanceof AtomicReferenceArray ||
o instanceof AtomicBoolean ||
o instanceof AtomicInteger ||
o instanceof AtomicIntegerArray ||
o instanceof AtomicLong ||
o instanceof AtomicLongArray ||
isArray(o);
}

public boolean hasPotentialCyclingValues() {
return isPotentialCyclingValue(actual) && isPotentialCyclingValue(expected);
}
Expand Down
Expand Up @@ -20,6 +20,7 @@
import static java.util.stream.Collectors.toSet;
import static org.assertj.core.configuration.ConfigurationProvider.CONFIGURATION_PROVIDER;
import static org.assertj.core.data.MapEntry.entry;
import static org.assertj.core.internal.RecursiveHelper.isContainer;
import static org.assertj.core.internal.TypeComparators.defaultTypeComparators;
import static org.assertj.core.util.Lists.list;
import static org.assertj.core.util.Sets.newLinkedHashSet;
Expand Down Expand Up @@ -1095,17 +1096,23 @@ void checkComparedFieldsExist(Object actual) {

private Optional<Entry<FieldLocation, String>> checkComparedFieldExists(Object actual, FieldLocation comparedFieldLocation) {
Object node = actual;
for (int nestingLevel = 0; nestingLevel < comparedFieldLocation.getDecomposedPath().size(); nestingLevel++) {
int nestingLevel = 0;
while (nestingLevel < comparedFieldLocation.getDecomposedPath().size()) {
if (node == null) {
// won't be able to get children nodes, assume the field is known as we can't check it
return Optional.empty();
}
if (isContainer(node)) {
// TODO: supported with https://github.com/assertj/assertj/issues/3354
return Optional.empty();
}
String comparedFieldNodeNameElement = comparedFieldLocation.getDecomposedPath().get(nestingLevel);
Set<String> nodeNames = introspectionStrategy.getChildrenNodeNamesOf(node);
if (!nodeNames.contains(comparedFieldNodeNameElement)) {
return Optional.of(entry(comparedFieldLocation, comparedFieldNodeNameElement));
}
node = introspectionStrategy.getChildNodeValue(comparedFieldNodeNameElement, node);
nestingLevel++;
}
return Optional.empty();
}
Expand Down
@@ -0,0 +1,29 @@
package org.assertj.core.internal;

import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicIntegerArray;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicLongArray;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.AtomicReferenceArray;

import static org.assertj.core.util.Arrays.isArray;

public class RecursiveHelper {
public static boolean isContainer(Object o) {
return o instanceof Iterable ||
o instanceof Map ||
o instanceof Optional ||
o instanceof AtomicReference ||
o instanceof AtomicReferenceArray ||
o instanceof AtomicBoolean ||
o instanceof AtomicInteger ||
o instanceof AtomicIntegerArray ||
o instanceof AtomicLong ||
o instanceof AtomicLongArray ||
isArray(o);
}
}
Expand Up @@ -300,7 +300,6 @@ private static Stream<Arguments> should_fail_when_non_existent_fields_specified(
Person alice = new Person("Alice");
Person jack = new Person("Jack");
Person joan = new Person("Joan");
Person joe = new Person("Joe");
john.neighbour = jack;
alice.neighbour = joan;
jack.neighbour = john;
Expand All @@ -318,8 +317,8 @@ private static Stream<Arguments> should_fail_when_non_existent_fields_specified(
arguments(john, alice, array("name", "neighbour", "number"), "{number}"),
arguments(john, alice, array("neighbor"), "{neighbor}"),
arguments(john, alice, array("neighbour.neighbor.name"), "{neighbor in <neighbour.neighbor.name>}"),
arguments(sherlockHolmes, drWatson, array("friends.other"), "{other in <friends.other>}"),
arguments(sherlockHolmes, drWatson, array("friends.name"), "{name in <friends.name>}"),
// TODO for https://github.com/assertj/assertj/issues/3354
// arguments(sherlockHolmes, drWatson, array("friends.other"), "{other in <friends.other>}"),
arguments(john, alice, array("neighbour.neighbour.name", "neighbour.neighbour.number"),
"{number in <neighbour.neighbour.number>}"));
}
Expand Down Expand Up @@ -426,15 +425,15 @@ void should_only_check_compared_fields_existence_at_the_root_level() {
.isEqualTo(expected);
}

class WithNames {
static class WithNames {
Collection<Name> names;

public WithNames(Collection<Name> names) {
this.names = names;
}
}

class Name {
static class Name {
String first;
String last;

Expand All @@ -444,4 +443,17 @@ public Name(String first, String last) {
}
}

// https://github.com/assertj/assertj/issues/3354
@Test
void checking_compared_fields_existence_should_skip_containers_in_field_location() {
// GIVEN
FriendlyPerson sherlock1 = new FriendlyPerson("Sherlock Holmes");
sherlock1.friends.add(new FriendlyPerson("Dr. John Watson"));
FriendlyPerson sherlock2 = new FriendlyPerson("Sherlock Holmes");
sherlock2.friends.add(new FriendlyPerson("Dr. John Watson"));
// WHEN/THEN
then(sherlock1).usingRecursiveComparison()
.comparingOnlyFields("friends.name")
.isEqualTo(sherlock2);
}
}

0 comments on commit ae62aca

Please sign in to comment.