Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Feature/flexible upload support #1939

Merged
merged 51 commits into from
Mar 9, 2018
Merged

Conversation

rnicholus
Copy link
Member

@rnicholus rnicholus commented Nov 8, 2017

rnicholus and others added 30 commits September 11, 2017 16:20
- Manual tests w/ chunking and concurrent chunking look good.

TODO:
- handle resolve result
- handle failure
- unit tests
- docs uppdate
- typescript defs update
Use case: When onUpload is called, you make a request to your server to see if the file already exists. If it does, you want to let your user decide if they want to overwrite the file, or cancel the upload entirely. While waiting for user input you don't want to hold a spot in the upload queue. If the user decided to overwrite the file, call the `continueUpload` API method.
closes #848
chunking.partSize now accepts a function, which passes the file ID and size
closes #1697
@rnicholus
Copy link
Member Author

rnicholus commented Dec 18, 2017

Note that I cannot guarantee that this will not break S3 or Azure workflows as I have no longer have a way to run manual tests against either of these (even S3) anymore.

# Conflicts:
#	client/js/version.js
#	package.json
@rnicholus rnicholus requested review from singhjusraj and removed request for singhjusraj December 18, 2017 04:39
@singhjusraj
Copy link
Member

@rnicholus do this needs to be reviewed by someone other than you in order to merge?

@rnicholus
Copy link
Member Author

a review would be nice, but I am mostly waiting for my own internal testing in a closed source app to reveal any issues before releasing. I am unable to test against S3, so if you or anyone can do that, this would be hlepful.

@rnicholus
Copy link
Member Author

rnicholus commented Dec 20, 2017

Something that came up during manual/internal testing in a real-world app that's using v5.16.0-beta.0: the fact that you can cancel or pause an upload when Fine Uploader is waiting for a chunking success XHR response is problematic. Integrations, such as React Fine Uploader, see that the upload is not yet complete, and that's true, so pause/cancel buttons are rendered. Furthermore, the API accepts pause/cancel commands while in this state.

Why is this bad? Well, it's not guaranteed that the request can be cancelled before the file has been "finalized" on the server fielding the request. So, in the case of a pause, a subsequent resume may result in an error. And if the upload is cancelled at this point, it may still be considered "completed" on the server. So the library should probably be updated as follows:

  1. Reject cancel and pause API calls while in this state.
  2. Broadcast a new status change event (such as qq.status.UPLOAD_FINALIZING). This will be a signal to integrators that the last byte has been sent, and they can react accordingly. Responses to this state may include removing the cancel/pause buttons and rendering a custom status message ("your file is being verified by the server").

@xistext
Copy link

xistext commented Jan 4, 2018

When do you think this branch might be merged? I was hoping to be able to use fine-uploader to access s3 with federated identities but have been stuck for a couple weeks. It seems like the AWS signed urls might be the ticket. Do you take donations to help with motivation? Thanks!

@rnicholus
Copy link
Member Author

@xistext I'm sorry - i cannot accept personal donations to influence my time on this project. I'm quite focused on my "day" job at the moment, which is demanding of my time and also spawned all of the substantial changes you see in this PR. I am however actively looking for others to help me regularly maintain this project.

That said, you can try all of this out now - it's published to npm and I'm using it on a closed source project. The latest release on npm is 5.16.0-RC1, and the description of this PR is updated as the version changes. Once I'm satisfied with the performance in my closed source project, i'll officially release. Please do let me know if you see any issues or have questions though.

P.S. The S3 demo on fineuploader.com/demos has been dead for a while as I don't want to continue paying the AWS bills myself (~$40/month). So if you or anyone else would like to take on this support, that would be helpful.

@xistext
Copy link

xistext commented Jan 4, 2018

I have installed 5.16.0-RC1. How do I tell it not to use the signature endpoint but instead to use the signed url? I am trying to use the url generated by s3.getSignedUrl as the request endpoint. Thanks!

@rnicholus
Copy link
Member Author

Pretty much all of these changes focus on the traditional non-s3/azure uploaders. Note that onUpload and onUploadChunk events both accept a Promise as a return value. You can either use whichever methods you choose in onUpload to influence the file to be uploaded, or you can do so in each onUploadChunk handler by resolving your Promise with and object that contains an endpoint URI.

@rnicholus
Copy link
Member Author

@xistext
Copy link

xistext commented Jan 4, 2018

I tried setting the signature endpoint as the docs specify '/s3/signature' but get an error 'Failed to load resource: the server responded with a status of 405 (Method Not Allowed)' on that url when I upload my image. I have tried setting version:4 or not but saw no difference. Thanks for your replies.

@bhavikup
Copy link

bhavikup commented Jan 5, 2018

Hello @rnicholus,

Im interested in using the per-file chunk sizes in my project with a stable version of fineuploader. I am thinking of forking the latest stable version and adding the commits which relate to that feature. Would something like this be possible? Which commits beyond 8eb98d7 and 53b397d relate to this feature? I have been using this feature in development and it seems to work well.

Thanks for your work on this project

@uriahcarpenter
Copy link

I may be able to provide some delegated S3 buckets from our corporate account to keep smoke tests viable. PM me if interested.

@rnicholus
Copy link
Member Author

Thanks @uriahcarpenter. Will do.

@rnicholus
Copy link
Member Author

@bhupadhy is the current RC1 working will for you?

@bhavikup
Copy link

@rnicholus ,

I am currently using RC1 and I haven't ran into any issues yet.

@rnicholus rnicholus mentioned this pull request Feb 12, 2018
5 tasks
@rnicholus
Copy link
Member Author

I'm planning to release this as v5.16.0 very soon. I'd like to fit in this update as well - #1939 (comment) - but I don't believe this is a regression, and may just push that out afterwards as v5.16.1.

@rnicholus rnicholus merged commit 5be9ba1 into master Mar 9, 2018
@rnicholus rnicholus deleted the feature/flexible-upload-support branch March 9, 2018 05:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants