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; } /**