diff --git a/Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift b/Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift index 62960d67..73b886f0 100644 --- a/Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift +++ b/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) diff --git a/Sources/MuxUploadSDK/PublicAPI/UploadManager.swift b/Sources/MuxUploadSDK/PublicAPI/UploadManager.swift index ee2d3306..28803290 100644 --- a/Sources/MuxUploadSDK/PublicAPI/UploadManager.swift +++ b/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( + 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( + 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 { - 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 { + 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 } diff --git a/Sources/MuxUploadSDK/Upload/UploadInfo.swift b/Sources/MuxUploadSDK/Upload/UploadInfo.swift index a39f6a30..1385f92a 100644 --- a/Sources/MuxUploadSDK/Upload/UploadInfo.swift +++ b/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 */ diff --git a/Sources/MuxUploadSDK/Upload/UploadPersistence.swift b/Sources/MuxUploadSDK/Upload/UploadPersistence.swift index 8114a95a..99299a41 100644 --- a/Sources/MuxUploadSDK/Upload/UploadPersistence.swift +++ b/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,9 +182,9 @@ 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) } } @@ -192,7 +192,7 @@ struct UploadsFileContents : Codable { self.entriesAbsFileUrlToUploadInfo = entries } - init(mapOf items: [URL : PersistenceEntry]) { + init(mapOf items: [String : PersistenceEntry]) { self.entriesAbsFileUrlToUploadInfo = items.compactMap({ (key, value) in value }) } } diff --git a/Tests/MuxUploadSDKTests/Upload Tests/UploadPersistenceTests.swift b/Tests/MuxUploadSDKTests/Upload Tests/UploadPersistenceTests.swift index e30cc75a..06b682b3 100644 --- a/Tests/MuxUploadSDKTests/Upload Tests/UploadPersistenceTests.swift +++ b/Tests/MuxUploadSDKTests/Upload Tests/UploadPersistenceTests.swift @@ -31,11 +31,14 @@ final class UploadPersistenceTests: XCTestCase { uploadInfo: renameDummyUploadInfo(basename: "e2") ) let persistence = UploadPersistence(innerFile: makeSimulatedUploadsFile(), atURL: makeDummyFileURL(basename: "fake-cache-file")) - // Shouldn't throw - try! persistence.write(entry: e1, forFileAt: makeDummyFileURL(basename: "e1")) - try! persistence.write(entry: e2, forFileAt: makeDummyFileURL(basename: "e2")) + XCTAssertNoThrow( + try persistence.write(entry: e1, for: e1.uploadInfo.id) + ) + XCTAssertNoThrow( + try persistence.write(entry: e2, for: e2.uploadInfo.id) + ) - let readItem = try persistence.readEntry(forFileAt: makeDummyFileURL(basename: "e1")) + let readItem = try persistence.readEntry(uploadID: e1.uploadInfo.id) XCTAssertEqual( try persistence.readAll().count, 2, @@ -62,14 +65,24 @@ final class UploadPersistenceTests: XCTestCase { uploadInfo: renameDummyUploadInfo(basename: "e2") ) let persistence = UploadPersistence(innerFile: makeSimulatedUploadsFile(), atURL: makeDummyFileURL(basename: "fake-cache-file")) - // Shouldn't throw (but not specifically part of the test) - try! persistence.write(entry: e1, forFileAt: makeDummyFileURL(basename: "e1")) - try! persistence.write(entry: e2, forFileAt: makeDummyFileURL(basename: "e2")) + XCTAssertNoThrow( + try persistence.write(entry: e1, for: e1.uploadInfo.id) + ) + + XCTAssertNoThrow( + try persistence.write(entry: e2, for: e2.uploadInfo.id) + ) // Read them in a different order to ensure idempotence - let readItem1 = try persistence.readEntry(forFileAt: makeDummyFileURL(basename: "e2")) - let readItem2 = try persistence.readEntry(forFileAt: makeDummyFileURL(basename: "e1")) - let allItems = try persistence.readAll() + let readItem1 = try XCTUnwrap( + persistence.readEntry(uploadID: e2.uploadInfo.id) + ) + let readItem2 = try XCTUnwrap( + persistence.readEntry(uploadID: e1.uploadInfo.id) + ) + let allItems = try XCTUnwrap( + persistence.readAll() + ) XCTAssertEqual( allItems.count, @@ -78,13 +91,13 @@ final class UploadPersistenceTests: XCTestCase { ) XCTAssertEqual( // remember, they're swapped - readItem2!.uploadInfo.videoFile, + readItem2.uploadInfo.videoFile, e1.uploadInfo.videoFile, "read() should return the right item" ) XCTAssertEqual( // remember, they're swapped - readItem1!.uploadInfo.videoFile, + readItem1.uploadInfo.videoFile, e2.uploadInfo.videoFile, "read() should return the right item" ) @@ -105,11 +118,17 @@ final class UploadPersistenceTests: XCTestCase { ) let persistence = UploadPersistence(innerFile: makeSimulatedUploadsFile(), atURL: makeDummyFileURL(basename: "fake-cache-file")) // Shouldn't throw (but not specifically part of the test) - try! persistence.write(entry: e1, forFileAt: makeDummyFileURL(basename: "e1")) - try! persistence.write(entry: e2, forFileAt: makeDummyFileURL(basename: "e2")) - try! persistence.remove(entryAtAbsUrl: makeDummyFileURL(basename: "e2")) + XCTAssertNoThrow( + try persistence.write(entry: e1, for: e1.uploadInfo.id) + ) + XCTAssertNoThrow( + try persistence.write(entry: e2, for: e2.uploadInfo.id) + ) + XCTAssertNoThrow( + try persistence.remove(entryAtID: e2.uploadInfo.id) + ) - let readItem = try persistence.readEntry(forFileAt: makeDummyFileURL(basename: "e1")) + let readItem = try persistence.readEntry(uploadID: e1.uploadInfo.id) XCTAssertEqual( try persistence.readAll().count, 1, @@ -139,11 +158,21 @@ final class UploadPersistenceTests: XCTestCase { let persistence = UploadPersistence(innerFile: uploadsFile, atURL: makeDummyFileURL(basename: "fake-cache-file")) // Should not throw (not specifically under test) - try! persistence.write(entry: newerEntry, forFileAt: makeDummyFileURL(basename: "newer")) - try! persistence.write(entry: olderEntry, forFileAt: makeDummyFileURL(basename: "older")) + XCTAssertNoThrow( + try persistence.write( + entry: newerEntry, for: newerEntry.uploadInfo.id + ) + ) + XCTAssertNoThrow( + try persistence.write( + entry: olderEntry, for: olderEntry.uploadInfo.id + ) + ) let persistenceNextSession = UploadPersistence(innerFile: uploadsFile, atURL: makeDummyFileURL(basename: "fake-cache-file")) - let entriesAfterCleanup = try! persistenceNextSession.readAll() + let entriesAfterCleanup = try XCTUnwrap( + persistenceNextSession.readAll() + ) XCTAssertEqual( 1, entriesAfterCleanup.count, @@ -170,6 +199,7 @@ final class UploadPersistenceTests: XCTestCase { private func renameDummyUploadInfo(basename: String) -> UploadInfo { return UploadInfo( + id: UUID().uuidString, uploadURL: URL(string: "https://dummy.site/page/\(basename)")!, videoFile: URL(string: "file://path/to/dummy/file/\(basename)")!, chunkSize: 100, diff --git a/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift b/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift new file mode 100644 index 00000000..ac2d8867 --- /dev/null +++ b/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift @@ -0,0 +1,59 @@ +// +// UploadManagerTests.swift +// + +import Foundation +import XCTest + +@testable import MuxUploadSDK + +class UploadManagerTests: XCTestCase { + + func testUploadManagerURLDeduplication() throws { + + let uploadManager = UploadManager() + + let uploadURL = try XCTUnwrap( + URL(string: "https://www.example.com/upload") + ) + + let videoInputURL = try XCTUnwrap( + URL(string: "file://path/to/dummy/file/") + ) + + let upload = MuxUpload( + uploadInfo: UploadInfo( + id: UUID().uuidString, + uploadURL: uploadURL, + videoFile: videoInputURL, + chunkSize: 8, + retriesPerChunk: 2, + optOutOfEventTracking: true + ), + uploadManager: uploadManager + ) + + let duplicateUpload = MuxUpload( + uploadInfo: UploadInfo( + id: UUID().uuidString, + uploadURL: uploadURL, + videoFile: videoInputURL, + chunkSize: 8, + retriesPerChunk: 2, + optOutOfEventTracking: true + ), + uploadManager: uploadManager + ) + + upload.start(forceRestart: false) + duplicateUpload.start(forceRestart: false) + + XCTAssertEqual( + uploadManager.allManagedUploads().count, + 1, + "There should only be one active upload for a given URL" + ) + + } + +}