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
commit 7818b0fabf83a7a21ac1631d05f219793ccb1150 Author: John Engebretson <[email protected]> AuthorDate: Fri Mar 8 16:30:43 2024 +0000 Updated per comments on PR --- .../map}/StringArrayThreadContextMapTest.java | 110 +++++--- .../map}/UnmodifiableArrayBackedMapTest.java | 297 ++++++++++++--------- .../org/apache/logging/log4j/ThreadContext.java | 2 +- .../map}/StringArrayThreadContextMap.java | 70 +---- .../map}/UnmodifiableArrayBackedMap.java | 92 ++++++- .../logging/log4j/spi/ThreadContextMapFactory.java | 1 - 6 files changed, 332 insertions(+), 240 deletions(-) diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/StringArrayThreadContextMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java similarity index 74% rename from log4j-api-test/src/test/java/org/apache/logging/log4j/spi/StringArrayThreadContextMapTest.java rename to log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java index 601927bf6c..2f845d8bf7 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/StringArrayThreadContextMapTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java @@ -14,21 +14,21 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.logging.log4j.spi; +package org.apache.logging.log4j.internal.map; -import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; -import org.apache.logging.log4j.test.junit.InitializesThreadContext; -import org.apache.logging.log4j.test.junit.SetTestProperty; +import java.util.Set; import org.apache.logging.log4j.test.junit.UsingThreadContextMap; +import org.apache.logging.log4j.util.TriConsumer; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ClearSystemProperty; /** * Tests the {@code StringArrayThreadContextMap} class. @@ -56,27 +56,20 @@ public class StringArrayThreadContextMapTest { @Test public void testGet() { final StringArrayThreadContextMap map1 = createMap(); - assertEquals(null, map1.get("test")); + assertNull(map1.get("test")); map1.put("test", "test"); assertEquals("test", map1.get("test")); - assertEquals(null, map1.get("not_present")); - } - - @Test - public void testDoesNothingIfConstructedWithUseMapIsFalse() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(false); - assertTrue(map.isEmpty()); - assertFalse(map.containsKey("key")); - map.put("key", "value"); + assertNull(map1.get("not_present")); + assertEquals("test", map1.getValue("test")); + assertNull(map1.getValue("not_present")); - assertTrue(map.isEmpty()); - assertFalse(map.containsKey("key")); - assertNull(map.get("key")); + map1.clear(); + assertNull(map1.get("not_present")); } @Test public void testPut() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertTrue(map.isEmpty()); assertFalse(map.containsKey("key")); map.put("key", "value"); @@ -88,7 +81,7 @@ public class StringArrayThreadContextMapTest { @Test public void testPutAll() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertTrue(map.isEmpty()); assertFalse(map.containsKey("key")); final int mapSize = 10; @@ -106,7 +99,7 @@ public class StringArrayThreadContextMapTest { /** * Test method for - * {@link org.apache.logging.log4j.spi.StringArrayThreadContextMap#remove(java.lang.String)} + * {@link org.apache.logging.log4j.internal.map.StringArrayThreadContextMap#remove(java.lang.String)} * . */ @Test @@ -118,6 +111,9 @@ public class StringArrayThreadContextMapTest { map.remove("key"); assertFalse(map.containsKey("key")); assertEquals("value2", map.get("key2")); + + map.clear(); + map.remove("test"); } @Test @@ -132,6 +128,9 @@ public class StringArrayThreadContextMapTest { map.removeAll(newValues.keySet()); map.put("3", "value3"); + + map.clear(); + map.removeAll(newValues.keySet()); } @Test @@ -148,7 +147,7 @@ public class StringArrayThreadContextMapTest { * @return */ private StringArrayThreadContextMap createMap() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertTrue(map.isEmpty()); map.put("key", "value"); map.put("key2", "value2"); @@ -159,7 +158,7 @@ public class StringArrayThreadContextMapTest { @Test public void testGetCopyReturnsMutableMap() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertTrue(map.isEmpty()); final Map<String, String> copy = map.getCopy(); assertTrue(copy.isEmpty()); @@ -173,7 +172,7 @@ public class StringArrayThreadContextMapTest { @Test public void testGetCopyReturnsMutableCopy() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); map.put("key1", "value1"); assertFalse(map.isEmpty()); final Map<String, String> copy = map.getCopy(); @@ -194,14 +193,14 @@ public class StringArrayThreadContextMapTest { @Test public void testGetImmutableMapReturnsNullIfEmpty() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertTrue(map.isEmpty()); assertNull(map.getImmutableMapOrNull()); } @Test public void testGetImmutableMapReturnsImmutableMapIfNonEmpty() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); map.put("key1", "value1"); assertFalse(map.isEmpty()); @@ -214,7 +213,7 @@ public class StringArrayThreadContextMapTest { @Test public void testGetImmutableMapCopyNotAffectdByContextMapChanges() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); map.put("key1", "value1"); assertFalse(map.isEmpty()); @@ -230,7 +229,7 @@ public class StringArrayThreadContextMapTest { @Test public void testToStringShowsMapContext() { - final StringArrayThreadContextMap map = new StringArrayThreadContextMap(true); + final StringArrayThreadContextMap map = new StringArrayThreadContextMap(); assertEquals("{}", map.toString()); map.put("key1", "value1"); @@ -242,24 +241,47 @@ public class StringArrayThreadContextMapTest { } @Test - @ClearSystemProperty(key = StringArrayThreadContextMap.INHERITABLE_MAP) - @InitializesThreadContext - public void testThreadLocalNotInheritableByDefault() { - ThreadContextMapFactory.init(); - final ThreadLocal<Object[]> threadLocal = StringArrayThreadContextMap.createThreadLocalMap(true); - assertFalse(threadLocal instanceof InheritableThreadLocal<?>); + public void testEmptyMap() { + assertNull(UnmodifiableArrayBackedMap.EMPTY_MAP.get("test")); } @Test - @SetTestProperty(key = StringArrayThreadContextMap.INHERITABLE_MAP, value = "true") - @InitializesThreadContext - public void testThreadLocalInheritableIfConfigured() { - ThreadContextMapFactory.init(); - try { - final ThreadLocal<Object[]> threadLocal = StringArrayThreadContextMap.createThreadLocalMap(true); - assertTrue(threadLocal instanceof InheritableThreadLocal<?>); - } finally { - System.clearProperty(StringArrayThreadContextMap.INHERITABLE_MAP); - } + public void testForEachBiConsumer_Log4jUtil() { + StringArrayThreadContextMap map = createMap(); + Set<String> keys = new HashSet<>(); + org.apache.logging.log4j.util.BiConsumer<String, String> log4j_util_action = + new org.apache.logging.log4j.util.BiConsumer<String, String>() { + @Override + public void accept(String key, String value) { + keys.add(key); + } + }; + map.forEach(log4j_util_action); + assertEquals(map.toMap().keySet(), keys); + + map.clear(); + keys.clear(); + map.forEach(log4j_util_action); + assertTrue(keys.isEmpty()); + } + + @Test + public void testForEachTriConsumer() { + StringArrayThreadContextMap map = createMap(); + HashMap<String, String> iterationResultMap = new HashMap<>(); + TriConsumer<String, String, Map<String, String>> triConsumer = + new TriConsumer<String, String, Map<String, String>>() { + @Override + public void accept(String k, String v, Map<String, String> s) { + s.put(k, v); + } + }; + map.forEach(triConsumer, iterationResultMap); + assertEquals(map.toMap(), iterationResultMap); + + map.clear(); + iterationResultMap.clear(); + map.forEach(triConsumer, iterationResultMap); + assertTrue(iterationResultMap.isEmpty()); } } diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java similarity index 83% rename from log4j-api-test/src/test/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMapTest.java rename to log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java index 16d2c696b7..efcee80a15 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMapTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java @@ -14,13 +14,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.logging.log4j.spi; +package org.apache.logging.log4j.internal.map; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.util.ArrayList; import java.util.Collections; @@ -30,6 +31,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; +import org.apache.logging.log4j.util.TriConsumer; import org.junit.jupiter.api.Test; public class UnmodifiableArrayBackedMapTest { @@ -49,20 +51,52 @@ public class UnmodifiableArrayBackedMapTest { } @Test - public void testReads() { - assertEquals(UnmodifiableArrayBackedMap.EMPTY_MAP.get("test"), null); + public void testCopyAndPut() { + UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP; + testMap = testMap.copyAndPut("1", "value1"); + assertTrue(testMap.containsKey("1")); + assertEquals(testMap.get("1"), "value1"); + + testMap = testMap.copyAndPut("1", "another value"); + assertTrue(testMap.containsKey("1")); + assertEquals(testMap.get("1"), "another value"); + + HashMap<String, String> newValues = getTestParameters(); + testMap = testMap.copyAndPutAll(newValues); + assertEquals(testMap.get("1"), "value1"); + assertEquals(testMap.get("4"), "value4"); + } + + @Test + public void testCopyAndRemove() { + // general removing from well-populated set HashMap<String, String> params = getTestParameters(); UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); - for (Map.Entry<String, String> entry : params.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - assertTrue(testMap.containsKey(key)); - assertTrue(testMap.containsValue(value)); - assertEquals(testMap.get(key), params.get(key)); - } - assertFalse(testMap.containsKey("not_present")); - assertFalse(testMap.containsValue("not_present")); - assertEquals(null, testMap.get("not_present")); + testMap = testMap.copyAndRemove("2"); + testMap = testMap.copyAndRemove("not_present"); + assertEquals(4, testMap.size()); + assertFalse(testMap.containsKey("2")); + assertTrue(testMap.containsKey("1")); + assertFalse(testMap.containsValue("value2")); + + // test removing from empty set + testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test", "test"); + testMap = testMap.copyAndRemove("test"); + assertTrue(testMap.isEmpty()); + + // test removing first of two elements + testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test1", "test1"); + testMap = testMap.copyAndPut("test2", "test2"); + testMap = testMap.copyAndRemove("test1"); + assertFalse(testMap.containsKey("test1")); + assertTrue(testMap.containsKey("test2")); + + // test removing second of two elements + testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test1", "test1"); + testMap = testMap.copyAndPut("test2", "test2"); + testMap = testMap.copyAndRemove("test2"); + assertTrue(testMap.containsKey("test1")); + assertFalse(testMap.containsKey("test2")); } @Test @@ -111,29 +145,8 @@ public class UnmodifiableArrayBackedMapTest { } @Test - public void testCopyAndPut() { - UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP; - testMap = testMap.copyAndPut("1", "value1"); - assertTrue(testMap.containsKey("1")); - assertEquals(testMap.get("1"), "value1"); - - testMap = testMap.copyAndPut("1", "another value"); - assertTrue(testMap.containsKey("1")); - assertEquals(testMap.get("1"), "another value"); - - HashMap<String, String> newValues = getTestParameters(); - testMap = testMap.copyAndPutAll(newValues); - assertEquals(testMap.get("1"), "value1"); - assertEquals(testMap.get("4"), "value4"); - } - - @Test - public void testInstanceCopy() { - HashMap<String, String> params = getTestParameters(); - UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); - - UnmodifiableArrayBackedMap testMap2 = new UnmodifiableArrayBackedMap(testMap); - assertEquals(testMap, testMap2); + public void testEmptyMap() { + assertNull(UnmodifiableArrayBackedMap.EMPTY_MAP.get("test")); } @Test @@ -176,76 +189,6 @@ public class UnmodifiableArrayBackedMapTest { } } - @Test - public void testNullValue() { - UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP; - testMap = testMap.copyAndPut("key", null); - assertTrue(testMap.containsKey("key")); - assertTrue(testMap.containsValue(null)); - assertTrue(testMap.size() == 1); - assertEquals(testMap.get("key"), null); - } - - @Test - public void testMutatorsBlocked() { - UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters()); - try { - testMap.put("a", "a"); - fail("put() wasn't blocked"); - } catch (UnsupportedOperationException e) { - } - - try { - testMap.putAll(new HashMap<>()); - fail("putAll() wasn't blocked"); - } catch (UnsupportedOperationException e) { - } - - try { - testMap.remove("1"); - fail("remove() wasn't blocked"); - } catch (UnsupportedOperationException e) { - } - - try { - testMap.clear(); - fail("clear() wasn't blocked"); - } catch (UnsupportedOperationException e) { - } - } - - @Test - public void testCopyAndRemove() { - // general removing from well-populated set - HashMap<String, String> params = getTestParameters(); - UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); - testMap = testMap.copyAndRemove("2"); - testMap = testMap.copyAndRemove("not_present"); - assertEquals(4, testMap.size()); - assertFalse(testMap.containsKey("2")); - assertTrue(testMap.containsKey("1")); - assertFalse(testMap.containsValue("value2")); - - // test removing from empty set - testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test", "test"); - testMap = testMap.copyAndRemove("test"); - assertTrue(testMap.isEmpty()); - - // test removing first of two elements - testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test1", "test1"); - testMap = testMap.copyAndPut("test2", "test2"); - testMap = testMap.copyAndRemove("test1"); - assertFalse(testMap.containsKey("test1")); - assertTrue(testMap.containsKey("test2")); - - // test removing second of two elements - testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test1", "test1"); - testMap = testMap.copyAndPut("test2", "test2"); - testMap = testMap.copyAndRemove("test2"); - assertTrue(testMap.containsKey("test1")); - assertFalse(testMap.containsKey("test2")); - } - /** * Tests various situations with .equals(). Test tries comparisons in both * directions, to make sure that HashMap.equals(UnmodifiableArrayBackedMap) work @@ -261,11 +204,14 @@ public class UnmodifiableArrayBackedMapTest { } @Test - public void testEqualsWhenOneValueDiffers() { + public void testEqualsHashCodeWithOneEmptyMap() { HashMap<String, String> params = getTestParameters(); UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); - assertNotEquals(params, testMap.copyAndPut("1", "different value")); - assertNotEquals(testMap.copyAndPut("1", "different value"), params); + // verify empty maps are not equal to non-empty maps + assertNotEquals(params, UnmodifiableArrayBackedMap.EMPTY_MAP); + assertNotEquals(new HashMap<>(), testMap); + assertNotEquals(UnmodifiableArrayBackedMap.EMPTY_MAP, params); + assertNotEquals(testMap, new HashMap<>()); } @Test @@ -283,14 +229,56 @@ public class UnmodifiableArrayBackedMapTest { } @Test - public void testEqualsHashCodeWithOneEmptyMap() { + public void testEqualsWhenOneValueDiffers() { HashMap<String, String> params = getTestParameters(); UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); - // verify empty maps are not equal to non-empty maps - assertNotEquals(params, UnmodifiableArrayBackedMap.EMPTY_MAP); - assertNotEquals(new HashMap<>(), testMap); - assertNotEquals(UnmodifiableArrayBackedMap.EMPTY_MAP, params); - assertNotEquals(testMap, new HashMap<>()); + assertNotEquals(params, testMap.copyAndPut("1", "different value")); + assertNotEquals(testMap.copyAndPut("1", "different value"), params); + } + + @Test + public void testForEachBiConsumer_JavaUtil() { + UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters()); + Set<String> keys = new HashSet<>(); + java.util.function.BiConsumer<String, String> java_util_action = + new java.util.function.BiConsumer<String, String>() { + @Override + public void accept(String key, String value) { + keys.add(key); + } + }; + map.forEach(java_util_action); + assertEquals(map.keySet(), keys); + } + + @Test + public void testForEachBiConsumer_Log4jUtil() { + UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters()); + Set<String> keys = new HashSet<>(); + org.apache.logging.log4j.util.BiConsumer<String, String> log4j_util_action = + new org.apache.logging.log4j.util.BiConsumer<String, String>() { + @Override + public void accept(String key, String value) { + keys.add(key); + } + }; + map.forEach(log4j_util_action); + assertEquals(map.keySet(), keys); + } + + @Test + public void testForEachTriConsumer() { + UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters()); + HashMap<String, String> iterationResultMap = new HashMap<>(); + TriConsumer<String, String, Map<String, String>> triConsumer = + new TriConsumer<String, String, Map<String, String>>() { + @Override + public void accept(String k, String v, Map<String, String> s) { + s.put(k, v); + } + }; + map.forEach(triConsumer, iterationResultMap); + assertEquals(map, iterationResultMap); } @Test @@ -306,6 +294,70 @@ public class UnmodifiableArrayBackedMapTest { assertEquals(originalMap, params); } + @Test + public void testInstanceCopy() { + HashMap<String, String> params = getTestParameters(); + UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); + + UnmodifiableArrayBackedMap testMap2 = new UnmodifiableArrayBackedMap(testMap); + assertEquals(testMap, testMap2); + } + + @Test + public void testMutatorsBlocked() { + UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters()); + try { + testMap.put("a", "a"); + fail("put() wasn't blocked"); + } catch (UnsupportedOperationException e) { + } + + try { + testMap.putAll(new HashMap<>()); + fail("putAll() wasn't blocked"); + } catch (UnsupportedOperationException e) { + } + + try { + testMap.remove("1"); + fail("remove() wasn't blocked"); + } catch (UnsupportedOperationException e) { + } + + try { + testMap.clear(); + fail("clear() wasn't blocked"); + } catch (UnsupportedOperationException e) { + } + } + + @Test + public void testNullValue() { + UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP; + testMap = testMap.copyAndPut("key", null); + assertTrue(testMap.containsKey("key")); + assertTrue(testMap.containsValue(null)); + assertTrue(testMap.size() == 1); + assertEquals(testMap.get("key"), null); + } + + @Test + public void testReads() { + assertEquals(UnmodifiableArrayBackedMap.EMPTY_MAP.get("test"), null); + HashMap<String, String> params = getTestParameters(); + UnmodifiableArrayBackedMap testMap = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(params); + for (Map.Entry<String, String> entry : params.entrySet()) { + String key = entry.getKey(); + String value = entry.getValue(); + assertTrue(testMap.containsKey(key)); + assertTrue(testMap.containsValue(value)); + assertEquals(testMap.get(key), params.get(key)); + } + assertFalse(testMap.containsKey("not_present")); + assertFalse(testMap.containsValue("not_present")); + assertEquals(null, testMap.get("not_present")); + } + @Test public void testState() { UnmodifiableArrayBackedMap originalMap; @@ -325,4 +377,11 @@ public class UnmodifiableArrayBackedMapTest { newMap = UnmodifiableArrayBackedMap.getInstance(originalMap.getBackingArray()); assertEquals(originalMap, newMap); } + + @Test + public void testToMap() { + UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPut("test", "test"); + // verify same instance, not just equals() + assertTrue(map == map.toMap()); + } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/ThreadContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/ThreadContext.java index 52fb10571c..02806b59d5 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/ThreadContext.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/ThreadContext.java @@ -24,13 +24,13 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import org.apache.logging.log4j.internal.map.StringArrayThreadContextMap; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.spi.CleanableThreadContextMap; import org.apache.logging.log4j.spi.DefaultThreadContextMap; import org.apache.logging.log4j.spi.DefaultThreadContextStack; import org.apache.logging.log4j.spi.NoOpThreadContextMap; import org.apache.logging.log4j.spi.ReadOnlyThreadContextMap; -import org.apache.logging.log4j.spi.StringArrayThreadContextMap; import org.apache.logging.log4j.spi.ThreadContextMap; import org.apache.logging.log4j.spi.ThreadContextMap2; import org.apache.logging.log4j.spi.ThreadContextMapFactory; diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/StringArrayThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java similarity index 72% rename from log4j-api/src/main/java/org/apache/logging/log4j/spi/StringArrayThreadContextMap.java rename to log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java index a606020a70..608ca77ffb 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/StringArrayThreadContextMap.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java @@ -14,13 +14,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.logging.log4j.spi; +package org.apache.logging.log4j.internal.map; 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.PropertiesUtil; import org.apache.logging.log4j.util.ReadOnlyStringMap; import org.apache.logging.log4j.util.TriConsumer; @@ -43,48 +43,14 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt */ public static final String INHERITABLE_MAP = "isThreadContextMapInheritable"; - private final boolean useMap; - private final ThreadLocal<Object[]> threadLocalMapState; - - private static boolean inheritableMap; - - static { - init(); - } - - // LOG4J2-479: by default, use a plain ThreadLocal, only use InheritableThreadLocal if configured. - // (This method is package protected for JUnit tests.) - static ThreadLocal<Object[]> createThreadLocalMap(final boolean isMapEnabled) { - if (inheritableMap) { - return new InheritableThreadLocal<Object[]>() { - @Override - protected Object[] childValue(final Object[] parentValue) { - return parentValue; - } - }; - } - // if not inheritable, return plain ThreadLocal with null as initial value - return new ThreadLocal<>(); - } - - static void init() { - inheritableMap = PropertiesUtil.getProperties().getBooleanProperty(INHERITABLE_MAP); - } + private ThreadLocal<Object[]> threadLocalMapState; public StringArrayThreadContextMap() { - this(true); - } - - public StringArrayThreadContextMap(final boolean useMap) { - this.useMap = useMap; - this.threadLocalMapState = createThreadLocalMap(useMap); + threadLocalMapState = new ThreadLocal<>(); } @Override public void put(final String key, final String value) { - if (!useMap) { - return; - } final Object[] state = threadLocalMapState.get(); final UnmodifiableArrayBackedMap modifiedMap = UnmodifiableArrayBackedMap.getInstance(state).copyAndPut(key, value); @@ -92,9 +58,6 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt } public void putAll(final Map<String, String> m) { - if (!useMap) { - return; - } final Object[] state = threadLocalMapState.get(); final UnmodifiableArrayBackedMap modifiedMap = UnmodifiableArrayBackedMap.getInstance(state).copyAndPutAll(m); @@ -152,27 +115,17 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt return; } final UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.getInstance(state); - for (final Map.Entry<String, String> entry : map.entrySet()) { - // BiConsumer should be able to handle values of any type V. In our case the values are of type String. - @SuppressWarnings("unchecked") - final V value = (V) entry.getValue(); - action.accept(entry.getKey(), value); - } + map.forEach(action); } @Override public <V, S> void forEach(final TriConsumer<String, ? super V, S> action, final S state) { - Object[] localState = threadLocalMapState.get(); + final Object[] localState = threadLocalMapState.get(); if (localState == null) { return; } - UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.getInstance(localState); - for (final Map.Entry<String, String> entry : map.entrySet()) { - // TriConsumer should be able to handle values of any type V. In our case the values are of type String. - @SuppressWarnings("unchecked") - final V value = (V) entry.getValue(); - action.accept(entry.getKey(), value, state); - } + final UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.getInstance(localState); + map.forEach(action, state); } @SuppressWarnings("unchecked") @@ -224,7 +177,6 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt + ((state == null) ? 0 : UnmodifiableArrayBackedMap.getInstance(state).hashCode()); - result = prime * result + Boolean.valueOf(this.useMap).hashCode(); return result; } @@ -236,12 +188,6 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt if (obj == null) { return false; } - if (obj instanceof StringArrayThreadContextMap) { - final StringArrayThreadContextMap other = (StringArrayThreadContextMap) obj; - if (this.useMap != other.useMap) { - return false; - } - } if (!(obj instanceof ThreadContextMap)) { return false; } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java similarity index 85% rename from log4j-api/src/main/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMap.java rename to log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java index 849d23bf06..84d2868b91 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/UnmodifiableArrayBackedMap.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMap.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.logging.log4j.spi; +package org.apache.logging.log4j.internal.map; import java.io.Serializable; import java.util.AbstractMap; @@ -25,6 +25,8 @@ import java.util.Iterator; import java.util.Map; import java.util.Objects; import java.util.Set; +import org.apache.logging.log4j.util.ReadOnlyStringMap; +import org.apache.logging.log4j.util.TriConsumer; /** * This class represents an immutable map, which stores its state inside a single Object[]: @@ -55,14 +57,19 @@ import java.util.Set; * </ul> * */ -class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements Serializable { +class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements Serializable, ReadOnlyStringMap { /** * Implementation of Map.Entry. The implementation is simple since each instance * contains an index in the array, then getKey() and getValue() retrieve from * the array. Blocks modifications. */ private class UnmodifiableEntry implements Map.Entry<String, String> { - private final int index; + /** + * This field is functionally final, but marking it as such can cause + * performance problems. Consider marking it final after + * https://bugs.openjdk.org/browse/JDK-8324186 is solved. + */ + private int index; public UnmodifiableEntry(int index) { this.index = index; @@ -143,9 +150,6 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements public static final UnmodifiableArrayBackedMap EMPTY_MAP = new UnmodifiableArrayBackedMap(0); - private final Object[] backingArray; - private int numEntries; - private static final int NUM_FIXED_ARRAY_ENTRIES = 1; private static int getArrayIndexForKey(int entryIndex) { @@ -156,11 +160,6 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements return 2 * entryIndex + 1 + NUM_FIXED_ARRAY_ENTRIES; } - private UnmodifiableArrayBackedMap(int capacity) { - this.backingArray = new Object[capacity * 2 + 1]; - this.backingArray[0] = 0; - } - static UnmodifiableArrayBackedMap getInstance(Object[] backingArray) { if (backingArray == null || backingArray.length == 1) { return EMPTY_MAP; @@ -169,6 +168,20 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements } } + /** + * backingArray is functionally final, but marking it as such can cause + * performance problems. Consider marking it final after + * https://bugs.openjdk.org/browse/JDK-8324186 is solved. + */ + private Object[] backingArray; + + private int numEntries; + + private UnmodifiableArrayBackedMap(int capacity) { + this.backingArray = new Object[capacity * 2 + 1]; + this.backingArray[0] = 0; + } + private UnmodifiableArrayBackedMap(Object[] backingArray) { this.numEntries = (backingArray == null ? 0 : (int) backingArray[0]); this.backingArray = backingArray; @@ -195,6 +208,11 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements */ @Override public boolean containsKey(Object key) { + return containsKey((String) key); + } + + @Override + public boolean containsKey(String key) { int hashCode = key.hashCode(); for (int i = 0; i < numEntries; i++) { if (backingArray[getArrayIndexForKey(i)].hashCode() == hashCode @@ -292,7 +310,6 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements * @return */ UnmodifiableArrayBackedMap copyAndRemove(String key) { - UnmodifiableArrayBackedMap newMap = new UnmodifiableArrayBackedMap(numEntries); int indexToRemove = -1; for (int oldIndex = 0; oldIndex < numEntries; oldIndex++) { if (backingArray[getArrayIndexForKey(oldIndex)].hashCode() == key.hashCode() @@ -309,6 +326,7 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements // we have 1 item and we're about to remove it return EMPTY_MAP; } + UnmodifiableArrayBackedMap newMap = new UnmodifiableArrayBackedMap(numEntries); if (indexToRemove > 0) { // copy entries before the removed one System.arraycopy(backingArray, 1, newMap.backingArray, 1, indexToRemove * 2); @@ -419,6 +437,43 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements backingArray[0] = numEntries; } + /** + * This version of forEach is defined on the Map interface. + */ + @Override + public void forEach(java.util.function.BiConsumer<? super String, ? super String> action) { + for (int i = 0; i < numEntries; i++) { + // BiConsumer should be able to handle values of any type V. In our case the values are of type String. + final String key = (String) backingArray[getArrayIndexForKey(i)]; + final String value = (String) backingArray[getArrayIndexForValue(i)]; + action.accept(key, value); + } + } + + /** + * This version of forEach is defined on the ReadOnlyStringMap interface. + */ + @SuppressWarnings("unchecked") + @Override + public <V> void forEach(final org.apache.logging.log4j.util.BiConsumer<String, ? super V> action) { + for (int i = 0; i < numEntries; i++) { + // BiConsumer should be able to handle values of any type V. In our case the values are of type String. + final String key = (String) backingArray[getArrayIndexForKey(i)]; + final V value = (V) backingArray[getArrayIndexForValue(i)]; + action.accept(key, value); + } + } + + @SuppressWarnings("unchecked") + public <V, S> void forEach(final TriConsumer<String, ? super V, S> action, final S state) { + for (int i = 0; i < numEntries; i++) { + // TriConsumer should be able to handle values of any type V. In our case the values are of type String. + final String key = (String) backingArray[getArrayIndexForKey(i)]; + final V value = (V) backingArray[getArrayIndexForValue(i)]; + action.accept(key, value, state); + } + } + @Override public Set<Entry<String, String>> entrySet() { return new UnmodifiableEntrySet(); @@ -429,6 +484,12 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements */ @Override public String get(Object key) { + return getValue((String) key); + } + + @SuppressWarnings("unchecked") + @Override + public <V> V getValue(String key) { if (numEntries == 0) { return null; } @@ -436,7 +497,7 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements for (int i = 0; i < numEntries; i++) { if (backingArray[getArrayIndexForKey(i)].hashCode() == hashCode && backingArray[getArrayIndexForKey(i)].equals(key)) { - return (String) backingArray[getArrayIndexForValue(i)]; + return (V) backingArray[getArrayIndexForValue(i)]; } } return null; @@ -483,4 +544,9 @@ class UnmodifiableArrayBackedMap extends AbstractMap<String, String> implements public int size() { return numEntries; } + + @Override + public Map<String, String> toMap() { + return this; + } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ThreadContextMapFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ThreadContextMapFactory.java index 14c0fe632a..566ae1463c 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ThreadContextMapFactory.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ThreadContextMapFactory.java @@ -70,7 +70,6 @@ public final class ThreadContextMapFactory { CopyOnWriteSortedArrayThreadContextMap.init(); GarbageFreeSortedArrayThreadContextMap.init(); DefaultThreadContextMap.init(); - StringArrayThreadContextMap.init(); initPrivate(); }
