8023275: Wrapping collections should override default methods
Reviewed-by: mduigou, psandoz
--- a/jdk/src/share/classes/java/util/Collections.java Tue Aug 27 11:46:25 2013 -0700
+++ b/jdk/src/share/classes/java/util/Collections.java Mon Aug 26 22:32:50 2013 -0700
@@ -27,7 +27,6 @@
import java.io.Serializable;
import java.io.ObjectOutputStream;
import java.io.IOException;
-import java.io.InvalidObjectException;
import java.lang.reflect.Array;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
@@ -35,6 +34,7 @@
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.UnaryOperator;
+import java.util.stream.IntStream;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
@@ -1148,7 +1148,16 @@
public Spliterator<E> spliterator() {
return (Spliterator<E>)c.spliterator();
}
-
+ @SuppressWarnings("unchecked")
+ @Override
+ public Stream<E> stream() {
+ return (Stream<E>)c.stream();
+ }
+ @SuppressWarnings("unchecked")
+ @Override
+ public Stream<E> parallelStream() {
+ return (Stream<E>)c.parallelStream();
+ }
}
/**
@@ -2009,8 +2018,8 @@
* through the returned collection.<p>
*
* It is imperative that the user manually synchronize on the returned
- * collection when traversing it via {@link Iterator} or
- * {@link Spliterator}:
+ * collection when traversing it via {@link Iterator}, {@link Spliterator}
+ * or {@link Stream}:
* <pre>
* Collection c = Collections.synchronizedCollection(myCollection);
* ...
@@ -2120,6 +2129,14 @@
public Spliterator<E> spliterator() {
return c.spliterator(); // Must be manually synched by user!
}
+ @Override
+ public Stream<E> stream() {
+ return c.stream(); // Must be manually synched by user!
+ }
+ @Override
+ public Stream<E> parallelStream() {
+ return c.parallelStream(); // Must be manually synched by user!
+ }
private void writeObject(ObjectOutputStream s) throws IOException {
synchronized (mutex) {s.defaultWriteObject();}
}
@@ -3172,6 +3189,10 @@
}
@Override
public Spliterator<E> spliterator() {return c.spliterator();}
+ @Override
+ public Stream<E> stream() {return c.stream();}
+ @Override
+ public Stream<E> parallelStream() {return c.parallelStream();}
}
/**
@@ -5096,6 +5117,22 @@
") > toIndex(" + toIndex + ")");
return new CopiesList<>(toIndex - fromIndex, element);
}
+
+ // Override default methods in Collection
+ @Override
+ public Stream<E> stream() {
+ return IntStream.range(0, n).mapToObj(i -> element);
+ }
+
+ @Override
+ public Stream<E> parallelStream() {
+ return IntStream.range(0, n).parallel().mapToObj(i -> element);
+ }
+
+ @Override
+ public Spliterator<E> spliterator() {
+ return stream().spliterator();
+ }
}
/**
@@ -5503,6 +5540,10 @@
@Override
public Spliterator<E> spliterator() {return s.spliterator();}
+ @Override
+ public Stream<E> stream() {return s.stream();}
+ @Override
+ public Stream<E> parallelStream() {return s.parallelStream();}
private static final long serialVersionUID = 2454657854757543876L;
@@ -5568,10 +5609,14 @@
@Override
public void forEach(Consumer<? super E> action) {q.forEach(action);}
@Override
- public Spliterator<E> spliterator() {return q.spliterator();}
- @Override
public boolean removeIf(Predicate<? super E> filter) {
return q.removeIf(filter);
}
+ @Override
+ public Spliterator<E> spliterator() {return q.spliterator();}
+ @Override
+ public Stream<E> stream() {return q.stream();}
+ @Override
+ public Stream<E> parallelStream() {return q.parallelStream();}
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/Collections/Wrappers.java Mon Aug 26 22:32:50 2013 -0700
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @run testng Wrappers
+ * @summary Ensure Collections wrapping classes provide non-default implementations
+ */
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+import org.testng.annotations.Test;
+import org.testng.annotations.DataProvider;
+
+import static org.testng.Assert.assertFalse;
+
+@Test(groups = "unit")
+public class Wrappers {
+ static Object[][] collections;
+
+ @DataProvider(name="collections")
+ public static Object[][] collectionCases() {
+ if (collections != null) {
+ return collections;
+ }
+
+ List<Object[]> cases = new ArrayList<>();
+ LinkedList<Integer> seedList = new LinkedList<>();
+ ArrayList<Integer> seedRandomAccess = new ArrayList<>();
+ TreeSet<Integer> seedSet = new TreeSet<>();
+ TreeMap<Integer, Integer> seedMap = new TreeMap<>();
+
+ for (int i = 1; i <= 10; i++) {
+ seedList.add(i);
+ seedRandomAccess.add(i);
+ seedSet.add(i);
+ seedMap.put(i, i);
+ }
+
+ cases.add(new Object[] { Collections.unmodifiableCollection(seedList) });
+ cases.add(new Object[] { Collections.unmodifiableList(seedList) });
+ cases.add(new Object[] { Collections.unmodifiableList(seedRandomAccess) });
+ cases.add(new Object[] { Collections.unmodifiableSet(seedSet) });
+ cases.add(new Object[] { Collections.unmodifiableSortedSet(seedSet) });
+ cases.add(new Object[] { Collections.unmodifiableNavigableSet(seedSet) });
+
+ // As sets from map also need to be unmodifiable, thus a wrapping
+ // layer exist and should not have default methods
+ cases.add(new Object[] { Collections.unmodifiableMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.unmodifiableMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.unmodifiableMap(seedMap).values() });
+ cases.add(new Object[] { Collections.unmodifiableSortedMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.unmodifiableSortedMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.unmodifiableSortedMap(seedMap).values() });
+ cases.add(new Object[] { Collections.unmodifiableNavigableMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.unmodifiableNavigableMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.unmodifiableNavigableMap(seedMap).values() });
+
+ // Synchronized
+ cases.add(new Object[] { Collections.synchronizedCollection(seedList) });
+ cases.add(new Object[] { Collections.synchronizedList(seedList) });
+ cases.add(new Object[] { Collections.synchronizedList(seedRandomAccess) });
+ cases.add(new Object[] { Collections.synchronizedSet(seedSet) });
+ cases.add(new Object[] { Collections.synchronizedSortedSet(seedSet) });
+ cases.add(new Object[] { Collections.synchronizedNavigableSet(seedSet) });
+
+ // As sets from map also need to be synchronized on the map, thus a
+ // wrapping layer exist and should not have default methods
+ cases.add(new Object[] { Collections.synchronizedMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.synchronizedMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.synchronizedMap(seedMap).values() });
+ cases.add(new Object[] { Collections.synchronizedSortedMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.synchronizedSortedMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.synchronizedSortedMap(seedMap).values() });
+ cases.add(new Object[] { Collections.synchronizedNavigableMap(seedMap).entrySet() });
+ cases.add(new Object[] { Collections.synchronizedNavigableMap(seedMap).keySet() });
+ cases.add(new Object[] { Collections.synchronizedNavigableMap(seedMap).values() });
+
+ // Checked
+ cases.add(new Object[] { Collections.checkedCollection(seedList, Integer.class) });
+ cases.add(new Object[] { Collections.checkedList(seedList, Integer.class) });
+ cases.add(new Object[] { Collections.checkedList(seedRandomAccess, Integer.class) });
+ cases.add(new Object[] { Collections.checkedSet(seedSet, Integer.class) });
+ cases.add(new Object[] { Collections.checkedSortedSet(seedSet, Integer.class) });
+ cases.add(new Object[] { Collections.checkedNavigableSet(seedSet, Integer.class) });
+ cases.add(new Object[] { Collections.checkedQueue(seedList, Integer.class) });
+
+ // asLifoQueue is another wrapper
+ cases.add(new Object[] { Collections.asLifoQueue(seedList) });
+
+ collections = cases.toArray(new Object[0][]);
+ return collections;
+ }
+
+ static Method[] defaultMethods;
+
+ static {
+ List<Method> list = new ArrayList<>();
+ Method[] methods = Collection.class.getMethods();
+ for (Method m: methods) {
+ if (m.isDefault()) {
+ list.add(m);
+ }
+ }
+ defaultMethods = list.toArray(new Method[0]);
+ }
+
+ @Test(dataProvider = "collections")
+ public static void testAllDefaultMethodsOverridden(Collection c) throws NoSuchMethodException {
+ Class cls = c.getClass();
+ for (Method m: defaultMethods) {
+ Method m2 = cls.getMethod(m.getName(), m.getParameterTypes());
+ // default had been override
+ assertFalse(m2.isDefault(), cls.getCanonicalName());
+ }
+ }
+}
+