Allow fineuploader to send custom payload for each chunk

[krraghavan]

NEW FEATURE REQUEST

1. Have a server side implementation for receiving chunks on a particular REST endpoint say /file-upload/{id}/chunk where {id} is a unique id assigned to this upload.
2. The implementation of this API requires one part called (say) "chunk-info" of Content-Type application/json which contains an application specific JSON object that describes the chunk being uploaded
3. As fine uploader sends each chunk it adds "chunk-info" for each chunk with the information that the application expects

   Detailed description of the feature

In some server implementations esp using REST APIs with frameworks (like Spring MVC), the REST controllers that handle the upload methods can deserialize the payloads into JSON objects allowing the Java developer to work with POJOs instead of individual form parameters. For example, we want to send a part (say) called "chunk-info" which has a mime type of application/json and contains a JSON object that carries the chunk information. We do not want your client implementation (Fineuploader) dictating our server side API specification. This change will allow the fineuploader client library to be used with any server side implementation with both sides being able to evolve independently

[rnicholus]

Well written feature request, but one thing to keep in mind when sending files: you have two options regarding the payload. One option is to include the entire chunk as the payload. In that case, any parameters must be attached to the request uri (url encoded). The other option is to send a multipart encoded request that includes the file bytes in one part, and any other parameters as separate parts in the multipart encoded request. I'm not sure how that fact fits into or changes your feature request.

Also, it's not clear how you would expect to associate different params with each chunk, if this data is determined before the file starts uploading, that is one thing and may not be too difficult to implement. But if you expect to set params for eac part on the fly, that seems like it would be quite challenging to manage for the user and to implement, for little obvious benefit.

Perhaps you can address all of this with some more details, a set of sample request payloads, and an explanation of how this would work in terms of the API.

[krraghavan]

I will submit a pull request for this in a few days. The way I got this working for us is to:

1. Create a chunkParamsStore and corresponding API method (set and get) on that store to set the parameters to set per chunk. Since my use case was only targeting sending requests using Ajax requests I only changed the xhr method of uploading.
2. Implement the onUploadChunk method in our application that sets these parameters in the store
3. Added these additional parameters in the setParamsAndGetEntityToSend method from the chunkParamsStore

Our server signature (Spring MVC) looks like this. We post the "chunk-info" param with content-type = application/json. ChunkInfoInput is a POJO that contains information about the chunk itself (again our goal is to create a REST API that works with any client - our UI client uses Fineuploader so we don't want to tie our server signature to any specific client). The Blob itself is sent in the "chunk-data" parameter (using the normal Fineuploader capability).

So this change will allow you to send arbitrary JSON (or any mime-type for that matter) with each chunk.

@RequestMapping(method = RequestMethod.POST,
                                path = "/{oid}/chunk",
                                consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
                                produces = {MediaType.APPLICATION_JSON_UTF8_VALUE})
    public ResponseEntity saveChunk(@PathVariable String oid,
                                                          @RequestPart("chunk-info") ChunkInfoInput chunkInfoInput,
                                                          @RequestPart("chunk-data") MultipartFile imageFile,
                                                          UriComponentsBuilder uriComponentsBuilder)  {
// upload handling code here
}

Let me know if you see problems with this approach or if something else needs to be done. I can paste the diff or you can take a look at the diff on my fork (krraghavan/fine-uploader)

[rnicholus]

Does this fundamentally change the way data is sent with each chunk upload? Will a user upgrading to the version of Fine Uploader with your changes have to make any updates to their server code?

[krraghavan]

No it does not. If the chunkParamsStore does not have any data for the chunk nothing happens. Since the only way for this store to have something is through custom client code (via the onUpLoad method) nothing else should be impacted.

[ddileep]

Pls let me know if you're ok with this - I can generate a Pull request to merge it.

[krraghavan]

Sorry sent that last comment as a different user. Please let me know if I should generate a PR. I'm kind of hoping it can get into 5.10 so I can use the official version of fineuploader in our code.

[rnicholus]

5.10 has already been released. I'm not sure about this change yet. Unless it has obvious benefit to a large number of users, it's unlikely to make it into a near future release. As you can see from the issue tracker, there are a lot of feature requests still open. I generally prioritize the ones that have are most appealing to the user base as a whole. Features that are specific to an uncommon workflow or set of requirements are usually passed over in favor of more popular features. Also, the library is arguably bigger than it should be already, and I'm hesitant to add features that will make the library larger or more complex.

My time is limited, and each new feature or change will eventually result in more support requests and maintenance needs. Even though all new features must include updates to documentation and unit tests, this is still the case.

My time is currently gravitating towards a library for Fine Uploader that will make it much easier and more elegant to integrate into a react app.

[krraghavan]

Thanks for the response. I respectfully disagree that this is an uncommon workflow - REST API signatures should not be dictated by client capabilities. Unfortunately most JavaScript client implementations have chosen to go with a simple approach. Google in fact defines the API in terms of multipart payloads that can take JSON but their client implementation does not. Fineuploader was one library I thought had the right architecture to allow this capability without any complications (my PR is only changing 4 files and < 100 lines of code to provide this functionality which will allow server implementations to be decoupled from client implementations).

I do agree with your comment on support/maintenance and am happy to help out as needed but I don't really know/understand the burden adding a new capability like this puts on you.

[rnicholus]

Please do submit a pull request so we can discuss further. I will likely be able to spot issues with your code in the context of the library and its users. Also, I have a hard requirement that all new features must include documentation updates to properly explain to new users how to make use of the new feature, as well as unit tests. I don't promise that any feature will make it into the library quickly, even if it is scheduled for a near future release. I like to take time to think over all changes to the library. I find this helps to keep it stable and prevent breaking changes later.

[rnicholus]

Also, due to the complexity of the library, I would consider a change of about 100 lines to be a big one. Anything you can do to simplify and reduce changes will be helpful. This is often where my insight into the code base comes in handy though.

[krraghavan]

Thx - also just checked my commit and it's 31 lines of code to provide this new functionality. Will look into the testing and documentation comment you mention above. Sending a PR as well for your comments.

[rnicholus]

I'll be happy to assist any way I can during review of the PR.

[LusciousPear]

Just adding my $0.02 as an App Engine user, this would be pretty helpful.

[rnicholus]

@LusciousPear Check out #1597. It needs unit test work, and I have yet to review the changes.

[krraghavan]

I have fixed the unit tests so the build is now passing. Please review my Pull request and let me know if you need me to do anything else. Raghavan

On Tue, Sep 6, 2016 at 1:30 PM, Ray Nicholus notifications@github.com wrote:

@LusciousPear https://github.com/LusciousPear Check out #1597 https://github.com/FineUploader/fine-uploader/pull/1597. It needs unit test work, and I have yet to review the changes.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/FineUploader/fine-uploader/issues/1591#issuecomment-245080610, or mute the thread https://github.com/notifications/unsubscribe-auth/AGe9nqAFW3gM-aGxl4Zza1o9VRgDEudUks5qnc1ngaJpZM4I5Rie .

[krraghavan]

Hi Ray, Tests were passing and it's been a while so pinging you again. I need this feature now for another project as well and would prefer to use the released version rather than my fork. Pls let me know if I need to do anything more to move the ball forward.

[rnicholus]

Thanks for the reminder. I've added an alert to my TODO list so this doesn't fall off my radar again. However, it may be at least a week before I can look at this closely. I'll see if I can fit it in sooner.

[manvesh]

Do you have any updates on this? I would like to use this feature in our project.

[rnicholus]

@manvesh Check out #1597. It needs unit test work, and I have yet to review the changes.