# HG changeset patch # User mduigou # Date 1371596590 25200 # Node ID 6c3c0ff49eb5b3e3ccb8468fecb1037ce362a2ed # Parent 6a32863b0186dffa2506149259616b64c1aeafdd 8016446: Improve forEach/replaceAll for Map, HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap, ConcurrentMap Reviewed-by: forax, mduigou, psandoz Contributed-by: Mike Duigou , Remi Forax diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/HashMap.java --- a/jdk/src/share/classes/java/util/HashMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/HashMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -29,6 +29,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.concurrent.ThreadLocalRandom; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.BiFunction; import java.util.function.Function; @@ -1299,10 +1300,112 @@ */ public V remove(Object key) { Entry e = removeEntryForKey(key); - return (e == null ? null : e.value); + return (e == null ? null : e.value); + } + + // optimized implementations of default methods in Map + + @Override + public void forEach(BiConsumer action) { + Objects.requireNonNull(action); + final int expectedModCount = modCount; + if (nullKeyEntry != null) { + forEachNullKey(expectedModCount, action); + } + Object[] tab = this.table; + for (int index = 0; index < tab.length; index++) { + Object item = tab[index]; + if (item == null) { + continue; + } + if (item instanceof HashMap.TreeBin) { + eachTreeNode(expectedModCount, ((TreeBin)item).first, action); + continue; + } + @SuppressWarnings("unchecked") + Entry entry = (Entry)item; + while (entry != null) { + action.accept(entry.key, entry.value); + entry = (Entry)entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + } + + private void eachTreeNode(int expectedModCount, TreeNode node, BiConsumer action) { + while (node != null) { + @SuppressWarnings("unchecked") + Entry entry = (Entry)node.entry; + action.accept(entry.key, entry.value); + node = (TreeNode)entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } } - // optimized implementations of default methods in Map + private void forEachNullKey(int expectedModCount, BiConsumer action) { + action.accept(null, nullKeyEntry.value); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + + @Override + public void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + final int expectedModCount = modCount; + if (nullKeyEntry != null) { + replaceforNullKey(expectedModCount, function); + } + Object[] tab = this.table; + for (int index = 0; index < tab.length; index++) { + Object item = tab[index]; + if (item == null) { + continue; + } + if (item instanceof HashMap.TreeBin) { + replaceEachTreeNode(expectedModCount, ((TreeBin)item).first, function); + continue; + } + @SuppressWarnings("unchecked") + Entry entry = (Entry)item; + while (entry != null) { + entry.value = function.apply(entry.key, entry.value); + entry = (Entry)entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + } + + private void replaceEachTreeNode(int expectedModCount, TreeNode node, BiFunction function) { + while (node != null) { + @SuppressWarnings("unchecked") + Entry entry = (Entry)node.entry; + entry.value = function.apply(entry.key, entry.value); + node = (TreeNode)entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + + private void replaceforNullKey(int expectedModCount, BiFunction function) { + nullKeyEntry.value = function.apply(null, nullKeyEntry.value); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } @Override public V putIfAbsent(K key, V value) { @@ -2297,12 +2400,12 @@ if (e == null) throw new NoSuchElementException(); - if (e instanceof Entry) { + if (e instanceof TreeNode) { // TreeBin + retVal = (Entry)((TreeNode)e).entry; + next = retVal.next; + } else { retVal = (Entry)e; next = ((Entry)e).next; - } else { // TreeBin - retVal = (Entry)((TreeNode)e).entry; - next = retVal.next; } if (next == null) { // Move to next bin diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/Hashtable.java --- a/jdk/src/share/classes/java/util/Hashtable.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/Hashtable.java Tue Jun 18 16:03:10 2013 -0700 @@ -932,19 +932,39 @@ public synchronized void forEach(BiConsumer action) { Objects.requireNonNull(action); // explicit check required in case // table is empty. - Entry[] tab = table; - for (Entry entry : tab) { + final int expectedModCount = modCount; + + Entry[] tab = table; + for (Entry entry : tab) { while (entry != null) { action.accept((K)entry.key, (V)entry.value); entry = entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } } } } @Override - public synchronized void replaceAll( - BiFunction function) { - Map.super.replaceAll(function); + public synchronized void replaceAll(BiFunction function) { + Objects.requireNonNull(function); // explicit check required in case + // table is empty. + final int expectedModCount = modCount; + + Entry[] tab = (Entry[])table; + for (Entry entry : tab) { + while (entry != null) { + entry.value = Objects.requireNonNull( + function.apply(entry.key, entry.value)); + entry = entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } } @Override @@ -1058,7 +1078,7 @@ } @Override - public V computeIfPresent(K key, BiFunction remappingFunction) { + public synchronized V computeIfPresent(K key, BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); Entry tab[] = table; @@ -1087,7 +1107,7 @@ } @Override - public V compute(K key, BiFunction remappingFunction) { + public synchronized V compute(K key, BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); Entry tab[] = table; @@ -1122,7 +1142,7 @@ } @Override - public V merge(K key, V value, BiFunction remappingFunction) { + public synchronized V merge(K key, V value, BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); Entry tab[] = table; diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/IdentityHashMap.java --- a/jdk/src/share/classes/java/util/IdentityHashMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/IdentityHashMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -27,6 +27,8 @@ import java.io.*; import java.lang.reflect.Array; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Consumer; /** @@ -1337,6 +1339,42 @@ tab[i + 1] = value; } + @Override + public void forEach(BiConsumer action) { + Objects.requireNonNull(action); + int expectedModCount = modCount; + + Object[] t = table; + for (int index = 0; index < t.length; index += 2) { + Object k = t[index]; + if (k != null) { + action.accept((K) unmaskNull(k), (V) t[index + 1]); + } + + if (modCount != expectedModCount) { + throw new ConcurrentModificationException(); + } + } + } + + @Override + public void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + int expectedModCount = modCount; + + Object[] t = table; + for (int index = 0; index < t.length; index += 2) { + Object k = t[index]; + if (k != null) { + t[index + 1] = function.apply((K) unmaskNull(k), (V) t[index + 1]); + } + + if (modCount != expectedModCount) { + throw new ConcurrentModificationException(); + } + } + } + /** * Similar form as array-based Spliterators, but skips blank elements, * and guestimates size as decreasing by half per split. diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/LinkedHashMap.java --- a/jdk/src/share/classes/java/util/LinkedHashMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/LinkedHashMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -25,6 +25,8 @@ package java.util; import java.io.*; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; /** *

