diff --git a/NextcloudTalk/Chat/BaseChatViewController.swift b/NextcloudTalk/Chat/BaseChatViewController.swift index f96889154..9ba64b0b3 100644 --- a/NextcloudTalk/Chat/BaseChatViewController.swift +++ b/NextcloudTalk/Chat/BaseChatViewController.swift @@ -28,7 +28,6 @@ import Toast ShareViewControllerDelegate, QLPreviewControllerDelegate, QLPreviewControllerDataSource, - NCChatFileControllerDelegate, ShareConfirmationViewControllerDelegate, AVAudioRecorderDelegate, AVAudioPlayerDelegate, @@ -90,8 +89,6 @@ import Toast private var isVoiceRecordingLocked = false - private var actionTypeTranscribeVoiceMessage = "transcribe-voice-message" - private var imagePicker: UIImagePickerController? private var stopTypingTimer: Timer? @@ -1425,11 +1422,9 @@ import Toast } func didPressTranscribeVoiceMessage(for message: NCChatMessage) { - let downloader = NCChatFileController(account: self.account) - downloader.delegate = self - downloader.messageType = kMessageTypeVoiceMessage - downloader.actionType = actionTypeTranscribeVoiceMessage - downloader.downloadFile(withFileId: message.file().parameterId) + self.downloadFile(for: message) { [weak self] fileStatus in + self?.transcribeVoiceMessage(with: fileStatus) + } } func didPressEdit(for message: NCChatMessage) { @@ -4064,14 +4059,9 @@ import Toast } } - if fileParameter.fileStatus != nil && fileParameter.fileStatus?.isDownloading ?? false { - print("File already downloading -> skipping new download") - return + self.downloadFile(for: message) { [weak self] fileStatus in + self?.previewFile(with: fileStatus) } - - let downloader = NCChatFileController(account: self.account) - downloader.delegate = self - downloader.downloadFile(withFileId: fileParameter.parameterId) } public func cellHasDownloadedImagePreview(withSize size: CGSize, for message: NCChatMessage) { @@ -4109,11 +4099,6 @@ import Toast return } - if fileParameter.fileStatus != nil && fileParameter.fileStatus?.isDownloading ?? false { - print("File already downloading -> skipping new download") - return - } - // Resume an already loaded voice message if let voiceMessagesPlayer = self.voiceMessagesPlayer, let playerAudioFileStatus = self.playerAudioFileStatus, @@ -4134,10 +4119,9 @@ import Toast return } - let downloader = NCChatFileController(account: self.account) - downloader.delegate = self - downloader.messageType = kMessageTypeVoiceMessage - downloader.downloadFile(withFileId: fileParameter.parameterId) + self.downloadFile(for: message) { [weak self] fileStatus in + self?.setupVoiceMessagePlayer(with: fileStatus) + } } public func cellWants(toPauseAudioFile fileParameter: NCMessageFileParameter) { @@ -4295,19 +4279,41 @@ import Toast // Do nothing -> override in subclass } - // MARK: - NCChatFileControllerDelegate + // MARK: - File downloads - public func fileControllerDidLoadFile(_ fileController: NCChatFileController, with fileStatus: NCChatFileStatus) { - if fileController.messageType == kMessageTypeVoiceMessage { - if fileController.actionType == actionTypeTranscribeVoiceMessage { - self.transcribeVoiceMessage(with: fileStatus) - } else { - self.setupVoiceMessagePlayer(with: fileStatus) - } + /// Downloads the file of a message and hands it to `completionHandler`, or shows why it failed. + /// + /// Downloads are deduplicated by file id, so requesting the same file twice downloads it once + /// and calls both handlers. + /// + @MainActor + private func downloadFile(for message: NCChatMessage, completionHandler: @escaping (_ fileStatus: NCChatFileStatus) -> Void) { + guard let fileParameter = message.file() else { return } - return + ChatFileDownloader.shared.downloadFile(withFileId: fileParameter.parameterId, fromAccount: self.account) { [weak self] result in + guard let self else { return } + + switch result { + case .success(let fileStatus): + completionHandler(fileStatus) + case .failure(.fileUnavailable(let errorDescription)), .failure(.downloadFailed(let errorDescription)): + self.showUnableToLoadFileAlert(with: errorDescription) + case .failure(.cancelled): + break + } } + } + private func showUnableToLoadFileAlert(with errorDescription: String) { + let alert = UIAlertController(title: NSLocalizedString("Unable to load file", comment: ""), + message: errorDescription, + preferredStyle: .alert) + + alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), style: .default)) + NCUserInterfaceController.sharedInstance().presentAlertViewController(alert) + } + + private func previewFile(with fileStatus: NCChatFileStatus) { if self.isPreviewControllerShown { // We are showing a file already, no need to open another one return @@ -4373,15 +4379,6 @@ import Toast } } - public func fileControllerDidFailLoadingFile(_ fileController: NCChatFileController, withFileId fileId: String, withErrorDescription errorDescription: String) { - let alert = UIAlertController(title: NSLocalizedString("Unable to load file", comment: ""), - message: errorDescription, - preferredStyle: .alert) - - alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), style: .default)) - NCUserInterfaceController.sharedInstance().presentAlertViewController(alert) - } - // MARK: - QLPreviewControllerDelegate/DataSource public func numberOfPreviewItems(in controller: QLPreviewController) -> Int { diff --git a/NextcloudTalk/Chat/Chat cells/BaseChatTableViewCell+File.swift b/NextcloudTalk/Chat/Chat cells/BaseChatTableViewCell+File.swift index 2820803af..3e559d9d6 100644 --- a/NextcloudTalk/Chat/Chat cells/BaseChatTableViewCell+File.swift +++ b/NextcloudTalk/Chat/Chat cells/BaseChatTableViewCell+File.swift @@ -97,14 +97,8 @@ extension BaseChatTableViewCell { self.requestPreview(for: message, with: account) - if !message.sendingFailed { - if message.isTemporary { - self.addActivityIndicator(with: 0) - } else if let fileStatus = message.file().fileStatus { - if fileStatus.isDownloading, fileStatus.downloadProgress < 1 { - self.addActivityIndicator(with: Float(fileStatus.downloadProgress)) - } - } + if !message.sendingFailed, message.isTemporary { + self.addActivityIndicator(with: 0) } if let contactImage = message.file().contactPhotoImage { @@ -231,11 +225,11 @@ extension BaseChatTableViewCell { } private func downloadGifPreview(for message: NCChatMessage, withFileId fileId: String, cacheKey: String, with account: TalkAccount) { - NCChatFileControllerWrapper.shared.downloadFile(withFileId: fileId, fromAccount: account) { [weak self] fileLocalPath in + ChatFileDownloader.shared.downloadFile(withFileId: fileId, fromAccount: account) { [weak self] result in // Delivered on the main thread, so check we are still the same cell before doing any work guard let self, self.message?.file()?.parameterId == fileId else { return } - guard let fileLocalPath else { + guard let fileLocalPath = try? result.get().fileLocalPath else { // No file, try to request a normal preview self.requestDefaultPreview(for: message, withPlaceholderImage: nil, with: account) return diff --git a/NextcloudTalk/Chat/ChatFileDownloadError.swift b/NextcloudTalk/Chat/ChatFileDownloadError.swift new file mode 100644 index 000000000..accb2652a --- /dev/null +++ b/NextcloudTalk/Chat/ChatFileDownloadError.swift @@ -0,0 +1,22 @@ +// +// SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: GPL-3.0-or-later +// + +import Foundation + +/// Reasons for downloading a file of a conversation to fail. +/// +/// Deliberately without user facing messages: how a failed download is reported depends on the +/// caller, which knows whether it can show an alert, fall back to a preview or nothing at all. +public enum ChatFileDownloadError: Error { + + /// The file could not be found on the server, or its metadata could not be read. + case fileUnavailable(errorDescription: String) + + /// Downloading the file from the server failed. + case downloadFailed(errorDescription: String) + + /// The download was cancelled by the caller. + case cancelled +} diff --git a/NextcloudTalk/Chat/ChatFileDownloader.swift b/NextcloudTalk/Chat/ChatFileDownloader.swift new file mode 100644 index 000000000..9eea75f83 --- /dev/null +++ b/NextcloudTalk/Chat/ChatFileDownloader.swift @@ -0,0 +1,46 @@ +// +// SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors +// SPDX-License-Identifier: GPL-3.0-or-later +// + +import Foundation + +/// Downloads files of conversations, sharing one download between all callers asking for the same file. +/// +/// Does not offer cancellation on purpose: the download is shared, so one caller losing interest +/// must not stop it for the others. +public class ChatFileDownloader: NSObject { + typealias FileId = String + + private var completionHandlers = [FileId: [NCChatFileController.CompletionHandler]]() + private var fileControllers = [FileId: NCChatFileController]() + + static let shared = ChatFileDownloader() + + @MainActor + public func downloadFile(withFileId fileId: String, fromAccount account: TalkAccount, completionHandler: @escaping NCChatFileController.CompletionHandler) { + completionHandlers[fileId, default: []].append(completionHandler) + + // We are already downloading this file, don't do it again, the handler above is called once it finishes + guard fileControllers[fileId] == nil else { return } + + let fileController = NCChatFileController(account: account) + fileControllers[fileId] = fileController + + fileController.downloadFile(withFileId: fileId) { [weak self] result in + self?.executeCompletionHandlers(forFileId: fileId, with: result) + } + } + + private func executeCompletionHandlers(forFileId fileId: String, with result: Result) { + DispatchQueue.main.async { [self] in + fileControllers.removeValue(forKey: fileId) + + guard let handlers = completionHandlers.removeValue(forKey: fileId) else { return } + + for handler in handlers { + handler(result) + } + } + } +} diff --git a/NextcloudTalk/Chat/NCChatFileController.swift b/NextcloudTalk/Chat/NCChatFileController.swift index a7e85050b..46114b986 100644 --- a/NextcloudTalk/Chat/NCChatFileController.swift +++ b/NextcloudTalk/Chat/NCChatFileController.swift @@ -11,23 +11,18 @@ public extension NSNotification.Name { static let NCChatFileControllerDidChangeDownloadProgress = NSNotification.Name("NCChatFileControllerDidChangeDownloadProgressNotification") } -public protocol NCChatFileControllerDelegate: AnyObject { - func fileControllerDidLoadFile(_ fileController: NCChatFileController, with fileStatus: NCChatFileStatus) - func fileControllerDidFailLoadingFile(_ fileController: NCChatFileController, withFileId fileId: String, withErrorDescription errorDescription: String) -} - public class NCChatFileController: NSObject { - public weak var delegate: NCChatFileControllerDelegate? + /// Called exactly once with the downloaded file, or with the reason why it could not be downloaded. + public typealias CompletionHandler = (Result) -> Void - public var messageType: String? - public var actionType: String? public private(set) var tempDirectoryPath = "" private let account: TalkAccount private let deleteFilesOlderThanDays = 7 private var fileStatus: NCChatFileStatus? private var cancelDownloadHandler: (() -> Void)? + private var completionHandler: CompletionHandler? private var isCancelled = false init(account: TalkAccount) { @@ -174,7 +169,7 @@ public class NCChatFileController: NSObject { return URL(fileURLWithPath: filePath) } - // Stops an ongoing download. No delegate method is called afterwards. + // Stops an ongoing download. The completion handler is called with `.cancelled`. public func cancelDownload() { self.isCancelled = true self.cancelDownloadHandler?() @@ -183,10 +178,13 @@ public class NCChatFileController: NSObject { if self.fileStatus?.isDownloading == true { self.didChangeIsDownloadingNotification(isDownloading: false) } + + self.finish(with: .failure(.cancelled)) } - public func downloadFile(withFileId fileId: String) { + public func downloadFile(withFileId fileId: String, completionHandler: @escaping CompletionHandler) { self.isCancelled = false + self.completionHandler = completionHandler // getFileById already sets up NextcloudKit NCAPIController.sharedInstance().getFileById(forAccount: self.account, withFileId: fileId) { file, error in @@ -194,7 +192,7 @@ public class NCChatFileController: NSObject { guard let file else { print("An error occurred while getting file with fileId \(fileId): \(error?.errorDescription ?? "")") - self.delegate?.fileControllerDidFailLoadingFile(self, withFileId: fileId, withErrorDescription: error?.errorDescription ?? "") + self.finish(with: .failure(.fileUnavailable(errorDescription: error?.errorDescription ?? ""))) return } @@ -217,8 +215,8 @@ public class NCChatFileController: NSObject { if self.isFileInCache(fileLocalPath, withModificationDate: file.date as Date, withSize: file.size) { print("Found file in cache: \(fileLocalPath)") - self.delegate?.fileControllerDidLoadFile(self, with: fileStatus) self.didChangeIsDownloadingNotification(isDownloading: false) + self.finish(with: .success(fileStatus)) return } @@ -232,22 +230,30 @@ public class NCChatFileController: NSObject { guard !self.isCancelled else { return } + self.didChangeIsDownloadingNotification(isDownloading: false) + if error.errorCode == 0 { // Set modification date to invalidate our cache // Set creation date to delete older files from cache self.setDate(onFile: fileLocalPath, withCreationDate: Date(), withModificationDate: file.date as Date) - self.delegate?.fileControllerDidLoadFile(self, with: fileStatus) + self.finish(with: .success(fileStatus)) } else { print("Error downloading file: \(error.errorCode) - \(error.errorDescription)") - self.delegate?.fileControllerDidFailLoadingFile(self, withFileId: fileStatus.fileId, withErrorDescription: error.errorDescription) + self.finish(with: .failure(.downloadFailed(errorDescription: error.errorDescription))) } - - self.didChangeIsDownloadingNotification(isDownloading: false) } } } + /// Reports the outcome of a download, making sure the completion handler runs only once. + private func finish(with result: Result) { + let completionHandler = self.completionHandler + self.completionHandler = nil + + completionHandler?(result) + } + private func didChangeIsDownloadingNotification(isDownloading: Bool) { guard let fileStatus else { return } diff --git a/NextcloudTalk/Chat/NCChatFileControllerWrapper.swift b/NextcloudTalk/Chat/NCChatFileControllerWrapper.swift deleted file mode 100644 index df8b78de4..000000000 --- a/NextcloudTalk/Chat/NCChatFileControllerWrapper.swift +++ /dev/null @@ -1,59 +0,0 @@ -// -// SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors -// SPDX-License-Identifier: GPL-3.0-or-later -// - -import Foundation - -public class NCChatFileControllerWrapper: NSObject, NCChatFileControllerDelegate { - typealias FileId = String - typealias FileDownloadCompletionBlock = ((_ fileLocalPath: String?) -> Void) - - private var completionBlocks = [FileId: [FileDownloadCompletionBlock]]() - private var fileControllers = [FileId: NCChatFileController]() - - static let shared = NCChatFileControllerWrapper() - - @MainActor - public func downloadFile(withFileId fileId: String, fromAccount account: TalkAccount, completionBlock: @escaping (_ fileLocalPath: String?) -> Void) { - if var existingBlocks = completionBlocks[fileId] { - // We are already downloading this file, don't do it again, just ensure we call the completion block later on - existingBlocks.append(completionBlock) - completionBlocks[fileId] = existingBlocks - - return - } - - completionBlocks[fileId, default: []].append(completionBlock) - - let fileController = NCChatFileController(account: account) - fileController.delegate = self - - fileControllers[fileId] = fileController - - fileController.downloadFile(withFileId: fileId) - } - - private func executeCompletionBlocks(forFileId fileId: String, withPath fileLocalPath: String?) { - DispatchQueue.main.async { [self] in - if let existingBlocks = completionBlocks[fileId] { - for block in existingBlocks { - block(fileLocalPath) - } - - completionBlocks.removeValue(forKey: fileId) - } - - fileControllers.removeValue(forKey: fileId) - } - } - - public func fileControllerDidLoadFile(_ fileController: NCChatFileController, with fileStatus: NCChatFileStatus) { - executeCompletionBlocks(forFileId: fileStatus.fileId, withPath: fileStatus.fileLocalPath) - } - - public func fileControllerDidFailLoadingFile(_ fileController: NCChatFileController, withFileId fileId: String, withErrorDescription errorDescription: String) { - executeCompletionBlocks(forFileId: fileId, withPath: nil) - } - -} diff --git a/NextcloudTalk/Media Viewer/NCMediaViewerPageViewController.swift b/NextcloudTalk/Media Viewer/NCMediaViewerPageViewController.swift index 7fb6d6032..897f28a2a 100644 --- a/NextcloudTalk/Media Viewer/NCMediaViewerPageViewController.swift +++ b/NextcloudTalk/Media Viewer/NCMediaViewerPageViewController.swift @@ -15,7 +15,7 @@ import SwiftyGif @objc func mediaViewerPageStateDidChange(_ controller: NCMediaViewerPageViewController) } -@objcMembers class NCMediaViewerPageViewController: UIViewController, NCChatFileControllerDelegate, NCZoomableViewDelegate { +@objcMembers class NCMediaViewerPageViewController: UIViewController, NCZoomableViewDelegate { // What the user is looking at. Never goes back to a lower quality state on its own. private enum MediaState { @@ -191,8 +191,6 @@ import SwiftyGif self.zoomableView.replaceContentView(self.imageView) - fileDownloader.delegate = self - self.navigationItem.title = self.message.file()?.name NotificationCenter.default.addObserver(self, selector: #selector(didChangeDownloadProgress(notification:)), name: NSNotification.Name.NCChatFileControllerDidChangeDownloadProgress, object: nil) @@ -385,7 +383,19 @@ import SwiftyGif self.downloadDidFail = false self.updateProgressPresentation() - self.fileDownloader.downloadFile(withFileId: fileId) + self.fileDownloader.downloadFile(withFileId: fileId) { [weak self] result in + guard let self else { return } + + switch result { + case .success(let fileStatus): + self.didLoadFile(with: fileStatus) + case .failure(.fileUnavailable(let errorDescription)), .failure(.downloadFailed(let errorDescription)): + self.didFailLoadingFile(with: errorDescription) + case .failure(.cancelled): + // We cancelled this download ourselves and already updated our state + break + } + } } // MARK: - Display @@ -667,7 +677,7 @@ import SwiftyGif // MARK: - NCChatFileController delegate - func fileControllerDidLoadFile(_ fileController: NCChatFileController, with fileStatus: NCChatFileStatus) { + private func didLoadFile(with fileStatus: NCChatFileStatus) { self.isDownloading = false guard let localPath = fileStatus.fileLocalPath else { @@ -693,7 +703,7 @@ import SwiftyGif self.displayFile(at: url, isValidated: true) } - func fileControllerDidFailLoadingFile(_ fileController: NCChatFileController, withFileId fileId: String, withErrorDescription errorDescription: String) { + private func didFailLoadingFile(with errorDescription: String) { print("Error downloading picture: " + errorDescription) self.flushSharableFileHandlers(with: nil) diff --git a/NextcloudTalk/Rooms/RoomInfo/RoomInfoFileSection.swift b/NextcloudTalk/Rooms/RoomInfo/RoomInfoFileSection.swift index 921b8ba5d..9c582a1f0 100644 --- a/NextcloudTalk/Rooms/RoomInfo/RoomInfoFileSection.swift +++ b/NextcloudTalk/Rooms/RoomInfo/RoomInfoFileSection.swift @@ -64,10 +64,10 @@ struct RoomInfoFileSection: View { self.isDownloadingPreview = true - NCChatFileControllerWrapper.shared.downloadFile(withFileId: room.objectId, fromAccount: account) { @MainActor fileLocalPath in + ChatFileDownloader.shared.downloadFile(withFileId: room.objectId, fromAccount: account) { @MainActor result in self.isDownloadingPreview = false - guard let fileLocalPath else { return } + guard let fileLocalPath = try? result.get().fileLocalPath else { return } self.quickLookUrl = URL(fileURLWithPath: fileLocalPath) } diff --git a/NextcloudTalk/Rooms/RoomSharedItemsTableViewController.swift b/NextcloudTalk/Rooms/RoomSharedItemsTableViewController.swift index 80e271aad..e19611093 100644 --- a/NextcloudTalk/Rooms/RoomSharedItemsTableViewController.swift +++ b/NextcloudTalk/Rooms/RoomSharedItemsTableViewController.swift @@ -8,7 +8,6 @@ import QuickLook import PassKit @objcMembers class RoomSharedItemsTableViewController: UITableViewController, - NCChatFileControllerDelegate, QLPreviewControllerDelegate, QLPreviewControllerDataSource, VLCKitVideoViewControllerDelegate { @@ -262,12 +261,21 @@ import PassKit cell.fileParameter = file - let downloader = NCChatFileController(account: account) - downloader.delegate = self - downloader.downloadFile(withFileId: file.parameterId) + ChatFileDownloader.shared.downloadFile(withFileId: file.parameterId, fromAccount: account) { [weak self] result in + guard let self else { return } + + switch result { + case .success(let fileStatus): + self.didLoadFile(with: fileStatus) + case .failure(.fileUnavailable(let errorDescription)), .failure(.downloadFailed(let errorDescription)): + self.didFailLoadingFile(with: errorDescription) + case .failure(.cancelled): + break + } + } } - func fileControllerDidLoadFile(_ fileController: NCChatFileController, with fileStatus: NCChatFileStatus) { + private func didLoadFile(with fileStatus: NCChatFileStatus) { DispatchQueue.main.async { if self.isPreviewControllerShown { return @@ -309,7 +317,7 @@ import PassKit } } - func fileControllerDidFailLoadingFile(_ fileController: NCChatFileController, withFileId fileId: String, withErrorDescription errorDescription: String) { + private func didFailLoadingFile(with errorDescription: String) { let alertTitle = NSLocalizedString("Unable to load file", comment: "") let alert = UIAlertController( title: alertTitle,