File upload endpoint #205
File upload endpoint #205
Conversation
Adds an upload endpoint to our API. This has been implemented using multer as a middleware for file validations and restrictions on file size. Basic happy/unhappy flow tests pass.
Increases coverage threshold based on updated test suite Updates the types used in lib/users to match updated API types Generated updated Swagger
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Add validations on file type and size
To gradually clean up the repo, we agreed on restructuring helpers from utils to more cleanly scoped libs. Additionally, added some tests
| Tags, | ||
| UploadedFiles, | ||
| } from "tsoa"; | ||
| import { StorageService } from "../services/StorageService.js"; |
There was a problem hiding this comment.
Non-blocking suggestion:
What are your thoughts on organizing imports by separating them into distinct groups with a newline in between?
Suggested order:
Global imports (e.g., libraries)
<newline>
Imports from files outside the current folder
<newline>
Imports from files within the same folder
| * POST /v1/upload | ||
| * Content-Type: multipart/form-data | ||
| * | ||
| * files: [File, File, ...] |
There was a problem hiding this comment.
Is that really a correct example request? If I understand multipart form data correctly, shouldn't it be something like:
POST /v1/upload HTTP/1.1
Content-Type: multipart/form-data; boundary=SomeBoundaryXYZ
--SomeBoundaryXYZ
Content-Disposition: form-data; name="files"; filename="file1.txt"
Content-Type: text/plain
<binary data of file1.txt>
--SomeBoundaryXYZ
Content-Disposition: form-data; name="files"; filename="image1.png"
Content-Type: image/png
<binary data of image1.png>
--SomeBoundaryXYZ--
| } | ||
|
|
||
| const blobs = files?.map( | ||
| (file) => new Blob([file.buffer], { type: file.mimetype }), |
There was a problem hiding this comment.
Where does Blob come from? Is that a native JS type?
| fileName: file.originalname, | ||
| }; | ||
| } catch (error) { | ||
| throw { |
There was a problem hiding this comment.
I think it's better to create a custom Error here instead of throwing a JavaScript object.
| ); | ||
|
|
||
| const successful = uploadResults | ||
| .filter( |
There was a problem hiding this comment.
These next lines are really difficult to read and understand due to the generic, extra brackets and nesting. Would it make sense to extract the type narrowing function out into the module scope and use here?
| .map((result) => result.value); | ||
|
|
||
| const failed = uploadResults | ||
| .filter( |
There was a problem hiding this comment.
Same as the above albeit much less difficult to understand.
| if (failed.length > 0) { | ||
| this.setStatus(failed.length === uploadResults.length ? 500 : 207); | ||
| return { | ||
| success: false, |
There was a problem hiding this comment.
success as boolean doesn't really cut it. How about a string field like this:
{
uploaded: "all" | "some" | "none"
...
}
The goal of this PR is to provide a basic IPFS upload endpoint to self-reported evaluations on hypercerts data. Primarily, it should allow impact creators to provide updates on the data represented withing an hypercert, like evaluation reports or proof of work/attendance/etc..