fix callback issue

customerio · Aug 31, 2023 · 671b636 · 671b636
1 parent c34637f
commit 671b636
Show file tree

Hide file tree

Showing 23 changed files with 68 additions and 92 deletions.
diff --git a/Sources/Common/Background Queue/Queue.swift b/Sources/Common/Background Queue/Queue.swift
@@ -171,9 +171,13 @@ public class CioQueue: Queue {
     }
 
     public func run(onComplete: @escaping () -> Void) {
-        logger.info("queue run request sent")
+        // not using [weak self] to assert that the queue will complete and callback once started.
+        // this might keep this class in memory and not get garbage collected once customer is done using it
+        // but it will get released once the queue is done running.
 
-        runRequest.start(onComplete: onComplete)
+        runRequest.start {
+            self.logger.info("queue completed all tasks")
+        }
     }
 
     /// We determine the queue needs to run by (1) if there are many tasks in the queue
@@ -190,10 +194,7 @@ public class CioQueue: Queue {
             // cancel timer if one running since we will run the queue now
             queueTimer.cancel()
 
-            // not using [weak self] to assert that the queue will complete and callback once started.
-            // this might keep this class in memory and not get garbage collected once customer is done using it
-            // but it will get released once the queue is done running.
-            runRequest.start {
+            run {
                 self.logger.info("queue completed all tasks")
             }
         } else {

diff --git a/Sources/Common/Background Queue/QueueRequestManager.swift b/Sources/Common/Background Queue/QueueRequestManager.swift
@@ -8,43 +8,28 @@ import Foundation
  running at one time to prevent race conditions and tasks running multiple times.
 
  This class is small and separate from the rest of the queue logic for some readability/scalability value but
- mostly memory safety.
-
- We want to avoid making our queue classes singletons because these classes may have lots of
- dependencies inside of them (especially the runner). We want to avoid keeping all of these dependencies sitting in
- memory.
+ mostly memory safety by keeping dependencies low in class (prefer none).
  */
 public protocol QueueRequestManager: AutoMockable {
     /// call when a runner run request is complete
     func requestComplete()
-    /// call when a new run request is requested. adds callback to list of callbacks
-    /// to call when run request is done running.
-    /// returns is an existing run request is currently running or not.
-    func startRequest(onComplete: @escaping () -> Void) -> Bool
+    /// call when a new run request is requested.
+    /// returns `true` if the queue is already running.
+    func startIfNotAlready() -> Bool
 }
 
 // sourcery: InjectRegister = "QueueRequestManager"
 // sourcery: InjectSingleton
 public class CioQueueRequestManager: QueueRequestManager {
     @Atomic var isRunningRequest = false
-    @Atomic var callbacks: [() -> Void] = []
 
     public func requestComplete() {
-        let existingCallbacks = callbacks
-
-        callbacks = []
         isRunningRequest = false
-
-        existingCallbacks.forEach { callback in
-            callback()
-        }
     }
 
-    public func startRequest(onComplete: @escaping () -> Void) -> Bool {
+    public func startIfNotAlready() -> Bool {
         let isQueueRunningARequest = isRunningRequest
 
-        callbacks.append(onComplete)
-
         if !isQueueRunningARequest {
             isRunningRequest = true
         }

diff --git a/Sources/Common/Background Queue/QueueRunRequest.swift b/Sources/Common/Background Queue/QueueRunRequest.swift
@@ -16,6 +16,8 @@ public class CioQueueRunRequest: QueueRunRequest {
 
     private let shortTaskId: (String) -> String = { $0[0 ..< 5] }
 
+    private var onCompleteCallback: (() -> Void)?
+
     init(
         runner: QueueRunner,
         storage: QueueStorage,
@@ -33,9 +35,10 @@ public class CioQueueRunRequest: QueueRunRequest {
     }
 
     public func start(onComplete: @escaping () -> Void) {
-        let isRequestCurrentlyRunning = requestManager.startRequest(onComplete: onComplete)
+        let isRequestCurrentlyRunning = requestManager.startIfNotAlready()
 
         if !isRequestCurrentlyRunning {
+            onCompleteCallback = onComplete
             startNewRequestRun()
         }
     }
@@ -68,6 +71,8 @@ public class CioQueueRunRequest: QueueRunRequest {
 
             queryRunner.reset()
 
+            onCompleteCallback?()
+
             return requestManager.requestComplete()
         }
 

diff --git a/Sources/Common/autogenerated/AutoDependencyInjection.generated.swift b/Sources/Common/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/Common/autogenerated/AutoLenses.generated.swift b/Sources/Common/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/Common/autogenerated/AutoMockable.generated.swift b/Sources/Common/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 
@@ -1871,9 +1871,7 @@ public class QueueRequestManagerMock: QueueRequestManager, Mock {
         requestCompleteCallsCount = 0
 
         mockCalled = false // do last as resetting properties above can make this true
-        startRequestCallsCount = 0
-        startRequestReceivedArguments = nil
-        startRequestReceivedInvocations = []
+        startIfNotAlreadyCallsCount = 0
 
         mockCalled = false // do last as resetting properties above can make this true
     }
@@ -1899,35 +1897,29 @@ public class QueueRequestManagerMock: QueueRequestManager, Mock {
         requestCompleteClosure?()
     }
 
-    // MARK: - startRequest
+    // MARK: - startIfNotAlready
 
     /// Number of times the function was called.
-    public private(set) var startRequestCallsCount = 0
+    public private(set) var startIfNotAlreadyCallsCount = 0
     /// `true` if the function was ever called.
-    public var startRequestCalled: Bool {
-        startRequestCallsCount > 0
+    public var startIfNotAlreadyCalled: Bool {
+        startIfNotAlreadyCallsCount > 0
     }
 
-    /// The arguments from the *last* time the function was called.
-    public private(set) var startRequestReceivedArguments: (() -> Void)?
-    /// Arguments from *all* of the times that the function was called.
-    public private(set) var startRequestReceivedInvocations: [() -> Void] = []
     /// Value to return from the mocked function.
-    public var startRequestReturnValue: Bool!
+    public var startIfNotAlreadyReturnValue: Bool!
     /**
      Set closure to get called when function gets called. Great way to test logic or return a value for the function.
      The closure has first priority to return a value for the mocked function. If the closure returns `nil`,
-     then the mock will attempt to return the value for `startRequestReturnValue`
+     then the mock will attempt to return the value for `startIfNotAlreadyReturnValue`
      */
-    public var startRequestClosure: ((@escaping () -> Void) -> Bool)?
+    public var startIfNotAlreadyClosure: (() -> Bool)?
 
-    /// Mocked function for `startRequest(onComplete: @escaping () -> Void)`. Your opportunity to return a mocked value and check result of mock in test code.
-    public func startRequest(onComplete: @escaping () -> Void) -> Bool {
+    /// Mocked function for `startIfNotAlready()`. Your opportunity to return a mocked value and check result of mock in test code.
+    public func startIfNotAlready() -> Bool {
         mockCalled = true
-        startRequestCallsCount += 1
-        startRequestReceivedArguments = onComplete
-        startRequestReceivedInvocations.append(onComplete)
-        return startRequestClosure.map { $0(onComplete) } ?? startRequestReturnValue
+        startIfNotAlreadyCallsCount += 1
+        return startIfNotAlreadyClosure.map { $0() } ?? startIfNotAlreadyReturnValue
     }
 }
 

diff --git a/Sources/MessagingInApp/autogenerated/AutoDependencyInjection.generated.swift b/Sources/MessagingInApp/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingInApp/autogenerated/AutoLenses.generated.swift b/Sources/MessagingInApp/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingInApp/autogenerated/AutoMockable.generated.swift b/Sources/MessagingInApp/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPush/autogenerated/AutoDependencyInjection.generated.swift b/Sources/MessagingPush/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPush/autogenerated/AutoLenses.generated.swift b/Sources/MessagingPush/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPush/autogenerated/AutoMockable.generated.swift b/Sources/MessagingPush/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushAPN/autogenerated/AutoDependencyInjection.generated.swift b/Sources/MessagingPushAPN/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushAPN/autogenerated/AutoLenses.generated.swift b/Sources/MessagingPushAPN/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushAPN/autogenerated/AutoMockable.generated.swift b/Sources/MessagingPushAPN/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushFCM/autogenerated/AutoDependencyInjection.generated.swift b/Sources/MessagingPushFCM/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushFCM/autogenerated/AutoLenses.generated.swift b/Sources/MessagingPushFCM/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/MessagingPushFCM/autogenerated/AutoMockable.generated.swift b/Sources/MessagingPushFCM/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/Tracking/autogenerated/AutoDependencyInjection.generated.swift b/Sources/Tracking/autogenerated/AutoDependencyInjection.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/Tracking/autogenerated/AutoLenses.generated.swift b/Sources/Tracking/autogenerated/AutoLenses.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Sources/Tracking/autogenerated/AutoMockable.generated.swift b/Sources/Tracking/autogenerated/AutoMockable.generated.swift
@@ -1,4 +1,4 @@
-// Generated using Sourcery 2.0.2 — https://github.com/krzysztofzablocki/Sourcery
+// Generated using Sourcery 2.0.3 — https://github.com/krzysztofzablocki/Sourcery
 // DO NOT EDIT
 // swiftlint:disable all
 

diff --git a/Tests/Common/Background Queue/QueueRequestManagerTest.swift b/Tests/Common/Background Queue/QueueRequestManagerTest.swift
@@ -19,42 +19,35 @@ class QueueRequestManagerTest: UnitTest {
 
         manager.requestComplete()
 
-        let actual = manager.startRequest {}
+        let actual = manager.startIfNotAlready()
         XCTAssertFalse(actual)
     }
 
-    func test_requestComplete_expectCallCallbacksComplete() {
-        var callbackCalled = false
-        let givenCallback: () -> Void = {
-            callbackCalled = true
-        }
-        manager.callbacks = [givenCallback]
-
-        manager.requestComplete()
-
-        XCTAssertTrue(callbackCalled)
-    }
-
-    // MARK: startRequest
+    // move to another test class?
+//    func test_requestComplete_expectCallCallbacksComplete() {
+//        var callbackCalled = false
+//        let givenCallback: () -> Void = {
+//            callbackCalled = true
+//        }
+//        manager.callbacks = [givenCallback]
+//
+//        manager.requestComplete()
+//
+//        XCTAssertTrue(callbackCalled)
+//    }
+
+    // MARK: startIfNotAlready
 
     func test_startRequest_givenNotRunningARequest_expectReturnFalse() {
-        let actual = manager.startRequest {}
+        let actual = manager.startIfNotAlready()
 
         XCTAssertFalse(actual)
     }
 
     func test_startRequest_givenRunningARequest_expectReturnTrue() {
         manager.isRunningRequest = true
 
-        let actual = manager.startRequest {}
+        let actual = manager.startIfNotAlready()
         XCTAssertTrue(actual)
     }
-
-    func test_startRequest_expectAddCallback() {
-        XCTAssertEqual(manager.callbacks.count, 0)
-
-        _ = manager.startRequest {}
-
-        XCTAssertEqual(manager.callbacks.count, 1)
-    }
 }
diff --git a/Tests/Common/Background Queue/QueueRunRequestTest.swift b/Tests/Common/Background Queue/QueueRunRequestTest.swift
@@ -33,7 +33,7 @@ class QueueRunRequestTest: UnitTest {
         storageMock.deleteReturnValue = true
 
         // Not already running a queue run request. When you want to start a new one, start it.
-        requestManagerMock.startRequestReturnValue = false
+        requestManagerMock.startIfNotAlreadyReturnValue = false
     }
 
     // our indictor if run request is running the queue
@@ -44,15 +44,15 @@ class QueueRunRequestTest: UnitTest {
     // MARK: start
 
     func test_start_givenAlreadyRunningARequest_expectDoNotStartNewRun() {
-        requestManagerMock.startRequestReturnValue = true
+        requestManagerMock.startIfNotAlreadyReturnValue = true
 
         runRequest.start {}
 
         XCTAssertFalse(didStartARun)
     }
 
     func test_start_givenNotAlreadyRunningRequest_expectStartNewRun() {
-        requestManagerMock.startRequestReturnValue = false
+        requestManagerMock.startIfNotAlreadyReturnValue = false
         storageMock.getInventoryReturnValue = []
 
         runRequest.start {}