Fix #23025: Avoid unnecessary copying files #24570
Conversation
In the process of uploading an image from the Photos app, there are two places that creates file using `mediaFileManager`, with the same filename and at the same directory. 1. Copying the selected image file, which is in a temp directory, to the app's "local media directory". That's done by the code that's moved in this commit. 2. Creating a new image based on the selected image file, for the purpose of removing some exif data. Since these two operations creates the same file at the same directory, the second copy creates a file with "-1" suffix, which is uploaded to the site's media library. That's the root cause of why there is a "-1" in the title of uploaded images.
|
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 27812 | |
| Version | PR #24570 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 23bad6d | |
| Installation URL | 2majdpeeoo9u8 |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 27812 | |
| Version | PR #24570 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 23bad6d | |
| Installation URL | 4cl5o4hp6qaeg |
| @@ -67,15 +70,18 @@ final class ItemProviderMediaExporter: MediaExporter { | |||
|
|
|||
| // Retaining `self` on purpose. | |||
| do { | |||
| let copyURL = try self.mediaFileManager.makeLocalMediaURL(withFilename: url.lastPathComponent, fileExtension: url.pathExtension) | |||
| try FileManager.default.copyItem(at: url, to: copyURL) | |||
There was a problem hiding this comment.
It's a bit unsafe to remove this code because of how loadFileRepresentation works:
This method writes a copy of the file’s data to a temporary file, which the system deletes when the completion handler returns.
There was a problem hiding this comment.
Good catch. I think it's best to keep the copying code. I have created another solution in #24595.
I noticed that too. It's added by the app. The app copies the image files to a specific "local Media" directory ( However, keeping the uploaded files in the local Media directory means that, in theory, the storage the app uses will continue to increase as users upload more files. That's probably something worth looking into. |
Description
In the process of uploading an image from the Photos app, there are two places that creates file using
mediaFileManager, with the same filename and at the same directory.Since these two operations creates the same file at the same directory, the second copy creates a file with "-1" suffix, which is uploaded to the site's media library. That's the root cause of why there is a "-1" in the title of uploaded images.
Testing instructions
Verify the issue is fixed, on WP.com sites and self-hosted sites.