[gwt-contrib] Re: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
Which to me leaves the question: is our usage described at [1] an intended use case, that's here to stay? or simply a fortunate unspecified behavior, and we should change our code to this new append() feature? For now, I'm going to call it a fortunate unspecified behavior. At some point in the future when I can get time allocated to add robust polymorphism support to RequestFactory, that behavior might go away if it helps promote uniformity between proxy-entity and context-code type hierarchy mappings. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
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 ReceiverVoid() {
@Override
public void onSuccess(Void response) {
reallyDone[0] = true;
if (reallyDone[0] reallyDone[1]) {
finishTest();
}
}
});
req.simpleBarRequest().reset().fire(new ReceiverVoid() {
@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 ReceiverVoid() {
@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 ReceiverVoid() {
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 ReceiverVoid() {
Nice. NICE!
:-)
http://gwt-code-reviews.appspot.com/1423805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
r10052
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.
On 2011/04/21 19:36:38, rjrjr wrote:
Don't they also land here via create?
Done.
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));
On 2011/04/21 19:36:38, rjrjr wrote:
should you test a create from c3 as well? I could imagine there being
upstream /
downstream issues for non-neighbors.
Done.
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);
On 2011/04/21 19:36:38, rjrjr wrote:
because c3 has already been appended?
Done.
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 ReceiverVoid() {
On 2011/04/21 19:36:38, rjrjr wrote:
what happens if you fire c1 or c2 instead? Should that be tested?
Done.
http://gwt-code-reviews.appspot.com/1423805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
On 2011/04/21 19:36:37, rjrjr wrote: LGTM Wow. Wow. This is a very big deal, and from such a simple change! Which to me leaves the question: is our usage described at [1] an intended use case, that's here to stay? or simply a fortunate unspecified behavior, and we should change our code to this new append() feature? (BTW, good job Bob, as always!) [1] http://code.google.com/p/google-web-toolkit/issues/detail?id=5807#c3 http://gwt-code-reviews.appspot.com/1423805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
