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

feat(storage): list API to accept both prefix and path #13100

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
dde5049
updating type info
ashika112 Feb 27, 2024
0b2b2c8
update downloadData implementation
ashika112 Feb 27, 2024
e33051c
fix output type
ashika112 Feb 27, 2024
b585605
path dynamic construction
ashika112 Feb 29, 2024
53befdb
downloadData gen2
ashika112 Mar 1, 2024
482551d
Merge branch 'gen2-storage' of github.com:aws-amplify/amplify-js into…
ashika112 Mar 1, 2024
f33bfd8
resolve merge conflit
ashika112 Mar 1, 2024
610c830
cleanup after merge
ashika112 Mar 4, 2024
509db01
fix failing test
ashika112 Mar 4, 2024
5d9b10e
add unit tests for path
ashika112 Mar 5, 2024
b80c0b0
add deprecation warning
ashika112 Mar 5, 2024
2b90493
update bundle size
ashika112 Mar 5, 2024
51ea16b
update deprecation warning
ashika112 Mar 5, 2024
2cd55fd
remove unwated assertions
ashika112 Mar 5, 2024
fbf1297
updae test description
ashika112 Mar 5, 2024
bb9d156
misc changes
ashika112 Mar 5, 2024
d962ff1
integ test update
ashika112 Mar 6, 2024
00341ae
update integ
ashika112 Mar 6, 2024
08e9b2f
wip: initial commit with direct path
Samaritan1011001 Mar 1, 2024
9aa3c43
chore: minor lint fixes
Samaritan1011001 Mar 5, 2024
cd0a947
feat: list api with prefix and path input
Samaritan1011001 Mar 9, 2024
cf1cedd
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 11, 2024
eab94c6
chore: unit test for path cases and input validation update
Samaritan1011001 Mar 12, 2024
245223a
chore: update comments on types
Samaritan1011001 Mar 12, 2024
a1fde78
chore: slightly increase bundle size limit
Samaritan1011001 Mar 12, 2024
efe6736
chore: revert the expose list API to original format
Samaritan1011001 Mar 13, 2024
3c5e7c8
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 13, 2024
0c976e5
feat: refactor list to have less branches
Samaritan1011001 Mar 14, 2024
eb7fb98
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 14, 2024
444f8d7
wip: clean up list api reduce complexity
Samaritan1011001 Mar 15, 2024
9002a0f
chore: revert outward type changes and reduce list complexity
Samaritan1011001 Mar 15, 2024
4a3664a
chore: validation function tests
Samaritan1011001 Mar 18, 2024
f2b12f5
chore: minor type updates
Samaritan1011001 Mar 18, 2024
f9b35ef
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 19, 2024
d104c30
chore: comment a test for leading slash
Samaritan1011001 Mar 19, 2024
a752ab7
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 20, 2024
d21709a
chore: enable integ test
Samaritan1011001 Mar 20, 2024
d2cce19
Revert "chore: enable integ test"
Samaritan1011001 Mar 20, 2024
19886dd
chore: transfered review comments from remove pr and made path expect…
Samaritan1011001 Mar 21, 2024
06c0175
feat: assert when path has a leading slash
Samaritan1011001 Mar 21, 2024
8d7eb1e
wip: verdaccio
Samaritan1011001 Mar 21, 2024
85ca6cf
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 22, 2024
cb94be9
chore: docstring update
Samaritan1011001 Mar 22, 2024
9953848
chore: review comments
Samaritan1011001 Mar 22, 2024
40f391b
chore: updated test it.each
Samaritan1011001 Mar 22, 2024
85da851
chore: update function name
Samaritan1011001 Mar 22, 2024
64c39cb
chore: rename internal type
Samaritan1011001 Mar 22, 2024
34ad9f6
chore: nits
Samaritan1011001 Mar 22, 2024
cfed633
chore: nits on if/else and depcreation message
Samaritan1011001 Mar 25, 2024
b2db90c
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 25, 2024
0c4c191
fix: validation to not allow both path and prefix in input
Samaritan1011001 Mar 25, 2024
857a1b7
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 25, 2024
6da14f1
chore; bundle size increase
Samaritan1011001 Mar 25, 2024
308ffd7
chore: update test with new error message
Samaritan1011001 Mar 25, 2024
53358da
chore: increase bundle size
Samaritan1011001 Mar 25, 2024
e37c3e9
chore: increase bundle size
Samaritan1011001 Mar 25, 2024
e746180
feat: update path prefix validation
Samaritan1011001 Mar 25, 2024
7dd29eb
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 25, 2024
b356d4b
chore: remove unwanted test for path empty string
Samaritan1011001 Mar 26, 2024
0466441
Merge branch 'gen2-storage' into feat/list-remove-gen2
Samaritan1011001 Mar 26, 2024
6b1a164
chore: adjust bunle size limit
Samaritan1011001 Mar 26, 2024
9a827a7
chore: update error message
Samaritan1011001 Mar 26, 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
8 changes: 4 additions & 4 deletions packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -473,19 +473,19 @@
"name": "[Storage] getProperties (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ getProperties }",
"limit": "13.53 kB"
"limit": "13.55 kB"
},
{
"name": "[Storage] getUrl (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ getUrl }",
"limit": "14.61 kB"
"limit": "14.63 kB"
},
{
"name": "[Storage] list (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ list }",
"limit": "14.00 kB"
"limit": "14.2 kB"
},
{
"name": "[Storage] remove (S3)",
Expand All @@ -497,7 +497,7 @@
"name": "[Storage] uploadData (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ uploadData }",
"limit": "18.50 kB"
"limit": "18.52 kB"
}
]
}
229 changes: 186 additions & 43 deletions packages/storage/__tests__/providers/s3/apis/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { Amplify } from '@aws-amplify/core';
import { listObjectsV2 } from '../../../../src/providers/s3/utils/client';
import { list } from '../../../../src/providers/s3';
import {
ListAllOptions,
ListPaginateOptions,
ListAllOptionsPrefix,
ListPaginateOptionsPrefix,
} from '../../../../src/providers/s3/types';
import { StorageValidationErrorCode } from '../../../../src/errors/types/validation';

