You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by je...@apache.org on 2009/12/09 02:01:04 UTC
svn commit: r888668 - in /harmony/enhanced/classlib/trunk/modules/luni/src:
main/java/java/util/ test/api/common/org/apache/harmony/luni/tests/java/util/
Author: jessewilson
Date: Wed Dec 9 01:01:03 2009
New Revision: 888668
URL: http://svn.apache.org/viewvc?rev=888668&view=rev
Log:
Fix for HARMONY-6396; Null handling bugs in collections.
Modified:
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java
harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractMap.java Wed Dec 9 01:01:03 2009
@@ -133,19 +133,22 @@
return false;
}
- Iterator<Map.Entry<K, V>> it = entrySet().iterator();
-
try {
- while (it.hasNext()) {
- Entry<K, V> entry = it.next();
+ for (Entry<K, V> entry : entrySet()) {
K key = entry.getKey();
- V value = entry.getValue();
- Object obj = map.get(key);
- if( null != obj && (!obj.equals(value)) || null == obj && obj != value) {
+ V mine = entry.getValue();
+ Object theirs = map.get(key);
+ if (mine == null) {
+ if (theirs != null || !map.containsKey(key)) {
+ return false;
+ }
+ } else if (!mine.equals(theirs)) {
return false;
}
}
- } catch (ClassCastException cce) {
+ } catch (NullPointerException ignored) {
+ return false;
+ } catch (ClassCastException ignored) {
return false;
}
return true;
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/AbstractSet.java Wed Dec 9 01:01:03 2009
@@ -55,7 +55,9 @@
try {
return size() == s.size() && containsAll(s);
- } catch (ClassCastException cce) {
+ } catch (NullPointerException ignored) {
+ return false;
+ } catch (ClassCastException ignored) {
return false;
}
}
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/Collections.java Wed Dec 9 01:01:03 2009
@@ -19,6 +19,7 @@
import java.io.IOException;
import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
import java.io.Serializable;
import java.lang.reflect.Array;
@@ -188,8 +189,15 @@
@SuppressWarnings("unchecked")
public static final Map EMPTY_MAP = new EmptyMap();
+ /**
+ * This class is a singleton so that equals() and hashCode() work properly.
+ */
private static final class ReverseComparator<T> implements Comparator<T>,
Serializable {
+
+ private static final ReverseComparator<Object> INSTANCE
+ = new ReverseComparator<Object>();
+
private static final long serialVersionUID = 7207038068494060240L;
@SuppressWarnings("unchecked")
@@ -197,6 +205,10 @@
Comparable<T> c2 = (Comparable<T>) o2;
return c2.compareTo(o1);
}
+
+ private Object readResolve() throws ObjectStreamException {
+ return INSTANCE;
+ }
}
private static final class ReverseComparatorWithComparator<T> implements
@@ -213,6 +225,18 @@
public int compare(T o1, T o2) {
return comparator.compare(o2, o1);
}
+
+ @Override
+ public boolean equals(Object o) {
+ return o instanceof ReverseComparatorWithComparator
+ && ((ReverseComparatorWithComparator) o).comparator
+ .equals(comparator);
+ }
+
+ @Override
+ public int hashCode() {
+ return ~comparator.hashCode();
+ }
}
private static final class SingletonSet<E> extends AbstractSet<E> implements
@@ -352,34 +376,17 @@
}
public Map.Entry<K, V> next() {
- if (hasNext) {
- hasNext = false;
- return new Map.Entry<K, V>() {
- @Override
- public boolean equals(Object object) {
- return contains(object);
- }
-
- public K getKey() {
- return k;
- }
-
- public V getValue() {
- return v;
- }
-
- @Override
- public int hashCode() {
- return (k == null ? 0 : k.hashCode())
- ^ (v == null ? 0 : v.hashCode());
- }
-
- public V setValue(V value) {
- throw new UnsupportedOperationException();
- }
- };
+ if (!hasNext) {
+ throw new NoSuchElementException();
}
- throw new NoSuchElementException();
+
+ hasNext = false;
+ return new MapEntry<K, V>(k, v) {
+ @Override
+ public V setValue(V value) {
+ throw new UnsupportedOperationException();
+ }
+ };
}
public void remove() {
@@ -1684,6 +1691,12 @@
*/
public static <T> T max(Collection<? extends T> collection,
Comparator<? super T> comparator) {
+ if (comparator == null) {
+ @SuppressWarnings("unchecked") // null comparator? T is comparable
+ T result = (T) max((Collection<Comparable>) collection);
+ return result;
+ }
+
Iterator<? extends T> it = collection.iterator();
T max = it.next();
while (it.hasNext()) {
@@ -1734,6 +1747,12 @@
*/
public static <T> T min(Collection<? extends T> collection,
Comparator<? super T> comparator) {
+ if (comparator == null) {
+ @SuppressWarnings("unchecked") // null comparator? T is comparable
+ T result = (T) min((Collection<Comparable>) collection);
+ return result;
+ }
+
Iterator<? extends T> it = collection.iterator();
T min = it.next();
while (it.hasNext()) {
@@ -1793,8 +1812,9 @@
* @see Comparable
* @see Serializable
*/
+ @SuppressWarnings("unchecked")
public static <T> Comparator<T> reverseOrder() {
- return new ReverseComparator<T>();
+ return (Comparator) ReverseComparator.INSTANCE;
}
/**
@@ -1815,6 +1835,9 @@
if (c == null) {
return reverseOrder();
}
+ if (c instanceof ReverseComparatorWithComparator) {
+ return ((ReverseComparatorWithComparator<T>) c).comparator;
+ }
return new ReverseComparatorWithComparator<T>(c);
}
@@ -2660,8 +2683,8 @@
* class of object that should be
* @return specified object
*/
- static <E> E checkType(E obj, Class<E> type) {
- if (!type.isInstance(obj)) {
+ static <E> E checkType(E obj, Class<? extends E> type) {
+ if (obj != null && !type.isInstance(obj)) {
// luni.05=Attempt to insert {0} element into collection with
// element type {1}
throw new ClassCastException(Messages.getString(
@@ -2769,20 +2792,11 @@
*/
@SuppressWarnings("unchecked")
public boolean addAll(Collection<? extends E> c1) {
- int size = c1.size();
- if (size == 0) {
- return false;
+ Object[] array = c1.toArray();
+ for (Object o : array) {
+ checkType(o, type);
}
- E[] arr = (E[]) new Object[size];
- Iterator<? extends E> it = c1.iterator();
- for (int i = 0; i < size; i++) {
- arr[i] = checkType(it.next(), type);
- }
- boolean added = false;
- for (int i = 0; i < size; i++) {
- added |= c.add(arr[i]);
- }
- return added;
+ return c.addAll((List<E>) Arrays.asList(array));
}
/**
@@ -2928,16 +2942,11 @@
*/
@SuppressWarnings("unchecked")
public boolean addAll(int index, Collection<? extends E> c1) {
- int size = c1.size();
- if (size == 0) {
- return false;
- }
- E[] arr = (E[]) new Object[size];
- Iterator<? extends E> it = c1.iterator();
- for (int i = 0; i < size; i++) {
- arr[i] = checkType(it.next(), type);
+ Object[] array = c1.toArray();
+ for (Object o : array) {
+ checkType(o, type);
}
- return l.addAll(index, Arrays.asList(arr));
+ return l.addAll(index, (List<E>) Arrays.asList(array));
}
/**
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeMap.java Wed Dec 9 01:01:03 2009
@@ -160,6 +160,9 @@
@SuppressWarnings("unchecked")
private static <T> Comparable<T> toComparable(T obj) {
+ if (obj == null) {
+ throw new NullPointerException();
+ }
return (Comparable) obj;
}
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/TreeSet.java Wed Dec 9 01:01:03 2009
@@ -35,9 +35,10 @@
private static final long serialVersionUID = -2479143000061671589L;
- private transient SortedMap<E, E> backingMap;
+ /** Keys are this set's elements. Values are always Boolean.TRUE */
+ private transient SortedMap<E, Object> backingMap;
- private TreeSet(SortedMap<E, E> map) {
+ private TreeSet(SortedMap<E, Object> map) {
this.backingMap = map;
}
@@ -46,7 +47,7 @@
* ordering.
*/
public TreeSet() {
- backingMap = new TreeMap<E, E>();
+ backingMap = new TreeMap<E, Object>();
}
/**
@@ -73,7 +74,7 @@
* the comparator to use.
*/
public TreeSet(Comparator<? super E> comparator) {
- backingMap = new TreeMap<E, E>(comparator);
+ backingMap = new TreeMap<E, Object>(comparator);
}
/**
@@ -107,7 +108,7 @@
*/
@Override
public boolean add(E object) {
- return backingMap.put(object, object) == null;
+ return backingMap.put(object, Boolean.TRUE) == null;
}
/**
@@ -153,10 +154,10 @@
try {
TreeSet<E> clone = (TreeSet<E>) super.clone();
if (backingMap instanceof TreeMap) {
- clone.backingMap = (SortedMap<E, E>) ((TreeMap<E, E>) backingMap)
+ clone.backingMap = (SortedMap<E, Object>) ((TreeMap<E, Object>) backingMap)
.clone();
} else {
- clone.backingMap = new TreeMap<E, E>(backingMap);
+ clone.backingMap = new TreeMap<E, Object>(backingMap);
}
return clone;
} catch (CloneNotSupportedException e) {
@@ -373,14 +374,14 @@
private void readObject(ObjectInputStream stream) throws IOException,
ClassNotFoundException {
stream.defaultReadObject();
- TreeMap<E, E> map = new TreeMap<E, E>((Comparator<? super E>) stream
- .readObject());
+ TreeMap<E, Object> map = new TreeMap<E, Object>(
+ (Comparator<? super E>) stream.readObject());
int size = stream.readInt();
if (size > 0) {
- TreeMap.Node<E,E> lastNode = null;
+ TreeMap.Node<E, Object> lastNode = null;
for(int i=0; i<size; i++) {
E elem = (E)stream.readObject();
- lastNode = map.addToLast(lastNode,elem,elem);
+ lastNode = map.addToLast(lastNode,elem, Boolean.TRUE);
}
}
backingMap = map;
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java?rev=888668&r1=888667&r2=888668&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/AbstractMapTest.java Wed Dec 9 01:01:03 2009
@@ -19,6 +19,7 @@
import java.util.AbstractMap;
import java.util.AbstractSet;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
@@ -355,6 +356,57 @@
assertEquals("Should be equal", amt, ht);
}
+ public void testEqualsWithNullValues() {
+ Map<String, String> a = new HashMap<String, String>();
+ a.put("a", null);
+ a.put("b", null);
+
+ Map<String, String> b = new HashMap<String, String>();
+ a.put("c", "cat");
+ a.put("d", "dog");
+
+ assertFalse(a.equals(b));
+ assertFalse(b.equals(a));
+ }
+
+ public void testNullsOnViews() {
+ Map<String, String> nullHostile = new Hashtable<String, String>();
+
+ nullHostile.put("a", "apple");
+ testNullsOnView(nullHostile.entrySet());
+
+ nullHostile.put("a", "apple");
+ testNullsOnView(nullHostile.keySet());
+
+ nullHostile.put("a", "apple");
+ testNullsOnView(nullHostile.values());
+ }
+
+ private void testNullsOnView(Collection<?> view) {
+ try {
+ assertFalse(view.contains(null));
+ } catch (NullPointerException optional) {
+ }
+
+ try {
+ assertFalse(view.remove(null));
+ } catch (NullPointerException optional) {
+ }
+
+ Set<Object> setOfNull = Collections.singleton(null);
+ assertFalse(view.equals(setOfNull));
+
+ try {
+ assertFalse(view.removeAll(setOfNull));
+ } catch (NullPointerException optional) {
+ }
+
+ try {
+ assertTrue(view.retainAll(setOfNull)); // destructive
+ } catch (NullPointerException optional) {
+ }
+ }
+
protected void setUp() {
}