From 3392532ddccf27d21c3a36af4306f0c02dc60639 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 15 Mar 2024 11:03:49 -0700 Subject: [PATCH 01/27] feat: add gen2 path to getProperties API --- .../src/providers/s3/apis/getProperties.ts | 45 +++++++++++++++++-- .../s3/apis/internal/getProperties.ts | 28 ++++++++---- .../providers/s3/apis/server/getProperties.ts | 28 ++++++++++-- .../storage/src/providers/s3/types/index.ts | 7 ++- .../storage/src/providers/s3/types/inputs.ts | 17 +++++-- .../storage/src/providers/s3/types/options.ts | 4 +- .../storage/src/providers/s3/types/outputs.ts | 7 ++- packages/storage/src/types/index.ts | 3 +- packages/storage/src/types/inputs.ts | 8 +++- 9 files changed, 120 insertions(+), 27 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index 4b98d529f55..710597d7c3c 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -3,7 +3,15 @@ import { Amplify } from '@aws-amplify/core'; -import { GetPropertiesInput, GetPropertiesOutput, S3Exception } from '../types'; +import { + GetPropertiesInput, + GetPropertiesInputKey, + GetPropertiesInputPath, + GetPropertiesOutput, + GetPropertiesOutputKey, + GetPropertiesOutputPath, + S3Exception, +} from '../types'; import { StorageValidationErrorCode } from '../../../errors/types/validation'; import { getProperties as getPropertiesInternal } from './internal/getProperties'; @@ -17,8 +25,37 @@ import { getProperties as getPropertiesInternal } from './internal/getProperties * @throws A {@link S3Exception} when the underlying S3 service returned error. * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. */ -export const getProperties = ( + +interface GetProperties { + /** + * Gets the properties of a file. The properties include S3 system metadata and + * the user metadata that was provided when uploading the file. + * + * @param input - The GetPropertiesInput object. + * @returns Requested object properties. + * @throws An {@link S3Exception} when the underlying S3 service returned error. + * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. + */ + (input: GetPropertiesInputPath): Promise; + /** + * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. + * Please use {@link https://docs.amplify.aws/react/build-a-backend/storage/download/#downloaddata | path} instead. + * + * Gets the properties of a file. The properties include S3 system metadata and + * the user metadata that was provided when uploading the file. + * + * @param input - The GetPropertiesInput object. + * @returns Requested object properties. + * @throws An {@link S3Exception} when the underlying S3 service returned error. + * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. + */ + (input: GetPropertiesInputKey): Promise; +} + +export const getProperties: GetProperties = < + Output extends GetPropertiesOutput, +>( input: GetPropertiesInput, -): Promise => { - return getPropertiesInternal(Amplify, input); +): Promise => { + return getPropertiesInternal(Amplify, input) as Promise; }; diff --git a/packages/storage/src/providers/s3/apis/internal/getProperties.ts b/packages/storage/src/providers/s3/apis/internal/getProperties.ts index db854f8635b..b934497da6c 100644 --- a/packages/storage/src/providers/s3/apis/internal/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/internal/getProperties.ts @@ -5,24 +5,31 @@ import { AmplifyClassV6 } from '@aws-amplify/core'; import { StorageAction } from '@aws-amplify/core/internals/utils'; import { GetPropertiesInput, GetPropertiesOutput } from '../../types'; -import { resolveS3ConfigAndInput } from '../../utils'; +import { + resolveS3ConfigAndInput, + validateStorageOperationInput, +} from '../../utils'; import { headObject } from '../../utils/client'; import { getStorageUserAgentValue } from '../../utils/userAgent'; import { logger } from '../../../../utils'; +import { STORAGE_INPUT_KEY } from '../../utils/constants'; export const getProperties = async ( amplify: AmplifyClassV6, input: GetPropertiesInput, action?: StorageAction, ): Promise => { - const { key, options } = input; - const { s3Config, bucket, keyPrefix } = await resolveS3ConfigAndInput( - amplify, - options, + const { options: getPropertiesOptions } = input; + const { s3Config, bucket, keyPrefix, identityId } = + await resolveS3ConfigAndInput(amplify, getPropertiesOptions); + const { inputType, objectKey } = validateStorageOperationInput( + input, + identityId, ); - const finalKey = `${keyPrefix}${key}`; + const finalKey = + inputType === STORAGE_INPUT_KEY ? keyPrefix + objectKey : objectKey; - logger.debug(`get properties of ${key} from ${finalKey}`); + logger.debug(`get properties of ${objectKey} from ${finalKey}`); const response = await headObject( { ...s3Config, @@ -36,8 +43,7 @@ export const getProperties = async ( }, ); - return { - key, + const result = { contentType: response.ContentType, size: response.ContentLength, eTag: response.ETag, @@ -45,4 +51,8 @@ export const getProperties = async ( metadata: response.Metadata, versionId: response.VersionId, }; + + return inputType === STORAGE_INPUT_KEY + ? { key: objectKey, ...result } + : { path: finalKey, ...result }; }; diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index d56ee3d77f4..4b61bfa83a9 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -6,15 +6,35 @@ import { getAmplifyServerContext, } from '@aws-amplify/core/internals/adapter-core'; -import { GetPropertiesInput, GetPropertiesOutput } from '../../types'; +import { + GetPropertiesInput, + GetPropertiesInputKey, + GetPropertiesInputPath, + GetPropertiesOutput, + GetPropertiesOutputKey, + GetPropertiesOutputPath, +} from '../../types'; import { getProperties as getPropertiesInternal } from '../internal/getProperties'; -export const getProperties = ( +interface GetProperties { + ( + contextSpec: AmplifyServer.ContextSpec, + input: GetPropertiesInputPath, + ): Promise; + ( + contextSpec: AmplifyServer.ContextSpec, + input: GetPropertiesInputKey, + ): Promise; +} + +export const getProperties: GetProperties = < + Output extends GetPropertiesOutput, +>( contextSpec: AmplifyServer.ContextSpec, input: GetPropertiesInput, -): Promise => { +): Promise => { return getPropertiesInternal( getAmplifyServerContext(contextSpec).amplify, input, - ); + ) as Promise; }; diff --git a/packages/storage/src/providers/s3/types/index.ts b/packages/storage/src/providers/s3/types/index.ts index 773dde39361..ada7e0fd40f 100644 --- a/packages/storage/src/providers/s3/types/index.ts +++ b/packages/storage/src/providers/s3/types/index.ts @@ -4,7 +4,8 @@ export { GetUrlOptions, UploadDataOptions, - GetPropertiesOptions, + GetPropertiesOptionsKey, + GetPropertiesOptionsPath, ListAllOptions, ListPaginateOptions, RemoveOptions, @@ -23,12 +24,16 @@ export { ListAllOutput, ListPaginateOutput, GetPropertiesOutput, + GetPropertiesOutputKey, + GetPropertiesOutputPath, CopyOutput, RemoveOutput, } from './outputs'; export { CopyInput, GetPropertiesInput, + GetPropertiesInputKey, + GetPropertiesInputPath, GetUrlInput, ListAllInput, ListPaginateInput, diff --git a/packages/storage/src/providers/s3/types/inputs.ts b/packages/storage/src/providers/s3/types/inputs.ts index 3ec41b7d47a..53f86725e76 100644 --- a/packages/storage/src/providers/s3/types/inputs.ts +++ b/packages/storage/src/providers/s3/types/inputs.ts @@ -7,7 +7,8 @@ import { StorageCopyInput, StorageDownloadDataInputKey, StorageDownloadDataInputPath, - StorageGetPropertiesInput, + StorageGetPropertiesInputKey, + StorageGetPropertiesInputPath, StorageGetUrlInput, StorageListInput, StorageRemoveInput, @@ -18,7 +19,8 @@ import { CopySourceOptions, DownloadDataOptionsKey, DownloadDataOptionsPath, - GetPropertiesOptions, + GetPropertiesOptionsKey, + GetPropertiesOptionsPath, GetUrlOptions, ListAllOptions, ListPaginateOptions, @@ -38,8 +40,15 @@ export type CopyInput = StorageCopyInput< /** * Input type for S3 getProperties API. */ -export type GetPropertiesInput = - StorageGetPropertiesInput; +export type GetPropertiesInput = StrictUnion< + GetPropertiesInputKey | GetPropertiesInputPath +>; + +/** @deprecated Use {@link GetPropertiesInputPath} instead. */ +export type GetPropertiesInputKey = + StorageGetPropertiesInputKey; +export type GetPropertiesInputPath = + StorageGetPropertiesInputPath; /** * Input type for S3 getUrl API. diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 6356ca81a93..d89964842f6 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -49,7 +49,9 @@ interface TransferOptions { /** * Input options type for S3 getProperties API. */ -export type GetPropertiesOptions = ReadOptions & CommonOptions; +/** @deprecated Use {@link GetPropertiesOptionsPath} instead. */ +export type GetPropertiesOptionsKey = ReadOptions & CommonOptions; +export type GetPropertiesOptionsPath = CommonOptions; /** * Input options type for S3 getProperties API. diff --git a/packages/storage/src/providers/s3/types/outputs.ts b/packages/storage/src/providers/s3/types/outputs.ts index 35b6784af93..1ebc5817b22 100644 --- a/packages/storage/src/providers/s3/types/outputs.ts +++ b/packages/storage/src/providers/s3/types/outputs.ts @@ -59,10 +59,15 @@ export type GetUrlOutput = StorageGetUrlOutput; */ export type UploadDataOutput = UploadTask; +export type GetPropertiesOutputKey = ItemKey; +export type GetPropertiesOutputPath = ItemPath; + /** * Output type for S3 getProperties API. */ -export type GetPropertiesOutput = ItemKey; +export type GetPropertiesOutput = + | GetPropertiesOutputKey + | GetPropertiesOutputPath; /** * Output type for S3 list API. Lists all bucket objects. diff --git a/packages/storage/src/types/index.ts b/packages/storage/src/types/index.ts index 78569a76f36..02e903d4a7a 100644 --- a/packages/storage/src/types/index.ts +++ b/packages/storage/src/types/index.ts @@ -10,7 +10,8 @@ export { export { StorageOperationInput, StorageListInput, - StorageGetPropertiesInput, + StorageGetPropertiesInputKey, + StorageGetPropertiesInputPath, StorageRemoveInput, StorageDownloadDataInputKey, StorageDownloadDataInputPath, diff --git a/packages/storage/src/types/inputs.ts b/packages/storage/src/types/inputs.ts index 0514feaba30..5a9998546ac 100644 --- a/packages/storage/src/types/inputs.ts +++ b/packages/storage/src/types/inputs.ts @@ -40,8 +40,12 @@ export interface StorageOperationInput { options?: Options; } -export type StorageGetPropertiesInput = - StorageOperationInput; +/** @deprecated Use {@link StorageGetPropertiesInputPath} instead. */ +export type StorageGetPropertiesInputKey = + StorageOperationInputKey & StorageOperationInput; + +export type StorageGetPropertiesInputPath = StorageOperationInputPath & + StorageOperationOptionsInput; export interface StorageRemoveInput { key: string; From 943fde64197481cf30a168fe5be14967c653f087 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 15 Mar 2024 16:08:02 -0700 Subject: [PATCH 02/27] test: add unit tests for path parameter --- .../providers/s3/apis/getProperties.test.ts | 127 ++++++++++++++++-- 1 file changed, 119 insertions(+), 8 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 6ee31b031ed..6617a05fdb7 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -5,7 +5,10 @@ import { headObject } from '../../../../src/providers/s3/utils/client'; import { getProperties } from '../../../../src/providers/s3'; import { AWSCredentials } from '@aws-amplify/core/internals/utils'; import { Amplify } from '@aws-amplify/core'; -import { GetPropertiesOptions } from '../../../../src/providers/s3/types'; +import { + GetPropertiesOptionsKey, + GetPropertiesOptionsPath, +} from '../../../../src/providers/s3/types'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -30,10 +33,12 @@ const credentials: AWSCredentials = { sessionToken: 'sessionToken', secretAccessKey: 'secretAccessKey', }; +const key = 'key'; +const path = 'path'; const targetIdentityId = 'targetIdentityId'; const defaultIdentityId = 'defaultIdentityId'; -describe('getProperties api', () => { +describe('getProperties with key', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ credentials, @@ -48,9 +53,9 @@ describe('getProperties api', () => { }, }); }); - describe('getProperties happy path ', () => { + describe('getProperties happy path with key', () => { const expected = { - key: 'key', + key, size: '100', contentType: 'text/plain', eTag: 'etag', @@ -63,7 +68,6 @@ describe('getProperties api', () => { region: 'region', userAgentValue: expect.any(String), }; - const key = 'key'; beforeEach(() => { mockHeadObject.mockReturnValueOnce({ ContentLength: '100', @@ -111,7 +115,7 @@ describe('getProperties api', () => { expect( await getProperties({ key, - options: options as GetPropertiesOptions, + options: options as GetPropertiesOptionsKey, }), ).toEqual(expected); expect(headObject).toHaveBeenCalledTimes(1); @@ -133,7 +137,7 @@ describe('getProperties api', () => { ); expect.assertions(3); try { - await getProperties({ key: 'keyed' }); + await getProperties({ key }); } catch (error: any) { expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith( @@ -144,7 +148,114 @@ describe('getProperties api', () => { }, { Bucket: 'bucket', - Key: 'public/keyed', + Key: `public/${key}`, + }, + ); + expect(error.$metadata.httpStatusCode).toBe(404); + } + }); + }); +}); + +describe('getProperties with path', () => { + beforeAll(() => { + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId: defaultIdentityId, + }); + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket, + region, + }, + }, + }); + }); + describe('getProperties with path', () => { + const expected = { + path, + size: '100', + contentType: 'text/plain', + eTag: 'etag', + metadata: { key: 'value' }, + lastModified: 'last-modified', + versionId: 'version-id', + }; + const config = { + credentials, + region: 'region', + useAccelerateEndpoint: true, + userAgentValue: expect.any(String), + }; + beforeEach(() => { + mockHeadObject.mockReturnValueOnce({ + ContentLength: '100', + ContentType: 'text/plain', + ETag: 'etag', + LastModified: 'last-modified', + Metadata: { key: 'value' }, + VersionId: 'version-id', + }); + }); + afterEach(() => { + jest.clearAllMocks(); + }); + [ + { + path: 'path', + expectedKey: 'path', + }, + { + path: () => 'path', + expectedKey: 'path', + }, + ].forEach(({ path, expectedKey }) => { + it(`should getProperties with path: ${path} and expectedKey ${expectedKey}`, async () => { + const headObjectOptions = { + Bucket: 'bucket', + Key: expectedKey, + }; + expect.assertions(3); + expect( + await getProperties({ + path, + options: { + useAccelerateEndpoint: true, + } as GetPropertiesOptionsPath, + }), + ).toEqual(expected); + expect(headObject).toHaveBeenCalledTimes(1); + expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); + }); + }); + }); + + describe('getProperties error path with 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.assertions(3); + try { + await getProperties({ path }); + } catch (error: any) { + expect(headObject).toHaveBeenCalledTimes(1); + expect(headObject).toHaveBeenCalledWith( + { + credentials, + region: 'region', + userAgentValue: expect.any(String), + }, + { + Bucket: 'bucket', + Key: path, }, ); expect(error.$metadata.httpStatusCode).toBe(404); From 265a494e5f0a3511aff5f8cb0b0e3038189218b7 Mon Sep 17 00:00:00 2001 From: erinleigh90 <106691284+erinleigh90@users.noreply.github.com> Date: Fri, 15 Mar 2024 16:45:33 -0700 Subject: [PATCH 03/27] nit: remove unnecessary colon --- .../storage/__tests__/providers/s3/apis/getProperties.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 6617a05fdb7..3208711edcf 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -211,7 +211,7 @@ describe('getProperties with path', () => { expectedKey: 'path', }, ].forEach(({ path, expectedKey }) => { - it(`should getProperties with path: ${path} and expectedKey ${expectedKey}`, async () => { + it(`should getProperties with path ${path} and expectedKey ${expectedKey}`, async () => { const headObjectOptions = { Bucket: 'bucket', Key: expectedKey, From 78550ea5b6b2fd0f31c8da6fef8171df6531a4ce Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 18 Mar 2024 14:15:33 -0700 Subject: [PATCH 04/27] feat: add path to types --- packages/storage/src/providers/s3/types/index.ts | 3 ++- .../storage/src/providers/s3/types/inputs.ts | 16 +++++++++++++--- .../storage/src/providers/s3/types/options.ts | 9 +++++++-- packages/storage/src/types/index.ts | 3 ++- packages/storage/src/types/inputs.ts | 8 ++++++-- 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/storage/src/providers/s3/types/index.ts b/packages/storage/src/providers/s3/types/index.ts index 773dde39361..93671d91edb 100644 --- a/packages/storage/src/providers/s3/types/index.ts +++ b/packages/storage/src/providers/s3/types/index.ts @@ -2,7 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 export { - GetUrlOptions, + GetUrlOptionsKey, + GetUrlOptionsPath, UploadDataOptions, GetPropertiesOptions, ListAllOptions, diff --git a/packages/storage/src/providers/s3/types/inputs.ts b/packages/storage/src/providers/s3/types/inputs.ts index 3ec41b7d47a..4ed734600bd 100644 --- a/packages/storage/src/providers/s3/types/inputs.ts +++ b/packages/storage/src/providers/s3/types/inputs.ts @@ -8,7 +8,8 @@ import { StorageDownloadDataInputKey, StorageDownloadDataInputPath, StorageGetPropertiesInput, - StorageGetUrlInput, + StorageGetUrlInputKey, + StorageGetUrlInputPath, StorageListInput, StorageRemoveInput, StorageUploadDataInput, @@ -19,7 +20,8 @@ import { DownloadDataOptionsKey, DownloadDataOptionsPath, GetPropertiesOptions, - GetUrlOptions, + GetUrlOptionsKey, + GetUrlOptionsPath, ListAllOptions, ListPaginateOptions, RemoveOptions, @@ -44,7 +46,15 @@ export type GetPropertiesInput = /** * Input type for S3 getUrl API. */ -export type GetUrlInput = StorageGetUrlInput; +export type GetUrlInput = StrictUnion< + GetUrlInputKey | GetUrlInputPath +>; + +/** @deprecated Use {@link GetPropertiesInputPath} instead. */ +export type GetUrlInputKey = + StorageGetUrlInputKey; +export type GetUrlInputPath = + StorageGetUrlInputPath; /** * Input type for S3 list API. Lists all bucket objects. diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 6356ca81a93..52ebb4d555c 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -73,7 +73,7 @@ export type ListPaginateOptions = StorageListPaginateOptions & /** * Input options type for S3 getUrl API. */ -export type GetUrlOptions = ReadOptions & +export type GetUrlOptions = CommonOptions & { /** * Whether to head object to make sure the object existence before downloading. @@ -85,7 +85,12 @@ export type GetUrlOptions = ReadOptions & * @default 900 (15 minutes) */ expiresIn?: number; - }; +}; + + +/** @deprecated Use {@link GetPropertiesOptionsPath} instead. */ +export type GetUrlOptionsKey = ReadOptions & GetUrlOptions; +export type GetUrlOptionsPath = GetUrlOptions; /** * Input options type for S3 downloadData API. diff --git a/packages/storage/src/types/index.ts b/packages/storage/src/types/index.ts index 78569a76f36..32a82d08fda 100644 --- a/packages/storage/src/types/index.ts +++ b/packages/storage/src/types/index.ts @@ -16,7 +16,8 @@ export { StorageDownloadDataInputPath, StorageUploadDataInput, StorageCopyInput, - StorageGetUrlInput, + StorageGetUrlInputKey, + StorageGetUrlInputPath, StorageUploadDataPayload, } from './inputs'; export { diff --git a/packages/storage/src/types/inputs.ts b/packages/storage/src/types/inputs.ts index 0514feaba30..cd10101cdb8 100644 --- a/packages/storage/src/types/inputs.ts +++ b/packages/storage/src/types/inputs.ts @@ -55,8 +55,12 @@ export interface StorageListInput< options?: Options; } -export type StorageGetUrlInput = - StorageOperationInput; +/** @deprecated Use {@link StorageGetUrlInputPath} instead. */ +export type StorageGetUrlInputKey = + StorageOperationInputKey & StorageOperationInput; + +export type StorageGetUrlInputPath = StorageOperationInputPath & + StorageOperationOptionsInput; export type StorageUploadDataInput = StorageOperationInput & { From 4fdc01f817781f0cc23f5f514166411946f31c00 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 18 Mar 2024 14:32:07 -0700 Subject: [PATCH 05/27] test: first pass to add path coverage --- .../providers/s3/apis/getUrl.test.ts | 101 +++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 1cdf1a59b72..1b5609b44d3 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -8,7 +8,7 @@ import { getPresignedGetObjectUrl, headObject, } from '../../../../src/providers/s3/utils/client'; -import { GetUrlOptions } from '../../../../src/providers/s3/types'; +import { GetUrlOptionsKey, GetUrlOptionsPath } from '../../../../src/providers/s3/types'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -35,7 +35,7 @@ const credentials: AWSCredentials = { const targetIdentityId = 'targetIdentityId'; const defaultIdentityId = 'defaultIdentityId'; -describe('getUrl test', () => { +describe('getUrl test with key', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ credentials, @@ -147,3 +147,100 @@ describe('getUrl test', () => { }); }); }); + +describe('getUrl test with path', () => { + beforeAll(() => { + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId: defaultIdentityId, + }); + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket, + region, + }, + }, + }); + }); + + describe('getUrl happy path', () => { + const config = { + credentials, + region, + userAgentValue: expect.any(String), + }; + const path = 'path'; + beforeEach(() => { + (headObject as jest.Mock).mockImplementation(() => { + return { + Key: path, + ContentLength: '100', + ContentType: 'text/plain', + ETag: 'etag', + LastModified: 'last-modified', + Metadata: { key: 'value' }, + }; + }); + (getPresignedGetObjectUrl as jest.Mock).mockReturnValueOnce({ + url: new URL('https://google.com'), + }); + }); + afterEach(() => { + jest.clearAllMocks(); + }); + [ + { + path: 'path', + expectedKey: 'path', + }, + { + path: () => 'path', + expectedKey: 'path', + }, + ].forEach(({ path, expectedKey }) => { + it(`should getUrl with path ${path} and expectedKey ${expectedKey}`, async () => { + const headObjectOptions = { + Bucket: bucket, + Key: expectedKey, + }; + expect.assertions(4); + const result = await getUrl({ + path, + options: { + validateObjectExistence: true, + } as GetUrlOptionsPath, + }); + expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); + expect(headObject).toHaveBeenCalledTimes(1); + 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(() => { + throw Object.assign(new Error(), { + $metadata: { httpStatusCode: 404 }, + name: 'NotFound', + }); + }); + expect.assertions(2); + try { + await getUrl({ + key: 'invalid_key', + options: { validateObjectExistence: true }, + }); + } catch (error: any) { + expect(headObject).toHaveBeenCalledTimes(1); + expect(error.$metadata?.httpStatusCode).toBe(404); + } + }); + }); +}); From 0cad02273bde619e28a35c6a7bc350727e0d303a Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 18 Mar 2024 14:51:26 -0700 Subject: [PATCH 06/27] fix: adjust tests --- .../providers/s3/apis/getProperties.test.ts | 10 ++--- .../providers/s3/apis/getUrl.test.ts | 19 +++++---- .../src/providers/s3/apis/internal/getUrl.ts | 39 ++++++++++++++----- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 3208711edcf..ec3955d86d3 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -34,7 +34,7 @@ const credentials: AWSCredentials = { secretAccessKey: 'secretAccessKey', }; const key = 'key'; -const path = 'path'; +const path = '/path'; const targetIdentityId = 'targetIdentityId'; const defaultIdentityId = 'defaultIdentityId'; @@ -203,12 +203,12 @@ describe('getProperties with path', () => { }); [ { - path: 'path', - expectedKey: 'path', + path: '/path', + expectedKey: '/path', }, { - path: () => 'path', - expectedKey: 'path', + path: () => '/path', + expectedKey: '/path', }, ].forEach(({ path, expectedKey }) => { it(`should getProperties with path ${path} and expectedKey ${expectedKey}`, async () => { diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 1b5609b44d3..710fb77f75e 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -8,7 +8,10 @@ import { getPresignedGetObjectUrl, headObject, } from '../../../../src/providers/s3/utils/client'; -import { GetUrlOptionsKey, GetUrlOptionsPath } from '../../../../src/providers/s3/types'; +import { + GetUrlOptionsKey, + GetUrlOptionsPath, +} from '../../../../src/providers/s3/types'; jest.mock('../../../../src/providers/s3/utils/client'); jest.mock('@aws-amplify/core', () => ({ @@ -112,7 +115,7 @@ describe('getUrl test with key', () => { options: { ...options, validateObjectExistence: true, - } as GetUrlOptions, + } as GetUrlOptionsKey, }); expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledTimes(1); @@ -170,7 +173,7 @@ describe('getUrl test with path', () => { region, userAgentValue: expect.any(String), }; - const path = 'path'; + const path = '/path'; beforeEach(() => { (headObject as jest.Mock).mockImplementation(() => { return { @@ -191,12 +194,12 @@ describe('getUrl test with path', () => { }); [ { - path: 'path', - expectedKey: 'path', + path: '/path', + expectedKey: '/path', }, { - path: () => 'path', - expectedKey: 'path', + path: () => '/path', + expectedKey: '/path', }, ].forEach(({ path, expectedKey }) => { it(`should getUrl with path ${path} and expectedKey ${expectedKey}`, async () => { @@ -234,7 +237,7 @@ describe('getUrl test with path', () => { expect.assertions(2); try { await getUrl({ - key: 'invalid_key', + path: '/invalid_key', options: { validateObjectExistence: true }, }); } catch (error: any) { diff --git a/packages/storage/src/providers/s3/apis/internal/getUrl.ts b/packages/storage/src/providers/s3/apis/internal/getUrl.ts index 15110d535b7..0bcab3f8e68 100644 --- a/packages/storage/src/providers/s3/apis/internal/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/internal/getUrl.ts @@ -7,12 +7,16 @@ import { StorageAction } from '@aws-amplify/core/internals/utils'; import { GetUrlInput, GetUrlOutput } from '../../types'; import { StorageValidationErrorCode } from '../../../../errors/types/validation'; import { getPresignedGetObjectUrl } from '../../utils/client'; -import { resolveS3ConfigAndInput } from '../../utils'; +import { + resolveS3ConfigAndInput, + validateStorageOperationInput, +} from '../../utils'; import { assertValidationError } from '../../../../errors/utils/assertValidationError'; import { DEFAULT_PRESIGN_EXPIRATION, MAX_URL_EXPIRATION, } from '../../utils/constants'; +import { STORAGE_INPUT_KEY } from '../../utils/constants'; import { getProperties } from './getProperties'; @@ -20,18 +24,35 @@ export const getUrl = async ( amplify: AmplifyClassV6, input: GetUrlInput, ): Promise => { - const { key, options } = input; + const { options: getUrlOptions } = input; - if (options?.validateObjectExistence) { - await getProperties(amplify, { key, options }, StorageAction.GetUrl); + if (getUrlOptions?.validateObjectExistence) { + if (input.key) { + await getProperties( + amplify, + { key: input.key, options: getUrlOptions }, + StorageAction.GetUrl, + ); + } else if (input.path) { + await getProperties( + amplify, + { path: input.path, options: getUrlOptions }, + StorageAction.GetUrl, + ); + } } - const { s3Config, keyPrefix, bucket } = await resolveS3ConfigAndInput( - amplify, - options, + const { s3Config, keyPrefix, bucket, identityId } = + await resolveS3ConfigAndInput(amplify, getUrlOptions); + const { inputType, objectKey } = validateStorageOperationInput( + input, + identityId, ); + const finalKey = + inputType === STORAGE_INPUT_KEY ? keyPrefix + objectKey : objectKey; - let urlExpirationInSec = options?.expiresIn ?? DEFAULT_PRESIGN_EXPIRATION; + let urlExpirationInSec = + getUrlOptions?.expiresIn ?? DEFAULT_PRESIGN_EXPIRATION; const awsCredExpiration = s3Config.credentials?.expiration; if (awsCredExpiration) { const awsCredExpirationInSec = Math.floor( @@ -54,7 +75,7 @@ export const getUrl = async ( }, { Bucket: bucket, - Key: `${keyPrefix}${key}`, + Key: finalKey, }, ), expiresAt: new Date(Date.now() + urlExpirationInSec * 1000), From 23213af24d059774ad65aa942502f72ca729df1d Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Tue, 19 Mar 2024 14:37:46 -0700 Subject: [PATCH 07/27] chore: remove links from tsdocs --- packages/storage/src/providers/s3/apis/getUrl.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index 708e601cc87..4b154b23015 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -3,9 +3,7 @@ import { Amplify } from '@aws-amplify/core'; -import { StorageValidationErrorCode } from '../../../errors/types/validation'; -import { GetUrlInput, GetUrlOutput, S3Exception } from '../types'; -import { StorageError } from '../../../errors/StorageError'; +import { GetUrlInput, GetUrlOutput } from '../types'; import { getUrl as getUrlInternal } from './internal/getUrl'; @@ -16,12 +14,12 @@ import { getUrl as getUrlInternal } from './internal/getUrl'; * * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` * to true, this method will verify the given object already exists in S3 before returning a presigned - * URL, and will throw {@link StorageError} if the object does not exist. + * URL, and will throw StorageError if the object does not exist. * * @param input - The GetUrlInput object. * @returns Presigned URL and timestamp when the URL MAY expire. - * @throws service: {@link S3Exception} - thrown when checking for existence of the object - * @throws validation: {@link StorageValidationErrorCode } - Validation errors + * @throws service: S3Exception - thrown when checking for existence of the object + * @throws validation: StorageValidationErrorCode - Validation errors * thrown either username or key are not defined. * */ From ceaa7a49e07ad905a39484df2422c48ce3e4a5ef Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Tue, 19 Mar 2024 15:46:16 -0700 Subject: [PATCH 08/27] test: fix expected Key --- .../__tests__/providers/s3/apis/getProperties.test.ts | 11 +++++------ .../__tests__/providers/s3/apis/getUrl.test.ts | 7 +++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index ec3955d86d3..50cccc68668 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -34,7 +34,6 @@ const credentials: AWSCredentials = { secretAccessKey: 'secretAccessKey', }; const key = 'key'; -const path = '/path'; const targetIdentityId = 'targetIdentityId'; const defaultIdentityId = 'defaultIdentityId'; @@ -174,7 +173,7 @@ describe('getProperties with path', () => { }); describe('getProperties with path', () => { const expected = { - path, + path: 'path', size: '100', contentType: 'text/plain', eTag: 'etag', @@ -204,11 +203,11 @@ describe('getProperties with path', () => { [ { path: '/path', - expectedKey: '/path', + expectedKey: 'path', }, { path: () => '/path', - expectedKey: '/path', + expectedKey: 'path', }, ].forEach(({ path, expectedKey }) => { it(`should getProperties with path ${path} and expectedKey ${expectedKey}`, async () => { @@ -244,7 +243,7 @@ describe('getProperties with path', () => { ); expect.assertions(3); try { - await getProperties({ path }); + await getProperties({ path: '/path' }); } catch (error: any) { expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith( @@ -255,7 +254,7 @@ describe('getProperties with path', () => { }, { Bucket: 'bucket', - Key: path, + Key: 'path', }, ); 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 710fb77f75e..5b2f1fea6dd 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -173,11 +173,10 @@ describe('getUrl test with path', () => { region, userAgentValue: expect.any(String), }; - const path = '/path'; beforeEach(() => { (headObject as jest.Mock).mockImplementation(() => { return { - Key: path, + Key: 'path', ContentLength: '100', ContentType: 'text/plain', ETag: 'etag', @@ -195,11 +194,11 @@ describe('getUrl test with path', () => { [ { path: '/path', - expectedKey: '/path', + expectedKey: 'path', }, { path: () => '/path', - expectedKey: '/path', + expectedKey: 'path', }, ].forEach(({ path, expectedKey }) => { it(`should getUrl with path ${path} and expectedKey ${expectedKey}`, async () => { From 7b1137d0cace5699da293e2b3e937160f5139fc6 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Tue, 19 Mar 2024 15:58:21 -0700 Subject: [PATCH 09/27] chore: prettier fix --- .../storage/src/providers/s3/types/options.ts | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 57d089010d1..6a17ed29954 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -75,20 +75,18 @@ export type ListPaginateOptions = StorageListPaginateOptions & /** * Input options type for S3 getUrl API. */ -export type GetUrlOptions = - CommonOptions & { - /** - * Whether to head object to make sure the object existence before downloading. - * @default false - */ - validateObjectExistence?: boolean; - /** - * Number of seconds till the URL expires. - * @default 900 (15 minutes) - */ - expiresIn?: number; +export type GetUrlOptions = CommonOptions & { + /** + * Whether to head object to make sure the object existence before downloading. + * @default false + */ + validateObjectExistence?: boolean; + /** + * Number of seconds till the URL expires. + * @default 900 (15 minutes) + */ + expiresIn?: number; }; - /** @deprecated Use {@link GetPropertiesOptionsPath} instead. */ export type GetUrlOptionsKey = ReadOptions & GetUrlOptions; From 972b655331df89079b0e7fc8ac0dc14bb82f3140 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Tue, 19 Mar 2024 16:01:38 -0700 Subject: [PATCH 10/27] chore: increase getUrl bundle size --- packages/aws-amplify/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 9b0a0c825b5..7e11111cd25 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -479,7 +479,7 @@ "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "14.50 kB" + "limit": "14.55 kB" }, { "name": "[Storage] list (S3)", From 2fda5b5ca01442dee431174b1578a3af2658c72a Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Wed, 20 Mar 2024 10:51:34 -0700 Subject: [PATCH 11/27] chore: clean up tsdocs in getProperties --- .../src/providers/s3/apis/getProperties.ts | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index 710597d7c3c..d301120ef4a 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -10,22 +10,10 @@ import { GetPropertiesOutput, GetPropertiesOutputKey, GetPropertiesOutputPath, - S3Exception, } from '../types'; -import { StorageValidationErrorCode } from '../../../errors/types/validation'; import { getProperties as getPropertiesInternal } from './internal/getProperties'; -/** - * Gets the properties of a file. The properties include S3 system metadata and - * the user metadata that was provided when uploading the file. - * - * @param input - The GetPropertiesInput object. - * @returns Requested object properties. - * @throws A {@link S3Exception} when the underlying S3 service returned error. - * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. - */ - interface GetProperties { /** * Gets the properties of a file. The properties include S3 system metadata and @@ -33,8 +21,8 @@ interface GetProperties { * * @param input - The GetPropertiesInput object. * @returns Requested object properties. - * @throws An {@link S3Exception} when the underlying S3 service returned error. - * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. + * @throws An S3Exception when the underlying S3 service returned error. + * @throws A StorageValidationErrorCode when API call parameters are invalid. */ (input: GetPropertiesInputPath): Promise; /** @@ -46,8 +34,8 @@ interface GetProperties { * * @param input - The GetPropertiesInput object. * @returns Requested object properties. - * @throws An {@link S3Exception} when the underlying S3 service returned error. - * @throws A {@link StorageValidationErrorCode} when API call parameters are invalid. + * @throws An S3Exception when the underlying S3 service returned error. + * @throws A StorageValidationErrorCode when API call parameters are invalid. */ (input: GetPropertiesInputKey): Promise; } From c3eda69ce4f699b17a13f5d98e18483fd2c95cd6 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Wed, 20 Mar 2024 11:34:20 -0700 Subject: [PATCH 12/27] chore: consolidate constants imports --- packages/storage/src/providers/s3/apis/internal/getUrl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/providers/s3/apis/internal/getUrl.ts b/packages/storage/src/providers/s3/apis/internal/getUrl.ts index 0bcab3f8e68..2f93d906a78 100644 --- a/packages/storage/src/providers/s3/apis/internal/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/internal/getUrl.ts @@ -15,8 +15,8 @@ import { assertValidationError } from '../../../../errors/utils/assertValidation import { DEFAULT_PRESIGN_EXPIRATION, MAX_URL_EXPIRATION, + STORAGE_INPUT_KEY, } from '../../utils/constants'; -import { STORAGE_INPUT_KEY } from '../../utils/constants'; import { getProperties } from './getProperties'; From 59d541df303d7f7807b24c3dd03837e4c68d6927 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Thu, 21 Mar 2024 15:16:30 -0700 Subject: [PATCH 13/27] test: remove leading slash from path --- .../__tests__/providers/s3/apis/getProperties.test.ts | 6 +++--- packages/storage/__tests__/providers/s3/apis/getUrl.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 50cccc68668..2913e3fccad 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -202,11 +202,11 @@ describe('getProperties with path', () => { }); [ { - path: '/path', + path: 'path', expectedKey: 'path', }, { - path: () => '/path', + path: () => 'path', expectedKey: 'path', }, ].forEach(({ path, expectedKey }) => { @@ -243,7 +243,7 @@ describe('getProperties with path', () => { ); expect.assertions(3); try { - await getProperties({ path: '/path' }); + await getProperties({ path: 'path' }); } catch (error: any) { expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith( diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index 5b2f1fea6dd..c9f872bb0bf 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -193,11 +193,11 @@ describe('getUrl test with path', () => { }); [ { - path: '/path', + path: 'path', expectedKey: 'path', }, { - path: () => '/path', + path: () => 'path', expectedKey: 'path', }, ].forEach(({ path, expectedKey }) => { From 19d6dd81c0e33f8ecf7135d419a719b01c294882 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Thu, 21 Mar 2024 15:58:37 -0700 Subject: [PATCH 14/27] chore: address feedback --- .../providers/s3/apis/getProperties.test.ts | 10 +++--- .../providers/s3/apis/getUrl.test.ts | 10 +++--- .../src/providers/s3/apis/getProperties.ts | 4 +-- .../storage/src/providers/s3/apis/getUrl.ts | 2 +- .../src/providers/s3/apis/internal/getUrl.ts | 31 +++++++++---------- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 2913e3fccad..60b8bf75f6b 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -52,7 +52,7 @@ describe('getProperties with key', () => { }, }); }); - describe('getProperties happy path with key', () => { + describe('Happy cases: With key', () => { const expected = { key, size: '100', @@ -110,7 +110,6 @@ describe('getProperties with key', () => { Bucket: 'bucket', Key: expectedKey, }; - expect.assertions(3); expect( await getProperties({ key, @@ -123,7 +122,7 @@ describe('getProperties with key', () => { }); }); - describe('getProperties error path', () => { + describe('Error cases : With key', () => { afterEach(() => { jest.clearAllMocks(); }); @@ -156,7 +155,7 @@ describe('getProperties with key', () => { }); }); -describe('getProperties with path', () => { +describe('Happy cases: With path', () => { beforeAll(() => { mockFetchAuthSession.mockResolvedValue({ credentials, @@ -215,7 +214,6 @@ describe('getProperties with path', () => { Bucket: 'bucket', Key: expectedKey, }; - expect.assertions(3); expect( await getProperties({ path, @@ -230,7 +228,7 @@ describe('getProperties with path', () => { }); }); - describe('getProperties error path with path', () => { + describe('Error cases : With 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 c9f872bb0bf..d45447abdcd 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -54,7 +54,7 @@ describe('getUrl test with key', () => { }); }); - describe('getUrl happy path', () => { + describe('Happy cases: With key', () => { const config = { credentials, region, @@ -109,7 +109,6 @@ describe('getUrl test with key', () => { Bucket: bucket, Key: expectedKey, }; - expect.assertions(4); const result = await getUrl({ key, options: { @@ -126,7 +125,7 @@ describe('getUrl test with key', () => { }); }); }); - describe('getUrl error path', () => { + describe('Error cases : With key', () => { afterAll(() => { jest.clearAllMocks(); }); @@ -167,7 +166,7 @@ describe('getUrl test with path', () => { }); }); - describe('getUrl happy path', () => { + describe('Happy cases: With path', () => { const config = { credentials, region, @@ -206,7 +205,6 @@ describe('getUrl test with path', () => { Bucket: bucket, Key: expectedKey, }; - expect.assertions(4); const result = await getUrl({ path, options: { @@ -222,7 +220,7 @@ describe('getUrl test with path', () => { }); }); }); - describe('getUrl error path', () => { + describe('Error cases : With path', () => { afterAll(() => { jest.clearAllMocks(); }); diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index d301120ef4a..1bb5a10b2ca 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -21,7 +21,7 @@ interface GetProperties { * * @param input - The GetPropertiesInput object. * @returns Requested object properties. - * @throws An S3Exception when the underlying S3 service returned error. + * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A StorageValidationErrorCode when API call parameters are invalid. */ (input: GetPropertiesInputPath): Promise; @@ -34,7 +34,7 @@ interface GetProperties { * * @param input - The GetPropertiesInput object. * @returns Requested object properties. - * @throws An S3Exception when the underlying S3 service returned error. + * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A StorageValidationErrorCode when API call parameters are invalid. */ (input: GetPropertiesInputKey): Promise; diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index 4b154b23015..353dff9763f 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -18,7 +18,7 @@ import { getUrl as getUrlInternal } from './internal/getUrl'; * * @param input - The GetUrlInput object. * @returns Presigned URL and timestamp when the URL MAY expire. - * @throws service: S3Exception - thrown when checking for existence of the object + * @throws service: `S3Exception` - thrown when checking for existence of the object * @throws validation: StorageValidationErrorCode - Validation errors * thrown either username or key are not defined. * diff --git a/packages/storage/src/providers/s3/apis/internal/getUrl.ts b/packages/storage/src/providers/s3/apis/internal/getUrl.ts index 2f93d906a78..ea2fd60d741 100644 --- a/packages/storage/src/providers/s3/apis/internal/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/internal/getUrl.ts @@ -25,32 +25,29 @@ export const getUrl = async ( input: GetUrlInput, ): Promise => { const { options: getUrlOptions } = input; - - if (getUrlOptions?.validateObjectExistence) { - if (input.key) { - await getProperties( - amplify, - { key: input.key, options: getUrlOptions }, - StorageAction.GetUrl, - ); - } else if (input.path) { - await getProperties( - amplify, - { path: input.path, options: getUrlOptions }, - StorageAction.GetUrl, - ); - } - } - const { s3Config, keyPrefix, bucket, identityId } = await resolveS3ConfigAndInput(amplify, getUrlOptions); const { inputType, objectKey } = validateStorageOperationInput( input, identityId, ); + const finalKey = inputType === STORAGE_INPUT_KEY ? keyPrefix + objectKey : objectKey; + if (getUrlOptions?.validateObjectExistence) { + await getProperties( + amplify, + { + options: getUrlOptions, + ...((inputType === STORAGE_INPUT_KEY + ? { key: input.key } + : { path: input.path }) as GetUrlInput), + }, + StorageAction.GetUrl, + ); + } + let urlExpirationInSec = getUrlOptions?.expiresIn ?? DEFAULT_PRESIGN_EXPIRATION; const awsCredExpiration = s3Config.credentials?.expiration; From d8b4b1273464902744060e092ac3ab27861f8ba6 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 10:21:19 -0700 Subject: [PATCH 15/27] chore: fix test and bundle size check --- packages/aws-amplify/package.json | 2 +- packages/storage/__tests__/providers/s3/apis/getUrl.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 073dc324ce8..042b46881be 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -479,7 +479,7 @@ "name": "[Storage] getUrl (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ getUrl }", - "limit": "14.55 kB" + "limit": "14.60 kB" }, { "name": "[Storage] list (S3)", diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index d45447abdcd..e9d74ee67bc 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -234,7 +234,7 @@ describe('getUrl test with path', () => { expect.assertions(2); try { await getUrl({ - path: '/invalid_key', + path: 'invalid_key', options: { validateObjectExistence: true }, }); } catch (error: any) { From ee54ff31282385f07316da2382c1fa10f3ce1b17 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 10:38:48 -0700 Subject: [PATCH 16/27] chore: add back-tics to tsdocs --- packages/storage/src/providers/s3/apis/getProperties.ts | 8 ++++---- packages/storage/src/providers/s3/apis/getUrl.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index 1bb5a10b2ca..e74cc28de70 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -19,10 +19,10 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param input - The GetPropertiesInput object. + * @param input - The `GetPropertiesInput` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. - * @throws A StorageValidationErrorCode when API call parameters are invalid. + * @throws A `StorageValidationErrorCode` when API call parameters are invalid. */ (input: GetPropertiesInputPath): Promise; /** @@ -32,10 +32,10 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param input - The GetPropertiesInput object. + * @param input - The `GetPropertiesInput` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. - * @throws A StorageValidationErrorCode when API call parameters are invalid. + * @throws A `StorageValidationErrorCode` when API call parameters are invalid. */ (input: GetPropertiesInputKey): Promise; } diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index 353dff9763f..fc68d0827a6 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -14,12 +14,12 @@ import { getUrl as getUrlInternal } from './internal/getUrl'; * * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` * to true, this method will verify the given object already exists in S3 before returning a presigned - * URL, and will throw StorageError if the object does not exist. + * URL, and will throw `StorageError` if the object does not exist. * - * @param input - The GetUrlInput object. + * @param input - The `GetUrlInput` object. * @returns Presigned URL and timestamp when the URL MAY expire. * @throws service: `S3Exception` - thrown when checking for existence of the object - * @throws validation: StorageValidationErrorCode - Validation errors + * @throws validation: `StorageValidationErrorCode` - Validation errors * thrown either username or key are not defined. * */ From f138c318ceffdda5792b2dcdcbb105d6f1a7d9c9 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 10:58:47 -0700 Subject: [PATCH 17/27] chore: use test.each instead of forEach --- .../providers/s3/apis/getProperties.test.ts | 26 +++++++++--------- .../providers/s3/apis/getUrl.test.ts | 27 +++++++++---------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index 60b8bf75f6b..b73b1ab0362 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -80,7 +80,7 @@ describe('getProperties with key', () => { afterEach(() => { jest.clearAllMocks(); }); - [ + test.each([ { expectedKey: `public/${key}`, }, @@ -100,12 +100,9 @@ describe('getProperties with key', () => { options: { accessLevel: 'protected', targetIdentityId }, expectedKey: `protected/${targetIdentityId}/${key}`, }, - ].forEach(({ options, expectedKey }) => { - const accessLevelMsg = options?.accessLevel ?? 'default'; - const targetIdentityIdMsg = options?.targetIdentityId - ? `and targetIdentityId` - : ''; - it(`should getProperties with ${accessLevelMsg} accessLevel ${targetIdentityIdMsg}`, async () => { + ])( + 'should getProperties with key $expectedKey', + async ({ options, expectedKey }) => { const headObjectOptions = { Bucket: 'bucket', Key: expectedKey, @@ -118,8 +115,8 @@ describe('getProperties with key', () => { ).toEqual(expected); expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); - }); - }); + }, + ); }); describe('Error cases : With key', () => { @@ -199,7 +196,7 @@ describe('Happy cases: With path', () => { afterEach(() => { jest.clearAllMocks(); }); - [ + test.each([ { path: 'path', expectedKey: 'path', @@ -208,8 +205,9 @@ describe('Happy cases: With path', () => { path: () => 'path', expectedKey: 'path', }, - ].forEach(({ path, expectedKey }) => { - it(`should getProperties with path ${path} and expectedKey ${expectedKey}`, async () => { + ])( + 'should getProperties with path $path and expectedKey $expectedKey', + async ({ path, expectedKey }) => { const headObjectOptions = { Bucket: 'bucket', Key: expectedKey, @@ -224,8 +222,8 @@ describe('Happy cases: With path', () => { ).toEqual(expected); expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith(config, headObjectOptions); - }); - }); + }, + ); }); describe('Error cases : With path', () => { diff --git a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts index e9d74ee67bc..28804743c7d 100644 --- a/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getUrl.test.ts @@ -79,7 +79,7 @@ describe('getUrl test with key', () => { afterEach(() => { jest.clearAllMocks(); }); - [ + test.each([ { expectedKey: `public/${key}`, }, @@ -99,12 +99,9 @@ describe('getUrl test with key', () => { options: { accessLevel: 'protected', targetIdentityId }, expectedKey: `protected/${targetIdentityId}/${key}`, }, - ].forEach(({ options, expectedKey }) => { - const accessLevelMsg = options?.accessLevel ?? 'default'; - const targetIdentityIdMsg = options?.targetIdentityId - ? `and targetIdentityId` - : ''; - it(`should getUrl with ${accessLevelMsg} accessLevel ${targetIdentityIdMsg}`, async () => { + ])( + 'should getUrl with key $expectedKey', + async ({ options, expectedKey }) => { const headObjectOptions = { Bucket: bucket, Key: expectedKey, @@ -122,8 +119,8 @@ describe('getUrl test with key', () => { expect(result.url).toEqual({ url: new URL('https://google.com'), }); - }); - }); + }, + ); }); describe('Error cases : With key', () => { afterAll(() => { @@ -190,7 +187,8 @@ describe('getUrl test with path', () => { afterEach(() => { jest.clearAllMocks(); }); - [ + + test.each([ { path: 'path', expectedKey: 'path', @@ -199,8 +197,9 @@ describe('getUrl test with path', () => { path: () => 'path', expectedKey: 'path', }, - ].forEach(({ path, expectedKey }) => { - it(`should getUrl with path ${path} and expectedKey ${expectedKey}`, async () => { + ])( + 'should getUrl with path $path and expectedKey $expectedKey', + async ({ path, expectedKey }) => { const headObjectOptions = { Bucket: bucket, Key: expectedKey, @@ -217,8 +216,8 @@ describe('getUrl test with path', () => { expect(result.url).toEqual({ url: new URL('https://google.com'), }); - }); - }); + }, + ); }); describe('Error cases : With path', () => { afterAll(() => { From b069077d2cc9242857b827f93190e7f50ac4b075 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 14:33:43 -0700 Subject: [PATCH 18/27] chore: fix deprecation and tsdocs --- .../providers/s3/apis/server/getProperties.ts | 23 +++++++++++++++++++ .../storage/src/providers/s3/types/outputs.ts | 1 + 2 files changed, 24 insertions(+) diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index 4b61bfa83a9..ac1e3d69574 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -17,10 +17,33 @@ import { import { getProperties as getPropertiesInternal } from '../internal/getProperties'; interface GetProperties { + /** + * Gets the properties of a file. The properties include S3 system metadata and + * the user metadata that was provided when uploading the file. + * + * @param contextSpec - The isolated server context. + * @param input - The `GetPropertiesInput` object. + * @returns Requested object properties. + * @throws An `S3Exception` when the underlying S3 service returned error. + * @throws A `StorageValidationErrorCode` when API call parameters are invalid. + */ ( contextSpec: AmplifyServer.ContextSpec, input: GetPropertiesInputPath, ): Promise; + /** + * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. + * Please use {@link https://docs.amplify.aws/react/build-a-backend/storage/download/#downloaddata | path} instead. + * + * Gets the properties of a file. The properties include S3 system metadata and + * the user metadata that was provided when uploading the file. + * + * @param contextSpec - The isolated server context. + * @param input - The `GetPropertiesInput` object. + * @returns Requested object properties. + * @throws An `S3Exception` when the underlying S3 service returned error. + * @throws A `StorageValidationErrorCode` when API call parameters are invalid. + */ ( contextSpec: AmplifyServer.ContextSpec, input: GetPropertiesInputKey, diff --git a/packages/storage/src/providers/s3/types/outputs.ts b/packages/storage/src/providers/s3/types/outputs.ts index 4f3e9f4117d..d9aba46f502 100644 --- a/packages/storage/src/providers/s3/types/outputs.ts +++ b/packages/storage/src/providers/s3/types/outputs.ts @@ -61,6 +61,7 @@ export type GetUrlOutput = StorageGetUrlOutput; */ export type UploadDataOutput = UploadTask; +/** @deprecated Use {@link GetPropertiesOutputPath} instead. */ export type GetPropertiesOutputKey = ItemKey; export type GetPropertiesOutputPath = ItemPath; From f3305c800f133b5e90f57290a635eda96ba93e8e Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 14:37:03 -0700 Subject: [PATCH 19/27] chore: add contextSpec to copy tsdocs --- packages/storage/src/providers/s3/apis/copy.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/storage/src/providers/s3/apis/copy.ts b/packages/storage/src/providers/s3/apis/copy.ts index 1978d9595b3..6e56b33b877 100644 --- a/packages/storage/src/providers/s3/apis/copy.ts +++ b/packages/storage/src/providers/s3/apis/copy.ts @@ -18,6 +18,7 @@ interface Copy { /** * Copy an object from a source to a destination object within the same bucket. * + * @param contextSpec - The isolated server context. * @param input - The CopyInputPath object. * @returns Output containing the destination object path. * @throws service: `S3Exception` - Thrown when checking for existence of the object @@ -32,6 +33,7 @@ interface Copy { * Copy an object from a source to a destination object within the same bucket. Can optionally copy files across * different accessLevel or identityId (if source object's accessLevel is 'protected'). * + * @param contextSpec - The isolated server context. * @param input - The CopyInputKey object. * @returns Output containing the destination object key. * @throws service: `S3Exception` - Thrown when checking for existence of the object From 23a5076c6daeff22bca1196e246a38df6eff25d5 Mon Sep 17 00:00:00 2001 From: erinleigh90 <106691284+erinleigh90@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:23:50 -0700 Subject: [PATCH 20/27] Update packages/storage/src/providers/s3/apis/copy.ts Co-authored-by: ashika112 <155593080+ashika112@users.noreply.github.com> --- packages/storage/src/providers/s3/apis/copy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storage/src/providers/s3/apis/copy.ts b/packages/storage/src/providers/s3/apis/copy.ts index 6e56b33b877..44820fbbb9c 100644 --- a/packages/storage/src/providers/s3/apis/copy.ts +++ b/packages/storage/src/providers/s3/apis/copy.ts @@ -18,7 +18,7 @@ interface Copy { /** * Copy an object from a source to a destination object within the same bucket. * - * @param contextSpec - The isolated server context. + * @param contextSpec - The isolated server context. * @param input - The CopyInputPath object. * @returns Output containing the destination object path. * @throws service: `S3Exception` - Thrown when checking for existence of the object From b2c4ccebdb594f02f46c0d8d8e87986167641cdd Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 15:26:32 -0700 Subject: [PATCH 21/27] chore: another tsdoc fix --- packages/storage/src/providers/s3/apis/copy.ts | 2 -- packages/storage/src/providers/s3/apis/server/copy.ts | 2 ++ .../storage/src/providers/s3/apis/server/getProperties.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/copy.ts b/packages/storage/src/providers/s3/apis/copy.ts index 44820fbbb9c..1978d9595b3 100644 --- a/packages/storage/src/providers/s3/apis/copy.ts +++ b/packages/storage/src/providers/s3/apis/copy.ts @@ -18,7 +18,6 @@ interface Copy { /** * Copy an object from a source to a destination object within the same bucket. * - * @param contextSpec - The isolated server context. * @param input - The CopyInputPath object. * @returns Output containing the destination object path. * @throws service: `S3Exception` - Thrown when checking for existence of the object @@ -33,7 +32,6 @@ interface Copy { * Copy an object from a source to a destination object within the same bucket. Can optionally copy files across * different accessLevel or identityId (if source object's accessLevel is 'protected'). * - * @param contextSpec - The isolated server context. * @param input - The CopyInputKey object. * @returns Output containing the destination object key. * @throws service: `S3Exception` - Thrown when checking for existence of the object diff --git a/packages/storage/src/providers/s3/apis/server/copy.ts b/packages/storage/src/providers/s3/apis/server/copy.ts index be7f6a3d012..308bd05a89a 100644 --- a/packages/storage/src/providers/s3/apis/server/copy.ts +++ b/packages/storage/src/providers/s3/apis/server/copy.ts @@ -19,6 +19,7 @@ interface Copy { /** * Copy an object from a source to a destination object within the same bucket. * + * @param contextSpec - The isolated server context. * @param input - The CopyInputPath object. * @returns Output containing the destination object path. * @throws service: `S3Exception` - Thrown when checking for existence of the object @@ -36,6 +37,7 @@ interface Copy { * Copy an object from a source to a destination object within the same bucket. Can optionally copy files across * different accessLevel or identityId (if source object's accessLevel is 'protected'). * + * @param contextSpec - The isolated server context. * @param input - The CopyInputKey object. * @returns Output containing the destination object key. * @throws service: `S3Exception` - Thrown when checking for existence of the object diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index ac1e3d69574..9ea4345ee46 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -21,7 +21,7 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param contextSpec - The isolated server context. + * @param contextSpec - The isolated server context. * @param input - The `GetPropertiesInput` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. @@ -38,7 +38,7 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param contextSpec - The isolated server context. + * @param contextSpec - The isolated server context. * @param input - The `GetPropertiesInput` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. From 644aee18c5d5e385d2314f610df75f48b5e9faf7 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Fri, 22 Mar 2024 15:35:28 -0700 Subject: [PATCH 22/27] chore: add tsdocs to server getUrl --- .../src/providers/s3/apis/server/getUrl.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/storage/src/providers/s3/apis/server/getUrl.ts b/packages/storage/src/providers/s3/apis/server/getUrl.ts index 8cdfe2ffb4e..a6c809aa233 100644 --- a/packages/storage/src/providers/s3/apis/server/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/server/getUrl.ts @@ -9,6 +9,23 @@ import { import { GetUrlInput, GetUrlOutput } from '../../types'; import { getUrl as getUrlInternal } from '../internal/getUrl'; +/** + * Get a temporary presigned URL to download the specified S3 object. + * The presigned URL expires when the associated role used to sign the request expires or + * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. + * + * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` + * to true, this method will verify the given object already exists in S3 before returning a presigned + * URL, and will throw `StorageError` if the object does not exist. + * + * @param contextSpec - The isolated server context. + * @param input - The `GetUrlInput` object. + * @returns Presigned URL and timestamp when the URL MAY expire. + * @throws service: `S3Exception` - thrown when checking for existence of the object + * @throws validation: `StorageValidationErrorCode` - Validation errors + * thrown either username or key are not defined. + * + */ export const getUrl = async ( contextSpec: AmplifyServer.ContextSpec, input: GetUrlInput, From ec73fc8a439bd9fde819f2a2818aa6b16c1db073 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 25 Mar 2024 09:00:08 -0700 Subject: [PATCH 23/27] address feedback --- .../providers/s3/apis/getProperties.test.ts | 19 ++++++++++--------- .../s3/apis/internal/getProperties.ts | 2 +- .../storage/src/providers/s3/types/inputs.ts | 2 +- .../storage/src/providers/s3/types/options.ts | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts index b73b1ab0362..a2f0df8e886 100644 --- a/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts +++ b/packages/storage/__tests__/providers/s3/apis/getProperties.test.ts @@ -34,6 +34,7 @@ const credentials: AWSCredentials = { secretAccessKey: 'secretAccessKey', }; const key = 'key'; +const path = 'path'; const targetIdentityId = 'targetIdentityId'; const defaultIdentityId = 'defaultIdentityId'; @@ -169,7 +170,7 @@ describe('Happy cases: With path', () => { }); describe('getProperties with path', () => { const expected = { - path: 'path', + path, size: '100', contentType: 'text/plain', eTag: 'etag', @@ -198,23 +199,23 @@ describe('Happy cases: With path', () => { }); test.each([ { - path: 'path', - expectedKey: 'path', + testPath: path, + expectedKey: path, }, { - path: () => 'path', - expectedKey: 'path', + testPath: () => path, + expectedKey: path, }, ])( 'should getProperties with path $path and expectedKey $expectedKey', - async ({ path, expectedKey }) => { + async ({ testPath, expectedKey }) => { const headObjectOptions = { Bucket: 'bucket', Key: expectedKey, }; expect( await getProperties({ - path, + path: testPath, options: { useAccelerateEndpoint: true, } as GetPropertiesOptionsPath, @@ -239,7 +240,7 @@ describe('Happy cases: With path', () => { ); expect.assertions(3); try { - await getProperties({ path: 'path' }); + await getProperties({ path }); } catch (error: any) { expect(headObject).toHaveBeenCalledTimes(1); expect(headObject).toHaveBeenCalledWith( @@ -250,7 +251,7 @@ describe('Happy cases: With path', () => { }, { Bucket: 'bucket', - Key: 'path', + Key: path, }, ); expect(error.$metadata.httpStatusCode).toBe(404); diff --git a/packages/storage/src/providers/s3/apis/internal/getProperties.ts b/packages/storage/src/providers/s3/apis/internal/getProperties.ts index b934497da6c..e7f20187a36 100644 --- a/packages/storage/src/providers/s3/apis/internal/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/internal/getProperties.ts @@ -54,5 +54,5 @@ export const getProperties = async ( return inputType === STORAGE_INPUT_KEY ? { key: objectKey, ...result } - : { path: finalKey, ...result }; + : { path: objectKey, ...result }; }; diff --git a/packages/storage/src/providers/s3/types/inputs.ts b/packages/storage/src/providers/s3/types/inputs.ts index b1b78c6b7d3..22f3cd73095 100644 --- a/packages/storage/src/providers/s3/types/inputs.ts +++ b/packages/storage/src/providers/s3/types/inputs.ts @@ -62,7 +62,7 @@ export type GetPropertiesInputPath = */ export type GetUrlInput = StrictUnion; -/** @deprecated Use {@link GetPropertiesInputPath} instead. */ +/** @deprecated Use {@link GetUrlInputPath} instead. */ export type GetUrlInputKey = StorageGetUrlInputKey; export type GetUrlInputPath = StorageGetUrlInputPath; diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 62316431261..92f32ef0187 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -96,7 +96,7 @@ export type GetUrlOptions = CommonOptions & { expiresIn?: number; }; -/** @deprecated Use {@link GetPropertiesOptionsPath} instead. */ +/** @deprecated Use {@link GetUrlOptionsPath} instead. */ export type GetUrlOptionsKey = ReadOptions & GetUrlOptions; export type GetUrlOptionsPath = GetUrlOptions; From 7b530cf9f7f19909904c9870dff41d622aaeaf17 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 25 Mar 2024 09:14:27 -0700 Subject: [PATCH 24/27] address feedback --- packages/storage/src/providers/s3/apis/getProperties.ts | 6 ++---- packages/storage/src/providers/s3/apis/getUrl.ts | 5 ++--- .../storage/src/providers/s3/apis/server/getProperties.ts | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index e74cc28de70..8180df35ea7 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -27,7 +27,7 @@ interface GetProperties { (input: GetPropertiesInputPath): Promise; /** * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. - * Please use {@link https://docs.amplify.aws/react/build-a-backend/storage/download/#downloaddata | path} instead. + * Please use {@link https://docs.amplify.aws/javascript/build-a-backend/storage/get-properties/ | path} instead. * * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. @@ -44,6 +44,4 @@ export const getProperties: GetProperties = < Output extends GetPropertiesOutput, >( input: GetPropertiesInput, -): Promise => { - return getPropertiesInternal(Amplify, input) as Promise; -}; +): Promise => getPropertiesInternal(Amplify, input) as Promise; diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index fc68d0827a6..79ecc691db4 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -23,6 +23,5 @@ import { getUrl as getUrlInternal } from './internal/getUrl'; * thrown either username or key are not defined. * */ -export const getUrl = (input: GetUrlInput): Promise => { - return getUrlInternal(Amplify, input); -}; +export const getUrl = (input: GetUrlInput): Promise => + getUrlInternal(Amplify, input); diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index 9ea4345ee46..95762d393b3 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -33,7 +33,7 @@ interface GetProperties { ): Promise; /** * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. - * Please use {@link https://docs.amplify.aws/react/build-a-backend/storage/download/#downloaddata | path} instead. + * Please use {@link https://docs.amplify.aws/javascript/build-a-backend/storage/get-properties/ | path} instead. * * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. From 3b5a4026690965a6b939739bbac9f1c588f7f9a9 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 25 Mar 2024 09:21:33 -0700 Subject: [PATCH 25/27] address feedback --- .../storage/src/providers/s3/apis/server/getProperties.ts | 5 ++--- packages/storage/src/providers/s3/apis/server/getUrl.ts | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index 95762d393b3..33f09463bbf 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -55,9 +55,8 @@ export const getProperties: GetProperties = < >( contextSpec: AmplifyServer.ContextSpec, input: GetPropertiesInput, -): Promise => { - return getPropertiesInternal( +): Promise => + getPropertiesInternal( getAmplifyServerContext(contextSpec).amplify, input, ) as Promise; -}; diff --git a/packages/storage/src/providers/s3/apis/server/getUrl.ts b/packages/storage/src/providers/s3/apis/server/getUrl.ts index a6c809aa233..86e689b6a92 100644 --- a/packages/storage/src/providers/s3/apis/server/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/server/getUrl.ts @@ -29,6 +29,5 @@ import { getUrl as getUrlInternal } from '../internal/getUrl'; export const getUrl = async ( contextSpec: AmplifyServer.ContextSpec, input: GetUrlInput, -): Promise => { - return getUrlInternal(getAmplifyServerContext(contextSpec).amplify, input); -}; +): Promise => + getUrlInternal(getAmplifyServerContext(contextSpec).amplify, input); From 30d5b860fcd15109bc2c7efb88c9c0fb465d5563 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 25 Mar 2024 09:37:13 -0700 Subject: [PATCH 26/27] chore: add overload for deprecation messaging to getUrl --- .../storage/src/providers/s3/apis/getUrl.ts | 65 ++++++++++++++----- .../storage/src/providers/s3/types/index.ts | 2 + 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index 79ecc691db4..de3b61e82bd 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -3,25 +3,54 @@ import { Amplify } from '@aws-amplify/core'; -import { GetUrlInput, GetUrlOutput } from '../types'; +import { + GetUrlInput, + GetUrlInputKey, + GetUrlInputPath, + GetUrlOutput, +} from '../types'; import { getUrl as getUrlInternal } from './internal/getUrl'; -/** - * Get a temporary presigned URL to download the specified S3 object. - * The presigned URL expires when the associated role used to sign the request expires or - * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. - * - * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` - * to true, this method will verify the given object already exists in S3 before returning a presigned - * URL, and will throw `StorageError` if the object does not exist. - * - * @param input - The `GetUrlInput` object. - * @returns Presigned URL and timestamp when the URL MAY expire. - * @throws service: `S3Exception` - thrown when checking for existence of the object - * @throws validation: `StorageValidationErrorCode` - Validation errors - * thrown either username or key are not defined. - * - */ -export const getUrl = (input: GetUrlInput): Promise => +interface GetUrl { + /** + * Get a temporary presigned URL to download the specified S3 object. + * The presigned URL expires when the associated role used to sign the request expires or + * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. + * + * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` + * to true, this method will verify the given object already exists in S3 before returning a presigned + * URL, and will throw `StorageError` if the object does not exist. + * + * @param input - The `GetUrlInput` object. + * @returns Presigned URL and timestamp when the URL MAY expire. + * @throws service: `S3Exception` - thrown when checking for existence of the object + * @throws validation: `StorageValidationErrorCode` - Validation errors + * thrown either username or key are not defined. + * + */ + (input: GetUrlInputPath): Promise; + /** + * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. + * Please use {@link https://docs.amplify.aws/javascript/build-a-backend/storage/download/#generate-a-download-url | path} instead. + * + * Get a temporary presigned URL to download the specified S3 object. + * The presigned URL expires when the associated role used to sign the request expires or + * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. + * + * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` + * to true, this method will verify the given object already exists in S3 before returning a presigned + * URL, and will throw `StorageError` if the object does not exist. + * + * @param input - The `GetUrlInput` object. + * @returns Presigned URL and timestamp when the URL MAY expire. + * @throws service: `S3Exception` - thrown when checking for existence of the object + * @throws validation: `StorageValidationErrorCode` - Validation errors + * thrown either username or key are not defined. + * + */ + (input: GetUrlInputKey): Promise; +} + +export const getUrl: GetUrl = (input: GetUrlInput): Promise => getUrlInternal(Amplify, input); diff --git a/packages/storage/src/providers/s3/types/index.ts b/packages/storage/src/providers/s3/types/index.ts index 6524176229b..e8306cd507f 100644 --- a/packages/storage/src/providers/s3/types/index.ts +++ b/packages/storage/src/providers/s3/types/index.ts @@ -40,6 +40,8 @@ export { GetPropertiesInputKey, GetPropertiesInputPath, GetUrlInput, + GetUrlInputKey, + GetUrlInputPath, ListAllInput, ListPaginateInput, RemoveInput, From b604e0a6aa5b6fc5de25b9aa5b9442703ab3b897 Mon Sep 17 00:00:00 2001 From: Erin Beal Date: Mon, 25 Mar 2024 10:41:26 -0700 Subject: [PATCH 27/27] address feedback --- .../src/providers/s3/apis/getProperties.ts | 4 +- .../storage/src/providers/s3/apis/getUrl.ts | 4 +- .../providers/s3/apis/server/getProperties.ts | 4 +- .../src/providers/s3/apis/server/getUrl.ts | 73 ++++++++++++++----- 4 files changed, 60 insertions(+), 25 deletions(-) diff --git a/packages/storage/src/providers/s3/apis/getProperties.ts b/packages/storage/src/providers/s3/apis/getProperties.ts index 8180df35ea7..227406bc369 100644 --- a/packages/storage/src/providers/s3/apis/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/getProperties.ts @@ -19,7 +19,7 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param input - The `GetPropertiesInput` object. + * @param input - The `GetPropertiesInputPath` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A `StorageValidationErrorCode` when API call parameters are invalid. @@ -32,7 +32,7 @@ interface GetProperties { * Gets the properties of a file. The properties include S3 system metadata and * the user metadata that was provided when uploading the file. * - * @param input - The `GetPropertiesInput` object. + * @param input - The `GetPropertiesInputKey` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A `StorageValidationErrorCode` when API call parameters are invalid. diff --git a/packages/storage/src/providers/s3/apis/getUrl.ts b/packages/storage/src/providers/s3/apis/getUrl.ts index de3b61e82bd..a49210063a3 100644 --- a/packages/storage/src/providers/s3/apis/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/getUrl.ts @@ -22,7 +22,7 @@ interface GetUrl { * to true, this method will verify the given object already exists in S3 before returning a presigned * URL, and will throw `StorageError` if the object does not exist. * - * @param input - The `GetUrlInput` object. + * @param input - The `GetUrlInputPath` object. * @returns Presigned URL and timestamp when the URL MAY expire. * @throws service: `S3Exception` - thrown when checking for existence of the object * @throws validation: `StorageValidationErrorCode` - Validation errors @@ -42,7 +42,7 @@ interface GetUrl { * to true, this method will verify the given object already exists in S3 before returning a presigned * URL, and will throw `StorageError` if the object does not exist. * - * @param input - The `GetUrlInput` object. + * @param input - The `GetUrlInputKey` object. * @returns Presigned URL and timestamp when the URL MAY expire. * @throws service: `S3Exception` - thrown when checking for existence of the object * @throws validation: `StorageValidationErrorCode` - Validation errors diff --git a/packages/storage/src/providers/s3/apis/server/getProperties.ts b/packages/storage/src/providers/s3/apis/server/getProperties.ts index 33f09463bbf..a3c7e0c254e 100644 --- a/packages/storage/src/providers/s3/apis/server/getProperties.ts +++ b/packages/storage/src/providers/s3/apis/server/getProperties.ts @@ -22,7 +22,7 @@ interface GetProperties { * the user metadata that was provided when uploading the file. * * @param contextSpec - The isolated server context. - * @param input - The `GetPropertiesInput` object. + * @param input - The `GetPropertiesInputPath` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A `StorageValidationErrorCode` when API call parameters are invalid. @@ -39,7 +39,7 @@ interface GetProperties { * the user metadata that was provided when uploading the file. * * @param contextSpec - The isolated server context. - * @param input - The `GetPropertiesInput` object. + * @param input - The `GetPropertiesInputKey` object. * @returns Requested object properties. * @throws An `S3Exception` when the underlying S3 service returned error. * @throws A `StorageValidationErrorCode` when API call parameters are invalid. diff --git a/packages/storage/src/providers/s3/apis/server/getUrl.ts b/packages/storage/src/providers/s3/apis/server/getUrl.ts index 86e689b6a92..7843b6323cf 100644 --- a/packages/storage/src/providers/s3/apis/server/getUrl.ts +++ b/packages/storage/src/providers/s3/apis/server/getUrl.ts @@ -6,27 +6,62 @@ import { getAmplifyServerContext, } from '@aws-amplify/core/internals/adapter-core'; -import { GetUrlInput, GetUrlOutput } from '../../types'; +import { + GetUrlInput, + GetUrlInputKey, + GetUrlInputPath, + GetUrlOutput, +} from '../../types'; import { getUrl as getUrlInternal } from '../internal/getUrl'; -/** - * Get a temporary presigned URL to download the specified S3 object. - * The presigned URL expires when the associated role used to sign the request expires or - * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. - * - * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` - * to true, this method will verify the given object already exists in S3 before returning a presigned - * URL, and will throw `StorageError` if the object does not exist. - * - * @param contextSpec - The isolated server context. - * @param input - The `GetUrlInput` object. - * @returns Presigned URL and timestamp when the URL MAY expire. - * @throws service: `S3Exception` - thrown when checking for existence of the object - * @throws validation: `StorageValidationErrorCode` - Validation errors - * thrown either username or key are not defined. - * - */ -export const getUrl = async ( +interface GetUrl { + /** + * Get a temporary presigned URL to download the specified S3 object. + * The presigned URL expires when the associated role used to sign the request expires or + * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. + * + * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` + * to true, this method will verify the given object already exists in S3 before returning a presigned + * URL, and will throw `StorageError` if the object does not exist. + * + * @param contextSpec - The isolated server context. + * @param input - The `GetUrlInputPath` object. + * @returns Presigned URL and timestamp when the URL MAY expire. + * @throws service: `S3Exception` - thrown when checking for existence of the object + * @throws validation: `StorageValidationErrorCode` - Validation errors + * thrown either username or key are not defined. + * + */ + ( + contextSpec: AmplifyServer.ContextSpec, + input: GetUrlInputPath, + ): Promise; + /** + * @deprecated The `key` and `accessLevel` parameters are deprecated and may be removed in the next major version. + * Please use {@link https://docs.amplify.aws/javascript/build-a-backend/storage/download/#generate-a-download-url | path} instead. + * + * Get a temporary presigned URL to download the specified S3 object. + * The presigned URL expires when the associated role used to sign the request expires or + * the option `expiresIn` is reached. The `expiresAt` property in the output object indicates when the URL MAY expire. + * + * By default, it will not validate the object that exists in S3. If you set the `options.validateObjectExistence` + * to true, this method will verify the given object already exists in S3 before returning a presigned + * URL, and will throw `StorageError` if the object does not exist. + * + * @param contextSpec - The isolated server context. + * @param input - The `GetUrlInputKey` object. + * @returns Presigned URL and timestamp when the URL MAY expire. + * @throws service: `S3Exception` - thrown when checking for existence of the object + * @throws validation: `StorageValidationErrorCode` - Validation errors + * thrown either username or key are not defined. + * + */ + ( + contextSpec: AmplifyServer.ContextSpec, + input: GetUrlInputKey, + ): Promise; +} +export const getUrl: GetUrl = async ( contextSpec: AmplifyServer.ContextSpec, input: GetUrlInput, ): Promise =>