From 64e86c663d4d5d2cd6f55797466325f1efe9dfc5 Mon Sep 17 00:00:00 2001 From: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Date: Tue, 5 Sep 2023 10:02:09 -0700 Subject: [PATCH 1/3] fix(adapter-nextjs): remove usage of node http (#11970) --- ...torageAdapterFromNextServerContext.test.ts | 2 +- ...okieStorageAdapterFromNextServerContext.ts | 46 +++++++++++++------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts index 8769cc57038..ba506bd3dc2 100644 --- a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts +++ b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts @@ -224,7 +224,7 @@ describe('createCookieStorageAdapterFromNextServerContext', () => { it('should throw error when no cookie storage adapter is created from the context', () => { expect(() => createCookieStorageAdapterFromNextServerContext({ - request: {} as any, + request: undefined, response: new ServerResponse({} as any), } as any) ).toThrowError(); diff --git a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts index faae65ef037..51fe055a765 100644 --- a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts +++ b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts @@ -7,18 +7,41 @@ import { AmplifyServerContextError, CookieStorage, } from '@aws-amplify/core/internals/adapter-core'; -import { IncomingMessage, ServerResponse } from 'http'; export const DATE_IN_THE_PAST = new Date(0); export const createCookieStorageAdapterFromNextServerContext = ( context: NextServer.Context ): CookieStorage.Adapter => { - const { request, response } = context as - | NextServer.NextRequestAndNextResponseContext - | NextServer.NextRequestAndResponseContext; + const { request: req, response: res } = + context as Partial<NextServer.GetServerSidePropsContext>; + + // When the server context is from `getServerSideProps`, the `req` is an instance + // of IncomingMessage, and the `res` is an instance of ServerResponse. + // We cannot import these two classes here from `http` as it breaks in Next + // Edge Runtime. Hence, we check the methods that we need to use for creating + // cookie adapter. + if ( + req && + res && + Object.prototype.toString.call(req.cookies) === '[object Object]' && + typeof res.setHeader === 'function' + ) { + return createCookieStorageAdapterFromGetServerSidePropsContext(req, res); + } - if (request instanceof NextRequest && response) { + const { request, response } = context as Partial< + | NextServer.NextRequestAndNextResponseContext + | NextServer.NextRequestAndResponseContext + >; + + // When the server context is from `middleware`, the `request` is an instance + // of `NextRequest`. + // When the server context is from a route handler, the `request` is an `Proxy` + // wrapped `Request`. + // The `NextRequest` and the `Proxy` are sharing the same interface by Next + // implementation. So we don't need to detect the difference. + if (request && response) { if (response instanceof NextResponse) { return createCookieStorageAdapterFromNextRequestAndNextResponse( request, @@ -32,21 +55,14 @@ export const createCookieStorageAdapterFromNextServerContext = ( } } - const { cookies } = context as - | NextServer.ServerComponentContext - | NextServer.ServerActionContext; + const { cookies } = context as Partial< + NextServer.ServerComponentContext | NextServer.ServerActionContext + >; if (typeof cookies === 'function') { return createCookieStorageAdapterFromNextCookies(cookies); } - const { request: req, response: res } = - context as NextServer.GetServerSidePropsContext; - - if (req instanceof IncomingMessage && res instanceof ServerResponse) { - return createCookieStorageAdapterFromGetServerSidePropsContext(req, res); - } - // This should not happen normally. throw new AmplifyServerContextError({ message: From e561195255d4eb0e92bb1f2a4d5c0fbd31b9c28b Mon Sep 17 00:00:00 2001 From: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Date: Tue, 5 Sep 2023 13:04:48 -0700 Subject: [PATCH 2/3] fix(storage): resolveS3ConfigAndInput not using injected fetchAuthSession (#11976) --- .../storage/__tests__/providers/s3/apis/copy.test.ts | 11 +++++------ .../__tests__/providers/s3/apis/downloadData.test.ts | 7 +++++-- .../providers/s3/apis/getProperties.test.ts | 12 +++++++----- .../__tests__/providers/s3/apis/getUrl.test.ts | 7 +++++-- .../storage/__tests__/providers/s3/apis/list.test.ts | 7 +++++-- .../__tests__/providers/s3/apis/remove.test.ts | 7 +++++-- .../s3/apis/uploadData/multipartHandlers.test.ts | 8 +++++--- .../s3/apis/uploadData/putObjectJob.test.ts | 7 +++++-- .../s3/apis/utils/resolveS3ConfigAndInput.test.ts | 8 +++++--- .../providers/s3/utils/resolveS3ConfigAndInput.ts | 8 ++------ 10 files changed, 49 insertions(+), 33 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index 29c9222b13c..a15fb06a8b3 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -2,11 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { - Amplify, - StorageAccessLevel, - fetchAuthSession, -} from '@aws-amplify/core'; +import { Amplify, StorageAccessLevel } from '@aws-amplify/core'; import { copyObject } from '../../../../src/providers/s3/utils/client'; import { copy } from '../../../../src/providers/s3/apis'; @@ -15,10 +11,13 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); const mockCopyObject = copyObject as jest.Mock; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; const sourceKey = 'sourceKey'; diff --git a/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts b/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts index 7d6eb9d34ca..7dec964df35 100644 --- a/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/downloadData.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { getObject } from '../../../../src/providers/s3/utils/client'; import { downloadData } from '../../../../src/providers/s3'; import { createDownloadTask } from '../../../../src/providers/s3/utils'; @@ -12,6 +12,9 @@ jest.mock('../../../../src/providers/s3/utils'); jest.mock('@aws-amplify/core', () => ({ Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, fetchAuthSession: jest.fn(), })); @@ -22,7 +25,7 @@ const credentials: Credentials = { }; const identityId = 'identityId'; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockCreateDownloadTask = createDownloadTask as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index cf2861b46fc..f993720c27d 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -4,19 +4,21 @@ import { headObject } from '../../../../src/providers/s3/utils/client'; import { getProperties } from '../../../../src/providers/s3'; import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; jest.mock('../../../../src/providers/s3/utils/client'); -const mockHeadObject = headObject as jest.Mock; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; -const mockGetConfig = Amplify.getConfig as jest.Mock; - jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); +const mockHeadObject = headObject as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; +const mockGetConfig = Amplify.getConfig as jest.Mock; const bucket = 'bucket'; const region = 'region'; diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 1c0bace8457..464a92680e9 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -3,7 +3,7 @@ import { getUrl } from '../../../../src/providers/s3/apis'; import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { getPresignedGetObjectUrl, headObject, @@ -14,12 +14,15 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); const bucket = 'bucket'; const region = 'region'; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; const credentials: Credentials = { accessKeyId: 'accessKeyId', diff --git a/packages/storage/__tests__/providers/s3/apis/list.test.ts b/packages/storage/__tests__/providers/s3/apis/list.test.ts index fff394e71e6..fa23e9a035b 100644 --- a/packages/storage/__tests__/providers/s3/apis/list.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/list.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { listObjectsV2 } from '../../../../src/providers/s3/utils/client'; import { list } from '../../../../src/providers/s3/apis'; @@ -11,9 +11,12 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; const mockListObject = listObjectsV2 as jest.Mock; const key = 'path/itemsKey'; diff --git a/packages/storage/__tests__/providers/s3/apis/remove.test.ts b/packages/storage/__tests__/providers/s3/apis/remove.test.ts index d11a2d7ab21..b682a4d068b 100644 --- a/packages/storage/__tests__/providers/s3/apis/remove.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/remove.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { deleteObject } from '../../../../src/providers/s3/utils/client'; import { remove } from '../../../../src/providers/s3/apis'; @@ -11,10 +11,13 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); const mockDeleteObject = deleteObject as jest.Mock; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; const key = 'key'; const bucket = 'bucket'; diff --git a/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts b/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts index 7695bacf670..be3f70399cd 100644 --- a/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/uploadData/multipartHandlers.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { Amplify, LocalStorage, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify, LocalStorage } from '@aws-amplify/core'; import { createMultipartUpload, uploadPart, @@ -27,8 +27,10 @@ jest.mock('@aws-amplify/core', () => ({ Amplify: { getConfig: jest.fn(), libraryOptions: {}, + Auth: { + fetchAuthSession: jest.fn(), + }, }, - fetchAuthSession: jest.fn(), })); jest.mock( '../../../../../src/providers/s3/apis/uploadData/multipart/uploadCache/kvStorage', @@ -50,7 +52,7 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const identityId = 'identityId'; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const bucket = 'bucket'; const region = 'region'; const defaultKey = 'key'; diff --git a/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts b/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts index dbf856ca437..80c97bdb405 100644 --- a/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/uploadData/putObjectJob.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { putObject } from '../../../../../src/providers/s3/utils/client'; import { calculateContentMd5 } from '../../../../../src/providers/s3/utils'; import { putObjectJob } from '../../../../../src/providers/s3/apis/uploadData/putObjectJob'; @@ -19,6 +19,9 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); const credentials: Credentials = { @@ -27,7 +30,7 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const identityId = 'identityId'; -const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const mockPutObject = putObject as jest.Mock; mockFetchAuthSession.mockResolvedValue({ diff --git a/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts b/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts index 8f825d8b7b2..fa81c393967 100644 --- a/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/utils/resolveS3ConfigAndInput.test.ts @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { resolveS3ConfigAndInput } from '../../../../../src/providers/s3/utils'; import { resolvePrefix } from '../../../../../src/utils/resolvePrefix'; @@ -11,16 +11,18 @@ import { } from '../../../../../src/errors/types/validation'; jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), + Auth: { + fetchAuthSession: jest.fn(), + }, }, })); jest.mock('../../../../../src/utils/resolvePrefix'); -const mockFetchAuthSession = fetchAuthSession as jest.Mock; const mockGetConfig = Amplify.getConfig as jest.Mock; const mockDefaultResolvePrefix = resolvePrefix as jest.Mock; +const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock; const bucket = 'bucket'; const region = 'region'; diff --git a/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts b/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts index c9c3bc39241..6606ff794b5 100644 --- a/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts +++ b/packages/storage/src/providers/s3/utils/resolveS3ConfigAndInput.ts @@ -1,11 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { - AmplifyClassV6, - fetchAuthSession, - StorageAccessLevel, -} from '@aws-amplify/core'; +import { AmplifyClassV6, StorageAccessLevel } from '@aws-amplify/core'; import { assertValidationError } from '../../../errors/utils/assertValidationError'; import { StorageValidationErrorCode } from '../../../errors/types/validation'; import { StorageError } from '../../../errors/StorageError'; @@ -44,7 +40,7 @@ export const resolveS3ConfigAndInput = async ( apiOptions?: S3ApiOptions ): Promise<ResolvedS3ConfigAndInput> => { // identityId is always cached in memory if forceRefresh is not set. So we can safely make calls here. - const { credentials, identityId } = await fetchAuthSession({ + const { credentials, identityId } = await amplify.Auth.fetchAuthSession({ forceRefresh: false, }); assertValidationError( From 7188d04534f8e8a41a51bca714479fe299b9c128 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota <34170013+kvramyasri7@users.noreply.github.com> Date: Tue, 5 Sep 2023 14:23:35 -0700 Subject: [PATCH 3/3] (chore)storage: add accessLevel unit tests to getProperties, getUrl apis (#11962) * chore: add accessLevel tests for getProperties and getUrl --- .../__tests__/providers/s3/apis/copy.test.ts | 5 +- .../providers/s3/apis/getProperties.test.ts | 159 +++++++++++------- .../providers/s3/apis/getUrl.test.ts | 143 ++++++++++------ .../__tests__/providers/s3/apis/list.test.ts | 1 - .../providers/s3/apis/remove.test.ts | 1 - 5 files changed, 191 insertions(+), 118 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index a15fb06a8b3..83d8dbd2176 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -8,7 +8,6 @@ import { copy } from '../../../../src/providers/s3/apis'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), Auth: { @@ -42,7 +41,7 @@ const copyObjectClientBaseParams = { /** * bucket is appended at start if it's a sourceKey - * guest: public/${targetIdentityId}/${key}` + * guest: public/${key}` * private: private/${targetIdentityId}/${key}` * protected: protected/${targetIdentityId}/${key}` */ @@ -51,6 +50,8 @@ const buildClientRequestKey = ( KeyType: 'source' | 'destination', accessLevel: StorageAccessLevel ) => { + const targetIdentityId = 'targetIdentityId'; + const bucket = 'bucket'; const finalAccessLevel = accessLevel == 'guest' ? 'public' : accessLevel; let finalKey = KeyType == 'source' ? `${bucket}/` : ''; finalKey += `${finalAccessLevel}/`; diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index f993720c27d..0485b8c5869 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -5,10 +5,10 @@ import { headObject } from '../../../../src/providers/s3/utils/client'; import { getProperties } from '../../../../src/providers/s3'; import { Credentials } from '@aws-sdk/types'; import { Amplify } from '@aws-amplify/core'; +import { StorageOptions } from '../../../../src/types'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), Auth: { @@ -28,87 +28,116 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; +const identityId = 'identityId'; -describe('getProperties test', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - mockFetchAuthSession.mockResolvedValue({ - credentials, - identityId: targetIdentityId, - }); - mockGetConfig.mockReturnValue({ - Storage: { - S3: { - bucket, - region, - }, - }, - }); - it('getProperties happy path case with private check', async () => { - expect.assertions(3); - mockHeadObject.mockReturnValueOnce({ - ContentLength: '100', - ContentType: 'text/plain', - ETag: 'etag', - LastModified: 'last-modified', - Metadata: { key: 'value' }, - VersionId: 'version-id', +describe('getProperties api', () => { + beforeAll(() => { + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId, }); - const metadata = { key: 'value' }; - expect( - await getProperties({ - key: 'key', - options: { - targetIdentityId: 'targetIdentityId', - accessLevel: 'protected', + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket, + region, }, - }) - ).toEqual({ + }, + }); + }); + describe('getProperties happy path ', () => { + const expected = { key: 'key', size: '100', contentType: 'text/plain', eTag: 'etag', - metadata, + metadata: { key: 'value' }, lastModified: 'last-modified', versionId: 'version-id', + }; + const config = { + credentials, + region: 'region', + }; + const key = 'key'; + beforeEach(() => { + mockHeadObject.mockReturnValueOnce({ + ContentLength: '100', + ContentType: 'text/plain', + ETag: 'etag', + LastModified: 'last-modified', + Metadata: { key: 'value' }, + VersionId: 'version-id', + }); + }); + afterEach(() => { + jest.clearAllMocks(); }); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + it.each([ + { + options: { accessLevel: 'guest' }, + expectedKey: 'public/key', + }, { - credentials, - region: 'region', + options: { accessLevel: 'protected', targetIdentityId }, + expectedKey: 'protected/targetIdentityId/key', }, { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', + options: { accessLevel: 'protected' }, + expectedKey: 'protected/identityId/key', + }, + { + options: { accessLevel: 'private' }, + expectedKey: 'private/identityId/key', + }, + ])( + 'getProperties api with $options.accessLevel', + async ({ options, expectedKey }) => { + const headObjectOptions = { + Bucket: 'bucket', + Key: expectedKey, + }; + expect.assertions(3); + expect( + await getProperties({ + key, + options: options as StorageOptions, + }) + ).toEqual(expected); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); } ); }); - it('getProperties should return a not found error', async () => { - mockHeadObject.mockRejectedValueOnce( - Object.assign(new Error(), { - $metadata: { httpStatusCode: 404 }, - name: 'NotFound', - }) - ); - try { - await getProperties({ key: 'keyed' }); - } catch (error) { - expect.assertions(3); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'public/keyed', - } + describe('getProperties error path', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('getProperties should return a not found error', async () => { + mockHeadObject.mockRejectedValueOnce( + Object.assign(new Error(), { + $metadata: { httpStatusCode: 404 }, + name: 'NotFound', + }) ); - expect(error.$metadata.httpStatusCode).toBe(404); - } + try { + await getProperties({ key: 'keyed' }); + } catch (error) { + expect.assertions(3); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + }, + { + Bucket: 'bucket', + Key: 'public/keyed', + } + ); + expect(error.$metadata.httpStatusCode).toBe(404); + } + }); }); }); diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 464a92680e9..4cd7d7586bf 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -8,10 +8,10 @@ import { getPresignedGetObjectUrl, headObject, } from '../../../../src/providers/s3/utils/client'; +import { StorageOptions } from '../../../../src/types'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), Auth: { @@ -30,60 +30,105 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; +const identityId = 'identityId'; -describe('getProperties test', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - mockFetchAuthSession.mockResolvedValue({ - credentials, - identityId: targetIdentityId, - }); - mockGetConfig.mockReturnValue({ - Storage: { - S3: { - bucket, - region, +describe('getUrl test', () => { + beforeAll(() => { + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId, + }); + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket, + region, + }, }, - }, - }); - it('get presigned url happy case', async () => { - expect.assertions(2); - (headObject as jest.Mock).mockImplementation(() => { - return { - Key: 'key', - ContentLength: '100', - ContentType: 'text/plain', - ETag: 'etag', - LastModified: 'last-modified', - Metadata: { key: 'value' }, - }; }); - (getPresignedGetObjectUrl as jest.Mock).mockReturnValueOnce({ - url: new URL('https://google.com'), + }); + describe('getUrl happy path', () => { + const config = { + credentials, + region: 'region', + }; + const key = 'key'; + beforeEach(() => { + (headObject as jest.Mock).mockImplementation(() => { + return { + Key: 'key', + ContentLength: '100', + ContentType: 'text/plain', + ETag: 'etag', + LastModified: 'last-modified', + Metadata: { key: 'value' }, + }; + }); + (getPresignedGetObjectUrl as jest.Mock).mockReturnValueOnce({ + url: new URL('https://google.com'), + }); }); - const result = await getUrl({ key: 'key' }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(result.url).toEqual({ - url: new URL('https://google.com'), + afterEach(() => { + jest.clearAllMocks(); }); - }); - test('Should return not found error when the object is not found', async () => { - (headObject as jest.Mock).mockImplementation(() => - Object.assign(new Error(), { - $metadata: { httpStatusCode: 404 }, - name: 'NotFound', - }) - ); - try { - await getUrl({ - key: 'invalid_key', - options: { validateObjectExistence: true }, + it.each([ + { + options: { accessLevel: 'guest' }, + expectedKey: 'public/key', + }, + { + options: { accessLevel: 'protected', targetIdentityId }, + expectedKey: 'protected/targetIdentityId/key', + }, + { + options: { accessLevel: 'protected' }, + expectedKey: 'protected/identityId/key', + }, + { + options: { accessLevel: 'private' }, + expectedKey: 'private/identityId/key', + }, + ])('getUrl with $options.accessLevel', async ({ options, expectedKey }) => { + const headObjectOptions = { + Bucket: 'bucket', + Key: expectedKey, + }; + const optionsVal = { ...options, validateObjectExistence: true }; + + expect.assertions(4); + const result = await getUrl({ + key, + options: optionsVal as StorageOptions, }); - } catch (error) { - expect.assertions(2); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); expect(headObject).toBeCalledTimes(1); - expect(error.$metadata?.httpStatusCode).toBe(404); - } + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); + expect(result.url).toEqual({ + url: new URL('https://google.com'), + }); + }); + }); + describe('getUrl error path', () => { + afterAll(() => { + jest.clearAllMocks(); + }); + it('Should return not found error when the object is not found', async () => { + (headObject as jest.Mock).mockImplementation(() => + Object.assign(new Error(), { + $metadata: { httpStatusCode: 404 }, + name: 'NotFound', + }) + ); + try { + await getUrl({ + key: 'invalid_key', + options: { validateObjectExistence: true }, + }); + } catch (error) { + expect.assertions(2); + expect(headObject).toBeCalledTimes(1); + expect(error.$metadata?.httpStatusCode).toBe(404); + } + }); }); }); diff --git a/packages/storage/__tests__/providers/s3/apis/list.test.ts b/packages/storage/__tests__/providers/s3/apis/list.test.ts index fa23e9a035b..6defa0e89aa 100644 --- a/packages/storage/__tests__/providers/s3/apis/list.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/list.test.ts @@ -8,7 +8,6 @@ import { list } from '../../../../src/providers/s3/apis'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), Auth: { diff --git a/packages/storage/__tests__/providers/s3/apis/remove.test.ts b/packages/storage/__tests__/providers/s3/apis/remove.test.ts index b682a4d068b..365d38ecb9c 100644 --- a/packages/storage/__tests__/providers/s3/apis/remove.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/remove.test.ts @@ -8,7 +8,6 @@ import { remove } from '../../../../src/providers/s3/apis'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ - fetchAuthSession: jest.fn(), Amplify: { getConfig: jest.fn(), Auth: {