Wednesday, February 27, 2013

Re: RequestFactory issue with overloaded setters in domain type

Thanks for the quick and detailed reply.

Given a big win using RequestFactory is having it work well with existing server side data models it'd seem like something we'd want to fix, relative to other priorities of course. In this specifc case I don't own the domain class as it's from a 3rd party and it's been fantastic with RF to just use a proxy interface to pass the data across without another DTO. I can extend it in this specific case to override the behaviour of setUnit(SomeUnitEnum) but that's not always possible and I'm still at the mercy of the order the methods are iterated in as to what type I have to use in my Proxy interface, this throws it into the want to fix case for me (not surprising perhaps given I'm the one asking the question).

I'll file an issue for it and am fully behind the fact that if I think it's important that I should get a patch out for it so will try to do that. The geTop().getGetter(domainClass, property) suggestion you made sounds like a quick and mildly dirty way through; although I may look at the first fallback based solution you mentioned.

Cheers,
James




On 27 February 2013 16:05, Thomas Broyer <t.broyer@gmail.com> wrote:


On Wednesday, February 27, 2013 4:05:30 PM UTC+1, James Horsley wrote:
I'm running into an issue where RequestFactory isn't calling the correct setter on the domain object in DevMode and I've not tried it compiled yet. Looking at the code it appears that ReflectiveServiceLayer.getBeanMethod is just picking the first setter it finds based on SET/SET_BUILDER, are only verifying that the method is a setter, and the method name.

I was going to submit an issue for it and see about getting a patch together but thought I'd ask here first to see if I'm missing something.

A more concrete example that I'm running into is that the domain class has something like the following:

DomainClass.java
void setUnit(String unit) { this.unit = unit; }
void setUnit(SomeUnitEnum unit) { this.unit = unit.toString(); }

ProxyInterface.java
void setUnit(String unit);
String getUnit();

when unit is null, the server side RF code is calling the enum version which is causing an NPE.

Anything I'm missing or should just file the issue?

Fixing this would require breaking changes in ServiceLayerDecorator though (getSetter would need a new expectedType parameter, similar to setProperty) so I wonder how best we could provide backwards compatibility (ideally, getSetter(Class,String,Class) would fallback to getSetter(Class,String) (to ensure backwards compatibility with custom ServiceLayerDecorators) but we'd then like ReflectiveServiceLayer#getSetter(Class,String) to answer based on the Class originally passed to getSetter(Class,String,Class). Alternatively, ReflectiveServiceLayer#getSetter could call getTop().getGetter(domainClass, property) and then use the getter's return type to disambiguate overloaded setters. That would then make it impossible to have proxies with setter-only properties (see https://code.google.com/p/google-web-toolkit/issues/detail?id=5760, there's much to be done before this can be supported, so the server-side implementation might be revisited at that point, i.e. not a showstopper)

The question is: is this something we really *want* to fix? And honestly, I don't have a problem rejecting overloaded setters (see also the comment on https://code.google.com/p/google-web-toolkit/issues/detail?id=6587: if it weren't for protobuf where the idiom is common, having "overloaded" hasXxx and getXxx getters would have been declared illegal).

Feel free to file an issue, but be warned that I'll categorize it as PatchesWelcome.

--
You received this message because you are subscribed to the Google Groups "Google Web Toolkit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit+unsubscribe@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--
You received this message because you are subscribed to the Google Groups "Google Web Toolkit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit+unsubscribe@googlegroups.com.
To post to this group, send email to google-web-toolkit@googlegroups.com.
Visit this group at http://groups.google.com/group/google-web-toolkit?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

No comments:

Post a Comment