-
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): add new internal downloadData api #13887
Changes from all commits
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,76 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import { downloadData as advancedDownloadData } from '../../../src/internals'; | ||
import { downloadData as downloadDataInternal } from '../../../src/providers/s3/apis/internal/downloadData'; | ||
|
||
jest.mock('../../../src/providers/s3/apis/internal/downloadData'); | ||
const mockedDownloadDataInternal = jest.mocked(downloadDataInternal); | ||
|
||
describe('downloadData (internal)', () => { | ||
beforeEach(() => { | ||
mockedDownloadDataInternal.mockReturnValue({ | ||
result: Promise.resolve({ | ||
path: 'output/path/to/mock/object', | ||
body: { | ||
blob: () => Promise.resolve(new Blob()), | ||
json: () => Promise.resolve(''), | ||
text: () => Promise.resolve(''), | ||
}, | ||
}), | ||
cancel: jest.fn(), | ||
state: 'SUCCESS', | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('should pass advanced option locationCredentialsProvider to internal downloadData', 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 onProgress = jest.fn(); | ||
const bytesRange = { start: 1024, end: 2048 }; | ||
|
||
const output = await advancedDownloadData({ | ||
path: 'input/path/to/mock/object', | ||
options: { | ||
useAccelerateEndpoint, | ||
bucket, | ||
locationCredentialsProvider, | ||
onProgress, | ||
bytesRange, | ||
}, | ||
}); | ||
|
||
expect(mockedDownloadDataInternal).toHaveBeenCalledTimes(1); | ||
expect(mockedDownloadDataInternal).toHaveBeenCalledWith({ | ||
path: 'input/path/to/mock/object', | ||
options: { | ||
useAccelerateEndpoint, | ||
bucket, | ||
locationCredentialsProvider, | ||
onProgress, | ||
bytesRange, | ||
}, | ||
}); | ||
|
||
expect(await output.result).toEqual({ | ||
path: 'output/path/to/mock/object', | ||
body: { | ||
blob: expect.any(Function), | ||
json: expect.any(Function), | ||
text: expect.any(Function), | ||
}, | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import { downloadData as downloadDataInternal } from '../../providers/s3/apis/internal/downloadData'; | ||
import { DownloadDataInput } from '../types/inputs'; | ||
import { DownloadDataOutput } from '../types/outputs'; | ||
|
||
/** | ||
* Download S3 object data to memory | ||
* | ||
* @param input - The `DownloadDataInput` object. | ||
* @returns A cancelable task exposing result promise from `result` property. | ||
* @throws service: `S3Exception` - thrown when checking for existence of the object | ||
* @throws validation: `StorageValidationErrorCode` - Validation errors | ||
* | ||
* @example | ||
* ```ts | ||
* // Download a file from s3 bucket | ||
* const { body, eTag } = await downloadData({ path, options: { | ||
* onProgress, // Optional progress callback. | ||
* } }).result; | ||
* ``` | ||
* @example | ||
* ```ts | ||
* // Cancel a task | ||
* const downloadTask = downloadData({ path }); | ||
* //... | ||
* downloadTask.cancel(); | ||
* try { | ||
* await downloadTask.result; | ||
* } catch (error) { | ||
* if(isCancelError(error)) { | ||
* // Handle error thrown by task cancelation. | ||
* } | ||
* } | ||
*``` | ||
* | ||
* @internal | ||
*/ | ||
export const downloadData = (input: DownloadDataInput): DownloadDataOutput => | ||
downloadDataInternal({ | ||
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. Why not just pass down the whole input? 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. There was a discussion this is better ATM 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. This prevents JS users reuse the option object and unintentionally include the options like |
||
path: input.path, | ||
options: { | ||
useAccelerateEndpoint: input?.options?.useAccelerateEndpoint, | ||
bucket: input?.options?.bucket, | ||
locationCredentialsProvider: input?.options?.locationCredentialsProvider, | ||
bytesRange: input?.options?.bytesRange, | ||
onProgress: input?.options?.onProgress, | ||
}, | ||
// Type casting is necessary because `downloadDataInternal` supports both Gen1 and Gen2 signatures, but here | ||
// given in input can only be Gen2 signature, the return can only ben Gen2 signature. | ||
}) as DownloadDataOutput; |
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.
Nit: I did this on getUrl, but I wonder if we should remove inline docs (besides the
@internal
annotation) from these internal APIs to dissuade use? Don't need to address now, we can do it all at once in a follow up PR.