Problem/Motivation
When a file is missing on the file system, it crashes when moving the file and the batch process stops.
Steps to reproduce
- Manually delete a file from the files folder
- Start the move process
- The process will throw an error and stop
There are also significant bugs in the original filefield_paths_filefield_paths_process_file() code.
It gets $temporary_scheme_name and $source_scheme_name incorrectly, causing the file move/rename not to happen at all in some cases.
This is fixed by using StreamWrapperManager::getScheme(). See this commit on the MR.
Proposed resolution
Adding a try-catch around the move process, fix the code to correctly get the schema names.
Remaining tasks
Review the MR
Comments
Comment #2
solideogloria commentedThe patch changes other things besides adding a try-catch. It removed the
prepareDirectoryfunction from the if check, which shouldn't have been changed. It doesn't throw an exception on failure; it returns false. So having twotrystatements isn't helpful.Comment #4
solideogloria commentedComment #5
solideogloria commentedThe test failures appear to be unrelated. I reran the tests for the previously submitted patch file, which had all tests passing previously, but now tests fail. I think this means the branch's tests are failing, and I can't figure out what is causing them.
Comment #6
solideogloria commentedComment #7
solideogloria commentedI found significant bugs in the original
filefield_paths_filefield_paths_process_file()code.It gets
$temporary_scheme_nameand$source_scheme_nameincorrectly, causing the file move/rename not to happen at all in some cases.This is fixed by using
StreamWrapperManager::getScheme(). See the latest commit on the MR.Comment #8
solideogloria commentedComment #9
solideogloria commentedComment #10
solideogloria commentedComment #11
solideogloria commentedHmm, the test that fails succeeded in PHP 7.4 with Drupal 9.5.
Comment #12
volegerNeeds rebase.
Comment #13
solideogloria commentedComment #14
volegerThanks. Changes look good, but two PHPCS issues must be addressed before merging.
Comment #15
solideogloria commentedComment #17
volegerThanks