Use MuxUpload id instead if the input URL when looking up or writing state in the SDK (#33)

Keep MuxUpload id immutable

Avoid duplicating the id across MuxUpload and UploadInfo

Use XCTUnwrap to read persistence entries in tests

Add a no throw check when writing persistence entries in tests

Use XCTUnwrap when reading all

Expose init method internally to rest of SDK module for testing

Attempt to add an upload dedupe test

Use XCTUnwrap for constructing test URLs

Expose UploadManager init to rest of module for testing

Use the public init

Author: andrewjl-mux

Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift

@@ -48,7 +48,9 @@ public final class MuxUpload : Hashable, Equatable {
 
     private let uploadInfo: UploadInfo
     private let manageBySDK: Bool
-    private let id: String = UUID().uuidString
+    private var id: String {
+        uploadInfo.id
+    }
     private let uploadManager: UploadManager
 
     private var lastSeenStatus: Status = Status(progress: Progress(totalUnitCount: 0), updatedTime: 0, startTime: 0, isPaused: false)
@@ -96,6 +98,7 @@ public final class MuxUpload : Hashable, Equatable {
         optOutOfEventTracking: Bool = false
     ) {
         let uploadInfo = UploadInfo(
+            id: UUID().uuidString,
             uploadURL: uploadURL,
             videoFile: videoFileURL,
             chunkSize: chunkSize,
@@ -209,7 +212,7 @@ public final class MuxUpload : Hashable, Equatable {
      */
     public func cancel() {
         fileWorker?.cancel()
-        uploadManager.acknowledgeUpload(ofFile: videoFile)
+        uploadManager.acknowledgeUpload(id: id)
 
         lastSeenStatus = Status(progress: nil, updatedTime: 0, startTime: 0, isPaused: true)
         progressHandler = nil
@@ -273,14 +276,22 @@ public final class MuxUpload : Hashable, Equatable {
     }
 
 
-    private init (uploadInfo: UploadInfo, manage: Bool = true, uploadManager: UploadManager) {
+    internal init (
+        uploadInfo: UploadInfo,
+        manage: Bool = true,
+        uploadManager: UploadManager
+    ) {
         self.uploadInfo = uploadInfo
         self.manageBySDK = manage
         self.uploadManager = uploadManager
     }
 
     internal convenience init(wrapping uploader: ChunkedFileUploader, uploadManager: UploadManager) {
-        self.init(uploadInfo: uploader.uploadInfo, manage: true, uploadManager: uploadManager)
+        self.init(
+            uploadInfo: uploader.uploadInfo,
+            manage: true,
+            uploadManager: uploadManager
+        )
         self.fileWorker = uploader
 
         handleStateUpdate(uploader.currentState)

Sources/MuxUploadSDK/PublicAPI/UploadManager.swift

@@ -28,7 +28,7 @@ import Foundation
 ///
 public final class UploadManager {
 
-    private var uploadersByURL: [URL : ChunkedFileUploader] = [:]
+    private var uploadersByID: [String : ChunkedFileUploader] = [:]
     private var uploadsUpdateDelegatesByToken: [ObjectIdentifier : any UploadsUpdatedDelegate] = [:]
     private let uploadActor = UploadCacheActor()
     private lazy var uploaderDelegate: FileUploaderDelegate = FileUploaderDelegate(manager: self)
@@ -37,7 +37,12 @@ public final class UploadManager {
     /// to track and control its state
     /// Returns nil if there was no uplod in progress for thr given file
     public func findStartedUpload(ofFile url: URL) -> MuxUpload? {
-        if let uploader = uploadersByURL[url] {
+        if let uploader = Dictionary<URL, ChunkedFileUploader>(
+            uniqueKeysWithValues: uploadersByID.mapValues { value in
+                (value.uploadInfo.videoFile, value)
+            }
+            .values
+        )[url] {
             return MuxUpload(wrapping: uploader, uploadManager: self)
         } else {
             return nil
@@ -48,7 +53,7 @@ public final class UploadManager {
     /// Uploads are managed while in-progress or compelted.
     ///  Uploads become un-managed when canceled, or if the process dies after they complete
     public func allManagedUploads() -> [MuxUpload] {
-        return uploadersByURL.compactMap { (key, value) in MuxUpload(wrapping: value, uploadManager: self) }
+        return uploadersByID.compactMap { (key, value) in MuxUpload(wrapping: value, uploadManager: self) }
     }
 
     /// Attempts to resume an upload that was previously paused or interrupted by process death
@@ -94,23 +99,28 @@ public final class UploadManager {
         uploadsUpdateDelegatesByToken.removeValue(forKey: ObjectIdentifier(delegate))
     }
 
-    internal func acknowledgeUpload(ofFile url: URL) {
-        if let uploader = uploadersByURL[url] {
+    internal func acknowledgeUpload(id: String) {
+        if let uploader = uploadersByID[id] {
             uploader.cancel()
         }
-        uploadersByURL.removeValue(forKey: url)
+        uploadersByID.removeValue(forKey: id)
         Task.detached {
-            await self.uploadActor.remove(uploadAt:url)
+            await self.uploadActor.remove(uploadID: id)
             self.notifyDelegates()
         }
     }
 
     internal func findUploaderFor(videoFile url: URL) -> ChunkedFileUploader? {
-        return uploadersByURL[url]
+        return Dictionary<URL, ChunkedFileUploader>(
+            uniqueKeysWithValues: uploadersByID.mapValues { value in
+                (value.uploadInfo.videoFile, value)
+            }
+            .values
+        )[url]
     }
 
     internal func registerUploader(_ fileWorker: ChunkedFileUploader, withId id: String) {
-        uploadersByURL.updateValue(fileWorker, forKey: fileWorker.uploadInfo.videoFile)
+        uploadersByID.updateValue(fileWorker, forKey: fileWorker.uploadInfo.id)
         fileWorker.addDelegate(withToken: UUID().uuidString, uploaderDelegate)
         Task.detached {
             await self.uploadActor.updateUpload(
@@ -134,7 +144,7 @@ public final class UploadManager {
 
     /// The shared instance of this object that should be used
     public static let shared = UploadManager()
-    private init() { }
+    internal init() { }
 
     private struct FileUploaderDelegate : ChunkedFileUploaderDelegate {
         let manager: UploadManager
@@ -145,7 +155,7 @@ public final class UploadManager {
                 manager.notifyDelegates()
             }
             switch state {
-            case .success(_), .canceled: manager.acknowledgeUpload(ofFile: uploader.uploadInfo.videoFile)
+            case .success(_), .canceled: manager.acknowledgeUpload(id: uploader.uploadInfo.id)
             default: do { }
             }
         }
@@ -169,16 +179,31 @@ internal actor UploadCacheActor {
         }
     }
 
-    func getUpload(ofFileAt url: URL) async -> ChunkedFileUploader? {
+    func getUpload(uploadID: String) async -> ChunkedFileUploader? {
         // reminder: doesn't start the uploader, just makes it
         return await Task<ChunkedFileUploader?, Never> {
-            let optEntry = try? persistence.readEntry(forFileAt: url)
+            let optEntry = try? persistence.readEntry(uploadID: uploadID)
             guard let entry = optEntry else {
                 return nil
             }
             return ChunkedFileUploader(uploadInfo: entry.uploadInfo, startingAtByte: entry.lastSuccessfulByte)
         }.value
     }
+
+    func getUpload(ofFileAt: URL) async -> ChunkedFileUploader? {
+        return await Task<ChunkedFileUploader?, Never> {
+            guard let matchingEntry = try? persistence.readAll().filter({
+                $0.uploadInfo.uploadURL == ofFileAt
+            }).first else {
+                return nil
+            }
+
+            return ChunkedFileUploader(
+                uploadInfo: matchingEntry.uploadInfo,
+                startingAtByte: matchingEntry.lastSuccessfulByte
+            )
+        }.value
+    }
 
     func getAllUploads() async -> [ChunkedFileUploader] {
         return await Task<[ChunkedFileUploader]?, Never> {
@@ -188,9 +213,9 @@ internal actor UploadCacheActor {
         }.value ?? []
     }
 
-    func remove(uploadAt url: URL) async {
+    func remove(uploadID: String) async {
         await Task<(), Never> {
-            try? persistence.remove(entryAtAbsUrl: url)
+            try? persistence.remove(entryAtID: uploadID)
         }.value
     }
 

Sources/MuxUploadSDK/Upload/UploadInfo.swift

@@ -11,6 +11,8 @@ import Foundation
  Internal representation of a video upload
  */
 struct UploadInfo : Codable {
+
+    var id: String
     /**
      URI of the upload's destinatoin
      */

Sources/MuxUploadSDK/Upload/UploadPersistence.swift

@@ -15,16 +15,16 @@ class UploadPersistence {
     private static let ENTRY_TTL: TimeInterval = 3 * 24 * 60 * 60 // 3 days
 
     private let fileURL: URL
-    private var cache: [URL : PersistenceEntry]? // populated on first write for this object (see ensureCache())
+    private var cache: [String : PersistenceEntry]? // populated on first write for this object (see ensureCache())
     private let uploadsFile: UploadsFile
 
     func update(uploadState state: ChunkedFileUploader.InternalUploadState, forUpload upload: UploadInfo) {
         do {
             // If the new state is persistable, persist it (overwriting the old) otherwise delete it
             if let entry = PersistenceEntry.fromUploadState(state, forUpload: upload) {
-                try write(entry: entry, forFileAt: upload.videoFile)
+                try write(entry: entry, for: upload.id)
             } else {
-                try remove(entryAtAbsUrl: upload.uploadURL)
+                try remove(entryAtID: upload.id)
             }
         } catch {
             // This makes a lot of noise on the emulator, but might be worth logging if you're having issues around here
@@ -46,10 +46,10 @@ class UploadPersistence {
         }
     }
 
-    func remove(entryAtAbsUrl url: URL) throws {
+    func remove(entryAtID id: String) throws {
         try maybeOpenCache()
         if var cache = cache {
-            cache.removeValue(forKey: url)
+            cache.removeValue(forKey: id)
             self.cache = cache
             // write-through
             try uploadsFile.writeContents(of: UploadsFileContents(mapOf: cache))
@@ -58,10 +58,10 @@ class UploadPersistence {
 
     /// Directly writes entry. Updates are written-through the internal cache to the backing file
     ///  This method does I/O
-    func write(entry: PersistenceEntry, forFileAt fileUrl: URL) throws {
+    func write(entry: PersistenceEntry, for uploadID: String) throws {
         try maybeOpenCache()
         if var cache = cache {
-            cache.updateValue(entry, forKey: fileUrl)
+            cache.updateValue(entry, forKey: uploadID)
             self.cache = cache
             try uploadsFile.writeContents(
                 of: UploadsFileContents(entries: cache.map { (key, value) in value })
@@ -71,10 +71,10 @@ class UploadPersistence {
     }
 
     /// Directly reads a single entry based on the file URL given
-    func readEntry(forFileAt fileUrl: URL) throws -> PersistenceEntry? {
+    func readEntry(uploadID: String) throws -> PersistenceEntry? {
         try maybeOpenCache()
         if let cache = cache {
-            return cache[fileUrl.absoluteURL]
+            return cache[uploadID]
         } else {
             return nil
         }
@@ -93,7 +93,7 @@ class UploadPersistence {
         let nowish = Date().timeIntervalSince1970
         for entry in allEntries {
             if (nowish - entry.savedAt) > UploadPersistence.ENTRY_TTL {
-                try remove(entryAtAbsUrl: entry.uploadInfo.videoFile)
+                try remove(entryAtID: entry.uploadInfo.id)
             }
         }
     }
@@ -182,17 +182,17 @@ protocol UploadsFile {
 struct UploadsFileContents : Codable {
     let entriesAbsFileUrlToUploadInfo: [PersistenceEntry]
 
-    func asDictionary() -> [URL : PersistenceEntry] {
+    func asDictionary() -> [String : PersistenceEntry] {
         return entriesAbsFileUrlToUploadInfo.reduce(into: [:]) { (map, ent) -> () in
-            map.updateValue(ent, forKey: ent.uploadInfo.videoFile)
+            map.updateValue(ent, forKey: ent.uploadInfo.id)
         }
     }
 
     init(entries: [PersistenceEntry]) {
         self.entriesAbsFileUrlToUploadInfo = entries
     }
 
-    init(mapOf items: [URL : PersistenceEntry]) {
+    init(mapOf items: [String : PersistenceEntry]) {
         self.entriesAbsFileUrlToUploadInfo = items.compactMap({ (key, value) in value })
     }
 }