makes StorageBackgroundEventsRegistry work as a singleton so the acto…

…r protects state
aws-amplify · Oct 24, 2022 · 175dc7d · 175dc7d
1 parent fbde41b
commit 175dc7d
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 27 deletions.
diff --git a/...fyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/...fyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift
@@ -120,7 +120,7 @@ extension AWSS3StoragePlugin {
     }
 
     public func handleEventsForBackgroundURLSession(identifier: String) async -> Bool {
-        await StorageBackgroundEventsRegistry.handleEventsForBackgroundURLSession(identifier: identifier)
+        await StorageBackgroundEventsRegistry.shared.handleEventsForBackgroundURLSession(identifier: identifier)
     }
 
 }
diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService.swift
@@ -108,7 +108,9 @@ class AWSS3StorageService: AWSS3StorageServiceBehaviour, StorageServiceProxy {
         self.awsS3 = awsS3
         self.bucket = bucket
 
-        StorageBackgroundEventsRegistry.register(identifier: identifier)
+        Task {
+            await StorageBackgroundEventsRegistry.shared.register(identifier: identifier)
+        }
 
         delegate.storageService = self
 
@@ -125,7 +127,9 @@ class AWSS3StorageService: AWSS3StorageServiceBehaviour, StorageServiceProxy {
     }
 
     deinit {
-        StorageBackgroundEventsRegistry.unregister(identifier: identifier)
+        Task {
+            await StorageBackgroundEventsRegistry.shared.unregister(identifier: identifier)
+        }
     }
 
     func reset() {

diff --git a/...Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageBackgroundEventsRegistry.swift b/...Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageBackgroundEventsRegistry.swift
@@ -18,18 +18,26 @@ extension Notification.Name {
 ///    independently of the Amplify Storage plugin and this function will indiciate if it will handle the given identifier.
 actor StorageBackgroundEventsRegistry {
     typealias StorageBackgroundEventsContinuation = CheckedContinuation<Bool, Never>
-    static var identifier: String?
-    static var continuation: StorageBackgroundEventsContinuation?
+
+    @MainActor
+    static let shared = StorageBackgroundEventsRegistry()
+
+    private var identifier: String?
+    private var continuation: StorageBackgroundEventsContinuation?
 
     // override for use with unit tests
-    static var notificationCenter: NotificationCenter?
+    internal private(set) var notificationCenter: NotificationCenter?
+
+    func change(notificationCenter: NotificationCenter?) {
+        self.notificationCenter = notificationCenter
+    }
 
     /// Handles background events for URLSession on iOS.
     /// - Parameters:
     ///   - identifier: session identifier
     ///   - completionHandler: completion handler
     /// - Returns: indicates if the identifier was registered and will be handled
-    static func handleEventsForBackgroundURLSession(identifier: String) async -> Bool {
+    func handleEventsForBackgroundURLSession(identifier: String) async -> Bool {
         guard self.identifier == identifier else { return false }
 
         return await withCheckedContinuation { (continuation: CheckedContinuation<Bool, Never>) in
@@ -41,29 +49,29 @@ actor StorageBackgroundEventsRegistry {
     /// Notifies observes when waiting for continuation to be resumed.
     /// - Parameters:
     ///   - identifier: session identifier
-    private static func notifyWaiting(for identifier: String) {
+    private func notifyWaiting(for identifier: String) {
         notificationCenter?.post(name: Notification.Name.StorageBackgroundEventsRegistryWaiting, object: identifier)
     }
 
     // The storage plugin will register the session identifier when it is configured.
-    static func register(identifier: String) {
+    func register(identifier: String) {
         self.identifier = identifier
     }
 
     // When the storage function is deinitialized it will unregister the session identifier.
-    static func unregister(identifier: String) {
+    func unregister(identifier: String) {
         if self.identifier == identifier {
             self.identifier = nil
         }
     }
 
     // When URLSession is done processing background events it will use this function to get the completion handler.
-    static func getContinuation(for identifier: String) -> StorageBackgroundEventsContinuation? {
+    func getContinuation(for identifier: String) -> StorageBackgroundEventsContinuation? {
         self.identifier == identifier ? continuation : nil
     }
 
     // Once the background event completion handler is used it can be cleared.
-    static func removeContinuation(for identifier: String) {
+    func removeContinuation(for identifier: String) {
         if self.identifier == identifier {
             self.continuation = nil
         }

diff --git a/...s/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageServiceSessionDelegate.swift b/...s/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageServiceSessionDelegate.swift
@@ -53,13 +53,15 @@ extension StorageServiceSessionDelegate: URLSessionDelegate {
     func urlSessionDidFinishEvents(forBackgroundURLSession session: URLSession) {
         logURLSessionActivity("Session did finish background events")
 
-        if let identifier = session.configuration.identifier,
-           let continuation = StorageBackgroundEventsRegistry.getContinuation(for: identifier) {
-            // Must be run on main thread as covered by Apple Developer docs.
-            Task { @MainActor in
-                continuation.resume(returning: true)
+        Task {
+            if let identifier = session.configuration.identifier,
+               let continuation = await StorageBackgroundEventsRegistry.shared.getContinuation(for: identifier) {
+                // Must be run on main thread as covered by Apple Developer docs.
+                Task { @MainActor in
+                    continuation.resume(returning: true)
+                }
+                await StorageBackgroundEventsRegistry.shared.removeContinuation(for: identifier)
             }
-            StorageBackgroundEventsRegistry.removeContinuation(for: identifier)
         }
     }
 

diff --git a/...Tests/AWSS3StoragePluginTests/Support/Internal/StorageBackgroundEventsRegistryTests.swift b/...Tests/AWSS3StoragePluginTests/Support/Internal/StorageBackgroundEventsRegistryTests.swift
@@ -16,12 +16,14 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
     func testRegisteringAndUnregister() async throws {
         let identifier = UUID().uuidString
         let otherIdentifier = UUID().uuidString
-        StorageBackgroundEventsRegistry.register(identifier: identifier)
+        await StorageBackgroundEventsRegistry.shared.register(identifier: identifier)
 
         let notificationCenter = NotificationCenter()
-        StorageBackgroundEventsRegistry.notificationCenter = notificationCenter
+        await StorageBackgroundEventsRegistry.shared.change(notificationCenter: notificationCenter)
         defer {
-            StorageBackgroundEventsRegistry.notificationCenter = nil
+            Task {
+                await StorageBackgroundEventsRegistry.shared.change(notificationCenter: nil)
+            }
         }
 
         let done = asyncExpectation(description: "done")
@@ -39,7 +41,7 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
         }
 
         Task {
-            let handled = await StorageBackgroundEventsRegistry.handleEventsForBackgroundURLSession(identifier: identifier)
+            let handled = await StorageBackgroundEventsRegistry.shared.handleEventsForBackgroundURLSession(identifier: identifier)
             await done.fulfill()
             XCTAssertTrue(handled)
         }
@@ -53,7 +55,7 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
         let otherDone = asyncExpectation(description: "other done")
 
         Task {
-            let otherHandled = await StorageBackgroundEventsRegistry.handleEventsForBackgroundURLSession(identifier: otherIdentifier)
+            let otherHandled = await StorageBackgroundEventsRegistry.shared.handleEventsForBackgroundURLSession(identifier: otherIdentifier)
             await otherDone.fulfill()
             XCTAssertFalse(otherHandled)
         }
@@ -66,12 +68,12 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
     func testHandlingUnregisteredIdentifier() async throws {
         let identifier = UUID().uuidString
         let otherIdentifier = UUID().uuidString
-        StorageBackgroundEventsRegistry.register(identifier: otherIdentifier)
+        await StorageBackgroundEventsRegistry.shared.register(identifier: otherIdentifier)
 
         let done = asyncExpectation(description: "done")
 
         Task {
-            let handled = await StorageBackgroundEventsRegistry.handleEventsForBackgroundURLSession(identifier: identifier)
+            let handled = await StorageBackgroundEventsRegistry.shared.handleEventsForBackgroundURLSession(identifier: identifier)
             await done.fulfill()
             XCTAssertFalse(handled)
         }
@@ -83,9 +85,9 @@ class StorageBackgroundEventsRegistryTests: XCTestCase {
     func handleEvents(for identifier: String) async -> Bool {
         await Task.yield()
 
-        if let continuation = StorageBackgroundEventsRegistry.getContinuation(for: identifier) {
+        if let continuation = await StorageBackgroundEventsRegistry.shared.getContinuation(for: identifier) {
             continuation.resume(returning: true)
-            StorageBackgroundEventsRegistry.removeContinuation(for: identifier)
+            await StorageBackgroundEventsRegistry.shared.removeContinuation(for: identifier)
             return true
         } else {
             print("No continuation for identifier: \(identifier)")