LGTM
Wow. Wow. This is a very big deal, and from such a simple change!
The patch includes its own demonstration of why it matters. Look at
RequestFactoryTestBase. It's an asynchronous test that can't declare
itself to be finished until two separate reset() messages are
acknowledged by the server. Until now that has required two separate
HTTP requests and some ugly daisy chaining code.
What was:
protected void finishTestAndReset() {
final boolean[] reallyDone = {false, false};
req.simpleFooRequest().reset().fire(new Receiver<Void>() {
@Override
public void onSuccess(Void response) {
reallyDone[0] = true;
if (reallyDone[0] && reallyDone[1]) {
finishTest();
}
}
});
req.simpleBarRequest().reset().fire(new Receiver<Void>() {
@Override
public void onSuccess(Void response) {
reallyDone[1] = true;
if (reallyDone[0] && reallyDone[1]) {
finishTest();
}
}
});
}
Is now:
protected void finishTestAndReset() {
SimpleFooRequest ctx = req.simpleFooRequest();
ctx.reset();
ctx.append(req.simpleBarRequest()).reset();
ctx.fire(new Receiver<Void>() {
@Override
public void onSuccess(Void response) {
finishTest();
}
});
}
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java
File
user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java
(right):
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java#newcode31
user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java:31:
<T extends RequestContext> T append(T other);
Nice, very simple.
http://gwt-code-reviews.appspot.com/1423805/diff/1/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/1423805/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode103
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:103:
* an invocation argument.
Don't they also land here via create?
http://gwt-code-reviews.appspot.com/1423805/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):
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode239
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:239:
assertSame(foo2, c3.edit(foo2));
should you test a create from c3 as well? I could imagine there being
upstream / downstream issues for non-neighbors.
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode243
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:243:
fail("Should have thrown IllegalStateException");
because c3 has already been appended?
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode262
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:262:
c3.fire(new Receiver<Void>() {
what happens if you fire c1 or c2 instead? Should that be tested?
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java
(right):
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java#newcode148
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java:148:
ctx.fire(new Receiver<Void>() {
Nice. NICE!
:-)
http://gwt-code-reviews.appspot.com/1423805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
