Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(storage): add foundation deleteObject client #13795

Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1d15dd0
chore: merge changes from storage-browser/main to storage-browser/int…
AllanZhengYP Jul 25, 2024
5758c09
chore: merge changes from storage-browser/main to storage-browser/int…
AllanZhengYP Aug 22, 2024
001263d
chore(storage): add durability check for urls in put object (#13746)
scyrizales-amz Aug 28, 2024
328f0b2
feat(storage): add foundation DeleteObject client
Sep 13, 2024
a3be1b4
code cleanup
Sep 13, 2024
512cd94
add additional unit tests
Sep 17, 2024
4b2ee57
refactor: remove factories/serviceClients/s3/utils dir
Sep 17, 2024
024857b
refactor: move runtime platform specific utils to DI dir with TODO
Sep 17, 2024
9aa87b8
chore(CreateDeleteObjectDeserializer): expect deserializer to return …
Sep 18, 2024
2609808
fix: runtime clients for different platforms
Sep 19, 2024
f0acfc2
chore: temporarily reduce test coverage threshold
Sep 19, 2024
9e015a8
refactor: breakup serializeHelpers and deserializeHelpers into indivi…
Sep 23, 2024
dcbabd5
refactor dir structure
Sep 24, 2024
01ab4a6
code cleanup
Sep 24, 2024
6f85da8
chore: move s3data/shared/serde/ to s3data/serde/
Sep 25, 2024
59b0679
chore: rename shared/serde to shared/serdeUtils
Sep 26, 2024
989aee4
add todo to use rn base64 encoder
Sep 26, 2024
ee117c1
core: update dts-bundler.config.js as per new dir
Sep 26, 2024
30e5484
Merge branch 'storage-browser/foundation' into storage-browser/founda…
ashwinkumar6 Sep 27, 2024
a64f125
chore: increase bundle size
Sep 27, 2024
4c02ecf
update TODO message
Sep 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@
"name": "[Storage] uploadData (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ uploadData }",
"limit": "21.68 kB"
"limit": "21.83 kB"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { composeServiceApi } from '@aws-amplify/core/internals/aws-client-utils/composers';

import * as serviceClients from '../../../../../src/foundation/factories/serviceClients';
import { DEFAULT_SERVICE_CLIENT_API_CONFIG } from '../../../../../src/foundation/factories/serviceClients/s3data/constants';

jest.mock('@aws-amplify/core/internals/aws-client-utils/composers', () => ({
...jest.requireActual(
'@aws-amplify/core/internals/aws-client-utils/composers',
),
composeServiceApi: jest.fn(),
}));

export const mockComposeServiceApi = jest.mocked(composeServiceApi);

describe('service clients', () => {
const mockClient = jest.fn();
const serviceClientFactories = Object.keys(
serviceClients,
) as (keyof typeof serviceClients)[];

beforeEach(() => {
mockComposeServiceApi.mockImplementation(() => {
return mockClient;
});
});

afterEach(() => {
mockComposeServiceApi.mockClear();
mockClient.mockClear();
});

it.each(serviceClientFactories)(
'factory `%s` should invoke composeServiceApi with expected parameters',
serviceClientFactory => {
// eslint-disable-next-line import/namespace
const createClient = serviceClients[serviceClientFactory];
const client = createClient();
expect(client).toBe(mockClient);

expect(mockComposeServiceApi).toHaveBeenCalledWith(
expect.any(Function),
expect.any(Function),
expect.any(Function),
expect.objectContaining(DEFAULT_SERVICE_CLIENT_API_CONFIG),
);
},
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { AmplifyUrl } from '@aws-amplify/core/internals/utils';

// ../../src/foundation/factories/serviceClients/s3data/endpointResolver
import { endpointResolver } from '../../../../../src/foundation/factories/serviceClients/s3data/endpointResolver';
import { SERVICE_NAME } from '../../../../../src/foundation/constants';

const region = 'us-west-2';

describe('endpointResolver', () => {
it('should return default base endpoint', async () => {
const { url } = endpointResolver({ region });

expect(url instanceof AmplifyUrl).toBe(true);
expect(url.toString()).toStrictEqual(
`https://${SERVICE_NAME}.${region}.amazonaws.com/`,
);
});

it('should return custom endpoint', async () => {
const customEndpoint = 'http://test.com/';
const { url } = endpointResolver({ region, customEndpoint });

expect(url instanceof AmplifyUrl).toBe(true);
expect(url.toString()).toStrictEqual(`${customEndpoint}`);
});

it('should return accelerate endpoint', async () => {
const { url } = endpointResolver({ region, useAccelerateEndpoint: true });

expect(url instanceof AmplifyUrl).toBe(true);
expect(url.toString()).toStrictEqual(
`https://${SERVICE_NAME}-accelerate.amazonaws.com/`,
);
});

it('should return endpoint with bucket name', async () => {
const bucketName = 'mybucket';
const { url } = endpointResolver({ region }, { Bucket: bucketName });

expect(url instanceof AmplifyUrl).toBe(true);
expect(url.toString()).toStrictEqual(
`https://${bucketName}.${SERVICE_NAME}.${region}.amazonaws.com/`,
);
});

it('should return endpoint with bucket name with forcePathStyle enabled', async () => {
const bucketName = 'mybucket';
const { url } = endpointResolver(
{ region, forcePathStyle: true },
{ Bucket: bucketName },
);

expect(url instanceof AmplifyUrl).toBe(true);
expect(url.toString()).toStrictEqual(
`https://${SERVICE_NAME}.${region}.amazonaws.com/${bucketName}`,
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { HttpResponse } from '@aws-amplify/core/internals/aws-client-utils';
import * as clientUtils from '@aws-amplify/core/internals/aws-client-utils';

import { createDeleteObjectDeserializer } from '../../../../../../../src/foundation/factories/serviceClients/s3data/shared/serde';
import { StorageError } from '../../../../../../../src/errors/StorageError';

describe('createDeleteObjectDeserializer', () => {
const deserializer = createDeleteObjectDeserializer();

it('returns body for 2xx status code', async () => {
const response: HttpResponse = {
statusCode: 200,
headers: {
'x-amz-id-2': 'requestId2',
'x-amz-request-id': 'requestId',
},
body: {
json: () => Promise.resolve({}),
blob: () => Promise.resolve(new Blob()),
text: () => Promise.resolve(''),
},
};
const output = await deserializer(response);

expect(output).toEqual(
expect.objectContaining({
$metadata: {
requestId: response.headers['x-amz-request-id'],
extendedRequestId: response.headers['x-amz-id-2'],
httpStatusCode: 200,
},
}),
);
});

it('throws StorageError for 4xx status code', async () => {
const expectedErrorName = 'TestError';
const expectedErrorMessage = '400';
const expectedError = new Error(expectedErrorMessage);
expectedError.name = expectedErrorName;

jest
.spyOn(clientUtils, 'parseJsonError')
.mockReturnValueOnce(expectedError as any);

const response: HttpResponse = {
statusCode: 400,
body: {
json: () => Promise.resolve({}),
blob: () => Promise.resolve(new Blob()),
text: () => Promise.resolve(''),
},
headers: {},
};

expect(deserializer(response as any)).rejects.toThrow(
new StorageError({
name: expectedErrorName,
message: expectedErrorMessage,
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { AmplifyUrl } from '@aws-amplify/core/internals/utils';

import { createDeleteObjectSerializer } from '../../../../../../../src/foundation/factories/serviceClients/s3data/shared/serde';

describe('createDeleteObjectSerializer', () => {
it('should serialize deleteObject request', async () => {
const input = { Bucket: 'bucket', Key: 'myKey' };
const endPointUrl = 'http://test.com';
const endpoint = { url: new AmplifyUrl(endPointUrl) };

const serializer = createDeleteObjectSerializer();
const result = serializer(input, endpoint);

expect(result).toEqual({
method: 'DELETE',
headers: {},
url: expect.objectContaining({
href: `${endPointUrl}/${input.Key}`,
}),
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { isDnsCompatibleBucketName } from '../../../../../../src/foundation/factories/serviceClients/s3data/validators/isDnsCompatibleBucketName';

describe('isDnsCompatibleBucketName', () => {
it.each([
'valid-bucket-name',
'a.bucket.name',
'bucket123',
'123invalid-start',
'bucket--name',
])('should return true for dns compatible bucket name: %s', bucketName => {
expect(isDnsCompatibleBucketName(bucketName)).toBe(true);
});

it.each([
'', // Empty string
'fo', // too short
'bucketnamewithtoolongcharactername'.repeat(5), // Name longer than 63 characters
'invalid.bucket..name', // consecutive dots
'bucket_name_with_underscores', // contains underscores
'.bucketwithleadingperiod', // leading period
'.bucketwithtrailingperiod', // trailing period
'bucketCapitalLetters', // capital letters
'bucketnameendswith-', // ends with a hyphen
'255.255.255.255', // IP address
'::1', // IPv6 address
])('should return false for dns incompatible bucket name: %s', bucketName => {
expect(isDnsCompatibleBucketName(bucketName)).toBe(false);
});
});
39 changes: 22 additions & 17 deletions packages/storage/__tests__/providers/s3/apis/remove.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { AWSCredentials } from '@aws-amplify/core/internals/utils';
import { Amplify, StorageAccessLevel } from '@aws-amplify/core';

import { deleteObject } from '../../../../src/providers/s3/utils/client/s3data';
import { createDeleteObjectClient } from '../../../../src/foundation/factories/serviceClients';
import { remove } from '../../../../src/providers/s3/apis';
import { StorageValidationErrorCode } from '../../../../src/errors/types/validation';
import {
Expand All @@ -15,7 +15,7 @@ import {
} from '../../../../src/providers/s3/types';
import './testUtils';

jest.mock('../../../../src/providers/s3/utils/client/s3data');
jest.mock('../../../../src/foundation/factories/serviceClients');
jest.mock('@aws-amplify/core', () => ({
ConsoleLogger: jest.fn().mockImplementation(function ConsoleLogger() {
return { debug: jest.fn() };
Expand All @@ -27,7 +27,8 @@ jest.mock('@aws-amplify/core', () => ({
},
},
}));
const mockDeleteObject = deleteObject as jest.Mock;
const mockDeleteObject = jest.fn();
const mockCreateDeleteObjectClient = jest.mocked(createDeleteObjectClient);
const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock;
const mockGetConfig = jest.mocked(Amplify.getConfig);
const inputKey = 'key';
Expand Down Expand Up @@ -72,6 +73,7 @@ describe('remove API', () => {
Metadata: { key: 'value' },
};
});
mockCreateDeleteObjectClient.mockReturnValueOnce(mockDeleteObject);
});
afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -106,8 +108,8 @@ describe('remove API', () => {
options,
});
expect(key).toEqual(inputKey);
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
deleteObjectClientConfig,
{
Bucket: bucket,
Expand All @@ -127,8 +129,8 @@ describe('remove API', () => {
bucket: { bucketName: mockBucketName, region: mockRegion },
},
});
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region: mockRegion,
Expand All @@ -147,8 +149,8 @@ describe('remove API', () => {
bucket: 'default-bucket',
},
});
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
Expand All @@ -172,6 +174,7 @@ describe('remove API', () => {
Metadata: { key: 'value' },
};
});
mockCreateDeleteObjectClient.mockReturnValueOnce(mockDeleteObject);
});
afterEach(() => {
jest.clearAllMocks();
Expand All @@ -193,8 +196,8 @@ describe('remove API', () => {
it(`should remove object for the given path`, async () => {
const { path } = await removeWrapper({ path: inputPath });
expect(path).toEqual(resolvedPath);
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
deleteObjectClientConfig,
{
Bucket: bucket,
Expand All @@ -214,8 +217,8 @@ describe('remove API', () => {
bucket: { bucketName: mockBucketName, region: mockRegion },
},
});
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region: mockRegion,
Expand All @@ -234,8 +237,8 @@ describe('remove API', () => {
bucket: 'default-bucket',
},
});
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
Expand All @@ -262,13 +265,15 @@ describe('remove API', () => {
name: 'NotFound',
}),
);
mockCreateDeleteObjectClient.mockReturnValueOnce(mockDeleteObject);

expect.assertions(3);
const key = 'wrongKey';
try {
await remove({ key });
} catch (error: any) {
expect(deleteObject).toHaveBeenCalledTimes(1);
await expect(deleteObject).toBeLastCalledWithConfigAndInput(
expect(mockDeleteObject).toHaveBeenCalledTimes(1);
await expect(mockDeleteObject).toBeLastCalledWithConfigAndInput(
deleteObjectClientConfig,
{
Bucket: bucket,
Expand Down
Loading
Loading