8022326: Spliterator for values of j.u.c.ConcurrentSkipListMap does not report ORDERED
Reviewed-by: martin, chegar
--- a/jdk/src/share/classes/java/util/TreeMap.java Thu Aug 08 23:40:46 2013 -0700
+++ b/jdk/src/share/classes/java/util/TreeMap.java Fri Aug 09 11:44:38 2013 +0200
@@ -2944,16 +2944,11 @@
@Override
public Comparator<Map.Entry<K, V>> getComparator() {
- // Since SORTED is reported and Map.Entry elements are not comparable
- // then a non-null comparator needs to be returned
+ // Adapt or create a key-based comparator
if (tree.comparator != null) {
- // Adapt the existing non-null comparator to compare entries
- // by key
return Map.Entry.comparingByKey(tree.comparator);
}
else {
- // Return a comparator of entries by key, with K assumed to be
- // of Comparable
return (Comparator<Map.Entry<K, V>> & Serializable) (e1, e2) -> {
@SuppressWarnings("unchecked")
Comparable<? super K> k1 = (Comparable<? super K>) e1.getKey();
--- a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java Thu Aug 08 23:40:46 2013 -0700
+++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java Fri Aug 09 11:44:38 2013 +0200
@@ -34,6 +34,7 @@
*/
package java.util.concurrent;
+import java.io.Serializable;
import java.util.AbstractCollection;
import java.util.AbstractMap;
import java.util.AbstractSet;
@@ -44,11 +45,15 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.NavigableMap;
import java.util.NavigableSet;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.SortedMap;
+import java.util.SortedSet;
import java.util.Spliterator;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentNavigableMap;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.BiConsumer;
@@ -108,9 +113,7 @@
* @since 1.6
*/
public class ConcurrentSkipListMap<K,V> extends AbstractMap<K,V>
- implements ConcurrentNavigableMap<K,V>,
- Cloneable,
- java.io.Serializable {
+ implements ConcurrentNavigableMap<K,V>, Cloneable, Serializable {
/*
* This class implements a tree-like two-dimensionally linked skip
* list in which the index levels are represented in separate
@@ -1412,6 +1415,8 @@
/**
* Saves this map to a stream (that is, serializes it).
*
+ * @param s the stream
+ * @throws java.io.IOException if an I/O error occurs
* @serialData The key (Object) and value (Object) for each
* key-value mapping represented by the map, followed by
* {@code null}. The key-value mappings are emitted in key-order
@@ -1436,6 +1441,10 @@
/**
* Reconstitutes this map from a stream (that is, deserializes it).
+ * @param s the stream
+ * @throws ClassNotFoundException if the class of a serialized object
+ * could not be found
+ * @throws java.io.IOException if an I/O error occurs
*/
@SuppressWarnings("unchecked")
private void readObject(final java.io.ObjectInputStream s)
@@ -2548,8 +2557,7 @@
* @serial include
*/
static final class SubMap<K,V> extends AbstractMap<K,V>
- implements ConcurrentNavigableMap<K,V>, Cloneable,
- java.io.Serializable {
+ implements ConcurrentNavigableMap<K,V>, Cloneable, Serializable {
private static final long serialVersionUID = -7647078645895051609L;
/** Underlying map */
@@ -3180,6 +3188,7 @@
public long estimateSize() {
return Long.MAX_VALUE;
}
+
}
final class SubMapValueIterator extends SubMapIter<V> {
@@ -3434,7 +3443,8 @@
}
public int characteristics() {
- return Spliterator.CONCURRENT | Spliterator.NONNULL;
+ return Spliterator.CONCURRENT | Spliterator.ORDERED |
+ Spliterator.NONNULL;
}
}
@@ -3528,8 +3538,17 @@
}
public final Comparator<Map.Entry<K,V>> getComparator() {
- return comparator == null ? null :
- Map.Entry.comparingByKey(comparator);
+ // Adapt or create a key-based comparator
+ if (comparator != null) {
+ return Map.Entry.comparingByKey(comparator);
+ }
+ else {
+ return (Comparator<Map.Entry<K,V>> & Serializable) (e1, e2) -> {
+ @SuppressWarnings("unchecked")
+ Comparable<? super K> k1 = (Comparable<? super K>) e1.getKey();
+ return k1.compareTo(e2.getKey());
+ };
+ }
}
}
--- a/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java Thu Aug 08 23:40:46 2013 -0700
+++ b/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java Fri Aug 09 11:44:38 2013 +0200
@@ -23,7 +23,7 @@
/**
* @test
- * @bug 8020156 8020009
+ * @bug 8020156 8020009 8022326
* @run testng SpliteratorCharacteristics
*/
@@ -32,80 +32,134 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
+import java.util.Map;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.SortedSet;
import java.util.Spliterator;
import java.util.TreeMap;
import java.util.TreeSet;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.ConcurrentSkipListSet;
import static org.testng.Assert.*;
@Test
public class SpliteratorCharacteristics {
- public void testTreeMap() {
- TreeMap<Integer, String> tm = new TreeMap<>();
- tm.put(1, "4");
- tm.put(2, "3");
- tm.put(3, "2");
- tm.put(4, "1");
+ // TreeMap
- assertCharacteristics(tm.keySet(),
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNullComparator(tm.keySet());
-
- assertCharacteristics(tm.values(),
- Spliterator.SIZED | Spliterator.ORDERED);
- assertISEComparator(tm.values());
-
- assertCharacteristics(tm.entrySet(),
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNotNullComparator(tm.entrySet());
+ public void testTreeMap() {
+ assertSortedMapCharacteristics(new TreeMap<>(),
+ Spliterator.SIZED | Spliterator.DISTINCT |
+ Spliterator.SORTED | Spliterator.ORDERED);
}
public void testTreeMapWithComparator() {
- TreeMap<Integer, String> tm = new TreeMap<>(Comparator.<Integer>reverseOrder());
- tm.put(1, "4");
- tm.put(2, "3");
- tm.put(3, "2");
- tm.put(4, "1");
-
- assertCharacteristics(tm.keySet(),
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNotNullComparator(tm.keySet());
-
- assertCharacteristics(tm.values(),
- Spliterator.SIZED | Spliterator.ORDERED);
- assertISEComparator(tm.values());
-
- assertCharacteristics(tm.entrySet(),
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNotNullComparator(tm.entrySet());
+ assertSortedMapCharacteristics(new TreeMap<>(Comparator.reverseOrder()),
+ Spliterator.SIZED | Spliterator.DISTINCT |
+ Spliterator.SORTED | Spliterator.ORDERED);
}
- public void testTreeSet() {
- TreeSet<Integer> ts = new TreeSet<>();
- ts.addAll(Arrays.asList(1, 2, 3, 4));
+
+ // TreeSet
- assertCharacteristics(ts,
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNullComparator(ts);
+ public void testTreeSet() {
+ assertSortedSetCharacteristics(new TreeSet<>(),
+ Spliterator.SIZED | Spliterator.DISTINCT |
+ Spliterator.SORTED | Spliterator.ORDERED);
}
public void testTreeSetWithComparator() {
- TreeSet<Integer> ts = new TreeSet<>(Comparator.reverseOrder());
- ts.addAll(Arrays.asList(1, 2, 3, 4));
+ assertSortedSetCharacteristics(new TreeSet<>(Comparator.reverseOrder()),
+ Spliterator.SIZED | Spliterator.DISTINCT |
+ Spliterator.SORTED | Spliterator.ORDERED);
+ }
+
+
+ // ConcurrentSkipListMap
+
+ public void testConcurrentSkipListMap() {
+ assertSortedMapCharacteristics(new ConcurrentSkipListMap<>(),
+ Spliterator.CONCURRENT | Spliterator.NONNULL |
+ Spliterator.DISTINCT | Spliterator.SORTED |
+ Spliterator.ORDERED);
+ }
- assertCharacteristics(ts,
- Spliterator.SIZED | Spliterator.DISTINCT |
- Spliterator.SORTED | Spliterator.ORDERED);
- assertNotNullComparator(ts);
+ public void testConcurrentSkipListMapWithComparator() {
+ assertSortedMapCharacteristics(new ConcurrentSkipListMap<>(Comparator.<Integer>reverseOrder()),
+ Spliterator.CONCURRENT | Spliterator.NONNULL |
+ Spliterator.DISTINCT | Spliterator.SORTED |
+ Spliterator.ORDERED);
+ }
+
+
+ // ConcurrentSkipListSet
+
+ public void testConcurrentSkipListSet() {
+ assertSortedSetCharacteristics(new ConcurrentSkipListSet<>(),
+ Spliterator.CONCURRENT | Spliterator.NONNULL |
+ Spliterator.DISTINCT | Spliterator.SORTED |
+ Spliterator.ORDERED);
+ }
+
+ public void testConcurrentSkipListSetWithComparator() {
+ assertSortedSetCharacteristics(new ConcurrentSkipListSet<>(Comparator.reverseOrder()),
+ Spliterator.CONCURRENT | Spliterator.NONNULL |
+ Spliterator.DISTINCT | Spliterator.SORTED |
+ Spliterator.ORDERED);
}
+ //
+
+ void assertSortedMapCharacteristics(SortedMap<Integer, String> m, int keyCharacteristics) {
+ initMap(m);
+
+ boolean hasComparator = m.comparator() != null;
+
+ Set<Integer> keys = m.keySet();
+ assertCharacteristics(keys, keyCharacteristics);
+ if (hasComparator) {
+ assertNotNullComparator(keys);
+ }
+ else {
+ assertNullComparator(keys);
+ }
+
+ assertCharacteristics(m.values(),
+ keyCharacteristics & ~(Spliterator.DISTINCT | Spliterator.SORTED));
+ assertISEComparator(m.values());
+
+ assertCharacteristics(m.entrySet(), keyCharacteristics);
+ assertNotNullComparator(m.entrySet());
+ }
+
+ void assertSortedSetCharacteristics(SortedSet<Integer> s, int keyCharacteristics) {
+ initSet(s);
+
+ boolean hasComparator = s.comparator() != null;
+
+ assertCharacteristics(s, keyCharacteristics);
+ if (hasComparator) {
+ assertNotNullComparator(s);
+ }
+ else {
+ assertNullComparator(s);
+ }
+ }
+
+ void initMap(Map<Integer, String> m) {
+ m.put(1, "4");
+ m.put(2, "3");
+ m.put(3, "2");
+ m.put(4, "1");
+ }
+
+ void initSet(Set<Integer> s) {
+ s.addAll(Arrays.asList(1, 2, 3, 4));
+ }
+
void assertCharacteristics(Collection<?> c, int expectedCharacteristics) {
assertCharacteristics(c.spliterator(), expectedCharacteristics);
}