Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

core: Restrictions improvements #1726

Merged
merged 5 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 40 additions & 34 deletions packages/@uppy/core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class Uppy {

this.log(`Using Core v${this.constructor.VERSION}`)

if (this.opts.restrictions.allowedFileTypes &&
this.opts.restrictions.allowedFileTypes !== null &&
!Array.isArray(this.opts.restrictions.allowedFileTypes)) {
throw new Error(`'restrictions.allowedFileTypes' must be an array`)
}

// i18n
this.translator = new Translator([ this.defaultLocale, this.opts.locale ])
this.locale = this.translator.locale
Expand Down Expand Up @@ -355,10 +361,10 @@ class Uppy {
}

/**
* Check if minNumberOfFiles restriction is reached before uploading.
*
* @private
*/
* Check if minNumberOfFiles restriction is reached before uploading.
*
* @private
*/
_checkMinNumberOfFiles (files) {
const { minNumberOfFiles } = this.opts.restrictions
if (Object.keys(files).length < minNumberOfFiles) {
Expand All @@ -367,12 +373,12 @@ class Uppy {
}

/**
* Check if file passes a set of restrictions set in options: maxFileSize,
* maxNumberOfFiles and allowedFileTypes.
*
* @param {Object} file object to check
* @private
*/
* Check if file passes a set of restrictions set in options: maxFileSize,
* maxNumberOfFiles and allowedFileTypes.
*
* @param {Object} file object to check
* @private
*/
_checkRestrictions (file) {
const { maxFileSize, maxNumberOfFiles, allowedFileTypes } = this.opts.restrictions

Expand All @@ -384,8 +390,6 @@ class Uppy {

if (allowedFileTypes) {
const isCorrectFileType = allowedFileTypes.some((type) => {
// if (!file.type) return false

// is this is a mime-type
if (type.indexOf('/') > -1) {
if (!file.type) return false
Expand Down Expand Up @@ -414,12 +418,12 @@ class Uppy {
}

/**
* Add a new file to `state.files`. This will run `onBeforeFileAdded`,
* try to guess file type in a clever way, check file against restrictions,
* and start an upload if `autoProceed === true`.
*
* @param {Object} file object to add
*/
* Add a new file to `state.files`. This will run `onBeforeFileAdded`,
* try to guess file type in a clever way, check file against restrictions,
* and start an upload if `autoProceed === true`.
*
* @param {Object} file object to add
*/
addFile (file) {
const { files, allowNewUpload } = this.getState()

Expand All @@ -434,6 +438,9 @@ class Uppy {
onError(new Error('Cannot add new files: already uploading.'))
}

const fileType = getFileType(file)
file.type = fileType

const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(file, files)

if (onBeforeFileAddedResult === false) {
Expand All @@ -442,14 +449,9 @@ class Uppy {
}

if (typeof onBeforeFileAddedResult === 'object' && onBeforeFileAddedResult) {
// warning after the change in 0.24
if (onBeforeFileAddedResult.then) {
throw new TypeError('onBeforeFileAdded() returned a Promise, but this is no longer supported. It must be synchronous.')
}
file = onBeforeFileAddedResult
}

const fileType = getFileType(file)
let fileName
if (file.name) {
fileName = file.name
Expand Down Expand Up @@ -510,7 +512,9 @@ class Uppy {
this.scheduledAutoProceed = setTimeout(() => {
this.scheduledAutoProceed = null
this.upload().catch((err) => {
console.error(err.stack || err.message || err)
if (!err.isRestriction) {
this.uppy.log(err.stack || err.message || err)
}
})
}, 4)
}
Expand Down Expand Up @@ -1264,6 +1268,14 @@ class Uppy {
* @returns {Promise}
*/
upload () {
const onError = (err) => {
const message = typeof err === 'object' ? err.message : err
const details = (typeof err === 'object' && err.details) ? err.details : ''
this.log(`${message} ${details}`)
this.info({ message: message, details: details }, 'error', 5000)
throw (typeof err === 'object' ? err : new Error(err))
}

if (!this.plugins.uploader) {
this.log('No uploader type plugins are used', 'warning')
}
Expand All @@ -1276,11 +1288,6 @@ class Uppy {
}

if (onBeforeUploadResult && typeof onBeforeUploadResult === 'object') {
// warning after the change in 0.24
if (onBeforeUploadResult.then) {
throw new TypeError('onBeforeUpload() returned a Promise, but this is no longer supported. It must be synchronous.')
}

files = onBeforeUploadResult
}

Expand All @@ -1304,11 +1311,10 @@ class Uppy {
return this._runUpload(uploadID)
})
.catch((err) => {
const message = typeof err === 'object' ? err.message : err
const details = typeof err === 'object' ? err.details : null
this.log(`${message} ${details}`)
this.info({ message: message, details: details }, 'error', 4000)
return Promise.reject(typeof err === 'object' ? err : new Error(err))
if (err.isRestriction) {
this.emit('restriction-failed', null, err)
goto-bus-stop marked this conversation as resolved.
Show resolved Hide resolved
}
onError(err)
})
}
}
Expand Down
13 changes: 13 additions & 0 deletions packages/@uppy/core/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,19 @@ describe('src/Core', () => {
}
})

it('should throw if allowedFileTypes is not an array', () => {
try {
const core = Core({
restrictions: {
allowedFileTypes: 'image/gif'
}
})
core.log('hi')
} catch (err) {
expect(err).toMatchObject(new Error(`'restrictions.allowedFileTypes' must be an array`))
}
})

it('should enforce the allowedFileTypes rule with file extensions', () => {
const core = new Core({
restrictions: {
Expand Down
8 changes: 0 additions & 8 deletions packages/@uppy/dashboard/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,6 @@ module.exports = class Dashboard extends Plugin {
this.superFocusOnEachUpdate()
}

startUpload = (ev) => {
this.uppy.upload().catch((err) => {
// Log error.
this.uppy.log(err.stack || err.message || err)
})
}

cancelUpload = (fileID) => {
this.uppy.removeFile(fileID)
}
Expand Down Expand Up @@ -803,7 +796,6 @@ module.exports = class Dashboard extends Plugin {
metaFields: pluginState.metaFields,
resumableUploads: capabilities.resumableUploads || false,
individualCancellation: capabilities.individualCancellation,
startUpload: this.startUpload,
pauseUpload: this.uppy.pauseResume,
retryUpload: this.uppy.retryUpload,
cancelUpload: this.cancelUpload,
Expand Down
8 changes: 4 additions & 4 deletions packages/@uppy/status-bar/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ module.exports = class StatusBar extends Plugin {
this.translator = new Translator([ this.defaultLocale, this.uppy.locale, this.opts.locale ])
this.i18n = this.translator.translate.bind(this.translator)

this.startUpload = this.startUpload.bind(this)
this.render = this.render.bind(this)
this.install = this.install.bind(this)
}
Expand All @@ -97,10 +96,11 @@ module.exports = class StatusBar extends Plugin {
return Math.round(totalBytesRemaining / totalSpeed * 10) / 10
}

startUpload () {
startUpload = () => {
return this.uppy.upload().catch((err) => {
this.uppy.log(err.stack || err.message || err)
// Ignore
if (!err.isRestriction) {
this.uppy.log(err.stack || err.message || err)
}
})
}

Expand Down