This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.x by this push:
new 49cdf7457e ReadOnlyStringMap: Generalize `equals`/`hashCode` across
implementations (#3675)
49cdf7457e is described below
commit 49cdf7457eab03df3cc09d9419bb4f40c173dbc8
Author: Ryan Schmitt <[email protected]>
AuthorDate: Mon Jun 2 11:21:28 2025 -0700
ReadOnlyStringMap: Generalize `equals`/`hashCode` across implementations
(#3675)
This change rewrites the `equals` and `hashCode` implementations
throughout the `ReadOnlyStringMap` hierarchy to use a common
implementation that supports value-based equality between
implementations.
Fixes #3669.
---
.../org/apache/log4j/bridge/LogEventWrapper.java | 19 +++++++++
.../logging/log4j/spi/DefaultThreadContextMap.java | 16 +++++---
.../logging/log4j/util/SortedArrayStringMap.java | 33 ++++------------
.../apache/logging/log4j/util/package-info.java | 2 +-
.../log4j/core/impl/FactoryTestStringMap.java | 23 +++++++++++
.../FactoryTestStringMapWithoutIntConstructor.java | 23 +++++++++++
.../core/impl/JdkMapAdapterStringMapTest.java | 21 ++++++++++
.../log4j/core/impl/JdkMapAdapterStringMap.java | 14 +++++--
.../logging/log4j/perf/nogc/OpenHashStringMap.java | 46 ++++------------------
.../3669_generalize_ReadOnlyStringMap_equality.xml | 10 +++++
10 files changed, 132 insertions(+), 75 deletions(-)
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 15565aa040..3c5448b210 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 class LogEventWrapper implements LogEvent {
public <V> 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<String, String> thisMap = toMap();
+ final Map<String, String> 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 f8ea436284..62752428f5 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 class DefaultThreadContextMap implements
ThreadContextMap, ReadOnlyString
@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 ac6b3ed199..f74b10fbf2 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 class SortedArrayStringMap implements
IndexedStringMap {
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<String, String> thisMap = toMap();
+ final Map<String, String> 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 ea9e726d42..23fcfbd931 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 3c9b2dce9a..8a9ec1bfa0 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 class FactoryTestStringMap implements
IndexedStringMap {
public Map<String, String> 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<String, String> thisMap = toMap();
+ final Map<String, String> 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 6359224090..74947ba75f 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 class FactoryTestStringMapWithoutIntConstructor
implements StringMap {
@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<String, String> thisMap = toMap();
+ final Map<String, String> 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 7b06a23a32..f9c5ab3a40 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.HashMap;
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 @@ class JdkMapAdapterStringMapTest {
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),
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 09c4441394..d4833bb40c 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 class JdkMapAdapterStringMap implements StringMap {
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<String, String> thisMap = toMap();
+ final Map<String, String> 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 2e26a3b152..5e1f3f4fc5 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.Collections;
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 class OpenHashStringMap<K, V> implements
StringMap, ThreadContextMap {
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<String, String> thisMap = toMap();
+ final Map<String, String> otherMap = ((ReadOnlyStringMap) obj).toMap();
+ return thisMap.equals(otherMap);
}
@Override
@@ -699,25 +685,7 @@ public class OpenHashStringMap<K, V> implements StringMap,
ThreadContextMap {
*/
@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 0000000000..a7a6ee4a08
--- /dev/null
+++ b/src/changelog/.2.x.x/3669_generalize_ReadOnlyStringMap_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 ReadOnlyStringMap implementations now support equality comparisons
against each other.
+ </description>
+</entry>
