Skip to content

Commit

Permalink
[Multi-Bucket] Update Copy API (#13607)
Browse files Browse the repository at this point in the history
* copy multibucket draft 1

* update copy with key and tests

* update the path route

* cleanup

* update bundle size

* address minor nits
  • Loading branch information
ashika112 authored Jul 19, 2024
1 parent 9073217 commit b9cfe68
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 33 deletions.
14 changes: 7 additions & 7 deletions packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
]
}
94 changes: 94 additions & 0 deletions packages/storage/__tests__/providers/s3/apis/copy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -64,6 +65,7 @@ describe('copy API', () => {
S3: {
bucket,
region,
buckets: { 'bucket-1': { bucketName: bucket, region } },
},
},
});
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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',
},
);
});
});
});

Expand Down Expand Up @@ -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,
);
}
});
});
});
4 changes: 4 additions & 0 deletions packages/storage/src/errors/types/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export enum StorageValidationErrorCode {
NoBucket = 'NoBucket',
NoRegion = 'NoRegion',
InvalidStorageBucket = 'InvalidStorageBucket',
InvalidCopyOperationStorageBucket = 'InvalidCopyOperationStorageBucket',
InvalidStorageOperationPrefixInput = 'InvalidStorageOperationPrefixInput',
InvalidStorageOperationInput = 'InvalidStorageOperationInput',
InvalidStoragePathInput = 'InvalidStoragePathInput',
Expand Down Expand Up @@ -75,4 +76,7 @@ export const validationErrorMap: AmplifyErrorMap<StorageValidationErrorCode> = {
message:
'Unable to lookup bucket from provided name in Amplify configuration.',
},
[StorageValidationErrorCode.InvalidCopyOperationStorageBucket]: {
message: 'Missing bucket option in either source or destination.',
},
};
65 changes: 43 additions & 22 deletions packages/storage/src/providers/s3/apis/internal/copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
CopyWithPathInput,
CopyWithPathOutput,
} from '../../types';
import { ResolvedS3Config } from '../../types/options';
import { ResolvedS3Config, StorageBucket } from '../../types/options';
import {
isInputWithPath,
resolveS3ConfigAndInput,
Expand All @@ -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,
Expand All @@ -40,8 +53,18 @@ const copyWithPath = async (
input: CopyWithPathInput,
): Promise<CopyWithPathOutput> => {
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(
Expand All @@ -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,
});

Expand All @@ -77,41 +100,39 @@ export const copyWithKey = async (
amplify: AmplifyClassV6,
input: CopyInput,
): Promise<CopyOutput> => {
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,
};
};

Expand Down
2 changes: 2 additions & 0 deletions packages/storage/src/providers/s3/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export {
DownloadDataOptionsWithKey,
CopyDestinationOptionsWithKey,
CopySourceOptionsWithKey,
CopyWithPathSourceOptions,
CopyWithPathDestinationOptions,
} from './options';
export {
UploadDataOutput,
Expand Down
7 changes: 6 additions & 1 deletion packages/storage/src/providers/s3/types/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
import {
CopyDestinationOptionsWithKey,
CopySourceOptionsWithKey,
CopyWithPathDestinationOptions,
CopyWithPathSourceOptions,
DownloadDataOptionsWithKey,
DownloadDataOptionsWithPath,
GetPropertiesOptionsWithKey,
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions packages/storage/src/providers/s3/types/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
6 changes: 3 additions & 3 deletions packages/storage/src/types/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ export interface StorageCopyInputWithKey<
destination: DestinationOptions;
}

export interface StorageCopyInputWithPath {
source: StorageOperationInputWithPath;
destination: StorageOperationInputWithPath;
export interface StorageCopyInputWithPath<SourceOptions, DestinationOptions> {
source: StorageOperationInputWithPath & SourceOptions;
destination: StorageOperationInputWithPath & DestinationOptions;
}

/**
Expand Down

0 comments on commit b9cfe68

Please sign in to comment.