feat: make APIClient run synchronized

Bucketeer.xcodeproj/project.pbxproj

@@ -121,6 +121,7 @@
 		93AC8F8028E351C500A4719B /* ScheduledTask.swift in Sources */ = {isa = PBXBuildFile; fileRef = 93AC8F7F28E351C500A4719B /* ScheduledTask.swift */; };
 		941E007E2A4FD964002CBFBB /* BKTConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 941E007D2A4FD964002CBFBB /* BKTConfigTests.swift */; };
 		949542E42A3AEBC0008D0C60 /* MetricsEventUniqueKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 949542E32A3AEBC0008D0C60 /* MetricsEventUniqueKeyTests.swift */; };
+		94E130F22A8DD07700120F84 /* Logger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94E130F12A8DD07700120F84 /* Logger.swift */; };
 		E2BE019D2A531C120040F40F /* BKTUserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2BE019C2A531C120040F40F /* BKTUserTests.swift */; };
 		EB2310E2209D91640023A98D /* SecondViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = EB2310E1209D91640023A98D /* SecondViewController.swift */; };
 		EB2310E4209D92570023A98D /* ThirdViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = EB2310E3209D92570023A98D /* ThirdViewController.swift */; };
@@ -288,6 +289,7 @@
 		93AC8F7F28E351C500A4719B /* ScheduledTask.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScheduledTask.swift; sourceTree = "<group>"; };
 		941E007D2A4FD964002CBFBB /* BKTConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BKTConfigTests.swift; sourceTree = "<group>"; };
 		949542E32A3AEBC0008D0C60 /* MetricsEventUniqueKeyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MetricsEventUniqueKeyTests.swift; sourceTree = "<group>"; };
+		94E130F12A8DD07700120F84 /* Logger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Logger.swift; sourceTree = "<group>"; };
 		E2BE019C2A531C120040F40F /* BKTUserTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BKTUserTests.swift; sourceTree = "<group>"; };
 		EB2310E1209D91640023A98D /* SecondViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecondViewController.swift; sourceTree = "<group>"; };
 		EB2310E3209D92570023A98D /* ThirdViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThirdViewController.swift; sourceTree = "<group>"; };
@@ -681,6 +683,7 @@
 				EB2310E1209D91640023A98D /* SecondViewController.swift */,
 				EB26128C209863F100D62282 /* SplashViewController.swift */,
 				EB2310E3209D92570023A98D /* ThirdViewController.swift */,
+				94E130F12A8DD07700120F84 /* Logger.swift */,
 			);
 			path = Example;
 			sourceTree = "<group>";
@@ -1037,6 +1040,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				EB7867F9209722050071D524 /* AppDelegate.swift in Sources */,
+				94E130F22A8DD07700120F84 /* Logger.swift in Sources */,
 				EB7867FB209722050071D524 /* FirstViewController.swift in Sources */,
 				EB2310E2209D91640023A98D /* SecondViewController.swift in Sources */,
 				EB26128D209863F100D62282 /* SplashViewController.swift in Sources */,

Bucketeer/Sources/Internal/Event/EventInteractor.swift

@@ -220,26 +220,19 @@ final class EventInteractorImpl: EventInteractor {
     }
 
     func sendEvents(force: Bool, completion: ((Result<Bool, BKTError>) -> Void)?) {
-        // https://github.com/bucketeer-io/ios-client-sdk/issues/23
-        // Only allow one `sendEvents` action
-        // The `Semaphore` will unlock after the request is success or fail
-        sendEventSemaphore.wait()
-        let callback : ((Result<Bool, BKTError>) -> Void) = { [weak self] rs in
-            completion?(rs)
-            // Release lock
-            self?.sendEventSemaphore.signal()
-        }
+        logger?.debug(message:"sendEvents called")
         do {
             let currentEvents = try eventDao.getEvents()
             guard !currentEvents.isEmpty else {
                 logger?.debug(message: "no events to register")
-                callback(.success(false))
+                completion?(.success(false))
                 return
             }
 
+            logger?.debug(message:"currentEvents.count \(currentEvents.count)")
             guard force || currentEvents.count >= eventsMaxBatchQueueCount else {
                 logger?.debug(message: "event count is less than threshold - current: \(currentEvents.count), threshold: \(eventsMaxBatchQueueCount)")
-                callback(.success(false))
+                completion?(.success(false))
                 return
             }
             let sendingEvents: [Event] = Array(currentEvents.prefix(eventsMaxBatchQueueCount))
@@ -260,21 +253,21 @@ final class EventInteractorImpl: EventInteractor {
                     do {
                         try self?.eventDao.delete(ids: deletedIds)
                         self?.updateEventsAndNotify()
-                        callback(.success(true))
+                        completion?(.success(true))
                     } catch let error {
-                        callback(.failure(BKTError(error: error)))
+                        completion?(.failure(BKTError(error: error)))
                     }
                 case .failure(let error):
                     do {
                         try self?.trackRegisterEventsFailure(error: error)
                     } catch let error {
                         self?.logger?.error(error)
                     }
-                    callback(.failure(BKTError(error: error)))
+                    completion?(.failure(BKTError(error: error)))
                 }
             }
         } catch let error {
-            callback(.failure(BKTError(error: error)))
+            completion?(.failure(BKTError(error: error)))
         }
     }
 
