Only use the appGroupTemporaryDirectory to access a file from the share extension. (#4002)

… and switch back to the plain `URL.temporaryDirectory` for everything else.

* Fix documentation

Co-authored-by: Stefan Ceriu <stefan.ceriu@gmail.com>

Author: pixlwave

ElementX/Sources/Application/AppCoordinator.swift

@@ -125,6 +125,9 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
 
         if let previousVersion = appSettings.lastVersionLaunched.flatMap(Version.init) {
             performMigrationsIfNecessary(from: previousVersion, to: currentVersion)
+            
+            // Manual clean to handle the potential case where the app crashes before moving a shared file.
+            cleanAppGroupTemporaryDirectory()
         } else {
             // The app has been deleted since the previous run. Reset everything.
             wipeUserData(includingSettings: true)
@@ -252,12 +255,17 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
                 } else {
                     handleAppRoute(.childEventOnRoomAlias(eventID: eventID, alias: alias))
                 }
-            case .share:
+            case .share(let payload):
                 guard isExternalURL else {
                     MXLog.error("Received unexpected internal share route")
                     break
                 }
-                handleAppRoute(route)
+                
+                do {
+                    try handleAppRoute(.share(payload.withDefaultTemporaryDirectory()))
+                } catch {
+                    MXLog.error("Failed moving payload out of the app group container: \(error)")
+                }
             default:
                 break
             }
@@ -408,6 +416,31 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
         userSessionStore.reset()
     }
 
