From f87e49fb2275417e6cfc40f520599e94f013276d Mon Sep 17 00:00:00 2001 From: kean Date: Wed, 2 Aug 2023 15:41:58 -0400 Subject: [PATCH 1/4] Fix core data threading violation when scheduling reminders --- .../BloggingRemindersScheduler.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift index feaa744b0ff2..98aceda6147c 100644 --- a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift +++ b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift @@ -277,16 +277,16 @@ class BloggingRemindersScheduler { } 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 = self else { + return + } + guard allowed else { + completion(.failure(Error.needsPermissionForPushNotifications)) + return + } + self.pushAuthorizationReceived(blog: blog, schedule: schedule, time: time, completion: completion) } - - self.pushAuthorizationReceived(blog: blog, schedule: schedule, time: time, completion: completion) } } From 47e9447fd72bf1d7305d5d91a342000481f8a9ee Mon Sep 17 00:00:00 2001 From: kean Date: Fri, 4 Aug 2023 13:20:44 -0400 Subject: [PATCH 2/4] Reload blog on the main thread/context --- .../BloggingRemindersScheduler.swift | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift index 98aceda6147c..dd643994d55f 100644 --- a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift +++ b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift @@ -17,7 +17,7 @@ extension InteractiveNotificationsManager: PushNotificationAuthorizer { /// Main interface for scheduling blogging reminders /// -class BloggingRemindersScheduler { +final class BloggingRemindersScheduler { // MARK: - Convenience Typealiases @@ -276,16 +276,20 @@ class BloggingRemindersScheduler { return } + let blogObjectID = blog.objectID pushNotificationAuthorizer.requestAuthorization { [weak self] allowed in DispatchQueue.main.async { - guard let self = self else { - return - } + guard let self else { return } guard allowed else { completion(.failure(Error.needsPermissionForPushNotifications)) return } - self.pushAuthorizationReceived(blog: blog, schedule: schedule, time: time, completion: completion) + 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)) + } } } } From 456b5e3bf48db09e569efc925ba5dcdb499d4ba2 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 8 Aug 2023 08:35:46 -0400 Subject: [PATCH 3/4] Update release notes --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 8d4e98c0dc5e..857db87e1e7c 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,6 +1,6 @@ 23.1 ----- - +* [*] [internal] Fix Core Data multithreaded access exception in Blogging Reminders [#21232] 23.0 ----- From 35a553ad570f0b741ba715dcacd8e9cae10c2202 Mon Sep 17 00:00:00 2001 From: kean Date: Tue, 8 Aug 2023 10:27:57 -0400 Subject: [PATCH 4/4] Fix unit tests --- .../BloggingRemindersScheduler.swift | 24 ++++++++++--------- .../BloggingRemindersSchedulerTests.swift | 22 ++++++++++------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift index dd643994d55f..a2f91bc5bc0f 100644 --- a/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift +++ b/WordPress/Classes/Utility/Blogging Reminders/BloggingRemindersScheduler.swift @@ -17,7 +17,7 @@ extension InteractiveNotificationsManager: PushNotificationAuthorizer { /// Main interface for scheduling blogging reminders /// -final class BloggingRemindersScheduler { +class BloggingRemindersScheduler { // MARK: - Convenience Typealiases @@ -92,9 +92,7 @@ final 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 @@ final 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 @@ final 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 diff --git a/WordPress/WordPressTest/BloggingRemindersSchedulerTests.swift b/WordPress/WordPressTest/BloggingRemindersSchedulerTests.swift index eb3957247b13..619a9461597a 100644 --- a/WordPress/WordPressTest/BloggingRemindersSchedulerTests.swift +++ b/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) }