Rewrite modifyItem to make it non-blocking on the main thread in FileProviderExtension
authorClaudio Cambra <claudio.cambra@nextcloud.com>
Wed, 15 Mar 2023 16:21:10 +0000 (17:21 +0100)
committerClaudio Cambra <claudio.cambra@nextcloud.com>
Fri, 12 May 2023 05:29:53 +0000 (13:29 +0800)
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift

index d6b699c3c13824ae0d8683e33a0858b3cb7f41fb..4e59e550141d1b769ebc23897cf458a686f9f6ed 100644 (file)
@@ -389,66 +389,71 @@ class FileProviderExtension: NSObject, NSFileProviderReplicatedExtension, NKComm
 
         var modifiedItem = item
 
+        // Create a serial dispatch queue
+        // We want to wait for network operations to finish before we fire off subsequent network
+        // operations, or we might cause explosions (e.g. trying to modify items that have just been
+        // moved elsewhere)
+        let dispatchQueue = DispatchQueue(label: "modifyItemQueue", qos: .userInitiated)
+
         if changedFields.contains(.filename) || changedFields.contains(.parentItemIdentifier) {
-            let ocId = item.itemIdentifier.rawValue
-            Logger.fileProviderExtension.debug("Changed fields for item \(ocId, privacy: .public) with filename \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)) includes filename or parentitemidentifier...")
+            dispatchQueue.async {
+                let ocId = item.itemIdentifier.rawValue
+                Logger.fileProviderExtension.debug("Changed fields for item \(ocId, privacy: .public) with filename \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)) includes filename or parentitemidentifier...")
 
-            guard let metadata = dbManager.itemMetadataFromOcId(ocId) else {
-                Logger.fileProviderExtension.error("Could not acquire metadata of item with identifier: \(item.itemIdentifier.rawValue, privacy: .public)")
-                completionHandler(item, [], false, NSFileProviderError(.noSuchItem))
-                return Progress()
-            }
+                guard let metadata = dbManager.itemMetadataFromOcId(ocId) else {
+                    Logger.fileProviderExtension.error("Could not acquire metadata of item with identifier: \(item.itemIdentifier.rawValue, privacy: .public)")
+                    completionHandler(item, [], false, NSFileProviderError(.noSuchItem))
+                    return
+                }
 
-            var renameError: NSFileProviderError?
-            let oldServerUrlFileName = metadata.serverUrl + "/" + metadata.fileName
+                var renameError: NSFileProviderError?
+                let oldServerUrlFileName = metadata.serverUrl + "/" + metadata.fileName
 
-            // We want to wait for network operations to finish before we fire off subsequent network
-            // operations, or we might cause explosions (e.g. trying to modify items that have just been
-            // moved elsewhere)
-            let dispatchGroup = DispatchGroup()
-            dispatchGroup.enter()
+                let moveFileOrFolderDispatchGroup = DispatchGroup() // Make this block wait until done
+                moveFileOrFolderDispatchGroup.enter()
 
-            self.ncKit.moveFileOrFolder(serverUrlFileNameSource: oldServerUrlFileName,
-                                        serverUrlFileNameDestination: newServerUrlFileName,
-                                        overwrite: false) { account, error in
-                guard error == .success else {
-                    Logger.fileTransfer.error("Could not move file or folder: \(oldServerUrlFileName, privacy: OSLogPrivacy.auto(mask: .hash)) to \(newServerUrlFileName, privacy: OSLogPrivacy.auto(mask: .hash)), received error: \(error, privacy: .public)")
-                    renameError = error.toFileProviderError()
-                    dispatchGroup.leave()
-                    return
-                }
+                self.ncKit.moveFileOrFolder(serverUrlFileNameSource: oldServerUrlFileName,
+                                            serverUrlFileNameDestination: newServerUrlFileName,
+                                            overwrite: false) { account, error in
+                    guard error == .success else {
+                        Logger.fileTransfer.error("Could not move file or folder: \(oldServerUrlFileName, privacy: OSLogPrivacy.auto(mask: .hash)) to \(newServerUrlFileName, privacy: OSLogPrivacy.auto(mask: .hash)), received error: \(error.error, privacy: .public)")
+                        renameError = error.toFileProviderError()
+                        moveFileOrFolderDispatchGroup.leave()
+                        return
+                    }
 
-                // Remember that a folder metadata's serverUrl is its direct server URL, while for
-                // an item metadata the server URL is the parent folder's URL
-                if itemTemplateIsFolder {
-                    dbManager.renameDirectoryAndPropagateToChildren(ocId: ocId, newServerUrl: newServerUrlFileName, newFileName: item.filename)
-                } else {
-                    dbManager.renameItemMetadata(ocId: ocId, newServerUrl: parentItemMetadata.serverUrl, newFileName: item.filename)
-                }
+                    // Remember that a folder metadata's serverUrl is its direct server URL, while for
+                    // an item metadata the server URL is the parent folder's URL
+                    if itemTemplateIsFolder {
+                        dbManager.renameDirectoryAndPropagateToChildren(ocId: ocId, newServerUrl: newServerUrlFileName, newFileName: item.filename)
+                    } else {
+                        dbManager.renameItemMetadata(ocId: ocId, newServerUrl: parentItemMetadata.serverUrl, newFileName: item.filename)
+                    }
 
-                guard let newMetadata = dbManager.itemMetadataFromOcId(ocId) else {
-                    Logger.fileTransfer.error("Could not acquire metadata of item with identifier: \(ocId, privacy: .public), cannot correctly inform of modification")
-                    renameError = NSFileProviderError(.noSuchItem)
-                    dispatchGroup.leave()
-                    return
-                }
+                    guard let newMetadata = dbManager.itemMetadataFromOcId(ocId) else {
+                        Logger.fileTransfer.error("Could not acquire metadata of item with identifier: \(ocId, privacy: .public), cannot correctly inform of modification")
+                        renameError = NSFileProviderError(.noSuchItem)
+                        moveFileOrFolderDispatchGroup.leave()
+                        return
+                    }
 
-                modifiedItem = FileProviderItem(metadata: newMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit)
-                dispatchGroup.leave()
-            }
+                    modifiedItem = FileProviderItem(metadata: newMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit)
+                    moveFileOrFolderDispatchGroup.leave()
+                }
 
-            dispatchGroup.wait()
+                moveFileOrFolderDispatchGroup.wait()
 
-            guard renameError == nil else {
-                Logger.fileTransfer.error("Stopping rename of item with ocId \(ocId, privacy: .public) due to error: \(renameError, privacy: .public)")
-                completionHandler(modifiedItem, [], false, renameError)
-                return Progress()
-            }
+                guard renameError == nil else {
+                    Logger.fileTransfer.error("Stopping rename of item with ocId \(ocId, privacy: .public) due to error: \(renameError, privacy: .public)")
+                    completionHandler(modifiedItem, [], false, renameError)
+                    return
+                }
 
-            guard !itemTemplateIsFolder else {
-                Logger.fileTransfer.debug("Only handling renaming for folders. ocId: \(ocId, privacy: .public)")
-                completionHandler(modifiedItem, [], false, nil)
-                return Progress()
+                guard !itemTemplateIsFolder else {
+                    Logger.fileTransfer.debug("Only handling renaming for folders. ocId: \(ocId, privacy: .public)")
+                    completionHandler(modifiedItem, [], false, nil)
+                    return
+                }
             }
         }
 
@@ -461,74 +466,76 @@ class FileProviderExtension: NSObject, NSFileProviderReplicatedExtension, NKComm
         let progress = Progress()
 
         if changedFields.contains(.contents) {
-            Logger.fileProviderExtension.debug("Item modification for \(item.itemIdentifier.rawValue, privacy: .public) \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)) includes contents")
-
-            guard newContents != nil else {
-                Logger.fileProviderExtension.warning("WARNING. Could not upload modified contents as was provided nil contents url. ocId: \(item.itemIdentifier.rawValue, privacy: .public)")
-                completionHandler(modifiedItem, [], false, NSFileProviderError(.noSuchItem))
-                return Progress()
-            }
-
-            let ocId = item.itemIdentifier.rawValue
-            guard let metadata = dbManager.itemMetadataFromOcId(ocId) else {
-                Logger.fileProviderExtension.error("Could not acquire metadata of item with identifier: \(ocId, privacy: .public)")
-                completionHandler(item, NSFileProviderItemFields(), false, NSFileProviderError(.noSuchItem))
-                return Progress()
-            }
-
-            dbManager.setStatusForItemMetadata(metadata, status: NextcloudItemMetadataTable.Status.uploading) { updatedMetadata in
+            dispatchQueue.async {
+                Logger.fileProviderExtension.debug("Item modification for \(item.itemIdentifier.rawValue, privacy: .public) \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)) includes contents")
 
-                if updatedMetadata == nil {
-                    Logger.fileProviderExtension.warning("Could not acquire updated metadata of item with identifier: \(ocId, privacy: .public), unable to update item status to uploading")
+                guard newContents != nil else {
+                    Logger.fileProviderExtension.warning("WARNING. Could not upload modified contents as was provided nil contents url. ocId: \(item.itemIdentifier.rawValue, privacy: .public)")
+                    completionHandler(modifiedItem, [], false, NSFileProviderError(.noSuchItem))
+                    return
                 }
 
-                self.ncKit.upload(serverUrlFileName: newServerUrlFileName,
-                                  fileNameLocalPath: fileNameLocalPath,
-                                  requestHandler: { request in
-                    progress.setHandlersFromAfRequest(request)
-                }, taskHandler: { task in
-                    NSFileProviderManager(for: self.domain)?.register(task, forItemWithIdentifier: item.itemIdentifier, completionHandler: { _ in })
-                }, progressHandler: { uploadProgress in
-                    uploadProgress.copyCurrentStateToProgress(progress)
-                }) { account, ocId, etag, date, size, _, _, error  in
-                    if error == .success, let ocId = ocId {
-                        Logger.fileProviderExtension.info("Successfully uploaded item with identifier: \(ocId, privacy: .public) and filename: \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash))")
-
-                        if size != item.documentSize as? Int64 {
-                            Logger.fileTransfer.warning("Created item upload reported as successful, but there are differences between the received file size (\(size, privacy: .public)) and the original file size (\(item.documentSize ?? 0))")
-                        }
-
-                        let newMetadata = NextcloudItemMetadataTable()
-                        newMetadata.date = (date ?? NSDate()) as Date
-                        newMetadata.etag = etag ?? ""
-                        newMetadata.account = account
-                        newMetadata.fileName = item.filename
-                        newMetadata.fileNameView = item.filename
-                        newMetadata.ocId = ocId
-                        newMetadata.size = size
-                        newMetadata.contentType = item.contentType?.preferredMIMEType ?? ""
-                        newMetadata.directory = itemTemplateIsFolder
-                        newMetadata.serverUrl = parentItemMetadata.serverUrl
-                        newMetadata.session = ""
-                        newMetadata.sessionError = ""
-                        newMetadata.sessionTaskIdentifier = 0
-                        newMetadata.status = NextcloudItemMetadataTable.Status.normal.rawValue
-
-                        dbManager.addLocalFileMetadataFromItemMetadata(newMetadata)
-                        dbManager.addItemMetadata(newMetadata)
-
-                        modifiedItem = FileProviderItem(metadata: newMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit)
-                        completionHandler(modifiedItem, [], false, nil)
-                    } else {
-                        Logger.fileTransfer.error("Could not upload item \(item.itemIdentifier.rawValue, privacy: .public) with filename: \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)), received error: \(error, privacy: .public)")
+                let ocId = item.itemIdentifier.rawValue
+                guard let metadata = dbManager.itemMetadataFromOcId(ocId) else {
+                    Logger.fileProviderExtension.error("Could not acquire metadata of item with identifier: \(ocId, privacy: .public)")
+                    completionHandler(item, NSFileProviderItemFields(), false, NSFileProviderError(.noSuchItem))
+                    return
+                }
 
-                        metadata.status = NextcloudItemMetadataTable.Status.uploadError.rawValue
-                        metadata.sessionError = error.errorDescription
+                dbManager.setStatusForItemMetadata(metadata, status: NextcloudItemMetadataTable.Status.uploading) { updatedMetadata in
 
-                        dbManager.addItemMetadata(metadata)
+                    if updatedMetadata == nil {
+                        Logger.fileProviderExtension.warning("Could not acquire updated metadata of item with identifier: \(ocId, privacy: .public), unable to update item status to uploading")
+                    }
 
-                        completionHandler(modifiedItem, [], false, error.toFileProviderError())
-                        return
+                    self.ncKit.upload(serverUrlFileName: newServerUrlFileName,
+                                      fileNameLocalPath: fileNameLocalPath,
+                                      requestHandler: { request in
+                        progress.setHandlersFromAfRequest(request)
+                    }, taskHandler: { task in
+                        NSFileProviderManager(for: self.domain)?.register(task, forItemWithIdentifier: item.itemIdentifier, completionHandler: { _ in })
+                    }, progressHandler: { uploadProgress in
+                        uploadProgress.copyCurrentStateToProgress(progress)
+                    }) { account, ocId, etag, date, size, _, _, error  in
+                        if error == .success, let ocId = ocId {
+                            Logger.fileProviderExtension.info("Successfully uploaded item with identifier: \(ocId, privacy: .public) and filename: \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash))")
+
+                            if size != item.documentSize as? Int64 {
+                                Logger.fileTransfer.warning("Created item upload reported as successful, but there are differences between the received file size (\(size, privacy: .public)) and the original file size (\(item.documentSize ?? 0))")
+                            }
+
+                            let newMetadata = NextcloudItemMetadataTable()
+                            newMetadata.date = (date ?? NSDate()) as Date
+                            newMetadata.etag = etag ?? ""
+                            newMetadata.account = account
+                            newMetadata.fileName = item.filename
+                            newMetadata.fileNameView = item.filename
+                            newMetadata.ocId = ocId
+                            newMetadata.size = size
+                            newMetadata.contentType = item.contentType?.preferredMIMEType ?? ""
+                            newMetadata.directory = itemTemplateIsFolder
+                            newMetadata.serverUrl = parentItemMetadata.serverUrl
+                            newMetadata.session = ""
+                            newMetadata.sessionError = ""
+                            newMetadata.sessionTaskIdentifier = 0
+                            newMetadata.status = NextcloudItemMetadataTable.Status.normal.rawValue
+
+                            dbManager.addLocalFileMetadataFromItemMetadata(newMetadata)
+                            dbManager.addItemMetadata(newMetadata)
+
+                            modifiedItem = FileProviderItem(metadata: newMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit)
+                            completionHandler(modifiedItem, [], false, nil)
+                        } else {
+                            Logger.fileTransfer.error("Could not upload item \(item.itemIdentifier.rawValue, privacy: .public) with filename: \(item.filename, privacy: OSLogPrivacy.auto(mask: .hash)), received error: \(error.error, privacy: .public)")
+
+                            metadata.status = NextcloudItemMetadataTable.Status.uploadError.rawValue
+                            metadata.sessionError = error.errorDescription
+
+                            dbManager.addItemMetadata(metadata)
+
+                            completionHandler(modifiedItem, [], false, error.toFileProviderError())
+                            return
+                        }
                     }
                 }
             }