diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/LogEventWrapper.java b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/LogEventWrapper.java index 15565aa040c..3c5448b210d 100644 --- a/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/LogEventWrapper.java +++ b/log4j-1.2-api/src/main/java/org/apache/log4j/bridge/LogEventWrapper.java @@ -213,5 +213,24 @@ public void forEach(final TriConsumer action, final public V getValue(final String key) { return (V) super.get(key); } + + @Override + public boolean equals(final Object obj) { + if (obj == this) { + return true; + } + if (obj instanceof ReadOnlyStringMap) { + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) obj).toMap(); + return thisMap.equals(otherMap); + } + return super.equals(obj); + } + + @Override + public int hashCode() { + return toMap().hashCode(); + } } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java index f8ea436284e..62752428f50 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java @@ -181,21 +181,25 @@ public String toString() { @Override public int hashCode() { - final int prime = 31; - int result = 1; - final Object[] state = localState.get(); - result = prime * result + ((state == null) ? 0 : getMap(state).hashCode()); - return result; + return toMap().hashCode(); } @Override - public boolean equals(final Object obj) { + public boolean equals(Object obj) { if (this == obj) { return true; } if (obj == null) { return false; } + if (obj instanceof ReadOnlyStringMap) { + if (size() != ((ReadOnlyStringMap) obj).size()) { + return false; + } + + // Convert to maps and compare + obj = ((ReadOnlyStringMap) obj).toMap(); + } if (!(obj instanceof ThreadContextMap)) { return false; } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java index ac6b3ed199b..f74b10fbf2f 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/SortedArrayStringMap.java @@ -408,39 +408,22 @@ public boolean equals(final Object obj) { if (obj == this) { return true; } - if (!(obj instanceof SortedArrayStringMap)) { + if (!(obj instanceof ReadOnlyStringMap)) { return false; } - final SortedArrayStringMap other = (SortedArrayStringMap) obj; - if (this.size() != other.size()) { + if (size() != ((ReadOnlyStringMap) obj).size()) { return false; } - for (int i = 0; i < size(); i++) { - if (!Objects.equals(keys[i], other.keys[i])) { - return false; - } - if (!Objects.equals(values[i], other.values[i])) { - return false; - } - } - return true; + + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) obj).toMap(); + return thisMap.equals(otherMap); } @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; + return toMap().hashCode(); } @Override diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/package-info.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/package-info.java index ea9e726d424..23fcfbd931d 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/package-info.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/package-info.java @@ -20,7 +20,7 @@ * There are no guarantees for binary or logical compatibility in this package. */ @Export -@Version("2.24.1") +@Version("2.25.0") package org.apache.logging.log4j.util; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMap.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMap.java index 3c9b2dce9a9..8a9ec1bfa01 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMap.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMap.java @@ -114,4 +114,27 @@ public int size() { public Map toMap() { return null; } + + @Override + public boolean equals(final Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof ReadOnlyStringMap)) { + return false; + } + if (size() != ((ReadOnlyStringMap) obj).size()) { + return false; + } + + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) obj).toMap(); + return thisMap.equals(otherMap); + } + + @Override + public int hashCode() { + return toMap().hashCode(); + } } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMapWithoutIntConstructor.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMapWithoutIntConstructor.java index 63592240904..74947ba75fb 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMapWithoutIntConstructor.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMapWithoutIntConstructor.java @@ -81,4 +81,27 @@ public void putValue(final String key, final Object value) {} @Override public void remove(final String key) {} + + @Override + public boolean equals(final Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof ReadOnlyStringMap)) { + return false; + } + if (size() != ((ReadOnlyStringMap) obj).size()) { + return false; + } + + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) obj).toMap(); + return thisMap.equals(otherMap); + } + + @Override + public int hashCode() { + return toMap().hashCode(); + } } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java index 7b06a23a32e..f9c5ab3a409 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java +++ b/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 testImmutability() { return Stream.of( Arguments.of(new HashMap<>(), false), diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java index 09c44413940..d4833bb40c7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java @@ -219,15 +219,21 @@ public boolean equals(final Object object) { if (object == this) { return true; } - if (!(object instanceof JdkMapAdapterStringMap)) { + if (!(object instanceof ReadOnlyStringMap)) { return false; } - final JdkMapAdapterStringMap other = (JdkMapAdapterStringMap) object; - return map.equals(other.map) && immutable == other.immutable; + if (size() != ((ReadOnlyStringMap) object).size()) { + return false; + } + + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) object).toMap(); + return thisMap.equals(otherMap); } @Override public int hashCode() { - return map.hashCode() + (immutable ? 31 : 0); + return toMap().hashCode(); } } diff --git a/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/nogc/OpenHashStringMap.java b/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/nogc/OpenHashStringMap.java index 2e26a3b1523..5e1f3f4fc59 100644 --- a/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/nogc/OpenHashStringMap.java +++ b/log4j-perf-test/src/main/java/org/apache/logging/log4j/perf/nogc/OpenHashStringMap.java @@ -24,7 +24,6 @@ import java.util.ConcurrentModificationException; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import org.apache.logging.log4j.spi.ThreadContextMap; import org.apache.logging.log4j.util.BiConsumer; import org.apache.logging.log4j.util.ReadOnlyStringMap; @@ -301,27 +300,14 @@ public boolean equals(final Object obj) { if (!(obj instanceof ReadOnlyStringMap)) { return false; } - final ReadOnlyStringMap other = (ReadOnlyStringMap) obj; - if (other.size() != size()) { + if (size() != ((ReadOnlyStringMap) obj).size()) { return false; } - int pos = arraySize; - if (containsNullKey) { - if (!Objects.equals(getObjectValue(null), other.getValue(null))) { - return false; - } - } - --pos; - final K myKeys[] = this.keys; - for (; pos >= 0; pos--) { - K k; - if ((k = myKeys[pos]) != null) { - if (!Objects.equals(values[pos], other.getValue((String) k))) { - return false; - } - } - } - return true; + + // Convert to maps and compare + final Map thisMap = toMap(); + final Map otherMap = ((ReadOnlyStringMap) obj).toMap(); + return thisMap.equals(otherMap); } @Override @@ -699,25 +685,7 @@ protected void rehash(final int newN) { */ @Override public int hashCode() { - int result = 0; - for (int j = realSize(), i = 0, t = 0; j-- != 0; ) { - while (keys[i] == null) { - i++; - } - if (this != keys[i]) { - t = keys[i].hashCode(); - } - if (this != values[i]) { - t ^= (values[i] == null ? 0 : values[i].hashCode()); - } - result += t; - i++; - } - // Zero / null keys have hash zero. - if (containsNullKey) { - result += (values[arraySize] == null ? 0 : values[arraySize].hashCode()); - } - return result; + return toMap().hashCode(); } @SuppressWarnings("unchecked") diff --git a/src/changelog/.2.x.x/3669_generalize_ReadOnlyStringMap_equality.xml b/src/changelog/.2.x.x/3669_generalize_ReadOnlyStringMap_equality.xml new file mode 100644 index 00000000000..a7a6ee4a080 --- /dev/null +++ b/src/changelog/.2.x.x/3669_generalize_ReadOnlyStringMap_equality.xml @@ -0,0 +1,10 @@ + + + + + The ReadOnlyStringMap implementations now support equality comparisons against each other. + +