Add file upload lock handling to FileUploadHandler

Changed title from 'move' to 'add' since we aren't moving in this issue.

Change looks good and is all green. The CR could use some updates though.

Thanks!

Updated change record

Thanks!

I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions.

But this needs to be rebased.

Re-rolled. Back to RTBC

I think we should postpone this on 📌 Make CKEditor5ImageController reuse FileUploadHandler Active so we have an example of moving the locking rather just adding it as we do here.

Blocker is in

Rebased on 11.x

Seems to have failing tests, reran but still failed.

Needed to set $throws to FALSE after the change in 📌 Create an UploadedFile validator and deprecate error checking methods on UploadedFileInterface RTBC

That seemed to be it!

Left some comments on the MR

Feedback appears to be addressed.

@trigger_error('Calling ' . __METHOD__ . '() without the $lock argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/3389017', E_USER_DEPRECATED);

I don't think this deprecation message means what it says, which is that calls to the FileUploadHandler constructor will be required to omit the $lock argument in drupal:11.0.0. There are a number of spots like this in the patch.

Perhaps you meant "prohibited" or "prevented" where you have "required"?

#19 No it's correct as it is. We are adding a new $lock dependency, which will throw a deprecation warning if it's not provided in 10.3.0, and as a BC layer, we look up the service using \Drupal::service(). But in Drupal 11.0.0 we remove this BC layer and it will throw an error if it's not provided.

@kim.pepper Your description in #20 matches what I assumed you meant the deprecation message to convey, but that's not what it actually says. If you read its language carefully, you'll realize that "Calling" is the subject of both verbs, so parsed correctly, the meaning is as follows:

"Calling __construct() without the $lock argument is deprecated in drupal:10.3.0."

and

"Calling __construct() without the $lock argument is required in drupal:11.0.0."

'Calling ' . __METHOD__ . '() without the $lock argument is deprecated in drupal:10.3.0 and is required in drupal:11.0.0' is correct.

Sigh. Oh, well, at least they're just deprecation messages, so they be gone eventually, along with the incorrect wording. 🙄

Feel free to open a [Policy, No Patch] issue to suggest changes to the wording. We have a phpcs rule to ensure it matches the official pattern. See https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... →

This is a nice improvement to the API.

Committed and pushed 6236d36180 to 11.x and e44ea05058 to 10.3.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

kim.pepper → closed merge request !4847