From 907321730840d17359c1c0d2d5d3f2bda529b576 Mon Sep 17 00:00:00 2001 From: ashika112 <155593080+ashika112@users.noreply.github.com> Date: Thu, 18 Jul 2024 09:39:46 -0700 Subject: [PATCH] [Multi-bucket] Update options to include bucket (#13574) * update options and test * update assertion error & download test * update getUrl & getProp test * update remove and list tests * update multi-part tests * update list & download test * update more storage tests * update bundle size * address comments --- packages/aws-amplify/package.json | 14 +- .../src/singleton/AmplifyOutputs/types.ts | 6 + .../providers/s3/apis/downloadData.test.ts | 133 ++++++++++++++- .../providers/s3/apis/getProperties.test.ts | 107 +++++++++++- .../providers/s3/apis/getUrl.test.ts | 99 +++++++++++ .../__tests__/providers/s3/apis/list.test.ts | 143 +++++++++++++++- .../providers/s3/apis/remove.test.ts | 93 ++++++++++- .../apis/uploadData/multipartHandlers.test.ts | 127 ++++++++++++++ .../s3/apis/uploadData/putObjectJob.test.ts | 155 +++++++++++++++++- .../utils/resolveS3ConfigAndInput.test.ts | 36 +++- .../storage/src/errors/types/validation.ts | 5 + .../storage/src/providers/s3/types/options.ts | 7 + .../s3/utils/resolveS3ConfigAndInput.ts | 54 ++++-- 13 files changed, 947 insertions(+), 32 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index d0e9d94f1d2..6a464af7441 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -461,43 +461,43 @@ "name": "[Storage] copy (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ copy }", - "limit": "14.64 kB" + "limit": "14.76 kB" }, { "name": "[Storage] downloadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ downloadData }", - "limit": "15.25 kB" + "limit": "15.38 kB" }, { "name": "[Storage] getProperties (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getProperties }", - "limit": "14.51 kB" + "limit": "14.64 kB" }, { "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "15.60 kB" + "limit": "15.74 kB" }, { "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "15.02 kB" + "limit": "15.15 kB" }, { "name": "[Storage] remove (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ remove }", - "limit": "14.37 kB" + "limit": "14.50 kB" }, { "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "19.68 kB" + "limit": "19.79 kB" } ] } diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index 4d0dfebdada..2968955158f 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -44,13 +44,19 @@ export interface AmplifyOutputsAuthProperties { } export interface AmplifyOutputsStorageBucketProperties { + /** Friendly bucket name provided in Amplify Outputs */ name: string; + /** Actual S3 bucket name given */ bucket_name: string; + /** Region for the bucket */ aws_region: string; } export interface AmplifyOutputsStorageProperties { + /** Default region for Storage */ aws_region: string; + /** Default bucket for Storage */ bucket_name: string; + /** List of buckets for Storage */ buckets?: AmplifyOutputsStorageBucketProperties[]; } diff --git a/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts b/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts index 57d402b1f24..35b790366bc 100644 --- a/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts @@ -24,6 +24,7 @@ import { ItemWithPath, } from '../../../../src/providers/s3/types/outputs'; import './testUtils'; +import { BucketInfo } from '../../../../src/providers/s3/types/options'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('../../../../src/providers/s3/utils'); @@ -62,7 +63,7 @@ const mockDownloadResultBase = { const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockCreateDownloadTask = createDownloadTask as jest.Mock; const mockValidateStorageInput = validateStorageOperationInput as jest.Mock; -const mockGetConfig = Amplify.getConfig as jest.Mock; +const mockGetConfig = jest.mocked(Amplify.getConfig); describe('downloadData with key', () => { beforeAll(() => { @@ -75,6 +76,7 @@ describe('downloadData with key', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -220,6 +222,70 @@ describe('downloadData with key', () => { }), ); }); + + describe('bucket passed in options', () => { + it('should override bucket in getObject call when bucket is object', async () => { + (getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' }); + const abortController = new AbortController(); + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + + downloadData({ + key: inputKey, + options: { + bucket: bucketInfo, + }, + }); + + const { job } = mockCreateDownloadTask.mock.calls[0][0]; + await job(); + + expect(getObject).toHaveBeenCalledTimes(1); + await expect(getObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketInfo.bucketName, + Key: `public/${inputKey}`, + }, + ); + }); + + it('should override bucket in getObject call when bucket is string', async () => { + (getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' }); + const abortController = new AbortController(); + + downloadData({ + key: inputKey, + options: { + bucket: 'default-bucket', + }, + }); + + const { job } = mockCreateDownloadTask.mock.calls[0][0]; + await job(); + + expect(getObject).toHaveBeenCalledTimes(1); + await expect(getObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: `public/${inputKey}`, + }, + ); + }); + }); }); describe('downloadData with path', () => { @@ -233,6 +299,7 @@ describe('downloadData with path', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -366,4 +433,68 @@ describe('downloadData with path', () => { }), ); }); + + describe('bucket passed in options', () => { + it('should override bucket in getObject call when bucket is object', async () => { + (getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' }); + const abortController = new AbortController(); + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + + downloadData({ + path: inputPath, + options: { + bucket: bucketInfo, + }, + }); + + const { job } = mockCreateDownloadTask.mock.calls[0][0]; + await job(); + + expect(getObject).toHaveBeenCalledTimes(1); + await expect(getObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketInfo.bucketName, + Key: inputPath, + }, + ); + }); + + it('should override bucket in getObject call when bucket is string', async () => { + (getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' }); + const abortController = new AbortController(); + + downloadData({ + path: inputPath, + options: { + bucket: 'default-bucket', + }, + }); + + const { job } = mockCreateDownloadTask.mock.calls[0][0]; + await job(); + + expect(getObject).toHaveBeenCalledTimes(1); + await expect(getObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: inputPath, + }, + ); + }); + }); }); diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index bb5a5b957a7..0fcd989453e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -13,6 +13,7 @@ import { GetPropertiesWithPathOutput, } from '../../../../src/providers/s3/types'; import './testUtils'; +import { BucketInfo } from '../../../../src/providers/s3/types/options'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -28,7 +29,7 @@ jest.mock('@aws-amplify/core', () => ({ })); const mockHeadObject = headObject as jest.MockedFunction; const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; -const mockGetConfig = Amplify.getConfig as jest.Mock; +const mockGetConfig = jest.mocked(Amplify.getConfig); const bucket = 'bucket'; const region = 'region'; @@ -65,14 +66,16 @@ describe('getProperties with key', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); }); + describe('Happy cases: With key', () => { const config = { credentials, - region: 'region', + region, userAgentValue: expect.any(String), }; beforeEach(() => { @@ -152,6 +155,56 @@ describe('getProperties with key', () => { ); }, ); + + describe('bucket passed in options', () => { + it('should override bucket in headObject call when bucket is object', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + const headObjectOptions = { + Bucket: bucketInfo.bucketName, + Key: `public/${inputKey}`, + }; + + await getPropertiesWrapper({ + key: inputKey, + options: { + bucket: bucketInfo, + }, + }); + expect(headObject).toHaveBeenCalledTimes(1); + await expect(headObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + + userAgentValue: expect.any(String), + }, + headObjectOptions, + ); + }); + it('should override bucket in headObject call when bucket is string', async () => { + await getPropertiesWrapper({ + key: inputKey, + options: { + bucket: 'default-bucket', + }, + }); + expect(headObject).toHaveBeenCalledTimes(1); + await expect(headObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: `public/${inputKey}`, + }, + ); + }); + }); }); describe('Error cases : With key', () => { @@ -201,6 +254,7 @@ describe('Happy cases: With path', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -275,6 +329,55 @@ describe('Happy cases: With path', () => { ); }, ); + describe('bucket passed in options', () => { + it('should override bucket in headObject call when bucket is object', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + const headObjectOptions = { + Bucket: bucketInfo.bucketName, + Key: inputPath, + }; + + await getPropertiesWrapper({ + path: inputPath, + options: { + bucket: bucketInfo, + }, + }); + expect(headObject).toHaveBeenCalledTimes(1); + await expect(headObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + + userAgentValue: expect.any(String), + }, + headObjectOptions, + ); + }); + it('should override bucket in headObject call when bucket is string', async () => { + await getPropertiesWrapper({ + path: inputPath, + options: { + bucket: 'default-bucket', + }, + }); + expect(headObject).toHaveBeenCalledTimes(1); + await expect(headObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: inputPath, + }, + ); + }); + }); }); describe('Error cases : With path', () => { diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 994f4a0b648..47136f139d1 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -16,6 +16,7 @@ import { GetUrlWithPathOutput, } from '../../../../src/providers/s3/types'; import './testUtils'; +import { BucketInfo } from '../../../../src/providers/s3/types/options'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -56,6 +57,7 @@ describe('getUrl test with key', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -135,6 +137,52 @@ describe('getUrl test with key', () => { expect({ url, expiresAt }).toEqual(expectedResult); }, ); + describe('bucket passed in options', () => { + it('should override bucket in getPresignedGetObjectUrl call when bucket is object', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + await getUrlWrapper({ + key: 'key', + options: { + bucket: bucketInfo, + }, + }); + expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); + await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + expiration: expect.any(Number), + }, + { + Bucket: bucketInfo.bucketName, + Key: 'public/key', + }, + ); + }); + it('should override bucket in getPresignedGetObjectUrl call when bucket is string', async () => { + await getUrlWrapper({ + key: 'key', + options: { + bucket: 'default-bucket', + }, + }); + expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); + await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + expiration: expect.any(Number), + }, + { + Bucket: bucket, + Key: 'public/key', + }, + ); + }); + }); }); describe('Error cases : With key', () => { afterAll(() => { @@ -175,6 +223,7 @@ describe('getUrl test with path', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -235,7 +284,57 @@ describe('getUrl test with path', () => { }); }, ); + + describe('bucket passed in options', () => { + it('should override bucket in getPresignedGetObjectUrl call when bucket is object', async () => { + const inputPath = 'path/'; + const bucketInfo: BucketInfo = { + bucketName: 'bucket-1', + region: 'region-1', + }; + await getUrlWrapper({ + path: inputPath, + options: { + bucket: bucketInfo, + }, + }); + expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); + await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + expiration: expect.any(Number), + }, + { + Bucket: bucketInfo.bucketName, + Key: inputPath, + }, + ); + }); + it('should override bucket in getPresignedGetObjectUrl call when bucket is string', async () => { + const inputPath = 'path/'; + await getUrlWrapper({ + path: inputPath, + options: { + bucket: 'default-bucket', + }, + }); + expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); + await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + expiration: expect.any(Number), + }, + { + Bucket: bucket, + Key: inputPath, + }, + ); + }); + }); }); + describe('Error cases : With path', () => { afterAll(() => { jest.clearAllMocks(); diff --git a/packages/storage/__tests__/providers/s3/apis/list.test.ts b/packages/storage/__tests__/providers/s3/apis/list.test.ts index 9629129d7a2..20ccc32b366 100644 --- a/packages/storage/__tests__/providers/s3/apis/list.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/list.test.ts @@ -31,7 +31,7 @@ jest.mock('@aws-amplify/core', () => ({ }, })); const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; -const mockGetConfig = Amplify.getConfig as jest.Mock; +const mockGetConfig = jest.mocked(Amplify.getConfig); const mockListObject = listObjectsV2 as jest.Mock; const inputKey = 'path/itemsKey'; const bucket = 'bucket'; @@ -93,6 +93,7 @@ describe('list API', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -304,6 +305,76 @@ describe('list API', () => { }); }, ); + + describe('bucket passed in options', () => { + it('should override bucket in listObject call when bucket is object', async () => { + mockListObject.mockImplementationOnce(() => { + return { + Contents: [ + { + ...listObjectClientBaseResultItem, + Key: inputKey, + }, + ], + NextContinuationToken: nextToken, + }; + }); + const mockBucketName = 'bucket-1'; + const mockRegion = 'region-1'; + await listPaginatedWrapper({ + prefix: inputKey, + options: { + bucket: { bucketName: mockBucketName, region: mockRegion }, + }, + }); + expect(listObjectsV2).toHaveBeenCalledTimes(1); + await expect(listObjectsV2).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + userAgentValue: expect.any(String), + }, + { + Bucket: mockBucketName, + MaxKeys: 1000, + Prefix: `public/${inputKey}`, + }, + ); + }); + + it('should override bucket in listObject call when bucket is string', async () => { + mockListObject.mockImplementationOnce(() => { + return { + Contents: [ + { + ...listObjectClientBaseResultItem, + Key: inputKey, + }, + ], + NextContinuationToken: nextToken, + }; + }); + await listPaginatedWrapper({ + prefix: inputKey, + options: { + bucket: 'default-bucket', + }, + }); + expect(listObjectsV2).toHaveBeenCalledTimes(1); + await expect(listObjectsV2).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + MaxKeys: 1000, + Prefix: `public/${inputKey}`, + }, + ); + }); + }); }); describe('Path: Happy Cases:', () => { @@ -482,6 +553,76 @@ describe('list API', () => { ); }, ); + + describe('bucket passed in options', () => { + it('should override bucket in listObject call when bucket is object', async () => { + mockListObject.mockImplementationOnce(() => { + return { + Contents: [ + { + ...listObjectClientBaseResultItem, + Key: 'path/', + }, + ], + NextContinuationToken: nextToken, + }; + }); + const mockBucketName = 'bucket-1'; + const mockRegion = 'region-1'; + await listPaginatedWrapper({ + path: 'path/', + options: { + bucket: { bucketName: mockBucketName, region: mockRegion }, + }, + }); + expect(listObjectsV2).toHaveBeenCalledTimes(1); + await expect(listObjectsV2).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + userAgentValue: expect.any(String), + }, + { + Bucket: mockBucketName, + MaxKeys: 1000, + Prefix: 'path/', + }, + ); + }); + + it('should override bucket in listObject call when bucket is string', async () => { + mockListObject.mockImplementationOnce(() => { + return { + Contents: [ + { + ...listObjectClientBaseResultItem, + Key: 'path/', + }, + ], + NextContinuationToken: nextToken, + }; + }); + await listPaginatedWrapper({ + path: 'path/', + options: { + bucket: 'default-bucket', + }, + }); + expect(listObjectsV2).toHaveBeenCalledTimes(1); + await expect(listObjectsV2).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + MaxKeys: 1000, + Prefix: 'path/', + }, + ); + }); + }); }); describe('Error Cases:', () => { diff --git a/packages/storage/__tests__/providers/s3/apis/remove.test.ts b/packages/storage/__tests__/providers/s3/apis/remove.test.ts index ca1107f0912..eb3407eb610 100644 --- a/packages/storage/__tests__/providers/s3/apis/remove.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/remove.test.ts @@ -29,7 +29,7 @@ jest.mock('@aws-amplify/core', () => ({ })); const mockDeleteObject = deleteObject as jest.Mock; const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; -const mockGetConfig = Amplify.getConfig as jest.Mock; +const mockGetConfig = jest.mocked(Amplify.getConfig); const inputKey = 'key'; const bucket = 'bucket'; const region = 'region'; @@ -56,6 +56,7 @@ describe('remove API', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -115,6 +116,51 @@ describe('remove API', () => { ); }); }); + + describe('bucket passed in options', () => { + it('should override bucket in deleteObject call when bucket is object', async () => { + const mockBucketName = 'bucket-1'; + const mockRegion = 'region-1'; + await removeWrapper({ + key: inputKey, + options: { + bucket: { bucketName: mockBucketName, region: mockRegion }, + }, + }); + expect(deleteObject).toHaveBeenCalledTimes(1); + await expect(deleteObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + userAgentValue: expect.any(String), + }, + { + Bucket: mockBucketName, + Key: `public/${inputKey}`, + }, + ); + }); + it('should override bucket in deleteObject call when bucket is string', async () => { + await removeWrapper({ + key: inputKey, + options: { + bucket: 'default-bucket', + }, + }); + expect(deleteObject).toHaveBeenCalledTimes(1); + await expect(deleteObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: `public/${inputKey}`, + }, + ); + }); + }); }); describe('With Path', () => { const removeWrapper = ( @@ -157,6 +203,51 @@ describe('remove API', () => { ); }); }); + + describe('bucket passed in options', () => { + it('should override bucket in deleteObject call when bucket is object', async () => { + const mockBucketName = 'bucket-1'; + const mockRegion = 'region-1'; + await removeWrapper({ + path: 'path/', + options: { + bucket: { bucketName: mockBucketName, region: mockRegion }, + }, + }); + expect(deleteObject).toHaveBeenCalledTimes(1); + await expect(deleteObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + userAgentValue: expect.any(String), + }, + { + Bucket: mockBucketName, + Key: 'path/', + }, + ); + }); + it('should override bucket in deleteObject call when bucket is string', async () => { + await removeWrapper({ + path: 'path/', + options: { + bucket: 'default-bucket', + }, + }); + expect(deleteObject).toHaveBeenCalledTimes(1); + await expect(deleteObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: 'path/', + }, + ); + }); + }); }); }); diff --git a/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts b/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts index c40e5c83de6..f7515c04cf2 100644 --- a/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts @@ -143,6 +143,7 @@ describe('getMultipartUploadHandlers with key', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -347,6 +348,69 @@ describe('getMultipartUploadHandlers with key', () => { expect(mockUploadPart).toHaveBeenCalledTimes(2); expect(mockCompleteMultipartUpload).not.toHaveBeenCalled(); }); + + describe('bucket passed in options', () => { + const mockData = 'Ü'.repeat(4 * MB); + it('should override bucket in putObject call when bucket as object', async () => { + const mockBucket = 'bucket-1'; + const mockRegion = 'region-1'; + mockMultipartUploadSuccess(); + const { multipartUploadJob } = getMultipartUploadHandlers({ + key: 'key', + data: mockData, + options: { + bucket: { bucketName: mockBucket, region: mockRegion }, + }, + }); + await multipartUploadJob(); + await expect( + mockCreateMultipartUpload, + ).toBeLastCalledWithConfigAndInput( + expect.objectContaining({ + credentials, + region: mockRegion, + abortSignal: expect.any(AbortSignal), + }), + expect.objectContaining({ + Bucket: mockBucket, + Key: 'public/key', + ContentType: defaultContentType, + }), + ); + expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1); + expect(mockUploadPart).toHaveBeenCalledTimes(2); + expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1); + }); + + it('should override bucket in putObject call when bucket as string', async () => { + mockMultipartUploadSuccess(); + const { multipartUploadJob } = getMultipartUploadHandlers({ + key: 'key', + data: mockData, + options: { + bucket: 'default-bucket', + }, + }); + await multipartUploadJob(); + await expect( + mockCreateMultipartUpload, + ).toBeLastCalledWithConfigAndInput( + expect.objectContaining({ + credentials, + region, + abortSignal: expect.any(AbortSignal), + }), + expect.objectContaining({ + Bucket: bucket, + Key: 'public/key', + ContentType: defaultContentType, + }), + ); + expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1); + expect(mockUploadPart).toHaveBeenCalledTimes(2); + expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1); + }); + }); }); describe('upload caching', () => { @@ -665,6 +729,7 @@ describe('getMultipartUploadHandlers with path', () => { S3: { bucket, region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -861,6 +926,68 @@ describe('getMultipartUploadHandlers with path', () => { expect(mockUploadPart).toHaveBeenCalledTimes(2); expect(mockCompleteMultipartUpload).not.toHaveBeenCalled(); }); + + describe('bucket passed in options', () => { + const mockData = 'Ü'.repeat(4 * MB); + it('should override bucket in putObject call when bucket as object', async () => { + const mockBucket = 'bucket-1'; + const mockRegion = 'region-1'; + mockMultipartUploadSuccess(); + const { multipartUploadJob } = getMultipartUploadHandlers({ + path: 'path/', + data: mockData, + options: { + bucket: { bucketName: mockBucket, region: mockRegion }, + }, + }); + await multipartUploadJob(); + await expect( + mockCreateMultipartUpload, + ).toBeLastCalledWithConfigAndInput( + expect.objectContaining({ + credentials, + region: mockRegion, + abortSignal: expect.any(AbortSignal), + }), + expect.objectContaining({ + Bucket: mockBucket, + Key: 'path/', + ContentType: defaultContentType, + }), + ); + expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1); + expect(mockUploadPart).toHaveBeenCalledTimes(2); + expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1); + }); + it('should override bucket in putObject call when bucket as string', async () => { + mockMultipartUploadSuccess(); + const { multipartUploadJob } = getMultipartUploadHandlers({ + path: 'path/', + data: mockData, + options: { + bucket: 'default-bucket', + }, + }); + await multipartUploadJob(); + await expect( + mockCreateMultipartUpload, + ).toBeLastCalledWithConfigAndInput( + expect.objectContaining({ + credentials, + region, + abortSignal: expect.any(AbortSignal), + }), + expect.objectContaining({ + Bucket: bucket, + Key: 'path/', + ContentType: defaultContentType, + }), + ); + expect(mockCreateMultipartUpload).toHaveBeenCalledTimes(1); + expect(mockUploadPart).toHaveBeenCalledTimes(2); + expect(mockCompleteMultipartUpload).toHaveBeenCalledTimes(1); + }); + }); }); describe('upload caching', () => { diff --git a/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts b/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts index 335e804c0ea..aa9cf2ff8cd 100644 --- a/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts @@ -38,6 +38,8 @@ const credentials: AWSCredentials = { const identityId = 'identityId'; const mockFetchAuthSession = jest.mocked(Amplify.Auth.fetchAuthSession); const mockPutObject = jest.mocked(putObject); +const bucket = 'bucket'; +const region = 'region'; mockFetchAuthSession.mockResolvedValue({ credentials, @@ -46,8 +48,9 @@ mockFetchAuthSession.mockResolvedValue({ jest.mocked(Amplify.getConfig).mockReturnValue({ Storage: { S3: { - bucket: 'bucket', - region: 'region', + bucket, + region, + buckets: { 'default-bucket': { bucketName: bucket, region } }, }, }, }); @@ -102,14 +105,14 @@ describe('putObjectJob with key', () => { await expect(mockPutObject).toBeLastCalledWithConfigAndInput( { credentials, - region: 'region', + region, abortSignal: abortController.signal, onUploadProgress: expect.any(Function), useAccelerateEndpoint: true, userAgentValue: expect.any(String), }, { - Bucket: 'bucket', + Bucket: bucket, Key: `public/${inputKey}`, Body: data, ContentType: mockContentType, @@ -139,6 +142,76 @@ describe('putObjectJob with key', () => { await job(); expect(calculateContentMd5).toHaveBeenCalledWith('data'); }); + + describe('bucket passed in options', () => { + it('should override bucket in putObject call when bucket as object', async () => { + const abortController = new AbortController(); + const data = 'data'; + const bucketName = 'bucket-1'; + const mockRegion = 'region-1'; + + const job = putObjectJob( + { + key: 'key', + data, + options: { + bucket: { + bucketName, + region: mockRegion, + }, + }, + }, + new AbortController().signal, + ); + await job(); + + await expect(mockPutObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketName, + Key: 'public/key', + Body: data, + ContentType: 'application/octet-stream', + }, + ); + }); + + it('should override bucket in putObject call when bucket as string', async () => { + const abortController = new AbortController(); + const data = 'data'; + const job = putObjectJob( + { + key: 'key', + data, + options: { + bucket: 'default-bucket', + }, + }, + new AbortController().signal, + ); + await job(); + + await expect(mockPutObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: 'public/key', + Body: data, + ContentType: 'application/octet-stream', + }, + ); + }); + }); }); describe('putObjectJob with path', () => { @@ -195,14 +268,14 @@ describe('putObjectJob with path', () => { await expect(mockPutObject).toBeLastCalledWithConfigAndInput( { credentials, - region: 'region', + region, abortSignal: abortController.signal, onUploadProgress: expect.any(Function), useAccelerateEndpoint: true, userAgentValue: expect.any(String), }, { - Bucket: 'bucket', + Bucket: bucket, Key: expectedKey, Body: data, ContentType: mockContentType, @@ -233,4 +306,74 @@ describe('putObjectJob with path', () => { await job(); expect(calculateContentMd5).toHaveBeenCalledWith('data'); }); + + describe('bucket passed in options', () => { + it('should override bucket in putObject call when bucket as object', async () => { + const abortController = new AbortController(); + const data = 'data'; + const bucketName = 'bucket-1'; + const mockRegion = 'region-1'; + + const job = putObjectJob( + { + path: 'path/', + data, + options: { + bucket: { + bucketName, + region: mockRegion, + }, + }, + }, + new AbortController().signal, + ); + await job(); + + await expect(mockPutObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: mockRegion, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketName, + Key: 'path/', + Body: data, + ContentType: 'application/octet-stream', + }, + ); + }); + + it('should override bucket in putObject call when bucket as string', async () => { + const abortController = new AbortController(); + const data = 'data'; + const job = putObjectJob( + { + path: 'path/', + data, + options: { + bucket: 'default-bucket', + }, + }, + new AbortController().signal, + ); + await job(); + + await expect(mockPutObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region, + abortSignal: abortController.signal, + userAgentValue: expect.any(String), + }, + { + Bucket: bucket, + Key: 'path/', + Body: data, + ContentType: 'application/octet-stream', + }, + ); + }); + }); }); diff --git a/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts b/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts index e26cb63b6c7..022c2f0c1fb 100644 --- a/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts @@ -9,6 +9,8 @@ import { StorageValidationErrorCode, validationErrorMap, } from '../../../../../src/errors/types/validation'; +import { BucketInfo } from '../../../../../src/providers/s3/types/options'; +import { StorageError } from '../../../../../src/errors/StorageError'; jest.mock('@aws-amplify/core', () => ({ ConsoleLogger: jest.fn(), @@ -21,7 +23,7 @@ jest.mock('@aws-amplify/core', () => ({ })); jest.mock('../../../../../src/utils/resolvePrefix'); -const mockGetConfig = Amplify.getConfig as jest.Mock; +const mockGetConfig = jest.mocked(Amplify.getConfig); const mockDefaultResolvePrefix = resolvePrefix as jest.Mock; const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; @@ -49,6 +51,7 @@ describe('resolveS3ConfigAndInput', () => { S3: { bucket, region, + buckets: { 'bucket-1': { bucketName: bucket, region } }, }, }, }); @@ -132,7 +135,7 @@ describe('resolveS3ConfigAndInput', () => { S3: { bucket, region, - dangerouslyConnectToHttpEndpointForTesting: true, + dangerouslyConnectToHttpEndpointForTesting: 'true', }, }, }); @@ -214,4 +217,33 @@ describe('resolveS3ConfigAndInput', () => { }); expect(keyPrefix).toEqual('prefix'); }); + + it('should resolve bucket and region with overrides when bucket API option is passed', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-2', + region: 'region-2', + }; + + const { + bucket: resolvedBucket, + s3Config: { region: resolvedRegion }, + } = await resolveS3ConfigAndInput(Amplify, { + bucket: bucketInfo, + }); + + expect(mockGetConfig).toHaveBeenCalled(); + expect(resolvedBucket).toEqual(bucketInfo.bucketName); + expect(resolvedRegion).toEqual(bucketInfo.region); + }); + + it('should throw when unable to lookup bucket from the config when bucket API option is passed', async () => { + try { + await resolveS3ConfigAndInput(Amplify, { + bucket: 'error-bucket', + }); + } catch (error: any) { + expect(error).toBeInstanceOf(StorageError); + expect(error.name).toBe(StorageValidationErrorCode.InvalidStorageBucket); + } + }); }); diff --git a/packages/storage/src/errors/types/validation.ts b/packages/storage/src/errors/types/validation.ts index d72b9852162..b84443ad78f 100644 --- a/packages/storage/src/errors/types/validation.ts +++ b/packages/storage/src/errors/types/validation.ts @@ -13,6 +13,7 @@ export enum StorageValidationErrorCode { NoDestinationPath = 'NoDestinationPath', NoBucket = 'NoBucket', NoRegion = 'NoRegion', + InvalidStorageBucket = 'InvalidStorageBucket', InvalidStorageOperationPrefixInput = 'InvalidStorageOperationPrefixInput', InvalidStorageOperationInput = 'InvalidStorageOperationInput', InvalidStoragePathInput = 'InvalidStoragePathInput', @@ -70,4 +71,8 @@ export const validationErrorMap: AmplifyErrorMap = { [StorageValidationErrorCode.InvalidStoragePathInput]: { message: 'Input `path` does not allow a leading slash (/).', }, + [StorageValidationErrorCode.InvalidStorageBucket]: { + message: + 'Unable to lookup bucket from provided name in Amplify configuration.', + }, }; diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index b2b7dfd0ddc..0642d943ced 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -10,12 +10,19 @@ import { StorageListPaginateOptions, } from '../../../types/options'; +export interface BucketInfo { + bucketName: string; + region: string; +} + +export type StorageBucket = string | BucketInfo; interface CommonOptions { /** * Whether to use accelerate endpoint. * @default false */ useAccelerateEndpoint?: boolean; + bucket?: StorageBucket; } /** @deprecated This may be removed in the next major version. */ diff --git a/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts b/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts index ae7a185c93c..928d1876251 100644 --- a/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts +++ b/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts @@ -6,7 +6,7 @@ import { AmplifyClassV6, StorageAccessLevel } from '@aws-amplify/core'; import { assertValidationError } from '../../../errors/utils/assertValidationError'; import { StorageValidationErrorCode } from '../../../errors/types/validation'; import { resolvePrefix as defaultPrefixResolver } from '../../../utils/resolvePrefix'; -import { ResolvedS3Config } from '../types/options'; +import { BucketInfo, ResolvedS3Config, StorageBucket } from '../types/options'; import { DEFAULT_ACCESS_LEVEL, LOCAL_TESTING_S3_ENDPOINT } from './constants'; @@ -14,6 +14,7 @@ interface S3ApiOptions { accessLevel?: StorageAccessLevel; targetIdentityId?: string; useAccelerateEndpoint?: boolean; + bucket?: StorageBucket; } interface ResolvedS3ConfigAndInput { @@ -62,8 +63,16 @@ export const resolveS3ConfigAndInput = async ( return credentials; }; - const { bucket, region, dangerouslyConnectToHttpEndpointForTesting } = - amplify.getConfig()?.Storage?.S3 ?? {}; + const { + bucket: defaultBucket, + region: defaultRegion, + dangerouslyConnectToHttpEndpointForTesting, + buckets, + } = amplify.getConfig()?.Storage?.S3 ?? {}; + + const { bucket = defaultBucket, region = defaultRegion } = + (apiOptions?.bucket && resolveBucketConfig(apiOptions, buckets)) || {}; + assertValidationError(!!bucket, StorageValidationErrorCode.NoBucket); assertValidationError(!!region, StorageValidationErrorCode.NoRegion); @@ -73,15 +82,14 @@ export const resolveS3ConfigAndInput = async ( isObjectLockEnabled, } = amplify.libraryOptions?.Storage?.S3 ?? {}; - const keyPrefix = await prefixResolver({ - accessLevel: - apiOptions?.accessLevel ?? defaultAccessLevel ?? DEFAULT_ACCESS_LEVEL, - // use conditional assign to make tsc happy because StorageOptions is a union type that may not have targetIdentityId - targetIdentityId: - apiOptions?.accessLevel === 'protected' - ? apiOptions?.targetIdentityId ?? identityId - : identityId, - }); + const accessLevel = + apiOptions?.accessLevel ?? defaultAccessLevel ?? DEFAULT_ACCESS_LEVEL; + const targetIdentityId = + accessLevel === 'protected' + ? apiOptions?.targetIdentityId ?? identityId + : identityId; + + const keyPrefix = await prefixResolver({ accessLevel, targetIdentityId }); return { s3Config: { @@ -101,3 +109,25 @@ export const resolveS3ConfigAndInput = async ( isObjectLockEnabled, }; }; + +const resolveBucketConfig = ( + apiOptions: S3ApiOptions, + buckets: Record | undefined, +): { bucket: string; region: string } | undefined => { + if (typeof apiOptions.bucket === 'string') { + const bucketConfig = buckets?.[apiOptions.bucket]; + assertValidationError( + !!bucketConfig, + StorageValidationErrorCode.InvalidStorageBucket, + ); + + return { bucket: bucketConfig.bucketName, region: bucketConfig.region }; + } + + if (typeof apiOptions.bucket === 'object') { + return { + bucket: apiOptions.bucket.bucketName, + region: apiOptions.bucket.region, + }; + } +};