From a55d3855ee2950ad5b4bc83191b9d3d5c9a5aec1 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 10:56:54 +0100 Subject: [PATCH 01/45] [feat] enable complete swift concurrency checking - Fix build error nonisolated MainActor due to UIDevice usage --- .../project.pbxproj | 2 + .../xcshareddata/swiftpm/Package.resolved | 86 +++++++++++++++++++ Package.resolved | 86 +++++++++++++++++++ Package.swift | 1 + .../EndpointRequestStorageProcessor.swift | 36 +++++--- .../MultipeerConnectivityManager.swift | 61 +++++++------ 6 files changed, 232 insertions(+), 40 deletions(-) create mode 100644 NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved create mode 100644 Package.resolved diff --git a/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.pbxproj b/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.pbxproj index 50dd980f..7d75390b 100644 --- a/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.pbxproj +++ b/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.pbxproj @@ -553,6 +553,7 @@ ); PRODUCT_BUNDLE_IDENTIFIER = "com.strv.networking-sample-app"; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2"; }; @@ -573,6 +574,7 @@ ); PRODUCT_BUNDLE_IDENTIFIER = "com.strv.networking-sample-app"; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2"; }; diff --git a/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved new file mode 100644 index 00000000..63931882 --- /dev/null +++ b/NetworkingSampleApp/NetworkingSampleApp.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -0,0 +1,86 @@ +{ + "pins" : [ + { + "identity" : "collectionconcurrencykit", + "kind" : "remoteSourceControl", + "location" : "https://github.com/JohnSundell/CollectionConcurrencyKit.git", + "state" : { + "revision" : "b4f23e24b5a1bff301efc5e70871083ca029ff95", + "version" : "0.2.0" + } + }, + { + "identity" : "cryptoswift", + "kind" : "remoteSourceControl", + "location" : "https://github.com/krzyzanowskim/CryptoSwift.git", + "state" : { + "revision" : "32f641cf24fc7abc1c591a2025e9f2f572648b0f", + "version" : "1.7.2" + } + }, + { + "identity" : "sourcekitten", + "kind" : "remoteSourceControl", + "location" : "https://github.com/jpsim/SourceKitten.git", + "state" : { + "revision" : "b6dc09ee51dfb0c66e042d2328c017483a1a5d56", + "version" : "0.34.1" + } + }, + { + "identity" : "swift-argument-parser", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-argument-parser.git", + "state" : { + "revision" : "8f4d2753f0e4778c76d5f05ad16c74f707390531", + "version" : "1.2.3" + } + }, + { + "identity" : "swift-syntax", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-syntax.git", + "state" : { + "revision" : "74203046135342e4a4a627476dd6caf8b28fe11b", + "version" : "509.0.0" + } + }, + { + "identity" : "swiftlint", + "kind" : "remoteSourceControl", + "location" : "https://github.com/realm/SwiftLint.git", + "state" : { + "revision" : "6d2e58271ebc14c37bf76d7c9f4082cc15bad718", + "version" : "0.53.0" + } + }, + { + "identity" : "swiftytexttable", + "kind" : "remoteSourceControl", + "location" : "https://github.com/scottrhoyt/SwiftyTextTable.git", + "state" : { + "revision" : "c6df6cf533d120716bff38f8ff9885e1ce2a4ac3", + "version" : "0.9.0" + } + }, + { + "identity" : "swxmlhash", + "kind" : "remoteSourceControl", + "location" : "https://github.com/drmohundro/SWXMLHash.git", + "state" : { + "revision" : "a853604c9e9a83ad9954c7e3d2a565273982471f", + "version" : "7.0.2" + } + }, + { + "identity" : "yams", + "kind" : "remoteSourceControl", + "location" : "https://github.com/jpsim/Yams.git", + "state" : { + "revision" : "0d9ee7ea8c4ebd4a489ad7a73d5c6cad55d6fed3", + "version" : "5.0.6" + } + } + ], + "version" : 2 +} diff --git a/Package.resolved b/Package.resolved new file mode 100644 index 00000000..63931882 --- /dev/null +++ b/Package.resolved @@ -0,0 +1,86 @@ +{ + "pins" : [ + { + "identity" : "collectionconcurrencykit", + "kind" : "remoteSourceControl", + "location" : "https://github.com/JohnSundell/CollectionConcurrencyKit.git", + "state" : { + "revision" : "b4f23e24b5a1bff301efc5e70871083ca029ff95", + "version" : "0.2.0" + } + }, + { + "identity" : "cryptoswift", + "kind" : "remoteSourceControl", + "location" : "https://github.com/krzyzanowskim/CryptoSwift.git", + "state" : { + "revision" : "32f641cf24fc7abc1c591a2025e9f2f572648b0f", + "version" : "1.7.2" + } + }, + { + "identity" : "sourcekitten", + "kind" : "remoteSourceControl", + "location" : "https://github.com/jpsim/SourceKitten.git", + "state" : { + "revision" : "b6dc09ee51dfb0c66e042d2328c017483a1a5d56", + "version" : "0.34.1" + } + }, + { + "identity" : "swift-argument-parser", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-argument-parser.git", + "state" : { + "revision" : "8f4d2753f0e4778c76d5f05ad16c74f707390531", + "version" : "1.2.3" + } + }, + { + "identity" : "swift-syntax", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-syntax.git", + "state" : { + "revision" : "74203046135342e4a4a627476dd6caf8b28fe11b", + "version" : "509.0.0" + } + }, + { + "identity" : "swiftlint", + "kind" : "remoteSourceControl", + "location" : "https://github.com/realm/SwiftLint.git", + "state" : { + "revision" : "6d2e58271ebc14c37bf76d7c9f4082cc15bad718", + "version" : "0.53.0" + } + }, + { + "identity" : "swiftytexttable", + "kind" : "remoteSourceControl", + "location" : "https://github.com/scottrhoyt/SwiftyTextTable.git", + "state" : { + "revision" : "c6df6cf533d120716bff38f8ff9885e1ce2a4ac3", + "version" : "0.9.0" + } + }, + { + "identity" : "swxmlhash", + "kind" : "remoteSourceControl", + "location" : "https://github.com/drmohundro/SWXMLHash.git", + "state" : { + "revision" : "a853604c9e9a83ad9954c7e3d2a565273982471f", + "version" : "7.0.2" + } + }, + { + "identity" : "yams", + "kind" : "remoteSourceControl", + "location" : "https://github.com/jpsim/Yams.git", + "state" : { + "revision" : "0d9ee7ea8c4ebd4a489ad7a73d5c6cad55d6fed3", + "version" : "5.0.6" + } + } + ], + "version" : 2 +} diff --git a/Package.swift b/Package.swift index 231c4530..6417096b 100644 --- a/Package.swift +++ b/Package.swift @@ -23,6 +23,7 @@ let package = Package( // Targets can depend on other targets in this package, and on products in packages this package depends on. .target( name: "Networking", + swiftSettings: [.unsafeFlags(["-Xfrontend", "-strict-concurrency=complete"])], plugins: [.plugin(name: "SwiftLintPlugin", package: "SwiftLint")] ), .testTarget( diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 91ec57a2..c327ab59 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -27,20 +27,30 @@ open class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing private lazy var responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") private lazy var requestCounter = Counter() - private lazy var multipeerConnectivityManager: MultipeerConnectivityManager? = { - #if DEBUG - // Initialise only in DEBUG mode otherwise it could pose a security risk for production apps. - guard let multiPeerSharingConfig = config.multiPeerSharing else { + + // This would ideally also be a lazy var, however it has to be async. + private var _multipeerConnectivityManager: MultipeerConnectivityManager? + private var multipeerConnectivityManager: MultipeerConnectivityManager? { + get async { + #if DEBUG + // Initialise only in DEBUG mode otherwise it could pose a security risk for production apps. + guard _multipeerConnectivityManager == nil else { + return _multipeerConnectivityManager + } + + guard let multiPeerSharingConfig = config.multiPeerSharing else { + return nil + } + + let initialBuffer = multiPeerSharingConfig.shareHistory ? getAllStoredModels() : [] + _multipeerConnectivityManager = await MultipeerConnectivityManager(buffer: initialBuffer) + return _multipeerConnectivityManager + #else return nil + #endif } - - let initialBuffer = multiPeerSharingConfig.shareHistory ? getAllStoredModels() : [] - return .init(buffer: initialBuffer) - #else - return nil - #endif - }() - + } + // MARK: Default shared instance public static let shared = EndpointRequestStorageProcessor( config: .init( @@ -183,7 +193,7 @@ private extension EndpointRequestStorageProcessor { fileUrl: self.createFileUrl(endpointRequest) ) - multipeerConnectivityManager?.send(model: storageModel) + await multipeerConnectivityManager?.send(model: storageModel) } } diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift index 5475ab58..caa661bb 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift @@ -19,36 +19,43 @@ open class MultipeerConnectivityManager: NSObject { private var buffer: [EndpointRequestStorageModel] private var peers = Set() - private lazy var myPeerId: MCPeerID = { - #if os(macOS) - let deviceName = Host.current().localizedName ?? "macOS" - #else - let deviceName = UIDevice.current.name - #endif - - #if targetEnvironment(simulator) - return MCPeerID(displayName: "Simulator - " + deviceName) - #else - return MCPeerID(displayName: deviceName) - #endif - }() - - private lazy var session = MCSession( - peer: myPeerId, - securityIdentity: nil, - encryptionPreference: .none - ) - private lazy var nearbyServiceAdvertiser = MCNearbyServiceAdvertiser( - peer: myPeerId, - discoveryInfo: nil, - serviceType: MultipeerConnectivityManager.service - ) - - init(buffer: [EndpointRequestStorageModel]) { + + private let session: MCSession + private let nearbyServiceAdvertiser: MCNearbyServiceAdvertiser + + // The init has to be async because of @MainActor UIDevice usage. + init(buffer: [EndpointRequestStorageModel]) async { self.buffer = buffer + let deviceName = await { @MainActor in + #if os(macOS) + return Host.current().localizedName ?? "macOS" + #else + return UIDevice.current.name + #endif + }() + + let myPeerId: MCPeerID = { + #if targetEnvironment(simulator) + return MCPeerID(displayName: "Simulator - " + deviceName) + #else + return MCPeerID(displayName: deviceName) + #endif + }() + + self.session = MCSession( + peer: myPeerId, + securityIdentity: nil, + encryptionPreference: .none + ) + self.nearbyServiceAdvertiser = MCNearbyServiceAdvertiser( + peer: myPeerId, + discoveryInfo: nil, + serviceType: MultipeerConnectivityManager.service + ) + super.init() - + session.delegate = self nearbyServiceAdvertiser.delegate = self nearbyServiceAdvertiser.startAdvertisingPeer() From 06a981953c2dab162a2a54882051ca41fd16443d Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 11:54:25 +0100 Subject: [PATCH 02/45] [fix] change shared static var property into get only --- .../Networking/Core/RetryConfiguration.swift | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Sources/Networking/Core/RetryConfiguration.swift b/Sources/Networking/Core/RetryConfiguration.swift index e793f5d0..c1e8ead3 100644 --- a/Sources/Networking/Core/RetryConfiguration.swift +++ b/Sources/Networking/Core/RetryConfiguration.swift @@ -27,23 +27,25 @@ public struct RetryConfiguration { } // default configuration ignores - public static var `default` = RetryConfiguration( - retries: 3, - delay: .constant(2) - ) { error in - /// Do not retry authorization errors. - if error is AuthorizationError { - return false - } - - /// But retry certain HTTP errors. - guard let networkError = error as? NetworkError, - case let .unacceptableStatusCode(statusCode, _, _) = networkError - else { - return true - } + public static var `default`: RetryConfiguration { + .init( + retries: 3, + delay: .constant(2) + ) { error in + /// Do not retry authorization errors. + if error is AuthorizationError { + return false + } - return !(HTTPStatusCode.nonRetriableCodes ~= statusCode) + /// But retry certain HTTP errors. + guard let networkError = error as? NetworkError, + case let .unacceptableStatusCode(statusCode, _, _) = networkError + else { + return true + } + + return !(HTTPStatusCode.nonRetriableCodes ~= statusCode) + } } } From 641bc10cd79572de8a7413610312bd3835cda316 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 12:23:07 +0100 Subject: [PATCH 03/45] [feat] add sendable conformances --- Sources/Networking/Core/EndpointRequest.swift | 2 +- Sources/Networking/Core/Requestable.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Networking/Core/EndpointRequest.swift b/Sources/Networking/Core/EndpointRequest.swift index 248ef39f..62488848 100644 --- a/Sources/Networking/Core/EndpointRequest.swift +++ b/Sources/Networking/Core/EndpointRequest.swift @@ -11,7 +11,7 @@ import Foundation // MARK: - Struct wrapping one call to the API endpoint /// A wrapper structure which contains API endpoint with additional info about the session within which it's being called and an API call identifier. -public struct EndpointRequest: Identifiable { +public struct EndpointRequest: Identifiable, Sendable { public let id: String public let sessionId: String public let endpoint: Requestable diff --git a/Sources/Networking/Core/Requestable.swift b/Sources/Networking/Core/Requestable.swift index 0b66719f..d2ecda94 100644 --- a/Sources/Networking/Core/Requestable.swift +++ b/Sources/Networking/Core/Requestable.swift @@ -11,7 +11,7 @@ import Foundation // MARK: - Endpoint definition /// A type that represents an API endpoint. -public protocol Requestable: EndpointIdentifiable { +public protocol Requestable: EndpointIdentifiable, Sendable { /// The host URL of REST API. var baseURL: URL { get } From ca85c82ed867f5080d6f1714060e4d7e07ce6c45 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 15:30:16 +0100 Subject: [PATCH 04/45] [feat] conform authorization protocols to Actor --- .../NetworkingSampleApp/API/SampleAuthorizationManager.swift | 2 +- .../Interceptors/Authorization/AuthorizationData.swift | 2 +- .../Interceptors/Authorization/AuthorizationManaging.swift | 2 +- .../Authorization/AuthorizationStorageManaging.swift | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift index f85d1624..fa43f4a3 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift @@ -8,7 +8,7 @@ import Networking import Foundation -final class SampleAuthorizationManager: AuthorizationManaging { +final actor SampleAuthorizationManager: AuthorizationManaging { // MARK: Public properties let storage: AuthorizationStorageManaging = SampleAuthorizationStorageManager() diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationData.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationData.swift index e6d0a6b5..45e2cc56 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationData.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationData.swift @@ -7,7 +7,7 @@ import Foundation -public struct AuthorizationData { +public struct AuthorizationData: Sendable { public let accessToken: String public let refreshToken: String public let expiresIn: Date? diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift index f177adb9..861b2c38 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift @@ -9,7 +9,7 @@ import Foundation /// AuthorizationManaging authorizes requests and manages refresh token mechanism /// AuthorizationStorageManaging is required to read & store `AuthorizationData` -public protocol AuthorizationManaging { +public protocol AuthorizationManaging: Actor { var storage: any AuthorizationStorageManaging { get } func refreshAuthorizationData(with refreshToken: String) async throws -> AuthorizationData diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift index 4939e932..4c7dd305 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift @@ -9,7 +9,7 @@ import Foundation /// Basic operations to store `AuthorizationData` /// To keep consistency all operations are async -public protocol AuthorizationStorageManaging { +public protocol AuthorizationStorageManaging: Actor { func saveData(_ data: AuthorizationData) async throws func getData() async throws -> AuthorizationData func deleteData() async throws From fbaa27b3c1537cd05a8a2c62f1c4d0945fcf1a25 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 17:18:11 +0100 Subject: [PATCH 05/45] [feat] conform RetryConfiguration to sendable --- Sources/Networking/Core/RetryConfiguration.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Networking/Core/RetryConfiguration.swift b/Sources/Networking/Core/RetryConfiguration.swift index c1e8ead3..5636e744 100644 --- a/Sources/Networking/Core/RetryConfiguration.swift +++ b/Sources/Networking/Core/RetryConfiguration.swift @@ -8,19 +8,19 @@ import Foundation /// Retry of API calls allows various options wrapped into `RetryConfiguration` struct. -public struct RetryConfiguration { +public struct RetryConfiguration: Sendable { /// The number of retries. let retries: Int /// The delay between each retry to avoid overwhelming API. let delay: DelayConfiguration /// A handler which determines wether a request should be retried or not based on an error. /// By default errors with status codes `HTTPStatusCode.nonRetriableCodes` are not being retried. - let retryHandler: (Error) -> Bool + let retryHandler: @Sendable (Error) -> Bool public init( retries: Int, delay: DelayConfiguration, - retryHandler: @escaping (Error) -> Bool) { + retryHandler: @Sendable @escaping (Error) -> Bool) { self.retries = retries self.delay = delay self.retryHandler = retryHandler @@ -51,7 +51,7 @@ public struct RetryConfiguration { public extension RetryConfiguration { /// A type that defines the delay strategy for retry logic. - enum DelayConfiguration { + enum DelayConfiguration : Sendable { /// The delay cumulatively increases after each retry. case progressive(TimeInterval) /// The delay is the same after each retry. From 71f2a194b60836e00df44f1f68c5abf6ee8357b7 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 17:23:50 +0100 Subject: [PATCH 06/45] [chore] supress non-sendable type 'AnyCancellables' warning --- Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift b/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift index d75e7c84..cada6145 100644 --- a/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift +++ b/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift @@ -6,7 +6,8 @@ // import Foundation -import Combine +// The @preconcurrency suppresses capture of non-sendable type 'AnyCancellables' warning, which doesn't yet conform to Sendable. +@preconcurrency import Combine extension URLSessionTask { func asyncResponse() async throws -> URLResponse { From 196b8f774fa9930e24dec05c16630b7e1f55d479 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 24 Nov 2023 17:36:45 +0100 Subject: [PATCH 07/45] [fix] DownloadManager swift concurrency warnings --- .../Download/DownloadProgressViewModel.swift | 2 +- .../Networking/Core/DownloadAPIManager.swift | 65 +++++++++---------- .../Networking/Core/DownloadAPIManaging.swift | 2 +- .../Misc/ThreadSafeDictionary.swift | 3 + .../Misc/URLSessionTask+DownloadState.swift | 2 +- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Download/DownloadProgressViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Download/DownloadProgressViewModel.swift index 9349e11b..9f825828 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Download/DownloadProgressViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Download/DownloadProgressViewModel.swift @@ -19,7 +19,7 @@ final class DownloadProgressViewModel: ObservableObject { } func startObservingDownloadProgress() async { - let stream = DownloadAPIManager.shared.progressStream(for: task) + let stream = await DownloadAPIManager.shared.progressStream(for: task) for try await downloadState in stream { var newState = DownloadProgressState() diff --git a/Sources/Networking/Core/DownloadAPIManager.swift b/Sources/Networking/Core/DownloadAPIManager.swift index 4b106848..7a3715ba 100644 --- a/Sources/Networking/Core/DownloadAPIManager.swift +++ b/Sources/Networking/Core/DownloadAPIManager.swift @@ -6,10 +6,11 @@ // import Foundation -import Combine +// The @preconcurrency suppresses Sendable warning for AnyCancellables which doesn't yet conform to Sendable. +@preconcurrency import Combine /// Default Download API manager -open class DownloadAPIManager: NSObject, Retryable { +public actor DownloadAPIManager: NSObject, Retryable { private let requestAdapters: [RequestAdapting] private let responseProcessors: [ResponseProcessing] private let errorProcessors: [ErrorProcessing] @@ -19,7 +20,7 @@ open class DownloadAPIManager: NSObject, Retryable { private var taskStateCancellables = ThreadSafeDictionary() private var downloadStateDict = ThreadSafeDictionary() - internal var retryCounter = Counter() + let retryCounter = Counter() public var allTasks: [URLSessionDownloadTask] { get async { @@ -120,7 +121,7 @@ private extension DownloadAPIManager { /// downloadTask must be initiated by resume() before we try to await a response from downloadObserver, because it gets the response from URLSessionDownloadDelegate methods downloadTask.resume() - updateTasks() + await updateTasks() let urlResponse = try await downloadTask.asyncResponse() @@ -156,39 +157,37 @@ private extension DownloadAPIManager { /// Creates a record in the `downloadStateDict` for each task and observes their states. /// Every `downloadStateDict` update triggers an event to the `downloadStateDictSubject` /// which then leads to a task state update from `progressStream`. - func updateTasks() { - Task { - for task in await allTasks where await downloadStateDict.getValue(for: task) == nil { - /// In case there is no DownloadState for a given task in the dictionary, we need to create one. - await downloadStateDict.set(value: .init(task: task), for: task) - - /// We need to observe URLSessionTask.State via KVO individually for each task, because there is no delegate callback for the state change. - let cancellable = task - .publisher(for: \.state) - .sink { [weak self] state in - guard let self else { - return - } - - Task { - await self.downloadStateDict.update(task: task, for: \.taskState, with: state) - self.downloadStateDictSubject.send(await self.downloadStateDict.getValues()) - - if state == .completed { - await self.taskStateCancellables.set(value: nil, for: task) - } + func updateTasks() async { + for task in await allTasks where await downloadStateDict.getValue(for: task) == nil { + /// In case there is no DownloadState for a given task in the dictionary, we need to create one. + await downloadStateDict.set(value: .init(task: task), for: task) + + /// We need to observe URLSessionTask.State via KVO individually for each task, because there is no delegate callback for the state change. + let cancellable = task + .publisher(for: \.state) + .sink { [weak self] state in + guard let self else { + return + } + + Task { + await self.downloadStateDict.update(task: task, for: \.taskState, with: state) + self.downloadStateDictSubject.send(await self.downloadStateDict.getValues()) + + if state == .completed { + await self.taskStateCancellables.set(value: nil, for: task) } } - - await taskStateCancellables.set(value: cancellable, for: task) - } + } + + await taskStateCancellables.set(value: cancellable, for: task) } } } // MARK: URLSession Delegate extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { - public func urlSession(_: URLSession, downloadTask: URLSessionDownloadTask, didWriteData _: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) { + nonisolated public func urlSession(_: URLSession, downloadTask: URLSessionDownloadTask, didWriteData _: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) { Task { await downloadStateDict.update(task: downloadTask, for: \.downloadedBytes, with: totalBytesWritten) await downloadStateDict.update(task: downloadTask, for: \.totalBytes, with: totalBytesExpectedToWrite) @@ -196,7 +195,7 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { } } - public func urlSession(_: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) { + nonisolated public func urlSession(_: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) { do { guard let response = downloadTask.response else { return @@ -207,7 +206,7 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { Task { await downloadStateDict.update(task: downloadTask, for: \.downloadedFileURL, with: tempURL) downloadStateDictSubject.send(await downloadStateDict.getValues()) - updateTasks() + await updateTasks() } } catch { Task { @@ -216,11 +215,11 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { } } - public func urlSession(_: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { + nonisolated public func urlSession(_: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { Task { await downloadStateDict.update(task: task, for: \.error, with: error) downloadStateDictSubject.send(await downloadStateDict.getValues()) - updateTasks() + await updateTasks() } } } diff --git a/Sources/Networking/Core/DownloadAPIManaging.swift b/Sources/Networking/Core/DownloadAPIManaging.swift index 5b0fb33c..bc5d63f3 100644 --- a/Sources/Networking/Core/DownloadAPIManaging.swift +++ b/Sources/Networking/Core/DownloadAPIManaging.swift @@ -13,7 +13,7 @@ public typealias DownloadResult = (URLSessionDownloadTask, Response) /// A definition of an API layer with methods for handling data downloading. /// Recommended to be used as singleton. /// If you wish to use multiple instances, make sure you manually invalidate url session by calling the `invalidateSession` method. -public protocol DownloadAPIManaging { +public protocol DownloadAPIManaging: Actor { /// List of all currently ongoing download tasks. var allTasks: [URLSessionDownloadTask] { get async } diff --git a/Sources/Networking/Misc/ThreadSafeDictionary.swift b/Sources/Networking/Misc/ThreadSafeDictionary.swift index 864ffed5..d065bad3 100644 --- a/Sources/Networking/Misc/ThreadSafeDictionary.swift +++ b/Sources/Networking/Misc/ThreadSafeDictionary.swift @@ -35,3 +35,6 @@ actor ThreadSafeDictionary { } } } + +// KeyPaths are not yet conforming to Sendable (https://github.com/apple/swift/issues/68943), hence we at least suppress the non-sendable warning. +extension WritableKeyPath: @unchecked Sendable {} diff --git a/Sources/Networking/Misc/URLSessionTask+DownloadState.swift b/Sources/Networking/Misc/URLSessionTask+DownloadState.swift index 0417013e..43b5600c 100644 --- a/Sources/Networking/Misc/URLSessionTask+DownloadState.swift +++ b/Sources/Networking/Misc/URLSessionTask+DownloadState.swift @@ -8,7 +8,7 @@ import Foundation public extension URLSessionTask { - struct DownloadState { + struct DownloadState: Sendable { public var downloadedBytes: Int64 public var totalBytes: Int64 public var taskState: URLSessionTask.State From 7ba3e6b82ccb29db0ee9692767fb432a92686f7d Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 27 Nov 2023 11:36:19 +0100 Subject: [PATCH 08/45] [fix] UploadManager swift concurrency warnings --- .../Scenes/Upload/FormUploadsViewModel.swift | 2 +- .../Scenes/Upload/UploadItemViewModel.swift | 2 ++ .../NetworkingSampleApp/Scenes/Upload/UploadService.swift | 8 ++++++-- Sources/Networking/Core/RetryConfiguration.swift | 2 +- .../Core/Upload/MultipartFormData+BodyPart.swift | 4 +++- Sources/Networking/Core/Upload/MultipartFormData.swift | 8 ++++---- Sources/Networking/Core/Upload/UploadAPIManager.swift | 8 ++++---- Sources/Networking/Core/Upload/UploadAPIManaging.swift | 2 +- Sources/Networking/Core/Upload/UploadTask+State.swift | 2 +- Sources/Networking/Core/Upload/UploadTask.swift | 5 +++-- 10 files changed, 26 insertions(+), 17 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift index ebf78050..426474a6 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift @@ -58,7 +58,7 @@ extension FormUploadsViewModel { // MARK: - Prepare multipartForm data private extension FormUploadsViewModel { func createMultipartFormData() throws -> MultipartFormData { - let multipartFormData = MultipartFormData() + var multipartFormData = MultipartFormData() multipartFormData.append(Data(username.utf8), name: "username-textfield") if let fileUrl { try multipartFormData.append(from: fileUrl, name: "attachment") diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadItemViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadItemViewModel.swift index c47e14e1..e910e5b6 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadItemViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadItemViewModel.swift @@ -6,6 +6,8 @@ // import Foundation +// This import suppresses warning: Non-sendable type 'AsyncPublisher>' ... +@preconcurrency import Combine @MainActor final class UploadItemViewModel: ObservableObject { diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift index 9d745649..30677b87 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift @@ -7,8 +7,10 @@ import Foundation import Networking +// This import suppresses warning: Non-sendable type 'AsyncPublisher>' ... +@preconcurrency import Combine -final class UploadService { +final class UploadService: Sendable { private let uploadManager: UploadAPIManaging init(uploadManager: UploadAPIManaging = UploadAPIManager()) { @@ -16,7 +18,9 @@ final class UploadService { } deinit { - uploadManager.invalidateSession(shouldFinishTasks: false) + Task { + await uploadManager.invalidateSession(shouldFinishTasks: false) + } } } diff --git a/Sources/Networking/Core/RetryConfiguration.swift b/Sources/Networking/Core/RetryConfiguration.swift index 5636e744..c84091ad 100644 --- a/Sources/Networking/Core/RetryConfiguration.swift +++ b/Sources/Networking/Core/RetryConfiguration.swift @@ -51,7 +51,7 @@ public struct RetryConfiguration: Sendable { public extension RetryConfiguration { /// A type that defines the delay strategy for retry logic. - enum DelayConfiguration : Sendable { + enum DelayConfiguration: Sendable { /// The delay cumulatively increases after each retry. case progressive(TimeInterval) /// The delay is the same after each retry. diff --git a/Sources/Networking/Core/Upload/MultipartFormData+BodyPart.swift b/Sources/Networking/Core/Upload/MultipartFormData+BodyPart.swift index 043b0c09..d647eea7 100644 --- a/Sources/Networking/Core/Upload/MultipartFormData+BodyPart.swift +++ b/Sources/Networking/Core/Upload/MultipartFormData+BodyPart.swift @@ -7,9 +7,11 @@ import Foundation +extension InputStream: @unchecked Sendable {} + public extension MultipartFormData { /// Represents an individual part of the `multipart/form-data`. - struct BodyPart { + struct BodyPart: Sendable { /// The input stream containing the data of the part's body. let dataStream: InputStream diff --git a/Sources/Networking/Core/Upload/MultipartFormData.swift b/Sources/Networking/Core/Upload/MultipartFormData.swift index 03a86fed..f3caf33b 100644 --- a/Sources/Networking/Core/Upload/MultipartFormData.swift +++ b/Sources/Networking/Core/Upload/MultipartFormData.swift @@ -9,7 +9,7 @@ import Foundation /// The `MultipartFormData` class provides a convenient way to handle multipart form data. /// It allows you to construct a multipart form data payload by adding multiple body parts, each representing a separate piece of data. -open class MultipartFormData { +public struct MultipartFormData: Sendable { /// The total size of the `multipart/form-data`. /// It is calculated as the sum of sizes of all the body parts added to the `MultipartFormData` instance. public var size: UInt64 { @@ -39,7 +39,7 @@ public extension MultipartFormData { /// - name: The name parameter of the `Content-Disposition` header field associated with this body part. /// - fileName: An optional filename parameter of the `Content-Disposition` header field associated with this body part. /// - mimeType: An optional MIME type of the body part. - func append( + mutating func append( _ data: Data, name: String, fileName: String? = nil, @@ -62,7 +62,7 @@ public extension MultipartFormData { /// - name: The name parameter of the `Content-Disposition` header field associated with this body part. /// - fileName: An optional filename parameter of the `Content-Disposition` header field associated with this body part. If not provided, the last path component of the fileUrl is used as the filename (if any). /// - mimeType: An optional MIME type of the body part. If not provided, the MIME type is inferred from the file extension of the file. - func append( + mutating func append( from fileUrl: URL, name: String, fileName: String? = nil, @@ -97,7 +97,7 @@ public extension MultipartFormData { // MARK: - Private private extension MultipartFormData { - func append( + mutating func append( dataStream: InputStream, name: String, size: UInt64, diff --git a/Sources/Networking/Core/Upload/UploadAPIManager.swift b/Sources/Networking/Core/Upload/UploadAPIManager.swift index 501d85d5..fa0a82b5 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManager.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManager.swift @@ -9,7 +9,7 @@ import Combine import Foundation /// Default upload API manager -open class UploadAPIManager: NSObject { +public actor UploadAPIManager: NSObject { // MARK: - Public Properties public var activeTasks: [UploadTask] { get async { @@ -61,7 +61,7 @@ open class UploadAPIManager: NSObject { // MARK: URLSessionDataDelegate extension UploadAPIManager: URLSessionDataDelegate { - public func urlSession( + nonisolated public func urlSession( _ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data @@ -93,7 +93,7 @@ extension UploadAPIManager: URLSessionDataDelegate { // MARK: - URLSessionTaskDelegate extension UploadAPIManager: URLSessionTaskDelegate { - public func urlSession( + nonisolated public func urlSession( _ session: URLSession, task: URLSessionTask, didSendBodyData bytesSent: Int64, @@ -107,7 +107,7 @@ extension UploadAPIManager: URLSessionTaskDelegate { } } - public func urlSession( + nonisolated public func urlSession( _ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error? diff --git a/Sources/Networking/Core/Upload/UploadAPIManaging.swift b/Sources/Networking/Core/Upload/UploadAPIManaging.swift index ac0c5d05..ac7cc608 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManaging.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManaging.swift @@ -8,7 +8,7 @@ import Combine import Foundation -public protocol UploadAPIManaging { +public protocol UploadAPIManaging: Actor { typealias StateStream = AsyncPublisher> /// Currently active upload tasks. diff --git a/Sources/Networking/Core/Upload/UploadTask+State.swift b/Sources/Networking/Core/Upload/UploadTask+State.swift index 8b5bd2e5..37251b6e 100644 --- a/Sources/Networking/Core/Upload/UploadTask+State.swift +++ b/Sources/Networking/Core/Upload/UploadTask+State.swift @@ -9,7 +9,7 @@ import Foundation public extension UploadTask { /// The upload task's state. - struct State { + struct State: Sendable { /// Number of bytes sent. public let sentBytes: Int64 diff --git a/Sources/Networking/Core/Upload/UploadTask.swift b/Sources/Networking/Core/Upload/UploadTask.swift index 35d79f61..fcf34c2e 100644 --- a/Sources/Networking/Core/Upload/UploadTask.swift +++ b/Sources/Networking/Core/Upload/UploadTask.swift @@ -5,11 +5,12 @@ // Created by Tony Ngo on 12.06.2023. // -import Combine +// The @preconcurrency suppresses non-sendable warning for CurrentValueSubject. +@preconcurrency import Combine import Foundation /// Represents and manages an upload task and provides its state. -public struct UploadTask { +public struct UploadTask: Sendable { // swiftlint:disable:next type_name public typealias ID = String From b8a20bc7d67ed3412bf0feb269ed4104365395aa Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 27 Nov 2023 11:39:45 +0100 Subject: [PATCH 09/45] [fix] warning: reference to static mutable property not thread safe --- .../Extensions/DownloadAPIManager+SharedInstance.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift b/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift index 0364dacb..9f65ba6b 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift @@ -8,7 +8,7 @@ import Networking extension DownloadAPIManager { - static var shared: DownloadAPIManaging = { + static let shared: DownloadAPIManaging = { var responseProcessors: [ResponseProcessing] = [ LoggingInterceptor.shared, StatusCodeProcessor.shared From 155dbb1a2d2fe3a658a72f4da8f0ff2faca046e7 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 27 Nov 2023 15:13:54 +0100 Subject: [PATCH 10/45] [feat] make processors and adapters conform to Sendable --- Sources/Networking/Core/ErrorProcessing.swift | 2 +- Sources/Networking/Core/RequestAdapting.swift | 2 +- Sources/Networking/Core/ResponseProcessing.swift | 2 +- .../Modifiers/Interceptors/LoggingInterceptor.swift | 8 ++++---- .../Modifiers/Processors/StatusCodeProcessor.swift | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/Networking/Core/ErrorProcessing.swift b/Sources/Networking/Core/ErrorProcessing.swift index ef244efd..2ec61a81 100644 --- a/Sources/Networking/Core/ErrorProcessing.swift +++ b/Sources/Networking/Core/ErrorProcessing.swift @@ -8,7 +8,7 @@ import Foundation /// A type that is able to customize error returned after failed network request. -public protocol ErrorProcessing { +public protocol ErrorProcessing: Sendable { /// Modifies a given `Error`. /// - Parameters: /// - error: The error to be processed. diff --git a/Sources/Networking/Core/RequestAdapting.swift b/Sources/Networking/Core/RequestAdapting.swift index 77181572..1dd07bf4 100644 --- a/Sources/Networking/Core/RequestAdapting.swift +++ b/Sources/Networking/Core/RequestAdapting.swift @@ -11,7 +11,7 @@ import Foundation // MARK: - Modifying the request before it's been sent /// A type that is able to modify a request before sending it to an API. -public protocol RequestAdapting { +public protocol RequestAdapting: Sendable { /// Modifies a given `URLRequest`. /// - Parameters: /// - request: The request to be adapted. diff --git a/Sources/Networking/Core/ResponseProcessing.swift b/Sources/Networking/Core/ResponseProcessing.swift index e7bf5b72..72d257cb 100644 --- a/Sources/Networking/Core/ResponseProcessing.swift +++ b/Sources/Networking/Core/ResponseProcessing.swift @@ -11,7 +11,7 @@ import Foundation // MARK: - Defines modifying the response after it's been received /// A type that is able to modify a ``Response`` when it's received from the network layer. -public protocol ResponseProcessing { +public protocol ResponseProcessing: Sendable { /// Modifies a given ``Response``. /// - Parameters: /// - response: The response to be processed. diff --git a/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift b/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift index 4cb36cff..b46f0cda 100644 --- a/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift +++ b/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift @@ -15,7 +15,7 @@ import Foundation // MARK: - Pretty logging modifier /// ``RequestInterceptor`` which logs requests & responses info into console in pretty way -open class LoggingInterceptor: RequestInterceptor { +public final class LoggingInterceptor: RequestInterceptor { // MARK: Default shared instance public static let shared = LoggingInterceptor() @@ -26,7 +26,7 @@ open class LoggingInterceptor: RequestInterceptor { /// - request: The request to be logged. /// - endpointRequest: An endpoint request wrapper. /// - Returns: The the original `URLRequest`. - open func adapt(_ request: URLRequest, for endpointRequest: EndpointRequest) -> URLRequest { + public func adapt(_ request: URLRequest, for endpointRequest: EndpointRequest) -> URLRequest { prettyRequestLog(request, from: endpointRequest.endpoint) return request } @@ -37,7 +37,7 @@ open class LoggingInterceptor: RequestInterceptor { /// - request: The original URL request. /// - endpointRequest: An endpoint request wrapper. /// - Returns: The original ``Response``. - open func process(_ response: Response, with urlRequest: URLRequest, for endpointRequest: EndpointRequest) throws -> Response { + public func process(_ response: Response, with urlRequest: URLRequest, for endpointRequest: EndpointRequest) throws -> Response { prettyResponseLog(response, from: endpointRequest.endpoint) return response } @@ -47,7 +47,7 @@ open class LoggingInterceptor: RequestInterceptor { /// - error: The error to be logged. /// - endpointRequest: An endpoint request wrapper. /// - Returns: The original `Error`. - open func process(_ error: Error, for endpointRequest: EndpointRequest) async -> Error { + public func process(_ error: Error, for endpointRequest: EndpointRequest) async -> Error { prettyErrorLog(error, from: endpointRequest.endpoint) return error } diff --git a/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift b/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift index cd907b5a..ad86213a 100644 --- a/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift @@ -10,7 +10,7 @@ import Foundation // MARK: - Modifier handling validity of response http status codes /// A response processor validating ``Response`` http status code against ``Requestable`` API endpoint definition. -open class StatusCodeProcessor: ResponseProcessing { +public final class StatusCodeProcessor: ResponseProcessing { // MARK: Default shared instance public static let shared = StatusCodeProcessor() From e617286395b8e67ee4a10b44e14b076644bc809c Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 27 Nov 2023 18:07:39 +0100 Subject: [PATCH 11/45] [feat] add FileManager sendable conformance --- Sources/Networking/Utils/FileManager+Sendable.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 Sources/Networking/Utils/FileManager+Sendable.swift diff --git a/Sources/Networking/Utils/FileManager+Sendable.swift b/Sources/Networking/Utils/FileManager+Sendable.swift new file mode 100644 index 00000000..d8e79772 --- /dev/null +++ b/Sources/Networking/Utils/FileManager+Sendable.swift @@ -0,0 +1,11 @@ +// +// File.swift +// +// +// Created by Matej Molnár on 27.11.2023. +// + +import Foundation + +// FileManager does not yet conforming to Sendable, hence we at least suppress the non-sendable warning. +extension FileManager: @unchecked Sendable {} From 7ca81c7f02f2b01274999d33e42a9e35a26cfec5 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 27 Nov 2023 18:44:11 +0100 Subject: [PATCH 12/45] [feat] change EndpointRequestStorageProcessor and MultipeerConnectivityManager to actors --- .../API/SampleErrorProcessor.swift | 4 +- .../Upload/MultipartFormDataEncoder.swift | 2 +- .../Upload/MultipartFormDataEncoding.swift | 2 +- .../EndpointRequestStorageModel.swift | 2 +- .../EndpointRequestStorageProcessor.swift | 19 ++++---- .../MultipeerConnectivityManager.swift | 43 +++++++++++-------- 6 files changed, 40 insertions(+), 32 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleErrorProcessor.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleErrorProcessor.swift index dd90d80a..39dfca2f 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleErrorProcessor.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleErrorProcessor.swift @@ -10,8 +10,8 @@ import Foundation /// Maps NetworkError's unacceptableStatusCode 400 error to SampleAPIError. final class SampleErrorProcessor: ErrorProcessing { - private lazy var decoder = JSONDecoder() - + private let decoder = JSONDecoder() + func process(_ error: Error, for endpointRequest: EndpointRequest) -> Error { guard let networkError = error as? NetworkError, case let .unacceptableStatusCode(statusCode, _, response) = networkError, diff --git a/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift b/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift index d5fc6b76..2d6aa64a 100644 --- a/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift +++ b/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift @@ -7,7 +7,7 @@ import Foundation -open class MultipartFormDataEncoder { +public final class MultipartFormDataEncoder { /// A string representing a carriage return and line feed. private let crlf = "\r\n" diff --git a/Sources/Networking/Core/Upload/MultipartFormDataEncoding.swift b/Sources/Networking/Core/Upload/MultipartFormDataEncoding.swift index 6050f454..11521507 100644 --- a/Sources/Networking/Core/Upload/MultipartFormDataEncoding.swift +++ b/Sources/Networking/Core/Upload/MultipartFormDataEncoding.swift @@ -7,7 +7,7 @@ import Foundation -public protocol MultipartFormDataEncoding { +public protocol MultipartFormDataEncoding: Sendable { /// Encodes the specified `MultipartFormData` object into a `Data` object. /// - Parameter multipartFormData: The `MultipartFormData` object to encode. /// - Returns: A `Data` object containing the encoded `multipartFormData`. diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageModel.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageModel.swift index 3b02cfcc..2bf95a46 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageModel.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageModel.swift @@ -10,7 +10,7 @@ import Foundation // MARK: - Defines data model storing full endpoint request /// A model containing all necessary info about request and related response to be replayed as mocked data. -public struct EndpointRequestStorageModel: Codable { +public struct EndpointRequestStorageModel: Codable, Sendable { public let sessionId: String public let date: Date public let path: String diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index c327ab59..a778030a 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -19,14 +19,13 @@ import Foundation /// /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. -open class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { +public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager private let jsonEncoder: JSONEncoder private let config: Config - - private lazy var responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") - private lazy var requestCounter = Counter() + private let responsesDirectory: URL + private let requestCounter = Counter() // This would ideally also be a lazy var, however it has to be async. private var _multipeerConnectivityManager: MultipeerConnectivityManager? @@ -67,8 +66,10 @@ open class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing self.fileManager = fileManager self.jsonEncoder = jsonEncoder ?? .default self.config = config - - deleteStoredSessionsExceedingLimit() + self.responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") + Task { + await deleteStoredSessionsExceedingLimit() + } } /// Stores the `Response` in file system on background thread and returns unmodified response. @@ -111,7 +112,7 @@ open class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing // MARK: - Config public extension EndpointRequestStorageProcessor { - struct Config { + struct Config: Sendable { public static let `default` = Config() /// If `nil` the MultiPeerConnectivity session won't get initialised. @@ -128,7 +129,7 @@ public extension EndpointRequestStorageProcessor { } } - struct MultiPeerSharingConfig { + struct MultiPeerSharingConfig: Sendable { /// If `true` it loads all stored responses and shares them at the start. /// If `false` it only shares the responses from the current session. let shareHistory: Bool @@ -153,7 +154,7 @@ private extension EndpointRequestStorageProcessor { return } - self.createFolderIfNeeded(endpointRequest.sessionId) + await self.createFolderIfNeeded(endpointRequest.sessionId) // for http responses read headers let httpResponse = response.response as? HTTPURLResponse diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift index caa661bb..416d085f 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift @@ -11,9 +11,10 @@ import OSLog #endif -import MultipeerConnectivity +// @preconcurrency suppresses a swift concurrency warning: Non-sendable type ... +@preconcurrency import MultipeerConnectivity -open class MultipeerConnectivityManager: NSObject { +public actor MultipeerConnectivityManager: NSObject { public static let service = "networking-jobs" public static let macOSAppDisplayName = "networking-macos-app" @@ -91,11 +92,25 @@ private extension MultipeerConnectivityManager { buffer.removeAll() os_log("🎈 Request data were successfully sent via multipeer connection") } + + func stateChanged(_ state: MCSessionState, for peerID: MCPeerID) { + switch state { + case .connected: + peers.insert(peerID) + if !buffer.isEmpty { + sendBuffer(to: peerID) + } + case .notConnected, .connecting: + peers.remove(peerID) + @unknown default: + break + } + } } // MARK: - MCNearbyServiceAdvertiserDelegate extension MultipeerConnectivityManager: MCNearbyServiceAdvertiserDelegate { - public func advertiser( + nonisolated public func advertiser( _ advertiser: MCNearbyServiceAdvertiser, didReceiveInvitationFromPeer peerID: MCPeerID, withContext context: Data?, @@ -107,22 +122,14 @@ extension MultipeerConnectivityManager: MCNearbyServiceAdvertiserDelegate { // MARK: - MCSessionDelegate extension MultipeerConnectivityManager: MCSessionDelegate { - public func session(_ session: MCSession, peer peerID: MCPeerID, didChange state: MCSessionState) { - switch state { - case .connected: - peers.insert(peerID) - if !buffer.isEmpty { - sendBuffer(to: peerID) - } - case .notConnected, .connecting: - peers.remove(peerID) - @unknown default: - break + nonisolated public func session(_ session: MCSession, peer peerID: MCPeerID, didChange state: MCSessionState) { + Task { + await stateChanged(state, for: peerID) } } - public func session(_ session: MCSession, didReceive data: Data, fromPeer peerID: MCPeerID) {} - public func session(_ session: MCSession, didReceive stream: InputStream, withName streamName: String, fromPeer peerID: MCPeerID) {} - public func session(_ session: MCSession, didStartReceivingResourceWithName resourceName: String, fromPeer peerID: MCPeerID, with progress: Progress) {} - public func session(_ session: MCSession, didFinishReceivingResourceWithName resourceName: String, fromPeer peerID: MCPeerID, at localURL: URL?, withError error: Error?) {} + nonisolated public func session(_ session: MCSession, didReceive data: Data, fromPeer peerID: MCPeerID) {} + nonisolated public func session(_ session: MCSession, didReceive stream: InputStream, withName streamName: String, fromPeer peerID: MCPeerID) {} + nonisolated public func session(_ session: MCSession, didStartReceivingResourceWithName resourceName: String, fromPeer peerID: MCPeerID, with progress: Progress) {} + nonisolated public func session(_ session: MCSession, didFinishReceivingResourceWithName resourceName: String, fromPeer peerID: MCPeerID, at localURL: URL?, withError error: Error?) {} } From 78b039f15707e53c9ecc18fd9c6c0182fcb70826 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 4 Dec 2023 16:53:42 +0100 Subject: [PATCH 13/45] [fix] lint warning --- Sources/Networking/Utils/FileManager+Sendable.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Networking/Utils/FileManager+Sendable.swift b/Sources/Networking/Utils/FileManager+Sendable.swift index d8e79772..f591499d 100644 --- a/Sources/Networking/Utils/FileManager+Sendable.swift +++ b/Sources/Networking/Utils/FileManager+Sendable.swift @@ -1,6 +1,6 @@ // // File.swift -// +// // // Created by Matej Molnár on 27.11.2023. // From 4107039691ef1a5351f1208a520c31bbd0c8dde7 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 4 Dec 2023 17:11:20 +0100 Subject: [PATCH 14/45] [fix] MultipeerConnectivityManager init crash --- .../EndpointRequestStorageProcessor.swift | 16 ++++++++++++---- .../MultipeerConnectivityManager.swift | 13 ++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index a778030a..18363ce5 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -11,6 +11,7 @@ import Foundation import os #else import OSLog + import UIKit #endif // MARK: - Modifier storing endpoint requests @@ -19,6 +20,7 @@ import Foundation /// /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. + public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager @@ -27,12 +29,12 @@ public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorPro private let responsesDirectory: URL private let requestCounter = Counter() - // This would ideally also be a lazy var, however it has to be async. + // This would ideally also be a lazy var, however it has to be async, because UIDevice.current.name needs to be called on MainActor. private var _multipeerConnectivityManager: MultipeerConnectivityManager? private var multipeerConnectivityManager: MultipeerConnectivityManager? { get async { - #if DEBUG // Initialise only in DEBUG mode otherwise it could pose a security risk for production apps. + #if DEBUG guard _multipeerConnectivityManager == nil else { return _multipeerConnectivityManager } @@ -42,12 +44,17 @@ public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorPro } let initialBuffer = multiPeerSharingConfig.shareHistory ? getAllStoredModels() : [] - _multipeerConnectivityManager = await MultipeerConnectivityManager(buffer: initialBuffer) + + _multipeerConnectivityManager = MultipeerConnectivityManager( + buffer: initialBuffer, + deviceName: await MainActor.run { UIDevice.current.name } + ) return _multipeerConnectivityManager #else return nil #endif } + } // MARK: Default shared instance @@ -67,6 +74,7 @@ public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorPro self.jsonEncoder = jsonEncoder ?? .default self.config = config self.responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") + Task { await deleteStoredSessionsExceedingLimit() } @@ -149,7 +157,7 @@ private extension EndpointRequestStorageProcessor { endpointRequest: EndpointRequest, urlRequest: URLRequest ) { - Task(priority: .background) { [weak self] in + Task.detached(priority: .background) { [weak self] in guard let self else { return } diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift index 416d085f..ae6ed13b 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift @@ -24,16 +24,11 @@ public actor MultipeerConnectivityManager: NSObject { private let session: MCSession private let nearbyServiceAdvertiser: MCNearbyServiceAdvertiser - // The init has to be async because of @MainActor UIDevice usage. - init(buffer: [EndpointRequestStorageModel]) async { + init( + buffer: [EndpointRequestStorageModel], + deviceName: String + ) { self.buffer = buffer - let deviceName = await { @MainActor in - #if os(macOS) - return Host.current().localizedName ?? "macOS" - #else - return UIDevice.current.name - #endif - }() let myPeerId: MCPeerID = { #if targetEnvironment(simulator) From 410f39fead477a7544585b0782d710d634e67a49 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 4 Dec 2023 17:12:36 +0100 Subject: [PATCH 15/45] [chore] remove unnecessary final keywords --- .../NetworkingSampleApp/API/SampleAuthorizationManager.swift | 2 +- .../Authorization/AuthorizationTokenInterceptor.swift | 2 +- .../EndpointRequestStorageProcessor.swift | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift index fa43f4a3..314fa114 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift @@ -8,7 +8,7 @@ import Networking import Foundation -final actor SampleAuthorizationManager: AuthorizationManaging { +actor SampleAuthorizationManager: AuthorizationManaging { // MARK: Public properties let storage: AuthorizationStorageManaging = SampleAuthorizationStorageManager() diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift index 2c2c5d29..6bc5a41d 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift @@ -8,7 +8,7 @@ import Foundation // MARK: - Defines authentication handling in requests -public final actor AuthorizationTokenInterceptor: RequestInterceptor { +public actor AuthorizationTokenInterceptor: RequestInterceptor { private var authorizationManager: AuthorizationManaging private var refreshTask: Task? diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 18363ce5..1f88f218 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -21,7 +21,7 @@ import Foundation /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. -public final actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { +public actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager private let jsonEncoder: JSONEncoder From bd75c8d6fd2643ce707b43be3665a1568123b195 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 4 Dec 2023 18:00:56 +0100 Subject: [PATCH 16/45] [fix] UploadService deinit retain cycle with singleton --- .../NetworkingSampleApp/Scenes/Upload/UploadService.swift | 8 ++------ .../Scenes/Upload/UploadsViewModel.swift | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift index 30677b87..830279aa 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift @@ -11,17 +11,13 @@ import Networking @preconcurrency import Combine final class UploadService: Sendable { + static let shared = UploadService() + private let uploadManager: UploadAPIManaging init(uploadManager: UploadAPIManaging = UploadAPIManager()) { self.uploadManager = uploadManager } - - deinit { - Task { - await uploadManager.invalidateSession(shouldFinishTasks: false) - } - } } extension UploadService { diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadsViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadsViewModel.swift index a07f2485..53e73b5f 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadsViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadsViewModel.swift @@ -16,7 +16,7 @@ final class UploadsViewModel: ObservableObject { private let uploadService: UploadService - init(uploadService: UploadService = UploadService()) { + init(uploadService: UploadService = .shared) { self.uploadService = uploadService } } From 82c53a30d34e01e6bc9f38dcd871e3e63d10110f Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 4 Dec 2023 18:56:58 +0100 Subject: [PATCH 17/45] [fix] tests --- .../AuthorizationTokenInterceptorTests.swift | 26 ++++++++++++------- .../MultipartFormDataEncoderTests.swift | 4 +-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift b/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift index b3bc1ffb..acfb21f9 100644 --- a/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift +++ b/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift @@ -69,8 +69,8 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let refreshedAuthData = AuthorizationData.makeValidAuthorizationData() - authManager.refreshedAuthorizationData = refreshedAuthData - + await authManager.setRefreshedAuthorizationData(refreshedAuthData) + let requestable = MockRouter.testAuthenticationRequired let request = URLRequest(url: requestable.baseURL) let endpointRequest = EndpointRequest(requestable, sessionId: mockSessionId) @@ -105,13 +105,13 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let expiredAuthData = AuthorizationData.makeExpiredAuthorizationData() /// Token refresh is going to take 0.5 seconds in order to test wether other requests actually wait for the refresh to finish. - authManager.sleepNanoseconds = 500_000_000 + await authManager.setSleepNanoseconds(500_000_000) try await authManager.storage.saveData(expiredAuthData) let refreshedAuthData = AuthorizationData.makeValidAuthorizationData() - authManager.refreshedAuthorizationData = refreshedAuthData - + await authManager.setRefreshedAuthorizationData(refreshedAuthData) + let requestable = MockRouter.testAuthenticationRequired let request = URLRequest(url: requestable.baseURL) let endpointRequest = EndpointRequest(requestable, sessionId: mockSessionId) @@ -137,7 +137,7 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let expiredAuthData = AuthorizationData.makeExpiredAuthorizationData() /// Token refresh is going to take 0.5 seconds in order to test wether other requests actually wait for the refresh to finish. - authManager.sleepNanoseconds = 500_000_000 + await authManager.setSleepNanoseconds(500_000_000) try await authManager.storage.saveData(expiredAuthData) let requestable = MockRouter.testAuthenticationRequired @@ -180,12 +180,20 @@ private actor MockAuthorizationStorageManager: AuthorizationStorageManaging { } } -private class MockAuthorizationManager: AuthorizationManaging { +private actor MockAuthorizationManager: AuthorizationManaging { let storage: AuthorizationStorageManaging = MockAuthorizationStorageManager() - var sleepNanoseconds: UInt64 = 0 - var refreshedAuthorizationData: AuthorizationData? + private var sleepNanoseconds: UInt64 = 0 + private var refreshedAuthorizationData: AuthorizationData? + func setSleepNanoseconds(_ nanoseconds: UInt64) { + sleepNanoseconds = nanoseconds + } + + func setRefreshedAuthorizationData(_ authorisationData: AuthorizationData) { + refreshedAuthorizationData = authorisationData + } + func refreshAuthorizationData(with refreshToken: String) async throws -> Networking.AuthorizationData { try await Task.sleep(nanoseconds: sleepNanoseconds) diff --git a/Tests/NetworkingTests/MultipartFormDataEncoderTests.swift b/Tests/NetworkingTests/MultipartFormDataEncoderTests.swift index 80e8a252..27d75d5c 100644 --- a/Tests/NetworkingTests/MultipartFormDataEncoderTests.swift +++ b/Tests/NetworkingTests/MultipartFormDataEncoderTests.swift @@ -33,7 +33,7 @@ final class MultipartFormDataEncoderTests: XCTestCase { func test_encode_encodesDataAsExpected() throws { let sut = makeSUT() - let formData = MultipartFormData(boundary: "--boundary--123") + var formData = MultipartFormData(boundary: "--boundary--123") let data1 = Data("Hello".utf8) formData.append(data1, name: "first-data") @@ -56,7 +56,7 @@ final class MultipartFormDataEncoderTests: XCTestCase { func test_encode_encodesToFileAsExpected() throws { let sut = makeSUT() - let formData = MultipartFormData(boundary: "--boundary--123") + var formData = MultipartFormData(boundary: "--boundary--123") let data = Data("Hello".utf8) formData.append(data, name: "first-data") From f75e1edc8b554477141fed496fe967e735563ba7 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 5 Dec 2023 10:08:33 +0100 Subject: [PATCH 18/45] [chore] revert let to lazy var --- .../EndpointRequestStorageProcessor.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 1f88f218..2a7cedd7 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -26,8 +26,8 @@ public actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessin private let fileManager: FileManager private let jsonEncoder: JSONEncoder private let config: Config - private let responsesDirectory: URL - private let requestCounter = Counter() + private lazy var responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") + private lazy var requestCounter = Counter() // This would ideally also be a lazy var, however it has to be async, because UIDevice.current.name needs to be called on MainActor. private var _multipeerConnectivityManager: MultipeerConnectivityManager? @@ -73,7 +73,6 @@ public actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessin self.fileManager = fileManager self.jsonEncoder = jsonEncoder ?? .default self.config = config - self.responsesDirectory = fileManager.temporaryDirectory.appendingPathComponent("responses") Task { await deleteStoredSessionsExceedingLimit() From 644e5e4150466e4d8e2d3ff5dde7adc70ce06adf Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 18 Dec 2023 16:57:13 +0100 Subject: [PATCH 19/45] [chore] fix comment types --- Sources/Networking/Core/RetryConfiguration.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Sources/Networking/Core/RetryConfiguration.swift b/Sources/Networking/Core/RetryConfiguration.swift index c84091ad..8217c80a 100644 --- a/Sources/Networking/Core/RetryConfiguration.swift +++ b/Sources/Networking/Core/RetryConfiguration.swift @@ -25,19 +25,18 @@ public struct RetryConfiguration: Sendable { self.delay = delay self.retryHandler = retryHandler } - - // default configuration ignores + public static var `default`: RetryConfiguration { .init( retries: 3, delay: .constant(2) ) { error in - /// Do not retry authorization errors. + // Do not retry authorization errors. if error is AuthorizationError { return false } - /// But retry certain HTTP errors. + // But retry certain HTTP errors. guard let networkError = error as? NetworkError, case let .unacceptableStatusCode(statusCode, _, _) = networkError else { From 8bdffbe7205ae7f7c7bd950a2e22e950285060ca Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 18 Dec 2023 17:31:37 +0100 Subject: [PATCH 20/45] [feat] change Actor protocol conformances to Sendable --- Sources/Networking/Core/DownloadAPIManaging.swift | 6 +++--- Sources/Networking/Core/Upload/UploadAPIManager.swift | 2 +- Sources/Networking/Core/Upload/UploadAPIManaging.swift | 4 ++-- .../Interceptors/Authorization/AuthorizationManaging.swift | 2 +- .../Authorization/AuthorizationStorageManaging.swift | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/Networking/Core/DownloadAPIManaging.swift b/Sources/Networking/Core/DownloadAPIManaging.swift index bc5d63f3..3fdc4bfd 100644 --- a/Sources/Networking/Core/DownloadAPIManaging.swift +++ b/Sources/Networking/Core/DownloadAPIManaging.swift @@ -13,14 +13,14 @@ public typealias DownloadResult = (URLSessionDownloadTask, Response) /// A definition of an API layer with methods for handling data downloading. /// Recommended to be used as singleton. /// If you wish to use multiple instances, make sure you manually invalidate url session by calling the `invalidateSession` method. -public protocol DownloadAPIManaging: Actor { +public protocol DownloadAPIManaging: Sendable { /// List of all currently ongoing download tasks. var allTasks: [URLSessionDownloadTask] { get async } /// Invalidates urlSession instance. /// - Parameters: /// - shouldFinishTasks: Indicates whether all currently active tasks should be able to finish before invalidating. Otherwise they will be cancelled. - func invalidateSession(shouldFinishTasks: Bool) + func invalidateSession(shouldFinishTasks: Bool) async /// Initiates a download request for a given endpoint, with optional resumable data and retry configuration. /// - Parameters: @@ -38,7 +38,7 @@ public protocol DownloadAPIManaging: Actor { /// Provides real time download updates for a given `URLSessionTask` /// - Parameter task: The task whose updates are requested. /// - Returns: An async stream of download states describing the task's download progress. - func progressStream(for task: URLSessionTask) -> AsyncStream + func progressStream(for task: URLSessionTask) async -> AsyncStream } // MARK: - Provide request with default nil resumable data, retry configuration diff --git a/Sources/Networking/Core/Upload/UploadAPIManager.swift b/Sources/Networking/Core/Upload/UploadAPIManager.swift index fa0a82b5..ff17ce26 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManager.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManager.swift @@ -5,7 +5,7 @@ // Created by Tony Ngo on 12.06.2023. // -import Combine +@preconcurrency import Combine import Foundation /// Default upload API manager diff --git a/Sources/Networking/Core/Upload/UploadAPIManaging.swift b/Sources/Networking/Core/Upload/UploadAPIManaging.swift index ac7cc608..700db925 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManaging.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManaging.swift @@ -8,7 +8,7 @@ import Combine import Foundation -public protocol UploadAPIManaging: Actor { +public protocol UploadAPIManaging: Sendable { typealias StateStream = AsyncPublisher> /// Currently active upload tasks. @@ -69,7 +69,7 @@ public protocol UploadAPIManaging: Actor { /// /// The internal implementation uses Apple's delegate pattern which retains a strong reference to the delegate. You must call this method to allow the manager to be released from the memory, otherwise your app will be leaking until your app exits or the session is invalidated. /// - Parameter shouldFinishTasks: Determines whether all outstanding tasks should finish before invalidating the session or be immediately cancelled. - func invalidateSession(shouldFinishTasks: Bool) + func invalidateSession(shouldFinishTasks: Bool) async } public extension UploadAPIManaging { diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift index 861b2c38..da84d509 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift @@ -9,7 +9,7 @@ import Foundation /// AuthorizationManaging authorizes requests and manages refresh token mechanism /// AuthorizationStorageManaging is required to read & store `AuthorizationData` -public protocol AuthorizationManaging: Actor { +public protocol AuthorizationManaging: Sendable { var storage: any AuthorizationStorageManaging { get } func refreshAuthorizationData(with refreshToken: String) async throws -> AuthorizationData diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift index 4c7dd305..b8fbeafe 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift @@ -9,7 +9,7 @@ import Foundation /// Basic operations to store `AuthorizationData` /// To keep consistency all operations are async -public protocol AuthorizationStorageManaging: Actor { +public protocol AuthorizationStorageManaging: Sendable { func saveData(_ data: AuthorizationData) async throws func getData() async throws -> AuthorizationData func deleteData() async throws From 9e7263079283dc809572086d49bba4a05a4d5c4b Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 18 Dec 2023 17:45:05 +0100 Subject: [PATCH 21/45] [chore] update package config --- Package.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index 6417096b..c3c71b45 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ let package = Package( // Targets can depend on other targets in this package, and on products in packages this package depends on. .target( name: "Networking", - swiftSettings: [.unsafeFlags(["-Xfrontend", "-strict-concurrency=complete"])], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency")], plugins: [.plugin(name: "SwiftLintPlugin", package: "SwiftLint")] ), .testTarget( From d385d34cb6ac090a131ab7851993d9da1afc7366 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 18 Dec 2023 18:16:32 +0100 Subject: [PATCH 22/45] [feat] change internal var to let --- Sources/Networking/Core/APIManager.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Networking/Core/APIManager.swift b/Sources/Networking/Core/APIManager.swift index 238fe93c..f908e5ed 100644 --- a/Sources/Networking/Core/APIManager.swift +++ b/Sources/Networking/Core/APIManager.swift @@ -14,8 +14,8 @@ open class APIManager: APIManaging, Retryable { private let errorProcessors: [ErrorProcessing] private let responseProvider: ResponseProviding private let sessionId: String - internal var retryCounter = Counter() - + let retryCounter = Counter() + public init( urlSession: URLSession = .init(configuration: .default), requestAdapters: [RequestAdapting] = [], From 3386b03f05687cdf1cce3b3d96cd729a54bfc49d Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 29 Dec 2023 15:03:47 +0100 Subject: [PATCH 23/45] [feat] add NetworkingActor --- .../API/SampleAuthorizationManager.swift | 3 ++- .../Authorization/AuthorizationViewModel.swift | 2 ++ .../Scenes/Users/UsersViewModel.swift | 1 + Sources/Networking/Core/APIManager.swift | 3 ++- Sources/Networking/Core/MockResponseProvider.swift | 5 +++-- Sources/Networking/Core/NetworkingActor.swift | 12 ++++++++++++ Sources/Networking/Core/Retryable.swift | 7 ++++--- Sources/Networking/Misc/Counter.swift | 3 ++- .../EndpointRequestStorageProcessor.swift | 12 +++++------- .../EndpointRequestStorageProcessorTests.swift | 1 + Tests/NetworkingTests/ErrorProcessorTests.swift | 1 + .../NetworkingTests/MockResponseProviderTests.swift | 1 + 12 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 Sources/Networking/Core/NetworkingActor.swift diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift index 314fa114..54b72863 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift @@ -8,7 +8,8 @@ import Networking import Foundation -actor SampleAuthorizationManager: AuthorizationManaging { +@NetworkingActor +class SampleAuthorizationManager: AuthorizationManaging { // MARK: Public properties let storage: AuthorizationStorageManaging = SampleAuthorizationStorageManager() diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Authorization/AuthorizationViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Authorization/AuthorizationViewModel.swift index 6a16f7ae..c1fb92fc 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Authorization/AuthorizationViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Authorization/AuthorizationViewModel.swift @@ -9,7 +9,9 @@ import Foundation import Networking final class AuthorizationViewModel: ObservableObject { + @NetworkingActor private lazy var authManager = SampleAuthorizationManager() + @NetworkingActor private lazy var apiManager: APIManager = { let authorizationInterceptor = AuthorizationTokenInterceptor(authorizationManager: authManager) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift index 4d3e020f..558b7dc2 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift @@ -22,6 +22,7 @@ final class UsersViewModel: ObservableObject { return decoder }() + @NetworkingActor private lazy var apiManager: APIManager = { var responseProcessors: [ResponseProcessing] = [ LoggingInterceptor.shared, diff --git a/Sources/Networking/Core/APIManager.swift b/Sources/Networking/Core/APIManager.swift index f908e5ed..15920d06 100644 --- a/Sources/Networking/Core/APIManager.swift +++ b/Sources/Networking/Core/APIManager.swift @@ -8,6 +8,7 @@ import Foundation /// Default API manager +@NetworkingActor open class APIManager: APIManaging, Retryable { private let requestAdapters: [RequestAdapting] private let responseProcessors: [ResponseProcessing] @@ -69,7 +70,7 @@ private extension APIManager { response = try await responseProcessors.process(response, with: request, for: endpointRequest) /// reset retry count - await retryCounter.reset(for: endpointRequest.id) + retryCounter.reset(for: endpointRequest.id) return response } catch { diff --git a/Sources/Networking/Core/MockResponseProvider.swift b/Sources/Networking/Core/MockResponseProvider.swift index 307c471c..efb90a3a 100644 --- a/Sources/Networking/Core/MockResponseProvider.swift +++ b/Sources/Networking/Core/MockResponseProvider.swift @@ -17,6 +17,7 @@ import Foundation // MARK: - MockResponseProvider definition /// A response provider which creates responses for requests from corresponding data files stored in Assets. +@NetworkingActor open class MockResponseProvider: ResponseProviding { private let bundle: Bundle private let sessionId: String @@ -62,11 +63,11 @@ private extension MockResponseProvider { /// Loads a corresponding file from Assets for a given ``URLRequest`` and decodes the data to `EndpointRequestStorageModel`. func loadModel(for request: URLRequest) async throws -> EndpointRequestStorageModel? { // counting from 0, check storage request processing - let count = await requestCounter.count(for: request.identifier) + let count = requestCounter.count(for: request.identifier) if let data = NSDataAsset(name: "\(sessionId)_\(request.identifier)_\(count)", bundle: bundle)?.data { // store info about next indexed api call - await requestCounter.increment(for: request.identifier) + requestCounter.increment(for: request.identifier) return try decoder.decode(EndpointRequestStorageModel.self, from: data) } diff --git a/Sources/Networking/Core/NetworkingActor.swift b/Sources/Networking/Core/NetworkingActor.swift new file mode 100644 index 00000000..fd17d24a --- /dev/null +++ b/Sources/Networking/Core/NetworkingActor.swift @@ -0,0 +1,12 @@ +// +// File.swift +// +// +// Created by Matej Molnár on 29.12.2023. +// + +import Foundation + +@globalActor public actor NetworkingActor { + public static let shared = NetworkingActor() +} diff --git a/Sources/Networking/Core/Retryable.swift b/Sources/Networking/Core/Retryable.swift index 7ad94f52..956f751f 100644 --- a/Sources/Networking/Core/Retryable.swift +++ b/Sources/Networking/Core/Retryable.swift @@ -6,6 +6,7 @@ // /// Provides retry utility functionality to subjects that require it. +@NetworkingActor protocol Retryable { /// Keeps count of executed retries so far given by `RetryConfiguration.retries`. var retryCounter: Counter { get } @@ -26,7 +27,7 @@ protocol Retryable { extension Retryable { func sleepIfRetry(for error: Error, endpointRequest: EndpointRequest, retryConfiguration: RetryConfiguration?) async throws { - let retryCount = await retryCounter.count(for: endpointRequest.id) + let retryCount = retryCounter.count(for: endpointRequest.id) guard let retryConfiguration = retryConfiguration, @@ -34,12 +35,12 @@ extension Retryable { retryConfiguration.retries > retryCount else { /// reset retry count - await retryCounter.reset(for: endpointRequest.id) + retryCounter.reset(for: endpointRequest.id) throw error } /// count the delay for retry - await retryCounter.increment(for: endpointRequest.id) + retryCounter.increment(for: endpointRequest.id) var sleepDuration: UInt64 switch retryConfiguration.delay { diff --git a/Sources/Networking/Misc/Counter.swift b/Sources/Networking/Misc/Counter.swift index 8862c504..745876b8 100644 --- a/Sources/Networking/Misc/Counter.swift +++ b/Sources/Networking/Misc/Counter.swift @@ -8,7 +8,8 @@ import Foundation /// A thread safe wrapper for count dictionary. -actor Counter { +@NetworkingActor +final class Counter { private var dict = [String: Int]() func count(for key: String) -> Int { diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 2a7cedd7..79223c00 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -20,8 +20,8 @@ import Foundation /// /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. - -public actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { +@NetworkingActor +public class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager private let jsonEncoder: JSONEncoder @@ -74,9 +74,7 @@ public actor EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessin self.jsonEncoder = jsonEncoder ?? .default self.config = config - Task { - await deleteStoredSessionsExceedingLimit() - } + deleteStoredSessionsExceedingLimit() } /// Stores the `Response` in file system on background thread and returns unmodified response. @@ -219,8 +217,8 @@ private extension EndpointRequestStorageProcessor { } func createFileUrl(_ endpointRequest: EndpointRequest) async -> URL { - let count = await requestCounter.count(for: endpointRequest.endpoint.identifier) - await requestCounter.increment(for: endpointRequest.endpoint.identifier) + let count = requestCounter.count(for: endpointRequest.endpoint.identifier) + requestCounter.increment(for: endpointRequest.endpoint.identifier) let fileName = "\(endpointRequest.sessionId)_\(endpointRequest.endpoint.identifier)_\(count)" diff --git a/Tests/NetworkingTests/EndpointRequestStorageProcessorTests.swift b/Tests/NetworkingTests/EndpointRequestStorageProcessorTests.swift index 3980fde9..d549e170 100644 --- a/Tests/NetworkingTests/EndpointRequestStorageProcessorTests.swift +++ b/Tests/NetworkingTests/EndpointRequestStorageProcessorTests.swift @@ -12,6 +12,7 @@ import XCTest // MARK: - Test Endpoint request storage processor +@NetworkingActor final class EndpointRequestStorageProcessorTests: XCTestCase { private let sessionId = "sessionId_request_storage" private let fileManager = FileManager.default diff --git a/Tests/NetworkingTests/ErrorProcessorTests.swift b/Tests/NetworkingTests/ErrorProcessorTests.swift index 195768e2..d6d1c2ad 100644 --- a/Tests/NetworkingTests/ErrorProcessorTests.swift +++ b/Tests/NetworkingTests/ErrorProcessorTests.swift @@ -9,6 +9,7 @@ import XCTest import Foundation +@NetworkingActor final class ErrorProcessorTests: XCTestCase { enum MockRouter: Requestable { case testMockSimpleError diff --git a/Tests/NetworkingTests/MockResponseProviderTests.swift b/Tests/NetworkingTests/MockResponseProviderTests.swift index c3ca6027..6675b242 100644 --- a/Tests/NetworkingTests/MockResponseProviderTests.swift +++ b/Tests/NetworkingTests/MockResponseProviderTests.swift @@ -8,6 +8,7 @@ @testable import Networking import XCTest +@NetworkingActor final class MockResponseProviderTests: XCTestCase { // swiftlint:disable:next force_unwrapping private lazy var mockUrlRequest = URLRequest(url: URL(string: "https://reqres.in/api/users?page=2")!) From 5bde238eef2f8fe3bf1185ec6eddc7a0001feefb Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 29 Dec 2023 15:04:31 +0100 Subject: [PATCH 24/45] [feat] conform DownloadAPIManaging to NetworkingActor --- .../DownloadAPIManager+SharedInstance.swift | 2 +- .../Networking/Core/DownloadAPIManager.swift | 58 +++++++++++-------- .../Networking/Core/DownloadAPIManaging.swift | 3 +- 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift b/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift index 9f65ba6b..072d5563 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Extensions/DownloadAPIManager+SharedInstance.swift @@ -8,7 +8,7 @@ import Networking extension DownloadAPIManager { - static let shared: DownloadAPIManaging = { + static let shared: DownloadAPIManager = { var responseProcessors: [ResponseProcessing] = [ LoggingInterceptor.shared, StatusCodeProcessor.shared diff --git a/Sources/Networking/Core/DownloadAPIManager.swift b/Sources/Networking/Core/DownloadAPIManager.swift index 7a3715ba..34d19a47 100644 --- a/Sources/Networking/Core/DownloadAPIManager.swift +++ b/Sources/Networking/Core/DownloadAPIManager.swift @@ -10,16 +10,21 @@ import Foundation @preconcurrency import Combine /// Default Download API manager -public actor DownloadAPIManager: NSObject, Retryable { +@NetworkingActor +open class DownloadAPIManager: NSObject, Retryable { private let requestAdapters: [RequestAdapting] private let responseProcessors: [ResponseProcessing] private let errorProcessors: [ErrorProcessing] private let sessionId: String private let downloadStateDictSubject = CurrentValueSubject<[URLSessionTask: URLSessionTask.DownloadState], Never>([:]) private var urlSession = URLSession(configuration: .default) - private var taskStateCancellables = ThreadSafeDictionary() - private var downloadStateDict = ThreadSafeDictionary() - + private var taskStateCancellables = [URLSessionTask: AnyCancellable]() + private var downloadStateDict = [URLSessionTask: URLSessionTask.DownloadState]() { + didSet { + downloadStateDictSubject.send(downloadStateDict) + } + } + let retryCounter = Counter() public var allTasks: [URLSessionDownloadTask] { @@ -129,7 +134,7 @@ private extension DownloadAPIManager { let response = try await responseProcessors.process((Data(), urlResponse), with: request, for: endpointRequest) /// reset retry count - await retryCounter.reset(for: endpointRequest.id) + retryCounter.reset(for: endpointRequest.id) /// create download AsyncStream return (downloadTask, response) @@ -158,9 +163,9 @@ private extension DownloadAPIManager { /// Every `downloadStateDict` update triggers an event to the `downloadStateDictSubject` /// which then leads to a task state update from `progressStream`. func updateTasks() async { - for task in await allTasks where await downloadStateDict.getValue(for: task) == nil { + for task in await allTasks where downloadStateDict[task] == nil { /// In case there is no DownloadState for a given task in the dictionary, we need to create one. - await downloadStateDict.set(value: .init(task: task), for: task) + downloadStateDict[task] = .init(task: task) /// We need to observe URLSessionTask.State via KVO individually for each task, because there is no delegate callback for the state change. let cancellable = task @@ -171,16 +176,18 @@ private extension DownloadAPIManager { } Task { - await self.downloadStateDict.update(task: task, for: \.taskState, with: state) - self.downloadStateDictSubject.send(await self.downloadStateDict.getValues()) + var mutableTask = self.downloadStateDict[task] + mutableTask?.taskState = state + self.downloadStateDict[task] = mutableTask + self.downloadStateDictSubject.send(self.downloadStateDict) if state == .completed { - await self.taskStateCancellables.set(value: nil, for: task) + self.taskStateCancellables[task] = nil } } } - await taskStateCancellables.set(value: cancellable, for: task) + taskStateCancellables[task] = cancellable } } } @@ -188,10 +195,11 @@ private extension DownloadAPIManager { // MARK: URLSession Delegate extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { nonisolated public func urlSession(_: URLSession, downloadTask: URLSessionDownloadTask, didWriteData _: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) { - Task { - await downloadStateDict.update(task: downloadTask, for: \.downloadedBytes, with: totalBytesWritten) - await downloadStateDict.update(task: downloadTask, for: \.totalBytes, with: totalBytesExpectedToWrite) - downloadStateDictSubject.send(await downloadStateDict.getValues()) + Task { @NetworkingActor in + var mutableTask = downloadStateDict[downloadTask] + mutableTask?.downloadedBytes = totalBytesWritten + mutableTask?.totalBytes = totalBytesExpectedToWrite + downloadStateDict[downloadTask] = mutableTask } } @@ -203,22 +211,26 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { // Move downloaded contents to documents as location will be unavailable after scope of this method. let tempURL = try location.moveContentsToDocuments(response: response) - Task { - await downloadStateDict.update(task: downloadTask, for: \.downloadedFileURL, with: tempURL) - downloadStateDictSubject.send(await downloadStateDict.getValues()) + Task { @NetworkingActor in + var mutableTask = downloadStateDict[downloadTask] + mutableTask?.downloadedFileURL = tempURL + downloadStateDict[downloadTask] = mutableTask await updateTasks() } } catch { - Task { - await downloadStateDict.update(task: downloadTask, for: \.error, with: error) + Task { @NetworkingActor in + var mutableTask = downloadStateDict[downloadTask] + mutableTask?.error = error + downloadStateDict[downloadTask] = mutableTask } } } nonisolated public func urlSession(_: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - Task { - await downloadStateDict.update(task: task, for: \.error, with: error) - downloadStateDictSubject.send(await downloadStateDict.getValues()) + Task { @NetworkingActor in + var mutableTask = downloadStateDict[task] + mutableTask?.error = error + downloadStateDict[task] = mutableTask await updateTasks() } } diff --git a/Sources/Networking/Core/DownloadAPIManaging.swift b/Sources/Networking/Core/DownloadAPIManaging.swift index 3fdc4bfd..ce5a7e8c 100644 --- a/Sources/Networking/Core/DownloadAPIManaging.swift +++ b/Sources/Networking/Core/DownloadAPIManaging.swift @@ -13,7 +13,8 @@ public typealias DownloadResult = (URLSessionDownloadTask, Response) /// A definition of an API layer with methods for handling data downloading. /// Recommended to be used as singleton. /// If you wish to use multiple instances, make sure you manually invalidate url session by calling the `invalidateSession` method. -public protocol DownloadAPIManaging: Sendable { +@NetworkingActor +public protocol DownloadAPIManaging { /// List of all currently ongoing download tasks. var allTasks: [URLSessionDownloadTask] { get async } From bde749f11d756ee2dc63b7af02ff1acebfdc6b92 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 29 Dec 2023 15:05:09 +0100 Subject: [PATCH 25/45] [feat] add APIManagerTests --- Tests/NetworkingTests/APIManagerTests.swift | 62 +++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Tests/NetworkingTests/APIManagerTests.swift diff --git a/Tests/NetworkingTests/APIManagerTests.swift b/Tests/NetworkingTests/APIManagerTests.swift new file mode 100644 index 00000000..cf03565f --- /dev/null +++ b/Tests/NetworkingTests/APIManagerTests.swift @@ -0,0 +1,62 @@ +// +// APIManagerTests.swift +// +// +// Created by Matej Molnár on 28.12.2023. +// + +@testable import Networking +import XCTest + +@NetworkingActor +final class APIManagerTests: XCTestCase { + enum UserRouter: Requestable { + case users(page: Int) + + var baseURL: URL { + // swiftlint:disable:next force_unwrapping + URL(string: "https://reqres.in/api")! + } + + var path: String { + switch self { + case .users: + "users" + } + } + + var urlParameters: [String: Any]? { + switch self { + case let .users(page): + ["page": page] + } + } + + var method: HTTPMethod { + switch self { + case .users: + .get + } + } + } + + private let mockSessionId = "2023-01-04T16:15:29Z" + + func testMultiThreadRequests() async throws { + let mockResponseProvider = MockResponseProvider(with: Bundle.module, sessionId: mockSessionId) + let apiManager = APIManager( + responseProvider: mockResponseProvider, + responseProcessors: [] + ) + + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + try await apiManager.request(UserRouter.users(page: 2)) + } + } + + try await group.waitForAll() + } + } +} From 41ecd2499eb06c4f0dba64c57a8b08d3ae28553c Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Fri, 29 Dec 2023 15:48:26 +0100 Subject: [PATCH 26/45] [feat] conform UploadAPIManaging to NetworkingActor --- .../Scenes/Upload/FormUploadsViewModel.swift | 6 +-- .../Scenes/Upload/UploadService.swift | 7 +--- .../Core/Upload/UploadAPIManager.swift | 42 ++++++++----------- .../Core/Upload/UploadAPIManaging.swift | 3 +- 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift index 426474a6..95ce761f 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/FormUploadsViewModel.swift @@ -25,11 +25,7 @@ final class FormUploadsViewModel: ObservableObject { return fileName } - private let uploadService: UploadService - - init(uploadService: UploadService = .init()) { - self.uploadService = uploadService - } + private let uploadService = UploadService.shared } extension FormUploadsViewModel { diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift index 830279aa..d8eb6514 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift @@ -10,14 +10,11 @@ import Networking // This import suppresses warning: Non-sendable type 'AsyncPublisher>' ... @preconcurrency import Combine +@NetworkingActor final class UploadService: Sendable { static let shared = UploadService() - private let uploadManager: UploadAPIManaging - - init(uploadManager: UploadAPIManaging = UploadAPIManager()) { - self.uploadManager = uploadManager - } + private let uploadManager = UploadAPIManager() } extension UploadService { diff --git a/Sources/Networking/Core/Upload/UploadAPIManager.swift b/Sources/Networking/Core/Upload/UploadAPIManager.swift index ff17ce26..99e628ed 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManager.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManager.swift @@ -5,25 +5,23 @@ // Created by Tony Ngo on 12.06.2023. // -@preconcurrency import Combine +import Combine import Foundation /// Default upload API manager -public actor UploadAPIManager: NSObject { +@NetworkingActor +open class UploadAPIManager: NSObject { // MARK: - Public Properties public var activeTasks: [UploadTask] { get async { let activeTasks = await urlSession.allTasks.compactMap { $0 as? URLSessionUploadTask } - return await uploadTasks - .getValues() - .values - // Values may contain inactive tasks - .filter { activeTasks.contains($0.task) } + // Values may contain inactive tasks + return uploadTasks.values.filter { activeTasks.contains($0.task) } } } // MARK: - Private Properties - private var uploadTasks = ThreadSafeDictionary() + private var uploadTasks = [String: UploadTask]() private lazy var urlSession = URLSession( configuration: urlSessionConfiguration, @@ -191,13 +189,13 @@ extension UploadAPIManager: UploadAPIManaging { public func retry(taskId: String) async throws { // Get stored upload task to invoke the request with the same arguments - guard let existingUploadTask = await uploadTasks.getValue(for: taskId) else { + guard let existingUploadTask = uploadTasks[taskId] else { throw NetworkError.unknown } // Removes the existing task from internal storage so that the `uploadRequest` // invocation treats the request/task as new - await uploadTasks.set(value: nil, for: taskId) + uploadTasks[taskId] = nil try await uploadRequest( existingUploadTask.uploadable, @@ -205,11 +203,8 @@ extension UploadAPIManager: UploadAPIManaging { ) } - public func stateStream(for uploadTaskId: UploadTask.ID) async -> StateStream { - let uploadTask = await uploadTasks - .getValues() - .values - .first { $0.id == uploadTaskId } + public func stateStream(for uploadTaskId: UploadTask.ID) -> StateStream { + let uploadTask = uploadTasks.values.first { $0.id == uploadTaskId } return uploadTask?.stateStream ?? Empty().eraseToAnyPublisher().values } @@ -230,14 +225,14 @@ private extension UploadAPIManager { for: urlRequest ) - let uploadTask = await existingUploadTaskOrNew( + let uploadTask = existingUploadTaskOrNew( for: sessionUploadTask, request: request, uploadable: uploadable ) // Store the task for future processing - await uploadTasks.set(value: uploadTask, for: request.id) + uploadTasks[request.id] = uploadTask sessionUploadTask.resume() return uploadTask @@ -251,8 +246,8 @@ private extension UploadAPIManager { for sessionUploadTask: URLSessionUploadTask, request: EndpointRequest, uploadable: Uploadable - ) async -> UploadTask { - guard var existingUploadTask = await uploadTasks.getValue(for: request.id) else { + ) -> UploadTask { + guard var existingUploadTask = uploadTasks[request.id] else { return UploadTask( sessionUploadTask: sessionUploadTask, endpointRequest: request, @@ -279,7 +274,7 @@ private extension UploadAPIManager { // Cleanup on successful task completion await uploadTask.cleanup() - await uploadTasks.set(value: nil, for: uploadTask.endpointRequest.id) + uploadTasks[uploadTask.endpointRequest.id] = nil } func handleUploadTaskError( @@ -336,11 +331,8 @@ private extension UploadAPIManager { return adaptedRequest } - func uploadTask(for task: URLSessionTask) async -> UploadTask? { - await uploadTasks - .getValues() - .values - .first { $0.taskIdentifier == task.taskIdentifier } + func uploadTask(for task: URLSessionTask) -> UploadTask? { + uploadTasks.values.first { $0.taskIdentifier == task.taskIdentifier } } func temporaryFileUrl(for request: EndpointRequest) throws -> URL { diff --git a/Sources/Networking/Core/Upload/UploadAPIManaging.swift b/Sources/Networking/Core/Upload/UploadAPIManaging.swift index 700db925..77fff3cf 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManaging.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManaging.swift @@ -8,6 +8,7 @@ import Combine import Foundation +@NetworkingActor public protocol UploadAPIManaging: Sendable { typealias StateStream = AsyncPublisher> @@ -63,7 +64,7 @@ public protocol UploadAPIManaging: Sendable { /// i.e., `UploadTask.State.error` is non-nil. In such case, you can call `retry(taskId:)` to re-activate the stream for the specified `uploadTaskId`. /// - Parameter uploadTaskId: The identifier of the task to observe. /// - Returns: An asynchronous stream of upload state. If there is no such upload task the return stream finishes immediately. - func stateStream(for uploadTaskId: UploadTask.ID) async -> StateStream + func stateStream(for uploadTaskId: UploadTask.ID) -> StateStream /// Invalidates the session with the option to wait for all outstanding (active) tasks. /// From fb2fc9517b01e6b89c7d6bd3077b081db88ce037 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Sat, 30 Dec 2023 17:05:01 +0100 Subject: [PATCH 27/45] [chore] remove unnecessary ThreadSafeDictionary --- .../Misc/ThreadSafeDictionary.swift | 40 ------------------- 1 file changed, 40 deletions(-) delete mode 100644 Sources/Networking/Misc/ThreadSafeDictionary.swift diff --git a/Sources/Networking/Misc/ThreadSafeDictionary.swift b/Sources/Networking/Misc/ThreadSafeDictionary.swift deleted file mode 100644 index d065bad3..00000000 --- a/Sources/Networking/Misc/ThreadSafeDictionary.swift +++ /dev/null @@ -1,40 +0,0 @@ -// -// ThreadSafeDictionary.swift -// -// -// Created by Dominika Gajdová on 25.05.2023. -// - -import Foundation - -/// A thread safe generic wrapper for dictionary. -actor ThreadSafeDictionary { - private var values = [Key: Value]() - - func getValues() -> [Key: Value] { - values - } - - func getValue(for task: Key) -> Value? { - values[task] - } - - func set(value: Value?, for task: Key) { - values[task] = value - } - - /// Updates the property of a given keyPath. - func update( - task: Key, - for keyPath: WritableKeyPath, - with value: Type - ) { - if var state = values[task] { - state[keyPath: keyPath] = value - values[task] = state - } - } -} - -// KeyPaths are not yet conforming to Sendable (https://github.com/apple/swift/issues/68943), hence we at least suppress the non-sendable warning. -extension WritableKeyPath: @unchecked Sendable {} From 9febe713db4be9bc8bbe7cc8980a96cbfaa7a56d Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Sat, 30 Dec 2023 17:23:33 +0100 Subject: [PATCH 28/45] [feat] conform other protocols and classes to @NetworkingActor --- .../API/SampleAuthorizationStorageManager.swift | 2 +- .../Scenes/Upload/UploadService.swift | 4 +--- .../NetworkingSampleApp/Scenes/Users/User.swift | 2 +- Sources/Networking/Core/APIManager.swift | 1 - Sources/Networking/Core/APIManaging.swift | 3 ++- .../Networking/Core/DownloadAPIManager.swift | 1 - .../Networking/Core/DownloadAPIManaging.swift | 2 +- Sources/Networking/Core/ErrorProcessing.swift | 1 + Sources/Networking/Core/NetworkingActor.swift | 3 ++- Sources/Networking/Core/RequestAdapting.swift | 1 + .../Networking/Core/ResponseProcessing.swift | 1 + .../Core/Upload/UploadAPIManager.swift | 17 ++++++++--------- .../Authorization/AuthorizationManaging.swift | 1 + .../AuthorizationStorageManaging.swift | 1 + .../AuthorizationTokenInterceptor.swift | 6 +++--- .../EndpointRequestStorageProcessor.swift | 1 - .../MultipeerConnectivityManager.swift | 3 ++- .../AuthorizationTokenInterceptorTests.swift | 14 +++++++------- .../StatusCodeProcessorTests.swift | 1 + 19 files changed, 34 insertions(+), 31 deletions(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationStorageManager.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationStorageManager.swift index d1b4c9c4..fdb28646 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationStorageManager.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationStorageManager.swift @@ -7,7 +7,7 @@ import Networking -actor SampleAuthorizationStorageManager: AuthorizationStorageManaging { +final class SampleAuthorizationStorageManager: AuthorizationStorageManaging { private var storage: AuthorizationData? func saveData(_ data: AuthorizationData) async throws { diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift index d8eb6514..761fd6ee 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Upload/UploadService.swift @@ -7,8 +7,6 @@ import Foundation import Networking -// This import suppresses warning: Non-sendable type 'AsyncPublisher>' ... -@preconcurrency import Combine @NetworkingActor final class UploadService: Sendable { @@ -57,7 +55,7 @@ extension UploadService { } func uploadStateStream(for uploadTaskId: String) async -> UploadAPIManaging.StateStream { - await uploadManager.stateStream(for: uploadTaskId) + uploadManager.stateStream(for: uploadTaskId) } func pause(taskId: String) async { diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift index f58b7bab..8a73f9e6 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift @@ -8,7 +8,7 @@ import Foundation /// Data structure of sample API user response -struct User: Codable, Identifiable { +struct User: Codable, Identifiable, Sendable { enum CodingKeys: String, CodingKey { case id case email diff --git a/Sources/Networking/Core/APIManager.swift b/Sources/Networking/Core/APIManager.swift index 15920d06..9ea6e07a 100644 --- a/Sources/Networking/Core/APIManager.swift +++ b/Sources/Networking/Core/APIManager.swift @@ -8,7 +8,6 @@ import Foundation /// Default API manager -@NetworkingActor open class APIManager: APIManaging, Retryable { private let requestAdapters: [RequestAdapting] private let responseProcessors: [ResponseProcessing] diff --git a/Sources/Networking/Core/APIManaging.swift b/Sources/Networking/Core/APIManaging.swift index d7766d83..4fcff361 100644 --- a/Sources/Networking/Core/APIManaging.swift +++ b/Sources/Networking/Core/APIManaging.swift @@ -11,7 +11,8 @@ import Foundation // MARK: - Defines API managing /// A definition of an API layer with methods for handling API requests. -public protocol APIManaging { +@NetworkingActor +public protocol APIManaging: Sendable { /// A default JSONDecoder used for all requests. var defaultDecoder: JSONDecoder { get } diff --git a/Sources/Networking/Core/DownloadAPIManager.swift b/Sources/Networking/Core/DownloadAPIManager.swift index 34d19a47..37de3f96 100644 --- a/Sources/Networking/Core/DownloadAPIManager.swift +++ b/Sources/Networking/Core/DownloadAPIManager.swift @@ -10,7 +10,6 @@ import Foundation @preconcurrency import Combine /// Default Download API manager -@NetworkingActor open class DownloadAPIManager: NSObject, Retryable { private let requestAdapters: [RequestAdapting] private let responseProcessors: [ResponseProcessing] diff --git a/Sources/Networking/Core/DownloadAPIManaging.swift b/Sources/Networking/Core/DownloadAPIManaging.swift index ce5a7e8c..66ed254d 100644 --- a/Sources/Networking/Core/DownloadAPIManaging.swift +++ b/Sources/Networking/Core/DownloadAPIManaging.swift @@ -14,7 +14,7 @@ public typealias DownloadResult = (URLSessionDownloadTask, Response) /// Recommended to be used as singleton. /// If you wish to use multiple instances, make sure you manually invalidate url session by calling the `invalidateSession` method. @NetworkingActor -public protocol DownloadAPIManaging { +public protocol DownloadAPIManaging: Sendable { /// List of all currently ongoing download tasks. var allTasks: [URLSessionDownloadTask] { get async } diff --git a/Sources/Networking/Core/ErrorProcessing.swift b/Sources/Networking/Core/ErrorProcessing.swift index 2ec61a81..ae0a7696 100644 --- a/Sources/Networking/Core/ErrorProcessing.swift +++ b/Sources/Networking/Core/ErrorProcessing.swift @@ -8,6 +8,7 @@ import Foundation /// A type that is able to customize error returned after failed network request. +@NetworkingActor public protocol ErrorProcessing: Sendable { /// Modifies a given `Error`. /// - Parameters: diff --git a/Sources/Networking/Core/NetworkingActor.swift b/Sources/Networking/Core/NetworkingActor.swift index fd17d24a..29889169 100644 --- a/Sources/Networking/Core/NetworkingActor.swift +++ b/Sources/Networking/Core/NetworkingActor.swift @@ -1,12 +1,13 @@ // // File.swift -// +// // // Created by Matej Molnár on 29.12.2023. // import Foundation +/// The only singleton actor in this library where all networking related operations should synchronize. @globalActor public actor NetworkingActor { public static let shared = NetworkingActor() } diff --git a/Sources/Networking/Core/RequestAdapting.swift b/Sources/Networking/Core/RequestAdapting.swift index 1dd07bf4..f51b204b 100644 --- a/Sources/Networking/Core/RequestAdapting.swift +++ b/Sources/Networking/Core/RequestAdapting.swift @@ -11,6 +11,7 @@ import Foundation // MARK: - Modifying the request before it's been sent /// A type that is able to modify a request before sending it to an API. +@NetworkingActor public protocol RequestAdapting: Sendable { /// Modifies a given `URLRequest`. /// - Parameters: diff --git a/Sources/Networking/Core/ResponseProcessing.swift b/Sources/Networking/Core/ResponseProcessing.swift index 72d257cb..65846a1b 100644 --- a/Sources/Networking/Core/ResponseProcessing.swift +++ b/Sources/Networking/Core/ResponseProcessing.swift @@ -11,6 +11,7 @@ import Foundation // MARK: - Defines modifying the response after it's been received /// A type that is able to modify a ``Response`` when it's received from the network layer. +@NetworkingActor public protocol ResponseProcessing: Sendable { /// Modifies a given ``Response``. /// - Parameters: diff --git a/Sources/Networking/Core/Upload/UploadAPIManager.swift b/Sources/Networking/Core/Upload/UploadAPIManager.swift index 99e628ed..9bdb0a98 100644 --- a/Sources/Networking/Core/Upload/UploadAPIManager.swift +++ b/Sources/Networking/Core/Upload/UploadAPIManager.swift @@ -9,8 +9,7 @@ import Combine import Foundation /// Default upload API manager -@NetworkingActor -open class UploadAPIManager: NSObject { +open class UploadAPIManager: NSObject, UploadAPIManaging { // MARK: - Public Properties public var activeTasks: [UploadTask] { get async { @@ -128,8 +127,8 @@ extension UploadAPIManager: URLSessionTaskDelegate { } // MARK: - UploadAPIManaging -extension UploadAPIManager: UploadAPIManaging { - public func invalidateSession(shouldFinishTasks: Bool) { +public extension UploadAPIManager { + func invalidateSession(shouldFinishTasks: Bool) { if shouldFinishTasks { urlSession.finishTasksAndInvalidate() } else { @@ -137,7 +136,7 @@ extension UploadAPIManager: UploadAPIManaging { } } - public func upload( + func upload( data: Data, to endpoint: Requestable ) async throws -> UploadTask { @@ -148,7 +147,7 @@ extension UploadAPIManager: UploadAPIManaging { ) } - public func upload( + func upload( fromFile fileUrl: URL, to endpoint: Requestable ) async throws -> UploadTask { @@ -159,7 +158,7 @@ extension UploadAPIManager: UploadAPIManaging { ) } - public func upload( + func upload( multipartFormData: MultipartFormData, sizeThreshold: UInt64 = 10_000_000, to endpoint: Requestable @@ -187,7 +186,7 @@ extension UploadAPIManager: UploadAPIManaging { } } - public func retry(taskId: String) async throws { + func retry(taskId: String) async throws { // Get stored upload task to invoke the request with the same arguments guard let existingUploadTask = uploadTasks[taskId] else { throw NetworkError.unknown @@ -203,7 +202,7 @@ extension UploadAPIManager: UploadAPIManaging { ) } - public func stateStream(for uploadTaskId: UploadTask.ID) -> StateStream { + func stateStream(for uploadTaskId: UploadTask.ID) -> StateStream { let uploadTask = uploadTasks.values.first { $0.id == uploadTaskId } return uploadTask?.stateStream ?? Empty().eraseToAnyPublisher().values diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift index da84d509..36f8dc02 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationManaging.swift @@ -9,6 +9,7 @@ import Foundation /// AuthorizationManaging authorizes requests and manages refresh token mechanism /// AuthorizationStorageManaging is required to read & store `AuthorizationData` +@NetworkingActor public protocol AuthorizationManaging: Sendable { var storage: any AuthorizationStorageManaging { get } diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift index b8fbeafe..042111cf 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationStorageManaging.swift @@ -9,6 +9,7 @@ import Foundation /// Basic operations to store `AuthorizationData` /// To keep consistency all operations are async +@NetworkingActor public protocol AuthorizationStorageManaging: Sendable { func saveData(_ data: AuthorizationData) async throws func getData() async throws -> AuthorizationData diff --git a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift index 6bc5a41d..d3e302c6 100644 --- a/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift +++ b/Sources/Networking/Modifiers/Interceptors/Authorization/AuthorizationTokenInterceptor.swift @@ -8,7 +8,7 @@ import Foundation // MARK: - Defines authentication handling in requests -public actor AuthorizationTokenInterceptor: RequestInterceptor { +open class AuthorizationTokenInterceptor: RequestInterceptor { private var authorizationManager: AuthorizationManaging private var refreshTask: Task? @@ -79,10 +79,10 @@ private extension AuthorizationTokenInterceptor { /// Perform the actual refresh logic. try await self?.authorizationManager.refreshAuthorizationData() /// Make sure to clear refreshTask property after refreshing finishes. - await self?.clearRefreshTask() + self?.clearRefreshTask() } catch { /// Make sure to clear refreshTask property after refreshing finishes. - await self?.clearRefreshTask() + self?.clearRefreshTask() throw error } } diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 79223c00..5e51bdac 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -20,7 +20,6 @@ import Foundation /// /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. -@NetworkingActor public class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift index ae6ed13b..8cae914e 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/MultipeerConnectivityManager.swift @@ -14,7 +14,8 @@ // @preconcurrency suppresses a swift concurrency warning: Non-sendable type ... @preconcurrency import MultipeerConnectivity -public actor MultipeerConnectivityManager: NSObject { +@NetworkingActor +public final class MultipeerConnectivityManager: NSObject { public static let service = "networking-jobs" public static let macOSAppDisplayName = "networking-macos-app" diff --git a/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift b/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift index acfb21f9..9535ffcb 100644 --- a/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift +++ b/Tests/NetworkingTests/AuthorizationTokenInterceptorTests.swift @@ -9,7 +9,7 @@ import XCTest // MARK: - Tests - +@NetworkingActor final class AuthorizationTokenInterceptorTests: XCTestCase { let mockSessionId = "mockSessionId" @@ -69,7 +69,7 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let refreshedAuthData = AuthorizationData.makeValidAuthorizationData() - await authManager.setRefreshedAuthorizationData(refreshedAuthData) + authManager.setRefreshedAuthorizationData(refreshedAuthData) let requestable = MockRouter.testAuthenticationRequired let request = URLRequest(url: requestable.baseURL) @@ -105,12 +105,12 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let expiredAuthData = AuthorizationData.makeExpiredAuthorizationData() /// Token refresh is going to take 0.5 seconds in order to test wether other requests actually wait for the refresh to finish. - await authManager.setSleepNanoseconds(500_000_000) + authManager.setSleepNanoseconds(500_000_000) try await authManager.storage.saveData(expiredAuthData) let refreshedAuthData = AuthorizationData.makeValidAuthorizationData() - await authManager.setRefreshedAuthorizationData(refreshedAuthData) + authManager.setRefreshedAuthorizationData(refreshedAuthData) let requestable = MockRouter.testAuthenticationRequired let request = URLRequest(url: requestable.baseURL) @@ -137,7 +137,7 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { let expiredAuthData = AuthorizationData.makeExpiredAuthorizationData() /// Token refresh is going to take 0.5 seconds in order to test wether other requests actually wait for the refresh to finish. - await authManager.setSleepNanoseconds(500_000_000) + authManager.setSleepNanoseconds(500_000_000) try await authManager.storage.saveData(expiredAuthData) let requestable = MockRouter.testAuthenticationRequired @@ -160,7 +160,7 @@ final class AuthorizationTokenInterceptorTests: XCTestCase { } // MARK: - Mock helper classes -private actor MockAuthorizationStorageManager: AuthorizationStorageManaging { +private class MockAuthorizationStorageManager: AuthorizationStorageManaging { private var storage: AuthorizationData? func saveData(_ data: AuthorizationData) async throws { @@ -180,7 +180,7 @@ private actor MockAuthorizationStorageManager: AuthorizationStorageManaging { } } -private actor MockAuthorizationManager: AuthorizationManaging { +private class MockAuthorizationManager: AuthorizationManaging { let storage: AuthorizationStorageManaging = MockAuthorizationStorageManager() private var sleepNanoseconds: UInt64 = 0 diff --git a/Tests/NetworkingTests/StatusCodeProcessorTests.swift b/Tests/NetworkingTests/StatusCodeProcessorTests.swift index 3240b711..6e390e0f 100644 --- a/Tests/NetworkingTests/StatusCodeProcessorTests.swift +++ b/Tests/NetworkingTests/StatusCodeProcessorTests.swift @@ -8,6 +8,7 @@ @testable import Networking import XCTest +@NetworkingActor final class StatusCodeProcessorTests: XCTestCase { private let sessionId = "sessionId_status_code" From 1effc96f83f96cbf33de2edeb8d04e7e8f67a4bf Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Sat, 30 Dec 2023 17:23:46 +0100 Subject: [PATCH 29/45] [fix] concurrency warning --- .../NetworkingSampleApp/Scenes/Users/UsersViewModel.swift | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift index 558b7dc2..66ded15b 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/UsersViewModel.swift @@ -58,7 +58,13 @@ extension UsersViewModel { } } - return try await group.reduce(into: [User]()) { $0.append($1) } + var results = [User]() + + for try await value in group { + results.append(value) + } + + return results } } else { // Fetch user add it to users array and wait for 0.5 seconds, before fetching the next one. From ab9ebcc93731034abd3f86e9ca177f32403a1069 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Sat, 30 Dec 2023 17:31:00 +0100 Subject: [PATCH 30/45] [chore] add comments --- Tests/NetworkingTests/APIManagerTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/NetworkingTests/APIManagerTests.swift b/Tests/NetworkingTests/APIManagerTests.swift index cf03565f..77796fd3 100644 --- a/Tests/NetworkingTests/APIManagerTests.swift +++ b/Tests/NetworkingTests/APIManagerTests.swift @@ -46,9 +46,11 @@ final class APIManagerTests: XCTestCase { let mockResponseProvider = MockResponseProvider(with: Bundle.module, sessionId: mockSessionId) let apiManager = APIManager( responseProvider: mockResponseProvider, + // Since one of the mocked responses returns 400 we don't want the test fail. responseProcessors: [] ) + // Create 15 parallel requests on multiple threads to test the managers thread safety. try await withThrowingTaskGroup(of: Void.self) { group in for _ in 0..<15 { group.addTask { From 2312444f9c9aa0a13e372ce92ebc839d85dddf7f Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 1 Jan 2024 12:40:13 +0100 Subject: [PATCH 31/45] [feat] add multithread DownloadAPIManagerTests --- .../DownloadAPIManagerTests.swift | 55 +++++++++++++++++++ .../Resources/download_test.txt | 1 + 2 files changed, 56 insertions(+) create mode 100644 Tests/NetworkingTests/DownloadAPIManagerTests.swift create mode 100644 Tests/NetworkingTests/Resources/download_test.txt diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift new file mode 100644 index 00000000..4b4b53e2 --- /dev/null +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -0,0 +1,55 @@ +// +// File.swift +// +// +// Created by Matej Molnár on 01.01.2024. +// + +@testable import Networking +import XCTest + +@NetworkingActor +final class DownloadAPIManagerTests: XCTestCase { + enum DownloadRouter: Requestable { + case download(url: URL) + + var baseURL: URL { + switch self { + case let .download(url): + url + } + } + + var path: String { + switch self { + case .download: + "" + } + } + } + + func testMultiThreadRequests() async throws { + // Set empty response processors since the mock download requests return status code 0 and we don't want the test fail. + let apiManager = DownloadAPIManager(responseProcessors: []) + + // We can simulate the download even with a local file. + guard let downloadUrl = Bundle.module.url(forResource: "download_test", withExtension: "txt") else { + XCTFail("Resource not found") + return + } + + // Create 15 parallel requests on multiple threads to test the managers thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.downloadRequest( + DownloadRouter.download(url: downloadUrl), + retryConfiguration: nil + ) + } + } + + try await group.waitForAll() + } + } +} diff --git a/Tests/NetworkingTests/Resources/download_test.txt b/Tests/NetworkingTests/Resources/download_test.txt new file mode 100644 index 00000000..9452afb6 --- /dev/null +++ b/Tests/NetworkingTests/Resources/download_test.txt @@ -0,0 +1 @@ +just a test download file From f2bff45e078b68e2d5bb43148f7389c1db6cedf9 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 1 Jan 2024 12:45:43 +0100 Subject: [PATCH 32/45] [feat] change configuration --- Tests/NetworkingTests/DownloadAPIManagerTests.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index 4b4b53e2..c3813253 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -29,8 +29,12 @@ final class DownloadAPIManagerTests: XCTestCase { } func testMultiThreadRequests() async throws { - // Set empty response processors since the mock download requests return status code 0 and we don't want the test fail. - let apiManager = DownloadAPIManager(responseProcessors: []) + let apiManager = DownloadAPIManager( + // A session configuration that uses no persistent storage for caches, cookies, or credentials. + urlSessionConfiguration: .ephemeral, + // Set empty response processors since the mock download requests return status code 0 and we don't want the test fail. + responseProcessors: [] + ) // We can simulate the download even with a local file. guard let downloadUrl = Bundle.module.url(forResource: "download_test", withExtension: "txt") else { From 9dd468a66d681e2d50d1e5095333e4533c0fa5dd Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 1 Jan 2024 12:48:53 +0100 Subject: [PATCH 33/45] [chore] fix typo --- Tests/NetworkingTests/APIManagerTests.swift | 2 +- Tests/NetworkingTests/DownloadAPIManagerTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/NetworkingTests/APIManagerTests.swift b/Tests/NetworkingTests/APIManagerTests.swift index 77796fd3..c529fa45 100644 --- a/Tests/NetworkingTests/APIManagerTests.swift +++ b/Tests/NetworkingTests/APIManagerTests.swift @@ -50,7 +50,7 @@ final class APIManagerTests: XCTestCase { responseProcessors: [] ) - // Create 15 parallel requests on multiple threads to test the managers thread safety. + // Create 15 parallel requests on multiple threads to test the manager's thread safety. try await withThrowingTaskGroup(of: Void.self) { group in for _ in 0..<15 { group.addTask { diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index c3813253..9792c685 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -42,7 +42,7 @@ final class DownloadAPIManagerTests: XCTestCase { return } - // Create 15 parallel requests on multiple threads to test the managers thread safety. + // Create 15 parallel requests on multiple threads to test the manager's thread safety. try await withThrowingTaskGroup(of: Void.self) { group in for _ in 0..<15 { group.addTask { From 24b7b307acf92083be5d48bdf8e7ec97624dcbd2 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 1 Jan 2024 15:59:07 +0100 Subject: [PATCH 34/45] [feat] add UploadAPIManagerTests --- .../UploadAPIManagerTests.swift | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Tests/NetworkingTests/UploadAPIManagerTests.swift diff --git a/Tests/NetworkingTests/UploadAPIManagerTests.swift b/Tests/NetworkingTests/UploadAPIManagerTests.swift new file mode 100644 index 00000000..029fee33 --- /dev/null +++ b/Tests/NetworkingTests/UploadAPIManagerTests.swift @@ -0,0 +1,48 @@ +// +// File.swift +// +// +// Created by Matej Molnár on 01.01.2024. +// + +@testable import Networking +import XCTest + +@NetworkingActor +final class UploadAPIManagerTests: XCTestCase { + enum UploadRouter: Requestable { + case mock + + var baseURL: URL { + // swiftlint:disable:next force_unwrapping + URL(string: "https://uploadAPIManager.tests")! + } + + var path: String { + "/mock" + } + + var method: HTTPMethod { + .post + } + } + + func testMultiThreadRequests2() async throws { + let apiManager = UploadAPIManager( + // A session configuration that uses no persistent storage for caches, cookies, or credentials. + urlSessionConfiguration: .ephemeral + ) + let data = Data("Test data".utf8) + + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.upload(data: data, to: UploadRouter.mock) + } + } + + try await group.waitForAll() + } + } +} From f54eb2bf15e41f16c50ddc539e8e7b8eddba7aa2 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 2 Jan 2024 17:22:48 +0100 Subject: [PATCH 35/45] Update Tests/NetworkingTests/UploadAPIManagerTests.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Kodeš --- Tests/NetworkingTests/UploadAPIManagerTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NetworkingTests/UploadAPIManagerTests.swift b/Tests/NetworkingTests/UploadAPIManagerTests.swift index 029fee33..bb1b2e24 100644 --- a/Tests/NetworkingTests/UploadAPIManagerTests.swift +++ b/Tests/NetworkingTests/UploadAPIManagerTests.swift @@ -27,7 +27,7 @@ final class UploadAPIManagerTests: XCTestCase { } } - func testMultiThreadRequests2() async throws { + func testMultiThreadRequests() async throws { let apiManager = UploadAPIManager( // A session configuration that uses no persistent storage for caches, cookies, or credentials. urlSessionConfiguration: .ephemeral From 2590c044708b1d624a34d4afbceb34cbacc3cdb2 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 2 Jan 2024 17:23:13 +0100 Subject: [PATCH 36/45] Update Tests/NetworkingTests/DownloadAPIManagerTests.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Kodeš --- Tests/NetworkingTests/DownloadAPIManagerTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index 9792c685..ee2a1597 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -1,5 +1,5 @@ // -// File.swift +// DownloadAPIManagerTests.swift // // // Created by Matej Molnár on 01.01.2024. From 348161639510f94b7cd11e59af13607ca13a32d4 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 2 Jan 2024 18:01:00 +0100 Subject: [PATCH 37/45] [fix] multithread managers tests --- Tests/NetworkingTests/APIManagerTests.swift | 23 ++++++++++----- .../DownloadAPIManagerTests.swift | 29 ++++++++++++------- .../UploadAPIManagerTests.swift | 24 +++++++++------ 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/Tests/NetworkingTests/APIManagerTests.swift b/Tests/NetworkingTests/APIManagerTests.swift index c529fa45..57022e19 100644 --- a/Tests/NetworkingTests/APIManagerTests.swift +++ b/Tests/NetworkingTests/APIManagerTests.swift @@ -42,7 +42,7 @@ final class APIManagerTests: XCTestCase { private let mockSessionId = "2023-01-04T16:15:29Z" - func testMultiThreadRequests() async throws { + func testMultiThreadRequests() { let mockResponseProvider = MockResponseProvider(with: Bundle.module, sessionId: mockSessionId) let apiManager = APIManager( responseProvider: mockResponseProvider, @@ -50,15 +50,22 @@ final class APIManagerTests: XCTestCase { responseProcessors: [] ) - // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - try await apiManager.request(UserRouter.users(page: 2)) + let expectation = XCTestExpectation(description: "Requests completed") + + Task { + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + try await apiManager.request(UserRouter.users(page: 2)) + } } - } - try await group.waitForAll() + try await group.waitForAll() + expectation.fulfill() + } } + + wait(for: [expectation], timeout: 1) } } diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index ee2a1597..b01976c1 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -28,7 +28,7 @@ final class DownloadAPIManagerTests: XCTestCase { } } - func testMultiThreadRequests() async throws { + func testMultiThreadRequests() { let apiManager = DownloadAPIManager( // A session configuration that uses no persistent storage for caches, cookies, or credentials. urlSessionConfiguration: .ephemeral, @@ -41,19 +41,26 @@ final class DownloadAPIManagerTests: XCTestCase { XCTFail("Resource not found") return } + + let expectation = XCTestExpectation(description: "Downloads completed") - // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - _ = try await apiManager.downloadRequest( - DownloadRouter.download(url: downloadUrl), - retryConfiguration: nil - ) + Task { + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.downloadRequest( + DownloadRouter.download(url: downloadUrl), + retryConfiguration: nil + ) + } } - } - try await group.waitForAll() + try await group.waitForAll() + expectation.fulfill() + } } + + wait(for: [expectation], timeout: 1) } } diff --git a/Tests/NetworkingTests/UploadAPIManagerTests.swift b/Tests/NetworkingTests/UploadAPIManagerTests.swift index bb1b2e24..fc62de09 100644 --- a/Tests/NetworkingTests/UploadAPIManagerTests.swift +++ b/Tests/NetworkingTests/UploadAPIManagerTests.swift @@ -27,22 +27,28 @@ final class UploadAPIManagerTests: XCTestCase { } } - func testMultiThreadRequests() async throws { + func testMultiThreadRequests() { let apiManager = UploadAPIManager( // A session configuration that uses no persistent storage for caches, cookies, or credentials. urlSessionConfiguration: .ephemeral ) let data = Data("Test data".utf8) - - // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - _ = try await apiManager.upload(data: data, to: UploadRouter.mock) + let expectation = XCTestExpectation(description: "Uploads completed") + + Task { + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.upload(data: data, to: UploadRouter.mock) + } } - } - try await group.waitForAll() + try await group.waitForAll() + expectation.fulfill() + } } + + wait(for: [expectation], timeout: 1) } } From c45f120d05a4e6c623bae80262245e31805a511d Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Wed, 3 Jan 2024 17:13:34 +0100 Subject: [PATCH 38/45] [fix] manager tests --- Tests/NetworkingTests/APIManagerTests.swift | 20 +++++++++------- .../DownloadAPIManagerTests.swift | 24 +++++++++++-------- .../UploadAPIManagerTests.swift | 20 +++++++++------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/Tests/NetworkingTests/APIManagerTests.swift b/Tests/NetworkingTests/APIManagerTests.swift index 57022e19..d53414b2 100644 --- a/Tests/NetworkingTests/APIManagerTests.swift +++ b/Tests/NetworkingTests/APIManagerTests.swift @@ -53,16 +53,20 @@ final class APIManagerTests: XCTestCase { let expectation = XCTestExpectation(description: "Requests completed") Task { - // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - try await apiManager.request(UserRouter.users(page: 2)) + do { + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + try await apiManager.request(UserRouter.users(page: 2)) + } } - } - try await group.waitForAll() - expectation.fulfill() + try await group.waitForAll() + expectation.fulfill() + } + } catch { + XCTFail(error.localizedDescription) } } diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index b01976c1..92406079 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -46,18 +46,22 @@ final class DownloadAPIManagerTests: XCTestCase { Task { // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - _ = try await apiManager.downloadRequest( - DownloadRouter.download(url: downloadUrl), - retryConfiguration: nil - ) + do { + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.downloadRequest( + DownloadRouter.download(url: downloadUrl), + retryConfiguration: nil + ) + } } - } - try await group.waitForAll() - expectation.fulfill() + try await group.waitForAll() + expectation.fulfill() + } + } catch { + XCTFail(error.localizedDescription) } } diff --git a/Tests/NetworkingTests/UploadAPIManagerTests.swift b/Tests/NetworkingTests/UploadAPIManagerTests.swift index fc62de09..f4496e88 100644 --- a/Tests/NetworkingTests/UploadAPIManagerTests.swift +++ b/Tests/NetworkingTests/UploadAPIManagerTests.swift @@ -36,16 +36,20 @@ final class UploadAPIManagerTests: XCTestCase { let expectation = XCTestExpectation(description: "Uploads completed") Task { - // Create 15 parallel requests on multiple threads to test the manager's thread safety. - try await withThrowingTaskGroup(of: Void.self) { group in - for _ in 0..<15 { - group.addTask { - _ = try await apiManager.upload(data: data, to: UploadRouter.mock) + do { + // Create 15 parallel requests on multiple threads to test the manager's thread safety. + try await withThrowingTaskGroup(of: Void.self) { group in + for _ in 0..<15 { + group.addTask { + _ = try await apiManager.upload(data: data, to: UploadRouter.mock) + } } - } - try await group.waitForAll() - expectation.fulfill() + try await group.waitForAll() + expectation.fulfill() + } + } catch { + XCTFail(error.localizedDescription) } } From dee8532cf03fc209dc0896cdaca1e765454fd5bc Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Mon, 8 Jan 2024 22:30:16 +0100 Subject: [PATCH 39/45] [chore] revert some classes back to open --- .../Networking/Modifiers/Interceptors/LoggingInterceptor.swift | 2 +- .../EndpointRequestStorageProcessor.swift | 2 +- .../Networking/Modifiers/Processors/StatusCodeProcessor.swift | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift b/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift index b46f0cda..2ef7327e 100644 --- a/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift +++ b/Sources/Networking/Modifiers/Interceptors/LoggingInterceptor.swift @@ -15,7 +15,7 @@ import Foundation // MARK: - Pretty logging modifier /// ``RequestInterceptor`` which logs requests & responses info into console in pretty way -public final class LoggingInterceptor: RequestInterceptor { +open class LoggingInterceptor: RequestInterceptor { // MARK: Default shared instance public static let shared = LoggingInterceptor() diff --git a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift index 5e51bdac..315c1ec3 100644 --- a/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/EndpointRequestStorageProcessor/EndpointRequestStorageProcessor.swift @@ -20,7 +20,7 @@ import Foundation /// /// The filename is created from a sessionId and a corresponding request identifier. /// Stored files are stored under session folder and can be added to NSAssetCatalog and read via `SampleDataNetworking` to replay whole session. -public class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { +open class EndpointRequestStorageProcessor: ResponseProcessing, ErrorProcessing { // MARK: Private variables private let fileManager: FileManager private let jsonEncoder: JSONEncoder diff --git a/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift b/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift index ad86213a..cd907b5a 100644 --- a/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift +++ b/Sources/Networking/Modifiers/Processors/StatusCodeProcessor.swift @@ -10,7 +10,7 @@ import Foundation // MARK: - Modifier handling validity of response http status codes /// A response processor validating ``Response`` http status code against ``Requestable`` API endpoint definition. -public final class StatusCodeProcessor: ResponseProcessing { +open class StatusCodeProcessor: ResponseProcessing { // MARK: Default shared instance public static let shared = StatusCodeProcessor() From 9645825e7a50d477137b60bf1246347958bb4581 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 9 Jan 2024 17:03:15 +0100 Subject: [PATCH 40/45] [chore] update file name comments --- Sources/Networking/Core/NetworkingActor.swift | 2 +- Tests/NetworkingTests/UploadAPIManagerTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Networking/Core/NetworkingActor.swift b/Sources/Networking/Core/NetworkingActor.swift index 29889169..46f6c798 100644 --- a/Sources/Networking/Core/NetworkingActor.swift +++ b/Sources/Networking/Core/NetworkingActor.swift @@ -1,5 +1,5 @@ // -// File.swift +// NetworkingActor.swift // // // Created by Matej Molnár on 29.12.2023. diff --git a/Tests/NetworkingTests/UploadAPIManagerTests.swift b/Tests/NetworkingTests/UploadAPIManagerTests.swift index f4496e88..260350ce 100644 --- a/Tests/NetworkingTests/UploadAPIManagerTests.swift +++ b/Tests/NetworkingTests/UploadAPIManagerTests.swift @@ -1,5 +1,5 @@ // -// File.swift +// UploadAPIManagerTests.swift // // // Created by Matej Molnár on 01.01.2024. From 296c37222d49a2e6f6c8c313d45ca1418b15148a Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 9 Jan 2024 17:03:47 +0100 Subject: [PATCH 41/45] [chore] remove unnecessary explicit Sendable conformance --- NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift index 8a73f9e6..f58b7bab 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/Scenes/Users/User.swift @@ -8,7 +8,7 @@ import Foundation /// Data structure of sample API user response -struct User: Codable, Identifiable, Sendable { +struct User: Codable, Identifiable { enum CodingKeys: String, CodingKey { case id case email From 987d49d65d6955816111da08d1f1ca6f5138e0fc Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Tue, 9 Jan 2024 17:04:00 +0100 Subject: [PATCH 42/45] [chore] add final statement --- .../NetworkingSampleApp/API/SampleAuthorizationManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift index 54b72863..d9668089 100644 --- a/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift +++ b/NetworkingSampleApp/NetworkingSampleApp/API/SampleAuthorizationManager.swift @@ -9,7 +9,7 @@ import Networking import Foundation @NetworkingActor -class SampleAuthorizationManager: AuthorizationManaging { +final class SampleAuthorizationManager: AuthorizationManaging { // MARK: Public properties let storage: AuthorizationStorageManaging = SampleAuthorizationStorageManager() From d6613e04bf8a407d7f39811de3af503a5ed261ac Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Thu, 11 Jan 2024 21:55:35 +0100 Subject: [PATCH 43/45] [feat] change class to open --- Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift b/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift index 2d6aa64a..8f3929fa 100644 --- a/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift +++ b/Sources/Networking/Core/Upload/MultipartFormDataEncoder.swift @@ -7,7 +7,7 @@ import Foundation -public final class MultipartFormDataEncoder { +open class MultipartFormDataEncoder: @unchecked Sendable { /// A string representing a carriage return and line feed. private let crlf = "\r\n" From 0b170b4118af9594038482016cb1fd14c1df958e Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Thu, 11 Jan 2024 22:12:09 +0100 Subject: [PATCH 44/45] [feat] remove unnecessary async statement --- Sources/Networking/Core/DownloadAPIManaging.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Networking/Core/DownloadAPIManaging.swift b/Sources/Networking/Core/DownloadAPIManaging.swift index 66ed254d..e372fb62 100644 --- a/Sources/Networking/Core/DownloadAPIManaging.swift +++ b/Sources/Networking/Core/DownloadAPIManaging.swift @@ -21,7 +21,7 @@ public protocol DownloadAPIManaging: Sendable { /// Invalidates urlSession instance. /// - Parameters: /// - shouldFinishTasks: Indicates whether all currently active tasks should be able to finish before invalidating. Otherwise they will be cancelled. - func invalidateSession(shouldFinishTasks: Bool) async + func invalidateSession(shouldFinishTasks: Bool) /// Initiates a download request for a given endpoint, with optional resumable data and retry configuration. /// - Parameters: From 7b8c5df3c5141ce47f59e3e2c8567bff77609367 Mon Sep 17 00:00:00 2001 From: Matej Molnar Date: Sun, 14 Jan 2024 19:19:53 +0100 Subject: [PATCH 45/45] [fix] DownloadAPIManager tests by adjusting asyncResponse method --- Sources/Networking/Core/DownloadAPIManager.swift | 14 +++++--------- ...ift => URLSessionTask+ResumeWithResponse.swift} | 10 ++++++---- .../NetworkingTests/DownloadAPIManagerTests.swift | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) rename Sources/Networking/Misc/{URLSessionTask+AsyncResponse.swift => URLSessionTask+ResumeWithResponse.swift} (84%) diff --git a/Sources/Networking/Core/DownloadAPIManager.swift b/Sources/Networking/Core/DownloadAPIManager.swift index 37de3f96..5ff7a7dd 100644 --- a/Sources/Networking/Core/DownloadAPIManager.swift +++ b/Sources/Networking/Core/DownloadAPIManager.swift @@ -121,17 +121,15 @@ private extension DownloadAPIManager { return urlSession.downloadTask(with: request) } }() - + /// downloadTask must be initiated by resume() before we try to await a response from downloadObserver, because it gets the response from URLSessionDownloadDelegate methods - downloadTask.resume() - - await updateTasks() - - let urlResponse = try await downloadTask.asyncResponse() - + let urlResponse = try await downloadTask.resumeWithResponse() + /// process response let response = try await responseProcessors.process((Data(), urlResponse), with: request, for: endpointRequest) + await updateTasks() + /// reset retry count retryCounter.reset(for: endpointRequest.id) @@ -214,7 +212,6 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { var mutableTask = downloadStateDict[downloadTask] mutableTask?.downloadedFileURL = tempURL downloadStateDict[downloadTask] = mutableTask - await updateTasks() } } catch { Task { @NetworkingActor in @@ -230,7 +227,6 @@ extension DownloadAPIManager: URLSessionDelegate, URLSessionDownloadDelegate { var mutableTask = downloadStateDict[task] mutableTask?.error = error downloadStateDict[task] = mutableTask - await updateTasks() } } } diff --git a/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift b/Sources/Networking/Misc/URLSessionTask+ResumeWithResponse.swift similarity index 84% rename from Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift rename to Sources/Networking/Misc/URLSessionTask+ResumeWithResponse.swift index cada6145..c313392c 100644 --- a/Sources/Networking/Misc/URLSessionTask+AsyncResponse.swift +++ b/Sources/Networking/Misc/URLSessionTask+ResumeWithResponse.swift @@ -1,5 +1,5 @@ // -// URLSessionTask+AsyncResponse.swift +// URLSessionTask+ResumeWithResponse.swift // // // Created by Dominika Gajdová on 12.05.2023. @@ -10,12 +10,12 @@ import Foundation @preconcurrency import Combine extension URLSessionTask { - func asyncResponse() async throws -> URLResponse { + func resumeWithResponse() async throws -> URLResponse { var cancellable: AnyCancellable? - + return try await withTaskCancellationHandler( operation: { - try await withCheckedThrowingContinuation { continuation in + return try await withCheckedThrowingContinuation { continuation in cancellable = Publishers.CombineLatest( publisher(for: \.response), publisher(for: \.error) @@ -30,6 +30,8 @@ extension URLSessionTask { continuation.resume(returning: response) } } + + resume() } }, onCancel: { [cancellable] in diff --git a/Tests/NetworkingTests/DownloadAPIManagerTests.swift b/Tests/NetworkingTests/DownloadAPIManagerTests.swift index 92406079..54f3f396 100644 --- a/Tests/NetworkingTests/DownloadAPIManagerTests.swift +++ b/Tests/NetworkingTests/DownloadAPIManagerTests.swift @@ -65,6 +65,6 @@ final class DownloadAPIManagerTests: XCTestCase { } } - wait(for: [expectation], timeout: 1) + wait(for: [expectation], timeout: 5) } }