fix: Prevent starting the same file download twice

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 {

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

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
+}

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<NCChatFileStatus, ChatFileDownloadError>) {
+        DispatchQueue.main.async { [self] in
+            fileControllers.removeValue(forKey: fileId)
+
+            guard let handlers = completionHandlers.removeValue(forKey: fileId) else { return }
+
+            for handler in handlers {
+                handler(result)
+            }
+        }
+    }
+}

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<NCChatFileStatus, ChatFileDownloadError>) -> 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,18 +178,21 @@ 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
             guard !self.isCancelled else { return }
 
             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<NCChatFileStatus, ChatFileDownloadError>) {
+        let completionHandler = self.completionHandler
+        self.completionHandler = nil
+
+        completionHandler?(result)
+    }
+
     private func didChangeIsDownloadingNotification(isDownloading: Bool) {
         guard let fileStatus else { return }