[BREAKING] Refactor getfilecontent and upload methods #239
Conversation
mrcnski
commented
Dec 15, 2020
•
mrcnski
commented
- [BREAKING] Upload methods return object
- [BREAKING] Remove upload request
- [BREAKING] Make getMetadata return object containing metadata and skylink
- Return object containing contentType, metadata, skylink from getFileContent
- getFileContent returns a skylink prefixed with sia:
- Add getFileContentHns
Previously, getFileContent would return a string for plaintext files and an object for json files. It should only return a string -- in the case of json files it will stringify the json.
src/download.ts
Outdated
| export type GetFileContentResponse<T = unknown> = { | ||
| data: T; | ||
| contentType: string; | ||
| metadata: Record<string, unknown>; | ||
| skylink: string; | ||
| }; | ||
|
|
||
| /** | ||
| * The response for a get metadata request. | ||
| * | ||
| * @property metadata - The metadata in JSON format. | ||
| * @property skylink - 46-character skylink. | ||
| */ | ||
| export type GetMetadataResponse = { | ||
| metadata: Record<string, unknown>; | ||
| skylink: string; | ||
| }; |
There was a problem hiding this comment.
Should we combine these return types with data?: T ? contentType is returned on HEAD calls and could serve a purpose.
There was a problem hiding this comment.
I want to keep the response types separate, but I added contentType to the metadata response. I'm can see potential use cases for it
| ); | ||
| } | ||
|
|
||
| // TODO: Return null instead if header not found? |
There was a problem hiding this comment.
IMO an empty content type is acceptable, especially if we're doing it for the metadata call. It's better than null because the client can treat it as a string in a safe way without needing a nullish check.
| expect(data).toEqual(data); | ||
| }); | ||
|
|
||
| it("Should get JSON file contents", async () => { | ||
| const json = { key: "testdownload" }; | ||
|
|
||
| // Upload the data to acquire its skylink | ||
| const file = new File([JSON.stringify(json)], dataKey, { type: "application/json" }); |
There was a problem hiding this comment.
What happens if we don't specify the content type? Would it be detected?
No need to cover this now but it might be interesting to verify the behaviour.
