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/27 14:40:24, rdayal wrote:
On 2012/04/10 08:05:33, tbroyer wrote:
> 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)
Ok, thanks for the explanation, that helps a lot. Two things I'd say:
-we need to document this somewhere, because it's pretty subtle
-it seems bad that we can get into states where ValueProxy.equals() is
doing
"the wrong thing", but I'm not familiar enough with this code to
propose a good
solution. I'd just say that it seems like a pitfall if we need to have
an
IdentityHashMap to work around issues like this..
Done.
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/27 14:40:24, rdayal wrote:
On 2012/04/10 08:05:33, tbroyer wrote:
> 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/%257E/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.
AH!!! I did not know that. Thanks again. BTW, I can't read that wave -
can you
add me to it?
Unfortunately, Wave is readonly so I cannot add you to the wave, so I
exported it and mailed it to you.
BTW, with() is documented in
https://developers.google.com/web-toolkit/doc/latest/DevGuideRequestFactory#relationships
;-)
https://gwt-code-reviews.appspot.com/1646803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
