Repository: logging-log4j2 Updated Branches: refs/heads/master cfa4aa577 -> 1c0359314
LOG4J2-1611 ensure JdkMapAdapterStringMap::forEach returns consistent (alphabetic) order for predictable test results Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2621cfd5 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2621cfd5 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2621cfd5 Branch: refs/heads/master Commit: 2621cfd5cc08cdb109e4f8bdfb99c7bcf6b9540a Parents: 11101ce Author: rpopma <[email protected]> Authored: Sun Sep 25 01:41:59 2016 +0900 Committer: rpopma <[email protected]> Committed: Sun Sep 25 01:41:59 2016 +0900 ---------------------------------------------------------------------- .../log4j/core/impl/JdkMapAdapterStringMap.java | 50 +++++++++++++++++--- .../core/impl/JdkMapAdapterStringMapTest.java | 33 +++++++------ 2 files changed, 60 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2621cfd5/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMap.java ---------------------------------------------------------------------- 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 4e22a25..4efc6e8 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 @@ -16,10 +16,11 @@ */ package org.apache.logging.log4j.core.impl; +import java.util.Arrays; +import java.util.Comparator; import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.TreeMap; import org.apache.logging.log4j.util.BiConsumer; import org.apache.logging.log4j.util.ReadOnlyStringMap; @@ -32,9 +33,22 @@ import org.apache.logging.log4j.util.TriConsumer; class JdkMapAdapterStringMap implements StringMap { private static final long serialVersionUID = -7348247784983193612L; private static final String FROZEN = "Frozen collection cannot be modified"; + private static final Comparator<? super String> NULL_FIRST_COMPARATOR = new Comparator<String>() { + @Override + public int compare(final String left, final String right) { + if (left == null) { + return -1; + } + if (right == null) { + return 1; + } + return left.compareTo(right); + } + }; private final Map<String, String> map; private boolean immutable = false; + private transient String[] sortedKeys; public JdkMapAdapterStringMap() { this(new HashMap<String, String>()); @@ -63,19 +77,29 @@ class JdkMapAdapterStringMap implements StringMap { @SuppressWarnings("unchecked") @Override public <V> void forEach(final BiConsumer<String, ? super V> action) { - for (Map.Entry<String, String> entry : map.entrySet()) { - action.accept(entry.getKey(), (V) entry.getValue()); + final String[] keys = getSortedKeys(); + for (int i = 0; i < keys.length; i++) { + action.accept(keys[i], (V) map.get(keys[i])); } } @SuppressWarnings("unchecked") @Override public <V, S> void forEach(final TriConsumer<String, ? super V, S> action, final S state) { - for (Map.Entry<String, String> entry : map.entrySet()) { - action.accept(entry.getKey(), (V) entry.getValue(), state); + final String[] keys = getSortedKeys(); + for (int i = 0; i < keys.length; i++) { + action.accept(keys[i], (V) map.get(keys[i]), state); } } + private String[] getSortedKeys() { + if (sortedKeys == null) { + sortedKeys = map.keySet().toArray(new String[map.size()]); + Arrays.sort(sortedKeys, NULL_FIRST_COMPARATOR); + } + return sortedKeys; + } + @SuppressWarnings("unchecked") @Override public <V> V getValue(final String key) { @@ -99,6 +123,7 @@ class JdkMapAdapterStringMap implements StringMap { } assertNotFrozen(); map.clear(); + sortedKeys = null; } @Override @@ -115,6 +140,7 @@ class JdkMapAdapterStringMap implements StringMap { public void putAll(final ReadOnlyStringMap source) { assertNotFrozen(); source.forEach(PUT_ALL, map); + sortedKeys = null; } private static TriConsumer<String, String, Map<String, String>> PUT_ALL = new TriConsumer<String, String, Map<String, String>>() { @@ -128,6 +154,7 @@ class JdkMapAdapterStringMap implements StringMap { public void putValue(final String key, final Object value) { assertNotFrozen(); map.put(key, value == null ? null : String.valueOf(value)); + sortedKeys = null; } @Override @@ -137,11 +164,22 @@ class JdkMapAdapterStringMap implements StringMap { } assertNotFrozen(); map.remove(key); + sortedKeys = null; } @Override public String toString() { - return new TreeMap<>(map).toString(); + final StringBuilder result = new StringBuilder(map.size() * 13); + result.append('{'); + final String[] keys = getSortedKeys(); + for (int i = 0; i < keys.length; i++) { + if (i > 0) { + result.append(", "); + } + result.append(keys[i]).append('=').append(map.get(keys[i])); + } + result.append('}'); + return result.toString(); } @Override http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2621cfd5/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java index 027ca45..c8c8db1 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/JdkMapAdapterStringMapTest.java @@ -21,7 +21,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.util.ConcurrentModificationException; import java.util.HashMap; import java.util.Map; @@ -451,8 +450,8 @@ public class JdkMapAdapterStringMapTest { assertEquals("ccc", original.getValue("c")); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationBiConsumerPut() { + @Test + public void testNoConcurrentModificationBiConsumerPut() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -467,8 +466,8 @@ public class JdkMapAdapterStringMapTest { }); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationBiConsumerPutValue() { + @Test + public void testNoConcurrentModificationBiConsumerPutValue() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -483,8 +482,8 @@ public class JdkMapAdapterStringMapTest { }); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationBiConsumerRemove() { + @Test + public void testNoConcurrentModificationBiConsumerRemove() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -497,8 +496,8 @@ public class JdkMapAdapterStringMapTest { }); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationBiConsumerClear() { + @Test + public void testNoConcurrentModificationBiConsumerClear() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -513,8 +512,8 @@ public class JdkMapAdapterStringMapTest { }); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationTriConsumerPut() { + @Test + public void testNoConcurrentModificationTriConsumerPut() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -528,8 +527,8 @@ public class JdkMapAdapterStringMapTest { }, null); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationTriConsumerPutValue() { + @Test + public void testNoConcurrentModificationTriConsumerPutValue() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -544,8 +543,8 @@ public class JdkMapAdapterStringMapTest { }, null); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationTriConsumerRemove() { + @Test + public void testNoConcurrentModificationTriConsumerRemove() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa"); @@ -558,8 +557,8 @@ public class JdkMapAdapterStringMapTest { }, null); } - @Test(expected = ConcurrentModificationException.class) - public void testConcurrentModificationTriConsumerClear() { + @Test + public void testNoConcurrentModificationTriConsumerClear() { final JdkMapAdapterStringMap original = new JdkMapAdapterStringMap(); original.putValue("a", "aaa"); original.putValue("b", "aaa");
