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/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.
Sorry for the long delay on this, but doing a bit of thinking - it seems
incorrect to add an empty ValueProxy object to the
clientObjectToResolutionMap when there's no representation of it. It
seems that items should be added to this list when they're actually
resolved (i.e filled in; not empty).
Can we come up with a solution that satisfies this? It may require the
addition of another "unresolved" List, and then migration over to the
map when items are filled in.
Though it's going to add more code, I think the workaround we have here
is a bit of a hack. I think it's an elegant hack, but it just feels
wrong to be working around ValueProxy.equals(....) because we're trying
to add empty value proxies to a "resolved" map when they really haven't
been totally resolved yet.
Thoughts?
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/27 15:01:36, tbroyer wrote:
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/%25257E/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
;-)
Always feel free to tell me to RTFM ;).
https://gwt-code-reviews.appspot.com/1646803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
