From 3d70792afc120ac38aed145b1c87797890262184 Mon Sep 17 00:00:00 2001 From: israx <70438514+israx@users.noreply.github.com> Date: Tue, 23 Jul 2024 08:19:21 -0400 Subject: [PATCH 1/2] fix(storage): omit subPathStrategy when prefix is defined (#13618) * fix(storage): omit subPathStrategy when prefix is defined (#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 --- .../__tests__/providers/s3/types/list.test.ts | 154 ++++++++++++++++++ .../__tests__/providers/s3/types/utils.ts | 6 + .../src/providers/s3/apis/internal/list.ts | 16 +- .../storage/src/providers/s3/types/options.ts | 9 +- .../storage/src/providers/s3/types/outputs.ts | 10 +- packages/storage/src/types/options.ts | 2 - 6 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 packages/storage/__tests__/providers/s3/types/list.test.ts create mode 100644 packages/storage/__tests__/providers/s3/types/utils.ts diff --git a/packages/storage/__tests__/providers/s3/types/list.test.ts b/packages/storage/__tests__/providers/s3/types/list.test.ts new file mode 100644 index 00000000000..0993be017e3 --- /dev/null +++ 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>, + Expect>, + Expect< + Equal + >, + Expect>, + ]; + type Result = Expect>; + } + }); +}); + +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>, + Expect>, + Expect< + Equal + >, + Expect>, + ]; + + type Result = Expect>; + } + }); +}); diff --git a/packages/storage/__tests__/providers/s3/types/utils.ts b/packages/storage/__tests__/providers/s3/types/utils.ts new file mode 100644 index 00000000000..4dd26ef396c --- /dev/null +++ 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; + +export type Equal = 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 index 1035559bedc..7fe8ccf1ed0 100644 --- 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 index b2b7dfd0ddc..c042d167263 100644 --- 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 index 44524536a3b..ec3b89941e0 100644 --- 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; +export type ListAllOutput = Omit< + StorageListOutput, + 'excludedSubpaths' +>; /** * Output type with path for S3 list API. Lists all bucket objects. @@ -105,7 +108,10 @@ export type ListAllWithPathOutput = StorageListOutput; * @deprecated Use {@link ListPaginateWithPathOutput} instead. * Output type for S3 list API. Lists bucket objects with pagination. */ -export type ListPaginateOutput = StorageListOutput & { +export type ListPaginateOutput = Omit< + StorageListOutput, + 'excludedSubpaths' +> & { nextToken?: string; }; diff --git a/packages/storage/src/types/options.ts b/packages/storage/src/types/options.ts index 93e35acc9f7..8b9880cfd23 100644 --- 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; From b473ce34bac46c4bec6226e4a62f078dbd75fb94 Mon Sep 17 00:00:00 2001 From: Ashwin Kumar Date: Tue, 23 Jul 2024 10:13:25 -0700 Subject: [PATCH 2/2] fix(rtn-web-browser): signInWithRedirect needs to be called twice on Android * fix(rtn-web-browser): signInWithRedirect needs to be called twice on Android * chore: add code owner * address feedback * update codeowners * update codeowners * Update .github/CODEOWNERS --------- Co-authored-by: Ashwin Kumar --- .github/CODEOWNERS | 1 + .../src/apis/openAuthSessionAsync.ts | 34 ++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 90671e2cce4..78ed19747ec 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -27,6 +27,7 @@ /packages/core/src/clients/internal @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF /packages/core/src/Hub @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF /packages/adapter-nextjs @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF +/packages/rtn-web-browser @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF /packages/storage/src/providers/s3/apis/internal @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF /packages/storage/src/providers/s3/apis/server @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF /packages/api-rest/src/apis/server.ts @ukhan-amazon @haverchuck @cshfang @jimblanc @HuiSF diff --git a/packages/rtn-web-browser/src/apis/openAuthSessionAsync.ts b/packages/rtn-web-browser/src/apis/openAuthSessionAsync.ts index e6091abdeac..7ddee8ec30f 100644 --- a/packages/rtn-web-browser/src/apis/openAuthSessionAsync.ts +++ b/packages/rtn-web-browser/src/apis/openAuthSessionAsync.ts @@ -61,34 +61,44 @@ const openAuthSessionAndroid = async (url: string, redirectUrls: string[]) => { return redirectUrl; } finally { - appStateListener?.remove(); - redirectListener?.remove(); - appStateListener = undefined; - redirectListener = undefined; + removeAppStateListener(); + removeRedirectListener(); } }; const getAppStatePromise = (): Promise => new Promise(resolve => { - appStateListener = AppState.addEventListener('change', nextAppState => { - // if current state is null, the change is from initialization - if (AppState.currentState === null) { - return; - } + // remove any stray listeners before creating new ones + removeAppStateListener(); - if (nextAppState === 'active') { - appStateListener?.remove(); - appStateListener = undefined; + let previousState = AppState.currentState; + appStateListener = AppState.addEventListener('change', nextAppState => { + if (previousState !== 'active' && nextAppState === 'active') { + removeAppStateListener(); resolve(null); } + previousState = nextAppState; }); }); const getRedirectPromise = (redirectUrls: string[]): Promise => new Promise(resolve => { + // remove any stray listeners before creating new ones + removeRedirectListener(); + redirectListener = Linking.addEventListener('url', event => { if (redirectUrls.some(url => event.url.startsWith(url))) { resolve(event.url); } }); }); + +const removeAppStateListener = () => { + appStateListener?.remove(); + appStateListener = undefined; +}; + +const removeRedirectListener = () => { + redirectListener?.remove(); + redirectListener = undefined; +};