Custom Field Serializer for EnumMap (Issue 1634804)

Custom Field Serializer for EnumMap (Issue 1634804)

[jat, 2012-05-02 23:02:06]

There are some formatting issues and one issue about exceptions, but otherwise
looks good, assuming tests pass internally.

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/cli...
File
user/src/com/google/gwt/user/client/rpc/core/java/util/EnumMap_CustomFieldSerializer.java
(right):

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/cli...
user/src/com/google/gwt/user/client/rpc/core/java/util/EnumMap_CustomFieldSerializer.java:5:
* in compliance with the License. You may obtain a copy of the License at
The file header comment shouldn't be reformatted, if your IDE settings force
doing it, please just copy one from an existing file after you do a reformat.

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/cli...
user/src/com/google/gwt/user/client/rpc/core/java/util/EnumMap_CustomFieldSerializer.java:65:
e.printStackTrace();
Shouldn't these throw a SerializationException instead?

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/ser...
File
user/src/com/google/gwt/user/server/rpc/core/java/util/EnumMap_ServerCustomFieldSerializer.java
(right):

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/ser...
user/src/com/google/gwt/user/server/rpc/core/java/util/EnumMap_ServerCustomFieldSerializer.java:42:
public static void deserialize(ServerSerializationStreamReader streamReader,
EnumMap instance, Type[] expectedParameterTypes,
Line length.

http://codereview.appspot.com/6174044/diff/1/user/src/com/google/gwt/user/ser...
user/src/com/google/gwt/user/server/rpc/core/java/util/EnumMap_ServerCustomFieldSerializer.java:44:
throws SerializationException
The style here is to put the brace on the end of the previous line.

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/cl...
File user/test/com/google/gwt/user/client/rpc/CollectionsTest.java (right):

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/cl...
user/test/com/google/gwt/user/client/rpc/CollectionsTest.java:367: service
Weird formatting, see other tests or use the proper format settings.

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/cl...
File user/test/com/google/gwt/user/client/rpc/CollectionsTestServiceAsync.java
(right):

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/cl...
user/test/com/google/gwt/user/client/rpc/CollectionsTestServiceAsync.java:76: 
Trailing spaces added.

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/se...
File user/test/com/google/gwt/user/server/rpc/CollectionsTestServiceImpl.java
(right):

http://codereview.appspot.com/6174044/diff/1/user/test/com/google/gwt/user/se...
user/test/com/google/gwt/user/server/rpc/CollectionsTestServiceImpl.java:508:
EnumMap<MarkerTypeEnum, MarkerTypeEnumMapValue> expected =
TestSetFactory.createEnumMapEnumKey();
Line > 100 chars.

[bradley, 2012-05-03 14:52:47]

I believe the second patch set address all your concerns.
Thanks.

[skybrian1, 2012-06-07 02:06:04]

On 2012/05/03 14:52:47, bradley wrote:
> I believe the second patch set address all your concerns.
> Thanks.

This looks reasonable. I'm going to try running out tests against it. Did you
sign the Contributor License Agreement? I didn't see your name in the list.

Instructions at the bottom of:
https://developers.google.com/web-toolkit/makinggwtbetter#clas

Here's the list of contributors:
http://code.google.com/p/google-web-toolkit/source/browse/CLA-SIGNERS

[skybrian1, 2012-06-07 18:05:46]

I'm getting exceptions running tests in web mode. The line number in
EnumMap_CustomFieldSerializer doesn't make sense to me, but here is one of them:

3)
testEmptyEnumMap(com.google.gwt.rpc.client.RpcCollectionsTest)java.lang.RuntimeException:
Remote test failed at 10.58.163.225 / Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.9.0.19) Gecko/2010031422 Firefox/3.0.19


Caused by: java.util.NoSuchElementException
...
at
java.util.NoSuchElementException.NoSuchElementException(NoSuchElementException.java:25)
	at java.util.EnumSet$EnumSetImpl$IteratorImpl.$next(EnumSet.java:59)
	at java.util.EnumSet.next(EnumSet.java:57)
	at
com.google.gwt.user.client.rpc.core.java.util.EnumMap_CustomFieldSerializer.serialize(EnumMap_CustomFieldSerializer.java:65)
	at
com.google.gwt.rpc.client.impl.TypeOverrides$SerializeFunction.$serialize(TypeOverrides.java:35)
	at
