Better concurrency support in iOS API Manager

Example/FlagsmithClient/AppDelegate.swift

@@ -9,11 +9,15 @@
 import UIKit
 import FlagsmithClient
 
+func isSuccess<T,F>(_ result: Result<T,F>) -> Bool {
+    if case .success = result { return true } else { return false }
+}
+
 @UIApplicationMain
 class AppDelegate: UIResponder, UIApplicationDelegate {
 
   var window: UIWindow?
-
+  let concurrentQueue = DispatchQueue(label: "concurrentQueue", attributes: .concurrent)
 
   func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
     // Override point for customization after application launch.
@@ -48,6 +52,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
     Flagsmith.shared.hasFeatureFlag(withID: "freeze_delinquent_accounts") { (result) in
       print(result)
     }
+
+    // Try getting the feature flags concurrently
+    for concurrentIteration in 1...20 {
+        concurrentQueue.async {
+          Flagsmith.shared.getFeatureFlags() { (result) in
+            print("Concurrent \(concurrentIteration) result success: \(isSuccess(result)) on \(Thread.current)")
+          }
+        }
+    }
+
     //Flagsmith.shared.setTrait(Trait(key: "<my_key>", value: "<my_value>"), forIdentity: "<my_identity>") { (result) in print(result) }
     //Flagsmith.shared.getIdentity("<my_key>") { (result) in print(result) }
     return true

FlagsmithClient/Classes/Internal/APIManager.swift

@@ -21,8 +21,9 @@ class APIManager : NSObject, URLSessionDataDelegate {
   var apiKey: String?
 
   // store the completion handlers and accumulated data for each task
-  private var tasksToCompletionHandlers:[URLSessionDataTask:(Result<Data, Error>) -> Void] = [:]
-  private var tasksToData:[URLSessionDataTask:NSMutableData] = [:]
+  private var tasksToCompletionHandlers:[Int:(Result<Data, Error>) -> Void] = [:]
+  private var tasksToData:[Int:Data] = [:]
+    private let serialAccessQueue = DispatchQueue(label: "flagsmithSerialAccessQueue")
 
   override init() {
     super.init()
@@ -31,36 +32,42 @@ class APIManager : NSObject, URLSessionDataDelegate {
   }
 
   func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
-    if let dataTask = task as? URLSessionDataTask {
-      if let completion = tasksToCompletionHandlers[dataTask] {
-        if let error = error {
-          completion(.failure(FlagsmithError.unhandled(error)))
-        }
-        else {
-          let data = tasksToData[dataTask] ?? NSMutableData()
-          completion(.success(data as Data))
+    serialAccessQueue.sync {
+      if let dataTask = task as? URLSessionDataTask {
+        if let completion = tasksToCompletionHandlers[dataTask.taskIdentifier] {
+          if let error = error {
+            DispatchQueue.main.async { completion(.failure(FlagsmithError.unhandled(error))) }
+          }
+          else {
+            let data = tasksToData[dataTask.taskIdentifier] ?? Data()
+            DispatchQueue.main.async { completion(.success(data)) }
+          }
         }
+        tasksToCompletionHandlers[dataTask.taskIdentifier] = nil
+        tasksToData[dataTask.taskIdentifier] = nil
       }
-      tasksToCompletionHandlers[dataTask] = nil
-      tasksToData[dataTask] = nil
     }
   }
 
-  func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, willCacheResponse proposedResponse: CachedURLResponse, completionHandler: @escaping (CachedURLResponse?) -> Void) {
-
-    // intercept and modify the cache settings for the response
-    if Flagsmith.shared.cacheConfig.useCache {
-      let newResponse = proposedResponse.response(withExpirationDuration: Int(Flagsmith.shared.cacheConfig.cacheTTL))
-      completionHandler(newResponse)
-    } else {
-      completionHandler(proposedResponse)
+  func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, willCacheResponse proposedResponse: CachedURLResponse, 
+                  completionHandler: @escaping (CachedURLResponse?) -> Void) {
+    serialAccessQueue.sync {
+      // intercept and modify the cache settings for the response
+      if Flagsmith.shared.cacheConfig.useCache {
+        let newResponse = proposedResponse.response(withExpirationDuration: Int(Flagsmith.shared.cacheConfig.cacheTTL))
+        DispatchQueue.main.async { completionHandler(newResponse) }
+      } else {
+        DispatchQueue.main.async { completionHandler(proposedResponse) }
+      }
     }
   }
 
   func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
-    let existingData = tasksToData[dataTask] ?? NSMutableData()
-    existingData.append(data)
-    tasksToData[dataTask] = existingData
+    serialAccessQueue.sync {
+      var existingData = tasksToData[dataTask.taskIdentifier] ?? Data()
+      existingData.append(data)
+      tasksToData[dataTask.taskIdentifier] = existingData
+    }
   }
 
     func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) {
@@ -98,9 +105,11 @@ class APIManager : NSObject, URLSessionDataDelegate {
     }
 
     // we must use the delegate form here, not the completion handler, to be able to modify the cache
-    let task = session.dataTask(with: request)
-    tasksToCompletionHandlers[task] = completion
-    task.resume()
+    serialAccessQueue.sync {
+      let task = session.dataTask(with: request)
+      tasksToCompletionHandlers[task.taskIdentifier] = completion
+      task.resume()
+    }
   }
 
   /// Requests a api route and only relays success or failure of the action.

FlagsmithClient/Classes/Internal/Router.swift

@@ -10,7 +10,7 @@ import Foundation
 import FoundationNetworking
 #endif
 
-enum Router {
+enum Router: Sendable {
   private enum HTTPMethod: String {
     case get = "GET"
     case post = "POST"

FlagsmithClient/Tests/APIManagerTests.swift

@@ -60,4 +60,25 @@ final class APIManagerTests: FlagsmithClientTestCase {
             return
         }
     }
+
+    func testConcurrentRequests() throws {
+        apiManager.apiKey = "8D5ABC87-6BBF-4AE7-BC05-4DC1AFE770DF"
+        let concurrentQueue = DispatchQueue(label: "concurrentQueue", attributes: .concurrent)
+
+        var expectations:[XCTestExpectation] = [];
+        let iterations = 1000
+
+        for concurrentIteration in 1...iterations {
+            let expectation = XCTestExpectation(description: "Multiple threads can access the APIManager \(concurrentIteration)")
+            expectations.append(expectation)
+            concurrentQueue.async {
+                self.apiManager.request(.getFlags) { (result: Result<Void, Error>) in
+                    // We're not fussed at this point what the result is
+                    expectation.fulfill()
+                }
+            }
+        }
+
+        wait(for: expectations, timeout: 5)
+    }
 }