Merge pull request #21232 from wordpress-mobile/fix/core-data-violati…

…on-reminders

Fix Core Data threading violation when scheduling reminders

RELEASE-NOTES.txt

@@ -1,6 +1,6 @@
 23.1
 -----
-
+* [*] [internal] Fix Core Data multithreaded access exception in Blogging Reminders [#21232]
 
 23.0
 -----

WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift

@@ -92,9 +92,7 @@ class BloggingRemindersScheduler {
     ///
     private let pushNotificationAuthorizer: PushNotificationAuthorizer
 
-    private var coreDataStack: CoreDataStackSwift {
-        ContextManager.shared
-    }
+    private let coreDataStack: CoreDataStackSwift
 
     /// The time of the day when blogging reminders will be received for the given blog
     /// - Parameter blog: the given blog
@@ -238,11 +236,13 @@ class BloggingRemindersScheduler {
     init(
         store: BloggingRemindersStore,
         notificationCenter: NotificationScheduler = UNUserNotificationCenter.current(),
-        pushNotificationAuthorizer: PushNotificationAuthorizer = InteractiveNotificationsManager.shared) {
-
-        self.store = store
-        self.notificationScheduler = notificationCenter
-        self.pushNotificationAuthorizer = pushNotificationAuthorizer
+        pushNotificationAuthorizer: PushNotificationAuthorizer = InteractiveNotificationsManager.shared,
+        coreDataStack: CoreDataStackSwift = ContextManager.shared
+    ) {
+            self.store = store
+            self.notificationScheduler = notificationCenter
+            self.pushNotificationAuthorizer = pushNotificationAuthorizer
+            self.coreDataStack = coreDataStack
     }
 
     /// Default initializer.  Allows overriding the blogging reminders store and the notification center for testing purposes.
@@ -254,11 +254,13 @@ class BloggingRemindersScheduler {
     ///
     init(
         notificationCenter: NotificationScheduler = UNUserNotificationCenter.current(),
-        pushNotificationAuthorizer: PushNotificationAuthorizer = InteractiveNotificationsManager.shared) throws {
-
+        pushNotificationAuthorizer: PushNotificationAuthorizer = InteractiveNotificationsManager.shared,
+        coreDataStack: CoreDataStackSwift = ContextManager.shared
+    ) throws {
         self.store = try Self.defaultStore()
         self.notificationScheduler = notificationCenter
         self.pushNotificationAuthorizer = pushNotificationAuthorizer
+        self.coreDataStack = coreDataStack
     }
 
     // MARK: - Scheduling
@@ -276,17 +278,21 @@ class BloggingRemindersScheduler {
             return
         }
 
+        let blogObjectID = blog.objectID
         pushNotificationAuthorizer.requestAuthorization { [weak self] allowed in
-            guard let self = self else {
-                return
-            }
-
-            guard allowed else {
-                completion(.failure(Error.needsPermissionForPushNotifications))
-                return
+            DispatchQueue.main.async {
+                guard let self else { return }
+                guard allowed else {
+                    completion(.failure(Error.needsPermissionForPushNotifications))
+                    return
+                }
+                do {
+                    let blog = try self.coreDataStack.mainContext.existingObject(with: blogObjectID) as! Blog
+                    self.pushAuthorizationReceived(blog: blog, schedule: schedule, time: time, completion: completion)
+                } catch {
+                    completion(.failure(error))
+                }
             }
-
-            self.pushAuthorizationReceived(blog: blog, schedule: schedule, time: time, completion: completion)
         }
     }
 

WordPress/WordPressTest/BloggingRemindersSchedulerTests.swift

@@ -73,10 +73,15 @@ class BloggingRemindersSchedulerTests: XCTestCase {
         let scheduler = BloggingRemindersScheduler(
             store: store,
             notificationCenter: notificationCenter,
-            pushNotificationAuthorizer: PushNotificationsAuthorizerMock())
+            pushNotificationAuthorizer: PushNotificationsAuthorizerMock(),
+            coreDataStack: contextManager
+        )
 
+        let expectation = self.expectation(description: "schedule-updated")
         scheduler.schedule(schedule, for: blog) { _ in
+            expectation.fulfill()
         }
+        wait(for: [expectation], timeout: 1)
 
         XCTAssertEqual(scheduler.schedule(for: blog), schedule)
     }
@@ -114,7 +119,9 @@ class BloggingRemindersSchedulerTests: XCTestCase {
         let scheduler = BloggingRemindersScheduler(
             store: store,
             notificationCenter: notificationCenter,
-            pushNotificationAuthorizer: PushNotificationsAuthorizerMock())
+            pushNotificationAuthorizer: PushNotificationsAuthorizerMock(),
+            coreDataStack: contextManager
+        )
 
         let storeHasRemindersExpectation = expectation(description: "The notifications are in the store")
         let storeIsEmptyExpectation = expectation(description: "The notifications have been cleared from the store")
@@ -123,15 +130,14 @@ class BloggingRemindersSchedulerTests: XCTestCase {
             if store.scheduledReminders(for: blog.objectID.uriRepresentation()) != .none {
                 storeHasRemindersExpectation.fulfill()
             }
-        }
-
-        scheduler.schedule(.none, for: blog) { _ in
-            if store.scheduledReminders(for: blog.objectID.uriRepresentation()) == .none {
-                storeIsEmptyExpectation.fulfill()
+            scheduler.schedule(.none, for: blog) { _ in
+                if store.scheduledReminders(for: blog.objectID.uriRepresentation()) == .none {
+                    storeIsEmptyExpectation.fulfill()
+                }
             }
         }
 
-        waitForExpectations(timeout: 0.1) { error in
+        waitForExpectations(timeout: 1) { error in
             if let error = error {
                 XCTFail(error.localizedDescription)
             }