harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jessewil...@apache.org
Subject 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/
Date Wed, 09 Dec 2009 01:01:04 GMT
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() {
     }
 



Mime
View raw message