parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

feat: Allow large Parse File uploads

Open dalyaidan1 opened this issue 1 year ago • 9 comments
trafficstars

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 FilesRouter and GridFSBucketAdapter where the creation/upload happens, and the file is handled as a Buffer. I also added a new FilesRouter test spec for these changes.

Tasks

  • [x] Add tests
  • [x] Add changes to documentation (guides, repository pages, code comments)

dalyaidan1 avatar Aug 18 '24 00:08 dalyaidan1

Thanks for opening this pull request!

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.52%. Comparing base (30a836d) to head (a87a452).

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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 18 '24 13:08 codecov[bot]

There seems to be some test coverage missing, could you add that?

mtrezza avatar Aug 18 '24 13:08 mtrezza

Could you take a look at the failing postgres tests?

mtrezza avatar Oct 08 '24 18:10 mtrezza

Could you take a look at the failing postgres tests?

@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.

dalyaidan1 avatar Nov 02 '24 19:11 dalyaidan1

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.

mtrezza avatar Nov 03 '24 11:11 mtrezza

Yes that is correct. There would need to be inbuilt storage adapters to test for postgres.

I've made the other PRs:

  • https://github.com/parse-community/parse-server-fs-adapter/pull/76
  • https://github.com/parse-community/parse-server-s3-adapter/pull/229

dalyaidan1 avatar Nov 03 '24 13:11 dalyaidan1

Could you change the test from it to it_only_db('mongo') I believe it's called.

mtrezza avatar Nov 03 '24 16:11 mtrezza

Could you change the test from it to it_only_db('mongo') I believe it's called.

I think I covered this in 18157be already?

The one current fail appears to be one of the flaky tests mentioned in here

dalyaidan1 avatar Nov 03 '24 17:11 dalyaidan1