search

neo09 / gwt-platform

Automatically exported from code.google.com/p/gwt-platform
0 stars 0 forks source link

Return com.google.gwt.http.client.Request instead of void in DispatchAsync interfaces #149

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Problem:

Currently there does not seem to have a way to abort an ongoing Action because 
the DispatchAsync and DispatchServiceAsync interfaces only return void. GWT's 
com.google.gwt.http.client.Request let us call its cancel() method which 
basically calls XHR's abort() method that allows us to drop the socket from the 
browser side. 

Solution:
To get access to Request object, we need to change the async interfaces to 
return com.google.gwt.http.client.Request instead of void. Three files in GWTP 
need to be changed - DispatchAsync, DispatchServiceAsync and DefaultDisptchAsync

Additional Information: Please see discussion thread 
http://groups.google.com/group/gwt-platform/browse_thread/thread/6a3e671cbdc8343
c
The attachment contains the proposed changes to the above files.

Original issue reported on code.google.com by youareh...@gmail.com on 3 Aug 2010 at 3:04

Attachments:

GoogleCodeExporter commented 9 years ago 
You got my vote !

Original comment by goudreau...@gmail.com on 3 Aug 2010 at 1:46

GoogleCodeExporter commented 9 years ago 
Good. I think with this change we may have chances to provide smart dispatchers 
that can control submitting certain actions.
For example, consider this simplified class:
public class ExpensiveActionDispatchAsync extends DefaultDispatchAsync
{
    private Request previousRequestForExpensiveAction;

    //only allow one expensive action at a time
    public <A extends Action<R>, R extends Result> Request execute(final A expensiveAction, final AsyncCallback<R> callback) {
    {
        if(previousRequestForExpensiveAction!=null&&previousRequestForExpensiveAction.isPending())
        {
            previousRequestForExpensiveAction.cancel();
        }
        return previousRequestForExpensiveAction = super.execute(expensiveAction,callback);
    }
}

Original comment by youareh...@gmail.com on 3 Aug 2010 at 10:33

GoogleCodeExporter commented 9 years ago 
I think we must create a dedicated interface to return and not use directly the 
Request object. The Request is implementation-specific. If a day we support 
some other way like XSS, Web Socket... we will be happy to not change again 
this code.

Original comment by olivier....@free.fr on 6 Aug 2010 at 7:44

GoogleCodeExporter commented 9 years ago 
I agree it's a good idea to abstract that into some kind of Request interface.

Original comment by philippe.beaudoin on 7 Aug 2010 at 5:21

GoogleCodeExporter commented 9 years ago 
Here is the generated code found for DispatchService_Proxy.java after returning 
Request object:

  public com.google.gwt.http.client.Request execute(java.lang.String cookieSentByRPC, com.gwtplatform.dispatch.shared.Action action, com.google.gwt.user.client.rpc.AsyncCallback callback) {
    int requestId = getNextRequestId();
    boolean toss = isStatsAvailable() && stats(timeStat("DispatchService_Proxy.execute", requestId, "begin"));
    SerializationStreamWriter streamWriter = createStreamWriter();
    // createStreamWriter() prepared the stream
    try {
      streamWriter.writeString(REMOTE_SERVICE_INTERFACE_NAME);
      streamWriter.writeString("execute");
      streamWriter.writeInt(2);
      streamWriter.writeString("java.lang.String/2004016611");
      streamWriter.writeString("com.gwtplatform.dispatch.shared.Action");
      streamWriter.writeString(cookieSentByRPC);
      streamWriter.writeObject(action);
      String payload = streamWriter.toString();
      toss = isStatsAvailable() && stats(timeStat("DispatchService_Proxy.execute", requestId, "requestSerialized"));
      return doInvoke(ResponseReader.OBJECT, "DispatchService_Proxy.execute", requestId, payload, callback);
    } catch (SerializationException ex) {
      callback.onFailure(ex);
      return new com.google.gwt.user.client.rpc.impl.FailedRequest();
    }
  }

I also like the idea of making a dedicated interface. But looking at the GWT 
generated code above, I feel this abstraction should be outside the dispatch 
interface. In another word, the current interface is built on top of GWT RPC 
and let it be. Thoughts?

Original comment by youareh...@gmail.com on 8 Aug 2010 at 2:42

GoogleCodeExporter commented 9 years ago

Original comment by goudreau...@gmail.com on 14 Aug 2010 at 2:19

GoogleCodeExporter commented 9 years ago 
Ok, there's a complication...
public interface DispatchServiceAsync {
  com.google.gwt.http.client.Request execute( String cookieSentByRPC, Action<?> action, AsyncCallback<Result> callback );

Async Service can only return void, Request or RequestBuilder

Original comment by goudreau...@gmail.com on 14 Aug 2010 at 2:43

GoogleCodeExporter commented 9 years ago 
First returning any one of the three choices will not cause compilation problem 
since we started with returning void. Now the question is returning Request or 
RequestBuilder. I don't see the value of returning RequestBuilder unless its 
api can return the Request it builds. From its api I can not tell that. On the 
other hand, the dispatch framework really focuses on GWT-RPC - not how to use 
RequestBuilder mannually to send GET/POST request. It seems to me returning 
Request sounds right. Thoughts?

Original comment by youareh...@gmail.com on 14 Aug 2010 at 8:35

GoogleCodeExporter commented 9 years ago 
Same though, I already used Request and implemented a DispatchRequest interface 
and a factory for cases like Olivier told.

You can see the CR here:
http://codereview.appspot.com/1980045/show

You can forget de .pref files :D

Original comment by goudreau...@gmail.com on 14 Aug 2010 at 9:00

GoogleCodeExporter commented 9 years ago 
I like it. I confused DispatchAsync interface with DispatchServiceAsync 
interface but now it is clear. The DispatchRequest on the DispatchAsync totally 
makes sense.

Original comment by youareh...@gmail.com on 15 Aug 2010 at 4:52

GoogleCodeExporter commented 9 years ago 
THis has been merged into the trunk. Thanks Christian and Jian!

Original comment by philippe.beaudoin on 16 Aug 2010 at 6:52