StringMap: Generalize equals/hashCode across implementations

This change rewrites the `equals` and `hashCode` implementations in
`SortedArrayStringMap` and `JdkMapAdapterStringMap` to support
value-based equality between the two implementations.

Fixes #3669.

Author: rschmitt

log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java

@@ -408,18 +408,19 @@ public boolean equals(final Object obj) {
         if (obj == this) {
             return true;
         }
-        if (!(obj instanceof SortedArrayStringMap)) {
+        if (!(obj instanceof StringMap)) {
             return false;
         }
-        final SortedArrayStringMap other = (SortedArrayStringMap) obj;
+        final StringMap other = (StringMap) obj;
         if (this.size() != other.size()) {
             return false;
         }
-        for (int i = 0; i < size(); i++) {
-            if (!Objects.equals(keys[i], other.keys[i])) {
+        for (int i = 0; i < size; i++) {
+            String key = keys[i];
+            if (!other.containsKey(key)) {
                 return false;
             }
-            if (!Objects.equals(values[i], other.values[i])) {
+            if (!Objects.equals(getValue(key), other.getValue(key))) {
                 return false;
             }
         }
@@ -428,19 +429,13 @@ public boolean equals(final Object obj) {
 
     @Override
     public int hashCode() {
-        int result = 37;
-        result = HASHVAL * result + size;
-        result = HASHVAL * result + hashCode(keys, size);
-        result = HASHVAL * result + hashCode(values, size);
-        return result;
-    }
-
-    private static int hashCode(final Object[] values, final int length) {
-        int result = 1;
-        for (int i = 0; i < length; i++) {
-            result = HASHVAL * result + (values[i] == null ? 0 : values[i].hashCode());
-        }
-        return result;
+        int[] result = new int[] {HASHVAL};
+        forEach(
+                (k, v, r) -> {
+                    r[0] += Objects.hashCode(k) ^ Objects.hashCode(v);
+                },
+                result);
+        return result[0];
     }
 
     @Override

log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java

@@ -35,6 +35,7 @@
 import java.util.Map;
 import java.util.stream.Stream;
 import org.apache.logging.log4j.util.BiConsumer;
+import org.apache.logging.log4j.util.SortedArrayStringMap;
 import org.apache.logging.log4j.util.TriConsumer;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -850,6 +851,26 @@ void testForEachTriConsumer() {
         assertEquals(state.count, original.size());
     }
 
+    @Test
+    void testEqualityWithOtherImplementations() {
+        final JdkMapAdapterStringMap left = new JdkMapAdapterStringMap();
+        final SortedArrayStringMap right = new SortedArrayStringMap();
+        assertEquals(left, right);
+        assertEquals(left.hashCode(), right.hashCode());
+
+        left.putValue("a", "avalue");
+        left.putValue("B", "Bvalue");
+        right.putValue("B", "Bvalue");
+        right.putValue("a", "avalue");
+        assertEquals(left, right);
+        assertEquals(left.hashCode(), right.hashCode());
+
+        left.remove("a");
+        right.remove("a");
+        assertEquals(left, right);
+        assertEquals(left.hashCode(), right.hashCode());
+    }
+
     static Stream<Arguments> testImmutability() {
         return Stream.of(
                 Arguments.of(new HashMap<>(), false),

log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java

@@ -219,15 +219,32 @@ public boolean equals(final Object object) {
         if (object == this) {
             return true;
         }
-        if (!(object instanceof JdkMapAdapterStringMap)) {
+        if (!(object instanceof StringMap)) {
             return false;
         }
-        final JdkMapAdapterStringMap other = (JdkMapAdapterStringMap) object;
-        return map.equals(other.map) && immutable == other.immutable;
+        final StringMap other = (StringMap) object;
+        if (size() != other.size()) {
+            return false;
+        }
+        for (String key : map.keySet()) {
+            if (!other.containsKey(key)) {
+                return false;
+            }
+            if (!Objects.equals(getValue(key), other.getValue(key))) {
+                return false;
+            }
+        }
+        return true;
     }
 
     @Override
     public int hashCode() {
-        return map.hashCode() + (immutable ? 31 : 0);
+        int[] result = new int[] {31};
+        forEach(
+                (k, v, r) -> {
+                    r[0] += Objects.hashCode(k) ^ Objects.hashCode(v);
+                },
+                result);
+        return result[0];
     }
 }

src/changelog/.2.x.x/3669_generalize_StringMap_equality.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+  <issue id="3669" link="https://github.com/apache/logging-log4j2/issues/3669"/>
+  <description format="asciidoc">
+    The two major StringMap implementations, SortedArrayStringMap and JdkMapAdapterStringMap, now support equality comparisons against each other.
+  </description>
+</entry>