diff --git a/examples/custom-provider/client/MyCustomProvider.jsx b/examples/custom-provider/client/MyCustomProvider.jsx index 44eefb5966..01aa0a6d7b 100644 --- a/examples/custom-provider/client/MyCustomProvider.jsx +++ b/examples/custom-provider/client/MyCustomProvider.jsx @@ -27,6 +27,8 @@ export default class MyCustomProvider extends UIPlugin { pluginId: this.id, }) + uppy.registerRequestClient(MyCustomProvider.name, this.provider) + this.defaultLocale = { strings: { pluginNameMyUnsplash: 'MyUnsplash', diff --git a/packages/@uppy/aws-s3-multipart/src/index.js b/packages/@uppy/aws-s3-multipart/src/index.js index 0acac85986..a2ddf19f54 100644 --- a/packages/@uppy/aws-s3-multipart/src/index.js +++ b/packages/@uppy/aws-s3-multipart/src/index.js @@ -878,7 +878,7 @@ export default class AwsS3Multipart extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = file.remote.requestClient.uploadRemoteFile( + const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile( file, this.#getCompanionClientArgs(file), { signal: controller.signal, getQueue }, diff --git a/packages/@uppy/aws-s3/src/index.js b/packages/@uppy/aws-s3/src/index.js index 5aa4a0cdbe..230ebb7864 100644 --- a/packages/@uppy/aws-s3/src/index.js +++ b/packages/@uppy/aws-s3/src/index.js @@ -281,7 +281,7 @@ export default class AwsS3 extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = file.remote.requestClient.uploadRemoteFile( + const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile( file, this.#getCompanionClientArgs(file), { signal: controller.signal, getQueue }, diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index 82470ef0ec..dedb36e66a 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/packages/@uppy/core/src/Uppy.js @@ -1386,6 +1386,30 @@ class Uppy { } } + // We need to store request clients by a unique ID, so we can share RequestClient instances across files + // this allows us to do rate limiting and synchronous operations like refreshing provider tokens + // example: refreshing tokens: if each file has their own requestclient, + // we don't have any way to synchronize all requests in order to + // - block all requests + // - refresh the token + // - unblock all requests and allow them to run with a the new access token + // back when we had a requestclient per file, once an access token expired, + // all 6 files would go ahead and refresh the token at the same time + // (calling /refresh-token up to 6 times), which will probably fail for some providers + #requestClientById = new Map() + + registerRequestClient(id, client) { + this.#requestClientById.set(id, client) + } + + /** @protected */ + getRequestClientForFile (file) { + if (!file.remote) throw new Error(`Tried to get RequestClient for a non-remote file ${file.id}`) + const requestClient = this.#requestClientById.get(file.remote.requestClientId) + if (requestClient == null) throw new Error(`requestClientId "${file.remote.requestClientId}" not registered for file "${file.id}"`) + return requestClient + } + /** * Restore an upload by its ID. */ diff --git a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx index ef79b256b6..acb5711753 100644 --- a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx +++ b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx @@ -75,6 +75,8 @@ export default class ProviderView extends View { isSearchVisible: false, currentSelection: [], }) + + this.registerRequestClient() } // eslint-disable-next-line class-methods-use-this @@ -399,7 +401,7 @@ export default class ProviderView extends View { // finished all async operations before we add any file // see https://github.com/transloadit/uppy/pull/4384 this.plugin.uppy.log('Adding files from a remote provider') - this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file))) + this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file, this.requestClientId))) this.plugin.setPluginState({ filterInput: '' }) messages.forEach(message => this.plugin.uppy.info(message)) diff --git a/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx b/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx index 48a64a3605..a37b7c8cbb 100644 --- a/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx +++ b/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx @@ -54,6 +54,8 @@ export default class SearchProviderView extends View { // Set default state for the plugin this.plugin.setPluginState(this.defaultState) + + this.registerRequestClient() } // eslint-disable-next-line class-methods-use-this diff --git a/packages/@uppy/provider-views/src/View.js b/packages/@uppy/provider-views/src/View.js index 7c49d5ef79..6029e8c984 100644 --- a/packages/@uppy/provider-views/src/View.js +++ b/packages/@uppy/provider-views/src/View.js @@ -57,6 +57,11 @@ export default class View { uppy.info({ message, details: error.toString() }, 'error', 5000) } + registerRequestClient() { + this.requestClientId = this.provider.provider; + this.plugin.uppy.registerRequestClient(this.requestClientId, this.provider) + } + // todo document what is a "tagFile" or get rid of this concept getTagFile (file) { const tagFile = { @@ -78,15 +83,10 @@ export default class View { }, providerName: this.provider.name, provider: this.provider.provider, + requestClientId: this.requestClientId, }, } - // all properties on this object get saved into the Uppy store. - // Some users might serialize their store (for example using JSON.stringify), - // or when using Golden Retriever it will serialize state into e.g. localStorage. - // However RequestClient is not serializable so we need to prevent it from being serialized. - Object.defineProperty(tagFile.remote, 'requestClient', { value: this.provider, enumerable: false }) - const fileType = getFileType(tagFile) // TODO Should we just always use the thumbnail URL if it exists? diff --git a/packages/@uppy/tus/src/index.js b/packages/@uppy/tus/src/index.js index 09829d61cb..694d7afcb7 100644 --- a/packages/@uppy/tus/src/index.js +++ b/packages/@uppy/tus/src/index.js @@ -489,7 +489,7 @@ export default class Tus extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = file.remote.requestClient.uploadRemoteFile( + const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile( file, this.#getCompanionClientArgs(file), { signal: controller.signal, getQueue }, diff --git a/packages/@uppy/url/src/Url.jsx b/packages/@uppy/url/src/Url.jsx index 7c3ff32ea7..a4889a2e0f 100644 --- a/packages/@uppy/url/src/Url.jsx +++ b/packages/@uppy/url/src/Url.jsx @@ -48,6 +48,7 @@ function getFileNameFromUrl (url) { const { pathname } = new URL(url) return pathname.substring(pathname.lastIndexOf('/') + 1) } + /** * Url * @@ -55,6 +56,8 @@ function getFileNameFromUrl (url) { export default class Url extends UIPlugin { static VERSION = packageJson.version + static requestClientId = Url.name + constructor (uppy, opts) { super(uppy, opts) this.id = this.opts.id || 'Url' @@ -88,6 +91,8 @@ export default class Url extends UIPlugin { companionHeaders: this.opts.companionHeaders, companionCookiesRule: this.opts.companionCookiesRule, }) + + this.uppy.registerRequestClient(Url.requestClientId, this.client) } getMeta (url) { @@ -132,11 +137,10 @@ export default class Url extends UIPlugin { fileId: url, url, }, + requestClientId: Url.requestClientId, }, } - Object.defineProperty(tagFile.remote, 'requestClient', { value: this.client, enumerable: false }) - this.uppy.log('[Url] Adding remote file') try { return this.uppy.addFile(tagFile) diff --git a/packages/@uppy/xhr-upload/src/index.js b/packages/@uppy/xhr-upload/src/index.js index 0a0029d31c..a22ccc7514 100644 --- a/packages/@uppy/xhr-upload/src/index.js +++ b/packages/@uppy/xhr-upload/src/index.js @@ -473,7 +473,7 @@ export default class XHRUpload extends BasePlugin { } this.uppy.on('file-removed', removedHandler) - const uploadPromise = file.remote.requestClient.uploadRemoteFile( + const uploadPromise = this.uppy.getRequestClientForFile(file).uploadRemoteFile( file, this.#getCompanionClientArgs(file), { signal: controller.signal, getQueue },