@@ -328,7 +321,8 @@ final class EventInteractorImpl: EventInteractor {
 
     private func updateEventsAndNotify() {
         do {
-            eventUpdateListener?.onUpdate(events: try eventDao.getEvents())
+            let events = try eventDao.getEvents()
+            eventUpdateListener?.onUpdate(events: events)
         } catch let error {
             logger?.error(error)
         }

Bucketeer/Sources/Internal/Remote/ApiClientImpl.swift

@@ -10,6 +10,13 @@ final class ApiClientImpl: ApiClient {
     private let session: Session
     private let defaultRequestTimeoutMills: Int64
     private let logger: Logger?
+    private let semaphore = DispatchSemaphore(value: 0)
+
+    deinit {
+        // over-signaling does not introduce new problems
+        // https://stackoverflow.com/questions/70457141/safe-to-signal-semaphore-before-deinitialization-just-in-case
+        semaphore.signal()
+    }
 
     init(
         apiEndpoint: URL,
@@ -92,13 +99,16 @@ final class ApiClientImpl: ApiClient {
         )
     }
 
+    // noted: this method will run `synchronized`. It will blocking the current queue please do not call it from the app main thread
     func send<RequestBody: Encodable, Response: Decodable>(
         requestBody: RequestBody,
         path: String,
         timeoutMillis: Int64,
         encoder: JSONEncoder = JSONEncoder(),
         completion: ((Result<(Response, URLResponse), Error>) -> Void)?) {
-
+        let requestId = Date().unixTimestamp
+        logger?.debug(message: "[API] RequestID enqueue: \(requestId)")
+        logger?.debug(message: "[API] Register events: \(requestBody)")
         do {
             if #available(iOS 11.0, *) {
                 encoder.outputFormatting = [encoder.outputFormatting, .prettyPrinted, .sortedKeys]
@@ -113,35 +123,57 @@ final class ApiClientImpl: ApiClient {
             ]
             request.httpBody = body
             request.timeoutInterval = TimeInterval(timeoutMillis) / 1000
-
-            session.task(with: request) { data, urlResponse, error in
+            // `result` shared resource between the network queue and the SDK queue
+            var result : Result<(Response, URLResponse), Error>?
+            let responseParser : (Data?, URLResponse?, Error?) -> Result<(Response, URLResponse), Error> = { data, urlResponse, error in
                 guard let data = data else {
                     guard let error = error else {
-                        completion?(.failure(ResponseError.unknown(urlResponse)))
-                        return
+                        return .failure(ResponseError.unknown(urlResponse))
                     }
-                    completion?(.failure(error))
-                    return
+                    return .failure(error)
                 }
 
                 guard let urlResponse = urlResponse as? HTTPURLResponse else {
-                    completion?(.failure(ResponseError.unknown(urlResponse)))
-                    return
+                    return .failure(ResponseError.unknown(urlResponse))
                 }
                 do {
                     guard 200..<300 ~= urlResponse.statusCode else {
                         let response: ErrorResponse? = try JSONDecoder().decode(ErrorResponse.self, from: data)
                         let error = ResponseError.unacceptableCode(code: urlResponse.statusCode, response: response)
-                        completion?(.failure(error))
-                        return
+                        return .failure(error)
                     }
                     let response = try JSONDecoder().decode(Response.self, from: data)
-                    completion?(.success((response, urlResponse)))
+                    return .success((response, urlResponse))
                 } catch let error {
-                    completion?(.failure(error))
+                    return .failure(error)
                 }
             }
+
+            let requestHandler : (Result<(Response, URLResponse), Error>) -> Void = { [weak self] data in
+                result = data
+                // unlock from network queue
+                self?.logger?.debug(message: "[API] Resource available")
+                self?.semaphore.signal()
+            }
+
+            session.task(with: request) { data, urlResponse, error in
+                let output = responseParser(data, urlResponse, error)
+                requestHandler(output)
+            }
+
+            logger?.debug(message: "[API] RequestID wait: \(requestId)")
+            semaphore.wait()
+            logger?.debug(message: "[API] RequestID finished: \(requestId)")
+            guard result != nil else {
+                completion?(.failure(BKTError.illegalState(message: "fail to handle the request result")))
+                return
+            }
+            completion?(result!)
         } catch let error {
+            // catch error may throw before sending the request
+            // runtime error will handle in the session callback
+            // so that we will not required call `semephore.signal()` here
+            logger?.debug(message: "[API] RequestID: \(requestId) could not request with error \(error.localizedDescription)")
             completion?(.failure(error))
         }
     }
@@ -166,3 +198,19 @@ private struct AnyKey: CodingKey {
         self.intValue = intValue
     }
 }
+
+private typealias UnixTimestamp = Int
+
+fileprivate extension Date {
+    /// Date to Unix timestamp.
+    var unixTimestamp: UnixTimestamp {
+        return UnixTimestamp(self.timeIntervalSince1970 * 1_000) // millisecond precision
+    }
+}
+
+fileprivate extension UnixTimestamp {
+    /// Unix timestamp to date.
+    var date: Date {
+        return Date(timeIntervalSince1970: TimeInterval(self / 1_000)) // must take a millisecond-precise Unix timestamp
+    }
+}

Bucketeer/Sources/Internal/Scheduler/EvaluationForegroundTask.swift

@@ -28,7 +28,9 @@ final class EvaluationForegroundTask: ScheduledTask {
             queue: queue,
             logger: component.config.logger,
             handler: { [weak self] _ in
-                self?.fetchEvaluations()
+                self?.queue.async {
+                    self?.fetchEvaluations()
+                }
             }
         )
         poller?.start()

Bucketeer/Sources/Internal/Scheduler/EventBackgroundTask.swift

@@ -38,10 +38,12 @@ final class EventBackgroundTask {
         scheduleAppRefresh()
 
         guard let component = self.component else { return }
-        BKTClient.flushSync(component: component) { [weak self] error in
-            task?.setTaskCompleted(success: error == nil)
-            if let error {
-                self?.component?.config.logger?.error(error)
+        queue.async {
+            BKTClient.flushSync(component: component) { [weak self] error in
+                task?.setTaskCompleted(success: error == nil)
+                if let error {
+                    self?.component?.config.logger?.error(error)
+                }
             }
         }
         // Provide the background task with an expiration handler that cancels the operation.

Bucketeer/Sources/Internal/Scheduler/EventForegroundTask.swift

@@ -18,7 +18,9 @@ final class EventForegroundTask {
             queue: queue,
             logger: component.config.logger,
             handler: { [weak self] _ in
-                self?.component?.eventInteractor.sendEvents(force: true, completion: nil)
+                self?.queue.async {
+                    self?.component?.eventInteractor.sendEvents(force: true, completion: nil)
+                }
             }
         )
         poller?.start()
@@ -40,11 +42,13 @@ extension EventForegroundTask: ScheduledTask {
 
 extension EventForegroundTask: EventUpdateListener {
     func onUpdate(events: [Event]) {
-        component?.eventInteractor.sendEvents(force: false) { [weak self] result in
-            guard case .success(let success) = result, success else {
-                return
+        queue.async { [weak self] in
+            self?.component?.eventInteractor.sendEvents(force: false) { result in
+                guard case .success(let success) = result, success else {
+                    return
+                }
+                self?.reschedule()
             }
-            self?.reschedule()
         }
     }
 }

Bucketeer/Sources/Internal/Scheduler/TaskScheduler.swift

@@ -27,7 +27,7 @@ final class TaskScheduler {
             NotificationCenter.default.addObserver(
                 self,
                 selector: #selector(onForeground),
-                name: UIScene.willConnectNotification,
+                name: UIScene.didActivateNotification,
                 object: nil
             )
             NotificationCenter.default.addObserver(
@@ -39,7 +39,7 @@ final class TaskScheduler {
             NotificationCenter.default.addObserver(
                 self,
                 selector: #selector(onBackground),
-                name: UIScene.didEnterBackgroundNotification,
+                name: UIScene.willDeactivateNotification,
                 object: nil
             )
         } else {
@@ -58,7 +58,7 @@ final class TaskScheduler {
             NotificationCenter.default.addObserver(
                 self,
                 selector: #selector(onBackground),
-                name: UIApplication.didEnterBackgroundNotification,
+                name: UIApplication.willResignActiveNotification,
                 object: nil
             )
         }