Re: File upload progress

Lance wrote:

2009/6/7 Mike Wilson <mikewse@...>

Lance wrote:  

dwr.engine.cancelUpload("myUpload");

</javascript>

engine.js generates an invocationId which is passed to the server which can be used to cancel an upload or poll it's current progress.

2) Generic progress and cancellation

As you mentioned in an earlier mail, it could be interesting to provide a generic progress/cancellation mechanism for all calls, not just for file uploads. I then think it would be natural to return an object from any async call following the Deferred/Future pattern. This object could be used for asking about progress, completion status or to trigger cancellation, and would replace (but possibly contain) the invocationId in your example.

If providing progress for all kinds of requests, then we can not depend on always getting a percentage to the progress callback, as we typically don't know the size of outbound data. The parameters could instead be designed this way:

function(transferredBytes, totalBytes)

and the totalBytes would only be set in the cases when we do have that information (and could easily be converted to a percentage). When totalBytes is undefined, a progress dialog could still show the running transferredBytes count.

Hmm... this locks us into a file upload now rather than a more generic process percentage. I'm not 100% on this but could be talked into it. 

My point was actually the opposite, ie not to lock us into file upload ;-).

Normally, it is only at upload (file or data) that we know the total number of bytes we are going to send and can thus calculate the percentage. If we want to use generic progress handling for large sets of outbound data ("download") then we normally don't know the total size at it is processed in a streaming fashion. This makes the percentage impossible to calculate for this case, and therefore I suggested the "transferred byte count so far" to be used in the progress callback.

 

Also, I believe that it could be interesting for the progress indicator to be able, at will, to show the byte count and not just the percentage. Percentages can easily be derived from the bytes if the total is available.

3) Invocation id

After thinking a fair bit on this I'm a little worried about us taking the responsibility to create a unique id in client code. Joe had the same dilemma for the script session ids and decided to go with server-side generation. Also, we already have the scriptSessionId that is such a unique number that identifies a loaded page in the browser.

The invocationId needs to uniquely identify each call to the server and basing off the scriptSessionId we would just have to add a sequential call index number that we can keep locally in the client layer.

What I just recommended doesn't work if we disable script sessions, and this is a configuration I am personally interested in. Though, if disabling script sessions I think it is ok to require the user code to invent a good enough invocationId. So, by default I think we should use the scriptSessionId algorithm, and then allow it to be overridden in the options object.

Adding my own followup:

I should mention that we already have this sequential number as well; the batch id. So, the combination of scriptSessionId and batchId uniquely identifies every call to the server, including file uploads.

My original invocationId used scriptSessionId + batchId but I moved to a random number to support a DWR mode without script sessions.  

I believe it might have been me asking you not to depend on script sessions, and I think it is good to support other configurations.

Perhaps I could check if scriptSessionId is null and only use the random number in this case.

As I wrote earlier I think we should avoid trying to generate a unique id in the DWR client layer, and leave this to the application code when the default script session based implementation is not used. With this I mean we should not have any random number generation at all in our code, not even as a fallback.

 

About a potentially unset scriptSessionId (because of a fresh page), I think the following behaviour could be appropriate:

 

Default upload:

Remote.uploadFile(dwr.util.byId("myFile"), 

{
    callback: ...,
    progressCallback:  ... 

});

 

Here the default ScriptSession-based incovationId is used, so the client layer needs to know the scriptSessionId before sending along the upload, as we would otherwise not know what invocationId to poll progress about.

Thus, if scriptSessionId has not yet been set we need to send a pre-flight call that retrieves this id (this is always done by the first call from a fresh page, f ex by reverse Ajax polls when using that mode). After the return of that request we can then start the real upload request.

Custom upload:

Remote.uploadFile(dwr.util.byId("myFile"), 

{
    callback: ...,
    progressCallback:  ...,

    invocationId: ...

});

 

Here a custom incovationId is used, so we already know what invocationId to poll for in the progress handling. No scriptSessionId or pre-flight call needed.

Summing these suggestions up, this is how I would expect them to work:

 

Specifying no progress callback means no progress handling will be done. The returned Deferred object allows us to manually check progress or cancel upload:

var deferredInvocation = Remote.uploadFile(
    dwr.util.byId("myFile"),

    {
        callback: ...

    }
);

dwr.engine.getProgress(deferredInvocation) // sends progress check request

dwr.engine.isCompleted(deferredInvocation) // internal flag, no request needed

dwr.engine.cancel(deferredInvocation)

[Some more thought could be put into these method names]

Specifying a progress callback means progress handling will be active (in this case with scriptSessionId algorithm):

var deferredInvocation = Remote.uploadFile(
    dwr.util.byId("myFile"),

    {
        callback: ...,
        progressCallback: function(...) {},

        progressInterval: 500 // msec

    }
);

And finally, also specifying an invocationId means that we override the scriptSessionId algorithm with our own:

var deferredInvocation = Remote.uploadFile(
    dwr.util.byId("myFile"),

    {
        callback: ...,
        progressCallback: function(...) {},

        progressInterval: 500, // msec

        invocationId: "my Internet-wide unique string"

    }
);

This works for me... I originally considered returning an object from a DWR call but wasn't sold on the idea since in sync mode {async: false} DWR returns the remote response. I wasn't sure of the implications here. This will obviously require a bit more work to implement this approach.

Yes, I thought about the sync thing as well. Conceptually though, when you use a sync call you have already said that you want to use JavaScript's single thread to wait for the operation to complete before doing anything else, including updating progress (most browsers also freeze the UI during a sync call).

It is only when you do an async call that you could normally let your single JavaScript thread do other work during the upload, like updating progress.

So I think it is quite "correct", and follows standard patterns, to return a handle to the running operation as a return value only from an async call, and not do this for sync calls (it is natural for sync calls to return the resulting value).

 

Also, normally you cannot use sync mode for file uploads as sync requires XHR (though Firefox has added file upload capabilities that can be used by XHR). 

If the scriptSessionId suggestion above works out, then I guess the default upload manager should store progress on the script session.

It's trivial to plug in a ScriptSessionUploadManager  

Yes, should be.

Another (default) stores to a local map. 

I'm not sure what you mean with "local map"?

I just mean a member variable on the DefaultUploadManager singleton  

Ah, right, the application global version. This is the hardest one to make secure (user code-generated invocationIds must be unique over all current users of the site) so this should not be the default upload manager. 

I remember from a previous discussion that it was suggested that progress could be delivered by Reverse Ajax, if active on the page. I'm not sure how much that would complicate things.

I think that reverse ajax polls are less frequent than what is required to update a progress widget. The two could be combined in the future but I'm not going to attempt this at this stage. 

I think the goal in that previous discussion was to use reverse ajax in streaming mode so updates are transmitted immediately, but within the same request. Ie, a single poll and then pushing progress from the server into the poll request's response stream every 500ms or whatever has been set. This certainly doesn't have to be implemented now, but it would be good if you could take a quick look to see if it will be possible to add this feature within the current public APIs we are designing now.

 

Best regards

Mike