[#44848] - [5.3] Media: fix large file upload by using of binary upload instead of base64, and display correct upload progress
- Pending
- Medium
- Build: 5.3-dev
- # 44848
- Diff
- Fedik:media-blb-upload
User tests: Successful: Unsuccessful:
Summary of Changes
The PR changes the media manager to use FormData for file upload, instead of base64 payload.
Which improves responsiveness, performance and handling of large files in general.
However the api/ calls still uses base64 because it is hard to do in b/c maner.
Bonus: the upload progres now showing actual upload progress.
Testing Instructions
Apply patch.
Run npm install.
Go to media manager and try upload something bigger than 100Mb
Addittionaly, test with any 3d filesystem plugin.
Actual result BEFORE applying this Pull Request
The browser hangs for a while, and dependent from your luck the upload will be completed after some waiting.
Expected result AFTER applying this Pull Request
The upload starts immediately, and you can see progress (a litle 5px blue line over the files list)
Link to documentations
Please select:
- Documentation link for docs.joomla.org:
- No documentation changes for docs.joomla.org needed
- Pull Request link for manual.joomla.org: IDK
- No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Administration com_media NPM Change Repository Front End Plugins |
| Labels |
Added:
Feature
NPM Resource Changed
PR-5.3-dev
Performance
|
||
| Title |
|
||||||
ApiController
Hmhm,
I would read the file content it in the api controller and then pass it to the adapter the same way as before that no changes are needed in the adapter
No no, this not going to work, you will get "out of memory" error when try to read 100Mb file when PHP limit is 60Mb.
This will bring back problem with large file upload.
Loading whole file in to memory should never happen while uploading (unless when it realy need).
What about stringable object?
We wrap uploaded file in to that object, and the file will be loaded in to memory only when the plugin expecting a string. Addittionaly we deprecate this behavior.
What do you think?
Not sure if stringable would solve it. But I think the main goal should be to do it in a bc way.
| Category | JavaScript Administration com_media NPM Change Repository Front End Plugins | ⇒ | JavaScript Administration com_media NPM Change Repository Libraries Front End Plugins |
I updated the code to use stringable object.
In my tests it works well even when try to access the data as to the string.
Would be good to have some more test, to be sure, with 3rd filesystem adapters.
I like that change, but this will break 3rd party extensions when the data can be all of the sudden a resource. I would read the file content it in the api controller and then pass it to the adapter the same way as before that no changes are needed in the adapter. Additionally, the check content logic can probably be moved to the, but not sure about this.