jest.mock('../../../../src/providers/s3/utils/client');
jest.mock('@aws-amplify/core', () => ({
Expand All @@ -26,7 +27,6 @@ const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock;
const mockGetConfig = Amplify.getConfig as jest.Mock;
const mockListObject = listObjectsV2 as jest.Mock;
const key = 'path/itemsKey';
const path = key;
const bucket = 'bucket';
const region = 'region';
const nextToken = 'nextToken';
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('list API', () => {
},
});
});
describe('Happy Cases:', () => {
describe('Prefix: Happy Cases:', () => {
afterEach(() => {
jest.clearAllMocks();
});
Expand All @@ -99,33 +99,37 @@ describe('list API', () => {
expectedPath: `public/`,
},
{
path,
expectedPath: `public/${path}`,
options: { accessLevel: 'guest' },
expectedPath: `public/`,
},
{
key,
expectedPath: `public/${key}`,
},
{
path,
key,
options: { accessLevel: 'guest' },
expectedPath: `public/${path}`,
expectedPath: `public/${key}`,
},
{
path,
key,
options: { accessLevel: 'private' },
expectedPath: `private/${defaultIdentityId}/${path}`,
expectedPath: `private/${defaultIdentityId}/${key}`,
},
{
path,
key,
options: { accessLevel: 'protected' },
expectedPath: `protected/${defaultIdentityId}/${path}`,
expectedPath: `protected/${defaultIdentityId}/${key}`,
},
{
path,
key,
options: { accessLevel: 'protected', targetIdentityId },
expectedPath: `protected/${targetIdentityId}/${path}`,
expectedPath: `protected/${targetIdentityId}/${key}`,
},
];

accessLevelTests.forEach(({ path, options, expectedPath }) => {
const pathMsg = path ? 'custom' : 'default';
accessLevelTests.forEach(({ key, options, expectedPath }) => {
const pathMsg = key ? 'custom' : 'default';
const accessLevelMsg = options?.accessLevel ?? 'default';
const targetIdentityIdMsg = options?.targetIdentityId
? `with targetIdentityId`
Expand All @@ -139,14 +143,11 @@ describe('list API', () => {
NextContinuationToken: nextToken,
};
});
expect.assertions(4);
let response = await list({
prefix: path,
options: options as ListPaginateOptions,
prefix: key,
options: options as ListPaginateOptionsPrefix,
});
expect(response.items).toEqual([
{ ...listResultItem, key: path ?? '' },
]);
expect(response.items).toEqual([{ ...listResultItem, key: key ?? '' }]);
expect(response.nextToken).toEqual(nextToken);
expect(listObjectsV2).toHaveBeenCalledTimes(1);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Expand All @@ -157,8 +158,8 @@ describe('list API', () => {
});
});

accessLevelTests.forEach(({ path, options, expectedPath }) => {
const pathMsg = path ? 'custom' : 'default';
accessLevelTests.forEach(({ key, options, expectedPath }) => {
const pathMsg = key ? 'custom' : 'default';
const accessLevelMsg = options?.accessLevel ?? 'default';
const targetIdentityIdMsg = options?.targetIdentityId
? `with targetIdentityId`
Expand All @@ -172,19 +173,16 @@ describe('list API', () => {
NextContinuationToken: nextToken,
};
});
expect.assertions(4);
const customPageSize = 5;
const response = await list({
prefix: path,
prefix: key,
options: {
...(options as ListPaginateOptions),
...(options as ListPaginateOptionsPrefix),
pageSize: customPageSize,
nextToken: nextToken,
},
});
expect(response.items).toEqual([
{ ...listResultItem, key: path ?? '' },
]);
expect(response.items).toEqual([{ ...listResultItem, key: key ?? '' }]);
expect(response.nextToken).toEqual(nextToken);
expect(listObjectsV2).toHaveBeenCalledTimes(1);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Expand All @@ -196,8 +194,8 @@ describe('list API', () => {
});
});

accessLevelTests.forEach(({ path, options, expectedPath }) => {
const pathMsg = path ? 'custom' : 'default';
accessLevelTests.forEach(({ key, options, expectedPath }) => {
const pathMsg = key ? 'custom' : 'default';
const accessLevelMsg = options?.accessLevel ?? 'default';
const targetIdentityIdMsg = options?.targetIdentityId
? `with targetIdentityId`
Expand All @@ -206,13 +204,12 @@ describe('list API', () => {
mockListObject.mockImplementationOnce(() => {
return {};
});
expect.assertions(3);
let response = await list({
prefix: path,
options: options as ListPaginateOptions,
prefix: key,
options: options as ListPaginateOptionsPrefix,
});
expect(response.items).toEqual([]);
//

expect(response.nextToken).toEqual(undefined);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Bucket: bucket,
Expand All @@ -222,21 +219,20 @@ describe('list API', () => {
});
});

accessLevelTests.forEach(({ path, options, expectedPath }) => {
const pathMsg = path ? 'custom' : 'default';
accessLevelTests.forEach(({ key, options, expectedPath }) => {
const pathMsg = key ? 'custom' : 'default';
const accessLevelMsg = options?.accessLevel ?? 'default';
const targetIdentityIdMsg = options?.targetIdentityId
? `with targetIdentityId`
: '';
it(`should list all objects having three pages with ${pathMsg} path, ${accessLevelMsg} accessLevel ${targetIdentityIdMsg}`, async () => {
expect.assertions(5);
mockListObjectsV2ApiWithPages(3);
const result = await list({
prefix: path,
options: { ...options, listAll: true } as ListAllOptions,
prefix: key,
options: { ...options, listAll: true } as ListAllOptionsPrefix,
});

const listResult = { ...listResultItem, key: path ?? '' };
const listResult = { ...listResultItem, key: key ?? '' };
expect(result.items).toEqual([listResult, listResult, listResult]);
expect(result).not.toHaveProperty(nextToken);

Expand Down Expand Up @@ -269,6 +265,153 @@ describe('list API', () => {
});
});

describe('Path: Happy Cases:', () => {
const resolvePath = (path: string | Function) =>
typeof path === 'string' ? path : path({ identityId: defaultIdentityId });
afterEach(() => {
jest.clearAllMocks();
mockListObject.mockClear();
});
const pathAsFunctionAndStringTests = [
{
path: `public/${key}`,
},
{
path: ({ identityId }: any) => `protected/${identityId}/${key}`,
},
ashwinkumar6 marked this conversation as resolved.
Show resolved Hide resolved
];

it.each(pathAsFunctionAndStringTests)(
'should list objects with pagination, default pageSize, custom path',
async ({ path }) => {
mockListObject.mockImplementationOnce(() => {
return {
Contents: [
{
...listObjectClientBaseResultItem,
Key: resolvePath(path),
},
],
NextContinuationToken: nextToken,
};
});
let response = await list({
path,
});
expect(response.items).toEqual([
{ ...listResultItem, path: resolvePath(path) },
]);
expect(response.nextToken).toEqual(nextToken);
expect(listObjectsV2).toHaveBeenCalledTimes(1);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Bucket: bucket,
MaxKeys: 1000,
Prefix: resolvePath(path),
});
},
);

it.each(pathAsFunctionAndStringTests)(
'should list objects with pagination using custom pageSize, nextToken and custom path: ${path}',
async ({ path }) => {
mockListObject.mockImplementationOnce(() => {
return {
Contents: [
{
...listObjectClientBaseResultItem,
Key: resolvePath(path),
},
],
NextContinuationToken: nextToken,
};
});
const customPageSize = 5;
const response = await list({
path,
options: {
pageSize: customPageSize,
nextToken: nextToken,
},
});
expect(response.items).toEqual([
{ ...listResultItem, path: resolvePath(path) ?? '' },
]);
expect(response.nextToken).toEqual(nextToken);
expect(listObjectsV2).toHaveBeenCalledTimes(1);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Bucket: bucket,
Prefix: resolvePath(path),
ContinuationToken: nextToken,
MaxKeys: customPageSize,
});
},
);

it.each(pathAsFunctionAndStringTests)(
'should list objects with zero results with custom path: ${path}',
async ({ path }) => {
mockListObject.mockImplementationOnce(() => {
return {};
});
let response = await list({
path,
});
expect(response.items).toEqual([]);

expect(response.nextToken).toEqual(undefined);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Bucket: bucket,
MaxKeys: 1000,
Prefix: resolvePath(path),
});
},
);

it.each(pathAsFunctionAndStringTests)(
'should list all objects having three pages with custom path: ${path}',
async ({ path }) => {
mockListObjectsV2ApiWithPages(3);
const result = await list({
path,
options: { listAll: true },
});

const listResult = {
...listResultItem,
path: resolvePath(path),
};
expect(result.items).toEqual([listResult, listResult, listResult]);
expect(result).not.toHaveProperty(nextToken);

// listing three times for three pages
expect(listObjectsV2).toHaveBeenCalledTimes(3);

// first input recieves undefined as the Continuation Token
expect(listObjectsV2).toHaveBeenNthCalledWith(
1,
listObjectClientConfig,
{
Bucket: bucket,
Prefix: resolvePath(path),
MaxKeys: 1000,
ContinuationToken: undefined,
},
);
// last input recieves TEST_TOKEN as the Continuation Token
expect(listObjectsV2).toHaveBeenNthCalledWith(
3,
listObjectClientConfig,
{
Bucket: bucket,
Prefix: resolvePath(path),
MaxKeys: 1000,
ContinuationToken: nextToken,
},
);
},
);
});

describe('Error Cases:', () => {
afterEach(() => {
jest.clearAllMocks();
Expand All @@ -280,10 +423,10 @@ describe('list API', () => {
name: 'NotFound',
}),
);
expect.assertions(3);
try {
await list({});
} catch (error: any) {
expect.assertions(3);
expect(listObjectsV2).toHaveBeenCalledTimes(1);
expect(listObjectsV2).toHaveBeenCalledWith(listObjectClientConfig, {
Bucket: bucket,
Expand Down
Loading
Loading