http://gwt-code-reviews.appspot.com/1395804/diff/11/user/test/com/google/gwt/user/client/rpc/TestSetFactory.java File user/test/com/google/gwt/user/client/rpc/TestSetFactory.java (right):
http://gwt-code-reviews.appspot.com/1395804/diff/11/user/test/com/google/gwt/user/client/rpc/TestSetFactory.java#newcode508 user/test/com/google/gwt/user/client/rpc/TestSetFactory.java:508: return map; On 2011/03/30 20:46:32, scottb wrote:
Sorry for not catching this the first time, but I realized looking at
the
TestSetValidator code for this, that this is kind of squirrelly. It
skirts
around the issue of identity in various ways. I would propose you
define two
enums, a key enum type and a value enum type, and use that for the IdentityHashMap test. Then you can actually get the correct Object
identity
semantics, since enum values must serialize as singleton objects. And
it should
make the test code in TestSetValidator much simpler.
You've caused me to think about it some more (always a good thing). Testing this, and even defining the right behavior, is non-trivial. Say that on the client I add two objects that are reference-distinct but equals-identical. The IdentityHashMap should store those separately. When serialized, it seems that the server should reconstruct an identity map with two distinct entries, and be able to send it back. We're not testing this at all, so I'll make an attempt to generate such a case. As far as I can tell, the case you are referring to occurs when you place some object in an IdentityHashMap, serialize and get back the map. The element in the deserialized map should be reference equal to the original element. I don't see how we can guarantee such semantics. We're probably getting to the heart of why nobody ever hit this bug in the first place. It's not clear why you would ever want to serialize an IdentityHashMap. http://gwt-code-reviews.appspot.com/1395804/diff/11/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java File user/test/com/google/gwt/user/client/rpc/TestSetValidator.java (right): http://gwt-code-reviews.appspot.com/1395804/diff/11/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java#newcode367 user/test/com/google/gwt/user/client/rpc/TestSetValidator.java:367: Set<?> mapEntries = map.entrySet(); On 2011/03/30 20:46:32, scottb wrote:
Hoist this out of the loop.
Done. http://gwt-code-reviews.appspot.com/1395804/diff/11/user/test/com/google/gwt/user/client/rpc/TestSetValidator.java#newcode418 user/test/com/google/gwt/user/client/rpc/TestSetValidator.java:418: } On 2011/03/30 20:46:32, scottb wrote:
No need to check here, equals() will.
Done. http://gwt-code-reviews.appspot.com/1395804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
