https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java File user/src/com/google/web/bindery/requestfactory/server/Resolver.java (right):
https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java#newcode365 user/src/com/google/web/bindery/requestfactory/server/Resolver.java:365: new IdentityHashMap<BaseProxy, Resolution>(); On 2012/04/09 15:44:43, rdayal wrote:
Looks good, but I must say that I dont' quite understand how/why this
fixes the
problem. Can you point out how the code execution in this class would
have
resulted in a breakage with a HashMap vs. an IdentityHashMap?
At line 632, a collection is populated with the result of resolveClientValue(Object,Type), which is always an empty proxy (properties are populated later) –modulo reuse of a proxy if it has already been resolved–. This isn't an issue with EntityProxies, as each proxy as at a minimum a stableId that keys it to a domain object and distinguishes it from other proxies (for other domain objects), but ValueProxies compare equals() to each other by their properties' values only, independently of the underlying domain object. So, when you resolve a collection of ValueProxies, the first iteration of the loop (at line 632) creates a new, empty ValueProxy and puts it in the clientObjectsToResolutions map. In the second iteration, resolveClientValue will create another ValueProxy, but will return the same Resolution because the ValueProxy compares equals() to the previous one, so getting the Resolution from the map will return the one for the first ValueProxy. Using an IdentityHashMap fixes that lookup, so that we have one Resolution instance per proxy instance. Because we have other guards for EntityProxies (keyed by stableId in state.getBeansForPayload(), via resolveClientProxy), that change is safe: the goal is to have a most one proxy per domain object (which is guaranteed in RequestState for EntityProxies) https://gwt-code-reviews.appspot.com/1646803/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java (right): https://gwt-code-reviews.appspot.com/1646803/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode587 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:587: simpleFooRequest().returnValueProxies().with("simpleFoo.fooField").fire(new Receiver<List<SimpleValueProxy>>() { On 2012/04/09 15:44:43, rdayal wrote:
Sorry if this is naive question, but since the returnValueProxies
method
implementation always fills in all of the properties (such as
fooField), doesn't
the with("simpleFoo.fooField") become redundant? Based on the
implementation of
returnValueProxies(), it seems that simpleFoo.fooField will always be
filled in,
regardless of whether or not you use the 'with' statement.
The with() is not passed down to the domain method (which is another issue: https://wave.google.com/wave/waveref/googlewave.com/w+WU4iAICkI/~/conv+root/b+QDorc1lYB ), it's a wire-protocol thing (it's not even sent to the server when using the JsonRpc dialect). By default, for EntityProxies, only "value-type" properties are populated (those that can be encoded by ValueCodex; and lists of those). If you want any other kind of object to be serialized and sent to the client, it has to be asked for explicitly using with(); independently of whether the domain method will populate the property on the server-side. https://gwt-code-reviews.appspot.com/1646803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors