Deprecate file_managed_file_save_upload(), file_save_upload() and _file_save_upload_from_form() and replace with a service

This deprecates:

• file_managed_file_save_upload()
• _file_save_upload_from_form()
• and file_save_upload()

and replaces them with a few new services.

\Drupal\file\Upload\FileElementHelper::saveFileUploads(array $element, FormStateInterface $formState) replaces file_managed_file_save_upload($element, FormStateInterface $form_state) and _file_save_upload_from_form(). The intention is to separate all the form api and user-facing messaging into this service.

\Drupal\file\Upload\FormFileUploadHandler::saveFileUploads(string $uploadKey, array $validators, callable $errorHandler, ?string $destination = 'temporary://', string $replace = FileSystemInterface::EXISTS_RENAME) replaces file_save_upload($form_field_name, $validators = [], $destination = FALSE, $delta = NULL, $replace = FileSystemInterface::EXISTS_RENAME).

callable $errorHandler that accept three arguments:

• \Drupal\file\Upload\UploadedFileInterface $uploadedFile
• string $destination
• \Exception $e

We then have a \Drupal\file\Upload\MessageCollectingErrorCallback which we pass to collect any errors and allows us to remove a load of the messy message deleting, and re-adding that we had in _file_save_upload_from_form()

\Drupal\file\Upload\FormUploadedFileRetriever removes the duplicated code of retrieving the uploaded files from the request, and is injected into FileElementHelper and FormFileUploadHandler.

Lastly, in order to not have to add a message for each file rename in our API-level code, instead we dispatch a \Drupal\file\Upload\FileUploadedEvent and added a MessengerFileUploadedSubscriber to add a status message if a file was renamed.

I'm confused by the phpstan failure.

First it says its comparing against 10.2.x where the MR is for 11.x. Then its showing an error about a variable $file_upload which I couldn't find anywhere in file.module.

Running PHPStan on *all* files.
 ------ -------------------------------------------------------------------- 
  Line   core/modules/file/file.module                                       
 ------ -------------------------------------------------------------------- 
         Ignored error pattern #^Variable \$file_upload in empty\(\) always  
         exists and is not falsy\.$# in path                                 
         /var/www/html/core/modules/file/file.module was not matched in      
         reported errors

Edit:
Running ./core/scripts/dev/commit-code-check.sh --branch 11.x locally produces no errors.

Looks like I have an issue somewhere with setting the form errors and them being double-escaped:

One or more files could not be uploaded.<ul><li>The file is <em class="placeholder">1.25 MB</em> exceeding the maximum file size of <em class="placeholder">1 MB</em>.</li></ul>

Updating title and remaining tasks:

• Add tests for new services
• Add legacy tests for deprecated functions
• Update mermain diagrams with current architecture
• Update issue summary

The image test fail is due to changes in the messages being displayed:

Old:

The specified file image-test.jpg could not be uploaded.

    The image is too small. The minimum dimensions are 50x50 pixels and the image size is 40x20 pixels.

New:

One or more files could not be uploaded.

    The image is too small. The minimum dimensions are 50x50 pixels and the image size is 40x20 pixels.

Finally, tests passing! 😅

Updated the mermaid diagram to match the current MR.

Updated IS and CR

I fixed that. There is no difference between HEAD and this MR now as to what is output.

Rebase with 11.x

Merge with 11.x

Postponed on #3375447: Create an UploadedFile validator and deprecate error checking methods on UploadedFileInterface since we can remove some of the exception handling once that is in.

Going to try and avoid the whole 'message collector' business, and just return a new FileResults (plural) object that has a ConstraintViolationList. Might be a nice utility to write violations to Drupal::addMessage() or something like that.

Finally tests back to green. ✅ Ready for reviews again.

According to @larowlan we should be deprecating in 10.4.0 for removal in 12.0.0. https://drupal.slack.com/archives/C1BMUQ9U6/p1716324332447859?thread_ts=...

Updated all deprecation versions to drupal:10.4.0 and is removed from drupal:12.0.0

Updated IS

This got stalled most likely because it was trying to do too much.

I'm starting to split things off into smaller issues starting with #3555115: Add a UploadedFilesExtractor and remove duplicate code

Propose converting this to a meta issue under #3221796: [META] Modernise file upload logic, since it stalled from combining too much scope. #3555115: Add a UploadedFilesExtractor and remove duplicate code already replaces FormUploadedFileRetriever with UploadedFilesExtractor.

Suggested child issues, in dependency order:

- Request-scoped memory cache for file uploads (no dependencies).
- FileUploadResult::getErrors() for FileExistsException and FileException; handler stops throwing them (no dependencies).
- New multi-file upload service returning array (depends on the two above).
- Deprecate file_save_upload() with a BC shim (depends on the new service).
- Migrate the ManagedFile widget and remove _file_save_upload_from_form() (depends on the new service).
- Migrate ThemeSettingsForm, ImportForm, and navigation SettingsForm (depends on the new service).
- Test cleanup: rewrite file_test, re-home SaveUploadFormTest assertions, convert Functional to Kernel tests where possible (depends on all above).

This replaces the FileElementHelper/FormFileUploadHandler design with smaller, independently reviewable issues.

I'll go ahead an start creating these sub-issues.

Created sub-issues:

• #3619416: Replace file_save_upload()'s static cache with the cache.memory bin
• #3619417: Return FileExistsException and FileException as data on FileUploadResult
• #3619418: Add a service to replace file_save_upload() and deprecate it
• #3619420: Remove _file_save_upload_from_form() and deprecate file_managed_file_save_upload()

Flagging the IS needs updating with the new sub-task approach

Since @nicxvan worked on this PR overnight, I've done some cleanup. We can postpone the sub-issues I created if this approach is viable.

• Fixed the build by changing the deprecated MemoryCacheInterface to BackendCacheInterface
• Changed the cache ID to be scoped per form field name (file:uploads:<field_name>).
• Renamed FileElementHelper to ManagedFileElementHelper to avoid confusion between the form element \Drupal\file\Element\ManagedFile and the render element \Drupal\Core\Render\Element\File, and updated all references.

Here's a before/after diagram.

I reviewed this code (it has been a while) and I think we should remove as much user facing code from FormFileUploader and let calling code handle Messenger calls. This resulted in a new result value object and bringing in #3619417: Return FileExistsException and FileException as data on FileUploadResult into this MR.

• Incorporated #3619417 into this issue.
• Added FileUploadResult::addError()/getErrors()/hasErrors().
• FileUploadHandler::handleFileUpload() now returns FileExistsException and FileException as data instead of throwing them.
• FormFileUploader no longer depends on Messenger or the Renderer; it returns errors as data on the result object.

I have reviewed the changes that @nicxvan has been making, and (one minor nitpick remaining) I think this is ready. RTBC+1

Note for committers: the API is pretty much a 1-to-1 conversion from the procedural code. @alexpott highlighted (somewhere?) the security risk of rewriting the whole API in one go, so I think we need to make these changes incrementally. Having said that, I'm keen to keep pushing forward on cleaning up remaining issues (e.g. the Messenger calls in what should be a pure API) in follow ups.

The last couple of commits we just updating the default $destination param handling and a minor typehint change `int|null` -> `?int`. Looks good, so back to RTBC