[Fix]: Storage Input/Output types #13270
Conversation
This reverts commit a81fc29.
| export function getUrl( | ||
| contextSpec: AmplifyServer.ContextSpec, | ||
| input: GetUrlInputWithPath, | ||
| ): Promise<GetUrlOutputWithPath>; |
There was a problem hiding this comment.
Wondering on getUrl if we should duplicate this for consistency or make it GetUrlOutput for both? Keeping together makes it just this API users in NMV.
| @@ -77,7 +82,14 @@ describe('copy API', () => { | |||
| afterEach(() => { | |||
| jest.clearAllMocks(); | |||
| }); | |||
| [ | |||
| const testCases: Array<{ | |||
| source: { accessLevel?: StorageAccessLevel; targetIdentityId?: string }; | |||
There was a problem hiding this comment.
Note on this: This is how a customer would have to pass in today if it is not done directly on API call. Please let me know if that is not the case since this is the pattern I followed on all APIs
| source: StorageOperationInputWithPath; | ||
| destination: StorageOperationInputWithPath; |
There was a problem hiding this comment.
Should the suffix be Input?
There was a problem hiding this comment.
there are still some internal type refernces WithKey and WithPath we do this for clear separation of concern but we should think about cleaning some of these up.
packages/storage/src/providers/s3/apis/uploadData/multipart/uploadHandlers.ts
Show resolved
Hide resolved
| const copyWrapper = async ( | ||
| input: CopyWithPathInput, | ||
| ): Promise<CopyWithPathOutput> => copy(input); |
There was a problem hiding this comment.
Nit: Possible to hoist this up a few levels to re-use between suites?
There was a problem hiding this comment.
So i purposefully did this so we can have both direct usage and wrapper usage. Meaning This is used in all path happy cases but error cases one will have the direct usage. I mean we want to test both ways
packages/storage/src/providers/s3/apis/uploadData/multipart/uploadHandlers.ts
Show resolved
Hide resolved
|
lgtm, great work !!! 🚀🚀🚀 |
Description of changes
Refactoring the input and output type to export both withKey and withPath types to customers
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.