Updated patch: (a) incorporates ConstantMapTest, (b) Reverts the mappings
change I introduced.
Amit
On Thu, Oct 30, 2008 at 12:38 PM, Scott Blum <[EMAIL PROTECTED]> wrote:
> Sweet.
>
>
> On Thu, Oct 30, 2008 at 12:32 PM, Amit Manjhi <[EMAIL PROTECTED]>wrote:
>
>> ConcurrentModificationException. I will handle the exception :).
>>
>> On second thoughts, I did see the utility of adding equals check in the
>> other direction. The most recent patch contains the checks. I also
>> discovered the missing equals and hashCode methods, which I also added in
>> the patch. I will add the updates to ConstantMapTest and check the
>> difference in mappings, and send you an updated patch shortly.
>>
>> Amit
>>
>>
>> On Thu, Oct 30, 2008 at 12:21 PM, Scott Blum <[EMAIL PROTECTED]> wrote:
>>
>>> On Wed, Oct 29, 2008 at 8:45 PM, Amit Manjhi <[EMAIL PROTECTED]>wrote:
>>>
>>>> Thanks for the feedback, Scott.
>>>>
>>>> On Wed, Oct 29, 2008 at 8:22 PM, Scott Blum <[EMAIL PROTECTED]> wrote:
>>>>
>>>>> I18NSuite: why is the test commented out?
>>>>>
>>>>
>>>> Since ConstantMap now uses Jsni, it needs a Gwt environment. There did
>>>> not seem an obvious way for the ConstantMapTest to extend both MapTestBase
>>>> and GWTTestCase.
>>>>
>>>
>>> Ahh... hmm. Well, ConstantMapTest isn't doing much right now, because it
>>> needs to implement makeFullMap() to really do anything. I played around
>>> with the idea of subclassing ConstantMap and overriding the JSNI methods to
>>> use an internal hash map. Doing so uncovered at least one bug... our map
>>> entries don't implement equals, hashCode, toString. Here's what I did, you
>>> can build on this:
>>>
>>> public class ConstantMapTest extends MapTestBase {
>>>
>>> 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 putImpl(String key, String value) {
>>> noJsniImpl.put(key, value);
>>> }
>>>
>>> @Override
>>> protected void init() {
>>> noJsniImpl = new HashMap<String, String>();
>>> }
>>> }
>>>
>>> @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 isRemoveModifiable() {
>>> return false;
>>> }
>>>
>>> @Override
>>> protected boolean useNullKey() {
>>> return false;
>>> }
>>>
>>> @Override
>>> protected boolean useNullValue() {
>>> return false;
>>> }
>>> }
>>>
>>>
>>>
>>>> - Line 194, 295, etc should be assertEquals instead of assertTrue unless
>>>>> there's some compelling reason not to. Also, it would be good to test
>>>>> equality in both directions in all of these cases
>>>>>
>>>> Will change it to assertEquals. But I don't see the need to check in
>>>> both directions since we can assume Map.equals to be commutative.
>>>>
>>>
>>> Actually, we should NOT assume equals is commutative.. we need to test
>>> that it's actually commutative! For example, the fact that our map entries
>>> don't implement equals at all is an example of this... equality works in one
>>> direction but not the other.
>>>
>>>
>>>> - Reason for the changes starting on lines 439 and 511?
>>>>>
>>>>
>>>> 511: Previously the order of the map was guaranteed. Now it can be any
>>>> order.
>>>>
>>>
>>> Are we looking at the same thing? This is what I'm seeing:
>>>
>>> Before:
>>> assertEquals("{innerInner=4.321, outer=outer}", extendsAnotherInner);
>>>
>>> After:
>>>
>>> assertTrue("{innerInner=4.321, outer=outer}".equals(extendsAnotherInner)
>>> || "{innerInner=outer,
>>> outer=4.321}".equals(extendsAnotherInner));
>>>
>>> This actually looks like a change in mappings, not merely a change in
>>> order.
>>>
>>>
>>
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
Index: user/test/com/google/gwt/i18n/ConstantMapTest.java
===================================================================
--- user/test/com/google/gwt/i18n/ConstantMapTest.java (revision 3885)
+++ user/test/com/google/gwt/i18n/ConstantMapTest.java (working copy)
@@ -17,19 +17,71 @@
import com.google.gwt.i18n.client.impl.ConstantMap;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
/**
- * 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
+ public Set<String> keySet() {
+ return noJsniImpl.keySet();
+ }
+
+ @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;
+ }
}
+
Index: user/test/com/google/gwt/i18n/I18NSuite.java
===================================================================
--- user/test/com/google/gwt/i18n/I18NSuite.java (revision 3885)
+++ user/test/com/google/gwt/i18n/I18NSuite.java (working copy)
@@ -54,7 +54,7 @@
suite.addTestSuite(DateTimeParse_zh_CN_Test.class);
suite.addTestSuite(I18NTest.class);
suite.addTestSuite(I18N2Test.class);
- suite.addTestSuite(LocaleInfo_ar_Test.class);
+ suite.addTestSuite(LocaleInfo_ar_Test.class);
suite.addTestSuite(LocaleInfoTest.class);
suite.addTestSuite(MessageFormatParserTest.class);
suite.addTestSuite(NumberFormat_ar_Test.class);
Index: user/test/com/google/gwt/i18n/client/I18NTest.java
===================================================================
--- user/test/com/google/gwt/i18n/client/I18NTest.java (revision 3885)
+++ user/test/com/google/gwt/i18n/client/I18NTest.java (working copy)
@@ -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;
@@ -52,7 +52,8 @@
return "com.google.gwt.i18n.I18NTest";
}
- @SuppressWarnings("unchecked") // intentional test of raw map
+ @SuppressWarnings("unchecked")
+ // intentional test of raw map
public void testAnnotatedConstants() {
TestAnnotatedConstants c = GWT.create(TestAnnotatedConstants.class);
assertEquals(14, c.fourteen());
@@ -189,161 +190,146 @@
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);
+
+ /*
+ * 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(failureMessage + " on map.remove");
+ } catch (UnsupportedOperationException e) {
+ }
+ try {
+ map.put("keyA", "allA");
+ fail(failureMessage + "on map.put of existing key");
+ } catch (UnsupportedOperationException e) {
+ }
+ 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) {
+ }
- 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());
+ /* 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) {
+ }
- 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 map.values() operations */
try {
- map.remove("keyA");
- fail("Should have thrown UnsupportedOperationException");
+ map.values().add("valueZ");
+ fail(failureMessage + " on map.values().add");
} catch (UnsupportedOperationException e) {
- // good if an exception was caught
}
+ 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 {
- keys.clear();
- fail("Should have thrown UnsupportedOperationException");
+ map.entrySet().clear();
+ fail(failureMessage + "on map.entrySet().clear");
} catch (UnsupportedOperationException e) {
- // good if an exception was caught
}
-
- // 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
- // }
+ 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() {
+ 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.
+ * 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 +437,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);
@@ -537,6 +501,28 @@
assertEquals("estednay underscoray", m.nestedUnderscore());
}
+ // 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 testShapesFamily() {
Shapes shapes = (Shapes) GWT.create(Shapes.class);
// test overload
@@ -588,6 +574,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 +624,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;
+ }
}
Index: user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
===================================================================
--- user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
(revision 3885)
+++ user/src/com/google/gwt/i18n/rebind/ConstantsWithLookupImplCreator.java
(working copy)
@@ -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;
}
});
Index: user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
===================================================================
--- user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
(revision 3885)
+++ user/src/com/google/gwt/i18n/rebind/ConstantsMapMethodCreator.java
(working copy)
@@ -26,6 +26,9 @@
* 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.
*
@@ -59,12 +62,12 @@
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 + "();");
+ println("args = new " + constantMapClassName + "(new String[] {");
String value;
try {
value = resourceList.getRequiredStringExt(key, null);
@@ -73,17 +76,30 @@
throw e;
}
String[] args = ConstantsStringArrayMethodCreator.split(value);
-
+
+ indent();
+ indent();
for (int i = 0; i < args.length; i++) {
+ // prepend the keys with ":" since they will be stored in a JSO
+ println(wrap(":" + args[i]) + ", ");
+ }
+ outdent();
+ println("},");
+ indent();
+ println("new String[] {");
+ for (int i = 0; i < args.length; i++) {
try {
- key = args[i];
- String keyValue = getResources().getString(key);
- println("args.put(" + wrap(key) + ", " + wrap(keyValue) + ");");
+ String keyValue = getResources().getString(args[i]);
+ println(wrap(keyValue) + ",");
} catch (MissingResourceException e) {
e.setDuring("implementing map");
throw e;
}
}
+ outdent();
+ println("});");
+ outdent();
+ println("args = java.util.Collections.unmodifiableMap(args);");
println("cache.put(" + wrap(methodName) + ", args);");
outdent();
println("};");
Index: user/src/com/google/gwt/i18n/client/impl/ConstantMap.java
===================================================================
--- user/src/com/google/gwt/i18n/client/impl/ConstantMap.java (revision 3885)
+++ user/src/com/google/gwt/i18n/client/impl/ConstantMap.java (working copy)
@@ -15,33 +15,43 @@
*/
package com.google.gwt.i18n.client.impl;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Iterator;
+import com.google.gwt.core.client.JavaScriptObject;
+
+import java.util.AbstractMap;
+import java.util.HashSet;
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 static class MapEntry implements Map.Entry<String, String> {
private final String key;
-
private final String value;
- DummyMapEntry(String key, String value) {
+ MapEntry(String key, String value) {
this.key = key;
this.value = value;
}
+ /**
+ * Implement equals as per Sun's specification.
+ */
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof Entry)) {
+ return false;
+ }
+ Entry e2 = (Entry) o;
+ return (key == null ? e2.getKey() == null : key.equals(e2.getKey()))
+ && (value == null ? e2.getValue() == null
+ : value.equals(e2.getValue()));
+ }
+
public String getKey() {
return key;
}
@@ -50,102 +60,107 @@
return value;
}
+ /**
+ * 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;
+ }
+
public String setValue(String arg0) {
throw new UnsupportedOperationException();
}
}
- private class OrderedConstantSet<T> extends ArrayList<T> implements Set<T> {
- private class ImmutableIterator implements Iterator<T> {
- private final Iterator<T> base;
+ // keySet computed on demand.
+ private Set<String> keySet;
- ImmutableIterator(Iterator<T> base) {
- this.base = base;
- }
+ /*
+ * Accesses need to be prefixed with ':' to prevent conflict with built-in
+ * JavaScript properties.
+ */
+ // map is used only in jsni methods
+ @SuppressWarnings("unused")
+ private JavaScriptObject map;
- public boolean hasNext() {
- return base.hasNext();
- }
+ private Set<Map.Entry<String, String>> entries;
- public T next() {
- return base.next();
- }
-
- public void remove() {
- throw new UnsupportedOperationException("Immutable set");
- }
+ public ConstantMap(String keys[], String values[]) {
+ // a separate function so that testing in nonJsni environment is possible
+ init();
+ int i = 0;
+ for (String key : keys) {
+ putImpl(key, values[i++]);
}
-
- @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 keySet().contains(key);
}
@Override
- public Set<Map.Entry<String, String>> entrySet() {
+ public Set<java.util.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));
+ entries = new HashSet<Map.Entry<String, String>>();
+ for (String key : keySet()) {
+ entries.add(new MapEntry(key, get(key)));
}
}
return entries;
}
@Override
- public Set<String> keySet() {
- return keys;
+ public String get(Object ob) {
+ if (ob instanceof String) {
+ return get((String) ob);
+ }
+ return null;
}
+ // Prepend ':' to avoid conflicts with built-in Object properties.
+ public native String get(String key) /*-{
+ return [EMAIL PROTECTED]::map[':' + key];
+ }-*/;
+
@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);
+ public Set<String> keySet() {
+ if (keySet == null) {
+ keySet = new HashSet<String>();
+ addAllKeysFromJavascriptObject(keySet, map);
}
- return super.put(key, value);
+ return keySet;
}
@Override
- public String remove(Object key) {
- throw unsupported("remove");
+ public int size() {
+ return keySet().size();
}
- @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));
- }
+ protected native void init() /*-{
+ [EMAIL PROTECTED]::map = [];
+ }-*/;
+
+ // Already prepended ':' to avoid conflicts with built-in Object properties.
+ protected native void putImpl(String key, String value) /*-{
+ [EMAIL PROTECTED]::map[key] = value;
+ }-*/;
+
+ // only count keys with ':' prefix
+ private native void addAllKeysFromJavascriptObject(Set<String> s,
+ JavaScriptObject javaScriptObject) /*-{
+ for(var key in javaScriptObject) {
+ if (key.charAt(0) != ':') continue;
+ [EMAIL PROTECTED]::add(Ljava/lang/Object;)(key.substring(1));
}
- return values;
- }
-
- private UnsupportedOperationException unsupported(String operation) {
- return new UnsupportedOperationException(operation
- + " not supported on a constant map");
- }
+ }-*/;
}
