fix(storage): omit subPathStrategy when prefix is defined (aws-amplif…

…y#13618) * fix(storage): omit subPathStrategy when prefix is defined (aws-amplify#13606) * fix: omit subPathStrategy on prefix * chore: fix build * chore: address feedback * chore: omit subpathStrategy from options * chore: add unit tests * chore: update tests * chore: fix test * chore: move subpathstrategy to service options * chore: update comment * chore: fix type
ashwinkumar6 · Jul 23, 2024 · 3d70792 · 3d70792
1 parent 9fb5988
commit 3d70792
Show file tree

Hide file tree

Showing 6 changed files with 185 additions and 12 deletions.
diff --git a/packages/storage/__tests__/providers/s3/types/list.test.ts b/packages/storage/__tests__/providers/s3/types/list.test.ts
@@ -0,0 +1,154 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+/* eslint-disable unused-imports/no-unused-vars */
+
+import { StorageAccessLevel } from '@aws-amplify/core';
+
+import {
+	ListAllInput,
+	ListAllOutput,
+	ListAllWithPathInput,
+	ListAllWithPathOutput,
+	ListOutputItem,
+	ListOutputItemWithPath,
+	ListPaginateInput,
+	ListPaginateOutput,
+	ListPaginateWithPathInput,
+	ListPaginateWithPathOutput,
+} from '../../../../src/providers/s3/types';
+import { StorageSubpathStrategy } from '../../../../src/types';
+
+import { Equal, Expect } from './utils';
+
+interface Input {
+	targetIdentityId?: string;
+	prefix?: string;
+	path: string;
+	subpathStrategy?: StorageSubpathStrategy;
+	nextToken: string;
+	pageSize: number;
+	useAccelerateEndpoint: boolean;
+	accessLevel: StorageAccessLevel;
+	listAll: boolean;
+}
+
+interface Output {
+	listOutputItems: ListOutputItem[];
+	listOutputItemsWithPath: ListOutputItemWithPath[];
+	excludedSubpaths: string[];
+	nextToken: string;
+}
+
+describe('List API input types', () => {
+	test('should compile', () => {
+		function handleTest({
+			targetIdentityId,
+			prefix,
+			path,
+			subpathStrategy,
+			nextToken,
+			pageSize,
+			useAccelerateEndpoint,
+			accessLevel,
+		}: Input) {
+			const listPaginateInput: ListPaginateInput = {
+				prefix,
+				options: {
+					accessLevel: 'protected',
+					targetIdentityId,
+					// @ts-expect-error subpathStrategy is not part of this input
+					subpathStrategy,
+				},
+			};
+
+			const listAllInput: ListAllInput = {
+				prefix,
+				options: {
+					listAll: true,
+					accessLevel: 'protected',
+					targetIdentityId,
+					// @ts-expect-error subpathStrategy is not part of this input
+					subpathStrategy,
+				},
+			};
+
+			const listPaginateWithPathInput: ListPaginateWithPathInput = {
+				path,
+				options: {
+					subpathStrategy,
+					useAccelerateEndpoint,
+					pageSize,
+					nextToken,
+				},
+			};
+
+			const listAllWithPathInput: ListAllWithPathInput = {
+				path,
+				options: {
+					listAll: true,
+					subpathStrategy,
+					useAccelerateEndpoint,
+					// @ts-expect-error pageSize is not part of this input
+					pageSize,
+				},
+			};
+
+			type Tests = [
+				Expect<Equal<typeof listPaginateInput, ListPaginateInput>>,
+				Expect<Equal<typeof listAllInput, ListAllInput>>,
+				Expect<
+					Equal<typeof listPaginateWithPathInput, ListPaginateWithPathInput>
+				>,
+				Expect<Equal<typeof listAllWithPathInput, ListAllWithPathInput>>,
+			];
+			type Result = Expect<Equal<Tests, [true, true, true, true]>>;
+		}
+	});
+});
+
+describe('List API ouput types', () => {
+	test('should compile', () => {
+		function handleTest({
+			listOutputItems,
+			nextToken,
+			excludedSubpaths,
+			listOutputItemsWithPath,
+		}: Output) {
+			const listPaginateOutput: ListPaginateOutput = {
+				items: listOutputItems,
+				nextToken,
+				// @ts-expect-error excludeSubpaths is not part of this output
+				excludedSubpaths,
+			};
+
+			const listAllOutput: ListAllOutput = {
+				items: listOutputItems,
+				// @ts-expect-error excludeSubpaths is not part of this output
+				excludedSubpaths,
+			};
+
+			const listPaginateWithPathOutput: ListPaginateWithPathOutput = {
+				items: listOutputItemsWithPath,
+				nextToken,
+				excludedSubpaths,
+			};
+
+			const listAllWithPathOutput: ListAllWithPathOutput = {
+				items: listOutputItemsWithPath,
+				excludedSubpaths,
+			};
+
+			type Tests = [
+				Expect<Equal<typeof listPaginateOutput, ListPaginateOutput>>,
+				Expect<Equal<typeof listAllOutput, ListAllOutput>>,
+				Expect<
+					Equal<typeof listPaginateWithPathOutput, ListPaginateWithPathOutput>
+				>,
+				Expect<Equal<typeof listAllWithPathOutput, ListAllWithPathOutput>>,
+			];
+
+			type Result = Expect<Equal<Tests, [true, true, true, true]>>;
+		}
+	});
+});
diff --git a/packages/storage/__tests__/providers/s3/types/utils.ts b/packages/storage/__tests__/providers/s3/types/utils.ts
@@ -0,0 +1,6 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+export type Expect<T extends true> = T;
+
+export type Equal<X, Y> = X extends Y ? true : false;
diff --git a/packages/storage/src/providers/s3/apis/internal/list.ts b/packages/storage/src/providers/s3/apis/internal/list.ts
@@ -20,7 +20,11 @@ import {
 	resolveS3ConfigAndInput,
 	validateStorageOperationInputWithPrefix,
 } from '../../utils';
-import { ResolvedS3Config } from '../../types/options';
+import {
+	ListAllOptionsWithPath,
+	ListPaginateOptionsWithPath,
+	ResolvedS3Config,
+} from '../../types/options';
 import {
 	ListObjectsV2Input,
 	ListObjectsV2Output,
@@ -30,7 +34,6 @@ import { getStorageUserAgentValue } from '../../utils/userAgent';
 import { logger } from '../../../../utils';
 import { DEFAULT_DELIMITER, STORAGE_INPUT_PREFIX } from '../../utils/constants';
 import { CommonPrefix } from '../../utils/client/types';
-import { StorageSubpathStrategy } from '../../../../types';
 
 const MAX_PAGE_SIZE = 1000;
 
@@ -76,12 +79,13 @@ export const list = async (
 			} ${anyOptions?.nextToken ? `nextToken: ${anyOptions?.nextToken}` : ''}.`,
 		);
 	}
+
 	const listParams = {
 		Bucket: bucket,
 		Prefix: isInputWithPrefix ? `${generatedPrefix}${objectKey}` : objectKey,
 		MaxKeys: options?.listAll ? undefined : options?.pageSize,
 		ContinuationToken: options?.listAll ? undefined : options?.nextToken,
-		Delimiter: getDelimiter(options.subpathStrategy),
+		Delimiter: getDelimiter(options),
 	};
 	logger.debug(`listing items from "${listParams.Prefix}"`);
 
@@ -263,9 +267,9 @@ const mapCommonPrefixesToExcludedSubpaths = (
 };
 
 const getDelimiter = (
-	subpathStrategy?: StorageSubpathStrategy,
+	options?: ListAllOptionsWithPath | ListPaginateOptionsWithPath,
 ): string | undefined => {
-	if (subpathStrategy?.strategy === 'exclude') {
-		return subpathStrategy?.delimiter ?? DEFAULT_DELIMITER;
+	if (options?.subpathStrategy?.strategy === 'exclude') {
+		return options?.subpathStrategy?.delimiter ?? DEFAULT_DELIMITER;
 	}
 };
diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts
@@ -8,6 +8,7 @@ import { TransferProgressEvent } from '../../../types';
 import {
 	StorageListAllOptions,
 	StorageListPaginateOptions,
+	StorageSubpathStrategy,
 } from '../../../types/options';
 
 interface CommonOptions {
@@ -89,7 +90,9 @@ export type ListAllOptionsWithPath = Omit<
 	StorageListAllOptions,
 	'accessLevel'
 > &
-	CommonOptions;
+	CommonOptions & {
+		subpathStrategy?: StorageSubpathStrategy;
+	};
 
 /**
  * Input options type with path for S3 list API to paginate items.
@@ -98,7 +101,9 @@ export type ListPaginateOptionsWithPath = Omit<
 	StorageListPaginateOptions,
 	'accessLevel'
 > &
-	CommonOptions;
+	CommonOptions & {
+		subpathStrategy?: StorageSubpathStrategy;
+	};
 
 /**
  * Input options type for S3 getUrl API.

diff --git a/packages/storage/src/providers/s3/types/outputs.ts b/packages/storage/src/providers/s3/types/outputs.ts
@@ -94,7 +94,10 @@ export type GetPropertiesWithPathOutput = ItemBase & StorageItemWithPath;
  * @deprecated Use {@link ListAllWithPathOutput} instead.
  * Output type for S3 list API. Lists all bucket objects.
  */
-export type ListAllOutput = StorageListOutput<ListOutputItem>;
+export type ListAllOutput = Omit<
+	StorageListOutput<ListOutputItem>,
+	'excludedSubpaths'
+>;
 
 /**
  * Output type with path for S3 list API. Lists all bucket objects.
@@ -105,7 +108,10 @@ export type ListAllWithPathOutput = StorageListOutput<ListOutputItemWithPath>;
  * @deprecated Use {@link ListPaginateWithPathOutput} instead.
  * Output type for S3 list API. Lists bucket objects with pagination.
  */
-export type ListPaginateOutput = StorageListOutput<ListOutputItem> & {
+export type ListPaginateOutput = Omit<
+	StorageListOutput<ListOutputItem>,
+	'excludedSubpaths'
+> & {
 	nextToken?: string;
 };
 

diff --git a/packages/storage/src/types/options.ts b/packages/storage/src/types/options.ts
@@ -10,14 +10,12 @@ export interface StorageOptions {
 
 export type StorageListAllOptions = StorageOptions & {
 	listAll: true;
-	subpathStrategy?: StorageSubpathStrategy;
 };
 
 export type StorageListPaginateOptions = StorageOptions & {
 	listAll?: false;
 	pageSize?: number;
 	nextToken?: string;
-	subpathStrategy?: StorageSubpathStrategy;
 };
 
 export type StorageRemoveOptions = StorageOptions;