Starting to add unit tests for JsonRpc support in RF would be great
(though not easy, as GWT doesn't provide any server-side utility).

Otherwise, LGTM (but see comments below).


http://gwt-code-reviews.appspot.com/1618806/diff/2001/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
File
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
(right):

http://gwt-code-reviews.appspot.com/1618806/diff/2001/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode269
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:269:
assert (obj instanceof Collection);
How about casting 'obj' to a Collection at the call site instead? (and
thus declare the argument here of type Collection)

http://gwt-code-reviews.appspot.com/1618806/diff/2001/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode274
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:274:
sb.append(nonCollectionEncode(it.next()));
Using encode() here would pave the way for supporting collections of
collections (see issue 5974).
You'd then turn the RuntimeException in nonCollectionEncode into a
simple 'assert'.
I mean, in the case of JsonRpc, GWT doesn't provide the backend, so
there's theoretically no reason to disallow collections of collections,
except for parity with the "standard" RF protocol.

http://gwt-code-reviews.appspot.com/1618806/diff/2001/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode291
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:291:
if (obj.getClass().isEnum() && getAutoBeanFactory() instanceof EnumMap)
{
Hmm, looks like there's the same issue as was fixed some time ago in
ValueCodex re. enum values with their own "body", or when the enum has a
constructor with arguments: in this case, their class is an anonymous
subclass of the enum class, one for which isEnum() returns false.
Using "obj instanceof Enum" instead of obj.getClass().isEnum() would fix
it.

See issue 6504.

http://gwt-code-reviews.appspot.com/1618806/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to