com.google.gwt.rpc.client.impl.CommandClientSerializationStreamWriter.$invokeCustomSerializer(CommandClientSerializationStreamWriter.java:159)
	at
com.google.gwt.rpc.client.impl.CommandClientSerializationStreamWriter.makeValue(CommandClientSerializationStreamWriter.java:113)
	at
com.google.gwt.rpc.client.impl.CommandSerializationStreamWriterBase.writeObject(CommandSerializationStreamWriterBase.java:64)
	at
com.google.gwt.rpc.client.impl.CommandToStringWriter.writeObject(CommandToStringWriter.java:99)
	at
com.google.gwt.user.client.rpc.CollectionsTestService_Proxy.echoEmptyEnumMap(CollectionsTestService_Proxy.java:501)
	at
com.google.gwt.user.client.rpc.CollectionsTest.$testEmptyEnumMap(CollectionsTest.java:257)

[bradley, 2012-06-07 18:38:19]

On 2012/06/07 18:05:46, skybrian1 wrote:
> I'm getting exceptions running tests in web mode. The line number in
> EnumMap_CustomFieldSerializer doesn't make sense to me, but here is one of
them:
> 
> 3)
>
testEmptyEnumMap(com.google.gwt.rpc.client.RpcCollectionsTest)java.lang.RuntimeException:
> Remote test failed at 10.58.163.225 / Mozilla/5.0 (Windows; U; Windows NT 5.1;
> en-US; rv:1.9.0.19) Gecko/2010031422 Firefox/3.0.19
> 
> 
> Caused by: java.util.NoSuchElementException
> ...
> at
>
java.util.NoSuchElementException.NoSuchElementException(NoSuchElementException.java:25)
> 	at java.util.EnumSet$EnumSetImpl$IteratorImpl.$next(EnumSet.java:59)
> 	at java.util.EnumSet.next(EnumSet.java:57)
> 	at
>
com.google.gwt.user.client.rpc.core.java.util.EnumMap_CustomFieldSerializer.serialize(EnumMap_CustomFieldSerializer.java:65)
> 	at
>
com.google.gwt.rpc.client.impl.TypeOverrides$SerializeFunction.$serialize(TypeOverrides.java:35)
> 	at
>
com.google.gwt.rpc.client.impl.CommandClientSerializationStreamWriter.$invokeCustomSerializer(CommandClientSerializationStreamWriter.java:159)
> 	at
>
com.google.gwt.rpc.client.impl.CommandClientSerializationStreamWriter.makeValue(CommandClientSerializationStreamWriter.java:113)
> 	at
>
com.google.gwt.rpc.client.impl.CommandSerializationStreamWriterBase.writeObject(CommandSerializationStreamWriterBase.java:64)
> 	at
>
com.google.gwt.rpc.client.impl.CommandToStringWriter.writeObject(CommandToStringWriter.java:99)
> 	at
>
com.google.gwt.user.client.rpc.CollectionsTestService_Proxy.echoEmptyEnumMap(CollectionsTestService_Proxy.java:501)
> 	at
>
com.google.gwt.user.client.rpc.CollectionsTest.$testEmptyEnumMap(CollectionsTest.java:257)

It looks like the tests are using the wrong version of
EnumMap_CustomFieldSerializer. In web mode the supersource version should be
called. There may be a .gwt.xml file somewhere that needs to be modified, but I
didn't find any such reference for the super-sourced
LinkedHashMap_CustomFieldSerializer which I used as a reference.

[skybrian1, 2012-06-07 21:51:28]

This is a web mode test and it looks like we're using the right version because
the line number matches up. Here's the line in question:

    exemplar = privateKeySet.iterator().next(); 

So this looks like an actual bug. The exemplar approach doesn't work for an
empty EnumSet (obvious in retrospect), so we'll have to think of something else.
Maybe we could use a generator and choose the exemplar at compile time?

Since the code freeze for GWT 2.5 is tomorrow, I don't think we're going to get
this in. Maybe 2.5.1.

[bradley, 2012-06-12 17:24:35]

I meant to get the private array that holds the universe of allowed enums.
Instead I grabbed an iterator to the live keySet. This patch should fix that. I
tested it under hosted and web mode, as well as with the junit tests.

[skybrian1, 2012-06-12 17:49:57]

Looks good; I'll give it a try.