feat: Allow large Parse File uploads #9286
Conversation
Thanks for opening this pull request! |
spec/FilesRouter.spec.js
Outdated
|
|
||
| afterAll(async () => { | ||
| // clean up the server for resuse | ||
| if (server && server.close) { |
There was a problem hiding this comment.
Wouldn't it be enough to just call await reconfigureServer() to init the server with the default config?
There was a problem hiding this comment.
Your right. Missed that, new to testing here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## alpha #9286 +/- ##
==========================================
+ Coverage 93.50% 93.52% +0.02%
==========================================
Files 186 186
Lines 14810 14834 +24
==========================================
+ Hits 13848 13874 +26
+ Misses 962 960 -2 ☔ View full report in Codecov by Sentry. |
|
There seems to be some test coverage missing, could you add that? |
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
Signed-off-by: Manuel <5673677+mtrezza@users.noreply.github.com>
|
Could you take a look at the failing postgres tests? |
a storage adapter is needed to test postgres
@mtrezza I changed those failing tests to only mongo. Without pulling in a storage adapter such as the fs adapter, those tests will not be able to pass in postgres. Let me know if pulling in one as a dev dependency would be preferable? All the adapters will also need changes similar to the inbuilt GridFS adapters changes in order to handle the large files. I have adjustments for the fs and s3 adapters that I can also make PRs in their respective repositories for as needed. |
|
I understand the change is really for the GridFSStorageAdapter and it just so happens that the adapter is bundled with Parse Server. It isn't related to DB adapters, so maybe the tests should be MongoDB only in this PR. Does that make sense? Only if the Postgres adapter supported Parse.File upload (like MongoDB does with GridFS), then there would be some Postgres related changes needed in this PR, does it? Yes, for other storage adapters the PRs in their respective repos would be great. |
|
Yes that is correct. There would need to be inbuilt storage adapters to test for postgres. I've made the other PRs: |
|
Could you change the test from |
Pull Request
Issue
Closes: #9283
Approach
During routing if a file size is larger than can be handled by the V8 engine as a string, it is used as a Blob. This changes touches both the
FilesRouterandGridFSBucketAdapterwhere the creation/upload happens, and the file is handled as a Buffer. I also added a new FilesRouter test spec for these changes.Tasks