[6.0] Media: fix large file upload by using of binary upload instead of base64, and display correct upload progress #44848
+957
−567
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joomla-cms-bot
added
NPM Resource Changed
brianteeman
reviewed
Feb 10, 2025
administrator/components/com_media/resources/scripts/store/mutation-types.es6.js
Outdated
Show resolved
Hide resolved
Fedik
changed the title
|
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. |
@laoneo Moved to the ... what? |
|
ApiController |
|
Hmhm,
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. What about stringable object? |
|
Not sure if stringable would solve it. But I think the main goal should be to do it in a bc way. |
|
I updated the code to use stringable object. Would be good to have some more test, to be sure, with 3rd filesystem adapters. |
* Change list() to array destruct for libraries code
…oomla#44970) * Change list() to array destruct for modules and plugins code --------- Co-authored-by: Quy Ton <quy@nomonkeybiz.com>
* Add deprecation message to content modules --------- Co-authored-by: Quy <quy@nomonkeybiz.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
The `CYPRESS_CACHE_FOLDER` environment variable must be set within the `sudo` command.
* [Cypress] PHP Backend Notice com_media/Files.cy.js Fixing all Joomla backend PHP notices from Cypress test file `api/com_media/Files.cy.js` and doing some refactoring. Fixed PHP notices are: ``` [Sat Feb 22 18:28:44.019593 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: exif_imagetype(): Error reading from /var/www/html/files/test-image-1.jpg! in /var/www/html/libraries/src/Helper/MediaHelper.php on line 93 [Sat Feb 22 18:28:44.022074 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: getimagesize(): Error reading from /var/www/html/files/test-image-1.jpg! in /var/www/html/libraries/src/Image/Image.php on line 177 [Sat Feb 22 18:28:44.258619 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: exif_imagetype(): Error reading from /var/www/html/files/test-dir/test-image-1-subfolder.jpg! in /var/www/html/libraries/src/Helper/MediaHelper.php on line 93 [Sat Feb 22 18:28:44.259092 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: getimagesize(): Error reading from /var/www/html/files/test-dir/test-image-1-subfolder.jpg! in /var/www/html/libraries/src/Image/Image.php on line 177 [Sat Feb 22 18:28:47.177357 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: exif_imagetype(): Error reading from /var/www/html/files/test-dir/todelete.jpg! in /var/www/html/libraries/src/Helper/MediaHelper.php on line 93 [Sat Feb 22 18:28:47.178099 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: getimagesize(): Error reading from /var/www/html/files/test-dir/todelete.jpg! in /var/www/html/libraries/src/Image/Image.php on line 177 [Sat Feb 22 18:28:47.564807 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: exif_imagetype(): Error reading from /var/www/html/images/test-dir/todelete.jpg! in /var/www/html/libraries/src/Helper/MediaHelper.php on line 93 [Sat Feb 22 18:28:47.565288 2025] [php:notice] [pid 31:tid 31] [client 10.0.0.1:64714] PHP Notice: getimagesize(): Error reading from /var/www/html/images/test-dir/todelete.jpg! in /var/www/html/libraries/src/Image/Image.php on line 177 ``` All 8 x PHP notices are fixed. Instead of using 1-Byte files real images are used from Cypress fixtures. Please consider there are still 4 x PHP warning to be fixed (I assume they need to be fixed in PHP API backend code) from `api/com_media/Files.cy.js` and more warnings and notices in the overall Joomla System Tests. Some refactoring: * Deleted 3 images in `tests/System/data/com_media`, 2 are not used and Cypress default is to use `fixtures` folder * The image file names are counted through and the images show name and path in different colors. * Second `test-dir` was named `test-dir2` to distinguish clearly. * `afterEach()` is reduced to `after()`, as it is only a clean-up and not needed to execute the tests. * The last 4 tests can use the prepared files too, as with `beforeEach` the files are restored before each test. * Corrected Image Description to test-dir2 Used command: ``` convert -size 100x100 -gravity center -pointsize 11 xc:lightgreen -fill red -annotate +0+0 'Joomla\nSystem Tests\n\nimages/test-dir2\ntest-image-3.jpg' tests/System/fixtures/com_media/test-image-3.jpg ``` * Fix for Windows Path and Read-Only Overwrites - Creating relative fixtures folder path now also works on Windows with backslashes as separators and a drive letter - All overwrite files are now created with explicit read-write 0o666, as the default 0o444 creates read-only files, leading to PHP Warnings `Failed to open` --------- Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com> Co-authored-by: Robert Deutz <rdeutz@googlemail.com>
Conflicts: plugins/filesystem/local/src/Adapter/LocalAdapter.php
|
This pull request has been automatically rebased to 6.0-dev. |
HLeithner
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: