From 1bb40d0a310b06231e505372b8ce7a30f0f622bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 08:19:05 +0100 Subject: [PATCH 1/8] chore: Remove legacy cache info in Sentry --- kDrive/UI/Controller/Menu/StorageTableViewController.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/kDrive/UI/Controller/Menu/StorageTableViewController.swift b/kDrive/UI/Controller/Menu/StorageTableViewController.swift index 454f76f9d..6bb4419ba 100644 --- a/kDrive/UI/Controller/Menu/StorageTableViewController.swift +++ b/kDrive/UI/Controller/Menu/StorageTableViewController.swift @@ -24,6 +24,7 @@ import kDriveResources import Kingfisher import UIKit + final class StorageTableViewController: UITableViewController { private enum Section: CaseIterable { case header, directories, files @@ -111,13 +112,9 @@ final class StorageTableViewController: UITableViewController { self.tableView.reloadData() } - // Check old size calculation and new one, send a sentry. Task { - let legacySize = cacheDirectories.reduce(0) { $0 + $1.size } - let metadata = [ "usedSize": usedSize, - "legacySize": legacySize, "appSize": appSize, "appGroupSize": appGroupSize ] From 8bbd1394ae1c4c2dbf97ebcb7db645a5fe0446dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 11:38:27 +0100 Subject: [PATCH 2/8] fix: Chunk generation is correctly done within a folder --- .../UploadQueue/Operation/UploadOperation+UploadChunk.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift index ed02699ab..095874efe 100644 --- a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift +++ b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift @@ -98,7 +98,7 @@ extension UploadOperation { // Write buffer let chunkName = chunkName(number: number, fileId: uploadFileId, hash: hash) - let chunkPath = tempChunkFolder.appendingPathExtension(chunkName) + let chunkPath = tempChunkFolder.appendingPathComponent(chunkName) try buffer.write(to: chunkPath, options: [.atomic]) Log.uploadOperation("wrote chunk:\(chunkPath) ufid:\(uploadFileId)") From b9a6fdac7fe5249f73764bbc3b9c09785989ee1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 13:19:10 +0100 Subject: [PATCH 3/8] fix: Store chunks at the root of NSTemporaryDirectory in order to not leave behind folders --- .../UploadOperation+UploadChunk.swift | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift index 095874efe..506ddbb05 100644 --- a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift +++ b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift @@ -87,18 +87,16 @@ extension UploadOperation { chunksToGenerateCount: chunksToGenerateCount) } - func storeChunk(_ buffer: Data, number: Int64, uploadFileId: String, sessionToken: String, + /// Store a chunk at the root of NSTemporaryDirectory with a stable name + func storeChunk(_ buffer: Data, number: Int64, + uploadFileId: String, + sessionToken: String, hash: String) throws -> URL { - // Create subfolders if needed - let tempChunkFolder = buildFolderPath(fileId: uploadFileId, sessionToken: sessionToken) - Log.uploadOperation("using chunk folder:'\(tempChunkFolder)' ufid:\(uploadFileId)") - if !fileManager.fileExists(atPath: tempChunkFolder.path, isDirectory: nil) { - try fileManager.createDirectory(at: tempChunkFolder, withIntermediateDirectories: true, attributes: nil) - } + // Store chunks at the root of NSTemporaryDirectory + let tempRoot = URL(fileURLWithPath: NSTemporaryDirectory()) + let chunkName = chunkName(number: number, fileId: uploadFileId, sessionToken: sessionToken, hash: hash) + let chunkPath = tempRoot.appendingPathComponent(chunkName) - // Write buffer - let chunkName = chunkName(number: number, fileId: uploadFileId, hash: hash) - let chunkPath = tempChunkFolder.appendingPathComponent(chunkName) try buffer.write(to: chunkPath, options: [.atomic]) Log.uploadOperation("wrote chunk:\(chunkPath) ufid:\(uploadFileId)") @@ -217,23 +215,12 @@ extension UploadOperation { } } - private func chunkName(number: Int64, fileId: String, hash: String) -> String { + private func chunkName(number: Int64, fileId: String, sessionToken: String, hash: String) -> String { // Hashing name as it can break path building. Also it keeps it short - let fileName = "upload_\(fileId)_\(hash)_\(number)".SHA256DigestString + let fileName = "upload_\(fileId)_\(hash)_\(number)_\(sessionToken)".SHA256DigestString return fileName + ".part" } - private func buildFolderPath(fileId: String, sessionToken: String) -> URL { - // NSTemporaryDirectory is perfect for this use case. - // Cleaned after ≈ 3 days, our session is valid 12h. - // https://cocoawithlove.com/2009/07/temporary-files-and-folders-in-cocoa.html - - // fileId and sessionToken can break the URL path, hashing makes sure it works - let folderUrlString = NSTemporaryDirectory() + "/\(fileId.SHA256DigestString)_\(sessionToken.SHA256DigestString)" - let folderPath = URL(fileURLWithPath: folderUrlString) - return folderPath.standardizedFileURL - } - /// Make sure all `uploadTasks` canceled or completed are up to date in database. /// - Parameter iterator: A view on `uploadTasks` func cleanUploadSessionUploadTaskNotUploading(iterator: inout Dictionary.Iterator) throws { From c37fee8218db426dbea72097c755b425dda75682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 13:19:29 +0100 Subject: [PATCH 4/8] fix: Make sure to clean the chunks right away --- .../Data/UploadQueue/Operation/UploadOperation.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/kDriveCore/Data/UploadQueue/Operation/UploadOperation.swift b/kDriveCore/Data/UploadQueue/Operation/UploadOperation.swift index 7b0ba81f0..69f00cec7 100644 --- a/kDriveCore/Data/UploadQueue/Operation/UploadOperation.swift +++ b/kDriveCore/Data/UploadQueue/Operation/UploadOperation.swift @@ -388,13 +388,15 @@ public final class UploadOperation: AsynchronousOperation, UploadOperationable { assertionFailure("unable to lookup chunk task id, ufid:\(self.uploadFileId)") } - // Some cleanup if we have the chance + // Cleanup chunks in storage if let path = chunkTask.path { let url = URL(fileURLWithPath: path, isDirectory: false) let chunkNumber = chunkTask.chunkNumber - DispatchQueue.global(qos: .background).async { - Log.uploadOperation("cleanup chunk:\(chunkNumber) ufid:\(self.uploadFileId)") - try? self.fileManager.removeItem(at: url) + Log.uploadOperation("cleanup chunk:\(chunkNumber) ufid:\(self.uploadFileId)") + do { + try self.fileManager.removeItem(at: url) + } catch { + Log.uploadOperation("failed to clean chunk \(error) ufid:\(self.uploadFileId)", level: .error) } } } notFound: { From f8e56d52f4c9a633228c047ac8630dd6fdb60b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 13:45:03 +0100 Subject: [PATCH 5/8] fix: The cleanup of the cache directory will happen earlier when space is missing. --- .../Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift index 550f5429b..67f6d0c00 100644 --- a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift +++ b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift @@ -185,7 +185,7 @@ public struct FreeSpaceService { } // Only clean if reaching the minimum space required for upload - guard freeSpaceInTemporaryDirectory < minimalSpaceRequiredForChunkUpload * 2 else { + guard freeSpaceInTemporaryDirectory < minimalSpaceRequiredForChunkUpload * 4 else { return } From 08d82406d3e4b05c0a3b4bd38e4e082fd90c3e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 13:52:03 +0100 Subject: [PATCH 6/8] feat: Clear cache folder not enough free space before we rebuild the upload queue --- kDriveCore/Data/UploadQueue/Queue/UploadQueue+Queue.swift | 5 +++++ .../UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/kDriveCore/Data/UploadQueue/Queue/UploadQueue+Queue.swift b/kDriveCore/Data/UploadQueue/Queue/UploadQueue+Queue.swift index de9cafcf4..e1ad324e8 100644 --- a/kDriveCore/Data/UploadQueue/Queue/UploadQueue+Queue.swift +++ b/kDriveCore/Data/UploadQueue/Queue/UploadQueue+Queue.swift @@ -20,6 +20,7 @@ import Algorithms import CocoaLumberjackSwift import Foundation import InfomaniakCore +import InfomaniakDI import RealmSwift // MARK: - Publish @@ -74,6 +75,10 @@ extension UploadQueue: UploadQueueable { public func rebuildUploadQueueFromObjectsInRealm(_ caller: StaticString = #function) { Log.uploadQueue("rebuildUploadQueueFromObjectsInRealm caller:\(caller)") concurrentQueue.sync { + // Clean cache if necessary before we try to restart the uploads. + @InjectService var freeSpaceService: FreeSpaceService + freeSpaceService.cleanCacheIfAlmostFull() + guard let uploadFileQuery else { Log.uploadQueue("\(#function) disabled in \(appContextService.context.rawValue)", level: .error) return diff --git a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift index 67f6d0c00..2cf4bca19 100644 --- a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift +++ b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift @@ -174,8 +174,10 @@ public struct FreeSpaceService { } } - /// On devices with low free space, we clear the temporaryDirectory on exit - private func cleanCacheIfAlmostFull() { + /// On devices with low free space, we clear the temporaryDirectory + /// + /// Safe to call before rebuilding the upload queue + public func cleanCacheIfAlmostFull() { let freeSpaceInTemporaryDirectory: Int64 do { freeSpaceInTemporaryDirectory = try freeSpace(url: Self.temporaryDirectoryURL) From 3e9a03146364785bd793c7048fb590665255256d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 6 Nov 2024 13:58:10 +0100 Subject: [PATCH 7/8] chore: PR Feedback --- kDrive/UI/Controller/Menu/StorageTableViewController.swift | 1 - .../Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/kDrive/UI/Controller/Menu/StorageTableViewController.swift b/kDrive/UI/Controller/Menu/StorageTableViewController.swift index 6bb4419ba..ea88efc6b 100644 --- a/kDrive/UI/Controller/Menu/StorageTableViewController.swift +++ b/kDrive/UI/Controller/Menu/StorageTableViewController.swift @@ -24,7 +24,6 @@ import kDriveResources import Kingfisher import UIKit - final class StorageTableViewController: UITableViewController { private enum Section: CaseIterable { case header, directories, files diff --git a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift index 2cf4bca19..33b45e770 100644 --- a/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift +++ b/kDriveCore/Data/UploadQueue/Servicies/FreeSpace/FreeSpaceService.swift @@ -175,7 +175,7 @@ public struct FreeSpaceService { } /// On devices with low free space, we clear the temporaryDirectory - /// + /// /// Safe to call before rebuilding the upload queue public func cleanCacheIfAlmostFull() { let freeSpaceInTemporaryDirectory: Int64 From 71b0c84501fc2f20579f27b77f9efbef4836ffee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Thu, 7 Nov 2024 13:17:35 +0100 Subject: [PATCH 8/8] chore: PR Feedback --- .../UploadQueue/Operation/UploadOperation+UploadChunk.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift index 506ddbb05..10596ad6c 100644 --- a/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift +++ b/kDriveCore/Data/UploadQueue/Operation/UploadOperation+UploadChunk.swift @@ -93,7 +93,7 @@ extension UploadOperation { sessionToken: String, hash: String) throws -> URL { // Store chunks at the root of NSTemporaryDirectory - let tempRoot = URL(fileURLWithPath: NSTemporaryDirectory()) + let tempRoot = FileManager.default.temporaryDirectory let chunkName = chunkName(number: number, fileId: uploadFileId, sessionToken: sessionToken, hash: hash) let chunkPath = tempRoot.appendingPathComponent(chunkName)