Repository: assertj/assertj

Problem statement:
`ignoringFields` on recursive comparison no longer works with maps with null values
**Describe the bug**

As part of solving https://github.com/assertj/assertj/issues/2988, https://github.com/assertj/assertj/commit/835c1dc3aee2348eb234f3209488fdd078702c2f changed how maps were checked for equality with a new `filterIgnoredFields` function. This function uses Java's `Collectors.toMap` to convert a stream back to a map, which internally uses `uniqKeysMapAccumulator`, which throws a `NullPointerException` if any map entry values are null.

This is not ideal for a testing framework, which should be designed to handle all kinds of possible map scenarios, including ones with null entry values. Please replace this `toMap` call ([deeplink](https://github.com/assertj/assertj/commit/835c1dc3aee2348eb234f3209488fdd078702c2f#diff-f64c53f85fc4dab2341ef1f3d02beecf1347b3c9f5df1e7467782c04d6afeee4R693)) with a different function to convert to a map which allows for null values, for example this one suggested in Stack Overflow: https://stackoverflow.com/questions/24630963/nullpointerexception-in-collectors-tomap-with-null-entry-values/32648397#32648397. Thanks!

* assertj core version: [3.27.0](https://github.com/assertj/assertj/releases/tag/assertj-build-3.27.0)
* java version: 21

**Test case reproducing the bug**

```java
  @Test
  public void assertj_ignoringFieldsWithNullValue_shouldNotThrow() {
    Map<String, String> actualMap = new HashMap<>();
    actualMap.put("exampleKey", "exampleValue");
    actualMap.put("nullKey", null);

    Map<String, String> expectedMap = new HashMap<>();
    expectedMap.put("exampleKey", "exampleValue");
    expectedMap.put("nullKey", null);

    assertThat(actualMap)
      .usingRecursiveComparison()
      .ignoringFields("exampleKey")
      .isEqualTo(expectedMap);
  }
```

---

Relates to:
* [JDK-8148463](https://bugs.openjdk.org/browse/JDK-8148463)


Hints:
Thanks for the reproducer, @grantaveryatgfs! We'll look into it.

Patch:
diff --git a/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonDifferenceCalculator.java b/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonDifferenceCalculator.java
--- a/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonDifferenceCalculator.java
+++ b/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonDifferenceCalculator.java
@@ -17,7 +17,6 @@
 import static java.util.stream.Collectors.groupingBy;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
-import static java.util.stream.Collectors.toMap;
 import static java.util.stream.StreamSupport.stream;
 import static org.assertj.core.api.recursive.comparison.ComparisonDifference.rootComparisonDifference;
 import static org.assertj.core.api.recursive.comparison.DualValue.DEFAULT_ORDERED_COLLECTION_TYPES;
@@ -35,6 +34,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -46,9 +46,11 @@
 import java.util.concurrent.atomic.AtomicLongArray;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.atomic.AtomicReferenceArray;
+import java.util.function.Function;
 import java.util.regex.Pattern;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
-
 import org.assertj.core.internal.DeepDifference;
 
 /**
@@ -575,7 +577,7 @@ private static void compareUnorderedIterables(DualValue dualValue, ComparisonSta
       // It may be that expectedElement matches an actual element in a different hash bucket, to account for this, we check the
       // other actual elements for matches. This may result in O(n^2) complexity in the worst case.
       if (!expectedElementMatched) {
-        for (Map.Entry<Integer, ? extends List<?>> actualElementsEntry : actualElementsGroupedByHashCode.entrySet()) {
+        for (Entry<Integer, ? extends List<?>> actualElementsEntry : actualElementsGroupedByHashCode.entrySet()) {
           // avoid checking the same bucket twice
           if (actualElementsEntry.getKey().equals(expectedHash)) continue;
           Iterator<?> actualElementsIterator = actualElementsEntry.getValue().iterator();
@@ -627,6 +629,8 @@ private static <K, V> void compareSortedMap(DualValue dualValue, ComparisonState
 
     Map<?, ?> actualMap = filterIgnoredFields((Map<?, ?>) dualValue.actual, dualValue.fieldLocation,
                                               comparisonState.recursiveComparisonConfiguration);
+
+    @SuppressWarnings("unchecked")
     Map<K, V> expectedMap = (Map<K, V>) filterIgnoredFields((Map<?, ?>) dualValue.expected,
                                                             dualValue.fieldLocation,
                                                             comparisonState.recursiveComparisonConfiguration);
@@ -636,9 +640,9 @@ private static <K, V> void compareSortedMap(DualValue dualValue, ComparisonState
       // no need to inspect entries, maps are not equal as they don't have the same size
       return;
     }
-    Iterator<Map.Entry<K, V>> expectedMapEntries = expectedMap.entrySet().iterator();
-    for (Map.Entry<?, ?> actualEntry : actualMap.entrySet()) {
-      Map.Entry<?, ?> expectedEntry = expectedMapEntries.next();
+    Iterator<Entry<K, V>> expectedMapEntries = expectedMap.entrySet().iterator();
+    for (Entry<?, ?> actualEntry : actualMap.entrySet()) {
+      Entry<?, ?> expectedEntry = expectedMapEntries.next();
       // check keys are matched before comparing values as keys represents a field
       if (!java.util.Objects.equals(actualEntry.getKey(), expectedEntry.getKey())) {
         // report a missing key/field.
@@ -683,17 +687,29 @@ private static void compareUnorderedMap(DualValue dualValue, ComparisonState com
   }
 
   private static Map<?, ?> filterIgnoredFields(Map<?, ?> map, FieldLocation fieldLocation,
-                                               RecursiveComparisonConfiguration recursiveComparisonConfiguration) {
-    Set<String> ignoredFields = recursiveComparisonConfiguration.getIgnoredFields();
-    List<Pattern> ignoredFieldsRegexes = recursiveComparisonConfiguration.getIgnoredFieldsRegexes();
+                                               RecursiveComparisonConfiguration configuration) {
+    Set<String> ignoredFields = configuration.getIgnoredFields();
+    List<Pattern> ignoredFieldsRegexes = configuration.getIgnoredFieldsRegexes();
     if (ignoredFields.isEmpty() && ignoredFieldsRegexes.isEmpty()) {
       return map;
     }
     return map.entrySet().stream()
-              .filter(e -> !recursiveComparisonConfiguration.matchesAnIgnoredField(fieldLocation.field(e.getKey().toString())))
-              .filter(e -> !recursiveComparisonConfiguration.matchesAnIgnoredFieldRegex(fieldLocation.field(e.getKey()
-                                                                                                             .toString())))
-              .collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
+              .filter(e -> !configuration.matchesAnIgnoredField(fieldLocation.field(e.getKey().toString())))
+              .filter(e -> !configuration.matchesAnIgnoredFieldRegex(fieldLocation.field(e.getKey().toString())))
+              .collect(toMap(Entry::getKey, Entry::getValue));
+  }
+
+  // workaround for https://bugs.openjdk.org/browse/JDK-8148463
+  private static <T, K, U> Collector<T, ?, Map<K, U>> toMap(Function<? super T, ? extends K> keyMapper,
+                                                            Function<? super T, ? extends U> valueMapper) {
+    @SuppressWarnings("unchecked")
+    U none = (U) new Object();
+    Collector<T, ?, Map<K, U>> downstream = Collectors.toMap(keyMapper, valueMapper.andThen(v -> v == null ? none : v));
+    Function<Map<K, U>, Map<K, U>> finisher = map -> {
+      map.replaceAll((k, v) -> v == none ? null : v);
+      return map;
+    };
+    return Collectors.collectingAndThen(downstream, finisher);
   }
 
   private static FieldLocation keyFieldLocation(FieldLocation parentFieldLocation, Object key) {


Test patch:
diff --git a/assertj-tests/assertj-integration-tests/assertj-core-tests/src/test/java/org/assertj/tests/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_ignoringFields_Test.java b/assertj-tests/assertj-integration-tests/assertj-core-tests/src/test/java/org/assertj/tests/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_ignoringFields_Test.java
--- a/assertj-tests/assertj-integration-tests/assertj-core-tests/src/test/java/org/assertj/tests/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_ignoringFields_Test.java
+++ b/assertj-tests/assertj-integration-tests/assertj-core-tests/src/test/java/org/assertj/tests/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_ignoringFields_Test.java
@@ -804,6 +804,22 @@ public void should_honor_ignored_fields_in_nested_map() {
               .isEqualTo(mapB);
   }
 
+  @Test
+  void should_honor_ignored_fields_in_map_with_null_value() {
+    // GIVEN
+    Map<String, String> actual = new HashMap<>();
+    actual.put("foo", "value1");
+    actual.put("bar", null);
+
+    Map<String, String> expected = new HashMap<>();
+    expected.put("foo", "value2");
+    expected.put("bar", null);
+    // WHEN/THEN
+    then(actual).usingRecursiveComparison()
+                .ignoringFields("foo")
+                .isEqualTo(expected);
+  }
+
   static class Data {
     private final InnerData innerData;
     private final List<InnerData> innerDataList;
@@ -839,4 +855,5 @@ public String getField1() {
       }
     }
   }
+
 }