diff --git a/e2e/clients/dashboard-aws-multipart/app.js b/e2e/clients/dashboard-aws-multipart/app.js index 67a743ae87..d4a3f9f015 100644 --- a/e2e/clients/dashboard-aws-multipart/app.js +++ b/e2e/clients/dashboard-aws-multipart/app.js @@ -11,16 +11,6 @@ const uppy = new Uppy() limit: 2, companionUrl: process.env.VITE_COMPANION_URL, shouldUseMultipart: true, - // This way we can test that the user provided API still works - // as expected in the flow. We call the default internal function for this, - // otherwise we would have to run another server to pre-sign requests - // and we don't care about that, just that the flow works. - async prepareUploadParts (file, { uploadId, key, parts, signal }) { - const { number: partNumber, chunk: body } = parts[0] - const plugin = uppy.getPlugin('AwsS3Multipart') - const { url } = await plugin.signPart(file, { uploadId, key, partNumber, body, signal }) - return { presignedUrls: { [partNumber]: url } } - }, }) // Keep this here to access uppy in tests diff --git a/packages/@uppy/aws-s3/src/index.test.ts b/packages/@uppy/aws-s3/src/index.test.ts index 97f1b45904..bddbc19923 100644 --- a/packages/@uppy/aws-s3/src/index.test.ts +++ b/packages/@uppy/aws-s3/src/index.test.ts @@ -117,159 +117,17 @@ describe('AwsS3Multipart', () => { }), completeMultipartUpload: vi.fn(async () => ({ location: 'test' })), abortMultipartUpload: vi.fn(), - prepareUploadParts: vi.fn( - async (file, { parts }: { parts: { number: number }[] }) => { - const presignedUrls: Record = {} - parts.forEach(({ number }) => { - presignedUrls[number] = - `https://bucket.s3.us-east-2.amazonaws.com/test/upload/multitest.dat?partNumber=${number}&uploadId=6aeb1980f3fc7ce0b5454d25b71992&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIATEST%2F20210729%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20210729T014044Z&X-Amz-Expires=600&X-Amz-SignedHeaders=host&X-Amz-Signature=test` - }) - return { presignedUrls, headers: { 1: { 'Content-MD5': 'foo' } } } - }, - ), + signPart: vi.fn(async (file, { number }) => { + return { + url: `https://bucket.s3.us-east-2.amazonaws.com/test/upload/multitest.dat?partNumber=${number}&uploadId=6aeb1980f3fc7ce0b5454d25b71992&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIATEST%2F20210729%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20210729T014044Z&X-Amz-Expires=600&X-Amz-SignedHeaders=host&X-Amz-Signature=test`, + headers: number === 1 ? { 'Content-MD5': 'foo' } : undefined, + } + }), listParts: undefined as any, }) awsS3Multipart = core.getPlugin('AwsS3Multipart') as any }) - it('Calls the prepareUploadParts function totalChunks / limit times', async () => { - const scope = nock( - 'https://bucket.s3.us-east-2.amazonaws.com', - ).defaultReplyHeaders({ - 'access-control-allow-headers': '*', - 'access-control-allow-method': 'PUT', - 'access-control-allow-origin': '*', - 'access-control-expose-headers': 'ETag, Content-MD5', - }) - // 6MB file will give us 2 chunks, so there will be 2 PUT and 2 OPTIONS - // calls to the presigned URL from 1 prepareUploadParts calls - const fileSize = 5 * MB + 1 * MB - - scope - .options((uri) => - uri.includes('test/upload/multitest.dat?partNumber=1'), - ) - .reply(function replyFn() { - expect(this.req.headers['access-control-request-headers']).toEqual( - 'Content-MD5', - ) - return [200, ''] - }) - scope - .options((uri) => - uri.includes('test/upload/multitest.dat?partNumber=2'), - ) - .reply(function replyFn() { - expect( - this.req.headers['access-control-request-headers'], - ).toBeUndefined() - return [200, ''] - }) - scope - .put((uri) => uri.includes('test/upload/multitest.dat?partNumber=1')) - .reply(200, '', { ETag: 'test1' }) - scope - .put((uri) => uri.includes('test/upload/multitest.dat?partNumber=2')) - .reply(200, '', { ETag: 'test2' }) - - core.addFile({ - source: 'vi', - name: 'multitest.dat', - type: 'application/octet-stream', - data: new File([new Uint8Array(fileSize)], '', { - type: 'application/octet-stream', - }), - }) - - await core.upload() - - expect( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls.length, - ).toEqual(2) - - scope.done() - }) - - it('Calls prepareUploadParts with a Math.ceil(limit / 2) minimum, instead of one at a time for the remaining chunks after the first limit batch', async () => { - const scope = nock( - 'https://bucket.s3.us-east-2.amazonaws.com', - ).defaultReplyHeaders({ - 'access-control-allow-headers': '*', - 'access-control-allow-method': 'PUT', - 'access-control-allow-origin': '*', - 'access-control-expose-headers': 'ETag', - }) - // 50MB file will give us 10 chunks, so there will be 10 PUT and 10 OPTIONS - // calls to the presigned URL from 3 prepareUploadParts calls - // - // The first prepareUploadParts call will be for 5 parts, the second - // will be for 3 parts, the third will be for 2 parts. - const fileSize = 50 * MB - - scope - .options((uri) => uri.includes('test/upload/multitest.dat')) - .reply(200, '') - scope - .put((uri) => uri.includes('test/upload/multitest.dat')) - .reply(200, '', { ETag: 'test' }) - scope.persist() - - core.addFile({ - source: 'vi', - name: 'multitest.dat', - type: 'application/octet-stream', - data: new File([new Uint8Array(fileSize)], '', { - type: 'application/octet-stream', - }), - }) - - await core.upload() - - function validatePartData( - { parts }: { parts: { number: number; chunk: unknown }[] }, - expected: number[], - ) { - expect(parts.map((part) => part.number)).toEqual(expected) - - for (const part of parts) { - expect(part.chunk).toBeDefined() - } - } - - expect( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls.length, - ).toEqual(10) - - validatePartData( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls[0][1], - [1], - ) - validatePartData( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls[1][1], - [2], - ) - validatePartData( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls[2][1], - [3], - ) - - const completeCall = (awsS3Multipart.opts as any).completeMultipartUpload - .mock.calls[0][1] - - expect(completeCall.parts).toEqual([ - { ETag: 'test', PartNumber: 1 }, - { ETag: 'test', PartNumber: 2 }, - { ETag: 'test', PartNumber: 3 }, - { ETag: 'test', PartNumber: 4 }, - { ETag: 'test', PartNumber: 5 }, - { ETag: 'test', PartNumber: 6 }, - { ETag: 'test', PartNumber: 7 }, - { ETag: 'test', PartNumber: 8 }, - { ETag: 'test', PartNumber: 9 }, - { ETag: 'test', PartNumber: 10 }, - ]) - }) - it('Keeps chunks marked as busy through retries until they complete', async () => { const scope = nock( 'https://bucket.s3.us-east-2.amazonaws.com', @@ -364,9 +222,9 @@ describe('AwsS3Multipart', () => { } } - expect( - (awsS3Multipart.opts as any).prepareUploadParts.mock.calls.length, - ).toEqual(10) + expect((awsS3Multipart.opts as any).signPart.mock.calls.length).toEqual( + 10, + ) }) }) @@ -623,7 +481,7 @@ describe('AwsS3Multipart', () => { await core.upload() expect(createMultipartUpload).toHaveBeenCalled() - expect(signPart).toHaveBeenCalledTimes(10) + expect(signPart).toHaveBeenCalledTimes(11) expect(completeMultipartUpload).toHaveBeenCalled() }) diff --git a/packages/@uppy/aws-s3/src/index.ts b/packages/@uppy/aws-s3/src/index.ts index 6e0aa32f70..4b84221489 100644 --- a/packages/@uppy/aws-s3/src/index.ts +++ b/packages/@uppy/aws-s3/src/index.ts @@ -197,25 +197,6 @@ type AWSS3MultipartWithoutCompanionMandatorySignPart< opts: SignPartOptions, ) => MaybePromise } -/** @deprecated Use signPart instead */ -type AWSS3MultipartWithoutCompanionMandatoryPrepareUploadParts< - M extends Meta, - B extends Body, -> = { - /** @deprecated Use signPart instead */ - prepareUploadParts: ( - file: UppyFile, - partData: { - uploadId: string - key: string - parts: [{ number: number; chunk: Blob }] - signal?: AbortSignal - }, - ) => MaybePromise<{ - presignedUrls: Record - headers?: Record> - }> -} type AWSS3MultipartWithoutCompanionMandatory = { getChunkSize?: (file: UppyFile) => number createMultipartUpload: (file: UppyFile) => MaybePromise @@ -236,10 +217,7 @@ type AWSS3MultipartWithoutCompanionMandatory = { signal: AbortSignal }, ) => MaybePromise<{ location?: string }> -} & ( - | AWSS3MultipartWithoutCompanionMandatorySignPart - | AWSS3MultipartWithoutCompanionMandatoryPrepareUploadParts -) +} & AWSS3MultipartWithoutCompanionMandatorySignPart type AWSS3MultipartWithoutCompanion< M extends Meta, @@ -387,33 +365,6 @@ export default class AwsS3Multipart< ) } } - if ( - (opts as AWSS3MultipartWithoutCompanionMandatoryPrepareUploadParts) - ?.prepareUploadParts != null && - (opts as AWSS3MultipartWithoutCompanionMandatorySignPart) - .signPart == null - ) { - this.opts.signPart = async ( - file: UppyFile, - { uploadId, key, partNumber, body, signal }: SignPartOptions, - ) => { - const { presignedUrls, headers } = await ( - opts as AWSS3MultipartWithoutCompanionMandatoryPrepareUploadParts< - M, - B - > - ).prepareUploadParts(file, { - uploadId, - key, - parts: [{ number: partNumber, chunk: body }], - signal, - }) - return { - url: presignedUrls?.[partNumber], - headers: headers?.[partNumber], - } - } - } /** * Simultaneous upload limiting is shared across all uploads with this plugin. @@ -729,7 +680,7 @@ export default class AwsS3Multipart< ;(error as any).source = { status: 403 } reject(error) }) - xhr.addEventListener('load', (ev) => { + xhr.addEventListener('load', () => { cleanup() if (