Improvify ConstantMap!

1) It's very lightweight now.

2) It's small in code size.

3) It's not serializable anymore! (It used to extend HashMap.)

4) New and improved tests.

5) Empty maps are allowed.

Patch by: amitmanjhi
Suggested by: scottb
Review by: scottb


git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@3915 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/i18n/client/impl/ConstantMap.java b/user/src/com/google/gwt/i18n/client/impl/ConstantMap.java
index 9a7779b..0ecec17 100644
--- a/user/src/com/google/gwt/i18n/client/impl/ConstantMap.java
+++ b/user/src/com/google/gwt/i18n/client/impl/ConstantMap.java
@@ -15,137 +15,112 @@
  */
 package com.google.gwt.i18n.client.impl;
 
-import java.util.ArrayList;
-import java.util.Collection;
+import com.google.gwt.core.client.JavaScriptObject;
+
+import java.util.AbstractMap;
+import java.util.AbstractSet;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
 /**
- * Read only Map used when returning <code>Constants</code> maps. Preserves
- * order. ConstantMap should only be created or modified by GWT, as constant
- * maps are constructed using a very stereotyped algorithm, which allows
- * <code>ConstantMap</code> to maintain order with very little code. In
- * specific, no elements are every removed from them and all elements are added
- * before the first user operation.
+ * Map used when creating <code>Constants</code> maps. This class is to be
+ * used only by the GWT code. The map is immediately wrapped in
+ * Collections.unmodifiableMap(..) preventing any changes after construction.
  */
