Re: RequestFactory issue with overloaded setters in domain type

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.