From 9083f982d942a16844d23575de8ab9ebcbbdc61e Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Tue, 14 Mar 2023 23:15:02 +0100 Subject: [PATCH] Use completion handler to retrieve updated metadata after status change, fixing race condition in FileProviderDatabaseManager Signed-off-by: Claudio Cambra --- .../NextcloudFilesDatabaseManager.swift | 13 ++-- .../FileProviderExtension.swift | 77 ++++++++++--------- 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Database/NextcloudFilesDatabaseManager.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Database/NextcloudFilesDatabaseManager.swift index 1215f4c6a..df3616626 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Database/NextcloudFilesDatabaseManager.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Database/NextcloudFilesDatabaseManager.swift @@ -194,29 +194,26 @@ class NextcloudFilesDatabaseManager : NSObject { } } - func setStatusForItemMetadata(_ metadata: NextcloudItemMetadataTable, status: NextcloudItemMetadataTable.Status) -> NextcloudItemMetadataTable? { + func setStatusForItemMetadata(_ metadata: NextcloudItemMetadataTable, status: NextcloudItemMetadataTable.Status, completionHandler: @escaping(_ updatedMetadata: NextcloudItemMetadataTable?) -> Void) { let database = ncDatabase() - var result: NextcloudItemMetadataTable? do { try database.write { guard let result = database.objects(NextcloudItemMetadataTable.self).filter("ocId == %@", metadata.ocId).first else { + Logger.ncFilesDatabase.debug("Did not update status for item metadata as it was not found. ocID: \(metadata.ocId, privacy: .public)") return } result.status = status.rawValue database.add(result, update: .all) Logger.ncFilesDatabase.debug("Updated status for item metadata. ocID: \(metadata.ocId, privacy: .public), etag: \(metadata.etag, privacy: .public), fileName: \(metadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash))") + + completionHandler(NextcloudItemMetadataTable(value: result)) } } catch let error { Logger.ncFilesDatabase.error("Could not update status for item metadata with ocID: \(metadata.ocId, privacy: .public), etag: \(metadata.etag, privacy: .public), fileName: \(metadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash)), received error: \(error, privacy: .public)") + completionHandler(nil) } - - if result != nil { - return NextcloudItemMetadataTable(value: result!) - } - - return nil } func addItemMetadata(_ metadata: NextcloudItemMetadataTable) { diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift index 26787b4dc..18c7008c2 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift @@ -149,49 +149,52 @@ class FileProviderExtension: NSObject, NSFileProviderReplicatedExtension, NKComm do { let fileNameLocalPath = try localPathForNCFile(ocId: metadata.ocId, fileNameView: metadata.fileNameView) - guard let updatedMetadata = dbManager.setStatusForItemMetadata(metadata, status: NextcloudItemMetadataTable.Status.downloading) else { - Logger.fileProviderExtension.error("Could not acquire updated metadata of item with identifier: \(itemIdentifier.rawValue, privacy: .public)") - completionHandler(nil, nil, NSFileProviderError(.noSuchItem)) - return Progress() - } + dbManager.setStatusForItemMetadata(metadata, status: NextcloudItemMetadataTable.Status.downloading) { updatedMetadata in - self.ncKit.download(serverUrlFileName: serverUrlFileName, - fileNameLocalPath: fileNameLocalPath.path, - requestHandler: { _ in + guard let updatedMetadata = updatedMetadata else { + Logger.fileProviderExtension.error("Could not acquire updated metadata of item with identifier: \(itemIdentifier.rawValue, privacy: .public), unable to update item status to downloading") + completionHandler(nil, nil, NSFileProviderError(.noSuchItem)) + return + } - }, taskHandler: { task in - self.outstandingSessionTasks[serverUrlFileName] = task - NSFileProviderManager(for: self.domain)?.register(task, forItemWithIdentifier: itemIdentifier, completionHandler: { _ in }) - }, progressHandler: { downloadProgress in - downloadProgress.copyCurrentStateToProgress(progress) - }) { _, etag, date, _, _, _, error in - self.outstandingSessionTasks.removeValue(forKey: serverUrlFileName) - - if error == .success { - Logger.fileTransfer.debug("Acquired contents of item with identifier: \(itemIdentifier.rawValue, privacy: .public) and filename: \(updatedMetadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash))") - updatedMetadata.status = NextcloudItemMetadataTable.Status.normal.rawValue - updatedMetadata.date = (date ?? NSDate()) as Date - updatedMetadata.etag = etag ?? "" - - dbManager.addLocalFileMetadataFromItemMetadata(updatedMetadata) - dbManager.addItemMetadata(updatedMetadata) - - guard let parentItemIdentifier = dbManager.parentItemIdentifierFromMetadata(updatedMetadata) else { - completionHandler(nil, nil, NSFileProviderError(.noSuchItem)) - return - } - let fpItem = FileProviderItem(metadata: updatedMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit) + self.ncKit.download(serverUrlFileName: serverUrlFileName, + fileNameLocalPath: fileNameLocalPath.path, + requestHandler: { _ in + + }, taskHandler: { task in + self.outstandingSessionTasks[serverUrlFileName] = task + NSFileProviderManager(for: self.domain)?.register(task, forItemWithIdentifier: itemIdentifier, completionHandler: { _ in }) + }, progressHandler: { downloadProgress in + downloadProgress.copyCurrentStateToProgress(progress) + }) { _, etag, date, _, _, _, error in + self.outstandingSessionTasks.removeValue(forKey: serverUrlFileName) + + if error == .success { + Logger.fileTransfer.debug("Acquired contents of item with identifier: \(itemIdentifier.rawValue, privacy: .public) and filename: \(updatedMetadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash))") + updatedMetadata.status = NextcloudItemMetadataTable.Status.normal.rawValue + updatedMetadata.date = (date ?? NSDate()) as Date + updatedMetadata.etag = etag ?? "" + + dbManager.addLocalFileMetadataFromItemMetadata(updatedMetadata) + dbManager.addItemMetadata(updatedMetadata) + + guard let parentItemIdentifier = dbManager.parentItemIdentifierFromMetadata(updatedMetadata) else { + completionHandler(nil, nil, NSFileProviderError(.noSuchItem)) + return + } + let fpItem = FileProviderItem(metadata: updatedMetadata, parentItemIdentifier: parentItemIdentifier, ncKit: self.ncKit) - completionHandler(fileNameLocalPath, fpItem, nil) - } else { - Logger.fileTransfer.error("Could not acquire contents of item with identifier: \(itemIdentifier.rawValue, privacy: .public) and fileName: \(updatedMetadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash))") + completionHandler(fileNameLocalPath, fpItem, nil) + } else { + Logger.fileTransfer.error("Could not acquire contents of item with identifier: \(itemIdentifier.rawValue, privacy: .public) and fileName: \(updatedMetadata.fileName, privacy: OSLogPrivacy.auto(mask: .hash))") - updatedMetadata.status = NextcloudItemMetadataTable.Status.downloadError.rawValue - updatedMetadata.sessionError = error.errorDescription + updatedMetadata.status = NextcloudItemMetadataTable.Status.downloadError.rawValue + updatedMetadata.sessionError = error.errorDescription - dbManager.addItemMetadata(updatedMetadata) + dbManager.addItemMetadata(updatedMetadata) - completionHandler(nil, nil, error.toFileProviderError()) + completionHandler(nil, nil, error.toFileProviderError()) + } } } } catch let error { -- 2.30.2