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.

Sign up for GitHub

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

Update Path assertion #13160

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Update Path assertion #13160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6195d2d
update validation on path assertion
ashika112 Mar 21, 2024
23dfcff
Update test message
ashika112 Mar 21, 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/storage/__tests__/providers/s3/apis/copy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,15 @@ describe('copy API', () => {

test.each([
{
sourcePath: '/sourcePathAsString',
sourcePath: 'sourcePathAsString',
expectedSourcePath: 'sourcePathAsString',
ashika112 marked this conversation as resolved.
Show resolved Hide resolved
destinationPath: '/destinationPathAsString',
destinationPath: 'destinationPathAsString',
expectedDestinationPath: 'destinationPathAsString',
},
{
sourcePath: () => '/sourcePathAsFunction',
sourcePath: () => 'sourcePathAsFunction',
expectedSourcePath: 'sourcePathAsFunction',
destinationPath: () => '/destinationPathAsFunction',
destinationPath: () => 'destinationPathAsFunction',
expectedDestinationPath: 'destinationPathAsFunction',
},
])(
Expand Down
4 changes: 2 additions & 2 deletions packages/storage/__tests__/providers/s3/apis/downloadData.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,11 @@ describe('downloadData with path', () => {

test.each([
{
path: '/path',
path: 'path',
expectedKey: 'path',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have expectedPath, can just have the path in this object

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for readability and consistency.

},
{
path: () => '/path',
path: () => 'path',
expectedKey: 'path',
},
])(
Expand Down
8 changes: 4 additions & 4 deletions packages/storage/__tests__/providers/s3/apis/utils/validateStorageOperationInput.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {

describe('validateStorageOperationInput', () => {
it('should return inputType as STORAGE_INPUT_PATH and objectKey as testPath when input is path as string', () => {
const input = { path: '/testPath' };
const input = { path: 'testPath' };
const result = validateStorageOperationInput(input);
expect(result).toEqual({
inputType: STORAGE_INPUT_PATH,
Expand All @@ -24,7 +24,7 @@ describe('validateStorageOperationInput', () => {
it('should return inputType as STORAGE_INPUT_PATH and objectKey as result of path function when input is path as function', () => {
const input = {
path: ({ identityId }: { identityId?: string }) =>
`/testPath/${identityId}`,
`testPath/${identityId}`,
};
const result = validateStorageOperationInput(input, '123');
expect(result).toEqual({
Expand All @@ -42,8 +42,8 @@ describe('validateStorageOperationInput', () => {
});
});

it('should throw an error when input path does not start with a /', () => {
const input = { path: 'test' } as any;
it('should throw an error when input path starts with a /', () => {
const input = { path: '/leading-slash-path' };
ashika112 marked this conversation as resolved.
Show resolved Hide resolved
expect(() => validateStorageOperationInput(input)).toThrow(
validationErrorMap[
StorageValidationErrorCode.InvalidStoragePathInput
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/errors/types/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ export const validationErrorMap: AmplifyErrorMap<StorageValidationErrorCode> = {
message: 'Missing path or key parameter in Input.',
},
[StorageValidationErrorCode.InvalidStoragePathInput]: {
message: 'Input `path` is missing a leading slash (/).',
message: 'Input `path` does not allow a leading slash (/).',
},
};
12 changes: 10 additions & 2 deletions packages/storage/src/providers/s3/types/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,16 @@ interface CommonOptions {

/** @deprecated This may be removed in the next major version. */
type ReadOptions =
| { accessLevel?: 'guest' | 'private' }
| { accessLevel: 'protected'; targetIdentityId?: string };
| {
/** @deprecated This may be removed in the next major version. */
accessLevel?: 'guest' | 'private';
}
| {
/** @deprecated This may be removed in the next major version. */
accessLevel: 'protected';
/** @deprecated This may be removed in the next major version. */
targetIdentityId?: string;
};

/** @deprecated This may be removed in the next major version. */
interface WriteOptions {
Expand Down
4 changes: 2 additions & 2 deletions packages/storage/src/providers/s3/utils/validateStorageOperationInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ export const validateStorageOperationInput = (
const { path } = input;
const objectKey = typeof path === 'string' ? path : path({ identityId });
assertValidationError(
objectKey.startsWith('/'),
!objectKey.startsWith('/'),
StorageValidationErrorCode.InvalidStoragePathInput,
);

return {
inputType: STORAGE_INPUT_PATH,
objectKey: objectKey.slice(1),
objectKey,
};
} else {
return { inputType: STORAGE_INPUT_KEY, objectKey: input.key };
Expand Down
Loading