address feedback

aws-amplify · Jul 22, 2024 · da383b3 · da383b3
1 parent b2fdbb4
commit da383b3
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 35 deletions.
diff --git a/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts b/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts
@@ -182,7 +182,7 @@ describe('resolveS3ConfigAndInput', () => {
 	it('should resolve prefix with given access level', async () => {
 		mockDefaultResolvePrefix.mockResolvedValueOnce('prefix');
 		const { keyPrefix } = await resolveS3ConfigAndInput(Amplify, {
-			accessLevel: 'someLevel' as any,
+			options: { accessLevel: 'someLevel' as any },
 		});
 		expect(mockDefaultResolvePrefix).toHaveBeenCalledWith({
 			accessLevel: 'someLevel',
@@ -232,7 +232,9 @@ describe('resolveS3ConfigAndInput', () => {
 				},
 			});
 			const { s3Config } = await resolveS3ConfigAndInput(Amplify, {
-				locationCredentialsProvider: mockLocationCredentialsProvider,
+				options: {
+					locationCredentialsProvider: mockLocationCredentialsProvider,
+				},
 			});
 
 			if (typeof s3Config.credentials === 'function') {
@@ -267,18 +269,16 @@ describe('resolveS3ConfigAndInput', () => {
 			const testCases = [...deprecatedInputs, ...callbackPathInputs];
 
 			it.each(testCases)('should throw when input is %s', async input => {
-				const { s3Config } = await resolveS3ConfigAndInput(
-					Amplify,
-					{ locationCredentialsProvider: mockLocationCredentialsProvider },
-					input,
-				);
+				const { s3Config } = await resolveS3ConfigAndInput(Amplify, {
+					...input,
+					options: {
+						locationCredentialsProvider: mockLocationCredentialsProvider,
+					},
+				});
 				if (typeof s3Config.credentials === 'function') {
 					await expect(s3Config.credentials()).rejects.toThrow(
 						expect.objectContaining({
 							name: INVALID_STORAGE_INPUT,
-							message: 'The input needs to have a path as a string value.',
-							recoverySuggestion:
-								'Please provide a valid path as a string value for the input.',
 						}),
 					);
 				} else {

diff --git a/packages/storage/src/providers/s3/apis/downloadData.ts b/packages/storage/src/providers/s3/apis/downloadData.ts
@@ -115,11 +115,7 @@ const downloadDataJob =
 	> => {
 		const { options: downloadDataOptions } = downloadDataInput;
 		const { bucket, keyPrefix, s3Config, identityId } =
-			await resolveS3ConfigAndInput(
-				Amplify,
-				downloadDataOptions,
-				downloadDataInput,
-			);
+			await resolveS3ConfigAndInput(Amplify, downloadDataInput);
 		const { inputType, objectKey } = validateStorageOperationInput(
 			downloadDataInput,
 			identityId,

diff --git a/packages/storage/src/providers/s3/apis/internal/copy.ts b/packages/storage/src/providers/s3/apis/internal/copy.ts
@@ -42,7 +42,6 @@ const copyWithPath = async (
 	const { source, destination } = input;
 	const { s3Config, bucket, identityId } = await resolveS3ConfigAndInput(
 		amplify,
-		{},
 		input,
 	);
 
@@ -95,11 +94,13 @@ export const copyWithKey = async (
 		s3Config,
 		bucket,
 		keyPrefix: sourceKeyPrefix,
-	} = await resolveS3ConfigAndInput(amplify, input.source, input);
+	} = await resolveS3ConfigAndInput(amplify, {
+		...input,
+		options: input.source,
+	});
 	const { keyPrefix: destinationKeyPrefix } = await resolveS3ConfigAndInput(
 		amplify,
-		input.destination,
-		input,
+		{ ...input, options: 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}`

diff --git a/packages/storage/src/providers/s3/apis/internal/getProperties.ts b/packages/storage/src/providers/s3/apis/internal/getProperties.ts
@@ -24,9 +24,8 @@ export const getProperties = async (
 	input: GetPropertiesInput | GetPropertiesWithPathInput,
 	action?: StorageAction,
 ): Promise<GetPropertiesOutput | GetPropertiesWithPathOutput> => {
-	const { options: getPropertiesOptions } = input;
 	const { s3Config, bucket, keyPrefix, identityId } =
-		await resolveS3ConfigAndInput(amplify, getPropertiesOptions, input);
+		await resolveS3ConfigAndInput(amplify, input);
 	const { inputType, objectKey } = validateStorageOperationInput(
 		input,
 		identityId,

diff --git a/packages/storage/src/providers/s3/apis/internal/getUrl.ts b/packages/storage/src/providers/s3/apis/internal/getUrl.ts
@@ -31,7 +31,7 @@ export const getUrl = async (
 ): Promise<GetUrlOutput | GetUrlWithPathOutput> => {
 	const { options: getUrlOptions } = input;
 	const { s3Config, keyPrefix, bucket, identityId } =
-		await resolveS3ConfigAndInput(amplify, getUrlOptions, input);
+		await resolveS3ConfigAndInput(amplify, input);
 	const { inputType, objectKey } = validateStorageOperationInput(
 		input,
 		identityId,

diff --git a/packages/storage/src/providers/s3/apis/internal/list.ts b/packages/storage/src/providers/s3/apis/internal/list.ts
@@ -58,7 +58,7 @@ export const list = async (
 		bucket,
 		keyPrefix: generatedPrefix,
 		identityId,
-	} = await resolveS3ConfigAndInput(amplify, options, input);
+	} = await resolveS3ConfigAndInput(amplify, input);
 
 	const { inputType, objectKey } = validateStorageOperationInputWithPrefix(
 		input,

diff --git a/packages/storage/src/providers/s3/apis/internal/remove.ts b/packages/storage/src/providers/s3/apis/internal/remove.ts
@@ -23,9 +23,8 @@ export const remove = async (
 	amplify: AmplifyClassV6,
 	input: RemoveInput | RemoveWithPathInput,
 ): Promise<RemoveOutput | RemoveWithPathOutput> => {
-	const { options = {} } = input ?? {};
 	const { s3Config, keyPrefix, bucket, identityId } =
-		await resolveS3ConfigAndInput(amplify, options, input);
+		await resolveS3ConfigAndInput(amplify, input);
 
 	const { inputType, objectKey } = validateStorageOperationInput(
 		input,

diff --git a/packages/storage/src/providers/s3/apis/uploadData/multipart/uploadHandlers.ts b/packages/storage/src/providers/s3/apis/uploadData/multipart/uploadHandlers.ts
@@ -73,7 +73,6 @@ export const getMultipartUploadHandlers = (
 		const { options: uploadDataOptions, data } = uploadDataInput;
 		const resolvedS3Options = await resolveS3ConfigAndInput(
 			Amplify,
-			uploadDataOptions,
 			uploadDataInput,
 		);
 

diff --git a/packages/storage/src/providers/s3/apis/uploadData/putObjectJob.ts b/packages/storage/src/providers/s3/apis/uploadData/putObjectJob.ts
@@ -29,11 +29,7 @@ export const putObjectJob =
 	async (): Promise<ItemWithKey | ItemWithPath> => {
 		const { options: uploadDataOptions, data } = uploadDataInput;
 		const { bucket, keyPrefix, s3Config, isObjectLockEnabled, identityId } =
-			await resolveS3ConfigAndInput(
-				Amplify,
-				uploadDataOptions,
-				uploadDataInput,
-			);
+			await resolveS3ConfigAndInput(Amplify, uploadDataInput);
 		const { inputType, objectKey } = validateStorageOperationInput(
 			uploadDataInput,
 			identityId,

diff --git a/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts b/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts
@@ -17,7 +17,21 @@ import {
 	StorageOperationInputWithPrefix,
 } from '../../../types/inputs';
 import { StorageError } from '../../../errors/StorageError';
-import { CopyInput } from '../types';
+import {
+	CopyInput,
+	CopyWithPathInput,
+	DownloadDataInput,
+	DownloadDataWithPathInput,
+	GetPropertiesInput,
+	GetPropertiesWithPathInput,
+	GetUrlInput,
+	GetUrlWithPathInput,
+	ListAllInput,
+	ListAllWithPathInput,
+	ListPaginateInput,
+	ListPaginateWithPathInput,
+	RemoveWithPathInput,
+} from '../types';
 import { INVALID_STORAGE_INPUT } from '../../../errors/constants';
 
 import { DEFAULT_ACCESS_LEVEL, LOCAL_TESTING_S3_ENDPOINT } from './constants';
@@ -37,14 +51,30 @@ interface ResolvedS3ConfigAndInput {
 	identityId?: string;
 }
 export type DeprecatedStorageInput =
+	| DownloadDataInput
+	| GetUrlInput
 	| StorageOperationInputWithKey
 	| StorageOperationInputWithPrefix
+	| CopyInput
+	| ListAllInput
+	| ListPaginateInput
+	| GetPropertiesInput
 	| CopyInput;
 
 export type CallbackPathStorageInput =
 	| StorageOperationInputWithPath
 	| StorageCopyInputWithPath;
 
+type StorageInput =
+	| DeprecatedStorageInput
+	| DownloadDataWithPathInput
+	| RemoveWithPathInput
+	| ListAllWithPathInput
+	| ListPaginateWithPathInput
+	| GetPropertiesWithPathInput
+	| GetUrlWithPathInput
+	| CopyWithPathInput;
+
 /**
  * resolve the common input options for S3 API handlers from Amplify configuration and library options.
  *
@@ -58,9 +88,9 @@ export type CallbackPathStorageInput =
  */
 export const resolveS3ConfigAndInput = async (
 	amplify: AmplifyClassV6,
-	apiOptions?: S3ApiOptions,
-	input?: DeprecatedStorageInput | CallbackPathStorageInput,
+	apiInput?: StorageInput & { options?: S3ApiOptions },
 ): Promise<ResolvedS3ConfigAndInput> => {
+	const { options: apiOptions } = apiInput ?? {};
 	/**
 	 * IdentityId is always cached in memory so we can safely make calls here. It
 	 * should be stable even for unauthenticated users, regardless of credentials.
@@ -75,7 +105,9 @@ export const resolveS3ConfigAndInput = async (
 	 */
 	const credentialsProvider = async () => {
 		if (isLocationCredentialsProvider(apiOptions)) {
-			assertStorageInput(input);
+			assertStorageInput(
+				apiInput as DeprecatedStorageInput | CallbackPathStorageInput,
+			);
 		}
 
 		const { credentials } = isLocationCredentialsProvider(apiOptions)