Hash table and linked list implementation of the Map interface, @@ -296,6 +298,32 @@ header.before = header.after = header; } + @Override + public void forEach(BiConsumer action) { + Objects.requireNonNull(action); + int expectedModCount = modCount; + for (Entry entry = header.after; entry != header; entry = entry.after) { + action.accept(entry.key, entry.value); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + + @Override + public void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + int expectedModCount = modCount; + for (Entry entry = header.after; entry != header; entry = entry.after) { + entry.value = function.apply(entry.key, entry.value); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + /** * LinkedHashMap entry. */ diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/Map.java --- a/jdk/src/share/classes/java/util/Map.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/Map.java Tue Jun 18 16:03:10 2013 -0700 @@ -545,6 +545,7 @@ k = entry.getKey(); v = entry.getValue(); } catch(IllegalStateException ise) { + // this usually means the entry is no longer in the map. throw new ConcurrentModificationException(ise); } action.accept(k, v); @@ -599,9 +600,19 @@ k = entry.getKey(); v = entry.getValue(); } catch(IllegalStateException ise) { + // this usually means the entry is no longer in the map. throw new ConcurrentModificationException(ise); } - entry.setValue(function.apply(k, v)); + + // ise thrown from function is not a cme. + v = function.apply(k, v); + + try { + entry.setValue(v); + } catch(IllegalStateException ise) { + // this usually means the entry is no longer in the map. + throw new ConcurrentModificationException(ise); + } } } diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/TreeMap.java --- a/jdk/src/share/classes/java/util/TreeMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/TreeMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -25,6 +25,8 @@ package java.util; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Consumer; /** @@ -945,6 +947,33 @@ return tailMap(fromKey, true); } + @Override + public void forEach(BiConsumer action) { + Objects.requireNonNull(action); + int expectedModCount = modCount; + for (Entry e = getFirstEntry(); e != null; e = successor(e)) { + action.accept(e.key, e.value); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + + @Override + public void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + int expectedModCount = modCount; + + for (Entry e = getFirstEntry(); e != null; e = successor(e)) { + e.value = Objects.requireNonNull(function.apply(e.key, e.value)); + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + // View class support class Values extends AbstractCollection { diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/WeakHashMap.java --- a/jdk/src/share/classes/java/util/WeakHashMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/WeakHashMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -28,6 +28,8 @@ import java.lang.ref.WeakReference; import java.lang.ref.ReferenceQueue; import java.util.concurrent.ThreadLocalRandom; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Consumer; @@ -1036,6 +1038,48 @@ } } + @Override + public void forEach(BiConsumer action) { + Objects.requireNonNull(action); + int expectedModCount = modCount; + + Entry[] tab = getTable(); + for (Entry entry : tab) { + while (entry != null) { + Object key = entry.get(); + if (key != null) { + action.accept((K)WeakHashMap.unmaskNull(key), entry.value); + } + entry = entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + } + + @Override + public void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + int expectedModCount = modCount; + + Entry[] tab = getTable();; + for (Entry entry : tab) { + while (entry != null) { + Object key = entry.get(); + if (key != null) { + entry.value = function.apply((K)WeakHashMap.unmaskNull(key), entry.value); + } + entry = entry.next; + + if (expectedModCount != modCount) { + throw new ConcurrentModificationException(); + } + } + } + } + /** * Similar form as other hash Spliterators, but skips dead * elements. diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java Tue Jun 18 16:03:10 2013 -0700 @@ -35,6 +35,8 @@ package java.util.concurrent; import java.util.Map; +import java.util.Objects; +import java.util.function.BiFunction; /** * A {@link java.util.Map} providing additional atomic @@ -183,4 +185,26 @@ * or value prevents it from being stored in this map */ V replace(K key, V value); + + /** + * {@inheritDoc} + * + * @implNote This implementation assumes that the ConcurrentMap cannot + * contain null values and get() returning null unambiguously means the key + * is absent. Implementations which support null values + * must override this default implementation. + */ + @Override + default void replaceAll(BiFunction function) { + Objects.requireNonNull(function); + forEach((k,v) -> { + while(!replace(k, v, function.apply(k, v))) { + // v changed or k is gone + if( (v = get(k)) == null) { + // k is no longer in the map. + break; + } + } + }); + } } diff -r 6a32863b0186 -r 6c3c0ff49eb5 jdk/test/java/util/Map/Defaults.java --- a/jdk/test/java/util/Map/Defaults.java Tue Jun 18 14:11:45 2013 -0700 +++ b/jdk/test/java/util/Map/Defaults.java Tue Jun 18 16:03:10 2013 -0700 @@ -36,6 +36,7 @@ import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; +import java.util.HashSet; import java.util.Hashtable; import java.util.IdentityHashMap; import java.util.Iterator; @@ -47,6 +48,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListMap; +import java.util.function.BiFunction; import java.util.function.Supplier; import org.testng.annotations.Test; @@ -60,14 +62,14 @@ public class Defaults { - @Test(dataProvider = "Nulls Map") + @Test(dataProvider = "Map rw=all keys=withNull values=withNull") public void testGetOrDefaultNulls(String description, Map map) { - assertTrue(map.containsKey(null), "null key absent"); - assertNull(map.get(null), "value not null"); - assertSame(map.get(null), map.getOrDefault(null, EXTRA_VALUE), "values should match"); + assertTrue(map.containsKey(null), description + ": null key absent"); + assertNull(map.get(null), description + ": value not null"); + assertSame(map.get(null), map.getOrDefault(null, EXTRA_VALUE), description + ": values should match"); } - @Test(dataProvider = "Map") + @Test(dataProvider = "Map rw=all keys=all values=all") public void testGetOrDefault(String description, Map map) { assertTrue(map.containsKey(KEYS[1]), "expected key missing"); assertSame(map.get(KEYS[1]), map.getOrDefault(KEYS[1], EXTRA_VALUE), "values should match"); @@ -76,7 +78,7 @@ assertNull(map.getOrDefault(EXTRA_KEY, null), "null not returned as default"); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testPutIfAbsentNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -96,7 +98,7 @@ assertSame(map.get(null), EXTRA_VALUE, "value not expected"); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testPutIfAbsent(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object expected = map.get(KEYS[1]); @@ -109,7 +111,7 @@ assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); } - @Test(dataProvider = "Nulls Map") + @Test(dataProvider = "Map rw=all keys=all values=all") public void testForEach(String description, Map map) { IntegerEnum[] EACH_KEY = new IntegerEnum[map.size()]; @@ -120,10 +122,43 @@ assertSame(v, map.get(k)); }); - assertEquals(KEYS, EACH_KEY); + assertEquals(KEYS, EACH_KEY, description); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=all values=all") + public static void testReplaceAll(String description, Map map) { + IntegerEnum[] EACH_KEY = new IntegerEnum[map.size()]; + Set EACH_REPLACE = new HashSet<>(map.size()); + + map.replaceAll((k,v) -> { + int idx = (null == k) ? 0 : k.ordinal(); // substitute for index. + assertNull(EACH_KEY[idx]); + EACH_KEY[idx] = (idx == 0) ? KEYS[0] : k; // substitute for comparison. + assertSame(v, map.get(k)); + String replacement = v + " replaced"; + EACH_REPLACE.add(replacement); + return replacement; + }); + + assertEquals(KEYS, EACH_KEY, description); + assertEquals(map.values().size(), EACH_REPLACE.size(), description + EACH_REPLACE); + assertTrue(EACH_REPLACE.containsAll(map.values()), description + " : " + EACH_REPLACE + " != " + map.values()); + assertTrue(map.values().containsAll(EACH_REPLACE), description + " : " + EACH_REPLACE + " != " + map.values()); + } + + @Test(dataProvider = "Map rw=true keys=nonNull values=nonNull") + public static void testReplaceAllNoNullReplacement(String description, Map map) { + assertThrows( + () -> { map.replaceAll(null); }, + NullPointerException.class, + description); + assertThrows( + () -> { map.replaceAll((k,v) -> null); }, + NullPointerException.class, + description); + } + + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public static void testRemoveNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -136,7 +171,7 @@ assertFalse(map.remove(null, null)); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public static void testRemove(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object expected = map.get(KEYS[1]); @@ -151,7 +186,7 @@ assertFalse(map.remove(EXTRA_KEY, EXTRA_VALUE)); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testReplaceKVNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -159,7 +194,7 @@ assertSame(map.get(null), EXTRA_VALUE); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testReplaceKV(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object expected = map.get(KEYS[1]); @@ -177,7 +212,7 @@ assertSame(map.get(EXTRA_KEY), expected); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testReplaceKVVNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -189,7 +224,7 @@ assertSame(map.get(null), EXTRA_VALUE); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testReplaceKVV(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object expected = map.get(KEYS[1]); @@ -212,7 +247,7 @@ assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testComputeIfAbsentNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -220,7 +255,7 @@ assertSame(map.get(null), EXTRA_VALUE, description); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testComputeIfAbsent(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object expected = map.get(KEYS[1]); @@ -234,7 +269,7 @@ assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testComputeIfPresentNulls(String description, Map map) { assertTrue(map.containsKey(null)); assertNull(map.get(null)); @@ -246,7 +281,7 @@ assertSame(map.get(null), null, description); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testComputeIfPresent(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object value = map.get(KEYS[1]); @@ -267,7 +302,7 @@ assertSame(map.get(EXTRA_KEY), null); } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testComputeNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -287,7 +322,7 @@ }), null, description); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testCompute(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object value = map.get(KEYS[1]); @@ -314,7 +349,7 @@ } - @Test(dataProvider = "R/W Nulls Map") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testMergeNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -327,7 +362,7 @@ assertSame(map.get(null), EXTRA_VALUE, description); } - @Test(dataProvider = "R/W Map") + @Test(dataProvider = "Map rw=true keys=all values=all") public void testMerge(String description, Map map) { assertTrue(map.containsKey(KEYS[1])); Object value = map.get(KEYS[1]); @@ -391,52 +426,58 @@ private static final IntegerEnum EXTRA_KEY = IntegerEnum.EXTRA_KEY; private static final String EXTRA_VALUE = String.valueOf(TEST_SIZE); - @DataProvider(name = "Map", parallel = true) - public static Iterator allNullsMapProvider() { + @DataProvider(name = "Map rw=all keys=all values=all", parallel = true) + public static Iterator allMapProvider() { return makeAllMaps().iterator(); } - @DataProvider(name = "Nulls Map", parallel = true) - public static Iterator allMapProvider() { - return makeRWMaps(true).iterator(); + @DataProvider(name = "Map rw=all keys=withNull values=withNull", parallel = true) + public static Iterator allMapWithNullsProvider() { + return makeAllMapsWithNulls().iterator(); } - @DataProvider(name = "R/W Map", parallel = true) - public static Iterator rwMapProvider() { + @DataProvider(name = "Map rw=true keys=nonNull values=nonNull", parallel = true) + public static Iterator rwNonNullMapProvider() { + return makeRWNoNullsMaps().iterator(); + } + + @DataProvider(name = "Map rw=true keys=nonNull values=all", parallel = true) + public static Iterator rwNonNullKeysMapProvider() { return makeRWMapsNoNulls().iterator(); } - @DataProvider(name = "R/W Nulls Map", parallel = true) - public static Iterator rwNullsMapProvider() { - return makeRWMaps(true).iterator(); + @DataProvider(name = "Map rw=true keys=all values=all", parallel = true) + public static Iterator rwMapProvider() { + return makeAllRWMaps().iterator(); } - private static Collection makeAllMapsNoNulls() { + @DataProvider(name = "Map rw=true keys=withNull values=withNull", parallel = true) + public static Iterator rwNullsMapProvider() { + return makeAllRWMapsWithNulls().iterator(); + } + + private static Collection makeAllRWMapsWithNulls() { Collection all = new ArrayList<>(); - all.addAll(makeRWMaps(false)); - all.addAll(makeRWNoNullsMaps()); - all.addAll(makeROMaps(false)); + all.addAll(makeRWMaps(true, true)); return all; } + private static Collection makeRWMapsNoNulls() { Collection all = new ArrayList<>(); - all.addAll(makeRWMaps(false)); + all.addAll(makeRWNoNullKeysMaps(false)); all.addAll(makeRWNoNullsMaps()); return all; } - private static Collection makeAllMaps() { + private static Collection makeAllROMaps() { Collection all = new ArrayList<>(); all.addAll(makeROMaps(false)); - all.addAll(makeRWMaps(false)); - all.addAll(makeRWNoNullsMaps()); - all.addAll(makeRWMaps(true)); all.addAll(makeROMaps(true)); return all; @@ -445,52 +486,99 @@ private static Collection makeAllRWMaps() { Collection all = new ArrayList<>(); - all.addAll(makeRWMaps(false)); all.addAll(makeRWNoNullsMaps()); - all.addAll(makeRWMaps(true)); + all.addAll(makeRWMaps(false,true)); + all.addAll(makeRWMaps(true,true)); + all.addAll(makeRWNoNullKeysMaps(true)); + return all; + } + + private static Collection makeAllMaps() { + Collection all = new ArrayList<>(); + + all.addAll(makeAllROMaps()); + all.addAll(makeAllRWMaps()); return all; } - private static Collection makeRWMaps(boolean nulls) { + private static Collection makeAllMapsWithNulls() { + Collection all = new ArrayList<>(); + + all.addAll(makeROMaps(true)); + all.addAll(makeRWMaps(true,true)); + + return all; + } + /** + * + * @param nullKeys include null keys + * @param nullValues include null values + * @return + */ + private static Collection makeRWMaps(boolean nullKeys, boolean nullValues) { return Arrays.asList( - new Object[]{"HashMap", makeMap(HashMap::new, nulls)}, - new Object[]{"IdentityHashMap", makeMap(IdentityHashMap::new, nulls)}, - new Object[]{"LinkedHashMap", makeMap(LinkedHashMap::new, nulls)}, - new Object[]{"WeakHashMap", makeMap(WeakHashMap::new, nulls)}, - new Object[]{"Collections.checkedMap(HashMap)", Collections.checkedMap(makeMap(HashMap::new, nulls), IntegerEnum.class, String.class)}, - new Object[]{"Collections.synchronizedMap(HashMap)", Collections.synchronizedMap(makeMap(HashMap::new, nulls))}, - new Object[]{"ExtendsAbstractMap", makeMap(ExtendsAbstractMap::new, nulls)}); + new Object[]{"HashMap", makeMap(HashMap::new, nullKeys, nullValues)}, + new Object[]{"IdentityHashMap", makeMap(IdentityHashMap::new, nullKeys, nullValues)}, + new Object[]{"LinkedHashMap", makeMap(LinkedHashMap::new, nullKeys, nullValues)}, + new Object[]{"WeakHashMap", makeMap(WeakHashMap::new, nullKeys, nullValues)}, + new Object[]{"Collections.checkedMap(HashMap)", Collections.checkedMap(makeMap(HashMap::new, nullKeys, nullValues), IntegerEnum.class, String.class)}, + new Object[]{"Collections.synchronizedMap(HashMap)", Collections.synchronizedMap(makeMap(HashMap::new, nullKeys, nullValues))}, + new Object[]{"ExtendsAbstractMap", makeMap(ExtendsAbstractMap::new, nullKeys, nullValues)}); + } + + /** + * + * @param nulls include null values + * @return + */ + private static Collection makeRWNoNullKeysMaps(boolean nulls) { + return Arrays.asList( + // null key hostile + new Object[]{"EnumMap", makeMap(() -> new EnumMap(IntegerEnum.class), false, nulls)}, + new Object[]{"Collections.synchronizedMap(EnumMap)", Collections.synchronizedMap(makeMap(() -> new EnumMap(IntegerEnum.class), false, nulls))} + ); } private static Collection makeRWNoNullsMaps() { return Arrays.asList( - // null hostile - new Object[]{"EnumMap", makeMap(() -> new EnumMap(IntegerEnum.class), false)}, - new Object[]{"Hashtable", makeMap(Hashtable::new, false)}, - new Object[]{"TreeMap", makeMap(TreeMap::new, false)}, - new Object[]{"ConcurrentHashMap", makeMap(ConcurrentHashMap::new, false)}, - new Object[]{"ConcurrentSkipListMap", makeMap(ConcurrentSkipListMap::new, false)}, - new Object[]{"Collections.checkedMap(ConcurrentHashMap)", Collections.checkedMap(makeMap(ConcurrentHashMap::new, false), IntegerEnum.class, String.class)}, - new Object[]{"Collections.synchronizedMap(EnumMap)", Collections.synchronizedMap(makeMap(() -> new EnumMap(IntegerEnum.class), false))}, - new Object[]{"ImplementsConcurrentMap", makeMap(ImplementsConcurrentMap::new, false)}); + // null key and value hostile + new Object[]{"Hashtable", makeMap(Hashtable::new, false, false)}, + new Object[]{"TreeMap", makeMap(TreeMap::new, false, false)}, + new Object[]{"ConcurrentHashMap", makeMap(ConcurrentHashMap::new, false, false)}, + new Object[]{"ConcurrentSkipListMap", makeMap(ConcurrentSkipListMap::new, false, false)}, + new Object[]{"Collections.checkedMap(ConcurrentHashMap)", Collections.checkedMap(makeMap(ConcurrentHashMap::new, false, false), IntegerEnum.class, String.class)}, + new Object[]{"ImplementsConcurrentMap", makeMap(ImplementsConcurrentMap::new, false, false)} + ); } + /** + * + * @param nulls include nulls + * @return + */ private static Collection makeROMaps(boolean nulls) { return Arrays.asList(new Object[][]{ - new Object[]{"Collections.unmodifiableMap(HashMap)", Collections.unmodifiableMap(makeMap(HashMap::new, nulls))} + new Object[]{"Collections.unmodifiableMap(HashMap)", Collections.unmodifiableMap(makeMap(HashMap::new, nulls, nulls))} }); } - private static Map makeMap(Supplier> supplier, boolean nulls) { + /** + * + * @param supplier a supplier of mutable map instances. + * + * @param nullKeys include null keys + * @param nullValues include null values + * @return + */ + private static Map makeMap(Supplier> supplier, boolean nullKeys, boolean nullValues) { Map result = supplier.get(); for (int each = 0; each < TEST_SIZE; each++) { - if (nulls) { - result.put((each == 0) ? null : KEYS[each], null); - } else { - result.put(KEYS[each], VALUES[each]); - } + IntegerEnum key = nullKeys ? (each == 0) ? null : KEYS[each] : KEYS[each]; + String value = nullValues ? (each == 0) ? null : VALUES[each] : VALUES[each]; + + result.put(key, value); } return result; @@ -520,6 +608,12 @@ : "Failed to throw " + throwable.getCanonicalName()); } + public static void assertThrows(Class throwable, String message, Thrower... throwers) { + for(Thrower thrower : throwers) { + assertThrows(thrower, throwable, message); + } + } + public static void assertInstance(T actual, Class expected) { assertInstance(expected.isInstance(actual), null); }