-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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): internal GetProperties API #13869
Changes from all commits
a3ef0af
d4ed4bf
6f1b4bb
e3c517a
0c367c0
7860519
7fe5590
ba41b0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
import { AmplifyClassV6 } from '@aws-amplify/core'; | ||
|
||
import { getProperties as advancedGetProperties } from '../../../src/internals'; | ||
import { getProperties as getPropertiesInternal } from '../../../src/providers/s3/apis/internal/getProperties'; | ||
|
||
jest.mock('../../../src/providers/s3/apis/internal/getProperties'); | ||
const mockedGetPropertiesInternal = jest.mocked(getPropertiesInternal); | ||
|
||
describe('getProperties (internal)', () => { | ||
beforeEach(() => { | ||
mockedGetPropertiesInternal.mockResolvedValue({ | ||
path: 'output/path/to/mock/object', | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('should pass advanced option locationCredentialsProvider to internal getProperties', async () => { | ||
const useAccelerateEndpoint = true; | ||
const bucket = { bucketName: 'bucket', region: 'us-east-1' }; | ||
const locationCredentialsProvider = async () => ({ | ||
credentials: { | ||
accessKeyId: 'akid', | ||
secretAccessKey: 'secret', | ||
sessionToken: 'token', | ||
expiration: new Date(), | ||
}, | ||
}); | ||
const result = await advancedGetProperties({ | ||
path: 'input/path/to/mock/object', | ||
options: { | ||
useAccelerateEndpoint, | ||
bucket, | ||
locationCredentialsProvider, | ||
}, | ||
}); | ||
expect(mockedGetPropertiesInternal).toHaveBeenCalledTimes(1); | ||
expect(mockedGetPropertiesInternal).toHaveBeenCalledWith( | ||
expect.any(AmplifyClassV6), | ||
{ | ||
path: 'input/path/to/mock/object', | ||
options: { | ||
useAccelerateEndpoint, | ||
bucket, | ||
locationCredentialsProvider, | ||
}, | ||
}, | ||
); | ||
expect(result).toEqual({ | ||
path: 'output/path/to/mock/object', | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,33 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Amplify } from '@aws-amplify/core'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { getProperties as getPropertiesInternal } from '../../providers/s3/apis/internal/getProperties'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { GetPropertiesInput } from '../types/inputs'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { GetPropertiesOutput } from '../types/outputs'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* 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 `GetPropertiesWithPathInput` object. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @returns Requested object properties. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @throws An `S3Exception` when the underlying S3 service returned error. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @throws A `StorageValidationErrorCode` when API call parameters are invalid. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @internal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export function getProperties( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
input: GetPropertiesInput, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
): Promise<GetPropertiesOutput> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return getPropertiesInternal(Amplify, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path: input.path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
options: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useAccelerateEndpoint: input?.options?.useAccelerateEndpoint, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bucket: input?.options?.bucket, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
locationCredentialsProvider: input?.options?.locationCredentialsProvider, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ashika112 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Type casting is necessary because `getPropertiesInternal` supports both Gen1 and Gen2 signatures, but here | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// given in input can only be Gen2 signature, the return can only ben Gen2 signature. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) as Promise<GetPropertiesOutput>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+20
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. super nit,
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,16 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import { | ||
StorageCopyInputWithPath, | ||
StorageOperationInputWithPath, | ||
StorageOperationOptionsInput, | ||
} from '../../types/inputs'; | ||
import { | ||
CopyWithPathInput, | ||
GetPropertiesWithPathInput, | ||
} from '../../providers/s3'; | ||
|
||
import { CredentialsProvider, ListLocationsInput } from './credentials'; | ||
import { Permission, PrefixType, Privilege } from './common'; | ||
|
||
|
@@ -26,3 +36,49 @@ export interface GetDataAccessInput { | |
region: string; | ||
scope: string; | ||
} | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export type GetPropertiesInput = ExtendInputWithAdvancedOptions< | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im confused here. Why are we creating this and not using this type? but rather have a duplicate again here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, we should keep types as simple as we can, and avoid type programming as much as we can 😅 when we deprecating the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ashika112 I think I was debating all the different conventions, |
||
GetPropertiesWithPathInput, | ||
{ | ||
locationCredentialsProvider?: CredentialsProvider; | ||
} | ||
>; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export type CopyInput = ExtendCopyInputWithAdvancedOptions< | ||
CopyWithPathInput, | ||
{ | ||
locationCredentialsProvider?: CredentialsProvider; | ||
} | ||
>; | ||
|
||
/** | ||
* Generic types that extend the public non-copy API input types with extended | ||
* options. This is a temporary solution to support advanced options from internal APIs. | ||
*/ | ||
type ExtendInputWithAdvancedOptions<InputType, ExtendedOptionsType> = | ||
InputType extends StorageOperationInputWithPath & | ||
StorageOperationOptionsInput<infer PublicInputOptionsType> | ||
? { | ||
path: InputType['path']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about key? Not supporting it here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, we only support Gen2 signature with extended option. |
||
options?: PublicInputOptionsType & ExtendedOptionsType; | ||
} | ||
: never; | ||
|
||
/** | ||
* Generic types that extend the public copy API input type with extended options. | ||
* This is a temporary solution to support advanced options from internal APIs. | ||
*/ | ||
type ExtendCopyInputWithAdvancedOptions<InputType, ExtendedOptionsType> = | ||
InputType extends StorageCopyInputWithPath | ||
? { | ||
source: InputType['source']; | ||
destination: InputType['destination']; | ||
options?: ExtendedOptionsType; | ||
} | ||
: never; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is getting confusing 😅