Convert file_save_upload() to a service

The file_save_upload() function needs to continue to exist because existing code still uses it. The function should emit a deprecation notice and then call the new service. There should be a test for the deprecation notice.

Tests need to be passing before marking this "Needs review".

I was sure there was an issue for this already, but I might be confusing with 📌 [PP-4] Unify file upload logic of REST and JSON:API Postponed

You may also been thinking of the other related ones.

Sorry I will update it.

Hi does anyone has an idea how to fix this test ?

The patch adds file_save.upload service. AutowireTest::testCoreServiceAliases() is looking for a particular list of services. The new service needs to be added to the test.

I just rebased I'm keep working on it

The patch adds file_save.upload service. AutowireTest::testCoreServiceAliases() is looking for a particular list of services. The new service needs to be added to the test.

Dou you have an idea how to do that?

Look at AutowireTest::testCoreServiceAliases() to see what it is testing. There will be a place where it is testing things like file.validator and content_translation.synchronizer. The new service needs to be added.

Hi I'm seeing tests failed because deprecation notices should I remove the deprecation things and then remove the tests about it ?

see an example:

24x: /app/core/modules/file/file.field.inc is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Functions have been moved to file.module. See https://www.drupal.org/node/3369330
    24x in BlockContentListTest::testListing from Drupal\Tests\block_content\Functional

Other deprecation notices (1)

  1x: /app/core/modules/file/file.field.inc is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Functions have been moved to file.module. See https://www.drupal.org/node/3369330
    1x in BlockContentListTest::testListing from Drupal\Tests\block_content\Functional

Hi please revise.

Thanks for the patch.

file_save_upload() should be changed so that it uses the new service and emits a deprecation notice. Look at other functions that became services to see an example.

It might be deprecated in some version of Drupal 10 and removed in Drupal 11. The @deprecated comment could be written to reflect this.

Hi @Liam Morland I tried follow you suggestion, please can you see if that's correct?

You don't need the line with "move this logic to a service"; it's being done here.

You don't need the references to: https://www.drupal.org/node/2244513 →

Calling file_save_upload() needs to trigger a deprecation notice to appear in logs so that module maintainers now to update their code.

For the points in #19

Added the meta as a parent 📌 [META] Modernise file upload logic Active .

I think we need to look at the logic in the call stack and see if we can make an improvement across all of these:

1. file_managed_file_save_upload()
2. _file_save_upload_from_form()
3. file_save_upload()
4. FileUploadHandler::handleFileUpload()

Let's discuss in the parent and devise a plan.

Closing this as we're looking to do all three at the same time in 📌 Deprecate file_managed_file_save_upload(), file_save_upload() and _file_save_upload_from_form() and replace with a service Needs review