From dfadb1d5f7c5c83b7da73dfc0d3b83a25adcb4e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Mon, 30 Dec 2024 10:19:57 +0100 Subject: [PATCH 1/3] refactor(DownloadOperation): Made a dedicated DownloadPublicShareOperation --- .../DownloadArchiveOperation.swift | 0 .../DownloadOperation.swift | 59 ++---------- .../DownloadPublicShareOperation.swift | 95 +++++++++++++++++++ .../Data/DownloadQueue/DownloadQueue.swift | 2 +- 4 files changed, 103 insertions(+), 53 deletions(-) rename kDriveCore/Data/DownloadQueue/{ => DownloadOperation}/DownloadArchiveOperation.swift (100%) rename kDriveCore/Data/DownloadQueue/{ => DownloadOperation}/DownloadOperation.swift (83%) create mode 100644 kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift diff --git a/kDriveCore/Data/DownloadQueue/DownloadArchiveOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadArchiveOperation.swift similarity index 100% rename from kDriveCore/Data/DownloadQueue/DownloadArchiveOperation.swift rename to kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadArchiveOperation.swift diff --git a/kDriveCore/Data/DownloadQueue/DownloadOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift similarity index 83% rename from kDriveCore/Data/DownloadQueue/DownloadOperation.swift rename to kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift index 2002c47cc..ef54e3646 100644 --- a/kDriveCore/Data/DownloadQueue/DownloadOperation.swift +++ b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift @@ -36,20 +36,19 @@ public class DownloadOperation: Operation, DownloadOperationable { // MARK: - Attributes private let fileManager = FileManager.default - private let driveFileManager: DriveFileManager - private let urlSession: FileDownloadSession - private let publicShareProxy: PublicShareProxy? - private let itemIdentifier: NSFileProviderItemIdentifier? - private var progressObservation: NSKeyValueObservation? private var backgroundTaskIdentifier: UIBackgroundTaskIdentifier = .invalid - @LazyInjectService(customTypeIdentifier: kDriveDBID.uploads) private var uploadsDatabase: Transactionable - + @LazyInjectService(customTypeIdentifier: kDriveDBID.uploads) var uploadsDatabase: Transactionable @LazyInjectService var accountManager: AccountManageable @LazyInjectService var driveInfosManager: DriveInfosManager @LazyInjectService var downloadManager: BackgroundDownloadSessionManager @LazyInjectService var appContextService: AppContextServiceable + let urlSession: FileDownloadSession + let driveFileManager: DriveFileManager + let itemIdentifier: NSFileProviderItemIdentifier? + var progressObservation: NSKeyValueObservation? + public let file: File public var task: URLSessionDownloadTask? public var error: DriveError? @@ -94,13 +93,11 @@ public class DownloadOperation: Operation, DownloadOperationable { file: File, driveFileManager: DriveFileManager, urlSession: FileDownloadSession, - publicShareProxy: PublicShareProxy? = nil, itemIdentifier: NSFileProviderItemIdentifier? = nil ) { self.file = File(value: file) self.driveFileManager = driveFileManager self.urlSession = urlSession - self.publicShareProxy = publicShareProxy self.itemIdentifier = itemIdentifier } @@ -112,7 +109,6 @@ public class DownloadOperation: Operation, DownloadOperationable { self.driveFileManager = driveFileManager self.urlSession = urlSession self.task = task - publicShareProxy = nil itemIdentifier = nil } @@ -179,48 +175,7 @@ public class DownloadOperation: Operation, DownloadOperationable { override public func main() { DDLogInfo("[DownloadOperation] Start for \(file.id) with session \(urlSession.identifier)") - if let publicShareProxy { - downloadPublicShareFile(publicShareProxy: publicShareProxy) - } else { - downloadFile() - } - } - - private func downloadPublicShareFile(publicShareProxy: PublicShareProxy) { - DDLogInfo("[DownloadOperation] Downloading publicShare \(file.id) with session \(urlSession.identifier)") - - let url = Endpoint.download(file: file, publicShareProxy: publicShareProxy).url - - // Add download task to Realm - let downloadTask = DownloadTask( - fileId: file.id, - isDirectory: file.isDirectory, - driveId: file.driveId, - userId: driveFileManager.drive.userId, - sessionId: urlSession.identifier, - sessionUrl: url.absoluteString - ) - - try? uploadsDatabase.writeTransaction { writableRealm in - writableRealm.add(downloadTask, update: .modified) - } - - let request = URLRequest(url: url) - task = urlSession.downloadTask(with: request, completionHandler: downloadCompletion) - progressObservation = task?.progress.observe(\.fractionCompleted, options: .new) { [fileId = file.id] _, value in - guard let newValue = value.newValue else { - return - } - DownloadQueue.instance.publishProgress(newValue, for: fileId) - } - if let itemIdentifier { - driveInfosManager.getFileProviderManager(for: driveFileManager.drive) { manager in - manager.register(self.task!, forItemWithIdentifier: itemIdentifier) { _ in - // META: keep SonarCloud happy - } - } - } - task?.resume() + downloadFile() } private func downloadFile() { diff --git a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift new file mode 100644 index 000000000..210678ac6 --- /dev/null +++ b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift @@ -0,0 +1,95 @@ +/* + Infomaniak kDrive - iOS App + Copyright (C) 2024 Infomaniak Network SA + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + */ + +import CocoaLumberjackSwift +import FileProvider +import Foundation +import InfomaniakCore +import InfomaniakCoreDB +import InfomaniakDI +import InfomaniakLogin + +public final class DownloadPublicShareOperation: DownloadOperation { + private let publicShareProxy: PublicShareProxy + + override public init( + file: File, + driveFileManager: DriveFileManager, + urlSession: FileDownloadSession, + itemIdentifier: NSFileProviderItemIdentifier? = nil + ) { + fatalError("Unavailable") + } + + public init( + file: File, + driveFileManager: DriveFileManager, + urlSession: FileDownloadSession, + publicShareProxy: PublicShareProxy, + itemIdentifier: NSFileProviderItemIdentifier? = nil + ) { + self.publicShareProxy = publicShareProxy + super.init(file: file, + driveFileManager: driveFileManager, + urlSession: urlSession, + itemIdentifier: itemIdentifier) + } + + override public func main() { + DDLogInfo("[DownloadPublicShareOperation] Start for \(file.id) with session \(urlSession.identifier)") + + downloadPublicShareFile(publicShareProxy: publicShareProxy) + } + + private func downloadPublicShareFile(publicShareProxy: PublicShareProxy) { + DDLogInfo("[DownloadPublicShareOperation] Downloading publicShare \(file.id) with session \(urlSession.identifier)") + + let url = Endpoint.download(file: file, publicShareProxy: publicShareProxy).url + + // Add download task to Realm + let downloadTask = DownloadTask( + fileId: file.id, + isDirectory: file.isDirectory, + driveId: file.driveId, + userId: driveFileManager.drive.userId, + sessionId: urlSession.identifier, + sessionUrl: url.absoluteString + ) + + try? uploadsDatabase.writeTransaction { writableRealm in + writableRealm.add(downloadTask, update: .modified) + } + + let request = URLRequest(url: url) + task = urlSession.downloadTask(with: request, completionHandler: downloadCompletion) + progressObservation = task?.progress.observe(\.fractionCompleted, options: .new) { [fileId = file.id] _, value in + guard let newValue = value.newValue else { + return + } + DownloadQueue.instance.publishProgress(newValue, for: fileId) + } + if let itemIdentifier { + driveInfosManager.getFileProviderManager(for: driveFileManager.drive) { manager in + manager.register(self.task!, forItemWithIdentifier: itemIdentifier) { _ in + // META: keep SonarCloud happy + } + } + } + task?.resume() + } +} diff --git a/kDriveCore/Data/DownloadQueue/DownloadQueue.swift b/kDriveCore/Data/DownloadQueue/DownloadQueue.swift index 0fd65e28b..d9fccc088 100644 --- a/kDriveCore/Data/DownloadQueue/DownloadQueue.swift +++ b/kDriveCore/Data/DownloadQueue/DownloadQueue.swift @@ -127,7 +127,7 @@ public final class DownloadQueue: ParallelismHeuristicDelegate { OperationQueueHelper.disableIdleTimer(true) - let operation = DownloadOperation( + let operation = DownloadPublicShareOperation( file: file, driveFileManager: driveFileManager, urlSession: self.bestSession, From 0afa9b38243e64e7b4982ddd76b1c944ee39bcf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Mon, 30 Dec 2024 10:37:08 +0100 Subject: [PATCH 2/3] refactor(DownloadOperation): Factorised download request --- .../DownloadOperation/DownloadOperation.swift | 34 +++++++++++-------- .../DownloadPublicShareOperation.swift | 16 +-------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift index ef54e3646..606fa665b 100644 --- a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift +++ b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift @@ -200,27 +200,31 @@ public class DownloadOperation: Operation, DownloadOperationable { if let token = getToken() { var request = URLRequest(url: url) request.setValue("Bearer \(token.accessToken)", forHTTPHeaderField: "Authorization") - task = urlSession.downloadTask(with: request, completionHandler: downloadCompletion) - progressObservation = task?.progress.observe(\.fractionCompleted, options: .new) { [fileId = file.id] _, value in - guard let newValue = value.newValue else { - return - } - DownloadQueue.instance.publishProgress(newValue, for: fileId) - } - if let itemIdentifier { - driveInfosManager.getFileProviderManager(for: driveFileManager.drive) { manager in - manager.register(self.task!, forItemWithIdentifier: itemIdentifier) { _ in - // META: keep SonarCloud happy - } - } - } - task?.resume() + downloadRequest(request) } else { error = .unknownToken // Other error? end(sessionUrl: url) } } + func downloadRequest(_ request: URLRequest) { + task = urlSession.downloadTask(with: request, completionHandler: downloadCompletion) + progressObservation = task?.progress.observe(\.fractionCompleted, options: .new) { [fileId = file.id] _, value in + guard let newValue = value.newValue else { + return + } + DownloadQueue.instance.publishProgress(newValue, for: fileId) + } + if let itemIdentifier { + driveInfosManager.getFileProviderManager(for: driveFileManager.drive) { manager in + manager.register(self.task!, forItemWithIdentifier: itemIdentifier) { _ in + // META: keep SonarCloud happy + } + } + } + task?.resume() + } + override public func cancel() { DDLogInfo("[DownloadOperation] Download of \(file.id) canceled") super.cancel() diff --git a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift index 210678ac6..28d755993 100644 --- a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift +++ b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadPublicShareOperation.swift @@ -76,20 +76,6 @@ public final class DownloadPublicShareOperation: DownloadOperation { } let request = URLRequest(url: url) - task = urlSession.downloadTask(with: request, completionHandler: downloadCompletion) - progressObservation = task?.progress.observe(\.fractionCompleted, options: .new) { [fileId = file.id] _, value in - guard let newValue = value.newValue else { - return - } - DownloadQueue.instance.publishProgress(newValue, for: fileId) - } - if let itemIdentifier { - driveInfosManager.getFileProviderManager(for: driveFileManager.drive) { manager in - manager.register(self.task!, forItemWithIdentifier: itemIdentifier) { _ in - // META: keep SonarCloud happy - } - } - } - task?.resume() + downloadRequest(request) } } From 42a8b052d8ca545d5f2105af6974e2a1965db807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Mon, 30 Dec 2024 11:12:46 +0100 Subject: [PATCH 3/3] chore: PR Feedback --- .../DownloadOperation/DownloadOperation.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift index 606fa665b..8e104f79f 100644 --- a/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift +++ b/kDriveCore/Data/DownloadQueue/DownloadOperation/DownloadOperation.swift @@ -36,6 +36,7 @@ public class DownloadOperation: Operation, DownloadOperationable { // MARK: - Attributes private let fileManager = FileManager.default + private let itemIdentifier: NSFileProviderItemIdentifier? private var backgroundTaskIdentifier: UIBackgroundTaskIdentifier = .invalid @LazyInjectService(customTypeIdentifier: kDriveDBID.uploads) var uploadsDatabase: Transactionable @@ -46,7 +47,6 @@ public class DownloadOperation: Operation, DownloadOperationable { let urlSession: FileDownloadSession let driveFileManager: DriveFileManager - let itemIdentifier: NSFileProviderItemIdentifier? var progressObservation: NSKeyValueObservation? public let file: File @@ -301,7 +301,10 @@ public class DownloadOperation: Operation, DownloadOperationable { return } - assert(file.isDownloaded, "Expecting to be downloaded at the end of the downloadOperation error:\(error)") + assert( + file.isDownloaded, + "Expecting to be downloaded at the end of the downloadOperation error:\(String(describing: error))" + ) try? uploadsDatabase.writeTransaction { writableRealm in guard let task = writableRealm.objects(DownloadTask.self)