Fixes issue #1456; map entries now correctly write-through to the underlying map, and also reflected changes in the underlying map. This fixes HashMap and IdentityHashMap. Also adds tests for this behavior. Found by: nlwillia Review by: tobyr git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2475 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java b/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java index f775124..b54bc1b 100644 --- a/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java +++ b/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java
@@ -93,11 +93,10 @@ public EntrySetIterator() { List<Map.Entry<K, V>> list = new ArrayList<Map.Entry<K, V>>(); if (nullSlotLive) { - MapEntryImpl<K, V> entryImpl = new MapEntryImpl<K, V>(null, nullSlot); - list.add(entryImpl); + list.add(new MapEntryNull()); } - addAllStringEntries(stringMap, list); - addAllHashEntries(hashCodeMap, list); + addAllStringEntries(list); + addAllHashEntries(list); this.iter = list.iterator(); } @@ -120,34 +119,50 @@ } } - private static native void addAllHashEntries(JavaScriptObject hashCodeMap, - Collection<?> dest) /*-{ - for (var hashCode in hashCodeMap) { - // sanity check that it's really an integer - if (hashCode == parseInt(hashCode)) { - var array = hashCodeMap[hashCode]; - for (var i = 0, c = array.length; i < c; ++i) { - dest.@java.util.Collection::add(Ljava/lang/Object;)(array[i]); - } - } - } - }-*/; + private final class MapEntryNull extends AbstractMapEntry<K, V> { - private static native void addAllStringEntries(JavaScriptObject stringMap, - Collection<?> dest) /*-{ - for (var key in stringMap) { - // only keys that start with a colon ':' count - if (key.charCodeAt(0) == 58) { - var value = stringMap[key]; - var entry = @java.util.MapEntryImpl::create(Ljava/lang/Object;Ljava/lang/Object;)(key.substring(1), value); - dest.@java.util.Collection::add(Ljava/lang/Object;)(entry); - } + public K getKey() { + return null; } - }-*/; + + public V getValue() { + return nullSlot; + } + + public V setValue(V object) { + return putNullSlot(object); + } + } + + // Instantiated from JSNI + @SuppressWarnings("unused") + private final class MapEntryString extends AbstractMapEntry<K, V> { + + private final String key; + + public MapEntryString(String key) { + this.key = key; + } + + @SuppressWarnings("unchecked") + public K getKey() { + return (K) key; + } + + public V getValue() { + return getStringValue(key); + } + + public V setValue(V object) { + return putStringValue(key, object); + } + } /** * A map of integral hashCodes onto entries. */ + // Used from JSNI. + @SuppressWarnings("unused") private transient JavaScriptObject hashCodeMap; /** @@ -162,6 +177,8 @@ /** * A map of Strings onto values. */ + // Used from JSNI. + @SuppressWarnings("unused") private transient JavaScriptObject stringMap; { @@ -197,8 +214,8 @@ @Override public boolean containsKey(Object key) { - return (key == null) ? nullSlotLive : (!(key instanceof String) ? hasHashValue( - key, getHashCode(key)) : hasStringValue((String) key)); + return (key == null) ? nullSlotLive : (!(key instanceof String) + ? hasHashValue(key, getHashCode(key)) : hasStringValue((String) key)); } @Override @@ -233,8 +250,9 @@ @Override public V remove(Object key) { - return (key == null) ? removeNullSlot() : (!(key instanceof String) ? removeHashValue( - key, getHashCode(key)) : removeStringValue((String) key)); + return (key == null) ? removeNullSlot() : (!(key instanceof String) + ? removeHashValue(key, getHashCode(key)) + : removeStringValue((String) key)); } @Override @@ -254,6 +272,30 @@ */ protected abstract int getHashCode(Object key); + private native void addAllHashEntries(Collection<?> dest) /*-{ + var hashCodeMap = this.@java.util.AbstractHashMap::hashCodeMap; + for (var hashCode in hashCodeMap) { + // sanity check that it's really an integer + if (hashCode == parseInt(hashCode)) { + var array = hashCodeMap[hashCode]; + for (var i = 0, c = array.length; i < c; ++i) { + dest.@java.util.Collection::add(Ljava/lang/Object;)(array[i]); + } + } + } + }-*/; + + private native void addAllStringEntries(Collection<?> dest) /*-{ + var stringMap = this.@java.util.AbstractHashMap::stringMap; + for (var key in stringMap) { + // only keys that start with a colon ':' count + if (key.charCodeAt(0) == 58) { + var entry = @java.util.AbstractHashMap$MapEntryString::new(Ljava/util/AbstractHashMap;Ljava/lang/String;)(this, key.substring(1)); + dest.@java.util.Collection::add(Ljava/lang/Object;)(entry); + } + } + }-*/; + private void clearImpl() { hashCodeMap = JavaScriptObject.createArray(); stringMap = JavaScriptObject.createObject(); @@ -339,7 +381,7 @@ private native V getStringValue(String key) /*-{ return (_ = this.@java.util.AbstractHashMap::stringMap[':' + key]) == null ? null : _ ; }-*/; - + /** * Returns true if the a key exists in the hashCodeMap that is Object equal to * <code>key</code>, provided that <code>key</code>'s hash code is @@ -365,7 +407,7 @@ private native boolean hasStringValue(String key) /*-{ return (':' + key) in this.@java.util.AbstractHashMap::stringMap; }-*/; - + /** * Sets the specified key to the specified value in the hashCodeMap. Returns * the value previously at that key. Returns <code>null</code> if the @@ -387,7 +429,7 @@ } else { array = this.@java.util.AbstractHashMap::hashCodeMap[hashCode] = []; } - var entry = @java.util.MapEntryImpl::create(Ljava/lang/Object;Ljava/lang/Object;)(key, value); + var entry = @java.util.MapEntryImpl::new(Ljava/lang/Object;Ljava/lang/Object;)(key, value); array.push(entry); ++this.@java.util.AbstractHashMap::size; return null; @@ -456,8 +498,8 @@ /** * Removes the specified key from the stringMap and returns the value that was - * previously there. Returns <code>null</code> if the specified key - * does not exist. + * previously there. Returns <code>null</code> if the specified key does not + * exist. */ private native V removeStringValue(String key) /*-{ key = ':' + key;
diff --git a/user/super/com/google/gwt/emul/java/util/AbstractMapEntry.java b/user/super/com/google/gwt/emul/java/util/AbstractMapEntry.java new file mode 100644 index 0000000..2739072 --- /dev/null +++ b/user/super/com/google/gwt/emul/java/util/AbstractMapEntry.java
@@ -0,0 +1,58 @@ +/* + * Copyright 2008 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package java.util; + +import static java.util.Utility.equalsWithNullCheck; + +/** + * Basic {@link Map.Entry} implementation that implements hashCode, equals, and + * toString. + */ +abstract class AbstractMapEntry<K, V> implements Map.Entry<K, V> { + + @Override + public final boolean equals(Object other) { + if (other instanceof Map.Entry) { + Map.Entry<?, ?> entry = (Map.Entry<?, ?>) other; + if (equalsWithNullCheck(getKey(), entry.getKey()) + && equalsWithNullCheck(getValue(), entry.getValue())) { + return true; + } + } + return false; + } + + /** + * Calculate the hash code using Sun's specified algorithm. + */ + @Override + public final int hashCode() { + int keyHash = 0; + int valueHash = 0; + if (getKey() != null) { + keyHash = getKey().hashCode(); + } + if (getValue() != null) { + valueHash = getValue().hashCode(); + } + return keyHash ^ valueHash; + } + + @Override + public final String toString() { + return getKey() + "=" + getValue(); + } +}
diff --git a/user/super/com/google/gwt/emul/java/util/MapEntryImpl.java b/user/super/com/google/gwt/emul/java/util/MapEntryImpl.java index 0fd5368..beac1d5 100644 --- a/user/super/com/google/gwt/emul/java/util/MapEntryImpl.java +++ b/user/super/com/google/gwt/emul/java/util/MapEntryImpl.java
@@ -1,5 +1,5 @@ /* - * Copyright 2007 Google Inc. + * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of @@ -15,19 +15,10 @@ */ package java.util; -import static java.util.Utility.equalsWithNullCheck; - /** * An {@link Map.Entry} shared by several {@link Map} implementations. */ -class MapEntryImpl<K, V> implements Map.Entry<K, V> { - - /** - * Helper method for constructing Map.Entry objects from JSNI code. - */ - static <K, V> Map.Entry<K, V> create(K key, V value) { - return new MapEntryImpl<K, V>(key, value); - } +class MapEntryImpl<K, V> extends AbstractMapEntry<K, V> { private K key; @@ -41,18 +32,6 @@ this.value = value; } - @Override - public boolean equals(Object other) { - if (other instanceof Map.Entry) { - Map.Entry<?, ?> entry = (Map.Entry<?, ?>) other; - if (equalsWithNullCheck(key, entry.getKey()) - && equalsWithNullCheck(value, entry.getValue())) { - return true; - } - } - return false; - } - public K getKey() { return key; } @@ -61,30 +40,9 @@ return value; } - /** - * Calculate the hash code using Sun's specified algorithm. - */ - @Override - public int hashCode() { - int keyHash = 0; - int valueHash = 0; - if (key != null) { - keyHash = key.hashCode(); - } - if (value != null) { - valueHash = value.hashCode(); - } - return keyHash ^ valueHash; - } - public V setValue(V object) { V old = value; value = object; return old; } - - @Override - public String toString() { - return getKey() + "=" + getValue(); - } }
diff --git a/user/test/com/google/gwt/emultest/java/util/HashMapTest.java b/user/test/com/google/gwt/emultest/java/util/HashMapTest.java index 9ba1efc..1a68faa 100644 --- a/user/test/com/google/gwt/emultest/java/util/HashMapTest.java +++ b/user/test/com/google/gwt/emultest/java/util/HashMapTest.java
@@ -1,5 +1,5 @@ /* - * Copyright 2006 Google Inc. + * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of @@ -114,12 +114,6 @@ return "com.google.gwt.emultest.EmulSuite"; } - public void testAddWatch() { - HashMap m = new HashMap(); - m.put("watch", "watch"); - assertEquals(m.get("watch"), "watch"); - } - public void testAddEqualKeys() { final HashMap expected = new HashMap(); assertEquals(expected.size(), 0); @@ -133,6 +127,12 @@ iterateThrough(expected); } + public void testAddWatch() { + HashMap m = new HashMap(); + m.put("watch", "watch"); + assertEquals(m.get("watch"), "watch"); + } + /* * Test method for 'java.util.HashMap.clear()' */ @@ -248,6 +248,60 @@ } /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterNonString() { + HashMap hashMap = new HashMap(); + hashMap.put(1, 2); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue(3); + assertEquals(3, hashMap.get(1)); + + hashMap.put(1, 4); + assertEquals(4, entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterNull() { + HashMap hashMap = new HashMap(); + hashMap.put(null, 2); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue(3); + assertEquals(3, hashMap.get(null)); + + hashMap.put(null, 4); + assertEquals(4, entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterString() { + HashMap hashMap = new HashMap(); + hashMap.put("A", "B"); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue("C"); + assertEquals("C", hashMap.get("A")); + + hashMap.put("A", "D"); + assertEquals("D", entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* * Used to test the entrySet remove method. */ public void testEntrySetRemove() {
diff --git a/user/test/com/google/gwt/emultest/java/util/IdentityHashMapTest.java b/user/test/com/google/gwt/emultest/java/util/IdentityHashMapTest.java index 6bddf46..2b4783e 100644 --- a/user/test/com/google/gwt/emultest/java/util/IdentityHashMapTest.java +++ b/user/test/com/google/gwt/emultest/java/util/IdentityHashMapTest.java
@@ -1,5 +1,5 @@ /* - * Copyright 2006 Google Inc. + * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of @@ -20,6 +20,7 @@ import org.apache.commons.collections.TestMap; import java.util.Collection; +import java.util.HashMap; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.Map; @@ -133,12 +134,6 @@ return "com.google.gwt.emultest.EmulSuite"; } - public void testAddWatch() { - IdentityHashMap m = new IdentityHashMap(); - m.put("watch", "watch"); - assertEquals(m.get("watch"), "watch"); - } - public void testAddEqualKeys() { final IdentityHashMap expected = new IdentityHashMap(); assertEquals(expected.size(), 0); @@ -152,6 +147,12 @@ iterateThrough(expected); } + public void testAddWatch() { + IdentityHashMap m = new IdentityHashMap(); + m.put("watch", "watch"); + assertEquals(m.get("watch"), "watch"); + } + /* * Test method for 'java.util.IdentityHashMap.clear()' */ @@ -269,6 +270,62 @@ } /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterNonString() { + HashMap hashMap = new HashMap(); + Integer key = 1; + hashMap.put(key, 2); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue(3); + assertEquals(3, hashMap.get(key)); + + hashMap.put(key, 4); + assertEquals(4, entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterNull() { + HashMap hashMap = new HashMap(); + hashMap.put(null, 2); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue(3); + assertEquals(3, hashMap.get(null)); + + hashMap.put(null, 4); + assertEquals(4, entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* + * Used to test the entrySet entry's set method. + */ + public void testEntrySetEntrySetterString() { + HashMap hashMap = new HashMap(); + String key = "A"; + hashMap.put(key, "B"); + Set entrySet = hashMap.entrySet(); + Entry entry = (Entry) entrySet.iterator().next(); + + entry.setValue("C"); + assertEquals("C", hashMap.get(key)); + + hashMap.put(key, "D"); + assertEquals("D", entry.getValue()); + + assertEquals(1, hashMap.size()); + } + + /* * Used to test the entrySet remove method. */ public void testEntrySetRemove() { @@ -681,11 +738,11 @@ assertEquals(val, VALUE_VAL); } - protected Map makeEmptyMap() { + protected Map makeConfirmedMap() { return new IdentityHashMap(); } - protected Map makeConfirmedMap() { + protected Map makeEmptyMap() { return new IdentityHashMap(); }