+    /// Manually cleans up any files in the app group's `tmp` directory.
+    ///
+    /// **Note:** If there is a single file we consider it to be an active share payload and ignore it.
+    private func cleanAppGroupTemporaryDirectory() {
+        let fileURLs: [URL]
+        do {
+            fileURLs = try FileManager.default.contentsOfDirectory(at: URL.appGroupTemporaryDirectory, includingPropertiesForKeys: nil, options: [])
+        } catch {
+            MXLog.warning("Failed to enumerate app group temporary directory: \(error)")
+            return
+        }
+        
+        guard fileURLs.count > 1 else {
+            return // If there is only a single item in here, there's likely a pending share payload that is yet to be processed.
+        }
+        
+        for url in fileURLs {
+            do {
+                try FileManager.default.removeItem(at: url)
+            } catch {
+                MXLog.warning("Failed to remove file from app group temporary directory: \(error)")
+            }
+        }
+    }
+    
     private func setupStateMachine() {
         stateMachine.addTransitionHandler { [weak self] context in
             guard let self else { return }

ElementX/Sources/Other/Extensions/FileManager.swift

@@ -37,8 +37,9 @@ extension FileManager {
     }
 
     @discardableResult
-    func writeDataToTemporaryDirectory(data: Data, fileName: String) throws -> URL {
-        let newURL = URL.appGroupTemporaryDirectory.appendingPathComponent(fileName)
+    func writeDataToTemporaryDirectory(data: Data, fileName: String, withinAppGroupContainer: Bool = false) throws -> URL {
+        let baseURL: URL = withinAppGroupContainer ? .appGroupTemporaryDirectory : .temporaryDirectory
+        let newURL = baseURL.appendingPathComponent(fileName)
 
         try data.write(to: newURL)
 

ElementX/Sources/Other/Extensions/NSItemProvider.swift

@@ -28,20 +28,23 @@ extension NSItemProvider {
         try? await loadItem(forTypeIdentifier: UTType.text.identifier) as? String
     }
 
-    func storeData() async -> URL? {
+    /// Stores the item's data from the provider within the temporary directory, returning the URL on success.
+    /// - Parameter withinAppGroupContainer: Whether the data needs to be shared between bundles.
+    /// If passing `true` you will need to manually clean up the file once you have the data in the receiving bundle.
+    func storeData(withinAppGroupContainer: Bool = false) async -> URL? {
         guard let contentType = preferredContentType else {
             MXLog.error("Invalid NSItemProvider: \(self)")
             return nil
         }
 
         if contentType.type.identifier == UTType.image.identifier {
-            return await generateURLForUIImage(contentType)
+            return await generateURLForUIImage(contentType, withinAppGroupContainer: withinAppGroupContainer)
         } else {
-            return await generateURLForGenericData(contentType)
+            return await generateURLForGenericData(contentType, withinAppGroupContainer: withinAppGroupContainer)
         }
     }
 
-    private func generateURLForUIImage(_ contentType: PreferredContentType) async -> URL? {
+    private func generateURLForUIImage(_ contentType: PreferredContentType, withinAppGroupContainer: Bool) async -> URL? {
         guard let uiImage = try? await loadItem(forTypeIdentifier: contentType.type.identifier) as? UIImage else {
             MXLog.error("Failed casting UIImage, invalid NSItemProvider: \(self)")
             return nil
@@ -52,22 +55,25 @@ extension NSItemProvider {
             return nil
         }
 
+        let filename = if let suggestedName = suggestedName as NSString?,
+                          // Suggestions are nice but their extension is `jpeg`
+                          let filename = (suggestedName.deletingPathExtension as NSString).appendingPathExtension(contentType.fileExtension) {
+            filename
+        } else {
+            "\(UUID().uuidString).\(contentType.fileExtension)"
+        }
+        
         do {
-            if let suggestedName = suggestedName as? NSString,
-               // Suggestions are nice but their extension is `jpeg`
-               let filename = (suggestedName.deletingPathExtension as NSString).appendingPathExtension(contentType.fileExtension) {
-                return try FileManager.default.writeDataToTemporaryDirectory(data: pngData, fileName: filename)
-            } else {
-                let filename = "\(UUID().uuidString).\(contentType.fileExtension)"
-                return try FileManager.default.writeDataToTemporaryDirectory(data: pngData, fileName: filename)
-            }
+            return try FileManager.default.writeDataToTemporaryDirectory(data: pngData,
+                                                                         fileName: filename,
+                                                                         withinAppGroupContainer: withinAppGroupContainer)
         } catch {
             MXLog.error("Failed storing NSItemProvider data \(self) with error: \(error)")
             return nil
         }
     }
 
-    private func generateURLForGenericData(_ contentType: PreferredContentType) async -> URL? {
+    private func generateURLForGenericData(_ contentType: PreferredContentType, withinAppGroupContainer: Bool) async -> URL? {
         let providerDescription = description
         let shareData: Data? = await withCheckedContinuation { continuation in
             _ = loadDataRepresentation(for: contentType.type) { data, error in
@@ -92,15 +98,19 @@ extension NSItemProvider {
             return nil
         }
 
+        let filename = if let suggestedName = suggestedName as NSString?,
+                          suggestedName.hasPathExtension {
+            suggestedName as String
+        } else if let suggestedName {
+            "\(suggestedName).\(contentType.fileExtension)"
+        } else {
+            "\(UUID().uuidString).\(contentType.fileExtension)"
+        }
+        
         do {
-            if let filename = suggestedName {
-                let hasExtension = !(filename as NSString).pathExtension.isEmpty
-                let filename = hasExtension ? filename : "\(filename).\(contentType.fileExtension)"
-                return try FileManager.default.writeDataToTemporaryDirectory(data: shareData, fileName: filename)
-            } else {
-                let filename = "\(UUID().uuidString).\(contentType.fileExtension)"
-                return try FileManager.default.writeDataToTemporaryDirectory(data: shareData, fileName: filename)
-            }
+            return try FileManager.default.writeDataToTemporaryDirectory(data: shareData,
+                                                                         fileName: filename,
+                                                                         withinAppGroupContainer: withinAppGroupContainer)
         } catch {
             MXLog.error("Failed storing NSItemProvider data \(self) with error: \(error)")
             return nil
@@ -164,3 +174,7 @@ extension NSItemProvider {
         return mimeType.hasPrefix("image/") || mimeType.hasPrefix("video/") || mimeType.hasPrefix("application/")
     }
 }
+
+private extension NSString {
+    var hasPathExtension: Bool { !pathExtension.isEmpty }
+}

ElementX/Sources/Other/Extensions/URL.swift

@@ -75,7 +75,10 @@ extension URL: @retroactive ExpressibleByStringLiteral {
         return url
     }
 
-    /// The app group temporary directory
+    /// The app group temporary directory (useful for transferring files between different bundles).
+    ///
+    /// **Note:** This `tmp` directory doesn't appear to behave as expected as it isn't being tidied up by the system.
+    /// Make sure to manually tidy up any files you place in here once you've transferred them from one bundle to another.
     static var appGroupTemporaryDirectory: URL {
         let url = appGroupContainerDirectory
             .appendingPathComponent("tmp", isDirectory: true)

ElementX/Sources/ShareExtension/ShareExtensionModels.swift

@@ -22,9 +22,30 @@ enum ShareExtensionPayload: Hashable, Codable {
             roomID
         }
     }
+    
+    /// Moves any files in the payload from our `appGroupTemporaryDirectory` to the
+    /// system's `temporaryDirectory` returning a modified payload with updated file URLs.
+    func withDefaultTemporaryDirectory() throws -> Self {
+        switch self {
+        case .mediaFile(let roomID, let mediaFile):
+            let path = mediaFile.url.path.replacing(URL.appGroupTemporaryDirectory.path, with: "").trimmingPrefix("/")
+            let newURL = URL.temporaryDirectory.appending(path: path)
+            
+            try? FileManager.default.removeItem(at: newURL)
+            try FileManager.default.moveItem(at: mediaFile.url, to: newURL)
+            
+            return .mediaFile(roomID: roomID, mediaFile: mediaFile.replacingURL(with: newURL))
+        case .text:
+            return self
+        }
+    }
 }
 
 struct ShareExtensionMediaFile: Hashable, Codable {
     let url: URL
     let suggestedName: String?
+    
+    fileprivate func replacingURL(with newURL: URL) -> ShareExtensionMediaFile {
+        ShareExtensionMediaFile(url: newURL, suggestedName: suggestedName)
+    }
 }

ShareExtension/Sources/ShareExtensionViewController.swift

@@ -44,7 +44,7 @@ class ShareExtensionViewController: UIViewController {
 
         let roomID = (extensionContext?.intent as? INSendMessageIntent)?.conversationIdentifier
 
-        if let fileURL = await itemProvider.storeData() {
+        if let fileURL = await itemProvider.storeData(withinAppGroupContainer: true) {
             return .mediaFile(roomID: roomID, mediaFile: .init(url: fileURL, suggestedName: fileURL.lastPathComponent))
         } else if let url = await itemProvider.loadTransferable(type: URL.self) {
             return .text(roomID: roomID, text: url.absoluteString)