-public class ConstantMap extends HashMap<String, String> {
+public class ConstantMap extends AbstractMap<String, String> {
 
-  private static class DummyMapEntry implements Map.Entry<String, String> {
-    private final String key;
+  /**
+   * A cache of a synthesized entry set.
+   */
+  private Set<Map.Entry<String, String>> entries;
 
-    private final String value;
+  /**
+   * The original set of keys.
+   */
+  private final String[] keys;
 
-    DummyMapEntry(String key, String value) {
-      this.key = key;
-      this.value = value;
-    }
+  /*
+   * Stores a fast lookup in a JSO using ':' to prevent conflict with built-in
+   * JavaScript properties.
+   */
+  @SuppressWarnings("unused")
+  private JavaScriptObject map;
 
-    public String getKey() {
-      return key;
-    }
+  public ConstantMap(String keys[], String values[]) {
+    this.keys = keys;
 
-    public String getValue() {
-      return value;
-    }
+    init();
 
-    public String setValue(String arg0) {
-      throw new UnsupportedOperationException();
+    for (int i = 0; i < keys.length; ++i) {
+      putImpl(keys[i], values[i]);
     }
   }
 
-  private class OrderedConstantSet<T> extends ArrayList<T> implements Set<T> {
-    private class ImmutableIterator implements Iterator<T> {
-      private final Iterator<T> base;
-
-      ImmutableIterator(Iterator<T> base) {
-        this.base = base;
-      }
-
-      public boolean hasNext() {
-        return base.hasNext();
-      }
-
-      public T next() {
-        return base.next();
-      }
-
-      public void remove() {
-        throw new UnsupportedOperationException("Immutable set");
-      }
-    }
-
-    @Override
-    public void clear() {
-      throw new UnsupportedOperationException("Immutable set");
-    }
-
-    @Override
-    public Iterator<T> iterator() {
-      Iterator<T> base = super.iterator();
-      return new ImmutableIterator(base);
-    }
-  }
-
-  private OrderedConstantSet<Map.Entry<String, String>> entries;
-
-  private final OrderedConstantSet<String> keys = new OrderedConstantSet<String>();
-
-  private OrderedConstantSet<String> values;
-
   @Override
-  public void clear() {
-    throw unsupported("clear");
+  public boolean containsKey(Object key) {
+    return get(key) != null;
   }
 
   @Override
   public Set<Map.Entry<String, String>> entrySet() {
     if (entries == null) {
-      entries = new OrderedConstantSet<Map.Entry<String, String>>();
-      for (int i = 0; i < keys.size(); i++) {
-        String key = keys.get(i);
-        String value = get(key);
-        entries.add(new DummyMapEntry(key, value));
+      Map<String, String> copy = new HashMap<String, String>();
+      for (String key : keys) {
+        copy.put(key, get(key));
       }
+      entries = Collections.unmodifiableMap(copy).entrySet();
     }
     return entries;
   }
 
   @Override
+  public String get(Object key) {
+    return (key instanceof String) ? get((String) key) : null;
+  }
+
+  public native String get(String key) /*-{
+    // Prepend ':' to avoid conflicts with built-in Object properties.
+    return this.@com.google.gwt.i18n.client.impl.ConstantMap::map[':' + key];
+  }-*/;
+
+  @Override
   public Set<String> keySet() {
-    return keys;
-  }
-
-  @Override
-  public String put(String key, String value) {
-    // We may want to find a more efficient implementation later.
-    boolean exists = keys.contains(key);
-    if (!exists) {
-      keys.add(key);
-    }
-    return super.put(key, value);
-  }
-
-  @Override
-  public String remove(Object key) {
-    throw unsupported("remove");
-  }
-
-  @Override
-  public Collection<String> values() {
-    if (values == null) {
-      values = new OrderedConstantSet<String>();
-      for (int i = 0; i < keys.size(); i++) {
-        Object element = keys.get(i);
-        values.add(this.get(element));
+    return new AbstractSet<String>() {
+      @Override
+      public boolean contains(Object o) {
+        return containsKey(o);
       }
-    }
-    return values;
+
+      @Override
+      public Iterator<String> iterator() {
+        return Arrays.asList(keys).iterator();
+      }
+
+      @Override
+      public int size() {
+        return ConstantMap.this.size();
+      }
+    };
   }
 
-  private UnsupportedOperationException unsupported(String operation) {
-    return new UnsupportedOperationException(operation
-        + " not supported on a constant map");
+  @Override
+  public int size() {
+    return keys.length;
   }
+
+  /**
+   * Overridable for testing purposes, see ConstantMapTest.
+   */
+  protected void init() {
+    map = JavaScriptObject.createObject();
+  }
+
+  protected native void putImpl(String key, String value) /*-{
+    // Prepend ':' to avoid conflicts with built-in Object properties.
+    this.@com.google.gwt.i18n.client.impl.ConstantMap::map[':' + key] = value;
+  }-*/;
 }
diff --git a/user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java b/user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
index e18625c..a192966 100644
--- a/user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
+++ b/user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
@@ -22,10 +22,16 @@
 import com.google.gwt.i18n.rebind.AbstractResource.ResourceList;
 import com.google.gwt.user.rebind.AbstractGeneratorClassCreator;
 
+import java.util.SortedMap;
+import java.util.TreeMap;
+
 /**
  * Creator for methods of the form Map getX() .
  */
 class ConstantsMapMethodCreator extends AbstractLocalizableMethodCreator {
+
+  static final String GENERIC_STRING_MAP_TYPE = "java.util.Map<java.lang.String, java.lang.String>";
+
   /**
    * Constructor for localizable returnType method creator.
    * 
@@ -40,13 +46,13 @@
    * 
    * @param logger TreeLogger instance for logging
    * @param method method body to create
-   * @param key value to create map from
+   * @param mapName name to create map from
    * @param resourceList AbstractResource for key lookup
    * @param locale locale to use for localized string lookup
    */
   @Override
-  public void createMethodFor(TreeLogger logger, JMethod method, String key,
-      ResourceList resourceList, String locale) {
+  public void createMethodFor(TreeLogger logger, JMethod method,
+      String mapName, ResourceList resourceList, String locale) {
     String methodName = method.getName();
     if (method.getParameters().length > 0) {
       error(
@@ -59,31 +65,53 @@
     enableCache();
     // check cache for array
     String constantMapClassName = ConstantMap.class.getCanonicalName();
-    println(constantMapClassName + " args = (" + constantMapClassName + ") cache.get("
-        + wrap(methodName) + ");");
+    println(GENERIC_STRING_MAP_TYPE + " args = (" + GENERIC_STRING_MAP_TYPE
+        + ") cache.get(" + wrap(methodName) + ");");
     // if not found create Map
     println("if (args == null) {");
     indent();
-    println("args = new " + constantMapClassName + "();");
-    String value;
+    println("args = new " + constantMapClassName + "(new String[] {");
+    String keyString;
     try {
-      value = resourceList.getRequiredStringExt(key, null);
+      keyString = resourceList.getRequiredStringExt(mapName, null);
     } catch (MissingResourceException e) {
       e.setDuring("getting key list");
       throw e;
     }
-    String[] args = ConstantsStringArrayMethodCreator.split(value);
 
-    for (int i = 0; i < args.length; i++) {
+    String[] keys = ConstantsStringArrayMethodCreator.split(keyString);
+    ResourceList resources = getResources();
+    SortedMap<String, String> map = new TreeMap<String, String>();
+    for (String key : keys) {
+      if (key.length() == 0) {
+        continue;
+      }
+
       try {
-        key = args[i];
-        String keyValue = getResources().getString(key);
-        println("args.put(" + wrap(key) + ", " + wrap(keyValue) + ");");
+        String value = resources.getRequiredString(key);
+        map.put(key, value);
       } catch (MissingResourceException e) {
         e.setDuring("implementing map");
         throw e;
       }
     }
+
+    indent();
+    indent();
+    for (String key : map.keySet()) {
+      println(wrap(key) + ", ");
+    }
+    outdent();
+    println("},");
+    indent();
+    println("new String[] {");
+    for (String key : map.keySet()) {
+      String value = map.get(key);
+      println(wrap(value) + ",");
+    }
+    outdent();
+    println("});");
+    outdent();
     println("cache.put(" + wrap(methodName) + ", args);");
     outdent();
     println("};");
diff --git a/user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java b/user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
index 433f698..8a7272b 100644
--- a/user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
+++ b/user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
@@ -23,7 +23,6 @@
 import com.google.gwt.core.ext.typeinfo.JType;
 import com.google.gwt.core.ext.typeinfo.TypeOracle;
 import com.google.gwt.core.ext.typeinfo.TypeOracleException;
-import com.google.gwt.i18n.client.impl.ConstantMap;
 import com.google.gwt.i18n.rebind.AbstractResource.ResourceList;
 import com.google.gwt.user.rebind.AbstractMethodCreator;
 import com.google.gwt.user.rebind.SourceWriter;
@@ -125,7 +124,7 @@
       namesToMethodCreators.put("getMap", new LookupMethodCreator(this, mapType) {
         @Override
         public String getReturnTypeName() {
-          return ConstantMap.class.getCanonicalName();
+          return ConstantsMapMethodCreator.GENERIC_STRING_MAP_TYPE;
         }
       });
 
diff --git a/user/test/com/google/gwt/i18n/ConstantMapTest.java b/user/test/com/google/gwt/i18n/ConstantMapTest.java
index 95a0b7f..8503fc7 100644
--- a/user/test/com/google/gwt/i18n/ConstantMapTest.java
+++ b/user/test/com/google/gwt/i18n/ConstantMapTest.java
@@ -17,19 +17,64 @@
 
 import com.google.gwt.i18n.client.impl.ConstantMap;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 
 /**
- * TODO: document me.
+ * Test ConstantMap using Apache's tests.
  */
 public class ConstantMapTest extends MapTestBase {
 
-  protected Map makeEmptyMap() {
-    return new ConstantMap();
+  private static final class ConstantMapNoJsni extends ConstantMap {
+    private Map<String, String> noJsniImpl;
+
+    private ConstantMapNoJsni(String[] keys, String[] values) {
+      super(keys, values);
+    }
+
+    @Override
+    public String get(String key) {
+      return noJsniImpl.get(key);
+    }
+
+    @Override
+    protected void init() {
+      noJsniImpl = new HashMap<String, String>();
+    }
+
+    @Override
+    protected void putImpl(String key, String value) {
+      noJsniImpl.put(key, value);
+    }
   }
 
+  @Override
   protected boolean isRemoveModifiable() {
     return false;
   }
 
+  @Override
+  protected Map<String, String> makeEmptyMap() {
+    return Collections.unmodifiableMap(new ConstantMapNoJsni(new String[] {},
+        new String[] {}));
+  }
+
+  @Override
+  protected Map<String, String> makeFullMap() {
+    String[] keys = Arrays.asList(getSampleKeys()).toArray(new String[0]);
+    String[] values = Arrays.asList(getSampleValues()).toArray(new String[0]);
+    return Collections.unmodifiableMap(new ConstantMapNoJsni(keys, values));
+  }
+
+  @Override
+  protected boolean useNullKey() {
+    return false;
+  }
+
+  @Override
+  protected boolean useNullValue() {
+    return false;
+  }
 }
diff --git a/user/test/com/google/gwt/i18n/client/I18NTest.java b/user/test/com/google/gwt/i18n/client/I18NTest.java
index 5889825..394b090 100644
--- a/user/test/com/google/gwt/i18n/client/I18NTest.java
+++ b/user/test/com/google/gwt/i18n/client/I18NTest.java
@@ -20,7 +20,6 @@
 import com.google.gwt.i18n.client.gen.Colors;
 import com.google.gwt.i18n.client.gen.Shapes;
 import com.google.gwt.i18n.client.gen.TestMessages;
-import com.google.gwt.i18n.client.impl.ConstantMap;
 import com.google.gwt.i18n.client.resolutiontest.Inners;
 import com.google.gwt.i18n.client.resolutiontest.Inners.ExtendsInnerInner;
 import com.google.gwt.i18n.client.resolutiontest.Inners.HasInner;
@@ -37,6 +36,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.MissingResourceException;
@@ -189,161 +189,157 @@
     TestConstants types = (TestConstants) GWT.create(TestConstants.class);
 
     Map<String, String> map = types.mapABCD();
-    assertEquals(4, map.size());
-    assertEquals("valueA", map.get("keyA"));
-    assertEquals("valueB", map.get("keyB"));
-    assertEquals("valueC", map.get("keyC"));
-    assertEquals("valueD", map.get("keyD"));
-
+    Map<String, String> expectedMap = getMapFromArrayUsingASimpleRule(new String[] {
+        "A", "B", "C", "D"});
     assertNull(map.get("bogus"));
+    compareMapsComprehensively(map, expectedMap);
 
-    Set<String> keys = map.keySet();
-    Iterator<String> keyIter = keys.iterator();
-    assertEquals("keyA", keyIter.next());
-    assertEquals("keyB", keyIter.next());
-    assertEquals("keyC", keyIter.next());
-    assertEquals("keyD", keyIter.next());
-    assertFalse(keyIter.hasNext());
-
-    Collection<String> values = map.values();
-    Iterator<String> valueIter = values.iterator();
-    assertEquals("valueA", valueIter.next());
-    assertEquals("valueB", valueIter.next());
-    assertEquals("valueC", valueIter.next());
-    assertEquals("valueD", valueIter.next());
-    assertFalse(keyIter.hasNext());
-
+    /*
+     * Test if the returned map can be modified in any way. Things are working
+     * as expected if exceptions are thrown in each case.
+     */
+    String failureMessage = "Should have thrown UnsupportedOperationException";
+    /* test map operations */
     try {
       map.remove("keyA");
-      fail("Should have thrown UnsupportedOperationException");
+      fail(failureMessage + " on map.remove");
     } catch (UnsupportedOperationException e) {
-      // good if an exception was caught
     }
-
     try {
-      keys.clear();
-      fail("Should have thrown UnsupportedOperationException");
+      map.put("keyA", "allA");
+      fail(failureMessage + "on map.put of existing key");
     } catch (UnsupportedOperationException e) {
-      // good if an exception was caught
+    }
+    try {
+      map.put("keyZ", "allZ");
+      fail(failureMessage + "on map.put of new key");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.clear();
+      fail(failureMessage + " on map.clear");
+    } catch (UnsupportedOperationException e) {
     }
 
-    // TODO: fixme -- values are supposed to be backed by the map and should
-    // fail if modified
-    // try {
-    // Iterator nonmutableIter = keys.iterator();
-    // nonmutableIter.next();
-    // nonmutableIter.remove();
-    // fail("Should have thrown UnsupportedOperationException");
-    // } catch (UnsupportedOperationException e) {
-    // // good if an exception was caught
-    // }
+    /* test map.keySet() operations */
+    try {
+      map.keySet().add("keyZ");
+      fail(failureMessage + " on map.keySet().add");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.keySet().remove("keyA");
+      fail(failureMessage + " on map.keySet().remove");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.keySet().clear();
+      fail(failureMessage + " on map.keySet().clear");
+    } catch (UnsupportedOperationException e) {
+    }
+
+    /* test map.values() operations */
+    try {
+      map.values().add("valueZ");
+      fail(failureMessage + " on map.values().add");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.values().remove("valueA");
+      fail(failureMessage + " on map.values().clear()");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.values().clear();
+      fail(failureMessage + " on map.values().clear()");
+    } catch (UnsupportedOperationException e) {
+    }
+
+    /* test map.entrySet() operations */
+    Map.Entry<String, String> firstEntry = map.entrySet().iterator().next();
+    try {
+      map.entrySet().clear();
+      fail(failureMessage + "on map.entrySet().clear");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.entrySet().remove(firstEntry);
+      fail(failureMessage + " on map.entrySet().remove");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.entrySet().add(firstEntry);
+      fail(failureMessage + "on map.entrySet().add");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      firstEntry.setValue("allZ");
+      fail(failureMessage + "on firstEntry.setValue");
+    } catch (UnsupportedOperationException e) {
+    }
+    try {
+      map.clear();
+      fail(failureMessage + " on map.clear");
+    } catch (UnsupportedOperationException e) {
+    }
   }
 
   /**
-   * Tests focus on just the key order, since ABCD exercises the map.
+   * Tests exercise the cache.
    */
   public void testConstantMapBACD() {
     TestConstants types = (TestConstants) GWT.create(TestConstants.class);
-
-    ConstantMap map = (ConstantMap) types.mapBACD();
-
-    Set<String> keys = map.keySet();
-    Iterator<String> keyIter = keys.iterator();
-    assertEquals("keyB", keyIter.next());
-    assertEquals("keyA", keyIter.next());
-    assertEquals("keyC", keyIter.next());
-    assertEquals("keyD", keyIter.next());
-
-    Collection<String> values = map.values();
-    Iterator<String> valueIter = values.iterator();
-    assertEquals("valueB", valueIter.next());
-    assertEquals("valueA", valueIter.next());
-    assertEquals("valueC", valueIter.next());
-    assertEquals("valueD", valueIter.next());
+    Map<String, String> map = types.mapBACD();
+    Map<String, String> expectedMap = getMapFromArrayUsingASimpleRule(new String[] {
+        "B", "A", "C", "D"});
+    compareMapsComprehensively(map, expectedMap);
   }
 
   /**
-   * Tests focus on correctness of entries, since ABCD exercises the map.
+   * Tests exercise the cache.
    */
   public void testConstantMapBBB() {
     TestConstants types = (TestConstants) GWT.create(TestConstants.class);
-
-    ConstantMap map = (ConstantMap) types.mapBBB();
-
-    assertEquals(1, map.size());
-
-    Set<String> keys = map.keySet();
-    assertEquals(1, keys.size());
-    Iterator<String> keyIter = keys.iterator();
-    assertEquals("keyB", keyIter.next());
-
-    Collection<String> values = map.values();
-    assertEquals(1, values.size());
-    Iterator<String> valueIter = values.iterator();
-    assertEquals("valueB", valueIter.next());
+    Map<String, String> map = types.mapBBB();
+    Map<String, String> expectedMap = getMapFromArrayUsingASimpleRule(new String[] {"B"});
+    compareMapsComprehensively(map, expectedMap);
   }
 
   /**
-   * Tests focus on just the key order, since ABCD exercises the map.
+   * Tests exercise the cache and check if Map works as the declared return
+   * type.
    */
-  public void testConstantMapDBCA() {
+  @SuppressWarnings("unchecked")
+  public void testConstantMapDCBA() {
     TestConstants types = (TestConstants) GWT.create(TestConstants.class);
-
-    ConstantMap map = (ConstantMap) types.mapDCBA();
-
-    Set<String> keys = map.keySet();
-    Iterator<String> keyIter = keys.iterator();
-    assertEquals("keyD", keyIter.next());
-    assertEquals("keyC", keyIter.next());
-    assertEquals("keyB", keyIter.next());
-    assertEquals("keyA", keyIter.next());
-
-    Collection<String> values = map.values();
-    Iterator<String> valueIter = values.iterator();
-    assertEquals("valueD", valueIter.next());
-    assertEquals("valueC", valueIter.next());
-    assertEquals("valueB", valueIter.next());
-    assertEquals("valueA", valueIter.next());
+    Map<String, String> map = types.mapDCBA();
+    Map<String, String> expectedMap = getMapFromArrayUsingASimpleRule(new String[] {
+        "D", "C", "B", "A"});
+    compareMapsComprehensively(map, expectedMap);
   }
 
   /**
    * Tests focus on correctness of entries, since ABCD exercises the map.
    */
+  public void testConstantMapEmpty() {
+    TestConstants types = (TestConstants) GWT.create(TestConstants.class);
+    Map<String, String> map = types.mapEmpty();
+    Map<String, String> expectedMap = new HashMap<String, String>();
+    compareMapsComprehensively(map, expectedMap);
+  }
+
+  /**
+   * Tests exercise the cache and check if Map works as the declared return
+   * type.
+   */
   public void testConstantMapXYZ() {
     TestConstants types = (TestConstants) GWT.create(TestConstants.class);
-
-    ConstantMap map = (ConstantMap) types.mapXYZ();
-
-    assertEquals(3, map.size());
-
-    Set<String> keys = map.keySet();
-    assertEquals(3, keys.size());
-    Iterator<String> keyIter = keys.iterator();
-    assertEquals("keyX", keyIter.next());
-    assertEquals("keyY", keyIter.next());
-    assertEquals("keyZ", keyIter.next());
-
-    Collection<String> values = map.values();
-    assertEquals(3, values.size());
-    Iterator<String> valueIter = values.iterator();
-    assertEquals("valueZ", valueIter.next());
-    assertEquals("valueZ", valueIter.next());
-    assertEquals("valueZ", valueIter.next());
-
-    Set<Map.Entry<String, String>> entries = map.entrySet();
-    assertEquals(3, entries.size());
-    Iterator<Map.Entry<String, String>> entryIter = entries.iterator();
-    Map.Entry<String, String> entry;
-
-    entry = entryIter.next();
-    assertEquals("keyX", entry.getKey());
-    assertEquals("valueZ", entry.getValue());
-    entry = entryIter.next();
-    assertEquals("keyY", entry.getKey());
-    assertEquals("valueZ", entry.getValue());
-    entry = entryIter.next();
-    assertEquals("keyZ", entry.getKey());
-    assertEquals("valueZ", entry.getValue());
+    Map<String, String> map = types.mapXYZ();
+    Map<String, String> expectedMap = new HashMap<String, String>();
+    expectedMap.put("keyX", "valueZ");
+    expectedMap.put("keyY", "valueZ");
+    expectedMap.put("keyZ", "valueZ");
+    compareMapsComprehensively(map, expectedMap);
   }
 
   public void testConstantStringArrays() {
@@ -451,28 +447,6 @@
     assertEquals(Integer.MIN_VALUE, types.intMin());
   }
 
-  // Uncomment for desk tests
-  // /**
-  // * Tests focus on correctness of entries, since ABCD exercises the map.
-  // */
-  // public void testConstantMapEmpty() {
-  // TestConstants types = (TestConstants) GWT.create(TestConstants.class);
-  //
-  // ConstantMap map = (ConstantMap) types.mapEmpty();
-  //
-  // assertEquals(0, map.size());
-  //
-  // Set keys = map.keySet();
-  // assertEquals(0, keys.size());
-  // Iterator keyIter = keys.iterator();
-  // assertFalse(keyIter.hasNext());
-  //
-  // Collection values = map.values();
-  // assertEquals(0, values.size());
-  // Iterator valueIter = values.iterator();
-  // assertFalse(valueIter.hasNext());
-  // }
-
   public void testLocalizableInner() {
     // Check simple inner
     LocalizableSimpleInner s = (LocalizableSimpleInner) GWT.create(Inners.LocalizableSimpleInner.class);
@@ -522,8 +496,9 @@
 
     // Protected InnerClass
     InnerClass innerClass = new Inners.InnerClass();
-    String extendsAnotherInner = innerClass.testExtendsAnotherInner();
-    assertEquals("{innerInner=4.321, outer=outer}", extendsAnotherInner);
+    Map<String, String> extendsAnotherInner = innerClass.testExtendsAnotherInner();
+    assertEquals("4.321", extendsAnotherInner.get("innerInner"));
+    assertEquals("outer", extendsAnotherInner.get("outer"));
 
     // ExtendProtectedInner
     String extendProtectedInner = innerClass.testExtendsProtectedInner();
@@ -588,6 +563,47 @@
     }
   }
 
+  private <T> boolean compare(Collection<T> collection1,
+      Collection<T> collection2) {
+    if (collection1 == null) {
+      return (collection2 == null);
+    }
+    if (collection2 == null) {
+      return false;
+    }
+    if (collection1.size() != collection2.size()) {
+      return false;
+    }
+    for (T element1 : collection1) {
+      boolean found = false;
+      for (T element2 : collection2) {
+        if (element1.equals(element2)) {
+          found = true;
+          break;
+        }
+      }
+      if (!found) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  // compare the map, entrySet, keySet, and values
+  private void compareMapsComprehensively(Map<String, String> map,
+      Map<String, String> expectedMap) {
+    // checking both directions to verify that the equals implementation is
+    // correct both ways
+    assertEquals(expectedMap, map);
+    assertEquals(map, expectedMap);
+    assertEquals(expectedMap.entrySet(), map.entrySet());
+    assertEquals(map.entrySet(), expectedMap.entrySet());
+    assertEquals(expectedMap.keySet(), map.keySet());
+    assertEquals(map.keySet(), expectedMap.keySet());
+    assertTrue(compare(expectedMap.values(), map.values()));
+    assertTrue(compare(map.values(), expectedMap.values()));
+  }
+
   private native void createDummyDictionaries() /*-{
     $wnd.testDic = new Object();
     $wnd.testDic.formattedMessage = "3 {2},{2},{2}, one {0}, two {1} {1}";
@@ -597,4 +613,12 @@
     $wnd.emptyDic = new Object();
     $wnd.malformedDic = 4;
   }-*/;
+
+  private Map<String, String> getMapFromArrayUsingASimpleRule(String array[]) {
+    Map<String, String> map = new HashMap<String, String>();
+    for (String str : array) {
+      map.put("key" + str, "value" + str);
+    }
+    return map;
+  }
 }
diff --git a/user/test/com/google/gwt/i18n/client/TestConstants.java b/user/test/com/google/gwt/i18n/client/TestConstants.java
index 96d307a..e97674a 100644
--- a/user/test/com/google/gwt/i18n/client/TestConstants.java
+++ b/user/test/com/google/gwt/i18n/client/TestConstants.java
@@ -84,6 +84,10 @@
   @SuppressWarnings("unchecked")
   Map mapDCBA();
 
+  Map<String, String> mapEmpty();
+
+  // Map<String, String> mapWithMissingKey();
+
   Map<String, String> mapXYZ();
 
   String[] stringArrayABCDEFG();
@@ -113,10 +117,4 @@
   String stringJapaneseRed();
 
   String stringTrimsLeadingWhitespace();
-
-  // uncomment for desk tests
-  // Map mapWithMissingKey();
-
-  // uncomment for desk tests
-  // Map mapEmpty();
 }
diff --git a/user/test/com/google/gwt/i18n/client/TestConstants.properties b/user/test/com/google/gwt/i18n/client/TestConstants.properties
index aeb7d51..35d7acf 100644
--- a/user/test/com/google/gwt/i18n/client/TestConstants.properties
+++ b/user/test/com/google/gwt/i18n/client/TestConstants.properties
@@ -81,7 +81,7 @@
 # mapXYZ: "keyX", "keyY", "keyZ"
 
 # map with a missing key (uncomment during desk tests; won't work in JUnit)
-# mapWithMissingKey: keyA, , keyB 
+# mapWithMissingKey: keyA, asdf, keyB 
 
-# empty map (uncomment during desk tests; won't work in JUnit)
-# mapEmpty: 
+# empty map
+mapEmpty: 
diff --git a/user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java b/user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java
index 566bdad..8abbc8b 100644
--- a/user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java
+++ b/user/test/com/google/gwt/i18n/client/resolutiontest/Inners.java
@@ -140,10 +140,10 @@
     }
 
     /** Tests Protected Inner Class. */
-    public String testExtendsAnotherInner() {
+    public Map<String, String> testExtendsAnotherInner() {
       ExtendsAnotherInner clazz = (ExtendsAnotherInner) GWT.create(ExtendsAnotherInner.class);
-      Map answer = clazz.extendsAnotherInner();
-      return answer.toString();
+      Map<String, String> answer = clazz.extendsAnotherInner();
+      return answer;
     }
 
     /** Test for ExtendProtectedInner. */
@@ -164,7 +164,7 @@
         }
 
         /** Test maps in extension. */
-        Map extendsAnotherInner();
+        Map<String, String> extendsAnotherInner();
       }
 
       /**