Create content disposition filename extractor and deprecate duplicate code in REST and JSON API

• Creates the content disposition filename parser
• Replaces usage in REST module
• Replaces usage in JSON API module

Seems like a good change.
CR reads well also and includes removed functions.

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

I skimmed the MR. At first I thought that the constant REQUEST_HEADER_FILENAME_REGEX would need to be renamed because of the coding standards for Constants → . That states that "Module-defined constant names should also be prefixed by an uppercase spelling of the module that defines them." But the I searched core and found it used in two other files, one in the files module and one in jsonapi. It would be better to keep the name the same for now and fix it later.

I think the CR could use a more succinct title and before/after code samples.

"Module-defined constant names should also be prefixed by an uppercase spelling of the module that defines them."

That makes sense if it's in a *.module file, but this will always be called with \Drupal\file\Upload\ContentDispositionFilenameParser::REQUEST_HEADER_FILENAME_REGEX which clearly shows the module and the class it comes from.

In regards to the CR, I wasn't sure if this issue actually justified one. I don't think there is a before and after needed as no one is calling this code. Its an internal protected method that is duplicated in two places: TemporaryJsonapiFileFieldUploader and FileUploadResource. This issue removes the duplication into shared code.

@kim.pepper, thanks for the explanations. I do see that I must read more carefully!

Blocking a critical 📌 [PP-4] Unify file upload logic of REST and JSON:API Postponed therefore this is also critical.

I changed this to a static method, as we are only using the FileSystem dependency for the basename method. And since we are just parsing a filename in content disposition header, we don't need to support StreamWrappers. Therefore the standard PHP basename function should be fine.

One less service to inject!

Seems change to static method is fine. Not sure if it should be noted in the CR or if that matters.

Needs a re-roll after 📌 Create a InputStreamFileWriter for writing the input stream to a file RTBC

Re-rolled

Committed and pushed to 11.x
Published change record

Updated the change record to reflect this uses a static now and not a service

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

kim.pepper → closed merge request !4575