Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

support callback which will only called once before upload #208

Closed
oeway opened this issue Aug 30, 2016 · 10 comments
Closed

support callback which will only called once before upload #208

oeway opened this issue Aug 30, 2016 · 10 comments
Milestone

Comments

@oeway
Copy link

oeway commented Aug 30, 2016

As mentioned in #207, currently, onBeforeUpload will called on each chunk upload, which could raise potential performance drop if some expensive operation is contained in onBeforeUpload. Would be great to support callback which would only execute once, either by change the current onBeforeUpload or add something like onInitiateUpload.

@dr-dimitru dr-dimitru added this to the v.Next milestone Aug 30, 2016
@dr-dimitru
Copy link
Member

I'll stay with onInitiateUpload. As changing onBeforeUpload current behaviour, may break some current usage, or will request Major version release

dr-dimitru added a commit that referenced this issue Aug 30, 2016
 - Documented as `onInitiateUpload` in
[Constructor](https://github.com/VeliovGroup/Meteor-Files/wiki/Construct
or)
@dr-dimitru dr-dimitru mentioned this issue Aug 30, 2016
dr-dimitru added a commit that referenced this issue Aug 30, 2016
v1.7.2
 - Fix #207 
 - Fix availability of `this.userId` in `onBeforeUpload` hook
 - `this.userId` now always `null` if user not logged in, previously
sometimes it might be `undefined`
 - Implement #208 : Documented as `onInitiateUpload` in [Constructor](https://github.com/VeliovGroup/Meteor-Files/wiki/Constructor)
 - Fix mime-type in base64 uploads, thanks to @FinnFrotscher
@dr-dimitru
Copy link
Member

Implemented in v1.7.2

@oeway
Copy link
Author

oeway commented Aug 30, 2016

@dr-dimitru It worked, but it seems that the returned value from onInitiateUpload is not used to reject a upload?

@oeway
Copy link
Author

oeway commented Aug 30, 2016

However, I can throw new Meteor.Error('not-authorized'); in onInitiateUpload to stop the upload.

@dr-dimitru
Copy link
Member

dr-dimitru commented Aug 30, 2016

Why not use onBeforeUpload to control/stop/continue upload?
onInitiateUpload is create for purpose you've described - if some expensive operation is contained. And it's asynchronous, throwing an exception will stop nodejs process, but it might be somewhere in the middle or at the end of upload

@livimonte
Copy link

It would be better to make the file validation (check weight and extension for example) in onInitiateUpload instead of onBeforeUpload? Since that runs only once and before starting the upload.

@dr-dimitru
Copy link
Member

This will be removed in v2.0
As it comes from early stages of this package it may break someone's integration

It was done this way to protect unfinished uploads from fraud actions. As previously was no control of uploaded size, currently any size overflow will abort upload.

@HungryJake
Copy link

I've looked into the code, seems like onInitiateUpload is not being called if onBeforeUpload is not being defined, is this done intentionally?

@dr-dimitru
Copy link
Member

@livimonte onBeforeUpload is made this way to avoid file mitigation during upload

@dr-dimitru
Copy link
Member

dr-dimitru commented May 18, 2017

@DrkCoater you right, this shouldn't be this way.
Fix will be released soon see this commit, thank you

dr-dimitru added a commit that referenced this issue May 18, 2017
 - Call `onInitiateUpload` even if `onBeforeUpload` is not defined, see
[this
comment](#208 (comment)
ment-301208873), thanks to @DrkCoater
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants