Skip to content

Commit

Permalink
fix: containsExactly does not work properly with maps not using equal…
Browse files Browse the repository at this point in the history 
…s to compare keys (with contribution from Sára Juhošová)

Fix #2165
  • Loading branch information
@wouterpolet @joel-costigliola
wouterpolet authored and joel-costigliola committed Nov 4, 2023
1 parent 2c62b77 commit 498ee5b
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 27 deletions.
37 changes: 35 additions & 2 deletions assertj-core/src/main/java/org/assertj/core/internal/Maps.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,10 @@ private static <K, V> Map<K, V> clone(Map<K, V> map) throws NoSuchMethodExceptio
}
}

private static boolean isSingletonMap(Map<?, ?> map) {
return map.size() == 1;
}

private static boolean isMultiValueMapAdapterInstance(Map<?, ?> map) {
return isInstanceOf(map, "org.springframework.util.MultiValueMapAdapter");
}
Expand All @@ -453,6 +457,16 @@ private static boolean isInstanceOf(Object object, String className) {
}
}

private static <K, V> Map<K, V> createEmptyMap(Map<K, V> map) {
try {
Map<K, V> cloned = clone(map);
cloned.clear();
return cloned;
} catch (NoSuchMethodException | RuntimeException e) {
return new LinkedHashMap<>();
}
}

public <K, V> void assertContainsValue(AssertionInfo info, Map<K, V> actual, V value) {
assertNotNull(info, actual);
if (!containsValue(actual, value)) throw failures.failure(info, shouldContainValue(actual, value));
Expand Down Expand Up @@ -551,6 +565,16 @@ public <K, V> void assertContainsExactly(AssertionInfo info, Map<K, V> actual, E
failIfEntriesIsEmptySinceActualIsNotEmpty(info, actual, entries);
assertHasSameSizeAs(info, actual, entries);

if (isSingletonMap(actual)) {
// shortcut for any singleton map but specifically for org.apache.commons.collections4.map.SingletonMap that is immutable
// and fail when we try to remove elements from them in compareActualMapAndExpectedEntries
// we only have to compare the map unique element
if (!actual.containsKey(entries[0].getKey()) || !actual.containsValue(entries[0].getValue())) {
throw failures.failure(info, elementsDifferAtIndex(actual.entrySet().iterator().next(), entries[0], 0));
}
return;
}

Set<Entry<? extends K, ? extends V>> notFound = new LinkedHashSet<>();
Set<Entry<? extends K, ? extends V>> notExpected = new LinkedHashSet<>();

Expand All @@ -559,11 +583,15 @@ public <K, V> void assertContainsExactly(AssertionInfo info, Map<K, V> actual, E
if (notExpected.isEmpty() && notFound.isEmpty()) {
// check entries order
int index = 0;
// Create a map with the same type as actual to use the Map built-in comparison, ex: maps string case insensitive keys.
Map<K, V> emptyMap = createEmptyMap(actual);
for (K keyFromActual : actual.keySet()) {
if (!deepEquals(keyFromActual, entries[index].getKey())) {
emptyMap.put(keyFromActual, null);
if (!emptyMap.containsKey(entries[index].getKey())) {
Entry<K, V> actualEntry = entry(keyFromActual, actual.get(keyFromActual));
throw failures.failure(info, elementsDifferAtIndex(actualEntry, entries[index], index));
}
emptyMap.remove(keyFromActual);
index++;
}
// all entries are in the same order.
Expand All @@ -577,7 +605,12 @@ private <K, V> void compareActualMapAndExpectedEntries(Map<K, V> actual, Entry<?
Set<Entry<? extends K, ? extends V>> notExpected,
Set<Entry<? extends K, ? extends V>> notFound) {
Map<K, V> expectedEntries = entriesToMap(entries);
Map<K, V> actualEntries = new LinkedHashMap<>(actual);
Map<K, V> actualEntries = null;

Check warning on line 608 in assertj-core/src/main/java/org/assertj/core/internal/Maps.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused assignment 

Variable `actualEntries` initializer `null` is redundant
try {
actualEntries = clone(actual);
} catch (NoSuchMethodException | RuntimeException e) {
actualEntries = new LinkedHashMap<>(actual);
}
for (Entry<K, V> entry : expectedEntries.entrySet()) {
if (containsEntry(actualEntries, entry(entry.getKey(), entry.getValue()))) {
// this is an expected entry
Expand Down
3 changes: 3 additions & 0 deletions assertj-core/src/test/java/org/assertj/core/internal/MapsBaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ public class MapsBaseTest extends WithPlayerData {
MapsBaseTest::persistentMap,
MapsBaseTest::persistentSortedMap);

protected static final Supplier<Map<String, String>> PERSISTENT_MAP = MapsBaseTest::persistentMap;
protected static final Supplier<Map<String, String>> PERSISTENT_SORTED_MAP = MapsBaseTest::persistentSortedMap;

private static <K, V> PersistentMap<K, V> persistentMap() {
return hibernateMap(PersistentMap::new, HashMap::new);
}
Expand Down
188 changes: 163 additions & 25 deletions ...tj-core/src/test/java/org/assertj/core/internal/maps/Maps_assertContainsExactly_Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,50 @@
package org.assertj.core.internal.maps;

import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static java.util.Collections.unmodifiableMap;
import static java.util.stream.Stream.concat;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatNullPointerException;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
import static org.assertj.core.api.BDDAssertions.entry;
import static org.assertj.core.api.BDDAssertions.then;
import static org.assertj.core.data.MapEntry.entry;
import static org.assertj.core.error.ShouldBeEmpty.shouldBeEmpty;
import static org.assertj.core.error.ShouldContainExactly.elementsDifferAtIndex;
import static org.assertj.core.error.ShouldContainExactly.shouldContainExactly;
import static org.assertj.core.error.ShouldHaveSameSizeAs.shouldHaveSameSizeAs;
import static org.assertj.core.internal.ErrorMessages.entriesToLookForIsNull;
import static org.assertj.core.test.TestData.someInfo;
import static org.assertj.core.test.Maps.mapOf;
import static org.assertj.core.util.Arrays.array;
import static org.assertj.core.util.Arrays.asList;
import static org.assertj.core.util.AssertionsUtil.assertThatAssertionErrorIsThrownBy;
import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
import static org.assertj.core.util.FailureMessages.actualIsNull;
import static org.assertj.core.util.Lists.list;
import static org.assertj.core.util.Sets.set;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.mockito.Mockito.verify;

import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Stream;

import org.assertj.core.api.AssertionInfo;
import org.apache.commons.collections4.map.CaseInsensitiveMap;
import org.apache.commons.collections4.map.SingletonMap;
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import org.assertj.core.data.MapEntry;
import org.assertj.core.internal.MapsBaseTest;
import org.assertj.core.test.jdk11.Jdk11;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import com.google.common.collect.ImmutableMap;

/**
* Tests for
Expand All @@ -66,7 +82,7 @@ void should_fail_if_actual_is_null() {
actual = null;
MapEntry<String, String>[] expected = array(entry("name", "Yoda"));
// WHEN
AssertionError assertionError = expectAssertionError(() -> maps.assertContainsExactly(someInfo(), actual, expected));
AssertionError assertionError = expectAssertionError(() -> maps.assertContainsExactly(info, actual, expected));
// THEN
then(assertionError).hasMessage(actualIsNull());
}
Expand All @@ -76,14 +92,13 @@ void should_fail_if_given_entries_array_is_null() {
// GIVEN
MapEntry<String, String>[] entries = null;
// WHEN/THEN
assertThatNullPointerException().isThrownBy(() -> maps.assertContainsExactly(someInfo(), linkedActual, entries))
assertThatNullPointerException().isThrownBy(() -> maps.assertContainsExactly(info, linkedActual, entries))
.withMessage(entriesToLookForIsNull());
}

@Test
void should_fail_if_given_entries_array_is_empty() {
// GIVEN
AssertionInfo info = someInfo();
MapEntry<String, String>[] expected = emptyEntries();
// WHEN
expectAssertionError(() -> maps.assertContainsExactly(info, actual, expected));
Expand All @@ -93,18 +108,16 @@ void should_fail_if_given_entries_array_is_empty() {

@Test
void should_pass_if_actual_and_entries_are_empty() {
maps.assertContainsExactly(someInfo(), emptyMap(), array());
maps.assertContainsExactly(info, emptyMap(), array());
}

@Test
void should_pass_if_actual_contains_given_entries_in_order() {
maps.assertContainsExactly(someInfo(), linkedActual, array(entry("name", "Yoda"), entry("color", "green")));
maps.assertContainsExactly(info, linkedActual, array(entry("name", "Yoda"), entry("color", "green")));
}

@Test
void should_fail_if_actual_contains_given_entries_in_disorder() {
// GIVEN
AssertionInfo info = someInfo();
// WHEN
expectAssertionError(() -> maps.assertContainsExactly(info, linkedActual,
array(entry("color", "green"), entry("name", "Yoda"))));
Expand All @@ -115,7 +128,6 @@ void should_fail_if_actual_contains_given_entries_in_disorder() {
@Test
void should_fail_if_actual_and_expected_entries_have_different_size() {
// GIVEN
AssertionInfo info = someInfo();
MapEntry<String, String>[] expected = array(entry("name", "Yoda"));
// WHEN
ThrowingCallable code = () -> maps.assertContainsExactly(info, linkedActual, expected);
Expand All @@ -127,28 +139,160 @@ void should_fail_if_actual_and_expected_entries_have_different_size() {
@Test
void should_fail_if_actual_does_not_contains_every_expected_entries_and_contains_unexpected_one() {
// GIVEN
AssertionInfo info = someInfo();
MapEntry<String, String>[] expected = array(entry("name", "Yoda"), entry("color", "green"));
Map<String, String> underTest = newLinkedHashMap(entry("name", "Yoda"), entry("job", "Jedi"));
// WHEN
expectAssertionError(() -> maps.assertContainsExactly(info, underTest, expected));
// THEN
verify(failures).failure(info, shouldContainExactly(underTest, list(expected),
newHashSet(entry("color", "green")),
newHashSet(entry("job", "Jedi"))));
set(entry("color", "green")),
set(entry("job", "Jedi"))));
}

@Test
void should_fail_if_actual_contains_entry_key_with_different_value() {
// GIVEN
AssertionInfo info = someInfo();
MapEntry<String, String>[] expectedEntries = array(entry("name", "Yoda"), entry("color", "yellow"));
// WHEN
expectAssertionError(() -> maps.assertContainsExactly(info, actual, expectedEntries));
// THEN
verify(failures).failure(info, shouldContainExactly(actual, asList(expectedEntries),
newHashSet(entry("color", "yellow")),
newHashSet(entry("color", "green"))));
set(entry("color", "yellow")),
set(entry("color", "green"))));
}

@ParameterizedTest
@MethodSource({
"orderedSensitiveSuccessfulArguments",
"orderedInsensitiveSuccessfulArguments",
"unorderedSensitiveSuccessfulArguments",
"unorderedInsensitiveSuccessfulArguments"
})
void should_pass(Map<String, String> map, MapEntry<String, String>[] entries) {
assertThatNoException().as(map.getClass().getName())
.isThrownBy(() -> maps.assertContainsExactly(info, map, entries));
}

@ParameterizedTest
@MethodSource({
"orderedSensitiveFailureArguments",
"orderedInsensitiveFailureArguments",
"unorderedSensitiveFailureArguments",
"unorderedInsensitiveFailureArguments"
})
void should_fail(Map<String, String> map, MapEntry<String, String>[] entries) {
assertThatExceptionOfType(AssertionError.class).as(map.getClass().getName())
.isThrownBy(() -> maps.assertContainsExactly(info, map, entries));
}

private static Stream<MapEntry<String, String>[]> orderedFailureTestCases() {
return Stream.of(array(entry("potato", "vegetable")),
array(entry("banana", "fruit"), entry("potato", "vegetable"), entry("tomato", "vegetable")),
array(entry("banana", "fruit"), entry("tomato", "vegetable")),
array(entry("banana", "fruit"), entry("potato", "food")),
array(entry("potato", "vegetable"), entry("banana", "fruit")));
}

private static Stream<MapEntry<String, String>[]> orderedSuccessTestCases() {
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "fruit"), entry("poTATo", "vegetable")));
}

private static Stream<MapEntry<String, String>[]> unorderedFailureTestCases() {
return Stream.of(array(entry("banana", "fruit"), entry("potato", "vegetable")),
array(entry("strawberry", "fruit")),
array(entry("banana", "food")),
array());
}

private static Stream<MapEntry<String, String>[]> unorderedSuccessTestCases() {
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "fruit")));
}

private static Stream<MapEntry<String, String>[]> unorderedInsensitiveFailureTestCases() {
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "FRUIT")));
}

private static Stream<MapEntry<String, String>[]> unorderedInsensitiveSuccessTestCases() {
return Stream.<MapEntry<String, String>[]> of(array(entry("BANANA", "fruit")));
}

private static Stream<MapEntry<String, String>[]> orderedInsensitiveFailureTestCases() {
return Stream.of(array(entry("banana", "fruit"), entry("tomato", "vegetable")),
array(entry("potato", "vegetable"), entry("BANANA", "fruit")),
array(entry("banana", "vegetable"), entry("tomato", "fruit")),
array(entry("banana", "plane"), entry("poTATo", "train")));
}

private static Stream<MapEntry<String, String>[]> orderedInsensitiveSuccessTestCases() {
return Stream.<MapEntry<String, String>[]> of(array(entry("BANANA", "fruit"), entry("potato", "vegetable")));
}

private static Stream<Arguments> orderedSensitiveSuccessfulArguments() {
Stream<Map<String, String>> maps = Stream.of(LinkedHashMap::new, PERSISTENT_SORTED_MAP)
.map(supplier -> mapOf(supplier,
entry("banana", "fruit"),
entry("poTATo", "vegetable")));
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::orderedSuccessTestCases);
}

private static Stream<Arguments> orderedInsensitiveSuccessfulArguments() {
Stream<Map<String, String>> maps = Stream.of(CASE_INSENSITIVE_MAPS)
.map(supplier -> mapOf(supplier,
entry("banana", "fruit"),
entry("poTATo", "vegetable")));
return mapsAndEntriesToArguments(maps, () -> concat(orderedSuccessTestCases(), orderedInsensitiveSuccessTestCases()));
}

private static Stream<Arguments> orderedSensitiveFailureArguments() {
Stream<Map<String, String>> maps = Stream.of(LinkedHashMap::new, PERSISTENT_SORTED_MAP)
.map(supplier -> mapOf(supplier,
entry("banana", "fruit"),
entry("poTATo", "vegetable")));
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::orderedFailureTestCases);
}

private static Stream<Arguments> orderedInsensitiveFailureArguments() {
Stream<Map<String, String>> maps = Stream.of(CASE_INSENSITIVE_MAPS)
.map(supplier -> mapOf(supplier, entry("banana", "fruit"),
entry("poTATo", "vegetable")));
return mapsAndEntriesToArguments(maps, () -> concat(orderedFailureTestCases(), orderedInsensitiveFailureTestCases()));
}

private static Stream<Arguments> unorderedSensitiveSuccessfulArguments() {
Stream<Map<String, String>> maps = concat(Stream.of(HashMap::new, IdentityHashMap::new, PERSISTENT_MAP)
.map(supplier -> mapOf(supplier, entry("banana", "fruit"))),
Stream.of(singletonMap("banana", "fruit"),
new SingletonMap<>("banana", "fruit"),
unmodifiableMap(mapOf(entry("banana", "fruit"))),
ImmutableMap.of("banana", "fruit"),
Jdk11.Map.of("banana", "fruit")));
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::unorderedSuccessTestCases);
}

private static Stream<Arguments> unorderedInsensitiveSuccessfulArguments() {
Stream<Map<String, String>> maps = Stream.of(mapOf(CaseInsensitiveMap::new, entry("banana", "fruit")));
return mapsAndEntriesToArguments(maps, () -> concat(unorderedSuccessTestCases(), unorderedInsensitiveSuccessTestCases()));
}

private static Stream<Arguments> unorderedSensitiveFailureArguments() {
Stream<Map<String, String>> maps = concat(Stream.of(HashMap::new, IdentityHashMap::new, PERSISTENT_MAP)
.map(supplier -> mapOf(supplier, entry("banana", "fruit"))),
Stream.of(singletonMap("banana", "fruit"),
new SingletonMap<>("banana", "fruit"),
unmodifiableMap(mapOf(entry("banana", "fruit"))),
ImmutableMap.of("banana", "fruit"),
Jdk11.Map.of("banana", "fruit")));
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::unorderedInsensitiveSuccessTestCases);
}

private static Stream<Arguments> unorderedInsensitiveFailureArguments() {
Stream<Map<String, String>> maps = Stream.of(mapOf(CaseInsensitiveMap::new, entry("banana", "fruit")));
return mapsAndEntriesToArguments(maps, () -> concat(unorderedFailureTestCases(), unorderedInsensitiveFailureTestCases()));
}

private static Stream<Arguments> mapsAndEntriesToArguments(Stream<Map<String, String>> maps,
Supplier<Stream<MapEntry<String, String>[]>> entries) {
return maps.flatMap(m -> entries.get().map(entryArray -> arguments(m, entryArray)));
}

@SafeVarargs
Expand All @@ -159,10 +303,4 @@ private static Map<String, String> newLinkedHashMap(MapEntry<String, String>...
}
return result;
}

private static <K, V> Set<MapEntry<K, V>> newHashSet(MapEntry<K, V> entry) {
LinkedHashSet<MapEntry<K, V>> result = new LinkedHashSet<>();
result.add(entry);
return result;
}
}

0 comments on commit 498ee5b

Please sign in to comment.