From 45f0ca765bbf0622717dfc6b1a76981ae9da7fd9 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Thu, 31 Aug 2023 21:01:50 -0700 Subject: [PATCH 01/20] chore: add accessLevel tests for getProperties api --- .../providers/s3/apis/getProperties.test.ts | 206 ++++++++++++------ 1 file changed, 143 insertions(+), 63 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index cf2861b46fc..b2c08fea7d1 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -27,74 +27,91 @@ const credentials: Credentials = { }; const targetIdentityId = 'targetIdentityId'; -describe('getProperties test', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - mockFetchAuthSession.mockResolvedValue({ - credentials, - identityId: targetIdentityId, - }); - mockGetConfig.mockReturnValue({ - Storage: { - S3: { - bucket, - region, +describe('getProperties api', () => { + beforeAll(() => { + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId: targetIdentityId, + }); + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket, + region, + }, }, - }, + }); + afterEach(() => { + jest.clearAllMocks(); + }); }); - 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 happy path ', () => { + beforeEach(() => { + mockHeadObject.mockReturnValueOnce({ + ContentLength: '100', + ContentType: 'text/plain', + ETag: 'etag', + LastModified: 'last-modified', + Metadata: { key: 'value' }, + VersionId: 'version-id', + }); }); - const metadata = { key: 'value' }; - expect( - await getProperties({ + afterEach(() => { + jest.clearAllMocks(); + }); + + it('getProperties with guest accessLevel', async () => { + expect.assertions(3); + const metadata = { key: 'value' }; + expect( + await getProperties({ + key: 'key', + options: { + accessLevel: 'guest', + }, + }) + ).toEqual({ key: 'key', - options: { - targetIdentityId: 'targetIdentityId', - accessLevel: 'protected', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata, + lastModified: 'last-modified', + versionId: 'version-id', + }); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', }, - }) - ).toEqual({ - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata, - lastModified: 'last-modified', - versionId: 'version-id', + { + Bucket: 'bucket', + Key: 'public/key', + } + ); }); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - } - ); - }); - 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) { + it('getProperties with protected accessLevel', async () => { expect.assertions(3); + const metadata = { key: 'value' }; + expect( + await getProperties({ + key: 'key', + options: { + targetIdentityId: 'targetIdentityId', + accessLevel: 'protected', + }, + }) + ).toEqual({ + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata, + lastModified: 'last-modified', + versionId: 'version-id', + }); expect(headObject).toBeCalledTimes(1); expect(headObject).toHaveBeenCalledWith( { @@ -103,10 +120,73 @@ describe('getProperties test', () => { }, { Bucket: 'bucket', - Key: 'public/keyed', + Key: 'protected/targetIdentityId/key', } ); - expect(error.$metadata.httpStatusCode).toBe(404); - } + }); + + it('getProperties with private accessLevel', async () => { + expect.assertions(3); + const metadata = { key: 'value' }; + expect( + await getProperties({ + key: 'key', + options: { + targetIdentityId: 'targetIdentityId', + accessLevel: 'protected', + }, + }) + ).toEqual({ + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata, + lastModified: 'last-modified', + versionId: 'version-id', + }); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + }, + { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + } + ); + }); + }); + + 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', + }) + ); + 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); + } + }); }); }); From 69b21dac43f38a01a60b9d5c986f3f3a1b5741e8 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Thu, 31 Aug 2023 21:18:07 -0700 Subject: [PATCH 02/20] chore: add getUrl accessLevel unit test case --- .../providers/s3/apis/getProperties.test.ts | 2 +- .../providers/s3/apis/getUrl.test.ts | 174 +++++++++++++----- 2 files changed, 128 insertions(+), 48 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index b2c08fea7d1..1796fa75855 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -159,7 +159,7 @@ describe('getProperties api', () => { }); }); - describe('getProperties Error path', () => { + describe('getProperties error path', () => { afterEach(() => { jest.clearAllMocks(); }); diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 1c0bace8457..8bd4331bd48 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -28,59 +28,139 @@ const credentials: Credentials = { }; const targetIdentityId = 'targetIdentityId'; -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: targetIdentityId, + }); + 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' }, - }; + describe('getUrl happy path', () => { + 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'), + }); }); - (getPresignedGetObjectUrl as jest.Mock).mockReturnValueOnce({ - url: new URL('https://google.com'), + afterAll(() => { + jest.clearAllMocks(); }); - const result = await getUrl({ key: 'key' }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(result.url).toEqual({ - url: new URL('https://google.com'), + it('get presigned url with guest accessLevel', async () => { + expect.assertions(4); + const result = await getUrl({ + key: 'key', + options: { + accessLevel: 'guest', + validateObjectExistence: true, + }, + }); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + }, + { + Bucket: 'bucket', + Key: 'public/key', + } + ); + expect(result.url).toEqual({ + url: new URL('https://google.com'), + }); }); - }); - 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('get presigned url with protected accessLevel', async () => { + expect.assertions(4); + const result = await getUrl({ + key: 'key', + options: { + accessLevel: 'protected', + targetIdentityId, + validateObjectExistence: true, + }, }); - } catch (error) { - expect.assertions(2); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); expect(headObject).toBeCalledTimes(1); - expect(error.$metadata?.httpStatusCode).toBe(404); - } + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + }, + { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + } + ); + expect(result.url).toEqual({ + url: new URL('https://google.com'), + }); + }); + it('get presigned url with private accessLevel', async () => { + expect.assertions(4); + const result = await getUrl({ + key: 'key', + options: { + accessLevel: 'protected', + targetIdentityId, + validateObjectExistence: true, + }, + }); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + }, + { + Bucket: 'bucket', + Key: 'private/targetIdentityId/key', + } + ); + 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); + } + }); }); }); From 037ee6d5c6f90ea90404d38b017c03b83b3c33eb Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 09:38:57 -0700 Subject: [PATCH 03/20] chore: wip --- packages/storage/__tests__/providers/s3/apis/getUrl.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 8bd4331bd48..0d6e4911f8e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -59,9 +59,10 @@ describe('getUrl test', () => { url: new URL('https://google.com'), }); }); - afterAll(() => { + afterEach(() => { jest.clearAllMocks(); }); + it('get presigned url with guest accessLevel', async () => { expect.assertions(4); const result = await getUrl({ @@ -118,8 +119,7 @@ describe('getUrl test', () => { const result = await getUrl({ key: 'key', options: { - accessLevel: 'protected', - targetIdentityId, + accessLevel: 'private', validateObjectExistence: true, }, }); From ebb40653d0d9cc88e20ba88c50219efb33f91629 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 14:06:14 -0700 Subject: [PATCH 04/20] chore: increase bundle size --- packages/core/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/package.json b/packages/core/package.json index f80f59f825d..bbfc5aab16d 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -128,7 +128,7 @@ "name": "Custom clients (url presigner)", "path": "./lib-esm/clients/middleware/signing/signer/signatureV4/index.js", "import": "{ presignUrl }", - "limit": "6.3 kB" + "limit": "6.31 kB" }, { "name": "Cache (default browser storage)", From 5f37f5d1bb648b2273a45ae4abbe49d1b1303fff Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 15:53:20 -0700 Subject: [PATCH 05/20] chore: getProperties use ir.each() method --- .../providers/s3/apis/getProperties.test.ts | 130 ++++++++---------- 1 file changed, 55 insertions(+), 75 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 1796fa75855..7033ad4fcf1 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -5,6 +5,7 @@ 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 { StorageOptions } from '../../../../src/types'; jest.mock('../../../../src/providers/s3/utils/client'); const mockHeadObject = headObject as jest.Mock; @@ -59,28 +60,19 @@ describe('getProperties api', () => { afterEach(() => { jest.clearAllMocks(); }); - - it('getProperties with guest accessLevel', async () => { - expect.assertions(3); - const metadata = { key: 'value' }; - expect( - await getProperties({ + it.each([ + [ + 'key', + { accessLevel: 'guest' }, + { key: 'key', - options: { - accessLevel: 'guest', - }, - }) - ).toEqual({ - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata, - lastModified: 'last-modified', - versionId: 'version-id', - }); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, { credentials, region: 'region', @@ -88,32 +80,20 @@ describe('getProperties api', () => { { Bucket: 'bucket', Key: 'public/key', - } - ); - }); - - it('getProperties with protected accessLevel', async () => { - expect.assertions(3); - const metadata = { key: 'value' }; - expect( - await getProperties({ + }, + ], + [ + 'key', + { accessLevel: 'protected', targetIdentityId: 'targetIdentityId' }, + { key: 'key', - options: { - targetIdentityId: 'targetIdentityId', - accessLevel: 'protected', - }, - }) - ).toEqual({ - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata, - lastModified: 'last-modified', - versionId: 'version-id', - }); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, { credentials, region: 'region', @@ -121,42 +101,42 @@ describe('getProperties api', () => { { Bucket: 'bucket', Key: 'protected/targetIdentityId/key', - } - ); - }); - - it('getProperties with private accessLevel', async () => { - expect.assertions(3); - const metadata = { key: 'value' }; - expect( - await getProperties({ + }, + ], + [ + 'key', + { accessLevel: 'private' }, + { key: 'key', - options: { - targetIdentityId: 'targetIdentityId', - accessLevel: 'protected', - }, - }) - ).toEqual({ - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata, - lastModified: 'last-modified', - versionId: 'version-id', - }); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, { credentials, region: 'region', }, { Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - } - ); - }); + Key: 'private/targetIdentityId/key', + }, + ], + ])( + 'getProperties with all accessLevels', + async (key, options, expected, config, headObjectOptions) => { + expect( + await getProperties({ + key, + options: options as StorageOptions, + }) + ).toEqual(expected); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); + } + ); }); describe('getProperties error path', () => { From 462331ec7827a5b336f31fb2450cf6ea68143a36 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 16:20:35 -0700 Subject: [PATCH 06/20] chore: add getUrl it.each() method --- .../providers/s3/apis/getProperties.test.ts | 1 + .../providers/s3/apis/getUrl.test.ts | 85 +++++++------------ 2 files changed, 34 insertions(+), 52 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 7033ad4fcf1..18428bd3ff8 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -127,6 +127,7 @@ describe('getProperties api', () => { ])( 'getProperties with all accessLevels', async (key, options, expected, config, headObjectOptions) => { + expect.assertions(3); expect( await getProperties({ key, diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 0d6e4911f8e..740866089e4 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -8,6 +8,7 @@ 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', () => ({ @@ -62,19 +63,10 @@ describe('getUrl test', () => { afterEach(() => { jest.clearAllMocks(); }); - - it('get presigned url with guest accessLevel', async () => { - expect.assertions(4); - const result = await getUrl({ - key: 'key', - options: { - accessLevel: 'guest', - validateObjectExistence: true, - }, - }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + it.each([ + [ + 'key', + { accessLevel: 'guest', validateObjectExistence: true }, { credentials, region: 'region', @@ -82,25 +74,15 @@ describe('getUrl test', () => { { Bucket: 'bucket', Key: 'public/key', - } - ); - expect(result.url).toEqual({ - url: new URL('https://google.com'), - }); - }); - it('get presigned url with protected accessLevel', async () => { - expect.assertions(4); - const result = await getUrl({ - key: 'key', - options: { + }, + ], + [ + 'key', + { accessLevel: 'protected', - targetIdentityId, + targetIdentityId: 'targetIdentityId', validateObjectExistence: true, }, - }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( { credentials, region: 'region', @@ -108,24 +90,11 @@ describe('getUrl test', () => { { Bucket: 'bucket', Key: 'protected/targetIdentityId/key', - } - ); - expect(result.url).toEqual({ - url: new URL('https://google.com'), - }); - }); - it('get presigned url with private accessLevel', async () => { - expect.assertions(4); - const result = await getUrl({ - key: 'key', - options: { - accessLevel: 'private', - validateObjectExistence: true, }, - }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith( + ], + [ + 'key', + { accessLevel: 'private', validateObjectExistence: true }, { credentials, region: 'region', @@ -133,12 +102,24 @@ describe('getUrl test', () => { { Bucket: 'bucket', Key: 'private/targetIdentityId/key', - } - ); - expect(result.url).toEqual({ - url: new URL('https://google.com'), - }); - }); + }, + ], + ])( + 'getUrl with accessLevels', + async (key, options, config, headObjectOptions) => { + expect.assertions(4); + const result = await getUrl({ + key, + options: options as StorageOptions, + }); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); + expect(result.url).toEqual({ + url: new URL('https://google.com'), + }); + } + ); }); describe('getUrl error path', () => { afterAll(() => { From 39d2c554b80ef77dd78799d34236b110374fe870 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 16:51:26 -0700 Subject: [PATCH 07/20] chore: remove after each block --- .../storage/__tests__/providers/s3/apis/getProperties.test.ts | 3 --- packages/storage/__tests__/providers/s3/apis/getUrl.test.ts | 3 --- 2 files changed, 6 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 18428bd3ff8..aacd7d1d382 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -42,9 +42,6 @@ describe('getProperties api', () => { }, }, }); - afterEach(() => { - jest.clearAllMocks(); - }); }); describe('getProperties happy path ', () => { beforeEach(() => { diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 740866089e4..d3b9e069053 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -60,9 +60,6 @@ describe('getUrl test', () => { url: new URL('https://google.com'), }); }); - afterEach(() => { - jest.clearAllMocks(); - }); it.each([ [ 'key', From d162e94820131bef6d5a3e6eb5408af6e6c6d883 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Fri, 1 Sep 2023 17:20:20 -0700 Subject: [PATCH 08/20] chore: more clone up --- .../providers/s3/apis/getProperties.test.ts | 111 +++++++----------- .../providers/s3/apis/getUrl.test.ts | 86 +++++++------- 2 files changed, 88 insertions(+), 109 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index aacd7d1d382..0e62bbb731f 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -27,7 +27,50 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; +const getPropertiesInput = { + key: 'key', + options: { accessLevel: 'guest' }, + expected: { + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'public/key', + }, +}; +const getPropertiesInputWithALlAccessLevels = [ + getPropertiesInput, + Object.assign(getPropertiesInput, { + options: { + accessLevel: 'protected', + targetIdentityId: 'targetIdentityId', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + }, + }), + Object.assign(getPropertiesInput, { + options: { + accessLevel: 'private', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'private/targetIdentityId/key', + }, + }), +]; describe('getProperties api', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ @@ -57,73 +100,9 @@ describe('getProperties api', () => { afterEach(() => { jest.clearAllMocks(); }); - it.each([ - [ - 'key', - { accessLevel: 'guest' }, - { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'public/key', - }, - ], - [ - 'key', - { accessLevel: 'protected', targetIdentityId: 'targetIdentityId' }, - { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, - ], - [ - 'key', - { accessLevel: 'private' }, - { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, - ], - ])( + it.each(getPropertiesInputWithALlAccessLevels)( 'getProperties with all accessLevels', - async (key, options, expected, config, headObjectOptions) => { + async ({ key, options, expected, config, headObjectOptions }) => { expect.assertions(3); expect( await getProperties({ diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index d3b9e069053..5bd6c344740 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -29,6 +29,44 @@ const credentials: Credentials = { }; const targetIdentityId = 'targetIdentityId'; +const getUrlInput = { + key: 'key', + options: { accessLevel: 'guest', validateObjectExistence: true }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'public/key', + }, +}; + +const getUrlInputWithAllAccessLevels = [ + getUrlInput, + Object.assign(getUrlInput, { + options: { + accessLevel: 'protected', + targetIdentityId: 'targetIdentityId', + validateObjectExistence: true, + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + }, + }), + Object.assign(getUrlInput, { + options: { + accessLevel: 'private', + validateObjectExistence: true, + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'private/targetIdentityId/key', + }, + }), +]; + describe('getUrl test', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ @@ -60,50 +98,12 @@ describe('getUrl test', () => { url: new URL('https://google.com'), }); }); - it.each([ - [ - 'key', - { accessLevel: 'guest', validateObjectExistence: true }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'public/key', - }, - ], - [ - 'key', - { - accessLevel: 'protected', - targetIdentityId: 'targetIdentityId', - validateObjectExistence: true, - }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, - ], - [ - 'key', - { accessLevel: 'private', validateObjectExistence: true }, - { - credentials, - region: 'region', - }, - { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, - ], - ])( + afterEach(() => { + jest.clearAllMocks(); + }); + it.each(getUrlInputWithAllAccessLevels)( 'getUrl with accessLevels', - async (key, options, config, headObjectOptions) => { + async ({ key, options, config, headObjectOptions }) => { expect.assertions(4); const result = await getUrl({ key, From 2af8d4c4126eb75442352518b9092b780bd90364 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Sun, 3 Sep 2023 12:33:35 -0700 Subject: [PATCH 09/20] chore: change input options to it.each --- .../providers/s3/apis/getProperties.test.ts | 111 +++++++++++------- .../providers/s3/apis/getUrl.test.ts | 87 +++++++------- 2 files changed, 113 insertions(+), 85 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 0e62bbb731f..eac056e2b57 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -27,50 +27,7 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; -const getPropertiesInput = { - key: 'key', - options: { accessLevel: 'guest' }, - expected: { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - config: { - credentials, - region: 'region', - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'public/key', - }, -}; -const getPropertiesInputWithALlAccessLevels = [ - getPropertiesInput, - Object.assign(getPropertiesInput, { - options: { - accessLevel: 'protected', - targetIdentityId: 'targetIdentityId', - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, - }), - Object.assign(getPropertiesInput, { - options: { - accessLevel: 'private', - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, - }), -]; describe('getProperties api', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ @@ -100,8 +57,72 @@ describe('getProperties api', () => { afterEach(() => { jest.clearAllMocks(); }); - it.each(getPropertiesInputWithALlAccessLevels)( - 'getProperties with all accessLevels', + it.each([ + { + key: 'key', + options: { accessLevel: 'guest' }, + expected: { + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'public/key', + }, + }, + { + key: 'key', + options: { accessLevel: 'protected', targetIdentityId }, + expected: { + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + }, + }, + { + key: 'key', + options: { accessLevel: 'private' }, + expected: { + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'private/targetIdentityId/key', + }, + }, + ])( + 'getProperties api with $options.accessLevel', async ({ key, options, expected, config, headObjectOptions }) => { expect.assertions(3); expect( diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 5bd6c344740..4817a42407e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -29,44 +29,6 @@ const credentials: Credentials = { }; const targetIdentityId = 'targetIdentityId'; -const getUrlInput = { - key: 'key', - options: { accessLevel: 'guest', validateObjectExistence: true }, - config: { - credentials, - region: 'region', - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'public/key', - }, -}; - -const getUrlInputWithAllAccessLevels = [ - getUrlInput, - Object.assign(getUrlInput, { - options: { - accessLevel: 'protected', - targetIdentityId: 'targetIdentityId', - validateObjectExistence: true, - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, - }), - Object.assign(getUrlInput, { - options: { - accessLevel: 'private', - validateObjectExistence: true, - }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, - }), -]; - describe('getUrl test', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ @@ -101,8 +63,53 @@ describe('getUrl test', () => { afterEach(() => { jest.clearAllMocks(); }); - it.each(getUrlInputWithAllAccessLevels)( - 'getUrl with accessLevels', + it.each([ + { + key: 'key', + options: { accessLevel: 'guest', validateObjectExistence: true }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'public/key', + }, + }, + { + key: 'key', + options: { + accessLevel: 'protected', + targetIdentityId, + validateObjectExistence: true, + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'protected/targetIdentityId/key', + }, + }, + { + key: 'key', + options: { + accessLevel: 'private', + targetIdentityId, + validateObjectExistence: true, + }, + config: { + credentials, + region: 'region', + }, + headObjectOptions: { + Bucket: 'bucket', + Key: 'private/targetIdentityId/key', + }, + }, + ])( + 'getUrl with $options.accessLevel', async ({ key, options, config, headObjectOptions }) => { expect.assertions(4); const result = await getUrl({ From bbed029cbcbb23c959f644e14b323a0d797409bc Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 10:58:39 -0700 Subject: [PATCH 10/20] chore: remove common part for getProperties --- .../providers/s3/apis/getProperties.test.ts | 58 +++++-------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index eac056e2b57..d5c42ebd05e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -44,6 +44,20 @@ describe('getProperties api', () => { }); }); describe('getProperties happy path ', () => { + const expected = { + key: 'key', + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }; + const config = { + credentials, + region: 'region', + }; + const key = 'key'; beforeEach(() => { mockHeadObject.mockReturnValueOnce({ ContentLength: '100', @@ -59,63 +73,21 @@ describe('getProperties api', () => { }); it.each([ { - key: 'key', options: { accessLevel: 'guest' }, - expected: { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - config: { - credentials, - region: 'region', - }, headObjectOptions: { Bucket: 'bucket', Key: 'public/key', }, }, { - key: 'key', options: { accessLevel: 'protected', targetIdentityId }, - expected: { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - config: { - credentials, - region: 'region', - }, headObjectOptions: { Bucket: 'bucket', Key: 'protected/targetIdentityId/key', }, }, { - key: 'key', options: { accessLevel: 'private' }, - expected: { - key: 'key', - size: '100', - contentType: 'text/plain', - eTag: 'etag', - metadata: { key: 'value' }, - lastModified: 'last-modified', - versionId: 'version-id', - }, - config: { - credentials, - region: 'region', - }, headObjectOptions: { Bucket: 'bucket', Key: 'private/targetIdentityId/key', @@ -123,7 +95,7 @@ describe('getProperties api', () => { }, ])( 'getProperties api with $options.accessLevel', - async ({ key, options, expected, config, headObjectOptions }) => { + async ({ options, headObjectOptions }) => { expect.assertions(3); expect( await getProperties({ From b4b8248064205338978dd4512b999a6a5dac0e81 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 11:00:06 -0700 Subject: [PATCH 11/20] chore: remove getUrl common part --- .../providers/s3/apis/getUrl.test.ts | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 4817a42407e..e139d88c61b 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -45,6 +45,11 @@ describe('getUrl test', () => { }); }); describe('getUrl happy path', () => { + const config = { + credentials, + region: 'region', + }; + const key = 'key'; beforeEach(() => { (headObject as jest.Mock).mockImplementation(() => { return { @@ -65,44 +70,30 @@ describe('getUrl test', () => { }); it.each([ { - key: 'key', options: { accessLevel: 'guest', validateObjectExistence: true }, - config: { - credentials, - region: 'region', - }, headObjectOptions: { Bucket: 'bucket', Key: 'public/key', }, }, { - key: 'key', options: { accessLevel: 'protected', targetIdentityId, validateObjectExistence: true, }, - config: { - credentials, - region: 'region', - }, + headObjectOptions: { Bucket: 'bucket', Key: 'protected/targetIdentityId/key', }, }, { - key: 'key', options: { accessLevel: 'private', targetIdentityId, validateObjectExistence: true, }, - config: { - credentials, - region: 'region', - }, headObjectOptions: { Bucket: 'bucket', Key: 'private/targetIdentityId/key', @@ -110,7 +101,7 @@ describe('getUrl test', () => { }, ])( 'getUrl with $options.accessLevel', - async ({ key, options, config, headObjectOptions }) => { + async ({ options, headObjectOptions }) => { expect.assertions(4); const result = await getUrl({ key, From 65a9838ed0fe57a2e3599a9d58fef1554fdcf211 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 11:21:02 -0700 Subject: [PATCH 12/20] chore: address feedback --- .../providers/s3/apis/getProperties.test.ts | 21 +++----- .../providers/s3/apis/getUrl.test.ts | 49 ++++++++----------- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index d5c42ebd05e..59ed879b5d6 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -74,28 +74,23 @@ describe('getProperties api', () => { it.each([ { options: { accessLevel: 'guest' }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'public/key', - }, + expectedKey: 'public/key', }, { options: { accessLevel: 'protected', targetIdentityId }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, + expectedKey: 'protected/targetIdentityId/key', }, { options: { accessLevel: 'private' }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, + expectedKey: 'private/targetIdentityId/key', }, ])( 'getProperties api with $options.accessLevel', - async ({ options, headObjectOptions }) => { + async ({ options, expectedKey }) => { + const headObjectOptions = { + Bucket: 'bucket', + Key: expectedKey, + }; expect.assertions(3); expect( await getProperties({ diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index e139d88c61b..e59fb0ba479 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -71,10 +71,7 @@ describe('getUrl test', () => { it.each([ { options: { accessLevel: 'guest', validateObjectExistence: true }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'public/key', - }, + expectedKey: 'public/key', }, { options: { @@ -82,11 +79,7 @@ describe('getUrl test', () => { targetIdentityId, validateObjectExistence: true, }, - - headObjectOptions: { - Bucket: 'bucket', - Key: 'protected/targetIdentityId/key', - }, + expectedKey: 'protected/targetIdentityId/key', }, { options: { @@ -94,27 +87,25 @@ describe('getUrl test', () => { targetIdentityId, validateObjectExistence: true, }, - headObjectOptions: { - Bucket: 'bucket', - Key: 'private/targetIdentityId/key', - }, + expectedKey: 'private/targetIdentityId/key', }, - ])( - 'getUrl with $options.accessLevel', - async ({ options, headObjectOptions }) => { - expect.assertions(4); - const result = await getUrl({ - key, - options: options as StorageOptions, - }); - expect(getPresignedGetObjectUrl).toBeCalledTimes(1); - expect(headObject).toBeCalledTimes(1); - expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); - expect(result.url).toEqual({ - url: new URL('https://google.com'), - }); - } - ); + ])('getUrl with $options.accessLevel', async ({ options, expectedKey }) => { + const headObjectOptions = { + Bucket: 'bucket', + Key: expectedKey, + }; + expect.assertions(4); + const result = await getUrl({ + key, + options: options as StorageOptions, + }); + expect(getPresignedGetObjectUrl).toBeCalledTimes(1); + expect(headObject).toBeCalledTimes(1); + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); + expect(result.url).toEqual({ + url: new URL('https://google.com'), + }); + }); }); describe('getUrl error path', () => { afterAll(() => { From 9a24d322acf32cba157f5959252a37ccb7015715 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 12:49:24 -0700 Subject: [PATCH 13/20] chore: take out common function for test --- .../apis/__utils__/buildClientRequestKey.ts | 22 +++++++++++++++++++ .../__tests__/providers/s3/apis/copy.test.ts | 20 +---------------- 2 files changed, 23 insertions(+), 19 deletions(-) create mode 100644 packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts diff --git a/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts b/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts new file mode 100644 index 00000000000..6fb05b3102a --- /dev/null +++ b/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts @@ -0,0 +1,22 @@ +import { StorageAccessLevel } from '@aws-amplify/core'; + +/** + * bucket is appended at start if it's a sourceKey + * guest: public/${key}` + * private: private/${targetIdentityId}/${key}` + * protected: protected/${targetIdentityId}/${key}` + */ +export const buildClientRequestKey = ( + key: string, + KeyType: 'source' | 'destination', + accessLevel: StorageAccessLevel +) => { + const targetIdentityId = 'targetIdentityId'; + const bucket = 'bucket'; + const finalAccessLevel = accessLevel == 'guest' ? 'public' : accessLevel; + let finalKey = KeyType == 'source' ? `${bucket}/` : ''; + finalKey += `${finalAccessLevel}/`; + finalKey += finalAccessLevel != 'public' ? `${targetIdentityId}/` : ''; + finalKey += `${key}`; + return finalKey; +}; diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index 29c9222b13c..8f2ea177b5b 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -9,6 +9,7 @@ import { } from '@aws-amplify/core'; import { copyObject } from '../../../../src/providers/s3/utils/client'; import { copy } from '../../../../src/providers/s3/apis'; +import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -41,25 +42,6 @@ const copyObjectClientBaseParams = { MetadataDirective: 'COPY', }; -/** - * bucket is appended at start if it's a sourceKey - * guest: public/${targetIdentityId}/${key}` - * private: private/${targetIdentityId}/${key}` - * protected: protected/${targetIdentityId}/${key}` - */ -const buildClientRequestKey = ( - key: string, - KeyType: 'source' | 'destination', - accessLevel: StorageAccessLevel -) => { - const finalAccessLevel = accessLevel == 'guest' ? 'public' : accessLevel; - let finalKey = KeyType == 'source' ? `${bucket}/` : ''; - finalKey += `${finalAccessLevel}/`; - finalKey += finalAccessLevel != 'public' ? `${targetIdentityId}/` : ''; - finalKey += `${key}`; - return finalKey; -}; - const interAccessLevelTest = async ( sourceAccessLevel, destinationAccessLevel From c3b3fc73a031be00b7c4db242156076e83b150eb Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 12:49:43 -0700 Subject: [PATCH 14/20] ignore __utils__while testing --- packages/storage/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/storage/package.json b/packages/storage/package.json index 7c19cc94f88..a284802e43c 100644 --- a/packages/storage/package.json +++ b/packages/storage/package.json @@ -121,7 +121,8 @@ "testPathIgnorePatterns": [ "xmlParser-fixture.ts", "testUtils", - "cases" + "cases", + "__utils__" ], "modulePathIgnorePatterns": [ "dist", From b7ccdbc419c6ea370a63e5d2bdbffa915ebf88fe Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 12:53:22 -0700 Subject: [PATCH 15/20] chore: update tests with common method --- .../providers/s3/apis/getProperties.test.ts | 33 ++++++++++---- .../providers/s3/apis/getUrl.test.ts | 43 +++++++++++-------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 59ed879b5d6..1d92edb880e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -4,8 +4,13 @@ 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, + StorageAccessLevel, + fetchAuthSession, +} from '@aws-amplify/core'; import { StorageOptions } from '../../../../src/types'; +import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); const mockHeadObject = headObject as jest.Mock; @@ -73,24 +78,34 @@ describe('getProperties api', () => { }); it.each([ { - options: { accessLevel: 'guest' }, - expectedKey: 'public/key', + accessLevel: 'guest', }, { - options: { accessLevel: 'protected', targetIdentityId }, - expectedKey: 'protected/targetIdentityId/key', + accessLevel: 'protected', }, { - options: { accessLevel: 'private' }, - expectedKey: 'private/targetIdentityId/key', + accessLevel: 'private', }, ])( 'getProperties api with $options.accessLevel', - async ({ options, expectedKey }) => { + async ({ accessLevel }) => { const headObjectOptions = { Bucket: 'bucket', - Key: expectedKey, + Key: buildClientRequestKey( + key, + 'destination', + accessLevel as StorageAccessLevel + ), }; + const options = + accessLevel === 'protected' + ? { + accessLevel, + targetIdentityId, + } + : { + accessLevel, + }; expect.assertions(3); expect( await getProperties({ diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index e59fb0ba479..775155c336c 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -3,12 +3,17 @@ import { getUrl } from '../../../../src/providers/s3/apis'; import { Credentials } from '@aws-sdk/types'; -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; +import { + Amplify, + StorageAccessLevel, + fetchAuthSession, +} from '@aws-amplify/core'; import { getPresignedGetObjectUrl, headObject, } from '../../../../src/providers/s3/utils/client'; import { StorageOptions } from '../../../../src/types'; +import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -70,30 +75,34 @@ describe('getUrl test', () => { }); it.each([ { - options: { accessLevel: 'guest', validateObjectExistence: true }, - expectedKey: 'public/key', + accessLevel: 'guest', }, { - options: { - accessLevel: 'protected', - targetIdentityId, - validateObjectExistence: true, - }, - expectedKey: 'protected/targetIdentityId/key', + accessLevel: 'protected', }, { - options: { - accessLevel: 'private', - targetIdentityId, - validateObjectExistence: true, - }, - expectedKey: 'private/targetIdentityId/key', + accessLevel: 'private', }, - ])('getUrl with $options.accessLevel', async ({ options, expectedKey }) => { + ])('getUrl with $options.accessLevel', async ({ accessLevel }) => { const headObjectOptions = { Bucket: 'bucket', - Key: expectedKey, + Key: buildClientRequestKey( + key, + 'destination', + accessLevel as StorageAccessLevel + ), }; + const options = + accessLevel === 'protected' + ? { + accessLevel, + targetIdentityId, + validateObjectExistence: true, + } + : { + accessLevel, + validateObjectExistence: true, + }; expect.assertions(4); const result = await getUrl({ key, From eb0120c1b10147137d294967547b6131bb62f6e5 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 13:10:17 -0700 Subject: [PATCH 16/20] chore: remove fetchAuthSession old mocks --- packages/storage/__tests__/providers/s3/apis/copy.test.ts | 1 - packages/storage/__tests__/providers/s3/apis/list.test.ts | 1 - packages/storage/__tests__/providers/s3/apis/remove.test.ts | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index a1f6ccbc8c8..129289cb07c 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -9,7 +9,6 @@ import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; 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/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: { From 051abc93a1d5bce529621c9e215ea5ae8f974797 Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 13:48:43 -0700 Subject: [PATCH 17/20] chore: change input array --- .../providers/s3/apis/getProperties.test.ts | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 48c82efc1aa..a95a8c97d32 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -4,12 +4,8 @@ import { headObject } from '../../../../src/providers/s3/utils/client'; import { getProperties } from '../../../../src/providers/s3'; import { Credentials } from '@aws-sdk/types'; -import { - Amplify, - StorageAccessLevel, -} from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { StorageOptions } from '../../../../src/types'; -import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -78,34 +74,24 @@ describe('getProperties api', () => { }); it.each([ { - accessLevel: 'guest', + options: { accessLevel: 'guest' }, + expectedKey: 'public/key', }, { - accessLevel: 'protected', + options: { accessLevel: 'protected', targetIdentityId }, + expectedKey: 'protected/targetIdentityId/key', }, { - accessLevel: 'private', + options: { accessLevel: 'private' }, + expectedKey: 'private/targetIdentityId/key', }, ])( 'getProperties api with $options.accessLevel', - async ({ accessLevel }) => { + async ({ options, expectedKey }) => { const headObjectOptions = { Bucket: 'bucket', - Key: buildClientRequestKey( - key, - 'destination', - accessLevel as StorageAccessLevel - ), + Key: expectedKey, }; - const options = - accessLevel === 'protected' - ? { - accessLevel, - targetIdentityId, - } - : { - accessLevel, - }; expect.assertions(3); expect( await getProperties({ From 20c9d7118644198a4a5c5c17f028fddd85d2f7dd Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 13:51:00 -0700 Subject: [PATCH 18/20] chore: update getUrl tests --- .../providers/s3/apis/getUrl.test.ts | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 1cf0bd406e1..74278de98ea 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -3,16 +3,12 @@ import { getUrl } from '../../../../src/providers/s3/apis'; import { Credentials } from '@aws-sdk/types'; -import { - Amplify, - StorageAccessLevel, -} from '@aws-amplify/core'; +import { Amplify } from '@aws-amplify/core'; import { getPresignedGetObjectUrl, headObject, } from '../../../../src/providers/s3/utils/client'; import { StorageOptions } from '../../../../src/types'; -import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -76,38 +72,28 @@ describe('getUrl test', () => { }); it.each([ { - accessLevel: 'guest', + options: { accessLevel: 'guest' }, + expectedKey: 'public/key', }, { - accessLevel: 'protected', + options: { accessLevel: 'protected', targetIdentityId }, + expectedKey: 'protected/targetIdentityId/key', }, { - accessLevel: 'private', + options: { accessLevel: 'private' }, + expectedKey: 'private/targetIdentityId/key', }, - ])('getUrl with $options.accessLevel', async ({ accessLevel }) => { + ])('getUrl with $options.accessLevel', async ({ options, expectedKey }) => { const headObjectOptions = { Bucket: 'bucket', - Key: buildClientRequestKey( - key, - 'destination', - accessLevel as StorageAccessLevel - ), + Key: expectedKey, }; - const options = - accessLevel === 'protected' - ? { - accessLevel, - targetIdentityId, - validateObjectExistence: true, - } - : { - accessLevel, - validateObjectExistence: true, - }; + const optionsVal = { ...options, validateObjectExistence: true }; + expect.assertions(4); const result = await getUrl({ key, - options: options as StorageOptions, + options: optionsVal as StorageOptions, }); expect(getPresignedGetObjectUrl).toBeCalledTimes(1); expect(headObject).toBeCalledTimes(1); From 293e30e8e65ad0ac4281ed7ed385a17f51f2d6cb Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 13:57:42 -0700 Subject: [PATCH 19/20] chore: add private without targetIdentityId --- .../__tests__/providers/s3/apis/getProperties.test.ts | 9 +++++++-- .../storage/__tests__/providers/s3/apis/getUrl.test.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index a95a8c97d32..0485b8c5869 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -28,12 +28,13 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; +const identityId = 'identityId'; describe('getProperties api', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ credentials, - identityId: targetIdentityId, + identityId, }); mockGetConfig.mockReturnValue({ Storage: { @@ -81,9 +82,13 @@ describe('getProperties api', () => { options: { accessLevel: 'protected', targetIdentityId }, expectedKey: 'protected/targetIdentityId/key', }, + { + options: { accessLevel: 'protected' }, + expectedKey: 'protected/identityId/key', + }, { options: { accessLevel: 'private' }, - expectedKey: 'private/targetIdentityId/key', + expectedKey: 'private/identityId/key', }, ])( 'getProperties api with $options.accessLevel', diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 74278de98ea..4cd7d7586bf 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -30,12 +30,13 @@ const credentials: Credentials = { secretAccessKey: 'secretAccessKey', }; const targetIdentityId = 'targetIdentityId'; +const identityId = 'identityId'; describe('getUrl test', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ credentials, - identityId: targetIdentityId, + identityId, }); mockGetConfig.mockReturnValue({ Storage: { @@ -79,9 +80,13 @@ describe('getUrl test', () => { options: { accessLevel: 'protected', targetIdentityId }, expectedKey: 'protected/targetIdentityId/key', }, + { + options: { accessLevel: 'protected' }, + expectedKey: 'protected/identityId/key', + }, { options: { accessLevel: 'private' }, - expectedKey: 'private/targetIdentityId/key', + expectedKey: 'private/identityId/key', }, ])('getUrl with $options.accessLevel', async ({ options, expectedKey }) => { const headObjectOptions = { From 0616ab056df8b8dda1f800890a57a1d26926d86e Mon Sep 17 00:00:00 2001 From: Venkata Ramyasri Kota Date: Tue, 5 Sep 2023 14:16:46 -0700 Subject: [PATCH 20/20] chore: remove util function --- .../apis/__utils__/buildClientRequestKey.ts | 22 ------------------- .../__tests__/providers/s3/apis/copy.test.ts | 22 ++++++++++++++++++- packages/storage/package.json | 3 +-- 3 files changed, 22 insertions(+), 25 deletions(-) delete mode 100644 packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts diff --git a/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts b/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts deleted file mode 100644 index 6fb05b3102a..00000000000 --- a/packages/storage/__tests__/providers/s3/apis/__utils__/buildClientRequestKey.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { StorageAccessLevel } from '@aws-amplify/core'; - -/** - * bucket is appended at start if it's a sourceKey - * guest: public/${key}` - * private: private/${targetIdentityId}/${key}` - * protected: protected/${targetIdentityId}/${key}` - */ -export const buildClientRequestKey = ( - key: string, - KeyType: 'source' | 'destination', - accessLevel: StorageAccessLevel -) => { - const targetIdentityId = 'targetIdentityId'; - const bucket = 'bucket'; - const finalAccessLevel = accessLevel == 'guest' ? 'public' : accessLevel; - let finalKey = KeyType == 'source' ? `${bucket}/` : ''; - finalKey += `${finalAccessLevel}/`; - finalKey += finalAccessLevel != 'public' ? `${targetIdentityId}/` : ''; - finalKey += `${key}`; - return finalKey; -}; diff --git a/packages/storage/__tests__/providers/s3/apis/copy.test.ts b/packages/storage/__tests__/providers/s3/apis/copy.test.ts index 129289cb07c..83d8dbd2176 100644 --- a/packages/storage/__tests__/providers/s3/apis/copy.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/copy.test.ts @@ -5,7 +5,6 @@ import { Credentials } from '@aws-sdk/types'; import { Amplify, StorageAccessLevel } from '@aws-amplify/core'; import { copyObject } from '../../../../src/providers/s3/utils/client'; import { copy } from '../../../../src/providers/s3/apis'; -import { buildClientRequestKey } from './__utils__/buildClientRequestKey'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -40,6 +39,27 @@ const copyObjectClientBaseParams = { MetadataDirective: 'COPY', }; +/** + * bucket is appended at start if it's a sourceKey + * guest: public/${key}` + * private: private/${targetIdentityId}/${key}` + * protected: protected/${targetIdentityId}/${key}` + */ +const buildClientRequestKey = ( + key: string, + KeyType: 'source' | 'destination', + accessLevel: StorageAccessLevel +) => { + const targetIdentityId = 'targetIdentityId'; + const bucket = 'bucket'; + const finalAccessLevel = accessLevel == 'guest' ? 'public' : accessLevel; + let finalKey = KeyType == 'source' ? `${bucket}/` : ''; + finalKey += `${finalAccessLevel}/`; + finalKey += finalAccessLevel != 'public' ? `${targetIdentityId}/` : ''; + finalKey += `${key}`; + return finalKey; +}; + const interAccessLevelTest = async ( sourceAccessLevel, destinationAccessLevel diff --git a/packages/storage/package.json b/packages/storage/package.json index a284802e43c..7c19cc94f88 100644 --- a/packages/storage/package.json +++ b/packages/storage/package.json @@ -121,8 +121,7 @@ "testPathIgnorePatterns": [ "xmlParser-fixture.ts", "testUtils", - "cases", - "__utils__" + "cases" ], "modulePathIgnorePatterns": [ "dist",