From bfb3b00696d1a266b65698c87b289f20f64c24cb Mon Sep 17 00:00:00 2001 From: ashika112 <155593080+ashika112@users.noreply.github.com> Date: Thu, 11 Jul 2024 11:36:21 -0700 Subject: [PATCH 1/8] [Multibucket]Parse AmplifyOutputs update (#13556) * update parseAmplifyOutput * update bundle size * address comments * update bundle size --- packages/aws-amplify/package.json | 56 ++++++++-------- .../__tests__/parseAmplifyOutputs.test.ts | 65 +++++++++++++++++++ packages/core/src/parseAmplifyOutputs.ts | 28 +++++++- .../src/singleton/AmplifyOutputs/types.ts | 6 ++ packages/core/src/singleton/Storage/types.ts | 9 +++ packages/core/src/singleton/types.ts | 2 + packages/interactions/package.json | 6 +- 7 files changed, 139 insertions(+), 33 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 31564a92ec3..d0e9d94f1d2 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -293,31 +293,31 @@ "name": "[Analytics] record (Pinpoint)", "path": "./dist/esm/analytics/index.mjs", "import": "{ record }", - "limit": "17.09 kB" + "limit": "17.18 kB" }, { "name": "[Analytics] record (Kinesis)", "path": "./dist/esm/analytics/kinesis/index.mjs", "import": "{ record }", - "limit": "48.56 kB" + "limit": "48.61 kB" }, { "name": "[Analytics] record (Kinesis Firehose)", "path": "./dist/esm/analytics/kinesis-firehose/index.mjs", "import": "{ record }", - "limit": "45.68 kB" + "limit": "45.76 kB" }, { "name": "[Analytics] record (Personalize)", "path": "./dist/esm/analytics/personalize/index.mjs", "import": "{ record }", - "limit": "49.50 kB" + "limit": "49.58 kB" }, { "name": "[Analytics] identifyUser (Pinpoint)", "path": "./dist/esm/analytics/index.mjs", "import": "{ identifyUser }", - "limit": "15.59 kB" + "limit": "15.68 kB" }, { "name": "[Analytics] enable", @@ -335,7 +335,7 @@ "name": "[API] generateClient (AppSync)", "path": "./dist/esm/api/index.mjs", "import": "{ generateClient }", - "limit": "40.09 kB" + "limit": "40.14 kB" }, { "name": "[API] REST API handlers", @@ -353,13 +353,13 @@ "name": "[Auth] resetPassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ resetPassword }", - "limit": "12.44 kB" + "limit": "12.53 kB" }, { "name": "[Auth] confirmResetPassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmResetPassword }", - "limit": "12.39 kB" + "limit": "12.47 kB" }, { "name": "[Auth] signIn (Cognito)", @@ -371,7 +371,7 @@ "name": "[Auth] resendSignUpCode (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ resendSignUpCode }", - "limit": "12.40 kB" + "limit": "12.49 kB" }, { "name": "[Auth] confirmSignUp (Cognito)", @@ -383,31 +383,31 @@ "name": "[Auth] confirmSignIn (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmSignIn }", - "limit": "28.27 kB" + "limit": "28.38 kB" }, { "name": "[Auth] updateMFAPreference (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updateMFAPreference }", - "limit": "11.74 kB" + "limit": "11.83 kB" }, { "name": "[Auth] fetchMFAPreference (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ fetchMFAPreference }", - "limit": "11.78 kB" + "limit": "11.86 kB" }, { "name": "[Auth] verifyTOTPSetup (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ verifyTOTPSetup }", - "limit": "12.6 kB" + "limit": "12.71 kB" }, { "name": "[Auth] updatePassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updatePassword }", - "limit": "12.63 kB" + "limit": "12.73 kB" }, { "name": "[Auth] setUpTOTP (Cognito)", @@ -419,85 +419,85 @@ "name": "[Auth] updateUserAttributes (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updateUserAttributes }", - "limit": "11.87 kB" + "limit": "11.95 kB" }, { "name": "[Auth] getCurrentUser (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ getCurrentUser }", - "limit": "7.75 kB" + "limit": "7.85 kB" }, { "name": "[Auth] confirmUserAttribute (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmUserAttribute }", - "limit": "12.61 kB" + "limit": "12.71 kB" }, { "name": "[Auth] signInWithRedirect (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signInWithRedirect }", - "limit": "21.10 kB" + "limit": "21.15 kB" }, { "name": "[Auth] fetchUserAttributes (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ fetchUserAttributes }", - "limit": "11.69 kB" + "limit": "11.77 kB" }, { "name": "[Auth] Basic Auth Flow (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signIn, signOut, fetchAuthSession, confirmSignIn }", - "limit": "30.06 kB" + "limit": "30.15 kB" }, { "name": "[Auth] OAuth Auth Flow (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signInWithRedirect, signOut, fetchAuthSession }", - "limit": "21.47 kB" + "limit": "21.58 kB" }, { "name": "[Storage] copy (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ copy }", - "limit": "14.54 kB" + "limit": "14.64 kB" }, { "name": "[Storage] downloadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ downloadData }", - "limit": "15.17 kB" + "limit": "15.25 kB" }, { "name": "[Storage] getProperties (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getProperties }", - "limit": "14.43 kB" + "limit": "14.51 kB" }, { "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "15.51 kB" + "limit": "15.60 kB" }, { "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "14.94 kB" + "limit": "15.02 kB" }, { "name": "[Storage] remove (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ remove }", - "limit": "14.29 kB" + "limit": "14.37 kB" }, { "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "19.64 kB" + "limit": "19.68 kB" } ] } diff --git a/packages/core/__tests__/parseAmplifyOutputs.test.ts b/packages/core/__tests__/parseAmplifyOutputs.test.ts index 7d15f7b1f52..bb93d12116c 100644 --- a/packages/core/__tests__/parseAmplifyOutputs.test.ts +++ b/packages/core/__tests__/parseAmplifyOutputs.test.ts @@ -229,6 +229,71 @@ describe('parseAmplifyOutputs tests', () => { }, }); }); + it('should parse storage multi bucket', () => { + const amplifyOutputs: AmplifyOutputs = { + version: '1', + storage: { + aws_region: 'us-west-2', + bucket_name: 'storage-bucket-test', + buckets: [ + { + name: 'default-bucket', + bucket_name: 'storage-bucket-test', + aws_region: 'us-west-2', + }, + { + name: 'bucket-2', + bucket_name: 'storage-bucket-test-2', + aws_region: 'us-west-2', + }, + ], + }, + }; + + const result = parseAmplifyOutputs(amplifyOutputs); + + expect(result).toEqual({ + Storage: { + S3: { + bucket: 'storage-bucket-test', + region: 'us-west-2', + buckets: { + 'bucket-2': { + bucketName: 'storage-bucket-test-2', + region: 'us-west-2', + }, + 'default-bucket': { + bucketName: 'storage-bucket-test', + region: 'us-west-2', + }, + }, + }, + }, + }); + }); + it('should throw for storage multi bucket parsing with same friendly name', () => { + const amplifyOutputs: AmplifyOutputs = { + version: '1', + storage: { + aws_region: 'us-west-2', + bucket_name: 'storage-bucket-test', + buckets: [ + { + name: 'default-bucket', + bucket_name: 'storage-bucket-test', + aws_region: 'us-west-2', + }, + { + name: 'default-bucket', + bucket_name: 'storage-bucket-test-2', + aws_region: 'us-west-2', + }, + ], + }, + }; + + expect(() => parseAmplifyOutputs(amplifyOutputs)).toThrow(); + }); }); describe('analytics tests', () => { diff --git a/packages/core/src/parseAmplifyOutputs.ts b/packages/core/src/parseAmplifyOutputs.ts index ed742189266..3a9265720e1 100644 --- a/packages/core/src/parseAmplifyOutputs.ts +++ b/packages/core/src/parseAmplifyOutputs.ts @@ -4,7 +4,7 @@ /* This is because JSON schema contains keys with snake_case */ /* eslint-disable camelcase */ -/* Does not like exahaustive checks */ +/* Does not like exhaustive checks */ /* eslint-disable no-case-declarations */ import { @@ -25,11 +25,13 @@ import { AmplifyOutputsDataProperties, AmplifyOutputsGeoProperties, AmplifyOutputsNotificationsProperties, + AmplifyOutputsStorageBucketProperties, AmplifyOutputsStorageProperties, } from './singleton/AmplifyOutputs/types'; import { AnalyticsConfig, AuthConfig, + BucketInfo, GeoConfig, LegacyConfig, ResourcesConfig, @@ -56,12 +58,13 @@ function parseStorage( return undefined; } - const { bucket_name, aws_region } = amplifyOutputsStorageProperties; + const { bucket_name, aws_region, buckets } = amplifyOutputsStorageProperties; return { S3: { bucket: bucket_name, region: aws_region, + buckets: buckets && createBucketInfoMap(buckets), }, }; } @@ -333,3 +336,24 @@ function getMfaStatus( return 'off'; } + +function createBucketInfoMap( + buckets: AmplifyOutputsStorageBucketProperties[], +): Record { + const mappedBuckets: Record = {}; + + buckets.forEach(({ name, bucket_name: bucketName, aws_region: region }) => { + if (name in mappedBuckets) { + throw new Error( + `Duplicate friendly name found: ${name}. Name must be unique.`, + ); + } + + mappedBuckets[name] = { + bucketName, + region, + }; + }); + + return mappedBuckets; +} diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index 9f03f49a7fb..4d0dfebdada 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -43,9 +43,15 @@ export interface AmplifyOutputsAuthProperties { mfa_methods?: string[]; } +export interface AmplifyOutputsStorageBucketProperties { + name: string; + bucket_name: string; + aws_region: string; +} export interface AmplifyOutputsStorageProperties { aws_region: string; bucket_name: string; + buckets?: AmplifyOutputsStorageBucketProperties[]; } export interface AmplifyOutputsGeoProperties { diff --git a/packages/core/src/singleton/Storage/types.ts b/packages/core/src/singleton/Storage/types.ts index b21413a797a..5bca120c9b3 100644 --- a/packages/core/src/singleton/Storage/types.ts +++ b/packages/core/src/singleton/Storage/types.ts @@ -6,6 +6,13 @@ import { AtLeastOne } from '../types'; /** @deprecated This may be removed in the next major version. */ export type StorageAccessLevel = 'guest' | 'protected' | 'private'; +/** Information on bucket used to store files/objects */ +export interface BucketInfo { + /** Actual bucket name */ + bucketName: string; + /** Region of the bucket */ + region: string; +} export interface S3ProviderConfig { S3: { bucket?: string; @@ -16,6 +23,8 @@ export interface S3ProviderConfig { * @internal */ dangerouslyConnectToHttpEndpointForTesting?: string; + /** Map of friendly name for bucket to its information */ + buckets?: Record; }; } diff --git a/packages/core/src/singleton/types.ts b/packages/core/src/singleton/types.ts index e2acbeb6611..3419be9a1ec 100644 --- a/packages/core/src/singleton/types.ts +++ b/packages/core/src/singleton/types.ts @@ -19,6 +19,7 @@ import { import { GeoConfig } from './Geo/types'; import { PredictionsConfig } from './Predictions/types'; import { + BucketInfo, LibraryStorageOptions, StorageAccessLevel, StorageConfig, @@ -77,6 +78,7 @@ export { PredictionsConfig, StorageAccessLevel, StorageConfig, + BucketInfo, AnalyticsConfig, CognitoIdentityPoolConfig, GeoConfig, diff --git a/packages/interactions/package.json b/packages/interactions/package.json index f111dbb0fd3..c380d359555 100644 --- a/packages/interactions/package.json +++ b/packages/interactions/package.json @@ -89,19 +89,19 @@ "name": "Interactions (default to Lex v2)", "path": "./dist/esm/index.mjs", "import": "{ Interactions }", - "limit": "52.52 kB" + "limit": "52.61 kB" }, { "name": "Interactions (Lex v2)", "path": "./dist/esm/lex-v2/index.mjs", "import": "{ Interactions }", - "limit": "52.52 kB" + "limit": "52.61 kB" }, { "name": "Interactions (Lex v1)", "path": "./dist/esm/lex-v1/index.mjs", "import": "{ Interactions }", - "limit": "47.33 kB" + "limit": "47.41 kB" } ] } 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 2/8] [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, + }; + } +}; From b9cfe68f11ed41fb83a1d559471937242ca6f167 Mon Sep 17 00:00:00 2001 From: ashika112 <155593080+ashika112@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:36:54 -0700 Subject: [PATCH 3/8] [Multi-Bucket] Update Copy API (#13607) * copy multibucket draft 1 * update copy with key and tests * update the path route * cleanup * update bundle size * address minor nits --- packages/aws-amplify/package.json | 14 +-- .../__tests__/providers/s3/apis/copy.test.ts | 94 +++++++++++++++++++ .../storage/src/errors/types/validation.ts | 4 + .../src/providers/s3/apis/internal/copy.ts | 65 ++++++++----- .../storage/src/providers/s3/types/index.ts | 2 + .../storage/src/providers/s3/types/inputs.ts | 7 +- .../storage/src/providers/s3/types/options.ts | 9 ++ packages/storage/src/types/inputs.ts | 6 +- 8 files changed, 168 insertions(+), 33 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 6a464af7441..ef5cbfac39a 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.76 kB" + "limit": "14.86 kB" }, { "name": "[Storage] downloadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ downloadData }", - "limit": "15.38 kB" + "limit": "15.45 kB" }, { "name": "[Storage] getProperties (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getProperties }", - "limit": "14.64 kB" + "limit": "14.70 kB" }, { "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "15.74 kB" + "limit": "15.79 kB" }, { "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "15.15 kB" + "limit": "15.21 kB" }, { "name": "[Storage] remove (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ remove }", - "limit": "14.50 kB" + "limit": "14.56 kB" }, { "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "19.79 kB" + "limit": "19.85 kB" } ] } diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index 55547ae8e7c..00ed7bd9859 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -15,6 +15,7 @@ import { CopyWithPathOutput, } 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', () => ({ @@ -64,6 +65,7 @@ describe('copy API', () => { S3: { bucket, region, + buckets: { 'bucket-1': { bucketName: bucket, region } }, }, }, }); @@ -198,6 +200,34 @@ describe('copy API', () => { }); }, ); + + it('should override bucket in copyObject call when bucket option is passed', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-2', + region: 'region-2', + }; + await copyWrapper({ + source: { key: 'sourceKey', bucket: 'bucket-1' }, + destination: { + key: 'destinationKey', + bucket: bucketInfo, + }, + }); + expect(copyObject).toHaveBeenCalledTimes(1); + await expect(copyObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketInfo.bucketName, + MetadataDirective: 'COPY', + CopySource: `${bucket}/public/sourceKey`, + Key: 'public/destinationKey', + }, + ); + }); }); describe('With path', () => { @@ -253,6 +283,33 @@ describe('copy API', () => { ); }, ); + it('should override bucket in copyObject call when bucket option is passed', async () => { + const bucketInfo: BucketInfo = { + bucketName: 'bucket-2', + region: 'region-2', + }; + await copyWrapper({ + source: { path: 'sourcePath', bucket: 'bucket-1' }, + destination: { + path: 'destinationPath', + bucket: bucketInfo, + }, + }); + expect(copyObject).toHaveBeenCalledTimes(1); + await expect(copyObject).toBeLastCalledWithConfigAndInput( + { + credentials, + region: bucketInfo.region, + userAgentValue: expect.any(String), + }, + { + Bucket: bucketInfo.bucketName, + MetadataDirective: 'COPY', + CopySource: `${bucket}/sourcePath`, + Key: 'destinationPath', + }, + ); + }); }); }); @@ -316,5 +373,42 @@ describe('copy API', () => { expect(error.name).toBe(StorageValidationErrorCode.NoDestinationKey); } }); + + it('should throw an error when only source has bucket option', async () => { + expect.assertions(2); + try { + await copy({ + source: { path: 'source', bucket: 'bucket-1' }, + destination: { + path: 'destination', + }, + }); + } catch (error: any) { + console.log(error); + expect(error).toBeInstanceOf(StorageError); + expect(error.name).toBe( + StorageValidationErrorCode.InvalidCopyOperationStorageBucket, + ); + } + }); + + it('should throw an error when only one destination has bucket option', async () => { + expect.assertions(2); + try { + await copy({ + source: { key: 'source' }, + destination: { + key: 'destination', + bucket: 'bucket-1', + }, + }); + } catch (error: any) { + console.log(error); + expect(error).toBeInstanceOf(StorageError); + expect(error.name).toBe( + StorageValidationErrorCode.InvalidCopyOperationStorageBucket, + ); + } + }); }); }); diff --git a/packages/storage/src/errors/types/validation.ts b/packages/storage/src/errors/types/validation.ts index b84443ad78f..7fb1bd89765 100644 --- a/packages/storage/src/errors/types/validation.ts +++ b/packages/storage/src/errors/types/validation.ts @@ -14,6 +14,7 @@ export enum StorageValidationErrorCode { NoBucket = 'NoBucket', NoRegion = 'NoRegion', InvalidStorageBucket = 'InvalidStorageBucket', + InvalidCopyOperationStorageBucket = 'InvalidCopyOperationStorageBucket', InvalidStorageOperationPrefixInput = 'InvalidStorageOperationPrefixInput', InvalidStorageOperationInput = 'InvalidStorageOperationInput', InvalidStoragePathInput = 'InvalidStoragePathInput', @@ -75,4 +76,7 @@ export const validationErrorMap: AmplifyErrorMap = { message: 'Unable to lookup bucket from provided name in Amplify configuration.', }, + [StorageValidationErrorCode.InvalidCopyOperationStorageBucket]: { + message: 'Missing bucket option in either source or destination.', + }, }; diff --git a/packages/storage/src/providers/s3/apis/internal/copy.ts b/packages/storage/src/providers/s3/apis/internal/copy.ts index e0c96a1fba4..29554ce4978 100644 --- a/packages/storage/src/providers/s3/apis/internal/copy.ts +++ b/packages/storage/src/providers/s3/apis/internal/copy.ts @@ -10,7 +10,7 @@ import { CopyWithPathInput, CopyWithPathOutput, } from '../../types'; -import { ResolvedS3Config } from '../../types/options'; +import { ResolvedS3Config, StorageBucket } from '../../types/options'; import { isInputWithPath, resolveS3ConfigAndInput, @@ -26,6 +26,19 @@ const isCopyInputWithPath = ( input: CopyInput | CopyWithPathInput, ): input is CopyWithPathInput => isInputWithPath(input.source); +const storageBucketAssertion = ( + sourceBucket?: StorageBucket, + destBucket?: StorageBucket, +) => + // Throw assertion error when either one of bucket options is empty + { + assertValidationError( + (sourceBucket !== undefined && destBucket !== undefined) || + (!destBucket && !sourceBucket), + StorageValidationErrorCode.InvalidCopyOperationStorageBucket, + ); + }; + export const copy = async ( amplify: AmplifyClassV6, input: CopyInput | CopyWithPathInput, @@ -40,8 +53,18 @@ const copyWithPath = async ( input: CopyWithPathInput, ): Promise => { const { source, destination } = input; - const { s3Config, bucket, identityId } = - await resolveS3ConfigAndInput(amplify); + + storageBucketAssertion(source.bucket, destination.bucket); + + const { bucket: sourceBucket, identityId } = await resolveS3ConfigAndInput( + amplify, + input.source, + ); + + const { s3Config, bucket: destBucket } = await resolveS3ConfigAndInput( + amplify, + input.destination, + ); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. assertValidationError(!!source.path, StorageValidationErrorCode.NoSourcePath); assertValidationError( @@ -58,14 +81,14 @@ const copyWithPath = async ( identityId, ); - const finalCopySource = `${bucket}/${sourcePath}`; + const finalCopySource = `${sourceBucket}/${sourcePath}`; const finalCopyDestination = destinationPath; logger.debug(`copying "${finalCopySource}" to "${finalCopyDestination}".`); await serviceCopy({ source: finalCopySource, destination: finalCopyDestination, - bucket, + bucket: destBucket, s3Config, }); @@ -77,41 +100,39 @@ export const copyWithKey = async ( amplify: AmplifyClassV6, input: CopyInput, ): Promise => { - const { - source: { key: sourceKey }, - destination: { key: destinationKey }, - } = input; + const { source, destination } = input; + + storageBucketAssertion(source.bucket, destination.bucket); - assertValidationError(!!sourceKey, StorageValidationErrorCode.NoSourceKey); + assertValidationError(!!source.key, StorageValidationErrorCode.NoSourceKey); assertValidationError( - !!destinationKey, + !!destination.key, StorageValidationErrorCode.NoDestinationKey, ); + const { bucket: sourceBucket, keyPrefix: sourceKeyPrefix } = + await resolveS3ConfigAndInput(amplify, source); + const { s3Config, - bucket, - keyPrefix: sourceKeyPrefix, - } = await resolveS3ConfigAndInput(amplify, input.source); - const { keyPrefix: destinationKeyPrefix } = await resolveS3ConfigAndInput( - amplify, - input.destination, - ); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. + bucket: destBucket, + keyPrefix: destinationKeyPrefix, + } = await resolveS3ConfigAndInput(amplify, destination); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. // TODO(ashwinkumar6) V6-logger: warn `You may copy files from another user if the source level is "protected", currently it's ${srcLevel}` - const finalCopySource = `${bucket}/${sourceKeyPrefix}${sourceKey}`; - const finalCopyDestination = `${destinationKeyPrefix}${destinationKey}`; + const finalCopySource = `${sourceBucket}/${sourceKeyPrefix}${source.key}`; + const finalCopyDestination = `${destinationKeyPrefix}${destination.key}`; logger.debug(`copying "${finalCopySource}" to "${finalCopyDestination}".`); await serviceCopy({ source: finalCopySource, destination: finalCopyDestination, - bucket, + bucket: destBucket, s3Config, }); return { - key: destinationKey, + key: destination.key, }; }; diff --git a/packages/storage/src/providers/s3/types/index.ts b/packages/storage/src/providers/s3/types/index.ts index 4299687cd8e..4efd666fb33 100644 --- a/packages/storage/src/providers/s3/types/index.ts +++ b/packages/storage/src/providers/s3/types/index.ts @@ -17,6 +17,8 @@ export { DownloadDataOptionsWithKey, CopyDestinationOptionsWithKey, CopySourceOptionsWithKey, + CopyWithPathSourceOptions, + CopyWithPathDestinationOptions, } from './options'; export { UploadDataOutput, diff --git a/packages/storage/src/providers/s3/types/inputs.ts b/packages/storage/src/providers/s3/types/inputs.ts index f7bd6c5db44..214f910e0d9 100644 --- a/packages/storage/src/providers/s3/types/inputs.ts +++ b/packages/storage/src/providers/s3/types/inputs.ts @@ -20,6 +20,8 @@ import { import { CopyDestinationOptionsWithKey, CopySourceOptionsWithKey, + CopyWithPathDestinationOptions, + CopyWithPathSourceOptions, DownloadDataOptionsWithKey, DownloadDataOptionsWithPath, GetPropertiesOptionsWithKey, @@ -47,7 +49,10 @@ export type CopyInput = StorageCopyInputWithKey< /** * Input type with path for S3 copy API. */ -export type CopyWithPathInput = StorageCopyInputWithPath; +export type CopyWithPathInput = StorageCopyInputWithPath< + CopyWithPathSourceOptions, + CopyWithPathDestinationOptions +>; /** * @deprecated Use {@link GetPropertiesWithPathInput} instead. diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 0642d943ced..e5d7318f010 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -170,14 +170,23 @@ export type UploadDataOptionsWithPath = UploadDataOptions; export type CopySourceOptionsWithKey = ReadOptions & { /** @deprecated This may be removed in the next major version. */ key: string; + bucket?: StorageBucket; }; /** @deprecated This may be removed in the next major version. */ export type CopyDestinationOptionsWithKey = WriteOptions & { /** @deprecated This may be removed in the next major version. */ key: string; + bucket?: StorageBucket; }; +export interface CopyWithPathSourceOptions { + bucket?: StorageBucket; +} +export interface CopyWithPathDestinationOptions { + bucket?: StorageBucket; +} + /** * Internal only type for S3 API handlers' config parameter. * diff --git a/packages/storage/src/types/inputs.ts b/packages/storage/src/types/inputs.ts index 403a2a14332..a8136e5d1a6 100644 --- a/packages/storage/src/types/inputs.ts +++ b/packages/storage/src/types/inputs.ts @@ -90,9 +90,9 @@ export interface StorageCopyInputWithKey< destination: DestinationOptions; } -export interface StorageCopyInputWithPath { - source: StorageOperationInputWithPath; - destination: StorageOperationInputWithPath; +export interface StorageCopyInputWithPath { + source: StorageOperationInputWithPath & SourceOptions; + destination: StorageOperationInputWithPath & DestinationOptions; } /** From 043d913f0702a6c86b30c154c826d2f03b760ed7 Mon Sep 17 00:00:00 2001 From: ashika112 <155593080+ashika112@users.noreply.github.com> Date: Fri, 19 Jul 2024 16:03:54 -0700 Subject: [PATCH 4/8] [Chore] Enable Integ test and tagged release on Multi-bucket work (#13619) --- .github/workflows/push-preid-release.yml | 2 +- .../src/providers/s3/apis/internal/copy.ts | 21 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/push-preid-release.yml b/.github/workflows/push-preid-release.yml index 9837290ed14..8634313df1f 100644 --- a/.github/workflows/push-preid-release.yml +++ b/.github/workflows/push-preid-release.yml @@ -9,7 +9,7 @@ on: push: branches: # Change this to your branch name where "example-preid" corresponds to the preid you want your changes released on - - feat/example-preid-branch/main + - feat/multi-bucket jobs: e2e: diff --git a/packages/storage/src/providers/s3/apis/internal/copy.ts b/packages/storage/src/providers/s3/apis/internal/copy.ts index 29554ce4978..5035f897017 100644 --- a/packages/storage/src/providers/s3/apis/internal/copy.ts +++ b/packages/storage/src/providers/s3/apis/internal/copy.ts @@ -29,15 +29,18 @@ const isCopyInputWithPath = ( const storageBucketAssertion = ( sourceBucket?: StorageBucket, destBucket?: StorageBucket, -) => - // Throw assertion error when either one of bucket options is empty - { - assertValidationError( - (sourceBucket !== undefined && destBucket !== undefined) || - (!destBucket && !sourceBucket), - StorageValidationErrorCode.InvalidCopyOperationStorageBucket, - ); - }; +) => { + /** For multi-bucket, both source and destination bucket needs to be passed in + * or both can be undefined and we fallback to singleton's default value + */ + assertValidationError( + // Both src & dest bucket option is present is acceptable + (sourceBucket !== undefined && destBucket !== undefined) || + // or both are undefined is also acceptable + (!destBucket && !sourceBucket), + StorageValidationErrorCode.InvalidCopyOperationStorageBucket, + ); +}; export const copy = async ( amplify: AmplifyClassV6, From 381ca11248a34e5cd6fb00dfa09df635e3f43845 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Tue, 23 Jul 2024 13:01:22 -0700 Subject: [PATCH 5/8] update bundle size --- packages/aws-amplify/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 04ff7146b2f..35c982c2a71 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -485,7 +485,7 @@ "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "15.21 kB" + "limit": "15.30 kB" }, { "name": "[Storage] remove (S3)", From 6e5b3c58fe2d1c3e01eed99fd53036ddeb075a35 Mon Sep 17 00:00:00 2001 From: Allan Zheng Date: Tue, 23 Jul 2024 18:25:30 -0700 Subject: [PATCH 6/8] feat(storage): resolve merge issue with multibucket --- .../src/providers/s3/apis/internal/copy.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/internal/copy.ts b/packages/storage/src/providers/s3/apis/internal/copy.ts index ae0117334c1..a8562ac3beb 100644 --- a/packages/storage/src/providers/s3/apis/internal/copy.ts +++ b/packages/storage/src/providers/s3/apis/internal/copy.ts @@ -56,20 +56,27 @@ const copyWithPath = async ( input: CopyWithPathInput, ): Promise => { const { source, destination } = input; - // TODO(@AllanZhengYP) - await resolveS3ConfigAndInput(amplify, input); storageBucketAssertion(source.bucket, destination.bucket); - const { bucket: sourceBucket, identityId } = await resolveS3ConfigAndInput( - amplify, - input.source, - ); + const { bucket: sourceBucket } = await resolveS3ConfigAndInput(amplify, { + path: input.source.path, + options: { ...input.source }, + }); - const { s3Config, bucket: destBucket } = await resolveS3ConfigAndInput( - amplify, - input.destination, - ); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. + // The bucket, region, credentials of s3 client are resolved from destination. + // Whereas the source bucket and path are a input parameter of S3 copy operation. + const { + s3Config, + bucket: destBucket, + identityId, + } = await resolveS3ConfigAndInput(amplify, { + path: input.destination.path, + options: { + locationCredentialsProvider: input.options?.locationCredentialsProvider, + ...input.destination, + }, + }); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. assertValidationError(!!source.path, StorageValidationErrorCode.NoSourcePath); assertValidationError( @@ -121,6 +128,8 @@ export const copyWithKey = async ( options: input.source, }); + // The bucket, region, credentials of s3 client are resolved from destination. + // Whereas the source bucket and path are a input parameter of S3 copy operation. const { s3Config, bucket: destBucket, From 048ea00a8233bae5eec06894664fe02bd49ffc53 Mon Sep 17 00:00:00 2001 From: Allan Zheng Date: Wed, 24 Jul 2024 09:14:56 -0700 Subject: [PATCH 7/8] chore: update bundle size for config change and s3 multibucket --- packages/aws-amplify/package.json | 54 +++++++++++++++--------------- packages/interactions/package.json | 6 ++-- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 35c982c2a71..47c23fac960 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -293,31 +293,31 @@ "name": "[Analytics] record (Pinpoint)", "path": "./dist/esm/analytics/index.mjs", "import": "{ record }", - "limit": "17.18 kB" + "limit": "17.23 kB" }, { "name": "[Analytics] record (Kinesis)", "path": "./dist/esm/analytics/kinesis/index.mjs", "import": "{ record }", - "limit": "48.61 kB" + "limit": "48.65 kB" }, { "name": "[Analytics] record (Kinesis Firehose)", "path": "./dist/esm/analytics/kinesis-firehose/index.mjs", "import": "{ record }", - "limit": "45.76 kB" + "limit": "45.81 kB" }, { "name": "[Analytics] record (Personalize)", "path": "./dist/esm/analytics/personalize/index.mjs", "import": "{ record }", - "limit": "49.58 kB" + "limit": "49.63 kB" }, { "name": "[Analytics] identifyUser (Pinpoint)", "path": "./dist/esm/analytics/index.mjs", "import": "{ identifyUser }", - "limit": "15.68 kB" + "limit": "15.73 kB" }, { "name": "[Analytics] enable", @@ -335,7 +335,7 @@ "name": "[API] generateClient (AppSync)", "path": "./dist/esm/api/index.mjs", "import": "{ generateClient }", - "limit": "40.14 kB" + "limit": "40.19 kB" }, { "name": "[API] REST API handlers", @@ -353,13 +353,13 @@ "name": "[Auth] resetPassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ resetPassword }", - "limit": "12.53 kB" + "limit": "12.58 kB" }, { "name": "[Auth] confirmResetPassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmResetPassword }", - "limit": "12.47 kB" + "limit": "12.52 kB" }, { "name": "[Auth] signIn (Cognito)", @@ -371,7 +371,7 @@ "name": "[Auth] resendSignUpCode (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ resendSignUpCode }", - "limit": "12.49 kB" + "limit": "12.53 kB" }, { "name": "[Auth] confirmSignUp (Cognito)", @@ -383,31 +383,31 @@ "name": "[Auth] confirmSignIn (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmSignIn }", - "limit": "28.38 kB" + "limit": "28.42 kB" }, { "name": "[Auth] updateMFAPreference (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updateMFAPreference }", - "limit": "11.83 kB" + "limit": "11.87 kB" }, { "name": "[Auth] fetchMFAPreference (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ fetchMFAPreference }", - "limit": "11.86 kB" + "limit": "11.91 kB" }, { "name": "[Auth] verifyTOTPSetup (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ verifyTOTPSetup }", - "limit": "12.71 kB" + "limit": "12.75 kB" }, { "name": "[Auth] updatePassword (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updatePassword }", - "limit": "12.73 kB" + "limit": "12.76 kB" }, { "name": "[Auth] setUpTOTP (Cognito)", @@ -419,7 +419,7 @@ "name": "[Auth] updateUserAttributes (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ updateUserAttributes }", - "limit": "11.95 kB" + "limit": "11.99 kB" }, { "name": "[Auth] getCurrentUser (Cognito)", @@ -431,73 +431,73 @@ "name": "[Auth] confirmUserAttribute (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ confirmUserAttribute }", - "limit": "12.71 kB" + "limit": "12.75 kB" }, { "name": "[Auth] signInWithRedirect (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signInWithRedirect }", - "limit": "21.15 kB" + "limit": "21.19 kB" }, { "name": "[Auth] fetchUserAttributes (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ fetchUserAttributes }", - "limit": "11.77 kB" + "limit": "11.82 kB" }, { "name": "[Auth] Basic Auth Flow (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signIn, signOut, fetchAuthSession, confirmSignIn }", - "limit": "30.15 kB" + "limit": "30.20 kB" }, { "name": "[Auth] OAuth Auth Flow (Cognito)", "path": "./dist/esm/auth/index.mjs", "import": "{ signInWithRedirect, signOut, fetchAuthSession }", - "limit": "21.58 kB" + "limit": "21.62 kB" }, { "name": "[Storage] copy (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ copy }", - "limit": "14.86 kB" + "limit": "15.24 kB" }, { "name": "[Storage] downloadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ downloadData }", - "limit": "15.45 kB" + "limit": "15.76 kB" }, { "name": "[Storage] getProperties (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getProperties }", - "limit": "14.70 kB" + "limit": "15.03 kB" }, { "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "15.79 kB" + "limit": "16.09 kB" }, { "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "15.30 kB" + "limit": "15.65 kB" }, { "name": "[Storage] remove (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ remove }", - "limit": "14.56 kB" + "limit": "14.88 kB" }, { "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "19.85 kB" + "limit": "20.18 kB" } ] } diff --git a/packages/interactions/package.json b/packages/interactions/package.json index 9194df2c3b6..98359327196 100644 --- a/packages/interactions/package.json +++ b/packages/interactions/package.json @@ -89,19 +89,19 @@ "name": "Interactions (default to Lex v2)", "path": "./dist/esm/index.mjs", "import": "{ Interactions }", - "limit": "52.61 kB" + "limit": "52.66 kB" }, { "name": "Interactions (Lex v2)", "path": "./dist/esm/lex-v2/index.mjs", "import": "{ Interactions }", - "limit": "52.61 kB" + "limit": "52.66 kB" }, { "name": "Interactions (Lex v1)", "path": "./dist/esm/lex-v1/index.mjs", "import": "{ Interactions }", - "limit": "47.41 kB" + "limit": "47.46 kB" } ] } From 4f9f9d8e8c0d7bfa0fa9877cd46378ac94b0fb31 Mon Sep 17 00:00:00 2001 From: Allan Zheng Date: Wed, 24 Jul 2024 10:43:44 -0700 Subject: [PATCH 8/8] chore: address feedbacks --- .../src/providers/s3/apis/internal/copy.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/internal/copy.ts b/packages/storage/src/providers/s3/apis/internal/copy.ts index a8562ac3beb..9119917efa1 100644 --- a/packages/storage/src/providers/s3/apis/internal/copy.ts +++ b/packages/storage/src/providers/s3/apis/internal/copy.ts @@ -61,7 +61,10 @@ const copyWithPath = async ( const { bucket: sourceBucket } = await resolveS3ConfigAndInput(amplify, { path: input.source.path, - options: { ...input.source }, + options: { + locationCredentialsProvider: input.options?.locationCredentialsProvider, + ...input.source, + }, }); // The bucket, region, credentials of s3 client are resolved from destination. @@ -125,7 +128,12 @@ export const copyWithKey = async ( const { bucket: sourceBucket, keyPrefix: sourceKeyPrefix } = await resolveS3ConfigAndInput(amplify, { ...input, - options: input.source, + options: { + // @ts-expect-error: 'options' does not exist on type 'CopyInput'. In case of JS users set the location + // credentials provider option, resolveS3ConfigAndInput will throw validation error. + locationCredentialsProvider: input.options?.locationCredentialsProvider, + ...input.source, + }, }); // The bucket, region, credentials of s3 client are resolved from destination. @@ -136,7 +144,12 @@ export const copyWithKey = async ( keyPrefix: destinationKeyPrefix, } = await resolveS3ConfigAndInput(amplify, { ...input, - options: input.destination, + options: { + // @ts-expect-error: 'options' does not exist on type 'CopyInput'. In case of JS users set the location + // credentials provider option, resolveS3ConfigAndInput will throw validation error. + locationCredentialsProvider: input.options?.locationCredentialsProvider, + ...input.destination, + }, }); // resolveS3ConfigAndInput does not make extra API calls or storage access if called repeatedly. // TODO(ashwinkumar6) V6-logger: warn `You may copy files from another user if the source level is "protected", currently it's ${srcLevel}`